Skip to content

fsm: build toRange on ascii.range instead of re-implementing it - #1575

Merged
sergey-shandar merged 2 commits into
mainfrom
claude/todo-implementation-rifq4g
Aug 15, 2026
Merged

fsm: build toRange on ascii.range instead of re-implementing it#1575
sergey-shandar merged 2 commits into
mainfrom
claude/todo-implementation-rifq4g

Conversation

@sergey-shandar

Copy link
Copy Markdown
Contributor

Implements fjs/fsm/todo/torange-ascii-range.md (deleted here).

The bug

toRange read the second character of its argument unconditionally:

export const toRange = s => {
    const [b, e] = toArray(stringToList(s))
    return range([b, e])
}

so a one-character argument destructured e to undefined and byte_set.range computed one(undefined - b + 1). Reproduced on main before touching anything:

toRange('a')  → RangeError: The number NaN cannot be converted to a BigInt

fjs/text/ascii already owns "two-character string → inclusive Range" and handles the one-character caserange returns [f0, f0] when s.length === 1. The duplicate was worse than the original it duplicated.

What

export const toRange = compose(asciiRange)(range)

toRange('a') is now the singleton set 1n << 97n, and fjs/fsm drops its stringToList import along with the hand-rolled destructuring.

toUnion — not moved to byte_set

The TODO's second task was "move toUnion to fjs/types/byte_set (or inline it into the proof)". The first option is the wrong one, and specifically so: toUnion takes a string, so byte_set would have to import fjs/text/utf16 to read one — adding a fjs/text dependency to a types leaf. That is the same layering inversion #1566 removed from this very module pair a few PRs ago, when toRangeMap stopped naming SortedSet<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.mjs also sheds union, one and empty from its byte_set import.

This drops toUnion from fjs/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.

toRange stays 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 tsc clean.
  • fjs test: 2793 pass, 0 fail.
  • npm run cov: fjs/fsm/module.f.mjs at 100% lines/branches/functions.
  • New toRange proof cases cover the regression directly — 'az' against byteSetRange([0x61, 0x7a]), 'a' against both byteSetRange([0x61, 0x61]) and one(0x61), and '\0' at the low boundary.
  • Differential check against main on 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:

  • BREAKING CHANGES: fsm: toRange no longer throws on a one-character argument — it is the singleton range, via text/ascii's range. toUnion is 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

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
@chatgpt-codex-connector

Copy link
Copy Markdown

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
@cloudflare-workers-and-pages

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 e4f744f Commit Preview URL

Branch Preview URL
Aug 15 2026, 01:33 AM

@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. 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 prepack from a clean tree → 0 (both trees).
  • npm test: 2725 pass / 0 fail on 104bfa34, 2729 pass / 0 fail here — +4, the four new toRange cases, nothing else moved.
  • Broken-link sets are identical to main's (diff empty), so removing fjs/fsm/todo/torange-ascii-range.md stranded nothing — no other file references it.
  • Emitted fjs/fsm/module.f.d.mts diff vs main is exactly: toRange's new doc block, and toUnion's declaration gone. toRange keeps (s: string) => ByteSet — no widening to any, no elided. The @module header survives emit. @import { Fold } is still earned (three @type tags below still use it), so removing toUnionOp left 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.

@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 15, 2026
Merged via the queue into main with commit 06b7fcb Aug 15, 2026
18 checks passed
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