Avoid project skill refresh scans for unrelated repo updates#12040
Conversation
Co-Authored-By: Oz <oz-agent@warp.dev>
Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR carries precise repository metadata deltas through file-tree update events and uses them to avoid refreshing project skills for unrelated incremental changes, while keeping conservative refreshes for opaque updates. The added tests cover unrelated local and remote deltas, skill-file additions, provider-directory updates, and known skill removals.
Concerns
- No blocking correctness, security, or spec-drift concerns found in the annotated diff.
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
| return true; | ||
| } | ||
|
|
||
| let is_local_repo = matches!(repo_id, RepositoryIdentifier::Local(_)); |
There was a problem hiding this comment.
Hmmm why are we filtering by local repo here?
There was a problem hiding this comment.
we currently only support symlinking in the local path. it'll be possible after #12047, i can make a follow-up PR to refactor here once that PR is merged
| { | ||
| return true; | ||
| } | ||
|
|
There was a problem hiding this comment.
Do we have to always query the entire update subtree? Is there any pre-filtering we could apply here that allows us to early return?
There was a problem hiding this comment.
i can't really think of a good pre-filter solution here because skills can be anywhere in the subtree?
2 participants
Description
In the PR https://github.com/warpdotdev/warp/pull/11559/changes, we introduced triggering
refresh_project_skills_for_repo()on everyFileTreeEntryUpdated. This PR attempts to address that by havingFileTreeEntryUpdatedonly triggerrefresh_project_skills_for_repo()if there is a skills relevant update by passing through the exact updates.Tested by locally creating a repository with approximately 90,000 files and on dev, unrelated file edits entered the expensive project-skill refresh path on the main thread. With this change, identical unrelated edits no longer invoke project-skill refresh traversal, while edits to
.agents/skills/repro/SKILL.mdstill refresh correctly.I'm purposefully not entirely moving the full tree traversal off the main thread. This already existed before the PR, the regression is specifically that we're triggering the scan too often on unnecessary file changes. It'd still be good to address the full tree traversal on the main thread separately, specifically for the large repo case.
Testing
Manual regression validation:
On dev: making an unrelated edit captured
refresh_project_skills_for_repoat approximately 1146 ms andfind_symlinked_skill_files_in_treeat approximately 695 ms on the main thread.Positive control: editing
/tmp/warp-skill-refresh-repro-7reije9h/.agents/skills/repro/SKILL.mdin the patched build still triggeredSkillWatcher::refresh_project_skills_for_repo(approximately 1600 ms) throughfind_project_skill_files_in_tree.Regression check: sampling the patched build for 45 seconds during 30 edits to
/tmp/warp-skill-refresh-repro-7reije9h/packages/pkg-000/src/unrelated.txtcaptured zero matches forSkillWatcher,refresh_project_skills_for_repo,find_project_skill_files_in_tree,find_skill_files_in_tree,find_symlinked_skill_files_in_tree,get_repo_contents, orcollect_contents_recursive.I have manually tested my changes locally with a
WarpLocalbuild launched viawrun.Agent Mode
CHANGELOG-BUG-FIX: Fixed project skill refreshes causing UI stalls when unrelated files change in large repositories.