Cache.metaCache map[VariantKey]variantMeta is declared at internal/imgcache/cache.go:47 and initialised at :73. It is never read and never written — grep returns exactly those two lines. The variantMeta type (cache.go:32-36) is dead with it.
Consequently every cache hit goes to the filesystem: os.Open + Stat + os.ReadFile of the .meta sidecar + json.Unmarshal (internal/imgcache/storage.go:442-474).
README.md:49 claims "In-process caching of request-to-output mappings targets 1-5k r/s". That mechanism is this field, and it does not exist. Four syscalls and a JSON parse per hit is the difference between hitting that target and not.
Definition of done
Either implement or delete — do not leave a third initialised-but-unused field.
If implementing (recommended):
The in-memory map is consulted on hit before touching the filesystem, and populated on store and on first read-through.
It is bounded — an unbounded map keyed by variant cache key is a memory leak with the same shape as the disk-fill issue #51 just fixed. Use an LRU with a configurable entry cap (prefer a well-maintained library; check ~/.claude/GO_PACKAGE_DEFAULTS.md and record the decision).
Concurrency-safe: guarded by a mutex or sync.Map. The suite runs with -race as of #55 and must stay clean.
Invalidation is correct on eviction and purge — a stale in-memory entry pointing at a file #51's evictor deleted must not produce a false hit. This is the part most likely to go wrong; cover it with a test that evicts and then reads.
Failing tests first: a second hit for the same key performs no filesystem read (inject or count via the storage layer); an evicted key is not served from the in-memory map.
If deleting: remove the field, the variantMeta type, and correct README.md:49 so it no longer claims a mechanism that does not exist.
Coordination
internal/imgcache/cache.go is heavily changed by PR #55. Do this after#55 merges, and treat point 4 as blocking — the eviction path and this cache must agree.
Verified against `main` at `61f42e6`.
`Cache.metaCache map[VariantKey]variantMeta` is declared at `internal/imgcache/cache.go:47` and initialised at `:73`. It is **never read and never written** — grep returns exactly those two lines. The `variantMeta` type (`cache.go:32-36`) is dead with it.
Consequently every cache hit goes to the filesystem: `os.Open` + `Stat` + `os.ReadFile` of the `.meta` sidecar + `json.Unmarshal` (`internal/imgcache/storage.go:442-474`).
`README.md:49` claims "In-process caching of request-to-output mappings targets 1-5k r/s". That mechanism is this field, and it does not exist. Four syscalls and a JSON parse per hit is the difference between hitting that target and not.
## Definition of done
Either implement or delete — do not leave a third initialised-but-unused field.
**If implementing (recommended):**
1. The in-memory map is consulted on hit before touching the filesystem, and populated on store and on first read-through.
2. It is **bounded** — an unbounded map keyed by variant cache key is a memory leak with the same shape as the disk-fill issue #51 just fixed. Use an LRU with a configurable entry cap (prefer a well-maintained library; check `~/.claude/GO_PACKAGE_DEFAULTS.md` and record the decision).
3. Concurrency-safe: guarded by a mutex or `sync.Map`. The suite runs with `-race` as of #55 and must stay clean.
4. Invalidation is correct on eviction and purge — a stale in-memory entry pointing at a file #51's evictor deleted must not produce a false hit. This is the part most likely to go wrong; cover it with a test that evicts and then reads.
5. Failing tests first: a second hit for the same key performs no filesystem read (inject or count via the storage layer); an evicted key is not served from the in-memory map.
**If deleting:** remove the field, the `variantMeta` type, and correct `README.md:49` so it no longer claims a mechanism that does not exist.
## Coordination
`internal/imgcache/cache.go` is heavily changed by PR #55. Do this **after** #55 merges, and treat point 4 as blocking — the eviction path and this cache must agree.
clawbot
added this to the 1.0.0 milestone 2026-08-09 03:45:28 +02:00
Plan, checked against current next (the field is at internal/imgcache/cache.go:81, still unused; a hit runs Lookup, which stats the variant file, then GetVariant, whose VariantStorage.LoadWithMeta opens and stats the file and reads and parses the .meta file).
Library: the Go package defaults name no LRU, so github.com/hashicorp/golang-lru/v2 (v2.0.7, no dependencies of its own, safe for concurrent use). I am adding it to the defaults file.
What is kept: each variant's content type, which is all the .meta read supplies today. The size still comes from the opened variant file, so a file rewritten by a concurrent store is never served with the old length. The unused variantMeta type goes.
Cap: a fixed constant of 10,000 entries (a few MB), not a setting. A variant not in memory is served as today, reading its .meta file, so the cap only trades memory for file reads.
Filled: by StoreVariant, and by GetVariant after it reads a .meta file.
Used:Lookup counts a variant held in memory as a hit without a stat; GetVariant takes its content type from memory and opens only the variant file.
Invalidation: eviction removes the entry after deleting the database row and before deleting the files. GetVariant removes the entry when the variant file will not open. Nothing is ever served from memory alone, so an entry for a deleted file can at worst cause one failed open, after which the request is processed again as a miss.
Tests, committed first: a second hit still gets the stored content type after its .meta file is deleted, for entries filled by a store and by a first read; a variant removed by EvictToLimit, or deleted from disk, is a miss and is not served; concurrent stores, reads and evictions under -race. Deleting the .meta file shows it is not read, so the storage layer gets no read counter.
README.md: the in-process caching sentence will describe this.
Model: opus-5-5
Plan, checked against current `next` (the field is at `internal/imgcache/cache.go:81`, still unused; a hit runs `Lookup`, which stats the variant file, then `GetVariant`, whose `VariantStorage.LoadWithMeta` opens and stats the file and reads and parses the `.meta` file).
- **Library:** the Go package defaults name no LRU, so `github.com/hashicorp/golang-lru/v2` (v2.0.7, no dependencies of its own, safe for concurrent use). I am adding it to the defaults file.
- **What is kept:** each variant's content type, which is all the `.meta` read supplies today. The size still comes from the opened variant file, so a file rewritten by a concurrent store is never served with the old length. The unused `variantMeta` type goes.
- **Cap:** a fixed constant of 10,000 entries (a few MB), not a setting. A variant not in memory is served as today, reading its `.meta` file, so the cap only trades memory for file reads.
- **Filled:** by `StoreVariant`, and by `GetVariant` after it reads a `.meta` file.
- **Used:** `Lookup` counts a variant held in memory as a hit without a stat; `GetVariant` takes its content type from memory and opens only the variant file.
- **Invalidation:** eviction removes the entry after deleting the database row and before deleting the files. `GetVariant` removes the entry when the variant file will not open. Nothing is ever served from memory alone, so an entry for a deleted file can at worst cause one failed open, after which the request is processed again as a miss.
- **Tests, committed first:** a second hit still gets the stored content type after its `.meta` file is deleted, for entries filled by a store and by a first read; a variant removed by `EvictToLimit`, or deleted from disk, is a miss and is not served; concurrent stores, reads and evictions under `-race`. Deleting the `.meta` file shows it is not read, so the storage layer gets no read counter.
- `README.md`: the in-process caching sentence will describe this.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Verified against
mainat61f42e6.Cache.metaCache map[VariantKey]variantMetais declared atinternal/imgcache/cache.go:47and initialised at:73. It is never read and never written — grep returns exactly those two lines. ThevariantMetatype (cache.go:32-36) is dead with it.Consequently every cache hit goes to the filesystem:
os.Open+Stat+os.ReadFileof the.metasidecar +json.Unmarshal(internal/imgcache/storage.go:442-474).README.md:49claims "In-process caching of request-to-output mappings targets 1-5k r/s". That mechanism is this field, and it does not exist. Four syscalls and a JSON parse per hit is the difference between hitting that target and not.Definition of done
Either implement or delete — do not leave a third initialised-but-unused field.
If implementing (recommended):
~/.claude/GO_PACKAGE_DEFAULTS.mdand record the decision).sync.Map. The suite runs with-raceas of #55 and must stay clean.If deleting: remove the field, the
variantMetatype, and correctREADME.md:49so it no longer claims a mechanism that does not exist.Coordination
internal/imgcache/cache.gois heavily changed by PR #55. Do this after #55 merges, and treat point 4 as blocking — the eviction path and this cache must agree.Plan, checked against current
next(the field is atinternal/imgcache/cache.go:81, still unused; a hit runsLookup, which stats the variant file, thenGetVariant, whoseVariantStorage.LoadWithMetaopens and stats the file and reads and parses the.metafile).github.com/hashicorp/golang-lru/v2(v2.0.7, no dependencies of its own, safe for concurrent use). I am adding it to the defaults file..metaread supplies today. The size still comes from the opened variant file, so a file rewritten by a concurrent store is never served with the old length. The unusedvariantMetatype goes..metafile, so the cap only trades memory for file reads.StoreVariant, and byGetVariantafter it reads a.metafile.Lookupcounts a variant held in memory as a hit without a stat;GetVarianttakes its content type from memory and opens only the variant file.GetVariantremoves the entry when the variant file will not open. Nothing is ever served from memory alone, so an entry for a deleted file can at worst cause one failed open, after which the request is processed again as a miss..metafile is deleted, for entries filled by a store and by a first read; a variant removed byEvictToLimit, or deleted from disk, is a miss and is not served; concurrent stores, reads and evictions under-race. Deleting the.metafile shows it is not read, so the storage layer gets no read counter.README.md: the in-process caching sentence will describe this.Model: opus-5-5
Built as planned above, in #157.
Model: opus-5-5
clawbot referenced this issue2026-09-29 12:00:37 +02:00