fix(hooks): scope Stop-hook clippy + test to changed crates (#465) (#467)

`check-on-stop.py` previously ran `cargo clippy --workspace
--all-targets` and `cargo test --workspace` on every Stop where any
file changed. A one-line edit in `fbuild-core` rebuilt + retested
every test in every crate — ~5+ min for tests, ~2-3 min for workspace
clippy.

Now `check-on-stop.py` parses `git status --porcelain -z`, classifies
the changed paths into:

- `crates/<name>/...` → run `cargo {clippy,test} -p <name>` for each
  changed crate
- root `Cargo.toml`, `Cargo.lock`, `rust-toolchain.toml`,
  `rustfmt.toml`, `clippy.toml`, anything under `.cargo/` → fall back
  to `--workspace` (cross-crate deps may have shifted)
- per-crate `Cargo.toml` → scopes to that crate, not workspace
- non-Rust-only change (markdown, json outside crates/) → skip entirely

`rustfmt --all --check` always runs (cheap, catches cross-crate
formatting). The acceptance from #465 is preserved: workspace-wide
verification still runs when build config changes; per-crate edits
get per-crate work.

Tests: `ci/hooks/test_check_on_stop.py` covers root vs per-crate
`Cargo.toml`, all workspace triggers, Windows backslash path
normalization, mixed workspace+per-crate (workspace wins), and the
no-Rust-changes skip path. 11 tests, all green.

Closes #465.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: zackees

ci/hooks/check-on-stop.py

@@ -1,12 +1,18 @@
 #!/usr/bin/env python3
-"""Stop hook: runs full workspace lint and tests.
+"""Stop hook: runs lint and tests scoped to the crates touched this session.
 
 Smart mode: only runs if files were actually changed during this session.
 Session fingerprint is captured at session start (check-on-start.py) and
 compared here. If nothing changed during the session, everything is skipped.
 
+Scoping rules (issue #465):
+  - Cargo.toml / Cargo.lock / rust-toolchain.toml / .cargo/** → workspace-wide
+  - changes under crates/<name>/ → per-crate clippy + test for each <name>
+  - changes only outside crates/ (and not workspace-wide) → format check only
+  - no Rust-relevant changes at all → skip
+
 Exit codes:
-  0 - All passed or skipped (no changes during session)
+  0 - All passed or skipped
   2 - Lint or test failures (stderr fed back to Claude)
 """
 
@@ -21,6 +27,21 @@
 PROJECT_ROOT = SCRIPT_DIR.parent.parent
 SESSION_FINGERPRINT_FILE = PROJECT_ROOT / ".cache" / "session_fingerprint.json"
 
+# Repo-root files whose change forces a workspace-wide lint+test —
+# they affect every crate (cross-crate deps, toolchain pin, lint config).
+WORKSPACE_TRIGGER_FILES = frozenset({
+    "Cargo.toml",
+    "Cargo.lock",
+    "rust-toolchain.toml",
+    "rustfmt.toml",
+    "clippy.toml",
+})
+# Path prefixes that also force workspace-wide (config / shared infra).
+WORKSPACE_PREFIXES = (".cargo/",)
+# Extensions that participate in the lint/test gate. Anything else
+# (e.g. .md, .txt, generated JSON outside crates/) doesn't trigger work.
+RUST_EXTENSIONS = (".rs",)
+
 
 def run_cmd(cmd):
     """Run a command rooted at PROJECT_ROOT."""
@@ -87,27 +108,133 @@ def should_skip():
     return False
 
 
+def get_dirty_files():
+    """Return paths of files dirty in the worktree (modified + untracked).
+
+    Uses `git status --porcelain -z` so filenames with spaces or unusual
+    characters are handled correctly. Renames are reported with both
+    source and destination — we count the destination (the path that
+    exists on disk and might compile).
+    """
+    result = run_cmd(["git", "status", "--porcelain", "-z"])
+    if result.returncode != 0 or not result.stdout:
+        return []
+    out = []
+    parts = result.stdout.split("\0")
+    i = 0
+    while i < len(parts):
+        entry = parts[i]
+        if not entry:
+            i += 1
+            continue
+        # `XY ` prefix + filename, where XY is two status chars + a space.
+        # For renames ("R "), the destination is on this entry and source
+        # is the next NUL-separated token; consume both, keep destination.
+        if len(entry) < 4:
+            i += 1
+            continue
+        status = entry[:2]
+        path = entry[3:]
+        if status.startswith("R") or status.startswith("C"):
+            # Destination is this entry's path; source is next, skip it.
+            i += 2
+        else:
+            i += 1
+        out.append(path)
+    return out
+
+
+def classify_changes(files):
+    """Map a list of changed paths to (crates_set, needs_workspace, has_rust).
+
+    - crates_set: distinct crate names under `crates/<name>/...` that changed
+    - needs_workspace: True if any change forces a workspace-wide run
+      (Cargo.lock, Cargo.toml at root, rust-toolchain.toml, .cargo/**)
+    - has_rust: True if any `.rs` file changed anywhere (so even non-crate
+      Rust files in benches or examples still get a workspace check)
+    """
+    crates: set[str] = set()
+    needs_workspace = False
+    has_rust = False
+    for raw in files:
+        # Normalize Windows separators for matching.
+        path = raw.replace("\\", "/")
+        if path in WORKSPACE_TRIGGER_FILES:
+            needs_workspace = True
+        if any(path.startswith(p) for p in WORKSPACE_PREFIXES):
+            needs_workspace = True
+        if path.startswith("crates/"):
+            parts = path.split("/")
+            if len(parts) >= 2 and parts[1]:
+                crates.add(parts[1])
+        if path.endswith(RUST_EXTENSIONS):
+            has_rust = True
+    return crates, needs_workspace, has_rust
+
+
+def run_lint(crates, needs_workspace):
+    """Run rustfmt --check + clippy, scoped to the changed crates."""
+    # rustfmt is workspace-cheap and catches cross-crate consistency;
+    # always run --all. Use --check (no autofix) because Stop is read-only
+    # from the user's POV — they shouldn't see surprise modifications.
+    fmt = run_cmd(["soldr", "cargo", "fmt", "--all", "--", "--check"])
+    if fmt.returncode != 0:
+        report_failure("Formatting check failed (run `soldr cargo fmt --all` to fix)", fmt)
+        return fmt
+    cmd = ["soldr", "cargo", "clippy"]
+    if needs_workspace or not crates:
+        cmd += ["--workspace"]
+    else:
+        for c in sorted(crates):
+            cmd += ["-p", c]
+    cmd += ["--all-targets", "--", "-D", "warnings"]
+    return run_cmd(cmd)
+
+
+def run_tests(crates, needs_workspace):
+    """Run cargo test, scoped to the changed crates."""
+    cmd = ["soldr", "cargo", "test"]
+    if needs_workspace or not crates:
+        cmd += ["--workspace"]
+    else:
+        for c in sorted(crates):
+            cmd += ["-p", c]
+    return run_cmd(cmd)
+
+
 def main():
     if should_skip():
         print("Skipping stop checks (no changes during this session)", file=sys.stderr)
         return 0
 
-    print("Running full workspace checks (changes detected)", file=sys.stderr)
+    dirty = get_dirty_files()
+    crates, needs_workspace, has_rust = classify_changes(dirty)
 
-    lint_script = str(PROJECT_ROOT / "ci" / "lint.py")
-    test_script = str(PROJECT_ROOT / "ci" / "test.py")
+    if not has_rust and not needs_workspace:
+        # No Rust-relevant changes (markdown, json outside crates/, etc.)
+        print(
+            f"Skipping stop checks (changes touched no Rust code: {len(dirty)} non-Rust file(s))",
+            file=sys.stderr,
+        )
+        return 0
+
+    scope_label = (
+        "workspace-wide (Cargo.toml/lock/toolchain change)"
+        if needs_workspace
+        else f"scoped to {len(crates)} crate(s): {', '.join(sorted(crates))}"
+        if crates
+        else "workspace-wide (no per-crate attribution available)"
+    )
+    print(f"Running stop checks: {scope_label}", file=sys.stderr)
 
-    # Run lint and tests concurrently
     lint_results = []
     test_results = []
 
     def do_lint():
-        result = run_cmd(["uv", "run", "--script", lint_script, "--fix"])
-        lint_results.append(result)
+        lint_results.append(run_lint(crates, needs_workspace))
 
     def do_test():
-        result = run_cmd(["uv", "run", "--script", test_script])
-        test_results.append(result)
+        test_results.append(run_tests(crates, needs_workspace))
 
     lint_thread = threading.Thread(target=do_lint)
     test_thread = threading.Thread(target=do_test)

ci/hooks/test_check_on_stop.py

@@ -0,0 +1,105 @@
+#!/usr/bin/env python3
+"""Unit tests for the Stop hook's per-crate scoping classifier (issue #465)."""
+
+import importlib.util
+import unittest
+from pathlib import Path
+
+# The hook script's filename has a dash, so plain `import` won't work —
+# load it through importlib instead.
+_SCRIPT = Path(__file__).parent / "check-on-stop.py"
+_spec = importlib.util.spec_from_file_location("check_on_stop", _SCRIPT)
+assert _spec is not None and _spec.loader is not None
+_module = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_module)
+classify_changes = _module.classify_changes
+
+
+class ClassifyChangesTests(unittest.TestCase):
+    def test_root_cargo_toml_triggers_workspace(self):
+        crates, needs_workspace, has_rust = classify_changes(["Cargo.toml"])
+        self.assertTrue(needs_workspace)
+        self.assertFalse(has_rust)
+        self.assertEqual(crates, set())
+
+    def test_cargo_lock_triggers_workspace(self):
+        _, needs_workspace, _ = classify_changes(["Cargo.lock"])
+        self.assertTrue(needs_workspace)
+
+    def test_toolchain_change_triggers_workspace(self):
+        _, needs_workspace, _ = classify_changes(["rust-toolchain.toml"])
+        self.assertTrue(needs_workspace)
+
+    def test_dot_cargo_config_triggers_workspace(self):
+        _, needs_workspace, _ = classify_changes([".cargo/config.toml"])
+        self.assertTrue(needs_workspace)
+
+    def test_crate_change_scopes_to_that_crate(self):
+        crates, needs_workspace, has_rust = classify_changes(
+            ["crates/fbuild-core/src/lib.rs"]
+        )
+        self.assertFalse(needs_workspace)
+        self.assertTrue(has_rust)
+        self.assertEqual(crates, {"fbuild-core"})
+
+    def test_multiple_crate_changes_collect_all(self):
+        crates, needs_workspace, _ = classify_changes(
+            [
+                "crates/fbuild-core/src/lib.rs",
+                "crates/fbuild-build/src/symbol_analyzer.rs",
+                "crates/fbuild-core/src/symbol_analysis/cref.rs",
+            ]
+        )
+        self.assertFalse(needs_workspace)
+        self.assertEqual(crates, {"fbuild-core", "fbuild-build"})
+
+    def test_crate_cargo_toml_does_not_force_workspace(self):
+        # A per-crate Cargo.toml change should only re-test that crate;
+        # only the ROOT Cargo.toml (which defines workspace members) does.
+        crates, needs_workspace, has_rust = classify_changes(
+            ["crates/fbuild-core/Cargo.toml"]
+        )
+        self.assertFalse(needs_workspace, "per-crate Cargo.toml must NOT trigger workspace")
+        self.assertEqual(crates, {"fbuild-core"})
+        self.assertFalse(has_rust)
+
+    def test_markdown_only_change_skips(self):
+        # No Rust files, no workspace triggers — main() should skip,
+        # but the classifier still reports the facts.
+        crates, needs_workspace, has_rust = classify_changes(
+            ["docs/CLAUDE.md", "README.md"]
+        )
+        self.assertFalse(needs_workspace)
+        self.assertFalse(has_rust)
+        self.assertEqual(crates, set())
+
+    def test_windows_backslash_paths_are_normalized(self):
+        # `git status -z` on Windows can emit backslash separators;
+        # the classifier must treat them as `/`.
+        crates, _, has_rust = classify_changes(
+            ["crates\\fbuild-core\\src\\lib.rs"]
+        )
+        self.assertTrue(has_rust)
+        self.assertEqual(crates, {"fbuild-core"})
+
+    def test_workspace_and_per_crate_change_together_picks_workspace(self):
+        # Mixed: Cargo.lock change + crate code change. Workspace wins
+        # because cross-crate deps may have shifted under the per-crate
+        # code.
+        crates, needs_workspace, _ = classify_changes(
+            ["Cargo.lock", "crates/fbuild-core/src/lib.rs"]
+        )
+        self.assertTrue(needs_workspace)
+        # crates set is still populated (informational), but the runner
+        # will use --workspace anyway.
+        self.assertEqual(crates, {"fbuild-core"})
+
+    def test_empty_input_returns_empty(self):
+        crates, needs_workspace, has_rust = classify_changes([])
+        self.assertFalse(needs_workspace)
+        self.assertFalse(has_rust)
+        self.assertEqual(crates, set())
+
+
+if __name__ == "__main__":
+    unittest.main()