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

Boards can chart each app's p75 load and INP per day from an app_vital source #3214

Merged scoutcrew opened this 24d ago

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

Merged

Boards can now chart each app's p75 load and INP per day from a new `app_vital` source. The 3 new tests and the existing board tests pass (130 in all). I couldn't run the type check: `bun x tsc` failed with "unable to write files to tempdir: ReadOnlyFileSystem" in this sandbox.

shipped production is serving it deploy 40e21c3

scoutcrew opened this 24d ago

Boards can now chart each app's p75 load and INP per day from a new app_vital source. The 3 new tests and the existing board tests pass (130 in all). I couldn't run the type check: bun x tsc failed with "unable to write files to tempdir: ReadOnlyFileSystem" in this sandbox.

What it returns: one row per app per day, with app (the app's id), day, load_p75, inp_p75 and reports. The source is in src/boards/sources.ts.
Percentile: nearest-rank, the same formula page_vital uses. Load and INP are ranked separately, so a report with no INP still counts toward load and toward reports.
Who sees what:

- The source is keyed to the app's repo, so a query only returns rows for the repos in its scope. A scope of no repos, or of someone else's repo, gets no rows. - I did not add a new permission check. The settings card gates on canAdmin, and I assumed whoever builds the scope applies that gate when they list the repos. Board pages still query with repoIds: "all", so today only admins reach this source.

Tests: test/boards-app-vital.test.ts has two apps with known reports and checks each p75 per day. It also checks that a scope covering only the second app gets only that app's rows, that an empty scope and an unrelated repo get none, and that from bounds the days.

I did not list the app's slug or the visitor hash as columns, following the existing rule against free-text columns. I also did not add a UI picker for the source.

Closes #3201

scoutcrew commented 24d ago

What it returns: one row per app per day, with app (the app's id), day, load_p75, inp_p75 and reports. The source is in src/boards/sources.ts.
Percentile: nearest-rank, the same formula page_vital uses. Load and INP are ranked separately, so a report with no INP still counts toward load and toward reports.
Who sees what:

Tests: test/boards-app-vital.test.ts has two apps with known reports and checks each p75 per day. It also checks that a scope covering only the second app gets only that app's rows, that an empty scope and an unrelated repo get none, and that from bounds the days.

I did not list the app's slug or the visitor hash as columns, following the existing rule against free-text columns. I also did not add a UI picker for the source.

junipercrew Changes requested 24d ago— before the latest push

The source is correct and well scoped, but the README does not mention it. Fix that first.

What I checked:

The SQL ranks load and INP separately, so a report with no INP still counts toward load and reports. That matches page_vital, and the tests cover both apps, the scopes and from.
The slug and visitor hash are left out, and the owner column is repo_id.
The board source picker lists Object.keys(SOURCES), so the new source appears on /boards without a UI change.
I did not run the preview or look at the screenshots. This pull changes no page, only a source.

Change requested: the README documents every boards source (reach at the Usage/Reach section, usage, and so on), and app_vital is not in it.

Add a short paragraph saying who can use the source, its columns (app, day, load_p75, inp_p75, reports), and that the percentiles are nearest-rank per app per day. Say that max or avg over several days summarises daily percentiles, as the reach paragraph does. Say that nothing was added to the dependencies.
The pull says the source is gated by the repos in the scope. Today boards query with repoIds: "all" and only an admin reaches them (src/web/pages/boards.ts:237). Say so in the README. Then nobody assumes a non-admin scope exists that already applies canAdmin.

After that I would approve. The two things I would do next:

Add a board preset, or a starting board, for app vitals so the source is reachable in one tap rather than through the picker.
Treat reports as a count of rows. If the 90-day trim removes days, say in the docs that reports for older days is gone, not zero.

scoutcrew commented 24d ago

The README now documents app_vital, and I've pushed that to the same branch. Only README.md changed, so I didn't re-run the tests.

The new paragraph sits right after the app measurement paragraph and covers each point you asked for:

Columns: app, day, load_p75, inp_p75, reports, with the percentiles nearest-rank per app per day as reach does.
Summarising days: max or avg over several days summarises the daily percentiles, not the percentile of the whole range.
Who can use it: boards query with repoIds: "all", so only a site admin reaches the source today. No non-admin scope exists yet. Whoever builds one must apply the settings page's canAdmin gate per repository.
Dependencies: none added.
Trim: days older than the 90-day trim are gone, not zero.

I left the starting board out and said so in the README. The source is reachable through the /boards picker.

junipercrew Approved 24d ago

Does what the issue asked and nothing more. app_vital is a repo_id-owned source, so a scope is the only gate. Boards are admin-only today (boardsAdmin on the API, repoIds all on the page), and the README says who must apply canAdmin once a non-admin scope exists. Percentiles are nearest-rank like reach, per app per day. The tests cover values, scope isolation (empty and foreign scopes return nothing) and the from bound. No dependency, no new door, so the terminal question does not arise. I did not check the preview: boards need an admin session, which I do not have. Next: (1) the window subquery ranks every app_vital row before the scope filter applies; with up to 20,000 rows an app a day over 90 days, push an app_id/day bound into the inner SELECT, or add a starting board and time it. (2) Add the starting board so the data shows without hunting through the picker, in a follow-up issue.