fix: eliminate timeline disruption when adopting auto-review - #700
Merged
Conversation
Three changes to prevent the multi-second timeline gap when clicking "New Review" to adopt an auto-review: 1. Add `skipFetch` param to `get_branch_timeline` — when true, uses `FetchMode::Never` instead of `FetchMode::Ttl`, avoiding slow git fetch on session completion and auto-review adoption reloads. 2. Optimistically update the local timeline to set `isAuto: false` on the adopted review before the backend reload, so it renders instantly. 3. Filter session completion events: skip reload for adopted auto-review sessions (tracked via `adoptedSessionId`), and skip reload when the completing session belongs to a different branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Make `get_branch_timeline` always use `FetchMode::Never` so it returns instantly from locally-cached refs. The two-stream path (fast partial + slow fetch), the `timeline-partial` event, and the `skipFetch` parameter are all removed. A new `refresh_branch_git_state` Tauri command runs a TTL-gated `git fetch` + ref comparison and emits a `git-state-updated` event that the frontend merges into the existing timeline. This is triggered as a background task after each timeline load. Every `loadTimeline()` call is now fast with no conditional slow path, and git state rows show stale-but-present data instead of flashing away during fetch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
The loadTimeline callback accepts optional parameters but was typed as () => void, causing implicit-any and type-assignment errors. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![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: 955a731479
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+630
to
+632
| if (eventSessionId === sessionMgr.adoptedSessionId) { | ||
| sessionMgr.adoptedSessionId = null; | ||
| return; |
There was a problem hiding this comment.
When a user adopts an auto-review that is still running, the optimistic update in adoptAutoReview only flips isAuto and then reloads before the session has terminal review data. The later terminal session-status-changed event is the code path that invalidates/reloads the timeline so session_status, completion, and final review/comment details are picked up; returning here suppresses that refresh for the adopted session, leaving the adopted review shown as running or with stale details until an unrelated reload occurs.
Useful? React with 👍 / 👎.
matt2e
added a commit
that referenced
this pull request
Aug 5, 2026
Review 0a5af850 on 9ffc931 flagged that the commit before it had spent effort keeping a dead shell script in sync: `BATCH_FAST_SCRIPT`'s inlined `git log` format was rewritten to the `%x1f` delimiter and pinned with a drift test, for a script nothing runs. Note be1f03ab traced when it died. Commit 9318b97 ("fix: eliminate timeline disruption when adopting auto-review", PR #700, May 2026) removed fetching from timeline loading, which retired the two-stream split the family implemented: a fast local-only stream (HEAD, branch, status, commits — no fetch) that emitted a partial timeline with a placeholder git state, front-running a slow stream that blocked on `git fetch`. Once the build never fetches there is no latency to front-run, so that commit deleted every call site, in Rust and in the frontend's `timeline-partial` handling, and left the definitions behind. Nothing has called them in the fifteen months since. So delete the whole family from `git/state.rs`: `FastGitState` and `into_placeholder_git_state`, `compute_fast_local_git_state`, `complete_local_git_state`, `BATCH_FAST_SCRIPT` with `BatchFastOutput` / `parse_batch_fast_output` / `into_fast_git_state` / `compute_fast_git_state_batched`, plus the two-stream scheduling helpers `needs_fetch` (whose doc names the path it chose between) and `local_git_state_cache_key`, which 9318b97 orphaned in the same edit. The `git/mod.rs` re-exports go with them. `BRANCH_COMMIT_LOG_FIELDS` goes too: it existed only so the script could inline the field list that `BRANCH_COMMIT_LOG_FORMAT` passes as an argument, so with the script gone its sole remaining consumer was the test asserting the two match. Keeping it would leave behind a fresh instance of exactly what the review objected to. The producer count for the shared commit-log format drops from four to three — worktree.rs's doc comment naming the script is gone, and the three surviving producers all take the format as an argument. No behaviour change; nothing referenced any of this. Verified with `cargo check --all-targets` (no new warnings), `cargo clippy --all-targets` (only the pre-existing warnings in test_utils.rs, store/tests.rs and the acp_stream_probe example), `cargo fmt --check`, and `cargo test --lib git::` — 82 passed. Signed-off-by: Matt Toohey <contact@matttoohey.com>
matt2e
added a commit
that referenced
this pull request
Aug 5, 2026
Review 0a5af850 on 9ffc931 flagged that the commit before it had spent effort keeping a dead shell script in sync: `BATCH_FAST_SCRIPT`'s inlined `git log` format was rewritten to the `%x1f` delimiter and pinned with a drift test, for a script nothing runs. Note be1f03ab traced when it died. Commit 9318b97 ("fix: eliminate timeline disruption when adopting auto-review", PR #700, May 2026) removed fetching from timeline loading, which retired the two-stream split the family implemented: a fast local-only stream (HEAD, branch, status, commits — no fetch) that emitted a partial timeline with a placeholder git state, front-running a slow stream that blocked on `git fetch`. Once the build never fetches there is no latency to front-run, so that commit deleted every call site, in Rust and in the frontend's `timeline-partial` handling, and left the definitions behind. Nothing has called them in the fifteen months since. So delete the whole family from `git/state.rs`: `FastGitState` and `into_placeholder_git_state`, `compute_fast_local_git_state`, `complete_local_git_state`, `BATCH_FAST_SCRIPT` with `BatchFastOutput` / `parse_batch_fast_output` / `into_fast_git_state` / `compute_fast_git_state_batched`, plus the two-stream scheduling helpers `needs_fetch` (whose doc names the path it chose between) and `local_git_state_cache_key`, which 9318b97 orphaned in the same edit. The `git/mod.rs` re-exports go with them. `BRANCH_COMMIT_LOG_FIELDS` goes too: it existed only so the script could inline the field list that `BRANCH_COMMIT_LOG_FORMAT` passes as an argument, so with the script gone its sole remaining consumer was the test asserting the two match. Keeping it would leave behind a fresh instance of exactly what the review objected to. The producer count for the shared commit-log format drops from four to three — worktree.rs's doc comment naming the script is gone, and the three surviving producers all take the format as an argument. No behaviour change; nothing referenced any of this. Verified with `cargo check --all-targets` (no new warnings), `cargo clippy --all-targets` (only the pre-existing warnings in test_utils.rs, store/tests.rs and the acp_stream_probe example), `cargo fmt --check`, and `cargo test --lib git::` — 82 passed. Signed-off-by: Matt Toohey <contact@matttoohey.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
loadTimelinecallback typing inBranchCardSessionManagerto match the actual signature (accepts optionaltimelineKey/forceparams).Test plan
svelte-checkandcargo clippypass cleanly🤖 Generated with Claude Code