Collections groups - #78
Merged
Merged
Conversation
…ites Two data-integrity problems in the collections API. 1. Group payloads could override the collection's own membership list. Because normalizeCollectionGroups() ran without an allow-list on the branch where the request carried `groups`, and flattenCollectionModelIds() then unioned group members back into modelIds, a client shipping a stale groups array silently resurrected models the user had just removed - and re-hid them in their munchie sidecars. modelIds is now the single source of truth. Groups may only reference models already in the collection; anything else is a 400 rather than a silent mutation. 2. loadCollections() normalized every collection on every read, and each mutating endpoint then wrote the whole array back. A write to one collection therefore rewrote all the others on disk: renaming a collection injected groups:[] into its neighbours, and any legacy or hand-edited field elsewhere in the file was silently reshaped by an unrelated request. loadCollections() now returns stored data verbatim; response shaping moves to toCollectionView(), which does not persist, and group ids are minted on the write path only - so GET /api/collections is idempotent instead of handing out fresh random ids for id-less groups on every call. Also on the write path: - reject a model claimed by two groups instead of deleting the second group - keep a group that has lost its last member, so its name and description survive - build group objects key by key instead of spreading, so arbitrary client keys (including unbounded nested group trees) can no longer be persisted - cap group count, members per group, and name/description length - treat an absent modelIds as "preserve" and an empty array as "clear", so a plain rename no longer wipes membership and unhides every member Rewrites the groups test, which encoded the old union behaviour, and adds coverage for each rule above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
getCollectionCoverImages() returned member thumbnails first and only fell back to collection.images when no member had a thumbnail, so the collage always won for a non-empty collection. A cover uploaded through the edit drawer's "Add Images" control was still stored but could never be seen again. Cover resolution is now explicit: an uploaded image, then a member picked as coverModelId, then the generated collage, then the folder placeholder. This wires up resolveCollectionCoverImage(), which the collage had left unused. getValidCollectionCoverId() no longer falls back to modelIds[0]. Inventing a cover on nearly every save made "no cover chosen" indistinguishable from a deliberate choice; it now only keeps a selection that still points at a member. Cover images also carry the collection name as alt text again, which is what the existing CollectionCard test asserts - it has been failing since the collage landed with alt="". Finally, the sorted list branch in ModelGrid passed the filtered `models` array to CollectionListRow where the other three call sites pass `collectionModels`, so collection covers degraded in list view whenever a sort was active. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ment The new collection filter is held in lastFilters, which survives view changes, but openCollection() only reset the sidebar's own controls. Three problems followed from that. Deleting the collection you were filtering by left its id in lastFilters. The dropdown fell back to displaying "All Collections" while still emitting the missing id, so every model was filtered out and stayed out - Clear Filters was the only way back. An effect now drops a collectionId that no longer resolves. Entering a collection showed the full collection while the sidebar displayed the library's search, category and tags, and the first touch of any control collapsed the grid. openCollection() and the back action now reset lastFilters to the configured defaults, so the controls and the results always match. The two refresh paths reapplied lastFilters.collectionId inside collection view, where the sidebar deliberately hides that filter, which could intersect the open collection with an unrelated one. Both now scrub it, alongside the existing fileType scrub. Also: refreshCollections() reports success instead of swallowing every error, so the upload dialog can tell the user when their files landed but the grid did not refresh, and drops the dead collection-created listener, which had no dispatcher anywhere in the app. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collections can now hold named groups, so a set of scaled or near-identical variants collapses to a single expandable row instead of filling the grid. Adds a Collection filter to the sidebar, generated collage covers for collections without a chosen cover, and an "add to collection" step in the upload dialog. Grouping is offered alongside collection creation rather than replacing it. The selection toolbar inside a collection had its Collection action relabelled to Group, which removed both "create a new collection from this selection" and the move-to-another-collection flow with no replacement; both are back, and the group action is now its own button. A group with no members stays on screen with an empty state, matching the server keeping empty groups, so it can still be renamed or ungrouped instead of vanishing. Group thumbnails carry a Group badge in the same position collection covers use theirs, so a group is not read as an unlabelled collection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
README gains a Collections and groups section explaining how the two differ - a collection is global, filterable and hides its members, while a group is presentational and scoped to one collection - plus the collections endpoints and the updated filter list. CLAUDE.md records the invariants a contributor would otherwise have to rediscover: modelIds is authoritative, empty groups are kept, read paths must not normalize, and the cover precedence order. AGENTS.md was a verbatim copy of CLAUDE.md with the tool name swapped, already missing everything above. It now points at CLAUDE.md so the two cannot drift. Also refreshes the in-app release notes dialog for the 0.18 line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Expanded groups rendered full ModelCards in a three-column grid, which fought the point of grouping: the variants inside a group differ by little more than scale, so large cards repeated the same thumbnail and description while pushing the rest of the collection off screen. Groups now render rows. The list row markup was duplicated twice inside ModelGrid, so it is extracted to ModelListRow and shared by all three call sites - the two list-view branches keep the existing full-size layout, and groups use a new compact variant: 56px thumbnail, tighter padding, stats on one line, and no description or tags, since those are identical across variants and are what made the cards redundant. Net effect on ModelGrid is 254 fewer lines with no change to its rendering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Lockfile-only bump via `npm audit fix --package-lock-only`; package.json is untouched, so every change stayed inside the declared semver ranges and no direct dependency changed major version. Clears both criticals (vitest UI arbitrary file read, node-tar PAX override) and 13 of 14 highs, including the runtime-facing ones: multer 2.1.1 -> 2.2.0 (DoS via deeply nested fields), fast-xml-parser 5.5.8 -> 5.10.1, and the express chain (body-parser, type-is, qs, content-type, form-data). Build and test tooling moves too: vite 6.4.1 -> 6.4.3, vitest 4.0.15 -> 4.1.10. fast-xml-parser 5.10 splits its internals into anynum, xml-naming, is-unsafe, fast-xml-builder and @nodable/entities, which is why the lock gains packages; all are published by the fast-xml-parser author's org. Deferred: 5 advisories (1 high, 4 moderate) remain, all inside the npm CLI bundled into semantic-release's dependency tree. Clearing them needs `--force`, which downgrades semantic-release 25.0.2 -> 24.2.9 - a major change to the release pipeline that generates our versions and CHANGELOG. Not worth that risk: semantic-release is a devDependency that only runs in CI, and the production image installs with `npm ci --only=production`, so none of it ships. Verified: full suite green on the new versions (41 files, 336 passed); 3MF parsing checked directly against three real files for names, designers, thumbnails, print settings and MD5; and a real multipart upload through multer saved its file and generated metadata. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🎉 This PR is included in version 0.18.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Builds on @25hash's collections/groups work from #77, with follow-up fixes rather than a review round-trip. Their two commits are preserved at the base of this branch with authorship intact — please merge with "Create a merge commit", not squash, or that attribution is lost.
Closes #77.
What this adds
Follow-up fixes on top of #77
Reviewed with two audit agents (server API, frontend regressions) plus live-browser verification against a copy of a real library. 14 issues found; all fixed.
Data integrity
modelIdsis now authoritative for group membership. Agroupspayload could previously override it — a client sending a stale groups array silently resurrected models the user had just removed, and re-hid them in their munchie sidecars. Out-of-range group members are now a 400 instead of a silent mutation.loadCollections()normalized on every read and every endpoint wrote the whole array back, so renaming one collection rewrote all the others on disk.GET /api/collectionsis now idempotent too (it was minting fresh random group ids per call).Regressions
coverModelId→ collage → folder icon.Also
ModelGrid, so it's extracted toModelListRowand shared by all three call sites —ModelGridloses 295 lines with no change to its rendering.Groupbadge, matching how collection covers are labelled.fix(deps): lockfile-only, clearing 22 of 27 audited advisories including both criticals.package.jsonuntouched — nothing left its declared semver range.Deferred
5 advisories remain (1 high, 4 moderate), all inside the npm CLI bundled into
semantic-release's tree. Clearing them needsnpm audit fix --force, which downgradessemantic-release25.0.2 → 24.2.9 — a major change to the pipeline that generates our versions and CHANGELOG. It's a devDependency that only runs in CI, and the production image installs withnpm ci --only=production, so none of it ships. The real fix is upstream in@semantic-release/npm.Testing
CollectionCardalt text, now fixed).GETbyte-identical across calls, unrelated collections unchanged after a write, cover precedence.Note on CI
.github/workflows/only builds the Docker image on PRs — vitest never runs. That's why the failing test on #77 wasn't visible as a red check. Worth adding a test job separately.Release impact
commit-analyzerreturnsminorfor this branch, so merging cuts v0.18.0. The in-app release-notes dialog has been updated for the 0.18 line and keys offpackage.json, so it will surface automatically.