Skip to content

Commit 5b808da

Browse files
committed
fix: address PR review findings
- Log CPR-settle failures in `_settle_cursor_after_handoff` via `_handoff_trace` + `logger.debug` instead of silently suppressing, matching the sibling `_reset_prompt_renderer` / `_safe_prompt_invalidate` handlers (no silent swallow). - Construct `OAuthManager` off the event loop in `refresh_managed_models` via `asyncio.to_thread`, so its synchronous `_migrate_oauth_storage()` file lock cannot stall async refresh. - Add a failure-path test asserting the scrollback-handoff `finally` still settles the cursor (and the fail path resets the renderer) when emit raises.
1 parent 12bc0c0 commit 5b808da

3 files changed

Lines changed: 63 additions & 2 deletions

File tree

src/pythinker_code/auth/platforms.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from __future__ import annotations
22

3+
import asyncio
34
import os
45
from typing import Any, NamedTuple, cast
56

@@ -353,7 +354,10 @@ def providers_match(
353354
if provider.oauth and oauth_manager is None:
354355
from pythinker_code.auth.oauth import OAuthManager
355356

356-
oauth_manager = OAuthManager(working_config)
357+
# Construct off the event loop: OAuthManager.__init__ can run
358+
# _migrate_oauth_storage(), which takes a synchronous cross-process
359+
# file lock (5s timeout) and would otherwise stall async refresh.
360+
oauth_manager = await asyncio.to_thread(OAuthManager, working_config)
357361
provider = working_config.providers.get(provider_key)
358362
if provider is None:
359363
continue

src/pythinker_code/ui/shell/visualize/_interactive.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,8 +246,11 @@ async def _settle_cursor_after_handoff(self) -> None:
246246
output = getattr(app, "output", None)
247247
if output is None or not getattr(output, "responds_to_cpr", False):
248248
return
249-
with suppress(Exception):
249+
try:
250250
await app.renderer.wait_for_cpr_responses()
251+
except Exception as exc: # noqa: BLE001 — settling is best-effort, must not break cleanup
252+
_handoff_trace(f"CPR_SETTLE_FAIL\t{type(exc).__name__}:{exc}")
253+
logger.debug("CPR settle failed after scrollback handoff: {}", exc)
251254

252255
def _defer_scrollback_handoff(self) -> bool:
253256
"""Backpressure: defer permanent scrollback while preamble geometry is unstable."""

tests/ui_and_conv/test_visualize_running_prompt.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,60 @@ async def _settle() -> None:
272272
assert view._scrollback_handoff_depth == 0
273273

274274

275+
@pytest.mark.asyncio
276+
async def test_scrollback_handoff_settles_cursor_on_failure(monkeypatch) -> None:
277+
"""When the handoff emit raises, the finally block must still settle the cursor
278+
(and the fail path resets the renderer) before the exception propagates — the
279+
ordering guarantee that keeps the prompt recoverable after a failed handoff.
280+
"""
281+
from pythinker_code.ui.shell.visualize._blocks import _ContentBlock
282+
283+
events: list[str] = []
284+
depth_at_settle: list[int] = []
285+
286+
class _PromptSession:
287+
def invalidate(self) -> None:
288+
return None
289+
290+
async def _run_in_terminal(func, *args, **kwargs): # noqa: ANN001, ANN002, ANN003
291+
raise RuntimeError("emit failed")
292+
293+
monkeypatch.setattr(_interactive_mod, "run_in_terminal", _run_in_terminal)
294+
monkeypatch.setattr(_live_view_mod.console, "_force_terminal", True)
295+
monkeypatch.setattr(
296+
_live_view_mod,
297+
"emit_scrollback_block",
298+
lambda _console, renderable: None,
299+
)
300+
301+
view = _PromptLiveView(
302+
StatusUpdate(),
303+
prompt_session=cast(Any, _PromptSession()),
304+
steer=lambda _content: None,
305+
)
306+
307+
async def _settle() -> None:
308+
events.append("settle")
309+
depth_at_settle.append(view._scrollback_handoff_depth)
310+
311+
monkeypatch.setattr(view, "_settle_cursor_after_handoff", _settle)
312+
monkeypatch.setattr(
313+
view, "_reset_prompt_renderer", lambda reason: events.append(f"reset:{reason}")
314+
)
315+
block = _ContentBlock(is_think=False)
316+
block.append("First paragraph.\n\nMutable tail")
317+
assert block._committed_renderables
318+
view._current_content_block = block
319+
320+
with pytest.raises(RuntimeError, match="emit failed"):
321+
await view._emit_incremental_content_commits()
322+
323+
assert "settle" in events, "finally must settle the cursor even when the handoff raises"
324+
assert "reset:handoff-fail" in events, "fail path must reset the renderer"
325+
assert depth_at_settle == [1], "cursor settles while the handoff is still suppressed"
326+
assert view._scrollback_handoff_depth == 0
327+
328+
275329
def test_status_loop_has_no_midstream_commit_throttle() -> None:
276330
"""Regression guard: the per-tick mid-stream commit (the source of the
277331
run_in_terminal "jump") must not come back. Completed prose stays in the

0 commit comments

Comments
 (0)