From a070ae9d2ef4a4d382b0bc22cac8f5e00ab75fc7 Mon Sep 17 00:00:00 2001 From: Artificium Date: Mon, 27 Jul 2026 23:43:27 +0000 Subject: [PATCH] fix(python): tolerate server response fields the SDK does not declare (#161) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Python SDK builds every response type with `Cls(**raw)`, so any key the server returns that the dataclass does not declare raises TypeError. The issue described this as an `explain`-only edge case. It is not: QueryResult has been broken on EVERY non-empty query response since v3.8, because all four warm-tier SELECT paths emit context_signals, epistemic_status and evidence_count unconditionally (all three are NOT NULL columns). Four more types are affected: MemoryHealth (3 fields, every /health call), SleepCycleResult (13, four of them on every /sleep call), ConsolidateResult (batchesProcessed, both return paths), and AgentStats when embeddings are enabled. Verified by running the pre-fix dataclasses against realistic current payloads — all raise. Two halves, both required. `from_response()` drops unknown keys so a future server field cannot break callers again; the missing fields are also declared with defaults so the new data is actually readable and an SDK newer than its server still parses. Filtering alone would silently discard every v3.8-v3.12 field. QueryResult.summary moves after `rank` so it can carry a default: shared-pool rows set `summary: pr.summary ?? undefined`, which JSON.stringify drops, so the key can be absent entirely. This changes positional construction, which is a real if narrow break — acceptable for an unpublished 0.1.0 deserialization type, and preferable to defaulting five fields that are always present. The Python SDK had zero tests and CI never ran Python, which is how a total breakage of client.query() survived five releases. Adds 65 tests covering realistic payloads, older-server payloads with the new fields absent, and an unknown-future-field guard per type — the test that would have caught this. Plus a CI job on the 3.10/3.13 bounds that pyproject claims to support. resilient.py is documented but unchanged: it converts these parse errors into silent empty results, which deserves its own decision. Fixes #161 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011xCqQo49d3CEbn6oEvb3Ru --- .github/workflows/ci.yml | 24 +++ python/memforge/__init__.py | 3 +- python/memforge/client.py | 22 +-- python/memforge/resilient.py | 9 + python/memforge/types.py | 88 ++++++++- python/tests/payloads.py | 271 ++++++++++++++++++++++++++++ python/tests/test_client_parsing.py | 150 +++++++++++++++ python/tests/test_types.py | 259 ++++++++++++++++++++++++++ 8 files changed, 811 insertions(+), 15 deletions(-) create mode 100644 python/tests/payloads.py create mode 100644 python/tests/test_client_parsing.py create mode 100644 python/tests/test_types.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c204e99..1a74fe8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,6 +61,30 @@ jobs: fi echo "dist/ OK — $(find dist -type f | wc -l) files produced" + test-python: + name: Python SDK tests (Python ${{ matrix.python }}) + # Parsing-layer tests only — no server, no Postgres. Runs in seconds and + # guards the SDK against server response fields it does not yet declare. + runs-on: ubuntu-latest + timeout-minutes: 10 + strategy: + fail-fast: false + matrix: + # Floor and ceiling of the range pyproject.toml claims to support. + python: ["3.10", "3.13"] + defaults: + run: + working-directory: python + steps: + - uses: actions/checkout@v7 + - uses: actions/setup-python@v6 + with: + python-version: ${{ matrix.python }} + cache: pip + cache-dependency-path: python/pyproject.toml + - run: pip install -e ".[dev]" + - run: pytest -q + test-security: name: Security tests (Node ${{ matrix.node }}) runs-on: ubuntu-latest diff --git a/python/memforge/__init__.py b/python/memforge/__init__.py index d4b3a63..579d0c3 100644 --- a/python/memforge/__init__.py +++ b/python/memforge/__init__.py @@ -20,7 +20,7 @@ from .types import ( AddResult, QueryResult, ConsolidateResult, ClearResult, AgentStats, MemoryHealth, ResumeContext, FeedbackResult, SleepCycleResult, - ReflectionResult, MemoryHints, + ReflectionResult, MemoryHints, from_response, ) __version__ = "0.1.0" @@ -41,4 +41,5 @@ "FeedbackResult", "SleepCycleResult", "ReflectionResult", + "from_response", ] diff --git a/python/memforge/client.py b/python/memforge/client.py index b3eb97c..1ec3f5e 100644 --- a/python/memforge/client.py +++ b/python/memforge/client.py @@ -20,7 +20,7 @@ from .types import ( AddResult, QueryResult, ConsolidateResult, ClearResult, AgentStats, MemoryHealth, ResumeContext, FeedbackResult, SleepCycleResult, - ReflectionResult, MemoryHints, + ReflectionResult, MemoryHints, from_response, ) @@ -112,7 +112,7 @@ async def add( if session_id: body["session_id"] = session_id raw = await self._post(f"/memory/{agent_id}/add", body) - return AddResult(**{k: raw[k] for k in ("id", "agent_id", "created_at") if k in raw}) + return from_response(AddResult, raw) async def query( self, @@ -155,7 +155,7 @@ async def query( if explain: params["explain"] = "true" raw = await self._get(f"/memory/{agent_id}/query", params) - return [QueryResult(**r) for r in raw] if isinstance(raw, list) else [] + return [from_response(QueryResult, r) for r in raw] if isinstance(raw, list) else [] async def timeline( self, @@ -196,12 +196,12 @@ async def consolidate( if target_namespace: body["target_namespace"] = target_namespace raw = await self._post(f"/memory/{agent_id}/consolidate", body) - return ConsolidateResult(**raw) + return from_response(ConsolidateResult, raw) async def clear(self, agent_id: str) -> ClearResult: """Archive all hot+warm memory to cold tier.""" raw = await self._post(f"/memory/{agent_id}/clear") - return ClearResult(**raw) + return from_response(ClearResult, raw) async def stats(self, agent_id: str, namespace: str | None = None) -> AgentStats: """Get memory tier statistics.""" @@ -209,7 +209,7 @@ async def stats(self, agent_id: str, namespace: str | None = None) -> AgentStats if namespace: params["namespace"] = namespace raw = await self._get(f"/memory/{agent_id}/stats", params or None) - return AgentStats(**raw) + return from_response(AgentStats, raw) # ── Knowledge Graph ────────────────────────────────────────────────── @@ -235,7 +235,7 @@ async def reflect( ) -> ReflectionResult: """Trigger LLM reflection on recent memories.""" raw = await self._post(f"/memory/{agent_id}/reflect", {"trigger": trigger, "limit": limit}) - return ReflectionResult(**raw) + return from_response(ReflectionResult, raw) async def get_reflections(self, agent_id: str, limit: int = 10) -> list[dict[str, Any]]: """Retrieve stored reflections.""" @@ -276,12 +276,12 @@ async def sleep( if include_reflection is not None: body["includeReflection"] = include_reflection raw = await self._post(f"/memory/{agent_id}/sleep", body) - return SleepCycleResult(**raw) + return from_response(SleepCycleResult, raw) async def memory_health(self, agent_id: str) -> MemoryHealth: """Get memory health metrics.""" raw = await self._get(f"/memory/{agent_id}/health") - return MemoryHealth(**raw) + return from_response(MemoryHealth, raw) # ── Epistemic Confidence Model (v3.9) ───────────────────────────────── @@ -412,7 +412,7 @@ async def resume(self, agent_id: str, limit: int = 5, namespace: str | None = No if namespace: params["namespace"] = namespace raw = await self._get(f"/memory/{agent_id}/resume", params) - return ResumeContext(**raw) + return from_response(ResumeContext, raw) # ── Feedback ───────────────────────────────────────────────────────── @@ -428,7 +428,7 @@ async def feedback( if metadata: body["metadata"] = metadata raw = await self._post(f"/memory/{agent_id}/feedback", body) - return FeedbackResult(**raw) + return from_response(FeedbackResult, raw) async def active_recall(self, agent_id: str, context: str, limit: int = 5) -> dict[str, Any]: """Proactively surface relevant memories for a context.""" diff --git a/python/memforge/resilient.py b/python/memforge/resilient.py index 0f480ac..f5c22f8 100644 --- a/python/memforge/resilient.py +++ b/python/memforge/resilient.py @@ -30,6 +30,15 @@ class ResilientMemForgeClient: On failure, returns safe defaults (empty lists, None, zeroed stats) and optionally calls an ``on_error`` callback. + + Note that "failure" here includes *parse* failures, not just transport + ones: a response this SDK cannot deserialize is reported to ``on_error`` + and then reads to the caller as an empty result, indistinguishable from + the server having no data. That masking is how issue #161 stayed hidden + — every query against a v3.8+ server returned ``[]`` rather than + raising. It is deliberately left as-is here; narrowing the caught + exceptions is a behavior change for every existing caller and belongs in + its own change. """ def __init__( diff --git a/python/memforge/types.py b/python/memforge/types.py index 4656b01..75a8ccd 100644 --- a/python/memforge/types.py +++ b/python/memforge/types.py @@ -1,8 +1,42 @@ -"""MemForge Python SDK — type definitions.""" +"""MemForge Python SDK — type definitions. + +The server grows its response payloads in minor releases: a memory row that +carried 8 keys in v3.7 carries 12 in v3.12. Two rules keep this SDK usable +across that drift, and both are load-bearing: + +1. Every response dataclass is built through :func:`from_response`, which + drops keys the dataclass does not declare. Without it a single new server + field raises ``TypeError`` on *every* call to the affected endpoint — + which is exactly how issue #161 shipped. +2. Every field the server added after the dataclass was first written carries + a default, so an SDK running against an *older* server still parses. The + defaults are chosen to match what the server's omission means, not merely + to be falsy — see the per-field notes below. + +Together these make dataclass and server independently upgradable in either +direction. Filtering alone would silently discard real data; adding fields +alone would leave the next server release to break callers again. +""" from __future__ import annotations + +import dataclasses from dataclasses import dataclass, field -from typing import Any, Optional +from typing import Any, Mapping, Optional, TypeVar + +T = TypeVar("T") + + +def from_response(cls: type[T], raw: Mapping[str, Any]) -> T: + """Build a response dataclass from a server payload, ignoring unknown keys. + + The server adds response fields in minor releases; an SDK that rejects + them breaks every caller on upgrade. Keys absent from ``raw`` fall back + to the dataclass default, which is how this SDK stays compatible with + servers older than itself. + """ + known = {f.name for f in dataclasses.fields(cls)} # type: ignore[arg-type] + return cls(**{k: v for k, v in raw.items() if k in known}) @dataclass @@ -10,6 +44,7 @@ class AddResult: id: int agent_id: str created_at: str + # Only emitted when the write collapsed onto an existing hot-tier row. deduplicated: bool = False @@ -17,13 +52,31 @@ class AddResult: class QueryResult: id: int content: str - summary: Optional[str] metadata: dict[str, Any] consolidated_at: str time_start: Optional[str] time_end: Optional[str] rank: float + # `summary` is only populated under LLM consolidation, and shared-pool + # rows omit the key entirely, so it cannot be a required argument. + summary: Optional[str] = None + + # v3.8 — sentiment/urgency/session_type merged from contributing hot rows. + # Absent means "no signals recorded", which is what an empty dict says. + context_signals: dict[str, Any] = field(default_factory=dict) + + # v3.9 — calibrated uncertainty. None means the server did not report a + # status; it is not the same as any of the five status values. + epistemic_status: Optional[str] = None + # v3.9 — corroborating retrievals. The server's floor is 1, so 0 would be + # a lie; None means "not reported". + evidence_count: Optional[int] = None + + # v3.10 — per-result rank factors, present only when query(explain=True). + # None distinguishes "not requested" from "requested, no factors". + explanation: Optional[list[dict[str, Any]]] = None + @dataclass class ConsolidateResult: @@ -54,6 +107,10 @@ class AgentStats: last_consolidation: Optional[str] last_seen: Optional[str] + # v3.4 — warm rows awaiting re-embedding under the current model. Omitted + # when embeddings are disabled, where the answer is "unknown", not zero. + stale_embedding_count: Optional[int] = None + @dataclass class MemoryHealth: @@ -68,6 +125,12 @@ class MemoryHealth: retrieval_count_24h: int contradiction_rate: float + # v2.6 — staleness and knowledge-gap tracking. Always sent by v2.6+ + # servers; the zero defaults cover pre-v2.6 ones. + stale_memory_count: int = 0 + avg_staleness: float = 0.0 + knowledge_gap_count_7d: int = 0 + @dataclass class ResumeContext: @@ -100,6 +163,25 @@ class SleepCycleResult: tokens_used: int duration_ms: int + # Counters the engine always emits. Zero is the correct reading both when + # the phase did no work and when the server predates the phase. + phase5b_cold_purged: int = 0 + schemas_detected: int = 0 + conflicts_resolved: int = 0 + audit_records_archived: int = 0 + + # Counters the engine emits only when non-zero — it assigns each key + # post-hoc behind `if counter > 0`, so an omitted key means exactly 0. + capacity_evicted: int = 0 + temporal_expired: int = 0 + procedures_evolved: int = 0 + embeddings_migrated: int = 0 + embeddings_migration_backlog: int = 0 + deprecated_decayed: int = 0 + epistemic_promoted: int = 0 # v3.9 — Sleep Phase 5.12 + causal_edges_updated: int = 0 # v3.10 — Sleep Phase 6.1 + principles_extracted: int = 0 # v3.11 — Sleep Phase 5.11 + @dataclass class ReflectionResult: diff --git a/python/tests/payloads.py b/python/tests/payloads.py new file mode 100644 index 0000000..e9fd98e --- /dev/null +++ b/python/tests/payloads.py @@ -0,0 +1,271 @@ +"""Server response fixtures for the SDK parsing tests. + +Two payloads exist per dataclass and both matter: + +``CURRENT`` mirrors what a v3.12 server actually puts on the wire, copied +from the SELECT lists and result literals in ``src/memory-manager.ts`` and +``src/sleep-cycle.ts``. It is the payload that broke the SDK in issue #161, +so it is the payload the tests must parse. + +``LEGACY`` is the same response as an older server sent it, with every +later-added key removed. It proves the field defaults are reachable, which +is what lets one SDK version talk to servers on either side of it. +""" + +from __future__ import annotations + +from typing import Any + +# ── AddResult — POST /memory/:id/add ───────────────────────────────────────── + +ADD_RESULT_CURRENT: dict[str, Any] = { + "id": 90211, + "agent_id": "agent-1", + "created_at": "2026-07-20T18:04:11.223Z", + "deduplicated": True, +} + +ADD_RESULT_LEGACY: dict[str, Any] = { + "id": 90211, + "agent_id": "agent-1", + "created_at": "2026-07-20T18:04:11.223Z", +} + +# ── QueryResult — GET /memory/:id/query ────────────────────────────────────── + +QUERY_RESULT_CURRENT: dict[str, Any] = { + "id": 4211, + "content": "User prefers dark mode in the terminal and dislikes light themes.", + "summary": "Dark mode preference", + "metadata": {"source": "chat", "turn": 12}, + "consolidated_at": "2026-07-20T18:04:11.223Z", + "time_start": "2026-07-20T17:55:00.000Z", + "time_end": "2026-07-20T18:02:00.000Z", + "context_signals": { + "urgency": "low", + "sentiment": "positive", + "session_type": "explore", + }, + "epistemic_status": "established", + "evidence_count": 4, + "rank": 0.8734, + "explanation": [ + { + "name": "epistemic_status", + "weight": 1.0, + "detail": "Status: established, evidence count: 4", + }, + {"name": "keyword_overlap", "weight": 0.3, "detail": "2 of 3 query tokens"}, + ], +} + +QUERY_RESULT_LEGACY: dict[str, Any] = { + "id": 4211, + "content": "User prefers dark mode in the terminal and dislikes light themes.", + "summary": "Dark mode preference", + "metadata": {"source": "chat", "turn": 12}, + "consolidated_at": "2026-07-20T18:04:11.223Z", + "time_start": None, + "time_end": None, + "rank": 0.8734, +} + +# Shared-pool rows take a different code path (src/memory-manager.ts mergePool +# results): the literal omits the v3.8/v3.9 keys outright, and sets summary to +# `undefined` when the source row has none — which JSON.stringify drops, so the +# key never reaches the client. +QUERY_RESULT_POOL_ROW: dict[str, Any] = { + "id": 771, + "content": "Deploys to prod are frozen on Fridays.", + "metadata": { + "_from_pool": "team-alpha", + "_source_agent": "agent-9", + "_trust_score": 0.612, + }, + "consolidated_at": "2026-07-19T09:00:00.000Z", + "time_start": None, + "time_end": None, + "rank": 0.4102, +} + +# ── ConsolidateResult — POST /memory/:id/consolidate ───────────────────────── + +CONSOLIDATE_RESULT_CURRENT: dict[str, Any] = { + "run_id": 88, + "agent_id": "agent-1", + "hot_rows_processed": 140, + "warm_rows_created": 12, + "consolidation_mode": "concat", + "status": "complete", +} + +# ── ClearResult — POST /memory/:id/clear ───────────────────────────────────── + +CLEAR_RESULT_CURRENT: dict[str, Any] = { + "agent_id": "agent-1", + "hot_archived": 140, + "warm_archived": 12, +} + +# ── AgentStats — GET /memory/:id/stats ─────────────────────────────────────── + +AGENT_STATS_CURRENT: dict[str, Any] = { + "agent_id": "agent-1", + "hot_count": 140, + "warm_count": 812, + "cold_count": 3301, + "entity_count": 64, + "relationship_count": 91, + "reflection_count": 7, + "last_consolidation": "2026-07-20T18:04:11.223Z", + "last_seen": "2026-07-20T18:30:00.000Z", + "stale_embedding_count": 45, +} + +AGENT_STATS_LEGACY: dict[str, Any] = { + "agent_id": "agent-1", + "hot_count": 140, + "warm_count": 812, + "cold_count": 3301, + "entity_count": 64, + "relationship_count": 91, + "reflection_count": 7, + "last_consolidation": None, + "last_seen": "2026-07-20T18:30:00.000Z", +} + +# ── MemoryHealth — GET /memory/:id/health ──────────────────────────────────── + +MEMORY_HEALTH_CURRENT: dict[str, Any] = { + "agent_id": "agent-1", + "total_memories": 812, + "avg_importance": 0.41, + "avg_confidence": 0.77, + "memories_below_eviction": 9, + "memories_below_revision": 31, + "revision_velocity_24h": 4, + "knowledge_stability_pct": 96.2, + "retrieval_count_24h": 210, + "contradiction_rate": 0.03, + "stale_memory_count": 22, + "avg_staleness": 0.18, + "knowledge_gap_count_7d": 6, +} + +MEMORY_HEALTH_LEGACY: dict[str, Any] = { + "agent_id": "agent-1", + "total_memories": 812, + "avg_importance": 0.41, + "avg_confidence": 0.77, + "memories_below_eviction": 9, + "memories_below_revision": 31, + "revision_velocity_24h": 4, + "knowledge_stability_pct": 96.2, + "retrieval_count_24h": 210, + "contradiction_rate": 0.03, +} + +# ── ResumeContext — GET /memory/:id/resume ─────────────────────────────────── + +RESUME_CONTEXT_CURRENT: dict[str, Any] = { + "agent_id": "agent-1", + "time_since_last_activity_ms": 3_600_000, + "top_memories": [ + { + "id": 4211, + "content": "User prefers dark mode.", + "importance": 0.91, + "consolidated_at": "2026-07-20T18:04:11.223Z", + } + ], + "active_procedures": [ + {"condition": "user reports a crash", "action": "ask for the stack trace", "confidence": 0.82} + ], + "open_contradictions": ["prefers dark mode vs. asked for light theme on 2026-06-02"], + "memory_health": {"total_memories": 812, "avg_importance": 0.41, "avg_confidence": 0.77}, +} + +# ── FeedbackResult — POST /memory/:id/feedback ─────────────────────────────── + +FEEDBACK_RESULT_CURRENT: dict[str, Any] = { + "agent_id": "agent-1", + "updated": 3, + "outcome": "positive", +} + +# ── SleepCycleResult — POST /memory/:id/sleep ──────────────────────────────── + +# A busy cycle: every conditional counter came back non-zero, so the server +# emits all 24 keys. +SLEEP_RESULT_CURRENT: dict[str, Any] = { + "agent_id": "agent-1", + "phase1_scores_updated": 812, + "phase2_evicted": 9, + "phase2_flagged_for_revision": 31, + "phase3_revised": 5, + "phase3_skipped": 26, + "phase4_edges_invalidated": 2, + "phase4_entities_merged": 3, + "phase5_reflection": True, + "phase5b_cold_purged": 120, + "schemas_detected": 4, + "conflicts_resolved": 2, + "audit_records_archived": 500, + "tokens_used": 18_400, + "duration_ms": 9_215, + "capacity_evicted": 14, + "temporal_expired": 6, + "procedures_evolved": 3, + "embeddings_migrated": 100, + "embeddings_migration_backlog": 712, + "deprecated_decayed": 8, + "epistemic_promoted": 11, + "causal_edges_updated": 27, + "principles_extracted": 2, +} + +# A quiet cycle on the same v3.12 server: the engine assigns the optional +# counters only behind `if counter > 0`, so they are absent from the wire. +SLEEP_RESULT_QUIET_CYCLE: dict[str, Any] = { + "agent_id": "agent-1", + "phase1_scores_updated": 12, + "phase2_evicted": 0, + "phase2_flagged_for_revision": 0, + "phase3_revised": 0, + "phase3_skipped": 0, + "phase4_edges_invalidated": 0, + "phase4_entities_merged": 0, + "phase5_reflection": False, + "phase5b_cold_purged": 0, + "schemas_detected": 0, + "conflicts_resolved": 0, + "audit_records_archived": 0, + "tokens_used": 0, + "duration_ms": 41, +} + +SLEEP_RESULT_LEGACY: dict[str, Any] = { + "agent_id": "agent-1", + "phase1_scores_updated": 12, + "phase2_evicted": 0, + "phase2_flagged_for_revision": 0, + "phase3_revised": 0, + "phase3_skipped": 0, + "phase4_edges_invalidated": 0, + "phase4_entities_merged": 0, + "phase5_reflection": False, + "tokens_used": 0, + "duration_ms": 41, +} + +# ── ReflectionResult — POST /memory/:id/reflect ────────────────────────────── + +REFLECTION_RESULT_CURRENT: dict[str, Any] = { + "id": 19, + "agent_id": "agent-1", + "insights_count": 4, + "contradictions_count": 1, + "source_memories_reviewed": 20, + "trigger_type": "manual", + "reflection_level": 1, +} diff --git a/python/tests/test_client_parsing.py b/python/tests/test_client_parsing.py new file mode 100644 index 0000000..ab9c2ae --- /dev/null +++ b/python/tests/test_client_parsing.py @@ -0,0 +1,150 @@ +"""End-to-end parse tests for the typed client methods (issue #161). + +``test_types`` proves the dataclasses tolerate the current server. These +prove the client actually routes through that tolerant path — the bug was +not in the dataclasses alone but in ``client.py`` calling ``Cls(**raw)``, so +a fix applied only to ``types.py`` would leave every one of these red. + +The transport is stubbed at ``_get``/``_post``: the HTTP layer is httpx's +problem, and the defect under test lives strictly above it. No network, no +event-loop plugin — each test drives the coroutine with ``asyncio.run``. +""" + +from __future__ import annotations + +import asyncio + +import pytest + +from memforge import MemForgeClient + +import payloads + + +@pytest.fixture +def client(): + c = MemForgeClient(base_url="http://memforge.test", token="test-token") + yield c + asyncio.run(c.close()) + + +def serve(client, payload): + """Answer every request on this client with ``payload``.""" + + async def _get(path, params=None): + return payload + + async def _post(path, body=None): + return payload + + client._get = _get + client._post = _post + + +def test_query_parses_current_server_row(client): + """The exact failure in #161: a v3.8+ row raised TypeError inside + `[QueryResult(**r) for r in raw]`, breaking every non-empty query.""" + serve(client, [payloads.QUERY_RESULT_CURRENT]) + + results = asyncio.run(client.query("agent-1", q="preferences")) + + assert len(results) == 1 + assert results[0].epistemic_status == "established" + assert results[0].context_signals["sentiment"] == "positive" + + +def test_query_parses_shared_pool_row(client): + serve(client, [payloads.QUERY_RESULT_POOL_ROW]) + + results = asyncio.run(client.query("agent-1", q="deploys")) + + assert results[0].summary is None + + +def test_query_returns_empty_list_for_empty_response(client): + serve(client, []) + + assert asyncio.run(client.query("agent-1", q="nothing")) == [] + + +def test_query_ignores_unknown_future_row_key(client): + serve(client, [{**payloads.QUERY_RESULT_CURRENT, "some_field_from_v9": 1}]) + + results = asyncio.run(client.query("agent-1", q="preferences")) + + assert results[0].id == payloads.QUERY_RESULT_CURRENT["id"] + + +def test_sleep_parses_current_server_result(client): + serve(client, payloads.SLEEP_RESULT_CURRENT) + + result = asyncio.run(client.sleep("agent-1")) + + assert result.conflicts_resolved == 2 + assert result.principles_extracted == 2 + + +def test_memory_health_parses_current_server_result(client): + serve(client, payloads.MEMORY_HEALTH_CURRENT) + + result = asyncio.run(client.memory_health("agent-1")) + + assert result.knowledge_gap_count_7d == 6 + + +def test_stats_parses_current_server_result(client): + serve(client, payloads.AGENT_STATS_CURRENT) + + result = asyncio.run(client.stats("agent-1")) + + assert result.stale_embedding_count == 45 + + +def test_add_surfaces_deduplicated_flag(client): + """The old hand-rolled key filter in add() dropped `deduplicated`, so + callers could not tell a real write from a collapsed duplicate.""" + serve(client, payloads.ADD_RESULT_CURRENT) + + result = asyncio.run(client.add("agent-1", "User prefers dark mode")) + + assert result.deduplicated is True + + +def test_resume_parses_current_server_result(client): + serve(client, payloads.RESUME_CONTEXT_CURRENT) + + result = asyncio.run(client.resume("agent-1")) + + assert result.time_since_last_activity_ms == 3_600_000 + + +def test_reflect_parses_current_server_result(client): + serve(client, payloads.REFLECTION_RESULT_CURRENT) + + result = asyncio.run(client.reflect("agent-1")) + + assert result.insights_count == 4 + + +def test_consolidate_parses_current_server_result(client): + serve(client, payloads.CONSOLIDATE_RESULT_CURRENT) + + result = asyncio.run(client.consolidate("agent-1")) + + assert result.warm_rows_created == 12 + + +def test_clear_parses_current_server_result(client): + serve(client, payloads.CLEAR_RESULT_CURRENT) + + result = asyncio.run(client.clear("agent-1")) + + assert result.hot_archived == 140 + + +def test_feedback_parses_current_server_result(client): + serve(client, payloads.FEEDBACK_RESULT_CURRENT) + + result = asyncio.run(client.feedback("agent-1", [1, 2, 3], "positive")) + + assert result.updated == 3 diff --git a/python/tests/test_types.py b/python/tests/test_types.py new file mode 100644 index 0000000..66f3ac7 --- /dev/null +++ b/python/tests/test_types.py @@ -0,0 +1,259 @@ +"""Parsing-layer tests for the response dataclasses (issue #161). + +Three properties are asserted here, and the SDK is only compatible across +server versions if all three hold: + +* a payload from the *current* server parses and exposes its new fields; +* a payload from an *older* server parses, falling back to defaults; +* a payload from a *future* server parses, ignoring keys we do not know. + +The third is the regression guard for #161 proper. The SDK used to build +these with ``Cls(**raw)``, so the day the server grew ``context_signals`` +every non-empty query response raised ``TypeError``. +""" + +from __future__ import annotations + +import dataclasses + +import pytest + +from memforge.types import ( + AddResult, + AgentStats, + ClearResult, + ConsolidateResult, + FeedbackResult, + MemoryHealth, + QueryResult, + ReflectionResult, + ResumeContext, + SleepCycleResult, + from_response, +) + +import payloads + +# Every response dataclass paired with a realistic current-server payload. +# Parametrizing the cross-cutting tests off one table means a new dataclass +# gets the unknown-key guard the moment it is added here. +CURRENT_PAYLOADS = [ + (AddResult, payloads.ADD_RESULT_CURRENT), + (QueryResult, payloads.QUERY_RESULT_CURRENT), + (ConsolidateResult, payloads.CONSOLIDATE_RESULT_CURRENT), + (ClearResult, payloads.CLEAR_RESULT_CURRENT), + (AgentStats, payloads.AGENT_STATS_CURRENT), + (MemoryHealth, payloads.MEMORY_HEALTH_CURRENT), + (ResumeContext, payloads.RESUME_CONTEXT_CURRENT), + (FeedbackResult, payloads.FEEDBACK_RESULT_CURRENT), + (SleepCycleResult, payloads.SLEEP_RESULT_CURRENT), + (ReflectionResult, payloads.REFLECTION_RESULT_CURRENT), +] + +CURRENT_IDS = [cls.__name__ for cls, _ in CURRENT_PAYLOADS] + +# The same endpoints as an older server answered them, with every +# later-added key stripped. +LEGACY_PAYLOADS = [ + (AddResult, payloads.ADD_RESULT_LEGACY), + (QueryResult, payloads.QUERY_RESULT_LEGACY), + (AgentStats, payloads.AGENT_STATS_LEGACY), + (MemoryHealth, payloads.MEMORY_HEALTH_LEGACY), + (SleepCycleResult, payloads.SLEEP_RESULT_LEGACY), +] + +LEGACY_IDS = [cls.__name__ for cls, _ in LEGACY_PAYLOADS] + + +# ── Cross-cutting guards ───────────────────────────────────────────────────── + +@pytest.mark.parametrize("cls,payload", CURRENT_PAYLOADS, ids=CURRENT_IDS) +def test_current_server_payload_parses(cls, payload): + parsed = from_response(cls, payload) + + assert isinstance(parsed, cls) + + +@pytest.mark.parametrize("cls,payload", CURRENT_PAYLOADS, ids=CURRENT_IDS) +def test_current_server_payload_populates_every_declared_field(cls, payload): + """No key the server sends may be silently dropped on the floor. + + A filter that ignores unknown keys is only safe if the dataclass has in + fact caught up with the server. This asserts the other half of the fix: + every field the current server sends is declared and carries the sent + value, so filtering discards nothing real. + """ + declared = {f.name for f in dataclasses.fields(cls)} + undeclared = sorted(set(payload) - declared) + assert undeclared == [], f"{cls.__name__} does not declare: {undeclared}" + + parsed = from_response(cls, payload) + + for key, value in payload.items(): + assert getattr(parsed, key) == value, f"{cls.__name__}.{key} did not round-trip" + + +@pytest.mark.parametrize("cls,payload", CURRENT_PAYLOADS, ids=CURRENT_IDS) +def test_unknown_future_field_does_not_raise(cls, payload): + """The guard that would have prevented #161.""" + future = {**payload, "some_field_from_v9": 1, "another_field_from_v9": {"a": "b"}} + + parsed = from_response(cls, future) + + assert not hasattr(parsed, "some_field_from_v9") + + +@pytest.mark.parametrize("cls,payload", LEGACY_PAYLOADS, ids=LEGACY_IDS) +def test_older_server_payload_parses(cls, payload): + """Fields the server added later must carry defaults, or an SDK newer + than the server it talks to raises TypeError on a missing argument.""" + parsed = from_response(cls, payload) + + assert isinstance(parsed, cls) + + +# ── QueryResult — the dataclass #161 was filed against ─────────────────────── + +def test_query_result_exposes_v38_context_signals(): + parsed = from_response(QueryResult, payloads.QUERY_RESULT_CURRENT) + + assert parsed.context_signals == { + "urgency": "low", + "sentiment": "positive", + "session_type": "explore", + } + + +def test_query_result_exposes_v39_epistemic_fields(): + parsed = from_response(QueryResult, payloads.QUERY_RESULT_CURRENT) + + assert parsed.epistemic_status == "established" + assert parsed.evidence_count == 4 + + +def test_query_result_exposes_v310_explanation(): + parsed = from_response(QueryResult, payloads.QUERY_RESULT_CURRENT) + + assert [f["name"] for f in parsed.explanation] == ["epistemic_status", "keyword_overlap"] + + +def test_query_result_explanation_is_none_when_explain_not_requested(): + """None distinguishes "explain=false" from "explained, but no factors".""" + parsed = from_response(QueryResult, payloads.QUERY_RESULT_LEGACY) + + assert parsed.explanation is None + + +def test_query_result_defaults_context_signals_to_empty_dict_on_older_server(): + parsed = from_response(QueryResult, payloads.QUERY_RESULT_LEGACY) + + assert parsed.context_signals == {} + + +def test_query_result_leaves_epistemic_fields_none_on_older_server(): + """0 would be a lie for evidence_count — the server's floor is 1.""" + parsed = from_response(QueryResult, payloads.QUERY_RESULT_LEGACY) + + assert parsed.epistemic_status is None + assert parsed.evidence_count is None + + +def test_query_result_parses_shared_pool_row_without_summary(): + """Pool rows serialize summary as `undefined`, which never reaches the + wire — so `summary` cannot be a required argument.""" + parsed = from_response(QueryResult, payloads.QUERY_RESULT_POOL_ROW) + + assert parsed.summary is None + assert parsed.metadata["_from_pool"] == "team-alpha" + + +# ── SleepCycleResult ───────────────────────────────────────────────────────── + +def test_sleep_result_exposes_always_emitted_counters(): + parsed = from_response(SleepCycleResult, payloads.SLEEP_RESULT_CURRENT) + + assert parsed.phase5b_cold_purged == 120 + assert parsed.schemas_detected == 4 + assert parsed.conflicts_resolved == 2 + assert parsed.audit_records_archived == 500 + + +def test_sleep_result_exposes_conditional_counters(): + parsed = from_response(SleepCycleResult, payloads.SLEEP_RESULT_CURRENT) + + assert parsed.capacity_evicted == 14 + assert parsed.temporal_expired == 6 + assert parsed.procedures_evolved == 3 + assert parsed.embeddings_migrated == 100 + assert parsed.embeddings_migration_backlog == 712 + assert parsed.deprecated_decayed == 8 + assert parsed.epistemic_promoted == 11 + assert parsed.causal_edges_updated == 27 + assert parsed.principles_extracted == 2 + + +def test_sleep_result_conditional_counters_default_to_zero_on_quiet_cycle(): + """The engine omits each of these behind `if counter > 0`, so an absent + key means exactly zero — not unknown.""" + parsed = from_response(SleepCycleResult, payloads.SLEEP_RESULT_QUIET_CYCLE) + + assert parsed.capacity_evicted == 0 + assert parsed.epistemic_promoted == 0 + assert parsed.principles_extracted == 0 + + +def test_sleep_result_parses_older_server_response(): + parsed = from_response(SleepCycleResult, payloads.SLEEP_RESULT_LEGACY) + + assert parsed.phase5b_cold_purged == 0 + assert parsed.schemas_detected == 0 + assert parsed.conflicts_resolved == 0 + assert parsed.audit_records_archived == 0 + + +# ── MemoryHealth ───────────────────────────────────────────────────────────── + +def test_memory_health_exposes_staleness_fields(): + parsed = from_response(MemoryHealth, payloads.MEMORY_HEALTH_CURRENT) + + assert parsed.stale_memory_count == 22 + assert parsed.avg_staleness == pytest.approx(0.18) + assert parsed.knowledge_gap_count_7d == 6 + + +def test_memory_health_parses_older_server_response(): + parsed = from_response(MemoryHealth, payloads.MEMORY_HEALTH_LEGACY) + + assert parsed.stale_memory_count == 0 + assert parsed.avg_staleness == 0.0 + assert parsed.knowledge_gap_count_7d == 0 + + +# ── AgentStats ─────────────────────────────────────────────────────────────── + +def test_agent_stats_exposes_stale_embedding_count(): + parsed = from_response(AgentStats, payloads.AGENT_STATS_CURRENT) + + assert parsed.stale_embedding_count == 45 + + +def test_agent_stats_stale_embedding_count_is_none_when_embeddings_disabled(): + """The server omits the key entirely rather than sending 0, because with + embeddings off the answer is unknown, not zero.""" + parsed = from_response(AgentStats, payloads.AGENT_STATS_LEGACY) + + assert parsed.stale_embedding_count is None + + +# ── AddResult ──────────────────────────────────────────────────────────────── + +def test_add_result_exposes_deduplicated_flag(): + parsed = from_response(AddResult, payloads.ADD_RESULT_CURRENT) + + assert parsed.deduplicated is True + + +def test_add_result_deduplicated_defaults_false_when_key_absent(): + parsed = from_response(AddResult, payloads.ADD_RESULT_LEGACY) + + assert parsed.deduplicated is False