diff --git a/fjs/cas/todo/filecasoperation-duplicates.md b/fjs/cas/todo/filecasoperation-duplicates.md new file mode 100644 index 0000000000..c81bf1c764 --- /dev/null +++ b/fjs/cas/todo/filecasoperation-duplicates.md @@ -0,0 +1,23 @@ +## `FileCasOperation` lists three members twice + +**Priority:** P5 +**Status:** open + +### Problem + +`types.ts:19-22`: + +```ts +export type FileCasOperation = + | ReadBytes | Mkdir | Readdir | Access | Rename | Rm + | RandomInt | Now | CreateExclusive | WriteBytes | Stat + | Now | Readdir | Rm +``` + +`Now`, `Readdir`, and `Rm` appear twice. Harmless for a union type, but it is +copy-paste residue in a type whose doc comment enumerates the members one by +one, and it misleads a reader into hunting for a difference. + +### Tasks + +- [ ] Drop the duplicate line diff --git a/fjs/ci/todo/dead-nix-flake-job.md b/fjs/ci/todo/dead-nix-flake-job.md new file mode 100644 index 0000000000..f1cdfc6fb8 --- /dev/null +++ b/fjs/ci/todo/dead-nix-flake-job.md @@ -0,0 +1,47 @@ +## `nodeNixFlakeJob` is a dead duplicate + +**Priority:** P4 +**Status:** open + +### Problem + +`fjs/ci/node/module.f.mjs:134-137` and `fjs/ci/module.f.mjs:49` are the same +expression: + +```js +export const nodeNixFlakeJob = ubuntuArm([nixInstall, ...nodeNixVersionSteps]) +const nixFlakeJob = ubuntuArm([nixInstall, ...nodeNixVersionSteps]) +``` + +Nothing imports `nodeNixFlakeJob` — the exported copy (with a 12-line +rationale comment) is dead, and the live private copy carries a shorter, +differently-worded comment. Two descriptions of one job that must be deleted +together when the flakes are adopted. + +The same ~100 lines hold three no-op indirections: + +- `fjs/ci/node/module.f.mjs:73` — `const nodeJob = steps => ubuntuArm(steps)`, + an eta-expansion of `ubuntuArm`; +- `:139` — `export const nodeMainSteps = platformNodeSteps`, an alias + imported by `fjs/ci/module.f.mjs` while `platformNodeSteps` is also + exported; +- `fjs/ci/module.f.mjs:44` — `const nixJobs = nodeNixJobs`. + +`basicNode` (`:32-35`) is exported but referenced only by its own proof. + +### Proposal + +Delete `nodeNixFlakeJob` (moving its rationale comment onto the surviving +`nixFlakeJob`), drop the three aliases, and either use `basicNode` from +`nodeInstall`'s callers or make it private. + +### Tasks + +- [ ] Delete the dead export, keep the better comment +- [ ] Remove the `nodeJob` / `nodeMainSteps` / `nixJobs` aliases + +### Related + +- [669-ci-ubuntu-job-factory](669-ci-ubuntu-job-factory.md) — the + `ubuntu`/`ubuntuArm` factory; the flake-job pair is a separate duplication + its factory does not remove diff --git a/fjs/effects/node/todo/state-types-conventions.md b/fjs/effects/node/todo/state-types-conventions.md new file mode 100644 index 0000000000..da7334ed6d --- /dev/null +++ b/fjs/effects/node/todo/state-types-conventions.md @@ -0,0 +1,43 @@ +## Bring the node/virtual types onto the record-type rules + +**Priority:** P3 +**Status:** open + +### Problem + +Three deviations from rules AGENTS.md states explicitly: + +1. **`Env` re-rolls `StringMap` in a file that already imports it.** + `fjs/effects/node/types.ts:291-293` spells out + `{ readonly [k: string]: string|undefined }` — that is + `StringMap`, imported at `:12` and used two lines apart for + `Headers` and `Module`. +2. **Two index signatures without `?`.** + `fjs/effects/node/virtual/types.ts` — `internet: + { readonly[url: string]: Vec }` and `memoryValues: + { readonly [key: string]: unknown }`. §6.2: without `?`, TypeScript types + every access as `T` while the value can be `undefined` at runtime. The + consumer proves it — `virtual/module.f.mjs:361-364` checks + `result === undefined` on a read the type says is always a `Vec`. Both + should be `StringMap<…>`. (The recursive `Dir` in the same file is the + documented inline-form exception and stays.) +3. **`State`'s fields are all mutable.** `virtual/types.ts:28-41` — no + `readonly` on `stdout`, `stderr`, `stdin`, `root`, `internet`, `epochNs`, + `memoryNext`, `memoryValues`, `randomNext`, while every operation rebuilds + the record by spread. The mutability is unused and unenforced; `Dir` and + `_Entity` in the same file already have `readonly`. + +### Proposal + +`Env = StringMap`, `internet: StringMap`, +`memoryValues: StringMap`, and `readonly` on every `State` field. + +### Tasks + +- [ ] Replace the three inline record types with `StringMap` +- [ ] Mark `State` fields `readonly`; fix any compile fallout + +### Related + +- [node-module-layering](../../todo/node-module-layering.md) — flags the + `NodeProgramOptions.std` deviation only; these are the remaining ones diff --git a/fjs/fsc/todo/orphaned-json-grammar.md b/fjs/fsc/todo/orphaned-json-grammar.md new file mode 100644 index 0000000000..f50745b20d --- /dev/null +++ b/fjs/fsc/todo/orphaned-json-grammar.md @@ -0,0 +1,41 @@ +## A third JSON grammar copy is dead code + +**Priority:** P3 +**Status:** open + +### Problem + +`fjs/fsc/json.f.mjs` (125 lines) is a complete JSON grammar written with +`fjs/bnf` combinators — `string`/`character`/`escape`/`hex`, +`number`/`uint`/`fraction0`/`exponent0`, `object`/`array`/`member`, +`ws0`/`ws1` — duplicating `deterministic` in `fjs/bnf/testlib.f.mjs:136-196` +rule for rule. + +[bnf-grammar-single-owner](../../media/json/todo/bnf-grammar-single-owner.md) +inventories the JSON grammar as existing in exactly two places +(`fjs/bnf/testlib` and `fjs/djs/tokenizer`); this third copy is not in that +inventory, so implementing the todo as written would strand it. + +It is also dead: `fjs/fsc/bnf.f.mjs` is the only importer of `json.f.mjs`, +and nothing imports `bnf.f.mjs` (`wsModule` has zero consumers). Neither file +has proof coverage — `fjs/fsc/proof.f.mjs` imports only `./module.f.mjs`. And +`fjs/fsc` is the compiler, not a media format, so the JSON half is in the +wrong module regardless. + +### Proposal + +Either delete both files, or keep only `bnf.f.mjs`'s genuinely +FunctionalScript-specific rules (`fjs`, `lineComment`, `multiLine`, +`id`/`alpha`) and have them import the JSON half from the future +`fjs/media/json` grammar owner. Either way, add this pair to +`bnf-grammar-single-owner`'s inventory. + +### Tasks + +- [ ] Decide: delete, or rebase on the shared JSON grammar +- [ ] Update `bnf-grammar-single-owner`'s inventory and task list + +### Related + +- [bnf-grammar-single-owner](../../media/json/todo/bnf-grammar-single-owner.md) + — the two-copy inventory this pair is missing from diff --git a/fjs/fsm/todo/sorted-set-key.md b/fjs/fsm/todo/sorted-set-key.md new file mode 100644 index 0000000000..d5ed1fa41d --- /dev/null +++ b/fjs/fsm/todo/sorted-set-key.md @@ -0,0 +1,46 @@ +## Don't use the JSON serializer as a set key + +**Priority:** P3 +**Status:** open + +### Problem + +The subset construction needs a canonical key for a `SortedSet` and +reaches for a media-format serializer to get one (`module.f.mjs:19, 30, 71`): + +```js +import { stringify } from '../media/json/module.f.mjs' +const stringifyIdentity = stringify(identity) +... +const s = stringifyIdentity(set) +if (s in dfa) { return dfa } +``` + +That inverts the layering: `fjs/fsm` is generic automaton tooling, and +`fjs/media/json/module.f.mjs` imports its tokenizer, which imports the +795-line `fjs/js/tokenizer` — so building a DFA transitively loads the whole +JavaScript lexer. It also pays for full JSON string escaping on every +state-set key. [recognizer-backend](../../bnf/todo/recognizer-backend.md) +proposes generalizing exactly this subset construction, so the dependency +would propagate. + +(Also visible at `:71`: `s in dfa` reads a `StringMap` with `in` instead of +`at` from `fjs/types/object`.) + +### Proposal + +Name the operation for what it is — a canonical key for a sorted string +set — and put it where the data lives (`fjs/types/sorted_set` or +`fjs/types/string_set`), implemented as a `join` over a separator. +`fjs/fsm` then imports nothing from `fjs/media`. + +### Tasks + +- [ ] Add a canonical-key function to the sorted-set module with proof + coverage +- [ ] Convert `fjs/fsm` and drop its `fjs/media/json` import + +### Related + +- [recognizer-backend](../../bnf/todo/recognizer-backend.md) — will inherit + whichever key mechanism `fsm` uses diff --git a/fjs/fsm/todo/torange-ascii-range.md b/fjs/fsm/todo/torange-ascii-range.md new file mode 100644 index 0000000000..ea1870c5c2 --- /dev/null +++ b/fjs/fsm/todo/torange-ascii-range.md @@ -0,0 +1,42 @@ +## `toRange` re-implements `ascii.range` and crashes on one character + +**Priority:** P3 +**Status:** open + +### Problem + +```js +export const toRange = s => { + const [b, e] = toArray(stringToList(s)) + return range([b, e]) +} +``` + +`fjs/text/ascii/module.f.mjs:20-25` already owns "two-character string → +inclusive `Range`", including the one-character case. `fsm.toRange` +(`module.f.mjs:32-36`) is `compose(asciiRange)(byteSetRange)` written out by +hand — and the duplicate is worse than the original: + +``` +toRange('a') → RangeError: The number NaN cannot be converted to a BigInt +``` + +because `e` destructures to `undefined` and `byte_set.range` computes +`one(undefined - b + 1)`. `fjs/fsc/module.f.mjs:66` shows the correct +composition (`fn(asciiRange).map(codePointRange)`). + +`toUnion` (`:39-45`) — "byte set from the characters of a string" — is +likewise generic byte-set vocabulary, the same shape as `fjs/bnf`'s `set(s)` +for its own alphabet. Both exports have no consumer outside +`fjs/fsm/proof.f.mjs`. + +### Proposal + +`export const toRange = compose(asciiRange)(byteSetRange)` (or drop the +export if the proof stays the only caller); move `toUnion` to +`fjs/types/byte_set` next to `one`/`set`. + +### Tasks + +- [ ] Rebuild `toRange` on `ascii.range`; cover the one-character case +- [ ] Move `toUnion` to `byte_set` (or inline it into the proof) diff --git a/fjs/mcp/todo/casmcpserver-share-cas.md b/fjs/mcp/todo/casmcpserver-share-cas.md new file mode 100644 index 0000000000..5d7637f1fc --- /dev/null +++ b/fjs/mcp/todo/casmcpserver-share-cas.md @@ -0,0 +1,48 @@ +## Flatten `casMcpServer` and build `fileCas` once + +**Priority:** P3 +**Status:** open + +### Problem + +`module.f.mjs:81-88` is the exact shape §6.4 forbids — the second `step` +nests only so the continuation can still see `cacheKey`, which is the case +`historyStep` exists for: + +```js +export const casMcpServer = home => step( + initEvo(fileCas(sha256)(home)), + cacheKey => step( + create(uninitializedState), + sessionKey => + stdioTransport(mcpStep(casConfig)(casMcpHandlers(home)(cacheKey))(sessionKey)), + ), +) +``` + +Flattening also surfaces the real defect: `fileCas(sha256)(home)` is +constructed three times for one server — here (`:82`), in +`casMcpHandlers`' `evoToolRegistry(evo(fileCas(sha256)(home))(cacheKey))` +(`:57`), and inside `casToolRegistry` +(`fjs/mcp/cas/module.f.mjs:180-181`). It also exposes an asymmetry between +the sibling registries: `evoToolRegistry(e)` is injected with a built +`Evo`, while `casToolRegistry(home)` takes a path and builds its own +store. + +### Proposal + +`casToolRegistry(cas)(cacheKey)` to mirror `evoToolRegistry`, with one +`const cas = fileCas(sha256)(home)` at the composition root, and the body +rewritten flat with `history`/`historyStep`/`step` +(`fjs/dev/update/module.f.mjs:99-106` is the in-repo model). + +### Tasks + +- [ ] Inject a built `Cas` into `casToolRegistry` +- [ ] Flatten `casMcpServer` with `historyStep`; construct `fileCas` once + +### Related + +- [66k-cas-cli-mcp-shared-core](../../cas/todo/66k-cas-cli-mcp-shared-core.md) + — CLI-vs-MCP sharing; this issue is the intra-server construction and §6.4 + shape diff --git a/fjs/media/json/todo/escape-table-single-owner.md b/fjs/media/json/todo/escape-table-single-owner.md new file mode 100644 index 0000000000..f259a48e04 --- /dev/null +++ b/fjs/media/json/todo/escape-table-single-owner.md @@ -0,0 +1,50 @@ +## The string-escape table has three copies + +**Priority:** P3 +**Status:** open + +### Problem + +One mapping — `" \ / b f n r t` ↔ `" \ / BS FF LF CR HT`, plus `\uXXXX` — +is written three times: + +- `fjs/media/json/serializer/module.f.mjs:38-46` — `escapeTable`, a lookup + table (encode side); +- `fjs/js/tokenizer/module.f.mjs:582-591` — a range-map dispatch (decode + side); +- `fjs/djs/tokenizer/module.f.mjs:344-374` — a switch in `stringDecodeScan` + (decode side). + +[667-js-tokenizer-handler-literals](../../../js/todo/667-js-tokenizer-handler-literals.md) +§3 proposes an `escapeTo` `(letter, char)` table, but scoped inside +`js/tokenizer` only — it never mentions `djs/tokenizer`'s decoder or the +serializer's encode-side table, so the cross-module owner question stays +open. + +Also worth flagging to whoever picks up [157](../../../djs/todo/157.md): that +issue opens by asserting both tokenizers "delegate all character +classification, escape decoding, and number parsing" to `js/tokenizer`. That +is no longer true of the grammar-based DJS tokenizer, which imports only +`isKeywordToken` and re-implements escape decoding, keyword classification, +and number decoding itself. + +### Proposal + +One module owning the bidirectional simple-escape table — natural home: next +to the JSON string grammar that +[bnf-grammar-single-owner](bnf-grammar-single-owner.md) creates — consumed by +the serializer's `escapeCodePoint` and both decoders. + +### Tasks + +- [ ] Define the table once (letter ↔ code point pairs), derive encode and + decode views from it +- [ ] Convert the three sites +- [ ] Correct 157's premise when touching it + +### Related + +- [bnf-grammar-single-owner](bnf-grammar-single-owner.md) — same "one owner" + move for the grammar itself +- [667-js-tokenizer-handler-literals](../../../js/todo/667-js-tokenizer-handler-literals.md) + — the `js/tokenizer`-local half of this diff --git a/fjs/protocol/json_rpc/todo/dispatch-at-lookup.md b/fjs/protocol/json_rpc/todo/dispatch-at-lookup.md new file mode 100644 index 0000000000..a9fb14ce98 --- /dev/null +++ b/fjs/protocol/json_rpc/todo/dispatch-at-lookup.md @@ -0,0 +1,53 @@ +## `dispatch` looks up handlers with bracket indexing + +**Priority:** P2 +**Status:** open + +### Problem + +`module.f.mjs:103-107`: + +```js +/** @type {Handler | undefined} */ +const handler = handlers[method] +if (handler === undefined) { + return errorResponseOf(id)(methodNotFound) +} +``` + +`method` is untrusted wire data and `handlers` is an ordinary object, so an +inherited `Object.prototype` name resolves to a callable. Reproduced against +the real module: + +``` +{method: 'constructor'} → TypeError thrown out of dispatch +{method: 'toString'} → {"jsonrpc":"2.0","error":"o","id":1} +``` + +(the second destructures `"[object Undefined]"` character-wise). A pure +dispatcher documented to answer `-32601` instead throws out of the caller or +emits a schema-violating response. + +Every other dispatch site in the repo already uses `at` from +`fjs/types/object`, each with a comment naming this exact hazard — +`match` (`fjs/effects/module.f.mjs:344-355`), `cli/dispatch` +(`fjs/cli/module.f.mjs:36-45`), `addRevisionToCache` +(`fjs/cas/evo/module.f.mjs:143-149`). `json_rpc` is the one that reads raw. + +### Proposal + +`const handler = at(method)(handlers)`, testing `=== null`. The hand-written +`/** @type {Handler | undefined} */` annotation (needed only because the +index-signature type lies) goes away too. Add `throw`-free proofs for +`constructor` / `toString` methods answering `methodNotFound`. + +### Tasks + +- [ ] Switch the lookup to `at` and drop the annotation +- [ ] Add proofs for prototype-name methods + +### Related + +- [effectful-dispatch-skeleton](effectful-dispatch-skeleton.md) — quotes + these lines but leaves the lookup as-is; whichever lands first should carry + the fix diff --git a/fjs/protocol/mcp/todo/result-adapter.md b/fjs/protocol/mcp/todo/result-adapter.md new file mode 100644 index 0000000000..6bafb2fd11 --- /dev/null +++ b/fjs/protocol/mcp/todo/result-adapter.md @@ -0,0 +1,44 @@ +## Own the `Result` → `ToolsCallResult` adapter + +**Priority:** P3 +**Status:** open + +### Problem + +`fjs/mcp/evo/module.f.mjs:138-151` repeats the same dispatch in two tool +handlers, differing only in the ok-side rendering (`toJson` vs. identity): + +```js +result => pure(result[0] === 'error' ? errorResult(result[1]) : okResult(toJson(result[1]))) +... +result => pure(result[0] === 'error' ? errorResult(result[1]) : okResult(result[1])) +``` + +`fjs/protocol/mcp` already owns this vocabulary — it exports `okResult` and +derives `errorResult` from it (`module.f.mjs:177-187`), and +[response-constructors](../../json_rpc/todo/response-constructors.md) cites +that pair as the model of the owner exporting the whole family. The +`Result → ToolsCallResult` adapter is the missing third member. + +### Proposal + +```js +/** @type {(render: (value: T) => string) => (r: Result) => ToolsCallResult} */ +export const resultResult = render => ([tag, value]) => + tag === 'error' ? errorResult(value) : okResult(render(value)) +``` + +Both handlers become `mapStep`-shaped one-liners +(`resultResult(toJson)` / `resultResult(identity)`), and every future +`Evo`-shaped tool gets it for free. Satisfies §6.3 ("factor out what two +branches share") and its destructuring rule in one move. + +### Tasks + +- [ ] Export the adapter next to `okResult`/`errorResult` with proof coverage +- [ ] Convert the `evo_revision` / `evo_add` handlers + +### Related + +- [map-step-combinator](../../../effects/todo/map-step-combinator.md) — the + `step(e, x => pure(f(x)))` wrapper around these same sites diff --git a/fjs/text/ascii/todo/hex-digit-codec.md b/fjs/text/ascii/todo/hex-digit-codec.md new file mode 100644 index 0000000000..31365b8988 --- /dev/null +++ b/fjs/text/ascii/todo/hex-digit-codec.md @@ -0,0 +1,46 @@ +## Own the hex-digit ↔ value codec + +**Priority:** P3 +**Status:** open + +### Problem + +The mapping between a hex-digit code point and its numeric value — the three +offsets `digit0`, `latinSmallLetterA - 10`, `latinCapitalLetterA - 10` — is +recomputed in three modules, in both directions: + +```js +// fjs/media/json/serializer/module.f.mjs:48-50 (value → char) +const hexDigit = value => + fromCharCode(value < 10 ? digit0 + value : latinSmallLetterA + value - 10) + +// fjs/js/tokenizer/module.f.mjs:599-612 (char → value, range-map dispatch) +const parseUnicodeCharHex = offset => state => input => { ... input - offset ... } + +// fjs/djs/tokenizer/module.f.mjs:360-366 (char → value, ternary chain) +const digit = contains(...digitRange)(cp) ? cp - digit0 + : contains(...rangeCapitalAF)(cp) ? cp - (latinCapitalLetterA - 10) + : cp - (latinSmallLetterA - 10) +``` + +The `af`/`AF` ranges are likewise built independently in `js/tokenizer:153-154` +and `djs/tokenizer:336`. Note the djs copy's fallthrough: the last branch +assumes lowercase without checking, so a non-hex code point silently yields a +garbage digit, where the js copy has a real reject path. + +### Proposal + +`fjs/text/ascii` already owns `digit0`, `latinSmallLetterA`, +`latinCapitalLetterA` — and `latinCapitalLetterF`/`latinSmallLetterF`, which +exist for no other reason. Add: + +- `hexDigitValue: (cp: number) => Nullable` +- `hexDigitCodePoint: (v: number) => number` +- the shared digit/`af`/`AF` ranges + +and convert the three consumers. + +### Tasks + +- [ ] Add the codec pair and ranges with proof coverage +- [ ] Convert `media/json/serializer`, `js/tokenizer`, `djs/tokenizer` diff --git a/fjs/types/array/todo/head-tail-null-guard.md b/fjs/types/array/todo/head-tail-null-guard.md new file mode 100644 index 0000000000..ce58109f49 --- /dev/null +++ b/fjs/types/array/todo/head-tail-null-guard.md @@ -0,0 +1,36 @@ +## `head`/`tail` re-inline the emptiness guard + +**Priority:** P4 +**Status:** open + +### Problem + +```js +export const tail = a => a.length === 0 ? null : uncheckTail(a) // :59 +export const head = a => a.length === 0 ? null : uncheckHead(a) // :72 +``` + +`tail` is the second projection of `splitFirst` and `head` is the first +projection of `splitLast`; both re-inline the emptiness guard that +`first(a) === null` / `last(a) === null` already expresses, and both write the +same `a.length === 0 ? null : …` line by hand. In a module whose house style +routes absence through `nullable.map` (`splitFirst`, `:64-69`), these are two +holdouts writing the guard themselves. + +### Proposal + +`export const tail = a => map(([, t]) => t)(splitFirst(a))` and +`export const head = a => map(([h]) => h)(splitLast(a))` — the four accessors +become one family with the null-dispatch stated once. If the intermediate +tuple is unwanted, at minimum factor the shared shape: +`const onNonEmpty = f => a => a.length === 0 ? null : f(a)` with +`tail = onNonEmpty(uncheckTail)`, `head = onNonEmpty(uncheckHead)`. + +### Tasks + +- [ ] Derive `head`/`tail` from the split functions (or a shared guard) + +### Related + +- [split-last-nullable-map](split-last-nullable-map.md) — same rule applied + to `splitLast`'s own body diff --git a/fjs/types/bigfloat/todo/from-decimal.md b/fjs/types/bigfloat/todo/from-decimal.md new file mode 100644 index 0000000000..81284aaca3 --- /dev/null +++ b/fjs/types/bigfloat/todo/from-decimal.md @@ -0,0 +1,40 @@ +## Own decimal literal → `BigFloat` + +**Priority:** P3 +**Status:** open + +### Problem + +Both tokenizers convert a decimal number literal into the same +`BigFloat = readonly [mantissa: bigint, exp: number]`, by different +algorithms, and neither lives in this module: + +- `fjs/js/tokenizer/module.f.mjs:267-286` — incremental accumulator threaded + through the scan state (`addFracDigit`, `addExpDigit`, + `bufferToNumberToken` computing `[b.s * b.m, b.f + b.es * b.e]`); +- `fjs/djs/tokenizer/module.f.mjs:378-390` — `decodeNumber` doing string + surgery over the matched lexeme (`BigInt(intDigits + fracDigits)`, + `exp - fracDigits.length`). + +`fjs/types/bigfloat` exports only `multiply` and `decToBin`, so the +conversion has no owner and both lexers grew their own. The exponent/fraction +sign bookkeeping is exactly the kind of arithmetic that should be stated +once. + +### Proposal + +Add `fromDecimalParts(sign, intDigits, fracDigits, expSign, expDigits)` (or a +`parseDecimal(lexeme)`) to `fjs/types/bigfloat`, used by both tokenizers. + +### Tasks + +- [ ] Design the input shape (parts record vs. lexeme string) against both + call sites +- [ ] Implement with proof coverage; convert both tokenizers + +### Related + +- [tokenizer-finish-number-shared](../../../js/todo/tokenizer-finish-number-shared.md) + — number *completeness* classification, a different concern +- [round53-overflow](round53-overflow.md) — `decToBin`, the next stage of the + same pipeline diff --git a/fjs/types/bit_vec/todo/front-from-unpack-split.md b/fjs/types/bit_vec/todo/front-from-unpack-split.md new file mode 100644 index 0000000000..040d04738b --- /dev/null +++ b/fjs/types/bit_vec/todo/front-from-unpack-split.md @@ -0,0 +1,53 @@ +## Derive `front`/`removeFront` from `unpackSplit` + +**Priority:** P3 +**Status:** open + +### Problem + +`bo` already derives `unpackPopFront` generically from the injected +`unpackSplit` (`module.f.mjs:258-267`), yet `_Base` also demands hand-written +`front` and `removeFront` from each bit order (`module.f.mjs:186-187`), and +both `lsb` (`:330-337`) and `msb` (`:356-366`) re-derive exactly those two +projections by hand. Substituting each order's `unpackSplit` into +`unpackPopFront` reproduces both bodies: `front(len)(v)` is +`unpackPopFront(len)(unpack(v))[0]` and `removeFront(len)(v)` is +`pack(unpackPopFront(len)(unpack(v))[1])` (`vec` re-masks, so the unmasked +rest is fine). + +`startsWith` (`:311-314`) shows the cost from the other side — it needed "the +first `n` bits" and reached for `popFront(n)(v)[0]` instead of `front`, so the +same extraction now exists four ways in one module. + +The same section has three call-invariant partial applications rebuilt per +call (AGENTS.md §6.3): + +- `:313` — `popFront(n)(v)[0]` rebuilds `popFront(n)` per `v`, though `n` + comes from `prefix`, bound one scope up. +- `:288-289` — `unpackListToVec(unpackConcat)` and `map(unpack)` are + independent of `list`; `map(unpack)` doesn't even depend on `bo` and belongs + at module scope. +- `:381-383` — the `b => ({ length: 8n, uint: BigInt(b) })` helper closes over + nothing and is re-created per list. + +### Proposal + +Drop `front`/`removeFront` from `_Base` (and from the `lsb`/`msb` literals) +and define them once inside `bo` next to `popFront`, derived from +`unpackPopFront`. Make `startsWith` use the derived `front`. The `_Base` +injection surface shrinks to the genuinely order-specific parts (`norm`, +`uintCmp`, `unpackSplit`, `unpackConcatUint`). Hoist the three call-invariant +partial applications to their dependency's scope. + +### Tasks + +- [ ] Remove `front`/`removeFront` from `_Base` and derive them in `bo` +- [ ] Rewrite `startsWith` through the derived `front`, binding it once per + `prefix` +- [ ] Hoist `map(unpack)` and the u8 `Unpacked` constructor to module scope; + bind `unpackListToVec(unpackConcat)` once per bit order + +### Related + +- [92](../../todo/92.md) — nominal MSB/LSB types touch the same surface +- [195](../../todo/195.md) — `listToVec` concatenation order, same module diff --git a/fjs/types/byte_set/todo/torangemap-payload.md b/fjs/types/byte_set/todo/torangemap-payload.md new file mode 100644 index 0000000000..08851cacc2 --- /dev/null +++ b/fjs/types/byte_set/todo/torangemap-payload.md @@ -0,0 +1,44 @@ +## `toRangeMap` bakes in an FSM payload + +**Priority:** P3 +**Status:** open + +### Problem + +Everything else in `byte_set` is `ByteSet → ByteSet`/`boolean` bitmask +algebra. `toRangeMap` (`module.f.mjs:56-66`) alone drags in `list`, +`range_map`, and `sorted_set`, and hardcodes a `SortedSet` state-name +payload: + +```js +/** @type {(n: ByteSet) => (s: string) => (i: number) => RangeMap>} */ +const toRangeMapOp = n => s => i => { ... prev ? [s] : [] ... } +``` + +That is a DFA-construction concept: its only caller is `fsm`'s `foldOp` +(`fjs/fsm/module.f.mjs:55`), which merges the result into a +`RangeMap>` keyed by rule name. A `types` leaf naming +`string` and `SortedSet` for one higher-level consumer is a layering +inversion (AGENTS.md §5.4: move logic to its natural module even with a +single consumer when it is conceptually distinct). + +[bit-set-factory](../../todo/bit-set-factory.md) already decided `toRangeMap` +does not belong in the shared bitmask factory; this issue is about the other +half — the payload and the dependency direction. + +### Proposal + +Either move `toRangeMap` into `fjs/fsm`, or make the payload generic — +`(n: ByteSet) => (v: T) => RangeMap` — so `byte_set` stops naming +`string`/`SortedSet` and drops its dependency on two higher-level +container modules. + +### Tasks + +- [ ] Pick a home (move to `fjs/fsm`) or generalize the payload type +- [ ] Update `fsm.foldOp` and the proofs + +### Related + +- [bit-set-factory](../../todo/bit-set-factory.md) — rules `toRangeMap` out + of the factory; this issue covers what remains diff --git a/fjs/types/patricia_trie/todo/declarative-stack-fold.md b/fjs/types/patricia_trie/todo/declarative-stack-fold.md new file mode 100644 index 0000000000..639c9695a3 --- /dev/null +++ b/fjs/types/patricia_trie/todo/declarative-stack-fold.md @@ -0,0 +1,31 @@ +## Fold the trie stack declaratively + +**Priority:** P3 +**Status:** open + +### Problem + +`module.f.mjs:14-36` has three deviations in 22 lines: + +- `stack[stack.length - 1]`, `stack[stack.length - 2]`, and + `stack.slice(0, -2)` re-implement `last` and `splitLast` from the sibling + `fjs/types/array` (§6.3 also prefers destructuring over indexed access). +- `end` is a right fold over the stack written as a manual descending index + loop with `let h` and a mutating destructuring assignment + (`[h, storage] = create(lHash, h, storage)`) — squarely §5.5 and §6.1. +- `end`'s `stack.length === 0` early return plus "seed from the last element" + is the standard no-seed reduce; expressed over `splitLast(stack)` the empty + case is the `null` branch and the guard disappears. + +### Proposal + +Import `last`/`splitLast` from `fjs/types/array`; express `end` as a +`reduceRight` (or a `list.fold` over the reversed stack) threading +`[h, storage]` as the accumulator. `push`'s loop is a genuine +carry-propagation loop and can stay imperative, but should destructure +through `splitLast` twice instead of three index expressions. + +### Tasks + +- [ ] Rewrite `end` as a fold over `splitLast(stack)` +- [ ] Replace the index arithmetic in `push` with `array` accessors diff --git a/fjs/types/result/todo/ok-then.md b/fjs/types/result/todo/ok-then.md new file mode 100644 index 0000000000..92aea5b993 --- /dev/null +++ b/fjs/types/result/todo/ok-then.md @@ -0,0 +1,49 @@ +## Add `okThen`, the pure `Result` bind + +**Priority:** P2 +**Status:** open + +### Problem + +The pure `Result` short-circuit is hand-rolled at roughly ten sites. +`fjs/cas/evo/module.f.mjs:396-415` has five consecutive copies in one +function: + +```js +if (parentsResult[0] === 'error') { return pure(parentsResult) } +const subjectResult = resolveSubject(input)(parentsResult[1]) +if (subjectResult[0] === 'error') { return pure(subjectResult) } +... +``` + +plus `resolveParents` (`:283-287`), `resolveParent` (`:259-265`), +`decodeReadRevision` (`:495-505`), and +`fjs/dev/package_json/module.f.mjs:145-150`. + +`fjs/types/result` exports only `ok`, `error`, `unwrap`, `invert`, `mapOk` — +the functor map but not the monad bind. `fjs/effects` already ships the +effectful twin, `okStep` (`fjs/effects/module.f.mjs:300-309`), whose JSDoc +states the case verbatim: "Collapses the hand-written +`r[0] === 'error' ? pure(r) : f(r[1])` check that recurs at every site." +The pure sibling was never written, so every pure chain re-derives it. + +### Proposal + +```js +/** @type {(f: (value: T) => Result) => (r: Result) => Result} */ +export const okThen = f => r => r[0] === 'error' ? r : f(r[1]) +``` + +next to `mapOk`. `addRevision`'s five guards become one composition, and the +`[0]`/`[1]` index accesses disappear per §6.3's destructuring rule. + +### Tasks + +- [ ] Add `okThen` to `fjs/types/result` with proof coverage +- [ ] Convert the `cas/evo` and `dev/package_json` sites + +### Related + +- [044-error-handling-pattern](../../../../todo/044-error-handling-pattern.md) + — the `?` operator as a future language feature; this is today's library + form diff --git a/fjs/types/sorted_list/todo/tail-reduce-shadowing.md b/fjs/types/sorted_list/todo/tail-reduce-shadowing.md new file mode 100644 index 0000000000..71396c4c54 --- /dev/null +++ b/fjs/types/sorted_list/todo/tail-reduce-shadowing.md @@ -0,0 +1,44 @@ +## Two opposite `tailReduce`s share one name + +**Priority:** P3 +**Status:** open + +### Problem + +`module.f.mjs` binds the name `tailReduce` twice with contradictory meanings, +one shadowing the other: + +```js +export const merge = cmp => { + /** @type {TailReduce} */ + const tailReduce = mergeTail // :58 — keeps the remaining tail + return genericMerge({ reduceOp: cmpReduce(cmp), tailReduce })(null) +} +const mergeTail = () => identity // :74 +const tailReduce = () => () => null // :76 — discards the tail +export const intersect = cmp => + genericMerge({ reduceOp: intersectReduce(cmp), tailReduce })(null) // :95 +``` + +A reader at `:59` and a reader at `:95` see the same identifier meaning +opposite tail policies. The local binding exists only to carry a JSDoc +annotation; it captures nothing, so §6.3 says hoist it. + +### Proposal + +Rename the two to say what they do (`keepTail` / `dropTail`), annotate +`keepTail` at module scope, and delete the shadowing local. `merge` and +`intersect` then read as the same shape differing only in `reduceOp` and +tail policy. + +### Tasks + +- [ ] Rename `mergeTail` → `keepTail` (with the module-scope annotation) and + the module-level `tailReduce` → `dropTail` +- [ ] Remove the shadowing local in `merge` + +### Related + +- [66b-sorted-list-cmp-reduce-factory](../../todo/66b-sorted-list-cmp-reduce-factory.md) + — covers `cmpReduce` vs `intersectReduce`; this issue covers the tail + policies diff --git a/fjs/types/uint8array/todo/tovec-precomputed-bound.md b/fjs/types/uint8array/todo/tovec-precomputed-bound.md new file mode 100644 index 0000000000..128dd4083c --- /dev/null +++ b/fjs/types/uint8array/todo/tovec-precomputed-bound.md @@ -0,0 +1,36 @@ +## `toVec` precomputes a size bound + +**Priority:** P3 +**Status:** open + +### Problem + +Two adjacent functions do the same job with opposite discipline +(`module.f.mjs:30-39`): + +```js +export const toVec = input => { + assert(input.length <= maxLengthBytes, "the array is too big") + return u8ListToVecMsb(fromArrayLike(input)) +} + +export const listToVec = input => + assertNotNullish(tryU8ListToVecMsb(flat(m(input))), "the array is too big") +``` + +AGENTS.md §5.6 ("Never precompute a size to predict whether something fits") +names `tryU8ListToVec` as the `try*` variant to use instead — `listToVec` +obeys it, `toVec` re-derives a byte-count bound. The guard is also redundant: +`u8ListToVec` is the unwrapping form of `tryU8ListToVec`, so the real check +already runs inside. + +### Proposal + +`export const toVec = input => listToVec([input])` — the same list +(`flat(map(fromArrayLike)([input]))`), the same error message, and `assert` / +`maxLengthBytes` drop out of the imports. + +### Tasks + +- [ ] Rewrite `toVec` through `listToVec` and drop the `assert` / + `maxLengthBytes` imports diff --git a/nanvm-lib/todo/error-constructors.md b/nanvm-lib/todo/error-constructors.md new file mode 100644 index 0000000000..12a415db07 --- /dev/null +++ b/nanvm-lib/todo/error-constructors.md @@ -0,0 +1,44 @@ +## One owner for thrown error values + +**Priority:** P3 +**Status:** open + +### Problem + +Every error the VM can throw is built by `"literal".into()` at the throw +site, with three naming conventions and no shared constructor: + +- `src/vm/impls/try_from.rs:3-5` — a file-local + `fn error() -> Result>` returning `"Type Error"`; +- `src/vm/number_coercion.rs:67` — + `"TypeError: Cannot convert a BigInt value to a number"`; +- `src/vm/primitive_coercion.rs:8` — a file-local + `const CANNOT_CONVERT_TO_PRIMITIVE_VALUE`; +- `src/vm/numeric.rs:21` — `"TODO: Cannot multiply Number and BigInt"`; +- `src/vm/bigint/shl.rs:8-12` — a file-local `TOO_LARGE` const plus a + `too_large()` wrapper, i.e. one file already invented the missing + abstraction privately. + +Nothing enforces that a thrown value is even TypeError-shaped, and when real +`Error` objects land (per the ECMAScript references in these files) every +site must be edited in lockstep. The `shl` message additionally leaks into +test assertions (`shl.rs:329, 337`), so it is load-bearing with no single +definition. + +### Proposal + +A `vm/error.rs` owning the thrown-value vocabulary: +`fn type_error(message: &str) -> Result>` plus named +constructors for the recurring cases (`cannot_convert_to_primitive`, +`bigint_to_number`, `mixed_numeric_operands`, `shift_amount_too_large`). +`try_from::error` and `shl::too_large` collapse into calls; "what does a +thrown value look like" becomes one module's decision. + +### Tasks + +- [ ] Add `vm/error.rs` with the constructors +- [ ] Convert the five sites and the `shl` test assertions + +### Related + +- [131](131.md) — the allocator's failure channel, a different concern diff --git a/nanvm-lib/todo/numeric-operator-home.md b/nanvm-lib/todo/numeric-operator-home.md new file mode 100644 index 0000000000..889edc52e6 --- /dev/null +++ b/nanvm-lib/todo/numeric-operator-home.md @@ -0,0 +1,52 @@ +## `Numeric` operators are split across three modules + +**Priority:** P3 +**Status:** open + +### Problem + +`Numeric` owns `Mul` (`src/vm/numeric.rs:14-23`) but its unary minus is +open-coded on `Any` in `src/vm/any/neg.rs:9-16`: + +```rust +match self.to_numeric() { + Ok(Numeric::Number(n)) => { let m = -n; Ok(Unpacked::Number(m).into()) } + Ok(Numeric::BigInt(bi)) => Ok(Unpacked::BigInt(-bi).into()), + Err(e) => Err(e), +} +``` + +Three problems visible in that block: (a) `Numeric`'s negation lives outside +`numeric.rs`, so "what can you do with a `Numeric`" finds only `Mul`; +(b) `Err(e) => Err(e)` is a hand-written `?`; (c) the `Numeric → Unpacked` +wrapping is re-derived per variant — the sibling `Primitive` has +`impl From> for Unpacked` (`src/vm/primitive.rs:15-24`) but +`Numeric` has no such impl, which is why both `neg.rs` and `Numeric::mul` +wrap by hand. `Mul for Numeric` also boxes into `Any` inside the operator +(`Output = Result, Any>`) instead of staying in the numeric +domain. + +### Proposal + +In `numeric.rs`: + +- `impl Neg for Numeric { type Output = Self; … }` (negation + never throws); +- `impl From> for Unpacked`; +- change `Mul for Numeric` to `Output = Result, Any>`. + +`Any::neg` collapses to `Ok((-self.to_numeric()?).into())` and +`Mul for Any` to `Ok((self.to_numeric()? * rhs.to_numeric()?)?.into())`. +Every future numeric operator (`-`, `/`, `%`, `**`) then has one obvious +home. + +### Tasks + +- [ ] Add `Neg` and `From for Unpacked` in `numeric.rs` +- [ ] Retype `Mul for Numeric`; simplify `any/neg.rs` and `impls/mul.rs` + +### Related + +- [86](86.md) — coercion traits for a future VM, a different layer +- [sign-algebra](sign-algebra.md) — the same "give the type its algebra" + move for `Sign` diff --git a/nanvm-lib/todo/operator-impl-placement.md b/nanvm-lib/todo/operator-impl-placement.md new file mode 100644 index 0000000000..40d480154c --- /dev/null +++ b/nanvm-lib/todo/operator-impl-placement.md @@ -0,0 +1,40 @@ +## Move `String` `Add` and `Any` `Mul` out of `impls/` + +**Priority:** P4 +**Status:** open + +### Problem + +Two more single-type impls sit in the `vm/impls/` grab-bag — the same +violation already filed for `Debug for String` and the UTF-16 `From` impls: + +- `src/vm/impls/add.rs:5-10` and `src/vm/impls/add_assign.rs:5-9` — + `Add`/`AddAssign for String` is string concatenation, pure + string-domain logic, while `vm/string/` exists and already holds + `index.rs`, `partial_eq.rs`, `serializable.rs`, `sized_index.rs`, + `to_string.rs`. `impls/add.rs` mixes it with the unrelated + `Add for Unpacked`. +- `src/vm/impls/mul.rs:5-11` — `Mul for Any`, while `Any`'s other + operators live in `vm/any/add.rs`, `vm/any/neg.rs`, + `vm/any/partial_eq.rs`. The convention is stated in the opposite direction + at `src/vm/bigint/mul.rs:9`: "BigInt's Mul is implemented here, not under + impls, because it needs private BigInt's stuff." + +### Proposal + +`vm/string/add.rs` (both `Add` and `AddAssign`) and `vm/any/mul.rs`; +`vm/impls/` keeps only the genuinely cross-type impls (`Unpacked`, and the +conversions [65Y](65y-nanvm-conversion-macros.md) will rework). Worth landing +together with the two filed moves so `vm/impls/` ends with one defensible +rule instead of a residue. + +### Tasks + +- [ ] Move the `String` and `Any` operator impls next to their types +- [ ] Land with [string-debug-placement](string-debug-placement.md) and + [string-utf16-from-impls](string-utf16-from-impls.md) + +### Related + +- [string-debug-placement](string-debug-placement.md) — same category +- [string-utf16-from-impls](string-utf16-from-impls.md) — same category diff --git a/nanvm-lib/todo/sized-index-for-refs.md b/nanvm-lib/todo/sized-index-for-refs.md new file mode 100644 index 0000000000..6f7e4685ad --- /dev/null +++ b/nanvm-lib/todo/sized-index-for-refs.md @@ -0,0 +1,47 @@ +## `SizedIndex` for references + +**Priority:** P3 +**Status:** open + +### Problem + +`IContainer`'s default bodies hand-roll indexed loops +(`src/vm/internal/icontainer.rs:40-49, 52-61`): + +```rust +for i in 0..len { + if a[i] != b[i] { return false; } +} +``` + +`items_eq` is `Iter::eq_by_` (`src/common/iter.rs:44-60`) re-implemented — +and `eq_by_`'s only consumer in the whole repo is `tests/test/main.rs:121`, +so the crate's own container equality does not use the crate's own equality +combinator. + +The blocker is structural: `SizedIndex::index_iter` +(`src/common/sized_index.rs:17-22`) takes `self` by value and requires +`Self: Sized`, while `IContainer::items()` returns `&Self::Items` with +`Items: ?Sized`. The one iteration abstraction the crate has is unreachable +from the one accessor that returns items, so every consumer falls back to +`0..len` indexing: `icontainer.rs:44, 57`, `container_fmt.rs:11`, +`function/debug.rs:11, 19`, `bigint/debug.rs:18-22`. + +### Proposal + +`impl + ?Sized> SizedIndex for &T` (with the +matching `Index`), making `items()` directly iterable. Then `items_eq` +becomes header check plus +`a.index_iter().eq_by_(b.index_iter(), PartialEq::eq)`, and `serialize` a +`for item in items.index_iter()`. Also the missing piece that unblocks +[debug-delimited-fmt-helper](debug-delimited-fmt-helper.md) cleanly. + +### Tasks + +- [ ] Add the reference impls +- [ ] Convert `items_eq`, `serialize`, and the debug/format loops + +### Related + +- [debug-delimited-fmt-helper](debug-delimited-fmt-helper.md) — the two + `Debug` sites; this issue removes the indexing they were forced into diff --git a/nanvm-lib/todo/zip-longest.md b/nanvm-lib/todo/zip-longest.md new file mode 100644 index 0000000000..aab08dede0 --- /dev/null +++ b/nanvm-lib/todo/zip-longest.md @@ -0,0 +1,51 @@ +## `zip_longest` for the dual-sequence walks + +**Priority:** P3 +**Status:** open + +### Problem + +Three hand-rolled "walk two sequences to the longer end" loops, each spelled +differently: + +- `abs_add_vec` (`src/vm/bigint/mod.rs:150-169`) — a `loop` over + `(iter_a.next(), iter_b.next())` with a four-arm match; +- `abs_sub_vec` (`mod.rs:185-205`) — a `for` over `self.index_iter()` with + `iter_b.next().unwrap_or_default()` inside and a trailing + `iter_b.next().is_some()` length check; +- `Iter::eq_by_` (`src/common/iter.rs:49-60`) — the same skeleton with an + equality payload. + +`abs_cmp_vec` (`mod.rs:132-140`) hand-rolls a descending index `while` loop +using neither `index_iter` nor iterators. `mod.rs:104` even carries the note +"use .index_iter in abs_* helpers" — recorded, but only partially taken up. + +Separately, `Iter::try_reduce` (`iter.rs:17-30`) has zero call sites in +`src/` or `tests/` — 14 lines of `Result`-threading semantics (including a +silent `Ok(default())` on empty) that nothing exercises; §5.4 says extract +once the second real consumer exists. `common/iter.rs` carries an unused +combinator while three call sites hand-roll a missing one. + +### Proposal + +Add `Iter::zip_longest(self, other)` to `common/iter.rs` (it already has +`Either`, exactly the machinery needed). Then: + +- `abs_add_vec` becomes a scan over the carry; +- `abs_sub_vec` becomes a scan over the borrow, the over-long-`rhs` case + falling out of the pair shape; +- `eq_by_` becomes `zip_longest(...).all(...)`; +- `abs_cmp_vec` becomes + `len_a.cmp(&len_b).then_with(|| /* reversed lexicographic compare */)`. + +Remove `try_reduce` (restore it when a consumer appears). + +### Tasks + +- [ ] Add `zip_longest` with tests +- [ ] Convert the four walks; delete `try_reduce` + +### Related + +- [bigint-shift-decode](bigint-shift-decode.md) — explicitly rules the + mirrored carry loops out of its scope; this issue picks them up