From 08a207338d43c0b9480f1725baabdda54f2f4edd Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 4 Sep 2026 17:13:40 +0000 Subject: [PATCH] fix(classifier): dedupe tool-error iteration numbers PhaseError plus a tool-ish verification failure used to append the same iteration twice, producing evidence like [3, 3, 5]. Use else-if so each iteration is recorded at most once. Fixes #10 --- internal/classifier/classify.go | 4 +- internal/classifier/classify_test.go | 69 ++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/internal/classifier/classify.go b/internal/classifier/classify.go index be16cec..3d4e5b2 100644 --- a/internal/classifier/classify.go +++ b/internal/classifier/classify.go @@ -99,8 +99,8 @@ func detectToolError(t *trace.Trace) *Classification { for i, it := range t.Iterations { if it.Phase == trace.PhaseError { errorIters = append(errorIters, i+1) - } - if it.Verification != nil && it.Verification.Status == "fail" { + } else if it.Verification != nil && it.Verification.Status == "fail" { + // else-if: PhaseError + tool-ish verify fail must not record the same iter twice output := strings.ToLower(it.Verification.Output) if strings.Contains(output, "file not found") || strings.Contains(output, "permission denied") || diff --git a/internal/classifier/classify_test.go b/internal/classifier/classify_test.go index 0aaba02..733511f 100644 --- a/internal/classifier/classify_test.go +++ b/internal/classifier/classify_test.go @@ -124,3 +124,72 @@ func TestNoClassification_HealthyTrace(t *testing.T) { t.Errorf("expected 0 classifications for healthy trace, got %d: %v", len(results), results) } } + +func TestDetectToolError_NoDuplicateIterations(t *testing.T) { + // Iteration with PhaseError AND a tool-ish verify fail used to be recorded twice + // (e.g. "Tool errors at iterations [3, 3, 5]"). + tr := trace.New([]trace.Iteration{ + {Number: 1, Phase: trace.PhaseAct}, + {Number: 2, Phase: trace.PhaseAct}, + { + Number: 3, + Phase: trace.PhaseError, + Verification: &trace.Verification{ + Status: "fail", + Output: "Error: file not found: src/missing.ts", + }, + }, + {Number: 4, Phase: trace.PhaseAct}, + { + Number: 5, + Phase: trace.PhaseError, + Verification: &trace.Verification{ + Status: "fail", + Output: "permission denied", + }, + }, + }) + + results := Classify(tr) + var evidence string + found := false + for _, r := range results { + if r.Category == CatToolError { + found = true + evidence = r.Evidence + } + } + if !found { + t.Fatal("expected tool_error classification") + } + if evidence != "Tool errors at iterations [3 5], never recovered" { + t.Fatalf("expected deduped iterations [3 5], got evidence %q", evidence) + } +} + +func TestDetectToolError_VerifyFailWithoutPhaseError(t *testing.T) { + tr := trace.New([]trace.Iteration{ + { + Number: 1, + Phase: trace.PhaseVerify, + Verification: &trace.Verification{ + Status: "fail", + Output: "command not found: foo", + }, + }, + }) + + results := Classify(tr) + found := false + for _, r := range results { + if r.Category == CatToolError { + found = true + if r.Evidence != "Tool errors at iterations [1], never recovered" { + t.Fatalf("unexpected evidence: %q", r.Evidence) + } + } + } + if !found { + t.Fatal("expected tool_error when verify fail has tool-ish output") + } +}