perf(retrieval): trust unchanged indexes instead of rebuilding and rescanning - #1403
Draft
HaD0Yun wants to merge 2 commits into
Draft
perf(retrieval): trust unchanged indexes instead of rebuilding and rescanning#1403HaD0Yun wants to merge 2 commits into
HaD0Yun wants to merge 2 commits into
Conversation
chardet.detect() dominates the cost of reading a document. On a 4KB file it takes 672us, while actually decoding the bytes takes 10us and NFC normalization takes 1us. It runs on every file, including the ones where every candidate encoding would decode identically. Skip detection when the buffer is pure ASCII: at least 4 bytes, every byte in 0x01-0x7F, no ESC. For such a buffer chardet can only return ASCII, UTF-8, ISO-8859-x, Shift_JIS, Big5, EUC-JP, EUC-KR or GB18030. UTF-16 and UTF-32 need a BOM or NUL bytes, ISO-2022 needs ESC, and the windows-125x recognizers need C1 bytes (0x80-0x9F) — all excluded by the predicate. Every remaining candidate maps 0x01-0x7F to the same code points, and NFC is the identity on that range, so the decoded string is provably unchanged. Buffers shorter than 4 bytes stay on the legacy path. chardet's UTF-32LE heuristic can claim UTF-32LE for short control-character buffers and iconv-lite then drops the incomplete unit, so the existing output for those is the empty string. Preserving that keeps this a pure optimization rather than a silent behavior change. Cold refresh of a 200-document ASCII corpus: 175ms -> 42ms (4.2x, median of 5 runs against the same corpus on the same machine). Korean corpora are unchanged at ~1.0x, since the fast path does not apply to them. test/parser/decode-text.test.ts keeps a copy of the previous implementation and asserts both produce identical output across boundary lengths, boundary byte values, BOM, NUL bytes, CP949 text, combining characters, invalid sequences and seeded pseudo-random buffers.
…scanning Two hot paths did redundant work, and both are fixed by the same piece of state, so they land together. **Refresh rebuilt an unchanged index.** A refresh rewrote the entire BM25 index and re-read every mirror even when nothing on disk had changed; BM25 was 61% of that refresh. Record a sha256 of the normalized mirror content on each parsed entry (the string is already in memory, so this adds no read) and commit a fingerprint sidecar after the artifact. A refresh whose recomputed fingerprint matches the stored one reuses the artifact. Legacy entries without a digest are backfilled once. **Every query rescanned the corpus.** The fallback engine re-read and re-tokenized the whole index per query, then walked every chunk to compute document frequencies. Cost was driven by corpus size alone: a query matching two documents cost the same as one matching all of them. Cache the prepared state (tokenized chunks plus postings) keyed by that same fingerprint sidecar, derive document frequencies from postings, and narrow candidates to chunks that contain a query term. Narrowing cannot change output: bm25Score skips zero-frequency terms and its idf is positive for every df <= N, so a chunk with no query term scores exactly zero and the existing score > 0 filter already dropped it. Scoped document frequencies still count only postings inside the scope. The tantivy path opened the index and built a searcher per query; that handle is cached under the same key. **The fingerprint has to be trustworthy for either to be safe.** The CLI builds a fresh agent per command, so 'autorag watch' in one terminal and 'autorag refresh' in another are two processes over one index directory; without an on-disk lock they can commit a fingerprint describing a newer corpus than the artifact it points at, and every later refresh then trusts it. This adds a cross-process refresh lock wrapping the existing acquireFileLock primitive, at .autorag/refresh.lock — outside every directory 'index reset' removes. 'index reset' and 'index rebuild' take the same lock. Code that shapes artifact meaning is pinned by source-region markers a test digests: chunking, chunk id hashing, fallback persistence schema, tantivy schema construction, the fingerprint material itself, and mirror normalization. Changing any of them fails until INDEX_SEMANTICS_VERSION is bumped or the recorded digest is refreshed deliberately. The guard is a manual allowlist and does not discover new files on its own. Query-time code (tokenize, bm25Score, k1, b) is deliberately outside those regions: changing the tokenizer changes results immediately and correctly against the stored artifact, so guarding it would force needless global rebuilds. A test mutates the tokenizer and asserts the artifact mtime stays unchanged while results change. Behavior change: a refresh refused because another holds the lock now exits 1 with ok:false and outcome:"busy" instead of exiting 0. Automation checking only the exit code previously read a refused refresh as a successful one. 'index reset'/'index rebuild' likewise exit 1 when refused, and a non-directory file at the lock path is now an explicit error rather than a silent no-op. Watch ticks are unaffected; busy is normal backpressure there. No-op refresh on 2,000 documents: 78ms -> 44ms. Selective query on 1,200 chunks: 19.0ms -> 0.021ms, and growing the corpus 8x no longer grows selective query cost.
This was referenced Aug 8, 2026
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
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.
The problem
Two hot paths did redundant work.
Refresh rebuilt an unchanged index. A refresh rewrote the entire BM25 index and re-read every mirror even when nothing on disk had changed. BM25 was 61% of that refresh, all redundant.
Every query rescanned the corpus. The fallback engine re-read and re-tokenized the whole index per query, then walked every chunk to compute document frequencies. Cost was driven by corpus size alone — a query matching 2 documents cost the same as one matching all of them, because the scan was linear rather than an inverted lookup.
mainWhy these land together
Both are fixed by the same piece of state: a fingerprint sidecar committed after the artifact. Refresh compares it to decide whether to rebuild; the query cache uses it as its cross-process invalidation key. Splitting them would mean introducing the same mechanism twice.
Why there is a lock in a performance PR
Skipping work based on a stored fingerprint is only sound if the artifact and its fingerprint cannot interleave across processes.
The CLI builds a fresh agent per command, so
autorag watchin one terminal andautorag refreshin another are two OS processes over one index directory. Without an on-disk lock they can commit a fingerprint describing a newer corpus than the artifact it points at — and every later refresh then trusts it. Permanently.The lock wraps the existing
acquireFileLockprimitive (same onememory.tsandsubagents/runtime.tsuse) and lives at.autorag/refresh.lock, outside every directoryindex resetremoves.index resetandindex rebuildtake the same lock.Behavior changes — please push back if you'd prefer otherwise
autorag refreshrefused because another holds the lockok:false,outcome:"busy"index reset/index rebuildrefused.autorag/refresh.lockThese were all cases where the tool reported success while doing nothing. Anything checking only the exit code read a refused refresh as a successful one. This can break existing
set -eCI or cron that treated contention as success; conversely, exit-code-based retry can now detect contention and back off.watchis unaffected — busy is normal backpressure there and ticks still exit 0.The part I'd most like your opinion on
Because the fingerprint decides whether a stored artifact is trusted, forgetting to bump
INDEX_SEMANTICS_VERSIONafter changing chunking or serialization would silently serve stale results. So this adds source-region markers around the code that shapes artifact meaning, and a test that digests those regions and fails until the version is bumped or the recorded digest is refreshed deliberately.It works — adversarial testing broke the guard three times during development (
hash(),normalizeMarkdown(), andcomputeFingerprint()itself were each outside the guarded set at some point, and each let a real semantic change through while the whole bench suite stayed green). But:If you'd rather not carry that, the alternative is documenting "bump the version when you change these functions" and accepting the risk. I have no attachment to my version.
Query-time code (
tokenize,bm25Score,k1,b) is deliberately outside the regions. Changing the tokenizer changes results immediately and correctly against the stored artifact — a test mutates it and asserts the artifact mtime stays unchanged while results change. Guarding it would force needless global rebuilds.Known limitations
index rebuildis concurrency-atomic but not rollback-atomic. Delete and rebuild are covered by one lock, so nothing can interleave. But if building fails after the delete, the indexes stay removed and you must rerun.artifactBytesonly checks size. A same-length tampered artifact passes. Truncation detection, not integrity verification.autoengine sticks to fallback after one tantivy failure, since the engine is part of the fingerprint. Results stay correct but quality silently stays lower untilindex rebuild.--force. An unchanged source with the same mtime/size is not re-mirrored, so a new parser or achardet/iconv-liteupgrade does not re-parse existing documents. Pre-existing, but worth stating since the fingerprint now depends on mirror content.mkdiratomicity is unverified on NFS.Verification
biome check(252 files, 0 warnings),tsc --noEmit, andbun run buildpass. CI passed on the combined branch before I split it.Test count 1,249 → 1,365. New coverage: fingerprint skip, artifact integrity, the semantics guard, serial mirror-sync contracts, query cache equivalence across both engines and every scope, cache bounds, lock behavior in-process and across real OS processes, and
index resetparticipation in the lock.Local
vitest runshows 3 failures: two need a Java runtime this machine lacks (PDF parser), one is a load-dependent flake that passes 10/10 in isolation. That last category is not introduced here — runningvitest runon unmodifiedmainon this machine produced more failures (12 files / 15 tests) than this branch (10 / 13) in the same session.vitest.config.tssets no pool or concurrency limits, so wall-clock-budgeted tests compete under load. Happy to open that separately.Not included
An earlier revision parallelized mirror parsing. I reverted it before opening this: measured gain was 1.01–1.10x on markdown (parsing is dominated by synchronous CPU, not I/O wait), and adversarial testing found a deadlock on a zero concurrency limit, unbounded fan-out on
NaN, and a checkpoint/disk digest mismatch that could serve stale results indefinitely. It needs a deterministic commit-prefix design and real PDF/Office measurements, so it belongs in its own change.Questions