Skip to content

Conversation

mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Sep 5, 2025

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

flowchart LR
  A["LSP env detection"] -- "is_LSP_enabled()" --> B["Console logger wrappers"]
  B -- "lsp_log prefixes :::: when LSP" --> C["Rich logger output"]
  C -- "stderr handler configured" --> D["VS Code output channel"]
  E["Function optimizer"] -- "skip zero-count log" --> F["Cleaner logs"]
Loading

File Walkthrough

Relevant files
Enhancement
cli.py
Enable DEBUG logging when LSP is active                                   

codeflash/cli_cmds/cli.py

  • Treat LSP mode as verbose logging.
  • Use is_LSP_enabled to set DEBUG level.
+1/-1     
console.py
Prefix console logs conditionally for LSP                               

codeflash/cli_cmds/console.py

  • Import lsp_log helper.
  • Wrap logger.info and logger.debug with lsp_log.
  • Preserve original log functions.
+7/-0     
helpers.py
LSP helpers for env check and log prefixing                           

codeflash/lsp/helpers.py

  • Add is_LSP_enabled with caching.
  • Introduce lsp_log wrapper to prefix logs.
+8/-0     
server_entry.py
Configure LSP server to emit via rich logger                         

codeflash/lsp/server_entry.py

  • Target "rich" logger, clear its handlers.
  • Add stderr stream handler at DEBUG level.
  • Remove custom formatter and root-level INFO.
+2/-4     
Bug fix
function_optimizer.py
Guard refinement logging on non-empty results                       

codeflash/optimization/function_optimizer.py

  • Update candidate count and log only if new items.
  • Prevent logging zero refinements.
+6/-4     

Copy link

github-actions bot commented Sep 5, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Type Hint Mismatch

The callable type for actual_log_fn in lsp_log likely doesn't match the actual logging function signatures; using a fixed (str, Any, Any) may be too strict and misleading for *args, **kwargs. Consider loosening the type to Callable[..., None] to avoid type-checker issues.

def lsp_log(msg: str, actual_log_fn: Callable[[str, Any, Any], None], *args: Any, **kwargs: Any) -> None:  # noqa: ANN401
    if is_LSP_enabled():
        actual_log_fn(f"::::{msg}", *args, **kwargs)
    else:
        actual_log_fn(msg, *args, **kwargs)
Logger Monkey-Patching

Overwriting logger.info and logger.debug with lambdas can break introspection and tooling; consider wrapping via a LoggerAdapter or a custom Handler/Filter to prepend LSP prefixes instead of monkey-patching methods.

real_info_log_fn = logger.info
logger.info = lambda msg, *args, **kwargs: lsp_log(msg, real_info_log_fn, *args, **kwargs)

real_debug_log_fn = logger.debug
logger.debug = lambda msg, *args, **kwargs: lsp_log(msg, real_debug_log_fn, *args, **kwargs)
Logger Selection

Switching to logging.getLogger("rich") may omit logs from other modules; confirm all project logs propagate to this logger or set levels/handlers on the root logger as well to avoid missing messages in the VS Code channel.

root_logger = logging.getLogger("rich")
root_logger.handlers.clear()

# Set up stderr handler for VS Code output channel with [LSP-Server] prefix
handler = logging.StreamHandler(sys.stderr)
handler.setLevel(logging.DEBUG)

# Configure root logger
root_logger.addHandler(handler)

# Also configure the pygls logger specifically
pygls_logger = logging.getLogger("pygls")
pygls_logger.setLevel(logging.INFO)

Copy link

github-actions bot commented Sep 5, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure correct root logger setup

Targeting the "rich" logger may miss logs emitted by other modules; also
root_logger.setLevel is removed, risking logs being filtered by a higher default
level. Configure the root logger (getLogger()) and explicitly set its level to DEBUG
to ensure stderr logging works reliably.

codeflash/lsp/server_entry.py [19-28]

-root_logger = logging.getLogger("rich")
+root_logger = logging.getLogger()
 root_logger.handlers.clear()
 
 handler = logging.StreamHandler(sys.stderr)
 handler.setLevel(logging.DEBUG)
 
 root_logger.addHandler(handler)
+root_logger.setLevel(logging.DEBUG)
Suggestion importance[1-10]: 8

__

Why: Accurately notes that switching to "rich" logger and removing setLevel may drop logs; recommending root logger with explicit DEBUG level improves reliability and aligns with typical logging practices.

Medium
Avoid risky logger monkey-patching

Monkey-patching logger.info/debug after obtaining real_*_log_fn can create recursion
if other parts rebind the logger, and it bypasses method attributes (like
.isEnabledFor). Wrap via a LoggerAdapter or helper that calls lsp_log without
reassigning methods directly to avoid hard-to-trace logging issues.

codeflash/cli_cmds/console.py [46-51]

-real_info_log_fn = logger.info
-logger.info = lambda msg, *args, **kwargs: lsp_log(msg, real_info_log_fn, *args, **kwargs)
+class _LspLoggerAdapter(logging.LoggerAdapter):
+    def process(self, msg, kwargs):
+        return (msg, kwargs)
+    def info(self, msg, *args, **kwargs):
+        lsp_log(msg, self.logger.info, *args, **kwargs)
+    def debug(self, msg, *args, **kwargs):
+        lsp_log(msg, self.logger.debug, *args, **kwargs)
 
-real_debug_log_fn = logger.debug
-logger.debug = lambda msg, *args, **kwargs: lsp_log(msg, real_debug_log_fn, *args, **kwargs)
+logger = _LspLoggerAdapter(logger, {})
Suggestion importance[1-10]: 7

__

Why: Valid concern about monkey-patching logger.info/debug; using a LoggerAdapter is safer and maintains logger behavior, though it’s a structural change rather than a critical bug fix.

Medium
General
Prevent unintended LSP-wide verbosity

Enabling debug logs solely based on is_LSP_enabled() can unintentionally force
verbose logging for all LSP sessions. Gate the LSP-driven verbosity behind a
separate env/config or keep LSP default at INFO unless args.verbose is set. This
prevents log noise and potential performance impact in production LSP usage.

codeflash/cli_cmds/cli.py [114-116]

-if args.verbose or is_LSP_enabled():
+if args.verbose or (is_LSP_enabled() and os.getenv("CODEFLASH_LSP_VERBOSE", "false").lower() == "true"):
     logging_config.set_level(logging.DEBUG, echo_setting=not is_init)
Suggestion importance[1-10]: 6

__

Why: Correctly identifies that is_LSP_enabled() forces DEBUG in LSP mode and proposes a reasonable env-gated fix, but it introduces a new env without PR context and requires adding os import, so impact is moderate.

Low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant