Skip to content

#80 — git-rename detection on PR-diff scope - #184

Merged
cmbays merged 4 commits into
mainfrom
feature-80-prdiff-rename-detection
Jun 10, 2026
Merged

cmbays merged 4 commits into
mainfrom
feature-80-prdiff-rename-detection

Conversation

@cmbays

@cmbays cmbays commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #80

A renamed model in a --pr-diff patch was a documented v0.1 fidelity limit: git's default rename detection (on since git 2.9) emits a pure rename (100% similarity) as only the rename from/rename to extended headers — no +++ b/ header, no hunks — so the renamed model scoped nothing. This PR layers rename detection onto the existing parse → index → scope pipeline, additively.

Input forms supported (verified against real git 2.51 output)

producer form shape behavior
git diff --unified=0 (default rename detection), pure rename rename from/rename to, no hunks new: both paths join the scope keyset; the current manifest resolves the new path → model scopes under its new name
same, rename + edit rename headers + ---/+++ + hunks new path already scoped via its file entry; the index merge keeps its hunks — scopes once, not twice
git diff --no-renames delete + add pair unchanged (already worked via the new-file entry)
every existing non-rename input — byte-identical behavior (full suite green; rename-free PrDiff serializes without the new field)

--name-status (R<score>\told\tnew) was not added as an input form: since #96 the flag takes a raw unified diff (the hunks drive block precision and the inline diffs), the recipe's actual producer already emits rename headers in that patch, and a second input grammar would carry none of the hunk information the rest of the feature set needs.

Design

  • cli::pr_diff — the line-oriented state machine recognizes rename from/rename to in header territory only (the in_hunk disambiguation keeps a +rename from … added body line from forging a pair). Paths taken verbatim — git emits them un-prefixed and unquoted-for-spaces; C-quoted non-ASCII paths are not dequoted (parity with the +++ b/<path> parser).
  • domain::pr_diff — new RenamePair POD; PrDiff.renames with #[serde(default, skip_serializing_if)] (pre-Deferred (v0.2+): git-rename detection layer on PR-diff scope #80 payloads deserialize; rename-free payloads stay byte-identical). NormalizedDiffIndex::new puts both normalized sides of every pair into the changed-file keyset (strip-applied), never attaching hunks via the rename.
  • domain::scope — zero code change: scope selection consults the keyset; the rename mapping lives entirely in the index.
  • Lane-A only: no template/render/golden changes. The rename lineage (old ↔ new) is preserved as pairs on the POD so a future report affordance can surface it without re-parsing.

Semantics decisions

  • A pure rename scopes the renamed model (per the task brief) — the current node at the new path, its tests in scope as context (a model rename doesn't mark a test's YAML changed).
  • A purely-renamed declaring YAML marks its tests file-granular changed at scope level; the existing feature: block-precise PrDiff updated-test detection + inline YAML diff #96 block-precise refinement then narrows them to context (zero hunks touch no block) — mechanical consequence of existing machinery, pinned by a unit test.
  • The old path is kept in the keyset conservatively: it matches no current-manifest node after a clean rename, so it is at worst inert.

Tests

  • TDD red→green: 4 index tests failed before the index change, green after; 803/803 nextest, 88/88 BDD scenarios.
  • Parser: pure rename, rename+edit, spaced paths, CRLF, multi-rename, body-line forgery, stray/dangling header defensives.
  • Property test (house style — exhaustive, no proptest dep): index keyset == normalized union of file paths ∪ both sides of every rename pair, over every file-count × rename-count × strip combination.
  • Scope: pure rename scopes at new path; rename+edit scopes once not twice; inert old path; YAML rename; project-root strip on both sides.
  • BDD: the .feature fidelity-limit comment is replaced by two executable scenarios; the harness synthesizer emits real git rename header blocks (shape verified against git 2.51).

Docs

Gates (run directly — lefthook skips in fresh worktrees)

fmt ✓ · clippy --all-targets --locked -D warnings exit 0 ✓ · nextest 803/803 ✓ · bdd 88/88 ✓ · headless_zero_egress ✓ · headless_toggle 29/29 ✓ · rustdoc -D warnings --document-private-items --locked exit 0 ✓ · deny ✓ · baseline-required-grep + feature-count (12) mirrors ✓ · mdbook build ✓

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added git rename detection to PR diffs. Renamed models are now included in PR reports under their new names.
  • Documentation

    • Updated documentation to describe how renamed models are handled in PR scoping.

A pure rename (100% similarity) in a git diff carries only the
'rename from'/'rename to' extended headers — no '+++ b/' header, no
hunks — so the renamed model scoped nothing. The parser now collects
the rename pairs into PrDiff.renames (serde(default) +
skip_serializing_if keeps rename-free payloads byte-identical), and
NormalizedDiffIndex puts BOTH sides of every pair into the
changed-file keyset. Scope selection is untouched: the current
manifest (compiled at the PR head) resolves the new path to the
renamed node, so the model scopes under its new name; the old path
matches no current node and is inert. A rename-with-edit already
carried its hunks on the new path's file entry — the index merge
keeps them, so the model scopes once, not twice.

Verified against real 'git diff --unified=0' output (git 2.51): pure
rename, rename+edit, spaced paths (rename headers are unquoted and
un-prefixed), and the --no-renames delete+add fallback (which already
worked via the new-file entry). Rename paths are taken verbatim;
C-quoted non-ASCII paths are not dequoted — parity with the
'+++ b/<path>' parser.

Docs: the CI recipe's producer command gains --find-renames (explicit
+ config-proof; both forms keep working), and the renamed-model
fidelity-limit bullets in how-it-works and the recipe are replaced
with the supported behavior. The .feature fidelity comment becomes
two executable scenarios (pure rename / rename+edit) on the existing
synthesizer, extended to emit real rename header blocks.

The Windows-path deferral that piggybacked on cute-dbt#80's tracking
comments is re-pointed at its own issue (tracked: cute-dbt#183).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 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 42 minutes and 21 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: 2333246e-959a-48df-aeeb-6ee072e911db

📥 Commits

Reviewing files that changed from the base of the PR and between aceec13 and 39cf5cf.

📒 Files selected for processing (1)
  • src/cli/pr_diff.rs
📝 Walkthrough

Walkthrough

This PR implements deferred feature #80: git-rename detection for PR-diff scoping. The parser now extracts git rename from/rename to headers, stores them in the PrDiff data model, integrates them into scope indexing so both rename sides participate in changed-path selection, and ensures models are correctly scoped under their new names even for pure (content-free) renames.

Changes

Git rename detection for PR-diff scoping

Layer / File(s) Summary
Rename data model and parser
src/domain/pr_diff.rs, src/cli/pr_diff.rs
PrDiff extends with renames: Vec<RenamePair> field. The parse_unified_diff parser detects rename from/rename to git headers and collects them (handling pure renames, renames with edits, CRLF trimming, and stray headers). Serde maintains backward compatibility with payloads lacking rename information.
Scope indexing with renamed paths
src/domain/pr_diff.rs
NormalizedDiffIndex::new inserts both sides of rename pairs (after applying project-root strip rules) into the normalized changed-paths keyset, ensuring pure renames participate in scope selection. Comprehensive tests verify keyset membership, stripping behavior on both sides, hunk preservation when rename destination has content edits, and keyset equality across all combinations.
Scope selection with rename lineage
src/domain/scope.rs
Scope selection logic delegates rename handling to NormalizedDiffIndex. New tests verify pure renames scope the model under its new path, renames with edits scope exactly once with expected cardinality, inert renames yield no phantom results, renamed YAML files scope tests as both in-scope and changed, and project-root stripping applies to both rename sides.
CI workflow rename detection
.github/workflows/examples/cute-dbt-pr-review.yml, .github/workflows/report-preview.yml, .github/workflows/...
Three workflows add --find-renames to their git diff commands to enable consistent git rename detection across CI environments.
User-facing documentation
book/src/how-it-works.md, book/src/recipes/github-actions-pr-review.md
Documentation explains how --pr-diff handles git renames by mapping both old and new paths, including behavior for pure (content-free) renames. The v0.1 fidelity-limits section is rewritten to confirm renamed models are now in-scope (previously deferred).
Test infrastructure and BDD support
tests/steps/world.rs, tests/steps/pr_diff_scoping.rs, features/pr_diff_scoping.feature
Test framework extends with World.renames field and RenameDirective struct to represent rename-only and rename-with-edit scenarios. Patch synthesis emits corresponding git diff headers. Two new Cucumber steps allow test authors to declare renames, and the feature file adds "Renamed models" scenarios covering both cases.
Test fixture updates
src/cli/mod.rs, src/domain/cell_diff.rs, tests/cell_table_diff.rs, tests/headless_toggle.rs, src/domain/path.rs, tests/path_matching.rs
Various test fixtures and helpers updated to include renames: Vec::new() for struct consistency, and path documentation updated to reflect current tracking notes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • breezy-bays-labs/cute-dbt#119: Overlaps in the report-preview.yml workflow's prdiff-preview git diff command, where this PR adds the --find-renames flag that PR was investigating.
  • breezy-bays-labs/cute-dbt#86: Both PRs extend src/domain/scope.rs tests and builders around select_in_scope and PrDiff inputs, with this PR introducing PR-diff-based scope selection tests that build on the infrastructure introduced there.

Poem

🐰 A model renamed, yet scope remembers—
Both old path and new in the diff's embers.
Pure renames now scoped without a hunk's trace,
Git detects the dance, finds each file's new place.
Thump-thump! Our renaming rabbit rejoices.

🚥 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 '#80 — git-rename detection on PR-diff scope' directly and clearly describes the main feature addition: implementing git-rename detection for PR-diff scoping, matching the PR's primary objective.
Linked Issues check ✅ Passed The PR implements all key acceptance criteria from issue #80: git-rename signal layered on PR-diff scope with rename pairs collected in PrDiff.renames, parser detects and records git rename headers, NormalizedDiffIndex inserts both rename sides into changed-file keyset ensuring pure renames scope correctly, and comprehensive tests added.
Out of Scope Changes check ✅ Passed All changes directly support git-rename detection: parser/index/scope/test infrastructure for rename handling, CI workflow updates to enable --find-renames, documentation updates clarifying rename behavior, and Windows-path deferral appropriately references issue #183 as out-of-scope follow-up.
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 feature-80-prdiff-rename-detection

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

…e-dbt#80)

Add --find-renames to the two workflow git-diff producer invocations
(report-preview.yml prdiff-preview job — the live dogfood — and the
committed example workflow), matching the recipe's documented producer
command. Dogfood-alongside: without this the rename-detection feature
would be invisible in the repo's own live PR preview. The flag only
makes git's default rename detection explicit (config-proof against a
runner setting diff.renames=false); patch shape is otherwise identical.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@ghost

ghost commented Jun 10, 2026 •

Copy link
Copy Markdown

Ready to review this PR? Stage has broken it down into 6 individual chapters for you:

Title
1 Define RenamePair domain model and update index
2 Implement git rename detection in diff parser
3 Update scope selection and path normalization docs
4 Update unit tests for rename support
5 Update BDD scenarios and test harness
6 Update documentation and CI recipes
Open in Stage

Chapters generated by Stage for commit 39cf5cf on Jun 10, 2026 3:32pm UTC.

@github-actions

github-actions Bot commented Jun 10, 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
playground-report.html ▶ Open ↗ ⬇ Download
jaffle-shop-report.html ▶ Open ↗ ⬇ Download

🐶 Live dogfood preview

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

▶ Open ↗ opens the report in your browser in one click —
published to this repo's GitHub Pages under /pr-184/.
⬇ 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 27287294599 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.html

Posted by report-preview.yml for 39cf5cfe384447becdad13a51d38aaac0eb50585. Affordance only — never blocks merge.

@gemini-code-assist

Copy link
Copy Markdown

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@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: 1

🤖 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 `@book/src/how-it-works.md`:
- Around line 149-154: Update the paragraph that currently says "since git 2.9"
to state the correct timeline/wording (e.g., "since April 2016 (commit
5d2a30d7d8777319c745804f040fa405d02169ce)") and clarify that this default
applies to porcelain commands like `git diff`/`git log`; note that adding
`--find-renames` to `git diff` is therefore redundant in modern versions (only
needed for older git or other commands). Also replace the jargon "maps both
paths onto the scope match" with clearer wording such as "maps both paths to the
model under the renamed path" while keeping the rest of the "rename
from`/`rename to`" and "pure rename (100% similarity, no hunks at all)" wording
intact.
🪄 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: ae5aada0-ba28-44c1-9dd3-8f09e272d55e

📥 Commits

Reviewing files that changed from the base of the PR and between 2e4b37d and aceec13.

📒 Files selected for processing (16)
  • .github/workflows/examples/cute-dbt-pr-review.yml
  • .github/workflows/report-preview.yml
  • book/src/how-it-works.md
  • book/src/recipes/github-actions-pr-review.md
  • features/pr_diff_scoping.feature
  • src/cli/mod.rs
  • src/cli/pr_diff.rs
  • src/domain/cell_diff.rs
  • src/domain/path.rs
  • src/domain/pr_diff.rs
  • src/domain/scope.rs
  • tests/cell_table_diff.rs
  • tests/headless_toggle.rs
  • tests/path_matching.rs
  • tests/steps/pr_diff_scoping.rs
  • tests/steps/world.rs

Comment thread book/src/how-it-works.md
… -> 22)

The rename-header branches (cute-dbt#80) pushed parse_unified_diff to
complexity 31 against the crap4rs threshold of 25 (coverage is 100%,
so CRAP == CC). Extract the header-territory classification into two
helpers, mirroring the existing consume_body_line seam:

- consume_path_header  — the '--- '/'+++ ' path headers (CC 3)
- consume_rename_header — the 'rename from'/'rename to' pair, incl.
  the stray-'rename to' lenience (CC 5)

parse_unified_diff keeps the state machine's spine (file boundaries,
hunk open/close, saw_structure) and lands at CC 22. Behavior is
byte-identical: zero test changes; 803 nextest + 88 BDD scenarios
green. Scorecard verified with BOTH the local crap4rs v0.6.0 and the
CI-pinned v0.4.0 ('crap4rs --config crap4rs.toml --coverage
lcov.info'): 0 functions above threshold, repo worst is the
pre-existing normalize_path at 23, exit 0.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cmbays
cmbays merged commit 9557507 into main Jun 10, 2026
31 checks passed
@cmbays
cmbays deleted the feature-80-prdiff-rename-detection branch June 10, 2026 15:38
github-actions Bot added a commit that referenced this pull request Jun 10, 2026
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.

Deferred (v0.2+): git-rename detection layer on PR-diff scope

1 participant