fix(ai,node-llama-cpp,google-gemini): five HIGH-severity cache-checkpoint correctness fixes (follow-up to #641)#645
Merged
sroussey merged 6 commits intoJul 21, 2026
Conversation
The prior renderer flattened a checkpoint prefix to raw "role: text" strings
via renderLlamaCppPrefixText and preloaded that text — bypassing the model's
chat wrapper and silently dropping tool_use / tool_result blocks. Warm-up
tokens no longer matched what the consumer's generateResponse produced, so
cache reuse was defeated and any prior tool exchange in the prefix was lost
by the missing-state fallback.
Replace with a ChatHistoryItem[] renderer that reuses the existing
convertMessagesToChatHistory pipeline (extracted as a pure helper that skips
the trailing empty-user placeholder). Warm-up and every missing-state
fallback (Chat, TextGeneration, ToolCalling) now call
session.setChatHistory(history) then session.preloadPrompt("",
{ functions }), so the KV state is populated through the same wrapper the
consumer uses. Tools are handed to preloadPrompt via buildChatModelFunctions
so the wrapper embeds their descriptions identically. Deleted
renderLlamaCppPrefixText so no future path can silently drop tool blocks.
Test plan updated: flip the reconstructed-checkpoint preloadPrompts
expectation to [""] and assert the setChatHistory payload; add a
tool_use/tool_result preservation regression, plus dedicated unit tests for
the new render helpers (system-only, tools-only, mixed text+tool_use,
tool_result matching).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01413eP4Awsvn2HSXqr4MFYh
checkpointModelKey returns "" for a missing model_id, and validateParentCheckpoint short-circuited its mismatch check on either side being empty — so two keyless models on the same provider silently shared a fungible checkpoint slot (cross-model contamination) and an emit path could mint a slot with no identity at all. Add requireCheckpointModelKey (throws TaskConfigurationError for a keyless model) and route every mint / validate site through it: the parent-validate site (mismatch comparison is now unconditional as defense in depth), the resolveCheckpointSession pre-dispatch gate (so emit paths fail before createSession mints), and CacheCheckpointTask.prepareCheckpoint's mint site (the returned key is threaded straight into registerCheckpoint). Tests: dedicated requireCheckpointModelKey unit coverage plus a model-key fail-closed L2 describe covering all five paths (warm-up rejection with no run-fn invocation, parent-consume rejection before dispatch, keyless-vs-keyless slot contamination, emit path rejection before createSession, and a regression on the existing keyed-mismatch path). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01413eP4Awsvn2HSXqr4MFYh
…n atomic steal get + delete on the session map bracketed async work in both _TextGeneration and _ToolCalling stream paths; two concurrent consumers of the same immutable checkpoint id both observed the same cached state and both called .generate() on the shared LlamaContextSequence — a live sequence advances in place, so the second consumer's turn was corrupted. withModelInUse is a refcount, not a mutex, so it did not close the window. Add stealLlamaCppSession(id) in LlamaCpp_Runtime — a synchronous Map.get + Map.delete that is race-free on JS's single thread — and route both stream paths through it when consuming an immutable checkpoint (isCheckpoint && !ownedSession). Under contention exactly one caller wins; losers observe undefined and re-encode via the existing missing-state fallback (which acquires its own sequence). The ownership-tracking flag is now decided at the same point as the steal-vs-get split, dropping the downstream flip-flop that redundantly deleted the map entry on a hit. AiChatTask's ownedSession mode is the caller's mutable session (never a checkpoint), so LlamaCpp_Chat continues to use the non-consuming getLlamaCppSession — documented on the lookup site so a future refactor does not "unify" the two paths back into a shared race. Tests: stealLlamaCppSession unit coverage (atomic get+delete on hit, no-op on unknown id); a race test in the ToolCalling checkpoint suite covering the two-consumer scenario (winner reuses warmed sequence, loser re-encodes with its own) plus an ownedSession regression asserting racing ownedSession runs do not evict each other's map entries; and a new TextGenerationCheckpoint fixture mirroring the ToolCalling one so the text-generation path has parity coverage. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01413eP4Awsvn2HSXqr4MFYh
The Gemini cache-checkpoint warm-up wrapped ai.caches.create in a blanket try/catch that logged and swallowed every error, so a caller-initiated abort, a 401/403 auth failure, a 429 quota, and a 5xx transport error all silently degraded to the same "no cache, replay inline" fate as a genuine prefix-too- small 400. It also failed to clean up when a resource had been minted but a downstream step (bookkeeping / write) threw, leaving a server-side CachedContent billing until TTL. Classify the error into abort / degrade / throw. Abort and throw rethrow (with a best-effort delete of any resource whose name we saw); only a 400 / INVALID_ARGUMENT whose message names the too-small-prefix / cache-unsupported condition degrades to inline replay. Track `createdName` across the try / catch so the partial-delete cleanup can fire. Gemini_CacheStore gets a matching `createdAtMs` field on each entry, an `isGeminiCacheEntryStale` helper (default 3_500_000 ms — a hair under the 1h TTL used at write time), and a `deleteGeminiCachedContentLocal` for runtime-only eviction. These land here so PR #641's follow-up (H2, the consume-side fallback) can chain on top. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UC9LGu3iokhuhAB46yt1ee
…achedContent Gemini's explicit CachedContent is TTL-bound (~1h at creation) and can also vanish server-side outside the consumer's control. The text.generation and tool-calling run-fns treated the cache handle as guaranteed to resolve: an entry near-TTL still went out with `cachedContent`, and a NOT_FOUND surfaced as a hard failure to the caller. Add Gemini_CachedContentFallback: a proactive stale check (evicts the runtime-local entry before the request, leaves the server-side to its own TTL) plus a reactive NOT_FOUND catch that evicts (locally + best-effort server delete), rebuilds the request inline (prefix + tail), and retries once. Refactor both run-fns to share `buildCachedRequest` / `buildInlineReplayRequest` locals so the retry rebuilds an identical shape. Streaming contract unchanged (providers still yield text-delta/object-delta then finish; no accumulation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UC9LGu3iokhuhAB46yt1ee
…fake session
The L1 checkpoint-prefix-render fix routes the missing-state fallback in
LlamaCpp_Chat through session.setChatHistory(history) + preloadPrompt(""),
matching the real LlamaChatSession API. The checkpoint mocks under
ai-provider-nodellama were updated to stub setChatHistory, but the
FakeLlamaChatSession in this ai-provider suite was missed, so the
checkpoint-seeded owned-session case threw "setChatHistory is not a
function". Add the no-op stub to align the fake with the real session.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YMyipztz9RFDp4aZPSNqEV
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sroussey
merged commit Jul 21, 2026
e40442b
into
ai-provider-cache-checkpoints
19 of 20 checks passed
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
2 participants
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.
Summary
Consolidates the five HIGH-severity cache-checkpoint correctness fixes previously split across #642 (node-llama-cpp / ai) and #643 (google-gemini). Both of those targeted
main/ stacked awkwardly on the branch; this branch is cut directly fromai-provider-cache-checkpointsso the diff is exactly the five fixes and nothing else. Each fix is a self-contained commit with targeted tests.From #642 — node-llama-cpp / ai
L1 — LlamaCpp checkpoint prefix rendered through the model's chat template.
renderLlamaCppPrefixTextflattened the prefix to raw"role: text"strings and silently droppedtool_use/tool_resultblocks, so warm-up KV tokens no longer matched the consumer's chat-template rendering — cache reuse was defeated and prior tool exchanges were lost. Replaced with aChatHistoryItem[]renderer routed throughsetChatHistory+preloadPrompt("", { functions }); the raw-text renderer is deleted so no future path can silently drop tool blocks.L2 — CheckpointRegistry model-key fail-closed.
checkpointModelKeyreturned""for a missingmodel_idandvalidateParentCheckpointshort-circuited its mismatch check when either side was empty — two keyless models on the same provider shared a fungible checkpoint slot. AddedrequireCheckpointModelKey(throwsTaskConfigurationError), routed all mint/validate sites through it, and made the mismatch comparison unconditional as defense in depth.L3 — LlamaCpp checkpoint concurrent-consumer race.
get+deleteon the session map bracketed async work in_TextGenerationand_ToolCalling; two concurrent consumers of the same checkpoint id both generated on the sharedLlamaContextSequence. AddedstealLlamaCppSession(id)(synchronous get+delete, race-free on JS's single thread); under contention exactly one consumer wins, losers deterministically re-encode via the existing fallback.From #643 — google-gemini
H1 — Narrow the warm-up catch. The warm-up wrapped
ai.caches.createin a blanket try/catch that swallowed every error, so an abort, a 401/403, a 429, and a 5xx all degraded to the same "no cache, replay inline" fate as a genuine prefix-too-small 400. IntroducedclassifyGeminiCacheError(abort / degrade / throw): abort and throw rethrow with a best-effort delete of any minted resource; only a 400/INVALID_ARGUMENT naming the too-small-prefix / unsupported condition degrades to inline replay. AddscreatedAtMs,isGeminiCacheEntryStale, anddeleteGeminiCachedContentLocaltoGemini_CacheStore.H2 — Fallback in the two consume paths. The consumers treated the cache handle as guaranteed to resolve. Added
Gemini_CachedContentFallback.generateGeminiStreamWithCacheFallback: a proactive stale check up front and, on a reactive 404/NOT_FOUND, evict (locally + best-effort server delete), rebuild inline, and retry once. Streaming contract unchanged.Verification (this branch, cut from
ai-provider-cache-checkpoints)bun run build:types— clean across all 40 packagesbun scripts/test.ts provider-nodellama vitest— 81 passed, 4 skipped (L1/L3)CacheCheckpoint.test.ts— passes incl. therequireCheckpointModelKey/ model-key fail-closed coverage (L2)OpenAIGeminiCheckpointParams.test.ts+Gemini_SessionDispose.test.ts— passes (H1/H2, no regressions)Provenance
Commits are cherry-picks of the fix commits from the two prior PRs, replayed cleanly onto the checkpoint branch:
9520ece← fix(ai,node-llama-cpp): three HIGH-severity checkpoint correctness fixes (follow-up to #641) #642 L1 (a64b086)b2570af← fix(ai,node-llama-cpp): three HIGH-severity checkpoint correctness fixes (follow-up to #641) #642 L2 (4a14d26)9b8077a← fix(ai,node-llama-cpp): three HIGH-severity checkpoint correctness fixes (follow-up to #641) #642 L3 (b1cdb68)e2703d8← fix(google-gemini): two HIGH-severity cache-checkpoint error-handling fixes (follow-up to #641) #643 H1 (46405c9)510c851← fix(google-gemini): two HIGH-severity cache-checkpoint error-handling fixes (follow-up to #641) #643 H2 (04fd0b9)Supersedes and closes #642 and #643.
🤖 Generated with Claude Code
https://claude.ai/code/session_01YMyipztz9RFDp4aZPSNqEV
Generated by Claude Code