Repository navigation
fix(middleman): stop 500ing on models whose lab has no dispatch class [SEN-237] - #1404
Open
metr-background-agents[bot] wants to merge 1 commit into
Open
metr-background-agents[bot] wants to merge 1 commit into
metr-background-agents[bot] wants to merge 1 commit into
Conversation
metr-background-agents
Bot
temporarily deployed
to
prd-pulumi-preview
August 20, 2026 21:24
Inactive
🥥
|
tbroadley
reviewed
Aug 24, 2026
tbroadley
left a comment
Contributor
There was a problem hiding this comment.
Let's add an alert for this metric?
Can we have some check that fails when someone tries to create a model with an unknown lab?
Comment on lines
+176
to
+179
| # A model config can name a lab we have no dispatch class for. Don't let one | ||
| # such row KeyError the whole listing — /permitted_models_info maps this over | ||
| # every permitted model, so it would 500 for everyone in that model's group. | ||
| # Report the capability fields as unknown rather than guessing. |
Contributor
There was a problem hiding this comment.
I think we should 400 in this case instead of 500ing, but I don't think we should silently report incomplete data.
This branch was previously deployed
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.
Overview
A model config whose
labhas no entry in middleman's provider-dispatch table (api_to_class) madePOST /completionsreturn 500 SafeInternalError — a request that can never succeed, reported to Sentry on every attempt. In prd this fired forgpt-4-assistants(labopenai-assistants, a lab middleman has never implemented).While confirming the root cause I found the same bad row also
KeyErrors the entire/permitted_models_infolisting for every user in that model's group, which is the more serious of the two. Both are fixed here.Linear: https://linear.app/metrevals/issue/SEN-237/safeinternalerror-unknown-lab-openai-assistants-for-model-gpt-4
Sentry: https://metr-sh.sentry.io/issues/HAWK-483
Approach
Nothing validates
labbefore request time:ModelInfois a plain dataclass,LabNameis a static-onlyLiteral, and the admin read schema deliberately tolerates unknown labs (admin/schemas.py:112), so DB JSONB rows can carry anything. It's reachable through the validated admin API too —LabNameis only asserted to be a superset ofapi_to_class(test_admin_models.py:522), anddummy-chatis inLabNamewhile itsapi_to_classentry is commented out.Three small changes, no behaviour change for well-configured models:
models.py_load_all_models— logmodel_config.unknown_lab(structured,errorlevel) at load time, so the misconfiguration stays visible to operators once the request-time 500 stops paging Sentry. Mirrors themodel_config.unknown_fields_droppedtolerate-and-log precedent immediately above it.apis.py:581— raiseBadReq(400) instead ofSafeInternalError(500).validate_completions_reqalready answers "model you asked for isn't usable" withBadReqon this same path, and passthrough already treats this exact condition as a client-visible404 model not found(passthrough.py:578/714/787) — the unified path was the sole outlier. A 5xx tells callers to retry something unsatisfiable, and Sentry's Starlette integration reports 5xxHTTPExceptions (hencehandled: yes/mechanism: starletteon the event).models.pyto_public— guardedapi_to_class.get(...); reportfeatures/is_chatasNone(both alreadyNone-able onPublicModelInfo, and theare_details_secretbranch already returnsNonefor both, so consumers handle it) instead ofKeyError-ing the whole listing.Alternative ruled out: dropping undispatchable models from the registry.
deadmodels are deliberately retained "for permission checks on old data", andget_labs_for_public_names()feeds the cross-lab scan safeguard on private transcripts (models.py:383→server.py:566). Removing rows would silently strip group/lab metadata for historical data and make that safeguard fail open — the exact failure mode PLT-671 is open to close. Keeping the row registered but unusable preserves both invariants.Side benefit: the old message interpolated
model.labinto a client-visible response body (SafeInternalErrorisn't redacted), disclosing the lab of a secret model — precisely whatare_details_secretexists to hide. The lab now only appears in server-side logs.Not addressed here: the underlying prd data.
openai-assistantsisn't a real middleman lab, so thatgpt-4-assistantsrow is unusable regardless and should be removed or repointed atopenai-chat/openai-responses. Flagged on the Linear ticket; touching prod data is out of scope for this change.Testing & validation
Three tests, each confirmed failing on
mainbefore the fix:test_undispatchable_lab_is_a_bad_request_not_an_internal_error— reproduces the reported event exactly (gpt-4-assistants/openai-assistants); wasSafeInternalError: 500, nowBadReq.test_to_public_survives_unknown_lab— wasKeyError: 'openai-assistants', now degrades tofeatures=None/is_chat=None.test_unknown_lab_is_logged_at_load_time— asserts the structured log fires once, and that the row stays registered (guards the invariant above).Code quality
pre-commit run --all-filespasses (ruff, basedpyright/mypy, eslint/prettier/tsc, shellcheck — what CI's Lint job runs)Before merging