Repository navigation
#341 — domain: composite-key enforcement inference (unique_combination_of_columns join) - #342
Conversation
…_of_columns join) Closes #341. Part of #260 (epic — PR-review governance surfaces). The composite-aware enforcement-reality inference, completing the Slice 3 (#339) deferral. A declared COMPOSITE primary-key / unique constraint asserts the TUPLE is unique (backed by unique_combination_of_columns, not per-column), so it now emits ONE tuple-keyed finding — never the false per-column "unbacked" claim the #339 guard silenced. - domain (governance.rs): new `backing_test_for_columns(manifest, model, columns) -> Option<&NodeId>` — the composite-aware uniqueness backing join by SET EQUALITY (case-insensitive). REUSES the #259 grain recognizer (`uniqueness_test_columns`, now `pub(crate)`) for "which columns does this test assert unique?" — one authority across grain + enforcement. Set equality, NOT subset: a test over a proper subset (or superset) of the tuple does NOT back it (the #341 partial-overlap AC). `backing_test_for` (the #339 single-column seam) now delegates its PK/Unique path to `backing_test_for_columns(&[column])` (so a single column also matches a 1-element combination test) and keeps the not_null path. Real-node-id attribution preserved (`Verdict::Covered.by` carries the real unique_combination_of_columns test id). - domain (checks.rs): the detector model moves from `(column, kind)` to a `DeclaredConstraint { columns, kind }` tuple — model-level constraints (single OR composite, no longer filtered) + column-level (single). The construct names the tuple: `constraint.primary_key[(a, b)]`; a single column keeps `[id]`. Removed the #339 `columns().len() == 1` composite filter. DOGFOOD (the dogfood rule): added a synthetic COMPOSITE primary_key constraint on fct_patient_summary over (patient_id, year_actual) to playground-current.json — matching the model's ALREADY-PRESENT dbt_utils.unique_combination_of_columns test — so the diff-showcase golden surfaces a real Covered composite annotation end-to-end. Surgical single-field edit (validated: only that node's `constraints` changed); MANIFEST.toml sha256 + description updated (synthetic_only intact, no root_path leak). Gating verified: the new constraint changes ONLY the diff-showcase (experimental:"1") — the governance-off playground report AND the gate-free explore page stay byte-identical (the #339 leak fix holds). Tests: backing_test_for_columns (set-equal / order+case-independent / subset-not-covered / superset-not-covered / single-column / empty / disabled); detector (one tuple finding / covered-by-combination / order-independent / partial-overlap-uncovered / single-column-unique-does- not-cover-tuple / single-column-still-fires); not_null + Other-kind backing edges; the #259 grain test scoped to its group (the composite PK now also fires enforcement on that model); the findings snapshot regenerated (composite finding only); composite headless guard (the tuple annotation renders on the showcase, real Chromium). Gates (both golden arms): fmt; clippy --all-targets --locked -D warnings on a CLEAN build (exit 0); nextest (1838); coverage 98.61% (>=85); crap4rs PASS (backing_test_for_columns 2, all touched fns <15); bdd (28); doc -D warnings; deny; heuristics ledger byte-identical; fixture-manifest gate; REPORT goldens (jaffle/playground/diff-showcase) AND EXPLORE golden (dag.html + tests.html) byte-identical; governance + enforcement + composite-enforcement + explore-zero-egress headless. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 35 minutes and 49 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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 (8)
✨ 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 |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR doesn't touch 🧭 Explore previewThe two-page 🟡 Golden exploreThe committed
🐶 Live exploreThis PR doesn't touch ▶ Open ↗ opens the report or explorer in your browser in one 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 27466101241 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements composite-key enforcement inference (cute-dbt#341), updating the constraint checking logic to support column tuples instead of flattening them to single columns. It introduces the DeclaredConstraint struct, updates the backing test matching to use set-equality over column sets, and adds comprehensive tests. The review feedback suggests two performance and correctness improvements: first, sorting columns case-insensitively and deferring owned String allocations during composite constraint deduplication to ensure order-independent deduplication; second, avoiding temporary BTreeSet allocations when checking uniqueness test set equality by using direct iterator-based checks.
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.
…ensitive dedup Re-gate corrective on #342 — a correctness inconsistency the founder gate caught + 2 gemini threads: 1. SUBSET, not set-equality (the correctness bug). `backing_test_for_columns` used set-EQUALITY, contradicting the #259 grain `covers_key` two functions away (which uses test_cols ⊆ key). Grain is right: mathematically `{a}` unique ⟹ `(a, b)` unique, so a uniqueness test on a proper SUBSET of a composite key IS a stronger guarantee that backs it. Set-equality emitted a false "unbacked" whenever a subset test existed — a never-a-false-claim violation. Changed the matcher to `test_cols.is_subset(key)` (mirroring grain's direction exactly, reusing the same `uniqueness_test_columns` extractor). Exact match is the subset boundary (still backs); a superset or partial-overlap (not contained) does not. Fixed the contradictory doc comments (governance.rs + checks.rs). Tests rewritten: subset-backs (was wrongly does-not-back), single-column-unique-covers-composite, exact-match-backs; added superset-not-covers + partial-overlap-not-covers (both at the backing_test_for_columns unit level AND the detector level). 2. Order-insensitive dedup (gemini, checks.rs). `declared_unique_constraints` deduped on an ordered `Vec<String>`, so `(a, b)` and `(b, a)` — the same uniqueness key — counted twice. Dedup now keys on the lowercased column SET (BTreeSet); the first-declared order is kept for the construct id's display. Added a both-orders → one-finding detector test. 3. perf nit (gemini, governance.rs) — `uniqueness_test_covers_key` builds a small per-candidate column set; N (PK/unique columns per test) is tiny. Negligible; replied on the thread, not changed. Dogfood golden UNCHANGED: the showcase's fct_patient_summary composite PK is backed by an EXACT-match combination test, which still backs under subset (exact match ⊆ itself) — diff-showcase byte-identical (verified). Gates (both golden arms): fmt; clippy --all-targets --locked -D warnings on a CLEAN build (exit 0); nextest (1843); coverage 98.62% (>=85); crap4rs PASS (backing_test_for_columns 2, uniqueness_test_covers_key 6, all <15); bdd (28); doc -D warnings; deny; heuristics ledger byte-identical; REPORT goldens (jaffle/playground/diff-showcase) AND EXPLORE golden (dag.html + tests.html) byte-identical; governance + enforcement + composite headless. Closes #341. Part of #260. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Closes #341. Part of #260 (PR-review governance surfaces).
The composite-aware enforcement-reality inference, completing the Slice 3 (#339) deferral. A declared COMPOSITE primary-key / unique constraint asserts the TUPLE is unique (backed by
unique_combination_of_columns, not per-column), so it now emits ONE tuple-keyed finding — never the false per-column "unbacked" claim the #339 guard silenced.What this adds
governance.rs):backing_test_for_columns(manifest, model, columns) -> Option<&NodeId>— the composite-aware uniqueness backing join by set equality (case-insensitive). REUSES the domain: coverage truthfulness pack — severity/where/disabled/singular-test attribution honesty #259 grain recognizer (uniqueness_test_columns, nowpub(crate)) for "which columns does this test assert unique?" — one authority across the grain and enforcement surfaces. Set equality, not subset: a test over a proper subset (or superset) of the tuple does NOT back it (the Composite-key enforcement inference (one tuple-keyed finding + unique_combination_of_columns join) #341 partial-overlap AC). The #260 — domain: enforcement-reality annotation + backing-test join (Slice 3) #339 single-columnbacking_test_forseam now delegates its PK/Unique path tobacking_test_for_columns(&[column])and keeps thenot_nullpath. Real-node-id attribution preserved (Verdict::Covered.bycarries the real combination-test id).checks.rs): the detector model moves from(column, kind)to aDeclaredConstraint { columns, kind }tuple — model-level constraints (single OR composite, no longer filtered) + column-level (single). The construct names the tuple:constraint.primary_key[(a, b)]; a single column keeps[id]. Removed the #260 — domain: enforcement-reality annotation + backing-test join (Slice 3) #339columns().len() == 1filter.AC coverage
unique_combination_of_columns(any namespace) whose set matches the declared key — reuses the domain: coverage truthfulness pack — severity/where/disabled/singular-test attribution honesty #259uniqueness_test_columnsrecognizer.Verdict::Covered.byattributes the real combination-test node id;Uncoveredotherwise.constraint.primary_key[(order_id, line_id)]).enforcementgroup (unchanged from #260 — domain: enforcement-reality annotation + backing-test join (Slice 3) #339).Dogfood (the dogfood rule)
Added a synthetic COMPOSITE
primary_keyconstraint onfct_patient_summaryover(patient_id, year_actual)toplayground-current.json— matching the model's already-presentdbt_utils.unique_combination_of_columnstest over the same set — so the diff-showcase golden surfaces a real Covered composite annotation end-to-end (constraint.primary_key[(patient_id, year_actual)], backed by the real test node). Surgical single-field edit (validated: only that node'sconstraintschanged);MANIFEST.tomlsha256 + description updated (synthetic_only intact, noroot_pathleak).Gating verified — the #339 leak fix holds: the new constraint changes ONLY the diff-showcase (
experimental:"1"). The governance-offplaygroundreport AND the gate-freeexplorepage stay byte-identical (enforcement is experiment-gated; both use the gated default policy).Gates (all green, both golden arms)
--all-targets --locked -D warningson a clean build (exit 0) · nextest (1838) · coverage 98.61% (≥85) · crap4rs PASS (backing_test_for_columns2, all touched fns <15)-D warnings· deny · heuristics ledger byte-identical · fixture-manifest gate🤖 Generated with Claude Code