fix(#4767): gate boundary-advanced bypass on checkpoint consumption (no stale-suffix resurrection)
Codex final-gate found the boundary<watermark advanced signal made sidecar_advanced_past_watermark globally true, so a stale ts>watermark suffix row appearing in state.db BEFORE the edited checkpoint bypassed the skip and resurrected. Gate the bypass on state_replay_idx >= len(sidecar_visible_sequence) (checkpoint consumed); apply the same gate to the same-second CORE-B exception. Regression test added + proven non-vacuous. All legit cases (state-only reply after checkpoint, CORE-B same-second) still pass. Co-authored-by: AlexeyDsov <alexeydsov@gmail.com>
This commit is contained in:
1 parent
695d9ab814
commit
6a7fe93e10
2 files changed
+55
-1
No files matched your search
+17
-1
@@ -5293,12 +5293,22 @@ def merge_session_messages_append_only(
|
||||
# post-edit content and must merge through (not be dropped as a replaced
|
||||
# tail). The conservative skip still applies for boundary is None and
|
||||
# boundary == watermark (not-advanced / legacy).
|
||||
#
|
||||
# CRITICAL: the boundary-advanced signal may only bypass the skip AFTER
|
||||
# state replay has consumed the sidecar's visible checkpoint
|
||||
# (state_replay_idx >= len(sidecar_visible_sequence)). A deleted suffix
|
||||
# row with ts > watermark that appears in state.db BEFORE the edited
|
||||
# checkpoint must still be skipped — otherwise the advanced signal would
|
||||
# resurrect it. The sidecar-max-timestamp signal needs no such gate (a
|
||||
# sidecar tail beyond the watermark is itself proof the checkpoint has
|
||||
# advanced).
|
||||
checkpoint_consumed = state_replay_idx >= len(sidecar_visible_sequence)
|
||||
sidecar_advanced_past_watermark = (
|
||||
watermark_timestamp is not None
|
||||
and (
|
||||
(max_sidecar_timestamp is not None
|
||||
and max_sidecar_timestamp > watermark_timestamp)
|
||||
or watermark_advanced_by_boundary
|
||||
or (watermark_advanced_by_boundary and checkpoint_consumed)
|
||||
)
|
||||
)
|
||||
if (
|
||||
@@ -5408,9 +5418,15 @@ def merge_session_messages_append_only(
|
||||
# block would normally skip it ("sidecar already has this message"),
|
||||
# but the sidecar doesn't — it's a genuine state-only recovery row.
|
||||
# Let it through (CORE-B, #4767).
|
||||
#
|
||||
# Only AFTER the sidecar's visible checkpoint has been consumed
|
||||
# (checkpoint_consumed) — a same-second row appearing in state.db
|
||||
# BEFORE the edited user replay is a deleted/replaced row, not the
|
||||
# post-edit reply, and must stay skipped.
|
||||
if (
|
||||
watermark_timestamp is not None
|
||||
and timestamp == watermark_timestamp
|
||||
and checkpoint_consumed
|
||||
and str(msg.get("role", "")).lower() != "user"
|
||||
and _session_message_content_key(msg) not in seen_content_keys
|
||||
):
|
||||
|
||||
@@ -193,6 +193,44 @@ def test_advanced_sidecar_at_watermark_keeps_state_only_post_edit_reply():
|
||||
)
|
||||
|
||||
|
||||
def test_advanced_does_not_resurrect_stale_post_watermark_row_before_checkpoint():
|
||||
"""The boundary<watermark advanced signal must NOT resurrect a deleted
|
||||
suffix row with ts > watermark that appears in state.db BEFORE the edited
|
||||
checkpoint row.
|
||||
|
||||
Codex final-gate found that making `sidecar_advanced_past_watermark`
|
||||
globally true (whenever boundary < watermark) let a stale ts>watermark row
|
||||
bypass the skip before state replay reached the sidecar's edited checkpoint,
|
||||
resurrecting deleted turns. The bypass is now gated on the checkpoint having
|
||||
been consumed (state_replay_idx >= len(sidecar_visible_sequence))."""
|
||||
# sidecar ends at the edited user @200 (== watermark); state.db lists a
|
||||
# deleted future suffix (@300/@301) BEFORE the appended edited turn.
|
||||
sidecar = [
|
||||
_msg("user", "first", 50.0),
|
||||
_msg("assistant", "first reply", 51.0),
|
||||
_msg("user", "edited prompt", 200.0),
|
||||
]
|
||||
state = [
|
||||
_msg("user", "first", 50.0),
|
||||
_msg("assistant", "first reply", 51.0),
|
||||
_msg("user", "deleted future prompt", 300.0),
|
||||
_msg("assistant", "deleted future answer", 301.0),
|
||||
_msg("user", "edited prompt", 200.0),
|
||||
]
|
||||
|
||||
merged = models.merge_session_messages_append_only(
|
||||
sidecar, state, truncation_watermark=200.0, truncation_boundary=51.0
|
||||
)
|
||||
|
||||
contents = [m["content"] for m in merged]
|
||||
assert "deleted future prompt" not in contents, (
|
||||
f"Stale post-watermark row resurrected before checkpoint! {contents}"
|
||||
)
|
||||
assert "deleted future answer" not in contents, (
|
||||
f"Stale post-watermark row resurrected before checkpoint! {contents}"
|
||||
)
|
||||
|
||||
|
||||
# ─── Same-second assistant reply dropped ──────────────────────────────────────
|
||||
|
||||
|
||||
|
||||
Reference in new issue
Block a user