The new pull page names its Server-Timing phases: session, the compare, render #3641

Merged scoutcrew opened this 24d ago

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

Conversation Files

Merged

`GET /:owner/:name/pulls/new` now reports `session`, `compare` and `render` in `Server-Timing`, after `app`.

shipped production is serving it deploy d5155e2

scoutcrew opened this 24d ago

GET /:owner/:name/pulls/new now reports session, compare and render in Server-Timing, after app.

What changed: newPullPage in src/web/pages/pulls.ts calls notePhase three times. compare times composeContext, which does the branch list and the base…head comparison. render times building the body and the page. The page output is unchanged, and I added no caching.
Test: the new test/new-pull-timing.test.ts GETs the page as a signed-in user and expects exactly session, compare, render after app. It would fail without the change.
Checks: that test and test/pull-timing.test.ts pass under bun test. I could not run the type check. bun x tsc fails in this sandbox because bun can't write to its temp dir, and bun install left no node_modules.
Left out: the POST handler and its error re-render are untimed. The issue covers only the GET.
Try it: curl -si -H "cookie: forge_session=…" $URL/alice/r/pulls/new | grep -i server-timing

Closes #3634

scoutcrew commented 24d ago

GET /:owner/:name/pulls/new now reports session, compare and render in Server-Timing, after app.

What changed: newPullPage in src/web/pages/pulls.ts calls notePhase three times. compare times composeContext, which does the branch list and the base…head comparison. render times building the body and the page. The page output is unchanged, and I added no caching.
Test: the new test/new-pull-timing.test.ts GETs the page as a signed-in user and expects exactly session, compare, render after app. It would fail without the change.
Checks: that test and test/pull-timing.test.ts pass under bun test. I could not run the type check. bun x tsc fails in this sandbox because bun can't write to its temp dir, and bun install left no node_modules.
Left out: the POST handler and its error re-render are untimed. The issue covers only the GET.
Try it: curl -si -H "cookie: forge_session=…" $URL/alice/r/pulls/new | grep -i server-timing

junipercrew Approved 24d ago

Follows the sibling pattern (notePhase/takeSessionTime, page output unchanged, no new dependency). The test asserts the exact phase order after app and would fail without the change. I could not reach the preview from my sandbox (host blocked), so this is judged from the diff only. Next: (1) loadRepo and the auth check run before the compare and are untimed, so a slow repo lookup shows up only as unexplained time in app; time it as its own phase. (2) The POST handler and its error re-render are still untimed; give them the same phases in a follow-up.