Get checked the negative cache before it looked for the variant on disk. One failed fetch of a source (as when another variant misses after the source blob was evicted and the upstream answers 503) then made every cached variant of that source answer 502 for 5 minutes, although the files were on disk. Get now serves a cached variant first and checks the negative cache only on a miss, before processing. A cache hit is counted as before; a refusal from the negative cache is still not counted as a miss.
The 502 entry under /v1/image/ in README.md now says an image already in the cache is still served. TODO.md has the entry.
Judgement call: the check sits in Get right after the variant lookup, not inside the fetch path, so a variant that is not on disk is refused during those 5 minutes even when its source blob is still cached, as today. The plan asks for a variant not on disk to answer as it does today.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/pixa/issues/245.
`Get` checked the negative cache before it looked for the variant on disk. One failed fetch of a source (as when another variant misses after the source blob was evicted and the upstream answers 503) then made every cached variant of that source answer 502 for 5 minutes, although the files were on disk. `Get` now serves a cached variant first and checks the negative cache only on a miss, before processing. A cache hit is counted as before; a refusal from the negative cache is still not counted as a miss.
The 502 entry under `/v1/image/` in `README.md` now says an image already in the cache is still served. `TODO.md` has the entry.
Judgement call: the check sits in `Get` right after the variant lookup, not inside the fetch path, so a variant that is not on disk is refused during those 5 minutes even when its source blob is still cached, as today. The plan asks for a variant not on disk to answer as it does today.
Model: opus-5-5
internal/imgcache/service.go, Get: Get still checks the negative cache before any processing, not only before a fetch. So a variant that is not on disk, but whose source image is still in the cache, answers 502 for 5 minutes, although pixa could make it without asking the upstream. This happens in the issue's own case. Two variants of an evicted source miss at the same time, and the upstream refuses one fetch (503) but answers the other. The source is then cached and the URL has a negative-cache row. The definition of done in #245 says the negative cache "then gates only fetches". Acceptable: consult the negative cache only where pixa is about to fetch from the upstream because no cached source can be used, and keep the comment above the check true of where it sits.
internal/imgcache/negative_cache_internal_test.go, TestService_Get_ServesCachedVariantOfNegativeCachedURL: the test's first request caches the source. Its last case then asks for another variant of that source and expects ErrNegativeCached, which locks in the refusal from finding 1. Acceptable: expect a variant made from a cached source to be served, and test the refusal with a variant whose source is not in the cache, for example after removing it as an eviction would.
Model: opus-5-5
**Review: failed (needs-rework)**
1. `internal/imgcache/service.go`, `Get`: `Get` still checks the negative cache before any processing, not only before a fetch. So a variant that is not on disk, but whose source image is still in the cache, answers 502 for 5 minutes, although pixa could make it without asking the upstream. This happens in the issue's own case. Two variants of an evicted source miss at the same time, and the upstream refuses one fetch (503) but answers the other. The source is then cached and the URL has a negative-cache row. The definition of done in https://git.eeqj.de/sneak/pixa/issues/245 says the negative cache "then gates only fetches". Acceptable: consult the negative cache only where pixa is about to fetch from the upstream because no cached source can be used, and keep the comment above the check true of where it sits.
2. `internal/imgcache/negative_cache_internal_test.go`, `TestService_Get_ServesCachedVariantOfNegativeCachedURL`: the test's first request caches the source. Its last case then asks for another variant of that source and expects `ErrNegativeCached`, which locks in the refusal from finding 1. Acceptable: expect a variant made from a cached source to be served, and test the refusal with a variant whose source is not in the cache, for example after removing it as an eviction would.
Model: opus-5-5
Decision: take both findings as the reviewer states them. The definition of done governs: the negative cache gates only fetches from the upstream, so a variant whose source is still cached is made from that source even while the URL has a negative-cache row. The plan's "on a miss" was looser than that and is superseded. The test the reviewer names is new in this PR, not on next, so it may change.
Model: opus-5-5
Decision: take both findings as the reviewer states them. The definition of done governs: the negative cache gates only fetches from the upstream, so a variant whose source is still cached is made from that source even while the URL has a negative-cache row. The plan's "on a miss" was looser than that and is superseded. The test the reviewer names is new in this PR, not on `next`, so it may change.
Model: opus-5-5
Get refuses a cached variant with the negative-cache error while its
source URL has a negative-cache row. The new test caches a variant,
stores a row for its URL, and expects the variant to be served from the
cache, and a variant not in the cache to be refused as before. It fails
until Get checks the negative cache only on a miss
(#245).
Model: opus-5-5
Get checked the negative cache before it looked for the variant on
disk, so after one failed fetch of a source every cached variant of it
answered 502 for 5 minutes. Get now serves a cached variant first and
checks the negative cache only on a miss, before processing. The 502
line under /v1/image/ in README.md and TODO.md say so.
Model: opus-5-5
The negative cache refuses only a fetch from the upstream: a variant not
in the cache is made from the cached source, and refused once that
source is evicted.
Model: opus-5-5
The check moves from Get to the start of fetchAndProcess, so a variant
not in the cache is still made from a cached source while the URL is in
the negative cache. A refusal now counts as a miss.
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.
Closes #245.
Getchecked the negative cache before it looked for the variant on disk. One failed fetch of a source (as when another variant misses after the source blob was evicted and the upstream answers 503) then made every cached variant of that source answer 502 for 5 minutes, although the files were on disk.Getnow serves a cached variant first and checks the negative cache only on a miss, before processing. A cache hit is counted as before; a refusal from the negative cache is still not counted as a miss.The 502 entry under
/v1/image/inREADME.mdnow says an image already in the cache is still served.TODO.mdhas the entry.Judgement call: the check sits in
Getright after the variant lookup, not inside the fetch path, so a variant that is not on disk is refused during those 5 minutes even when its source blob is still cached, as today. The plan asks for a variant not on disk to answer as it does today.Model: opus-5-5
Review: failed (needs-rework)
internal/imgcache/service.go,Get:Getstill checks the negative cache before any processing, not only before a fetch. So a variant that is not on disk, but whose source image is still in the cache, answers 502 for 5 minutes, although pixa could make it without asking the upstream. This happens in the issue's own case. Two variants of an evicted source miss at the same time, and the upstream refuses one fetch (503) but answers the other. The source is then cached and the URL has a negative-cache row. The definition of done in #245 says the negative cache "then gates only fetches". Acceptable: consult the negative cache only where pixa is about to fetch from the upstream because no cached source can be used, and keep the comment above the check true of where it sits.internal/imgcache/negative_cache_internal_test.go,TestService_Get_ServesCachedVariantOfNegativeCachedURL: the test's first request caches the source. Its last case then asks for another variant of that source and expectsErrNegativeCached, which locks in the refusal from finding 1. Acceptable: expect a variant made from a cached source to be served, and test the refusal with a variant whose source is not in the cache, for example after removing it as an eviction would.Model: opus-5-5
Decision: take both findings as the reviewer states them. The definition of done governs: the negative cache gates only fetches from the upstream, so a variant whose source is still cached is made from that source even while the URL has a negative-cache row. The plan's "on a miss" was looser than that and is superseded. The test the reviewer names is new in this PR, not on
next, so it may change.Model: opus-5-5
73f4cf03c1todd78dbc8d03e5d6750dbto3321627eb6View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.