From 09d351a0e73d20d9c3e231b64425c720774bc959 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 20 Sep 2026 22:05:56 +0200 Subject: [PATCH 01/21] docs(ci): the two gate jobs queue, they do not run in parallel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Declared side by side, so they read as parallel; the lab runs a single act_runner with one slot, so the backend job starts at the exact second the frontend one ends. Measured across runs #86-#91: ~4 + ~6 = ~10 minutes from the push, not ~6. Written down because believing the comment cost something real — the ops release read-back budgeted the max instead of the sum and timed out on its first live run, reporting a still-running gate as a failing one. --- .gitea/workflows/check.yml | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/.gitea/workflows/check.yml b/.gitea/workflows/check.yml index 21684623..54a66221 100644 --- a/.gitea/workflows/check.yml +++ b/.gitea/workflows/check.yml @@ -1,8 +1,14 @@ # Server-side mirror of the local `siftlode check` gate (E1 S3 / DX-1), so `main` is always green -# independent of any one machine. Two parallel jobs match the two local lanes: +# independent of any one machine. Two jobs match the two local lanes: # frontend — tsc (3 projects) + eslint + prettier --check + knip + vitest # backend — ruff + the full pytest suite (pure-logic + the DB-backed lane) against a real Postgres # +# They are declared side by side but they DO NOT run in parallel: the lab has a single act_runner +# with one slot, so they queue. Measured across runs #86-#91 — the backend job starts at the exact +# second the frontend one ends, every time, and the pair costs ~4 + ~6 = **~10 minutes from the +# push**. Anything that waits on this gate has to budget for the sum, not the max; the ops +# `release` read-back timed out on its first real run for believing otherwise. +# # Runner: the lab act_runner (LXC 115) mounts the Docker socket, so service containers work. The backend # runs pytest NATIVELY against a `postgres` service (not the local docker-compose.test.yml, whose # bind-mount is unreliable under a mounted-socket runner); fidelity is kept by installing the SAME pinned From 51daf3e3889c81bbf56ef182b9380e9df2cb616a Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 20 Sep 2026 22:51:32 +0200 Subject: [PATCH 02/21] feat(security): cap every request body before auth runs (SEC-20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FastAPI reads a declared body IN FULL before the first dependency, so `POST /auth/password-login` drained an arbitrary upload from an anonymous caller and only then answered 401. 70 of the 72 body-declaring routes behaved the same way: one unauthenticated client could spend the process's memory and time at will. A raw ASGI middleware, not `@app.middleware("http")` — BaseHTTPMiddleware cannot wrap `receive`, so it could only trust `Content-Length`, which a client may omit (chunked) or understate. This refuses an oversized declaration outright and counts the bytes of everything else as they arrive, the same rule `capped_body` already applies per-route. Registered directly inside the access log: a refusal is still logged with its request id, but the session, CORS, the setup gate and every auth dependency never run for a body that is already too large. Two routes opt out, under one stated rule: an exemption is ONLY for a route whose own cap is larger than the global one. That is why `/api/diagnostics/events` is absent — it caps itself at 512 KiB, below the ceiling, so exempting it could only widen what it accepts. The list names route templates and a test resolves each against the live route table, so a rename fails the gate instead of silently dropping an exemption in prod. The 413 is byte-identical to `capped_body`'s and the test compares the two against each other rather than against a literal — two shapes for one fact is how the `cancelled`/`canceled` bug happened. --- CLAUDE.md | 3 +- backend/app/bodylimit.py | 183 ++++++++++++++++++++++++ backend/app/main.py | 9 ++ backend/tests/test_body_limit.py | 235 +++++++++++++++++++++++++++++++ 4 files changed, 429 insertions(+), 1 deletion(-) create mode 100644 backend/app/bodylimit.py create mode 100644 backend/tests/test_body_limit.py diff --git a/CLAUDE.md b/CLAUDE.md index 84c8184f..e7a6f481 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ how it gets built. Concretely: `routes/*` (HTTP only: validation, auth-dependency, serialization) → domain packages `sync/`, `plex/`, `downloads/`, `youtube/`, `issues/` (shared by routes + scheduler + worker) → flat core (`auth`, `quota`, -`sysconfig`, `state`, `audit`, `notifications`, `userscope`, `realtime`, `ratelimit`, `progress`, +`sysconfig`, `state`, `audit`, `notifications`, `userscope`, `realtime`, `ratelimit`, `bodylimit`, `progress`, `security`, `logs`, `logsink`, `diagnostics`, `debugsession`) → `models/db/config`. **Dependency direction is one-way: core never imports routes.** Frontend: `main.tsx` provider tree → `App.tsx` shell → `moduleRegistry` route-chunks → module components → the **platform layer `components/ui/`** (domain-free primitives: the popover engine and @@ -78,6 +78,7 @@ CQRS, a repository/DI framework, a router library, a generic CRUD-router factory | ----------------------- | ------------------------------------------------------------ | ------------------------------------------------------ | | ORM | SQLAlchemy 2.0 `select()` | `db.query(...)` | | Request body | a Pydantic model | `payload: dict` | +| Request SIZE | nothing — `bodylimit` caps everything (SEC-20) | `capped_body`, except for a LARGER per-route cap | | Ownership check | `owned_or_404` / `get_or_404` | ad-hoc `_own_playlist`/`_owned`/`_own_job` | | Admin gate import | `app.auth` | the `app.routes.admin` re-export | | Upstream failure status | `422` with JSON detail | `502` (reserve 502 for raw byte-proxies) | diff --git a/backend/app/bodylimit.py b/backend/app/bodylimit.py new file mode 100644 index 00000000..68ffd36c --- /dev/null +++ b/backend/app/bodylimit.py @@ -0,0 +1,183 @@ +"""The global request-body cap (SEC-20). + +**The hole.** FastAPI reads a declared body IN FULL before auth or any other dependency runs — the +body parameter has to be parsed to be passed in. So `POST /auth/password-login` drains a 32 MiB +upload from an anonymous caller and only then answers 401. 70 of the 72 body-declaring routes behave +the same way: one unauthenticated client can spend the process's memory and time at will. + +**Why a middleware and not `capped_body` on every route.** `routes/_common.py:capped_body` is the +right tool where a route genuinely takes bulk bytes, and the two routes that use it use it +correctly. But putting it on all 70 others would (a) fight the canonical "Request body → a Pydantic +model" row, since capping means taking the body as raw bytes and parsing it by hand, and (b) leave +the NEXT route added uncovered, because nothing would remind its author. A default that has to be +opted into is not a default. This module inverts it: the cap applies to everything, and the few +routes that need more opt OUT, in one list, next to the code that reads it. + +**Why raw ASGI and not `@app.middleware("http")`.** Starlette's BaseHTTPMiddleware cannot wrap +`receive`, so it can only read `Content-Length` — which a client may omit entirely (chunked) or +simply understate. The bytes have to be counted as they arrive, which is the same rule `capped_body` +already applies per-route, and counting needs the receive channel. + +**Why the refusal is byte-identical to `capped_body`'s.** Two shapes for one fact is how the +`cancelled`/`canceled` bug happened. The 413 body here and the 413 body from `capped_body` are the +same JSON, and `tests/test_body_limit.py` compares them against each other rather than against a +literal, so they cannot drift apart. + +**No admin knob, deliberately.** The ceiling is a constant, not a `sysconfig` value: an attacker- +facing limit that an admin can raise from the UI is a limit that can be raised by whoever gets the +admin session, and nobody has asked to tune it. If a self-hoster ever needs a different ceiling it +belongs in `config.py` (env, set before the process starts), not in the database. +""" + +import json +import re + +from starlette.types import ASGIApp, Message, Receive, Scope, Send + +# 1 MiB. The largest body any non-exempt route legitimately takes is 64 KiB (the debug-session form, +# `routes/debug_sessions.py:FORM_MAX_BYTES`), so this leaves ~16x headroom: a ceiling that refuses a +# legitimate request would be a worse bug than the one this module fixes. +DEFAULT_MAX_BODY_BYTES = 1024 * 1024 + +# The routes that opt out, as (method, path TEMPLATE) — the template, not a concrete path, so the +# list reads like the route decorators it mirrors and `test_body_limit.py` can assert that each one +# still resolves to a real route. A renamed route therefore fails the gate instead of silently +# losing its exemption in prod. +# +# A route belongs here ONLY IF ITS OWN CAP IS LARGER THAN THE GLOBAL ONE. That is the whole +# admission rule, and it is why `POST /api/diagnostics/events` is absent: its `capped_body` limit is +# 512 KiB, already below this ceiling, so the global cap never fires on it and an exemption would +# only widen what it accepts. Both entries below enforce their own limit on the bytes they actually +# read, which is what makes opting them out safe rather than merely convenient: +# /api/downloads/upload streams in 1 MiB chunks, caps on the real byte count (E7 S5); +# ingesting a local video file, so it is legitimately unbounded here +# /api/debug-sessions/{sid}/screenshots capped_body at SCREENSHOT_MAX_BYTES (10 MiB), and the +# owner check runs as a dependency BEFORE the body is read +EXEMPT_ROUTES: frozenset[tuple[str, str]] = frozenset( + { + ("POST", "/api/downloads/upload"), + ("POST", "/api/debug-sessions/{sid}/screenshots"), + } +) + +# The refusal, in the exact shape `capped_body` raises — `HTTPException(413, {"code": "too_large"})` +# renders as this. Built once; the test asserts it against a real route's response. +TOO_LARGE_BODY = json.dumps({"detail": {"code": "too_large"}}).encode() + +_PATH_PARAM = re.compile(r"\{[^/{}]+\}") + + +def template_to_regex(template: str) -> re.Pattern[str]: + """`/api/debug-sessions/{sid}/screenshots` → a regex matching one concrete path. + + A path parameter matches `[^/]+` — one segment, never across a `/`, so `{sid}` cannot swallow a + deeper path and hand an exemption to a route that was never granted one. + """ + literals = [re.escape(part) for part in _PATH_PARAM.split(template)] + return re.compile("^" + "[^/]+".join(literals) + "$") + + +def _declared_length(scope: Scope) -> int | None: + """The `Content-Length` header as an int, or None when absent or unparseable (a chunked body has + none, and a malformed one is not a number we may trust — both fall through to counting).""" + for name, value in scope.get("headers", ()): + if name == b"content-length": + try: + return int(value) + except ValueError: + # silent-ok: a malformed Content-Length is the caller's doing, not a fault of ours, + # and it is about to be answered anyway — returning None drops the request into the + # counting path, which refuses on the real bytes. Logging it would hand an anonymous + # client a free line in our log for every request it cares to malform. + return None + return None + + +async def _send_too_large(send: Send) -> None: + await send( + { + "type": "http.response.start", + "status": 413, + "headers": [ + (b"content-type", b"application/json"), + (b"content-length", str(len(TOO_LARGE_BODY)).encode()), + ], + } + ) + await send({"type": "http.response.body", "body": TOO_LARGE_BODY}) + + +class BodyLimitMiddleware: + """Refuses any request body over `limit` bytes, before the application sees it. + + Registered second-outermost in `main.py` — inside the access log so a refusal is still logged + with its request id, outside everything else so the session, CORS, setup-gate and auth layers + never run for a request that is already too large. + """ + + def __init__( + self, + app: ASGIApp, + *, + limit: int = DEFAULT_MAX_BODY_BYTES, + exempt: frozenset[tuple[str, str]] = EXEMPT_ROUTES, + ) -> None: + self.app = app + self.limit = limit + self._exempt = tuple((method, template_to_regex(path)) for method, path in exempt) + + def is_exempt(self, method: str, path: str) -> bool: + return any(m == method and rx.match(path) for m, rx in self._exempt) + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http" or self.is_exempt( + scope.get("method", ""), scope.get("path", "") + ): + await self.app(scope, receive, send) + return + + declared = _declared_length(scope) + if declared is not None and declared > self.limit: + # Refused on the declaration alone: nothing downstream is called, and not one byte of + # the body is read. + await _send_too_large(send) + return + + seen = 0 + refused = False + started = False + + async def counting_receive() -> Message: + nonlocal seen, refused + message = await receive() + if message["type"] != "http.request": + return message + seen += len(message.get("body", b"")) + if seen > self.limit: + # The app is blocked on this receive, so it has not answered yet. Telling it the + # client went away is the only in-band way to stop it reading: `request.stream()` + # turns an `http.disconnect` into ClientDisconnect, which unwinds the handler. Our + # own 413 goes out below, once the app has finished unwinding. + refused = True + return {"type": "http.disconnect"} + return message + + async def guarded_send(message: Message) -> None: + nonlocal started + if refused and not started: + return # discard the app's answer to the disconnect; the 413 below is the response + started = True + await send(message) + + try: + await self.app(scope, counting_receive, guarded_send) + except Exception: # noqa: BLE001 — only swallowed when WE caused it, see below + # ClientDisconnect (or whatever a handler raises on a truncated read) is the expected + # consequence of the disconnect we just faked, and the caller must get the 413 rather + # than a 500. Anything raised when we did NOT refuse is a real failure and is re-raised + # untouched. + if not refused: + raise + + if refused and not started: + await _send_too_large(send) diff --git a/backend/app/main.py b/backend/app/main.py index c3856349..457b2742 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -23,6 +23,7 @@ from fastapi.staticfiles import StaticFiles from sqlalchemy.exc import DataError from starlette.middleware.sessions import SessionMiddleware +from app import bodylimit from app import logsink from app import state from app import sysconfig @@ -257,6 +258,14 @@ async def security_headers(request, call_next): return response +# SEC-20 — the global request-body cap (app/bodylimit.py). Registered SECOND-TO-LAST, i.e. directly +# INSIDE the access log: a refusal is still logged with its request id and duration, but everything +# below this line — the session, CORS, the setup gate, and every auth dependency under the router — +# never runs for a body that is already too large. FastAPI reads a declared body in full before the +# first dependency, so without this the anonymous login route is a free memory/time sink. +app.add_middleware(bodylimit.BodyLimitMiddleware) + + # The access log (app/logs.py): request id, route template, status, duration — one line, never a # query string. Registered LAST so it is the OUTERMOST http middleware and therefore times the whole # request, including what the three above do. Being outermost also puts it OUTSIDE SessionMiddleware, diff --git a/backend/tests/test_body_limit.py b/backend/tests/test_body_limit.py new file mode 100644 index 00000000..d8a03ade --- /dev/null +++ b/backend/tests/test_body_limit.py @@ -0,0 +1,235 @@ +"""SEC-20 — the global request-body cap (`app/bodylimit.py`). + +Two halves, deliberately kept apart: + +* the MIDDLEWARE tests drive a toy ASGI app, so they pin the mechanism (declared length, counted + bytes, the exemption match) with no router, no auth and no database in the way; +* the APP tests drive the real application, so they pin that the middleware is actually wired in, + sits ahead of the auth dependencies, and refuses in the same shape `capped_body` does. + +The pair that matters most is "an oversized body is refused" together with "a normal body still +gets through": the first alone would pass just as well if the cap refused *everything*. +""" + +import json + +import pytest +from fastapi.testclient import TestClient +from starlette.requests import Request + +from app import bodylimit +from app.bodylimit import DEFAULT_MAX_BODY_BYTES, BodyLimitMiddleware +from app.routes import diagnostics as diagnostics_routes + +SMALL = b"x" * 16 +OVER = b"x" * (DEFAULT_MAX_BODY_BYTES + 1) + + +async def _reading_app(scope, receive, send): + """A toy app that reads the whole body the way a route does, then reports its length. + + It uses a real Starlette `Request`, so the faked `http.disconnect` reaches it as the real + ClientDisconnect a real handler would see — the middleware's exception path is exercised, not + stubbed around. + """ + body = await Request(scope, receive).body() + payload = json.dumps({"read": len(body)}).encode() + await send( + { + "type": "http.response.start", + "status": 200, + "headers": [(b"content-type", b"application/json")], + } + ) + await send({"type": "http.response.body", "body": payload}) + + +def _client(**kwargs) -> TestClient: + return TestClient(BodyLimitMiddleware(_reading_app, **kwargs)) + + +def _chunks(total: int, size: int = 64 * 1024): + """A body with NO Content-Length: httpx sends an iterator chunked, which is exactly the shape a + declared-length check cannot see.""" + sent = 0 + while sent < total: + n = min(size, total - sent) + yield b"x" * n + sent += n + + +# --- the mechanism ------------------------------------------------------------------------------ + + +def test_a_declared_length_over_the_cap_is_refused_without_reading_the_body(): + r = _client().post("/anything", content=OVER) + assert r.status_code == 413 + assert r.content == bodylimit.TOO_LARGE_BODY + + +def test_a_body_under_the_cap_is_passed_through_untouched(): + # The mutation guard for the test above: if the cap refused everything, this would fail. + r = _client().post("/anything", content=SMALL) + assert r.status_code == 200 + assert r.json() == {"read": len(SMALL)} + + +def test_a_chunked_body_is_refused_on_the_COUNTED_bytes(): + # No Content-Length at all, so the only thing that can stop this is counting as it arrives. + r = _client().post("/anything", content=_chunks(DEFAULT_MAX_BODY_BYTES + 64 * 1024)) + assert r.status_code == 413 + assert r.content == bodylimit.TOO_LARGE_BODY + + +def test_a_chunked_body_under_the_cap_still_arrives_whole(): + size = DEFAULT_MAX_BODY_BYTES - 1 + r = _client().post("/anything", content=_chunks(size)) + assert r.status_code == 200 + assert r.json() == {"read": size} + + +def test_an_understated_content_length_does_not_buy_a_pass(): + # The declared-length check is the cheap half; a client that lies must still be counted out. + r = _client().post("/anything", content=OVER, headers={"content-length": "10"}) + assert r.status_code == 413 + + +def test_exactly_at_the_cap_is_allowed(): + size = DEFAULT_MAX_BODY_BYTES + r = _client(limit=size).post("/anything", content=b"x" * size) + assert r.status_code == 200 + assert r.json() == {"read": size} + + +# --- the exemption list ------------------------------------------------------------------------- + + +def test_an_exempt_route_may_send_a_body_over_the_global_cap(): + client = _client(exempt=frozenset({("POST", "/api/things/{tid}/blob")})) + r = client.post("/api/things/abc/blob", content=OVER) + assert r.status_code == 200 + assert r.json() == {"read": len(OVER)} + + +def test_the_exemption_is_scoped_to_its_method_and_its_path(): + client = _client(exempt=frozenset({("POST", "/api/things/{tid}/blob")})) + # Same path, different method. + assert client.put("/api/things/abc/blob", content=OVER).status_code == 413 + # A neighbour that merely starts the same way. + assert client.post("/api/things/abc/blob/extra", content=OVER).status_code == 413 + + +def test_a_path_parameter_cannot_swallow_a_slash(): + # `{tid}` matching greedily across `/` would hand the exemption to routes that never got one. + mw = BodyLimitMiddleware(_reading_app, exempt=frozenset({("POST", "/a/{tid}/b")})) + assert mw.is_exempt("POST", "/a/one/b") + assert not mw.is_exempt("POST", "/a/one/two/b") + + +def _declared_routes(root) -> set[tuple[str, str]]: + """Every (method, path template) the app serves. + + It has to RECURSE: since FastAPI 0.141 / Starlette 1.6 an `include_router` leaves an opaque + `_IncludedRouter` in `app.routes` and keeps the real routes under its `original_router`, instead + of flattening them into the parent as older versions did. Reading `app.routes` alone finds 35 + entries and none of ours; walking the tree finds all 252. + """ + found: set[tuple[str, str]] = set() + stack = [root] + while stack: + node = stack.pop() + for route in getattr(node, "routes", ()) or (): + inner = getattr(route, "original_router", None) + if inner is not None: + stack.append(inner) + continue + for method in getattr(route, "methods", None) or (): + found.add((method, route.path)) + return found + + +@pytest.mark.parametrize(("method", "template"), sorted(bodylimit.EXEMPT_ROUTES)) +def test_every_exempt_template_still_resolves_to_a_real_route(method, template): + """The drift guard. The exemption list names route templates that live in another file; a + renamed route must fail the gate here, not lose its exemption silently in production.""" + from app.main import app + + assert (method, template) in _declared_routes(app) + + +def test_a_route_that_caps_itself_below_the_global_ceiling_is_NOT_exempt(): + """The admission rule, enforced rather than described: an exemption is for routes whose own cap + is LARGER than the global one. The client log sink caps itself at 512 KiB, so the global ceiling + never fires on it and exempting it would only widen what it accepts.""" + assert diagnostics_routes.BODY_MAX_BYTES <= DEFAULT_MAX_BODY_BYTES + assert not any(path.startswith("/api/diagnostics") for _, path in bodylimit.EXEMPT_ROUTES) + + +def test_the_screenshot_exemption_is_justified_by_its_own_larger_cap(): + from app import debugsession + + assert debugsession.SCREENSHOT_MAX_BYTES > DEFAULT_MAX_BODY_BYTES + assert ("POST", "/api/debug-sessions/{sid}/screenshots") in bodylimit.EXEMPT_ROUTES + + +# --- the real application ----------------------------------------------------------------------- + + +def test_the_login_route_is_refused_before_a_single_dependency_runs(client, monkeypatch): + """`/auth/password-login` is the reason SEC-20 exists: it is anonymous, it declares a body, and + FastAPI reads that body in full before the first dependency. `_login_limiter.allow` is the very + first statement of the handler, so it is the sharpest available proof that nothing ran.""" + from app.routes import auth as auth_routes + + reached = False + + def _record(*args, **kwargs): + nonlocal reached + reached = True + return True + + monkeypatch.setattr(auth_routes._login_limiter, "allow", _record) + + r = client.post("/auth/password-login", content=OVER, headers={"content-type": "text/plain"}) + assert r.status_code == 413 + assert not reached, "the handler ran — the cap is inside the router instead of ahead of it" + + +def test_a_normal_login_body_still_reaches_the_handler(client, monkeypatch): + """The mutation check for the test above. If the cap refused every body, the 413 test would + pass while the app was broken; this one would not.""" + from app.routes import auth as auth_routes + + reached = False + + def _record(*args, **kwargs): + nonlocal reached + reached = True + return True + + monkeypatch.setattr(auth_routes._login_limiter, "allow", _record) + + r = client.post("/auth/password-login", json={"email": "nobody@example.com", "password": "x"}) + assert r.status_code == 401 + assert reached + + +def test_the_global_refusal_is_byte_identical_to_the_per_route_one(db, as_user, make_user): + """Two shapes for one fact is how the `cancelled`/`canceled` bug happened. The log sink caps + itself with `capped_body`; its 413 and the middleware's must be the same bytes. + + Signed in on purpose: `current_user` resolves before the body dependency, so an anonymous call + would be answered 401 and never reach the per-route cap this compares against. The body is over + the SINK's 512 KiB but under the global 1 MiB, so it is `capped_body` that refuses it. + """ + user = make_user() + db.commit() + over_the_sink = b"x" * (diagnostics_routes.BODY_MAX_BYTES + 1) + assert len(over_the_sink) < DEFAULT_MAX_BODY_BYTES + r = as_user(user).post( + "/api/diagnostics/events", + content=over_the_sink, + headers={"content-type": "application/json"}, + ) + assert r.status_code == 413 + assert r.json() == json.loads(bodylimit.TOO_LARGE_BODY) From c9101e3450880cd4b6dcff96a204a4348e3aa60b Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 20 Sep 2026 23:02:02 +0200 Subject: [PATCH 03/21] =?UTF-8?q?ci:=20type-check=20the=20backend=20?= =?UTF-8?q?=E2=80=94=20the=20gate's=20ninth=20lane=20(E27=20S3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three tsc projects held the frontend and NOTHING held the backend: ~36k lines of `app/`, annotated in 113 of 128 files, that no lane had ever read. Asymmetric by accident, not by decision. basedpyright, in basic mode. Measured against mypy on this tree with the dependencies actually installed — without them basedpyright errors on every import and mypy's --ignore-missing-imports quietly turns them into Any and UNDER-reports. Both land ~190-200 errors with the same distribution, so the choice fell to what differs: basedpyright is ~2x faster (4.4s vs 7.4s, and this runs on every `siftlode lint`), resolves our `app.*` imports from its own config with no MYPYPATH, and has a first-class `--writebaseline`. That last one is the ratchet itself; mypy would have needed a third-party tool for it. 198 pre-existing errors are baselined, so the lane fails on NEW errors only and the list burns down epic by epic. A green lane means no new damage, not a clean codebase. It runs in the pytest test image rather than on the host, and that is forced rather than chosen: `requirements.lock` cannot be installed on Windows at all — uvloop refuses to build there — so a host venv could only ever be the lock minus a package, and a type checker without the real third-party packages reports nonsense rather than nothing. The image installs the pin from requirements-dev.txt, so the lane's version IS the pin and there is no host copy to drift. The cost is that `siftlode lint` now needs Docker, which it did not before. --- .gitea/workflows/check.yml | 7 + CLAUDE.md | 3 +- backend/.basedpyright/baseline.json | 1682 +++++++++++++++++++++++++++ backend/pyrightconfig.json | 24 + backend/requirements-dev.txt | 4 + 5 files changed, 1719 insertions(+), 1 deletion(-) create mode 100644 backend/.basedpyright/baseline.json create mode 100644 backend/pyrightconfig.json diff --git a/.gitea/workflows/check.yml b/.gitea/workflows/check.yml index 54a66221..e42ea2b5 100644 --- a/.gitea/workflows/check.yml +++ b/.gitea/workflows/check.yml @@ -93,6 +93,13 @@ jobs: run: python -m ruff check --config backend/ruff.toml backend scripts - name: ruff format --check run: python -m ruff format --check --config backend/ruff.toml backend scripts + # The ninth lane (E27 S3). Run from backend/ so it resolves exactly as the local lane does — + # pyrightconfig.json's include/extraPaths are relative to the config, but the baseline is + # found relative to the working directory. New errors fail; the ~200 pre-existing ones are in + # .basedpyright/baseline.json and burn down epic by epic. + - name: basedpyright + run: python -m basedpyright --project pyrightconfig.json + working-directory: backend - name: pytest run: python -m pytest working-directory: backend diff --git a/CLAUDE.md b/CLAUDE.md index e7a6f481..b256535c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,12 +12,13 @@ provider. ## The gate — never merge or ship red -The local gate is **ruff (`check` + `format --check`) + typecheck (3 tsc projects) + eslint + prettier +The local gate is **ruff (`check` + `format --check`) + basedpyright (backend) + typecheck (3 tsc projects) + eslint + prettier --check + knip + vitest + pytest**. It runs on every `publish`, which **refuses to build on failure — there is deliberately no `--skip` flag.** Run it and get it green **before** any merge, release, or handing work to review. - Full gate (agents with the private ops dispatcher): `siftlode check`. - Directly, without the dispatcher: `cd frontend && npm run typecheck && npm run lint && npm run knip && npm run test`, plus backend `ruff check backend` + `ruff format --check backend` and `pytest` (the DB-lane needs Postgres). +- **The backend type lane runs in the test image, not on the host** — `requirements.lock` cannot be installed on Windows (uvloop), and a type checker without the real packages reports nonsense. It fails on NEW errors only; the pre-existing ones sit in `backend/.basedpyright/baseline.json` and burn down epic by epic. Fixed some? Regenerate with `--writebaseline`. - New DB-backed test? It must actually run, not silently skip — assume `SIFTLODE_TEST_REQUIRE_DB=1`. ## Workflow diff --git a/backend/.basedpyright/baseline.json b/backend/.basedpyright/baseline.json new file mode 100644 index 00000000..50e22559 --- /dev/null +++ b/backend/.basedpyright/baseline.json @@ -0,0 +1,1682 @@ +{ + "files": { + "./app/audit.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 18, + "endColumn": 26, + "lineCount": 1 + } + } + ], + "./app/debugsession.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 18, + "endColumn": 26, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 18, + "endColumn": 26, + "lineCount": 1 + } + } + ], + "./app/diagnostics.py": [ + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 40, + "endColumn": 49, + "lineCount": 1 + } + } + ], + "./app/discover/pins.py": [ + { + "code": "reportOptionalOperand", + "range": { + "startColumn": 8, + "endColumn": 9, + "lineCount": 5 + } + } + ], + "./app/discover/queries.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 50, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 33, + "endColumn": 44, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 33, + "endColumn": 44, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 33, + "endColumn": 44, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 28, + "endColumn": 39, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 28, + "endColumn": 39, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 28, + "endColumn": 39, + "lineCount": 1 + } + } + ], + "./app/discover/rails.py": [ + { + "code": "reportCallIssue", + "range": { + "startColumn": 4, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 18, + "endColumn": 41, + "lineCount": 1 + } + } + ], + "./app/downloads/adapters.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 30, + "endColumn": 38, + "lineCount": 1 + } + } + ], + "./app/downloads/backfill.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 34, + "endColumn": 38, + "lineCount": 1 + } + }, + { + "code": "reportReturnType", + "range": { + "startColumn": 23, + "endColumn": 27, + "lineCount": 1 + } + } + ], + "./app/downloads/credentials.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 27, + "endColumn": 30, + "lineCount": 1 + } + } + ], + "./app/downloads/gc.py": [ + { + "code": "reportOperatorIssue", + "range": { + "startColumn": 16, + "endColumn": 28, + "lineCount": 1 + } + } + ], + "./app/downloads/pipeline.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 34, + "endColumn": 38, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 22, + "endColumn": 27, + "lineCount": 1 + } + }, + { + "code": "reportGeneralTypeIssues", + "range": { + "startColumn": 10, + "endColumn": 18, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 12, + "endColumn": 13, + "lineCount": 10 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 31, + "endColumn": 36, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 36, + "endColumn": 43, + "lineCount": 1 + } + } + ], + "./app/downloads/service.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 18, + "endColumn": 26, + "lineCount": 1 + } + } + ], + "./app/downloads/urlguard.py": [ + { + "code": "reportReturnType", + "range": { + "startColumn": 11, + "endColumn": 41, + "lineCount": 1 + } + } + ], + "./app/feedquery.py": [ + { + "code": "reportCallIssue", + "range": { + "startColumn": 37, + "endColumn": 5, + "lineCount": 3 + } + }, + { + "code": "reportAssignmentType", + "range": { + "startColumn": 37, + "endColumn": 5, + "lineCount": 3 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 97, + "lineCount": 1 + } + } + ], + "./app/logsink.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 18, + "endColumn": 26, + "lineCount": 1 + } + } + ], + "./app/notifications.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 52, + "endColumn": 58, + "lineCount": 1 + } + } + ], + "./app/plex/lookups.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 48, + "endColumn": 58, + "lineCount": 1 + } + } + ], + "./app/plex/people.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 15, + "endColumn": 23, + "lineCount": 1 + } + } + ], + "./app/plex/stream.py": [ + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 51, + "endColumn": 55, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 23, + "endColumn": 25, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 18, + "endColumn": 31, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 75, + "endColumn": 79, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 10, + "endColumn": 23, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 10, + "endColumn": 12, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 10, + "endColumn": 17, + "lineCount": 1 + } + } + ], + "./app/plex/sync.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 21, + "endColumn": 69, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 21, + "endColumn": 69, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 70, + "endColumn": 83, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 76, + "endColumn": 89, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 27, + "endColumn": 33, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 17, + "endColumn": 18, + "lineCount": 1 + } + } + ], + "./app/plex/tmdb.py": [ + { + "code": "reportAssignmentType", + "range": { + "startColumn": 16, + "endColumn": 9, + "lineCount": 10 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 30, + "endColumn": 36, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 29, + "endColumn": 35, + "lineCount": 1 + } + } + ], + "./app/plex/watch_sync.py": [ + { + "code": "reportReturnType", + "range": { + "startColumn": 11, + "endColumn": 5, + "lineCount": 11 + } + } + ], + "./app/ratelimit.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 14, + "endColumn": 25, + "lineCount": 1 + } + } + ], + "./app/routes/admin.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 9, + "endColumn": 13, + "lineCount": 1 + } + } + ], + "./app/routes/admin_logs.py": [ + { + "code": "reportReturnType", + "range": { + "startColumn": 15, + "endColumn": 47, + "lineCount": 1 + } + } + ], + "./app/routes/audit.py": [ + { + "code": "reportCallIssue", + "range": { + "startColumn": 17, + "endColumn": 98, + "lineCount": 1 + } + }, + { + "code": "reportAssignmentType", + "range": { + "startColumn": 17, + "endColumn": 98, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 22, + "endColumn": 97, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 37, + "endColumn": 45, + "lineCount": 1 + } + } + ], + "./app/routes/channels.py": [ + { + "code": "reportReturnType", + "range": { + "startColumn": 11, + "endColumn": 11, + "lineCount": 6 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 15, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 49, + "endColumn": 56, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 38, + "endColumn": 44, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 33, + "endColumn": 39, + "lineCount": 1 + } + } + ], + "./app/routes/discover.py": [ + { + "code": "reportInvalidTypeForm", + "range": { + "startColumn": 19, + "endColumn": 33, + "lineCount": 1 + } + } + ], + "./app/routes/downloads.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 29, + "endColumn": 33, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 37, + "endColumn": 47, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 29, + "endColumn": 33, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 40, + "endColumn": 50, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 17, + "endColumn": 19, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 82, + "endColumn": 84, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 87, + "endColumn": 89, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 30, + "endColumn": 32, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 49, + "endColumn": 54, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 29, + "endColumn": 33, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 40, + "endColumn": 50, + "lineCount": 1 + } + } + ], + "./app/routes/messages.py": [ + { + "code": "reportCallIssue", + "range": { + "startColumn": 11, + "endColumn": 43, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 23, + "endColumn": 27, + "lineCount": 1 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 13, + "endColumn": 5, + "lineCount": 7 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 15, + "lineCount": 5 + } + }, + { + "code": "reportReturnType", + "range": { + "startColumn": 15, + "endColumn": 72, + "lineCount": 1 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 30, + "endColumn": 48, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 41, + "endColumn": 44, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 36, + "endColumn": 38, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 71, + "endColumn": 73, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 30, + "endColumn": 39, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 27, + "endColumn": 29, + "lineCount": 1 + } + } + ], + "./app/routes/playlists.py": [ + { + "code": "reportOptionalOperand", + "range": { + "startColumn": 8, + "endColumn": 9, + "lineCount": 5 + } + }, + { + "code": "reportOptionalOperand", + "range": { + "startColumn": 8, + "endColumn": 9, + "lineCount": 5 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 13, + "endColumn": 5, + "lineCount": 7 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 15, + "lineCount": 5 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 16, + "endColumn": 5, + "lineCount": 11 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 15, + "lineCount": 9 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 13, + "endColumn": 5, + "lineCount": 5 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 15, + "lineCount": 3 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 12, + "endColumn": 31, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 23, + "endColumn": 27, + "lineCount": 1 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 12, + "endColumn": 34, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 26, + "endColumn": 30, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 12, + "endColumn": 28, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 23, + "endColumn": 27, + "lineCount": 1 + } + } + ], + "./app/routes/plex/browse.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 26, + "endColumn": 61, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 26, + "endColumn": 61, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 11, + "endColumn": 13, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 25, + "endColumn": 27, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 11, + "endColumn": 13, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 25, + "endColumn": 27, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 15, + "endColumn": 17, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 28, + "endColumn": 30, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 15, + "endColumn": 17, + "lineCount": 1 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 28, + "endColumn": 30, + "lineCount": 1 + } + } + ], + "./app/routes/plex/collections.py": [ + { + "code": "reportAssignmentType", + "range": { + "startColumn": 11, + "endColumn": 39, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 76, + "endColumn": 80, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 75, + "endColumn": 79, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 80, + "endColumn": 84, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 20, + "endColumn": 45, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 20, + "endColumn": 46, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 56, + "endColumn": 59, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 56, + "endColumn": 59, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 39, + "endColumn": 47, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 56, + "endColumn": 59, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 46, + "endColumn": 49, + "lineCount": 1 + } + } + ], + "./app/routes/plex/discover.py": [ + { + "code": "reportCallIssue", + "range": { + "startColumn": 4, + "endColumn": 63, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 18, + "endColumn": 48, + "lineCount": 1 + } + }, + { + "code": "reportCallIssue", + "range": { + "startColumn": 4, + "endColumn": 63, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 18, + "endColumn": 48, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 39, + "endColumn": 45, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 38, + "endColumn": 43, + "lineCount": 1 + } + } + ], + "./app/routes/plex/playback.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 6, + "endColumn": 14, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 51, + "endColumn": 54, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 51, + "endColumn": 54, + "lineCount": 1 + } + } + ], + "./app/routes/plex/playlists.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 77, + "endColumn": 83, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 77, + "endColumn": 83, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 9, + "lineCount": 5 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 8, + "endColumn": 9, + "lineCount": 5 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 10, + "endColumn": 18, + "lineCount": 1 + } + } + ], + "./app/routes/public.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 57, + "endColumn": 71, + "lineCount": 1 + } + } + ], + "./app/routes/scheduler.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 42, + "endColumn": 55, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 25, + "endColumn": 28, + "lineCount": 1 + } + }, + { + "code": "reportOptionalMemberAccess", + "range": { + "startColumn": 25, + "endColumn": 28, + "lineCount": 1 + } + } + ], + "./app/routes/search.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 25, + "endColumn": 31, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 6, + "endColumn": 14, + "lineCount": 1 + } + } + ], + "./app/routes/sync.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 42, + "endColumn": 65, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 42, + "endColumn": 50, + "lineCount": 1 + } + } + ], + "./app/sync/explore.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 6, + "endColumn": 14, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 6, + "endColumn": 14, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 6, + "endColumn": 14, + "lineCount": 1 + } + }, + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 6, + "endColumn": 14, + "lineCount": 1 + } + } + ], + "./app/sync/maintenance.py": [ + { + "code": "reportOptionalOperand", + "range": { + "startColumn": 19, + "endColumn": 42, + "lineCount": 1 + } + } + ], + "./app/sync/playlists.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 55, + "endColumn": 72, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 51, + "endColumn": 68, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 39, + "endColumn": 56, + "lineCount": 1 + } + } + ], + "./app/sync/runner.py": [ + { + "code": "reportAssignmentType", + "range": { + "startColumn": 19, + "endColumn": 88, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 16, + "endColumn": 24, + "lineCount": 1 + } + }, + { + "code": "reportOptionalIterable", + "range": { + "startColumn": 23, + "endColumn": 31, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 45, + "endColumn": 72, + "lineCount": 1 + } + }, + { + "code": "reportAssignmentType", + "range": { + "startColumn": 19, + "endColumn": 9, + "lineCount": 7 + } + }, + { + "code": "reportOptionalSubscript", + "range": { + "startColumn": 19, + "endColumn": 27, + "lineCount": 1 + } + }, + { + "code": "reportOptionalIterable", + "range": { + "startColumn": 23, + "endColumn": 31, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 47, + "endColumn": 55, + "lineCount": 1 + } + } + ], + "./app/sync/videos.py": [ + { + "code": "reportAttributeAccessIssue", + "range": { + "startColumn": 18, + "endColumn": 26, + "lineCount": 1 + } + } + ], + "./app/worker.py": [ + { + "code": "reportArgumentType", + "range": { + "startColumn": 36, + "endColumn": 40, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 36, + "endColumn": 40, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 38, + "endColumn": 43, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 35, + "endColumn": 43, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 24, + "endColumn": 32, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 28, + "endColumn": 36, + "lineCount": 1 + } + }, + { + "code": "reportArgumentType", + "range": { + "startColumn": 46, + "endColumn": 52, + "lineCount": 1 + } + } + ], + "./app/youtube/rss.py": [ + { + "code": "reportGeneralTypeIssues", + "range": { + "startColumn": 10, + "endColumn": 15, + "lineCount": 1 + } + } + ] + } +} \ No newline at end of file diff --git a/backend/pyrightconfig.json b/backend/pyrightconfig.json new file mode 100644 index 00000000..89837125 --- /dev/null +++ b/backend/pyrightconfig.json @@ -0,0 +1,24 @@ +// The backend type-check lane (E27 S3) — the gate's ninth lane, and the first thing that has ever +// type-checked the ~36k lines of `app/`. Read as JSONC: pyright accepts comments here. +// +// basedpyright, not mypy: measured on this tree with the dependencies actually installed, both land +// ~190-200 errors with the same distribution, so the choice was made on the things that differ — +// basedpyright is ~2x faster, needs no MYPYPATH to resolve our `app.*` imports, and has a FIRST-CLASS +// baseline (`--writebaseline`), which is the whole ratchet. mypy would have needed a third-party tool +// for that. +// +// It runs in the pytest test image, never on the host: `requirements.lock` cannot be installed on +// Windows at all (uvloop refuses to build there), so a host run would type-check against absent +// third-party packages and report nonsense. The version is pinned in requirements-dev.txt, which the +// image installs — so the lane's version IS the pin, with no host-vs-pin check to drift. +// +// `pythonVersion` is not free-standing: `/.python-version` is the single source of truth and +// `tests/test_python_engine_contract.py` fails this gate if this file, the two Dockerfiles, the CI +// container or ruff's target-version disagree with it. +{ + "include": ["app"], + "extraPaths": ["."], + "typeCheckingMode": "basic", + "pythonVersion": "3.12", + "pythonPlatform": "Linux" +} diff --git a/backend/requirements-dev.txt b/backend/requirements-dev.txt index 6f25a4da..311a35bd 100644 --- a/backend/requirements-dev.txt +++ b/backend/requirements-dev.txt @@ -4,6 +4,10 @@ # backend lint lane runs on the host (the gate fails when the host's ruff is not this one). httpx2: # Starlette 1.x's TestClient is built on it (`import httpx2 as httpx`) and only falls back to the # app's own httpx with a deprecation warning on every run. +# basedpyright: the backend type-check lane (E27 S3). Pinned like everything else here — a type +# checker whose version floats silently changes what "clean" means, and the ratchet baseline it +# writes is keyed to the diagnostics THIS version produces. +basedpyright==1.40.1 pytest==8.4.2 ruff==0.15.21 httpx2==2.13.0 From cc9fc3b544811db5f13c05dc9559ea3ba4bae533 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 20 Sep 2026 23:02:11 +0200 Subject: [PATCH 04/21] build: pin one Python version across the app, the images and the gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 0.93.2 shipped a week's worth of confusion about which Node this repo runs on; the Python side had the identical shape and nobody had looked. Prod (Dockerfile), the test image (Dockerfile.test) and CI (check.yml) each spelled 3.12 as an independent literal, there was no `.python-version`, no `requires-python`, and ruff had no target-version so it judged our syntax against some other interpreter. The dev machine had no 3.12 installed at all — only 3.13 and 3.14 — so every ad-hoc host script ran two majors ahead of production. `.python-version` is now the single source of truth, and the contract is held by tests rather than by prose. It is split in two because no single runner can see every link: the pytest suite runs inside the test image, which mounts backend/ alone and cannot read the repo-root Dockerfile or the workflow. test_python_engine_contract.py : running interpreter == pyrightconfig == ruff.toml pythonEngine.contract.test.ts : .python-version == pyrightconfig == ruff.toml == Dockerfile == Dockerfile.test == check.yml Together they close the loop, and the backend half carries the assertion that pays — that the interpreter running the suite is the pinned one. A green suite on another minor does not mean the shipped image is green; it means they have drifted and nobody has noticed yet. --- .python-version | 1 + backend/ruff.toml | 4 + backend/tests/test_python_engine_contract.py | 66 ++++++++++++++ .../src/lib/pythonEngine.contract.test.ts | 86 +++++++++++++++++++ 4 files changed, 157 insertions(+) create mode 100644 .python-version create mode 100644 backend/tests/test_python_engine_contract.py create mode 100644 frontend/src/lib/pythonEngine.contract.test.ts diff --git a/.python-version b/.python-version new file mode 100644 index 00000000..e4fba218 --- /dev/null +++ b/.python-version @@ -0,0 +1 @@ +3.12 diff --git a/backend/ruff.toml b/backend/ruff.toml index 9bb4cdf0..f69ce16c 100644 --- a/backend/ruff.toml +++ b/backend/ruff.toml @@ -7,6 +7,10 @@ # the repo wrap at the same width, and it's the width `ruff format` enforces (gated via # `ruff format --check` alongside `ruff check`). line-length = 100 +# The interpreter this code actually runs on, so ruff judges syntax and upgrade hints against the +# right one. Left unset it falls back to a default that has nothing to do with us. `/.python-version` +# is the single source of truth; tests/test_python_engine_contract.py holds this to it. +target-version = "py312" [lint] # Beyond the default set, the rules that hold a LOG LINE to what E22.5 makes of it: diff --git a/backend/tests/test_python_engine_contract.py b/backend/tests/test_python_engine_contract.py new file mode 100644 index 00000000..49a940aa --- /dev/null +++ b/backend/tests/test_python_engine_contract.py @@ -0,0 +1,66 @@ +"""The Python major is a contract, not a coincidence — the backend half (E27 S3). + +`0.93.2` was released to fix exactly this bug on the JavaScript side: four machines built or checked +the app and none of them agreed on which Node they ran, the server gate went red for 73 of its last +90 runs, and nothing told anyone. The Python side had the same shape and nobody had looked: prod +(`Dockerfile`), the test image (`Dockerfile.test`) and CI (`check.yml`) each spelled **3.12** as an +independent literal, there was no `.python-version`, no `requires-python`, and ruff had no +`target-version` — while the dev machine had no 3.12 installed at all, only 3.13 and 3.14. + +**The contract is a chain, and it is split across two test files because no single runner can see +every link.** This suite runs inside the test image, which mounts `backend/` and nothing else, so it +cannot read the repo-root Dockerfile or the workflow. It therefore holds the link it CAN see, and the +one that matters most here: + + the interpreter actually running == backend/pyrightconfig.json == backend/ruff.toml + +and `frontend/src/lib/pythonEngine.contract.test.ts` — which runs on the host, with the whole tree — +holds the rest of the chain: + + /.python-version == pyrightconfig.json == ruff.toml == Dockerfile == Dockerfile.test == check.yml + +Together they close the loop. To move the version: edit `/.python-version`, then follow the failures +in both files. Every place they name is a place that would otherwise have been forgotten. +""" + +import re +import sys +from pathlib import Path + +BACKEND = Path(__file__).resolve().parents[1] + + +def _version_in(path: Path, pattern: str) -> tuple[int, int]: + """The (major, minor) the given file declares. A rename or a moved line lands here — re-point + the test, the contract did not go away with it.""" + text = path.read_text(encoding="utf-8") + match = re.search(pattern, text, re.MULTILINE) + assert match is not None, f"{pattern!r} not found in {path.name}" + return int(match.group(1)), int(match.group(2)) + + +# pyrightconfig.json is JSONC (it carries comments), so it is read by regex rather than json.loads. +PINNED = _version_in(BACKEND / "pyrightconfig.json", r'"pythonVersion"\s*:\s*"(\d+)\.(\d+)"') + + +def test_pins_a_plausible_python_version(): + """Guards against a typo agreeing with nothing at all.""" + assert PINNED >= (3, 12), PINNED + + +def test_this_interpreter_is_the_pinned_one(): + """The assertion that pays. A green suite on another minor does not mean the shipped image is + green — it means the suite and the image have drifted and nobody noticed yet.""" + running = sys.version_info[:2] + assert running == PINNED, ( + f"these tests are running on Python {running[0]}.{running[1]}, but the app is pinned to " + f"{PINNED[0]}.{PINNED[1]} (backend/pyrightconfig.json, /.python-version). The suite runs in " + f"the test image, so this failing means backend/Dockerfile.test has drifted from the pin." + ) + + +def test_ruff_judges_the_pinned_version(): + """Unset, ruff falls back to a default that has nothing to do with us, and its syntax and + upgrade advice are then aimed at some other interpreter.""" + major, minor = _version_in(BACKEND / "ruff.toml", r'^target-version\s*=\s*"py(\d)(\d+)"') + assert (major, minor) == PINNED diff --git a/frontend/src/lib/pythonEngine.contract.test.ts b/frontend/src/lib/pythonEngine.contract.test.ts new file mode 100644 index 00000000..a0eb6829 --- /dev/null +++ b/frontend/src/lib/pythonEngine.contract.test.ts @@ -0,0 +1,86 @@ +import { readFileSync } from "node:fs"; +import { expect, it } from "vitest"; + +// The Python major is a contract, not a coincidence — the host half. The twin of +// `nodeEngine.contract.test.ts`, and written the day after that one was: `0.93.2` shipped because +// four machines disagreed about Node, and the Python side turned out to have the identical shape +// with nobody watching. Prod (`Dockerfile`), the test image (`backend/Dockerfile.test`) and CI +// (`check.yml`) each spelled 3.12 as its own literal, there was no `.python-version`, and the dev +// machine had only 3.13 and 3.14 installed — so every ad-hoc host script ran two majors ahead of +// production. +// +// It lives in the FRONTEND suite for one boring reason: this is the only runner that can see the +// whole repository. The backend suite runs inside the test image, which mounts `backend/` alone, so +// it cannot read the repo-root Dockerfile or the workflow. It holds the other half of the chain — +// including "the interpreter running these tests IS the pinned one", which only it can assert: +// +// backend/tests/test_python_engine_contract.py : running interpreter == pyrightconfig == ruff.toml +// here : .python-version == pyrightconfig == ruff.toml +// == Dockerfile == Dockerfile.test == check.yml +// +// To move the version: edit `.python-version`, then follow the failures in both files. +const readRoot = (rel: string) => readFileSync(new URL(`../../../${rel}`, import.meta.url), "utf8"); + +function versionIn(source: string, pattern: RegExp, where: string): string { + const m = pattern.exec(source); + // A rename or a moved line lands here. Re-point the test — the contract did not go away with it. + expect(m, `${pattern} not found in ${where}`).not.toBeNull(); + return `${m![1]}.${m![2]}`; +} + +const pinned = versionIn(readRoot(".python-version"), /^(\d+)\.(\d+)\s*$/m, ".python-version"); + +it("pins a plausible Python version", () => { + // Guards against a typo'd `.python-version` silently agreeing with nothing. + expect(Number(pinned.split(".")[1])).toBeGreaterThanOrEqual(12); + expect(pinned.split(".")[0]).toBe("3"); +}); + +it("runs the shipped backend on the pinned version", () => { + // The image our users' instance actually runs. + expect(versionIn(readRoot("Dockerfile"), /^FROM python:(\d+)\.(\d+)-slim/m, "Dockerfile")).toBe( + pinned, + ); +}); + +it("tests on the pinned version", () => { + // The image the whole pytest suite runs in — including the backend half of this contract, which + // asserts its own `sys.version_info` against the pin. + expect( + versionIn( + readRoot("backend/Dockerfile.test"), + /^FROM python:(\d+)\.(\d+)-slim/m, + "Dockerfile.test", + ), + ).toBe(pinned); +}); + +it("runs the server gate on the pinned version", () => { + // The backend job's container. Its NODE is deliberately outside the Node contract (it only runs + // the checkout action), but its PYTHON runs our code and our tests, so it is inside this one. + expect( + versionIn( + readRoot(".gitea/workflows/check.yml"), + /^\s*container: nikolaik\/python-nodejs:python(\d+)\.(\d+)-nodejs\d+$/m, + "check.yml", + ), + ).toBe(pinned); +}); + +it("type-checks against the pinned version", () => { + // A type checker aimed at another version green-lights APIs the shipped interpreter does not have + // — the same defect as the image split, moved to check time. + expect( + versionIn( + readRoot("backend/pyrightconfig.json"), + /"pythonVersion"\s*:\s*"(\d+)\.(\d+)"/, + "pyrightconfig.json", + ), + ).toBe(pinned); +}); + +it("lints against the pinned version", () => { + expect( + versionIn(readRoot("backend/ruff.toml"), /^target-version = "py(\d)(\d+)"$/m, "ruff.toml"), + ).toBe(pinned); +}); From 7d682c90d567976e2a0961ca7b5844c96c89439d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 20 Sep 2026 23:21:08 +0200 Subject: [PATCH 05/21] build(frontend): React 18 -> 19 with the React Compiler (E27 S1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The compiler auto-memoises, which is the point: it infers what the app's 112 `useCallback` and 48 `useMemo` call sites were hand-written for and applies the equivalent itself. It is on repo-wide with no opt-out list — it bails on anything it cannot prove safe, and says so through the lint rules. Verified in the built bundle rather than assumed: 369 `react.memo_cache_sentinel` markers across 35 of the 76 chunks. ONE transform, shared by vite.config.ts and vitest.config.ts through reactPlugin.ts. Not two `react()` calls: if the compiler ran only in the production build, every vitest run would exercise code the user never receives and a compiler-induced change would ship green — the same gate-vs-image split 0.93.2 was released to fix. The plugin is imported rather than named by the usual string, because a string is invisible to knip and, worse, a typo in it is a silent no-op that ships an uncompiled bundle, while a bad import is a tsc error. eslint-plugin-react-hooks 5 -> 7 brings the whole compiler rule set: 283 findings, triaged rather than bulk-suppressed, because the two halves needed different answers. react-hooks/refs (217) is OFF. 153 of those 217 are this app's popover architecture, not its history: 79 are @floating-ui/react's documented `pop.refs.setReference` / `getReferenceProps` callback-ref setters, and 74 more are passing the popover handle down as a prop, which is what components/ui's popover engine is for. Suppressing them would not help the next popover anyone writes. The remaining ~64 are genuine render-phase `ref.current = x` writes and are real, but the rule cannot tell them apart here; they are E27 S2's work. The other 66 go into eslint-suppressions.json as a ratchet — new violations fail the gate, and `--prune-suppressions` makes a fixed one fail too until it is removed. 45 of them are `set-state-in-effect`, which IS E27 S2's subject, so that sprint now has a measured worklist. react-i18next deliberately stays on 14. It never declared a React-19 peer — every version from 14 to 17 asks only for `react >= 16.8.0` — and what the newer lines actually require is i18next 25/26 against our 23. That is an i18n runtime migration wearing a React hat, and it buys this sprint nothing. Two React 19 type changes: `useRef(null)` is now `RefObject` (the honest shape — the element is null until it mounts), so PlexPlayerSettings' prop follows; and reactPlugin.ts joins tsconfig.node.json's include. The compiler skips memoising VirtualGrid and reports it: TanStack Virtual's `useVirtualizer()` returns functions that cannot be memoised without going stale. That is the right trade for a component whose cost is its row window, and it is the one such site in the app, so it carries a targeted disable with the reason — a second incompatible library anywhere else must still stop the gate. Nine lanes green, vitest 1558. Browser UAT on the dev server against the still-running React 18 build as a control: boot, feed (virtualized grid, popover menu, a filter change that moved the list 195,187 -> 64 items with the URL mirrored, so no stale memoised UI), command palette, player modal — no console errors on any of it. --- frontend/eslint-suppressions.json | 253 ++++++++++++++++++ frontend/eslint.config.js | 19 ++ frontend/package-lock.json | 134 ++++++---- frontend/package.json | 11 +- frontend/reactPlugin.ts | 26 ++ .../src/components/PlexPlayerSettings.tsx | 5 +- frontend/src/components/ui/VirtualGrid.tsx | 9 + frontend/tsconfig.node.json | 11 +- frontend/vite.config.ts | 4 +- frontend/vitest.config.ts | 4 +- 10 files changed, 409 insertions(+), 67 deletions(-) create mode 100644 frontend/eslint-suppressions.json create mode 100644 frontend/reactPlugin.ts diff --git a/frontend/eslint-suppressions.json b/frontend/eslint-suppressions.json new file mode 100644 index 00000000..9eb1d0c3 --- /dev/null +++ b/frontend/eslint-suppressions.json @@ -0,0 +1,253 @@ +{ + "src/components/BootGate.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ChannelPage.tsx": { + "react-hooks/set-state-in-effect": { + "count": 2 + } + }, + "src/components/ChannelTags.tsx": { + "react-hooks/set-state-in-effect": { + "count": 2 + } + }, + "src/components/ChannelsProvider.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ConfigPanel.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/DateFilterSelect.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/DebugRecorder.dom.test.tsx": { + "react-hooks/globals": { + "count": 1 + } + }, + "src/components/DebugRecorder.tsx": { + "react-hooks/purity": { + "count": 1 + } + }, + "src/components/DiagnosticsSettings.tsx": { + "react-hooks/purity": { + "count": 1 + } + }, + "src/components/DownloadCenter.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/Feed.tsx": { + "react-hooks/immutability": { + "count": 1 + }, + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/FeedFiltersProvider.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/GlassTuner.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + }, + "react-hooks/use-memo": { + "count": 1 + } + }, + "src/components/Home.tsx": { + "react-hooks/set-state-in-effect": { + "count": 2 + } + }, + "src/components/IssueComposer.tsx": { + "react-hooks/immutability": { + "count": 2 + } + }, + "src/components/MyLogViewer.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/NavigationProvider.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/PersonalViewsModal.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/PlayerModal.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/Playlists.tsx": { + "react-hooks/immutability": { + "count": 2 + }, + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/PlexBrowse.tsx": { + "react-hooks/immutability": { + "count": 3 + }, + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/PlexInfo.tsx": { + "react-hooks/static-components": { + "count": 3 + } + }, + "src/components/PlexPlayer.tsx": { + "react-hooks/immutability": { + "count": 1 + }, + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/PlexSearchBox.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/SavedViewsProvider.tsx": { + "react-hooks/set-state-in-effect": { + "count": 3 + } + }, + "src/components/Scheduler.tsx": { + "react-hooks/set-state-in-effect": { + "count": 2 + } + }, + "src/components/SetupWizard.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/TagFilterChips.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/WatchPage.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/Welcome.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/WizardProvider.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/comfortPin.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/downloadRow.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ui/Avatar.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ui/BackToTop.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ui/Combobox.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ui/CommandPalette.tsx": { + "react-hooks/set-state-in-effect": { + "count": 2 + } + }, + "src/components/ui/Pager.tsx": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ui/Toaster.tsx": { + "react-hooks/globals": { + "count": 1 + }, + "react-hooks/set-state-in-effect": { + "count": 1 + }, + "react-hooks/static-components": { + "count": 1 + } + }, + "src/components/ui/anchoredPopoverHooks.ts": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/components/ui/sortableA11y.ts": { + "react-hooks/immutability": { + "count": 1 + } + }, + "src/components/usePlexPlaylistEdits.ts": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/lib/plexPlayerHooks.subtitle.dom.test.tsx": { + "react-hooks/immutability": { + "count": 1 + } + }, + "src/lib/plexPlayerHooks.ts": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/lib/useCardPager.ts": { + "react-hooks/set-state-in-effect": { + "count": 1 + } + }, + "src/lib/useFocusKeeper.restore.dom.test.tsx": { + "react-hooks/globals": { + "count": 1 + } + } +} diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index 4dd46e4e..baf79b1d 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -187,6 +187,25 @@ export default tseslint.config( }, rules: { ...reactHooks.configs.recommended.rules, + // eslint-plugin-react-hooks 7's `recommended` is no longer two rules — it carries the whole + // React Compiler rule set (E27 S1). On this codebase that lands 283 findings, and they were + // triaged rather than bulk-suppressed, because the two answers are different: + // + // react-hooks/refs 217 — OFF, see below. + // the other 66 — real, and suppressed into eslint-suppressions.json as a RATCHET: + // new violations fail the gate, the existing list burns down in E27 S2 + // (45 of them are `set-state-in-effect`, which IS that sprint's subject). + // + // `refs` is off because it fires on this app's popover architecture, not on its history, so + // suppressing it would not help the next popover anyone writes. Measured over the 217: + // 79 are `pop.refs.setReference` / `getReferenceProps` — @floating-ui/react's callback-ref + // setters, which are the documented way to use it — and 74 more are passing the popover handle + // down as a prop (`popover={pop}`, `ref={fade.ref}`), which is what `components/ui`'s popover + // engine is FOR. That is 153 of 217 flagged for having the word `refs` in an object that is not + // a React ref. The remaining ~64 are genuine render-phase `someRef.current = x` writes and are + // real; they are E27 S2's work, not this rule's, because the rule cannot tell them apart here. + // Re-evaluate when the rule can: it shipped in 1.0.0 and this looks like over-approximation. + "react-hooks/refs": "off", "local/jsx-comment-text": "error", "local/focus-safe-disabled": "error", "local/literal-a11y-text": "error", diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 8d6a49e1..9294d8cc 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -18,8 +18,8 @@ "hls.js": "^1.6.16", "i18next": "^23.11.5", "lucide-react": "^0.408.0", - "react": "^18.3.1", - "react-dom": "^18.3.1", + "react": "19.3.0", + "react-dom": "19.3.0", "react-i18next": "^14.1.2" }, "devDependencies": { @@ -28,12 +28,13 @@ "@tailwindcss/postcss": "^4.3.3", "@testing-library/react": "^16.3.2", "@types/node": "^24.13.6", - "@types/react": "^18.3.3", - "@types/react-dom": "^18.3.0", + "@types/react": "19.3.0", + "@types/react-dom": "19.3.0", "@vitejs/plugin-react": "^4.3.1", + "babel-plugin-react-compiler": "1.0.0", "eslint": "9.39.1", "eslint-config-prettier": "10.1.8", - "eslint-plugin-react-hooks": "5.2.0", + "eslint-plugin-react-hooks": "7.1.1", "eslint-plugin-react-refresh": "0.4.24", "globals": "16.5.0", "jsdom": "^25.0.1", @@ -2922,32 +2923,24 @@ "undici-types": "~7.18.0" } }, - "node_modules/@types/prop-types": { - "version": "15.7.15", - "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.15.tgz", - "integrity": "sha512-F6bEyamV9jKGAFBEmlQnesRPGOQqS2+Uwi0Em15xenOxHaf2hv6L8YCVn3rPdPJOiJfPiCnLIRyvwVaqMY3MIw==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/react": { - "version": "18.3.31", - "resolved": "https://registry.npmjs.org/@types/react/-/react-18.3.31.tgz", - "integrity": "sha512-vfEqpXTvwT91yhmwdfouStN2hSKwTvyRs8qpLfADyrq/kxDw0hZM7Wk9Ug1FELj8hIby+S/+kQCSRFF32nv2Qw==", + "version": "19.3.0", + "resolved": "https://registry.npmjs.org/@types/react/-/react-19.3.0.tgz", + "integrity": "sha512-N0rFCuH9YoxG9/m61l9MfpJKfmLOVU0em7ipIz6TRgSSkvReLB9vL85GB+yr8Bs5leqpvg96JSwF4ZS1s4viQg==", "dev": true, "license": "MIT", "dependencies": { - "@types/prop-types": "*", "csstype": "^3.2.2" } }, "node_modules/@types/react-dom": { - "version": "18.3.7", - "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-18.3.7.tgz", - "integrity": "sha512-MEe3UeoENYVFXzoXEWsvcpg6ZvlrFNlOQ7EOsvhI3CfAXwzPfO8Qwuxd40nepsYKqyyVQnTdEfv68q91yLcKrQ==", + "version": "19.3.0", + "resolved": "https://registry.npmjs.org/@types/react-dom/-/react-dom-19.3.0.tgz", + "integrity": "sha512-ZI7bU42mZXXKHn/qNLEw2IrbiINU7X5+vfgdixBHkCNpYWXjKgfQ/P+uyGb5CjOLB9UcnTeg3rylQtV2hym44Q==", "dev": true, "license": "MIT", "peerDependencies": { - "@types/react": "^18.0.0" + "@types/react": "^19.3.0" } }, "node_modules/@typescript-eslint/eslint-plugin": { @@ -3469,6 +3462,16 @@ "dev": true, "license": "MIT" }, + "node_modules/babel-plugin-react-compiler": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/babel-plugin-react-compiler/-/babel-plugin-react-compiler-1.0.0.tgz", + "integrity": "sha512-Ixm8tFfoKKIPYdCCKYTsqv+Fd4IJ0DQqMyEimo+pxUOMUR9cVPlwTrFt9Avu+3cb6Zp3mAzl+t1MrG2fxxKsxw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@babel/types": "^7.26.0" + } + }, "node_modules/balanced-match": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", @@ -4084,16 +4087,23 @@ } }, "node_modules/eslint-plugin-react-hooks": { - "version": "5.2.0", - "resolved": "https://registry.npmjs.org/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-5.2.0.tgz", - "integrity": "sha512-+f15FfK64YQwZdJNELETdn5ibXEUQmW1DZL6KXhNnc2heoy/sg9VJJeT7n8TlMWouzWqSWavFkIhHyIbIAEapg==", + "version": "7.1.1", + "resolved": "https://registry.npmjs.org/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-7.1.1.tgz", + "integrity": "sha512-f2I7Gw6JbvCexzIInuSbZpfdQ44D7iqdWX01FKLvrPgqxoE7oMj8clOfto8U6vYiz4yd5oKu39rRSVOe1zRu0g==", "dev": true, "license": "MIT", + "dependencies": { + "@babel/core": "^7.24.4", + "@babel/parser": "^7.24.4", + "hermes-parser": "^0.25.1", + "zod": "^3.25.0 || ^4.0.0", + "zod-validation-error": "^3.5.0 || ^4.0.0" + }, "engines": { - "node": ">=10" + "node": ">=18" }, "peerDependencies": { - "eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0" + "eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0 || ^9.0.0 || ^10.0.0" } }, "node_modules/eslint-plugin-react-refresh": { @@ -4586,6 +4596,23 @@ "node": ">= 0.4" } }, + "node_modules/hermes-estree": { + "version": "0.25.1", + "resolved": "https://registry.npmjs.org/hermes-estree/-/hermes-estree-0.25.1.tgz", + "integrity": "sha512-0wUoCcLp+5Ev5pDW2OriHC2MJCbwLwuRx+gAqMTOkGKJJiBCLjtrvy4PWUGn6MIVefecRpzoOZ/UV6iGdOr+Cw==", + "dev": true, + "license": "MIT" + }, + "node_modules/hermes-parser": { + "version": "0.25.1", + "resolved": "https://registry.npmjs.org/hermes-parser/-/hermes-parser-0.25.1.tgz", + "integrity": "sha512-6pEjquH3rqaI6cYAXYPcz9MS4rY6R4ngRgrgfDshRptUZIc3lw0MCIJIGDj9++mfySOuPTHB4nrSW99BCvOPIA==", + "dev": true, + "license": "MIT", + "dependencies": { + "hermes-estree": "0.25.1" + } + }, "node_modules/hls.js": { "version": "1.6.16", "resolved": "https://registry.npmjs.org/hls.js/-/hls.js-1.6.16.tgz", @@ -4776,6 +4803,7 @@ "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", "integrity": "sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ==", + "dev": true, "license": "MIT" }, "node_modules/js-yaml": { @@ -5279,18 +5307,6 @@ "dev": true, "license": "MIT" }, - "node_modules/loose-envify": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/loose-envify/-/loose-envify-1.4.0.tgz", - "integrity": "sha512-lyuxPGr/Wfhrlem2CL/UcnUc1zcqKAImBDzukY7Y5F/yQiNdko6+fRLevlw1HgMySw7f611UIY408EtxRSoK3Q==", - "license": "MIT", - "dependencies": { - "js-tokens": "^3.0.0 || ^4.0.0" - }, - "bin": { - "loose-envify": "cli.js" - } - }, "node_modules/loupe": { "version": "3.2.1", "resolved": "https://registry.npmjs.org/loupe/-/loupe-3.2.1.tgz", @@ -5824,28 +5840,24 @@ "license": "MIT" }, "node_modules/react": { - "version": "18.3.1", - "resolved": "https://registry.npmjs.org/react/-/react-18.3.1.tgz", - "integrity": "sha512-wS+hAgJShR0KhEvPJArfuPVN1+Hz1t0Y6n5jLrGQbkb4urgPE/0Rve+1kMB1v/oWgHgm4WIcV+i7F2pTVj+2iQ==", + "version": "19.3.0", + "resolved": "https://registry.npmjs.org/react/-/react-19.3.0.tgz", + "integrity": "sha512-E8LUcbtBWt20bbl2YoHfx4ZDBdxVTfOKtCZn9cDSJ4l6/nuoApcpIBcj47t2wZoVX8g2ZHuMHbiShgCR1T5Sog==", "license": "MIT", - "dependencies": { - "loose-envify": "^1.1.0" - }, "engines": { "node": ">=0.10.0" } }, "node_modules/react-dom": { - "version": "18.3.1", - "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-18.3.1.tgz", - "integrity": "sha512-5m4nQKp+rZRb09LNH59GM4BxTh9251/ylbKIbpe7TpGxfJ+9kv6BLkLBXIjjspbgbnIBNqlI23tRnTWT0snUIw==", + "version": "19.3.0", + "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.3.0.tgz", + "integrity": "sha512-JDk8dgif51OjFoDE70+OT9ICyYr+69HlmihNwp1+Nsfbna3t5sIiCa9ZJktDmQ4/1b/rn26hIAR2uYXDMr5r0Q==", "license": "MIT", "dependencies": { - "loose-envify": "^1.1.0", - "scheduler": "^0.23.2" + "scheduler": "^0.28.0" }, "peerDependencies": { - "react": "^18.3.1" + "react": "^19.3.0" } }, "node_modules/react-i18next": { @@ -6016,13 +6028,10 @@ } }, "node_modules/scheduler": { - "version": "0.23.2", - "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.23.2.tgz", - "integrity": "sha512-UOShsPwz7NrMUqhR6t0hWjFduvOzbtv7toDH1/hIrfRNIDBnnBWd0CwJTGvTpngVlmwGCdP9/Zl/tVrDqcuYzQ==", - "license": "MIT", - "dependencies": { - "loose-envify": "^1.1.0" - } + "version": "0.28.0", + "resolved": "https://registry.npmjs.org/scheduler/-/scheduler-0.28.0.tgz", + "integrity": "sha512-juorfCmIkIw8tT+p5BXSm6PJjQF/ycEYmKyzURCIt/RaZIhL+PulbQ9Yu2z1HdOJDdqDTlxA1+xKBmHXJsczAw==", + "license": "MIT" }, "node_modules/semver": { "version": "6.3.1", @@ -6836,6 +6845,19 @@ "funding": { "url": "https://github.com/sponsors/colinhacks" } + }, + "node_modules/zod-validation-error": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/zod-validation-error/-/zod-validation-error-4.0.2.tgz", + "integrity": "sha512-Q6/nZLe6jxuU80qb/4uJ4t5v2VEZ44lzQjPDhYJNztRQ4wyWc6VF3D3Kb/fAuPetZQnhS3hnajCf9CsWesghLQ==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=18.0.0" + }, + "peerDependencies": { + "zod": "^3.25.0 || ^4.0.0" + } } } } diff --git a/frontend/package.json b/frontend/package.json index ab3e43d6..904a0586 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -34,8 +34,8 @@ "hls.js": "^1.6.16", "i18next": "^23.11.5", "lucide-react": "^0.408.0", - "react": "^18.3.1", - "react-dom": "^18.3.1", + "react": "19.3.0", + "react-dom": "19.3.0", "react-i18next": "^14.1.2" }, "devDependencies": { @@ -44,12 +44,13 @@ "@tailwindcss/postcss": "^4.3.3", "@testing-library/react": "^16.3.2", "@types/node": "^24.13.6", - "@types/react": "^18.3.3", - "@types/react-dom": "^18.3.0", + "@types/react": "19.3.0", + "@types/react-dom": "19.3.0", "@vitejs/plugin-react": "^4.3.1", + "babel-plugin-react-compiler": "1.0.0", "eslint": "9.39.1", "eslint-config-prettier": "10.1.8", - "eslint-plugin-react-hooks": "5.2.0", + "eslint-plugin-react-hooks": "7.1.1", "eslint-plugin-react-refresh": "0.4.24", "globals": "16.5.0", "jsdom": "^25.0.1", diff --git a/frontend/reactPlugin.ts b/frontend/reactPlugin.ts new file mode 100644 index 00000000..044a1da6 --- /dev/null +++ b/frontend/reactPlugin.ts @@ -0,0 +1,26 @@ +import react from "@vitejs/plugin-react"; +import reactCompiler from "babel-plugin-react-compiler"; + +// ONE React transform, shared by vite.config.ts and vitest.config.ts (E27 S1). +// +// It is a shared factory rather than two `react()` calls because the two configs must not drift: +// if the compiler ran only in the production build, every vitest run would be exercising code the +// user never receives, and a compiler-induced behaviour change would ship green. That is precisely +// the defect `0.93.2` was released to fix — a gate and a build image that had quietly stopped being +// the same thing — so the rule here is that the gate compiles the code the same way the build does. +// +// The React Compiler auto-memoises: it infers what `useCallback`/`useMemo` were being hand-written +// for and applies the equivalent itself. It is enabled repo-wide with no opt-out list because it +// bails out on its own for anything it cannot prove safe (its rules are also linted, via +// eslint-plugin-react-hooks). `target` follows the installed React major and must move with it. +// +// The plugin is IMPORTED rather than named by the usual `"babel-plugin-react-compiler"` string. Two +// reasons, both about the gate: a string is invisible to knip, which then reports the dependency as +// unused, and — worse — a typo in that string is a silent no-op that ships an uncompiled bundle, +// while a bad import is a resolve error on the tsc lane. +export const reactWithCompiler = () => + react({ + babel: { + plugins: [[reactCompiler, { target: "19" }]], + }, + }); diff --git a/frontend/src/components/PlexPlayerSettings.tsx b/frontend/src/components/PlexPlayerSettings.tsx index 0a8e9211..994f1d86 100644 --- a/frontend/src/components/PlexPlayerSettings.tsx +++ b/frontend/src/components/PlexPlayerSettings.tsx @@ -12,7 +12,10 @@ type SettingsTab = "sync" | "playback" | "subtitle" | "clock"; // panel owns no state of its own. `now` is the live wall clock (passed in, not a second ticker) used // only for the clock-date-style preview buttons. export function PlexPlayerSettings(props: { - menuRef: RefObject; + // `| null` is not optional under React 19's types: `useRef(null)` is now `RefObject`, + // which is the honest shape — the element is null until the div mounts. The old `RefObject` lied + // about that and 18's types let it. + menuRef: RefObject; settingsTabs: { key: SettingsTab; label: string }[]; activeTab: SettingsTab; setSettingsTab: (tab: SettingsTab) => void; diff --git a/frontend/src/components/ui/VirtualGrid.tsx b/frontend/src/components/ui/VirtualGrid.tsx index 9180df48..98ff4a8e 100644 --- a/frontend/src/components/ui/VirtualGrid.tsx +++ b/frontend/src/components/ui/VirtualGrid.tsx @@ -127,6 +127,15 @@ export default function VirtualGrid({ const rows = useMemo(() => chunk(items, colCount), [items, colCount]); + // The React Compiler (E27 S1) SKIPS memoising this whole component, and says so: TanStack + // Virtual's `useVirtualizer()` hands back functions that cannot be memoised without going stale, + // so the compiler bails rather than risk a frozen viewport. That is the correct trade here — this + // component's cost is the row window it mounts, not its render closures — and it is the ONE such + // site in the app, which is why this is a targeted disable rather than the rule being turned off: + // a second incompatible library anywhere else must still stop the gate. The caution in the rule's + // own text applies, though: values taken from this API and handed to a memoised child can go + // stale, so keep the virtualizer's output inside this component. + // eslint-disable-next-line react-hooks/incompatible-library -- see above; @tanstack/react-virtual const virtualizer = useVirtualizer({ count: rows.length, getScrollElement: () => scrollEl, diff --git a/frontend/tsconfig.node.json b/frontend/tsconfig.node.json index 60b25c23..1c9ac7a5 100644 --- a/frontend/tsconfig.node.json +++ b/frontend/tsconfig.node.json @@ -30,5 +30,14 @@ "allowJs": true, "checkJs": true }, - "include": ["vite.config.ts", "vitest.config.ts", "tailwind.config.js", "postcss.config.js"] + // reactPlugin.ts is the ONE React transform both configs import (E27 S1) — it is build tooling, + // not app source, so it belongs here and not under tsconfig.json's ["src"]. A file imported by a + // listed one but absent from `include` is TS6307, not a silent pass. + "include": [ + "vite.config.ts", + "vitest.config.ts", + "reactPlugin.ts", + "tailwind.config.js", + "postcss.config.js" + ] } diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 970969ee..b49c66a3 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -1,5 +1,5 @@ import { defineConfig } from "vite"; -import react from "@vitejs/plugin-react"; +import { reactWithCompiler } from "./reactPlugin"; // During `vite dev` (host with Node), proxy API calls to the backend container. // Use 127.0.0.1 (not "localhost") so Node doesn't resolve to IPv6 ::1, which the @@ -20,7 +20,7 @@ const proxy = { }; export default defineConfig({ - plugins: [react()], + plugins: [reactWithCompiler()], server: { port: 5173, proxy }, build: { outDir: "dist" }, }); diff --git a/frontend/vitest.config.ts b/frontend/vitest.config.ts index 96c816ae..3545f74a 100644 --- a/frontend/vitest.config.ts +++ b/frontend/vitest.config.ts @@ -1,5 +1,5 @@ import { defineConfig } from "vitest/config"; -import react from "@vitejs/plugin-react"; +import { reactWithCompiler } from "./reactPlugin"; // Unit + component/hook tests, deliberately separate from vite.config.ts so the production build // never has to load vitest. The DEFAULT environment is node (the pure src/lib helpers — fast, no @@ -9,7 +9,7 @@ import react from "@vitejs/plugin-react"; // free React transform; it is a no-op for the plain .ts node tests (they carry no JSX). jsdom is a // devDependency referenced only via that docblock env string — knip's vitest plugin still sees it. export default defineConfig({ - plugins: [react()], + plugins: [reactWithCompiler()], test: { // .tsx included on purpose: a test file the glob misses does not fail, it is simply never // collected, and the gate still prints green. From 7e1aeeb9fef4529e472e80974b7099c5d35a0f5d Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sun, 20 Sep 2026 23:57:08 +0200 Subject: [PATCH 06/21] refactor(frontend): honest effect deps in the app shell and the ui layer (E27 S2) Burns the first 14 of the 113 hook suppressions. Three verdicts, applied per site: A useEffect that should not exist (6): - Avatar remembered "the image failed" as a boolean an effect reset on src change. The component is recycled across items in the virtualized feed, so after one avatar 429s the next item's slot committed the fallback initial over a perfectly good image and only the post-paint effect cleared it. The failure is now keyed BY URL; derived state cannot be stale. - Combobox cleared the typed query from an effect on `open`. Moved into setOpenAnd, the single funnel every close goes through. That also stops a spurious onQueryChange("") AT MOUNT, which told a controlled caller its own initial query had been cleared before the user touched it. - CommandPalette reset query + picked row from an effect. Closed now means UNMOUNTED, not hidden, so a fresh open starts blank because mounting already means that -- and the ranking memo over every channel, playlist and saved view stops running while the palette is shut. - Pager's page box held a copy of the page that an effect re-synced; it now derives it and keeps a draft only while being edited. - App's ?link= / ?oauth= / ?admin= one-shots asserted "once per mount" with an empty dep list. The ref guard the codebase already uses in 22 places ENFORCES it, so the honest deps can include `t` -- which a language switch legitimately changes, and which would otherwise have re-fired the toast. Honest deps that also fix something (5): - App's module preload was keyed on the account id, so a permission change that alters the module set without moving the id never re-preloaded. `preload()` is idempotent, so depending on `me` costs nothing. - App's access-request nudge now re-issues on a language switch; it used to stay in the old language until the pending count changed. - App's diagnostics effect: applyDiagnostics already no-ops on an unchanged effective level, so picking four fields out by hand was guarding nothing and missed any field added later. - DataTable's two filter effects: a ref holds the latest onFiltersChange so commitFilters can be stable, which is what lets the effects name it instead of switching the check off. Genuine escape hatches (7) moved OUT of eslint-suppressions.json into inline directives that name the invariant at the site -- the toast handover's render-phase global, the live region's deliberate arming tick, the docked one-slot component, the popover engine's floating-ui list state, dnd-kit's per-drag `moved` flag, BackToTop's DOM read and the palette's debounced announcement. A reader of the code now sees why; a JSON file no one opens does not. Tests: Avatar (3, new file) and Pager (3) pin the derived-state frames with a layout-phase probe -- layout effects run before any passive effect, so they read the frame the browser would paint. Combobox (2) pins the close-clears-query contract and the no-callback-at-mount rule. All three sets mutation-checked against the effect-based originals. Suppressions 113 -> 99; exhaustive-deps 45 -> 35. --- frontend/eslint-suppressions.json | 46 -------- frontend/src/App.tsx | 102 ++++++++++++------ .../src/components/ui/Avatar.dom.test.tsx | 64 +++++++++++ frontend/src/components/ui/Avatar.tsx | 13 ++- frontend/src/components/ui/BackToTop.tsx | 1 + .../src/components/ui/Combobox.dom.test.tsx | 48 +++++++++ frontend/src/components/ui/Combobox.tsx | 13 +-- frontend/src/components/ui/CommandPalette.tsx | 21 ++-- frontend/src/components/ui/DataTable.tsx | 34 ++++-- frontend/src/components/ui/Pager.dom.test.tsx | 78 ++++++++++++++ frontend/src/components/ui/Pager.tsx | 18 ++-- frontend/src/components/ui/Toaster.tsx | 6 ++ .../src/components/ui/anchoredPopoverHooks.ts | 1 + frontend/src/components/ui/sortableA11y.ts | 5 + 14 files changed, 334 insertions(+), 116 deletions(-) create mode 100644 frontend/src/components/ui/Avatar.dom.test.tsx diff --git a/frontend/eslint-suppressions.json b/frontend/eslint-suppressions.json index 9eb1d0c3..c714654c 100644 --- a/frontend/eslint-suppressions.json +++ b/frontend/eslint-suppressions.json @@ -179,52 +179,6 @@ "count": 1 } }, - "src/components/ui/Avatar.tsx": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/components/ui/BackToTop.tsx": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/components/ui/Combobox.tsx": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/components/ui/CommandPalette.tsx": { - "react-hooks/set-state-in-effect": { - "count": 2 - } - }, - "src/components/ui/Pager.tsx": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/components/ui/Toaster.tsx": { - "react-hooks/globals": { - "count": 1 - }, - "react-hooks/set-state-in-effect": { - "count": 1 - }, - "react-hooks/static-components": { - "count": 1 - } - }, - "src/components/ui/anchoredPopoverHooks.ts": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/components/ui/sortableA11y.ts": { - "react-hooks/immutability": { - "count": 1 - } - }, "src/components/usePlexPlaylistEdits.ts": { "react-hooks/set-state-in-effect": { "count": 1 diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 33fc48b7..38104105 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,4 +1,4 @@ -import { lazy, Suspense, useEffect, useState } from "react"; +import { lazy, Suspense, useEffect, useRef, useState } from "react"; import { qk } from "./lib/queryKeys"; import { useTranslation } from "react-i18next"; import { useQuery, useQueryClient } from "@tanstack/react-query"; @@ -131,6 +131,9 @@ export default function App() { }); const needsSetup = setupQuery.data?.configured === false; const meQuery = useQuery({ queryKey: qk.me(), queryFn: api.me, enabled: false }); + // The signed-in account, or null/undefined while signed out. Hoisted here because half a dozen + // effects below depend on it, and a dep list is only honest if it names the value the body reads. + const me = meQuery.data; // The `global` key scope: `h` opens the cheat sheet, Mod+K the jump-to palette, Shift+W jumps to // Home ("mit nézzek?"). Bound only once signed in, because that is where the overlays are mounted @@ -141,32 +144,40 @@ export default function App() { // page underneath (owner decision 2026-09-09: the key always ends with the answer on screen). // The `page` scope rides the same binding: the `g` chords jump to a module, but only to one this // account can reach (the nav rail's own list, plus Settings, which is not in the rail's order). - useGlobalKeymap( - !!meQuery.data, - setPage, - meQuery.data ? [...moduleOrder(meQuery.data), "settings"] : [], - ); + useGlobalKeymap(!!me, setPage, me ? [...moduleOrder(me), "settings"] : []); // The account's consented diagnostics level (E22 S5), applied to this tab's log: raised, the log is // kept in more detail and streamed to the server until the level lapses. A debug session that is // recording (E16) raises it for its own run and tags what it keeps. A lapse (its own timer, or the // server saying it is off) refetches `me`, so Settings and the recorder show what is in force. - const diagnostics = meQuery.data?.diagnostics; - const debugSession = meQuery.data?.debug_session; + // Re-running this on every `me` refetch is FREE and therefore the deps are the honest ones: + // applyDiagnostics computes an effective-level key and returns early when it has not moved + // (lib/diagnostics.ts), so an unchanged level is a no-op. It does refresh the `lapsed` callback + // before that early return, which is the half we WANT re-run. The old dep list picked out four + // primitive fields to dodge the fresh object identity a refetch produces; that is the checker's + // job to verify, not a comment's to assert, and it silently missed any other field the module + // may come to read. + const diagnostics = me?.diagnostics; + const debugSession = me?.debug_session; useEffect(() => { applyDiagnostics( diagnostics, debugSession, () => void queryClient.invalidateQueries({ queryKey: qk.me() }), ); - }, [diagnostics?.level, diagnostics?.until, debugSession?.id, debugSession?.ended_at]); // eslint-disable-line react-hooks/exhaustive-deps + }, [diagnostics, debugSession, queryClient]); // Fetch the code of every module this account can open — and the channel page — once the first page // is on screen, so a module's first visit renders at once instead of flashing the chunk fallback. // Idle-scheduled so it never competes with the first page's own requests (Safari has no // requestIdleCallback, hence the timer). + // Depending on the whole `me` rather than its id is both honest and MORE correct: the module set + // is derived from `me` through each module's own gate (lib/modules moduleOrder), so a permission + // change — an admin grant, a Plex server appearing — alters it without the id moving, and the old + // `[me.id]` dep never re-preloaded for those. Re-running costs nothing: `preload()` is idempotent + // (lazyWithPreload memoises the pending promise), so a refetch that changes nothing fetches + // nothing; only the idle callback is re-scheduled. useEffect(() => { - const me = meQuery.data; if (!me) return; const run = () => { preloadPages([...moduleOrder(me), "settings"]); @@ -179,7 +190,7 @@ export default function App() { } const id = window.setTimeout(run, 1000); return () => window.clearTimeout(id); - }, [meQuery.data?.id]); // eslint-disable-line react-hooks/exhaustive-deps + }, [me]); // Measure the scrollbar's gutter width once and publish it as --sbw, so the floating header and the // toolbar band can pad their right edge to line up with the scrolled cards (the app scroller reserves @@ -197,9 +208,13 @@ export default function App() { // Once signed in, drop any leftover ONE-TIME query params from the pre-login flow (?access, ?verify, // ?reset) — but NOT the nav/filter params, which the logged-in app now reads from and keeps in the // URL. Gated on auth so the logged-out Welcome page still reads its params first. + // Keyed on "is there a session at all", which is the whole condition the body tests. Re-running + // on an account switch was never the point — by then the params are already gone and a second + // clearParams is a no-op. + const signedIn = !!me; useEffect(() => { - if (meQuery.data) clearParams("access", "verify", "reset"); - }, [meQuery.data?.id]); // eslint-disable-line react-hooks/exhaustive-deps + if (signedIn) clearParams("access", "verify", "reset"); + }, [signedIn]); // Any request that 401s means the session ended server-side. If we were signed in (cached // `me` is a user object), reload to the login page at once (also covers any click that hits @@ -218,12 +233,20 @@ export default function App() { // Returning from an in-app Google link/upgrade round-trip (auth.py redirects to ?link=…). // Surface the outcome, refresh `me` (can_read/can_write/has_google may have flipped), land on // Settings → Account so the new state is visible, then strip the param for a clean URL. + // "Exactly once per mount" is now ENFORCED by the ref rather than asserted by an empty dep list: + // `t` changes identity on a language switch, so the honest dep list would otherwise re-fire the + // toast when the user changes language. (The old `[]` got away with it only because clearParams + // had already removed the param — an invariant held by a side effect two lines further down.) + const linkAnswered = useRef(false); + const { refetch: refetchMe } = meQuery; useEffect(() => { + if (linkAnswered.current) return; const result = new URLSearchParams(window.location.search).get("link"); if (!result) return; + linkAnswered.current = true; if (result === "ok") { notify({ level: "success", message: t("settings.account.googleLink.linked") }); - void meQuery.refetch(); + void refetchMe(); setAccountRaw(LS.settingsTab, "account"); setPage("settings"); } else { @@ -231,42 +254,59 @@ export default function App() { notify({ level: "error", message: t(`settings.account.googleLink.${key}`) }); } clearParams("link"); - }, []); // eslint-disable-line react-hooks/exhaustive-deps + }, [t, setPage, refetchMe]); // A Google OAuth flow started while already signed in (add-another-account, link, or upgrade) was // cancelled at Google's consent screen. The backend only emits this for a live session, so a // neutral toast is the right feedback — the pre-auth login banner would never render here. + // Same one-shot shape as the `?link=` handler above, same reason for the ref. + const oauthAnswered = useRef(false); useEffect(() => { + if (oauthAnswered.current) return; if (new URLSearchParams(window.location.search).get("oauth") !== "cancelled") return; + oauthAnswered.current = true; notify({ level: "info", message: t("settings.account.googleCancelled") }); clearParams("oauth"); - }, []); // eslint-disable-line react-hooks/exhaustive-deps + }, [t]); // Deep-link from the "new access request" admin email (?admin=access-requests) → jump straight to // the admin Users → Access requests view. Snapshot the param up front (read once at mount); act once // `me` is known, and only for an admin (others just land on their default page). const [adminDeepLink] = useState(() => new URLSearchParams(window.location.search).get("admin")); + // Ref-guarded like the two above, and here it closes a real hole rather than only satisfying the + // checker: the snapshot lives for the whole session, so with honest deps a later role change — + // or any `me` refetch, under the old id-only key — would yank the user to Users long after the + // deep link was answered. + const adminDeepLinkAnswered = useRef(false); useEffect(() => { - if (adminDeepLink !== "access-requests" || !meQuery.data) return; - if (meQuery.data.role === "admin") { + if (adminDeepLinkAnswered.current) return; + if (adminDeepLink !== "access-requests" || !me) return; + adminDeepLinkAnswered.current = true; + if (me.role === "admin") { focusAccessRequestsTab(); setPage("users"); } clearParams("admin"); - }, [adminDeepLink, meQuery.data?.id]); // eslint-disable-line react-hooks/exhaustive-deps + }, [adminDeepLink, me, setPage]); // Nudge admins when access requests are waiting (in lieu of a server-push bell). Keyed on the // pending COUNT so it re-resolves the moment an approval/denial changes it — not only on a // fresh load (which is why the notice used to linger until F5 after an approval). + // Honest deps, and `t` among them is a fix rather than a concession: the nudge is a persisted + // toast whose text is translated at issue time, so a language switch must re-issue it. It did not + // before — the notice stayed in the old language until the count changed. + const role = me?.role; + const pendingInvites = me?.pending_invites ?? 0; + const pendingActivations = me?.pending_activations ?? 0; useEffect(() => { - if (meQuery.data?.role !== "admin") return; + if (role !== "admin") return; // Keep a single, current nudge: drop any prior one (persisted copies reload as dismissed // history, so they'd otherwise pile up one-per-reload) before re-issuing the live notice. removeByMetaKind("access-requests"); // Both kinds of "someone is waiting on you": a request needing a decision, and a sign-up whose // address was approved before they registered. The second used to produce NO signal at all — // nothing lands in the queue for it — which is how the 2026-09-03 registrant went unnoticed. - const pending = (meQuery.data.pending_invites ?? 0) + (meQuery.data.pending_activations ?? 0); + const pending = pendingInvites + pendingActivations; if (pending > 0) { notify({ level: "warning", @@ -285,8 +325,7 @@ export default function App() { }, }); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [meQuery.data?.role, meQuery.data?.pending_invites, meQuery.data?.pending_activations]); + }, [role, pendingInvites, pendingActivations, t, setPage]); // Language is edited outside the Settings page (sidebar/login), so it stays instant. function changeLanguage(code: LangCode) { @@ -302,7 +341,7 @@ export default function App() { if (needsSetup) return ; // /api/me answers 200 always (null body = logged out), and BootGate has already handled a failed // probe — so no data here means "not signed in" → the public landing. - if (!meQuery.data) return ; + if (!me) return ; // Shown while a page's chunk is still on its way: only if its visit beats the preload above (or the // preload failed), and like every loading state it stays invisible for a load that is quick anyway. @@ -323,16 +362,15 @@ export default function App() { // page the account can't reach (a gated module via stale nav / a deep link) falls back to the feed, // exactly as the old ternary's fallthrough did. A channel page overlays the content column, so no // side rail shows behind it. - const me = meQuery.data!; const pageGate = moduleDef(page).gate; - const activePage: Page = !pageGate || pageGate(me) ? page : "feed"; + const activePage: Page = !pageGate || pageGate(me!) ? page : "feed"; const { Component: PageContent, maxWidth } = PAGE_CONTENT[activePage]; const railDef = channelView ? undefined : PAGE_RAIL[activePage]; return (
setAboutOpen(true)} onChangeLanguage={changeLanguage} language={i18n.language as LangCode} @@ -368,7 +406,7 @@ export default function App() { key={channelView.id} channelId={channelView.id} initialName={channelView.name} - me={meQuery.data!} + me={me!} view={view} setView={setView} onShowInFeed={() => { @@ -393,11 +431,11 @@ export default function App() { } + header={
} banners={ <> - {meQuery.data!.is_demo && } + {me!.is_demo && } openReleaseNotes(FRONTEND_VERSION)} /> } @@ -417,9 +455,9 @@ export default function App() { "take me to X" the same way. Their keys are bound by useGlobalKeymap above. */} - {meQuery.data && !meQuery.data.is_demo && } + {me && !me.is_demo && } {/* The debug recorder (E16): its start dialog, and its pill docked under the toasts. */} - {meQuery.data && !meQuery.data.is_demo && } + {me && !me.is_demo && } {aboutOpen && ( { + painted.push(document.body.innerHTML); + }); + return ; +} + +describe("Avatar · a failed image must not follow the component to the next item", () => { + it("shows the new src in the SAME commit that changes it, with no fallback frame between", () => { + const painted: string[] = []; + const { rerender } = render(); + + fireEvent.error(screen.getByRole("img", { name: "avatar" })); + expect(screen.queryByRole("img")).toBeNull(); + expect(screen.getByText("Z")).toBeTruthy(); + + painted.length = 0; + rerender(); + + // The FIRST frame after the src change already carries the image. Reading the last snapshot + // instead would let a passive-effect repair pass, which is precisely the bug. + expect(painted.length).toBeGreaterThan(0); + expect(painted[0]).toContain("https://cdn.test/b.jpg"); + expect(painted[0]).not.toContain(">Z<"); + }); + + it("keeps the fallback for the URL that actually failed, not for whatever is current", () => { + const { rerender } = render( + , + ); + fireEvent.error(screen.getByRole("img", { name: "avatar" })); + expect(screen.getByText("Z")).toBeTruthy(); + + rerender(); + expect(screen.getByRole("img", { name: "avatar" }).getAttribute("src")).toBe( + "https://cdn.test/b.jpg", + ); + }); + + it("falls back when there is no src at all", () => { + render(); + expect(screen.queryByRole("img")).toBeNull(); + expect(screen.getByText("Z")).toBeTruthy(); + }); +}); diff --git a/frontend/src/components/ui/Avatar.tsx b/frontend/src/components/ui/Avatar.tsx index c7477b5c..3dad0c1a 100644 --- a/frontend/src/components/ui/Avatar.tsx +++ b/frontend/src/components/ui/Avatar.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useState } from "react"; // Avatar image with a graceful fallback. // @@ -20,8 +20,13 @@ export default function Avatar({ fallback?: string; // a name/title; its first letter is shown when there's no image className?: string; }) { - const [failed, setFailed] = useState(false); - useEffect(() => setFailed(false), [src]); + // The failure is remembered BY URL, not as a boolean that an effect resets when `src` changes. + // That distinction is visible: this component is recycled across items in the virtualized feed, so + // one avatar's 429 used to paint the fallback initial over the NEXT item's perfectly good image + // for a frame — React committed the render with the new `src` and the stale `failed`, and only the + // post-paint effect cleared it. Derived state cannot be stale, so there is nothing to reset. + const [failedSrc, setFailedSrc] = useState(null); + const failed = !!src && failedSrc === src; if (src && !failed) { return ( @@ -30,7 +35,7 @@ export default function Avatar({ alt={alt} loading="lazy" referrerPolicy="no-referrer" - onError={() => setFailed(true)} + onError={() => setFailedSrc(src)} className={className} /> ); diff --git a/frontend/src/components/ui/BackToTop.tsx b/frontend/src/components/ui/BackToTop.tsx index ec9ff758..51399a3a 100644 --- a/frontend/src/components/ui/BackToTop.tsx +++ b/frontend/src/components/ui/BackToTop.tsx @@ -36,6 +36,7 @@ export default function BackToTop({ dep, threshold = 600 }: { dep?: unknown; thr useEffect(() => { const el = document.querySelector("main"); scrollerRef.current = el; + // eslint-disable-next-line react-hooks/set-state-in-effect -- reading the DOM is the point —
is swapped by the page switch above us, and only after that commit can its scrollTop be read at all. Nothing derivable in render knows where the new page is scrolled to. setShow(!!el && el.scrollTop > threshold); }, [dep, threshold]); diff --git a/frontend/src/components/ui/Combobox.dom.test.tsx b/frontend/src/components/ui/Combobox.dom.test.tsx index 1feb7335..fdc55403 100644 --- a/frontend/src/components/ui/Combobox.dom.test.tsx +++ b/frontend/src/components/ui/Combobox.dom.test.tsx @@ -246,6 +246,54 @@ describe("Combobox", () => { expect(onChange).toHaveBeenCalledWith(["Action"]); // replaced, not appended expect(screen.getByRole("combobox").getAttribute("aria-expanded")).toBe("false"); // closed }); + + // The typed query is discarded when the list closes, and it happens IN the close — the clear used + // to be an effect on `open`, which fired a render later and, in controlled mode, reported the + // empty query outside any event (E27 S2). The list is for "find and pick": a leftover filter would + // hide everything worth seeing the next time it opens. + it("discards the typed query when the list closes, so the next open shows everything", () => { + wrap(); + const input = screen.getByRole("combobox"); + fireEvent.focus(input); + fireEvent.change(input, { target: { value: "com" } }); + expect(input).toHaveProperty("value", "com"); + expect(screen.queryByText("Drama")).toBeNull(); // filtered away + + fireEvent.keyDown(input, { key: "Escape" }); + expect(input.getAttribute("aria-expanded")).toBe("false"); + expect(input).toHaveProperty("value", ""); + + fireEvent.focus(input); + expect(screen.getByText("Drama")).toBeTruthy(); // the whole list is back + }); + + it("does not report a query change until something actually changes it", () => { + // A clear driven by an effect on `open` fires at MOUNT too — `open` starts false — so a + // controlled caller was told its own initial query had been cleared before the user touched + // anything. Driving it from the close event instead means no event, no callback. + const onQueryChange = vi.fn(); + const onOpenChange = vi.fn(); + wrap( + , + ); + expect(onQueryChange).not.toHaveBeenCalled(); + + fireEvent.focus(screen.getByRole("combobox")); + expect(onQueryChange).not.toHaveBeenCalled(); + fireEvent.click(screen.getByText("Action")); + expect(onOpenChange).toHaveBeenLastCalledWith(false); + expect(onQueryChange.mock.calls).toEqual([[""]]); + }); }); // PageUp/PageDown (E11, owner request): the sidebar's channel filter and the Plex pickers run to diff --git a/frontend/src/components/ui/Combobox.tsx b/frontend/src/components/ui/Combobox.tsx index 7fb775a2..6d314796 100644 --- a/frontend/src/components/ui/Combobox.tsx +++ b/frontend/src/components/ui/Combobox.tsx @@ -106,6 +106,13 @@ export default function Combobox({ const setOpenAnd = (next: boolean) => { setOpen(next); onOpenChange?.(next); + // Closing discards the typed query: the list is for "find and pick", and a leftover filter would + // hide everything worth seeing on the next open. It belongs HERE, not in an effect on `open`, + // because this is the single funnel every close goes through — floating-ui's dismiss, Escape and + // outside-click all arrive as onOpenChange, and the single-select commit calls it directly + // (verified: `setOpen` is called nowhere else). So the clear lands in the same commit as the + // close, and the controlled caller's onQueryChange fires once, from an event rather than a render. + if (!next) setQuery(""); }; const pop = useAnchoredPopover({ open, onOpenChange: setOpenAnd, mode: "combobox" }); // The input is the combobox reference for floating-ui AND our own focus target (the field's @@ -127,12 +134,6 @@ export default function Combobox({ // would make Enter a no-op over a highlighted row. const { activeIndex, highlightIndex } = pop; - // Clear the typed query when the list closes. - useEffect(() => { - if (!open) setQuery(""); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [open]); - const filtered = useMemo(() => { if (!clientFilter) return options; const q = query.trim().toLowerCase(); diff --git a/frontend/src/components/ui/CommandPalette.tsx b/frontend/src/components/ui/CommandPalette.tsx index d21fddbc..b661d342 100644 --- a/frontend/src/components/ui/CommandPalette.tsx +++ b/frontend/src/components/ui/CommandPalette.tsx @@ -81,6 +81,15 @@ export default function CommandPalette({ onClose: () => void; groups: CommandGroup[]; }) { + // Closed means UNMOUNTED, not merely hidden, and that one line replaces the effect that used to + // blank the query and the picked row on every open: fresh state is what mounting already means. + // It also stops the ranking memo below — which folds and scores every channel, playlist and saved + // view — from re-running on every parent render while the palette is shut. + if (!open) return null; + return ; +} + +function CommandPaletteBody({ onClose, groups }: { onClose: () => void; groups: CommandGroup[] }) { const { t } = useTranslation(); const listId = useId(); const groupId = useId(); @@ -94,15 +103,6 @@ export default function CommandPalette({ const activeRef = useRef(null); const listRef = useRef(null); - // A fresh open starts from a blank field: the palette is for "take me somewhere", and a leftover - // query from last time hides everything you might want now. - useEffect(() => { - if (open) { - setQuery(""); - setPicked(null); - } - }, [open]); - // The GROUP order stays fixed — modules, channels, playlists, views, actions read the same every // time — and the rows are ranked inside each group. The best row overall starts highlighted, so // Enter takes the best match even when it sits in a later group. @@ -144,6 +144,7 @@ export default function CommandPalette({ const [announced, setAnnounced] = useState(""); useEffect(() => { if (!summary) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- the announcement is DEBOUNCED on purpose (ANNOUNCE_DELAY_MS above): a polite region rewritten per keystroke queues one utterance per letter. A timer's result cannot be derived in render, and the empty branch is the same timer's cancel. setAnnounced(""); return; } @@ -151,8 +152,6 @@ export default function CommandPalette({ return () => window.clearTimeout(timer); }, [summary]); - if (!open) return null; - // Close, wait for this dialog's own Back-to-close entry to be reclaimed, THEN run: most commands // navigate, and a pushState issued before that reclaim is undone by it. Only THIS dialog closes — // a command that navigates closes whatever is underneath itself, one that does not (a theme) must diff --git a/frontend/src/components/ui/DataTable.tsx b/frontend/src/components/ui/DataTable.tsx index d13e2ba3..c84b246e 100644 --- a/frontend/src/components/ui/DataTable.tsx +++ b/frontend/src/components/ui/DataTable.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState, type ReactNode } from "react"; +import { useCallback, useEffect, useRef, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import { sortRows, cycleSort, type SortState } from "../../lib/columnSort"; import Pager from "./Pager"; @@ -149,10 +149,19 @@ export default function DataTable({ // `filters` as a dep (which would re-run them on every filter change). const filtersRef = useRef(filters); filtersRef.current = filters; - const commitFilters = (next: FilterMap) => { - if (controlledFilters) onFiltersChange!(next); - else setInternalFilters(next); - }; + // …and the same trick for the CALLBACK, one line further: a controlled caller almost always + // passes an inline arrow, so `onFiltersChange` is a fresh function on every render. Reading it + // through a ref is what lets `commitFilters` be stable, and a stable commitFilters is what lets + // the two effects below NAME it in their dep lists instead of switching the check off. + const onFiltersChangeRef = useRef(onFiltersChange); + onFiltersChangeRef.current = onFiltersChange; + const commitFilters = useCallback( + (next: FilterMap) => { + if (controlledFilters) onFiltersChangeRef.current!(next); + else setInternalFilters(next); + }, + [controlledFilters], + ); // Persist page/size always, but only the sort/filter slices this table actually OWNS — a controlled // caller persists those itself, so writing them here would just leave a stale, never-read duplicate. @@ -169,12 +178,16 @@ export default function DataTable({ ); }, [persistKey, controlledSort, controlledFilters, sort, filters, page, size]); + // Destructured so the deps are the values the body actually reads. `externalFilter` itself is an + // object literal at every call site, so depending on it would re-apply the filter — and reset the + // page — on every parent render. + const externalKey = externalFilter?.key; + const externalValue = externalFilter?.value; useEffect(() => { - if (!externalFilter) return; - commitFilters({ ...filtersRef.current, [externalFilter.key]: externalFilter.value }); + if (externalKey === undefined || externalValue === undefined) return; + commitFilters({ ...filtersRef.current, [externalKey]: externalValue }); setPage(0); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [externalFilter?.key, externalFilter?.value]); + }, [externalKey, externalValue, commitFilters]); // Clear column filters when the reset token advances. The ref starts at 0 (not at the prop) // so a navigation that bumped the token still clears even when it remounts this table fresh @@ -186,8 +199,7 @@ export default function DataTable({ resetRef.current = resetFiltersToken; commitFilters({}); setPage(0); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [resetFiltersToken]); + }, [resetFiltersToken, commitFilters]); const sorted = sortRows(filterRows(rows, columns, filters), columns, sort); diff --git a/frontend/src/components/ui/Pager.dom.test.tsx b/frontend/src/components/ui/Pager.dom.test.tsx index 2f187cbb..92f9d550 100644 --- a/frontend/src/components/ui/Pager.dom.test.tsx +++ b/frontend/src/components/ui/Pager.dom.test.tsx @@ -1,4 +1,5 @@ // @vitest-environment jsdom +import { useLayoutEffect } from "react"; import { describe, it, expect, vi, afterEach } from "vitest"; import { render, screen, fireEvent, cleanup } from "@testing-library/react"; @@ -32,3 +33,80 @@ describe("Pager · rows per page", () => { expect(onSizeChange.mock.calls).toEqual([[50], [0]]); }); }); + +// The page box DERIVES the current page (E27 S2): it holds a draft only while being edited, so the +// parent moving the page needs no re-sync effect. The layout-phase probe is what proves "derived" +// rather than "re-synced after paint" — a passive effect could not have run yet when it reads. +describe("Pager · the page box follows the page without an effect", () => { + function Probe({ page, painted }: { page: number; painted: string[] }) { + useLayoutEffect(() => { + painted.push( + (document.querySelector('input[type="number"]') as HTMLInputElement | null)?.value ?? "", + ); + }); + return ( + {}} + onSizeChange={() => {}} + sizeLabel="Rows per page" + /> + ); + } + + it("shows the new page in the first frame after the parent moves it", () => { + const painted: string[] = []; + const { rerender } = render(); + expect(screen.getByLabelText("datatable.pager.pageLabel")).toHaveProperty("value", "1"); + + painted.length = 0; + rerender(); + expect(painted[0]).toBe("5"); + }); + + it("keeps what you type until you commit it, then follows the page again", () => { + const onPageChange = vi.fn(); + render( + {}} + sizeLabel="Rows per page" + />, + ); + const box = screen.getByLabelText("datatable.pager.pageLabel"); + fireEvent.change(box, { target: { value: "7" } }); + expect(box).toHaveProperty("value", "7"); + fireEvent.keyDown(box, { key: "Enter" }); + expect(onPageChange).toHaveBeenCalledWith(6); + // The parent in this test never moves, so dropping the draft must put page 1 back — proof the + // box stopped holding its own copy rather than merely being overwritten. + expect(box).toHaveProperty("value", "1"); + }); + + it("discards an unparseable entry instead of committing it", () => { + const onPageChange = vi.fn(); + render( + {}} + sizeLabel="Rows per page" + />, + ); + const box = screen.getByLabelText("datatable.pager.pageLabel"); + fireEvent.change(box, { target: { value: "abc" } }); + fireEvent.blur(box); + expect(onPageChange).not.toHaveBeenCalled(); + expect(box).toHaveProperty("value", "3"); + }); +}); diff --git a/frontend/src/components/ui/Pager.tsx b/frontend/src/components/ui/Pager.tsx index c7bbaa47..2623ad1c 100644 --- a/frontend/src/components/ui/Pager.tsx +++ b/frontend/src/components/ui/Pager.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from "react"; +import { useState } from "react"; import { useTranslation } from "react-i18next"; import { ChevronLeft, ChevronRight } from "lucide-react"; import { hasPagerContent, pageCount } from "./pagerContent"; @@ -34,9 +34,13 @@ export default function Pager({ const totalPages = pageCount(total, size); const safePage = Math.min(page, totalPages - 1); - // Local text for the editable "jump to page" box (committed on Enter/blur). - const [pageInput, setPageInput] = useState(String(safePage + 1)); - useEffect(() => setPageInput(String(safePage + 1)), [safePage]); + // Local text for the editable "jump to page" box (committed on Enter/blur). `null` means "not + // being edited", so the box simply DERIVES the current page instead of holding a copy that an + // effect has to re-sync after every page change — one render instead of two, and no window in + // which the box shows the page you just left. The draft is dropped on commit, which is the only + // place the two can disagree: clicking any other pager control blurs the input first. + const [draft, setDraft] = useState(null); + const pageInput = draft ?? String(safePage + 1); // Turning the page — or resizing it, which re-slices from a different first item — swaps the whole // list, so put the reader at its top; landing mid-list in a page you haven't seen is disorienting. @@ -51,8 +55,10 @@ export default function Pager({ } function commitPageInput() { const n = parseInt(pageInput, 10); + // Dropping the draft either way is what makes the box follow the real page again — on a valid + // jump because the page moves to it, on garbage because there is nothing to commit. + setDraft(null); if (!isNaN(n)) goToPage(Math.min(totalPages - 1, Math.max(0, n - 1))); - else setPageInput(String(safePage + 1)); } if (!hasPagerContent(total, size)) return null; @@ -75,7 +81,7 @@ export default function Pager({ min={1} max={totalPages} value={pageInput} - onChange={(e) => setPageInput(e.target.value)} + onChange={(e) => setDraft(e.target.value)} onBlur={commitPageInput} onKeyDown={(e) => e.key === "Enter" && commitPageInput()} aria-label={t("datatable.pager.pageLabel")} diff --git a/frontend/src/components/ui/Toaster.tsx b/frontend/src/components/ui/Toaster.tsx index 5f52dedf..263cbbf3 100644 --- a/frontend/src/components/ui/Toaster.tsx +++ b/frontend/src/components/ui/Toaster.tsx @@ -89,10 +89,12 @@ export default function Toaster({ // AFTER the snapshot above has read the outgoing host's screen — this host's own first render as // top would otherwise overwrite the handover with the toasts it is about to classify. Writing the // same value twice is harmless; that is the point of the ref. + // eslint-disable-next-line react-hooks/globals -- the handover between toast hosts is module-level BY DESIGN and must be written during render — see the 12 lines above, which record why a useMemo produced the wrong answer under StrictMode. The write is idempotent, which is what makes a repeated render invocation safe. if (isTop) lastTopIds = toasts.map((n) => n.id); const [armed, setArmed] = useState(false); useEffect(() => { if (!isTop) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- arming is a TIMER, not derived state: the live region must be mounted and empty for a tick before content lands in it or a screen reader never announces the first toast. Deriving `isTop && armed` would let a host that regains top skip that tick. setArmed(false); return; } @@ -196,6 +198,10 @@ export default function Toaster({
{isTop && Docked && (
+ {/* lib/toastDock is a one-slot registry: this identity changes only when a surface docks + or undocks, and remounting is then the correct behaviour, not the hazard the rule + warns about. */} + {/* eslint-disable-next-line react-hooks/static-components */}
)} diff --git a/frontend/src/components/ui/anchoredPopoverHooks.ts b/frontend/src/components/ui/anchoredPopoverHooks.ts index f8dc7692..0b81da66 100644 --- a/frontend/src/components/ui/anchoredPopoverHooks.ts +++ b/frontend/src/components/ui/anchoredPopoverHooks.ts @@ -143,6 +143,7 @@ export function useAnchoredPopover(opts: AnchoredPopoverOptions) { // menu opens; clear it whenever the surface closes. useEffect(() => { if (!opts.open) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- both indices are also floating-ui's list-navigation state (passed to useListNavigation below), so they cannot be derived from `open` without lying to the library about where the cursor is. Clearing on close is what stops a stale row stealing the next open's initial focus. setActiveIndex(null); setHoverIndex(null); } diff --git a/frontend/src/components/ui/sortableA11y.ts b/frontend/src/components/ui/sortableA11y.ts index cf8b817b..3c4dd5bc 100644 --- a/frontend/src/components/ui/sortableA11y.ts +++ b/frontend/src/components/ui/sortableA11y.ts @@ -61,6 +61,11 @@ export function useSortableA11y(describe: (id: UniqueIdentifier) => SortableSubj }, onDragOver: ({ active, over }) => { if (!over || (!moved && over.id === active.id)) return undefined; + // `moved` is per-DRAG state, and the announcement closures capturing it is the point: + // dnd-kit reports the item over ITSELF in the pick-up tick (see above), and this flag is + // the only thing separating that from a real move. A ref would be the wrong lifetime — + // one drag, not the component. + // eslint-disable-next-line react-hooks/immutability moved = true; return t("common.dnd.movedOver", landing(active.id, over.id)); }, From 6a7ff330f6f16f08374949a100991be0c449b2b0 Mon Sep 17 00:00:00 2001 From: npeter83 Date: Mon, 21 Sep 2026 00:07:12 +0200 Subject: [PATCH 07/21] refactor(frontend): the shared hooks in lib/ stop lying about their deps (E27 S2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten more suppressions, and React 19 turns out to supply the missing primitive. useEffectEvent (stable since React 19.2, which S1 brought in) is the canonical answer to the "bind the listener once, but read the latest values" pattern these hooks were disabling the checker for. Three of playerModalHooks' four escape hatches become honest dep lists: the wheel-volume handler, the native-controls handover, and the fullscreen-change sync. The contract is no longer prose — the compiler enforces it. The fourth stays, deliberately, and now says why at the site: the YouTube player's lifecycle effect must run once per video (re-running destroys and rebuilds the iframe), and YT.Player takes its callbacks as an object literal, which an effect event may not be passed into. Fixed, not suppressed: - useDismiss captured onClose when the popover opened, so a caller that re-rendered with a different handler while it stayed open kept getting the OLD one called. The disabled dep list asserted the opposite ("read fresh from this run's closure"). Both the handler and the ref list are read through refs now; three tests, the two that matter mutation-checked. - usePref wrapped its seed in a useCallback whose dep list had to be excused. useState calls a lazy initializer exactly once, so the memo was saving nothing; inlined. - useCardPager clamped an out-of-range page from an effect. Adjusted during render instead — React discards that pass rather than committing it, so no out-of-range page is ever painted. - plexPlayerHooks' auto-skip countdown reads its marker from the ref that already existed, so the dep list names its real inputs. That also made the set-state-in-effect finding go away: the progress is genuinely timer-driven, and the checker can now see that. - useUrlMirror and useChipOverflowFit only needed their stable refs listed. Two test-harness suppressions (a render-phase probe ref, a module-level setState handle) moved inline with the reason they exist: both exist to observe a hook from outside without changing what is being measured. Suppressions 99 -> 89; exhaustive-deps 35 -> 27. --- frontend/eslint-suppressions.json | 20 ----- frontend/src/lib/playerModalHooks.ts | 72 +++++++++------- .../lib/plexPlayerHooks.subtitle.dom.test.tsx | 1 + frontend/src/lib/plexPlayerHooks.ts | 14 ++- frontend/src/lib/useCardPager.ts | 12 +-- frontend/src/lib/useChipOverflowFit.ts | 5 +- frontend/src/lib/useDismiss.dom.test.tsx | 85 +++++++++++++++++++ frontend/src/lib/useDismiss.ts | 22 +++-- .../lib/useFocusKeeper.restore.dom.test.tsx | 1 + frontend/src/lib/usePref.ts | 12 +-- frontend/src/lib/useUrlMirror.ts | 10 ++- 11 files changed, 175 insertions(+), 79 deletions(-) create mode 100644 frontend/src/lib/useDismiss.dom.test.tsx diff --git a/frontend/eslint-suppressions.json b/frontend/eslint-suppressions.json index c714654c..594be87a 100644 --- a/frontend/eslint-suppressions.json +++ b/frontend/eslint-suppressions.json @@ -183,25 +183,5 @@ "react-hooks/set-state-in-effect": { "count": 1 } - }, - "src/lib/plexPlayerHooks.subtitle.dom.test.tsx": { - "react-hooks/immutability": { - "count": 1 - } - }, - "src/lib/plexPlayerHooks.ts": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/lib/useCardPager.ts": { - "react-hooks/set-state-in-effect": { - "count": 1 - } - }, - "src/lib/useFocusKeeper.restore.dom.test.tsx": { - "react-hooks/globals": { - "count": 1 - } } } diff --git a/frontend/src/lib/playerModalHooks.ts b/frontend/src/lib/playerModalHooks.ts index b81e656c..ec2f52e9 100644 --- a/frontend/src/lib/playerModalHooks.ts +++ b/frontend/src/lib/playerModalHooks.ts @@ -1,5 +1,6 @@ import { useEffect, + useEffectEvent, useRef, useState, type Dispatch, @@ -133,20 +134,20 @@ export function useYtVolume({ // Mouse-wheel volume over the WHOLE modal. Non-passive so preventDefault stops the modal/page // scrolling. The cross-origin iframe swallows wheel over its native strips, but the central // interaction overlay is our element (wheel there bubbles here), and everything outside works. + const onWheelSpin = useEffectEvent((e: WheelEvent) => { + // Ctrl/⌘+wheel is the browser's zoom gesture — never swallow it. + if (e.ctrlKey || e.metaKey) return; + e.preventDefault(); + focusModal(); // re-grab focus if it had slipped into the native controls + nudgeVolume(e.deltaY < 0 ? 5 : -5); + }); useEffect(() => { const el = dialogRef.current; if (!el) return; - const onWheel = (e: WheelEvent) => { - // Ctrl/⌘+wheel is the browser's zoom gesture — never swallow it. - if (e.ctrlKey || e.metaKey) return; - e.preventDefault(); - focusModal(); // re-grab focus if it had slipped into the native controls - nudgeVolume(e.deltaY < 0 ? 5 : -5); - }; + const onWheel = (e: WheelEvent) => onWheelSpin(e); el.addEventListener("wheel", onWheel, { passive: false }); return () => el.removeEventListener("wheel", onWheel); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [dialogRef]); // On unmount, clear the flash timer and flush any pending debounced write — closing the player right // after a wheel spin (the normal way to leave) must not drop the level the user just set. @@ -183,18 +184,19 @@ export function useNativeControlsYield({ setNativeControls: (on: boolean) => void; } { const [nativeControls, setNativeControls] = useState(false); + const handOverToIframe = useEffectEvent(() => { + const p = playerRef.current; + const f = p && typeof p.getIframe === "function" ? p.getIframe() : null; + if (f && document.activeElement === f) setNativeControls(true); + }); + const rearmOurControls = useEffectEvent(() => { + setNativeControls(false); + focusModal(); + }); useEffect(() => { - const iframeFocused = () => { - const p = playerRef.current; - const f = p && typeof p.getIframe === "function" ? p.getIframe() : null; - return !!f && document.activeElement === f; - }; // activeElement settles on the tick after blur, so defer the check. - const onBlur = () => window.setTimeout(() => iframeFocused() && setNativeControls(true), 0); - const rearm = () => { - setNativeControls(false); - focusModal(); - }; + const onBlur = () => window.setTimeout(() => handOverToIframe(), 0); + const rearm = () => rearmOurControls(); const stage = stageRef.current; window.addEventListener("blur", onBlur); window.addEventListener("focus", rearm); @@ -204,8 +206,7 @@ export function useNativeControlsYield({ window.removeEventListener("focus", rearm); stage?.removeEventListener("mouseleave", rearm); }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [stageRef]); return { nativeControls, setNativeControls }; } @@ -264,19 +265,17 @@ export function useYtBigView({ // YouTube's own fullscreen — entered from its control bar, left by its button or Esc. On LEAVING, // focus is stuck in the iframe (the user had to click into the player to reach YT's button), so our // window-level shortcuts never fire — take focus back so F/Space/arrows work the moment fullscreen ends. + const syncFullscreen = useEffectEvent(() => { + const fsEl = fullscreenElement(); + const p = playerRef.current; + const frame = p && typeof p.getIframe === "function" ? p.getIframe() : null; + const ours = !!fsEl && (fsEl === stageRef.current || fsEl === frame); + isFullscreenRef.current = ours; // read by the once-bound keydown handler + setIsFullscreen(ours); // drives the overlay's pointer-events + if (!fsEl) focusModal(); + }); useEffect(() => { - const onChange = () => { - const fsEl = fullscreenElement(); - // Refs only — this listener is bound once, so it must not close over render values. - const p = playerRef.current; - const frame = p && typeof p.getIframe === "function" ? p.getIframe() : null; - const ours = !!fsEl && (fsEl === stageRef.current || fsEl === frame); - isFullscreenRef.current = ours; // read by the once-bound keydown handler - setIsFullscreen(ours); // drives the overlay's pointer-events - if (!fsEl) focusModal(); - }; - return onFullscreenChange(onChange); // both spellings — see lib/fullscreen - // eslint-disable-next-line react-hooks/exhaustive-deps + return onFullscreenChange(() => syncFullscreen()); // both spellings — see lib/fullscreen }, []); return { @@ -485,6 +484,13 @@ export function useYtProgress({ } playerRef.current = null; }; + // ONE PLAYER PER VIDEO is the invariant, and it is why this dep list is deliberately just the + // id: re-running would destroy and rebuild the YouTube iframe, losing the playing position and + // every listener attached to it. Nothing in the body reads a render value — the mutable state it + // needs mid-session (currentIdRef, modeRef, navRef, sessionPosRef, autoMarkedRef, + // errorStreakRef, playerRef) is all behind refs, which is the enforcement, not this comment. + // useEffectEvent cannot help here the way it does above: YT.Player takes its callbacks as an + // object literal, and an effect event may not be passed around as a value. // eslint-disable-next-line react-hooks/exhaustive-deps }, [activeId]); } diff --git a/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx b/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx index a56c0cdf..eb8b6017 100644 --- a/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx +++ b/frontend/src/lib/plexPlayerHooks.subtitle.dom.test.tsx @@ -330,6 +330,7 @@ function TrackHarness() { absRef: { current: 3600 }, loadStartRef: { current: 0 }, }); + // eslint-disable-next-line react-hooks/immutability -- a TEST probe: the hook's return has to reach the assertions, and a render-phase ref write is the only way to read it from outside without a wrapper that would change what is being measured. harness.current = tracks; return (