spec: Reload File Tree action in Command Palette (#10003)#10025
spec: Reload File Tree action in Command Palette (#10003)#10025lonexreb wants to merge 1 commit intowarpdotdev:masterfrom
Conversation
Adds product.md and tech.md for issue warpdotdev#10003: a Command Palette action that forces Wildtree to re-scan disk for the active workspace, giving users a deliberate fallback when automatic refresh misses a change. V1 scope: - New static palette action labeled "Reload File Tree" with aliases "Refresh File Tree", "Reload Wildtree", "Refresh Wildtree" - Dispatches through reload_active_file_tree, which branches on is_git to call either LocalRepoMetadataModel::rebuild_repository (Git case) or the non-Git snapshot-rebuild path (per the issue's explicit call-out that this is needed for non-Git directories) - Non-blocking toast confirms success with duration; failure toast preserves existing tree state - New FileTreeReloadInvoked telemetry event with duration_ms Out of V1 (tracked as follow-ups): - Default keyboard shortcut binding (let usage data drive) - Per-folder reload scope - Visible refresh button on the tree itself (explicitly rejected by reporter) - Investigating the underlying automatic-refresh failure (orthogonal concern) - Reload-on-focus behavior change 8 testable behavior invariants. Implementation surface is small: one static action const, one dispatch arm, one new orchestration function. Reuses existing rebuild paths the automatic refresh already exercises — this is purely a manual trigger surface for them.
|
I'm starting a first review of this spec-only 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 |
There was a problem hiding this comment.
Overview
This spec PR defines a Command Palette action for manually reloading the File Tree, with product behavior, proposed implementation wiring, telemetry, and tests.
Concerns
- The central reload contract is not specified against real force-rebuild APIs; the current model paths described by the spec appear to return early once already indexed rather than forcing a rescan.
- The Command Palette integration plan references static-action and dispatch concepts that do not match the checked-out palette action architecture, so the implementation path is ambiguous.
- The product UX promises a one-frame visible update, which conflicts with the asynchronous/large-project behavior described in the tech spec.
- One telemetry test expectation would be flaky for fast reloads.
Found: 1 critical, 2 important, 1 suggestions
Verdict
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
| 1. User notices the File Tree is out of sync with disk (e.g. after a `rm -rf old-dir/` that doesn't reflect in the tree). | ||
| 2. User opens the Command Palette (`Cmd+P` / `Ctrl+P`). | ||
| 3. User types `reload file tree`, `refresh file tree`, or `reload wildtree`. All three queries match the same action via aliasing in the search index. | ||
| 4. Selecting the action triggers a tree re-scan from disk for the active session's project root. The tree visibly updates within one frame. |
There was a problem hiding this comment.
The tree visibly updates within one frame conflicts with the tech spec’s asynchronous and large-project reload paths. Define the UX as non-blocking while the tree updates after the rebuild completes, and specify whether any in-progress feedback is shown.
|
|
||
| ### 1. New static palette action | ||
|
|
||
| **File:** `app/src/search/command_palette/static_actions.rs` (or wherever existing actions like `Reload Settings` are registered — verify at implementation time). |
There was a problem hiding this comment.
static_actions.rs / PaletteActionId registry, so this plan points implementers at nonexistent integration points. Update this section to name the actual binding/action registration and dispatch path, or explicitly specify a new palette data source.
| } | ||
| ``` | ||
|
|
||
| `rebuild_repository` and `rebuild_from_disk` are conceptual placeholders; the actual function names need to be confirmed at implementation time. The key contract: both paths exist already (they handle the automatic-refresh case); reload simply invokes them on user demand. |
There was a problem hiding this comment.
🚨 [CRITICAL] The spec’s key contract relies on force-rebuild entry points that are still placeholders; in current code, indexed Git and lazy-loaded non-Git paths return early instead of rebuilding. Specify the real force-reindex API/state transition, including how it replaces the old tree atomically, before implementation.
| | 5 (success toast) | unit | dispatch_tests — invoke the action with a healthy workspace, assert `show_toast` is called with a message matching `^File tree reloaded \(\d+ ms\)\.$`. | | ||
| | 6 (failure toast + state preserved) | unit | dispatch_tests — invoke the action with a workspace whose root has been unmounted (mock the entry point to return Err); assert the toast surfaces the error and the in-memory snapshot is unchanged. | | ||
| | 7 (alias resolution) | unit | static_actions_tests — feed each of `refresh file tree`, `reload wildtree`, `refresh wildtree` to the palette search index, assert all three return the same action ID. | | ||
| | 8 (telemetry event) | unit | dispatch_tests — invoke the action, assert `FileTreeReloadInvoked` is emitted with a non-zero `duration_ms`. | |
There was a problem hiding this comment.
💡 [SUGGESTION] as_millis() can be 0 for fast reloads, so asserting a non-zero duration would be flaky. Require that the event is emitted with a bounded non-negative duration, or use a fake clock in the dispatch test.
Adds a product+tech spec for #10003: a Command Palette action that forces Wildtree to re-scan disk for the active workspace, giving users a deliberate fallback when automatic refresh misses a filesystem change.
Files
Total: 282 insertions, 0 deletions. No code changes — this is a spec PR.
Why this is small
The reload paths already exist — `LocalRepoMetadataModel::rebuild_repository` (Git case) and the non-Git snapshot-rebuild path are what the automatic-refresh already invokes when watcher events fire. This spec just adds a manual trigger surface for them: a Command Palette action that calls the existing rebuild via a tiny orchestrator that picks the right path based on `is_git`.
The implementation surface is genuinely small:
V1 scope
Out of V1 (tracked as follow-ups)
Why this spec
Open questions for maintainers
Happy to iterate further.