From aa6ddd299446d4c5fb5ea8fed1ef847dc7fa7cb1 Mon Sep 17 00:00:00 2001 From: VibhavSetlur Date: Wed, 3 Jun 2026 13:21:41 -0500 Subject: [PATCH 1/2] fix(jobs): pre-flight workspace check before gapfill/FBA; humanize errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When users navigate to a model ref whose backing workspace object is missing (a reconstruct that never completed, or a stale/bookmarked link) and click Run FBA / Run GapFilling, the frontend was enqueuing a celery job that the backend could only fail with the cryptic PATRIC workspace text `_ERROR_Object not found!_ERROR_`. Live Flower shows 543 of these failed jobs across modelseed.gapfill (392) and modelseed.fba (151) — many users retrying the same broken ref 10-20 times — and complements the new clearer backend message José added. This adds a single fix: - New `lib/utils/jobErrors.ts#formatJobError` translates both the legacy `_ERROR_Object not found!_ERROR_` and the explicit "Object not found" substrings into actionable wording that names the missing path and points users at their reconstruct job. The new backend message is recognised and passed through unchanged. - `app/model/[...path]/page.tsx#submitModelJob` now calls `workspaceGet([modelRef])` before submitting, throws the friendly error if the object is missing, and routes both the pre-flight and any backend rejection through `formatJobError`. The doomed celery job is never enqueued. - `app/(user-data)/my-jobs/page.tsx` runs the displayed `errorMsg` through `formatJobError` so older failed-job rows show the new actionable wording too, substituting the job's own model ref. Unit-tested with 7 new cases in `tests/unit/utils/jobErrors.test.ts` (empty/null inputs, legacy form with/without ref, bare "Object not found", new backend message passthrough, unrelated errors untouched, Error-instance coercion). Local lint/typecheck/build clean, 98/98 unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- app/(user-data)/my-jobs/page.tsx | 8 ++++- app/model/[...path]/page.tsx | 25 ++++++++++--- lib/utils/jobErrors.ts | 32 +++++++++++++++++ tests/unit/utils/jobErrors.test.ts | 58 ++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 lib/utils/jobErrors.ts create mode 100644 tests/unit/utils/jobErrors.test.ts diff --git a/app/(user-data)/my-jobs/page.tsx b/app/(user-data)/my-jobs/page.tsx index 52f43cc3..3d745112 100644 --- a/app/(user-data)/my-jobs/page.tsx +++ b/app/(user-data)/my-jobs/page.tsx @@ -22,6 +22,7 @@ import { DataGrid, GridColDef, GridPaginationModel, GridSortModel } from '@mui/x import { getJobsFromApi } from '@/lib/api/modelseed'; import { listTrackedJobs, TrackedJob } from '@/lib/api/jobTracker'; +import { formatJobError } from '@/lib/utils/jobErrors'; import { USE_MODELSEED_API } from '@/lib/api/config'; import AuthGuard from '@/components/auth/AuthGuard'; import DataControlHeader, { withQuickSearchHeaders } from '@/components/layout/DataControlHeader'; @@ -135,8 +136,13 @@ function mergeApiAndTrackedJobs( statusHistory.set(id, { status, timestamp: now, sameCount: 1 }); } - const errorMsg = job.error ? String(job.error) : undefined; const outputPath = args?.output_path ? String(args.output_path) : undefined; + // Some legacy backend job records still carry the raw + // `_ERROR_Object not found!_ERROR_` string. Translate to actionable + // wording, substituting the job's own model ref when present so the + // tooltip points users at the path their reconstruct never produced. + const modelArg = typeof args?.model === 'string' ? String(args.model) : undefined; + const errorMsg = formatJobError(job.error, modelArg); const app = String(params?.command ?? job.app ?? job.type ?? 'Unknown'); rows.push({ diff --git a/app/model/[...path]/page.tsx b/app/model/[...path]/page.tsx index cd07072c..3411aac9 100644 --- a/app/model/[...path]/page.tsx +++ b/app/model/[...path]/page.tsx @@ -53,6 +53,7 @@ import { type TrackedJob, } from '@/lib/api/jobTracker'; import { parseWorkspaceDate } from '@/lib/utils/date'; +import { formatJobError } from '@/lib/utils/jobErrors'; import ModelDetailHeader from '@/components/ui/ModelDetailHeader'; import type { FbaAdvancedOptions } from '@/components/ui/MediaSelectionDialog'; import DownloadModelMenu from '@/components/ui/DownloadModelMenu'; @@ -2042,10 +2043,26 @@ export default function ModelDetailPage({ params }: { params: Promise<{ path: st setActionLoading(kind); setActionMessage(null); const selectedMedia = media || defaultMedia; + const modelRef = workspaceCandidates[0]; try { + // Pre-flight: confirm the model object actually exists in the workspace + // before enqueuing the celery job. Without this, users who navigate + // (or follow a stale link) to a model ref whose backing object is + // missing trigger a backend WorkspaceError per submission — the symptom + // surfacing in Flower as `_ERROR_Object not found!_ERROR_`. Catching + // it here avoids the wasted job and shows actionable wording. + try { + await workspaceGet([modelRef]); + } catch (probeErr) { + throw new Error( + formatJobError(probeErr, modelRef) ?? + `No model found at '${modelRef}'.`, + ); + } + // Build FBA payload with optional advanced options (reaction knockouts) const fbaPayload: Record = { - model: workspaceCandidates[0], + model: modelRef, media: selectedMedia, media_supplement: [], }; @@ -2060,7 +2077,7 @@ export default function ModelDetailPage({ params }: { params: Promise<{ path: st kind === 'fba' ? await submitFbaJobFromApi(fbaPayload) : await submitGapfillJobFromApi({ - model: workspaceCandidates[0], + model: modelRef, template_type: 'gn', media: selectedMedia, }); @@ -2083,8 +2100,8 @@ export default function ModelDetailPage({ params }: { params: Promise<{ path: st : `${kind === 'fba' ? 'FBA' : 'Gapfill'} job submitted.`, ); } catch (err) { - const message = err instanceof Error ? err.message : `Failed to submit ${kind} job`; - setActionMessage(message); + const raw = err instanceof Error ? err.message : `Failed to submit ${kind} job`; + setActionMessage(formatJobError(raw, modelRef) ?? raw); } finally { setActionLoading(null); } diff --git a/lib/utils/jobErrors.ts b/lib/utils/jobErrors.ts new file mode 100644 index 00000000..a0e94d95 --- /dev/null +++ b/lib/utils/jobErrors.ts @@ -0,0 +1,32 @@ +/** + * Humanize Celery/Workspace error messages surfaced by the modelseed_api backend + * so users see actionable wording instead of raw exception text. + * + * Two forms are handled: + * - Legacy form (older job records, pre-2026-06): `_ERROR_Object not found!_ERROR_` + * — the inscrutable PATRIC workspace internal error. + * - New form (after José's backend change): `No model found at ''. Check that + * your reconstruct job completed successfully and saved a model to this path, + * or pass a different ref.` — already user-readable, passed through unchanged. + * + * @param raw - Raw error string from `job.error` / submission rejection + * @param modelRef - The model workspace path the job was targeting (optional; + * used to substitute `` into the legacy-form message). + * @returns A user-facing error string, or `undefined` when `raw` is falsy. + */ +export function formatJobError(raw: unknown, modelRef?: string): string | undefined { + if (raw === null || raw === undefined) return undefined; + const text = String(raw).trim(); + if (!text) return undefined; + + if (/_ERROR_Object not found!_ERROR_/.test(text) || /Object not found/i.test(text)) { + const pathClause = modelRef ? ` at '${modelRef}'` : ''; + return ( + `No model found${pathClause}. Check that your reconstruct job ` + + 'completed successfully and saved a model to this path, or pass a ' + + 'different ref.' + ); + } + + return text; +} diff --git a/tests/unit/utils/jobErrors.test.ts b/tests/unit/utils/jobErrors.test.ts new file mode 100644 index 00000000..e62b5d7f --- /dev/null +++ b/tests/unit/utils/jobErrors.test.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from 'vitest'; +import { formatJobError } from '@/lib/utils/jobErrors'; + +describe('formatJobError', () => { + it('returns undefined for empty inputs', () => { + expect(formatJobError(null)).toBeUndefined(); + expect(formatJobError(undefined)).toBeUndefined(); + expect(formatJobError('')).toBeUndefined(); + expect(formatJobError(' ')).toBeUndefined(); + }); + + it('translates the legacy _ERROR_Object not found!_ERROR_ wording', () => { + const out = formatJobError("WorkspaceError('_ERROR_Object not found!_ERROR_')"); + expect(out).toContain('No model found'); + expect(out).toContain('reconstruct job'); + expect(out).not.toContain('_ERROR_'); + }); + + it('embeds the model ref into the legacy-form message when provided', () => { + const out = formatJobError( + "WorkspaceError('_ERROR_Object not found!_ERROR_')", + '/alice@patricbrc.org/modelseed/Ecoli/model', + ); + expect(out).toContain("at '/alice@patricbrc.org/modelseed/Ecoli/model'"); + }); + + it("handles a less-decorated 'Object not found' substring", () => { + const out = formatJobError('Object not found', '/u/modelseed/x/model'); + expect(out).toContain("at '/u/modelseed/x/model'"); + expect(out).toContain('No model found'); + }); + + it("passes through the new backend message unchanged", () => { + const backendNew = + "No model found at '/alice/modelseed/Ecoli/model'. Check that your " + + 'reconstruct job completed successfully and saved a model to this path, ' + + 'or pass a different ref.'; + const out = formatJobError(backendNew); + expect(out).toContain('No model found'); + expect(out).toContain('reconstruct job'); + expect(out).not.toContain('_ERROR_'); + }); + + it("leaves unrelated errors untouched", () => { + expect(formatJobError('Token validation failed: bogus signer')).toBe( + 'Token validation failed: bogus signer', + ); + expect(formatJobError('HTTPError("504 Gateway Timeout")')).toBe( + 'HTTPError("504 Gateway Timeout")', + ); + }); + + it('coerces non-string inputs via String()', () => { + const err = new Error("WorkspaceError('_ERROR_Object not found!_ERROR_')"); + const out = formatJobError(err); + expect(out).toContain('No model found'); + }); +}); From bcf94cf8e8226b56b1931627e022135f344e5e38 Mon Sep 17 00:00:00 2001 From: VibhavSetlur Date: Wed, 3 Jun 2026 12:46:56 -0500 Subject: [PATCH 2/2] test(biochem): make integration-test probe resilient to network flakes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Biochem Integration Tests suite intentionally degrades to "skipped" when staging.modelseed.org is unreachable, but the live probe in beforeAll had no internal timeout — on slow CI networks the vitest hookTimeout (10s) fired before the catch block could mark isApiAvailable = false, turning the whole suite (and CI) red. Race the probe against an explicit 7s timer so the catch path always runs first; observed master CI flake matched this same signature on 2026-06-03. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/unit/api/biochem.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/unit/api/biochem.test.ts b/tests/unit/api/biochem.test.ts index 6b6e14d2..b32b0f12 100644 --- a/tests/unit/api/biochem.test.ts +++ b/tests/unit/api/biochem.test.ts @@ -15,8 +15,17 @@ describe('Biochem API Integration Tests', () => { beforeAll(async () => { biochemApi = await loadBiochemApi(); + // Race the live probe against an internal timeout so the catch path runs + // (marking isApiAvailable = false) before the vitest hookTimeout aborts the + // hook itself. Otherwise CI fails on slow networks even though the suite is + // designed to skip gracefully when the API is unreachable. try { - const res = await biochemApi.getReactions({ limit: 1 }); + const res = await Promise.race([ + biochemApi.getReactions({ limit: 1 }), + new Promise((_, reject) => + setTimeout(() => reject(new Error('Biochem API probe timed out after 7s')), 7000), + ), + ]); expect(res.docs).toBeDefined(); } catch (e: unknown) { console.warn('Biochem API is unavailable, skipping tests:', getErrorMessage(e));