Skip to content

Conversation

@KRRT7
Copy link
Contributor

@KRRT7 KRRT7 commented Dec 12, 2025

PR Type

Bug fix, Tests, Enhancement


Description

  • Improve git remote validation and messaging

  • Handle missing/invalid repos without warnings

  • Strengthen CLI error handling for PR flow

  • Stabilize e2e Futurehouse test expectations


Diagram Walkthrough

flowchart LR
  CLI["CLI arg parsing"] -- "validate repo & remote" --> GitUtils["Git utilities"]
  GitUtils -- "raise/return friendly errors" --> CLI
  AISvc["AI service repo info"] -- "safe get owner/name" --> Logger["Logging"]
  Tests["E2E Futurehouse tests"] -- "config + paths + pyproject-aware" --> Runner["Test runner"]
Loading

File Walkthrough

Relevant files
Bug fix
aiservice.py
Quiet handling of missing git repo/remotes                             

codeflash/api/aiservice.py

  • Import git for exception handling
  • Suppress expected git errors without warnings
  • Keep logging only for unexpected exceptions
+8/-0     
functions_to_optimize.py
Non-noisy repo info retrieval                                                       

codeflash/discovery/functions_to_optimize.py

  • Treat ValueError like no-repo case
  • Downgrade log to debug for expected issues
+2/-2     
Error handling
cli.py
Clear CLI errors for repo/remote issues                                   

codeflash/cli_cmds/cli.py

  • Guard repo discovery with try/except
  • Show actionable error for missing remotes
  • Improve branch push failure message
+19/-5   
Enhancement
git_utils.py
Enforce and explain git remote requirements                           

codeflash/code_utils/git_utils.py

  • Validate presence of git remotes
  • Error when requested remote missing
  • User guidance on configuring remotes
+29/-0   
Tests
end_to_end_test_futurehouse.py
Stabilize Futurehouse e2e test setup                                         

tests/scripts/end_to_end_test_futurehouse.py

  • Use Path for file paths
  • Fix import path to utilities
  • Expect two unit tests
  • Clarify working directory variable
+12/-7   
end_to_end_test_utilities.py
Test runner respects pyproject configuration                         

tests/scripts/end_to_end_test_utilities.py

  • Add tomllib and contextlib
  • Respect pyproject codeflash config
  • Only pass roots when not configured
  • Minor cleanup of command flow
+16/-3   

@github-actions
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

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

Exception Handling

Broadly importing git and catching git.exc.InvalidGitRepositoryError ties runtime to GitPython presence. Ensure GitPython is always available here or guard the import; otherwise NameError could occur where git is not installed.

import git
import requests
from pydantic.json import pydantic_encoder

from codeflash.cli_cmds.console import console, logger
from codeflash.code_utils.code_replacer import is_zero_diff
from codeflash.code_utils.code_utils import unified_diff_strings
UX Message Consistency

check_and_push_branch returns False with detailed logs, and callers exit with additional generic messages. Consider ensuring messages aren’t duplicated and are consistent across CLI paths.

def check_and_push_branch(repo: git.Repo, git_remote: str | None = "origin", *, wait_for_push: bool = False) -> bool:
    current_branch = repo.active_branch
    current_branch_name = current_branch.name
    available_remotes = get_git_remotes(repo)
    if not available_remotes:
        logger.error(
            f"❌ No git remotes configured in this repository.\n"
            f"This appears to be a local-only git repository. To use codeflash with PR features, you need to:\n"
            f"  1. Create a repository on GitHub (or another git hosting service)\n"
            f"  2. Add it as a remote: git remote add origin <repository-url>\n"
            f"  3. Push your branch: git push -u origin {current_branch_name}\n\n"
            f"Alternatively, you can run codeflash with the '--no-pr' flag to optimize locally without creating PRs."
        )
        return False

    # Check if the specified remote exists
    if git_remote not in available_remotes:
        logger.error(
            f"❌ Git remote '{git_remote}' does not exist in this repository.\n"
            f"Available remotes: {', '.join(available_remotes)}\n\n"
            f"You can either:\n"
            f"  1. Use one of the existing remotes by setting 'git-remote' in pyproject.toml\n"
            f"  2. Add the '{git_remote}' remote: git remote add {git_remote} <repository-url>\n"
            f"  3. Run codeflash with '--no-pr' to optimize locally without creating PRs."
        )
        return False
Error Scope

Only ValueError is handled when determining repo owner/name; other expected git errors may surface. Confirm get_repo_owner_and_name consistently raises ValueError for missing remotes across code paths to avoid uncaught exceptions.

try:
    owner, repo = get_repo_owner_and_name()
    require_github_app_or_exit(owner, repo)
except ValueError as e:
    exit_with_message(
        f"Cannot determine repository information: {e}\n"
        f"Please ensure your git repository has a remote configured, or use '--no-pr' to optimize locally.",
        error_on_exit=True
    )

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Normalize optional remote name

When git_remote is None, the membership test and error message will be misleading or
raise. Normalize git_remote to a default (e.g., 'origin') before validation. This
prevents unexpected crashes and ensures clear errors when no remote name is
provided.

codeflash/code_utils/git_utils.py [81-88]

 def get_remote_url(repo: Repo | None = None, git_remote: str | None = "origin") -> str:
     repository: Repo = repo if repo else git.Repo(search_parent_directories=True)
     available_remotes = get_git_remotes(repository)
     if not available_remotes:
-        raise ValueError(f"No git remotes configured in this repository")
-    if git_remote not in available_remotes:
-        raise ValueError(f"Git remote '{git_remote}' does not exist. Available remotes: {', '.join(available_remotes)}")
-    return repository.remote(name=git_remote).url
+        raise ValueError("No git remotes configured in this repository")
+    # Normalize None to default remote
+    remote_name = git_remote or "origin"
+    if remote_name not in available_remotes:
+        raise ValueError(f"Git remote '{remote_name}' does not exist. Available remotes: {', '.join(available_remotes)}")
+    return repository.remote(name=remote_name).url
Suggestion importance[1-10]: 7

__

Why: Correctly handles when git_remote is None to prevent misleading checks or crashes; moderate impact and aligns with surrounding code that may pass None.

Medium
Default missing remote safely

If git_remote is None, the existence check and repo.remote(name=git_remote) may
fail. Default git_remote to a valid remote (e.g., 'origin' or the first available)
before checks to avoid runtime errors and provide a clear path forward.

codeflash/code_utils/git_utils.py [133-161]

 def check_and_push_branch(repo: git.Repo, git_remote: str | None = "origin", *, wait_for_push: bool = False) -> bool:
     current_branch = repo.active_branch
     current_branch_name = current_branch.name
     available_remotes = get_git_remotes(repo)
     if not available_remotes:
         logger.error(
-            f"❌ No git remotes configured in this repository.\n"
-            f"This appears to be a local-only git repository. To use codeflash with PR features, you need to:\n"
-            f"  1. Create a repository on GitHub (or another git hosting service)\n"
-            f"  2. Add it as a remote: git remote add origin <repository-url>\n"
+            "❌ No git remotes configured in this repository.\n"
+            "This appears to be a local-only git repository. To use codeflash with PR features, you need to:\n"
+            "  1. Create a repository on GitHub (or another git hosting service)\n"
+            "  2. Add it as a remote: git remote add origin <repository-url>\n"
             f"  3. Push your branch: git push -u origin {current_branch_name}\n\n"
-            f"Alternatively, you can run codeflash with the '--no-pr' flag to optimize locally without creating PRs."
+            "Alternatively, you can run codeflash with the '--no-pr' flag to optimize locally without creating PRs."
         )
         return False
-      
-    # Check if the specified remote exists
-    if git_remote not in available_remotes:
+    
+    # Normalize None to a valid remote (prefer 'origin' if present)
+    remote_name = git_remote or ("origin" if "origin" in available_remotes else available_remotes[0])
+    if remote_name not in available_remotes:
         logger.error(
-            f"❌ Git remote '{git_remote}' does not exist in this repository.\n"
+            f"❌ Git remote '{remote_name}' does not exist in this repository.\n"
             f"Available remotes: {', '.join(available_remotes)}\n\n"
-            f"You can either:\n"
-            f"  1. Use one of the existing remotes by setting 'git-remote' in pyproject.toml\n"
-            f"  2. Add the '{git_remote}' remote: git remote add {git_remote} <repository-url>\n"
-            f"  3. Run codeflash with '--no-pr' to optimize locally without creating PRs."
+            "You can either:\n"
+            "  1. Use one of the existing remotes by setting 'git-remote' in pyproject.toml\n"
+            f"  2. Add the '{remote_name}' remote: git remote add {remote_name} <repository-url>\n"
+            "  3. Run codeflash with '--no-pr' to optimize locally without creating PRs."
         )
         return False
-      
-    remote = repo.remote(name=git_remote)
+    
+    remote = repo.remote(name=remote_name)
Suggestion importance[1-10]: 7

__

Why: Provides a safe fallback when git_remote is None and avoids failures at repo.remote(name=git_remote); useful robustness improvement but not critical.

Medium
General
Avoid brittle unit test count

expected_unit_tests=2 may cause deterministic failures if the sample repo’s tests
count differs across environments. Derive the count dynamically from the tests
directory or relax this assertion to avoid brittle end-to-end failures.

tests/scripts/end_to_end_test_futurehouse.py [13-28]

 def run_test(expected_improvement_pct: int) -> bool:
     config = TestConfig(
         file_path=Path("src/aviary/common_tags.py"),
-        expected_unit_tests=2,
+        expected_unit_tests=0,  # derive dynamically to avoid brittleness
         min_improvement_x=0.05,
         coverage_expectations=[
             CoverageExpectation(
                 function_name="find_common_tags",
                 expected_coverage=100.0,
                 expected_lines=[5, 6, 7, 8, 9, 11, 12, 13, 14],
             )
         ],
     )
     future_house_working_dir = (
         Path(__file__).parent.parent.parent / "code_to_optimize" / "code_directories" / "futurehouse_structure"
     ).resolve()
     return run_codeflash_command(future_house_working_dir, config, expected_improvement_pct)
Suggestion importance[1-10]: 4

__

Why: Suggestion relaxes expected_unit_tests contrary to the PR's explicit increase to 2; it's speculative and may mask real regressions, offering limited benefit.

Low

formatter
Saga4
Saga4 previously approved these changes Dec 12, 2025
@KRRT7 KRRT7 requested a review from Saga4 December 12, 2025 15:39
Saga4
Saga4 previously approved these changes Dec 12, 2025
The 'expected_unit_tests' config actually validates the number of test
invocations (function calls in tests), not the number of test files.

Updated regex to match the global discovery message format:
'Discovered X existing unit tests and Y replay tests in Z.Zs at /path'

This counts how many times functions are called/invoked in test files,
which is what num_discovered_tests represents in the codebase.
@KRRT7 KRRT7 requested a review from Saga4 December 14, 2025 17:37
misrasaurabh1
misrasaurabh1 previously approved these changes Dec 16, 2025
@misrasaurabh1
Copy link
Contributor

tests are failing due to this change which you will need to re-concile

@misrasaurabh1
Copy link
Contributor

Hey @KRRT7 can we please merge this change?

KRRT7 and others added 2 commits December 18, 2025 21:41
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