Repository navigation
feat(check): name the saved report file and print one stderr line per check step - #7344
Conversation
…every 30 s while it runs long
There was a problem hiding this comment.
Request changes at 49d530fe.
Blocker: it prints a line on every enter() call, not once per step. enter() is called as a progress ping inside loops, so stderr gets flooded:
checkPipeline.ts:574-576: two calls per layout sample (enter("samples", "layout sample at …s"), thenenter("samples")). The existingcheck: samples n/Nline also prints for each sample.checkPipeline.ts:1536:reportingProgresscallsenter("analysis", name)on every driver method call.checkPipeline.ts:615: once per overlap detail.checkBrowser.ts:740/747: once per frame and once per inspect target.
I measured it on the head with the same call pattern (start, settle, 40 samples, 5 driver calls): 87 stderr lines, alternating checking samples (layout sample at 0s) / checking samples …. A real check with hundreds of samples and driver calls prints far more. That's the opposite of "one line per step", and it's a lot of noise for an agent reading stderr under a token budget.
Suggested fix: in enter(), announce only when the phase changes. For example, read const changed = this.work?.phase !== phase before reassigning this.work, then if (changed) this.announce(…). The 30 s heartbeat can still show work.detail so a long sample stays identifiable. Add a test that calls enter("samples", …) repeatedly and expects one line.
Verified otherwise:
- The 5 named suites (
checkDeadline,checkDeadline.process,checkPipeline.reportingProgress,checkBrowser,commands/check) at head: 256 passed. fails without the fix:holds. With main'scheckDeadline.ts, "names each step and repeats it every 30 s on stderr…" fails.- The interval is
unref'd and cleared in bothclose()andexpire(), stdout stays JSON, and leaving outcopiesis consistent withstartCopyHeartbeat.
Edit accuracy: accurate 267 (base branch 267), smooth 94 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (2)
|
There was a problem hiding this comment.
Approve at 04cc554b.
My earlier blocker is fixed. enter() now prints only when the phase changes, so per-sample, per-driver-call and per-frame re-entries are silent. "names a step once when it is entered again in a loop" pins this.
I also reviewed the scope added since 49d530fe:
- The report id is now created before the run, so the path is printed at start.
saveCheckReportwrites to that same id (wx). Under--json, the saved path is printed on stderr and stdout stays pure JSON. - The
npx hyperframes checkdoc line is consistent with that.
Verified at this head:
checkDeadline,checkDeadline.process,checkPipeline.reportingProgress,checkBrowser,commands/check: 258 passed.fails without the fix:holds. With main'scheckDeadline.tsandcheck.ts, all three new tests fail.
Non-blocking:
- The start line says the report "is saved … when the check ends", but when
captureArtifactRevisionor the save fails, no file is written. TheCheck report was not recordedwarning still goes to stderr, so the reader is told. Softening the wording ("is saved to … unless a 'not recorded' warning follows") would close the gap. - The test plan's "256 passed" is now 258, and "Size: one class plus one test" now covers
check.ts, its test and a doc too.
There was a problem hiding this comment.
Approve at 92210d97.
Since my approval at 04cc554b, two changes:
check.ts: the start line now reads… when the check ends, unless a "not recorded" warning follows. This addresses my non-blocking nit: it matcheswarnNotRecorded'sCheck report was not recordedon stderr.skills-manifest.json: newhyperframes-clihash for the doc line. Runningbun packages/cli/scripts/gen-skills-manifest.tsat this head leaves the tree clean, so the hash is correct.
Verified at this head:
checkDeadline.test.ts+commands/check.test.ts: 178 passed. No test asserts the start-line text.- The
fails without the fix:result from my last review still holds, since no test or logic changed.
There was a problem hiding this comment.
Approve at d25add63.
Since my approval at 92210d97, one line changed: the start line now ends unless a warning says it could not be saved. That's needed. artifactCapture.test.ts:106 asserts that a successful run's stderr does not contain not recorded, so the previous wording would have tripped it on every run.
Verified at this head:
- CI for
d25add63per the Actions API: all 8 workflows completed successfully (CI, CodeQL, regression, preview-regression, Windows render verification, Player perf, PR captures, Comments). - Locally,
checkDeadline,commands/checkandcheckArtifacts: all pass.artifactCapture's spawned-CLI case also fails onmainin my sandbox (the run never starts, sostderris undefined), so that failure is my environment, not this PR. CI runs it green. - No logic or test changed, so the
fails without the fix:result from my earlier reviews still holds.
…-com#7336-heygen-com#7358 Adopt heygen-com#7304 (without CLI renderDiagnostic, needs heygen-com#6725), heygen-com#7331, heygen-com#7332, heygen-com#7334, heygen-com#7337, heygen-com#7336, heygen-com#7339, heygen-com#7289, heygen-com#5594, heygen-com#6900, heygen-com#7322, heygen-com#7342, heygen-com#7343, heygen-com#7350 (onProgress only), heygen-com#7352, heygen-com#7348, heygen-com#7344, heygen-com#7353, heygen-com#7356 and heygen-com#7355. Defer heygen-com#5708 (needs heygen-com#6911) and heygen-com#7345 (READ_VERSION 4 vs 7); heygen-com#7307 not applicable. Re-read 14 moved pins, pin 6 new PRs (monitored 126). Make checkReportFile module-private and suppress the cross-package closeTestChrome duplicate for fallow. Co-authored-by: Miguel Ángel <miguel.sierra@heygen.com> Co-authored-by: SophiaHuangCc <129631224+SophiaHuangCc@users.noreply.github.com> Co-authored-by: Dante <duanjl.china@gmail.com> Co-authored-by: Muzzy5150 <muzzy5150yt@gmail.com> Co-authored-by: ump45nose <52391318+ump45nose@users.noreply.github.com> Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
What
checknow names the file it saves its report to on stderr (at start, and again after a--jsonrun), and prints one line on stderr when it enters each step (starting Chrome, loading the page, waiting for the page to be ready, ...) and repeatsstill <step> (N s in this step)every 30 s while a step runs. Each step prints once, however often it is re-entered. stdout stays pure JSON. Thecheckreference doc now says to runnpx hyperframes check, never throughnpm run.Why
A
check --jsonrun printed nothing for minutes. The existingcheck: samples 3/10lines only start once sampling begins; loading the page and waiting for it to be ready (the 45 s render-ready wait and more) print nothing, so a slow scene looks hung to a person or to an agent with a tool timeout.Why the report file
Under
npm run check, a tool timeout's SIGTERM makes npm exit 143 at once with only its own header on stdout, whilecheckgoes on and writes its report to a pipe nobody reads. The report was already saved under.hyperframes/artifacts/checks/, but nobody was told where. Measured on a real run: TERM at 40 s, npm exit 143 with 109 bytes on stdout, and the saved file was complete JSON.How
CheckDeadline.enteris the one place every step passes through, so it prints the line;start()runs a 30 s unref'd interval thatclose()andexpire()clear. The copy step is left out:startCopyHeartbeatincheckBrowser.tsalready reports its own percent (searchedheartbeatunderpackages/cli/src/utils;runningCli.tsis an unrelated process heartbeat).Size
A small change in one class, the check command, their tests and one doc line, separate from the word-count fix because it has a different root. No open PR touches
checkDeadline.ts.Test plan
checkDeadline.test.ts(fake timers; one line per step re-entered in a loop) andcheck.test.ts(exit line names the saved file).npm run checkon a real WebGL fixture leaves a complete report file; its path was printed on stderr at start and exit.checkDeadline.ts.checkDeadline,checkDeadline.process,checkPipeline.reportingProgress,checkBrowserandcommands/checktest files: 258 passed, 3 runs in a row.checkDeadline.tsunchanged at 0.46%.