Skip to content

Commit 108cb6e

Browse files
committed
fix(session-summary): address yellow review — stale-sidecar docs, subpath skip note, conformance clock alignment
- sessions.py: fast-path doctring note + inline return comment now say 'complete and fresh' sidecars never short-page; a present-but-stale sidecar (summary.mtime < list_sessions.mtime) is routed through the same gap-fill as a missing one and can short-page. - types.py: SessionStore.list_session_summaries docstring explicitly notes that subagent (subpath) keys must be skipped by the fold and that results are scoped to project_key like list_sessions. - conformance: replace tautological 'refolded[mtime] >= summ[mtime]' check (the fold preserves prev mtime verbatim) with an explicit equality, and add a clock-alignment assertion under has_list_sessions that catches adapters deriving sidecar mtime from entry ISO timestamps (which would make every sidecar look stale to the fast-path freshness check). - test: pin test_limit_offset_applied_after_sidechain_filter to the slow path (list_session_summaries -> NotImplementedError) so it keeps covering slow-path filter-THEN-paginate — the fast path deliberately locks in paginate-THEN-drop for summary-backed sidechain slots.
1 parent e3ebe57 commit 108cb6e

4 files changed

Lines changed: 43 additions & 10 deletions

File tree

src/claude_agent_sdk/_internal/sessions.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1602,11 +1602,12 @@ async def list_sessions_from_store(
16021602
.. note::
16031603
If the store implements ``list_session_summaries``, this is one batch
16041604
summary call plus one cheap ``list_sessions()`` enumeration to
1605-
gap-fill sessions missing a sidecar — zero per-session ``load()``
1606-
calls when sidecars are complete. Otherwise falls back to one
1607-
``store.load()`` per session (bounded at 16 concurrent), which on
1608-
remote backends with many or large sessions can be expensive (e.g.,
1609-
S3 egress, Postgres large-row reads).
1605+
gap-fill sessions missing a sidecar or whose sidecar is stale
1606+
(``summary.mtime < list_sessions.mtime``) — zero per-session
1607+
``load()`` calls when sidecars are complete and fresh. Otherwise
1608+
falls back to one ``store.load()`` per session (bounded at 16
1609+
concurrent), which on remote backends with many or large sessions
1610+
can be expensive (e.g., S3 egress, Postgres large-row reads).
16101611
16111612
Gap-fill requires ``list_sessions``: if the store implements
16121613
``list_session_summaries`` but not ``list_sessions``, sessions
@@ -1697,7 +1698,9 @@ async def list_sessions_from_store(
16971698
# extractable summary after load) are dropped here, AFTER
16981699
# pagination — that case alone can short-page. Summary-backed
16991700
# slots were already pre-filtered above, so a store with complete
1700-
# sidecars never short-pages.
1701+
# and fresh sidecars never short-pages; a present-but-stale
1702+
# sidecar is routed through gap-fill (same as a missing one) and
1703+
# can short-page if load() yields no extractable summary.
17011704
return [sl["info"] for sl in page if sl["info"] is not None]
17021705

17031706
if not has_list_sessions:

src/claude_agent_sdk/testing/session_store_conformance.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,13 +195,27 @@ async def fresh() -> SessionStore:
195195
summ = by_id["summ-sess"]
196196
# mtime must be epoch-ms; >1e12 rules out epoch-seconds.
197197
assert math.isfinite(summ["mtime"]) and summ["mtime"] > 1e12
198+
# Clock alignment: sidecar mtime is storage write time (adapter-
199+
# stamped at persist), and must share a clock with
200+
# list_sessions().mtime for the same session. Adapters that derive
201+
# sidecar mtime from entry ISO timestamps would report a strictly
202+
# older value than list_sessions()'s storage-time mtime and make
203+
# every sidecar look stale to the fast-path freshness check in
204+
# list_sessions_from_store(); this assertion catches that.
205+
if has_list_sessions:
206+
ls_by_id = {
207+
e["session_id"]: e["mtime"] for e in await store.list_sessions("proj")
208+
}
209+
assert summ["mtime"] >= ls_by_id["summ-sess"]
198210
# data is opaque; the contract is that it round-trips into the fold.
199211
assert isinstance(summ["data"], dict)
200212
refolded = fold_session_summary(
201213
summ, key, [_e({"timestamp": "2024-01-01T00:00:03.000Z"})]
202214
)
203215
assert refolded["session_id"] == "summ-sess"
204-
assert refolded["mtime"] >= summ["mtime"]
216+
# The fold preserves prev["mtime"] verbatim — mtime is stamped by
217+
# the adapter after persisting, not by the fold.
218+
assert refolded["mtime"] == summ["mtime"]
205219
# Subagent appends must NOT affect the main session's summary.
206220
await store.append(
207221
{**key, "subpath": "subagents/agent-1"},

src/claude_agent_sdk/types.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,11 @@ async def list_session_summaries(
12661266
"""Return incrementally-maintained summaries for all sessions in one call.
12671267
12681268
Stores should maintain these via :func:`fold_session_summary` inside
1269-
:meth:`append`.
1269+
:meth:`append`. Skip the fold for keys with a ``subpath`` — subagent
1270+
transcripts must not contribute to the main session's summary.
1271+
1272+
Like :meth:`list_sessions`, results are scoped to a single
1273+
``project_key`` and exclude ``subpath`` entries.
12701274
12711275
Optional — if unimplemented, ``list_sessions_from_store()`` falls back
12721276
to ``list_sessions()`` + per-session ``load()``.

tests/test_session_helpers_store.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,20 @@ async def test_limit_offset_applied_after_sidechain_filter(self) -> None:
157157
first and ``_apply_sort_limit_offset`` paginates the filtered set, so
158158
``limit=N`` returns N rows even when sidechains exist. The store path
159159
must do the same — paginating before filtering would return short
160-
pages and let sidechains consume page slots."""
161-
store = InMemorySessionStore()
160+
pages and let sidechains consume page slots.
161+
162+
Pinned to the slow path (``list_session_summaries`` suppressed)
163+
because the fast path deliberately locks in paginate-THEN-drop for
164+
sidechain-shaped summary slots (see
165+
``test_sidechain_summary_short_pages``); slow-path filter-THEN-
166+
paginate is what this test covers.
167+
"""
168+
169+
class SlowPathStore(InMemorySessionStore):
170+
async def list_session_summaries(self, project_key): # type: ignore[override]
171+
raise NotImplementedError
172+
173+
store = SlowPathStore()
162174
valid_sids: list[str] = []
163175
for _ in range(5):
164176
sid = str(uuid_mod.uuid4())

0 commit comments

Comments
 (0)