Skip to content

Conversation

@mohammedahmed18
Copy link
Contributor

@mohammedahmed18 mohammedahmed18 commented Dec 8, 2025

PR Type

Bug fix, Enhancement


Description

  • Move ETA message to optimizer run flow

  • Respect '--no-pr' in ETA messaging

  • Remove duplicate messaging from discovery path

  • Import 'humanize_runtime' where used


Diagram Walkthrough

flowchart LR
  discovery["discovery/functions_to_optimize.py\n(remove ETA + PR message)"]
  optimizer["optimization/optimizer.py\n(add ETA + PR-aware message)"]
  timeutils["code_utils.time_utils.humanize_runtime\n(import here)"]

  discovery -- "remove message and import" --> optimizer
  timeutils -- "ETA formatting" --> optimizer
Loading

File Walkthrough

Relevant files
Enhancement
functions_to_optimize.py
Remove redundant ETA messaging from discovery                       

codeflash/discovery/functions_to_optimize.py

  • Remove ETA and PR message for --all path
  • Drop unused 'humanize_runtime' import
  • Keep logging of functions count
+0/-8     
Bug fix
optimizer.py
Centralize ETA and PR-aware messaging in optimizer             

codeflash/optimization/optimizer.py

  • Add ETA logging when '--all' is set
  • Respect '--no-pr' by adjusting message
  • Import 'humanize_runtime' for ETA formatting
+11/-0   

@github-actions
Copy link

github-actions bot commented Dec 8, 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

Possible Issue

The ETA calculation uses a hard-coded nanoseconds-per-function constant (1.8e11). Confirm this unit aligns with humanize_runtime expectations and that it produces reasonable estimates across environments; otherwise, users may see misleading ETAs.

three_min_in_ns = int(1.8e11)
console.rule()
pr_message = (
    "\nCodeflash will keep opening pull requests as it finds optimizations." if not self.args.no_pr else ""
)
logger.info(
    f"It might take about {humanize_runtime(num_optimizable_functions * three_min_in_ns)} to fully optimize this project.{pr_message}"
)
Message Logic

The PR-aware message is appended without spacing logic besides a leading newline in the PR text. Verify formatting in both --no-pr and default cases to avoid awkward punctuation or spacing in the composed log line.

pr_message = (
    "\nCodeflash will keep opening pull requests as it finds optimizations." if not self.args.no_pr else ""
)
logger.info(
    f"It might take about {humanize_runtime(num_optimizable_functions * three_min_in_ns)} to fully optimize this project.{pr_message}"
)

@github-actions
Copy link

github-actions bot commented Dec 8, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Fix conditional PR message output

Ensure the PR-related message is only shown when PRs will actually be opened and
keep logging output on a single line to avoid formatting issues. Move the newline
into the main string only when needed and make the condition explicit.

codeflash/optimization/optimizer.py [274-282]

 if self.args.all:
     three_min_in_ns = int(1.8e11)
     console.rule()
-    pr_message = (
-        "\nCodeflash will keep opening pull requests as it finds optimizations." if not self.args.no_pr else ""
-    )
-    logger.info(
-        f"It might take about {humanize_runtime(num_optimizable_functions * three_min_in_ns)} to fully optimize this project.{pr_message}"
-    )
+    if not self.args.no_pr:
+        logger.info(
+            f"It might take about {humanize_runtime(num_optimizable_functions * three_min_in_ns)} to fully optimize this project. "
+            "Codeflash will keep opening pull requests as it finds optimizations."
+        )
+    else:
+        logger.info(
+            f"It might take about {humanize_runtime(num_optimizable_functions * three_min_in_ns)} to fully optimize this project."
+        )
Suggestion importance[1-10]: 6

__

Why: The suggestion refactors the message construction to only show the PR-related text when applicable and removes the embedded newline, improving clarity and log formatting without changing behavior. It's a minor but reasonable improvement; the existing logic was already correct but slightly awkward.

Low

@mohammedahmed18 mohammedahmed18 merged commit 786d3f7 into main Dec 8, 2025
22 of 23 checks passed
@mohammedahmed18 mohammedahmed18 deleted the fix/all-pr-message branch December 8, 2025 12:02
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