Skip to content

Commit c13b96c

Browse files
committed
fix: make agentic warmup handoff robust
Signed-off-by: Cam Quilici <cjquilici@gmail.com>
1 parent 3d77dd3 commit c13b96c

8 files changed

Lines changed: 179 additions & 31 deletions

File tree

src/aiperf/credit/callback_handler.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -492,23 +492,27 @@ async def on_credit_return(
492492
# behind ``can_send_child_turn`` instead — the phase-level
493493
# sending-complete flag is driven by root sampling exhaustion, not
494494
# by DAG work, but the global ``--request-count`` cap still
495-
# applies. When the cap blocks a non-final child continuation, we
496-
# notify the orchestrator (``on_child_stopped``) so the parent's
497-
# join still drains instead of deadlocking on a child whose
498-
# remaining turns will never be issued. Final-turn child returns
499-
# are always passed through (the strategy is a no-op for them, but
500-
# observer hooks still need to fire).
495+
# applies. In terminal phases, a blocked non-final child notifies the
496+
# orchestrator (``on_child_stopped``) so the parent's join can drain.
497+
# Strategies requesting stopped returns (such as quota warmup) instead
498+
# preserve resumable children across a phase handoff. Final-turn child
499+
# returns are always passed through (the strategy is a no-op for them,
500+
# but observer hooks still need to fire).
501+
wants_stopped_returns = (
502+
getattr(handler.strategy, "wants_returns_after_sending_complete", False)
503+
is True
504+
)
501505
is_child = credit.agent_depth > 0
502506
if not is_child:
503-
wants_stopped_returns = (
504-
getattr(handler.strategy, "wants_returns_after_sending_complete", False)
505-
is True
506-
)
507507
if handler.stop_checker.can_send_any_turn() or wants_stopped_returns:
508508
await handler.strategy.handle_credit_return(
509509
credit, error=credit_return.error
510510
)
511-
elif credit.is_final_turn or handler.stop_checker.can_send_child_turn():
511+
elif (
512+
credit.is_final_turn
513+
or handler.stop_checker.can_send_child_turn()
514+
or wants_stopped_returns
515+
):
512516
await handler.strategy.handle_credit_return(
513517
credit, error=credit_return.error
514518
)

src/aiperf/dataset/loader/weka_synth_buf.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ def _assert_trailing_user(self) -> None:
431431
def turn_delta(self) -> TurnDelta:
432432
"""Compute the raw_messages to emit for the just-completed turn.
433433
434-
Three cases:
434+
Four cases:
435435
1. First call after ``init_turn_0`` (``_emitted_segment_count == 0``):
436436
emit ALL current segments, ``reset_context=False``. This is
437437
turn 0's baseline state.
@@ -441,6 +441,10 @@ def turn_delta(self) -> TurnDelta:
441441
3. Disturbance touched a previously-emitted segment (index
442442
``< _emitted_segment_count``): emit ALL current segments,
443443
``reset_context=True``.
444+
4. An equal-context retry appended no segments: re-emit ALL current
445+
segments with ``reset_context=True``. An empty delta would
446+
otherwise render as an invalid empty user message instead of the
447+
recorded repeated request.
444448
445449
Updates ``_emitted_segment_count`` to ``len(self._segments)`` on
446450
return. Clears ``_last_disturbance_at`` to ``None``.
@@ -449,9 +453,16 @@ def turn_delta(self) -> TurnDelta:
449453
self._last_disturbance_at is not None
450454
and self._last_disturbance_at < self._emitted_segment_count
451455
)
452-
if self._emitted_segment_count == 0 or disturbed_emitted:
456+
unchanged_retry = (
457+
self._emitted_segment_count > 0
458+
and self._emitted_segment_count == len(self._segments)
459+
and not disturbed_emitted
460+
)
461+
if self._emitted_segment_count == 0 or disturbed_emitted or unchanged_retry:
453462
source = self._segments
454-
reset = self._emitted_segment_count != 0 and disturbed_emitted
463+
reset = self._emitted_segment_count != 0 and (
464+
disturbed_emitted or unchanged_retry
465+
)
455466
else:
456467
source = self._segments[self._emitted_segment_count :]
457468
reset = False

src/aiperf/timing/strategies/agentic_replay.py

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -254,8 +254,9 @@ def __init__(
254254
self.scheduler.set_drain_observer(self.enforce_system_idle_cap)
255255
# Idle-gap cap (ms) for the t* boundary the load-time warp can't see (t*
256256
# is the sampling instant, not a request). Consumed two ways:
257-
# - WARMUP: clamps each warmup lead so priming doesn't start hours
258-
# early (``_capped_warmup_lead_ms``); priming spacing is meaningless.
257+
# - WARMUP: this cap and the global system-idle cap both clamp each
258+
# warmup lead so priming doesn't start hours early
259+
# (``_capped_warmup_lead_ms``); priming spacing is meaningless.
259260
# - PROFILING: a single uniform shift caps the leading idle (t* ->
260261
# earliest stream) while preserving recorded inter-stream spacing
261262
# (``_leading_idle_shift_ms``); a per-stream clamp would collapse
@@ -548,9 +549,19 @@ def _capped_warmup_lead_ms(self, lead_ms: float) -> float:
548549
PROFILING dispatch offsets are NOT clamped this way -- see
549550
:meth:`_leading_idle_shift_ms`.
550551
"""
551-
if self._phase_offset_cap_ms is not None:
552-
return min(lead_ms, self._phase_offset_cap_ms)
553-
return lead_ms
552+
caps_ms = [
553+
cap
554+
for cap in (
555+
self._phase_offset_cap_ms,
556+
(
557+
self._system_idle_gap_cap_seconds * MILLIS_PER_SECOND
558+
if self._system_idle_gap_cap_seconds is not None
559+
else None
560+
),
561+
)
562+
if cap is not None
563+
]
564+
return min(lead_ms, *caps_ms) if caps_ms else lead_ms
554565

555566
def _leading_idle_shift_ms(self, offsets: Iterable[float]) -> float:
556567
"""Excess to subtract UNIFORMLY from every PROFILING dispatch offset so
@@ -878,15 +889,21 @@ async def _handle_accelerated_warmup_return(self, credit: Credit) -> None:
878889
await self.credit_issuer.issue_credit(turn)
879890

880891
async def _issue_child_continuation_or_drain(self, turn: TurnToSend) -> None:
881-
"""Dispatch a DAG child continuation, draining the join on refusal.
892+
"""Dispatch a DAG child continuation, draining terminal refusals.
882893
883894
``dispatch_child_turn`` returns True iff the turn reached the wire; on
884-
any refusal (e.g. the ``--request-count`` cap) notify the orchestrator
885-
so the parent's join drains deterministically instead of deadlocking on
886-
a child whose remaining turns will never be issued.
895+
a terminal refusal notify the orchestrator so the parent's join drains
896+
deterministically instead of deadlocking on a child whose remaining
897+
turns will never be issued. Accelerated warmup refusals are different:
898+
the remaining child and its active join are persisted for profiling, so
899+
marking the child stopped here would release the parent prematurely.
887900
"""
888901
on_wire = await self.credit_issuer.dispatch_child_turn(turn)
889-
if not on_wire and self.branch_orchestrator is not None:
902+
if (
903+
not on_wire
904+
and self.branch_orchestrator is not None
905+
and not self.allows_pending_branch_handoff_after_sending_complete
906+
):
890907
await self.branch_orchestrator.on_child_stopped(turn.x_correlation_id)
891908

892909
async def finalize_phase(self) -> None:
@@ -1460,11 +1477,12 @@ async def _dispatch_next_turn(self, credit: Credit) -> None:
14601477
14611478
DAG child continuations (``agent_depth > 0``) go through the single
14621479
child-issuance chokepoint (``_issue_child_continuation_or_drain``) so a
1463-
``--request-count`` cap refusal is routed to ``on_child_stopped`` (drain
1464-
the parent join) instead of being silently swallowed by the discarded
1465-
``issue_credit`` return -- including on the delayed (``delay_ms``) path,
1466-
where the refusal would otherwise fire long after the callback handler
1467-
decided the child could proceed. Root continuations keep ``issue_credit``.
1480+
terminal refusal is routed to ``on_child_stopped`` (drain the parent
1481+
join) instead of being silently swallowed by the discarded
1482+
``issue_credit`` return. A warmup cutoff is preserved for profiling
1483+
handoff instead. This applies equally to delayed continuations, whose
1484+
refusal may happen after the callback handler decided the child could
1485+
proceed. Root continuations keep ``issue_credit``.
14681486
"""
14691487
next_meta = self.conversation_source.get_next_turn_metadata(credit)
14701488
turn = TurnToSend.from_previous_credit(credit, next_meta)

tests/unit/credit/test_callback_handler.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,42 @@ async def test_cache_warmup_handoff_allows_paused_dag_work(
790790
mock_branch_orchestrator.has_pending_branch_work.assert_called_once_with()
791791

792792

793+
@pytest.mark.asyncio
794+
async def test_cache_warmup_handoff_preserves_non_final_child(
795+
callback_handler,
796+
mock_progress,
797+
mock_lifecycle,
798+
mock_stop_checker,
799+
mock_strategy,
800+
mock_branch_orchestrator,
801+
):
802+
"""A quota-stopped warmup child remains live for profiling handoff."""
803+
mock_stop_checker.can_send_child_turn = MagicMock(return_value=False)
804+
mock_strategy.wants_returns_after_sending_complete = True
805+
mock_branch_orchestrator.has_pending_branch_work = MagicMock(return_value=True)
806+
mock_branch_orchestrator.intercept = AsyncMock(return_value=False)
807+
mock_branch_orchestrator.on_child_stopped = AsyncMock()
808+
callback_handler.set_branch_orchestrator(mock_branch_orchestrator)
809+
callback_handler.register_phase(
810+
phase=CreditPhase.WARMUP,
811+
progress=mock_progress,
812+
lifecycle=mock_lifecycle,
813+
stop_checker=mock_stop_checker,
814+
strategy=mock_strategy,
815+
)
816+
817+
credit = make_credit(
818+
phase=CreditPhase.WARMUP,
819+
turn_index=1,
820+
num_turns=7,
821+
agent_depth=1,
822+
)
823+
await callback_handler.on_credit_return("worker-1", make_credit_return(credit))
824+
825+
mock_strategy.handle_credit_return.assert_awaited_once_with(credit, error=None)
826+
mock_branch_orchestrator.on_child_stopped.assert_not_awaited()
827+
828+
793829
# =============================================================================
794830
# Test: Credit Return - Unregistered/Complete Phase
795831
# =============================================================================

tests/unit/dataset/loader/test_weka_pathological.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,29 @@ def test_duplicate_hash_ids_in_request_inflate_theoretical_hit_to_full(tmp_path)
501501
)
502502

503503

504+
def test_equal_context_retry_reemits_nonempty_full_prompt(tmp_path):
505+
"""An identical retry must not become an invalid empty user message."""
506+
trace = _base_trace(
507+
[
508+
_normal(0.0, [1, 2], in_tokens=128),
509+
_normal(1.0, [1, 2], in_tokens=128),
510+
],
511+
trace_id="equal_retry",
512+
)
513+
path = tmp_path / "t.json"
514+
path.write_text(json.dumps(trace))
515+
loader = _make_loader(path, _mk_user_config())
516+
517+
convs = loader.convert_to_conversations(loader.load_dataset())
518+
first, retry = convs[0].turns
519+
520+
assert first.raw_messages
521+
assert retry.raw_messages == first.raw_messages
522+
assert retry.reset_context is True
523+
assert retry.raw_messages[-1]["role"] == "user"
524+
assert retry.raw_messages[-1]["content"]
525+
526+
504527
def test_empty_requests_trace_reconstructs_empty_conversation(tmp_path):
505528
"""A trace with zero requests yields a single empty Conversation, no crash."""
506529
trace = _base_trace([], trace_id="empty_trace")

tests/unit/dataset/loader/test_weka_synth_buf_turn_delta.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,36 @@ def test_turn_delta_case_1_strict_append_emits_only_new_segments():
119119
assert r._last_disturbance_at is None
120120

121121

122+
def test_turn_delta_equal_context_retry_reemits_full_context():
123+
"""An unchanged retry resets to the full prompt instead of emitting ``[]``."""
124+
r = _make_recon()
125+
hash_ids = [1, 2]
126+
in_tokens = 2 * BLOCK_SIZE
127+
r.init_turn_0(
128+
hash_ids=hash_ids,
129+
in_tokens=in_tokens,
130+
tool_tokens=0,
131+
system_tokens=0,
132+
seed="t:0",
133+
)
134+
first = r.turn_delta()
135+
136+
r.advance_turn(
137+
prev_hash_ids=hash_ids,
138+
prev_in_tokens=in_tokens,
139+
prev_out_tokens=BLOCK_SIZE,
140+
curr_hash_ids=hash_ids,
141+
curr_in_tokens=in_tokens,
142+
seed="t:1",
143+
)
144+
retry = r.turn_delta()
145+
146+
assert retry.reset_context is True
147+
assert retry.delta_messages == first.delta_messages
148+
assert retry.delta_messages
149+
assert retry.delta_messages[-1]["role"] == "user"
150+
151+
122152
def test_turn_delta_case_1_strict_append_three_turns_chain():
123153
"""Three sequential strict-append advances: each delta is incremental."""
124154
r = _make_recon()

tests/unit/timing/strategies/test_agentic_replay.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -818,8 +818,10 @@ async def capture(turn):
818818
credit_issuer=issuer,
819819
lifecycle=lifecycle,
820820
)
821-
# Idle-gap cap of 60s (what the agentx scenario sets).
822-
strategy._phase_offset_cap_ms = 60_000.0
821+
# The AgentX scenario sets the global system-idle cap, not the per-trace
822+
# timestamp-warp cap. Warmup priming must honor that real configuration.
823+
strategy._phase_offset_cap_ms = None
824+
strategy._system_idle_gap_cap_seconds = 60.0
823825

824826
await strategy.setup_phase()
825827
await strategy.execute_phase()

tests/unit/timing/strategies/test_agentic_replay_child_continuation.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,14 @@ def _make_strategy(
1919
branch_orchestrator: MagicMock | None = None,
2020
dispatch_result: bool = True,
2121
delay_ms: float | None = None,
22+
phase: CreditPhase = CreditPhase.PROFILING,
23+
accelerated_warmup: bool = False,
2224
) -> tuple[AgenticReplayStrategy, MagicMock, MagicMock]:
2325
"""Build a strategy with only the attributes ``_dispatch_next_turn`` reads."""
2426
strategy = AgenticReplayStrategy.__new__(AgenticReplayStrategy)
27+
strategy.config = MagicMock(phase=phase)
28+
strategy._cache_warmup_duration = None
29+
strategy._cache_warmup_requests_per_lane = 10 if accelerated_warmup else None
2530

2631
conversation_source = MagicMock()
2732
conversation_source.get_next_turn_metadata.return_value = TurnMetadata(
@@ -104,6 +109,25 @@ async def test_child_at_cap_routes_to_on_child_stopped() -> None:
104109
orch.on_child_stopped.assert_awaited_once_with("child-xcid")
105110

106111

112+
@pytest.mark.asyncio
113+
async def test_child_at_warmup_quota_is_preserved_for_profiling_handoff() -> None:
114+
"""A resumable warmup refusal must not prematurely satisfy the parent join."""
115+
orch = MagicMock()
116+
orch.on_child_stopped = AsyncMock()
117+
strategy, issuer, _ = _make_strategy(
118+
branch_orchestrator=orch,
119+
dispatch_result=False,
120+
phase=CreditPhase.WARMUP,
121+
accelerated_warmup=True,
122+
)
123+
124+
await strategy._dispatch_next_turn(_child_credit())
125+
126+
issuer.dispatch_child_turn.assert_awaited_once()
127+
issuer.issue_credit.assert_not_called()
128+
orch.on_child_stopped.assert_not_called()
129+
130+
107131
@pytest.mark.asyncio
108132
async def test_child_at_cap_without_orchestrator_swallows_silently() -> None:
109133
"""No orchestrator wired: refusal must not raise."""

0 commit comments

Comments
 (0)