ci(probe): record emulator health-check durations per iteration

[tyler-reitz]

Why

The grpc-js override was measured and does not fix the flake: 120 full-suite runs gave baseline 5/60 vs override 4/60, Fisher exact p = 1.0. So the default arm list drops to ["baseline"], with the arm kept as a dispatch input rather than deleted.

What replaces it is timing. Firestore's health check is a bare addDoc round trip, and every waitFor in test/firestore.test.tsx uses the 1000ms default, which would explain the failures landing at 1058ms and 1142ms.

What changes

Each iteration records the health-check duration for firestore, auth and database into a new probe-iterations.tsv, and the summary compares median and max across outcomes:

Outcome	n	firestore	auth	database
pass	3	195 / 210	95 / 101	120 / 130
flake	2	2245 / 2245	2100 / 2100	1800 / 1800

All three, not firestore alone: a slow firestore round trip only explains #776 if firestore is slow specifically. The illustrative shape above, with everything rising together, would instead say runner-wide contention and send the fix elsewhere.

Two implementation notes

Durations come from vitest's json reporter, not the log. The default reporter prints no per-test line at all on a passing run, so the log would yield durations only for runs that went wrong. The json reporter runs alongside it, leaving the existing classifier greps untouched.

Extraction is scoped by file path. All three test files open with a test named double check - emulator is running, so matching on the title would record whichever vitest emitted first.

Verification

• One real local iteration against the firestore emulator, running the workflow's exact command: extraction returned 1623 na na, the nas correct for firestore-only mode.
• Against a fixture holding all three files, returns 2246 111 786, so each column takes its own file.
• Missing json, unparseable json and a failing run all yield na, never a bogus 0.
• YAML parses; zizmor clean.

⚠️ workflow_dispatch is only offered for workflows on the default branch, so this cannot be run from the branch. Merging is what makes it dispatchable, and the first real run is the first proof, same as #780.

Refs #776, and does not close it. #783 stays open too.

[armando-navarro]

Thanks for this, and for putting the reasoning in the workflow file rather than only in the PR description. The code itself is right and I have run it.

I am requesting changes for one thing only, the wrong sentence about the vitest reporter. Everything else here is yours to take or leave.

On the timing question itself

The durations for all 120 iterations are recoverable from the logs you already uploaded, they show no separation between the runs that failed and the runs that passed, and I think the reason is that the health check times a write while the two failing tests wait on the Listen stream.

The reporter claim is not right, and this is the one thing I am blocking on

The new comment and the commit message both say the default reporter prints no per-test line on a passing run. What I found instead:

• It prints one for any test over the 300ms slow threshold, pass or fail.
• In your artifact, firestore and database health checks appear on all 120 iterations, including the 111 that passed. That is how I recovered the durations.

The json reporter is still the right call, for a better reason: the log is censored at 300ms, so it drops auth entirely and truncates everything else at the fast end. As the comment stands, someone could test the claim, find durations in the log, and conclude the reporter was not needed.

This is the only thing I am blocking on, and purely because of where it ends up. The comment stays editable. The commit message does not, once this squashes into main, so a wrong statement about the tooling would sit in the history for whoever picks it up next.

Nothing else in the review needs to happen. Push the fix and I will re-review straight away.

Three small ones in the summary table

• The n column counts rows for that outcome, but the duration cells are computed only over the finite ones, since series filters on Number.isFinite and Number("na") is NaN. So an infra row can read n = 5 off two actual measurements. The footnote below the table gives a total for missing firestore durations but does not attribute it to a row.
• q(a, 0.5) takes the upper of the two middle values, so at n = 2 the median and max cells print the same number. The two compound: five infra rows of which two had durations, 1200 and 1400, render as n of 5 with a firestore cell of 1400 / 1400. The flake row is where n will be smallest, 1 to 3 per arm last time.
• The table pools arms. probe-iterations.tsv carries the arm in column 2 and the summary drops it, so a dispatch with both arms puts baseline and override rows into one distribution while the counts tables above stay per-arm. The baseline-only default makes this rare rather than gone.

[tyler-reitz]

You're right on the reporter, and I ran it rather than taking it on reading: standalone vitest 4.1.10, three passing tests at 0ms, 150ms and 400ms, default reporter. Only the 400ms one printed a line. So the log is censored at the slow threshold rather than silent on passing runs, which is your reason and the better one. Fixed in the workflow comment and in the commit message.

Took the three table items as well:

Ask	Now
n counted rows, cells counted finite values	each cell carries its own (n=…), row column renamed runs, missing durations attributed per outcome
upper of the two middles at even n	mean of the two
arms pooled	one table per arm

Verified against fabricated rows reproducing your case (5 infra, two measurements at 1200 and 1400): reads runs 5 | 1300 / 1400 (n=2). Restoring the old quantile prints 1400 / 1400, so the check can fail.

On the listen-vs-write point: if the health check times a write while the two failing tests wait on the Listen stream, this probe is timing the wrong round trip. Dispatch it once merged anyway, or instrument the Listen path first?

[armando-navarro]

Thanks for turning this around so fast, and for running the reporter case rather than taking it on reading. Approving.

One optional wording fix

Take it or leave it, and it changes no behavior.

The workflow comment and the commit message both say the default reporter prints a per-test line "only above its 300ms slow threshold". There is a second case:

• Every test in a file that failed prints regardless of its duration.
• The last branch of printTestCase reads else if (this.renderSucceed || moduleState === "failed").
• Running vitest 4.1.9 over two files, the passing one printed only its 400ms test, while the failing one also printed its 1ms and 152ms tests.

Worth a sentence because it sharpens your own argument. The log is not censored uniformly, it is censored in a way that tracks outcome: a file that passed gives up only its slow tests, a file that failed gives up every test in it. The bias runs along the same axis the summary compares, which is a better reason to prefer the json reporter than the fast end being cut off.

[armando-navarro]

On your listen versus write question: I would dispatch it, and treat instrumenting the Listen path as the next step rather than a prerequisite.

Two reasons to dispatch as it stands:

• The auth column is genuinely new. No test in auth.test.tsx printed a line in any of the 120 iterations of run 31223456796, so that column is absent from those logs and the json reporter is the way to get it.
• The database column gives you the runner-wide comparison the table was set up for, whichever stream firestore's own check happens to exercise.

I would not wait on the firestore column to settle #776 though, because I think it will come back flat. The durations for that column turn out to be recoverable from run 31223456796 for all 120 iterations, and the flake and pass distributions sit on top of each other. Numbers and method are in a comment on #776.

On instrumenting Listen, here is what made me raise it in the first place:

• The health check is addDoc, so it times a write.
• Both failing waitFor calls are on the Listen stream.
• The json report carries a duration for every test, so keying on a Listen-based test looks like a change to the title filter rather than a rewrite.

Which test to key on is your call, and if you think the write round trip is the better proxy after all I would like to hear why.

[tyler-reitz]

Merged with the commit message updated to match what we now know; the timing frame it was written under is dead. Mechanism and plan are on #776 (comment). Dispatch waits for the long-polling PR, then one 30-iteration run is the proof.