Skip to content

feat(ir): program package with graph-local ids; lower assigns ids and origins - #2137

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

devin-ai-integration[bot] wants to merge 6 commits into
developfrom
feat/ir-program-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 1 of the execution-owned IR migration (docs/internals/execution-ir-design.md, "Migration as a stack of pull requests", row 1). Today a lowered graph's identity is the AST pointer of each record; this step adds the identity the later steps move the executors onto, without changing any consumer.

  • New package internal/ir/program: the per-graph dense id types (NodeID, EdgeID, StmtID, ExprID, StateID, RegionID, PseudoID, TransID; zero is "none"), NodeKind, Origin (document, span, path, kind word, name, docs) whose String() renders what nodeDescription renders today, and GraphID declared as a type — its encoder is a later step. The package reads no syntax: tests/hygiene/layering_test.go registers it in the ir layer and forbids program → syntax/ast.
  • lower assigns an id to every record it already builds and keeps the back-map beside the graph: ActionGraph.ID, Origins, Kinds, Provenance; StateGraph.ID, Origins, RegionID/RegionOrigins, PseudoID/PseudoOrigins, TransID/TransOrigins, Provenance. Additive: nothing removed, no behaviour change, no consumer reads the new fields yet.
  • Origin.Element and Origin.Docs are declared but left empty by lower: no identity table or source-text model exists at lowering time; a later step fills them where one does.

How it was verified

  • internal/ir/program unit tests: Origin.String() table, NodeKind names, GraphID zero.
  • lower unit tests (identity_test.go): ids dense, in declaration order, deterministic across two lowerings, Origin.Path unique per graph, for action graphs and for the four state-graph tables; the cyclic-specialization error path still returns an id-bearing graph.
  • TestOriginStringMatchesNodeDescription (runtime): Origin.String() equals nodeDescription(node) over every action node (5,618) of every fixture under internal/exec/runtime/testdata/conformance/, block graphs included.
  • Gate on this head: go build ./..., go vet ./..., gofmt -l . empty, make lint, go test ./... (three packages hit the 10m default under the suite's own load — cmd/sysml, tests/codegen, tests/export — and pass when run alone; TestEmbeddedSnapshotIsCurrent failed once on a stale gitignored stdlib.snapshot and passes once regenerated), go test -count=1 ./tests/corpus -run 'TestTrainingExamples|TestPilotCorpora' with both corpora downloaded, go test ./tests/hygiene, go test -C tools ./..., the two-line comment check. TestGolden and every ratchet are untouched.

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md)
  • Changelog entry — none: internal, not user-visible (CONTRIBUTING.md § changelog)
  • 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/559734afe3dd4a7ea2db7fc21f2c1f72
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/559734afe3dd4a7ea2db7fc21f2c1f72?variant=devin
Requested by: @HuiJun


🟢 Devin Review found nothing to fix · View findings · Follow Devin's work

Devin Review

Declare the execution-owned identity every lowered record will carry:
the per-graph dense id types, NodeKind, Origin (document, span, path,
kind word, name) with the String rendering today's messages use, and
GraphID as a type whose encoder is not written yet. The package reads
no syntax; the layering test registers it in the ir layer and forbids
program -> syntax/ast.

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

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 2 findings. Devin automatically fixed all 2 findings.

Fixed by Devin (2)
  • Case flow nodes lack correct IDs — assignIDs now runs after addCasePerformNodes/addCaseStatementRuns in caseSteps; regression test TestAssignIDs_CaseFlowCoversNodesAddedAfterLowering.
  • Declared regions lose document and owner — assignRegionIDs falls back to declaredIn[region] as regionScope does; regression test TestAssignIDs_RegionOriginsCarryDocumentAndOwner.

View findings in Devin Review · Follow Devin's work

Devin Review

devin-ai-integration Bot and others added 3 commits November 11, 2026 16:36
…raphs

Every node of an ActionGraph and every state, region, pseudostate and
transition of a StateGraph gets a dense id in declaration order, an
Origin naming it the way messages do, and a Provenance mapping the ids
back to the declarations. Additive: no consumer reads the new fields yet.
Pins: ids dense, in order, deterministic across two lowerings, Origin.Path
unique per graph, and Origin.String() equal to nodeDescription over every
action node of the conformance fixtures.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
An unstated case flow gains its performed actions and statement runs after
lowerActionFlow returns, so the ids are assigned once those are in place.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
A region written in the machine's own body records its scope in declaredIn,
not regionScopeOf, so its origin lost the document and owner. A terminate
usage is a terminate node.

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

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