Skip to content

Commit e431d24

Browse files
lsteinclaudeJPPhoto
authored
fix(ui): don't strand the viewer under the progress overlay, for images or videos (invoke-ai#9475)
* fix(ui): resolve the viewer preview on the thumbnail and stop it sticking The image viewer holds the last progress preview on screen until the final image's onLoad fires. Two problems with that. The reveal was gated on a preload of imageDTO.image_url — the full-resolution PNG — so on a slow connection the stale latent preview stayed up for the entire multi-megabyte download. A 256px thumbnail is already generated for every image and is typically higher resolution than the preview it replaces. Gate on that instead; DndImage renders it via Chakra's fallbackSrc and swaps the full image in, in place, once it arrives. The preload also used the raw URL while DndImage requests useMediaUrl(...), which appends ?media_cookie_version=N. Different key, so the bytes were fetched twice (measured: 2 requests mismatched vs 1 matched). Route the preload through useMediaUrl so it is byte-identical. The reuse is the document's list of available images, keyed by URL rather than the HTTP cache, so it still holds in multiuser mode where images are served Cache-Control: private, no-store. Separately, the viewer's progress atoms are distinct stores from the global ones in services/events/stores, and only the latter were reset on socket lifecycle transitions. socket.io has no event replay, so a drop spanning the terminal queue_item_status_changed loses that event permanently and nothing is left to clear the opaque overlay covering the finished image — the reported "backgrounded the tab, came back, only a reload fixes it". Reset the viewer's atoms on connect/connect_error/disconnect too, matching setEventListeners. onLoadImage is not a guaranteed callback in any case: Chakra reports a failed load as onError, useImage only re-runs when src changes, the load can beat the terminal event, and an all-intermediate item never changes the selection. So the deferred clear also gets a backstop deadline. The armed flag and its timer live together in createDeferredClear — as separate state, a path that reset the flag but leaked the timer let a deadline outlive the generation that armed it and blank a later one's live preview. The backstop does not clear while other sessions still have previews, since nulling $progressImage tears down the whole overlay including multi-GPU tiles, and the reconnect reset only replaces the map when it holds something, because connect_error fires once per reconnection attempt. The terminal-status policy moves to a pure getTerminalProgressAction so the branchy decision is testable without a socket or a React tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): stop auto-switch flashing the previous image over the next preview Starting a new generation soon after the previous one finishes made the viewer flicker: the new previews would appear, then the previous generation's finished image would cover them for two seconds, then the previews resumed. Waiting between generations avoided it. The flash is the "reveal selected image" feature (invoke-ai#9217), which briefly hides the progress overlay so a mid-generation gallery click is visible. Its only guard against the auto-switch handoff was $isProgressImageResolving — a timing guard, and the timing loses: the auto-switch selection is dispatched only after onInvocationComplete's async DTO fetch, then waits for the thumbnail preload, and the next generation's first invocation_progress event slots into that window and resets the flag. By the time the handoff reaches the viewer it is indistinguishable from a user click, so the reveal fires over the live preview. Distinguish them by identity instead of timing: auto-switch records the image name in a small registry at dispatch, and the reveal effect consumes it on the selection's first render. Consumption happens on every rendered-image change, not only when the reveal conditions hold, because in the common (unraced) case the image renders with no progress showing and a leftover entry would suppress a genuine user selection of the same image later. Entries also expire after 30 seconds. Recording is unconditional but consumption requires the image to actually render, so a superseded auto-switch (two completions within one thumbnail-fetch window — routine with parallel multi-GPU sessions), a viewer unmounted by comparison mode, or a duplicate invocation_complete event would otherwise leave an immortal entry whose only future effect is to swallow a genuine click on that image — the very dead-click the reveal exists to prevent. The TTL is generous for the dispatch-to-render handoff it protects; expiring early merely readmits the 2-second flash on a very slow connection, which is the milder failure. The suppression branch still lowers $isTemporarilyShowingSelectedImage — the effect has already cancelled any running reveal's timer by that point, so returning with the atom raised would wedge the reveal on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): don't strand the viewer under the video progress overlay During a video render, the progress-preview overlay swallowed every gallery thumbnail click: the selection changed underneath, but the opaque overlay stayed on top, so nothing visibly happened until a tab switch remounted the viewer. Three causes, three fixes: - CurrentVideoPreview never implemented the temporary reveal that CurrentImagePreview got in invoke-ai#9217. Port it: clicking a thumbnail mid-render now lifts the overlay for 2 s so the click visibly lands, then the live preview returns. An actively-playing video is never re-covered (audio would keep running under an opaque overlay with unreachable controls); the overlay returns when the player closes. - The reveal's previous-item tracking was per-component, so any click that switched media type (image <-> video swaps the mounted preview component) reset it and the reveal was swallowed. The ref now lives in the shared ImageViewerContext; the image side is careful not to null it while a preload is still pending (adversarial-review finding: the mount run would otherwise erase the previous-video fact and kill the video->image reveal). - After completion, the "preview resolves into the final media" clear only fired from the final media's load callback. On a slow connection that lags far behind completion, and an errored <video> never fires it - stranding the overlay permanently. The video error handler now clears a pending resolve, and a 10 s failsafe in the context drops the illusion rather than strand the overlay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): tile concurrent session previews in the video viewer (multi-GPU) CurrentImagePreview tiles per-session previews when more than one render runs concurrently; CurrentVideoPreview only ever rendered the single shared latest preview, so parallel sessions overwrote each other's frames in place. Port the ProgressImageTiles branch, mirroring the image viewer exactly ($activeProgressData is already tracked per-session in the shared context). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(ui): resolve knip warnings * fix(ui): don't treat an auto-switch to a finished video as a user reveal CurrentVideoPreview treated any change of the selected video as a mid-render gallery click, so an auto-switch to a just-finished video hid the *next* render's live preview behind it for 2 seconds. The auto-switch selection is dispatched only after onInvocationComplete's DTO fetch resolves, so a quickly-started next render's first progress event can land ahead of it and reset $isProgressImageResolving. By the time the selection reaches the reveal effect it is indistinguishable by timing from a user click, which is why invoke-ai#9434 fixed the image side by identity instead: the name being auto-switched to is recorded at dispatch and consumed on the selection's first render. Port that to videos: addVideosToGallery records the video name, and the video reveal effect consumes it on every change of the rendered video (consuming unconditionally, or a leftover entry would swallow a genuine later click on the same video). The registry is keyed by gallery item name, which is polymorphic across images and videos, so autoSwitchedImages.ts and its test are byte-identical copies of invoke-ai#9434's — whichever branch merges second adds nothing. Reported by JPPhoto in review of invoke-ai#9475. * test(ui): pin the promoted session's preview against a stale resolve timer Covers JPPhoto's second review scenario on invoke-ai#9475 directly: two sessions generating, the one owning the shared preview finishes, and the other is left with no further progress event. Nothing armed by the completion — neither the resolve timeout nor the finished session's own late image load — may take the promoted session's preview down. * fix(ui): address review — deadline ownership handoff + duplicate-completion gallery work Two fixes from JPPhoto's review: 1. When the resolve deadline fired while other sessions were still active (multi-GPU), it only disarmed, leaving the shared progress atoms owned by the finished item. The surviving sessions' terminal events then saw a foreign owner and ignored them, stranding the opaque overlay on a stale preview after the last session ended. The deadline now promotes the most recently active session into the shared atoms, so its own terminal event clears or re-arms them normally. 2. Duplicate invocation_complete deliveries re-ran the gallery work, which double-counted optimistic board totals and re-recorded the auto-switch marker after it had been consumed — suppressing a later genuine gallery click on that image. The handler now tracks processed invocations itself (the shared dedupe map can't be used: the workflow coordinator pre-marks first-delivery events for non-active workflow items) and returns before any gallery work on a duplicate. The auto-switch registry additionally settles on every rendered-image change: a match drops all older entries, a miss clears the registry, so no stale entry survives past the next render to swallow a genuine click. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(ui): drop a comment reference to a symbol this tree doesn't have autoSwitchedImages.ts was copied from invoke-ai#9434, whose TTL comment points at that branch's PROGRESS_IMAGE_RESOLVE_TIMEOUT_MS. Main's equivalent is RESOLVE_TIMEOUT_MS in viewerProgressLifecycle, and the cross-reference adds nothing here either way. * fix(ui): close four holes an adversarial review found in the video reveal - The reveal effect could return with $isTemporarilyShowingSelectedImage still true after it had already cleared the running reveal's timer, wedging the overlay off for the rest of the render. Reachable on a plain mount under StrictMode: the double-invoked effect re-enters with the shared ref already holding this video's name and takes the previous-name early return. Every path out of the effect now lowers the atom; the image side gets the same treatment, where sharing the ref makes the shape reachable in principle too. - A rejected play() routed through the video error handler, which now ends a pending resolve illusion. A play rejection is not a load failure — the element is intact — so for an unattributable session it could cut short an illusion belonging to a different render. The toast path is split from the element-error path. - Playback running to its end left isPlaying true, so the overlay never came back and the live preview stayed hidden for the rest of the generation. onEnded now drops back to the idle still. - Concurrent completions (multi-GPU) recorded two auto-switched names but only the last selection ever rendered, orphaning the first entry for its full 30 s TTL — and that orphan, unlike the ones the TTL was written for, never self-heals, so the user's next click on that item was a dead click. consume() now drops every entry recorded before the one that rendered, since the selection moved on without those ever rendering. * fix(ui): address review round 2 — selection-scoped auto-switch marker + lookup-failure retry Two of JPPhoto's findings applied to the current head (his third, the deadline-promotion blocker, was fixed ahead of this round by the merge of main's invoke-ai#9389: the lifecycle's onTerminal hands the shared preview to the freshest surviving session on every terminal status, and its test 'keeps promoting through a chain of terminations' pins his exact promote-B/cancel-B/C-still-active sequence). 1. The auto-switch marker is now scoped to the selection it was recorded for, not keyed by image name with a TTL. A redux listener settles the marker on every action that moves the gallery selection (matched by state change, not action type, so new selection-writing reducers are covered automatically). An auto-switch that never renders — because the user clicked elsewhere first, even without a rendered-image change — is dropped the moment the selection moves on, so it can never swallow the user's later click on that image. At most one marker exists, and only while its selection stands, so the TTL and pending bound are gone. 2. A completion delivery whose DTO lookups all fail no longer poisons the dedupe key: the key is dropped so a re-delivery can redo the gallery work instead of being turned away as a duplicate of a delivery that never landed. Partial failures keep the key — the fetched DTOs' board totals and optimistic inserts were already dispatched, and a retry would double-count them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(ui): cover the reveal suppression, and narrow the retry to gallery work An adversarial review of the round-3 changes found the mechanism sound but its verification hollow: deleting the auto-switch record(), or making the suppression branch unreachable, left all 1888 tests green. Both mutations remove the behavior this PR exists to deliver. - The reveal decision moves into a pure getSelectedItemRevealDecision, unit tested branch by branch. It answers 'reveal' or 'hide' and nothing else: the caller clears the running reveal's timer before asking, so a path that returned without writing the atom would strand the reveal on with no timer left to end it. That was reachable in the old shape via the previous-name early return. - onInvocationComplete gains tests that the auto-switched selection is actually marked (and is not when auto-switch is off), and that the video half of the retry condition is load-bearing, via a mixed image+video result. Two lower-severity findings from the same review: - A retry re-ran the whole handler, including two side effects that are global rather than per-event: the canvas processing flag and $lastProgressEvent. A re-delivery arriving after the user started another run would stop that run's spinner and blank its progress. The dedupe entry now records what is outstanding ('done' vs 'gallery-retryable') and a retry redoes only the gallery work. - The retry condition read "DTOs fetched", but two paths return before dispatching anything — a first intermediate image, and an all- intermediate video result. An event whose surviving output was intermediate therefore kept its key with nothing dispatched, so the lookup that failed alongside it could never be retried. The counts now mean "gallery work dispatched". Every fix above is pinned by a test that fails without it. * fix(ui): stop exporting a type nothing imports knip fails the frontend checks on it: SelectedItemRevealDecision is only ever the return type of the function declared beside it. * fix(ui): address review round 2 — selection-scoped marker, completion dedupe, reveal controller Two merge blockers and four findings from JPPhoto's review. 1. The name-keyed auto-switch registry is replaced with invoke-ai#9434's selection-scoped marker, taken verbatim (autoSwitchedImages.ts, the settle listener, and its tests are byte-identical between the two branches, so whichever merges second does so cleanly). A marker now survives only while the selection it was recorded for stands: a redux listener settles it on every selection move, so an auto-switch that never rendered can no longer swallow a later genuine click on the same item — the miss-leaves-marker-pending blocker. 2. Duplicate completion deliveries are deduped at the top of the handler (also ported from invoke-ai#9434, which grew the same guard for the image path), so a re-delivered video completion no longer re-records the marker or re-dispatches the auto-switch selection. A delivery whose DTO lookups all fail drops its dedupe key so a re-delivery can retry; partial failures keep it, since the fetched DTOs' gallery work already went out. addVideosToGallery keeps this branch's marker record for the video auto-switch — the one line invoke-ai#9434's copy of the file doesn't have. 3. The reveal sequencing moves out of the two preview components' effects into a shared controller (selectedItemReveal.ts) whose sequences are unit-tested, closing the three holes found in the inline version: - StrictMode's double-invoked mount effect killed every cross-media first reveal in development: the second run found the shared ref already holding the name the first run wrote. The controller remembers which item its own in-flight reveal is showing and re-arms instead of lowering. - A click landing inside a resolve window was dead forever: the ref advanced past it before the resolving guard ran. The controller leaves the ref and the marker untouched while resolving, so the click (or auto-switch) keeps its identity and is classified when the window ends. - Clearing the selection reset the ref to null, making the next click read as the viewer's first render. A cleared selection now moves the ref to a sentinel that is neither null nor a name, so the next selection — including the same item — reveals. 4. The metadata panel is gated on !isPlaying and !isTemporarilyShowingSelectedImage explicitly rather than riding on !withProgress, which both of those states turn off — it used to land exactly on top of the native controls or the just-revealed video. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): carry invoke-ai#9434's per-output retry, and unblock knip The completion-dedupe port in the previous commit took invoke-ai#9434's state at the time, which JPPhoto has since found two defects in — both now fixed there and brought across so the branches stay one implementation: - A duplicate arriving while the first delivery was still fetching was rejected outright, so when that delivery lost its output to a failed DTO lookup, the only event that could have retried it was already gone. Duplicates now await the in-flight delivery and become its retry; several waiters serialize into one. - The dedupe was event-wide, so a partial failure (plausible for image collections) permanently abandoned the output that failed while the rest landed. The entry now names the outputs whose lookup failed and a retry re-fetches only those, which is what makes recovering a partial failure safe from double-counting. The two handlers are now identical but for the video auto-switch marker record, which is this branch's. Also moves the three video-workflow query hooks out of the @knipignore block in videos.ts: they have call sites on this branch, so knip fails the frontend checks on the unused tags. Same change invoke-ai#9434 carries. * fix(ui): repair five defects an adversarial review found in the retry rework All five are in code this branch shares with invoke-ai#9434, and are fixed there too. - A result can name the same image twice (an image collection concatenates its inputs without deduping). Each occurrence was fetched and counted separately, inflating the board total — and because the retry set is keyed by name, a retry re-admitted the occurrence that had already landed and counted it again. Outputs are now fetched once per distinct name. - An intermediate output returned from the whole pass, abandoning siblings that belong in the gallery. That was survivable when the dedupe was event-wide; with per-output tracking those siblings are in nobody's missing set, so no re-delivery could ever recover them. Intermediates are now filtered, as the video path already did. - A retry re-ran the auto-switch, moving the user's selection (and possibly their board) long after they had chosen something else. A retry now lands the lost output and nothing more. - A throw inside the gallery work escaped as an unhandled rejection: both call sites discard this handler's promise. It is logged, and the outputs whose lookups failed are still recorded as retryable. - addBoardIdSelectedListener matched galleryViewChanged, which the auto-switch dispatches immediately before imageSelected. The probe it started woke on that very selection and re-selected the first name in a stale list, undoing the auto-switch — and the viewer then revealed that wrong image over the live preview, which is exactly the flash the marker exists to prevent. An explicit selection now cancels the probe. Each fix has a test that fails without it, including a real-store test for the listener. * fix(ui): don't put the overlay back over a reveal the user already earned A second adversarial pass, which independently confirmed the five fixes in the previous commit, found one more and three unpinned invariants. The controller lowered any in-flight reveal the moment a generation started resolving. A user clicking mid-render could therefore have their click covered again 200ms into its two seconds — by an unrelated session finishing — and, if that session produced no gallery output, stay covered until the 3s resolve backstop. A reveal already granted for the item on screen is now re-armed through the resolve window instead of lowered; one belonging to a different item still lowers. Newly pinned, each verified by mutation: the timer cancel that every run depends on (two live timers means the older one cuts the newer reveal short), the resolve window's lowering path, and that consuming one item's marker cannot spend another's — reachable whenever the rendered item lags a just-auto-switched selection. * fix(ui): repair five defects an adversarial review found in the retry rework Carried across from invoke-ai#9475, which shares this code — the two handlers stay one implementation. - A result can name the same image twice (an image collection concatenates its inputs without deduping). Each occurrence was fetched and counted separately, inflating the board total; and because the retry set is keyed by name, a retry re-admitted the occurrence that had already landed. Outputs are fetched once per distinct name. - An intermediate output returned from the whole pass, abandoning siblings that belong in the gallery. Survivable when the dedupe was event-wide; with per-output tracking those siblings are in nobody's missing set, so nothing could recover them. Intermediates are filtered, as the video path already did. - A retry re-ran the auto-switch, moving the user's selection (and possibly their board) long after they had chosen something else. - A throw inside the gallery work escaped as an unhandled rejection — both call sites discard this handler's promise. - addBoardIdSelectedListener matched galleryViewChanged, which the auto-switch dispatches immediately before imageSelected. The probe it started woke on that very selection and re-selected the first name in a stale list, undoing the auto-switch and revealing the wrong image over the live preview. An explicit selection now cancels the probe. Also pins that consuming one item's marker cannot spend another's. * fix(ui): cancel the board probe on any selection, not just imageSelected Two from JPPhoto's third round. The probe cancellation added last round matched imageSelected only, but the ordinary gallery paths — thumbnail clicks, shift/ctrl range selects, keyboard navigation — dispatch selectionChanged. A board or view change followed by any of those left the probe running, so it still replaced (or cleared) what the user had just picked, and the viewer then revealed that item over the live progress. The listener now matches the *change of active item* rather than a list of actions, which covers every writer, including any added later. Second: the reveal controller returned during a resolve window before recording that the selection had been cleared. Clearing and then re-selecting the same item inside that window left the ref on that item, so when the window ended the re-selection read as "nothing changed" and stayed hidden under the overlay. A clear is now recorded even while resolving — it has no pending identity to preserve, unlike the landed- but-unclassified render the deferral exists for. Both have tests that fail without the fix. * fix(ui): cancel the board probe on any selection, not just imageSelected Carried from invoke-ai#9475, where JPPhoto found it: the cancellation added last round matched imageSelected only, but thumbnail clicks, range selects and keyboard navigation dispatch selectionChanged. A board or view change followed by any of those left the probe running, so it still replaced or cleared what the user had just picked — and the auto-switch path this PR protects goes through exactly that window. The listener now matches the change of active item rather than a list of actions. * fix(ui): cancel the probe on any selection write, and stop losing outputs Two merge blockers from JPPhoto's fourth round. The board/view probe compared only the *active* item to decide whether a selection had landed under it. Narrowing a multi-selection, or re-picking the item already active, leaves that item unchanged while still being the user settling what they want — so the probe survived those and replaced their selection when it woke. It now compares the whole selection: the state is immutable, so a new array reference is exactly "the selection was written", and cancelling a probe more often than strictly necessary costs nothing. An output lost to a transient DTO lookup failure was only recovered if the server happened to re-deliver the completion event. Nothing re-emits one, so in practice the image was simply absent from the gallery and the board counts until something unrelated refetched them. A delivery that leaves outputs missing now refetches exactly those names at 1s, 3s and 9s, bounded — an output still missing after that is not coming back from a retry — with the entry left retryable so a re-delivery can still recover it for free. Each attempt takes the same path as a re-delivery, so only missing names are fetched, landed outputs are untouched, and the global side effects are not re-run; a new pass supersedes the refetch already queued, so duplicates cannot run two chains. This is the retry work from the follow-up branch, brought down to where the blocker was raised. Tests come with it, including the sequences from the review. * fix(ui): cancel the probe on any selection write, and stop losing outputs Carried from invoke-ai#9475, where JPPhoto raised both as merge blockers; this branch shares the listener and the completion handler. - The board/view probe compared only the active item, so narrowing a multi-selection or re-picking the item already active left it running to overwrite the user's selection. It now compares the whole selection. - An output lost to a transient DTO lookup failure was only recovered if the server happened to re-deliver the completion event, which nothing makes it do. Missing outputs are now refetched at 1s, 3s and 9s, bounded, each attempt taking the same path as a re-delivery so landed outputs are untouched and the global side effects are not re-run. * test(ui): pin the reveal's connection to the overlay atom The merge resolution replaced invoke-ai#9434's inlined reveal with this branch's controller, and rewrote the wiring test that went with it. That test carried the only assertion on either branch that the component writes $isTemporarilyShowingSelectedImage -- it matched the literal hide path the inlined version had -- and the rewrite dropped it. Nothing else covers it. selectedItemReveal.test.ts substitutes its own setRevealed, so the controller tests are structurally incapable of observing the atom, and CurrentVideoPreview's assertions are all on the read side (withProgress, the metadata gate). Neither component is ever mounted: this directory has no DOM test environment. An adversarial review of the merge proved the gap by replacing setRevealed with a no-op in both previews: all 26 assertions across the two wiring tests still passed, and so did the full suite, with the reveal completely dead -- a mid-render gallery click doing nothing, which is the bug both PRs exist to fix. Both tests now fail against that mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ui): don't let retries outlive their session, or swallow the first click Two merge blockers and two findings from JPPhoto's fifth round. Scheduled refetches survived socket and auth teardown. The timers close over an event from the session that scheduled them but dispatch into whatever store is current when they fire, so a logout, account switch or reconnect inside the 13s window would fetch the old session's output and insert it into the new session's gallery and board caches. setEventListeners now returns a disposer, and useSocketIO calls it before disconnecting. The reveal suppressed the first item the viewer ever rendered, on the grounds that the viewer opening onto an existing selection is not a click. But a viewer sitting empty while a generation runs is a state the user has been shown, and their first click there is a click like any other — it was landing behind the overlay. An empty selection now records the cleared-selection sentinel whether or not anything rendered before, so that click reveals while the open-onto-a-selection render still does not. Also, since this is the third round it has come up: the video reveal no longer starts its two seconds at mount. preload="metadata" and the near-zero seek do not prove a frame exists, so the reveal could run out over a black element and then re-cover it. The controller now holds the claim until the item reports a decoded frame (onLoadedData), bounded by a 1s grace so media that never loads still makes the click land. Readiness is reported as *which* item has painted rather than a boolean, because a boolean would be reset from a different effect than the one that reads it. And retry success no longer bumps cached board totals: seconds after the fact another refresh may already have inserted the output, and unlike the name-list insert those increments do not dedupe. A retry now asks the server for the affected boards instead. * fix(ui): end a completion handler's session properly, and restore the socket suite Restores services/events/setEventListeners.test.ts, which I destroyed in b874b90: I wrote the file without checking it existed, and its 644 lines of executable socket coverage — workflow invalidation, queue cancellation, own/foreign routing, cross-user isolation — went with it, replaced by two source-string checks. The suite is back, and the teardown it needed is now an executable test in it rather than a grep: the mocked handler carries a dispose(), and the disposer setEventListeners returns is asserted to call it. The blocker behind that test: disposal only cleared queued timers. A DTO request already in flight came back afterwards, dispatched into whatever store had replaced the old one, and scheduled fresh retries against it — so a logout or account switch during a lookup could put one user's output in the next user's gallery. The handler now knows it has been disposed and checks after every await: nothing already fetched is dispatched, nothing new is scheduled, and an event delivered after teardown does nothing at all. Also, a duplicate delivery restarted the backoff from one second, so a stream of duplicates during an outage could keep starting fresh chains — the bound existed but nothing was bounded by it. The attempt count lives on the retry state now, and a duplicate resumes the chain where it had got to. * fix(ui): close the disposal escapes a self-review found before the next round A fresh adversarial pass over 042350c, done deliberately before the reviewer's next round, converged on his known categories. - An in-flight delivery disposed mid-fetch still ran $lastProgressEvent.set(null) after the gallery guards returned. That store is module-global across handler sessions, so a stale delivery resolving after a logout or account switch blanked the progress event the new session had put there. Guarded, and the disposal test now asserts the store is never touched. - The DTO fetch loops kept issuing lookups after disposal — requests under the replacement session's credentials, cache writes into its store. Both loops now stop. - Three load-bearing pieces had no test that failed without them, and test-insensitivity is where review rounds keep coming from: - the video path's post-fetch disposal guard (every disposal test used image events; its mutation survived the whole suite); - the reveal controller's resolve-window hold for an unpainted claim (deleting it silently re-created the swallowed-click failure); - the settle-listener registration in store.ts (every listener test builds its own store; deleting the registration failed nothing). Each now has a test pinned via mutation from a verified cwd — two of this session's mutation runs previously "passed" by running against a path that did not exist. * test(ui): mount the reveal wiring in a real DOM instead of grepping for it JPPhoto's round-7 finding, and a fair one: the preview components' tests inspected source text, so a wiring or ordering regression could leave the overlay or the reveal broken with every test green. The package had no DOM test environment at all. The wiring the two components shared — one controller per mount, run on every input change with a cleanup that cancels only the timer, the flag lowered on unmount — moves into useSelectedItemReveal, and the video's painted-name readiness into usePaintedItemName beside it. Both components become a hook call; the hook is mounted under happy-dom (new dev dependency) with real effect lifecycles and mutation-verified coverage for exactly the things source text cannot see: - the image -> video component swap over the shared ref, including the outgoing component's timer being cancelled before it can cut the incoming reveal short; - unmount lowering the flag with no timer left to re-raise it; - StrictMode's double-invoked effects; - readiness driven by a real <video> element's loadeddata event, and reset when the element is swapped for another video. The remaining source-text assertions shrink to what they are good for: pinning that the components actually call the tested hook, with the right readiness expression on each path. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Jonathan <34005131+JPPhoto@users.noreply.github.com> Co-authored-by: JPPhoto <jpollack@jpollackphoto.com>
1 parent 8e71a8a commit e431d24

26 files changed

Lines changed: 3190 additions & 116 deletions

invokeai/frontend/web/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@
142142
"eslint-plugin-storybook": "^10.3.6",
143143
"eslint-plugin-unused-imports": "^4.4.1",
144144
"globals": "^16.5.0",
145+
"happy-dom": "^20.11.6",
145146
"knip": "^5.77.4",
146147
"magic-string": "^0.30.21",
147148
"openapi-types": "^12.1.3",

invokeai/frontend/web/pnpm-lock.yaml

Lines changed: 72 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
import { configureStore, createListenerMiddleware } from '@reduxjs/toolkit';
2+
import type { AppStartListening } from 'app/store/store';
3+
import { autoSwitchedImages } from 'features/gallery/store/autoSwitchedImages';
4+
import {
5+
boardIdSelected,
6+
gallerySliceConfig,
7+
imageSelected,
8+
selectionChanged,
9+
} from 'features/gallery/store/gallerySlice';
10+
import { beforeEach, describe, expect, it } from 'vitest';
11+
12+
import { addAutoSwitchedSelectionListener } from './autoSwitchedSelection';
13+
14+
// A store with the real gallery reducer and the real listener, so the predicate is exercised
15+
// against actual selection-writing actions rather than a hand-built state pair.
16+
const buildStore = () => {
17+
const listenerMiddleware = createListenerMiddleware();
18+
addAutoSwitchedSelectionListener(listenerMiddleware.startListening as unknown as AppStartListening);
19+
return configureStore({
20+
reducer: { gallery: gallerySliceConfig.slice.reducer },
21+
middleware: (getDefaultMiddleware) => getDefaultMiddleware().prepend(listenerMiddleware.middleware),
22+
});
23+
};
24+
25+
describe('addAutoSwitchedSelectionListener', () => {
26+
beforeEach(() => {
27+
// The marker is a module singleton; drop anything a previous test left on it.
28+
autoSwitchedImages.settle(null);
29+
});
30+
31+
it('keeps the marker when the auto-switch selection lands', () => {
32+
const store = buildStore();
33+
autoSwitchedImages.record('a.png');
34+
store.dispatch(imageSelected('a.png'));
35+
expect(autoSwitchedImages.consume('a.png')).toBe(true);
36+
});
37+
38+
it('drops the marker once the user selects something else', () => {
39+
// The dead click this exists to prevent: the auto-switch to A never rendered because the user
40+
// clicked B first, so their later click on A must still get its reveal.
41+
const store = buildStore();
42+
autoSwitchedImages.record('a.png');
43+
store.dispatch(imageSelected('a.png'));
44+
store.dispatch(imageSelected('b.png'));
45+
store.dispatch(imageSelected('a.png'));
46+
expect(autoSwitchedImages.consume('a.png')).toBe(false);
47+
});
48+
49+
it('settles on every action that writes the selection, not just imageSelected', () => {
50+
const store = buildStore();
51+
52+
autoSwitchedImages.record('a.png');
53+
store.dispatch(imageSelected('a.png'));
54+
store.dispatch(selectionChanged(['b.png']));
55+
expect(autoSwitchedImages.consume('a.png')).toBe(false);
56+
57+
autoSwitchedImages.record('c.png');
58+
store.dispatch(imageSelected('c.png'));
59+
store.dispatch(boardIdSelected({ boardId: 'other', select: { selection: ['d.png'], galleryView: 'images' } }));
60+
expect(autoSwitchedImages.consume('c.png')).toBe(false);
61+
});
62+
63+
it('leaves the marker alone when an action does not move the selection', () => {
64+
const store = buildStore();
65+
autoSwitchedImages.record('a.png');
66+
store.dispatch(imageSelected('a.png'));
67+
// Selecting the same item again, and a board switch that carries no selection, must not
68+
// discard a marker whose image has not rendered yet.
69+
store.dispatch(imageSelected('a.png'));
70+
store.dispatch(boardIdSelected({ boardId: 'other' }));
71+
expect(autoSwitchedImages.consume('a.png')).toBe(true);
72+
});
73+
});
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
import type { AppStartListening } from 'app/store/store';
2+
import { autoSwitchedImages } from 'features/gallery/store/autoSwitchedImages';
3+
import { selectLastSelectedItem } from 'features/gallery/store/gallerySelectors';
4+
5+
/**
6+
* Keeps the auto-switch marker scoped to the selection it was recorded for.
7+
*
8+
* onInvocationComplete records the item it is about to auto-switch to, so the viewer's reveal
9+
* effect can tell that handoff apart from a user's gallery click. The marker is only meaningful
10+
* while that selection stands: once the selection moves on, the recorded auto-switch will never
11+
* render, and leaving the marker behind would make the user's next click on that item read as an
12+
* auto-switch and get no reveal.
13+
*
14+
* Matched by state rather than by action type on purpose — the selection is written by several
15+
* reducers (imageSelected, selectionChanged, boardIdSelected, comparedImagesSwapped,
16+
* showVirtualBoardsChanged, logout), and a new one added later would silently escape an
17+
* action-type list, leaving exactly the stale marker this exists to prevent.
18+
*/
19+
export const addAutoSwitchedSelectionListener = (startAppListening: AppStartListening) => {
20+
startAppListening({
21+
predicate: (_action, currentState, previousState) =>
22+
selectLastSelectedItem(currentState) !== selectLastSelectedItem(previousState),
23+
effect: (_action, { getState }) => {
24+
autoSwitchedImages.settle(selectLastSelectedItem(getState()) ?? null);
25+
},
26+
});
27+
};

0 commit comments

Comments
 (0)