A delete landing while a push read YouTube left a plan over no items, so the push emptied the
YouTube playlist a local-only delete keeps: the push now stops at a vanished list (PlaylistGone,
404), and a delete is refused while a sync of the list runs. The closing baseline fingerprints the
title YouTube got, not the one its lock re-reads, so a rename made during the writes stays dirty.
The other-tab test helper bounds its lock wait, so a lock-order regression fails instead of hanging.
The mirror refresh re-ingested inside its row lock, and that ingest's commit let go of the lock
before the replace: a remove made then was undone and the list read as synced. Push, re-pull and a
delete touched items before the list row, a deadlock beside the playlist-first route edits. A push
now re-plans from the items under the lock and derives dirty after its YouTube writes.
lock_playlists moves to the sync package. Tests pin both ids of a move's lock and the claim's
own connection.
- Every playlist item write (add, remove, move, reorder, rename; Plex remove and remove-bulk) row-locks
the playlist before it touches an item, a move both lists in id order: remove beside reorder was a
deadlock, and two adds of one video a unique-key 500.
- The scheduled mirror refresh re-reads dirty under that lock, so an edit made during its YouTube calls
is not replaced by YouTube's order.
- A user's bulk pull holds a per-user advisory claim: two at once created duplicate mirrors. The manual
sync answers 409 sync_all_busy instead of '0 synced', and like the re-pull spends its cooldown only
on a pull that runs.
- Deleted mirrors are committed at once, not only when a later list commits.
- The claims open an unpooled connection of their own instead of a third pooled one per push.
The client's in-flight flag covers the menus of one page only. A push reads YouTube, then writes
both sides for seconds and commits step by step, so a row lock ends at its first commit: two tabs,
or a push and the scheduled mirror refresh, could interleave into a doubled YouTube item or a
baseline recorded over the other's writes. `sync_claim` takes a session-level advisory lock on a
connection of its own (it survives those commits; a crashed worker's lock dies with its
connection, so no claim column is left stuck). Push and revert answer 409 `sync_busy`; the
scheduled refresh skips a list being synced and picks it up next run.
The reads the frontend polls (unread counts, /api/me, the download index,
sync status, subtitle progress) are logged at DEBUG: at INFO they were ~95%
of the stored log on prod. A scheduler job failure and a playlist sync
failure carry event codes, and a Google account without a YouTube channel
has an empty playlist list instead of a warning every six hours.
- yt-dlp's text is scrubbed wherever it is logged, not only in the adapter:
`formats.log_safe` takes out the URL, the `[extractor] <id>:` prefix and
file paths, and the job-failed, refused, edit-failed and backfill lines use
it. Done lines log the asset id, not the title-built path. The PII contract
now bans `rel`/`rel_path`/`title`.
- A traceback goes with a download/edit failure only when it is our own code
failing; for yt-dlp's DownloadError, our RuntimeErrors and OSErrors its foot
is the unscrubbed message.
- yt-dlp's `error` maps to WARNING: it fires on attempts the fallback client
set recovers from; the job's verdict is `download.job.failed`.
- The sink groups a bare `%s` pass-through by its text (numbers folded), and
a process's last batch closes the repeat window, so a short-lived download
child still reports what it dropped.
- Worker-side failures (stall, wall-clock, no asset, cached asset error) log
`download.job.failed` with a cause; they left no line at all before. The
crash, supervisor-error and non-zero-exit lines carry the code too.
- An edit failure is one coded line in `_fail_edit`, for every path.
- `logs.get_logger("__main__")` resolves to the `-m` module, so the worker
and the download child keep their names.
- TRACE lines carrying user content are gated on `logs.consented_trace()`,
not on the process level; a contract test pins it.
- The OAuth token refresh goes through `_upstream_failure`; transport errors
carry reason `transport`. Subscribe recognises a duplicate by reason code
only (the prose fallback was dead).
- Once-only guards for a stale sysconfig override and a refusing HLS session
directory; item_detail splits Plex failures from our own parse bugs.
- The playlist breaker line names the playlist and the action; comments that
were false (runner noqa, `_merge_vtt`, sink grouping) are corrected.
114 exception handlers neither logged nor re-raised. The defect was never that
they are silent — plenty of them should be — it is that their silence said
nothing, so nobody could tell a decision from an oversight. Each one has now been
read. 44 got a line; the other 70 carry `# silent-ok: <why>`.
What the silent ones had been hiding, in the cases that got a line:
- A failed ffprobe sent every HDR file down the 8-bit pipeline, so playback
degraded to a software transcode with nothing saying why; a failed audio probe
collapsed a multi-audio file to one track and the player showed no language
picker; a failed first-PTS probe left subtitles seconds out of sync.
- A subtitle sidecar that existed but would not parse made a PARTIAL track read as
complete, permanently — the viewer loses every cue past the cached window.
- An unreadable crop/trim/cut-list in an edit spec was dropped, so the clip came
out uncropped, untrimmed, or shorter than the one the user built.
- A dropped `max_height` downloaded at best quality against an explicit cap.
- A failed videos.list enrich made every candidate undroppable-from-judgement and
therefore dropped: a working search rendered as "no results".
- An unusable password hash answered as "wrong password", locking the owner out
and telling them they mistyped; a rotated TOKEN_ENCRYPTION_KEY turned every
stored token into "not connected".
- A stored admin setting that would not coerce fell back to the env default while
the settings page went on showing the saved value.
- A one-off InnerTube config failure pinned the whole process to the hard-coded
fallback key, and searches return nothing when that goes stale.
- The HLS sweep reported 0 swept — like a clean run — when it could not list its
own root, leaving every crashed session's segments on disk forever.
Four of them are counted rather than logged per item (dropped cues, dropped cut
list segments, unreclaimed GC files) and reported once per call, because the
alternative is one line per cue over a whole subtitle track.
ruff gains S110 and BLE001. Every blind `except Exception` that remains carries
`# noqa: BLE001 — <why>`, which is the point: catching everything is sometimes
right, and always worth a sentence. plex/sync.py's three best-effort backfill
catches now keep the traceback — a blind catch over a batch that fetches AND
writes could be Plex, the database or our own bug, and `%s` of the exception told
them apart badly. main.py's boot-time log-level read likewise.
logs.event's `code` is positional-only now: `event("x", code=...)` was a TypeError,
and `code` is exactly the word a caller reaches for when the thing that failed has
one of its own.
tests/test_swallowed_exceptions.py is the wider rule ruff cannot see: a handler
that neither logs nor re-raises must say why. Both cases mutation-checked — a
marker removed and a log line removed each fail it, naming the handler.
25 literal names covered 34 modules. plex/client.py, plex/stream.py,
plex/subtitles.py, plex/sync.py and two Plex route modules all called themselves
`siftlode.plex`; downloads/pipeline.py called itself `siftlode.worker`;
routes/channels.py called itself `siftlode.api`. The `logger` column is in every
stored row so E22.6's console can filter on it, and so an operator can pin a level
to one part of the system — neither works when six modules share a name.
`logs.get_logger(__name__)` derives it: app/plex/sync.py -> siftlode.plex.sync.
One name per module, and it cannot drift from the file it sits in.
tests/test_logger_names.py pins the derivation, that everything stays under the
one `siftlode` tree the handler and the tests attach to, and that no module
hand-rolls a name again unless the line says why (`# logger-name-ok: <why>`).
Two do: get_logger's own call, and `siftlode.http`, which is named for the traffic
it carries rather than the module it lives in — it replaced uvicorn's access log
and is what an operator pins when they want request lines and nothing else.
app/scheduler.py called its logger `logger`; every other module calls it `log`.
ruff gains LOG, G, T20, TRY400, TRY401 and S112. Four findings cleared with them:
worker.py logged `str(exc)[:120]` where the traceback was the whole question, and
three prints/f-strings that are deliberate now say so on the line. S110 and BLE001
stay off until S3 — 23 handlers are outstanding and each is a judgement about one
of them; enabling the rules first would just mean 23 blanket noqas.
- Button: `loading` always disables (|| not ??) — stops a double-submit
when a call-site passes an explicit boolean `disabled` (VideoEditor).
- playlists: track un-ingestable-but-live YouTube ids in the sync baseline
so a linked playlist containing a private/deleted video converges instead
of re-classifying it as a pending pull forever; re-attempt ingestion each
sync so a restored (private->public) video heals, and prune ids that
leave YouTube.
- SettingsPanel: download-auth section renders an honest error/loading
state (StateMessage + retry) instead of a bare empty section on a failed
credentials fetch; HU+EN loadError string.
- tests: reorder-order, partial-failure baseline hold, un-ingestable
converge/heal/prune, Button loading-disables, download-auth error/retry.
"Sync to YouTube" was a one-directional destructive push (local wins, deletes
from YouTube every item not in the local list), so items added on YouTube were
lost and a dirty pull was skipped entirely. Replace it with a real 3-way merge:
each item is classified against the last-synced baseline as a local-add, a
YouTube-add, or a genuine delete, so an add or delete on either side survives.
- Persist the ordered baseline id set (new playlists.synced_video_ids, migration
0075); the fingerprint hash alone couldn't drive a merge. NULL baseline (fresh
export / pre-S7 link) unions losslessly on the first sync and records one.
- Pure merge_plan(baseline, local, youtube) -> add/remove per side + merged order
(local order, YouTube-only additions appended). merge_playlist applies the
local side directly and the YouTube side through the failure breaker; the
baseline only advances on full success so a partial failure re-converges.
- plan_merge/push-plan now report what each side gains and loses; the confirm
dialog shows the two-way plan before applying (pushPlanMerge, HU+EN).
- Tests: 10 new (pure classification incl. delete-vs-stale-copy; DB apply proving
a YouTube-add and a Siftlode-add both survive on both sides, a genuine delete
propagates without collateral loss, no-baseline never deletes, create path).
This codebase leans on "why" comments as its architecture doc, so a header that
claims the opposite of the code below it is worse than none. Fix the seven the
review flagged: channels.py (subscribe/unsubscribe shipped), playlists.py +
sync/playlists.py (YouTube two-way sync + write-back shipped), setup.py (drop the
"epic 6c" ref — the wizard steps exist), config.py ("auth._client_ip" ->
"routes.auth._client_ip"), db.py ("routes/plex.py" -> "routes/plex/" after the
package split), sysconfig.py (the "Phase C" note — the watch-sync job that reads
plex_watch_sync_interval_min has landed; clarified it's a boot default, not a knob).
Docs only.
A nullable published_at forced the feed keyset into a non-sargable
`... OR published_at IS NULL` disjunction: with the 0067 index the planner still
applied it as a FILTER and scanned from the top of the index, so a deep `newest`
page discarded every earlier row (O(depth)).
- Migration 0068: backfill any NULLs (0 in prod) -> SET NOT NULL + DEFAULT now().
- Model: Video.published_at is now non-Optional, nullable=False, server_default now().
- feedquery: the `pub` key is nullable=False, and keyset_predicate is rebuilt
recursively so a non-nullable leading key gets a non-strict range bound
(`published_at <= v`) as an AND conjunct — an index range Cond instead of a
filter. Nullable keys keep the NULL-aware OR form (a `<= v` bound would drop
their NULL group). Equivalence proven by test_feed_keyset (all 13 sorts,
cursor-walk == unpaged).
- Ingest paths (stub_from_playlist_item, apply_video_details, playlists sync)
fall back to now() so they never write an explicit NULL.
Measured on the prod-sized localdev reseed at a ~50k-deep cursor (0067 index +
prod planner): OLD `Rows Removed by Filter: 50001`, 25,687 buffers / 9.16 ms ->
NEW `Index Cond: published_at <= v`, 1 row filtered, 47 buffers / 0.037 ms. SET
NOT NULL on 257k rows: 0.32s. Gate green (334 pytest).
- 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
- 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
Verified each deferred per-subsystem item against current source, then fixed the
real ones (tradeoffs left as-is, see siftlode-ops/CODE-HYGIENE.md closeout):
- search BUG-3: don't finalise shorts_probed on un-enriched stubs (enrichment
failure/omitted rows) — restores the scheduler's enriched_at guard so a real
Short can't leak into the feed and never get reclassified.
- downloads GC: 4th pass reaps orphaned errored, fileless, unreferenced assets
(they carry no TTL, so the expiry passes never cleared them).
- downloads B6: quota/edit errors now return a structured {code,reason,limit}
the trilingual client localises, instead of hardcoded English + dot-decimal GB.
- channels BB1: reset-backfill re-arms deep sync on every subscriber (bulk update)
instead of 404ing when the admin isn't personally subscribed.
- channels BB2: enrich the stub on BOTH the normal and "already exists" desync
path (nest the insert try inside the client `with`).
- channels CB3: drop the redundant second _discovery_rows read (identity-mapped
rows are already enriched; re-sort in Python for the title tie-break).
- playlists PC1: read the live playlist once in push() and share it with plan_push
+ push_playlist (halves read-quota, closes the second TOCTOU window).
- playlists PC2/PC5: batch the cover thumbnails in one window query (was N+1);
rename combines count+duration into one aggregate + a single cover lookup.
- messages MB2: get_thread only resolves a messageable partner OR one we already
share a thread with (closes the user-enumeration oracle).
- messages MB3: push a live unread-count to the user's other tabs after mark-read.
- messages MC4: list_conversations uses DISTINCT ON + grouped COUNT instead of
loading the whole message history into memory.
- config AB3: per-spec allow_empty so smtp_user/youtube_api_proxy can be blanked
to disable them rather than snapping back to the env default.
- 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.
Machine-baseline harvest (ruff + knip), all tsc/parse-green, no runtime change:
- backend: remove 3 unused imports (channels/playlists/youtube) via ruff; drop the
unused `job` binding in unshare_download (keep the _own_job ownership guard call).
- frontend: remove `export` from 23 internally-used-only symbols (knip "unused
exports") to shrink the public surface; delete 2 genuinely-dead declarations
(PlexBrowseResult — leftover from the removed /browse route; WIDGET_TITLES —
hardcoded English titles superseded by i18n).
Held back for a decision (unused here = possibly-unwired, NOT dead — flagged, not
removed): e2ee.lock()/clearDevice() (security primitives never wired to logout / a
"forget device" feature) and loadDefaultViewFilters (App reimplements it inline — a
DRY issue). See siftlode-ops/CODE-HYGIENE.md.
- feed_columns() is the single feed-row projection, consumed by both /feed and the
playlist-detail query (was copy-pasted, drift risk on every new column).
- _replace_items_from_youtube() dedups the YouTube playlist-mirror block shared by
sync_user_playlists and repull_playlist.
- _next_playlist_position/_next_item_position/_add_item replace the append-position
idiom (x4) and the exists-then-insert blocks (x2).
Replace the leftover 'subfeed' name across logger names + log_config,
frontend localStorage keys, Postgres user/db/volume defaults in the
compose files, .env.example, config.py, backup/restore scripts and the
README. Pure rename; no behavioural change. localStorage keys move from
subfeed.* to siftlode.* (one-time UI-state reset is acceptable).
The quota-attribution action keys were inconsistent (mixed verbs, ad-hoc names,
English-only labels). Introduce a QuotaAction constants holder (one source of
truth), rename every attribution call site to it, and add migration 0020 to
rename historical quota_events.action values. The display label now resolves
from i18n (quotaActions.<key>, EN/HU/DE) instead of a hard-coded English map.
Scheduler job ids and progress phase labels are a separate namespace and are
left untouched.
Every background job now reports progress, so the dashboard can show a live
bar for any run (not only enrich/backfill/shorts/maintenance): rss_poll and
subscriptions (runner), autotag, and playlist_sync gained progress.report
calls over their loops.
RSS polling now fetches the channel feeds concurrently (16 workers) instead
of one-at-a-time — a slow/unreachable feed no longer blocks the rest, cutting
a full poll of the catalogue from minutes to seconds. Feed fetches are
network-bound and run in the pool; DB inserts stay on the session's thread
(SQLAlchemy sessions aren't thread-safe), applied as each fetch completes.
Split the DB-write half of poll_rss_channel into apply_rss_feed so the fetch
and apply phases compose cleanly.
The session has autoflush off, so recompute_dirty's query read the last-committed
state, not the current request's pending item/order edits — so the first reorder
after a sync compared against itself and missed dirty, and only the next edit showed
it. Flush pending changes before computing the current fingerprint. Verified: a swap
now flips dirty immediately, and reverting to the original clears it.
Store a SHA-256 of each playlist's name + ordered video ids as last synced from /
pushed to YouTube (migration 0013, backfilled for already-clean linked playlists).
'dirty' is now recomputed on every edit by comparing the current fingerprint to that
baseline instead of being stickily set true. So manually restoring the original order
(or undo back to it) clears dirty on its own — the Reset to YouTube button disappears
and no YouTube read is needed. Baseline is set on read-sync, repull (reset), and a
clean push; a missing baseline counts as dirty (unknown YT state). Verified the
migration's fingerprint matches the app's runtime fingerprint exactly.
The in-session undo/redo is lost when switching playlists, leaving no way to undo
local edits to a YouTube-linked list afterwards. Add a per-playlist 'Reset to YouTube'
action (shown when a linked playlist is dirty) that force-repulls that single playlist
from YouTube, replacing its items/order/name and clearing dirty — even though the bulk
read-sync skips dirty playlists. Backend: repull_playlist() + POST /api/playlists/
{id}/revert-youtube (read-scope gated). Confirm dialog (destructive). Trilingual.
Mirrors (source='youtube') were read-only; per the original two-way design they can
now be edited locally (add/remove/reorder/rename) and pushed back. The read-sync
skips a dirty mirror so it won't clobber unpushed edits; a successful push clears
dirty and lets the mirror refresh. _mark_dirty now covers any YouTube-linked list
(exported local or mirror), and rename marks dirty too. push/delete no longer reject
mirrors. Push reconciles the title as well (cheap playlists.list compare, then
playlists.update only if changed) so a local rename doesn't revert on the next pull.
Frontend: editable = not-built-in (was also excluding mirrors); rows/reorder enabled
for all owned lists; AddToPlaylist popover lists mirrors too (with a YT marker); the
origin chip now reads 'edits sync back'. Trilingual.
Local 'user' playlists (not Watch later, not YouTube mirrors) can now be pushed
to YouTube: create a new playlist on first push, else reconcile membership + order
so YouTube matches local (local wins). plan_push computes a dry-run estimate
(inserts/deletes/reorders + quota units + divergence) read from the live YouTube
state; push refuses if the estimate exceeds the remaining daily quota so a big
playlist can't strand half-pushed. Reorder uses an insertion-sort move count that
matches what executes. Edits to a linked playlist set dirty; a successful push
clears it. DELETE accepts on_youtube to also delete the playlist on YouTube. The
read-sync now skips YouTube playlists already owned by a local export, avoiding
duplicate read-only mirrors.
Mirror each user's own YouTube playlists into local source='youtube' playlists,
one-way (YT -> local). New client methods iter_my_playlists / iter_my_playlist_video_ids
(OAuth, so private playlists work); sync/playlists.py reconciles the mirror (matching YT
order) and ingests any playlist videos not in the shared catalog yet (with stub channels).
POST /api/playlists/sync-youtube for manual sync (read-scope gated, per-user quota) plus a
scheduler job (playlist_sync_minutes, default 6h) that syncs all read-scope users. YouTube's
Watch Later / History are not API-accessible and are never synced.
peter