Repository navigation
#267 — config-tree attribution: fqn-prefix widening + provenance chips - #284
Conversation
…pe widening, provenance chips The C2 slice of epic #262 (founder gate 2026-06-11): a dbt_project.yml config-tree edit is the ONE widening category — by-DEFINITION change, TOTAL tier. - domain/project_def: ConfigLeafPath (structured leaf identity on ProjectChange.tree, one label/path format authority), ConfigAttribution, and attribute_config_tree_changes — fusion's get_config_for_fqn fqn-segment prefix descent (dbt-parser dbt_project_config.rs:109-122 @ 9977b6cbb1b7) with deepest-match-wins shadowing; chips name the edited winning subtree; empty-fqn nodes are never guessed; ProjectFacts gains the per-model attribution map (degrade arms attribute nothing). - domain/scope: widen_with_config_attributions — union semantics (attributed models join models_in_scope, their unit tests join in_scope as context, changed untouched, changed ⊆ in_scope preserved). - cli: gather_project_facts moves BEFORE scope selection and now takes the manifest; categorized panels carry attributions; the run loop widens the selection so scope counts + Stage-2 preflight see the true in-scope set. - adapters/render + templates: ModelPayload.config_attributions chips ("+materialized via dbt_project.yml · models.shop.marts", JS-rendered per model selection), and the panel's models:-section config-tree rows gain the affected-models listing — counts always explicit, names inline up to the R1b cap (10), past it collapsed into a <details> listing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…apshot updates - features/project_definition.feature: two subprocess round-trip scenarios — the marts-folder edit widens the subtree's models into scope with chips + the affected-models row (and pulls the widened model's unit test in as context, not changed); the project-level edit honors deepest-match-wins (the marts-shadowed model stays out, the staging model widens with a models.bdd_project chip). Same feature file — the 23-count mirrors are untouched. - steps: canonical dbt_project.yml gains the project-level +materialized line (line 11, lines 1-10 byte-stable); fqn-bearing manifest builders (model_node_with_fqn). - headless_toggle: the committed-showcase guard now pins the "affects 20 models" sentence, the R1b overflow <details>, staging exclusion, and the landing model's chip text; new synthetic guard for chip visible/hidden states. - insta snapshots: the static model-attribution element + #267 CSS (intended-only drift, audited). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… diff-showcase) The existing synthetic patch's marts +materialized hunk now widens the 20 marts models into the diff-showcase scope: every one renders with its provenance chip, and the panel's config-tree row shows the capped affected-models listing (20 > the R1b inline cap, so the collapsed listing is exercised in the canonical artifact). jaffle-shop + playground pick up only the static chrome additions (baseline arm never widens). Explore pages unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…c backticks Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 42 minutes and 57 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ 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 (3)
📝 WalkthroughWalkthroughThis PR implements config-tree change attribution (issue ChangesConfig-tree attribution and provenance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 10 individual chapters for you: Chapters generated by Stage for commit 5de5beb on Jun 12, 2026 6:59am UTC. |
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR doesn't touch ▶ 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 27400081243 -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 implements config-tree change attribution and scope widening (cute-dbt#267), allowing dbt_project.yml config-tree edits to automatically bring affected models and their unit tests into the report scope. It introduces FQN-prefix matching with deepest-match-wins resolution, updates the CLI execution flow, and adds UI elements like provenance chips and affected-model listings. The review feedback suggests performance optimizations in affected_models_strings and widen_with_config_attributions to avoid unnecessary string cloning and collection allocations by using borrowed slices and in-place mutation.
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.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/domain/project_def.rs (1)
1024-1057: ⚡ Quick winExpand the serde matrix for the new additive fields.
These tests only cover one populated
ConfigAttributionand one populatedProjectFactscase. They don't pin the load-bearing combinations introduced here:ProjectChange.treeomitted vs present,ConfigLeafPath.segmentsempty vs non-empty, andconfig_attributionsomitted vs populated. This payload is explicitly additive, so the repo's usual table-driven round-trip matrix would give much better regression coverage here.As per coding guidelines, property tests are required for JSON serde round-trip behavior. Based on learnings, this repo expects those round-trips to be covered with exhaustive table-driven matrices rather than one-off examples.
🤖 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 `@src/domain/project_def.rs` around lines 1024 - 1057, Add table-driven/property tests to expand JSON serde coverage for the new additive fields: replace or supplement the one-off tests (config_attribution_round_trips_through_json and project_facts_with_attributions_round_trip_and_empty_map_is_omitted) with a matrix of cases that exercise combinations of ProjectChange.tree omitted vs present, ConfigLeafPath.segments empty vs non-empty, and config_attributions omitted vs populated; for each case serialize with serde_json::to_string and deserialize back with serde_json::from_str and assert equality (use ConfigAttribution, ProjectFacts, ProjectChange, ConfigLeafPath and the config_attributions map identifiers to build test inputs), and add a property/test harness (table of struct cases or proptest) so all combinations are round-trip covered.Sources: Coding guidelines, Learnings
🤖 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 `@src/adapters/render.rs`:
- Around line 3231-3291: Add a table-driven serde round-trip test that
exhaustively covers presence/absence combinations of config_attributions for
models instead of only asserting serialized key count in
build_payload_attaches_config_attribution_chips_to_widened_models: create a
matrix of cases (e.g., model with Some(vec!), model with Some(empty vec), model
with None/omitted) and for each case call build_payload_with_externals to
produce payload, serialize with serde_json::to_string, then deserialize back
into the payload type and assert equality as well as explicit checks for whether
config_attributions is omitted from JSON when None/empty according to
skip_serializing_if; reference the existing test function name, the
ConfigAttribution struct, payload.models entries (fct and stg lookups), and
build_payload_with_externals to locate where to add the new table-driven
assertions.
- Around line 1519-1528: The dedupe currently inserts leaf_segment(node_id) into
the BTreeSet which collapses distinct full model IDs that share the same bare
name; change affected_models_by_leaf to insert the full node_id (or the unique
model ID string) into the BTreeSet (e.g., out.entry((attribution.path.clone(),
attribution.key.clone())).or_default().insert(node_id.clone())) so
deduping/counting uses full IDs, then defer deriving display names (calling
leaf_segment or applying disambiguation) at render time when building the
human-facing list; apply the same change wherever leaf_segment(node_id) is
inserted into these dedupe sets within this function.
In `@src/domain/project_def.rs`:
- Around line 573-580: changed_models_tree_keys currently collects keys into a
BTreeSet which deduplicates identical keys and collapses multiple edits for the
same model; change it to preserve every occurrence (and original order) by
returning a Vec<&String> (or Vec<&str>) and collecting with .map(|t|
&t.key).collect::<Vec<_>>() instead of .collect() into a BTreeSet; update the
signature of changed_models_tree_keys and any callers (e.g., attribute_one_fqn
and the analogous logic flagged at 588-614) to iterate over the Vec and emit a
ConfigAttribution for each contributing tree/key occurrence rather than only
once per unique key.
---
Nitpick comments:
In `@src/domain/project_def.rs`:
- Around line 1024-1057: Add table-driven/property tests to expand JSON serde
coverage for the new additive fields: replace or supplement the one-off tests
(config_attribution_round_trips_through_json and
project_facts_with_attributions_round_trip_and_empty_map_is_omitted) with a
matrix of cases that exercise combinations of ProjectChange.tree omitted vs
present, ConfigLeafPath.segments empty vs non-empty, and config_attributions
omitted vs populated; for each case serialize with serde_json::to_string and
deserialize back with serde_json::from_str and assert equality (use
ConfigAttribution, ProjectFacts, ProjectChange, ConfigLeafPath and the
config_attributions map identifiers to build test inputs), and add a
property/test harness (table of struct cases or proptest) so all combinations
are round-trip covered.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d7435d15-7a42-4b1f-a2c3-bfbfdb581d6d
⛔ Files ignored due to path filters (2)
tests/snapshots/golden_report__rendered_report_skeleton.snapis excluded by!**/*.snaptests/snapshots/render_integration__rendered_chrome_jaffle_shop.snapis excluded by!**/*.snap
📒 Files selected for processing (15)
examples/diff-showcase-report.htmlexamples/jaffle-shop-report.htmlexamples/playground-report.htmlfeatures/project_definition.featuresrc/adapters/render.rssrc/cli/mod.rssrc/domain/mod.rssrc/domain/project_def.rssrc/domain/scope.rstemplates/interaction.jstemplates/report.csstemplates/report.htmltests/headless_toggle.rstests/steps/builders.rstests/steps/project_definition.rs
Gemini review threads on #284: - affected_models_strings: the inline (≤ R1b cap) arm now joins straight from borrowed &str — the owned name list is cloned only on the over-cap arm that actually returns it. (The inline arm still allocates its Vec<&str> + the joined String — the win is the N avoided String clones, not zero allocation.) - widen_with_config_attributions: in-place union over the by-value ScopeSelection via new Extend impls on InScopeSet/ModelInScopeSet (the FromIterator companions, ModifiedSet::insert mutator precedent) — pre-existing members are no longer re-cloned into rebuilt sets; the per-key NodeId is allocated once and reused for both the manifest existence probe and the membership set. A fully borrowed &str keyset would not save that allocation: Manifest::node keys on &NodeId with no Borrow<str> bridge, so the probe needs the owned id either way. Pure refactor: output byte-identical (goldens + snapshots untouched). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…bare-name twins CodeRabbit thread on #284: affected_models_by_leaf deduped by bare model name, so a section-root edit selecting same-named models across packages (legal since dbt 1.6 two-arg ref) would under-count — a TOTAL-tier truthfulness corner. The dedupe/count set now carries full node ids; display names derive at sentence-build time (affected_display_names), keeping the bare selector-vocabulary form wherever unique and falling back to the full node id only on collision, so the listing never shows two indistinguishable entries. Also pins the config_attributions wire shape explicitly in the payload test (present-with-content + absent — the Vec field's two states). Committed goldens are byte-identical (single-package fixtures; verified by regen). New regression: affected_models_count_by_node_id_and_ disambiguate_bare_name_twins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Overnight orchestration wrap-upMerged as What shipped (#262 C2, the lane's ONE widening category): the fqn-prefix descent matcher verified against fusion's 🤖 Generated with Claude Code |
…panel intents Both #262 slices land on the same panel: #267's config-tree rows keep their affected-models sentence + R1b overflow listing and the provenance chips; #269's hooks rows keep the operation-node SQL diff slot and the dispatch row its UNKNOWN-tier banner. ProjectChange carries both additive fields (hook + tree); project_change_panel threads the manifest once for both consumers (hook enrichment + fqn attribution); project_panel_row composes both row decorations. Goldens regenerated over the merged tree (never hand-picked) — the diff-showcase now renders the 20-model widening listing AND the hooks/dispatch rows; deltas audited intended-only. Full gate sweep re-run green (fmt, clippy --locked, nextest 1489, bdd 179, headless 11+79, doc both modes, deny, crap4rs 2278 fns 0 above threshold). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
The #262 epic's C2 slice (CORE appetite): a
dbt_project.ymlconfig-tree edit is the ONE scope-widening category — by-DEFINITION change, TOTAL tier (founder gate 2026-06-11). A+configsubtree edit now selects the models whose fqn falls under the edited tree path into the report's scope, with provenance chips naming the contributing subtree.Closes #267
What landed
Domain — the matcher (
src/domain/project_def.rs)ConfigLeafPath— structured leaf identity carried onProjectChange.treefor config-tree changes; one label/dotted-path format authority, so the panel rows and the attribution can never drift.attribute_config_tree_changes— mirrors fusion'sget_config_for_fqnexactly (dbt-parserdbt_project_config.rs:109-122@9977b6cbb1b761065536300037560d8e3c037011): fqn-segment prefix descent breaking on the first missing child, deepest-match-wins per key (children inherit unset fields —recur_build_dbt_project_config:297-336,default_to). A model is selected exactly when its fqn-resolved value differs between the old and newmodels:trees — an edit shadowed by a deeper unchanged setting selects nothing; package subtrees never select own-project models; empty-fqn nodes are never guessed.ProjectFacts.config_attributions— per-model attribution map; every panel-degrade arm attributes nothing (no parsed pair ⇒ never a guessed widening).Scope widening (
src/domain/scope.rs)widen_with_config_attributions— union semantics, never replacement: attributed models joinmodels_in_scope, their unit tests joinin_scopeas context (the sametarget_modified ⇒ test in-scopeOR-arm both scope sources apply),changeduntouched,changed ⊆ in_scopepreserved by construction.CLI (
src/cli/mod.rs)gather_project_factsmoves before scope selection (it now feeds it) and takes the manifest; the run loop widens the selection, so scope counts integrate and Stage-2 preflight sees the true in-scope set. Baseline arm and every fallback arm widen nothing.Render (
src/adapters/render.rs, templates)ModelPayload.config_attributionschips —+materialized via dbt_project.yml · models.healthcare_analytics.marts— rendered per model selection beside the model selector; hidden (out of the AT tree) for unattributed models.models:-section config-tree rows gain the affected-models listing — counts always explicit (TOTAL tier, a truthful "affects 0 models in this manifest" included); names inline up to the R1b cap (10), past it collapsed into a<details>listing ("listed, not individually rendered"). Non-models:sections make no claim. Panel template edits stay inside the row loop (hooks/dispatch row region untouched — adapters: hooks + dispatch panel rows — operation-node surfacing + diff-index path normalization #269 runs concurrently).Matcher property-test inventory (house exhaustive style, no proptest dep)
attribution_membership_is_exactly_contiguous_fqn_prefix_descent— exhaustive edit-path × fqn pool: selection ⇔ contiguous-prefix membershipattribution_deepest_match_wins_a_shadowed_edit_selects_nothing_under_the_shadowattribution_root_tree_edit_selects_all_own_project_modelsattribution_package_subtree_never_selects_own_project_modelsattribution_selects_models_under_the_edited_subtree_by_fqn_prefixattribution_removal_attributes_to_the_removed_leaf/attribution_addition_attributes_to_the_added_leafattribution_of_a_non_config_tree_change_is_empty(reflexivity),attribution_never_selects_an_empty_fqn_or_non_model_node,attribution_ignores_non_models_sectionsConfigAttribution,ProjectFacts(+ empty-map omission for pre-domain: config-tree change attribution — fqn prefix matching, scope widening + provenance chips #267 byte stability)widen_adds_attributed_models_and_their_tests_as_context,widen_with_empty_attributions_is_identity,widen_unions_with_an_existing_selection_never_replaces,widen_skips_an_attribution_for_a_node_absent_from_the_manifestBDD (same feature file — the 23-count mirrors untouched)
fct_daily(chipmaterializedviamodels.bdd_project.marts), keepsstg_rawout, pulls the widened model's unit test in as context (changed: false), panel row states "affects 1 model — widened into report scope: fct_daily".models.bdd_projectedit widensstg_raw(chip viamodels.bdd_project) while the marts-shadowedfct_dailystays out.Dogfood — diff-showcase golden
The existing synthetic patch's marts
+materialized view→tablehunk now widens the 20 marts models into the showcase: all 20 render with chips, the panel row shows "affects 20 models — widened into report scope, listed below" with the R1b overflow<details>exercised in the canonical artifact; staging/intermediate models stay out (precision visible). jaffle-shop + playground goldens pick up only the static chrome additions (baseline arm never widens); explore pages unchanged.git diff --text -U0 -- examples/audited intended-only; no path leaks (/Users|cmbaysgrep clean).Gates (run directly, fresh-worktree lefthook caveat honored)
cargo fmt --all --check✓ ·cargo clippy --all-targets --locked -- -D warnings✓ (by exit code)cargo nextest run --locked✓ 1459 passed · BDDcargo test --test bdd✓ 23 features / 176 scenarios / 1150 stepsheadless_zero_egress -- --ignored(11) +headless_toggle -- --ignored(78, incl. the new chip + affected-listing guards)cargo doc --no-deps --locked-D warnings✓ and--document-private-items✓ ·cargo deny check✓crap4rs.toml+ fresh lcov): 0 above threshold (25), worst 24.1, PASS🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes