fix(context_chips): stop GitDiffStats flicker from shell fallback#9244
Conversation
When a per-repo `GitRepoStatusModel` is attached to `CurrentPrompt`, the `GitDiffStats` chip is updated via filesystem-watcher events that include untracked files in the change count. However, every time the prompt context is rebuilt (e.g. after a command completes) `run_chips` was still running the chip's periodic shell generator once. For `GitDiffStats` that generator is `shell_git_line_changes()`, which executes `git diff --shortstat HEAD` and counts only tracked changes. The result briefly overwrote the watcher's all-files count until the watcher emitted its next `MetadataChanged` event, causing the chip to flicker between the tracked-only count and the all-files count when untracked files were present. Fix: when a chip is updated externally, only run an `initial_value_generator` if one is provided (used by `ShellGitBranch` to seed from the prompt context). For chips like `GitDiffStats` that have no contextual initial generator, leave the value to the watcher subscription, which already kicks off an initial metadata refresh on registration. Fixes warpdotdev#9228 Co-Authored-By: Claude <noreply@anthropic.com>
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @zerone0x on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. |
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @moirahuang. Comment Powered by Oz |
vkodithala
left a comment
There was a problem hiding this comment.
Was looking into this earlier - this is the right fix! Verified that this all works locally. Make sure that you sign our CLA and you're good to go!
|
@zerone0x Thanks for opening this PR! When you get a chance, please sign the CLA so we can move forward with merging this PR. |
|
signed! |
There was a problem hiding this comment.
Overview
This PR prevents externally-driven periodic context chips from falling back to their shell generator when a watcher is already responsible for updates. That keeps GitDiffStats on the watcher-provided all-files semantics and preserves ShellGitBranch's prompt-context initial value path.
Concerns
- No blocking concerns found in the changed hunk.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…rpdotdev#9244) ## Summary Fixes warpdotdev#9228. When a per-repo `GitRepoStatusModel` is attached to `CurrentPrompt`, the `GitDiffStats` chip is updated via filesystem-watcher events whose `DiffStateModel::diff_metadata_against_head()` count includes untracked files (it walks `git status --untracked-files=all`). However, every time the prompt context is rebuilt (e.g. when a new block's metadata arrives after a command completes), `run_chips` was still firing the chip's periodic shell-based generator once. For `GitDiffStats` that fallback is `shell_git_line_changes()`, which runs `git diff --shortstat HEAD` and counts only tracked changes. Its result was overwriting the watcher's structured `GitLineChanges` value until the next watcher `MetadataChanged` event restored it — causing the chip to visibly flicker between the tracked-only count and the all-files count whenever untracked files were present. ## Fix In `CurrentPrompt::run_chips`, when a chip is `is_updated_externally`, only run an `initial_value_generator` if one is provided. The shell-based `chip.generator()` is no longer used as a fallback for externally-driven chips: - `ShellGitBranch` continues to seed its initial value from `current_environment.git_branch()` via `initial_value_generator`, then receives updates from the watcher. - `GitDiffStats` has no `initial_value_generator`, so the periodic shell command is now skipped entirely. Initial population is handled by the watcher: `GitRepoStatusModel::new()` kicks off `refresh_metadata` immediately and emits `MetadataChanged` once metadata is computed, which `set_git_repo_status` handles by updating `GitDiffStats` with the structured `GitLineChanges`. Existing chip values survive across `clear_chips()` (which only aborts in-flight generators), so there is no transient empty state on subsequent prompt refreshes. This keeps the diff-count semantics consistent (always all-files) without changing the behavior of remote/non-watched sessions, which still fall back to the shell generator via the periodic timer branch. ## Test plan - [ ] In a repo with both modified tracked files and untracked files, run a command (e.g. `ls`) and observe the `GitDiffStats` chip — it should stay on the all-files count without flickering down to the tracked-only count. - [ ] In a repo without the watcher available (e.g. remote session), confirm the chip still updates periodically via the shell fallback (unchanged code path). - [ ] `ShellGitBranch` continues to update on branch changes via the watcher and shows the correct branch on first paint. CHANGELOG-BUG-FIX: Fix git diff chip flickering between tracked-only and all-files count when untracked files are present --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: vkodithala <varoon@warp.dev>
Summary
Fixes #9228.
When a per-repo
GitRepoStatusModelis attached toCurrentPrompt, theGitDiffStatschip is updated via filesystem-watcher events whoseDiffStateModel::diff_metadata_against_head()count includes untracked files (it walksgit status --untracked-files=all).However, every time the prompt context is rebuilt (e.g. when a new block's metadata arrives after a command completes),
run_chipswas still firing the chip's periodic shell-based generator once. ForGitDiffStatsthat fallback isshell_git_line_changes(), which runsgit diff --shortstat HEADand counts only tracked changes. Its result was overwriting the watcher's structuredGitLineChangesvalue until the next watcherMetadataChangedevent restored it — causing the chip to visibly flicker between the tracked-only count and the all-files count whenever untracked files were present.Fix
In
CurrentPrompt::run_chips, when a chip isis_updated_externally, only run aninitial_value_generatorif one is provided. The shell-basedchip.generator()is no longer used as a fallback for externally-driven chips:ShellGitBranchcontinues to seed its initial value fromcurrent_environment.git_branch()viainitial_value_generator, then receives updates from the watcher.GitDiffStatshas noinitial_value_generator, so the periodic shell command is now skipped entirely. Initial population is handled by the watcher:GitRepoStatusModel::new()kicks offrefresh_metadataimmediately and emitsMetadataChangedonce metadata is computed, whichset_git_repo_statushandles by updatingGitDiffStatswith the structuredGitLineChanges. Existing chip values survive acrossclear_chips()(which only aborts in-flight generators), so there is no transient empty state on subsequent prompt refreshes.This keeps the diff-count semantics consistent (always all-files) without changing the behavior of remote/non-watched sessions, which still fall back to the shell generator via the periodic timer branch.
Test plan
ls) and observe theGitDiffStatschip — it should stay on the all-files count without flickering down to the tracked-only count.ShellGitBranchcontinues to update on branch changes via the watcher and shows the correct branch on first paint.CHANGELOG-BUG-FIX: Fix git diff chip flickering between tracked-only and all-files count when untracked files are present