Repository navigation
Internal transitions, group hooks, TryFire, and per-row hook plans - #2
Merged
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
…ernal 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What porting grpcproc's Raft elector onto fsm asked of it (
floatdrop/grpcprocbranchleader-fsm), and two design decisions that port showed to be wrong.Reversed decisions
From(a).On(ev).Stay(). Guards and actions run, the state is not left, no hook runs (OnTransitionincluded).To(a)keeps its meaning, an external self-transition. DESIGN.md said "anActioncovers that case"; it does not, since an action runs only as part of a transition, and the one that keeps the state runs the hooks. Without it, the events a Raft elector sees most (granting and counting votes, a follower's heartbeat) went around the machine. An internal transition is not a way out (Terminalskeeps a state that has only these), triggers no Via/With hook, and DOT draws it as a dashed loop.OnEnterGroup,OnExitGroupand their…Withvariants. They run only on a transition that crosses the group's boundary, so a count kept by them never dips. DESIGN.md held that a flat table had nowhere to record that suppression; every row knows its source and target, so it is worked out per row inNew. The condition the design set — a superstate that genuinely needs its own hook — is a Raft leader's step-down, which must run when leading or handing over is left for a follower, not between the two. Order is a hierarchy's: larger groups entered first and left last; same-size groups by which group's hooks were declared first, except twins (same members), which nest by declaration as DOT draws them.Added
TryFire/TrySend: a refusal (no transition, or a guard that rejects) isfalsewith a nil error and no allocation; a failed action or a state written by a callback is still an error. For a controller that fires on every turn.//comment, rather than a box around the wrong states. Outputs checked against Graphviz; disjoint-group output is byte-identical.init()for a package-level machine whose hooks fire it (otherwise an initialization cycle).Faster
Every hook a row runs is now one plan built by
New— exit hooks and the groups left, then transition hooks, the groups entered and entry hooks — soFiredoes one lookup instead of four. This is the per-edge plan DESIGN.md once named as what group hooks would take. Interleaved against main on an M3 Max:BenchmarkFireWithHooks66.5 → 42 ns a round trip;BenchmarkFireunchanged (~33 ns).Fireitself stays one function (splitting it cost ~6 ns);TryFireshareslookupand carries a transition out throughapply, whichTestTryFireRunsTheHooksFireDoesand the fuzzer keep in step withFire. README's numbers are re-measured on this machine, comparison table included.Review
Two code-review passes; fixed: twin groups' hooks interleaving (first pass), the tie-break then reordering crossing same-size groups (second), a start with only internal transitions reported as having "no outgoing transition", stale comments and docs. Not taken: merging
Fire's two early returns, which measured slower on the hookless path.Testing
Each commit passes the gate in CLAUDE.md: gofmt, vet,
go test -race, golangci-lint,TestFireDoesNotAllocate, 100% coverage,FuzzMachineInvariantsfor 60s. The fuzzer now also generates internal transitions (acceptance at 200 bytes ~87% → ~79%, measured). New behaviour has named tests, mutation-checked; duplicate-transition and new error texts are pinned exactly.🤖 Generated with Claude Code