Skip to content

Commit 7cdbf26

Browse files
committed
Merge branch 'pr2-add-vero' into pr3-harness-bench
2 parents 3bae4a8 + 77f119b commit 7cdbf26

2 files changed

Lines changed: 108 additions & 22 deletions

File tree

vero/src/vero/evaluation/engine.py

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -363,14 +363,23 @@ async def _execute_record(
363363
).load()
364364
await asyncio.shield(self._record(cancelled))
365365
if charged:
366-
await asyncio.shield(
367-
self.budget_ledger.refund(
368-
backend_id,
369-
request.evaluation_set,
370-
cost,
371-
principal,
366+
try:
367+
await asyncio.shield(
368+
self.budget_ledger.refund(
369+
backend_id,
370+
request.evaluation_set,
371+
cost,
372+
principal,
373+
)
372374
)
373-
)
375+
except Exception as refund_error:
376+
# The shield covers cancellation, not a refund that fails on
377+
# its own: an OSError from the ledger's write propagates here
378+
# in place of error, so the caller never sees the original
379+
# signal while the reservation stays charged -- the leak these
380+
# handlers exist to prevent. Chain it and re-raise the
381+
# original, as BudgetStore.refund already does.
382+
raise error from refund_error
374383
raise
375384
except EvaluationExecutionError as error:
376385
failure = EvaluationStore(
@@ -381,16 +390,25 @@ async def _execute_record(
381390
# refund and leak the reservation's budget.
382391
await asyncio.shield(self._record(failure))
383392
if charged:
384-
await asyncio.shield(
385-
self.budget_ledger.refund(
386-
backend_id,
387-
request.evaluation_set,
388-
cost,
389-
principal,
393+
try:
394+
await asyncio.shield(
395+
self.budget_ledger.refund(
396+
backend_id,
397+
request.evaluation_set,
398+
cost,
399+
principal,
400+
)
390401
)
391-
)
402+
except Exception as refund_error:
403+
# The shield covers cancellation, not a refund that fails on
404+
# its own: an OSError from the ledger's write propagates here
405+
# in place of error, so the caller never sees the original
406+
# signal while the reservation stays charged -- the leak these
407+
# handlers exist to prevent. Chain it and re-raise the
408+
# original, as BudgetStore.refund already does.
409+
raise error from refund_error
392410
raise
393-
except asyncio.CancelledError:
411+
except asyncio.CancelledError as cancellation:
394412
# Last line of defence for the reservation. The evaluator's own
395413
# handlers convert cancellation and failure into the two typed errors
396414
# above, but each of them has to await a persist before it can raise,
@@ -403,14 +421,23 @@ async def _execute_record(
403421
# evaluation id, so there is no record to load. The evaluator already
404422
# persisted one on its shielded paths.
405423
if charged:
406-
await asyncio.shield(
407-
self.budget_ledger.refund(
408-
backend_id,
409-
request.evaluation_set,
410-
cost,
411-
principal,
424+
try:
425+
await asyncio.shield(
426+
self.budget_ledger.refund(
427+
backend_id,
428+
request.evaluation_set,
429+
cost,
430+
principal,
431+
)
412432
)
413-
)
433+
except Exception as refund_error:
434+
# The shield covers cancellation, not a refund that fails on
435+
# its own: an OSError from the ledger's write propagates here
436+
# in place of cancellation, so the caller never sees the original
437+
# signal while the reservation stays charged -- the leak these
438+
# handlers exist to prevent. Chain it and re-raise the
439+
# original, as BudgetStore.refund already does.
440+
raise cancellation from refund_error
414441
raise
415442
# Shielded for the same reason as the two handlers above, and one more:
416443
# the budget was charged for an evaluation that has now actually run, so

vero/tests/test_v05_evaluation_runtime.py

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,65 @@ async def test_execution_error_refunds_reservation(tmp_path: Path):
789789
)
790790

791791

792+
@pytest.mark.asyncio
793+
async def test_engine_refund_failure_preserves_the_cancellation(
794+
tmp_path: Path, monkeypatch
795+
):
796+
"""A refund that fails on its own must not replace the CancelledError.
797+
798+
The engine's last-line-of-defence handler shields the refund against
799+
cancellation, but not against the refund raising by itself. An OSError from
800+
the ledger's write then propagated in place of the cancellation, so the
801+
caller's structured scope never saw the teardown signal while the
802+
reservation stayed charged -- the exact leak this handler exists to prevent.
803+
"""
804+
805+
class BlockingBackend(StubBackend):
806+
async def evaluate(self, *, context, request):
807+
self.evaluate_calls += 1
808+
await asyncio.Event().wait()
809+
810+
workspace = StubWorkspace(tmp_path / "repo")
811+
backend = BlockingBackend()
812+
evaluation_set = request().evaluation_set
813+
ledger = BudgetLedger(
814+
[
815+
EvaluationBudget(
816+
backend_id="default",
817+
evaluation_set_key=evaluation_set.budget_key("default"),
818+
total_runs=1,
819+
)
820+
],
821+
path=tmp_path / "budgets.json",
822+
)
823+
ledger.save()
824+
825+
async def failing_refund(*args, **kwargs):
826+
raise OSError("durable refund write failed")
827+
828+
monkeypatch.setattr(ledger, "refund", failing_refund)
829+
830+
engine = EvaluationEngine(
831+
evaluator=evaluator(tmp_path, workspace),
832+
backends=BackendRegistry({"default": backend}),
833+
database=EvaluationDatabase(id="session"),
834+
database_path=tmp_path / "database.json",
835+
budget_ledger=ledger,
836+
authorization_resolver=allow_all_evaluations,
837+
)
838+
evaluation = asyncio.create_task(
839+
engine.evaluate_record(backend_id="default", request=request())
840+
)
841+
while backend.evaluate_calls == 0:
842+
await asyncio.sleep(0)
843+
evaluation.cancel()
844+
845+
# The cancellation wins; the refund failure is chained, not substituted.
846+
with pytest.raises(asyncio.CancelledError) as raised:
847+
await evaluation
848+
assert isinstance(raised.value.__cause__, OSError)
849+
850+
792851
@pytest.mark.asyncio
793852
async def test_cancelled_evaluation_is_terminal_indexed_and_refunded(tmp_path: Path):
794853
class BlockingBackend(StubBackend):

0 commit comments

Comments
 (0)