Skip to content

#84 — BDD: pr_diff_scoping.feature step impls (Scenario 14 rewrite + gate updates) - #88

Merged
cmbays merged 2 commits into
mainfrom
test-84-pr-diff-scoping-bdd
May 28, 2026
Merged

cmbays merged 2 commits into
mainfrom
test-84-pr-diff-scoping-bdd

Conversation

@cmbays

@cmbays cmbays commented May 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Wires features/pr_diff_scoping.feature (14 scenarios) to step implementations for the --scope-from-pr-diff CI/PR-review path (built on #85's CLI surface). Each scenario builds a synthetic in-memory Manifest, serializes it to a temp file, runs the real cute-dbt subprocess, and asserts against the embedded cute-dbt-data JSON payload — no committed fixture files, so the synthetic-only-fixture invariant is satisfied trivially (CQO option b).

No production code: this PR touches only tests/, the two CI gate files, and one doc comment.

Step partition (cucumber-rs has one global step namespace)

  • Reuse — the exit code is {0,non-zero}, no file "report.html" is written, a baseline manifest "baseline.json" (existing impls).
  • Re-authored Givens — two audited wordings collide with diff_scoping.rs (one .unwrap()s a None baseline → panic; one builds no node). Every When/Then kept verbatim (the audited behavioral contract); only the colliding Given prose changed to pr-diff-unique, self-contained phrasing that builds the manifest.

Amendments (logged in pipeline observations.md)

  • Scenario 14 (Fix A1): rewritten from the GITHUB_EVENT_PATH contract (production GitHub PR events do not carry the changed-file list) to the real @file argument form.
  • Scenario 11: gained an in-scope unit test — Stage-2 preflight_compiled checks the targets of in-scope tests, not bare models, so the as-audited scenario would not have fail-closed.
  • Real CLI finding (not a test bug): --project-root is existence-validated by clap (renderer: surface raw unit_test YAML block (+ leading/trailing comments) in test details #69); the sub-directory scenario runs the subprocess from a temp workdir with the project-root sub-dir created (--manifest/--out are absolute).

Gate updates (ci.yml + lefthook.yml, atomically)

Tests

cargo test --test bdd → 58 scenarios (44 prior + 14 new), 338 steps, all green.

Local gates green: fmt, clippy --all-targets --locked -- -D warnings, nextest (424), cargo deny, mdbook build, non-mirror-guard, fixture-manifest gate, baseline-required-grep, feature-count, lefthook run pre-push (all 10 hooks).

Epic: #78

Closes #84

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests

    • Added comprehensive test coverage for PR-diff scoping functionality, including scenarios for scope derivation from changed files, argument validation, error handling, and edge cases.
  • Chores

    • Updated CI workflow and pre-push hook validation to enforce correct usage of baseline and PR-diff scoping options.

Review Change Stack

…ates)

Wire features/pr_diff_scoping.feature (14 scenarios) to step impls for
the --scope-from-pr-diff CI/PR-review path (#84). Each scenario builds a
synthetic in-memory Manifest, serializes it to a temp file, runs the
real cute-dbt subprocess, and asserts against the embedded cute-dbt-data
JSON payload — no committed fixture files (synthetic-only invariant
satisfied trivially).

cucumber-rs has one global step namespace, so colliding Given wordings
were re-authored to pr-diff-unique, self-contained phrasing that builds
world.current_manifest; every When/Then is kept verbatim (the audited
behavioral contract). Scenario 14 rewritten from the GITHUB_EVENT_PATH
contract (which production GitHub events do not carry) to the real @file
argument form (Fix A1). Scenario 11 gained an in-scope unit test —
Stage-2 preflight checks in-scope tests, not bare models, so the
as-audited scenario would not have fail-closed without one.

Gates: baseline-required-grep now accepts either scope source
(--baseline-manifest OR --scope-from-pr-diff); feature-count 9->10. Both
mirrored in ci.yml + lefthook.yml; job display-names frozen (#68).

Closes #84

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 28, 2026 •

Copy link
Copy Markdown

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 54 minutes and 43 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ 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: a38acbb7-af25-45eb-86aa-f45d202ebb38

📥 Commits

Reviewing files that changed from the base of the PR and between 592faae and 0f6d076.

📒 Files selected for processing (1)
  • tests/steps/pr_diff_scoping.rs
📝 Walkthrough

Walkthrough

PR adds comprehensive BDD integration tests for the --scope-from-pr-diff CLI feature. New feature file defines 15 scenarios covering happy-path scoping, edge cases, validation, and failure modes. Test helpers synthesize in-memory manifests and serialize them for subprocess execution. Step definitions orchestrate scenario execution, HTML report parsing, and assertion checking. CI and hook guards are updated to reflect the new feature count and scope-source requirements.

Changes

PR-Diff Scoping Test Suite

Layer / File(s) Summary
Feature Specification
features/pr_diff_scoping.feature
Defines 15 BDD scenarios covering happy-path model/test scoping from PR changed files, non-inclusion of unchanged siblings, zero-scope edge cases, path rewriting via --project-root, CLI validation errors, fail-closed contracts (uncompiled models), fidelity limits (YAML-only, packages.yml changes), and @file-based diff sourcing. Includes path-normalization and git-diff fallback contract comments.
CI & Pre-Push Guardrails
.github/workflows/ci.yml, lefthook.yml, tests/bdd.rs
Updates baseline-required-grep and feature-count invariants: scenarios must include either --baseline-manifest or --scope-from-pr-diff (not both, not neither); feature-count job expects 10 .feature files (up from 9). CI and lefthook comments refactored to reflect "exactly-one scope source" policy and removal of fixed scenario-count language.
Test Infrastructure
tests/steps/world.rs, tests/steps/builders.rs, tests/steps/mod.rs
Extends World with changed_files: Vec<String> and changed_files_path: Option<PathBuf> fields for PR-diff test state. Adds builders: model_node_with_original_file_path, unit_test_with_path, and serialize_to_tmp (writes Manifest to $CARGO_TARGET_TMPDIR/{name}.json and returns PathBuf). Module adds pub mod pr_diff_scoping; declaration.
Step Definitions
tests/steps/pr_diff_scoping.rs
New ~465-line module implementing Given/When/Then steps: Given constructs synthetic manifests with models/tests, sets changed-file lists, and writes @file tokens; When invokes cute-dbt binary with scope-source flags, resolves temp paths, and captures exit code/stderr/HTML; Then parses embedded JSON report and asserts models-in-scope, unit-test rows, CTE diagram presence, empty-state banners, and CLI error messages (offending node, recommended command).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • breezy-bays-labs/cute-dbt#80: Requests git-rename detection for PR-diff scoping; this PR documents rename fidelity limits as a non-blocking constraint and lays test infrastructure for future enhancements.
  • breezy-bays-labs/cute-dbt#85: Describes the --scope-from-pr-diff CLI flag and mutual-exclusivity validation; this PR implements the full BDD contract and step definitions for that behavior.

Possibly related PRs

  • breezy-bays-labs/cute-dbt#86: Introduces Node::original_file_path domain logic for PR-diff matching; this PR's builders wire that field and test helpers derive model names from original_file_path.
  • breezy-bays-labs/cute-dbt#43: Established the cucumber ATDD infrastructure (steps/world); this PR extends tests/steps/builders.rs and World to support new PR-diff synthetic-manifest scenarios.
  • breezy-bays-labs/cute-dbt#72: Also updates .github/workflows/ci.yml and lefthook.yml feature-count invariants; both PRs coordinate on the same features/*.feature count expectations.

Poem

🐰 A feature born from PR diffs bright,
Scopes models by changed files—not manifest light!
With Cucumber steps and synthetic JSON,
The paths are now tested, the contracts live on. ✨

🚥 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 title clearly and specifically summarizes the PR's main change: implementing BDD step definitions for pr_diff_scoping.feature and rewriting Scenario 14, with gate updates.
Linked Issues check ✅ Passed All acceptance criteria from issue #84 are met: pr_diff_scoping.rs with ~14 steps, builders.rs extensions (serialize_to_tmp, unit_test_with_path, model_node_with_original_file_path), world.rs fields (changed_files, changed_files_path), mod.rs update, Scenario 14 rewrite to @file form, all 14 scenarios passing, synthetic-only fixture invariant preserved, and unique wording for Given steps.
Out of Scope Changes check ✅ Passed All changes align with issue #84 objectives: test infrastructure for pr_diff_scoping.feature (pr_diff_scoping.rs, builders.rs, world.rs, mod.rs), gate updates in ci.yml and lefthook.yml to reflect new feature count and dual scope sources, and doc comment in bdd.rs—no unrelated alterations 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 test-84-pr-diff-scoping-bdd

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.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces the --scope-from-pr-diff CLI flag to cute-dbt, enabling the derivation of in-scope models and unit tests directly from a PR's file diff. It adds a new BDD feature file (pr_diff_scoping.feature), updates the lefthook pre-push hooks to enforce scope source rules, and implements the corresponding test steps and builders in Rust. The review feedback suggests replacing specific version references (e.g., v0.1.x) with generic placeholders (e.g., v0.x) in the feature file's comments to prevent documentation rot.

Comment thread features/pr_diff_scoping.feature
Comment thread features/pr_diff_scoping.feature
Comment thread features/pr_diff_scoping.feature
Comment thread features/pr_diff_scoping.feature
The pr_diff_scoping When step created the subprocess workdir via
create_dir_all(workdir.join(project_root)). For --project-root ".", that
builds a `…/workdir/.` path whose trailing "." component makes
create_dir_all fail with NotFound on Linux (macOS tolerates it) — green
locally, red in CI. Create `workdir` directly (no "." suffix) and only
join the sub-dir for a non-"." project root.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Actionable comments posted: 0

@github-actions

Copy link
Copy Markdown
Contributor

📄 Rendered report preview

All examples regenerated cleanly.

Example Artifact
jaffle-shop-report.html Download
playground-report.html Download

Click Download to fetch the rendered HTML. Each artifact
is a zip containing the single-file report — extract and open
in any browser. Reports are fully self-contained (zero
external resource requests).

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

Posted by report-preview.yml for 0f6d076bdbb56d3fa16e356e576c59a9c918bcfd. Affordance only — never blocks merge.

@cmbays
cmbays merged commit 0b557e5 into main May 28, 2026
28 checks passed
@cmbays
cmbays deleted the test-84-pr-diff-scoping-bdd branch May 28, 2026 19:48
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.

BDD: pr_diff_scoping.feature step impls + Scenario 14 rewrite (Fix A1)

1 participant