fix(folder): stable root ids in source_ref; migrate legacy rows in place - #38
Merged
Conversation
folder source_ref embedded the root's POSITION in the config list
("{idx}:{rel}"), so reordering roots orphaned every chunk and
re-embedded the corpus. The live cost is real: reordering already
produced thousands of duplicate chunk identities, and the orphaned
rows surface as duplicate/stale FTS hits.
source_ref now uses a stable per-root id ("r" + 10 hex of the resolved
root path's SHA-256), immune to config order. Resolution accepts every
format ever written (stable, positional, absolute, bare relative).
A new optional adapter hook — migrate_source_refs(store), discovered by
duck-typing so external adapters keep validating — runs at the start of
every indexing pass and heals legacy rows in place: positional and
in-root absolute refs are rewritten; rows whose rewrite target already
exists (past drift duplicates) are merged with their vectors preserved
on the survivor; unmappable refs are left alone and counted.
End-to-end regression: index roots [A,B], extend with [B,A] — zero
chunks re-created, zero vectors re-embedded.
Contributor
Author
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aa7588ec7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review: mapping a legacy positional ref through the CURRENT root order mis-assigns it when the user reordered roots before upgrading — "0:alpha.md" written under old root A would take B's stable id, and the walk would then re-create (and re-embed) A's chunk anyway. The item's corpus_ref — the absolute file path item_key is hashed from — is the order-independent authority; the migration now consults it first. The positional fallback (corpus_ref missing) only rewrites when the file actually exists at the mapped location. Regression test covers the exact scenario, with a same-named decoy file in the wrong root so naive order-mapping would succeed wrongly.
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.
Problem
FolderAdapterwritessource_refas{root_index}:{relative_path}— the root's position in the config list. Reordering, adding, or removing folder roots silently orphans every existing chunk (identity includessource_ref) and re-embeds the corpus at full provider cost. Measured on a real DB: 12,319 of 64,466 folder chunk identities exist under more than one ref variant, 6,426 of them orphaned pre-v0.2.4 absolute-path rows — andsearch_fulltextsurfaces those rows as duplicate/stale hits.Fix
source_refis nowr<10-hex-of-root-path-hash>:{rel}— immune to config order. Resolution accepts every historical format (stable / positional / absolute / bare relative).migrate_source_refs(store), called byrun_indexingbefore walking (duck-typed — NOT added to theCorpusAdapterprotocol, so existing external adapters keep passingisinstancechecks). Positional and in-root absolute refs are rewritten in place; rows whose rewrite target already exists (past drift duplicates) are merged, vectors preserved — the legacy row's vectors move to the survivor wherever the run slot is free. Unmappable refs (absolute paths outside every configured root) are left untouched and counted as skipped. Idempotent.Tests
[A,B], extend with[B,A]→ 0 chunks re-created, 0 vectors re-embedded, all skipped.