Repository navigation
#173 — check pair join.left-null-propagation + join.anti-join (supersedes showcase) - #195
Conversation
…ck 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>
…e 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>
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>
- 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>
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>
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>
|
Warning Review limit reached
More reviews will be available in 56 minutes and 59 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 selected for processing (15)
✨ 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 5 individual chapters for you: Chapters generated by Stage for commit 8a3e8e9 on Jun 11, 2026 12:17am UTC. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
📄 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 27315010630 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
There was a problem hiding this comment.
Code Review
This pull request introduces two new coverage checks, join.left-null-propagation and join.anti-join, along with their corresponding detectors, heuristic definitions, documentation, and comprehensive tests. The join.anti-join check is designed to supersede join.left-null-propagation when an anti-join pattern is detected, recommending a matching given row to verify exclusion. The SQL parser engine and domain layers have been updated to extract and propagate LeftJoinFacts. There are no review comments to address, and the implementation is complete and well-tested.
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.
… + 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>
…+ 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 #173
The supersedes showcase (catalog C4 + the agenda §4b anti-join refinement): two new
join.*checks where the more specific pattern recognizes the construct instead of forcing the user to suppress a false positive.What ships
join.left-null-propagation(HIGH, instrumentboth) — a LEFT JOIN whose right-side columns provably reach the projection, with no unit-test given carrying an unmatched left row. UNCOVERED emits a copy-pasteable no-match given YAML sketch; key matching is literal-cell exact on the diff: value-infer csv unit-test cells so a dict↔csv reformat shows no diff (value-normalization) #127-normalized equality key (NULL/absent left keys never match — SQL join semantics).join.anti-join(HIGH, instrumentunit-test) —LEFT JOIN … WHERE <right key> IS NULL(the IS NULL column must be an ON equi-key right column). 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.LeftJoinFactonCteGraph,#[serde(skip)]so the embedded payload stays byte-stable). Same parsed AST — no second parse. Facts hang off the graph (not nodes) so a WITH-less model (the catalog's canonical C4 shape) still surfaces its joins.model_findings→ payload path.src/adapters/render.rs,templates/*untouched.AC checklist
compute_shape_factsextension — the where-predicate fact (where_is_null_columns, top-level AND conjuncts only) rides the existing engine pass; engine tests cover alias resolution, OR-branch and unqualified IS NULL rejection, USING/non-equi degradation, derived-table right sides, Spark multi-alias projection.anti_join_supersedes_left_null_on_the_same_construct(stage 1 emits BOTH findings on the SAME construct, proving left-null fired and was silenced by resolution, not a detector skip) +anti_join_supersedes_left_null_through_the_real_engine(real sqlparser parse → production pipeline). Display-layer invariant against the real registry through #171 — check selection + suppression: [checks] config modes, suppress entries, inline pragma #193's[checks]config path:disabling_anti_join_via_checks_config_never_resurrects_left_null(disable = ["join.anti-join"]→ zero join findings) andsuppressing_anti_join_marks_it_and_never_resurrects_left_null. Wire-level: the newcoverage_checks.featurescenario asserts the payload carriesjoin.anti-joinand NOjoin.left-null-propagationthrough the real subprocess.not_exists_anti_join_is_silent_through_the_real_engine). Both checks are HIGH tier: construct triggers are near-total, but satisfaction relies on closure binding (simple-FROM chains assumed column-preserving — the documented residual) — cue, never assertion.left_null_distinct_dedup_routes_to_the_data_test: when the SELECT containing the LEFT JOIN dedups with DISTINCT (dedup after a fan-out join), the finding's evidence carriesinstrument routing+ asuggested data testsketch (unique/unique_combination_of_columnson the right key) and NO unit-test fixture sketch. Spec instrument isboth(the C4 fork).int_patients__with_conditionsUNCOVERED with the closure-bound sketch (dim_patients/stg_synthea__conditions); jaffle-shopcustomerspins two UNCOVERED constructs + one honest UNKNOWN (non-simple aggregate chain). No committed fixture carries an anti-join, sodbt-project/models/marts/customers_with_no_orders.sqlships the live showcase: the PR preview's dbt-project row rendersjoin.anti-joinUNCOVERED with the inverted sketch (verified locally against the ephemeral fusion manifest, dbt-fusion 2.0.0-preview.177).heuristics/registry.toml,book/src/checks/join.left-null-propagation.md,book/src/checks/join.anti-join.md, SUMMARY wiring;cargo test --test heuristics_ledgergreen.Golden churn
Zero. All three golden examples regenerated and byte-identical — the join-firing models are not in any golden's render scope (verified by payload inspection: jaffle golden scopes only
stg_customers; playground golden scopesdim_payers/fct_encounters_incremental/int_dq_quarantine__encounters/mart_dq_summary; diff-showcase scopesfct_provider_metrics/mart_dq_summary— none triggers the pair). The visible showcase is the live dbt-project preview row, per the golden-vs-live convention.Gates (run directly — lefthook skips in fresh worktrees)
cargo fmt --checkcargo clippy --all-targets --locked -- -D warningscargo nextest runcargo test --test bddcargo test --test headless_toggle -- --ignoredcargo test --test headless_zero_egress -- --ignoredRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --lockedcargo deny checkNo
.featurefile added (scenarios extendcoverage_checks.feature) — feature-count stays 14 in both ci.yml and lefthook.yml. No BREAKING change.🤖 Generated with Claude Code