From bc64d9e708b0a47f4370c3c49fe52958eb827434 Mon Sep 17 00:00:00 2001 From: BeaconCat Date: Mon, 13 Apr 2026 13:05:52 +0800 Subject: [PATCH] fix: address Copilot review round 4 - runReport splits results by mode suffix into token/llm for PrintComparison - backward compat fallback (ValidF1Count=0 -> TotalQuestions) only for non-LLM modes; LLM modes keep ValidF1Count=0 when all scores sentinel - MaxRetries==0 means no retry; only negative falls back to default 3 - truncateStr uses []rune to avoid cutting multi-byte UTF-8 characters - Complete() returns error on empty LLM response (vs silent empty string) --- cmd/membench/eval.go | 7 ++++--- cmd/membench/eval_llm.go | 5 +++-- cmd/membench/llm_client.go | 5 ++++- cmd/membench/main.go | 10 +++++++++- 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/cmd/membench/eval.go b/cmd/membench/eval.go index 44a5c2764..729c9f97f 100644 --- a/cmd/membench/eval.go +++ b/cmd/membench/eval.go @@ -305,9 +305,10 @@ func SaveAggregated(results []EvalResult, outDir string) error { func computeModeAgg(results []EvalResult) AggMetrics { agg := AggMetrics{ByCategory: map[int]*CatMetrics{}} for _, r := range results { - // Backward compat: old eval JSON without ValidF1Count → use TotalQuestions. + // Backward compat: old eval JSON (token mode) without ValidF1Count → use TotalQuestions. + // LLM modes may legitimately have ValidF1Count==0 (all failures). vf1 := r.Agg.ValidF1Count - if vf1 == 0 && r.Agg.TotalQuestions > 0 { + if vf1 == 0 && r.Agg.TotalQuestions > 0 && !strings.HasSuffix(r.Mode, "-llm") { vf1 = r.Agg.TotalQuestions } agg.OverallF1 += r.Agg.OverallF1 * float64(vf1) @@ -321,7 +322,7 @@ func computeModeAgg(results []EvalResult) AggMetrics { agg.ByCategory[cat] = existing } cvf1 := cm.ValidF1Count - if cvf1 == 0 && cm.QuestionCount > 0 { + if cvf1 == 0 && cm.QuestionCount > 0 && !strings.HasSuffix(r.Mode, "-llm") { cvf1 = cm.QuestionCount } existing.F1 += cm.F1 * float64(cvf1) diff --git a/cmd/membench/eval_llm.go b/cmd/membench/eval_llm.go index 47f9f9218..995155346 100644 --- a/cmd/membench/eval_llm.go +++ b/cmd/membench/eval_llm.go @@ -276,8 +276,9 @@ func countTotalQA(samples []LocomoSample) int { func truncateStr(s string, maxLen int) string { s = strings.ReplaceAll(s, "\n", " ") - if len(s) > maxLen { - return s[:maxLen] + "..." + runes := []rune(s) + if len(runes) > maxLen { + return string(runes[:maxLen]) + "..." } return s } diff --git a/cmd/membench/llm_client.go b/cmd/membench/llm_client.go index a6fd58c09..f84173192 100644 --- a/cmd/membench/llm_client.go +++ b/cmd/membench/llm_client.go @@ -38,7 +38,7 @@ func NewLLMClient(opts LLMClientOptions) *LLMClient { opts.Timeout = 120 * time.Second } maxRetries := opts.MaxRetries - if maxRetries <= 0 { + if maxRetries < 0 { maxRetries = 3 } return &LLMClient{ @@ -170,5 +170,8 @@ func (c *LLMClient) Complete(ctx context.Context, systemPrompt, userPrompt strin if idx := strings.Index(content, ""); idx >= 0 { content = strings.TrimSpace(content[idx+len(""):]) } + if content == "" { + return "", fmt.Errorf("empty LLM response") + } return content, nil } diff --git a/cmd/membench/main.go b/cmd/membench/main.go index ed61feeff..f4adbcfca 100644 --- a/cmd/membench/main.go +++ b/cmd/membench/main.go @@ -272,7 +272,15 @@ func runReport(cmd *cobra.Command, args []string) error { return fmt.Errorf("no eval results found in %s", flagOut) } - PrintComparison(allResults, nil) + var tokenResults, llmResults []EvalResult + for _, r := range allResults { + if strings.HasSuffix(r.Mode, "-llm") { + llmResults = append(llmResults, r) + } else { + tokenResults = append(tokenResults, r) + } + } + PrintComparison(tokenResults, llmResults) return nil }