fix(routes): gate state.db row backstop on absence of truncation_boundary prefix
Reviewer regression (reproduced): _STATE_DB_DISPLAY_ROW_BACKSTOP (newest-50k SQL cap) was applied UNCONDITIONALLY to the GET /api/session display path's state.db read. But merge_session_messages_append_only needs the rows at/around the session's truncation_boundary (the preserved prefix) to reconcile correctly. A newest-N-only cap drops those boundary rows for a >N-row session and corrupts the merge (silently losing the preserved prefix), and also prevents older-page (msg_before) requests from ever reaching them. So a pathological-but-real >50k- row compressed session got a corrupted merge / unreachable older history rather than a clean bound. Fix: gate the backstop on the SAME conditions _state_db_since_timestamp_for_limited_display uses to decide a read is boundary-free — not msg_before paging, and no truncation_watermark / truncation_boundary. Those cases stay on the full (uncapped) read; the backstop is now a defensive net for the common uncompressed initial-tail path only. Extracted into _state_db_backstop_limit_for_display for direct test coverage. The get_state_db_session_messages(limit=) signature + newest-rows subquery are unchanged — only the display-path APPLICATION of the cap is gated. Test: tests/test_state_db_backstop_boundary_gate.py — backstop applies with no boundary prefix; SKIPPED when truncation_boundary / truncation_watermark set or msg_before paging; empty-string markers treated as unset (matching the existing helper). Existing backstop + reconciliation suites (53 tests) unchanged. Model Used: builtin:zai-coding-plan/GLM-5.2 (ZCode agent).
This commit is contained in:
1 parent
b537be3008
commit
f445c3ae39
2 files changed
+101
-7
No files matched your search
+33
-7
@@ -8452,6 +8452,31 @@ _LIMITED_TOOL_CONTENT_MAX_CHARS = 4096
|
||||
# already handles the common tail-load case. The full-history model-context
|
||||
# callers (reconciliation, new-turn context) do NOT use this cap.
|
||||
_STATE_DB_DISPLAY_ROW_BACKSTOP = 50000
|
||||
|
||||
|
||||
def _state_db_backstop_limit_for_display(session, msg_before) -> int | None:
|
||||
"""Return the row backstop to apply to the display path's state.db read, or
|
||||
``None`` for an uncapped (full-history) read.
|
||||
|
||||
The backstop is a defensive net against a pathological/huge state.db, NOT a
|
||||
semantic window. It is applied ONLY on provably-safe reads where no
|
||||
``truncation_boundary`` prefix is required for the merge:
|
||||
``merge_session_messages_append_only`` needs the rows at/around the session's
|
||||
``truncation_boundary`` to reconcile correctly, and a newest-N-only SQL cap
|
||||
would drop those boundary rows for a >N-row session and corrupt the merge
|
||||
(silently losing the preserved prefix). So this mirrors the same conditions
|
||||
``_state_db_since_timestamp_for_limited_display`` uses to decide a read is
|
||||
boundary-free: not ``msg_before`` paging, and no ``truncation_watermark`` /
|
||||
``truncation_boundary``. Extracted for direct test coverage.
|
||||
"""
|
||||
has_boundary_prefix = (
|
||||
msg_before is not None
|
||||
or getattr(session, "truncation_watermark", None) not in (None, "")
|
||||
or getattr(session, "truncation_boundary", None) not in (None, "")
|
||||
)
|
||||
return None if has_boundary_prefix else _STATE_DB_DISPLAY_ROW_BACKSTOP
|
||||
|
||||
|
||||
_LIMITED_TOOL_CONTENT_NOTICE = (
|
||||
"\n\n[Tool output truncated in paginated session response; "
|
||||
"load the full transcript to inspect the complete result.]"
|
||||
@@ -12435,13 +12460,14 @@ def handle_get(handler, parsed) -> bool:
|
||||
_state_db_reader_kwargs = {"profile": _session_profile}
|
||||
if state_db_since_timestamp is not None:
|
||||
_state_db_reader_kwargs["since_timestamp"] = state_db_since_timestamp
|
||||
# Apply the display-path row backstop (see
|
||||
# _STATE_DB_DISPLAY_ROW_BACKSTOP). This is a defensive cap, not a
|
||||
# semantic window — it only bounds a pathological state.db. The
|
||||
# since_timestamp optimization above already bounds the common
|
||||
# uncompressed tail load; this covers the compressed-session bail
|
||||
# case where that optimization returns None.
|
||||
_state_db_reader_kwargs["limit"] = _STATE_DB_DISPLAY_ROW_BACKSTOP
|
||||
# Apply the display-path row backstop ONLY on provably-safe
|
||||
# reads where no truncation_boundary prefix is required for the
|
||||
# merge — see _state_db_backstop_limit_for_display. Compressed
|
||||
# sessions and msg_before paging need their full prefix rows for
|
||||
# correct reconciliation, so those stay uncapped.
|
||||
_backstop = _state_db_backstop_limit_for_display(s, msg_before)
|
||||
if _backstop is not None:
|
||||
_state_db_reader_kwargs["limit"] = _backstop
|
||||
state_db_messages = get_state_db_session_messages(
|
||||
sid,
|
||||
**_state_db_reader_kwargs,
|
||||
|
||||
Reference in new issue
Block a user