Skip to content

Commit 4f64e85

Browse files
committed
fix: Stop duplicate critic side effects during replay
1 parent b7e0672 commit 4f64e85

2 files changed

Lines changed: 39 additions & 1 deletion

File tree

openhands_cli/tui/widgets/richlog_visualizer.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ def _render_history_event(
528528
if not critic_widgets:
529529
return current_anchor
530530

531-
self._emit_critic_result_side_effects(critic_result)
531+
# Skip side effects (PostHog, CriticResultReceived) during replay
532532
for critic_widget in critic_widgets:
533533
self._add_widget_after(current_anchor, critic_widget)
534534
current_anchor = critic_widget

tests/tui/widgets/test_richlog_visualizer.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,44 @@ async def test_load_more_events_pairs_action_and_observation(
389389
assert SUCCESS_ICON in str(collapsible.title)
390390
assert history_widget_labels(container)[1:] == ["> msg2", "> msg3"]
391391

392+
@pytest.mark.asyncio
393+
async def test_replay_does_not_emit_critic_side_effects(self, mock_cli_settings):
394+
"""Replaying history should render critic widgets but NOT emit side effects.
395+
396+
Side effects include PostHog analytics events and CriticResultReceived
397+
messages. These must only fire on the live path, not during replay.
398+
"""
399+
from openhands.sdk.critic.result import CriticResult
400+
401+
# Create a MessageEvent with a critic_result attached
402+
message = Message(role="assistant", content=[TextContent(text="analysis")])
403+
event = MessageEvent(llm_message=message, source="agent")
404+
event = event.model_copy(
405+
update={"critic_result": CriticResult(score=0.85, message="Good: 0.85")}
406+
)
407+
408+
events = [create_user_message_event("hi"), event]
409+
app = ReplayHistoryTestApp(events)
410+
411+
async with app.run_test() as pilot:
412+
container = app.query_one("#scroll_view", VerticalScroll)
413+
visualizer = ConversationVisualizer(container, cast(OpenHandsApp, app))
414+
415+
with mock_cli_settings(visualizer=visualizer, default_cells_expanded=True):
416+
with patch.object(
417+
visualizer, "_emit_critic_result_side_effects"
418+
) as mock_emit:
419+
# Replay all events via _render_history_event
420+
anchor = container
421+
for ev in events:
422+
if ConversationVisualizer.is_user_initiated_message(ev):
423+
visualizer.render_user_message(str(ev.visualize).strip())
424+
else:
425+
anchor = visualizer._render_history_event(ev, anchor)
426+
await pilot.pause()
427+
428+
mock_emit.assert_not_called()
429+
392430

393431
class TestVisualizerWithoutEscaping:
394432
"""Tests that demonstrate what happens WITHOUT the escaping fix.

0 commit comments

Comments
 (0)