Ellipsize long outputs from commands #356
Conversation
![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
The pull request introduces functionality to ellipsize long outputs from shell commands, which is a valuable addition for managing verbose command outputs. The implementation correctly handles the full_output flag and truncates stdout and stderr when full_output is False. A new test case has been added to cover this new functionality, which is good. The logic for calculating the number of lines to keep from the beginning and end of the output is sound.
Signed-off-by: Nikola Forró <[email protected]>
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request introduces a valuable feature to ellipsize long command outputs, improving readability and preventing excessive log sizes. The implementation correctly handles full output and truncated output, including the insertion of an ellipsis. The new test case test_run_shell_command_huge_output effectively validates this new functionality, covering both scenarios. Overall, the changes are well-implemented and tested.
| def ellipsize(output): | ||
| if output is None: | ||
| return None | ||
| if tool_input.full_output: | ||
| return output | ||
| lines = output.splitlines(keepends=True) | ||
| if len(lines) <= ELLIPSIZED_LINES: | ||
| return output | ||
| return "".join( | ||
| lines[: math.floor((ELLIPSIZED_LINES - 1) / 2)] | ||
| + ["[...]\n"] | ||
| + lines[-math.ceil((ELLIPSIZED_LINES - 1) / 2) :] | ||
| ) |
There was a problem hiding this comment.
The ellipsize function is currently defined as a nested function within _run. While functional, defining it as a private method (e.g., _ellipsize) of the RunShellCommandTool class would improve code organization and avoid recreating the function object on every call to _run. This enhances maintainability and makes the helper function more explicitly part of the class's behavior.
def _ellipsize(output: str | None, full_output_flag: bool) -> str | None:
if output is None:
return None
if full_output_flag:
return output
lines = output.splitlines(keepends=True)
if len(lines) <= ELLIPSIZED_LINES:
return output
return "".join(
lines[: math.floor((ELLIPSIZED_LINES - 1) / 2)]
+ ["[...]
"]
+ lines[-math.ceil((ELLIPSIZED_LINES - 1) / 2) :]
)
result = {
"exit_code": exit_code,
"stdout": _ellipsize(stdout, tool_input.full_output),
"stderr": _ellipsize(stderr, tool_input.full_output),
}
2 participants
No description provided.