Skip to content

refactor: quality pass over the render path - #10

Merged
pingviini merged 19 commits into
mainfrom
refactor/render-path-quality-pass
Sep 16, 2026
Merged

pingviini merged 19 commits into
mainfrom
refactor/render-path-quality-pass

Conversation

@pingviini

Copy link
Copy Markdown
Collaborator

A /simplify pass over the whole repository: 19 commits removing duplication
and misplaced logic, with no intended change to what a caller receives except
where called out below. Each commit stands alone and explains its own
reasoning; this is the map.

What it changes

The render path. One _OutputPlan now drives compile and delivery, which
retired three duplications: a string test on the output filename standing in
for the plan's archive flag, an optional output argument that existed only
for test call sites, and a second directory scan that stat'd every page file
again. _finalise_output and _finalise_archive had grown a shared tail that
had already drifted, and now delegate to one. The prelude walk and the project
teardown moved off the event loop.

The output taxonomy. It was spelled three times — OUTPUT_MODELS,
FORMAT_MAP, and an isinstance ladder against concrete image classes in
three places. Each model now declares its own extension, media type and typst
flags; _ImageOutputBase became the public ImageOutput, so "is this an
image" is one check against the class that actually declares archive.

The published documents. /v1/constraints was written out twice: eleven
Pydantic models declared its shape and two modules built the same shape as
dict[str, Any] literals, meeting only at runtime, so the type checker saw
nothing. The builders now construct their models.

Error and metric handling. unsupported_format was recovered by
pattern-matching Pydantic's internals, including the quotes it puts around a
discriminator name; the rule now lives on the output field. render_total
and render_errors_total were emitted in five arms that had drifted into
three label vocabularies, and are emitted once.

Middleware. Both layers are plain ASGI rather than BaseHTTPMiddleware,
which wrapped every request in an anyio task group and a pair of memory object
streams. Measured on the real app: /health 591us -> 249us, /v1/constraints
874us -> 576us.

Tests. 28 hand-written fake_run closures became one fake_typst
fixture; the client, token, configured-app dance and render fakes moved into
conftest. test_renderer.py lost 194 lines. Coverage went up where the
branch created new obligations: temporary-project cleanup (previously
TemporaryDirectory's job and untested), the key-count cap at the route, and
seven conformance vectors pinning the published rule-evaluation order.

CI. ruff format --check is now a gate, and ty checks scripts as well
as app. Both gaps were found by this work: three files kept coming back
reformatted and had to be reverted out of each commit.

Breaking changes

Two, both in metrics, neither in the HTTP contract:

  • prelum_render_errors_total{error_type} carries the published AppError
    code instead of the Python class name, so a dashboard survives a class
    being renamed — which the API contract says is free. Existing queries need
    updating once.
  • prelum_render_timeouts_total is removed. It was incremented at the same
    instant as render_errors_total{error_type="timeout"} and
    render_total{status="timeout"} and carried nothing they do not.

Caller-visible, but not contract

An unsupported format keeps its 400, its unsupported_format code and a
context with no rule. What changes is the prose in detail and the
errors[].type, which is now ours rather than Pydantic's. A files mapping over
the configured cap gets the same 400 and the same context, but is refused
before a render permit is taken, so it no longer counts in render_total —
consistent with every other validation-time caller fault.

Published artefacts

Regenerated and diffed against main: openapi.json, files-key-rules.json
and error-codes.json are byte-identical. output-rules.json grows by the
seven ordering vectors and nothing else — rules, rule_evaluation and
output_rules_version are unchanged, because no rule changed.

Deployment

No configuration, dependency or container-image change. The memory posture is
deliberately unchanged: an archive is still built on disk rather than in a
BytesIO, because holding the buffer and its copy would be two copies of
something that may be max_output_bytes, with max_concurrent_renders in
flight, against a config docstring that documents worst-case memory as the one
multiplied by the other.

Verification

make checks (ruff, ruff format, ty) clean. 733 unit tests and 30 integration
tests against real typst pass. make docs-build --strict passes. The rule-order
vectors were verified to fail on a deliberate reordering, and the format gate to
fail on a deliberately misformatted file.

One caveat for review

CONTRIBUTING asks a pull request to remain limited to one coherent change, and
this one is not: it is a review pass touching 30 files across six themes. The
commits are separable and each is self-contained, so it may be easier to read
commit by commit — or to ask for it split, which is straightforward given the
history.

A quality pass over the render path. No caller-visible behaviour changes:
the same responses, problem codes, log events and metrics as before.

_OutputPlan now carries the output model, so _run_typst takes the layout and
the plan instead of four separate arguments. Three duplications go with it:
the plan's `archive` flag replaces the string test on the output filename,
`output: RenderOutput | None = None` with its PdfOutput() fallback existed
only for test call sites, and _run_typst returns the page files it already
collected, so the archive builder no longer scans the directory and stats
every page a second time.

_finalise_output and _finalise_archive had grown a shared tail — naming the
artefact, the optional debug copy, the render.complete event and the
RenderResult — which had already drifted, one carrying a `files` field the
other lacked. Both now delegate to _deliver.

Three pieces of work leave the event loop. The prelude walk is pure Python
and O(request size), with `data` bounded only by max_request_body_bytes, so
it moves into the worker thread that writes the bound template. The project
teardown was TemporaryDirectory.__exit__ running rmtree over the entry
point, every caller file and every page file inline, so it becomes an
explicit mkdtemp and a shielded rmtree in a thread — shielded so a cancelled
request cannot leak the tree. The archive is built in a BytesIO and streamed
page by page rather than written to disk, stat'd and read back, and the
on-disk copy is now written only when debug_output_dir asks for one.

The prlimit prefix moves to a module-level resource_limit_args(settings) that
the startup probe calls too. The probe's docstring claimed it ran the real
wrapper argv; it assembled its own copy, which would have kept passing the
boot check after the wrapper form changed while every render failed.

Also drops an unreachable branch in _sanitize_filename: `filtered` is already
stripped of leading and trailing "._-", so it cannot be all dots and the
"rendered" fallback below the empty-string guard could never run.

Adds the temporary-project cleanup tests that TemporaryDirectory never
needed, now that the teardown is ours.
tests/test_renderer.py defined `async def fake_run` 28 times, 11 of those
bodies identical, plus five near-duplicate FakeProc subclasses differing only
in a return code or a sleep. Every one re-applied the argv indices by hand, so
a change to how the renderer invokes typst meant editing 28 closures.

One `fake_typst` fixture replaces them. It varies in the three ways the tests
actually needed: what the compile writes (`content`, `pages`, or nothing),
how the process settles (`proc`, with FakeProc taking a return code or a
blocking wait), and what the test reads back. Recorded invocations carry the
argv, the process options and the project contents as a TypstCall, which
names the template and output paths so the -2/-1 convention is stated once.

The project contents are snapshotted while the call is in flight because the
renderer removes the temporary project before the test regains control; that
is the only point they can be read, and it removes the need for any per-test
inspection hook. The snapshot reads through open() rather than
Path.read_bytes: a test that forbids the renderer from reading a file patches
Path.read_bytes, and must not be tripped by bookkeeping done on its behalf.

test_page_selection_beyond_the_limit_fails_before_rendering now asserts
`typst.calls == []` instead of raising inside the fake, so a regression there
reports what happened rather than travelling through the renderer's own error
handling.

tests/test_renderer.py loses 194 lines. No test's subject or assertions
changed otherwise.
The token header and the dev default were transcribed in the HTTP test
modules — twelve literal spellings across five files, none of which imported
API_TOKEN_HEADER or DEV_DEFAULT_TOKEN. Renaming the header or rotating the
default needed a grep over files that named neither, which is the drift
API_TOKEN_HEADER's own comment warns about. conftest now builds TOKEN from
both constants, and holds the one TestClient over the module-level app.

test_routes_auth hand-rolled "setenv, cache_clear, create_app, try/finally,
cache_clear" five times, where one missed finally poisons unrelated tests.
A `configured_client(**environment)` fixture does it once and clears on
teardown. It clears all three cached dependencies, not just settings:
get_renderer and get_render_semaphore each close over the settings they were
built from, so leaving either behind hands the next test a collaborator
configured for the previous one. test_load_shedding and test_startup_checks
still have their own variants, which a later commit folds in.

`patched_render(outcome)` replaces the four hand-written
patch.object(TypstRenderer, "render", AsyncMock(...)) blocks, taking either
the result to return or the error to raise, and `fake_render_result` replaces
the `_result()` helper that two modules defined identically.

The two remaining literal header names now read API_TOKEN_HEADER: the
wrong-token test and the non-ASCII-token test still send their own values,
which is the point of both, but no longer restate the header's spelling.
test_sentry_optional keeps its literals deliberately — it parametrises the
header's casing.
… conftest

Three modules still kept their own version of the configured-app dance.
test_load_shedding had an autouse fixture clearing three caches twice over
plus a `shedding_app` that cleared two of them again by hand;
test_startup_checks cleared only config.get_settings, a fourth spelling of
the same intent. Both now use `configured_app`, which `configured_client`
is built on, so "set the environment, clear the caches, build the app" is
stated once and the teardown cannot be forgotten.

test_examples opened `with TestClient(app)` twice for the lifespan events;
`lifespan_client` does that. Its TOKEN literal and test_load_shedding's, and
test_load_shedding's copy of the `_result()` helper, come from conftest now,
which retires the last of both duplications outside test_sentry_optional —
whose literals are deliberate, since it parametrises the header's casing.

In test_inline_file_key_properties, the published rules document and its
segment character class are derived once at import instead of per generated
example. Note what this is not: measured over the module, it makes no
difference to the runtime (11.75s to 11.61s, inside the noise), because
Hypothesis's own work dominates and files_key_rules() is cheap. It is a
simplification — an immutable document rebuilt 7,000 times, and one fewer
argument threaded through the mirror — not the speedup it looked like.
The four docs/api exporters were identical apart from a builder function and
a filename: each repeated the repo root, the destination, the mkdir, and
json.dumps(indent=2, sort_keys=True) with its trailing newline. That
serialisation is contract — callers diff these files in their own CI — yet it
was written out four times, so changing the indent or the key order in one
would silently reformat one artefact and not its three siblings.

scripts/_artefacts.py now holds ROOT, API_DOCS and write_json_artefact, and
each exporter is three lines of its own.

Deliberately not collapsed into one table-driven module, which is what the
duplication first suggests: tests/test_docs.py pins each `python -m
scripts.export_*` target and each .gitignore entry as the documentation
build's interface, so the module names and Makefile targets are themselves
published. The duplication worth removing was the format, not the modules.

Verified by regenerating all four artefacts before and after: byte-identical.
…s once

Two changes to the render path's error and metric handling.

handle_validation_error recovered "the caller asked for a format we do not
have" by pattern-matching Pydantic's internals: an `enum` error located under
output.format, or a `union_tag_invalid` whose ctx names the discriminator —
compared against the literal "'format'", quotes included. A fourth format, a
renamed discriminator or a Pydantic release that reshaped either would have
degraded the answer to `invalid_request` silently, with one conformance
vector as the only thing standing in the way. RenderRequest already has a
before-validator normalising output.format; it now rejects a format
OUTPUT_MODELS does not claim, and the existing classified-error table picks
it up, so the sniffing is gone.

Which errors travel through Pydantic that way was spelled twice — a tuple in
models.py and a dict rebuilt per request in main.py. errors.py now owns both
as VALIDATION_ERRORS and VALIDATION_ERRORS_BY_CODE.

routes.render emitted render_total in five arms and render_errors_total in
three, which had drifted into three label vocabularies. Both are emitted once
in a finally from an outcome the arms settle.

BREAKING CHANGE: render_errors_total's error_type label carries the published
AppError `code` (e.g. inline_template_error, render_timeout) instead of the
Python class name, so dashboards survive a class being renamed — which the
API contract says is free. prelum_render_timeouts_total is removed: it was
incremented at the same instant as render_errors_total{error_type="timeout"}
and render_total{status="timeout"} and carried nothing they do not.

Caller-visible, but not contract: an unsupported format keeps its 400 and its
`unsupported_format` code, and its context still carries no `rule`. What
changes is the prose in `detail`, the published `errors[].type` (now
`unsupported_format` rather than Pydantic's own), and `loc`, which is now
["body"] because a model-level before-validator reports the model root — the
same shape bound_files_count already produces. The value is still quoted and
still bounded, which the two adjusted tests in test_errors_middleware pin.

Verified: all four docs/api artefacts regenerate byte-identical.
add_request_id, handle_app_error, handle_http_exception and
handle_validation_error were defined inside create_app, and were roughly a
hundred of its hundred and thirty-five lines — so what create_app actually
wires was buried in the middle of them. None of the four closes over
anything there: they reference module-level imports and constants only.

At module level they are importable, so a future test can drive one directly
instead of having to build a whole app and reach it through a client, which
is why every test of this behaviour currently goes through TestClient. They
are also no longer rebuilt on each create_app() call, of which the test suite
makes many.

Registration order is unchanged — BodyLimitMiddleware, the instrumentator,
then the request-id middleware — and @app.middleware("http") was already
add_middleware(BaseHTTPMiddleware, dispatch=...), so the request path is the
same. The exported OpenAPI schema is unchanged.
OUTPUT_MODELS carries the comment "Every OutputFormat must be claimed by
exactly one of them", but its only consumers were the two frozensets
output_rules.py publishes. The taxonomy was in fact spelled three times: that
registry, renderer.FORMAT_MAP for the extension and media type, and an
isinstance ladder against the concrete (PngOutput, SvgOutput) tuple in three
separate places deciding "is this an image". Adding a format meant editing
five sites, and forgetting one failed silently — default flags, the wrong
disposition — rather than loudly.

Each output model now declares its own extension and content type, and its
own typst flags as cli_args(). _typst_output_args and FORMAT_MAP are gone
with the ladder: PngOutput.cli_args delegates to the image base and prepends
--ppi, which is all that ever distinguished the two image arms.

_ImageOutputBase becomes the public ImageOutput, so "is this an image" is one
isinstance against one class rather than a tuple of every concrete image
format — and it is the class that actually declares `archive`, `page` and
`pages`, so the narrowing the renderer needs is the honest one.

test_every_output_format_has_render_metadata existed only because FORMAT_MAP
was a second list of formats. It is replaced by the invariant the registry
itself now has to hold: every format claimed, every model describing itself,
no two sharing an extension.

The published openapi.json and output-rules.json regenerate unchanged.
BodyLimitMiddleware subclassed BaseHTTPMiddleware and the request-id handler
was registered through it. That base wraps every request in an anyio task
group and a pair of memory object streams, and a task per layer. Measured
against a bare app answering /health in 153us, each layer cost about 190us,
so the two together more than doubled the cost of every request — for a byte
counter and a header copy. /v1/render never noticed it beside a typst fork;
the operational endpoints paid it in full.

Measured on the real app, before and after:

  /health           591us -> 249us  (-58%)
  /v1/constraints   874us -> 576us  (-34%)

The body limiter needed a different shape, and the existing tests caught why:
FastAPI wraps anything raised while it reads the body into a bare
HTTPException(400), so RequestTooLargeError never reaches the handler
registered for it. BaseHTTPMiddleware buffers the response, which is what let
the old code replace that 400 with the published 413 — the `exceeded` flag was
load-bearing, not defensive, and my first attempt at this dropped it and
answered 400. Plain ASGI cannot swap a buffered response, so it drops the
app's response messages instead: once the body is refused nothing the app
produced reaches the server, and the 413 is sent in its place. Same answer,
and it still holds if a route swallows the error.

The request-id middleware only wraps `send`. It assigns the header rather
than appending it, matching what the old `response.headers[...] = ...` did,
and writes request_id into scope["state"], which is where request.state reads
from, so a route can still reach it by that name.

The exported OpenAPI schema is unchanged.
The /v1/constraints document was written out twice. Eleven Pydantic models in
models.py declared its shape — some 150 lines of it — and constraints.py and
output_rules.py then built the same shape independently as dict[str, Any]
literals. The two met only at runtime, in
ConstraintsResponse.model_validate(build_constraints(settings)), so the type
checker saw nothing at all: a misspelt key or a wrong type surfaced only when
a test happened to serve the endpoint. That is the drift both modules' own
docstrings say they exist to prevent, reintroduced one layer up.

The builders now construct the models. files_key_rules returns FilesKeyRules,
output_rules returns OutputRulesDocument, build_constraints returns
ConstraintsResponse, and the route returns it without revalidating. Every
field name and type in the document is checked statically.

ConstraintsResponse becomes a subclass of FilesKeyRules, which is the new
model for the static half the artefact publishes on its own. build_constraints
widens that half rather than restating any of its fields. Pydantic flattens an
inherited model into one schema, so the published shape and field order are
unchanged — verified below.

The conformance-vector and rule tables stay literal dicts: they are data
tables, and that is the form both a reader and a caller's mirror want. They
are validated into their models once at import, so a wrong key or type in one
fails every deployment's boot and every test run rather than the first request
that serves it.

PublishedDocument.published() is now the one serialisation of these documents.
Its exclude_none matches response_model_exclude_none on the endpoint, so the
static artefact and the HTTP response are the same document rather than two
renderings of it — an accepted vector carries no rejection fields in either.

ConstraintsResponse's docstring is published as its schema description, so the
reason for the inheritance sits in a comment above the class instead of in it.
That is the one thing the first attempt got wrong, and the schema diff caught.

Verified: openapi.json, files-key-rules.json and output-rules.json all
regenerate byte-identical.
One published rule had two enforcement points at two depths. The structural
cap ran at the model layer, where bound_files_count's docstring explains why
late enforcement was wrong. The configurable cap ran again inside
_write_inline_files, with its own message and a hand-copied
`"rule": "too_many_keys"` context — after the semaphore had granted a permit
and a temporary project had been created.

The limit a caller actually meets is effective_max_files, the lower of the
two, and that was the one enforced deepest: an over-limit request spent a
render slot to be refused, which is capacity taken from a request that could
have been served.

ensure_key_count_fits now takes the limit. The model passes the structural
cap, which is deployment-independent and all it can know; the route passes
the effective cap before any permit is taken, because that is the
deployment's own policy and the route is where settings are. The renderer
stops checking it, so the message and the context dict exist once.

templates.effective_key_limit is the one expression for "the lower of the
two". constraints.py already computed it for publication and now reads it
from there, so the number a caller is refused by and the number published as
limits.effective_max_files cannot diverge.

The two renderer tests that drove this through TypstRenderer are replaced by
two route tests, which is the layer that now owns the rule and the layer a
caller sees. The first asserts the render mock was never awaited, which is
the property the change is for; the second pins that a mapping within the cap
still reaches the renderer.

The problem body is unchanged — same 400, same invalid_file_data, same
context — because it is the same error from the same helper. What changes is
that a request refused this way no longer counts in render_total: it is now
rejected before the metrics block, alongside every other validation-time
caller fault, instead of inside it.

All three published artefacts regenerate byte-identical.
Mapping[str, JSONValue] already covers dict[str, JSONValue], so the union
spelled the same thing twice.
…encoder

The example request's `files` entry was built by hand as {"encoding": "text",
"content": ...}, while render_examples.as_render_file decides text or base64
from the suffix and exists precisely because guessing from the bytes would
quietly corrupt an asset that happened to decode. examples/render-request.json
is a caller-facing artefact, so its encoding decision was being made by a
different rule from every other example asset's.

It could not reuse that function: run by path, `scripts/` rather than the
repository root is on sys.path, so no sibling import resolves. Every other
script is already invoked with `-m`; this one was the exception, and its
hyphenated name is what made it one. Renamed with underscores, invoked as a
module, and the encoder is now shared — so adding a binary file to the example
request needs no decision at all.

The three places that spelled the old command now name the make target, which
is what CONTRIBUTING already told contributors to run.

Verified: examples/render-request.json regenerates unchanged.
{"code", "origin", "title", "status", "detail", "instance", "context"} was
transcribed in four assertions across three files, while models.Problem
declares exactly those seven fields and exists, per its own docstring, so the
shape has one statement. Adding or renaming a member meant editing five
places; four of them would have failed loudly, but as "fix the test" rather
than as one authoritative change, and test_docs' copy is the one guarding the
published OpenAPI contract.

conftest now derives PROBLEM_MEMBERS from Problem.model_fields and the four
assertions read it.
conftest.make_renderer's docstring says it keeps the collaborator wiring in
one place, but test_renderer defined its own `renderer` fixture as a literal
re-implementation of it, and the integration tests constructed
TypstRenderer(Settings(cli_path=Path("typst"), ...)) by hand ten times. A
third collaborator, or a required argument, would have had to be applied to
every one of those sites — which is what the fixture was written to prevent.

The `renderer` fixture now delegates to make_renderer, and the ten
integration sites call it with their overrides. The one remaining direct
construction is in the module-level `_plan` helper, where a fixture is not
available.
…tors

The document publishes `rule_evaluation` — "rules are listed in the order
Prelum applies them; the first to fail is the one reported in context.rule" —
as a contract, but the order actually lives in the statement sequence of
parse_page_selection and validate_pdf_options. Reordering two _reject calls
is a natural thing to do while adding a rule, and only one pair was pinned:
the published order would have become false, a mirror built from the document
would have reported a different id from the service for the same object, and
nothing would have failed.

Six vectors, each breaking two adjacent rules at once, so the id reported is
the claim. Verified by swapping ua_1_with_pdf_a_4 and
version_conflicts_with_standard in models.py: the matching vector fails with
exactly that substitution.

These six are every adjacent pair an `output` object can break together, so
every observable ordering is now checked. The remaining pairs carry no claim
to check because no request can reach them: duplicate_standards with
multiple_pdf_a_standards, and multiple_pdf_a_standards with
ua_1_with_pdf_a_4, each need a third standard that max_standards refuses at
the field level first; pages_requires_archive needs `archive` absent where
page_with_archive needs it present; and the PDF and image rules govern
different formats, so their relative order is not an order at all. That
reasoning is recorded beside the vectors rather than left to be rediscovered.

output-rules.json grows by the six vectors, which is what the artefact is
for. No rule changed, so OUTPUT_RULES_VERSION does not move. openapi.json and
files-key-rules.json are unchanged.
Six findings, all confirmed against the branch before fixing.

app/models.py — the unsupported-format check ran as a mode="before" validator
on RenderRequest, so raising it abandoned the request's other errors and
reported the format's location as the body. A request wrong in several ways
came back with one error instead of four, and a caller mapping
context.errors[].loc to form fields could no longer tell which field the
format error was about. It is now a BeforeValidator on the `output` field, so
the union's own field is the location and sibling errors still collect:
verified that the four-fault request reports all four again, with
loc ["body", "output"]. The validator sits on the field rather than inside the
RenderOutput alias — put there it displaced the discriminator, which turned
the union into a plain one and produced literal_error in place of value_error
and an invalid OpenAPI document. Seventeen tests caught that.

app/render/renderer.py — the archive went back to being built on disk.
Building it in a BytesIO and returning getvalue() holds two copies of an
archive that may be max_output_bytes, with max_concurrent_renders of them in
flight, against a config docstring that documents worst-case memory as the one
multiplied by the other. The efficiency finding this came from described the
on-disk build as an "extra write", but the write *is* the construction; the
only genuinely wasted call was one stat, and measuring a streamed file is how
its size is known. The per-page streaming, which was the real win, stays.

app/main.py — `with suppress(RequestTooLargeError)` swallowed that error
whatever raised it. Latent today, but a route or dependency raising it for its
own reason would have had its response dropped and nothing sent in its place,
leaving the caller a bare 500 where the contract promises a 413 problem body.
Now re-raised unless this middleware is the one that refused the body.

app/core/output_rules.py — the comment claimed the six vectors were every
adjacent pair an output object can break together. They were not:
page_range_end_precedes_start with duplicate_standards is observable, because
`pages` is validated on the field and so runs before validate_pdf_options. A
seventh vector pins it. I re-enumerated all eleven adjacent pairs and probed
the four unpinned ones: two need a third standard that max_standards refuses
first, one needs `archive` both absent and present, and one spans formats.
The corrected claim is now checked rather than asserted.

design/output-delivery.md and AGENTS.md — both named things this branch
changed: render_timeouts_total, which it deletes, and _resource_limit_args,
which became module-level resource_limit_args. AGENTS.md is the standing agent
instruction set, so its load-bearing --data= warning now points at the current
name and says the startup probe forks the same prefix.

openapi.json and files-key-rules.json regenerate byte-identical;
output-rules.json grows by the seven ordering vectors.
`make format` existed but nothing verified it, so the formatter was advisory:
a file drifted out of shape as soon as someone edited it without running the
target, and the next contributor who did run it produced a diff of unrelated
reformatting alongside their own change. That happened repeatedly while this
branch was being written — three files kept coming back reformatted and had to
be reverted out of each commit to keep the diffs readable.

`make format-check` is the read-only half, wired into `checks` and into the
CI step beside `ruff check`, so the drift cannot accumulate again.

The tree is already clean under it: the two files that were drifting were
reshaped by this branch's own edits, so the check is green as added rather
than green after a reformatting commit. Verified both directions — `make
checks` exits 2 with a deliberately misformatted file and 0 without.
CI ran `ty check app` while `make type-check` runs `ty check app scripts`, so
the scripts were checked locally and not in CI. That gap now matters: scripts/
carries shared code — `_artefacts.write_json_artefact`, `render_examples`'s
encoder, which the example regenerator imports — and the documentation build
depends on all of it.

Verified in the CI image rather than assumed: the Dockerfile COPYs scripts/
into the builder stage, and the exact command set (`ruff check`, `ruff format
--check`, `ty check app scripts`) passes inside it.
@pingviini
pingviini merged commit 68e4c17 into main Sep 16, 2026
5 checks passed
@pingviini
pingviini deleted the refactor/render-path-quality-pass branch September 16, 2026 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant