Skip to content

fix: resolve forward refs between YAML-only snapshots - #84

Merged
eitsupi merged 3 commits into
mainfrom
fix/yaml-only-snapshot-forward-ref
Jun 6, 2026
Merged

fix: resolve forward refs between YAML-only snapshots#84
eitsupi merged 3 commits into
mainfrom
fix/yaml-only-snapshot-forward-ref

Conversation

@eitsupi

@eitsupi eitsupi commented Jun 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • process_yaml_snapshot_nodes registered nodes and resolved edges in a single pass, causing order-dependent lineage for YAML-only snapshots
  • A snapshot with relation: ref(other_snapshot) where other_snapshot is declared later in the same file would incorrectly create a phantom model.<name> node instead of linking to snapshot.<name>
  • Fix: split into two passes — register all YAML-only snapshot nodes first, then resolve edges

Test plan

  • Added test_build_graph_yaml_only_snapshot_ref_forward_declaration covering the forward-declaration case (no phantom node created, correct snapshot-to-snapshot edge)
  • All 529 existing tests pass
  • cargo fmt and cargo clippy clean

🤖 Generated with Claude Code

eitsupi and others added 2 commits June 6, 2026 08:25
`process_yaml_snapshot_nodes` registered nodes and resolved edges in a
single pass, so a snapshot whose `relation: ref(...)` pointed to another
YAML-only snapshot declared later in the file would fail to find it in
`node_map`. `resolve_ref` would fall through to the `model.<name>`
default and create an incorrect phantom node instead.

Split the function into two passes: first register all YAML-only
snapshot nodes, then resolve their upstream edges. SQL-registered
snapshots are excluded from both passes (their edges are handled by
`process_sql_edges`).

Add a regression test covering the forward-declaration case.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tion

Pass 1 used `sql_registered` to skip SQL-registered snapshot nodes but
did not guard against duplicate YAML definitions sharing the same name.
Two definitions with identical names would create two graph nodes with
the same `unique_id`; `node_map` would be overwritten to the last one,
leaving the first as an orphan.

Replace the `sql_registered` set with a `yaml_registered` set that
records which unique_ids were actually added by Pass 1. Pass 1 now
checks `gb.node_map.contains_key` to skip both SQL-registered nodes
(present before the pass) and YAML duplicates (inserted earlier in the
same pass). Pass 2 iterates only over `yaml_registered`, which cleanly
excludes both categories.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes order-dependent lineage resolution for dbt v1.9+ YAML-only snapshots by ensuring forward ref()s between snapshots declared in the same YAML file resolve to snapshot.<name> nodes (instead of incorrectly creating phantom model.<name> nodes).

Changes:

  • Refactored YAML-only snapshot processing into a two-pass flow: register all snapshot nodes first, then resolve relation: edges.
  • Added a regression test covering forward-declared YAML-only snapshot ref() dependencies.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
crates/dlin-core/src/graph/builder/mod.rs Splits YAML-only snapshot handling into node-registration and edge-resolution passes to remove order dependence.
crates/dlin-core/src/graph/builder/tests/build_graph.rs Adds a test ensuring forward ref()s between YAML-only snapshots don’t create phantom nodes and produce the correct edge.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

`yaml_registered.contains()` matched every definition sharing a name,
so duplicate YAML snapshot entries could all add edges to the single
registered node in pass 2 — producing duplicate or conflicting edges.

Switch to `yaml_registered.remove()` so only the first definition seen
(which matches the one registered in pass 1) contributes edges.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@eitsupi
eitsupi merged commit 2f8d01a into main Jun 6, 2026
12 checks passed
@eitsupi
eitsupi deleted the fix/yaml-only-snapshot-forward-ref branch June 6, 2026 08:37
@eitsupi eitsupi mentioned this pull request Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants