docs(muda): define accelerator error handling contract - #13
Conversation
📝 WalkthroughWalkthroughThe PR expands the accelerator error ADR with deferred validation, typed failures, batch atomicity, retry behavior, and ChangesAccelerator error model
Workspace dependency overrides
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@ports/muda/docs/adr/0001-observable-key-accelerator-errors.md`:
- Around line 68-74: Clarify the prevalidation behavior for Menu, Submenu, and
GTK initialization so the aggregate failures list contains only
accelerator-related failures representable by AcceleratorBatchApplicationFailed.
Explicitly state that unrelated errors such as NotAChildOfThisMenu and
AlreadyInitialized remain direct Error returns, or describe separate handling if
the implementation provides it; keep single-item and try_build() behavior
unchanged.
- Around line 157-160: Clarify the GTK failure requirement in the ADR so that
recorded children retain their existing tree structure and ordering, while each
child’s accelerator configuration remains editable for correction before retry.
Update the surrounding test requirements to verify both unchanged
structure/order and successful configuration editing without implying child
configuration is immutable.
- Around line 84-96: Clarify the `try_build()` documentation that resolving an
auto-generated `MenuId` via `COUNTER.next()` may consume IDs when validation
rejects an item. State that “side-effect-free” applies only to native backend
state, while monotonic ID allocation remains an expected side effect.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a86aec2-dbbc-4933-b4e6-06f1253d3d9c
📒 Files selected for processing (1)
ports/muda/docs/adr/0001-observable-key-accelerator-errors.md
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
try_build()validation plus per-item and aggregate accelerator failure payloadsWhy
This is a documentation follow-up to #12. The initial ADR preserved builder compatibility but left traceability, batch behavior, and GTK recovery semantics underspecified.
Impact
Documentation and architecture only; this PR does not implement the Rust API yet. It establishes the reviewable contract for a follow-up implementation without changing existing builder signatures.
Validation
devbase: onlyports/muda/docs/adr/0001-observable-key-accelerator-errors.mdchangesgit diff --checkpasses in both worktreesSummary by CodeRabbit
New Features
try_build()validation for menu item builders without native side effects.Documentation