-
Notifications
You must be signed in to change notification settings - Fork 21
[LSP] stderr verbose logs for the thought process #718
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 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
…flash into lsp/verbose-quiet-logs
codeflash/api/aiservice.py
Outdated
@@ -133,7 +133,7 @@ def optimize_python_code( # noqa: D417 | |||
"repo_name": git_repo_name, | |||
} | |||
|
|||
logger.info("Generating optimized candidates…") | |||
logger.info("!lsp|tags|Generating optimized candidates…") |
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.
Maybe we can just have it broken in json object with "message", component. That would be easy for anyone to use.
or lets just drop tags as its confusing.
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.
@Saga4 the idea of tags is to use the same message in console and lsp,
sending a json, we would re-write all logger lines, so I think this is the alternative option but dropping it will just not give us control over displaying the messages in the extension (the markdown)
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.
Okay, lets then we can remove "tags" as delimeter as thats not helping, pipe is enough I guess.
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.
yeah right, that would be cleaner
codeflash/api/aiservice.py
Outdated
console.rule() | ||
end_time = time.perf_counter() | ||
logger.debug(f"Generating optimizations took {end_time - start_time:.2f} seconds.") | ||
logger.debug(f"!lsp|tags|Generating optimizations took {end_time - start_time:.2f} seconds.") |
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 think the messaging could be "Generating possible optimizations took {end_time - start_time:.2f} seconds."
or "Candidate generation for optimizations took {end_time - start_time:.2f} seconds."
⚡️ Codeflash found optimizations for this PR📄 150% (1.50x) speedup for
|
PR Type
Enhancement, Bug fix
Description
Enable quiet LSP logs by default
Prefix LSP logs via helper wrapper
Adjust logging target to rich logger
Avoid empty refinement log updates
Diagram Walkthrough
File Walkthrough
cli.py
Enable DEBUG logging when LSP is active
codeflash/cli_cmds/cli.py
console.py
Prefix console logs conditionally for LSP
codeflash/cli_cmds/console.py
helpers.py
LSP helpers for env check and log prefixing
codeflash/lsp/helpers.py
server_entry.py
Configure LSP server to emit via rich logger
codeflash/lsp/server_entry.py
function_optimizer.py
Guard refinement logging on non-empty results
codeflash/optimization/function_optimizer.py