-
Notifications
You must be signed in to change notification settings - Fork 616
feat(assets): adopt cursor pagination in the Generated tab jobs walk #12769
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 17 commits
cc04846
4cadbb8
e498c4a
880582a
8d23daa
1fee449
5ef89c7
1f810a1
69a4d78
f318718
753b0b4
4290619
c207f56
89fdbcd
c238145
6f7686b
6c3ead5
ea0f8a9
8b40f6a
e471b64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,12 +18,43 @@ import type { | |
| } from './jobTypes' | ||
| import { zJobDetail, zJobsListResponse, zWorkflowContainer } from './jobTypes' | ||
|
|
||
| /** | ||
| * Position of the page to fetch. `after` is an opaque keyset cursor from a | ||
| * prior response's `nextCursor` and takes precedence over `offset`; `offset` | ||
| * remains as the fallback for random access and for backends that don't mint | ||
| * cursors. | ||
| */ | ||
| export type JobsPageRequest = | ||
| | { after: string; offset?: never } | ||
| | { offset?: number; after?: never } | ||
|
|
||
| /** | ||
| * Non-ok response from the jobs API. Carries the HTTP status so callers can | ||
| * tell a rejected cursor (400 INVALID_CURSOR) apart from transient failures. | ||
| */ | ||
| const MAX_ERROR_BODY_LENGTH = 200 | ||
|
|
||
| export class JobsApiError extends Error { | ||
| constructor( | ||
| readonly status: number, | ||
| body: string | ||
| ) { | ||
| const truncated = | ||
| body.length > MAX_ERROR_BODY_LENGTH | ||
| ? `${body.slice(0, MAX_ERROR_BODY_LENGTH)}…` | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (non-blocking): the delta's hardening fixes shipped without direct test coverage — this 200-char body truncation, and the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in ea0f8a9.
Both tests fail if the catch guard is removed, which is the regression lock you asked for. |
||
| : body | ||
| super(`[Jobs API] Failed to fetch jobs: ${status} ${truncated}`.trim()) | ||
| this.name = 'JobsApiError' | ||
| } | ||
| } | ||
|
|
||
| interface FetchJobsRawResult { | ||
| jobs: RawJobListItem[] | ||
| total: number | ||
| offset: number | ||
| limit: number | ||
| hasMore: boolean | ||
| nextCursor?: string | ||
| } | ||
|
|
||
| export interface FetchHistoryPageResult { | ||
|
|
@@ -32,43 +63,39 @@ export interface FetchHistoryPageResult { | |
| offset: number | ||
| limit: number | ||
| hasMore: boolean | ||
| nextCursor?: string | ||
| } | ||
|
|
||
| /** | ||
| * Fetches raw jobs from /jobs endpoint | ||
| * Fetches raw jobs from /jobs endpoint. | ||
| * Throws on failure so callers can tell a failed page apart from an empty | ||
| * last page (e.g. a stale cursor rejected with 400 INVALID_CURSOR). | ||
| * @internal | ||
| */ | ||
| async function fetchJobsRaw( | ||
| fetchApi: (url: string) => Promise<Response>, | ||
| statuses: JobStatus[], | ||
| maxItems: number = 200, | ||
| offset: number = 0 | ||
| page: JobsPageRequest = {} | ||
| ): Promise<FetchJobsRawResult> { | ||
| const statusParam = statuses.join(',') | ||
| const url = `/jobs?status=${statusParam}&limit=${maxItems}&offset=${offset}` | ||
| try { | ||
| const res = await fetchApi(url) | ||
| if (!res.ok) { | ||
| console.error(`[Jobs API] Failed to fetch jobs: ${res.status}`) | ||
| return { | ||
| jobs: [], | ||
| total: 0, | ||
| offset, | ||
| limit: maxItems, | ||
| hasMore: false | ||
| } | ||
| } | ||
| const data = zJobsListResponse.parse(await res.json()) | ||
| return { | ||
| jobs: data.jobs, | ||
| total: data.pagination.total, | ||
| offset: data.pagination.offset, | ||
| limit: data.pagination.limit, | ||
| hasMore: data.pagination.has_more | ||
| } | ||
| } catch (error) { | ||
| console.error('[Jobs API] Error fetching jobs:', error) | ||
| return { jobs: [], total: 0, offset, limit: maxItems, hasMore: false } | ||
| const pageParam = | ||
| page.after != null | ||
| ? `after=${encodeURIComponent(page.after)}` | ||
| : `offset=${page.offset ?? 0}` | ||
| const url = `/jobs?status=${statusParam}&limit=${maxItems}&${pageParam}` | ||
| const res = await fetchApi(url) | ||
| if (!res.ok) { | ||
| throw new JobsApiError(res.status, await res.text().catch(() => '')) | ||
|
mattmillerai marked this conversation as resolved.
|
||
| } | ||
| const data = zJobsListResponse.parse(await res.json()) | ||
| return { | ||
| jobs: data.jobs, | ||
| total: data.pagination.total, | ||
| offset: data.pagination.offset, | ||
| limit: data.pagination.limit, | ||
| hasMore: data.pagination.has_more, | ||
| nextCursor: data.pagination.next_cursor ?? undefined | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -98,7 +125,7 @@ export async function fetchHistory( | |
| maxItems: number = 200, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick (non-blocking): |
||
| offset: number = 0 | ||
| ): Promise<JobListItem[]> { | ||
| const { jobs } = await fetchHistoryPage(fetchApi, maxItems, offset) | ||
| const { jobs } = await fetchHistoryPage(fetchApi, maxItems, { offset }) | ||
| return jobs | ||
| } | ||
|
|
||
|
|
@@ -108,13 +135,13 @@ export async function fetchHistory( | |
| export async function fetchHistoryPage( | ||
| fetchApi: (url: string) => Promise<Response>, | ||
| maxItems: number = 200, | ||
| offset: number = 0 | ||
| page: JobsPageRequest = {} | ||
| ): Promise<FetchHistoryPageResult> { | ||
| const result = await fetchJobsRaw( | ||
| fetchApi, | ||
| ['completed', 'failed', 'cancelled'], | ||
| maxItems, | ||
| offset | ||
| page | ||
| ) | ||
|
|
||
| // History gets priority based on total count (lower than queue) | ||
|
|
@@ -123,7 +150,8 @@ export async function fetchHistoryPage( | |
| total: result.total, | ||
| offset: result.offset, | ||
| limit: result.limit, | ||
| hasMore: result.hasMore | ||
| hasMore: result.hasMore, | ||
| nextCursor: result.nextCursor | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -134,12 +162,7 @@ export async function fetchHistoryPage( | |
| export async function fetchQueue( | ||
| fetchApi: (url: string) => Promise<Response> | ||
| ): Promise<{ Running: JobListItem[]; Pending: JobListItem[] }> { | ||
| const { jobs } = await fetchJobsRaw( | ||
| fetchApi, | ||
| ['in_progress', 'pending'], | ||
| 200, | ||
| 0 | ||
| ) | ||
| const { jobs } = await fetchJobsRaw(fetchApi, ['in_progress', 'pending']) | ||
|
|
||
| const running = jobs.filter((j) => j.status === 'in_progress') | ||
| const pending = jobs.filter((j) => j.status === 'pending') | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.