From 2ebda26e19bc690f87ae6d316f41ada46d52d279 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:01:12 +0500 Subject: [PATCH 1/8] Add TryFire and TrySend, where a refusal is no news A controller that fires an event on every turn and lets guards decide whether there is anything to do paid for an error value on every quiet turn, and had to sort refusals from failures by type. TryFire reports a state that does not take the event, or a guard that rejects it, as false with a nil error, and allocates nothing; a failed action and a state written by a callback are still errors. Fire keeps its whole body: splitting it into lookup and apply steps cost BenchmarkFire six nanoseconds a round trip, since a private generic method is not inlined. TryFire and Check share lookup, and TryFire carries a transition out through apply, a copy of Fire's tail, which TestTryFireRunsTheHooksFireDoes and the fuzzer keep in step with Fire. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 14 +++- README.md | 8 ++ docs/DESIGN.md | 8 ++ fsm.go | 124 +++++++++++++++++++++++++++--- fsm_test.go | 202 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 345 insertions(+), 11 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4b581ef..e484080 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,8 +8,8 @@ generic methods, with no dependency outside the standard library. that way. This file is the working detail behind it; the two are edited together. -Library: `fsm.go` (package doc, `Event`, `Machine`, `Fire`, `Send`, `Check`, -`Can`, `To`, the four error types), `rules.go` (`New`/`MustNew`, the `Rule` +Library: `fsm.go` (package doc, `Event`, `Machine`, `Fire`, `Send`, `TryFire`, +`TrySend`, `Check`, `Can`, `To`, the four error types), `rules.go` (`New`/`MustNew`, the `Rule` interface and everything that produces one — the `From`/`FromEach`/`FromGroup` chain through `On`/`To` with its `Guard`/`Action` methods, plus `Rules`, `Group`, `OnEnter`, `OnExit`, `OnEnterVia`, `OnExitVia`, `OnEnterWith`, @@ -84,6 +84,16 @@ must keep that pairing, or the assertion becomes a runtime panic on a path that is supposed to return errors. `TestFireDoesNotAllocate` is the guard on the allocation half; it runs without `-race`, which changes the profile. +**`Fire` is one function, on purpose.** A call to a private generic method is +not inlined, and splitting `Fire` into a lookup and an apply step cost +`BenchmarkFire` six nanoseconds a round trip. So `Fire` keeps its whole body; +`TryFire` and `Check` share `lookup`, and `TryFire` carries a transition out +through `apply`, a copy of `Fire`'s tail. `TestTryFireRunsTheHooksFireDoes` +fires the same transitions both ways with every kind of hook and compares the +logs, and `FuzzMachineInvariants` checks that `TryFire` fires exactly when +`To` resolves: those are what keep the copy honest. A refusal from `TryFire` +builds no error value (`TestTryFireRefusalDoesNotAllocate`). + **Multiple guards and actions are combined at `On` time, not at `Fire` time,** for the same reason: the combining closure is built where `A` is still known. Guards run in registration order and the first rejection wins, reported under diff --git a/README.md b/README.md index 3a37370..e4f4cce 100644 --- a/README.md +++ b/README.md @@ -188,6 +188,14 @@ tr, err := recordingFSM.Send(ctx, &r.state, evRecUpload) // for events declared `Fire` returns the `Transition` it made: `tr.From`, `tr.To`, and `tr.Event()` for display. Names are not unique, so branch with `tr.Is(evRecStop)`, never on the string. +A caller that fires whenever an event *might* apply — a controller reconciling on every turn — wants a refusal to be no news: + +```go +tr, fired, err := m.TryFire(ctx, &st, evReconcile, x) // fired is false, err nil, when nothing applies +``` + +`TryFire` (and `TrySend`) report a state that does not take the event, or a guard that rejects it, as `false` with a nil error, and allocate nothing doing so. A failed action, or a state written by a callback, is still an error. + To ask without firing: ```go diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 2325a68..cf195a7 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -220,6 +220,14 @@ same error without firing, when you want the reason but not the transition; When several guards are registered on one transition they run in order and the first rejection wins, reported under the description it was declared with. +A refusal is not always news. A controller that fires `reconcile` on every turn +and lets the guards say whether there is anything to do would pay for an error +value on every quiet turn, and would have to tell refusals from failures by +type. `TryFire` reports a missing transition or a rejecting guard as `false` +with a nil error, and allocates nothing; the failures that still are errors are +the ones that mean something went wrong: an action that failed, a callback that +wrote the state. + ## Entry/exit hooks cannot fail They exist for bookkeeping that must stay paired with the state change. Work diff --git a/fsm.go b/fsm.go index b653d2a..9787427 100644 --- a/fsm.go +++ b/fsm.go @@ -264,12 +264,122 @@ func (m *Machine[S]) Fire[A any](ctx context.Context, st *S, ev Event[A], arg A) return t, nil } +// TryFire is [Machine.Fire] for a caller that fires whenever an event might +// apply — a controller that reconciles on every turn, say — to which a +// refusal is no news. A state that does not accept ev, or a guard that +// rejects it, reports false with a nil error and allocates nothing; the +// state is untouched and no hook runs. A failed action and a state written +// by a guard or action are still errors, as from Fire, and so are a nil +// state pointer and the zero Event. +// +// Use Fire, or [Machine.Check], when the reason for a refusal matters. +func (m *Machine[S]) TryFire[A any](ctx context.Context, st *S, ev Event[A], arg A) (Transition[S], bool, error) { + var none Transition[S] + if err := m.callerBug(st, ev.def, "fired"); err != nil { + return none, false, err + } + from := *st + to, ok, desc, err := m.lookup(ctx, from, ev, arg) + switch { + case !ok: + return none, false, nil + case *st != from: + return none, false, &StateChangedError[S]{Machine: m.name, From: from, To: to, Found: *st, Event: ev.def.name, Guard: desc, Err: err} + case err != nil: + return none, false, nil + } + t, err := m.apply(ctx, st, from, to, ev, arg) + return t, err == nil, err +} + +// callerBug reports a nil state pointer or the zero Event, which are a +// caller's mistakes rather than machine events. +func (m *Machine[S]) callerBug(st *S, def *eventDef, verb string) error { + if st == nil { + return fmt.Errorf("fsm %s: nil state pointer", m.name) + } + if def == nil { + return fmt.Errorf("fsm %s: %s the zero Event; declare it with fsm.Define or fsm.Signal", m.name, verb) + } + return nil +} + +// lookup finds where ev leads from from, and runs its guards. desc names the +// guard that rejected, if one did. ev's A is the one the row's guard was +// registered with, which is what makes the assertion safe. +func (m *Machine[S]) lookup[A any](ctx context.Context, from S, ev Event[A], arg A) (to S, ok bool, desc string, err error) { + e := edge[S]{from: from, ev: ev.def} + if to, ok = m.table[e]; !ok { + return to, false, "", nil + } + if raw, guarded := m.guards[e]; guarded { + desc, err = raw.(func(context.Context, A) (string, error))(ctx, arg) + } + return to, true, desc, err +} + +// apply carries out a transition its guards allowed: the action, then the +// hooks around the assignment. It is the tail of Fire, which keeps its own +// copy rather than call this: the call costs Fire about two nanoseconds. +func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Event[A], arg A) (Transition[S], error) { + var none Transition[S] + e := edge[S]{from: from, ev: ev.def} + if raw, ok := m.actions[e]; ok { + // A write is reported ahead of a failure, as for guards: the caller + // must learn that *st no longer holds from. + err := raw.(func(context.Context, A) error)(ctx, arg) + if *st != from { + return none, &StateChangedError[S]{Machine: m.name, From: from, To: to, Found: *st, Event: ev.def.name, Err: err} + } + if err != nil { + return none, &ActionError[S]{Machine: m.name, From: from, To: to, Event: ev.def.name, Err: err} + } + } + + t := Transition[S]{From: from, To: to, trigger: ev.def} + if !m.hasHooks { + *st = to + return t, nil + } + + for _, h := range m.onExit[from] { + h(ctx, t) + } + if len(m.onExitVia) > 0 { + for _, raw := range m.onExitVia[e] { + raw.(func(context.Context, Transition[S], A))(ctx, t, arg) + } + } + + *st = to + + // Before the entry hooks, so an entry hook that fires again logs after + // this one. That is why a transition hook must not fire itself. + for _, h := range m.onAll { + h(ctx, t) + } + for _, h := range m.onEnter[to] { + h(ctx, t) + } + if len(m.onEnterVia) > 0 { + for _, raw := range m.onEnterVia[edge[S]{from: to, ev: ev.def}] { + raw.(func(context.Context, Transition[S], A))(ctx, t, arg) + } + } + return t, nil +} + // Send fires an event that carries no payload. It is [Machine.Fire] with the // payload fixed to Unit{}. func (m *Machine[S]) Send(ctx context.Context, st *S, ev Event[Unit]) (Transition[S], error) { return m.Fire(ctx, st, ev, Unit{}) } +// TrySend is [Machine.TryFire] for an event that carries no payload. +func (m *Machine[S]) TrySend(ctx context.Context, st *S, ev Event[Unit]) (Transition[S], bool, error) { + return m.TryFire(ctx, st, ev, Unit{}) +} + // Check reports why firing ev from state from would fail, evaluating guards // but running no action and no hook. It returns nil when the transition would // be allowed, and otherwise the same error [Machine.Fire] would return. @@ -280,16 +390,12 @@ func (m *Machine[S]) Check[A any](ctx context.Context, from S, ev Event[A], arg if ev.def == nil { return fmt.Errorf("fsm %s: checked the zero Event; declare it with fsm.Define or fsm.Signal", m.name) } - - e := edge[S]{from: from, ev: ev.def} - to, ok := m.table[e] - if !ok { + to, ok, desc, err := m.lookup(ctx, from, ev, arg) + switch { + case !ok: return &NoTransitionError[S]{Machine: m.name, From: from, Event: ev.def.name} - } - if raw, ok := m.guards[e]; ok { - if desc, err := raw.(func(context.Context, A) (string, error))(ctx, arg); err != nil { - return &GuardError[S]{Machine: m.name, From: from, To: to, Event: ev.def.name, Guard: desc, Err: err} - } + case err != nil: + return &GuardError[S]{Machine: m.name, From: from, To: to, Event: ev.def.name, Guard: desc, Err: err} } return nil } diff --git a/fsm_test.go b/fsm_test.go index 660aef1..c9f8193 100644 --- a/fsm_test.go +++ b/fsm_test.go @@ -1380,6 +1380,9 @@ func TestZeroEventIsAnErrorNotAPanic(t *testing.T) { if to, ok := m.To(idle, zero); ok { t.Errorf("To resolved %v for the zero Event", to) } + if _, fired, err := m.TryFire(t.Context(), &st, zero, 0); fired || err == nil || !strings.Contains(err.Error(), "zero Event") { + t.Errorf("TryFire: fired=%v, err %v; want the zero Event reported", fired, err) + } } // --- Zero values, nil rules and error text -------------------------------- @@ -1427,8 +1430,193 @@ func TestFireRejectsANilStatePointer(t *testing.T) { if !strings.Contains(err.Error(), "nil state pointer") { t.Errorf("error %q does not explain the problem", err) } + if _, fired, err := m.TrySend(t.Context(), nil, evStart); fired || err == nil || !strings.Contains(err.Error(), "nil state pointer") { + t.Errorf("TrySend: fired=%v, err %v; want the nil state pointer reported", fired, err) + } +} + +// --- TryFire ---------------------------------------------------------------- + +// TryFire makes the transition Fire would, hooks and all, and says it did. +func TestTryFireFiresWhatFireWould(t *testing.T) { + t.Parallel() + + var entered []state + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done).Action(func(context.Context, int) error { return nil }), + fsm.OnEnter(running, func(_ context.Context, tr fsm.Transition[state]) { entered = append(entered, tr.To) }), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx := t.Context() + st := idle + + tr, fired, err := m.TrySend(ctx, &st, evStart) + if !fired || err != nil || st != running || tr.From != idle || tr.To != running || !tr.Is(evStart) { + t.Fatalf("TrySend: %v -> %v, fired=%v, err %v, state %v; want idle -> running", tr.From, tr.To, fired, err, st) + } + if !slices.Equal(entered, []state{running}) { + t.Errorf("entry hooks ran for %v, want [running]", entered) + } + if _, fired, err := m.TryFire(ctx, &st, evFinish, 3); !fired || err != nil || st != done { + t.Errorf("TryFire: fired=%v, err %v, state %v; want done", fired, err, st) + } +} + +// TryFire runs every kind of hook Fire does, in the same order: it carries +// out a transition with its own copy of Fire's tail, and this is what keeps +// the two from drifting apart. +func TestTryFireRunsTheHooksFireDoes(t *testing.T) { + t.Parallel() + + run := func(try bool) []string { + var log []string + note := func(what string) fsm.Hook[state] { + return func(_ context.Context, tr fsm.Transition[state]) { + log = append(log, fmt.Sprintf("%s %v->%v", what, tr.From, tr.To)) + } + } + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.OnExit(idle, note("exit")), + fsm.OnExitVia(running, evFinish, func(_ context.Context, _ fsm.Transition[state], n int) { + log = append(log, fmt.Sprintf("exit via finish %d", n)) + }), + fsm.OnTransition(note("transition")), + fsm.OnEnter(running, note("enter")), + fsm.OnEnterVia(done, evFinish, func(_ context.Context, _ fsm.Transition[state], n int) { + log = append(log, fmt.Sprintf("enter via finish %d", n)) + }), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + if try { + _, _, _ = m.TrySend(ctx, &st, evStart) + _, _, _ = m.TryFire(ctx, &st, evFinish, 7) + } else { + _, _ = m.Send(ctx, &st, evStart) + _, _ = m.Fire(ctx, &st, evFinish, 7) + } + return log + } + if fire, try := run(false), run(true); !slices.Equal(fire, try) { + t.Errorf("hooks differ:\nFire %v\nTryFire %v", fire, try) + } +} + +// A state that does not take the event, and a guard that rejects it, are +// refusals: false and no error, the state untouched, no action and no hook. +func TestTryFireReportsARefusalAsFalse(t *testing.T) { + t.Parallel() + + var ran []string + errNotYet := errors.New("not yet") + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running). + Guard("ready", func(context.Context, fsm.Unit) error { return errNotYet }). + Action(func(context.Context, fsm.Unit) error { ran = append(ran, "action"); return nil }), + fsm.OnExit(idle, func(context.Context, fsm.Transition[state]) { ran = append(ran, "exit") }), + fsm.From(running).On(evFinish).To(done), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx := t.Context() + + for _, tc := range []struct { + name string + from state + try func(*state) (fsm.Transition[state], bool, error) + }{ + {"no transition", done, func(s *state) (fsm.Transition[state], bool, error) { return m.TrySend(ctx, s, evStart) }}, + {"guard rejects", idle, func(s *state) (fsm.Transition[state], bool, error) { return m.TrySend(ctx, s, evStart) }}, + } { + st := tc.from + tr, fired, err := tc.try(&st) + if fired || err != nil || tr != (fsm.Transition[state]{}) || st != tc.from { + t.Errorf("%s: %v, fired=%v, err %v, state %v; want a quiet refusal from %v", tc.name, tr, fired, err, st, tc.from) + } + } + if len(ran) != 0 { + t.Errorf("a refusal ran %v", ran) + } } +// What would be news from Fire is news from TryFire: a failed action, and a +// state written by a guard or an action. +func TestTryFireReportsFailuresAsErrors(t *testing.T) { + t.Parallel() + + errBoom := errors.New("boom") + var st state + m, err := fsm.New("job", + fsm.From(running).On(evFinish).To(done).Action(func(context.Context, int) error { return errBoom }), + fsm.From(idle).On(evStart).To(running).Guard("writes", func(context.Context, fsm.Unit) error { + st = done + return errBoom + }), + fsm.From(running).On(evCancel).To(cancelled).Action(func(context.Context, fsm.Unit) error { + st = idle + return nil + }), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx := t.Context() + + st = running + if _, fired, err := m.TryFire(ctx, &st, evFinish, 0); fired || !errors.Is(err, errBoom) { + t.Errorf("failed action: fired=%v, err %v; want an ActionError", fired, err) + } else if _, ok := errors.AsType[*fsm.ActionError[state]](err); !ok { + t.Errorf("failed action: %T, want *fsm.ActionError", err) + } + for _, tc := range []struct { + name string + from, at state + ev fsm.Event[fsm.Unit] + }{ + {"guard", idle, done, evStart}, + {"action", running, idle, evCancel}, + } { + st = tc.from + _, fired, err := m.TrySend(ctx, &st, tc.ev) + sce, ok := errors.AsType[*fsm.StateChangedError[state]](err) + if fired || !ok || sce.Found != tc.at { + t.Errorf("%s writing the state: fired=%v, err %v; want a StateChangedError finding %v", tc.name, fired, err, tc.at) + } + } +} + +// The point of TryFire: a caller firing on every turn pays nothing for the +// turns with nothing to do. +func TestTryFireRefusalDoesNotAllocate(t *testing.T) { + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running).Guard("never", func(context.Context, fsm.Unit) error { return errNever }), + fsm.From(running).On(evFinish).To(done), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx := t.Context() + st := idle + + avg := testing.AllocsPerRun(1000, func() { + _, _, _ = m.TrySend(ctx, &st, evStart) // a guard rejects + _, _, _ = m.TryFire(ctx, &st, evFinish, 0) // idle takes no finish + }) + if avg != 0 { + t.Errorf("a refusal allocates %.1f times per round trip, want 0", avg) + } +} + +var errNever = errors.New("never") + // The error text is what a caller reads in a log, so every fire-time error // names the machine and the edge it happened on. func TestFireErrorsNameTheirEdge(t *testing.T) { @@ -2581,6 +2769,20 @@ func FuzzMachineInvariants(f *testing.F) { case !ok && st != from: t.Fatalf("Fire %s from %v failed but moved the state to %v", ev, from, st) } + // TryFire is Fire with a refusal as false: with no guards or + // actions here, it fires exactly when To resolves. + st = from + tr, fired, err := m.TryFire(ctx, &st, ev, 0) + switch { + case err != nil: + t.Fatalf("TryFire %s from %v: %v", ev, from, err) + case fired != ok: + t.Fatalf("TryFire %s from %v fired=%v, but To resolves=%v", ev, from, fired, ok) + case fired && (st != want || tr.To != want || !tr.Is(ev)): + t.Fatalf("TryFire %s from %v left %v (-> %v), want %v", ev, from, st, tr.To, want) + case !fired && st != from: + t.Fatalf("TryFire %s from %v refused but moved the state to %v", ev, from, st) + } } } }) From 743b257013ddfbaff419c075f15fa82685afe909 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:07:52 +0500 Subject: [PATCH 2/8] Add Stay, an internal transition From(a).On(ev).Stay() handles ev in a without leaving it: guards and actions run as on any transition, Fire returns a transition from a to itself, and no hook runs, OnTransition included. To(a) keeps its meaning, an external self-transition that runs exit and entry. This reverses "there is no internal transition; an Action covers that case". An action runs only as part of a transition, and the one that keeps the state runs the hooks, so an event a state handles without moving went around the machine, and the table stopped saying what the state accepts. A Raft elector sees mostly such events: granting and counting votes, a follower's heartbeat. With Stay they are rows again: NoTransitionError means a refusal, Check and Can answer for them, and DOT draws them as dashed loops. An internal transition is not a way out (Terminals keeps a state that has only these) and enters and leaves nothing, so it triggers no Via or With hook, and one that only it could trigger is rejected. Fire looks for one only inside its hook block: a hookless machine pays nothing, and a hooked one about a nanosecond. The fuzzer generates them too; its acceptance at 200 bytes goes from ~87% to ~79%, since they add no reachability. Duplicate-transition errors now say what each declaration does, and the test pins their text, which it only checked a fragment of before. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 23 +++-- README.md | 11 +++ docs/DESIGN.md | 27 +++++- fsm.go | 18 +++- fsm_test.go | 242 ++++++++++++++++++++++++++++++++++++++++++++++--- introspect.go | 8 +- rules.go | 140 ++++++++++++++++++++-------- 7 files changed, 407 insertions(+), 62 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e484080..054d85e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -248,7 +248,15 @@ before and after touching the fire path. **Self-transitions are UML's external kind.** `From(a).On(ev).To(a)` runs exit then entry, so a counter kept by those hooks dips and returns. Pinned by `TestSelfTransitionRunsExitAndEntry`; do not turn it into an internal -transition that skips the hooks. +transition that skips the hooks. The internal kind is its own declaration, +`From(a).On(ev).Stay()`: guards and actions run, then `Fire` returns before +any hook, `OnTransition` included (`TestStayRunsGuardsAndActionsButNoHook`). +`Machine.internal` holds those rows, and `Fire` and `apply` look it up only +inside the hook block, behind a length check, so a hookless machine pays +nothing. An internal row is registered with `to` set to its source, which +keeps `To`, `Check` and the fuzzer's Fire-agrees-with-To invariant uniform, +but it is not a way out (`Terminals` skips it) and it enters and leaves +nothing (`decl.internal` keeps it out of every Via and With hook). **Declaration order is the output order.** `Machine.states` and `Machine.edges` are slices kept alongside the maps purely so `States`, `Edges`, `Terminals` and @@ -284,11 +292,12 @@ thing there is to test against, and that is the right target anyway. state closes over the shared `st`. - **`FuzzMachineInvariants` covers what nobody wrote down.** It builds a machine from fuzzed `(source, event, target)` rows — sources are the four - states plus two groups overlapping on `running`, and two of the three events - share the name `a` — then asserts what must hold for any definition `New` - accepted: `DOT` is deterministic, every edge endpoint is in `States()`, an + states plus two groups overlapping on `running`, targets the four states or + `Stay`, and two of the three events share the name `a` — then asserts what + must hold for any definition `New` accepted: `DOT` is deterministic, every + edge endpoint is in `States()`, an internal edge loops on its source, an inherited edge names a group that `Has` its source, `Events()` repeats no - name, `Terminals()` is exactly the states with no outgoing edge, + name, `Terminals()` is exactly the states with no outgoing external edge, `Unreachable` is closed under the edge relation, a declared `Initial` leaves nothing unreachable and is not terminal, and `Fire` agrees with `To` on every pair — failing without moving the state. @@ -298,7 +307,9 @@ thing there is to test against, and that is the right target anyway. systematically, and each one makes `New` return an error, which skips the whole invariant block: unfiltered, acceptance at the ~200-byte inputs the engine actually generates is **0%**, so the fuzzer only ever exercised - `New`'s error paths. Filtered it is ~87%. Measure acceptance before changing + `New`'s error paths. Filtered it is ~79% (it was ~87% before internal + targets, which add no reachability, so more definitions leave a state + unreachable from their start). Measure acceptance before changing the generator, and keep the three named tests that cover those errors. - **A fuzz failure is a reproducer worth keeping.** Go writes it to `testdata/fuzz` and prints only the path, so CI uploads that directory as an diff --git a/README.md b/README.md index e4f4cce..f113dcd 100644 --- a/README.md +++ b/README.md @@ -131,6 +131,7 @@ Each of these returns a `Rule[S]`, the single option type. Rules are plain value | `From(a).On(ev).To(b)` | one transition | | `FromEach(a, b).On(ev).To(c)` | the same transition from several sources | | `FromGroup(g).On(ev).To(c)` | a transition inherited by every member of a [group](#groups) | +| `From(a).On(ev).Stay()` | an [internal transition](#internal-transitions): the event is handled in `a`, which it does not leave | | `OnEnter(s, h)` / `OnExit(s, h)` | [hooks](#hooks) around the assignment | | `OnEnterVia(s, ev, h)` / `OnExitVia(s, ev, h)` | hooks that see the payload of one event | | `OnEnterWith(s, h)` / `OnExitWith(s, h)` | hooks that see the payload of every event entering or leaving `s` | @@ -242,6 +243,16 @@ The order inside `Fire` is fixed: `OnTransition` precedes the entry hooks so an entry hook that fires again is logged after its cause; the price is that a transition hook must observe and not fire. A self-transition `From(a).On(ev).To(a)` is UML's *external* kind — it runs exit then entry. +### Internal transitions + +An event a state handles without leaving it — a heartbeat that refreshes a deadline, a vote that is counted — is declared with `Stay`, UML's *internal* transition: + +```go +fsm.From(follower).On(evHeartbeat).Stay().Action(recordLeader) +``` + +Guards and actions run as on any transition, and `Fire` returns a transition from the state to itself; the state is not left, so no hook runs, `OnTransition` included. Keeping such events in the table is what lets it say what each state accepts: a `NoTransitionError` still means the state refuses the event, `Check` and `Can` answer for it, and `DOT` draws it as a dashed loop. It is not a way out, so a state with only internal transitions is still in `Terminals()`. + ### Typed hooks A plain `Hook` sees the transition but not the payload, because a state can be entered by events carrying different types. `OnEnterVia` and `OnExitVia` name the event, which fixes the type: diff --git a/docs/DESIGN.md b/docs/DESIGN.md index cf195a7..acf07f7 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -325,8 +325,31 @@ if tr.Is(evPcpKick) { … } `From(a).On(ev).To(a)` is allowed, and is UML's *external* self-transition: it runs the exit hooks, assigns, then runs the entry hooks. A counter kept by those -hooks therefore dips to zero and comes back. There is no internal transition -that skips the hooks; an `Action` covers that case. +hooks therefore dips to zero and comes back. + +### Internal transitions run no hook + +`From(a).On(ev).Stay()` is UML's *internal* transition: guards and actions +run, the state is not left, and no hook runs — `OnTransition` included, since +nothing changed. It is a separate declaration, not a reading of `To(a)`, so an +external self-transition keeps its meaning. + +This package once held that an `Action` covered the case. It does not: an +action runs only as part of a transition, and the one transition that keeps +the state, `To(a)`, runs the hooks. So an event a state handles without +leaving it went around the machine, as code beside it, and the table stopped +saying what the state accepts. A Raft elector shows how much that is: granting +a vote, counting one, a follower's heartbeat and a leader's acks are the +events it sees most, and none of them moves it. With `Stay` they are rows +again: `NoTransitionError` means the state refuses the event rather than +"handled elsewhere", `Check` and `Can` answer for them, and `DOT` draws them. + +An internal transition enters and leaves nothing, so it is not a way out +(`Terminals` keeps a state that has only these), and it triggers no Via or +With hook — one that only an internal transition could trigger is rejected +as never running. `Fire` checks for one only inside its hook block, so a +machine without hooks pays nothing for the feature, and one without internal +transitions pays a length check. ## Fire does not allocate diff --git a/fsm.go b/fsm.go index 9787427..2390a00 100644 --- a/fsm.go +++ b/fsm.go @@ -150,6 +150,10 @@ type Machine[S comparable] struct { onEnterVia map[edge[S]][]any onExitVia map[edge[S]][]any + // The rows declared with [OnStep.Stay], which run no hook. Empty in a + // machine that declares none, which then pays one length check per fire. + internal map[edge[S]]bool + // Hooks that run after every transition, declared with [OnTransition]. onAll []Hook[S] @@ -178,7 +182,9 @@ func (m *Machine[S]) Name() string { return m.name } // action, run the exit hooks of the old state, assign the new state, run the // [OnTransition] hooks, then the entry hooks of the new state. If the lookup // fails, a guard rejects, or the action returns an error, Fire assigns -// nothing, runs no hook, and returns the zero Transition. +// nothing, runs no hook, and returns the zero Transition. An internal +// transition, declared with [OnStep.Stay], stops after the action: the state +// is not left, so no hook runs. // // The payload often aliases the state. A guard or action that writes *st, or // fires this machine on it, is reported as a [StateChangedError], again with @@ -236,6 +242,11 @@ func (m *Machine[S]) Fire[A any](ctx context.Context, st *S, ev Event[A], arg A) *st = to return t, nil } + // An internal transition leaves nothing, so it runs no hook; *st is + // already to. + if len(m.internal) > 0 && m.internal[e] { + return t, nil + } for _, h := range m.onExit[from] { h(ctx, t) @@ -341,6 +352,11 @@ func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Eve *st = to return t, nil } + // An internal transition leaves nothing, so it runs no hook; *st is + // already to. + if len(m.internal) > 0 && m.internal[e] { + return t, nil + } for _, h := range m.onExit[from] { h(ctx, t) diff --git a/fsm_test.go b/fsm_test.go index c9f8193..45d65d5 100644 --- a/fsm_test.go +++ b/fsm_test.go @@ -735,18 +735,27 @@ func TestEntryAndExitHooksStayPaired(t *testing.T) { } } +// A source takes an event one way: a second declaration, external or +// internal, is reported with what each would do. func TestBuildRejectsDuplicateTransition(t *testing.T) { t.Parallel() - _, err := fsm.New("job", - fsm.From(idle).On(evStart).To(running), - fsm.From(idle).On(evStart).To(cancelled), - ) - if err == nil { - t.Fatal("expected a duplicate-transition error") - } - if !strings.Contains(err.Error(), "duplicate transition") { - t.Errorf("error %q does not mention the duplicate", err) + for _, tc := range []struct { + name string + first, second fsm.Rule[state] + want string + }{ + {"two targets", fsm.From(idle).On(evStart).To(running), fsm.From(idle).On(evStart).To(cancelled), + "fsm job: duplicate transition from idle on start: already goes to running, redeclared to cancelled"}, + {"a target, then stay", fsm.From(idle).On(evStart).To(running), fsm.From(idle).On(evStart).Stay(), + "fsm job: duplicate transition from idle on start: already goes to running, redeclared to stay"}, + {"stay, then a target", fsm.From(idle).On(evStart).Stay(), fsm.From(idle).On(evStart).To(running), + "fsm job: duplicate transition from idle on start: already stays, redeclared to running"}, + } { + _, err := fsm.New("job", tc.first, tc.second, fsm.From(running).On(evFinish).To(done)) + if err == nil || err.Error() != tc.want { + t.Errorf("%s: got %v, want %q", tc.name, err, tc.want) + } } } @@ -1435,6 +1444,196 @@ func TestFireRejectsANilStatePointer(t *testing.T) { } } +// --- Internal transitions ------------------------------------------------- + +// Stay handles the event without leaving the state: guards and actions run +// as on any transition, no hook does, and Fire reports a transition from the +// state to itself. +func TestStayRunsGuardsAndActionsButNoHook(t *testing.T) { + t.Parallel() + + var log []string + note := func(what string) fsm.Hook[state] { + return func(context.Context, fsm.Transition[state]) { log = append(log, what) } + } + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).Stay(). + Guard("positive", func(_ context.Context, n int) error { + if n <= 0 { + return errNever + } + return nil + }). + Action(func(_ context.Context, n int) error { log = append(log, fmt.Sprint("action ", n)); return nil }), + fsm.OnExit(running, note("exit")), + fsm.OnEnter(running, note("enter")), + fsm.OnTransition(note("transition")), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), running + + tr, err := m.Fire(ctx, &st, evFinish, 2) + if err != nil || tr.From != running || tr.To != running || !tr.Is(evFinish) || st != running { + t.Fatalf("Fire: %v -> %v, err %v, state %v; want running -> running", tr.From, tr.To, err, st) + } + if _, fired, err := m.TryFire(ctx, &st, evFinish, 3); !fired || err != nil { + t.Errorf("TryFire: fired=%v, err %v", fired, err) + } + if !slices.Equal(log, []string{"action 2", "action 3"}) { + t.Errorf("ran %v, want only the actions", log) + } + // It is refused as any transition is. + if _, err := m.Fire(ctx, &st, evFinish, 0); !errors.Is(err, errNever) { + t.Errorf("a rejecting guard: %v, want a GuardError", err) + } + if !m.Can(ctx, running, evFinish, 1) || m.Can(ctx, idle, evFinish, 1) { + t.Error("Can does not see the internal transition, or sees it from idle") + } + if to, ok := m.To(running, evFinish); !ok || to != running { + t.Errorf("To = %v, %v; want running, true", to, ok) + } +} + +// An internal transition is not a way out: a state with only those is +// terminal, and a start with only those is dead. +func TestStayIsNotAWayOut(t *testing.T) { + t.Parallel() + + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evCancel).Stay(), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + if got := m.Terminals(); !slices.Equal(got, []state{running}) { + t.Errorf("Terminals() = %v, want [running]", got) + } + _, err = fsm.New("job", fsm.Initial(idle), fsm.From(idle).On(evStart).Stay()) + if want := "fsm job: initial state idle has no outgoing transition"; err == nil || err.Error() != want { + t.Errorf("got %v, want %q", err, want) + } +} + +// An internal transition enters and leaves nothing, so a typed hook that +// only it could trigger would never run, and New says so. +func TestStayTriggersNoTypedHook(t *testing.T) { + t.Parallel() + + stay := fsm.From(running).On(evFinish).Stay() + in := fsm.From(idle).On(evStart).To(running) + for _, tc := range []struct { + name string + hook fsm.Rule[state] + want string + }{ + {"OnEnterVia", fsm.OnEnterVia(running, evFinish, func(context.Context, fsm.Transition[state], int) {}), + "fsm job: OnEnterVia for state running: no transition on finish enters it, so the hook would never run"}, + {"OnExitVia", fsm.OnExitVia(running, evFinish, func(context.Context, fsm.Transition[state], int) {}), + "fsm job: OnExitVia for state running: no transition on finish leaves it, so the hook would never run"}, + {"OnExitWith", fsm.OnExitWith(running, func(context.Context, fsm.Transition[state], int) {}), + "fsm job: OnExitWith for state running: no transition leaves it, so the hook would never run"}, + } { + _, err := fsm.New("job", in, stay, tc.hook) + if err == nil || err.Error() != tc.want { + t.Errorf("%s: got %v, want %q", tc.name, err, tc.want) + } + } +} + +// A group can declare an internal transition, each member staying in +// itself, and a member overrides it, or a group's external one, as usual. +func TestStayInAGroup(t *testing.T) { + t.Parallel() + + live := fsm.NewGroup("live", idle, running) + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.FromGroup(live).On(evCancel).Stay(), + fsm.From(running).On(evCancel).To(cancelled), // overrides the group's + fsm.FromGroup(live).On(evFinish).To(done), + fsm.From(idle).On(evFinish).Stay(), // overrides the group's + ) + if err != nil { + t.Fatalf("build: %v", err) + } + for _, tc := range []struct { + from, want state + ev fsm.Event[fsm.Unit] + }{ + {idle, idle, evCancel}, + {running, cancelled, evCancel}, + } { + st := tc.from + if _, err := m.Send(t.Context(), &st, tc.ev); err != nil || st != tc.want { + t.Errorf("%v on %s: %v, err %v; want %v", tc.from, tc.ev, st, err, tc.want) + } + } + var got []string + for _, e := range m.Edges() { + got = append(got, fmt.Sprintf("%v-%s->%v internal=%v group=%q", e.From, e.Event(), e.To, e.Internal, e.Group)) + } + want := []string{ + "idle-start->running internal=false group=\"\"", + "running-cancel->cancelled internal=false group=\"\"", + "idle-finish->idle internal=true group=\"\"", + "idle-cancel->idle internal=true group=\"live\"", + "running-finish->done internal=false group=\"live\"", + } + if !slices.Equal(got, want) { + t.Errorf("Edges()\n got %q\nwant %q", got, want) + } +} + +// Error messages name an internal transition as one. +func TestStayIsNamedInErrors(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + rule fsm.Rule[state] + want string + }{ + {fsm.From(idle).On(evStart).Stay().Guard("g", nil), + `fsm job: internal transition idle: guard "g" is nil`}, + {fsm.FromEach(idle, running).On(evStart).Stay().Guard("g", nil), + `fsm job: internal transition [idle running]: guard "g" is nil`}, + {fsm.FromGroup(fsm.NewGroup("live", idle, running)).On(evStart).Stay().Guard("g", nil), + `fsm job: internal transition group live: guard "g" is nil`}, + } { + _, err := fsm.New("job", tc.rule, fsm.From(idle).On(evCancel).To(cancelled), fsm.From(running).On(evFinish).To(done)) + if err == nil || err.Error() != tc.want { + t.Errorf("got %v, want %q", err, tc.want) + } + } +} + +// DOT draws an internal transition as a dashed loop on its state. +func TestStayIsDrawnDashed(t *testing.T) { + t.Parallel() + + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).Stay().Guard("positive", func(context.Context, int) error { return nil }), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + want := `digraph "job" { + rankdir=LR; + "idle" [shape=box]; + "running" [shape=doublecircle]; + "idle" -> "running" [label="start"]; + "running" -> "running" [label="finish\n[positive]", style=dashed]; +} +` + if got := m.DOT(); got != want { + t.Errorf("DOT\n%s\nwant\n%s", got, want) + } +} + // --- TryFire ---------------------------------------------------------------- // TryFire makes the transition Fire would, hooks and all, and says it did. @@ -1762,6 +1961,7 @@ func TestFireDoesNotAllocate(t *testing.T) { m, err := fsm.New("job", fsm.From(idle).On(evStart).To(running).Guard("always", func(context.Context, fsm.Unit) error { return nil }), fsm.From(running).On(evFinish).To(idle).Action(func(context.Context, int) error { return nil }), + fsm.From(running).On(evCancel).Stay().Action(func(context.Context, fsm.Unit) error { return nil }), fsm.OnEnter(running, func(context.Context, fsm.Transition[state]) {}), fsm.OnExit(running, func(context.Context, fsm.Transition[state]) {}), // Payload hooks run in the same path and must not box the payload. @@ -1776,6 +1976,7 @@ func TestFireDoesNotAllocate(t *testing.T) { avg := testing.AllocsPerRun(1000, func() { _, _ = m.Send(ctx, &st, evStart) + _, _ = m.Send(ctx, &st, evCancel) // internal: running stays _, _ = m.Fire(ctx, &st, evFinish, 7) }) if avg != 0 { @@ -2668,7 +2869,8 @@ func FuzzMachineInvariants(f *testing.F) { seen := map[[2]int]bool{} claimedBy := map[int]int{} // event -> the group source holding it for i := 0; i+2 < len(rows); i += 3 { - r := row{int(rows[i]) % (len(states) + len(groups)), int(rows[i+1]) % len(events), int(rows[i+2]) % len(states)} + // A target past the states is Stay: an internal transition. + r := row{int(rows[i]) % (len(states) + len(groups)), int(rows[i+1]) % len(events), int(rows[i+2]) % (len(states) + 1)} if seen[[2]int{r.src, r.ev}] { continue } @@ -2688,11 +2890,17 @@ func FuzzMachineInvariants(f *testing.F) { }) }) for _, r := range picked { - ev, to := events[r.ev], states[r.to] + ev := events[r.ev] + var from fsm.FromStep[state] if r.src < len(states) { - rules = append(rules, fsm.From(states[r.src]).On(ev).To(to)) + from = fsm.From(states[r.src]) + } else { + from = fsm.FromGroup(groups[r.src-len(states)]) + } + if r.to == len(states) { + rules = append(rules, from.On(ev).Stay()) } else { - rules = append(rules, fsm.FromGroup(groups[r.src-len(states)]).On(ev).To(to)) + rules = append(rules, from.On(ev).To(states[r.to])) } } @@ -2718,7 +2926,13 @@ func FuzzMachineInvariants(f *testing.F) { t.Fatalf("edge %v --%s--> %v claims group %q, which does not hold %v", e.From, e.Event(), e.To, e.Group, e.From) } } - outgoing[e.From] = true + // An internal transition is a loop, and not a way out. + if e.Internal && e.To != e.From { + t.Fatalf("internal edge %v --%s--> %v leaves its state", e.From, e.Event(), e.To) + } + if !e.Internal { + outgoing[e.From] = true + } } // Events deduplicates by name; States and Edges do not repeat a row. diff --git a/introspect.go b/introspect.go index 5373fb8..5bd8004 100644 --- a/introspect.go +++ b/introspect.go @@ -35,14 +35,15 @@ func (m *Machine[S]) Events() []string { // Initial returns the state declared with [Initial], and false when none was. func (m *Machine[S]) Initial() (S, bool) { return m.initial, m.hasInitial } -// Terminals returns the states with no outgoing transition. +// Terminals returns the states with no outgoing transition. An internal +// transition, declared with [OnStep.Stay], is not a way out. // // A machine's terminal set is worth asserting in a test: an unintended // terminal state is a state something can get stuck in. func (m *Machine[S]) Terminals() []S { out := []S{} for _, s := range m.states { - if !slices.ContainsFunc(m.edges, func(e Edge[S]) bool { return e.From == s }) { + if !slices.ContainsFunc(m.edges, func(e Edge[S]) bool { return e.From == s && !e.Internal }) { out = append(out, s) } } @@ -168,6 +169,9 @@ func (m *Machine[S]) DOT() string { } var attrs string + if e.Internal { + attrs = ", style=dashed" // handled in the state, which it does not leave + } if e.Group != "" && collapsible(e, byName[e.Group], rows[of(e)], whole[e.Group]) { k := of(e) if drawn[k] { diff --git a/rules.go b/rules.go index 2a905d3..4cd9570 100644 --- a/rules.go +++ b/rules.go @@ -71,7 +71,7 @@ type builder[S comparable] struct { // Targets of the group transitions declared so far, so that one declared // twice reads as a duplicate rather than as a group clashing with itself. - groupEdges map[groupEdge]S + groupEdges map[groupEdge]string // what the first declaration does } // groupEdge identifies one group transition, before it expands to a row per @@ -82,9 +82,10 @@ type groupEdge struct { } type decl[S comparable] struct { - key edge[S] - to S - payload any // (*A)(nil) for the event's payload type A + key edge[S] + to S + internal bool // an internal transition enters and leaves nothing + payload any // (*A)(nil) for the event's payload type A } // payloadToken identifies a payload type without reflect: two typed nil @@ -106,10 +107,11 @@ func New[S comparable](name string, rules ...Rule[S]) (*Machine[S], error) { onExit: make(map[S][]Hook[S]), onEnterVia: make(map[edge[S]][]any), onExitVia: make(map[edge[S]][]any), + internal: make(map[edge[S]]bool), }, seen: make(map[S]bool), inherited: make(map[edge[S]]string), - groupEdges: make(map[groupEdge]S), + groupEdges: make(map[groupEdge]string), } b.apply("rule", rules) @@ -224,26 +226,45 @@ type OnStep[S comparable, A any] struct { // A self-transition — To naming the state On came from — is allowed and runs // the full sequence: the exit hooks of the state, the assignment, then its // entry hooks. That is UML's external self-transition, and it means a counter -// kept by those hooks decrements and increments back to where it was. There is no -// internal transition that skips the hooks; use an [ToStep.Action] for that. +// kept by those hooks decrements and increments back to where it was. For an +// event handled without leaving the state, see [OnStep.Stay]. func (o OnStep[S, A]) To(to S) *ToStep[S, A] { return &ToStep[S, A]{froms: o.froms, group: o.group, ev: o.ev, to: to} } +// Stay completes the transition as an internal one: the state takes the +// event and stays where it is, UML's internal transition. +// +// fsm.From(follower).On(evHeartbeat).Stay().Action(recordLeader) +// +// Its guards and actions run as on any transition, and [Machine.Fire] +// returns a Transition from the state to itself; but the state is not left, +// so no hook runs, [OnTransition] included. That is what an external +// self-transition, To naming its own source, cannot do. From [FromEach] or +// [FromGroup], each source stays in itself. +// +// An internal transition is still a transition: it is what the state does +// with the event, rather than refuse it. It is not a way out, so it does not +// keep the state out of [Machine.Terminals]. +func (o OnStep[S, A]) Stay() *ToStep[S, A] { + return &ToStep[S, A]{froms: o.froms, group: o.group, ev: o.ev, internal: true} +} + // ToStep is a complete transition, optionally carrying guards and actions. // It satisfies [Rule]. // // Its methods mutate and return the same value, so a guard attached to a // stored ToStep takes effect whether or not the result is reassigned. type ToStep[S comparable, A any] struct { - froms []S - group *Group[S] - to S - ev Event[A] - guards []func(context.Context, A) error - descs []string - actions []func(context.Context, A) error - errs []error + froms []S + group *Group[S] + to S + internal bool // declared with Stay: each source keeps its state + ev Event[A] + guards []func(context.Context, A) error + descs []string + actions []func(context.Context, A) error + errs []error } // Guard rejects the transition when f returns a non-nil error. The error is @@ -292,10 +313,11 @@ func (t *ToStep[S, A]) applyTo(b *builder[S]) { } r := row[S]{ - to: t.to, - ev: t.ev.def, - payload: payloadToken[A](), - desc: joinDescs(t.descs), + to: t.to, + internal: t.internal, + ev: t.ev.def, + payload: payloadToken[A](), + desc: joinDescs(t.descs), } r.guard, r.action = t.combine() @@ -309,9 +331,9 @@ func (t *ToStep[S, A]) applyTo(b *builder[S]) { } for _, s := range t.froms { - if prev, dup := b.m.table[edge[S]{from: s, ev: r.ev}]; dup { - b.errs = append(b.errs, fmt.Errorf("duplicate transition from %v on %s: already goes to %v, redeclared to %v", - s, r.ev.name, prev, t.to)) + if _, dup := b.m.table[edge[S]{from: s, ev: r.ev}]; dup { + b.errs = append(b.errs, fmt.Errorf("duplicate transition from %v on %s: already %s, redeclared to %s", + s, r.ev.name, b.does(edge[S]{from: s, ev: r.ev}), t.redeclared())) continue } r.from = s @@ -331,15 +353,17 @@ func (t *ToStep[S, A]) expandGroup(b *builder[S], r row[S], where string) { gk := groupEdge{group: g.name, ev: r.ev} if prev, dup := b.groupEdges[gk]; dup { b.errs = append(b.errs, fmt.Errorf( - "duplicate transition from group %s on %s: already goes to %v, redeclared to %v", - g.name, r.ev.name, prev, t.to)) + "duplicate transition from group %s on %s: already %s, redeclared to %s", + g.name, r.ev.name, prev, t.redeclared())) return } - b.groupEdges[gk] = t.to + b.groupEdges[gk] = t.does() // The target is a state of the machine whether or not any member ends up // inheriting the edge, and declaring it now lets member validation see it. - b.declare(t.to) + if !t.internal { + b.declare(t.to) + } b.expand = append(b.expand, func(b *builder[S]) { var inherited, clashes int @@ -401,16 +425,46 @@ func (t *ToStep[S, A]) combine() (guard, action any) { } // describe names the rule in error messages: one source prints as a -> b, -// several as [a b] -> c. +// several as [a b] -> c, and an internal transition as internal transition a. func (t *ToStep[S, A]) describe() string { + var from string switch { case t.group != nil: - return fmt.Sprintf("transition group %s -> %v", t.group.name, t.to) + from = "group " + t.group.name case len(t.froms) == 1: - return fmt.Sprintf("transition %v -> %v", t.froms[0], t.to) + from = fmt.Sprint(t.froms[0]) default: - return fmt.Sprintf("transition %v -> %v", t.froms, t.to) + from = fmt.Sprint(t.froms) + } + if t.internal { + return "internal transition " + from + } + return fmt.Sprintf("transition %s -> %v", from, t.to) +} + +// does and redeclared say what the transition does, for a duplicate's error +// message: "already goes to b, redeclared to c", "already stays, redeclared +// to c", "already goes to b, redeclared to stay". +func (t *ToStep[S, A]) does() string { + if t.internal { + return "stays" } + return fmt.Sprintf("goes to %v", t.to) +} + +func (t *ToStep[S, A]) redeclared() string { + if t.internal { + return "stay" + } + return fmt.Sprint(t.to) +} + +// does says what the registered row e does, as ToStep.does. +func (b *builder[S]) does(e edge[S]) string { + if b.m.internal[e] { + return "stays" + } + return fmt.Sprintf("goes to %v", b.m.table[e]) } // row is one line of the transition table. Its fields are named because a @@ -418,6 +472,7 @@ func (t *ToStep[S, A]) describe() string { // and easy to transpose. type row[S comparable] struct { from, to S + internal bool // to is from, and no hook runs ev *eventDef guard any // func(context.Context, A) (string, error) action any // func(context.Context, A) error @@ -428,9 +483,15 @@ type row[S comparable] struct { // register adds one row to the transition table. func (b *builder[S]) register(r row[S]) { + if r.internal { + r.to = r.from + } e := edge[S]{from: r.from, ev: r.ev} b.m.table[e] = r.to - b.decls = append(b.decls, decl[S]{key: e, to: r.to, payload: r.payload}) + if r.internal { + b.m.internal[e] = true + } + b.decls = append(b.decls, decl[S]{key: e, to: r.to, internal: r.internal, payload: r.payload}) b.declare(r.from) b.declare(r.to) @@ -441,7 +502,7 @@ func (b *builder[S]) register(r row[S]) { b.m.actions[e] = r.action } b.m.edges = append(b.m.edges, Edge[S]{ - From: r.from, To: r.to, Guard: r.desc, Group: r.group, trigger: r.ev, + From: r.from, To: r.to, Guard: r.desc, Group: r.group, Internal: r.internal, trigger: r.ev, }) } @@ -470,7 +531,8 @@ func OnExit[S comparable](s S, h Hook[S]) Rule[S] { }) } -// OnTransition declares a hook that runs after every transition, just after +// OnTransition declares a hook that runs after every transition that leaves +// a state — every one but an internal transition ([OnStep.Stay]) — just after // the assignment and before the entry hooks of the new state, so an entry // hook that fires the machine again is logged after the transition that // caused it. It is where an audit log or a trace that must see every change @@ -545,7 +607,7 @@ func OnEnterVia[S comparable, A any](s S, ev Event[A], h func(context.Context, T } k := edge[S]{from: s, ev: ev.def} b.m.onEnterVia[k] = append(b.m.onEnterVia[k], h) - b.requireVia("OnEnterVia", "enters", s, ev.def, func(d decl[S]) bool { return d.to == s }) + b.requireVia("OnEnterVia", "enters", s, ev.def, func(d decl[S]) bool { return d.to == s && !d.internal }) }) } @@ -560,7 +622,7 @@ func OnExitVia[S comparable, A any](s S, ev Event[A], h func(context.Context, Tr } k := edge[S]{from: s, ev: ev.def} b.m.onExitVia[k] = append(b.m.onExitVia[k], h) - b.requireVia("OnExitVia", "leaves", s, ev.def, func(d decl[S]) bool { return d.key.from == s }) + b.requireVia("OnExitVia", "leaves", s, ev.def, func(d decl[S]) bool { return d.key.from == s && !d.internal }) }) } @@ -618,7 +680,7 @@ func (b *builder[S]) requireVia(what, verb string, s S, def *eventDef, match fun // It runs after the plain and the [OnEnterVia] entry hooks of the same state. func OnEnterWith[S comparable, A any](s S, h func(context.Context, Transition[S], A)) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.attachWith("OnEnterWith", "enters", s, h, b.m.onEnterVia, func(d decl[S]) bool { return d.to == s }) + b.attachWith("OnEnterWith", "enters", s, h, b.m.onEnterVia, func(d decl[S]) bool { return d.to == s && !d.internal }) }) } @@ -628,7 +690,7 @@ func OnEnterWith[S comparable, A any](s S, h func(context.Context, Transition[S] // the same state, and still before the state changes. func OnExitWith[S comparable, A any](s S, h func(context.Context, Transition[S], A)) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.attachWith("OnExitWith", "leaves", s, h, b.m.onExitVia, func(d decl[S]) bool { return d.key.from == s }) + b.attachWith("OnExitWith", "leaves", s, h, b.m.onExitVia, func(d decl[S]) bool { return d.key.from == s && !d.internal }) }) } @@ -761,6 +823,10 @@ type Edge[S comparable] struct { // rows. Group string + // Internal is set on an internal transition, declared with + // [OnStep.Stay]: To is From, and no hook runs. + Internal bool + trigger *eventDef } From 4a3447be0d46036ea6e0f11dcb0dfbe7086038b2 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:23:23 +0500 Subject: [PATCH 3/8] Add group entry and exit hooks, run across the boundary only OnEnterGroup and OnExitGroup, and their With variants that see the payload, run as a superstate's hooks do: only on a transition that crosses the group's boundary, never on a move between members, an external self-transition of a member, or an internal transition. So a count of what is in a group, kept by them, never dips. This reverses "a group has no entry or exit hooks", which held that a flat table had nowhere to record that suppression. Every row knows its source and target, so which groups it leaves and enters is a fact about the row: New works it out last, after expansion, and Fire runs the list. The condition the design set, a superstate that genuinely needs its own hook, is met by a Raft leader, whose step-down must run when leading or handing over is left for a follower, and not between the two. The order is a hierarchy's: the state's exit hooks, then its groups', smaller first; the assignment and transition hooks; the groups' entry hooks, larger first, then the state's. A group entry hook must not fire. New rejects a group hook no transition runs, and a With hook a crossing event would miss. Fire checks precomputed flags for internal transitions and group hooks, not map lengths, which cost a nanosecond more: a machine with hooks is about 1.4 ns a round trip slower than before both, one without unchanged. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 27 +++-- README.md | 14 ++- docs/DESIGN.md | 54 ++++++---- fsm.go | 69 ++++++++++--- fsm_test.go | 262 +++++++++++++++++++++++++++++++++++++++++++++++++ rules.go | 161 +++++++++++++++++++++++++++++- 6 files changed, 544 insertions(+), 43 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 054d85e..a93636b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -155,12 +155,25 @@ which group owns each inherited row so the two cases can be told apart. With no nesting there is no specificity rule to fall back on. **Groups are not states and `Fire` knows nothing about them.** They expand to -ordinary rows before `New` returns. Do not add group entry/exit hooks: the -only reason to want them is superstate hook suppression, which a flat table -cannot express, and a group counter kept by them would dip on an intra-group -move. `docs/DESIGN.md` has the argument and the sum-the-members -answer. `TestGroupFireDoesNotAllocate` pins that the expansion produces rows -indistinguishable from hand-written ones. +ordinary rows before `New` returns. `TestGroupFireDoesNotAllocate` pins that +the expansion produces rows indistinguishable from hand-written ones. + +**Group hooks are worked out per row, after everything else.** A flat table +can suppress a superstate's hooks on an intra-group move after all: each row +knows its source and target, so `attachGroupHooks` — the last step of `New`, +after expansion and the deferred pass, so inherited rows count — gives every +row the hooks of the groups it leaves (source in, target out) and enters, in +the order they run: leaving, smaller groups first; entering, larger first; +each group's in declaration order. `Fire` then runs `groupExit[e]` after the +state's exit hooks and `groupEnter[e]` before its entry hooks, behind +`hasGroupHooks`, a precomputed bool: a map-length check there cost a +nanosecond more. An internal row runs none, and an external self-transition +of a member crosses nothing. Pinned by `TestGroupHooksRunOnlyAcrossTheBoundary`, +`TestGroupHooksRunBetweenStateHooks`, `TestGroupHooksRunOutsideInAndInsideOut` +and `TestGroupHooksKeepAnExactCount`. A group entry hook must not fire on the +same state, for the reason a transition hook must not: the state entered has +not run its own entry hooks. Typed group hooks check the payload of every +crossing row, one error per hook and event, as `OnEnterWith` does. **`ToStep.combine` builds the guard/action closures once and every expanded row shares them.** This is what keeps the one-directional type erasure intact @@ -252,7 +265,7 @@ transition that skips the hooks. The internal kind is its own declaration, `From(a).On(ev).Stay()`: guards and actions run, then `Fire` returns before any hook, `OnTransition` included (`TestStayRunsGuardsAndActionsButNoHook`). `Machine.internal` holds those rows, and `Fire` and `apply` look it up only -inside the hook block, behind a length check, so a hookless machine pays +inside the hook block, behind `hasInternal`, so a hookless machine pays nothing. An internal row is registered with `to` set to its source, which keeps `To`, `Check` and the fuzzer's Fire-agrees-with-To invariant uniform, but it is not a way out (`Terminals` skips it) and it enters and leaves diff --git a/README.md b/README.md index f113dcd..38520da 100644 --- a/README.md +++ b/README.md @@ -136,6 +136,7 @@ Each of these returns a `Rule[S]`, the single option type. Rules are plain value | `OnEnterVia(s, ev, h)` / `OnExitVia(s, ev, h)` | hooks that see the payload of one event | | `OnEnterWith(s, h)` / `OnExitWith(s, h)` | hooks that see the payload of every event entering or leaving `s` | | `OnTransition(h)` | a hook on every transition | +| `OnEnterGroup(g, h)` / `OnExitGroup(g, h)` | [hooks](#group-hooks) on the transitions that cross `g`'s boundary; `…With` variants see the payload | | `NewGroup(name, a, b)` | a [group](#groups), usable as a rule on its own | | `Rules(...)` | several rules as one value | @@ -352,7 +353,18 @@ for _, e := range participantFSM.Edges() { `New` rejects an unknown member, a repeated one, a name reused for different members, two groups claiming one event for the same state, and a group transition every member overrides. -Groups cover most of what substates are for, but not all: no group hooks or group counters, no nesting, no initial member. Why, in [docs/DESIGN.md](docs/DESIGN.md#groups-are-a-build-time-expansion). +### Group hooks + +A group has entry and exit hooks, which run as a superstate's do: only on a transition that crosses its boundary. + +```go +fsm.OnEnterGroup(pcpLive, func(_ context.Context, tr fsm.Transition[pcpState]) { live.Inc() }) +fsm.OnExitGroup(pcpLive, func(_ context.Context, tr fsm.Transition[pcpState]) { live.Dec() }) +``` + +A move between members — `connected` to `reconnecting` and back — runs neither, so the count never dips. `OnEnterGroupWith` and `OnExitGroupWith` see the payload, which every event crossing the boundary must then carry. A transition leaving several groups leaves the smaller one first, and one entering several enters the larger one first, as a substate is left before its superstate and entered after it. The exit hooks of a group run after the state's own, and its entry hooks before the state's own. + +Groups cover most of what substates are for, but not all: no nesting as states, no initial member. Why, in [docs/DESIGN.md](docs/DESIGN.md#groups-are-a-build-time-expansion). ## Introspection diff --git a/docs/DESIGN.md b/docs/DESIGN.md index acf07f7..3700c34 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -129,25 +129,37 @@ what to skip, group expansion is a deferred pass — and it must run *before* the `OnEnterWith`/`OnExitWith` pass, which reads the finished table. So there are two phases, not one: expansion adds edges, then the rules that read edges run. -### What groups deliberately are not - -A group has no entry or exit hooks. The reason to want them is exactly the -thing a flat expansion cannot reproduce: in a real hierarchy, moving between -two substates of the same superstate does *not* run the superstate's hooks. -With every row flat there is nowhere to record that, so a counter of what is in -the group would decrement and increment on a move a hierarchy would treat as -staying put — the same dip as a self-transition, in a counter whose whole -purpose is not to drift. - -The usual demand for it dissolves on inspection. "How many participants are -live" is the sum of the per-state gauges, computed where the counters are -read; `sum by (state)` in a metrics backend needs no group at all, and has no -dip to observe. Adding exact suppression means precomputing the ordered hook -sequence per edge and restructuring `Machine` around a single -`map[edge[S]]*plan[S]` — worth doing if a superstate ever genuinely needs its -own hook, and not before. - -Groups do not nest, and entering one does not select an initial member. +### Group hooks run where a hierarchy would run them + +A group has entry and exit hooks, `OnEnterGroup` and `OnExitGroup`, and they +run only on a transition that crosses its boundary: in a real hierarchy, +moving between two substates of the same superstate does *not* run the +superstate's hooks, and neither does a move between members here. So a count +of what is in the group, kept by them, never dips. + +This package used to leave them out, on the grounds that a flat table had +nowhere to record that suppression, and that summing the members' gauges +answered "how many are live". The first was wrong: every row knows its source +and target, so which groups it leaves and enters is a fact about the row, +worked out once the table is finished, and `Fire` only runs the list. What +made it worth doing is a superstate whose hook is not a counter. A Raft +leader is one: leading, and handing over to a successor, are two states of +it, and stepping down — failing the calls waiting on the leader, answering +the hand-over — must run when either is left for a follower, and not when +leading becomes handing over. Before, that was a flag beside the machine. + +The order is the hierarchy's. Leaving: the state's exit hooks, then its +groups', the smaller group first; entering: the larger group first, then the +state's own. With overlapping groups there is no hierarchy to appeal to, and +size, then declaration order, decides. A group entry hook runs before the +state entered has run its own, so it must not fire the machine, as a +transition hook must not. The cost is one flag check per direction in a +machine with hooks, and none in one without. + +### What groups still are not + +Groups do not nest as states do, and entering one does not select an initial +member. Overlapping groups are allowed, but two groups claiming the same event for one state is an error rather than a silent most-specific-wins, because with no nesting there is no specificity to appeal to. @@ -371,8 +383,8 @@ boxed. See [Benchmark](../README.md#benchmark). inject, so this has not been a problem — but it rules out mocking the machine itself. Test against the states instead. - **Flat states only.** [`Group`](#groups-are-a-build-time-expansion) covers - shared transitions and per-member overrides, but not the rest of a - hierarchy: no group entry/exit hooks or gauges, no nesting, no initial + shared transitions, per-member overrides and superstate entry/exit hooks, + but not the rest of a hierarchy: no nesting as states, no initial transition into a group, no orthogonal regions. - **No built-in async.** No trigger queue, no run-to-completion mode. `Fire` is synchronous: if your state is already serialized behind a queue or a mutex, diff --git a/fsm.go b/fsm.go index 2390a00..a50854e 100644 --- a/fsm.go +++ b/fsm.go @@ -29,16 +29,15 @@ // [Machine.Fire] neither knows about groups nor pays for them. // // That expansion is most of what a hierarchical state machine's substates are -// used for, and deliberately not all of it. A group has no entry or exit -// hooks, because the reason to want them is the one thing expansion cannot -// reproduce: in a real hierarchy, moving between two substates of the same -// superstate does not run the superstate's hooks, and there is nowhere to -// record that when every row is flat. So there is also no counter of what is -// in a group — it would decrement and increment on a move that a hierarchy -// would treat as staying put. Count the member states individually and sum -// them where the counters are read. +// used for. A group also has entry and exit hooks ([OnEnterGroup], +// [OnExitGroup]), which run as a superstate's do: only on a transition that +// crosses the group's boundary, never on a move between members. Each row +// learns, once the table is finished, which groups it leaves and enters, so +// a count of what is in a group, kept by those hooks, is exact. // -// Groups do not nest, and entering a group does not select an initial member. +// Groups do not nest as states do — the hooks of a group inside another run +// inside the other's, but nothing more — and entering a group does not +// select an initial member. package fsm import ( @@ -113,6 +112,13 @@ func (t Transition[S]) Is[A any](ev Event[A]) bool { return t.trigger != nil && t.trigger == ev.def } +// groupHook is one group hook on one row: plain, or typed in the payload +// of the row's event, func(context.Context, Transition[S], A). +type groupHook[S comparable] struct { + plain Hook[S] + typed any +} + // edge is the key of the transition table. type edge[S comparable] struct { from S @@ -154,6 +160,13 @@ type Machine[S comparable] struct { // machine that declares none, which then pays one length check per fire. internal map[edge[S]]bool + // Group hooks, per row: the ones a row runs because it leaves, or + // enters, a group, in the order they run. Worked out once the table is + // finished, so a move between members of a group, which crosses no + // boundary, has none. + groupExit map[edge[S]][]groupHook[S] + groupEnter map[edge[S]][]groupHook[S] + // Hooks that run after every transition, declared with [OnTransition]. onAll []Hook[S] @@ -161,6 +174,10 @@ type Machine[S comparable] struct { // hook block and its lookups. hasHooks bool + // True when the machine declares an internal transition, or a group + // hook: one load each in the hook block, cheaper than a map's length. + hasInternal, hasGroupHooks bool + // The state declared with [Initial], if any. initial S hasInitial bool @@ -244,7 +261,7 @@ func (m *Machine[S]) Fire[A any](ctx context.Context, st *S, ev Event[A], arg A) } // An internal transition leaves nothing, so it runs no hook; *st is // already to. - if len(m.internal) > 0 && m.internal[e] { + if m.hasInternal && m.internal[e] { return t, nil } @@ -256,6 +273,11 @@ func (m *Machine[S]) Fire[A any](ctx context.Context, st *S, ev Event[A], arg A) raw.(func(context.Context, Transition[S], A))(ctx, t, arg) } } + if m.hasGroupHooks { + for _, g := range m.groupExit[e] { + runGroupHook(ctx, g, t, arg) + } + } *st = to @@ -264,6 +286,11 @@ func (m *Machine[S]) Fire[A any](ctx context.Context, st *S, ev Event[A], arg A) for _, h := range m.onAll { h(ctx, t) } + if m.hasGroupHooks { + for _, g := range m.groupEnter[e] { + runGroupHook(ctx, g, t, arg) + } + } for _, h := range m.onEnter[to] { h(ctx, t) } @@ -354,7 +381,7 @@ func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Eve } // An internal transition leaves nothing, so it runs no hook; *st is // already to. - if len(m.internal) > 0 && m.internal[e] { + if m.hasInternal && m.internal[e] { return t, nil } @@ -366,6 +393,11 @@ func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Eve raw.(func(context.Context, Transition[S], A))(ctx, t, arg) } } + if m.hasGroupHooks { + for _, g := range m.groupExit[e] { + runGroupHook(ctx, g, t, arg) + } + } *st = to @@ -374,6 +406,11 @@ func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Eve for _, h := range m.onAll { h(ctx, t) } + if m.hasGroupHooks { + for _, g := range m.groupEnter[e] { + runGroupHook(ctx, g, t, arg) + } + } for _, h := range m.onEnter[to] { h(ctx, t) } @@ -519,3 +556,13 @@ func (e *StateChangedError[S]) Error() string { return fmt.Sprintf("fsm %s: state changed to %v during %v --%s--> %v", e.Machine, e.Found, e.From, e.Event, e.To) } + +// runGroupHook runs g with the payload, if it takes one. Its A is the +// payload type of the row it was attached to, which New checked. +func runGroupHook[S comparable, A any](ctx context.Context, g groupHook[S], t Transition[S], arg A) { + if g.plain != nil { + g.plain(ctx, t) + return + } + g.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) +} diff --git a/fsm_test.go b/fsm_test.go index 45d65d5..d522897 100644 --- a/fsm_test.go +++ b/fsm_test.go @@ -1634,6 +1634,264 @@ func TestStayIsDrawnDashed(t *testing.T) { } } +// --- Group hooks ------------------------------------------------------------ + +// A group's hooks run on a transition that crosses its boundary, and on no +// other: not on a move between members, not on a member's self-transition, +// not on an internal transition. +func TestGroupHooksRunOnlyAcrossTheBoundary(t *testing.T) { + t.Parallel() + + var log []string + note := func(what string) fsm.Hook[state] { + return func(_ context.Context, tr fsm.Transition[state]) { + log = append(log, fmt.Sprintf("%s %v->%v", what, tr.From, tr.To)) + } + } + busy := fsm.NewGroup("busy", running, done) + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.From(done).On(evStart).To(done), + fsm.From(done).On(evFinish).Stay(), + fsm.From(done).On(evCancel).To(cancelled), + fsm.OnEnterGroup(busy, note("enter busy")), + fsm.OnExitGroup(busy, note("exit busy")), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + steps := []func() error{ + func() error { _, err := m.Send(ctx, &st, evStart); return err }, // idle -> running: enters + func() error { _, err := m.Fire(ctx, &st, evFinish, 1); return err }, // running -> done: inside + func() error { _, err := m.Send(ctx, &st, evStart); return err }, // done -> done: inside + func() error { _, err := m.Fire(ctx, &st, evFinish, 1); return err }, // internal + func() error { _, err := m.Send(ctx, &st, evCancel); return err }, // done -> cancelled: leaves + } + for i, step := range steps { + if err := step(); err != nil { + t.Fatalf("step %d: %v", i, err) + } + } + if want := []string{"enter busy idle->running", "exit busy done->cancelled"}; !slices.Equal(log, want) { + t.Errorf("ran %q, want %q", log, want) + } +} + +// Group hooks sit between the state's own: exit the state, then the group; +// assign; the transition hooks; enter the group, then the state. +func TestGroupHooksRunBetweenStateHooks(t *testing.T) { + t.Parallel() + + var log []string + note := func(what string) fsm.Hook[state] { + return func(context.Context, fsm.Transition[state]) { log = append(log, what) } + } + busy := fsm.NewGroup("busy", running) + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evCancel).To(idle), + fsm.OnEnter(running, note("enter running")), + fsm.OnExit(running, note("exit running")), + fsm.OnEnterGroup(busy, note("enter busy")), + fsm.OnExitGroup(busy, note("exit busy")), + fsm.OnTransition(note("transition")), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + _, _ = m.Send(ctx, &st, evStart) + _, _ = m.Send(ctx, &st, evCancel) + want := []string{ + "transition", "enter busy", "enter running", + "exit running", "exit busy", "transition", + } + if !slices.Equal(log, want) { + t.Errorf("ran %q, want %q", log, want) + } +} + +// A transition across several groups enters the larger one first and +// leaves the smaller one first, as a hierarchy enters a superstate before +// its substate and leaves it after. +func TestGroupHooksRunOutsideInAndInsideOut(t *testing.T) { + t.Parallel() + + var log []string + note := func(what string) fsm.Hook[state] { + return func(context.Context, fsm.Transition[state]) { log = append(log, what) } + } + inner := fsm.NewGroup("inner", running) + outer := fsm.NewGroup("outer", running, done) + m, err := fsm.New("job", + // Declared inner first, to show the order is by size, not declaration. + fsm.OnEnterGroup(inner, note("enter inner")), + fsm.OnExitGroup(inner, note("exit inner")), + fsm.OnEnterGroup(outer, note("enter outer")), + fsm.OnExitGroup(outer, note("exit outer")), + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.From(done).On(evCancel).To(cancelled), + fsm.From(running).On(evCancel).To(cancelled), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx := t.Context() + for _, tc := range []struct { + from state + fire func(*state) error + want []string + }{ + {idle, func(s *state) error { _, err := m.Send(ctx, s, evStart); return err }, []string{"enter outer", "enter inner"}}, + {running, func(s *state) error { _, err := m.Send(ctx, s, evCancel); return err }, []string{"exit inner", "exit outer"}}, + {running, func(s *state) error { _, err := m.Fire(ctx, s, evFinish, 0); return err }, []string{"exit inner"}}, + } { + log = nil + st := tc.from + if err := tc.fire(&st); err != nil { + t.Fatal(err) + } + if !slices.Equal(log, tc.want) { + t.Errorf("from %v: ran %q, want %q", tc.from, log, tc.want) + } + } +} + +// The With variants hand the hook the payload of the event that crossed. +func TestGroupWithHooksSeeThePayload(t *testing.T) { + t.Parallel() + + var got []int + busy := fsm.NewGroup("busy", running) + m, err := fsm.New("job", + fsm.From(idle).On(evFinish).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.OnEnterGroupWith(busy, func(_ context.Context, _ fsm.Transition[state], n int) { got = append(got, n) }), + fsm.OnExitGroupWith(busy, func(_ context.Context, _ fsm.Transition[state], n int) { got = append(got, -n) }), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + _, _ = m.Fire(ctx, &st, evFinish, 3) + _, _ = m.Fire(ctx, &st, evFinish, 4) + if !slices.Equal(got, []int{3, -4}) { + t.Errorf("hooks saw %v, want [3 -4]", got) + } +} + +// A count of what is in a group, kept by its hooks, is exact on every move, +// moves between members included: it never dips. +func TestGroupHooksKeepAnExactCount(t *testing.T) { + t.Parallel() + + busy := fsm.NewGroup("busy", running, done) + var inBusy int + var seen []int + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.From(done).On(evStart).To(running), + fsm.FromGroup(busy).On(evCancel).To(idle), + fsm.OnEnterGroup(busy, func(context.Context, fsm.Transition[state]) { inBusy++ }), + fsm.OnExitGroup(busy, func(context.Context, fsm.Transition[state]) { inBusy-- }), + fsm.OnTransition(func(context.Context, fsm.Transition[state]) { seen = append(seen, inBusy) }), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + _, _ = m.Send(ctx, &st, evStart) // enters + _, _ = m.Fire(ctx, &st, evFinish, 0) // inside + _, _ = m.Send(ctx, &st, evStart) // inside + _, _ = m.Send(ctx, &st, evCancel) // leaves + // OnTransition runs after the exit hooks and before the entry hooks. + if want := []int{0, 1, 1, 0}; !slices.Equal(seen, want) || inBusy != 0 { + t.Errorf("count at each transition %v, ending %d; want %v ending 0", seen, inBusy, want) + } +} + +// New rejects a group hook that could never run, a With hook a crossing +// event would miss, and a nil one; a hook declares its group. +func TestBuildChecksGroupHooks(t *testing.T) { + t.Parallel() + + busy := fsm.NewGroup("busy", running) + base := []fsm.Rule[state]{ + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + } + for _, tc := range []struct { + name string + hook fsm.Rule[state] + want string + }{ + {"never leaves", fsm.OnExitGroup(fsm.NewGroup("all", idle, running, done), func(context.Context, fsm.Transition[state]) {}), + "fsm job: OnExitGroup for group all: no transition leaves it, so the hook would never run"}, + {"never enters", fsm.OnEnterGroup(fsm.NewGroup("start", idle), func(context.Context, fsm.Transition[state]) {}), + "fsm job: OnEnterGroup for group start: no transition enters it, so the hook would never run"}, + {"payload mismatch", fsm.OnEnterGroupWith(busy, func(context.Context, fsm.Transition[state], int) {}), + "fsm job: OnEnterGroupWith for group busy: transition idle --start--> running carries a different payload type, so the hook would miss it"}, + {"nil enter", fsm.OnEnterGroup(busy, nil), "fsm job: nil OnEnterGroup hook for group busy"}, + {"nil exit", fsm.OnExitGroup(busy, nil), "fsm job: nil OnExitGroup hook for group busy"}, + {"nil enter with", fsm.OnEnterGroupWith[state, int](busy, nil), "fsm job: nil OnEnterGroupWith hook for group busy"}, + {"nil exit with", fsm.OnExitGroupWith[state, int](busy, nil), "fsm job: nil OnExitGroupWith hook for group busy"}, + {"unnamed group", fsm.OnEnterGroup(fsm.NewGroup("", running), func(context.Context, fsm.Transition[state]) {}), + "fsm job: group declared with no name"}, + } { + _, err := fsm.New("job", append(slices.Clone(base), tc.hook)...) + if err == nil || err.Error() != tc.want { + t.Errorf("%s: got %v, want %q", tc.name, err, tc.want) + } + } + + m, err := fsm.New("job", append(slices.Clone(base), fsm.OnEnterGroup(busy, func(context.Context, fsm.Transition[state]) {}))...) + if err != nil { + t.Fatalf("build: %v", err) + } + if got := m.Groups(); len(got) != 1 || got[0].Name() != "busy" { + t.Errorf("Groups() = %v, want the hook's group", got) + } +} + +// A broken rule is reported alone, not also as a group hook that never runs. +func TestBrokenRuleDoesNotAlsoReportTheGroupHook(t *testing.T) { + t.Parallel() + + _, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running).Guard("g", nil), + fsm.OnExitGroup(fsm.NewGroup("busy", running), func(context.Context, fsm.Transition[state]) {}), + ) + if want := `fsm job: transition idle -> running: guard "g" is nil`; err == nil || err.Error() != want { + t.Errorf("got %v, want %q", err, want) + } +} + +// Group hooks run on Fire's path, and must not allocate there either. +func TestGroupHooksDoNotAllocate(t *testing.T) { + busy := fsm.NewGroup("busy", running) + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(idle), + fsm.OnEnterGroup(busy, func(context.Context, fsm.Transition[state]) {}), + fsm.OnExitGroupWith(busy, func(context.Context, fsm.Transition[state], int) {}), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + avg := testing.AllocsPerRun(1000, func() { + _, _ = m.Send(ctx, &st, evStart) + _, _ = m.Fire(ctx, &st, evFinish, 7) + }) + if avg != 0 { + t.Errorf("group hooks allocate %.1f times per round trip, want 0", avg) + } +} + // --- TryFire ---------------------------------------------------------------- // TryFire makes the transition Fire would, hooks and all, and says it did. @@ -1689,6 +1947,10 @@ func TestTryFireRunsTheHooksFireDoes(t *testing.T) { fsm.OnEnterVia(done, evFinish, func(_ context.Context, _ fsm.Transition[state], n int) { log = append(log, fmt.Sprintf("enter via finish %d", n)) }), + fsm.OnEnterGroup(fsm.NewGroup("busy", running), note("enter group")), + fsm.OnExitGroupWith(fsm.NewGroup("busy", running), func(_ context.Context, _ fsm.Transition[state], n int) { + log = append(log, fmt.Sprintf("exit group %d", n)) + }), ) if err != nil { t.Fatalf("build: %v", err) diff --git a/rules.go b/rules.go index 4cd9570..8811789 100644 --- a/rules.go +++ b/rules.go @@ -1,6 +1,7 @@ package fsm import ( + "cmp" "context" "errors" "fmt" @@ -72,6 +73,20 @@ type builder[S comparable] struct { // Targets of the group transitions declared so far, so that one declared // twice reads as a duplicate rather than as a group clashing with itself. groupEdges map[groupEdge]string // what the first declaration does + + // Group hooks in declaration order, attached to the rows that cross + // their group's boundary once the table is finished. + groupHooks []groupHookDecl[S] +} + +// groupHookDecl is one declared group hook, before it is attached to rows. +type groupHookDecl[S comparable] struct { + index int // in declaration order + what string // the declaring function, for errors + group Group[S] + enter bool + hook groupHook[S] + payload any // (*A)(nil) for a typed hook, nil for a plain one } // groupEdge identifies one group transition, before it expands to a row per @@ -108,6 +123,8 @@ func New[S comparable](name string, rules ...Rule[S]) (*Machine[S], error) { onEnterVia: make(map[edge[S]][]any), onExitVia: make(map[edge[S]][]any), internal: make(map[edge[S]]bool), + groupEnter: make(map[edge[S]][]groupHook[S]), + groupExit: make(map[edge[S]][]groupHook[S]), }, seen: make(map[S]bool), inherited: make(map[edge[S]]string), @@ -138,12 +155,16 @@ func New[S comparable](name string, rules ...Rule[S]) (*Machine[S], error) { for _, d := range b.deferred { d(b) } + b.attachGroupHooks() if len(b.m.table) == 0 { b.errs = append(b.errs, errors.New("no transitions declared")) } b.m.hasHooks = len(b.m.onEnter) > 0 || len(b.m.onExit) > 0 || - len(b.m.onEnterVia) > 0 || len(b.m.onExitVia) > 0 || len(b.m.onAll) > 0 + len(b.m.onEnterVia) > 0 || len(b.m.onExitVia) > 0 || len(b.m.onAll) > 0 || + len(b.m.groupEnter) > 0 || len(b.m.groupExit) > 0 + b.m.hasInternal = len(b.m.internal) > 0 + b.m.hasGroupHooks = len(b.m.groupEnter) > 0 || len(b.m.groupExit) > 0 if len(b.errs) > 0 { return nil, fmt.Errorf("fsm %s: %w", name, errors.Join(b.errs...)) } @@ -731,6 +752,140 @@ func (b *builder[S]) attachWith[A any](what, verb string, s S, h func(context.Co }) } +// OnEnterGroup declares a hook that runs when the machine enters g from a +// state outside it: just after the [OnTransition] hooks, before the entry +// hooks of the state entered. A move between members of g crosses no +// boundary and does not run it, nor does an external self-transition of a +// member, so a count of what is in the group, kept by it and +// [OnExitGroup], never dips on an intra-group move. +// +// When a transition enters several groups at once, the larger group's hooks +// run first, as a superstate's entry runs before its substate's; a group's +// own hooks run in declaration order. Entering the initial state is not a +// transition, so no group hook runs for it. +// +// A group entry hook must not fire the machine on the same state: the state +// entered has not run its entry hooks yet. [New] rejects one that no +// transition could run. +func OnEnterGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { + return ruleFunc[S](func(b *builder[S]) { + b.declareGroupHook("OnEnterGroup", g, true, groupHook[S]{plain: h}, nil, h == nil) + }) +} + +// OnExitGroup declares a hook that runs when the machine leaves g for a +// state outside it: just after the exit hooks of the state left, before the +// assignment. When a transition leaves several groups, the smaller group's +// hooks run first. See [OnEnterGroup]. +func OnExitGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { + return ruleFunc[S](func(b *builder[S]) { + b.declareGroupHook("OnExitGroup", g, false, groupHook[S]{plain: h}, nil, h == nil) + }) +} + +// OnEnterGroupWith is [OnEnterGroup] with the payload of the event that +// entered g, which every event entering g must carry: [New] reports one +// that does not, as for [OnEnterWith]. +func OnEnterGroupWith[S comparable, A any](g Group[S], h func(context.Context, Transition[S], A)) Rule[S] { + return ruleFunc[S](func(b *builder[S]) { + b.declareGroupHook("OnEnterGroupWith", g, true, groupHook[S]{typed: h}, payloadToken[A](), h == nil) + }) +} + +// OnExitGroupWith is [OnExitGroup] with the payload of the event that left +// g, which every event leaving g must carry. +func OnExitGroupWith[S comparable, A any](g Group[S], h func(context.Context, Transition[S], A)) Rule[S] { + return ruleFunc[S](func(b *builder[S]) { + b.declareGroupHook("OnExitGroupWith", g, false, groupHook[S]{typed: h}, payloadToken[A](), h == nil) + }) +} + +// declareGroupHook records a group hook, declaring its group, for +// attachGroupHooks to attach once the table is finished. +func (b *builder[S]) declareGroupHook(what string, g Group[S], enter bool, h groupHook[S], payload any, nilHook bool) { + if nilHook { + b.errs = append(b.errs, fmt.Errorf("nil %s hook for group %s", what, g.name)) + return + } + if !b.declareGroup(g) { + return + } + b.groupHooks = append(b.groupHooks, groupHookDecl[S]{ + index: len(b.groupHooks), what: what, group: g, enter: enter, hook: h, payload: payload, + }) +} + +// attachGroupHooks gives every row the group hooks it runs: those of the +// groups it leaves, smallest first, and of the groups it enters, largest +// first, each group's in declaration order. It runs last, against the +// finished table, so group-inherited rows count and declaration order does +// not matter. +func (b *builder[S]) attachGroupHooks() { + if len(b.groupHooks) == 0 { + return + } + bySize := func(asc bool) []groupHookDecl[S] { + out := slices.Clone(b.groupHooks) + slices.SortStableFunc(out, func(x, y groupHookDecl[S]) int { + if asc { + return cmp.Compare(len(x.group.members), len(y.group.members)) + } + return cmp.Compare(len(y.group.members), len(x.group.members)) + }) + return out + } + touched := make([]bool, len(b.groupHooks)) + // One error per hook and event: rows fanning in on an event share its + // payload type. + type hookEvent struct { + hook int + ev *eventDef + } + reported := make(map[hookEvent]bool) + attach := func(decls []groupHookDecl[S], enter bool) { + for _, d := range b.decls { + if d.internal { + continue + } + for _, h := range decls { + if h.enter != enter || h.group.Has(d.key.from) == enter || h.group.Has(d.to) != enter { + continue + } + touched[h.index] = true + if h.payload != nil && h.payload != d.payload { + if k := (hookEvent{h.index, d.key.ev}); !reported[k] { + reported[k] = true + b.errs = append(b.errs, fmt.Errorf( + "%s for group %s: transition %v --%s--> %v carries a different payload type, so the hook would miss it", + h.what, h.group.name, d.key.from, d.key.ev.name, d.to)) + } + continue + } + hooks := b.m.groupExit + if enter { + hooks = b.m.groupEnter + } + hooks[d.key] = append(hooks[d.key], h.hook) + } + } + } + attach(bySize(true), false) + attach(bySize(false), true) + if b.broken { + return // a broken rule may be what dropped the rows + } + for i, h := range b.groupHooks { + if !touched[i] { + verb := "leaves" + if h.enter { + verb = "enters" + } + b.errs = append(b.errs, fmt.Errorf( + "%s for group %s: no transition %s it, so the hook would never run", h.what, h.group.name, verb)) + } + } +} + // Group is a named set of states that share transitions. A transition declared // with [FromGroup] applies to every member that does not declare that event // itself, so adding a member inherits the group's transitions and a member can @@ -748,8 +903,8 @@ func (b *builder[S]) attachWith[A any](what, verb string, s S, h func(context.Co // A Group is not a state. A *S never holds one and it does not appear in // [Machine.States] — it is a declaration-time grouping that expands to // ordinary rows in the transition table, so [Machine.Fire] neither knows nor -// pays for it. Group entry and exit hooks are deliberately absent; see the -// package documentation for what that rules out. +// pays for it beyond the hooks declared on it ([OnEnterGroup], [OnExitGroup]), +// which run only on the transitions that cross its boundary. // // A Group is itself a [Rule], so a group used only for [Group.Has] or for DOT // output can be passed to [New] on its own. From 8eb10f60f935f1543c9c495c79e38d14b4beff12 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:26:37 +0500 Subject: [PATCH 4/8] DOT: draw nested groups nested, and no cluster for a crossing group A group inside another is now drawn inside its cluster, and each state in the smallest group that holds it; two groups with the same members nest by declaration order. A group that partly overlaps an earlier one cannot be drawn as a cluster without boxing other states than it has, which is what it got before: the first cluster to claim a shared state kept it, and the later group was drawn around what was left. It now gets no cluster, and a comment in the output says why. Every output was checked against Graphviz: nested clusters, a boundary arrow out of an inner one, and the comment line draw with no warnings. Machines whose groups are disjoint draw as before, byte for byte. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 21 +++++--- README.md | 2 +- docs/DESIGN.md | 15 ++++-- fsm.go | 6 +-- fsm_test.go | 128 +++++++++++++++++++++++++++++++++++++++++++++++++ introspect.go | 119 +++++++++++++++++++++++++++++++++++++-------- 6 files changed, 258 insertions(+), 33 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a93636b..63ae562 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -186,17 +186,26 @@ states and three `any` values positionally.** That is the same argument as `From`/`To`: transposable parameters of the same type do not belong in this package. +**`DOT` draws only the groups it can draw truthfully.** `drawableGroups` +keeps, in declaration order, each group that is disjoint from, inside, or +around every one kept before it; clusters nest, so those are drawn exactly — +a group in the smallest that contains it (`innermost`), a state in the +smallest group that holds it, a later twin inside an earlier one. A group +that partly overlaps a kept one gets no cluster and a `//` comment instead +(`TestGroupDOTSkipsACrossingGroup`): a node sits in one cluster, so any +cluster for it would show other members than it has. + **`DOT` collapses a group edge to the cluster boundary only when all three conditions in `collapsible` hold.** `ltail` plus `compound=true` draws one arrow from the cluster and skips the sibling rows, so a wrong `yes` does not just mislabel — Graphviz drops the `ltail` with a warning and the skipped rows vanish from the diagram. The three: every member inherited it (otherwise the -arrow claims to cover the member that overrode it); the group holds all its -members in its own cluster (an overlapping group loses members to whichever -cluster is emitted first, and a cluster cannot be the tail of an edge whose -tail node is elsewhere); and the target is not itself a member (that is a -self-loop out of its own cluster, which Graphviz also refuses). Each has a -regression test, and the outputs were checked against real `dot`. +arrow claims to cover the member that overrode it); the group is drawn as a +cluster, which then holds all its members, nested clusters included; and the +target is not itself a member (that is a self-loop out of its own cluster, +which Graphviz also refuses). Each has a regression test, and the outputs, +nested clusters and the comment line included, were checked against real +`dot`. **The collapse is keyed on `(group, *eventDef)`, never on the event name.** Two events can share a name, so counting rows by name merges two group diff --git a/README.md b/README.md index 38520da..a21ff29 100644 --- a/README.md +++ b/README.md @@ -425,7 +425,7 @@ digraph "recording" { a start dot pointing at active, active to stopped on stop, stopped to finished on finish guarded by all chunks and tracks uploaded, finished to uploaded

-A boundary arrow is drawn only when it would be accurate — per-member arrows are used when a member overrides the event, when overlapping groups cannot both hold all their members, or when the target is itself a member. Output follows declaration order, so it can be committed next to the code and diffed. +A group inside another is drawn inside its cluster. A group that partly overlaps an earlier one cannot be drawn as a cluster without boxing the wrong states, so it gets none, and a comment in the output says so. A boundary arrow is drawn only when it would be accurate — per-member arrows are used when a member overrides the event, when the group has no cluster, or when the target is itself a member. Output follows declaration order, so it can be committed next to the code and diffed. ## Performance diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 3700c34..8078aec 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -169,10 +169,19 @@ nesting there is no specificity to appeal to. `DOT` draws one arrow from a cluster boundary rather than one per member, and skips the sibling rows when it does. That makes a wrong decision worse than cosmetic: Graphviz refuses an `ltail` it cannot honour and the skipped rows -disappear, so the picture shows a transition table that does not exist. Three +disappear, so the picture shows a transition table that does not exist. Two cases therefore fall back to per-member arrows — a member that overrode the -event, a group that lost members to an overlapping cluster, and a target that -is itself a member. +event, and a target that is itself a member — and a group that is not drawn +as a cluster has no boundary to draw from. + +Clusters nest, so groups that are disjoint or contain one another are drawn +as they are: a group inside the smallest that contains it, a state inside the +smallest group that holds it, and two groups with the same members the later +inside the earlier. A group that partly overlaps an earlier drawn one cannot +be — a node sits in one cluster, so either cluster would show other members +than it has — and drawing it anyway, as this package once did, put a box +around the wrong states. It gets no cluster, and a `//` comment in the output +says why. The count behind that decision is keyed on the trigger's identity, not its name. Keying on the name merges two same-named events into one group diff --git a/fsm.go b/fsm.go index a50854e..ec32ca8 100644 --- a/fsm.go +++ b/fsm.go @@ -35,9 +35,9 @@ // learns, once the table is finished, which groups it leaves and enters, so // a count of what is in a group, kept by those hooks, is exact. // -// Groups do not nest as states do — the hooks of a group inside another run -// inside the other's, but nothing more — and entering a group does not -// select an initial member. +// Groups do not nest as states do — a group inside another is drawn inside +// it, and its hooks run inside the other's, but nothing more — and entering +// a group does not select an initial member. package fsm import ( diff --git a/fsm_test.go b/fsm_test.go index d522897..e41b570 100644 --- a/fsm_test.go +++ b/fsm_test.go @@ -3009,6 +3009,134 @@ func TestGroupDOTKeepsPerMemberEdgesForOverlappingGroups(t *testing.T) { } } +// A group inside another is drawn inside its cluster, and each state in the +// innermost cluster that holds it; a boundary arrow may leave the inner one. +func TestGroupDOTNestsAContainedGroup(t *testing.T) { + t.Parallel() + + outer := fsm.NewGroup("outer", idle, running, done) + inner := fsm.NewGroup("inner", running, done) + m, err := fsm.New("nested", + fsm.Initial(idle), + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.FromGroup(inner).On(evCancel).To(idle), + fsm.FromGroup(outer).On(evStart).To(cancelled), + inner, + ) + if err != nil { + t.Fatalf("build: %v", err) + } + want := `digraph "nested" { + rankdir=LR; + compound=true; + "__start" [shape=point]; + subgraph "cluster_outer" { + label="outer"; + style=rounded; + subgraph "cluster_inner" { + label="inner"; + style=rounded; + "running" [shape=box]; + "done" [shape=box]; + } + "idle" [shape=box]; + } + "cancelled" [shape=doublecircle]; + "__start" -> "idle"; + "idle" -> "running" [label="start"]; + "running" -> "done" [label="finish"]; + "running" -> "idle" [label="cancel", ltail="cluster_inner"]; + "running" -> "cancelled" [label="start"]; + "done" -> "cancelled" [label="start"]; +} +` + if got := m.DOT(); got != want { + t.Errorf("DOT\n%s\nwant\n%s", got, want) + } +} + +// A group that partly overlaps an earlier one cannot be drawn as a cluster +// without showing other members than it has: it is left out, with a +// comment, and its inherited edges are drawn per member. +func TestGroupDOTSkipsACrossingGroup(t *testing.T) { + t.Parallel() + + first := fsm.NewGroup("first", idle, running) + second := fsm.NewGroup("second", running, done) + m, err := fsm.New("crossing", + first, + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.FromGroup(second).On(evCancel).To(cancelled), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + want := `digraph "crossing" { + rankdir=LR; + compound=true; + subgraph "cluster_first" { + label="first"; + style=rounded; + "idle" [shape=box]; + "running" [shape=box]; + } + // group second overlaps another without containing it, so it is not drawn as a cluster + "done" [shape=box]; + "cancelled" [shape=doublecircle]; + "idle" -> "running" [label="start"]; + "running" -> "done" [label="finish"]; + "running" -> "cancelled" [label="cancel"]; + "done" -> "cancelled" [label="cancel"]; +} +` + if got := m.DOT(); got != want { + t.Errorf("DOT\n%s\nwant\n%s", got, want) + } +} + +// Two groups with the same members nest by declaration order: the later +// inside the earlier, the members inside both. +func TestGroupDOTNestsTwinGroups(t *testing.T) { + t.Parallel() + + a := fsm.NewGroup("a", running, done) + b := fsm.NewGroup("b", running, done) + m, err := fsm.New("twins", + a, b, + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evFinish).To(done), + fsm.FromGroup(b).On(evCancel).To(cancelled), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + want := `digraph "twins" { + rankdir=LR; + compound=true; + subgraph "cluster_a" { + label="a"; + style=rounded; + subgraph "cluster_b" { + label="b"; + style=rounded; + "running" [shape=box]; + "done" [shape=box]; + } + } + "idle" [shape=box]; + "cancelled" [shape=doublecircle]; + "idle" -> "running" [label="start"]; + "running" -> "done" [label="finish"]; + "running" -> "cancelled" [label="cancel", ltail="cluster_b"]; +} +` + if got := m.DOT(); got != want { + t.Errorf("DOT\n%s\nwant\n%s", got, want) + } +} + // A target inside the group would make the boundary arrow a self-loop out of // its own cluster, which Graphviz refuses. func TestGroupDOTKeepsPerMemberEdgesWhenTargetIsAMember(t *testing.T) { diff --git a/introspect.go b/introspect.go index 5bd8004..948ae61 100644 --- a/introspect.go +++ b/introspect.go @@ -106,34 +106,63 @@ func (m *Machine[S]) DOT() string { fmt.Fprintf(&b, "%s\"%s\" [shape=%s];\n", indent, dotEscape(fmt.Sprint(s)), shape) } - // Graphviz puts a node in one cluster, so a state in overlapping groups is - // drawn in the first that claims it. + // Graphviz puts a node in one cluster, and nests clusters, so groups + // that are disjoint or contain one another are drawn as they are, a + // group inside the smallest that contains it and a state inside the + // smallest group that holds it. A group that partly overlaps an earlier + // drawn one cannot be: it gets no cluster, and a comment says so. + drawn := m.drawableGroups() + parent := make(map[string]string, len(drawn)) owner := make(map[S]string, len(m.states)) - for _, g := range m.groups { - fmt.Fprintf(&b, "\tsubgraph \"cluster_%s\" {\n", dotEscape(g.name)) - fmt.Fprintf(&b, "\t\tlabel=\"%s\";\n", dotEscape(g.name)) - b.WriteString("\t\tstyle=rounded;\n") + for _, g := range drawn { + if h, ok := innermost(drawn, g.members, g.name); ok { + parent[g.name] = h + } + } + for _, s := range m.states { + if h, ok := innermost(drawn, []S{s}, ""); ok { + owner[s] = h + } + } + var cluster func(g Group[S], indent string) + cluster = func(g Group[S], indent string) { + fmt.Fprintf(&b, "%ssubgraph \"cluster_%s\" {\n", indent, dotEscape(g.name)) + fmt.Fprintf(&b, "%s\tlabel=\"%s\";\n", indent, dotEscape(g.name)) + fmt.Fprintf(&b, "%s\tstyle=rounded;\n", indent) + for _, h := range drawn { + if parent[h.name] == g.name { + cluster(h, indent+"\t") + } + } for _, s := range g.members { - if _, taken := owner[s]; taken { - continue + if owner[s] == g.name { + node(indent+"\t", s) } - owner[s] = g.name - node("\t\t", s) } - b.WriteString("\t}\n") + fmt.Fprintf(&b, "%s}\n", indent) + } + for _, g := range drawn { + if _, inner := parent[g.name]; !inner { + cluster(g, "\t") + } + } + for _, g := range m.groups { + if !slices.ContainsFunc(drawn, func(h Group[S]) bool { return h.name == g.name }) { + fmt.Fprintf(&b, "\t// group %s overlaps another without containing it, so it is not drawn as a cluster\n", + commentReplacer.Replace(g.name)) + } } for _, s := range m.states { - if _, taken := owner[s]; !taken { + if _, grouped := owner[s]; !grouped { node("\t", s) } } // A cluster can only be the tail of an edge if it really holds every one - // of its members; an overlapping group that lost some to an earlier - // cluster cannot, and Graphviz would drop the ltail with a warning. - whole := make(map[string]bool, len(m.groups)) - for _, g := range m.groups { - whole[g.name] = !slices.ContainsFunc(g.members, func(s S) bool { return owner[s] != g.name }) + // of its members, which a drawn group does, nested clusters included. + whole := make(map[string]bool, len(drawn)) + for _, g := range drawn { + whole[g.name] = true } // Rows are counted per group transition, identified by the group and the @@ -159,7 +188,7 @@ func (m *Machine[S]) DOT() string { if m.hasInitial { fmt.Fprintf(&b, "\t\"%s\" -> \"%s\";\n", start, dotEscape(fmt.Sprint(m.initial))) } - drawn := make(map[boundary]bool) + collapsed := make(map[boundary]bool) for _, e := range m.edges { // The guard is appended after escaping so that the \n stays a // Graphviz line break rather than becoming a literal backslash-n. @@ -174,10 +203,10 @@ func (m *Machine[S]) DOT() string { } if e.Group != "" && collapsible(e, byName[e.Group], rows[of(e)], whole[e.Group]) { k := of(e) - if drawn[k] { + if collapsed[k] { continue } - drawn[k] = true + collapsed[k] = true attrs = fmt.Sprintf(`, ltail="cluster_%s"`, dotEscape(e.Group)) } fmt.Fprintf(&b, "\t\"%s\" -> \"%s\" [label=\"%s\"%s];\n", @@ -188,6 +217,53 @@ func (m *Machine[S]) DOT() string { return b.String() } +// drawableGroups returns the groups that can be drawn as clusters, in +// declaration order: each one disjoint from, inside, or around every earlier +// one drawn. One that partly overlaps an earlier one is left out. +func (m *Machine[S]) drawableGroups() []Group[S] { + var out []Group[S] + for _, g := range m.groups { + if !slices.ContainsFunc(out, func(h Group[S]) bool { return crosses(g, h) }) { + out = append(out, g) + } + } + return out +} + +// crosses reports whether g and h share a member while neither holds all of +// the other's, which no nesting of clusters can draw. +func crosses[S comparable](g, h Group[S]) bool { + shared := slices.ContainsFunc(g.members, h.Has) + return shared && !subset(g.members, h) && !subset(h.members, g) +} + +func subset[S comparable](ss []S, g Group[S]) bool { + return !slices.ContainsFunc(ss, func(s S) bool { return !g.Has(s) }) +} + +// innermost names the smallest group of drawn holding all of ss, other +// than self, and reports whether there is one. Between two groups with the +// same members, the later declared is inside the earlier, so a group with +// a twin holds it only if declared before self. +func innermost[S comparable](drawn []Group[S], ss []S, self string) (string, bool) { + best, found := -1, false + for i, h := range drawn { + if h.name == self || !subset(ss, h) { + continue + } + if self != "" && len(h.members) == len(ss) && slices.IndexFunc(drawn, func(x Group[S]) bool { return x.name == self }) < i { + continue // a twin declared after self is inside it, not around it + } + if !found || len(h.members) <= len(drawn[best].members) { + best, found = i, true + } + } + if !found { + return "", false + } + return drawn[best].name, true +} + // collapsible reports whether e may be drawn once from its cluster's boundary // instead of once per member. Every case that says no would otherwise lose a // row from the diagram, because Graphviz refuses the ltail and the other rows @@ -201,6 +277,9 @@ func collapsible[S comparable](e Edge[S], g Group[S], rows int, whole bool) bool return whole && rows == len(g.members) && !g.Has(e.To) } +// commentReplacer keeps a name on its line in a // comment. +var commentReplacer = strings.NewReplacer("\n", " ", "\r", " ") + // dotReplacer makes a string safe inside double quotes in Graphviz. var dotReplacer = strings.NewReplacer(`\`, `\\`, `"`, `\"`, "\n", `\n`) From 888dcc851442cdc6ee67194d17268b6c12feab5e Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:27:02 +0500 Subject: [PATCH 5/8] docs: one payload for a machine whose hooks need the aggregate; init for a machine its hooks fire Two things the Raft port of grpcproc's elector ran into that need no code. A machine whose hooks all need the aggregate gives every event one payload type, so OnEnterWith serves every state; DESIGN.md says why a With hook does not take an interface instead. And a package-level machine whose hook fires it is an initialization cycle in Go, so it is built in init. Co-Authored-By: Claude Opus 5.5 --- README.md | 18 ++++++++++++++++++ docs/DESIGN.md | 7 ++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a21ff29..d589ed2 100644 --- a/README.md +++ b/README.md @@ -276,6 +276,24 @@ They read the finished transition table, group-inherited edges included, so an e `New` rejects a typed hook that no transition could trigger, so a terminal state takes `OnEnterWith` without `OnExitWith`. Plain hooks run first, then Via hooks, then With hooks. +A machine whose hooks all need the aggregate — a controller, an elector — tends to give every event one payload type: a small struct holding the aggregate and whatever the event adds. That is what lets `OnEnterWith` serve every state, at the price of per-event typing; name events with distinct payloads where that typing matters more. + +### Hooks that fire the machine + +An entry hook may fire the machine again, on the state it just entered: a start that fails can move straight back out. A package-level machine whose hook names it is an initialization cycle in Go, so build that one in `init`: + +```go +var lifecycle *fsm.Machine[phase] + +func init() { + lifecycle = fsm.MustNew("singleton", + fsm.From(idle).On(evReconcile).To(starting), + fsm.From(starting).On(evStartFailed).To(idle), + fsm.OnEnterVia(starting, evReconcile, launch), // launch fires lifecycle when it cannot start + ) +} +``` + ## Groups `NewGroup` names a set of states. A transition declared on the group applies to every member: diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 8078aec..3de8b3b 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -321,7 +321,12 @@ different payload type, so the hook would miss it ``` A machine whose events carry different payloads can still use them on the -states where they agree, and use `OnEnterVia` per event where they do not. A +states where they agree, and use `OnEnterVia` per event where they do not. +Letting a With hook take an interface its events' payloads implement would +need reflection at build time and a conversion at fire time, which the +one-directional type erasure exists to avoid; a machine whose hooks need the +aggregate everywhere gives its events one payload type instead, a small +struct holding the aggregate and what the event adds. A With hook no transition can trigger is rejected like a Via hook, so a terminal state takes `OnEnterWith` alone: the `OnExitWith` half could never run. From 8c5598ff142775afbb4f8da0feb67195b976df18 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:33:40 +0500 Subject: [PATCH 6/8] Nest twin groups' hooks as DOT draws them; name a start with only internal ways Group hooks were ordered by member count alone, so two groups with the same members ran their hooks interleaved in declaration order, and their exits were not the reverse of their entries, while DOT draws the later twin inside the earlier. Groups are now ranked outermost first, larger then declared earlier, and entering runs every hook of one group before the next, leaving the reverse. A start whose transitions are all internal was rejected as having "no outgoing transition", which it has. The message now says its transitions are all internal. Found by code review. Co-Authored-By: Claude Opus 5.5 --- fsm_test.go | 39 +++++++++++++++++++++++++++++++++++++-- rules.go | 26 +++++++++++++++++++------- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/fsm_test.go b/fsm_test.go index e41b570..78cc020 100644 --- a/fsm_test.go +++ b/fsm_test.go @@ -1512,8 +1512,8 @@ func TestStayIsNotAWayOut(t *testing.T) { if got := m.Terminals(); !slices.Equal(got, []state{running}) { t.Errorf("Terminals() = %v, want [running]", got) } - _, err = fsm.New("job", fsm.Initial(idle), fsm.From(idle).On(evStart).Stay()) - if want := "fsm job: initial state idle has no outgoing transition"; err == nil || err.Error() != want { + _, err = fsm.New("job", fsm.Initial(idle), fsm.From(idle).On(evStart).Stay(), fsm.From(running).On(evFinish).To(idle)) + if want := "fsm job: initial state idle has no way out: its transitions are all internal"; err == nil || err.Error() != want { t.Errorf("got %v, want %q", err, want) } } @@ -1760,6 +1760,41 @@ func TestGroupHooksRunOutsideInAndInsideOut(t *testing.T) { } } +// Two groups with the same members nest by declaration order, as DOT draws +// them: entering runs every hook of the earlier group before the later's, +// leaving the reverse. +func TestTwinGroupHooksNestByDeclaration(t *testing.T) { + t.Parallel() + + var log []string + note := func(what string) fsm.Hook[state] { + return func(context.Context, fsm.Transition[state]) { log = append(log, what) } + } + a := fsm.NewGroup("a", running, done) + b := fsm.NewGroup("b", running, done) + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(running), + fsm.From(running).On(evCancel).To(idle), + fsm.From(running).On(evFinish).To(done), + fsm.OnEnterGroup(a, note("enter a1")), + fsm.OnEnterGroup(b, note("enter b1")), + fsm.OnEnterGroup(a, note("enter a2")), + fsm.OnExitGroup(a, note("exit a1")), + fsm.OnExitGroup(b, note("exit b1")), + fsm.OnExitGroup(a, note("exit a2")), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + _, _ = m.Send(ctx, &st, evStart) + _, _ = m.Send(ctx, &st, evCancel) + want := []string{"enter a1", "enter a2", "enter b1", "exit b1", "exit a1", "exit a2"} + if !slices.Equal(log, want) { + t.Errorf("ran %q, want %q", log, want) + } +} + // The With variants hand the hook the payload of the event that crossed. func TestGroupWithHooksSeeThePayload(t *testing.T) { t.Parallel() diff --git a/rules.go b/rules.go index 8811789..de9536d 100644 --- a/rules.go +++ b/rules.go @@ -596,7 +596,11 @@ func Initial[S comparable](s S) Rule[S] { } // A dead start makes every other state unreachable; report the cause. if slices.Contains(b.m.Terminals(), s) { - b.errs = append(b.errs, fmt.Errorf("initial state %v has no outgoing transition", s)) + if slices.ContainsFunc(b.m.edges, func(e Edge[S]) bool { return e.From == s }) { + b.errs = append(b.errs, fmt.Errorf("initial state %v has no way out: its transitions are all internal", s)) + } else { + b.errs = append(b.errs, fmt.Errorf("initial state %v has no outgoing transition", s)) + } return } if u := b.m.Unreachable(s); len(u) > 0 { @@ -824,13 +828,21 @@ func (b *builder[S]) attachGroupHooks() { if len(b.groupHooks) == 0 { return } - bySize := func(asc bool) []groupHookDecl[S] { + // Groups from outermost to innermost: larger first, and of two the same + // size the one declared first, as DOT nests them. Entering runs their + // hooks in that order and leaving in its reverse, a group's own hooks in + // declaration order either way. + rank := func(g Group[S]) int { + return slices.IndexFunc(b.m.groups, func(h Group[S]) bool { return h.name == g.name }) + } + byNesting := func(outerFirst bool) []groupHookDecl[S] { out := slices.Clone(b.groupHooks) slices.SortStableFunc(out, func(x, y groupHookDecl[S]) int { - if asc { - return cmp.Compare(len(x.group.members), len(y.group.members)) + c := cmp.Or(cmp.Compare(len(y.group.members), len(x.group.members)), cmp.Compare(rank(x.group), rank(y.group))) + if !outerFirst { + c = -c } - return cmp.Compare(len(y.group.members), len(x.group.members)) + return c }) return out } @@ -869,8 +881,8 @@ func (b *builder[S]) attachGroupHooks() { } } } - attach(bySize(true), false) - attach(bySize(false), true) + attach(byNesting(false), false) + attach(byNesting(true), true) if b.broken { return // a broken rule may be what dropped the rows } From 047c70756891db6d1fdd8c4d887dedb18788b8a2 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:42:13 +0500 Subject: [PATCH 7/8] Run each row's hooks from one plan built by New Every hook a row runs, the source's exit hooks and the groups it leaves, then the transition hooks, the groups it enters and the target's entry hooks, is now one list per row, worked out last in New. Fire does one lookup for it instead of four, and a row with no hooks, an internal one included, has none, so the internal and group-hook flags are gone and TryFire's copy of Fire's tail is the action and two short loops. The declared hooks live on the builder, not the Machine. BenchmarkFireWithHooks goes from 68 to 43 ns a round trip, a third faster than main; BenchmarkFire is unchanged. This is the per-edge plan DESIGN.md once named as what group hooks would take. Also from code review: comments that justified the edit are shorter, the docs describe plans, internal transitions and group hooks where they described the old fields, README's OnTransition row excepts internal transitions, and innermost takes its group's index rather than looking it up per candidate. README's numbers are re-measured, on an M3 Max, with the comparison table, which the old numbers came from an M3 Pro for. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 38 +++++++----- README.md | 16 ++--- docs/DESIGN.md | 21 ++++--- fsm.go | 154 ++++++++++++++----------------------------------- introspect.go | 18 +++--- rules.go | 106 +++++++++++++++++++++++----------- 6 files changed, 170 insertions(+), 183 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 63ae562..585aecc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,7 +13,8 @@ Library: `fsm.go` (package doc, `Event`, `Machine`, `Fire`, `Send`, `TryFire`, interface and everything that produces one — the `From`/`FromEach`/`FromGroup` chain through `On`/`To` with its `Guard`/`Action` methods, plus `Rules`, `Group`, `OnEnter`, `OnExit`, `OnEnterVia`, `OnExitVia`, `OnEnterWith`, -`OnExitWith`, `OnTransition`, `Initial`), `introspect.go` (`States`, `Events`, `Edges`, +`OnExitWith`, `OnTransition`, `Initial`, `Stay`, `OnEnterGroup`/`OnExitGroup` and +their With variants, and `buildPlans`), `introspect.go` (`States`, `Events`, `Edges`, `Groups`, `Initial`, `Terminals`, `Unreachable`, `DOT`). Tests are `fsm_test.go` and the worked machines in `example_test.go`. @@ -88,7 +89,8 @@ allocation half; it runs without `-race`, which changes the profile. not inlined, and splitting `Fire` into a lookup and an apply step cost `BenchmarkFire` six nanoseconds a round trip. So `Fire` keeps its whole body; `TryFire` and `Check` share `lookup`, and `TryFire` carries a transition out -through `apply`, a copy of `Fire`'s tail. `TestTryFireRunsTheHooksFireDoes` +through `apply`, a copy of `Fire`'s tail — the action and the plan's two +loops. `TestTryFireRunsTheHooksFireDoes` fires the same transitions both ways with every kind of hook and compares the logs, and `FuzzMachineInvariants` checks that `TryFire` fires exactly when `To` resolves: those are what keep the copy honest. A refusal from `TryFire` @@ -99,8 +101,20 @@ for the same reason: the combining closure is built where `A` is still known. Guards run in registration order and the first rejection wins, reported under the description it was declared with. +**Every hook a row runs is one plan, built by `New`.** `buildPlans` runs last +and gives each non-internal row a `plan`: `exit` — its source's plain, Via and +With exit hooks, then the exit hooks of the groups it leaves — and `enter` — +the `OnTransition` hooks, the entry hooks of the groups it enters, then its +target's plain, Via and With entry hooks. The declared hooks live on the +`builder`, not the `Machine`; `Fire` does one `plans` lookup, behind +`hasHooks`, and an internal row, or one no hook concerns, has no plan. A +`call` is a plain `Hook` or a typed func asserted to the row's `A`, which is +safe for the reason the guard assertion is. This replaced four lookups and +cut `BenchmarkFireWithHooks` by a third. + **The order inside `Fire` is fixed** — lookup, guards, action, the -state-changed check, `OnExit(from)`, assign, `OnTransition`, `OnEnter(to)`. +state-changed check, `OnExit(from)` and the groups left, assign, +`OnTransition`, the groups entered and `OnEnter(to)`. `OnTransition` precedes the entry hooks so an entry hook that fires again is logged after its cause (`TestOnTransitionKeepsCausalOrder`); the price is that a transition hook is an observer and must not fire, or the new state's exit @@ -164,10 +178,11 @@ knows its source and target, so `attachGroupHooks` — the last step of `New`, after expansion and the deferred pass, so inherited rows count — gives every row the hooks of the groups it leaves (source in, target out) and enters, in the order they run: leaving, smaller groups first; entering, larger first; -each group's in declaration order. `Fire` then runs `groupExit[e]` after the -state's exit hooks and `groupEnter[e]` before its entry hooks, behind -`hasGroupHooks`, a precomputed bool: a map-length check there cost a -nanosecond more. An internal row runs none, and an external self-transition +each group's in declaration order, groups ranked outermost first — larger, +then declared earlier, so twins nest as `DOT` draws them +(`TestTwinGroupHooksNestByDeclaration`). `buildPlans` puts them in each row's +plan after the state's exit hooks and before its entry hooks. An internal row +runs none, and an external self-transition of a member crosses nothing. Pinned by `TestGroupHooksRunOnlyAcrossTheBoundary`, `TestGroupHooksRunBetweenStateHooks`, `TestGroupHooksRunOutsideInAndInsideOut` and `TestGroupHooksKeepAnExactCount`. A group entry hook must not fire on the @@ -261,9 +276,7 @@ once already, in `DOT`'s group-edge collapse, and produced an arrow that described a machine nobody had declared. **A machine with no hooks must not pay for hooks.** `Machine.hasHooks` is set -at construction and short-circuits the whole block in `Fire`, including the -map lookups the plain hooks would do — it is why the hookless path is faster -than it was before payload hooks existed. `BenchmarkFire` and +at construction and skips the plan lookup in `Fire`. `BenchmarkFire` and `BenchmarkFireWithHooks` are the pair that keeps this honest; measure both before and after touching the fire path. @@ -273,9 +286,8 @@ exit then entry, so a counter kept by those hooks dips and returns. Pinned by transition that skips the hooks. The internal kind is its own declaration, `From(a).On(ev).Stay()`: guards and actions run, then `Fire` returns before any hook, `OnTransition` included (`TestStayRunsGuardsAndActionsButNoHook`). -`Machine.internal` holds those rows, and `Fire` and `apply` look it up only -inside the hook block, behind `hasInternal`, so a hookless machine pays -nothing. An internal row is registered with `to` set to its source, which +`builder.internal` holds those rows, and `buildPlans` gives them no plan, so +`Fire` needs no check of its own. An internal row is registered with `to` set to its source, which keeps `To`, `Check` and the fuzzer's Fire-agrees-with-To invariant uniform, but it is not a way out (`Terminals` skips it) and it enters and leaves nothing (`decl.internal` keeps it out of every Via and With hook). diff --git a/README.md b/README.md index d589ed2..2b5f360 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,7 @@ Each of these returns a `Rule[S]`, the single option type. Rules are plain value | `OnEnter(s, h)` / `OnExit(s, h)` | [hooks](#hooks) around the assignment | | `OnEnterVia(s, ev, h)` / `OnExitVia(s, ev, h)` | hooks that see the payload of one event | | `OnEnterWith(s, h)` / `OnExitWith(s, h)` | hooks that see the payload of every event entering or leaving `s` | -| `OnTransition(h)` | a hook on every transition | +| `OnTransition(h)` | a hook on every transition but an internal one | | `OnEnterGroup(g, h)` / `OnExitGroup(g, h)` | [hooks](#group-hooks) on the transitions that cross `g`'s boundary; `…With` variants see the payload | | `NewGroup(name, a, b)` | a [group](#groups), usable as a rule on its own | | `Rules(...)` | several rules as one value | @@ -447,12 +447,12 @@ A group inside another is drawn inside its cluster. A group that partly overlaps ## Performance -Guards and actions are combined at construction, where the payload type is still known, and stored as a concrete `func(context.Context, A) error`. `Fire` asserts back to that type and passes the payload directly, so nothing is boxed; a machine with no hooks skips the hook block on one flag check. +Guards and actions are combined at construction, where the payload type is still known, and stored as a concrete `func(context.Context, A) error`. `Fire` asserts back to that type and passes the payload directly, so nothing is boxed. Hooks are worked out at construction too: every row gets one list of what it runs, so a machine with hooks does one lookup per fire, and one without skips even that on a flag. ``` -goos: darwin, goarch: arm64, cpu: Apple M3 Pro -BenchmarkFire-12 32.54 ns/op 0 B/op 0 allocs/op -BenchmarkFireWithHooks-12 65.17 ns/op 0 B/op 0 allocs/op +goos: darwin, goarch: arm64, cpu: Apple M3 Max +BenchmarkFire-14 33.00 ns/op 0 B/op 0 allocs/op +BenchmarkFireWithHooks-14 43.69 ns/op 0 B/op 0 allocs/op ``` One iteration is two fires, one with an action, so a single `Fire` is about 16 ns. `BenchmarkFireWithHooks` is the same round trip with an entry hook, an exit hook and both payload hooks. @@ -463,9 +463,9 @@ The same `a → b → a` cycle, built once and fired in a loop; one iteration is | | plain | guard + action | payload | | --- | --- | --- | --- | -| **`floatdrop/fsm`** | **25.6 ns**, 0 allocs | **52.8 ns**, 0 allocs | **34.8 ns**, 0 allocs | -| [`qmuntal/stateless`](https://github.com/qmuntal/stateless) v1.8.0 | 184 ns, 0 allocs | 269 ns, 6 allocs | 260 ns, 6 allocs | -| [`looplab/fsm`](https://github.com/looplab/fsm) v1.0.4 | 384 ns, 10 allocs | 553 ns, 10 allocs | 549 ns, 12 allocs | +| **`floatdrop/fsm`** | **26.1 ns**, 0 allocs | **49.6 ns**, 0 allocs | **35.6 ns**, 0 allocs | +| [`qmuntal/stateless`](https://github.com/qmuntal/stateless) v1.8.0 | 188 ns, 0 allocs | 269 ns, 6 allocs | 267 ns, 6 allocs | +| [`looplab/fsm`](https://github.com/looplab/fsm) v1.0.4 | 405 ns, 10 allocs | 562 ns, 10 allocs | 545 ns, 12 allocs | These are different designs, not a ranking: diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 3de8b3b..a5f92a7 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -153,8 +153,8 @@ groups', the smaller group first; entering: the larger group first, then the state's own. With overlapping groups there is no hierarchy to appeal to, and size, then declaration order, decides. A group entry hook runs before the state entered has run its own, so it must not fire the machine, as a -transition hook must not. The cost is one flag check per direction in a -machine with hooks, and none in one without. +transition hook must not. They are part of each row's plan, so they cost +nothing beyond running them. ### What groups still are not @@ -260,7 +260,7 @@ lookup → guards → action → exit(from) → *st = to → transition hooks ``` If the lookup, a guard, or the action fails, the state is untouched and no hook -runs. `OnTransition` hooks run for every edge, just after the assignment; they +runs. `OnTransition` hooks run for every edge but an internal one, just after the assignment; they are where an audit log or a trace goes, instead of one `OnEnter` per state that a new state would silently escape. They run before the entry hooks so that an entry hook which fires the machine again is logged after the @@ -333,9 +333,13 @@ state takes `OnEnterWith` alone: the `OnExitWith` half could never run. ### Ordering and cost Plain hooks of a state run first, then its Via hooks, then its With hooks, -which attach last because they wait for the finished table. None of them costs -anything on a machine that declares none: a single flag set at construction skips the whole -hook block, including the lookups the plain hooks would do. +which attach last because they wait for the finished table. `New` then works +out, for every row, the one list of hooks it runs — its source's exit hooks +and the groups it leaves, then after the assignment the transition hooks, the +groups it enters and its target's entry hooks — so `Fire` does a single lookup +for them, and a machine that declares no hook skips even that on a flag. That +made `BenchmarkFireWithHooks` a third faster than the four lookups it +replaced. ### Branch on the trigger, not on its name @@ -373,9 +377,8 @@ again: `NoTransitionError` means the state refuses the event rather than An internal transition enters and leaves nothing, so it is not a way out (`Terminals` keeps a state that has only these), and it triggers no Via or With hook — one that only an internal transition could trigger is rejected -as never running. `Fire` checks for one only inside its hook block, so a -machine without hooks pays nothing for the feature, and one without internal -transitions pays a length check. +as never running. An internal row has no hooks in its plan, so `Fire` needs no +check of its own for it. ## Fire does not allocate diff --git a/fsm.go b/fsm.go index ec32ca8..910bd9e 100644 --- a/fsm.go +++ b/fsm.go @@ -112,13 +112,19 @@ func (t Transition[S]) Is[A any](ev Event[A]) bool { return t.trigger != nil && t.trigger == ev.def } -// groupHook is one group hook on one row: plain, or typed in the payload -// of the row's event, func(context.Context, Transition[S], A). -type groupHook[S comparable] struct { +// call is one hook as a row runs it: plain, or typed in the payload of the +// row's event, func(context.Context, Transition[S], A). +type call[S comparable] struct { plain Hook[S] typed any } +// plan is every hook a row runs, in the order it runs them: exit before +// the assignment; enter after it, the transition hooks first. +type plan[S comparable] struct { + exit, enter []call[S] +} + // edge is the key of the transition table. type edge[S comparable] struct { from S @@ -146,37 +152,13 @@ type Machine[S comparable] struct { table map[edge[S]]S guards map[edge[S]]any // func(context.Context, A) (string, error) actions map[edge[S]]any // func(context.Context, A) error - onEnter map[S][]Hook[S] - onExit map[S][]Hook[S] - - // Payload-aware hooks, keyed by (state, event) rather than (from, event). - // Each holds func(context.Context, Transition[S], A) for the A of that - // event. Kept separate so a machine that declares none pays one length - // check per fire. - onEnterVia map[edge[S]][]any - onExitVia map[edge[S]][]any - - // The rows declared with [OnStep.Stay], which run no hook. Empty in a - // machine that declares none, which then pays one length check per fire. - internal map[edge[S]]bool - - // Group hooks, per row: the ones a row runs because it leaves, or - // enters, a group, in the order they run. Worked out once the table is - // finished, so a move between members of a group, which crosses no - // boundary, has none. - groupExit map[edge[S]][]groupHook[S] - groupEnter map[edge[S]][]groupHook[S] - - // Hooks that run after every transition, declared with [OnTransition]. - onAll []Hook[S] - - // True when any hook is declared. A machine that declares none skips the - // hook block and its lookups. - hasHooks bool - // True when the machine declares an internal transition, or a group - // hook: one load each in the hook block, cheaper than a map's length. - hasInternal, hasGroupHooks bool + // plans holds the hooks of every row that runs any, worked out by New. + // A row with none, an internal one included, has no plan. + plans map[edge[S]]*plan[S] + + // True when some row has a plan; a machine with none skips the lookup. + hasHooks bool // The state declared with [Initial], if any. initial S @@ -259,44 +241,24 @@ func (m *Machine[S]) Fire[A any](ctx context.Context, st *S, ev Event[A], arg A) *st = to return t, nil } - // An internal transition leaves nothing, so it runs no hook; *st is - // already to. - if m.hasInternal && m.internal[e] { + p := m.plans[e] + if p == nil { // a row with no hooks, an internal one included + *st = to return t, nil } - - for _, h := range m.onExit[from] { - h(ctx, t) - } - if len(m.onExitVia) > 0 { - for _, raw := range m.onExitVia[e] { - raw.(func(context.Context, Transition[S], A))(ctx, t, arg) + for _, c := range p.exit { + if c.plain != nil { + c.plain(ctx, t) + } else { + c.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) } } - if m.hasGroupHooks { - for _, g := range m.groupExit[e] { - runGroupHook(ctx, g, t, arg) - } - } - *st = to - - // Before the entry hooks, so an entry hook that fires again logs after - // this one. That is why a transition hook must not fire itself. - for _, h := range m.onAll { - h(ctx, t) - } - if m.hasGroupHooks { - for _, g := range m.groupEnter[e] { - runGroupHook(ctx, g, t, arg) - } - } - for _, h := range m.onEnter[to] { - h(ctx, t) - } - if len(m.onEnterVia) > 0 { - for _, raw := range m.onEnterVia[edge[S]{from: to, ev: ev.def}] { - raw.(func(context.Context, Transition[S], A))(ctx, t, arg) + for _, c := range p.enter { + if c.plain != nil { + c.plain(ctx, t) + } else { + c.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) } } return t, nil @@ -357,8 +319,8 @@ func (m *Machine[S]) lookup[A any](ctx context.Context, from S, ev Event[A], arg } // apply carries out a transition its guards allowed: the action, then the -// hooks around the assignment. It is the tail of Fire, which keeps its own -// copy rather than call this: the call costs Fire about two nanoseconds. +// row's plan around the assignment. It is the tail of Fire, which keeps its +// own copy. func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Event[A], arg A) (Transition[S], error) { var none Transition[S] e := edge[S]{from: from, ev: ev.def} @@ -379,44 +341,24 @@ func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Eve *st = to return t, nil } - // An internal transition leaves nothing, so it runs no hook; *st is - // already to. - if m.hasInternal && m.internal[e] { + p := m.plans[e] + if p == nil { // a row with no hooks, an internal one included + *st = to return t, nil } - - for _, h := range m.onExit[from] { - h(ctx, t) - } - if len(m.onExitVia) > 0 { - for _, raw := range m.onExitVia[e] { - raw.(func(context.Context, Transition[S], A))(ctx, t, arg) + for _, c := range p.exit { + if c.plain != nil { + c.plain(ctx, t) + } else { + c.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) } } - if m.hasGroupHooks { - for _, g := range m.groupExit[e] { - runGroupHook(ctx, g, t, arg) - } - } - *st = to - - // Before the entry hooks, so an entry hook that fires again logs after - // this one. That is why a transition hook must not fire itself. - for _, h := range m.onAll { - h(ctx, t) - } - if m.hasGroupHooks { - for _, g := range m.groupEnter[e] { - runGroupHook(ctx, g, t, arg) - } - } - for _, h := range m.onEnter[to] { - h(ctx, t) - } - if len(m.onEnterVia) > 0 { - for _, raw := range m.onEnterVia[edge[S]{from: to, ev: ev.def}] { - raw.(func(context.Context, Transition[S], A))(ctx, t, arg) + for _, c := range p.enter { + if c.plain != nil { + c.plain(ctx, t) + } else { + c.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) } } return t, nil @@ -556,13 +498,3 @@ func (e *StateChangedError[S]) Error() string { return fmt.Sprintf("fsm %s: state changed to %v during %v --%s--> %v", e.Machine, e.Found, e.From, e.Event, e.To) } - -// runGroupHook runs g with the payload, if it takes one. Its A is the -// payload type of the row it was attached to, which New checked. -func runGroupHook[S comparable, A any](ctx context.Context, g groupHook[S], t Transition[S], arg A) { - if g.plain != nil { - g.plain(ctx, t) - return - } - g.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) -} diff --git a/introspect.go b/introspect.go index 948ae61..94dcdce 100644 --- a/introspect.go +++ b/introspect.go @@ -114,13 +114,13 @@ func (m *Machine[S]) DOT() string { drawn := m.drawableGroups() parent := make(map[string]string, len(drawn)) owner := make(map[S]string, len(m.states)) - for _, g := range drawn { - if h, ok := innermost(drawn, g.members, g.name); ok { + for i, g := range drawn { + if h, ok := innermost(drawn, g.members, i); ok { parent[g.name] = h } } for _, s := range m.states { - if h, ok := innermost(drawn, []S{s}, ""); ok { + if h, ok := innermost(drawn, []S{s}, -1); ok { owner[s] = h } } @@ -242,16 +242,16 @@ func subset[S comparable](ss []S, g Group[S]) bool { } // innermost names the smallest group of drawn holding all of ss, other -// than self, and reports whether there is one. Between two groups with the -// same members, the later declared is inside the earlier, so a group with -// a twin holds it only if declared before self. -func innermost[S comparable](drawn []Group[S], ss []S, self string) (string, bool) { +// than drawn[self] (self is -1 for a state), and reports whether there is +// one. Of two groups with the same members the later is inside the earlier, +// so a twin holds drawn[self] only if declared before it. +func innermost[S comparable](drawn []Group[S], ss []S, self int) (string, bool) { best, found := -1, false for i, h := range drawn { - if h.name == self || !subset(ss, h) { + if i == self || !subset(ss, h) { continue } - if self != "" && len(h.members) == len(ss) && slices.IndexFunc(drawn, func(x Group[S]) bool { return x.name == self }) < i { + if self >= 0 && len(h.members) == len(ss) && self < i { continue // a twin declared after self is inside it, not around it } if !found || len(h.members) <= len(drawn[best].members) { diff --git a/rules.go b/rules.go index de9536d..c2f7ea1 100644 --- a/rules.go +++ b/rules.go @@ -77,6 +77,13 @@ type builder[S comparable] struct { // Group hooks in declaration order, attached to the rows that cross // their group's boundary once the table is finished. groupHooks []groupHookDecl[S] + + // Hooks as declared, which buildPlans turns into each row's plan. + onEnter, onExit map[S][]Hook[S] + onEnterVia, onExitVia map[edge[S]][]any // keyed by (state, event); func(context.Context, Transition[S], A) + onAll []Hook[S] + groupEnter, groupExit map[edge[S]][]call[S] // keyed by row, in the order they run + internal map[edge[S]]bool // the rows declared with Stay } // groupHookDecl is one declared group hook, before it is attached to rows. @@ -85,7 +92,7 @@ type groupHookDecl[S comparable] struct { what string // the declaring function, for errors group Group[S] enter bool - hook groupHook[S] + hook call[S] payload any // (*A)(nil) for a typed hook, nil for a plain one } @@ -114,18 +121,19 @@ func payloadToken[A any]() any { return (*A)(nil) } func New[S comparable](name string, rules ...Rule[S]) (*Machine[S], error) { b := &builder[S]{ m: &Machine[S]{ - name: name, - table: make(map[edge[S]]S), - guards: make(map[edge[S]]any), - actions: make(map[edge[S]]any), - onEnter: make(map[S][]Hook[S]), - onExit: make(map[S][]Hook[S]), - onEnterVia: make(map[edge[S]][]any), - onExitVia: make(map[edge[S]][]any), - internal: make(map[edge[S]]bool), - groupEnter: make(map[edge[S]][]groupHook[S]), - groupExit: make(map[edge[S]][]groupHook[S]), + name: name, + table: make(map[edge[S]]S), + guards: make(map[edge[S]]any), + actions: make(map[edge[S]]any), + plans: make(map[edge[S]]*plan[S]), }, + onEnter: make(map[S][]Hook[S]), + onExit: make(map[S][]Hook[S]), + onEnterVia: make(map[edge[S]][]any), + onExitVia: make(map[edge[S]][]any), + internal: make(map[edge[S]]bool), + groupEnter: make(map[edge[S]][]call[S]), + groupExit: make(map[edge[S]][]call[S]), seen: make(map[S]bool), inherited: make(map[edge[S]]string), groupEdges: make(map[groupEdge]string), @@ -156,15 +164,12 @@ func New[S comparable](name string, rules ...Rule[S]) (*Machine[S], error) { d(b) } b.attachGroupHooks() + b.buildPlans() if len(b.m.table) == 0 { b.errs = append(b.errs, errors.New("no transitions declared")) } - b.m.hasHooks = len(b.m.onEnter) > 0 || len(b.m.onExit) > 0 || - len(b.m.onEnterVia) > 0 || len(b.m.onExitVia) > 0 || len(b.m.onAll) > 0 || - len(b.m.groupEnter) > 0 || len(b.m.groupExit) > 0 - b.m.hasInternal = len(b.m.internal) > 0 - b.m.hasGroupHooks = len(b.m.groupEnter) > 0 || len(b.m.groupExit) > 0 + b.m.hasHooks = len(b.m.plans) > 0 if len(b.errs) > 0 { return nil, fmt.Errorf("fsm %s: %w", name, errors.Join(b.errs...)) } @@ -482,7 +487,7 @@ func (t *ToStep[S, A]) redeclared() string { // does says what the registered row e does, as ToStep.does. func (b *builder[S]) does(e edge[S]) string { - if b.m.internal[e] { + if b.internal[e] { return "stays" } return fmt.Sprintf("goes to %v", b.m.table[e]) @@ -510,7 +515,7 @@ func (b *builder[S]) register(r row[S]) { e := edge[S]{from: r.from, ev: r.ev} b.m.table[e] = r.to if r.internal { - b.m.internal[e] = true + b.internal[e] = true } b.decls = append(b.decls, decl[S]{key: e, to: r.to, internal: r.internal, payload: r.payload}) b.declare(r.from) @@ -535,7 +540,7 @@ func OnEnter[S comparable](s S, h Hook[S]) Rule[S] { b.errs = append(b.errs, fmt.Errorf("nil OnEnter hook for state %v", s)) return } - b.m.onEnter[s] = append(b.m.onEnter[s], h) + b.onEnter[s] = append(b.onEnter[s], h) b.declare(s) }) } @@ -547,7 +552,7 @@ func OnExit[S comparable](s S, h Hook[S]) Rule[S] { b.errs = append(b.errs, fmt.Errorf("nil OnExit hook for state %v", s)) return } - b.m.onExit[s] = append(b.m.onExit[s], h) + b.onExit[s] = append(b.onExit[s], h) b.declare(s) }) } @@ -568,7 +573,7 @@ func OnTransition[S comparable](h Hook[S]) Rule[S] { b.errs = append(b.errs, errors.New("nil OnTransition hook")) return } - b.m.onAll = append(b.m.onAll, h) + b.onAll = append(b.onAll, h) }) } @@ -631,7 +636,7 @@ func OnEnterVia[S comparable, A any](s S, ev Event[A], h func(context.Context, T return } k := edge[S]{from: s, ev: ev.def} - b.m.onEnterVia[k] = append(b.m.onEnterVia[k], h) + b.onEnterVia[k] = append(b.onEnterVia[k], h) b.requireVia("OnEnterVia", "enters", s, ev.def, func(d decl[S]) bool { return d.to == s && !d.internal }) }) } @@ -646,7 +651,7 @@ func OnExitVia[S comparable, A any](s S, ev Event[A], h func(context.Context, Tr return } k := edge[S]{from: s, ev: ev.def} - b.m.onExitVia[k] = append(b.m.onExitVia[k], h) + b.onExitVia[k] = append(b.onExitVia[k], h) b.requireVia("OnExitVia", "leaves", s, ev.def, func(d decl[S]) bool { return d.key.from == s && !d.internal }) }) } @@ -705,7 +710,7 @@ func (b *builder[S]) requireVia(what, verb string, s S, def *eventDef, match fun // It runs after the plain and the [OnEnterVia] entry hooks of the same state. func OnEnterWith[S comparable, A any](s S, h func(context.Context, Transition[S], A)) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.attachWith("OnEnterWith", "enters", s, h, b.m.onEnterVia, func(d decl[S]) bool { return d.to == s && !d.internal }) + b.attachWith("OnEnterWith", "enters", s, h, b.onEnterVia, func(d decl[S]) bool { return d.to == s && !d.internal }) }) } @@ -715,7 +720,7 @@ func OnEnterWith[S comparable, A any](s S, h func(context.Context, Transition[S] // the same state, and still before the state changes. func OnExitWith[S comparable, A any](s S, h func(context.Context, Transition[S], A)) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.attachWith("OnExitWith", "leaves", s, h, b.m.onExitVia, func(d decl[S]) bool { return d.key.from == s && !d.internal }) + b.attachWith("OnExitWith", "leaves", s, h, b.onExitVia, func(d decl[S]) bool { return d.key.from == s && !d.internal }) }) } @@ -773,7 +778,7 @@ func (b *builder[S]) attachWith[A any](what, verb string, s S, h func(context.Co // transition could run. func OnEnterGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.declareGroupHook("OnEnterGroup", g, true, groupHook[S]{plain: h}, nil, h == nil) + b.declareGroupHook("OnEnterGroup", g, true, call[S]{plain: h}, nil, h == nil) }) } @@ -783,7 +788,7 @@ func OnEnterGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { // hooks run first. See [OnEnterGroup]. func OnExitGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.declareGroupHook("OnExitGroup", g, false, groupHook[S]{plain: h}, nil, h == nil) + b.declareGroupHook("OnExitGroup", g, false, call[S]{plain: h}, nil, h == nil) }) } @@ -792,7 +797,7 @@ func OnExitGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { // that does not, as for [OnEnterWith]. func OnEnterGroupWith[S comparable, A any](g Group[S], h func(context.Context, Transition[S], A)) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.declareGroupHook("OnEnterGroupWith", g, true, groupHook[S]{typed: h}, payloadToken[A](), h == nil) + b.declareGroupHook("OnEnterGroupWith", g, true, call[S]{typed: h}, payloadToken[A](), h == nil) }) } @@ -800,13 +805,13 @@ func OnEnterGroupWith[S comparable, A any](g Group[S], h func(context.Context, T // g, which every event leaving g must carry. func OnExitGroupWith[S comparable, A any](g Group[S], h func(context.Context, Transition[S], A)) Rule[S] { return ruleFunc[S](func(b *builder[S]) { - b.declareGroupHook("OnExitGroupWith", g, false, groupHook[S]{typed: h}, payloadToken[A](), h == nil) + b.declareGroupHook("OnExitGroupWith", g, false, call[S]{typed: h}, payloadToken[A](), h == nil) }) } // declareGroupHook records a group hook, declaring its group, for // attachGroupHooks to attach once the table is finished. -func (b *builder[S]) declareGroupHook(what string, g Group[S], enter bool, h groupHook[S], payload any, nilHook bool) { +func (b *builder[S]) declareGroupHook(what string, g Group[S], enter bool, h call[S], payload any, nilHook bool) { if nilHook { b.errs = append(b.errs, fmt.Errorf("nil %s hook for group %s", what, g.name)) return @@ -873,9 +878,9 @@ func (b *builder[S]) attachGroupHooks() { } continue } - hooks := b.m.groupExit + hooks := b.groupExit if enter { - hooks = b.m.groupEnter + hooks = b.groupEnter } hooks[d.key] = append(hooks[d.key], h.hook) } @@ -898,6 +903,41 @@ func (b *builder[S]) attachGroupHooks() { } } +// buildPlans gives every row the hooks it runs, in order: the exit hooks of +// its source, plain, Via and With, then of the groups it leaves; and after +// the assignment the transition hooks, the entry hooks of the groups it +// enters, then of its target, plain, Via and With. An internal row runs +// none. +func (b *builder[S]) buildPlans() { + plain := func(hs []Hook[S]) []call[S] { + out := make([]call[S], len(hs)) + for i, h := range hs { + out[i] = call[S]{plain: h} + } + return out + } + typed := func(hs []any) []call[S] { + out := make([]call[S], len(hs)) + for i, h := range hs { + out[i] = call[S]{typed: h} + } + return out + } + for _, d := range b.decls { + if d.internal { + continue + } + p := &plan[S]{ + exit: slices.Concat(plain(b.onExit[d.key.from]), typed(b.onExitVia[d.key]), b.groupExit[d.key]), + enter: slices.Concat(plain(b.onAll), b.groupEnter[d.key], + plain(b.onEnter[d.to]), typed(b.onEnterVia[edge[S]{from: d.to, ev: d.key.ev}])), + } + if len(p.exit) > 0 || len(p.enter) > 0 { + b.m.plans[d.key] = p + } + } +} + // Group is a named set of states that share transitions. A transition declared // with [FromGroup] applies to every member that does not declare that event // itself, so adding a member inherits the group's transitions and a member can From 6fa81ae4841d8cec8a55c0a7d61bdc144c467100 Mon Sep 17 00:00:00 2001 From: Vsevolod Strukchinsky Date: Sat, 3 Oct 2026 22:49:47 +0500 Subject: [PATCH 8/8] Order same-size crossing groups' hooks by declaration; share plan lists The twin rule ranked every pair of same-size groups by group declaration and reversed it on leaving, so two groups that only partly overlap, which DOT does not nest, had their exits flipped: x and y left as exit y, exit x. Group hooks now sort by (size, which group's hooks came first, group rank), and only twins, groups with the same members, reverse on leaving. TestCrossingGroupHooksKeepDeclarationOrder pins it, and the godoc states the rule. Hooks are stored as calls when declared, so buildPlans joins the lists without rewrapping them, and a row whose hooks come from one list, every row with only transition hooks say, shares it rather than copying it. Group ranks are computed once rather than looked up in the comparator, and TryFire's apply runs a plan through one helper; Fire keeps its loops inline. Stale comments and docs that said attachGroupHooks runs last, or that an internal row has an empty plan, are fixed. Found by a second code review. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 13 +++-- README.md | 2 +- docs/DESIGN.md | 8 +-- fsm.go | 18 +++---- fsm_test.go | 31 ++++++++++++ rules.go | 134 +++++++++++++++++++++++++++---------------------- 6 files changed, 128 insertions(+), 78 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 585aecc..02323a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -174,13 +174,16 @@ the expansion produces rows indistinguishable from hand-written ones. **Group hooks are worked out per row, after everything else.** A flat table can suppress a superstate's hooks on an intra-group move after all: each row -knows its source and target, so `attachGroupHooks` — the last step of `New`, -after expansion and the deferred pass, so inherited rows count — gives every +knows its source and target, so `attachGroupHooks` — run after expansion and +the deferred pass, so inherited rows count, and just before `buildPlans` — gives every row the hooks of the groups it leaves (source in, target out) and enters, in the order they run: leaving, smaller groups first; entering, larger first; -each group's in declaration order, groups ranked outermost first — larger, -then declared earlier, so twins nest as `DOT` draws them -(`TestTwinGroupHooksNestByDeclaration`). `buildPlans` puts them in each row's +each group's in declaration order. Of groups the same size, the one whose +hooks were declared first goes first both ways +(`TestCrossingGroupHooksKeepDeclarationOrder`), except twins — same members +— which nest by group declaration as `DOT` draws them, so leaving runs the +later twin first (`TestTwinGroupHooksNestByDeclaration`). The sort key is +(size, twin set by first hook, group rank), so it is a total order. `buildPlans` puts them in each row's plan after the state's exit hooks and before its entry hooks. An internal row runs none, and an external self-transition of a member crosses nothing. Pinned by `TestGroupHooksRunOnlyAcrossTheBoundary`, diff --git a/README.md b/README.md index 2b5f360..456046a 100644 --- a/README.md +++ b/README.md @@ -447,7 +447,7 @@ A group inside another is drawn inside its cluster. A group that partly overlaps ## Performance -Guards and actions are combined at construction, where the payload type is still known, and stored as a concrete `func(context.Context, A) error`. `Fire` asserts back to that type and passes the payload directly, so nothing is boxed. Hooks are worked out at construction too: every row gets one list of what it runs, so a machine with hooks does one lookup per fire, and one without skips even that on a flag. +Guards and actions are combined at construction, where the payload type is still known, and stored as a concrete `func(context.Context, A) error`. `Fire` asserts back to that type and passes the payload directly, so nothing is boxed. Hooks are worked out at construction too: every row gets a plan, the hooks it runs before the assignment and after it, so a machine with hooks does one lookup per fire, and one without skips even that on a flag. ``` goos: darwin, goarch: arm64, cpu: Apple M3 Max diff --git a/docs/DESIGN.md b/docs/DESIGN.md index a5f92a7..0465245 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -150,8 +150,10 @@ leading becomes handing over. Before, that was a flag beside the machine. The order is the hierarchy's. Leaving: the state's exit hooks, then its groups', the smaller group first; entering: the larger group first, then the -state's own. With overlapping groups there is no hierarchy to appeal to, and -size, then declaration order, decides. A group entry hook runs before the +state's own. Of two groups the same size there is no hierarchy to appeal to, +and the one whose hooks were declared first goes first, entering and leaving +alike — unless they have the same members, when the later-declared one is +inside the earlier, as `DOT` draws them, and is left first. A group entry hook runs before the state entered has run its own, so it must not fire the machine, as a transition hook must not. They are part of each row's plan, so they cost nothing beyond running them. @@ -377,7 +379,7 @@ again: `NoTransitionError` means the state refuses the event rather than An internal transition enters and leaves nothing, so it is not a way out (`Terminals` keeps a state that has only these), and it triggers no Via or With hook — one that only an internal transition could trigger is rejected -as never running. An internal row has no hooks in its plan, so `Fire` needs no +as never running. An internal row has no plan at all, so `Fire` needs no check of its own for it. ## Fire does not allocate diff --git a/fsm.go b/fsm.go index 910bd9e..defeb71 100644 --- a/fsm.go +++ b/fsm.go @@ -346,22 +346,22 @@ func (m *Machine[S]) apply[A any](ctx context.Context, st *S, from, to S, ev Eve *st = to return t, nil } - for _, c := range p.exit { - if c.plain != nil { - c.plain(ctx, t) - } else { - c.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) - } - } + run(ctx, p.exit, t, arg) *st = to - for _, c := range p.enter { + run(ctx, p.enter, t, arg) + return t, nil +} + +// run runs calls with the payload, for apply. Fire keeps the loop inline: +// the call would cost it. +func run[S comparable, A any](ctx context.Context, calls []call[S], t Transition[S], arg A) { + for _, c := range calls { if c.plain != nil { c.plain(ctx, t) } else { c.typed.(func(context.Context, Transition[S], A))(ctx, t, arg) } } - return t, nil } // Send fires an event that carries no payload. It is [Machine.Fire] with the diff --git a/fsm_test.go b/fsm_test.go index 78cc020..23936e5 100644 --- a/fsm_test.go +++ b/fsm_test.go @@ -1795,6 +1795,37 @@ func TestTwinGroupHooksNestByDeclaration(t *testing.T) { } } +// Groups the same size that only partly overlap are no hierarchy: the one +// whose hooks were declared first goes first, entering and leaving alike. +func TestCrossingGroupHooksKeepDeclarationOrder(t *testing.T) { + t.Parallel() + + var log []string + note := func(what string) fsm.Hook[state] { + return func(context.Context, fsm.Transition[state]) { log = append(log, what) } + } + x := fsm.NewGroup("x", running, done) + y := fsm.NewGroup("y", done, cancelled) + m, err := fsm.New("job", + fsm.From(idle).On(evStart).To(done), + fsm.From(done).On(evCancel).To(idle), + fsm.From(running).On(evFinish).To(cancelled), + fsm.OnEnterGroup(x, note("enter x")), + fsm.OnEnterGroup(y, note("enter y")), + fsm.OnExitGroup(x, note("exit x")), + fsm.OnExitGroup(y, note("exit y")), + ) + if err != nil { + t.Fatalf("build: %v", err) + } + ctx, st := t.Context(), idle + _, _ = m.Send(ctx, &st, evStart) + _, _ = m.Send(ctx, &st, evCancel) + if want := []string{"enter x", "enter y", "exit x", "exit y"}; !slices.Equal(log, want) { + t.Errorf("ran %q, want %q", log, want) + } +} + // The With variants hand the hook the payload of the event that crossed. func TestGroupWithHooksSeeThePayload(t *testing.T) { t.Parallel() diff --git a/rules.go b/rules.go index c2f7ea1..f7e9559 100644 --- a/rules.go +++ b/rules.go @@ -78,10 +78,10 @@ type builder[S comparable] struct { // their group's boundary once the table is finished. groupHooks []groupHookDecl[S] - // Hooks as declared, which buildPlans turns into each row's plan. - onEnter, onExit map[S][]Hook[S] - onEnterVia, onExitVia map[edge[S]][]any // keyed by (state, event); func(context.Context, Transition[S], A) - onAll []Hook[S] + // Hooks as declared, which buildPlans joins into each row's plan. + onEnter, onExit map[S][]call[S] + onEnterVia, onExitVia map[edge[S]][]call[S] // keyed by (state, event) + onAll []call[S] groupEnter, groupExit map[edge[S]][]call[S] // keyed by row, in the order they run internal map[edge[S]]bool // the rows declared with Stay } @@ -127,10 +127,10 @@ func New[S comparable](name string, rules ...Rule[S]) (*Machine[S], error) { actions: make(map[edge[S]]any), plans: make(map[edge[S]]*plan[S]), }, - onEnter: make(map[S][]Hook[S]), - onExit: make(map[S][]Hook[S]), - onEnterVia: make(map[edge[S]][]any), - onExitVia: make(map[edge[S]][]any), + onEnter: make(map[S][]call[S]), + onExit: make(map[S][]call[S]), + onEnterVia: make(map[edge[S]][]call[S]), + onExitVia: make(map[edge[S]][]call[S]), internal: make(map[edge[S]]bool), groupEnter: make(map[edge[S]][]call[S]), groupExit: make(map[edge[S]][]call[S]), @@ -540,7 +540,7 @@ func OnEnter[S comparable](s S, h Hook[S]) Rule[S] { b.errs = append(b.errs, fmt.Errorf("nil OnEnter hook for state %v", s)) return } - b.onEnter[s] = append(b.onEnter[s], h) + b.onEnter[s] = append(b.onEnter[s], call[S]{plain: h}) b.declare(s) }) } @@ -552,7 +552,7 @@ func OnExit[S comparable](s S, h Hook[S]) Rule[S] { b.errs = append(b.errs, fmt.Errorf("nil OnExit hook for state %v", s)) return } - b.onExit[s] = append(b.onExit[s], h) + b.onExit[s] = append(b.onExit[s], call[S]{plain: h}) b.declare(s) }) } @@ -573,7 +573,7 @@ func OnTransition[S comparable](h Hook[S]) Rule[S] { b.errs = append(b.errs, errors.New("nil OnTransition hook")) return } - b.onAll = append(b.onAll, h) + b.onAll = append(b.onAll, call[S]{plain: h}) }) } @@ -636,7 +636,7 @@ func OnEnterVia[S comparable, A any](s S, ev Event[A], h func(context.Context, T return } k := edge[S]{from: s, ev: ev.def} - b.onEnterVia[k] = append(b.onEnterVia[k], h) + b.onEnterVia[k] = append(b.onEnterVia[k], call[S]{typed: h}) b.requireVia("OnEnterVia", "enters", s, ev.def, func(d decl[S]) bool { return d.to == s && !d.internal }) }) } @@ -651,7 +651,7 @@ func OnExitVia[S comparable, A any](s S, ev Event[A], h func(context.Context, Tr return } k := edge[S]{from: s, ev: ev.def} - b.onExitVia[k] = append(b.onExitVia[k], h) + b.onExitVia[k] = append(b.onExitVia[k], call[S]{typed: h}) b.requireVia("OnExitVia", "leaves", s, ev.def, func(d decl[S]) bool { return d.key.from == s && !d.internal }) }) } @@ -727,7 +727,7 @@ func OnExitWith[S comparable, A any](s S, h func(context.Context, Transition[S], // attachWith attaches h to every event on a row that match selects, once the // table is finished, so declaration order does not matter. func (b *builder[S]) attachWith[A any](what, verb string, s S, h func(context.Context, Transition[S], A), - hooks map[edge[S]][]any, match func(decl[S]) bool) { + hooks map[edge[S]][]call[S], match func(decl[S]) bool) { if h == nil { b.errs = append(b.errs, fmt.Errorf("nil %s hook for state %v", what, s)) return @@ -751,7 +751,7 @@ func (b *builder[S]) attachWith[A any](what, verb string, s S, h func(context.Co continue } k := edge[S]{from: s, ev: d.key.ev} - hooks[k] = append(hooks[k], h) + hooks[k] = append(hooks[k], call[S]{typed: h}) } // A broken rule may be what dropped the row; report it alone. if !touched && !b.broken { @@ -769,9 +769,12 @@ func (b *builder[S]) attachWith[A any](what, verb string, s S, h func(context.Co // [OnExitGroup], never dips on an intra-group move. // // When a transition enters several groups at once, the larger group's hooks -// run first, as a superstate's entry runs before its substate's; a group's -// own hooks run in declaration order. Entering the initial state is not a -// transition, so no group hook runs for it. +// run first, as a superstate's entry runs before its substate's. Of groups +// the same size, the one whose hooks were declared first runs first, except +// that of two with the same members the earlier-declared group is the outer +// one, as [Machine.DOT] draws it. A group's own hooks run in declaration +// order. Entering the initial state is not a transition, so no group hook +// runs for it. // // A group entry hook must not fire the machine on the same state: the state // entered has not run its entry hooks yet. [New] rejects one that no @@ -785,7 +788,8 @@ func OnEnterGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { // OnExitGroup declares a hook that runs when the machine leaves g for a // state outside it: just after the exit hooks of the state left, before the // assignment. When a transition leaves several groups, the smaller group's -// hooks run first. See [OnEnterGroup]. +// hooks run first, and of two with the same members the later-declared, +// inner one's; otherwise as [OnEnterGroup]. func OnExitGroup[S comparable](g Group[S], h Hook[S]) Rule[S] { return ruleFunc[S](func(b *builder[S]) { b.declareGroupHook("OnExitGroup", g, false, call[S]{plain: h}, nil, h == nil) @@ -824,30 +828,40 @@ func (b *builder[S]) declareGroupHook(what string, g Group[S], enter bool, h cal }) } -// attachGroupHooks gives every row the group hooks it runs: those of the -// groups it leaves, smallest first, and of the groups it enters, largest -// first, each group's in declaration order. It runs last, against the -// finished table, so group-inherited rows count and declaration order does -// not matter. +// attachGroupHooks gives every row the group hooks it runs, in the order +// they run: entering, larger groups first, and leaving, smaller first. Of +// groups the same size, the one whose hooks were declared first goes first +// either way, unless the two have the same members: such twins nest by +// declaration, as DOT draws them, so leaving runs the later one's first. A +// group's own hooks keep their declaration order. It runs against the +// finished table, so group-inherited rows count. func (b *builder[S]) attachGroupHooks() { if len(b.groupHooks) == 0 { return } - // Groups from outermost to innermost: larger first, and of two the same - // size the one declared first, as DOT nests them. Entering runs their - // hooks in that order and leaving in its reverse, a group's own hooks in - // declaration order either way. - rank := func(g Group[S]) int { - return slices.IndexFunc(b.m.groups, func(h Group[S]) bool { return h.name == g.name }) + rank := make(map[string]int, len(b.m.groups)) + for i, g := range b.m.groups { + rank[g.name] = i } - byNesting := func(outerFirst bool) []groupHookDecl[S] { + type key struct{ size, twins, rank int } + keys := make([]key, len(b.groupHooks)) + var twins []Group[S] // one group per member set, by its first hook + for i, h := range b.groupHooks { + t := slices.IndexFunc(twins, func(g Group[S]) bool { return subset(g.members, h.group) && subset(h.group.members, g) }) + if t < 0 { + twins, t = append(twins, h.group), len(twins) + } + keys[i] = key{size: len(h.group.members), twins: t, rank: rank[h.group.name]} + } + byNesting := func(entering bool) []groupHookDecl[S] { out := slices.Clone(b.groupHooks) slices.SortStableFunc(out, func(x, y groupHookDecl[S]) int { - c := cmp.Or(cmp.Compare(len(y.group.members), len(x.group.members)), cmp.Compare(rank(x.group), rank(y.group))) - if !outerFirst { - c = -c + kx, ky := keys[x.index], keys[y.index] + size, nest := cmp.Compare(ky.size, kx.size), cmp.Compare(kx.rank, ky.rank) + if !entering { + size, nest = -size, -nest } - return c + return cmp.Or(size, cmp.Compare(kx.twins, ky.twins), nest) }) return out } @@ -903,39 +917,39 @@ func (b *builder[S]) attachGroupHooks() { } } -// buildPlans gives every row the hooks it runs, in order: the exit hooks of -// its source, plain, Via and With, then of the groups it leaves; and after -// the assignment the transition hooks, the entry hooks of the groups it -// enters, then of its target, plain, Via and With. An internal row runs -// none. +// buildPlans gives every row the hooks it runs, in order: before the +// assignment, the exit hooks of its source, plain, Via and With, then of the +// groups it leaves; after it, the transition hooks, the entry hooks of the +// groups it enters, then of its target, plain, Via and With. It runs last. +// An internal row, and one no hook concerns, gets no plan. func (b *builder[S]) buildPlans() { - plain := func(hs []Hook[S]) []call[S] { - out := make([]call[S], len(hs)) - for i, h := range hs { - out[i] = call[S]{plain: h} - } - return out - } - typed := func(hs []any) []call[S] { - out := make([]call[S], len(hs)) - for i, h := range hs { - out[i] = call[S]{typed: h} - } - return out - } for _, d := range b.decls { if d.internal { continue } - p := &plan[S]{ - exit: slices.Concat(plain(b.onExit[d.key.from]), typed(b.onExitVia[d.key]), b.groupExit[d.key]), - enter: slices.Concat(plain(b.onAll), b.groupEnter[d.key], - plain(b.onEnter[d.to]), typed(b.onEnterVia[edge[S]{from: d.to, ev: d.key.ev}])), + exit := join(b.onExit[d.key.from], b.onExitVia[d.key], b.groupExit[d.key]) + enter := join(b.onAll, b.groupEnter[d.key], b.onEnter[d.to], b.onEnterVia[edge[S]{from: d.to, ev: d.key.ev}]) + if len(exit) > 0 || len(enter) > 0 { + b.m.plans[d.key] = &plan[S]{exit: exit, enter: enter} } - if len(p.exit) > 0 || len(p.enter) > 0 { - b.m.plans[d.key] = p + } +} + +// join concatenates lists of calls, sharing the one that is not empty when +// only one is: plans are never written once built, so rows with the same +// hooks, every row with only transition hooks say, keep one list. +func join[S comparable](lists ...[]call[S]) []call[S] { + var only []call[S] + n := 0 + for _, l := range lists { + if len(l) > 0 { + only, n = l, n+1 } } + if n <= 1 { + return only + } + return slices.Concat(lists...) } // Group is a named set of states that share transitions. A transition declared