Repository navigation
#200 — design-2 data contracts: manifest_nodes lookup, TestPayload.overrides (Value passthrough), ModelPayload.description - #216
Conversation
…ob (#200) Node gains the authored model description (empty-string unset shapes dropped at the adapter, the #165 precedent) and the resolved top-level tags (the deduplicated wire list — never the duplicate-carrying config.tags), via a with_model_metadata builder. UnitTest retains the FULL overrides blob as the grouped UnitTestOverrides map (group -> name -> serde_json::Value): native scalars preserved end-to-end per the #197 founder decision — the BTreeMap<String, BTreeMap<String, String>> sketch was rejected as lossy. is_incremental_mode (#145) keeps its lifted field; with_overrides follows the same no-constructor-churn builder pattern. Domain stays std+serde only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ive-scalar overrides (#200) WireNode reads the node's top-level description (fusion omits unset — skip_serializing_none on ManifestMaterializableCommonAttributes, dbt-schemas manifest_nodes.rs @ 9977b6cbb1b761065536300037560d8e3c037011; dbt-core emits "" — dropped) and top-level tags (Option for null-fill tolerance; config.tags deliberately unread — the committed playground fixture shows it carrying project+model merge duplicates). WireUnitTestOverrides widens to all three fusion channels ({env_vars, macros, vars}: Option<BTreeMap<String, YmlValue>> per UnitTestOverrides in dbt-schemas unit_test_properties.rs @ 9977b6cb…) and folds them into the domain's grouped map, dropping null AND empty-map channels individually (both unset shapes verified on real committed fixtures: jaffle "overrides": null; playground "vars": {}, "env_vars": {} beside a populated macros). The #145 is_incremental lift is unchanged. Real-fixture coverage: jaffle (authored description verbatim, "" dropped, null overrides -> None) + playground (deduplicated top-level tags, the #125 splice's macros group with a native JSON bool). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…+ ModelPayload.description (#200) ReportPayload.manifest_nodes: BTreeMap<String, ManifestNodePayload> keyed by bare model name — the in-scope models + every model referenced by a rendered test's given.input ref(), never the whole project graph (source() inputs and unresolvable refs contribute nothing; absent entry = graceful no-hover). Each entry carries description / materialized (#145 re-plumb) / tags / declared columns (name, description, type from the ingested Node.columns data_type map, and the SHIPPED ColumnTestPayload §2.2 tests — reused, no parallel type) / model_tests (the NEW grouping: attached_node = model AND column_name = None, mapped through column_test_payload to {name, detail}). TestPayload.overrides serializes the grouped map with serde Value values (native bool/number/ string — the #197 founder decision), skip-when-None. ModelPayload.description rides skip-when-None. All-empty manifest_nodes entries are skipped and every new field is skip_serializing_if, so findings-free + manifest_nodes-free payloads stay byte-stable; emission stays on the #194 payload_json_for_html_script path. That escaper gains '=' in the tag-opener set: authored descriptions put SQL-ish prose like 'encounter_start_at <= current_timestamp' on the wire (the committed playground fixture), and tl-based extractors mis-scan a raw '<=' — same class as the #170 '<letter'/'<?' fix; round-trips to the original text through JSON.parse. Chrome snapshot: the real jaffle fixture now carries its manifest_nodes entry (materialized + the customer_id column tests; the "" description correctly absent). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
One scenario in report_generation.feature (no new .feature file — the feature-count mirror stays 14): a synthetic current/baseline pair built from the domain builders, serialized natively (description/tags are top-level node fields and the grouped overrides matches the wire shape, so serialize_to_tmp round-trips through the REAL manifest reader — no wire injection needed), run through the real subprocess. Asserts ModelPayload.description, the manifest_nodes entries for the in-scope model AND its NOT-in-scope ref()-ed upstream (proving the ref() arm), and the overrides groups carrying a native JSON number + bool (type probes would fail on "7"/"true" strings). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…yload order_metrics gains config.tags ['marts', 'dogfood'] — the live dbt-project dogfood previously carried zero model tags, so the new manifest_nodes tags row would have been invisible on the PR-preview report (the dogfood-pairing rule). Its #125 overrides block (vars + env_vars) and the model/column descriptions already light up the other new fields. Golden regen (all three, the ci.yml recipe). Byte audit: every changed byte is one of the new contract fields — manifest_nodes on all three (jaffle: stg_customers, materialized + column tests, "" description correctly absent; playground: 7 in-scope models + 4 ref()-ed upstreams; diff-showcase: 3 + 4), ModelPayload.description on every described in-scope model, overrides {macros: {is_incremental: true}} (native bool, empty channels dropped) on the playground incremental test, and three \u003c= escapes inside NEW description content. Zero pre-existing payload bytes changed (old payloads carried no raw <=). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The lefthook pre-push docs hook runs --document-private-items, where a domain doc link to the adapters' ModelPayload is unresolvable (and names an outward item the domain must not reference anyway). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR implements the Phase-2 manifest context feature by adding model descriptions, tags, and unit-test overrides as rich payloads. Domain models are extended with optional metadata fields; the manifest adapter translates dbt v12 data into grouped overrides while preserving native JSON types; the render layer builds manifest_nodes lookups with scoped model context for hover cards; and Cucumber BDD tests validate the full end-to-end data contracts. ChangesPhase 2 Manifest Context and Overrides
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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.43.0)examples/jaffle-shop-report.htmlThanks 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 6 individual chapters for you: Chapters generated by Stage for commit 4054129 on Jun 11, 2026 3:44am UTC. |
…howcase) + golden regen Conflict resolution by REGENERATION, never manual picking: all three examples re-rendered from the merged tree with the ci.yml recipe. Non-payload bytes are byte-identical to origin/main's goldens (the #213 template/CSS delta carries through untouched); the payload delta vs origin/main is exclusively the #200 contract fields — manifest_nodes, ModelPayload.description, the native-scalar overrides, and the three \u003c= escapes inside new description content. The #214 external-fixture given rides through unchanged (no existing payload key altered). Double-render byte-identity verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📄 Rendered report previewAll golden examples regenerated cleanly. 🟡 Golden examplesCommitted to
🐶 Live dogfood previewThis PR's own
▶ 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 27322326748 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.htmlPosted by |
- tests/steps/world.rs: kept BOTH accumulators — main's ContextPlan (#216) and the explore fields/structs (#100). - tests/steps/report_generation.rs: the NEW #216 context-report invocation (landed on main pre-verb) gains the report verb. - examples: report goldens taken from main and verified byte-identical over the merged tree under the report verb; explore goldens REGENERATED over the merged tree (stale twice: #214 changed the playground manifest, #216 added payload fields tests.html embeds) + double-render byte-identity + leak-grep clean. - ci.yml/lefthook.yml auto-merged: verb-aware gate + tripwire + feature-count 16 intact (no upstream bump — no renumber). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Summary
Implements the three genuinely-new Phase-2 data contracts (design-return-2 integration audit D12–D14), Rust + fixtures only — the consuming shelf/hover-card JS lands in #201/#202 and the report degrades gracefully until then.
ReportPayload.manifest_nodes—BTreeMap<String, ManifestNodePayload>keyed by bare model name:{description?, materialized?, tags[], columns[], model_tests[]}. Scope is the in-scope models plus every model referenced by a rendered test'sgiven.inputref()— never the whole project graph (source(...)inputs and unresolvable refs contribute nothing; an absent entry is the graceful no-hover). Columns reuse the SHIPPEDColumnTestPayload§2.2 display mapping (#165 — column-header tooltips: authored descriptions + column-level data tests #166/#179 — design integration PR-3: full model path + column-test display alignment #189 — no parallel type) and taketypefrom the already-ingestedNode.columnsdata_type map;model_testsis the new grouping (attached_node = model ∧ column_name = None→{name, detail?}). Mostly re-plumbing per the integration audit — materialized (feat: surface incremental-model unit-test semantics (incremental badge + expect=merged-rows tooltip) #145) and column type/descriptions (feature: column-header tooltips on unit-test tables — authored descriptions + column-level data tests #165) were already ingested.TestPayload.overrides— the FULL grouped blob withserde_json::Valuepassthrough values (the founder decision on Epic: design-system integration — Claude Design Phase-2 (8 themes, fold steppers, node shelf, hover cards, manifest_nodes/overrides contracts) #197/feature: design-2 integration PR-3 — data contracts: report manifest_nodes lookup + TestPayload overrides + ModelPayload description #200):macros.is_incremental: true,vars.encounter_lookback_days: 7,vars.dq_quarantine_threshold: 0.05stay native bool/number on the wire — the README'sBTreeMap<String, BTreeMap<String, String>>sketch is rejected as lossy stringification. The feat: surface incremental-model unit-test semantics (incremental badge + expect=merged-rows tooltip) #145 liftedis_incremental_modeand the diff: surface unit-testoverrides(env_vars/macros/vars) changes in the YAML text-diff fallback #125 YAML-slice diffs both stay; this is additive context.ModelPayload.description— the authored model description (new ingestion: top-leveldescription+tagson model nodes,with_model_metadatabuilder; empty-string unset shapes dropped, the feature: column-header tooltips on unit-test tables — authored descriptions + column-level data tests #165 precedent).Everything is
skip_serializing_if(all-emptymanifest_nodesentries are skipped too), so findings-free + manifest_nodes-free payloads stay byte-stable, and emission stays on the #194payload_json_for_html_scriptpath. That escaper gains=in its tag-opener set: the new description content puts SQL-ish prose likeencounter_start_at <= current_timestampon the wire andtl-based extractors mis-scan a raw<=— the same scanner-hostile class #170 fixed for<letter/<?; output still JSON-decodes to the original text.Fusion-first verification (pinned
9977b6cbb1b761065536300037560d8e3c037011)UnitTestOverrides(crates/dbt-schemas/src/schemas/properties/unit_test_properties.rs:32) is{env_vars, macros, vars}: Option<BTreeMap<String, YmlValue>>under#[skip_serializing_none]— three sibling open maps with arbitrary scalar values, unset = key absent.ManifestUnitTest.overrides: Option<UnitTestOverrides>(manifest_nodes.rs:268).env_varsIS modeled by fusion (the Discovery question — parity confirmed at the type level).ManifestMaterializableCommonAttributes.{description: Option<String>, tags: Vec<String>}(manifest_nodes.rs) — top-level on the serialized node. The""-emitting custom serializer (serialize_dbt_column_desc,dbt_column.rs) is column-specific; the model-level description is plain skip-none.tests/fixtures/playground-current.jsoncarries the diff: surface unit-testoverrides(env_vars/macros/vars) changes in the YAML text-diff fallback #125 overrides splice:{"env_vars": {}, "macros": {"is_incremental": true}, "vars": {}}— a native JSON bool plus empty-map unset channels, and five tests with explicit"overrides": null. The grouped ingestion drops null AND{}channels individually (asserted on the real fixture intests/manifest_ingestion.rs).mart_dq_summarytop-leveltags = ["analytics","data_quality","marts"]whileconfig.tags = ["analytics","data_quality","marts","analytics","data_quality"]— the nested list carries project+model merge duplicates, so cute-dbt reads only the authoritative deduplicated top-level list.jaffle-shop-current.json: authoredcustomersdescription ingests verbatim;stg_customers's""description is dropped.manifest_nodesentries with{description, materialized, tags, columns[{name, description, type, tests}], model_tests[{name, detail}]}, and three tests carrying native-scalar overrides (true,7,0.05,"ci").Tests
Node.description/tagsand the grouped overrides (native bool/int/float/string types pinned; deterministic BTreeMap group order; pre-feature: design-2 integration PR-3 — data contracts: report manifest_nodes lookup + TestPayload overrides + ModelPayload description #200 JSON without the new keys still deserializes).nullblob /{}blob / null channels / empty channels / per-channel drops / native scalar retention; model description""-drop + null/absent tags.manifest_nodesentry carries description/materialized/tags/columns/model_tests; ref()-ed upstream included + unrelated model excluded (+this/source()/unresolvable-ref arms); all-empty entries skipped and the key omitted (byte-stability); overrides group+scalar round-trip at the serialized-JSON level; description threads + omitted-when-None;<=escaper round-trip.manifest_nodesentry).report_generation.feature(no new .feature file — feature-count stays 14, no mirror bump) through the real subprocess: manifest_nodes for the in-scope model and its NOT-in-scope ref()-ed upstream, ModelPayload.description, and native-scalar overrides (type probes reject"7"/"true").Dogfood + goldens
dbt-project/models/marts/_marts__models.yml:order_metricsgainsconfig.tags ['marts','dogfood']— the live dogfood had zero model tags; its diff: surface unit-testoverrides(env_vars/macros/vars) changes in the YAML text-diff fallback #125 overrides block (vars+env_vars) and the existing descriptions light up the other new fields on the PR-preview report.tests/fixtures/playground-current.jsonsplice — the committed fixture already carries descriptions, tags, and the feat: surface incremental-model unit-test semantics (incremental badge + expect=merged-rows tooltip) #145/diff: surface unit-testoverrides(env_vars/macros/vars) changes in the YAML text-diff fallback #125 overrides, so the contended file is untouched.git diff --text -U0 -- examples/, decoded + compared key-by-key): every changed byte is a new contract field —manifest_nodeson all three (playground: 7 in-scope + 4 ref()-ed upstream entries),descriptionon described in-scope models,overrides: {"macros":{"is_incremental":true}}on the playground incremental test, and three<=escapes inside NEW description content. Zero pre-existing payload bytes changed (old payloads carried no raw<=).Gates (run directly — lefthook skips in fresh worktrees)
fmt ✓ · clippy --all-targets --locked -D warnings ✓ (exit 0) · nextest 1111 ✓ · BDD 114 scenarios ✓ · headless ignored pair (zero-egress 2 + toggle 38) ✓ · cargo doc -D warnings (incl. --document-private-items) ✓ · cargo deny ✓ · crap4rs PASS (0 above threshold 25, worst 23.0)
Closes #200
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Tests