fix(python): harden prompt isolation and centralize error handling#143
fix(python): harden prompt isolation and centralize error handling#143
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @calchiwo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the robustness and maintainability of the Python runtime. It introduces mechanisms to prevent prompt injection by isolating and escaping repository content within LLM prompts. Concurrently, it refactors error handling by centralizing common Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request does a great job of centralizing error handling and hardening the prompts against injection. The introduction of safe_read_repo_files and generate_with_exit significantly improves code structure by removing duplicated try/except blocks. The prompt isolation techniques, like escaping content and adding explicit XML-like tags, are excellent for robustness.
My feedback focuses on a couple of areas for improvement:
- Consistently using
stderrfor warnings and error messages to follow standard CLI practices. - Addressing a minor formatting issue in prompt construction that could lead to unintended extra newlines.
Overall, these are solid improvements to the tool's reliability and maintainability.
| try: | ||
| return read_repo_signal_files(owner, repo) | ||
| except Exception as e: | ||
| print(f"warning: could not read repository files: {e}") |
There was a problem hiding this comment.
It's standard practice for command-line tools to write warning messages to stderr. This allows users to redirect stdout (the primary output) without losing important diagnostic messages. Please direct this warning to sys.stderr.
| print(f"warning: could not read repository files: {e}") | |
| print(f"warning: could not read repository files: {e}", file=sys.stderr) |
| print("Failed to generate explanation.") | ||
| print(f"error: {e}") | ||
| print("\nfix:") | ||
| print("- Ensure GEMINI_API_KEY is set") | ||
| print("- Or run: explainthisrepo --doctor") |
There was a problem hiding this comment.
Error messages and user guidance for fixing issues should be printed to stderr rather than stdout. This separates diagnostic output from the program's main output, which is a standard convention for CLI applications.
| print("Failed to generate explanation.") | |
| print(f"error: {e}") | |
| print("\nfix:") | |
| print("- Ensure GEMINI_API_KEY is set") | |
| print("- Or run: explainthisrepo --doctor") | |
| print("Failed to generate explanation.", file=sys.stderr) | |
| print(f"error: {e}", file=sys.stderr) | |
| print("\nfix:", file=sys.stderr) | |
| print("- Ensure GEMINI_API_KEY is set", file=sys.stderr) | |
| print("- Or run: explainthisrepo --doctor", file=sys.stderr) |
This PR hardens the Python runtime to match the Node implementation
It introduces prompt isolation blocks and escapes repository content to prevent delimiter breakout
It also centralizes error handling by extracting
safe_read_repo_filesandgenerate_with_exit, removing duplicatedtry/exceptlogic