From 200c24bc00a5831c4a182cc46143a4fbf76e20a6 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 00:46:31 +0900 Subject: [PATCH] fix: avoid unbounded ticket board queries --- .../console/worker-console.ui.test.ts | 15 +- .../src/lib/workspace/styles/tickets.css | 9 -- .../workspace/tickets/ticket-panel.test.ts | 57 ++----- .../src/lib/workspace/tickets/ticket-panel.ts | 63 +------- .../w/[workspaceId]/tickets/+page.svelte | 139 ++++++------------ .../routes/w/[workspaceId]/tickets/+page.ts | 63 ++------ 6 files changed, 74 insertions(+), 272 deletions(-) diff --git a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts index 80ed257d..67c9ef34 100644 --- a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts +++ b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts @@ -218,23 +218,22 @@ Deno.test("workspace Tickets surface provides Kanban and lifecycle controls", as "Tickets and Objectives should each be a single sidebar link", ); assert( - !ticketsLoad.includes("?limit=1000") && - ticketsLoad.includes('workspaceApiPath(workspaceId, "/tickets/query")') && - ticketsLoad.includes("ticketLaneDefinitions()") && - ticketsLoad.includes("ticketLaneQuery(lane)") && + ticketsLoad.includes("?limit=1000") && + !ticketsLoad.includes("/tickets/query") && ticketsPage.includes('class="ticket-kanban"') && ticketsPage.includes('class="ticket-lane-cards"') && + ticketsPage.includes("lane.tickets.slice(0, lane.visibleCount)") && ticketsPage.includes("handleLaneScroll") && - ticketsPage.includes("Loading 30 more"), - "Tickets list should query and incrementally scroll each Kanban lane", + ticketsPage.includes("revealNextTickets"), + "Tickets list should fetch lightweight summaries once and incrementally reveal each Kanban lane", ); assert( ticketPanelModel.includes('label: "Ready + Planning"') && ticketPanelModel.includes('label: "In progress + Queued"') && ticketPanelModel.includes('label: "Done + Closed"') && ticketPanelModel.includes("TICKET_LANE_PAGE_SIZE = 30") && - ticketPanelModel.includes('sort: "updated_desc"'), - "Ticket Kanban should combine related states into independent cursor pages", + ticketPanelModel.includes("nextTicketLaneVisibleCount"), + "Ticket Kanban should combine related states into independent 30-item display windows", ); assert( generatedTicketApi.includes("Generated from yoi-workspace-server") && diff --git a/web/workspace/src/lib/workspace/styles/tickets.css b/web/workspace/src/lib/workspace/styles/tickets.css index f9f50bad..f172464d 100644 --- a/web/workspace/src/lib/workspace/styles/tickets.css +++ b/web/workspace/src/lib/workspace/styles/tickets.css @@ -189,15 +189,6 @@ font-size: 0.7rem; text-align: center; } - .ticket-lane-load-error { - display: grid; - gap: 0.5rem; - color: #d66; - padding: 0.5rem; - } - .ticket-lane-load-error button { - justify-self: start; - } .ticket-card { display: grid; gap: 0.55rem; diff --git a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts index 8c1b3386..d3089a7b 100644 --- a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts +++ b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts @@ -1,8 +1,6 @@ import { - appendUniqueTicketSummaries, + nextTicketLaneVisibleCount, TICKET_LANE_PAGE_SIZE, - ticketLaneDefinitions, - ticketLaneQuery, ticketLanes, ticketWorkerLaunchHref, ticketWorkerMessage, @@ -83,47 +81,13 @@ Deno.test("ticketLanes combines workflow states and sorts by state then update t ]); }); -Deno.test("ticket lane queries request independent pages of 30", () => { - const [readyPlanning, inprogressQueued, doneClosed] = ticketLaneDefinitions(); - +Deno.test("ticket lane visibility advances in bounded pages of 30", () => { assertEquals(TICKET_LANE_PAGE_SIZE, 30); - assertEquals(ticketLaneQuery(readyPlanning).states, ["ready", "planning"]); - assertEquals(ticketLaneQuery(inprogressQueued).states, [ - "inprogress", - "queued", - ]); - assertEquals(ticketLaneQuery(doneClosed, "next-page"), { - attention: [], - cursor: "next-page", - event_kinds: [], - evidence: [], - limit: 30, - linked_objective_id: null, - query: null, - related_ticket_id: null, - relation_kind: null, - review_status: null, - sort: "updated_desc", - states: ["done", "closed"], - updated_after: null, - updated_before: null, - }); -}); - -Deno.test("incremental Ticket pages preserve order and discard duplicate ids", () => { - const current = [ - { id: "first", title: "First", state: "ready", priority: "1" }, - { id: "second", title: "Second", state: "planning", priority: "2" }, - ] as TicketSummary[]; - const incoming = [ - { id: "second", title: "Duplicate", state: "planning", priority: "2" }, - { id: "third", title: "Third", state: "planning", priority: "3" }, - ] as TicketSummary[]; - - assertEquals( - appendUniqueTicketSummaries(current, incoming).map((ticket) => ticket.id), - ["first", "second", "third"], - ); + assertEquals(nextTicketLaneVisibleCount(0, 95), 30); + assertEquals(nextTicketLaneVisibleCount(30, 95), 60); + assertEquals(nextTicketLaneVisibleCount(60, 95), 90); + assertEquals(nextTicketLaneVisibleCount(90, 95), 95); + assertEquals(nextTicketLaneVisibleCount(95, 95), 95); }); Deno.test("ticket worker launch uses the common Worker route and bounded Ticket context", () => { @@ -167,15 +131,12 @@ Deno.test("ticket panel starts the Orchestrator explicitly and gates orchestrati assertIncludes(panelSource, '{ method: "POST" }'); assertIncludes(panelSource, "Start Orchestrator"); assertIncludes(panelSource, "orchestrator.data?.online"); - assertIncludes( - panelSource, - 'workspaceApiPath(data.workspaceId, "/tickets/query")', - ); + assertIncludes(panelSource, "lane.tickets.slice(0, lane.visibleCount)"); assertIncludes( panelSource, "onscroll={(event) => handleLaneScroll(event, lane.id)}", ); - assertIncludes(panelSource, "Loading 30 more…"); + assertIncludes(panelSource, "Scroll for"); assertIncludes(detailSource, "{#if orchestratorOnline}"); assertIncludes(detailSource, "!orchestratorOnline"); assertIncludes(detailSource, "Orchestrator offline"); diff --git a/web/workspace/src/lib/workspace/tickets/ticket-panel.ts b/web/workspace/src/lib/workspace/tickets/ticket-panel.ts index dc6c4466..6162dc62 100644 --- a/web/workspace/src/lib/workspace/tickets/ticket-panel.ts +++ b/web/workspace/src/lib/workspace/tickets/ticket-panel.ts @@ -1,9 +1,4 @@ -import type { - TicketDetail, - TicketQueryItem, - TicketQueryRequest, - TicketSummary, -} from "$lib/generated/ticket-api"; +import type { TicketDetail, TicketSummary } from "$lib/generated/ticket-api"; export const TICKET_STATES = [ "planning", @@ -77,57 +72,11 @@ export type TicketLane = { tickets: TicketCardSummary[]; }; -export function ticketLaneDefinitions(): readonly TicketLaneDefinition[] { - return LANE_DEFINITIONS; -} - -export function ticketLaneQuery( - lane: { states: readonly TicketState[] }, - cursor: string | null = null, -): TicketQueryRequest { - return { - attention: [], - cursor, - event_kinds: [], - evidence: [], - limit: TICKET_LANE_PAGE_SIZE, - linked_objective_id: null, - query: null, - related_ticket_id: null, - relation_kind: null, - review_status: null, - sort: "updated_desc", - states: [...lane.states], - updated_after: null, - updated_before: null, - }; -} - -export function ticketSummaryFromQueryItem( - item: TicketQueryItem, -): TicketCardSummary { - return { - id: item.id, - priority: item.priority === null ? "normal" : String(item.priority), - state: item.state, - title: item.title, - updated_at: item.updated_at, - }; -} - -export function appendUniqueTicketSummaries( - current: TicketCardSummary[], - incoming: TicketCardSummary[], -): TicketCardSummary[] { - const ids = new Set(current.map((ticket) => ticket.id)); - return [ - ...current, - ...incoming.filter((ticket) => { - if (ids.has(ticket.id)) return false; - ids.add(ticket.id); - return true; - }), - ]; +export function nextTicketLaneVisibleCount( + current: number, + total: number, +): number { + return Math.min(total, current + TICKET_LANE_PAGE_SIZE); } function updatedAt(ticket: TicketCardSummary): number { diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte index b0139cdd..2bcdaee8 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte @@ -1,40 +1,34 @@ @@ -160,11 +108,15 @@
{displayedTicketCount} - tickets loaded + tickets displayed
+ {#if data.tickets.error} +

Tickets: {data.tickets.error}

+ {/if} + {#if orchestrator.error}

Orchestrator status: {orchestrator.error} @@ -177,6 +129,8 @@

{#each lanes as lane (lane.id)} + {@const displayedTickets = lane.tickets.slice(0, lane.visibleCount)} + {@const hasMore = lane.visibleCount < lane.tickets.length}
@@ -184,7 +138,7 @@

{lane.label}

- {lane.tickets.length}{lane.hasMore ? "+" : ""} + {displayedTickets.length}{hasMore ? "+" : ""}
@@ -192,7 +146,7 @@ class="ticket-lane-cards" onscroll={(event) => handleLaneScroll(event, lane.id)} > - {#each lane.tickets as ticket (ticket.id)} + {#each displayedTickets as ticket (ticket.id)} {:else} - {#if !lane.error} -
No tickets
- {/if} +
No tickets
{/each} - {#if lane.loading} -

Loading 30 more…

- {:else if lane.error} -
- {lane.error} - -
- {:else if !lane.hasMore && lane.tickets.length > 0} -

All tickets loaded.

+ {#if hasMore} +

+ Scroll for {Math.min(TICKET_LANE_PAGE_SIZE, lane.tickets.length - lane.visibleCount)} more +

+ {:else if displayedTickets.length > 0} +

All tickets displayed.

{/if}
diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts index 0e943920..2520733d 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts @@ -1,63 +1,20 @@ -import type { TicketQueryResponse } from "$lib/generated/ticket-api"; +import type { TicketListResponse } from "$lib/generated/ticket-api"; import { loadJson, workspaceApiPath } from "$lib/workspace/api/http"; -import { - ticketLaneDefinitions, - ticketLaneQuery, - ticketSummaryFromQueryItem, - type WorkspaceOrchestratorStatus, -} from "$lib/workspace/tickets/ticket-panel"; +import type { WorkspaceOrchestratorStatus } from "$lib/workspace/tickets/ticket-panel"; import type { PageLoad } from "./$types"; -export const load: PageLoad = async ({ fetch, params }) => { +export const load = (async ({ fetch, params }) => { const workspaceId = params.workspaceId; - const ticketLanePagesPromise = Promise.all( - ticketLaneDefinitions().map(async (lane) => { - try { - const page = await loadJson( - fetch, - workspaceApiPath(workspaceId, "/tickets/query"), - { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify(ticketLaneQuery(lane)), - }, - ); - return { - id: lane.id, - label: lane.label, - states: [...lane.states], - tickets: page.data?.items.map(ticketSummaryFromQueryItem) ?? [], - nextCursor: page.data?.page.next_cursor ?? null, - hasMore: page.data?.page.has_more ?? false, - error: page.error, - }; - } catch (error) { - return { - id: lane.id, - label: lane.label, - states: [...lane.states], - tickets: [], - nextCursor: null, - hasMore: false, - error: error instanceof Error - ? error.message - : "Unable to load Tickets.", - }; - } - }), - ); - - const [ticketLanePages, orchestrator] = await Promise.all([ - ticketLanePagesPromise, + const [tickets, orchestrator] = await Promise.all([ + loadJson( + fetch, + `${workspaceApiPath(workspaceId, "/tickets")}?limit=1000`, + ), loadJson( fetch, workspaceApiPath(workspaceId, "/orchestrator"), ), ]); - return { - workspaceId, - ticketLanePages, - orchestrator, - }; -}; + return { workspaceId, tickets, orchestrator }; +}) satisfies PageLoad;