Add the missing collapse trigger to the expanded PlexSidebar (state + collapsed rail
already wired via the shared filterCollapsed); mirrors the feed Sidebar. E2E-verified.
The collapse feature was only half-present on the Plex page: App already passed the
shared filterCollapsed state + toggle to PlexSidebar, and PlexSidebar rendered the
CollapsedFilterRail when collapsed — but the EXPANDED PlexSidebar had no trigger, so
you could only collapse it by first collapsing on the feed page. Add the same
ChevronLeft 'Filters' collapse header the feed Sidebar has (reusing the existing
sidebar.* i18n keys). E2E-verified: collapse→rail→expand round-trips on the Plex page.
- CollapsedFilterRail: the byte-identical 31-line collapsed sidebar rail was in
both Sidebar (feed) and PlexSidebar → one shared component.
- VideoCard: the title + channel-button + meta block was duplicated across the
list-row and card layouts → a local `textBlock` element (only one layout branch
renders, so reusing the element is safe).
- subsColumn<T>(t): the identical 'subscribers' DataTable column in Channels +
ChannelDiscovery → a generic factory in channelColumns.tsx.
All behavior-neutral; jscpd now reports 0 clones (was 8 at Phase 0 baseline).
tsc + knip green.
The standard input style was copy-pasted verbatim into 5 components
(DownloadCenter/DownloadDialog/ProfileEditor/ShareDialog/VideoEditor) and the
text-button style into 2 (ShareDialog/VideoEditor). Export both from ui/form.tsx
and import them. Byte-identical strings → no visual change.
The settings-family panels (Settings/Config/Setup/Welcome) use a DIFFERENT input
style (bg-card/rounded-xl/focus:border-accent); unifying the two is a design call
left to the glass-consistency epic, not this DRY extract.
Callback-ref the grid's IntersectionObserver sentinel so it re-attaches on the
grid's unmount/remount cycle (drill into info/show/season → Back), instead of
observing a detached old node and freezing the feed at the loaded pages.
The unified-library grid unmounts when you open an item's info/show/season page
and remounts as a NEW element on Back. The IntersectionObserver effect keyed only
on [hasNextPage, isFetchingNextPage, fetchNextPage] — none of which change across a
pure navigation — so it never re-ran: the observer kept watching the detached old
sentinel node and fetchNextPage never fired again, leaving the feed frozen at the
already-loaded pages (e.g. 80/1227) even though more exist. Intermittent because a
coincident isFetchingNextPage toggle around the nav could re-run the effect.
Fix: observe via a callback ref (state) instead of useRef, so the effect re-runs on
every sentinel mount/unmount and always watches the live node.
Closes the Plex #9 backend backlog (frontend was the prior follow-up). Behavior-
neutral cleanup + two two-way-sync bug fixes; PW2 verified a non-bug.
fix(plex): watch-sync PW1 pagination + PW3 lost-update guard
- PW1 watch_history: the history is a GLOBAL cross-account feed read as a single
500-row page, then filtered to the owner — on a busy family server the owner's
recent views can sit past page 1 and be missed. Now paginate (desc) until the
since-cutoff, bounded by max_pages (daily reconcile is the backstop; logs if hit).
- PW3 push_state_to_plex: it flagged synced_to_plex=True unconditionally after a
push, so a local edit landing between the push and the flag-set was marked clean
and never pushed (lost update). Capture the row's freshest timestamp when the
push is scheduled (_push_watch) and only settle the flag if the row is unchanged.
- PW2 was flagged as "accountID hardcoded to 1" but is NOT a bug: on a PMS account 1
is reserved for the owner/admin and an owner link always holds the admin token —
added a clarifying comment so it isn't re-flagged.
chore(plex): backend dedup
- PC2 unified_library + facets shared a ~13-field filter Query signature + p-dict →
one LibraryFilters dataclass injected via Depends(); as_dict() feeds the builders.
- PS-C1 sync.py collection upsert dup (resync_collection ≈ _sync_collections) →
_upsert_collection().
- PS-C2 sync.py 8-field filterable-metadata block dup (_apply_item ≈ _sync_shows) →
_apply_facet_fields().
- PW-C1 _repush_dirty read duration from the mirrored PlexItem.duration_s instead of
a per-row plex.metadata() round-trip.
- PW-C2 rk→id map built inline in two places → _rk_to_id() helper.
- PP1: reset cancelledMarkerRef on item change — a cancelled intro/credits
auto-skip on one episode suppressed a same-offset marker on the next (auto-skip
silently dead for the rest of a binge).
- PP2: loadSession no longer depends on the i18n `t` (read via a ref). A
mid-playback language switch rebuilt loadSession → re-ran the detail effect →
reloaded the session from the STALE saved resume position, losing live progress.
- PP3: tracks (audio/subtitle) menu now stopPropagation on wheel, like the gear
menu — scrolling the list no longer changes volume.
- PS1: collapsed PlexSidebar filter badge clamps to "9+" (was a raw count),
matching the feed Sidebar.
- format.ts: new formatRuntime() ("1h 30m"); PlexBrowse dur() + PlexInfo
fmtDur() were byte-identical copies, now both call it.
- PlexPlayer fmt() delegates to formatDuration (keeps the NaN/negative clamp
the live media clock needs); local reimplementation gone.
- Register LS.plexPlayerPrefs; PlexPlayer uses it instead of a bare string.
- Remove the dead PlexPlayerPrefs.wasPlaying field (written on play/pause, never
read) + its two patchPrefs writes.
- PlexBrowse item page: drop the redundant onStateChange={() => q.refetch()} —
PlexInfo.setState already invalidates ["plex-item", id], so q refetches itself.
- plex.py: delete the no-op _enabled() (never wired as a Depends/called; the real
gate is sysconfig plex_enabled).
- PlexBrowse.tsx: collapse toggleEpisodeWatched into toggleWatched — they were
byte-identical (both take a PlexCard).
- Delete 6 dead plex.json keys (loadMore, playerSoon, filter.library,
playlist.up/down/remove) from en/hu/de — verified 0 refs (the live
playlist.removeShow/removeSeason keys are kept).
Behavior-neutral. tsc green, ruff clean on touched files, localdev boots, Plex renders.
stream.py enforced a hardcoded _MAX_SESSIONS = 4 and never read the
DB-overridable plex_max_transcodes ConfigSpec, so the Configuration → Plex →
"Max concurrent transcodes" knob was dead (same env-vs-DB drift class as the
Downloads/Admin fixes). _enforce_cap(cap) now takes the cap from
sysconfig.get_int(db, "plex_max_transcodes") (>=1 so playback can't be capped to
zero). Bumped the config default 1→4 so the effective default matches the old
hardcoded cap (no behavior change for non-overriders; the knob now works).
From the auth /security-review (contained fixes; architectural SA3 proxy-trust +
SA4 session-revocation deferred to the user):
- SB1: encrypt the OAuth access_token at rest (was plaintext while refresh_token
was encrypted) — a ~1h Google bearer credential. New security.decrypt_optional()
falls back to a refresh for legacy plaintext tokens; column is unbounded String.
E2E-verified: token refreshed → stored as Fernet ciphertext → 319-subscription
YouTube sync succeeded.
- SA5: password_login is no longer a timing/enumeration oracle — it always runs
argon2 (against a decoy hash for unknown/passwordless emails), so response time
can't reveal whether an account exists.
- SB2: Google login only ADOPTS+activates a pre-existing password account when
Google actually attests email_verified (defense-in-depth against takeover); and
the email sync won't overwrite with a value another account owns (avoids a 500).
- demo_login clears the wallet first (demo can't switch to / act as a real account
via the multi-account header); switch_account rejects a suspended target (which
would otherwise clear the whole session on the next request).
Re-review clean; ruff clean; localdev boots; YouTube auth path E2E-verified.
set_user_role (demote) and admin_delete_user counted ALL admins incl. suspended
ones, so with e.g. 1 active + 1 suspended admin you could demote/delete the only
ACTIVE admin — leaving a single suspended admin who can never sign in → permanent
admin lockout needing DB surgery. Now both guards mirror the (already-correct)
suspend guard: block only when the target is an active admin AND it's the last one
(`not target.is_suspended and count_admins(active_only=True) <= 1`) — which also
correctly still allows removing a SUSPENDED admin while one active admin remains.
Resolves the deferred clearDevice security gap: logout never removed the E2EE
private key from IndexedDB, so on a shared machine the conversations stayed
decryptable after sign-out (ChatDock auto-unlocks from the persisted key next
visit). Per the user's decision (clear on EVERY logout), NavSidebar.logout() now
awaits e2ee.clearDevice(me.id) before reload — guarded so an IndexedDB failure
(e.g. private mode) can never block the logout. Re-entering the message
passphrase is required on the next visit.
Resolves the Phase-1 NEEDS-DECISION #3 (clearDevice) + retires the last e2ee
unused export (knip now only flags loadDefaultViewFilters). tsc green, re-review
clean (found + fixed an idb-open throw path blocking logout).
Fix: AES-GCM nonce reuse in e2ee.setup() (+ drop redundant key_check oracle,
backward-compatible); silent 404/429 send failures now toast; scroll/render-churn
in ChatThread + Messages; chatDock LS helper; deleted dead lock(). Verified: tsc,
re-review clean, localdev boots, self-E2E green (unlock + full-thread decrypt on a
real existing key, user entered the passphrase).
- ChatThread send had no onError: a 404 (recipient gone) or 429 (rate-limited)
send failed silently (api.req shows no modal for those "caller-handled" statuses).
Added onError → toast for 404/429 (400/409/500 already raise the global dialog);
draft is kept for retry. New trilingual messages.sendFailed key.
- ChatThread scroll-to-bottom effect keyed on `plain` too, so every 20s poll's
decrypt pass yanked a scrolled-up reader back to the bottom. Key on items.length.
- ChatThread + Messages decrypt effects keyed on `items` (a fresh `?? []` array
every render) → a render storm while loading. Key on q.dataUpdatedAt.
- chatDock.ts: use LS.chatDock(userId) instead of the bare "siftlode.chatDock.*"
literal (matches the storage LS registry).
tsc green, re-review clean, localdev boots, Messages renders (locked-state) w/o console errors.
setup() encrypted BOTH the wrapped private key AND the key_check under the same
wrapKey with the SAME wrapIv — a GCM nonce-reuse bug (leaks keystream + enables
GHASH/tag forgery, which matters under E2EE's malicious-server threat model).
Fixes:
- setup() gives key_check its own independent nonce (checkIv).
- unlock() no longer decrypts key_check to verify the passphrase — that was
redundant (a wrong passphrase already fails the wrapped_private_key GCM auth
tag) AND was the second consumer of the reused nonce.
Backward-compatible: the uploaded bundle shape is unchanged, and existing users'
bundles still unlock (unlock only presence-checks key_check now; the private-key
decrypt path with wrap_iv is untouched). Also removed the unused, architecturally
ineffective lock() export (re-unlocks from IndexedDB on next mount; the meaningful
primitive is clearDevice). Re-review clean; tsc green.
Bugs:
- ConfigPanel (CB1): unchecking a Plex library from the "all" (empty) state made
that library the ONLY selected one instead of all-except-it, because the toggle
ADDED the key to an empty list. Expand "all" to the explicit section set before
toggling. E2E-verified: uncheck from all → all-except-one.
- Scheduler (SB1): "Purge discovery" bulk-deleted search videos/videos/channels
immediately with no confirm (every AdminUsers destructive action uses useConfirm).
Wrapped in a danger useConfirm dialog (+ trilingual purgeConfirmTitle key).
E2E-verified: dialog appears, Cancel aborts.
Cleanups (behavior-neutral):
- Register LS.configTab; ConfigPanel uses it instead of the bare "siftlode.configTab".
- ConfigPanel + SettingsPanel import SaveState from ui/DraftSaveBar instead of
redeclaring the union (PrefsSaveState is now an alias).
tsc green, re-review clean, localdev boots.
- Extract runner.token_users(db): the "all users with a stored refresh token"
query was spelled 3× (get_service_user, run_subscription_resync,
sync_all_playlists). get_service_user now reuses it ([0]); sync_all_playlists
imports it from runner (no cycle — runner doesn't import playlists); the now-
unused OAuthToken import is dropped there.
- scheduler.py: _job("rss_poll", lambda db: run_rss_poll(db)) → _job("rss_poll",
run_rss_poll) — the lambda was dead indirection; all sibling jobs pass the
callable directly and _job calls fn(db).
Behavior-neutral. ruff clean, localdev boots, re-review clean.
download_layout and download_retention_days are registered ConfigSpec keys
(admin-editable on the Configuration page), but worker.py read them from
settings.* (env) at download/edit finalize time — so an admin's edit was a
silent no-op (files kept the old layout; TTL stayed at the env default). Read
them via sysconfig (_download_settings), which falls back to the env default
when there's no DB override. Same class as the Downloads B3 fix.
ruff clean, localdev boots, re-review clean.
Fix: short videos were auto-marked watched almost immediately (negative finish
margin). Cleanups: extract format.formatDate, reuse goPrev/goNext for the in-bar
buttons, memoize the savedPrefs cache read. Verified: tsc, knip, localdev healthy.
BUG (YB1): the auto-watch checkpoint marked a video watched once position
crossed `duration - FINISH_MARGIN` (10s). For clips shorter than ~10s that
threshold is negative, and for ~11-20s clips it's only a few seconds in, so the
5s checkpoint tick marked short videos watched almost immediately (clearing their
resume position). Cap the margin at half the clip: Math.min(FINISH_MARGIN, dur/2).
Cleanups (behavior-neutral):
- Extract format.formatDate(iso, lang) — the day/month/year toLocaleDateString
option object was duplicated verbatim in PlayerModal (fullDate) and VideoCard.
(ChannelPage's "joined" is month/year only, so it's left as-is.)
- The in-bar prev/next buttons now reuse goPrev/goNext instead of re-implementing
the step + bounds inline (they already exist for the side arrows + keyboard).
- Memoize savedPrefs so the ['me'] cache read + spread runs once, not on every
render (it only seeds the initial autoMode/loopMode state).
tsc green, knip clean, localdev boots healthy.
Cleanup (_remove_item dedup; drop dead Check import + plName shadow) plus
review-found bug fixes: reorder position-collision under concurrent edits,
push/sync/revert YouTube 403s now route to the Connect affordance, and
removeItem rolls back on failure. Verified: ruff, tsc, localdev boots healthy.
BUGS:
- push/sync/revert (PF1/PF2/PF4) swallowed every failure into a generic warning,
so a missing-write-scope 403 gave no hint and no "Connect your YouTube account"
affordance (unlike Channels). Route all three through notifyYouTubeActionError
(no wizard handle on this screen, so message-only for the 403).
- removeItem (PF5) reset the order optimistically then awaited the delete with no
catch: a failed call left the row gone locally (and an unhandled rejection).
Now resync via refreshAll on either path (api.req surfaces the error dialog).
CLEANUP:
- Drop the dead `Check` lucide import (PC6; only AddToPlaylist uses it).
- Use the module plName(detail) helper instead of the inline watch_later-vs-name
ternary that shadowed it in push/revert (PC7).
tsc green, localdev boots healthy.
BUG (PB1): reorder_items only repositioned the items present in the payload,
leaving any omitted item (e.g. one added concurrently in another tab) at its old
position — which then collides with the freshly assigned 0..N-1 range, so
get_playlist's order-by-position returns a nondeterministic/duplicated order.
Now the omitted items get fresh trailing positions (keeping their relative
order), guaranteeing unique contiguous positions.
CLEANUP (PC4): extract _remove_item(db, pl, video_id, mark_dirty=) mirroring the
existing _add_item — remove_watch_later and remove_item duplicated the same
find-by-(playlist,video)/delete/commit block.
Behavior-neutral for the normal full-payload reorder. ruff clean, localdev boots.
BUG (FB3): the channel page's Subscribe mutation had no onError, so a read-only
user (no YouTube write scope) clicking Subscribe got a silent 403 — nothing
happened, no toast. Wire notifyYouTubeActionError (no wizard handle here, so the
message without the Connect button). Block is a local-only action and can't 403
for scope, so it's left as-is.
BUG (FB4): subscribe.onSuccess invalidated channel/feed/channels but omitted
my-status and discovered-channels (which ChannelDiscovery invalidates for the
same action), so after subscribing from a channel page the Discovery list and
the manager stats stayed stale. Added both.
tsc green, localdev boots healthy.
- Extract notifyYouTubeActionError (new lib/youtubeErrors.ts): the "403 → Connect
your YouTube account, else fallback toast" logic was duplicated in Channels.tsx
and ChannelDiscovery.tsx. (Separate file, not lib/notifications, to avoid the
api ↔ notifications import cycle.)
- Extract format.formatCountOrDash(): the `n != null ? formatViews(n) : "—"` count
cell was repeated across the subs/videos columns in Channels + ChannelDiscovery.
- Register channelsTable / channelDiscoveryTable in storage.ts LS instead of bare
"siftlode.*" string literals.
- Delete 7 dead channels.json keys (filterPlaceholder, tags.newTag/createTag,
row.stored/subs/getFullHistory/getFullHistoryHint) from en/hu/de — verified 0
refs (createTag matches were the unrelated api.createTag method, not the key).
Behavior-neutral. tsc green, knip no new unused, localdev boots.
- Extract _is_blocked(db, user, channel_id): the BlockedChannel EXISTS query was
copy-pasted in channel_detail, explore_channel, and block_channel.
- Extract _channel_summary(ch): the shared id/title/handle/thumbnail/subscriber/
video_count projection was hand-rolled in list_channels and discover_channels
(the detail dict interleaves these with extras, so it's left as-is).
Behavior-neutral. ruff clean, localdev boots healthy.
BUG-1 (search.py, high): the scrape search source logged one VIDEOS_SEARCH
quota event PER continuation page inside the paging loop, but actions_today
counts events — so a single user search that pages N times consumed N against
search_daily_limit_per_user (its docstring even warns it only works for
once-per-action logging). Log exactly once per request after the loop instead;
the API source already logs once via record_usage (it never auto-pages).
BUG-2 (Feed.tsx, medium): the optimistic-override reset effect keyed on
query.dataUpdatedAt also fired on fetchNextPage (infinite scroll bumps
dataUpdatedAt while page 1 is unchanged), so a just-hidden card flashed back
mid-scroll. Guard with a page-count ref: only reset on a real refetch, not an
append. The two override-reset effects legitimately differ now (resolves the
would-be C-F1 "identical effects" cleanup).
tsc green, ruff clean, localdev boots healthy.
- Hoist withOverrides(loaded) into `merged` (was mapped twice per render for
items and playerQueue).
- Remove the needless always-true {(<>…</>)} expression wrapper around the
Source selector.
- Extract dropFeedCaches() — the feed/feed-count/facets removeQueries trio was
duplicated across the back and clear-now handlers.
Behavior-neutral. tsc green.
- feed.py _filtered_query: drop the dead 2nd return element (status_expr was
used only internally for WHERE filters; all 3 callers discarded it as _status).
Now returns (query, rank_expr); fixed the stale docstring.
- youtube/client.py: extract _iter_playlist_items() — iter_my_playlist_video_ids
and iter_playlist_items_with_ids were near-identical playlistItems paging loops
(jscpd [173-187]≈[267-281]); both now map over the shared generator.
- sync/videos.py: extract _apply_video_batch() with an on_missing callback —
enrich_pending and refresh_live shared the fetch-map-apply skeleton, differing
only in the query and how they retire rows YouTube omits.
- models.py: add LIVE_OR_UPCOMING = ("live","upcoming"); replace the 4 duplicated
copies (feed.HIDDEN_LIVE, search._LIVE_HIDDEN, videos.py + channels.py inline).
Behavior-neutral. ruff clean on touched files, localdev boots, feed/worker healthy.
- Delete dead top-level downloads.rename.* i18n block (en/hu/de); the
metadata modal moved to downloads.edit.*. Keep downloads.actions.rename
(still used by ProfileEditor).
- Rename misnamed state renameJob/setRenameJob -> metaJob/setMetaJob in
DownloadCenter (it opens MetaEditModal, not a rename dialog).
- Extract the duplicated [downloads]/[download-index]/[download-usage]
invalidation trio into lib/downloads.ts invalidateDownloads(qc);
used by DownloadCenter and DownloadDialog.
Behavior-neutral. tsc green, knip shows no new unused export, JSON valid.
Found during the Phase-2 Downloads-frontend review:
- VideoEditor: onLoaded now reconciles the still-pristine full-length segment to the
REAL decoded duration (its guard `end<=0` was dead since the metadata srcDur is
always >0). Without this the untouched last segment kept the rounded metadata
length, so isFullSingle read false and "Create" was enabled on an unmodified
video — producing a bogus edit that dropped the tail.
- DownloadCenter: fall back to the "queue" tab when a persisted tab (e.g. admin-only
"system") isn't in the current set, instead of rendering a blank page.
- VideoEditor: reencode = cropOn || accurate (was a no-op `willJoin ? accurate : accurate`).
tsc clean. GUI-affecting → pending an E2E visual test (needs an authed localdev
session) before user UAT. Cross-cutting UI-consistency cleanup deferred to Phase 3.
Sticky errored asset re-download, ref_count leak on errored-job delete, admin
storage cap reads the DB value, and 400-not-500 on malformed edit/format specs.
Reviewed clean (a B2 concurrency race was caught + fixed). Local dev only — prod
publish waits for the end of the whole code-hygiene review.
- B1 (sticky errored asset): get_or_create_asset now resets a reused status=='error'
asset back to 'pending' (+clears the error), so a fresh enqueue/edit of a once-failed
(source,format) pair actually re-downloads instead of the worker short-circuiting the
new job with the stale error. Errored assets carry no expires_at, so without this the
pair was permanently poisoned for all users until someone hit resume.
- B2 (ref_count leak): _release_asset counts 'error' as a holding state, so deleting an
errored job decrements the ref_count that enqueue always incremented (the worker never
decrements on failure). Errored rows are deliberately NOT deleted here — a concurrent
B1 reuse could otherwise be lost-updated + FK-nulled; the row is fileless and harmless.
- B3: admin storage dashboard reads sysconfig.get_int(db,'download_total_max_bytes')
(the admin-editable DB value GC enforces) instead of the raw env default.
- B4: the single-trim branch of normalize_edit_spec guards its float() coercion like the
crop/segments branches — a malformed trim now yields a 400, not an unhandled 500.
- B5: formats.normalize guards int(max_height) → falls back to "best" instead of 500.
Reviewed (race in an earlier B2 draft caught + fixed); localdev boots, B4/B5 unit-verified.
