Recognize .f.mjs in proof discovery and coverage tooling - #1422
Conversation
Authored `.f.mjs` is the FunctionalScript extension a module moves to once the current parser and compiler accept it. Before the first rename, the test and coverage tooling has to treat it like `.f.ts`, otherwise migrating a module silently drops its proofs and its coverage rows. - `shouldLoad` matches `.f.mjs`, so a migrated module (and its internal `proof` export) stays in bulk-load discovery. Ordinary `.mjs` files remain opt-in through the `proof.mjs` convention. - `npm run cov` and `deno task cov` include `module.f.mjs` alongside `module.f.ts`. - `fjs/ci/deno/module.f.ts` exports `coverageInclude` as the single source of the generated Deno coverage filter, with a proof that fails if `.f.mjs` support is removed; `fjs/ci/proof.f.ts` asserts against the same constant instead of a duplicated literal. The checked-in workflow is regenerated. - `AGENTS.md` §3.2 and the `CONTRIBUTING.md` summary apply mandatory proof coverage to both authored extensions and document the mixed `module.f.mjs` / `proof.f.ts` layout used during migration. The issue's two runtime fixtures do not land here: a `proof.f.ts` cannot import a `module.f.mjs` until `allowJs`/`checkJs` are on, and enabling those alone breaks `npm run prepack` with TS5055 — the repeatable-emission work owned by `f-mjs-package-support.md`. The issue is updated to record that dependency, the assert-helper question for an internal `.f.mjs` proof, and the resulting ordering between the two prerequisites. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimQDf1MdP9oPskJj7nfrn
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimQDf1MdP9oPskJj7nfrn
| */ | ||
| export const shouldLoad = (s: string): boolean => | ||
| s.endsWith('.f.ts') || s.endsWith('.f.js') || | ||
| s.endsWith('.f.ts') || s.endsWith('.f.js') || s.endsWith('.f.mjs') || |
There was a problem hiding this comment.
Make it symmetrical, accept:
- All FunctionalScript files: ends with:
.f.ts,.f.mts,.f.jsand.f.mjs, - All unpure JavaScript/TypeScript proofs: ends with:
proof.ts,proof.mts,proof.jsandproof.mjs.
There was a problem hiding this comment.
Done in 60a534f — shouldLoad is now the two symmetrical families you described, and the doc comment states them as such:
- FunctionalScript modules, loaded whatever the file name:
.f.ts,.f.mts,.f.js,.f.mjs - impure JavaScript/TypeScript, loaded only under the
proofname:proof.ts,proof.mts,proof.js,proof.mjs
The proof now covers both families exhaustively, including the negative side for every impure extension (module.ts / module.mts / module.js / module.mjs are all rejected), and the allFiles discovery proof gained a .f.mts entry.
I left the coverage filters (npm run cov, deno task cov, the generated Deno CI step) on module.f.ts + module.f.mjs, since those are the two authored implementation extensions in fjs/fsc/README.md. Say the word if you'd like .f.mts there too.
Generated by Claude Code
Review feedback: state the rule as two symmetrical families instead of an ad-hoc list. FunctionalScript modules are loaded whatever the file name (`.f.ts`, `.f.mts`, `.f.js`, `.f.mjs`); impure JavaScript/TypeScript is loaded only under the `proof` name (`proof.ts`, `proof.mts`, `proof.js`, `proof.mjs`). This adds `.f.mts` to discovery and covers both families exhaustively in the proofs, including the negative side for every impure extension. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JimQDf1MdP9oPskJj7nfrn
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
My original item was wrong twice. It proposed adding `--test-coverage-include=**/module.f.mjs`, which #1422 already added, and it blamed the include globs for the vacuous report -- but those only filter which files appear in the report, so they cannot make a run that executed nothing report something. The cause is discovery: node --test's default patterns don't match proof.f.mjs / module.f.mjs, and the only file that does match is fjs/emergent_testing/all.test.ts, where behaviour is Node-version dependent -- 0 tests on v23 per review, while on v22.22.2 it discovers that file and runs the suite through it. Rewritten around that, with the version split recorded so the next person pins one. Also qualifies the "continue upward" checkbox: the scenarios are authored TypeScript proof source, so the bare [x] read as contradicting the new section that says so. Reported by o2alexanderfedin, who re-measured every other number in the section and found them accurate.
Implements the tooling half of
fjs/emergent_testing/todo/f-mjs-test-and-coverage.md(P1).
Authored
.f.mjsis the extension a FunctionalScript module moves to once thecurrent parser and compiler accept it. Before the first rename, the test and
coverage tooling has to treat it like
.f.ts— otherwise migrating a modulesilently drops its proofs and its coverage rows while CI stays green.
Changes
shouldLoad(fjs/dev/module.f.ts) matches.f.mjs,so a migrated
module.f.mjsand its internalproofexport stay inbulk-load discovery. Ordinary
.mjsfiles remain opt-in through the existingproof.mjsconvention. Covered by an extendedshouldLoadproof and a newvirtual-filesystem
allFilesproof.npm run covnow passes--test-coverage-includeforboth
**/module.f.tsand**/module.f.mjs.deno task covand the generated CI step use.*module\.f\.(ts|mjs).fjs/ci/deno/module.f.tsexportscoverageIncludeas thesingle source of that filter. New
fjs/ci/deno/proof.f.tsfails if.f.mjssupport is removed from it, and
fjs/ci/proof.f.tsnow asserts against thesame constant instead of a duplicated literal.
.github/workflows/ci.ymlisregenerated from the generator.
AGENTS.md§2/§3.1/§3.2 and the matching summary inCONTRIBUTING.mdapply mandatory 100% proof coverage to both authoredextensions and document the mixed
module.f.mjs/proof.f.tslayout usedduring incremental migration, plus the dependency-closure rule a
proof.f.mjsmust satisfy.What is not in this PR, and why
The issue also asked for two runtime fixtures. Both turned out to depend on
fjs/ci/todo/f-mjs-package-support.md:proof.f.tsimporting amodule.f.mjsfailsnpx tscwithTS7016while
allowJsis off. TurningallowJs/checkJson makesnpx tscpass,but
npm run prepack(tsc --NoEmit false) then fails withTS5055because authored
.mjsbecomes both an input and a JavaScript emit target —exactly the repeatable-emission work owned by the package-support issue.
proofinside a.f.mjsmodule has no assert helper it mayimport:
AGENTS.md§3.3 requiresassert/assertEqfromfjs/asserts/module.f.ts, but the dependency-closure rule forbids authored.f.mjsfrom importing a relative.f.tsmodule.Rather than duplicate the package-support work here, the issue file is rewritten
to cover just the remaining fixtures, marked
blockedon package support, withboth obstacles and a proposed resolution for the assert-helper question recorded.
f-mjs-package-support.mdandfjs/fsc/README.mdare updated so the orderingbetween the two prerequisites is stated in one direction everywhere.
Verification
npx tsc— clean.npm test— 2290 pass, 0 fail.npm run ci-update— regenerated workflow matches the committed one.node --test --experimental-test-coveragewith both include flags — accepted,report unchanged for
.f.tsmodules.Deno is not installed in this environment, so
deno task covwas not executed;the filter change is covered by the generator proof instead.
Generated by Claude Code