Add check suppression accounting by jonathanong · Pull Request #634 · jonathanong/no-mistakes

jonathanong · 2026-08-09T23:58:39Z

Summary

Adds deterministic, request-scoped suppression accounting to no-mistakes check and its async N-API counterpart. Audit mode is opt-in via --include-suppressed / includeSuppressed, preserving the option-off schema and adding suppressed: [] when requested with no matches.

The aggregate checker now defers directive filtering until one SourceStore-backed accounting pass across ordinary rules, filesystem checks, React, queues, integrations, unique exports, Next.js caching/API routes, direct and reachable dynamic imports, server-route client boundaries, Agents.md size checks, Storybook coverage, and finite-set completeness. Standalone rule APIs retain their existing suppression behavior.

Suppressed findings include the directive kind (file, line, or nextLine) and exact directive source line. React findings are accounted per fetch occurrence while their internal locations remain outside the public React DTO. CLI check, direct N-API check, and analyzeProject({ check: ... }) all use the same finalization contract.

Validation

cargo clippy --workspace --all-targets -- -D warnings
cargo fmt --all -- --check
built repository cargo run -p no-mistakes -- check --root .
focused N-API check suite: 20 passed
analyzeProject suppression parity regression
Storybook suppression/source ownership: 16 passed
dynamic-import config source ownership: 3 passed
combined Rust scanner: 5 passed
unique exports: 42 passed
standalone Next.js caching/API route, server boundary, Storybook, and finite-set regressions
React multi-fetch, exact directive precedence, and option-off schema regressions
git diff --check

All regression projects are static root fixtures under fixtures/check/**.

Root cause

Several analyzers applied suppression while extracting or selecting facts. That made aggregate audit mode unable to report the suppressed finding, and simply removing those filters changed stable standalone APIs or downstream comparison semantics. The corrected design uses explicit aggregate-only deferred entrypoints: extraction and graph preparation retain candidates for check, then one request-scoped adapter applies suppression and records provenance. Standalone entrypoints continue filtering at their established boundary.

Shepherd Journal

CI required structural module splits and typed request structs instead of lint exceptions; no allow/coverage exemption was added.
Unique exports retain canonical origin identity while deferring aggregate filtering, including file, line, and next-line directives before canonical selection.
React suppression is evaluated per fetch occurrence, including multiple calls in one file; provenance precedence matches the stable suppression predicate.
Combined Rust scans, Next.js checks, dynamic imports, server boundaries, Agents.md, and Storybook use explicit aggregate deferral; standalone behavior remains fixture-tested.
Finite-set suppression intentionally controls comparison soundness without discarding the extracted finding; the counterintuitive invariant has a fixture and explanatory test comment.
The final ownership audit found and fixed three paths beyond the original review: analyzeProject now shares aggregate finalization, and combined Rust/Storybook/dynamic-import configuration reads now borrow the request SourceStore instead of creating competing reads.
Final Codex review fix: file-disabled dynamic-import parse failures are skipped before parse-error propagation in audit mode, so unrelated findings remain visible; a static malformed-file fixture proves baseline/audit parity.
Addressed suppression review: normalized explicitly configured gitignored test-runner configs remain valid supplemental reads through the request-owned SourceStore; the visible inventory stays bounded and no second store or fact pass is created.
Addressed PRRT_kwDOSaSRXM6YDaK8, PRRT_kwDOSaSRXM6YDaLB, and PRRT_kwDOSaSRXM6YDaLD: canonical export dedup now prefers a visible same-origin representative; dynamic-import checks consume the authoritative prepared source snapshot (aggregate snapshots originate in the request SourceStore); and the architecture assertion now checks helper symbols structurally.
Addressed PRRT_kwDOSaSRXM6YDmKN: explicit Storybook config paths are authorized into the existing request-owned SourceStore before preparation, preserving ignored/out-of-inventory config semantics without creating a second store or an arbitrary lower-layer read path.
No code change for transient Codecov project/rust "No coverage information found on head" while Rust tests and coverage remains in progress after b22e561.

coderabbitai

Actionable comments posted: 4

🧹 Nitpick comments (1)

crates/no-mistakes/src/napi_api/options.rs (1)
22-22: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Add regression coverage for omitted includeSuppressed.

ProjectOptions deserializes the struct with #[serde(default)], so omitted includeSuppressed uses bool::default() (false). Add a regression test that parses an N-API payload without includeSuppressed to cover this compatibility case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/no-mistakes/src/napi_api/options.rs` at line 22, Add regression
coverage for ProjectOptions deserialization by parsing an N-API payload that
omits includeSuppressed, then assert the resulting include_suppressed field is
false. Reuse the existing options parsing test patterns and preserve current
behavior for explicitly provided values.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/no-mistakes/src/codebase/rules/source_access.rs`:
- Around line 5-7: Update the CLI codebase-check flow and its check_with_files
callers to reuse the session-owned SourceStore from
visible_paths.source_store_for(root) instead of constructing one through
source_store_for_files or
SourceStore::new(FileInventory::from_paths(all_files)). Ensure rules including
config_path_references, production_dependency_declarations, agents_md_max_size,
and fact_collection receive the prepared store so findings use the same
file/source inventory as the collected facts.

In `@crates/no-mistakes/src/codebase/ts_source/disable_comments/directives.rs`:
- Around line 20-26: Update the directive resolution flow to check
super::has_disable_comment before super::has_disable_line_comment, returning
DisableDirective::NextLine when both directives match so it matches
finding_is_suppressed precedence; retain the existing Line fallback and add a
regression case covering both directives and asserting the NextLine provenance.

In `@crates/no-mistakes/src/napi_api/tests.rs`:
- Line 357: Update the test around the fetching result assertion to first
require that the “fetches” value is a nonempty array and that its first entry is
serialized, then check that entry’s “line” field is absent. Ensure missing or
empty fetch records cause the test to fail rather than passing through null
indexing.

In `@packages/no-mistakes/report-types.d.ts`:
- Around line 33-34: Update the documentation for the suppressed field in the
report type declaration to state that it is present whenever includeSuppressed
is requested, including when no directives match and the value is an empty
array.

---

Nitpick comments:
In `@crates/no-mistakes/src/napi_api/options.rs`:
- Line 22: Add regression coverage for ProjectOptions deserialization by parsing
an N-API payload that omits includeSuppressed, then assert the resulting
include_suppressed field is false. Reuse the existing options parsing test
patterns and preserve current behavior for explicitly provided values.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d82ead5b-7879-412d-b45e-c6505f234543

📥 Commits

Reviewing files that changed from the base of the PR and between 4ffcf14 and 3fd3290.

📒 Files selected for processing (72)

crates/no-mistakes/src/benchmark_support.rs
crates/no-mistakes/src/check.rs
crates/no-mistakes/src/check_parallel.rs
crates/no-mistakes/src/check_runner.rs
crates/no-mistakes/src/check_runner/results.rs
crates/no-mistakes/src/check_runner/results/suppression.rs
crates/no-mistakes/src/check_runner/run_all.rs
crates/no-mistakes/src/check_runner/tests.rs
crates/no-mistakes/src/check_runner/tests/architecture.rs
crates/no-mistakes/src/check_runner/tests/integration_gitignore.rs
crates/no-mistakes/src/check_tasks.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/entrypoints.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/tests.rs
crates/no-mistakes/src/codebase/rules/mod.rs
crates/no-mistakes/src/codebase/rules/run.rs
crates/no-mistakes/src/codebase/rules/run/prepared.rs
crates/no-mistakes/src/codebase/rules/run/prepared/execution.rs
crates/no-mistakes/src/codebase/rules/run/standalone.rs
crates/no-mistakes/src/codebase/rules/source_access.rs
crates/no-mistakes/src/codebase/rules/suppression.rs
crates/no-mistakes/src/codebase/rules/suppression/accounting.rs
crates/no-mistakes/src/codebase/ts_source/disable_comments.rs
crates/no-mistakes/src/codebase/ts_source/disable_comments/directives.rs
crates/no-mistakes/src/codebase/ts_source/tests.rs
crates/no-mistakes/src/codebase/unique_exports.rs
crates/no-mistakes/src/codebase/unique_exports/collector.rs
crates/no-mistakes/src/codebase/unique_exports/origin.rs
crates/no-mistakes/src/codebase/unique_exports/scan.rs
crates/no-mistakes/src/codebase/unique_exports/scan/test_support.rs
crates/no-mistakes/src/codebase/unique_exports/tests.rs
crates/no-mistakes/src/codebase/unique_exports/tests/origin.rs
crates/no-mistakes/src/codebase/unique_exports/tests/shared_facts_disable.rs
crates/no-mistakes/src/codebase/unique_exports/types.rs
crates/no-mistakes/src/codebase/unique_exports/with_facts.rs
crates/no-mistakes/src/codebase/unique_exports/with_facts/prepared.rs
crates/no-mistakes/src/codebase/unique_exports/with_facts/prepared/aggregate.rs
crates/no-mistakes/src/fetch/visit_helpers.rs
crates/no-mistakes/src/fetch/visitor/tests.rs
crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs
crates/no-mistakes/src/napi_api/cli_parity.rs
crates/no-mistakes/src/napi_api/options.rs
crates/no-mistakes/src/napi_api/tests.rs
crates/no-mistakes/src/napi_api/tests/check.rs
crates/no-mistakes/src/react_traits/analyze/file.rs
crates/no-mistakes/src/react_traits/pipeline/check.rs
crates/no-mistakes/src/react_traits/pipeline/run/tests.rs
crates/no-mistakes/src/react_traits/pipeline/run_with_facts/tests.rs
crates/no-mistakes/src/react_traits/report/text/tests.rs
crates/no-mistakes/src/react_traits/report/types.rs
docs/cli/check.md
docs/rules/README.md
fixtures/check/suppression-accounting/.no-mistakes.yml
fixtures/check/suppression-accounting/src/a.ts
fixtures/check/suppression-accounting/src/b.ts
fixtures/check/suppression-accounting/src/c.ts
fixtures/check/suppression-accounting/tsconfig.json
fixtures/check/suppression-filesystem/.no-mistakes.yml
fixtures/check/suppression-filesystem/src/placeholder.ts
fixtures/check/suppression-integration/.no-mistakes.yml
fixtures/check/suppression-integration/helpers/openai.mts
fixtures/check/suppression-integration/tests/uses-openai.test.mts
fixtures/check/suppression-integration/vitest.config.mts
fixtures/check/suppression-queues/.no-mistakes.yml
fixtures/check/suppression-queues/src/queues.ts
fixtures/check/suppression-react/.no-mistakes.yml
fixtures/check/suppression-react/app/Fetcher.tsx
fixtures/codebase/unique-exports-suppressed-origin/src/barrel.ts
fixtures/codebase/unique-exports-suppressed-origin/src/source.ts
packages/no-mistakes/report-types.d.ts
packages/no-mistakes/traversal-types.d.ts

coderabbitai

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs (1)
62-69: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve audit output on the empty-report path.

When include_suppressed is true, this early return bypasses finalize_domain_checks, which receives and stores that option. empty_results([None]) has no inclusion argument. An empty analyzeProject check report can therefore omit suppressed instead of returning suppressed: [].

Make the empty-result helper flag-aware. Add an N-API regression test for an empty check report with includeSuppressed: true.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs` around
lines 62 - 69, Make the early empty-report return in the check-run flow preserve
include_suppressed behavior by passing the flag through the empty-results helper
or finalization path, ensuring suppressed: [] is present when enabled. Update
the relevant empty-result helper API and add an N-API regression test covering
analyzeProject with includeSuppressed: true and no reportable files.

🧹 Nitpick comments (1)

crates/no-mistakes/src/codebase/rules/rust_rules_combined.rs (1)
31-47: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Remove _exclusive_files from the combined Rust-rule API and its callers. The scanner now reads every file through SourceStore; the former exclusive branch only bypassed that store with std::fs::read_to_string. Update the focused tests accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/no-mistakes/src/codebase/rules/rust_rules_combined.rs` around lines 31
- 47, Remove the unused _exclusive_files parameter from
check_with_files_sources_and_deferred_suppression and update every caller,
including focused tests, to use the revised signature. Ensure all Rust scanning
continues through SourceStore and eliminate any remaining exclusive-file
handling or std::fs::read_to_string bypass associated with this API.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/no-mistakes/src/codebase/rules/require_storybook_stories.rs`:
- Line 80: Update the public check boundary to create a single AnalysisSession
first, then obtain the visible-file inventory, facts, and SourceStore from that
session. Pass this same session to check_with_facts_and_catalog, removing any
separately constructed VisiblePathSnapshot, SourceStore, or AnalysisSession so
all source access and prepared facts remain session-owned.

In
`@crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts/graph.rs`:
- Around line 31-41: Move SourceStore ownership to the public request boundary:
have the public paths that create AnalysisSession also construct and retain the
SourceStore. Extend check_with_prepared_facts_and_session to accept a borrowed
&SourceStore, and update check_with_prepared_facts_graph_and_session to pass the
request-owned store instead of constructing one from shared.files().

In `@crates/no-mistakes/src/codebase/unique_exports/collector.rs`:
- Around line 65-67: Update the re-export handling around
ExportOccurrence.suppressed so named and wildcard chains retain the child/origin
suppression state while OR-combining it with the current file and re-export
location directives. Apply this consistently in both affected paths, and add
regression coverage for named and wildcard re-exports using the
suppressed-origin fixture.

In `@crates/no-mistakes/src/napi_api/tests/check.rs`:
- Around line 339-360: Update
check_json_keeps_unsuppressed_duplicate_when_suppressed_export_sorts_first to
assert that both baseline["codebase"] and audit["codebase"] contain the expected
unsuppressed unique-exports duplicate, including its file and relevant rule
details. Retain the existing assertion verifying the suppressed duplicate in
audit["suppressed"].

In `@fixtures/check/aggregate-agents-md-max-size/AGENTS.md`:
- Line 1: Remove the no-mistakes-disable-file directive from the
aggregate-agents-md-max-size fixture’s AGENTS.md, preserving the file’s
intentional length so it continues to produce the normal agents-md-max-size
finding. Relocate any suppression-accounting coverage to a permitted fixture or
rule instead.

In `@fixtures/check/aggregate-nextjs-no-caching/web/app/page.ts`:
- Line 3: Update the fetch call in the page fixture to retain cache:
'force-cache' while replacing the hardcoded /api/ route prefix with a neutral
URL or a route value sourced from fixture configuration.
- Line 2: Update the suppression comment for the nextjs-no-caching directive in
page.ts so its rationale states that caching is intentional, while preserving
the directive and request behavior.

---

Outside diff comments:
In `@crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs`:
- Around line 62-69: Make the early empty-report return in the check-run flow
preserve include_suppressed behavior by passing the flag through the
empty-results helper or finalization path, ensuring suppressed: [] is present
when enabled. Update the relevant empty-result helper API and add an N-API
regression test covering analyzeProject with includeSuppressed: true and no
reportable files.

---

Nitpick comments:
In `@crates/no-mistakes/src/codebase/rules/rust_rules_combined.rs`:
- Around line 31-47: Remove the unused _exclusive_files parameter from
check_with_files_sources_and_deferred_suppression and update every caller,
including focused tests, to use the revised signature. Ensure all Rust scanning
continues through SourceStore and eliminate any remaining exclusive-file
handling or std::fs::read_to_string bypass associated with this API.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8f0317d9-f66f-4c29-99d3-604cd22a8a5c

📥 Commits

Reviewing files that changed from the base of the PR and between 3fd3290 and 0e46a14.

📒 Files selected for processing (89)

crates/no-mistakes/src/check_parallel.rs
crates/no-mistakes/src/check_runner.rs
crates/no-mistakes/src/check_runner/results/suppression.rs
crates/no-mistakes/src/check_runner/tests/architecture.rs
crates/no-mistakes/src/codebase/rules/agents_md_max_size.rs
crates/no-mistakes/src/codebase/rules/agents_md_max_size/agents_md_max_size_budget.rs
crates/no-mistakes/src/codebase/rules/agents_md_max_size/tests.rs
crates/no-mistakes/src/codebase/rules/banned_paths/tests.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/run_rule.rs
crates/no-mistakes/src/codebase/rules/finite_set_consistency/tests/call_literals_regressions.rs
crates/no-mistakes/src/codebase/rules/mod.rs
crates/no-mistakes/src/codebase/rules/nextjs_no_api_routes.rs
crates/no-mistakes/src/codebase/rules/nextjs_no_api_routes/aggregate.rs
crates/no-mistakes/src/codebase/rules/nextjs_no_api_routes/tests.rs
crates/no-mistakes/src/codebase/rules/nextjs_no_caching.rs
crates/no-mistakes/src/codebase/rules/nextjs_no_caching/tests.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/config.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/prepared.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/runner.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/selection.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/tests/coverage_helpers.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/tests/coverage_rule_cases.rs
crates/no-mistakes/src/codebase/rules/run.rs
crates/no-mistakes/src/codebase/rules/run/prepared.rs
crates/no-mistakes/src/codebase/rules/run/prepared/execution.rs
crates/no-mistakes/src/codebase/rules/run/prepared/execution/helpers.rs
crates/no-mistakes/src/codebase/rules/run/standalone.rs
crates/no-mistakes/src/codebase/rules/rust_max_lines_per_file.rs
crates/no-mistakes/src/codebase/rules/rust_rules_combined.rs
crates/no-mistakes/src/codebase/rules/rust_rules_combined/scan.rs
crates/no-mistakes/src/codebase/rules/rust_rules_combined/tests.rs
crates/no-mistakes/src/codebase/rules/server_route_client_boundary.rs
crates/no-mistakes/src/codebase/rules/server_route_client_boundary/execution.rs
crates/no-mistakes/src/codebase/rules/server_route_client_boundary/tests.rs
crates/no-mistakes/src/codebase/rules/suppression.rs
crates/no-mistakes/src/codebase/rules/suppression_tests.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/filter.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/prepared.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/prepared_tests.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/tests.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts/graph.rs
crates/no-mistakes/src/codebase/ts_source/disable_comments/directives.rs
crates/no-mistakes/src/codebase/unique_exports/collector.rs
crates/no-mistakes/src/codebase/unique_exports/findings.rs
crates/no-mistakes/src/codebase/unique_exports/types.rs
crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs
crates/no-mistakes/src/napi_api/analyze_project/context/scope_project_reports.rs
crates/no-mistakes/src/napi_api/analyze_project/tests.rs
crates/no-mistakes/src/napi_api/tests.rs
crates/no-mistakes/src/napi_api/tests/check.rs
crates/no-mistakes/src/react_traits/pipeline/check.rs
crates/no-mistakes/src/react_traits/report/text/tests.rs
crates/no-mistakes/src/react_traits/report/types.rs
fixtures/check/aggregate-agents-md-max-size/.no-mistakes.yml
fixtures/check/aggregate-agents-md-max-size/AGENTS.md
fixtures/check/aggregate-nextjs-no-api-routes/.no-mistakes.yml
fixtures/check/aggregate-nextjs-no-api-routes/web/pages/api/legacy.ts
fixtures/check/aggregate-nextjs-no-caching/.no-mistakes.yml
fixtures/check/aggregate-nextjs-no-caching/web/app/page.ts
fixtures/check/aggregate-require-storybook-stories/.no-mistakes.yml
fixtures/check/aggregate-require-storybook-stories/web/components/ComponentSuppressed.tsx
fixtures/check/aggregate-require-storybook-stories/web/components/FileSuppressed.tsx
fixtures/check/aggregate-require-storybook-stories/web/stories/empty.stories.tsx
fixtures/check/aggregate-server-route-client-boundary/.no-mistakes.yml
fixtures/check/aggregate-server-route-client-boundary/backend/api/client.ts
fixtures/check/aggregate-server-route-client-boundary/backend/api/users.ts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports/.no-mistakes.yml
fixtures/check/aggregate-test-no-unmocked-dynamic-imports/src/leaf.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports/src/reachable.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports/tests/direct.test.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports/tests/reachable.test.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports/vitest.config.mts
fixtures/check/suppression-directive-precedence/.no-mistakes.yml
fixtures/check/suppression-directive-precedence/app/Fetcher.tsx
fixtures/check/suppression-react-multiple/.no-mistakes.yml
fixtures/check/suppression-react-multiple/app/Fetcher.tsx
fixtures/check/suppression-rust-combined/.no-mistakes.yml
fixtures/check/suppression-rust-combined/src/lib.rs
fixtures/check/suppression-unique-canonical/.no-mistakes.yml
fixtures/check/suppression-unique-canonical/src/a.ts
fixtures/check/suppression-unique-canonical/src/b.ts
fixtures/check/suppression-unique-canonical/tsconfig.json
packages/no-mistakes/report-types.d.ts

🚧 Files skipped from review as they are similar to previous changes (14)

crates/no-mistakes/src/codebase/rules/run/standalone.rs
crates/no-mistakes/src/codebase/unique_exports/types.rs
crates/no-mistakes/src/check_runner/tests/architecture.rs
crates/no-mistakes/src/react_traits/report/types.rs
crates/no-mistakes/src/react_traits/report/text/tests.rs
crates/no-mistakes/src/check_runner.rs
crates/no-mistakes/src/check_parallel.rs
crates/no-mistakes/src/codebase/rules/mod.rs
crates/no-mistakes/src/codebase/rules/suppression.rs
crates/no-mistakes/src/check_runner/results/suppression.rs
crates/no-mistakes/src/react_traits/pipeline/check.rs
packages/no-mistakes/report-types.d.ts
crates/no-mistakes/src/codebase/ts_source/disable_comments/directives.rs
crates/no-mistakes/src/codebase/rules/run.rs

Implemented all review findings with request-owned SourceStore propagation, exact suppression provenance, aggregate-only deferral, and fixture-backed parse-failure/API parity regressions.

All actionable suppression findings were addressed with fixture-backed regressions; request ownership and exact directive provenance now remain unified across aggregate adapters.

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (2)

crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable/deferred.rs (1)

58-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared import-evaluation loop.

The shared-facts branch (Lines 58-79) and the fallback branch (Lines 87-109) build an identical DynamicCheckContext and run the same loop. They differ only in the source and the import list. They also state the same suppression predicate in two inverted shapes, so a future edit can easily change one branch and not the other.

Extract one helper that takes source: &str and imports: &[DynamicImport], and call it from both branches.

♻️ Sketch of the extracted helper

#[allow(clippy::too_many_arguments)]
fn evaluate_file_imports(
    ctx: &ReachableContext<'_>,
    file: &Path,
    source: &str,
    imports: &[DynamicImport],
    mocks: &HashSet<PathBuf>,
    dependency_cache: &DashMap<PathBuf, Arc<Vec<PathBuf>>>,
    defer_suppression: bool,
    result: &mut ReachableResult,
) {
    let mut local_findings = Vec::new();
    let check_context = DynamicCheckContext {
        root: ctx.root,
        file,
        resolver: ctx.resolver,
        graph: ctx.graph,
        graph_files: ctx.graph_files,
        file_universe: ctx.file_universe,
        mocks,
        dependency_cache,
        findings: &mut local_findings,
    };
    for import in imports {
        if defer_suppression || !has_disable_comment(source, import.line as u32, RULE_ID) {
            collect_outcome(result, evaluate_dynamic_import(&check_context, import.clone()));
        }
    }
}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable/deferred.rs`
around lines 58 - 109, Extract the duplicated import-processing logic into an
evaluate_file_imports helper accepting source, imports, and the existing context
parameters. Move DynamicCheckContext construction, suppression checking, and
collect_outcome/evaluate_dynamic_import handling into the helper, using one
consistent predicate. Replace both the shared-facts and fallback loops with
calls to this helper, passing their respective source and dynamic import list
while preserving file-level suppression behavior.

crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/discovery/visible.rs (1)

108-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Report missing configured config paths. If a non-glob pattern does not resolve to a file, return a diagnostic with the pattern, resolved path, and remediation. Do not fall back to discovered_configs_from_visible when configured patterns produce no files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/discovery/visible.rs`
around lines 108 - 109, Update the configured-pattern discovery flow around
normalize_path and discovered_configs_from_visible to detect non-glob patterns
that do not resolve to a file, return a diagnostic containing the original
pattern, resolved path, and remediation guidance, and avoid falling back to
discovered_configs_from_visible when configured patterns produce no files.
Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/no-mistakes/src/codebase/rules/require_storybook_stories/runner.rs`:
- Around line 184-202: Update
crates/no-mistakes/src/codebase/rules/require_storybook_stories/runner.rs#L184-L202
so component_is_suppressed uses the prepared SourceStore via direct
normalized-path lookup or an indexed source lookup, avoiding repeated
CheckFactMap::ts scans. Update
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts.rs#L132-L147
to add sources to per_test::Request and pass it through to directive-aware
downstream checks; ensure all source consumers use the session-owned store and
invariant lookup work is outside per-component/test loops.

In
`@crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable/deferred.rs`:
- Around line 44-109: Update the file-processing branch around shared TypeScript
facts so it never calls get_or_cache_file when ctx.shared exists but source or
dynamic_imports is missing. Treat incomplete shared facts as unavailable or
reject them explicitly, while continuing to use the prepared source and
dynamic-import facts when both are present; only use the file cache fallback
when ctx.shared itself is absent.

---

Nitpick comments:
In
`@crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/discovery/visible.rs`:
- Around line 108-109: Update the configured-pattern discovery flow around
normalize_path and discovered_configs_from_visible to detect non-glob patterns
that do not resolve to a file, return a diagnostic containing the original
pattern, resolved path, and remediation guidance, and avoid falling back to
discovered_configs_from_visible when configured patterns produce no files.

In
`@crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable/deferred.rs`:
- Around line 58-109: Extract the duplicated import-processing logic into an
evaluate_file_imports helper accepting source, imports, and the existing context
parameters. Move DynamicCheckContext construction, suppression checking, and
collect_outcome/evaluate_dynamic_import handling into the helper, using one
consistent predicate. Replace both the shared-facts and fallback loops with
calls to this helper, passing their respective source and dynamic import list
while preserving file-level suppression behavior.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 443d4761-2efe-41bd-afd9-f987ae05190f

📥 Commits

Reviewing files that changed from the base of the PR and between 0e46a14 and ccdcb91.

📒 Files selected for processing (58)

crates/no-mistakes/src/check_runner/results.rs
crates/no-mistakes/src/check_runner/results/suppression.rs
crates/no-mistakes/src/check_runner/results/suppression_tests.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_helpers.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/runner.rs
crates/no-mistakes/src/codebase/rules/rust_rules_combined.rs
crates/no-mistakes/src/codebase/rules/rust_rules_combined/tests.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/discovery/visible.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable/deferred.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts/graph.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts/per_test.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts/tsconfig_catalog.rs
crates/no-mistakes/src/codebase/unique_exports/collector.rs
crates/no-mistakes/src/codebase/unique_exports/findings.rs
crates/no-mistakes/src/codebase/unique_exports/origin.rs
crates/no-mistakes/src/codebase/unique_exports/tests.rs
crates/no-mistakes/src/codebase/unique_exports/types.rs
crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs
crates/no-mistakes/src/napi_api/analyze_project/tests.rs
crates/no-mistakes/src/napi_api/tests/check.rs
crates/no-mistakes/src/napi_api/tests/check_suppression.rs
crates/no-mistakes/src/react_traits/mod.rs
crates/no-mistakes/src/react_traits/pipeline/check.rs
crates/no-mistakes/src/react_traits/pipeline/run.rs
crates/no-mistakes/src/react_traits/pipeline/run_with_facts.rs
crates/no-mistakes/src/react_traits/report/text/tests.rs
crates/no-mistakes/src/react_traits/report/types.rs
fixtures/check/aggregate-agents-md-max-size/AGENTS.md
fixtures/check/aggregate-dynamic-import-gitignored-config/.gitignore
fixtures/check/aggregate-dynamic-import-gitignored-config/.no-mistakes.yml
fixtures/check/aggregate-dynamic-import-gitignored-config/src/leaf.mts
fixtures/check/aggregate-dynamic-import-gitignored-config/tests/visible.test.mts
fixtures/check/aggregate-nextjs-no-caching/web/app/page.ts
fixtures/check/aggregate-require-storybook-stories/.no-mistakes.yml
fixtures/check/aggregate-test-no-unmocked-dynamic-imports-disabled-parse-error/.no-mistakes.yml
fixtures/check/aggregate-test-no-unmocked-dynamic-imports-disabled-parse-error/src/leaf.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports-disabled-parse-error/src/other.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports-disabled-parse-error/tests/direct.test.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports-disabled-parse-error/tests/disabled-mock.test.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports-disabled-parse-error/tests/disabled.test.mts
fixtures/check/aggregate-test-no-unmocked-dynamic-imports-disabled-parse-error/vitest.config.mts
fixtures/check/suppression-react-multiple/app/Child.tsx
fixtures/check/suppression-react-multiple/app/Fetcher.tsx
fixtures/check/suppression-unique-canonical/.no-mistakes.yml
fixtures/check/suppression-unique-canonical/shared/suppressed-origin.ts
fixtures/check/suppression-unique-canonical/shared/type-origin.ts
fixtures/check/suppression-unique-canonical/src/c.ts
fixtures/check/suppression-unique-canonical/src/chained-visible.ts
fixtures/check/suppression-unique-canonical/src/named-barrel.ts
fixtures/check/suppression-unique-canonical/src/type-barrel.ts
fixtures/check/suppression-unique-canonical/src/type-visible.ts
fixtures/check/suppression-unique-canonical/src/wild-barrel.ts

💤 Files with no reviewable changes (4)

crates/no-mistakes/src/codebase/rules/rust_rules_combined.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_helpers.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rs
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs

🚧 Files skipped from review as they are similar to previous changes (12)

fixtures/check/suppression-unique-canonical/.no-mistakes.yml
crates/no-mistakes/src/react_traits/report/text/tests.rs
fixtures/check/aggregate-nextjs-no-caching/web/app/page.ts
crates/no-mistakes/src/react_traits/pipeline/check.rs
crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs
fixtures/check/aggregate-require-storybook-stories/.no-mistakes.yml
crates/no-mistakes/src/codebase/rules/rust_rules_combined/tests.rs
crates/no-mistakes/src/napi_api/tests/check.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts/graph.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories.rs
crates/no-mistakes/src/check_runner/results.rs
crates/no-mistakes/src/check_runner/results/suppression.rs

All actionable findings from this superseded changes-requested review are addressed at 3966ff9 with saved regressions, strict Clippy, repository check, and focused suites passing.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/no-mistakes/src/codebase/rules/tests/extended.rs`:
- Around line 115-129: Update the test setup around collect_check_facts to
obtain the source from the prepared facts.ts data before applying the directive
override, instead of rereading the file with std::fs::read_to_string. Preserve
the snapshot construction and ensure all source access uses the session-owned
prepared SourceStore.

In `@packages/no-mistakes/analyze-project-types.d.ts`:
- Around line 28-31: Preserve the existing include option in BatchedCheckOptions
by adding "include" alongside "includeSuppressed" in
packages/no-mistakes/analyze-project-types.d.ts lines 28-31. Update the
declaration assertion in packages/no-mistakes/scripts/api.test.js lines 488-489
to require both "include" and "includeSuppressed".

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a861db85-dc35-4321-a6ac-a27336f95ad8

📥 Commits

Reviewing files that changed from the base of the PR and between ccdcb91 and 82a6d43.

📒 Files selected for processing (35)

crates/no-mistakes/src/check_runner/results.rs
crates/no-mistakes/src/check_runner/results/suppression.rs
crates/no-mistakes/src/check_runner/tests/architecture.rs
crates/no-mistakes/src/codebase/rules/agents_md_max_size.rs
crates/no-mistakes/src/codebase/rules/agents_md_max_size/agents_md_max_size_budget.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/runner.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/suppression.rs
crates/no-mistakes/src/codebase/rules/suppression/accounting.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable/deferred.rs
crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/tests.rs
crates/no-mistakes/src/codebase/rules/tests/extended.rs
crates/no-mistakes/src/codebase/unique_exports/findings.rs
crates/no-mistakes/src/codebase/unique_exports/types.rs
crates/no-mistakes/src/napi_api/tests/check_suppression.rs
fixtures/check/aggregate-agents-md-advisory-suppression/.no-mistakes.yml
fixtures/check/aggregate-agents-md-advisory-suppression/GUIDANCE.md
fixtures/check/aggregate-dynamic-import-same-line/.no-mistakes.yml
fixtures/check/aggregate-dynamic-import-same-line/src/leaf.mts
fixtures/check/aggregate-dynamic-import-same-line/tests/same-line.test.mts
fixtures/check/aggregate-dynamic-import-same-line/vitest.config.mts
fixtures/check/suppression-react-inherited-parents/.no-mistakes.yml
fixtures/check/suppression-react-inherited-parents/app/Child.tsx
fixtures/check/suppression-react-inherited-parents/app/ParentA.tsx
fixtures/check/suppression-react-inherited-parents/app/ParentB.tsx
fixtures/check/suppression-unique-canonical/shared/collision-origin.ts
fixtures/check/suppression-unique-canonical/shared/identity-origin.ts
fixtures/check/suppression-unique-canonical/src/collision-a.ts
fixtures/check/suppression-unique-canonical/src/collision-b.ts
fixtures/check/suppression-unique-canonical/src/collision-c.ts
fixtures/check/suppression-unique-canonical/src/identity-a.ts
fixtures/check/suppression-unique-canonical/src/identity-b.ts
packages/no-mistakes/analyze-project-types.d.ts
packages/no-mistakes/report-types.d.ts
packages/no-mistakes/scripts/api.test.js

🚧 Files skipped from review as they are similar to previous changes (7)

packages/no-mistakes/report-types.d.ts
crates/no-mistakes/src/codebase/unique_exports/types.rs
crates/no-mistakes/src/check_runner/tests/architecture.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories.rs
crates/no-mistakes/src/codebase/rules/require_storybook_stories/runner.rs
crates/no-mistakes/src/check_runner/results/suppression.rs
crates/no-mistakes/src/check_runner/results.rs

Applied the prepared-source, additive batched-check typing, formatting, and SourceStore reuse fixes with focused regressions and strict validation.

jonathanong · 2026-08-11T21:57:26Z

@codex review

github-actions Bot added documentation Improvements or additions to documentation configuration labels Aug 9, 2026

This comment has been minimized.

Sign in to view

jonathanong force-pushed the agent-suppression-contract branch from 8593134 to 444b6b9 Compare August 10, 2026 16:20

This comment has been minimized.

Sign in to view

jonathanong force-pushed the agent-suppression-contract branch from 20feb9c to 3fd3290 Compare August 10, 2026 17:32

jongleberry-bot marked this pull request as ready for review August 10, 2026 17:38

This comment was marked as resolved.

Sign in to view

coderabbitai Bot previously requested changes Aug 10, 2026

View reviewed changes

Comment thread crates/no-mistakes/src/codebase/rules/source_access.rs

Comment thread crates/no-mistakes/src/codebase/ts_source/disable_comments/directives.rs Outdated

Comment thread crates/no-mistakes/src/napi_api/tests.rs

Comment thread packages/no-mistakes/report-types.d.ts Outdated

jonathanong force-pushed the agent-suppression-contract branch from 3fd3290 to ec21c43 Compare August 10, 2026 18:51

This comment was marked as resolved.

Sign in to view

jonathanong force-pushed the agent-suppression-contract branch from ec21c43 to 0e46a14 Compare August 10, 2026 19:26

This comment was marked as resolved.

Sign in to view

coderabbitai Bot previously requested changes Aug 10, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

coderabbitai Bot previously requested changes Aug 10, 2026

View reviewed changes

Comment thread crates/no-mistakes/src/codebase/rules/require_storybook_stories/runner.rs Outdated

Comment thread crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/reachable/deferred.rs

This comment was marked as resolved.

Sign in to view

coderabbitai Bot approved these changes Aug 10, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

coderabbitai Bot previously requested changes Aug 10, 2026

View reviewed changes

Comment thread crates/no-mistakes/src/codebase/rules/tests/extended.rs

Comment thread packages/no-mistakes/analyze-project-types.d.ts Outdated

This comment was marked as resolved.

Sign in to view

coderabbitai Bot approved these changes Aug 10, 2026

View reviewed changes

jonathanong added 4 commits August 10, 2026 22:15

refactor: split domain check inputs

93a9ea5

fix: preserve ordinary suppression contracts

e69aba0

perf: account for Rust suppression source reads

833f798

test: cover prepared suppression contracts

35c9dab

This comment was marked as resolved.

Sign in to view

fix: preserve ordinary react suppression alignment

b22e561

This comment was marked as resolved.

Sign in to view

fix: preserve suppression parity in audit mode

822a4c3

This comment was marked as resolved.

Sign in to view

jonathanong added 2 commits August 11, 2026 11:24

test: cover lexical named re-export suppression

a2560d3

fix: avoid React check fact clones

0faeae0

This comment was marked as resolved.

Sign in to view

jonathanong added 2 commits August 11, 2026 11:58

refactor: reduce unique export collector complexity

78cb781

fix: retain unique export suppression provenance

a04f85e

This comment was marked as resolved.

Sign in to view

jonathanong added 2 commits August 11, 2026 12:07

fix: scope suppressed findings to check options

cdbaebe

style: format analyze project check options

41d7939

This comment was marked as resolved.

Sign in to view

fix: preserve unique export suppression lines

f10fdc2

This comment was marked as resolved.

Sign in to view

fix: preserve active unique export parity

1f751e8

This comment was marked as resolved.

Sign in to view

jonathanong added 2 commits August 11, 2026 13:25

fix: preserve origin suppression parity

729fff7

fix: preserve public unique export findings

530b01f

This comment was marked as resolved.

Sign in to view

fix: preserve suppression provenance

5d2329e

This comment has been minimized.

Sign in to view

jonathanong merged commit 89f21f3 into main Aug 11, 2026
21 checks passed

jonathanong deleted the agent-suppression-contract branch August 11, 2026 22:15

Conversation

jonathanong commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Root cause

Shepherd Journal

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment was marked as resolved.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

This comment was marked as resolved.

Uh oh!

jonathanong commented Aug 11, 2026

Uh oh!

This comment has been minimized.

Uh oh!

Reviewers

jonathanong commented Aug 9, 2026 •

edited

Loading