Skip to content

Commit f00d5e7

Browse files
matt-aitkenclaude
andcommitted
fix(webapp): correct backward pagination slice in listRunIds
Backward pagination dropped the wrong end of the result set. listRunRows fetches page.size + 1 rows to detect hasMore; that extra row is the one farthest from the cursor in both directions (forward orders DESC, backward orders ASC), so it is always the trailing element. Forward correctly sliced rows[0..size); backward+hasMore sliced rows[1..size+1], dropping the row closest to the cursor and keeping the has-more sentinel. The result was a page that straddled two logical pages (one run from the correct previous page plus one from the page before it) — so paging "newer" across a boundary repeated and skipped runs. Both directions now slice rows[0..size). Forward pagination, the hasMore=false backward path, and all cursor values were already correct and are unchanged. Adds a regression test that walks forward across multiple pages, then walks backward from the last page and asserts each backward page exactly reproduces the corresponding forward page (and the full traversal covers every run once). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent ef04cc3 commit f00d5e7

3 files changed

Lines changed: 143 additions & 5 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Fix an off-by-one in `ClickHouseRunsRepository.listRunIds` backward pagination.
7+
When paging backward with more rows before the page (`hasMore`), the displayed
8+
page was sliced as `rows.slice(1, size + 1)`, which dropped the row closest to
9+
the cursor and kept the extra "has-more" sentinel — returning a page that
10+
straddled two logical pages (one row from the correct previous page plus one
11+
from the page before it). The result set is always the first `page.size` rows
12+
(the sentinel is the trailing element in both directions), so the slice is now
13+
`rows.slice(0, size)` for forward and backward alike. Forward pagination and the
14+
cursor values were already correct and are unchanged.

apps/webapp/app/services/runsRepository/clickhouseRunsRepository.server.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,11 +123,13 @@ export class ClickHouseRunsRepository implements IRunsRepository {
123123
}
124124
}
125125

126-
const runIds = (
127-
direction === "backward" && hasMore
128-
? rows.slice(1, options.page.size + 1)
129-
: rows.slice(0, options.page.size)
130-
).map((row) => row.runId);
126+
// The page is always the first `page.size` rows of the result. listRunRows
127+
// fetches one extra row only to detect `hasMore`; that extra row is the
128+
// farthest from the cursor in BOTH directions (forward orders DESC, backward
129+
// orders ASC), so it's always the trailing element to drop — never the
130+
// leading one. (Slicing `[1, size+1]` for backward dropped the row closest
131+
// to the cursor and kept the has-more sentinel, straddling two pages.)
132+
const runIds = rows.slice(0, options.page.size).map((row) => row.runId);
131133

132134
return { runIds, pagination: { nextCursor, previousCursor } };
133135
}

apps/webapp/test/runsRepositoryCursor.test.ts

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,4 +296,126 @@ describe("RunsRepository cursor pagination", () => {
296296
]);
297297
}
298298
);
299+
300+
replicationContainerTest(
301+
"backward pagination across multiple pages returns each page intact (no straddling)",
302+
async ({ clickhouseContainer, redisOptions, postgresContainer, prisma }) => {
303+
const { clickhouse } = await setupClickhouseReplication({
304+
prisma,
305+
databaseUrl: postgresContainer.getConnectionUri(),
306+
clickhouseUrl: clickhouseContainer.getConnectionUrl(),
307+
redisOptions,
308+
});
309+
310+
const organization = await prisma.organization.create({
311+
data: { title: "test", slug: "test" },
312+
});
313+
const project = await prisma.project.create({
314+
data: {
315+
name: "test",
316+
slug: "test",
317+
organizationId: organization.id,
318+
externalRef: "test",
319+
},
320+
});
321+
const runtimeEnvironment = await prisma.runtimeEnvironment.create({
322+
data: {
323+
slug: "test",
324+
type: "DEVELOPMENT",
325+
projectId: project.id,
326+
organizationId: organization.id,
327+
apiKey: "test",
328+
pkApiKey: "test",
329+
shortcode: "test",
330+
},
331+
});
332+
333+
// Five runs so a backward page has more rows before it (hasMore === true),
334+
// which is the case the off-by-one in the backward slice corrupts.
335+
const ids = [
336+
"aaaaaaaaaaaaaaaaaaaaaaaa",
337+
"bbbbbbbbbbbbbbbbbbbbbbbb",
338+
"cccccccccccccccccccccccc",
339+
"dddddddddddddddddddddddd",
340+
"eeeeeeeeeeeeeeeeeeeeeeee",
341+
];
342+
const base = new Date("2026-06-04T16:55:07.000Z").getTime();
343+
for (let i = 0; i < ids.length; i++) {
344+
await prisma.taskRun.create({
345+
data: {
346+
id: ids[i],
347+
createdAt: new Date(base + (ids.length - 1 - i) * 1000),
348+
friendlyId: `run_${ids[i]}`,
349+
taskIdentifier: "my-task",
350+
payload: JSON.stringify({ foo: "bar" }),
351+
traceId: `trace_${i}`,
352+
spanId: `span_${i}`,
353+
queue: "test",
354+
runtimeEnvironmentId: runtimeEnvironment.id,
355+
projectId: project.id,
356+
organizationId: organization.id,
357+
environmentType: "DEVELOPMENT",
358+
engine: "V2",
359+
},
360+
});
361+
}
362+
363+
await setTimeout(1000);
364+
365+
const runsRepository = new RunsRepository({ prisma, clickhouse });
366+
const baseOptions = {
367+
projectId: project.id,
368+
environmentId: runtimeEnvironment.id,
369+
organizationId: organization.id,
370+
};
371+
372+
const sortIds = (page: string[]) => page.slice().sort();
373+
374+
// Walk every forward page, capturing the run ids and the previousCursor.
375+
const forwardPages: Array<{ ids: string[]; previousCursor: string | null }> = [];
376+
let cursor: string | undefined = undefined;
377+
for (let guard = 0; guard < 20; guard++) {
378+
const page = await runsRepository.listRuns({
379+
...baseOptions,
380+
page: { size: 2, cursor, direction: cursor ? "forward" : undefined },
381+
});
382+
forwardPages.push({
383+
ids: page.runs.map((r) => r.id),
384+
previousCursor: page.pagination.previousCursor,
385+
});
386+
if (!page.pagination.nextCursor) break;
387+
cursor = page.pagination.nextCursor;
388+
}
389+
390+
// Forward pagination itself must cover every run exactly once, in 3 pages.
391+
expect(forwardPages.flatMap((p) => p.ids).sort()).toEqual(ids.slice().sort());
392+
expect(forwardPages).toHaveLength(3);
393+
394+
// Walk backward from the last page. Each backward page must equal the
395+
// corresponding forward page exactly — no row from an adjacent page
396+
// bleeding in (the straddle bug returned e.g. {b,c} instead of {c,d}).
397+
const backwardPages: string[][] = [];
398+
let backCursor: string | null = forwardPages[forwardPages.length - 1].previousCursor;
399+
for (let guard = 0; guard < 20 && backCursor; guard++) {
400+
const page = await runsRepository.listRuns({
401+
...baseOptions,
402+
page: { size: 2, cursor: backCursor, direction: "backward" },
403+
});
404+
backwardPages.push(page.runs.map((r) => r.id));
405+
backCursor = page.pagination.previousCursor;
406+
}
407+
408+
const expectedBackward = forwardPages
409+
.slice(0, -1)
410+
.reverse()
411+
.map((p) => sortIds(p.ids));
412+
expect(backwardPages.map(sortIds)).toEqual(expectedBackward);
413+
414+
// And the full backward traversal (last page + everything walked back to
415+
// the start) covers every run exactly once.
416+
const seen = [...forwardPages[forwardPages.length - 1].ids, ...backwardPages.flat()];
417+
expect(seen.slice().sort()).toEqual(ids.slice().sort());
418+
expect(new Set(seen).size).toBe(ids.length);
419+
}
420+
);
299421
});

0 commit comments

Comments
 (0)