Skip to content

Migrate 10 modules to .f.mjs under the stage-1 TypeScript-to-mjs migration - #1484

Merged
sergey-shandar merged 15 commits into
mainfrom
claude/convert-f-ts-to-f-mjs-7
Aug 11, 2026
Merged

Migrate 10 modules to .f.mjs under the stage-1 TypeScript-to-mjs migration#1484
sergey-shandar merged 15 commits into
mainfrom
claude/convert-f-ts-to-f-mjs-7

Conversation

@sergey-shandar

@sergey-shandar sergey-shandar commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Migrates ten modules from authored TypeScript (.f.ts) to JSDoc-typed
JavaScript (.f.mjs) under the stage-1 TypeScript-to-mjs migration
(todo/migrate-typescript-to-mjs.md), following the types.ts convention
introduced by #1481/#1483 for modules with a non-trivial public type
surface:

  • fjs/types/object — splits OptionalMap, RequiredMap, StringMap,
    Entry, OneKey, NotUnion, SingleProperty into types.ts.
  • fjs/crypto/hmac — no public types; runtime-only conversion.
  • fjs/path — no public types; runtime-only conversion.
  • fjs/sul/level/literal — splits Word, EncodeState, Level,
    PipelineState, LiteralToVec into types.ts.
  • fjs/types/sorted_set — splits SortedSet<T> into types.ts.
  • fjs/types/byte_set — splits ByteSet into types.ts; the
    module-private Byte typedef is kept _-prefixed (_Byte) in
    module.f.mjs since JSDoc typedefs are always exported.
  • fjs/types/string_set — splits StringSet into types.ts.
  • fjs/media/json/serializer — no public types; runtime-only
    conversion.
  • fjs/sul/id — splits Id into types.ts.
  • fjs/sul/level/hash — splits Add, EncodeState into types.ts.

Every dependent across the repo is updated to import runtime values from
module.f.mjs and types from types.ts, splitting mixed
single-statement imports into separate runtime/type-only imports where
needed.

Test plan

  • npx tsc --noEmit — clean, 0 errors, matching main
  • node ./fjs/module.ts t — all 2356 tests pass, 0 failures
  • Public type surface diffed against main via prepack — 0 removed,
    0 changed
  • No any in the emitted declarations for any migrated module
  • CHANGELOG.md updated with a **BREAKING CHANGES:** entry for each
    of the ten migrations

claude added 2 commits August 11, 2026 01:23
Splits the type-level API (OptionalMap, RequiredMap, StringMap, Entry,
OneKey, NotUnion, SingleProperty) into a sibling types.ts, and converts
the runtime helpers (at, sort, fromEntries, fromMap, isObject,
definedValues, definedEntries) to JSDoc-typed module.f.mjs, following
the convention introduced by #1481/#1483. Updates all dependents to
import runtime values from module.f.mjs and types from types.ts,
splitting mixed import statements where needed.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  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 4f5e0cc Commit Preview URL

Branch Preview URL
Aug 11 2026, 02:51 AM

claude added 2 commits August 11, 2026 01:28
Converts module.f.ts and proof.f.ts to JSDoc-typed module.f.mjs and
proof.f.mjs under the stage-1 TypeScript-to-mjs migration. Updates the
sole dependent, fjs/crypto/sign, to import from the .f.mjs specifier.
Converts module.f.ts and proof.f.ts to JSDoc-typed module.f.mjs and
proof.f.mjs under the stage-1 TypeScript-to-mjs migration. Updates the
six dependents to import from the .f.mjs specifier.

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head 04f2633 against origin/main (30e7c64). The change itself looks correct — I have no objection to the code. Three notes, one of which is about the test plan rather than the diff.

What I verified

  • npx tscclean, 0 errors.
  • node ./fjs/module.ts t — 2356 pass / 0 fail, matching main.
  • Public type surface: built main and this branch separately, ran prepack in both, diffed every emitted export type keyed by name. 0 removed, 0 changed. Four added, all _-prefixed assertion typedefs from object/proof.f.mjs — JSDoc @typedef has no non-exported form, so this is inherent to the migration, and _NodeList in patricia_trie/proof.f.d.mts is already the same shape on main. Not a new problem.
  • No any leakage in the emitted declarations for either migrated module — every export in object/module.f.d.mts and hmac/module.f.d.mts carries its full named type. Worth stating explicitly given #1478.
  • hmac's runtime logic is unchanged; the JSDoc conversion is faithful.
  • CHANGELOG has entries for both migrations.

1. The test plan's tsc claim is not accurate

npx tsc --noEmit — error count unchanged at the pre-existing baseline (105 errors, confirmed identical via git stash/git stash pop against origin/main)

There is no 105-error baseline. origin/main at 30e7c64 type-checks with 0 errors, and so does this branch — I ran both.

This is worth correcting even though the outcome is fine, because the two claims set very different bars. "Error count unchanged at 105" passes whether or not the change introduces type errors, as long as some other 105 disappear; "clean, and still clean" is the bar this repo actually holds, and the one CI enforces. Had this PR introduced errors, the stated method would not have caught them.

I tried to reproduce 105 to identify the cause and could not: not from the git stash hypothesis (untracked new files surviving a stash of tracked edits — still 0), and not from a missing @types/node (that yields 1). So I can't tell you where the number came from, only that it does not describe this repo. Suggest re-running and replacing the claim with the real result.

2. The title and summary don't mention the hmac migration

The third commit migrates fjs/crypto/hmac to .f.mjs. The title says fjs/types/object, and the summary's four bullets are all about fjs/types/object. A reviewer reading either would not know a crypto module is in scope.

Credit where due — it is in the CHANGELOG, and correctly marked breaking, so it is not undisclosed to consumers. This is only about the PR's own description. Same note I made on #1483; worth getting into the habit since the summary is what describes this in history.

3. hmac loses @param/@returns prose that the convention would keep

The conversion replaced the documented signature with a bare @type:

- * @param hashFunc - The hash function implementation to use.
- * @returns - A function that takes a key and returns another function
- * that takes a message and computes the HMAC.
+ * @type {(hashFunc: Sha2) => Reduce}

That prose was in the emitted declaration on main and is gone from it now. It didn't have to be — typed JSDoc keeps both, which is how the other migrated modules do it (99 @param uses across module.f.mjs files; types/result/module.f.mjs is a clean example):

/**
 * @template T
 * @param {T} value - The value to wrap.
 * @returns {Ok<T>} A successful result containing the value.
 */

@type is fine where there was never any prose. Here there was, and it was dropped rather than translated.


Not blocking on any of this from a correctness standpoint — the migration is sound and I'd be happy to approve once the test plan reflects a real run. CI is green.

The bare @type collapsed the documented parameter/return description
that main carries. Use @param/@returns instead, matching the JSDoc
convention used elsewhere in the migration (e.g. types/result/module.f.mjs).
@sergey-shandar sergey-shandar changed the title Migrate fjs/types/object to .f.mjs, split types into types.ts Migrate fjs/types/object, fjs/crypto/hmac, and fjs/path to .f.mjs Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review — addressed all three:

  1. The 105-error claim was wrong, and you found the real bug in my method. My working directory had ~260 stale .d.ts/.d.mts files accumulated across a long session from repeated tsc --emitDeclarationOnly runs on earlier PRs. Since this repo's tsconfig.json has no include/files, tsc auto-includes every .ts/.mts file under the tree — so those leftover declaration files were silently polluting every subsequent --noEmit check, including the git stash/git stash pop comparison, which doesn't touch untracked files and so never got rid of them. Deleted them all and reran: npx tsc --noEmit is clean, 0 errors, matching main — same as your result. Good catch; that stash-based method was validating the wrong thing.
  2. Updated the PR title and summary to cover all three migrations (fjs/types/object, fjs/crypto/hmac, fjs/path).
  3. Restored the @param/@returns prose on hmac (d011033) instead of the bare @type, matching the convention elsewhere in the migration (e.g. types/result/module.f.mjs).

Also added a third module (fjs/path) to this PR since it was raised.


Generated by Claude Code

Splits the type-level API (Word, EncodeState, Level, PipelineState,
LiteralToVec) into a sibling types.ts, and converts the runtime
implementation to JSDoc-typed module.f.mjs/proof.f.mjs under the
stage-1 TypeScript-to-mjs migration. Updates the two dependents
(fjs/sul/module.f.ts, fjs/sul/id/module.f.ts) to import runtime values
from module.f.mjs and the PipelineState type from types.ts.
claude added 5 commits August 11, 2026 02:01
Splits the SortedSet<T> type into a sibling types.ts, and converts the
runtime implementation (union, intersect, has) to JSDoc-typed
module.f.mjs/proof.f.mjs under the stage-1 TypeScript-to-mjs migration.
Updates the three dependents (fjs/types/range_map, fjs/types/byte_set,
fjs/fsm) to import runtime values from module.f.mjs and the SortedSet
type from types.ts.
Splits the ByteSet type into a sibling types.ts, and converts the
runtime implementation to JSDoc-typed module.f.mjs/proof.f.mjs under
the stage-1 TypeScript-to-mjs migration. Updates the two dependents
(fjs/fsm/module.f.ts, fjs/fsm/proof.f.ts) to import runtime values from
module.f.mjs and the ByteSet type from types.ts, and fixes a doc link
in fjs/types/nibble_set/module.f.mjs.
Splits the StringSet type into a sibling types.ts, and converts the
runtime implementation (values, empty, contains, set, fromValues,
remove) to JSDoc-typed module.f.mjs/proof.f.mjs under the stage-1
TypeScript-to-mjs migration. Updates the sole dependent,
fjs/bnf/ll1/module.f.ts, to import runtime values from module.f.mjs
and the StringSet type from types.ts.
Converts module.f.ts and proof.f.ts to JSDoc-typed module.f.mjs and
proof.f.mjs under the stage-1 TypeScript-to-mjs migration. Updates the
two dependents (fjs/media/json/module.f.ts, fjs/djs/serializer/module.f.ts)
to import from the .f.mjs specifier.
Splits the Id type into a sibling types.ts, and converts the runtime
implementation to JSDoc-typed module.f.mjs/proof.f.mjs under the
stage-1 TypeScript-to-mjs migration. Updates the four dependents
(fjs/sul/module.f.ts, fjs/sul/proof.f.ts, fjs/sul/level/hash/module.f.ts,
fjs/sul/level/hash/proof.f.ts) to import runtime values from
module.f.mjs and the Id type from types.ts.

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Re-verified at head 493f06c against origin/main (30e7c64).

All three of my earlier notes are addressed, and two of them beyond what I asked:

  • The tsc claim now reads "clean, 0 errors, matching main" — which matches what I measure.
  • The title and summary name the migrated modules instead of just fjs/types/object.
  • hmac's @param/@returns prose is restored in the typed form the other migrated modules use (@param {Sha2} hashFunc - … / @returns {Reduce} …), so it is back in the emitted declaration rather than merely back in the source.

Verification at 493f06c

Nine modules now migrate here — types/object, crypto/hmac, path, sul/level/literal, sul/id, types/sorted_set, types/byte_set, types/string_set, media/json/serializer.

  • npx tsc — clean, 0 errors.
  • 2356 pass / 0 fail, matching main exactly.
  • Public type surface: prepack in both trees, every emitted export type diffed by name — 0 removed, 0 changed.
  • No any anywhere in the emitted declarations. Zero hits repo-wide once prose matches are excluded (the two raw hits are the words "before any symbol is seen" and "any JSON that satisfies"). Checking this properly matters given #1478, so: every export in all nine modules carries its full named type.
  • CHANGELOG has a **BREAKING CHANGES:** entry for all nine, including sul/id from the last commit.

Two non-blocking follow-ups

1. Byte becomes public without the _ marker. On main, fjs/types/byte_set/module.f.ts declared type Byte = number deliberately non-exported — it appears in the emitted .d.ts as a bare type Byte, used by has, one, range, unset. It is now /** @typedef {number} Byte */ in module.f.mjs, and JSDoc typedefs are always exported, so the emitted declaration gained export type Byte = number;.

It is the only such case — the other four additions are the _-prefixed proof assertions in object/proof.f.mjs, which are inherent to the same JSDoc limitation and already have a precedent on main in patricia_trie/proof.f.d.mts.

The repo already has the answer for a private type that public signatures need: keep it exported but _-prefixed, as sorted_list/types.ts does with _MergeReduce. Renaming to _Byte would keep the intent that the main spelling had. Cosmetic, and I would not hold the PR for it.

2. The summary is behind the branch again. It describes types/object, crypto/hmac, and path; six more modules landed after it was written. The CHANGELOG — the part consumers actually read — is complete and correct for all nine, so this is only the PR's own description, and I mention it just because it is the third time on this branch. Worth a refresh before merge since the summary is what describes this in history.

CI was green at fdfe178 and is still running on 493f06c; I verified locally at 493f06c directly.

Splits the Add/EncodeState types into a sibling types.ts, and converts
the runtime implementation to JSDoc-typed module.f.mjs/proof.f.mjs
under the stage-1 TypeScript-to-mjs migration. Updates the two
dependents (fjs/sul/module.f.ts, fjs/sul/proof.f.ts) to import runtime
values from module.f.mjs and the Add type from types.ts.

Also renames fjs/types/byte_set/module.f.mjs's private Byte typedef to
_Byte: JSDoc typedefs are always exported, so the unprefixed name
leaked into the emitted declaration where main kept it private,
following the existing _-prefix convention for private public-facing
types (e.g. sorted_list/types.ts's _MergeReduce).
@sergey-shandar sergey-shandar changed the title Migrate fjs/types/object, fjs/crypto/hmac, and fjs/path to .f.mjs Migrate 10 modules to .f.mjs under the stage-1 TypeScript-to-mjs migration Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the approval! Addressed both follow-ups in c8be1d2, plus one more module landed alongside:

  1. Renamed Byte_Byte in fjs/types/byte_set/module.f.mjs, matching the _-prefix convention sorted_list/types.ts already uses for _MergeReduce. It no longer leaks into the public declaration unprefixed.
  2. Refreshed the PR title/summary to cover all ten migrated modules (the ninth from your review plus fjs/sul/level/hash, added in the same commit).

tsc --noEmit stays clean (0 errors) and all 2356 tests pass with both changes.


Generated by Claude Code

@sergey-shandar

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8be1d2429

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

Comment thread fjs/types/byte_set/proof.f.mjs Outdated
import { every, countdown, map, toArray } from '../list/module.f.mjs'
import { stringify as jsonStringify, type Unknown } from '../../media/json/module.f.ts'
import { sort } from '../object/module.f.ts'
import { stringify as jsonStringify } from '../../media/json/module.f.ts'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the byte-set proof in TypeScript for now

Because fjs/media/json/module.f.ts has not migrated and is outside this migration group, renaming this proof to .f.mjs leaves a runtime dependency on authored TypeScript and falsely marks the proof graph as JavaScript-ready. Keep this as proof.f.ts until the JSON module migrates, or migrate that dependency first.

AGENTS.md reference: AGENTS.md:L179-L181

Useful? React with 👍 / 👎.

Comment thread fjs/types/sorted_set/proof.f.mjs Outdated
import { stringify, type Unknown } from '../../media/json/module.f.ts'
import { sort } from '../object/module.f.ts'
import { has, intersect, union } from './module.f.mjs'
import { stringify } from '../../media/json/module.f.ts'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the sorted-set proof in TypeScript for now

Because fjs/media/json/module.f.ts remains an authored TypeScript runtime dependency outside this migration group, this .f.mjs proof does not form a migrated JavaScript-only graph. Leave the proof as proof.f.ts until the JSON module migrates, or migrate that dependency in the same group.

AGENTS.md reference: AGENTS.md:L179-L181

Useful? React with 👍 / 👎.

Comment thread fjs/types/object/types.ts
Comment on lines 9 to 10
/** A record over the keys of `K`, each value possibly missing at runtime. */
export type OptionalMap<K extends string, T> = { readonly[k in K]?: T }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the mandated object-type import path

After moving OptionalMap, RequiredMap, and StringMap into this companion, AGENTS.md still requires contributors to import these types from the now-deleted fjs/types/object/module.f.ts; any future code following that repository instruction gets an unresolved import. Update the instruction to name fjs/types/object/types.ts as part of this migration.

AGENTS.md reference: AGENTS.md:L626-L629

Useful? React with 👍 / 👎.

* word-level output symbol, and `false` for Patricia trie internal merges.
* Implementations record the triple in a content-addressed store.
*/
export type Add<S> = (left: Id, right: Id, merged: Id, isSymbol: boolean, storage: S) => S

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update the SUL Add import example

Now that Add lives in this companion and fjs/sul/level/hash/module.f.ts is deleted, the public example at fjs/sul/README.md:26 still imports Add from the removed module, so copying the documented API fails module resolution. Change that example to import from ./level/hash/types.ts.

AGENTS.md reference: AGENTS.md:L188-L193

Useful? React with 👍 / 👎.

fjs/types/sorted_set/proof.f.mjs and fjs/types/byte_set/proof.f.mjs
each have a runtime dependency on fjs/media/json/module.f.ts, which is
still unmigrated TypeScript. AGENTS.md requires a proof.f.mjs's
runtime imports to target already-migrated .f.mjs modules, so revert
both proofs to proof.f.ts until fjs/media/json migrates (their
module.f.mjs implementations are unaffected and stay migrated).

Also fixes three stale doc references left pointing at deleted
module.f.ts paths after this PR's migrations: AGENTS.md's StringMap/
RequiredMap/OptionalMap guidance and definedEntries/definedValues
example now point at fjs/types/object/types.ts and module.f.mjs
respectively, and fjs/sul/README.md's Add import example now points
at fjs/sul/level/hash/types.ts.

Copy link
Copy Markdown
Contributor Author

All four Codex findings were valid — fixed in 1b80d7e:

1/2. Reverted fjs/types/sorted_set/proof.f.mjs and fjs/types/byte_set/proof.f.mjs back to proof.f.ts. Both have a runtime dependency on fjs/media/json/module.f.ts, which is still unmigrated — AGENTS.md requires a proof.f.mjs's runtime imports to target already-migrated .f.mjs modules, so migrating these proofs was premature. Their module.f.mjs implementations are unaffected and stay migrated; the CHANGELOG entries are updated to say module.f.ts specifically.
3. Updated AGENTS.md's StringMap/RequiredMap/OptionalMap guidance to point at fjs/types/object/types.ts, and its definedEntries/definedValues example to module.f.mjs.
4. Updated fjs/sul/README.md's Add import example to fjs/sul/level/hash/types.ts.

tsc --noEmit stays clean (0 errors) and all 2356 tests pass.


Generated by Claude Code

@sergey-shandar

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

/**
* JSON serializer for deterministic string output.
*

P2 Badge Update issue documents after the module renames

Beyond the corrected AGENTS/SUL examples, a repo-wide rg still finds active todo/ documents pointing at paths deleted by this migration: for example, fjs/media/json/todo/number-edge-cases.md:202 and remove-native-json.md:141 link to ../serializer/module.f.ts, while several object, path, SUL, byte-set, and string-set issues cite their removed .f.ts modules. Because todo/ is this repository's issue tracker, these links and snippets now resolve to nonexistent modules and will misdirect follow-up work; sweep all ten renamed groups to their .f.mjs/types.ts paths.

AGENTS.md reference: AGENTS.md:L10-L11

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

Comment thread CHANGELOG.md Outdated
Comment on lines +23 to +27
- **BREAKING CHANGES:** `fjs/sul/level/hash` migrates from authored
TypeScript (`.f.ts`) to JSDoc-typed JavaScript (`.f.mjs`) under the
stage-1 TypeScript-to-mjs migration, splitting the `Add`/`EncodeState`
types into a sibling `types.ts` — importers must use the `.f.mjs`
specifier for runtime values and the `types.ts` specifier for types

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Condense each migration changelog entry

Each of the ten new entries spans roughly five to eight wrapped lines and repeats implementation details about the migration and import specifiers; this exceeds the repository limit of about three wrapped lines/~250 characters per entry. Condense each entry to the user-facing breaking change and PR link, leaving the migration walkthrough in the PR description.

AGENTS.md reference: AGENTS.md:L1041-L1046

Useful? React with 👍 / 👎.

Condenses the ten migration entries in CHANGELOG.md's Unreleased
section into two short batched entries, matching AGENTS.md's ~3-line
entry guidance instead of repeating the migration walkthrough already
in the PR description.

Sweeps todo/ documents referencing paths this PR deleted
(fjs/types/object, sorted_set, byte_set, string_set, sul/id,
sul/level/hash, sul/level/literal, crypto/hmac, media/json/serializer)
to their .f.mjs/types.ts equivalents, so issue-tracker links and code
snippets resolve again.

Copy link
Copy Markdown
Contributor Author

Both findings addressed in 67fc299:

  1. Condensed the ten CHANGELOG.md entries into two short batched entries (matching AGENTS.md's ~3-wrapped-line/~250-char guidance) — the walkthrough stays in the PR description where it belongs.
  2. Swept every todo/ document that linked to a path this PR deleted (fjs/types/object, sorted_set, byte_set, string_set, sul/id, sul/level/hash, sul/level/literal, crypto/hmac, media/json/serializer) across 20 files, repointing prose references and code snippets to the .f.mjs/types.ts equivalents. Left fjs/media/json/module.f.ts references alone since that module itself is still unmigrated.

tsc --noEmit stays clean (0 errors) and all 2356 tests pass.


Generated by Claude Code

@sergey-shandar

Copy link
Copy Markdown
Contributor Author

@codex review

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at 67fc299. _Byte is done, and the two Codex rounds landed well. Everything substantive still verifies clean; two small things below, both introduced by the newest commits.

Verified at 67fc299

  • npx tsc clean; 2356 pass / 0 fail, matching main.
  • Public type surface: 0 removed, 0 changed.
  • No any as a type anywhere in the emitted declarations — zero hits repo-wide.
  • CHANGELOG condensation is right. §8.3 asks for ~3 wrapped lines and says migration walkthroughs belong in the PR description, so dropping the per-module type enumerations is what the convention wants, not a loss. The wording "the module.f.ts of fjs/types/sorted_set and fjs/types/byte_set" is a nice touch — it stays accurate after the proof reverts.

The two proof reverts are correct, and worth saying explicitly since it is a subtle rule. §2 permits module.f.mjs + proof.f.ts outright, and only allows proof.f.mjs once every authored runtime dependency outside the migration group is already .f.mjs. Both proofs import from fjs/media/json/module.f.ts, which is still TypeScript, so they could not legitimately be .f.mjs yet. I also checked the reverted files against main: the (v: any) in byte_set/proof.f.ts and the explicit stringify/str annotations are all pre-existing there, so the revert restores them rather than introducing anything. By contrast sul/level/hash/proof.f.mjs imports only .f.mjs runtime modules, so its migration is fine.

1. NodeList needs the _ prefix — same case as Byte

fjs/sul/level/hash/proof.f.mjs:7 declares /** @typedef {readonly (readonly [Id, Id, Id, boolean])[]} NodeList */. On main this was type NodeList = … in proof.f.ts, deliberately non-exported. As a JSDoc typedef it now emits into the shipped proof.f.d.mts as export type NodeList, so it is the one unmarked new public type in the whole PR.

The precedent is the closest possible: fjs/types/patricia_trie/proof.f.mjs names the identical concept _NodeList, and that is already how it ships on main. This is just the Byte_Byte fix applied to the module that arrived after it.

Minor extra reason to prefix it: NodeList is a DOM global, so it is a more collision-prone name than most to export from the package.

2. Five doc references to files this PR renamed are still stale

1b80d7e and 67fc299 both sweep stale todo/ links, but five renamed paths were missed. I diffed broken references against main to be sure these are yours and not pre-existing — main has 49 broken refs, this head has 54, and the five added are:

Stale reference Now Where
fjs/path/module.f.ts module.f.mjs fjs/cas/todo/shard-dir-helper.md:26,52
fjs/sul/id/proof.f.ts proof.f.mjs fjs/emergent_testing/todo/65y-proof-asserteq-adoption.md:31,140, todo/camel-case-proof-keys.md:14
fjs/sul/level/hash/proof.f.ts proof.f.mjs .../65y-proof-asserteq-adoption.md:31,141, todo/camel-case-proof-keys.md:15
fjs/sul/level/literal/proof.f.ts proof.f.mjs fjs/sul/todo/66m-sul-literal-level-reuse.md:71
fjs/media/json/serializer/proof.f.ts proof.f.mjs fjs/media/json/todo/remove-native-json.md:42

65y-proof-asserteq-adoption.md:140 shows the sweep was partial rather than skipped — the line already reads fjs/sul/id/module.f.mjs:19, with fjs/sul/id/proof.f.ts:1 left un-updated right beside it.

(For the avoidance of scope creep: the other 49 are pre-existing on main and not this PR's problem.)


Neither is a correctness issue and CI is green. Happy to approve once these are in — the rest of the change verifies clean.

fjs/sul/level/hash/proof.f.mjs's NodeList typedef is renamed to
_NodeList: JSDoc typedefs are always exported, and main kept this type
private, same case as the earlier Byte -> _Byte fix.

Fixes five doc references the earlier todo/ sweep missed, all pointing
at .f.ts proof/module paths this PR renamed: fjs/path/module.f.mjs
(fjs/cas/todo/shard-dir-helper.md), fjs/sul/id/proof.f.mjs and
fjs/sul/level/hash/proof.f.mjs (fjs/emergent_testing/todo/
65y-proof-asserteq-adoption.md, todo/camel-case-proof-keys.md),
fjs/sul/level/literal/proof.f.mjs (fjs/sul/todo/
66m-sul-literal-level-reuse.md), and fjs/media/json/serializer/
proof.f.mjs (fjs/media/json/todo/remove-native-json.md).

Copy link
Copy Markdown
Contributor Author

Both fixed in 4f5e0cc:

  1. NodeList_NodeList in fjs/sul/level/hash/proof.f.mjs, same fix as Byte_Byte.
  2. Fixed the five remaining stale doc references: fjs/path/module.f.mjs in fjs/cas/todo/shard-dir-helper.md; fjs/sul/id/proof.f.mjs and fjs/sul/level/hash/proof.f.mjs in fjs/emergent_testing/todo/65y-proof-asserteq-adoption.md and todo/camel-case-proof-keys.md; fjs/sul/level/literal/proof.f.mjs in fjs/sul/todo/66m-sul-literal-level-reuse.md; fjs/media/json/serializer/proof.f.mjs in fjs/media/json/todo/remove-native-json.md.

tsc --noEmit stays clean (0 errors) and all 2356 tests pass.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 67fc299f57

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

Comment thread fjs/sul/id/proof.f.mjs
import { mask } from '../../types/bigint/module.f.mjs'
import { vec } from '../../types/bit_vec/module.f.mjs'
import { compress, hashId, isHash, isRaw, level3Id, rawId } from './module.f.ts'
import { compress, hashId, isHash, isRaw, level3Id, rawId } from './module.f.mjs'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Update todo links for the renamed proofs

After these proof renames, a repo-wide search for the exact old paths still finds fjs/sul/id/proof.f.ts and fjs/sul/level/hash/proof.f.ts in fjs/emergent_testing/todo/65y-proof-asserteq-adoption.md and todo/camel-case-proof-keys.md, plus the removed literal and JSON serializer proof paths in their respective todo files. These open issue documents now direct future work and verification to nonexistent files; update all of those references to the new .f.mjs paths.

AGENTS.md reference: AGENTS.md:L337-L339

Useful? React with 👍 / 👎.

@sergey-shandar

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 4f5e0cc1ad

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

@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit cbcc7ab Aug 11, 2026
19 checks passed
@sergey-shandar
sergey-shandar deleted the claude/convert-f-ts-to-f-mjs-7 branch August 11, 2026 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants