Repository navigation
#172 — check union.arm-coverage: unexercised UNION ALL arms - #191
Conversation
Catalog class C3: a model whose body UNION ALLs N arms where the unit-test givens leave one or more arms unexercised. The detector consumes EXISTING CteGraph union facts (union-typed edges + body_leaf_table_refs, cute-dbt#40) and the cute-dbt#131 given↔leaf-ref binding — no new AST pass, no new parsing. - CheckContext gains the model's already-parsed CteGraph (Option; the renderer threads its single parse_cte_graph pass through model_findings) — the second evidence family on the extraction ladder. - Tier HIGH (honest arm-attribution accuracy), instrument unit-test. - Discovery settled + encoded + documented in the generated book page: a given bound to a relation shared by several arms exercises every arm whose closure reads it (rows provably enter each arm's scan; per-arm filter survival is out of scope — cue, never assertion). UNCOVERED requires a provably-unfed arm; unbindable arms, external-fixture/non-literal-sql givens, and ungiven seed inputs (fusion render_unit_test mocks only given entries, 9977b6cbb1b761065536300037560d8e3c037011) degrade to honest UNKNOWN. - Recommendation payload carries the unexercised arm + a concrete given-row YAML sketch built from the input model's declared columns. - Ledger regenerated (registry.toml + book check pages + SUMMARY). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#172) Runs the engine over the committed fusion-compiled playground fixture with each model's REAL CteGraph threaded through (the renderer's single-parse pass): mart_dq_summary COVERED with test attribution, dim_payers' sentinel arm honest UNKNOWN, fct_clinical_events UNCOVERED with the per-arm given-row sketch. A second insta snapshot pins the serialized findings payload (the cute-dbt#170 render-surface contract) across all three verdict shapes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The catalog C3 charges/refunds worked example, self-dogfooded: a new stg_refunds staging model feeds a third UNION ALL arm in order_metrics.all_statuses, and the order_metrics unit test mocks it DELIBERATELY EMPTY (rows: []) — the live PR-diff preview report now surfaces the union.arm-coverage UNCOVERED finding with its given-row recommendation sketch, alongside the COVERED status_dim union (dogfood-alongside-feature rule; cute-dbt#125 precedent). Verified against dbt-fusion 2.0.0-preview.177 locally: compile clean, test_order_metrics_computes_amount_share passes with the empty given (the arm contributes zero rows; expect unchanged). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) playground: dim_payers gains the UNKNOWN sentinel-arm finding, mart_dq_summary the COVERED finding with test attribution; diff-showcase mirrors mart_dq_summary. jaffle-shop is byte-identical (no UNION-bearing model). Non-payload HTML verified byte-identical — the diff is exactly the appended findings payload entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 37 minutes and 35 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 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 configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Ready to review this PR? Stage has broken it down into 8 individual chapters for you: Chapters generated by Stage for commit f7feac0 on Jun 10, 2026 11:01pm UTC. |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR's own
▶ Open ↗ opens the report in your browser in one click — The Pages preview may take ~1 min to update after this comment Alternative: GitHub CLI# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27311902644 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Code Review
This pull request introduces the union.arm-coverage heuristic check to identify unexercised UNION arms in dbt models based on unit-test fixtures. It includes updates to the documentation, registry configuration, model rendering integration, and extensive unit tests with real-world playground data. Feedback on the implementation suggests simplifying the parsing logic in given_input_leaf for better readability when identifying ref and source calls.
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.
Find the first '(' and compare the keyword before it, instead of the
per-keyword prefix probe — semantically identical (same accept set for
ref/source with optional whitespace), easier to read. Suggested by the
PR review bot.
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>
…suppress entries, inline pragma (#193) * feat(domain): [checks] selection + suppression policy, pragma scanner (#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> * feat(cli): build + thread the check display policy into payload assembly (#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> * feat(adapters): fail-closed [checks] validation at --config parse time (#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> * test: BDD scenarios for [checks] modes, suppression, and the inline pragma (#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> * docs: book page for check selection, suppression, and the pragma grammar (#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> * test: fix render call site + pedantic lints in test code (#171) 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> * docs: qualify the resolve_supersedes intra-doc link in check_config (#171) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(domain): pragma reason may contain a closing parenthesis (#171) 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> * test: extend selection + suppression invariant coverage to union.arm-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> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ecklist + tiered findings + suppressed-count The render lane of the coverage-intelligence epic (#168): the per-model Coverage checks panel rendering the #186/#191 engine verdicts in-context. - Payload (render.rs): FindingPayload wraps the domain Finding (flattened, wire keys unchanged) with render-resolved pin_node (DAG anchor: bracketed constructs pin the named CTE node, model-level constructs the terminal) and sketches (the union check's 'suggested given' evidence lifted into copyable blocks). ReportPayload gains check_specs — the offline spec catalog (name/tier/instrument/conditions/exclusions/rationale + the click-only book_href), keyed by fired check only, serde-skipped when empty so findings-free payloads stay byte-stable. - Surface (report.html/interaction.js/report.css): a self-contained .model-findings section per model — three-valued checklist (mark + word, never colour-only), labeled tier chips (TOTAL solid / HIGH outlined / advisory muted — never blended, no percentages), covered-by attribution, evidence rows, evidence pinning (select + smooth-scroll + flash), the recommendation with #188-pattern copyable YAML sketches, the inline 'What is this check?' rationale drawer (fully offline; the book link is a plain click-only anchor), and the visible-but-quiet suppressed reveal (collapsed count -> rows with reason + source). Token-native on the #187 chassis; tier-chip tooltips use the #146/#188 bubble contract. - Dogfood: playground fixture gains unique_key on mart_dq_summary (grain UNCOVERED), an inline ignore pragma on dim_payers (suppressed-with-reason), and the synthetic-body-mod idiom on fct_clinical_events (union UNCOVERED with three given-row sketches) — all three goldens now show the panel, jaffle-shop deliberately renders the quiet empty state. - Tests: 7 render.rs payload units, 2 BDD scenarios (check_specs catalog facts), 2 headless guards (panel/checklist/tier distinctness/tooltip/ sketch+copy/rationale/book-link/pin/both-toggle-views/empty-state; suppressed collapsed-count + reason reveal); insta snapshots + goldens regenerated intended-only. Closes #170 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…+ tiered findings + suppressed-count (#194) * feat(adapters): #170 — report findings surface: per-model coverage checklist + tiered findings + suppressed-count The render lane of the coverage-intelligence epic (#168): the per-model Coverage checks panel rendering the #186/#191 engine verdicts in-context. - Payload (render.rs): FindingPayload wraps the domain Finding (flattened, wire keys unchanged) with render-resolved pin_node (DAG anchor: bracketed constructs pin the named CTE node, model-level constructs the terminal) and sketches (the union check's 'suggested given' evidence lifted into copyable blocks). ReportPayload gains check_specs — the offline spec catalog (name/tier/instrument/conditions/exclusions/rationale + the click-only book_href), keyed by fired check only, serde-skipped when empty so findings-free payloads stay byte-stable. - Surface (report.html/interaction.js/report.css): a self-contained .model-findings section per model — three-valued checklist (mark + word, never colour-only), labeled tier chips (TOTAL solid / HIGH outlined / advisory muted — never blended, no percentages), covered-by attribution, evidence rows, evidence pinning (select + smooth-scroll + flash), the recommendation with #188-pattern copyable YAML sketches, the inline 'What is this check?' rationale drawer (fully offline; the book link is a plain click-only anchor), and the visible-but-quiet suppressed reveal (collapsed count -> rows with reason + source). Token-native on the #187 chassis; tier-chip tooltips use the #146/#188 bubble contract. - Dogfood: playground fixture gains unique_key on mart_dq_summary (grain UNCOVERED), an inline ignore pragma on dim_payers (suppressed-with-reason), and the synthetic-body-mod idiom on fct_clinical_events (union UNCOVERED with three given-row sketches) — all three goldens now show the panel, jaffle-shop deliberately renders the quiet empty state. - Tests: 7 render.rs payload units, 2 BDD scenarios (check_specs catalog facts), 2 headless guards (panel/checklist/tier distinctness/tooltip/ sketch+copy/rationale/book-link/pin/both-toggle-views/empty-state; suppressed collapsed-count + reason reveal); insta snapshots + goldens regenerated intended-only. Closes #170 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(adapters): #170 review dispositions — in-place Cytoscape pin, flash-timer reset, own-property tally guard - finding-pin under the Cytoscape engine now emits a tap on the live cy node (the exact bound click path: in-place lineage classes + __cuteSelectNode) instead of renderDag() — honoring the #180 no-rebuild-per-click contract (CodeRabbit); headless guard added (same cy instance + .sel class after a pin). - rapid re-pins clear the pending flash timer so an old timeout can't cut the new animation short (gemini). - verdict tally uses an own-property guard (gemini). - goldens + chrome snapshot regenerated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(adapters): #170 post-rebase hardening — tag-like payload escaping + qualified-construct pins Rebased onto main with the #173 join check pair (#195), whose spec prose ('WHERE <right>.<key> IS NULL') now rides the payload through the #170 check_specs catalog: - payload_json_for_html_script escapes EVERY tag-opening '<' shape ('</', '<!', '<?', '<letter') to \u003c — inert in browsers either way, but raw '<right>' read as markup to non-HTML5 tag scanners (the tl-based BDD payload extractors choked); bare '< ' / '<digit' (compiled-SQL comparisons) stay raw, preserving the documented contract. - resolve_pin_node understands the #173 qualified construct form (left_join[<consumer>:<right>]) and pins the consumer CTE; unit test added. - the #195 suggested-given BDD steps read the sketch from the finding's 'sketches' array (the #170 lift moved it out of evidence — their sketches get the copy-button affordance for free); the step now also pins that the entry is never duplicated back into evidence. 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>
Closes #172
Second check in the coverage-intelligence registry (epic #168, catalog class C3):
union.arm-coverage— a model whose body UNION ALLs N arms where the unit-test givens leave one or more arms unexercised. The detector pressure-tests the engine seam with a second evidence family: the existing CteGraph union facts rather than manifest config.Acceptance criteria
CheckContextgains the model's already-parsedCteGraph(Option<&CteGraph>); the renderer threads its singleparse_cte_graphpass throughmodel_findings(one-line call-site change inbuild_model_payload). Arms = union-typed edges (UnionAll/UnionDistinct); per-arm inputs = the arm source's upstream CTE closure's externalbody_leaf_table_refs(cute-dbt#40 facts); given binding mirrors the Cell-diff: carry per-given identity so duplicate same-ref givens resolve to the correct table diff #131/feature: messy import CTE fallback — relax the clean-import-CTE v0.1 assumption #34 ref/source leaf-name binding (case-insensitive, last-quoted-argument).unexercised arm(refunds — no given row reaches ref('stg_refunds')) plus a copy-pasteablesuggested givenblock (- input: ref('…')\n rows:\n - {col: ..., …}) built from the input model's declared columns — the catalog's charges/refunds worked example, live in theplayground_union_arm_findingssnapshot.detectmatch. Five declared exclusions, each with a paired negative test: invisible arm shapes (no union edge ⇒ silent), constant/unbindable arms ⇒ UNKNOWN, external-fixture/non-literal-sql givens ⇒ UNKNOWN, ungiven seed inputs ⇒ UNKNOWN,thisgivens never feed an arm.union_shared_ref_given_exercises_every_arm_it_reaches; documented inbook/src/checks/union.arm-coverage.md(generated from the spec).mart_dq_summaryCOVERED with attribution,dim_payerssentinel arm UNKNOWN,fct_clinical_eventsUNCOVERED with per-arm sketches (tests/check_engine.rs+ insta snapshot). Dogfood-alongside: newstg_refundsstaging model + a third UNION ALL arm inorder_metrics.all_statusesthat the unit test mocks deliberately empty — the live PR-diff preview on this PR self-renders the UNCOVERED finding + sketch (verified locally against dbt-fusion 2.0.0-preview.177: compile clean,test_order_metrics_computes_amount_sharestill passes;dbt buildconfirms the empty arm contributes zero rows).Fusion-first verification
Per-given runtime semantics pinned to dbt-fusion
9977b6cbb1b761065536300037560d8e3c037011(render_unit_test,dbt-tasks-sa/src/renderable/renderable/unit_test.rs): fusion builds one mock CTE pergivenentry — a relation with no given keeps reading its real table. That is why an unmocked seed input degrades to honest UNKNOWN (dbt allows seed inputs to go ungiven), never UNCOVERED.Golden churn (reviewed)
Rebased onto
main@ 8690d42 (post-#189/#190), goldens regenerated. Non-payload HTML verified byte-identical; the diff is exactly the appended findings entries:examples/playground-report.html:dim_payers+UNKNOWN,mart_dq_summary+COVEREDexamples/diff-showcase-report.html:mart_dq_summary+COVEREDexamples/jaffle-shop-report.html: byte-identical (no UNION-bearing model)Gates (run directly, post-rebase)
cargo fmt --check✓ ·cargo clippy --all-targets --locked -- -D warnings✓ (exit 0) ·cargo nextest run941 passed ✓ ·cargo test --test bdd97 scenarios / 612 steps ✓ ·headless_toggle -- --ignored32 passed ✓ ·headless_zero_egress -- --ignored✓ ·RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --locked✓ ·cargo deny check✓ · heuristics ledger regenerated + byte-gate ✓ · crap4rs: 1342 functions, 0 above threshold 25, worst 23.0, PASS (new detector fns all 100% covered, worst CRAP 16).No new
.featurefile (feature-count gates untouched). No BREAKING CHANGE.🤖 Generated with Claude Code