Migrate the last 12 authored .f.ts files to .f.mjs — completes stage 1 - #1505
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 09f54fd | Commit Preview URL Branch Preview URL |
Aug 12 2026, 10:31 PM |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
The rest parameter's type is the tuple itself, not its element type.
main's `const m: TemplateType = '<html>Hello</html>'` is a checked annotation; migrating it to an inline `/** @type {T} */ (expr)` cast made it an `as` assertion, which is what the proof exists to avoid. Verified by negative control: with a non-matching string the cast form compiles clean, while the declaration form reports TS2322 as main does. Per AGENTS.md section 6 -- prefer annotating a separate const declaration over casting an expression inline.
The typed tokenizeString const becomes a JSDoc @type annotation, and the JsonToken type import becomes an @import in a leading block. Doc references updated in 66e-parser-container-stack-bookkeeping.md and stringify-sorted-canonical.md; the latter's line citation was already off by one before this migration (line 13 was blank, the binding was on 14), so it is corrected to the binding's actual new line rather than merely shifted.
sergey-shandar
changed the title
|
Every other completed check on this commit is green ( A re-run should clear it — I don't have permission to trigger one ( Generated by Claude Code |
sergey-shandar
changed the title
The explicit generic instantiation match<ReadFile, IoResult<Vec>>(...) becomes an inline @type cast on the operation map, following the precedent set for match/do_ in fjs/effects/proof.f.mjs. The list.empty/list.nonEmpty instantiations cannot rely on the callee's contextual type -- O widens to Operation rather than never -- so they become checked @type {List<never, IoResult<Vec>>} declarations rather than casts. The seven pre-existing `as` assertions carry over as inline @type casts. Verified the match cast is load-bearing, not inert: renaming the map's command key and returning a wrong type from the handler each produce tsc errors, while the unmutated file compiles clean.
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Reviewed at 3ce4c78f (the head moved from f2657444 to 3ce4c78f mid-review — the
fjs/effects/node/proof.f.ts migration commit was added; everything below was re-run at
the new head).
One real problem, everything else verified clean.
todo/proof.f.mjs introduces an unprefixed public type TemplateType
todo/proof.f.mjs:1
/** @typedef {`<html>${string}</html>`} TemplateType */On main, TemplateType was a non-exported TypeScript type alias, and
todo/proof.f.d.ts contained only export declare const proof. As a JSDoc @typedef
it is emitted as a public alias — the new todo/proof.f.d.mts begins:
export type TemplateType = `<html>${string}</html>`;package.json files includes **/*.d.mts, so this ships in the tarball. It is an
implementation-only type used only inside this one file, so per AGENTS.md §6.2
("Name implementation-only JSDoc typedefs with a leading _") and
fjs/fsc/README.md#private-jsdoc-typedefs it should be _TemplateType.
Every other @typedef in a proof.f.mjs in the repo already follows this — 14 files,
all _-prefixed (_TestOp, _Benchmark, _NodeList, _Tests, _AddOp, …). This is
the only exception.
A whole-tree, file-aware diff of exported type names between emitted declarations on
origin/main and this head returns exactly one line:
> todo/proof.f.d.mts::TemplateType
Worth noting for future rounds: bin/extract.mjs-style scans that only walk fjs/
miss this, because the file is under todo/.
Verified clean
npx tsc --noEmit→ exit 0.npm run prepackfrom a freshly cleaned tree → exit 0 (both passes).npm test→ 2495 pass / 0 fail, exactly matchingorigin/main(2495) with a clean
tree before each run.- §2 precondition. Walked the transitive runtime import graph of all four migrated
proofs with an acorn parse of every reachable module: 47 files, zero relative.ts
runtime edges, no missing targets. All four were eligible to become.f.mjs. - Token equivalence. Compared
main's compiled.f.jsagainst this PR's authored
.f.mjsby acorn AST comparison (position/rawstripped), not a regex normalizer —
these files contain'//singleline comment'and'/* multiline comment */'string
literals that a comment-stripping regex would corrupt.fjs/js/tokenizer/proof,fjs/media/json/parser/proof,todo/proof— AST-identical.fjs/effects/node/proof— identical except the two deliberate hoists of
listEmpty()/listNonEmpty(...)intoconst chunks(JSDoc cannot express
explicit generic instantiation at a call site). Both hoisted expressions are pure,
so the evaluation-order shift is unobservable; re-inlining them by hand makes the
file AST-identical to main's compiled output.- Negative-controlled in both directions: one-token mutations on the PR side and on the
main side (including a mutation inside a'/* … */'string literal) all report
AST-DIFFERENT.
- The
match(...)cast keeps its checking./** @type {OperationMap<ReadFile, IoResult<Vec>>} */ ({ readFile: … })is a cast, so I tested rather than assumed:
renaming the key toreadFileXgivesTS2352(Property 'readFile' is missing) plus
TS7006onpath, and changing the body took(42)givesTS2352
(Type 'number' is not comparable to type 'Vec'). Key names, parameter contextual
typing and return values are all still checked. The remaining new casts
(@type {Dir},@type {never},@type {{ code?: unknown }}) replaceascasts
one-for-one. - The
@typeannotations are annotations, not casts. Mutation-probed all of them on
a cleaned tree (no emitted.d.mtsaround to change resolution):
'<htmlHello</html>'→ TS2322 against the template-literal type;
withoutMetadareturning the whole token → TS2322JsTokenWithMetadatanot assignable
toJsToken;tokenizeStringreturningnumber[]→ TS2322;const chunks = 5→
TS2322 againstList<never, IoResult<Vec>>. - Public surface. Exported const signatures: 0 added, 0 removed, 0 changed — nothing
widened toany. Exported types: the single addition above. - Link check. 141 broken relative markdown links, an identical set to
origin/main— the rename introduced none. The threeproof.f.tsreferences in
fjs/djs/todo/66e-…md,fjs/media/json/todo/stringify-sorted-canonical.mdand
fjs/effects/node/todo/ornotfound-combinator.mdwere all updated, and the only
remainingproof.f.tsmentions in the tree are the four CHANGELOG entries, which
correctly name the old path. - CHANGELOG. Four entries, all under
## Unreleased, all with the
**BREAKING CHANGES:**prefix (a.f.ts→.f.mjsspecifier change is breaking), each
linking only/pull/1505. No released section touched. @importplacement in a standalone leading JSDoc block matches the established shape
for proof files (fjs/cas/proof.f.mjs,fjs/crypto/sha2/proof.f.mjs,
fjs/types/sorted_set/proof.f.mjs, …), each followed by the load-bearing blank line.
Not verified: coverage — npm run cov reports a vacuous 100.00 over 0 tests in this
environment on main too, so it tells us nothing about this PR either way. Rust gates and
the generator round-trip were not run; nanvm-lib/ and fjs/nanvm/ are untouched.
The six module-local type aliases become _-prefixed JSDoc @typedefs, since a JSDoc @typedef has no non-exported form. The mockRun<RegisterMockOps, RegisterMockState>(...) instantiation becomes an inline @type cast built from the same Parameters<typeof mockRun<...>>[0] type the TypeScript source already spelled out. `as const` assertions become inline @type {const} casts, per the rule that the const form cannot be hoisted to a declaration annotation. Verified the annotations are load-bearing rather than inert: breaking a field type in the rtti event schema, passing a non-number to the reporter's summary, and renaming an op key in the mockRun map each produce tsc errors, while the unmutated file compiles clean. Doc references updated in four files. Two carried line citations that were already wrong before this migration -- remove-native-json.md cited a blank line, and lift-js-lexical-predicates cited 313-328 (the register-runner mock) for cases that actually lived at 387-402 -- so both are corrected to the content's real new location.
TemplateType was a non-exported TypeScript type alias on main, but a JSDoc @typedef has no non-exported form, so it emitted as a public `export type TemplateType` in todo/proof.f.d.mts and shipped in the tarball via package.json `files`. Renamed _TemplateType per the API visibility convention. Verified with a path-aware diff of every exported type name in every emitted declaration against origin/main: the only remaining additions are the seven _-prefixed typedefs this branch introduces, and no unprefixed public type is added or removed.
|
Good catch — fixed in I confirmed the finding rather than taking it on faith: I also reran your whole-tree check with the scan rooted at the repository rather than No unprefixed public type is added or removed. (First attempt at this comparison was against a stale baseline — cloning the local repo resolves Since your review,
Generated by Claude Code |
sergey-shandar
changed the title
|
Update on Same conclusion as before: no code change is warranted, and it needs a re-run from someone with permission (I get 403 on Generated by Claude Code |
The function-local `type _ = Assert<Equal<...>>` round-trips stay function-local as JSDoc @typedefs. A JSDoc @typedef inside a function body is function-scoped -- same-named typedefs in sibling functions do not collide -- and is not emitted into the .d.mts, so this keeps the module's public type surface identical to main and needs no hoisting or renaming of the recursive schema thunks. unwrap<T>(...) instantiations become checked @type declarations rather than casts: T appears only in return position, so the contextual type drives inference. The 30 `as const` / `as number` assertions carry over as inline @type casts, which is the only form available for @type {const}. Verified the local typedefs are still load-bearing, probing on a cleaned tree so stale emitted declarations could not intercept resolution: breaking the boolean round-trip, the asserted recursive _A shape, the asserted `list` schema tag, and the tree round-trip each produce a tsc error, with the unmutated file clean. The two sites main left unasserted remain unasserted, and the emitted declaration exports no type, as on main.
b2c88e0 to
e031c36
Compare
The 23 function-local `type _ = Assert<Equal<...>>` round-trips stay function-local as JSDoc @typedefs, so the ten function-local schema consts they name via `Ts<typeof t>` need no hoisting and the emitted declaration exports no type, as on main. funcParam/funcObj additionally carry local generic aliases and a generic arrow: `type F0<T>` becomes a `@template` @typedef and `const func = <T>(f0, f1) => ...` becomes a @template/@param/@returns block. Verified load-bearing on a cleaned tree: mutating the boolean, tuple, record, union and option round-trips, the _F0 signature, and funcObj's f1 body each produce a tsc error, with the unmutated file clean. proof-shared-asserts.md's six line citations are recomputed from the suite boundaries they bracket rather than shifted by a guessed offset.
The four typed consts become JSDoc @type annotations. This is a live proof module -- its 12 tests run in the suite -- not an inert fixture. Doc reference updated in todo-property.md.
sergey-shandar
changed the title
The runtime proofs convert directly -- no annotations to carry. The trailing index-signature guidance block cannot: it uses `declare const`, which has no JavaScript form. Per the migration doc's rule for mixed runtime/type modules, those declarations move into the sibling types.ts rather than being dropped or given an invented runtime value. They are non-exported and _-prefixed, matching the existing pin in fjs/media/json/types.ts. Verified they add nothing to the emitted declaration -- fjs/types/ts/types.d.ts still exports exactly Equal and Printer, and proof.f.d.mts exports no type, as on main. Verified the moved asserts still assert on a cleaned tree: breaking the _X0 and _X3 index-signature claims each produce a tsc error.
`declare const … : unique symbol` has no JavaScript form, so the two brand carriers move to the sibling types.ts along with the two types keyed on them, per the migration doc's rule for mixed runtime/type modules. The proof keeps the expressions themselves -- they are statement-level demonstrations of what TypeScript does and does not reject, which cannot be expressed as declarations in types.ts. The two moved types must be exported for the proof to @import them, so they are _-prefixed and private by contract; nothing else in the module's surface changes. Verified the proof still demonstrates what it claims, by uncommenting the two comparisons it documents as compile errors: `strA > strB` on a Nominal and `a < b` on the symbol-intersection brand each still report TS2469, and the file is otherwise clean.
sergey-shandar
changed the title
| /** @type {_StringKeyBranded} */ | ||
| const x = { _brand: 'NoCompare' } | ||
| // No Error | ||
| if (x < x) { } |
| const b = /** @type {_SymbolKeyBranded} */ ({}) | ||
|
|
||
| // No Error | ||
| a < b |
| const b = /** @type {_IntersectionSafeId} */ ({ value: 2 }) | ||
|
|
||
| // No Compile-time error | ||
| if (a < b) { } |
| const b = /** @type {_IntersectionSafeId} */ ({ value: 2 }) | ||
|
|
||
| // No Compile-time error | ||
| if (a < b) { } |
| /** @type {_StringKeyBranded} */ | ||
| const x = { _brand: 'NoCompare' } | ||
| // No Error | ||
| if (x < x) { } |
| /** @type {_StringKeyBranded} */ | ||
| const x = { _brand: 'NoCompare' } | ||
| // No Error | ||
| if (x < x) { } |
| const b = /** @type {_SymbolKeyBranded} */ ({}) | ||
|
|
||
| // No Error | ||
| a < b |
| const b = /** @type {_SymbolKeyBranded} */ ({}) | ||
|
|
||
| // No Error | ||
| a < b |
| } | ||
| { | ||
|
|
||
| const a = /** @type {_SymbolIntersectionBranded} */ (/** @type {any} */ (undefined)) |
| { | ||
|
|
||
| const a = /** @type {_SymbolIntersectionBranded} */ (/** @type {any} */ (undefined)) | ||
| const b = /** @type {_SymbolIntersectionBranded} */ (/** @type {any} */ (undefined)) |
|
Answering the ten They are all correct as observations and all intentional: None of it is introduced by this PR. Every flagged construct is verbatim from
What this PR changed in that file is only how the types are spelled: the two I'd rather not "fix" these — deleting the comparisons or the two unused consts would delete what the proof asserts. I did verify the split preserved it, by uncommenting the two comparisons the file documents as compile errors: If the noise is unwanted, the options are a scoped suppression for Generated by Claude Code |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Re-reviewed at 57546e81. The head moved twice while I was working: I started at
f4d7e413 (eight modules), and by the time I went to post, 32b51288 and
57546e81 had added fjs/types/ts and fjs/types/nominal, taking the scope to
ten modules. Everything below was re-run from scratch at 57546e81, so the
numbers are for the current head, not for f4d7e413.
Previous finding: resolved
The one finding from my review at 3ce4c78f — todo/proof.f.ts's new
@typedef TemplateType being unprefixed, and therefore emitting as a public
export type TemplateType in the shipped todo/proof.f.d.mts — is fixed. It is
now _TemplateType, and the CHANGELOG entry explains why the rename was needed
("it was never exported, but a JSDoc @typedef has no non-exported form").
I re-ran the whole added-type surface at this head rather than just re-checking
that one file. Against origin/main (3859e7d4), with npm run prepack in both
trees, the emitted declarations gain exactly nine exported type aliases and lose
none:
+ _Event (emergent_testing/proof)
+ _TestReporter (emergent_testing/proof)
+ _RegisterMockState (emergent_testing/proof)
+ _RegisterMockOps (emergent_testing/proof)
+ _RegisterRunner (emergent_testing/proof)
+ _RegisterTestOp (emergent_testing/proof)
+ _TemplateType (todo/proof)
+ _SymbolKeyBranded (types/nominal/types.ts)
+ _SymbolIntersectionBranded (types/nominal/types.ts)
All nine carry the _ prefix required by AGENTS.md §6.2. No unprefixed public
type is added anywhere in the ten modules. On the const axis, 893 exported const
signatures on both sides, with the only difference being
emergent_testing/example.f.d.ts::* → example.f.d.mts::* — same five names,
byte-identical signatures, no silent widening to any.
The 23 Assert<Equal<...>> round-trips in types/rtti/validate/proof.f.mjs and
their counterparts in parse are declared inside function bodies and, as the
CHANGELOG claims, do not reach the emitted declarations — none of them appear in
the extracted type surface.
One thing worth fixing
fjs/types/ts/types.ts:28 — the import type { Assert } sits in the middle of
the file, after two exported declarations.
AGENTS.md §4 lays out the required order for a TypeScript file: module JSDoc,
blank line, then type-only imports, then runtime import groups, then
declarations. Here the import lands at line 28, after export type Equal
(line 8) and export type Printer (line 16), because the moved index-signature
block was appended verbatim to the end of the file and its import came along with
it.
This is the only types.ts in the repo that does it — I checked all of them:
$ for f in $(git ls-files '*types.ts'); do <first-decl vs last-import check>; done
fjs/types/ts/types.ts firstDecl line 8 lastImport line 28
The precedent the CHANGELOG entry cites, fjs/media/json/types.ts, keeps all six
of its import type lines at the top, ahead of every declaration. Moving this one
line up next to the @module block would match it.
Nothing breaks as a result — tsc is clean, prepack is clean, and I confirmed
the @module header still survives declaration emit (grep -c '@module' returns
1 in each of fjs/types/ts/types.d.ts, fjs/types/nominal/types.d.ts, and
fjs/media/json/types.d.ts). It is a layout nit, not a defect.
What I verified
Gates. npx tsc --noEmit exit 0. npm run prepack exit 0 from a freshly
cleaned tree (both passes). npm test: pass: 2495, fail: 0, total: 2495,
exactly matching origin/main in a separate clean worktree — a behaviour-
preserving migration should match, and it does.
§2 precondition. I walked the transitive runtime import graph out of all ten
migrated files rather than assuming it. 59 runtime modules reachable from the
first eight, 6 from the two added later; zero relative .ts runtime edges in
either walk. Every .ts edge that remains is type-only (import type /
@import) into a types.ts, which §2 allows.
Runtime equivalence. For each of the ten, I compared main's compiled
.f.js against this PR's authored .f.mjs by acorn AST comparison (positions,
raw and comments stripped). Nine come back byte-identical at the AST level:
IDENTICAL emergent_testing/proof js/tokenizer/proof media/json/parser/proof
IDENTICAL types/rtti/parse/proof types/rtti/validate/proof
IDENTICAL emergent_testing/example todo/proof
IDENTICAL types/nominal/proof types/ts/proof
The tenth, fjs/effects/node/proof.f.mjs, differs in exactly the two places the
CHANGELOG describes: listEmpty<never, IoResult<Vec>>() and
listNonEmpty<never, IoResult<Vec>>(...) are hoisted into
/** @type {List<never, IoResult<Vec>>} */ const chunks because JSDoc has no
inline call-type-argument form. Both hoists move a pure call earlier in the same
statement sequence; nothing observable changes.
I negative-controlled the comparator both ways before trusting any IDENTICAL,
including a one-token mutation inside a '/* multiline comment */' string
literal in js/tokenizer/proof.f.mjs — the case that defeats regex-based
normalizers. All three controls reported DIFFERENT at the mutated node:
DIFFERENT js/tokenizer/proof "/* multiline comment */" -> "/* multilineX comment */"
DIFFERENT media/json/parser/proof sort -> sortX
DIFFERENT todo/proof '<html>Hello</html>' -> '<html>Hellox</html>'
DIFFERENT types/nominal/proof asNominal(123) -> asNominal(124)
Casts still check. The migration replaces a lot of as T with
/** @type {T} */ (...), and a cast on an object literal can silently drop
contextual checking. I mutation-probed the ones that matter rather than assuming;
every probe produced the error it should, and each was reverted afterwards:
match(/** @type {OperationMap<ReadFile, IoResult<Vec>>} */ ({...}))—
returningok('not-a-vec')gives TS2352 (Ok<string>not comparable to
IoResult<Vec>), andpath.nonExistentPropgives TS2339 onstring. The
operation map is still fully checked, including its callback parameters.mockRun(/** @type {Parameters<typeof mockRun<...>>[0]} */ ({...}))— the
inner lambdas dropped their explicit parameter annotations and now rely on the
cast for contextual typing.s.nonExistentPropin both theawaitand the
allreducer gives TS2339 on_RegisterMockState, so they are contextually
typed, notany.writeEvent(['summary', pass, fail, 'x'])gives TS2322 — the_Eventshape is
enforced through the@typedef.- The function-scoped
Assert<Equal<...>>typedefs are live, not decoration:
flipping the first one inparse(boolean→number) gives
proof.f.mjs(64,34): error TS2344: Type 'false' does not satisfy the constraint 'true', and flipping a late duplicate-named one invalidate
(number | undefined→number | null) gives the same at (271,34). Neither
shadowing nor position weakens them. - The block moved into
fjs/types/ts/types.tsis still checked there:
_X3'sbigint | undefined→bigintgives TS2344 attypes.ts(60,19). - In
types/nominal/proof.f.mjs, uncommentingto(userId2A)gives TS2345
(nominal brands still distinct),_ForbiddenCompare<number>gives TS2344
against the@template {object}constraint, and uncommentinga < bon
_SymbolIntersectionBrandedgives exactly the TS2469 the surviving comment
claims. The type-level demonstrations survived the move totypes.tsintact.
Docs and packaging. linkcheck broken-link sets are identical to
origin/main (163 lines, diff empty), so no todo/*.md reference was
stranded. The eleven .md updates repoint .f.ts → .f.mjs and re-anchor the
line references; I spot-checked several (emergent_testing/proof.f.mjs:47 is
indeed the JSON.stringify line, media/json/parser/proof.f.mjs:16 the
stringify alias, rtti/parse/proof.f.mjs:13-38 the shared-helper block) and
they land correctly. No stale .f.ts reference to any of the ten survives
outside the released CHANGELOG history at line ~292, which is correctly
untouched. package.json files already covers **/*.mjs and **/*.d.mts, and
there is no exports map to update.
CHANGELOG. Ten Unreleased entries, one per migrated module, each with the
**BREAKING CHANGES:** prefix (§8.4) — correct, since every one of these is a
specifier change — and each linking only /pull/1505, no issue or todo/ link.
Released sections untouched. I checked the counts the entries assert: the
"23 function-local Assert<Equal<...>> round-trips" in validate is exactly 23,
and the fjs/types/ts entry's claim that "no public type is added or removed"
matches the emitted types.d.ts, which contains only Equal and Printer.
Not verified
npm run covreports a vacuous100.00over 0 tests on Node v23 in this
environment, and does the same onorigin/main. Coverage is unverifiable here;
I am not reporting a number either way.- Rust gates and the nanvm generator round-trip were skipped —
nanvm-lib/and
fjs/nanvm/are untouched by this PR.
Two djs source-format examples with no type annotations; `input`'s import specifier updates to ./m.f.mjs. Nothing references them -- the djs proofs compile a virtual 'input.f.js' -- so this is a rename with no consumer to update. Note they now ship in the package: `files` covers `**/*.mjs` but no `**/*.ts`, so these two were previously excluded from the tarball.
Appending the index-signature block left its `import type { Assert }` at
line 28, after two exported declarations. AGENTS.md section 4 puts
type-only imports directly after the module JSDoc block, ahead of every
declaration -- which is what the file this change cites as precedent,
fjs/media/json/types.ts, already does.
Layout only: tsc and prepack stay clean, the @module header still
survives declaration emit, and the moved _X3 assert still reports
TS2344 when broken.
|
Fixed in Checked rather than assumed: last import is line 8, first declaration line 10; Two notes on scope, since the head has moved again:
Generated by Claude Code |
|
@codex review |
![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: fd329dd343
ℹ️ 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".
| - **BREAKING CHANGES:** `fjs/djs/examples/input.f.ts` and | ||
| `fjs/djs/examples/m.f.ts` migrate from authored TypeScript to | ||
| JavaScript (`.f.mjs`) — no type annotations to convert; `input`'s | ||
| import specifier updates to `./m.f.mjs`. Being `.mjs`, they are now | ||
| covered by `package.json`'s `files` and ship in the tarball, which |
There was a problem hiding this comment.
This change adds ten separate, detailed entries for the same #1505 migration, totaling roughly 79 lines. Collapse these into one short, user-facing release note; the repository limits each change to a few wrapped lines and directs implementation walkthroughs to the PR description.
AGENTS.md reference: AGENTS.md:L1221-L1226
Useful? React with 👍 / 👎.
| @@ -1,4 +1,4 @@ | |||
| import c from "./m.f.ts" | |||
| import c from "./m.f.mjs" | |||
There was a problem hiding this comment.
Renaming this fixture to input.f.mjs leaves fjs/djs/README.md:22 linking to the deleted examples/input.f.ts, so readers following the documentation now receive a broken link. Update that link as part of the rename.
Useful? React with 👍 / 👎.
The examples rename left fjs/djs/README.md:22 pointing at the deleted examples/input.f.ts. My earlier sweep for that rename searched .mjs/.ts/ .json/.sh but not .md, which is how it slipped through; a repo-wide check of every relative markdown link to a source file now passes except one pre-existing link to fjs/types/rtti/data/, a directory that has never existed and is not a rename artifact. Also repoints an already-stale link to text/utf16/module.f.ts, migrated in an earlier PR, and its label. The eleven #1505 changelog entries are cut from a median of 494 characters to 244, per AGENTS.md's "at most a few lines, ~250 characters" -- the detail they carried is already in the PR description. They stay one-per-module: that is how all 130 existing Unreleased entries are written, each an independent breaking specifier change, many sharing a PR link.
|
Both addressed in README link (P2) — correct, and my miss.
Changelog (P1) — half agreed. You're right on length: my entries ran a median of 494 characters against AGENTS.md's "~250". They're now cut to a median of 244, max 263, with the detail living in the PR description where §8.3 says it belongs. I did not collapse them into one entry, because that would break the pattern rather than follow it.
Generated by Claude Code |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Re-reviewed at 6e84d20b (head moved twice during this pass: fd329dd3 → 6e84d20b; everything below was re-run at 6e84d20b, and the diff fd329dd3..6e84d20b is docs-only — CHANGELOG.md, fjs/djs/README.md, fjs/media/json/todo/remove-native-json.md).
Approving. The one open item from the 57546e81 round is fixed, and the title's central claim checks out.
The prior open finding is closed
fjs/types/ts/types.ts — the import type { Assert } now sits at the top of the file, directly after the /** … @module */ block and its blank line, ahead of both exported declarations (commit fd329dd3). AGENTS.md §4 ordering restored, and the header still survives declaration emit:
fjs/types/ts/types.d.ts: @module count = 1
fjs/types/nominal/types.d.ts: @module count = 1
"the last ten" — checked
Verified, and it is if anything an understatement. origin/main (3859e7d4) carries 12 authored .f.ts files; this PR migrates all 12, and
$ find . -name '*.f.ts' -not -path './node_modules/*' | wc -l
0
Nine of the twelve are literally proof.f.ts; the tenth "proof module" is fjs/emergent_testing/example.f.ts, and the last round added the two fjs/djs/examples fixtures, which are not proofs. So the count reads fine but the scope line no longer covers everything in the branch. Substantively: this closes AGENTS.md §2 Stage 1 — there is no authored implementation or proof .f.ts left anywhere in the tree.
Verification battery (all at 6e84d20b)
npx tsc --noEmit— exit 0.npm run prepackfrom a freshly cleaned tree — exit 0 (both passes).npm test—pass: 2495, fail: 0, total: 2495, exactly matchingfs-mainon3859e7d4(pass: 2495, fail: 0, total: 2495). Cleaned before testing so the emitted.f.jsdid not inflate the count.- Public surface, dual axis.
bin/extract.mjs/bin/consts.mjsover the whole tree (sanity-checked non-empty and non-blind:/todo::proofpresent in both snapshots).- Types: 514 → 523. All nine additions are
_-prefixed —_Event,_RegisterMockOps,_RegisterMockState,_RegisterRunner,_RegisterTestOp,_SymbolIntersectionBranded,_SymbolKeyBranded,_TemplateType,_TestReporter. No unprefixed publicexport typeadded, so the defect that recurred through six rounds on #1503 and once here asTemplateTypedoes not reappear. No existing type changed or was removed. - Consts: 893 → 893, zero changed signatures, nothing widened to
any. The only key movement isfjs/emergent_testing/example.f.d.ts::*→…f.d.mts::*with byte-identical signatures. - Emitted declarations for all 12 migrated modules:
elided= 0,any= 0 in every one.(i: any)intodo/proof.f.mjsis verbatim fromtodo/proof.f.tson main, not new.
- Types: 514 → 523. All nine additions are
- §2 precondition, verified rather than assumed. Every relative runtime import in all 12 migrated modules targets
.f.mjs. Repo-wide, no.mjsfile imports a.tsat runtime, and since zero authored.f.tsremain the transitive runtime graph cannot reach one. - Token/AST equivalence, main's compiled
.f.jsvs the PR's authored.f.mjs, via acorn AST comparison (positions andrawstripped, module specifier extensions normalised). 11 of 12 IDENTICAL. Negative-controlled in both directions with a one-character mutation inside the'[{"kind":"/*","value":" multiline comment "},…]'string literal infjs/js/tokenizer/proof.f.mjs— the exact literal that defeats regex normalisers — and both directions correctly reported DIFFERENT while the unmutated pair reported IDENTICAL.- The one difference is
fjs/effects/node/proof.f.mjs, and it is the disclosed hoist:listEmpty()/listNonEmpty(…)moved out of thewriteFromStreamargument into preceding/** @type {List<never, IoResult<Vec>>} */ const chunks = …declarations. Both are pure constructors, so the evaluation-order shift is inert, and the CHANGELOG names it.
- The one difference is
- Casts, mutation-probed (clean tree, so nothing resolved through emitted
.d.mts; baselinetscconfirmed clean first):@type {List<never, IoResult<Vec>>} const chunks→= 42givesTS2322. Real annotation, still checked.match(/** @type {OperationMap<ReadFile, IoResult<Vec>>} */ ({…}))with a wrongreadFilereturn givesTS2352. Value types still checked.mockRun(/** @type {Parameters<…>[0]} */ ({…}))with a wrongawaitreturn givesTS2352.
- Link check. Broken relative link set went 141 → 140 versus main: nothing stranded, and
fjs/media/json/todo/remove-native-json.md'sfjs/text/utf16/module.f.tsreference was fixed along the way. At the earlierfd329dd3head this was 141 → 142, because thefjs/djs/examplesmigration had strandedfjs/djs/README.md:22→./examples/input.f.ts;6e84d20bfixed it before I could report it. - CHANGELOG §8.3. Additions only,
Unreleasedonly, no released section rewritten,**BREAKING CHANGES:**prefix on every migration entry, and the sole link in all added lines is/pull/1505— no issue ortodo/links. The condensed entries are still accurate; I checked the tarball claim specifically, andnpm pack --dry-runat this head does shipfjs/djs/examples/{input,m}.f.mjsplus their.d.mts, which.f.tsdid not. - Rust and generator gates skipped:
nanvm-lib/andfjs/nanvm/are untouched. npm run covis unverifiable here — it reports a vacuous100.00over 0 tests onmaintoo (Node v23), so it is the environment, not this PR.
One non-blocking nit
fjs/effects/node/proof.f.mjs:22 — wrapping the operation map in a @type cast turns assignability into comparability, which keeps value-type checking (the TS2352 probe above) but drops excess-property checking. An unknown operation key now type-checks:
- on
origin/main, insertingbogusOp: 1intomatch<ReadFile, IoResult<Vec>>({…})→TS2353: Object literal may only specify known properties, and 'bogusOp' does not exist in type 'OperationMap<ReadFile, IoResult<Vec>>' - at this head, inserting it into the cast →
tsc --noEmitclean
This is a narrow loss on a four-line, one-key map in a proof, and it matches existing house practice — mockRun in fjs/emergent_testing/proof.f.ts already used as Parameters<…>[0] on main and is equally lax there, so that half is pre-existing, not a regression. Not worth holding the PR for. If you want it back, the pattern you already used for chunks in this same file restores it; I confirmed both halves compile as expected:
/** @type {OperationMap<ReadFile, IoResult<Vec>>} */
const readHelloMap = {
readFile: path => {
assertEq(path, 'hello')
return ok(vec8(0x15n))
},
}
const readHello = match(readHelloMap)That form is tsc-clean as written, and re-adding bogusOp: 1 to it brings TS2353 back.
Nothing else outstanding.
A `@type` cast checks comparability, not assignability, so it drops
excess-property checking: `bogusOp: 1` in the operation map compiled
clean, where main's match<ReadFile, IoResult<Vec>>({...}) rejected it
with TS2353.
Binding the literal to an annotated const restores that -- the same
declaration form already used for `chunks` in this file, and the form
AGENTS.md section 6 prefers over an inline cast. Verified: `bogusOp: 1`
is TS2353 again, and a wrong handler return is now TS2322
(assignability) rather than TS2352 (comparability), so value checking
is strictly stronger than before too.
Reported by o2alexanderfedin as a non-blocking nit; taking it because
it recovers a real check for four lines of change.
sergey-shandar
changed the title
|
Thanks — I took the nit rather than leaving it, in I reproduced the loss before changing anything: I left the Also updated the title — you're right that "the last ten" undercounts. It's 12 authored
Generated by Claude Code |
3 participants
Summary
Continues the stage-1 TypeScript-to-
.mjsmigration (following #1503, #1504). Ten modules, one commit each. After this PR no authored TypeScript proof or implementation module remains — only fixtures whose.tsextension is load-bearing.todo/proof.f.tsTemplateType→_TemplateTypefjs/media/json/parser/proof.f.tsJsonToken→@importfjs/js/tokenizer/proof.f.tsfjs/effects/node/proof.f.tsmatch<ReadFile, IoResult<Vec>>→@typecast on the operation map;list.empty/nonEmpty→ checked@type {List<never, IoResult<Vec>>}declarations; 7as→ inline castsfjs/emergent_testing/proof.f.ts_-prefixed@typedefs;mockRun<...>→ cast from the source's ownParameters<typeof mockRun<...>>[0]fjs/types/rtti/parse/proof.f.tsunwrap<T>→ checked@typedeclarations; 30as const/as numberfjs/types/rtti/validate/proof.f.tsfunc→@templatetypedefs and a@template/@param/@returnsblockfjs/emergent_testing/example.f.tsfjs/types/ts/proof.f.tsdeclare const) → siblingtypes.ts, non-exportedfjs/types/nominal/proof.f.tsunique symbolbrands + their types → siblingtypes.tsas_-prefixed exportsDoc references updated in twelve files; CHANGELOG.md updated under
## Unreleased.Conventions applied
/** @type {T} */above theconst), never an inline/** @type {T} */ (expr)cast — an inline cast isasand silently absorbs mismatches (AGENTS.md§6). Inline casts appear only wheremainalready usedas, or where JSDoc has no other way to instantiate a generic.typealiases stay function-local as JSDoc@typedefs. These are function-scoped (same-named typedefs in sibling functions do not collide) and are not emitted into declarations, so the tworttiproofs needed no hoisting or renaming.declare constandunique symbolhave no JavaScript form, so those declarations move to the siblingtypes.tsper the migration doc, rather than being dropped or given an invented runtime value.fjs/media/json/types.tsalready uses this shape for itsAssert<Equal<…>>pin.@moduleis not added to proofs: it belongs tomodule.*entry points, so each proof's@importtags form a standalone leading block.Test plan
npm run prepack— cleannpm test— 2495/2495 pass, matchingorigin/main.d.mtsfiles otherwise intercept resolution and make probes report false negatives). Includes thematchmap's key and return type; the rtti event schema, reportersummaryand mockRun op key; all thevalidate/parseround-trips, the_F0signature andfuncObj'sf1body; the moved_X0/_X3index-signature claims. All producetscerrors; the unmutated tree is clean.nominalstill demonstrates what it claims. Uncommenting the two comparisons the file documents as compile errors —strA > strBon aNominal, anda < bon the symbol-intersection brand — each still reportsTS2469after the split.origin/main: only_-prefixed additions.fjs/types/ts/types.d.tsstill exports exactlyEqualandPrinter; both migratedrttiproofs andexampleexport no type, as onmain.js/tokenizerbody verified byte-identical viadiffWhat remains — fixtures only
fjs/emergent_testing/scenarios/*.ts,all.ts,all.test.ts— cross-runner fixtures whose extension is load-bearing:run.shdispatches on*.pass.ts/*.fail.tsand hard-links them to_scenario.proof.ts/_all.test.ts.fjs/djs/examples/input.f.ts,m.f.ts— djs source-format examples. Nothing references them (the djs proofs use a virtual'input.f.js'), so whether they should be migrated, kept as format examples, or deleted is a call for a maintainer, not a mechanical rename.todo/samples.mdalready tracks reorganising them.