Migrate fjs/crypto/vdf to .f.mjs - #1486
Conversation
Leaf module with no unmigrated dependents; updates the stale line references in its iterate-combinator todo doc.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 2931e58 | Commit Preview URL Branch Preview URL |
Aug 11 2026, 05:28 AM |
Leaf module with no dependents.
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approving. Reviewed at 37df907 against main (e9e34c6). Two modules — fjs/crypto/vdf and fjs/bnf/token_symbol. Nothing to flag.
The 1024-bit modulus is intact
crypto/vdf is the one place in this migration where a silent digit change would be both catastrophic and invisible, so I did not settle for a token diff. Against main's compiled output the module looks different, but the only difference is numeric separators: TypeScript's emitter strips _ from 0xf2346eae06a23388_2814ff16…, while the authored .mjs keeps the grouping exactly as main's source wrote it. Discount separators and the two are identical.
Then I checked the values rather than the text — imported both module versions and compared:
p equal: true (1024 bits)
sloth.p equal: true
quadRes agree: true
modSqrt agree: true
eval(50)(12345) equal: true
verify agrees: true -> true
So the modulus, the quadratic-residue test, the modular square root, and a full eval/verify round-trip all match main. bnf/token_symbol is token-identical outright.
Everything else
npx tscclean; 2356 pass / 0 fail.- Public API unchanged on both axes — 0 removed / 0 added / 0 changed for exported type aliases and for exported const signatures. (I started checking the second of those after
eofslipped past the alias-only check on #1485; nothing here.) - No
anyas a type in any emitted declaration. - Markdown links: 144 broken on
main, 144 here — none added — and 0 label/target mismatches.crypto/vdf's owntodo/doc was updated as part of the change rather than left for review. - Both new
types.tsfiles carry the@moduleheader with the blank line, so it survives into the emittedtypes.jsinstead of being swallowed by the following import. - Both proofs import only
.f.mjsruntime modules, soproof.f.mjsis legitimate under §2. - CHANGELOG has a correctly-scoped
**BREAKING CHANGES:**entry for each, in the short §8.3 form.
One nice detail worth naming: the updated line references in todo/iterate-combinator.md are accurate, not approximate. module.f.mjs:29-37 lands exactly on repeatSeq's JSDoc and body, and 51-57 covers both call sites (squareLoop and modSqrtLoop). Stale line numbers are the easiest thing to hand-wave in a rename; these were actually recomputed.
CI is still running on 37df907; I verified locally at that commit.
|
@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: 37df907c32
ℹ️ 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".
| duplicateName: () => { encoding(['a', 'b', 'a'] as const) }, | ||
| unregisteredName: () => { encoding<string>(['a']).encode('b') }, | ||
| duplicateName: () => { encoding(['a', 'b', 'a']) }, | ||
| unregisteredName: () => { encoding(/** @type {readonly string[]} */ (['a'])).encode('b') }, |
There was a problem hiding this comment.
In the migrated proof, this inline cast replaces an explicit generic argument with a type assertion, so the fixture's initializer is no longer checked against the intended readonly string[] declaration in the repository's preferred way. Declare a separately annotated test value and pass it to encoding; this preserves the deliberate widening needed to call encode('b') without introducing an unsafe cast.
AGENTS.md reference: AGENTS.md:L572-L589
Useful? React with 👍 / 👎.
| @@ -1,4 +1,4 @@ | |||
| import { sloth, p } from './module.f.ts' | |||
| import { sloth, p } from './module.f.mjs' | |||
There was a problem hiding this comment.
When this proof is renamed to .f.mjs, fjs/crypto/vdf/README.md:28 still directs readers to proof.f.ts. A repo-wide filename search confirms that path no longer exists in this directory, so the test-vector documentation should be updated to reference proof.f.mjs as part of the rename.
Useful? React with 👍 / 👎.
token_symbol/proof.f.mjs now declares a separately annotated const instead of an inline JSDoc cast, per AGENTS.md's preferred pattern. crypto/vdf/README.md pointed at the deleted proof.f.ts.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. 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". |
3 participants
Summary
fjs/crypto/vdf/module.f.ts(andproof.f.ts) from authored TypeScript to JSDoc-typed.f.mjs, pertodo/migrate-typescript-to-mjs.md.Slothtype into a siblingtypes.ts.types/prime_field,types/nullable, andtypes/bigint; nothing else in the repo importscrypto/vdf.module.f.tsline references infjs/crypto/vdf/todo/iterate-combinator.mdto match the new file and line numbers.Test plan
npx tsc --noEmitcleannode ./fjs/module.ts t— 2356/2356 passGenerated by Claude Code