fallow: Agent for Claude Code

Review Rust code changes in the fallow project. Focus on:

What to check

1. Correctness: Logic errors, edge cases, panic paths
2. Performance: Unnecessary allocations, missing &str over String, clone() where borrow works
3. Project conventions:
   • #[expect(clippy::...)] not #[allow]
   • FxHashMap/FxHashSet instead of HashMap/HashSet
   • Size assertions on hot-path structs
   • Early returns with guard clauses
4. Cross-platform: Path separator issues (use .replace('\\', "/") in tests)
5. Cache friendliness: Flat storage patterns, avoid Arc/Rc where not needed

Surface-specific checks

For each Rust-touching diff, walk this list in addition to the generic checks above:

• #[expect] over #[allow]: all new clippy/compiler lint suppressions use #[expect]
• No panics in non-test code: no unwrap() or expect() on user-facing paths
• Struct constructors complete: if you added a field to a struct, grep for ALL constructors of that struct in the entire codebase (tests included). Rust catches this at compile time, but test code may lag.
• Cache version bumped: if extraction logic changed (new/modified visitor handlers, changed member_accesses/whole_object_uses/MemberInfo output), verify CACHE_VERSION in crates/extract/src/cache/types.rs was incremented. Without this, stale cached data persists and the fix has no effect for users with warm caches.
• Glob matching uses relative paths: any GlobSet::is_match() or GlobMatcher::is_match() on user-supplied patterns (ignorePatterns, health.ignore, duplicates.ignore, overrides.files, boundary zone patterns) must match against a path stripped of the project root via path.strip_prefix(&config.root).unwrap_or(path). User patterns are relative (e.g., src/generated/**), but walker entries and graph paths are absolute. Matching without stripping silently fails for patterns without **/ prefix.
• Regex line-ending portability: any regex in extraction code (crates/extract/) that uses literal \n for line boundaries must use \r?\n instead. Windows CI checks out files with CRLF (\r\n), and \n-only patterns silently fail to match. Applies to: SFC parsers (sfc.rs), Astro frontmatter (astro.rs), MDX extraction (mdx.rs), HTML extraction (html.rs). Grep for \\n in regex strings: grep -n '\\\\n' crates/extract/src/*.rs | grep -v 'r\?\\\\n'.
• Decorator-based credit must validate the framework import source: if the change adds detection that flips a flag, credits an export, or skips a check based on a class decorator (@customElement, @Component, @Injectable, @Controller, @Get, @Pipe, @Module, @Entity, etc.), the credit must be gated against an import whose source is the framework's actual module path (e.g. lit/decorators.js, @angular/core, @nestjs/common, typeorm) AND whose imported symbol resolves to the canonical decorator name. Matching only on the syntactic callee identifier (Expression::Identifier(id) if id.name == "customElement") credits any local function with that name and produces silent false negatives on unused-export. The validator must compare both sides: ImportInfo.local_name == decorator_callee_name AND ImportedName::Named(canonical) | ImportedName::Namespace. Named-aliased imports (import { customElement as ce }) and namespace-call shapes (@decorators.customElement('x')) both flow through this check by tracking the local binding through the imports list. Verification grep: for every new pattern that matches a decorator callee by NAME alone, search for an accompanying check against an ImportInfo.source constant. Companion check: ship a negative-case fixture where a local function with the same name decorates a class, asserting the export is still reported as unused. Existing precedent: extract_angular_inject_target gates the inject callee against @angular/core. Caught 2026-05-05 on the Lit @customElement PR after the implement-phase visitor matched the bare identifier name.
• Audit ALL plugin discovery phases before claiming a behavior fix: when a change touches plugin/registry path semantics or workspace bucketing (crates/core/src/plugins/registry/, crates/core/src/lib.rs::bucket_files_by_workspace, run_workspace_fast), list every discovery phase (Phase 1 dep activation, Phase 2 static patterns, Phase 3a relative_files matcher, Phase 3b filesystem fallback) and identify which the change affects. A regression in one phase may not surface end-to-end if a sibling phase covers the same ground. Concrete check: write a SCENARIO test where ONLY the changed phase can find the config file (e.g., for Phase 3a: place the config file at a location Phase 3b cannot reach, such as apps/web/sub/vite.config.ts since Phase 3b only scans workspace_root non-recursively for non-**/ patterns). Also: globset literal/brace patterns basename-match across path components, so do not assume pattern.ts requires the path to be exactly pattern.ts. Caught 2026-04-29 on the workspace plugin-bucketing change in crates/core/src/lib.rs: review called it a "meaningful behavior fix" but Phase 3b filesystem fallback already covered workspace-local configs, making the change mostly a perf optimization. Cross-ref project_workspace_plugin_discovery_phases.md.
• Node-builtin callee gates must accept both prefixed and bare source forms: when reviewing a callee-import gate where the gated source is a Node builtin (node:module, node:fs, node:path, node:url, node:crypto, node:os, node:child_process, node:stream, node:buffer, node:events, node:net, node:http, node:https, node:zlib, etc.), confirm the gate accepts BOTH node:M AND bare M (Node still resolves the bare form for backwards compatibility with code from before the node: prefix was added in Node 16). Concrete grep target during review: when the diff calls is_named_import_from(_, "node:M", _) for any Node-builtin M, also grep for a sibling call against bare "M". If absent, flag CONCERN and require a wrapper helper that accepts both (pattern: is_node_module_register_import calling is_named_import_from twice). Caught 2026-05-06 on issue #293: narrow gate against "node:module" silently rejected import { register } from 'module' used by turbopack's own test fixture.
• Path-resolution fixes that swap project_root mid-pipeline must absolutize at the public entry point: when a fix introduces relative-path resolution against a project_root argument, grep for every recursive caller that DIFFERENTLY rebinds project_root (audit's compute_base_snapshot swaps to a worktree path; future cases might be cross-workspace recursion or any feature with multiple "root contexts"). For each such caller, the fix must absolutize at the OUTER entry point (e.g., audit::run_audit) so the inner recursion doesn't re-resolve against the wrong root. Pattern target list: any function in audit.rs / combined.rs / future cross-workspace driver that calls run_audit_* / execute_* with a recursively-rebuilt AuditOptions / CheckOptions / similar struct. The Phase 6 smoke that catches this kind of bug is "run from cwd != --root != base-worktree" with relative paths in flag values; if all three are the same path, the bug is invisible. Caught 2026-05-07 on the --coverage relative-path fix: load_istanbul_coverage resolved against the recursively-rebound base_root worktree path, breaking audit --base for relative --coverage.
• Four parallel attribution surfaces for path-anchored issue types: when the review covers a NEW issue type with a path-anchored finding, enumerate these four surfaces and confirm the new type appears in each: (1) crates/engine/src/changed_files.rs::filter_results_by_changed_files (--changed-since retain); (2) crates/engine/src/dead_code.rs::apply_rule_severities per-file override branch AND base branch; (3) crates/engine/src/effective_severity.rs: a RuleSeverity impl for the new finding wrapper, which the exit code, the CI format severities and the audit ledger all read (the exhaustive visitors in that file do not compile until the new field is listed); (4) crates/api/src/audit_keys.rs::dead_code_keys + retain_introduced_dead_code + annotate_dead_code_json (audit attribution trio). Concrete grep target for each new finding type's array name:

  for file in crates/engine/src/changed_files.rs crates/engine/src/dead_code.rs crates/engine/src/effective_severity.rs crates/api/src/audit_keys.rs; do
    echo "=== $file ==="
    grep -nE "<new_field_name>" "$file" || echo "  MISSING -- BLOCK"
  done
  

  Missing in any one = BLOCK. The audit trio is particularly easy to miss because the three functions are spread across 250+ lines and each has its own per-type loop; do not assume "the verdict count was right" implies all three are wired (the count comes from dead_code_keys which DOES key the type even when retain_introduced_dead_code and annotate_dead_code_json don't, so the attribution number can look right while the findings array is empty and the introduced field missing). Companion rule: when a new Vec<X> field on AnalysisResults has X.path: PathBuf, confirm X.path is stored as an ABSOLUTE filesystem path internally (NOT strip_prefix-ed) AND that JSON serialization uses #[serde(serialize_with = "serde_path::serialize")] to strip the root for output. Caught 2026-05-12 by codex's parallel /fallow-review on PR #340 (unresolved-catalog-references): three BLOCKs (changed-since filter, rules override, audit attribution trio) plus a storage-convention mismatch (relative-internal vs absolute-internal).
• Verify plugin enablers activate on the fixture before relying on integration tests as regression-strength: for any plugin-gated detection fix (the fix lives inside a Plugin::resolve_config, Plugin::path_aliases, Plugin::config_patterns, or any code path that runs only when a specific enabler matches), the fixture's package.json MUST list the enabler dep so the plugin is invoked during analysis. Common enablers: typescript (TypeScript plugin), vite (Vite plugin), webpack (Webpack plugin), eslint (ESLint plugin), vitest / jest / mocha (test plugins), next (Next.js), nuxt (Nuxt), @angular/core (Angular). Concrete pre-test check: open the fixture's package.json and confirm the enabler dep for the plugin whose code the fix touches is listed in dependencies OR devDependencies. Without it, the integration test silently exercises a code path the fix didn't change. Verification recipe: stash the fix, run the integration test, confirm it FAILS with an assertion message matching the expected pre-fix behavior. If the test passes on OLD source, either the fixture is missing the enabler dep, or the assertion is too loose, or the wrong code path is exercised. Caught 2026-05-11 on issue #327: wildcard_tsconfig_paths_do_not_misclassify_bare_imports fixture's package.json listed @types/node but not typescript, so the TypeScript plugin never ran, no ("", "src") path-alias entry landed, and the bug couldn't reproduce on OLD source.
• Verify library-behavior claims with a standalone test: when the review's reasoning depends on how an external library (globset, regex, serde_json, oxc_*, ureq, etc.) treats some input class, do NOT rely on memory or comment claims about that library's behavior. Write a 10-line standalone repro and run it against the EXACT version pinned in Cargo.lock. Concrete recipe: mkdir -p /tmp/lib-verify && cd /tmp/lib-verify && cargo init --bin && cargo add <crate>@<version-from-Cargo.lock>, write the smallest probe that exercises the claimed behavior, cargo run --release and assert the output matches the claim. Memory rules and code comments can drift across library upgrades; the only durable verification is empirical against the current pinned version. When the verification contradicts a project memory, also UPDATE the memory rule with a corrected claim and a "verify with cargo test ..." pointer to a test that locks the new behavior in. Caught 2026-05-06 during perf-loop round 8: project_workspace_plugin_discovery_phases.md claimed bare globset patterns basename-match across path components, but a standalone repro against globset 0.4.18 showed bare webpack.config.{ts,js,mjs,cjs} returned false against apps/web/webpack.config.ts; only the **/-prefixed form matched.