Skip to content

Unify LLM call-site configuration under llm.{default,profiles,callSites}#26159

Merged
siddseethepalli merged 41 commits into
mainfrom
siddseethepalli/unify-llm-callsites
Apr 18, 2026
Merged

Unify LLM call-site configuration under llm.{default,profiles,callSites}#26159
siddseethepalli merged 41 commits into
mainfrom
siddseethepalli/unify-llm-callsites

Conversation

@siddseethepalli
Copy link
Copy Markdown
Contributor

@siddseethepalli siddseethepalli commented Apr 17, 2026

Summary

Consolidates ~26 distinct LLM call sites in the assistant under a unified llm.{default, profiles, callSites} schema. Previously, configuration was scattered across config.json with three different override mechanisms (modelIntent strings, modelOverride strings, ad-hoc speed/model kwargs). Now every LLM call gets fully independent configuration — provider, model, maxTokens, thinking, effort, speed, temperature per call site, with a shared default baseline and reusable named profiles.

The macOS Settings UI gains:

  • An "N per-task overrides" badge on the Inference card
  • A grouped editable sheet for managing per-call-site overrides
  • An override-aware confirmation dialog when changing the global default provider

Self-review result

PASS after 2 remediation rounds (5 round-1 fix PRs + 1 round-2 fix PR).

PRs merged into feature branch

Plan PRs (in merge order)

Schema + resolver + migration (Waves 1-3):

Agent loop + bespoke call-site adoption (Waves 4-5):

macOS UI (Waves 4-7):

Cleanup (Wave 8):

Round-1 self-review fix PRs

Round-2 self-review fix PR

Migration safety

Two workspace migrations land:

  • 038-unify-llm-callsite-configs: backfills llm.{default, callSites, pricingOverrides} from scattered legacy keys (idempotent, deep-merges with any pre-existing llm.callSites/profiles)
  • 039-drop-legacy-llm-keys: strips the now-deprecated keys from config.json after callers stop reading them (idempotent)

Both have test coverage in assistant/src/__tests__/workspace-migration-{038,039}-*.test.ts.

Part of plan: unify-llm-callsites.md


Open with Devin

siddseethepalli and others added 30 commits April 16, 2026 15:47
…efines (#26089)

* config(llm): add unified llm schema with call-site enum and profile refines

* fix(llm-schema): replace deepPartialObject helper with explicit .partial().extend()

Zod 4's readonly shape typing tripped TS2542 in the LSP for the generic walker.
Inline the one-level expansion for ContextWindowSchema and switch the superRefine
issue code to the string literal (Zod 4 deprecated ZodIssueCode).
* config(llm): add resolveCallSiteConfig resolver with deep merge

* fix(llm-resolver): deep-clone nested objects so resolved configs are isolated snapshots

Codex flagged that the merge helper aliased nested objects from llm.default
when no override touched them, so a caller mutating the returned config
would silently corrupt the source. Recurse into plain-object sources
unconditionally and add a regression test.
…ge) (#26095)

* config(llm): add llm field to AssistantConfigSchema (no behavior change)

* fix(llm-schema): add field-level defaults so partial llm configs don't trigger full config reset

Codex flagged that requiring all LLMConfigBase fields meant the loader's
leaf-deletion recovery couldn't repair partial/invalid llm blocks — falling
through to cloneDefaultConfig() and discarding the user's other valid
settings. Add .default(...) to every leaf so LLMSchema.parse({}) returns a
fully-defaulted object, matching the pattern used by sibling config schemas.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…re (#26101)

* workspace: migrate scattered LLM config keys into unified llm structure

* fix(migration): preserve existing llm subtree; map notification intent to both call sites

Codex flagged two issues:
- The migration assignment replaced config.llm wholesale, destroying any
  pre-existing llm.callSites/profiles when llm.default was absent. Now
  merges into existing config.llm, preserving non-conflicting entries.
- notifications.decisionModelIntent drives both notification classification
  and preference extraction, but the migration only seeded
  notificationDecision. Now seeds both call sites.
…llbacks (#26115)

* daemon: thread callSite through processMessage options and adapter callbacks

* fix(callsite-threading): complete interface contract and server.ts symmetry

Devin flagged two gaps in PR #26115:
- ProcessConversationContext interface missing callSite in its
  runAgentLoop options type (works via structural typing but contract
  was incomplete; mocks would silently drop the field).
- DaemonServer.persistAndProcessMessage didn't thread callSite to
  conversation.runAgentLoop, while DaemonServer.processMessage did.
  Aligned.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(callsite): don't default unspecified callers to 'mainAgent'

Codex flagged that defaulting to mainAgent for every turn routes them
through the new RetryProvider call-site resolver, which reads from
llm.default — but config-model.setModel still writes to services.inference
without syncing llm.default. Result: stale/incompatible model IDs after a
model switch.

Defer the cutover. agent-loop turns now keep using the legacy modelIntent
path (turnCallSite = options?.callSite, no fallback). PRs 7-11 still
explicitly pass callSite and route through the new resolver as intended.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…26128)

* macos(settings): add SettingsStore APIs for per-call-site overrides

* fix(callsite-overrides): harden setCallSiteOverrides against dup-id crash and batch divergence

Devin and Codex flagged two issues:
- Dictionary(uniqueKeysWithValues:) crashes if callers pass duplicate
  CallSiteOverride.id values (external input — must be tolerant). Switch
  to Dictionary(_:uniquingKeysWith:) with last-write-wins.
- Batch updates locally cleared entries omitted from the input but only
  PATCHed entries that were present, so omitted entries appeared cleared
  in the UI but reappeared on next sync. Now the PATCH payload includes
  NSNull clears for every catalog entry not in the batch, aligning remote
  with local.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(callsite-overrides): null entire entry on clear so non-UI leaves get cleared too

Codex P2 (PR #26128 cycle 2): clearCallSiteOverride only nulled
provider/model/profile, but call-site config supports additional leaves
(maxTokens, effort, speed, thinking, contextWindow). If those were set
via manual edits, the UI would report cleared while the daemon kept
applying hidden overrides.

Switch the PATCH payload from { provider: null, model: null, profile: null }
to a single null on the entry itself. The Zod fragment treats null as
absent, so the resolver falls back to llm.default. Same fix applies to the
omitted-catalog-entry clears in setCallSiteOverrides batch.

Tests updated to assert the new shape.

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…st sheet (#26135)

* macos(settings): show 'N call-site overrides' badge with read-only list sheet

* fix(comments): drop PR-number breadcrumbs in callsite override files

Devin flagged that comments referencing PR 22/23/24 violate clients/AGENTS.md
'Comment Quality' rule (no breadcrumbs). Replaced with timeless descriptions
of code intent.
…model pickers (#26136)

* macos(settings): make per-task override sheet editable with provider/model pickers

* fix(callsite-sheet): preserve external updates and seed override from active default provider

Codex flagged two P1s:
- syncDraftsFromStore compared drafts against the NEW persisted value to
  decide 'touched', so external store updates were treated as user edits
  and got overwritten by Save All. Track the previously-persisted value
  in lastSyncedFromStore and consider a row touched only when the draft
  differs from that baseline.
- Toggling 'Override default' on initialized provider from
  providerIds.first instead of the user's actual default provider, which
  could pin the wrong provider on save. Pass the user's default provider
  into CallSiteOverrideRow and seed from it.

* fix(callsite-sheet): use entry-level null path for cleared rows in saveAll/resetAll

Devin flagged that saveAll() and resetAll() were passing all-nil entries
to setCallSiteOverrides, which routed them through the field-level null
path (provider/model/profile = null). That left advanced leaves
(maxTokens, effort, temperature, contextWindow) untouched on the daemon.

Fix:
- saveAll(): filter to entries with hasOverride == true; toggled-off rows
  fall through to the entry-level null path.
- resetAll(): pass an empty list so every catalog entry hits the
  entry-level null path.
@siddseethepalli siddseethepalli self-assigned this Apr 17, 2026
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 4 potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Comment thread assistant/src/config/schemas/llm.ts Outdated
Comment thread assistant/src/__tests__/conversation-agent-loop-overflow.test.ts Outdated
Comment on lines +2189 to 2196
if lastDaemonProvider == nil {
if let provider = llmDefault?["provider"] as? String {
self.selectedInferenceProvider = provider
}
if let model = llmDefault?["model"] as? String {
self.selectedModel = model
}
}
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot Apr 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: macOS SettingsStore loadServiceModes reads from llm.default with services.inference fallback

The loadServiceModes change at SettingsStore.swift reads provider/model from llm.default.* first, falling back per-field to services.inference.*. The per-field fallback (rather than block-level) means a partial llm.default block (e.g., only provider set, no model) correctly takes provider from llm.default and model from services.inference. This covers: (1) fully-migrated configs where llm.default has everything, (2) unmigrated configs where only services.inference exists, and (3) edge cases where migration 038 ran but the config was partially written. The services.inference.mode field correctly stays under services since it governs managed-vs-your-own routing, orthogonal to model selection.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread assistant/src/providers/retry.ts
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

const providerConfig: Record<string, unknown> = {
max_tokens: turnMaxTokens,
};

P2 Badge Let call-site maxTokens override AgentLoop defaults

AgentLoop.run always sets config.max_tokens before sending the request. In RetryProvider.normalizeSendMessageOptions, call-site resolution only writes resolved.maxTokens when max_tokens is undefined, so llm.callSites.<id>.maxTokens can never take effect for AgentLoop turns (including mainAgent and other loop-based call sites). As a result, the new per-call-site token-cap setting is ignored in these paths.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const createPromise = (async () => {
const config = getConfig();
let provider = getProvider(config.services.inference.provider);
let provider = getProvider(config.llm.default.provider);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Resolve conversation provider from the active call-site

getOrCreateConversation always constructs Conversation with config.llm.default.provider, even when the queued turn carries options.callSite (e.g. heartbeat, filing, analyze, or any per-task override). Because the provider transport is fixed at construction time, llm.callSites.<id>.provider overrides are never honored for AgentLoop-backed flows; the later callSite threading only adjusts request metadata, not which provider client is used. This makes provider overrides in the new per-call-site config/UI silently no-op whenever they differ from the default provider.

Useful? React with 👍 / 👎.

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

View 11 additional findings in Devin Review.

Open in Devin Review

// Helpers — self-contained per workspace migrations AGENTS.md
// ---------------------------------------------------------------------------

const EFFORT_VALUES = new Set(["low", "medium", "high", "max"]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Migration 038 EFFORT_VALUES set missing "xhigh", silently drops user's effort setting

The EFFORT_VALUES set in migration 038 is ["low", "medium", "high", "max"] but the runtime EffortEnum schema (assistant/src/config/schemas/llm.ts:73) includes "xhigh". When readEnum(config.effort, EFFORT_VALUES) encounters "xhigh", it returns undefined, and the ?? "max" fallback at assistant/src/workspace/migrations/038-unify-llm-callsite-configs.ts:84 replaces it. Since migration 039 then deletes the original top-level effort key, the user's "xhigh" setting is permanently lost and silently replaced with "max".

Suggested change
const EFFORT_VALUES = new Set(["low", "medium", "high", "max"]);
const EFFORT_VALUES = new Set(["low", "medium", "high", "xhigh", "max"]);
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const parsed = LLMSchema.parse({});
expect(parsed.default).toEqual({
provider: "anthropic",
model: "claude-opus-4-6",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Migration 038 default model (claude-opus-4-6) vs schema default (claude-opus-4-7) discrepancy

The migration at assistant/src/workspace/migrations/038-unify-llm-callsite-configs.ts:82 defaults llm.default.model to "claude-opus-4-6" for configs with no explicit model, while LLMConfigBase at assistant/src/config/schemas/llm.ts:222 defaults to "claude-opus-4-7". This means existing installs that never set a model explicitly will be pinned to claude-opus-4-6 by the migration, while fresh installs default to claude-opus-4-7. The test at assistant/src/__tests__/llm-schema.test.ts:73 also expects "claude-opus-4-6" from LLMSchema.parse({}) which contradicts the actual schema default of "claude-opus-4-7" — that test will fail in CI. The migration discrepancy may be intentional (frozen snapshot), but the test vs schema mismatch is a clear error that CI should catch.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

siddseethepalli and others added 5 commits April 17, 2026 19:31
…ation gaps (#26271)

* Fix Chrome extension allowlist ID and clarify README dev setup (#26259)

Update the canonical allowlist to use the correct published CWS
extension ID (hphbdmpffeigpcdjkckleobjmhhokpne). Restructure the
Chrome extension README to clearly explain the allowlist merge
strategy, separate the macOS app (automatic) path from the manual
native messaging setup, and show how dev + prod extensions work
side-by-side.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(clients): enable non-contiguous glyph layout for NSTextView-backed code views (#26242)

TextKit 1 defaults NSLayoutManager.allowsNonContiguousLayout to false,
which forces full-document glyph layout from character 0 on the main
thread whenever a glyph range is queried. Attaching an NSTextView to
its scroll view (setDocumentView: -> _setSuperview: ->
setNeedsDisplayInRect: -> _glyphRangeForBoundingRect:) triggers that
query during makeNSView, producing multi-second hangs on large code
blocks.

Opt into non-contiguous layout on every TextKit 1 stack we build via
NSViewRepresentable so glyph generation is confined to the requested
bounding rect.

Also replace NSLayoutManager.ensureLayout(for:) in the code-view
sizeThatFits paths with direct lineCount * fixedLineHeight math: the
text container is unbounded horizontally (no wrapping) and paragraph
style pins minimumLineHeight == maximumLineHeight, so the geometry is
exact and avoids a second O(glyph count) main-thread path.

Fixes VELLUM-ASSISTANT-MACOS-J2.

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: ashlee@vellum.ai <ashlee@vellum.ai>

* fix(contacts): show Assistant badge for assistant-type contacts (LUM-1009) (#26239)

* fix(contacts): show Assistant badge for assistant-type contacts (LUM-1009)

* Move role/contactType derivation onto Kind for valid initializer

---------

Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(llm-callsite): UI override state divergence, null-as-delete, migration gaps

- deepMergeOverwrite: null on scalar/null targets assigns null (preserves
  nullable config fields like activeHoursStart); null on object targets
  still deletes (call-site clearing). Fixes regression where PATCH with
  null for nullable fields was deleted then re-defaulted.
- InferenceServiceCard: override confirmation dialog only fires when the
  resolved provider ID actually changes, not on mode-only toggles where
  both old and new resolve to the same provider.
- CallSiteOverridesSheet: per-row Save uses replaceCallSiteOverride
  (clear-then-set) so stale daemon-side leaves are removed. The
  partial-update setCallSiteOverride would retain fields the draft nil'd.
- CallSiteOverrideRow: merge consecutive .padding modifiers into single
  EdgeInsets call per macOS AGENTS.md layout rule.
- SettingsStore: add replaceCallSiteOverride for full-entry replacement.

---------

Co-authored-by: Noa Flaherty <noa@vellum.ai>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: ashlee@vellum.ai <ashlee@vellum.ai>
…rovider routing (#26275)

* fix(meet-bot): address review feedback — Docker build, scraper races, audio capture, storage writer (#26264)

* fix(meet): chat concurrency, dispose teardown, and wake adapter fidelity (#26265)

* fix: heartbeat dual-emit, analysis dedup, test hermiticity, credential executor discovery (#26266)

* fix: model default fallback, empty-response nudge scan (#26268)

- Update FALLBACK_DEFAULT_MODEL to claude-opus-4-7 + test
- Fix resolveModel to check Anthropic catalog (not just current default)
  so stale persisted defaults (e.g. claude-opus-4-6) don't get sent
  to non-Anthropic providers
- Fix priorAssistantHadVisibleText backward scan to check ALL prior
  assistant messages, not just the most recent one

Addresses review feedback from PRs #26247, #26164.

* fix(meet): TTS stream races, barge-in tracking, ffmpeg error classification (#26267)

* Fix extension-id-sync-guard test after canonical ID update (#26263)

The guard test asserts that canonical extension IDs appear only in the
allowlist config file. After updating the canonical ID to match the
published CWS extension, it now collides with CWS URLs in README and
browser-execution.ts. Fix by stripping CWS URLs before checking for
bare ID occurrences, and ignore .codex-worktrees (repo copies).
Also remove hardcoded CWS ID from README in favor of reading from
the canonical config.

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(llm-callsite): seed latency-optimized defaults, fix guardian provider routing, clean stale comments

- Add LATENCY_OPTIMIZED_CALLSITE_DEFAULTS to schema for new installs
- Create migration 040 to seed latency-optimized call-site entries for existing workspaces
- Fix guardian-action-generators to use getConfiguredProvider() instead of bypassing call-site resolution
- Restore commitMessage maxTokens: 120 and temperature: 0.2 via call-site defaults
- Remove stale PR-reference comments from analyze-conversation.ts and voice-session-bridge.ts

Addresses consolidated review feedback from PRs #26101-#26140.

---------

Co-authored-by: Noa Flaherty <noa@vellum.ai>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@siddseethepalli
Copy link
Copy Markdown
Contributor Author

Follow-up PR #26275 merged — addresses consolidated review feedback from the callsite unification PRs:

  1. Latency-optimized routing restored: Migration 038 only wrote callSite entries for explicit legacy overrides. Most users had none → latency-sensitive call sites fell through to llm.default (opus). Fixed with schema defaults (new installs) + migration 040 (existing workspaces).
  2. Guardian provider routing fixed: guardian-action-generators.ts bypassed call-site provider resolution. Now uses getConfiguredProvider("guardianQuestionCopy").
  3. Commit message token cap restored: maxTokens: 120 + temperature: 0.2 seeded via call-site defaults.
  4. Stale PR-reference comments removed from analyze-conversation.ts and voice-session-bridge.ts.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 3 new potential issues.

View 12 additional findings in Devin Review.

Open in Devin Review

Comment on lines 40 to 53
if (startNull !== endNull) {
// Emit on both fields so validateWithSchema's delete-and-retry strips
// both sides in one pass. Single-emit on the null side can cascade when
// the explicit value happens to equal the opposite default (e.g.
// { start: null, end: 8 } → strip start → default 8 → equal check fires
// → loader falls back to full defaults, wiping unrelated keys like
// maxTokens).
// Emit only on the null side so validateWithSchema's delete-and-retry
// preserves the explicit non-null value. Dual-emit would delete both
// keys, losing valid explicit values for mixed-null configs like
// { activeHoursStart: null, activeHoursEnd: 20 } → (8, 22) instead of
// retaining the explicit 20.
const message =
"heartbeat.activeHoursStart and heartbeat.activeHoursEnd must both be set or both be null";
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ["activeHoursStart"],
message,
});
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ["activeHoursEnd"],
path: [startNull ? "activeHoursStart" : "activeHoursEnd"],
message,
});
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Heartbeat superRefine single-emit causes cascade to full config defaults when explicit value equals opposite default

The change from dual-emit to single-emit in the heartbeat superRefine re-introduces a config-wipe cascade for specific activeHours configurations. When a user has e.g. { heartbeat: { activeHoursStart: null, activeHoursEnd: 8 } }, the single-emit path emits an issue only on activeHoursStart (the null side). The loader's validateWithSchema (assistant/src/config/loader.ts:70-88) deletes that key and retries. On retry, activeHoursStart defaults to 8 (its schema default), creating start=8, end=8 — which triggers the equal-hours check, emitting a new issue. Since the loader performs only one delete-and-retry pass, this second failure cascades to cloneDefaultConfig(), wiping all unrelated user settings (e.g. llm.default.maxTokens). The old dual-emit approach deleted both sides in one pass, so the retry got (8, 22) — valid — and unrelated fields survived. The same cascade affects { activeHoursStart: 22, activeHoursEnd: null } and { activeHoursStart: N, activeHoursEnd: N } for values equal to the opposite side's default. The existing tests at assistant/src/__tests__/config-schema.test.ts:2316-2356 still expect maxTokens to be preserved, but the mechanical behavior under single-emit would wipe it.

Prompt for agents
The heartbeat superRefine was changed from dual-emit (emitting Zod issues on both activeHoursStart and activeHoursEnd) to single-emit (only on the null side). This introduces a cascade: when the non-null explicit value equals the opposite side's schema default, the loader's single delete-and-retry pass cannot recover — it deletes the null side, the default recreates an equal-hours mismatch, and the retry fails, causing cloneDefaultConfig() to wipe unrelated settings.

The fix should restore dual-emit for the null-mismatch branch (and keep dual-emit for the equal-hours branch) so both sides are deleted in one pass and the retry succeeds with both values at their defaults. This is the behavior the tests at config-schema.test.ts:2316-2356 expect.

Alternatively, if the single-emit behavior is desired (to preserve explicit values), the loader in config/loader.ts would need to be enhanced to support multiple rounds of delete-and-retry, but that is a larger change.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +269 to +290
const LATENCY_OPTIMIZED_FRAGMENT = {
model: "claude-haiku-4-5-20251001",
effort: "low" as const,
thinking: { enabled: false },
};

export const LATENCY_OPTIMIZED_CALLSITE_DEFAULTS: Partial<
Record<LLMCallSite, z.input<typeof LLMCallSiteConfig>>
> = {
guardianQuestionCopy: LATENCY_OPTIMIZED_FRAGMENT,
watchCommentary: LATENCY_OPTIMIZED_FRAGMENT,
interactionClassifier: LATENCY_OPTIMIZED_FRAGMENT,
skillCategoryInference: LATENCY_OPTIMIZED_FRAGMENT,
inviteInstructionGenerator: LATENCY_OPTIMIZED_FRAGMENT,
notificationDecision: LATENCY_OPTIMIZED_FRAGMENT,
preferenceExtraction: LATENCY_OPTIMIZED_FRAGMENT,
commitMessage: {
...LATENCY_OPTIMIZED_FRAGMENT,
maxTokens: 120,
temperature: 0.2,
},
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 LATENCY_OPTIMIZED_CALLSITE_DEFAULTS hardcodes Anthropic model IDs in schema defaults

The LATENCY_OPTIMIZED_CALLSITE_DEFAULTS at assistant/src/config/schemas/llm.ts:269-290 hardcodes model: 'claude-haiku-4-5-20251001' for all latency-optimized call sites. This model ID is Anthropic-specific. When llm.default.provider is set to a non-Anthropic provider (e.g., OpenAI), these schema-level defaults will seed call-site entries with an Anthropic model ID that doesn't exist on the target provider. The migration 040-seed-latency-callsite-defaults.ts correctly resolves provider-appropriate models via its PROVIDER_LATENCY_MODELS table, but the schema defaults are static and always Anthropic. Users on non-Anthropic providers who haven't run migration 040 (or who add new call sites after migration) would get Anthropic model IDs in their call-site defaults. The resolveModel function in assistant/src/providers/registry.ts:85-88 does catch Anthropic models on non-Anthropic providers and substitutes a fallback, which mitigates the issue at the provider initialization level.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread assistant/src/providers/call-site-routing.ts
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 13 additional findings in Devin Review.

Open in Devin Review

Comment on lines +1048 to +1054
provider = new CallSiteRoutingProvider(provider, (name) => {
try {
return getProvider(name);
} catch {
return undefined;
}
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Alternative providers from CallSiteRoutingProvider bypass RateLimitProvider

In assistant/src/daemon/server.ts:1048-1061, the provider stack for conversations is RateLimitProvider(CallSiteRoutingProvider(RetryProvider(defaultProvider))). When CallSiteRoutingProvider routes to an alternative provider via getProviderByName, that alternative comes directly from the registry (getProvider(name)) at line 1050 and is only wrapped in RetryProvider — NOT in RateLimitProvider. This means per-call-site provider overrides that route to a different transport bypass the conversation's rate limiter. Whether this is intentional (different providers have separate rate budgets) or a gap depends on the desired rate-limiting semantics. The same pattern appears in assistant/src/subagent/manager.ts:224-229.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@siddseethepalli siddseethepalli merged commit f8e0abd into main Apr 18, 2026
14 of 15 checks passed
@siddseethepalli siddseethepalli deleted the siddseethepalli/unify-llm-callsites branch April 18, 2026 02:15
siddseethepalli added a commit that referenced this pull request Apr 18, 2026
PR #26159 (LLM callsite unification) accidentally reverted two earlier
fixes when squash-merged from a stale base:

1. Re-applies the heartbeat dual-emit superRefine from #26278 so
   validateWithSchema's delete-and-retry strips both activeHours sides
   in one pass. Single-emit cascades when the explicit value equals the
   opposite default (e.g. { start: null, end: 8 }), causing the loader
   to fall back to full defaults and wipe unrelated fields like
   llm.default.maxTokens.

2. Changes LLMSchema.callSites to default to {} instead of
   LATENCY_OPTIMIZED_CALLSITE_DEFAULTS. Latency-optimized call-site
   defaults are owned by workspace migration 040 (which seeds them into
   the user's on-disk config), not the schema layer. Leaving the schema
   default populated polluted parsed configs with unrequested entries
   and broke the 'empty callSites by default' invariant that tests and
   downstream callers rely on. LATENCY_OPTIMIZED_FRAGMENT /
   LATENCY_OPTIMIZED_CALLSITE_DEFAULTS are now unused and removed.
siddseethepalli added a commit that referenced this pull request Apr 18, 2026
…ult (#26286)

PR #26159 (LLM callsite unification) accidentally reverted two earlier
fixes when squash-merged from a stale base:

1. Re-applies the heartbeat dual-emit superRefine from #26278 so
   validateWithSchema's delete-and-retry strips both activeHours sides
   in one pass. Single-emit cascades when the explicit value equals the
   opposite default (e.g. { start: null, end: 8 }), causing the loader
   to fall back to full defaults and wipe unrelated fields like
   llm.default.maxTokens.

2. Changes LLMSchema.callSites to default to {} instead of
   LATENCY_OPTIMIZED_CALLSITE_DEFAULTS. Latency-optimized call-site
   defaults are owned by workspace migration 040 (which seeds them into
   the user's on-disk config), not the schema layer. Leaving the schema
   default populated polluted parsed configs with unrequested entries
   and broke the 'empty callSites by default' invariant that tests and
   downstream callers rely on. LATENCY_OPTIMIZED_FRAGMENT /
   LATENCY_OPTIMIZED_CALLSITE_DEFAULTS are now unused and removed.
siddseethepalli added a commit that referenced this pull request Apr 18, 2026
… llm.default API

The tests were added in #26251 against the old `setInferenceProvider` /
`services.inference.provider` API. #26159 merged afterward renamed that
API to `setLLMDefaultProvider` and moved the config path to
`llm.default.provider`, leaving the tests unable to compile.

Rename the calls and update the patch assertions to match the new shape.
siddseethepalli added a commit that referenced this pull request Apr 18, 2026
… llm.default API (#26287)

The tests were added in #26251 against the old `setInferenceProvider` /
`services.inference.provider` API. #26159 merged afterward renamed that
API to `setLLMDefaultProvider` and moved the config path to
`llm.default.provider`, leaving the tests unable to compile.

Rename the calls and update the patch assertions to match the new shape.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant