Fix git diff chip refresh flicker with untracked files#9302
Fix git diff chip refresh flicker with untracked files#9302solomonneas wants to merge 4 commits intowarpdotdev:masterfrom
Conversation
|
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 @solomonneas 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. Once you have done so, please comment |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
@solomonneas Once you feel this PR is ready for review, you're welcome to take it out of draft and we'll do a review. |
|
My bad, I didn’t realize I’d left this as a draft. It should be good to go now. |
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I requested changes on this pull request and posted feedback. Comment I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I completed the review and posted feedback on this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR changes the current prompt git chip refresh path so externally driven git diff stats use GitRepoStatusModel metadata instead of falling back to shell git diff output, and adds tests for avoiding the tracked-only shell fallback and clearing stale timers.
Concerns
- The new repo-status update path can reuse metadata from the previous repository during navigation before async repo detection replaces
git_repo_status, which can leaveGitDiffStatsshowing stale counts for the wrong repo.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| chip_kind: &ContextChipKind, | ||
| ctx: &AppContext, | ||
| ) { | ||
| if let Some(metadata) = self.latest_git_status_metadata(ctx) { |
There was a problem hiding this comment.
git_repo_status handle: BlockMetadataReceived updates the prompt context before async repo detection swaps the repo-status model, so navigating between repos can populate GitDiffStats from the previous repo until the new metadata arrives. Guard that the status model’s repo_path() still matches the active working directory before applying its metadata, or clear the chip while waiting.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR moves externally driven git prompt chips toward using GitRepoStatusModel metadata and cancels stale shell refreshes to avoid the tracked-only diff fallback flicker.
Concerns
- The new repo-status freshness check compares the shell-reported cwd directly against the native/canonical repo path. On Windows Git Bash/MSYS-style sessions, those path formats can differ for the same directory, causing valid metadata to be treated as stale and the git chips to be cleared.
- Supplemental security pass found no security-specific issues in the changed code.
Verdict
Found: 0 critical, 1 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| let status_model = h.as_ref(ctx); | ||
| let active_working_directory = std::path::Path::new(active_working_directory); | ||
|
|
||
| if !active_working_directory.starts_with(status_model.repo_path()) { |
There was a problem hiding this comment.
active_working_directory is the shell-native cwd, while repo_path() is the canonical/native repo path. For Windows Git Bash/MSYS2 this can be /c/Users/... vs C:\\Users\\..., so this guard treats valid repo metadata as stale and clears the git chips. Convert through the active session's launch data before starts_with, like terminal repo detection does.
The freshness check in latest_git_status_metadata compared the raw shell-reported cwd against the OS-native repo_path. On MSYS2 / Git Bash the shell reports paths like /c/Users/foo/proj while DetectedRepositories stores C:\Users\foo\proj, so starts_with fails and the git chips get cleared even though they are valid for the current repo. Run the cwd through ShellLaunchData::maybe_convert_absolute_path first (handles MSYS2 and WSL via existing helpers, identity for native shells) and fall back to the raw string when no conversion is available. Extract working_directory_belongs_to_repo so the resolution logic is covered by direct unit tests, including the MSYS2-style regression where the converted path is the only one that matches repo_path.
|
Pushed 56137b8 — the cwd is now run through /oz-review |
There was a problem hiding this comment.
Overview
This PR updates the current prompt git chips to use GitRepoStatusModel metadata when an externally driven repo-status model is attached, clears stale shell refresh timers, and adds regression coverage for repo-status-driven diff stats and working-directory mismatch handling.
Concerns
- No blocking correctness or security concerns found in the changed diff lines.
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
…epo-status # Conflicts: # app/src/context_chips/current_prompt.rs
Description
Fixes #9228.
The git diff context chip can be updated from
GitRepoStatusModel, which includes untracked files, but prompt refreshes could still fall back to the shell generator. That shell fallback runsgit diff --shortstat HEAD, which only reports tracked-file changes. When a repo had untracked files, the chip could briefly flip between the structured repo-status count and the tracked-only shell count.This change keeps externally driven git chips on repo-status metadata when available, clears stale git chip timers when repo status attaches, and still allows initial-value generators for externally driven chips that need them.
Testing
cargo fmt --checkgit diff --check./script/run-clang-format.py -r --extensions 'c,h,cpp,m' ./crates/warpui/src/ ./app/src/find . -name '*.wgsl' -exec wgslfmt --check {} +cargo clippy --workspace --exclude warp_completer --all-targets --tests -- -D warningscargo clippy -p warp_completer --all-targets --tests -- -D warningscargo test -p warp context_chips::current_prompt::tests::test_externally_driven_git_diff_stats_uses_repo_status_without_shell_fallback --features local_fscargo test -p warp context_chips::current_prompt::tests::test_git_diff_stats_clears_shell_timer_when_repo_status_attaches_late --features local_fscargo test -p warp context_chips::current_prompt::tests::test_git_status_change_updates_chip_value --features local_fscargo test -p warp context_chips::current_prompt::tests::test_externally_driven_chip_skips_periodic_timer --features local_fscargo nextest run -p warp_completer --features v2cargo test --docAttempted full workspace nextest in a clean headless LXC with
CARGO_BUILD_JOBS=1 cargo nextest run --no-fail-fast --workspace --exclude command-signatures-v2; it compiled and ran but failed environment-dependent UI/font tests unrelated to this change.Server API dependencies
None.
Agent Mode
Changelog Entries for Stable
CHANGELOG-BUG-FIX: Fixed the git diff prompt chip flickering between tracked-only and all-file counts when untracked files are present.