From fb664538ac968e880e4232cc084b8d74ae754f8c Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Fri, 5 Jun 2026 10:12:27 -0400 Subject: [PATCH 1/4] fix: propagate state engine partial failures and remove nondeterminism Parallel region transitions now surface a failing action as an ActionFailedError classified OutcomeEffectError, matching the flat and compound commit path instead of silently discarding the error. The expr assign merge decodes the base context with json UseNumber so a large int64 sibling field survives the round-trip exactly, and a rich guard/assign records its catalog entry before mutating the registry so a duplicate name no longer leaves a half-registered reducer behind. Evolution Diff keys guarded sibling branches by their guard signature so removing or retargeting one branch of a same-event fork is no longer hidden, while a lone transition that gains or loses a guard still reads as a guard change. Analysis surfaces a duplicate_state finding when two declared states collide on their rendered name rather than silently merging them. Conformance RunAgainst rejects a scenario whose declared InitialState disagrees with the resolved start state. Symbolic Overlaps iterates transition groups in first-seen order for deterministic output. Signed-off-by: Joshua Temple --- state/analysis/analysis.go | 33 +++++- state/analysis/analysis_test.go | 49 +++++++++ state/conformance/errors.go | 16 +++ state/conformance/scenario.go | 24 +++- state/conformance/units_test.go | 50 +++++++++ state/evolution/evolution.go | 98 ++++++++++++++--- state/evolution/evolution_test.go | 76 +++++++++++++ state/expr/assign.go | 39 +++++-- state/expr/assign_test.go | 18 +++ state/expr/guard.go | 5 +- state/parallel.go | 24 +++- state/parallel_actionerror_test.go | 145 +++++++++++++++++++++++++ state/verify/symbolic/overlaps.go | 10 +- state/verify/symbolic/overlaps_test.go | 47 ++++++++ 14 files changed, 595 insertions(+), 39 deletions(-) create mode 100644 state/parallel_actionerror_test.go diff --git a/state/analysis/analysis.go b/state/analysis/analysis.go index f9cee75..4466362 100644 --- a/state/analysis/analysis.go +++ b/state/analysis/analysis.go @@ -52,6 +52,15 @@ const ( // that are always false at run time, the state is stuck despite looking live // here. Skipped entirely when the machine declares no final states. KindCannotReachFinal Kind = "cannot_reach_final" + + // KindDuplicateState marks two distinct declared states that flatten to the + // same name. The analysis graph keys nodes by the state's rendered name, so a + // collision (two states in different regions or branches whose names render + // identically) would silently merge into one node and quietly mask the other + // state's reachability, dead-end, and liveness defects. Surfacing it as a + // finding makes the ambiguity explicit instead of analyzing a graph that does + // not match the declared machine. + KindDuplicateState Kind = "duplicate_state" ) // Severity ranks a finding's seriousness. @@ -154,6 +163,16 @@ func Analyze[S comparable, E comparable, C any](m *state.Machine[S, E, C], opts } var r Report + if cfg.enabled(KindDuplicateState) { + for _, name := range g.duplicates { + r.Findings = append(r.Findings, Finding{ + Kind: KindDuplicateState, + Severity: SeverityError, + State: name, + Message: fmt.Sprintf("state %q is declared more than once; its analysis node collides and other declarations are masked", name), + }) + } + } if cfg.enabled(KindUnreachableState) { checkUnreachable(g, &r) } @@ -271,6 +290,9 @@ type graph struct { initial string hasInitial bool hasFinal bool + // duplicates names every state that collided with an already-recorded node + // during flatten, in declaration order, so Analyze can surface the ambiguity. + duplicates []string } // buildGraph serializes the machine to its IR and flattens the (possibly @@ -349,8 +371,15 @@ func flatten[S comparable, E comparable, C any](g *graph, s *state.State[S, E, C } } - g.nodes[name] = n - g.order = append(g.order, name) + if _, exists := g.nodes[name]; exists { + // A second state flattened to a name already recorded: the graph would + // otherwise silently merge the two. Record the collision so Analyze reports + // it; keep the first node so the rest of the pass stays deterministic. + g.duplicates = append(g.duplicates, name) + } else { + g.nodes[name] = n + g.order = append(g.order, name) + } for i := range s.Children { flatten(g, &s.Children[i], name) diff --git a/state/analysis/analysis_test.go b/state/analysis/analysis_test.go index dc78388..3a95d1e 100644 --- a/state/analysis/analysis_test.go +++ b/state/analysis/analysis_test.go @@ -88,6 +88,55 @@ func TestUnreachableState(t *testing.T) { } } +// collKey is a state type whose String renders only its Name, so two distinct +// keys with the same Name collide when the analysis graph flattens them by their +// rendered name. It models the real hazard: distinct typed states (here in +// separate branches) that are indistinguishable once stringified. +type collKey struct { + ID int + Name string +} + +func (c collKey) String() string { return c.Name } + +// TestDuplicateState_CollisionIsReported proves that two distinct states whose +// rendered names collide are surfaced as a duplicate_state error rather than +// being silently merged into one analysis node. +func TestDuplicateState_CollisionIsReported(t *testing.T) { + a := collKey{ID: 1, Name: "dup"} + b := collKey{ID: 2, Name: "dup"} + final := collKey{ID: 3, Name: "done"} + + m := state.Forge[collKey, string, any]("coll"). + State(a). + Transition(a).On("go").GoTo(final). + State(b). + Transition(b).On("go").GoTo(final). + State(final).Final(). + Initial(a). + Quench() + + r := analysis.Analyze(m) + dups := r.OfKind(analysis.KindDuplicateState) + if len(dups) == 0 { + t.Fatalf("expected a duplicate_state finding for the colliding name; report:\n%s", r) + } + for _, f := range dups { + if f.State != "dup" { + t.Fatalf("duplicate_state finding state = %q, want dup", f.State) + } + if f.Severity != analysis.SeverityError { + t.Fatalf("duplicate_state finding should be an error, got %s", f.Severity) + } + } + + // Without restricts the pass: excluding the kind suppresses the finding. + rr := analysis.Analyze(m, analysis.Without(analysis.KindDuplicateState)) + if len(rr.OfKind(analysis.KindDuplicateState)) != 0 { + t.Fatalf("Without(KindDuplicateState) should suppress the finding; report:\n%s", rr) + } +} + func TestDeadTransition(t *testing.T) { // The self-loop on the unreachable "orphan" can never fire. m := forge("dead-transition"). diff --git a/state/conformance/errors.go b/state/conformance/errors.go index a81342f..d714b19 100644 --- a/state/conformance/errors.go +++ b/state/conformance/errors.go @@ -23,6 +23,22 @@ func (e *ErrUnknownEvent) Error() string { return fmt.Sprintf("conformance: scenario references unknown event %q", e.Name) } +// ErrInitialStateMismatch is returned when a scenario declares a non-empty +// InitialState that does not match the typed start state the caller resolved for +// the run. Running anyway would silently replay the event sequence from a +// different state than the serialized scenario describes, so the run is rejected. +type ErrInitialStateMismatch struct { + // Declared is the scenario's serialized InitialState. + Declared string + // Resolved is the rendered form of the typed start state passed by the caller. + Resolved string +} + +func (e *ErrInitialStateMismatch) Error() string { + return fmt.Sprintf("conformance: scenario initial state %q does not match the resolved start state %q", + e.Declared, e.Resolved) +} + // Mismatch is one field-level divergence found by an oracle comparison. type Mismatch struct { // Scenario is the name of the scenario whose run diverged. diff --git a/state/conformance/scenario.go b/state/conformance/scenario.go index 37fad35..5600f24 100644 --- a/state/conformance/scenario.go +++ b/state/conformance/scenario.go @@ -156,8 +156,13 @@ func (r ScenarioResult[S]) Passed() bool { // RunAgainst fires the scenario's event sequence against a freshly Cast instance // of the machine and builds a ScenarioResult. The codec resolves each event name // to its typed value; an unresolved name is a fatal scenario error. The entity -// is supplied by the caller (the kernel binds guards and actions to it) and the -// starting state is taken from the scenario. +// is supplied by the caller (the kernel binds guards and actions to it). +// +// The run starts from the typed startState the caller resolved. When the scenario +// also declares a non-empty InitialState, it must match startState's rendered +// form; a disagreement is reported as ErrInitialStateMismatch and the events are +// not fired, so a serialized scenario can never silently replay from a different +// state than it describes. func RunAgainst[S comparable, E comparable, C any]( m *state.Machine[S, E, C], sc Scenario, @@ -165,11 +170,22 @@ func RunAgainst[S comparable, E comparable, C any]( codec EventCodec[E], startState S, ) ScenarioResult[S] { + res := ScenarioResult[S]{FinalState: startState} + tr := Trace{MachineID: m.Name(), FromState: sc.InitialState} + + if sc.InitialState != "" { + if resolved := fmt.Sprint(startState); resolved != sc.InitialState { + res.Err = &ErrInitialStateMismatch{Declared: sc.InitialState, Resolved: resolved} + tr.ToState = resolved + res.Trace = tr + res.Assertions = evaluate(sc.Assertions, res) + return res + } + } + // Full trace is required so EffectsEmitted, GuardsEvaluated, and the cascade // fields are populated for scenario assertion evaluation. inst := m.Cast(entity, state.WithInitialState(startState), state.WithFullTrace[S]()) - res := ScenarioResult[S]{FinalState: startState} - tr := Trace{MachineID: m.Name(), FromState: sc.InitialState} for _, ev := range sc.Events { typed, ok := codec.Resolve(ev.Event) diff --git a/state/conformance/units_test.go b/state/conformance/units_test.go index 0bb0ad1..e193e36 100644 --- a/state/conformance/units_test.go +++ b/state/conformance/units_test.go @@ -2,6 +2,7 @@ package conformance_test import ( "encoding/json" + "errors" "strings" "testing" @@ -114,6 +115,55 @@ func TestRunAgainst_RecordsEffectsAndTrace(t *testing.T) { } } +func TestRunAgainst_InitialStateMismatchIsRejected(t *testing.T) { + m := buildDocMachine() + // The scenario declares it starts in Submitted, but the caller resolves the + // start state to draft: the events must not be replayed from the wrong state. + sc := conformance.Scenario{ + MachineID: "document", InitialState: "Submitted", + Events: []conformance.Event{{Event: "Submit"}}, + Assertions: []conformance.Assertion{ + {Type: conformance.AssertNoErrors, Expected: true}, + }, + } + res := conformance.RunAgainst(m, sc, newDoc(), docCodec(), draft) + + var mismatch *conformance.ErrInitialStateMismatch + if !errors.As(res.Err, &mismatch) { + t.Fatalf("err = %v, want *ErrInitialStateMismatch", res.Err) + } + if mismatch.Declared != "Submitted" || mismatch.Resolved != "Draft" { + t.Fatalf("mismatch = %+v, want Declared=Submitted Resolved=Draft", mismatch) + } + // No events were fired: the run aborted before stepping the instance. + if len(res.Trace.Steps) != 0 { + t.Fatalf("trace steps = %d, want 0 (run aborted)", len(res.Trace.Steps)) + } + // The NoErrors assertion must observe the recorded error and fail. + if res.Passed() { + t.Fatal("a mismatched scenario must not pass its NoErrors assertion") + } +} + +func TestRunAgainst_EmptyInitialStateSkipsCheck(t *testing.T) { + m := buildDocMachine() + // An empty InitialState defers entirely to the caller's start state. + sc := conformance.Scenario{ + MachineID: "document", + Events: []conformance.Event{{Event: "Submit"}}, + Assertions: []conformance.Assertion{ + {Type: conformance.AssertNoErrors, Expected: true}, + }, + } + res := conformance.RunAgainst(m, sc, newDoc(), docCodec(), draft) + if res.Err != nil { + t.Fatalf("empty InitialState should skip the consistency check, got err %v", res.Err) + } + if len(res.Trace.Steps) != 1 { + t.Fatalf("trace steps = %d, want 1", len(res.Trace.Steps)) + } +} + func TestRunAgainst_FailingAssertionsMarkedNotPassed(t *testing.T) { m := buildDocMachine() sc := conformance.Scenario{ diff --git a/state/evolution/evolution.go b/state/evolution/evolution.go index 038f852..1540315 100644 --- a/state/evolution/evolution.go +++ b/state/evolution/evolution.go @@ -302,13 +302,56 @@ type transitionKey struct { on string } +// diffTransitions compares the transitions of one state across two definitions. +// Transitions are grouped by (From, On); within a group the old and new branches +// are matched so that guarded sibling branches on the same event are diffed +// independently rather than collapsed into a single map slot. +// +// When a group has exactly one branch on each side, the two are paired directly +// regardless of guard signature, so adding or removing a guard on a lone +// transition reads as a guard add/remove (a loosening/flagged-tightening) rather +// than a transition add+remove. When either side has multiple branches, branches +// are matched by guard signature: a branch whose guard set has no counterpart is +// reported as added or removed, surfacing a breaking change that the old (From, +// On)-only key silently hid. func (d *differ[S, E, C]) diffTransitions(statePath string, oldTr, newTr []state.Transition[S, E, C]) { - oldByKey := indexTransitions(oldTr) - newByKey := indexTransitions(newTr) + oldGroups := groupTransitions(oldTr) + newGroups := groupTransitions(newTr) - for key, ot := range oldByKey { + for key, oldBranches := range oldGroups { tpath := statePath + "/" + key.on - nt, ok := newByKey[key] + newBranches := newGroups[key] + d.diffTransitionGroup(tpath, key, oldBranches, newBranches) + } + for key, newBranches := range newGroups { + if _, ok := oldGroups[key]; ok { + continue + } + tpath := statePath + "/" + key.on + d.diffTransitionGroup(tpath, key, nil, newBranches) + } +} + +// diffTransitionGroup diffs the branches sharing one (From, On) key. +func (d *differ[S, E, C]) diffTransitionGroup( + tpath string, key transitionKey, oldBranches, newBranches []state.Transition[S, E, C], +) { + // Single branch on each side: pair directly so a guard added/removed on a lone + // transition is classified as a guard change, not an add+remove. + if len(oldBranches) == 1 && len(newBranches) == 1 { + d.diffTransition(tpath, oldBranches[0], newBranches[0]) + return + } + + // Multiple branches: match by guard signature so guarded siblings stay distinct. + newBySig := make(map[string]state.Transition[S, E, C], len(newBranches)) + for _, nt := range newBranches { + newBySig[guardSignature(nt)] = nt + } + matched := make(map[string]bool, len(newBranches)) + for _, ot := range oldBranches { + sig := guardSignature(ot) + nt, ok := newBySig[sig] if !ok { d.r.add(Change{ Kind: KindTransitionRemoved, @@ -318,19 +361,19 @@ func (d *differ[S, E, C]) diffTransitions(statePath string, oldTr, newTr []state }) continue } + matched[sig] = true d.diffTransition(tpath, ot, nt) } - - for key := range newByKey { - if _, ok := oldByKey[key]; !ok { - tpath := statePath + "/" + key.on - d.r.add(Change{ - Kind: KindTransitionAdded, - Path: tpath, - Description: fmt.Sprintf("new transition on %q from an existing state; existing Fire calls unaffected", key.on), - Breaking: false, - }) + for _, nt := range newBranches { + if matched[guardSignature(nt)] { + continue } + d.r.add(Change{ + Kind: KindTransitionAdded, + Path: tpath, + Description: fmt.Sprintf("new transition on %q from an existing state; existing Fire calls unaffected", key.on), + Breaking: false, + }) } } @@ -416,14 +459,35 @@ func guardRefs[S comparable, E comparable, C any](t state.Transition[S, E, C]) [ return refs } -func indexTransitions[S comparable, E comparable, C any](trs []state.Transition[S, E, C]) map[transitionKey]state.Transition[S, E, C] { - m := make(map[transitionKey]state.Transition[S, E, C], len(trs)) +// groupTransitions buckets transitions by their (From, On) key, preserving every +// guarded sibling branch in declaration order rather than collapsing them. +func groupTransitions[S comparable, E comparable, C any](trs []state.Transition[S, E, C]) map[transitionKey][]state.Transition[S, E, C] { + m := make(map[transitionKey][]state.Transition[S, E, C], len(trs)) for _, t := range trs { - m[transitionKey{from: str(t.From), on: str(t.On)}] = t + key := transitionKey{from: str(t.From), on: str(t.On)} + m[key] = append(m[key], t) } return m } +// guardSignature renders a transition's guard requirements as a stable string so +// guarded sibling branches on the same (from, on) get distinct transition keys. +// The signature folds the plain guard refs together with the composite guard's +// named-ref and stateIn leaves, sorted, so it is order-independent and +// deterministic across runs. +func guardSignature[S comparable, E comparable, C any](t state.Transition[S, E, C]) string { + refs := guardRefs(t) + if len(refs) == 0 { + return "" + } + names := make([]string, 0, len(refs)) + for _, r := range refs { + names = append(names, r.Name) + } + sort.Strings(names) + return strings.Join(names, ",") +} + func refNameSet(refs []state.Ref) map[string]struct{} { m := make(map[string]struct{}, len(refs)) for _, r := range refs { diff --git a/state/evolution/evolution_test.go b/state/evolution/evolution_test.go index bfe8670..374bded 100644 --- a/state/evolution/evolution_test.go +++ b/state/evolution/evolution_test.go @@ -232,6 +232,82 @@ func TestDiff_RemoveGuard_Additive(t *testing.T) { } } +// guardedBranchMachine: a single state "router" routing event "go" to one of two +// targets depending on a guard. This is the canonical "same event, different +// guard -> different target" pattern; the two branches share (From, On) but are +// distinct transitions. +func guardedBranchMachine() *state.IR[string, string, any] { + return &state.IR[string, string, any]{ + Name: "router", + Initial: "router", + HasInitial: true, + States: []state.State[string, string, any]{ + {Name: "router", Transitions: []state.Transition[string, string, any]{ + {From: "router", On: "go", To: "fast", Guards: []state.Ref{{Name: "isPriority"}}}, + {From: "router", On: "go", To: "slow", Guards: []state.Ref{{Name: "isStandard"}}}, + }}, + {Name: "fast", IsFinal: true}, + {Name: "slow", IsFinal: true}, + }, + } +} + +// TestDiff_GuardedBranches_RemovedBranchIsBreaking proves that removing one of two +// guarded branches sharing (From, On) is reported as a breaking transition +// removal. Keying transitions only by (From, On) would collapse the two branches +// into one slot and silently hide the removal. +func TestDiff_GuardedBranches_RemovedBranchIsBreaking(t *testing.T) { + old := guardedBranchMachine() + updated := guardedBranchMachine() + // Drop the isStandard -> slow branch entirely. + updated.States[0].Transitions = updated.States[0].Transitions[:1] + + r := evolution.Diff(old, updated) + if !r.Breaking() { + t.Fatalf("removing a guarded branch must be breaking, got:\n%s", r) + } + if !hasKind(r, evolution.KindTransitionRemoved) { + t.Fatalf("expected transition_removed for the dropped branch, got:\n%s", r) + } +} + +// TestDiff_GuardedBranches_RetargetOneBranchIsBreaking proves that retargeting a +// single guarded branch (leaving its sibling untouched) surfaces a breaking +// retarget. The (From, On)-only key kept whichever branch the map saw last, so a +// retarget of the other branch was invisible. +func TestDiff_GuardedBranches_RetargetOneBranchIsBreaking(t *testing.T) { + old := guardedBranchMachine() + updated := guardedBranchMachine() + // Retarget the isStandard branch from slow to fast. + updated.States[0].Transitions[1].To = "fast" + + r := evolution.Diff(old, updated) + if !r.Breaking() { + t.Fatalf("retargeting a guarded branch must be breaking, got:\n%s", r) + } + if !hasKind(r, evolution.KindTransitionRetargeted) { + t.Fatalf("expected transition_retargeted for the changed branch, got:\n%s", r) + } +} + +// TestDiff_GuardedBranches_AddBranchIsAdditive proves that adding a third guarded +// branch on an existing (From, On) is additive, not a spurious breaking change. +func TestDiff_GuardedBranches_AddBranchIsAdditive(t *testing.T) { + old := guardedBranchMachine() + updated := guardedBranchMachine() + updated.States = append(updated.States, state.State[string, string, any]{Name: "express", IsFinal: true}) + updated.States[0].Transitions = append(updated.States[0].Transitions, + state.Transition[string, string, any]{From: "router", On: "go", To: "express", Guards: []state.Ref{{Name: "isExpress"}}}) + + r := evolution.Diff(old, updated) + if r.Breaking() { + t.Fatalf("adding a guarded branch must be additive, got:\n%s", r) + } + if !hasKind(r, evolution.KindTransitionAdded) { + t.Fatalf("expected transition_added for the new branch, got:\n%s", r) + } +} + func TestDiff_EffectAndRemoval(t *testing.T) { old := docMachine() updated := docMachine() diff --git a/state/expr/assign.go b/state/expr/assign.go index f406442..0b2ae86 100644 --- a/state/expr/assign.go +++ b/state/expr/assign.go @@ -1,6 +1,7 @@ package expr import ( + "bytes" "encoding/json" "fmt" "reflect" @@ -60,8 +61,8 @@ func Assign[C any](reg *state.Registry[C], name, source string, schema state.Con return fmt.Errorf("assign %q: build program: %w", name, err) } - reg.Reducer(name, celAssign[C](program, schema)) - + // Record the catalog entry before mutating the registry so a duplicate-name + // collision fails authoring without leaving a half-registered reducer behind. if cfg.catalog != nil { astBytes, err := checkedASTBytes(ast) if err != nil { @@ -75,6 +76,8 @@ func Assign[C any](reg *state.Registry[C], name, source string, schema state.Con return fmt.Errorf("assign %q: %w", name, err) } } + + reg.Reducer(name, celAssign[C](program, schema)) return nil } @@ -96,10 +99,12 @@ func celAssign[C any](program cel.Program, schema state.ContextSchema) state.Ass if err != nil { return in.Entity } - // ConvertToNative to the map[string]any target type yields that type or an - // error, so the assertion is safe; an empty update set is a no-op. - updates := native.(map[string]any) - if len(updates) == 0 { + // ConvertToNative to the map[string]any target type yields that type on + // success, but guard the assertion defensively: a reducer is total and must + // never panic, so an unexpected dynamic type is treated as a no-op. An empty + // update set is likewise a no-op. + updates, ok := native.(map[string]any) + if !ok || len(updates) == 0 { return in.Entity } return mergeUpdates(in.Entity, updates) @@ -110,13 +115,18 @@ func celAssign[C any](program cel.Program, schema state.ContextSchema) state.Ass // projection: the prior context is marshaled to a map, the updates replace the named // keys, and the result is unmarshaled back into a fresh context value. A marshaling // failure leaves the entity unchanged. +// +// The base map is decoded with UseNumber so that numeric fields not named by the +// update set keep their exact textual representation rather than being widened to +// float64. Without it, an int64 sibling larger than 2^53 would lose precision on +// the re-marshal round-trip even though the assign never touched it. func mergeUpdates[C any](entity C, updates map[string]any) C { base, err := json.Marshal(entity) if err != nil { return entity } - var m map[string]any - if err = json.Unmarshal(base, &m); err != nil { + m, err := decodeMapUseNumber(base) + if err != nil { return entity } for k, v := range updates { @@ -132,3 +142,16 @@ func mergeUpdates[C any](entity C, updates map[string]any) C { } return next } + +// decodeMapUseNumber unmarshals a JSON object into a map while preserving every +// number as a json.Number, so large integers survive the merge round-trip without +// being coerced to float64. +func decodeMapUseNumber(data []byte) (map[string]any, error) { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + var m map[string]any + if err := dec.Decode(&m); err != nil { + return nil, err + } + return m, nil +} diff --git a/state/expr/assign_test.go b/state/expr/assign_test.go index 5b9587a..8ad1891 100644 --- a/state/expr/assign_test.go +++ b/state/expr/assign_test.go @@ -4,6 +4,7 @@ import ( "context" "math" "testing" + "time" "github.com/stablekernel/crucible/state" "github.com/stablekernel/crucible/state/expr" @@ -136,6 +137,23 @@ func TestAssign_UnmarshalableContextIsNoOp(t *testing.T) { } } +// TestAssign_PreservesLargeInt64Sibling confirms that an assign which only touches +// one field leaves an untouched int64 sibling (here a large time.Duration) exact, +// rather than corrupting it through a float64 JSON round-trip. A value above 2^53 +// nanoseconds cannot be represented exactly as a float64, so a non-UseNumber merge +// would silently mangle it. +func TestAssign_PreservesLargeInt64Sibling(t *testing.T) { + // 2^53 + 7 nanoseconds: the smallest range where float64 loses integer precision. + const huge = time.Duration((int64(1) << 53) + 7) + got := fireAssign(t, `{"status": "updated"}`, order{Status: "orig", Window: huge}) + if got.Status != "updated" { + t.Fatalf("status = %q, want updated", got.Status) + } + if got.Window != huge { + t.Fatalf("window = %d, want %d (large int64 sibling must survive the merge exactly)", got.Window, huge) + } +} + // TestAssign_RejectsNonMapResult fails authoring when the expression does not // evaluate to a map of field updates. func TestAssign_RejectsNonMapResult(t *testing.T) { diff --git a/state/expr/guard.go b/state/expr/guard.go index e011358..5ab614f 100644 --- a/state/expr/guard.go +++ b/state/expr/guard.go @@ -77,8 +77,8 @@ func Guard[S comparable, C any]( return state.GuardNode[S]{}, fmt.Errorf("guard %q: build program: %w", name, err) } - reg.BindGuard(name, celGuard[C]{program: program, schema: schema}) - + // Record the catalog entry before mutating the registry so a duplicate-name + // collision fails authoring without leaving a half-bound guard behind. if cfg.catalog != nil { astBytes, err := checkedASTBytes(ast) if err != nil { @@ -93,6 +93,7 @@ func Guard[S comparable, C any]( } } + reg.BindGuard(name, celGuard[C]{program: program, schema: schema}) return richNode[S](name), nil } diff --git a/state/parallel.go b/state/parallel.go index fa531cb..0d967a2 100644 --- a/state/parallel.go +++ b/state/parallel.go @@ -113,9 +113,14 @@ func (i *Instance[S, E, C]) fireParallel(ctx context.Context, parallel S, event } // regionErrOutcome maps a region-level error to the appropriate outcome, -// mirroring the main commit path: assign reducer failures use -// OutcomeAssignFailed while guard failures use OutcomeGuardFailed. +// mirroring the main commit path: a bound action that errored surfaces as +// OutcomeEffectError, an assign reducer panic as OutcomeAssignFailed, and a +// failing guard predicate as OutcomeGuardFailed. func regionErrOutcome(err error) Outcome { + var afe *ActionFailedError + if errors.As(err, &afe) { + return OutcomeEffectError + } var ap *AssignPanicError if errors.As(err, &ap) { return OutcomeAssignFailed @@ -195,8 +200,11 @@ func (i *Instance[S, E, C]) applyRegionTransition( for _, s := range exits { tr.recordExit(m.label(s)) if n, ok := m.resolveNode(s); ok { - eff, _, _ := i.runActions(n.state.OnExit, entity, tr) + eff, errName, err := i.runActions(n.state.OnExit, entity, tr) effects = append(effects, eff...) + if err != nil { + return effects, &ActionFailedError{TransitionName: transName(leaf, to), ActionName: errName, Cause: err} + } next, _, aErr := i.applyAssigns(n.state.OnExitAssign, i.entity, eventData, tr) if aErr != nil { return effects, aErr @@ -217,8 +225,11 @@ func (i *Instance[S, E, C]) applyRegionTransition( // Swap this region's leaf in the configuration. i.replaceRegionLeaf(r, leaf, m.descendToLeaves(to)) - eff, _, _ := i.runActions(t.Effects, entity, tr) + eff, errName, err := i.runActions(t.Effects, entity, tr) effects = append(effects, eff...) + if err != nil { + return effects, &ActionFailedError{TransitionName: transName(leaf, to), ActionName: errName, Cause: err} + } next, _, aErr := i.applyAssigns(t.Assigns, i.entity, eventData, tr) if aErr != nil { return effects, aErr @@ -228,8 +239,11 @@ func (i *Instance[S, E, C]) applyRegionTransition( for _, s := range entries { tr.recordEntry(m.label(s)) if n, ok := m.resolveNode(s); ok { - eff, _, _ := i.runActions(n.state.OnEntry, entity, tr) + eff, errName, err := i.runActions(n.state.OnEntry, entity, tr) effects = append(effects, eff...) + if err != nil { + return effects, &ActionFailedError{TransitionName: transName(leaf, to), ActionName: errName, Cause: err} + } next, _, aErr := i.applyAssigns(n.state.OnEntryAssign, i.entity, eventData, tr) if aErr != nil { return effects, aErr diff --git a/state/parallel_actionerror_test.go b/state/parallel_actionerror_test.go new file mode 100644 index 0000000..f41e22c --- /dev/null +++ b/state/parallel_actionerror_test.go @@ -0,0 +1,145 @@ +package state_test + +import ( + "context" + "errors" + "testing" + + "github.com/stablekernel/crucible/state" +) + +// regionActionErrorMachine builds a parallel machine whose "work" region has a +// transition wIdle->wArmed on "arm" that runs a failing transition effect, plus +// variants that fail on an exited substate's OnExit action and on an entered +// substate's OnEntry action. The "side" region holds a single flat leaf so the +// parallel state stays active while "work" advances. +func regionActionErrorMachine(boom error, on string) *state.Machine[string, string, *trec] { + f := state.Forge[string, string, *trec]("region-action-error"). + Action("fail", func(state.ActionCtx[*trec]) (state.Effect, error) { + return nil, boom + }). + State("off"). + Transition("off").On("go").GoTo("par"). + SuperState("par"). + Region("work"). + Initial("wIdle"). + SubState("wIdle") + + // The exited leaf's OnExit action fails. + if on == "exit" { + f = f.OnExit("fail") + } + f = f.SubState("wArmed") + // The entered leaf's OnEntry action fails. + if on == "entry" { + f = f.OnEntry("fail") + } + + f = f.EndRegion(). + Region("side"). + Initial("sIdle"). + SubState("sIdle"). + EndRegion(). + EndSuperState(). + Initial("off"). + CurrentStateFn(func(*trec) string { return "off" }) + + tr := f.Transition("wIdle").On("arm").GoTo("wArmed") + // The transition effect itself fails. + if on == "transition" { + tr = tr.Do("fail") + } + return tr.Quench() +} + +// TestRegion_TransitionActionError_PropagatesEffectError proves a failing action +// on a region-internal transition surfaces a typed *ActionFailedError and +// records OutcomeEffectError, consistent with the flat/compound commit path, +// instead of being silently discarded. +func TestRegion_TransitionActionError_PropagatesEffectError(t *testing.T) { + for _, tc := range []struct { + name string + on string + }{ + {"transition-effect", "transition"}, + {"exit-action", "exit"}, + {"entry-action", "entry"}, + } { + t.Run(tc.name, func(t *testing.T) { + boom := errors.New("boom") + m := regionActionErrorMachine(boom, tc.on) + inst := m.Cast(&trec{}, state.WithInitialState("off")) + ctx := context.Background() + + // Enter the parallel state cleanly. + if res := inst.Fire(ctx, "go"); res.Err != nil { + t.Fatalf("entering parallel state: %v", res.Err) + } + + res := inst.Fire(ctx, "arm") + var af *state.ActionFailedError + if !errors.As(res.Err, &af) { + t.Fatalf("err = %v, want *ActionFailedError", res.Err) + } + if !errors.Is(res.Err, boom) { + t.Fatalf("err does not unwrap to boom: %v", res.Err) + } + if res.Trace.Outcome != state.OutcomeEffectError { + t.Fatalf("outcome = %v, want OutcomeEffectError", res.Trace.Outcome) + } + }) + } +} + +// TestRegion_MultiRegionActionError_AggregatesAndClassifies proves that when two +// orthogonal regions both fail an action on the same event, the failures are +// aggregated into a *MultiRegionError whose Unwrap exposes each region's typed +// *ActionFailedError and the outcome is classified OutcomeEffectError. +func TestRegion_MultiRegionActionError_AggregatesAndClassifies(t *testing.T) { + boom := errors.New("boom") + m := state.Forge[string, string, *trec]("region-multi-error"). + Action("fail", func(state.ActionCtx[*trec]) (state.Effect, error) { + return nil, boom + }). + State("off"). + Transition("off").On("go").GoTo("par"). + SuperState("par"). + Region("a"). + Initial("aIdle"). + SubState("aIdle"). + SubState("aNext"). + EndRegion(). + Region("b"). + Initial("bIdle"). + SubState("bIdle"). + SubState("bNext"). + EndRegion(). + EndSuperState(). + Initial("off"). + CurrentStateFn(func(*trec) string { return "off" }). + Transition("aIdle").On("step").GoTo("aNext").Do("fail"). + Transition("bIdle").On("step").GoTo("bNext").Do("fail"). + Quench() + + inst := m.Cast(&trec{}, state.WithInitialState("off")) + ctx := context.Background() + if res := inst.Fire(ctx, "go"); res.Err != nil { + t.Fatalf("entering parallel state: %v", res.Err) + } + + res := inst.Fire(ctx, "step") + var mre *state.MultiRegionError + if !errors.As(res.Err, &mre) { + t.Fatalf("err = %v, want *MultiRegionError", res.Err) + } + if len(mre.Errors) != 2 { + t.Fatalf("MultiRegionError.Errors = %d, want 2", len(mre.Errors)) + } + var af *state.ActionFailedError + if !errors.As(res.Err, &af) { + t.Fatalf("MultiRegionError does not unwrap to *ActionFailedError: %v", res.Err) + } + if res.Trace.Outcome != state.OutcomeEffectError { + t.Fatalf("outcome = %v, want OutcomeEffectError", res.Trace.Outcome) + } +} diff --git a/state/verify/symbolic/overlaps.go b/state/verify/symbolic/overlaps.go index 2db60b0..4cf8066 100644 --- a/state/verify/symbolic/overlaps.go +++ b/state/verify/symbolic/overlaps.go @@ -60,12 +60,20 @@ func Overlaps[S comparable, E comparable, C any](m *state.Machine[S, E, C]) ([]O } var overlaps []Overlap[S, E] for si := range states { + // Group same-source/same-event transitions, tracking each key's first-seen + // order so the scan is deterministic: ranging the map directly would emit + // overlaps in random order across runs and break golden/idempotency checks. groups := map[key][]state.Transition[S, E, C]{} + var order []key for _, t := range states[si].Transitions { k := key{from: t.From, on: t.On} + if _, seen := groups[k]; !seen { + order = append(order, k) + } groups[k] = append(groups[k], t) } - for k, ts := range groups { + for _, k := range order { + ts := groups[k] for i := 0; i < len(ts); i++ { for j := i + 1; j < len(ts); j++ { if !Disjoint(transitionGuard(ts[i]), transitionGuard(ts[j]), schema) { diff --git a/state/verify/symbolic/overlaps_test.go b/state/verify/symbolic/overlaps_test.go index 6347b73..37991cf 100644 --- a/state/verify/symbolic/overlaps_test.go +++ b/state/verify/symbolic/overlaps_test.go @@ -55,6 +55,53 @@ func TestOverlaps_OverlappingGuardsAreReported(t *testing.T) { } } +// TestOverlaps_DeterministicOrder builds a state with several distinct overlap +// groups (different events) and asserts repeated scans return byte-identical +// ordered results. Without a deterministic group iteration order the overlaps +// would shuffle between runs and break golden assertions. +func TestOverlaps_DeterministicOrder(t *testing.T) { + build := func() *state.Machine[string, string, ctx] { + return state.Forge[string, string, ctx]("multi"). + WithContextSchema(state.SchemaOf[ctx]()). + Guard("g", func(state.GuardCtx[ctx]) bool { return true }). + State("s"). + // Three distinct events, each with two opaque-guarded competing edges. + Transition("s").On("e1").GoTo("a1").WhenExpr(state.Guard[string]("g")). + Transition("s").On("e1").GoTo("b1").WhenExpr(state.Guard[string]("g")). + Transition("s").On("e2").GoTo("a2").WhenExpr(state.Guard[string]("g")). + Transition("s").On("e2").GoTo("b2").WhenExpr(state.Guard[string]("g")). + Transition("s").On("e3").GoTo("a3").WhenExpr(state.Guard[string]("g")). + Transition("s").On("e3").GoTo("b3").WhenExpr(state.Guard[string]("g")). + State("a1").State("b1").State("a2").State("b2").State("a3").State("b3"). + Initial("s"). + Quench() + } + + first, err := symbolic.Overlaps(build()) + if err != nil { + t.Fatalf("Overlaps: %v", err) + } + if len(first) != 3 { + t.Fatalf("Overlaps = %+v, want three groups", first) + } + // Repeated scans of freshly built (structurally identical) machines must agree + // on order, run after run. + for i := 0; i < 50; i++ { + got, err := symbolic.Overlaps(build()) + if err != nil { + t.Fatalf("Overlaps[%d]: %v", i, err) + } + if len(got) != len(first) { + t.Fatalf("Overlaps[%d] length = %d, want %d", i, len(got), len(first)) + } + for j := range got { + if got[j] != first[j] { + t.Fatalf("Overlaps[%d][%d] = %+v, want %+v (nondeterministic order)", i, j, got[j], first[j]) + } + } + } +} + func TestOverlaps_OpaqueGuardsAreConservativelyReported(t *testing.T) { // Named-ref guards are opaque to the analyzer, so it cannot prove them disjoint // and reports the pair — a false positive is the safe direction for From 69fe2c86f135df704f860b1a8e0780a3b87d2062 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Fri, 5 Jun 2026 10:19:46 -0400 Subject: [PATCH 2/4] perf: cut per-fire and per-verify allocations in the state engine matchingTransitions returns the specific or wildcard slice directly when only one kind is present, so the steady-state bubble path no longer allocates a merged slice. activeParallelAncestor presizes its scratch map from the leaf count, and runActions builds the formatted effect label and probes the comm microstep only in full trace mode, keeping the lite hot path free of the Sprintf and effectLabel allocation. Verify rounds the machine through its public IR once and threads it into the topology, search-graph, and config-graph builders instead of re-serializing per builder. The config graph caches its inverted child index at build time so leavesUnder no longer rebuilds and re-sorts it on every call. A new CompileChecked binds a stored rich AST to its environment once for reuse across evaluations, with EvalCheckedAST now a one-shot wrapper over it. Signed-off-by: Joshua Temple --- state/expr/coverage_test.go | 55 +++++++++++++++++++++++++++++++ state/expr/program.go | 42 +++++++++++++++++++++--- state/fire.go | 26 ++++++++++++--- state/parallel.go | 6 +++- state/verify/invariant.go | 65 +++++++++++++++++++++++-------------- state/verify/ir.go | 20 ++++++++++-- state/verify/reach.go | 25 ++++++++++---- state/verify/verify.go | 32 +++++++++++++++--- 8 files changed, 222 insertions(+), 49 deletions(-) diff --git a/state/expr/coverage_test.go b/state/expr/coverage_test.go index 74e365e..a07ffd5 100644 --- a/state/expr/coverage_test.go +++ b/state/expr/coverage_test.go @@ -45,6 +45,61 @@ func TestEvalCheckedAST_RejectsBadBytes(t *testing.T) { } } +// TestCompileChecked_ReusesProgramAcrossEvals asserts a CompiledChecked built once +// evaluates many context values and agrees with the one-shot EvalCheckedAST on each, +// proving the cached program path is equivalent to the rebuild-every-call path. +func TestCompileChecked_ReusesProgramAcrossEvals(t *testing.T) { + reg := state.NewRegistry[order]() + cat := expr.NewCatalog() + if _, err := expr.Guard[string](reg, "big", `total > 100.0`, orderSchema(), expr.WithCatalog(cat)); err != nil { + t.Fatalf("Guard: %v", err) + } + entry, ok := cat.Entry("big") + if !ok { + t.Fatal("catalog recorded no entry") + } + + compiled, err := expr.CompileChecked(entry.CheckedAST, orderSchema()) + if err != nil { + t.Fatalf("CompileChecked: %v", err) + } + + for _, tc := range []struct { + total float64 + want bool + }{ + {total: 50, want: false}, + {total: 150, want: true}, + {total: 100, want: false}, + {total: 101, want: true}, + } { + entity := order{Total: tc.total} + got, err := compiled.Eval(entity) + if err != nil { + t.Fatalf("Eval(total=%v): %v", tc.total, err) + } + if got != tc.want { + t.Fatalf("Eval(total=%v) = %v, want %v", tc.total, got, tc.want) + } + // The reusable program agrees with the rebuild-every-call helper. + oneShot, err := expr.EvalCheckedAST(entry.CheckedAST, orderSchema(), entity) + if err != nil { + t.Fatalf("EvalCheckedAST(total=%v): %v", tc.total, err) + } + if oneShot != got { + t.Fatalf("CompiledChecked.Eval=%v disagrees with EvalCheckedAST=%v at total=%v", got, oneShot, tc.total) + } + } +} + +// TestCompileChecked_RejectsBadBytes asserts malformed checked-AST bytes fail at +// compile time rather than at evaluation. +func TestCompileChecked_RejectsBadBytes(t *testing.T) { + if _, err := expr.CompileChecked([]byte{0xff, 0xfe, 0xfd}, orderSchema()); err == nil { + t.Fatal("malformed checked-AST bytes should fail to compile") + } +} + // TestLoadCatalog_RejectsMalformedSidecar asserts a sidecar that is not an object, // and one whose checked-AST is not valid base64, are both rejected. func TestLoadCatalog_RejectsMalformedSidecar(t *testing.T) { diff --git a/state/expr/program.go b/state/expr/program.go index e61214d..27d7bf2 100644 --- a/state/expr/program.go +++ b/state/expr/program.go @@ -96,23 +96,55 @@ func checkedASTBytes(ast *cel.Ast) ([]byte, error) { // The AST is rebound against the schema-derived env so its variables resolve; an env // whose variables do not match the AST's declarations surfaces as an eval error. func EvalCheckedAST(checkedAST []byte, schema state.ContextSchema, entity any) (bool, error) { - ast, err := astFromCheckedBytes(checkedAST) + compiled, err := CompileChecked(checkedAST, schema) if err != nil { return false, err } + return compiled.Eval(entity) +} + +// CompiledChecked is a rich AST that has been rebuilt and bound to its +// schema-derived environment exactly once, ready to evaluate against many context +// values. EvalCheckedAST rebuilds the env and program on every call, which is fine +// for a one-shot tooling probe but wasteful when the same stored AST is replayed +// repeatedly; CompileChecked pays that cost once and Eval reuses the program. The +// type is immutable after construction and its Eval is safe for the same +// synchronous, single-evaluator use as a celGuard. +type CompiledChecked struct { + program cel.Program + schema state.ContextSchema +} + +// CompileChecked rebuilds a program from stored canonical cel.dev/expr CheckedExpr +// bytes and binds it to the schema-derived environment once, returning a reusable +// CompiledChecked. It performs the same env-rebuild and program-build EvalCheckedAST +// does, so a malformed AST or an env whose variables do not match the AST's +// declarations surfaces here rather than per evaluation. +func CompileChecked(checkedAST []byte, schema state.ContextSchema) (*CompiledChecked, error) { + ast, err := astFromCheckedBytes(checkedAST) + if err != nil { + return nil, err + } env, err := newEnv(schema) if err != nil { - return false, fmt.Errorf("rebuild env: %w", err) + return nil, fmt.Errorf("rebuild env: %w", err) } program, err := env.Program(ast, cel.CostLimit(defaultCostLimit)) if err != nil { - return false, fmt.Errorf("rebuild program: %w", err) + return nil, fmt.Errorf("rebuild program: %w", err) } - activation, err := marshalActivation(entity, schema) + return &CompiledChecked{program: program, schema: schema}, nil +} + +// Eval projects the entity into the bound environment and evaluates the compiled +// program, returning its boolean verdict. It reuses the program built by +// CompileChecked, so repeated evaluations skip the env/AST/program rebuild. +func (c *CompiledChecked) Eval(entity any) (bool, error) { + activation, err := marshalActivation(entity, c.schema) if err != nil { return false, err } - out, _, err := program.Eval(activation) + out, _, err := c.program.Eval(activation) if err != nil { return false, fmt.Errorf("eval: %w", err) } diff --git a/state/fire.go b/state/fire.go index 818da3c..92d6599 100644 --- a/state/fire.go +++ b/state/fire.go @@ -422,6 +422,10 @@ func (i *Instance[S, E, C]) fireSpine(ctx context.Context, event E, tr Trace) Fi // events outrank the wildcard — and the wildcard outranks bubbling to an // ancestor. func matchingTransitions[S comparable, E comparable, C any](s *State[S, E, C], event E) []*Transition[S, E, C] { + // The overwhelmingly common case is a state with one or more specific matches + // and no wildcard. Collect specifics first; only allocate a second slice and + // merge when a wildcard is actually present, so the steady-state path returns a + // single slice with no merge allocation. A state with no matches returns nil. var specific, wild []*Transition[S, E, C] for ti := range s.Transitions { t := &s.Transitions[ti] @@ -435,7 +439,15 @@ func matchingTransitions[S comparable, E comparable, C any](s *State[S, E, C], e specific = append(specific, t) } } - return append(specific, wild...) + if len(wild) == 0 { + return specific + } + if len(specific) == 0 { + return wild + } + out := make([]*Transition[S, E, C], 0, len(specific)+len(wild)) + out = append(out, specific...) + return append(out, wild...) } // forbids reports whether state s explicitly forbids event: a transition marked @@ -756,9 +768,15 @@ func (i *Instance[S, E, C]) runActions(refs []Ref, entity C, tr *Trace) (effects return effects, a.Name, aerr } effects = append(effects, e) - tr.recordEffect(fmt.Sprintf("%s:%s", a.Name, effectLabel(e))) - if ms, ok := commMicrostep(e); ok { - tr.note(ms) + // recordEffect/note are no-ops in lite mode, so build the formatted effect + // label and probe the comm microstep only when the full trace will keep them. + // This keeps the default (lite) hot path free of the Sprintf and effectLabel + // allocation on every action. + if tr.full { + tr.recordEffect(fmt.Sprintf("%s:%s", a.Name, effectLabel(e))) + if ms, ok := commMicrostep(e); ok { + tr.note(ms) + } } } return effects, "", nil diff --git a/state/parallel.go b/state/parallel.go index 0d967a2..47e8f3f 100644 --- a/state/parallel.go +++ b/state/parallel.go @@ -20,7 +20,11 @@ func (i *Instance[S, E, C]) activeParallelAncestor() (S, bool) { return zero, false } // Candidate parallel states are the ancestors shared by the config leaves. - seen := map[S]int{} + // Presize from the leaf count so the map does not grow-and-rehash while the + // orthogonal config is scanned. This runs only when already in a parallel + // configuration (len(config) >= 2); flat machines return above without + // allocating. + seen := make(map[S]int, len(i.config)) for _, leaf := range i.config { for _, anc := range m.ancestors(leaf) { n, ok := m.resolveNode(anc) diff --git a/state/verify/invariant.go b/state/verify/invariant.go index fb19a18..2fcebd8 100644 --- a/state/verify/invariant.go +++ b/state/verify/invariant.go @@ -118,23 +118,27 @@ type configGraph struct { // edges maps a source state to its path-advancing outgoing transitions in // declaration order, so exploration is deterministic. edges map[string][]searchEdge + // children inverts parent into a sorted child list per state, computed once at + // build time. leavesUnder walks it on the hot successors path, so caching it + // avoids rebuilding and re-sorting the whole index on every call. + children map[string][]string } // buildConfigGraph flattens the machine's public IR into a configGraph. A machine // whose IR cannot be read yields the zero graph (hasInitial false) rather than // panicking, matching Verify's no-panic contract. func buildConfigGraph[S comparable, E comparable, C any](m *state.Machine[S, E, C]) configGraph { - g := configGraph{ - parent: map[string]string{}, - leaf: map[string]bool{}, - compoundInitial: map[string]string{}, - regionInitials: map[string][]string{}, - edges: map[string][]searchEdge{}, - } ir, ok := loadIR(m) if !ok { - return g + return emptyConfigGraph() } + return buildConfigGraphFromIR(ir) +} + +// buildConfigGraphFromIR builds the configGraph from an already-loaded IR, so a +// caller that round-tripped the machine once can reuse it. +func buildConfigGraphFromIR[S comparable, E comparable, C any](ir *state.IR[S, E, C]) configGraph { + g := emptyConfigGraph() if ir.HasInitial { g.hasInitial = true g.initial = fmt.Sprint(ir.Initial) @@ -142,9 +146,36 @@ func buildConfigGraph[S comparable, E comparable, C any](m *state.Machine[S, E, for i := range ir.States { collectConfig(&ir.States[i], "", &g) } + g.children = buildChildIndex(g.parent) return g } +// emptyConfigGraph returns a configGraph with initialized maps and no states. +func emptyConfigGraph() configGraph { + return configGraph{ + parent: map[string]string{}, + leaf: map[string]bool{}, + compoundInitial: map[string]string{}, + regionInitials: map[string][]string{}, + edges: map[string][]searchEdge{}, + } +} + +// buildChildIndex inverts the parent map into a sorted child list per state, so a +// subtree walk is deterministic and does not range a map directly. +func buildChildIndex(parent map[string]string) map[string][]string { + idx := map[string][]string{} + for name, p := range parent { + if p != "" { + idx[p] = append(idx[p], name) + } + } + for p := range idx { + sort.Strings(idx[p]) + } + return idx +} + // collectConfig records one state's structure — parent, leaf-ness, compound and // region initial children, and path-advancing edges — and recurses through its // children and region states in declaration order. @@ -348,7 +379,6 @@ func (g configGraph) successors(leaves []string) []configStep { // it is every descendant leaf, found by walking the parent relation (which covers // both compound children and region states). func (g configGraph) leavesUnder(node string) map[string]bool { - children := g.childIndex() out := map[string]bool{} var walk func(n string) walk = func(n string) { @@ -356,7 +386,7 @@ func (g configGraph) leavesUnder(node string) map[string]bool { out[n] = true return } - for _, c := range children[n] { + for _, c := range g.children[n] { walk(c) } } @@ -367,21 +397,6 @@ func (g configGraph) leavesUnder(node string) map[string]bool { return out } -// childIndex inverts the parent map into a sorted child list per state, so a -// subtree walk is deterministic and does not range a map directly. -func (g configGraph) childIndex() map[string][]string { - idx := map[string][]string{} - for name, p := range g.parent { - if p != "" { - idx[p] = append(idx[p], name) - } - } - for p := range idx { - sort.Strings(idx[p]) - } - return idx -} - // invariantFor decides a single invariant over an already-built exploration, // returning the finding. The exploration is walked in breadth-first discovery // order so the reported counterexample is the nearest violating configuration, diff --git a/state/verify/ir.go b/state/verify/ir.go index da5e3fe..d0363e6 100644 --- a/state/verify/ir.go +++ b/state/verify/ir.go @@ -25,11 +25,17 @@ type topology struct { // cannot be read yields the zero topology rather than panicking, matching // Verify's no-panic contract. func readTopology[S comparable, E comparable, C any](m *state.Machine[S, E, C]) topology { - t := topology{parent: map[string]string{}} ir, ok := loadIR(m) if !ok { - return t + return topology{parent: map[string]string{}} } + return topologyFromIR(ir) +} + +// topologyFromIR builds a topology from an already-loaded IR, so a caller that has +// round-tripped the machine once can reuse it instead of re-serializing. +func topologyFromIR[S comparable, E comparable, C any](ir *state.IR[S, E, C]) topology { + t := topology{parent: map[string]string{}} for i := range ir.States { collectStates(&ir.States[i], "", &t) } @@ -57,7 +63,15 @@ func collectStates[S comparable, E comparable, C any](s *state.State[S, E, C], p // declares no initial state. func initialName[S comparable, E comparable, C any](m *state.Machine[S, E, C]) string { ir, ok := loadIR(m) - if !ok || !ir.HasInitial { + if !ok { + return "" + } + return initialNameFromIR(ir) +} + +// initialNameFromIR returns the initial state name from an already-loaded IR. +func initialNameFromIR[S comparable, E comparable, C any](ir *state.IR[S, E, C]) string { + if !ir.HasInitial { return "" } return fmt.Sprint(ir.Initial) diff --git a/state/verify/reach.go b/state/verify/reach.go index ed4913b..697ea25 100644 --- a/state/verify/reach.go +++ b/state/verify/reach.go @@ -65,16 +65,17 @@ type searchEdge struct { // whose IR cannot be read yields the zero graph (hasInitial false) rather than // panicking, matching Verify's no-panic contract. func buildSearchGraph[S comparable, E comparable, C any](m *state.Machine[S, E, C]) searchGraph { - g := searchGraph{ - nodes: map[string]bool{}, - parent: map[string]string{}, - initialChildren: map[string][]string{}, - edges: map[string][]searchEdge{}, - } ir, ok := loadIR(m) if !ok { - return g + return emptySearchGraph() } + return buildSearchGraphFromIR(ir) +} + +// buildSearchGraphFromIR builds the searchGraph from an already-loaded IR, so a +// caller that round-tripped the machine once can reuse it. +func buildSearchGraphFromIR[S comparable, E comparable, C any](ir *state.IR[S, E, C]) searchGraph { + g := emptySearchGraph() if ir.HasInitial { g.hasInitial = true g.initial = fmt.Sprint(ir.Initial) @@ -85,6 +86,16 @@ func buildSearchGraph[S comparable, E comparable, C any](m *state.Machine[S, E, return g } +// emptySearchGraph returns a searchGraph with initialized maps and no states. +func emptySearchGraph() searchGraph { + return searchGraph{ + nodes: map[string]bool{}, + parent: map[string]string{}, + initialChildren: map[string][]string{}, + edges: map[string][]searchEdge{}, + } +} + // collectSearch records one state's structure — parent, initial-descent children, // and path-advancing edges — and recurses through its children and region states // in declaration order. diff --git a/state/verify/verify.go b/state/verify/verify.go index f230308..ae67b46 100644 --- a/state/verify/verify.go +++ b/state/verify/verify.go @@ -334,8 +334,22 @@ func Verify[S comparable, E comparable, C any](m *state.Machine[S, E, C], opts . o(&cfg) } - res := &Result{initial: initialName(m)} - top := readTopology(m) + // Round-trip the machine to its public IR once and reuse it across every + // structural builder below. Each builder previously re-serialized the machine, + // so a single Verify paid for the JSON marshal/unmarshal several times over. + ir, irOK := loadIR(m) + + var ( + res *Result + top topology + ) + if irOK { + res = &Result{initial: initialNameFromIR(ir)} + top = topologyFromIR(ir) + } else { + res = &Result{initial: ""} + top = topology{parent: map[string]string{}} + } // Authoritative reachability comes from the analysis package's static pass: // its KindUnreachableState finding is the proven verdict, and it correctly @@ -386,7 +400,12 @@ func Verify[S comparable, E comparable, C any](m *state.Machine[S, E, C], opts . // their own structural search, so build the search graph once when either is // requested. if len(cfg.reachAvoiding) > 0 || len(cfg.alwaysEventually) > 0 { - g := buildSearchGraph(m) + var g searchGraph + if irOK { + g = buildSearchGraphFromIR(ir) + } else { + g = emptySearchGraph() + } for _, q := range cfg.reachAvoiding { if !g.nodes[q.target] { @@ -414,7 +433,12 @@ func Verify[S comparable, E comparable, C any](m *state.Machine[S, E, C], opts . // whole configurations of co-active leaves over the configuration-product space, // so they share one configGraph, built once when any is requested. if len(cfg.invariants) > 0 || len(cfg.boundedSims) > 0 || cfg.coverageRequested { - cg := buildConfigGraph(m) + var cg configGraph + if irOK { + cg = buildConfigGraphFromIR(ir) + } else { + cg = emptyConfigGraph() + } if len(cfg.invariants) > 0 { exp := cg.explore() for _, inv := range cfg.invariants { From ea14968abc720b9aca5da1a27493bf5aad9f5f49 Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Fri, 5 Jun 2026 10:25:46 -0400 Subject: [PATCH 3/4] test: close state engine coverage gaps and fix two doc/label slips Cover the fireFromState cross-cutting branches (commit, guard fail, guard panic, forbidden, unhandled), the full-trace mergeTrace path through FireSeq, the kernel typed Error and Unwrap methods, DiffMachines plus the evolution Serialize/Decode error types, Path.States, the conformance outcomeName branches, and the symbolic litNumber int forms. conformance outcomeName now renders OutcomeAssignFailed as AssignFailed instead of collapsing it to Unknown, so an assign failure stays distinct in a scenario trace. The analysis Step godoc, previously truncated mid-sentence, now describes the From/Event/To segment fully. Signed-off-by: Joshua Temple --- state/analysis/paths.go | 8 +- state/analysis/paths_test.go | 36 ++++ state/conformance/internal_test.go | 44 +++++ state/conformance/scenario.go | 2 + state/errors_test.go | 173 ++++++++++++++++++ state/evolution/evolution_test.go | 75 ++++++++ state/fireseq_mergetrace_test.go | 69 +++++++ state/parallel_crosscut_test.go | 173 ++++++++++++++++++ state/verify/symbolic/domain_internal_test.go | 38 ++++ 9 files changed, 614 insertions(+), 4 deletions(-) create mode 100644 state/conformance/internal_test.go create mode 100644 state/errors_test.go create mode 100644 state/fireseq_mergetrace_test.go create mode 100644 state/parallel_crosscut_test.go create mode 100644 state/verify/symbolic/domain_internal_test.go diff --git a/state/analysis/paths.go b/state/analysis/paths.go index 5f0f0fc..72240f9 100644 --- a/state/analysis/paths.go +++ b/state/analysis/paths.go @@ -23,10 +23,10 @@ import ( // conformance harness draws from. // Step is one segment of a path: the event that fires from the segment's source -// state, and the destination state it leads to. A path is read as: starting at the -// initial state, fire Step[0].Event to reach Step[0].To, then Step[1].Event, and so -// on. A path segment carries its event plus the resulting -// `state`). +// state and the destination state it leads to. A path is read as: starting at the +// initial state, fire Step[0].Event to reach Step[0].To, then Step[1].Event to +// reach Step[1].To, and so on. Each segment carries its source state (From), the +// fired event (Event), and the state reached (To). type Step struct { // Event is the string label of the event fired for this segment; "always" for an // eventless transition traversed on the path. diff --git a/state/analysis/paths_test.go b/state/analysis/paths_test.go index b2e5434..2af1ad5 100644 --- a/state/analysis/paths_test.go +++ b/state/analysis/paths_test.go @@ -71,6 +71,42 @@ func TestShortestPaths_CoversAllReachable(t *testing.T) { } } +// TestPath_StatesAndEvents asserts Path.States renders the ordered states visited +// (initial first, Target last) and stays consistent with Path.Events: a path of n +// steps yields n events and n+1 states. +func TestPath_StatesAndEvents(t *testing.T) { + paths, err := analysis.ShortestPaths(feedbackMachine()) + if err != nil { + t.Fatalf("ShortestPaths: %v", err) + } + + // The empty path (initial state reaching itself) is just the initial state. + if got := paths["question"].States("question"); len(got) != 1 || got[0] != "question" { + t.Fatalf("States for the empty path = %v, want [question]", got) + } + + // A two-step path to thanks via form: question -> form -> thanks. + simple, err := analysis.SimplePaths(feedbackMachine()) + if err != nil { + t.Fatalf("SimplePaths: %v", err) + } + viaForm := simple["thanks"][1] // sorted shortest-first; [1] is the two-step route + states := viaForm.States("question") + wantStates := []string{"question", "form", "thanks"} + if len(states) != len(wantStates) { + t.Fatalf("States = %v, want %v", states, wantStates) + } + for i := range wantStates { + if states[i] != wantStates[i] { + t.Fatalf("States[%d] = %q, want %q (full %v)", i, states[i], wantStates[i], states) + } + } + // States is always one longer than Events (it includes the initial state). + if len(states) != len(viaForm.Events())+1 { + t.Fatalf("len(States)=%d, len(Events)=%d; want States == Events+1", len(states), len(viaForm.Events())) + } +} + // TestShortestPaths_MatchesPlanPath asserts that for each single target, the // shortest path's length matches the kernel's PlanPath length on the same guard-free // machine — ShortestPaths is the multi-target generalization of PlanPath. diff --git a/state/conformance/internal_test.go b/state/conformance/internal_test.go new file mode 100644 index 0000000..03f13d8 --- /dev/null +++ b/state/conformance/internal_test.go @@ -0,0 +1,44 @@ +package conformance + +import ( + "testing" + + "github.com/stablekernel/crucible/state" +) + +// TestOutcomeName_AllKernelOutcomes asserts outcomeName renders every kernel +// Outcome by a distinct stable name, so a conformance trace never collapses two +// different failure classes onto the same label (which would make a regression +// compare equal). An out-of-range value falls back to "Unknown". +func TestOutcomeName_AllKernelOutcomes(t *testing.T) { + cases := []struct { + outcome state.Outcome + want string + }{ + {state.OutcomeSuccess, "Success"}, + {state.OutcomeInvalidTransition, "InvalidTransition"}, + {state.OutcomeGuardFailed, "GuardFailed"}, + {state.OutcomeGuardPanic, "GuardPanic"}, + {state.OutcomePolicyDenied, "PolicyDenied"}, + {state.OutcomeEffectError, "EffectError"}, + {state.OutcomeAssignFailed, "AssignFailed"}, + {state.Outcome(-1), "Unknown"}, + {state.Outcome(9999), "Unknown"}, + } + + seen := map[string]state.Outcome{} + for _, tc := range cases { + got := outcomeName(tc.outcome) + if got != tc.want { + t.Fatalf("outcomeName(%d) = %q, want %q", tc.outcome, got, tc.want) + } + // Every named (non-Unknown) outcome must be distinct from the others, so a + // trace diff distinguishes the failure classes. + if tc.want != "Unknown" { + if prev, dup := seen[got]; dup { + t.Fatalf("outcomeName collision: %d and %d both render %q", prev, tc.outcome, got) + } + seen[got] = tc.outcome + } + } +} diff --git a/state/conformance/scenario.go b/state/conformance/scenario.go index 5600f24..78b51f9 100644 --- a/state/conformance/scenario.go +++ b/state/conformance/scenario.go @@ -242,6 +242,8 @@ func outcomeName(o state.Outcome) string { return "PolicyDenied" case state.OutcomeEffectError: return "EffectError" + case state.OutcomeAssignFailed: + return "AssignFailed" default: return "Unknown" } diff --git a/state/errors_test.go b/state/errors_test.go new file mode 100644 index 0000000..6866572 --- /dev/null +++ b/state/errors_test.go @@ -0,0 +1,173 @@ +package state_test + +import ( + "errors" + "strings" + "testing" + "time" + + "github.com/stablekernel/crucible/state" +) + +// TestTypedErrors_Messages asserts every typed kernel error renders a non-empty, +// self-describing message that names the offending entity. These are the +// diagnostic surface a host logs and matches on, so the messages are part of the +// contract. +func TestTypedErrors_Messages(t *testing.T) { + cases := []struct { + name string + err error + contains []string + }{ + { + name: "InvalidTransition with target", + err: &state.InvalidTransitionError{From: "a", To: "b", Event: "go", Reason: "guard failed"}, + contains: []string{"invalid transition", "a", "b", "go", "guard failed"}, + }, + { + name: "InvalidTransition without target", + err: &state.InvalidTransitionError{From: "a", Event: "go", Reason: "no transition"}, + contains: []string{"invalid transition", "a", "go", "no transition"}, + }, + { + name: "GuardFailed", + err: &state.GuardFailedError{GuardName: "isReady", Reason: "predicate returned false"}, + contains: []string{"guard", "isReady", "predicate returned false"}, + }, + { + name: "GuardPanic", + err: &state.GuardPanicError{GuardName: "isReady", Recovered: "nil deref"}, + contains: []string{"guard", "isReady", "panicked", "nil deref"}, + }, + { + name: "AssignPanic", + err: &state.AssignPanicError{AssignName: "fold", Recovered: "boom"}, + contains: []string{"assign", "fold", "panicked", "boom"}, + }, + { + name: "PolicyDenied", + err: &state.PolicyDeniedError{PolicyName: "rbac", Reason: "no role"}, + contains: []string{"policy", "rbac", "denied", "no role"}, + }, + { + name: "UndeclaredState", + err: &state.UndeclaredStateError{State: "ghost"}, + contains: []string{"undeclared state", "ghost"}, + }, + { + name: "UnboundRef", + err: &state.UnboundRefError{Kind: "guard", Name: "g"}, + contains: []string{"unbound", "guard", "g"}, + }, + { + name: "MicrostepOverflow", + err: &state.MicrostepOverflowError{Limit: 64, State: "loop"}, + contains: []string{"run-to-completion", "64", "loop"}, + }, + { + name: "NoPath", + err: &state.NoPathError{From: "a", To: "z"}, + contains: []string{"no path", "a", "z"}, + }, + { + name: "WaitTimeout", + err: &state.WaitTimeoutError{Machine: "m", Timeout: 5 * time.Second, Last: "idle"}, + contains: []string{"WaitFor", "m", "5s", "idle"}, + }, + { + name: "NoInitialState", + err: &state.NoInitialStateError{Machine: "m"}, + contains: []string{"cannot Cast", "m", "no CurrentStateFn"}, + }, + { + name: "UnknownBuiltin", + err: &state.UnknownBuiltinError{Name: "crucible.bogus"}, + contains: []string{"unknown built-in action", "crucible.bogus"}, + }, + { + name: "UnboundActor", + err: &state.UnboundActorError{Name: "child"}, + contains: []string{"unbound actor ref", "child"}, + }, + { + name: "Snapshot with state", + err: &state.SnapshotError{Op: "restore", State: "leaf", Reason: "not declared"}, + contains: []string{"snapshot restore", "leaf", "not declared"}, + }, + { + name: "Snapshot without state", + err: &state.SnapshotError{Op: "marshal", Reason: "encode failed"}, + contains: []string{"snapshot marshal", "encode failed"}, + }, + { + name: "SnapshotVersion", + err: &state.SnapshotVersionError{Kind: "machineVersion", Machine: "m", Got: "2", Want: "1", Reason: "major bump"}, + contains: []string{"version mismatch", "machineVersion", "m", "2", "1", "major bump"}, + }, + { + name: "UnsupportedSchema", + err: &state.UnsupportedSchemaError{Got: "2.0", Supported: "1.0"}, + contains: []string{"unsupported schema version", "2.0", "1.0"}, + }, + { + name: "UnknownEffectKind", + err: &state.UnknownEffectKindError{Kind: "foreign"}, + contains: []string{"unknown effect kind", "foreign"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + msg := tc.err.Error() + if msg == "" { + t.Fatal("error message is empty") + } + for _, want := range tc.contains { + if !strings.Contains(msg, want) { + t.Fatalf("message %q does not contain %q", msg, want) + } + } + }) + } +} + +// TestActionFailedError_WrapsCause asserts ActionFailedError renders its context +// and unwraps to the underlying cause for errors.Is / errors.As. +func TestActionFailedError_WrapsCause(t *testing.T) { + cause := errors.New("downstream boom") + err := &state.ActionFailedError{TransitionName: "a->b", ActionName: "charge", Cause: cause} + + msg := err.Error() + for _, want := range []string{"action", "charge", "a->b", "downstream boom"} { + if !strings.Contains(msg, want) { + t.Fatalf("message %q does not contain %q", msg, want) + } + } + if !errors.Is(err, cause) { + t.Fatal("ActionFailedError should unwrap to its cause") + } +} + +// TestMultiRegionError_AggregatesAndUnwraps asserts MultiRegionError renders each +// region's message and exposes them for errors.As traversal. +func TestMultiRegionError_AggregatesAndUnwraps(t *testing.T) { + g := &state.GuardFailedError{GuardName: "g", Reason: "false"} + a := &state.AssignPanicError{AssignName: "fold", Recovered: "boom"} + err := &state.MultiRegionError{Errors: []error{g, a}} + + msg := err.Error() + for _, want := range []string{"2 regions errored", "g", "fold"} { + if !strings.Contains(msg, want) { + t.Fatalf("message %q does not contain %q", msg, want) + } + } + + var gf *state.GuardFailedError + if !errors.As(err, &gf) { + t.Fatal("MultiRegionError should expose a *GuardFailedError via errors.As") + } + var ap *state.AssignPanicError + if !errors.As(err, &ap) { + t.Fatal("MultiRegionError should expose an *AssignPanicError via errors.As") + } +} diff --git a/state/evolution/evolution_test.go b/state/evolution/evolution_test.go index 374bded..bc036cd 100644 --- a/state/evolution/evolution_test.go +++ b/state/evolution/evolution_test.go @@ -380,6 +380,81 @@ func TestDiff_NestedChildAdded_Additive(t *testing.T) { } } +// TestDiffMachines_AgreesWithDiff drives the Quenched-machine entry point and +// asserts it classifies a breaking change identically to Diff over the same IRs. +func TestDiffMachines_AgreesWithDiff(t *testing.T) { + oldM := state.Forge[string, string, any]("doc"). + State("draft"). + Transition("draft").On("submit").GoTo("review"). + State("review"). + Transition("review").On("approve").GoTo("done"). + State("done").Final(). + Initial("draft"). + Quench() + // The updated machine drops the review->done transition: a breaking removal. + newM := state.Forge[string, string, any]("doc"). + State("draft"). + Transition("draft").On("submit").GoTo("review"). + State("review"). + State("done").Final(). + Initial("draft"). + Quench() + + r, err := evolution.DiffMachines(oldM, newM) + if err != nil { + t.Fatalf("DiffMachines: %v", err) + } + if !r.Breaking() { + t.Fatalf("DiffMachines should report the removal as breaking, got:\n%s", r) + } + if !hasKind(r, evolution.KindTransitionRemoved) { + t.Fatalf("expected transition_removed, got:\n%s", r) + } +} + +// TestDiffMachines_IdenticalIsEmpty asserts DiffMachines over the same definition +// reports no change. +func TestDiffMachines_IdenticalIsEmpty(t *testing.T) { + build := func() *state.Machine[string, string, any] { + return state.Forge[string, string, any]("doc"). + State("draft"). + Transition("draft").On("submit").GoTo("done"). + State("done").Final(). + Initial("draft"). + Quench() + } + r, err := evolution.DiffMachines(build(), build()) + if err != nil { + t.Fatalf("DiffMachines: %v", err) + } + if !r.Empty() { + t.Fatalf("identical machines should diff empty, got:\n%s", r) + } +} + +// TestEvolutionErrorTypes_FormatAndUnwrap covers the Error and Unwrap methods of +// SerializeError and DecodeError directly, since a Machine that fails to serialize +// cannot be produced through the normal Forge/Quench path. +func TestEvolutionErrorTypes_FormatAndUnwrap(t *testing.T) { + cause := errors.New("boom") + + se := &evolution.SerializeError{Side: "old", Err: cause} + if !strings.Contains(se.Error(), "serialize old machine") { + t.Fatalf("SerializeError.Error() = %q", se.Error()) + } + if !errors.Is(se, cause) { + t.Fatal("SerializeError should unwrap to its cause") + } + + de := &evolution.DecodeError{Side: "new", Err: cause} + if !strings.Contains(de.Error(), "decode new machine") { + t.Fatalf("DecodeError.Error() = %q", de.Error()) + } + if !errors.Is(de, cause) { + t.Fatal("DecodeError should unwrap to its cause") + } +} + func TestDiffJSON_RoundTrip(t *testing.T) { old := docMachine() updated := docMachine() diff --git a/state/fireseq_mergetrace_test.go b/state/fireseq_mergetrace_test.go new file mode 100644 index 0000000..2ab40e4 --- /dev/null +++ b/state/fireseq_mergetrace_test.go @@ -0,0 +1,69 @@ +package state_test + +import ( + "context" + "testing" + + "github.com/stablekernel/crucible/state" +) + +// TestFireSeq_FullTraceMergesPerStepDiagnostics drives a guarded, effect-emitting +// machine through FireSeq in full-trace mode and asserts the merged Trace +// accumulates each step's guards and effects, exercising the full-mode merge +// branch of mergeTrace (which is a no-op in lite mode). +func TestFireSeq_FullTraceMergesPerStepDiagnostics(t *testing.T) { + m := state.Forge[string, string, *trec]("seq"). + Guard("ok", func(state.GuardCtx[*trec]) bool { return true }). + Action("note", noteAction("do")). + State("a"). + Transition("a").On("go").GoTo("b").When("ok").Do("note", state.P{"t": "first"}). + State("b"). + Transition("b").On("go").GoTo("c").When("ok").Do("note", state.P{"t": "second"}). + State("c").Final(). + Initial("a"). + Quench() + + inst := m.Cast(&trec{}, state.WithInitialState("a"), state.WithFullTrace[string]()) + br := inst.FireSeq(context.Background(), []string{"go", "go"}) + + if br.Err != nil { + t.Fatalf("FireSeq errored: %v", br.Err) + } + if len(br.Steps) != 2 { + t.Fatalf("steps = %d, want 2", len(br.Steps)) + } + if br.Trace.Outcome != state.OutcomeSuccess { + t.Fatalf("merged outcome = %v, want success", br.Trace.Outcome) + } + // Both steps evaluated the "ok" guard, so the merged trace carries both. + if len(br.Trace.GuardsEvaluated) != 2 { + t.Fatalf("merged GuardsEvaluated = %v, want two entries", br.Trace.GuardsEvaluated) + } + // Both steps emitted the "note" effect, merged in order. + if len(br.Trace.EffectsEmitted) != 2 { + t.Fatalf("merged EffectsEmitted = %v, want two entries", br.Trace.EffectsEmitted) + } +} + +// TestFireSeq_LiteTraceDoesNotMerge confirms that in the default lite mode the +// merged trace stays empty of the rich per-step fields: mergeTrace short-circuits. +func TestFireSeq_LiteTraceDoesNotMerge(t *testing.T) { + m := state.Forge[string, string, *trec]("seq-lite"). + Guard("ok", func(state.GuardCtx[*trec]) bool { return true }). + Action("note", noteAction("do")). + State("a"). + Transition("a").On("go").GoTo("b").When("ok").Do("note", state.P{"t": "first"}). + State("b").Final(). + Initial("a"). + Quench() + + inst := m.Cast(&trec{}, state.WithInitialState("a")) // lite by default + br := inst.FireSeq(context.Background(), []string{"go"}) + if br.Err != nil { + t.Fatalf("FireSeq errored: %v", br.Err) + } + if len(br.Trace.GuardsEvaluated) != 0 || len(br.Trace.EffectsEmitted) != 0 { + t.Fatalf("lite merged trace should carry no rich fields, got guards=%v effects=%v", + br.Trace.GuardsEvaluated, br.Trace.EffectsEmitted) + } +} diff --git a/state/parallel_crosscut_test.go b/state/parallel_crosscut_test.go new file mode 100644 index 0000000..00a47e6 --- /dev/null +++ b/state/parallel_crosscut_test.go @@ -0,0 +1,173 @@ +package state_test + +import ( + "context" + "errors" + "testing" + + "github.com/stablekernel/crucible/state" +) + +// crosscutMachine builds a parallel state "par" with two single-leaf regions. The +// regions handle "tick" internally; the parallel state declares a cross-cutting +// transition on "abort" to "done" (exiting all regions). A guard on the abort edge +// is supplied so the guarded-cross-cutting branch is exercised. "done" is final. +func crosscutMachine(allowAbort bool) *state.Machine[string, string, *trec] { + return state.Forge[string, string, *trec]("crosscut"). + Guard("canAbort", func(state.GuardCtx[*trec]) bool { return allowAbort }). + State("off"). + Transition("off").On("go").GoTo("par"). + SuperState("par"). + // The cross-cutting transition lives on the parallel superstate: no region + // consumes "abort", so it bubbles to fireFromState. + Transition("par").On("abort").GoTo("done").When("canAbort"). + Region("a"). + Initial("aIdle"). + SubState("aIdle"). + EndRegion(). + Region("b"). + Initial("bIdle"). + SubState("bIdle"). + EndRegion(). + EndSuperState(). + State("done").Final(). + Initial("off"). + CurrentStateFn(func(*trec) string { return "off" }). + Quench() +} + +// TestFireFromState_CrossCuttingTransitionCommits proves an event no region +// handles bubbles from the parallel state to a guarded cross-cutting transition +// that commits, exiting the parallel configuration. +func TestFireFromState_CrossCuttingTransitionCommits(t *testing.T) { + m := crosscutMachine(true) + inst := m.Cast(&trec{}, state.WithInitialState("off")) + ctx := context.Background() + if res := inst.Fire(ctx, "go"); res.Err != nil { + t.Fatalf("entering parallel: %v", res.Err) + } + + res := inst.Fire(ctx, "abort") + if res.Err != nil { + t.Fatalf("cross-cutting abort errored: %v", res.Err) + } + if res.NewState != "done" { + t.Fatalf("state = %v, want done", res.NewState) + } + if res.Trace.Outcome != state.OutcomeSuccess { + t.Fatalf("outcome = %v, want success", res.Trace.Outcome) + } +} + +// TestFireFromState_CrossCuttingGuardFails proves a cross-cutting transition whose +// guard fails leaves the configuration in the parallel state and reports an +// invalid transition (no other candidate handled the event). +func TestFireFromState_CrossCuttingGuardFails(t *testing.T) { + m := crosscutMachine(false) + inst := m.Cast(&trec{}, state.WithInitialState("off")) + ctx := context.Background() + if res := inst.Fire(ctx, "go"); res.Err != nil { + t.Fatalf("entering parallel: %v", res.Err) + } + + res := inst.Fire(ctx, "abort") + var ite *state.InvalidTransitionError + if !errors.As(res.Err, &ite) { + t.Fatalf("err = %v, want *InvalidTransitionError (guard blocked the only candidate)", res.Err) + } + // The parallel configuration is unchanged. + cfg := inst.Configuration() + if len(cfg) != 2 { + t.Fatalf("configuration = %v, want both regions still active", cfg) + } +} + +// TestFireFromState_CrossCuttingGuardPanics proves a panicking guard on a +// cross-cutting transition surfaces a *GuardPanicError classified +// OutcomeGuardPanic, exercising the guard-panic branch of fireFromState. +func TestFireFromState_CrossCuttingGuardPanics(t *testing.T) { + m := state.Forge[string, string, *trec]("crosscut-panic"). + Guard("boom", func(state.GuardCtx[*trec]) bool { panic("guard blew up") }). + State("off"). + Transition("off").On("go").GoTo("par"). + SuperState("par"). + Transition("par").On("abort").GoTo("done").When("boom"). + Region("a").Initial("aIdle").SubState("aIdle").EndRegion(). + Region("b").Initial("bIdle").SubState("bIdle").EndRegion(). + EndSuperState(). + State("done").Final(). + Initial("off"). + CurrentStateFn(func(*trec) string { return "off" }). + Quench() + + inst := m.Cast(&trec{}, state.WithInitialState("off")) + ctx := context.Background() + if res := inst.Fire(ctx, "go"); res.Err != nil { + t.Fatalf("entering parallel: %v", res.Err) + } + + res := inst.Fire(ctx, "abort") + var gpe *state.GuardPanicError + if !errors.As(res.Err, &gpe) { + t.Fatalf("err = %v, want *GuardPanicError", res.Err) + } + if res.Trace.Outcome != state.OutcomeGuardPanic { + t.Fatalf("outcome = %v, want OutcomeGuardPanic", res.Trace.Outcome) + } +} + +// TestFireFromState_ForbiddenEventIsConsumed proves an event forbidden on the +// parallel state is consumed (success, no state change) rather than bubbling, +// exercising the forbids branch of fireFromState. +func TestFireFromState_ForbiddenEventIsConsumed(t *testing.T) { + m := state.Forge[string, string, *trec]("crosscut-forbid"). + State("off"). + Transition("off").On("go").GoTo("par"). + SuperState("par"). + Forbid("abort"). + Region("a").Initial("aIdle").SubState("aIdle").EndRegion(). + Region("b").Initial("bIdle").SubState("bIdle").EndRegion(). + EndSuperState(). + State("done").Final(). + Initial("off"). + CurrentStateFn(func(*trec) string { return "off" }). + Quench() + + inst := m.Cast(&trec{}, state.WithInitialState("off")) + ctx := context.Background() + if res := inst.Fire(ctx, "go"); res.Err != nil { + t.Fatalf("entering parallel: %v", res.Err) + } + + res := inst.Fire(ctx, "abort") + if res.Err != nil { + t.Fatalf("forbidden event should be consumed cleanly, got err %v", res.Err) + } + if res.Trace.Outcome != state.OutcomeSuccess { + t.Fatalf("outcome = %v, want success (forbidden, consumed)", res.Trace.Outcome) + } + if len(inst.Configuration()) != 2 { + t.Fatalf("configuration = %v, want both regions still active", inst.Configuration()) + } +} + +// TestFireFromState_UnhandledEventIsInvalid proves an event neither a region nor a +// cross-cutting transition handles surfaces an InvalidTransitionError naming the +// event, exercising the no-candidate branch of fireFromState. +func TestFireFromState_UnhandledEventIsInvalid(t *testing.T) { + m := crosscutMachine(true) + inst := m.Cast(&trec{}, state.WithInitialState("off")) + ctx := context.Background() + if res := inst.Fire(ctx, "go"); res.Err != nil { + t.Fatalf("entering parallel: %v", res.Err) + } + + res := inst.Fire(ctx, "nonsense") + var ite *state.InvalidTransitionError + if !errors.As(res.Err, &ite) { + t.Fatalf("err = %v, want *InvalidTransitionError", res.Err) + } + if ite.Event != "nonsense" { + t.Fatalf("invalid transition event = %q, want nonsense", ite.Event) + } +} diff --git a/state/verify/symbolic/domain_internal_test.go b/state/verify/symbolic/domain_internal_test.go new file mode 100644 index 0000000..923778f --- /dev/null +++ b/state/verify/symbolic/domain_internal_test.go @@ -0,0 +1,38 @@ +package symbolic + +import ( + "testing" + + "github.com/stablekernel/crucible/state" +) + +// TestLitNumber_NumericForms asserts litNumber extracts a float64 from every +// supported numeric Go form a Literal.Value can carry — float64, int64, int, and +// int32 — and rejects a non-numeric value. The int forms arise from hand-built or +// cross-package literals, not just the int64 the Int constructor emits. +func TestLitNumber_NumericForms(t *testing.T) { + for _, tc := range []struct { + name string + value any + want float64 + ok bool + }{ + {"float64", float64(3.5), 3.5, true}, + {"int64", int64(7), 7, true}, + {"int", int(9), 9, true}, + {"int32", int32(11), 11, true}, + {"string-not-numeric", "nope", 0, false}, + {"bool-not-numeric", true, 0, false}, + {"nil", nil, 0, false}, + } { + t.Run(tc.name, func(t *testing.T) { + got, ok := litNumber(state.Literal{Value: tc.value}) + if ok != tc.ok { + t.Fatalf("litNumber(%v) ok = %v, want %v", tc.value, ok, tc.ok) + } + if ok && got != tc.want { + t.Fatalf("litNumber(%v) = %v, want %v", tc.value, got, tc.want) + } + }) + } +} From 0476844b3fc9647f32467c0d5558a0fbb76472fb Mon Sep 17 00:00:00 2001 From: Joshua Temple Date: Fri, 5 Jun 2026 10:33:37 -0400 Subject: [PATCH 4/4] refactor: drop verify IR helpers orphaned by single-roundtrip readTopology and initialName became unused when the perf commit threaded a single loadIR result through Verify's builders via the FromIR variants. Remove both wrapper functions; update the two prose comments in reach.go and invariant.go that named readTopology. Signed-off-by: Joshua Temple --- state/verify/invariant.go | 2 +- state/verify/ir.go | 25 ++----------------------- state/verify/reach.go | 2 +- 3 files changed, 4 insertions(+), 25 deletions(-) diff --git a/state/verify/invariant.go b/state/verify/invariant.go index 2fcebd8..dc2403b 100644 --- a/state/verify/invariant.go +++ b/state/verify/invariant.go @@ -98,7 +98,7 @@ func NeverActive(s string) Invariant { // configGraph is the configuration-product view of a machine's IR: the structural // information needed to enumerate every reachable configuration of active leaves. -// It is built from the same serialized public IR readTopology flattens, so a +// It is built from the same serialized public IR Verify loads once, so a // code-built and a JSON-loaded machine explore identically. type configGraph struct { // initial is the machine's initial state name; hasInitial guards the empty case. diff --git a/state/verify/ir.go b/state/verify/ir.go index d0363e6..9548707 100644 --- a/state/verify/ir.go +++ b/state/verify/ir.go @@ -21,19 +21,8 @@ type topology struct { parent map[string]string } -// readTopology flattens the machine's IR into a topology. A machine whose IR -// cannot be read yields the zero topology rather than panicking, matching -// Verify's no-panic contract. -func readTopology[S comparable, E comparable, C any](m *state.Machine[S, E, C]) topology { - ir, ok := loadIR(m) - if !ok { - return topology{parent: map[string]string{}} - } - return topologyFromIR(ir) -} - -// topologyFromIR builds a topology from an already-loaded IR, so a caller that has -// round-tripped the machine once can reuse it instead of re-serializing. +// topologyFromIR builds a topology from an already-loaded IR. Verify loads the IR +// once and passes it here rather than letting each builder re-serialize. func topologyFromIR[S comparable, E comparable, C any](ir *state.IR[S, E, C]) topology { t := topology{parent: map[string]string{}} for i := range ir.States { @@ -59,16 +48,6 @@ func collectStates[S comparable, E comparable, C any](s *state.State[S, E, C], p } } -// initialName returns the machine's initial state name, or "" when the machine -// declares no initial state. -func initialName[S comparable, E comparable, C any](m *state.Machine[S, E, C]) string { - ir, ok := loadIR(m) - if !ok { - return "" - } - return initialNameFromIR(ir) -} - // initialNameFromIR returns the initial state name from an already-loaded IR. func initialNameFromIR[S comparable, E comparable, C any](ir *state.IR[S, E, C]) string { if !ir.HasInitial { diff --git a/state/verify/reach.go b/state/verify/reach.go index 697ea25..f750cc8 100644 --- a/state/verify/reach.go +++ b/state/verify/reach.go @@ -33,7 +33,7 @@ import ( // searchGraph is the constrained-search view of a machine's IR: the structural // edges plus, for each node, the active-configuration members its entry brings -// online. It is built from the same serialized public IR readTopology flattens, +// online. It is built from the same serialized public IR Verify loads once, // so a code-built and a JSON-loaded machine search identically. type searchGraph struct { // initial is the machine's initial state name; hasInitial guards the empty case.