hoist the per-record LLM route pin (provider/model/effort) into one shared lib - #4817
Merged
Conversation
… each subsystem re-deriving it (#4793) Eight subsystems each hand-rolled `{ providerId, model, effort }` on a record — a zod fragment, a sanitizer, and a resolver deciding how a per-call override interacts with the saved pin. Every copy re-derived the same rule, and the `effort` validation had already drifted (some sites took a capped free string, so an unknown level passed the door check and was silently clamped by the runner instead of being rejected). `server/lib/llmRoutePin.js` now owns the shape once: `LLM_ROUTE_PIN_LIMITS`, `llmRoutePinSchema` (with `effort` as the shared `EFFORT_LEVELS` enum), `sanitizeLlmRoutePin`, and `resolveLlmRoutePin` — which carries the never-cross-providers rule: a per-call pick beats the pin, and the pin's model and effort are inherited only while the effective provider is still the one they were picked for. - `resolveSeriesLlmOverride` becomes the series-shaped adapter over it, keeping its exported name, its `providerMatchesSeries` field, and its `undefined` (not `null`) unset values, so no Pipeline caller changes. - FableLoom's `sanitizePlaySettings` / `playRouting` / play-pin schema drop their copies. `LOOM_LIMITS` sheds the three pin caps it no longer owns. No behavior change for any existing caller; the existing series and FableLoom suites hold that.
This was referenced Aug 22, 2026
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.
Summary
Hoists the per-record LLM route pin —
{ providerId, model, effort }saved on a record — intoserver/lib/llmRoutePin.js, so the eight subsystems that each hand-rolled it stop re-deriving the same rule.The new module owns:
LLM_ROUTE_PIN_LIMITS— the field caps, one source of truth for both the door check and the sanitizer.llmRoutePinSchema— the zod fragment, witheffortas the sharedEFFORT_LEVELSenum rather than a capped free string (that drift is the reason an unknown level used to pass validation and get silently clamped by the runner).sanitizeLlmRoutePin(raw)— trim/cap each dimension,nullfor a non-object or an all-empty pin.resolveLlmRoutePin(pin, perCall)— the never-cross-providers rule in one place: a per-call pick beats the pin, and the pin's model and effort are inherited only while the effective provider is still the one they were picked for.Migrated onto it:
resolveSeriesLlmOverrideis now the series-shaped adapter over the shared resolver. Its exported name, itsproviderMatchesSeriesreturn field, and itsundefined-not-nullunset values are unchanged, so no Pipeline caller moves.sanitizePlaySettingsandplayRoutingare deleted in favor of the shared sanitizer/resolver, and the route schema'splaySettings/ per-call pick fields come fromllmRoutePinSchema.LOOM_LIMITSsheds the three pin caps it no longer owns.Not a behavior change for any existing caller — the existing series and FableLoom suites hold that, and the new module gets its own unit suite.
Decisions
loom.playSettingstoloom.llm = { play: {…} }. The issue hedges it with "consider", and installs already have savedplaySettings, so it needs a migration plus a client-side change for no behavior gain. Left as-is; a second FableLoom lane pin can revisit it with the migration it would need.Remaining
Left for a follow-up, with the reason each is not mechanical:
resolveStagePin(server/services/creativeDirector/projectsLogic.js) — it merges two pin layers (per-project override vs. global stage assignment) with whole-object precedence: when the override names aproviderId, the override's model is used even if empty. The shared resolver would instead inherit the global model when the override names the same provider. That is arguably the better rule, but it is a behavior change, and the return values are''rather thannull.server/lib/cosValidation.js(resolveReviewerPins) — reviewer effort is validated against each reviewer's own ladder (effortLevelsForProvider), not the unionEFFORT_LEVELS, and deliberately rejects rather than clamps. The shared enum would widen it.server/services/cosTaskGenerator.js/server/services/appTaskProviderPin.js— the pin walk there carries the api-harness fallback and a lazily-read Schedule pin thunk (see d89e88e, 0aa27a7); the layering is provider-capability-aware, not a plain pin/per-call merge.renderTargets.js,cloudProviderConfig.js) — image/video targets, not an LLM route pin.Refs #4793
Test plan
cd server && npm test— full suite green (1569 files, 32855 tests).server/lib/llmRoutePin.test.jscovers the sanitizer (trim, null-for-empty, caps), the schema (per-field nulls, off-laddereffortrejected, over-length ids rejected), and the resolver (full-pin fallback, same-provider inherit, cross-provider drop, explicit per-call pick, missing pin).server/lib/seriesLlmOverride.test.jsand the FableLoom records/weave/route suites pass unchanged — they are the no-behavior-change guard.