fix(gate): the plan-code guard catches bare sprint and phase codes, and the purge takes the P ones
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.
This commit is contained in:
@@ -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."""
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -391,7 +391,7 @@ export async function req<T = unknown>(
|
||||
// `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.
|
||||
|
||||
@@ -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`)
|
||||
/(?<!S\d{1,2}\s?·?\s?)\b(?:E\d{1,2}(?:\.\d+)?|R\d{1,2})\b(?!E)/,
|
||||
// a sprint cited on its own (`(S6)`, `S3e2`) or a review round (`round 4`, `deep-review`) — not
|
||||
// `S3h`, the media-card decision docs/DESIGN.md is headed by and the design lab labels
|
||||
/\(S\d{1,2}[a-z]?\d?\)|\bS(?!3h\b)\d{1,2}[a-z]\d?\b|\b[Rr]ound[ -]\d|deep-review/,
|
||||
// an epic (`E28`, `E9.5`, `E28/E30`), an older roadmap item (`R14`), a sprint (`S6`, `S3e2`) or a
|
||||
// phase (`P1`) — not `E2E`, not a ruff rule (`S110`), not `S3h`, the media-card decision
|
||||
// docs/DESIGN.md is headed by
|
||||
/\b(?:E\d{1,2}(?:\.\d+)?|R\d{1,2}|S(?!3h\b)\d{1,2}(?:[a-z]\d?)?|P\d)\b/,
|
||||
// a review round (`round 4`, `deep-review`)
|
||||
/\b[Rr]ound[ -]\d|deep-review/,
|
||||
];
|
||||
|
||||
const isPlanCode = (line: string) => {
|
||||
@@ -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([]);
|
||||
|
||||
+11
-14
@@ -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),
|
||||
|
||||
Reference in New Issue
Block a user