Resolve the search bundle so a symlinked root works - #86
Merged
Merged
Conversation
rg prints resolved paths. search() built each hit's display path against the bundle the caller passed, so when that bundle still held a symlink the two did not match and relative_to raised. Search failed outright. Root cause is the bundle, not the line that raised. Every engine derives its paths from the bundle it is handed, so resolving once at the top of search() makes candidate_files and the display loop agree. One syscall per call, not one per file, which is the property _filter_rg_hits already protects. candidate_files keeps taking the bundle as given. Tests call it directly and expect paths under the bundle they passed, and _filter_rg_hits already resolves for its own comparison, so that path was never broken. Not macOS-only. /var -> /private/var makes every temporary bundle reproduce it there; a symlinked checkout or a container bind mount is the same shape on Linux. CI has neither, which is why this shipped. Adds test_symlinked_bundle_agrees_across_engines, which searches through a real symlink on all three engines. Verified it fails with the one-line fix reverted and passes with it. tools/ci-local.sh reports 27 passed, 0 failed. Closes #85. Item 01M2XWFX063T5QMJ7SC5ZE2R6Q. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #85.
Why
rgprints resolved paths.search()built each hit's display path against the bundle the caller passed, so when that bundle still held a symlink the two did not match andrelative_toraised. Search failed outright.This is not macOS-only.
/varis a symlink to/private/varthere, so every temporary bundle reproduces it. A symlinked checkout or a container bind mount is the same shape on Linux. CI has neither, which is why it shipped.What
One line. The root cause is the bundle, not the line that raised. Every engine derives its paths from the bundle it is handed, so resolving once at the top of
search()makescandidate_filesand the display loop agree. One syscall per call, not one per file, which is the property_filter_rg_hitsalready protects.candidate_fileskeeps taking the bundle as given. Tests call it directly and expect paths under the bundle they passed, and_filter_rg_hitsalready resolves for its own comparison, so that path was never broken. Resolving there instead broketest_index_candidates_honor_prefix_without_scan, which is how the narrower placement was chosen.Verification
New
test_symlinked_bundle_agrees_across_enginessearches through a real symlink on all three engines (scan, rg, index) and asserts they return the same path.tests/test_pkc.pyERROR: test_symlinked_bundle_agrees_across_enginesRan 112 tests,OKtools/ci-local.shreports 27 passed, 0 failed. It was red on macOS before this.🤖 Generated with Claude Code