Skip to content

Conversation

@misrasaurabh1
Copy link
Contributor

@misrasaurabh1 misrasaurabh1 commented Nov 15, 2025

User description

add this until gitpython adds support to git v3 index gitpython-developers/GitPython#1960


PR Type

Bug fix


Description

  • Safely handle Git submodule introspection errors

  • Add try/except around git_repo.submodules

  • Log or print warnings instead of crashing


Diagram Walkthrough

flowchart LR
  repo["Git repo detection"] -- "valid repo" --> submodules["Retrieve submodules"]
  submodules -- "exception" --> warn["Warn/print error"]
  submodules -- "success" --> paths["Return resolved submodule paths"]
  repo -- "not a repo" --> empty["Return empty list"]
Loading

File Walkthrough

Relevant files
Bug fix
functions_to_optimize.py
Add error handling for submodule listing                                 

codeflash/discovery/functions_to_optimize.py

  • Wrap submodule access in try/except.
  • Log a warning on exception.
  • Fall back to empty list on failure.
+4/-1     
tracing_utils.py
Robust submodule path retrieval in tracer                               

codeflash/tracing/tracing_utils.py

  • Wrap submodule access in try/except.
  • Print error message when retrieval fails.
  • Preserve empty list fallback behavior.
+4/-1     

Signed-off-by: Codeflash Bot <bot@codeflash.ai>
@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: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Broad Exception

Catching a blanket Exception may hide actionable errors and make debugging harder; consider narrowing to the specific GitPython exceptions expected when accessing submodules or at least logging with more context.

    try:
        return [Path(git_repo.working_tree_dir, submodule.path).resolve() for submodule in git_repo.submodules]
    except Exception as e:
        logger.warning(f"Error getting submodule paths: {e}")
return []
Inconsistent Logging

One path uses logger.warning while another uses print, which can lead to inconsistent observability; consider standardizing on a logging approach even in tracing utilities (e.g., optional logger or stderr).

    try:
        return [Path(working_tree_dir, submodule.path).resolve() for submodule in git_repo.submodules]
    except Exception as e:
        print(f"Failed to get submodule paths {str(e)}")  # no logger since used in the tracer
return []
Lost Context

The warning message drops useful context like the repo path; include module_root or working_tree_dir in the log to aid troubleshooting.

    try:
        return [Path(git_repo.working_tree_dir, submodule.path).resolve() for submodule in git_repo.submodules]
    except Exception as e:
        logger.warning(f"Error getting submodule paths: {e}")
return []

@github-actions
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Ensure fallback return and better logging

Avoid swallowing the error by returning None implicitly; always return a list to
keep the function contract consistent. Also include exception type in the log and
set exc_info to preserve stacktrace for debugging.

codeflash/discovery/functions_to_optimize.py [408-411]

 try:
     return [Path(git_repo.working_tree_dir, submodule.path).resolve() for submodule in git_repo.submodules]
 except Exception as e:
-    logger.warning(f"Error getting submodule paths: {e}")
+    logger.warning("Error getting submodule paths (%s): %s", type(e).__name__, e, exc_info=True)
+    return []
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly ensures a consistent return type by adding a fallback empty list and improves logging with exception type and stack trace. It aligns with the new hunk code and is a solid robustness improvement, though not critical.

Medium
Return empty list on failure

Guarantee a consistent return type by returning an empty list on failure to avoid
callers receiving None. Use a plain message without f-string evaluation of the
exception to avoid potential encoding issues and include the exception class for
clarity.

codeflash/tracing/tracing_utils.py [41-44]

 try:
     return [Path(working_tree_dir, submodule.path).resolve() for submodule in git_repo.submodules]
 except Exception as e:
-    print(f"Failed to get submodule paths {str(e)}")  # no logger since used in the tracer
+    print(f"Failed to get submodule paths ({type(e).__name__}): {e}")  # no logger since used in the tracer
+    return []
Suggestion importance[1-10]: 7

__

Why: Returning an empty list maintains the function contract and avoids implicit None, and the logging message is clearer with the exception class. This is accurate to the PR context and improves reliability without being a critical fix.

Medium

Signed-off-by: Codeflash Bot <bot@codeflash.ai>
@aseembits93
Copy link
Contributor

we should get back to a fix where we see new files being created in PRs

@aseembits93 aseembits93 merged commit 0e8f5e4 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