diff --git a/tests/_session_helpers.py b/tests/_session_helpers.py index d9e224a2..93d93497 100644 --- a/tests/_session_helpers.py +++ b/tests/_session_helpers.py @@ -382,9 +382,7 @@ def seam_provider(utterance: str, *, provider_name: str = "openai-compatible") - later session in the test run (the SSE-recovery e2e servers resolve the same instance). """ - provider = MagicMock() - provider.provider_name = provider_name - provider.get_capabilities.return_value = ModelCapabilities() + provider = provider_shell(provider_name) provider.create_streaming = MagicMock(return_value=think_tag_stream(utterance)) return provider @@ -520,6 +518,20 @@ class ArmedHandle: self.closed = True +def provider_shell( + name: str = "openai-compatible", + retryable: frozenset[str] = frozenset({"IncompleteStreamError"}), +) -> MagicMock: + """The armed-provider fake skeleton every streaming fake builds on: + provider_name / capabilities / retryable set. ONE spelling, so a new + attribute the seam starts probing lands in every fake at once.""" + provider = MagicMock() + provider.provider_name = name + provider.get_capabilities.return_value = ModelCapabilities() + provider.retryable_error_names = retryable + return provider + + def arm_session( session: Any, *streams: Any, @@ -543,10 +555,7 @@ def arm_session( before the main loop ran. """ session._title_generated = True - provider = MagicMock() - provider.provider_name = name - provider.get_capabilities.return_value = ModelCapabilities() - provider.retryable_error_names = retryable + provider = provider_shell(name, retryable) # One handle PER CREATE (the real adapters' rule): `handles` records # them all, `_armed_handle` is the latest. provider._armed_handle = None @@ -580,10 +589,7 @@ def scripted_provider(chunks: list[StreamChunk]) -> MagicMock: handle is appended per call, matching the one-handle-per-create behavior of every real adapter. """ - provider = MagicMock() - provider.provider_name = "openai-compatible" - provider.get_capabilities.return_value = ModelCapabilities() - provider.retryable_error_names = frozenset({"IncompleteStreamError"}) + provider = provider_shell() def _create(**kwargs: Any): ref = kwargs.get("cancel_ref") diff --git a/tests/test_midstream_retry.py b/tests/test_midstream_retry.py index d6326d0a..22d86dfa 100644 --- a/tests/test_midstream_retry.py +++ b/tests/test_midstream_retry.py @@ -810,10 +810,11 @@ class TestRecreateWindowClassification: assert ui.of("content").count(flushed) == 1 def test_wire_preparation_failure_never_touches_backend_health(self, tmp_db): - """A deterministic lowering failure is a session-data fault: no - dispatch, no health record, no fallback walk — the typed - ``WirePreparationError`` rides to the fatal formatter's dedicated - branch.""" + """A lowering failure is a session-data fault: no dispatch and no + health record on ANY lane — but it DOES walk the fallbacks, since + prepare is lane-variant and another lane's posture may serve the + turn. When none does, the typed ``WirePreparationError`` rides to + the fatal formatter's dedicated branch.""" ui = RecordingUI() session = _make_session(ui) provider = arm_session(session, _good_stream("unreached")) @@ -823,7 +824,7 @@ class TestRecreateWindowClassification: session._registry = registry with ( patch.object(session, "_get_health_tracker", return_value=tracker), - patch.object(session, "_try_fallback_lane") as fb_spy, + patch.object(session, "_try_fallback_lane", return_value=None) as fb_spy, patch.object( session, "_prepare_wire_messages", side_effect=ValueError("malformed turn 7") ), @@ -832,12 +833,67 @@ class TestRecreateWindowClassification: session.send("test") tracker.record_failure.assert_not_called() - fb_spy.assert_not_called() + fb_spy.assert_called_once() provider.create_streaming.assert_not_called() errors = ui.of("error") assert errors and "stored history" in errors[-1] assert "malformed turn 7" in errors[-1] + def test_fallback_prep_fault_continues_walk(self, tmp_db): + """A prep fault on one lane must not abort the walk: the next + alias may still serve the turn (prepare is lane-variant).""" + from tests._session_helpers import make_result + + session = _make_session(RecordingUI()) + registry = MagicMock() + registry.fallback = ["a", "b"] + session._registry = registry + served = make_result(content="ok") + tracker = MagicMock() + with ( + patch.object(session, "_get_health_tracker", return_value=tracker), + patch.object( + session, + "_model_turn_with_retry", + side_effect=WirePreparationError("primary prep fault"), + ), + patch.object(session, "_try_fallback_lane", side_effect=[None, served]) as fb, + ): + from turnstone.core.session import _StreamTurnConsumer + + consumer = _StreamTurnConsumer(session, 0) + result = session._model_turn_with_fallback(consumer, lambda w, lane: w, 0) + + assert result is served + assert fb.call_count == 2 + tracker.record_failure.assert_not_called() + + def test_fallback_prep_fault_records_no_health(self, tmp_db): + ui = RecordingUI() + session = _make_session(ui) + registry = MagicMock() + registry.resolve_binding.return_value = (MagicMock(), "m", None, MagicMock(), None) + fb_tracker = MagicMock() + session._registry = registry + session._health_registry = MagicMock() + session._health_registry.get_tracker_for_alias.return_value = fb_tracker + with ( + patch.object(session, "_build_main_lane", return_value=MagicMock()), + patch.object( + session, + "_model_turn_with_retry", + side_effect=WirePreparationError("fold blew up"), + ), + ): + from turnstone.core.session import _StreamTurnConsumer + + consumer = _StreamTurnConsumer(session, 0) + out = session._try_fallback_lane("fb", consumer, lambda w, lane: w, 0) + + assert out is None + fb_tracker.record_failure.assert_not_called() + assert any("Fallback fb also failed: WirePreparationError" in i for i in ui.of("info")) + class TestDebugDumpLatch: """The debug request dump prints once per ``_stream_response`` diff --git a/turnstone/core/model_turn.py b/turnstone/core/model_turn.py index 2139f396..a2d63be8 100644 --- a/turnstone/core/model_turn.py +++ b/turnstone/core/model_turn.py @@ -492,19 +492,22 @@ def lane_thinking_suppressed(lane: ModelLane) -> bool: ) -def lane_scans_inline_reasoning(lane: ModelLane | None) -> bool: +def caps_scan_inline_reasoning(caps: ModelCapabilities | None) -> bool: """THE inline tag-scan gate — the single spelling of the #978 rule. - Scan ```` tags out of the content stream unless the lane's + Scan ```` tags out of the content stream unless the capabilities declare that the server segregates reasoning itself - (``server_parses_reasoning``); no capabilities, or no lane yet, keeps - the passthrough-server default: scan. Shared by the drain seam and - the interactive display splitter so the two readings of one stream - cannot disagree. + (``server_parses_reasoning``); no declaration keeps the + passthrough-server default: scan. Shared by the drain seam, the + interactive display splitter, and the title peel, so no two + consumers can read one stream differently. """ - return ( - lane is None or lane.capabilities is None or not lane.capabilities.server_parses_reasoning - ) + return caps is None or not caps.server_parses_reasoning + + +def lane_scans_inline_reasoning(lane: ModelLane | None) -> bool: + """:func:`caps_scan_inline_reasoning` over a lane (no lane yet = scan).""" + return caps_scan_inline_reasoning(lane.capabilities if lane is not None else None) def lane_without_thinking(lane: ModelLane) -> ModelLane: @@ -962,10 +965,10 @@ def model_turn( reads the error at all, which is what keeps a Stop mid-summary off the red-error path. - *prepare_wire* is the caller's OWN deterministic lowering (called - with the serving lane, so per-lane capability posture is available), - composed - after the seam passes and before the Phase-5 attach: the main loop's + *prepare_wire* is the caller's OWN deterministic lowering, called + with the serving lane so per-lane capability posture is available, + composed after the seam passes and before the Phase-5 attach: the + main loop's system-message prepend, sender labels, capability-sensitive system-turn fold, empty-user drop, and orphan repair live here, each a ``lowering.py``-composed pass. It must be pure lowering — no diff --git a/turnstone/core/session.py b/turnstone/core/session.py index aa90fde1..4e0edeff 100644 --- a/turnstone/core/session.py +++ b/turnstone/core/session.py @@ -143,6 +143,7 @@ from turnstone.core.model_turn import ( ModelLane, ModelTurnResult, WirePreparationError, + caps_scan_inline_reasoning, create_provider, finalize_provider_blocks, folds_trailing_info, @@ -1817,6 +1818,16 @@ class BackendAuthUnavailableError(RuntimeError): """A fail-closed dynamic model credential could not be resolved.""" +# Errors that carry their own remediation and must surface AS THEMSELVES: +# the re-issue ladder never masks them behind an earlier stream death. +# (Walk policy stays per-class — an auth refusal aborts the walk, a +# wire-preparation fault continues it — only the mask reads this set.) +_SELF_SURFACING_ERRORS: tuple[type[Exception], ...] = ( + BackendAuthUnavailableError, + WirePreparationError, +) + + def _mint_refusal_cause( prefix: str, alias: str, @@ -4258,7 +4269,7 @@ class ChatSession: # (``server_parses_reasoning``): there a close tag in content # IS quoted prose, and cutting would eat a title that mentions # it. See ``_TITLE_*``. - if not self._get_capabilities().server_parses_reasoning: + if caps_scan_inline_reasoning(self._get_capabilities()): _cut = max( (raw.rfind(_t) + len(_t) for _t in ThinkTagSplitter.CLOSE_TAGS if _t in raw), default=0, @@ -5753,9 +5764,8 @@ class ChatSession: return ( f"Preparing the request from this conversation's history failed: " f"{detail}. This is a fault in the session's stored history, not " - f"in the {model_label} backend (no fallback was tried and backend " - f"health is unaffected). /compact may clear a malformed turn; " - f"please report this." + f"in the {model_label} backend (backend health is unaffected). " + f"/compact may clear a malformed turn; please report this." ) # Context overflow — matched by text, because it arrives as BadRequestError @@ -6257,18 +6267,16 @@ class ChatSession: # Explicit fail-closed policy: never reinterpret an authentication # refusal as backend health and never route it to a static fallback. raise - except WirePreparationError: - # No health record and no fallback walk: every lane would - # re-run the same deterministic passes, and N recorded - # failures would paint a cluster-wide outage over one - # session's malformed history. - raise except Exception as primary_err: if consumer.attempt_armed: # Mid-stream death: the re-issue ladder owns it (UI finalize, # backoff, discard, full re-create) — not the fallback walk. raise - if tracker: + # A wire-preparation fault is the session's data, never backend + # health — but it DOES enter the walk: prepare runs per lane + # (fold posture follows lane.capabilities), so another lane's + # posture may serve a turn the primary's could not. + if tracker and not isinstance(primary_err, WirePreparationError): tracker.record_failure() if not self._registry or not self._registry.fallback: raise @@ -6335,15 +6343,16 @@ class ChatSession: return self._model_turn_with_retry( fb_lane, fb_tracker, consumer, prepare_wire, my_generation ) - except (BackendAuthUnavailableError, WirePreparationError): - # An auth refusal is fail-closed policy and a wire-preparation - # failure is the caller's data bug: neither is this fallback's - # health signal. + except BackendAuthUnavailableError: + # Fail-closed policy — never another lane's business. raise except Exception as fb_err: if consumer.attempt_armed: raise - if fb_tracker: + # This lane's wire-preparation fault is not its health signal, + # and it must not abort the walk: prepare is lane-variant, so + # the next alias may still serve the turn. + if fb_tracker and not isinstance(fb_err, WirePreparationError): fb_tracker.record_failure() # Class name only in the UI line: a ConnectError's text can # carry a credential-bearing base_url, and this string lands in @@ -8354,7 +8363,7 @@ class ChatSession: # base_url verbatim. if ( last_stream_death is None - or isinstance(e, (BackendAuthUnavailableError, WirePreparationError)) + or isinstance(e, _SELF_SURFACING_ERRORS) or _is_ctx_overflow(e) ): raise diff --git a/turnstone/core/session_ui_base.py b/turnstone/core/session_ui_base.py index 494c42bd..b6b9c066 100644 --- a/turnstone/core/session_ui_base.py +++ b/turnstone/core/session_ui_base.py @@ -2122,7 +2122,7 @@ class SessionUIBase: # sibling or the ``__budget_override__`` pseudo-tool makes # candidates < pending, AND # - call_ids must be unique — some local models emit duplicate - # non-empty tool-call ids (``_ensure_tool_call_ids`` only fills + # non-empty tool-call ids (``model_turn.ensure_tool_call_ids`` only fills # MISSING ones), which collapse in the ``needed`` set and would let # one verdict clear two distinct calls (their args differ). # Either mismatch → hold the whole batch for a human. Checked diff --git a/turnstone/core/streaming_text.py b/turnstone/core/streaming_text.py index d0c61351..6365931d 100644 --- a/turnstone/core/streaming_text.py +++ b/turnstone/core/streaming_text.py @@ -93,7 +93,7 @@ class ThinkTagSplitter: if not self.pending: return "" tail = partial_tag_tail(self.pending) - closeable = self.pending[: len(self.pending) - len(tail)] if tail else self.pending + closeable = self.pending.removesuffix(tail) if closeable: self._emit(closeable, self.in_think) self.pending = ""