From 8d35a2792fa595960d6a287328cbb79152245bd1 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 25 Jul 2026 23:16:00 +0200 Subject: [PATCH 1/4] fix(616): document paging as 0-based, expose channelId on playout detail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #616 filed three MCP/API paging traps. Two were real; one was not, and one was already half-fixed on main. Verified each against the code before changing it. REAL — pageNum documented as 1-based. `ToolCatalog.Page()` described pageNum as "1-based page number" while every paged controller defaults it to 0, floors it with `Math.Max(0, pageNum)`, and skips `PageNum * PageSize`. A caller that trusted the description started at page 1 and silently lost the first page: no error, just a short set that reads as data loss rather than an off-by-one (it cost #487 a verification pass). Fixed in the description rather than by making the MCP layer 1-based: /api/v1 is additive-only post-freeze, 0-based is load-bearing in a dozen controllers and the SPA, and a 1-based wrapper over a 0-based API would make the same parameter name mean two different things on two surfaces a reader reads together. NOT REAL — "pageSize caps the page but the offset honors the requested value". Not reproducible on any endpoint. Every controller clamps before passing, every handler skips by the clamped size, and GetCollectionItemsHandler re-clamps defensively. The reported observation (pageSize=500&pageNum=2 on a 204-item collection returning 4 items) is exactly correct 0-based behaviour at the clamped width of 100 — page 2 is items 201-204. The issue's own trap-1 table states this. Pinned by test rather than "fixed". ALREADY FIXED — playout LIST rows gained channelId in #297 (2026-07-22), three days before #616 was filed; the report was measured against prod, which runs an older :prod image. The DETAIL response (PlayoutResponseModel) genuinely still lacked it, so channelId is added there (additive) and the reset_channel_playout argument now names the trap: the id spaces overlap numerically, so passing a playout id silently resets a different channel and returns a plausible 202. Tests, both mutation-verified (each fails when its fix is reverted): - ToolCatalogTests pins "0-based" on EVERY paged tool's pageNum description, with a non-empty guard so it can't pass vacuously over an empty tool set. - GetCollectionItemsHandlerTests pins 0-based page boundaries and proves the offset derives from the clamped pageSize (page 1 at pageSize=500 returns items 101-150; the mutation that honors 500 returns an empty page). Docs: new decision record api.paging-zero-based (catalog regenerated), the api-conventions paging bullet, and a Paging section in docs/mcp.md. OpenAPI v1.json + web/src/api/generated/v1.d.ts regenerated for the added field. fixes #616 Co-Authored-By: Claude Opus 5 (1M context) --- .../Api/Playouts/PlayoutResponseModel.cs | 3 + ErsatzTV.Mcp.Tests/ToolCatalogTests.cs | 29 +++++++ ErsatzTV.Mcp/ToolCatalog.cs | 21 ++++- .../GetCollectionItemsHandlerTests.cs | 87 +++++++++++++++++++ .../Controllers/PlayoutControllerTests.cs | 20 +++++ ErsatzTV/Controllers/Api/PlayoutController.cs | 1 + ErsatzTV/wwwroot/openapi/v1.json | 5 ++ docs/api-conventions.md | 7 +- docs/decisions/README.md | 1 + .../records/api/paging-zero-based.md | 42 +++++++++ docs/mcp.md | 16 ++++ web/src/api/generated/v1.d.ts | 1 + 12 files changed, 229 insertions(+), 4 deletions(-) create mode 100644 docs/decisions/records/api/paging-zero-based.md diff --git a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs index 00d023216..15cfebba5 100644 --- a/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs +++ b/ErsatzTV.Core/Api/Playouts/PlayoutResponseModel.cs @@ -8,6 +8,7 @@ public record PlayoutResponseModel( PlayoutScheduleKind ScheduleKind, string ChannelName, string ChannelNumber, + int ChannelId, ChannelPlayoutMode PlayoutMode, string ScheduleName, string? ScheduleFile, @@ -23,6 +24,7 @@ public record PlayoutResponseModel( PlayoutScheduleKind scheduleKind, string channelName, string channelNumber, + int channelId, ChannelPlayoutMode playoutMode, string scheduleName, string? scheduleFile, @@ -37,6 +39,7 @@ public record PlayoutResponseModel( scheduleKind, channelName, channelNumber, + channelId, playoutMode, scheduleName, scheduleFile, diff --git a/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs b/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs index e7fb2d7c4..ee0a5aaa2 100644 --- a/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs +++ b/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs @@ -202,6 +202,35 @@ public class ToolCatalogTests (tool.QueryParameters ?? new HashSet()).ShouldNotContain("ifMatch"); } + // #616: the catalog described pageNum as "1-based" while every paged REST controller defaults it + // to 0 and skips pageNum * pageSize. A caller that trusted the description started at page 1 and + // silently lost the first page — no error, just a short set that reads like missing data. The + // description is the whole contract an MCP client has, so it is pinned here for EVERY paged tool. + [Test] + public void Paged_Tools_Should_Document_PageNum_As_Zero_Based() + { + ToolDefinition[] paged = ToolCatalog.All + .Where(t => t.InputSchema.RootElement.GetProperty("properties").TryGetProperty("pageNum", out _)) + .ToArray(); + + // Guard the guard: if the catalog stops exposing paged tools this test must fail loudly + // rather than pass vacuously over an empty set. + paged.ShouldNotBeEmpty(); + + foreach (ToolDefinition tool in paged) + { + string description = tool.InputSchema.RootElement + .GetProperty("properties") + .GetProperty("pageNum") + .GetProperty("description") + .GetString() + .ShouldNotBeNull(); + + description.ShouldContain("0-based"); + description.ShouldNotContain("1-based"); + } + } + [Test] public void Scan_Library_Tool_Should_Register_Deep_As_A_Query_Parameter() { diff --git a/ErsatzTV.Mcp/ToolCatalog.cs b/ErsatzTV.Mcp/ToolCatalog.cs index 8c3e675f6..93fa059be 100644 --- a/ErsatzTV.Mcp/ToolCatalog.cs +++ b/ErsatzTV.Mcp/ToolCatalog.cs @@ -161,7 +161,12 @@ public static class ToolCatalog "ersatztv_reset_channel_playout", "Queue a rebuild of a channel's playout (202 Accepted; 409 if a build is already running).", "/api/v1/channels/{id}/playout/reset", - [IdPath("Channel id.")], + [ + IdPath( + "CHANNEL id — not the playout id. The two id spaces overlap numerically, so " + + "passing a playout id here silently resets a different channel; take this " + + "value from a playout row's channelId field (issue #616).") + ], [Str("mode", "Optional playout build mode; omit for the default. GET a playout to see valid values.", arg: In.Query)]), Delete("ersatztv_delete_channel", "Delete a channel.", "/api/v1/channels/{id}", IdPath("Channel id.")), Put( @@ -230,10 +235,20 @@ public static class ToolCatalog private static Arg ObjArray(string name, string description, bool required = false) => new(name, "array", description, required, In.Body, ItemType: "object"); + // Paging mirrors the REST API it wraps, which is 0-BASED everywhere (issue #616): every paged + // controller defaults pageNum to 0 and skips `pageNum * pageSize`. The description said "1-based", + // so a caller that trusted it silently skipped the first page and read the result as data loss. + // Documented rather than translated: a 1-based MCP over a 0-based API would make the same + // parameter name mean two different things depending on which surface you were reading. + // pageSize is clamped server-side (100 on most endpoints), so an over-large value yields + // clamped-width pages — it does not widen the offset. private static Arg[] Page() => [ - Int("pageNum", "1-based page number (optional).", arg: In.Query), - Int("pageSize", "Page size (optional).", arg: In.Query) + Int("pageNum", "0-based page number; the first page is 0 (optional, default 0).", arg: In.Query), + Int( + "pageSize", + "Page size (optional). Clamped server-side, so pages may be narrower than requested.", + arg: In.Query) ]; // The channel create/update body (CreateChannelRequest / UpdateChannelRequest — the id comes from diff --git a/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs b/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs index 99ecf15bf..8f7e4255f 100644 --- a/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs +++ b/ErsatzTV.Tests/Application/MediaCollections/GetCollectionItemsHandlerTests.cs @@ -61,6 +61,93 @@ public class GetCollectionItemsHandlerTests page.Page.Select(i => i.Title).ShouldBe(["Zeta", "Alpha", "Beta"]); } + // Paging semantics, pinned because #616 reported them as two bugs that measurement did not + // support. pageNum is 0-BASED (the trap: the MCP catalog documented it as 1-based, so a caller + // starting at 1 silently skipped the first page and read a short set as data loss). + [Test] + public async Task Handle_Should_Treat_PageNum_As_Zero_Based() + { + await SeedNumberedCollection(150); + var handler = new GetCollectionItemsHandler(_db.Factory); + + Either first = + await handler.Handle(new GetCollectionItems(10, 0, 10), CancellationToken.None); + Either second = + await handler.Handle(new GetCollectionItems(10, 1, 10), CancellationToken.None); + + // Page 0 is the FIRST page, not a skipped one; page 1 is the second. + first.RightToSeq().Single().Page.Select(i => i.Title).First().ShouldBe("Item 001"); + second.RightToSeq().Single().Page.Select(i => i.Title).First().ShouldBe("Item 011"); + } + + // The second half of #616's claim was that an over-large pageSize caps the returned page but + // leaves the OFFSET computed from the requested value, so page 1 at pageSize=500 would land past + // item 500. It does not: the size is clamped first and the offset derives from the clamped value. + [Test] + public async Task Handle_Should_Derive_Offset_From_The_Clamped_PageSize() + { + await SeedNumberedCollection(150); + var handler = new GetCollectionItemsHandler(_db.Factory); + + // pageSize 500 clamps to 100, so page 1 starts at item 101 and runs to the end (50 items). + // If the offset honored the requested 500, this page would start past the collection and be + // empty — which is exactly what the mutation of this fix produces. + Either result = + await handler.Handle(new GetCollectionItems(10, 1, 500), CancellationToken.None); + + PagedLibraryBrowseItemsResponseModel page = result.RightToSeq().Single(); + page.TotalCount.ShouldBe(150); + page.Page.Count.ShouldBe(50); + page.Page.Select(i => i.Title).First().ShouldBe("Item 101"); + page.Page.Select(i => i.Title).Last().ShouldBe("Item 150"); + } + + private async Task SeedNumberedCollection(int count) + { + await using TvContext context = _db.CreateContext(); + + var library = new LocalLibrary + { + Id = 1, + Name = "Library", + MediaKind = LibraryMediaKind.Movies, + Paths = [] + }; + var path = new LibraryPath + { + Id = 1, + Path = "/media", + Library = library, + LibraryFolders = [], + MediaItems = [] + }; + library.Paths.Add(path); + + var collection = new Collection + { + Id = 10, + Name = "Manual", + UseCustomPlaybackOrder = false, + MediaItems = [], + CollectionItems = [], + MultiCollections = [], + MultiCollectionItems = [] + }; + + var movies = new List(); + for (var i = 1; i <= count; i++) + { + // Zero-padded so the handler's title ordering matches numeric order. + movies.Add(MakeMovie(1000 + i, path, $"Item {i:D3}")); + collection.CollectionItems.Add(new CollectionItem { MediaItemId = 1000 + i }); + } + + context.LocalLibraries.Add(library); + context.Movies.AddRange(movies); + context.Collections.Add(collection); + await context.SaveChangesAsync(); + } + private async Task SeedCollectionGraph(bool useCustomPlaybackOrder) { await using TvContext context = _db.CreateContext(); diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 9a23ce239..ab59c4893 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -166,6 +166,25 @@ public class PlayoutControllerTests .Seed.ShouldBe(4242); } + // #616: the playout DETAIL response carried channelName/channelNumber but no channelId, while + // reset_channel_playout takes a CHANNEL id. The two id spaces overlap numerically, so a caller + // that reached for the row's `id` reset a different channel and got a plausible 202 back. The + // list rows gained channelId in #297; this pins the same field on the detail response, and the + // distinct ids below prove it is the channel's, not the playout's. + [Test] + public async Task GetById_Should_Surface_ChannelId_Distinct_From_PlayoutId() + { + _mediator.Send(Arg.Any(), Arg.Any()) + .Returns(Option.Some(MakePlayout(9) with { ChannelId = 400 })); + + IActionResult result = await _controller.GetById(9, CancellationToken.None); + + PlayoutResponseModel body = result.ShouldBeOfType() + .Value.ShouldBeOfType(); + body.Id.ShouldBe(9); + body.ChannelId.ShouldBe(400); + } + [Test] public async Task GetAll_Should_Surface_Seed() { @@ -1432,6 +1451,7 @@ public class PlayoutControllerTests vm.ScheduleKind, vm.ChannelName, vm.ChannelNumber, + vm.ChannelId, vm.PlayoutMode, vm.ScheduleName, vm.ScheduleFile, diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index a52f8d85a..4fa94da83 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -818,6 +818,7 @@ public class PlayoutController(IMediator mediator, IEntityLocker entityLocker) : vm.ScheduleKind, vm.ChannelName, vm.ChannelNumber, + vm.ChannelId, vm.PlayoutMode, vm.ScheduleName, vm.ScheduleFile, diff --git a/ErsatzTV/wwwroot/openapi/v1.json b/ErsatzTV/wwwroot/openapi/v1.json index 02dec0207..5d384d488 100644 --- a/ErsatzTV/wwwroot/openapi/v1.json +++ b/ErsatzTV/wwwroot/openapi/v1.json @@ -28954,6 +28954,7 @@ "scheduleKind", "channelName", "channelNumber", + "channelId", "playoutMode", "scheduleName", "scheduleFile", @@ -28979,6 +28980,10 @@ "channelNumber": { "type": "string" }, + "channelId": { + "type": "integer", + "format": "int32" + }, "playoutMode": { "$ref": "#/components/schemas/ChannelPlayoutMode" }, diff --git a/docs/api-conventions.md b/docs/api-conventions.md index 636ae60f9..db3546bd5 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -41,7 +41,12 @@ Exemplars: - **Paged GET with clamped params**: `ErsatzTV/Controllers/Api/LogsController.cs` — `pageNum` clamped via `Math.Max(0, pageNum)`, `pageSize` via `Math.Clamp(pageSize, 1, MaxPageSize)` (`MaxPageSize = 100`). Any new paged endpoint should clamp the same way — don't trust client - input for page math. + input for page math. **`pageNum` is 0-based** across the whole surface (the first page is `0`) and + the offset is always derived from the *clamped* `pageSize`, so an over-large `pageSize` yields + narrower pages — it never widens the offset. Say "0-based" in the description of any paging + parameter you expose, including on wrapper surfaces like the MCP tool catalog: describing it as + 1-based makes a caller skip the first page silently, which reads as data loss rather than as an + off-by-one (ersatztv#616). See `api.paging-zero-based`. - **Sortable GET with allow-listed sort params**: same file — `sortField`/`sortDirection` are normalized against a fixed allow-list (`AllowedSortFields`) rather than trusted or rejected with a 422: an unrecognized `sortField` silently falls back to the default field, an unrecognized diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 83b55d094..fe7e19d72 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -19,6 +19,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `api.logs-sort-params` | `GET /api/logs` takes allow-listed `sortField` (`timestamp`\|`level`) and `sortDirection` (`asc`\|`desc`) query params, normalized (not rejected) on an unrecognized value. | 2026-07-11 | [link](records/api/logs-sort-params.md) | | `api.mediatr-passthrough` | The REST API is thin controllers over existing MediatR handlers, with no new service/business-logic layer. | 2026-06 | [link](records/api/mediatr-passthrough.md) | | `api.openapi-mirrors-runtime` | The generated OpenAPI spec is made to match the runtime Newtonsoft wire contract (via `NewtonsoftSchemaNamingTransformer`), not the reverse. | 2026-07-09 | [link](records/api/openapi-mirrors-runtime.md) | +| `api.paging-zero-based` | `pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the CLAMPED `pageSize`, so an over-large `pageSize` narrows the page without widening the offset. A paging parameter description that omits or contradicts "0-based" is a defect. | 2026-07-25 | [link](records/api/paging-zero-based.md) | | `api.parentid-drillin` | Media drill-in (season/episode/artist/music-video) is served by an optional `parentId` query param on library-browse, not dedicated per-kind child-listing endpoints. | 2026-07-07 | [link](records/api/parentid-drillin.md) | | `api.playout-build-lock-409` | Every id-keyed playout/channel mutation endpoint checks `IEntityLocker.IsPlayoutLocked(id)` and returns 409 Conflict while a build is in-flight, mirroring Blazor's disabled-buttons behavior; reset-all stays 202 and silently skips locked playouts. | 2026-07-10 | [link](records/api/playout-build-lock-409.md) | | `api.postcommit-cancellation-none` | Once a mutation commits, its entire compensating side effect (enqueues, publishes, reindexes, cache refresh, and any post-commit lookup gating one of those) runs on `CancellationToken.None` so a late client disconnect can't half-abort an already-committed change. | 2026-07-11 | [link](records/api/postcommit-cancellation-none.md) | diff --git a/docs/decisions/records/api/paging-zero-based.md b/docs/decisions/records/api/paging-zero-based.md new file mode 100644 index 000000000..a232d6138 --- /dev/null +++ b/docs/decisions/records/api/paging-zero-based.md @@ -0,0 +1,42 @@ +--- +key: api.paging-zero-based +title: 2026-07-25 — Paging is 0-based everywhere, and every wrapper says so (#616) +status: active +since: '2026-07-25' +supersedes: none +superseded-by: none +rule: '`pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the CLAMPED `pageSize`, so an over-large `pageSize` narrows the page without widening the offset. A paging parameter description that omits or contradicts "0-based" is a defect.' +signals: '`pageNum`, `pageSize`, `Math.Clamp(pageSize`, `Skip(pageNum * pageSize)`, "1-based", off-by-one paging, short result set, MCP `Page()` · paths: `ErsatzTV.Mcp/ToolCatalog.cs`, `ErsatzTV/Controllers/Api/*Controller.cs` · issues: #616, #487, #58' +mechanics: '`Math.Max(0, pageNum)` + `Math.Clamp(pageSize, 1, MaxPageSize)` in the controller, `Skip(PageNum * PageSize)` in the handler' +--- + +Every paged controller on `/api/v1` defaults `pageNum` to `0`, floors it with `Math.Max(0, pageNum)`, +clamps `pageSize` with `Math.Clamp(pageSize, 1, MaxPageSize)`, and passes **both clamped values** to a +handler that skips `PageNum * PageSize`. That makes paging uniformly 0-based, and makes the offset a +function of the clamped size rather than the requested one. + +The convention was correct in code and unwritten everywhere else, which is how it produced a bug +report. The MCP tool catalog described `pageNum` as "1-based page number", so a caller that started +at `pageNum=1` skipped the first page: a 15-item collection returned 0 items and a 204-item +collection returned 104. Nothing errored — the caller just got a short set, which reads as *data +loss*, not as an off-by-one, and cost a verification pass being chased as one (#487). + +The same report's second claim — that `pageSize` is capped for the returned page while the offset +still honours the requested value — was **not** reproducible and is not true of any endpoint. The +observation behind it (`pageSize=500&pageNum=2` on a 204-item collection returning 4 items) is +exactly correct 0-based behaviour at the clamped width of 100: page 2 is items 201–204. Both +behaviours are now pinned by mutation-verified tests in `GetCollectionItemsHandlerTests`, so the next +reader does not have to re-derive which half was real. + +**Direction of the fix.** The alternative was to make the MCP layer 1-based and translate. Rejected: +`/api/v1` is additive-only post-freeze (`api.versioning-v1`), 0-based is already load-bearing in a +dozen controllers and the SPA, and a 1-based wrapper over a 0-based API would make the *same +parameter name* mean different things on two surfaces a reader routinely reads together — trading a +documented off-by-one for an undocumented one. Accuracy in the description is the cheaper contract. + +**Corollary — ids in paged rows.** A row that names a related entity must expose that entity's id, not +only its display fields. `reset_channel_playout` takes a *channel* id while playout rows exposed only +the playout `id` plus channel name/number; the id spaces overlap numerically, so passing the row's id +silently reset a different channel and returned a plausible 202. List rows gained `channelId` in #297; +#616 added it to `PlayoutResponseModel` (the detail response) and named the trap in the MCP argument +description. diff --git a/docs/mcp.md b/docs/mcp.md index 17cd40f0d..dcb760b18 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -84,6 +84,22 @@ When a response carries an `ETag` header (versioned aggregates emit it on GET an replace PUT), the tool result appends a `\n[etag: "N"]` marker so an agent can round-trip it as `ifMatch` on a subsequent write. +### Paging (`api.paging-zero-based`) + +The paged tools take `pageNum`/`pageSize` and pass them through unchanged — the MCP layer does **not** +translate them, so they mean exactly what they mean on `/api/v1`: + +- **`pageNum` is 0-based.** The first page is `0`. Starting at `1` silently skips a page and returns a + short set with no error, which is easy to misread as missing data (ersatztv#616 — the catalog used + to describe it as 1-based, and #487 lost a verification pass to it). +- **`pageSize` is clamped server-side** (100 on most endpoints, 200 on a few). An over-large value + gives you narrower pages, not a wider offset: `pageSize=500&pageNum=1` returns items 101–200, not + 501–1000. Page to completeness against `totalCount` rather than assuming your requested size held. + +Rows that reference another entity carry that entity's id, not just its display fields — take the +channel id for `ersatztv_reset_channel_playout` from a playout row's `channelId`, never from its `id` +(that is the *playout* id, and the two id spaces overlap numerically). + ### Optimistic concurrency (`api-conventions.md` §7a) Only the **replace-all aggregate PUTs** honor `If-Match` — here that is diff --git a/web/src/api/generated/v1.d.ts b/web/src/api/generated/v1.d.ts index 5e4ad129b..f1edb164a 100644 --- a/web/src/api/generated/v1.d.ts +++ b/web/src/api/generated/v1.d.ts @@ -1168,6 +1168,7 @@ export interface components { "scheduleKind": components["schemas"]["PlayoutScheduleKind"]; "channelName": string; "channelNumber": string; + "channelId": number; "playoutMode": components["schemas"]["ChannelPlayoutMode"]; "scheduleName": string; "scheduleFile": null | string; From 5648f8e92e72c8ae4b3d5224144b41fd2b60a5f4 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 25 Jul 2026 23:24:40 +0200 Subject: [PATCH 2/4] =?UTF-8?q?docs(616):=20scope=20the=20pageSize=20cap?= =?UTF-8?q?=20per=20endpoint=20=E2=80=94=20it=20is=20not=20one=20number?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of the previous commit caught an overclaim I introduced. I wrote the paging docs as if the pageSize cap were uniformly 100. It is not: most reads (collections, library browse, logs, playouts) Math.Clamp(.., 1, 100) GetAutoTuneChannelMembers pageSize <= 0 ? 100 : Min(.., 200) GET /api/v1/search/all-items default 500, cap 1000 That made a concrete example in docs/mcp.md simply false. It claimed `pageSize=500&pageNum=1` returns items 101-200 "not 501-1000" — but ersatztv_search_all_items also takes Page(), and 500 is UNDER its cap, so there page 1 really is items 501-1000. A caller following that example on the one tool most likely to be paged hard would have mis-derived its offsets, which is the same class of silent-short-set error this issue is about. The invariant that actually holds everywhere is the derivation, not any single cap: the offset comes from the EFFECTIVE (bounded) page size, never the requested one. Reworded to say that, in docs/mcp.md, the api.paging-zero-based record (rule + mechanics + body), and the ToolCatalog Page() comment. Catalog regenerated. The record's mechanics line no longer claims every controller uses Math.Clamp — ChannelController does not. No behaviour change; MCP suite still 59/59. Co-Authored-By: Claude Opus 5 (1M context) --- ErsatzTV.Mcp/ToolCatalog.cs | 5 +++-- docs/decisions/README.md | 2 +- docs/decisions/records/api/paging-zero-based.md | 17 ++++++++++++----- docs/mcp.md | 10 +++++++--- 4 files changed, 23 insertions(+), 11 deletions(-) diff --git a/ErsatzTV.Mcp/ToolCatalog.cs b/ErsatzTV.Mcp/ToolCatalog.cs index 93fa059be..e3765393d 100644 --- a/ErsatzTV.Mcp/ToolCatalog.cs +++ b/ErsatzTV.Mcp/ToolCatalog.cs @@ -240,8 +240,9 @@ public static class ToolCatalog // so a caller that trusted it silently skipped the first page and read the result as data loss. // Documented rather than translated: a 1-based MCP over a 0-based API would make the same // parameter name mean two different things depending on which surface you were reading. - // pageSize is clamped server-side (100 on most endpoints), so an over-large value yields - // clamped-width pages — it does not widen the offset. + // pageSize is clamped server-side and the cap is PER-ENDPOINT (100 for most reads, 200 for + // auto-tune members, 1000 for search/all-items). The invariant is not a single number: the + // offset always derives from the EFFECTIVE page size, never the requested one. private static Arg[] Page() => [ Int("pageNum", "0-based page number; the first page is 0 (optional, default 0).", arg: In.Query), diff --git a/docs/decisions/README.md b/docs/decisions/README.md index fe7e19d72..7ee6fde6d 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -19,7 +19,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `api.logs-sort-params` | `GET /api/logs` takes allow-listed `sortField` (`timestamp`\|`level`) and `sortDirection` (`asc`\|`desc`) query params, normalized (not rejected) on an unrecognized value. | 2026-07-11 | [link](records/api/logs-sort-params.md) | | `api.mediatr-passthrough` | The REST API is thin controllers over existing MediatR handlers, with no new service/business-logic layer. | 2026-06 | [link](records/api/mediatr-passthrough.md) | | `api.openapi-mirrors-runtime` | The generated OpenAPI spec is made to match the runtime Newtonsoft wire contract (via `NewtonsoftSchemaNamingTransformer`), not the reverse. | 2026-07-09 | [link](records/api/openapi-mirrors-runtime.md) | -| `api.paging-zero-based` | `pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the CLAMPED `pageSize`, so an over-large `pageSize` narrows the page without widening the offset. A paging parameter description that omits or contradicts "0-based" is a defect. | 2026-07-25 | [link](records/api/paging-zero-based.md) | +| `api.paging-zero-based` | `pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the EFFECTIVE (bounded) `pageSize`, never the requested one, so a `pageSize` above an endpoint's cap narrows the page without widening the offset. The cap itself is per-endpoint (100 typical, 200 auto-tune members, 1000 search/all-items) and must not be documented as one number. A paging parameter description that omits or contradicts "0-based" is a defect. | 2026-07-25 | [link](records/api/paging-zero-based.md) | | `api.parentid-drillin` | Media drill-in (season/episode/artist/music-video) is served by an optional `parentId` query param on library-browse, not dedicated per-kind child-listing endpoints. | 2026-07-07 | [link](records/api/parentid-drillin.md) | | `api.playout-build-lock-409` | Every id-keyed playout/channel mutation endpoint checks `IEntityLocker.IsPlayoutLocked(id)` and returns 409 Conflict while a build is in-flight, mirroring Blazor's disabled-buttons behavior; reset-all stays 202 and silently skips locked playouts. | 2026-07-10 | [link](records/api/playout-build-lock-409.md) | | `api.postcommit-cancellation-none` | Once a mutation commits, its entire compensating side effect (enqueues, publishes, reindexes, cache refresh, and any post-commit lookup gating one of those) runs on `CancellationToken.None` so a late client disconnect can't half-abort an already-committed change. | 2026-07-11 | [link](records/api/postcommit-cancellation-none.md) | diff --git a/docs/decisions/records/api/paging-zero-based.md b/docs/decisions/records/api/paging-zero-based.md index a232d6138..19dcd27f6 100644 --- a/docs/decisions/records/api/paging-zero-based.md +++ b/docs/decisions/records/api/paging-zero-based.md @@ -5,15 +5,22 @@ status: active since: '2026-07-25' supersedes: none superseded-by: none -rule: '`pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the CLAMPED `pageSize`, so an over-large `pageSize` narrows the page without widening the offset. A paging parameter description that omits or contradicts "0-based" is a defect.' +rule: '`pageNum` is 0-based across the entire `/api/v1` surface and every wrapper of it (MCP tool catalog, SPA hooks, docs); the page offset is always derived from the EFFECTIVE (bounded) `pageSize`, never the requested one, so a `pageSize` above an endpoint''s cap narrows the page without widening the offset. The cap itself is per-endpoint (100 typical, 200 auto-tune members, 1000 search/all-items) and must not be documented as one number. A paging parameter description that omits or contradicts "0-based" is a defect.' signals: '`pageNum`, `pageSize`, `Math.Clamp(pageSize`, `Skip(pageNum * pageSize)`, "1-based", off-by-one paging, short result set, MCP `Page()` · paths: `ErsatzTV.Mcp/ToolCatalog.cs`, `ErsatzTV/Controllers/Api/*Controller.cs` · issues: #616, #487, #58' -mechanics: '`Math.Max(0, pageNum)` + `Math.Clamp(pageSize, 1, MaxPageSize)` in the controller, `Skip(PageNum * PageSize)` in the handler' +mechanics: '`Math.Max(0, pageNum)` + a per-endpoint upper bound on `pageSize` (`Math.Clamp(pageSize, 1, MaxPageSize)` in most controllers) then `Skip(PageNum * PageSize)` in the handler' --- Every paged controller on `/api/v1` defaults `pageNum` to `0`, floors it with `Math.Max(0, pageNum)`, -clamps `pageSize` with `Math.Clamp(pageSize, 1, MaxPageSize)`, and passes **both clamped values** to a -handler that skips `PageNum * PageSize`. That makes paging uniformly 0-based, and makes the offset a -function of the clamped size rather than the requested one. +bounds `pageSize` above by a per-endpoint maximum, and passes **both bounded values** to a handler +that skips `PageNum * PageSize`. That makes paging uniformly 0-based, and makes the offset a function +of the effective size rather than the requested one. + +The *maximum* is deliberately not uniform and must not be documented as if it were: most reads clamp +`Math.Clamp(pageSize, 1, MaxPageSize)` with `MaxPageSize = 100`, `GetAutoTuneChannelMembers` uses +`pageSize <= 0 ? 100 : Math.Min(pageSize, 200)`, and `GET /api/v1/search/all-items` defaults to 500 +and caps at 1000. So `pageSize=500` is narrowed to 100 on a collection listing and honored verbatim +on all-items. The invariant that holds everywhere is the *derivation* (offset from the effective +size), not any single cap. The convention was correct in code and unwritten everywhere else, which is how it produced a bug report. The MCP tool catalog described `pageNum` as "1-based page number", so a caller that started diff --git a/docs/mcp.md b/docs/mcp.md index dcb760b18..0cbd39962 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -92,9 +92,13 @@ translate them, so they mean exactly what they mean on `/api/v1`: - **`pageNum` is 0-based.** The first page is `0`. Starting at `1` silently skips a page and returns a short set with no error, which is easy to misread as missing data (ersatztv#616 — the catalog used to describe it as 1-based, and #487 lost a verification pass to it). -- **`pageSize` is clamped server-side** (100 on most endpoints, 200 on a few). An over-large value - gives you narrower pages, not a wider offset: `pageSize=500&pageNum=1` returns items 101–200, not - 501–1000. Page to completeness against `totalCount` rather than assuming your requested size held. +- **`pageSize` is clamped server-side, and the cap is per-endpoint** — 100 for most reads + (collections, library browse, logs, playouts), 200 for auto-tune channel members, 1000 for + `ersatztv_search_all_items` (whose *default* is 500). The invariant is not a single number: it is + that the offset is derived from the **effective** page size, never the requested one. So + `pageSize=500` on a 100-capped tool gives 100-wide pages (page 1 = items 101–200), while the same + value on `search_all_items` is under its cap and is honored (page 1 = items 501–1000). Don't assume + your requested size held — page to completeness against `totalCount`. Rows that reference another entity carry that entity's id, not just its display fields — take the channel id for `ersatztv_reset_channel_playout` from a playout row's `channelId`, never from its `id` From af39123e8e4a092025d2b78961249be87da058e2 Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 25 Jul 2026 23:41:54 +0200 Subject: [PATCH 3/4] fix(616): fix a live 1-based SPA caller and un-cap two MCP playout tools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Findings from the cold cross-family review of 8d35a279/5648f8e9. The review confirmed the three conclusions in 8d35a279 (the offset math is sound on all 12 paged endpoints; the list DTO really did already carry ChannelId; the new tests are non-vacuous) but found the previous commits had documented a convention without checking who actually violates it. HIGH — a live instance of this issue's own bug class, in the SPA. web/src/screens/SchedulesScreen.tsx asked for the rerun-collection picker with `pageNum: 1, pageSize: 1000`. pageNum is 0-based and pageSize clamps to 100, so the request skipped the first 100 rows: with <=100 rerun collections (the normal case) the picker was served an EMPTY page and silently offered no rerun collections at all; above 100 it dropped rows 1-100. Exactly the silent-short-set failure #616 is about, shipped in the UI. Now `pageNum: 0`, with a vitest that asserts the offset and is mutation-verified (restoring `pageNum: 1` fails it). Checked the rest of the SPA rather than assuming: every other pageNum caller is 0-based. CollectionsScreen's paging loop starts at 1 but only after fetching page 0 explicitly, so it is correct — verified before touching it. MEDIUM — two MCP tools wrapped paged endpoints while declaring no paging args. ersatztv_list_playouts and ersatztv_get_playout_items had no pageNum/pageSize, and ToolArgumentValidator rejects undeclared arguments, so an agent was hard capped at the first 100 rows with no way to ask for more and no error saying so. Both now take Page(). A catalog-wide sweep confirmed these were the only two: the other paged endpoints are not exposed as MCP tools at all. That same gap made the new ToolCatalog test vacuous in the direction that mattered — it filtered on tools that ALREADY declare pageNum, so a tool missing paging entirely escaped it. It now pins the expected set by name, so a new tool over a paged endpoint has to be added deliberately. Record corrections (these are read as normative, so over-broad claims are defects): "every wrapper says 0-based" was false for the OpenAPI surface, whose 12 pageNum parameters carry no description — named as a remaining gap instead of claimed as done. "Every controller floors with Math.Max" ignored SearchController's Math.Clamp. The ids-in-rows corollary was stated as an audit result when PlayoutListItemResponseModel.ScheduleName has no schedule id; restated as a rule about actionable ids. Verification: .NET 1900 + MCP 59 pass; web 996 pass across 105 files; tsc clean; eslint clean; format gate exit 0; no BOMs. Co-Authored-By: Claude Opus 5 (1M context) --- ErsatzTV.Mcp.Tests/ToolCatalogTests.cs | 22 ++++++++++++++++--- ErsatzTV.Mcp/ToolCatalog.cs | 9 ++++++-- .../records/api/paging-zero-based.md | 19 +++++++++++----- docs/mcp.md | 8 +++++-- web/src/screens/SchedulesScreen.test.tsx | 16 ++++++++++++++ web/src/screens/SchedulesScreen.tsx | 5 ++++- 6 files changed, 66 insertions(+), 13 deletions(-) diff --git a/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs b/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs index ee0a5aaa2..d220511f3 100644 --- a/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs +++ b/ErsatzTV.Mcp.Tests/ToolCatalogTests.cs @@ -213,9 +213,25 @@ public class ToolCatalogTests .Where(t => t.InputSchema.RootElement.GetProperty("properties").TryGetProperty("pageNum", out _)) .ToArray(); - // Guard the guard: if the catalog stops exposing paged tools this test must fail loudly - // rather than pass vacuously over an empty set. - paged.ShouldNotBeEmpty(); + // Guard the guard twice over. An emptiness check alone is not enough: this test filters on + // tools that ALREADY declare pageNum, so a tool wrapping a paged endpoint while declaring no + // paging args escapes the filter entirely and the test still passes. That is not + // hypothetical — ersatztv_list_playouts and ersatztv_get_playout_items did exactly that, and + // because ToolArgumentValidator rejects undeclared arguments, an MCP caller was hard-capped + // at the first 100 rows with no way to ask for more. So the expected set is named here: a + // new tool over a paged endpoint must be added to it, and dropping paging from any of these + // fails the test rather than silently shrinking its scope. + string[] mustDeclarePaging = + [ + "ersatztv_get_collection_items", + "ersatztv_get_playout_items", + "ersatztv_list_playouts", + "ersatztv_search", + "ersatztv_search_all_items" + ]; + + paged.Select(t => t.Name).OrderBy(n => n, StringComparer.Ordinal) + .ShouldBe(mustDeclarePaging.OrderBy(n => n, StringComparer.Ordinal)); foreach (ToolDefinition tool in paged) { diff --git a/ErsatzTV.Mcp/ToolCatalog.cs b/ErsatzTV.Mcp/ToolCatalog.cs index e3765393d..f18c93032 100644 --- a/ErsatzTV.Mcp/ToolCatalog.cs +++ b/ErsatzTV.Mcp/ToolCatalog.cs @@ -29,9 +29,14 @@ public static class ToolCatalog Get("ersatztv_list_schedules", "List schedules.", "/api/v1/schedules"), Get("ersatztv_get_schedule", "Get a schedule by id.", "/api/v1/schedules/{id}", IdPath("Schedule id.")), Get("ersatztv_get_schedule_items", "Get a schedule's items. Emits the schedule ETag.", "/api/v1/schedules/{id}/items", IdPath("Schedule id.")), - Get("ersatztv_list_playouts", "List playouts.", "/api/v1/playouts"), + Get("ersatztv_list_playouts", "List playouts (paged).", "/api/v1/playouts", [], Page()), Get("ersatztv_get_playout", "Get a playout by id.", "/api/v1/playouts/{id}", IdPath("Playout id.")), - Get("ersatztv_get_playout_items", "Get upcoming items (and unscheduled gaps) for a playout.", "/api/v1/playouts/{id}/items", IdPath("Playout id.")), + Get( + "ersatztv_get_playout_items", + "Get upcoming items (and unscheduled gaps) for a playout (paged).", + "/api/v1/playouts/{id}/items", + [IdPath("Playout id.")], + Page()), Get("ersatztv_list_ffmpeg_profiles", "List FFmpeg profiles.", "/api/v1/ffmpeg/profiles"), Get("ersatztv_get_ffmpeg_profile", "Get an FFmpeg profile by id.", "/api/v1/ffmpeg/profiles/{id}", IdPath("FFmpeg profile id.")), Get( diff --git a/docs/decisions/records/api/paging-zero-based.md b/docs/decisions/records/api/paging-zero-based.md index 19dcd27f6..6cd85d6bd 100644 --- a/docs/decisions/records/api/paging-zero-based.md +++ b/docs/decisions/records/api/paging-zero-based.md @@ -10,9 +10,10 @@ signals: '`pageNum`, `pageSize`, `Math.Clamp(pageSize`, `Skip(pageNum * pageSize mechanics: '`Math.Max(0, pageNum)` + a per-endpoint upper bound on `pageSize` (`Math.Clamp(pageSize, 1, MaxPageSize)` in most controllers) then `Skip(PageNum * PageSize)` in the handler' --- -Every paged controller on `/api/v1` defaults `pageNum` to `0`, floors it with `Math.Max(0, pageNum)`, -bounds `pageSize` above by a per-endpoint maximum, and passes **both bounded values** to a handler -that skips `PageNum * PageSize`. That makes paging uniformly 0-based, and makes the offset a function +Every paged controller on `/api/v1` defaults `pageNum` to `0`, floors it at 0 (`Math.Max(0, pageNum)` +— `search/all-items` uses `Math.Clamp(pageNum, 0, 2_000_000)` because it additionally needs an upper +bound to keep `pageNum * pageSize` inside `int`), bounds `pageSize` above by a per-endpoint maximum, +and passes **both bounded values** to a handler that skips `PageNum * PageSize`. That makes paging uniformly 0-based, and makes the offset a function of the effective size rather than the requested one. The *maximum* is deliberately not uniform and must not be documented as if it were: most reads clamp @@ -41,8 +42,16 @@ dozen controllers and the SPA, and a 1-based wrapper over a 0-based API would ma parameter name* mean different things on two surfaces a reader routinely reads together — trading a documented off-by-one for an undocumented one. Accuracy in the description is the cheaper contract. -**Corollary — ids in paged rows.** A row that names a related entity must expose that entity's id, not -only its display fields. `reset_channel_playout` takes a *channel* id while playout rows exposed only +**Where "0-based" is stated, and where it still isn't.** The MCP tool catalog and these docs say it +explicitly. The generated OpenAPI `pageNum` parameters carry **no description at all** (12 of them), +so a REST consumer reading only `v1.json` still has to infer the base from the default — a real +remaining gap, tracked separately rather than fixed here. Treat "every wrapper says 0-based" as the +target this record sets, not a property already true of the OpenAPI surface. + +**Corollary — ids in paged rows.** A row that names a related entity should expose that entity's id, +not only its display fields, wherever a caller is expected to act on that entity. This is a rule about +actionable ids, not an audit result: `PlayoutListItemResponseModel.ScheduleName` still ships without +a schedule id, which is fine while nothing asks a caller to address a schedule from that row. `reset_channel_playout` takes a *channel* id while playout rows exposed only the playout `id` plus channel name/number; the id spaces overlap numerically, so passing the row's id silently reset a different channel and returned a plausible 202. List rows gained `channelId` in #297; #616 added it to `PlayoutResponseModel` (the detail response) and named the trap in the MCP argument diff --git a/docs/mcp.md b/docs/mcp.md index 0cbd39962..f34a6f3c7 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -86,8 +86,12 @@ replace PUT), the tool result appends a `\n[etag: "N"]` marker so an agent can r ### Paging (`api.paging-zero-based`) -The paged tools take `pageNum`/`pageSize` and pass them through unchanged — the MCP layer does **not** -translate them, so they mean exactly what they mean on `/api/v1`: +Tools whose endpoint is paged declare `pageNum`/`pageSize` and pass them through unchanged — the MCP +layer does **not** translate them, so they mean exactly what they mean on `/api/v1`. (A tool that +wraps a paged endpoint *without* declaring these is a bug, not a "no paging needed" signal: +`ToolArgumentValidator` rejects undeclared arguments, so the caller is hard-capped at the first page +with no way to ask for more. `ersatztv_list_playouts` and `ersatztv_get_playout_items` were capped +that way until #616. `ToolCatalogTests` pins the expected set.) - **`pageNum` is 0-based.** The first page is `0`. Starting at `1` silently skips a page and returns a short set with no error, which is easy to misread as missing data (ersatztv#616 — the catalog used diff --git a/web/src/screens/SchedulesScreen.test.tsx b/web/src/screens/SchedulesScreen.test.tsx index 72ddbfe75..bc527c8cf 100644 --- a/web/src/screens/SchedulesScreen.test.tsx +++ b/web/src/screens/SchedulesScreen.test.tsx @@ -170,6 +170,22 @@ describe('SchedulesScreen — load', () => { expect(screen.getByLabelText('Schedule lineup')).toBeTruthy(); }); + it('requests the rerun-collection picker from page 0, not page 1 (#616)', async () => { + const handle = await renderReady(); + + const rerunRequests = handle.requests.filter((r) => r.url.startsWith('/api/v1/rerun-collections')); + expect(rerunRequests.length).toBeGreaterThan(0); + + // pageNum is 0-based (api.paging-zero-based). This asked for page 1, and because pageSize is + // clamped to 100 server-side that skipped the first 100 rows — so the picker showed nothing at + // all for the ordinary case of <=100 rerun collections. Assert the offset, not just the URL + // shape: `pageNum=1` here is a silent empty picker, never an error. + for (const request of rerunRequests) { + const pageNum = new URL(request.url, 'http://localhost').searchParams.get('pageNum'); + expect(pageNum).toBe('0'); + } + }); + it('renders the "Active schedule" selector non-full-width so it cannot overflow the header (#463)', async () => { await renderReady(); const select = screen.getByLabelText('Active schedule'); diff --git a/web/src/screens/SchedulesScreen.tsx b/web/src/screens/SchedulesScreen.tsx index a00b7b820..b3a660858 100644 --- a/web/src/screens/SchedulesScreen.tsx +++ b/web/src/screens/SchedulesScreen.tsx @@ -49,7 +49,10 @@ type BootState = async function loadPickerData(): Promise { const [rerun, playlistGroups, watermarks, graphicsElements, languages, pre, mid, post, tail, fallback] = await Promise.all([ - getRerunCollections({ pageNum: 1, pageSize: 1000 }), + // pageNum is 0-BASED (api.paging-zero-based). This asked for page 1, which skipped the first + // page entirely: pageSize is clamped to 100 server-side, so the picker was served rows 101+ + // and showed nothing at all for the normal case of <=100 rerun collections (ersatztv#616). + getRerunCollections({ pageNum: 0, pageSize: 1000 }), getPlaylistGroups(), getWatermarks(), getGraphicsElements(), From 78ec997eae07d76db4db80b1f938142655a47f4f Mon Sep 17 00:00:00 2001 From: Timothy Date: Sat, 25 Jul 2026 23:51:14 +0200 Subject: [PATCH 4/4] docs(616): stop the record title and mcp.md from contradicting their own bodies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two LOWs from the re-review of af39123e. Both are the same defect I have already corrected twice on this branch: a universal claim that the corrected body right below it disproves. These read as normative, so a reader who stops at the title gets the false version. - The record was titled "...and every wrapper says so" while its own body admits the 12 OpenAPI pageNum parameters carry no description at all (tracked as #633). Retitled to state the target and name the exception. - docs/mcp.md led with "Rows that reference another entity carry that entity's id" — but PlayoutListItemResponseModel.ScheduleName ships with no schedule id. Restated as a rule about ACTIONABLE references, matching the wording the record already uses, with the known exception named. The two remaining LOWs are accepted deferrals, not fixed here: the >100 rerun collection truncation needs paging-to-completeness plus a multi-page fixture (#634), and the catalog test's forward-looking gap (it compares tool names, not PathTemplate against the paged-endpoint set) would need the MCP test project to reference the controllers to close properly. Docs only; catalog regenerated. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) --- docs/decisions/records/api/paging-zero-based.md | 2 +- docs/mcp.md | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/docs/decisions/records/api/paging-zero-based.md b/docs/decisions/records/api/paging-zero-based.md index 6cd85d6bd..4b5d8f930 100644 --- a/docs/decisions/records/api/paging-zero-based.md +++ b/docs/decisions/records/api/paging-zero-based.md @@ -1,6 +1,6 @@ --- key: api.paging-zero-based -title: 2026-07-25 — Paging is 0-based everywhere, and every wrapper says so (#616) +title: 2026-07-25 — Paging is 0-based everywhere; every wrapper must say so (OpenAPI still doesn't) (#616) status: active since: '2026-07-25' supersedes: none diff --git a/docs/mcp.md b/docs/mcp.md index f34a6f3c7..251570a7f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -104,9 +104,12 @@ that way until #616. `ToolCatalogTests` pins the expected set.) value on `search_all_items` is under its cap and is honored (page 1 = items 501–1000). Don't assume your requested size held — page to completeness against `totalCount`. -Rows that reference another entity carry that entity's id, not just its display fields — take the -channel id for `ersatztv_reset_channel_playout` from a playout row's `channelId`, never from its `id` -(that is the *playout* id, and the two id spaces overlap numerically). +Where a row is meant to be acted on, it carries the *id* of the entity you act on and not only its +display name — take the channel id for `ersatztv_reset_channel_playout` from a playout row's +`channelId`, never from its `id` (that is the *playout* id, and the two id spaces overlap +numerically, so the wrong one silently succeeds against a different channel). This is a rule for +actionable references, not a claim that every row is fully id-annotated: `scheduleName` on a playout +row still ships without a schedule id, because nothing asks you to address a schedule from there. ### Optimistic concurrency (`api-conventions.md` §7a)