Repository navigation
fix(runtime): perform every target of several successions out of an ordinary action node - #837
devin-ai-integration[bot] wants to merge 9 commits into
Conversation
…rdinary action node Co-Authored-By: jason.han <hanhuijun@gmail.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ilure Co-Authored-By: jason.han <hanhuijun@gmail.com>
…inary node is enabled Co-Authored-By: jason.han <hanhuijun@gmail.com>
…budget Co-Authored-By: jason.han <hanhuijun@gmail.com>
… solver budget
The one-guard fan-out fixture again assigns x := 3 in q, so an engine that ignores
the false guard reaches an outcome the set does not admit.
The referee's completion query over the default 40 moves takes z3 about a minute
locally (unsat, not unknown), and on CI it ran past the five-minute referee timeout.
Every run of the case ends within 12 moves, and the solve time is heavy-tailed in
the unrolling rather than in the model, so the case states "solverBudget":
{"moves": 20}. The completion query still proves every run ends within that bound.
Both fixtures with an admissible set that the fork-identity fix added gain the
check expectations TestCheckConformanceOracles requires.
Co-Authored-By: jason.han <hanhuijun@gmail.com>
…-out Co-Authored-By: jason.han <hanhuijun@gmail.com> # Conflicts: # internal/exec/runtime/action_executor.go
…-out Co-Authored-By: jason.han <hanhuijun@gmail.com> # Conflicts: # internal/exec/runtime/action_executor.go # internal/exec/runtime/action_step_multiplicity.go # internal/exec/smt/support.go
|
This branch now conflicts with
To resolve: merge current Planned merge order for the execution PRs: #844 → #850 → #838 → (#851 → #853 → #857) → #830 → #833 → #842 → #837 → #834 → #816. Re-run the full gate ( |
|
Hold on pushes: please don't push to this branch, including |
What and why
An action body such as
action a; first start then a; succession a then done; succession a then b; action b;failed with "more than one succession is enabled: action node a has multiple successors". An action node, a statement node (if,while,loop,for,assign,send), the start node and a[0]step now follow every enabled succession out of them. The token splits as a fork's does (follow/splitinaction_executor.go, whichstepForkNodenow shares), after the node's data flows have been applied once. Branch tokens are appended in declaration order.ErrAmbiguousSuccessionhad no remaining producer, so it is removed.Policy choices:
validateJoinNodeOutgoingSuccessions. For the merge it followsvalidateMergeNodeOutgoingSuccessions(SysML v2 §8.3.17, already enforced statically asmerge-outgoing-successions).MergeAction's library text constrains only incoming links, but the validation constraint bounds outgoing ones, andTestControlNodeStaticRuleAgreesWithRuntimepins the runtime to that static rule. A robustness case now covers this refusal.synchronizerule. A plain node that several successions reach is one performance after every succession that can still deliver. A pruned link never delivers, so the node runs once on the surviving branch. This is how a fork with a pruned branch into a plain node already behaved. A join instead awaits every declared incoming succession, so a pruned branch into a join deadlocks (ErrActionDeadlock).a[3]: the strict end-multiplicity policy is kept. A plainthenout of it is still refused, and written end multiplicities on each edge work. A[0]source forwards to every successor.-schedule reverseandexplore/-engine checkreach both writes of a shared feature (x = 1,x = 2).Flow.fansOut,tokenStep.fork).sizeSlotscounts every fan-out, andCycliccovers a fan-out on a cycle. A plain node that fan-out branches rejoin at would need an implicit join, which the encoding does not cover. That case gets the preciseUnsupportedError{Construct: "implicit join"}refusal. The encoding never follows just one branch.gateOf: unchanged. It looks for the unique decision that gates a path. A node with several edges is a fan-out, not a gate, so stopping there is still correct.Specification basis
Occurrences::HappensBefore. It orders its source performance completely before its target performance, and it does not select or exclude a target. A step with no written multiplicity is one performance (Actions::Action.subactions : Action[0..*]). So two successions out ofamean both targets are performed aftera, unordered relative to each other.Actions::ForkAction"has no inherent behavior". A fork only requires the1..1target multiplicity that an ordinary node's successions leave unwritten.DecisionAction/DecisionPerformancesays "exactly one of the Successions" (ControlPerformances.kerml).NonStateTransitionPerformance(Actions::DecisionTransitionAction). Its guard constrains its owntransitionLink : HappensBefore[0..1], and nothing makes the guards exclusive. So every holding guard is followed, and a false guard prunes only its own link.The compliance map gains a row for ordinary-node fan-out (✅ Faithful). The fork row and the guarded-succession row no longer say that two holding guards are reported.
behavior-semantic-oracle.mdgains the section "Several successions out of an ordinary action node: every target follows, in which order is open".Pre-existing expectations changed
All three of these asserted that two holding guards out of an ordinary node are an error because "which one wins is not stated". Under the derivation above, nothing has to win: both links exist.
conformance/action_succession_guard_two_hold: was the errorhas multiple successors, nowlevel = 12, low = 1, high = 1. The derivation is in the fixture comment, and a trace golden was added.action_executor_test.goTestActionExecutor_GuardedSuccession_TwoGuardsHold: now asserts one token at each target.robustness_test.go, "two guards hold at once" and the first-node two-successions case: both now expect both branch outputs instead of the ambiguity error.robustness_object_flow_test.goguarded_branches_with_object_flows_stay_ambiguous(fromdevelop): renamedguarded_branches_with_object_flows_both_follow; two holding guards out ofproducenow run both targets instead ofErrAmbiguousSuccession, for the same reason.object_flow_implicit_forkandobject_flow_queued_value_control_arrival(fromdevelop): token IDs only.developkept the source token on the first succession flow; this PR gives every branch a fresh token, as an explicit fork does and as the SMT encoding counts, so the branches readtoken 2,token 3instead oftoken 1,token 2. Steps, statements and outcomes are unchanged.develop'sambiguousSuccession/advanceand SMTFlow.checkSuccessors(which allowed fan-out only along succession flows) are replaced byfollowandFlow.fansOut; their compliance row and themigrate-object-flowschangelog fragment no longer say two plain successions are ambiguous.How it was verified
New tests:
action_succession_fan_out_rejoin(with.trace.order)_guard_pruned_rejoin_data_flows_statement_node_from_start_writes_one_feature: both outcomes, plus.check.expected.json,.declaredand.seed-1goldens and.trace.orderaction_step_multiplicity_fan_out_fan_out_plain(refused)_zero_fan_outTestStatementNodeFanOut, coveringassign,if,while,loop,forandsend.robustness_succession_fan_out_test.goTestRuntimeRobustnessSuccessionFanOut, covering:action_succession_fan_out_one_guard_holdsandaction_fork_one_branch_beside_live_token, each with.check.expected.json. They pin the SMT token identity: an ordinary node with one enabled succession keeps its token, and a fork with one branch replaces it. The SMT referee replays both witnesses of each. In the first,qwritesx := 3behind a false guard, so an engine that ignored the guard would reach an outcome the set does not admit.TestEncodeFanOutJoinCompletesandTestEncodeFanOutPlainRejoinRefused(SMT).A case may now state
"solverBudget": {"moves": N}besideoutcomes. The SMT referee then encodes the case to N moves instead of the default 40. The conformance schema check rejects a budget stated without outcomes or with fewer than 1 move, and the conformance README documents it.action_succession_fan_out_one_guard_holdsstates 20 moves: every run ends within 12. At 40 moves, z3 takes about a minute locally to show that every schedule ends (unsat). On CI that ran past the referee's five-minute timeout. An explicit-fork rewrite of the same model times out at 40 moves even locally, so the cost comes from the fork encoding, not from the fan-out. The completion query still proves that every run ends within the bound.action_succession_fan_out.Gates run locally:
go build ./...,go vet ./...andgofmt -l .(empty).go test ./...make lint,make docs-checkandpython3 scripts/changelog.py check.go test -raceoninternal/exec/runtime,internal/exec/smtandtests/parser.OPENSYSML_REQUIRE_*variables set.training_examples_expected.txtis untouched.Checklist
make testandmake lintpass locally (go test ./...in full,-raceon the touched packages)changes/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build)F4,K5) in the body, docs, or changelogLink to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/0274f5a5856342c6802690fe0df9478e
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/0274f5a5856342c6802690fe0df9478e?variant=devin
Requested by: @HuiJun