Repository navigation
#57 — source() binding: extend messy-import-CTE fallback to dbt source() references - #204
Conversation
…, name) lookup The cute-dbt#57 domain widening: a separate SourceNode POD (not a Node kind-variant — sources carry no checksum and dbt keeps them a distinct type end-to-end), keyed by the wire map key in a new Manifest.sources map. Manifest::with_sources follows the Node::with_column_descriptions builder precedent so the 40 existing Manifest::new call sites stay untouched; source_by_name resolves the authored (source_name, name) pair case-insensitively, symmetric with the ref binding contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ns to import CTEs Adapter half of cute-dbt#57. WireSource is the tolerant projection of a top-level sources entry — every field #[serde(default)] Option per the cute-dbt#145 engine-divergence rule (core emits explicit nulls, fusion omits keys; a degenerate entry never fails the manifest, ADR-5). parse_source_ref is the exact sibling of parse_ref_name (verbatim authored string in both engines; case-insensitive keyword, whitespace tolerant, single-quoted positional args). Binding resolves the authored (source_name, name) pair through the sources map, derives the leaf token identifier → relation_name last segment → name (quote-stripped), and feeds it through the existing two-pass find_import_node_id — no new matching machinery, no preflight change (an unresolvable pair stays unbound, fail-open like an unresolvable ref). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…inding coverage The cute-dbt#57 AC fixture: stg_mixed carries BOTH a ref()-based and a source()-based import CTE, the unit test mocks both inputs, and the sources block ships both engine dialects (core-style explicit nulls + fusion-style absent keys). Registered in tests/fixtures/MANIFEST.toml (synthetic_only = true, SHA-256 pinned). render_integration proves the file → Stage-1 → sources → payload-binding vertical; the headless test proves the Node-detail panel renders the bound source() given's fixture card (not the empty-state copy) on node click. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces the 'source() references are not bound' v0.1 fidelity limit (closed by cute-dbt#57) with the residual authored-form caveat, and documents the sources-block resolution hop ahead of the two-pass match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThe PR extends cute-dbt's messy import-CTE fallback to handle dbt ChangesSource binding for unit test givens
Sequence Diagram(s)sequenceDiagram
participant given as Given input
participant parse as parse_source_ref
participant manifest as Manifest.sources
participant derive as source_relation_token
participant cte as find_import_node_id
given->>parse: source('a','b')
parse->>parse: extract (a,b)
parse-->>given: Some((a,b))
given->>manifest: source_by_name(a,b)
manifest-->>given: SourceNode
given->>derive: compute leaf token
derive->>derive: strip identifier quotes
derive-->>given: unquoted_token
given->>cte: match token in CTE body
cte-->>given: matched_node_id
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 478e7f8 on Jun 11, 2026 2:43am 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 27320273995 -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 support for binding source() unit-test inputs to leaf CTE nodes (cute-dbt#57). It introduces a new SourceNode domain model, updates manifest parsing to handle both dbt-core and dbt-fusion dialects of the top-level sources block, and adds logic to resolve and bind source('source_name', 'table_name') inputs. The review feedback suggests optimizing performance in src/adapters/render.rs by returning a &str slice from source_relation_token instead of an owned String to avoid redundant heap allocations, and consequently passing the token directly to find_import_node_id without double referencing.
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.
…view) Gemini PR 204 finding: every branch of the token derivation is a quote-strip view of the SourceNode's own fields, so the function can return &str borrowed from the node and skip the .to_owned() heap allocation; find_import_node_id takes the slice directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…#205) + golden re-render The PR-body-promised verification now that cute-dbt#205 landed the regenerated playground pair: the REAL dbt-core-1.11-compiled test_stg_synthea__patients_renames_and_hashes_source_columns with given input source('synthea_raw', 'patients') binds to the canonical 'source' import CTE through the manifest sources block — encoded as a permanent integration test over the committed fixture (the fusion rule's verify-vs-real-fixture requirement). examples/playground-report.html regenerated per the example-report-check recipe; the byte diff is exactly ONE payload field (non-payload HTML byte-identical): models[6].tests[0].given[0].bound_to_node: null -> "source" (stg_synthea__patients / the source given). jaffle-shop and diff-showcase are unchanged (the showcase patch does not scope stg_synthea__patients into the pr-diff report). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
…mini fix delta) All three examples re-rendered per the example-report-check recipe after merging origin/main (#204/#205/#211/#212). Byte audit: every changed line traces to the truthful fold-toggle/copy-outcome fix (a4c95d7) — main's cross-join/source-binding golden content came through the merge intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mini fix delta) All three examples re-rendered per the example-report-check recipe after merging origin/main (#204/#205/#211/#212). Byte audit: every changed line traces to the truthful fold-toggle/copy-outcome fix (a4c95d7) — main's cross-join/source-binding golden content came through the merge intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng, split-view folds, per-diff fold toggle, copy-icon buttons (#213) * feat(render): design-2 fold steppers — gutter +/− steppers, expand-step setting, split-view folds, per-diff fold toggle, copy-icon buttons Ports the pass-2 directional fold model onto the shipped diff renderers (cute-dbt#199, design package design-return-2): - settings.expandStep in cute-dbt.settings.v1 (default 20, clamp 0–500, 0 = all; NaN-tolerant hydrate) + the static #settings-expand-step row (the #188 static-markup contract). Read live by expandFold/contractFold — no re-render on change. - Unified renderer: data-fold-dir (leading fold expands UP toward the hunk, else DOWN), gutter .fold-steppers (+ by step / − by step, disabled at bounds), per-hunk .fold-collapse-all once anything is revealed, label progression Show N unchanged lines → All N lines shown, and updateFoldControl re-parks the control adjacent to the remaining hidden run (below the run when fully revealed). Reveal stays parent-scoped via closest(code, tbody) — the #132 duplicate-fold-id rule. - Split renderer folds long context runs with the SAME fold model (fold row: stepper gutter + colspan label cell) and gains the ds-c-num/ds-c-code colgroup (3.8em/auto) for a true 50/50; setAllFolds drives both layouts from one control set, so __cuteExpandAllFolds / __cuteCollapseAllFolds keep mirroring every fold control (label + aria-expanded + steppers + collapse-all — the #136 symmetric-mirror invariant on the new anatomy). - Per-diff fold toggle (buildFoldToggleBtn, aria-pressed) in the Model-SQL and Model-YAML diff code headers; the #132 top-of-report .diff-expand-all strip is removed (diffAllExpanded / renderExpandAllToggle / bindDiffViewControls retired with it). - copyIconBtn (inline-SVG icon, aria-label Copy, copied-class flash, execCommand fallback) replaces the absolutely-positioned text Copy in the Model-SQL header and is added to the Model-YAML header; code-header padding per engine/base.css. Functional-SVG-only iconography (README §2.4) — no icon fonts, nothing external, zero-egress untouched. No payload change; no src/domain change. Goldens + insta snapshots regenerated — every changed byte traces to this delta. Closes #199 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: supersede the #132/#136 fold guards with the #199 step-expansion contract Deliberate guard migration (never deleted, each replaced with the new-behavior assertion): - block_diff_folds_long_context_runs_and_reveals_on_activate: activate = expand-by-step (default step 20 >= hidden 4 still reveals all in one activation); the 'Hide N unchanged lines' relabel assertion is superseded by 'All N lines shown' + disabled + stepper + visible collapse-all; re-collapse moved from band-click to the explicit .fold-collapse-all. Kept: parent-scoped reveal, control stays visible, aria-expanded truth, Enter/Space keyboard activation, short-block never folds, new data-fold-dir/steppers markup assertions. - global_expand_collapse_mirrors_every_fold: the #136 bidirectional mirror retained on the new control anatomy (label, stepper disabled-states, collapse-all visibility); now also pins the retired top-of-report .diff-view-controls strip at 0 nodes. - settings_context_lines_refolds_block_diffs_live: contextLines re-render still live; extended with the expandStep no-re-render proof (a block mounted before the setting change steps by the new value). - split_diff_renders_the_same_block_diff_as_unified: parity extends to folds — same directional control anatomy, count, colgroup geometry, band activation + setAllFolds mirror on the split tbody. - model_sql_section_defaults_to_diff_and_toggles_to_raw / yaml_diff_drawer_defaults_to_diff_and_toggles_to_authored: assert the per-diff fold toggle + the inline-SVG copy-icon button (real focusable <button>, aria-label Copy) in both code headers. - NEW expand_step_steppers_reveal_contract_directionally_and_persist: + reveals exactly step lines toward the hunk, − re-hides them mirroring direction, collapse-all restores, up/down direction proofs, control re-parking, and cute-dbt.settings.v1 persistence across reload. - NEW per_diff_fold_toggle_drives_its_own_diffs_folds: the header toggle expands/restores every fold in its own diff (unified + split) with aria-pressed + label tracking. render_block_diff_honors_a_configurable_fold_pad is unchanged — its fold-pad arithmetic and labels survive the new model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(render): truthful fold-toggle + copy outcome state (Gemini PR #213 findings) Applies all three Gemini findings — each holds the new controls to the same symmetric-DOM-mirror standard the #136 guards assert: 1. copyIconBtn signals failure truthfully: success flashes 'copied', failure flashes 'copy-failed' (title AND aria-label carry the outcome; both reset after the flash). Write strategy now mirrors copySql — writeText, then the shared execCommand fallbackCopy (which reports its own success) on rejection/absence. The .copy-failed CSS is no longer dead. 2. buildFoldToggleBtn is stateless: the click derives intent from the DOM at activation time (any hidden folded line in this diff => expand-all, else collapse-all) instead of a cached boolean that desyncs under per-hunk stepping or the __cute hooks. 3. setAllFolds keeps the per-diff toggles truthful: every fold mutation funnels through updateFoldControl -> syncFoldToggles, which relabels each toggle (text + aria-pressed) from ITS OWN diff's DOM truth via the root accessor stored on the element. Per-root truth is deliberately stronger than relabeling 'toggles in scope' to one shared state: a partial-scope op never lies about an untouched sibling diff, and per-hunk steppers stay covered too. New/extended guards (headless_toggle): - per_diff_fold_toggle_drives_its_own_diffs_folds: (a) global __cuteExpandAllFolds flips the toggle's label/aria-pressed; a click after the global op acts on DOM truth and collapses; (b) stepping the unified fold fully keeps the toggle truthful while the split twin still holds hidden rows, and the next click expands the remainder. - NEW copy_icon_button_signals_failure_truthfully: both write paths stubbed to fail in-page (writeText rejects, execCommand false) => copy-failed + 'Copy failed' title/aria-label, never 'copied', reset to rest state after the flash. insta render_integration snapshot regenerated (inlined interaction.js). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: regenerate goldens on the merged tree (main @ 3cd2441 + the Gemini fix delta) All three examples re-rendered per the example-report-check recipe after merging origin/main (#204/#205/#211/#212). Byte audit: every changed line traces to the truthful fold-toggle/copy-outcome fix (a4c95d7) — main's cross-join/source-binding golden content came through the merge intact. 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>
Summary
Extends the messy-import-CTE fallback (#34 / PR 17) to dbt
source('a', 'b')references — the v0.2-deferred half of #34. dbt compiles{{ source('a','b') }}into the resolved relation at compile time, so binding resolves the authored pair through the manifest's top-levelsourcesblock instead of the compiled SQL.Closes #57
What changed
SourceNodePOD (separate type, not aNodekind-variant: sources carry nochecksum, and fusion keeps them a distinct type end-to-end) +Manifest.sourcesmap.Manifest::with_sourcesbuilder (theNode::with_column_descriptionsprecedent — 40 existingManifest::newcall sites untouched);source_by_nameresolves the authored(source_name, name)pair case-insensitively.WireSourcetolerant projection: every field#[serde(default)] Optionper the feat: surface incremental-model unit-test semantics (incremental badge + expect=merged-rows tooltip) #145 engine-divergence rule (dbt-core emits explicitnulls; fusion omits keys entirely). A degenerate entry never fails the manifest (ADR-5) — it just never matches, fail-open.parse_source_ref, the exact sibling ofparse_ref_name(both engines serializegiven[].inputverbatim;source()takes exactly two args — fusion-source-verified, SHA-pinned in the research note). Binding flow: resolve the pair → derive the leaf token (identifier→ last segment ofrelation_name→name, quote-stripped) → feed through the existing two-passfind_import_node_id. No new matching machinery, no preflight change, no newPreflightErrorvariant (sources are referenced by models, never analyzed).bound_to_node+interaction.jsfilter; zero template changes (kept clear of builder-198's surface).tests/fixtures/ref-and-source-import-cte.json: a model with BOTH a ref-based and a source-based import CTE, unit test mocking both, sources block in both engine dialects (core-style explicit nulls + fusion-style absent keys). Registered intests/fixtures/MANIFEST.toml(synthetic_only = true, SHA-256 pinned).MANIFEST.toml, so no AUDIT.md edit was needed.Tests
parse_source_refsuite mirroringparse_ref_name's, token-derivation fallbacks (identifier override, embedded-quote"GROUP"case, relation-name fallback, name fallback), payload binding (canonicalwith source as (…)shape, identifier override, fusion-minimal entry, unresolvable pair → unbound, no-sources-map → unbound, ref+source mixed test binding each given to its own CTE).Gates (all run directly — lefthook skips in fresh worktrees)
cargo fmt --checkcargo clippy --all-targets --locked -- -D warningscargo nextest runcargo test --test bddcargo test --test headless_zero_egress --locked -- --ignoredcargo test --test headless_toggle --locked -- --ignoredRUSTDOCFLAGS="-D warnings" cargo doc --no-deps --lockedcargo deny checkcargo llvm-cov+crap4rs --config crap4rs.tomlGoldens
All three golden examples (
jaffle-shop,playground,diff-showcase) regenerated per theexample-report-checkmatrix — zero byte diff, as expected: no committed example carries asource()given yet, and the binding adds no payload keys to ref-only reports.Coordination
Real-fixture verification vs the regenerated playground pair lands when #64 merges; this PR will rebase + verify then (the playground at c8270a4 carries
test_stg_synthea__patients_renames_and_hashes_source_columnswithinput: source('synthea_raw', 'patients')— exactly the canonical shape this PR's synthetic fixture mirrors). This PR deliberately does not touchtests/fixtures/playground-*,examples/playground-report.html, or any template/CSS surface.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
source()calls in unit test givens, enabling unit tests to resolve source-based import CTEs alongside existingref()support.Documentation
source()parsing limits in unit test givens (positional single-quoted form only) and refined Import-CTE binding behavior for source resolution with fallback handling.