Record what remains after stage 1 in the migration TODO - #1506
Conversation
Stage 1 source migration is done -- no authored .f.ts remains -- but the task list still read as though none of it had happened, and the work it was blocking was never written down as concrete steps. Marks the seven tasks verifiably complete (phantom -> types.ts, the mixed-module splits, leaf identification, proof migration, the no-JS-depends-on-TS invariant, the upward walk, and the renames), and corrects the header-loss list: fjs/common/monoid now keeps its @module through declaration emit, the other four still do not. Adds a "Remaining after stage 1" section with seven subtasks, each stated with the measurement behind it so the next person can re-check rather than re-derive: the cov globs that match nothing, the types.js resolution question and the two cleanups it gates, the scenario fixtures whose .ts extension is load-bearing, the 88 stale prose mentions across 42 files, and the one broken link that is not a rename artifact. Docs only -- no CHANGELOG entry, per AGENTS.md section 8.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 76b5489 | Commit Preview URL Branch Preview URL |
Aug 13 2026, 12:04 AM |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Documentation-only (one file, todo/migrate-typescript-to-mjs.md), so no CHANGELOG entry is
needed per AGENTS.md §8.3 and the Rust gates do not apply. I re-measured every number in the
new section against the tree at 6d7b944b. Almost all of them are exactly right — I found one
item whose diagnosis and proposed fix do not hold up.
What I verified
| claim | measured |
|---|---|
no authored .f.ts remains |
find . -name '*.f.ts' -not -path './node_modules/*' → 0 |
second tsc pass emits 96 files |
96 .js: 85 types.js (one per each of the 85 authored types.ts) + 11 under fjs/emergent_testing (10 scenarios + all.test.js) |
~800 types.ts specifiers in emitted declarations |
798 from '…/types.ts' after npm run prepack |
| pre-existing, not introduced by stage 1 | 775 at 3859e7d4 by the same regex (you quote 777 — a counting-method gap of two, the point stands) |
types.ts not in the tarball |
files = **/*.js, **/*.d.ts, **/*.mjs, **/*.d.mts; no **/*.ts |
.gitignore line 131 |
line 131 is **/*.js |
| header loss: monoid fixed, four remain | grep -c '@module' on emitted declarations: common/monoid 1; types/btree/remove, types/btree/set, types/list, types/nullable 0 |
88 stale .f.ts mentions across 42 .md |
88 / 42 exactly (resolving each mention against the tree, excluding CHANGELOG.md); top files 11 fjs/bnf/todo/proof-recognizer-and-fixtures.md, 5 fjs/fsc/README.md, 5 fjs/types/rtti/README.md, 6 this file |
AGENTS.md: 24 and 5 |
grep -c '\.f\.ts' → 24, grep -c 'import type' → 5 |
| scenario fixtures are extension-load-bearing | run.sh dispatches on *.pass.ts / *.fail.ts and hard-links to _scenario.proof.ts / _all.test.ts; runners are npm run fst, bun test, node --test, deno test |
serializable-data.md is the only non-rename broken link |
it links ../data/module.f.ts; git log --all -- fjs/types/rtti/data is empty, so the directory never existed; it is the only broken relative link pointing at a source file — every other one in the link check targets a .md |
Also: npx tsc --noEmit and npm run prepack both exit 0, npm test is 2495 pass / 0 fail, the
link check's broken-link set is byte-identical to origin/main (140 both, 0 label/target
mismatches), the #remaining-after-stage-1 anchor matches the #### Remaining after stage 1
heading, and ../fjs/ci/todo/f-mjs-package-support.md resolves.
The cov item is misdiagnosed
package.json'scovscript still passes--test-coverage-include=**/module.f.ts. No such
file exists any more, so the run reports a vacuous100.00over 0 tests […] Replace the
.f.tsglob with the.mjsforms actually in the tree (**/module.f.mjs,**/module.mjs)
and confirm the report becomes non-empty. This is a one-line change […]
Two problems:
-
**/module.f.mjsis already there. The script is
--test-coverage-include=**/module.f.ts --test-coverage-include=**/module.f.mjs; the second
glob was added in #1422 ("Recognize.f.mjsin proof discovery and coverage tooling"). So the
proposed replacement is already half-applied, and it has not made the report non-empty. -
The globs are not the cause. I ran exactly what the item prescribes:
$ node --test --experimental-test-coverage \ --test-coverage-include='**/module.f.mjs' --test-coverage-include='**/module.mjs' ℹ tests 0 ℹ …les | 100.00 | 100.00 | 100.00 |Still 0 tests, still a vacuous 100.00.
--test-coverage-includeonly filters which files
appear in the report; it cannot make a run that executed nothing report something. The 0
comes from test discovery: with no path argumentsnode --testlooks for its default
test-file patterns (*.test.*,test.*,*-test.*,*_test.*,test/**), and the repo's
proofs aremodule.f.mjs/proof.f.mjs, which match none of them. The real suite runs
through the repo's own runner —npm test→node ./fjs/module.mjs t→ 2495 tests.
scenarios/run.shis corroborating evidence: it has to hard-linkall.tsto_all.test.ts
precisely so thatnode --testwill find it.
So this is not a one-line glob fix. Restoring the signal means either giving node --test
entrypoints it will actually run, or collecting coverage through fjs's own runner. Since the
whole point of this section is that "the next person can re-check rather than re-derive", it
would be worth rewriting this item around the discovery problem — otherwise the next person
edits the globs, sees tests 0 again, and starts over.
Nit
The task "Continue upward through the runtime dependency graph […] until no authored TypeScript
implementation/proof source remains" is now [x], but fjs/emergent_testing/scenarios/*.pass.ts
are authored TypeScript that run.sh hard-links to _scenario.proof.ts — authored TypeScript
proof source. Your own new section says exactly this, so it is only the checkbox that is ahead
of the text; reading it as "within the migration group" is defensible. Mentioning it so the two
statements do not read as contradicting each other.
Everything else in this change is accurate and, unusually, re-derivable from what is written.
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.
|
You're right, and the I checked rather than just taking it: One refinement from reproducing it here. The discovery diagnosis is right, but the outcome is Node-version-dependent, so it's worth pinning rather than stating flatly:
Same root cause either way, since the proofs are Took the nit too. Generated by Claude Code |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Re-reviewed at 1a7d1c8; previous review was at 6d7b944. The delta is one file,
todo/migrate-typescript-to-mjs.md (+32/−9), and the PR as a whole is still docs-only
(git diff <merge-base> HEAD --stat → 1 file). Per §8.3 no CHANGELOG entry is due.
Both findings from the last round are resolved.
1. The cov item — resolved, and the new diagnosis holds up
Last time I said the item was misdiagnosed: it blamed the include globs, but
--test-coverage-include=**/module.f.mjs had been in the script since #1422 and running
the proposed globs still gave tests 0. The rewritten item now says exactly that, and
every claim in it checks out against the tree:
package.json'scovdoes pass both**/module.f.tsand**/module.f.mjs, and
git log -Sputs the.mjsglob at28d8b1b0— #1422. Correct.- "the only file in the tree that does match" node's default patterns is
fjs/emergent_testing/all.test.ts. Confirmed: it is the sole.ts/.jspath in
git ls-filesmatching*.test.*/*-test.*/*_test.*/test.*/test/**. - The Node-version split is real, and I could reproduce both halves:
npm run covon v23.11.0 →tests 0, vacuous100.00.- the identical invocation under v22.22.2 (
npx node@22.22.2 --test --experimental-test-coverage '--test-coverage-include=**/module.f.ts' '--test-coverage-include=**/module.f.mjs') →tests 2431 / pass 2431 / fail 0
and a full per-file report endingall files | 99.93 | 98.07 | 99.73.
npm test→Number of tests: pass: 2495, fail: 0, total: 2495. Matches the stated
2495 exactly.scenarios/run.shdoesln "$scendir/all.ts" "$allfile"withallfile=_all.test.ts.
Correct, and it is fair corroboration.
One extra data point that supports the diagnosis rather than contradicting it: on v23.11.0
the same file runs fine when named explicitly — node --test fjs/emergent_testing/all.test.ts
executes the whole suite. So it is default discovery that differs between the two
versions, not execution, which is exactly what the item says and exactly why "give
node --test entrypoints it actually runs" is the right shape of fix. The instruction to
pin the Node version is well earned.
Small, non-blocking: the parenthetical list of default patterns omits test-*.*. Nothing
in the tree matches it, so it changes nothing.
2. The [x] checkbox nit — resolved
The "until no authored TypeScript implementation/proof source remains" box now carries the
qualification. Both halves verified:
git ls-files | grep -c '\.f\.ts$'→ 0. "no.f.tsis left anywhere" is exact.fjs/emergent_testing/scenarios/*.pass.tsstill exist, andrun.shreally does
ln "$scenario" "$scendir/_scenario.proof.ts", dispatching on the*.pass.ts/
*.fail.tssuffix. So "their extension is the thing under test" is accurate, not a
rationalisation.- The cross-reference
[Remaining after stage 1](#remaining-after-stage-1)resolves —
#### Remaining after stage 1is at line 817.
The contradiction I flagged is gone; the box and the open item now agree.
Carried over, unchanged
The types.ts-specifier count is still stated as 777 at origin/main 3859e7d4, where my
re-derivation last round gave 775. That line is untouched by this delta and I did not
re-run it. It is a counting-method gap, not a wrong conclusion — the item's point (the
specifiers predate the migration) stands either way.
What I ran
npm run cov on v23.11.0 and the same flags on v22.22.2; npm test (2495 pass);
git ls-files pattern surveys; git log -S on package.json; link check against
origin/main at 76dff875 — the broken-link sets are byte-identical, not merely
equal in count. No Rust gates, no prepack, no surface diff: nothing outside todo/ moved.
Approving.
Names the cheapest candidate fix for coverage, measured rather than guessed: adding fjs/emergent_testing/all.test.ts as a path argument to `cov` gives tests 2431 / pass 2431 / fail 0 and a real per-file report on v22.22.2, and the same file named explicitly also runs on v23. Flags that 2431 against npm test's 2495 is itself a discrepancy to understand before trusting the number. Replaces the bare specifier counts with the command that produces them. Three of us have now measured this and got 800, 777 and 775 from different scopes and regexes; stating the command makes the figure re-checkable, which is the whole point of the section. With one method over the whole tree it is 801 at HEAD and 778 at 3859e7d. Adds the `test-*.*` default pattern omitted from the node --test list.
|
Thanks — your v22 run was more useful than a nit, so I followed it up in Your coverage measurement turned the item from "needs investigation" into a candidate one-liner. You showed the same invocation produces a full report on v22.22.2, and that naming the file explicitly works on v23. I tested the combination — entrypoint named as a path argument to So the item now names that as the cheapest candidate rather than leaving the fix open-ended. I also flagged something worth not papering over: 2431 here against The 777/775 gap — you were right to keep pointing at it, and I've fixed the cause rather than the digit. Three of us have now measured it and got 800, 777 and 775, purely from different scopes and regexes; mine scanned
Nothing outside Generated by Claude Code |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Re-reviewed the delta from 1a7d1c8 to 76b5489 (one commit, "Make the cov fix concrete and the specifier counts reproducible"). Still docs-only: todo/migrate-typescript-to-mjs.md is the only file in the PR, confirmed against the merge-base diff rather than the title. No CHANGELOG entry needed (§8.3).
What I re-derived in the new text, all at the PR head from a freshly cleaned tree:
npm test→pass: 2495, fail: 0, total: 2495. Matches.npm run cov -- fjs/emergent_testing/all.test.tson v23.11.0 →tests 2431 / pass 2431 / fail 0and a real per-file report endingall files | 99.93 | 98.04 | 99.73. That is exactly the fix the item proposes, and it confirms the "also runs on v23" half. I could not reproduce the v22.22.2 half — that version is not installed here, and v22.14.0 cannot run the entrypoint at all (ERR_UNKNOWN_FILE_EXTENSION ".ts", since unflagged type stripping only landed in 22.18). Nothing contradicts it.- Link check: the broken-link set is identical to
origin/main, not just equal in count. npm run prepackexits 0.
The specifier counts still do not reproduce from the command the doc now quotes
Running the quoted command verbatim after npm run prepack:
| tree | excluding node_modules |
including node_modules |
|---|---|---|
76b5489 (PR head) |
800 | 801 |
3859e7d4 |
777 | 778 |
The single extra hit is node_modules/@types/node/vm.d.ts:995 → from '../with-attrs.ts'. So the printed figures (801, 778) are the including-node_modules numbers, while the sentence says "(minus node_modules and .d.ts specifiers)". With node_modules excluded as stated, it is 800 and 777. I confirmed both twice — once with the grep as written, once with an independent Node walker over the emitted .d.ts/.d.mts files. The .d.ts-specifier half of the exclusion is a no-op: there are none in either set.
Smaller, same paragraph: of the 800 at head, 798 end in types.ts; the other two are from './module.f.ts'. Baseline is 775 of 777. So if the number is to mean literally "imports written from '…/types.ts'", it is 798 and 775 — and that is also the source of the earlier 777-vs-775 disagreement you are trying to close here: 777 was the all-.ts total, 775 the types.ts subset. Either scope is defensible; what closes it is the sentence and the command agreeing.
None of this moves the conclusion — roughly 800 such specifiers, not introduced by the source migration, up about two dozen across stage 1, still the gating question for a portable release. But making the counts reproducible is this commit's whole stated purpose, so the off-by-one is better fixed than carried into the next round.
Nit
The Node default-pattern list swapped *-test.* for test-*.*. Node has both — six patterns: **/*.test.*, **/*-test.*, **/*_test.*, **/test-*.*, **/test.*, **/test/**. I checked by dropping one file per name shape into a scratch directory on v23.11.0; all five name shapes were discovered and proof.f.mjs / module.f.mjs were not. The item's conclusion is unaffected: across git ls-files, fjs/emergent_testing/all.test.ts is still the only file in the tree matching any of the six.
3 participants
Summary
Stage 1 of the TypeScript-to-
.mjsmigration finished in #1505 — no authored.f.tsremains — buttodo/migrate-typescript-to-mjs.mdstill read as though none of it had happened (30 open tasks, 0 checked), and the work stage 1 was blocking was never written down as concrete steps. This updates the doc to match reality.Documentation only; no code touched, so no CHANGELOG entry per
AGENTS.md§8.Marks seven tasks complete
Only ones verified against the tree, not assumed:
fjs/types/phantom→types.ts, the mixed-moduletypes.tssplits, leaf identification,proof.f.ts→proof.f.mjs, the "no migrated JavaScript depends on authored TypeScript" invariant (now vacuous), the upward walk, and the renames themselves.Also corrects the header-loss list. It named five modules;
fjs/common/monoidnow keeps its@modulethrough declaration emit and four still do not (fjs/types/btree/{remove,set},fjs/types/list,fjs/types/nullable).Adds a "Remaining after stage 1" section
Seven subtasks, each written with the measurement behind it so the next person can re-check rather than re-derive:
covglobs match nothing--test-coverage-include=**/module.f.ts— no such file exists, so coverage reports a vacuous100.00over 0 teststypes.jsresolution question…/types.ts, whichfilesdoes not ship; TypeScript substitutes.d.ts, Deno does nottscpasstypes.js+ 11 scenario fixtures — so it cannot simply be deleted.gitignorerule.gitignoreline 131; blocked on the two aboverun.shdispatches on*.pass.ts/*.fail.ts; porting them deletes native-TypeScript-execution coverage rather than moving it.mdfiles name anX.f.tswhoseX.f.mjsexists;AGENTS.mdhas 24serializable-data.mdlinkfjs/types/rtti/data/, a directory that has never existed — the only broken link left that isn't a rename artifactThe ordering is deliberate:
covfirst because it is one line and restores a signal the rest want, then thetypes.jsexperiment because it gates both cleanups behind it.Test plan
npx tsc --noEmit— clean (no code changed)76dff87, the merge commit for Migrate the last 12 authored .f.ts files to .f.mjs — completes stage 1 #1505Generated by Claude Code