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

test/pull-timing.test.ts: the Files page's repeat-median ceiling fails under parallel load #5014

Merged opened this

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

Merged

In `test/pull-timing.test.ts`, the Files-page timing test now measures machine load beside each of its eight views instead of once before all of them. Before this, load that started partway through the views didn't raise the ceilings, so pulls that never touch the Files page could still fail it. That is how #4806 failed, with a 105 ms repeat median against the 100 ms ceiling.

shipped production is serving it deploy 674862e

opened this

In test/pull-timing.test.ts, the Files-page timing test now measures machine load beside each of its eight views instead of once before all of them. Before this, load that started partway through the views didn't raise the ceilings, so pulls that never touch the Files page could still fail it. That is how #4806 failed, with a 105 ms repeat median against the 100 ms ceiling.

  • Per-view scaling: idleEquivalent(view) runs the fixed CPU spin (fastest(2)) just before and just after each view. It divides the view's time by loadScale(before, after, idle), the slower of the two samples over the idle reference, kept between 1× and 8×. The medians of these adjusted times are then held to the fixed 250 ms (cold) and 100 ms (repeat) ceilings.
  • The idle bar is unchanged: the scale never goes below 1.0, so on an idle machine a real 105 ms repeat view still fails the 100 ms bar from #4215.
  • New unit test for loadScale:

- idle gives 1.0; - a machine running faster than when the test loaded gives 1.0, and 105 ms still fails; - load seen only before or only after a view counts; - the 8× cap holds.

Not done: I left the ceilings, the 8× cap and the spin workload as they were. Each view now costs about four extra spins, which adds roughly 0.1–0.2 s to the test.

Verified: the file passed 3 of 3 runs idle and type-checks with bun x tsc --noEmit. It also passed 3 of 3 runs with four CPU-burning processes started 1.5 s into the test, which is the "load arrives mid-run" case from the issue. I didn't run the full suite; the forge's check does.

Try it: bun test test/pull-timing.test.ts. To see it hold under load that starts mid-run, start a few bun -e 'for(;;){}' processes during the test.

🤖 Generated with Claude Code

Closes #5000

commented

In test/pull-timing.test.ts, the Files-page timing test now measures machine load beside each of its eight views instead of once before all of them. Before this, load that started partway through the views didn't raise the ceilings, so pulls that never touch the Files page could still fail it. That is how #4806 failed, with a 105 ms repeat median against the 100 ms ceiling.

  • Per-view scaling: idleEquivalent(view) runs the fixed CPU spin (fastest(2)) just before and just after each view. It divides the view's time by loadScale(before, after, idle), the slower of the two samples over the idle reference, kept between 1× and 8×. The medians of these adjusted times are then held to the fixed 250 ms (cold) and 100 ms (repeat) ceilings.
  • The idle bar is unchanged: the scale never goes below 1.0, so on an idle machine a real 105 ms repeat view still fails the 100 ms bar from #4215.
  • New unit test for loadScale:

- idle gives 1.0; - a machine running faster than when the test loaded gives 1.0, and 105 ms still fails; - load seen only before or only after a view counts; - the 8× cap holds.

Not done: I left the ceilings, the 8× cap and the spin workload as they were. Each view now costs about four extra spins, which adds roughly 0.1–0.2 s to the test.

Verified: the file passed 3 of 3 runs idle and type-checks with bun x tsc --noEmit. It also passed 3 of 3 runs with four CPU-burning processes started 1.5 s into the test, which is the "load arrives mid-run" case from the issue. I didn't run the full suite; the forge's check does.

Try it: bun test test/pull-timing.test.ts. To see it hold under load that starts mid-run, start a few bun -e 'for(;;){}' processes during the test.

🤖 Generated with Claude Code

Approved

Approved. A test-only change: the load scale is now sampled beside each view (slower of before/after, floored at 1.0, capped at 8x) and the fixed 250/100 ms ceilings are kept. That fixes the mid-run-load flake (#4806, 105 ms vs 100 ms). It adds no runtime surface, so no page, terminal, security or README concerns apply. The idle bar is unchanged, and the scale function has a unit test. It serves stability, since flaky checks block the merge queue. Two things I would do next: (1) drop the tautological assertion 105 / loadScale(8, 9, 10) > 100, which only restates that the scale is 1, and replace it with a real idle-failure case; (2) if the file still flakes, take the best of N repeat views instead of widening the cap, because dividing by a scale taken from a 2-spin sample is noisy.

commented

The repeat-view check on the Files page in test/pull-timing.test.ts no longer fails when other test files run at the same time. Before, the test measured machine load once, before the eight views, and stretched the 250 ms first-view and 100 ms repeat-view limits by that amount. Load that started partway through the views wasn't counted, so a 105 ms repeat median failed.

Now each view is timed with a load sample taken just before it and another just after. Its time is divided by the slower of the two, giving what it would have taken on an idle machine. The medians are then held to the limits as written. That divisor is loadScale in test/pull-timing.test.ts:

  • No load: it never goes below 1.0, so an idle machine still has to meet 250/100 ms.
  • Stalled machine: it is capped at 8×, so a stall can't excuse a slow render.

A new test checks loadScale against worked numbers:

  • An idle machine's 105 ms repeat view still fails the 100 ms limit.
  • When the after-sample is 1.5× the idle time, the same 105 ms run passes, which is the case from #5000.
  • The 8× cap holds.

Only the test changed; the page itself did not. I left out a retry-on-failure, because that would hide real slowdowns.

To try it: bun test test/pull-timing.test.ts gives 4 pass. To reproduce the original failure, run it next to the full suite with bun test and compare against the parent commit.

The new comment on #5000 is the automatic triage asking for a pillar; that's a person's decision, so this pull doesn't set one.

🤖 Generated with Claude Code

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs