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/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/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/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 37fad35..78b51f9 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) @@ -226,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/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/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.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..bc036cd 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() @@ -304,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/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/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/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/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/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.go b/state/parallel.go index fa531cb..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) @@ -113,9 +117,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 +204,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 +229,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 +243,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/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/invariant.go b/state/verify/invariant.go index fb19a18..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. @@ -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..9548707 100644 --- a/state/verify/ir.go +++ b/state/verify/ir.go @@ -21,15 +21,10 @@ 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 { +// 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{}} - ir, ok := loadIR(m) - if !ok { - return t - } for i := range ir.States { collectStates(&ir.States[i], "", &t) } @@ -53,11 +48,9 @@ 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 || !ir.HasInitial { +// 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..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. @@ -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/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) + } + }) + } +} 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 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 {