From eb784d7ae6c889c9a395f00e18af078bde292801 Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 19 Jun 2026 18:58:33 +0200 Subject: [PATCH] feat(coding-agent): refined cache invalidation to require prior warm read - Tighten invalidation logic to only trigger when a demonstrably warm cache (previously read) goes cold. - Ignore cold transitions following write-only turns to prevent spurious markers during initial cache warming or after natural TTL expiry. - Update test suite to verify that consecutive cold turns following an initial write do not trigger invalidation alerts. --- packages/coding-agent/CHANGELOG.md | 4 ++ .../components/cache-invalidation-marker.ts | 46 +++++++++++++------ .../test/cache-invalidation-marker.test.ts | 13 ++++-- 3 files changed, 43 insertions(+), 20 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index caafcff95..db3fc1d13 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Cache-miss marker no longer fires on a cold turn whose predecessor only *wrote* the prompt cache (never read it back). The session's opening request always writes the prefix with `cacheRead 0`, so a long-running first tool call (e.g. `gh run watch`) that outlived the provider's cache TTL surfaced a spurious `⊘ cache miss` divider right under the opening message. The marker now requires the previous turn to have actually read a warm prefix, so it flags only a demonstrably working cache going cold — and collapses a run of consecutive cold turns to a single marker at the moment the cache broke. + ## [16.1.3] - 2026-06-19 ### Changed diff --git a/packages/coding-agent/src/modes/components/cache-invalidation-marker.ts b/packages/coding-agent/src/modes/components/cache-invalidation-marker.ts index daba03a02..6acda1645 100644 --- a/packages/coding-agent/src/modes/components/cache-invalidation-marker.ts +++ b/packages/coding-agent/src/modes/components/cache-invalidation-marker.ts @@ -4,9 +4,9 @@ import { formatNumber } from "@oh-my-pi/pi-utils"; import { theme } from "../../modes/theme/theme"; /** - * Minimum cached prefix (read + write) the previous turn must have established - * before a collapse on the current turn counts as an invalidation. Filters out - * tiny contexts and providers below the cacheable-prefix floor, where a zero + * Minimum prefix the previous turn must have READ back from cache before a + * collapse on the current turn counts as an invalidation. Filters out tiny + * contexts and providers below the cacheable-prefix floor, where a zero * `cacheRead` is expected rather than a reset. */ const MIN_CACHE_FOOTPRINT = 2048; @@ -18,25 +18,41 @@ export interface CacheInvalidation { } /** - * Decide whether `current` turn lost the prompt cache that `prev` established. + * Decide whether `current` turn lost a *working* prompt cache that `prev` was + * reusing. * * The provider reports a warm prefix as `cacheRead`; a model/thinking/tool/ * system-prompt change (or a history rewrite) breaks the prefix, so the next - * request reads nothing from cache and re-pays for the whole prompt. We detect - * that as: the previous turn cached a meaningful prefix, yet this turn's + * request reads nothing from cache and re-pays for the whole prompt. We flag + * only the transition where a demonstrably warm cache goes cold: the previous + * turn must have actually READ a meaningful prefix back, and this turn's * `cacheRead` collapsed to zero while it still reprocessed a non-trivial prompt. - * Returns `undefined` (no marker) for the first turn, tiny contexts, turns - * that reused any cache, and — crucially — turns on providers with *implicit* - * best-effort caching. Only an explicit, prefix-controlled cache (Anthropic / - * Bedrock `cache_control`) re-creates the prefix on a cold turn (`cacheWrite > - * 0`); implicit caches (Google / OpenAI / Fireworks) report `cacheWrite: 0` and - * drop `cacheRead` to zero intermittently as routine propagation noise that - * self-heals the next turn, so flagging it would be a false positive. + * + * Requiring a prior warm read is deliberate. A turn that merely WROTE the prefix + * (`cacheRead` 0) has not proven the cache is live — that is the session's first + * request, or a re-write after expiry — so a following cold turn there is + * expected, not an invalidation the user caused (e.g. a long-running first tool + * call outliving the provider's 5-minute cache TTL surfaced a spurious "cache + * miss" right under the opening message). It also collapses a run of consecutive + * cold turns to the single marker at the moment the cache actually broke, instead + * of repeating the banner on every turn while it re-warms. + * + * Returns `undefined` (no marker) for the first turn, turns whose predecessor + * never read a warm prefix, tiny contexts, turns that reused any cache, and — + * crucially — turns on providers with *implicit* best-effort caching. Only an + * explicit, prefix-controlled cache (Anthropic / Bedrock `cache_control`) + * re-creates the prefix on a cold turn (`cacheWrite > 0`); implicit caches + * (Google / OpenAI / Fireworks) report `cacheWrite: 0` and drop `cacheRead` to + * zero intermittently as routine propagation noise that self-heals the next + * turn, so flagging it would be a false positive. */ export function detectCacheInvalidation(prev: Usage | undefined, current: Usage): CacheInvalidation | undefined { if (!prev) return undefined; - const prevFootprint = prev.cacheRead + prev.cacheWrite; - if (prevFootprint < MIN_CACHE_FOOTPRINT) return undefined; + // Only flag a warm→cold transition: the previous turn must have actually read + // a meaningful prefix from cache. A write-only predecessor (first request, or + // a re-write after expiry) has not proven the cache is live, so a cold turn + // behind it is expected — not an invalidation worth surfacing. + if (prev.cacheRead < MIN_CACHE_FOOTPRINT) return undefined; // Any cache reuse this turn means the prefix survived (at least partly). if (current.cacheRead > 0) return undefined; // Only an explicit, prefix-controlled cache re-creates the prefix on a cold diff --git a/packages/coding-agent/test/cache-invalidation-marker.test.ts b/packages/coding-agent/test/cache-invalidation-marker.test.ts index 321484084..d6aa3a378 100644 --- a/packages/coding-agent/test/cache-invalidation-marker.test.ts +++ b/packages/coding-agent/test/cache-invalidation-marker.test.ts @@ -34,16 +34,19 @@ describe("detectCacheInvalidation", () => { expect(detectCacheInvalidation(prev, current)).toEqual({ reprocessedTokens: 50_999 }); }); - it("flags a second consecutive cold turn using the prior turn's cacheWrite as footprint", () => { - // The prior turn re-cached (cacheWrite) but read nothing; the next turn - // still reads nothing — a genuine repeat invalidation. + it("does not flag a cold turn whose predecessor only wrote the cache (never read it)", () => { + // The session's opening request writes the prefix (cacheRead 0); a long + // first tool call then outlives the provider's cache TTL, so the follow-up + // re-writes cold. The cache was never proven live, so this is expected + // warming/expiry — not a user-caused invalidation worth a marker right + // under the opening message. const prev = usage({ cacheRead: 0, cacheWrite: 50_900, input: 99 }); const current = usage({ cacheRead: 0, cacheWrite: 51_113, input: 16 }); - expect(detectCacheInvalidation(prev, current)).toEqual({ reprocessedTokens: 51_129 }); + expect(detectCacheInvalidation(prev, current)).toBeUndefined(); }); it("does not flag a turn that reused any cache", () => { - const prev = usage({ cacheRead: 0, cacheWrite: 51_113, input: 16 }); + const prev = usage({ cacheRead: 50_900, cacheWrite: 980 }); const current = usage({ cacheRead: 50_900, cacheWrite: 3_459, input: 2 }); expect(detectCacheInvalidation(prev, current)).toBeUndefined(); });