Skip to content

fix(lower): perform inherited action steps and inherit step multiplicity - #830

Open
devin-ai-integration[bot] wants to merge 26 commits into
developfrom
fix/inherited-action-steps
Open

devin-ai-integration[bot] wants to merge 26 commits into
developfrom
fix/inherited-action-steps

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Two defects in action performance, both silent wrong answers:

  1. A specialized action definition did not perform inherited steps. action def Plain :> B1; completed with no results, and action def Extra :> B1 { action b {…} } gave c = 100 (only its own b). Inherited steps, successions, starts, flows and direct statements are now lowered into the specialization's ActionGraph (lower/action_inherited.go mergeInheritedActionContent), each recorded with its declaring scope so names resolve where they were written; the runtime only consumes the lowered graph.
    • Generals are walked nearest first, a diamond's shared general contributes once (resolve.ActionGeneralBodies).
    • A step redefined by name replaces the inherited one, and inherited successions/flows retarget to the redefining step.
    • Block flows (loop/branch bodies, case steps, the single-node flows of state and classifier behaviours) receive the resolver before their members are lowered, so inherited @Probability weights and the recursive-typing guard apply there too.
    • An inherited assert constraint follows develop's sequenced-assertion rule across the whole hierarchy. If any succession or flow in a merged body orders it (its own general, an intermediate general, or the specialization itself), it becomes a node, evaluated in its declaring scope, and a violation ends the run with ViolationError (inherited_action_steps_sequenced_assertion_holds/_violated). An assertion that no succession names stays unchecked in the specialization, as it does in its general.
    • An inherited node's pin values (action heat : Heat { in b = bread; }) bind to the specializing frame's inherited parameters, both at run time and in the action view. Pin-value bindings are lowered after the inherited content is merged. So Pins::Specialized and Pins::toaster now draw X.bread == heat.b beside the inherited pack.boxed == X.toast and the inherited flow heat.t => pack.t.
    • A gate on an inherited succession flow (first a if g then f; where f is a succession flow, added on develop) is resolved in the specialization like its general: the flow's carrying succession takes the gate's guard, so S :> G follows the same guarded path as G. A gate resolves its flow in the scope it was written in, so a specialization's own same-named flow from the same source does not capture an inherited gate.
    • A specialization's own flow may connect inherited steps (action def S :> G { flow f2 from a.y to b.v; }), conformance inherited_action_steps_own_flow_inherited_steps.
    • Stable typed errors: lower.ErrCyclicSpecialization, ErrRedefinedStepMissing, ErrIncompatibleRedefinedStep, ErrAmbiguousInheritedStep; check reports the same four on the same lowered graph.
  2. A redefining step without its own multiplicity performed once (regression from fix(exec): honor exact action-step multiplicities #784). ActionGraph.StepCount/HasStepMultiplicity now use the existing semantic derivation (semantics.Model.GoverningMultiplicityOf: own, else the redefined feature's, else 1; subsetting alone carries nothing). Runtime, check (passes/behavior/action_step_multiplicity.go), explore and the SMT refusal (exec/smt/support.go) share it. All fix(exec): honor exact action-step multiplicities #784 behaviour is kept: exact counts, [0] no-op, non-fixed counts refused, plain then around repeated steps refused, ErrIntegerUnaddressable beyond 64 bits.

Scope addition, same lowering:

  1. Body-stating typed usages merge with their type. A usage typed by a definition (action u : B { … }) or a typed nested node with executable members of its own is lowered exactly as action def P :> B; own redefinitions replace inherited steps, and the graph marks the merged subflow (ActionGraph.MergedTypedSubflows) so the type is not invoked a second time. A body that only binds features/pins keeps the existing invocation path.
  2. Entry/do/exit (and part-level perform) behaviours that perform an action and state a body now run the performed action specialized by the body, as one started flow (lower/state_behavior.go lowerMergedBehaviorBody). The former execution-time refusal (and its robustness subtest asserting the refusal) is removed; the known-limitations entry is deleted.

What now executes (hand-derived, each a conformance fixture)

Model Result
Plain :> B1, WithAttr :> B1 { attribute z … } c = 1
Extra :> B1 { action b { c += 100 } } c = 101
u1 : B1 c = 1 (unchanged)
multi-level, diamond / multiple generalization inherited content once per general, nearest first
Narrow :> Base { action :>> a[2]; } a twice → c = 2
Keep :> Base { action :>> a { c += 10 } } a three times (inherited [3]), each running the inherited c += 1 and own c += 10 → c = 33
two-level redefinition, redefined typed step, inherited direct assign/send, inherited unordered starts as derived in the oracle
perform of a specialized definition; entry/do/exit typed by a specialized definition inherited flow runs
typed usage / entry / do / exit / part perform with own body merged flow

Behaviour changes to existing fixtures (consequence of item 3)

A typed node's own statements are now unordered with the callee's steps (KerML: owned features of a specialization carry no ordering against inherited ones unless a succession states one). Previously the own body ran after the callee.

  • action_invoked_node_body_writes_output and state_block_flow_typed_node_body_writes_output: the own y := y + 1 may run before or after the callee's scaling, so the admissible results are {30, 31} (outcome sets; check reports the state fixture as divergent on y/seen; the action fixture's pinned traces are replaced by a partial .trace.order).
  • state_block_flow_typed_node: its own total += y would read y before scaling sets it in some orders, and ordering it with a succession from the inherited scaling is refused by the existing multiple-successors policy, so the fixture now reads scaled.y after the pin-only node; expected values (total = 120, runs = 3) are unchanged.
  • action_accept_nested_call_chain and action_accept_nested_chain_clock (added on develop while this PR was open): each read the callee's output inside the typed node's own body (action m : Mid2 { assign received := m.got; }), which under the merge is unordered with the callee and can read m.got before it is set. Each fixture now uses a bodyless m followed by a sequenced take { assign received := m.got; }; expected values (received = 7) are unchanged, and the trace goldens only gain the take step.
  • To make check/explore see these token-order choices inside state entry behaviours, entry flows run stepped under one-move schedulers (runtime/state_statements.go runBehavior). A clock wait inside such a stepped entry/exit still reports ErrStateBehaviorWaits, as under the fixed policy.

Specification basis

  • KerML 1.0 §7.3.2 (inherited memberships of a type), §7.3.4.5 (redefinition is subsetting; redefined features are not inherited), Type::multiplicity derivation and the redefinition multiplicity constraints.
  • KerML FeatureTyping as specialization for usages; SysML v2 §7.17 PerformActionUsage; Systems Library States.sysml StateAction for entry/do/exit.
  • Keep::a: the pilot implementation (TypeAdapter.getInheritedMemberships, FeatureAdapter.removeRedefinedFeatures/addRedefinitions, ActionUsageAdapter.getRelevantFeatures/getRedefinedFeature) removes only inherited members a subtype member (directly or indirectly) redefines. Base::a's unnamed assign is not redefined by Keep::a's own unnamed assign, so both are members of Keep::a. Nothing was left undetermined, so nothing is refused for that case.
  • Derivations are in docs/project/behavior-semantic-oracle.md; docs/project/spec-compliance.md rows are updated (the former "own body wins, definition content not inherited" approximation, the mixed entry/do/exit approximation and its known limitation are replaced; new rows for inherited steps, effective step multiplicity, typed-usage merge and entry/do/exit merge).

Known limitations

  • Block-flow (loop/conditional) repetition other than [1] stays refused, including inherited counts.
  • Values held only by repeated nested nodes are still omitted from results (fix(exec): honor exact action-step multiplicities #784 policy); fixtures observe repeated steps through outer features.
  • An executable typed body nested in a type whose content is already being lowered on its path (e.g. action def A { action x : A { assign … } }, directly or through inheritance) is refused when executed with ErrRecursiveActionTyping, because the merge would unfold without end. A self-typed body that binds only features, like the library's Action::subactions, stays a lazy invocation. A finite typed usage of a general type (P :> Base { action x : Base { … } }) still merges.

How it was verified

  • Conformance fixtures inherited_action_steps_* (+ trace goldens for the ordering-sensitive ones), TestRuntimeRobustnessInheritedActionSteps, TestInheritedActionStepFixturesAnalyseCleanly (every new fixture analyses with no errors), lowerer tests in lower/action_inherited_test.go, resolver/semantic multiplicity tests, checker tests for inherited fixed/non-fixed/loop/plain-then, SMT inherited-multiplicity refusal test.
  • Verification at the PR head (all pass; no baseline regenerated or changed):
Check Result
go build ./..., go vet ./..., gofmt -l . pass; gofmt output empty
make lint (staticcheck root + tools, gosec) pass
go test ./internal/ir/... ./internal/check/... ./internal/exec/... ./internal/frontend/... ./internal/translate/... ./tests/... pass
go test ./... -count=1 with OPENSYSML_REQUIRE_TRAINING_CORPUS=1 OPENSYSML_REQUIRE_PILOT_CORPORA=1 OPENSYSML_REQUIRE_PILOT_LIBRARY_XMI=1 OPENSYSML_REQUIRE_PSSM_SUITE=1 (all four corpora downloaded) pass
make docs-check, scripts/changelog.py check, scripts/check-doc-ids.py pass
PSSM referee -check pass; recorded counts unchanged (103: 53 pass, 11 fail, 34 not-expressible, 5 differs-by-design)
fUML referee -check pass; recorded counts unchanged (55: 23 pass, 0 fail, 28 not-expressible, 4 differs-by-design)
pilot-diff -check pass; examples digest unchanged, no -update needed
tests/corpus/testdata/training_examples_expected.txt untouched
  • Lowering the bundled standard library exercises the self-typed Action::subactions (typed by Action, with a reference-only body). It is covered by TestExprTypeCheckNoStdlibFalsePositives and by the lowerer tests for feature-only self-typing, direct and mutual recursion, and a finite typed usage of a general type.
  • End-to-end: a fresh cmd/sysml build was driven through the CLI and REPL. The results are in the PR comment.

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 added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved (compliance rows need nothing: the census is counted at docs build)
  • 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/b88f96ab8ab74044a8467c8e5060b190
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/b88f96ab8ab74044a8467c8e5060b190?variant=devin
Requested by: @HuiJun

devin-ai-integration Bot and others added 5 commits October 2, 2026 20:36
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…t the performance

Remove name-based write forwarding from invoked actions, observe state and
classifier perform bodies through bound inout pins, run state entry flows
stepped under one-move schedulers so check sees their token-order choices,
and record the open outcomes of typed nodes whose own statements race the
callee's steps.

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

devin-ai-integration Bot and others added 2 commits October 2, 2026 22:13
Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Runtime-tested a fresh cmd/sysml build (at 38a960f) through the one-shot CLI and the interactive REPL.

  • Passed: Plain :> B1, WithAttr :> B1, Extra :> B1 { action b … } and u1 : B1 return c = 1 / 1 / 101 / 1; stepping Plain shows start → a → done.
  • Passed: Narrow returns c = 2; Keep inherits [3] and returns c = 33.
  • Passed: cyclic specialization and a redefinition of a missing step fail cleanly with exit 2.
  • Passed: an inherited [3] step under plain then behaves like a directly owned one: validate warns (exit 0), check refuses (exit 1), run refuses (exit 2). The develop binary silently completed the inherited model.
  • Passed: the state-entry fixture with a merged perform-plus-body entry moves ready → active on Go, with hits = 11.
Before Go: ready, hits=0 After Go: active, hits=11
State before Go Merged entry execution

Not exercised in this pass: the separate do/exit and part-perform variants, which the conformance fixtures cover.

@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review October 3, 2026 01:17
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 6 commits October 3, 2026 01:47
…ts in stepped entry behaviors

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

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

# Conflicts:
#	internal/ir/lower/action_graph.go
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…on-steps

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

# Conflicts:
#	internal/ir/lower/action_graph.go
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 8 commits October 3, 2026 12:52
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…on-steps

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

# Conflicts:
#	internal/ir/lower/action_graph.go
#	internal/ir/lower/action_nodes.go
Co-Authored-By: jason.han <hanhuijun@gmail.com>
devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration Bot and others added 3 commits October 3, 2026 20:07
Co-Authored-By: jason.han <hanhuijun@gmail.com>
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

This branch now conflicts with develop. Conflicting files, and the merged PRs that changed them:

To resolve: merge current develop into this branch with an ordinary merge commit (no rebase or force-push).
Reconcile the code conflicts with #864's statement-order scheduling (calc, constraint and case-step bodies) rather than taking one side; a conflict that needs a design decision should be raised on this PR, not guessed.

Planned merge order for the execution PRs: #844 → #850 → #838 → (#851 → #853 → #857) → #830 → #833 → #842 → #837 → #834 → #816.

Re-run the full gate (go build ./..., go vet ./..., gofmt -l ., make lint, make docs-check, go test ./...) and wait for green CI before marking ready.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

Hold on pushes: please don't push to this branch, including develop merges or empty commits to retrigger CI, until a maintainer says the CI runners are free. Prepare the conflict resolution locally and push it then.

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