From 351eb5fd130090a34ee72bca6b7c92b82691ccd4 Mon Sep 17 00:00:00 2001
From: npeter83 <npeter83+claude@gmail.com>
Date: Sun, 26 Jul 2026 19:08:29 +0200
Subject: [PATCH] test(plex)+cleanup(quota): S2 code-review fixes (R6 S2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

/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.
---
 backend/app/quota.py                |  4 +--
 backend/tests/test_plex_sync_txn.py | 44 +++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 backend/tests/test_plex_sync_txn.py

diff --git a/backend/app/quota.py b/backend/app/quota.py
index 25772c7..9d31750 100644
--- a/backend/app/quota.py
+++ b/backend/app/quota.py
@@ -126,7 +126,7 @@ def log_action(user_id: int, action: str) -> None:
 
     Writes in its OWN session (like record_usage) — quota accounting is durable independent of the
     caller's transaction and must never commit the caller's session (that would flush a job's
-    partial state mid-run, defeating its own rollback). See [[quota-owns-its-session]]."""
+    partial state mid-run, defeating its own rollback)."""
     with SessionLocal() as qs:
         qs.add(QuotaEvent(user_id=user_id, action=action, units=0))
         qs.commit()
@@ -158,7 +158,7 @@ def record_usage(units: int) -> None:
     back (or is read-only and never commits) must NOT lose the record — under-counting the shared
     daily budget risks overspending the real API quota. Equally, quota must never commit the
     CALLER's session (the old behaviour) — that persisted a job's partial state mid-run behind its
-    own `except: db.rollback()`. See [[quota-owns-its-session]]."""
+    own `except: db.rollback()`."""
     if units <= 0:
         return
     day = pacific_today()
diff --git a/backend/tests/test_plex_sync_txn.py b/backend/tests/test_plex_sync_txn.py
new file mode 100644
index 0000000..0512a8c
--- /dev/null
+++ b/backend/tests/test_plex_sync_txn.py
@@ -0,0 +1,44 @@
+"""DB-backed test for R6 S2's Plex per-section commit: a PlexError on a LATER library must keep the
+libraries already mirrored (committed per section), not roll the whole reconcile back. Uses the DB
+lane (`db` fixture) with a faked PlexClient — skipped without Postgres.
+"""
+from sqlalchemy import select
+
+from app.plex import sync
+from app.plex.client import PlexError
+from app.models import PlexLibrary
+
+
+class _FakePlex:
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *exc):
+        return False  # don't suppress the PlexError
+
+    def sections(self):
+        return [
+            {"type": "movie", "key": "1", "title": "Lib One"},
+            {"type": "movie", "key": "2", "title": "Lib Two"},
+        ]
+
+
+def test_late_section_failure_keeps_earlier_sections(db, monkeypatch):
+    monkeypatch.setattr(sync.sysconfig, "get_bool", lambda *a, **k: True)  # plex_enabled
+    monkeypatch.setattr(sync, "_enabled_section_keys", lambda db: None)     # all sections wanted
+    monkeypatch.setattr(sync, "PlexClient", lambda db: _FakePlex())
+    monkeypatch.setattr(sync, "_sync_movies", lambda *a, **k: None)         # skip the real Plex fetch
+
+    # Section 1 completes and commits; section 2's collections step raises → its work rolls back.
+    def collections(db, plex, lib, stats):
+        if lib.plex_key == "2":
+            raise PlexError("boom on the second library")
+
+    monkeypatch.setattr(sync, "_sync_collections", collections)
+
+    result = sync.sync(db)
+
+    assert "error" in result  # the run reports the failure honestly
+    # Section 1's library survived the section-2 failure (per-section commit); section 2 rolled back.
+    keys = db.scalars(select(PlexLibrary.plex_key).order_by(PlexLibrary.plex_key)).all()
+    assert keys == ["1"]