Failed downloads stored the raw yt-dlp/ffmpeg text and dumped it as a truncated red
one-liner in the queue. Classify the failure into a category (unavailable / login_required /
geo_blocked / format_unavailable / drm / postprocess / source_missing / network / unknown)
and surface it properly:
- backend: app/downloads/errors.classify maps the raw message to a category code; the worker
stores it in the new download_jobs.error_code (migration 0061) at both failure sites; resume
clears it; the serializer exposes it. Raw text kept as technical detail.
- frontend: the queue row shows a clean localized reason + a "details" link opening a modal
with the explanation, the raw text behind a disclosure, and a Retry only for transient
categories (network/postprocess/unknown).
- tests: classify() unit coverage over verbatim yt-dlp/ffmpeg message shapes.
- Always land on the Queue tab (drop the persisted last-tab) so new work/failures are the default view
- Show the download date (relative) on Library rows
- Share dialog opens with a default link already generated when none exists; all link controls stay available
- Thumbnail falls back to the self-hosted poster when a remote (i.ytimg/fbcdn) thumbnail 404s
- Add a clipboard-paste affordance to the add-URL box (explicit button + best-effort on focus)
The S3b api.ts split introduced 5 exported-but-never-imported types
(PlaylistKind/PlaylistSource, DownloadJobKind, YT internal option/data shapes).
The publish gate's knip lane flagged them (my per-step checks ran tsc/eslint/
prettier/vitest but not knip). They're used inside their own modules by the
interfaces, so make them local (consumers compare against literals, never name
the type).
- barrel re-exports domains with `export type *` (not `export *`) so the
internal downloadsApi/plexApi/messagingApi slices stay behind the façade;
plex's runtime value exports (EMPTY_PLEX_FILTERS/plexFilterCount) re-exported
explicitly
- App.tsx: drop the now-redundant `prefs as Record<string, unknown>` self-cast
(Me.preferences is already that type) and its `prefsRec` alias
- plex.ts: comment why plexSetState keeps its status union inline (reusing
VideoStatus would make an api.ts↔plex.ts type cycle)
Behaviour-identical. Gate green: typecheck, eslint 0, prettier, 74 vitest.
The E2EE direct-message types (MessageUser/Message/Conversation/MyKeyBundle/
KeySetup) + the ~9 messaging methods move into a self-contained
api/messaging.ts exporting a `messagingApi` slice (imports only core's req).
api.ts spreads the slice and re-exports the types. api.ts 882 -> 819 lines; no
runtime change. Gate green: typecheck, eslint, prettier, 74 vitest.
All Plex types (browse cards, show/season/item details, collections, native
playlists, filters/facets, watch-link) + EMPTY_PLEX_FILTERS/plexFilterCount/
appendPlexFilters + the ~40 plex methods move into a self-contained api/plex.ts
exporting a `plexApi` slice (imports only core's req/beacon). api.ts spreads the
slice and re-exports the types via `export *`. api.ts 1352 -> 882 lines (halved
from the original 1793); no runtime change. Gate green: typecheck, eslint,
prettier, 74 vitest.
The download-center types (DownloadJob/Spec/Profile/EditSpec/ShareLink/... and
the admin storage/quota shapes) + all ~30 download methods move into a
self-contained api/downloads.ts exporting a `downloadsApi` slice. api.ts spreads
the slice into the `api` façade and re-exports the types via `export *`, so the
53 call sites are unchanged. Cleanly separable — the domain's types reference
only each other and its methods only touch core's req/getActiveAccount. api.ts
1583 -> 1352 lines; no runtime change. Gate green: typecheck, eslint, prettier,
74 vitest.
First step of splitting the api.ts god module behind its façade. Move req,
HttpError, localizeDetail, the connectivity toast, ReqConfig, the per-tab
active-account plumbing (getActiveAccount/setActiveAccount/clearActiveAccount/
beacon), setupHeaders, and setUnauthorizedHandler into lib/api/core.ts. api.ts
imports the machinery it uses and re-exports the public surface, so the 53 call
sites importing from "./api" are unchanged. api.ts 1793 -> 1583 lines; no
runtime change (pure move). Gate green: typecheck, eslint, prettier, 74 vitest.
Removes all 13 remaining `any`s and flips the ESLint rule from off to error so
new ones can't creep back:
- api.ts prefs/data JSON bags: Record<string, any> -> Record<string, unknown>,
narrowed at consumers (App language guard; NotificationsPanel shapes the
notification `data` per type behind its existing typeof guards)
- catch (e: any) -> catch (e) + `e instanceof HttpError` narrowing
(SettingsPanel, SetupWizard, PlexPlayer)
- the YouTube IFrame API boundary: new lib/youtube.ts types (YTPlayer/
YTPlayerEvent/YTNamespace + a Window augmentation) replace the 6 player anys;
tsc surfaced the exact method surface to declare
- NotificationsPanel `t` props: any-options -> i18next TFunction
Gate green: typecheck (app+node+e2e), eslint 0 errors (rule now enforced),
prettier, 74 vitest.
- FeedResponse claimed a required `limit`, but /api/search/youtube never
returns it (only /api/feed does) and nothing in the app reads it. Drop the
field so searchYoutube's Promise<FeedResponse> stops lying; documented for
re-add if a consumer appears.
- pageMeta's hand-written 13-case Page->title switch now derives from the
ModuleDef registry via a new optional `titleKey` (the longer tab title, e.g.
"Channel manager"; falls back to the short nav labelKey). A new page is one
touch (add it to MODULES) instead of also editing a switch. A test pins all
13 title keys byte-identical to the old switch (suite 74).
Video.status, Playlist.kind/source (+ PlaylistDetail), DownloadJob.job_kind
were bare `string`; type them as named unions verified against the backend
(VALID_STATES={new,watched,hidden}; kind user|watch_later; source local|
youtube; job_kind download|edit). VideoStatus then propagates through the
optimistic-override map, onState, and the VideoCard/VirtualFeed/PlayerModal
props — tsc now rejects a status typo (e.g. a stale "in_progress", which is a
derived filter, never a stored status) at every call site.
Code-review follow-ups on the S2 factory:
- add queryKeys.test.ts pinning every branch (bare vs param, null-segment
preservation, single-id, variadic composite spreads) — the contract the
epic exists to protect now has a test (suite 68 -> 73)
- unify feed/feedCount/facets on the `!== undefined` guard the rest of the
factory already uses (behaviour-identical for the always-truthy FeedFilters,
removes the collapse-on-null footgun)
Two findings skipped with rationale: variadic composite-key typing (heterogeneous
object/array segments make precise tuples high-effort; root centralized + shapes
now tested) and a pre-existing thread(null) no-op (unreachable in practice).
Extend lib/queryKeys.ts to the remaining ~34 key roots and migrate every
remaining literal (queryKey: [...] AND the positional useLiveQuery keys) in
36 files. All ~200 React Query keys across the app now route through the
factory; zero raw key literals remain outside queryKeys.ts.
The heterogeneous composite plex keys (plex-library/facets/collections/
playlists carry a filters object, a ratingKeys array, or a union/group
discriminator) take a variadic pass-through — the factory centralizes the
ROOT string; single-id keys stay precisely typed. Nullable keys (thread,
playlist, ytSearch) accept null so a null segment is preserved.
Pure substitution — byte-identical key arrays, so TanStack prefix matching
is unchanged. Gate green: typecheck (app+node+e2e), eslint (0 err),
prettier, 68 vitest. Runtime-verified on a fresh dev server: Feed + Plex
modules load, feed/facets/tags/my-status/plex-library/plex-facets/
plex-collections/plex-playlists all 200, full UI renders.
Introduce lib/queryKeys.ts as the single source of truth for React Query
keys — the R7 driver was ~200 hand-written key literals across 51 files
(`["feed"]` alone in 15+), where one typo silently orphans a key so an
invalidate never matches.
S2a migrates the three highest-traffic domains (feed/video, channels,
playlists) plus the shared me/my-status/tags roots: ~90 literal sites in
18 files now call qk.*(). Pure substitution — each factory fn returns the
byte-identical array the call sites used before, so TanStack's prefix
matching is unchanged (qk.feed() still invalidates qk.feed(filters)).
Co-invalidation clusters are deliberately NOT bundled into helpers (they
differ per site), so no query's refetch scope changes.
Nullable keys (playlist(selectedId), ytSearch(q)) accept null so a null
segment is preserved (["playlist", null]) while a no-arg call is the bare
prefix key — guard is `!== undefined`.
Gate green: typecheck, eslint (0 err), prettier, 68 vitest. Smoke: app
boots, feed/facets/tags/my-status all 200, no console errors. Factory
extended to plex/messaging/admin/downloads/notifications in S2b.
- localizeDetail: don't leak FastAPI's snake_case `loc` field names into a
translated 422 toast; return a clean, fully localized validation message
(drops the loc filter/pop parsing entirely)
- me(): drop the dead `r &&` guard now that req<T> types r non-nullable
req() returned Promise<any>, so every api.* return annotation was an
unchecked assertion and the 17 action endpoints leaked `any` to callers.
Make req generic with a single audited cast at the boundary; T flows from
each method's declared return type via contextual inference, so no call
site needs a type arg except where none existed:
- me(): type the /api/me probe ({ authenticated } + Partial<Me>)
- deepAll / syncSubscriptions: real backend-verified response shapes
- HttpError.detailData, localizeDetail(d), inner detailData: any -> unknown
Also teach localizeDetail the FastAPI 422 detail-ARRAY shape (carried in
from R6 S1b review): now that malformed requests return 422, surface the
offending field via a localized errors.validation key instead of falling
through to the generic toast.
No runtime change on normal flows (req is await-identical); the 422 branch
only fires on genuinely malformed input. Gate green: typecheck, eslint,
prettier, 68 vitest. Smoke-tested localdev: /api/me + feed 200, no console
errors.
Backend-heavy, frontend-invisible hardening over four sprints, all user-triggered
/code-review clean + UAT-passed on localdev:
- S1a: shared iso()/owned_or_404/require_write_scope helpers.
- S1b: all ~52 payload:dict endpoints → Pydantic request models (3 named prod-500s
on garbage input become clean 422s); status-code unification.
- S3 (pulled ahead of S2): DB-backed pytest lane (ephemeral Postgres, alembic head)
wired into the gate; /facets genre-null regression; migration 0060 normalises
existing JSONB-null plex rows.
- S2: quota accounting owns its session (durable across a caller rollback, never
commits the caller); Plex sync commits per-section. Proven by DB-lane tests.
Suite 159 -> 181 (incl. the new DB lane). db_revision 0060. No response-shape change.
/code-review high on S2 — 4 findings, 2 fixed, 2 no-change:
- test_plex_sync_txn: a faked PlexClient fails on the second library; asserts the
first library's row survives (per-section commit). Mutation-checked — moving the
commit back out of the loop turns it red ([] == ['1']).
- dropped the `[[quota-owns-its-session]]` memory slugs from quota.py docstrings
(a dangling private-note reference in shipped code; the reasoning is already inline).
No change: record_usage's per-call second connection is a transient spike, fine for
this app's concurrency; the two-site `with SessionLocal()` duplication isn't worth a
helper until a third caller appears.
Gate: ruff clean; DB lane 181 green.
Transaction ownership (C-A5):
- quota.record_usage / log_action write in their OWN SessionLocal(), committed
immediately, and no longer take/commit the caller's db. The spend stays DURABLE
regardless of the caller (read-only YouTube paths never commit; a failed job rolls
back) — under-counting the shared daily budget would risk overspending the real
YouTube quota — AND quota stops flushing a job's partial state mid-run behind its
own `except: db.rollback()`. units_used_today switches to a scalar SELECT so
measured()'s before/after diff sees the separate session's commits (READ COMMITTED).
Call sites updated (youtube/client.py ×3, routes/search.py ×1).
- plex sync commits PER SECTION (moved the end-of-loop commit inside), so a PlexError
on a later library keeps the ones already mirrored (idempotent; next run reconciles).
The sync jobs did NOT rely on record_usage's incidental commit for persistence — their
per-item functions (apply_rss_feed, backfill_channel_recent, import_subscriptions,
sync_user_playlists, apply_channel_autotags) all commit internally, so the in-loop
rollback handlers already isolate a failed item. No loop restructuring needed.
DB-lane tests (test_quota_ownership, 4): spend survives a caller rollback while the
caller's uncommitted row does NOT leak; accumulation; measured-style mid-txn read.
Gate: ruff clean; DB lane 180 green.
/code-review high on S3 — 3 low findings, 2 fixed, 1 no-change:
- _db_reachable() now probes via a throwaway engine with connect_timeout=3, so a
slow/firewalled DATABASE_URL can't hang the whole suite at import (the app engine
has no connect timeout).
- test_plex_facets sets the "configured" gate flag via monkeypatch (auto-restored)
instead of poking state._configured_cache in a manual finally — no leak risk, and
the now-unnecessary mark_configured DB write is dropped.
No change: test_plex_jsonb_normalize copying migration 0060's SQL is deliberate —
migrations must stay self-contained (no app imports) and a shipped migration is
immutable, so there's no real drift to guard against.
Gate: ruff clean; DB lane 176 passed; pure-logic 172 passed / 4 skipped (fast).
First real customers of the DB lane:
- test_plex_facets: GET /api/plex/facets?scope=show returns 200 (not the
"cannot extract elements from a scalar" 500) for a show whose genres is a JSONB
scalar `null` — the 0.53.1 read-guard's regression test. Mutation-checked:
removing the jsonb_typeof='array' guard turns it red with that exact error.
- migration 0060 (S3a): one-time UPDATE …=NULL WHERE jsonb_typeof=… 'null' over the
plex_items/plex_shows tag columns, so pre-0.53.1 JSONB-null rows become uniform SQL
NULL and the read-guards go belt-only. test_plex_jsonb_normalize verifies scalar-null
→ SQL NULL while a real array is preserved.
Gate: ruff clean; DB lane 176 passed; pure-logic lane unchanged (skips the DB tests).
The suite was pure-logic only (R2 S2b deferred the DB lane). Add an opt-in `db`
fixture (tests/conftest.py) that hands a test a real session against an ephemeral
Postgres; schema is built with `alembic upgrade head` (the real prod schema needs
the unaccent extension + unaccent_simple TS config that create_all can't provide,
and this also proves the migration chain applies). Each test starts from a TRUNCATEd
clean DB so tests that COMMIT — the whole point, for R6 S2's transaction work —
don't leak.
backend/docker-compose.test.yml wires an ephemeral tmpfs pg + the test image. When
no Postgres is reachable (a plain `docker run`), the DB tests SKIP and the pure-logic
lane is unchanged: 172 passed, 2 skipped. With the DB: 174 passed.
test_db_lane.py smoke-proves commit round-trip + TRUNCATE isolation. The gate wiring
(siftlode.sh) lands in the ops repo.
/code-review high round 1 on S1b — 5 findings, 1 fixed, 4 by-design/deferred:
- FIXED: VideoProgressIn / ItemProgressIn position_seconds & duration_seconds
typed `int` would 422 a raw float currentTime on the 10s progress checkpoint
(the old `int(payload.get(...))` truncated it). Back to `float`, truncated in
the handler; +test. The live frontend floors so this was latent, not live.
By-design / deferred (no change): the 422 detail-array isn't localized by the FE
(filed under R7); PATCH is-not-none (channels/quota, null-safe) vs model_fields_set
(nullable-clear) is a deliberate split; missing-field→422 is the point of the epic;
me/switch_account's numeric-string coercion is more correct and still session-gated.
Gate: ruff clean, pytest 172 green.
Pure-logic tests that the three ROADMAP-named 500-producers (channel priority,
tag_id, setup password) and admin quota now reject bad input at the model
boundary, and that the PATCH models separate absent from explicit-null via
model_fields_set. Mutation-checked: reverting a field type turns them red.
All 11 download endpoints off payload:dict. Highlights:
- admin_set_quota: AdminSetQuotaIn — the `int(payload["max_bytes"])` 500 on a
non-number is now a 422; each field set only when provided (is not None).
- profiles/enqueue/edit: CreateProfileIn / UpdateProfileIn / EnqueueDownloadIn /
EnqueueEditIn; spec/edit_spec typed as dict (non-object → 422). _resolve_spec now
takes (profile_id, spec) instead of the raw payload.
- meta/share/links: UpdateDownloadMetaIn / ShareDownloadIn / CreateLinkIn /
UpdateLinkIn; _link_expiry takes an int|None instead of digging the payload.
PATCH endpoints read model_fields_set so an explicit null still clears a field.
Gate: ruff clean, pytest 164 green.
CreatePlaylistIn / RenamePlaylistIn (PATCH via model_fields_set) / ReorderItemsIn,
plus a shared VideoRefIn(video_id) for both add-video endpoints (watch-later +
add-item). A non-string video_id is now a 422 instead of reaching db.get.
Gate: ruff clean.
- setup: SetupAdminIn (KILLS the 3rd named 500 — a non-string password reaching
argon2/len()), SetupTestEmailIn; setup_config stays a typed dict[str, Any] (a
genuinely dynamic key→value settings map, not a fixed schema).
- admin: AddInviteIn / SetRoleIn / SetSuspendedIn / AddDemoWhitelistIn. The
role/status business-rule 400s are unchanged; type errors are now 422.
All 3 ROADMAP-named 500-producers now fixed (channels priority + tag_id, setup
password). Gate: ruff clean, pytest 164 green.
- saved_views: CreateViewIn / UpdateViewIn / ReorderViewsIn. PATCH presence via
model_fields_set; filters typed as dict (a non-object is now 422, an explicit
null on update stays a 400 "filters must be an object").
- feed: VideoStateIn(status) / VideoProgressIn(position_seconds,duration_seconds) —
the manual int()+400 in progress becomes a 422 on a non-number; the status
membership 400 and the video 404 are unchanged.
Gate: ruff clean.
update_channel: payload:dict → ChannelUpdateIn (priority/hidden/deep_requested,
all optional for PATCH semantics). Kills the `int(payload["priority"])` 500 the
prod report hit — a non-numeric/garbage value now returns 422, not 500.
attach_tag: payload:dict → AttachTagIn(tag_id:int) — a non-int tag_id was a 500
in `db.get(Tag, ...)`; the personal-tag ownership check reuses owned_or_404.
Round-1 review finding #4 named only audit.py/plex.py, but the same misplaced
`from app.utils import iso` sat in channels/downloads/feed/notifications too — the
round missed them (ruff enforces no import order here, so the gate was silent).
Move them to alphabetical position for consistency. No behaviour change.
/code-review high round 1 (5 findings):
- iso(dt) now typed `date | None` — MediaAsset.upload_date is a plain `date`, not a
datetime; the helper's signature was too narrow for the type-contract epic. +test.
- restore alphabetical app.* import order in audit.py and plex.py (the new iso/_common
imports had been dropped in mid-block).
The other 3 findings are deliberate design choices, left as-is for UAT: push/push-plan
now gate write-scope before the 404 (dependency ordering); the unified 403 message; and
owned_or_404's owner_attr kept for S1b's DownloadShare.to_user_id-style owners.
Gate: ruff clean, pytest 164 green.
Establish the R6 API-contract infra before the S1b payload:dict migration:
- iso(dt) in utils.py replaces the ~16 hand-rolled `x.isoformat() if x else None`
serializer idioms across the routes.
- get_or_404 / owned_or_404 in routes/_common.py — one 404 + existence-leak policy;
plex's _own_playlist_or_404 and saved_views' _own_view now delegate to it.
- require_write_scope dependency in auth.py collapses the inline has_write_scope→403
checks (channels subscribe/unsubscribe, playlists push/push-plan) that had drifted
into three different messages into one; the conditional delete_playlist guard stays
inline (its message was already canonical).
No response-shape change (iso == isoformat for a present value). Gate: ruff clean,
pytest 163 green (+ test_utils for iso).
justify-center + overflow-x-auto stranded the left-end controls (Previous, the
N/M counter) when the bar overflowed a narrow window. justify-start makes the
overflow scroll from the start — and matches the conventional left-aligned
video-player control bar.
The overlaid, auto-hiding bar fought YouTube's seek bar for the same bottom
pixels: while ours showed it covered YT's seek + buttons, and reaching them
meant moving off the area and waiting for our bar to fade — a fiddly, "random"-
feeling dance keyed on mouse movement.
Now the bar is DOCKED and always visible: the video area is inset by the bar's
height (a wrapper around the mount, since YT.Player replaces the mount element
with its iframe and would drop a class set on it), so the bar sits in its own
strip and never overlaps the video. YouTube's seek/controls stay reachable by
hovering the video's bottom edge, just above ours. No auto-hide, no guessing,
no fight. Verified in Chrome: iframe.bottom == bar.top (no overlap), bar opacity 1.
UAT feedback on the F-maximise view — three fixes:
- Own control bar in the big views (maximise + our fullscreen): previous/next +
N/M, auto-advance, loop, add-to-playlist, download, mark-watched, exit, and a
true-fullscreen toggle. Auto-hides on idle. Add/download are hidden in true
fullscreen (their popovers portal to <body>, outside the fullscreen element).
- Wheel volume works in the big views again: the centre interaction overlay is
now armed in maximise/fullscreen (it only yields to YouTube's own settings
menu), so the wheel bubbles to our handler instead of the bare iframe. F also
offers true fullscreen that WE own (Fullscreen API on the stage), so wheel/
arrow volume and our controls work inside it too — YouTube's own fullscreen
button, which we never surface, is not involved.
- Back (mouse/browser) steps out one level at a time via nested history layers
(useBackToExit): fullscreen -> maximise -> the modal -> closed, instead of
closing the whole player at once.
Verified in Chrome: wheel volume in maximise + fullscreen, the control bar, and
the full Back ladder (fullscreen -> maximise -> modal -> closed).
A mirrored Shows library whose items have no genres stored `genres` as JSON
`null` — SQLAlchemy's JSONB persists a Python None as a JSON scalar, not SQL
NULL. /facets unnested it with jsonb_array_elements_text(), which raises
"cannot extract elements from a scalar", so a single bare library 500'd the
whole Plex module on open (the grid loads, the filter sidebar's facets call dies).
- Guard the /facets genre unnest with jsonb_typeof(genres)='array', mirroring
the guard people.py already applies to the same columns.
- Persist the JSONB tag columns (genres/directors/cast_names/collection_keys on
plex_items and plex_shows) with none_as_null=True, so an empty list round-trips
as SQL NULL and self-heals on the next sync — closing the trap for any future
unnest.
Verified against the real prod row (DumaTV show, genres = JSON null): the
unguarded query errors, the guarded one returns cleanly.
Found by running the round-8 fix in the real container instead of trusting the
unit test: a directory created 2ms AFTER `_PROCESS_START` was stamped with an
mtime 2ms BEFORE it, so the sweep deleted it as a leftover. The two sides come
from different clocks — a file's mtime from the kernel's coarse clock (one timer
tick of lag), `time.time()` from the fine one — and the unit test could not see it
because it supplies an artificial cutoff.
The window is small and lands exactly where it hurts: the sweep runs at boot, so
the session at risk is one started in the first instants after startup — the case
the guard exists for. Keep-side slack of 2s; the cost is that a leftover from the
last seconds before a restart survives until the next one.
Verified in the container: a backdated leftover is swept, a directory created by
this run is kept, and a registered live session with an old mtime is kept — 1 of 3.
Test pins the skew (mutation-checked: with the grace at 0 it goes red).
The worst one was, again, the previous round's own fix.
- Round 7 stopped awaiting the HLS sweep so startup could serve immediately — which
also made it run CONCURRENTLY with playback, and `sweep_orphans` snapshotted the
live-session set BEFORE listing the root and rmtree'd outside the lock. Since
`start_session` reuses a directory path for the same (key, offset, tag), a stale
name can become a live session at any moment. Two guards now: nothing whose mtime
is newer than this process's start is ever deleted, and the live check + rmtree
happen together under the lock, one directory at a time.
- The sweep task is awaited after cancel, so shutdown can't leave it pending.
- `_rate_limiter` used a truthiness test on its timestamp, so a `now()` of exactly
0.0 read as "never ran" — unreachable in production, but `now` exists to be
injected and a test clock starting at 0 is the obvious choice.
- The RSS total-outage error now carries the first underlying error: "all N channels
failed" is also what ONE dead feed id looks like on a small instance, and a red
card the user can't act on is its own kind of dishonest.
- The WebKit fullscreen handling moved into `lib/fullscreen` and PlexPlayer uses it
too — it had the same unprefixed-only reads in three places, including an Escape
branch that would have closed the whole player instead of leaving fullscreen.
- The `--max` CSS comment claimed `inset: 0` did the sizing; Tailwind's `w-full` is
still on the element and wins. Comment now states the real mechanism.
Verified in real Chrome (the pane can't composite or do native fullscreen):
- F fills the 1920x889 viewport; the exit pill renders top-right on the letterbox
band and overlaps NO YouTube control — the collision this round suspected does not
reproduce in this embed (its own buttons sit bottom-left / bottom-right).
- Four real ArrowDown presses while maximised: persisted volume 100 -> 80, with the
level flash visible on screen.
- A real click on the exit pill un-maximises and returns focus to the card.
- lib/fullscreen driven from a real click: enter (innerHeight 889 -> 1024), the
change listener fires exactly once per transition, exit returns to 889.
Backend suite 150 -> 158; the new sweep tests are mutation-checked, and they read
their cutoff from the filesystem rather than a clock (the container VM and the
bind-mounted host disagree by enough to make a clock-based assertion flaky).
Five of them sit where round 6 turned F from a Fullscreen API call into a CSS
maximise: the guards around it still asked `isFullscreen`, which that pivot made
permanently false.
- Arrow-key volume was dead in the maximised view: the scroll-deference checked
only `isFullscreenRef`, so the (now hidden but still scrollable) card kept
swallowing Up/Down. Both flags now count as "the video fills the screen".
Measured: 87px of scroll room on the focused card, ArrowDown moves 80 -> 75
and scrolls nothing.
- The maximised view had no way out once the pointer touched the video. Reaching
YouTube's controls means clicking into the cross-origin iframe, which takes
keyboard focus with it, and the re-arm paths (stage mouseleave, window focus)
need the pointer to leave the browser entirely while the stage fills it. So
the exit now lives on screen, in our DOM: a button that un-maximises AND pulls
focus back, re-arming every shortcut.
- WebKit fires only `webkitfullscreenchange`/`webkitFullscreenElement`, so on
Safari/iPadOS YouTube's own fullscreen never registered — no overlay yield, no
focus reclaim. One `fullscreenEl()` reader, both event spellings.
- The stage claimed `z-index: 9999`, the glass tuner's reserved level, for a job
that only needs to beat the modal chrome it is nested in (its siblings top out
at z-menu). Dropped to rail-level; nothing escapes the modal's stacking
context either way.
- `inset: 0` already sizes a fixed layer — the inherited `100vw`/`100vh`
over-constrained it, which is how a classic scrollbar gets a horizontal
overflow and a mobile URL bar hides the bottom edge (YouTube's control bar).
- The shortcut hint still promised "F: fullscreen" in both locales.
Backend, both "the fix stopped one layer short":
- The startup HLS sweep was awaited inside lifespan, so uvicorn still served
nothing until it finished — a thread doesn't help when nothing is listening
yet. Fire-and-forget task instead, cancelled on shutdown. Measured with 200
planted segments: "Application startup complete" now precedes "swept 1".
- An RSS poll where every channel failed returned normally, so the scheduler
recorded `ok` with the outage buried in the summary string. `failed == total`
now raises: a run that reached nothing is a failed run.
Suites: backend 147 -> 150.
UAT found a dead end: F -> YouTube's fullscreen button -> Esc left the player
maximised with no way back, and a further F produced a second, control-less
fullscreen with the wheel driving YouTube's own volume box.
Measured cause: reaching YouTube's control bar means clicking INTO the player,
which moves keyboard focus into the cross-origin iframe — and it stays there.
Our window-level shortcuts then never fire (document.activeElement === IFRAME),
so F went to YouTube instead of toggling our maximise.
Fix: on fullscreenchange, when fullscreen ends, focus the modal card again.
Verified in real Chrome, step by step:
- F -> maximised, no browser lock, focus on our card
- YouTube's own FS button -> real fullscreen (innerHeight 889 -> 1024)
- leaving fullscreen -> focus back on our card (was: still the iframe)
- F -> back to the normal modal view (was: a second fullscreen with no controls)
Root cause, measured in Chrome/Brave/Firefox alike (so: spec behaviour, not a
browser bug): a cross-origin embed reads its fullscreen state from ITS OWN
document. While we hold the browser's fullscreen lock — on our wrapper OR on the
iframe element — the YouTube player never learns it is fullscreen, so its own
fullscreen button stays in the "enter" state and clicking it changes nothing
visible. Every other YouTube control works, which is exactly what the user saw.
So we stop taking the lock: F now toggles a CSS layer (`player-stage--max`) that
fills the viewport. The page stays non-fullscreen, so the embed's own fullscreen
button behaves natively, and our own overlays and shortcuts keep working because
our DOM is what's on screen. Escape steps out of maximise before closing.
Verified end to end in real Chrome:
- F -> stage 1920x889, document.fullscreenElement null (no lock taken)
- YouTube's own fullscreen button -> real fullscreen (innerHeight 889 -> 1024,
fullscreenElement becomes the IFRAME) and its icon flips to "exit"
- clicking it again -> back to the maximised view, our class still applied
- wheel volume while maximised: 100 -> 85, with the level flash visible
Drops the "YouTube fullscreen" toolbar button added earlier today: with the lock
gone, YouTube's own button does that job correctly.
The reported bug: after F, the embed's own fullscreen button is dead. Root cause
measured in Chrome — the browser's fullscreen lock sits on OUR wrapper, so
YouTube's button would need a nested request from a cross-origin frame. Handing
the lock to the iframe fixes it, but `iframe.requestFullscreen()` only resolves
from a CLICK (from a key press the promise stays pending forever, and awaiting
it spends the activation so a fallback is denied too).
So: F keeps fullscreening our wrapper (unchanged, our overlays stay visible) and
a new toolbar button hands the lock to the iframe — verified in real Chrome:
document.fullscreenElement is the IFRAME at 1920x1024, i.e. a plain YouTube
fullscreen where its own controls work.
Round-6 review findings, all fixed:
- one _rate_limiter + one _cancel_poll shared by the yt-dlp hook and run_ffmpeg
(was three hand-rolled throttles; cancel latency differed 2s vs 1s)
- stepVolume takes {delta, fallback} — two adjacent number args were silently
transposable
- volume gestures before the player is ready are honoured instead of dropped
- normalizeVolume's fallback is typed unknown (its guard was unreachable) and
applyVolume takes a number again
- the throttle tests inject a clock instead of patching global time.monotonic
- run_ffmpeg stops the process from ONE exit path (`except BaseException`), so
an exception out of on_progress (a DB write) can no longer orphan a live
ffmpeg after a 10s join on its pipes; _stop is now a no-op on a dead process
- both run_ffmpeg callbacks are throttled to ~1s (they were a DB round-trip per
output line: ~10^5 queries on a long re-encode), mirroring the download hook
- volume arithmetic extracted to lib/playerVolume with unit tests; a corrupt
level now falls back to the LAST GOOD one instead of full blast, and the
sanitising happens where the stored value enters
- RSS is back to two except clauses (the shared tail already sits after the try)
- new tests: callback-exception cleanup, the two throttles; the fake Popen grew
poll() and its wait count is now asserted
Mutation-checked: each new group goes red when its fix is reverted.
- Feed memoizes onClose (like Playlists already did): the inline closure re-ran
PlayerModal's keydown effect on every Feed render, which since the debounce
landed also flushed the volume write and re-stole focus
- the player reads its volume with readAccountMerged again — a partial stored
object made the level undefined -> NaN -> a persisted null; applyVolume also
refuses a non-finite level outright
- that read moved into a lazy useState initializer (was running per render)
- the two RSS except branches share one tail, so a counter can't drift again
- run_ffmpeg tests cover the SIGTERM->SIGKILL escalation and the lingering-
process path; dropped an assertion that only restated the fixture
- storage.ts documents the actual { volume } shape (0 IS muted)
- the sweep guard anchors the rating_key to digits: the round-2 widening had
degenerated to "anything ending in _<digits>" (backup_2024, release_10)
- RSS counts apply-side failures too, so a read-only database no longer reports
a wholly failed poll as "ok — new=0, failed=0"
- the player reads/writes its volume directly (readAccount/writeAccount): the
persisted-object hook rendered nothing and cost a second write per settle
- new tests: the sweep guard's accept/reject table, and run_ffmpeg's threading
(drain, cancel while silent, stall watchdog, stderr tail on a nonzero exit)
- the breaker sentinel is checked one way (isinstance) at all four sites
- run_rss_poll is typed dict[str, int]
All three new test groups were mutation-checked: reverting each fix turns them
red (or, for the cancel/stall pair, hangs — which is the bug itself).
- the volume keys no longer defer to a scrollable card while in fullscreen
(mirrored into a ref: the keydown handler is bound once and state would be stale)
- the debounced volume flush writes straight to storage; persisting inside a
setState updater is not guaranteed to run for an unmounting component
- the HLS sweep also matches the original {key}_{start} directory naming, which
is what the oldest leftovers on disk actually use
- run_rss_poll returns {new, failed} so an egress outage stops rendering as
"ok — 0" in the scheduler card and the audit log; /api/sync/rss reports it too
- drop the unused poll_rss_channel (it silently gained a new exception)
- _Breaker.run is generic, so a call site keeps its item-id type
- test helper instead of the generator-throw idiom
- HLS sweep only deletes directories matching the session-name shape; the root
is admin-configurable and may be a shared path
- playlist unwrap picks the first entry that actually downloaded, not entries[0]
- concat quoting keeps backslashes literal (ffmpeg treats them so inside '')
- ArrowUp/Down defer while the focused element can still scroll that way
- the staging fallback skips yt-dlp working files (.part-Frag/.ytdl/.temp/.fNNN)
- the boot HLS sweep runs off the event loop
- volume persistence is debounced (was a setItem per wheel notch) and a spin no
longer loses notches to the iframe's lagging getVolume
- _Breaker.run owns the whole open/call/record protocol; four call sites shrink
- should_prune extracted and unit-tested, plus RSS error-vs-empty tests
- the worker exits non-zero when the schema never lands
- the interaction overlay yields fully in fullscreen: YouTube only reveals its
control bar while the iframe itself sees the mouse, so after pressing F the
native fullscreen button was unreachable
- remember the volume per account (a YT.Player is created per video and always
starts at 100), plus ArrowUp/ArrowDown volume keys that work in fullscreen
- Ctrl/Cmd+wheel falls through to the browser's zoom instead of the volume
ffprobe prints codec_name BEFORE codec_type, so the line parser attributed every
name to the previous stream: a plain H.264+AAC mp4 probed as (aac, None) and the
browser-playability check re-encoded a perfectly good file on EVERY download.
Verified on localdev — the same source now stores without a re-encode pass.
- never prune subscriptions on an empty fetch: one anomalous empty-but-200
response wiped every subscription (and its deep_requested flag) in one run
- push_playlist: stop after 5 consecutive write failures — a quota-dead or
scope-revoked account otherwise burns 50 units per remaining item
- RSS: raise RssError instead of returning [] on a transport/HTTP failure, so a
failed poll no longer stamps last_rss_at and a 404 feed stops looking healthy
- worker: exit when the schema never appears instead of "continuing" into a
crash one call later
- pick the produced file safely: Path("") is Path("."), which passes .exists(),
so a missing yt-dlp filepath skipped the staging fallback and crashed in
with_name ("PosixPath('.') has an empty name") on non-YouTube sources
- unwrap a playlist/multi_video extraction to the entry that carries the media
- run_ffmpeg: drain stderr while it runs (a full 64KB pipe blocked ffmpeg
forever and hung the worker thread), 1s cancel polling, stall watchdog and a
bounded wait on exit
- escape apostrophes/backslashes in the ffmpeg concat list (pre-0.51.0 names)
- sweep orphaned HLS segment dirs at startup (nothing else ever reaped them)
Review follow-up on the share-link password UI:
- Removing a link's password makes it publicly watchable, a protection
downgrade; gate it behind a danger-confirm like the neighbouring Revoke,
spelling out that anyone with the URL can then watch without a password.
- When setting a first password the input reused the create-form "Password
(optional)" placeholder, but it's required here (Save stays disabled until
non-empty); use the "Set a password" wording instead.
C-3.7 made a password rotation invalidate outstanding grants, but ShareDialog
only let you set a password at link creation — there was no way to rotate or
clear it afterward, so the protection wasn't reachable. Add a lock control to
each existing link row: set / change / remove the password via the existing
update_link endpoint (which bumps password_version and revokes old grants).
The atomic claim added in trigger_job only covered the manual path; a scheduled
fire sets running via _job without claiming, so a manual trigger racing it could
still double-run, and the claim (set outside _job) could leak if _job threw
before its own cleanup. Move _claim_running to the top of _job — the one point
both the scheduled fire and the manual trigger pass through — and release it in
_job's finally, so exactly one run wins whichever path it comes from and a
failure can never wedge the job "running". trigger_job's _is_running check is now
just a best-effort fast answer for the UI. Unit-test _claim_running (and
is_messageable_user).
- C-3.7: fold a per-link password_version into the signed watch grant (migration
0059 adds the column) and bump it on every password change, so rotating a share
link's password invalidates outstanding grants at once instead of letting them
live out the 6h TTL. make_grant/check_grant take the version; update_link bumps.
- C-3.8: the share recipient picker and share-by-email now reuse the messageable
filter (not-demo AND active AND not-suspended) instead of only excluding demo,
so suspended/deactivated addresses aren't leaked or reachable as targets.
- C-3.9: GET /keys/{user_id} adopts get_thread's MB2 guard — a non-messageable
target with no shared history returns the same "User not found" as a missing id,
so it can't be probed to enumerate users or fetch suspended users' keys.
- C-3.12: _register_account catches IntegrityError on the insert (the select-then-
insert TOCTOU) and treats it as the already-registered no-op.
- C-3.13: trigger_job claims the running flag atomically under _activity_lock
(compare-and-set) instead of a bare check, closing the double-start window.
- Move _messageable/is_messageable_user into a shared app/userscope.py so downloads
doesn't import the messages route module.
The decrypt-pass refactor hoisted partnerPub() above the loop and outside the
try, so a keyless partner (one who reset and hasn't re-set-up) or a fetch error
rejected the whole pass unhandled (setPlain never ran). Fetch the key lazily
inside the try and once, so an empty thread never fetches and a fetch failure
degrades to "can't decrypt" as before. Document the send-path staleness (a first
proactive send after a partner's key rotation uses the stale cached key) as a
known limitation alongside refreshPartnerPub.
- On a decrypt failure, refresh the partner's cached public key (and drop the
stale derived conversation key) once per pass and retry. A passphrase reset
rotates a keypair, so a partner who cached the old public key would otherwise
encrypt undecryptable messages until a reload; the first failed decrypt now
self-heals reads AND future sends. (pubCache was assumed set-once.)
- KeyGate reads has_password via a reactive useQuery(['me'], enabled:false)
passive observer instead of a one-shot getQueryData, so the reset password
field stays correct even if me resolves after mount.
- Document the remaining multi-device staleness (a reset on another device
leaves this one 'ready' with the dead key) as a known limitation in useKeyState.
The set-once key guard made a lost passphrase a permanent wall (409 on re-setup,
KeyGate offered only unlock/setup). Add an escape hatch:
- POST /api/messages/keys/reset drops the user's MessageKey so a new passphrase
can be set. Because every conversation key is derived from the keypair,
replacing it orphans all stored ciphertext in both directions, so the now-dead
`user` messages are deleted too (clean slate; system messages untouched).
Password accounts must re-verify (a hijacked session must not silently reset
messaging and read future messages); Google-only accounts rely on the session
+ the client danger-confirm. Rate-limited per user.
- KeyGate gains a "Forgot passphrase?" path in unlock mode → a danger reset view
(spells out the permanent history loss; a current-password field for password
accounts) → resets and returns to a fresh setup.
- e2ee.resetLocal wipes this device's key + derived conversation keys + the
IndexedDB copy; api.resetMessageKey; HU/EN strings.
Re-add the 127.0.0.1 callbacks to the dev host allowlist (now registered in the
Google console alongside the localhost ones). Deriving the redirect_uri from the
request host keeps the whole round-trip — login, Google callback, and the
session/state cookie — on one origin; a cross-host fallback (127.0.0.1 → the
fixed localhost callback) would silently lose the OAuth state, since a session
cookie set on 127.0.0.1 is never sent to localhost. Clarify that in the comment
and cover all four hosts in the unit test.
- OAuth-cancel feedback now keys on the live session, not link_uid: ANY
signed-in cancel (add-another-account via /auth/login, link, or upgrade)
redirects to /?oauth=cancelled and gets a neutral "Google sign-in was
cancelled" toast — the add-account path set no marker and was silent, and
the shared ?link= message wrongly called an upgrade "account linking"
- drop 127.0.0.1 from the dev OAuth host allowlist: only the localhost
callbacks are registered in the Google console, so a 127.0.0.1 origin now
falls back to the fixed callback instead of a redirect_uri_mismatch
- Welcome: extract the shared goResend handler (banner CTA + sign-in link) and
merge the two adjacent signin-only blocks
- link/upgrade OAuth cancel now redirects to /?link=cancelled (a channel App.tsx
surfaces as a toast) instead of the pre-auth /?login=oauth_cancelled banner a
signed-in user never sees; a plain sign-in cancel is unchanged
- the login banner (suspended / oauth_cancelled) is now state, cleared on the
first deliberate action like the verify banner — no more stale lingering
- add a "Didn't get the verification email?" resend entry on the sign-in screen
so a never-arrived verification mail isn't a dead end (was only reachable from
the expired-link banner)
- unit-test _oauth_redirect_uri (dev-host derive, unknown-host fallback, prod
ignores the Host header — the injection guard)
- collapse the 5-deep title/button ternaries into Record<Mode,string> maps
A Google login started on the Vite dev server (:5173) always came back to the
fixed :8080 callback, because login/link/upgrade passed the static
settings.oauth_redirect_url. Derive the redirect_uri from the request origin
instead, but only in local dev and only for a known dev-host allowlist —
production still uses the fixed configured URL and never trusts the Host header
(host-injection guard). Set the Vite proxy to changeOrigin:false so it forwards
the real Host:localhost:5173 (it was rewriting it to the 127.0.0.1:8080 target).
Both callbacks must be registered in the Google console.
The verifyInvalid banner (a one-time entry notice) lingered on later screens
after the user clicked its "Send a new link" CTA and returned to sign-in.
Clear the verify state in reset(), so any deliberate action — mode switch,
CTA, or submit — dismisses it.
- password reset also sets email_verified (the mail proves the mailbox),
rescuing an account that never clicked the verify link; is_active (admin
approval) is deliberately left untouched
- add POST /auth/verify/resend (rate-limited, off-response-path, anti-enum)
+ a "resend" mode/CTA on the verifyInvalid banner, so an expired/lost
verification link is no longer a dead end (re-register was a silent no-op)
- OAuth cancel/deny now redirects to /?login=oauth_cancelled with a friendly
banner instead of a raw 400 JSON page; pop the oauth_link markers BEFORE the
token exchange so an aborted link flow can't poison the next clean sign-in
The 'usage' query rendered both its loading and its error state as 'No usage' (the same false-empty
R3 targets), while the sibling 'status' query got a proper state. Split the fallback into
error(+retry) / loading / empty.
- notifyYouTubeActionError: on any non-403 error, show the server's own reason (err.detail, e.g.
"Not enough quota left today") instead of the caller's generic fallback, which was discarded
(U-3.2.8). 403 still offers the Connect affordance.
- App boot error now offers a Retry (StateMessage) instead of a dead "Something went wrong" (U-4.2).
- deletePlaylist prefers the server's reason over a blanket "couldn't delete on YouTube" that
misleads when the failure wasn't the YouTube side.
- Already covered in earlier sprints (verified): the live-search "Load more" already surfaces the
quota error inline via err.detail, and ChannelPage subscribe/unsubscribe already route through
notifyYouTubeActionError.
Replaces the plain "Loading…" text in every R3 loading branch with a single shared LoadingState
(a spinning lucide Loader2 in the accent colour + label, centered, role=status/aria-live) — one
component so all loads read the same and can be swapped for skeletons later in one place. Applied
across Feed, Channels, ChannelDiscovery, AuditLog, NotificationsPanel, Messages, Scheduler, Stats,
PlexBrowse (grid + subviews), PlexPlaylistView, ConfigPanel, DownloadCenter, AdminUsers, ChatThread.
Left the small "load more" footer and lazy Suspense fallbacks as-is.
- Blank-flash (showed an empty state on first load before data): DownloadCenter (queue + library
tabs), AdminUsers (roles + invites), ChatThread, PlaylistsRail now show a loading indicator
(and an error+retry) instead of a false-empty flash — guarded on `&& !data` so live data isn't
disturbed on refetch.
- Forever-loading (`isLoading || !data` loops forever on a failed fetch): ConfigPanel, the three
PlexBrowse subviews (playlist/show/season), and Stats' admin dashboard get an `isError && !data`
error+retry path before the loading branch.
- Playlists' detail pane already had an error path (isError+retry); its list lives in PlaylistsRail
(now covered), so no change needed there.
- The honest-state branch now guards on `isError && !data`, so a background/window-focus refetch
failure keeps already-visible content instead of replacing the list with a full error screen
(regression the bare `isError` introduced). Applied to AuditLog, Channels, ChannelDiscovery,
PlexBrowse, PlexPlaylistView, Messages(people), Feed — matching the guard already used in
Messages(conversations)/NotificationsPanel/Scheduler.
- Drop the unused DataTable loading/error/onRetry props (every table page uses the outer
StateMessage branch, not the props) — reverts DataTable to its simple empty block.
- StateMessage announces errors with role=alert/aria-live=assertive (was polite for all tones).
- Remove the now-unused common.empty string (en+hu), left over from the removed QueryState render-prop.
The error state didn't exist as a concept: a failed main query rendered as an "empty" list
("No channels" while the request actually died), so a backend restart read as "everything is
empty". Add the convention that an empty state may NEVER show while a query is erroring.
- New components/QueryState.tsx: shared StateMessage (loading / error+Retry, house style, with
role=status/aria-live — the app had zero live regions).
- DataTable gains loading/error/onRetry props (suppresses empty-text on error).
- isError branch (before the empty branch) added across the false-empty pages: Feed (retry),
Channels, ChannelDiscovery, AuditLog, NotificationsPanel, Messages (conversations + people),
Scheduler, Stats, PlexBrowse grid, PlexPlaylistView. ChannelPage's main content is Feed, now
covered. common.loadError/empty strings (en+hu).
Follow-up to user prod testing of 0.51.0:
- re-download now clears job.display_name so the card title + Content-Disposition filename are
re-derived from the freshly-cleaned title (previously an auto-name from an older un-cleaned
title — still carrying a "… views · … reactions |" prefix — survived the rebuild).
- on-disk AND served names are now a strict shell-safe slug: only [A-Za-z0-9._-] survive; every
other char (the "!" the user saw, plus ( ) [ ] & $ ' " ; | ? * …) collapses to "_". The id no
longer uses [brackets] (glob metacharacters). Shared _slug helper for sanitize + display_filename.
- GC gains a 5th pass: sweep on-disk files/dirs no ready asset owns (old accented channel dirs, a
poster from a superseded naming scheme left after a re-download), skipping the dot system trees
and anything newer than an hour so an in-flight download is never touched.
- Tests: slug strips !/$/()[]; rel_path is bracket-free; orphan-sweep keeps owned/system/fresh files.
- redownload now ALWAYS rebuilds into a fresh private asset instead of ever reusing a shared
cached file: the cache sig is salted with job_id + current asset_id, so the fork is distinct
from both the dedup pool and the job's own current asset. This resolves the co-held silent
no-op (finding #1) — a shared asset is left intact for its other holders while THIS job
re-fetches its own copy. Resolving the fork BEFORE releasing the old hold (and the unique
salted key) also removes the IntegrityError-rollback-after-file-delete window (finding #2).
- format_sig gains an optional `salt` for that fork identity.
- titles: don't consume the space after `|`, so a "stats | #hashtags" remainder keeps the
leading whitespace _TRAILING_HASHTAGS needs — and fall back to the prefix+hashtag-stripped
text, so neither the stats nor the SEO hashtags are resurrected when a title empties (#3).
- worker: early-fill asset.title falls back to the video id so a title that cleaned to empty
doesn't show a blank card while downloading (#5).
- Tests for the salt fork and the hashtag-remainder case.
(Finding #4 — quota on re-download — intentionally not changed: a re-download reuses the
existing job and re-fetches ~the same bytes, so calling check_enqueue would wrongly reject on
the job-count cap while footprint is unchanged.)
- redownload: never clobber a co-held (deduped) asset. Drop the force-reset-to-pending;
_release_asset already deletes+recreates a solo asset (genuine re-fetch), while a shared
ready asset is left intact and the re-queued job re-points to it via _finish_from_cache —
so one user's re-download can't delete another's file mid-serve (finding #1).
- redownload: reset queue_pos to the tail so a re-download doesn't front-run newer queued
jobs on the worker's `ORDER BY queue_pos` claim (finding #3). Made service.next_queue_pos public.
- formats: video-only mp4 selector prefers every video-only option before any progressive
`best`, so a "Video only" download stays audio-free whenever a video-only stream exists (#2).
- titles: a title that is only the engagement prefix no longer resurrects the stripped stats
(return the stripped text / empty, not the raw) (#4).
- worker: probe the file's duration when yt-dlp gave none, so a long re-encode shows real
progress instead of a frozen 0% (#5); tighten the ensure_browser_playable docstring to its
actual mp4-profile scope (#6).
Mini-epic "Downloads correctness & UX" (S1–S3), on top of the iOS-AV1 + 720p fixes.
S1 — naming & titles:
- ASCII-fold on-disk dir/file names AND the served (Content-Disposition) filename: no
spaces (underscore-joined), no accents (á→a, ő→o, ß→ss), pure ASCII — portable and free
of the `?` mojibake accented names show in non-UTF-8 tools. (Forward-only; existing files
clean up via re-download.)
- Strip a leading social engagement prefix ("1.6M views · 66K reactions | …") from titles.
S2 — universal browser playability (generalizes the iOS re-encode):
- ensure_browser_playable guarantees the stored file is H.264 + AAC/MP3 in mp4 — the only
combo every browser decodes. Re-encode just the offending stream(s) (video for AV1/VP9/HEVC,
audio for Opus/etc), or remux when only the container is wrong. Gated on the mp4 compat
profile (an explicit vcodec / non-mp4 custom profile opts out).
S3 — force re-download (preserves share links):
- POST /downloads/{job_id}/redownload keeps the job row so its public DownloadLink survives
and resolves to the freshly-downloaded file; deletes the old file and re-fetches under the
current rules. A failed re-download leaves the job in error (the intended broken-link case).
- Library "Re-download" action (icon + confirm), api method, hu/en strings.
Tests: naming/title-strip (test_naming), browser_transcode_flags matrix; existing
download_filename expectation updated for the underscore policy.
Shared /watch links showed a "broken play button" on iPad (any iOS
browser — all forced onto WebKit) while playing fine on desktop Chrome.
Root cause: the stored mp4's video codec was AV1 (confirmed via ffprobe),
which WebKit can't decode. The existing H.264 guard was only a
format_sort *preference*, so a non-YouTube source (Facebook) that ships
AV1 as its best video-only stream slipped through.
- B1: for the mp4 compat profile, prefer H.264 at the SELECTION level
with a progressive fallback (bestvideo[vcodec^=avc1]+bestaudio /
best[vcodec^=avc1] / anything) so an avc1 progressive is chosen over
an AV1-only video-only stream.
- B2: re-encode the rare AV1/VP9-only result to H.264+AAC (+faststart)
in the worker before storing, gated on the compat profile; corrects
the asset codec metadata to match.
- Bump _SIG_VERSION 2->3 so cached AV1 assets re-derive.
- Default download preset is now 720p (migration 0058 re-seeds builtins
720p-first; ProfileEditor BLANK 1080->720).
- Tests for the selector, compat opt-outs, and the transcode predicate.
/code-review (S1, finding 3): C-3.19 switched the banner from CURRENT_VERSION (always
RELEASE_NOTES[0]) to FRONTEND_VERSION (VITE_APP_VERSION), which is "dev" in the Vite
dev server and can drift from the notes — so the modal highlighted nothing. The modal
(which already imports RELEASE_NOTES, keeping App decoupled) now falls back to the
newest note when the passed version isn't listed.
The demo account is shared, so writing deep_requested onto its subscription polluted
shared state (harmless today — the scheduler's deep paths are human-only — but a
future scheduler path trusting the flag would leak quota). Guard the flag WRITE at the
source (not user.is_demo), which also makes deep_turned_on always False for demo, so
the redundant is_demo check on the immediate backfill is removed.
/code-review (S1, finding 1): sanitize truncated the title at 120 CHARACTERS, but a
path component's hard limit is 255 BYTES and rel_path packs channel + date + id + ext
into that same component — so a long accented/multi-byte title (Hungarian titles are
common here) could already overflow to ENAMETOOLONG, and the new _a{id} suffix shrank
the margin further. sanitize now truncates on a UTF-8 boundary by byte length, so the
title (≤120B) + channel (≤80B) + fixed parts stay comfortably under 255B. Two tests:
byte-cap on accented input, and every rel_path component ≤255B for a pathological
long-accented channel+title+asset_id.
/code-review caught that role="status" on the toast container implies
aria-atomic="true", so every new toast made a screen reader re-read the entire
stack instead of just the new one — the opposite of the U-F fix's intent. Use a bare
aria-live="polite" (defaults atomic=false, announcing only the added toast); error
toasts keep role="alert". Also simplified the conditional role to a plain ternary.
Review found my C-3.23 additions double-notified: the global error modal (api.ts req)
already surfaces 400/409/422/500 with the backend's specific message, so a caller
toast on those codes is a second, weaker surface. Only 403/404 are caller-handled and
silent — and only a YouTube WRITE action has a meaningful 403 (missing scope).
So the correct set is just the ChannelPage unsubscribe handler (a YouTube write, 403 →
"connect", matching subscribe). Reverted the redundant onErrors on block (a local DB
op, not YouTube), SettingsPanel Plex watch toggle/reimport, and the DownloadCenter job
action — the global modal already speaks for them. Removed the three now-unused keys.
Toaster: moved the polite live region onto the stable, always-mounted container (a
polite region must pre-exist to announce later insertions); error/fatal toasts keep
role="alert", which announces on insertion regardless.
peter