From 5ca2a0e6eefc2ddff7f3aad3c2b5e02efd9fcf81 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Mon, 5 Oct 2026 00:50:36 +0200 Subject: [PATCH] fix(gate): the plan-code guard catches bare sprint and phase codes, and the purge takes the P ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sprint alternative matched only `(S6)` or `S3e2`, so the commonest purged form (`# S6:`, `S3's`, `"""S6 —`, `S1 justified`) and every phase code (`P1`) would come back unflagged; the phase codes had in fact survived the purge in 16 lines (Plex client sections, the upload's header shape), now reworded around them. One pattern now covers epic, roadmap, sprint and phase codes; the named exceptions are TV episode notation, a fixture's season or episode title (`title="S1"`) and ruff's `E4/E7/E9`, and a ruff rule like `S110` does not match. The case test lists the purged forms and no longer claims every form. audit_metrics copies the definition (ASCII, as JavaScript's \b is) and reports it as plan_code_lines / plan_code_files. --- backend/app/plex/client.py | 12 +++---- backend/app/routes/downloads.py | 2 +- backend/app/sysconfig.py | 2 +- backend/tests/conftest.py | 2 +- backend/tests/test_upload.py | 4 +-- .../src/components/PlexCollectionEditor.tsx | 2 +- frontend/src/lib/api/core.test.ts | 4 +-- frontend/src/lib/api/core.ts | 2 +- frontend/src/lib/planCodes.contract.test.ts | 32 ++++++++++++------- scripts/audit_metrics.py | 25 +++++++-------- 10 files changed, 47 insertions(+), 40 deletions(-) diff --git a/backend/app/plex/client.py b/backend/app/plex/client.py index 38a698bd..bcdcb605 100644 --- a/backend/app/plex/client.py +++ b/backend/app/plex/client.py @@ -2,7 +2,7 @@ Server-side ONLY: uses the admin ``X-Plex-Token`` from sysconfig. Playback never goes through this client — Siftlode plays the LOCAL physical file (see app.plex.paths). Plex is used purely for -metadata, image proxying, and (P5) optional watch-state write-back. Requests JSON via the +metadata, image proxying, and optional watch-state write-back. Requests JSON via the ``Accept: application/json`` header (Plex serves XML by default). """ @@ -65,7 +65,7 @@ class PlexClient: except ValueError as e: raise PlexError("Plex returned a non-JSON response") from e - # --- Connectivity + sections (P0) --- + # --- Connectivity + sections --- def server_info(self) -> dict: """Root identity (friendlyName / version) — used by the admin 'Test connection'.""" return self._get("/") @@ -75,7 +75,7 @@ class PlexClient: mc = self._get("/library/sections") return mc.get("Directory", []) or [] - # --- Catalog mirror (P1) --- + # --- Catalog mirror --- def section_items( self, key: str, item_type: int, start: int = 0, size: int = 200 ) -> tuple[list[dict], int | None]: @@ -137,7 +137,7 @@ class PlexClient: mc = self._get(f"/library/collections/{rating_key}/children") return mc.get("Metadata", []) or [] - # --- Collection editing (P2 — write-back to Plex; all shared/global, admin-gated in the routes) --- + # --- Collection editing (write-back to Plex; all shared/global, admin-gated in the routes) --- @property def machine_id(self) -> str: """Server machineIdentifier — needed in the `uri` of collection add/create calls. Cached. @@ -206,7 +206,7 @@ class PlexClient: items = mc.get("Metadata", []) or [] return items[0] if items else None - # --- Watch-state write-back (P5 Phase B — Siftlode→Plex push; acts on the token's account) --- + # --- Watch-state write-back (Siftlode→Plex push; acts on the token's account) --- _LIBRARY_IDENTIFIER = "com.plexapp.plugins.library" def scrobble(self, rating_key: str) -> None: @@ -243,7 +243,7 @@ class PlexClient: f"&state={state}&time={int(time_ms)}&duration={int(duration_ms)}", ) - # --- Watch-state read for incremental sync (P5 Phase C) --- + # --- Watch-state read for incremental sync --- def accounts(self) -> list[dict]: """Server accounts (the owner + any managed/home users): id, name. Used to resolve the owner's accountID so the shared watch-history feed can be filtered to just their views.""" diff --git a/backend/app/routes/downloads.py b/backend/app/routes/downloads.py index 6469d76f..cb9e3e23 100644 --- a/backend/app/routes/downloads.py +++ b/backend/app/routes/downloads.py @@ -413,7 +413,7 @@ def enqueue_download( # The upload's filename and display name travel as HEADERS, not as multipart parts. That is not a # style choice: a DECLARED body parameter (`file: UploadFile = File(...)`) is read and spooled by # FastAPI *before the first dependency runs*, so the auth dependency could not gate it and an anonymous -# caller could make the process write the whole per-file allowance to temp, per request (P1; SEC-20 +# caller could make the process write the whole per-file allowance to temp, per request (SEC-20 # bounded the number but could not stop the writing). Reading the body ourselves, from a dependency # declared AFTER the auth one, is the only shape that closes it — the same one # `POST /api/debug-sessions/{sid}/screenshots` uses, and `test_body_limit.py` now enforces it for diff --git a/backend/app/sysconfig.py b/backend/app/sysconfig.py index 35210ffd..daf8b1ce 100644 --- a/backend/app/sysconfig.py +++ b/backend/app/sysconfig.py @@ -140,7 +140,7 @@ SPECS: tuple[ConfigSpec, ...] = ( max=100_000, ), # min=1, NOT 0: a stored 0 used to read as "no per-file cap" (the cap was enforced as - # `if cap and size > cap`, so a falsy value disabled it). Since P1 the route passes this number + # `if cap and size > cap`, so a falsy value disabled it). Now the route passes this number # straight to `capped_stream_to`, where 0 would refuse every upload instead — the opposite # failure, and the reason `config.download_upload_max_bytes` carries its own `ge=1`. Either way # a stored 0 is wrong, so it is floored at 1; the overall footprint is separately bounded by the diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 0caa7b15..d4d142af 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -254,7 +254,7 @@ def as_user(db, override_dep, _configured): @pytest.fixture def upload_headers(): - """Build the headers `POST /api/downloads/upload` takes since P1. + """Build the headers `POST /api/downloads/upload` takes (the file is the raw body). Shared, because two test modules drive that route and a second copy of this is exactly the drift that lets the wire shape change in one place only. The body IS the file now; the names ride as diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index 6bda72d7..39de2ca2 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -600,7 +600,7 @@ def test_the_display_name_header_reaches_the_job_and_is_bounded( ): """The second header, which had no backend test at all — and the length nothing bounded. - Since P1 the display name arrives in a REQUEST HEADER rather than as a form field, so the only + The display name arrives in a REQUEST HEADER rather than as a form field, so the only limit on it was the header size. The column is varchar(255), so a longer one reached Postgres as a DataError: a 500 AFTER the whole file had been transferred. It is truncated now, by the same helper the metadata PATCH uses. @@ -627,7 +627,7 @@ def test_the_display_name_header_reaches_the_job_and_is_bounded( def test_the_upload_holds_no_db_transaction_while_the_body_streams( as_user, make_user, db, _root, monkeypatch, upload_headers ): - """P1 moved the body read behind the auth dependency, which put the whole transfer INSIDE the + """Moving the body read behind the auth dependency put the whole transfer INSIDE the request's database session: resolving the caller and reading the cap are SELECTs, SQLAlchemy 2.0 autobegins on the first one, and the session then holds one of the twenty pooled connections — `idle in transaction` to Postgres, with no `idle_in_transaction_session_timeout` to end it — for diff --git a/frontend/src/components/PlexCollectionEditor.tsx b/frontend/src/components/PlexCollectionEditor.tsx index 37a513cd..3533a13d 100644 --- a/frontend/src/components/PlexCollectionEditor.tsx +++ b/frontend/src/components/PlexCollectionEditor.tsx @@ -14,7 +14,7 @@ import { Pressable } from "./ui/Pressable"; import { Input } from "./ui/Input"; import { SURFACE } from "./ui/surfaces"; -// Admin-only "manage this movie's collections" dialog (Collections P2). Lists the library's EDITABLE +// Admin-only "manage this movie's collections" dialog. Lists the library's EDITABLE // collections with an in/out toggle (add/remove the movie, written back to Plex), plus a field to // create a new collection seeded with this movie. Read-only collections (smart / IMDb-TMDb auto-lists) // never appear here. Opened from the movie info page. diff --git a/frontend/src/lib/api/core.test.ts b/frontend/src/lib/api/core.test.ts index 294faef0..9b68a2a6 100644 --- a/frontend/src/lib/api/core.test.ts +++ b/frontend/src/lib/api/core.test.ts @@ -79,7 +79,7 @@ beforeEach(() => { vi.stubGlobal("XMLHttpRequest", FakeXHR as unknown as typeof XMLHttpRequest); }); -// The upload body is a Blob and only a Blob: `FormData` was accepted until P1 and is gone, because +// The upload body is a Blob and only a Blob: `FormData` was accepted once and is gone, because // a multipart part is a DECLARED body that FastAPI spools before the route's auth dependency runs — // and nothing on the backend can parse one any more (`python-multipart` was dropped with it). const body = () => new Blob(["x"]); @@ -432,7 +432,7 @@ describe("the debug session a tab records into", () => { }); }); -// P1 — the upload's wire shape is a security boundary, not a detail. A multipart part is a DECLARED +// The upload's wire shape is a security boundary, not a detail. A multipart part is a DECLARED // body, which the server reads and spools BEFORE its auth dependency runs; the raw body is what lets // `current_user` resolve first. A revert to FormData here would silently re-open that, and the // backend's own test only sees the shape the client actually sends. diff --git a/frontend/src/lib/api/core.ts b/frontend/src/lib/api/core.ts index eacedb99..2e5011bb 100644 --- a/frontend/src/lib/api/core.ts +++ b/frontend/src/lib/api/core.ts @@ -391,7 +391,7 @@ export async function req( // `HttpError` throw — so callers handle failures exactly as they do a `req` call. // // `body` is a `Blob`/`File` — the bytes themselves, and nothing else. `FormData` was the other -// accepted shape until P1 and is deliberately gone: a multipart part is a DECLARED body, which +// accepted shape once and is deliberately gone: a multipart part is a DECLARED body, which // FastAPI spools to disk before the route's auth dependency runs, and the backend dropped // `python-multipart` altogether, so there is no longer anything on the other end that could parse // one. `cfg.headers` is how a caller sends the metadata that used to ride as sibling form fields. diff --git a/frontend/src/lib/planCodes.contract.test.ts b/frontend/src/lib/planCodes.contract.test.ts index 228a8be3..4e3cf420 100644 --- a/frontend/src/lib/planCodes.contract.test.ts +++ b/frontend/src/lib/planCodes.contract.test.ts @@ -19,17 +19,17 @@ const ALLOWED = new Set([ "frontend/src/lib/planCodes.contract.test.ts", "scripts/audit_metrics.py", ]); -// Named exceptions, cut from a line before it is matched: a test show's episode titled "E1" (the -// whole string, quotes included) and ruff's default rule families. Anything else that looks like a -// code is one — a docstring or a test title opening with `"""E30:` / `it("E36:` included. -const NOT_CODES = /"E\d{1,2}"|\bE4\/E7\/E9\b/g; +// Named exceptions, cut from a line before it is matched: TV episode notation (`S2 · E5`), a test +// fixture's season or episode title (`title="S1"`, `title: "E1"`) and ruff's default rule families. +// Anything else that looks like a code is one — `"""E30:`, `it("E36:`, `# S6:` and `S3's` included. +const NOT_CODES = /\bS\d{1,2} ?·? ?E\d+|\btitle[=:] ?"[ES]\d{1,2}"|\bE4\/E7\/E9\b/g; const PLAN_CODES = [ - // an epic (`E28`, `E9.5`, `E28/E30`) or an older roadmap item (`R14`) — not `E2E`, not a TV - // episode (`S2 · E5`) - /(? { @@ -53,7 +53,7 @@ function tracked(): string[] { } describe("plan codes", () => { - it("are recognised in every form a citation takes", () => { + it("are recognised in each form the purged citations took", () => { const cited = [ '"""E30: the new sink.', 'it("E36: keeps x", () => {', @@ -61,14 +61,24 @@ describe("plan codes", () => { "# the roadmap's R6/R7 pair", "// held since E9.5", "// (S6) moved it", + "# S6: attach the acting user's cookie", + '"""S6 — per-user cookies', + "// S3's canonical upsert", + "// S1 justified that", + "// the order since P1", + '"""P1\'s invariant', + "// a sprint then a roadmap item: S2 R14", "// deep-review round 4 found it", ]; const clean = [ 'title="E1",', + 'title: "S1",', "# ruff's default rule set (E4/E7/E9 + F)", "// an E2E run", "// S2 · E5 is the second episode", + 'expect(btn.textContent).toContain("S2 · E1");', "// the S3h media card", + '"S110", "S112",', ]; expect(cited.filter((l) => !isPlanCode(l))).toEqual([]); expect(clean.filter(isPlanCode)).toEqual([]); diff --git a/scripts/audit_metrics.py b/scripts/audit_metrics.py index c9c02aa1..5e86fe3b 100644 --- a/scripts/audit_metrics.py +++ b/scripts/audit_metrics.py @@ -27,12 +27,14 @@ BIG_FILE = 800 BE_COMPLEXITY = 10 # ruff's C901 default FE_COMPLEXITY = 20 # the frontend's line -# what planCodes.contract.test.ts holds the code to: an epic (`E28`, `E9.5`) or a roadmap item (`R14`), -# not `E2E`, not a TV episode (`S2 · E5`); NOT_CODES are its named exceptions, cut first (an episode -# titled "E1", ruff's default `E4/E7/E9`) -EPIC_CODE = re.compile(r"\b(E\d{1,2}(\.\d+)?|R\d{1,2})\b(?!E)") -EPISODE = re.compile(r"S\d{1,2}\s?·?\s?E\d+") -NOT_CODES = re.compile(r'"E\d{1,2}"|\bE4/E7/E9\b') +# planCodes.contract.test.ts's definition, copied: an epic, roadmap item, sprint or phase code, or a +# review round; NOT_CODES are its named exceptions, cut first (TV episode notation, a fixture's +# season or episode title, ruff's default `E4/E7/E9`). ASCII, because JavaScript's `\b` is. +NOT_CODES = re.compile(r'\bS\d{1,2} ?·? ?E\d+|\btitle[=:] ?"[ES]\d{1,2}"|\bE4/E7/E9\b', re.ASCII) +PLAN_CODE = re.compile( + r"\b(?:E\d{1,2}(?:\.\d+)?|R\d{1,2}|S(?!3h\b)\d{1,2}(?:[a-z]\d?)?|P\d)\b|\b[Rr]ound[ -]\d|deep-review", + re.ASCII, +) NARRATION = re.compile(r"\b(used to|no longer)\b", re.IGNORECASE) @@ -116,17 +118,12 @@ def _complexity() -> dict[str, object]: def _noise(be: list[tuple[Path, list[str]]], fe: list[tuple[Path, list[str]]]) -> dict[str, object]: every = be + fe - epic = [ - (p, ln) - for p, ls in every - for ln in ls - if EPIC_CODE.search(NOT_CODES.sub("", EPISODE.sub("", ln))) - ] + plan = [(p, ln) for p, ls in every for ln in ls if PLAN_CODE.search(NOT_CODES.sub("", ln))] be_c = sum(_comment_lines(ls, True) for _, ls in be) fe_c = sum(_comment_lines(ls, False) for _, ls in fe) return { - "epic_code_lines": len(epic), - "epic_code_files": len({p for p, _ in epic}), + "plan_code_lines": len(plan), + "plan_code_files": len({p for p, _ in plan}), "history_narration_lines": sum(1 for _, ls in every for ln in ls if NARRATION.search(ln)), "backend_comment_share": round(be_c / sum(len(ls) for _, ls in be), 3), "frontend_comment_share": round(fe_c / sum(len(ls) for _, ls in fe), 3),