-
Couldn't load subscription status.
- Fork 22
[FEAT][LSP] Worktrees #649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
PR Reviewer Guide 🔍(Review updated until commit 22b5065)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 22b5065
Previous suggestionsSuggestions up to commit b955ad8
|
…etached-worktrees`)
The optimization replaces expensive Path object construction with simple string formatting. The key change is on line 11:
**Original**: `candidate_path = str(Path(current_path.name) / last_added)`
**Optimized**: `candidate_path = f"{current_path.name}/{last_added}"`
This eliminates two major performance bottlenecks:
1. **Path object creation** - Creating a new Path object for each iteration is expensive
2. **Path division operator** - The `/` operator on Path objects involves internal path resolution logic
The f-string formatting is dramatically faster since it's just string concatenation, while Path operations involve filesystem-aware logic even when not needed.
The line profiler shows the critical loop line dropped from 56% of total runtime (42.1ms) to just 5.2% (1.8ms) - a **23x improvement** on the bottleneck operation.
This optimization is particularly effective for:
- **Deeply nested paths** (100+ levels): 394% speedup
- **Wide directory structures**: 151% speedup
- **Absolute paths**: 108-122% speedup
The performance gains scale with path depth since the optimization is applied once per directory level in the traversal loop. Simple paths see modest gains (1-4%), but complex nested structures see dramatic improvements due to eliminating repeated expensive Path operations.
…/detached-worktrees`)
The optimized code achieves a **35% speedup** through several key performance improvements:
**Primary optimizations:**
1. **Eliminated redundant function calls**: The original code called `is_LSP_enabled()` multiple times - once in the conditional expression and again in the fork check. The optimized version caches this result in a variable, reducing function call overhead.
2. **Improved shell config parsing**: Replaced `matches[-1] if matches else None` with `next(reversed(matches), None)`, which is more efficient for getting the last element without creating an intermediate list slice.
3. **Streamlined environment variable access**: Changed `os.getenv("CODEFLASH_LSP", default="false")` to `os.environ.get("CODEFLASH_LSP", "false")`, eliminating the keyword argument overhead of `default=`.
4. **Reduced file I/O operations**: Removed the intermediate `shell_contents` variable assignment, processing the file content directly in the regex operation.
5. **Optimized control flow**: Restructured the conditional logic to avoid redundant `is_LSP_enabled()` calls and simplified the fork detection path.
**Performance characteristics by test type:**
- **Basic environment variable tests**: 4-6% improvement due to reduced function call overhead
- **Missing API key scenarios**: Up to 503% faster due to streamlined error path logic
- **Shell config file operations**: Modest improvements (1-8%) from optimized file processing
- **LSP mode operations**: 5-9% faster due to cached LSP state checks
The optimizations are particularly effective for error cases and LSP mode scenarios where multiple conditional checks were previously duplicated.
⚡️ Codeflash found optimizations for this PR📄 36% (0.36x) speedup for
|
…eflash into feat/detached-worktrees
15809fc to
e33ff21
Compare
|
Persistent review updated to latest commit 22b5065 |
…t/detached-worktrees`)
The optimized code achieves a **81x speedup** by eliminating expensive Path object operations in the main loop. The key optimization replaces the original approach that repeatedly calls `current_path.parent` and constructs `Path(current_path.name) / last_added` objects with a more efficient strategy using `source_code_path.parts`.
**Key Changes:**
1. **Pre-compute path parts**: Instead of traversing up the directory tree with `.parent` calls, the code extracts all path components once using `source_code_path.parts`
2. **Replace Path construction with string formatting**: The expensive `str(Path(current_path.name) / last_added)` operation (96% of original runtime) is replaced with simple string formatting `f"{parts[i]}/{last_added}"`
3. **Eliminate parent traversal loop**: The `while current_path != current_path.parent` loop is replaced with a `for` loop that iterates through pre-computed parts
**Why This Is Faster:**
- Path object construction and filesystem-style operations are expensive in Python
- String concatenation with f-strings is much faster than Path operations
- Array indexing (`parts[i]`) is faster than repeated method calls (`.parent`)
- The optimization eliminates the bottleneck line that consumed 96% of the original runtime
**Performance Characteristics:**
The optimization shows excellent scaling - deeper directory structures see greater speedups (up to 104x faster for 1000-level nesting). All test cases benefit significantly, with the smallest improvement being 30% for simple cases and massive gains of 8000%+ for deeply nested paths.
| current_path = source_code_path.parent | ||
|
|
||
| last_added = source_code_path.name | ||
| while current_path != current_path.parent: | ||
| candidate_path = str(Path(current_path.name) / candidates[-1]) | ||
| candidates.append(candidate_path) | ||
| candidate_path = str(Path(current_path.name) / last_added) | ||
| candidates.add(candidate_path) | ||
| last_added = candidate_path | ||
| current_path = current_path.parent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⚡️Codeflash found 8,048% (80.48x) speedup for generate_candidates in codeflash/code_utils/coverage_utils.py
⏱️ Runtime : 162 milliseconds → 1.99 milliseconds (best of 365 runs)
📝 Explanation and details
The optimized code achieves a 81x speedup by eliminating expensive Path object operations in the main loop. The key optimization replaces the original approach that repeatedly calls current_path.parent and constructs Path(current_path.name) / last_added objects with a more efficient strategy using source_code_path.parts.
Key Changes:
- Pre-compute path parts: Instead of traversing up the directory tree with
.parentcalls, the code extracts all path components once usingsource_code_path.parts - Replace Path construction with string formatting: The expensive
str(Path(current_path.name) / last_added)operation (96% of original runtime) is replaced with simple string formattingf"{parts[i]}/{last_added}" - Eliminate parent traversal loop: The
while current_path != current_path.parentloop is replaced with aforloop that iterates through pre-computed parts
Why This Is Faster:
- Path object construction and filesystem-style operations are expensive in Python
- String concatenation with f-strings is much faster than Path operations
- Array indexing (
parts[i]) is faster than repeated method calls (.parent) - The optimization eliminates the bottleneck line that consumed 96% of the original runtime
Performance Characteristics:
The optimization shows excellent scaling - deeper directory structures see greater speedups (up to 104x faster for 1000-level nesting). All test cases benefit significantly, with the smallest improvement being 30% for simple cases and massive gains of 8000%+ for deeply nested paths.
✅ Correctness verification report:
| Test | Status |
|---|---|
| ⚙️ Existing Unit Tests | ✅ 40 Passed |
| 🌀 Generated Regression Tests | ✅ 38 Passed |
| ⏪ Replay Tests | 🔘 None Found |
| 🔎 Concolic Coverage Tests | ✅ 1 Passed |
| 📊 Tests Coverage | 100.0% |
⚙️ Existing Unit Tests and Runtime
| Test File::Test Function | Original ⏱️ | Optimized ⏱️ | Speedup |
|---|---|---|---|
test_code_utils.py::test_generate_candidates |
69.6μs | 7.84μs | 788%✅ |
🌀 Generated Regression Tests and Runtime
from __future__ import annotations
from pathlib import Path
# imports
import pytest # used for our unit tests
from codeflash.code_utils.coverage_utils import generate_candidates
# unit tests
# ------------- Basic Test Cases -------------
def test_basic_single_file():
# Basic case: file in root directory
path = Path("foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 7.91μs -> 5.19μs (52.3% faster)
def test_basic_nested_file():
# File in a nested directory
path = Path("src/app/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 25.5μs -> 5.87μs (334% faster)
def test_basic_deep_nested_file():
# File in a deeper nested directory
path = Path("a/b/c/d/e/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 46.4μs -> 6.39μs (626% faster)
# Should include all intermediate candidates
expected = {
"foo.py",
"e/foo.py",
"d/e/foo.py",
"c/d/e/foo.py",
"b/c/d/e/foo.py",
"a/b/c/d/e/foo.py"
}
# ------------- Edge Test Cases -------------
def test_edge_empty_path():
# Edge: empty path
path = Path("")
codeflash_output = generate_candidates(path); result = codeflash_output # 7.03μs -> 4.93μs (42.7% faster)
def test_edge_root_path():
# Edge: root path ("/foo.py" on Unix)
path = Path("/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 7.00μs -> 5.35μs (30.9% faster)
def test_edge_single_directory():
# Edge: file directly in a directory
path = Path("dir/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 17.2μs -> 5.06μs (241% faster)
def test_edge_dot_in_path():
# Edge: directories or files with dots in their names
path = Path("a.b/c.d/foo.e.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 25.5μs -> 5.80μs (340% faster)
def test_edge_trailing_slash():
# Edge: path with trailing slash
path = Path("src/app/foo.py/")
codeflash_output = generate_candidates(path); result = codeflash_output # 24.9μs -> 5.58μs (346% faster)
def test_edge_windows_path():
# Edge: Windows-style path
path = Path("src\\app\\foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 7.51μs -> 4.90μs (53.4% faster)
def test_edge_non_ascii_characters():
# Edge: non-ASCII characters in path
path = Path("src/üñîçødë/文件.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 26.1μs -> 6.15μs (324% faster)
def test_edge_path_with_spaces():
# Edge: path with spaces
path = Path("my project/code file.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 16.8μs -> 5.13μs (227% faster)
def test_edge_path_with_dot_and_slash():
# Edge: path with leading "./"
path = Path("./foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 7.34μs -> 4.83μs (52.1% faster)
def test_edge_path_with_parent_references():
# Edge: path with parent directory references
path = Path("a/b/../c/foo.py").resolve()
codeflash_output = generate_candidates(path); result = codeflash_output # 61.5μs -> 5.59μs (1001% faster)
# ------------- Large Scale Test Cases -------------
def test_large_deeply_nested_path():
# Large: deeply nested path (depth 20)
parts = [f"dir{i}" for i in range(20)]
path = Path(*parts, "foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 168μs -> 9.58μs (1662% faster)
# Check that each candidate is present
last = "foo.py"
for i in range(1, 21):
last = f"dir{20-i}/{last}"
def test_large_many_candidates():
# Large: wide directory tree (simulate many similar files)
# This test only checks one file but ensures no performance issue
path = Path("/".join(f"dir{i}" for i in range(50)) + "/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 494μs -> 15.0μs (3197% faster)
def test_large_unique_candidates():
# Large: ensure all candidates are unique
path = Path("a/b/c/d/e/f/g/h/i/j/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 80.9μs -> 7.11μs (1037% faster)
def test_large_path_performance():
# Large: path with many directories, performance check
path = Path("/".join(f"level{i}" for i in range(100)) + "/file.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 1.32ms -> 30.7μs (4198% faster)
def test_large_various_path_types():
# Large: mix of absolute and relative, dots, and unicode
path = Path("/abs/üñîçødë/./rel/../file.py").resolve()
codeflash_output = generate_candidates(path); result = codeflash_output # 23.8μs -> 4.38μs (445% faster)
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.
#------------------------------------------------
from __future__ import annotations
from pathlib import Path
# imports
import pytest # used for our unit tests
from codeflash.code_utils.coverage_utils import generate_candidates
# unit tests
# ---------------------- BASIC TEST CASES ----------------------
def test_single_file_in_root():
# File in the root directory
path = Path("foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 7.68μs -> 5.17μs (48.6% faster)
def test_file_in_one_subdirectory():
# File in a single subdirectory
path = Path("src/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 16.9μs -> 5.14μs (228% faster)
def test_file_in_two_subdirectories():
# File in two nested subdirectories
path = Path("src/bar/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 24.5μs -> 5.68μs (331% faster)
def test_file_with_dot_in_name():
# File with dots in the name
path = Path("src/foo.bar.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 16.8μs -> 5.00μs (237% faster)
def test_file_with_multiple_extensions():
# File with multiple extensions
path = Path("src/foo.test.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 16.5μs -> 4.99μs (230% faster)
# ---------------------- EDGE TEST CASES ----------------------
def test_file_at_filesystem_root():
# File at the filesystem root (Unix style)
path = Path("/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 6.79μs -> 5.13μs (32.4% faster)
def test_file_in_deeply_nested_structure():
# Deeply nested structure
path = Path("a/b/c/d/e/f/g/h/i/j/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 82.7μs -> 7.27μs (1037% faster)
expected = {
"foo.py",
"j/foo.py",
"i/j/foo.py",
"h/i/j/foo.py",
"g/h/i/j/foo.py",
"f/g/h/i/j/foo.py",
"e/f/g/h/i/j/foo.py",
"d/e/f/g/h/i/j/foo.py",
"c/d/e/f/g/h/i/j/foo.py",
"b/c/d/e/f/g/h/i/j/foo.py",
"a/b/c/d/e/f/g/h/i/j/foo.py",
}
def test_file_with_spaces():
# File with spaces in the name and directories
path = Path("my folder/another folder/my file.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 25.1μs -> 5.71μs (340% faster)
def test_file_with_unicode_characters():
# File with unicode characters
path = Path("src/测试.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 17.2μs -> 5.10μs (237% faster)
def test_file_with_leading_dot():
# Hidden file (leading dot)
path = Path(".hidden/foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 16.2μs -> 4.99μs (224% faster)
def test_file_with_empty_path():
# Empty path should raise an error
path = Path("")
codeflash_output = generate_candidates(path); result = codeflash_output # 7.08μs -> 4.72μs (50.1% faster)
def test_file_with_trailing_slash():
# Path with trailing slash (should treat as directory, not file)
path = Path("src/foo.py/")
codeflash_output = generate_candidates(path); result = codeflash_output # 16.7μs -> 5.05μs (231% faster)
def test_file_with_multiple_separators():
# Path with redundant separators
path = Path("src//bar///foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 24.7μs -> 5.74μs (330% faster)
def test_file_with_parent_directory_reference():
# Path with parent directory references
path = Path("src/bar/../foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 32.7μs -> 5.78μs (466% faster)
# Pathlib resolves "bar/../foo.py" to "src/foo.py"
resolved_path = path.resolve().relative_to(Path.cwd())
codeflash_output = generate_candidates(resolved_path); result = codeflash_output # 13.9μs -> 4.07μs (243% faster)
# ---------------------- LARGE SCALE TEST CASES ----------------------
def test_large_number_of_nested_directories():
# Test with 1000 nested directories (limited to 1000 as per instruction)
dirs = [f"dir{i}" for i in range(1000)]
path = Path("/".join(dirs)) / "foo.py"
codeflash_output = generate_candidates(path); result = codeflash_output # 80.6ms -> 991μs (8027% faster)
# Should contain 1001 candidates: "foo.py", "dir999/foo.py", ..., "dir0/dir1/.../dir999/foo.py"
expected = set()
last = "foo.py"
for i in reversed(range(1000)):
last = f"dir{i}/{last}"
expected.add(last)
expected.add("foo.py")
expected.add(str(path))
def test_performance_with_wide_directory_names():
# Test with 1000-character directory names
dirname = "a" * 1000
path = Path(dirname) / "foo.py"
codeflash_output = generate_candidates(path); result = codeflash_output # 18.2μs -> 6.26μs (191% faster)
def test_performance_with_long_file_name():
# Test with a very long file name (255 chars, common max for filesystems)
filename = "f" * 255 + ".py"
path = Path("src") / filename
codeflash_output = generate_candidates(path); result = codeflash_output # 16.6μs -> 5.29μs (213% faster)
def test_performance_with_many_candidates():
# Test with 1000 directories, ensure set size is correct
dirs = [f"d{i}" for i in range(1000)]
path = Path(*dirs, "foo.py")
codeflash_output = generate_candidates(path); result = codeflash_output # 78.5ms -> 748μs (10395% faster)
def test_path_with_non_ascii_and_long_names():
# Test with a mix of unicode and long names
dirname = "测试" * 200 # 600 characters
filename = "文件" * 50 + ".py" # 102 characters
path = Path(dirname) / filename
codeflash_output = generate_candidates(path); result = codeflash_output # 17.9μs -> 6.23μs (188% faster)
# codeflash_output is used to check that the output of the original code is the same as that of the optimized code.
#------------------------------------------------
from codeflash.code_utils.coverage_utils import generate_candidates
from pathlib import Path
def test_generate_candidates():
generate_candidates(Path())🔎 Concolic Coverage Tests and Runtime
| Test File::Test Function | Original ⏱️ | Optimized ⏱️ | Speedup |
|---|---|---|---|
codeflash_concolic_9d591alw/tmpkrdmrlwv/test_concolic_coverage.py::test_generate_candidates |
7.26μs | 4.90μs | 48.3%✅ |
To test or edit this optimization locally git merge codeflash/optimize-pr649-2025-08-14T13.45.27
| current_path = source_code_path.parent | |
| last_added = source_code_path.name | |
| while current_path != current_path.parent: | |
| candidate_path = str(Path(current_path.name) / candidates[-1]) | |
| candidates.append(candidate_path) | |
| candidate_path = str(Path(current_path.name) / last_added) | |
| candidates.add(candidate_path) | |
| last_added = candidate_path | |
| current_path = current_path.parent | |
| last_added = source_code_path.name | |
| parts = source_code_path.parts | |
| for i in range(len(parts) - 2, 0, -1): | |
| candidate_path = f"{parts[i]}/{last_added}" | |
| candidates.add(candidate_path) | |
| last_added = candidate_path |
codeflash/code_utils/git_utils.py
Outdated
| result = subprocess.run( | ||
| ["git", "worktree", "add", "-d", str(worktree_dir)], | ||
| cwd=git_root, | ||
| check=True, | ||
| stdout=subprocess.DEVNULL if is_LSP_enabled() else None, | ||
| stderr=subprocess.DEVNULL if is_LSP_enabled() else None, | ||
| ) | ||
| if result.returncode != 0: | ||
| logger.error(f"Failed to create worktree: {result.stderr}") | ||
| return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we do this via the git module we're already using?
codeflash/code_utils/git_utils.py
Outdated
| with tempfile.NamedTemporaryFile(mode="w+", suffix=".codeflash.patch", delete=False) as tmp_patch_file: | ||
| tmp_patch_file.write(uni_diff_text + "\n") # the new line here is a must otherwise the last hunk won't be valid | ||
| tmp_patch_file.flush() | ||
|
|
||
| patch_path = Path(tmp_patch_file.name).resolve() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we're using a namedtempfile, we should use w mode directly since the file wouldn't exist and thus not truncated
codeflash/models/models.py
Outdated
| def set_explanation(self, new_explanation: str) -> None: | ||
| object.__setattr__(self, "explanation", new_explanation) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prefer setattr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually removed the frozen attribute from OptimizedCandidate, so there is no need for this hacky way of mutating the explanation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving, we should aim to have more test coverage for the LSP once we have it a bit more polished
|
@KRRT7 okay let me just fix this mypy issue |
User description
--worktree in non-lsp mode is experimental
PR Type
Enhancement, Tests
Description
Add
--worktreeflag to CLI for detached worktreeImplement detached git worktree creation and cleanup
Generate patches from worktree for optimizations
Change coverage candidate generator to use unique set
Diagram Walkthrough
File Walkthrough
2 files
add --worktree CLI optionremove hardcoded sentry DSN6 files
switch generate_candidates to return setadd worktree snapshot and diff helpersintegrate worktree diff in LSP optimizeannotate optimizer and set worktree argadjust PR logic for worktree modeimplement worktree_mode and patch handling1 files
update test_generate_candidates to expect setPR Type
Enhancement, Tests
Description
Add
--worktreeflag for detached worktreeImplement detached worktree creation & cleanup
Generate patches from worktree for optimizations
Use unique set in coverage candidate generator
Diagram Walkthrough
File Walkthrough
14 files
Import `is_LSP_enabled` from `lsp.helpers`Use `is_LSP_enabled()` for version checksMake coverage candidates a unique setRefactor LSP detection in `env_utils`Use `is_LSP_enabled()` in `format_code`Implement detached worktree utility functionsIntegrate worktree into LSP optimization flowAdd `is_LSP_enabled()` helperEnable worktree mode in LSP server initAdd delimiter in LSP log formatterAllow updating `OptimizedCandidate` explanationPreserve candidate explanation after reviewAdd `worktree_mode` to `Optimizer` classImport `is_LSP_enabled` from LSP helpers2 files
Add `--worktree` CLI flagSilence console in LSP mode1 files
Remove console silencing in LSP init1 files
Restore console output in `main` entry1 files
Update test for `generate_candidates` set output