Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Nov 6, 2025

PR Type

Bug fix, Enhancement


Description

  • Require tool["codeflash"] in config parsing

  • Add console logger import to module

  • Preserve LSP fallback when config missing


Diagram Walkthrough

flowchart LR
  parser["config_parser.parse_config_file"] --> access["Access tool[\"codeflash\"]"]
  access -- "missing in LSP" --> lspFallback["Return {}, path"]
  access -- "present" --> proceed["Use explicit config"]
  module["Module imports"] --> loggerImport["Add console logger import"]
Loading

File Walkthrough

Relevant files
Bug fix
config_parser.py
Enforce config presence and add logger import                       

codeflash/code_utils/config_parser.py

  • Import logger from codeflash.cli_cmds.console.
  • Access config via tool["codeflash"] instead of get.
  • Maintain LSP mode fallback when config is absent.
+2/-1     

@KRRT7 KRRT7 enabled auto-merge (squash) November 6, 2025 07:19
@github-actions
Copy link

github-actions bot commented Nov 6, 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

Exception Handling

Accessing tool["codeflash"] without a non-LSP fallback will raise KeyError, but the except only catches tomlkit.exceptions.NonExistentKey. This may cause unintended crashes outside LSP; consider catching KeyError as well or validating presence before indexing.

try:
    tool = data["tool"]
    assert isinstance(tool, dict)
    config = tool["codeflash"]
except tomlkit.exceptions.NonExistentKey as e:
    if lsp_mode:
        # don't fail in lsp mode if codeflash config is not found.
        return {}, config_file_path
Unused Import

logger is imported but not used in this snippet. If not used elsewhere in this module after changes, remove it to avoid lint warnings.

from codeflash.lsp.helpers import is_LSP_enabled
from codeflash.cli_cmds.console import logger

@github-actions
Copy link

github-actions bot commented Nov 6, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Avoid KeyError and guide user

Indexing tool["codeflash"] raises a KeyError when the section is missing, causing a
hard failure even in non-LSP mode. Revert to a safe access pattern and explicitly
detect absence to provide a clear, actionable message.

codeflash/code_utils/config_parser.py [104]

-config = tool["codeflash"]
+config = tool.get("codeflash")
+if config is None:
+    raise ValueError(
+        "No [tool.codeflash] config found. Run 'codeflash init' to create one, or add it to your pyproject.toml."
+    )
Suggestion importance[1-10]: 5

__

Why: Switching from tool["codeflash"] to tool.get("codeflash") avoids a potential KeyError and provides a clearer error, but it changes current behavior (the PR intentionally moved from a safe get to strict indexing), so impact is moderate and context-dependent.

Low

@KRRT7
Copy link
Contributor Author

KRRT7 commented Nov 6, 2025

if the exception doesn't happen then it's not caught elsewhere and the user isn't warned

@KRRT7 KRRT7 merged commit dc6c4cd into main Nov 6, 2025
21 of 22 checks passed
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.

3 participants