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 variant of that source answer 502 for 5 minutes, also one on disk or one pixa could make from the cached source. The check now sits at the start of fetchAndProcess, where pixa is about to fetch because no cached source can be used. A cached variant is served, a variant not on disk is made from the cached source when there is one, and only a request that needs the source fetched is refused.
The check now runs inside the processing that requests missing the same variant at once share, so they share one check as they share the fetch.
Judgement call: a refusal from the negative cache now counts as a miss, as any other request that misses the variant cache does. Before, it was counted as neither.
The 502 entry under /v1/image/ in README.md says which images are still served. TODO.md has the entry.
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 variant of that source answer 502 for 5 minutes, also one on disk or one pixa could make from the cached source. The check now sits at the start of `fetchAndProcess`, where pixa is about to fetch because no cached source can be used. A cached variant is served, a variant not on disk is made from the cached source when there is one, and only a request that needs the source fetched is refused.
The check now runs inside the processing that requests missing the same variant at once share, so they share one check as they share the fetch.
Judgement call: a refusal from the negative cache now counts as a miss, as any other request that misses the variant cache does. Before, it was counted as neither.
The 502 entry under `/v1/image/` in `README.md` says which images are still served. `TODO.md` has the entry.
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
Done: the check moved from Get to the start of fetchAndProcess, which runs only when no cached source can be used; its comment is true there.
Done: the test now expects a variant made from the cached source to be served, and ErrNegativeCached for a further variant once evictSourceBlob has removed the source.
Judgement call: a refusal from the negative cache now counts as a miss (see the PR body).
Model: opus-5-5
1. Done: the check moved from `Get` to the start of `fetchAndProcess`, which runs only when no cached source can be used; its comment is true there.
2. Done: the test now expects a variant made from the cached source to be served, and `ErrNegativeCached` for a further variant once `evictSourceBlob` has removed the source.
Judgement call: a refusal from the negative cache now counts as a miss (see the PR body).
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
Rebased onto next: the only change is in TODO.md, where this PR's entry now sits at the top of Completed Steps, above the entry for #246, and appears once. No other file conflicted.
Model: opus-5-5
Rebased onto `next`: the only change is in `TODO.md`, where this PR's entry now sits at the top of Completed Steps, above the entry for https://git.eeqj.de/sneak/pixa/issues/246, and appears once. No other file conflicted.
Model: opus-5-5
clawbot
merged commit 2790125cb2 into next2026-10-15 13:45:26 +02:00
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 variant of that source answer 502 for 5 minutes, also one on disk or one pixa could make from the cached source. The check now sits at the start offetchAndProcess, where pixa is about to fetch because no cached source can be used. A cached variant is served, a variant not on disk is made from the cached source when there is one, and only a request that needs the source fetched is refused.The check now runs inside the processing that requests missing the same variant at once share, so they share one check as they share the fetch.
Judgement call: a refusal from the negative cache now counts as a miss, as any other request that misses the variant cache does. Before, it was counted as neither.
The 502 entry under
/v1/image/inREADME.mdsays which images are still served.TODO.mdhas the entry.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
73f4cf03c1todd78dbc8d03e5d6750dbto3321627eb6Getto the start offetchAndProcess, which runs only when no cached source can be used; its comment is true there.ErrNegativeCachedfor a further variant onceevictSourceBlobhas removed the source.Judgement call: a refusal from the negative cache now counts as a miss (see the PR body).
Model: opus-5-5
Review passed.
Model: opus-5-5
3321627eb6tobf2671cd9fRebased onto
next: the only change is inTODO.md, where this PR's entry now sits at the top of Completed Steps, above the entry for #246, and appears once. No other file conflicted.Model: opus-5-5