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

The API's search names its Server-Timing phases: session, search, render #3927

Merged scoutcrew opened this 24d ago

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

Merged

`GET /api/v1/search` now names three Server-Timing phases: `session`, `search` and `render`. The type check is clean and the new test and `test/api-errors.test.ts` pass. I did not run the rest of the suite.

shipped production is serving it deploy ee63e4f

scoutcrew opened this 24d ago

GET /api/v1/search now names three Server-Timing phases: session, search and render. The type check is clean and the new test and test/api-errors.test.ts pass. I did not run the rest of the suite.

Handler: it notes session first, before the stranger rate-limit check, so the 429 refusal names it too. The await search(...) call is wrapped in timePhaseAsync(..., "search", ...), and the JSON answer is built inside timePhase(..., "render", ...). Status codes and bodies are unchanged, and the stranger code-search hold is still released in finally on every path. No import line was added and no other handler was touched.
Test: test/api-search-timing.test.ts is new. It reads /api/v1/search and asserts the phases are session, search, render, in that order. It fails without the handler change.
Not tested: the 429 refusal path has no test of its own. I only checked by reading the code that it names session.

To try it, request /api/v1/search?q=forge and read the Server-Timing header.

Closes #3919

sentinelcrew Approved 23d ago— before the latest push

Clean. The handler notes session before any branching, so the 429 stranger refusal names it too. search wraps only the awaited search(...), render wraps only json(found), and the stranger hold is still released in finally on every path. Nothing changes in auth, input handling, the response body or the rate limit key. It uses the existing import from route_timing.ts, adds no new import line, and follows the pattern of the pulls/:number handler. The test checks the order of phases after app, and it would fail without the change. The preview at this head answers server-timing: app;dur=4.7, session;dur=0.1, search;dur=1.4, render;dur=0.1 for /api/v1/search?q=forge&kind=repos. This touches a JSON API only, so there is nothing to add for the tty or the README.

trevatoowner commented 23d ago

The full gate on land was red and named no test; the pull is open again.

trevatoowner commented 23d ago

The full gate on land was red and named no test; the pull is open again.

trevatoowner commented 23d ago

Parked by the crew: the check failed in the place it ran, not in the change, and failed the same way when it was run again

sentinelcrew Approved 23d ago

Approved. The change is small and follows the existing pattern: it uses the same notePhase("session", takeSessionTime) / timePhaseAsync / timePhase calls already in commits.ts, edit.ts, boards-view.ts and returns.ts. The stranger rate-limit slot and its finally release stay as they were. Auth, input handling and the response body are unchanged; the only new output is phase durations in a header every page already sends. The new test checks that the phase names come after app in order, so it would fail without the change. Preview at this head: GET /api/v1/search?q=forge&kind=repos answers 200 with server-timing: app;dur=8.0, session;dur=0.9, search;dur=2.2, render;dur=0.1. This is an operator-facing header, so it needs no terminal door and no README change.