From 0f845a319a065dd598605976786db2a45ffef41c Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 27 Aug 2026 22:34:34 -0400 Subject: [PATCH 1/2] fix(scripts): give the render-smoke overlap test room for process startup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test gave itself a 150ms budget that had to cover `script(1)` PTY allocation plus Node boot: `paintAt` is measured from the child's first JS tick, while `renderTimer` is armed in the parent at spawn. Measured idle, that startup is 66-114ms — ~36ms of slack at best — and under the load of a full `local:gate` run it goes over, so the render timer fires before the paint and the run is reported as "did not render", which is precisely the misdiagnosis this test exists to catch. It failed three gate runs in a row while passing standalone every time. The constants are widened rather than the assertion changed: the shape under test is the overlap (paint < timeoutMs < paint + surviveMs), and 1200/1500 keeps it with real margin on both sides. Verified by mutation — removing the `clearTimeout(renderTimer)` this test guards makes it fail at 1204ms, i.e. the timer still fires inside the survival window. Closes #2177 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev Signed-off-by: cliffhall --- scripts/lib/render-smoke.test.mjs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/scripts/lib/render-smoke.test.mjs b/scripts/lib/render-smoke.test.mjs index a77b5468b..dce174310 100644 --- a/scripts/lib/render-smoke.test.mjs +++ b/scripts/lib/render-smoke.test.mjs @@ -93,10 +93,27 @@ test("a paint landing just under the deadline is not failed by the render timer" `setTimeout(() => { console.log(${JSON.stringify(MARKER)}); ` + `setInterval(() => {}, 1000); }, ${paintAt});`, ), - // Deliberately tight: the render deadline expires while the survival window - // is still open, which is the whole shape of the bug. - timeoutMs: paintAt + 150, - surviveMs: 400, + // The shape being reproduced is an overlap, and only an overlap: + // + // actual paint < timeoutMs < actual paint + surviveMs + // + // i.e. the render deadline expires while the survival window is still + // open. Both margins are deliberately wide, because `paintAt` is measured + // from the child's first JS tick while `renderTimer` is armed in the + // parent at spawn — so the left margin has to absorb `script(1)` PTY + // allocation plus Node boot, which measured 66-114ms on an *idle* machine + // and goes past 150ms under the load of a full `local:gate` run. These + // constants used to be `paintAt + 150` / `400`, which left ~36ms of slack + // at best and failed deterministically inside the gate (#2177) — with the + // "did not render" diagnostic that is exactly the misreport this test + // exists to catch. + // + // Do not tighten these back up to make the test faster: the overlap is the + // point, not the tightness. Worst-case paint lands near 365ms, well under + // the 1200ms deadline, and 1200ms still falls inside a survival window + // that closes no earlier than ~1750ms. + timeoutMs: 1200, + surviveMs: 1500, }); assert.equal(r.code, 0, `expected pass, got: ${r.message}`); assert.doesNotMatch(r.message, /did not render/); From 7083c6c7aab2d2c7f2180f964b8e7d39409bebbc Mon Sep 17 00:00:00 2001 From: cliffhall Date: Thu, 27 Aug 2026 22:59:26 -0400 Subject: [PATCH 2/2] docs(scripts): correct the attribution in the render-smoke margin comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot round 1. The comment blamed `script(1)` PTY allocation, but this test's `stub()` spawns `process.execPath` directly — the PTY wrapper lives in `smoke-tui.mjs`, the caller — so the fixture never pays that cost and the 66-114ms figure (measured through the wrapper) was not this path's. Re-measured without it: Node boot alone is 46-59ms idle, which fits the old 150ms budget comfortably. That is *why* it read as a flake rather than a bug — what actually exhausted the budget was scheduling delay under a full gate run, on both the child's boot and the parent's timer. The fix is unchanged; only the reasoning was wrong. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013hzwzS8UvBsr4yZm7o7sev Signed-off-by: cliffhall --- scripts/lib/render-smoke.test.mjs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/scripts/lib/render-smoke.test.mjs b/scripts/lib/render-smoke.test.mjs index dce174310..64d4a7a37 100644 --- a/scripts/lib/render-smoke.test.mjs +++ b/scripts/lib/render-smoke.test.mjs @@ -98,20 +98,26 @@ test("a paint landing just under the deadline is not failed by the render timer" // actual paint < timeoutMs < actual paint + surviveMs // // i.e. the render deadline expires while the survival window is still - // open. Both margins are deliberately wide, because `paintAt` is measured - // from the child's first JS tick while `renderTimer` is armed in the - // parent at spawn — so the left margin has to absorb `script(1)` PTY - // allocation plus Node boot, which measured 66-114ms on an *idle* machine - // and goes past 150ms under the load of a full `local:gate` run. These - // constants used to be `paintAt + 150` / `400`, which left ~36ms of slack - // at best and failed deterministically inside the gate (#2177) — with the - // "did not render" diagnostic that is exactly the misreport this test - // exists to catch. + // open. Both margins are deliberately wide, because the two clocks start + // at different moments: `paintAt` is measured from the child's first JS + // tick, while `renderTimer` is armed in the parent at spawn. The left + // margin therefore has to absorb everything in between — child process + // startup, plus whatever scheduling delay the machine is under. + // + // `stub()` spawns `process.execPath` directly, so there is no `script(1)` + // in this path: the PTY wrapper lives in `smoke-tui.mjs`, the caller. Node + // boot alone measured 46-59ms idle here. That fits the old `paintAt + 150` + // budget with room to spare, which is exactly why this read as a flake + // rather than a bug — but under the load of a full `local:gate` run the + // remaining ~90ms of slack is not enough to cover scheduling delay on both + // the child's boot and the parent's timer, and it failed three gate runs + // in a row while passing every time standalone (#2177). The diagnostic it + // failed with, "did not render", is the misreport this test exists to + // catch. // // Do not tighten these back up to make the test faster: the overlap is the - // point, not the tightness. Worst-case paint lands near 365ms, well under - // the 1200ms deadline, and 1200ms still falls inside a survival window - // that closes no earlier than ~1750ms. + // point, not the tightness. 1200ms clears any plausible paint time, and + // still falls inside a survival window that cannot close before ~1750ms. timeoutMs: 1200, surviveMs: 1500, });