From baf389786d98e6dc01c91af6fc60dea56554ae1c Mon Sep 17 00:00:00 2001 From: BeaconCat Date: Mon, 13 Apr 2026 11:19:57 +0800 Subject: [PATCH] fix: address Copilot review round 3 - ValidF1Count=0 when all scores are sentinel (no forced =1) - Backward compat: old eval JSON without ValidF1Count falls back to TotalQuestions in computeModeAgg - Skip empty section in PrintComparison when tokenResults is empty - Update --api-base flag help to document /v1 default and version path - Add sentinel aggregation unit tests (partial, all, weighted) --- cmd/membench/eval.go | 30 ++++++++++----- cmd/membench/eval_test.go | 78 +++++++++++++++++++++++++++++++++++++++ cmd/membench/main.go | 6 ++- 3 files changed, 103 insertions(+), 11 deletions(-) diff --git a/cmd/membench/eval.go b/cmd/membench/eval.go index 337f83191..44a5c2764 100644 --- a/cmd/membench/eval.go +++ b/cmd/membench/eval.go @@ -236,9 +236,6 @@ func aggregateMetrics(qaResults []QAResult) AggMetrics { if nHit == 0 { nHit = 1 } - if validF1Count == 0 { - validF1Count = 1 - } byCat := map[int]*CatMetrics{} for cat, acc := range byCatAcc { cm := &CatMetrics{ @@ -253,8 +250,12 @@ func aggregateMetrics(qaResults []QAResult) AggMetrics { } byCat[cat] = cm } + var overallF1 float64 + if validF1Count > 0 { + overallF1 = totalF1 / float64(validF1Count) + } return AggMetrics{ - OverallF1: totalF1 / float64(validF1Count), + OverallF1: overallF1, OverallHitRate: totalHitRate / float64(nHit), ByCategory: byCat, TotalQuestions: len(qaResults), @@ -304,20 +305,29 @@ func SaveAggregated(results []EvalResult, outDir string) error { func computeModeAgg(results []EvalResult) AggMetrics { agg := AggMetrics{ByCategory: map[int]*CatMetrics{}} for _, r := range results { - agg.OverallF1 += r.Agg.OverallF1 * float64(r.Agg.ValidF1Count) + // Backward compat: old eval JSON without ValidF1Count → use TotalQuestions. + vf1 := r.Agg.ValidF1Count + if vf1 == 0 && r.Agg.TotalQuestions > 0 { + vf1 = r.Agg.TotalQuestions + } + agg.OverallF1 += r.Agg.OverallF1 * float64(vf1) agg.OverallHitRate += r.Agg.OverallHitRate * float64(r.Agg.TotalQuestions) agg.TotalQuestions += r.Agg.TotalQuestions - agg.ValidF1Count += r.Agg.ValidF1Count + agg.ValidF1Count += vf1 for cat, cm := range r.Agg.ByCategory { existing, ok := agg.ByCategory[cat] if !ok { existing = &CatMetrics{} agg.ByCategory[cat] = existing } - existing.F1 += cm.F1 * float64(cm.ValidF1Count) + cvf1 := cm.ValidF1Count + if cvf1 == 0 && cm.QuestionCount > 0 { + cvf1 = cm.QuestionCount + } + existing.F1 += cm.F1 * float64(cvf1) existing.HitRate += cm.HitRate * float64(cm.QuestionCount) existing.QuestionCount += cm.QuestionCount - existing.ValidF1Count += cm.ValidF1Count + existing.ValidF1Count += cvf1 } } if agg.ValidF1Count > 0 { @@ -392,7 +402,9 @@ func printSection(title string, results []EvalResult) { // PrintComparison outputs a human-readable comparison table to stdout. func PrintComparison(results []EvalResult, llmResults []EvalResult) { - printSection("No LLM generation", results) + if len(results) > 0 { + printSection("No LLM generation", results) + } if len(llmResults) > 0 { printSection("With LLM", llmResults) } diff --git a/cmd/membench/eval_test.go b/cmd/membench/eval_test.go index d500a38ca..32dea07c9 100644 --- a/cmd/membench/eval_test.go +++ b/cmd/membench/eval_test.go @@ -102,3 +102,81 @@ func TestComputeModeAgg(t *testing.T) { t.Errorf("TotalQuestions = %d, want 10", got.TotalQuestions) } } + +func TestAggregateMetricsSentinel(t *testing.T) { + qa := []QAResult{ + {Category: 1, TokenF1: 0.8, HitRate: 0.5}, + {Category: 1, TokenF1: -1.0, HitRate: 0.3}, + {Category: 1, TokenF1: 0.4, HitRate: 0.7}, + } + agg := aggregateMetrics(qa) + + if agg.ValidF1Count != 2 { + t.Errorf("ValidF1Count = %d, want 2", agg.ValidF1Count) + } + if agg.TotalQuestions != 3 { + t.Errorf("TotalQuestions = %d, want 3", agg.TotalQuestions) + } + wantF1 := (0.8 + 0.4) / 2.0 + if math.Abs(agg.OverallF1-wantF1) > 1e-9 { + t.Errorf("OverallF1 = %.6f, want %.6f", agg.OverallF1, wantF1) + } + wantHR := (0.5 + 0.3 + 0.7) / 3.0 + if math.Abs(agg.OverallHitRate-wantHR) > 1e-9 { + t.Errorf("OverallHitRate = %.6f, want %.6f", agg.OverallHitRate, wantHR) + } +} + +func TestAggregateMetricsAllSentinel(t *testing.T) { + qa := []QAResult{ + {Category: 1, TokenF1: -1.0, HitRate: 0.5}, + {Category: 1, TokenF1: -1.0, HitRate: 0.3}, + } + agg := aggregateMetrics(qa) + + if agg.ValidF1Count != 0 { + t.Errorf("ValidF1Count = %d, want 0", agg.ValidF1Count) + } + if agg.OverallF1 != 0 { + t.Errorf("OverallF1 = %.6f, want 0", agg.OverallF1) + } +} + +func TestComputeModeAggSentinelWeighting(t *testing.T) { + results := []EvalResult{ + { + Mode: "test", + SampleID: "s1", + QAResults: []QAResult{ + {Category: 1, TokenF1: 0.8, HitRate: 0.5}, + {Category: 1, TokenF1: -1.0, HitRate: 0.3}, + }, + }, + { + Mode: "test", + SampleID: "s2", + QAResults: []QAResult{ + {Category: 1, TokenF1: 0.4, HitRate: 0.6}, + {Category: 1, TokenF1: 0.6, HitRate: 0.8}, + }, + }, + } + for i := range results { + results[i].Agg = aggregateMetrics(results[i].QAResults) + } + + got := computeModeAgg(results) + + // s1: ValidF1Count=1, F1=0.8; s2: ValidF1Count=2, F1=0.5 + // Weighted: (0.8*1 + 0.5*2) / 3 = 1.8/3 = 0.6 + wantF1 := 0.6 + if math.Abs(got.OverallF1-wantF1) > 1e-9 { + t.Errorf("OverallF1 = %.6f, want %.6f", got.OverallF1, wantF1) + } + if got.ValidF1Count != 3 { + t.Errorf("ValidF1Count = %d, want 3", got.ValidF1Count) + } + if got.TotalQuestions != 4 { + t.Errorf("TotalQuestions = %d, want 4", got.TotalQuestions) + } +} diff --git a/cmd/membench/main.go b/cmd/membench/main.go index 340924e46..58930c675 100644 --- a/cmd/membench/main.go +++ b/cmd/membench/main.go @@ -58,7 +58,8 @@ func main() { evalCmd.Flags().IntVar(&flagBudget, "budget", 4000, "token budget for retrieval") evalCmd.Flags(). StringVar(&flagEvalMode, "eval-mode", "token", "evaluation mode: token (direct match) or llm (LLM-as-Judge)") - evalCmd.Flags().StringVar(&flagAPIBase, "api-base", "", "OpenAI-compatible API base URL (env: MEMBENCH_API_BASE)") + evalCmd.Flags(). + StringVar(&flagAPIBase, "api-base", "", "API base URL with version path, e.g. http://host/v1 (default: http://127.0.0.1:8080/v1, env: MEMBENCH_API_BASE)") evalCmd.Flags().StringVar(&flagAPIKey, "api-key", "", "API key for the LLM endpoint (env: MEMBENCH_API_KEY)") evalCmd.Flags().StringVar(&flagModel, "model", "", "model name for LLM eval (env: MEMBENCH_MODEL)") evalCmd.Flags(). @@ -84,7 +85,8 @@ func main() { runCmd.Flags().IntVar(&flagBudget, "budget", 4000, "token budget for retrieval") runCmd.Flags(). StringVar(&flagEvalMode, "eval-mode", "token", "evaluation mode: token (direct match) or llm (LLM-as-Judge)") - runCmd.Flags().StringVar(&flagAPIBase, "api-base", "", "OpenAI-compatible API base URL (env: MEMBENCH_API_BASE)") + runCmd.Flags(). + StringVar(&flagAPIBase, "api-base", "", "API base URL with version path, e.g. http://host/v1 (default: http://127.0.0.1:8080/v1, env: MEMBENCH_API_BASE)") runCmd.Flags().StringVar(&flagAPIKey, "api-key", "", "API key for the LLM endpoint (env: MEMBENCH_API_KEY)") runCmd.Flags().StringVar(&flagModel, "model", "", "model name for LLM eval (env: MEMBENCH_MODEL)") runCmd.Flags().