From ff3dcd37f6db6f036e8a23baddf26c27a774e0d2 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 3 May 2026 10:09:15 -0700 Subject: [PATCH] fix(chat-history): correct docstring inversion + pin empty-history JSON shape (#2485) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups from the multi-axis review of #2474: 1. **Docstring inversion** in tool_chat_history. The doc said '(source_id=peer)' meant 'this workspace is the sender' — actually it means the *peer* is the sender (source_id is where the activity came FROM). Reframed to 'where the peer is either the sender or the recipient' to match the underlying SQL semantics. 2. **Empty-history test**. TestChatHistory had 10 tests but no 200+[] happy-path pin. Added test_empty_history_returns_empty_json_list asserting result == '[]' on exact-equality (per assert-exact memory — substring '[]' would match envelope shapes too). Both changes are pure docs+tests — no behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) --- workspace/a2a_tools.py | 7 ++++--- workspace/tests/test_a2a_tools_impl.py | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/workspace/a2a_tools.py b/workspace/a2a_tools.py index cf855b61..a6ffed7e 100644 --- a/workspace/a2a_tools.py +++ b/workspace/a2a_tools.py @@ -559,9 +559,10 @@ async def tool_chat_history(peer_id: str, limit: int = 20, before_ts: str = "") Hits ``/workspaces//activity?peer_id=&limit=`` against the workspace-server, which returns activity rows where - this workspace is either the sender (``source_id=peer``) or the - recipient (``target_id=peer``) of an A2A turn — both sides of the - conversation in chronological order. + the peer is either the sender (``source_id=peer`` — they sent us + the message) or the recipient (``target_id=peer`` — we sent to + them) of an A2A turn — both sides of the conversation in + chronological order. Args: peer_id: The other workspace's UUID. Same value the agent diff --git a/workspace/tests/test_a2a_tools_impl.py b/workspace/tests/test_a2a_tools_impl.py index 1dd2fa14..5d994280 100644 --- a/workspace/tests/test_a2a_tools_impl.py +++ b/workspace/tests/test_a2a_tools_impl.py @@ -1050,6 +1050,27 @@ class TestChatHistory: assert mc.get.call_args.kwargs["params"]["before_ts"] == "2026-05-01T00:00:00Z" + async def test_empty_history_returns_empty_json_list(self): + """Pin the happy-path-with-no-rows shape: server returns 200 + with an empty list, the wheel returns the JSON literal ``"[]"``. + + Without this pin the surrounding tests all pre-populate rows; + none verify what an agent sees when there's literally no chat + history with this peer yet (a fresh A2A peering, or a peer + whose history was rotated out). #2485. + """ + import a2a_tools + + mc = _make_http_mock(get_resp=_resp(200, [])) + with patch("a2a_tools.httpx.AsyncClient", return_value=mc): + result = await a2a_tools.tool_chat_history(peer_id=_PEER) + + # Exact-equality on the JSON literal (per assert-exact memory) — + # substring "[]" would also match `{"items": []}` or any number + # of envelope shapes, only `result == "[]"` discriminates the + # bare-list contract callers depend on. + assert result == "[]" + async def test_reverses_desc_response_to_chronological(self): """Server returns DESC (newest first); the wheel reverses to chronological so the agent reads the chat top-down — same