Skip to content

feat(yaml): finite-state-machine entities via an fsms: block - #406

Merged
rapids-bot[bot] merged 6 commits into
rapidsai:mainfrom
johanpel:yaml-fsm-2
Jul 20, 2026
Merged

feat(yaml): finite-state-machine entities via an fsms: block#406
rapids-bot[bot] merged 6 commits into
rapidsai:mainfrom
johanpel:yaml-fsm-2

Conversation

@johanpel

Copy link
Copy Markdown
Contributor

Description

Adds finite-state-machine entities to the YAML DSL.

A top-level fsms: block declares an entity whose events represent transitions into its states, with cardinality derived from the topology and the FSM constraint validated at build time.

Each state lists its outgoing transitions under to:, terminating via the reserved exit target.

The instrumentation-build example models a Query FSM and decodes its events through the AnyEvent callback exporter.

Related Issues

Part of #352.

🤖 Generated with Claude Code

johanpel and others added 6 commits July 17, 2026 11:30
Lower a top-level `fsms:` block, keyed by entity name, into an FSM entity:
each state becomes one of the entity's events, its cardinality derived from
the topology, and `FsmEntityBuilder` validates the topology at build time.
The FSM constraint is builder-only; a hand-written one is rejected.

Demonstrate it, and the AnyEvent decoder, in the instrumentation-build
example: a Query FSM entity whose events are its states, printed through a
callback exporter that decodes each event via AnyEvent.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Declare FSM entities entirely under `fsms:` (their own doc/annotations plus
states) instead of an empty `entities:` shell alongside a separate overlay; an
entity declared as both is rejected. A state lists its outgoing transitions in
`to:`, and the reserved target `exit` terminates the FSM, replacing the
per-state `exit:` flag.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the per-constraint flatten helpers and report each constraint result's
error directly; an aggregate `Multiple` already renders its violations as a
bullet list.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
# Conflicts:
#	crates/instrumentation-build/example/src/main.rs
@johanpel
johanpel marked this pull request as ready for review July 20, 2026 09:50

@mbrobbel mbrobbel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice. We should also write some user docs for this stuff.

@johanpel

Copy link
Copy Markdown
Contributor Author

Very nice. We should also write some user docs for this stuff.

Absolutely: #419

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

FSM schema and instrumentation example

Layer / File(s) Summary
YAML FSM model contracts
crates/yaml/Cargo.toml, crates/yaml/src/ast.rs
The YAML AST now deserializes FSM definitions, states, attributes, initial markers, and transitions.
FSM entity lowering and validation
crates/yaml/src/lower.rs, crates/yaml/src/lib.rs
FSM blocks are lowered into validated entities, with collision, topology, and constraint diagnostics.
FSM YAML behavior coverage
crates/yaml/tests/fsm.rs
Tests cover valid event generation, cardinality, duplicate names, invalid topology, and unreachable states.
Instrumented FSM example
crates/instrumentation-build/example/model.yaml, crates/instrumentation-build/example/build.rs, crates/instrumentation-build/example/src/main.rs
The example defines a Query FSM, enables AnyEvent generation, emits query lifecycle events, and prints unified event representations.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • rapidsai/quent#392: Updates the FsmConstraint identifier used by this YAML lowering and validation.
  • rapidsai/quent#399: Refactors the FSM builder and topology validation used by fsm_entity_of.
  • rapidsai/quent#400: Adds the AnyEvent generation option used by the instrumentation example.

Suggested labels: feature request

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding FSM entities via an fsms: block.
Description check ✅ Passed The description explains the feature and related issue, but it omits the template's Testing and Screenshots sections.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
crates/yaml/src/lib.rs-86-95 (1)

86-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Preserve per-violation diagnostics in crates/yaml/src/lib.rs:86-95
RefTargetError, RefTreeError, and FsmError all have Multiple variants, so e.to_string() turns several validation failures into one diagnostic entry here. Recurse through nested errors instead, matching the FSM-specific flattening in lower.rs, so each violation keeps its own location.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/yaml/src/lib.rs` around lines 86 - 95, Update the error handling
around ref_target, ref_tree, and fsm in the report.results destructuring to
recursively flatten Multiple error variants and emit each underlying violation
separately through sink.error. Match the existing FSM-specific flattening
behavior in lower.rs, preserving each diagnostic’s individual location instead
of passing the aggregate error’s to_string() as one entry.
🧹 Nitpick comments (1)
crates/yaml/src/lower.rs (1)

148-176: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

No explicit rejection of a state literally named exit.

target.eq_ignore_ascii_case("exit") treats any transition target spelled "exit" as the reserved terminal marker, but a state key in states: can itself be named "exit" without any check. Such a state can never receive an incoming edge (any to: [exit] elsewhere is always swallowed as the terminal marker), so it will only ever surface as a confusing "unreachable" diagnostic rather than a clear "state name exit is reserved" error.

🛡️ Proposed fix: reject reserved state name explicitly
     for (state_name, state) in &spec.states {
+        if state_name.eq_ignore_ascii_case("exit") {
+            sink.error(
+                &format!("{path}.states.{state_name}"),
+                "state name `exit` is reserved for the exit transition",
+                None,
+            );
+            complete = false;
+            continue;
+        }
         let Some(state_id) = ident(state_name, &path, sink) else {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/yaml/src/lower.rs` around lines 148 - 176, Reject any state key named
"exit" case-insensitively during the state-processing loop before constructing
its StateDecl, report a clear reserved-name validation error through sink, mark
lowering incomplete, and skip that state. Keep the existing target handling in
the transition loop unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Other comments:
In `@crates/yaml/src/lib.rs`:
- Around line 86-95: Update the error handling around ref_target, ref_tree, and
fsm in the report.results destructuring to recursively flatten Multiple error
variants and emit each underlying violation separately through sink.error. Match
the existing FSM-specific flattening behavior in lower.rs, preserving each
diagnostic’s individual location instead of passing the aggregate error’s
to_string() as one entry.

---

Nitpick comments:
In `@crates/yaml/src/lower.rs`:
- Around line 148-176: Reject any state key named "exit" case-insensitively
during the state-processing loop before constructing its StateDecl, report a
clear reserved-name validation error through sink, mark lowering incomplete, and
skip that state. Keep the existing target handling in the transition loop
unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: 24497d1c-79c9-48c2-b86c-5fece033085b

📥 Commits

Reviewing files that changed from the base of the PR and between c0d3781 and bbe1a92.

⛔ Files ignored due to path filters (2)
  • Cargo.lock is excluded by !**/*.lock, !Cargo.lock
  • crates/instrumentation-build/example/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (8)
  • crates/instrumentation-build/example/build.rs
  • crates/instrumentation-build/example/model.yaml
  • crates/instrumentation-build/example/src/main.rs
  • crates/yaml/Cargo.toml
  • crates/yaml/src/ast.rs
  • crates/yaml/src/lib.rs
  • crates/yaml/src/lower.rs
  • crates/yaml/tests/fsm.rs

@johanpel

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 32f072f into rapidsai:main Jul 20, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants