docs: restructure separate-private-types TODO into two stages - #1746
Conversation
Stage 1 covers the source restructuring (typedef prohibition, types.ts closure, optional private.ts and meta/module.f.mjs, dependency order, breaking migrations). Stage 2 defers the packaging cleanup: deleting generated private.d.ts at prepack and semantic package validation. Stage 1 is shippable alone because types.ts must not depend on private.ts, so a shipped private.d.ts is declaration noise covered by the existing _ leak-tolerance policy, not a semantic dependency of the public surface. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | fdfeb14 | Commit Preview URL Branch Preview URL |
Aug 28 2026, 01:09 AM |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![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: 57ccdf3e33
ℹ️ 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".
Stage 1 bans file-scope JSDoc typedefs, so the documents that prescribe creating them (fjs/fsc/README.md) or waiting for @internal/stripInternal (the blocked TODO) must be rewritten in Stage 1, not Stage 2. Only the leak-tolerance contract for emitted _ names and shipped private.d.ts survives until Stage 2 unships the last private declaration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e369f2c90b
ℹ️ 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".
Stage 2 removes only generated private.d.ts. Underscore helpers retained in types.ts by the public declaration closure and underscore constants exported from meta/module.f.mjs keep shipping in emitted declarations, so the contract that emitted underscore names are not API survives Stage 2. Narrow the Stage 2 cleanup to the private.d.ts tolerance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd40ed4b28
ℹ️ 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".
todo/migrate-typescript-to-mjs.md prescribes file-scope _ typedefs and defers to stripInternal in its migration section and visibility task, and fjs/ci/todo/f-mjs-package-support.md repeats the prescription and requires such a typedef in a future fixture. Both must be reconciled in Stage 1 for its no-conflicting-prescriptions acceptance criterion to be meetable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc7f7c848e
ℹ️ 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".
Review keeps surfacing one more document that prescribes a file-scope typedef (latest: sync-interpreter-owner.md's proposed MemoryState); an enumerated list can never be shown complete. Make the sweep a repo-wide search for such prescriptions, keep the known instances as illustrative examples, and state the acceptance criterion as search-verified. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approved. Verified at fdfeb14a4: docs-only (one file, +107/-15), npm test 3478/3478 exit 0 and tsc --noEmit exit 0 on both sides, Status: open, all 20 task boxes unchecked, all eight links and the one anchor resolve, and every new factual claim checks out — the three cited sweep targets exist and contain the described text (todo/migrate-typescript-to-mjs.md:341-361, fjs/ci/todo/f-mjs-package-support.md:163-169, fjs/effects/memory/todo/sync-interpreter-owner.md:41-47).
The two-stage split is coherent and, more to the point, stage 1's acceptance criteria at :321-346 are things a reviewer can actually apply — "verified by a repo-wide search, not by checking an enumerated list" is a definition of done, not an aspiration.
I checked the commitment I have been tracking through this document's rewrites: the task to rewrite fjs/fsc/README.md's typedef prescription and delete-or-narrow the blocked @internal todo is still a real unchecked task at :301-304, and it is still correctly open — on main, fjs/fsc/README.md:129-190 still carries the unrewritten section and todo/blocked/jsdoc-typedef-strip-internal.md still exists. Nothing dropped, nothing falsely claimed done. The "authoritative until this migration is implemented" sentence is gone, but it went in #1740 rather than here, and :33-36/:38-42 now carry the same substance.
Worth noting for anyone reading the file: #1743 was closed, not merged — fjs/ci/prepack.mjs and fjs/djs/tokenizer/private.ts do not exist on main. This restructure correctly treats that work as future stage 2 rather than describing it as shipped.
3 participants
This change reorganizes the
separate-private-types.mdTODO document to clarify that the work can be completed in two independent, shippable stages.Summary
The document previously presented all requirements as a single monolithic task. This update splits the work into:
This staging allows Stage 1 to be completed and shipped independently, with Stage 2 following as a separate change.
Key changes
private.d.tsfiles are safe to ship during Stage 1:types.tsmust not depend onprivate.ts, so no shipped public declaration semantically depends onprivate.d.ts— it is declaration noise consolidated into one file per module, covered by the existing_leak-tolerance policyfjs/fsc/README.mdinstruction to create file-scope_typedefs, and the blocked wait-for-@internal/stripInternalTODO) are rewritten or deleted in Stage 1, not Stage 2_contract as permanent:_helpers retained intypes.tsby the public declaration closure and_constants exported frommeta/module.f.mjskeep shipping in emitted declarations after Stage 2, so Stage 2 retires only theprivate.d.tstoleranceNotable details
private.d.tsfiles to ship (they're removed in Stage 2)private.d.tstolerance stays documented until Stage 2 removes the last such leak; the_contract stays documented permanentlyhttps://claude.ai/code/session_01LqeS5t2ZKkSu3chRPMXR7n