10 Commits

Author	SHA1	Message	Date
Patrick Buckley	70165807c7	fix(reasoning): close the unmarked chain-of-thought leak, gate the tag scan by backend (#940) (#978) ... Some serving setups emit model reasoning inline with no think tags and no reasoning_content at all — nothing any parser can segregate (measured live on the dev vLLM: 20/20 sampled completions, streamed and not, proxied and direct). The drain seam correctly passes unmarked prose through, so it became the artifact on every bounded-artifact lane: workstream titles ("Thinking Process:"), compaction summaries that were ~90% chain-of- thought, and the web-fetch tool results #940 reports — which then ride every following turn as context. Three coordinated changes: * Utility lanes ask for no reasoning. _utility_completion (title, compaction, web-fetch extraction) pins the alias's declared thinking toggle off and withholds every reasoning-effort channel — the relayed session knob, the lane rung, the definition default, and the graded template key — via lane_without_thinking / lane_thinking_suppressed, the same suppression omni transcription already used (now shared as thinking_off_template_kwargs). Measured end-to-end: the extraction that returned 3.7k chars of reasoning returns a 258-char answer. * server_parses_reasoning capability. A backend that segregates reasoning into its own channel declares it, and the inline tag scan turns off on every lane: the drain seam, the interactive splitter (which now reads the ACTIVE stream's capabilities via the creation- time handoff register, never the primary alias's), and the title lane's cosmetic peel — so prose that merely quotes a tag can no longer be misrouted, and the utility suppression stands down where reasoning costs the artifact nothing. The built-in commercial capability tables declare it wholesale (known models and table-miss defaults); local compat lanes keep the passthrough default the scan exists for. Bool-typed capability overrides coerce string spellings instead of truthiness-flipping on hand-edited JSON. * Title selection follows the prompt's contract, not line position: the last line within the word cap that ends in a word character — rejecting explanation sentences, sign-offs, parentheticals, and reasoning headings in any script (terminal punctuation carries unspaced scripts where whitespace word counts are meaningless) — else the last non-empty line. 20/20 captured live responses title correctly (9/20 before, unchanged since well before the seam unification: the old and new pipelines scored identically on every sample, so the regression source was the backend's output shape, not #965). Also folded in from the review round: a think tag split across a reasoning-delta boundary reassembles in the drain (partial-tag tail carry; tool boundaries still flush), Turn.text joins text blocks with a newline so multi-block answers stop fusing words in notification bodies and every flattened read, the notify hook reads final_assistant_text directly instead of through a one-line shim, web-fetch extraction uses the shared _non_blank_or fallback, and the judge/output-guard suites use real ModelCapabilities instead of truthy mock attributes. Closes #940.	2026-08-05 12:58:55 -07:00
Patrick Buckley	bc3fa60011	fix(providers): segregate inline reasoning at the drain seam ... Passthrough servers (parserless vLLM/llama.cpp, LM Studio, bare gateways) emit reasoning as literal <think>/<reasoning> blocks inside content, and only three of nine drained lanes stripped them: web_fetch tool results persisted raw think blocks into every following turn (#940), judge verdicts parsed through tag noise, and a draft verdict inside a think block could shadow the real one at the output guard. One rule at the seam now. drain_stream accumulates content in RUNS bounded by interleaving signals (provider-parsed reasoning deltas, tool-call deltas) with the interactive consumer's within-chunk ordering — reasoning, then content, then the tool-call close — and splits each run through split_inline_reasoning, the one-shot form of the interactive lane's ThinkTagSplitter: a pure raw split, exactly equivalent to the streaming form on every catalog case. One trim policy exists and the drain owns it: blank edge lines are trimmed once over the joined runs when a tag was consumed, so tag residue dies at the edges while genuine inter-run paragraph separators survive. Extracted text is appended to result.reasoning after any server-parsed reasoning with a blank-line boundary and rides the native lane as the reasoning_text synth block. Orphan CLOSE tags deliberately pass through byte-identical: a close whose open never arrived is indistinguishable from prose QUOTING the tag, and drained lanes routinely quote third-party text — reclassifying would let a malicious page containing the literal tag destroy the extraction that cites it. The title lane keeps a local rfind peel as display-string formatting. The citations footer folds only onto non-blank content — sourcing for an answer that does not exist is dropped rather than handed to emptiness checks as a footer-only "answer". Every private strip is deleted: the title lane's strip, the summarizer strip, _strip_reasoning itself, and the optimizer's five regexes (_strip_markdown_fence is now the one fence rule, applied to normalized model output only, never to or-fallback values). Think-only and whitespace-only responses drain to blank content, and every lane's no-answer fallback gates on blankness: web_fetch returns an honest extraction-error card, the intent judge takes the empty-retry ladder, the task-agent synthesis reports "(no output)", and the optimizer keeps the current observer system and prompt verbatim on no-answer passes. Final-say reads (optimizer analyst, eval final_content, the notify hook) use trajectory.final_assistant_text — the last assistant turn only, never an earlier narration presented as the conclusion — while last_assistant_text is the salvage walk (task_agent partial-work recovery), skipping tool-call-only, all-reasoning, and whitespace-only turns. Perception memoizes every completed description immediately, including an empty one — one perceive per key, ever — under a commit-lock guard so an empty result never overwrites a concurrently memoized real description; an all-reasoning perception model pins the placeholder until restart, and the remediation is server-side (a reasoning parser or the template thinking toggle on the perception alias). A true double-reasoning shape (inline-extracted text alongside a native reasoning block) logs chars-only at the drain, where it is distinguishable from the routine reasoning_delta mirror. The dialect's semantics are pinned as one table (tests/_reasoning_dialect.py) driven through shared fixtures (think_tag_stream, seam_provider): one-shot conformance, the exact one-shot/streaming equivalence property, the drain seam rules including quoted-tag safety, run-boundary and separator-preservation pins, per-lane pins for all nine lanes, and the empty-content assistant wire shape. Closes #965. Closes #940.	2026-08-05 00:23:11 -07:00
Patrick Buckley	961a2017dc	fix(streaming): delete the retry window's shared slots and gate the send epilogue ... Fourth review round. The recurring defect family — cross-frame session slots racing an orphanable window — is removed structurally instead of gated again: - The wire-fold slot is deleted. The fold the stream was actually created from rides the returned message dict on the underscore lane (like _provider_content) and is popped at the single calibration site before commit, so a superseding generation can never alias it and there is nothing left to clear. Plain-dict test fakes fall through the pop to the frame-local fold. - The stream-provider slot is demoted to a creation-time handoff register: _try_stream stamps it, _stream_response copies it into a frame-local immediately after each create returns, and only that local feeds the retry gate. The fatal formatter returns to the consistent PRIMARY identity triple — pairing a fallback's provider name with the primary's base_url and alias sent operators to debug the wrong backend; stamping the full producing identity is #964. - send()'s epilogue is generation-gated: a superseded thread's escaped death no longer records a fatal error over the healthy successor turn (error banner, buffer-wiping error-state drain, wrong last_error for the coord), and a Ctrl-C on an orphan no longer mutates history. - The terminal arm discards as well as finalizes. Keeping the buffers bought nothing — the fatal path's error-state drain wipes them on every server lane — and the skipped discard let a mid-consumption overflow recovered by compact-and-retry concatenate the dead attempt's text with the recovered answer in the idle payload. Pinned with real-buffer tests for the overflow-recovery and orphan-epilogue paths. - stream.post_finish_blip regains usage_captured, tracked by transport_guarded from the chunks it forwards, restoring missing-spend attribution on both lanes. - TerminalUI.on_thinking_start is idempotent at the callee (a live spinner is stopped before being replaced), removing the caller-side stop-first dance and the leak the next unaware call site would have reintroduced. - The think-tag vocabulary in _strip_reasoning and the title lane is derived from ThinkTagSplitter, closing the drift channel that would leak raw reasoning into compaction summaries and titles. - on_stream_discarded's docstring states the true pending-batch semantics (defensive drop; the shipped sequence flushes via the preceding stream_end), and the live-suite recording fake gains the protocol method.	2026-08-04 04:53:17 -07:00
Patrick Buckley	a1dfe0bd4f	refactor(streaming): dedupe transport conversion, usage merge, cancel finalize ... Three behavior-preserving consolidations behind the #937 fix, each deleting a hand-rolled twin of a now-shared rule: - drain_stream consumes transport_guarded(chunks) and drops its inline `except httpx.TransportError` arm — one conversion rule for mid-body wire deaths across the drained and interactive lanes. The post-finish tolerance now logs under the wrapper's `stream.post_finish_blip` name (formerly `drain_stream.post_finish_blip`) and no longer carries `usage_captured`; changelog notes the rename for external log filters. The possible usage=None result on a post-finish blip is documented on drain_stream itself. - _stream_attempt's hand-rolled per-chunk usage max-merge becomes a local UsageInfo accumulator folded through merge_usage (drain's rule), re-projected into the _last_usage dict on EVERY usage chunk — that dict has mid-stream readers (_estimated_prompt_tokens, the status line), so the per-chunk write timing is load-bearing and unchanged. - The twin cancelled-partial sequences in _stream_attempt's two cancel arms (cooperative GenerationCancelled, stream-close-converted) merge into one local _record_cancelled_partial helper carrying both arms' tool_calls/_provider_content omission rationale in one place.	2026-08-04 04:53:17 -07:00
Patrick Buckley	5fb27e8f81	fix(session): survive mid-stream transport deaths in interactive turns (#937) ... A wire death during body streaming (ReadError on a TLS record failure, peer resets) surfaces after the request has already returned its stream handle, so neither the SDK's request retries nor the creation-time retry ladder ever saw it: the interactive turn died with a bare exception string, the partial output was discarded, and no log trace was left. Utility lanes already survived this through drain_stream's normalization; the interactive loop now gets the same treatment. - transport_guarded() in providers/_protocol.py: drain_stream's transport-death conversion made reusable for consumers that keep streaming semantics. Pre-finish deaths raise the retryable IncompleteStreamError (drain's exact message shape); post-finish blips end the stream cleanly, forfeiting only trailing metadata. - The single-pass chunk consumer renames to _stream_attempt; _stream_response is now the resilient wrapper owning ALL stream acquisition plus a bounded mid-stream re-issue ladder (_MID_STREAM_RETRIES, the shared _stop_retrying predicate with a per-loop cap, cancel-aware exponential backoff). Send()'s overflow compact-and-retry arm now wraps the whole turn and passes re-prepared msgs explicitly. - A dead attempt is finalized across every UI consumer before the retry (stream_end then turn_committed then notice then spinner), so retried text never appends onto the dead attempt's in any surface (browser transcript, CLI markdown fences, Slack/Discord streamed messages, SSE replay ring). - Before re-creating, the session re-resolves its registry binding: a concurrent ModelRegistry.reload() closes cached clients, and the retry must not stream into the closed one. A failing re-create logs stream.retry.recreate_failed and re-raises the ORIGINAL stream-death error rather than masking it. - _format_backend_error gains a stream-death branch naming the provider, endpoint, and model, with a short identity-bearing first sentence. _BACKEND_STREAM_EXC_NAMES joins _BACKEND_KNOWN_EXC_NAMES, which also removes those names from _is_ctx_overflow's text-detection eligibility (deliberate: their texts are fixed transport strings that never carry overflow phrases). - _record_fatal_error now logs session.fatal.recorded (INFO for KeyboardInterrupt, ERROR otherwise) so fatal turns leave a journal trace. - _assistant_pending_tokens resets at stream entry so a post-finish blip that loses the trailing usage chunk cannot append the previous turn's completion count as this turn's estimate. Offline SDK boundary pins (openai/anthropic mid-body death identity and no re-request, cross-thread client close surfacing httpx.ReadError) guard the assumptions the retry gate rests on.	2026-08-04 04:53:17 -07:00
Patrick Buckley	cf7cfe8932	fix(providers): review round 4 — wire-error retryability, tap/mirror slot parity, terminal completeness ... Correctness: - drain_stream chains raw httpx.TransportError from stream iteration into retryable IncompleteStreamError (original type+message preserved via __cause__): streaming moved the body read out of the SDK's APIConnectionError-wrapped request, so mid-body connection drops and read timeouts — retried transparently on 1.7 — were escaping every single-shot retry loop as instantly-fatal raw httpx names. - The index remap is extracted as ToolCallSlotter and GoogleProvider's raw tap slots THROUGH IT over the same delta sequence as the base iterator: round 3's mirror-side de-fusion had left the tap keying by wire index, so a degenerate stream produced 2 mirror calls vs 1 fused raw dict — _prepare_messages' length gate then silently dropped the thought_signature lane (400 on signature-strict Gemini models). - The slotter also splits ID-LESS degenerate parallel calls: a delta announcing a name for a slot that already accumulated arguments is a second whole call, not a fragment (fragmented single calls pinned unaffected). - A payload-less Responses terminal event keeps the provider_blocks already collected from output_item.done events (they came from the stream, not the missing payload); only usage is genuinely lost. - The truncation-rebuild path walks the terminal output's message annotations, so truncated web-search turns keep their Sources footer (the in-flight item never received output_item.done). Cleanup: one _raise_responses_failure ladder serves both in-band failure shapes (error events + response.failed); IncompleteStreamError joins the public providers export (docstrings tell callers to catch it); the de-fusion tests ride the file's existing _openai_stream_chunk helpers instead of a third hand-rolled SSE fake; the dead if-response guard in the terminal branch is gone. Deferred with note: classifying IncompleteStreamError once at the retry-predicate consultation site instead of per-provider strings is #832 territory (the predicate lives in ChatSession); the six-lane parametrized test guards the listing until then.	2026-07-13 22:39:19 -07:00
Patrick Buckley	56b7674dfa	fix(providers): review round 3 — in-band error events, terminal-marker tolerance, adapter-owned de-fusion ... Correctness: - Responses _iter_stream handles the SDK's in-band `error` SSE event (ResponseErrorEvent is YIELDED, not raised, and no response.failed need follow): the real API code/message now surfaces — code-gated for retryability like response.failed — instead of the stream exhausting finish-less and hiding the cause behind a retried IncompleteStreamError. - Anthropic message_stop supplies a missing stop_reason: it is a genuine terminal marker, so a compat /v1/messages shim whose message_delta omits stop_reason completes (blocks intact) rather than failing a generation that arrived — tolerance the retired non-streaming default provided, restored without weakening the died-mid-response gate. - A Responses terminal event without its response payload still emits the finish reason its type implies (lax compat servers), losing only usage/blocks rather than the whole result. Dispositions held (documented, not re-coded): the complete-or-error gate stays for finish-less Chat Completions streams — indistinguishable in-band from a died generation, and silent partial-storage is the worse failure; CHANGELOG now names the shape and each provider's accepted terminal markers. supports_streaming deletion and the stream_options wire delta were ruled earlier and keep their release-note remediations. Cleanup: index-degenerate de-fusion MOVED from drain_stream into the chat adapter's iterator (mirroring the Anthropic iterator's index assignment) so the interactive loop is fixed too and the drain returns to a plain mirror of the main-loop accumulator; a parametrized test locks "IncompleteStreamError is retryable" across all six provider lanes instead of trusting per-adapter memory; scripted_anthropic_client joins scripted_chat_client (shared _ScriptedClient class, no function attrs) and the two remaining hand-rolled anthropic closures convert.	2026-07-13 22:39:19 -07:00
Patrick Buckley	3ffa8b9057	fix(providers): review round 2 — complete-or-error drain, code-gated retries, truncation-safe blocks ... Correctness (3 confirmed + 2 plausible, all fixed): - drain_stream now raises typed, retryable IncompleteStreamError when a stream exhausts without any finish reason — every adapter emits one on a healthy stream, so its absence means the generation died mid-response behind a cleanly-closing proxy. This restores the retired transport's complete-or-error contract (a half-generated compaction summary was previously returned as finish=stop and stored, silently replacing real history) and DELETES round 1's suffix-info fold: with no finish-less success path there is nothing to classify, so a trailing status ping can never be stored as content either. - Index-degenerate parallel tool calls get distinct slots: a delta whose id differs from its slot's opens a new call (id-less fragments still follow their index's current call), so historical compat servers that emit every parallel call at index 0 no longer fuse distinct calls into concatenated garbage arguments. Result order stays index-sorted (stable) like the retired array parse. - response.failed retryability is code-gated: only transient codes (server_error, rate_limit_exceeded) raise the retryable typed error; deterministic rejections (invalid prompt, image fetch, policy) raise plain RuntimeError and stop retry loops on attempt zero instead of running the full backoff ladder against a doomed request. - Terminal Responses events rebuild provider_blocks from response.output when present: the item being generated at max_output_tokens truncation never receives output_item.done, and storing a reasoning item without its required following item made the next turn's replay a 400. - merge_usage's base case uses dataclasses.replace so a future UsageInfo field can't be silently zeroed on drained lanes. Cleanup: run_abortable_with_deadline bundles the three-point abort wiring (ref + cancel_ref + on_abandon) so it cannot be half-wired — both judges converted; scripted_chat_client hoists the 14 chat-lane fake_create closures (call scripts + .calls recording replace per-test counter cells); fake_chat_stream gains reasoning=, collapsing the reasoning-capture suite's hand-rolled chunk shape; FakeAnthropicBlock hoists the duplicated _Block test class; the class and judge PlantUML diagrams drop the retired create_completion flow. Also converts test_model_registry's agent-model fakes, which returned legacy response objects that iterated as EMPTY streams — they only passed through the old drain's silent finish=stop default, exactly the hazard the new gate exists to catch.	2026-07-13 22:39:19 -07:00
Patrick Buckley	08580f25f9	fix(providers): review round 1 — streaming parity gaps the collapse exposed ... Correctness (4 confirmed + 1 plausible fixed, 2 accepted+documented): - Anthropic _iter_anthropic_stream handles citations_delta: text-block citations now ride the raw block into provider_blocks, as replay requires (the retired non-streaming lane preserved them via model_dump; the streaming lane dropped them — a pre-existing main-loop gap the collapse would have extended to single-shot lanes). - Anthropic text blocks separate with "\n" at each subsequent block start, restoring the retired lane's "\n".join rendering on drained lanes AND un-fusing streamed web-search responses in the chat loop. - response.failed raises typed ResponsesStreamFailedError, listed in the provider's retryable_error_names — retry loops treat an in-band failure like the wire errors it stands in for instead of hard-stopping on a bare RuntimeError (judges keep their heuristic fallback after retries). - drain_stream folds a finish-less stream's terminal citations footer (suffix rule: pre-finish info invalidated by any later payload), so lax compat servers that never send finish_reason keep their Sources. - usage max-merge extracted as merge_usage() in _protocol.py — the one definition drain uses now and the session's inline consumer adopts on #832. Accepted + release-noted instead of coded around: strict pre-2024 compat servers that 400 on stream_options (such a server already cannot serve the chat loop; CHANGELOG caveat extended), and repeated-index parallel tool-call merging on legacy compat servers (identical to the main loop's accumulator semantics; a shared guard belongs in the #832 unification). Cleanup: run_with_deadline grows on_abandon (best-effort, cannot mask the deadline error) and both judges drop the copy-pasted abort choreography; StreamAbortRef documents the _CancelRef adoption plan; test_model_turn's fake replays through the shared as_stream adapter; docs/architecture.md drops the retired Protocol row. Tests: refusal handler pinned (was advertised, untested); typed-failed retryability; citations capture; text-block separator (plus the mixed text+search expectation updated for the separator chunk); finish-less citation fold; on_abandon firing matrix; StreamAbortRef arrival race.	2026-07-13 22:39:19 -07:00
Patrick Buckley	1e7ad7bcb6	feat(providers): one transport — drain create_streaming, retire create_completion (#831) ... Every single-shot lane (model_turn: judges, titles, compaction, web-fetch extraction, perception, eval, optimizer) now samples through the provider's streaming entry and accumulates via a shared drain_stream(), deleting create_completion from the Protocol and all three adapters (xai/google inherit). Request shaping can no longer drift between the two consumption styles, and callers keep the exact CompletionResult contract. The drain mirrors the main loop's proven chunk semantics: per-field max-merge for usage (Anthropic splits prompt/completion across message_start/message_delta), tool-call assembly by delta index, provider_blocks from the terminal emission, trailing citation info folded back into content (byte-matching the old format_citations append), mid-stream status pings dropped. Also in this change: - model_turn grows cancel_ref; both judges wire their run_with_deadline abandon paths to a new StreamAbortRef (deadline.py) that closes the SDK stream — a timed-out judge call now aborts its HTTP read instead of pinning a daemon thread until the next upstream chunk. The append hook covers the arrival race, mirroring ChatSession._CancelRef. - Responses streaming gains the response.incomplete terminal handler (truncated runs were mislabeled finish=stop and lost final usage AND collected provider_blocks) and a refusal handler ([Refused: …] content, matching the retired non-streaming rendering). Both also fix the main chat loop, which shared the gaps. - supports_streaming capability flag deleted (zero readers) along with its admin capability tile; o1-era models that reject streaming need a model alias pointing at a current model (release-noted). - Helpers that existed only for the deleted transport go with it: Responses._parse_response, chat/google._extract_tool_calls. Known behavioral deltas (release-noted): OpenAI-compatible servers that ignore stream_options.include_usage stop producing usage rows on these lanes; multiple Anthropic text blocks concatenate without the old "\n" joint (matching the main loop); model_turn lanes no longer risk client read-timeouts on long generations — the reason the Anthropic adapter already drained a stream internally. Tests: new test_drain_stream.py pins the accumulator rules; shared fakes (as_stream, fake_chat_stream, fake_anthropic_stream) migrate 11 suites to the streaming transport, with the task-agent and adapter suites now exercising the real _iter_stream + drain path end to end.	2026-07-13 22:39:19 -07:00