name the whole-layer LLM route-pin rule in the shared lib and put the CD stage pins on it

server/lib/README.md

@@ -86,7 +86,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub
 | `storyboardScenes.js` | Durable ids + id-first addressing for `stages.storyboards.scenes[]` (#3413). `ensureStoryboardIds(scenes)` stamps a deterministic `scene-NN` / `shot-NN` id on any entry lacking one — and re-stamps later copies of a DUPLICATE id (collision-escaped with a `-2` suffix, so peers and concurrent readers derive identical ids); `resolveStoryboardTarget(list, { id, index })` → `{ index, record, matchedBy, stale }` resolves a captured id against a fresh array, falling back to the index only when no id was captured, and reporting `stale: true` (409, never an index retarget) when a captured id is gone. |
 | `seasonStructure.js` | Season/episode structure recommendation. |
 | `seriesCharacterArc.js` | Per-character story-arc shapes (`series.characterArcs[]`): want/need, start → end state, transition beats. Sanitizers + `renderCharacterArcsForPrompt` for the `arc.transitions` editorial check. |
-| `llmRoutePin.js` | The per-record LLM route pin `{ providerId, model, effort }` — `LLM_ROUTE_PIN_LIMITS` + `llmRoutePinSchema` (door check, `effort` as the shared `EFFORT_LEVELS` enum), `sanitizeLlmRoutePin(raw)` (trim/cap, `null` for an all-empty pin), and `resolveLlmRoutePin(pin, perCall)` → `{ providerId, model, effort, providerMatchesPin }`. Owns the never-cross-providers rule once: a per-call pick beats the pin, and the pin's model/effort are inherited only while the effective provider is still the one they were picked for. Consumed by `seriesLlmOverride.js` and FableLoom's play pin. |
+| `llmRoutePin.js` | The per-record LLM route pin `{ providerId, model, effort }` — `LLM_ROUTE_PIN_LIMITS` + `llmRoutePinSchema` (door check, `effort` as the shared `EFFORT_LEVELS` enum), `sanitizeLlmRoutePin(raw)` (trim/cap, `null` for an all-empty pin), and `resolveLlmRoutePin(pin, perCall)` → `{ providerId, model, effort, providerMatchesPin }`. Owns the never-cross-providers rule once, in its two shapes: `resolveLlmRoutePin` merges a record pin with an independent per-call pick per FIELD (guarded by a provider-id comparison), while `pickLlmRoutePinLayer(...layers)` + `llmRoutePinNamesProvider(layer)` take the most specific layer WHOLE for pins whose provider and model were chosen together in one control (falling through to the base layer). Consumed by `seriesLlmOverride.js`, FableLoom's play pin, and the Creative Director stage pins. |
 | `seriesLlmOverride.js` | Pure `resolveSeriesLlmOverride(series, { overrideProvider, overrideModel })` → `{ provider, model, providerMatchesSeries }` — shared fallback so Pipeline LLM actions honor the series' configured provider/model, only inheriting the series model when the effective provider still matches. |
 | `bibleExtractor.js` | LLM bible-extraction stage + sanitization. |
 | `catalogBulkParsers.js` | Dependency-free markdown/CSV/JSON parsers for `POST /api/catalog/bulk-import` and YAML/markdown serializers for `GET /api/catalog/export`. |

server/lib/cosValidation.js

@@ -664,6 +664,16 @@ export function reviewerConfigMetadata(config) {
  * `agy --model <suffixed-id> --effort <tier>` invocation agy rejects. This is the
  * one call every site makes instead.
  *
+ * Deliberately NOT expressed on `lib/llmRoutePin.js` (#4793). That module owns a
+ * single `{ providerId, model, effort }` triple; a reviewer pin is two maps keyed
+ * by reviewer slug, with no provider dimension at all — the slug IS the routing
+ * key — and its precedence is whole-MAP (an explicitly empty task map overrides
+ * the defaults), not per-field. Its effort is also validated against each
+ * reviewer's OWN ladder via `normalizeReviewerEffort`, which is strictly narrower
+ * than the union `EFFORT_LEVELS` enum the shared schema uses: `agy` really does
+ * reject `--effort max`. There is no shape the two can meet in without losing
+ * that narrowing, so this stays hand-rolled.
+ *
  * @param {Object} [pins] - task metadata (or explicit options) carrying
  *   `reviewerModels` / `reviewerEfforts` maps; an absent map falls back to the
  *   Code Review Defaults, an explicitly empty one overrides them.

server/lib/llmRoutePin.js

@@ -17,6 +17,15 @@
  *
  * `null` (never `''`) marks an unset dimension, so "fall through to the next
  * layer" stays distinguishable from a deliberate empty choice.
+ *
+ * That rule has TWO shapes, and picking the wrong one silently crosses
+ * providers:
+ *
+ * - `resolveLlmRoutePin(pin, perCall)` — a saved record pin plus an independent
+ *   one-off pick. The two merge per FIELD, guarded by a provider-id comparison.
+ * - `pickLlmRoutePinLayer(...layers)` — configuration layers whose provider and
+ *   model were chosen together in one control. The winning layer is taken
+ *   WHOLE, which enforces the same rule without a comparison.
  */
 
 import { z } from 'zod';
@@ -84,3 +93,38 @@ export function resolveLlmRoutePin(pin, perCall = {}) {
     providerMatchesPin,
   };
 }
+
+/** Does this layer pin a route at all? A layer that names no `providerId` pins
+ *  nothing the runtime can key on — the model alone is unresolvable, since the
+ *  runtime looks the provider up first. */
+export function llmRoutePinNamesProvider(layer) {
+  return Boolean(layer?.providerId);
+}
+
+/**
+ * The OTHER precedence rule, for pins arranged in configuration LAYERS rather
+ * than pin-plus-per-call: the most specific layer that names a provider wins as
+ * a WHOLE, and the least specific layer is the base everything falls through to.
+ *
+ * Use this — not `resolveLlmRoutePin` — when each layer's provider and model
+ * were picked TOGETHER in one control (a drawer's provider + model selects, a
+ * settings assignment row). There, a layer that names a provider but no model
+ * means "that provider's default model", so merging the next layer's model in
+ * per-field would hand one provider a model chosen for another: the same
+ * never-cross-providers rule as `resolveLlmRoutePin`, enforced by taking the
+ * winning layer whole instead of by comparing provider ids.
+ *
+ * `resolveLlmRoutePin` stays right where the layers are a saved record pin and a
+ * one-off per-call pick, which are independent choices and DO merge per field.
+ *
+ * The final layer is returned even when it names no provider, so a base layer
+ * that carries only a model still reaches the caller. Returns `null` when no
+ * layer was supplied at all.
+ *
+ * @param {...({ providerId?: string|null, model?: string|null, effort?: string|null }|null|undefined)} layers
+ *   most specific first, base last
+ * @returns {object|null}
+ */
+export function pickLlmRoutePinLayer(...layers) {
+  return layers.find(llmRoutePinNamesProvider) ?? layers[layers.length - 1] ?? null;
+}

server/lib/llmRoutePin.test.js

@@ -4,6 +4,8 @@ import {
   llmRoutePinSchema,
   resolveLlmRoutePin,
   sanitizeLlmRoutePin,
+  llmRoutePinNamesProvider,
+  pickLlmRoutePinLayer,
 } from './llmRoutePin.js';
 
 describe('sanitizeLlmRoutePin', () => {
@@ -106,3 +108,38 @@ describe('resolveLlmRoutePin', () => {
     });
   });
 });
+
+describe('llmRoutePinNamesProvider', () => {
+  it('is true only for a layer carrying a non-empty providerId', () => {
+    expect(llmRoutePinNamesProvider({ providerId: 'claude' })).toBe(true);
+    expect(llmRoutePinNamesProvider({ providerId: '' })).toBe(false);
+    expect(llmRoutePinNamesProvider({ model: 'opus' })).toBe(false);
+    expect(llmRoutePinNamesProvider(null)).toBe(false);
+    expect(llmRoutePinNamesProvider(undefined)).toBe(false);
+  });
+});
+
+describe('pickLlmRoutePinLayer', () => {
+  it('takes the most specific layer that names a provider, whole', () => {
+    const specific = { providerId: 'claude' };
+    const base = { providerId: 'codex', model: 'gpt-x' };
+    // The base model is NOT merged in — it belongs to `codex`, not `claude`.
+    expect(pickLlmRoutePinLayer(specific, base)).toBe(specific);
+  });
+
+  it('skips layers that name no provider', () => {
+    const base = { providerId: 'codex', model: 'gpt-x' };
+    expect(pickLlmRoutePinLayer(null, { model: 'orphan' }, base)).toBe(base);
+  });
+
+  it('returns the base layer even when it names no provider', () => {
+    const base = { model: 'gpt-x' };
+    expect(pickLlmRoutePinLayer(null, base)).toBe(base);
+  });
+
+  it('returns null when no layer was supplied', () => {
+    expect(pickLlmRoutePinLayer()).toBeNull();
+    expect(pickLlmRoutePinLayer(null)).toBeNull();
+    expect(pickLlmRoutePinLayer(undefined, null)).toBeNull();
+  });
+});

server/services/appTaskProviderPin.js

@@ -21,6 +21,16 @@
  *
  *   per-app pin → (api-typed? adopt the Schedule pin instead) → Schedule pin →
  *   the install's default coding agent
+ *
+ * Deliberately NOT expressed on `lib/llmRoutePin.js` (#4793). Both of that
+ * module's rules are synchronous and decide on the pin VALUES alone; this walk is
+ * async and decides on a provider's resolved TYPE, healing an api-typed pin onto
+ * the next layer — a branch neither rule has a place for. It also carries two
+ * semantics the shared rules deliberately do not: the Schedule pin is a lazy
+ * thunk read at most once (so a resolving per-app pin never pays for it), and the
+ * model falls through with `??` rather than `||`, so an explicitly-blank per-app
+ * model stays blank instead of inheriting the Schedule pin's. Migrating would
+ * change both.
  */
 
 const NOT_AGENT_CAPABLE = 'provider-not-agent-capable';

server/services/creativeDirector/agentBridge.js

@@ -20,6 +20,7 @@ import { buildTreatmentPrompt, buildEvaluatePrompt, buildPlanPrompt } from '../.
 import { getToolSpecs } from '../creative/toolRegistry.js';
 import { getSettings } from '../settings.js';
 import { resolveStagePin } from './projectsLogic.js';
+import { llmRoutePinNamesProvider, pickLlmRoutePinLayer } from '../../lib/llmRoutePin.js';
 import { recordRun } from './local.js';
 import { DELIVERABLE_KINDS, deliverableMark } from './deliverableGate.js';
 
@@ -53,18 +54,24 @@ async function getStageAssignment(kind, project) {
   if (kind === 'evaluate') return {};
   // Only consult the commission when the project does NOT carry its own pin for
   // this stage — a drawer choice is the user's explicit override and must win.
-  const projectPin = project?.modelOverrides?.[kind]?.providerId ? project.modelOverrides[kind] : null;
+  // Shares `llmRoutePinNamesProvider` with `resolveStagePin`'s own layer test, so
+  // "what counts as a pin" can't drift between the skip check and the resolve.
+  const projectPinsStage = llmRoutePinNamesProvider(project?.modelOverrides?.[kind]);
   const [settings, commissionPin] = await Promise.all([
     getSettings().catch(() => ({})),
     // Lazy + only for a commission-owned project, so the CD graph doesn't take a
     // static dependency on the commission store for the common bare project.
-    (!projectPin && project?.commissionId)
+    (!projectPinsStage && project?.commissionId)
       ? import('../creativeCommissions/projectControl.js')
         .then(({ commissionStagePin }) => commissionStagePin(project.commissionId))
         .catch(() => null)
       : null,
   ]);
-  const assignment = commissionPin || resolveStagePin(kind, project, settings);
+  // The commission is the outermost layer of the same whole-layer ladder
+  // `resolveStagePin` resolves (commission → project override → global
+  // assignment); `commissionStagePin` returns null unless it names a usable
+  // provider, so it only ever wins by naming one.
+  const assignment = pickLlmRoutePinLayer(commissionPin, resolveStagePin(kind, project, settings));
   if (!assignment.providerId && !assignment.model) return {};
   return {
     ...(assignment.providerId ? { provider: assignment.providerId, providerId: assignment.providerId } : {}),

server/services/creativeDirector/projectsLogic.js

@@ -15,6 +15,7 @@ import { ServerError } from '../../lib/errorHandler.js';
 import { creativeDirectorTreatmentSchema, creativeDirectorPlanSchema } from '../../lib/validation.js';
 import { PROJECT_STATUSES, PLAN_STEP_TERMINAL_SUCCESS } from '../../lib/creativeDirectorPresets.js';
 import { compareNewerWins } from '../../lib/lwwTimestamp.js';
+import { pickLlmRoutePinLayer } from '../../lib/llmRoutePin.js';
 import { sanitizeSoftDeleteFields } from '../../lib/syncWire.js';
 import { localImageFilename } from '../../lib/localImageFilename.js';
 
@@ -72,14 +73,25 @@ export function normalizeRenderBackend(raw) {
  * per-project override wins when it names a providerId; otherwise the global
  * `settings.creativeDirector.<stage>` assignment applies (which itself may be
  * empty → the caller falls back to the system default / auto-resolution).
- * Returns `{ providerId, model }` with string values ('' when unset). Shared by
- * agentBridge (treatment/plan CoS-task pins) and sceneEvaluator (evaluation
- * vision-call pin) so the two resolution paths can never drift.
+ * Shared by agentBridge (treatment/plan CoS-task pins) and sceneEvaluator
+ * (evaluation vision-call pin) so the two resolution paths can never drift.
+ *
+ * The layer precedence is `pickLlmRoutePinLayer`'s — the winning layer is taken
+ * WHOLE, so a project that names a provider but no model gets that provider's
+ * default model rather than inheriting the global assignment's (which was picked
+ * for whatever provider the assignment names). See `lib/llmRoutePin.js` for why
+ * that differs from the per-field `resolveLlmRoutePin`.
+ *
+ * Returns `{ providerId, model }` with STRING values ('' when unset), not the
+ * shared lib's `null`s: `getStageAssignment` and `resolveVisionEvalTarget` both
+ * branch on plain falsiness and spread the result into task metadata, so keeping
+ * the two dimensions as strings is this resolver's own contract.
  */
 export function resolveStagePin(stage, project, settings) {
-  const override = project?.modelOverrides?.[stage];
-  const global = settings?.creativeDirector?.[stage];
-  const chosen = override?.providerId ? override : global;
+  const chosen = pickLlmRoutePinLayer(
+    project?.modelOverrides?.[stage],
+    settings?.creativeDirector?.[stage],
+  );
   return {
     providerId: isStr(chosen?.providerId) ? chosen.providerId : '',
     model: isStr(chosen?.model) ? chosen.model : '',

server/services/creativeDirector/projectsLogic.test.js

@@ -80,6 +80,39 @@ describe('modelOverrides (per-project provider/model pins)', () => {
     // Neither → empty strings.
     expect(resolveStagePin('plan', {}, settings)).toEqual({ providerId: '', model: '' });
   });
+
+  // The layer precedence is WHOLE-OBJECT, not per-field: the layer that names a
+  // provider supplies the model too, even when it has none. Pinned because it is
+  // exactly what separates this resolver from the shared per-field
+  // `resolveLlmRoutePin` — merging the global model in here would hand the
+  // project's provider a model the user picked for a different one.
+  it('resolveStagePin does not inherit the global model into a provider-only override', () => {
+    const settings = { creativeDirector: { treatment: { providerId: 'global', model: 'gm' } } };
+    // Different provider — inheriting the global model would cross providers.
+    expect(resolveStagePin('treatment', { modelOverrides: { treatment: { providerId: 'proj' } } }, settings))
+      .toEqual({ providerId: 'proj', model: '' });
+    // SAME provider — still not inherited: the drawer's empty model means "this
+    // provider's default model", not "fall through to the global assignment".
+    expect(resolveStagePin('treatment', { modelOverrides: { treatment: { providerId: 'global' } } }, settings))
+      .toEqual({ providerId: 'global', model: '' });
+  });
+
+  it('resolveStagePin ignores a model-only override and falls through to the global', () => {
+    const settings = { creativeDirector: { plan: { providerId: 'global', model: 'gm' } } };
+    expect(resolveStagePin('plan', { modelOverrides: { plan: { model: 'orphan' } } }, settings))
+      .toEqual({ providerId: 'global', model: 'gm' });
+  });
+
+  it('resolveStagePin keeps a model-only global assignment (the base layer is never skipped)', () => {
+    const settings = { creativeDirector: { plan: { model: 'gm' } } };
+    expect(resolveStagePin('plan', {}, settings)).toEqual({ providerId: '', model: 'gm' });
+  });
+
+  it('resolveStagePin coerces absent layers and non-string fields to empty strings', () => {
+    expect(resolveStagePin('treatment', null, null)).toEqual({ providerId: '', model: '' });
+    expect(resolveStagePin('treatment', { modelOverrides: { treatment: { providerId: 'proj', model: 7 } } }, {}))
+      .toEqual({ providerId: 'proj', model: '' });
+  });
 });
 
 describe('buildProjectRecord', () => {