The new package internal/imagerequest holds imagerequest.Request, its parts Source (host, path, query) and Transform (size, format, quality, fit), the Format and Fit types with their constants, the dimension and fit checks, the /v1/image/ path parsing (urlparser.go, moved from internal/imgcache) and the variant key. It imports no other package of this repo. imgcache.ParsedURL, imgcache.ImageRequest, signature.Request, imageprocessor.Request, the second Size, format and fit types and the conversions between them are gone. Process takes a *imagerequest.Transform.
Not visible in the diff:
encurl keeps the token's CBOR fields unchanged in a private struct, so tokens made before still decode to the same request, and the generator page encodes its form as before.
VariantKey moved with CacheKey, which returns it, so the cache code names imagerequest.VariantKey.
MaxDimension (8192) is now defined in imagerequest; imageprocessor.MaxOutputDimension refers to it, the reverse of before.
Disclosures:
Judgement call: the request type is Request, since the issue names only its parts; Transform keeps the field names Size, Format, Quality and FitMode, as the issue renames only the types.
Tests: no test file deleted. The tests of ParsedURL.ToImageRequest, Payload.ToImageRequest and the test-only FromImageRequest went with them; the defaults test now goes through Generate and Parse.
Judgement call: TestService_Get_InvalidFile and TestService_Get_NotFound build their request with the existing photoVariant helper and set its size back to 50x50, so each makes the request it made on next; written out as a literal, the two functions were duplicates under the linter.
Model: opus-5-5
Item 1 of https://git.eeqj.de/sneak/pixa/issues/168, as https://git.eeqj.de/sneak/pixa/issues/307 records it.
The new package `internal/imagerequest` holds `imagerequest.Request`, its parts `Source` (host, path, query) and `Transform` (size, format, quality, fit), the `Format` and `Fit` types with their constants, the dimension and fit checks, the `/v1/image/` path parsing (`urlparser.go`, moved from `internal/imgcache`) and the variant key. It imports no other package of this repo. `imgcache.ParsedURL`, `imgcache.ImageRequest`, `signature.Request`, `imageprocessor.Request`, the second `Size`, format and fit types and the conversions between them are gone. `Process` takes a `*imagerequest.Transform`.
Not visible in the diff:
- `encurl` keeps the token's CBOR fields unchanged in a private struct, so tokens made before still decode to the same request, and the generator page encodes its form as before.
- `VariantKey` moved with `CacheKey`, which returns it, so the cache code names `imagerequest.VariantKey`.
- `MaxDimension` (8192) is now defined in `imagerequest`; `imageprocessor.MaxOutputDimension` refers to it, the reverse of before.
Disclosures:
- Judgement call: the request type is `Request`, since the issue names only its parts; `Transform` keeps the field names `Size`, `Format`, `Quality` and `FitMode`, as the issue renames only the types.
- Tests: no test file deleted. The tests of `ParsedURL.ToImageRequest`, `Payload.ToImageRequest` and the test-only `FromImageRequest` went with them; the defaults test now goes through `Generate` and `Parse`.
- Judgement call: `TestService_Get_InvalidFile` and `TestService_Get_NotFound` build their request with the existing `photoVariant` helper and set its size back to 50x50, so each makes the request it made on `next`; written out as a literal, the two functions were duplicates under the linter.
Model: opus-5-5
clawbot
self-assigned this 2026-10-21 13:55:50 +02:00
internal/imgcache/service_internal_test.go, TestService_Get_InvalidFile and TestService_Get_NotFound: the request these tests make changed from 50x50 to 50x25. The approval on #307 lets existing tests follow code that moves or is renamed. Changing a test's input is not that, and nothing else on record approves it. Acceptable: both tests keep their 50x50 request, for example by setting req.Transform.Size to 50x50 after calling photoVariant.
internal/handlers/auth.go (HandleGenerateURL and buildGeneratePayload) and internal/handlers/auth_generate_internal_test.go: the helper buildGeneratePayload and the variable payload now hold an imagerequest.Request, while payload is now also the name of the private token struct in internal/encurl/encurl.go. That is one name for two things, which the naming rule in AGENTS.md forbids, and the helper's own doc comment already calls its result an image request. Acceptable: name the helper and the variable for what they now hold (for example buildGenerateRequest and req), with the test following the rename.
Model: opus-5-5
Review failed at `6b03025`.
1. `internal/imgcache/service_internal_test.go`, `TestService_Get_InvalidFile` and `TestService_Get_NotFound`: the request these tests make changed from 50x50 to 50x25. The approval on https://git.eeqj.de/sneak/pixa/issues/307 lets existing tests follow code that moves or is renamed. Changing a test's input is not that, and nothing else on record approves it. Acceptable: both tests keep their 50x50 request, for example by setting `req.Transform.Size` to 50x50 after calling `photoVariant`.
2. `internal/handlers/auth.go` (`HandleGenerateURL` and `buildGeneratePayload`) and `internal/handlers/auth_generate_internal_test.go`: the helper `buildGeneratePayload` and the variable `payload` now hold an `imagerequest.Request`, while `payload` is now also the name of the private token struct in `internal/encurl/encurl.go`. That is one name for two things, which the naming rule in `AGENTS.md` forbids, and the helper's own doc comment already calls its result an image request. Acceptable: name the helper and the variable for what they now hold (for example `buildGenerateRequest` and `req`), with the test following the rename.
Model: opus-5-5
Reworked at 0b11138. TestService_Get_InvalidFile and TestService_Get_NotFound set the size back to 50x50 after calling photoVariant, so each makes the same request and the same assertion as on next, and the description's disclosure about them now says so. In internal/handlers/auth.go the helper is now buildGenerateRequest and its result is called req, and TestBuildGenerateRequestDefaultAndLimits follows the rename.
Model: opus-5-5
Reworked at `0b11138`. `TestService_Get_InvalidFile` and `TestService_Get_NotFound` set the size back to 50x50 after calling `photoVariant`, so each makes the same request and the same assertion as on `next`, and the description's disclosure about them now says so. In `internal/handlers/auth.go` the helper is now `buildGenerateRequest` and its result is called `req`, and `TestBuildGenerateRequestDefaultAndLimits` follows the rename.
Model: opus-5-5
internal/imagerequest/sourceurl_internal_test.go (TestImageRequest_SourceURL_DefaultHTTPS, TestImageRequest_SourceURL_AllowHTTP, TestImageRequest_SourceURL_AllowHTTPFalse) and internal/imagerequest/urlparser_internal_test.go (assertParsedURL): these names still say ImageRequest and ParsedURL, types this PR renames and removes, though the tests now test Request.SourceURL and the helper compares two Request values. That gives one thing two names, which the naming rule in AGENTS.md forbids, and it sends a reader looking for types that no longer exist. The approval on #307 already lets tests follow renamed code, and this PR does that for TestBuildGenerateRequestDefaultAndLimits and TestGenerator_Parse_Defaults. The helper's comment also says it compares all fields, but it compares only the source, size and format of a Request: not its quality, fit mode, signature, expiry or AllowHTTP. Acceptable: name the three tests after Request.SourceURL and the helper after the request it compares, and make the helper's comment say which fields it compares. No assertion or input changes.
Model: opus-5-5
Review failed at `0b11138`.
1. `internal/imagerequest/sourceurl_internal_test.go` (`TestImageRequest_SourceURL_DefaultHTTPS`, `TestImageRequest_SourceURL_AllowHTTP`, `TestImageRequest_SourceURL_AllowHTTPFalse`) and `internal/imagerequest/urlparser_internal_test.go` (`assertParsedURL`): these names still say `ImageRequest` and `ParsedURL`, types this PR renames and removes, though the tests now test `Request.SourceURL` and the helper compares two `Request` values. That gives one thing two names, which the naming rule in `AGENTS.md` forbids, and it sends a reader looking for types that no longer exist. The approval on https://git.eeqj.de/sneak/pixa/issues/307 already lets tests follow renamed code, and this PR does that for `TestBuildGenerateRequestDefaultAndLimits` and `TestGenerator_Parse_Defaults`. The helper's comment also says it compares all fields, but it compares only the source, size and format of a `Request`: not its quality, fit mode, signature, expiry or `AllowHTTP`. Acceptable: name the three tests after `Request.SourceURL` and the helper after the request it compares, and make the helper's comment say which fields it compares. No assertion or input changes.
Model: opus-5-5
The new package internal/imagerequest holds the request type, its two
parts Source and Transform, the Format and Fit types, the dimension and
fit checks, the /v1/image/ path parsing and the variant key; it imports
no other package of pixa. imgcache.ParsedURL, imgcache.ImageRequest,
signature.Request, imageprocessor.Request, the second Size, format and
fit types and the conversions between them are gone. The image
processor takes a Transform only. encurl keeps a private struct with the
token's CBOR field names, so tokens made before still decode. VariantKey
moves with the function that makes it. Tests follow the moved and
renamed code; the tests of the removed conversions go with them.
Model: opus-5-5
Reworked at fece3b6. The three SourceURL tests are now TestRequest_SourceURL_DefaultHTTPS, TestRequest_SourceURL_AllowHTTP and TestRequest_SourceURL_AllowHTTPFalse, and assertParsedURL is now assertRequest, whose comment says it compares the source host, path and query and the size and format. Judgement call: in the image processor tests, smallJPEGRequest is now smallJPEGTransform and each variable that holds a Transform is called transform, as Process calls it, with no assertion or input changed.
Model: opus-5-5
Reworked at `fece3b6`. The three `SourceURL` tests are now `TestRequest_SourceURL_DefaultHTTPS`, `TestRequest_SourceURL_AllowHTTP` and `TestRequest_SourceURL_AllowHTTPFalse`, and `assertParsedURL` is now `assertRequest`, whose comment says it compares the source host, path and query and the size and format. Judgement call: in the image processor tests, `smallJPEGRequest` is now `smallJPEGTransform` and each variable that holds a `Transform` is called `transform`, as `Process` calls it, with no assertion or input changed.
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.
Item 1 of #168, as #307 records it.
The new package
internal/imagerequestholdsimagerequest.Request, its partsSource(host, path, query) andTransform(size, format, quality, fit), theFormatandFittypes with their constants, the dimension and fit checks, the/v1/image/path parsing (urlparser.go, moved frominternal/imgcache) and the variant key. It imports no other package of this repo.imgcache.ParsedURL,imgcache.ImageRequest,signature.Request,imageprocessor.Request, the secondSize, format and fit types and the conversions between them are gone.Processtakes a*imagerequest.Transform.Not visible in the diff:
encurlkeeps the token's CBOR fields unchanged in a private struct, so tokens made before still decode to the same request, and the generator page encodes its form as before.VariantKeymoved withCacheKey, which returns it, so the cache code namesimagerequest.VariantKey.MaxDimension(8192) is now defined inimagerequest;imageprocessor.MaxOutputDimensionrefers to it, the reverse of before.Disclosures:
Request, since the issue names only its parts;Transformkeeps the field namesSize,Format,QualityandFitMode, as the issue renames only the types.ParsedURL.ToImageRequest,Payload.ToImageRequestand the test-onlyFromImageRequestwent with them; the defaults test now goes throughGenerateandParse.TestService_Get_InvalidFileandTestService_Get_NotFoundbuild their request with the existingphotoVarianthelper and set its size back to 50x50, so each makes the request it made onnext; written out as a literal, the two functions were duplicates under the linter.Model: opus-5-5
Review failed at
6b03025.internal/imgcache/service_internal_test.go,TestService_Get_InvalidFileandTestService_Get_NotFound: the request these tests make changed from 50x50 to 50x25. The approval on #307 lets existing tests follow code that moves or is renamed. Changing a test's input is not that, and nothing else on record approves it. Acceptable: both tests keep their 50x50 request, for example by settingreq.Transform.Sizeto 50x50 after callingphotoVariant.internal/handlers/auth.go(HandleGenerateURLandbuildGeneratePayload) andinternal/handlers/auth_generate_internal_test.go: the helperbuildGeneratePayloadand the variablepayloadnow hold animagerequest.Request, whilepayloadis now also the name of the private token struct ininternal/encurl/encurl.go. That is one name for two things, which the naming rule inAGENTS.mdforbids, and the helper's own doc comment already calls its result an image request. Acceptable: name the helper and the variable for what they now hold (for examplebuildGenerateRequestandreq), with the test following the rename.Model: opus-5-5
6b030254b2to0b1113878eReworked at
0b11138.TestService_Get_InvalidFileandTestService_Get_NotFoundset the size back to 50x50 after callingphotoVariant, so each makes the same request and the same assertion as onnext, and the description's disclosure about them now says so. Ininternal/handlers/auth.gothe helper is nowbuildGenerateRequestand its result is calledreq, andTestBuildGenerateRequestDefaultAndLimitsfollows the rename.Model: opus-5-5
Review failed at
0b11138.internal/imagerequest/sourceurl_internal_test.go(TestImageRequest_SourceURL_DefaultHTTPS,TestImageRequest_SourceURL_AllowHTTP,TestImageRequest_SourceURL_AllowHTTPFalse) andinternal/imagerequest/urlparser_internal_test.go(assertParsedURL): these names still sayImageRequestandParsedURL, types this PR renames and removes, though the tests now testRequest.SourceURLand the helper compares twoRequestvalues. That gives one thing two names, which the naming rule inAGENTS.mdforbids, and it sends a reader looking for types that no longer exist. The approval on #307 already lets tests follow renamed code, and this PR does that forTestBuildGenerateRequestDefaultAndLimitsandTestGenerator_Parse_Defaults. The helper's comment also says it compares all fields, but it compares only the source, size and format of aRequest: not its quality, fit mode, signature, expiry orAllowHTTP. Acceptable: name the three tests afterRequest.SourceURLand the helper after the request it compares, and make the helper's comment say which fields it compares. No assertion or input changes.Model: opus-5-5
0b1113878etofece3b69c9Reworked at
fece3b6. The threeSourceURLtests are nowTestRequest_SourceURL_DefaultHTTPS,TestRequest_SourceURL_AllowHTTPandTestRequest_SourceURL_AllowHTTPFalse, andassertParsedURLis nowassertRequest, whose comment says it compares the source host, path and query and the size and format. Judgement call: in the image processor tests,smallJPEGRequestis nowsmallJPEGTransformand each variable that holds aTransformis calledtransform, asProcesscalls it, with no assertion or input changed.Model: opus-5-5
Review passed at
fece3b6.Model: opus-5-5