Do not eagerly expand subtrees on lazy loaded repo update#12211
Conversation
|
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 |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![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 changes lazy-loaded repository updates so newly created directories are inserted as unloaded placeholders instead of eagerly materializing their subtrees, and updates tests for lazy vs eager roots.
Concerns
- The new lazy directory branch skips the standing-query and directory-symlink handling that the existing subtree builder performed, so project rules/skills created under a new lazy directory can remain undiscovered until the directory is expanded.
- This is a user-facing file-tree behavior change, but the PR description does not include screenshots or a screen recording demonstrating the lazy directory creation/expansion flow end to end. Please attach visual evidence from a local desktop run or a coding agent with computer use enabled.
Security
No security-specific findings from the supplemental pass.
Spec validation
No approved or repository spec context was provided for this PR.
Verdict
Found: 0 critical, 2 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
kevinyang372
force-pushed
the
kevin/register-non-recursive-watcher-for-linux
branch
from
909261e to
cbd84ec
Compare
kevinyang372
force-pushed
the
kevin/lazy-root-placeholder-on-create
branch
2 times, most recently
from
88a3753 to
2134af0
Compare
| // Only a non-recursive root (lazy non-git roots on | ||
| // Linux) needs to watch newly added directories: nothing | ||
| // under such a root is covered by a recursive watch, so | ||
| // each new dir needs its own watch. Collect them before | ||
| // the mutations are consumed. | ||
| // | ||
| // Recursive roots don't need this: non-ignored new dirs | ||
| // are auto-followed by the recursive backend, and | ||
| // gitignored new dirs are added as lazy placeholders that | ||
| // get watched only when the user expands them (see | ||
| // `load_directory`). | ||
| let new_dirs: Vec<StandardizedPath> = if matches!( | ||
| model.repo_watches.get(&repo_path).map(|w| w.root_mode), |
There was a problem hiding this comment.
What was this trying to do before? Like what necessitated it
There was a problem hiding this comment.
Not really anything necessitated it -- the agent added it and I realized the whole architecture of updating the repo metadata tree for lazy loaded repos is borked
kevinyang372
force-pushed
the
kevin/lazy-root-placeholder-on-create
branch
from
2134af0 to
63532cd
Compare
kevinyang372
force-pushed
the
kevin/register-non-recursive-watcher-for-linux
branch
from
49fb6f8 to
b614b4e
Compare
For lazy (non-git) roots, the file watcher eagerly materialized the full subtree of any newly-created directory, even though lazy roots only materialize what the user has expanded. On Linux (non-recursive watching) this also left the loaded descendants unwatched (only the subtree root got a watch), so changes inside them were missed. Build added directories as unloaded placeholders for lazy roots instead (depth 0), still collecting standing-query results across their descendants so skills/rules are discovered. The directory is materialized and watched on demand when the user expands it via load_directory. This removes the now-unnecessary watch-on-create logic.
kevinyang372
force-pushed
the
kevin/lazy-root-placeholder-on-create
branch
from
fba4d15 to
a04c130
Compare
2 participants
Description
For lazy-loaded (non-git) roots, the file watcher previously eagerly materialized the full subtree of any newly-created directory, even though lazy roots only materialize what the user has actually expanded. This PR inserts a newly-created directory as an unloaded placeholder instead; its subtree is built on demand when the user expands it via
load_directory.Concretely, in
compute_file_tree_mutations, whenlazy_loadis set, a created directory now emits anAddEmptyDirectoryplaceholder mutation and skips the eagerbuild_treetraversal entirely. Eager (git/recursive) roots are unchanged and still build the full subtree.This also removes the now-unnecessary watch-on-create (
new_dirs) logic: because lazy roots no longer eagerly load descendants, there are no loaded-but-unwatched directories. On Linux (non-recursive watching) this closes a gap where a bulk-created nested directory tree could leave descendant directories loaded but unwatched — they are now placeholders, watched only when expanded (which already registers a watch viawatch_subdir).Net effect in
local_model.rsis a reduction in code.Linked Issue
ready-to-specorready-to-implement.Testing
This is internal repo-metadata logic with no UI surface, covered by unit tests:
Added
lazy_root_created_directory_inserted_as_placeholder: asserts a lazy root inserts the created directory as an unloaded placeholder with its subtree not materialized, while an eager root fully materializes it.Existing
compute_file_tree_mutationscall sites updated for the newlazy_loadargument.cargo nextest run -p repo_metadata --features local_fspasses (100 tests), withcargo clippyand./script/formatclean.I have manually tested my changes locally with
./script/runScreenshots / Videos
N/A — no user-visible/UI change.
Agent Mode
Warp conversation: https://staging.warp.dev/conversation/652230d5-e5fb-41bf-af9d-b478310176cf
CHANGELOG-NONE