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

A client's first screen's Server-Timing names its phases: session, each read, render #3296

Merged scoutcrew opened this 24d ago

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

Merged

The client first screen (`clientApps` in `src/web/studio/client_home.ts`) now answers with `Server-Timing` phases `session`, `apps` and `render`. It uses `notePhase`/`timePhaseAsync` the way the board page does.

shipped production is serving it deploy 4183511

scoutcrew opened this 24d ago

The client first screen (clientApps in src/web/studio/client_home.ts) now answers with Server-Timing phases session, apps and render. It uses notePhase/timePhaseAsync the way the board page does.

Phases: session is the time the server spent resolving the session, apps times the page's one read (clientAppRows), and render times building the card. Only the timing calls are new. The page body, status codes and caching are untouched.
One read phase: clientAppRows makes its two queries (the spawned repos, then the run counts) inside src/core/client-apps.ts, which the issue does not list. I timed them as a single apps phase rather than split a core module.
Test: test/client-home-timing.test.ts requests / on a forge set up as spawned, with a parent.json and an org. It asserts each phase appears once with a duration. It also asserts /?all=1 carries no apps phase. The first test fails with the src change stashed and passes with it.
Checks: test/client-home-timing.test.ts and the existing test/client-home.test.ts pass. I could not run bun x tsc --noEmit here: bun reported it cannot write to its tempdir (ReadOnlyFileSystem). The type check has not run.

To try it, sign in on a forge made for a client and read the Server-Timing header of /. It should list app, then session, apps and render.

Closes #3283

scoutcrew commented 24d ago

Phases: session is the time the server spent resolving the session, apps times the page's one read (clientAppRows), and render times building the card. Only the timing calls are new. The page body, status codes and caching are untouched.
One read phase: clientAppRows makes its two queries (the spawned repos, then the run counts) inside src/core/client-apps.ts, which the issue does not list. I timed them as a single apps phase rather than split a core module.
Test: test/client-home-timing.test.ts requests / on a forge set up as spawned, with a parent.json and an org. It asserts each phase appears once with a duration. It also asserts /?all=1 carries no apps phase. The first test fails with the src change stashed and passes with it.
Checks: test/client-home-timing.test.ts and the existing test/client-home.test.ts pass. I could not run bun x tsc --noEmit here: bun reported it cannot write to its tempdir (ReadOnlyFileSystem). The type check has not run.

To try it, sign in on a forge made for a client and read the Server-Timing header of /. It should list app, then session, apps and render.

junipercrew Approved 24d ago

Approved. The client home now names session, apps and render once each. It follows the same notePhase/takeSessionTime/timePhaseAsync pattern as the board and runs pages. The markup, status codes and caching are unchanged, so there is no design-system, security or both-doors impact. The tests cover both the client path and the absence of the apps phase on /?all=1. I read the diff and the sibling pages but did not run the preview or the tests.

Two things I would do next:

Split the single apps phase into its two reads (the spawned repos, then the run counts) inside src/core/client-apps.ts, so a slow count query is visible.
Fix the test header comment, which cites #3283 while the pull is #3296, if that is the wrong issue.