first version of shell tool optimization #10625
Conversation
Summary of ChangesHello @hminooei, 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 introduces a foundational set of operational guidelines aimed at optimizing the execution and output of shell commands. The primary goal is to foster more concise and high-signal interactions, thereby reducing unnecessary context and improving efficiency. These guidelines cover strategies for minimizing output verbosity using platform-specific quiet flags and for managing extensive command outputs by redirecting them to log files for selective review. Highlights
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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces optimizations for shell command usage by updating the agent's operational guidelines. The goal is to encourage more concise and efficient shell interactions, such as using quiet flags and redirecting output to files. My review focuses on improving the clarity and efficiency of these new guidelines. Specifically, I've suggested clarifying the output redirection instructions and, more importantly, updating the example workflow to leverage command exit codes for error checking, which is a more robust and efficient approach than unconditionally checking log files.
| - For commands that usually have long output, use cmd >out.log 2>err.log or cmd > out.log 2> err.log in cmd.exe or cmd > out.log 2> err.log in powershell, to separate the stdout vs stderr. | ||
| - Then use sed/tail/head or cmd.exe / powershell equivalent commands to read a specific line range (or all) of the log file. | ||
| <example> | ||
| user: Run the build and let me know if there are any errors. | ||
| model: [tool_call: run_shell_command for 'npm run build > build.log 2> build.err'] | ||
| (After command execution) | ||
| model: The build process is complete. I will now check the error log. | ||
| [tool_call: run_shell_command for 'tail -n 10 build.err'] | ||
| (After noticing errors exist) | ||
| model: Read the whole error log to understand the issues. | ||
| [tool_call: run_shell_command for 'cat build.err'] | ||
| (After reading the file) | ||
| model: The build failed due to a missing semicolon in src/index.ts and an undefined variable in src/app.ts. Would you like me to fix these issues? | ||
| </example> |
There was a problem hiding this comment.
The guidance for output redirection and the accompanying example can be improved for clarity and efficiency.
- Clarify Redirection Guidance: The current explanation for output redirection is a bit repetitive and platform-specific. A more universal and concise explanation would be easier for the model to understand and apply.
- Incorporate Exit Code Check: The example workflow for checking build errors is inefficient. It involves checking the error log file regardless of whether the command succeeded or failed. The
run_shell_commandtool returns an exit code, which is a much more reliable and efficient way to determine success or failure. The prompt should instruct the model to check the exit code first, and only inspect the error log if the exit code is non-zero.
Here is a suggested improvement for this section:
- For commands with potentially long output, redirect stdout and stderr to files (e.g., `command > out.log 2> err.log`). After the command runs, **check the exit code first**. A non-zero exit code indicates an error, and you should then inspect the `err.log` file.
- Then use `cat`, `tail`, or `head` (or platform equivalents) to read the log files.
<example>
user: Run the build and let me know if there are any errors.
model: [tool_call: run_shell_command for 'npm run build > build.log 2> build.err']
(After command execution, the tool returns a non-zero exit code)
model: The build failed. I will now check the error log to understand the issue.
[tool_call: run_shell_command for 'cat build.err']
(After reading the file)
model: The build failed due to a missing semicolon in src/index.ts and an undefined variable in src/app.ts. Would you like me to fix these issues?
</example>
| - For commands that usually have long output, use cmd >out.log 2>err.log or cmd > out.log 2> err.log in cmd.exe or cmd > out.log 2> err.log in powershell, to separate the stdout vs stderr. | ||
| - Then use sed/tail/head or cmd.exe / powershell equivalent commands to read a specific line range (or all) of the log file. | ||
| <example> | ||
| user: Run the build and let me know if there are any errors. | ||
| model: [tool_call: ${ShellTool.Name} for 'npm run build > build.log 2> build.err'] | ||
| (After command execution) | ||
| model: The build process is complete. I will now check the error log. | ||
| [tool_call: ${ShellTool.Name} for 'tail -n 10 build.err'] | ||
| (After noticing errors exist) | ||
| model: Read the whole error log to understand the issues. | ||
| [tool_call: ${ShellTool.Name} for 'cat build.err'] | ||
| (After reading the file) | ||
| model: The build failed due to a missing semicolon in src/index.ts and an undefined variable in src/app.ts. Would you like me to fix these issues? | ||
| </example> |
There was a problem hiding this comment.
The guidance for output redirection and the accompanying example can be improved for clarity and efficiency.
- Clarify Redirection Guidance: The current explanation for output redirection is a bit repetitive and platform-specific. A more universal and concise explanation would be easier for the model to understand and apply.
- Incorporate Exit Code Check: The example workflow for checking build errors is inefficient. It involves checking the error log file regardless of whether the command succeeded or failed. The
run_shell_commandtool returns an exit code, which is a much more reliable and efficient way to determine success or failure. The prompt should instruct the model to check the exit code first, and only inspect the error log if the exit code is non-zero.
Here is a suggested improvement for this section:
- For commands with potentially long output, redirect stdout and stderr to files (e.g., `command > out.log 2> err.log`). After the command runs, **check the exit code first**. A non-zero exit code indicates an error, and you should then inspect the `err.log` file.
- Then use `cat`, `tail`, or `head` (or platform equivalents) to read the log files.
<example>
user: Run the build and let me know if there are any errors.
model: [tool_call: ${ShellTool.Name} for 'npm run build > build.log 2> build.err']
(After command execution, the tool returns a non-zero exit code)
model: The build failed. I will now check the error log to understand the issue.
[tool_call: ${ShellTool.Name} for 'cat build.err']
(After reading the file)
model: The build failed due to a missing semicolon in src/index.ts and an undefined variable in src/app.ts. Would you like me to fix these issues?
</example>
|
Size Change: +1.65 kB (+0.01%) Total Size: 17.6 MB
ℹ️ View Unchanged
|
hminooei
changed the title
TLDR
Our goal is to optimize the shell commands to promote concise and high-signal outputs, reducing context bloat.
Dive Deeper
Please see
Reviewer Test Plan
No regression on swebench-v and Terminal Bench.
Linked issues / bugs
This PR makes progress on #10270