fix(runner): record a failed run when the provider name is unknown - #10
Merged
Conversation
`execute()` resolved the provider adapter with
`get_provider(spec.provider)` OUTSIDE the try/except that turns vendor
errors into recorded failed runs. A model whose `provider` names no
registered adapter is valid, routable config (`ModelSpec.provider` is an
unconstrained str) that clears policy and routing — then `get_provider`
raised `ProviderError` uncaught, crashing the whole call.
That violates the runner's contract ("Every path lands in the store AND
the ledger ... failed runs with the vendor error"): no store row, no
`run_failed` event, just a traceback. Move the resolution inside the try
so an unknown provider is recorded as a failed run like any other vendor
error. The `complete()` failure path is unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
bughunt gate —
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
A model whose
providerfield names no registered adapter (e.g.provider: cohere) is valid, routable config —ModelSpec.provideris an unconstrainedstr, so it passes loading, the data-policy gate, and routing. When such a model is selected,execute()crashed with an uncaughtProviderErrorinstead of recording a failed run.Root cause
runner.execute()resolved the adapter withactive_provider = provider or get_provider(spec.provider)outside thetry/except ProviderErrorblock that converts vendor errors into recorded failed runs.get_providerraisesProviderErrorfor an unknown name, so the exception propagated out ofexecute()— no store row, norun_failedledger event, just a traceback. That violates the runner's own contract:The existing
test_provider_failure_is_a_failed_run_not_a_crashonly covers a resolved provider whosecomplete()raises — it passes aFailingProviderinstance, soget_provideris never exercised. The unknown-provider path was unpinned.Fix
Move the provider resolution inside the existing
tryso an unknown provider is caught and recorded via the same_terminal("failed", ...)path as any other vendor error. One-line move; thecomplete()failure path is unchanged.Test
test_unknown_provider_is_a_failed_run_not_a_crash: a routable model withprovider: cohere, run offline → assertsstatus == "failed", the reason namescohere, and both the store row and the last ledger event (run_failed) are recorded. It raisesProviderErroronmainand passes with the fix (verified viagit stash).Full suite: 162 passed, 92.18% coverage (gate 85%);
ruffclean; offlineflightdeck demo+audit verifyintact.Found and fixed by an autonomous
bughuntiteration.