From 1bde7d12ee5c049dc3aa532f66100576dee4ae36 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:22:17 -0400 Subject: [PATCH 1/3] =?UTF-8?q?fix(domain):=20#254=20=E2=80=94=20resolve?= =?UTF-8?q?=20versioned=20models=20via=20tested=5Fnode=5Funique=5Fid?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A versioned dbt model's unique_id ends in its version suffix (model.proj.dim_customers.v2), so leaf-segment bare-name matching can never bind it — covered-by attribution and every name-keyed lookup silently missed versioned models. Ingest the engine-resolved ids on the domain UnitTest (tested_node_unique_id + the this_input_node_unique_id twin; fusion dbt-schemas manifest_nodes.rs:273-274 @ 9977b6cb) and add resolve_tested_model: prefer the direct nodes lookup when the id is present, fall back to resolve_target_model bare-name matching when absent/dangling/non-model (graceful absence for dbt-core and older manifests). Switch every unit-test target resolution in the domain (scope, state, preflight, checks) to the new bridge. Co-Authored-By: Claude Fable 5 --- src/domain/checks.rs | 7 +- src/domain/mod.rs | 2 +- src/domain/preflight.rs | 4 +- src/domain/scope.rs | 8 +-- src/domain/state.rs | 146 +++++++++++++++++++++++++++++++++++++--- src/domain/unit_test.rs | 101 +++++++++++++++++++++++++++ 6 files changed, 249 insertions(+), 19 deletions(-) diff --git a/src/domain/checks.rs b/src/domain/checks.rs index 7a54b742..cb413089 100644 --- a/src/domain/checks.rs +++ b/src/domain/checks.rs @@ -99,7 +99,7 @@ use crate::domain::cte::{ }; use crate::domain::grain::test_is_enabled; use crate::domain::manifest::{Manifest, Node, NodeConfig, NodeId}; -use crate::domain::state::resolve_target_model; +use crate::domain::state::{resolve_target_model, resolve_tested_model}; use crate::domain::unit_test::{UnitTest, UnitTestGiven}; use crate::domain::unit_test_table::{CellValue, FixtureTable, table_from_manifest_rows}; @@ -1148,8 +1148,7 @@ fn detect_union_arm_coverage(ctx: &CheckContext<'_>) -> Vec .unit_tests() .iter() .filter(|(_, ut)| { - resolve_target_model(ctx.manifest, ut.model()) - .is_some_and(|model| model.id() == ctx.model.id()) + resolve_tested_model(ctx.manifest, ut).is_some_and(|model| model.id() == ctx.model.id()) }) .collect(); tests.sort_by(|a, b| a.0.cmp(b.0)); @@ -1568,7 +1567,7 @@ fn tests_on_model<'a>(ctx: &'a CheckContext<'_>) -> Vec<(&'a String, &'a UnitTes .unit_tests() .iter() .filter(|(_, unit_test)| { - resolve_target_model(ctx.manifest, unit_test.model()) + resolve_tested_model(ctx.manifest, unit_test) .is_some_and(|model| model.id() == ctx.model.id()) }) .collect(); diff --git a/src/domain/mod.rs b/src/domain/mod.rs index 58bf2c69..5f60b478 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -88,7 +88,7 @@ pub use scope::{ScopeInput, ScopeSelection, all_models, changed_models, select_i pub use state::{ BANNER_EMPTY_SCOPE, BodyChecksumModifier, ConfigsModifier, ContractModifier, InScopeSet, MacrosModifier, ModelInScopeSet, ModifiedSet, ModifierKind, RelationModifier, StateComparator, - StateModifier, resolve_target_model, + StateModifier, resolve_target_model, resolve_tested_model, }; pub use unit_test::{UnitTest, UnitTestExpect, UnitTestGiven, UnitTestOverrides}; pub use unit_test_table::{ diff --git a/src/domain/preflight.rs b/src/domain/preflight.rs index 934b2d2b..077dac8c 100644 --- a/src/domain/preflight.rs +++ b/src/domain/preflight.rs @@ -30,7 +30,7 @@ use thiserror::Error; use crate::domain::manifest::Manifest; -use crate::domain::state::{InScopeSet, ModelInScopeSet, resolve_target_model}; +use crate::domain::state::{InScopeSet, ModelInScopeSet, resolve_tested_model}; /// Domain-level fail-closed currency for the two-stage preflight check. /// @@ -172,7 +172,7 @@ fn first_in_scope_test_for_model( let Some(unit_test) = current.unit_test(test_id) else { continue; }; - if let Some(resolved) = resolve_target_model(current, unit_test.model()) { + if let Some(resolved) = resolve_tested_model(current, unit_test) { if resolved.id() == model_id { return Some(unit_test.name().to_owned()); } diff --git a/src/domain/scope.rs b/src/domain/scope.rs index 37b11ac8..c89a98c3 100644 --- a/src/domain/scope.rs +++ b/src/domain/scope.rs @@ -34,7 +34,7 @@ use std::collections::{BTreeSet, HashMap}; use crate::domain::manifest::{Manifest, NodeId}; use crate::domain::pr_diff::NormalizedDiffIndex; use crate::domain::state::{ - InScopeSet, ModelInScopeSet, ModifierKind, StateComparator, resolve_target_model, + InScopeSet, ModelInScopeSet, ModifierKind, StateComparator, resolve_tested_model, }; /// Source of the in-scope set: either a baseline manifest (dbt @@ -209,7 +209,7 @@ fn select_in_scope_pr_diff(current: &Manifest, index: &NormalizedDiffIndex) -> S let mut in_scope_ids: Vec = Vec::new(); let mut changed_ids: Vec = Vec::new(); for (test_id, ut) in current.unit_tests() { - let target_path_modified = resolve_target_model(current, ut.model()) + let target_path_modified = resolve_tested_model(current, ut) .is_some_and(|model| path_modified_models.contains(model.id())); let test_yaml_changed = ut .original_file_path() @@ -232,7 +232,7 @@ fn select_in_scope_pr_diff(current: &Manifest, index: &NormalizedDiffIndex) -> S let tests_per_model: HashMap = current .unit_tests() .values() - .filter_map(|ut| resolve_target_model(current, ut.model()).map(|m| m.id().clone())) + .filter_map(|ut| resolve_tested_model(current, ut).map(|m| m.id().clone())) .fold(HashMap::new(), |mut acc, id| { *acc.entry(id).or_insert(0) += 1; acc @@ -241,7 +241,7 @@ fn select_in_scope_pr_diff(current: &Manifest, index: &NormalizedDiffIndex) -> S let mut model_ids: BTreeSet = BTreeSet::new(); for test_id in in_scope.iter() { if let Some(ut) = current.unit_test(test_id) { - if let Some(model) = resolve_target_model(current, ut.model()) { + if let Some(model) = resolve_tested_model(current, ut) { model_ids.insert(model.id().clone()); } } diff --git a/src/domain/state.rs b/src/domain/state.rs index e19fb831..fa29b200 100644 --- a/src/domain/state.rs +++ b/src/domain/state.rs @@ -424,8 +424,9 @@ impl StateComparator { /// Unit-test ids in scope for this diff. /// /// A unit test is in scope when **either** its target model is in the - /// modified set (resolved from the bare `model:` name via - /// [`resolve_target_model`]) **or** the unit test itself was added or + /// modified set (resolved via [`resolve_tested_model`] — the + /// engine-resolved id when present, the bare `model:` name + /// otherwise) **or** the unit test itself was added or /// changed relative to the baseline. The second arm is ADR-3's "a /// changed test on an unchanged model is in scope": because dbt unit /// tests are a top-level manifest map — not checksum-bearing `nodes` — @@ -449,7 +450,7 @@ impl StateComparator { ) -> InScopeSet { let mut in_scope = InScopeSet::new(); for (id, unit_test) in current.unit_tests() { - let target_modified = resolve_target_model(current, unit_test.model()) + let target_modified = resolve_tested_model(current, unit_test) .is_some_and(|node| modified.contains(node.id())); let test_changed = unit_test_is_changed(baseline, id, unit_test); if target_modified || test_changed { @@ -466,7 +467,7 @@ impl StateComparator { /// /// 1. Every model that is the resolved target of an in-scope unit test /// (the same models the existing `in_scope_unit_tests` would surface - /// via `resolve_target_model`). + /// via `resolve_tested_model`). /// 2. Every modified **`model`** node that has **zero** unit tests /// targeting it in the current manifest — the "no tests wired" /// signal. The modified set itself is resource-agnostic (dbt's @@ -495,7 +496,7 @@ impl StateComparator { let Some(unit_test) = current.unit_test(test_id) else { continue; }; - if let Some(model) = resolve_target_model(current, unit_test.model()) { + if let Some(model) = resolve_tested_model(current, unit_test) { ids.insert(model.id().clone()); } } @@ -505,7 +506,7 @@ impl StateComparator { // `state:modified` matcher is resource-agnostic; generic test / // seed / snapshot nodes all qualify), but only `model` nodes render // as cards — so arm 2 projects to `resource_type == "model"` - // exactly as arm 1 does via `resolve_target_model` and as the + // exactly as arm 1 does via `resolve_tested_model` and as the // PrDiff arm does in `select_in_scope_pr_diff` (cute-dbt#167). for modified_id in modified.iter() { let is_model = current @@ -603,17 +604,45 @@ fn leaf_segment(id: &str) -> &str { id.rsplit('.').next().unwrap_or(id) } +/// Resolve a unit test's target model, preferring the engine-resolved +/// id over bare-name matching (cute-dbt#254). +/// +/// When the manifest carries `tested_node_unique_id` (both engines emit +/// it on every resolvable unit test; for **versioned** models it is the +/// `.vN`-suffixed `unique_id` — fusion `dbt-parser` +/// `resolve_unit_tests.rs` @ `9977b6cb…`), the target is a direct +/// `nodes` lookup. This is the only resolution that can bind a +/// versioned model: its `unique_id` ends in the version suffix +/// (`model.shop.dim_customers.v2`), so [`resolve_target_model`]'s +/// leaf-segment match against the bare `model:` name can never reach it. +/// +/// Graceful absence (ADR-5 tolerance): a missing/null field, an id +/// dangling outside `nodes`, or an id naming a non-`model` node all +/// fall back to [`resolve_target_model`] — the exact pre-#254 behavior +/// for dbt-core and older manifests. +#[must_use] +pub fn resolve_tested_model<'m>(manifest: &'m Manifest, unit_test: &UnitTest) -> Option<&'m Node> { + if let Some(node) = unit_test + .tested_node_unique_id() + .and_then(|id| manifest.node(id)) + .filter(|node| node.resource_type() == "model") + { + return Some(node); + } + resolve_target_model(manifest, unit_test.model()) +} + /// Build a map from resolved model node id to the unit-test ids in /// `manifest` that target it. /// /// Used by [`StateComparator::models_in_scope`] to determine which /// modified models have zero unit tests targeting them. Resolution is -/// via [`resolve_target_model`]; unresolvable `model:` references +/// via [`resolve_tested_model`]; unresolvable `model:` references /// contribute nothing to the map (they are skipped, not failed). fn unit_test_targets(manifest: &Manifest) -> HashMap> { let mut map: HashMap> = HashMap::new(); for (test_id, unit_test) in manifest.unit_tests() { - if let Some(model) = resolve_target_model(manifest, unit_test.model()) { + if let Some(model) = resolve_tested_model(manifest, unit_test) { map.entry(model.id().clone()) .or_default() .push(test_id.clone()); @@ -1904,6 +1933,107 @@ mod tests { assert_eq!(resolved.map(|n| n.id().as_str()), Some("model.pkg_a.dup")); } + // ===== resolve_tested_model (cute-dbt#254) ===== + + /// A unit test targeting `model_bare` whose engine-resolved target id + /// is `tested_id` (the wire `tested_node_unique_id`). + fn unit_test_with_tested_id(model_bare: &str, tested_id: &str) -> UnitTest { + unit_test_for(model_bare, None).with_tested_node_unique_id(Some(NodeId::new(tested_id))) + } + + #[test] + fn resolve_target_model_misses_a_versioned_model_by_bare_name() { + // The cute-dbt#254 bug shape: a versioned model's unique_id ends + // in its version suffix (`model.shop.dim_customers.v2` → + // leaf `"v2"`), so bare-name leaf matching can never bind it. + // This pins WHY the engine-resolved id is load-bearing. + let m = manifest(vec![model("model.shop.dim_customers.v2", "c")], vec![]); + assert!(resolve_target_model(&m, &id("dim_customers")).is_none()); + } + + #[test] + fn resolve_tested_model_binds_a_versioned_model_via_engine_resolved_id() { + let m = manifest(vec![model("model.shop.dim_customers.v2", "c")], vec![]); + let ut = unit_test_with_tested_id("dim_customers", "model.shop.dim_customers.v2"); + assert_eq!( + resolve_tested_model(&m, &ut).map(|n| n.id().as_str()), + Some("model.shop.dim_customers.v2"), + ); + } + + #[test] + fn resolve_tested_model_falls_back_to_bare_name_when_id_absent() { + // Graceful absence: dbt-core / older manifests omit the field — + // resolution degrades to the exact pre-#254 behavior. + let m = manifest(vec![model("model.jaffle_shop.stg_customers", "c")], vec![]); + let ut = unit_test_for("stg_customers", None); + assert_eq!( + resolve_tested_model(&m, &ut).map(|n| n.id().as_str()), + Some("model.jaffle_shop.stg_customers"), + ); + } + + #[test] + fn resolve_tested_model_falls_back_when_the_id_dangles() { + // A tested_node_unique_id naming a node absent from `nodes` + // (hand-edited manifest) must not fail resolution — the bare-name + // path still binds (ADR-5 tolerance). + let m = manifest(vec![model("model.jaffle_shop.orders", "o")], vec![]); + let ut = unit_test_with_tested_id("orders", "model.jaffle_shop.gone"); + assert_eq!( + resolve_tested_model(&m, &ut).map(|n| n.id().as_str()), + Some("model.jaffle_shop.orders"), + ); + } + + #[test] + fn resolve_tested_model_falls_back_when_the_id_names_a_non_model() { + // Defensive: a unit test's target is always a model in dbt; an id + // resolving to any other resource type falls back to name + // matching rather than mis-binding a seed as the target. + let m = manifest( + vec![ + typed_node("seed.jaffle_shop.orders", "seed"), + model("model.jaffle_shop.orders", "o"), + ], + vec![], + ); + let ut = unit_test_with_tested_id("orders", "seed.jaffle_shop.orders"); + assert_eq!( + resolve_tested_model(&m, &ut).map(|n| n.id().as_str()), + Some("model.jaffle_shop.orders"), + ); + } + + #[test] + fn in_scope_unit_tests_includes_a_test_on_a_modified_versioned_model() { + // Attribution-level proof of the cute-dbt#254 fix: a unit test on + // a versioned model enters scope when the versioned node's + // checksum changes — pre-fix the bare-name resolution missed the + // `.v2` node and the test silently dropped out of scope. + let ut = unit_test_with_tested_id("dim_customers", "model.shop.dim_customers.v2"); + let test_id = "unit_test.shop.dim_customers.v2.t"; + let current = manifest( + vec![model("model.shop.dim_customers.v2", "after")], + vec![(test_id, ut.clone())], + ); + let baseline = manifest( + vec![model("model.shop.dim_customers.v2", "before")], + vec![(test_id, ut)], + ); + let comparator = StateComparator::body_only(); + let in_scope = comparator.in_scope_unit_tests(¤t, &baseline); + assert!( + in_scope.contains(test_id), + "the versioned model's unit test must be in scope" + ); + let models = comparator.models_in_scope(¤t, &baseline); + assert!( + models.contains(&id("model.shop.dim_customers.v2")), + "the versioned model itself must be in scope (arm 1)" + ); + } + // ===== InScopeSet ===== #[test] diff --git a/src/domain/unit_test.rs b/src/domain/unit_test.rs index 2f03f7aa..378b5d0f 100644 --- a/src/domain/unit_test.rs +++ b/src/domain/unit_test.rs @@ -190,6 +190,23 @@ pub struct UnitTest { /// the authoritative incremental-branch discriminator. #[serde(default, skip_serializing_if = "Option::is_none")] overrides: Option, + /// Engine-resolved `unique_id` of the tested model (the wire + /// `tested_node_unique_id`, cute-dbt#254). Authoritative over the + /// bare [`Self::model`] name: a **versioned** model's `unique_id` + /// ends in its version suffix (`model.shop.dim_customers.v2`), which + /// leaf-segment name matching can never bind. `None` when the + /// manifest omits or null-fills the field (dbt-core / older + /// manifests) — resolution then degrades to bare-name matching. + #[serde(default, skip_serializing_if = "Option::is_none")] + tested_node_unique_id: Option, + /// Engine-resolved `unique_id` of the node backing a `this` given + /// input (the wire `this_input_node_unique_id`, cute-dbt#254) — the + /// [`Self::tested_node_unique_id`] twin for incremental-model unit + /// tests whose `given` includes `this`. Carried for the same + /// versioned-model robustness; `None` when absent or null-filled + /// (dbt-fusion never populates it as of `9977b6cb…`). + #[serde(default, skip_serializing_if = "Option::is_none")] + this_input_node_unique_id: Option, } impl UnitTest { @@ -218,6 +235,8 @@ impl UnitTest { original_file_path, is_incremental_mode: None, overrides: None, + tested_node_unique_id: None, + this_input_node_unique_id: None, } } @@ -313,6 +332,39 @@ impl UnitTest { pub fn overrides(&self) -> Option<&UnitTestOverrides> { self.overrides.as_ref() } + + /// Set the engine-resolved tested-model id (builder, cute-dbt#254). + /// The adapter threads the wire's top-level `tested_node_unique_id` + /// here — the [`Self::with_incremental_mode`] precedent (no + /// constructor churn across the many test call sites). + #[must_use] + pub fn with_tested_node_unique_id(mut self, id: Option) -> Self { + self.tested_node_unique_id = id; + self + } + + /// Engine-resolved `unique_id` of the tested model (cute-dbt#254) — + /// `None` when the manifest omits or null-fills the field. + #[must_use] + pub fn tested_node_unique_id(&self) -> Option<&NodeId> { + self.tested_node_unique_id.as_ref() + } + + /// Set the engine-resolved `this`-input node id (builder, + /// cute-dbt#254) — the [`Self::with_tested_node_unique_id`] twin. + #[must_use] + pub fn with_this_input_node_unique_id(mut self, id: Option) -> Self { + self.this_input_node_unique_id = id; + self + } + + /// Engine-resolved `unique_id` of the node backing a `this` given + /// input (cute-dbt#254) — `None` when the manifest omits or + /// null-fills the field. + #[must_use] + pub fn this_input_node_unique_id(&self) -> Option<&NodeId> { + self.this_input_node_unique_id.as_ref() + } } #[cfg(test)] @@ -697,6 +749,55 @@ mod tests { assert_eq!(back.overrides(), Some(&sample_overrides())); } + // ----- cute-dbt#254 — engine-resolved target node ids ----- + + #[test] + fn resolved_node_ids_default_none_and_skip_keys() { + let ut = bare_unit_test(); + assert!( + ut.tested_node_unique_id().is_none(), + "no engine-resolved target id by default" + ); + assert!( + ut.this_input_node_unique_id().is_none(), + "no engine-resolved `this` input id by default" + ); + let json = serde_json::to_string(&ut).unwrap(); + assert!( + !json.contains("tested_node_unique_id"), + "None must skip the key (pre-#254 payloads stay byte-stable): {json}" + ); + assert!( + !json.contains("this_input_node_unique_id"), + "None must skip the key (pre-#254 payloads stay byte-stable): {json}" + ); + } + + #[test] + fn resolved_node_ids_roundtrip_when_present() { + // The versioned-model shape the fields exist for: the engine + // resolves the bare `model:` name to the `.vN`-suffixed unique_id + // cute-dbt's leaf-segment matching can never reach. + let ut = bare_unit_test() + .with_tested_node_unique_id(Some(NodeId::new("model.shop.order_events.v2"))) + .with_this_input_node_unique_id(Some(NodeId::new("model.shop.order_events.v2"))); + assert_eq!( + ut.tested_node_unique_id().map(NodeId::as_str), + Some("model.shop.order_events.v2") + ); + assert_eq!( + ut.this_input_node_unique_id().map(NodeId::as_str), + Some("model.shop.order_events.v2") + ); + let json = serde_json::to_string(&ut).unwrap(); + assert!( + json.contains("\"tested_node_unique_id\":\"model.shop.order_events.v2\""), + "Some must emit the key: {json}" + ); + let back: UnitTest = serde_json::from_str(&json).unwrap(); + assert_eq!(back, ut, "round-trip preserves the resolved ids"); + } + #[test] fn overrides_groups_serialize_in_deterministic_btreemap_order() { let ut = bare_unit_test().with_overrides(Some(sample_overrides())); From 856e2d2087da3f8e508c2c3da8d2ddec30dc0d49 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:22:25 -0400 Subject: [PATCH 2/3] =?UTF-8?q?fix(adapters):=20#254=20=E2=80=94=20ingest?= =?UTF-8?q?=20engine-resolved=20unit-test=20target=20ids?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WireUnitTest gains tested_node_unique_id / this_input_node_unique_id (Option + serde(default): tolerant of absent keys — every committed fixture predates the fields — and explicit nulls, the engine null-fill shape). into_domain threads them via builders. The render indexer (index_tests_for_models) and both explore attribution surfaces (unit_test_counts, unit_test_paths_by_model) switch to resolve_tested_model so a versioned model's tests bind on every surface. Goldens verified byte-identical (no committed fixture uses model versions). Co-Authored-By: Claude Fable 5 --- src/adapters/explore.rs | 67 +++++++++++++++++++++++------ src/adapters/manifest.rs | 93 ++++++++++++++++++++++++++++++++++++++++ src/adapters/render.rs | 9 ++-- 3 files changed, 152 insertions(+), 17 deletions(-) diff --git a/src/adapters/explore.rs b/src/adapters/explore.rs index ebd7d106..044487a8 100644 --- a/src/adapters/explore.rs +++ b/src/adapters/explore.rs @@ -51,7 +51,7 @@ use crate::adapters::asset_embed::{ }; use crate::adapters::render::{DagPayload, ReportPayload}; use crate::domain::{ - GrainKind, Manifest, ModelInScopeSet, Node, NodeId, model_grain_signals, resolve_target_model, + GrainKind, Manifest, ModelInScopeSet, Node, NodeId, model_grain_signals, resolve_tested_model, }; /// The explorer's external-drive contract version (cute-dbt#105). @@ -93,8 +93,8 @@ pub struct LineageNode { /// `data_test_counts` helper below). pub data_tests: usize, /// Unit tests targeting this model (cute-dbt#103) — manifest - /// `unit_tests` entries whose bare `model:` reference resolves here - /// ([`resolve_target_model`], the same bridge the report uses). + /// `unit_tests` entries whose target resolves here + /// ([`resolve_tested_model`], the same bridge the report uses). pub unit_tests: usize, } @@ -128,15 +128,16 @@ fn data_test_counts(current: &Manifest) -> HashMap<&NodeId, usize> { } /// Count the unit tests per target model (cute-dbt#103): each manifest -/// `unit_tests` entry stores the BARE model name, bridged to its node -/// by [`resolve_target_model`] (the report renderer's exact resolution -/// — the two surfaces cannot disagree on a test's target). An -/// unresolvable `model:` reference contributes nothing (skipped, not -/// failed — the explore fail-open posture). +/// `unit_tests` entry is bridged to its node by [`resolve_tested_model`] +/// (the engine-resolved id when present, the bare `model:` name +/// otherwise — the report renderer's exact resolution, so the two +/// surfaces cannot disagree on a test's target). An unresolvable +/// `model:` reference contributes nothing (skipped, not failed — the +/// explore fail-open posture). fn unit_test_counts(current: &Manifest) -> HashMap { let mut counts: HashMap = HashMap::new(); for unit_test in current.unit_tests().values() { - if let Some(model) = resolve_target_model(current, unit_test.model()) { + if let Some(model) = resolve_tested_model(current, unit_test) { *counts.entry(model.id().clone()).or_insert(0) += 1; } } @@ -479,8 +480,8 @@ fn model_detail(current: &Manifest, node: Option<&Node>) -> ModelDetailPayload { } /// Collect each model's unit-test file paths (cute-dbt#105), keyed by -/// resolved target model: each `unit_tests` entry stores the BARE model -/// name, bridged by [`resolve_target_model`] (the [`unit_test_counts`] +/// resolved target model: each `unit_tests` entry is bridged by +/// [`resolve_tested_model`] (the [`unit_test_counts`] /// twin — the badge count and the paths list cannot disagree on a /// test's target). Entries are name-ordered per model (the manifest /// `unit_tests` map iterates non-deterministically). An unresolvable @@ -489,7 +490,7 @@ fn model_detail(current: &Manifest, node: Option<&Node>) -> ModelDetailPayload { fn unit_test_paths_by_model(current: &Manifest) -> HashMap> { let mut by_model: HashMap> = HashMap::new(); for unit_test in current.unit_tests().values() { - let Some(model) = resolve_target_model(current, unit_test.model()) else { + let Some(model) = resolve_tested_model(current, unit_test) else { continue; }; // given-order fixture refs, then the expect's — verbatim off @@ -951,6 +952,46 @@ mod tests { ]) } + // ----- unit_test_counts (cute-dbt#254) --------------------------- + + #[test] + fn unit_test_counts_bind_a_versioned_model_via_engine_resolved_id() { + // A versioned model's leaf segment is its version suffix + // (`…dim_customers.v2` → `"v2"`), so bare-name resolution can + // never bind it — the engine-resolved `tested_node_unique_id` + // must carry the badge attribution (cute-dbt#254). + use crate::domain::{UnitTest, UnitTestExpect}; + let versioned = model("model.shop.dim_customers.v2", Some("select 1"), &[]); + let ut = UnitTest::new( + "t1", + NodeId::new("dim_customers"), + Vec::new(), + UnitTestExpect::new(serde_json::Value::Null, None, None), + None, + DependsOn::default(), + None, + None, + None, + ) + .with_tested_node_unique_id(Some(NodeId::new("model.shop.dim_customers.v2"))); + let current = Manifest::new( + ManifestMetadata::new("v12"), + [(versioned.id().clone(), versioned)].into_iter().collect(), + [("unit_test.shop.t1.v2".to_owned(), ut)] + .into_iter() + .collect(), + StdHashMap::new(), + ); + let counts = unit_test_counts(¤t); + assert_eq!( + counts + .get(&NodeId::new("model.shop.dim_customers.v2")) + .copied(), + Some(1), + "the versioned model's badge must count its unit test", + ); + } + // ----- build_lineage -------------------------------------------- #[test] @@ -1389,7 +1430,7 @@ mod tests { } /// A minimal unit test targeting `target_bare` (the manifest stores - /// the BARE model name — resolution is `resolve_target_model`). + /// the BARE model name — resolution is `resolve_tested_model`). fn unit_test_on(target_bare: &str) -> crate::domain::UnitTest { crate::domain::UnitTest::new( format!("test_{target_bare}"), diff --git a/src/adapters/manifest.rs b/src/adapters/manifest.rs index 28cc81ec..0b23553d 100644 --- a/src/adapters/manifest.rs +++ b/src/adapters/manifest.rs @@ -333,6 +333,24 @@ struct WireUnitTest { /// case — a bare struct field rejects `null` and fails the parse. #[serde(default)] overrides: Option, + /// Engine-resolved `unique_id` of the tested model (cute-dbt#254). + /// Top-level sibling of `model` (`ManifestUnitTest + /// .tested_node_unique_id`, `dbt-schemas` + /// `manifest/manifest_nodes.rs:273` @ `9977b6cb…`). For versioned + /// models this is the `.vN`-suffixed `unique_id` — the only handle + /// that binds them (the bare `model:` name never leaf-matches a + /// versioned id). `Option` + `#[serde(default)]` tolerate both the + /// absent key (every committed fixture predates the field) and an + /// explicit `null` (the engine null-fill shape). + #[serde(default)] + tested_node_unique_id: Option, + /// Engine-resolved `unique_id` of the node backing a `this` given + /// input — the [`Self::tested_node_unique_id`] twin + /// (`manifest_nodes.rs:274` @ `9977b6cb…`; fusion never populates + /// it as of that SHA — dbt-core parity field). Same tolerance + /// contract. + #[serde(default)] + this_input_node_unique_id: Option, } /// Tolerant wire projection of the `config` sub-object on a dbt unit-test @@ -453,6 +471,8 @@ impl WireUnitTest { ) .with_incremental_mode(is_incremental_mode) .with_overrides(overrides) + .with_tested_node_unique_id(self.tested_node_unique_id) + .with_this_input_node_unique_id(self.this_input_node_unique_id) } } @@ -764,6 +784,79 @@ mod tests { ); } + // ----- cute-dbt#254 — engine-resolved unit-test target ids ------ + + #[test] + fn parse_manifest_threads_engine_resolved_target_ids() { + // The versioned-model wire shape: fusion resolves each per-version + // unit test's target to the `.vN`-suffixed unique_id + // (`tested_node_unique_id` / `this_input_node_unique_id`, + // `dbt-schemas` `manifest/manifest_nodes.rs:273-274` @ `9977b6cb…`). + let json = format!( + r#"{{ + "metadata": {{ "dbt_schema_version": "{V12_URL}" }}, + "nodes": {{ + "model.shop.dim_customers.v2": {{ + "resource_type": "model", + "checksum": {{ "name": "sha256", "checksum": "deadbeef" }}, + "compiled_code": "select 1" + }} + }}, + "unit_tests": {{ + "unit_test.shop.dim_customers.t1.v2": {{ + "name": "t1", + "model": "dim_customers", + "expect": {{ "rows": [{{"id":1}}], "format": "dict" }}, + "tested_node_unique_id": "model.shop.dim_customers.v2", + "this_input_node_unique_id": "model.shop.dim_customers.v2" + }} + }} + }}"# + ); + let manifest = parse_manifest(&json).expect("versioned manifest parses"); + let ut = manifest + .unit_test("unit_test.shop.dim_customers.t1.v2") + .expect("unit test ingested"); + assert_eq!( + ut.tested_node_unique_id().map(NodeId::as_str), + Some("model.shop.dim_customers.v2"), + ); + assert_eq!( + ut.this_input_node_unique_id().map(NodeId::as_str), + Some("model.shop.dim_customers.v2"), + ); + } + + #[test] + fn parse_manifest_tolerates_absent_and_null_resolved_ids() { + // Absent keys: every committed fixture predates the fields. + let manifest = parse_manifest(&minimal_v12_manifest()).expect("valid v12 manifest"); + let ut = manifest.unit_test("unit_test.shop.t1").expect("ingested"); + assert!(ut.tested_node_unique_id().is_none()); + assert!(ut.this_input_node_unique_id().is_none()); + + // Explicit nulls: the engine null-fill shape (the cute-dbt#145 + // `"overrides": null` precedent) must not fail the parse. + let json = format!( + r#"{{ + "metadata": {{ "dbt_schema_version": "{V12_URL}" }}, + "unit_tests": {{ + "unit_test.shop.t1": {{ + "name": "t1", + "model": "stg_orders", + "expect": {{ "rows": [] }}, + "tested_node_unique_id": null, + "this_input_node_unique_id": null + }} + }} + }}"# + ); + let manifest = parse_manifest(&json).expect("null-filled ids parse"); + let ut = manifest.unit_test("unit_test.shop.t1").expect("ingested"); + assert!(ut.tested_node_unique_id().is_none()); + assert!(ut.this_input_node_unique_id().is_none()); + } + // ----- parse_manifest: happy path + translation ----------------- #[test] diff --git a/src/adapters/render.rs b/src/adapters/render.rs index 889c4c0a..85b323ca 100644 --- a/src/adapters/render.rs +++ b/src/adapters/render.rs @@ -71,7 +71,7 @@ use crate::domain::{ HeuristicId, InScopeSet, Instrument, Manifest, ModelInScopeSet, ModelYamlOutcome, Node, NodeId, SourceNode, TestMetadata, Tier, UnitTest, UnitTestDataDiff, UnitTestGiven, UnitTestOverrides, UnitTestYamlBlock, apply_check_policy, model_findings, resolve_target_model, - table_from_manifest_rows, + resolve_tested_model, table_from_manifest_rows, }; /// Snake-case wire key for an [`EdgeType`] — the exact JSON-serde string @@ -2270,8 +2270,9 @@ fn is_leaf_binding_candidate(graph: &CteGraph, index: usize) -> bool { /// Build a map from in-scope model id to **all** unit tests targeting it /// in the current manifest (cute-dbt#91 widening). /// -/// Resolved via [`resolve_target_model`] (the bare `model:` name → full -/// node id mapping). Unlike the prior in-scope-only indexer, this +/// Resolved via [`resolve_tested_model`] (the engine-resolved +/// `tested_node_unique_id` when present, the bare `model:` name +/// otherwise — cute-dbt#254). Unlike the prior in-scope-only indexer, this /// enumerates every unit test whose resolved target is one of /// `models_in_scope`, regardless of whether the test is itself in scope — /// so a model that entered scope solely via a changed test also carries @@ -2289,7 +2290,7 @@ pub fn index_tests_for_models<'m>( ) -> HashMap> { let mut map: HashMap> = HashMap::new(); for (test_id, unit_test) in current.unit_tests() { - let Some(model) = resolve_target_model(current, unit_test.model()) else { + let Some(model) = resolve_tested_model(current, unit_test) else { continue; }; if models_in_scope.contains(model.id()) { From d6aeb8e1fc837f03dddcf27c0db12f1ab138350e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 11 Jun 2026 21:32:45 -0400 Subject: [PATCH 3/3] =?UTF-8?q?fix(domain):=20#254=20=E2=80=94=20flatten?= =?UTF-8?q?=20resolve=5Ftested=5Fmodel=20to=20an=20Option=20combinator=20c?= =?UTF-8?q?hain?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback (gemini-code-assist on PR #263): replace the if-let early-return with a single .or_else() chain. Behavior-identical — the dangling-id and non-model fallback paths are pinned by resolve_tested_model_falls_back_when_the_id_dangles and resolve_tested_model_falls_back_when_the_id_names_a_non_model. Co-Authored-By: Claude Fable 5 --- src/domain/state.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/domain/state.rs b/src/domain/state.rs index fa29b200..f07caff3 100644 --- a/src/domain/state.rs +++ b/src/domain/state.rs @@ -622,14 +622,11 @@ fn leaf_segment(id: &str) -> &str { /// for dbt-core and older manifests. #[must_use] pub fn resolve_tested_model<'m>(manifest: &'m Manifest, unit_test: &UnitTest) -> Option<&'m Node> { - if let Some(node) = unit_test + unit_test .tested_node_unique_id() .and_then(|id| manifest.node(id)) .filter(|node| node.resource_type() == "model") - { - return Some(node); - } - resolve_target_model(manifest, unit_test.model()) + .or_else(|| resolve_target_model(manifest, unit_test.model())) } /// Build a map from resolved model node id to the unit-test ids in