[dyphire]

Need kmworks/kmrs#150

Surface the kmrs smart-list API: user-owned search filters created and edited through a two-step wizard (details, then filters inline), evaluated live by the server.

• smartListsApi + SmartList DTOs; SSE wiring for the smart-list events
• new 'entities' filter group kind (libraries / read lists / collections) with multi-select and is/is-not membership; referential options in the wizard follow the selected scope
• /smart-lists browse page and /smart-lists/:id detail pages for both targets, reusing the browse grid/filter bar; page-side filters post as an overlay body; owner-scoped edit/delete (new isOwner permission idiom); read/peek entry points on book lists
• two-step create/edit wizard reusing the filter building blocks inline (no drawer overlay), with a live match-count preview, stored filter round-tripping and a warning when a document contains conditions the editor cannot represent
• PosterManager gains the 'smartlist' kind (upload/select/delete covers, generated mosaic listed alongside uploads)
• smart-list cards use the generated/selected cover with cache busting
• i18n: smartlists namespace in all 10 locales plus nav/filter/poster keys

[everpcpc]

Reviewed the full diff against the repo conventions; pnpm build (typecheck + bundle) and pnpm lint both pass on this branch, and I verified the smartlists key set is mirrored across all 10 locales (CLDR plural shapes accounted for). The API layer, SSE wiring, PosterManager smartlist kind, card structure, and the Dialog fill prop all match the existing idioms nicely. Two findings need attention, one of them a real round-trip bug.

1. (bug, verified) searchToFilterState loses match-all mode on round-trip. In the group-branch handling, the bucket is keyed by the raw condition key while absorbLeaf pushes values into the mapped group key:

const b = bucket(key as GroupKey)   // 'tag'
...
if (!absorbLeaf(key, op)) lossy = true   // values land in bucket('tags')

The mode (b.mode = mode) is recorded on the wrong bucket, and the flush then pushes the invalid key 'tag' into state.matchAll — mode('tags') in the builder never sees it. I reproduced it with esbuild against this branch: tags=[manga, josei] + matchAll=['tags'] → buildBookSearch emits allOf, but searchToFilterState returns matchAll: [] with lossy: false, and re-emitting produces anyOf. So opening a list whose filter uses match-all in the wizard and saving it silently flips those groups to match-any, with no warning. Same for every mapped key (genre/publisher/sharingLabel/language/ageRating/mediaProfile/mediaStatus/oneShot/collectionId/readListId/author). Fix: resolve the condition key → GroupKey mapping once and use it for both the bucket and absorbLeaf.

2. (design mismatch) The wizard gates visibility to admins, but the API gives it to every owner. kmrs#150 lets any authenticated user share their own lists: share-targets is open to all, and create/PATCH accept visibility/sharedWithUserIds from any owner. Here the visibility picker renders only for admin and the body strips it otherwise (...(admin ? {...} : {}) with the "visibility is an admin-only concern" comment), so a non-admin owner can never create a PUBLIC/SHARED list through the UI — the sharing feature is unreachable for exactly the users it was built for. If admin-gated sharing is a deliberate product call, it belongs in the PR description (and arguably the API should reject non-admin visibility too); otherwise the gate should be ownership.

3. Admins get a broken "Manage posters" entry on other users' lists. canManage = isAdmin || isOwner gates the whole menu, but kmrs thumbnail writes are require_owner — an admin opening the PosterManager on someone else's list gets 403 on upload/select/delete. The posters item needs its own isOwner gate; edit/delete are correctly owner-or-admin.

4. SERIES detail passes SERIES_FILTER_GROUPS unfiltered. Non-admins see the admin-only deleted group; both existing series-side callers (SeriesGrid, collection detail) filter !g.adminOnly || admin.

Smaller items:

• useFilterStateState: the name stutters, and its doc comment is glued to the export (*/export function); same glitch in FilterGroups.tsx where LetterOptions's signature line absorbed const active.
• smartlists:editFilters is added to all locales but never referenced.
• The continue-target query key isn't invalidated by ReadProgressChanged, so the Read button's target can go stale after finishing a book until the page remounts.
• The step-2 preview fires a /list request per keystroke with no debounce (cheap size=1, but a 200–300 ms debounce would match the search-input pattern used elsewhere).

Non-issues checked: PWA worker bypass unaffected (everything under /api); usersApi.list correctly gated enabled: admin; the card's fallback for a missing visibility field on older kmrs builds is a nice touch.

[everpcpc]

Taking over the PR to land the review fixes directly — pushed 5f1a709. All four findings are addressed, and the round-trip fix turned out to run deeper than the bucket-key mix-up from the review:

1. Filter round-trip is now actually lossless. Beyond the mode/values bucket mismatch, verifying the fix empirically surfaced two more latent bugs in the same function:

• yearFromBranch compared the after-bound's year against itself (off by one), so it never matched any canonical pair — every stored release year flagged lossy. Fixed the bounds arithmetic.
• A single-group document (e.g. only match-all tags, or a single release year) is the group's own shape at the top level, but the parser always split the top-level allOf into single-leaf branches — losing the mode and flagging years lossy. The parser now recognizes whole-condition same-key anyOf/allOf runs and year-pair runs as one group branch.

Verified with an esbuild harness against this branch (the repo has no test targets): 13 round-trip cases — match-all/match-any tags, genres, authors, collections, negated groups, libraries, single/multiple/match-all release years, and a full combo — all preserve the emitted condition exactly, and 5 lossy-positive cases (mixed is/isNot in one group, unknown keys, inner mixed-key groups, wrong-target membership, non-canonical date bounds) all flag lossy so the editor warning shows instead of silently rewriting semantics. One behavior change worth noting: mixed is/isNot leaves in a group now flag lossy (the doc comment always claimed this; the code previously flipped the whole group to isNot).

2. Admins no longer see "Manage posters" on other users' lists — the menu item is gated on ownership, matching the API's owner-only cover writes; edit/delete stay owner-or-admin.

3. SERIES detail filters adminOnly groups for non-admins, same as SeriesGrid and the collection detail.

4. shareTargets() is only fetched for admins — the endpoint is admin-only since kmworks/kmrs#152, so the unconditional query would 403 for every regular user opening the wizard.

Also cleaned up: useFilterStateState renamed to useFilterState, the two glued-line formatting slips, and the unused smartlists:editFilters key in all 10 locales. pnpm build and pnpm lint pass.

[everpcpc]

Follow-up 3c9f67c: smart-lists queries now invalidate on ReadProgress(Series)Changed/Deleted (the items grid and the Read button's continue target both depend on read progress — the continue-target staleness from the review), and the wizard's match-count preview is debounced 300ms like the browse search input. Build + lint green; CI passed on the previous commit.

[everpcpc]

Holistic review of the PR as it now stands (original commit plus the three fix commits I pushed). CI green on every commit, locale parity verified after the editFilters removal, pnpm build/pnpm lint pass.

Verdict

The feature is complete and coherent with the merged backend (kmworks/kmrs#150 + #152). All round-1 findings are addressed, and the round-trip parser is now verified lossless across the entire builder vocabulary.

What holds up

• The two-step wizard reuses the browse filter building blocks verbatim (same group defs, same builders), so the stored document is by construction the same shape a browse page would emit — that symmetry is what makes lossless round-tripping possible at all.
• Overlay evaluation on the detail page maps exactly onto the server's AND-with-stored-filter semantics, including the full-text replacement rule.
• The permission surface now matches the API everywhere: create for all (always private), visibility admin-only (shareTargets fetched only for admins), edit/delete owner-or-admin, covers owner-only.
• SSE wiring covers list changes, thumbnail busting, and read-progress freshness.

The round-trip parser, after the fixes

Verified with an esbuild harness (the repo has no test targets): 13 round-trip cases preserve the emitted condition byte for byte (match-all/any tags, genres, authors, collections, negated groups, libraries, single/multi/match-all release years, full combo), and 7 lossy-positive cases all warn instead of silently rewriting (mixed is/isNot per group, unknown keys, inner mixed-key groups, wrong-target membership, non-canonical date bounds, repeated bare keys in a hand-crafted top-level allOf). The last guard — repeated bare keys — was pushed as ac90fa1: such documents are outside the builder's vocabulary, and parsing them as match-any would have silently turned AND into OR.

The contract the parser now fulfills: anything the wizard can produce round-trips exactly; anything else shows the lossy warning. Nothing from my side remains open.

Remaining accepted behavior (documented in the PR): documents with constructs outside the builder's vocabulary show the lossy warning on edit and are simplified on save.