From 2595fc8e580cb53d79b1017b3a2de4fc28270eda Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Sun, 6 Sep 2026 16:45:50 -0700 Subject: [PATCH] Fix: prioritize Stop findings over notices Keep stale design reminders opportunistic, compact attribution guidance at small budgets, and emit unknown guidance only for displayed findings. Add regressions for deduplication, finding caps, and single/grouped output limits. AI assistance: Codex, under maintainer direction. --- crates/hook/src/hook.rs | 30 +++++++++--- crates/hook/src/stop_baseline.rs | 1 + crates/hook/tests/hook_tests.rs | 83 ++++++++++++++++++++++++++++++-- 3 files changed, 104 insertions(+), 10 deletions(-) diff --git a/crates/hook/src/hook.rs b/crates/hook/src/hook.rs index 826ec3a0e..e3b0b6e9c 100644 --- a/crates/hook/src/hook.rs +++ b/crates/hook/src/hook.rs @@ -756,26 +756,42 @@ pub fn run_stop_hook(rt: &Runtime, stdin: &str) -> RunResult { ); } let short = footer_mode_short(&mut cache, &session_id); - let attribution_note = if unknown > 0 { + let mut attribution_note = if fresh_groups.iter().any(|group| { + group.findings.iter().any(|f| f.name.starts_with("[attribution unknown]")) + }) { format!("{ENVELOPE_PREFIX} {}", stop_baseline::UNKNOWN_NOTE) } else { String::new() }; - let reserve = design_note_reserve(rt, &scan, &mut cache, &session_id) - + if attribution_note.is_empty() { 0.0 } else { (utf16_len(&attribution_note) + 2) as f64 }; - let rendered = render_grouped_template( + // Findings and attribution take priority. Append the lower-priority stale + // DESIGN.md notice only if it fits, without consuming its session flag. + let render = |note: &str| render_grouped_template( rt, &fresh_groups, &config, &RenderOpts { cwd: Some(project_cwd.clone()), short_footer: short, - reserve_chars: reserve, + reserve_chars: if note.is_empty() { 0.0 } else { (utf16_len(note) + 2) as f64 }, }, ); + let mut rendered = render(&attribution_note); + if !attribution_note.is_empty() && !rendered.lines().any(|line| { + line.starts_with("- ") && (line.contains("[attribution unknown]") || line.contains("[new]")) + }) { + // At the minimum budget, a grouped header and policy footer may crowd + // out even the first finding. Shorten the notice before losing it. + attribution_note = format!("{ENVELOPE_PREFIX} {}", stop_baseline::COMPACT_UNKNOWN_NOTE); + rendered = render(&attribution_note); + } + // maxFindings / maxChars may also remove all unknown findings. Do not + // attach their guidance to an output that only shows confirmed new debt. + let shows_unknown = rendered.lines().any(|line| { + line.starts_with("- ") && line.contains("[attribution unknown]") + }); + let text = if shows_unknown { format!("{attribution_note}\n\n{rendered}") } else { rendered }; let text = - append_design_system_note_once(rt, &rendered, &scan, &mut cache, &session_id, &config); - let text = if attribution_note.is_empty() { text } else { format!("{attribution_note}\n\n{text}") }; + append_design_system_note_once(rt, &text, &scan, &mut cache, &session_id, &config); commit_footer_shown(rt, &mut cache, &session_id, &text); persist_cache(rt, &project_cwd, &cache); let all: usize = fresh_groups.iter().map(|g| g.findings.len()).sum(); diff --git a/crates/hook/src/stop_baseline.rs b/crates/hook/src/stop_baseline.rs index 36ec78df0..bd5d31b15 100644 --- a/crates/hook/src/stop_baseline.rs +++ b/crates/hook/src/stop_baseline.rs @@ -15,6 +15,7 @@ const FIELD: &str = "stopBaseline"; const MAX_BYTES: usize = 512 * 1024; const MAX_FINDINGS: usize = 256; pub const UNKNOWN_NOTE: &str = "Findings marked attribution unknown may predate this session; do not treat them as regressions or broaden the task without asking."; +pub const COMPACT_UNKNOWN_NOTE: &str = "Unknown findings may predate this session; ask before expanding scope."; fn independent(finding: &Finding) -> bool { !finding.antipattern.starts_with("design-system-") diff --git a/crates/hook/tests/hook_tests.rs b/crates/hook/tests/hook_tests.rs index 268d27e77..f086b60d4 100644 --- a/crates/hook/tests/hook_tests.rs +++ b/crates/hook/tests/hook_tests.rs @@ -271,18 +271,95 @@ fn stop_baseline_write_create_is_new_but_missing_update_preimage_is_unknown() { #[test] fn stop_baseline_unknown_notice_respects_small_output_budget() { + for (budget, stale) in [(500, false), (500, true), (8000, true)] { + let t = Tmp::new(); + let cwd = t.path(); + t.write("package.json", "{}"); + t.write(".impeccable/config.json", &json!({"hook":{"limits":{"maxChars":budget}}}).to_string()); + let file = t.write("card.css", SIDE_TAB_CSS); + let r = rt(&cwd); + hook::run_hook(&r, &edit_event(&cwd, &file, "s1")); + if stale { + // Make the notice eligible only at Stop; no sleeps or clock races. + t.write("DESIGN.md", "---\nname: Test\n---\n"); + let sidecar = t.write(".impeccable/design.json", "{}"); + std::fs::File::options().write(true).open(sidecar).unwrap() + .set_modified(std::time::UNIX_EPOCH + std::time::Duration::from_secs(1_600_000_000)).unwrap(); + assert!(design_system_options(&read_config(&cwd), &cwd).md_newer_than_json()); + } + let stop = hook::run_stop_hook(&r, &stop_event(&cwd, "s1")); + let output: Value = serde_json::from_str(&stop.stdout).unwrap(); + let text = output["hookSpecificOutput"]["additionalContext"].as_str().unwrap(); + assert!(text.encode_utf16().count() <= budget, "{text}"); + assert!(text.contains("may predate this session")); + assert!(text.contains("[side-tab]"), "{text}"); + assert!(text.contains("[attribution unknown]"), "{text}"); + assert!(text.contains("card.css"), "{text}"); + if stale { + assert_eq!(text.contains("DESIGN.md is newer"), budget > 500, "{text}"); + let cache: Value = serde_json::from_str(&t.read(".impeccable/hook.cache.json")).unwrap(); + assert_eq!(cache["sessions"]["s1"]["designNoteShown"] == json!(true), budget > 500); + } + } +} + +#[test] +fn stop_baseline_deduplicated_unknown_does_not_add_notice_to_new_finding() { + let t = Tmp::new(); + let cwd = t.path(); + t.write("package.json", "{}"); + let r = rt(&cwd); + let old = t.write("old/card.css", SIDE_TAB_CSS); + hook::run_hook(&r, &edit_event(&cwd, &old, "s1")); + assert!(hook::run_stop_hook(&r, &stop_event(&cwd, "s1")).stdout.contains("[attribution unknown]")); + let new = t.write("new/card.css", SIDE_TAB_CSS); + // Use a verified create event (an empty Edit preimage is not trusted). + let create = json!({"cwd": cwd, "session_id": "s1", "tool_name": "Write", + "tool_input": {"file_path": new, "content": SIDE_TAB_CSS}, + "tool_response": {"type": "create", "filePath": new, "content": SIDE_TAB_CSS, "originalFile": null}}).to_string(); + hook::run_hook(&r, &create); + let stop = hook::run_stop_hook(&r, &stop_event(&cwd, "s1")); + assert_eq!(stop.audit["unknownFindings"], json!(1), "audit retains the full scan"); + assert!(stop.stdout.contains("[new]"), "{}", stop.stdout); + assert!(!stop.stdout.contains("may predate this session"), "{}", stop.stdout); +} + +#[test] +fn stop_baseline_capped_unknown_does_not_add_notice_to_new_finding() { + let t = Tmp::new(); + let cwd = t.path(); + t.write("package.json", "{}"); + t.write(".impeccable/config.json", r#"{"hook":{"limits":{"maxFindings":1}}}"#); + let r = rt(&cwd); + let new = t.write("new/card.css", SIDE_TAB_CSS); + hook::run_hook(&r, &edit_with_original(&cwd, &new, "s1", ".card {}", ".card {}", SIDE_TAB_CSS)); + let old = t.write("old/card.css", SIDE_TAB_CSS); + hook::run_hook(&r, &edit_event(&cwd, &old, "s1")); + let stop = hook::run_stop_hook(&r, &stop_event(&cwd, "s1")); + assert_eq!(stop.audit["unknownFindings"], json!(1)); + assert!(stop.stdout.contains("[new]"), "{}", stop.stdout); + assert!(!stop.stdout.contains("[attribution unknown]"), "{}", stop.stdout); + assert!(!stop.stdout.contains("may predate this session"), "{}", stop.stdout); +} + +#[test] +fn stop_baseline_small_grouped_output_keeps_finding_and_attribution() { let t = Tmp::new(); let cwd = t.path(); t.write("package.json", "{}"); t.write(".impeccable/config.json", r#"{"hook":{"limits":{"maxChars":500}}}"#); - let file = t.write("card.css", SIDE_TAB_CSS); let r = rt(&cwd); - hook::run_hook(&r, &edit_event(&cwd, &file, "s1")); + for path in ["one/card.css", "two/card.css"] { + let file = t.write(path, SIDE_TAB_CSS); + hook::run_hook(&r, &edit_event(&cwd, &file, "s1")); + } let stop = hook::run_stop_hook(&r, &stop_event(&cwd, "s1")); let output: Value = serde_json::from_str(&stop.stdout).unwrap(); let text = output["hookSpecificOutput"]["additionalContext"].as_str().unwrap(); assert!(text.encode_utf16().count() <= 500, "{text}"); - assert!(text.contains("may predate this session")); + assert!(text.contains("[side-tab]"), "{text}"); + assert!(text.contains("[attribution unknown]"), "{text}"); + assert!(text.contains("may predate this session"), "{text}"); } #[test]