fsm: build toRange on ascii.range instead of re-implementing it - #1575
Conversation
Working notes; the PR title and description are the commit message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016HvbYkBMYWwQECL7myLhqs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016HvbYkBMYWwQECL7myLhqs
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | e4f744f | Commit Preview URL Branch Preview URL |
Aug 15 2026, 01:33 AM |
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approving. Baseline for everything below is origin/main = 104bfa34 (types/array: factor the emptiness guard out of head and tail (#1574)).
Exhaustive old-vs-new sweep
toRange's domain is ASCII, so I swept it exhaustively rather than sampling: the empty string, all 128 one-character strings, all 128×128 two-character strings, and 128 three-character strings — 16,641 inputs, comparing the returned ByteSet and the throw behaviour of main's
s => { const [b, e] = toArray(stringToList(s)); return range([b, e]) }against this PR's compose(asciiRange)(range).
domain=16641 identical=16512 divergent=129 negctrl_detected=16640
The negative control (newToRange(s) + 1n) is caught on every input that returns a value, so the comparator is live rather than trivially agreeing.
All 16,512 two- and three-character inputs are bit-identical, including the "ignore everything after the second character" behaviour. Every divergence is a one-character input or the empty string — nothing in the DRY replacement's intended domain moved:
| input | main | this PR | classification |
|---|---|---|---|
128× one-char ('\0'…'\x7f') |
throw RangeError |
the singleton range | latent bug fixed, as the todo predicted |
'' |
throw RangeError |
throw '' (ascii.at throws the string) |
incidental |
The one-character fix is exactly what fjs/fsm/todo/torange-ascii-range.md described (byte_set.range computing one(undefined - b + 1)), so deleting the todo alongside is right. The empty-string row isn't a regression in kind — both throw — but the thrown value changes from a RangeError to a bare string, which the changelog doesn't mention. Not worth holding the PR for; noting it in case a caller ever catches here.
Consumer sweep
fjs/fsm/module.f.mjs has no importer anywhere in the repo outside its own proof (the only other mention is a prose reference in types/function/operator/types.ts), and toUnion now appears only in fjs/fsm/proof.f.mjs and the changelog. So the todo's "both exports have no consumer outside fjs/fsm/proof.f.mjs" still held at this head — unlike #1522, there is no consumer here that was deliberately relying on the old non-equivalent behaviour. Moving toUnion into the proof rather than into types/byte_set is the better of the todo's two options for the reason the new comment gives: a types leaf taking a string would have to reach into fjs/text.
Gates
tsc --noEmit→ 0;npm run prepackfrom a clean tree → 0 (both trees).npm test: 2725 pass / 0 fail on104bfa34, 2729 pass / 0 fail here — +4, the four newtoRangecases, nothing else moved.- Broken-link sets are identical to main's (diff empty), so removing
fjs/fsm/todo/torange-ascii-range.mdstranded nothing — no other file references it. - Emitted
fjs/fsm/module.f.d.mtsdiff vs main is exactly:toRange's new doc block, andtoUnion's declaration gone.toRangekeeps(s: string) => ByteSet— no widening toany, noelided. The@moduleheader survives emit.@import { Fold }is still earned (three@typetags below still use it), so removingtoUnionOpleft no stale import.
Changelog
changelog/unreleased/1575.md carries **BREAKING CHANGES:**, which is right: dropping an export is a harder break than #1520's specifier change, which took the prefix. Single per-entry file, no headings, no link or PR number — matches house shape.
Implements
fjs/fsm/todo/torange-ascii-range.md(deleted here).The bug
toRangeread the second character of its argument unconditionally:so a one-character argument destructured
etoundefinedandbyte_set.rangecomputedone(undefined - b + 1). Reproduced onmainbefore touching anything:fjs/text/asciialready owns "two-character string → inclusiveRange" and handles the one-character case —rangereturns[f0, f0]whens.length === 1. The duplicate was worse than the original it duplicated.What
toRange('a')is now the singleton set1n << 97n, andfjs/fsmdrops itsstringToListimport along with the hand-rolled destructuring.toUnion— not moved tobyte_setThe TODO's second task was "move
toUniontofjs/types/byte_set(or inline it into the proof)". The first option is the wrong one, and specifically so:toUniontakes a string, sobyte_setwould have to importfjs/text/utf16to read one — adding afjs/textdependency to atypesleaf. That is the same layering inversion #1566 removed from this very module pair a few PRs ago, whentoRangeMapstopped namingSortedSet<string>. Re-adding it in the other direction would undo that.So I took the sanctioned alternative and inlined it into
fjs/fsm/proof.f.mjs, which the TODO notes is its only caller. Its JSDoc records why it lives there.fjs/fsm/module.f.mjsalso shedsunion,oneandemptyfrom itsbyte_setimport.This drops
toUnionfromfjs/fsm's exports — a breaking change on paper, though the export had no caller outside the module's own proof. The changelog entry is prefixed accordingly.toRangestays exported: it is now a one-line composition of two owned APIs and is the vocabulary fsm grammars are written in, and the TODO's "cover the one-character case" task implies keeping it reachable.Verification
npx tscclean.fjs test: 2793 pass, 0 fail.npm run cov:fjs/fsm/module.f.mjsat 100% lines/branches/functions.toRangeproof cases cover the regression directly —'az'againstbyteSetRange([0x61, 0x7a]),'a'against bothbyteSetRange([0x61, 0x61])andone(0x61), and'\0'at the low boundary.mainon seven two-character inputs ('az','AZ','09',' ','~~','aa','a0'): identical. The DFA proof's expected transition table is unchanged, which is the same result from the other end.Changelog:
fsm:toRangeno longer throws on a one-character argument — it is the singleton range, viatext/ascii'srange.toUnionis no longer exported; it was used only by the module's own proof🤖 Generated with Claude Code
https://claude.ai/code/session_016HvbYkBMYWwQECL7myLhqs
Generated by Claude Code