Skip to content

Conversation

@aseembits93
Copy link
Contributor

@aseembits93 aseembits93 commented Nov 10, 2025

PR Type

Enhancement, Bug fix


Description

  • Add formatter for generated tests

  • Apply formatting before embedding in markdown

  • Minor typo fix in comment


Diagram Walkthrough

flowchart LR
  A["Generated test source"] -- "format_generated_code" --> B["Condense blank lines"]
  B -- "embed in PR comment" --> C["Markdown code blocks"]
  D["formatter.py"] -- "typo fix" --> E["Comment correction"]
Loading

File Walkthrough

Relevant files
Enhancement
formatter.py
Introduce helper to format generated code                               

codeflash/code_utils/formatter.py

  • Add format_generated_code to condense blank lines.
  • Use regex to normalize multiple newlines.
  • Fix typo in a comment ("dont'" to "don't").
+16/-1   
function_optimizer.py
Apply formatting to generated test snippets                           

codeflash/optimization/function_optimizer.py

  • Import and use format_generated_code.
  • Format generated and concolic tests before markdown embedding.
+5/-3     

@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

github-actions bot commented Nov 10, 2025

PR Reviewer Guide 🔍

(Review updated until commit 65cba54)

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

API Consistency

The new function format_generated_code ignores any configured formatter and only condenses blank lines. If the project expects formatting (e.g., isort/black) similar to format_code, confirm this simplified behavior is intentional and sufficient for all generated tests.

def format_generated_code(generated_test_source: str) -> str:
    return re.sub(r"\n{2,}", "\n\n", generated_test_source)
    # formatter_name = formatter_cmds[0].lower() if formatter_cmds else "disabled"
    # if formatter_name == "disabled":
    #     return re.sub(r"\n{2,}", "\n\n", generated_test_source)
    # # try running formatter, if nothing changes (could be due to formatting failing or no actual formatting needed)
    # original_temp, test_dir_str, exit_on_failure = None, None, True
    # formatted_temp, formatted_code, changed = apply_formatter_cmds(
    #     formatter_cmds, original_temp, test_dir_str, print_status=False, exit_on_failure=exit_on_failure
    # )
    # if not changed:
    #     return re.sub(r"\n{2,}", "\n\n", formatted_code)
    # return formatted_code
Double Newlines

When building generated_tests_str, an extra "\n\n" is appended after already closing a code block that itself ends with a newline, which may create excessive blank space in the PR comment. Verify rendering doesn’t add unintended spacing.

generated_tests_str = ""
for test in generated_tests.generated_tests:
    formatted_generated_test = format_generated_code(test.generated_original_test_source)
    generated_tests_str += f"```python\n{formatted_generated_test}\n```"
    generated_tests_str += "\n\n"
Import Usage

format_generated_code is imported and used, but ensure sort_imports remains necessary in this module; otherwise consider removing it from the import list to avoid unused import warnings.

from codeflash.code_utils.formatter import format_code, format_generated_code, sort_imports

@github-actions
Copy link

github-actions bot commented Nov 10, 2025

PR Code Suggestions ✨

Latest suggestions up to 65cba54
Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Safer newline normalization

The current regex collapses any 2+ consecutive newlines globally, which can alter
intentional spacing (e.g., in multiline strings) and it doesn't trim
leading/trailing excess. Limit normalization to top-level structure and also strip
surrounding blank lines to avoid wrapping artifacts.

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

 def format_generated_code(generated_test_source: str) -> str:
-    return re.sub(r"\n{2,}", "\n\n", generated_test_source)
+    # Normalize only sequences of 3+ blank lines to 2, and trim surrounding whitespace
+    text = re.sub(r"(?:\r?\n){3,}", "\n\n", generated_test_source)
+    return text.strip() + "\n"
Suggestion importance[1-10]: 6

__

Why: The change reduces the chance of modifying intentional spacing and adds sane trimming, which is a reasonable minor robustness improvement; it's consistent with the PR intent to format generated code.

Low
Normalize code block spacing

Ensure consistent trailing newline handling to prevent accidental code block
concatenation or extra blank blocks. Append a single newline after the closing fence
and avoid double-adding blank lines.

codeflash/optimization/function_optimizer.py [1416-1418]

 formatted_generated_test = format_generated_code(test.generated_original_test_source)
-generated_tests_str += f"```python\n{formatted_generated_test}\n```"
-generated_tests_str += "\n\n"
+generated_tests_str += f"```python\n{formatted_generated_test}\n```\n"
Suggestion importance[1-10]: 5

__

Why: Adjusting to a single trailing newline after the code fence avoids double-blank spacing; impact is modest but correct given the existing two-step newline additions.

Low
Consistent concolic block spacing

Treat concolic blocks consistently with other test blocks to avoid mixed spacing and
duplicate newlines. Use a single trailing newline after the fence, relying on the
formatter to ensure final newline.

codeflash/optimization/function_optimizer.py [1420-1422]

 if concolic_test_str:
     formatted_generated_test = format_generated_code(concolic_test_str)
-    generated_tests_str += f"```python\n{formatted_generated_test}\n```\n\n"
+    generated_tests_str += f"```python\n{formatted_generated_test}\n```\n"
Suggestion importance[1-10]: 5

__

Why: Aligns concolic block spacing with the proposed single-newline convention, preventing mixed spacing; a small but coherent formatting improvement.

Low

Previous suggestions

Suggestions up to commit 60bb705
CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent index error and ensure fallback

Guard against empty formatter_cmds to avoid IndexError when accessing
formatter_cmds[0]. Also ensure formatted_code is always defined, returning the
original source when formatting fails or produces no output.

codeflash/code_utils/formatter.py [99-110]

 def format_generated_code(generated_test_source: str, formatter_cmds: Union[list[str], None] = None) -> str:
-    formatter_name = formatter_cmds[0].lower() if formatter_cmds else "disabled"
+    if not formatter_cmds:
+        return re.sub(r"\n{2,}", "\n\n", generated_test_source)
+    formatter_name = (formatter_cmds[0] or "").lower()
     if formatter_name == "disabled":
         return re.sub(r"\n{2,}", "\n\n", generated_test_source)
     # try running formatter, if nothing changes (could be due to formatting failing or no actual formatting needed)
     original_temp, test_dir_str, exit_on_failure = None, None, True
     formatted_temp, formatted_code, changed = apply_formatter_cmds(
         formatter_cmds, original_temp, test_dir_str, print_status=False, exit_on_failure=exit_on_failure
     )
-    if not changed:
-        return re.sub(r"\n{2,}", "\n\n", formatted_code)
+    if not changed or not formatted_code:
+        return re.sub(r"\n{2,}", "\n\n", generated_test_source)
     return formatted_code
Suggestion importance[1-10]: 7

__

Why: Guarding against empty formatter_cmds prevents a potential IndexError, and falling back to generated_test_source if formatting yields no output is a reasonable safety. However, the current code already safely handles empty formatter_cmds by setting formatter_name to "disabled", so the impact is moderate.

Medium
Preserve original when unchanged

Avoid normalizing line breaks on the formatter's output when no change was detected,
since it can still alter content and contradict the "no change" condition. Instead,
return the original unformatted source when no changes are reported.

codeflash/code_utils/formatter.py [108-110]

 if not changed:
-    return re.sub(r"\n{2,}", "\n\n", formatted_code)
+    return re.sub(r"\n{2,}", "\n\n", generated_test_source)
 return formatted_code
Suggestion importance[1-10]: 6

__

Why: Returning generated_test_source when changed is false avoids altering content via normalization and better matches the intent of "no change". It's a small but sensible correctness/readability tweak, though not critical.

Low

@aseembits93 aseembits93 marked this pull request as ready for review November 12, 2025 01:15
@github-actions
Copy link

Persistent review updated to latest commit 65cba54

@aseembits93 aseembits93 requested review from KRRT7, Saga4 and mohammedahmed18 and removed request for KRRT7 November 12, 2025 01:26
formatter_cmds, original_temp, test_dir_str, print_status=False, exit_on_failure=False
)
if not changed:
return re.sub(r"\n{2,}", "\n\n", formatted_code)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Can try to merge line 100 and 111 in 1 then repeating it.

Copy link
Contributor Author

@aseembits93 aseembits93 Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Saga4 it's a shortcut the function will take if there are no formatters provided which could happen often

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've yet to see a project without a formatter, know of any?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Saga4 @KRRT7 ok i have removed that part now

@aseembits93 aseembits93 requested a review from Saga4 November 12, 2025 04:32
@aseembits93 aseembits93 merged commit 4a6eaab into main Nov 13, 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.

6 participants