Skip to content

fix(utils): normalize path separators in resolve_chunk (semble #244) - #98

Merged
amondnet merged 1 commit into
mainfrom
amondnet/parity-normalize-path-separators-in-resolve_chun
Sep 4, 2026
Merged

fix(utils): normalize path separators in resolve_chunk (semble #244)#98
amondnet merged 1 commit into
mainfrom
amondnet/parity-normalize-path-separators-in-resolve_chun

Conversation

@amondnet

@amondnet amondnet commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Port of upstream semble #244 (b491200).

find_related (MCP tool and csp find-related) resolves the target chunk via resolve_chunk, which compared the incoming file_path against each chunk's stored path verbatim. Chunk paths are stored with the platform-native separator, so on Windows a path passed with the other separator (e.g. copied from docs, or typed with /) failed to resolve.

  • resolve_chunk now treats \ and / as equal on both sides via a zero-allocation byte comparison (paths_eq_normalized), equivalent to upstream's replace("\\", "/") on both sides.
  • The strict-inner-match / end-line-fallback behavior is unchanged.
  • Two unit tests: backslash input resolves a forward-slash chunk, and forward-slash input resolves a backslash chunk (including the boundary fallback and a non-match).
  • .please/docs/references/semble.md §4.18 notes the separator handling.

Related issue

Closes #88

Checklist

  • PR title follows Conventional Commits
  • Tests added or updated, and the suite passes (cargo test --workspace: 311 lib + 22 CLI passed)
  • Lint/format pass (cargo fmt --all && cargo clippy --all-targets --all-features -- -D warnings)
  • Documentation updated if behavior changed
  • No breaking change, or a BREAKING CHANGE: note is included

Summary by cubic

Fixes resolve_chunk to treat \ and / as equal when comparing file paths, so paths passed with the wrong separator (e.g., on Windows) no longer fail to resolve. This port of upstream semble #244 adds tests and updates the docs.

Written for commit 2729590. Summary will update on new commits.

find_related compared the incoming file_path against each chunk's stored
path verbatim. Chunk paths use the platform-native separator, so on Windows
a path passed with the other separator failed to resolve. Compare with
backslash and forward slash treated as equal on both sides, keeping the
strict-inner-match / end-line-fallback behavior.

Closes #88
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 7 complexity · 0 duplication

Metric Results
Complexity 7
Duplication 0

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

@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 updates the resolve_chunk utility in crates/csp/src/utils.rs to treat backslash and forward slash path separators as equal when matching chunk file paths, resolving upstream issue #244. It introduces a non-allocating helper function paths_eq_normalized to perform this comparison and adds corresponding unit tests to verify the behavior. The documentation in semble.md has also been updated to reflect this change. There are no review comments to assess, and the implementation looks correct and well-tested, so I have no additional feedback to provide.

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sonarqubecloud

sonarqubecloud Bot commented Sep 4, 2026

Copy link
Copy Markdown

@codspeed-hq

codspeed-hq Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 5 untouched benchmarks


Comparing amondnet/parity-normalize-path-separators-in-resolve_chun (2729590) with main (b2de30d)

Open in CodSpeed

@amondnet
amondnet marked this pull request as ready for review September 4, 2026 13:21

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 2 files

Architecture diagram
sequenceDiagram
    participant Caller as Caller (MCP tool / csp CLI)
    participant Resolver as resolve_chunk()
    participant Norm as paths_eq_normalized()
    participant Store as Chunk Store

    Note over Caller,Store: Chunk Resolution Flow (Cross-Platform)

    Caller->>Resolver: resolve_chunk(chunks, file_path, line)
    Note over Resolver: file_path may use / or \

    loop each chunk
        Resolver->>Norm: paths_eq_normalized(chunk.file_path, file_path)
        Note over Norm: Zero-alloc comparison, treats / and \ as equal
        Norm-->>Resolver: boolean (separator-normalized equality)
        
        alt paths match AND within line range
            alt line < chunk.end_line (strict inner match)
                Resolver-->>Caller: Some(chunk) (immediate return)
            else line == chunk.end_line (boundary fallback)
                Resolver->>Resolver: Save as fallback, continue scanning
            end
        else
            Resolver->>Resolver: Continue to next chunk
        end
    end

    alt Fallback was set
        Resolver-->>Caller: Some(fallback_chunk)
    else No match found
        Resolver-->>Caller: None
    end

    Note over Resolver: Example: chunk stored "src/lib/a.ts"<br/>input "src\lib\a.ts" resolves successfully
Loading

Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Sep 4, 2026

Copy link
Copy Markdown

Greptile Summary

This PR updates chunk resolution to compare forward and backward slashes as equivalent, adds bidirectional separator tests, and documents the behavior. The byte comparison correctly implements the intended normalization, but normalized matching can conflate distinct POSIX filenames containing literal backslashes.

Confidence Score: 4/5

The PR should not merge until exact path identity is preserved when separator-normalized paths are ambiguous.

resolve_chunk now considers distinct POSIX paths equal and returns the first matching chunk, which can seed related-code discovery from the wrong file.

Files Needing Attention: crates/csp/src/utils.rs

Important Files Changed

Filename Overview
crates/csp/src/utils.rs Adds allocation-free separator-normalized path comparison and tests, but normalized matching can select the wrong chunk when distinct POSIX paths alias.
.please/docs/references/semble.md Documents the newly supported slash/backslash equivalence in resolve_chunk.

Fix all with Greploop Fix All in Claude Code

Prompt To Fix All With AI
### Issue 1
crates/csp/src/utils.rs:139
**Normalized paths can collide**

On Unix, backslash is a valid filename character, so distinct indexed files such as `src/lib/a.ts` and a file literally named `src\lib\a.ts` can coexist. The normalized comparison matches both paths, even when the caller supplies an exact path, and `resolve_chunk` returns whichever matching chunk appears first. This can cause `find_related` to use the wrong file as its seed. Prefer an exact path match before falling back to separator-normalized matching.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(utils): normalize path separators in..." | Re-trigger Greptile

Comment thread crates/csp/src/utils.rs
@amondnet
amondnet merged commit fe75d1b into main Sep 4, 2026
13 of 14 checks passed
@amondnet
amondnet deleted the amondnet/parity-normalize-path-separators-in-resolve_chun branch September 4, 2026 14:50
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.

parity(#244): normalize path separators in resolve_chunk so find_related works on Windows

1 participant