Per-API issues for the casts the cleanup deliberately left - #1590
Conversation
`todo/inline-type-casts.md` ends with 36 sites where the cast overrides a real type mismatch, and says each needs an issue against the API it papers over rather than a different cast syntax. This opens those issues for the 20 that share a cause, and records that the remaining one-offs do not. - `do_` has nowhere to put a type parameter, so the three generic effect constructors are cast while their non-generic neighbours in the same file are annotated declarations. - Six `step`/`okStep` continuations cast their way to the operation union the caller wants, hiding any operation the runner cannot interpret. - `memoryOperationMap()` is not assignable to `ToAsyncOperationMap<O>`, and that cast is the exact hazard AGENTS.md describes: it stops each handler being checked against `O`. - Keyword and operator collections typed over `string` mean `has` cannot narrow to a `JsToken` kind, so four token constructions are cast. - `btree/find` casts every tuple it builds or indexes. - `mockRun`'s operation map reaches for `Parameters<typeof mockRun<…>>[0]`, which is what a call that cannot infer its type arguments looks like. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | 6c6b12e | Commit Preview URL Branch Preview URL |
Aug 15 2026, 07:01 PM |
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Reviewed just 7338ce8 (the rest is #1589). Sites and arithmetic hold: 19 of the 20 cited casts exist verbatim at the cited file:line, the six per-issue counts sum to exactly 20, the audit table has exactly 36 "cast overrides" rows against 74 total matching the reason summary, nothing double-counted, all relative links resolve. Causes check out against source — Func<O> genuinely has no type-parameter slot and its neighbours really are annotated declarations; step's signature matches the quote verbatim; the ToAsyncOperationMap<O> passage says what's claimed. npm test → 2828 pass / 0 fail.
One inaccuracy, in fjs/js/todo/token-kind-narrowing.md: the stated cause — the collections holding those strings are typed over string — holds for the two djs sites (keywordSet is ReadonlySet<string>), but not for fjs/js/tokenizer/module.f.mjs:262. There keywords is /** @type {const} */, so kind already carries the literal union; the cast is needed because _KeywordToken excludes true/false/null/undefined as separate variants, so { kind } won't distribute over JsToken. Retyping the collection won't remove that cast, so "and the four casts go" overreaches.
Minor: fjs/cas/evo/module.f.mjs:456 is really 466 (inherited from #1589's table, not introduced here).
…de/cast-followup-api-todos
…dant
`github-code-quality` flagged two unused variables in `types/nominal/proof`.
Following that up found the removals there had deleted the proof itself, and a
declaration-emit diff against `main` found four more removals that had changed
the published API. All ten type-checked; none were safe.
The API regressions, found by emitting `.d.mts` before and after:
- `effects/node` `createServer`: `<O extends Operation>(listener:
RequestListener<O>) => Effect<O | CreateServer, Server>` became
`(...payload: never) => Effect<Operation, never>`
- `effects/node` `log`/`error`: `Console` became the structural type
- `cas/evo` `emptyCache`: `Cache` became `{ bySubject: {} }`
- `types/range_map`: `RangeMapArray<T>` became `[T, number][]` — a purely
functional library publishing a mutable array type
Three are now annotated declarations; `createServer` goes back to an inline
cast, which is what do-generic-operation-signatures.md is about.
`types/nominal/proof` demonstrates, per branding strategy, whether `<` compiles
between two branded values. The casts *are* the demonstration, and a brand is
unconstructible by design, so the declaration form rejects what the inline cast
accepted — `const a = {}` compared against `const b = {}` proves nothing. In a
proof about types the annotation is the test, and "does it still compile"
cannot see it being deleted.
Also clears the 36 `@import` entries the cleanup orphaned, so
`--noUnusedLocals` reports exactly what it did on `main`, and records both new
checks in the audit's method.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
# Conflicts: # todo/inline-type-casts.md
88e8d79
into
claude/cast-type-annotations-refactor-hq8l8o
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
The fjs/cas/evo/module.f.mjs 456 → 466 reference is fixed (came in via the base branch).
The token-kind-narrowing.md point is still open — the file is byte-identical to 7338ce8; none of the new commits touch it. Re-proved it empirically this time: deleting the cast at fjs/js/tokenizer/module.f.mjs:262 gives TS2322 whose message shows kind is already the 49-member literal union, not string, and fails to distribute over _KeywordToken | _TrueToken | _FalseToken | _NullToken | _UndefinedToken. So retyping the collection won't remove that cast, and "the four casts go" still overreaches for that site.
Everything else in this PR's own content is unchanged and still correct: six new todos intact, links resolve, per-issue table still sums to 20. npm test → tsc clean, 2838 pass / 0 fail.
3 participants
Stacked on #1589 — its last commit is the diff to review here.
todo/inline-type-casts.mdends with 36 sites where the cast overrides a real type mismatch, and says each needs an issue against the API it papers over rather than a different cast syntax. This opens those issues for the 20 that share a cause, and records that the remaining one-offs do not.fjs/effects/todo/do-generic-operation-signatures.mdFunc<O>has nowhere to put a type parameter, somemCreate,memReadandallare cast while their non-generic neighbours in the same file are annotated declarationsfjs/effects/todo/step-continuation-operation-union.mdstep/okStepcontinuations cast their way to the operation union the caller wants, which hides any operation the runner cannot interpretfjs/effects/node/todo/async-operation-map-assignability.mdmemoryOperationMap()is not assignable toToAsyncOperationMap<O>fjs/js/todo/token-kind-narrowing.mdstring, sohascannot narrow to aJsTokenkindfjs/types/btree/todo/find-path-item-typing.mdbtree/findbuilds or indexesfjs/emergent_testing/todo/mockrun-parameters-inference.mdParameters<typeof mockRun<…>>[0]as an argument typeTwo of these are worth reading past the summary. The
ToAsyncOperationMapcast is the exact hazardfjs/AGENTS.mddescribes — a cast around a value handed to a generic parameter stops each handler being checked againstO, so a drifted shape is absorbed rather than reported. ThemockRunone has the same shape for the same reason.The
token-kind-narrowingissue also argues against the obvious fix: a type predicate would remove those four casts and AGENTS.md permits one where the alternative is a cast — but only where the predicate body is the structural check defining membership. ASetlookup asserting a union is not that, so the issue proposes typing the collection instead.The audit doc gains a table linking all six, and states plainly that the remaining one-offs — a deliberately wrong value in a negative test, a nominal brand,
Function→NodeProgram— have no shared cause and are listed in the site table rather than given an issue each.🤖 Generated with Claude Code
https://claude.ai/code/session_01TQcZKuWSt2rEZrCj1jVZih
Generated by Claude Code