DOC-913 | Supported LLM models - #1015
Conversation
|
Deploy Preview Available Via |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a centralized supported-model catalog, a Hugo shortcode to render it, and documentation updates that surface the supported LLM and embedding models for the suite and importer. ChangesLLM Model Registry and Documentation System
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
site/themes/arangodb-docs-theme/layouts/shortcodes/llm-models.md (1)
19-19: ⚡ Quick winConsider extracting complex inline logic into template variables.
Line 19 contains deeply nested conditionals that build the type label, default indicator, and services list all in a single expression. While functional, this reduces readability and makes the template harder to maintain.
♻️ Proposed refactor to improve readability
{{ range $models -}} +{{- $typeLabel := .type -}} +{{- if eq .type "chat" }}{{ $typeLabel = "Chat (LLM)" }}{{ else if eq .type "embedding" }}{{ $typeLabel = "Embedding" }}{{ end -}} +{{- $isDefault := in (.default_for | default slice) $service -}} +{{- $serviceNames := slice }}{{ range .services }}{{ $serviceNames = $serviceNames | append (index $svcMap .) }}{{ end -}} -| `{{ .name }}` | {{ if eq .type "chat" }}Chat (LLM){{ else if eq .type "embedding" }}Embedding{{ else }}{{ .type }}{{ end }} |{{ if $service }}{{ if in (.default_for | default slice) $service }} Yes |{{ else }} |{{ end }}{{ else }} {{ $names := slice }}{{ range .services }}{{ $names = $names | append (index $svcMap .) }}{{ end }}{{ delimit $names ", " }} |{{ end }} +| `{{ .name }}` | {{ $typeLabel }} |{{ if $service }}{{ if $isDefault }} Yes |{{ else }} |{{ end }}{{ else }} {{ delimit $serviceNames ", " }} |{{ end }} {{ end -}}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/themes/arangodb-docs-theme/layouts/shortcodes/llm-models.md` at line 19, The table row on line 19 is hard to read due to nested inline conditionals; extract the logic into template variables like $typeLabel (compute from .type with eq checks), $defaultIndicator (check $service against .default_for using in or default slice), and $serviceNames (map .services via $svcMap and delimit) then replace the long inline expression with those variables in the row; reference the fields .name, .type, .default_for, .services and the map $svcMap when creating these variables so the table cell simply uses $typeLabel, $defaultIndicator, and $serviceNames.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@site/data/llm_models.yaml`:
- Around line 32-48: The YAML defines conflicting default_for: [importer] for
both gpt-5.4-nano and mistralai/mistral-nemo, causing ambiguity; fix by removing
the default_for entry from the provider-specific model (remove default_for from
mistralai/mistral-nemo), keep gpt-5.4-nano as the canonical default_for:
[importer], and add a brief inline comment near the OpenRouter/models block
stating that provider URL overrides defaults (OpenAI -> gpt-5.4-nano, OpenRouter
-> mistralai/mistral-nemo) so the behavior is documented until a
conditional-default schema (e.g., default_for with provider) is implemented.
- Around line 19-59: The services map defines graphrag but no model lists
include graphrag, so templates like {{% llm-models "graphrag" %}} return
nothing; either add "graphrag" to the relevant providers' models' services
arrays (e.g., include graphrag in the services for appropriate chat and
embedding models such as gpt-5.4-nano, mistralai/mistral-nemo,
mistral-nemo-instruct, nomic-embed-text-v1) to associate those models with
GraphRAG, or remove the graphrag key from the top-level services section if
GraphRAG is not a selectable service; update llm_models.yaml accordingly so
graphrag is consistently present only where intended.
---
Nitpick comments:
In `@site/themes/arangodb-docs-theme/layouts/shortcodes/llm-models.md`:
- Line 19: The table row on line 19 is hard to read due to nested inline
conditionals; extract the logic into template variables like $typeLabel (compute
from .type with eq checks), $defaultIndicator (check $service against
.default_for using in or default slice), and $serviceNames (map .services via
$svcMap and delimit) then replace the long inline expression with those
variables in the row; reference the fields .name, .type, .default_for, .services
and the map $svcMap when creating these variables so the table cell simply uses
$typeLabel, $defaultIndicator, and $serviceNames.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0c4af65-31fd-40bf-962e-e4a3960bf4e5
📒 Files selected for processing (4)
site/content/agentic-ai-suite/importer/llm-configuration.mdsite/content/agentic-ai-suite/supported-llm-models.mdsite/data/llm_models.yamlsite/themes/arangodb-docs-theme/layouts/shortcodes/llm-models.md
|
Actionable comments posted: 0 |
|
Actionable comments posted: 0 |
bluepal-satya-lenka
left a comment
bluepal-satya-lenka
left a comment
There was a problem hiding this comment.
Thanks, @nicos-arango, for your work. I've left a few comments for your consideration. Please take a look before merging.
|
Our recommendation is OpenAI as the provider, with the OpenAI model list. Nothing else should be listed as a provider of its own. Anthropic, Google Gemini and OpenRouter are not separate providers in any of the three services. They are all custom endpoints that a user sets up themselves, using the OpenAI client with a different |
|
@pratham-ara , thank you for the feedback, I amended the PR |
| - `chat_model`: Specific language model to use for text generation and analysis. | ||
| - **OpenAI**: Defaults to `gpt-5.4-nano`. | ||
| - **OpenRouter**: Defaults to `mistralai/mistral-nemo`. | ||
| - **Other OpenAI-compatible APIs**: Defaults to `gpt-5.4-nano`. |
There was a problem hiding this comment.
Removing the OpenRouter default is right, but this line is inaccurate on two counts.
The Retriever code still has the special case: ConfigHelper.get_chat_model returns mistralai/mistral-nemo whenever chat_api_url contains openrouter.ai.
And a custom endpoint is not expected to fall back to an OpenAI model name at all. The Importer already has no default for custom, the caller has to supply chat_model. The Model Defaults list at line 62 carries the same claim.
| ### {{ .name }}{{ with .api }} ({{ . }}){{ end }} | ||
|
|
||
| {{ end -}} | ||
| | Model | Type | Default | |
There was a problem hiding this comment.
This column renders each model's supported services, not its default status, so every row on every page reads "Importer, AutoGraph, Retriever" and the header does not match the content. This is the empty-defaults point from the June review, still open.
The field doc at llm_models.yaml line 27 describes services as "the service ids the model is a recommended default for", but it is the support list.
The actual defaults, gpt-5.4-nano for chat and text-embedding-3-small for embeddings, are not marked anywhere in the table.
|
|
||
| A model is listed as supported by the suite only if it works seamlessly across | ||
| the Importer, Retriever, and AutoGraph services. Individual services may also | ||
| work with additional models — for the full list available to a specific |
There was a problem hiding this comment.
Every model in the data file is tagged for all three core services, so the suite table and the three per-service tables render identically. This sentence sends the reader to a copy of the same table.
The Importer page previously listed gpt-5.5, gpt-5.4 and its variants, gpt-5.2, gpt-5.1, gpt-4.1, gpt-4.1-mini, gpt-4o and gpt-4o-mini. Those are gone with nothing replacing them, so the docs now claim a narrower set of supported models than before.
| your own environment. For the models served through Triton, see | ||
| [Using Triton Inference Server](#using-triton-inference-server). | ||
|
|
||
| {{% llm-models "autograph" %}} |
There was a problem hiding this comment.
The Chat payload compatibility section further down this page still says to set chat_model to "for example gpt-5.4-nano, gpt-5.4-mini, gpt-4.1, gpt-4o". gpt-4.1 and gpt-4o are not in the table above, so the page contradicts itself. The Retriever page has the same problem and additionally names gpt-5.4 and gpt-5.1.
|
Thanks @pratham-ara , I addressed the comments. Could you please verify these points?
|
|
1. No. That is not supposed to work, and we will be fixing it. Use 2. Yes, in all three. The key check runs on the resolved client family, which includes 3. No, AutoGraph does default. Under 4. Not sure. @maxkernbach can you confirm whether NL2AQL is getting |
|
Thanks @pratham-ara , addressed the comments as discussed on the call today |
4 participants
Description
Upstream PRs
Summary by CodeRabbit
New Features
Documentation