fix(sessions): address Volva code-vs-contract drift (issue #2)

Volva's code-spec review (thread 01KS4EKVKKGF) surfaced three findings
on the TDD-passing sessions module. All three addressed; one carries
a collateral contract amendment to keep INV-002 truthful.

1) drift: archived=item.get("archived", False) returned None for an
explicit "archived": null in the response. dict.get(k, default) only
fires the default when the key is absent — it does NOT default for
explicit-null values. The dataclass type is `bool` (not `bool | None`)
and INV-002 says explicit-null → False; the .get() form silently
violated both. Fixed: archived=item.get("archived") or False
(handles absent, null, false, and true cleanly).

INV-002 wording was the source of the bug — I introduced the
mis-spelled form during the Volva amendment round. Updated to spell
out the .get(default) foot-gun explicitly so future readers (and
future paraphrase rounds) don't fall back to the broken pattern.

2) test-gap: no test exercised explicit-null archived/tags. The
_list_item() helper had its own defaulting layer (tags=None →
["work"]) so a happy path test couldn't catch the underlying drift.
Added test_explicit_null_list_defaults using a raw dict to bypass
the helper. Catches the drift directly.

3) precision: message_count=body.get("message_count") could silently
default to None while POST-003 required it non-None. INV-001 prose
literally said "body['message_count']" (bracket access) so the
STEP 5 .get() was the contract's own internal inconsistency.
Aligned the code to bracket access (matches sibling required
fields like session_id) and amended STEP 5 + INV-001 to spell out
the strict semantics explicitly.

Volva's meta-note: "modest weight" — TDD caught the main surface;
this round caught a narrow Python .get() semantics edge that no
human reading would have spotted without explicit-null priors.
Still pulls real weight: that's the kind of bug that ships and
shows up months later when a server starts emitting null where
it used to omit a field.

63 tests GREEN (42 sse_client + 20 sessions + 1 boundary).
Ruff clean. Drift check still GREEN against the pinned issue body.

docs/contracts/issues/2.contract.md

@@ -69,8 +69,8 @@ Convention-aligned with issue #1: caller owns the `httpx.AsyncClient` and Author
 
 ## Invariants
 
-- **INV-001 [hard]**: `create_session` returns a `SessionInfo` whose `session_id`, `agent_id`, `created_at`, `last_active`, and `metadata` are sourced from the 201 response body. `metadata` is taken from `body["metadata"]` when present and defaults to `{}` when absent (defensive against minor server-side spec drift; spec example always shows it present). `message_count` is taken from `body["message_count"]` (typically 0 for a fresh session). List-only fields are fixed: `name=None`, `archived=False`, `tags=[]`.
-- **INV-002 [hard]**: `list_sessions` returns a `SessionPage` where every `SessionInfo` has `session_id`, `agent_id`, `created_at`, `last_active`, and `metadata` from the response item (same defensive `metadata` default as INV-001). `name` is `item.get("name")` (may be `None`). `archived` is `item.get("archived", False)` (absent or explicit-null both yield `False`). `tags` is `item.get("tags") or []` (absent, explicit-null, or empty list all yield `[]`; a populated list passes through). `message_count` is `None` (the list endpoint does not include it — spec §GET /sessions: "`message_count` is not included in list items").
+- **INV-001 [hard]**: `create_session` returns a `SessionInfo` whose `session_id`, `agent_id`, `created_at`, `last_active`, and `metadata` are sourced from the 201 response body. `metadata` is taken from `body["metadata"]` when present and defaults to `{}` when absent (defensive against minor server-side spec drift; spec example always shows it present). `message_count` is taken from `body["message_count"]` (strict — bracket access, not `.get()`; the spec lists it as a response field and absent should surface as KeyError rather than silently default to None). List-only fields are fixed: `name=None`, `archived=False`, `tags=[]`.
+- **INV-002 [hard]**: `list_sessions` returns a `SessionPage` where every `SessionInfo` has `session_id`, `agent_id`, `created_at`, `last_active`, and `metadata` from the response item (same defensive `metadata` default as INV-001). `name` is `item.get("name")` (may be `None`). `archived` is `item.get("archived") or False` — absent, explicit-null, or explicit-false all yield `False`; explicit-true passes through. (Note: `item.get(key, default)` only fires `default` for absent keys, NOT for explicit-null values, so the `or False` form is load-bearing here.) `tags` is `item.get("tags") or []` (absent, explicit-null, or empty list all yield `[]`; a populated list passes through). `message_count` is `None` (the list endpoint does not include it — spec §GET /sessions: "`message_count` is not included in list items").
 - **INV-003 [hard]**: `list_sessions` treats cursors as opaque strings. The module never parses, base64-decodes, or constructs a cursor — it threads the server-provided `next_cursor` back verbatim on the next call. Per spec §Pagination ("Cursors are opaque to clients — do not parse or construct them.").
 - **INV-004 [hard]**: Both functions truncate exception `.body` payloads to `[:1024]` at construction. Matches the issue #1 precedent (`SseConnectFailed`, `CancelFailed`).
 - **INV-005 [hard]**: No `core.*` or `worldtree.*` imports. Boundary verified by `tests/test_no_worldtree_imports.py`.
@@ -127,7 +127,7 @@ STEPS:
         created_at=body["created_at"],
         last_active=body["last_active"],
         metadata=body.get("metadata", {}),     # INV-001 defensive default
-        message_count=body.get("message_count"),
+        message_count=body["message_count"],   # INV-001/POST-003: required, never defaulted
         name=None,                             # INV-001 fixed for create-origin
         archived=False,                        # INV-001 fixed for create-origin
         tags=[],                               # INV-001 fixed for create-origin
@@ -183,7 +183,7 @@ STEPS:
         metadata=item.get("metadata", {}),       # INV-002 defensive default
         message_count=None,                      # not in list response per spec
         name=item.get("name"),                   # INV-002: may be None
-        archived=item.get("archived", False),    # INV-002: absent/null → False
+        archived=item.get("archived") or False,  # INV-002: absent/null/false → False (the `or` form is load-bearing — .get(k, default) does not fire default on explicit null)
         tags=item.get("tags") or [],             # INV-002: absent/null/[] → []
      )
   7. [cleanup] RETURN SessionPage(items=infos, next_cursor=body.get("next_cursor"))

persistent-memory.md

@@ -83,6 +83,7 @@ decision. Captures rationale that won't be obvious from code alone.
 - `[2026-05-21]` **`ratatoskr.sse_client` implemented via TDD against issue #1's contract.** 37 contract-listed tests authored + GREEN per the tracer-bullet vertical-slice ordering (`_parse_sse_id` → `stream_turn` → `reconnect_turn` → `cancel_turn`). Refactor pass extracted `_iter_events` helper to dedupe INV-002 + INV-003 + terminal-break logic across `stream_turn` and `reconnect_turn`; `expected_turn_id=None` vs `expected_turn_id=N` distinguishes the two entry-point semantics Volva surfaced. Notable choices made during implementation: (a) regex `^-?\d+$` pre-check in `_parse_sse_id` to reject whitespace before `int()` (Python's `int(" 3 ")` would silently strip — this kept the strict-no-whitespace test honest); (b) `_DropAfter` AsyncByteStream subclass in tests to simulate mid-stream `RemoteProtocolError`; (c) ToolResult.result and ToolStart.arguments typed as `Any` (server JSON varies); (d) ruff line-length=100 (per pyproject) forced some test docstrings to be tighter than v0 draft.
 - `[2026-05-21]` **Issue #2 + contract: `ratatoskr.sessions`.** Scope is narrow — `create_session` (POST /sessions) + `list_sessions` (GET /sessions, cursor-paginated) + shared `SessionInfo` and `SessionPage` frozen dataclasses. Bundles two endpoints in one contract because they share the response envelope shape; splitting would duplicate the dataclass. Bifrost binding (Worldtree issue #160), ephemeral sessions, `GET /sessions/{id}`, `PATCH`, `DELETE`, and `GET /sessions/{id}/messages` (history) are explicitly out of scope (codified in the contract's `## Out of scope` H2 — first contract in this repo to carry that section, so future Volva consults resolve cleanly via the default path instead of needing `--out-of-scope` overrides). `prd:` pinned to issue #2 body SHA `01fbbd52b6d90eb0` at `2026-05-21T04:45:06+00:00`; drift check clean. `dependencies:` lists issue #1 as a convention-dependency (no code import; same API-consumption posture).
 - `[2026-05-21]` **`ratatoskr.sessions` implemented via TDD against issue #2's contract.** 19 contract-listed tests authored + GREEN per tracer-bullet vertical-slice (`create_session` first, then `list_sessions`). One internal-inconsistency in the contract spotted at TDD start — POST-003 and `happy_create` test description still said "archived is None, tags is None" while the freshly-amended INV-001 set them to `False` and `[]`; fixed the contract in-place before writing tests so the spec stayed coherent. Implementation is small (~115 LOC for src module); no refactor pass deemed worthwhile (the two functions are ~25 LOC each with distinct error-routing branches).
+- `[2026-05-21]` **Volva code-vs-contract review on `ratatoskr.sessions`.** Three findings, all "fix it" (one with collateral contract amendment). (1) Drift: `archived=item.get("archived", False)` returned `None` for explicit-null because `.get(key, default)` only fires on absent keys, not on null. Fixed to `item.get("archived") or False` (handles absent, null, False, True). INV-002 wording was the source — also amended to spell out the `.get(default)` foot-gun explicitly. (2) Test-gap: no test exercised explicit-null `archived`/`tags`. Added `test_explicit_null_list_defaults` using a raw item dict (the `_list_item()` helper masked the issue with its own defaulting). (3) Precision: `message_count=body.get("message_count")` could silently default to None while POST-003 required it non-None. Aligned: code now uses `body["message_count"]` (matches sibling fields like session_id which use bracket access); INV-001 + STEP 5 updated to spell out strict semantics. Volva's meta-note: "modest weight" — TDD caught the main surface, this caught a narrow Python `.get()` semantics edge that no human reading would have noticed without explicit-null priors. 63 tests GREEN post-fix.
 - `[2026-05-21]` **Volva paraphrase round on `docs/contracts/issues/2.contract.md`.** Volva flagged 5 ambiguities; operator approved amendments to 3 of them. (1) `tags`/`archived`/`name` defaulting semantics now explicit: `tags: list[str]` (default `[]`), `archived: bool` (default `False`), `name: str | None` (default `None`); INV-001/INV-002 + STEPS aligned. (2) `include_archived_query` test tightened: default → URL has NO `include_archived` param at all (was "no param OR explicit false" — softened the assertion against STEP 2's prescriptive behavior). (5) `metadata` populated-vs-defaulted slippage resolved: INV-001 + INV-002 now spell out the defensive `body.get("metadata", {})` default for spec drift tolerance. Volva flags #3 (exception `.body` sensitivity) and #4 (`assert` for runtime validation) reviewed and kept as-is — both intentional and consistent with issue #1's precedent. Drift check still clean (amendments don't touch the pinned issue body).
 - `[2026-05-21]` **Volva code-vs-contract review round on `ratatoskr.sse_client`.** Volva flagged 4 findings (3 drifts + 1 test-gap), all code-side "fix it" recommendations: (1) `_iter_events` fell off cleanly on EOF before terminal, violating INV-001 ("MUST NOT raise StopAsyncIteration before a terminal event arrives unless connection drops"); fix tracks `terminal_seen` flag and raises `SseConnectionDropped` on clean-EOF-without-terminal. (2) Both `SseConnectFailed.body` and `CancelFailed.body` stored full response bytes; ERROR_ROUTING specified truncation to `[:1024]`; fix truncates in `__init__` before storing. (3) `_parse_sse_id` PRE-001 specified `assert isinstance(raw, str)`, but code called `.split(":")` directly (incidental `AttributeError` on non-str); fix adds the assert. (4) Test-gap on cancel_turn's "other status → CancelFailed" branch; fix adds a 503 test with >1024-byte body that double-covers finding #2. Meta-note: Volva said TDD caught the main happy/adversarial shape; the misses were "negative space" cases (clean EOF, exception payload truncation, untested generic cancel branch) — calibration evidence that cross-model review pulls weight on the same-model author's blind spots. 43 tests GREEN post-fix (42 sse_client + 1 boundary), ruff clean.
 - `[2026-05-21]` **Volva paraphrase round on `docs/contracts/issues/1.contract.md`.** Volva flagged 5 ambiguities; operator approved amendments to 3 of them. (1) `reconnect_turn` STEP 2 punt resolved: signature now carries `content: str`; STEP 2 body is `json={"content": content}` matching spec §Reconnect flow example verbatim. Spec line 732 makes the agent's tools+LLM run "exactly once regardless of disconnects/reconnects" — the `content` is a wire-schema requirement, not re-processed server-side. (2) `_parse_sse_id` tightened: `turn_id ≥ 1` AND `seq ≥ 1` (was `≥ 0`); spec §SSE id format line 705 explicitly states `seq` starts at 1, and `turn_id` is SQLite autoincrement (≥1). Test `happy_zero_seq` flipped to `zero_seq [adversarial]`; new `zero_turn_id` + `negative_seq` adversarial tests added. (3) INV-003 clarified to spell out the two-entry-point semantics: `stream_turn` establishes `turn_id` from the first event (first event always yields); `reconnect_turn` parses the expected `turn_id` FROM `last_event_id` BEFORE the connection opens, so the first server event is already a flip-candidate and is NOT yielded on mismatch. Volva flags #3 (MalformedSseId-vs-ValueError split) and #5 (exactly-one-terminal as server-assumed) noted but kept as-is — deliberate distinctions. Drift check still clean against issue #1 (amending the contract doesn't touch the pinned issue body).

src/ratatoskr/sessions.py

@@ -108,7 +108,7 @@ async def list_sessions(
             metadata=item.get("metadata", {}),
             message_count=None,
             name=item.get("name"),
-            archived=item.get("archived", False),
+            archived=item.get("archived") or False,
             tags=item.get("tags") or [],
         )
         for item in body["items"]
@@ -133,7 +133,7 @@ async def create_session(client: httpx.AsyncClient, agent_id: str) -> SessionInf
         created_at=body["created_at"],
         last_active=body["last_active"],
         metadata=body.get("metadata", {}),
-        message_count=body.get("message_count"),
+        message_count=body["message_count"],
         name=None,
         archived=False,
         tags=[],

tests/test_sessions.py

@@ -164,6 +164,31 @@ def _list_item(
 
 
 class TestListSessions:
+    @respx.mock
+    async def test_explicit_null_list_defaults(self) -> None:
+        """INV-002: explicit-null archived -> False; explicit-null tags -> []."""
+        raw_item = {
+            "session_id": "s1",
+            "agent_id": "mimir",
+            "created_at": "2026-04-15T12:00:00+00:00",
+            "last_active": "2026-04-15T12:05:00+00:00",
+            "metadata": {},
+            "name": None,
+            "archived": None,
+            "tags": None,
+        }
+        respx.get("https://w.example/sessions").mock(
+            return_value=httpx.Response(
+                200, json={"items": [raw_item], "next_cursor": None}
+            )
+        )
+        async with httpx.AsyncClient(base_url="https://w.example") as client:
+            page = await list_sessions(client)
+        info = page.items[0]
+        assert info.archived is False, "explicit-null archived must default to False"
+        assert info.tags == [], "explicit-null tags must default to []"
+        assert info.name is None
+
     @respx.mock
     async def test_happy_first_page(self) -> None:
         """happy_first_page [happy,tracer]: one item + next_cursor -> SessionPage shape."""