emergent_testing: name a browser test the way `fjs t` does by sergey-shandar · Pull Request #1738 · functionalscript/functionalscript

sergey-shandar · 2026-08-27T11:14:46Z

Step 1 of emergent_testing/todo/share-browser-console-runner.md, which this PR
also sequences into seven steps, one per pull request.

Why this first

The reverted attempt did the whole issue at once — 2646 insertions and 1408
deletions across 35 files — and that is why its arguments could not be
separated: a question about scheduling became a question about the port. This is
the smallest piece of the issue that stands on its own, and it happens to be its
most visible symptom.

The change

Both runners already share collectTests and fmtPath, so the traversal agreed
while the two reports could not be compared:

	before	after
`fjs t`	`import("./a.f.mjs").proof.x(): ok, 0.3 ms`	unchanged
page	`PASS ./a.f.mjs .x (0.3 ms)`	`PASS import("./a.f.mjs").proof.x() (0.3 ms)`

Results now carry a name built by fmtImport — the function fjs t formats
its own result lines with — and the page renders it. nameMatchesTheConsoleRunner
asserts against that function rather than against a spelling, so the two cannot
drift again without a proof failing.

name is a field rather than something the renderer derives, because module
and path do not always recombine into one: a module that cannot be enumerated
and a proof exported as a bare function both carry an empty path, and only the
code that produced the result knows which it had. A module-level failure is
named by its source, which is what is known about it — unreadableModuleIsNamedByItsSource
covers that.

path stays for the consumers that read it. It is now redundant with name for
every leaf, and that is recorded in the todo as part of the report-shape
decision rather than changed here.

Why the field is required

renderBrowserReport is exported and reads name, so a report assembled by hand
against the previous { module, path, ... } shape renders undefined in place
of each identity. That is a breaking change and the changelog says so.

Making name optional with a fallback to ${module} ${path} was the
alternative, and it is the wrong one: it would keep a second spelling of a test
name alive inside the renderer, which is precisely what this change exists to
remove. Every report the module itself produces — runBrowserProofs,
startBrowserTests, startBrowserTestSources — carries the field, so no
in-repository caller is affected.

Notes on review

A module key is relative to the root a run was given: fjs t invoked in
fjs/types/list names a leaf import("./proof.f.mjs")... where the same leaf
from the repository root is import("./fjs/types/list/proof.f.mjs").... That is
fjs t differing from itself across roots — loadModuleMapStripsInitCwdPrefix
pins it deliberately — not the two runners differing. Given the same key the two
agree, which is what this PR establishes. What is worth keeping from that
observation is that nothing in a report declares its root, so two reports only
compare when their roots agree; that is a report-shape question and is recorded
with path in the todo rather than widening this PR.

The remaining steps

Recorded in the todo, each reviewable without the next: one sandbox; common
effects; a browser interpreter; one reporter; one skeleton; the layout move.
Steps 2 and 6 are the ones that change behaviour, so they are the ones to keep
smallest.

Also here

Two todos requested alongside this step, neither touching code:

fjs/website/todo/directory-index-pages.md — generate an index.html beside
every module.f.mjs, cataloguing that directory's files, subdirectories,
todo/ issues and local proofs, and running those proofs through the existing
browser runner with a narrowed manifest rather than a second runner.
the module-key note above, in share-browser-console-runner.md.

Verification

npx tsc clean, fjs t 3472/3472, npm run cov at 100% lines, branches and
functions. Two new proofs, both of which fail without the change.

Changelog:

BREAKING CHANGES: emergent_testing: a browser test result gains a
required name — the test identity fjs t prints, built by the same
fmtImport function — and the page renders it, so both runners spell a test
identically. renderBrowserReport reads name, so a report built by hand
against the previous { module, path, ... } shape renders undefined in
place of every identity; reports produced by runBrowserProofs,
startBrowserTests and startBrowserTestSources carry the field and are
unaffected. module and path are unchanged

🤖 Generated with Claude Code

https://claude.ai/code/session_016PyLwDNkPQM1uD1Tg5ApBg

First step of `todo/share-browser-console-runner.md`, which is now sequenced into seven of them, one per pull request. The reverted attempt did the whole issue at once, and that is why an argument about scheduling became an argument about the port. The page named a test `${module} ${path}` -- `./a.f.mjs .x` -- where the console runner prints `import("./a.f.mjs").proof.x()`. Both already share `collectTests` and `fmtPath`, so the traversal agreed while the two reports could not be compared, which is the visible half of the runners having drifted apart. Results now carry a `name` built by `fmtImport`, the function `fjs t` formats its own result lines with, and the page renders that. `nameMatchesTheConsoleRunner` asserts against the function rather than against a spelling, so the two cannot drift again without failing. `name` is a field rather than something the renderer derives, because `module` and `path` do not always recombine into one: a module that cannot be enumerated and a proof exported as a bare function both carry an empty `path`, and only the code that produced the result knows which it had. A module-level failure is named by its source, which is what is known about it. `path` stays for the consumers that read it, and is now redundant for every leaf -- recorded in the todo as part of the report shape rather than changed here. Changelog: - `emergent_testing`: the browser report's results carry a `name` -- the test identity `fjs t` prints, built by the same `fmtImport` function -- and the page renders it, so both runners spell a test identically

chatgpt-codex-connector · 2026-08-27T11:14:56Z

You have reached your Codex usage limits for security reviews. Please try again later.

cloudflare-workers-and-pages · 2026-08-27T11:15:14Z

Deploying with Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	functionalscript	`dd3866a`	Commit Preview URL Branch Preview URL	Aug 27 2026, 03:13 PM

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 03b3b06cf7

ℹ️ 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".

chatgpt-codex-connector · 2026-08-27T11:17:31Z

 /** @type {(module: string, path: readonly (string | null)[], throws: boolean, fn: () => unknown, result: (result: _BrowserTestResult) => void) => Promise<readonly _BrowserTestResult[]>} */
 const runOne = (module, path, throws, fn, result) => {
    const start = performance.now()
+    const name = fmtImport(module, path)


Canonicalize module paths before formatting names

When fjs t is run from a supported subtree, loadModuleMap makes its module keys relative to that working directory, whereas the browser manifest always supplies repository-root-relative sources. For example, the same leaf can therefore be named import("./module.f.mjs")... by the CLI and import("./fjs/types/foo/module.f.mjs")... by the browser. Passing the raw module to fmtImport means the new field does not provide the promised cross-runner identity in this supported context, so report comparison, deduplication, or filtering still treats the same test as different; normalize both runners to a common module identifier before formatting.

AGENTS.md reference: AGENTS.md:L60-L63

Useful? React with 👍 / 👎.

The observation is right and the diagnosis is not — normalizing inside fmtImport's callers would be the wrong fix.

Reproduced. From the repository root:

import("./fjs/types/list/proof.f.mjs").proof.cycle[0](): ok

and with INIT_CWD=fjs/types/list:

import("./proof.f.mjs").proof.cycle[0](): ok

But note what that comparison is between: both lines are fjs t. This is the CLI differing from itself across two invocations, not the CLI differing from the browser. loadModuleMap strips the INIT_CWD prefix deliberately — loadModuleMapStripsInitCwdPrefix in fjs/dev/module.f.mjs pins it — because a subtree run reports a subtree, and naming those leaves by a path the reader is not standing in would be the surprising behaviour.

So a name embeds a module key, and a module key is relative to the root a run was given. Given the same key, the two runners produce the same name, which is exactly what this PR changes and what nameMatchesTheConsoleRunner pins. Given different roots they produce different keys — and would still do so after any normalization that did not also invent a canonical root, which is the part that cannot be assumed: the browser application root is not the repository root in the design emergent_testing/todo/browser-testing.md describes, so "canonicalize to the repository" would be picking one host's root and calling it universal.

What is left of the finding is real and worth stating: two reports only compare when their roots agree, and nothing in the report declares its root. That is a report-shape question, not a naming one, and it now sits with the other open report-shape item (path, which this PR makes redundant) in emergent_testing/todo/share-browser-console-runner.md. Fixing it here would widen the PR past the one thing it does.

The PR body's "both runners spell a test identically" should be read as "for the same module key"; I have not changed the code.

Generated by Claude Code

o2alexanderfedin

Approved. Verified at 03b3b06cf: npm test 3472/3472 exit 0, tsc --noEmit exit 0 (main 12106a3b0: 3470/3470). Branch is current — main is already an ancestor, so the merge result is the head itself. The name comes from the same fmtImport the console runner uses (browser.mjs:203) rather than a re-derived spelling, and it is pinned: appending 'X' to the constructed name is KILLED — 3471/3472, failing exactly nameMatchesTheConsoleRunner(). Leaf-name diff against main is exactly the two new leaves and nothing dropped. Changelog: is literal and matches the entry file item-for-item.

…elative to `directory-index-pages.md`: the generated site is one page, while the repository is a tree of directories that each hold a module, its types, its proofs and its todos -- none of it reachable from the site. A generated `index.html` beside every `module.f.mjs` catalogues its files, subdirectories, `todo/` issues and local proofs, and runs those proofs through the existing browser runner with the manifest narrowed to that directory. It is not a second runner, and it is a second consumer of the traversal the website program already performs. Also records, in `share-browser-console-runner.md`, that a test name embeds a module key and a module key is relative to the root a run was given: `fjs t` in `fjs/types/list` names a leaf `import("./proof.f.mjs")...` where the same leaf from the repository root is `import("./fjs/types/list/proof.f.mjs")...`. That is `fjs t` differing from itself across roots rather than the two runners differing, and it is deliberate -- but two reports only compare when their roots agree, so which root a report declares belongs to the report-shape decision.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a9f175cb25

ℹ️ 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".

chatgpt-codex-connector · 2026-08-27T14:47:12Z

    item.setAttribute('data-status', result.status)
    const detail = result.status === 'failed' ? `: ${result.message}\n${result.stack}` : ''
-    item.textContent = `${result.status === 'passed' ? 'PASS' : 'FAIL'} ${result.module} ${result.path} (${result.duration.toFixed(1)} ms)${detail}`
+    item.textContent = `${result.status === 'passed' ? 'PASS' : 'FAIL'} ${result.name} (${result.duration.toFixed(1)} ms)${detail}`


Preserve reports that predate the name field

When the exported renderBrowserReport is called with a report created by an earlier version—or by an existing controller using the previous { module, path, ... } result shape—result.name is absent, so every row now renders as PASS undefined ... rather than displaying its identity. Either retain the old module/path rendering as a fallback or explicitly treat and document this as a breaking API change.

AGENTS.md reference: AGENTS.md:L91-L98

Useful? React with 👍 / 👎.

Correct, and taken as the second of the two options you named: documented as a breaking API change rather than given a fallback.

changelog/unreleased/1738.md now reads:

BREAKING CHANGES: emergent_testing: a browser test result gains a required name … renderBrowserReport reads name, so a report built by hand against the previous { module, path, … } shape renders undefined in place of every identity; reports produced by runBrowserProofs, startBrowserTests and startBrowserTestSources carry the field and are unaffected. module and path are unchanged

The fallback is the option I deliberately did not take. Retaining ${module} ${path} when name is absent would keep a second spelling of a test name alive inside the renderer, which is the exact thing this change exists to remove — a page that names a test one way for fresh reports and another way for older ones has two identities again, just conditionally. Every report the module produces carries the field, so no in-repository caller is affected, and the PR body now states the reasoning where a reader will look for it.

Generated by Claude Code

`renderBrowserReport` is exported and now reads `result.name`, so a report assembled by hand against the previous `{ module, path, ... }` result shape renders `undefined` where its identity should be. The field is required rather than optional on purpose -- a fallback to the old `${module} ${path}` spelling would keep two spellings of a test name alive, which is what this change exists to remove -- so the honest description is a breaking one. Reports produced by `runBrowserProofs`, `startBrowserTests` and `startBrowserTestSources` all carry the field, so no in-repository caller is affected.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dd3866aeea

ℹ️ 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".

chatgpt-codex-connector · 2026-08-27T15:15:27Z

 /** @type {(module: string, path: readonly (string | null)[], throws: boolean, fn: () => unknown, result: (result: _BrowserTestResult) => void) => Promise<readonly _BrowserTestResult[]>} */
 const runOne = (module, path, throws, fn, result) => {
    const start = performance.now()
+    const name = fmtImport(module, path)


Guard test-name formatting from proof-side mutation

When a proof or embedding page replaces JSON.stringify before a nested proof is scheduled, this synchronous fmtImport call throws before runOne installs its rejection handlers. Because fmtImport uses JSON.stringify, the exception rejects Promise.all and the entire runBrowserProofs call instead of producing a failed result, potentially leaving the page in running; the parent version resolves normally for the same identifier-only proof tree. Capture a safe formatter or handle name construction inside the protected execution path so this change does not introduce that regression.

AGENTS.md reference: AGENTS.md:L103-L106

Useful? React with 👍 / 👎.

o2alexanderfedin

Approval still stands. Verified at dd3866aee: npm test 3472/3472 exit 0, tsc --noEmit exit 0 (main 12106a3b0: 3470/3470). The delta touches zero .mjs/.ts files — only the changelog entry and two todo documents — so the naming feature I approved is untouched by construction.

The BREAKING marker is right, not over-broad: name is a required field on _BrowserTestResult (browser.mjs:77), which is the JSDoc type of the exported renderBrowserReport, and renderResult at :453 reads result.name with no fallback — a hand-built old-shape report prints undefined. package.json has no exports map, only a files allowlist, so consumers reach it by path. Changelog: is byte-identical to the entry file.

directory-index-pages.md is Status: open with everything unchecked and describes only future work; both claims I spot-checked hold (the generated site is a single writeFile('index.html', …) at website/module.f.mjs:64, and the discovery walk already visits every directory at browser-prepare.mjs:20), and all five links resolve. The additions to share-browser-console-runner.md and generate-website.md are notes, not shipped-work claims — worth stating explicitly since that file was restored by the #1737 revert: the note cites loadModuleMapStripsInitCwdPrefix (fjs/dev/module.f.mjs:181), which is unrelated to the reverted shared-runner code.

claude added 2 commits August 27, 2026 11:14

changelog: rename the entry to this PR's number

03b3b06

chatgpt-codex-connector Bot reviewed Aug 27, 2026

View reviewed changes

o2alexanderfedin approved these changes Aug 27, 2026

View reviewed changes

sergey-shandar added this pull request to the merge queue Aug 27, 2026

sergey-shandar removed this pull request from the merge queue due to a manual request Aug 27, 2026

chatgpt-codex-connector Bot reviewed Aug 27, 2026

View reviewed changes

o2alexanderfedin approved these changes Aug 27, 2026

View reviewed changes

sergey-shandar added this pull request to the merge queue Aug 27, 2026

Merged via the queue into main with commit 8609666 Aug 27, 2026
19 checks passed

sergey-shandar mentioned this pull request Aug 27, 2026

emergent_testing: one normalized result for both runners #1741

Merged

Uh oh!

Uh oh!

Conversation

sergey-shandar commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this first

The change

Why the field is required

Notes on review

The remaining steps

Also here

Verification

Uh oh!

chatgpt-codex-connector Bot commented Aug 27, 2026

Uh oh!

cloudflare-workers-and-pages Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying with Cloudflare Workers

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

sergey-shandar Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

o2alexanderfedin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

sergey-shandar Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

o2alexanderfedin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sergey-shandar commented Aug 27, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented Aug 27, 2026 •

edited

Loading