chore(comments): drop epic, sprint and review-round ids from the code
The code cited its plan instead of saying why: `E28 review J4`, `(R6 S3b)`, `S1 justified…`,
`deep-review round 5`. That plan lives in the private ops repo, so for every reader of this one the
citation was noise, and a sentence whose subject was the plan step ("round 2 showed…") hid its
lesson behind an id. Each one is gone and the "why" kept: a bare citation is deleted, a sentence
carrying a lesson is reworded around it ("a later attempt showed…").
Measured (`scripts/audit_metrics.py`, product code): 652 lines in 270 files carried an epic code by
the definition the next commit holds (507 / 241 by the old, two-digit-only one) -> 0. Across all
code, tests, scripts and configs: ~1100 epic-code lines plus ~320 sprint/round ids.
Behaviour unchanged, checked mechanically: every changed .py file has the same AST once docstrings
are blanked; css/sh/toml/yml/json files are identical once comments are stripped; the only
non-comment changes in TS/JS are test titles, one description string in a contract table, and five
ESLint messages, whose text is all the gates snapshot diff holds. Not touched: Markdown (docs and
the changelog keep their history), Alembic migrations, TV episode notation (`S2 · E5`), DESIGN.md
ids (`C1`, `B4`, `S3h`), tracker ids (`SEC-16`, `DX-17`).
One stale claim fixed on the way: test_hls_reaper's docstring said the player checkpoints "every
10 s whether or not it is playing", which plex/stream.py calls wrong; it now says what the code does.
This commit is contained in:
@@ -209,12 +209,12 @@ def test_the_default_ceiling_clears_the_largest_body_a_default_route_takes():
|
||||
# its route is not in ROUTE_MAX_BODY_BYTES, this middleware refuses a request the route would have
|
||||
# taken — silently, in production, on a path nobody tested. The point of the list is the third entry.
|
||||
#
|
||||
# BOTH helpers, not just `capped_body`: P1 added `capped_stream_to` as its sibling and the registry
|
||||
# BOTH helpers, not just `capped_body`: `capped_stream_to` was added later and the registry
|
||||
# did not follow, so the upload route — the one this whole invariant was written for — was the one
|
||||
# site the drift guard could not see (deep-review round 1). The names come from `_common.py` itself
|
||||
# site the drift guard could not see. The names come from `_common.py` itself
|
||||
# below, so a THIRD sibling cannot be added without this list noticing either — and a route that
|
||||
# skips the helpers altogether and awaits `request.body()` lands here as `<no ceiling declared>`
|
||||
# rather than vanishing (round 3).
|
||||
# rather than vanishing.
|
||||
#
|
||||
# The module is the path relative to `app/routes/`, so a site under `plex/` is spelled
|
||||
# `plex/admin.py` and cannot collide with the top-level `admin.py`.
|
||||
@@ -255,7 +255,7 @@ def test_the_registry_covers_every_body_reading_helper():
|
||||
# they are blind to the shape `_common.py` itself is written in. A route spelling `await
|
||||
# request.body()` inline declares no ceiling, hands no connection back, and would be invisible to
|
||||
# all three — reading as "no new sites" rather than as a failure, which is the exact way a drift
|
||||
# guard dies quietly (deep-review round 3).
|
||||
# guard dies quietly.
|
||||
RAW_BODY_READS = {"body", "stream", "form", "json"}
|
||||
|
||||
|
||||
@@ -476,7 +476,7 @@ def test_the_upload_ceiling_and_the_admin_knob_cannot_drift_apart():
|
||||
constant. If the knob could be set above the edge, an admin raising it would silently start
|
||||
getting 413s from a layer the admin page never mentions.
|
||||
|
||||
Since P1 the two count the SAME bytes — the body is the file, with no multipart envelope
|
||||
Now the two count the SAME bytes — the body is the file, with no multipart envelope
|
||||
wrapped round it — so the old allowance is gone and the edge is the knob's own maximum,
|
||||
exactly. A gap reappearing means someone re-wrapped the upload in an envelope without saying so.
|
||||
|
||||
@@ -509,13 +509,13 @@ def _route_objects(root):
|
||||
|
||||
@pytest.mark.parametrize(("method", "template"), sorted(bodylimit.ROUTE_MAX_BODY_BYTES))
|
||||
def test_no_route_with_a_raised_ceiling_declares_a_body_parameter(method, template):
|
||||
"""P1's invariant, and the reason this is a test rather than a comment.
|
||||
"""The upload fix's invariant, and the reason this is a test rather than a comment.
|
||||
|
||||
A raised ceiling is only honest when the route's own cap applies to the bytes AS THEY ARRIVE.
|
||||
FastAPI reads and spools a DECLARED body (`= File(...)`, `= Form(...)`, a Pydantic model)
|
||||
BEFORE the first dependency resolves — so on such a route this middleware's ceiling is the only
|
||||
thing between an anonymous caller and the whole allowance, the auth dependency never gets a say, and
|
||||
the route's in-handler cap measures bytes that have already landed. That is exactly what P1
|
||||
the route's in-handler cap measures bytes that have already landed. That is exactly what a fix
|
||||
closed on the upload route, which sat in `ROUTE_MAX_BODY_BYTES` with a 500 MiB ceiling and a
|
||||
`file: UploadFile = File(...)` body for one release.
|
||||
|
||||
@@ -529,7 +529,7 @@ def test_no_route_with_a_raised_ceiling_declares_a_body_parameter(method, templa
|
||||
itself consults when it decides to read the body. Measured on the pinned FastAPI: a body declared
|
||||
inside a sub-dependency leaves `body_params` EMPTY while the body is still read in full before
|
||||
auth, so the first version of this test was green over a reproduction of the very defect it
|
||||
exists to forbid (deep-review round 1). The upload route's own idiom — a body read from a
|
||||
exists to forbid. The upload route's own idiom — a body read from a
|
||||
`yield` dependency — is exactly the shape that would have slipped through."""
|
||||
from app.main import app
|
||||
|
||||
@@ -546,7 +546,7 @@ def test_no_route_with_a_raised_ceiling_declares_a_body_parameter(method, templa
|
||||
|
||||
def test_a_stored_override_outside_the_bounds_falls_back_on_READ(db):
|
||||
"""`set_value` refuses an out-of-range int, but neither bound has always been what it is now —
|
||||
the upload knob had no `max` before SEC-20 and its `min` was 0 before E7 — so a value that was
|
||||
the upload knob had no `max` before SEC-20 and its `min` used to be 0 — so a value that was
|
||||
legal when it was written can be sitting in the database. Served as-is it is exactly the drift
|
||||
the test above forbids: the route would accept bytes the edge refuses.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user