Propose 24 DRY / separation-of-concerns issues across fjs and nanvm-lib - #1510
Conversation
Documentation-only sweep filing new todo/ issues; no code changes. fjs findings include: json_rpc dispatch reads handlers with bracket indexing instead of object.at (prototype names reach the handler table); a missing pure Result bind (okThen) hand-rolled at ~10 sites; bit_vec front/removeFront derivable from unpackSplit; an orphaned third JSON BNF grammar in fjs/fsc; hex-digit and string-escape codecs written three times each; decimal-to-BigFloat duplicated in both tokenizers; fsm using the JSON serializer as a set key; a dead CI nix flake job; and record type/readonly deviations in effects/node types. nanvm-lib findings include: no owner for thrown error values; a missing zip_longest behind three hand-rolled dual-sequence walks; Numeric's operator algebra split across three modules; SizedIndex unreachable from IContainer::items forcing indexed loops; and two more operator impls stranded in vm/impls/. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qa71k3CG6oejRMYwfwCc8P
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
functionalscript | da5177a | Commit Preview URL Branch Preview URL |
Aug 13 2026, 06:11 AM |
sergey-shandar
changed the title
o2alexanderfedin
left a comment
o2alexanderfedin
left a comment
There was a problem hiding this comment.
Approving. I did not evaluate all 24 proposals — that is not a review, that is a design review of 24 separate issues. Instead I verified four in depth and machine-checked every path:line citation across all 24. The hit rate is high enough that I would trust the rest.
Head note: I started at 77d35a6d and the head moved to da5177ac while I was checking (two merges of main, pulling in #1507/#1508/#1509 — no proposal doc changed). Everything below is re-measured at da5177ac. Against the current main (20cd6aa9) this PR is exactly 24 files, all under todo/ directories, so §8.3 wants no CHANGELOG entry and none is present. node bin/linkcheck.mjs output is byte-identical to main, so all the new cross-references between these docs resolve.
Deep-checked (4 of 24) — the duplication is real in every one
fjs/types/array/todo/head-tail-null-guard.md — tail is at :59 and head at :72, both verbatim a => a.length === 0 ? null : uncheck…(a); splitFirst at :64-69 does route through map(…)(first(a)). The «two holdouts in a module whose house style is nullable.map» framing is accurate, and the sibling split-last-nullable-map.md it points at exists.
fjs/text/ascii/todo/hex-digit-codec.md — all five citations land: media/json/serializer:48-50, js/tokenizer:599-612, djs/tokenizer:360-366, js/tokenizer:153-154 (rangeSmallAF/rangeCapitalAF), djs/tokenizer:336. fjs/text/ascii does export digit0, latinCapitalLetterF (:162) and latinSmallLetterF (:197). The best part is not the DRY claim but the bug it noticed in passing: the djs copy's final branch really is an unguarded : cp - (latinSmallLetterA - 10), so a non-hex code point yields a garbage digit where the js copy rejects. That is worth its own issue even if the codec extraction never happens.
fjs/ci/todo/dead-nix-flake-job.md — every claim checks out: nodeNixFlakeJob (node/module.f.mjs:134-137) and nixFlakeJob (ci/module.f.mjs:49) are the same expression; grep -rn nodeNixFlakeJob fjs/ finds only its own definition, so the export really is dead; nodeJob = steps => ubuntuArm(steps) at :73, nodeMainSteps = platformNodeSteps at :139, nixJobs = nodeNixJobs at :44 are all present; basicNode (:32-35) is referenced only from fjs/ci/node/proof.f.mjs. One wrinkle for whoever implements it: «make it private» would leave basicNode with no consumer at all except the proof, so the real choice is «use it or delete it».
nanvm-lib/todo/zip-longest.md — the three walks are where it says (abs_add_vec's four-arm match (iter_a.next(), iter_b.next()), abs_sub_vec's trailing iter_b.next().is_some(), Iter::eq_by_'s identical skeleton), abs_cmp_vec does hand-roll a descending while using neither, and src/vm/bigint/mod.rs:104 carries literally // NOTE: use .index_iter in abs_* helpers. — nice catch, the codebase had already noticed half of this. grep -rn try_reduce src/ tests/ returns exactly one line, its own definition, so «zero call sites» is right, and Either is already imported in common/iter.rs as the proposal assumes. Its §5.4 citation is correct — that section is «Reuse, DRY, and separation of concerns».
Bulk check: 44 resolvable path:line citations, 2 out of range
I resolved every file:line / file:line-line reference in all 24 docs against the tree and checked the range fits the file. Two do not:
fjs/mcp/todo/casmcpserver-share-cas.mdcitesfjs/dev/update/module.f.mjs:99-106as «the in-repo model» — that file is 37 lines. The file is the right one (it does usehistory/historyStep/step), but the pattern lives at roughly:23-27.fjs/types/result/todo/ok-then.mdcitesfjs/dev/package_json/module.f.mjs:145-150— that file is 38 lines. Again the right file (thet === 'error'short-circuit is there), but at:34-36.
Both look like they were taken against a larger or older version of those two modules. Everything else in ok-then.md is exact, including all four fjs/cas/evo/module.f.mjs ranges (:396-415 really is five consecutive [0] === 'error' guards in one function, and :283-287, :495-505 land correctly) and fjs/effects/module.f.mjs:300-309 for okStep. The bare-basename shorthands in the nanvm docs (mod.rs:185-205, function/debug.rs:11, bigint/debug.rs:18-22) all resolve under nanvm-lib/src/vm/ — those are fine, my resolver just needed the prefix.
No convention conflicts found
All 24 follow the house issue shape (## title, **Priority:**, **Status:** open, Problem / Proposal / Tasks). None proposes try/catch, a node: import in FunctionalScript source, or anything else that cuts against AGENTS.md; the whole set is aimed squarely at §5.4, which is where it belongs.
Nit: the PR title says 24 and the head commit message says «Propose 19». 24 files is right.
Nothing here blocks. Filing these as todo/ entries costs nothing and each one I checked was a real, precisely located duplication.
3 participants
Documentation-only sweep filing 24 new
todo/issues; no code changes, so no CHANGELOG entry per AGENTS.md §8.3. Every finding was verified against the source (and, where noted, reproduced at runtime), and checked against the existingtodo/backlog so none duplicates an already-filed issue — each file's Related section links the nearest neighbors and states the boundary.fjs (19)
Protocol & MCP:
fjs/protocol/json_rpc/todo/dispatch-at-lookup.md(P2) —dispatchreads its handler table withhandlers[method], so wire-supplied prototype names (constructor,toString) resolve to callables; reproduced at runtime. Every other dispatcher already usesobject.atfor exactly this reason.fjs/protocol/mcp/todo/result-adapter.md— own theResult<T, string> → ToolsCallResultadapter next tookResult/errorResultfjs/mcp/todo/casmcpserver-share-cas.md— flattencasMcpServerper §6.4 and buildfileCas(sha256)(home)once instead of three timesTypes:
fjs/types/result/todo/ok-then.md(P2) — addokThen, the pureResultbind hand-rolled at ~10 sites;fjs/effectsalready ships the effectful twinokStepfjs/types/bit_vec/todo/front-from-unpack-split.md—front/removeFrontare hand-written per bit order but derivable from the injectedunpackSplitfjs/types/uint8array/todo/tovec-precomputed-bound.md—toVecprecomputes a size bound, the exact thing §5.6 forbids; its siblinglistToVecshows the fixfjs/types/bigfloat/todo/from-decimal.md— decimal literal →BigFloatis implemented twice (js and djs tokenizers), in neither module that ownsBigFloatfjs/types/byte_set/todo/torangemap-payload.md—toRangeMapbakes aSortedSet<string>DFA payload into a bitmask-algebra leaffjs/types/patricia_trie/todo/declarative-stack-fold.md—endis a right fold written as a descending index loop; index arithmetic re-implementsarray'slast/splitLastfjs/types/sorted_list/todo/tail-reduce-shadowing.md— two oppositetailReduces (keep vs. drop the tail) share one name, one shadowing the otherfjs/types/array/todo/head-tail-null-guard.md(P4) —head/tailre-inline the emptiness guard the split functions already ownText, media & parsing:
fjs/text/ascii/todo/hex-digit-codec.md— hex-digit ↔ value has no owner; three hand-rolled copies (the djs copy silently accepts non-hex input)fjs/media/json/todo/escape-table-single-owner.md— the JS/JSON string-escape table is written three times (table, range-map dispatch, switch); also flags a stale premise in issue 157fjs/fsc/todo/orphaned-json-grammar.md— a dead, unproofed third copy of the JSON BNF grammar thatbnf-grammar-single-owner's two-copy inventory would strandfjs/fsm/todo/sorted-set-key.md—fsmuses the JSON serializer as a canonical set key, transitively loading the whole JS tokenizer into DFA constructionfjs/fsm/todo/torange-ascii-range.md—toRangere-implementsascii.rangeby hand and crashes on a one-character string (RangeError, reproduced)Effects, CAS & CI:
fjs/effects/node/todo/state-types-conventions.md—Envre-rollsStringMap, two index signatures lack?(the runtime code checksundefinedthe type says can't happen),Statefields lackreadonlyfjs/ci/todo/dead-nix-flake-job.md(P4) —nodeNixFlakeJobis a dead, byte-identical duplicate of the privatenixFlakeJob, plus three no-op aliasesfjs/cas/todo/filecasoperation-duplicates.md(P5) —FileCasOperationlistsNow,Readdir,Rmtwicenanvm-lib (5)
error-constructors.md— thrown error values are ad-hoc string literals in five places with three naming conventions; propose avm/error.rsvocabularyzip-longest.md— three hand-rolled "walk two sequences to the longer end" loops (abs_add_vec,abs_sub_vec,eq_by_); addIter::zip_longest, drop the unusedtry_reducenumeric-operator-home.md—Numeric's algebra is split across three modules:Negopen-coded onAny, noFrom<Numeric> for Unpacked,Mulboxes intoAnyinside the operatorsized-index-for-refs.md—SizedIndex::index_itertakesselfby value whileIContainer::items()returns&(?Sized), so six sites fall back to0..lenindexing; also unblocksdebug-delimited-fmt-helperoperator-impl-placement.md(P4) —Add/AddAssign for StringandMul for Anysit in thevm/impls/grab-bag while their types' directories hold their other operators🤖 Generated with Claude Code
https://claude.ai/code/session_01Qa71k3CG6oejRMYwfwCc8P