From d4845c27f73782b5b35f0fce79c1fbb36c1fc18f Mon Sep 17 00:00:00 2001 From: sriram Date: Mon, 7 Sep 2026 12:37:25 +0530 Subject: [PATCH] Dagster Orchestration File updates --- src/pages/OrchestrationPanel.jsx | 18 +- src/pages/UploadResultsPanel.jsx | 285 +++++++++++++++++++++---------- 2 files changed, 214 insertions(+), 89 deletions(-) diff --git a/src/pages/OrchestrationPanel.jsx b/src/pages/OrchestrationPanel.jsx index ed059b5..4331e5d 100644 --- a/src/pages/OrchestrationPanel.jsx +++ b/src/pages/OrchestrationPanel.jsx @@ -42,6 +42,14 @@ const RUN_POLL_MS = 3000; const TERMINAL = new Set(['done', 'failed', 'partial', 'cancelled']); +// Recent runs is a PICKER, not a history: you click a row to load its eleven +// stages below, and three is enough to reach the run you came for. Capped at +// the request rather than sliced after, because this refetches every +// INBOX_POLL_MS and the endpoint over-fetches `limit * 10` manifests off disk +// before filtering - asking for 20 read 200 of them per tick to draw a list +// nobody scrolls. +const RECENT_LIMIT = 3; + export function OrchestrationPanel() { const [inbox, setInbox] = useState(null); const [recent, setRecent] = useState([]); @@ -74,10 +82,10 @@ export function OrchestrationPanel() { // the inbox it used to wait in is empty and the run has an id this screen was // never told. Without this list an auto-started batch is invisible: its // eleven stages are recorded and served, and nothing renders them. Slim - // responses, so twenty of these do not carry twenty product manifests. + // responses, so a row does not carry a whole product manifest with it. const loadRecent = async () => { try { - const { batches } = await api.listCatalogBatches(20); + const { batches } = await api.listCatalogBatches(RECENT_LIMIT); setRecent(batches || []); } catch { /* the inbox call surfaces auth/network trouble; one banner is enough */ @@ -346,9 +354,13 @@ export function OrchestrationPanel() { {run.batch_id.slice(0, 8)} + {/* The sender used to be appended here as "from ". + Taken out deliberately: this list is for picking a run to + follow, and who sent it is not what you pick on. Upload + Results still names the sender - identifying whose upload + it is IS that screen's job. */} {run.files_total} file(s) - {run.submitted_by ? ` from ${run.submitted_by}` : ''} {/* Read off the manifest, not assumed from .env, so it stays true if a setting is changed without a redeploy. */} diff --git a/src/pages/UploadResultsPanel.jsx b/src/pages/UploadResultsPanel.jsx index 4d3588f..38a5a61 100644 --- a/src/pages/UploadResultsPanel.jsx +++ b/src/pages/UploadResultsPanel.jsx @@ -4,8 +4,8 @@ import { Loader2, PlayCircle, RefreshCw, XCircle, ArrowRight, } from 'lucide-react'; import { api } from '../api/client'; -import { STAGE_FLAGS, ago, fmtBytes } from './pipelineFormat'; -import { FileStages, Stat } from './pipelineShared'; +import { ago, fmtBytes } from './pipelineFormat'; +import { FileStages } from './pipelineShared'; /* * Admin -> Upload Results. @@ -14,6 +14,19 @@ import { FileStages, Stat } from './pipelineShared'; * the newest colleague upload is found and followed automatically, all eleven * stages, down to which rows the validation gate refused and why. * + * THREE THINGS, IN THIS ORDER: the file's NAME, its ELEVEN STAGES, and the ROWS + * THAT FAILED. That ordering is the brief, and it is why the run counters, the + * "brands touched" line, the img/llm flag chips and the batch id were taken out + * - each was true, none was the question being asked, and together they pushed + * the filename down to eleven-point text in the middle of the card. What is + * left besides those three is the handful of banners (queued, interrupted, + * partial, storage error) that exist to explain a timeline which is entirely + * grey; without them an interrupted run reads as a broken screen rather than as + * a run waiting for a Resume. + * + * There is no history here and there is not meant to be one: exactly one run is + * ever on screen, the newest, replaced in place as newer ones arrive. + * * THIS TAB USED TO BE THE REVIEW INBOX, and the inbox is still here - see the * gate section below. It is just no longer the point of the screen, because * with UPLOAD_AUTORUN at its default of true a colleague's upload starts on @@ -32,6 +45,22 @@ import { FileStages, Stat } from './pipelineShared'; * POST /api/admin/catalog-batch/ingest never passes it at all, so it is null * there. A batch released from the inbox carries the original senders, which is * correct - it is still their upload, just started by hand. + * + * IT IS A PREFERENCE, NOT A GATE. It used to be a gate, and that made this tab + * a dead end: a colleague who walked over and used the Batch Catalog Ingestion + * tab instead of the API produced a run with a null `submitted_by`, which was + * filtered out permanently however recent it was, and the screen said "No + * colleague upload yet" - true of the field and false of the world. So when the + * window holds no colleague upload we now adopt the newest run of any origin + * and SAY SO, rather than showing nothing. The exactness above is what makes + * that label honest: a null sender means the Admin UI started it. + * + * AND A FAILED LOOKUP IS NOT AN EMPTY ONE. Every discovery error used to be + * swallowed into that same empty state, so an expired session, a 403, a dead + * backend and a genuinely quiet week were one indistinguishable sentence. + * `discoverError` now separates them. An error still never wipes a run already + * on screen - a blip must not blank a report someone is reading - it annotates + * it instead. */ // Two questions at two cadences. "Has anything new arrived?" is cheap and slow; @@ -96,6 +125,16 @@ function isColleagueUpload(batch) { ); } +/* The fallback: any run at all, as long as it is not a retired husk. + * + * Same second clause as above and for the same reason - a dismissed drop keeps + * its row but has no stages and no result, so adopting one would draw eleven + * grey rows that can never fill in. `pending` needs no clause here: GET + * /batches drops those server-side (batch_catalog.py). */ +function isShowableRun(batch) { + return Boolean(batch) && batch.status !== 'retired'; +} + function IssueList({ title, items, tone = 'slate' }) { if (!items.length) return null; const border = @@ -120,6 +159,11 @@ export function UploadResultsPanel({ onBatchStarted }) { // --- the results half --------------------------------------------------- const [run, setRun] = useState(null); const [discovered, setDiscovered] = useState(false); // first discovery done + // Why the panel is empty, when it is. '' means the last lookup succeeded. + const [discoverError, setDiscoverError] = useState(''); + // How many runs that lookup examined, so an empty screen can say "looked at + // 40 runs, none of them showable" rather than an unqualified "nothing". + const [scanned, setScanned] = useState(0); const runIdRef = useRef(null); // Whether the widened lookup has already been spent - see discover(). const escalatedRef = useRef(false); @@ -133,11 +177,19 @@ export function UploadResultsPanel({ onBatchStarted }) { const [useLlm, setUseLlm] = useState(true); const [fetchImages, setFetchImages] = useState(true); - /* Find the newest colleague upload, and read the inbox in the same tick. + /* Find the run to show, and read the inbox in the same tick. * - * A failed poll is swallowed rather than replacing the panel with an error: - * a transient blip must not wipe a run the admin is reading, and the next - * tick retries. Same reasoning as the badge poll in AdminPage. */ + * A colleague upload is preferred; failing that, the newest run of any + * origin, so the tab is never a dead end. See the header comment. + * + * A failed poll never replaces a run already on screen - a transient blip + * must not wipe a report the admin is reading, and the next tick retries. + * But it IS recorded in `discoverError`, and the two renderings differ: with + * a run up it is a staleness strip, with nothing up it is the whole card. + * Silently treating a failure as "no uploads" is the bug this tab had. + * + * The inbox read is kept separate and stays silent. It is the secondary half + * of the screen and its failure must not blank the results. */ const discover = useCallback(async () => { try { let { batches } = await api.listCatalogBatches(DISCOVER_LIMIT); @@ -152,18 +204,26 @@ export function UploadResultsPanel({ onBatchStarted }) { ({ batches } = await api.listCatalogBatches(DISCOVER_MAX_LIMIT)); latest = (batches || []).find(isColleagueUpload); } - if (latest && latest.batch_id !== runIdRef.current) { - runIdRef.current = latest.batch_id; + // No colleague upload in reach. Show the newest run there is rather than + // an empty screen - see the header comment. The label that goes with it is + // derived at render from `submitted_by`, which is exact. + const chosen = latest || (batches || []).find(isShowableRun) || null; + setScanned((batches || []).length); + setDiscoverError(''); + if (chosen && chosen.batch_id !== runIdRef.current) { + runIdRef.current = chosen.batch_id; // Show the slim row immediately - it already carries stages[] and the // counters, because slim strips only result.products - then let the // detail poll replace it with the full record. - setRun(latest); - } else if (!latest) { + setRun(chosen); + } else if (!chosen) { runIdRef.current = null; setRun(null); } - } catch { - /* next tick retries */ + } catch (err) { + // Deliberately NOT clearing `run`: a transient blip must not wipe a + // report someone is reading. The strip below says it may be stale. + setDiscoverError(err?.message || 'Could not read the list of ingestion runs.'); } try { setInbox(await api.listInbox()); @@ -179,6 +239,14 @@ export function UploadResultsPanel({ onBatchStarted }) { return () => clearInterval(timer); }, [discover]); + // An explicit retry buys back the widened lookup, so an admin who knows a + // colleague just uploaded can force the deep search. Shared by the header's + // Refresh button and the error card below, which must behave identically. + const retry = useCallback(() => { + escalatedRef.current = false; + discover(); + }, [discover]); + // Follow the watched run until it settles. Re-created whenever the id or the // status changes, so it stops itself the moment the run settles rather than // polling a finished batch forever. @@ -299,9 +367,21 @@ export function UploadResultsPanel({ onBatchStarted }) { const pending = inbox?.pending_count ?? 0; const submissions = inbox?.submissions || []; - const totals = run?.totals; + // Exact, not a guess: only POST /api/admin/catalog-batch/ingest leaves the + // sender null - uploads.py always sets one, and a release from the inbox + // carries the original senders through. + const fromColleague = isColleagueUpload(run); const files = run?.files || []; + /* The answer to "what did they send?", promoted to the headline. + * + * A drop can carry several files and each still gets its own named row in the + * timeline below, so naming the first and counting the rest loses nothing. + * `current_file` is the fallback for the window where a batch exists but its + * file list has not been rendered yet. */ + const headlineName = files[0]?.filename || run?.current_file || 'file name unavailable'; + const extraFiles = Math.max(0, files.length - 1); + const rejections = files.flatMap((f) => (f.result?.rejections || []).map((r) => { const where = r.row == null ? 'row unknown' : `row ${r.row}`; @@ -484,18 +564,14 @@ export function UploadResultsPanel({ onBatchStarted }) { The most recent spreadsheet a colleague sent to{' '} /api/uploads/catalog, and everything the 11-stage pipeline did with it — including which rows were refused and why. - It updates on its own; there is nothing to pick. Your own uploads live under{' '} - Batch Catalog Ingestion. + It updates on its own; there is nothing to pick. When no colleague upload is in + reach it falls back to the most recent run of any kind, and says so, rather than + showing you an empty screen.

+ + )} + {/* Gate open but nothing has run: "no upload yet" would be flatly false - there is one, it is sitting above waiting for a decision. */} - {discovered && !run && pending > 0 && ( + {discovered && !run && !discoverError && pending > 0 && (

@@ -523,65 +621,100 @@ export function UploadResultsPanel({ onBatchStarted }) {

)} - {discovered && !run && pending === 0 && ( + {/* Genuinely nothing to show - and it now says WHICH nothing. With the + any-origin fallback in discover(), `scanned > 0` survives only for the + case where every run in reach is a retired husk. */} + {discovered && !run && !discoverError && pending === 0 && (
-

- No colleague upload yet. The moment one arrives at{' '} - POST /api/uploads/catalog, its run appears here - with the full stage breakdown. -

+ {scanned === 0 ? ( +

+ No ingestion run at all yet. The moment a colleague sends a sheet to{' '} + POST /api/uploads/catalog, its run appears here + with the full stage breakdown. +

+ ) : ( +

+ Looked at the last {scanned} ingestion run(s) and none can be shown — every + one of them was dismissed from the review inbox. +

+ )}

- Files you upload yourself are not shown here — they are under{' '} - Batch Catalog Ingestion. Uploads older than a week are cleared by - retention. + Only the most recent {DISCOVER_MAX_LIMIT} runs are searched, and uploads older than a + week are cleared by retention.

)} {run && (
-
-

- - from {run.submitted_by} - - {ago(run.created_at)} · {run.files_total} file(s) + {/* What is below is whatever the last SUCCESSFUL lookup found. Saying + so is the point - showing it silently as live is how a dead backend + passed for a quiet one. */} + {discoverError && ( +
+ + + The last refresh failed ({discoverError}), so this may be out of date. Retrying + every {Math.round(DISCOVER_POLL_MS / 1000)}s. -

-
- {STAGE_FLAGS.map(({ key, label, title }) => ( - - {label} - - ))} - {run.status}
-
+ )} -

{run.batch_id}

+ {/* No colleague upload was in reach, so this is the newest run of any + origin. Never silent about it: the tab promises a colleague's + upload, and a run with no sender is not one. */} + {!fromColleague && ( +
+ + + No colleague upload in the last {scanned} ingestion run(s), so this is simply the + most recent run. It carries no sender, which means it was started from the Admin + UI (Batch Catalog Ingestion) rather than sent to{' '} + POST /api/uploads/catalog. Its eleven stages + are below all the same. + +
+ )} - {/* `partial` exists so this does not read as success. Some files - landed and some did not, and the counters below are the total of - only the ones that did. */} + {/* The filename IS the headline - `truncate` on a min-w-0 flex child + so a long name is cut rather than pushing the card sideways. */} +

+ + {headlineName} +

+ + {/* Everything that used to crowd the filename, demoted to one line. */} +

+ + {run.submitted_by ? ( + from {run.submitted_by} + ) : ( + started from the Admin UI + )} + · + {ago(run.created_at)} + · + {run.status} + {extraFiles > 0 && ( + <> + · + +{extraFiles} more file(s), listed below + + )} +

+ + {/* `partial` exists so this does not read as success: some files + landed and some did not. */} {run.status === 'partial' && (
{run.files_done} of {run.files_total} file(s) landed; {run.files_failed} did not. - The counters below cover only what succeeded.
)} @@ -633,29 +766,9 @@ export function UploadResultsPanel({ onBatchStarted }) { ))} - {/* Gated on something having landed, so a still-queued run shows the - stage list rather than a wall of zeros. */} - {totals && (run.files_done > 0 || run.files_failed > 0) && ( -
- - - - - - -
- )} - - {run.brands?.length > 0 && ( -

- Brands touched:{' '} - {run.brands.join(', ')} -

- )} - - {/* The reason this screen exists rather than sending people to the - Dagster tab: a refused row, named, with the sheet line number the - sender sees on screen (header counted as row 1). */} + {/* The third of the three things: a refused row, named, with the sheet + line number the sender sees on screen (header counted as row 1). + This is the only place in the app that reports them. */}