Invalidate cached local-file images in AI blocks when the file changes#12840
Invalidate cached local-file images in AI blocks when the file changes#12840zachlloyd wants to merge 1 commit into
Conversation
Local image files were cached by path only, with no modification-time or content check and no invalidation path, so re-displaying the same path in an AI response always returned the first-loaded image even after the file changed on disk. Add a LocalFileContentVersion (a NonZeroU64 hash of mtime + size) to AssetSource::LocalFile so the content version is part of both the decoded-bytes (AssetCache) and rendered (ImageCache) cache keys. The blocklist resolves it via AssetSource::with_local_file_content_version() inside the existing background link-detection task, so file metadata is read when the block resolves its image sources (creation/output completion), not on every render. Storing it as a NonZeroU64 keeps Option<LocalFileContentVersion> pointer-width so AssetSource does not grow. Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds an optional local-file content version to AssetSource::LocalFile and uses it for AI block image resolution so edited local images miss the existing path-only cache. The approach is wired through the blocklist image resolution path and leaves other callers on path-only caching.
Concerns
- Versioned local-file sources create a new
AssetCache/ImageCachekey for each detected file rewrite, but the existing eviction logic only boundsAssetSource::Rawentries. Repeatedly iterating on a large image can retain every previous decoded/rendered version until the cache is torn down.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| /// sensitive to on-disk changes so an edited file is re-read instead of | ||
| /// served stale. `None` preserves path-only caching for callers that do | ||
| /// not need invalidation. | ||
| content_version: Option<LocalFileContentVersion>, |
There was a problem hiding this comment.
content_version makes each rewrite of the same local path a distinct cache key, but the cache only evicts Raw assets today. Repeated image iteration can retain all old decoded/rendered versions, so evict or replace older LocalFile entries for the same path when loading a new version.
Description
Fixes a caching bug in inline image display in AI blocks (the PR #9993 feature): if an agent displays a local image, the file is then changed on disk, and the agent is asked to display the same path again, the second display showed the stale, first-loaded image instead of the updated file. This made it impossible to iterate on an image with the agent.
Root cause:
AssetSource::LocalFilewas keyed by path only.AssetCache::load_asset()reads a path once and never re-reads it, and the rendered-sizeImageCacheis keyed on the same source. There was no mtime/content check and no invalidation path forLocalFile(eviction only coveredAssetSource::Raw), so a changed file at the same path kept hitting the cached decode.Fix: include an optional content version in the local-file cache key.
LocalFileContentVersion(aNonZeroU64hash of last-modified time + file size) and acontent_version: Option<LocalFileContentVersion>field onAssetSource::LocalFile(crates/warpui_core/src/assets/asset_cache.rs). Because it is part of the source, it flows into both theAssetCache(decoded bytes) andImageCache(rendered) keys, so an edited file misses the cache and is re-read.AssetSource::with_local_file_content_version()reads the metadata; the AI block calls it inside the existing background link-detection task (app/src/ai/blocklist/block.rs), which populates the per-blockresolved_blocklist_image_sourcesmap.NonZeroU64soOption<LocalFileContentVersion>stays pointer-width (niche) andAssetSourcedoes not grow (a{u128,u64}version trippedclippy::large_enum_varianton an unrelated enum viaWarpTheme).Metadata is read only when the block resolves its image sources (block creation / output completion / restore / shell-change) — never on the per-streaming-token path and never on render. Renders reuse the cached resolved source, and the render-time fallback stays stat-free (
content_version: None). The stat also runs in the existing backgroundspawn_blockingtask, off the UI thread.This is a general
AssetCache/ImageCacheimprovement; non-blocklist callers (themes, notebooks) keep path-only caching (content_version: None), so their behavior is unchanged.https://www.loom.com/share/c0eab28c73f146bb9ba531d3ba5b8081
Linked Issue
#12802
ready-to-specorready-to-implement.Testing
Added 3 focused unit tests in
crates/warpui_core/src/assets/asset_cache_tests.rs:for_pathreturnsNonefor a missing filewith_local_file_content_version()leaves non-local sources unchangedcargo fmt -- --check,cargo check, andcargo clippy --tests -- -D warningsare clean for the changed crates and the app (warpwithlocal_fs); the new unit tests and the existing tests I touched pass.Could not run in this cloud sandbox: the full app test binary OOMs while linking (memory limit — it compiles fine), and the full wasm bundle build needs
clangplus app-level feature wiring unavailable here. The wasm changes are cfg-gated stubs (for_pathreturnsNoneon wasm). CI will run the full presubmit.I have manually tested my changes locally with
./script/runAgent Mode
CHANGELOG-BUG-FIX: Re-displaying a local image in an AI response now reflects edits made to the file on disk instead of showing a stale cached copy.
Conversation: https://staging.warp.dev/conversation/970500d6-1824-42c3-85a4-620b31258e19
Run: https://oz.staging.warp.dev/runs/019ed6bb-b701-78a8-8fa2-844979f25593
This PR was generated with Oz.