Repository navigation
feat(gallery): restore PBR map generation and image storage administration - #9807
joshistoast wants to merge 7 commits into
Conversation
…ation Add a Generate PBR Maps image action that queues one pbr_maps utility graph into the auto-add board, watches its settlement and refreshes the gallery when it completes. Add an admin-only Image storage editor to the Server settings: folder layout for new images, polled move/recovery status, and confirmed Start Move and Recover actions backed by a Gallery image-storage data module. Extend the mock backend with runtime-config layout, image move routes and enqueue_batch, and align the Gallery and configuration docs with the restored UI.
…r review Share the utility origin watcher between runUtilityGraph and submitUtilityGraph; the submission now takes an abort signal, prepends, and re-reads the item on reconnect and at the settle timeout. PBR map settlement handlers attach before any account check, cancellation is an info notice, and the layout/move transport moves to a dedicated gallery storage entry. Wrap long storage errors, describe action errors to their buttons, and cover rejected layout saves and unknown layouts.
…ings and PBR graph builder Request counts and chunk sets are unchanged; the growth is the new locale strings, and the PBR graph builder joins the editor's existing image-actions source set.
lstein
left a comment
There was a problem hiding this comment.
Reviewed at 233e61f. The wire contract matches the backend: field names, enums and nullability against image_moves.py, the partial PATCH /runtime_config (exclude_unset, so sending only image_subfolder_strategy can't overwrite other fields), the pbr_maps node fields, and admin gating (no admin-only request is issued for non-admins). The two blockers below are about lifecycle, not the contract.
Blockers
1. The "unhandled settlement promise" fix is incomplete (features/queue/runtime/utilityGraph.ts, submitUtilityGraph)
settled is marked handled only inside fail(). Two sequences reject it while await enqueue(...) is still pending, before the caller has received the promise:
- The account changes during the enqueue POST.
onAbort→finish()→settleReject(aborted). - A fast
failed/canceledqueue_item_status_changedevent for the origin arrives before the enqueue response. The listener is attached first so that a fast completion is not missed, and that also lets a fast failure through.
In both cases, the enqueue succeeds, so fail() never runs. The microtask checkpoint ends with settled rejected and no handler, unhandledrejection fires, and loggingRuntime logs app.unhandled-rejection. useImageActions attaches .then only later. The existing tests miss this because the runtime test aborts only after submitUtilityGraph resolves, and the useImageActions test mocks submitUtilityGraph.
Fix: call settled.catch(() => {}) right after creating the promise; callers still see the rejection through their own .then. Add a test that aborts during a deferred enqueue, and one that delivers a failed event before the enqueue resolves. Also, the !isSettled && guard on the timer is currently untested; the abort-during-enqueue test would cover it.
2. The layout can be changed while a move is running (ImageStorageSettings.tsx, imageStorage.ts)
PATCH /runtime_config has no maintenance guard. _get_new_subfolder reads the live config for each batch, so a layout change mid-move leaves the images split between two layouts. The editor is the only thing preventing that, and it relies on isRunning from a status cache that can be stale:
- 409 on Start/Recover: if another admin or tab already started a move, the error path does not refetch status. The cache still says idle and polling is off, so the Select and buttons stay enabled during the move.
- Request in flight: the Select is disabled by
layoutQuery.isPending || isSavingLayout || isRunning, not byisActing.POST /startcallscount_images_needing_move()(a full scan) before replying, so on a large library you can confirm Start and then change the layout during that window. The reverse also works: change the layout, then Start before the PATCH lands. The two sync routes run concurrently in the thread pool. - Initial load: while status is loading,
isRunningdefaults tofalse, so the Select is enabled before the editor knows whether a move is running. - Stale read overwrites the POST result:
postImageMovecallssetQueryDatawithout cancelling in-flight fetches. A status GET that started earlier (for example from the layout save'sinvalidateQueries, or a focus refetch) can resolve after the POST and overwrite "running" with "idle". Polling then stops mid-move and Start re-enables.
Fix: refetch or invalidate status in the action error path, disable the Select on isActing and until status has loaded, and call cancelQueries before setQueryData. A maintenance guard on the backend PATCH would be the sturdier fix, but it is out of scope here.
Non-blocking
- Gallery refresh after a move only happens while the editor is mounted. During a move,
is_maintenance_active()makes the image, board and board-image routes answer 409, so the gallery ends up in an error state. The running→idle transition inside the statusqueryFnis the only thing that recovers it, and there is no socket event when a move ends. If an admin closes Settings mid-move, polling stops. AftergcTime, reopening has nopreviousdata, so the refresh never fires. Other tabs and non-admins are never told the move ended. The PR description calls this a limitation, but it leaves the gallery broken, not just stale. - A fast move can skip the refresh.
_start_background_operationcomputesis_runningafterexecutor.submit, so a tiny move can already be finished when the 202 goes out.postImageMovestoresisRunning: false, polling never starts, no running→idle transition is ever seen, and any gallery request that got a 409 during the move is never retried.postImageMovecould invalidate the gallery when the accepted status is already not running. - A queue clear leaves the PBR watch hanging.
clear()emits status events only for items that are running, then deletes the rest. A pending, prepended PBR item disappears without a terminal event. Nothing settles the watch until the 30-minute timeout, where the 404 read is swallowed and the watch ends with a silenttimeout. Likewise, an item still pending or running at 30 minutes (paused queue, first model download) that finishes later gets no "ready" notice and no gallery refresh. Consider treating a 404 from the reconcile read as a removal and telling the user. - The Start/Recover loading spinner never shows.
confirmActionreturnsvoid, soConfirmDialogawaits nothing and immediately runsonClose→setPendingAction(null).isActing && pendingAction === 'start'is therefore never true, and the dialog's own pending state is skipped too. Returning therunActionpromise fromconfirmActionwould fix both. - The enqueue-failure toast shows raw JSON.
ApiError.messageis the raw response body (assertOk), andsubmitUtilityGraphwraps it unchanged, so users see{"detail":"…"}. UsegetApiErrorMessage(error.cause ?? error, …). TheuseImageActionstest rejects withnew Error('queue paused'), which hides this. - Re-recorded timing baselines. In
browser-baseline.json, editorlongestTaskMaxMswent 180→343, 259→365 and 170→308,routeReadyMedianMsrose 20–27%, and Launchpad (barely touched here) regressed 8–15%.timingPolicy.enforceis false today, but these numbers become the reference once it is enforced. Please keep the byte and source-set deltas and restore the timing fields. The PR's other claims hold: request counts and chunk sets are unchanged, and the only source-set change ispbrMapsGraph.ts. - Smaller items:
- An inconsistent enqueue response reports failure but does not cancel the accepted item;
runUtilityGraphdoes cancel in the same case. - Several new
@features/gallery/storageexports have no consumers:isTerminalImageMoveJobState, theImageMoveJob*/ImageMoveOperationtypes, andgetImageMoveStatus/imageStorageKeysoutside tests. - Removing the abort listener (
signal.removeEventListener) is untested. Without it, each PBR click leaves a closure on the account-lifetime signal. - The mock backend diverges from the real one in ways that hide items 2–4:
- It treats an
errorjob as needing recovery; the real_FINISHED_JOBcounts it as finished. - It never makes image routes return 409 during a move.
- It always returns
is_running: truefrom the POST.
- It treats an
- An inconsistent enqueue response reports failure but does not cancel the accepted item;
…ith the server The utility graph watch handles a settlement that rejects during the enqueue, reconciles on queue clears and removals, and keeps watching an item that is still open instead of timing out. The image storage editor locks the layout while a move runs or a request is in flight, re-reads status after a refusal, and keeps an accepted move from being overwritten by a stale status read; gallery thumbnails that fail during a move retry on their own. Timing baselines return to the merge-base values.
Preserve gallery actions, server settings, fixtures, and current performance limits. Apply board destination permissions to PBR submissions.
Summary
Two legacy capabilities were missing from webv2.
Generate PBR Maps. The Gallery context menu offers "Generate PBR Maps" for a single selected image. It queues one utility graph with a
pbr_mapsnode (tile size 512, no border, non-intermediate output on the gallery's auto-add board), at the front of the queue as the legacy action did, and reports enqueue, completion (refreshing the gallery) and failure. A canceled job is reported as information, not an error.Image storage administration. Settings → Server gains an admin-only "Image storage" editor: the folder layout for new images (flat, date, type or hash, saved as
image_subfolder_strategyalone), the state of the current move or recovery job with remaining count and errors, and Start Move / Recover with confirmation. Status polls only while a job runs and only while the editor is open; 403/409/422 responses surface inline. Non-admins never see the section.The utility-graph runtime that watches a queue item by origin is now shared between the existing canvas utility path and the new submission, takes an abort signal so an account change detaches its listener and timer, and reconciles against the queue item on reconnect, on a queue clear, and at a fixed interval while the item is still open; an item that disappears from the queue is reported as removed.
During a move the server answers thumbnail and image-file reads with 409. Gallery tiles whose thumbnail fails to load retry on a bounded backoff, so every tab, admin or not, recovers once the move ends.
Related Issues / Discussions
Follows up the legacy parity audit behind #9803.
QA Instructions
From
invokeai/frontend/webv2(Node 22.14, pnpm 10.12):pnpm check:releasepasses. Performance baselines were re-recorded for byte growth only (new locale strings; the PBR graph builder joins the image-actions source set); request counts and chunk sets are unchanged. Two unrelated browser suites each failed once under heavy machine load and passed on rerun.Follow-up at 29ac7cd:
pnpm check:releasepasses (lint, 10145 unit and 2913 browser tests, fixtures, architecture performance, project-file journeys, accessibility). Verified against the mock backend in Chromium: Start → confirm shows the pending spinner while the request runs and the layout Select is disabled behind the dialog; a 409 from a move another admin started re-reads status and locks the controls; thumbnails that answered 409 during a move load on their own once it ends; a PBR item cleared from the queue is reported as removed.Mock backend (
scripts/mock-backend.mjs, representative profile;/__faults?imageMoves=needs-recovery|queue-active|instant|failedseeds job states;needs-recovery: interrupted-job message, Start Move disabled, Recover enabled and works. Withqueue-active: Start Move shows the 409 detail inline. As a non-admin the Server section is hidden.POST /api/v1/queue/default/enqueue_batchis sent withprepend: true. The mock never completes items, so the "ready" notice and gallery refresh need a real backend (first run downloads the PBR models).Review
Material findings resolved: a PBR submission whose account changed, or whose item failed, during the enqueue left its settlement promise unhandled (now marked handled on creation); the watcher's socket listener and timer survived an account change (abort signal); a pending PBR item that a queue clear deleted left the watch hanging until a timeout (the watcher reconciles on
queue_cleared, treats a missing item as removed, and re-arms while the item is still open instead of timing out); the image storage editor let the layout change while a move ran when its status cache was stale (status is re-read after a refusal, the Select locks during a request and until status has loaded, and an accepted move cannot be overwritten by an older status read); a move that finished within the start request never refreshed the gallery; the confirm dialog's pending state never showed; enqueue failures showed raw JSON; the browser timing baselines had been re-recorded (restored to the merge-base values; only bytes and the source set changed). Thumbnails broken by a move did not recover outside the admin's open Settings editor; tiles now retry a failed load on a bounded backoff.Remaining limitations:
PATCH /app/runtime_confighas no server-side maintenance guard, so another client can still change the layout while a move runs; the editor's lock is client-side. A paused queue keeps a PBR watch open (one listener, one timer and one read per interval) until the item finishes or the account changes.Compatibility / Rollout
No persisted-state or API changes. The mock backend gained the image-move and runtime-config routes for fixtures.
Checklist
What's Newcopy (if doing a release after this PR)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.