6096dd69273cbe0d76972f3610ce2632daf9191b
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
e4ffd59ca7 |
fix(e24): review round 2 — a buried history entry, a stale hero, and a spinner that was not ours
Nine of the round's ten surviving findings. The tenth (a failed shelf reorder leaving an order the server rejected on screen) is on ROADMAP.md: its obvious remedy reintroduces the overlapping-move defect the code already documents, so it wants a design, not a review edit. - `navigateOverOverlays` buried the live overlay entries before knowing whether the navigation pushed one. `setPage` no-ops when the page is already current and Home is the default landing page, so Shift+W over a player ON Home marked its entry buried under a push that never happened: the unmount consumed the mark instead of reclaiming the entry, and the next browser Back was a press that did nothing. `setPage` now reports whether it navigated, and one `buryUnder` rule serves both entry points — `closeThenNavigate` takes the same no-op navigation from a palette command for the page you are already on. - The hero could answer a new time budget with the previous one's pick. The chip row resets seed/seen/press but cannot reset an answer that already resolved, and `keepPreviousData` chained through the disabled seed-0 query — so a reroll under "10 perc" showed the film picked for "Bármennyi" while it loaded, and kept it if the request failed. `lastPick` now carries the server's own budget echo, and the placeholder is scoped to the budget. - The reroll button spun and swallowed clicks after mark-watched, bookmark, pin and shelf moves: the pick key sits under the invalidated `discover` prefix on purpose, so raw `isFetching` was true for background refetches. It now means "a press of mine is outstanding" — which still covers the first press, where there is no placeholder. - Failed writes on Home toasted the page's LOAD-failure string over a fully loaded page; they get their own key (HU+EN). The empty-state reroll had no `loading`, so the press was unacknowledged and a second click skipped a tier. `danger` moved into the shared unpin row so the same command is not red on the shelf and neutral in the rail below it. - Two comments claimed things the code does not do: the `exclude` cap guards no "unbounded scan" (it never reaches SQL), and the route test that named the cap sends three keys. Tests: the burial rule gets its own file (it is the one assertion that lets a reconcile issue a real programmatic back()); App's keymap wiring gets a source contract, because dropping it left all 818 tests green; `add_pin`'s IntegrityError arm was reached by nothing. Every new test mutation-checked red, both edges where the predicate has two. Gate: ruff, ruff format, 3x tsc, eslint (0 errors), prettier, knip, vitest 822, pytest 1005. |
||
|
|
3709b71a94 |
fix(e24): the review's low findings — dead constants, honest labels, a 500 that should be a 404
`add_pin`'s IntegrityError handler assumed the unique constraint was the only way in and re-raised otherwise, so a target row pruned between `resolve_ref` confirming it and the INSERT (a Plex sync dropping a removed item) tripped the FK and handed the user a 500. Semantically that is the same "no such thing" the pre-check already returns. Untested: the race needs two concurrent transactions. The five per-kind unique constraints had no test at all — the route's own dedupe returns before reaching the DB, and the migration drift check filters constraint diffs out as noise, so they could go missing with every lane green. Frontend, all from the same review: the shelf's unpin row is now `danger` like every other destructive row in the app; the hero's artwork button says what it will actually do and names the item instead of always announcing "Play"; the empty-budget message offers a reroll rather than `StateMessage`'s "Retry", which promised a re-run of something that had not failed; `setState` and the watch-later toggle catch their rejections instead of leaving an unhandled promise and a click that silently does nothing. `StateMessage` gained a `compact` rung, which retires the frontend's only two `!important` overrides — they existed to beat the primitive's own padding from the call site, and were the copyable idiom for the next surface that wanted a compact empty state. Dead vocabulary removed or made load-bearing: `DISCOVER_BUDGETS` was exported, re-exported and consumed by nothing while `Home.tsx` re-typed the same four bands (a `readonly DiscoverBudget[]` accepts any subset, so a fifth band would have typechecked green and rendered no chip); `PIN_KINDS`' value export had no consumer at all; `useComfortPin` no longer publishes `pinIdOf`/`toggle`, which nothing outside the module calls. Three comments corrected against the code they describe: a pointer to a `resolve.py` that was never written, the file header's claim that Home defines no card component of its own, and the `seen` window still quoting the 8 that was measured and abandoned for 32. The rest of the review's survivors are logged on ROADMAP.md under "Deferred — E24 epic-end deep-review (2026-09-08)" — the ones whose dissent was substantive, and the ones whose fix is wider than the finding. |
||
|
|
ccd5f3739a |
feat(e24): pin to the comfort shelf from every card and row
S3 wires the shelf's editor. The gesture is the E10 right-click menu, so the decision "this is comfort media" can be made where the thing is — the feed, a Plex grid, a channel row, a playlist — rather than only on the page that lists the shelf. One shared piece, `components/comfortPin.tsx`: the five pin-kind constructors, the shelf query, and ONE `ActionMenuItem` that switches Pin/Unpin honestly (and says "not known yet" with a disabled row until the shelf has answered, since a confident "Pin" over something already pinned is exactly the quiet lie the house rules forbid). Surfaces that already have a row menu compose the item in (`Playlists`, `PlaylistsRail`); the rest mount `ComfortPinMenu`. Two structural notes: - The feed's menu lives in `VirtualFeed`, not in `Feed` — that is the list, and it is also what gives the channel page the gesture for free, since it renders `<Feed channelScoped>` all the way down. - Cards take `menuProps` as a FUNCTION of the row, not the built props object. `triggerProps` is stable across renders while its return value is not, and a fresh object prop would blow past `VideoCard`'s memo for every card in the virtualized feed on every parent render. `DataTable` and `ChannelLayoutGrid` grow a generic `rowProps` so a channel row answers the same gesture in both of the manager's layouts; the table stays domain-free and never learns what a menu is. On Home the shelf tile's menu is the whole editor: four move rows (front / left / right / end, reusing the playlist menu's ordering idiom) plus the unpin, over menu rows rather than drag & drop — dragging inside a horizontal, scrolling Carousel fights the scroll, and menu rows are keyboard-reachable for free. The reorder is optimistic on the cached payload and re-asks the server on failure rather than showing an order that was never accepted. The exhausted shelf tile (a pinned channel with nothing unwatched left) is `aria-disabled`, not `disabled`: measured in Chromium, a disabled button still receives contextmenu and pointerdown, but it loses focus — and with it Shift+F10, the only keyboard route to the menu it most needs. Verified end to end in the browser against the live account: all five pin kinds, the reorder (persisted across a reload), the unpin, and the honest Unpin label on a resume tile that is already on the shelf. |