ENG-736 - Delete model_prefixes: route models off the harness's own fetched-or-shipped list#65
Closed
druks-operator[bot] wants to merge 1 commit into
Closed
ENG-736 - Delete model_prefixes: route models off the harness's own fetched-or-shipped list#65druks-operator[bot] wants to merge 1 commit into
druks-operator[bot] wants to merge 1 commit into
Conversation
Owner
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.
Linear ticket: ENG-736
Plan
ENG-736 — Delete
model_prefixes; route models offHarnessSettings.allowed_modelsProblem
Model→harness routing still runs off
Harness.model_prefixes— a hand-maintained tuple that has to be edited every time a provider invents a naming scheme (e.g.o1/o3/o4). Since ENG-701 landed,HarnessSettings.models_fetched/allowed_modelsalready holds the provider's own selectable list (fetched on connect and refreshed by theRefreshModelscron), andHarnessSettings.allowed_modelsfalls back to each harness's shippedmodelstuple when the fetch hasn't landed. That row is already the single source of truth — the prefix tuple is a redundant, drifting second one.Approach
Route directly off
HarnessSettings.allowed_models, deleteHarness.model_prefixesandHarness.has_modeloutright, and collapse bothuser_settings/routes.pyvalidators ontoget_harness_for_modelso they stop callinghas_modelthemselves. Bare harness names as model strings ("claude","codex") keep working via themodel == row.namearm. The pricing-onlystartswithinsidecodex.py::_rate_for(line ~232) is deliberately left alone — it maps a model to a_Rate, not to a harness. The consequence — a brand-new provider model is unroutable (HarnessError) until the nextRefreshModels(0 6 * * *) or a reconnect — is accepted per the ticket; the manual-refresh endpoint / picker button follow-up is explicitly out of scope.Files to change
backend/druks/harnesses/base.pymodel_prefixes: ClassVar[tuple[str, ...]]declaration (currently line 76) and the classmethodhas_model(cls, model)(lines 90–94).model_prefixesor "namespace" — the identity/config seeds now enumerated areprovider,models,default_model,model_discovery_url, plus the effort/timeout defaults.backend/druks/harnesses/claude.pymodel_prefixes = ("claude-",)on line 39. No other change to this file.backend/druks/harnesses/codex.pymodel_prefixes = ("gpt-", "o1", "o3", "o4")on line 274. No other change to this file.startswithfallback inside_rate_for(~lines 230–233); it is pricing, not routing.backend/druks/harnesses/registry.pyget_harness_for_modelverbatim to the shape the ticket pins:get_harnessesandget_harnessunchanged (both are still used and don't need routing state).HarnessSettingsinside the function (or at module top level — either is fine here sinceuser_settings.modelsalready importsdruks.harnesses.registry.get_harnessat method-call time via a property, so a module-level import creates no cycle at import time; verify by running the test suite).backend/druks/user_settings/routes.pyupdate_harness_settings(line 65): replace theharness.has_model(updates["model"])check with aget_harness_for_modelcall. Reject with the existing 422 shape when the model does not resolve to this specific harness:detailstring (f"{updates['model']!r} is not a {harness.name} model.") so callers relying on the 422 shape don't churn._validate_model(line 119): replace theany(harness.has_model(value) for harness in get_harnesses())check with aget_harness_for_modelcall, keeping the existing 422 detail (f"No installed harness runs model {value!r}."):HarnessErrorimport fromdruks.harnesses.exceptions; drop the now-unusedget_harnessesimport if nothing else in the module needs it (currently only these two validators call it).backend/tests/test_harness_model_routing.pyRework end-to-end against the seeded
HarnessSettingsrows the session-scoped_test_schemafixture already provides (seebackend/tests/conftest.pyandbackend/druks/bootstrap.py::seed_harnesses). The rework must cover, at minimum:clauderow'smodels_fetchedto a list like[{"id": "claude-fable-5", "label": "Claude Fable 5"}], then assertget_harness_for_model("claude-fable-5") is ClaudeHarness. Also assert that a namespace-shaped string absent from that fetched list (e.g."claude-opus-5") no longer routes — this locks in the accepted consequence (namespace pass-through dies).codexrow'smodels_fetchedunset (or explicitlyNone) and assertget_harness_for_model(CodexHarness.models[0])returnsCodexHarness— that is,allowed_modelsfalls back to the shipped tuple when nothing has been fetched. Do the same for aclauderow that has never been fetched (freshly reset for this test).get_harness_for_model("claude") is ClaudeHarnessandget_harness_for_model("codex") is CodexHarnessregardless ofmodels_fetched.get_harness_for_model("llama-3-70b")raisesHarnessError, and specifically that an old-style namespace guess ("gpt-6","o4-mini","claude-opus-5") not present in either the fetched list or the shipped tuple also raises — the whole point of the change.The old
test_new_model_in_known_namespace_routes_without_a_releasecase is removed as part of the rework (its premise — pass-through by namespace — is exactly what this ticket kills). Use the transaction-rollback fixture already inconftest.py; the mutations don't need to be committed.Verification
Run the full verification profile against the changed backend:
uv run ruff check backenduv run ruff format --check backenduv run pyrightuv run pytest backend/The frontend commands in the profile aren't touched by this change but stay in the profile for completeness. GitHub CI runs the same commands.
Acceptance Criteria
model_prefixesclass variable is removed fromHarnessand from every subclass (nomodel_prefixesremains in the backend).grep -rn "model_prefixes" backend/returns no matches.Harness.has_modeland all references to it are removed from the backend.grep -rn "has_model" backend/returns no matches.backend/druks/harnesses/registry.py::get_harness_for_modelresolves viaHarnessSettings.all(), matchingmodel == row.nameor an entry inrow.allowed_modelsbym["id"], and raisesHarnessErroron miss.backend/druks/harnesses/registry.pyand confirm the body iteratesHarnessSettings.all()with the specified match arms and theHarnessError(f"no harness runs model {model!r}")raise; nostartswithand nohas_modelcall remains in this file.backend/druks/user_settings/routes.py,update_harness_settingsrejects a model that does not resolve (viaget_harness_for_model) to the harness whose row is being patched, returning HTTP 422 with detailf"{model!r} is not a {harness.name} model."— the existing shape.backend/druks/user_settings/routes.py::update_harness_settings; confirm it callsget_harness_for_model(nothas_model), rejects when the resolved harness is notharness, and preserves the 422 detail string. Backend tests around the settings PATCH endpoint (backend/tests/test_api_settings.py, or a new/updated case intest_harness_model_routing.pyif none is present) pass.backend/druks/user_settings/routes.py,_validate_modelno longer callshas_model; it wrapsget_harness_for_modelin aHarnessErrorguard and returns HTTP 422 with detailf"No installed harness runs model {value!r}."when the model is unroutable.backend/druks/user_settings/routes.py::_validate_modeland confirm it usesget_harness_for_modelinside atry/except HarnessErrorand raises the specified 422;grep -n "has_model" backend/druks/user_settings/routes.pyreturns no matches.backend/druks/harnesses/codex.py::_rate_for(around line 232) still contains astartswithfallback over_DEFAULT_RATES— pricing routing is not touched by this PR.grep -n "startswith" backend/druks/harnesses/codex.pyshows the_rate_forstartswith(known)line still present; the surrounding function body is unchanged.backend/tests/test_harness_model_routing.pyis rewritten to driveget_harness_for_modelthrough seededHarnessSettingsrows, and covers: (a) a model listed via a mutatedmodels_fetchedon the claude row routes toClaudeHarness; (b) a fresh row whosemodels_fetchedis unset falls back to the shipped tuple (aCodexHarness.models/ClaudeHarness.modelsentry still routes); (c) bare harness names"claude"and"codex"still resolve; (d) an unlisted model (e.g."llama-3-70b") and an old-style namespace guess absent from both fetched and shipped lists (e.g."claude-opus-5","gpt-6","o4-mini") raiseHarnessError.backend/tests/test_harness_model_routing.pyand confirm each of the four cases is present;uv run pytest backend/tests/test_harness_model_routing.pypasses.uv run ruff check backendexits 0 anduv run ruff format --check backendexits 0.uv run pyrightexits 0.uv run pytest backend/exits 0.