Skip to content

fix(spec): correct lifecycle diagram, defaults, and document readData timeout#44

Merged
jgeudens merged 3 commits intomasterfrom
claude/verify-adapter-statemachine-qnmCY
Apr 25, 2026
Merged

fix(spec): correct lifecycle diagram, defaults, and document readData timeout#44
jgeudens merged 3 commits intomasterfrom
claude/verify-adapter-statemachine-qnmCY

Conversation

@jgeudens
Copy link
Copy Markdown
Member

@jgeudens jgeudens commented Apr 25, 2026

  • Add missing adapter.describe step to Session Lifecycle diagram and Example
  • Fix int32LittleEndian default: spec said false, code (and describe defaults) use true
  • Fix persistent default: spec said false, code (and describe defaults) use true
  • Document the adapter.readData read-timeout error case (-32000)

Summary by CodeRabbit

Release Notes

  • Documentation
    • Updated adapter protocol documentation with revised connection and device configuration defaults
    • Expanded error documentation for timeout and overlapping read scenarios
    • Added comprehensive sequence diagram documenting message exchange patterns and error handling flows
    • Extended session lifecycle documentation with additional protocol examples

… timeout

- Add missing adapter.describe step to Session Lifecycle diagram and Example
- Fix int32LittleEndian default: spec said false, code (and describe defaults) use true
- Fix persistent default: spec said false, code (and describe defaults) use true
- Document the adapter.readData read-timeout error case (-32000)
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

Walkthrough

Documentation updates to the adapter JSON-RPC specification clarifying defaults, error codes, and session lifecycle. Introduces new sequence diagram documentation detailing message exchange flows, error paths, and state management between adapter client and Modbus adapter process.

Changes

Cohort / File(s) Summary
Adapter Specification
adapters/json-rpc-spec.md, docs/technical/adapter-protocol-spec.md
Updated default values for connection persistent (false → true) and device int32LittleEndian (false → true); expanded error documentation for -32000 code to cover overlapping reads and timeout scenarios; extended session lifecycle and example transcript to include adapter.describe call with renumbered subsequent request/response IDs.
Sequence Diagram Documentation
docs/adapter-sequence-diagram.md
New comprehensive documentation covering JSON-RPC 2.0 message exchange over Content-Length-framed stdio, including normal session flow, adapter notifications, error paths (handshake timeout, unexpected exit, lifecycle errors, read timeout, premature stopSession), and state machine summary.

Possibly related PRs

  • Improve diagnostics + adapter handling #39 — Modifies adapter description and specification defaults for connections and device fields, aligning with this PR's default value updates.
  • Update doc #41 — Updates adapter JSON-RPC protocol documentation and adapter.describe structure specification, complementing this PR's session lifecycle and sequence diagram additions.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: correcting the lifecycle diagram, fixing default values, and documenting the readData timeout error case.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/verify-adapter-statemachine-qnmCY

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (3)
adapters/json-rpc-spec.md (1)

510-511: Consider combining the two -32000 error scenarios.

Similar to the generic spec, both bullet points document the same error code. Consider combining them for clarity to match the Error Codes table entry at line 567.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@adapters/json-rpc-spec.md` around lines 510 - 511, Combine the two identical
`-32000` bullets into a single bullet that covers both "Read already in
progress" (when a previous readData has not completed) and "Read timed out"
(Modbus I/O exceeded timeout); update the combined bullet to mention both
conditions and ensure it matches the Error Codes table entry for `-32000` so the
spec is consistent.
docs/adapter-sequence-diagram.md (1)

9-9: Add language specifiers to fenced code blocks.

The fenced code blocks containing ASCII art sequence diagrams should specify a language for better markdown rendering. Add text after the opening triple backticks:

```text
(sequence diagram content)
```

This addresses the markdownlint warnings (MD040) and improves rendering consistency across different markdown viewers.

Also applies to: 144-144, 164-164, 174-174, 183-183, 197-197, 214-214

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/adapter-sequence-diagram.md` at line 9, Several fenced code blocks
containing ASCII sequence diagrams are missing a language specifier; update each
opening triple-backtick that starts an ASCII diagram block to use ```text (e.g.,
replace ``` with ```text) so markdownlint MD040 is satisfied and rendering is
consistent; search for the fenced blocks that contain the ASCII diagrams (the
blocks currently starting with ```) and add the language token `text` to each
occurrence.
docs/technical/adapter-protocol-spec.md (1)

374-376: Consider combining the two -32000 error scenarios into a single bullet point.

Both bullet points document the same error code with different scenarios. For clarity, consider merging them into one entry, e.g.:

  • -32000 — Read already in progress (a previous readData has not yet completed) or read timed out (data-source I/O did not complete within the timeout window)

This matches the combined description in the Error Codes table at line 431.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/technical/adapter-protocol-spec.md` around lines 374 - 376, Merge the
two separate bullets for error code `-32000` into a single bullet that covers
both scenarios; update the entry that currently reads two lines (one for "Read
already in progress" and one for "Read timed out") to a single line such as
"`-32000` — Read already in progress (a previous `readData` has not yet
completed) or read timed out (data-source I/O did not complete within the
timeout window)" so it matches the Error Codes table and references the
`readData` operation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@adapters/json-rpc-spec.md`:
- Around line 510-511: Combine the two identical `-32000` bullets into a single
bullet that covers both "Read already in progress" (when a previous readData has
not completed) and "Read timed out" (Modbus I/O exceeded timeout); update the
combined bullet to mention both conditions and ensure it matches the Error Codes
table entry for `-32000` so the spec is consistent.

In `@docs/adapter-sequence-diagram.md`:
- Line 9: Several fenced code blocks containing ASCII sequence diagrams are
missing a language specifier; update each opening triple-backtick that starts an
ASCII diagram block to use ```text (e.g., replace ``` with ```text) so
markdownlint MD040 is satisfied and rendering is consistent; search for the
fenced blocks that contain the ASCII diagrams (the blocks currently starting
with ```) and add the language token `text` to each occurrence.

In `@docs/technical/adapter-protocol-spec.md`:
- Around line 374-376: Merge the two separate bullets for error code `-32000`
into a single bullet that covers both scenarios; update the entry that currently
reads two lines (one for "Read already in progress" and one for "Read timed
out") to a single line such as "`-32000` — Read already in progress (a previous
`readData` has not yet completed) or read timed out (data-source I/O did not
complete within the timeout window)" so it matches the Error Codes table and
references the `readData` operation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 73083d88-dabd-4831-a55c-2ada266d56f9

📥 Commits

Reviewing files that changed from the base of the PR and between 3e11645 and 047ad67.

📒 Files selected for processing (3)
  • adapters/json-rpc-spec.md
  • docs/adapter-sequence-diagram.md
  • docs/technical/adapter-protocol-spec.md

@jgeudens jgeudens merged commit 54134fd into master Apr 25, 2026
10 checks passed
@jgeudens jgeudens deleted the claude/verify-adapter-statemachine-qnmCY branch April 25, 2026 18:37
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.

1 participant