935137f0d9
Show inline diffs in the CLI transcript when write_file, patch, or skill_manage modifies files. Captures a filesystem snapshot before the tool runs, computes a unified diff after, and renders it with ANSI coloring in the activity feed. Adds tool_start_callback and tool_complete_callback hooks to AIAgent for pre/post tool execution notifications. Also fixes _extract_parallel_scope_path to normalize relative paths to absolute, preventing the parallel overlap detection from missing conflicts when the same file is referenced with different path styles. Gated by display.inline_diffs config option (default: true). Based on PR #3774 by @kshitijk4poor.
203 lines
7.3 KiB
Python
203 lines
7.3 KiB
Python
"""Tests for agent/display.py — build_tool_preview() and inline diff previews."""
|
|
|
|
import os
|
|
import pytest
|
|
from unittest.mock import MagicMock, patch
|
|
|
|
from agent.display import (
|
|
build_tool_preview,
|
|
capture_local_edit_snapshot,
|
|
extract_edit_diff,
|
|
_render_inline_unified_diff,
|
|
_summarize_rendered_diff_sections,
|
|
render_edit_diff_with_delta,
|
|
)
|
|
|
|
|
|
class TestBuildToolPreview:
|
|
"""Tests for build_tool_preview defensive handling and normal operation."""
|
|
|
|
def test_none_args_returns_none(self):
|
|
"""PR #453: None args should not crash, should return None."""
|
|
assert build_tool_preview("terminal", None) is None
|
|
|
|
def test_empty_dict_returns_none(self):
|
|
"""Empty dict has no keys to preview."""
|
|
assert build_tool_preview("terminal", {}) is None
|
|
|
|
def test_known_tool_with_primary_arg(self):
|
|
"""Known tool with its primary arg should return a preview string."""
|
|
result = build_tool_preview("terminal", {"command": "ls -la"})
|
|
assert result is not None
|
|
assert "ls -la" in result
|
|
|
|
def test_web_search_preview(self):
|
|
result = build_tool_preview("web_search", {"query": "hello world"})
|
|
assert result is not None
|
|
assert "hello world" in result
|
|
|
|
def test_read_file_preview(self):
|
|
result = build_tool_preview("read_file", {"path": "/tmp/test.py", "offset": 1})
|
|
assert result is not None
|
|
assert "/tmp/test.py" in result
|
|
|
|
def test_unknown_tool_with_fallback_key(self):
|
|
"""Unknown tool but with a recognized fallback key should still preview."""
|
|
result = build_tool_preview("custom_tool", {"query": "test query"})
|
|
assert result is not None
|
|
assert "test query" in result
|
|
|
|
def test_unknown_tool_no_matching_key(self):
|
|
"""Unknown tool with no recognized keys should return None."""
|
|
result = build_tool_preview("custom_tool", {"foo": "bar"})
|
|
assert result is None
|
|
|
|
def test_long_value_truncated(self):
|
|
"""Preview should truncate long values."""
|
|
long_cmd = "a" * 100
|
|
result = build_tool_preview("terminal", {"command": long_cmd}, max_len=40)
|
|
assert result is not None
|
|
assert len(result) <= 43 # max_len + "..."
|
|
|
|
def test_process_tool_with_none_args(self):
|
|
"""Process tool special case should also handle None args."""
|
|
assert build_tool_preview("process", None) is None
|
|
|
|
def test_process_tool_normal(self):
|
|
result = build_tool_preview("process", {"action": "poll", "session_id": "abc123"})
|
|
assert result is not None
|
|
assert "poll" in result
|
|
|
|
def test_todo_tool_read(self):
|
|
result = build_tool_preview("todo", {"merge": False})
|
|
assert result is not None
|
|
assert "reading" in result
|
|
|
|
def test_todo_tool_with_todos(self):
|
|
result = build_tool_preview("todo", {"todos": [{"id": "1", "content": "test", "status": "pending"}]})
|
|
assert result is not None
|
|
assert "1 task" in result
|
|
|
|
def test_memory_tool_add(self):
|
|
result = build_tool_preview("memory", {"action": "add", "target": "user", "content": "test note"})
|
|
assert result is not None
|
|
assert "user" in result
|
|
|
|
def test_session_search_preview(self):
|
|
result = build_tool_preview("session_search", {"query": "find something"})
|
|
assert result is not None
|
|
assert "find something" in result
|
|
|
|
def test_false_like_args_zero(self):
|
|
"""Non-dict falsy values should return None, not crash."""
|
|
assert build_tool_preview("terminal", 0) is None
|
|
assert build_tool_preview("terminal", "") is None
|
|
assert build_tool_preview("terminal", []) is None
|
|
|
|
|
|
class TestEditDiffPreview:
|
|
def test_extract_edit_diff_for_patch(self):
|
|
diff = extract_edit_diff("patch", '{"success": true, "diff": "--- a/x\\n+++ b/x\\n"}')
|
|
assert diff is not None
|
|
assert "+++ b/x" in diff
|
|
|
|
def test_render_inline_unified_diff_colors_added_and_removed_lines(self):
|
|
rendered = _render_inline_unified_diff(
|
|
"--- a/cli.py\n"
|
|
"+++ b/cli.py\n"
|
|
"@@ -1,2 +1,2 @@\n"
|
|
"-old line\n"
|
|
"+new line\n"
|
|
" context\n"
|
|
)
|
|
|
|
assert "a/cli.py" in rendered[0]
|
|
assert "b/cli.py" in rendered[0]
|
|
assert any("old line" in line for line in rendered)
|
|
assert any("new line" in line for line in rendered)
|
|
assert any("48;2;" in line for line in rendered)
|
|
|
|
def test_extract_edit_diff_ignores_non_edit_tools(self):
|
|
assert extract_edit_diff("web_search", '{"diff": "--- a\\n+++ b\\n"}') is None
|
|
|
|
def test_extract_edit_diff_uses_local_snapshot_for_write_file(self, tmp_path):
|
|
target = tmp_path / "note.txt"
|
|
target.write_text("old\n", encoding="utf-8")
|
|
|
|
snapshot = capture_local_edit_snapshot("write_file", {"path": str(target)})
|
|
|
|
target.write_text("new\n", encoding="utf-8")
|
|
|
|
diff = extract_edit_diff(
|
|
"write_file",
|
|
'{"bytes_written": 4}',
|
|
function_args={"path": str(target)},
|
|
snapshot=snapshot,
|
|
)
|
|
|
|
assert diff is not None
|
|
assert "--- a/" in diff
|
|
assert "+++ b/" in diff
|
|
assert "-old" in diff
|
|
assert "+new" in diff
|
|
|
|
def test_render_edit_diff_with_delta_invokes_printer(self):
|
|
printer = MagicMock()
|
|
|
|
rendered = render_edit_diff_with_delta(
|
|
"patch",
|
|
'{"diff": "--- a/x\\n+++ b/x\\n@@ -1 +1 @@\\n-old\\n+new\\n"}',
|
|
print_fn=printer,
|
|
)
|
|
|
|
assert rendered is True
|
|
assert printer.call_count >= 2
|
|
calls = [call.args[0] for call in printer.call_args_list]
|
|
assert any("a/x" in line and "b/x" in line for line in calls)
|
|
assert any("old" in line for line in calls)
|
|
assert any("new" in line for line in calls)
|
|
|
|
def test_render_edit_diff_with_delta_skips_without_diff(self):
|
|
rendered = render_edit_diff_with_delta(
|
|
"patch",
|
|
'{"success": true}',
|
|
)
|
|
|
|
assert rendered is False
|
|
|
|
def test_render_edit_diff_with_delta_handles_renderer_errors(self, monkeypatch):
|
|
printer = MagicMock()
|
|
|
|
monkeypatch.setattr("agent.display._summarize_rendered_diff_sections", MagicMock(side_effect=RuntimeError("boom")))
|
|
|
|
rendered = render_edit_diff_with_delta(
|
|
"patch",
|
|
'{"diff": "--- a/x\\n+++ b/x\\n"}',
|
|
print_fn=printer,
|
|
)
|
|
|
|
assert rendered is False
|
|
assert printer.call_count == 0
|
|
|
|
def test_summarize_rendered_diff_sections_truncates_large_diff(self):
|
|
diff = "--- a/x.py\n+++ b/x.py\n" + "".join(f"+line{i}\n" for i in range(120))
|
|
|
|
rendered = _summarize_rendered_diff_sections(diff, max_lines=20)
|
|
|
|
assert len(rendered) == 21
|
|
assert "omitted" in rendered[-1]
|
|
|
|
def test_summarize_rendered_diff_sections_limits_file_count(self):
|
|
diff = "".join(
|
|
f"--- a/file{i}.py\n+++ b/file{i}.py\n+line{i}\n"
|
|
for i in range(8)
|
|
)
|
|
|
|
rendered = _summarize_rendered_diff_sections(diff, max_files=3, max_lines=50)
|
|
|
|
assert any("a/file0.py" in line for line in rendered)
|
|
assert any("a/file1.py" in line for line in rendered)
|
|
assert any("a/file2.py" in line for line in rendered)
|
|
assert not any("a/file7.py" in line for line in rendered)
|
|
assert "additional file" in rendered[-1]
|