Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion assistant/src/calls/guardian-question-copy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export async function generateGuardianCopy(
[userMessage(prompt)],
undefined,
undefined,
{ signal, config: { modelIntent: "latency-optimized" } },
{ signal, config: { callSite: "guardianQuestionCopy" } },

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.

🔴 callSite not passed to resolveConfiguredProvider(), causing potential provider/model mismatch

The PR migrates guardian-question-copy.ts from modelIntent to callSite in the sendMessage config (line 85), but the resolveConfiguredProvider() call at assistant/src/calls/guardian-question-copy.ts:55 is not updated to pass the callSite. resolveConfiguredProvider(callSite?) uses the call-site config to select the correct provider when callSite is given (assistant/src/providers/provider-send-message.ts:67-68), but falls back to the legacy services.inference.provider path when it's omitted. If a user configures llm.callSites.guardianQuestionCopy.provider = "openai", the code selects the default provider (e.g. Anthropic) at line 55 but RetryProvider.normalizeViaCallSite resolves an OpenAI model from the callSite config — sending an incompatible model name to the wrong provider, causing an API error and forcing a fallback.

Prompt for agents
The callSite "guardianQuestionCopy" is passed in the sendMessage config at line 85, but the provider is resolved at line 55 without the callSite argument. resolveConfiguredProvider() supports an optional callSite parameter that routes provider selection through the llm.callSites config (see provider-send-message.ts:48-69). To fix, pass the callSite to the provider resolution: change line 55 from resolveConfiguredProvider() to resolveConfiguredProvider("guardianQuestionCopy"). This ensures the provider selected matches the model/settings that the RetryProvider resolves via the callSite config.
Open in Devin Review

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

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.

🚩 AGENTS.md rule about modelIntent may need updating for callSite migration

The AGENTS.md states: "Use modelIntent ('latency-optimized', 'quality-optimized', 'vision-optimized') instead of hardcoded model IDs." This PR migrates away from modelIntent to the newer callSite system which supersedes it. The codebase clearly treats modelIntent as legacy (assistant/src/providers/retry.ts:91 comments "Legacy modelIntent path (preserved)") while callSite is the new unified path. The AGENTS.md rule should be updated to reflect the current callSite pattern as the preferred approach, with modelIntent documented as legacy. Not flagged as a bug since the spirit of the rule (avoid hardcoded model IDs) is preserved and the codebase has already established the callSite pattern.

Open in Devin Review

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep guardian copy on latency-optimized model

This migration drops the previous latency-optimized intent for guardian copy without providing an equivalent per-call-site override. Since unresolved call sites inherit llm.default (assistant/src/config/llm-resolver.ts:31-37), guardian copy now uses the default model (assistant/src/config/schemas/llm.ts:223-225) rather than the old latency intent mapping (assistant/src/providers/model-intents.ts:15), increasing the chance that the 5s generation window falls back to deterministic copy.

Useful? React with 👍 / 👎.

);

const text = extractText(response);
Expand Down
4 changes: 2 additions & 2 deletions assistant/src/daemon/watch-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ async function generateCommentary(session: WatchSession): Promise<void> {
systemPrompt,
{
config: {
modelIntent: "latency-optimized",
callSite: "watchCommentary",

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.

🔴 callSite not passed to getConfiguredProvider() for watchCommentary, causing potential provider/model mismatch

Same pattern as in guardian-question-copy: callSite: "watchCommentary" is passed in the sendMessage config, but getConfiguredProvider() at assistant/src/daemon/watch-handler.ts:116 is called without the callSite argument. getConfiguredProvider(callSite?) delegates to resolveConfiguredProvider(callSite) which selects the provider from llm.callSites[id].provider when given a callSite (assistant/src/providers/provider-send-message.ts:94-97). Without it, the legacy services.inference.provider is used, risking a provider/model mismatch if the user configured a different provider for watchCommentary.

Prompt for agents
The callSite "watchCommentary" is passed in the sendMessage config at line 167, but the provider is resolved at line 116 via getConfiguredProvider() without a callSite argument. getConfiguredProvider() supports an optional callSite parameter (see provider-send-message.ts:93-97). To fix, change line 116 from getConfiguredProvider() to getConfiguredProvider("watchCommentary"). This ensures the provider selected matches the model/settings resolved from the callSite config by the RetryProvider.
Open in Devin Review

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

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 Restore latency routing for live watch commentary

Replacing modelIntent: "latency-optimized" with callSite: "watchCommentary" changes default model selection because resolveCallSiteConfig falls back to llm.default when no llm.callSites.watchCommentary entry exists (assistant/src/config/llm-resolver.ts:31-37), and this repo does not seed that call-site override anywhere. On default config, that means commentary now runs on llm.default.model (claude-opus-4-6), not the prior latency model (claude-haiku-4-5-20251001), which can noticeably increase latency/cost for real-time commentary.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Pass callSite into provider selection for watch calls

The request now declares a call-site ID, but watch generation still obtains its transport via getConfiguredProvider() with no call-site argument, so provider choice stays on services.inference.provider (assistant/src/providers/provider-send-message.ts:66-69). At the same time, RetryProvider applies model values from the call-site config (assistant/src/providers/retry.ts:178-194), which can produce cross-provider mismatches (e.g., OpenAI model resolved for a call-site but request sent through Anthropic) and runtime failures when users set llm.callSites.watchCommentary/watchSummary.provider overrides.

Useful? React with 👍 / 👎.

max_tokens: 200,
},
},
Expand Down Expand Up @@ -329,7 +329,7 @@ export async function generateSummary(session: WatchSession): Promise<void> {
systemPrompt,
{
config: {
modelIntent: "quality-optimized",
callSite: "watchSummary",

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.

🔴 callSite not passed to getConfiguredProvider() for watchSummary, causing potential provider/model mismatch

Same pattern as the other two call sites: callSite: "watchSummary" is passed in the sendMessage config, but getConfiguredProvider() at assistant/src/daemon/watch-handler.ts:228 is called without the callSite argument. If a user configures llm.callSites.watchSummary.provider to a different provider than the default, the wrong provider is used for the actual API call while the RetryProvider resolves model/settings from the callSite config, causing a mismatch.

Prompt for agents
The callSite "watchSummary" is passed in the sendMessage config at line 332, but the provider is resolved at line 228 via getConfiguredProvider() without a callSite argument. getConfiguredProvider() supports an optional callSite parameter (see provider-send-message.ts:93-97). To fix, change line 228 from getConfiguredProvider() to getConfiguredProvider("watchSummary"). This ensures the provider selected matches the model/settings resolved from the callSite config by the RetryProvider.
Open in Devin Review

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

max_tokens: 2000,
},
},
Expand Down
Loading