Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Jul 30, 2025

PR Type

Bug fix, Enhancement


Description

  • Fix branch push detection and naming logic

  • Add console.rule demarcations in checkpoint prompts

  • Provide default cwd fallback in git diff

  • Enhance ignored-functions logging with Rich tree


Diagram Walkthrough

flowchart LR
  CP["checkpoint.py"] --> CP1["console.rule demarcation"]
  GITD["git_utils:get_git_diff"] --> GIT2["default cwd fallback"]
  CPB["git_utils:check_and_push_branch"] --> CPB2["fixed branch naming"]
  FO["functions_to_optimize.py"] --> FO2["Rich tree logging"]
Loading

File Walkthrough

Relevant files
Enhancement
checkpoint.py
Add console.rule demarcations to checkpoint                           

codeflash/code_utils/checkpoint.py

  • Imported console from CLI commands
  • Passed console to Confirm.ask prompt
  • Added console.rule() before and after prompt
+6/-1     
functions_to_optimize.py
Enhance ignored functions logging with Rich tree                 

codeflash/discovery/functions_to_optimize.py

  • Converted log_info values to (count, color) tuples
  • Built Rich Tree to display ignored functions
  • Printed tree only when children exist
  • Replaced plain logger.info with console.print(tree)
+16/-12 
Bug fix
git_utils.py
Fix branch push logic and diff defaults                                   

codeflash/code_utils/git_utils.py

  • Changed get_git_diff default repo_directory to None
  • Fallback to Path.cwd() when directory is None
  • Refactored check_and_push_branch to use current_branch.name
  • Improved warnings and logs for branch push prompts
+12/-9   

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Missing Imports

Several functions reference Path, sys, and time without importing them, which will cause NameError at runtime.

def get_git_diff(repo_directory: Path | None = None, *, uncommitted_changes: bool = False) -> dict[str, list[int]]:
    if repo_directory is None:
        repo_directory = Path.cwd()
    repository = git.Repo(repo_directory, search_parent_directories=True)
Duplicate Separator

console.rule() is called twice in ask_should_use_checkpoint_get_functions, leading to redundant separators in the console output.

        console=console,
    ):
        console.rule()
    else:
        previous_checkpoint_functions = None

console.rule()
Branch Push Argument

remote.push is invoked with a Branch object instead of its name string, which may not push the intended ref.

if sys.__stdin__.isatty() and Confirm.ask(
    f"⚡️ In order for me to create PRs, your current branch needs to be pushed. Do you want to push "
    f"the branch '{current_branch_name}' to the remote repository?",
    default=False,
):
    remote.push(current_branch)

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Remove redundant separator

Remove the unconditional console.rule() at the end so that a separator is only
printed when resuming from a checkpoint. This avoids cluttering the output with
extra rules when no checkpoint is detected or the user declines.

codeflash/code_utils/checkpoint.py [144-153]

 if previous_checkpoint_functions and Confirm.ask(
     "Previous Checkpoint detected from an incomplete optimization run, shall I continue the optimization from that point?",
     default=True,
     console=console,
 ):
     console.rule()
 else:
     previous_checkpoint_functions = None
 
-console.rule()
-
Suggestion importance[1-10]: 5

__

Why: The unconditional console.rule() at line 153 always prints a separator even when no checkpoint is resumed, so removing it reduces clutter and improves UX.

Low
Conditionally print separator

Move the console.rule() call inside the conditional so the separator is only printed
when there are ignored items to display. This prevents an empty rule from appearing
when nothing was ignored.

codeflash/discovery/functions_to_optimize.py [612-614]

-if len(tree.children) > 0:
+if tree.children:
     console.print(tree)
-console.rule()
+    console.rule()
Suggestion importance[1-10]: 5

__

Why: Moving the console.rule() inside the if tree.children block prevents an empty rule from appearing when no items are ignored, enhancing output clarity.

Low

@KRRT7 KRRT7 requested a review from a team July 30, 2025 18:26
@KRRT7 KRRT7 merged commit e806c5d into main Jul 30, 2025
17 checks passed
@KRRT7 KRRT7 deleted the git-branch-fixes branch July 30, 2025 21:50
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.

2 participants