Skip to content

fix LiteLLM model issue #68

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tysonthomas9 merged 2 commits into from
Nov 11, 2025
Merged

fix LiteLLM model issue #68

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69839dc
fix litellm model issue
tysonthomas9 Nov 11, 2025
76d53d0
update based on comments
tysonthomas9 Nov 11, 2025
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
36 changes: 24 additions & 12 deletions front_end/panels/ai_chat/agent_framework/ConfigurableAgentTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,9 +460,10 @@ export class ConfigurableAgentTool implements Tool<ConfigurableAgentArgs, Config
const apiKey = callCtx.apiKey;
const provider = callCtx.provider;

// Check if this provider requires an API key
// BrowserOperator doesn't require an API key (endpoint is hardcoded)
const requiresApiKey = provider !== 'browseroperator';
// Check if API key is required based on provider
// LiteLLM and BrowserOperator have optional API keys
// Other providers (OpenAI, Groq, OpenRouter) require API keys
const requiresApiKey = provider !== 'litellm' && provider !== 'browseroperator';
Comment on lines +463 to +466
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Reorder provider validation before API‑key gating to avoid misleading errors

If provider is undefined, the current order can return “API key not configured” instead of the clearer “Provider not provided…”. Validate provider first, then compute requiresApiKey.

-    // Check if API key is required based on provider
-    // LiteLLM and BrowserOperator have optional API keys
-    // Other providers (OpenAI, Groq, OpenRouter) require API keys
-    const requiresApiKey = provider !== 'litellm' && provider !== 'browseroperator';
-
-    if (requiresApiKey && !apiKey) {
+    // Validate required context first
+    if (!callCtx.provider) {
+      throw new Error(`Provider not provided in context for agent '${this.name}'. Ensure context includes provider.`);
+    }
+
+    // Check if API key is required based on provider (normalize for safety)
+    const p = String(callCtx.provider).toLowerCase();
+    const requiresApiKey = p !== 'litellm' && p !== 'browseroperator';
+
+    if (requiresApiKey && !apiKey) {
       const errorResult = this.createErrorResult(`API key not configured for ${this.name}`, [], 'error');
       // Create minimal error session
       const errorSession: AgentSession = {
         agentName: this.name,

Also applies to: 541-545

🤖 Prompt for AI Agents 
In front_end/panels/ai_chat/agent_framework/ConfigurableAgentTool.ts around
lines 463-466 (and also apply same change at 541-545): currently the code
computes requiresApiKey using the provider variable before validating that
provider is present, which can produce an “API key not configured” error when
provider is undefined; change the flow to validate that provider is provided
first (throw or return the clearer “Provider not provided” error) and only after
that compute requiresApiKey (e.g., determine provider-specific optional/required
status) so you never evaluate provider comparisons when provider is undefined.


if (requiresApiKey && !apiKey) {
const errorResult = this.createErrorResult(`API key not configured for ${this.name}`, [], 'error');
Expand Down Expand Up @@ -499,15 +500,17 @@ export class ConfigurableAgentTool implements Tool<ConfigurableAgentArgs, Config
// Resolve model name from context or configuration
let modelName: string;
if (this.config.modelName === MODEL_SENTINELS.USE_MINI) {
if (!callCtx.miniModel) {
throw new Error(`Mini model not provided in context for agent '${this.name}'. Ensure context includes miniModel.`);
// Fall back to main model if mini model is not configured
modelName = callCtx.miniModel || callCtx.mainModel || callCtx.model || '';
if (!modelName) {
Comment on lines 501 to +505
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Nov 11, 2025

Choose a reason for hiding this comment

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

P1 Badge Propagate fallback model into call context

The new fallback for MODEL_SENTINELS.USE_MINI/USE_NANO now chooses callCtx.mainModel/callCtx.model when a mini or nano model is absent, but the fallback is only assigned to the local modelName. The subsequent runnerConfig still forwards the original callCtx.miniModel/callCtx.nanoModel values (which are undefined), so tools that rely on ctx.miniModel or ctx.nanoModel (e.g. FullPageAccessibilityTreeToMarkdownTool checks ctx.nanoModel and returns “Missing LLM context”) will still fail even though a fallback model is available. Before this change the agent aborted early with a clear error; now it proceeds until a tool accesses the missing field. Update the context to include the resolved fallback model so mini/nano-dependent tools can run.

Useful? React with 👍 / 👎.

throw new Error(`Mini model not provided in context for agent '${this.name}'. Ensure context includes miniModel or mainModel.`);
}
Comment on lines +503 to 507
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Include ‘model’ in USE_MINI failure message to match actual fallback chain

Code falls back to callCtx.model, but the error mentions only miniModel/mainModel. Update message for accuracy.

-        throw new Error(`Mini model not provided in context for agent '${this.name}'. Ensure context includes miniModel or mainModel.`);
+        throw new Error(`Mini model not provided in context for agent '${this.name}'. Ensure context includes miniModel, mainModel, or model.`);
🤖 Prompt for AI Agents 
In front_end/panels/ai_chat/agent_framework/ConfigurableAgentTool.ts around
lines 503 to 507, the thrown error message only references miniModel and
mainModel while the fallback chain actually includes callCtx.model; update the
error string to include 'model' (or phrase like "miniModel, mainModel, or
model") so the message accurately reflects the fallback order, keeping the rest
of the logic unchanged.

modelName = callCtx.miniModel;
} else if (this.config.modelName === MODEL_SENTINELS.USE_NANO) {
if (!callCtx.nanoModel) {
throw new Error(`Nano model not provided in context for agent '${this.name}'. Ensure context includes nanoModel.`);
// Fall back through nano -> mini -> main model chain
modelName = callCtx.nanoModel || callCtx.miniModel || callCtx.mainModel || callCtx.model || '';
if (!modelName) {
throw new Error(`Nano model not provided in context for agent '${this.name}'. Ensure context includes nanoModel, miniModel, or mainModel.`);
}
Comment on lines +509 to 513
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Include ‘model’ in USE_NANO failure message to match actual fallback chain

The code falls back nano → mini → main → model; error omits model.

-        throw new Error(`Nano model not provided in context for agent '${this.name}'. Ensure context includes nanoModel, miniModel, or mainModel.`);
+        throw new Error(`Nano model not provided in context for agent '${this.name}'. Ensure context includes nanoModel, miniModel, mainModel, or model.`);
🤖 Prompt for AI Agents 
In front_end/panels/ai_chat/agent_framework/ConfigurableAgentTool.ts around
lines 509 to 513, the fallback chain used to determine modelName is nano -> mini
-> main -> model but the thrown Error message only mentions nanoModel,
miniModel, and mainModel; update the error text to include 'model' so it
reflects the actual fallback chain (e.g., "Ensure context includes nanoModel,
miniModel, mainModel, or model.") and keep the rest of the logic unchanged.

modelName = callCtx.nanoModel;
} else if (typeof this.config.modelName === 'function') {
modelName = this.config.modelName();
} else if (this.config.modelName) {
Expand All @@ -525,16 +528,25 @@ export class ConfigurableAgentTool implements Tool<ConfigurableAgentArgs, Config
if (callCtx.model && !this.config.modelName) {
modelName = callCtx.model;
}


// Update context with resolved fallback models for tools to use
// This ensures tools that check ctx.miniModel or ctx.nanoModel get the fallback
if (this.config.modelName === MODEL_SENTINELS.USE_MINI && !callCtx.miniModel) {
callCtx.miniModel = modelName; // Use the resolved fallback
}
if (this.config.modelName === MODEL_SENTINELS.USE_NANO && !callCtx.nanoModel) {
callCtx.nanoModel = modelName; // Use the resolved fallback
}

// Validate required context
if (!callCtx.provider) {
throw new Error(`Provider not provided in context for agent '${this.name}'. Ensure context includes provider.`);
}

const temperature = this.config.temperature ?? 0;
const systemPrompt = this.config.systemPrompt;
const tools = this.getToolInstances();

// Prepare initial messages
const internalMessages = this.prepareInitialMessages(args);
const runnerConfig: AgentRunnerConfig = {
Expand Down
4 changes: 2 additions & 2 deletions front_end/panels/ai_chat/tools/CombinedExtractionTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ export class CombinedExtractionTool implements Tool<CombinedExtractionArgs, Comb
// Get provider from context
const provider = ctx?.provider;

// BrowserOperator doesn't require API key
const requiresApiKey = provider !== 'browseroperator';
// LiteLLM and BrowserOperator have optional API keys
const requiresApiKey = provider !== 'litellm' && provider !== 'browseroperator';

if (requiresApiKey && !apiKey) {
return {
Expand Down
4 changes: 2 additions & 2 deletions front_end/panels/ai_chat/tools/FullPageAccessibilityTreeToMarkdownTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ export class FullPageAccessibilityTreeToMarkdownTool implements Tool<Record<stri
const provider = ctx.provider;
const model = ctx.nanoModel;

// BrowserOperator doesn't require API key
const requiresApiKey = provider !== 'browseroperator';
// LiteLLM and BrowserOperator have optional API keys
const requiresApiKey = provider !== 'litellm' && provider !== 'browseroperator';

if (requiresApiKey && !apiKey) {
return { error: 'API key not configured.' };
Expand Down
4 changes: 2 additions & 2 deletions front_end/panels/ai_chat/tools/HTMLToMarkdownTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ export class HTMLToMarkdownTool implements Tool<HTMLToMarkdownArgs, HTMLToMarkdo
// Get provider from context
const provider = ctx?.provider;

// BrowserOperator doesn't require API key
const requiresApiKey = provider !== 'browseroperator';
// LiteLLM and BrowserOperator have optional API keys
const requiresApiKey = provider !== 'litellm' && provider !== 'browseroperator';

if (requiresApiKey && !apiKey) {
return {
Expand Down
4 changes: 2 additions & 2 deletions front_end/panels/ai_chat/tools/SchemaBasedExtractorTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ Schema Examples:
// Get provider from context
const provider = ctx?.provider;

// BrowserOperator doesn't require API key
const requiresApiKey = provider !== 'browseroperator';
// LiteLLM and BrowserOperator have optional API keys
const requiresApiKey = provider !== 'litellm' && provider !== 'browseroperator';

if (requiresApiKey && !apiKey) {
return {
Expand Down
4 changes: 2 additions & 2 deletions front_end/panels/ai_chat/tools/StreamlinedSchemaExtractorTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ export class StreamlinedSchemaExtractorTool implements Tool<StreamlinedSchemaExt
// Get provider from context
const provider = ctx?.provider;

// BrowserOperator doesn't require API key
const requiresApiKey = provider !== 'browseroperator';
// LiteLLM and BrowserOperator have optional API keys
const requiresApiKey = provider !== 'litellm' && provider !== 'browseroperator';

if (requiresApiKey && !apiKey) {
return {
Expand Down
4 changes: 2 additions & 2 deletions front_end/panels/ai_chat/tools/Tools.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2967,8 +2967,8 @@ Important guidelines:
return { error: 'Missing LLM context (provider/model) for ObjectiveDrivenActionTool' };
}

// BrowserOperator doesn't require API key
const requiresApiKey = providerForAction !== 'browseroperator';
// LiteLLM and BrowserOperator have optional API keys
const requiresApiKey = providerForAction !== 'litellm' && providerForAction !== 'browseroperator';

if (requiresApiKey && !apiKey) {return { error: 'API key not configured.' };}
Comment on lines +2970 to 2973
Copy link

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Normalize provider; also update 404 confirmation path to honor LiteLLM/BrowserOperator exemption

First, normalize here:

-    // LiteLLM and BrowserOperator have optional API keys
-    const requiresApiKey = providerForAction !== 'litellm' && providerForAction !== 'browseroperator';
+    // LiteLLM and BrowserOperator have optional API keys
+    const p = String(providerForAction || '').toLowerCase();
+    const requiresApiKey = p !== 'litellm' && p !== 'browseroperator';

Additionally, confirmWith404LLM currently bails out if no API key even for litellm/browseroperator. Apply the same exemption there to enable 404 checks without a key:

// Inside confirmWith404LLM(...)
const apiKey = agentService.getApiKey();
const p = String(ctx?.provider || '').toLowerCase();
const requiresApiKey = p !== 'litellm' && p !== 'browseroperator';
if (requiresApiKey && !apiKey) {
  logger.warn('No API key available for 404 confirmation');
  return false;
}

if (typeof objective !== 'string' || objective.trim() === '') {
Expand Down