Skip to content

refactor(runtime): key state executor state by graph-local ids - #2145

Open
devin-ai-integration[bot] wants to merge 6 commits into
feat/ir-program-idsfrom
feat/ir-state-exec-ids
Open

devin-ai-integration[bot] wants to merge 6 commits into
feat/ir-program-idsfrom
feat/ir-state-exec-ids

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Step 3 of the execution-owned IR migration (docs/internals/execution-ir-design.md, "Migration as a stack of pull requests", row 3): the state executor's own records stop holding *ast.StateNode, *ast.StateRegion, *ast.PseudostateNode and *lower.Transition pointers as keys and hold the graph-local ids step 1 assigns. Stacked on #2137 (feat/ir-program-ids), which must merge first; this PR then retargets to develop.

Records keyed by id now: stateConfiguration (simpleState StateID, regionStates map[RegionID]StateID, defaultComplete map[RegionID]bool), stateAttrs/stateAttrCells/stateAttrIndex, stateStack, history/historyRecord, joinArrived/joinChosen, breakpointNodes map[VertexRef]bool/breakpointHit/pausedAt VertexRef, timerScheduled/timeTriggerVerdict/changeFired/changeObserved/changePending/changeReads/changeRearmed/firingChange by TransID, leftAhead/enteredAhead/entering/activeAtEntry/performing, pendingRouteEntryEffects, doAction.state, dispatchCandidate.leaf/source, route (target, choice, arrival, crossed, origin, terminate VertexRef), junctionDraw.at/origin, routeEffect.within, regionEntry, lazyEntry, unitHead.shared, firingScope, heldEntry, and the mirrors in imagedState (held image) and stateCapture (snapshot). Where a consumer still needs the declaration it goes through Provenance (state_ids.go: stateID/stateDecl, regionID/regionDecl, pseudoID/pseudoDecl, transID/transDecl, vertexRef/vertexDecl). The graph keeps its AST fields; no layering change.

Public surface:

  • New program.VertexKind (VertexNone, VertexState, VertexPseudostate, VertexTerminate) and program.VertexRef{Kind, Index} with StateVertex, PseudoVertex, TerminateVertex, IsZero, State, Pseudo, Terminate. A terminate action's index is its 1-based ordinal in StateGraph.Terminates, which lowering does not number yet.
  • runtime.FiredTransition is now {Transition TransID; Source, Target VertexRef; Entry EntryRef; Origin program.Origin}; new runtime.EntryRef{Owner StateID; Region RegionID; Index int} names an entry transition (lowering gives those no id yet) by its body and 1-based position. New (*StateExecutor).FiredNodes(FiredTransition) (decl, source, target, owner ast.Node) is the back-map the LSP debugger uses.
  • ActiveStates, ActiveLeaves, CurrentState, StateStack, EnclosingStates, StatePath, RegionOf, SetBreakpointAt, PausedAt keep their AST signatures and translate at the boundary (step 7 moves those consumers).

Where the design met the code:

  • Hidden states (the graph-only state a parallel body's typed usage stands for) are not in StateGraph.States, so step 1 gave them no id, yet the executor reaches them through ParentState, RegionState, HiddenRegionOf and StateAttributes. assignStateIDs now numbers States first and then those states (statesInOrder), so len(Origins) == len(ID)+1; the step 1 denseness test asserts against ID rather than States. The step 1 API is otherwise unchanged.
  • The machine state (StateGraph.Machine) is also outside the state table; the executor gives it the id one past the last numbered state (machineID). Step 8/9 should fold it into the table.
  • Entry transitions have no TransID/origin in step 1, so FiredTransition.Origin for one carries only the declaration's doc and span, with Kind: "entry transition".
  • Internal helpers that read AST-keyed graph tables keep declaration-typed parameters until step 8 removes those tables; the records they read and write are id-keyed.

No snapshot format bump: the snapshot is an in-memory capture and no serialized form changed. No conformance .expected.json, golden trace or ratchet moved.

One baseline moved: the gzipped wasm size budgets in tests/wasm (replGzipBudget 16100000 → 16200000, engineGzipBudget 9500000 → 9600000). On the base both binaries fit; with the id tables' accessors at every record read the RE REPL build measures 16108375 gzipped bytes and the engine 9526983, so the budgets were raised with headroom for steps 2 and 5, which add the same kind of code.

How it was verified

  • go build ./..., go vet ./..., gofmt -l . empty, make lint
  • go test ./... (with cmd/sysml, tests/codegen, tests/export run alone with -timeout 30m), go test -count=1 ./tests/corpus -run 'TestTrainingExamples|TestPilotCorpora' after the download scripts, go test ./tests/hygiene, go test -C tools ./... (PSSM referee record over the pinned suite)
  • Conformance state cases, TestExecutionTrace under every region-order policy, TestRuntimeRobustnessRegionOrder, …Terminate, …ResumableInlineDoBody unchanged
  • New: TestVertexRef* (program), TestAssignIDs_StateHiddenRegionOwnersNumbered (lower)
  • Comment check (git diff --merge-base origin/feat/ir-program-ids -- '*.go' | awk ...) prints nothing

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (none covers these internals)
  • Changelog entry: none, internal refactor
  • baselines regenerated and make docs-counts run if a gate count moved (none moved)
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/a4a5e131eec042ecb18c40d69306af1a
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/a4a5e131eec042ecb18c40d69306af1a?variant=devin
Requested by: @HuiJun


🟢 Devin fixed the finding · Follow Devin's work

Devin Review

devin-ai-integration Bot and others added 5 commits November 11, 2026 17:30
The state executor's own records name states, regions, pseudostates and
transitions by the program.StateID/RegionID/PseudoID/TransID values
lowering assigns, and vertices by the new program.VertexRef, in place of
AST and lower.Transition pointers: the active configuration, state stack,
state attributes, history, ahead sets, routes, region and lazy entries,
unit heads, firing scopes, dispatch candidates, held entries, join
arrivals, breakpoints, timer and change-trigger maps, the held image and
the snapshot. FiredTransition carries the transition id, source and target
VertexRefs, an EntryRef for an entry transition and the Origin;
StateExecutor.FiredNodes maps a firing back to its declarations for the
LSP. Lowering now numbers the hidden states a parallel body's typed usages
stand for, which the executor reaches through parent chains and region
tables.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>

# Conflicts:
#	internal/ir/lower/identity_test.go
firedNames spells a fired transition through FiredNodes, letting StateVertexName
name a missing source as the empty string rather than branching on it.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…cutor

Keying the state executor's records by graph-local ids adds the id tables'
accessors at every record read; the RE REPL wasm grew to 16108375 gzipped
bytes and the engine to 9526983, past the 16100000 and 9500000 budgets
the base holds under. The budgets move to 16200000 and 9600000.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

statesInOrder reaches hidden region owners through a worklist over parent
chains and composite regions, and regionsInOrder walks it rather than
States, so a hidden owner's own regions take ids too.

Co-Authored-By: jason.han <hanhuijun@gmail.com>

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 finding. Devin automatically fixed it.

Fixed by Devin (1)
  • Nested parallel regions share a zero ID — statesInOrder now reaches hidden owners through a worklist over parent chains and composite regions, and regionsInOrder walks it instead of States, so a hidden owner's regions take ids. Lowering currently rejects a parallel typed usage inside a parallel body ("region a has no initial state"), so no fixture reaches the case; the numbering is complete regardless.

View findings in Devin Review · Follow Devin's work

Devin Review

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant