Skip to content

feat: NoCellError names the state's accepted set - #29

Open
cansirin wants to merge 4 commits into
mainfrom
build/20-no-cell-error-accepted-types-d5e115fb
Open

feat: NoCellError names the state's accepted set#29
cansirin wants to merge 4 commits into
mainfrom
build/20-no-cell-error-accepted-types-d5e115fb

Conversation

@cansirin

@cansirin cansirin commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

NoCellError now carries acceptedTypes — the Msg types the refusing state admits — and states them in the message.

Before this, every refusal from a given state was byte-identical whatever you dispatched: "not this one" was the whole answer. Discovering that a state accepts nothing at all therefore cost one dispatch per Msg type, which is how #14 arrived — six probes to establish one fact about a frozen lane.

Where the set is read. lookupCell is the single cell selection both applyCell and tryApplyCell skin, and it holds the row at the moment the miss is detected. Reading the set anywhere else would be a second reader that could disagree with the selection about which cells exist, so it is read there and nowhere else. Its return type became a union: the miss arm carries acceptedTypes, the hit arm has no such field. A single optional field holding [] on a hit would read as "accepts nothing" to anyone who looked, and the keys array is now allocated on the miss arm only, so the dispatch hot path is unchanged.

Per form. The transitions form answers from the refusing state's own row (a state with no row at all — a type-bypassed state.type — accepts nothing, which is true of it). The reducer form answers from the flat table's keys, since dispatch there never consults the state; the stateName stays the same best-effort read, so an untagged state still yields (untagged state) rather than throwing.

The empty case is words. This state accepts no Msg at all. — not accepts: [], which a caller skims as a formatting artefact rather than a dead end. That case is the signal a caller acting on a possibly-final state most needs.

The old text is a prefix. The pre-#14 message is kept verbatim and the new clause only ever appends, so a caller matching on it keeps matching. Tests assert startsWith, never equality.

One site outside the kernel throws NoCellError by hand: the chart compiler's live-but-undecided cell. Every event gets a row key there, but an undecided one is not a Msg the state accepts, so those keys are tracked and excluded from the set the error reports.

Tests cover both forms, the empty accepted set in each, the untagged-state fallback, the missing row, and tryApplyCell/applyCell message parity.

Closes #20

Deviations

  • Guard or gate bypassedSaid: land the change on a fabrika build check --surface code green. Did: validated by running this repo's own CI gates in-tree instead — typecheck, typecheck:test, lint, docs:reference:check, check:merge-markers, check-export-stamps, test (1953 passed), build, verify-exports, all green — and took the prose surface's green for the changeset. Why: build check --surface code runs pnpm typecheck --force, and this repo's typecheck is a plain tsc --noEmit, which rejects --force with TS5093. The red is structural and reproduces on a clean tree; the flag is turbo's and this repo does not run turbo. Disposition: filed as build check --surface code is unconditionally red: it appends turbo's --force to a plain tsc typecheck #28 with the reproduction and the lint:worktree precedent for the fix.
  • Declined guidanceSaid: invoke every fabrika verb as node packages/fabrika-cli/src/bin.ts. Did: used the workspace binary via pnpm exec fabrika. Why: that path does not exist in this repo, and the phoenix copy refuses to serve a cwd in a different repository; the workspace binary is what this repo's CLAUDE.md prescribes. Disposition: stated here.
  • Out-of-scope changeSaid: NoCellError names the state's accepted set #20 names NoCellError, lookupCell and the two skins. Did: also updated src/chart/compile.ts's hand-thrown NoCellError and the error-idiom.test.ts construction. Why: the third constructor parameter is required, so every construction site had to state its set; leaving either as a default would have made the compiler's refusals claim the state accepts nothing. Disposition: stated here.

Closes #21

Closes #22

Closes #23
Closes #19

A refusal carried what was refused and where, but not what the state would
have taken, so learning that a state accepts nothing at all cost one dispatch
per Msg type. `acceptedTypes` is now read at `lookupCell` — the one selection
site, with the row already in hand — and stated in the message, the empty case
in words rather than as an empty pair of brackets.
@cansirin

Copy link
Copy Markdown
Contributor Author

review-code: PASS @ 531e69c content:eac98f96f446 — merge-ready

Graded against #20's acceptance criteria at 531e69c3. CI at head: green (2/2 test-and-build). Classes in scope: code (6 files), doc (1) — doc verdict posts separately. harness: false, self: false, so no governance namespace is derived.

Per-criterion

# Criterion Verdict Evidence
1 acceptedTypes public readonly, valued as the admitted msg types PASS src/pure/core.tspublic readonly acceptedTypes: readonly string[] as the third constructor parameter, beside msgType/stateName. All four construction sites at head pass it (pure/core.ts:338, runtime-types.ts:946, chart/compile.ts:381, error-idiom.test.ts:69); no default papers over a site.
2 A refusal from a state with cells names them PASS core.ts message appends This state accepts: "step", "halt". via acceptedTypes.map(t => "${t}").join(", "). Pinned in no-cell-error.test.ts for both forms.
3 A state with no cells says so in words, not an empty list PASS acceptedTypes.length === 0 branch emits This state accepts no Msg at all. The test asserts the message does not contain [], which is the assertion that actually forecloses the list-formatting shortcut the issue names.
4 The current text is a prefix of the new message at both forms PASS The pre-#14 template is reproduced verbatim in the test as legacy() and asserted with startsWith, never equality — transitions (start/running, UNBLOCKED/frozen) and reducer (unknown_wire/counting, untagged). The implementation only ever appends.
5 Reducer form yields the flat table's keys; an untagged state yields the placeholder, not a throw PASS lookupCell's reducer arm returns acceptedTypes: Object.keys(record) on the miss; stateName stays the stateNameOf best-effort read. Test reducer form: an untagged state still yields the placeholder AND the set pins stateName === "(untagged state)" alongside acceptedTypes === ["bump"].
6 Unit tests cover both forms, the empty set, the untagged fallback PASS Six new cases in no-cell-error.test.ts — transitions accepted set, transitions empty row, type-bypassed missing row, reducer accepted set, reducer untagged, reducer empty update — plus a parity assertion in try-apply-cell.test.ts that the thrown and Err-carried errors agree on acceptedTypes and on message.

Standing checks

  • Type design. CellLookup<S, M, C> as a union rather than one optional field is the right call and the reason is the load-bearing one: an optional acceptedTypes holding [] on a hit would read as "accepts nothing" to anyone who looked. The hit arm has no such field to misread. The keys array allocates on the miss arm only, so the dispatch hot path is unchanged — that claim is traceable to the code, not inferred from names.
  • Single reader. The set is read at lookupCell, the one selection both applyCell and tryApplyCell skin, so the two cannot disagree about which cells exist. The try-apply-cell.test.ts addition asserts that agreement rather than assuming it.
  • The compiler's hand-thrown refusal is correct. src/chart/compile.ts gives every event a row key, so the naive Object.keys(row) would have had a live-but-undecided cell listing itself as accepted. The undecided set plus the lazy read inside the throwing closure resolves that: the closure runs after the events loop has finished filling both row and undecided, so keys added after the throwing cell was installed are still excluded. Verified by reading the loop at head, not from the comment. The refused no-op cells (final states, out-of-scope, ignore) stay in the reported set, which is the truthful reading — dispatching them does not throw.
  • Test honesty. The one pre-existing assertion touched (error-idiom.test.ts) is a mechanical constructor-arity update; nothing was weakened or rewritten to assert the implementation against itself.
  • Release containment. The added constructor parameter is required, so new NoCellError(a, b) breaks. Disclosed in the changeset as breaking-for-direct-constructors and shipped minor, which is correct for @demlik/tea at 0.12.0.
  • Comment discipline. The added comments state constraints the code cannot show — why the read lives in lookupCell, why the union beats an optional field, why the closure reads lazily. The separator box in no-cell-error.test.ts matches that file's existing convention. No name-restaters, no control-flow narration.
  • CI. Not recomputed. Read structurally at head.

Non-blocking observations

  • Object.keys does not filter non-function values, so a type-bypassed { start: undefined } cell would be reported as accepted while still refusing. NoCellError names the state's accepted set #20 explicitly sanctions this reading — "best-effort in the same way stateNameOf already reads a string state.type best-effort" — so it is in-spec, not a finding.
  • The src/chart/compile.ts undecided filter has no test of its own; it is out-of-scope for NoCellError names the state's accepted set #20 (disclosed as such) and its logic verified by reading, but it is the subtlest line in the diff and a follow-up test would be cheap. Non-blocking, no criterion appended.

Deviations

Disclosed Matched against the diff
Guard bypassed — build check --surface code red on TS5093, validated with the repo's own gates instead, filed as #28 Substantiated. CI at head is green on the repo's real gate, which is the evidence that matters here.
Declined guidance — used pnpm exec fabrika because packages/fabrika-cli/src/bin.ts does not exist in this repo Substantiated; the path is genuinely absent from this tree.
Out-of-scope change — src/chart/compile.ts and error-idiom.test.ts Substantiated and unavoidable: the third parameter is required, so every construction site had to state its set. Leaving a default would have made the compiler's refusals falsely claim the state accepts nothing — the exact failure mode #20 exists to remove.

deviation-disclosure: PASS — nothing undisclosed that this gate could see.

Verdict-written: 2026-08-19T22:07:52Z

@cansirin

Copy link
Copy Markdown
Contributor Author

review-doc: PASS @ 531e69c content:eac98f96f446 — merge-ready

Doc-class slice at 531e69c3: one file, .changeset/no-cell-error-accepted-types.md. CI at head green.

Per-criterion

#20's criteria are all behavioural and are graded in the review-code namespace. The doc slice carries no criterion of its own, so this verdict is the hygiene checklist. [N/A] here is a positively-established non-obligation, not an unreadable row.

Hygiene

  • Right surface. PASS. A release note about a shipped API change belongs in .changeset/, and this is one. It does not try to be the ADR (no why-narrative about the design), it does not try to be a pattern doc, and it does not duplicate the PR body's job — the PR body argues the design, the changeset states the consequence for a consumer.
  • One Diátaxis mode. PASS. Explanation throughout, aimed at a consumer reading a release: what changed, why the shape is what it is, what it costs them. It never drifts into how-to (no instructions) and never into reference (no signature table). The **Breaking for direct constructors only:** paragraph is the one that could have slipped into how-to and does not — it states the fact and the blast radius, then stops.
  • Supersession. N/A. Nothing is replaced or contradicted; no second live doc answers this question.
  • Status sanity. PASS. Frontmatter is "@demlik/tea": minor, and minor is correct for a breaking change at 0.12.0 under the pre-1.0 changesets convention. The body's own "breaking for direct constructors only" scoping matches the bump rather than fighting it, which is the case where a minor on a breaking change usually goes wrong.
  • Claims trace. PASS. Every falsifiable claim is grounded in the diff I read: acceptedTypes: readonly string[] exists as stated; the set is read at lookupCell where the row is in hand; the transitions form answers from the state's row and the reducer form from the flat table's keys; the pre-existing text is kept verbatim as a prefix; and "every in-package construction site passes it" holds — four sites at head, all updated, no default.
  • Prose craft. PASS. Plain words, one idea per sentence, and the paragraph that matters most does the most work: the empty case is justified rather than asserted ("the one a caller acting on a possibly-final state most needs, so it is words rather than an empty pair of brackets"). A consumer learns the failure mode being removed, not just the field being added — which is the difference between a changeset and a diffstat.

Deviations

No deviation in the disclosed set is doc-class. The three entries are a gate bypass, a declined verb path, and an out-of-scope code change; each is matched on its substance in the review-code verdict.

deviation-disclosure: PASS — nothing undisclosed that this gate could see.

Verdict-written: 2026-08-19T22:08:15Z

cansirin and others added 2 commits August 22, 2026 00:59
The four compile-time refusals in the chart compiler — an edge naming a cell
with no implementation, a per-site cell missing an edge entry, an edge naming a
guard with no implementation, and the reducer-chart safety net — each ended at
the missing name, so an author could not tell a misspelling from an omission.

Each now lists what was there, all four through one shared `suppliedClause`
helper so the phrasing cannot drift: the cells supplied, the edges the per-site
bag carries, the guards supplied, and the edges the reducer chart declares. An
empty supplied set reads as words, not an empty list. The safety net says it is
a library net under a compile-time obligation, not a chart error. Each message's
prior text is kept verbatim as a prefix. Each site's probe lands with it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`LaneShapeError` is the lane's whole refusal surface and the one a second
consumer of the workflow importer meets first. Every refusal it raises — across
`defineLane`/`lowerRegion` (the authoring door) and `runLane`/its load-and-boot
`check` (the imported door) — was audited against the compiler's principle
(#20/#22): name the set that WOULD have been accepted, not only what was rejected.

Where a refusal admits a set, it now names it through the compiler's own
`suppliedClause` (exported for this second consumer, kept module-internal): a
missing or duplicated `initial: true` names the chart's states (or which carry
the marker); a chart with no final names its states; a persisted leaf naming a
task the lane does not run names the tasks it does; a leaf or its `was` in an
undeclared state names the declared states (the lane twin of `NoCellError`); a
task with no hand names the hands supplied, the empty case in words not `[]`.

Where a refusal admits no set — a non-object state, an edge delegating its
target to a cell, two terminals spelled alike, a dotted id or event name, a
foreign event, a missing leaf, a phase-less lane, a `maxRetries` mismatch — the
reason is recorded in the source beside the throw. The dispatch-time "no cell"
refusal is recorded as out of scope: it fires on a well-formed lane, so its
accepted set belongs to the `acceptedTypes` family (#20/#21), not this helper.

Each message's pre-existing text is kept verbatim as a prefix. The changed
cases are covered by tests; the probe check and export stamps stay green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant