From d5de49b0b62bb7fe2cfc78430de9040f799db32a Mon Sep 17 00:00:00 2001 From: Christopher Bays Date: Sat, 13 Jun 2026 21:21:53 -0400 Subject: [PATCH 1/4] =?UTF-8?q?feat(cli):=20findings=20envelope=20core=20?= =?UTF-8?q?=E2=80=94=20--findings-out=20sidecar=20+=20--fail-on-uncovered?= =?UTF-8?q?=20gate=20+=20row-5-reversal=20ADR?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap the already-Serialize-deriving Finding POD in a versioned metadata.schema_version header (epic #261, cute-dbt#386) and emit it as a machine-readable SIDECAR beside the HTML report — the sanctioned reversal of ARCHITECTURE conscious-simplification row-5 ("no JSON wire envelope"), authorized by the founder's ADR-4 amendment (2026-06-11) and the four #261 locked decisions. - domain (pure POD + gate): src/domain/findings_envelope.rs — FindingsEnvelope { metadata, findings: Vec }; EnvelopeMetadata pins schema_version=1 (integer) + id_stability="unstable-v0.x" from constants; EnvelopeScope tags baseline | pr-diff; has_total_uncovered is the D3 gate predicate (Total-tier Uncovered only, not configurable). - adapter (collect + emit): src/adapters/findings_emit.rs — mirrors the renderer's per-model model_findings -> apply_check_policy pipeline EXACTLY (parses each in-scope model's CTE graph), so the envelope's findings match the report's; serializes pretty JSON + writes the sidecar. Never touches report.html or render.rs. - cli: --findings-out (additive sidecar, NOT --format json) + --fail-on-uncovered (dedicated EXIT_GATE=3, distinct from usage/2 and fail-closed/1; the report is written BEFORE the gate trips) + a hidden --generated-at golden-regeneration override. generated_at is computed at the CLI I/O boundary (today_rfc3339_date, reusing the #260/#351 civil_from_days machinery — std-only, NO chrono/time) and emitted as an RFC3339 date (YYYY-MM-DD; a finer timestamp would need a date crate the std-only posture forbids). - envelope golden: examples/diff-showcase-findings.json (synthetic playground pr-diff combo, pinned --generated-at 2099-01-01), byte-identity-gated in the example-report-check diff-showcase row. - ARCHITECTURE row-5 amended to "reversed — now present"; AGENTS.md conscious-simplification enumeration updated. The HTML byte-identity goldens are UNCHANGED (git status examples/ shows only the new JSON). OpenLineage: fields shaped design-compatible, NO OL output (D4 deferred). Owner fields (#256): slot reserved, not populated. SARIF: not now. Closes #386 Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci.yml | 42 +- AGENTS.md | 18 +- ARCHITECTURE.md | 68 ++- examples/diff-showcase-findings.json | 678 +++++++++++++++++++++++++++ src/adapters/findings_emit.rs | 266 +++++++++++ src/adapters/mod.rs | 7 + src/cli/args.rs | 41 ++ src/cli/mod.rs | 198 +++++++- src/cli/review.rs | 10 + src/domain/findings_envelope.rs | 343 ++++++++++++++ src/domain/mod.rs | 5 + tests/run_loop.rs | 134 ++++++ 12 files changed, 1775 insertions(+), 35 deletions(-) create mode 100644 examples/diff-showcase-findings.json create mode 100644 src/adapters/findings_emit.rs create mode 100644 src/domain/findings_envelope.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d81b00e0..d6b48e31 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -650,6 +650,14 @@ jobs: # clause (fictional acme/shop url; never a real PR — the # synthetic-only fixture invariant). config: tests/fixtures/config-pr-showcase.toml + # cute-dbt#386 — the showcase also emits the machine-readable + # findings envelope sidecar and byte-identity-gates it. The + # `--generated-at` override pins a far-future date so the golden + # is wall-clock-independent (golden-determinism rule). The + # envelope wraps the SAME in-scope findings the HTML report + # surfaces, so this row proves the sidecar shape stays stable. + findings_out: examples/diff-showcase-findings.json + generated_at: "2099-01-01" # cute-dbt#265 Slice D — the GOLDEN macro-lens cap showcase. A # synthetic heavy macro (`mask_pii`) reaches 14 root-project # models (> the default inline-body cap of 10), so the committed @@ -720,6 +728,11 @@ jobs: # cute-dbt#346: optional --config (the diff-showcase row's # synthetic [pr] section); empty = no --config arg. CONFIG: ${{ matrix.config }} + # cute-dbt#386: optional findings-envelope sidecar golden (the + # diff-showcase row); empty = no --findings-out arg. GENERATED_AT + # pins a far-future date so the golden is wall-clock-independent. + FINDINGS_OUT: ${{ matrix.findings_out }} + GENERATED_AT: ${{ matrix.generated_at }} run: | set -euo pipefail mkdir -p target/example-check @@ -806,19 +819,46 @@ jobs: config_args=(--config "$CONFIG") config_hint=" --config $CONFIG" fi + # cute-dbt#386: the optional findings-envelope sidecar (the + # diff-showcase row). Rendered into a scratch path and + # byte-diffed against the committed golden, the same gate shape + # as the HTML. --generated-at pins the envelope timestamp so the + # golden is wall-clock-independent. + findings_args=() + findings_hint="" + rendered_findings="" + if [ -n "$FINDINGS_OUT" ]; then + rendered_findings="target/example-check/$(basename "$FINDINGS_OUT")" + findings_args=(--findings-out "$rendered_findings" --generated-at "$GENERATED_AT") + findings_hint=" --findings-out $FINDINGS_OUT --generated-at $GENERATED_AT" + fi cargo run --quiet --locked --bin cute-dbt -- report \ --manifest "$CURRENT_MANIFEST" \ "${scope_args[@]}" \ "${project_root_args[@]}" \ "${config_args[@]}" \ + "${findings_args[@]}" \ --out "$rendered" if ! diff -q "$rendered" "$OUTPUT_PATH" >/dev/null; then echo "::error::example-report-up-to-date: $OUTPUT_PATH does not match the renderer output ($EXAMPLE_NAME). Regenerate with:" - echo " ${regen_env}cargo run --bin cute-dbt -- report --manifest $CURRENT_MANIFEST$scope_hint$regen_hint$config_hint --out $OUTPUT_PATH" + echo " ${regen_env}cargo run --bin cute-dbt -- report --manifest $CURRENT_MANIFEST$scope_hint$regen_hint$config_hint$findings_hint --out $OUTPUT_PATH" echo "Sizes:" ls -l "$rendered" "$OUTPUT_PATH" exit 1 fi + # cute-dbt#386: byte-identity-gate the findings-envelope golden + # exactly like the HTML (same row, additive). The envelope is + # deterministic over (fixtures, pinned --generated-at), so + # byte-equality is the correct gate. + if [ -n "$FINDINGS_OUT" ]; then + if ! diff -q "$rendered_findings" "$FINDINGS_OUT" >/dev/null; then + echo "::error::example-report-up-to-date: $FINDINGS_OUT does not match the renderer output ($EXAMPLE_NAME findings envelope). Regenerate with:" + echo " ${regen_env}cargo run --bin cute-dbt -- report --manifest $CURRENT_MANIFEST$scope_hint$regen_hint$config_hint$findings_hint --out $OUTPUT_PATH" + echo "Sizes:" + ls -l "$rendered_findings" "$FINDINGS_OUT" + exit 1 + fi + fi example-report-up-to-date: # Stable aggregator. Presents a single check name to branch diff --git a/AGENTS.md b/AGENTS.md index a3d79f9b..4084e014 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,7 +72,12 @@ Enforced by module convention + clippy + review (a single crate cannot fail to compile on an inward `use`). The full layering invariant, the two-stage fail-closed contract, the StateComparator strategy, and the conscious design simplifications (no workspace, no per-crate versioning, no API shim, -no AST-purity grep, no JSON envelope) are recorded in [`ARCHITECTURE.md`](ARCHITECTURE.md). +no AST-purity grep) are recorded in [`ARCHITECTURE.md`](ARCHITECTURE.md). +The sixth original simplification — **no JSON wire envelope** — was +**consciously reversed** at cute-dbt#386 / epic #261 (founder ADR-4 +amendment, 2026-06-11): a machine-readable findings-envelope **sidecar** +now lands via `--findings-out` (additive — the HTML output is unchanged). +See [`ARCHITECTURE.md`](ARCHITECTURE.md) §2 (row 5 + the row-5-reversal note). ``` domain -> (no outward imports; std + serde derive only) @@ -86,10 +91,13 @@ cli/ -> clap derive, ExitCode mapping, run loop composition main.rs -> thin entry ``` -**Never import inward.** The five "conscious design simplifications" (no -workspace, no per-crate versioning, no API shim, no AST-purity grep, no -JSON envelope) are documented absences, not omissions. Adding any of them -is a regression, not a "pattern completion." +**Never import inward.** The "conscious design simplifications" (no +workspace, no per-crate versioning, no API shim, no AST-purity grep) are +documented absences, not omissions. Adding any of them is a regression, not +a "pattern completion." The one-time exception — the **JSON wire envelope** +— was sanctioned-reversed at cute-dbt#386 (findings-envelope sidecar); that +reversal is authorized by the founder's ADR-4 amendment + the #261 locked +decisions, not a contributor "completing the pattern." The `non-mirror-guard` CI job rejects: - a `[workspace]` table in `Cargo.toml` - `bans.deny.wrappers` in `deny.toml` diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 70922b04..b9e551ee 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -89,7 +89,7 @@ machinery that guards no invariant here. | 2 | Per-crate independent versioning | Single artifact version | Moot — one crate, one version. The release cadence is whole-product, not per-component. | Absence (no second crate to version independently). | | 3 | `public-api-shim` re-export pattern (`pub use crate::…::…` from `lib.rs` curating a stable surface for library consumers) | None | The binary is the product; there are no library consumers to shield from internal renames. An API shim with no consumer would add indirection that guards nothing. | **CI:** `non-mirror-guard` job rejects `pub use crate::…::…` in `src/lib.rs`. | | 4 | AST-purity `cargo-deny` bans + `ast-purity` CI grep (keep adapter AST libraries out of a shared core) | None | The AST-purity invariant exists to protect a *shared* core crate from adapter parser dependencies when several adapter crates each pull in different AST libraries. cute-dbt has exactly one parser (`sqlparser-rs`) and one consumer of it; there is no shared core and no rival AST surface — no invariant to enforce. (Bonus: `bans.deny.wrappers` is fragile under proc-macro dependency chains; this project never needs to depend on that mechanism.) | **CI:** `non-mirror-guard` job rejects `bans.deny.wrappers` in `deny.toml`. | -| 5 | `nested-json-envelope` ADR for wire output | None | Output is HTML-primary, single self-contained file. There is no JSON wire envelope to version. `--format json` is explicitly deferred to v0.2+; if it lands it will be a new ADR, not a retroactive shim. | Absence (no JSON output in v0.1). | +| 5 | `nested-json-envelope` ADR for wire output | **Reversed — now present** (the findings-envelope **sidecar**, cute-dbt#386 / epic #261) | Originally absent: output was HTML-primary, single self-contained file, with no JSON wire envelope to version. The conscious simplification has been **deliberately reversed** under the founder's ADR-4 amendment (2026-06-11) + the #261 locked decisions — see the note below the table. The reversal stays minimal: the envelope is an **additive `--findings-out ` sidecar** (mirroring dbt's manifest/run_results sidecars), **never a `--format json` swap**, so the HTML output and its byte-identity goldens are untouched. | New-output-path ADR (#261), not a retroactive shim. The envelope is byte-identity-gated like the HTML goldens (`examples/diff-showcase-findings.json` in `example-report-check`). | | 6 | `proc-macro2 span-locations` toolchain gate | None | No direct `proc-macro2` dependency. The `sqlparser` tokenizer carries its own spans, and the tokenizer pass itself (for `-- @desc` per-CTE breakdowns) is v0.2-deferred. There is no proc-macro span-precision invariant in scope to gate. | Absence (no `proc-macro2` direct dep). | **Enforcement layering.** Three of the six rows have a literal CI grep @@ -97,14 +97,72 @@ backing them — rows 1, 3, and 4 are guarded by the [`non-mirror-guard`](.github/workflows/ci.yml) job, which rejects the specific tripwires (`[workspace]`, `pub use crate::…::…`, `bans.deny.wrappers`) that would silently reintroduce the apparatus. Rows -2, 5, and 6 are enforced by **absence** — there is no second crate to -version, no JSON output path, and no `proc-macro2` direct dependency, so -the apparatus cannot be added incidentally; adding any of them would -require a discrete code change visible at review. +2 and 6 are enforced by **absence** — there is no second crate to +version and no `proc-macro2` direct dependency, so the apparatus cannot be +added incidentally; adding either would require a discrete code change +visible at review. Row 5 was absence-enforced until cute-dbt#386 reversed +it; the envelope's own shape is now pinned by a **byte-identity golden** +(`examples/diff-showcase-findings.json`), the same gate class as the HTML +examples. This is deliberate. The strongest tripwires get CI; the absence-enforced ones get this section. +### Row-5 reversal — the findings-envelope sidecar (cute-dbt#386, epic #261) + +Row 5's "no JSON wire envelope" simplification was **consciously +reversed** — a sanctioned reversal, not a "pattern completion." The +authority is the founder's **ADR-4 amendment (2026-06-11)** plus the four +**decisions locked** in the [#261 Wave-1 shaping +session](https://github.com/breezy-bays-labs/cute-dbt/issues/261). The +canonical decision record is the ops-repo ADR +(`decisions/cute-dbt/`); this section is the public-repo narrative. + +The reframe that made the reversal cheap: the `Finding` POD already derives +`Serialize` and already emits the full vocabulary (`check` / `tier` / +`verdict` / `evidence` / `recommendation` / `degraded` / `suppressed`), so +this is not "design a findings schema" — it is "wrap the already-serializing +PODs in a versioned header, emit them as a sidecar, and make four policy +commitments." The four locked decisions: + +1. **Delivery = sidecar.** A new `--findings-out ` flag emits the + envelope **alongside** the HTML report in one run — additive, **not** a + `--format json` swap. The HTML output is byte-for-byte unchanged (the + HTML goldens stay green); CI gets the human report *and* the machine + envelope from one invocation. +2. **check-id stability: v0.x churns, freeze at v1.0.** Individual + check-ids are **unstable** in v0.x and only freeze at v1.0. The envelope + carries this loudly and machine-readably: `metadata.id_stability: + "unstable-v0.x"`. **Consumers pin `metadata.schema_version` (an integer, + starting at `1`) — the only stability anchor pre-v1.0 — never an + individual check-id, and never a gate config hard-keyed to specific ids, + until v1.0.** +3. **Gate = Total-tier `Uncovered` only, not configurable.** + `--fail-on-uncovered` exits a dedicated non-zero code (distinct from the + usage-error and fail-closed codes) iff ≥1 `Tier::Total` + + `Verdict::Uncovered` finding is in the in-scope set. `Total` checks are + zero-false-positive by construction, so the gate never trips on a + heuristic guess. No tier knob — Total-only is the design tenet. +4. **OpenLineage: design-compatible, deferred.** The envelope fields are + shaped so a future OpenLineage-facet projection is a clean mapping, but + **no** OpenLineage output is emitted in this slice. + +Severity is the existing `Tier` enum (no new field). SARIF is a later, +lossy projection — never canonical, not here. Owner fields (soft-dep #256) +reserve their slot and are populated later, additively. + +The envelope POD + the gate predicate live in `src/domain/findings_envelope.rs` +(pure: `std` + serde derive only); the findings-collection + emit-to-file +live in the `src/adapters/findings_emit.rs` adapter (it parses each in-scope +model's CTE graph and runs the **same** `model_findings → apply_check_policy` +pipeline the renderer runs, so the envelope's findings match the report's +exactly). `generated_at` is computed at the CLI I/O boundary and threaded in +as a parameter (the golden-determinism rule — std-only, no `chrono`/`time`), +so the committed envelope golden is byte-stable. The envelope is emitted as +an **RFC3339 date** (`YYYY-MM-DD`): a finer-grained timestamp would need a +date-time formatting crate cute-dbt's std-only posture forbids, and the date +is the deterministic granularity the golden gate needs. + ## 3. Two-stage fail-closed contract Fail-closed inputs (a `dbt parse`-only manifest, a pre-1.8 manifest, an diff --git a/examples/diff-showcase-findings.json b/examples/diff-showcase-findings.json new file mode 100644 index 00000000..b60c68c8 --- /dev/null +++ b/examples/diff-showcase-findings.json @@ -0,0 +1,678 @@ +{ + "metadata": { + "schema_version": 1, + "cute_dbt_version": "0.1.0", + "generated_at": "2099-01-01", + "scope": { + "mode": "pr-diff" + }, + "id_stability": "unstable-v0.x" + }, + "findings": [ + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.dim_conditions", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_dim_conditions_condition_key.93802fac5a" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "condition_key" + } + ] + }, + { + "check": "union.arm-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.dim_conditions", + "construct": "union[final]", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "union", + "value": "final (2 arms: sequenced, unknown_member)" + }, + { + "label": "unexercised arm", + "value": "sequenced — no given row reaches ref('stg_synthea__conditions')" + }, + { + "label": "suggested given", + "value": "- input: ref('stg_synthea__conditions')\n rows:\n - {_loaded_at: ..., condition_code: ..., condition_description: ..., condition_end_date: ..., condition_id: ..., condition_start_date: ..., encounter_id: ..., patient_id: ...}" + }, + { + "label": "unattributable arm", + "value": "unknown_member — no resolvable upstream relation" + } + ], + "recommendation": "Add (or fill) a given row for each unexercised UNION arm's input so every arm contributes at least one row, then extend `expect` with the row(s) that arm should emit. This finding's evidence carries the per-arm input and a given-row sketch." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.dim_date", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_dim_date_date_key.0869fd48f9" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "date_key" + } + ] + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.dim_organizations", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_dim_organizations_organization_key.096582bf56" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "organization_key" + } + ] + }, + { + "check": "union.arm-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.dim_organizations", + "construct": "union[final]", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "union", + "value": "final (2 arms: sequenced, unknown_member)" + }, + { + "label": "unexercised arm", + "value": "sequenced — no given row reaches ref('stg_synthea__organizations')" + }, + { + "label": "suggested given", + "value": "- input: ref('stg_synthea__organizations')\n rows:\n - {_loaded_at: ..., address: ..., city: ..., latitude: ..., longitude: ..., organization_id: ..., organization_name: ..., phone: ..., revenue: ..., state: ..., utilization: ..., zip_code: ...}" + }, + { + "label": "unattributable arm", + "value": "unknown_member — no resolvable upstream relation" + } + ], + "recommendation": "Add (or fill) a given row for each unexercised UNION arm's input so every arm contributes at least one row, then extend `expect` with the row(s) that arm should emit. This finding's evidence carries the per-arm input and a given-row sketch." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.dim_patients", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_dim_patients_patient_key.a372c8896a" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "patient_key" + } + ] + }, + { + "check": "union.arm-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.dim_patients", + "construct": "union[final]", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "union", + "value": "final (2 arms: sequenced, unknown_member)" + }, + { + "label": "unexercised arm", + "value": "sequenced — no given row reaches ref('stg_synthea__patients')" + }, + { + "label": "suggested given", + "value": "- input: ref('stg_synthea__patients')\n rows:\n - {_loaded_at: ..., address: ..., birth_date: ..., birth_place: ..., city: ..., county: ..., death_date: ..., drivers_license: ..., ethnicity: ..., first_name: ..., gender: ..., healthcare_coverage: ..., healthcare_expenses: ..., last_name: ..., latitude: ..., longitude: ..., maiden_name: ..., marital_status: ..., name_prefix: ..., name_suffix: ..., passport_number: ..., patient_id: ..., race: ..., ssn_hash: ..., state: ..., zip_code: ...}" + }, + { + "label": "unattributable arm", + "value": "unknown_member — no resolvable upstream relation" + } + ], + "recommendation": "Add (or fill) a given row for each unexercised UNION arm's input so every arm contributes at least one row, then extend `expect` with the row(s) that arm should emit. This finding's evidence carries the per-arm input and a given-row sketch." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.dim_payers", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_dim_payers_payer_key.953b1f5fd2" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "payer_key" + } + ], + "degraded": [ + { + "by": "test.healthcare_analytics.unique_dim_payers_payer_key.953b1f5fd2", + "causes": [ + "severity: warn — a failing run warns instead of failing, so this backing is advisory, not gating" + ] + } + ] + }, + { + "check": "union.arm-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.dim_payers", + "construct": "union[final]", + "verdict": { + "status": "unknown" + }, + "evidence": [ + { + "label": "union", + "value": "final (2 arms: sequenced, unknown_member)" + }, + { + "label": "unattributable arm", + "value": "unknown_member — no resolvable upstream relation" + } + ], + "suppressed": { + "source": "pragma", + "reason": "unknown_member is a constant sentinel arm, exercised by design" + } + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.dim_providers", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_dim_providers_provider_key.02ac65c919" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "provider_key" + } + ] + }, + { + "check": "union.arm-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.dim_providers", + "construct": "union[final]", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "union", + "value": "final (2 arms: sequenced, unknown_member)" + }, + { + "label": "unexercised arm", + "value": "sequenced — no given row reaches ref('dim_organizations'), ref('stg_synthea__providers')" + }, + { + "label": "suggested given", + "value": "- input: ref('dim_organizations')\n rows:\n - {_loaded_at: ..., address: ..., city: ..., latitude: ..., longitude: ..., organization_id: ..., organization_key: ..., organization_name: ..., phone: ..., revenue: ..., state: ..., utilization: ..., zip_code: ...}\n- input: ref('stg_synthea__providers')\n rows:\n - {_loaded_at: ..., address: ..., city: ..., gender: ..., latitude: ..., longitude: ..., organization_id: ..., provider_id: ..., provider_name: ..., specialty: ..., state: ..., utilization: ..., zip_code: ...}" + }, + { + "label": "unattributable arm", + "value": "unknown_member — no resolvable upstream relation" + } + ], + "recommendation": "Add (or fill) a given row for each unexercised UNION arm's input so every arm contributes at least one row, then extend `expect` with the row(s) that arm should emit. This finding's evidence carries the per-arm input and a given-row sketch." + }, + { + "check": "join.left-null-propagation", + "tier": "high", + "instrument": "both", + "model_id": "model.healthcare_analytics.dim_providers", + "construct": "left_join[sequenced:organizations]", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "left join", + "value": "sequenced — LEFT JOIN organizations ON providers.organization_id = organizations.organization_id" + }, + { + "label": "suggested given", + "value": "- input: ref('stg_synthea__providers')\n rows:\n - {organization_id: 404} # 404 has no match below — the no-match path\n- input: ref('dim_organizations')\n rows:\n - {organization_id: 1}\n# expect: the no-match row with NULL dim_organizations columns (or the intended fallback)" + } + ], + "recommendation": "Add a no-match given: one left-side row whose join key is absent from the right-side given rows, then extend `expect` with that row carrying NULL right-side columns (or the intended fallback). This finding's evidence carries a copy-pasteable given sketch." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_clinical_events", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_clinical_events_clinical_event_key.2af827606b" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "clinical_event_key" + } + ] + }, + { + "check": "union.arm-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.fct_clinical_events", + "construct": "union[all_events]", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "union", + "value": "all_events (3 arms: conditions, medications, procedures)" + }, + { + "label": "unexercised arm", + "value": "conditions — no given row reaches ref('stg_synthea__conditions')" + }, + { + "label": "suggested given", + "value": "- input: ref('stg_synthea__conditions')\n rows:\n - {_loaded_at: ..., condition_code: ..., condition_description: ..., condition_end_date: ..., condition_id: ..., condition_start_date: ..., encounter_id: ..., patient_id: ...}" + }, + { + "label": "unexercised arm", + "value": "medications — no given row reaches ref('stg_synthea__medications')" + }, + { + "label": "suggested given", + "value": "- input: ref('stg_synthea__medications')\n rows:\n - {_loaded_at: ..., base_cost: ..., dispenses: ..., encounter_id: ..., medication_code: ..., medication_description: ..., medication_end_at: ..., medication_id: ..., medication_start_at: ..., patient_id: ..., payer_coverage: ..., payer_id: ..., reason_code: ..., reason_description: ..., total_cost: ...}" + }, + { + "label": "unexercised arm", + "value": "procedures — no given row reaches ref('stg_synthea__procedures')" + }, + { + "label": "suggested given", + "value": "- input: ref('stg_synthea__procedures')\n rows:\n - {_loaded_at: ..., base_cost: ..., encounter_id: ..., patient_id: ..., procedure_at: ..., procedure_code: ..., procedure_description: ..., procedure_id: ..., reason_code: ..., reason_description: ...}" + } + ], + "recommendation": "Add (or fill) a given row for each unexercised UNION arm's input so every arm contributes at least one row, then extend `expect` with the row(s) that arm should emit. This finding's evidence carries the per-arm input and a given-row sketch." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_condition_cohorts", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_condition_cohorts_condition_cohort_key.f8c8f721d9" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "condition_cohort_key" + } + ] + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_cost_analysis", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_cost_analysis_cost_analysis_key.2aecf62159" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "cost_analysis_key" + } + ] + }, + { + "check": "join.left-null-propagation", + "tier": "high", + "instrument": "both", + "model_id": "model.healthcare_analytics.fct_cost_analysis", + "construct": "left_join[encounters_enriched:dim_date]", + "verdict": { + "status": "unknown" + }, + "evidence": [ + { + "label": "left join", + "value": "encounters_enriched — LEFT JOIN dim_date ON encounters.encounter_start_date_key = dim_date.date_key" + }, + { + "label": "unattributable join", + "value": "encounters_enriched — join key or side binding not statically recoverable" + } + ] + }, + { + "check": "join.left-null-propagation", + "tier": "high", + "instrument": "both", + "model_id": "model.healthcare_analytics.fct_cost_analysis", + "construct": "left_join[encounters_enriched:procedures]", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "left join", + "value": "encounters_enriched — LEFT JOIN procedures ON encounters.encounter_id = procedures.encounter_id" + }, + { + "label": "suggested given", + "value": "- input: ref('fct_encounters')\n rows:\n - {encounter_id: 404} # 404 has no match below — the no-match path\n- input: ref('stg_synthea__procedures')\n rows:\n - {encounter_id: 1}\n# expect: the no-match row with NULL stg_synthea__procedures columns (or the intended fallback)" + } + ], + "recommendation": "Add a no-match given: one left-side row whose join key is absent from the right-side given rows, then extend `expect` with that row carrying NULL right-side columns (or the intended fallback). This finding's evidence carries a copy-pasteable given sketch." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_encounters", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_encounters_encounter_key.a3b77e9141" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "encounter_key" + } + ] + }, + { + "check": "enforcement.constraint-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_encounters", + "construct": "constraint.primary_key[encounter_key]", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_encounters_encounter_key.a3b77e9141" + ] + }, + "evidence": [ + { + "label": "constraint", + "value": "primary_key on encounter_key" + }, + { + "label": "adapter", + "value": "duckdb (metadata-only)" + }, + { + "label": "backing-test", + "value": "present" + } + ] + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_encounters_incremental", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_encounters_incremental_encounter_id.a165c01d01" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "encounter_id" + } + ] + }, + { + "check": "incremental.branch-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.fct_encounters_incremental", + "construct": "config.materialized", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "materialized", + "value": "incremental (strategy: merge)" + }, + { + "label": "branch coverage", + "value": "true-only — 1 test(s) override is_incremental to true; none exercise the initial full-build (false) branch" + }, + { + "label": "missing branch", + "value": "the initial full-build branch (is_incremental() = false) runs in no unit test" + }, + { + "label": "suggested given", + "value": "- name: test_fct_encounters_incremental_initial_build\n model: fct_encounters_incremental\n # no overrides block — dbt compiles is_incremental() as false by default\n given:\n - input: ref('...')\n rows:\n - {...}\n # expect: the full first-build output" + } + ], + "recommendation": "Add a unit test for each missing is_incremental() branch: one with `overrides: macros: is_incremental: true` (mock the prior model state with a `given: - input: this` entry) to exercise the incremental branch, and one without the override for the initial full build (dbt compiles is_incremental() as false by default). This finding's evidence carries a copy-pasteable unit-test sketch per missing branch." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_encounters_monthly", + "construct": "config.unique_key", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "unique_key", + "value": "year_month, encounter_class" + } + ], + "recommendation": "Add a uniqueness data test at the declared grain: `unique` on a single-column key, or `dbt_utils.unique_combination_of_columns` over the composite key columns." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_encounters_yearly", + "construct": "config.unique_key", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "unique_key", + "value": "year_actual, encounter_class" + } + ], + "recommendation": "Add a uniqueness data test at the declared grain: `unique` on a single-column key, or `dbt_utils.unique_combination_of_columns` over the composite key columns." + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_patient_summary", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_patient_summary_patient_summary_key.e532e1ff45" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "patient_summary_key" + } + ] + }, + { + "check": "enforcement.constraint-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_patient_summary", + "construct": "constraint.primary_key[(patient_id, year_actual)]", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.dbt_utils_unique_combination_of_columns_fct_patient_summary_patient_id__year_actual.cdb3418166" + ] + }, + "evidence": [ + { + "label": "constraint", + "value": "primary_key on (patient_id, year_actual)" + }, + { + "label": "adapter", + "value": "duckdb (metadata-only)" + }, + { + "label": "backing-test", + "value": "present" + } + ] + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.fct_provider_metrics", + "construct": "config.unique_key", + "verdict": { + "status": "covered", + "by": [ + "test.healthcare_analytics.unique_fct_provider_metrics_provider_metrics_key.d58939d602" + ] + }, + "evidence": [ + { + "label": "unique_key", + "value": "provider_metrics_key" + } + ], + "degraded": [ + { + "by": "test.healthcare_analytics.unique_fct_provider_metrics_provider_metrics_key.d58939d602", + "causes": [ + "where-filtered — only rows matching `year_actual >= 2024` are checked; the grain is unverified outside the filter", + "limit: 100 — failing-row reporting caps at 100 rows; the test still trips on the first failure, but the failure surface is truncated" + ] + } + ] + }, + { + "check": "grain.unique-key-unbacked", + "tier": "total", + "instrument": "data-test", + "model_id": "model.healthcare_analytics.mart_dq_summary", + "construct": "config.unique_key", + "verdict": { + "status": "uncovered" + }, + "evidence": [ + { + "label": "unique_key", + "value": "entity_type" + }, + { + "label": "exists but disabled", + "value": "test.healthcare_analytics.unique_mart_dq_summary_entity_type.4f2a9c7d10 — a uniqueness test on (entity_type) is declared but disabled (config.enabled: false); it asserts nothing and never counts as coverage" + } + ], + "recommendation": "Add a uniqueness data test at the declared grain: `unique` on a single-column key, or `dbt_utils.unique_combination_of_columns` over the composite key columns." + }, + { + "check": "union.arm-coverage", + "tier": "high", + "instrument": "unit-test", + "model_id": "model.healthcare_analytics.mart_dq_summary", + "construct": "union[combined_metrics]", + "verdict": { + "status": "covered", + "by": [ + "unit_test.healthcare_analytics.mart_dq_summary.test_mart_dq_summary_combines_encounter_and_medication_metrics", + "unit_test.healthcare_analytics.mart_dq_summary.test_mart_dq_summary_zero_quarantined_when_all_valid" + ] + }, + "evidence": [ + { + "label": "union", + "value": "combined_metrics (2 arms: encounter_metrics, medication_metrics)" + } + ] + } + ] +} diff --git a/src/adapters/findings_emit.rs b/src/adapters/findings_emit.rs new file mode 100644 index 00000000..0de9160b --- /dev/null +++ b/src/adapters/findings_emit.rs @@ -0,0 +1,266 @@ +//! Findings-envelope emit adapter (cute-dbt#386, epic #261). +//! +//! Collects the in-scope [`Finding`]s — mirroring the report's per-model +//! check pipeline exactly — wraps them in a versioned +//! [`FindingsEnvelope`], serializes the envelope to pretty JSON, and writes +//! it as a **sidecar** beside the HTML report (`--findings-out `). +//! +//! This adapter is the *only* findings-collection caller outside the HTML +//! render path: it parses each in-scope model's CTE graph and runs +//! `model_findings → apply_check_policy` exactly as +//! [`crate::adapters::render`] does during payload assembly, so the +//! envelope's findings are byte-for-byte the same vocabulary the report +//! displays — without touching `report.html` or the render path +//! (`ARCHITECTURE.md` §1: this collection is an adapter concern; the POD + +//! gate predicate live in [`crate::domain::findings_envelope`]). +//! +//! `generated_at` is supplied by the caller (the CLI I/O boundary), keeping +//! this adapter a pure function of `(manifest, scope, policy, version, +//! generated_at)` so the committed envelope golden is byte-stable. + +use std::fs; +use std::io; +use std::path::Path; + +use crate::adapters::cte_engine::parse_cte_graph; +use crate::domain::{ + CheckPolicy, EnvelopeMetadata, EnvelopeScope, Finding, FindingsEnvelope, HeuristicId, Manifest, + ModelInScopeSet, apply_check_policy, model_findings, +}; + +/// Collect the flat in-scope finding set the envelope reports over. +/// +/// Mirrors [`crate::adapters::render`]'s per-model pipeline exactly: for +/// every model in `models_in_scope`, parse its `compiled_code` once into a +/// CTE graph and run `apply_check_policy(model_findings(.., Some(&graph)), +/// policy)`. The same parse-and-evaluate the renderer performs during +/// payload assembly — so the envelope and the HTML report carry the +/// identical findings (the same suppression/selection policy applied). +/// +/// Models are visited in `models_in_scope` iteration order; within a model +/// the order is `model_findings`' deterministic output. An id that resolves +/// to no manifest node is skipped (the renderer's `continue` arm). +#[must_use] +pub fn collect_in_scope_findings( + manifest: &Manifest, + models_in_scope: &ModelInScopeSet, + check_policy: &CheckPolicy, +) -> Vec> { + let mut findings = Vec::new(); + for model_id in models_in_scope.iter() { + let Some(model) = manifest.node(model_id) else { + continue; + }; + // The renderer's exact graph build: parse the compiled SQL once, + // tolerating an empty/uncompiled body as an empty graph. + let graph = parse_cte_graph(model.compiled_code().unwrap_or_default()).unwrap_or_default(); + findings.extend(apply_check_policy( + model_findings(manifest, model, Some(&graph)), + check_policy, + )); + } + findings +} + +/// Assemble the full [`FindingsEnvelope`] for the in-scope set. +/// +/// `cute_dbt_version` is typically `env!("CARGO_PKG_VERSION")`; +/// `generated_at` the RFC3339 timestamp computed at the CLI I/O boundary; +/// `scope` the arm-specific [`EnvelopeScope`] built from the run's scope +/// source. The findings are collected via [`collect_in_scope_findings`]. +#[must_use] +pub fn build_findings_envelope( + manifest: &Manifest, + models_in_scope: &ModelInScopeSet, + check_policy: &CheckPolicy, + cute_dbt_version: impl Into, + generated_at: impl Into, + scope: EnvelopeScope, +) -> FindingsEnvelope { + let findings = collect_in_scope_findings(manifest, models_in_scope, check_policy); + let metadata = EnvelopeMetadata::new(cute_dbt_version, generated_at, scope); + FindingsEnvelope::new(metadata, findings) +} + +/// Serialize the envelope to pretty (2-space) JSON with a trailing newline. +/// +/// Pretty-printed + newline-terminated so the committed golden is a +/// readable, diff-friendly artifact and the byte-identity gate is stable +/// (matching the POSIX text-file convention the other goldens follow). +/// `serde_json::to_string_pretty` is infallible for this POD (no map with +/// non-string keys, no non-finite floats), but the `Result` is surfaced +/// rather than unwrapped so a future field change cannot panic at runtime. +/// +/// # Errors +/// +/// Returns the underlying `serde_json` error if serialization fails (not +/// reachable for the current POD shape; surfaced for forward safety). +pub fn envelope_to_json(envelope: &FindingsEnvelope) -> Result { + let mut json = serde_json::to_string_pretty(envelope)?; + json.push('\n'); + Ok(json) +} + +/// Write the envelope JSON sidecar to `path`. +/// +/// Additive to the HTML report — the run loop calls this *after* the HTML +/// is written, in the same invocation. Any parent directory must already +/// exist (the same contract as the `--out` HTML path); a write failure is +/// surfaced to the caller to map onto the run-loop's output-error path. +/// +/// # Errors +/// +/// Returns the I/O error if the file cannot be written, or the +/// serialization error if the envelope cannot be encoded. +pub fn write_sidecar(envelope: &FindingsEnvelope, path: &Path) -> io::Result<()> { + let json = envelope_to_json(envelope) + .map_err(|err| io::Error::new(io::ErrorKind::InvalidData, err))?; + fs::write(path, json) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::domain::checks::CheckId; + use crate::domain::{ + Checksum, DependsOn, InScopeSet, ManifestMetadata, Node, NodeConfig, NodeId, + has_total_uncovered, + }; + use std::collections::{BTreeMap, HashMap}; + + fn checksum(value: &str) -> Checksum { + Checksum::new("sha256", value) + } + + // A model declaring config.unique_key with no backing uniqueness test — + // the production registry's Total-tier `grain.unique-key-unbacked` + // check fires UNCOVERED on it (the render layer's + // `build_payload_carries_findings_for_an_unbacked_unique_key` fixture). + fn unbacked_unique_key_model() -> Node { + let mut config = BTreeMap::new(); + config.insert("unique_key".to_owned(), serde_json::json!("order_id")); + Node::new( + NodeId::new("model.shop.orders_rollup"), + "model", + checksum("body"), + Some("select 1".to_owned()), + None, + DependsOn::default(), + None, + NodeConfig::new(config, false), + None, + BTreeMap::new(), + ) + } + + fn manifest_with(nodes: Vec) -> Manifest { + Manifest::new( + ManifestMetadata::new("v12"), + nodes.into_iter().map(|n| (n.id().clone(), n)).collect(), + HashMap::new(), + HashMap::new(), + ) + } + + #[test] + fn collect_mirrors_the_render_findings_for_an_in_scope_model() { + let manifest = manifest_with(vec![unbacked_unique_key_model()]); + let models = ModelInScopeSet::from_iter([NodeId::new("model.shop.orders_rollup")]); + let findings = collect_in_scope_findings(&manifest, &models, &CheckPolicy::default()); + assert_eq!(findings.len(), 1, "the grain check fires once"); + assert_eq!(findings[0].check.as_str(), "grain.unique-key-unbacked"); + assert_eq!(findings[0].model_id.as_str(), "model.shop.orders_rollup"); + assert!(has_total_uncovered(&findings), "Total-tier uncovered gap"); + } + + #[test] + fn collect_skips_an_unresolvable_model_id() { + let manifest = manifest_with(vec![unbacked_unique_key_model()]); + // A model id NOT in the manifest is skipped (the renderer's + // `continue` arm) — no panic, no phantom finding. + let models = ModelInScopeSet::from_iter([ + NodeId::new("model.shop.orders_rollup"), + NodeId::new("model.shop.ghost"), + ]); + let findings = collect_in_scope_findings(&manifest, &models, &CheckPolicy::default()); + assert_eq!(findings.len(), 1, "only the resolvable model contributes"); + } + + #[test] + fn collect_is_empty_when_no_models_are_in_scope() { + let manifest = manifest_with(vec![unbacked_unique_key_model()]); + let findings = collect_in_scope_findings( + &manifest, + &ModelInScopeSet::from_iter([]), + &CheckPolicy::default(), + ); + assert!(findings.is_empty()); + // Sanity: the InScopeSet import is exercised so the empty-scope + // contract is explicit (no in-scope tests ⇒ no findings). + assert!(InScopeSet::new().iter().next().is_none()); + } + + #[test] + fn build_envelope_carries_metadata_and_collected_findings() { + let manifest = manifest_with(vec![unbacked_unique_key_model()]); + let models = ModelInScopeSet::from_iter([NodeId::new("model.shop.orders_rollup")]); + let envelope = build_findings_envelope( + &manifest, + &models, + &CheckPolicy::default(), + "0.1.0", + "2026-01-15", + EnvelopeScope::PrDiff { + source: Some("@diff.patch".to_owned()), + }, + ); + assert_eq!(envelope.metadata.schema_version, 1); + assert_eq!(envelope.metadata.cute_dbt_version, "0.1.0"); + assert_eq!(envelope.metadata.generated_at, "2026-01-15"); + assert_eq!(envelope.findings.len(), 1); + } + + #[test] + fn json_is_pretty_printed_and_newline_terminated() { + let manifest = manifest_with(vec![unbacked_unique_key_model()]); + let models = ModelInScopeSet::from_iter([NodeId::new("model.shop.orders_rollup")]); + let envelope = build_findings_envelope( + &manifest, + &models, + &CheckPolicy::default(), + "0.1.0", + "2026-01-15", + EnvelopeScope::Baseline { + baseline: "baseline.json".to_owned(), + }, + ); + let json = envelope_to_json(&envelope).expect("serializes"); + assert!(json.ends_with("}\n"), "trailing newline: {json:?}"); + assert!(json.contains(" \"metadata\""), "2-space indent"); + // Round-trips back to an equal value (structural stability). + let value: serde_json::Value = serde_json::from_str(&json).expect("parses"); + assert_eq!(value["metadata"]["id_stability"], "unstable-v0.x"); + assert_eq!(value["metadata"]["scope"]["mode"], "baseline"); + } + + #[test] + fn write_sidecar_writes_the_json_to_disk() { + let dir = std::env::temp_dir().join(format!("cute-dbt-envelope-{}", std::process::id())); + fs::create_dir_all(&dir).expect("mkdir"); + let path = dir.join("findings.json"); + let manifest = manifest_with(vec![unbacked_unique_key_model()]); + let models = ModelInScopeSet::from_iter([NodeId::new("model.shop.orders_rollup")]); + let envelope = build_findings_envelope( + &manifest, + &models, + &CheckPolicy::default(), + "0.1.0", + "2026-01-15", + EnvelopeScope::PrDiff { source: None }, + ); + write_sidecar(&envelope, &path).expect("writes"); + let on_disk = fs::read_to_string(&path).expect("reads back"); + assert_eq!(on_disk, envelope_to_json(&envelope).expect("serializes")); + let _ = fs::remove_dir_all(&dir); + } +} diff --git a/src/adapters/mod.rs b/src/adapters/mod.rs index 1aff0680..82819584 100644 --- a/src/adapters/mod.rs +++ b/src/adapters/mod.rs @@ -27,6 +27,12 @@ //! (`tests.html`). Reuses [`render`]'s engine-agnostic //! `build_payload` output; fail-open on uncompiled models (rendered //! as "not compiled"). +//! - [`findings_emit`] — the machine-readable findings-envelope sidecar +//! (cute-dbt#386, epic #261): collects the in-scope findings via the +//! SAME `model_findings → apply_check_policy` pipeline the renderer +//! runs, wraps them in the versioned `FindingsEnvelope` POD, and writes +//! the JSON beside the HTML report (`--findings-out`). Purely additive +//! — never touches `report.html` or [`render`]. //! - [`project_file`] — the v0.2 `ProjectFileReader` port impl //! (`FsProjectFileReader`). Reads project-relative files for the //! authoring-YAML drawer (cute-dbt#69) and the external unit-test @@ -44,6 +50,7 @@ pub mod asset_embed; pub mod config_reader; pub mod cte_engine; pub mod explore; +pub mod findings_emit; pub mod manifest; pub mod project_def; pub mod project_file; diff --git a/src/cli/args.rs b/src/cli/args.rs index 997b9659..14854042 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -265,6 +265,47 @@ pub struct ReportArgs { /// Overrides `[pr].number` in `--config`. #[arg(long, value_name = "N")] pub pr_number: Option, + + /// Emit the machine-readable **findings envelope** JSON to this path, + /// alongside the HTML `--out` report in the same run (cute-dbt#386). + /// + /// Additive sidecar — NOT a format swap: the HTML report is written + /// exactly as before, and this writes a second `{ metadata, findings }` + /// JSON file beside it. The envelope wraps the same in-scope findings + /// the report surfaces in a versioned header + /// (`metadata.schema_version` — the integer stability anchor; check-ids + /// themselves are unstable until v1.0, so pin `schema_version`, not + /// individual ids). The file's parent directory must already exist (the + /// same contract as `--out`). + #[arg(long, value_name = "PATH")] + pub findings_out: Option, + + /// Exit non-zero when the in-scope set carries any **Total-tier + /// `Uncovered`** finding (cute-dbt#386) — the deterministic + /// coverage-gap gate for CI. + /// + /// `Total` checks are zero-false-positive by construction, so this gate + /// never trips on a heuristic guess. The exit code is dedicated + /// (distinct from the usage-error and fail-closed codes); the HTML + /// report and the `--findings-out` sidecar (if requested) are still + /// written first. Not configurable — `Total`-only is the design tenet + /// (no tier knob in v0.1). + #[arg(long)] + pub fail_on_uncovered: bool, + + /// Override the findings envelope's `metadata.generated_at` with a + /// fixed `YYYY-MM-DD` date (cute-dbt#386). + /// + /// The envelope timestamp is normally "today" computed at the I/O + /// boundary. This flag pins it to a fixed value so the committed + /// envelope golden stays byte-identical across CI runs regardless of + /// the wall-clock date (the golden-determinism rule: the domain is a + /// pure function of `(facts, generated_at)`; this is the I/O-boundary + /// injection point). Hidden — it exists only for golden regeneration / + /// reproducible builds, never a normal user surface. Inert unless + /// `--findings-out` is also set. + #[arg(long, value_name = "YYYY-MM-DD", hide = true)] + pub generated_at: Option, } /// Arguments for `cute-dbt explore` (cute-dbt#100) — full-manifest, diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 2b71d06c..62f9345c 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -34,11 +34,16 @@ //! model renders as a "not compiled" node. `PreflightError` keeps its //! four variants; explore raises no fifth. //! -//! Three exit codes: `0` success, `1` a run-time failure (a fail-closed +//! Four exit codes: `0` success, `1` a run-time failure (a fail-closed //! manifest or an unwritable output path — no partial report is ever //! written), `2` an operator usage error (clap rejected the arguments, //! including a bare `cute-dbt` with no subcommand, or supplying neither -//! or both `report` scope sources — `--baseline-manifest` / `--pr-diff`). +//! or both `report` scope sources — `--baseline-manifest` / `--pr-diff`), +//! and `3` the `report` verb's `--fail-on-uncovered` coverage gate +//! (cute-dbt#386): the report (and any `--findings-out` sidecar) IS +//! written, then the run exits non-zero because the in-scope set carries a +//! Total-tier `Uncovered` finding — distinct from `1` so CI tells a real +//! coverage gap apart from unusable input. mod args; mod exit; @@ -75,6 +80,7 @@ use std::process::ExitCode; use clap::Parser; use crate::adapters::explore::render_explore; +use crate::adapters::findings_emit::{build_findings_envelope, write_sidecar}; use crate::adapters::manifest::{FileManifestSource, load_baseline}; use crate::adapters::project_def::parse as parse_project_definition; use crate::adapters::project_file::FsProjectFileReader; @@ -84,19 +90,20 @@ use crate::adapters::render::{ }; use crate::domain::{ BlockDiff, CheckPolicy, ConfigAttribution, DEFAULT_MACRO_BODY_CAP, DEFAULT_REPORT_TITLE, - DepDate, EnabledExperiments, Experiment, FixtureTableDiff, GovernanceFacts, HeuristicId, - InScopeSet, Manifest, ModelInScopeSet, ModelYamlOutcome, NamedTableDiff, NormalizedDiffIndex, - PrConfig, PrRef, PreflightError, ProjectChangePanel, ProjectFacts, ProjectFallbackReason, - ScopeInput, ScopeSelection, SeedCard, SeedInScopeSet, SuppressRule, SuppressionSource, - UnitTest, UnitTestDataDiff, UnitTestYamlBlock, VarReference, all_models, attach_hook_facts, - attach_model_yaml_diffs, attach_var_facts, attribute_config_tree_changes, + DepDate, EnabledExperiments, EnvelopeScope, Experiment, FixtureTableDiff, GovernanceFacts, + HeuristicId, InScopeSet, Manifest, ModelInScopeSet, ModelYamlOutcome, NamedTableDiff, + NormalizedDiffIndex, PrConfig, PrRef, PreflightError, ProjectChangePanel, ProjectFacts, + ProjectFallbackReason, ScopeInput, ScopeSelection, SeedCard, SeedInScopeSet, SuppressRule, + SuppressionSource, UnitTest, UnitTestDataDiff, UnitTestYamlBlock, VarReference, all_models, + attach_hook_facts, attach_model_yaml_diffs, attach_var_facts, attribute_config_tree_changes, attribute_var_changes, build_seed_cards, changed_macros_baseline, changed_macros_pr_diff, changed_models, check_by_id, diff_project_definitions, effective_fixture_format, external_fixture_table, extract_model_block, extract_unit_test_block, gather_governance, - hook_operations, macro_focus_set, preflight_compiled, raw_hunk_lines, reconstruct_block_diffs, - reconstruct_external_fixture_diff, reconstruct_model_sql_diffs, reconstruct_table_diffs, - refine_changed_by_hunks, resolve_check_policy, resolve_experimental_config, reverse_apply, - scan_pragmas, select_in_scope, select_seeds_in_scope, widen_with_config_attributions, + has_total_uncovered, hook_operations, macro_focus_set, preflight_compiled, raw_hunk_lines, + reconstruct_block_diffs, reconstruct_external_fixture_diff, reconstruct_model_sql_diffs, + reconstruct_table_diffs, refine_changed_by_hunks, resolve_check_policy, + resolve_experimental_config, reverse_apply, scan_pragmas, select_in_scope, + select_seeds_in_scope, widen_with_config_attributions, }; use crate::ports::{ManifestSource, ProjectFileReader}; @@ -109,6 +116,14 @@ const EXIT_FAILURE: u8 = 1; /// Exit code for an operator usage error (clap rejected the arguments). const EXIT_USAGE: u8 = 2; +/// Exit code for the `--fail-on-uncovered` coverage gate (cute-dbt#386): +/// the run produced its report (and the `--findings-out` sidecar, if +/// requested) successfully, but the in-scope set carries ≥1 Total-tier +/// `Uncovered` finding. Distinct from `EXIT_FAILURE` (a fail-closed manifest +/// — no report written) so CI can tell "a real coverage gap" apart from "the +/// input was unusable". +const EXIT_GATE: u8 = 3; + /// Binary entry point: parse arguments, dispatch the selected verb's /// composition, and map the outcome to a process exit code. #[must_use] @@ -120,19 +135,25 @@ pub fn run() -> ExitCode { // Per-verb dispatch; every run-time failure is mapped to one // stderr message + exit 1 here. `review` wraps its own cli-layer // `ReviewError` (cute-dbt#300) alongside the composed report's - // `RunError`, so its arm converts to the message eagerly. - let outcome: Result<(), String> = match &cli.command { - Command::Review(review_args) => { - review::execute_review(review_args).map_err(|failure| failure.message()) - } + // `RunError`, so its arm converts to the message eagerly. The + // `report` verb additionally carries the `--fail-on-uncovered` gate + // outcome (cute-dbt#386): a successful run can still exit + // `EXIT_GATE` when an in-scope Total-tier coverage gap is present. + let outcome: Result = match &cli.command { + Command::Review(review_args) => review::execute_review(review_args) + .map(|()| ReportOutcome::Success) + .map_err(|failure| failure.message()), Command::Report(report) => execute_report(report).map_err(|failure| failure.message()), - Command::Explore(explore) => execute_explore(explore).map_err(|failure| failure.message()), - Command::Skill(skill_args) => { - skill::execute_skill(skill_args).map_err(|failure| failure.message()) - } + Command::Explore(explore) => execute_explore(explore) + .map(|()| ReportOutcome::Success) + .map_err(|failure| failure.message()), + Command::Skill(skill_args) => skill::execute_skill(skill_args) + .map(|()| ReportOutcome::Success) + .map_err(|failure| failure.message()), }; match outcome { - Ok(()) => ExitCode::SUCCESS, + Ok(ReportOutcome::Success) => ExitCode::SUCCESS, + Ok(ReportOutcome::UncoveredGate) => ExitCode::from(EXIT_GATE), Err(message) => { eprintln!("{message}"); ExitCode::from(EXIT_FAILURE) @@ -140,6 +161,23 @@ pub fn run() -> ExitCode { } } +/// The success-side outcome of a verb run. +/// +/// Almost every run is [`ReportOutcome::Success`]; the `report` verb's +/// `--fail-on-uncovered` gate (cute-dbt#386) is the one path that produces +/// its output *and* signals a distinct non-zero exit +/// ([`ReportOutcome::UncoveredGate`]) — a deterministic Total-tier coverage +/// gap, not a fail-closed input. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ReportOutcome { + /// The run completed; exit `0`. + Success, + /// The run wrote its report (and any `--findings-out` sidecar) but the + /// in-scope set carries a Total-tier `Uncovered` finding and + /// `--fail-on-uncovered` was set; exit [`EXIT_GATE`]. + UncoveredGate, +} + /// Print a clap parse error and pick its exit code. /// /// clap routes genuine usage errors to stderr (exit `2`) and a `--help` @@ -217,7 +255,7 @@ impl RunError { // Splitting the loop into sub-loops would add indirection that buys nothing // at this single composition site (the `render_report` rationale, cli edition). #[allow(clippy::too_many_lines)] -fn execute_report(args: &ReportArgs) -> Result<(), RunError> { +fn execute_report(args: &ReportArgs) -> Result { let current = load_current(args)?; let scope_input = resolve_scope_input(args)?; // Experimental switch (cute-dbt#289, epic #288): the resolved @@ -456,7 +494,98 @@ fn execute_report(args: &ReportArgs) -> Result<(), RunError> { &seed_cards, ) .map_err(|err| RunError::output(&args.out, err))?; - Ok(()) + // cute-dbt#386 — the machine-readable findings envelope. Purely + // additive to the HTML report above (the render path is untouched): + // the SAME in-scope `model_findings → apply_check_policy` pipeline the + // renderer ran is re-derived in the emit adapter so the envelope's + // findings match the report's exactly. Two consumers, both gated on + // their flags and both no-ops by default: + // - `--findings-out ` writes the `{ metadata, findings }` + // sidecar JSON beside the HTML report; + // - `--fail-on-uncovered` exits `EXIT_GATE` iff the in-scope set + // carries a Total-tier `Uncovered` finding (deterministic gap). + // Both share one collection pass — skipped entirely when neither flag + // is set (zero added work on the default path). + finalize_findings( + args, + ¤t, + &models_in_scope, + &check_policy, + &scope_input, + ) +} + +/// The `--findings-out` / `--fail-on-uncovered` tail of the `report` run +/// loop (cute-dbt#386) — runs strictly AFTER the HTML report is written. +/// +/// Collects the in-scope findings once (via the emit adapter's mirror of +/// the renderer's `model_findings → apply_check_policy` pipeline) and feeds +/// both consumers from that single pass. Returns +/// [`ReportOutcome::UncoveredGate`] iff `--fail-on-uncovered` is set and a +/// Total-tier `Uncovered` finding is in scope; otherwise +/// [`ReportOutcome::Success`]. Both flags off ⇒ no collection, immediate +/// `Success` (the default path adds zero work). +/// +/// `generated_at` is the RFC3339 **date** ([`today_rfc3339_date`]) computed +/// here at the I/O boundary and threaded into the pure envelope builder, so +/// the committed envelope golden stays byte-stable (the golden-determinism +/// rule; the fixture pins a far-past/far-future date) and `cute-dbt` carries +/// no `chrono`/`time` dependency. +fn finalize_findings( + args: &ReportArgs, + current: &Manifest, + models_in_scope: &ModelInScopeSet, + check_policy: &CheckPolicy, + scope_input: &ScopeInput, +) -> Result { + if args.findings_out.is_none() && !args.fail_on_uncovered { + return Ok(ReportOutcome::Success); + } + // `generated_at` is the I/O-boundary date — the `--generated-at` + // override (golden regeneration / reproducible builds) over today's + // computed civil date. Threaded into the pure envelope builder so the + // committed golden is byte-stable. + let generated_at = args.generated_at.clone().unwrap_or_else(today_rfc3339_date); + let envelope = build_findings_envelope( + current, + models_in_scope, + check_policy, + env!("CARGO_PKG_VERSION"), + generated_at, + envelope_scope(args, scope_input), + ); + if let Some(path) = &args.findings_out { + write_sidecar(&envelope, path).map_err(|err| RunError::output(path, err))?; + } + let gate_tripped = args.fail_on_uncovered && has_total_uncovered(&envelope.findings); + Ok(if gate_tripped { + ReportOutcome::UncoveredGate + } else { + ReportOutcome::Success + }) +} + +/// Build the envelope's [`EnvelopeScope`] from the run's scope source — +/// the machine-readable twin of [`scope_banner`] (cute-dbt#386). +/// +/// `Baseline` carries the `--baseline-manifest` path verbatim (empty when +/// somehow absent — omitted from JSON). `PrDiff` deliberately carries NO +/// source label (`source: None`, omitted): the parsed [`PrDiff`] retains +/// only the changed-file facts, not the `@file` argument, and embedding the +/// raw `@file` path could bake a CI-runner-absolute path into the committed +/// artifact (the same `root_path`-leak class the manifest gitignore guards). +/// The slot is reserved (forward-compatible) but unpopulated — the `mode: +/// "pr-diff"` tag is the arm discriminator a consumer keys on. +fn envelope_scope(args: &ReportArgs, scope_input: &ScopeInput) -> EnvelopeScope { + match scope_input { + ScopeInput::Baseline { .. } => EnvelopeScope::Baseline { + baseline: args + .baseline_manifest + .as_ref() + .map_or_else(String::new, |p| p.display().to_string()), + }, + ScopeInput::PrDiff { .. } => EnvelopeScope::PrDiff { source: None }, + } } /// The named `explore` run loop (cute-dbt#100) — `load_current` → @@ -1449,6 +1578,24 @@ fn today_dep_date() -> DepDate { DepDate { year, month, day } } +/// The findings envelope's `generated_at`, computed at the I/O boundary +/// (cute-dbt#386). +/// +/// An **RFC3339 date** (`YYYY-MM-DD`) — a deliberate precision choice: a +/// full RFC3339 *date-time* would need timezone + time-of-field formatting +/// that std cannot produce without a `chrono`/`time` dependency, and +/// cute-dbt is std-only by posture (cargo-deny + the no-extra-date-crate +/// line). The date is the deterministic, golden-stable granularity the +/// envelope needs (the committed golden pins a fixed fixture date); a +/// finer-grained timestamp would defeat byte-identity gating. Reuses the +/// same `today_dep_date` → `civil_from_days` machinery as the governance +/// deprecation chips, so "today" is computed once, the same way, at the one +/// I/O boundary. `YYYY-MM-DD` is a valid RFC3339 `full-date`. +fn today_rfc3339_date() -> String { + let DepDate { year, month, day } = today_dep_date(); + format!("{year:04}-{month:02}-{day:02}") +} + /// Days-since-Unix-epoch → `(year, month, day)` (proleptic Gregorian) — /// Howard Hinnant's `civil_from_days` algorithm, std-only (the domain /// forbids `chrono`). Pure + total. `m`/`d` are in `[1, 12]` / `[1, 31]` @@ -1730,6 +1877,9 @@ mod tests { pr_url: None, pr_title: None, pr_number: None, + findings_out: None, + fail_on_uncovered: false, + generated_at: None, } } diff --git a/src/cli/review.rs b/src/cli/review.rs index d9018ea3..d41101f6 100644 --- a/src/cli/review.rs +++ b/src/cli/review.rs @@ -1544,6 +1544,16 @@ impl ComposeInputs { pr_url: self.pr.as_ref().map(|p| p.url.clone()), pr_title: self.pr.as_ref().map(|p| p.title.clone()), pr_number: self.pr.as_ref().map(|p| p.number), + // cute-dbt#386 — the findings-envelope sidecar + coverage gate + // are `report`-verb surfaces; `review` does not emit the + // envelope or apply the gate in this slice (both off). A future + // slice can wire `review`'s own `--findings-out` / gate flags + // through here. + findings_out: None, + fail_on_uncovered: false, + // No envelope on `review` ⇒ the `--generated-at` override is + // inert; `None` defers to the I/O-boundary date either way. + generated_at: None, } } } diff --git a/src/domain/findings_envelope.rs b/src/domain/findings_envelope.rs new file mode 100644 index 00000000..c93be3bf --- /dev/null +++ b/src/domain/findings_envelope.rs @@ -0,0 +1,343 @@ +//! The machine-readable **findings envelope** (cute-dbt#386, epic #261). +//! +//! A thin `metadata.schema_version` header wrapped around the already- +//! serializing [`Finding`] POD — emitted as a **sidecar** JSON file +//! beside the HTML report (`--findings-out `), never a `--format +//! json` swap. This is the sanctioned reversal of `ARCHITECTURE.md` +//! conscious-simplification row-5 ("no JSON wire envelope"): the founder's +//! ADR-4 amendment (2026-06-11) + the #261 locked decisions make the +//! additive sidecar land while the HTML output stays byte-identical. +//! +//! ## The four locked decisions ([#261](https://github.com/breezy-bays-labs/cute-dbt/issues/261)) +//! +//! - **D1 — sidecar delivery.** Additive emit beside the HTML report in +//! one run; mirrors dbt (`manifest`/`run_results` are sidecars). +//! - **D2 — check-id instability.** Check-ids churn freely in v0.x and +//! freeze at v1.0. The envelope carries a machine-readable +//! [`ID_STABILITY`] notice so consumers pin [`SCHEMA_VERSION`] — the +//! *only* stability anchor pre-v1.0 — not individual check-ids. +//! - **D3 — gate semantics.** [`has_total_uncovered`] is true iff ≥1 +//! [`Tier::Total`] + [`Verdict::Uncovered`] finding exists. Total-only, +//! not configurable — `Total` is deterministic (zero false positives), +//! so gating on it never breaks a build on a heuristic guess. +//! - **D4 — `OpenLineage`.** The fields are shaped to map cleanly onto a +//! future `OpenLineage` facet projection, but **no** OL output is emitted +//! in this slice. +//! +//! `severity` is the existing [`Tier`] enum (no new field). `schema_version` +//! is an **integer** starting at `1` (dbt-style, not a semver string). +//! Owner fields are reserved for #256 (populated later, additively) — this +//! slice does **not** carry them. +//! +//! ## Layering +//! +//! This module is **pure** ([`ARCHITECTURE.md` §1] domain discipline): +//! POD + serde derive + the gate predicate, no I/O. `generated_at` is +//! threaded in as a parameter computed at the CLI I/O boundary +//! (golden-determinism rule) so the envelope golden is byte-stable and the +//! domain stays a pure function of `(facts, generated_at)`. The +//! *emit-to-file* lives in the [`findings_emit`](crate::adapters) adapter. +//! +//! [`Finding`]: crate::domain::checks::Finding +//! [`Tier::Total`]: crate::domain::checks::Tier::Total +//! [`Verdict::Uncovered`]: crate::domain::checks::Verdict::Uncovered +//! [`ARCHITECTURE.md` §1]: https://github.com/breezy-bays-labs/cute-dbt/blob/main/ARCHITECTURE.md + +use serde::Serialize; + +use crate::domain::checks::{Finding, HeuristicId, Tier, Verdict}; + +/// The envelope schema version — an **integer**, the only stability anchor +/// pre-v1.0 (decision D2). Bumped only on a breaking envelope-shape change; +/// the inner check-ids are explicitly unstable in v0.x. +pub const SCHEMA_VERSION: u32 = 1; + +/// The machine-readable check-id-instability notice (decision D2). Consumers +/// pin [`SCHEMA_VERSION`], not individual check-ids, until v1.0. +pub const ID_STABILITY: &str = "unstable-v0.x"; + +/// Which scope source produced the in-scope set this envelope reports +/// over — the machine-readable twin of the report's diff-scope banner. +/// +/// Serializes with an internal `mode` tag (`"pr-diff"` | `"baseline"`) and +/// the arm-specific source field, mirroring the existing scope plumbing +/// (the `--baseline-manifest` path verbatim; the `--pr-diff` source label). +/// A field is omitted when absent so the shape stays minimal. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(tag = "mode", rename_all = "kebab-case")] +pub enum EnvelopeScope { + /// `--baseline-manifest` arm — diff against a baseline manifest. + Baseline { + /// The baseline manifest path the run diffed against (the + /// `--baseline-manifest` value verbatim). Omitted when empty. + #[serde(skip_serializing_if = "str::is_empty")] + baseline: String, + }, + /// `--pr-diff` arm — scope from a unified-diff patch. + #[serde(rename = "pr-diff")] + PrDiff { + /// The PR-diff source label (the `--pr-diff @file` argument, e.g. + /// `@diff.patch`). Omitted when absent. + #[serde(skip_serializing_if = "Option::is_none")] + source: Option, + }, +} + +/// The envelope header (decision-locked shape, #261). +/// +/// `schema_version` + `id_stability` are constants pinned from +/// [`SCHEMA_VERSION`] / [`ID_STABILITY`] by [`EnvelopeMetadata::new`] so a +/// caller cannot drift them. `cute_dbt_version` is the crate version; +/// `generated_at` is threaded from the CLI I/O boundary. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct EnvelopeMetadata { + /// The envelope schema version (integer; the only pre-v1.0 stability + /// anchor). Always [`SCHEMA_VERSION`]. + pub schema_version: u32, + /// The producing `cute-dbt` crate version (e.g. `"0.1.0"`). + pub cute_dbt_version: String, + /// Generation timestamp, RFC3339 (computed at the CLI I/O boundary so + /// the golden is deterministic). + pub generated_at: String, + /// Which scope source produced the in-scope finding set. + pub scope: EnvelopeScope, + /// The machine-readable check-id-instability notice (D2). Always + /// [`ID_STABILITY`] in v0.x. + pub id_stability: String, +} + +impl EnvelopeMetadata { + /// Build the metadata header, pinning [`SCHEMA_VERSION`] / + /// [`ID_STABILITY`] from the constants so they cannot drift. + /// + /// `cute_dbt_version` is typically `env!("CARGO_PKG_VERSION")` and + /// `generated_at` the RFC3339 timestamp computed at the CLI I/O + /// boundary — both supplied by the caller so the domain stays a pure + /// function of its inputs. + #[must_use] + pub fn new( + cute_dbt_version: impl Into, + generated_at: impl Into, + scope: EnvelopeScope, + ) -> Self { + Self { + schema_version: SCHEMA_VERSION, + cute_dbt_version: cute_dbt_version.into(), + generated_at: generated_at.into(), + scope, + id_stability: ID_STABILITY.to_owned(), + } + } +} + +/// The findings envelope: a `metadata` header + a flat `findings` list. +/// +/// Each [`Finding`] already carries `model_id` / `check` / `tier` / +/// `verdict` / `evidence` / `recommendation` / `degraded` / `suppressed`, +/// so consumers group by `model_id` / `check` / `tier` themselves — the +/// envelope adds no grouping, only the versioned header. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct FindingsEnvelope { + /// The versioned header (decision D2 stability anchor lives here). + pub metadata: EnvelopeMetadata, + /// The flat in-scope finding set, in the order the run loop collected + /// it (deterministic over the in-scope model set + each model's + /// `model_findings` output). + pub findings: Vec>, +} + +impl FindingsEnvelope { + /// Assemble an envelope from its already-computed parts. + #[must_use] + pub fn new(metadata: EnvelopeMetadata, findings: Vec>) -> Self { + Self { metadata, findings } + } +} + +/// The `--fail-on-uncovered` gate predicate (decision D3). +/// +/// `true` iff **any** finding is both [`Tier::Total`] and +/// [`Verdict::Uncovered`] — a deterministic coverage gap on a zero-false- +/// positive check. Not configurable: `Total`-only is the design tenet, so +/// the gate never trips on a `High`/`Advisory` heuristic guess and never +/// on a `Covered`/`Unknown` verdict. +/// +/// A `suppressed` Total-tier uncovered finding still counts: suppression is +/// a *display* acknowledgement, not a coverage fact — the gap is real, the +/// operator merely chose not to surface it in the HTML. (If a future slice +/// wants suppression to exempt the gate, that is a deliberate decision, not +/// a silent default; this predicate keeps the honest "the gap exists" read.) +#[must_use] +pub fn has_total_uncovered(findings: &[Finding]) -> bool { + findings + .iter() + .any(|f| f.tier == Tier::Total && f.verdict == Verdict::Uncovered) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::domain::checks::{HeuristicId, Verdict}; + use crate::domain::manifest::NodeId; + + // A minimal Finding builder over a chosen production-registry check. + // `Finding::new` denormalizes tier/instrument from the check's spec, so + // `total_finding` (the `grain.unique-key-unbacked` Total-tier check at + // index 0) and `high_finding` (the `union.arm-coverage` High-tier check + // at index 1) let us exercise the tier-sensitive gate predicate. + fn finding_with(check: HeuristicId, verdict: Verdict) -> Finding { + Finding::new( + check, + NodeId::new("model.shop.dim_x"), + "config.unique_key", + verdict, + Vec::new(), + ) + } + + fn finding(verdict: Verdict) -> Finding { + finding_with(HeuristicId::GrainUniqueKeyUnbacked, verdict) + } + + // ---- serde round-trip ------------------------------------------- + + #[test] + fn metadata_pins_schema_version_and_id_stability_from_constants() { + let meta = EnvelopeMetadata::new( + "0.1.0", + "2026-01-15", + EnvelopeScope::PrDiff { + source: Some("@diff.patch".to_owned()), + }, + ); + assert_eq!(meta.schema_version, SCHEMA_VERSION); + assert_eq!(meta.schema_version, 1); + assert_eq!(meta.id_stability, ID_STABILITY); + assert_eq!(meta.id_stability, "unstable-v0.x"); + } + + #[test] + fn envelope_serializes_with_metadata_header_and_flat_findings() { + let envelope = FindingsEnvelope::new( + EnvelopeMetadata::new( + "0.1.0", + "2026-01-15", + EnvelopeScope::Baseline { + baseline: "baseline.json".to_owned(), + }, + ), + vec![finding(Verdict::Uncovered)], + ); + let json = serde_json::to_value(&envelope).expect("serializes"); + assert_eq!(json["metadata"]["schema_version"], 1); + assert_eq!(json["metadata"]["cute_dbt_version"], "0.1.0"); + assert_eq!(json["metadata"]["generated_at"], "2026-01-15"); + assert_eq!(json["metadata"]["id_stability"], "unstable-v0.x"); + assert_eq!(json["metadata"]["scope"]["mode"], "baseline"); + assert_eq!(json["metadata"]["scope"]["baseline"], "baseline.json"); + assert!(json["findings"].is_array()); + assert_eq!(json["findings"].as_array().unwrap().len(), 1); + // The wrapped Finding keeps its existing serialized shape. + assert_eq!(json["findings"][0]["model_id"], "model.shop.dim_x"); + assert_eq!(json["findings"][0]["tier"], "total"); + assert_eq!(json["findings"][0]["verdict"]["status"], "uncovered"); + } + + #[test] + fn schema_version_is_an_integer_not_a_string() { + let meta = EnvelopeMetadata::new( + "0.1.0", + "2026-01-15", + EnvelopeScope::Baseline { + baseline: String::new(), + }, + ); + let json = serde_json::to_value(&meta).expect("serializes"); + assert!( + json["schema_version"].is_u64(), + "schema_version must be an integer, got {:?}", + json["schema_version"] + ); + } + + // ---- scope arms -------------------------------------------------- + + #[test] + fn pr_diff_scope_tags_mode_and_carries_source() { + let json = serde_json::to_value(EnvelopeScope::PrDiff { + source: Some("@pr.patch".to_owned()), + }) + .expect("serializes"); + assert_eq!(json["mode"], "pr-diff"); + assert_eq!(json["source"], "@pr.patch"); + } + + #[test] + fn pr_diff_scope_omits_source_when_absent() { + let json = + serde_json::to_value(EnvelopeScope::PrDiff { source: None }).expect("serializes"); + assert_eq!(json["mode"], "pr-diff"); + assert!( + json.get("source").is_none(), + "absent source must be omitted: {json:?}" + ); + } + + #[test] + fn baseline_scope_omits_empty_label() { + let json = serde_json::to_value(EnvelopeScope::Baseline { + baseline: String::new(), + }) + .expect("serializes"); + assert_eq!(json["mode"], "baseline"); + assert!( + json.get("baseline").is_none(), + "empty baseline label must be omitted: {json:?}" + ); + } + + // ---- gate predicate (D3) ---------------------------------------- + + #[test] + fn gate_trips_on_a_total_uncovered_finding() { + assert!(has_total_uncovered(&[finding(Verdict::Uncovered)])); + } + + #[test] + fn gate_does_not_trip_on_a_total_covered_finding() { + assert!(!has_total_uncovered(&[finding(Verdict::Covered { + by: vec!["test.shop.t".to_owned()] + })])); + } + + #[test] + fn gate_does_not_trip_on_a_total_unknown_finding() { + assert!(!has_total_uncovered(&[finding(Verdict::Unknown)])); + } + + #[test] + fn gate_does_not_trip_on_an_empty_finding_set() { + assert!(!has_total_uncovered(&[])); + } + + #[test] + fn gate_does_not_trip_on_a_high_tier_uncovered_finding() { + // `union.arm-coverage` is a High-tier check — an uncovered verdict + // on it is a heuristic cue, never a gate trip (D3 Total-only tenet). + let high = finding_with(HeuristicId::UnionArmCoverage, Verdict::Uncovered); + assert_eq!(high.tier, Tier::High); + assert!(!has_total_uncovered(&[high])); + } + + #[test] + fn gate_trips_when_any_total_uncovered_is_present_among_others() { + let findings = vec![ + finding(Verdict::Covered { + by: vec!["test.shop.t".to_owned()], + }), + finding(Verdict::Unknown), + finding(Verdict::Uncovered), + ]; + assert!(has_total_uncovered(&findings)); + } +} diff --git a/src/domain/mod.rs b/src/domain/mod.rs index b98a8338..b8bc451e 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -41,6 +41,7 @@ pub mod checks; pub mod config; pub mod cte; pub mod experimental; +pub mod findings_envelope; pub mod governance; pub mod grain; pub mod macro_lens; @@ -83,6 +84,10 @@ pub use experimental::{ DEFAULT_MACRO_BODY_CAP, EnabledExperiments, Experiment, ExperimentalConfig, ExperimentalError, parse_experimental_env, resolve_experimental_config, }; +pub use findings_envelope::{ + EnvelopeMetadata, EnvelopeScope, FindingsEnvelope, ID_STABILITY, SCHEMA_VERSION, + has_total_uncovered, +}; pub use governance::{ BlastRadius, BreakingReason, ColumnMetaTags, ConstraintSupport, ContractChange, ContractClass, ContractColumnDiff, DepDate, GovChip, GovernanceFacts, GroupChip, MetaPair, ModelMetaTags, diff --git a/tests/run_loop.rs b/tests/run_loop.rs index b1c19407..37c6f27b 100644 --- a/tests/run_loop.rs +++ b/tests/run_loop.rs @@ -612,3 +612,137 @@ fn flat_pre_verb_invocation_is_a_usage_error() { "the flat pre-verb shape is a usage error: {output:?}", ); } + +// ===== cute-dbt#386 — findings envelope + coverage gate ================= +// +// The playground manifest under `--pr-diff` carries exactly one Total-tier +// `Uncovered` finding (`model.healthcare_analytics.mart_dq_summary`, the +// `grain.unique-key-unbacked` check), so it exercises both consumers: the +// `--findings-out` sidecar shape and the `--fail-on-uncovered` gate trip. + +/// The four shared args for the playground `--pr-diff` showcase run. +fn playground_pr_diff_args(out: &Path) -> Vec { + vec![ + "report".to_owned(), + "--manifest".to_owned(), + s(&fixture("playground-current.json")).to_owned(), + "--pr-diff".to_owned(), + format!("@{}", s(&fixture("playground-pr-diff.patch"))), + "--project-root".to_owned(), + s(&fixture("playground-source")).to_owned(), + "--out".to_owned(), + s(out).to_owned(), + ] +} + +#[test] +fn findings_out_writes_the_envelope_sidecar_alongside_the_html() { + // The HTML report AND the machine-readable envelope are both written + // in one invocation (the sidecar delivery shape, decision D1). + let html = tmp("envelope_sidecar.html"); + let sidecar = tmp("envelope_sidecar.json"); + clear(&html); + clear(&sidecar); + let mut args = playground_pr_diff_args(&html); + args.push("--findings-out".to_owned()); + args.push(s(&sidecar).to_owned()); + let refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let output = run(&refs); + assert!( + output.status.success(), + "findings-out alone (no gate) exits 0: {output:?}" + ); + assert!(html.exists(), "the HTML report is still written"); + let json = std::fs::read_to_string(&sidecar).expect("the envelope sidecar was written"); + // The versioned header (decision D2 — the integer stability anchor + + // the id-instability notice) and the flat findings list. + assert!( + json.contains("\"schema_version\": 1"), + "integer schema_version header: {json}" + ); + assert!( + json.contains("\"id_stability\": \"unstable-v0.x\""), + "the machine-readable id-instability notice: {json}" + ); + assert!( + json.contains("\"mode\": \"pr-diff\""), + "the pr-diff scope mode: {json}" + ); + assert!( + json.contains("\"grain.unique-key-unbacked\""), + "the in-scope grain finding rides the envelope: {json}" + ); + assert!(json.ends_with("}\n"), "trailing newline (diff-friendly)"); +} + +#[test] +fn fail_on_uncovered_exits_the_gate_code_and_still_writes_the_report() { + // Decision D3: a Total-tier `Uncovered` finding in scope exits the + // dedicated gate code (3) — distinct from the usage (2) and fail-closed + // (1) codes — AFTER the HTML report (and any sidecar) is written. + let html = tmp("gate_trips.html"); + let sidecar = tmp("gate_trips.json"); + clear(&html); + clear(&sidecar); + let mut args = playground_pr_diff_args(&html); + args.push("--fail-on-uncovered".to_owned()); + args.push("--findings-out".to_owned()); + args.push(s(&sidecar).to_owned()); + let refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let output = run(&refs); + assert_eq!( + output.status.code(), + Some(3), + "a Total-tier uncovered gap exits the dedicated gate code: {output:?}" + ); + assert!( + html.exists(), + "the HTML report is written before the gate trips (gate ≠ fail-closed)" + ); + assert!( + sidecar.exists(), + "the sidecar is also written before the gate" + ); +} + +#[test] +fn fail_on_uncovered_exits_zero_when_no_total_uncovered_finding_is_in_scope() { + // The jaffle-shop diff carries no Total-tier uncovered finding, so the + // gate passes (exit 0) — the gate is silent on covered/unknown/empty. + let html = tmp("gate_passes.html"); + clear(&html); + let output = run(&[ + "report", + "--manifest", + s(&fixture("jaffle-shop-current.json")), + "--baseline-manifest", + s(&fixture("jaffle-shop-baseline.json")), + "--out", + s(&html), + "--fail-on-uncovered", + ]); + assert!( + output.status.success(), + "no Total-tier uncovered gap ⇒ exit 0: {output:?}" + ); + assert!(html.exists(), "the report is written"); +} + +#[test] +fn default_path_writes_no_envelope_sidecar() { + // Neither flag set ⇒ the envelope path is skipped entirely (zero added + // work) and no sidecar JSON is produced. + let html = tmp("no_envelope.html"); + let phantom_sidecar = tmp("no_envelope.json"); + clear(&html); + clear(&phantom_sidecar); + let refs: Vec = playground_pr_diff_args(&html); + let borrowed: Vec<&str> = refs.iter().map(String::as_str).collect(); + let output = run(&borrowed); + assert!(output.status.success(), "default run exits 0: {output:?}"); + assert!(html.exists(), "the HTML report is written"); + assert!( + !phantom_sidecar.exists(), + "no sidecar is written without --findings-out" + ); +} From 27c0f7285e790930249de93615e6e993ada4987a Mon Sep 17 00:00:00 2001 From: Christopher Bays Date: Sat, 13 Jun 2026 21:38:38 -0400 Subject: [PATCH 2/4] fix(cli): qualify PrDiff intra-doc link for --document-private-items doc gate --- src/cli/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 62f9345c..9c225ee5 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -570,7 +570,7 @@ fn finalize_findings( /// /// `Baseline` carries the `--baseline-manifest` path verbatim (empty when /// somehow absent — omitted from JSON). `PrDiff` deliberately carries NO -/// source label (`source: None`, omitted): the parsed [`PrDiff`] retains +/// source label (`source: None`, omitted): the parsed [`PrDiff`](crate::domain::pr_diff::PrDiff) retains /// only the changed-file facts, not the `@file` argument, and embedding the /// raw `@file` path could bake a CI-runner-absolute path into the committed /// artifact (the same `root_path`-leak class the manifest gitignore guards). From 2835bf6d51f30cf9db94b3154445d726ff963334 Mon Sep 17 00:00:00 2001 From: Christopher Bays Date: Sat, 13 Jun 2026 22:15:23 -0400 Subject: [PATCH 3/4] =?UTF-8?q?feat(cli):=20findings-envelope=20review=20a?= =?UTF-8?q?mendments=20=E2=80=94=20doc=20consistency,=20unwritable-sidecar?= =?UTF-8?q?=20test,=20reserved=20anchor=20slot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial-review CONCERNS + a founder decision, all additive/byte-safe (no blocker, no major; #386 code was correct). Three amendments to PR #388: A. Doc-consistency — the row-5 reversal now propagates to ALL four conscious-simplification enumerations (only ARCHITECTURE row-5 + AGENTS.md were updated before): - ARCHITECTURE.md §2 intro: dropped "JSON wire envelopes" from the "deliberately does not adopt" list + added a forward-pointer to row 5 (was self-contradicting the reversal subsection two lines below). - README.md: dropped "no JSON envelope" + noted the reversal. - CONTRIBUTING.md: "five" -> "four"; dropped "no JSON envelope ADR"; FIXED the false enforcement claim — non-mirror-guard enforces the workspace/API-shim/AST-purity tripwires; the reversed envelope (row 5) is byte-identity-GOLDEN-gated (examples/diff-showcase-findings.json), not absence-enforced. B. Silent-failure gap — added the unwritable-`--findings-out`-path integration tests (the `--out` HTML twin existed; the sidecar one did not, so a future `let _ = write_sidecar(...)` would have passed CI): - an_unwritable_findings_out_path_is_reported (non-existent parent dir -> exit 1 + "could not write"). - an_unwritable_findings_out_path_wins_over_the_uncovered_gate (the write failure (1) wins over the gate code (3) — precedence pinned). C. Reserved anchor slot (founder decision — lands in schema_version 1 before merge): downstream projections (GitHub annotations, #353 PR comments, SARIF) all need a finding->(path,line) anchor. Added an optional `anchor: Option` per envelope finding — `FindingAnchor { path, line, diff_context, anchor_hash }` (all Option), `DiffContext { added|removed|modified }` (snake_case). ALL fields None/reserved this slice — NO resolver logic. - BYTE-SAFE by construction: each Finding is `#[serde(flatten)]`ed into `EnvelopeFinding { #[serde(flatten)] finding, #[serde(skip_serializing_if)] anchor }`, so anchor=None emits ZERO bytes. The committed envelope golden (examples/diff-showcase-findings.json) regenerates BYTE-IDENTICAL — verified empirically (git status examples/ clean). - TDD: anchor-less EnvelopeFinding serializes byte-identically to a bare Finding; populated anchor round-trips + nests under `anchor`; default anchor omits every unset field; DiffContext snake_case. POD-only, domain-pure (findings_envelope.rs). - The gate (`has_total_uncovered`) reads the raw `Finding`s before the wrap; finalize_findings collects once, gates, then builds the envelope from the same vec (envelope_from_findings). Ratified, unchanged: the PrDiff scope.source = None deviation (leak-class avoidance + reserved slot). HTML byte-goldens AND the envelope JSON golden both unchanged. Rebased on origin/main (#372, #383). Closes #386 Co-Authored-By: Claude Opus 4.8 (1M context) --- ARCHITECTURE.md | 12 +- CONTRIBUTING.md | 18 ++- README.md | 6 +- src/adapters/findings_emit.rs | 18 +++ src/cli/mod.rs | 28 +++-- src/domain/findings_envelope.rs | 197 ++++++++++++++++++++++++++++++-- src/domain/mod.rs | 4 +- tests/run_loop.rs | 58 ++++++++++ 8 files changed, 309 insertions(+), 32 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b9e551ee..b7d5c029 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -77,11 +77,13 @@ output and exactly one parser in the dependency graph. Several pieces of common Rust apparatus exist for projects whose shape cute-dbt does not have — multi-crate workspaces for crates with multiple linkage-level consumers, public-API shims for library consumers, AST-purity bans for shared cores -with rival adapter parsers, JSON wire envelopes for machine-readable output. -cute-dbt deliberately does **not** adopt them. The absences are documented -architectural choices, not accidents — recording them stops a future -contributor (human or agent) from "completing the pattern" by adding -machinery that guards no invariant here. +with rival adapter parsers. cute-dbt deliberately does **not** adopt them. +The absences are documented architectural choices, not accidents — +recording them stops a future contributor (human or agent) from "completing +the pattern" by adding machinery that guards no invariant here. (One former +member of this list — a JSON wire envelope for machine-readable output — +was **consciously reversed** at cute-dbt#386; it is now present as the +findings-envelope sidecar. See row 5 and the row-5-reversal note below.) | # | Apparatus | cute-dbt | Why N/A | Enforcement | |---|---|---|---|---| diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 84f8d6cd..80b91a9f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -145,10 +145,20 @@ is **strict**: mapping, and the run-loop composition. Application orchestration lives here (single-crate composition choice — see ARCHITECTURE). -The five **conscious design simplifications** (no workspace, no per-crate -versioning, no public-API shim, no AST-purity grep, no JSON envelope ADR) -are documented in `ARCHITECTURE.md` and enforced by the `non-mirror-guard` -CI job. Adding one is a regression, not a "pattern completion." +The four **conscious design simplifications** (no workspace, no per-crate +versioning, no public-API shim, no AST-purity grep) are documented in +`ARCHITECTURE.md`. The `non-mirror-guard` CI job enforces three of them by +rejecting their tripwires (`[workspace]`, `pub use crate::…::…`, +`bans.deny.wrappers`); per-crate versioning is enforced by absence. Adding +one is a regression, not a "pattern completion." + +A sixth original simplification — **no JSON wire envelope** — was +**consciously reversed** at cute-dbt#386 (founder ADR-4 amendment): the +machine-readable findings-envelope sidecar (`--findings-out`) now lands. Its +shape is pinned not by absence but by a **byte-identity golden** +(`examples/diff-showcase-findings.json`, gated in `example-report-check`), +the same gate class as the HTML examples. See `ARCHITECTURE.md` §2 (row 5 + +the row-5-reversal note). ## Exclusions and tracking-issue rule diff --git a/README.md b/README.md index 0a8c7eee..c366483f 100644 --- a/README.md +++ b/README.md @@ -258,8 +258,10 @@ reach `compiled_code`. Single-crate Rust CLI, hexagonal **inward-dependency discipline**. `src/{domain, ports, adapters, cli}` + `main.rs`. The full architecture invariants, the two-stage fail-closed contract, and the conscious design -simplifications (no workspace, no public-API shim, no JSON envelope) are in -[`ARCHITECTURE.md`](ARCHITECTURE.md). +simplifications (no workspace, no public-API shim) are in +[`ARCHITECTURE.md`](ARCHITECTURE.md) — including the one that was +consciously reversed at cute-dbt#386 (the machine-readable findings-envelope +sidecar, the former "no JSON envelope" line). ## Documentation diff --git a/src/adapters/findings_emit.rs b/src/adapters/findings_emit.rs index 0de9160b..f1bbf8d5 100644 --- a/src/adapters/findings_emit.rs +++ b/src/adapters/findings_emit.rs @@ -78,6 +78,24 @@ pub fn build_findings_envelope( scope: EnvelopeScope, ) -> FindingsEnvelope { let findings = collect_in_scope_findings(manifest, models_in_scope, check_policy); + envelope_from_findings(findings, cute_dbt_version, generated_at, scope) +} + +/// Assemble a [`FindingsEnvelope`] from an **already-collected** finding set. +/// +/// The same builder as [`build_findings_envelope`] but over a finding `Vec` +/// the caller already holds — so the cli run loop can collect once, run the +/// `--fail-on-uncovered` gate on the raw `Finding`s +/// ([`has_total_uncovered`](crate::domain::has_total_uncovered) operates on +/// `&[Finding]`, not the anchor-wrapped envelope entries), and build the +/// envelope from the same `Vec` without a second collection pass. +#[must_use] +pub fn envelope_from_findings( + findings: Vec>, + cute_dbt_version: impl Into, + generated_at: impl Into, + scope: EnvelopeScope, +) -> FindingsEnvelope { let metadata = EnvelopeMetadata::new(cute_dbt_version, generated_at, scope); FindingsEnvelope::new(metadata, findings) } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 9c225ee5..4cd59696 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -80,7 +80,9 @@ use std::process::ExitCode; use clap::Parser; use crate::adapters::explore::render_explore; -use crate::adapters::findings_emit::{build_findings_envelope, write_sidecar}; +use crate::adapters::findings_emit::{ + collect_in_scope_findings, envelope_from_findings, write_sidecar, +}; use crate::adapters::manifest::{FileManifestSource, load_baseline}; use crate::adapters::project_def::parse as parse_project_definition; use crate::adapters::project_file::FsProjectFileReader; @@ -541,23 +543,29 @@ fn finalize_findings( if args.findings_out.is_none() && !args.fail_on_uncovered { return Ok(ReportOutcome::Success); } + // Collect the in-scope findings once. The gate reads the raw `Finding`s + // (`has_total_uncovered` operates on `&[Finding]`); the envelope wraps + // the SAME vec into anchor-bearing entries (cute-dbt#386). One pass. + let findings = collect_in_scope_findings(current, models_in_scope, check_policy); // `generated_at` is the I/O-boundary date — the `--generated-at` // override (golden regeneration / reproducible builds) over today's // computed civil date. Threaded into the pure envelope builder so the // committed golden is byte-stable. let generated_at = args.generated_at.clone().unwrap_or_else(today_rfc3339_date); - let envelope = build_findings_envelope( - current, - models_in_scope, - check_policy, - env!("CARGO_PKG_VERSION"), - generated_at, - envelope_scope(args, scope_input), - ); + // The gate decision is read off the raw findings BEFORE the sidecar is + // written, but a sidecar write failure still wins (returns below via `?` + // before the gate outcome is returned) — the write-failure-over-gate + // precedence the run_loop tests pin. + let gate_tripped = args.fail_on_uncovered && has_total_uncovered(&findings); if let Some(path) = &args.findings_out { + let envelope = envelope_from_findings( + findings, + env!("CARGO_PKG_VERSION"), + generated_at, + envelope_scope(args, scope_input), + ); write_sidecar(&envelope, path).map_err(|err| RunError::output(path, err))?; } - let gate_tripped = args.fail_on_uncovered && has_total_uncovered(&envelope.findings); Ok(if gate_tripped { ReportOutcome::UncoveredGate } else { diff --git a/src/domain/findings_envelope.rs b/src/domain/findings_envelope.rs index c93be3bf..3a58ac8e 100644 --- a/src/domain/findings_envelope.rs +++ b/src/domain/findings_envelope.rs @@ -130,27 +130,114 @@ impl EnvelopeMetadata { } } +/// Which side of a diff a finding's anchor sits on (cute-dbt#386 — the +/// reserved finding→line projection slot). +/// +/// Reserved for the follow-on resolver that maps a finding onto a concrete +/// `(path, line)` in a PR diff; this slice emits **no** value (every +/// envelope `anchor` is `None`). serde `snake_case` (`added` / `removed` / +/// `modified`) so a future projection (GitHub annotations, #353 PR +/// comments, SARIF) keys on a stable wire token. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum DiffContext { + /// The anchored line is a `+` addition. + Added, + /// The anchored line is a `-` removal. + Removed, + /// The anchored construct spans a modified region. + Modified, +} + +/// The reserved **finding→line projection anchor** (cute-dbt#386). +/// +/// The envelope is the canonical agent channel, and downstream projections +/// — GitHub annotations, #353 PR comments, SARIF — all need a finding's +/// concrete `(path, line)`. This POD **reserves that slot now**; a follow-on +/// resolver populates it. In this slice every field is `None`/reserved — no +/// resolver logic, no value emitted — so an `EnvelopeFinding` with +/// `anchor: None` serializes byte-identically to a bare [`Finding`] (the +/// committed envelope golden is unchanged). Pure data, no methods beyond the +/// constructor. +#[derive(Debug, Clone, PartialEq, Eq, Default, Serialize)] +pub struct FindingAnchor { + /// The project-relative source path the finding anchors to. Omitted from + /// JSON when absent. + #[serde(skip_serializing_if = "Option::is_none")] + pub path: Option, + /// The 1-based line within `path`. Omitted from JSON when absent. + #[serde(skip_serializing_if = "Option::is_none")] + pub line: Option, + /// Which diff side the line sits on (PR-diff projections). Omitted from + /// JSON when absent. + #[serde(skip_serializing_if = "Option::is_none")] + pub diff_context: Option, + /// A content-addressed hash stabilizing the anchor across reformatting + /// (the resolver's drift guard). Omitted from JSON when absent. + #[serde(skip_serializing_if = "Option::is_none")] + pub anchor_hash: Option, +} + +/// One envelope finding: the existing [`Finding`] wire shape plus the +/// reserved [`FindingAnchor`] slot (cute-dbt#386). +/// +/// The `Finding` is `#[serde(flatten)]`ed, so its keys sit directly on the +/// finding object exactly as before; `anchor` is `skip_serializing_if = +/// "Option::is_none"`, so an unresolved anchor (every anchor in this slice) +/// emits **zero** added bytes — the committed envelope golden stays +/// byte-identical. A populated anchor adds a single nested `anchor` object. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct EnvelopeFinding { + /// The finding itself — flattened so its keys (`check` / `tier` / + /// `model_id` / `verdict` / …) sit on this object directly. + #[serde(flatten)] + pub finding: Finding, + /// The reserved finding→line projection anchor. `None` (omitted) in this + /// slice; populated by a follow-on resolver. + #[serde(skip_serializing_if = "Option::is_none")] + pub anchor: Option, +} + +impl EnvelopeFinding { + /// Wrap a [`Finding`] with no anchor (the v0.1 emit — the slot is + /// reserved but unpopulated). + #[must_use] + pub fn new(finding: Finding) -> Self { + Self { + finding, + anchor: None, + } + } +} + /// The findings envelope: a `metadata` header + a flat `findings` list. /// -/// Each [`Finding`] already carries `model_id` / `check` / `tier` / -/// `verdict` / `evidence` / `recommendation` / `degraded` / `suppressed`, -/// so consumers group by `model_id` / `check` / `tier` themselves — the -/// envelope adds no grouping, only the versioned header. +/// Each [`EnvelopeFinding`] flattens the existing [`Finding`] (which already +/// carries `model_id` / `check` / `tier` / `verdict` / `evidence` / +/// `recommendation` / `degraded` / `suppressed`) and adds the reserved +/// `anchor` slot — so consumers group by `model_id` / `check` / `tier` +/// themselves; the envelope adds the versioned header and the (currently +/// unpopulated) anchor slot, nothing else. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] pub struct FindingsEnvelope { /// The versioned header (decision D2 stability anchor lives here). pub metadata: EnvelopeMetadata, /// The flat in-scope finding set, in the order the run loop collected /// it (deterministic over the in-scope model set + each model's - /// `model_findings` output). - pub findings: Vec>, + /// `model_findings` output). Each entry flattens a [`Finding`] + the + /// reserved anchor slot. + pub findings: Vec, } impl FindingsEnvelope { - /// Assemble an envelope from its already-computed parts. + /// Assemble an envelope from a header + the already-computed findings, + /// wrapping each [`Finding`] in an anchor-less [`EnvelopeFinding`]. #[must_use] pub fn new(metadata: EnvelopeMetadata, findings: Vec>) -> Self { - Self { metadata, findings } + Self { + metadata, + findings: findings.into_iter().map(EnvelopeFinding::new).collect(), + } } } @@ -237,10 +324,18 @@ mod tests { assert_eq!(json["metadata"]["scope"]["baseline"], "baseline.json"); assert!(json["findings"].is_array()); assert_eq!(json["findings"].as_array().unwrap().len(), 1); - // The wrapped Finding keeps its existing serialized shape. + // The wrapped Finding keeps its existing serialized shape — the + // anchor wrapper flattens transparently. assert_eq!(json["findings"][0]["model_id"], "model.shop.dim_x"); assert_eq!(json["findings"][0]["tier"], "total"); assert_eq!(json["findings"][0]["verdict"]["status"], "uncovered"); + // cute-dbt#386 byte-safety: an unpopulated anchor emits NO `anchor` + // key, so the envelope stays byte-identical to the pre-anchor shape. + assert!( + json["findings"][0].get("anchor").is_none(), + "an unresolved anchor must be omitted entirely: {}", + json["findings"][0] + ); } #[test] @@ -340,4 +435,88 @@ mod tests { ]; assert!(has_total_uncovered(&findings)); } + + // ---- reserved anchor slot (cute-dbt#386) ------------------------ + + #[test] + fn an_anchor_less_envelope_finding_is_byte_identical_to_a_bare_finding() { + // The byte-safety invariant: flattening the Finding + omitting the + // None anchor reproduces the bare Finding's wire shape exactly, so + // the committed envelope golden is unchanged by the anchor slot. + let bare = finding(Verdict::Uncovered); + let wrapped = EnvelopeFinding::new(bare.clone()); + let bare_json = serde_json::to_string(&bare).expect("serializes"); + let wrapped_json = serde_json::to_string(&wrapped).expect("serializes"); + assert_eq!( + bare_json, wrapped_json, + "an anchor-less EnvelopeFinding must serialize byte-identically \ + to the bare Finding" + ); + // And explicitly: no `anchor` key surfaces. + let value = serde_json::to_value(&wrapped).expect("serializes"); + assert!( + value.get("anchor").is_none(), + "None anchor must be omitted: {value}" + ); + } + + #[test] + fn a_populated_anchor_round_trips_and_nests_under_anchor() { + let mut wrapped = EnvelopeFinding::new(finding(Verdict::Uncovered)); + wrapped.anchor = Some(FindingAnchor { + path: Some("models/marts/dim_x.sql".to_owned()), + line: Some(42), + diff_context: Some(DiffContext::Added), + anchor_hash: Some("abc123".to_owned()), + }); + let value = serde_json::to_value(&wrapped).expect("serializes"); + // The finding keys still flatten onto the entry; the anchor nests. + assert_eq!(value["model_id"], "model.shop.dim_x"); + assert_eq!(value["anchor"]["path"], "models/marts/dim_x.sql"); + assert_eq!(value["anchor"]["line"], 42); + assert_eq!(value["anchor"]["diff_context"], "added"); + assert_eq!(value["anchor"]["anchor_hash"], "abc123"); + } + + #[test] + fn a_default_anchor_omits_every_unset_field() { + // The resolver may populate fields incrementally; an all-None anchor + // (the Default) serializes to an empty object — every field is + // skip_serializing_if None. + let value = serde_json::to_value(FindingAnchor::default()).expect("serializes"); + assert_eq!( + value, + serde_json::json!({}), + "empty anchor is {{}}: {value}" + ); + } + + #[test] + fn diff_context_serializes_snake_case() { + assert_eq!( + serde_json::to_value(DiffContext::Modified).expect("serializes"), + serde_json::json!("modified") + ); + assert_eq!( + serde_json::to_value(DiffContext::Removed).expect("serializes"), + serde_json::json!("removed") + ); + } + + #[test] + fn envelope_new_wraps_each_finding_with_a_none_anchor() { + let envelope = FindingsEnvelope::new( + EnvelopeMetadata::new( + "0.1.0", + "2026-01-15", + EnvelopeScope::PrDiff { source: None }, + ), + vec![finding(Verdict::Uncovered), finding(Verdict::Unknown)], + ); + assert_eq!(envelope.findings.len(), 2); + assert!( + envelope.findings.iter().all(|f| f.anchor.is_none()), + "every wrapped finding starts anchor-less" + ); + } } diff --git a/src/domain/mod.rs b/src/domain/mod.rs index b8bc451e..d386a211 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -85,8 +85,8 @@ pub use experimental::{ parse_experimental_env, resolve_experimental_config, }; pub use findings_envelope::{ - EnvelopeMetadata, EnvelopeScope, FindingsEnvelope, ID_STABILITY, SCHEMA_VERSION, - has_total_uncovered, + DiffContext, EnvelopeFinding, EnvelopeMetadata, EnvelopeScope, FindingAnchor, FindingsEnvelope, + ID_STABILITY, SCHEMA_VERSION, has_total_uncovered, }; pub use governance::{ BlastRadius, BreakingReason, ColumnMetaTags, ConstraintSupport, ContractChange, ContractClass, diff --git a/tests/run_loop.rs b/tests/run_loop.rs index 37c6f27b..67298d9c 100644 --- a/tests/run_loop.rs +++ b/tests/run_loop.rs @@ -746,3 +746,61 @@ fn default_path_writes_no_envelope_sidecar() { "no sidecar is written without --findings-out" ); } + +#[test] +fn an_unwritable_findings_out_path_is_reported() { + // The --findings-out twin of `an_unwritable_output_path_is_reported`: + // a sidecar path under a directory that does not exist makes + // `write_sidecar` fail, so the run loop reports the write error (exit 1) + // instead of swallowing it. Without this test a future `let _ = + // write_sidecar(...)` would silently regress the surfaced-error + // contract. The HTML --out path IS writable here, so the failure is + // specifically the sidecar's. + let html = tmp("envelope_unwritable.html"); + let sidecar = tmp("no_such_dir/findings.json"); + clear(&html); + let mut args = playground_pr_diff_args(&html); + args.push("--findings-out".to_owned()); + args.push(s(&sidecar).to_owned()); + let refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let output = run(&refs); + assert_eq!( + output.status.code(), + Some(1), + "an unwritable findings-out path exits 1: {output:?}" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("could not write"), + "stderr reports the sidecar-write failure: {stderr}" + ); +} + +#[test] +fn an_unwritable_findings_out_path_wins_over_the_uncovered_gate() { + // Write-failure precedence: the playground pr-diff carries a Total-tier + // uncovered finding (so --fail-on-uncovered would otherwise exit 3), but + // the sidecar write fails FIRST (finalize_findings writes the sidecar + // before evaluating the gate), so the run exits 1 (the surfaced write + // error) — never the gate code. A swallowed sidecar error would let the + // gate's exit 3 leak through and mask the real I/O failure. + let html = tmp("gate_vs_unwritable.html"); + let sidecar = tmp("no_such_dir/gate_findings.json"); + clear(&html); + let mut args = playground_pr_diff_args(&html); + args.push("--findings-out".to_owned()); + args.push(s(&sidecar).to_owned()); + args.push("--fail-on-uncovered".to_owned()); + let refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let output = run(&refs); + assert_eq!( + output.status.code(), + Some(1), + "the sidecar write failure (1) wins over the gate code (3): {output:?}" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("could not write"), + "stderr reports the write failure, not a silent gate trip: {stderr}" + ); +} From 7b2a72c194b5e3f4f30f894d04525bfeb13a665f Mon Sep 17 00:00:00 2001 From: Christopher Bays Date: Sat, 13 Jun 2026 23:24:04 -0400 Subject: [PATCH 4/4] fix(cli): reject --findings-out == --out and validate --generated-at at parse time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit hardening on PR #388 (cute-dbt#386): - `--findings-out` resolving to the same path as `--out` now fails as a clap-level usage error (exit 2, ArgumentConflict) BEFORE any artifact is written — the sidecar JSON would otherwise clobber the just-rendered HTML report on an otherwise successful run. Raised in `validate_argument_conflicts` (post-parse, since clap's derive cannot compare two PathBuf args), wired into `run()` through the same exit-2 path as a parse failure. Syntactic comparison (as-typed paths, no filesystem resolve) — the `--config` / baseline-missing precedent: a flag conflict is a usage error, never a fifth PreflightError variant. - `--generated-at` now carries a `value_parser` enforcing the documented RFC3339 `YYYY-MM-DD` `full-date` contract (exact NNNN-NN-NN shape, month 1-12, day 1-31). A malformed override is rejected at parse time rather than flowing into the emitted envelope JSON — fail fast on the one golden- regeneration override path. TDD: 8 args unit tests (4 generated-at reject + 2 accept, 3 conflict cases) plus a run_loop integration test asserting exit 2 with no artifact written. examples/ stays byte-identical (parse-time-only changes; no render path touched). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/cli/args.rs | 246 +++++++++++++++++++++++++++++++++++++++++++++- src/cli/mod.rs | 9 +- tests/run_loop.rs | 29 ++++++ 3 files changed, 282 insertions(+), 2 deletions(-) diff --git a/src/cli/args.rs b/src/cli/args.rs index 14854042..f445a147 100644 --- a/src/cli/args.rs +++ b/src/cli/args.rs @@ -304,7 +304,12 @@ pub struct ReportArgs { /// injection point). Hidden — it exists only for golden regeneration / /// reproducible builds, never a normal user surface. Inert unless /// `--findings-out` is also set. - #[arg(long, value_name = "YYYY-MM-DD", hide = true)] + /// + /// Validated at parse time ([`parse_generated_at`]): a malformed + /// value is a clap usage error (exit 2), never an envelope that + /// violates the documented `YYYY-MM-DD` contract — fail fast before + /// the run starts (cute-dbt#386, `CodeRabbit` on PR #388). + #[arg(long, value_name = "YYYY-MM-DD", hide = true, value_parser = parse_generated_at)] pub generated_at: Option, } @@ -436,6 +441,92 @@ fn parse_project_root(s: &str) -> Result { Ok(p) } +/// Post-parse validation clap's derive cannot express: reject a +/// `report --findings-out` that resolves to the same path as `--out` +/// (cute-dbt#386, `CodeRabbit` on PR #388). +/// +/// The findings sidecar is written *after* the HTML report, so +/// `--out report.html --findings-out report.html` would silently clobber +/// the just-rendered HTML with the envelope JSON on an otherwise +/// successful run — destroying the primary artifact and breaking the +/// additive-sidecar contract. This is a usage error, not a runtime +/// `PreflightError` (the enum stays at four variants — the +/// `--config` / baseline-missing precedent: a conflict between two flags +/// is a clap-level [`ArgGroup`]-style usage error, exit 2). It is raised +/// here rather than in the derive because clap cannot compare two +/// `PathBuf` arguments for equality. +/// +/// The comparison is **syntactic** (the as-typed [`PathBuf`]s), not a +/// canonicalized filesystem resolve: it catches the literal footgun +/// without an I/O round-trip (neither path is required to exist yet — the +/// same write-time `--out` contract). Only the `report` verb carries the +/// two flags; every other verb is a no-op here. +/// +/// # Errors +/// +/// Returns a [`clap::error::ErrorKind::ArgumentConflict`] error (exit 2) +/// when `report`'s `--findings-out` equals its `--out`. +pub fn validate_argument_conflicts(cli: &Cli) -> Result<(), clap::Error> { + use clap::CommandFactory; + if let Command::Report(report) = &cli.command + && report.findings_out.as_ref() == Some(&report.out) + { + return Err(Cli::command().error( + clap::error::ErrorKind::ArgumentConflict, + format!( + "--findings-out must differ from --out (both resolve to {}); \ + the sidecar JSON would overwrite the HTML report", + report.out.display() + ), + )); + } + Ok(()) +} + +/// clap value-parser: validate the `--generated-at` override against the +/// envelope's documented `YYYY-MM-DD` (RFC3339 `full-date`) contract +/// (cute-dbt#386, `CodeRabbit` on PR #388). +/// +/// `--generated-at` is the one override path for the findings-envelope +/// `metadata.generated_at` (golden regeneration / reproducible builds), so +/// an unvalidated string would flow straight into the emitted JSON and +/// produce an envelope that violates its own documented date shape. This +/// fails fast at parse time (a clap usage error, exit 2 — the +/// `--config` / baseline-missing precedent) rather than letting a malformed +/// date reach the I/O boundary. Strict shape: exactly `NNNN-NN-NN`, all +/// ASCII digits, with month in `[1, 12]` and day in `[1, 31]` (a calendar +/// upper bound — the envelope contract is the textual `full-date` shape, +/// not a per-month length check). Returns the validated string unchanged. +fn parse_generated_at(s: &str) -> Result { + let bytes = s.as_bytes(); + // Exact `YYYY-MM-DD` layout: 10 chars, dashes at 4 and 7, digits + // everywhere else. + let well_shaped = bytes.len() == 10 + && bytes[4] == b'-' + && bytes[7] == b'-' + && bytes + .iter() + .enumerate() + .all(|(i, b)| i == 4 || i == 7 || b.is_ascii_digit()); + if !well_shaped { + return Err(format!( + "--generated-at must be an RFC3339 date in YYYY-MM-DD form, got {s:?}" + )); + } + // Shape guarantees these slices are valid ASCII-digit runs. + let month: u32 = s[5..7].parse().map_err(|_| invalid_generated_at(s))?; + let day: u32 = s[8..10].parse().map_err(|_| invalid_generated_at(s))?; + if !(1..=12).contains(&month) || !(1..=31).contains(&day) { + return Err(invalid_generated_at(s)); + } + Ok(s.to_owned()) +} + +/// The shared `--generated-at` rejection message (cute-dbt#386). +fn invalid_generated_at(s: &str) -> String { + format!("--generated-at must be a valid YYYY-MM-DD date, got {s:?}") +} + /// Resolve the effective dbt project root. /// /// Resolution policy: @@ -1538,4 +1629,157 @@ tilte = "typo'd" .expect_err("explore takes no experimental flag"); assert_eq!(err.kind(), ErrorKind::UnknownArgument); } + + // ----- cute-dbt#386 — CodeRabbit PR #388 hardening ----- + + /// A valid `report` arg set carrying `--generated-at`. + fn report_with_generated_at(value: &str) -> Result { + parse_report(&[ + "cute-dbt", + "report", + "--manifest", + "m.json", + "--baseline-manifest", + "b.json", + "--out", + "o.html", + "--findings-out", + "f.json", + "--generated-at", + value, + ]) + } + + #[test] + fn generated_at_accepts_a_well_formed_date() { + let report = report_with_generated_at("2026-06-13").expect("a YYYY-MM-DD date parses"); + assert_eq!(report.generated_at.as_deref(), Some("2026-06-13")); + } + + #[test] + fn generated_at_accepts_a_far_future_golden_date() { + // The golden fixtures pin a far-future date so the byte-identity + // gate never drifts with wall-clock — that value must still parse. + let report = report_with_generated_at("2999-12-31").expect("a far-future date parses"); + assert_eq!(report.generated_at.as_deref(), Some("2999-12-31")); + } + + #[test] + fn generated_at_rejects_a_non_date_string() { + // The CodeRabbit footgun: `--generated-at not-a-date` must fail + // fast at parse time, never flow into the envelope JSON. + let err = report_with_generated_at("not-a-date").expect_err("a non-date is rejected"); + assert_eq!(err.kind(), ErrorKind::ValueValidation); + assert!(err.use_stderr(), "a bad --generated-at is a usage error"); + assert!( + err.to_string().contains("YYYY-MM-DD"), + "the error names the expected shape: {err}" + ); + } + + #[test] + fn generated_at_rejects_a_datetime() { + // A full RFC3339 date-time exceeds the documented `full-date` + // contract; the envelope's `generated_at` is a date only. + let err = + report_with_generated_at("2026-06-13T00:00:00Z").expect_err("a date-time is rejected"); + assert_eq!(err.kind(), ErrorKind::ValueValidation); + } + + #[test] + fn generated_at_rejects_an_out_of_range_month() { + let err = report_with_generated_at("2026-13-01").expect_err("month 13 is rejected"); + assert_eq!(err.kind(), ErrorKind::ValueValidation); + } + + #[test] + fn generated_at_rejects_an_out_of_range_day() { + let err = report_with_generated_at("2026-06-00").expect_err("day 00 is rejected"); + assert_eq!(err.kind(), ErrorKind::ValueValidation); + } + + #[test] + fn generated_at_rejects_a_non_digit_in_the_year() { + let err = report_with_generated_at("20x6-06-13").expect_err("a non-digit year is rejected"); + assert_eq!(err.kind(), ErrorKind::ValueValidation); + } + + #[test] + fn findings_out_equal_to_out_is_an_argument_conflict() { + // The CodeRabbit footgun: the sidecar JSON would clobber the HTML + // report. Rejected as a usage error (exit 2) BEFORE the run starts. + let cli = parse(&[ + "cute-dbt", + "report", + "--manifest", + "m.json", + "--baseline-manifest", + "b.json", + "--out", + "report.html", + "--findings-out", + "report.html", + ]) + .expect("the args themselves parse — the collision is a post-parse check"); + let err = validate_argument_conflicts(&cli) + .expect_err("--findings-out == --out is a usage conflict"); + assert_eq!(err.kind(), ErrorKind::ArgumentConflict); + assert!(err.use_stderr(), "the path collision is a usage error"); + let msg = err.to_string(); + assert!( + msg.contains("--findings-out") && msg.contains("--out"), + "the error names both flags: {msg}" + ); + } + + #[test] + fn distinct_findings_out_and_out_pass_validation() { + let cli = parse(&[ + "cute-dbt", + "report", + "--manifest", + "m.json", + "--baseline-manifest", + "b.json", + "--out", + "report.html", + "--findings-out", + "findings.json", + ]) + .expect("distinct output paths parse"); + validate_argument_conflicts(&cli).expect("distinct paths are not a conflict"); + } + + #[test] + fn findings_out_absent_passes_validation() { + // The default path (no sidecar) can never collide. + let cli = parse(&[ + "cute-dbt", + "report", + "--manifest", + "m.json", + "--baseline-manifest", + "b.json", + "--out", + "report.html", + ]) + .expect("a report without --findings-out parses"); + validate_argument_conflicts(&cli).expect("no sidecar ⇒ no possible collision"); + } + + #[test] + fn argument_conflict_validation_is_a_no_op_for_explore() { + // Only `report` carries the two flags; the validation must not + // misfire on any other verb. + let cli = parse(&[ + "cute-dbt", + "explore", + "--manifest", + "m.json", + "--out-dir", + "d", + ]) + .expect("an explore invocation parses"); + validate_argument_conflicts(&cli).expect("explore has no --findings-out/--out collision"); + } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index d68dee57..f260c810 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -109,7 +109,7 @@ use crate::domain::{ }; use crate::ports::{ManifestSource, ProjectFileReader}; -use args::{Cli, Command, ExploreArgs, ReportArgs}; +use args::{Cli, Command, ExploreArgs, ReportArgs, validate_argument_conflicts}; /// Exit code for a run-time failure: a fail-closed manifest (Stage-1 or /// Stage-2) or an unwritable `--out` path. @@ -134,6 +134,13 @@ pub fn run() -> ExitCode { Ok(cli) => cli, Err(err) => return report_arg_error(&err), }; + // Post-parse usage validation clap's derive cannot express + // (cute-dbt#386): `report --findings-out` must differ from `--out`, + // or the sidecar JSON would clobber the HTML report. Routed through + // the same exit-2 usage-error path as a parse failure. + if let Err(err) = validate_argument_conflicts(&cli) { + return report_arg_error(&err); + } // Per-verb dispatch; every run-time failure is mapped to one // stderr message + exit 1 here. `review` wraps its own cli-layer // `ReviewError` (cute-dbt#300) alongside the composed report's diff --git a/tests/run_loop.rs b/tests/run_loop.rs index 67298d9c..0bcebe6d 100644 --- a/tests/run_loop.rs +++ b/tests/run_loop.rs @@ -776,6 +776,35 @@ fn an_unwritable_findings_out_path_is_reported() { ); } +#[test] +fn findings_out_equal_to_out_is_a_usage_error_before_any_write() { + // CodeRabbit on PR #388: `--out X --findings-out X` would clobber the + // just-rendered HTML with the envelope JSON. Rejected as a usage error + // (exit 2 — the parse-time class, NOT the fail-closed code 1) BEFORE + // anything is written, so the report is never produced-then-destroyed. + let collide = tmp("collision_report.html"); + clear(&collide); + let mut args = playground_pr_diff_args(&collide); + args.push("--findings-out".to_owned()); + args.push(s(&collide).to_owned()); + let refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let output = run(&refs); + assert_eq!( + output.status.code(), + Some(2), + "a --findings-out == --out collision is a usage error: {output:?}" + ); + assert!( + !collide.exists(), + "the collision is rejected before any artifact is written" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("--findings-out") && stderr.contains("--out"), + "stderr names the conflicting flags: {stderr}" + ); +} + #[test] fn an_unwritable_findings_out_path_wins_over_the_uncovered_gate() { // Write-failure precedence: the playground pr-diff carries a Total-tier