Skip to content

#171 — check selection + suppression: [checks] config modes, suppress entries, inline pragma - #193

Merged
cmbays merged 10 commits into
mainfrom
domain-171-checks-selection-suppression
Jun 10, 2026
Merged

cmbays merged 10 commits into
mainfrom
domain-171-checks-selection-suppression

Conversation

@cmbays

@cmbays cmbays commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

User-facing check selection + suppression for the coverage-check engine (epic #168): the [checks] section of the existing --config AnalysisConfig TOML (sqlfluff-style opt-out/opt-in modes with group globs), targeted [[checks.suppress]] entries, and the inline SQL pragma -- cute-dbt: ignore(check-id, "reason") — all strictly display-layer, extending the #186 pipeline invariant.

Closes #171

Acceptance criteria — evidence

  • [checks] config POD + serde tests (mode/enable/disable validation) — src/domain/check_config.rs: ChecksConfig / ChecksMode / SuppressEntry PODs (deny_unknown_fields throughout), resolve_check_policy enforces enable ⇔ opt-in (required there; enable = [] is an explicit "display nothing"), disable ⇔ opt-out. Serde + validation-matrix unit tests; AnalysisConfig gains #[serde(default)] checks (absent section = identical-to-before default).
  • Group-glob resolution against the registry, fail-closed — exact ids + group.* globs only; unknown id/group/unsupported pattern is a usage error (exit 2, no report) whose remediation names the registry's known checks and groups. Validated at --config parse time (ConfigLoadError::Checks, the ARCHITECTURE §3 usage-error path). Glob expansion tested against a synthetic multi-check registry AND the production registry.
  • Suppression is display-layer ONLY — #169 — check-engine walking skeleton: heuristics! registry + supersedes pipeline + grain.unique-key-unbacked #186 invariant extended — apply_check_policy runs strictly after resolve_supersedes; new invariant tests: suppressing the superseding check does not resurrect the superseded finding (marked, never removed), disabling it via config removes without resurrecting, and policy application only removes-or-marks (never adds). Engine stages remain policy-blind by construction.
  • Inline pragma scanner + book grammar — scan_pragmas over the model's manifest raw_code (verbatim authored SQL; see deviation note below); full-line or trailing placement; applies model-wide. Grammar documented in book/src/check-selection.md (linked in SUMMARY; deliberately outside the generated book/src/checks/ dir, which is stale-gated).
  • Suppressed findings carry their reason into the payload — Finding.suppressed: Option<Suppression { source: config|pragma, reason }> (serde-skipped when None → pre-feature: check selection + suppression — [checks] config modes, suppress entries, inline pragma #171 payloads and goldens byte-stable). Real-fixture payload tests in tests/check_engine.rs pin both arms; the feature: report findings surface — per-model coverage checklist + tiered findings + suppressed-count #170 surface renders the collapsed set from this field.
  • BDD scenarios, synthetic only, feature-count gates bumped atomically — features/check_selection.feature (9 scenarios: both modes, empty opt-in, suppress entry, pragma with/without reason, both fail-closed error paths); expected=13 → 14 in ci.yml and lefthook.yml in the same commit. All fixtures are temp-file synthetic — nothing committed, tests/fixtures/MANIFEST.toml untouched.

Discovery calls

  • Pragma placement granularity: file-level, model-wide, any line. Construct-adjacent placement would need tokenizer span mapping for no current consumer (the one shipped check is construct-singular per model: config.unique_key). File-level is the simplest defensible grammar, matches the sqlfluff -- noqa trailing-comment ergonomics, and is documented as such in the book.
  • Reason: required for [[checks.suppress]], optional for the pragma. A config entry lives far from the code it silences, so it must carry its own justification ("we know and don't care" — say why); empty/whitespace reasons are rejected with remediation. The pragma sits beside the code it suppresses — the surrounding source + its review history are the justification surface, so the reason is optional there (and serde-skipped in the payload when absent).
  • Unknown pragma id: stderr warning, not a run failure. Config fails closed (exit 2); source text warns (cute-dbt: warning: … is not a registered check; the pragma has no effect) — failing a whole run on a typo'd SQL comment would be hostile, but silence would be comment rot.
  • No config-time warning for disabling a check with supersedes dependents (issue Discovery item 2): the non-resurrection behavior is the documented invariant, intentional configs shouldn't warn, and with a single-check registry there is no live edge to warn about. Revisit if a real supersedes pair ships and confuses someone.

Deviation (classified): pragma source is manifest raw_code, not a ProjectFileReader re-read

The issue sketch said "reuses the ProjectFileReader path". The manifest's raw_code is the verbatim authored model file (the #111 SQL-diff precedent), so re-reading the same bytes from disk would only add a --project-root requirement and a soft-failure mode — the pragma would silently stop working in baseline mode without a project root. Scanning raw_code keeps the scanner domain-pure and the pragma working in both scope modes unconditionally. Same data, strictly fewer failure modes.

Lane notes

Gates (run directly, post-rebase)

Gate Result
cargo fmt --check clean
cargo clippy --all-targets --locked -- -D warnings exit 0
cargo nextest run 972 passed, 33 skipped
cargo test --test bdd 105 scenarios / 663 steps passed
cargo test --test headless_toggle -- --ignored 32 passed
cargo test --test headless_zero_egress -- --ignored 1 passed
RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --locked exit 0
cargo deny check advisories/bans/licenses/sources ok
heuristics ledger byte-identical (SPECS unchanged — no regen needed)
crap4rs (v0.4.0 semantics, threshold 25) 1387 functions, 0 above threshold, worst 23.0, PASS
goldens all three examples regenerated locally and byte-identical; git diff -- examples/ empty

🤖 Generated with Claude Code


Open in Stage

@coderabbitai

coderabbitai Bot commented Jun 10, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

@cmbays, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 11 minutes and 20 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cb8807fd-07b6-4fed-a833-1b931cea2b28

📥 Commits

Reviewing files that changed from the base of the PR and between 33f2c5e and d106c4b.

📒 Files selected for processing (18)
  • .github/workflows/ci.yml
  • ARCHITECTURE.md
  • book/src/SUMMARY.md
  • book/src/check-selection.md
  • features/check_selection.feature
  • lefthook.yml
  • src/adapters/config_reader.rs
  • src/adapters/render.rs
  • src/cli/mod.rs
  • src/domain/check_config.rs
  • src/domain/checks.rs
  • src/domain/config.rs
  • src/domain/mod.rs
  • tests/check_engine.rs
  • tests/headless_toggle.rs
  • tests/steps/check_selection.rs
  • tests/steps/mod.rs
  • tests/steps/world.rs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch domain-171-checks-selection-suppression

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ghost

ghost commented Jun 10, 2026 •

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces check selection and suppression capabilities to cute-dbt, allowing users to tune coverage checks via a new [checks] configuration section or inline SQL pragmas. It supports dual selection modes (opt-out and opt-in) with group globs, targeted suppressions with required reasons, and inline pragmas with optional reasons. The display-layer policy is applied strictly after supersedes resolution, and invalid configurations fail closed with clear remediation text. The feedback identifies a potential parsing bug in src/domain/check_config.rs where a closing parenthesis inside a double-quoted pragma reason could prematurely truncate the parsed body, and suggests a more robust parsing approach.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/domain/check_config.rs Outdated
@github-actions

github-actions Bot commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor

📄 Rendered report preview

All golden examples regenerated cleanly.

🟡 Golden examples

Committed to examples/ and byte-identity gated — the canonical reports contributors and consumers browse. Stable across PRs.

Report View Download
diff-showcase-report.html ▶ Open ↗ ⬇ Download
jaffle-shop-report.html ▶ Open ↗ ⬇ Download
playground-report.html ▶ Open ↗ ⬇ Download

🐶 Live dogfood preview

This PR doesn't touch dbt-project/, so there's no live dogfood preview.

▶ Open ↗ opens the report in your browser in one click —
published to this repo's GitHub Pages under /pr-193/.
⬇ Download fetches the same self-contained HTML as a workflow
artifact (auth-gated; works fully offline). Either way the report
makes zero external resource requests.

The Pages preview may take ~1 min to update after this comment
posts. On PRs from forks the Open link is unavailable (read-only
token) — use Download.

Alternative: GitHub CLI
# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27312995792 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.html

Posted by report-preview.yml for d106c4bdf15febcffef8cf95886d41126bf21c1a. Affordance only — never blocks merge.

cmbays pushed a commit that referenced this pull request Jun 10, 2026
parse_pragma now consumes the quoted reason structurally before looking
for the call's closing paren, so ignore(id, "see RFC-12 (appendix B)")
parses whole instead of truncating at the inner ')' and silently failing
(PR #193 gemini review). Junk between the quoted reason and ')' is still
rejected; regression tests for both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions Bot and others added 9 commits June 10, 2026 19:13
…#171)

- ChecksConfig POD: sqlfluff-style dual modes (opt-out default with
  disable, opt-in with enable), [[checks.suppress]] entries
  (check + model + required reason), deny_unknown_fields throughout
- resolve_check_policy: fail-closed validation against the CheckId
  registry (mode/field legality, exact ids + group.* globs, unknown
  ids/globs error with remediation naming known checks/groups)
- scan_pragmas: inline '-- cute-dbt: ignore(check-id, "reason")'
  pragma grammar (file-level granularity, model-wide, reason optional)
- apply_check_policy: the grown filter_for_display stage — selection
  removes, suppression marks (Finding.suppressed carries source +
  reason into the payload); runs strictly after resolve_supersedes
- #186 invariant extended: suppressing/disabling the superseding check
  never resurrects the superseded finding

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bly (#171)

The src/adapters/render.rs touch is confined to mechanical parameter
threading (render_report_with_externals / build_payload_with_externals /
build_model_payload gain a check_policy param; the conveniences pass
CheckPolicy::default()) plus the one findings call site now applying
apply_check_policy AFTER model_findings' evaluate->resolve pipeline.
No template change — the findings surface is cute-dbt#170.

cli::build_check_policy resolves the validated [checks] config and
extends it with inline pragmas scanned from each in-scope model's
manifest raw_code (the cute-dbt#111 precedent: verbatim authored SQL,
no filesystem read, works in both scope modes). Unknown pragma ids warn
on stderr and stay inert — source text warns, config fails closed.

Real-fixture payload tests in tests/check_engine.rs pin the two arms:
disable removes (key serde-skipped), suppress keeps + marks with reason.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
#171)

load_config now resolves the [checks] section against the production
HeuristicId registry after the TOML parse; a mode/field-legality
failure, unknown check id/group glob, or glob/empty-reason suppress
entry is ConfigLoadError::Checks — the same clap usage-error path
(exit 2) as a TOML syntax error, with the CheckConfigError remediation
text naming the registry's known checks and groups.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ragma (#171)

features/check_selection.feature: 9 scenarios through the real subprocess
— opt-out disable (group glob), opt-in enable, opt-in empty enable,
suppress entry (reason carried into the payload), inline pragma with and
without a reason, and the two fail-closed usage-error paths (enable in
opt-out mode; unknown check id with remediation naming known checks).
Synthetic fixtures only (temp-file manifests + config TOML, nothing
committed). feature-count gate bumped 13 -> 14 in ci.yml AND lefthook.yml
atomically.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mar (#171)

book/src/check-selection.md (outside book/src/checks/ — that directory
is generated + stale-gated): dual selection modes with fail-closed glob
resolution, [[checks.suppress]] with required reason, the inline pragma
grammar (file-level granularity, model-wide, optional reason, unknown id
warns), and the display-layer invariant. ARCHITECTURE.md section 3
config paragraph updated for the [checks] section + ConfigLoadError::Checks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
headless_toggle's render_report_with_externals call gains the new
check_policy param (default policy); default_trait_access + an unused
test import surfaced by clippy --all-targets --locked.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…171)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
parse_pragma now consumes the quoted reason structurally before looking
for the call's closing paren, so ignore(id, "see RFC-12 (appendix B)")
parses whole instead of truncating at the inner ')' and silently failing
(PR #193 gemini review). Junk between the quoted reason and ')' is still
rejected; regression tests for both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…coverage (#171)

Post-rebase onto main @ 43b68d2 (#191): apply_check_policy now wraps the
3-arg model_findings(current, model, Some(&graph)) at the render call
site, keeping #191's CheckContext/cte_graph shape intact. The two
registry-shape-dependent tests become shape-robust (disabling an
id/group removes exactly that id/group, asserted across EVERY registered
check), and a new real-fixture test pins the display-layer invariant on
the new check: suppressing union.arm-coverage marks the finding and
changes nothing else; disabling union.* removes exactly it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cmbays
cmbays force-pushed the domain-171-checks-selection-suppression branch from 0becf4e to 98b69fd Compare June 10, 2026 23:20
@cmbays
cmbays merged commit c5d8314 into main Jun 10, 2026
32 checks passed
@cmbays
cmbays deleted the domain-171-checks-selection-suppression branch June 10, 2026 23:32
github-actions Bot added a commit that referenced this pull request Jun 10, 2026
cmbays added a commit that referenced this pull request Jun 11, 2026
…— supersedes showcase (#195)

* feat(domain): LeftJoinFact + JoinKeyPair graph facts for the join check pair

Engine-computed per-LEFT-JOIN structural facts (right-side relation
leaf, ON equi-key pairs, right-qualified IS NULL WHERE conjuncts,
provable right-column projection, DISTINCT dedup) hang off CteGraph —
tagged with their consumer body name so a WITH-less model still
surfaces its joins. #[serde(skip)]: render-pass internal, the embedded
payload stays byte-stable. cute-dbt#173

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(adapters): extract per-LEFT-JOIN facts in the existing CTE-engine pass

The cute-dbt#40 additive-facts pattern extended with the where-predicate
fact the anti-join check needs (cute-dbt#173): same parsed AST, never a
second parse. Captures alias-resolved ON equi-key pairs, top-level AND
WHERE IS NULL columns qualified by the joined relation, conservative
projection attribution (direct column / qualified wildcard / bare star
only), and the SELECT DISTINCT dedup signal. Derived-table right sides
and non-equi/USING constraints deliberately yield no facts (declared
check exclusions, negative-tested).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(domain): check pair join.left-null-propagation + join.anti-join

The supersedes showcase (cute-dbt#173, catalog C4 + agenda 4b):

- join.left-null-propagation (HIGH, instrument Both): LEFT JOIN whose
  right-side columns provably reach the projection, with no unit-test
  given carrying an unmatched left row. UNCOVERED emits the no-match
  given YAML sketch; when the containing SELECT dedups with DISTINCT
  (dedup after a fan-out join) the data-test recommendation wins —
  a uniqueness data test on the right key (C4/C10 routing, never
  unit-test-maximalist).
- join.anti-join (HIGH, unit-test): LEFT JOIN + WHERE <right key> IS
  NULL — the more specific shape SUPERSEDES left-null-propagation on
  the same construct and emits the INVERTED recommendation: a given
  row that DOES match, proving the matched class is excluded.

Both detectors enumerate one shared LeftJoinSite construct list, so the
(model, construct) supersedes join key is identical by construction.
Key matching is literal-cell exact on the #127-normalized equality key
(NULL/absent never matches — SQL join semantics); side bindings resolve
directly by external leaf or through single-external simple-FROM CTE
closures; everything else degrades to honest UNKNOWN. The NOT EXISTS
anti-join equivalent is a declared v1 exclusion. Display-layer
invariant pinned against the real registry through the #193 [checks]
config path: disabling or suppressing join.anti-join never resurrects
join.left-null-propagation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test: real-fixture + wire-level verification of the join pair

- check_engine.rs: jaffle-shop customers' three constructs pinned
  (two UNCOVERED, one honest UNKNOWN on the non-simple aggregate
  chain); playground int_patients__with_conditions UNCOVERED with the
  closure-bound no-match sketch (dim_patients / stg_synthea__conditions);
  the supersedes showcase end-to-end through the REAL sqlparser engine;
  the NOT EXISTS negative (silent, never misclassified). cute-dbt#173
- coverage_checks.feature: two subprocess wire round-trip scenarios —
  the anti-join construct carries ONLY join.anti-join with the inverted
  sketch; a plain LEFT JOIN carries only join.left-null-propagation
  with the no-match sketch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs: regenerate heuristics ledger + book pages for the join pair

GEN_HEURISTICS_LEDGER=1 cargo test --test heuristics_ledger; SUMMARY.md
wires the two generated pages. cute-dbt#173

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat: dbt-project anti-join dogfood model (supersedes showcase)

customers_with_no_orders — the canonical LEFT JOIN + WHERE right-key IS
NULL idiom, projecting the right join key so the general left-null
check provably fires and is visibly superseded. Deliberately carries NO
unit test: the live PR preview renders join.anti-join UNCOVERED with
the inverted matching-row given sketch. cute-dbt#173

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feature: check selection + suppression — [checks] config modes, suppress entries, inline pragma

1 participant