sscoble/speckl

Fork 0

[speckl#37.5] B1: Stop silently inventing let/forall stubs in TS generator #43

New issue

Closed

opened 2026-06-02 21:55:51 +00:00 by sscoble · 1 comment

sscoble commented

2026-06-02 21:55:51 +00:00

Owner

Problem

The TypeScript generator in compiler/src/generators/typescript-state-machine.ts:482 silently invents stubs whenever a referenced identifier is not in the parser's AST. Specifically, it emits const xxx: any = {} as any; // FIXME: parser dropped let/forall and tags the source line. The root cause is that parser.ts's parseActionBlockMultiline does not emit let or forall AST nodes, so the generator fakes them.

This affects 16/16 examples. The output is semantically wrong: state mutations are dropped.

Scope

Two acceptable resolutions (pick one and document which):

Parser-side fix (preferred): Extend parser.ts parseActionBlockMultiline to emit let and forall AST nodes. Update the generator's emitAction() to consume them. This benefits rust/z3/wasm generators automatically.
Generator-side fail-loud fix: Have the generator throw (or post a hard error to stderr) when it detects an undefined identifier instead of inventing a stub. Preserves the existing parser behavior but surfaces the bug instead of hiding it.

If you choose option 1, also update the AST type definitions in parser.ts (or wherever the node types live). If you choose option 2, no parser change is needed.

Out of scope

Do NOT touch any other generator target (rust, z3, wasm) directly. If the parser change in option 1 incidentally benefits them, fine, but do not chase that — different ticket.
Do NOT run nix build or nixos-rebuild. Build only the compiler.
Do NOT open a PR. Post the diff as a comment and let Scott choose between options 1 and 2.

Acceptance Criteria

Choose option 1 (parser + generator) or option 2 (generator fail-loud) and document the choice in a comment
All 16 examples no longer have // FIXME: parser dropped let/forall markers in the emitted TS
grep -r 'FIXME: parser dropped' compiler/dist/ 2>/dev/null | wc -l returns 0 (or the count of such markers is documented in the comment with reasoning)
All 16 examples still compile via tsc --noEmit --target es2020
Post the diff as a comment with file list, line counts changed, and which option was chosen

Why this exists

Triage on #39 found 6 bugs. B1 touches the most examples (16/16) and is the root cause of the most FIXME markers in the output. Needs parser + generator coordination; flagged as the high-risk child of this decomposition.

Depends on

None

Blocks

None

## Problem The TypeScript generator in `compiler/src/generators/typescript-state-machine.ts:482` silently invents stubs whenever a referenced identifier is not in the parser's AST. Specifically, it emits `const xxx: any = {} as any; // FIXME: parser dropped let/forall` and tags the source line. The root cause is that `parser.ts`'s `parseActionBlockMultiline` does not emit `let` or `forall` AST nodes, so the generator fakes them. This affects 16/16 examples. The output is semantically wrong: state mutations are dropped. ## Scope Two acceptable resolutions (pick one and document which): 1. **Parser-side fix (preferred):** Extend `parser.ts` `parseActionBlockMultiline` to emit `let` and `forall` AST nodes. Update the generator's `emitAction()` to consume them. This benefits rust/z3/wasm generators automatically. 2. **Generator-side fail-loud fix:** Have the generator throw (or post a hard error to stderr) when it detects an undefined identifier instead of inventing a stub. Preserves the existing parser behavior but surfaces the bug instead of hiding it. If you choose option 1, also update the AST type definitions in `parser.ts` (or wherever the node types live). If you choose option 2, no parser change is needed. ## Out of scope - Do NOT touch any other generator target (rust, z3, wasm) directly. If the parser change in option 1 incidentally benefits them, fine, but do not chase that — different ticket. - Do NOT run `nix build` or `nixos-rebuild`. Build only the compiler. - Do NOT open a PR. Post the diff as a comment and let Scott choose between options 1 and 2. ## Acceptance Criteria - [ ] Choose option 1 (parser + generator) or option 2 (generator fail-loud) and document the choice in a comment - [ ] All 16 examples no longer have `// FIXME: parser dropped let/forall` markers in the emitted TS - [ ] `grep -r 'FIXME: parser dropped' compiler/dist/ 2>/dev/null | wc -l` returns 0 (or the count of such markers is documented in the comment with reasoning) - [ ] All 16 examples still compile via `tsc --noEmit --target es2020` - [ ] Post the diff as a comment with file list, line counts changed, and which option was chosen ## Why this exists Triage on #39 found 6 bugs. B1 touches the most examples (16/16) and is the root cause of the most FIXME markers in the output. Needs parser + generator coordination; flagged as the high-risk child of this decomposition. ## Depends on None ## Blocks None

sscoble referenced this issue

2026-06-02 21:56:18 +00:00

[speckl#37.1] Triage TS generator bugs: enumerate specific bugs with file paths + repros #39

sscoble referenced this issue

2026-06-02 23:18:08 +00:00

[speckl#37.2] Fix top P0+P1 TS generator bugs from #37.1 triage #40

sscoble referenced this issue

2026-06-03 00:59:26 +00:00

[speckl#37.8] B6: Add readonly modifier + JSDoc on emitted state variables #46

~~sscoble referenced this issue 2026-06-03 01:41:02 +00:00~~

[speckl#37.9] B2.1: Forward constraint expressions through TS generator (rewriteExpr chain) #47

sscoble commented

2026-06-03 05:18:59 +00:00

Author

Owner

Closing — re-diagnosed from real code, premise was wrong

Scott directive 2026-06-02 ~19:00 PT: if the ticket framing is wrong, re-diagnose. I read parser.ts and typescript-state-machine.ts to verify, and the premise was wrong on three counts:

The parser already emits let AST nodes. parser.ts:136 declares { type: 'let'; name: string; expr: string } in the AST types, and parser.ts:1024 actually emits them via parseActionBlockMultiline. The TS generator already has case 'let': at typescript-state-machine.ts:374 that produces a real const/let declaration.
forall is not an action-block AST node at all. It only appears in constraint expression parsing (parser.ts:274, 1103). The framing of parser drops let/forall is a category error.
The actual bug is in the generator, not the parser. typescript-state-machine.ts:456-482 runs a preamble heuristic AFTER the structured emit. It scans the rendered body with a regex, finds any identifier not in the known set (state vars, locals, reserved words, let/const/var declared in body), and silently invents const x: any = {} for each one. The misleading comment // FIXME: parser dropped let/forall was wrong from day one.

Real symptom (Raft.ts `StartElection`)

StartElection(server: ServerId) {
  const xxx: any = {} as any;  // FIXME: parser dropped let/forall
  const term: any = {} as any;  // FIXME: parser dropped let/forall
  if (!(this.servers.has(server))) throw new Error(...);
  if (!(this.serverState.get(server)!.role === "follower")) throw new Error(...);
  const newTerm = this.serverState.get(server)!.currentTerm + 1;
  this.decisionLog.push({ server: server, term: newTerm } as ElectionStarted);
}

xxx is the residual of the heuristic matching itself in a regex; term is a real let binding the structured emit handled correctly (line 391: const newTerm = ...) but the heuristic is re-declaring it as a bogus any stub BEFORE the real code, which would shadow or redeclaration error if tsc were run with noEmitOnError: true. 16/16 examples have this noise.

Plus a separate init-block bug (out of scope for the new ticket)

The Raft.ts constructor contains the raw forall text rendered as a string literal: this.servers = {"s1", "s2", ...} forall s in servers: serverState[s] == ...; This is a different bug — the init-block emit is not iterating the state-value enum correctly, or the parser is producing a stringly-typed init body. This is in the same file region but is a different root cause. NOT in scope for the new ticket. Filed separately as a follow-up if needed.

What I'm doing

Closing this ticket. Opening a new ticket that:

Diagnoses the preamble heuristic specifically
Has accurate ACs (delete lines 456-482, verify 16/16 examples no longer have // FIXME: parser dropped markers, verify tsc --noEmit still passes on every example that was previously clean)
Does NOT pretend to fix the init-block raw-string bug (separate ticket, separate work)
Does NOT touch the parser (it's correct)
Treats forall as out of scope (it's a constraint-only keyword, never was an action-block AST node)

Tracking the init-block raw-string bug as a follow-up note in the new ticket's ## Follow-up section.

## Closing — re-diagnosed from real code, premise was wrong Scott directive 2026-06-02 ~19:00 PT: if the ticket framing is wrong, re-diagnose. I read `parser.ts` and `typescript-state-machine.ts` to verify, and the premise was wrong on three counts: 1. **The parser already emits `let` AST nodes.** `parser.ts:136` declares `{ type: 'let'; name: string; expr: string }` in the AST types, and `parser.ts:1024` actually emits them via `parseActionBlockMultiline`. The TS generator already has `case 'let':` at `typescript-state-machine.ts:374` that produces a real `const`/`let` declaration. 2. **`forall` is not an action-block AST node at all.** It only appears in `constraint` expression parsing (`parser.ts:274, 1103`). The framing of `parser drops let/forall` is a category error. 3. **The actual bug is in the generator, not the parser.** `typescript-state-machine.ts:456-482` runs a *preamble heuristic* AFTER the structured emit. It scans the rendered body with a regex, finds any identifier not in the known set (state vars, locals, reserved words, `let`/`const`/`var` declared in body), and silently invents `const x: any = {}` for each one. The misleading comment `// FIXME: parser dropped let/forall` was wrong from day one. ### Real symptom (Raft.ts `StartElection`) ```ts StartElection(server: ServerId) { const xxx: any = {} as any; // FIXME: parser dropped let/forall const term: any = {} as any; // FIXME: parser dropped let/forall if (!(this.servers.has(server))) throw new Error(...); if (!(this.serverState.get(server)!.role === "follower")) throw new Error(...); const newTerm = this.serverState.get(server)!.currentTerm + 1; this.decisionLog.push({ server: server, term: newTerm } as ElectionStarted); } ``` `xxx` is the residual of the heuristic matching itself in a regex; `term` is a *real* `let` binding the structured emit handled correctly (line 391: `const newTerm = ...`) but the heuristic is *re-declaring it as a bogus any stub* BEFORE the real code, which would shadow or `redeclaration` error if `tsc` were run with `noEmitOnError: true`. 16/16 examples have this noise. ### Plus a separate init-block bug (out of scope for the new ticket) The Raft.ts constructor contains the raw forall text rendered as a string literal: `this.servers = {"s1", "s2", ...} forall s in servers: serverState[s] == ...;` This is a different bug — the init-block emit is not iterating the state-value enum correctly, or the parser is producing a stringly-typed init body. This is in the same file region but is a different root cause. NOT in scope for the new ticket. Filed separately as a follow-up if needed. ### What I'm doing Closing this ticket. Opening a new ticket that: - Diagnoses the *preamble heuristic* specifically - Has accurate ACs (delete lines 456-482, verify 16/16 examples no longer have `// FIXME: parser dropped` markers, verify `tsc --noEmit` still passes on every example that was previously clean) - Does NOT pretend to fix the init-block raw-string bug (separate ticket, separate work) - Does NOT touch the parser (it's correct) - Treats `forall` as out of scope (it's a constraint-only keyword, never was an action-block AST node) Tracking the init-block raw-string bug as a follow-up note in the new ticket's `## Follow-up` section.

sscoble closed this issue

2026-06-03 05:18:59 +00:00

sscoble referenced this issue

2026-06-03 05:19:41 +00:00

[speckl#37.11] B1-redux: Delete preamble heuristic in TS generator emitAction (re-diagnosis) #52

sscoble referenced this issue

2026-06-03 15:45:28 +00:00

[speckl#37.11] B1-redux: Delete preamble heuristic in TS generator emitAction (re-diagnosis) #52