From cadcf983d559621178ecdda57979ebe670138768 Mon Sep 17 00:00:00 2001 From: Frank Song Date: Fri, 15 May 2026 18:04:53 +0800 Subject: [PATCH] Tighten silent failure shrink detection --- CHANGELOG.md | 3 +++ api/streaming.py | 11 +++++------ tests/test_silent_failure_detection.py | 14 ++++++-------- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 328683ef..7f447192 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ ### Fixed +- Silent-failure detection no longer treats old assistant messages in a shrunk + result history as proof that the current turn produced a new assistant reply. + - **PR #2270** by @Michaelyklam (closes #2226) — Firefox Android PWA installs from `/session/` pages now resolve the Hermes manifest and icons instead of falling back to a generated letter icon. The dynamic `` script now runs before manifest/favicon links, `/session/manifest.json` and `/session/manifest.webmanifest` return the real manifest JSON, and session-prefixed manifest routes are now marked as public auth-skip routes. Adds 211 lines of regression coverage for the manifest responses and the session-prefixed 512px icon path. - **PR #2268** by @eleboucher — `docker_init.bash` no longer fails under Kubernetes `runAsUser` configurations where the running UID has no `/etc/passwd` entry. Pre-fix, the bare `whoami` invocation aborted the script under `set -e` because `whoami` exited with a non-zero status on missing-passwd UIDs. Now falls back to a synthetic `uid-` name when `whoami` fails (`whoami 2>/dev/null || echo "uid-$(id -u)"`). Two-line change. diff --git a/api/streaming.py b/api/streaming.py index 04cba4db..ece2ddaa 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -140,17 +140,16 @@ def _has_new_assistant_reply(all_messages: list, prev_count: int) -> bool: messages at index >= prev_count are inspected so that historical assistant replies don't mask a silent failure on the current turn. - If ``len(all_messages) < prev_count`` (an edge-case shrink) we fall back - to scanning the full list — this keeps behaviour consistent with the - original code when offsets don't cleanly apply. When ``len == prev_count``, - there are no new messages and we return False. + If ``len(all_messages) < prev_count`` (an edge-case shrink), there is no + reliable new-message slice to inspect. Treat that as "no new assistant + reply" so stale historical assistant replies cannot mask a silent failure. + When ``len == prev_count``, there are no new messages and we return False. """ if len(all_messages) > prev_count: # Normal case: new messages appended beyond the pre-turn history. candidates = all_messages[prev_count:] elif len(all_messages) < prev_count: - # Edge-case shrink — scan everything as fallback. - candidates = all_messages + return False else: # Same length. In production this means no new messages were appended. # However, some test fixtures replace the entire message list rather diff --git a/tests/test_silent_failure_detection.py b/tests/test_silent_failure_detection.py index 3a1004b0..e08c3a44 100644 --- a/tests/test_silent_failure_detection.py +++ b/tests/test_silent_failure_detection.py @@ -95,12 +95,12 @@ class TestHasNewAssistantReply: assert _has_new_assistant_reply(all_msgs, len(prev)) is False # Scenario 8 ────────────────────────────────────────────────────────── - def test_result_shorter_than_prev_len_fallback(self): - """Edge-case: result messages < prev_count → fallback to scanning all. + def test_result_shorter_than_prev_len_returns_false(self): + """Edge-case: result messages < prev_count cannot prove a new reply. - The helper falls back to scanning all messages when the slice would - be empty. In this scenario, if an assistant message exists in the - (shorter) result it should still be detected. + Shrunken result history has no reliable new-message slice. Scanning + the shorter list can mistake an older assistant reply for a current + turn reply, which would hide the silent-failure banner. """ prev_count = 5 # Only 3 messages in result — shorter than prev_count @@ -109,10 +109,8 @@ class TestHasNewAssistantReply: _msg("assistant", "b"), _msg("user", "c"), ] - # Fallback scans all → assistant with content "b" is found - assert _has_new_assistant_reply(all_msgs, prev_count) is True + assert _has_new_assistant_reply(all_msgs, prev_count) is False - # But if no assistant content in the shorter result → False all_msgs_no_asst = [ _msg("user", "a"), _msg("user", "b"),