fix(ui): model + nodes manager correctness, QOL, and legacy parity - #81
Merged
Conversation
- pure compatibility rule + curated any-base allowances in core/relationships - shared relationships store replaces per-component fetches - related-models section now shows for every linkable type
The strings.ts extraction added a new source owner to both initial graphs, and installsStore importing getInstallSourceLabel from core/taxonomy split taxonomy into its own chunk, growing both entry chunks past the byte budget. - fold toTitleCase into core/baseIdentity (already in both initial graphs) - house getInstallSourceLabel in data/installsStore; UI imports it from there
A link/unlink response can outlive the section that issued it (the pane remounts per model key), so UI pending state cannot serialize the cache. - per-pair mutation stamps: only the latest mutation's response commits - deletion tombstones: a response settling after delete cannot resurrect keys
The equal-base rule rejected pairings the loaders accept (Z-Image+FLUX VAE, Krea-2+Qwen-Image/Anima VAEs, Flux2+FLUX VAE, Anima's VAEs, Z-Image+FLUX PiD decoder), and control_lora/flux_redux/pid_decoder could not link at all. - add CROSS_BASE_ALLOWANCES, cited per backend invocation like the null-base table - append the three concrete-based helper types to LINKABLE_TYPES
StarterSortField = Exclude<ModelSortField, 'size'> replaces the silent no-op 'size' arm and the comment-enforced menu filter: the filter menu is now generic over the sort-field subset, so offering or reporting 'size' to the starter list is a type error.
joshistoast
requested review from
JPPhoto,
blessedcoolant and
lstein
as code owners
August 9, 2026 05:29
Narrowing the model library filter and resetting it to All left the list rendering the filtered number of rows with the unfiltered list's content. Root cause (confirmed by probe): react-hook-tanstack-virtual applies option changes — including count — via setOptions in a layout effect, but virtual-core's setOptions never notifies, so nothing re-read the useSyncExternalStore snapshot until the next scroll, resize, or element-measure notification. A synthetic scroll recovered the list, proving the new count had landed and only the notification was missing. Growing lists froze at the old range; shrinking only looked right because out-of-range indexes render null. Fix the wrapper itself via a pnpm patch: after setOptions, call the virtualizer's memoized maybeNotify, which recalculates the range and notifies exactly when it changed — a no-op on every other commit, and safe on mount (listeners subscribe later and sync is false). This covers every virtualized surface, including ProjectsBrowser and GalleryImageGrid, which attach no measureElement refs and carried the same latent bug; lists with measureElement refs were only masked by row remounts re-notifying. The regression test drives the real ModelLibraryList through filter narrow/reset in the browser.
Installing a model from folder-scan results flipped the row to Installing while the job ran, but reverted to the Install button once it finished: the row's installed state came solely from the scan-time is_installed snapshot stored in the UI store, which nothing ever refreshed. HuggingFace file rows had the same staleness with no installed state at all. Source rows now also match against the live library — each model's recorded install source plus its resolved absolute path (shared helper mirroring the backend's models_path/model.path resolution, now also reused by the detail pane's path display). The badge flips to Installed the moment the post-install library refresh lands, covers copy installs via the recorded source, and the scan summary and install-all counts stay honest as the queue drains. The regression test walks a scan row through Install -> Installing -> Installed against the real stores.
joshistoast
changed the title
The busy-tracked account-scoped action wrapper has zero models-specific content — it imports only the account lifecycle and react — and the nodes manager needs the same semantics (including the re-entry guard) at five call sites. Move it beside useMountEffect; models call sites update import paths only.
The pack store shared the same absorbed-mid-flight-refresh bug the four model-manager stores were cured of: a refresh requested during a flight returned the stale in-flight promise and its data was lost. Adopt createTrailingSingleFlight, let ensureCustomNodePacksLoaded retry after an error and share the in-flight request, and export the snapshot getter the maintenance menu's explicit-refresh surfacing will need.
A failed install wrote only to the activity log, which defaults to collapsed — paste a bad URL and the spinner just stopped. Outcomes now toast: success includes the imported-workflow count the response always carried but nothing displayed, failures show the backend's message, and the requires-dependencies warning is sticky (it demands a manual pip install and restart; legacy pinned it too). The log's installing entry also resolves in place now — previously a second entry was appended and the first stayed installing forever, leaving the activity bar spinner running after the first install.
The Add tab's two sub-tabs over-promised: 'Scan Folder' scanned nothing — it was an info card showing the custom-nodes path. The tab is now one view: a validated Git-URL install with the manual drop-in path as a quiet card below. Validation mirrors the backend's pack-name rules and already-installed rejection, so a doomed source shows an inline error instead of a failed request. The typed source moves into the nodes UI store — the detail tabs unmount their content, and a tab flip used to discard the URL and hide a running install; busy state now derives from the shared install log so it survives remounts too. The manager defaults to the Add tab (models precedent), and the library's empty and search-miss states gain a CTA into it.
The reload endpoint's response body was discarded, so 'No custom nodes directory found.' toasted green. The status is prose, so only the backend's success phrasing earns a success toast; anything else surfaces as a warning with the reported text — unknown statuses fail honest, never fail green. Reload also adopts useScopedAction, closing its double-click window.
The library header offered only Reload. A quiet menu beside it adds an explicit list refresh — which finally surfaces the failures the store records silently while packs stay loaded — and copies the custom-nodes path for the drop-a-folder workflow. Reload keeps its own button: it is the manager's most frequent action and burying it a menu deep would regress the common path.
The pack list gains the models library's machinery scaled to its axes: a filter/sort menu (name, node count, path; reselect flips direction) plus a 'packs with no nodes' filter, with the pure policy extracted to core alongside unit tests. Zero registered nodes is the strongest health signal the catalog carries — the pack's import failed or a restart is pending — so those packs now show an orange badge with a hint in both the list row and the detail header, instead of a neutral gray count. The library column also adopts the models manager's fluid width (clamp(22rem, 32vw, 28rem)) in place of the fixed 22rem, with keep-in-sync comments on both features' layout constants.
Uninstalling a pack also deletes the workflows it imported at install time, and the confirm dialog never said so. A shared UninstallPackDialog (consolidating the two duplicated dialogs) fetches a best-effort count via the pack's workflow tag and states the deletion; the copy degrades to the generic body when the fetch fails or finds none. useNodePackActions moves onto useScopedAction — its hardcoded English toasts become i18n keys, and the context-menu uninstall finally gets busy feedback through the dialog's pending confirm. The context menu also gains Copy path.
Deletes the stray duplicated eslint-disable footer the nodes feature's files all carried (the only such files in the repo), finishes the feature's i18n (NodePreviewCard strings, the editor node tooltips that rendered raw English despite existing keys), moves plusMore from the models namespace to common (both consumers updated), drops the dead addCustomNodes key, hoists DetailPane's duplicated pack lookup, and gives NodesPage the code-split docblock its models sibling carries.
Reload and 'Refresh pack list' sat side by side doing overlapping things: reload re-scans the directory and refetches the list, refresh only refetched. The distinction reads as code trivia, not UI. The maintenance menu is gone; Reload is the single affordance and now also surfaces a swallowed refetch failure instead of celebrating over a stale list. The copy-path affordance moves to the Add tab's directory card (each pack row's context menu already has its own).
The nodes manager pass grew en.json past the recorded non-script byte budget on every journey; the initial script graphs are untouched.
joshistoast
changed the title
The Source row rendered URLs and HuggingFace repo ids as inert text. It now links out when a page exists — the URL itself, or the huggingface.co page derived from a repo-id source (qualifiers stripped) — with a quiet external-link icon; local paths stay plain text. The user-editable Source URL (e.g. a Civitai listing) was only visible inside the edit form; it gains its own linked row when set, with pre-validation records degrading to text. The attributes list was chosen over linking the model title (an identity element users expect to select, and a hidden destination) or a link squeezed under it (the header already carries badges and the path).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TL;DR: Model-manager overhaul in three passes — the original link-correctness + cache-race work, a sweep of data-layer correctness fixes and QOL (installs now progress and toast app-wide, Launchpad bundles open in the manager instead of blind-installing), and parity with the legacy frontend (checkpoint repair, bulk re-identify, settings export/import, and more) — plus a follow-on QOL pass over the nodes manager bringing it up to the same conventions. Two bugs found while testing are fixed at the root — including one via a patched dependency reviewers should note.
🔗 Related-model links: now backend-accurate
anybase is no longer a universal wildcard — it links only where a real pipeline consumes it (curated, invocation-cited allowance table)control_lora,flux_redux,pid_decoderare linkable now; the link picker hides itself for bases that can never link; fixed gemma2 + CLIP Vision allowances🧠 Relationships cache: race-proof
🩹 Data-layer correctness
allSettled— one failure no longer hides the rest; deleting a starter clears its "Installed" badge; manual refresh failures surface a toast; finished installs release their transient progress; relative paths likemodels/foo.safetensorsno longer classify as HuggingFace repos🚀 Launchpad + QOL
🧬 Legacy parity
upcast_attentioncheckbox is deliberately not ported — no current config class stores it, so PATCHing it is a silent no-op'default'sentinel) and LoRAweight_min/weight_maxwith validation mirroring the backend's model validator🐛 Fixed while testing
react-hook-tanstack-virtualapplies option changes in a layout effect but never notifies its external store, so a grown list stayed stuck until the next scroll. Fixed at the source viapatches/react-hook-tanstack-virtual.patch(registered inpnpm-workspace.yamlpatchedDependencies): the setOptions effect now fires the virtualizer's memoizedmaybeNotify. Covers all five virtualized surfaces, including two with the same latent bug. If the package is ever upgraded past 0.0.4, verify upstream before dropping the patch.🧩 Nodes manager: same love, smaller surface
useScopedActionlifted to@platform/react(it had zero models-specific content; nodes hand-rolled the same busy/scope boilerplate four times, double-submit hole included), the pack store adopts the trailing single-flight refresh, and the library column matches the models manager's fluid widtheslint-disablefooters (the only ones in the repo), remaining hardcoded English, dead keys, editor node tooltips that ignored existing i18n keys🌍 i18n & refactors (from the original pass)
StarterSortFieldmaking bogus sorts a compile error; API key cards split up; external provider configs get a real store; provider retry renders as loading, not a stale error✅ Verified
tsc, oxfmt, oxlint ✔️