Repository navigation
#470 — raw_dag nodes + node_map.compiled (N→1, omit-on-ambiguous) - #476
Conversation
…n-ambiguous) S2 of the raw⇄compiled DAG spine (epic #468). Delivers the raw structural DAG NODES + the N→1 compiled⇄raw correspondence, projected once from the assembled SourceMap into code_map. Edges + node_map.raw (1→N, observed) are S3 (#471) and are deliberately ABSENT here — never faked. What landed (in src/adapters/render.rs, beside CodeMapPayload/gather_raw_zones): - RawDagNodePayload { id, role: cte|terminal|zone, is_zone, presence } + RawDagPayload { nodes } + NodeMapPayload { compiled } — Serialize-only projections, omit-when-empty so pre-#470 goldens stay byte-stable. - build_raw_dag_and_node_map: every node a SOUND literal raw_code region — verbatim WITH CTEs (via the SHARED #469/#473 masking scan, presence compiled_in), the #448 zone entries (honest 3-state presence), and a WITH-less model → exactly ONE terminal raw node (mirrors from_cte_graph's terminal synthesis) so node_map.compiled stays total. - node_map.compiled (compiledId → rawId, N→1): verbatim-CTE identity, the WITH-less terminal, else a unique structural-containment fold to the ONE template zone (the {% for %}-fanned CTEs). unique_containing_zone returns None on zero-or-multiple → the key is OMITTED, never a guessed rawId. The three honesty rules: - omit-on-ambiguous: a compiled node with no UNIQUE raw origin omits its key. - sound literal nodes: every raw node is a real lexical region of raw_code (never a predicted/fabricated node). - WITH-less terminal totality + fanned→one-template: one terminal raw node for a WITH-less model; all fanned compiled CTEs map back to the ONE zone. A pruned {% if is_incremental() %} zone is compiled_out (NEVER compiled_in). TDD (tests before impl, render.rs test module): nodes-sound · with-less-single-terminal · fanned-nodes-map-to-one-template · macro-CTE-no-unique-origin-omits · ambiguous-two-zones-omits · incremental-guard-compiled_out · no-raw-structure-omits-both (byte-stable) · serialize-wire-shape. Dogfood fixture: incremental-showcase GOLDEN — a SYNTHETIC manifest whose fct_events wraps an inner CTE inside {% if is_incremental() %} and whose compiled_code is the fresh-build shape (guard PRUNED), so the regenerated golden shows the verbatim `base` CTE as compiled_in and the pruned guard as a first-class compiled_out zone raw node. Hand-authored compiled_code (no real dbt compile) → no root_path/username leak; listed in MANIFEST.toml, synthetic_only. Goldens: all example reports flipped (code_map JSON grew with raw_dag.nodes + node_map.compiled); regenerated + byte-verified. jaffle-shop chrome snapshot accepted (additive raw_dag/node_map only). Source-map completeness guard, fixture-manifest gate, fmt, clippy --all-targets --locked, full nextest, BDD, cargo doc -D warnings, cargo deny, crap4rs all green. Closes #470 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199AmBCec5kyVEEF1Qd7TSF
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 30 minutes and 21 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 To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. 📝 WalkthroughWalkthroughAdds ChangesRaw DAG projection and incremental-showcase validation
Sequence Diagram(s)sequenceDiagram
participant Renderer as CodeMapPayload::from_source_map
participant Builder as build_raw_dag_and_node_map
participant SourceMap as SourceMap / node_spans
participant Payload as CodeMapPayload (JSON output)
Renderer->>Builder: call with assembled SourceMap + compiled node_spans
Builder->>SourceMap: iterate CteBody raw spans (verbatim anchors)
SourceMap-->>Builder: verbatim CTE raw nodes
Builder->>SourceMap: check for WITH-less model
SourceMap-->>Builder: synthesize single terminal raw node
Builder->>SourceMap: iterate SpanRole::Zone entries
SourceMap-->>Builder: zone nodes with Presence strings
Builder->>SourceMap: fold each compiled DAG node via unique_containing_zone
SourceMap-->>Builder: compiledId → rawId (omit on zero/multiple matches)
Builder-->>Renderer: RawDagPayload + NodeMapPayload
Renderer->>Payload: attach raw_dag + node_map (skip if empty)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
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 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 28005704936 -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 a raw structural DAG representation and a compiled-to-raw node mapping (node_map.compiled) to capture literal raw_code regions before Jinja expansion, such as verbatim CTEs, WITH-less terminals, and control zones. It includes comprehensive unit tests and synthetic test fixtures to validate these changes. The review feedback focuses on performance optimizations in src/adapters/render.rs to eliminate unnecessary string allocations and clones. Specifically, the reviewer suggests refactoring unique_containing_zone to return a zone index (Option) rather than an owned String, storing only Option in zone_compiled, and formatting the zone_id on the fly.
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: 1
🤖 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 555-568: The issue is that entry.compiled (which is just a single
anchor span from resolve_zone_compiled) is being stored in zone_compiled and
later used by unique_containing_zone for containment checking, but this anchor
does not represent the full compiled expansion of the zone. This causes {% for
%}-fanned CTEs to not be properly contained by the zone span, resulting in
missing mappings in node_map.compiled. Store or derive a true compiled expansion
span for each zone before pushing to the zone_compiled vector instead of using
entry.compiled. Additionally, add an end-to-end test that builds the SourceMap
through append_zone_entries rather than hand-authoring the full zone_compiled
span to ensure all {% for %}-fanned compiled CTEs properly map back to their raw
nodes.
🪄 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: 334952ef-81b2-4871-afcf-a8d33b10998f
⛔ Files ignored due to path filters (1)
tests/snapshots/render_integration__rendered_chrome_jaffle_shop.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
.github/workflows/ci.ymlexamples/comments-showcase-report.htmlexamples/diff-showcase-report.htmlexamples/explore-macro/tests.htmlexamples/explore-seed/tests.htmlexamples/explore/tests.htmlexamples/incremental-showcase-report.htmlexamples/jaffle-shop-report.htmlexamples/playground-report.htmlexamples/prdiff-minidag-report.htmlexamples/seed-showcase-report.htmlsrc/adapters/render.rstests/fixtures/MANIFEST.tomltests/fixtures/incremental-showcase-current.jsontests/fixtures/incremental-showcase-pr-diff.patchtests/fixtures/incremental-showcase-source/models/marts/fct_events.sqltests/fixtures/incremental-showcase-source/models/staging/stg_events.sql
…efer fanned→zone mapping to S3 (CodeRabbit Major) `build_raw_dag_and_node_map` folded compiled CTEs to a template zone via `unique_containing_zone`, which checked whether a zone's `entry.compiled` span CONTAINS the compiled node. But `entry.compiled` (from `resolve_zone_compiled`) is a single unambiguous literal ANCHOR (one ≥3-char fragment), NOT the zone's compiled EXTENT — so it can never contain the separate fanned CTE bodies. The branch was dead in every real golden, and the test `fanned_compiled_nodes_map_to_one_template_zone` masked this by hand-authoring an unrealizable span covering two CTE bodies. CodeRabbit (Major) flagged it correctly. Honest rescope: S2 now maps ONLY uniquely-locatable origins — a verbatim CTE (same id) and the WITH-less terminal. Fanned/macro CTEs have no uniquely-locatable raw origin in S2 and are honestly OMITTED (never a guessed rawId). The observed fanned→zone mapping needs the zone's compiled EXTENT + node_map.raw, so it lands in S3 #471. - Remove the zone-containment fold branch + the `zone_compiled` vec it fed (the zone raw NODES are still emitted, unchanged). - Delete the now-unused `unique_containing_zone` fn. - Rewrite `fanned_compiled_nodes_map_to_one_template_zone` → `fanned_cte_with_no_verbatim_raw_origin_is_omitted_in_s2` (a realistic anchor, asserts the fanned CTEs are OMITTED + the zone node is present). - Delete `ambiguous_two_containing_zones_omits_the_key` (tested the deleted fn; the omit-on-ambiguous concern moves to S3's observed mapping). All goldens stay byte-identical (the branch was dead in all real goldens). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0199AmBCec5kyVEEF1Qd7TSF
Revision — honest rescope (CodeRabbit Major,
|
What landed
S2 of the raw⇄compiled DAG spine (epic #468): the raw structural DAG nodes
node_map.compiledcorrespondence, projected once from theassembled
SourceMapintocode_map. New Serialize-only PODs insrc/adapters/render.rsbesideCodeMapPayload/gather_raw_zones:RawDagNodePayload { id, role, is_zone, presence },RawDagPayload { nodes },NodeMapPayload { compiled }— all omit-when-empty so pre-Raw structural DAG nodes + node_map.compiled (N→1, partial) #470 goldens staybyte-stable.
raw_coderegion: verbatimWITHCTEs (via the shared Raw-coordinate CTE/column spans (raw_node_spans + raw_column_spans) #469/Refactor raw_scan masking to the sqlparser tokenizer (evaluation — before S2) #473 masking scan —raw_node_spans,not a fork), the Raw-Jinja zones (CORE): scanner + SpanRole::Zone facts + presence (Z1+Z2) #448
Zoneentries (honest 3-state presence), and aWITH-less model → exactly ONE terminal raw node (mirrors
from_cte_graph's terminal synthesis) sonode_map.compiledstays total.compiledId → rawId, N→1) — verbatim-CTE identity,the WITH-less terminal, else a unique structural-containment fold to the
ONE template zone (the
{% for %}-fanned CTEs).unique_containing_zonereturns
Noneon zero-or-multiple → the key is omitted, never guessed.The three honesty rules
key (covered by
macro-CTE-no-unique-origin-omits+ambiguous-two-zones).raw_code, never a predicted/fabricated node (nodes-sound).for a WITH-less model (
with-less-single-terminal); all fanned compiled CTEsmap back to the ONE zone (
fanned-nodes-map-to-one-template).{% if is_incremental() %}zone iscompiled_out, nevercompiled_in(incremental-guard-compiled_out).TDD tests (before impl)
raw_dag_nodes_are_sound_literal_regions_with_a_compiled_in_cte·with_less_model_emits_exactly_one_terminal_raw_node·fanned_compiled_nodes_map_to_one_template_zone·compiled_node_with_no_unique_origin_omits_its_key·ambiguous_two_containing_zones_omits_the_key·incremental_guard_zone_node_is_compiled_out_never_compiled_in·no_raw_structure_omits_both_keys_for_byte_stable_goldens·raw_dag_and_node_map_serialize_with_stable_wire_shape.Goldens note
All example reports flipped:
code_mapJSON grew withraw_dag.nodes+node_map.compiled. Regenerated + byte-verified (deterministic re-renderconfirmed). The jaffle-shop chrome insta snapshot was accepted — the diff is
only the additive
raw_dag/node_mapblocks. The source-map completenessguard stays green (it asserts
dag.nodes ⊆ code_map.node_spans, unaffected).Dogfood fixture
New
incremental-showcaseGOLDEN — a synthetic manifest whosefct_eventswraps an inner CTE inside
{% if is_incremental() %}and whosecompiled_codeis the fresh-build shape (guard pruned). The regenerated golden shows the
verbatim
baseCTE ascompiled_inand the pruned guard as a first-classcompiled_outzone raw node — the honest incremental-CTE visibility the spinewas built for.
compiled_codeis hand-authored (no realdbt compile) → noroot_path/username leak. Listed inMANIFEST.toml(synthetic_only), wiredas an
example-report-checkmatrix row.Edges deferred to S3
Edges +
node_map.raw(1→N, observed) are S3 (#471) and are not built here— the payloads carry only
nodes+node_map.compiled, never a faked edge orempty-but-faked field. The projection (
code_mapJSON) is the S2 consumer;tests assert its shape.
Closes #470
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests