Reviewed: next at 869b5ba67ff06a4aa8340e8c7c6005d56f205390 ("Stamp the tag or short commit in a plain docker build", 2026-10-02). Covered: every non-test Go file under cmd/ and internal/ (about 9,100 lines in 20 packages), the schema in internal/db/migrations/, README.md, CLAUDE.md, CONVENTIONS.md, TODO.md, config.example.yml, the two templates, and the issues that explain the current shape: #39, #51, #56, #64, #65, #68, #69, #70, #73, #74, #83, #84, #85, #87. Not covered: the tests, read only to tell test-only symbols from production ones; script/, the Dockerfiles and CI, which are not part of the model; and the two open PRs #160 and #162, which are not on next (160 changes Service.Get, so the pipeline items below should land after it). Nothing was built or run. This is design reasoning, not a defect audit: decisions already made are questioned where a simpler shape seems available, never reported as errors, and an item that reverses one says so and waits for your ruling. Several items delete the tests of the code they remove, which CLAUDE.md makes your approval; a yes on an item is that approval. Sizes: small is one package and a few hours; medium is several packages, a day or two; large touches storage, schema and their tests.
Recommendations, most valuable first
One request type instead of five. The same eight fields (host, path, query, width, height, format, quality, fit) exist as imgcache.ParsedURL, imgcache.ImageRequest, encurl.Payload, signature.Request and imageprocessor.Request; Size and FitMode are each defined twice (imgcache, imageprocessor), the format constants three times (imgcache.ImageFormat, imageprocessor.Format, magic.ImageFormat), and four converters join them (ParsedURL.ToImageRequest, Payload.ToImageRequest, signatureRequest in internal/imgcache/service.go, the literal in processAndStore). Put the type, its enums, its validation, the parsing of the /v1/image/ path and the variant key function into one small package that imports nothing else in the repo, and have signature, encurl, imageprocessor and the cache import it; encurl keeps only a private struct for its short CBOR field names. Give the type two parts, what to fetch (host, path, query) and what to do to it (width, height, format, quality, fit); the processor takes only the second. This also settles the last item of #39: parsing moves with the type it produces, so no separate urlparser package with a sixth copy of the fields is needed. Why: a newcomer following one request through the system today has to learn five shapes and four conversions; with one type there is nothing to convert. Size: medium. Depends on: nothing; do it first, every item below gets smaller with it. The package name is yours to pick; imagerequest is a proposal.
Store sources the way variants are stored: one row and one file per key, with no sharing between URLs. A variant is one variant_content row plus one file named by its key. A source is two rows (source_metadata for the URL, source_content for the bytes, joined on content_hash), a file named by the hash of the bytes, and a JSON file under cache/metadata/ that nothing reads (MetadataStorage.Load has no caller). Sharing one file between URLs with identical bytes is what forces the rest: contentLock (internal/imgcache/contentlock.go) to serialize StoreSource against evictSourceBlob, the transaction in evictSourceBlob that deletes every referencing row before the unlink, sourceReferences, evictSourceBlobTestHook, and the source half of reconcileAccounting. If a source is one row (host, path, query, content type, size, times) and one file named by a hash of host, path and query, then a store is "write file, upsert row", an eviction is "delete row, delete file", the lock and the transaction go, and a row whose file is missing is a miss, which LookupSource already treats it as. Reconciliation then has one job, sweeping files that have no row (and stale temp files) at startup, provided a failed row insert fails the store and removes the file instead of being best-effort. Cost: two URLs serving identical bytes store them twice. Why: the cache then has one mechanism used twice instead of two unrelated ones, and internal/imgcache/eviction.go (807 lines) and cache.go (642) lose their hardest parts. Size: large. Depends on: your ruling, since it reverses the sentence in README.md "Multiple source paths may reference the same content blob; the database tracks references rather than using filesystem refcounting" and the multi-reference design of #51. Confidence that nothing else needs the sharing: high; nothing outside the cache reads content_hash.
Keep a variant's content type and size in one place, the database row, and read them on a hit. A variant's content type is kept three times: the .meta file next to the variant, variant_content.content_type, and Cache.metaCache, the 10,000-entry LRU from #70. The comment in Cache.Lookup says "no DB needed for cache hits", but every hit then runs two database writes, touchVariant (from Lookup) and IncrementStats (from Service.Get), so reading the row in the same statement as the touch (UPDATE ... RETURNING content_type, size_bytes) costs nothing the hit path does not already pay, and removes the .meta sidecar, VariantMeta, LoadWithMeta, DeleteWithMeta, variantContentTypeFromSidecar, the LRU, the golang-lru dependency and the metacache tests. If the hit path should instead avoid the database entirely, the two writes have to go first; today they are there. Why: one source of truth for each fact, and GetVariant becomes "read row, open file". Size: medium. Depends on: your ruling, since #70 chose the LRU; fits naturally with the item above.
Rewrite the schema file to describe only what exists.001_schema.sql carries two tables nothing writes (output_content, request_cache, left in place by #56), source_metadata columns never written (expires_at, etag, last_modified, reserved by #83) or always the same (status_code is always 200), negative_cache.status_code always 502 (extractStatusCode has no other result for an error that is cached), source_metadata.id (its only reader, Cache.GetSourceMetadataID, has no caller) and path_hash (there only to name the unread JSON file). The file's comments also describe directories (cache/src-content, cache/src-metadata, cache/dst-content) that NewCache does not create (#74). Pre-1.0 with no installed base: edit 001_schema.sql in place to the tables of the two items above, add the columns of #83 when that lands, and change nothing by migration. Why: the schema is the first thing a newcomer reads to learn the data model, and today half of it is not the model. Size: small. Depends on: the two items above for the final table list; the deletions can go first on their own.
One mechanism for counters: Prometheus, not the cache_stats table.IncrementStats and IncrementTransformCount run up to three UPDATEs per request into cache_stats, and in production nothing reads them: Cache.Stats is reached only through Service.Stats, which no handler calls, and no route exposes it. #85 asks for the same numbers (hits, misses, upstream bytes, transcodes) as Prometheus metrics, and the Prometheus registry is already a dependency. Keep the counters in process memory as Prometheus counters and drop the table, Stats, CacheStats and the per-request writes. Why: two counting mechanisms for one job, one of them writing to disk on every request for no reader. Size: small to medium. Depends on: your ruling, since #56 just made these counters correct; best done as the first step of #85.
Remove the surface nothing uses. Beyond the items above: Service.Warm (calls Get and discards the result), Service.Purge (returns "not implemented"), the four interfaces in internal/imgcache/imgcache.go (ImageCache, SignatureValidator, Allowlist, Storage; none used as a type, and Storage does not match the concrete types, #73), imgcache.ParseImageURL (test-only twin of ParseImagePath), signature.ParseParams (the handler parses exp itself), Service.GenerateSignedURL and Signer.GenerateSignedURL (test-only; README.md documents the algorithm for clients), encurl.FromImageRequest, magic.PeekAndValidate, magic.IsSupportedMIMEType, magic.MIMEToImageFormat, magic.ImageFormatToMIME, imageprocessor.SupportedInputFormats and SupportedOutputFormats, allowlist.IsEmpty and Count, httpfetcher.ssrfSafeDialer, Cache.CleanExpired (expired failures are deleted lazily in checkNegativeCache), Cache.GetSourceMetadataID, imgcache.CacheStale, ErrCacheMiss, ErrNegativeCache, ImageResponse.LastModified (never set; #83 adds it for real), LookupResult (only Hit and the key are read; a bool and the key suffice), ContentStorage.Store, VariantStorage.Load, MetadataStorage.Load and Exists. CacheConfig.CacheTTL is set and never read; settle #69 by implementing or deleting it, not by carrying it. Why: every unused method is a promise a newcomer has to check before trusting the model. Size: small. Depends on: nothing; a test that uses a test-only helper takes the helper with it. Confidence: high; each symbol was checked for production callers at 869b5ba.
Build the pipeline in main's fx graph, with the config as the only place defaults live.handlers.New constructs the cache, fetcher, processor, session manager and encrypted-URL generator inside its OnStart hook (initImageService), so Handlers holds a *database.Database only to hand it on, and its fields are nil until start. Defaults are declared three times: config (DefaultUpstreamConnectionsPerHost, DefaultUpstreamFetchTimeout, DefaultUpstreamMaxResponseSize, and so on), httpfetcher.DefaultConfig() and imageprocessor.DefaultMaxInputBytes; initImageService copies validated config into httpfetcher.Config field by field (with a positive-value guard the config layer already enforces), and NewService reads AllowHTTP and MaxResponseSize back out of that struct. The one fact "disk cache off" is CacheMaxBytes == 0 in config, CacheConfig.DisableDiskCache at the boundary, and Cache.disabled plus a zero-limit check inside. Make each of cache, fetcher, processor, session manager, URL generator and pipeline an fx provider taking the values it needs, as CONVENTIONS.md already prescribes, with no zero-means-default in their constructors; handlers.New then receives finished objects. Why: a newcomer reads main.go and sees the whole object graph, and a setting has one default and one path. Size: medium. Depends on: nothing; easier after the first item.
Take the signature, the expiry, the scheme and the source URL off the request.ImageRequest carries Signature, Expires and AllowHTTP next to the fields that name the cached object; Service.Get writes req.AllowHTTP = s.allowHTTP, a process-wide setting, onto every request, and GenerateSignedURL writes Quality, FitMode, Expires and Signature back into its argument. signature.Request likewise contains the signature it is verified against. Service.ValidateRequest (allowlist, else signature) serves the /v1/image/ route alone, yet lives in the pipeline, which therefore owns a Signer and a HostAllowList it needs for nothing else. Verify as Verify(req, sig, expires) in the handler of the plain route, keep the expiry as a handler-local value for Cache-Control, let the fetcher take the source (host, path, query) and choose the scheme from its own AllowHTTP instead of re-parsing a string the request built, and give the pipeline one method, Get. Why: the request then means exactly "which image, transformed how", which is exactly what the variant key covers. Size: small. Depends on: the first item.
Derive every key from the one secret in one place. The signing key is used raw as the HMAC key in signature, and run through HKDF with four different salts in three places: internal/session/session.go (hashKeySalt, blockKeySalt), internal/encurl/encurl.go (urlKeySalt) and internal/handlers/csrf.go (csrfKeySalt); it is also the password compared on login. One function, in seal or next to the config, that derives the named keys once and hands each constructor the key it needs makes the secret model one screen long. Keep the raw key for signatures, since README.md documents that algorithm for clients. Why: today a newcomer finds out what the secret protects by grepping for SigningKey. Size: small. Depends on: nothing.
One handler body for the two image routes.HandleImage and HandleImageEnc repeat "get the image, set headers, copy, log" with two error mappers (respondImageError, handleImageError) that disagree (only the encrypted route maps ErrUpstreamTimeout to 504; only the plain route logs at Error first), and only the plain route handles ETag, If-None-Match and HEAD (#84). The routes should differ only in how the request is obtained, parse the path and verify the signature, or decrypt the token; then one serveImage(w, r, req, expires) and one error mapper. Why: one place to read for what an image response is. Size: small. Depends on: the two items above.
Split internal/imgcache by role. After the first item the package holds the pipeline (Service, service.go) and the cache (cache.go, storage.go, eviction.go, contentlock.go), plus imgcache.go and module.go (two constants). Move Service to its own package named for what it is (see Names) and keep imgcache for the cache. #39 kept these together as "tightly coupled"; the coupling is the shared types, and once those are a leaf package the two halves are independent and each fits in one reading. Give the cache one get and one put per kind (see Names) instead of the two-step Lookup then GetVariant and LookupSource then GetSourceContent, and keep checkNegativeCache behind the same door as the rest. Size: medium. Depends on: the first item; closes #39 and #73.
Collapse the three storage structs into one.ContentStorage, VariantStorage and MetadataStorage in internal/imgcache/storage.go each implement the same write-temp-file-then-rename, the same two-level directory fan-out and the same Load/Exists/Delete, differing only in who chooses the key and in the .meta sidecar. One type, "a directory of files named by a hex key", used once for sources and once for variants, is enough; the sidecar and the metadata directory go with the two storage items above. Why: one copy of the atomic-write code to read and to get right. Size: small once the sidecars are gone, medium before. Depends on: the two storage items above.
Decide an input's format from its bytes once. The format of a fetched image is established three times in three vocabularies: the fetcher's AllowedContentTypes check on the upstream Content-Type header (httpfetcher, strings), magic.ValidateMagicBytes requiring the detected magic.MIMEType to equal that header, and libvips' own detection in imageprocessor.detectFormat, which returns a plain string ("jpeg", "unknown") that formatFromString maps back to a Format, defaulting to JPEG (the silent fallback of #68). MIME strings are declared in httpfetcher, imageprocessor, magic and as literals in eviction.go. Keep the header check as the cheap filter before downloading, detect the format from the bytes once, carry it as the one Format type of the first item with one format-to-MIME function, and make an undetectable input an explicit error. Why: a newcomer asking "what format is this source" should find one answer in one type. Size: small. Depends on: the first item and the ruling on #68.
One list of settings in internal/config. Adding a setting today touches a key constant, isKnownConfigKey, envVarNames, a Config field, the literal in newFromSmartConfig, validate, README.md and config.example.yml, and must agree with TestEnvironmentSetsEveryKey. A single table (key, variable, type, default, check) that drives loading, the unknown-key and unknown-variable checks and the README table makes a setting one entry. Why: the config model is then visible in one place instead of being reassembled from five lists. Size: medium. Depends on: nothing. This is the least urgent item; the current code is correct, only long.
Describe the implemented model in README.md in the chosen words. The Design section describes directories that do not exist and an output store keyed by content hash, which is not how variants are stored (#74), and the repo has no one place that defines its nouns. After the renames below, a short "Concepts" list in the Design section (source, variant, variant key, signed URL, encrypted URL, failed fetch, disk cache limit), each in one sentence, is the newcomer's first page. Why: #74 is factual correction; this is the next step, one name per concept that the code then uses. Size: small. Depends on: the names below being accepted.
Names
Current name, proposed name, one-line reason. Every proposed name is a proposal for you to approve; those marked new are not yet words the repo's documents use.
imgcache.Service becomes Proxy in a package imageproxy (new): "Service" says nothing; README.md's own description of pixa, "proxies images from upstream sources, optionally resizing or transforming them, and serves the results", is exactly what this type does.
imgcache.ServiceConfig goes with it, as constructor arguments or imageproxy.Config.
imgcache.CacheConfig becomes imgcache.Config: the package already says cache; httpfetcher.Config is the precedent.
imageprocessor.Params becomes imageprocessor.Config: everywhere else in the repo Params is an fx injection struct (handlers.Params, server.Params); this one is not.
imageprocessor.ImageProcessor becomes Processor, and httpfetcher.HTTPFetcher becomes Client (the Fetcher interface stays for the mock): both stutter, which you ruled against for constructors on #41.
allowlist.HostAllowList becomes allowlist.Hosts: same stutter.
ImageRequest.SourceHost, SourcePath, SourceQuery become one field Source of type Source{Host, Path, Query} (new as a type), and Size, Format, Quality, FitMode become one field Transform of type Transform{Width, Height, Format, Quality, Fit} (new as a type; "transformed images" is README.md's phrase): the two halves of a request are what to fetch and what to do to it, and the processor needs only the second.
FitMode becomes Fit, and ImageFormat becomes Format: the URL parameter is fit, README.md says "fit", and the second copy of the format type is already called Format.
CacheKey(req), VariantKey and the column cache_key become req.VariantKey(), the type VariantKey, and a column key in the variants table: "cache key" is ambiguous now that sources are cached too; the thing keyed is the variant.
ContentHash and PathHash go with the storage items; sources are then keyed by a SourceKey (new) made the same way as the variant key.
Tables source_content plus source_metadata become sources, variant_content becomes variants, negative_cache becomes failed_fetches (new): one table per noun, named by the noun; "negative cache" appears nowhere in README.md, and a row of this table is a recent failed fetch.
Cache.Lookup plus GetVariant become one Variant(key); LookupSource plus GetSourceContent become one Source(src); StoreVariant and StoreSource become PutVariant and PutSource; checkNegativeCache and StoreNegative become Failure(src) and PutFailure(src): one get and one put per kind, named by the kind, and no two-step lookups.
ContentStorage, VariantStorage, MetadataStorage become one FileStore (new): what it is, a directory of files named by key.
encurl.Generator becomes encurl.Codec, with Encode and Decode for Generate and Parse: the type both makes and reads tokens, and "Generator.Parse" misleads.
imgcache.ImageResponse becomes imageproxy.Image (new): it is the image the proxy hands the handler (bytes, type, length, ETag, cache status), not an HTTP response.
handlers.Handlers fields imgSvc, imgCache, sessMgr, encGen become proxy, cache, sessions, tokens: CLAUDE.md asks for full words.
signing_key and Config.SigningKey become secret_key and SecretKey (new, user-facing): it is the one secret everything derives from (HMAC signing, URL encryption, session and CSRF keys, the login password), and "signing" names a fifth of it; pre-1.0, so the config key can change with it.
debug keeps the logging, and the second thing it does, plain HTTP for local development (CSRF plaintext mode, http:// generated URLs), gets its own setting such as plain_http (new): one flag controlling two unrelated things is a trap for whoever turns on debug logging in production.
Model: fable-5-1
Reviewed: `next` at `869b5ba67ff06a4aa8340e8c7c6005d56f205390` ("Stamp the tag or short commit in a plain docker build", 2026-10-02). Covered: every non-test Go file under `cmd/` and `internal/` (about 9,100 lines in 20 packages), the schema in `internal/db/migrations/`, `README.md`, `CLAUDE.md`, `CONVENTIONS.md`, `TODO.md`, `config.example.yml`, the two templates, and the issues that explain the current shape: https://git.eeqj.de/sneak/pixa/issues/39, https://git.eeqj.de/sneak/pixa/issues/51, https://git.eeqj.de/sneak/pixa/issues/56, https://git.eeqj.de/sneak/pixa/issues/64, https://git.eeqj.de/sneak/pixa/issues/65, https://git.eeqj.de/sneak/pixa/issues/68, https://git.eeqj.de/sneak/pixa/issues/69, https://git.eeqj.de/sneak/pixa/issues/70, https://git.eeqj.de/sneak/pixa/issues/73, https://git.eeqj.de/sneak/pixa/issues/74, https://git.eeqj.de/sneak/pixa/issues/83, https://git.eeqj.de/sneak/pixa/issues/84, https://git.eeqj.de/sneak/pixa/issues/85, https://git.eeqj.de/sneak/pixa/issues/87. Not covered: the tests, read only to tell test-only symbols from production ones; `script/`, the Dockerfiles and CI, which are not part of the model; and the two open PRs https://git.eeqj.de/sneak/pixa/pulls/160 and https://git.eeqj.de/sneak/pixa/pulls/162, which are not on `next` (160 changes `Service.Get`, so the pipeline items below should land after it). Nothing was built or run. This is design reasoning, not a defect audit: decisions already made are questioned where a simpler shape seems available, never reported as errors, and an item that reverses one says so and waits for your ruling. Several items delete the tests of the code they remove, which `CLAUDE.md` makes your approval; a yes on an item is that approval. Sizes: small is one package and a few hours; medium is several packages, a day or two; large touches storage, schema and their tests.
## Recommendations, most valuable first
- **One request type instead of five.** The same eight fields (host, path, query, width, height, format, quality, fit) exist as `imgcache.ParsedURL`, `imgcache.ImageRequest`, `encurl.Payload`, `signature.Request` and `imageprocessor.Request`; `Size` and `FitMode` are each defined twice (`imgcache`, `imageprocessor`), the format constants three times (`imgcache.ImageFormat`, `imageprocessor.Format`, `magic.ImageFormat`), and four converters join them (`ParsedURL.ToImageRequest`, `Payload.ToImageRequest`, `signatureRequest` in `internal/imgcache/service.go`, the literal in `processAndStore`). Put the type, its enums, its validation, the parsing of the `/v1/image/` path and the variant key function into one small package that imports nothing else in the repo, and have `signature`, `encurl`, `imageprocessor` and the cache import it; `encurl` keeps only a private struct for its short CBOR field names. Give the type two parts, what to fetch (host, path, query) and what to do to it (width, height, format, quality, fit); the processor takes only the second. This also settles the last item of https://git.eeqj.de/sneak/pixa/issues/39: parsing moves with the type it produces, so no separate `urlparser` package with a sixth copy of the fields is needed. Why: a newcomer following one request through the system today has to learn five shapes and four conversions; with one type there is nothing to convert. Size: medium. Depends on: nothing; do it first, every item below gets smaller with it. The package name is yours to pick; `imagerequest` is a proposal.
- **Store sources the way variants are stored: one row and one file per key, with no sharing between URLs.** A variant is one `variant_content` row plus one file named by its key. A source is two rows (`source_metadata` for the URL, `source_content` for the bytes, joined on `content_hash`), a file named by the hash of the bytes, and a JSON file under `cache/metadata/` that nothing reads (`MetadataStorage.Load` has no caller). Sharing one file between URLs with identical bytes is what forces the rest: `contentLock` (`internal/imgcache/contentlock.go`) to serialize `StoreSource` against `evictSourceBlob`, the transaction in `evictSourceBlob` that deletes every referencing row before the unlink, `sourceReferences`, `evictSourceBlobTestHook`, and the source half of `reconcileAccounting`. If a source is one row (host, path, query, content type, size, times) and one file named by a hash of host, path and query, then a store is "write file, upsert row", an eviction is "delete row, delete file", the lock and the transaction go, and a row whose file is missing is a miss, which `LookupSource` already treats it as. Reconciliation then has one job, sweeping files that have no row (and stale temp files) at startup, provided a failed row insert fails the store and removes the file instead of being best-effort. Cost: two URLs serving identical bytes store them twice. Why: the cache then has one mechanism used twice instead of two unrelated ones, and `internal/imgcache/eviction.go` (807 lines) and `cache.go` (642) lose their hardest parts. Size: large. Depends on: your ruling, since it reverses the sentence in `README.md` "Multiple source paths may reference the same content blob; the database tracks references rather than using filesystem refcounting" and the multi-reference design of https://git.eeqj.de/sneak/pixa/issues/51. Confidence that nothing else needs the sharing: high; nothing outside the cache reads `content_hash`.
- **Keep a variant's content type and size in one place, the database row, and read them on a hit.** A variant's content type is kept three times: the `.meta` file next to the variant, `variant_content.content_type`, and `Cache.metaCache`, the 10,000-entry LRU from https://git.eeqj.de/sneak/pixa/issues/70. The comment in `Cache.Lookup` says "no DB needed for cache hits", but every hit then runs two database writes, `touchVariant` (from `Lookup`) and `IncrementStats` (from `Service.Get`), so reading the row in the same statement as the touch (`UPDATE ... RETURNING content_type, size_bytes`) costs nothing the hit path does not already pay, and removes the `.meta` sidecar, `VariantMeta`, `LoadWithMeta`, `DeleteWithMeta`, `variantContentTypeFromSidecar`, the LRU, the `golang-lru` dependency and the `metacache` tests. If the hit path should instead avoid the database entirely, the two writes have to go first; today they are there. Why: one source of truth for each fact, and `GetVariant` becomes "read row, open file". Size: medium. Depends on: your ruling, since https://git.eeqj.de/sneak/pixa/issues/70 chose the LRU; fits naturally with the item above.
- **Rewrite the schema file to describe only what exists.** `001_schema.sql` carries two tables nothing writes (`output_content`, `request_cache`, left in place by https://git.eeqj.de/sneak/pixa/issues/56), `source_metadata` columns never written (`expires_at`, `etag`, `last_modified`, reserved by https://git.eeqj.de/sneak/pixa/issues/83) or always the same (`status_code` is always 200), `negative_cache.status_code` always 502 (`extractStatusCode` has no other result for an error that is cached), `source_metadata.id` (its only reader, `Cache.GetSourceMetadataID`, has no caller) and `path_hash` (there only to name the unread JSON file). The file's comments also describe directories (`cache/src-content`, `cache/src-metadata`, `cache/dst-content`) that `NewCache` does not create (https://git.eeqj.de/sneak/pixa/issues/74). Pre-1.0 with no installed base: edit `001_schema.sql` in place to the tables of the two items above, add the columns of https://git.eeqj.de/sneak/pixa/issues/83 when that lands, and change nothing by migration. Why: the schema is the first thing a newcomer reads to learn the data model, and today half of it is not the model. Size: small. Depends on: the two items above for the final table list; the deletions can go first on their own.
- **One mechanism for counters: Prometheus, not the `cache_stats` table.** `IncrementStats` and `IncrementTransformCount` run up to three `UPDATE`s per request into `cache_stats`, and in production nothing reads them: `Cache.Stats` is reached only through `Service.Stats`, which no handler calls, and no route exposes it. https://git.eeqj.de/sneak/pixa/issues/85 asks for the same numbers (hits, misses, upstream bytes, transcodes) as Prometheus metrics, and the Prometheus registry is already a dependency. Keep the counters in process memory as Prometheus counters and drop the table, `Stats`, `CacheStats` and the per-request writes. Why: two counting mechanisms for one job, one of them writing to disk on every request for no reader. Size: small to medium. Depends on: your ruling, since https://git.eeqj.de/sneak/pixa/issues/56 just made these counters correct; best done as the first step of https://git.eeqj.de/sneak/pixa/issues/85.
- **Remove the surface nothing uses.** Beyond the items above: `Service.Warm` (calls `Get` and discards the result), `Service.Purge` (returns "not implemented"), the four interfaces in `internal/imgcache/imgcache.go` (`ImageCache`, `SignatureValidator`, `Allowlist`, `Storage`; none used as a type, and `Storage` does not match the concrete types, https://git.eeqj.de/sneak/pixa/issues/73), `imgcache.ParseImageURL` (test-only twin of `ParseImagePath`), `signature.ParseParams` (the handler parses `exp` itself), `Service.GenerateSignedURL` and `Signer.GenerateSignedURL` (test-only; `README.md` documents the algorithm for clients), `encurl.FromImageRequest`, `magic.PeekAndValidate`, `magic.IsSupportedMIMEType`, `magic.MIMEToImageFormat`, `magic.ImageFormatToMIME`, `imageprocessor.SupportedInputFormats` and `SupportedOutputFormats`, `allowlist.IsEmpty` and `Count`, `httpfetcher.ssrfSafeDialer`, `Cache.CleanExpired` (expired failures are deleted lazily in `checkNegativeCache`), `Cache.GetSourceMetadataID`, `imgcache.CacheStale`, `ErrCacheMiss`, `ErrNegativeCache`, `ImageResponse.LastModified` (never set; https://git.eeqj.de/sneak/pixa/issues/83 adds it for real), `LookupResult` (only `Hit` and the key are read; a bool and the key suffice), `ContentStorage.Store`, `VariantStorage.Load`, `MetadataStorage.Load` and `Exists`. `CacheConfig.CacheTTL` is set and never read; settle https://git.eeqj.de/sneak/pixa/issues/69 by implementing or deleting it, not by carrying it. Why: every unused method is a promise a newcomer has to check before trusting the model. Size: small. Depends on: nothing; a test that uses a test-only helper takes the helper with it. Confidence: high; each symbol was checked for production callers at `869b5ba`.
- **Build the pipeline in `main`'s fx graph, with the config as the only place defaults live.** `handlers.New` constructs the cache, fetcher, processor, session manager and encrypted-URL generator inside its `OnStart` hook (`initImageService`), so `Handlers` holds a `*database.Database` only to hand it on, and its fields are nil until start. Defaults are declared three times: `config` (`DefaultUpstreamConnectionsPerHost`, `DefaultUpstreamFetchTimeout`, `DefaultUpstreamMaxResponseSize`, and so on), `httpfetcher.DefaultConfig()` and `imageprocessor.DefaultMaxInputBytes`; `initImageService` copies validated config into `httpfetcher.Config` field by field (with a positive-value guard the config layer already enforces), and `NewService` reads `AllowHTTP` and `MaxResponseSize` back out of that struct. The one fact "disk cache off" is `CacheMaxBytes == 0` in config, `CacheConfig.DisableDiskCache` at the boundary, and `Cache.disabled` plus a zero-limit check inside. Make each of cache, fetcher, processor, session manager, URL generator and pipeline an fx provider taking the values it needs, as `CONVENTIONS.md` already prescribes, with no zero-means-default in their constructors; `handlers.New` then receives finished objects. Why: a newcomer reads `main.go` and sees the whole object graph, and a setting has one default and one path. Size: medium. Depends on: nothing; easier after the first item.
- **Take the signature, the expiry, the scheme and the source URL off the request.** `ImageRequest` carries `Signature`, `Expires` and `AllowHTTP` next to the fields that name the cached object; `Service.Get` writes `req.AllowHTTP = s.allowHTTP`, a process-wide setting, onto every request, and `GenerateSignedURL` writes `Quality`, `FitMode`, `Expires` and `Signature` back into its argument. `signature.Request` likewise contains the signature it is verified against. `Service.ValidateRequest` (allowlist, else signature) serves the `/v1/image/` route alone, yet lives in the pipeline, which therefore owns a `Signer` and a `HostAllowList` it needs for nothing else. Verify as `Verify(req, sig, expires)` in the handler of the plain route, keep the expiry as a handler-local value for `Cache-Control`, let the fetcher take the source (host, path, query) and choose the scheme from its own `AllowHTTP` instead of re-parsing a string the request built, and give the pipeline one method, `Get`. Why: the request then means exactly "which image, transformed how", which is exactly what the variant key covers. Size: small. Depends on: the first item.
- **Derive every key from the one secret in one place.** The signing key is used raw as the HMAC key in `signature`, and run through HKDF with four different salts in three places: `internal/session/session.go` (`hashKeySalt`, `blockKeySalt`), `internal/encurl/encurl.go` (`urlKeySalt`) and `internal/handlers/csrf.go` (`csrfKeySalt`); it is also the password compared on login. One function, in `seal` or next to the config, that derives the named keys once and hands each constructor the key it needs makes the secret model one screen long. Keep the raw key for signatures, since `README.md` documents that algorithm for clients. Why: today a newcomer finds out what the secret protects by grepping for `SigningKey`. Size: small. Depends on: nothing.
- **One handler body for the two image routes.** `HandleImage` and `HandleImageEnc` repeat "get the image, set headers, copy, log" with two error mappers (`respondImageError`, `handleImageError`) that disagree (only the encrypted route maps `ErrUpstreamTimeout` to 504; only the plain route logs at `Error` first), and only the plain route handles `ETag`, `If-None-Match` and `HEAD` (https://git.eeqj.de/sneak/pixa/issues/84). The routes should differ only in how the request is obtained, parse the path and verify the signature, or decrypt the token; then one `serveImage(w, r, req, expires)` and one error mapper. Why: one place to read for what an image response is. Size: small. Depends on: the two items above.
- **Split `internal/imgcache` by role.** After the first item the package holds the pipeline (`Service`, `service.go`) and the cache (`cache.go`, `storage.go`, `eviction.go`, `contentlock.go`), plus `imgcache.go` and `module.go` (two constants). Move `Service` to its own package named for what it is (see Names) and keep `imgcache` for the cache. https://git.eeqj.de/sneak/pixa/issues/39 kept these together as "tightly coupled"; the coupling is the shared types, and once those are a leaf package the two halves are independent and each fits in one reading. Give the cache one get and one put per kind (see Names) instead of the two-step `Lookup` then `GetVariant` and `LookupSource` then `GetSourceContent`, and keep `checkNegativeCache` behind the same door as the rest. Size: medium. Depends on: the first item; closes https://git.eeqj.de/sneak/pixa/issues/39 and https://git.eeqj.de/sneak/pixa/issues/73.
- **Collapse the three storage structs into one.** `ContentStorage`, `VariantStorage` and `MetadataStorage` in `internal/imgcache/storage.go` each implement the same write-temp-file-then-rename, the same two-level directory fan-out and the same Load/Exists/Delete, differing only in who chooses the key and in the `.meta` sidecar. One type, "a directory of files named by a hex key", used once for sources and once for variants, is enough; the sidecar and the metadata directory go with the two storage items above. Why: one copy of the atomic-write code to read and to get right. Size: small once the sidecars are gone, medium before. Depends on: the two storage items above.
- **Decide an input's format from its bytes once.** The format of a fetched image is established three times in three vocabularies: the fetcher's `AllowedContentTypes` check on the upstream `Content-Type` header (`httpfetcher`, strings), `magic.ValidateMagicBytes` requiring the detected `magic.MIMEType` to equal that header, and libvips' own detection in `imageprocessor.detectFormat`, which returns a plain string ("jpeg", "unknown") that `formatFromString` maps back to a `Format`, defaulting to JPEG (the silent fallback of https://git.eeqj.de/sneak/pixa/issues/68). MIME strings are declared in `httpfetcher`, `imageprocessor`, `magic` and as literals in `eviction.go`. Keep the header check as the cheap filter before downloading, detect the format from the bytes once, carry it as the one `Format` type of the first item with one format-to-MIME function, and make an undetectable input an explicit error. Why: a newcomer asking "what format is this source" should find one answer in one type. Size: small. Depends on: the first item and the ruling on https://git.eeqj.de/sneak/pixa/issues/68.
- **One list of settings in `internal/config`.** Adding a setting today touches a key constant, `isKnownConfigKey`, `envVarNames`, a `Config` field, the literal in `newFromSmartConfig`, `validate`, `README.md` and `config.example.yml`, and must agree with `TestEnvironmentSetsEveryKey`. A single table (key, variable, type, default, check) that drives loading, the unknown-key and unknown-variable checks and the README table makes a setting one entry. Why: the config model is then visible in one place instead of being reassembled from five lists. Size: medium. Depends on: nothing. This is the least urgent item; the current code is correct, only long.
- **Describe the implemented model in `README.md` in the chosen words.** The Design section describes directories that do not exist and an output store keyed by content hash, which is not how variants are stored (https://git.eeqj.de/sneak/pixa/issues/74), and the repo has no one place that defines its nouns. After the renames below, a short "Concepts" list in the Design section (source, variant, variant key, signed URL, encrypted URL, failed fetch, disk cache limit), each in one sentence, is the newcomer's first page. Why: https://git.eeqj.de/sneak/pixa/issues/74 is factual correction; this is the next step, one name per concept that the code then uses. Size: small. Depends on: the names below being accepted.
## Names
Current name, proposed name, one-line reason. Every proposed name is a proposal for you to approve; those marked new are not yet words the repo's documents use.
- `imgcache.Service` becomes `Proxy` in a package `imageproxy` (new): "Service" says nothing; `README.md`'s own description of pixa, "proxies images from upstream sources, optionally resizing or transforming them, and serves the results", is exactly what this type does.
- `imgcache.ServiceConfig` goes with it, as constructor arguments or `imageproxy.Config`.
- `imgcache.CacheConfig` becomes `imgcache.Config`: the package already says cache; `httpfetcher.Config` is the precedent.
- `imageprocessor.Params` becomes `imageprocessor.Config`: everywhere else in the repo `Params` is an fx injection struct (`handlers.Params`, `server.Params`); this one is not.
- `imageprocessor.ImageProcessor` becomes `Processor`, and `httpfetcher.HTTPFetcher` becomes `Client` (the `Fetcher` interface stays for the mock): both stutter, which you ruled against for constructors on https://git.eeqj.de/sneak/pixa/pulls/41.
- `allowlist.HostAllowList` becomes `allowlist.Hosts`: same stutter.
- `ImageRequest.SourceHost`, `SourcePath`, `SourceQuery` become one field `Source` of type `Source{Host, Path, Query}` (new as a type), and `Size`, `Format`, `Quality`, `FitMode` become one field `Transform` of type `Transform{Width, Height, Format, Quality, Fit}` (new as a type; "transformed images" is `README.md`'s phrase): the two halves of a request are what to fetch and what to do to it, and the processor needs only the second.
- `FitMode` becomes `Fit`, and `ImageFormat` becomes `Format`: the URL parameter is `fit`, `README.md` says "fit", and the second copy of the format type is already called `Format`.
- `CacheKey(req)`, `VariantKey` and the column `cache_key` become `req.VariantKey()`, the type `VariantKey`, and a column `key` in the `variants` table: "cache key" is ambiguous now that sources are cached too; the thing keyed is the variant.
- `ContentHash` and `PathHash` go with the storage items; sources are then keyed by a `SourceKey` (new) made the same way as the variant key.
- Tables `source_content` plus `source_metadata` become `sources`, `variant_content` becomes `variants`, `negative_cache` becomes `failed_fetches` (new): one table per noun, named by the noun; "negative cache" appears nowhere in `README.md`, and a row of this table is a recent failed fetch.
- `Cache.Lookup` plus `GetVariant` become one `Variant(key)`; `LookupSource` plus `GetSourceContent` become one `Source(src)`; `StoreVariant` and `StoreSource` become `PutVariant` and `PutSource`; `checkNegativeCache` and `StoreNegative` become `Failure(src)` and `PutFailure(src)`: one get and one put per kind, named by the kind, and no two-step lookups.
- `ContentStorage`, `VariantStorage`, `MetadataStorage` become one `FileStore` (new): what it is, a directory of files named by key.
- `encurl.Generator` becomes `encurl.Codec`, with `Encode` and `Decode` for `Generate` and `Parse`: the type both makes and reads tokens, and "Generator.Parse" misleads.
- `imgcache.ImageResponse` becomes `imageproxy.Image` (new): it is the image the proxy hands the handler (bytes, type, length, ETag, cache status), not an HTTP response.
- `handlers.Handlers` fields `imgSvc`, `imgCache`, `sessMgr`, `encGen` become `proxy`, `cache`, `sessions`, `tokens`: `CLAUDE.md` asks for full words.
- `signing_key` and `Config.SigningKey` become `secret_key` and `SecretKey` (new, user-facing): it is the one secret everything derives from (HMAC signing, URL encryption, session and CSRF keys, the login password), and "signing" names a fifth of it; pre-1.0, so the config key can change with it.
- `debug` keeps the logging, and the second thing it does, plain HTTP for local development (CSRF plaintext mode, `http://` generated URLs), gets its own setting such as `plain_http` (new): one flag controlling two unrelated things is a trap for whoever turns on debug logging in production.
Model: fable-5-1
sneak
was assigned by clawbot2026-10-03 14:28:20 +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.
Reviewed:
nextat869b5ba67ff06a4aa8340e8c7c6005d56f205390("Stamp the tag or short commit in a plain docker build", 2026-10-02). Covered: every non-test Go file undercmd/andinternal/(about 9,100 lines in 20 packages), the schema ininternal/db/migrations/,README.md,CLAUDE.md,CONVENTIONS.md,TODO.md,config.example.yml, the two templates, and the issues that explain the current shape: #39, #51, #56, #64, #65, #68, #69, #70, #73, #74, #83, #84, #85, #87. Not covered: the tests, read only to tell test-only symbols from production ones;script/, the Dockerfiles and CI, which are not part of the model; and the two open PRs #160 and #162, which are not onnext(160 changesService.Get, so the pipeline items below should land after it). Nothing was built or run. This is design reasoning, not a defect audit: decisions already made are questioned where a simpler shape seems available, never reported as errors, and an item that reverses one says so and waits for your ruling. Several items delete the tests of the code they remove, whichCLAUDE.mdmakes your approval; a yes on an item is that approval. Sizes: small is one package and a few hours; medium is several packages, a day or two; large touches storage, schema and their tests.Recommendations, most valuable first
One request type instead of five. The same eight fields (host, path, query, width, height, format, quality, fit) exist as
imgcache.ParsedURL,imgcache.ImageRequest,encurl.Payload,signature.Requestandimageprocessor.Request;SizeandFitModeare each defined twice (imgcache,imageprocessor), the format constants three times (imgcache.ImageFormat,imageprocessor.Format,magic.ImageFormat), and four converters join them (ParsedURL.ToImageRequest,Payload.ToImageRequest,signatureRequestininternal/imgcache/service.go, the literal inprocessAndStore). Put the type, its enums, its validation, the parsing of the/v1/image/path and the variant key function into one small package that imports nothing else in the repo, and havesignature,encurl,imageprocessorand the cache import it;encurlkeeps only a private struct for its short CBOR field names. Give the type two parts, what to fetch (host, path, query) and what to do to it (width, height, format, quality, fit); the processor takes only the second. This also settles the last item of #39: parsing moves with the type it produces, so no separateurlparserpackage with a sixth copy of the fields is needed. Why: a newcomer following one request through the system today has to learn five shapes and four conversions; with one type there is nothing to convert. Size: medium. Depends on: nothing; do it first, every item below gets smaller with it. The package name is yours to pick;imagerequestis a proposal.Store sources the way variants are stored: one row and one file per key, with no sharing between URLs. A variant is one
variant_contentrow plus one file named by its key. A source is two rows (source_metadatafor the URL,source_contentfor the bytes, joined oncontent_hash), a file named by the hash of the bytes, and a JSON file undercache/metadata/that nothing reads (MetadataStorage.Loadhas no caller). Sharing one file between URLs with identical bytes is what forces the rest:contentLock(internal/imgcache/contentlock.go) to serializeStoreSourceagainstevictSourceBlob, the transaction inevictSourceBlobthat deletes every referencing row before the unlink,sourceReferences,evictSourceBlobTestHook, and the source half ofreconcileAccounting. If a source is one row (host, path, query, content type, size, times) and one file named by a hash of host, path and query, then a store is "write file, upsert row", an eviction is "delete row, delete file", the lock and the transaction go, and a row whose file is missing is a miss, whichLookupSourcealready treats it as. Reconciliation then has one job, sweeping files that have no row (and stale temp files) at startup, provided a failed row insert fails the store and removes the file instead of being best-effort. Cost: two URLs serving identical bytes store them twice. Why: the cache then has one mechanism used twice instead of two unrelated ones, andinternal/imgcache/eviction.go(807 lines) andcache.go(642) lose their hardest parts. Size: large. Depends on: your ruling, since it reverses the sentence inREADME.md"Multiple source paths may reference the same content blob; the database tracks references rather than using filesystem refcounting" and the multi-reference design of #51. Confidence that nothing else needs the sharing: high; nothing outside the cache readscontent_hash.Keep a variant's content type and size in one place, the database row, and read them on a hit. A variant's content type is kept three times: the
.metafile next to the variant,variant_content.content_type, andCache.metaCache, the 10,000-entry LRU from #70. The comment inCache.Lookupsays "no DB needed for cache hits", but every hit then runs two database writes,touchVariant(fromLookup) andIncrementStats(fromService.Get), so reading the row in the same statement as the touch (UPDATE ... RETURNING content_type, size_bytes) costs nothing the hit path does not already pay, and removes the.metasidecar,VariantMeta,LoadWithMeta,DeleteWithMeta,variantContentTypeFromSidecar, the LRU, thegolang-lrudependency and themetacachetests. If the hit path should instead avoid the database entirely, the two writes have to go first; today they are there. Why: one source of truth for each fact, andGetVariantbecomes "read row, open file". Size: medium. Depends on: your ruling, since #70 chose the LRU; fits naturally with the item above.Rewrite the schema file to describe only what exists.
001_schema.sqlcarries two tables nothing writes (output_content,request_cache, left in place by #56),source_metadatacolumns never written (expires_at,etag,last_modified, reserved by #83) or always the same (status_codeis always 200),negative_cache.status_codealways 502 (extractStatusCodehas no other result for an error that is cached),source_metadata.id(its only reader,Cache.GetSourceMetadataID, has no caller) andpath_hash(there only to name the unread JSON file). The file's comments also describe directories (cache/src-content,cache/src-metadata,cache/dst-content) thatNewCachedoes not create (#74). Pre-1.0 with no installed base: edit001_schema.sqlin place to the tables of the two items above, add the columns of #83 when that lands, and change nothing by migration. Why: the schema is the first thing a newcomer reads to learn the data model, and today half of it is not the model. Size: small. Depends on: the two items above for the final table list; the deletions can go first on their own.One mechanism for counters: Prometheus, not the
cache_statstable.IncrementStatsandIncrementTransformCountrun up to threeUPDATEs per request intocache_stats, and in production nothing reads them:Cache.Statsis reached only throughService.Stats, which no handler calls, and no route exposes it. #85 asks for the same numbers (hits, misses, upstream bytes, transcodes) as Prometheus metrics, and the Prometheus registry is already a dependency. Keep the counters in process memory as Prometheus counters and drop the table,Stats,CacheStatsand the per-request writes. Why: two counting mechanisms for one job, one of them writing to disk on every request for no reader. Size: small to medium. Depends on: your ruling, since #56 just made these counters correct; best done as the first step of #85.Remove the surface nothing uses. Beyond the items above:
Service.Warm(callsGetand discards the result),Service.Purge(returns "not implemented"), the four interfaces ininternal/imgcache/imgcache.go(ImageCache,SignatureValidator,Allowlist,Storage; none used as a type, andStoragedoes not match the concrete types, #73),imgcache.ParseImageURL(test-only twin ofParseImagePath),signature.ParseParams(the handler parsesexpitself),Service.GenerateSignedURLandSigner.GenerateSignedURL(test-only;README.mddocuments the algorithm for clients),encurl.FromImageRequest,magic.PeekAndValidate,magic.IsSupportedMIMEType,magic.MIMEToImageFormat,magic.ImageFormatToMIME,imageprocessor.SupportedInputFormatsandSupportedOutputFormats,allowlist.IsEmptyandCount,httpfetcher.ssrfSafeDialer,Cache.CleanExpired(expired failures are deleted lazily incheckNegativeCache),Cache.GetSourceMetadataID,imgcache.CacheStale,ErrCacheMiss,ErrNegativeCache,ImageResponse.LastModified(never set; #83 adds it for real),LookupResult(onlyHitand the key are read; a bool and the key suffice),ContentStorage.Store,VariantStorage.Load,MetadataStorage.LoadandExists.CacheConfig.CacheTTLis set and never read; settle #69 by implementing or deleting it, not by carrying it. Why: every unused method is a promise a newcomer has to check before trusting the model. Size: small. Depends on: nothing; a test that uses a test-only helper takes the helper with it. Confidence: high; each symbol was checked for production callers at869b5ba.Build the pipeline in
main's fx graph, with the config as the only place defaults live.handlers.Newconstructs the cache, fetcher, processor, session manager and encrypted-URL generator inside itsOnStarthook (initImageService), soHandlersholds a*database.Databaseonly to hand it on, and its fields are nil until start. Defaults are declared three times:config(DefaultUpstreamConnectionsPerHost,DefaultUpstreamFetchTimeout,DefaultUpstreamMaxResponseSize, and so on),httpfetcher.DefaultConfig()andimageprocessor.DefaultMaxInputBytes;initImageServicecopies validated config intohttpfetcher.Configfield by field (with a positive-value guard the config layer already enforces), andNewServicereadsAllowHTTPandMaxResponseSizeback out of that struct. The one fact "disk cache off" isCacheMaxBytes == 0in config,CacheConfig.DisableDiskCacheat the boundary, andCache.disabledplus a zero-limit check inside. Make each of cache, fetcher, processor, session manager, URL generator and pipeline an fx provider taking the values it needs, asCONVENTIONS.mdalready prescribes, with no zero-means-default in their constructors;handlers.Newthen receives finished objects. Why: a newcomer readsmain.goand sees the whole object graph, and a setting has one default and one path. Size: medium. Depends on: nothing; easier after the first item.Take the signature, the expiry, the scheme and the source URL off the request.
ImageRequestcarriesSignature,ExpiresandAllowHTTPnext to the fields that name the cached object;Service.Getwritesreq.AllowHTTP = s.allowHTTP, a process-wide setting, onto every request, andGenerateSignedURLwritesQuality,FitMode,ExpiresandSignatureback into its argument.signature.Requestlikewise contains the signature it is verified against.Service.ValidateRequest(allowlist, else signature) serves the/v1/image/route alone, yet lives in the pipeline, which therefore owns aSignerand aHostAllowListit needs for nothing else. Verify asVerify(req, sig, expires)in the handler of the plain route, keep the expiry as a handler-local value forCache-Control, let the fetcher take the source (host, path, query) and choose the scheme from its ownAllowHTTPinstead of re-parsing a string the request built, and give the pipeline one method,Get. Why: the request then means exactly "which image, transformed how", which is exactly what the variant key covers. Size: small. Depends on: the first item.Derive every key from the one secret in one place. The signing key is used raw as the HMAC key in
signature, and run through HKDF with four different salts in three places:internal/session/session.go(hashKeySalt,blockKeySalt),internal/encurl/encurl.go(urlKeySalt) andinternal/handlers/csrf.go(csrfKeySalt); it is also the password compared on login. One function, insealor next to the config, that derives the named keys once and hands each constructor the key it needs makes the secret model one screen long. Keep the raw key for signatures, sinceREADME.mddocuments that algorithm for clients. Why: today a newcomer finds out what the secret protects by grepping forSigningKey. Size: small. Depends on: nothing.One handler body for the two image routes.
HandleImageandHandleImageEncrepeat "get the image, set headers, copy, log" with two error mappers (respondImageError,handleImageError) that disagree (only the encrypted route mapsErrUpstreamTimeoutto 504; only the plain route logs atErrorfirst), and only the plain route handlesETag,If-None-MatchandHEAD(#84). The routes should differ only in how the request is obtained, parse the path and verify the signature, or decrypt the token; then oneserveImage(w, r, req, expires)and one error mapper. Why: one place to read for what an image response is. Size: small. Depends on: the two items above.Split
internal/imgcacheby role. After the first item the package holds the pipeline (Service,service.go) and the cache (cache.go,storage.go,eviction.go,contentlock.go), plusimgcache.goandmodule.go(two constants). MoveServiceto its own package named for what it is (see Names) and keepimgcachefor the cache. #39 kept these together as "tightly coupled"; the coupling is the shared types, and once those are a leaf package the two halves are independent and each fits in one reading. Give the cache one get and one put per kind (see Names) instead of the two-stepLookupthenGetVariantandLookupSourcethenGetSourceContent, and keepcheckNegativeCachebehind the same door as the rest. Size: medium. Depends on: the first item; closes #39 and #73.Collapse the three storage structs into one.
ContentStorage,VariantStorageandMetadataStorageininternal/imgcache/storage.goeach implement the same write-temp-file-then-rename, the same two-level directory fan-out and the same Load/Exists/Delete, differing only in who chooses the key and in the.metasidecar. One type, "a directory of files named by a hex key", used once for sources and once for variants, is enough; the sidecar and the metadata directory go with the two storage items above. Why: one copy of the atomic-write code to read and to get right. Size: small once the sidecars are gone, medium before. Depends on: the two storage items above.Decide an input's format from its bytes once. The format of a fetched image is established three times in three vocabularies: the fetcher's
AllowedContentTypescheck on the upstreamContent-Typeheader (httpfetcher, strings),magic.ValidateMagicBytesrequiring the detectedmagic.MIMETypeto equal that header, and libvips' own detection inimageprocessor.detectFormat, which returns a plain string ("jpeg", "unknown") thatformatFromStringmaps back to aFormat, defaulting to JPEG (the silent fallback of #68). MIME strings are declared inhttpfetcher,imageprocessor,magicand as literals ineviction.go. Keep the header check as the cheap filter before downloading, detect the format from the bytes once, carry it as the oneFormattype of the first item with one format-to-MIME function, and make an undetectable input an explicit error. Why: a newcomer asking "what format is this source" should find one answer in one type. Size: small. Depends on: the first item and the ruling on #68.One list of settings in
internal/config. Adding a setting today touches a key constant,isKnownConfigKey,envVarNames, aConfigfield, the literal innewFromSmartConfig,validate,README.mdandconfig.example.yml, and must agree withTestEnvironmentSetsEveryKey. A single table (key, variable, type, default, check) that drives loading, the unknown-key and unknown-variable checks and the README table makes a setting one entry. Why: the config model is then visible in one place instead of being reassembled from five lists. Size: medium. Depends on: nothing. This is the least urgent item; the current code is correct, only long.Describe the implemented model in
README.mdin the chosen words. The Design section describes directories that do not exist and an output store keyed by content hash, which is not how variants are stored (#74), and the repo has no one place that defines its nouns. After the renames below, a short "Concepts" list in the Design section (source, variant, variant key, signed URL, encrypted URL, failed fetch, disk cache limit), each in one sentence, is the newcomer's first page. Why: #74 is factual correction; this is the next step, one name per concept that the code then uses. Size: small. Depends on: the names below being accepted.Names
Current name, proposed name, one-line reason. Every proposed name is a proposal for you to approve; those marked new are not yet words the repo's documents use.
imgcache.ServicebecomesProxyin a packageimageproxy(new): "Service" says nothing;README.md's own description of pixa, "proxies images from upstream sources, optionally resizing or transforming them, and serves the results", is exactly what this type does.imgcache.ServiceConfiggoes with it, as constructor arguments orimageproxy.Config.imgcache.CacheConfigbecomesimgcache.Config: the package already says cache;httpfetcher.Configis the precedent.imageprocessor.Paramsbecomesimageprocessor.Config: everywhere else in the repoParamsis an fx injection struct (handlers.Params,server.Params); this one is not.imageprocessor.ImageProcessorbecomesProcessor, andhttpfetcher.HTTPFetcherbecomesClient(theFetcherinterface stays for the mock): both stutter, which you ruled against for constructors on #41.allowlist.HostAllowListbecomesallowlist.Hosts: same stutter.ImageRequest.SourceHost,SourcePath,SourceQuerybecome one fieldSourceof typeSource{Host, Path, Query}(new as a type), andSize,Format,Quality,FitModebecome one fieldTransformof typeTransform{Width, Height, Format, Quality, Fit}(new as a type; "transformed images" isREADME.md's phrase): the two halves of a request are what to fetch and what to do to it, and the processor needs only the second.FitModebecomesFit, andImageFormatbecomesFormat: the URL parameter isfit,README.mdsays "fit", and the second copy of the format type is already calledFormat.CacheKey(req),VariantKeyand the columncache_keybecomereq.VariantKey(), the typeVariantKey, and a columnkeyin thevariantstable: "cache key" is ambiguous now that sources are cached too; the thing keyed is the variant.ContentHashandPathHashgo with the storage items; sources are then keyed by aSourceKey(new) made the same way as the variant key.source_contentplussource_metadatabecomesources,variant_contentbecomesvariants,negative_cachebecomesfailed_fetches(new): one table per noun, named by the noun; "negative cache" appears nowhere inREADME.md, and a row of this table is a recent failed fetch.Cache.LookupplusGetVariantbecome oneVariant(key);LookupSourceplusGetSourceContentbecome oneSource(src);StoreVariantandStoreSourcebecomePutVariantandPutSource;checkNegativeCacheandStoreNegativebecomeFailure(src)andPutFailure(src): one get and one put per kind, named by the kind, and no two-step lookups.ContentStorage,VariantStorage,MetadataStoragebecome oneFileStore(new): what it is, a directory of files named by key.encurl.Generatorbecomesencurl.Codec, withEncodeandDecodeforGenerateandParse: the type both makes and reads tokens, and "Generator.Parse" misleads.imgcache.ImageResponsebecomesimageproxy.Image(new): it is the image the proxy hands the handler (bytes, type, length, ETag, cache status), not an HTTP response.handlers.HandlersfieldsimgSvc,imgCache,sessMgr,encGenbecomeproxy,cache,sessions,tokens:CLAUDE.mdasks for full words.signing_keyandConfig.SigningKeybecomesecret_keyandSecretKey(new, user-facing): it is the one secret everything derives from (HMAC signing, URL encryption, session and CSRF keys, the login password), and "signing" names a fifth of it; pre-1.0, so the config key can change with it.debugkeeps the logging, and the second thing it does, plain HTTP for local development (CSRF plaintext mode,http://generated URLs), gets its own setting such asplain_http(new): one flag controlling two unrelated things is a trap for whoever turns on debug logging in production.Model: fable-5-1