diff --git a/admin-ui/__specs__/gardener-page.spec.tsx b/admin-ui/__specs__/gardener-page.spec.tsx index 66c32a1e..dac30467 100644 --- a/admin-ui/__specs__/gardener-page.spec.tsx +++ b/admin-ui/__specs__/gardener-page.spec.tsx @@ -1,32 +1,54 @@ // @jest-environment jsdom // STORY-374 / STORY-376 / STORY-379 — The Gardener page: findings by kind, Keep this one, and the // file-action dry-run (SPEC F153.9-F153.10, F153.5, F154.5 · PLAN T378, T381) +// STORY-381/382/383 rider (SPEC F153.10 rider 2026-08-31 · PLAN T387 · gh-#654/#655/#657) — +// RECONCILED: the Gardener page is now server-rendered (`gardener/page.tsx`), one tab's own kind at +// a time. `GardenerView` (client LoadState/fetch-on-mount, the gh-#654 defect) and its own findings/ +// status fetch mocks retire with it. `GardenerSection` is now the page's own "use client" boundary +// (mirrors `catalog/CatalogTable.tsx`) — every row verb still re-fetches on success, but via +// `router.refresh()` rather than a client-held re-fetch closure, so this file mocks `next/navigation` +// (the `persona-catalog-page.spec.tsx`/`catalog-selection-toolbar.spec.tsx` convention) instead of +// mocking `GET /api/gardener/findings`/`GET /api/status` — `GardenerSection` no longer fetches +// either itself, it only receives them as props. The five-sections-in-fixed-order and "Showing +// first N of M" scenarios retire outright (the tab strip owns kind order now — see +// gardener-tabs.spec.tsx — and the flat-paging caveat is gone per the rider); the dead-file Purge +// trigger's cross-tab visibility likewise moves to gardener-tabs.spec.tsx's own "purge lives on the +// dead-files tab" scenario, which is the page-level surface that claim is actually about. What +// stays here: verb wiring (eligibility/never-play/re-enrich/dismiss/Keep this one), the gh-#655 +// purge label, the per-kind empty state, duplicate-group rendering, and T381's own file-action +// dry-run/confirm scenarios — all still true, just exercised through `GardenerSection` directly. // -// PLAN T378 (this file's own top section) BUILDS the Gardener page itself — -// app/(authed)/gardener/GardenerView.tsx + page.tsx (+ GardenerSection/GardenerRow/ -// DuplicateGroupCard), plus the shared `_components/PurgeUnavailableAction` and the Gardener nav -// entry (gardener-nav.spec.tsx) — replacing this file's own former T378 `it.todo` placeholders with -// real specs below. Runner: Jest (jsdom) + @testing-library/react. GardenerView fetches its own -// data client-side once on mount (no SSR props, no polling) — these specs mock `global.fetch` -// dispatched by URL+method (the personas-page.spec.tsx/catalog-purge-unavailable.spec.tsx -// convention) and render `` directly, wrapped in ConfirmDialogProvider + Toaster -// (Keep this one and Dismiss both need the confirm dialog; every verb toasts on failure). -// -// T378 review MED-5 — one assertion per `it`: the five-sections-in-order, Keep-this-one, and nav -// scenarios are each split across several `it`s rather than packed into one with several `expect`s. -// -// T381's own file-action dry-run/confirm scenarios (SPEC F154.5, STORY-379) stay PENDING at the -// bottom of this file — that surface does not exist yet (F154, a later task per the spec's own -// "the page mints no new mutation beyond dismiss (and F154, a later task)" line). +// Runner: Jest (jsdom) + @testing-library/react. + +jest.mock("next/navigation", () => ({ + ...jest.requireActual("next/navigation"), + useRouter: jest.fn(), +})); -import { describe, it, expect, beforeEach, afterEach, jest } from "@jest/globals"; +import { describe, it, expect, beforeAll, beforeEach, afterEach, jest } from "@jest/globals"; import { render, screen, fireEvent, act, waitFor, within } from "@testing-library/react"; import "@testing-library/jest-dom/jest-globals"; +import type { useRouter } from "next/navigation"; import { ConfirmDialogProvider } from "@/components/ui/confirm-dialog"; import { Toaster } from "@/components/ui/toast"; -import { GardenerView } from "../app/(authed)/gardener/GardenerView"; -import type { GardenerFindingDto, GardenerFindingsResponse } from "@/lib/gardener-api"; -import type { StatusResponse } from "@/lib/broadcast-api"; +import type { GardenerDuplicateGroupDto, GardenerFindingDto, GardenerGroupDto, GardenerKind } from "@/lib/gardener-api"; +import type { GardenerSection as GardenerSectionComponent } from "../app/(authed)/gardener/GardenerSection"; + +const mockedUseRouter = jest + .requireMock<{ useRouter: typeof useRouter }>("next/navigation") + .useRouter as jest.MockedFunction; +const mockedRefresh = jest.fn<() => void>(); + +// `GardenerSection` calls `useRouter()` unconditionally, so this module must be `import()`ed AFTER +// `jest.mock("next/navigation", ...)` has registered — a static top-level `import` here would bind +// the REAL `next/navigation` export before the mock factory above ever runs (this project's +// SWC-based jest transform does not hoist `jest.mock` past a static import), the same reason +// `persona-catalog-page.spec.tsx`'s own harness does this too. +let GardenerSection: typeof GardenerSectionComponent; + +beforeAll(async () => { + ({ GardenerSection } = await import("../app/(authed)/gardener/GardenerSection")); +}); // --------------------------------------------------------------------------- // Fixtures @@ -101,51 +123,18 @@ const STALE_FINDING: GardenerFindingDto = { media: media({ path: "/media/stale.flac", title: null, artist: null, rating: null }), }; -const SHELF_DUST_FINDING: GardenerFindingDto = { - id: 401, - mediaId: 4001, - state: "open", - evidence: { discovered_at: "2026-01-01T00:00:00Z", days_on_shelf: 240 }, - openedAt: "2026-01-01T00:00:00Z", - resolvedAt: null, - dismissedAt: null, - media: media({ path: "/media/shelf.flac", title: "Shelf Track", artist: "Artist S", plays: 0, rating: null }), -}; - -/** Groups deliberately NOT in `GARDENER_KIND_ORDER` order — proves the page renders its own fixed - * section order rather than whatever order the response happened to list groups in. `unreachable` - * carries no findings at all (the empty-kind Scenario). Only ONE `stale_metadata` row ships even - * though `status.gardener.open.staleMetadata` below says 5 (the "Showing first N of M" Scenario). */ -const FINDINGS_FIXTURE: GardenerFindingsResponse = { - groups: [ - { kind: "shelf_dust", findings: [SHELF_DUST_FINDING], duplicateGroups: [] }, - { - kind: "near_duplicate", - findings: [DUP_A, DUP_B, DUP_C], - duplicateGroups: [{ groupKey: "grp-1", members: [DUP_A, DUP_B, DUP_C] }], - }, - { kind: "unreachable", findings: [], duplicateGroups: [] }, - { kind: "dead_file", findings: [DEAD_FINDING], duplicateGroups: [] }, - { kind: "stale_metadata", findings: [STALE_FINDING], duplicateGroups: [] }, - ], -}; - -function statusFixture(): StatusResponse { - return { - startedAt: "2026-01-01T08:00:00.000Z", - catalog: { ready: 10, enriching: 0, failed: 0, unavailable: 0 }, - safeScope: { libraryIds: [1], playable: 5 }, - llm: { enabled: false, model: null, activePersona: null, lastOutcome: null, lastAttemptAt: null }, - voice: { engine: "kokoro", degraded: false, reason: null, checkedAt: null }, - gardener: { - open: { deadFile: 1, nearDuplicate: 3, staleMetadata: 5, unreachable: 0, shelfDust: 1 }, - total: 10, - }, - }; +function groupOf( + kind: GardenerKind, + findings: GardenerFindingDto[], + duplicateGroups: GardenerDuplicateGroupDto[] = [] +): GardenerGroupDto { + return { kind, findings, duplicateGroups }; } // --------------------------------------------------------------------------- -// Fetch mock +// Fetch mock — only the WRITE endpoints. GardenerSection is fully props-driven now (page.tsx does +// every GET server-side), so unlike the old GardenerView-era mock, there is no findings/status GET +// to intercept — an unexpected GET here would mean a verb regressed back to client-side fetching. // --------------------------------------------------------------------------- interface RouteResponseSpec { @@ -167,12 +156,6 @@ function makeFetchMock(): jest.MockedFunction { const method = init?.method ?? "GET"; const url = String(input); - if (method === "GET" && url.includes("/api/gardener/findings")) { - return toResponse({ status: 200, body: FINDINGS_FIXTURE }); - } - if (method === "GET" && url.includes("/api/status")) { - return toResponse({ status: 200, body: statusFixture() }); - } if (method === "POST" && url === "/api/media/eligibility") { return toResponse({ status: 200, body: { affected: 2 } }); } @@ -206,110 +189,53 @@ function findCallIndex( ); } -function renderPage(): ReturnType { +function renderSection( + kind: GardenerKind, + group: GardenerGroupDto, + options: { openCount?: number | null; total?: number } = {} +): ReturnType { return render( - + ); } -function sectionHeadings(): string[] { - return screen.getAllByRole("heading", { level: 2 }).map((heading) => heading.textContent ?? ""); -} - // --------------------------------------------------------------------------- -// Feature: the Gardener page +// Feature: the Gardener page's own section (SPEC F153.10) // --------------------------------------------------------------------------- -describe("Feature: the Gardener page (SPEC F153.10)", () => { - let originalFetch: typeof fetch; - +describe("Feature: the Gardener section (SPEC F153.10 rider 2026-08-31)", () => { beforeEach(() => { - originalFetch = global.fetch; + mockedRefresh.mockClear(); + mockedUseRouter.mockReturnValue({ refresh: mockedRefresh } as unknown as ReturnType); }); afterEach(() => { - global.fetch = originalFetch; jest.clearAllMocks(); }); - describe("Scenario: five sections in the fixed order (dead_file, near_duplicate, stale_metadata, unreachable, shelf_dust)", () => { - it("renders exactly five sections", async () => { - makeFetchMock(); - renderPage(); - - await waitFor(() => expect(sectionHeadings().length).toBe(5)); - }); - - it("renders Dead files first", async () => { - makeFetchMock(); - renderPage(); - - await waitFor(() => expect(sectionHeadings().length).toBe(5)); - expect(sectionHeadings()[0]).toContain("Dead files"); - }); - - it("renders Near duplicates second", async () => { - makeFetchMock(); - renderPage(); - - await waitFor(() => expect(sectionHeadings().length).toBe(5)); - expect(sectionHeadings()[1]).toContain("Near duplicates"); - }); - - it("renders Stale metadata third", async () => { - makeFetchMock(); - renderPage(); - - await waitFor(() => expect(sectionHeadings().length).toBe(5)); - expect(sectionHeadings()[2]).toContain("Stale metadata"); - }); - - it("renders Unreachable fourth", async () => { - makeFetchMock(); - renderPage(); - - await waitFor(() => expect(sectionHeadings().length).toBe(5)); - expect(sectionHeadings()[3]).toContain("Unreachable"); - }); - - it("renders Shelf dust fifth", async () => { - makeFetchMock(); - renderPage(); - - await waitFor(() => expect(sectionHeadings().length).toBe(5)); - expect(sectionHeadings()[4]).toContain("Shelf dust"); - }); - }); - describe("Scenario: an empty kind", () => { - it("shows a one-line, kind-named empty state for the kind with no findings", async () => { - makeFetchMock(); - renderPage(); + it("shows a one-line, kind-named empty state for the kind with no findings", () => { + renderSection("unreachable", groupOf("unreachable", [])); - const section = await screen.findByRole("region", { name: "Unreachable" }); + const section = screen.getByRole("region", { name: "Unreachable" }); expect(within(section).getByText("Nothing unreachable.")).toBeInTheDocument(); }); }); - describe("Scenario: the flat-paging caveat", () => { - it('shows "Showing first N of M" when the page has fewer rows for a kind than the status total', async () => { - makeFetchMock(); - renderPage(); - - const section = await screen.findByRole("region", { name: "Stale metadata" }); - expect(within(section).getByText("Showing first 1 of 5")).toBeInTheDocument(); - }); - }); - describe("Scenario: the eligibility control (T378 review BLOCK-2/BLOCK-1)", () => { it("posts eligible:false with only this row's own id when toggled off", async () => { const mockFetch = makeFetchMock(); - renderPage(); + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); - const section = await screen.findByRole("region", { name: "Dead files" }); + const section = screen.getByRole("region", { name: "Dead files" }); const checkbox = within(section).getByRole("checkbox", { name: "Eligible" }); await act(async () => { @@ -323,14 +249,29 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { const callIndex = findCallIndex(mockFetch, "POST", (url) => url === "/api/media/eligibility"); expect(requestBody(mockFetch, callIndex)).toEqual({ eligible: false, filter: { mediaIds: [1001] } }); }); + + it("calls router.refresh after a successful toggle", async () => { + makeFetchMock(); + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); + + const section = screen.getByRole("region", { name: "Dead files" }); + const checkbox = within(section).getByRole("checkbox", { name: "Eligible" }); + + await act(async () => { + fireEvent.click(checkbox); + await Promise.resolve(); + }); + + await waitFor(() => expect(mockedRefresh).toHaveBeenCalled()); + }); }); describe("Scenario: the never-play control (reused from catalog/NeverPlayControl.tsx)", () => { it("PUTs /api/media/{id}/never-play with neverPlay:true", async () => { const mockFetch = makeFetchMock(); - renderPage(); + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); - const section = await screen.findByRole("region", { name: "Dead files" }); + const section = screen.getByRole("region", { name: "Dead files" }); await act(async () => { fireEvent.click(within(section).getByRole("button", { name: "Never play" })); @@ -348,9 +289,9 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { describe("Scenario: re-enrich", () => { it("posts to /api/media/{id}/reenrich", async () => { const mockFetch = makeFetchMock(); - renderPage(); + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); - const section = await screen.findByRole("region", { name: "Dead files" }); + const section = screen.getByRole("region", { name: "Dead files" }); await act(async () => { fireEvent.click(within(section).getByRole("button", { name: "Re-enrich" })); @@ -363,30 +304,36 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { }); }); - describe("Scenario: the dead-file section's Purge unavailable trigger", () => { - it("renders the purge trigger in Dead files and no other section", async () => { - makeFetchMock(); - renderPage(); - await waitFor(() => expect(sectionHeadings().length).toBe(5)); - - const sectionsWithPurgeTrigger = [ - "Dead files", - "Near duplicates", - "Stale metadata", - "Unreachable", - "Shelf dust", - ].filter((label) => { - const section = screen.getByRole("region", { name: label }); - return within(section).queryByRole("button", { name: "Purge unavailable…" }) !== null; - }); + describe("Scenario: the dead-file Purge trigger (gh-#655 verb-object label)", () => { + it('reads "Purge dead tracks…", never a status reading', () => { + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); + + const section = screen.getByRole("region", { name: "Dead files" }); + expect(within(section).getByRole("button", { name: "Purge dead tracks…" })).toBeInTheDocument(); + }); + + it("renders no purge trigger once the kind's own total reaches zero", () => { + renderSection("dead_file", groupOf("dead_file", []), { total: 0 }); - expect(sectionsWithPurgeTrigger).toEqual(["Dead files"]); + const section = screen.getByRole("region", { name: "Dead files" }); + expect(within(section).queryByRole("button", { name: /purge/i })).not.toBeInTheDocument(); }); }); - describe("Scenario: Keep this one on a duplicate group of three (STORY-376 AC6)", () => { + describe("Scenario: Keep this one on a duplicate group of three (STORY-376 AC6, STORY-383 AC4)", () => { + // near_duplicate's own total is GROUP-scoped (STORY-382 AC6/AC8: 1 group here, even though + // the group holds 3 rows) — the header's ROW-scoped fallback openCount (3, from status) is + // supplied too, deliberately different, per this task's own "both true at once" rider. + function renderDuplicateGroup(): ReturnType { + return renderSection( + "near_duplicate", + groupOf("near_duplicate", [DUP_A, DUP_B, DUP_C], [{ groupKey: "grp-1", members: [DUP_A, DUP_B, DUP_C] }]), + { openCount: 3, total: 1 } + ); + } + async function openKeepThisOneDialog(): Promise { - const section = await screen.findByRole("region", { name: "Near duplicates" }); + const section = screen.getByRole("region", { name: "Near duplicates" }); const rowA = within(section).getByText("Song X").closest("div.py-3") as HTMLElement; await act(async () => { @@ -397,9 +344,16 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { return screen.findByRole("dialog"); } + it("renders all three members of the group in one card", () => { + renderDuplicateGroup(); + + const section = screen.getByRole("region", { name: "Near duplicates" }); + expect(within(section).getAllByText("Song X", { exact: false })).toHaveLength(3); + }); + it("shows a confirm dialog naming the sibling count", async () => { makeFetchMock(); - renderPage(); + renderDuplicateGroup(); const dialog = await openKeepThisOneDialog(); expect(within(dialog).getByText("Mark 2 siblings ineligible?")).toBeInTheDocument(); @@ -407,7 +361,7 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { it("does not post before the dialog is confirmed", async () => { const mockFetch = makeFetchMock(); - renderPage(); + renderDuplicateGroup(); await openKeepThisOneDialog(); expect(findCallIndex(mockFetch, "POST", (url) => url === "/api/media/eligibility")).toBe(-1); @@ -415,7 +369,7 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { it("posts eligible:false with the OTHER members' ids after confirming", async () => { const mockFetch = makeFetchMock(); - renderPage(); + renderDuplicateGroup(); const dialog = await openKeepThisOneDialog(); await act(async () => { @@ -430,9 +384,9 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { expect(requestBody(mockFetch, callIndex)).toEqual({ eligible: false, filter: { mediaIds: [2002, 2003] } }); }); - it("re-fetches the findings after success", async () => { - const mockFetch = makeFetchMock(); - renderPage(); + it("calls router.refresh after success", async () => { + makeFetchMock(); + renderDuplicateGroup(); const dialog = await openKeepThisOneDialog(); await act(async () => { @@ -440,16 +394,13 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { await Promise.resolve(); }); - await waitFor(() => { - const findingsCalls = mockFetch.mock.calls.filter(([url]) => String(url).includes("/api/gardener/findings")); - expect(findingsCalls.length).toBeGreaterThanOrEqual(2); - }); + await waitFor(() => expect(mockedRefresh).toHaveBeenCalled()); }); }); describe("Scenario: dismiss confirms first (SMOKE-1, SPEC F153.2 — dismissed is never reopened)", () => { async function openDismissDialog(): Promise { - const section = await screen.findByRole("region", { name: "Dead files" }); + const section = screen.getByRole("region", { name: "Dead files" }); await act(async () => { fireEvent.click(within(section).getByRole("button", { name: "Dismiss" })); @@ -461,7 +412,7 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { it("shows a confirm dialog explaining dismiss is forever", async () => { makeFetchMock(); - renderPage(); + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); const dialog = await openDismissDialog(); expect(within(dialog).getByText("The gardener will not raise this again for this track.")).toBeInTheDocument(); @@ -469,7 +420,7 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { it("posts to /api/gardener/findings/{id}/dismiss only after confirming", async () => { const mockFetch = makeFetchMock(); - renderPage(); + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); const dialog = await openDismissDialog(); await act(async () => { @@ -491,35 +442,25 @@ describe("Feature: the Gardener page (SPEC F153.10)", () => { // Feature: Gardener page — file actions (SPEC F154.1-F154.5, STORY-379, PLAN T381) — the "Fix…" // button (excluded for dead_file — see GardenerRow's own remarks) opens FileActionDialog, which // posts to the two endpoints below. Fixtures mirror the real GardenerFileActionsController wire -// shape (files/GenWave.Host/Api/GardenerFileActionsController.cs, PLAN T381). +// shape (src/GenWave.Host/Api/GardenerFileActionsController.cs, PLAN T381). Unaffected by T387's +// own rider — still exercised through GardenerSection directly. // --------------------------------------------------------------------------- describe("Feature: Gardener page file actions (SPEC F154, STORY-379, PLAN T381)", () => { - let originalFetch: typeof fetch; - beforeEach(() => { - originalFetch = global.fetch; + mockedRefresh.mockClear(); + mockedUseRouter.mockReturnValue({ refresh: mockedRefresh } as unknown as ReturnType); }); afterEach(() => { - global.fetch = originalFetch; jest.clearAllMocks(); }); - function makeFileActionFetchMock(routes: { - dryRun?: RouteResponseSpec; - confirm?: RouteResponseSpec; - }): jest.MockedFunction { + function makeFileActionFetchMock(routes: { dryRun?: RouteResponseSpec; confirm?: RouteResponseSpec }): jest.MockedFunction { const fn = jest.fn().mockImplementation(async (input, init) => { const method = init?.method ?? "GET"; const url = String(input); - if (method === "GET" && url.includes("/api/gardener/findings")) { - return toResponse({ status: 200, body: FINDINGS_FIXTURE }); - } - if (method === "GET" && url.includes("/api/status")) { - return toResponse({ status: 200, body: statusFixture() }); - } if (method === "POST" && url === "/api/gardener/file-actions/dry-run") { return toResponse(routes.dryRun ?? { status: 200, body: {} }); } @@ -535,17 +476,17 @@ describe("Feature: Gardener page file actions (SPEC F154, STORY-379, PLAN T381)" /** Opens the Fix dialog off the (single-row) Stale metadata section — never near_duplicate, whose * rows render through DuplicateGroupCard instead of straight into the section's own row list. */ async function openFixDialog(): Promise { - const section = await screen.findByRole("region", { name: "Stale metadata" }); + const section = screen.getByRole("region", { name: "Stale metadata" }); fireEvent.click(within(section).getByRole("button", { name: "Fix…" })); return screen.findByRole("dialog", { name: "Fix this file" }); } describe("Scenario: dead_file rows never offer Fix (T381 review N5 — the file is gone)", () => { - it("the Dead files section renders no Fix button", async () => { + it("the Dead files section renders no Fix button", () => { makeFileActionFetchMock({}); - renderPage(); + renderSection("dead_file", groupOf("dead_file", [DEAD_FINDING]), { total: 1 }); - const section = await screen.findByRole("region", { name: "Dead files" }); + const section = screen.getByRole("region", { name: "Dead files" }); expect(within(section).queryByRole("button", { name: "Fix…" })).not.toBeInTheDocument(); }); @@ -565,7 +506,7 @@ describe("Feature: Gardener page file actions (SPEC F154, STORY-379, PLAN T381)" }, }, }); - renderPage(); + renderSection("stale_metadata", groupOf("stale_metadata", [STALE_FINDING]), { total: 1 }); const dialog = await openFixDialog(); fireEvent.click(within(dialog).getByRole("button", { name: "Dry run" })); @@ -588,7 +529,7 @@ describe("Feature: Gardener page file actions (SPEC F154, STORY-379, PLAN T381)" }, confirm: { status: 200, body: { outcome: "done", to: "/media/stale.flac" } }, }); - renderPage(); + renderSection("stale_metadata", groupOf("stale_metadata", [STALE_FINDING]), { total: 1 }); const dialog = await openFixDialog(); fireEvent.click(within(dialog).getByRole("button", { name: "Dry run" })); @@ -611,7 +552,7 @@ describe("Feature: Gardener page file actions (SPEC F154, STORY-379, PLAN T381)" body: { detail: "Gardener:FileActions:Enabled is false — set it to true to use this endpoint." }, }, }); - renderPage(); + renderSection("stale_metadata", groupOf("stale_metadata", [STALE_FINDING]), { total: 1 }); const dialog = await openFixDialog(); fireEvent.click(within(dialog).getByRole("button", { name: "Dry run" })); @@ -635,15 +576,13 @@ describe("Feature: Gardener page file actions (SPEC F154, STORY-379, PLAN T381)" }, }, }); - renderPage(); + renderSection("stale_metadata", groupOf("stale_metadata", [STALE_FINDING]), { total: 1 }); const dialog = await openFixDialog(); fireEvent.click(within(dialog).getByRole("button", { name: "Dry run" })); expect( - await within(dialog).findByText( - "The catalog and the file's own tags already agree — there is nothing to retag." - ) + await within(dialog).findByText("The catalog and the file's own tags already agree — there is nothing to retag.") ).toBeInTheDocument(); }); @@ -657,13 +596,11 @@ describe("Feature: Gardener page file actions (SPEC F154, STORY-379, PLAN T381)" }, }, }); - renderPage(); + renderSection("stale_metadata", groupOf("stale_metadata", [STALE_FINDING]), { total: 1 }); const dialog = await openFixDialog(); fireEvent.click(within(dialog).getByRole("button", { name: "Dry run" })); - await within(dialog).findByText( - "The catalog and the file's own tags already agree — there is nothing to retag." - ); + await within(dialog).findByText("The catalog and the file's own tags already agree — there is nothing to retag."); expect(dialog.textContent ?? "").not.toContain("nothing_to_retag:"); }); diff --git a/admin-ui/__specs__/gardener-pagination.spec.tsx b/admin-ui/__specs__/gardener-pagination.spec.tsx index d3e43a5d..bea3a117 100644 --- a/admin-ui/__specs__/gardener-pagination.spec.tsx +++ b/admin-ui/__specs__/gardener-pagination.spec.tsx @@ -1,53 +1,390 @@ +// @jest-environment jsdom // STORY-382 — I page through a big kind at my own pace · STORY-383 AC4 — a whole cluster renders // together (SPEC F153.9/F153.10 riders 2026-08-31 · PLAN T387 · gh-#657) // -// BDD specification — Jest, pending (it.todo) until T387. The pager is the catalog page's own -// idiom: "page N of M" from the kind-scoped response's `total`, Previous/Next plain anchors, -// size picker 25/50/100/250 living in ?limit= only. /build-loop turns each todo into a real -// spec with one assertion; the production arc is T387's browser smoke. +// BDD specification — Jest (jsdom) + @testing-library/react. `resolveGardenerPageSize` (the pure +// resolver) is spec'd directly. `GardenerPageSizePicker`/`GardenerTabs` (no hooks, no fetch) are +// exercised directly with RTL. The pager math and beyond-end/whole-cluster scenarios drive the real +// server page (`gardener/page.tsx`) end to end, mirroring gardener-tabs.spec.tsx's own harness — +// `next/headers.cookies()` and `next/navigation`'s `useRouter` are mocked, `global.fetch` is mocked +// dispatched by URL+method, and the page is `await import()`ed fresh per test. -import { describe, it } from "@jest/globals"; +jest.mock("next/headers", () => ({ + cookies: jest.fn<() => Promise<{ toString: () => string }>>().mockResolvedValue({ toString: () => "session=test-cookie" }), +})); -describe("Feature: Gardener pagination", () => { - describe("Scenario: the default page", () => { - it.todo("renders 25 rows for a 60-row kind with no paging params"); - it.todo('reads "page 1 of 3"'); - it.todo("renders a Next anchor to page 2"); +jest.mock("next/navigation", () => ({ + ...jest.requireActual("next/navigation"), + useRouter: jest.fn(), +})); + +import { describe, it, expect, jest, beforeEach, afterEach } from "@jest/globals"; +import { render, screen, within } from "@testing-library/react"; +import "@testing-library/jest-dom/jest-globals"; +import type { useRouter } from "next/navigation"; +import { ConfirmDialogProvider } from "@/components/ui/confirm-dialog"; +import { Toaster } from "@/components/ui/toast"; +import { GardenerPageSizePicker } from "../app/(authed)/gardener/GardenerPageSizePicker"; +import { GardenerTabs } from "../app/(authed)/gardener/GardenerTabs"; +import { resolveGardenerPageSize } from "../app/(authed)/gardener/gardener-paging"; +import type { GardenerDuplicateGroupDto, GardenerFindingDto, GardenerKind, GardenerOpenCounts } from "@/lib/gardener-api"; + +const mockedUseRouter = jest + .requireMock<{ useRouter: typeof useRouter }>("next/navigation") + .useRouter as jest.MockedFunction; +const mockedRefresh = jest.fn<() => void>(); + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +function openCounts(overrides: Partial = {}): GardenerOpenCounts { + return { deadFile: 0, nearDuplicate: 0, staleMetadata: 0, unreachable: 0, shelfDust: 0, ...overrides }; +} + +function finding(n: number, overrides: Partial = {}): GardenerFindingDto { + return { + id: n, + mediaId: 1000 + n, + state: "open", + evidence: {}, + openedAt: "2026-08-01T00:00:00Z", + resolvedAt: null, + dismissedAt: null, + media: { + path: `/media/${n}.flac`, + title: `Track ${n}`, + artist: "Artist", + durationMs: 200000, + plays: 1, + rating: null, + neverPlay: false, + eligible: true, + ...overrides, + }, + }; +} + +/** A row-paged kind fixture (every kind but near_duplicate, STORY-382 AC6): the mock slices `all` + * by the request's own limit/offset, `total` is always `all.length`. */ +interface RowKindFixture { + paging: "rows"; + all: GardenerFindingDto[]; +} + +/** A group-paged kind fixture (near_duplicate only, STORY-383 AC4): already the one page's worth of + * rows/groups — `total` is the GROUPS count, not `findings.length` (STORY-382 AC6/AC8). */ +interface GroupKindFixture { + paging: "groups"; + findings: GardenerFindingDto[]; + duplicateGroups: GardenerDuplicateGroupDto[]; + total: number; +} + +type KindFixture = RowKindFixture | GroupKindFixture; + +interface FetchMockOptions { + findings: Partial>; + open: GardenerOpenCounts; +} + +function jsonResponse(status: number, body: unknown): Response { + return { + ok: status >= 200 && status < 300, + status, + json: jest.fn<() => Promise>().mockResolvedValue(body), + } as unknown as Response; +} + +function makeFetchMock(options: FetchMockOptions): jest.MockedFunction { + const fn = jest.fn().mockImplementation(async (input) => { + const url = new URL(String(input), "http://localhost"); + + if (url.pathname === "/api/status") { + return jsonResponse(200, { gardener: { open: options.open, total: 0 } }); + } + + if (url.pathname === "/api/gardener/findings") { + const kind = url.searchParams.get("kind") as GardenerKind | null; + const fixture = kind !== null ? options.findings[kind] : undefined; + if (fixture === undefined) { + throw new Error(`unexpected findings fetch for kind=${String(kind)}`); + } + + if (fixture.paging === "groups") { + return jsonResponse(200, { + groups: + fixture.findings.length > 0 + ? [{ kind, findings: fixture.findings, duplicateGroups: fixture.duplicateGroups }] + : [], + total: fixture.total, + }); + } + + const limit = Number(url.searchParams.get("limit")); + const offset = Number(url.searchParams.get("offset")); + const page = fixture.all.slice(offset, offset + limit); + return jsonResponse(200, { + groups: page.length > 0 ? [{ kind, findings: page, duplicateGroups: [] }] : [], + total: fixture.all.length, + }); + } + + throw new Error(`unexpected fetch call: ${String(input)}`); }); + global.fetch = fn as unknown as typeof fetch; + return fn; +} - describe("Scenario: a deep page from the URL", () => { - it.todo("renders rows 51-60 at ?page=3 of a 60-row kind"); - it.todo("renders a Previous anchor to page 2"); - it.todo("renders no live Next anchor on the last page"); +async function renderGardenerPage(sp: Record): Promise> { + const { default: GardenerPage } = await import("../app/(authed)/gardener/page"); + const node = await GardenerPage({ searchParams: Promise.resolve(sp) }); + return render( + + {node} + + + ); +} + +const SIXTY_DEAD_FILES: GardenerFindingDto[] = Array.from({ length: 60 }, (_, i) => finding(i + 1)); +const THIRTY_DEAD_FILES: GardenerFindingDto[] = Array.from({ length: 30 }, (_, i) => finding(i + 1)); + +// --------------------------------------------------------------------------- +// Feature: page-size resolution (pure) +// --------------------------------------------------------------------------- + +describe("Feature: Gardener page size resolution", () => { + describe("Scenario: out-of-set sizes read as 25", () => { + it("treats ?limit=999 as 25", () => { + expect(resolveGardenerPageSize("999")).toBe(25); + }); }); +}); + +// --------------------------------------------------------------------------- +// Feature: the size picker (component) +// --------------------------------------------------------------------------- +describe("Feature: the Gardener rows-per-page picker", () => { describe("Scenario: the size picker", () => { - it.todo("offers exactly 25, 50, 100, and 250"); - it.todo("writes limit=100 to the URL when 100 is picked"); - it.todo("resets to page 1 when the size changes"); + it("offers exactly 25, 50, 100, and 250", () => { + render(); + + const group = screen.getByRole("group", { name: "Rows per page" }); + const links = within(group).getAllByRole("link"); + expect(links.map((link) => link.textContent)).toEqual(["25", "50", "100", "250"]); + }); + + it("writes limit=100 to the URL when 100 is picked", () => { + render(); + + const group = screen.getByRole("group", { name: "Rows per page" }); + expect(within(group).getByRole("link", { name: "100" })).toHaveAttribute("href", "/gardener?limit=100"); + }); }); +}); + +// --------------------------------------------------------------------------- +// Feature: tab switch keeps size, resets page (component) +// --------------------------------------------------------------------------- +describe("Feature: tab switch keeps size, resets page", () => { describe("Scenario: tab switch keeps size, resets page", () => { - it.todo("keeps limit=100 in the target tab's URL"); - it.todo("resets the target tab to page 1"); + it("keeps limit=100 in the target tab's URL", () => { + render(); + + const nav = screen.getByRole("navigation", { name: "Gardener kinds" }); + expect(within(nav).getByRole("link", { name: "Near duplicates (0)" })).toHaveAttribute( + "href", + "/gardener?tab=near_duplicate&limit=100" + ); + }); + }); +}); + +// --------------------------------------------------------------------------- +// Feature: Gardener pagination — driven through the real server page +// --------------------------------------------------------------------------- + +describe("Feature: Gardener pagination — server page wiring", () => { + let originalFetch: typeof fetch; + + beforeEach(() => { + originalFetch = global.fetch; + mockedRefresh.mockClear(); + mockedUseRouter.mockReturnValue({ refresh: mockedRefresh } as unknown as ReturnType); + }); + + afterEach(() => { + global.fetch = originalFetch; + jest.clearAllMocks(); + }); + + describe("Scenario: the default page", () => { + it("renders 25 rows for a 60-row kind with no paging params", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({}); + + expect(screen.getAllByText(/^Track \d+$/)).toHaveLength(25); + }); + + it('reads "page 1 of 3"', async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({}); + + expect(screen.getByText("Page 1 of 3")).toBeInTheDocument(); + }); + + it("renders a Next anchor to page 2", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({}); + + expect(screen.getByRole("link", { name: "Next" })).toHaveAttribute("href", "/gardener?page=2"); + }); + }); + + describe("Scenario: a deep page from the URL", () => { + it("renders rows 51-60 at ?page=3 of a 60-row kind", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({ page: "3" }); + + expect(screen.getByText("Track 51")).toBeInTheDocument(); + expect(screen.getByText("Track 60")).toBeInTheDocument(); + expect(screen.queryByText("Track 50")).not.toBeInTheDocument(); + }); + + it("renders a Previous anchor to page 2", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({ page: "3" }); + + expect(screen.getByRole("link", { name: "Previous" })).toHaveAttribute("href", "/gardener?page=2"); + }); + + it("renders no live Next anchor on the last page", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({ page: "3" }); + + expect(screen.queryByRole("link", { name: "Next" })).not.toBeInTheDocument(); + }); }); describe("Scenario: the total comes from the response", () => { - it.todo('derives "page N of M" from the kind-scoped response total, not /api/status'); + it('derives "page N of M" from the kind-scoped response total, not /api/status', async () => { + // status says 5 open dead files; the findings response's own total says 60 — the pager must + // read the 60 (STORY-382 AC6/AC8), never the status figure. + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 5 }) }); + + await renderGardenerPage({}); + + expect(screen.getByText("Page 1 of 3")).toBeInTheDocument(); + }); }); describe("Scenario: a whole cluster renders together", () => { - it.todo("renders all members of a 4-member duplicate group in one card on one page"); + it("renders all members of a 4-member duplicate group in one card on one page", async () => { + const members = [finding(1, { title: "Song X" }), finding(2, { title: "Song X (Live)" }), finding(3, { title: "Song X (Demo)" }), finding(4, { title: "Song X (Remix)" })]; + makeFetchMock({ + findings: { + near_duplicate: { + paging: "groups", + findings: members, + duplicateGroups: [{ groupKey: "grp-1", members }], + total: 1, + }, + }, + open: openCounts({ nearDuplicate: 4 }), + }); + + await renderGardenerPage({ tab: "near_duplicate" }); + + expect(screen.getAllByText("Group grp-1")).toHaveLength(1); + expect(screen.getByText("Song X")).toBeInTheDocument(); + expect(screen.getByText("Song X (Live)")).toBeInTheDocument(); + expect(screen.getByText("Song X (Demo)")).toBeInTheDocument(); + expect(screen.getByText("Song X (Remix)")).toBeInTheDocument(); + }); }); - // ── Sad path ────────────────────────────────────────────────────────── describe("Scenario: out-of-set sizes read as 25", () => { - it.todo("treats ?limit=999 as 25"); - it.todo("shows 25 in the picker for an out-of-set ?limit="); + it("shows 25 in the picker for an out-of-set ?limit=", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({ limit: "999" }); + + const group = screen.getByRole("group", { name: "Rows per page" }); + expect(within(group).getByRole("link", { name: "25" })).toHaveAttribute("aria-current", "page"); + }); }); describe("Scenario: a page beyond the end recovers", () => { - it.todo("renders the empty state at ?page=3 of a 2-page kind"); - it.todo("keeps the pager live so Previous reaches page 2"); + it("renders the empty state at ?page=3 of a 2-page kind", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: THIRTY_DEAD_FILES } }, open: openCounts({ deadFile: 30 }) }); + + await renderGardenerPage({ page: "3" }); + + expect(screen.getByText("No dead files.")).toBeInTheDocument(); + }); + + it("keeps the pager live so Previous reaches page 2", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: THIRTY_DEAD_FILES } }, open: openCounts({ deadFile: 30 }) }); + + await renderGardenerPage({ page: "3" }); + + expect(screen.getByRole("link", { name: "Previous" })).toHaveAttribute("href", "/gardener?page=2"); + }); + }); + + // T387 review MED-1: `GardenerController`'s own `offset` query parameter is a C# `int?` — a + // derived offset beyond Int32.MaxValue fails ASP.NET model binding into a 400, which SPEC + // F153.10 rider's "never a 400" promise forbids. `resolveGardenerPaging` clamps `page` so the + // derived offset can never reach that ceiling. + describe("Scenario: an absurdly large ?page= never overflows the derived offset", () => { + it("clamps the page so a huge ?page= renders a live pager instead of the error branch", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({ page: "999999999999" }); + + // 2147483647 (Int32.MaxValue) / 25 (the default limit), floored, + 1 — the largest page whose + // `(page - 1) * limit` still fits — clamped down from the requested value, one page lower. + expect(screen.getByRole("link", { name: "Previous" })).toHaveAttribute("href", "/gardener?page=85899345"); + }); + }); + + // T387 review LOW-5: the isolated-component versions of these two specs (``/ + // `` rendered with no `page` prop at all) could never actually observe a + // stale page carrying forward — neither component accepts one, so the assertion was + // structurally guaranteed to pass even if the reset logic broke. These drive the real page with + // `?page=2` already on the URL, so a regression that threaded the stale page through would + // actually red them. + describe("Scenario: tab switch and size changes really do reset the page, not just structurally", () => { + it("keeps limit=100 and drops the current ?page= when switching tabs", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({ tab: "dead_file", page: "2", limit: "100" }); + + const nav = screen.getByRole("navigation", { name: "Gardener kinds" }); + expect(within(nav).getByRole("link", { name: "Near duplicates (0)" })).toHaveAttribute( + "href", + "/gardener?tab=near_duplicate&limit=100" + ); + }); + + it("drops the current ?page= when a new size is picked", async () => { + makeFetchMock({ findings: { dead_file: { paging: "rows", all: SIXTY_DEAD_FILES } }, open: openCounts({ deadFile: 60 }) }); + + await renderGardenerPage({ tab: "dead_file", page: "2", limit: "100" }); + + const group = screen.getByRole("group", { name: "Rows per page" }); + for (const link of within(group).getAllByRole("link")) { + expect(link.getAttribute("href")).not.toMatch(/page=/); + } + }); }); }); diff --git a/admin-ui/__specs__/gardener-tabs.spec.tsx b/admin-ui/__specs__/gardener-tabs.spec.tsx index f983343e..7e1ca551 100644 --- a/admin-ui/__specs__/gardener-tabs.spec.tsx +++ b/admin-ui/__specs__/gardener-tabs.spec.tsx @@ -1,50 +1,400 @@ +// @jest-environment jsdom // STORY-381 — I browse the queue one kind at a time (SPEC F153.10 rider 2026-08-31 · PLAN T387 · gh-#657) // -// BDD specification — Jest, pending (it.todo) until T387 rebuilds the Gardener page on the -// catalog idiom (server-rendered, URL-driven ?tab=&page=&limit=). /build-loop turns each todo -// into a real spec: render the page (or its extracted pure helpers/client children) with the -// scenario's searchParams and fetch fakes, one assertion per spec. The full production arc -// (real binary + browser) is T387's own acceptance smoke — these specs pin the component/helper -// behavior the way catalog-facet-pickers/gardener-page already do for their pages. +// BDD specification — Jest (jsdom) + @testing-library/react. `GardenerTabs` itself (no hooks, no +// fetch) is exercised directly with RTL, mirroring `catalog-kind-tabs.spec.tsx`'s own harness for +// `PersonaCatalogTabs`. The tab-scoped fetch/activation/purge/refresh scenarios drive the real +// server page (`gardener/page.tsx`) end to end — `next/headers.cookies()` and `next/navigation`'s +// `useRouter` are mocked (the `persona-catalog-page.spec.tsx` convention), `global.fetch` is mocked +// dispatched by URL+method, and the page is `await import()`ed fresh per test AFTER the mocks are +// registered (this project's SWC jest transform does not hoist `jest.mock` past a static import). // // gh-#655 rides this story (AC6): the purge trigger's verb-object label is pinned here. -import { describe, it } from "@jest/globals"; +jest.mock("next/headers", () => ({ + cookies: jest.fn<() => Promise<{ toString: () => string }>>().mockResolvedValue({ toString: () => "session=test-cookie" }), +})); + +jest.mock("next/navigation", () => ({ + ...jest.requireActual("next/navigation"), + useRouter: jest.fn(), +})); + +import { describe, it, expect, jest, beforeEach, afterEach } from "@jest/globals"; +import { render, screen, fireEvent, act, waitFor, within } from "@testing-library/react"; +import "@testing-library/jest-dom/jest-globals"; +import type { useRouter } from "next/navigation"; +import { ConfirmDialogProvider } from "@/components/ui/confirm-dialog"; +import { Toaster } from "@/components/ui/toast"; +import { GardenerTabs } from "../app/(authed)/gardener/GardenerTabs"; +import type { GardenerFindingDto, GardenerKind, GardenerOpenCounts } from "@/lib/gardener-api"; + +const mockedUseRouter = jest + .requireMock<{ useRouter: typeof useRouter }>("next/navigation") + .useRouter as jest.MockedFunction; +const mockedRefresh = jest.fn<() => void>(); + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +function openCounts(overrides: Partial = {}): GardenerOpenCounts { + return { deadFile: 0, nearDuplicate: 0, staleMetadata: 0, unreachable: 0, shelfDust: 0, ...overrides }; +} + +function finding(id: number, mediaId: number, title: string): GardenerFindingDto { + return { + id, + mediaId, + state: "open", + evidence: { reason: "failed" }, + openedAt: "2026-08-01T00:00:00Z", + resolvedAt: null, + dismissedAt: null, + media: { + path: `/media/${mediaId}.flac`, + title, + artist: "Artist", + durationMs: 200000, + plays: 1, + rating: null, + neverPlay: false, + eligible: true, + }, + }; +} + +interface KindFixture { + rows: GardenerFindingDto[]; + total: number; +} + +interface FetchMockOptions { + /** Per-kind findings — a kind absent here throws if requested (catches a stray fetch). */ + findings: Partial>; + /** `null` simulates a failed status fetch (degrades the tab badges, never the page itself). */ + open: GardenerOpenCounts | null; +} + +function jsonResponse(status: number, body: unknown): Response { + return { + ok: status >= 200 && status < 300, + status, + json: jest.fn<() => Promise>().mockResolvedValue(body), + } as unknown as Response; +} + +function makeFetchMock(options: FetchMockOptions): jest.MockedFunction { + const fn = jest.fn().mockImplementation(async (input) => { + // `apiGet` (page.tsx's own reads) always hands an absolute BACKEND_URL-prefixed request; the + // dismiss verb (GardenerRow, browser-side) fetches a bare relative path instead — a base origin + // lets URL() parse both the same way real `fetch()` resolution would. + const url = new URL(String(input), "http://localhost"); + + if (url.pathname === "/api/status") { + return options.open === null + ? jsonResponse(500, {}) + : jsonResponse(200, { gardener: { open: options.open, total: 0 } }); + } + + if (url.pathname === "/api/gardener/findings") { + const kind = url.searchParams.get("kind") as GardenerKind | null; + const fixture = kind !== null ? options.findings[kind] : undefined; + if (fixture === undefined) { + throw new Error(`unexpected findings fetch for kind=${String(kind)}`); + } + return jsonResponse(200, { + groups: fixture.rows.length > 0 ? [{ kind, findings: fixture.rows, duplicateGroups: [] }] : [], + total: fixture.total, + }); + } + + if (url.pathname.endsWith("/dismiss")) { + return jsonResponse(204, {}); + } + + throw new Error(`unexpected fetch call: ${String(input)}`); + }); + global.fetch = fn as unknown as typeof fetch; + return fn; +} + +async function renderGardenerPage(sp: Record): Promise> { + const { default: GardenerPage } = await import("../app/(authed)/gardener/page"); + const node = await GardenerPage({ searchParams: Promise.resolve(sp) }); + return render( + + {node} + + + ); +} + +// --------------------------------------------------------------------------- +// Feature: Gardener kind tabs — the strip itself +// --------------------------------------------------------------------------- describe("Feature: Gardener kind tabs", () => { describe("Scenario: five tabs, badged from status", () => { - it.todo("renders five tabs in the fixed kind order"); - it.todo("labels each tab with its kind's open count from /api/status"); + it("renders five tabs in the fixed kind order", () => { + render(); + + const nav = screen.getByRole("navigation", { name: "Gardener kinds" }); + const links = within(nav).getAllByRole("link"); + expect(links.map((link) => link.textContent)).toEqual([ + "Dead files (0)", + "Near duplicates (0)", + "Stale metadata (0)", + "Unreachable (0)", + "Shelf dust (0)", + ]); + }); + + it("labels each tab with its kind's open count from /api/status", () => { + render( + + ); + + const nav = screen.getByRole("navigation", { name: "Gardener kinds" }); + expect(within(nav).getByRole("link", { name: "Dead files (3)" })).toBeInTheDocument(); + expect(within(nav).getByRole("link", { name: "Near duplicates (7)" })).toBeInTheDocument(); + expect(within(nav).getByRole("link", { name: "Stale metadata (1)" })).toBeInTheDocument(); + expect(within(nav).getByRole("link", { name: "Unreachable (4)" })).toBeInTheDocument(); + expect(within(nav).getByRole("link", { name: "Shelf dust (12)" })).toBeInTheDocument(); + }); + }); + + describe("Scenario: an empty kind names itself", () => { + it("keeps the empty kind's badge at 0", () => { + render(); + + const nav = screen.getByRole("navigation", { name: "Gardener kinds" }); + expect(within(nav).getByRole("link", { name: "Unreachable (0)" })).toBeInTheDocument(); + }); + }); + + describe("Scenario: the URL owns the active tab", () => { + it("renders each tab as a link to its own ?tab= URL", () => { + render(); + + const nav = screen.getByRole("navigation", { name: "Gardener kinds" }); + expect(within(nav).getByRole("link", { name: "Dead files (0)" })).toHaveAttribute("href", "/gardener"); + expect(within(nav).getByRole("link", { name: "Near duplicates (0)" })).toHaveAttribute( + "href", + "/gardener?tab=near_duplicate" + ); + expect(within(nav).getByRole("link", { name: "Stale metadata (0)" })).toHaveAttribute( + "href", + "/gardener?tab=stale_metadata" + ); + expect(within(nav).getByRole("link", { name: "Unreachable (0)" })).toHaveAttribute("href", "/gardener?tab=unreachable"); + expect(within(nav).getByRole("link", { name: "Shelf dust (0)" })).toHaveAttribute("href", "/gardener?tab=shelf_dust"); + }); + }); +}); + +// --------------------------------------------------------------------------- +// Feature: Gardener kind tabs — driven through the real server page +// --------------------------------------------------------------------------- + +describe("Feature: Gardener kind tabs — server page wiring", () => { + let originalFetch: typeof fetch; + + beforeEach(() => { + originalFetch = global.fetch; + mockedRefresh.mockClear(); + mockedUseRouter.mockReturnValue({ refresh: mockedRefresh } as unknown as ReturnType); + }); + + afterEach(() => { + global.fetch = originalFetch; + jest.clearAllMocks(); }); describe("Scenario: a tab shows only its own kind", () => { - it.todo("fetches the active tab's kind with kind=&state=open"); - it.todo("renders only the active kind's rows"); + it("fetches the active tab's kind with kind=&state=open", async () => { + const mockFetch = makeFetchMock({ + findings: { near_duplicate: { rows: [], total: 0 } }, + open: openCounts(), + }); + + await renderGardenerPage({ tab: "near_duplicate" }); + + const call = mockFetch.mock.calls.find(([input]) => String(input).includes("/api/gardener/findings")); + expect(call).toBeDefined(); + const url = new URL(String(call?.[0])); + expect(url.searchParams.get("kind")).toBe("near_duplicate"); + expect(url.searchParams.get("state")).toBe("open"); + }); + + it("renders only the active kind's rows", async () => { + makeFetchMock({ + findings: { dead_file: { rows: [finding(1, 101, "Dead Track")], total: 1 } }, + open: openCounts({ deadFile: 1 }), + }); + + await renderGardenerPage({}); + + expect(screen.getByText("Dead Track")).toBeInTheDocument(); + // Only the active kind's own section renders — never a second kind's section alongside it. + expect(screen.getByRole("region", { name: "Dead files" })).toBeInTheDocument(); + expect(screen.queryByRole("region", { name: "Near duplicates" })).not.toBeInTheDocument(); + expect(screen.queryByRole("region", { name: "Stale metadata" })).not.toBeInTheDocument(); + expect(screen.queryByRole("region", { name: "Unreachable" })).not.toBeInTheDocument(); + expect(screen.queryByRole("region", { name: "Shelf dust" })).not.toBeInTheDocument(); + }); }); describe("Scenario: the URL owns the active tab", () => { - it.todo("activates the tab named by ?tab="); - it.todo("renders each tab as a link to its own ?tab= URL"); + it("activates the tab named by ?tab=", async () => { + makeFetchMock({ + findings: { shelf_dust: { rows: [], total: 0 } }, + open: openCounts(), + }); + + await renderGardenerPage({ tab: "shelf_dust" }); + + expect(screen.getByRole("link", { name: "Shelf dust (0)" })).toHaveAttribute("aria-current", "page"); + }); }); describe("Scenario: an empty kind names itself", () => { - it.todo("renders the kind's own empty state when it has zero open findings"); - it.todo("keeps the empty kind's badge at 0"); + it("renders the kind's own empty state when it has zero open findings", async () => { + makeFetchMock({ + findings: { unreachable: { rows: [], total: 0 } }, + open: openCounts(), + }); + + await renderGardenerPage({ tab: "unreachable" }); + + expect(screen.getByText("Nothing unreachable.")).toBeInTheDocument(); + }); }); describe("Scenario: purge lives on the dead-files tab", () => { - it.todo("renders the purge action in the dead-files tab header"); - it.todo("gives the purge trigger a verb-object label, never a status reading (gh-#655)"); - it.todo("renders no purge action on any other tab"); + it("renders the purge action in the dead-files tab header", async () => { + makeFetchMock({ + findings: { dead_file: { rows: [finding(1, 101, "Dead Track")], total: 1 } }, + open: openCounts({ deadFile: 1 }), + }); + + await renderGardenerPage({}); + + expect(screen.getByRole("button", { name: /purge/i })).toBeInTheDocument(); + }); + + it("gives the purge trigger a verb-object label, never a status reading (gh-#655)", async () => { + makeFetchMock({ + findings: { dead_file: { rows: [finding(1, 101, "Dead Track")], total: 1 } }, + open: openCounts({ deadFile: 1 }), + }); + + await renderGardenerPage({}); + + expect(screen.getByRole("button", { name: "Purge dead tracks…" })).toBeInTheDocument(); + }); + + it("renders no purge action on any other tab", async () => { + makeFetchMock({ + findings: { unreachable: { rows: [finding(1, 101, "Broken link")], total: 1 } }, + open: openCounts({ unreachable: 1 }), + }); + + await renderGardenerPage({ tab: "unreachable" }); + + expect(screen.queryByRole("button", { name: /purge/i })).not.toBeInTheDocument(); + }); }); describe("Scenario: verbs refresh the page", () => { - it.todo("refreshes via router.refresh after a dismiss completes"); + it("refreshes via router.refresh after a dismiss completes", async () => { + makeFetchMock({ + findings: { dead_file: { rows: [finding(1, 101, "Dead Track")], total: 1 } }, + open: openCounts({ deadFile: 1 }), + }); + + await renderGardenerPage({}); + + await act(async () => { + fireEvent.click(screen.getByRole("button", { name: "Dismiss" })); + await Promise.resolve(); + }); + const dialog = await screen.findByRole("dialog"); + await act(async () => { + fireEvent.click(within(dialog).getByRole("button", { name: "Dismiss" })); + await Promise.resolve(); + }); + + await waitFor(() => expect(mockedRefresh).toHaveBeenCalled()); + }); }); - // ── Sad path ────────────────────────────────────────────────────────── describe("Scenario: default and unknown tab fall back silently", () => { - it.todo("activates the first tab in kind order when ?tab= is absent"); - it.todo("activates the first tab in kind order when ?tab= is unrecognized, with no error"); + it("activates the first tab in kind order when ?tab= is absent", async () => { + makeFetchMock({ + findings: { dead_file: { rows: [], total: 0 } }, + open: openCounts(), + }); + + await renderGardenerPage({}); + + expect(screen.getByRole("link", { name: "Dead files (0)" })).toHaveAttribute("aria-current", "page"); + }); + + it("activates the first tab in kind order when ?tab= is unrecognized, with no error", async () => { + makeFetchMock({ + findings: { dead_file: { rows: [], total: 0 } }, + open: openCounts(), + }); + + await renderGardenerPage({ tab: "not-a-real-kind" }); + + expect(screen.getByRole("link", { name: "Dead files (0)" })).toHaveAttribute("aria-current", "page"); + expect(screen.queryByText(/error/i)).not.toBeInTheDocument(); + }); + }); + + // T387 review MED-4: the two unspecced degrade paths — the `open: null` fixture branch + // `makeFetchMock` above already supports becomes reachable here for the first time. + describe("Scenario: the status fetch fails (sad path)", () => { + it("still renders the tab strip unbadged, with rows and a working pager", async () => { + makeFetchMock({ + findings: { dead_file: { rows: [finding(1, 101, "Track A")], total: 30 } }, + open: null, + }); + + await renderGardenerPage({}); + + const nav = screen.getByRole("navigation", { name: "Gardener kinds" }); + expect(within(nav).getAllByRole("link").map((link) => link.textContent)).toEqual([ + "Dead files", + "Near duplicates", + "Stale metadata", + "Unreachable", + "Shelf dust", + ]); + expect(screen.getByRole("link", { name: "Next" })).toHaveAttribute("href", "/gardener?page=2"); + }); + }); + + describe("Scenario: the findings fetch fails (sad path)", () => { + it('shows "Unable to load the Gardener queue." with the tab strip still present', async () => { + const fn = jest.fn().mockImplementation(async (input) => { + const url = new URL(String(input), "http://localhost"); + if (url.pathname === "/api/status") return jsonResponse(200, { gardener: { open: openCounts(), total: 0 } }); + if (url.pathname === "/api/gardener/findings") return jsonResponse(500, {}); + throw new Error(`unexpected fetch call: ${String(input)}`); + }); + global.fetch = fn as unknown as typeof fetch; + + await renderGardenerPage({}); + + expect(screen.getByText("Unable to load the Gardener queue.")).toBeInTheDocument(); + expect(screen.getByRole("navigation", { name: "Gardener kinds" })).toBeInTheDocument(); + }); }); }); diff --git a/admin-ui/app/(authed)/catalog/page.tsx b/admin-ui/app/(authed)/catalog/page.tsx index ca82f710..cd5bfc02 100644 --- a/admin-ui/app/(authed)/catalog/page.tsx +++ b/admin-ui/app/(authed)/catalog/page.tsx @@ -10,6 +10,7 @@ import { PurgeUnavailableAction } from "./PurgeUnavailableAction"; import { YearFilterControl } from "./YearFilterControl"; import { FacetFilterControl } from "./FacetFilterControl"; import { MoodFilterControl } from "./MoodFilterControl"; +import { Pager } from "@/components/ui/pager"; import { Tooltip } from "@/components/ui/tooltip"; import type { AdminMediaDto, BulkFilter, Pagination } from "./types"; @@ -627,23 +628,7 @@ export default async function CatalogPage({ searchParams }: CatalogPageProps): P clearFiltersHref="/catalog" /> - {pagination.pages > 1 && ( - - )} + buildPageUrl(sp, page)} /> ); } diff --git a/admin-ui/app/(authed)/gardener/GardenerPageSizePicker.tsx b/admin-ui/app/(authed)/gardener/GardenerPageSizePicker.tsx new file mode 100644 index 00000000..679e1c59 --- /dev/null +++ b/admin-ui/app/(authed)/gardener/GardenerPageSizePicker.tsx @@ -0,0 +1,43 @@ +import Link from "next/link"; +import type { ReactNode } from "react"; +import { cn } from "@/lib/utils"; +import type { GardenerKind } from "@/lib/gardener-api"; +import { buildGardenerHref, GARDENER_PAGE_SIZES, type GardenerPageSize } from "./gardener-paging"; + +interface GardenerPageSizePickerProps { + kind: GardenerKind; + limit: GardenerPageSize; +} + +/** + * Rows-per-page picker (SPEC F153.10 rider 2026-08-31; STORY-382 AC3-AC4): plain anchors for each + * of {@link GARDENER_PAGE_SIZES}, the same "no client JS" pager idiom the catalog's own Previous/ + * Next links use — text links, never icon-only (T378 law). Picking a size always resets to page 1 + * (`buildGardenerHref` never carries a `page` param). Chip-scale radius and a 40px touch target + * (T387 review LOW-4 — design-aesthetic's chip/badge sizing, matching `TabStrip`'s own `min-h-10`). + */ +export function GardenerPageSizePicker({ kind, limit }: GardenerPageSizePickerProps): ReactNode { + return ( +
+ Rows per page +
+ {GARDENER_PAGE_SIZES.map((size) => { + const active = size === limit; + return ( + + {size} + + ); + })} +
+
+ ); +} diff --git a/admin-ui/app/(authed)/gardener/GardenerRow.tsx b/admin-ui/app/(authed)/gardener/GardenerRow.tsx index f22a2357..58fdf6de 100644 --- a/admin-ui/app/(authed)/gardener/GardenerRow.tsx +++ b/admin-ui/app/(authed)/gardener/GardenerRow.tsx @@ -20,7 +20,7 @@ interface GardenerRowProps { kind: GardenerKind; finding: GardenerFindingDto; /** Re-fetch trigger — called after any successful verb (SPEC F153.10: re-fetch, never a local - * patch — see GardenerView's own remarks). */ + * patch — see GardenerSection's own remarks; PLAN T387 wires this to `router.refresh()`). */ onChanged: () => void; /** A near-duplicate group's own "Keep this one" button (DuplicateGroupCard's slot) — absent for * every other kind, and for a duplicate group's own row when rendered standalone would never diff --git a/admin-ui/app/(authed)/gardener/GardenerSection.tsx b/admin-ui/app/(authed)/gardener/GardenerSection.tsx index 6dbe2106..4aacdac7 100644 --- a/admin-ui/app/(authed)/gardener/GardenerSection.tsx +++ b/admin-ui/app/(authed)/gardener/GardenerSection.tsx @@ -1,4 +1,7 @@ +"use client"; + import type { ReactNode } from "react"; +import { useRouter } from "next/navigation"; import { PurgeUnavailableAction } from "../_components/PurgeUnavailableAction"; import { GARDENER_KIND_EMPTY_LABELS, @@ -13,37 +16,54 @@ interface GardenerSectionProps { kind: GardenerKind; group: GardenerGroupDto; /** `GET /api/status`'s own per-kind OPEN total (SPEC F153.9) — `null` when the status fetch - * itself failed, in which case the header falls back to this page's own row count. */ + * itself failed, in which case the header falls back to {@link total}, EXCEPT for + * `near_duplicate` (T387 review LOW-2, RULED): `openCount` is a ROW count, but `near_duplicate`'s + * own `total` is a GROUP count (STORY-382 AC6/AC8) — falling back to it there would silently swap + * units, showing a group count as though it were an open-row count. That one kind suppresses the + * header count entirely instead (honest beats unit-swapped); every other kind's `total` is + * row-scoped, same unit as `openCount`, so the fallback stays correct there. The old "Showing + * first N of M" flat-paging caveat this header used to carry when the two disagreed is GONE + * (SPEC F153.10 rider 2026-08-31) — a real pager (`Pager`) replaces it. */ openCount: number | null; - onChanged: () => void; + /** The active tab's own exact total (the kind-scoped `GardenerFindingsResponse.total`, STORY-382 + * AC6/AC8) — the header's fallback source when status failed, and what gates the dead_file Purge + * trigger below: a beyond-end page can legitimately render zero rows while the kind itself still + * has dead files to purge, so gating on `total` (not this page's own row count) stays correct. */ + total: number; } /** - * One kind's section (SPEC F153.10, STORY-374 AC9): a header naming the kind and its open count, - * a per-kind empty state when nothing qualifies (LOW-2 — "Nothing here." read as generic; each - * kind now names itself), the "Showing first N of M" flat-paging caveat when this page's own row - * count for the kind is short of the status total (ORCHESTRATOR ruling 2 — rows are paged FLAT - * before grouping, so a page's own count for one kind can legitimately be less than that kind's - * real total), and either a flat row list (every kind but near_duplicate) or one - * {@link DuplicateGroupCard} per duplicate group (near_duplicate only — STORY-376 AC6). + * One kind's section — the tab strip's own content pane (SPEC F153.10 rider 2026-08-31; STORY-381/ + * 382; PLAN T387, gh-#654/#655/#657): a header naming the kind and its open count, a per-kind empty + * state when nothing qualifies (LOW-2 — "Nothing here." read as generic; each kind names itself), + * and either a flat row list (every kind but near_duplicate) or one {@link DuplicateGroupCard} per + * duplicate group (near_duplicate only — STORY-376 AC6, STORY-383 AC4 whole-cluster rendering). + * Exactly ONE kind renders per page load now — the tab strip (`GardenerTabs`) owns which. + * + * This is now the page's own "use client" boundary: `page.tsx` (a Server Component) renders this + * directly, mirroring `catalog/CatalogTable.tsx`'s own split — a top-level client component that + * owns `useRouter()` and threads a `router.refresh()` closure down to every verb, rather than a + * closure passed in as a prop from the server (which RSC cannot serialize). `GardenerView`'s own + * client LoadState/fetch-on-mount — the gh-#654 defect — retires with this: every row verb still + * re-fetches on success, but by asking Next.js to re-render this Server Component, not by holding + * a second client-side copy of the queue. Purge stays dead_file-tab-only, now carrying the gh-#655 + * verb-object label ("Purge dead tracks…"/"Purge dead tracks") — the old "Purge unavailable…" read + * as a status, never naming what the click actually does. * - * T378 review LOW-5/LOW-B: the duplicate-group branch renders from `group.duplicateGroups` — never - * a `kind === "near_duplicate"` check alone — because `duplicateGroups` (not `findings.length`) is - * the actual data that branch draws from. A group with no `groupKey` is filtered out BEFORE - * `hasDuplicateGroups` is computed (not inside the render map, LOW-B's own fix) — Keep this one's - * whole point is "mark the OTHER members of THIS group ineligible", meaningless without a real - * group identity, and filtering only at render time left `hasDuplicateGroups` true even when every - * group had been filtered away, rendering an empty header with nothing under it. Filtering first - * means an all-null set falls through to the flat row list (the SAME fallback every non- - * near_duplicate kind renders) instead. Never reachable from the real backend today — a - * near_duplicate finding always carries its own `group_key` — but this keeps a malformed/future - * response from rendering a Keep-this-one button (or an empty shell) with no group behind it. + * T378 review LOW-5/LOW-B (carried forward verbatim): the duplicate-group branch renders from + * `group.duplicateGroups` — never a `kind === "near_duplicate"` check alone — and a group with no + * `groupKey` is filtered out BEFORE `hasDuplicateGroups` is computed, so a malformed/future + * response falls through to the flat row list instead of an empty shell with nothing under it. */ -export function GardenerSection({ kind, group, openCount, onChanged }: GardenerSectionProps): ReactNode { +export function GardenerSection({ kind, group, openCount, total }: GardenerSectionProps): ReactNode { + const router = useRouter(); + const onChanged = (): void => router.refresh(); + const label = GARDENER_KIND_LABELS[kind]; const rowCount = group.findings.length; - const displayCount = openCount ?? rowCount; - const showingFewer = openCount !== null && rowCount < openCount; + // LOW-2 (RULED): near_duplicate's own `total` is a GROUP count, not a ROW count like `openCount` + // — falling back to it would silently swap units, so that one kind suppresses the count instead. + const displayCount: number | null = openCount ?? (kind === "near_duplicate" ? null : total); const duplicateGroups = group.duplicateGroups.filter((duplicateGroup) => duplicateGroup.groupKey !== null); const hasDuplicateGroups = duplicateGroups.length > 0; @@ -51,19 +71,16 @@ export function GardenerSection({ kind, group, openCount, onChanged }: GardenerS

- {label} · {displayCount} open + {label} + {displayCount !== null && ( + · {displayCount} open + )}

- {kind === "dead_file" && rowCount > 0 && ( - + {kind === "dead_file" && total > 0 && ( + )}
- {showingFewer && ( -

- Showing first {rowCount} of {openCount} -

- )} - {rowCount === 0 &&

{GARDENER_KIND_EMPTY_LABELS[kind]}

} {rowCount > 0 && hasDuplicateGroups && ( diff --git a/admin-ui/app/(authed)/gardener/GardenerTabs.tsx b/admin-ui/app/(authed)/gardener/GardenerTabs.tsx new file mode 100644 index 00000000..0655bb98 --- /dev/null +++ b/admin-ui/app/(authed)/gardener/GardenerTabs.tsx @@ -0,0 +1,42 @@ +import type { ReactNode } from "react"; +import { TabStrip, type TabStripTab } from "@/components/ui/tab-strip"; +import { + GARDENER_KIND_LABELS, + GARDENER_KIND_ORDER, + GARDENER_OPEN_COUNT_KEY, + type GardenerKind, + type GardenerOpenCounts, +} from "@/lib/gardener-api"; +import { buildGardenerHref, type GardenerPageSize } from "./gardener-paging"; + +interface GardenerTabsProps { + activeTab: GardenerKind; + limit: GardenerPageSize; + /** `GET /api/status`'s own per-kind OPEN totals (SPEC F153.9) — `null` when the status fetch + * itself failed, in which case every tab renders unbadged rather than a wrong number: the page + * fetches only the ACTIVE tab's own kind, so status is the only source for the other four. */ + open: GardenerOpenCounts | null; +} + +function tabLabel(kind: GardenerKind, open: GardenerOpenCounts | null): string { + const base = GARDENER_KIND_LABELS[kind]; + return open === null ? base : `${base} (${open[GARDENER_OPEN_COUNT_KEY[kind]]})`; +} + +/** + * The five rot-finding kind tabs (SPEC F153.10 rider 2026-08-31; STORY-381 AC1-AC3/AC7, gh-#654) — + * URL-driven via `?tab=`, the shared `TabStrip` markup (gh-#393's extraction), each label badged + * with that kind's own OPEN count from `/api/status` (STORY-381 AC1). `TabStrip` itself stays + * untouched (T387 scope: the count is embedded IN the label string here rather than widening the + * shared strip's own props) — every kind renders as its own tab regardless of count, the + * `WardrobeTabs`/`PersonaCatalogTabs` "always render every kind" ruling applied here too. + */ +export function GardenerTabs({ activeTab, limit, open }: GardenerTabsProps): ReactNode { + const tabs: TabStripTab[] = GARDENER_KIND_ORDER.map((kind) => ({ + id: kind, + label: tabLabel(kind, open), + href: buildGardenerHref(kind, limit), + })); + + return ; +} diff --git a/admin-ui/app/(authed)/gardener/GardenerView.tsx b/admin-ui/app/(authed)/gardener/GardenerView.tsx deleted file mode 100644 index f19200b6..00000000 --- a/admin-ui/app/(authed)/gardener/GardenerView.tsx +++ /dev/null @@ -1,81 +0,0 @@ -"use client"; - -import { useCallback, useEffect, useState, type ReactNode } from "react"; -import { fetchStatus, type StatusResponse } from "@/lib/broadcast-api"; -import { - fetchGardenerFindings, - GARDENER_KIND_ORDER, - type GardenerFindingsResponse, - type GardenerGroupDto, - type GardenerKind, - type GardenerOpenCounts, - GARDENER_OPEN_COUNT_KEY, -} from "@/lib/gardener-api"; -import { GardenerSection } from "./GardenerSection"; - -type LoadState = - | { kind: "loading" } - | { kind: "loaded"; findings: GardenerFindingsResponse; open: GardenerOpenCounts | null } - | { kind: "error" }; - -const EMPTY_GROUP = (kind: GardenerKind): GardenerGroupDto => ({ kind, findings: [], duplicateGroups: [] }); - -/** - * The Gardener page's data owner (SPEC F153.10, STORY-374 AC9, STORY-376 AC6, PLAN T378): loads - * `GET /api/gardener/findings?state=open&limit=1000` (ORCHESTRATOR ruling 2 — the whole open queue - * in one page, T377's own ceiling) and `GET /api/status` (the per-kind OPEN totals, for the - * "Showing first N of M" flat-paging caveat) in parallel, exactly ONCE on mount — no polling, this - * is a curation console an operator opens to act on, not a live dashboard. - * - * Every row verb (eligibility, never-play, re-enrich, dismiss, Keep this one, Purge unavailable) - * RE-FETCHES both endpoints afterward rather than patching local state — simple and always correct - * against the store's own reconcile passes, which can move a row between kinds/states on their own - * schedule independent of any one operator click; at this page's bounded size (at most 1000 rows) - * a refetch never costs enough to earn the extra state-sync code an optimistic-update model would - * need (ORCHESTRATOR ruling 2's own "pick re-fetch (simple, correct)" call). - */ -export function GardenerView(): ReactNode { - const [state, setState] = useState({ kind: "loading" }); - - const load = useCallback(async () => { - const [findings, status] = await Promise.all([ - fetchGardenerFindings(), - fetchStatus().catch((): StatusResponse | null => null), - ]); - if (findings === null) { - setState({ kind: "error" }); - return; - } - setState({ kind: "loaded", findings, open: status?.gardener?.open ?? null }); - }, []); - - useEffect(() => { - void load(); - }, [load]); - - if (state.kind === "loading") { - return

Loading…

; - } - - if (state.kind === "error") { - return

Couldn't load the Gardener queue — try refreshing.

; - } - - const groupsByKind = new Map( - state.findings.groups.map((group) => [group.kind, group]) - ); - - return ( -
- {GARDENER_KIND_ORDER.map((kind) => ( - void load()} - /> - ))} -
- ); -} diff --git a/admin-ui/app/(authed)/gardener/gardener-paging.ts b/admin-ui/app/(authed)/gardener/gardener-paging.ts new file mode 100644 index 00000000..40c44f5f --- /dev/null +++ b/admin-ui/app/(authed)/gardener/gardener-paging.ts @@ -0,0 +1,141 @@ +// The Gardener page's own `?tab=&page=&limit=` resolution and href builders (SPEC F153.10 rider +// 2026-08-31; STORY-381, STORY-382; PLAN T387, gh-#654/#655/#657). Plain TypeScript, no JSX and no +// Next.js request APIs — the ONE place this page's searchParams get parsed and re-assembled, kept +// separate from `page.tsx` (an async Server Component Jest can't easily drive for every scenario) +// so every rule below is testable by calling a function, mirroring the extraction `catalog/page.tsx` +// keeps inline only because its own test harness (the tree-walker in catalog-pages.spec.ts) can +// afford to call the whole async page function per case. +// +// URL semantics (binding, SPEC F153.10 rider): +// - tab: one of `GARDENER_KIND_ORDER`'s own tokens; absent, repeated, or unrecognised all fall +// back to the FIRST kind in that order, silently — never a 400, never an error state. +// - page: a positive integer; absent/non-numeric/less than 1 falls back to 1. Unclamped against +// the kind's own last page — a page past the real end is a legal request (the kind's empty +// state renders, the pager's own Previous link stays live). It IS clamped against Int32 +// overflow (T387 review MED-1): see `resolveGardenerPaging`'s own remarks. +// - limit: one of `GARDENER_PAGE_SIZES`; absent or out-of-set falls back to +// `DEFAULT_GARDENER_PAGE_SIZE` — the same "a paging value is a hint, never a contract a client +// can get wrong" posture `GardenerController`'s own server-side clamp already takes. +// - offset = (page - 1) * limit — a plain paging-unit count; whether that unit is rows or +// near_duplicate GROUPS is `Garden.RotFindingRepository`'s own concern, not this page's. + +import { GARDENER_KIND_ORDER, type GardenerKind } from "@/lib/gardener-api"; + +/** Raw `?tab=&page=&limit=` values exactly as Next.js hands them back — each MAY arrive as a + * string array if the query string repeats the key. None of these three is ever meant to repeat, + * but every resolver below treats a repeated value the same as an unrecognised one (falls back to + * default) rather than picking one arbitrarily — mirrors `resolveWardrobeTab`'s own defensive + * "absent, an array, or a stranger" posture (gh-#393). */ +export interface GardenerSearchParams { + tab?: string | string[]; + page?: string | string[]; + limit?: string | string[]; +} + +export const GARDENER_PAGE_SIZES = [25, 50, 100, 250] as const; +export type GardenerPageSize = (typeof GARDENER_PAGE_SIZES)[number]; +export const DEFAULT_GARDENER_PAGE_SIZE: GardenerPageSize = 25; + +export interface ResolvedGardenerPaging { + tab: GardenerKind; + page: number; + limit: GardenerPageSize; + offset: number; +} + +/** The URL's own founding tab — bare `/gardener` and every tab-preserving href omit `?tab=` + * entirely for this one kind, mirroring `CatalogTabs`' bare `/catalog` href for its own founding + * "tracks" tab. `GARDENER_KIND_ORDER`'s own `as const satisfies` typing (T387 review LOW-3) makes + * `[0]` a plain `GardenerKind` under `noUncheckedIndexedAccess` — no runtime empty-array guard + * needed for a five-entry constant that is never actually empty. */ +const FIRST_GARDENER_TAB: GardenerKind = GARDENER_KIND_ORDER[0]; + +function isGardenerKind(value: string): value is GardenerKind { + return (GARDENER_KIND_ORDER as readonly string[]).includes(value); +} + +/** Resolves `?tab=` (SPEC F153.10 rider, STORY-381 AC1/AC7) — absent, repeated, or unrecognised + * all fall back to {@link FIRST_GARDENER_TAB} silently. */ +export function resolveGardenerTab(raw: string | string[] | undefined): GardenerKind { + return typeof raw === "string" && isGardenerKind(raw) ? raw : FIRST_GARDENER_TAB; +} + +/** Resolves `?limit=` — one of {@link GARDENER_PAGE_SIZES} or {@link DEFAULT_GARDENER_PAGE_SIZE}. + * An out-of-set value (e.g. `?limit=999`) reads as the default rather than 400ing. */ +export function resolveGardenerPageSize(raw: string | string[] | undefined): GardenerPageSize { + if (typeof raw !== "string") return DEFAULT_GARDENER_PAGE_SIZE; + const parsed = Number(raw); + return (GARDENER_PAGE_SIZES as readonly number[]).includes(parsed) + ? (parsed as GardenerPageSize) + : DEFAULT_GARDENER_PAGE_SIZE; +} + +/** Resolves `?page=` — a positive integer, defaulting to 1 for anything absent, non-numeric, or + * less than 1. Unclamped against the kind's own last page here — {@link resolveGardenerPaging} + * applies the separate Int32-overflow clamp once it knows `limit` too. */ +export function resolveGardenerPageNumber(raw: string | string[] | undefined): number { + if (typeof raw !== "string") return 1; + const parsed = Number.parseInt(raw, 10); + return Number.isInteger(parsed) && parsed >= 1 ? parsed : 1; +} + +/** `GardenerController`'s own `offset` query parameter is a C# `int?` — a derived offset beyond + * `Int32.MaxValue` fails ASP.NET model binding, and `[ApiController]`'s automatic validation turns + * that into a 400 (T387 review MED-1). SPEC F153.10 rider's "a paging value is a hint, never a + * contract a client can get wrong" promise means an absurd `?page=` must degrade — same as an + * out-of-set `?limit=` — rather than error, so the resolved `page` is capped at the largest value + * whose `(page - 1) * limit` still fits in Int32. */ +const INT32_MAX = 2147483647; + +function clampGardenerPageForOffset(page: number, limit: number): number { + const maxPage = Math.floor(INT32_MAX / limit) + 1; + return Math.min(page, maxPage); +} + +/** Resolves the full `{ tab, page, limit, offset }` tuple `page.tsx` needs from one raw + * searchParams object — the single call site composing the resolvers above, plus the Int32-overflow + * clamp {@link clampGardenerPageForOffset} needs `limit` for. */ +export function resolveGardenerPaging(sp: GardenerSearchParams): ResolvedGardenerPaging { + const tab = resolveGardenerTab(sp.tab); + const limit = resolveGardenerPageSize(sp.limit); + const page = clampGardenerPageForOffset(resolveGardenerPageNumber(sp.page), limit); + return { tab, page, limit, offset: (page - 1) * limit }; +} + +/** "Page N of M" from a kind-scoped `total` (STORY-382 AC6/AC8's own EXACT per-kind count — GROUPS + * for `near_duplicate`, ROWS for every other kind, per `GardenerController`'s own remarks) — never + * derived from `/api/status`'s own OPEN-only count, which answers a different question entirely + * (SPEC F153.10 rider). At least 1, even over an empty kind, so a caller never special-cases a + * zero total before dividing. */ +export function resolveGardenerPageCount(total: number, limit: number): number { + return Math.max(1, Math.ceil(total / limit)); +} + +// ── Href builders ──────────────────────────────────────────────────────────────────────────── +// +// `limit` rides a href only when it differs from the default, and `page` only past page 1 — the +// common case stays the cleanest URL, mirroring `CatalogTabs`' own "founding tab omits ?tab=" +// convention. + +function assembleGardenerHref(kind: GardenerKind, limit: GardenerPageSize, page?: number): string { + const query = new URLSearchParams(); + if (kind !== FIRST_GARDENER_TAB) query.set("tab", kind); + if (limit !== DEFAULT_GARDENER_PAGE_SIZE) query.set("limit", String(limit)); + if (page !== undefined && page > 1) query.set("page", String(page)); + const qs = query.toString(); + return qs ? `/gardener?${qs}` : "/gardener"; +} + +/** A same-page-reset link for a given kind+limit (STORY-381 AC3, STORY-382 AC3-AC4) — the ONE + * builder for both the tab strip (switching kind, same `limit`) and the size picker (same kind, + * switching `limit`): both are, structurally, "the href for this kind+limit combination, page + * reset to 1" (T387 review LOW-1 — `buildGardenerTabHref` and `buildGardenerLimitHref` computed + * byte-identical output and are collapsed into this one name). */ +export function buildGardenerHref(kind: GardenerKind, limit: GardenerPageSize): string { + return assembleGardenerHref(kind, limit); +} + +/** A Previous/Next pager link (STORY-382 AC1-AC5, AC7) — same tab and `limit`, the target `page`. */ +export function buildGardenerPageHref(kind: GardenerKind, limit: GardenerPageSize, page: number): string { + return assembleGardenerHref(kind, limit, page); +} diff --git a/admin-ui/app/(authed)/gardener/page.tsx b/admin-ui/app/(authed)/gardener/page.tsx index 667f0cf1..c358f5e0 100644 --- a/admin-ui/app/(authed)/gardener/page.tsx +++ b/admin-ui/app/(authed)/gardener/page.tsx @@ -1,20 +1,109 @@ import type { ReactNode } from "react"; -import { GardenerView } from "./GardenerView"; - -// The Library Gardener's own admin page (SPEC F153.10, STORY-374 AC9, STORY-376 AC6, PLAN T378, -// gh-#529): one section per rot-finding kind, each row offering the existing eligibility/never-play/ -// re-enrich/dismiss verbs (plus a section-level Purge unavailable for dead files), and "Keep this -// one" on a near-duplicate group. No SSR prefetch, same posture as /live and /booth-log — auth is -// already enforced by middleware.ts on this route, and GardenerView loads its own data client-side -// once on mount (no polling: this is a curation console an operator opens to act on, not a live -// dashboard). -export default function GardenerPage(): ReactNode { +import { cookies } from "next/headers"; +import { apiGet } from "@/lib/api"; +import { Pager } from "@/components/ui/pager"; +import { + buildGardenerFindingsPath, + GARDENER_OPEN_COUNT_KEY, + type GardenerFindingsResponse, + type GardenerGroupDto, + type GardenerKind, + type GardenerOpenCounts, +} from "@/lib/gardener-api"; +import { GardenerTabs } from "./GardenerTabs"; +import { GardenerSection } from "./GardenerSection"; +import { GardenerPageSizePicker } from "./GardenerPageSizePicker"; +import { + buildGardenerPageHref, + resolveGardenerPageCount, + resolveGardenerPaging, + type GardenerSearchParams, +} from "./gardener-paging"; + +// The Library Gardener's own admin page (SPEC F153.10 rider 2026-08-31; STORY-381/382/383; PLAN +// T387, gh-#654/#655/#657): a server-rendered route reading `?tab=&page=&limit=` — one tab strip +// (GardenerTabs), one kind's own section (GardenerSection), and a plain-anchor pager/size picker, +// the catalog page's own idiom (`catalog/page.tsx`) applied here. Auth is already enforced by +// middleware.ts on this route. The queue changes via every row verb (dismiss/eligibility/ +// never-play/re-enrich/Keep this one/purge) — always re-render fresh, mirroring catalog/page.tsx's +// own posture, rather than the retired GardenerView's client-side polling-free-but-still-client +// fetch (gh-#654). +export const dynamic = "force-dynamic"; +export const fetchCache = "force-no-store"; + +interface GardenerPageProps { + searchParams: Promise; +} + +const EMPTY_GROUP = (kind: GardenerKind): GardenerGroupDto => ({ kind, findings: [], duplicateGroups: [] }); + +/** `GET /api/status`'s own `gardener.open` block — the tab strip's badge source (SPEC F153.9). + * Narrow, unvalidated read of a 2xx body (mirrors `personas/page.tsx`'s own `StatusRow`): a shape + * surprise degrades to "no badges" rather than throwing mid-render. */ +interface StatusRow { + gardener?: { open: GardenerOpenCounts }; +} + +const PAGE_TITLE =

Gardener

; + +export default async function GardenerPage({ searchParams }: GardenerPageProps): Promise { + const sp = await searchParams; + const { tab, page, limit, offset } = resolveGardenerPaging(sp); + const cookieStore = await cookies(); + const cookieHeader = cookieStore.toString(); + + // Status is a best-effort degrade (the tab badges only, mirrors personas/page.tsx's own + // on-air-badge posture) — a reject or non-2xx here must never take the required findings read + // down with it, so this rides Promise.allSettled rather than Promise.all. + const [statusResult, findingsResult] = await Promise.allSettled([ + apiGet("/api/status", { cookies: cookieHeader }), + apiGet(buildGardenerFindingsPath(tab, limit, offset), { cookies: cookieHeader }), + ]); + + const open: GardenerOpenCounts | null = + statusResult.status === "fulfilled" && statusResult.value.ok + ? ((await statusResult.value.json()) as StatusRow).gardener?.open ?? null + : null; + + if (findingsResult.status === "rejected" || !findingsResult.value.ok) { + return ( +
+ {PAGE_TITLE} +
+ +
+

Unable to load the Gardener queue.

+
+ ); + } + + const body = (await findingsResult.value.json()) as GardenerFindingsResponse; + const group = body.groups.find((candidate) => candidate.kind === tab) ?? EMPTY_GROUP(tab); + // `total` is a JSON NUMBER on a kind-scoped response (T386's own guaranteed shape, this call is + // always kind-scoped) — the fallback below only guards an off-shape/malformed body, mirroring + // this page's other unvalidated-2xx-body reads. + const total = typeof body.total === "number" ? body.total : group.findings.length; + const pages = resolveGardenerPageCount(total, limit); + return (
-

Gardener

+ {PAGE_TITLE} +
- + +
+ +
+
+ + buildGardenerPageHref(tab, limit, target)} /> +
); } diff --git a/admin-ui/components/ui/pager.tsx b/admin-ui/components/ui/pager.tsx new file mode 100644 index 00000000..95877cde --- /dev/null +++ b/admin-ui/components/ui/pager.tsx @@ -0,0 +1,40 @@ +import Link from "next/link"; +import type { ReactNode } from "react"; + +export interface PagerProps { + page: number; + pages: number; + /** The href for a given target page — every OTHER active query param (filters, tab, limit) is + * the caller's own concern, not this component's; this is just "page N" plugged into whatever + * URL shape the caller already owns. */ + hrefFor: (page: number) => string; +} + +/** + * "Page N of M" + Previous/Next plain anchors (T387 review MED-3) — the ONE pager implementation + * shared by the Catalog (`catalog/page.tsx`) and Gardener (`gardener/page.tsx`) pages, which had + * grown byte-identical copies of this same markup independently. No client JS: `page`/`pages` + * alone decide which anchors render — a `page` past `pages` (a legal, beyond-the-end request, SPEC + * F153.10 rider) still renders a live Previous, just no Next. + */ +export function Pager({ page, pages, hrefFor }: PagerProps): ReactNode { + if (pages <= 1) return null; + + return ( + + ); +} diff --git a/admin-ui/lib/gardener-api.ts b/admin-ui/lib/gardener-api.ts index c558b528..4e4ac841 100644 --- a/admin-ui/lib/gardener-api.ts +++ b/admin-ui/lib/gardener-api.ts @@ -18,14 +18,18 @@ export type GardenerKind = "dead_file" | "near_duplicate" | "stale_metadata" | " export type GardenerFindingState = "open" | "dismissed" | "resolved"; /** Section order (SPEC F153.10, ORCHESTRATOR ruling 2) — fixed, never derived from whatever order - * the api response happens to list groups in. */ -export const GARDENER_KIND_ORDER: readonly GardenerKind[] = [ + * the api response happens to list groups in. `as const satisfies` (T387 review LOW-3) keeps this a + * literal tuple — `noUncheckedIndexedAccess` then types `GARDENER_KIND_ORDER[0]` as `GardenerKind` + * directly rather than `GardenerKind | undefined`, so a caller needing the founding tab (e.g. + * `gardener-paging.ts`) never needs its own runtime empty-array guard for a five-entry constant + * that is never actually empty. */ +export const GARDENER_KIND_ORDER = [ "dead_file", "near_duplicate", "stale_metadata", "unreachable", "shelf_dust", -]; +] as const satisfies readonly GardenerKind[]; /** Section header copy — sentence-cased per Dean's copy rule (capitals open every sentence). */ export const GARDENER_KIND_LABELS: Record = { @@ -144,27 +148,27 @@ export interface GardenerGroupDto { export interface GardenerFindingsResponse { groups: GardenerGroupDto[]; + /** Present only for a kind-scoped call (SPEC F153.9 rider 2026-08-31; STORY-382 AC6/AC8; PLAN + * T386) — the exact count of paging units for that one kind: GROUPS for `near_duplicate`, ROWS + * for every other kind (`GardenerController.GetFindings`'s own remarks). Absent for an + * un-scoped call — `GardenerController`'s T377 shape stays byte-compatible. */ + total?: number; } -function isGardenerFindingsResponse(raw: unknown): raw is GardenerFindingsResponse { - return typeof raw === "object" && raw !== null && Array.isArray((raw as { groups?: unknown }).groups); -} - -/** `GET /api/gardener/findings?state=open&limit=1000` (ORCHESTRATOR ruling 2 — the page's own - * "whole queue in one page" read, T377's own ceiling). Never throws: a network failure, non-2xx, - * or off-shape 200 body all resolve to `null` so the page can render its own unavailable state. */ -export async function fetchGardenerFindings(): Promise { - try { - const response = await fetch("/api/gardener/findings?state=open&limit=1000", { - credentials: "include", - cache: "no-store", - }); - if (!response.ok) return null; - const raw = (await response.json()) as unknown; - return isGardenerFindingsResponse(raw) ? raw : null; - } catch { - return null; - } +/** `GET /api/gardener/findings`'s own path+query for a kind-scoped, open-state read (SPEC F153.10 + * rider 2026-08-31; STORY-381/382; PLAN T387) — always `state=open` (the Gardener page only ever + * shows the live queue) and always kind-scoped, so the response always carries + * {@link GardenerFindingsResponse.total}. The page's own server-side `apiGet` call + * (`gardener/page.tsx`) is the only caller (T387 review MED-2: the browser-fetcher counterpart this + * module used to also export, `fetchGardenerFindings`, had zero call sites once `GardenerView` + * retired — deleted rather than kept as unreachable API surface). */ +export function buildGardenerFindingsPath(kind: GardenerKind, limit: number, offset: number): string { + const query = new URLSearchParams(); + query.set("kind", kind); + query.set("state", "open"); + query.set("limit", String(limit)); + query.set("offset", String(offset)); + return `/api/gardener/findings?${query.toString()}`; } export type DismissFindingOutcome = { ok: true } | { ok: false; detail: string }; diff --git a/src/GenWave.Core/Abstractions/IRotFindingStore.cs b/src/GenWave.Core/Abstractions/IRotFindingStore.cs index 0ede5a91..4b736cc7 100644 --- a/src/GenWave.Core/Abstractions/IRotFindingStore.cs +++ b/src/GenWave.Core/Abstractions/IRotFindingStore.cs @@ -191,38 +191,93 @@ Task> ListAsync( RotKind? kind, RotState? state, CancellationToken ct, int limit = 200, int offset = 0); /// - /// 's own filters and ordering (newest-opened first — a match kind, then - /// so a pair's own rows sit - /// together, then descending), joined out to the + /// 's own filters, joined out to the /// library.media/library.media_rotation/library.media_rating row each finding - /// is about — T377's admin surface (SPEC F153.9, STORY-374 AC7) reuses this same paging AND the - /// SAME callee-enforced floor 's own remarks describe, rather than adding - /// its own bound. + /// is about — T377's admin surface (SPEC F153.9, STORY-374 AC7), extended at T385 (SPEC F153.9 + /// rider 2026-08-31; STORY-382 AC6, STORY-383) to return of + /// matching paging units alongside the page, computed against the SAME / + /// filter — EXACT for a kind-scoped read; (never + /// derived from a second query, T386 review — the type itself now carries "not computed") for the + /// kind-less read — see below for exactly which. /// /// - /// Rows are paged FLAT, before GardenerController ever groups them by kind (T377 - /// review MED). The limit/offset window applies to the ROW sequence this method - /// returns, not to "N groups" or "N findings per kind" — a caller paging with a small - /// can see a group split across a - /// page boundary (its own rows are adjacent within one page thanks to the group_key - /// ordering above, but a page edge can still fall inside a group). GardenerController's - /// own default is 200, ceiling 1000 ('s own bound) — the admin queue is - /// small enough in practice that a caller wanting the WHOLE thing in one page (T378's own review - /// queue) simply passes limit=1000. Per-kind OPEN totals are 's - /// own job (surfaced on GET /api/status) — a page of THIS method's own result is never the - /// right place to derive a total count from, since it is bounded by construction. + /// The kind-LESS read ( ) keeps T377's exact row + /// shape, verbatim (regression pin) — but is now + /// (T385 review LOW-2; made an actual rather than + /// a same-as--count stand-in at T386 review). Rows page FLAT, in + /// kind, group_key nulls last, opened_at desc, id order, BEFORE GardenerController + /// ever groups them by kind — a caller paging with a small can still see a + /// group split across a page boundary here (its own rows are + /// adjacent within one page thanks to the group_key ordering, but a page edge can still fall + /// inside a group). No second query ever runs to compute a true matching row count across every + /// kind for this shape. GardenerController puts on the + /// wire as total exactly when it is non-null (T386) — a kind-less call's response therefore + /// carries no total member at all (T377's own pinned response shape carries no count + /// field either, STORY-382 AC8). /// /// /// - /// T377 review LOW-2: the order by behind this method is NOT index-covered — - /// group_key sits mid-key (between kind and opened_at), and no index on - /// library.rot_finding leads with it, so Postgres sorts the whole filtered join result - /// before applying LIMIT rather than walking an already-ordered index. The bound above is - /// what keeps that sort cheap regardless of table size — see - /// Garden.RotFindingRepository.ListWithMediaAsync's own remarks for the query itself. + /// A kind-scoped read ( non-) pages WITHIN that + /// kind, and IS the exact matching count. For every kind + /// except , this is the SAME flat row paging, narrowed by + /// kind — is the exact matching row count. For + /// , the PAGING UNIT is the GROUP: / + /// count DISTINCT group_keys, ordered ascending (stable across + /// pages), and carries EVERY member row of every SELECTED group + /// — a page can never hold a partial group, so a caller acting on a whole cluster (Keep-this-one) + /// never sees a truncated one. Row order within a near-duplicate page stays group_key asc, + /// opened_at desc, id — the SAME relative order the flat kind-scoped shape's own group_key, + /// opened_at desc, id tail gives a kind already narrowed to one value, so + /// GardenerController's own grouping-by-group_key logic keeps working unchanged + /// either way. here is the exact count of DISTINCT matching + /// group_keys, never the row count of the returned page. + /// + /// + /// + /// RULED (T385 review HIGH-1): for the group-paged shape, + /// scopes which GROUPS qualify, never which MEMBER rows render. A + /// group qualifies the moment ANY ONE of its members matches kind = near_duplicate + + /// ; once a group qualifies, ALL of its member rows render in + /// regardless of each member's own individual state — the + /// whole-cluster contract above ("a page can never hold a partial group") is unconditional, per SPEC + /// F153.9 rider's own binding text: "the response returns every member row of every selected + /// group". A group where NO member matches (every member dismissed, under + /// state=open) does not qualify at all — it consumes no page slot and does not count into + /// . + /// + /// + /// + /// RULED (round-2 review HIGH-2): a RESOLVED member row never renders inside its group, even + /// though its own group_key survives a resolve untouched. A near-duplicate member that + /// left library.find_near_duplicates on its own (an operator retagged it; it is genuinely no + /// longer a duplicate, still eligible, still in rotation) must not keep appearing inside its old + /// group — a caller's Keep-this-one bulk write would otherwise pull that distinct, in-rotation track + /// out of rotation. dismissed = the operator closed the finding while the media is still a + /// duplicate → render; resolved = the system closed it because the media is no longer a duplicate → + /// don't render. This exclusion is member-side only — it never changes which GROUPS qualify + /// above. + /// + /// + /// + /// T385 review MED-4: the shared ClampPaging cap (1000, see Garden.RotFindingRepository's + /// own remarks) counts ROWS for every kind (and the kind-less read) but counts GROUPS for + /// — there bounds the number of + /// DISTINCT group_keys, so the row envelope for that page is at most 1000 groups × each + /// group's own member count (typically 2–5), never a flat 1000-row cap; a per-row cap would force a + /// partial group onto a page, which the whole-cluster contract above forbids. + /// + /// + /// + /// T377 review LOW-2 (unaffected by T385): the flat shape's own order by is NOT + /// index-covered — group_key sits mid-key (between kind and opened_at), and + /// no index on library.rot_finding leads with it, so Postgres sorts the whole filtered join + /// result before applying LIMIT rather than walking an already-ordered index. The + /// bound (ClampPaging's own 1000-row cap) is what keeps that sort + /// cheap regardless of table size — see Garden.RotFindingRepository.ListWithMediaAsync's + /// own remarks for the query text itself. /// /// - Task> ListWithMediaAsync( + Task ListWithMediaAsync( RotKind? kind, RotState? state, int limit, int offset, CancellationToken ct); /// diff --git a/src/GenWave.Core/Domain/RotFindingPage.cs b/src/GenWave.Core/Domain/RotFindingPage.cs new file mode 100644 index 00000000..511a5431 --- /dev/null +++ b/src/GenWave.Core/Domain/RotFindingPage.cs @@ -0,0 +1,38 @@ +namespace GenWave.Core.Domain; + +/// +/// One page of 's own joined read (SPEC +/// F153.9 rider 2026-08-31; STORY-382 AC6/AC8, STORY-383; PLAN T385/T386) — is the +/// limit/offset window's own rows. is the exact count of matching +/// PAGING UNITS for a KIND-SCOPED read — matching rows for a flat kind, matching DISTINCT +/// group_keys for (T385 review HIGH-1: a group counts once +/// it qualifies, i.e. at least one member matches the state filter — every one of its member rows then +/// renders regardless of that member's own state) — but is (T386 review, taking +/// T385's own carry-forward) for the KIND-LESS read, which never runs a second query to compute an +/// exact cross-kind total: the type itself now says "not computed" rather than overloading a real page +/// size into that meaning. is never the row count of itself for +/// a near-duplicate page, which is every MEMBER row of the selected groups, not one row per paging +/// unit. +/// +/// +/// RULED (round-2 review HIGH-2): a near-duplicate group's rendered member rows exclude any +/// member whose OWN state is — the resolve half never clears a row's +/// own group_key, so a member that left find_near_duplicates on its own (no longer a +/// duplicate, still eligible, still in rotation) would otherwise keep rendering inside its old group. +/// Dismissed = the operator closed the finding while the media is still a duplicate → render; +/// resolved = the system closed it because the media is no longer a duplicate → don't render. This +/// never changes which groups qualify into above, only which of a qualifying +/// group's own rows land in . +/// +/// +/// Every row the read returns for this page — for +/// , every member row of every group the page selected (a page +/// never holds a partial group); for every other kind (and the kind-less read), the flat row window +/// itself. +/// for the kind-less read (page-local paging with no exact +/// cross-kind total, never computed); otherwise the exact count of matching paging units for a +/// kind-scoped read — distinct group_keys for , matching rows +/// otherwise. GardenerController (T386) puts this on the wire as total exactly when it is +/// non-null — a kind-less call's response therefore carries no total member at all, the +/// T377-pinned shape, STORY-382 AC8. +public sealed record RotFindingPage(IReadOnlyList Items, int? Total); diff --git a/src/GenWave.Host/Api/GardenerController.cs b/src/GenWave.Host/Api/GardenerController.cs index f70472e2..fd5a7e9e 100644 --- a/src/GenWave.Host/Api/GardenerController.cs +++ b/src/GenWave.Host/Api/GardenerController.cs @@ -64,8 +64,8 @@ public sealed class GardenerController(IRotFindingStore store, ILogger - /// No count field, no X-Pagination header (T377 review MED, RULED). This - /// queue is not a browse table: rows are paged FLAT, in the SAME + /// No count field, no X-Pagination header, for an UN-SCOPED call (T377 review + /// MED, RULED). This queue is not a browse table: rows are paged FLAT, in the SAME /// kind, group_key nulls last, opened_at desc, id order /// 's own remarks describe, BEFORE this action /// ever groups them by kind — a group's own rows are adjacent within a page, but a page boundary @@ -79,6 +79,19 @@ public sealed class GardenerController(IRotFindingStore store, ILogger /// + /// + /// Rider (SPEC F153.9 rider 2026-08-31; STORY-382 AC6/AC8; PLAN T386): a kind-SCOPED + /// call now carries total. The MED finding above stands verbatim for an UN-SCOPED call + /// (no kind in the query string) — its response is still EXACTLY { groups }, no + /// total member at all, the T377 shape byte-for-byte. A kind-scoped call gains a + /// top-level total: the exact count of matching paging units for that one kind — GROUPS for + /// kind=near_duplicate ('s own group-paged + /// read, PLAN T385), ROWS for every other kind. This sidesteps the MED finding's own objection + /// entirely: once kind pins the read to one paging space, "the count within this page" and + /// "the kind's real total" are the SAME query, computed exactly + /// ('s own remarks), not a per-page approximation. + /// + /// /// kind/state are the store's own snake_case wire text (/ /// : dead_file, near_duplicate, stale_metadata, /// shelf_dust, unreachable / open, dismissed, resolved) — the @@ -126,14 +139,20 @@ public async Task GetFindings( var effectiveLimit = Math.Clamp(limit ?? DefaultLimit, 1, MaxLimit); var effectiveOffset = Math.Max(offset ?? 0, 0); - var rows = await store.ListWithMediaAsync(kindFilter, stateFilter, effectiveLimit, effectiveOffset, ct); + var page = await store.ListWithMediaAsync(kindFilter, stateFilter, effectiveLimit, effectiveOffset, ct); - var groups = rows + var groups = page.Items .GroupBy(row => row.Finding.Kind) .Select(BuildGroup) .ToList(); - return Ok(new { groups }); + // T386 (SPEC F153.9 rider 2026-08-31; STORY-382 AC6/AC8): page.Total is non-null exactly for a + // kind-scoped read (RotFindingPage's own contract) — total rides the wire only when the store + // actually computed one, so the kind-less call's response stays byte-compatible with T377's + // pinned shape (no "total" member at all, never a null-valued one). + return page.Total is int total + ? Ok(new { groups, total }) + : Ok(new { groups }); } /// diff --git a/src/GenWave.MediaLibrary/Garden/RotFindingRepository.cs b/src/GenWave.MediaLibrary/Garden/RotFindingRepository.cs index e4ffc46a..eb12e8a5 100644 --- a/src/GenWave.MediaLibrary/Garden/RotFindingRepository.cs +++ b/src/GenWave.MediaLibrary/Garden/RotFindingRepository.cs @@ -668,6 +668,34 @@ update library.rot_finding return rows > 0; } + /// + /// The kind/state "omitted means any" condition list every paged read in this file + /// needs (T385 review MED-2/LOW-4) — , , and + /// 's own count/group-CTE pair each hand-rolled this + /// same "if given, add a condition and bind it" shape before T385, with only the column ALIAS + /// differing between an unaliased read (, the group CTE) and one joined + /// under an alias ('s f., the near-duplicate member join's + /// own f.) — parameterises exactly that difference, one + /// spelling for the rest. Mutates / in + /// place (the caller's own accumulators, already built up this way at every call site) rather than + /// returning a second pair the caller would have to merge. + /// + static void AppendKindStateConditions( + List conditions, DynamicParameters parameters, string columnPrefix, RotKind? kind, RotState? state) + { + if (kind is not null) + { + conditions.Add($"{columnPrefix}kind = @kind::library.rot_kind"); + parameters.Add("kind", RotKindTokens.ToToken(kind.Value)); + } + + if (state is not null) + { + conditions.Add($"{columnPrefix}state = @state::library.rot_state"); + parameters.Add("state", RotStateTokens.ToToken(state.Value)); + } + } + /// /// Bounded, paged (T372 review LOW-2) — conditional predicates appended only when the matching /// filter is supplied, the same MediaRotationRepository.AppendSafeExclusion-style @@ -693,18 +721,7 @@ public async Task> ListAsync( var conditions = new List(); var parameters = new DynamicParameters(); - - if (kind is not null) - { - conditions.Add("kind = @kind::library.rot_kind"); - parameters.Add("kind", RotKindTokens.ToToken(kind.Value)); - } - - if (state is not null) - { - conditions.Add("state = @state::library.rot_state"); - parameters.Add("state", RotStateTokens.ToToken(state.Value)); - } + AppendKindStateConditions(conditions, parameters, columnPrefix: "", kind, state); parameters.Add("limit", limit); parameters.Add("offset", offset); @@ -728,82 +745,258 @@ limit @limit offset @offset } /// - /// 's own filters, joined out to library.media/ - /// library.media_rotation/library.media_rating for the admin surface's listing - /// (SPEC F153.9; STORY-374 AC7; PLAN T377) — GardenerController's ONE new joined read - /// rather than an N-lookup fan-out. Ordered kind, group_key nulls last, opened_at desc, id - /// — a group's own rows land adjacent (id breaks ties for a - /// stable order across pages) so the controller's own grouping never has to re-sort. join - /// (not left join) against library.media: a rot_finding row's own FK - /// guarantees the media row still exists (on delete cascade, db/41) — a genuine orphan - /// would be a data-integrity bug this query should surface as a thrown mapping failure, not paper - /// over with a null-media row the controller would then have to special-case. plays - /// defaults to 0 (never null) for a media id with no media_rotation row; rating - /// stays null (never the F33.2 ledger default of 50) so "never rated" and "rated 50" stay - /// distinguishable to an operator triaging a finding. + /// 's own join — library.rot_finding joined out to + /// library.media/library.media_rotation/library.media_rating (SPEC F153.9; + /// STORY-374 AC7; PLAN T377) — shared verbatim by 's own select + /// list and 's own member-row select (T385: one + /// definition, both statements, the DeadFilePredicate idiom applied to a select list + /// instead of a predicate). join (not left join) against library.media: a + /// rot_finding row's own FK guarantees the media row still exists (on delete cascade, + /// db/41) — a genuine orphan would be a data-integrity bug this query should surface as a thrown + /// mapping failure, not paper over with a null-media row the controller would then have to + /// special-case. plays defaults to 0 (never null) for a media id with no + /// media_rotation row; rating stays null (never the F33.2 ledger default of 50) so + /// "never rated" and "rated 50" stay distinguishable to an operator triaging a finding. + /// + const string FindingWithMediaSelectList = + """ + f.id, f.media_id, f.kind::text as kind, f.state::text as state, f.group_key, + f.evidence::text as evidence, f.opened_at, f.resolved_at, f.dismissed_at, f.updated_at, + m.path as locator, m.title, m.artist, m.duration_ms, + coalesce(rot.play_count, 0) as plays, r.score as rating, + coalesce(r.never_play, false) as never_play, m.eligible + """; + + /// + /// 's own join tail, shared verbatim by both this method's + /// callers so the two statements can never drift on which ledger/rating row a finding joins to. + /// + const string FindingWithMediaJoins = + """ + join library.media m on m.id = f.media_id + left join library.media_rotation rot on rot.media_id = m.id + left join library.media_rating r on r.media_id = m.id + """; + + /// + /// (SPEC F153.9 rider 2026-08-31; STORY-382 AC6, + /// STORY-383; PLAN T385) — pages by GROUP + /// (); every other kind, and the kind-less read, + /// page FLAT rows exactly as 's own T377 shape already did. Both + /// halves clamp / through the SAME + /// floor 's own remarks describe. + /// + public Task ListWithMediaAsync( + RotKind? kind, RotState? state, int limit, int offset, CancellationToken ct) + { + (limit, offset) = ClampPaging(limit, offset); + + return kind == RotKind.NearDuplicate + ? ListNearDuplicateGroupPageAsync(state, limit, offset, ct) + : ListFlatPageAsync(kind, state, limit, offset, ct); + } + + /// + /// The kind-less read's T377 shape, verbatim (regression pin), plus every OTHER kind's own + /// kind-scoped flat paging (T385) — ordered kind, group_key nulls last, opened_at desc, id, + /// a group's own rows landing adjacent (id breaks ties for a + /// stable order across pages) even though this branch never runs for kind = near_duplicate + /// itself once a kind is given (that goes to + /// instead) — the ordering still matters for the kind-LESS call, where a near_duplicate group can + /// appear alongside every other kind. /// /// - /// Same callee-enforced floor 's own remarks - /// describe — / are floored here regardless of - /// what GardenerController's own endpoint clamp already did. + /// Kind-scoped ( non-null): is the + /// exact matching row count, read via ONE Dapper QueryMultipleAsync round trip carrying + /// the count(*) statement and the page select back to back (T385 review LOW-3 — same + /// connection, same snapshot-adjacent read, half the awaited round trips of two separate calls). + /// Still a genuinely SEPARATE statement, never a count(*) over() window (T377's own + /// rationale, unchanged by T385): a page whose lands past the end returns + /// ZERO rows, and a window function computed per-row would then carry no total at all — STORY-383 + /// AC3's own "total is exact on every page" demands a total that survives an empty page. /// /// /// - /// T377 review LOW-2: this order by is NOT index-covered — group_key sits mid-key - /// (between kind and opened_at), and no index on this table leads with it, so - /// Postgres sorts the whole FILTERED join result before applying LIMIT, rather than - /// walking an already-ordered index. The bound ('s own 1000-row cap) is - /// what keeps that sort cheap regardless of how large library.rot_finding grows — the - /// same reason 's own (kind, state, opened_at desc) index note - /// exists, this query just does not get to reuse it once group_key joins the sort key. + /// Kind-LESS ( null): NO count round trip at all (T385 review LOW-2). + /// here is (T386 review — the type itself + /// now says "not computed" rather than overloading 's own count + /// into that meaning), never a true matching total across every kind, and never read off the wire + /// for this shape (GardenerController's own T377-pinned response carries no count + /// field at all for a kind-less call, STORY-382 AC8). The kind-less caller has never needed an + /// exact cross-kind total, so the extra round trip bought nothing any reader actually consumes. /// /// - public async Task> ListWithMediaAsync( + async Task ListFlatPageAsync( RotKind? kind, RotState? state, int limit, int offset, CancellationToken ct) { - (limit, offset) = ClampPaging(limit, offset); - var conditions = new List(); var parameters = new DynamicParameters(); + AppendKindStateConditions(conditions, parameters, columnPrefix: "f.", kind, state); - if (kind is not null) - { - conditions.Add("f.kind = @kind::library.rot_kind"); - parameters.Add("kind", RotKindTokens.ToToken(kind.Value)); - } + var where = conditions.Count > 0 ? "where " + string.Join(" and ", conditions) : ""; - if (state is not null) + parameters.Add("limit", limit); + parameters.Add("offset", offset); + + var pageSql = $""" + select {FindingWithMediaSelectList} + from library.rot_finding f + {FindingWithMediaJoins} + {where} + order by f.kind, f.group_key nulls last, f.opened_at desc, f.id + limit @limit offset @offset + """; + + await using var conn = await dataSource.OpenConnectionAsync(ct); + + if (kind is null) { - conditions.Add("f.state = @state::library.rot_state"); - parameters.Add("state", RotStateTokens.ToToken(state.Value)); + var rows = await conn.QueryAsync(new CommandDefinition( + pageSql, parameters, cancellationToken: ct)); + var items = rows.Select(ToFindingWithMedia).ToList(); + return new RotFindingPage(items, null); } + var countSql = $"select count(*) from library.rot_finding f {where}"; + + await using var multi = await conn.QueryMultipleAsync(new CommandDefinition( + $"{countSql};\n{pageSql}", parameters, cancellationToken: ct)); + + var total = await multi.ReadSingleAsync(); + var pageRows = await multi.ReadAsync(); + + return new RotFindingPage(pageRows.Select(ToFindingWithMedia).ToList(), total); + } + + /// + /// 's own group-paged read (SPEC F153.9 rider 2026-08-31's own + /// binding contract; STORY-383; PLAN T385) — the PAGING UNIT is the GROUP, not the row: + /// matching_groups selects DISTINCT group_key for every near_duplicate group with at + /// least one member matching (see below), ordered ascending (ORDER IS + /// SEMANTICS — group_key asc is what keeps page 2 disjoint from page 1 regardless of write + /// activity between the two calls), and / apply to + /// THAT distinct set. The outer select then joins back to EVERY rot_finding row sharing one + /// of the selected group_keys — every member row of every selected group, never a partial one, + /// exactly STORY-383 AC1/AC4's own contract. + /// + /// + /// RULED (T385 review HIGH-1, proven live): scopes which GROUPS + /// qualify, never which MEMBER rows render. A group qualifies the moment ANY ONE of its members + /// matches kind = near_duplicate + ; once a group qualifies, EVERY + /// member row of it renders regardless of that member's OWN state — SPEC F153.9 rider's own binding + /// text, "the response returns every member row of every selected group", is unconditional. A + /// group where NO member matches (e.g. every member dismissed, under + /// state=open) still correctly does not qualify — it never enters matching_groups, so + /// it consumes no page slot and does not count into . The member + /// join filters f.kind = 'near_duplicate' — kept explicit even though a group_key + /// value is written solely by 's own insert, so in + /// practice it is redundant with the join key alone — NEVER : an earlier + /// build also repeated the state filter on the member join, which truncated a mixed-state group + /// down to only its matching members (a live 3-member group with one dismissed rendered 2 rows + /// under state=open) — exactly the split-cluster bug this whole read exists to prevent. + /// + /// + /// + /// RULED (round-2 review HIGH-2): the member join ALSO excludes a RESOLVED row outright, + /// regardless of . The near-duplicate resolve half + /// () never clears a row's own group_key when it + /// resolves it — so a member that left find_near_duplicates on its own (an operator retagged + /// it; it is genuinely no longer a duplicate, still eligible, still in rotation) keeps rendering + /// inside its OLD group forever without this exclusion, and the UI's Keep-this-one bulk write would + /// then pull that distinct, in-rotation track out of rotation by mistake. dismissed = the + /// operator closed the finding while the media is still a duplicate → render; resolved = the system + /// closed it because the media is no longer a duplicate → don't render. A group with e.g. two + /// open members and one resolved member still qualifies (its two still-duplicate members are what + /// render) exactly as before. + /// + /// + /// + /// RULED (round-3 review MED-6, fix verified live): matching_groups' own qualification + /// ALSO excludes a RESOLVED row — state <> 'resolved'::library.rot_state sits + /// beside group_key is not null in the same list, so + /// qualification and member rendering agree about resolved. Without it, a FULLY-resolved group + /// (every member resolved, group_key still intact — permanently, since resolve never clears + /// it) would still enter matching_groups whenever is + /// (the endpoint's documented "any" default) — consuming a page slot and a + /// count while rendering ZERO member rows, a phantom that never + /// clears itself. =open stays byte-identical (an open-scoped + /// qualification already excludes resolved rows); =resolved now + /// self-consistently returns an empty page ( 0, zero rows) rather + /// than a Total with no matching member rows behind it — resolved near-duplicate groups are not + /// browsable as clusters. + /// + /// + /// + /// matching_groups' own where also requires group_key is not null (T385 review + /// LOW-1): select distinct and count(distinct) both treat SQL NULL as a distinct-able + /// value, so a phantom NULL group_key row could otherwise make the two aggregates disagree + /// (proven live: 4 slots vs. a total of 3) even though no genuine + /// finding carries one today. + /// + /// + /// + /// is count(distinct group_key) over the SAME filter + /// matching_groups uses, read via ONE Dapper QueryMultipleAsync round trip alongside + /// the group+member select (T385 review LOW-3 — same rationale as 's + /// own remarks: a genuinely separate statement, not a window function, so an + /// past the last group still returns the true total over an empty page, STORY-383 AC3). + /// + /// + async Task ListNearDuplicateGroupPageAsync( + RotState? state, int limit, int offset, CancellationToken ct) + { + var conditions = new List(); + var parameters = new DynamicParameters(); + AppendKindStateConditions(conditions, parameters, columnPrefix: "", RotKind.NearDuplicate, state); + conditions.Add("group_key is not null"); + // MED-6 (round 3): qualification agrees with the member join below about resolved — a group + // qualifies on its NON-resolved members only, so a fully-resolved group (every member + // resolved, group_key still intact) never consumes a page slot or a Total count while + // rendering zero rows. + conditions.Add("state <> 'resolved'::library.rot_state"); + + var where = "where " + string.Join(" and ", conditions); + + // HIGH-1: the member join scopes to the kind only — state is what decides which GROUPS + // qualify (above), never which member rows of a qualifying group render. HIGH-2 (round 2): + // still excludes a RESOLVED member outright — the resolve half never clears group_key, so a + // member that left find_near_duplicates on its own (no longer a duplicate, still eligible, + // still in rotation) must not render inside its old group. dismissed = the operator closed + // the finding while the media is still a duplicate → render; resolved = the system closed it + // because the media is no longer a duplicate → don't render. + var memberConditions = new List(); + AppendKindStateConditions(memberConditions, parameters, columnPrefix: "f.", RotKind.NearDuplicate, state: null); + memberConditions.Add("f.state <> 'resolved'::library.rot_state"); + var memberWhere = string.Join(" and ", memberConditions); + parameters.Add("limit", limit); parameters.Add("offset", offset); - var where = conditions.Count > 0 ? "where " + string.Join(" and ", conditions) : ""; + var countSql = $"select count(distinct group_key) from library.rot_finding {where}"; + var pageSql = $""" + with matching_groups as materialized ( + select distinct group_key + from library.rot_finding + {where} + order by group_key + limit @limit offset @offset + ) + select {FindingWithMediaSelectList} + from matching_groups mg + join library.rot_finding f + on f.group_key = mg.group_key and {memberWhere} + {FindingWithMediaJoins} + order by mg.group_key, f.opened_at desc, f.id + """; await using var conn = await dataSource.OpenConnectionAsync(ct); - var rows = await conn.QueryAsync(new CommandDefinition( - $""" - select - f.id, f.media_id, f.kind::text as kind, f.state::text as state, f.group_key, - f.evidence::text as evidence, f.opened_at, f.resolved_at, f.dismissed_at, f.updated_at, - m.path as locator, m.title, m.artist, m.duration_ms, - coalesce(rot.play_count, 0) as plays, r.score as rating, - coalesce(r.never_play, false) as never_play, m.eligible - from library.rot_finding f - join library.media m on m.id = f.media_id - left join library.media_rotation rot on rot.media_id = m.id - left join library.media_rating r on r.media_id = m.id - {where} - order by f.kind, f.group_key nulls last, f.opened_at desc, f.id - limit @limit offset @offset - """, - parameters, - cancellationToken: ct)); + await using var multi = await conn.QueryMultipleAsync(new CommandDefinition( + $"{countSql};\n{pageSql}", parameters, cancellationToken: ct)); + + var totalGroups = await multi.ReadSingleAsync(); + var rows = await multi.ReadAsync(); - return rows.Select(ToFindingWithMedia).ToList(); + return new RotFindingPage(rows.Select(ToFindingWithMedia).ToList(), totalGroups); } /// @@ -812,6 +1005,20 @@ limit @limit offset @offset /// can never drift apart on the bound: to at least 1, capped at 1000 /// (the LOW-2 finding's own figure); to at least 0 (a negative value /// errors in Postgres's own OFFSET clause rather than clamping there). + /// + /// + /// T385 review MED-4: the cap counts ROWS everywhere except , + /// where it counts GROUPS. For every other kind (and the kind-less read), 1000 is still the true + /// row ceiling. For , bounds + /// the number of DISTINCT group_keys selected — the ROW envelope for that page is at most + /// 1000 groups × each group's own member count, not 1000 rows outright (a per-row cap would force a + /// partial group onto a page, which STORY-383's own whole-cluster contract forbids). In practice + /// this envelope stays small: a near-duplicate group is 2–5 members (this file's own + /// find_near_duplicates callers never see larger clusters in a real library), and + /// GardenerController's own admin UI caps the size picker at 250 groups/page — so the + /// realistic near-duplicate page tops out in the low thousands of rows, never the unbounded shape a + /// naive "1000 rows always" reading would suggest. + /// /// static (int Limit, int Offset) ClampPaging(int limit, int offset) => (limit <= 0 ? 1 : Math.Min(limit, 1000), Math.Max(0, offset)); diff --git a/tests/GenWave.Host.Tests/FakeRotFindingStore.cs b/tests/GenWave.Host.Tests/FakeRotFindingStore.cs index 6fe4ef06..3bbf20c9 100644 --- a/tests/GenWave.Host.Tests/FakeRotFindingStore.cs +++ b/tests/GenWave.Host.Tests/FakeRotFindingStore.cs @@ -47,7 +47,7 @@ public Task> ListAsync( RotKind? kind, RotState? state, CancellationToken ct, int limit = 200, int offset = 0) => throw new NotSupportedException("unused by this double's current callers"); - public Task> ListWithMediaAsync( + public Task ListWithMediaAsync( RotKind? kind, RotState? state, int limit, int offset, CancellationToken ct) => throw new NotSupportedException("unused by this double's current callers"); } diff --git a/tests/GenWave.Host.Tests/Specs/Story382_KindScopedPagingOnTheWire.cs b/tests/GenWave.Host.Tests/Specs/Story382_KindScopedPagingOnTheWire.cs index 7bde31b2..04f88da6 100644 --- a/tests/GenWave.Host.Tests/Specs/Story382_KindScopedPagingOnTheWire.cs +++ b/tests/GenWave.Host.Tests/Specs/Story382_KindScopedPagingOnTheWire.cs @@ -3,8 +3,9 @@ // BDD specification — xUnit through the deployed entry point (WebApplicationFactory // against a real ephemeral Postgres — the Story374/Story378 arc idiom): these facts drive // GET /api/gardener/findings over HTTP with an authed admin session, never the repository -// directly. Specs are Skip-pinned until T386 wires the controller; /build-loop fills the -// bodies and removes the Skip. +// directly. One arc (KindScopedPagingArc) arranges everything every Scenario below reads — +// the SAME "arrange once, many read-only Scenarios" idiom GardenerFindingsCollection already +// establishes in Story374_TheGardenerTendsAQueue.cs. // // Under spec: a kind=-scoped response gains `total` (groups for near_duplicate, rows otherwise); // the near-duplicate path routes through T385's group-paged read; a call WITHOUT kind= stays @@ -12,6 +13,17 @@ // 400/clamp posture is T377's and is not re-pinned here. STORY-383 AC1–AC3's wire half lives // here; their store half is MediaLibrary.Tests Story383_DuplicateClustersNeverSplit.cs. +using System.Net; +using System.Net.Http.Json; +using System.Text.Json; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; +using GenWave.Host.Tests.Support; + namespace GenWave.Host.Tests.Specs; public static class FeatureKindScopedPagingOnTheWire @@ -21,45 +33,63 @@ public static class FeatureKindScopedPagingOnTheWire // --------------------------------------------------------------------- /// STORY-382 AC6 — a flat kind, scoped: 60 open dead_file findings seeded. - public sealed class ScenarioKindScopedResponseCarriesTotal + [Collection(KindScopedPagingCollection.Name)] + public sealed class ScenarioKindScopedResponseCarriesTotal(KindScopedPagingArc arc) { - [Fact(Skip = "Pending T386 — see docs/PLAN.md")] + [Fact] public void TotalIsTheExactOpenRowCountForTheKind() { // GET /api/gardener/findings?kind=dead_file&state=open&limit=25 → body.total == 60. - Assert.Fail("pending T386"); + Assert.Equal(60, arc.KindScopedTotal); } - [Fact(Skip = "Pending T386 — see docs/PLAN.md")] + [Fact] public void ThePageCarriesLimitRowsOfThatKindOnly() { // 25 findings, every group.kind == dead_file. - Assert.Fail("pending T386"); + Assert.Equal((25, "dead_file"), (arc.KindScopedFindingsCount, arc.KindScopedGroupKind)); + } + + [Fact] + public void TheResponseCarriesATotalProperty() + { + // Presence pin (LOW-3) — without this, a missing "total" only surfaces as a fixture + // arrangement exception (GetProperty throwing in InitializeAsync) shared across every + // fact in this file, never as a named failure of its own. + Assert.True(arc.KindScopedHasTotalProperty); } } /// STORY-383 AC1–AC3 on the wire — 30 seeded duplicate groups of 2–4 members. - public sealed class ScenarioNearDuplicatesPageByGroupOnTheWire + [Collection(KindScopedPagingCollection.Name)] + public sealed class ScenarioNearDuplicatesPageByGroupOnTheWire(KindScopedPagingArc arc) { - [Fact(Skip = "Pending T386 — see docs/PLAN.md")] + [Fact] public void LimitSelectsWholeGroupsNeverPartialOnes() { - // ?kind=near_duplicate&limit=25 → 25 duplicateGroups, each with ALL its members. - Assert.Fail("pending T386"); + // ?kind=near_duplicate&limit=25 → 25 duplicateGroups, each with ALL its members — the + // Story383 EveryReturnedGroupIsWhole shape (LOW-2): expected vs. actual member-count + // SEQUENCES, in group_key order, compared in one shot, so a failure names exactly which + // page-one group broke rather than collapsing every group into one precomputed bool. + Assert.Equal(arc.NearDupPage1ExpectedMemberSizes, arc.NearDupPage1ActualMemberSizes); } - [Fact(Skip = "Pending T386 — see docs/PLAN.md")] + [Fact] public void OffsetContinuesAtTheNextGroup() { - // ?offset=25 → the remaining 5 groups, disjoint from page one's groupKeys. - Assert.Fail("pending T386"); + // ?offset=25 → the remaining 5 groups, disjoint from page one's groupKeys — named counts + // (page count, intersection count) via a genuine set comparison (LOW-2, the Story383 + // SharesNoGroupKeyWithPageOne shape), not a precomputed bool. + Assert.Equal((5, 0), (arc.NearDupPage2Keys.Count, arc.NearDupPage2Keys.Intersect(arc.NearDupPage1Keys).Count())); } - [Fact(Skip = "Pending T386 — see docs/PLAN.md")] + [Fact] public void TotalCountsGroupsNotRows() { - // body.total == 30 while /api/status.gardener.open.nearDuplicate keeps the ROW count. - Assert.Fail("pending T386"); + // body.total == 30 (groups) while /api/status.gardener.open.nearDuplicate == 90 (rows) — + // named values (LOW-1), so a status regression (e.g. to 45) can no longer stay green + // behind a bare inequality (30 != 45 would still have been "true"). + Assert.Equal((30, 90), (arc.NearDupTotal, arc.StatusOpenNearDuplicateRowCount)); } } @@ -68,21 +98,241 @@ public void TotalCountsGroupsNotRows() // --------------------------------------------------------------------- /// STORY-382 AC8 — the T377 contract for un-scoped callers stands verbatim. - public sealed class ScenarioTheUnscopedCallKeepsTheT377Shape + [Collection(KindScopedPagingCollection.Name)] + public sealed class ScenarioTheUnscopedCallKeepsTheT377Shape(KindScopedPagingArc arc) { - [Fact(Skip = "Pending T386 — see docs/PLAN.md")] + [Fact] public void CarriesNoTotalProperty() { // GET /api/gardener/findings?state=open → the JSON body has no "total" member at all. - Assert.Fail("pending T386"); + Assert.False(arc.UnscopedHasTotalProperty); + } + + [Fact] + public void TheResponseHasExactlyOneTopLevelProperty() + { + // LOW-4 — the stronger sibling of CarriesNoTotalProperty: the root object's own property + // SET is exactly ["groups"], pinning byte-compatibility with T377 against "total" AND any + // future stray top-level member, not just the one named property. + Assert.Equal(["groups"], arc.UnscopedPropertyNames); } - [Fact(Skip = "Pending T386 — see docs/PLAN.md")] + [Fact] public void PagesFlatAcrossKinds() { // Seed enough dead_file rows to fill the page: the near_duplicate group is absent — // the gh-#654 behavior, correct for THIS un-scoped shape and pinned as such. - Assert.Fail("pending T386"); + Assert.DoesNotContain("near_duplicate", arc.UnscopedFloodKinds); + } + } +} + +// ── Collection definition — one ephemeral Postgres/factory shared by every Scenario above (the +// Story374 "arrange once, many read-only Scenarios" idiom, via ICollectionFixture). ── + +[CollectionDefinition(Name)] +public sealed class KindScopedPagingCollection : ICollectionFixture +{ + public const string Name = "Story382KindScopedPaging"; +} + +/// +/// Seeds 60 open dead_file findings and 30 open near_duplicate groups (2–4 members +/// each, 90 rows total) directly via raw SQL ( — the SAME +/// "independent read of what actually landed" posture Story374's own fixtures already establish; +/// never through a reconcile pass), then drives every query this file's Scenarios need over the +/// REAL production HTTP pipeline with a real admin session — the SAME +/// -subclass idiom Story378_KeepThisOneBulkEligibility.cs +/// already uses. The 60 dead_file rows do double duty: they prove an exact kind-scoped total +/// (STORY-382 AC6) AND, reused with a smaller limit, flood an un-scoped page past every +/// near_duplicate row (STORY-382 AC8's own gh-#654 regression pin) — one arrangement, no second +/// seed needed. Group keys are zero-padded (grp-01..grp-30) so lexicographic +/// group_key asc ordering matches numeric order, making page one/page two's own group +/// split deterministic. +/// +public sealed class KindScopedPagingArc : IAsyncLifetime +{ + const int DeadFileCount = 60; + const int NearDuplicateGroupCount = 30; + + public bool KindScopedHasTotalProperty { get; private set; } + public int KindScopedTotal { get; private set; } + public int KindScopedFindingsCount { get; private set; } + public string KindScopedGroupKind { get; private set; } = ""; + + /// LOW-2 — the seeded (expected) and observed (actual) per-group member counts for + /// page one, BOTH in group_key ascending order, so over + /// the two sequences names exactly which group (by position) broke, the Story383 + /// EveryReturnedGroupIsWhole shape — never a single precomputed bool collapsing every group's own + /// comparison into one opaque pass/fail. + public IReadOnlyList NearDupPage1ExpectedMemberSizes { get; private set; } = []; + public IReadOnlyList NearDupPage1ActualMemberSizes { get; private set; } = []; + + /// LOW-2 — the raw group_key sets for page one/page two, so the disjointness fact does + /// its own set comparison (Intersect) rather than reading a precomputed bool. + public IReadOnlySet NearDupPage1Keys { get; private set; } = new HashSet(); + public IReadOnlySet NearDupPage2Keys { get; private set; } = new HashSet(); + + public int NearDupTotal { get; private set; } + public int StatusOpenNearDuplicateRowCount { get; private set; } + + public bool UnscopedHasTotalProperty { get; private set; } + public IReadOnlyList UnscopedPropertyNames { get; private set; } = []; + public IReadOnlyList UnscopedFloodKinds { get; private set; } = []; + + public async Task InitializeAsync() + { + // A LOCAL, not a field — Story382KindScopedPagingDatabase is file-local (CS9051), the same + // reason Story374's/Story378's own arcs give for the identical shape. + await using var database = await Story382KindScopedPagingDatabase.StartAsync(); + + for (var i = 1; i <= DeadFileCount; i++) + { + var mediaId = await GardenerRotFixtures.InsertPlayableMediaRowAsync( + database.LibraryConnectionString, $"/test/t386-dead-{i:D2}.flac", 200_000, $"Dead Song {i}", "Artist Dead"); + await GardenerRotFixtures.InsertFindingAsync( + database.LibraryConnectionString, mediaId, "dead_file", "open", null, "{}"); + } + + var seededMemberCounts = new Dictionary(); + for (var g = 1; g <= NearDuplicateGroupCount; g++) + { + var groupKey = $"grp-{g:D2}"; + var memberCount = 2 + (g - 1) % 3; + seededMemberCounts[groupKey] = memberCount; + + for (var m = 1; m <= memberCount; m++) + { + var mediaId = await GardenerRotFixtures.InsertPlayableMediaRowAsync( + database.LibraryConnectionString, $"/test/t386-dup-{g:D2}-{m}.flac", 200_000 + m * 1_000, + $"Dup Song {g}", "Artist Dup"); + await GardenerRotFixtures.InsertFindingAsync( + database.LibraryConnectionString, mediaId, "near_duplicate", "open", groupKey, "{}"); + } } + + await using var factory = new Story382WebFactory(database); + var client = factory.CreateClient(); + var login = await client.PostAsJsonAsync( + "/api/auth/login", new { password = Story382WebFactory.Password }); + if (login.StatusCode != HttpStatusCode.NoContent) + throw new InvalidOperationException($"login unexpectedly returned {login.StatusCode}"); + + // STORY-382 AC6 — a flat kind, scoped. TryGetProperty first (LOW-3) so a missing "total" + // fails ONLY TheResponseCarriesATotalProperty, never crashes the whole arrangement for every + // other fact in this file. + var kindScoped = await client.GetAsync("/api/gardener/findings?kind=dead_file&state=open&limit=25"); + var kindScopedRoot = JsonDocument.Parse(await kindScoped.Content.ReadAsStringAsync()).RootElement; + KindScopedHasTotalProperty = kindScopedRoot.TryGetProperty("total", out var kindScopedTotalProperty); + KindScopedTotal = KindScopedHasTotalProperty ? kindScopedTotalProperty.GetInt32() : -1; + var kindScopedGroup = kindScopedRoot.GetProperty("groups").EnumerateArray().Single(); + KindScopedGroupKind = kindScopedGroup.GetProperty("kind").GetString() ?? ""; + KindScopedFindingsCount = kindScopedGroup.GetProperty("findings").GetArrayLength(); + + // STORY-383 AC1–AC3 on the wire — near-duplicate group paging, page one, both sides of the + // LOW-2 sequence comparison built in group_key ascending order (matching the store's own + // paging order, Garden.RotFindingRepository.ListNearDuplicateGroupPageAsync). + var nearDupPage1 = await client.GetAsync("/api/gardener/findings?kind=near_duplicate&limit=25"); + var nearDupPage1Root = JsonDocument.Parse(await nearDupPage1.Content.ReadAsStringAsync()).RootElement; + NearDupTotal = nearDupPage1Root.GetProperty("total").GetInt32(); + var page1Groups = nearDupPage1Root.GetProperty("groups").EnumerateArray().Single() + .GetProperty("duplicateGroups").EnumerateArray() + .OrderBy(duplicateGroup => duplicateGroup.GetProperty("groupKey").GetString(), StringComparer.Ordinal) + .ToList(); + // A FIXED Take(25) (the query's own limit), never page1Groups.Count — the whole point of a + // named expected sequence is to catch a wrong PAGE SIZE too, not just wrong per-group sizes; + // sizing "expected" off the actual response would silently agree with a truncated page. + NearDupPage1ExpectedMemberSizes = seededMemberCounts + .OrderBy(seeded => seeded.Key, StringComparer.Ordinal) + .Take(25) + .Select(seeded => seeded.Value) + .ToArray(); + NearDupPage1ActualMemberSizes = page1Groups + .Select(duplicateGroup => duplicateGroup.GetProperty("members").GetArrayLength()) + .ToArray(); + NearDupPage1Keys = page1Groups.Select(duplicateGroup => duplicateGroup.GetProperty("groupKey").GetString() ?? "").ToHashSet(); + + // Page two — offset continues at the next group. + var nearDupPage2 = await client.GetAsync("/api/gardener/findings?kind=near_duplicate&limit=25&offset=25"); + var nearDupPage2Root = JsonDocument.Parse(await nearDupPage2.Content.ReadAsStringAsync()).RootElement; + NearDupPage2Keys = nearDupPage2Root.GetProperty("groups").EnumerateArray().Single() + .GetProperty("duplicateGroups").EnumerateArray() + .Select(duplicateGroup => duplicateGroup.GetProperty("groupKey").GetString() ?? "") + .ToHashSet(); + + var status = await client.GetAsync("/api/status"); + var statusRoot = JsonDocument.Parse(await status.Content.ReadAsStringAsync()).RootElement; + StatusOpenNearDuplicateRowCount = statusRoot.GetProperty("gardener").GetProperty("open").GetProperty("nearDuplicate").GetInt32(); + + // STORY-382 AC8 — the un-scoped call stays byte-compatible with T377's pinned shape. + var unscoped = await client.GetAsync("/api/gardener/findings?state=open"); + var unscopedRoot = JsonDocument.Parse(await unscoped.Content.ReadAsStringAsync()).RootElement; + UnscopedHasTotalProperty = unscopedRoot.TryGetProperty("total", out _); + UnscopedPropertyNames = unscopedRoot.EnumerateObject().Select(property => property.Name).ToList(); + + // A limit well under the 60 seeded dead_file rows (which sort first, kind before + // near_duplicate in the library.rot_kind enum's own declaration order) — the page fills on + // dead_file alone, so no near_duplicate group ever reaches it (gh-#654's own regression). + var unscopedFlood = await client.GetAsync($"/api/gardener/findings?state=open&limit={DeadFileCount - 10}"); + var unscopedFloodRoot = JsonDocument.Parse(await unscopedFlood.Content.ReadAsStringAsync()).RootElement; + UnscopedFloodKinds = unscopedFloodRoot.GetProperty("groups").EnumerateArray() + .Select(group => group.GetProperty("kind").GetString() ?? "") + .ToList(); + } + + public Task DisposeAsync() => Task.CompletedTask; +} + +// ── Test harness — WebApplicationFactory + ephemeral Postgres subclasses (Story374's/Story378's +// own idiom; `file`-scoped types cannot cross files, so this file supplies its own, exactly as +// Story378's own remarks on EphemeralStationDatabase explain). ── + +/// +/// Boots the real production composition root against a real ephemeral Postgres with every hosted +/// service removed (no gardener/rotation/liquidsoap background loop reach) — this arc only needs +/// the real GardenerController endpoints and StatusController over a real admin +/// session. +/// +file sealed class Story382WebFactory(Story382KindScopedPagingDatabase db) : WebApplicationFactory +{ + public const string Password = "test-password-t386-kind-scoped-paging"; + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + builder.UseEnvironment("Development"); + builder.UseSetting("ConnectionStrings:Library", db.LibraryConnectionString); + builder.UseSetting("ConnectionStrings:Station", db.StationConnectionString); + builder.UseSetting("Admin:Password", Password); + builder.UseSetting("Station:Id", "genwave-1"); + builder.UseSetting("Station:Name", "GWAV 108.8"); + builder.UseSetting("Station:Voice", "af_heart"); + builder.UseSetting("Station:Scope:LibraryIds:0", "1"); + + builder.ConfigureTestServices(services => + { + services.RemoveAll(); + }); + } +} + +/// +/// This file's own thin subclass of the shared harness — see +/// that type's own remarks for the full "which compose file, why a unique project name + OS-assigned +/// port" rationale. Supplies only the "genwave-t386" compose project-name prefix this file's +/// own arc needs. +/// +file sealed class Story382KindScopedPagingDatabase : EphemeralStationDatabase +{ + Story382KindScopedPagingDatabase(string project, string composeFile, string libraryConnectionString, string stationConnectionString) + : base(project, composeFile, libraryConnectionString, stationConnectionString) + { + } + + public static async Task StartAsync() + { + var (project, composeFile, library, station) = Provision("genwave-t386"); + var db = new Story382KindScopedPagingDatabase(project, composeFile, library, station); + await db.WaitForSchemaAsync(); + return db; } } diff --git a/tests/GenWave.MediaLibrary.Tests/Fakes/RecordingRotFindingStore.cs b/tests/GenWave.MediaLibrary.Tests/Fakes/RecordingRotFindingStore.cs index 2cf6e01a..ac5310c4 100644 --- a/tests/GenWave.MediaLibrary.Tests/Fakes/RecordingRotFindingStore.cs +++ b/tests/GenWave.MediaLibrary.Tests/Fakes/RecordingRotFindingStore.cs @@ -42,7 +42,7 @@ public Task> ListAsync( RotKind? kind, RotState? state, CancellationToken ct, int limit = 200, int offset = 0) => throw new NotSupportedException("RecordingRotFindingStore only records ReconcileUnreachableAsync."); - public Task> ListWithMediaAsync( + public Task ListWithMediaAsync( RotKind? kind, RotState? state, int limit, int offset, CancellationToken ct) => throw new NotSupportedException("RecordingRotFindingStore only records ReconcileUnreachableAsync."); diff --git a/tests/GenWave.MediaLibrary.Tests/Specs/Story374_TheGardenerFindingsJoinMedia.cs b/tests/GenWave.MediaLibrary.Tests/Specs/Story374_TheGardenerFindingsJoinMedia.cs index a50cb8da..4bcd6c6d 100644 --- a/tests/GenWave.MediaLibrary.Tests/Specs/Story374_TheGardenerFindingsJoinMedia.cs +++ b/tests/GenWave.MediaLibrary.Tests/Specs/Story374_TheGardenerFindingsJoinMedia.cs @@ -75,7 +75,7 @@ public async Task ARowWithALedgerAndRatingCarriesTheLedgerPlayCount() await InsertRatingAsync(db, mediaId, score: 80); await InsertFindingAsync(db, mediaId, "dead_file"); - var rows = await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None); + var rows = (await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None)).Items; Assert.Equal(3, Assert.Single(rows).Plays); } @@ -89,7 +89,7 @@ public async Task ARowWithALedgerAndRatingCarriesTheRatingScore() await InsertRatingAsync(db, mediaId, score: 80); await InsertFindingAsync(db, mediaId, "dead_file"); - var rows = await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None); + var rows = (await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None)).Items; Assert.Equal(80, Assert.Single(rows).Rating); } @@ -104,7 +104,7 @@ public async Task ARowWithNeitherDefaultsPlaysToZero() var mediaId = await InsertReadyRowAsync(db, "/gardener/t377-join-b.flac", "Artist", "Song B", 200_000); await InsertFindingAsync(db, mediaId, "dead_file"); - var rows = await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None); + var rows = (await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None)).Items; Assert.Equal(0, Assert.Single(rows).Plays); } @@ -116,7 +116,7 @@ public async Task ARowWithNeitherLeavesRatingNull() var mediaId = await InsertReadyRowAsync(db, "/gardener/t377-join-b2.flac", "Artist", "Song B2", 200_000); await InsertFindingAsync(db, mediaId, "dead_file"); - var rows = await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None); + var rows = (await Repo(db).ListWithMediaAsync(RotKind.DeadFile, RotState.Open, 200, 0, CancellationToken.None)).Items; Assert.Null(Assert.Single(rows).Rating); } @@ -137,7 +137,7 @@ public async Task LimitZeroReturnsExactlyOneRow() await InsertFindingAsync(db, firstId, "dead_file"); await InsertFindingAsync(db, secondId, "stale_metadata"); - var rows = await Repo(db).ListWithMediaAsync(null, RotState.Open, limit: 0, offset: 0, ct: CancellationToken.None); + var rows = (await Repo(db).ListWithMediaAsync(null, RotState.Open, limit: 0, offset: 0, ct: CancellationToken.None)).Items; Assert.Single(rows); } @@ -152,7 +152,7 @@ public async Task NegativeOffsetDoesNotThrow() var mediaId = await InsertReadyRowAsync(db, "/gardener/t377-floor-c.flac", "Artist", "Song C", 200_000); await InsertFindingAsync(db, mediaId, "dead_file"); - var rows = await Repo(db).ListWithMediaAsync(null, RotState.Open, limit: 200, offset: -1, ct: CancellationToken.None); + var rows = (await Repo(db).ListWithMediaAsync(null, RotState.Open, limit: 200, offset: -1, ct: CancellationToken.None)).Items; Assert.Single(rows); } diff --git a/tests/GenWave.MediaLibrary.Tests/Specs/Story383_DuplicateClustersNeverSplit.cs b/tests/GenWave.MediaLibrary.Tests/Specs/Story383_DuplicateClustersNeverSplit.cs index 19649e72..83291587 100644 --- a/tests/GenWave.MediaLibrary.Tests/Specs/Story383_DuplicateClustersNeverSplit.cs +++ b/tests/GenWave.MediaLibrary.Tests/Specs/Story383_DuplicateClustersNeverSplit.cs @@ -2,8 +2,10 @@ // // BDD specification — xUnit, REAL Postgres via DatabaseFixture (the Story376 posture: these facts // drive the store's SQL against a live database, never a mock — the T362 loop law says every new -// SQL read gets a Postgres-backed fact). Specs are Skip-pinned until T385 wires the group-paged -// read; /build-loop fills the bodies and removes the Skip. +// SQL read gets a Postgres-backed fact). WIRED at T385 — every near_duplicate finding here is seeded +// DIRECTLY into library.rot_finding (never through ReconcileNearDuplicatesAsync): the file's own +// original guidance, since these facts target the READ's own group-paging shape, not the reconcile +// pass's own duplicate-detection SQL. // // Under spec: the kind-scoped joined read on IRotFindingStore/RotFindingRepository returns // (rows, total) where, for kind=near_duplicate, limit/offset count DISTINCT group_keys (ordered @@ -12,83 +14,508 @@ // keeps its current shape verbatim (regression pin). STORY-382 AC6 (total is exact) is pinned // here at the store; its wire half lives in Host.Tests Story382_KindScopedPagingOnTheWire.cs. +using Dapper; +using GenWave.Core.Domain; +using GenWave.MediaLibrary.Garden; + namespace GenWave.MediaLibrary.Tests.Specs; public static class FeatureDuplicateClustersNeverSplit { + // --------------------------------------------------------------------- + // Helpers + // --------------------------------------------------------------------- + + static RotFindingRepository Repo(DatabaseFixture db) => new(db.DataSource); + + /// A minimal media row — the FK target every rot_finding row needs. + /// ListWithMediaAsync's own join surfaces its columns, but no fact in this file asserts on + /// them, so no tag/duration/state is worth seeding here. + static async Task InsertMediaRowAsync(DatabaseFixture db, string path) + { + await using var conn = await db.DataSource.OpenConnectionAsync(); + return await conn.ExecuteScalarAsync( + "insert into library.media (path, format, size_bytes, mtime) values (@path, 'flac', 1024, now()) returning id", + new { path }); + } + + /// One OPEN near_duplicate finding, seeded directly (this file's own header + /// explains why) — isolates the group-paged READ under test from + /// 's own SQL entirely. + static async Task InsertNearDuplicateFindingAsync(DatabaseFixture db, long mediaId, string groupKey) + { + await using var conn = await db.DataSource.OpenConnectionAsync(); + return await conn.ExecuteScalarAsync( + """ + insert into library.rot_finding (media_id, kind, state, group_key, evidence) + values (@mediaId, 'near_duplicate'::library.rot_kind, 'open', @groupKey, '{}') + returning id + """, + new { mediaId, groupKey }); + } + + /// One RESOLVED near_duplicate finding with its left + /// intact — mirrors what ReconcileNearDuplicatesAsync's own resolve half actually leaves + /// behind (it flips state/resolved_at/updated_at only, never group_key): + /// a member that stopped being a duplicate on its own, still sitting inside its old group's row. + /// + static async Task InsertResolvedNearDuplicateFindingAsync(DatabaseFixture db, long mediaId, string groupKey) + { + await using var conn = await db.DataSource.OpenConnectionAsync(); + return await conn.ExecuteScalarAsync( + """ + insert into library.rot_finding (media_id, kind, state, group_key, evidence, resolved_at) + values (@mediaId, 'near_duplicate'::library.rot_kind, 'resolved', @groupKey, '{}', now()) + returning id + """, + new { mediaId, groupKey }); + } + + /// One OPEN dead_file finding with an explicit — the + /// flat kind-scoped fact () needs a DETERMINISTIC + /// opened_at desc order regardless of wall-clock jitter between inserts. + static async Task InsertDeadFileFindingAsync(DatabaseFixture db, long mediaId, DateTimeOffset openedAt) + { + await using var conn = await db.DataSource.OpenConnectionAsync(); + return await conn.ExecuteScalarAsync( + """ + insert into library.rot_finding (media_id, kind, state, evidence, opened_at) + values (@mediaId, 'dead_file'::library.rot_kind, 'open', '{}', @openedAt) + returning id + """, + new { mediaId, openedAt }); + } + + static async Task> ReadFindingIdsForGroupAsync(DatabaseFixture db, string groupKey) + { + await using var conn = await db.DataSource.OpenConnectionAsync(); + var ids = await conn.QueryAsync( + "select id from library.rot_finding where kind = 'near_duplicate'::library.rot_kind and group_key = @groupKey", + new { groupKey }); + return ids.ToList(); + } + + /// One OPEN near_duplicate finding with an explicit — + /// the MED-5 ordering fact needs a DETERMINISTIC opened_at desc sequence within one group, + /// regardless of wall-clock jitter between inserts. + static async Task InsertNearDuplicateFindingWithOpenedAtAsync( + DatabaseFixture db, long mediaId, string groupKey, DateTimeOffset openedAt) + { + await using var conn = await db.DataSource.OpenConnectionAsync(); + return await conn.ExecuteScalarAsync( + """ + insert into library.rot_finding (media_id, kind, state, group_key, evidence, opened_at) + values (@mediaId, 'near_duplicate'::library.rot_kind, 'open', @groupKey, '{}', @openedAt) + returning id + """, + new { mediaId, groupKey, openedAt }); + } + + /// ONE near_duplicate group with exactly OPEN member rows, + /// returned in insertion order — the HIGH-1 facts need a fixed size (3 members) rather than + /// 's own 2/3/4-cycling sizes, so they can dismiss a known subset. + /// + static async Task> SeedOneGroupAsync(DatabaseFixture db, string groupKey, int memberCount) + { + var ids = new List(); + for (var m = 0; m < memberCount; m++) + { + var mediaId = await InsertMediaRowAsync(db, $"/t385/{groupKey}-{m}.flac"); + ids.Add(await InsertNearDuplicateFindingAsync(db, mediaId, groupKey)); + } + + return ids; + } + + /// near_duplicate groups ("grp-00".."grp-NN", zero-padded + /// so group_key's own text-ascending order matches numeric order), sizes cycling 2/3/4 + /// members (STORY-383 AC1's own "2–4 members each") — every member row OPEN, none dismissed. + /// + static async Task> SeedGroupsAsync(DatabaseFixture db, int groupCount) + { + var groups = new List<(string GroupKey, int MemberCount)>(); + for (var g = 0; g < groupCount; g++) + { + var groupKey = $"grp-{g:D2}"; + var memberCount = 2 + g % 3; + for (var m = 0; m < memberCount; m++) + { + var mediaId = await InsertMediaRowAsync(db, $"/t385/{groupKey}-{m}.flac"); + await InsertNearDuplicateFindingAsync(db, mediaId, groupKey); + } + + groups.Add((groupKey, memberCount)); + } + + return groups; + } + + /// OPEN dead_file findings, one second apart, so + /// opened_at desc ordering is deterministic — index 0 is the OLDEST (last in the + /// desc-ordered read), the highest index the NEWEST (first). + static async Task> SeedDeadFileRowsAsync(DatabaseFixture db, int rowCount) + { + var baseTime = DateTimeOffset.UtcNow.AddHours(-1); + var ids = new List(); + for (var i = 0; i < rowCount; i++) + { + var mediaId = await InsertMediaRowAsync(db, $"/t385/dead-{i:D3}.flac"); + ids.Add(await InsertDeadFileFindingAsync(db, mediaId, baseTime.AddSeconds(i))); + } + + return ids; + } + // --------------------------------------------------------------------- // HAPPY PATH // --------------------------------------------------------------------- /// STORY-383 AC1 — 30 open near_duplicate groups of 2–4 members each; the store is /// asked for the first page of 25 groups. - public sealed class ScenarioLimitCountsGroups + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioLimitCountsGroups(DatabaseFixture db) { - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void ReturnsExactlyTwentyFiveDistinctGroupKeys() + [Fact] + public async Task ReturnsExactlyTwentyFiveDistinctGroupKeys() { - // var (rows, _) = await store.ListWithMediaAsync(RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct); - // Assert.Equal(25, rows.Select(r => r.Finding.GroupKey).Distinct().Count()); - Assert.Fail("pending T385"); + await db.ResetAsync(); + await SeedGroupsAsync(db, 30); + + var page = await Repo(db).ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + Assert.Equal(25, page.Items.Select(r => r.Finding.GroupKey).Distinct().Count()); } - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void EveryReturnedGroupIsWhole() + [Fact] + public async Task EveryReturnedGroupIsWhole() { - // For each returned group_key: the page's member count for it == the table's open member count for it. - Assert.Fail("pending T385"); + await db.ResetAsync(); + var groups = await SeedGroupsAsync(db, 30); + + var page = await Repo(db).ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + // ONE assertion over a homogeneous set (the T374 review MED-1 idiom): every returned + // group's own member count, in group_key order, compared against the table's own seeded + // member count for that same group, in one shot. + var actualSizes = page.Items + .GroupBy(r => r.Finding.GroupKey) + .OrderBy(g => g.Key, StringComparer.Ordinal) + .Select(g => g.Count()) + .ToArray(); + var expectedSizes = groups.Take(25).Select(g => g.MemberCount).ToArray(); + + Assert.Equal(expectedSizes, actualSizes); } } /// STORY-383 AC2 — the same 30 groups, second page (offset 25 groups). - public sealed class ScenarioOffsetCountsGroups + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioOffsetCountsGroups(DatabaseFixture db) { - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void ReturnsTheRemainingFiveWholeGroups() + [Fact] + public async Task ReturnsTheRemainingFiveWholeGroups() { - Assert.Fail("pending T385"); + await db.ResetAsync(); + await SeedGroupsAsync(db, 30); + + var page = await Repo(db).ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 25, ct: CancellationToken.None); + + Assert.Equal(5, page.Items.Select(r => r.Finding.GroupKey).Distinct().Count()); } - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void SharesNoGroupKeyWithPageOne() + [Fact] + public async Task SharesNoGroupKeyWithPageOne() { - // group_key asc ordering is stable: page1 keys ∩ page2 keys == ∅. - Assert.Fail("pending T385"); + await db.ResetAsync(); + await SeedGroupsAsync(db, 30); + var repo = Repo(db); + + var pageOne = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + var pageTwo = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 25, ct: CancellationToken.None); + + var pageOneKeys = pageOne.Items.Select(r => r.Finding.GroupKey).ToHashSet(); + var pageTwoKeys = pageTwo.Items.Select(r => r.Finding.GroupKey).ToHashSet(); + + Assert.Empty(pageOneKeys.Intersect(pageTwoKeys)); } } /// STORY-383 AC3 + STORY-382 AC6 — the total that rides beside the rows. - public sealed class ScenarioTotalCountsGroups + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioTotalCountsGroups(DatabaseFixture db) + { + [Fact] + public async Task TotalIsThirtyOnEveryPage() + { + await db.ResetAsync(); + await SeedGroupsAsync(db, 30); + var repo = Repo(db); + + var pageOne = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + var pageTwo = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 25, ct: CancellationToken.None); + + Assert.Equal((30, 30), (pageOne.Total, pageTwo.Total)); + } + + [Fact] + public async Task TotalCountsOnlyOpenGroups() + { + await db.ResetAsync(); + var groups = await SeedGroupsAsync(db, 30); + var repo = Repo(db); + var dismissedGroupKey = groups[0].GroupKey; + var findingIds = await ReadFindingIdsForGroupAsync(db, dismissedGroupKey); + foreach (var findingId in findingIds) + await repo.DismissAsync(findingId, CancellationToken.None); + + var page = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + Assert.Equal(29, page.Total); + } + } + + /// T385 review HIGH-1 (RULED, proven live) — state scopes which GROUPS qualify, + /// never which MEMBER rows render: a group with at least one matching member renders EVERY member, + /// regardless of that member's own state. + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioMixedStateGroupRendersWhole(DatabaseFixture db) + { + [Fact] + public async Task AllThreeMembersRenderWhenOneIsDismissed() + { + await db.ResetAsync(); + var repo = Repo(db); + var memberIds = await SeedOneGroupAsync(db, "grp-mixed", memberCount: 3); + await repo.DismissAsync(memberIds[0], CancellationToken.None); + + var page = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + var renderedMemberCount = page.Items.Count(r => r.Finding.GroupKey == "grp-mixed"); + + Assert.Equal(3, renderedMemberCount); + } + } + + /// Round-2 review HIGH-2 (RULED) — a RESOLVED member's own group_key survives the + /// resolve untouched (the resolve half never clears it), but it must never render inside its old + /// group: dismissed = the operator closed the finding while the media is still a duplicate → + /// render; resolved = the system closed it because the media is no longer a duplicate → don't + /// render. + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioResolvedMembersStayHidden(DatabaseFixture db) + { + [Fact] + public async Task OnlyTheStillDuplicateMembersRender() + { + await db.ResetAsync(); + var repo = Repo(db); + var openIdOne = await InsertNearDuplicateFindingAsync( + db, await InsertMediaRowAsync(db, "/t385/resolved-open-a.flac"), "grp-resolved"); + var openIdTwo = await InsertNearDuplicateFindingAsync( + db, await InsertMediaRowAsync(db, "/t385/resolved-open-b.flac"), "grp-resolved"); + await InsertResolvedNearDuplicateFindingAsync( + db, await InsertMediaRowAsync(db, "/t385/resolved-stale-c.flac"), "grp-resolved"); + + var page = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + var renderedIds = page.Items + .Where(r => r.Finding.GroupKey == "grp-resolved") + .Select(r => r.Finding.Id) + .OrderBy(id => id) + .ToArray(); + + Assert.Equal(new[] { openIdOne, openIdTwo }.OrderBy(id => id), renderedIds); + } + } + + /// Round-3 review MED-6 (RULED, fix verified live) — a FULLY-resolved group (every member + /// resolved, group_key still intact) must never become a phantom page slot: with + /// omitted (the endpoint's documented "any" default), group qualification + /// now excludes resolved rows exactly like member rendering already does, so + /// only ever counts groups that actually render at least one + /// row. + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioFullyResolvedGroupsNeverPhantom(DatabaseFixture db) + { + [Fact] + public async Task TotalMatchesRenderedGroupsWhenStateIsOmitted() + { + await db.ResetAsync(); + var repo = Repo(db); + await InsertResolvedNearDuplicateFindingAsync( + db, await InsertMediaRowAsync(db, "/t385/med6-resolved-a.flac"), "grp-med6-resolved"); + await InsertResolvedNearDuplicateFindingAsync( + db, await InsertMediaRowAsync(db, "/t385/med6-resolved-b.flac"), "grp-med6-resolved"); + await SeedOneGroupAsync(db, "grp-med6-open", memberCount: 2); + + var page = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, state: null, limit: 25, offset: 0, ct: CancellationToken.None); + + var renderedGroupCount = page.Items.Select(r => r.Finding.GroupKey).Distinct().Count(); + + Assert.Equal(renderedGroupCount, page.Total); + } + } + + /// T385 review HIGH-1's own flip side — a group where NO member matches the state filter + /// never qualifies at all, so it neither consumes a page slot nor counts into + /// . + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioFullyDismissedGroupDoesNotQualify(DatabaseFixture db) + { + [Fact] + public async Task TotalCountsOnlyTheStillOpenGroup() + { + await db.ResetAsync(); + var repo = Repo(db); + var dismissedGroupIds = await SeedOneGroupAsync(db, "grp-all-dismissed", memberCount: 3); + foreach (var findingId in dismissedGroupIds) + await repo.DismissAsync(findingId, CancellationToken.None); + await SeedOneGroupAsync(db, "grp-open", memberCount: 2); + + var page = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + Assert.Equal(1, page.Total); + } + } + + /// STORY-383 review MED-5 — the member sequence within one group matches + /// group_key asc, opened_at desc, id exactly; deterministic opened_at values, ascending + /// insertion order, so a correct opened_at desc read reverses the insertion order while an + /// (incorrect) id-only read would not. + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioMemberOrderWithinAGroup(DatabaseFixture db) { - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void TotalIsThirtyOnEveryPage() + [Fact] + public async Task MatchesOpenedAtDescThenId() { - // Same total from the offset:0 and offset:25 calls. - Assert.Fail("pending T385"); + await db.ResetAsync(); + var repo = Repo(db); + var baseTime = DateTimeOffset.UtcNow.AddHours(-1); + + var idOldest = await InsertNearDuplicateFindingWithOpenedAtAsync( + db, await InsertMediaRowAsync(db, "/t385/order-a.flac"), "grp-order", baseTime); + var idMiddle = await InsertNearDuplicateFindingWithOpenedAtAsync( + db, await InsertMediaRowAsync(db, "/t385/order-b.flac"), "grp-order", baseTime.AddSeconds(10)); + var idNewest = await InsertNearDuplicateFindingWithOpenedAtAsync( + db, await InsertMediaRowAsync(db, "/t385/order-c.flac"), "grp-order", baseTime.AddSeconds(20)); + + var page = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + var actualIds = page.Items.Select(r => r.Finding.Id).ToArray(); + + Assert.Equal([idNewest, idMiddle, idOldest], actualIds); } - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void TotalCountsOnlyOpenGroups() + /// Round-2 nit — the f.id tiebreak was unpinned: two members sharing the SAME + /// opened_at must still land in a deterministic, id-ascending order. + [Fact] + public async Task TiesOnOpenedAtBreakByIdAscending() { - // Dismissing every member of one group drops total to 29 on the next read. - Assert.Fail("pending T385"); + await db.ResetAsync(); + var repo = Repo(db); + var sameOpenedAt = DateTimeOffset.UtcNow.AddHours(-1); + + var idFirst = await InsertNearDuplicateFindingWithOpenedAtAsync( + db, await InsertMediaRowAsync(db, "/t385/tie-a.flac"), "grp-tie", sameOpenedAt); + var idSecond = await InsertNearDuplicateFindingWithOpenedAtAsync( + db, await InsertMediaRowAsync(db, "/t385/tie-b.flac"), "grp-tie", sameOpenedAt); + + var page = await repo.ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + var actualIds = page.Items.Select(r => r.Finding.Id).ToArray(); + + Assert.Equal([idFirst, idSecond], actualIds); } } /// STORY-383 AC5 + STORY-382 AC6 — a flat kind (dead_file) under the same read. - public sealed class ScenarioFlatKindsCountRows + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioFlatKindsCountRows(DatabaseFixture db) { - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void LimitAndOffsetCountRowsExactlyAsBefore() + [Fact] + public async Task LimitAndOffsetCountRowsExactlyAsBefore() { - // 60 open dead_file rows, limit 25 offset 25 → rows 26–50 in opened_at desc, id order. - Assert.Fail("pending T385"); + await db.ResetAsync(); + var findingIds = await SeedDeadFileRowsAsync(db, 60); + + var page = await Repo(db).ListWithMediaAsync( + RotKind.DeadFile, RotState.Open, limit: 25, offset: 25, ct: CancellationToken.None); + + // Desc-ordered read: highest index (newest) first. offset 25/limit 25 of 60 rows lands on + // original indices 34 down to 10 (25 rows), rows 26-50 in opened_at desc order. + var expected = findingIds.Skip(10).Take(25).Reverse().ToArray(); + var actual = page.Items.Select(r => r.Finding.Id).ToArray(); + + Assert.Equal(expected, actual); } - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void TotalIsTheExactMatchingRowCount() + [Fact] + public async Task TotalIsTheExactMatchingRowCount() { - Assert.Fail("pending T385"); + await db.ResetAsync(); + await SeedDeadFileRowsAsync(db, 60); + + var page = await Repo(db).ListWithMediaAsync( + RotKind.DeadFile, RotState.Open, limit: 25, offset: 25, ct: CancellationToken.None); + + Assert.Equal(60, page.Total); + } + } + + /// T385 review MED-3 — an offset past the last matching row/group returns an EMPTY + /// page, but must still reflect the true seeded total: the + /// rationale 's own remarks give for a + /// separate count read (over a count(*) over() window, which would carry no total at all on + /// a zero-row result) pinned as a live fact, one branch each. + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioOffsetPastTheEndStillPinsTotal(DatabaseFixture db) + { + [Fact] + public async Task NearDuplicateGroupsTotalSurvivesAnEmptyPage() + { + await db.ResetAsync(); + await SeedGroupsAsync(db, 30); + + var page = await Repo(db).ListWithMediaAsync( + RotKind.NearDuplicate, RotState.Open, limit: 25, offset: 100, ct: CancellationToken.None); + + Assert.Equal(30, page.Total); + } + + [Fact] + public async Task FlatKindTotalSurvivesAnEmptyPage() + { + await db.ResetAsync(); + await SeedDeadFileRowsAsync(db, 10); + + var page = await Repo(db).ListWithMediaAsync( + RotKind.DeadFile, RotState.Open, limit: 25, offset: 100, ct: CancellationToken.None); + + Assert.Equal(10, page.Total); } } @@ -97,14 +524,37 @@ public void TotalIsTheExactMatchingRowCount() // --------------------------------------------------------------------- /// STORY-382 AC8's store half — the kind-LESS read is byte-compatible with T377. - public sealed class ScenarioTheKindlessReadIsUnchanged + [Collection(DatabaseCollection.Name)] + [Trait("Category", "Integration")] + public sealed class ScenarioTheKindlessReadIsUnchanged(DatabaseFixture db) { - [Fact(Skip = "Pending T385 — see docs/PLAN.md")] - public void PagesFlatAcrossKindsInTheT377Order() + [Fact] + public async Task PagesFlatAcrossKindsInTheT377Order() { // No kind filter → kind, group_key nulls last, opened_at desc, id — a near_duplicate - // group MAY split at the page boundary here; that is the pinned old contract. - Assert.Fail("pending T385"); + // group MAY split at the page boundary here; that is the pinned old contract. 24 + // dead_file rows (kind sorts before near_duplicate in library.rot_kind's own declared + // order) plus one 4-member near_duplicate group: limit 25 exhausts on the group's FIRST + // member only. + await db.ResetAsync(); + for (var i = 0; i < 24; i++) + { + var mediaId = await InsertMediaRowAsync(db, $"/t385/split-dead-{i:D2}.flac"); + await InsertDeadFileFindingAsync(db, mediaId, DateTimeOffset.UtcNow); + } + + for (var i = 0; i < 4; i++) + { + var mediaId = await InsertMediaRowAsync(db, $"/t385/split-dup-{i}.flac"); + await InsertNearDuplicateFindingAsync(db, mediaId, "grp-split"); + } + + var page = await Repo(db).ListWithMediaAsync( + null, RotState.Open, limit: 25, offset: 0, ct: CancellationToken.None); + + var nearDuplicateRowsOnPage = page.Items.Count(r => r.Finding.Kind == RotKind.NearDuplicate); + + Assert.Equal(1, nearDuplicateRowsOnPage); } } }