Merge pull request #63 from dgenio/copilot/fix-stale-issues-and-fill-coverage-gaps

test: close coverage gaps in summarize.py, http.py, and transform.py

Author: dgenio

CHANGELOG.md

@@ -10,6 +10,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Added
 - GitHub Release step in publish workflow — creates a release with auto-generated notes and artifacts before publishing to PyPI.
 
+### Fixed
+- `HTTPDriver`: DELETE requests now forward args as query params instead of silently dropping them.
+
+### Removed
+- Dead `_truncate_str` helper in `firewall/transform.py` (defined but never called).
+
 ## [0.3.0] - 2026-03-09
 
 ### Added

src/agent_kernel/drivers/http.py

@@ -83,7 +83,7 @@ async def execute(self, ctx: ExecutionContext) -> RawResult:
         params: dict[str, Any] = {}
         json_body: dict[str, Any] | None = None
 
-        if endpoint.method.upper() == "GET":
+        if endpoint.method.upper() in ("GET", "DELETE"):
             params = {k: v for k, v in ctx.args.items() if k != "operation"}
         else:
             json_body = {k: v for k, v in ctx.args.items() if k != "operation"}

src/agent_kernel/firewall/transform.py

@@ -233,12 +233,6 @@ def _make_table(self, data: Any, *, max_rows: int) -> list[dict[str, Any]]:
         return result
 
 
-def _truncate_str(s: str, max_chars: int) -> str:
-    if len(s) <= max_chars:
-        return s
-    return s[:max_chars]
-
-
 def _cap_facts(facts: list[str], max_chars: int) -> list[str]:
     """Return as many facts as fit within *max_chars* total."""
     total = 0

tests/test_drivers.py

@@ -5,6 +5,7 @@
 from typing import Any
 from unittest.mock import AsyncMock, MagicMock, patch
 
+import httpx
 import pytest
 
 from agent_kernel import DriverError, InMemoryDriver
@@ -158,8 +159,6 @@ async def test_httpdriver_unknown_operation_raises() -> None:
 
 @pytest.mark.asyncio
 async def test_httpdriver_http_error_raises(monkeypatch: pytest.MonkeyPatch) -> None:
-    import httpx
-
     driver = HTTPDriver()
     endpoint = HTTPEndpoint(url="http://localhost:9999/fail", method="GET")
     driver.register_endpoint("fail_op", endpoint)
@@ -183,3 +182,136 @@ async def test_httpdriver_http_error_raises(monkeypatch: pytest.MonkeyPatch) ->
         )
         with pytest.raises(DriverError, match="HTTP 500"):
             await driver.execute(ctx)
+
+
+@pytest.mark.asyncio
+async def test_httpdriver_execute_post() -> None:
+    driver = HTTPDriver()
+    endpoint = HTTPEndpoint(url="http://localhost:9999/items", method="POST")
+    driver.register_endpoint("create_item", endpoint)
+
+    mock_response = MagicMock()
+    mock_response.json.return_value = {"created": True}
+    mock_response.status_code = 201
+    mock_response.raise_for_status = MagicMock()
+
+    mock_client = AsyncMock()
+    mock_client.__aenter__ = AsyncMock(return_value=mock_client)
+    mock_client.__aexit__ = AsyncMock(return_value=False)
+    mock_client.post = AsyncMock(return_value=mock_response)
+
+    with patch("agent_kernel.drivers.http.httpx.AsyncClient", return_value=mock_client):
+        ctx = ExecutionContext(
+            capability_id="cap.x",
+            principal_id="u1",
+            args={"operation": "create_item", "name": "test"},
+        )
+        result = await driver.execute(ctx)
+    assert result.data == {"created": True}
+    mock_client.post.assert_called_once()
+
+
+@pytest.mark.asyncio
+async def test_httpdriver_execute_put() -> None:
+    driver = HTTPDriver()
+    endpoint = HTTPEndpoint(url="http://localhost:9999/items/1", method="PUT")
+    driver.register_endpoint("update_item", endpoint)
+
+    mock_response = MagicMock()
+    mock_response.json.return_value = {"updated": True}
+    mock_response.status_code = 200
+    mock_response.raise_for_status = MagicMock()
+
+    mock_client = AsyncMock()
+    mock_client.__aenter__ = AsyncMock(return_value=mock_client)
+    mock_client.__aexit__ = AsyncMock(return_value=False)
+    mock_client.put = AsyncMock(return_value=mock_response)
+
+    with patch("agent_kernel.drivers.http.httpx.AsyncClient", return_value=mock_client):
+        ctx = ExecutionContext(
+            capability_id="cap.x",
+            principal_id="u1",
+            args={"operation": "update_item", "name": "updated"},
+        )
+        result = await driver.execute(ctx)
+    assert result.data == {"updated": True}
+    mock_client.put.assert_called_once()
+
+
+@pytest.mark.asyncio
+async def test_httpdriver_execute_delete() -> None:
+    driver = HTTPDriver()
+    endpoint = HTTPEndpoint(url="http://localhost:9999/items/1", method="DELETE")
+    driver.register_endpoint("delete_item", endpoint)
+
+    mock_response = MagicMock()
+    mock_response.json.return_value = {"deleted": True}
+    mock_response.status_code = 200
+    mock_response.raise_for_status = MagicMock()
+
+    mock_client = AsyncMock()
+    mock_client.__aenter__ = AsyncMock(return_value=mock_client)
+    mock_client.__aexit__ = AsyncMock(return_value=False)
+    mock_client.delete = AsyncMock(return_value=mock_response)
+
+    with patch("agent_kernel.drivers.http.httpx.AsyncClient", return_value=mock_client):
+        ctx = ExecutionContext(
+            capability_id="cap.x",
+            principal_id="u1",
+            args={"operation": "delete_item", "id": "1"},
+        )
+        result = await driver.execute(ctx)
+    assert result.data == {"deleted": True}
+    mock_client.delete.assert_called_once_with("http://localhost:9999/items/1", params={"id": "1"})
+
+
+@pytest.mark.asyncio
+async def test_httpdriver_execute_patch_uses_request() -> None:
+    driver = HTTPDriver()
+    endpoint = HTTPEndpoint(url="http://localhost:9999/items/1", method="PATCH")
+    driver.register_endpoint("patch_item", endpoint)
+
+    mock_response = MagicMock()
+    mock_response.json.return_value = {"patched": True}
+    mock_response.status_code = 200
+    mock_response.raise_for_status = MagicMock()
+
+    mock_client = AsyncMock()
+    mock_client.__aenter__ = AsyncMock(return_value=mock_client)
+    mock_client.__aexit__ = AsyncMock(return_value=False)
+    mock_client.request = AsyncMock(return_value=mock_response)
+
+    with patch("agent_kernel.drivers.http.httpx.AsyncClient", return_value=mock_client):
+        ctx = ExecutionContext(
+            capability_id="cap.x",
+            principal_id="u1",
+            args={"operation": "patch_item", "field": "value"},
+        )
+        result = await driver.execute(ctx)
+    assert result.data == {"patched": True}
+    mock_client.request.assert_called_once_with(
+        "PATCH", "http://localhost:9999/items/1", json={"field": "value"}
+    )
+
+
+@pytest.mark.asyncio
+async def test_httpdriver_request_error_raises() -> None:
+    driver = HTTPDriver()
+    endpoint = HTTPEndpoint(url="http://localhost:9999/unreachable", method="GET")
+    driver.register_endpoint("unreachable_op", endpoint)
+
+    mock_client = AsyncMock()
+    mock_client.__aenter__ = AsyncMock(return_value=mock_client)
+    mock_client.__aexit__ = AsyncMock(return_value=False)
+    mock_client.get = AsyncMock(
+        side_effect=httpx.ConnectError("Connection refused", request=MagicMock())
+    )
+
+    with patch("agent_kernel.drivers.http.httpx.AsyncClient", return_value=mock_client):
+        ctx = ExecutionContext(
+            capability_id="cap.x",
+            principal_id="u1",
+            args={"operation": "unreachable_op"},
+        )
+        with pytest.raises(DriverError, match="Request to .* failed"):
+            await driver.execute(ctx)

tests/test_firewall.py

@@ -6,6 +6,7 @@
 
 from agent_kernel import Firewall
 from agent_kernel.firewall.budgets import Budgets
+from agent_kernel.firewall.summarize import summarize
 from agent_kernel.models import Handle, RawResult
 
 
@@ -155,3 +156,133 @@ def test_max_depth_limiting() -> None:
     budgets = Budgets(max_depth=2)
     frame = _transform(deep, "summary", budgets=budgets)
     assert frame.response_mode == "summary"  # type: ignore[union-attr]
+
+
+# ── Raw mode budget warning ────────────────────────────────────────────────────
+
+
+def test_raw_mode_oversized_data_adds_warning() -> None:
+    large_data = {"payload": "x" * 10_000}
+    budgets = Budgets(max_chars=100)
+    frame = _transform(large_data, "raw", principal_roles=["admin"], budgets=budgets)
+    assert frame.response_mode == "raw"  # type: ignore[union-attr]
+    assert any("exceeds budget" in w for w in frame.warnings)  # type: ignore[union-attr]
+    assert frame.raw_data == large_data  # type: ignore[union-attr]
+
+
+# ── Table mode with non-list data ──────────────────────────────────────────────
+
+
+def test_table_mode_single_dict() -> None:
+    frame = _transform({"a": 1, "b": 2}, "table")
+    assert frame.response_mode == "table"  # type: ignore[union-attr]
+    assert len(frame.table_preview) == 1  # type: ignore[union-attr]
+    assert frame.table_preview[0]["a"] == 1  # type: ignore[union-attr]
+
+
+def test_table_mode_non_dict_rows() -> None:
+    frame = _transform([1, 2, 3], "table")
+    assert frame.response_mode == "table"  # type: ignore[union-attr]
+    assert frame.table_preview[0] == {"value": 1}  # type: ignore[union-attr]
+
+
+def test_table_mode_scalar_data() -> None:
+    frame = _transform(42, "table")
+    assert frame.response_mode == "table"  # type: ignore[union-attr]
+    assert frame.table_preview == [{"value": 42}]  # type: ignore[union-attr]
+
+
+# ── _cap_facts via public interface ────────────────────────────────────────────
+
+
+def test_summary_cap_facts_stops_at_budget() -> None:
+    # "Keys: key1, key2" (16 chars) fits in max_chars=20; the next fact (46+ chars)
+    # pushes the running total over budget, triggering the break in _cap_facts.
+    data = {"key1": "v" * 40, "key2": "v" * 40}
+    budgets = Budgets(max_chars=20)
+    frame = _transform(data, "summary", budgets=budgets)
+    assert frame.response_mode == "summary"  # type: ignore[union-attr]
+    assert len(frame.facts) == 1  # type: ignore[union-attr]
+    assert "Keys" in frame.facts[0]  # type: ignore[union-attr]
+
+
+def test_cap_facts_all_fit() -> None:
+    # Both short facts fit well within a generous budget — no break triggered.
+    data = {"a": 1, "b": 2}
+    budgets = Budgets(max_chars=10_000)
+    frame = _transform(data, "summary", budgets=budgets)
+    assert frame.response_mode == "summary"  # type: ignore[union-attr]
+    assert len(frame.facts) >= 2  # type: ignore[union-attr]
+
+
+# ── summarize() edge cases ─────────────────────────────────────────────────────
+
+
+def test_summarize_plain_list() -> None:
+    facts = summarize([1, 2, 3, "hello"])
+    assert facts[0] == "List of 4 items"
+    assert "1" in facts[1]
+
+
+def test_summarize_other_type_int() -> None:
+    facts = summarize(42)
+    assert facts == ["42"]
+
+
+def test_summarize_other_type_none() -> None:
+    facts = summarize(None)
+    assert facts == ["None"]
+
+
+def test_summarize_string_truncation() -> None:
+    long_str = "a" * 600
+    facts = summarize(long_str)
+    assert len(facts) == 1
+    assert "600 chars total" in facts[0]
+    assert facts[0].startswith("a" * 500)
+
+
+def test_summarize_list_of_dicts_numeric_max_facts() -> None:
+    rows = [{"n1": i, "n2": i * 2, "n3": i * 3} for i in range(5)]
+    # max_facts=3: "Total rows" + "Top keys" = 2, then 1 numeric fact hits limit
+    facts = summarize(rows, max_facts=3)
+    assert len(facts) <= 3
+
+
+def test_summarize_list_of_dicts_categorical_distribution() -> None:
+    rows = [{"status": s} for s in ["open", "closed", "open", "pending", "closed"]]
+    facts = summarize(rows)
+    assert any("distribution" in f for f in facts)
+
+
+def test_summarize_list_of_dicts_no_string_values_in_field() -> None:
+    # List values are not strings and not numeric — categorical loop skips them
+    rows = [{"items": [1, 2]}, {"items": [3, 4]}, {"items": [5]}]
+    facts = summarize(rows)
+    assert any("Total rows" in f for f in facts)
+
+
+def test_summarize_list_of_dicts_categorical_max_facts() -> None:
+    rows = [{"status": s, "kind": k} for s, k in [("a", "x"), ("b", "y"), ("a", "z"), ("b", "x")]]
+    # max_facts=3: "Total rows" + "Top keys" + 1 categorical fact, then break
+    facts = summarize(rows, max_facts=3)
+    assert len(facts) <= 3
+
+
+def test_summarize_dict_list_value() -> None:
+    data = {"items": [1, 2, 3], "count": 3}
+    facts = summarize(data)
+    assert any("list of 3 items" in f for f in facts)
+
+
+def test_summarize_dict_other_value_type() -> None:
+    # Tuple is not int/float/str/list/dict — falls through to repr()
+    data = {"pair": (1, 2), "count": 1}
+    facts = summarize(data)
+    assert any("(1, 2)" in f for f in facts)
+
+
+def test_summarize_dict_max_facts() -> None:
+    data = {"a": 1, "b": 2, "c": 3}
+    facts = summarize(data, max_facts=2)
+    assert len(facts) <= 2