Skip to content

refactor(runtime): encode menu lifetime states in an enum - #499

Merged
eval-exec merged 1 commit into
mainfrom
refactor/menu-lifetime-state
Oct 7, 2026
Merged

eval-exec merged 1 commit into
mainfrom
refactor/menu-lifetime-state

Conversation

@eval-exec

Copy link
Copy Markdown
Owner

Menu lifetime tracking stored the active and latest snapshot tokens independently, allowing contradictory states to be represented. Replace those fields with Unseen, Open(token), and Closed(token) so an open menu always carries its latest accepted token.

Preserve revision ordering, stale-request rejection, hide-before-show tombstones, and queued result delivery. Closed snapshots retain their revision, and only open snapshots can produce a selection result. No dependencies or public API changes.

Validation: all 59 focused menu session tests pass, including three additional tests covering close behavior, newer hide requests, and exactly-once result delivery after rejection. Formatting and diff checks pass. Test cases remain in the related tests/session_test.rs file.

Copilot AI balanced review requested due to automatic review settings October 7, 2026 17:26
@coderabbitai

coderabbitai Bot commented Oct 7, 2026

Copy link
Copy Markdown

Review in Change Stack →

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 02f654df-c7b0-4615-8571-e4b10567d5a0
📥 Commits

Reviewing files that changed from the base of the PR and between 4b227d0 and ee6d8cf.

📒 Files selected for processing (2)
  • crates/neomacs-display-runtime/src/menus/session.rs
  • crates/neomacs-display-runtime/src/menus/session/tests/session_test.rs
 _______________________________
< When in doubt, review it out. >
 -------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@eval-exec
eval-exec merged commit d842a7a into main Oct 7, 2026
26 of 34 checks passed

Copilot AI 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.

🟢 Approval recommended

The enum refactor is a faithful, self-contained translation of the prior two-field logic with no external callers affected and comprehensive test coverage for the lifecycle transitions.

0 open findings

What changed in this PR

This PR refactors the menu lifetime tracking in the display runtime's menu session. Previously, MenuLifetime stored two independent Option<MenuToken> fields (active and latest), which could in principle represent contradictory states. The change replaces them with a single MenuLifetimeState enum (Unseen, Open(token), Closed(token)), making an open menu always carry its latest accepted token and encoding the valid states explicitly.

I verified the translation is semantically faithful:

  • show rejects when the previous (open or closed) token is >= the incoming token (same as the old latest-based guard), otherwise transitions to Open.
  • close/finish move Open -> Closed (equivalent to clearing active while keeping latest).
  • hide acceptance matches the old boolean logic case-by-case: Unseen accepts, Open accepts only an exact token match, Closed accepts previous <= token (the old == plus < branches combine to <=).
  • reject/take_result are unchanged, preserving FIFO result ordering.

The old invariant that active is Some only when it equals latest holds across all mutators, so no reachable behavior is lost. No external callers referenced the removed fields (controller.rs only uses the public methods).

Changes:

  • Introduce MenuLifetimeState enum and replace active/latest fields with a single state field.
  • Rewrite show, close, hide, and finish to operate on the enum while preserving revision ordering, stale-request rejection, and hide-before-show tombstones.
  • Add three focused tests covering close-without-result, newer-hide-after-close, and exactly-once delivery after a rejection.
File Description
crates/​neomacs-display-runtime/​src/​menus/​session.rs Replaces the two-field menu lifetime representation with a MenuLifetimeState enum and rewrites the lifecycle methods equivalently.
crates/​neomacs-display-runtime/​src/​menus/​session/​tests/​session_test.rs Adds three tests covering close behavior, newer hide requests, and exactly-once result delivery after rejection.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

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