Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions .server-changes/runs-backward-pagination-slice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
area: webapp
type: fix
---

Fix an off-by-one in `ClickHouseRunsRepository.listRunIds` backward pagination.
When paging backward with more rows before the page (`hasMore`), the displayed
page was sliced as `rows.slice(1, size + 1)`, which dropped the row closest to
the cursor and kept the extra "has-more" sentinel — returning a page that
straddled two logical pages (one row from the correct previous page plus one
from the page before it). The result set is always the first `page.size` rows
(the sentinel is the trailing element in both directions), so the slice is now
`rows.slice(0, size)` for forward and backward alike. Forward pagination and the
cursor values were already correct and are unchanged.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Backward !hasMore edge case with partial pages produces null nextCursor

In the backward !hasMore branch (line 120), nextCursor = cursorFor(reversedRows.at(options.page.size - 1)). If the result has fewer rows than page.size (e.g., 1 row with page.size=2), reversedRows.at(1) is undefined → nextCursor becomes null. This means a user who backward-navigated to a partial first page cannot navigate forward from it. This is a pre-existing edge case (the cursor logic was not changed in this PR) and only affects degenerate scenarios where a backward navigation reaches the very first page with fewer items than page.size — in typical UI flows the forward pages were already visited so the user has other navigation paths.

(Refers to lines 119-121)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -123,11 +123,13 @@ export class ClickHouseRunsRepository implements IRunsRepository {
}
}

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

return { runIds, pagination: { nextCursor, previousCursor } };
}
Expand Down
122 changes: 122 additions & 0 deletions apps/webapp/test/runsRepositoryCursor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,4 +296,126 @@ describe("RunsRepository cursor pagination", () => {
]);
}
);

replicationContainerTest(
"backward pagination across multiple pages returns each page intact (no straddling)",
async ({ clickhouseContainer, redisOptions, postgresContainer, prisma }) => {
const { clickhouse } = await setupClickhouseReplication({
prisma,
databaseUrl: postgresContainer.getConnectionUri(),
clickhouseUrl: clickhouseContainer.getConnectionUrl(),
redisOptions,
});

const organization = await prisma.organization.create({
data: { title: "test", slug: "test" },
});
const project = await prisma.project.create({
data: {
name: "test",
slug: "test",
organizationId: organization.id,
externalRef: "test",
},
});
const runtimeEnvironment = await prisma.runtimeEnvironment.create({
data: {
slug: "test",
type: "DEVELOPMENT",
projectId: project.id,
organizationId: organization.id,
apiKey: "test",
pkApiKey: "test",
shortcode: "test",
},
});

// Five runs so a backward page has more rows before it (hasMore === true),
// which is the case the off-by-one in the backward slice corrupts.
const ids = [
"aaaaaaaaaaaaaaaaaaaaaaaa",
"bbbbbbbbbbbbbbbbbbbbbbbb",
"cccccccccccccccccccccccc",
"dddddddddddddddddddddddd",
"eeeeeeeeeeeeeeeeeeeeeeee",
];
const base = new Date("2026-06-04T16:55:07.000Z").getTime();
for (let i = 0; i < ids.length; i++) {
await prisma.taskRun.create({
data: {
id: ids[i],
createdAt: new Date(base + (ids.length - 1 - i) * 1000),
friendlyId: `run_${ids[i]}`,
taskIdentifier: "my-task",
payload: JSON.stringify({ foo: "bar" }),
traceId: `trace_${i}`,
spanId: `span_${i}`,
queue: "test",
runtimeEnvironmentId: runtimeEnvironment.id,
projectId: project.id,
organizationId: organization.id,
environmentType: "DEVELOPMENT",
engine: "V2",
},
});
}

await setTimeout(1000);

const runsRepository = new RunsRepository({ prisma, clickhouse });
const baseOptions = {
projectId: project.id,
environmentId: runtimeEnvironment.id,
organizationId: organization.id,
};

const sortIds = (page: string[]) => page.slice().sort();

// Walk every forward page, capturing the run ids and the previousCursor.
const forwardPages: Array<{ ids: string[]; previousCursor: string | null }> = [];
let cursor: string | undefined = undefined;
for (let guard = 0; guard < 20; guard++) {
const page = await runsRepository.listRuns({
...baseOptions,
page: { size: 2, cursor, direction: cursor ? "forward" : undefined },
});
forwardPages.push({
ids: page.runs.map((r) => r.id),
previousCursor: page.pagination.previousCursor,
});
if (!page.pagination.nextCursor) break;
cursor = page.pagination.nextCursor;
}

// Forward pagination itself must cover every run exactly once, in 3 pages.
expect(forwardPages.flatMap((p) => p.ids).sort()).toEqual(ids.slice().sort());
expect(forwardPages).toHaveLength(3);

// Walk backward from the last page. Each backward page must equal the
// corresponding forward page exactly — no row from an adjacent page
// bleeding in (the straddle bug returned e.g. {b,c} instead of {c,d}).
const backwardPages: string[][] = [];
let backCursor: string | null = forwardPages[forwardPages.length - 1].previousCursor;
for (let guard = 0; guard < 20 && backCursor; guard++) {
const page = await runsRepository.listRuns({
...baseOptions,
page: { size: 2, cursor: backCursor, direction: "backward" },
});
backwardPages.push(page.runs.map((r) => r.id));
backCursor = page.pagination.previousCursor;
}

const expectedBackward = forwardPages
.slice(0, -1)
.reverse()
.map((p) => sortIds(p.ids));
expect(backwardPages.map(sortIds)).toEqual(expectedBackward);

// And the full backward traversal (last page + everything walked back to
// the start) covers every run exactly once.
const seen = [...forwardPages[forwardPages.length - 1].ids, ...backwardPages.flat()];
expect(seen.slice().sort()).toEqual(ids.slice().sort());
expect(new Set(seen).size).toBe(ids.length);
}
);
});