Skip to content

#183 — Windows-style path separators in PR-diff path matching - #190

Merged
cmbays merged 2 commits into
mainfrom
domain-183-windows-path-separators
Jun 10, 2026
Merged

cmbays merged 2 commits into
mainfrom
domain-183-windows-path-separators

Conversation

@cmbays

@cmbays cmbays commented Jun 10, 2026 •

Copy link
Copy Markdown
Contributor

Closes #183

Summary

normalize_path now canonicalizes Windows-style \ separators to / as Step 0 (applied to the path and the --project-root strip prefix), closing a one-sided real gap: a dbt manifest compiled on Windows carries \-separated original_file_path while git diff emits / on every platform — so a Windows-compiled manifest silently missed PR-diff scoping. The fix lives in the domain's single normalization authority, so both the diff-side keyset and the declaring-side lookups (NormalizedDiffIndex) are covered.

Step-0 findings (research dbt-fusion source first)

Pinned to dbt-fusion commit 9977b6cbb1b761065536300037560d8e3c037011 (current main, same SHA as prior pins):

  1. Do dbt manifests on Windows emit / or \ in original_file_path? → \. Fusion types original_file_path: PathBuf in the manifest node PODs (crates/dbt-schemas/src/schemas/manifest/manifest_nodes.rs lines 93/121/288; crates/dbt-schemas/src/schemas/nodes.rs CommonAttributes line 4460) with no serialize_with/slash normalization — serde emits the native PathBuf verbatim, and on Windows those paths are built with \. Fusion itself confirms the divergence is real: dbt_common::path::path_separator_eq (crates/dbt-common/src/path.rs) compares strings "treating / and \ as equivalent", used in crates/dbt-adapter/src/time_machine/serde.rs:169-174 to "Normalize path separators so recordings made on Linux (/) match replay on Windows (\)". No to_slash-style normalization exists anywhere in the repo.
  2. Does git diff on Windows emit /? → Yes. Git stores index/tree entry path names with / as the path separator on every platform (git/git Documentation/gitformat-index.adoc lines 130–133 @ 1ff279f3404a482a83fb04c7457e41ab26884aea); diff headers print the stored names.

So the gap is one-sided (manifest \ vs diff /) — not docs-only. Canonicalizing \ → / in normalize_path mirrors fusion's own / ≡ \ equivalence semantics. A Unix filename containing a literal \ is therefore misread — accepted deliberately and documented (fusion's own path equality already conflates the two).

Acceptance criteria

  • normalize_path treats \ and / equivalently — Step-0 canonicalization in the domain (the single-normalization-authority arm, stronger than a boundary-only doc), applied to both the path and the strip prefix. Module docs carry the full source-pinned canonicalization rationale.
  • tests/path_matching.rs gains \-separator cases with mutation-kill parity — every / case shape has a mirrored \ case (12 normalize_path mirrors + 7 match_changed_path mirrors, incl. the real-world \-manifest ↔ /-diff bridge, the segment-aware-prefix guard, the substring-collision guard, and mixed-separator inputs). Bounded cargo mutants over src/domain/path.rs with kill detection restricted to the dedicated kill suite + lib unit tests: 13/13 caught (including all 4 canonicalize_separators replacement mutants). src/domain/path.rs also joins .cargo/mutants.toml examine_globs — it was extracted from scope.rs at feature: block-precise PrDiff updated-test detection + inline YAML diff #96 with its kill suite but never re-added to the focused scope.
  • The tracked: cute-dbt#183 deferral comments in src/domain/path.rs (module doc + fn doc) and tests/path_matching.rs are resolved into the canonicalization documentation (exclusions rule — no live deferral pointers remain; verified by grep).

Gates (run directly — lefthook skips in fresh worktrees)

  • cargo fmt --check — clean
  • cargo clippy --all-targets --locked -- -D warnings — exit 0, no issues
  • cargo nextest run — 911 passed, 32 skipped
  • cargo test --test bdd — 96 scenarios / 607 steps passed
  • RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --locked — exit 0
  • cargo deny check — advisories/bans/licenses/sources ok
  • cargo llvm-cov nextest + crap4rs --config crap4rs.toml --coverage lcov.info — PASS, 0/1305 functions above threshold (worst 23.0)
  • Goldens byte-unchanged: git diff --text -U0 -- examples/ empty
  • cargo mutants (bounded, path module only): 13 mutants tested, 13 caught

Tests are platform-independent string-level normalization cases — no filesystem dependence (no create_dir_all-class macOS/Linux divergence risk).

🤖 Generated with Claude Code


Open in Stage

Summary by CodeRabbit

  • Bug Fixes

    • Windows path separators (backslashes) are now correctly canonicalized to forward slashes during path normalization, enabling consistent path matching and handling across Unix and Windows platforms.
  • Tests

    • Added comprehensive test coverage for Windows-style path handling, including prefix stripping and path matching scenarios.

A dbt manifest compiled on Windows serializes original_file_path from a
native PathBuf verbatim -- backslash-separated (dbt-fusion
crates/dbt-schemas/src/schemas/manifest/manifest_nodes.rs @ 9977b6cb;
no slash normalization at serialization, and fusion's own
dbt_common::path::path_separator_eq treats / and \ as equivalent).
git diff, by contrast, emits /-separated repo paths on every platform
(index/tree entry path names are stored with / -- git/git
Documentation/gitformat-index.adoc @ 1ff279f34). Without
canonicalization a Windows-compiled manifest never matched the diff
keyset and PR-diff scoping silently missed.

normalize_path now canonicalizes backslash to / as Step 0, applied to
the path and the strip prefix, so every later step (./ strip, prefix
strip, separator-run collapse) sees one separator vocabulary. The
single normalization authority (NormalizedDiffIndex) normalizes both
the diff-side keyset and the declaring-side lookups through it, so the
fix covers both directions.

tests/path_matching.rs gains backslash-separator mirrors of every /
case shape (mutation-kill parity); src/domain/path.rs joins the
cargo-mutants examine_globs (it was extracted from scope.rs at #96
with its dedicated kill suite but never re-added to the focused
scope). Bounded mutation run over domain/path.rs: 13/13 caught.

The tracked: cute-dbt#183 deferral comments at both exclusion sites
are resolved into the canonicalization docs (exclusions rule).

Closes #183

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

coderabbitai Bot commented Jun 10, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2b50ef6a-106c-4e6e-8a59-5c9dbc92d048

📥 Commits

Reviewing files that changed from the base of the PR and between 86f0815 and 61f7e89.

📒 Files selected for processing (3)
  • .cargo/mutants.toml
  • src/domain/path.rs
  • tests/path_matching.rs

📝 Walkthrough

Walkthrough

This PR adds Windows path separator canonicalization to the path normalization module. Backslash separators are converted to forward slashes as the first normalization step, enabling consistent path matching across platforms. Implementation includes updated documentation, comprehensive test coverage for both inline validation and mutation kill, and configuration updates.

Changes

Windows Path Canonicalization

Layer / File(s) Summary
Core canonicalization implementation
src/domain/path.rs
canonicalize_separators converts \ to / at the start of normalize_path before leading ./ stripping and prefix handling. strip_prefix also canonicalizes both the input path and prefix argument before segment-aware matching. Module docs and v0.1 limitations updated to reflect cross-platform support.
Inline unit test validation
src/domain/path.rs
Unit tests validate normalize_path backslash handling (separator canonicalization, project-root prefix stripping, segment-aware guard preservation) and match_changed_path correctly matching backslash-separated manifests against forward-slash diff paths.
Complete Windows test suite
tests/path_matching.rs
Comprehensive test coverage mirrors the existing forward-slash suite with Windows-specific cases for normalize_path canonicalization, leading .\ stripping, repeated separator collapsing, prefix trimming (including trailing separator forms), and match_changed_path bidirectional matching across separator variants with substring collision rejection.
Mutation test scope configuration
.cargo/mutants.toml
src/domain/path.rs added to focused mutation testing scope with documentation noting file origin and dedicated test suite.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

  • #183: This PR directly implements the Windows path separator canonicalization and test coverage requested in the issue, resolving the mismatch between backslash manifest paths and forward-slash diff entries.

Poem

🐰 A rabbit's path through Windows trees,
Backslashes dance where forward slashes please—
Canonicalized with hop and care,
Now matching works both here and 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 title clearly and specifically describes the main change: Windows-style path separator support in path matching for PR diffs, which is the primary focus across all modified files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch domain-183-windows-path-separators

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 10, 2026 •

Copy link
Copy Markdown

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

Title
1 Implement Windows-style path separator canonicalization
2 Verify path matching with unit and integration tests
3 Enable mutation testing for path domain
Open in Stage

Chapters generated by Stage for commit 61f7e89 on Jun 10, 2026 10:38pm 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
playground-report.html ▶ Open ↗ ⬇ Download
diff-showcase-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-190/.
⬇ 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 27310909030 -R breezy-bays-labs/cute-dbt -n report-preview-playground
open report-preview-playground/playground-report.html

Posted by report-preview.yml for 61f7e8900d01c062730fc21b73327aaf3359c709. Affordance only — never blocks merge.

@cmbays

cmbays commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 10, 2026 •

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@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 Windows-style path separator canonicalization to forward slashes in path normalization and matching, addressing compatibility issues with Windows-compiled dbt manifests. It also includes comprehensive unit and integration tests to prevent regressions. The review feedback suggests optimizing the path normalization function to avoid a redundant heap allocation when handling Windows paths.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/domain/path.rs
@cmbays
cmbays merged commit 8690d42 into main Jun 10, 2026
32 checks passed
@cmbays
cmbays deleted the domain-183-windows-path-separators branch June 10, 2026 22:44
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.

Windows-style path separators in PR-diff path matching

1 participant