feat(vision-proxy): cache image descriptions per session to cut cost and stabilize prompt caching - #1696
feat(vision-proxy): cache image descriptions per session to cut cost and stabilize prompt caching#1696FFengIll wants to merge 13 commits into
Conversation
|
Approved and agreed the cache helps — but to be precise, it only ignores image position on a cache hit. The miss path still anchors "latest" on #1640 anchors the latest-turn test on the last image-bearing message so the first sight of the image is described (and then cached by your PR). They compose cleanly; suggest merging #1640 first and threading |
You are absolutely right. |
…mage) Vision Proxy previously re-described every image in the latest message on every request, even byte-identical repeats within a retry/failover burst — wasting vision-model calls and, since descriptions are non-deterministic, breaking downstream prompt-prefix caching. Adds a fixed-capacity in-memory LRU cache (internal/vision/visionproxy/describe_cache.go) keyed by (session, provider UUID, model, image content hash). Session is part of the key so a description is only reused within the same conversation — never across unrelated sessions, and never permanently stuck if one describe call came back wrong. Provider+model are part of the key so switching the configured vision service silently invalidates stale entries instead of serving another model's answer. Every image occurrence (latest or historical) now checks the cache first: a hit replaces immediately with the real description regardless of position; a historical miss still falls back to the fixed marker (no extra vision call); a latest-message miss goes through the describe fan-out as before and, only on success, writes the result to cache. VisionProxyProcessor.Process and Service.Apply gain a sessionID parameter, resolved independently inside applyVisionProxy via the existing resolveSessionID helper (a pure function of the gin context and typed request) — no reordering of the handlers' own session injection required.
Adds §10 to .design/vision-proxy.md covering the new describe cache: the problem it solves, why the cache key includes session (not just image content) and provider+model, the resulting change to the latest/historical splice decision, and the known IP-fallback session limitation it inherits from routing.ResolveSessionID. Partial pass only — the rest of the doc still references pre-split paths (internal/server/vision_proxy.go, internal/server/processor/...) from before the internal/vision + internal/protocolserver split; a full audit of the document is a separate follow-up.
Full re-check of .design/vision-proxy.md against current code after the internal/server → internal/protocolserver + internal/vision split (and the smart_routing → smartrouting package rename). Fixes, section by section: - §3.1: ExtensionVisionProxyService now lives in internal/constant/flag.go, not internal/server/config/flag.go's (nonexistent) VisionProxyServiceKey. - §4: the unified entry helper is ProtocolHandler.applyVisionProxy in internal/protocolserver/protocol_handler.go, not Server.applyVisionProxy in internal/server/vision_proxy.go (that file no longer exists; a dead, unused copy of the method survives on Server for now — noted as such). Rewrote the code samples to match the real Service.Apply/Resolve/ Processor.Process call chain, including the sessionID parameter this branch's cache work added. - §4.2: hook file is anthropic_message.go, not anthropic.go. - §4.3: processor lives in internal/vision/visionproxy/vision_proxy.go; it never routed through the smart-routing processor registry via a now-deleted registry (the registry itself is alive and used by other ops — only the proxy_vision-specific pieces were removed, see §7). Also: four request shapes are supported today (Responses API was added after this doc was last touched), not three. - §6.1: corrects a since-falsified claim that OpenAI's tool-role messages never carry images — a later fork change (#1609) added that, and vision proxy had to catch up. - §6.4: ctx now flows through Service.Apply as well. - §7: smart_routing → smartrouting throughout; clarifies the registry mechanism itself was not deleted, only proxy_vision's registration. - §8/§9: file index and test-coverage tables updated to current paths; §9 also picks up the four-shape and describe-cache test rows. Also drops the earlier §10 addition's link to .sdlc/docs/ — that directory is gitignored and the referenced spec file was never committed, so the doc now stands on its own instead of pointing at an untracked path.
README.md's wiring diagram, pipeline description, and fail-strip table predated this branch's cache work and had drifted further from stale signatures (NewServiceFromPool's dropped logger param, an old smart_routing package path). Updates: - Wiring diagram and Process pipeline walkthrough now show the cache lookup as the first decision point for every image, not just the latest/historical split. - Fail-strip table gains a cache-hit row; protocol coverage table notes OfTool as an image source alongside OfUser for ChatCompletion. - Testing section pointed at a visionproxytest/ package and an internal/server/openai_responses_vision_test.go that don't exist; replaced with the actual doubles (stub.go) and test files. - Dropped the 'Out of scope: caching describe results' bullet — this branch shipped exactly that. Also two small code-adjacent fixes found while writing the above: - stub.go's package doc comment still said 'package visionproxytest' (stale from before this file was merged into the main package); and NewProcessor() built a VisionProxyProcessor with no cache field, so it would have silently fallen back to sharing defaultDescribeCache across every caller. Gave it its own isolated cache, matching the pattern used for in-package test helpers. - vision_proxy_e2e_test.go's run command still named the old internal/server/module/visionproxy/... package path.
Every collectXxx call site repeated the same four-field imageRef literal (mediaType/b64/remoteURL/cacheKey built from the same three values, splice callback). Extracted newImageRef(session, usable, mediaType, b64, remoteURL, splice) to build it in one place across all 6 call sites — removes ~50 lines of repetition and the risk of the cache key ever being built from a different triple than the one actually described. Also: newVisionCacheKey now only hashes the base64 path when there isn't a remoteURL, instead of always hashing first and conditionally overwriting. And: two comments pointed at .sdlc/docs/vision-vision-proxy-description-cache-20260902.spec.md, which is gitignored and was never committed — redirected both to .design/vision-proxy.md §10, which is the actual committed home for this design. No behavior change; go test ./internal/vision/visionproxy/... -race and go vet ./... both clean.
… error report Replace the neutral "[image: (description unavailable)]" placeholder with wording that tells the downstream model the image was lost to a gateway failure and to relay that to the user — so failures get attributed to the proxy, not to the user's image or client. Update test assertions to match.
…r report Sync both READMEs and .design/vision-proxy.md with the new marker wording and record the intent: it is a deliberate proxy-side error report, not a neutral placeholder.
8007ac2 to
1f0f9ed
Compare
|
Rebased onto On top of that the branch now carries a persistent tier: the memory LRU fronts a Generated by Claude Code |
The describe cache keyed by (session, service, image) kept the downstream prompt prefix stable only while its in-memory entry survived. A gateway restart or an LRU eviction sent every image a live conversation carried back through the vision model: a session with a dozen screenshots paid a dozen vision calls again, and the downstream model received a dozen freshly worded descriptions, which is exactly the prefix break the cache exists to prevent. Descriptions now live in a vision_descriptions table on the StoreManager's shared tingly.db connection. There is no age expiry: a row is a few hundred bytes and only gains value the longer its conversation keeps coming back. The one safety valve is a 100000-row ceiling, least recently used first, applied at boot and then at most hourly from the write path; the last_used_at touch on a hit is throttled to once an hour per row. The session component of the key becomes "<source>:<value>" rather than SessionID.String(), which also carried the client IP: entries that outlive the process must not be invalidated by the user changing networks mid-conversation. Boot degrades to a process-local map when the store cannot be opened, and store read/write errors are logged and treated as a miss; the request path never fails because of the cache. The dead Server.applyVisionProxy is replaced by the store wiring helper. Concurrent requests carrying the same new image are deliberately not coalesced, to keep description diversity. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016rH3odGKA8ZSq19Ugxpjwb
…t-message test The "describe the latest message, mark the rest" rule had two problems once descriptions were cached. A historical image that missed the cache could never be described again: it is never the latest message again, so after a vision-model switch, a failed describe, enabling the proxy mid-conversation, or a store eviction, those images stayed markers for the rest of the session. And "which message is the latest" was a position heuristic that every protocol shape had to maintain separately (#1640 patched one such case for Claude Code's trailing system message). Position now only ranks cache misses; it never decides eligibility. Every image checks the cache first. Misses are collected in message order, reversed to newest-first, and the first describeLimit of them (default 8, TINGLY_VISION_DESCRIBE_LIMIT) go to the vision upstream in that order; older ones get a deferral marker with no call this turn. Described images are cached and stop consuming slots, so each following turn spends its slots on the next-oldest misses and a fully uncached history converges to fully described within a few turns, while one request's cost and latency stay bounded. latestImageAnchor and the lastIdx/isLast plumbing in the four collect walks are removed; the bound lives in Process alone. Tests that pinned "latest described, history marked" now pin the same shape with limit 1, plus new convergence, slot-accounting and env-parsing cases. Also from a review pass: URL-sourced keys are hashed rather than stored verbatim (a presigned URL of several kilobytes went into the unique index); with no usable vision service every miss is stripped uniformly instead of being split into "failed" and "not yet described" markers that mean the same thing; and a lookup under an empty service key skips the store, since nothing is ever written there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016rH3odGKA8ZSq19Ugxpjwb
…d describes With the newest-first bound, an image that always fails to describe (dead URL, bytes the upstream rejects) retried on every turn and held a describe slot every turn, starving every older image behind it. A failed describe is now remembered in memory for describeFailureTTL (10 minutes): within it the image is fail-stripped with no upstream call and no slot; after it a transient failure gets its retry and a recovery is cached as usual. The negative cache is never persisted, so a restart retries everything once. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016rH3odGKA8ZSq19Ugxpjwb
…e's single positive tier A request looks up every image its conversation carries, but a SQLite point lookup on the unique index costs tens of microseconds — less than hashing one image's base64 and far below the downstream model's latency. The memory tier in front of it bought that speed with two sources of truth, promotion and write-through logic, a global default instance, and a third capacity number; a review pass had already tripped over one of its details. The DescribeStore is now the only place a description lives. The negative cache stays memory-only by design. A bounded in-process map implements DescribeStore for tests and as the fallback when no database is available at boot. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016rH3odGKA8ZSq19Ugxpjwb
…g, xxhash keys, and a scenario matrix Three gaps from a scenario walk-through of the describe cache: - A describe ran on the caller's request context alone, so a hung vision upstream held the whole request until the client gave up. Each call is now bounded by TINGLY_VISION_DESCRIBE_TIMEOUT (default 60s); a timeout is a failure like any other and is negative-cached. A describe cut short by the caller's own context (user abort, client retry) is not: the image was never given a fair try, so the resend goes upstream. - The same screenshot in two tool results of one request was described twice with two different texts, only one of which the cache kept, so the other position changed on the next turn. Repeated images now fold into one describe spliced into every position. - Every image a conversation carries is hashed on every request; a 2 MB base64 cost ~8 ms under SHA-256, a quarter second per turn for a 30-screenshot session. Keys now use xxhash64 plus length; they are scoped per session and service, so the collision space is a few dozen images. vision_scenario_test.go drives the proxy through Service.Apply with Claude Code's real multi-turn message shape and a client whose wording changes on every call, so any re-describe is visible: tool loop, restart, model switch, proxy enabled mid-conversation, outage then recovery, duplicate screenshot, permanently bad image. .design/vision-proxy.md §11 is the matrix those tests implement, plus the scenarios judged to need nothing. Review follow-ups: the boot-time prune arms the hourly throttle, and two leftover references to the removed memory tier are reworded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016rH3odGKA8ZSq19Ugxpjwb
9165acf to
368ca70
Compare
Summary
Vision Proxy re-described every image in the latest message on every request, even byte-identical repeats within a retry/failover burst — wasting vision-model calls and, since descriptions are non-deterministic, breaking downstream prompt-prefix caching.
Adds a session-scoped describe cache so repeats within the same conversation are free and consistent, and brings the design docs up to date with it.
Closes #1692.
Key Changes
(session, provider, model, image-hash)— a resent image within a conversation (retry, failover, repeated tool call) skips the vision call and gets byte-identical replacement text. Content is xxhash64 + length of the base64 text (with media type) or of the URL; a cryptographic digest cost a 30-screenshot session a quarter second per turn.tingly.db, single tier: descriptions live in avision_descriptionstable, so a restart does not re-describe every image a live conversation carries. No in-memory LRU in front of it — a SQLite point lookup is cheaper than hashing the image, and one source of truth beats promotion/write-through logic. No age expiry; only a 100k-row LRU ceiling as a safety valve. The session key issource:value(no client-IP backup) so a network change keeps hitting. A bounded in-process map is the fallback when no database is available.TINGLY_VISION_DESCRIBE_LIMIT(default 8) are described and cached, in that order; older ones get a deferral marker this turn. A fully uncached history converges to fully described within a few turns while each request stays bounded.latestImageAnchorand the per-protocollastIdxplumbing are removed.TINGLY_VISION_DESCRIBE_TIMEOUT(default 60s) instead of running on the request context alone; a hung vision model no longer holds the request until the client gives up.vision_scenario_test.godrives the proxy throughService.Applywith Claude Code's real multi-turn message shape and a vision client whose wording changes on every call, so any re-describe shows up as a text change — tool loop, restart, model switch, proxy enabled mid-conversation, upstream outage then recovery, duplicate screenshot, permanently bad image..design/vision-proxy.md§11 is the matrix those tests implement, plus the scenarios judged to need nothing..design/vision-proxy.mdand the package README updated end-to-end to match current code and describe the cache design (key rationale, storage, retention, known limitations).Notes
tingly.db), not shared across instances.metadata.user_id) share one cache scope per client IP, asResolveSessionIDalready does for affinity; documented, not changed.