Skip to content

Gate dashboard API refreshes by lifecycle - #224

Merged
TraderSamwise merged 7 commits into
masterfrom
chore/next-20260622-tui-client-lifecycle
Jun 23, 2026
Merged

Gate dashboard API refreshes by lifecycle#224
TraderSamwise merged 7 commits into
masterfrom
chore/next-20260622-tui-client-lifecycle

Conversation

@TraderSamwise

@TraderSamwise TraderSamwise commented Jun 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a dashboard lifecycle task runner for fire-and-forget async UI work
  • route Project, Topology, Library, Coordination, notification hydration, and heartbeat refresh renders through the runner
  • preserve immediate API request start while suppressing stale success/error/finally UI callbacks after navigation or dashboard teardown

Verification

  • yarn vitest run src/multiplexer/dashboard-lifecycle.test.ts src/multiplexer/project.test.ts src/multiplexer/topology.test.ts src/multiplexer/library.test.ts src/multiplexer/coordination.test.ts src/multiplexer/notifications.test.ts src/multiplexer/dashboard-interaction.test.ts
  • yarn vitest run src/multiplexer/runtime-state.test.ts src/multiplexer/dashboard-lifecycle.test.ts src/multiplexer/project.test.ts src/multiplexer/topology.test.ts src/multiplexer/library.test.ts src/multiplexer/coordination.test.ts src/multiplexer/notifications.test.ts src/multiplexer/dashboard-interaction.test.ts
  • yarn typecheck && yarn lint && yarn vitest run && yarn build

Summary by CodeRabbit

  • New Features
    • Added a lifecycle task orchestrator to run dashboard refresh work with token-aware success, error, and finally handlers.
  • Refactor
    • Updated library, coordination notifications, project, topology, and runtime heartbeat/status refresh flows to use lifecycle tasks for consistent, lifecycle-safe rendering.
  • Bug Fixes
    • Prevented success/error handlers from firing after lifecycle tokens become stale.
    • Improved coordination reload after notification mutation success; handled mutation failures without stale UI flashes.
    • Prevented exceptions in success handlers from triggering error paths.
  • Tests
    • Expanded coverage for stale-token routing and manual refresh redraw behavior.

@vercel

vercel Bot commented Jun 22, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
app Ready Ready Preview, Comment Jun 23, 2026 12:32am

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 464103ea-8ca4-412f-8e74-ffc5e63da04d

📥 Commits

Reviewing files that changed from the base of the PR and between b2e35c0 and f92d114.

📒 Files selected for processing (1)
  • src/multiplexer/subscreens.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/multiplexer/subscreens.test.ts

📝 Walkthrough

Walkthrough

Introduces startDashboardLifecycleTask, a new generic async orchestrator in dashboard-lifecycle.ts that captures a lifecycle token and fires onSuccess/onError/onFinally callbacks only while the token remains current. All multiplexer modules (coordination, notifications, library, project, runtime-state, topology) are then migrated from the manual captureDashboardLifecycle/isDashboardLifecycleCurrent promise-chain pattern to this new abstraction, accompanied by supporting tests.

Changes

Dashboard Lifecycle Task Abstraction and Migration

Layer / File(s) Summary
New types and startDashboardLifecycleTask implementation
src/multiplexer/dashboard-lifecycle.ts
Introduces DashboardLifecycleCaptureOptions, DashboardLifecycleTaskHandlers<T>, and the exported startDashboardLifecycleTask<T> function that captures a token, runs work(token) with sync-throw handling, and gates each callback on isDashboardLifecycleCurrent, suppressing trailing chain errors.
startDashboardLifecycleTask tests
src/multiplexer/dashboard-lifecycle.test.ts
Adds a deferred<T>() helper, microtask flush utility, and four Vitest cases verifying callbacks are suppressed on stale tokens (epoch change, mode switch) and fire correctly with token data when the token stays current.
runtime-state heartbeat refresh migration
src/multiplexer/runtime-state.ts, src/multiplexer/runtime-state.test.ts
Replaces the manual lifecycle-capture + isDashboardLifecycleCurrent promise chain in the heartbeat refresh with startDashboardLifecycleTask; coordination re-refresh and conditional render now live in onSuccess. Test verifies immediate render on heartbeat-only flash visibility update.
coordination show and mutation migration
src/multiplexer/coordination.ts, src/multiplexer/subscreens.test.ts
Migrates showCoordination to startDashboardLifecycleTask for refresh+render orchestration. Rewrites applyNotificationMutation to run mutation inside a lifecycle task: reloads coordination on success, sets footer flash and starts nested lifecycle task for reload+re-render on failure. Test verifies stale mutation failures do not trigger side effects.
notifications coordination hydration migration
src/multiplexer/notifications.ts
Replaces coordination screen hydration's manual lifecycle chain with startDashboardLifecycleTask, calling refreshCoordinationFromService as work and renderCurrentDashboardView in onSuccess.
library screen migration and tests
src/multiplexer/library.ts, src/multiplexer/library.test.ts
Converts initial show and force-refresh flows from direct promise chains to startDashboardLifecycleTask with onSuccess callbacks; test verifies UI redraw on input change while screen remains active.
project screen migration and tests
src/multiplexer/project.ts, src/multiplexer/project.test.ts
Migrates showProject and force-refresh paths to startDashboardLifecycleTask with onSuccess render callbacks, removing inline screen-guard checks; test covers redraw on active screen with input change.
topology screen migration and tests
src/multiplexer/topology.ts, src/multiplexer/topology.test.ts
Refactors show and force-refresh to startDashboardLifecycleTask, removing inline screen-guard checks; test verifies redraw after manual refresh with input change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • TraderSamwise/aimux#222: Directly precedes this PR — introduces the captureDashboardLifecycle/isDashboardLifecycleCurrent primitives that the current PR now wraps and removes from call sites.
  • TraderSamwise/aimux#178: Modifies the same coordination.ts/notifications.ts refresh mutation paths that this PR is migrating, touching refreshCoordinationFromService/reloadCoordination wiring.
  • TraderSamwise/aimux#209: Both PRs modify the multiplexer notification/coordination flow — main PR switches to startDashboardLifecycleTask for reload/render, while the retrieved PR pivots notification handling to service-shaped coordination refresh.

Poem

🐇 A lifecycle token, captured with care,
No stale callbacks lurking anywhere.
onSuccess fires only when fresh,
The promise chain exits, no mess.
Each screen now marches in step — hooray!
The rabbit refactored the race away. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.56% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title "Gate dashboard API refreshes by lifecycle" directly and clearly summarizes the main change: introducing lifecycle-based gating for dashboard API refreshes across multiple components.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/next-20260622-tui-client-lifecycle

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

@TraderSamwise

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@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.

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 `@src/multiplexer/dashboard-lifecycle.test.ts`:
- Around line 78-82: The test does not properly flush the complete promise chain
before asserting on onFinally. The single await Promise.resolve() on line 79
only advances one microtask, but since the promise chain includes a .catch()
stage before reaching .finally(), the onFinally assertion on line 82 can pass
before the handler actually executes. Add additional microtask flushing (such as
another await Promise.resolve() call or a helper that flushes the entire chain)
after the pending.promise to ensure the complete chain including the .finally()
handler has executed before making assertions. Apply the same fix to the code
around line 114 where onSuccess and onFinally are both being asserted.

In `@src/multiplexer/dashboard-lifecycle.ts`:
- Around line 65-80: The promise chain is incorrectly routing exceptions thrown
by the onSuccess handler through the onError handler, making successful work
appear as failed. Replace the chained .then().catch() pattern with the
two-argument form of then() to handle success and error paths separately without
catching exceptions from onSuccess. Additionally, wrap the handler invocations
in both the synchronous catch block (around handlers.onError and
handlers.onFinally calls) and the promise chain with try-catch blocks to
suppress any exceptions thrown by the handlers themselves, ensuring they don't
propagate to subsequent catch blocks. Keep the final .catch(() => undefined) to
swallow any remaining handler exceptions.

In `@src/multiplexer/runtime-state.ts`:
- Around line 163-181: Move the rendering logic that checks
`dashboardNeedsRender` outside and before the `onSuccess` callback of the first
`startDashboardLifecycleTask` call (the one invoking
`refreshDashboardModelFromService`). This ensures that heartbeat-only UI updates
computed in the current tick render immediately via
`host.renderCurrentDashboardView()` without being blocked by the potentially
slow or failing service refresh, while the asynchronous coordination refresh and
its associated render remain in the `onSuccess` callback.
🪄 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

Run ID: 61c6513f-f6fc-442c-ae6f-2856e705ac2f

📥 Commits

Reviewing files that changed from the base of the PR and between d1a844e and b367b82.

📒 Files selected for processing (8)
  • src/multiplexer/coordination.ts
  • src/multiplexer/dashboard-lifecycle.test.ts
  • src/multiplexer/dashboard-lifecycle.ts
  • src/multiplexer/library.ts
  • src/multiplexer/notifications.ts
  • src/multiplexer/project.ts
  • src/multiplexer/runtime-state.ts
  • src/multiplexer/topology.ts

Comment thread src/multiplexer/dashboard-lifecycle.test.ts
Comment thread src/multiplexer/dashboard-lifecycle.ts
Comment thread src/multiplexer/runtime-state.ts
@TraderSamwise

Copy link
Copy Markdown
Owner Author

Sub-agent review findings resolved: the inputEpoch over-scoping issue was fixed in c7253b3 with screen-scoped view refreshes and regression tests; the follow-up lifecycle handler exception, heartbeat render delay, and test flush findings were fixed in 9700ddc.

@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.

Actionable comments posted: 1

🤖 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 `@src/multiplexer/subscreens.test.ts`:
- Around line 206-216: The test is missing a precondition assertion to verify
that the mutation request was actually initiated. Add an expect statement
immediately after the handleCoordinationKey call to assert that
postToProjectService was invoked with the expected parameters. This ensures the
test validates the request was started before checking the stale-callback side
effect assertions, preventing false positives if key routing logic changes.
🪄 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

Run ID: 50cf66d1-355f-47bb-8ef1-29aec7a67975

📥 Commits

Reviewing files that changed from the base of the PR and between 9700ddc and b2e35c0.

📒 Files selected for processing (2)
  • src/multiplexer/coordination.ts
  • src/multiplexer/subscreens.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/multiplexer/coordination.ts

Comment thread src/multiplexer/subscreens.test.ts
@TraderSamwise
TraderSamwise merged commit f2b8140 into master Jun 23, 2026
3 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.

1 participant