Skip to content

feat(auto-model): add reasoning support - #768

Merged
steebchen merged 4 commits into
mainfrom
terragon/support-reasoning-in-auto-model
Sep 10, 2025
Merged

steebchen merged 4 commits into
mainfrom
terragon/support-reasoning-in-auto-model

Conversation

@steebchen

@steebchen steebchen commented Sep 8, 2025

Copy link
Copy Markdown
Member

Summary

  • Adds support for reasoning capability in automatic model selection
  • Expands allowed auto models to include reasoning-capable models when reasoning_effort is specified
  • Filters model providers by reasoning capability in addition to context size

Changes

Core Functionality

  • Extended the list of allowed auto models to include reasoning-capable models if reasoning_effort is provided
  • Updated filtering logic to ensure only providers supporting reasoning are considered when reasoning_effort is specified

Test plan

  • Verify that models capable of reasoning are included in auto selection when reasoning_effort is set
  • Confirm that providers without reasoning capability are excluded when reasoning_effort is specified
  • Ensure existing behavior remains unchanged when reasoning_effort is not provided
  • Test with various context size requirements to validate filtering logic

🌿 Generated by Terry


ℹ️ Tag @terragon-labs to ask questions and address PR feedback

📎 Task: https://www.terragonlabs.com/task/2be69044-f5c3-4659-8752-af969fc9c0e5

Summary by CodeRabbit

  • New Features
    • Auto-routing now prefers reasoning-capable providers when a reasoning level is requested; otherwise only context-size is considered.
    • Explicit model requests validate reasoning support and return a clear 400 error if unsupported.
    • Auto/custom model selections accept reasoning preferences without upfront validation, enabling dynamic provider resolution.

- Expanded allowedAutoModels to include reasoning-capable models when reasoning_effort is specified
- Updated provider filtering to consider reasoning capability along with context size
- Ensures models/providers supporting reasoning are prioritized when reasoning_effort is used

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Sep 8, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Updates chat request handling to adjust reasoning_effort validation and auto-routing filtering. Validation now applies only to non-auto/custom models. Auto-routing filters providers by context size and, when reasoning_effort is present, by reasoning capability. No exported signatures changed.

Changes

Cohort / File(s) Summary
Chat auto-routing & reasoning validation
apps/gateway/src/chat/chat.ts
- Validate reasoning_effort only for non-auto/custom models; 400 if no provider supports reasoning
- In auto routing, when reasoning_effort is set, filter providers to those with reasoning capability and sufficient context; otherwise, filter by context only
- No public API signature changes

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant G as Chat Gateway
  participant R as Provider Registry
  participant P as Provider

  Note over C,G: Request includes model and optional reasoning_effort

  alt model is non-auto/custom
    G->>R: Check if any provider supports reasoning (when reasoning_effort defined)
    alt no reasoning-capable provider
      G-->>C: 400 Bad Request (reasoning unsupported)
    else provider(s) exist
      G->>P: Forward request to selected provider
      P-->>G: Response
      G-->>C: Response
    end
  else model is auto/custom
    G->>R: Get candidate providers
    alt reasoning_effort defined
      rect rgba(200,240,255,0.3)
      Note over G,R: Filter by context size AND reasoning==true
      end
    else no reasoning_effort
      Note over G,R: Filter by context size only
    end
    G->>P: Route to chosen provider
    P-->>G: Response
    G-->>C: Response
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • smakosh

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title “feat(auto-model): add reasoning support” concisely describes the primary change to enable reasoning capability in automatic model selection and aligns directly with the modifications made to filtering logic and model resolution in the pull request. It is clear, specific, and focused on the main feature introduced.
Description Check ✅ Passed The description outlines the reasoning-support feature, details the adjustments to model filtering and auto-selection behavior, and provides a relevant test plan that matches the summary of changes, demonstrating clear relevance to the pull request’s objectives. It remains on-topic and clearly conveys the scope and verification steps for the update.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch terragon/support-reasoning-in-auto-model

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot changed the title Support reasoning capability in auto model selection feat(auto-model): add reasoning support Sep 8, 2025
steebchen and others added 3 commits September 9, 2025 01:33
Eliminated reasoning-capable models from the auto selection list when `reasoning_effort` is specified. This ensures the list only contains hardcoded models for consistent behavior.
Skip reasoning support check for models set to "auto" or "custom" since they are resolved dynamically. This prevents incorrect validation failures when reasoning_effort is specified for these model types.

Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
@steebchen
steebchen marked this pull request as ready for review September 10, 2025 00:23
@steebchen
steebchen added this pull request to the merge queue Sep 10, 2025

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/gateway/src/chat/chat.ts (1)

2214-2244: Validate reasoning support against the explicitly requested provider (not just “any” provider).

Current check passes if any provider for the model supports reasoning, which allows reasoning_effort with a provider that doesn’t. This will later misroute or cause upstream errors. When a provider is explicitly requested (e.g., openai/...), validate that provider’s mapping has reasoning === true; otherwise, keep the “any provider” check.

Apply:

-// Skip this check for "auto" and "custom" models as they will be resolved dynamically
-if (
-  reasoning_effort !== undefined &&
-  requestedModel !== "auto" &&
-  requestedModel !== "custom"
-) {
-  // Check if any provider for this model supports reasoning
-  const supportsReasoning = modelInfo.providers.some(
-    (provider) => (provider as ProviderModelMapping).reasoning === true,
-  );
-
-  if (!supportsReasoning) {
+// Skip this check for "auto" and "custom" models as they will be resolved dynamically
+if (
+  reasoning_effort !== undefined &&
+  requestedModel !== "auto" &&
+  requestedModel !== "custom"
+) {
+  // If a provider was explicitly requested, require that exact mapping to support reasoning.
+  // Otherwise, ensure at least one provider supports reasoning.
+  const supportsReasoningForRequested =
+    requestedProvider
+      ? modelInfo.providers.some(
+          (p) =>
+            p.providerId === requestedProvider &&
+            (p as ProviderModelMapping).reasoning === true,
+        )
+      : modelInfo.providers.some(
+          (p) => (p as ProviderModelMapping).reasoning === true,
+        );
+
+  if (!supportsReasoningForRequested) {
     logger.error(
       `Reasoning effort specified for non-reasoning model: ${requestedModel}`,
       {
         requestedModel,
         requestedProvider,
         reasoning_effort,
         modelProviders: modelInfo.providers.map((p) => ({
           providerId: p.providerId,
           reasoning: (p as ProviderModelMapping).reasoning,
         })),
       },
     );
 
     throw new HTTPException(400, {
       message: `Model ${requestedModel} does not support reasoning. Remove the reasoning_effort parameter or use a reasoning-capable model.`,
     });
   }
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 54f9033 and 668c5eb.

📒 Files selected for processing (1)
  • apps/gateway/src/chat/chat.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use localStorage instead of cookies for client-side data persistence

Files:

  • apps/gateway/src/chat/chat.ts
**/*.{js,ts}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,ts}: Use drizzle with the latest object syntax for database operations
For read queries, always use db().query.<table>.findMany() or db().query.<table>.findFirst()

Files:

  • apps/gateway/src/chat/chat.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/general.mdc)

Never use as any or : any in TypeScript files.

Files:

  • apps/gateway/src/chat/chat.ts
apps/{api,gateway}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/{api,gateway}/**/*.{ts,tsx}: Use Drizzle with the latest object syntax for database operations
For read queries, use db().query.

.findMany() or db().query.
.findFirst()

apps/{api,gateway}/**/*.{ts,tsx}: Use the Hono framework for backend HTTP services in apps/api and apps/gateway
Use Zod for request/response validation in backend routes and handlers
Maintain OpenAPI/Swagger documentation for backend APIs

Files:

  • apps/gateway/src/chat/chat.ts
apps/gateway/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

In apps/gateway (Hono), always use Hono + Zod + OpenAPI for validation and typesafety

Files:

  • apps/gateway/src/chat/chat.ts
🧬 Code graph analysis (1)
apps/gateway/src/chat/chat.ts (1)
packages/models/src/models.ts (1)
  • ProviderModelMapping (23-100)

Comment on lines +2456 to 2471
// Filter by context size requirement and reasoning capability if needed
const suitableProviders = availableModelProviders.filter((provider) => {
// Use the provider's context size, defaulting to a reasonable value if not specified
const modelContextSize = provider.contextSize ?? 8192;
return modelContextSize >= requiredContextSize;
const contextSizeMet = modelContextSize >= requiredContextSize;

// If reasoning_effort is specified, only include providers that support reasoning
if (reasoning_effort !== undefined) {
return (
contextSizeMet &&
(provider as ProviderModelMapping).reasoning === true
);
}

return contextSizeMet;
});

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.

⚠️ Potential issue

Auto-routing ignores reasoning_effort because allowedAutoModels isn’t expanded; fallback silently drops the requirement.

  • You correctly filter providers by reasoning when reasoning_effort is set, but the candidate model list stays fixed to nanos. If none of those nanos are reasoning-capable, selection fails and we fall back to a non-reasoning model, violating the request/PR objective.
  • Expand allowedAutoModels to include all reasoning-capable models when reasoning_effort is provided, and avoid defaulting to non-reasoning models when none are available.

Add after defining allowedAutoModels (and after the free_models_only branch):

// If reasoning is requested, consider all reasoning-capable models for auto routing
if (reasoning_effort !== undefined) {
  const reasoningModelIds = Array.from(
    new Set(
      models
        .filter(
          (m) =>
            m.id !== "auto" &&
            m.id !== "custom" &&
            m.providers?.some((p) => (p as ProviderModelMapping).reasoning === true),
        )
        .map((m) => m.id),
    ),
  );
  allowedAutoModels = Array.from(new Set([...allowedAutoModels, ...reasoningModelIds]));
}

Also, enforce the requirement in the fallback block so we don’t silently choose a non-reasoning model:

// Before default fallback
if (!selectedModel) {
  if (reasoning_effort !== undefined) {
    throw new HTTPException(400, {
      message:
        "No reasoning-capable providers available for auto routing. Remove reasoning_effort or choose a specific reasoning model.",
    });
  }
  // existing non-reasoning fallback
  usedModel = "gpt-5-nano";
  usedProvider = "openai";
}
🤖 Prompt for AI Agents
In apps/gateway/src/chat/chat.ts around lines 2456 to 2471, the auto-routing
logic filters providers by reasoning but never expands allowedAutoModels to
include non-nano reasoning-capable models (so the candidate set can lack
reasoning providers) and the fallback silently picks a non-reasoning model; fix
it by, immediately after allowedAutoModels is defined (and after the
free_models_only branch), append all reasoning-capable model ids to
allowedAutoModels when reasoning_effort is set (dedupe via a Set), and in the
fallback where selectedModel is null, if reasoning_effort is provided throw an
HTTPException(400) with a clear message about no reasoning-capable providers
available instead of falling back to a non-reasoning model.

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