fix(cache): key semantic cache on the extraction prompt (#1939)#1942
Closed
SinghAman21 wants to merge 1 commit into
Closed
fix(cache): key semantic cache on the extraction prompt (#1939)#1942SinghAman21 wants to merge 1 commit into
SinghAman21 wants to merge 1 commit into
Conversation
…s#1939) The semantic cache keyed entries on sha256(file content + path) alone, with no component for the extraction prompt that produced them. After an upgrade that changed the prompt, every unchanged file was a cache hit and replayed the older prompt's extraction: the run exited 0, cost.json looked cheap, and the graph silently carried two prompt generations side by side. The reporter saw 506 of 512 docs replay an older vintage on a rebuild they expected to be cold; deleting the whole cache was the only workaround. Graphify-Labs#1252 left the semantic cache unversioned on purpose — its entries are LLM output, and invalidating them on every release would re-bill extraction for unchanged files. Fingerprinting the prompt itself keeps both properties: entries survive releases that don't touch the prompt, and invalidate only when it actually changed. Semantic entries now live under cache/semantic/p{fingerprint}/, mirroring the AST cache's v{version}/ layout. Both extraction paths pass their prompt: the Python/CLI path from llm.py's _EXTRACTION_SYSTEM (shared by every backend), and the skill path via a new prompt_file argument in Step B0/B3 naming the references/extraction-spec.md the subagents were handed. The fingerprint normalizes line endings so a CRLF checkout isn't mistaken for a new prompt. Pre-existing entries predate fingerprinting and have unknowable vintage, so they are still served rather than re-billing a whole corpus on upgrade — but check_semantic_cache now warns with the count, turning "no signal at all" into a visible one. merge_existing refuses to fuse such an entry into a current-vintage write, which would mix two prompts inside one entry and then attest the result to a prompt that produced half of it. Old-fingerprint entries are pruned by liveness only, never swept wholesale the way stale AST versions are: two hosts with different prompts (verbose vs compact extraction-spec) can share one graphify-out/, and a wholesale sweep would have each run delete the other's entries and re-bill on every alternation. prune/clear/cached_files glob recursively so fingerprinted entries can't become unprunable orphans (the Graphify-Labs#1527 failure mode). The two monolith skills (aider, devin) inline their prompt instead of shipping a spec sidecar and stay on the unfingerprinted path for now.
Collaborator
|
Landed on v8 in 0d018b4 (ships in 0.9.18), cherry-picked so your authorship is preserved in the history. Thanks @SinghAman21 — this keys the semantic cache on a fingerprint of the extraction prompt, so an upgrade that changes the prompt invalidates only the entries it should, while releases that leave the prompt untouched keep their cache. Verified end to end: same prompt hits, a changed prompt re-extracts instead of replaying stale output, and the fingerprint is CRLF-insensitive. Credited in the changelog alongside @HunterMcGrew for the report. |
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.
fix(cache): key the semantic cache on the extraction prompt
Fixes #1939
What was the issue
The semantic cache keyed entries on
sha256(file content + "\x00" + relative path)and nothing else. Nothing in the key described which extraction prompt produced the entry.So when a release changed the prompt, every unchanged file was still a cache hit and replayed whatever the older prompt had extracted. Nothing flagged it: the run exited 0,
cost.jsonlooked cheap, and the resultinggraph.jsonquietly carried two prompt generations side by side — nodes shaped by an old prompt sitting next to nodes from the current one.The reporter hit this through the bundled skill's prompt: their
graphify-out/cache/semantic/held 880 entries across two vintages, and a rebuild they expected to be cold re-extracted only 6 of 512 docs — the other 506 replayed the old prompt. Deleting the entire cache was the only workaround they found, which forfeits the AST cache too.This was a gap, not an oversight. #1252 versioned the AST cache and deliberately left the semantic cache unversioned: its entries are LLM output, and invalidating them on every release would re-bill extraction for unchanged files. That reasoning is sound, and any fix has to preserve it. #1894 later added mode namespacing (
semanticvssemantic-deep), but still no prompt dimension.Approach
Fingerprint the extraction prompt and namespace entries by it, rather than by package version.
This keeps both properties the issue asks for: entries survive releases that don't touch the prompt (no re-bill on a patch release — #1252's constraint holds), and invalidate only when the prompt actually changed.
Entries live under
cache/semantic/p{fingerprint}/, mirroring the AST cache'sv{version}/layout, so the two caches now read as one idea with two different validity keys — version for code-produced entries, prompt for LLM-produced ones.Two decisions worth reviewer attention:
Pre-existing entries are served, not dropped. They predate fingerprinting, so their vintage is unknowable. Ignoring them would be strictly correct but would re-bill a whole corpus on upgrade (506 docs for the reporter). Instead they still hit, and
check_semantic_cachewarns with the count — turning the "no signal at all" the issue describes into a visible one.--forcere-extracts them. The honest limitation: such an entry keeps serving its unknown vintage until its content changes or someone forces it. The transition is gradual, not immediate.Old-fingerprint entries are pruned by liveness only, never swept wholesale the way
_cleanup_stale_ast_entriessweeps old AST versions. Two hosts with different prompts (the verbose vs compactextraction-spec.md) can share onegraphify-out/; a wholesale sweep would have each run delete the other's entries and re-bill on every alternation. Liveness keeps the total bounded by live docs × prompts seen.Changes
graphify/cache.py— the core.prompt_fingerprint(prompt)accepts prompt text or aPathto a prompt file. Normalizes line endings and trailing whitespace before hashing, so a CRLF checkout isn't mistaken for a prompt change (which would re-bill every Windows run).cache_dir(..., prompt_fp=)adds thep{fingerprint}/subdir for semantic kinds.prompt_fp=Noneyields the historical flat layout, so existing callers are byte-identical.load_cached/save_cached/check_semantic_cache/save_semantic_cachetakeprompt=(text) andprompt_file=(path). These are separate parameters on purpose: the skill callers are markdown snippets an agent copies with a path substituted in, and passing that path aspromptwould hash the path string — a fingerprint that is stable, plausible, and tracks nothing. A silent wrong fingerprint is the exact failure class being fixed.merge_existingreads withallow_legacy=False: fusing a pre-fingerprint entry into a current-vintage write would mix two prompts inside one entry and then attest the result to a prompt that produced half of it.prune_semantic_cache/clear_cache/cached_filesglob recursively, so fingerprinted entries can't become unprunable orphans (the Semantic cache is never pruned — orphan entries accumulate unbounded (AST cache is pruned, semantic isn't) #1527 failure mode).(path, size, mtime_ns)signature —check_semantic_cacheresolves the prompt once per file, so a 500-doc run would otherwise re-read and re-hash the same spec 500 times.graphify/llm.py— the per-chunk checkpoint stamps entries with_extraction_system(deep=deep_mode), the prompt every backend actually sends.graphify/cli.py—extractpasses the same prompt to both the cache read and the write (a mismatch would strand writes where the next read won't look).cache-checkgains--prompt-file.tools/skillgen/fragments/core/core.md+ 14 rendered skills — Step B0/B3 passprompt_file='SPEC_PATH', thereferences/extraction-spec.mdthe subagents are handed, with instructions for substituting it. Regenerated viapython -m tools.skillgen;--checkis clean. The two monolith skills (aider, devin) inline their prompt rather than shipping a spec sidecar and stay on the unfingerprinted path — called out as a follow-up.Tests — 12 new, covering: prompt change invalidates; unchanged prompt still hits; CRLF insensitivity; prompt × mode compose independently; legacy entries served with the count warning; fingerprinted entry beats legacy;
merge_existingwon't fuse vintages; prune/clear reach the subdirs; unreadableprompt_filewarns and falls back; an edited spec defeats the memo;cache-check --prompt-file; and a skillgen guard asserting every split host passesprompt_fileto both calls (they live ~80 lines apart in the rendered body, so adding it to one and not the other would silently disable the cache rather than fail loudly).A few existing tests asserted the flat layout (
glob("*.json"), unpromptedload_cached) and were updated to the nested layout — behavior change, not a weakened assertion.Why
The alternative fixes each break something the issue explicitly wants kept:
graphify-out/on upgrade — the reporter's current workaround. Forfeits the AST cache, and is easy to forget.Fingerprinting is the only option that invalidates precisely when correctness demands and never otherwise.
Verification
tests/test_ollama_retry_cap.py, confirmed failing identically on a clean tree (openaioptional extra absent from a stock dev venv — unrelated, documented separately).python -m tools.skillgen --check: clean, 134 artifacts match.Cache: 1 hit, 0 miss(no re-bill)Cache: 0 hit, 1 miss(re-extract, no stale replay)Cache: 3 hit, 0 missplus3 semantic cache entries predate extraction-prompt fingerprinting..., confirmed visible under default warning filterscache/semantic/pa55707ed205d/{hash}.jsonConclusion
An upgrade that changes the extraction prompt now re-extracts affected files instead of silently mixing vintages, and an upgrade that doesn't touch the prompt still costs nothing — the property #1252 protected. Users upgrading with an existing cache keep their entries and get a count of how many are of unknown vintage, so the silent case the issue reports is now either correct or visible.
Known follow-ups, deliberately not in this PR:
modeeither, so a skill-driven--mode deepstill writes into the plain namespace — the gap extract: semantic cache key ignores extraction mode, and extract has no --force, so --mode deep over a warm cache is a silent no-op #1894's changelog already anticipates as skill-side follow-up..graphify_versionstale-skill warning remains the signal for that.