fix(633): document the 0-based paging contract on the OpenAPI parameters #646

Merged

timothy merged 3 commits from fix/633-openapi-paging-descriptions into main 2026-07-26 13:06:32 +02:00

Conversation 1 Commits 3 Files Changed 13

+312 -31

3 Commits

Author	SHA1	Message	Date
timothy	33e9abdd20	fix(633): assert the cap set, not the presence of one true cap claim ... Round-3 review finding, and a correction to what the previous commit claimed. That commit said the regex matched the cap "as a whole token" and called the result "exact". The whole-token part was true and did fix the 100-within-1000 substring hole. "Exact" was not: `capped at 100(?!\d)` asks only whether a correct claim is PRESENT, which is not the same as asking whether an incorrect one is ABSENT. A description reading "not capped at 1000 for this endpoint; capped at 100 …" satisfied it while publishing a wrong number to every consumer. Enumerate every `capped at <n>` in the description instead and require the set to be exactly one number, the right one. Mutation-verified on the constructed case: /logs naming both 1000 and 100 now reddens the test, where it passed under the previous form. This is the third round on this one assertion, and each round found the previous fix's blind spot rather than a fresh mistake — the failure mode was consistently "the new check tests presence of the right thing, not absence of the wrong thing." Note on verification: the reviewer could not run the suite (its sandbox could not create a temp dir, and a direct VSTest invocation could not bind its IPC socket), so it explicitly flagged the 1905/0 result as unverified rather than trusting it. That figure comes from my own run in this worktree, re-run after this change, and CI is the independent confirmation. Refs #633 Decisions-Edit: yes	2026-07-26 11:31:11 +02:00
timothy	23791c1bbb	fix(633): review fixes — the all-items upper clamp, and a cap test that couldn't fail ... Independent review (Codex) found three, two of them real defects rather than polish. 1. `search/all-items` is the one paged endpoint that clamps `pageNum` ABOVE as well as below — `Math.Clamp(pageNum, 0, MaxAllItemsPageNum)`, 2,000,000, so pageNum*pageSize cannot overflow int into a 500. The description documented only the lower clamp, so the published contract looked unbounded: a client sending pageNum=int.MaxValue is silently served page 2,000,000. Now stated, and called out as the exception it is. 2. The cap assertion could not fail in the direction that matters. `ShouldContain( "capped at 100")` is satisfied by the string "capped at 1000", so a cap-100 endpoint whose description claimed 1000 passed — precisely the wrong-cap defect the test was added to catch, and a test that cannot fail on its own subject is worse than none. Matched as a whole token instead, and mutation-verified: making /logs claim 1000 now reddens it, where before it stayed green. 3. `Description` used `First`, so a missing parameter threw "Sequence contains no matching element" — naming neither endpoint nor parameter, and reading as a broken test rather than the contract violation it is. Fails informatively now. The review confirmed independently that 12 is the complete paged set, that every other cap matches its controller, that the attributes are runtime-inert, and that the unchanged TypeScript client and endpoint index are correct rather than a missed regen. Refs #633 Decisions-Edit: yes	2026-07-26 11:23:51 +02:00
timothy	214fad2dcd	fix(633): document the 0-based paging contract on the OpenAPI parameters ... `api.paging-zero-based` says `pageNum` is 0-based across `/api/v1` and every wrapper of it. That was true of the MCP tool catalog and the docs, and not true of the generated OpenAPI document: all 24 paging parameters across the 12 paged operations were emitted with no `description` at all, so a consumer reading only `v1.json` — the intended contract, and what generated clients surface to their users — had to infer the base from `default: 0`. That is the same inference that cost #487 a verification pass on the MCP side, where the description was present but wrong. Annotates each `[FromQuery]` paging parameter with `[Description]` (`System.ComponentModel`), the mechanism `parentId` already used in ImagesController, and regenerates `v1.json`. `pageSize` states the endpoint's OWN cap, because the caps genuinely differ — 100 typical, 200 auto-tune members, 1000 search/all-items — and the record forbids documenting one global number; it also states that the offset derives from the effective (capped) size, so an over-large `pageSize` narrows the page instead of widening the offset. The generated TypeScript client covers DTOs only, not query parameters, so it is unchanged; `endpoint-index.md` carries summaries, not parameter descriptions, so it is unchanged too. Pinned by OpenApiPagingContractTests against the in-process generated document. The test NAMES the expected set of 12 paged operations rather than only filtering for parameters called `pageNum`: a filter cannot see an endpoint that should page and doesn't, which is exactly how two MCP tools escaped the equivalent check in #616. Set equality is asserted in both directions, and the caps are pinned per endpoint so a description naming the wrong cap fails — a wrong justification outlives a wrong line. Mutation-verified both ways: dropping one `[Description]` reddens the description test, and making one endpoint stop exposing `pageNum`/`pageSize` under those names reddens the set-equality test. Refs #633 Decisions-Edit: yes	2026-07-26 11:10:22 +02:00