-
Notifications
You must be signed in to change notification settings - Fork 21
LSP reduce no of candidates #729
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
base: main
Are you sure you want to change the base?
Conversation
PR Reviewer Guide 🔍(Review updated until commit 65a5b6a)Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Latest suggestions up to 65a5b6a
Previous suggestionsSuggestions up to commit 65a5b6a
|
Persistent review updated to latest commit 65a5b6a |
# LSP-specific | ||
N_CANDIDATES_LSP = 3 | ||
N_TESTS_TO_GENERATE_LSP = 1 | ||
TOTAL_LOOPING_TIME_LSP = 5.0 |
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.
we should not decrease this, as this can increase performance noise
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.
we should also enable line profiling but reduce the number of candidates for it
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.
we should also enable line profiling but reduce the number of candidates for it
We never disabled it for VSC, the base profiling and optimization candidate profiling all works same as cli. Yeah we can check on reduction of count though.
|
||
# LSP-specific | ||
N_CANDIDATES_LSP = 3 | ||
N_TESTS_TO_GENERATE_LSP = 1 |
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.
lets keep it 2
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.
lets move the line profiler candidate count to cli
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.
As discussed with @mohammedahmed18 N_candidates are hardcoded in aiservices currently. so need to make it parameterised with max value.
optimization_response_items, llm_cost = await optimize_python_code(
request.user, ctx, data.dependency_code, n=5, python_version=python_version
)
```
def get_n_candidates() -> int: | ||
from codeflash.lsp.helpers import is_LSP_enabled | ||
|
||
return N_CANDIDATES_LSP if is_LSP_enabled() else N_CANDIDATES | ||
|
||
|
||
def get_n_tests_to_generate() -> int: | ||
from codeflash.lsp.helpers import is_LSP_enabled | ||
|
||
return N_TESTS_TO_GENERATE_LSP if is_LSP_enabled() else N_TESTS_TO_GENERATE | ||
|
||
|
||
def get_total_looping_time() -> float: | ||
from codeflash.lsp.helpers import is_LSP_enabled | ||
|
||
return TOTAL_LOOPING_TIME_LSP if is_LSP_enabled() else TOTAL_LOOPING_TIME |
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.
let's inline this to the variable, these don't need to be funds that will be called constantly , codeflash should be able to optimize this away
PR Type
Enhancement
Description
Add LSP-aware dynamic config getters
Reduce LSP candidates, tests, runtime
Replace constants with getters across code
Improve profiling step logging
Diagram Walkthrough
File Walkthrough
config_consts.py
Add LSP-specific constants and dynamic getters
codeflash/code_utils/config_consts.py
git_utils.py
Use dynamic candidate count for worktrees
codeflash/code_utils/git_utils.py
function_optimizer.py
Dynamic tests/candidates/time; better profiling logs
codeflash/optimization/function_optimizer.py
test_runner.py
Test runners use dynamic total runtime
codeflash/verification/test_runner.py