Skip to content

#386 — feat: findings envelope core — --findings-out sidecar + --fail-on-uncovered gate + row-5-reversal ADR - #388

Merged
cmbays merged 5 commits into
mainfrom
cli-386-findings-envelope
Jun 14, 2026
Merged

cmbays merged 5 commits into
mainfrom
cli-386-findings-envelope

Conversation

@cmbays

@cmbays cmbays commented Jun 14, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Builds the machine-readable findings envelope core (epic #261). Wraps the already-Serialize-deriving Finding POD in a versioned metadata.schema_version header and emits it as a 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.

This is an ADR-gated conscious-simplification reversal — HELD for founder-mediated review. DO NOT MERGE until gated.

The four locked decisions (#261), honored

  • D1 — sidecar. New --findings-out <path.json> emits { metadata, findings } alongside the HTML report in one run. NOT a --format json swap — the HTML output and its byte-identity goldens are untouched.
  • D2 — check-id instability. metadata.schema_version is an integer (1), the only pre-v1.0 stability anchor; metadata.id_stability: "unstable-v0.x" says so machine-readably. The ADR/AGENTS narrative tells consumers to pin schema_version, never individual check-ids, until v1.0.
  • D3 — gate. --fail-on-uncovered exits a dedicated ExitCode (3, distinct from usage/2 and fail-closed/1) iff ≥1 Tier::Total + Verdict::Uncovered finding is in scope. Not configurable (Total-only tenet). The report (and any sidecar) is written before the gate trips.
  • D4 — OpenLineage. Fields shaped design-compatible; no OL output emitted.

severity = the existing Tier enum (no new field). SARIF = not now. Owner fields (#256) = slot reserved, not populated.

What landed

Layer File Role
domain (pure) src/domain/findings_envelope.rs FindingsEnvelope / EnvelopeMetadata / EnvelopeScope PODs + has_total_uncovered gate predicate. std + serde only.
adapter src/adapters/findings_emit.rs collect_in_scope_findings mirrors the renderer's per-model model_findings → apply_check_policy pipeline EXACTLY (parses each model's CTE graph); serialize + write_sidecar. Never touches report.html/render.rs.
cli src/cli/{args,mod,review}.rs --findings-out, --fail-on-uncovered, hidden --generated-at (golden override); EXIT_GATE=3; run-loop wiring.
golden examples/diff-showcase-findings.json synthetic playground pr-diff combo, pinned --generated-at 2099-01-01; byte-identity-gated in the diff-showcase example-report-check row.
docs ARCHITECTURE.md row-5, AGENTS.md the row-5 reversal narrative.

Determinism

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 threaded into the pure builder. Emitted as an RFC3339 date (YYYY-MM-DD): a finer-grained timestamp would need a date-time crate the std-only posture forbids, and the date is the deterministic granularity the golden gate needs. The hidden --generated-at flag pins it for the committed golden.

HTML goldens unchanged

The envelope is purely additive. git status examples/ shows ONLY the new diff-showcase-findings.json — every *.html golden is byte-identical. Verified by the headless zero-egress suite (12 pass), resource-ref lint (17 pass), and the golden-report skeleton test.

Gates (all run directly, by exit code)

  • cargo fmt --check — 0
  • cargo clippy --all-targets --locked -- -D warnings — 0
  • cargo nextest run — 2074 passed, 113 skipped
  • cargo test --test bdd — 226 scenarios passed (no .feature added — the exit-code contract is an integration test, not a product-level BDD scenario)
  • cargo test --test headless_zero_egress -- --ignored — 12 passed
  • cargo test --test resource_ref_lint — 17 passed
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --locked — 0
  • cargo deny check — 0
  • domain hexagonal-purity + non-mirror-guard — pass (the new domain module imports only std/serde/intra-domain; no [workspace], no bans.deny.wrappers, no pub use crate::)

Closes #386

🤖 Generated with Claude Code


Open in Stage

Summary by CodeRabbit

  • New Features

    • Added a machine-readable “findings envelope” JSON sidecar via --findings-out, emitted alongside the existing HTML report.
    • Added --fail-on-uncovered to gate runs when in-scope Total uncovered findings exist (exits with code 3).
    • Added --generated-at (hidden) to pin deterministic generated_at when generating the sidecar.
  • Tests / CI

    • Added integration coverage for sidecar output, gating behavior, and sidecar write-failure precedence.
    • Updated CI to validate the example’s machine-readable findings envelope deterministically.
  • Documentation

    • Updated architecture/discipline docs to reflect the supported findings-envelope behavior and stability/golden expectations.

@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 14, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@cmbays, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 31 minutes and 4 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ac2e57a7-3a60-40e4-bd63-9e3bf00e4cdf

📥 Commits

Reviewing files that changed from the base of the PR and between 34a3586 and 7b2a72c.

📒 Files selected for processing (3)
  • src/cli/args.rs
  • src/cli/mod.rs
  • tests/run_loop.rs
📝 Walkthrough

Walkthrough

Implements the findings-envelope sidecar feature: a new FindingsEnvelope domain model with versioned metadata wraps Finding records; a new findings_emit adapter collects and serializes findings; three new CLI flags (--findings-out, --fail-on-uncovered, --generated-at) extend cute-dbt report; a post-render tail stage writes the JSON sidecar and gates on Total-tier uncovered findings (exit code 3). A committed golden fixture, integration tests, CI byte-identity checks, and updated architecture documentation document the conscious reversal of the "no JSON wire envelope" design simplification.

Changes

Findings Envelope Sidecar Feature

Layer / File(s) Summary
FindingsEnvelope domain model: metadata, scope, envelope, gate predicate
src/domain/findings_envelope.rs, src/domain/mod.rs
Defines SCHEMA_VERSION/ID_STABILITY constants, EnvelopeScope enum (baseline/pr-diff with serde tagging and field omission), EnvelopeMetadata struct and constructor, DiffContext enum for diff-side context, FindingAnchor struct with optional fields, EnvelopeFinding that flattens Finding with optional anchor nesting, FindingsEnvelope struct, has_total_uncovered gate predicate, comprehensive serde/round-trip/anchor unit tests, and re-exports from domain root.
findings_emit adapter: collect, build, serialize, write sidecar
src/adapters/findings_emit.rs, src/adapters/mod.rs
New outward adapter mirrors the HTML renderer's model_findings→apply_check_policy pipeline per model; build_findings_envelope wraps findings into FindingsEnvelope; envelope_from_findings builds envelope from pre-collected findings; envelope_to_json serializes to pretty JSON with trailing newline for golden byte-stability; write_sidecar persists to disk. Unit tests cover collection mirroring, metadata/findings field correctness, JSON formatting with newline, structural round-trips, and disk write-back.
CLI args, ReportOutcome, finalize_findings tail stage, date helper
src/cli/args.rs, src/cli/mod.rs, src/cli/review.rs
Extends ReportArgs with --findings-out (optional path), --fail-on-uncovered (boolean gate), and hidden --generated-at (deterministic override). Adds EXIT_GATE=3 constant and ReportOutcome enum. Refactors execute_report to return ReportOutcome. Adds finalize_findings post-render tail stage that collects findings, computes generated_at at I/O boundary, optionally writes sidecar, and evaluates uncovered gate. Adds envelope_scope helper and today_rfc3339_date() helper. Wires review to disable findings controls and updates test scaffolding.
Golden fixture and integration tests
examples/diff-showcase-findings.json, tests/run_loop.rs
Adds 678-line byte-identity golden for diff-showcase (metadata header with schema/version/scope/id-stability + 30+ findings entries). Adds helper and five integration tests covering: sidecar content correctness, --fail-on-uncovered exit-code-3 gate trip, success when no Total-tier uncovered, no sidecar without --findings-out, unwritable path error handling, and error-precedence (write failure over gate trip).
Architecture docs, AGENTS/README/CONTRIBUTING updates, CI golden gate
ARCHITECTURE.md, AGENTS.md, CONTRIBUTING.md, README.md, .github/workflows/ci.yml
Updates ARCHITECTURE.md row-5 reversal subsection (sidecar delivery, id-instability notice, fail-gating rules, generated-at determinism policy, OpenLineage deferral). Updates AGENTS.md to carve out sanctioned envelope exception. Updates CONTRIBUTING.md/README.md to reflect conscious reversal. Extends CI example-report-check matrix with findings_out/generated_at fields for diff-showcase; exports to environment; updates report arm to conditionally pass --findings-out/--generated-at and perform byte-identity diff against golden.

Sequence Diagram(s)

sequenceDiagram
  actor User
  participant CLI as cute-dbt report
  participant RunLoop as execute_report
  participant RenderAdapter as render adapter
  participant EmitAdapter as findings_emit
  participant Domain as model_findings + apply_check_policy
  participant Disk as filesystem

  User->>CLI: --findings-out path.json [--fail-on-uncovered]
  CLI->>RunLoop: ReportArgs
  RunLoop->>RenderAdapter: render HTML report
  RenderAdapter->>Disk: write report.html
  RenderAdapter-->>RunLoop: ok
  RunLoop->>EmitAdapter: collect_in_scope_findings(manifest, models_in_scope, policy)
  loop per model
    EmitAdapter->>Domain: model_findings + apply_check_policy
    Domain-->>EmitAdapter: Vec<Finding>
  end
  EmitAdapter-->>RunLoop: Vec<Finding>
  RunLoop->>EmitAdapter: build_findings_envelope(findings, version, generated_at, scope)
  EmitAdapter-->>RunLoop: FindingsEnvelope
  alt --findings-out set
    RunLoop->>EmitAdapter: write_sidecar(envelope, path.json)
    EmitAdapter->>Disk: write findings JSON + trailing newline
  end
  alt --fail-on-uncovered AND has_total_uncovered(findings)
    RunLoop-->>CLI: ReportOutcome::UncoveredGate
    CLI-->>User: exit code 3
  else
    RunLoop-->>CLI: ReportOutcome::Success
    CLI-->>User: exit code 0
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • #393: Direct follow-up that defers implementation of the finding→line anchor resolver for GitHub workflow annotations; explicitly states it will populate the reserved anchor slot in EnvelopeFinding to share the resolver primitive with both envelope emission and PR comment projections, once this envelope-core PR merges.

Possibly related PRs

  • breezy-bays-labs/cute-dbt#63: Modifies the same .github/workflows/ci.yml example-report-check workflow to validate committed example report outputs via byte-identity diff, the same CI gate pattern this PR extends to validate the findings-envelope golden.

Poem

🐰 A rabbit wrote JSON with great care,
With metadata headers and findings laid bare.
--findings-out scribbles the sidecar with flair,
While --fail-on-uncovered guards with a stare.
Exit code 3 says "go fix what's not there!" 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main changes: implementation of a findings envelope feature with --findings-out sidecar, --fail-on-uncovered gate, and architectural row-5 reversal documentation.
Linked Issues check ✅ Passed All six acceptance criteria from issue #386 are met: FindingsEnvelope POD implemented with proper metadata structure, --findings-out flag emits JSON sidecar alongside HTML, --fail-on-uncovered gate exits code 3 for Total-tier uncovered findings, ADR + ARCHITECTURE documentation updated for row-5 reversal, envelope golden file committed and gated, HTML byte-identity preserved, and OpenLineage fields reserved but deferred.
Out of Scope Changes check ✅ Passed All changes are directly scoped to issue #386 requirements: CLI args/execution, domain findings envelope model, findings emit adapter, documentation updates, CI workflow validation, and test coverage. No unrelated changes to render logic, templates, or other systems detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cli-386-findings-envelope

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ghost

ghost commented Jun 14, 2026 •

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Jun 14, 2026 •

Copy link
Copy Markdown
Contributor

📄 Rendered report preview

All golden examples regenerated cleanly.

🟡 Golden examples

Committed to examples/ and byte-identity gated — the canonical reports contributors and consumers browse. Stable across PRs.

Report View Download
diff-showcase-report.html ▶ Open ↗ ⬇ Download
jaffle-shop-report.html ▶ Open ↗ ⬇ Download
seed-showcase-report.html ▶ Open ↗ ⬇ Download
macro-heavy-report.html ▶ Open ↗ ⬇ Download
playground-report.html ▶ Open ↗ ⬇ Download

🐶 Live dogfood preview

This PR doesn't touch dbt-project/, so there's no live dogfood preview.

🧭 Explore preview

The two-page cute-dbt explore explorer — dag.html (model lineage) + tests.html (unit-test viewer). Same golden/live split as the report.

🟡 Golden explore

The committed examples/explore/ playground golden (the full synthetic playground manifest). Byte-identity gated in Example report check. Stable across PRs.

Page View Download
explore/dag.html ▶ Open ↗ ⬇ Download
explore/tests.html ▶ Open ↗ ⬇ Download

🐶 Live explore

This PR doesn't touch dbt-project/, so there's no live explore preview.

▶ Open ↗ opens the report or explorer in your browser in one
click — published to this repo's GitHub Pages under
/pr-388/.
⬇ Download fetches the same self-contained HTML as a workflow
artifact (auth-gated; works fully offline). Either way the report
makes zero external resource requests.

The Pages preview may take ~1 min to update after this comment
posts. On PRs from forks the Open link is unavailable (read-only
token) — use Download.

Alternative: GitHub CLI
# gh CLI >= 2.63 extracts into ./report-preview-playground/.
gh run download 27487048429 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.html

Posted by report-preview.yml for 7b2a72c194b5e3f4f30f894d04525bfeb13a665f. Affordance only — never blocks merge.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/cli/review.rs (1)

1547-1556: ⚡ Quick win

Add a direct regression pin for the review-path findings defaults.

Line 1547-Line 1556 sets critical defaults (findings_out: None, fail_on_uncovered: false, generated_at: None), but current compose-args tests don’t assert them. Please add an assertion in report_plan_matches_composed_args (or a dedicated test) to lock this contract and prevent accidental behavior drift.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/review.rs` around lines 1547 - 1556, The review-path findings
defaults (findings_out: None, fail_on_uncovered: false, and generated_at: None)
lack test coverage to prevent accidental drift. Add assertions to the
`report_plan_matches_composed_args` test function (or create a dedicated test)
that explicitly verify each of these three default values are correctly set when
composing review arguments. These assertions will lock the contract and catch
any unintended changes to these critical defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/args.rs`:
- Around line 296-308: The `generated_at` field currently accepts any string
without validation, allowing invalid formats like `--generated-at not-a-date` to
flow into the envelope JSON and violate the documented `YYYY-MM-DD` contract.
Add a custom parser function to the `generated_at` field by including a
`value_parser` attribute that validates the input matches the exact `YYYY-MM-DD`
date format at parse time, rejecting invalid dates before the run starts. The
parser should return the validated string or an appropriate error message.

In `@src/cli/mod.rs`:
- Around line 536-538: The code currently writes the sidecar file after the HTML
report, which allows `--findings-out` to overwrite `--out` if they resolve to
the same path. Before the block that calls write_sidecar (lines 536-538), add
validation to check if the `args.findings_out` path resolves to the same path as
`args.out`. If they are the same, return a RunError to reject this
configuration. This ensures the sidecar remains additive and the primary
artifact is not silently destroyed by the overwrite.

---

Nitpick comments:
In `@src/cli/review.rs`:
- Around line 1547-1556: The review-path findings defaults (findings_out: None,
fail_on_uncovered: false, and generated_at: None) lack test coverage to prevent
accidental drift. Add assertions to the `report_plan_matches_composed_args` test
function (or create a dedicated test) that explicitly verify each of these three
default values are correctly set when composing review arguments. These
assertions will lock the contract and catch any unintended changes to these
critical defaults.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 76572b24-9dc3-4fc3-a754-6ea29e4103c5

📥 Commits

Reviewing files that changed from the base of the PR and between 468d3f0 and 2b7b9b4.

📒 Files selected for processing (12)
  • .github/workflows/ci.yml
  • AGENTS.md
  • ARCHITECTURE.md
  • examples/diff-showcase-findings.json
  • src/adapters/findings_emit.rs
  • src/adapters/mod.rs
  • src/cli/args.rs
  • src/cli/mod.rs
  • src/cli/review.rs
  • src/domain/findings_envelope.rs
  • src/domain/mod.rs
  • tests/run_loop.rs

Comment thread src/cli/args.rs
Comment thread src/cli/mod.rs
cmbays and others added 3 commits June 13, 2026 22:06
…n-uncovered gate + row-5-reversal ADR

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<Finding> }; 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 <path> (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) <noreply@anthropic.com>
…ritable-sidecar test, reserved anchor slot

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<FindingAnchor>` 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) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ARCHITECTURE.md`:
- Around line 429-433: In ARCHITECTURE.md at the section explaining how
tag-opening `<` characters are escaped (around line 431), the documentation
incorrectly states that the escape is `<` when it should be the JSON Unicode
escape sequence `\u003c`. Update the documentation to correctly show that
tag-opening `<` characters are converted to the JSON escape `\u003c` to
accurately reflect the actual escaping behavior described.

In `@CONTRIBUTING.md`:
- Around line 148-156: The CONTRIBUTING.md file contains an internal
inconsistency in the simplification count: Line 148 states "four conscious
design simplifications" while Line 155 references a "sixth original
simplification." To align with the six-row architecture model where row 5 is
reversed, change "four" on Line 148 to "five" so the count accurately reflects
that there are five active simplifications plus one (the sixth) that was
consciously reversed, making the numbering consistent throughout the document.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a0a3c51d-acaf-4d36-a148-41517b304260

📥 Commits

Reviewing files that changed from the base of the PR and between 2b7b9b4 and 34a3586.

📒 Files selected for processing (14)
  • .github/workflows/ci.yml
  • AGENTS.md
  • ARCHITECTURE.md
  • CONTRIBUTING.md
  • README.md
  • examples/diff-showcase-findings.json
  • src/adapters/findings_emit.rs
  • src/adapters/mod.rs
  • src/cli/args.rs
  • src/cli/mod.rs
  • src/cli/review.rs
  • src/domain/findings_envelope.rs
  • src/domain/mod.rs
  • tests/run_loop.rs
✅ Files skipped from review due to trivial changes (2)
  • README.md
  • examples/diff-showcase-findings.json
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/domain/mod.rs
  • src/cli/args.rs
  • src/adapters/findings_emit.rs
  • AGENTS.md
  • src/cli/review.rs
  • src/adapters/mod.rs
  • .github/workflows/ci.yml
  • src/cli/mod.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ARCHITECTURE.md`:
- Around line 429-433: In ARCHITECTURE.md at the section explaining how
tag-opening `<` characters are escaped (around line 431), the documentation
incorrectly states that the escape is `<` when it should be the JSON Unicode
escape sequence `\u003c`. Update the documentation to correctly show that
tag-opening `<` characters are converted to the JSON escape `\u003c` to
accurately reflect the actual escaping behavior described.

In `@CONTRIBUTING.md`:
- Around line 148-156: The CONTRIBUTING.md file contains an internal
inconsistency in the simplification count: Line 148 states "four conscious
design simplifications" while Line 155 references a "sixth original
simplification." To align with the six-row architecture model where row 5 is
reversed, change "four" on Line 148 to "five" so the count accurately reflects
that there are five active simplifications plus one (the sixth) that was
consciously reversed, making the numbering consistent throughout the document.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a0a3c51d-acaf-4d36-a148-41517b304260

📥 Commits

Reviewing files that changed from the base of the PR and between 2b7b9b4 and 34a3586.

📒 Files selected for processing (14)
  • .github/workflows/ci.yml
  • AGENTS.md
  • ARCHITECTURE.md
  • CONTRIBUTING.md
  • README.md
  • examples/diff-showcase-findings.json
  • src/adapters/findings_emit.rs
  • src/adapters/mod.rs
  • src/cli/args.rs
  • src/cli/mod.rs
  • src/cli/review.rs
  • src/domain/findings_envelope.rs
  • src/domain/mod.rs
  • tests/run_loop.rs
✅ Files skipped from review due to trivial changes (2)
  • README.md
  • examples/diff-showcase-findings.json
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/domain/mod.rs
  • src/cli/args.rs
  • src/adapters/findings_emit.rs
  • AGENTS.md
  • src/cli/review.rs
  • src/adapters/mod.rs
  • .github/workflows/ci.yml
  • src/cli/mod.rs
🛑 Comments failed to post (2)
ARCHITECTURE.md (1)

429-433: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Correct the JSON escape token in the script-escape explanation.

Line 431 currently says tag-opening < is converted to JSON escape <; that should be \u003c. As written, the sentence contradicts the escaping claim.

Suggested doc fix
-   ASCII letter) into the JSON escape `<`, so the browser's HTML
+   ASCII letter) into the JSON escape `\u003c`, so the browser's HTML
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

   `json_for_html_script` (`src/adapters/explore.rs`). Both escapers turn
   every *tag-opening* `<` (a `<` followed by `/`, `!`, `?`, `=`, or an
   ASCII letter) into the JSON escape `\u003c`, so the browser's HTML
   script-data state machine never encounters a `</script>`, `<!--`, or
   `<tag` opener that could break out of the script block, while
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ARCHITECTURE.md` around lines 429 - 433, In ARCHITECTURE.md at the section
explaining how tag-opening `<` characters are escaped (around line 431), the
documentation incorrectly states that the escape is `<` when it should be the
JSON Unicode escape sequence `\u003c`. Update the documentation to correctly
show that tag-opening `<` characters are converted to the JSON escape `\u003c`
to accurately reflect the actual escaping behavior described.
CONTRIBUTING.md (1)

148-156: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the simplification-count wording to avoid internal inconsistency.

Line 148 says “four” simplifications, then Line 155 references a “sixth original simplification.” Please align this count with the six-row architecture model (with row 5 reversed).

Suggested doc fix
-The four **conscious design simplifications** (no workspace, no per-crate
+The five still-absent **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."
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

The five still-absent **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
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@CONTRIBUTING.md` around lines 148 - 156, The CONTRIBUTING.md file contains an
internal inconsistency in the simplification count: Line 148 states "four
conscious design simplifications" while Line 155 references a "sixth original
simplification." To align with the six-row architecture model where row 5 is
reversed, change "four" on Line 148 to "five" so the count accurately reflects
that there are five active simplifications plus one (the sixth) that was
consciously reversed, making the numbering consistent throughout the document.

…at parse time

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) <noreply@anthropic.com>
@cmbays
cmbays merged commit 9838b17 into main Jun 14, 2026
41 checks passed
@cmbays
cmbays deleted the cli-386-findings-envelope branch June 14, 2026 03:33
github-actions Bot added a commit that referenced this pull request Jun 14, 2026
@cmbays

cmbays commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

Merge debrief — #386 findings envelope core (PR #388)

Merged to main as 9838b171 (squash). Closes #386. Wraps the envelope half of epic #261.

What shipped

  • --findings-out <path> sidecar — a machine-readable FindingsEnvelope JSON projection of the findings (additive; never a format-swap of the HTML report).
  • --fail-on-uncovered gate — exit code 3 on Total-Uncovered (the row-5 reversal, ADR'd).
  • The reserved anchor slot (EnvelopeFinding{#[serde(flatten)] finding, skip-none anchor} + FindingAnchor{path,line,diff_context,anchor_hash} + DiffContext) — all-None today, populated by the cli: finding→line anchor resolver + GitHub workflow-command annotations (review-UX projection) #393 resolver next.
  • generated_at = RFC3339 date computed at the I/O boundary (Hinnant civil_from_days, std-only).

Adversarial gate (ultracode): CONCERNS (no blocker/major; the envelope-can't-lie property verified; scope.source=None ratified).

Bot disposition (the merge-blocking pair, both FIXED — not rubber-stamped):

  1. --findings-out == --out collision → now a clap exit-2 usage error raised before any artifact is written (no produce-then-clobber). Kept as a usage conflict, NOT a 5th PreflightError variant.
  2. --generated-at → parse-time RFC3339 full-date validation (value_parser), std-only.
    Both with TDD (+ a run_loop integration test asserting exit-2-with-no-file-written). All threads resolved.

Gates: 2110 nextest, 226 BDD scenarios, headless zero-egress + toggle, doc/-D, deny — all green. Goldens byte-identical (the sidecar JSON diff-showcase-findings.json is the only new artifact; report.html unchanged).

Conflict note: resolved a clean additive union with #387 (ci.yml matrix + cli/mod.rs run-loop + domain/mod.rs mod-list).

Follow-ups: (1) #393 wiring + the ResolvedAnchor→envelope-slot population (fast-follow, in flight); (2) the envelope ADR + #261 epic closeout fire once #394 lands.

cmbays added a commit that referenced this pull request Jun 14, 2026
#393)

Completes the cute-dbt#393 fast-follow now that #388 (findings envelope
core) merged: the two disjoint primitives builder-393 shipped (the
finding→line anchor resolver + the GitHub workflow-command annotation
formatter) are now wired into the CLI, and the same resolver feeds BOTH
the annotation emit and the envelope's reserved anchor slot — the one
resolver / two projections close of the #261 arc.

- `--annotations` flag on `report` (passthrough on `review`): after the
  findings compute, resolve each finding's anchor and print the
  `::warning`/`::notice`/`::error file=,line=,title=cute-dbt: <id>::<rec>`
  workflow commands to stdout (Tier→level: Advisory=notice, High=warning,
  Total=error only under --fail-on-uncovered, else warning). Honors the
  ~10/step cap + overflow notice. No gh call, no token. Inert on baseline
  mode (no diff hunks ⇒ no resolvable line). Never written into
  report.html — view-time zero-egress untouched.
- Envelope anchor population: when --findings-out is set, each finding's
  `resolve_finding_anchor` → `ResolvedAnchor`/`AnchorSide` maps into the
  #388 `FindingAnchor`/`DiffContext` slot via new `From` conversions
  (domain) + `envelope_from_findings_anchored` (adapter). An unresolved
  anchor stays None (omitted), so the golden only grows the anchors that
  actually resolved.
- Dogfood: report-preview.yml's live prdiff-preview step gains
  --annotations so cute-dbt's own PRs print inline annotations on the
  Files-changed tab; the PR-review recipe documents the flag.

Golden change: examples/diff-showcase-findings.json gains exactly one
`anchor` object (fct_provider_metrics, line 50, modified — the only
in-scope finding whose model .sql is in the playground patch). Report
HTML goldens stay byte-identical.

Closes #393

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cli: findings envelope core — FindingsEnvelope POD + --findings-out sidecar + --fail-on-uncovered gate + row-5 ADR

1 participant