From 29b94bf75f0b1792cb7724ebb071c6e5fee30318 Mon Sep 17 00:00:00 2001
From: BeaconCat <BeaconCat@users.noreply.github.com>
Date: Sun, 12 Apr 2026 16:06:15 +0800
Subject: [PATCH] fix: address Copilot review feedback

- Validate --model is required for LLM eval mode
- Use rune-based truncation to preserve valid UTF-8
- Precompute totalQA count outside inner loop
- Log SearchMessages errors instead of silently skipping
---
 cmd/membench/eval_llm.go | 14 +++++++++-----
 cmd/membench/main.go     |  5 +++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/cmd/membench/eval_llm.go b/cmd/membench/eval_llm.go
index 7318b76bb..46b996cc9 100644
--- a/cmd/membench/eval_llm.go
+++ b/cmd/membench/eval_llm.go
@@ -27,9 +27,10 @@ Output ONLY the number, nothing else.`
 
 // generateAnswer asks the LLM to answer a question given retrieved context.
 func generateAnswer(ctx context.Context, client *LLMClient, contextText, question string) (string, error) {
-	// Truncate context to avoid exceeding model limits
-	if len(contextText) > 6000 {
-		contextText = contextText[:6000] + "\n... [truncated]"
+	// Truncate context to avoid exceeding model limits while preserving valid UTF-8.
+	contextRunes := []rune(contextText)
+	if len(contextRunes) > 6000 {
+		contextText = string(contextRunes[:6000]) + "\n... [truncated]"
 	}
 
 	userPrompt := fmt.Sprintf("## Conversation Context\n\n%s\n\n## Question\n\n%s", contextText, question)
@@ -74,6 +75,7 @@ func EvalLegacyLLM(
 	budgetTokens int,
 	client *LLMClient,
 ) []EvalResult {
+	totalQA := countTotalQA(samples)
 	results := make([]EvalResult, 0, len(samples))
 	total := 0
 	for si := range samples {
@@ -119,7 +121,7 @@ func EvalLegacyLLM(
 			})
 
 			log.Printf("[legacy-llm] sample=%s q=%d/%d score=%.2f answer=%q",
-				sample.SampleID, total, countTotalQA(samples), score, truncateStr(llmAnswer, 80))
+				sample.SampleID, total, totalQA, score, truncateStr(llmAnswer, 80))
 		}
 
 		results = append(results, EvalResult{
@@ -143,6 +145,7 @@ func EvalSeahorseLLM(
 	store := ir.Engine.GetRetrieval().Store()
 	retrieval := ir.Engine.GetRetrieval()
 
+	totalQA := countTotalQA(samples)
 	results := make([]EvalResult, 0, len(samples))
 	total := 0
 	for si := range samples {
@@ -168,6 +171,7 @@ func EvalSeahorseLLM(
 					Limit:          20,
 				})
 				if err != nil {
+					log.Printf("WARN: search failed for keyword %q: %v", kw, err)
 					continue
 				}
 				for _, sr := range searchResults {
@@ -232,7 +236,7 @@ func EvalSeahorseLLM(
 			})
 
 			log.Printf("[seahorse-llm] sample=%s q=%d/%d score=%.2f answer=%q",
-				sample.SampleID, total, countTotalQA(samples), score, truncateStr(llmAnswer, 80))
+				sample.SampleID, total, totalQA, score, truncateStr(llmAnswer, 80))
 		}
 
 		results = append(results, EvalResult{
diff --git a/cmd/membench/main.go b/cmd/membench/main.go
index c85105fb2..f75d4b1b4 100644
--- a/cmd/membench/main.go
+++ b/cmd/membench/main.go
@@ -284,6 +284,11 @@ func buildLLMOptions() (LLMClientOptions, error) {
 		base = "http://127.0.0.1:8080"
 	}
 	model := envOrFlag(flagModel, "MEMBENCH_MODEL")
+	if model == "" {
+		return LLMClientOptions{}, fmt.Errorf(
+			"--model or MEMBENCH_MODEL is required for LLM eval mode",
+		)
+	}
 	apiKey := envOrFlag(flagAPIKey, "MEMBENCH_API_KEY")
 
 	return LLMClientOptions{