fix!: upgrade to Fable 5.11 and fix chardata handling in string bindings - #123
Merged
Conversation
Fable 5.8.1 qualifies generated Erlang module names with the OTP app name
(test_maps.erl -> fable_beam_test_test_maps.erl). That silently broke
test_runner.erl's lists:prefix("test_", ...) discovery: it found zero tests
and still printed "All tests passed!".
Discover tests by exports (any module exporting test_*/0) instead of by
module name, and refuse to report success on an empty suite -- a discovery
bug must never again look like a green build.
Also point `dev=true` at ../Fable (capital F); the lowercase path never
resolved on a case-sensitive filesystem.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…/to_graphemes
The OTP string module returns *chardata* -- an iolist, or a charlist of
codepoints -- from these functions, not a binary. F# strings are Erlang
binaries, so the raw result was structurally wrong even though it printed
plausibly: string:pad("hi", 5) is [<<"hi">>,32,32,32], which compares
unequal to <<"hi ">>.
This was invisible because Assert.AreEqual never raised on the BEAM (fixed
upstream in Fable), so no assertion in the suite could fail.
An iolist is still valid chardata, so these values worked when handed
straight back to io:format or a Cowboy body -- OTP flattens them. They only
broke when compared, pattern-matched, or stored as a binary, which is
exactly what the F# type claims you are holding. Flatten at the binding
boundary with unicode:characters_to_binary/1, chosen over iolist_to_binary/1
because it also handles charlists and codepoints above 255.
BREAKING CHANGE: string:reverse and every string:pad arity return chardata,
so they cannot be bound through [<ImportAll>] -- there is nowhere to convert
the result. They move from the `str` interface to the module's typed-API
section, which is exactly where the file already says functions with
non-trivial Erlang return values belong. Module-level F# functions cannot
overload, so the pad arities become distinct names:
str.reverse s -> reverse s
str.pad (s, n) -> pad s n
str.pad (s, n, dir) -> padDir s n dir
str.pad (s, n, dir, c) -> padWith s n dir c
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Once assertions could actually fail, the tests got checked for the first
time too -- and these six were wrong. Every binding they cover was correct;
each was reproduced by hand in erl before being touched.
binary.longest_common_prefix: the common prefix of "foobar"/"foobaz"/"fooqux"
is "foo", i.e. 3. Only the first two share "fooba". The test asserted 4.
erlang send/receive: a nullary DU case compiles to a bare atom (`ping`); only
a case with fields becomes a tagged tuple (`Data 42` -> `{data, 42}`). The
test sent the 1-tuple `{ping}`, which never matched the generated receive
clause, so it sat out the full 1000ms timeout and took the None branch.
jsx labels: `labels` is a *decoder* option. jsx:is_json/2 does not accept it
and simply answers false, so is_json could never show the option took effect
-- the tests' own premise was false. Tellingly, the one test in the group
that used decode and inspected the key type passed all along. The four now
decode and assert on the resulting key type, and ExistingAtom asserts the
badarg rejection that gives it its name.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
spike/scriptorium runs Scriptorium (Nib assertions + the Quill runner) against real Fable.Beam bindings: `just spike`. There is no test_runner.erl and no [<Fact>] marker. Quill's runner is the [<EntryPoint>], which Fable emits as main:main/1; it halts the VM with the suite's exit code, so a failing test fails the recipe. A deliberate failure produces a real message, a diff, a clickable source link (CallerFilePath survives the BEAM) and exit code 1. That matters because neither failure mode this repo just hit is expressible under Scriptorium: Nib's assertions raise from F# itself rather than relying on the backend lowering Assert.AreEqual, and Quill registers tests explicitly instead of rediscovering them by naming convention. Scriptorium is consumed from the sibling checkout for the spike; a real migration would use the published packages (Scriptorium.Quill pulls in Nib, Ink and Parchment). BROKEN-BINDINGS.md documents the 14 failures the vacuous suite was hiding: 8 broken bindings, 6 broken tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
5.11.0 is the first release containing both upstream fixes this branch depended on: Assert.AreEqual/NotEqual now raise on the BEAM, and erased types ([<Erase>] Pid/Atom/Shutdown) no longer emit dangling reflection calls (fable-compiler/Fable#4775, #4776). The stock tool now compiles and runs the suite green -- `just test` and `just spike` no longer need `dev=true`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dbrattli
marked this pull request as ready for review
July 17, 2026 20:43
… constant binary:referenced_byte_size reports the size of the underlying memory a binary references, which OTP's docs call "a hint for optimization, not exact". It returned 5 on the OTP 25 dev box but 40 on CI's OTP 27 (and 256 for a shell literal), so the old `equal 5` assertion was not portable. Assert the only guarantee the function actually makes -- it references at least what it contains: referenced_byte_size s >= byte_size s >= 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dbrattli
added a commit
that referenced
this pull request
Jul 18, 2026
Every bug BROKEN-BINDINGS.md documented is fixed and merged, so a file named "broken bindings" that lists nothing broken is stale and misleading. The forensic account stays in git history and the #123 writeup. Its one reusable lesson not already in the guide — assert the invariant a function guarantees, not an implementation-defined value that varies by OTP release (the binary:referenced_byte_size 5-on-25 / 40-on-27 case) — is now an anti-pattern in BINDINGS-GUIDE.md. The chardata bug class it described is already covered by the Dual-API section; that section no longer links the deleted file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dbrattli
added a commit
that referenced
this pull request
Jul 18, 2026
…riants (#128) * feat(beam): add BeamChardata type and raw chardata string variants Fable.Beam is increasingly used to author OTP code, not just call into it (cf. fable-compiler/Fable#4771, pinning a module's Erlang name for OTP behaviours). In that world the chardata the OTP string/uri_string modules return is a feature, not an impurity: it is valid anywhere a binary or iodata is accepted (io:format, gen_tcp:send, Cowboy bodies), and keeping intermediate results unflattened is the idiomatic way to build output and flatten once at the I/O boundary. Add BeamChardata (erased unicode:chardata(), in the BeamList/BeamMap family) with ofString/toString, and a `*Raw` variant of every string function that returns chardata: reverseRaw, padRaw, padDirRaw, padWithRaw, replaceFirstRaw, replaceAllRaw, composeQueryRaw. The default string-returning functions are unchanged; the Raw variants skip the unicode:characters_to_binary flatten and hand back BeamChardata. This supersedes the commented-out ImportAll stubs left in String.fs: reverse and pad returned chardata and so never could be ImportAll members, but they are fully bound now (string form + raw form), so the stubs become one-line pointers to the working functions. Not BeamList<char>, as first sketched: only reverse yields a charlist; pad and replace yield iodata (nested binaries and integers), which BeamList<char> would misrepresent. One honest chardata type covers all of them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(bindings): document the dual-API (F#-friendly default + *Raw) convention Write down the convention BeamChardata introduces: when an OTP function returns a BEAM-native form (chardata, or a raw Erlang list) that is efficient for authoring Erlang but awkward in F#, bind it twice — a flattened/ref-wrapped default under the plain name, and a `*Raw` variant returning the native type (BeamChardata / BeamList<'T>). Includes a "where it applies" table: the chardata axis is done (string:reverse/ pad/replace, uri_string:compose_query); the raw-list axis (maps:keys/values/ to_list, string/binary/re:split, proplists:get_keys) and io_lib:format are identified candidates, not yet implemented. Also cross-referenced from the Quick Reference, the F#->Erlang type table, and the type-choice decision tree. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(beam): apply the dual-API convention to raw lists and add io_lib Implements the BINDINGS-GUIDE "Dual API" candidates surfaced in the survey. io_lib (new module IoLib.fs): io_lib:format is the canonical build-a-string- from-a-template function and returns chardata. Bound as `format` (-> string, flattened) + `formatRaw` (-> BeamChardata), mirroring the string module. Raw-list variants: every binding that ref-wraps a native Erlang list into an F# array now also offers a `*Raw` returning the native BeamList<'T>, for BEAM-side use without the new_ref round-trip: maps: keysRaw, valuesRaw, toListRaw string: splitFirstRaw, splitAllRaw binary: splitFirstRaw, splitAllRaw re: splitRaw, splitWithRaw, splitMPRaw, splitPartsRaw proplists: getKeysRaw The default array-returning members are unchanged. BeamList lives in Lists.fs, which the fsproj compiled *after* Maps.fs, so Maps could not reference it. Lists and Maps are mutually independent (no circular use), so Lists.fs now compiles first; Maps.fs opens Fable.Beam.Lists. String.fs and Re.fs likewise gain the open. +13 tests (380 total): raw lists assert the native form matches the array member (length / structural equality), and io_lib asserts the format/formatRaw round-trip. Guide's "where it applies" table updated to reflect what's done. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: retire BROKEN-BINDINGS.md; fold its durable lesson into the guide Every bug BROKEN-BINDINGS.md documented is fixed and merged, so a file named "broken bindings" that lists nothing broken is stale and misleading. The forensic account stays in git history and the #123 writeup. Its one reusable lesson not already in the guide — assert the invariant a function guarantees, not an implementation-defined value that varies by OTP release (the binary:referenced_byte_size 5-on-25 / 40-on-27 case) — is now an anti-pattern in BINDINGS-GUIDE.md. The chardata bug class it described is already covered by the Dual-API section; that section no longer links the deleted file. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(string): correct the pad/reverse stub comments The old comments claimed these "can't be ImportAll members" because they return chardata. That is only true for a `string` return (ImportAll codegen can't insert the unicode:characters_to_binary flatten). A BeamChardata-typed ImportAll member needs no conversion and would compile fine. State the real reason: the string form needs a flatten, so it is a module function; the raw form is exposed as `*Raw` to keep raw variants uniform across the library, not because an interface member is impossible. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: RFC for reducing per-module API surface count (follow-up) Captures the concern raised in #128 review: each binding module exposes up to three surfaces — the ImportAll interface (xxx.*, tupled), module helpers (curried), and the *Raw variants. The *Raw layer is justified; the wart is the arbitrary interface-vs-module split, driven by whether a return needs an Emit wrapper rather than by anything a caller sees. Repo-wide, predates the dual-API work. Records options (unify onto curried module functions / onto the interface / document only), recommends the curried-module direction prototyped on String first, and notes the breaking, repo-wide impact (Cowboy + synapse). Hand-off for a follow-up session; no code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Upgrades to Fable 5.11 and fixes the issues that upgrade surfaced.
Requires Fable ≥ 5.11.0, the first release with the two BEAM fixes this depends on
(#4775, #4776):
Assert.AreEqual/NotEqualnow raise on the BEAM, and erased types (Pid/Atom/Shutdown) nolonger emit reflection calls that fail to resolve. Verified against the stock tool.
What this fixes
Assert.AreEqualnow raises on the BEAM. Fable's BEAM backend previously lowered it to anequality expression whose boolean result was discarded, so assertions couldn't fail. Fixed upstream
in #4775; the same gap affected Fable's own BEAM test suite. With assertions working, this repo's
suite surfaced 14 real failures, all fixed here — it's now 367/367, and it can demonstrably fail
(it reported 14, then 6, then 0 as these landed).
Chardata handling in the
stringbindings (8 fixes). The OTPstringmodule returns chardata— an iolist, or a charlist of codepoints — not a binary. F# strings are binaries, so
string:pad("hi", 5)was[<<"hi">>,32,32,32], which compares unequal to<<"hi ">>. Affectsreplace_all,replace_first, the threepadarities,reverse,to_graphemesandcompose_query. Now flattened at the binding boundary withunicode:characters_to_binary/1, chosenover
iolist_to_binary/1because it also handles charlists and codepoints above 255.Worth knowing for anyone who has been using these: an iolist is still valid chardata, so these values
behaved correctly when passed straight to
io:format,file:writeor a Cowboy body — OTP flattensthem. The mismatch only bites when the value is compared, pattern-matched, or stored as a binary.
Six test corrections. The bindings they covered were all correct; each was reproduced by hand in
erlbefore being touched.longest_common_prefixgenuinely returns 3 for that input. The receivetest sent
{ping}where a nullary DU case compiles to the bare atomping. The four jsxlabelstests used
jsx:is_json/2, which doesn't accept thelabelsdecoder option and so could neverobserve it — they now decode and assert on the resulting key type.
Breaking change
string:reverseand everystring:padarity return chardata, so they can't be bound through[<ImportAll>]— there's nowhere to convert the result. They move to the module's typed-API section,which is where
String.fsalready says "functions with non-trivial Erlang return values" belong.Module-level F# functions can't overload, so the arities become distinct names:
str.reverse sreverse sstr.pad (s, n)pad s nstr.pad (s, n, dir)padDir s n dirstr.pad (s, n, dir, c)padWith s n dir cPre-1.0 (
-rc.x), andsynapsedoesn't use any of them, so no known downstream breakage.Test-runner hardening
Fable 5.8+ qualifies generated module names with the OTP app name (
test_maps→fable_beam_test_test_maps), which broketest_runner.erl'stest_-prefix discovery. It nowdiscovers tests by exports (any module exporting
test_*/0), so it's robust to naming changes,and it refuses to report success on an empty suite — a discovery bug can't look like a green
build.
Scriptorium spike
spike/scriptorium(just spike) runs Scriptorium — Nibassertions + the Quill runner — on the BEAM against real bindings. No
test_runner.erland no[<Fact>]: Quill's runner is the[<EntryPoint>], which Fable emits asmain:main/1and whichhalts the VM with the suite's exit code. Failures come with a diff and a clickable source link
(
CallerFilePathsurvives the BEAM).The case for adopting it: Nib's assertions raise from F# itself rather than depending on the backend
lowering
Assert.AreEqual, and Quill registers tests explicitly rather than rediscovering them bynaming convention — so neither of the two problems above is expressible. Migrating
test/off thehomegrown runner is the natural follow-up, deliberately not in this PR.
Verification
🤖 Generated with Claude Code