Skip to content

Commit f538708

Browse files
committed
fix(soul): re-check spend ceiling after billable compaction
The per-session spend ceiling was checked only at the top of the step loop, but proactive compact_context() makes a billable LLM call that raises _session_cost_usd. A session just under max_session_cost_usd could therefore pay for compaction and still run a full model step before the guard re-fired. Add a second ceiling check immediately after the proactive compaction (before _step()), reusing the existing budget_exhausted handoff. Regression test asserts no model step runs once compaction pushes the session over the ceiling.
1 parent 459ff11 commit f538708

2 files changed

Lines changed: 56 additions & 0 deletions

File tree

src/pythinker_code/soul/pythinkersoul.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1567,6 +1567,21 @@ async def _agent_loop(self) -> TurnOutcome:
15671567
)
15681568
raise
15691569

1570+
# Compaction makes a billable LLM call that folds into
1571+
# self._session_cost_usd. Re-check the ceiling here so a session
1572+
# just under the limit cannot pay for compaction *and* a full
1573+
# step before the top-of-loop guard fires again next iteration.
1574+
if _is_over_cost_ceiling(self._session_cost_usd, ceiling):
1575+
assert ceiling is not None # narrowed by _is_over_cost_ceiling
1576+
message = _budget_exhausted_message(self._session_cost_usd, ceiling)
1577+
await self._context.append_message(message)
1578+
wire_send(TextPart(text=message.extract_text(" ")))
1579+
return TurnOutcome(
1580+
stop_reason="budget_exhausted",
1581+
final_message=message,
1582+
step_count=step_no - 1, # this step's _step() never ran
1583+
)
1584+
15701585
logger.debug("Beginning step {step_no}", step_no=step_no)
15711586
await self._checkpoint()
15721587
self._denwa_renji.set_n_checkpoints(self._context.n_checkpoints)

tests/core/test_pythinkersoul_stuck_loop.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,47 @@ async def test_session_cost_ceiling_stops_turn(runtime: Runtime, tmp_path: Path)
431431
assert "ceiling" in context.history[-1].extract_text(" ").lower()
432432

433433

434+
@pytest.mark.asyncio
435+
async def test_compaction_overspend_stops_before_next_step(
436+
runtime: Runtime, tmp_path: Path
437+
) -> None:
438+
"""Proactive compaction makes its own billable LLM call. If that call pushes the
439+
session over the spend ceiling, the turn must stop *before* the following (paid)
440+
model step — not pay for compaction and then run a full step before the
441+
top-of-loop guard re-fires next iteration.
442+
443+
The load-bearing assertion is ``generate_attempts == 0``: no model step ran after
444+
the billed compaction. On the old code (single ceiling check at the loop top) the
445+
extra ``_step()`` would run first (``generate_attempts == 1``), then the *next*
446+
iteration's guard would still stop with ``budget_exhausted`` — so the stop_reason
447+
and history-text checks below pass on both old and new code and only corroborate.
448+
"""
449+
runtime.config.loop_control.max_session_cost_usd = 1.0
450+
runtime.config.loop_control.max_steps_per_turn = 50
451+
# Under the ceiling on entry (and > 0, so the entry guard passes for the right
452+
# reason — _is_over_cost_ceiling fails open at 0.0 for unpriced models).
453+
provider = _ScriptedToolCallProvider(["Ok"] * 10)
454+
context, soul = _make_soul(runtime, provider, tmp_path)
455+
soul._session_cost_usd = 0.5
456+
457+
async def _fake_compact(*_args: object, **_kwargs: object) -> None:
458+
# The compaction LLM call's spend folds into the cumulative total, pushing
459+
# the session strictly over the ceiling.
460+
soul._session_cost_usd = 5.0
461+
462+
with (
463+
patch("pythinker_code.soul.pythinkersoul.should_auto_compact", return_value=True),
464+
patch.object(soul, "compact_context", _fake_compact),
465+
patch("pythinker_code.telemetry.metrics.record_turn") as record_turn,
466+
):
467+
await run_soul(soul, "go", _drain_ui_messages, asyncio.Event())
468+
469+
# No model step ran after the billed compaction (the fix; old code -> 1).
470+
assert provider.generate_attempts == 0
471+
assert record_turn.call_args.kwargs["stop_reason"] == "budget_exhausted"
472+
assert "ceiling" in context.history[-1].extract_text(" ").lower()
473+
474+
434475
@pytest.mark.asyncio
435476
async def test_no_cost_ceiling_does_not_stop(runtime: Runtime, tmp_path: Path) -> None:
436477
"""With no ceiling configured (default None), accumulated cost never stops the turn."""

0 commit comments

Comments
 (0)