emergent_testing: one reporter for both runners - #1749
Conversation
Step 6 of todo/share-browser-console-runner.md: the leaf-landed and run-ended events are now one seam. `Reporter.result` receives the shared `TestResult` built by the runner (the raw `SandboxResult` and throw flag still travel with it — describing a thrown value stays each host's part), and `Reporter.summary` receives `RunTotals`, folded from the leaf results by the new shared `addResult`/`zeroTotals`. The `fjs t` summary line, its exit code, and the browser report's counts and pass/fail status all read that same fold; the browser report's own `duration` stays wall-clock, documented on `RunTotals`, because its leaves run concurrently. Printed output, exit codes and the browser wire report are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PyLwDNkPQM1uD1Tg5ApBg
|
You have reached your Codex usage limits for security reviews. Please try again later. |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PyLwDNkPQM1uD1Tg5ApBg
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 291f3c1 | Commit Preview URL Branch Preview URL |
Aug 28 2026, 07:07 AM |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1bc5aa5eb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The one-reporter change deleted the `addPass`/`addFail` pair that 66a-emergent-add-result existed to merge — the run's totals are now one fold (`addResult` over each leaf's `TestResult`), so there is no pair left to parameterize. The issue file goes with the code it described, and the two todos that quoted the old identifiers as examples now describe the current shape instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PyLwDNkPQM1uD1Tg5ApBg
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Measured at f96b0cf10 on node v23.11.0: npm test 3480/3480 exit 0, tsc --noEmit exit 0 (main 39b4724ef: 3478/3478). No coverage lost — comparing fully-qualified leaf names (bare names collide across files), zero disappear and two are added, matching the +2 exactly.
The sharing is real and, importantly, not the reverted machinery coming back: browser.mjs:22 imports addResult, collectTests, testResult and zeroTotals from module.f.mjs, and both runners build each leaf's status through the same testResult and fold through the same addResult. Rendering stays separate, which is right. I checked against todo/share-browser-console-runner.md:24-105, the record of the reverted #1737: no shared runModuleMap, no effects/common, no browser interpreter, no scheduling invention.
Two mutants survive, and the second is the one that matters.
- Forcing
testResultto always report'passed'(module.f.mjs:407) leavesnpm testat 3480/0, exit 0. It survives unfalsifiably: the proof that pins this,testResult.fails()atproof.f.mjs:658, is itself reported through the mutatedtestResult, so its own failure is relabelled "ok". - Making
addResultsilently drop one leaf from the fold (module.f.mjs:46) leaves exit 0 with the total quietly at 3479. The gate isfailed !== 0, so a lost passing result is invisible; only a human comparing printed totals would catch it.
That bears directly on the claim at share-browser-console-runner.md:176-183 — that a status-mislabelling defect is now caught because the counts read the same fold. That holds only when the corrupted result also reaches a passing assertion elsewhere; it does not hold for the reporter's own self-referential pinning test, and not at all for dropped results. Either assert through a mechanism independent of testResult, or say plainly in that file that reporter-fold correctness is a gap the exit code cannot see.
Also: the PR body has no Changelog: section at all, though changelog/unreleased/1749.md exists and correctly marks the two Reporter signature changes as **BREAKING CHANGES:**. CONTRIBUTING.md:189-195 and AGENTS.md:102 require the section, and the squash lands the body.
Todo hygiene is good: 66a-emergent-add-result.md proposed exactly the fold this PR implements and was correctly deleted as shipped, with its one inbound reference rewritten rather than left dangling.
One thing outside this PR's scope, noted only because it sits next to the work: browser.mjs:268-277's batchSize = 25 batching is unchanged from main, and it resembles the scheduling invention that same todo warns against.
Review measured two surviving mutants: a testResult forced to `passed` and a fold that never counts a failure both leave `fjs t` at exit 0, because the proofs pinning both functions are reported through the functions they test. The note now says that plainly, with the measured counterpart: the external-framework registration path consults neither function, and both mutants fail there under node --test (16 and 18 failures, exit 1), which CI runs on node, bun and deno. Also names the page's pre-existing batchSize=25 as a step-7 decision, not an inheritance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PyLwDNkPQM1uD1Tg5ApBg
|
Thank you — the mutation findings are right, and measuring them further shows where the independent mechanism already is. Reproduced both: The missing On Generated by Claude Code |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f80ac990b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The one-reporter change replaced TestState with RunTotals and its ad-hoc updaters with the addResult fold; todo-property.md and skip-property.md still directed their new counters at the old record. They now extend RunTotals through addResult — and since Reporter.summary receives the whole record, the todo counter no longer needs the signature change the old design assumed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016PyLwDNkPQM1uD1Tg5ApBg
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approved. Both findings are closed, and the first one properly. 6f80ac990 names both mutants — the forced passed status and the fold that drops a result — and states outright that fjs t alone cannot see either. It also identifies the falsifier, all.test.mjs's external-framework register path, and that claim is true rather than hopeful: registerOne in module.f.mjs never touches testResult or addResult. Confirmed empirically at this head — the testResult mutant still SURVIVES npm test (3480/0, exit 0) and is KILLED under node --test fjs/emergent_testing/all.test.mjs (exit 1). That is exactly the right disposition: the gap is real, named, and the escape hatch that can catch it is identified.
The body now ends with a literal Changelog: section matching changelog/unreleased/1749.md verbatim. Gates at 291f3c187: npm test 3480/3480 exit 0, tsc --noEmit exit 0 (main: 3478/3478).
3 participants
Step 6 of
todo/share-browser-console-runner.md: the reporting seam — a leaf landed, a run ended — is now one event stream both runners subscribe to. Step 2 gave them the shared value (TestResult); this gives them the seam it travels through.What changed
Reporter.resultnow receives the sharedTestResult, built by the runner withtestResultnext to the sandbox result it is read from — a reporter renders the value, it no longer derives its own. The rawSandboxResultand the throw flag still travel with it, deliberately: describing a thrown value is each host's part (step 2's finding), and the description needs the value.Reporter.summaryreceives oneRunTotalsrecord, and the totals are decided by one fold: the newaddResult/zeroTotalsturn a stream of leaf results into{passed, failed, duration}. Thefjs tsummary line, its exit code, and the browser report's counts and its run-level pass/fail status all read that same fold — the run-level "did it pass" now has one answer across the runners, like the leaf-level one has since step 2.durationstays wall-clock rather than the fold's summed durations: its leaves run concurrently, so the sum only means "how long the run took" for a sequential runner.RunTotalsdocuments this. Itsinfrastructure-errorstatus still overrides the folded decision, because a run that never reached its leaves has nothing to fold.This is what
todo/report-before-running.mdneeded before a start event could exist: adding one is now a third event kind on an existing stream, not building the stream first.Behaviour unchanged
Printed output, exit codes, and the serializable browser report (shape and values) are identical. The mock reporter's proofs now assert the formatted
pathoff the shared record (.outer().inner,.arr[0]) instead of raw key arrays — stronger pins on the same runs — andaddResultis pinned directly, next totestResult, since the summary and exit code now read it.A limit review measurement made explicit: with the exit code reading the fold,
fjs talone cannot falsify its own reporter — proofs pinningtestResult/addResultare reported through the functions they test. The independent check is the external-framework registration path (all.test.mjs), which consults neither and fails both measured mutants undernode --test; the todo's step-2 note now says this plainly.Verification
npx tscclean,fjs t3480/3480 (two newrunTotalsproofs),npm run covexit 0 (100% thresholds). Local bun is 1.3.11 against CI's 1.4.0 and shows the same known failure set with and without this diff; none of it touches the reporter.Changelog:
emergent_testing:Reporter.resultnow receives thenormalized
TestResultandReporter.summaryoneRunTotalsrecord; thenew
addResultandzeroTotalsfold leaf results into every runner'stotals. Printed output and the browser report are unchanged
🤖 Generated with Claude Code
https://claude.ai/code/session_016PyLwDNkPQM1uD1Tg5ApBg