[codex] skip unnecessary status untracked scans#423
Merged
Conversation
Signed-off-by: Eli Ma <eli@patch.sh>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes libra status worktree collection to avoid paying for untracked discovery when the user requests --untracked-files=no, and to avoid descending into fully-untracked top-level directories in normal mode when they have no tracked descendants. It introduces a dedicated status worktree collector and adds regression tests covering unreadable untracked artifact directories.
Changes:
- Add
status_untrackedworktree collector to skip untracked discovery forstatus -unoand to short-circuit top-level untracked directory traversal in normal mode. - Wire
statusto use the new collector and adjust index handling for the new flow. - Add Unix-only integration tests ensuring unreadable untracked directories don’t break
status -unoand that normal mode reports?? <dir>/without descending.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/command/status.rs |
Switches status data collection to use the new status-specific worktree collector. |
src/command/status_untracked.rs |
New module implementing tracked-worktree change detection plus optional untracked/ignored scanning with top-level untracked dir short-circuiting. |
src/command/mod.rs |
Registers the new status_untracked module. |
tests/command/status_test.rs |
Adds regressions for status -uno skipping unreadable untracked dirs and normal-mode top-level dir reporting. |
Comment on lines
418
to
422
| let needs_index = matches!(args.untracked_files, UntrackedFiles::Normal) | ||
| || matches!(args.porcelain, Some(PorcelainVersion::V2)); | ||
| let mut maybe_index = if needs_index { | ||
| Some(load_status_index()?) | ||
| Some(worktree.index) | ||
| } else { |
Comment on lines
+140
to
+147
| for entry in std::fs::read_dir(&dir).map_err(|source| StatusError::ListWorkdirFiles { | ||
| path: workdir.to_path_buf(), | ||
| source, | ||
| })? { | ||
| let entry = entry.map_err(|source| StatusError::ListWorkdirFiles { | ||
| path: workdir.to_path_buf(), | ||
| source, | ||
| })?; |
Comment on lines
+154
to
+166
| let file_type = entry | ||
| .file_type() | ||
| .map_err(|source| StatusError::ListWorkdirFiles { | ||
| path: workdir.to_path_buf(), | ||
| source, | ||
| })?; | ||
| let relative = path | ||
| .strip_prefix(workdir) | ||
| .map_err(|err| StatusError::ListWorkdirFiles { | ||
| path: workdir.to_path_buf(), | ||
| source: std::io::Error::other(err.to_string()), | ||
| })? | ||
| .to_path_buf(); |
Comment on lines
+47
to
+51
| unstaged.new = if matches!(untracked_mode, UntrackedFiles::Normal) && include_ignored { | ||
| collapse_untracked_directories(scan.untracked, &index) | ||
| } else { | ||
| scan.untracked | ||
| }; |
Comment on lines
+174
to
+178
| if matches!(untracked_mode, UntrackedFiles::Normal) | ||
| && !include_ignored | ||
| && is_top_level_path(&relative) | ||
| && !has_tracked_descendant(&relative, tracked_files) | ||
| { |
Signed-off-by: Eli Ma <eli@patch.sh>
Contributor
Author
|
Addressed the review feedback in 72de4ab:
Local verification:
GitHub CodeQL checks are passing; the Check, Build and Test workflow is still queued, not failing. |
Comment on lines
+413
to
+417
| let ignored_files = worktree | ||
| .ignored_files | ||
| .into_iter() | ||
| .map(util::workdir_to_current) | ||
| .collect(); |
Comment on lines
+169
to
+172
| pending_dirs.push(path); | ||
| } else if file_type.is_file() { | ||
| scan_file(&mut scan, workdir, index, &path, &relative, include_ignored)?; | ||
| } |
genedna
added a commit
that referenced
this pull request
Jul 5, 2026
The no-descend untracked-directory optimization (#423) emitted the '?? dir/' marker for any top-level untracked directory, deviating from git: a directory whose entire contents are skip-listed (.libra/.git) or ignored holds no visible untracked file and must stay invisible. Test harnesses' isolated '.libra-test-home/' (containing only a nested .libra) became visible and broke four clean-tree assertions across the suite. The marker is now gated on a lazy first-hit probe with the same skip rules; unreadable directories stay reported (cannot verify emptiness — matches the #423 permission tests). Regression test pins both directions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Eli Ma <eli@patch.sh>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
status -uno.Why
statusshould not pay for untracked discovery when the user explicitly hides untracked files, and normal mode can safely report a completely untracked top-level directory without recursively scanning build artifacts. This mirrors the performance concern addressed for pull/restore while preservingstatus -uallfull discovery behavior.Validation
cargo +nightly fmt --all --checkLIBRA_SKIP_WEB_BUILD=1 cargo clippy --all-targets --all-features -- -D warningsLIBRA_SKIP_WEB_BUILD=1 cargo test --test command_test untracked_directory -- --nocaptureLIBRA_SKIP_WEB_BUILD=1 cargo test --test command_test test_status_with_subdirectories -- --nocaptureLIBRA_SKIP_WEB_BUILD=1 cargo test --test command_test status_test -- --nocapture