Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented Nov 15, 2025

PR Type

Bug fix


Description

  • Skip formatting when no formatter provided

  • Normalize excessive blank lines in that case

  • Preserve existing temp-file formatting flow


Diagram Walkthrough

flowchart LR
  A["format_generated_code()"] -- "no formatter cmds" --> B["return with blank-line normalization"]
  A -- "formatter cmds present" --> C["write temp file and run formatter"]
Loading

File Walkthrough

Relevant files
Bug fix
formatter.py
Add guard to skip formatting when formatter missing           

codeflash/code_utils/formatter.py

  • Detect missing formatter via formatter_cmds check.
  • Bypass formatting when none provided.
  • Normalize multiple newlines to two in bypass path.
+3/-0     

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Codeflash Bot seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Possible Issue

Using only formatter_cmds[0] to decide "disabled" ignores cases where the first arg is a flag (e.g., "python", "-m", "black") or path-based invocation; detection may misclassify and skip formatting.

formatter_name = formatter_cmds[0].lower() if formatter_cmds else "disabled"
if formatter_name == "disabled":  # nothing to do if no formatter provided
    return re.sub(r"\n{2,}", "\n\n", generated_test_source)
Behavior Change

Normalizing 2+ consecutive newlines even when no formatter is provided alters output; confirm this side-effect is intended and safe for generated code consumers.

if formatter_name == "disabled":  # nothing to do if no formatter provided
    return re.sub(r"\n{2,}", "\n\n", generated_test_source)

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Robustly determine formatter presence

Avoid accessing the first item without verifying the list is non-empty and that the
first token isn't a flag. Safely parse formatter_cmds for an actual executable name
before deciding to disable formatting.

codeflash/code_utils/formatter.py [100-102]

-formatter_name = formatter_cmds[0].lower() if formatter_cmds else "disabled"
+formatter_name = "disabled"
+if formatter_cmds:
+    # pick first non-flag token as the formatter name
+    for tok in formatter_cmds:
+        if not tok.startswith("-"):
+            formatter_name = tok.lower()
+            break
 if formatter_name == "disabled":  # nothing to do if no formatter provided
     return re.sub(r"\n{2,}", "\n\n", generated_test_source)
Suggestion importance[1-10]: 6

__

Why: It's a reasonable robustness improvement to skip flag-like tokens when inferring formatter_name, avoiding false "disabled" when the first arg is a flag. Impact is moderate and the improved_code matches the existing_code context.

Low
General
Preserve intentional double newlines

The newline normalization regex can collapse necessary spacing at file start or end.
Constrain it to only collapse runs of 3+ newlines to 2 to preserve intentional
double breaks.

codeflash/code_utils/formatter.py [101-102]

 if formatter_name == "disabled":  # nothing to do if no formatter provided
-    return re.sub(r"\n{2,}", "\n\n", generated_test_source)
+    return re.sub(r"\n{3,}", "\n\n", generated_test_source)
Suggestion importance[1-10]: 5

__

Why: Changing the regex to collapse only 3+ newlines better preserves deliberate double breaks, with low risk. It's a minor behavioral tweak but contextually accurate and clearly mapped to the existing lines.

Low

@aseembits93
Copy link
Contributor Author

fixes

Screenshot 2025-11-14 at 17 11 03

@aseembits93 aseembits93 merged commit ad9666c into main Nov 15, 2025
20 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.

4 participants