fix(major2): en skrivefeil i finally fortrenger ikke lenger stoppet i flukt
Funn d71a5d72 (MINOR, MISSING_ERROR_HANDLING, run.py:1202). Begge finally-skriverne ligger i propageringsstien til nettopp det unntaket de er bevis for: en OSError fra mkdir/write_text mens BudgetExceeded eller ProposalReviewInputError er i flukt ERSTATTER den - og hverken `except ProposalReviewInputError` (:3404) eller nekt-tuppelen (:3412) fanger OSError, saa operatoeren fikk traceback og grunnen til at kjoeringen stoppet var borte. Review-skriveren gaar paa HVER kjoering med reviewer; parse-skriveren har samme form. `_write_or_report` er EN kopi for begge kallstedene (koe-(p)): en regel om hva en skriver faar gjoere med et unntak i flukt, kopiert, blir en regel anvendt paa bare det ene. Vakten er BETINGET, aldri en blanket except - uten noe i flukt finnes ingen stoppgrunn aa beskytte, og en kjoering som ikke fikk skrevet utboksen maa si fra ved aa feile. Feilen SIES uansett, fordi et fravaerende artefakt ellers leses som en kjoering uten noe aa registrere (T10/T11). `in_flight` fanges eksplisitt (`except BaseException as stop: ... raise`), ikke via `sys.exc_info()`, som ville lest et ytre except-lag hos en bibliotekkaller som en flukt her. MAALT mot HELE suiten, to mutasjoner, hver med sin egen signatur, kontroll 1367 passed / 5 skipped og golden `demo-transcript.stdout` BYTE-UENDRET (`shasum -a 1` av INNHOLDET = ea8c534773acdbe41ae68f2c55724d69aaf8be4f): MC vakten detached (2 roede - de to in-flight-armene) - MD svelg ubetinget (1 roed - KONTROLL-armen alene, altsaa er betingelsen selv gatet). Iron Law: begge in-flight-armene skrevet FOERST og maalt roede mot uendret run.py; kontroll-armen var groenn foer fiksen, som er nettopp diskrimineringen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
d4a0220b68
commit
257f44f0cb
2 changed files with 136 additions and 9 deletions
|
|
@ -672,6 +672,36 @@ def _features_of(proposal: SavingsProposal) -> ProposalFeatures:
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _write_or_report(
|
||||||
|
write: Callable[[], object], *, what: str, in_flight: BaseException | None
|
||||||
|
) -> None:
|
||||||
|
"""Run ONE ``finally`` writer so a disk error can never DISPLACE the run's stop reason.
|
||||||
|
|
||||||
|
Both artefacts below are written from a ``finally``, which is what makes them survive the run
|
||||||
|
that most needs them — and is also what put them in the propagation path of the very exception
|
||||||
|
they are evidence for. An ``OSError`` raised there REPLACES the in-flight ``BudgetExceeded`` or
|
||||||
|
``ProposalReviewInputError``: neither the CLI's ``except ProposalReviewInputError`` nor its
|
||||||
|
refusal tuple catches ``OSError``, so the operator got a traceback and the reason the run
|
||||||
|
stopped was gone.
|
||||||
|
|
||||||
|
The guard is CONDITIONAL, never a blanket ``except``: with nothing in flight there is no stop
|
||||||
|
reason to protect, and a run that could not write its outbox must say so by failing —
|
||||||
|
downgrading that to a clean return reports a success the run cannot evidence. Either way the
|
||||||
|
failure is SAID, because a missing artefact would otherwise read as a run that had nothing to
|
||||||
|
record, which is exactly the distinction T10/T11 exist to keep.
|
||||||
|
|
||||||
|
ONE helper for two call sites (kø-(p)): a rule about what a writer may do to an in-flight
|
||||||
|
exception, copied, is a rule that ends up applied to only one of them."""
|
||||||
|
import sys # deferred exactly as ``main()`` does — this module keeps ``sys`` off its top level
|
||||||
|
|
||||||
|
try:
|
||||||
|
write()
|
||||||
|
except OSError as exc:
|
||||||
|
print(f"run warning: could not write {what}: {exc}", file=sys.stderr)
|
||||||
|
if in_flight is None:
|
||||||
|
raise
|
||||||
|
|
||||||
|
|
||||||
def _default_factory(profile: Profile | str) -> Callable[[str], BaseChatClient]:
|
def _default_factory(profile: Profile | str) -> Callable[[str], BaseChatClient]:
|
||||||
def factory(role: str) -> BaseChatClient:
|
def factory(role: str) -> BaseChatClient:
|
||||||
return get_backend(profile).create_chat_client(model=resolve_model(profile, role))
|
return get_backend(profile).create_chat_client(model=resolve_model(profile, role))
|
||||||
|
|
@ -1176,11 +1206,17 @@ async def run_project(
|
||||||
|
|
||||||
coverage: tuple[ApproachOutcome, ...] = ()
|
coverage: tuple[ApproachOutcome, ...] = ()
|
||||||
evaluated: tuple[tuple[str, ValidatedProposal | Rejection], ...] = ()
|
evaluated: tuple[tuple[str, ValidatedProposal | Rejection], ...] = ()
|
||||||
|
in_flight: BaseException | None = None
|
||||||
try:
|
try:
|
||||||
if mandate is None:
|
if mandate is None:
|
||||||
validator_outcome = await _evaluate(None)
|
validator_outcome = await _evaluate(None)
|
||||||
else:
|
else:
|
||||||
validator_outcome, coverage, evaluated = await _evaluate_mandate(mandate, _evaluate)
|
validator_outcome, coverage, evaluated = await _evaluate_mandate(mandate, _evaluate)
|
||||||
|
except BaseException as stop:
|
||||||
|
# Recorded and re-raised UNTOUCHED. This arm decides nothing about the exception itself —
|
||||||
|
# only what the two writers in the ``finally`` are allowed to do to it (``_write_or_report``).
|
||||||
|
in_flight = stop
|
||||||
|
raise
|
||||||
finally:
|
finally:
|
||||||
# ``finally``, not ``except BudgetExceeded``: the round ledger is today's known way out, but
|
# ``finally``, not ``except BudgetExceeded``: the round ledger is today's known way out, but
|
||||||
# any exception leaving generation destroys the same evidence, and a per-exception-type list
|
# any exception leaving generation destroys the same evidence, and a per-exception-type list
|
||||||
|
|
@ -1188,10 +1224,14 @@ async def run_project(
|
||||||
# file's presence is the signal (a run whose replies all parse leaves the outbox unchanged).
|
# file's presence is the signal (a run whose replies all parse leaves the outbox unchanged).
|
||||||
if outbox_dir is not None and parse_failures:
|
if outbox_dir is not None and parse_failures:
|
||||||
assert run_id is not None # narrowed by the step-0 guard (no wall-clock default)
|
assert run_id is not None # narrowed by the step-0 guard (no wall-clock default)
|
||||||
outbox.write_parse_failures(
|
_write_or_report(
|
||||||
outbox_dir,
|
lambda: outbox.write_parse_failures(
|
||||||
run_id,
|
outbox_dir,
|
||||||
failures=[{"text": f.text, "error": f.error} for f in parse_failures],
|
run_id,
|
||||||
|
failures=[{"text": f.text, "error": f.error} for f in parse_failures],
|
||||||
|
),
|
||||||
|
what=f"{run_id}-parse-failures.json",
|
||||||
|
in_flight=in_flight,
|
||||||
)
|
)
|
||||||
# Same ``finally``, different write rule: IFF a reviewer was given, including when the
|
# Same ``finally``, different write rule: IFF a reviewer was given, including when the
|
||||||
# list is empty (D4). A reviewer-less run must leave the outbox byte-identical, while a
|
# list is empty (D4). A reviewer-less run must leave the outbox byte-identical, while a
|
||||||
|
|
@ -1199,12 +1239,16 @@ async def run_project(
|
||||||
# state rather than one an operator infers from an absent file.
|
# state rather than one an operator infers from an absent file.
|
||||||
if outbox_dir is not None and proposal_reviewer is not None:
|
if outbox_dir is not None and proposal_reviewer is not None:
|
||||||
assert run_id is not None # narrowed by the step-0 guard (no wall-clock default)
|
assert run_id is not None # narrowed by the step-0 guard (no wall-clock default)
|
||||||
outbox.write_proposal_reviews(
|
_write_or_report(
|
||||||
outbox_dir,
|
lambda: outbox.write_proposal_reviews(
|
||||||
run_id,
|
outbox_dir,
|
||||||
payload=proposal_reviews_payload(
|
run_id,
|
||||||
expert_reviews, key_of=lambda p: verdict_key(_features_of(p))
|
payload=proposal_reviews_payload(
|
||||||
|
expert_reviews, key_of=lambda p: verdict_key(_features_of(p))
|
||||||
|
),
|
||||||
),
|
),
|
||||||
|
what=f"{run_id}-proposal-reviews.json",
|
||||||
|
in_flight=in_flight,
|
||||||
)
|
)
|
||||||
proposal = validator_outcome.proposal
|
proposal = validator_outcome.proposal
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -884,6 +884,89 @@ async def test_t9_a_budget_stop_inside_generation_still_leaves_the_record(tmp_pa
|
||||||
]
|
]
|
||||||
|
|
||||||
|
|
||||||
|
class _WriteRefused(OSError):
|
||||||
|
"""A disk that will not take the artefact. Its own subclass so the control arm can assert that
|
||||||
|
THIS error escaped, rather than that some ``OSError`` did."""
|
||||||
|
|
||||||
|
|
||||||
|
def _refuse_writer(monkeypatch: pytest.MonkeyPatch, name: str) -> None:
|
||||||
|
def _raise(*_a: Any, **_k: Any) -> None:
|
||||||
|
raise _WriteRefused(f"{name} refused")
|
||||||
|
|
||||||
|
monkeypatch.setattr(run.outbox, name, _raise)
|
||||||
|
|
||||||
|
|
||||||
|
def _budget_stop_select(blob: str, _role: str) -> str:
|
||||||
|
"""T9's shape: the bought attempt's reply never parses, the retry exhausts the round ledger,
|
||||||
|
and ``BudgetExceeded`` leaves ``run_project`` as an exception with BOTH ``finally`` writers
|
||||||
|
armed (parse failures exist, and a reviewer was given)."""
|
||||||
|
if _GENERATION_MARK not in blob:
|
||||||
|
return "ok"
|
||||||
|
return "not json at all" if _FEEDBACK_SENTINEL in blob else _run_reply("Belysning", 30_000)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_write_error_never_displaces_the_stop_that_was_already_in_flight(
|
||||||
|
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""The review writer runs from a ``finally`` on EVERY reviewed run, so an ``OSError`` from it
|
||||||
|
lands while ``BudgetExceeded`` (or ``ProposalReviewInputError``) is propagating — and a bare
|
||||||
|
call there REPLACES the in-flight exception. Neither ``except ProposalReviewInputError`` nor
|
||||||
|
the CLI's refusal tuple catches ``OSError``, so the operator got a traceback instead of the
|
||||||
|
``run stopped:`` line, and the reason the run ended was gone.
|
||||||
|
|
||||||
|
Detach point: dropping the ``in_flight`` guard around the writers. The stop must survive, and
|
||||||
|
the write failure must still be SAID — swallowed silently, a missing artefact would look like
|
||||||
|
a run that had nothing to record (T10/T11's whole distinction)."""
|
||||||
|
_refuse_writer(monkeypatch, "write_proposal_reviews")
|
||||||
|
|
||||||
|
with pytest.raises(BudgetExceeded):
|
||||||
|
await _run_with(
|
||||||
|
outbox_dir=tmp_path / "outbox",
|
||||||
|
reviewer=_RunReviewer(),
|
||||||
|
select=_budget_stop_select,
|
||||||
|
max_rounds=2,
|
||||||
|
)
|
||||||
|
|
||||||
|
err = capsys.readouterr().err
|
||||||
|
assert f"{_RUN_ID}-proposal-reviews.json" in err
|
||||||
|
assert "write_proposal_reviews refused" in err
|
||||||
|
|
||||||
|
|
||||||
|
async def test_the_parse_failure_writer_carries_the_same_guard(
|
||||||
|
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str]
|
||||||
|
) -> None:
|
||||||
|
"""The SIBLING writer in the same ``finally``. It has the identical shape and runs only when a
|
||||||
|
parse failed — rarer, never absent — so it gets the same guard and its own witness: one
|
||||||
|
helper, two call sites, and a change to only one of them is what a shared seam exists to
|
||||||
|
prevent (kø-(p))."""
|
||||||
|
_refuse_writer(monkeypatch, "write_parse_failures")
|
||||||
|
|
||||||
|
with pytest.raises(BudgetExceeded):
|
||||||
|
await _run_with(
|
||||||
|
outbox_dir=tmp_path / "outbox",
|
||||||
|
reviewer=_RunReviewer(),
|
||||||
|
select=_budget_stop_select,
|
||||||
|
max_rounds=2,
|
||||||
|
)
|
||||||
|
|
||||||
|
assert f"{_RUN_ID}-parse-failures.json" in capsys.readouterr().err
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_write_error_with_nothing_in_flight_is_still_an_error(
|
||||||
|
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||||
|
) -> None:
|
||||||
|
"""The CONTROL, and the reason the guard is conditional rather than a blanket ``except``. On a
|
||||||
|
run that otherwise SUCCEEDED there is no stop reason to protect, so a disk that refused the
|
||||||
|
artefact must reach the caller: 'the outbox is unwritable' silently downgraded to a clean
|
||||||
|
return is how a run reports success it cannot evidence.
|
||||||
|
|
||||||
|
Without this arm the guard could be mutated to swallow unconditionally and stay green."""
|
||||||
|
_refuse_writer(monkeypatch, "write_proposal_reviews")
|
||||||
|
|
||||||
|
with pytest.raises(_WriteRefused):
|
||||||
|
await _run_with(outbox_dir=tmp_path / "outbox", reviewer=_RunReviewer())
|
||||||
|
|
||||||
|
|
||||||
async def test_t10_a_reviewer_nobody_could_consult_still_writes_an_empty_record(
|
async def test_t10_a_reviewer_nobody_could_consult_still_writes_an_empty_record(
|
||||||
tmp_path: Path,
|
tmp_path: Path,
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue