feat: async TUI syncing and 16x vitest speed optimization#65
Conversation
|
Warning Review limit reached
More reviews will be available in 44 minutes and 47 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more credits in the billing tab to continue. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (15)
📒 Files selected for processing (70)
✨ Finishing Touches🧪 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 |
Self-Code Review FindingsAn automated self-review has been conducted on the changes in this branch using the Code Rabbit persona. Severity Breakdown
Evidence Bundle & Detailed Findings
Hey @codex, please confirm if this matches our review guidelines and if there are any other edge cases to consider! |
Update: All Issues ResolvedThe code review finding regarding the premature sync toast expiration has been successfully resolved:
CC @codex |
|
To use Codex here, create an environment for this repo. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 129b0c327b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const expiresAt = Date.now() + 3000; | ||
| const syncMessage = msg.error ? `Sync failed: ${msg.error}` : 'Graph sync complete.'; | ||
| const syncVariant = msg.error ? 'error' : 'success'; | ||
| const showToast = !!(model.toast && model.toast.message.includes('Syncing')); |
There was a problem hiding this comment.
When remote sync takes longer than the fixed 3-second toast lifetime scheduled for remote-change/manual refresh, the dismiss-toast handler clears the "Syncing" toast before this check runs. In that case showToast is false, so both the success toast and a possible Sync failed: ... toast are suppressed even though the background sync just completed; keep the syncing toast alive until sync-complete replaces it.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| if (msg.type === 'sync-complete') { | ||
| if (msg.requestId !== model.requestId) return [model, []]; |
There was a problem hiding this comment.
When a watcher remote-change arrives while a dashboard load is already in progress, the early return only sets refreshPending; snapshot-loaded then bumps requestId for the queued refresh but does not launch a backgroundSync for that new id. The original sync-complete is discarded here as stale, so syncing can stay true and the queued remote update is refreshed without ever pulling coverage; schedule a sync for the pending refresh or don't invalidate the in-flight completion.
Useful? React with 👍 / 👎.
| deps.logger?.debug('dashboard background sync started', { requestId }); | ||
| const graph = await deps.graphPort.getGraph(); | ||
| if (typeof graph.syncCoverage === 'function') { | ||
| await graph.syncCoverage(); |
There was a problem hiding this comment.
When syncCoverage() imports remote patches, the graph's materialized basis can be stale until materialize() runs; this command emits sync-complete immediately and the completion handler refetches through dashboard observer sessions with syncCoverage: false, so the "complete" refresh can still read the old basis or hit stale-basis errors instead of showing the remote changes. Mirror the other post-sync paths by materializing before emitting completion.
Useful? React with 👍 / 👎.
| "@flyingrobots/bijou-node": "^7.1.0", | ||
| "@flyingrobots/bijou-tui": "^7.1.0", | ||
| "@git-stunts/git-warp": "^18.0.0", | ||
| "@git-stunts/plumbing": "^3.0.3", |
There was a problem hiding this comment.
With this bump to @git-stunts/plumbing 3.x, createDefault is async (the same change adds await in WarpGraphAdapter and inspect-graph), but rg still finds many direct script call sites such as scripts/generate-work-dag.ts:105 and scripts/repair-orphans.ts:62 passing the returned Promise into GitGraphAdapter. Those maintenance commands will fail at runtime when they touch the plumbing object; either migrate the remaining call sites or don't bump the dependency yet.
Useful? React with 👍 / 👎.
| // await graph.syncCoverage(); | ||
| // await graph.materialize(); |
There was a problem hiding this comment.
For npm run graph:work, removing both the coverage sync and materialization means the DAG is built from whatever graph state was already local/materialized when the process opened. In repositories where another writer has pushed graph refs, this now emits a stale work DAG even though the script previously synchronized first; keep the sync/materialize step for this offline report path.
Useful? React with 👍 / 👎.
| let memPersistence = WarpGraphAdapter.memoryBackends.get(this.cwd); | ||
| if (!memPersistence) { | ||
| memPersistence = new InMemoryGraphAdapter(); | ||
| WarpGraphAdapter.memoryBackends.set(this.cwd, memPersistence); |
There was a problem hiding this comment.
Under the global XYPH_TEST_IN_MEMORY=true test setup, this static map is keyed only by the cwd string and is never cleared, so a temp repo that is deleted and recreated at the same path (several tests use os.tmpdir() plus Date.now()) reuses the old in-memory graph instead of starting from an empty Git repo. That can leak nodes between tests and make the optimized suite flaky; include a per-repo incarnation key or clear the backend when the repo is recreated.
Useful? React with 👍 / 👎.
| const aiSuggestions: AiSuggestionNode[] = []; | ||
| for (const node of suggestionNodes) { | ||
| if (node.props['type'] !== 'ai_suggestion') continue; | ||
| if (node.props['type'] !== 'ai_suggestion' && node.props['type'] !== 'ai-suggestion') continue; |
There was a problem hiding this comment.
By broadening the read side to include raw type: 'ai-suggestion' nodes, those suggestions can now appear in the dashboard lanes, but the resolution path still rejects anything whose graph property is not exactly ai_suggestion in RecordService.adoptAiSuggestion/dismissAiSuggestion/supersedeAiSuggestion. In graphs containing dashed-type legacy or imported suggestions, the UI can offer adopt/dismiss/supersede actions that always fail as [NOT_FOUND]; normalize these nodes before display or accept the same type spelling in the write path.
Useful? React with 👍 / 👎.
…ution write paths
1 participant
Summary
This PR implements asynchronous remote syncing for the Bijou TUI and optimizes test suite performance.
Key Changes
{ syncOnQuery: false }to prevent reading adapters from blocking the main render thread.syncCoverageruns inDashboardAppfrom the event loop, running syncs asynchronously in a backgroundCmdboundary.Syncing...,Synced successfully) to highlight network updates without spamming the TUI.WarpGraphAdapterto optionally use@git-stunts/git-warp'sInMemoryGraphAdapterwhenprocess.env['XYPH_TEST_IN_MEMORY']is enabled.cwd(repository path) inside the adapter to preserve shared state semantics across derived worldlines and isolated read graphs during tests.test/setup.ts, boosting vitest runtime from ~129s to ~8.13s (16x overall speedup, and 350x speedup for the main integration test).