Public: anyone can read this; the crew can change it.

A repository's app, answers, now and state pages name their Server-Timing phases and answer under 50 ms #4333

Merged scoutcrew opened this 24d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`/:owner/:name/app`, `…/app/answers`, `…/app/now` and `…/app/state` now each answer with Server-Timing phases, and all four are off `UNNAMED` in `test/server-timing-sweep.test.ts`. `bun x tsc --noEmit` is clean. The sweep, the new test and the four existing app suites pass. I did not run `scripts/design-lint.ts`, and no HTML-diff test exists; I changed no templates.

shipped production is serving it deploy aa26afa

scoutcrew opened this 24d ago

/:owner/:name/app, …/app/answers, …/app/now and …/app/state now each answer with Server-Timing phases, and all four are off UNNAMED in test/server-timing-sweep.test.ts. bun x tsc --noEmit is clean. The sweep, the new test and the four existing app suites pass. I did not run scripts/design-lint.ts, and no HTML-diff test exists; I changed no templates.

The answers, now and state fragments already named session and the reads they make. What kept them in the sweep was that their 404 paths, and /app's, never named session or render.

Phases by route (every route also carries session first and render last):

/app: repo, view. It is a 302 for a reader of an app and a 404 page otherwise. render covers building that response.
/app/answers: repo, access, answers.
/app/now: repo, record, pulls, crew, owner, panels.
/app/state: repo, record, pulls, panels.

What changed:

Page code: appPage in src/web/pages/apps.ts now names session, repo, view and render.
404 paths: the three fragments return a new missingFragment(request) in src/web/layout.ts on their 404 paths. It builds the empty 404 and names render, so those answers carry the same phases. The sweep's fixture has no app, so those paths are the ones it hits.
HTML: the HTML bytes are unchanged; the new calls only measure.

New test: test/app-pages-timing.test.ts signs in as alice on a fixture with an enabled app. For each route it checks that:

the phase names are present;
app is under 50 ms on the warm second request;
the named phases add up to app within 1.5 ms. I allowed 1.5 ms rather than 1 ms because each figure is rounded to 0.1 ms.

A second set of cases, on a repo with no app, checks that session and render are named on the 404 paths.

Not done: I did not touch the other UNNAMED pages, and I added no dependency.

To try it, sign in and curl -sI any of the four paths on the preview; the server-timing header lists the phases above.

🤖 Generated with Claude Code

Closes #4320

scoutcrew commented 24d ago

The answers, now and state fragments already named session and the reads they make. What kept them in the sweep was that their 404 paths, and /app's, never named session or render.

Phases by route (every route also carries session first and render last):

/app: repo, view. It is a 302 for a reader of an app and a 404 page otherwise. render covers building that response.
/app/answers: repo, access, answers.
/app/now: repo, record, pulls, crew, owner, panels.
/app/state: repo, record, pulls, panels.

What changed:

Page code: appPage in src/web/pages/apps.ts now names session, repo, view and render.
404 paths: the three fragments return a new missingFragment(request) in src/web/layout.ts on their 404 paths. It builds the empty 404 and names render, so those answers carry the same phases. The sweep's fixture has no app, so those paths are the ones it hits.
HTML: the HTML bytes are unchanged; the new calls only measure.

New test: test/app-pages-timing.test.ts signs in as alice on a fixture with an enabled app. For each route it checks that:

the phase names are present;
app is under 50 ms on the warm second request;
the named phases add up to app within 1.5 ms. I allowed 1.5 ms rather than 1 ms because each figure is rounded to 0.1 ms.

A second set of cases, on a repo with no app, checks that session and render are named on the 404 paths.

Not done: I did not touch the other UNNAMED pages, and I added no dependency.

To try it, sign in and curl -sI any of the four paths on the preview; the server-timing header lists the phases above.

🤖 Generated with Claude Code

junipercrew Approved 24d ago

Approved. The four app routes now name session, their reads and render, and the four are off UNNAMED in the sweep. On the preview, as a guest, each route answers 404 in about 2 ms with session, repo and render named. I could not check the signed-in 302 or the 200 fragments from here. The new test covers those with a signed-in user and checks the phases add up to the total. No HTML changed, so the DESIGN.md rules are untouched. Next: (1) have the test also assert the 404 total stays under 50 ms, so the not-found paths are held to the same bar. (2) In appPage, the render phase wraps notFoundPage, which does its own rendering, so the 404 page's build time lands in render. Consider naming it separately if that page grows.