From 265e6f9a151de9c5d10ab0a15a945d0823198d61 Mon Sep 17 00:00:00 2001 From: Timmy Date: Sun, 17 May 2026 16:52:26 +0100 Subject: [PATCH] fix(1101): strip passing-test lines before classify() lint check; remove diagnostic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The merge gate classifier was matching trigger keywords like `missing_doc_comments` inside passing-test name lines (e.g. `test agents::gates::tests::classify_lint_from_missing_doc_comments ... ok`), causing every gate failure to be mis-classified as Lint and bounced back to a fixup coder. Strip `test … … ok` lines before scanning for lint triggers. Also removes the temporary diagnostic block in runner.rs that confirmed the bug. Applied directly to master because the 1101 feature branch carried stale work from an earlier incarnation of the story that semantically conflicted with master's later diagnostic commit (`is_fixup` deleted on the branch, referenced on master). Co-Authored-By: Claude Opus 4.7 (1M context) --- server/src/agents/gates.rs | 33 ++++++++++++-- .../src/agents/pool/pipeline/merge/runner.rs | 44 ------------------- 2 files changed, 29 insertions(+), 48 deletions(-) diff --git a/server/src/agents/gates.rs b/server/src/agents/gates.rs index f51c629d..bbdb8ff4 100644 --- a/server/src/agents/gates.rs +++ b/server/src/agents/gates.rs @@ -33,16 +33,28 @@ impl GateFailureKind { /// Called once when a gate fails to produce a typed kind. Downstream code /// matches on the variant and must not call this on subsequent reads. pub fn classify(output: &str) -> Self { + // Strip `test ... ok` lines before checking lint-trigger keywords so + // a passing test whose name contains e.g. `missing_doc_comments` or `clippy::` + // does not produce a false-positive Lint classification (story 1101). + let stripped_for_lint: String = output + .lines() + .filter(|l| { + let t = l.trim(); + !(t.starts_with("test ") && t.ends_with("... ok")) + }) + .collect::>() + .join("\n"); + let is_lint = stripped_for_lint.contains("error[clippy::") + || stripped_for_lint.contains("warning[clippy::") + || stripped_for_lint.contains("missing_doc_comments"); + if output.contains("CONFLICT (content):") || output.contains("Merge conflict:") { GateFailureKind::ContentConflict } else if output.contains("Diff in ") || output.contains("would reformat") { GateFailureKind::Fmt } else if output.contains("missing-docs direction") { GateFailureKind::SourceMapCheck - } else if output.contains("error[clippy::") - || output.contains("warning[clippy::") - || output.contains("missing_doc_comments") - { + } else if is_lint { GateFailureKind::Lint } else if output.contains("error[E") { // rustc compile errors (e.g. `error[E0063]: missing field`). @@ -871,6 +883,19 @@ mod tests { ); } + /// Story 1101: a passing test whose name contains a lint trigger keyword + /// must NOT produce a Lint classification. + #[test] + fn classify_does_not_false_positive_on_test_name_substring() { + let output = "test agents::gates::tests::classify_lint_from_missing_doc_comments ... ok\n\ + test result: ok. 1 passed; 0 failed"; + assert_ne!( + GateFailureKind::classify(output), + GateFailureKind::Lint, + "passing test name containing 'missing_doc_comments' must not classify as Lint" + ); + } + #[test] fn classify_source_map_check_from_missing_docs_direction() { assert_eq!( diff --git a/server/src/agents/pool/pipeline/merge/runner.rs b/server/src/agents/pool/pipeline/merge/runner.rs index a950cb68..f04026a2 100644 --- a/server/src/agents/pool/pipeline/merge/runner.rs +++ b/server/src/agents/pool/pipeline/merge/runner.rs @@ -186,50 +186,6 @@ impl AgentPool { .map(|k| k.is_self_evident_fix()) .unwrap_or(false); - // Bug 1101 diagnostic: log the classified failure_kind and the - // matched classifier-trigger substring with surrounding context, - // so we can confirm whether classify() is incorrectly matching - // a passing-step stdout substring (e.g. "Diff in " inside a - // failing test's panic message) and bouncing the story to a - // fixup coder. Remove once the fix lands. - if let Ok(r) = report.as_ref() - && let crate::agents::merge::MergeResult::GateFailure { - output: gate_output, - failure_kind: Some(k), - } = &r.result - { - const TRIGGERS: &[&str] = &[ - "CONFLICT (content):", - "Merge conflict:", - "Diff in ", - "would reformat", - "missing-docs direction", - "error[clippy::", - "warning[clippy::", - "missing_doc_comments", - "error[E", - ]; - let matched = TRIGGERS - .iter() - .find_map(|t| gate_output.find(t).map(|i| (*t, i))); - let (trigger, context) = match matched { - Some((t, i)) => { - let start = i.saturating_sub(30); - let end = (i + t.len() + 60).min(gate_output.len()); - let ctx = gate_output - .get(start..end) - .unwrap_or("") - .replace('\n', " "); - (Some(t), ctx) - } - None => (None, String::from("")), - }; - slog!( - "[merge] classify diagnostic for '{sid}': failure_kind={k:?} \ - is_fixup={is_fixup} trigger={trigger:?} context='{context}'" - ); - } - if is_no_commits { let reason = kind.display_reason(); if let Err(e) = crate::agents::lifecycle::transition_to_blocked(&sid, &reason) {