Ollama support - #16
Open
Blaumaus wants to merge 5 commits into
Open
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds Ollama/OpenWebUI as a supported provider with endpoint normalization/validation, stores optional per-provider baseUrl in the DB, updates web settings/UI to configure local endpoints, expands worker model discovery/creation to use Ollama endpoints, and updates docs, tests, and compose infra. ChangesOllama Provider Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/components/ChatInput.tsx (1)
672-681:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse Ollama-specific copy in the disconnected state.
When the Ollama tab is selected and not connected, this branch still tells the user to “Connect your API key”. That contradicts the new endpoint-based flow and sends them toward a credential they may not need.
Proposed fix
<div> <Text as="p" size="sm" weight="medium" className="mb-1"> {PROVIDER_NAMES[activeTab]} </Text> <Text as="p" size="sm" colour="muted"> - Connect your API key to use these models + {activeTab === 'ollama' + ? 'Connect your Ollama endpoint to use these models' + : 'Connect your API key to use these models'} </Text> </div>🤖 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 `@apps/web/app/components/ChatInput.tsx` around lines 672 - 681, The disconnected-state copy currently always shows "Connect your API key" for any provider; update the UI branch that renders when activeTab is not 'favorites' and not in connectedProviders to show Ollama-specific guidance when activeTab === 'ollama' instead of the API-key text: inside the block that renders ProviderLogo and the two Text elements, conditionally render PROVIDER_NAMES[activeTab] as before but change the second Text to show endpoint/configuration instructions for Ollama (e.g., "Configure an Ollama endpoint to use local/remote models") when activeTab === 'ollama', otherwise keep the existing "Connect your API key" copy so other providers are unchanged.
🧹 Nitpick comments (2)
apps/docs/content/docs/selfhosting/configuring.mdx (1)
166-166: ⚡ Quick winClarify OpenWebUI URL for Docker deployments.
The OpenWebUI example uses
http://localhost:3000/api, but when Chathouse runs in Docker and OpenWebUI runs on the host, users should usehttp://host.docker.internal:3000/apiinstead. Consider adding this clarification to avoid confusion.📝 Suggested clarification
-For Ollama, the base URL must be reachable from the Chathouse server and worker. Use `http://localhost:11434/v1` when Ollama runs beside the app, `http://host.docker.internal:11434/v1` when Chathouse runs in Docker and Ollama runs on the host, or `http://localhost:3000/api` for an OpenWebUI gateway. +For Ollama, the base URL must be reachable from the Chathouse server and worker. Use `http://localhost:11434/v1` when Ollama runs beside the app, `http://host.docker.internal:11434/v1` when Chathouse runs in Docker and Ollama runs on the host, or `http://localhost:3000/api` (or `http://host.docker.internal:3000/api` if Chathouse is in Docker and OpenWebUI is on the host) for an OpenWebUI gateway.🤖 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 `@apps/docs/content/docs/selfhosting/configuring.mdx` at line 166, Update the Ollama/OpenWebUI URL line to clarify Docker-host networking: note that the OpenWebUI example `http://localhost:3000/api` is correct when both services run on the same host, but when Chathouse runs in Docker and OpenWebUI runs on the host the URL should be `http://host.docker.internal:3000/api`; modify the sentence that currently lists `http://localhost:3000/api` to include this Docker-specific alternative and a brief parenthetical explaining when to use each form.apps/docs/content/docs/usage/connections.mdx (1)
41-47: ⚡ Quick winClarify OpenWebUI URL for Docker deployments.
Similar to the configuration documentation, when Chathouse runs in Docker and OpenWebUI runs on the host,
http://localhost:3000/apiwon't work — users needhttp://host.docker.internal:3000/apiinstead.📝 Suggested clarification
For Ollama, enter the base URL that the Chathouse server or worker can reach: - **Local Ollama** — `http://localhost:11434/v1` - **Docker host Ollama** — `http://host.docker.internal:11434/v1` -- **OpenWebUI gateway** — `http://localhost:3000/api` +- **OpenWebUI gateway** — `http://localhost:3000/api` (or `http://host.docker.internal:3000/api` if Chathouse is in Docker and OpenWebUI is on the host)🤖 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 `@apps/docs/content/docs/usage/connections.mdx` around lines 41 - 47, Update the OpenWebUI gateway example line that currently reads "OpenWebUI gateway — `http://localhost:3000/api`" to clarify Docker host scenarios: add the alternative URL `http://host.docker.internal:3000/api` and a short note stating that when Chathouse runs in Docker and OpenWebUI runs on the host, users must use the host.docker.internal address instead of localhost; modify the sentence about pasting an OpenWebUI API key only if authentication is required to remain accurate.
🤖 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 `@apps/web/app/lib/ollama.server.ts`:
- Line 3: The two helper functions getOllamaBaseUrlCandidates and
parseOpenAICompatibleModelIds are exported but not used externally causing knip
CI failures; remove the export by making them module-private (remove the
"export" keyword) or, if they should be used elsewhere, import them from their
intended consumers instead; update any references within
apps/web/app/lib/ollama.server.ts to call the now non-exported functions
directly and run the linter/knip to confirm the unused-export error is resolved.
- Around line 58-61: The fetch call that queries `${baseUrl}/models` using
`trimmedApiKey` lacks a timeout and can hang; update the code around the fetch
(the block that assigns `const response = await fetch(...)`) to use an
AbortController: create a controller, pass `signal: controller.signal` in the
fetch options (merging with existing headers), start a setTimeout to call
`controller.abort()` after a short timeout (e.g., 5s), and clear the timeout
after the fetch completes; also ensure callers handle the aborted request
(AbortError) appropriately so validation fails fast on network stalls.
In `@apps/web/app/lib/providers.ts`:
- Around line 3-8: ALL_PROVIDERS is declared with `as const` (a readonly tuple)
but uses `satisfies Provider[]`, which conflicts with readonly → mutable type in
strict mode; update the type to `satisfies readonly Provider[]` to match the
readonly tuple, and apply the same fix to the analogous WORKER_PROVIDERS
declaration (replace `satisfies Provider[]` with `satisfies readonly
Provider[]`) so the tuple and its satisfied type are compatible.
In `@apps/web/app/routes/settings.connections.tsx`:
- Around line 116-143: The Ollama branch uses raw form values for validation but
only trims the apiKey when storing; trim both apiKey and baseUrl before calling
validateOpenAICompatibleModelEndpoint to ensure pasted tokens/URLs with
surrounding whitespace validate properly. Concretely, derive a trimmedApiKey =
(formData.get('apiKey') as string)?.trim() and trimmedBaseUrl =
((formData.get('baseUrl') as string) || OLLAMA_DEFAULT_BASE_URL).trim(), use
trimmedBaseUrl and trimmedApiKey when calling
validateOpenAICompatibleModelEndpoint and when building the upsert payload
(encryptedKey via encrypt(trimmedApiKey)), leaving db.apiKey.upsert,
validateOpenAICompatibleModelEndpoint, OLLAMA_DEFAULT_BASE_URL and encrypt as
the referenced symbols to update.
In `@apps/web/app/routes/settings.models.tsx`:
- Around line 383-386: The current initializer for groups uses
Object.fromEntries and force-casts to Record<Provider, typeof filteredModels>,
which causes TS2352; replace that cast with an explicit object literal typed as
Record<Provider, typeof filteredModels> that lists all provider keys (the four
keys from ALL_PROVIDERS) and initializes each to filteredModels (or an empty
array of the same type). Update the declaration using the symbol groups and
ensure the literal's type matches typeof filteredModels so the typechecker
accepts it instead of the Object.fromEntries cast.
In `@apps/worker/src/ollama.ts`:
- Around line 54-57: fetchOpenAICompatibleModelIds calls fetchImpl to GET
`${baseUrl}/models` without a timeout; wrap this request in an AbortController
and pass its signal into the fetchImpl options (alongside headers when
trimmedApiKey is present), start a timer (e.g., 5–10s) that calls
controller.abort() on expiry, and clear the timer when the fetch completes to
avoid leaks; update the call site where fetchImpl is invoked (referencing
fetchImpl, baseUrl, trimmedApiKey, and fetchOpenAICompatibleModelIds) so hung
endpoints are aborted and the job processor cannot stall.
In `@apps/worker/src/processors/chat.ts`:
- Around line 135-157: The callbacks in getOllamaTitleModels lose type inference
causing TS7006; explicitly type the results of the Prisma queries and the
intermediate arrays: annotate models as Array<{ modelId: string }> (the result
of db.cachedModel.findMany) and settings as Array<{ modelId: string; enabled:
boolean; favorite: boolean }> (the result of db.enabledModel.findMany), ensure
settingsMap is typed as Map<string, { modelId: string; enabled: boolean;
favorite: boolean }>, and add explicit parameter types for the arrow callbacks
used in .map(), .filter(), and .toSorted() (e.g., (model: {modelId: string}) and
(a: {modelId: string}, b: {modelId: string})) so the chain no longer produces
implicit any errors in getOllamaTitleModels.
In `@apps/worker/src/processors/refresh.ts`:
- Around line 102-104: The refresh logic in the 'ollama' branch throws when
baseUrl is missing, preventing use of the same default endpoint used by
createLanguageModelForProvider; update the 'ollama' case to fall back to
OLLAMA_DEFAULT_BASE_URL when baseUrl is falsy (instead of throwing) and then
call fetchOpenAICompatibleModelIds with that fallback (preserve apiKey
handling), ensuring cachedModel can populate the same way as
createLanguageModelForProvider.
- Around line 74-83: The catch block around decrypt(encryptedKey) currently does
a bare continue which skips refreshing the entire provider; change it to only
skip for providers that absolutely require a valid API key and allow execution
to proceed for 'ollama' by not continuing — e.g., in the catch, log the error as
before but if provider === 'ollama' clear/leave apiKey undefined (so the
endpoint can be used without auth) and do not continue, otherwise continue for
providers that need the key; update references to encryptedKey, decrypt,
provider, userId and apiKey in the refresh logic to implement this conditional
behavior.
In `@apps/worker/src/utils.ts`:
- Around line 406-410: The barrel export currently re-exports
resolveProviderForModelId even though it is unused and flagged by Knip; remove
resolveProviderForModelId from the export list so only formatModelName,
isGoogleChatModelId, and isOpenAIModelId are exported, and if any callers need
resolveProviderForModelId update them to import it directly from
./model-utils.js (or delete those imports if unused).
- Around line 78-81: getProviderConnection is exported but not used outside this
module, causing a lint/CI failure; remove the public export by making the
function internal (drop the "export" on getProviderConnection) in
apps/worker/src/utils.ts and update any local references if necessary so the
function is still callable within the module; ensure the function signature and
return type stay the same but is not exported from the module.
- Around line 89-94: The current try/catch around
decrypt(connection.encryptedKey) returns null on any decryption failure, which
incorrectly marks providers like Ollama (where apiKey is optional) as
unconfigured; update the logic in the function that sets apiKey (look for
connection.encryptedKey, decrypt(), and the apiKey variable) so that a
decryption error does not return null unconditionally: catch the error, log or
warn about the failed decrypt, set apiKey to undefined (or leave it unset), and
only return null when the provider actually requires an API key (i.e., perform
the provider-specific check after attempting decryption and only return null for
providers that mandate apiKey).
---
Outside diff comments:
In `@apps/web/app/components/ChatInput.tsx`:
- Around line 672-681: The disconnected-state copy currently always shows
"Connect your API key" for any provider; update the UI branch that renders when
activeTab is not 'favorites' and not in connectedProviders to show
Ollama-specific guidance when activeTab === 'ollama' instead of the API-key
text: inside the block that renders ProviderLogo and the two Text elements,
conditionally render PROVIDER_NAMES[activeTab] as before but change the second
Text to show endpoint/configuration instructions for Ollama (e.g., "Configure an
Ollama endpoint to use local/remote models") when activeTab === 'ollama',
otherwise keep the existing "Connect your API key" copy so other providers are
unchanged.
---
Nitpick comments:
In `@apps/docs/content/docs/selfhosting/configuring.mdx`:
- Line 166: Update the Ollama/OpenWebUI URL line to clarify Docker-host
networking: note that the OpenWebUI example `http://localhost:3000/api` is
correct when both services run on the same host, but when Chathouse runs in
Docker and OpenWebUI runs on the host the URL should be
`http://host.docker.internal:3000/api`; modify the sentence that currently lists
`http://localhost:3000/api` to include this Docker-specific alternative and a
brief parenthetical explaining when to use each form.
In `@apps/docs/content/docs/usage/connections.mdx`:
- Around line 41-47: Update the OpenWebUI gateway example line that currently
reads "OpenWebUI gateway — `http://localhost:3000/api`" to clarify Docker host
scenarios: add the alternative URL `http://host.docker.internal:3000/api` and a
short note stating that when Chathouse runs in Docker and OpenWebUI runs on the
host, users must use the host.docker.internal address instead of localhost;
modify the sentence about pasting an OpenWebUI API key only if authentication is
required to remain accurate.
🪄 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 Plus
Run ID: f21ce64a-96a0-49ef-a418-7a2c791fd397
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (25)
README.mdapps/docs/content/docs/selfhosting/configuring.mdxapps/docs/content/docs/usage/connections.mdxapps/docs/content/docs/usage/conversations.mdxapps/docs/content/docs/usage/models.mdxapps/web/app/components/ChatInput.tsxapps/web/app/lib/ollama.server.tsapps/web/app/lib/providers.tsapps/web/app/routes/settings.about.tsxapps/web/app/routes/settings.connections.tsxapps/web/app/routes/settings.models.tsxapps/web/app/ui/Badge.tsxapps/web/app/ui/ProviderLogo.tsxapps/worker/package.jsonapps/worker/src/model-utils.tsapps/worker/src/ollama.tsapps/worker/src/processors/chat.tsapps/worker/src/processors/refresh.tsapps/worker/src/utils.tsapps/worker/test/ollama.test.tscompose-cloud.yamlpackages/database/prisma/migrations/20260520120000_add_ollama_connection/migration.sqlpackages/database/prisma/schema.prismapackages/database/src/index.tspackages/database/src/types.ts
There was a problem hiding this comment.
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 (2)
apps/web/app/lib/ollama.server.ts (1)
5-32:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftRestrict Ollama endpoints to a trusted allowlist.
This accepts any user-supplied
http(s)host and fetches it from the server, which gives authenticated users an SSRF primitive against internal services or metadata endpoints. Since the validated URL is then persisted and reused by refresh jobs, this needs an admin-controlled allowlist or an explicit unsafe opt-in instead of accepting arbitrary hosts by default.Also applies to: 57-84
🤖 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 `@apps/web/app/lib/ollama.server.ts` around lines 5 - 32, The function getOllamaBaseUrlCandidates currently accepts arbitrary http(s) hosts creating an SSRF risk; replace this behavior with an admin-controlled allowlist check (or explicit unsafe opt-in flag) before accepting the parsed URL: in getOllamaBaseUrlCandidates validate url.origin against a configured TRUSTED_OLLAMA_HOSTS (or a feature flag like ALLOW_UNSAFE_OLLAMA_HOSTS) and throw a clear error if the origin is not allowed; apply the same allowlist/opt-in enforcement to the analogous code block referenced at lines 57-84 (the other Ollama URL validation/normalization) so only origins in the allowlist (or when the explicit unsafe opt-in is enabled) are returned/ persisted.apps/web/app/routes/settings.models.tsx (1)
497-501:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHonor the selected provider filter when rendering sections.
After
filteredModelsis narrowed byfilterProvider, this loop still renders every connected provider. Selecting one provider leaves the other connected providers on screen with empty states, so the filter appears broken.Suggested fix
{ALL_PROVIDERS.map((provider) => { + if (filterProvider !== 'all' && provider !== filterProvider) return null + const models = groupedModels[provider] const isConnected = connectedProviders.includes(provider) if (models.length === 0 && !isConnected) return null🤖 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 `@apps/web/app/routes/settings.models.tsx` around lines 497 - 501, The rendering loop currently iterates ALL_PROVIDERS and shows connected providers even when filterProvider is set; update the map to iterate only providers present in filteredModels (or pre-filter ALL_PROVIDERS by filterProvider) so sections honor the selected provider filter: replace ALL_PROVIDERS.map(...) with Object.keys(groupedModels) or ALL_PROVIDERS.filter(p => !filterProvider || p === filterProvider).map(...) and keep the existing checks using groupedModels, connectedProviders, and models to avoid rendering unrelated provider sections.
🤖 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 `@apps/worker/src/utils.ts`:
- Around line 96-100: The decrypt-failure log currently includes a raw userId
(logger.error(... ${userId} ...)), which must be removed or redacted; update the
logger.error call in the decrypt failure path to omit the raw userId and instead
log only non-identifying context such as the provider or a redacted/id-hash
(e.g., "[REDACTED]" or a short hash of userId), preserving the existing message
about SECRET_KEY_BASE; keep the existing control flow (the provider check and
return null when provider !== 'ollama') unchanged and only change the contents
of the logger.error invocation.
---
Outside diff comments:
In `@apps/web/app/lib/ollama.server.ts`:
- Around line 5-32: The function getOllamaBaseUrlCandidates currently accepts
arbitrary http(s) hosts creating an SSRF risk; replace this behavior with an
admin-controlled allowlist check (or explicit unsafe opt-in flag) before
accepting the parsed URL: in getOllamaBaseUrlCandidates validate url.origin
against a configured TRUSTED_OLLAMA_HOSTS (or a feature flag like
ALLOW_UNSAFE_OLLAMA_HOSTS) and throw a clear error if the origin is not allowed;
apply the same allowlist/opt-in enforcement to the analogous code block
referenced at lines 57-84 (the other Ollama URL validation/normalization) so
only origins in the allowlist (or when the explicit unsafe opt-in is enabled)
are returned/ persisted.
In `@apps/web/app/routes/settings.models.tsx`:
- Around line 497-501: The rendering loop currently iterates ALL_PROVIDERS and
shows connected providers even when filterProvider is set; update the map to
iterate only providers present in filteredModels (or pre-filter ALL_PROVIDERS by
filterProvider) so sections honor the selected provider filter: replace
ALL_PROVIDERS.map(...) with Object.keys(groupedModels) or ALL_PROVIDERS.filter(p
=> !filterProvider || p === filterProvider).map(...) and keep the existing
checks using groupedModels, connectedProviders, and models to avoid rendering
unrelated provider sections.
🪄 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 Plus
Run ID: 4ac5d2c9-efb5-4917-81ff-a08f67afd700
📒 Files selected for processing (13)
apps/docs/content/docs/selfhosting/configuring.mdxapps/docs/content/docs/usage/connections.mdxapps/web/app/components/ChatInput.tsxapps/web/app/lib/ollama.server.tsapps/web/app/lib/providers.tsapps/web/app/routes/settings.connections.tsxapps/web/app/routes/settings.models.tsxapps/worker/src/model-utils.tsapps/worker/src/ollama.tsapps/worker/src/processors/chat.tsapps/worker/src/processors/refresh.tsapps/worker/src/utils.tsapps/worker/tsconfig.json
✅ Files skipped from review due to trivial changes (3)
- apps/worker/tsconfig.json
- apps/docs/content/docs/usage/connections.mdx
- apps/docs/content/docs/selfhosting/configuring.mdx
Contributor
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/app/lib/ollama.server.ts (1)
71-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject non-model payloads during endpoint validation.
Lines 71-83 return
[]for any non-{ data: [...] }body, and Line 108 treats that the same as a valid empty model list. That means any 200 response with the wrong payload shape can pass validation and save a broken endpoint with zero models.Proposed fix
-function parseOpenAICompatibleModelIds(value: unknown): string[] { +function parseOpenAICompatibleModelIds(value: unknown): string[] | null { if (!value || typeof value !== 'object' || !Array.isArray((value as { data?: unknown }).data)) { - return [] + return null } const ids = new Set<string>() for (const item of (value as { data: unknown[] }).data) { if (!item || typeof item !== 'object') continue const id = (item as { id?: unknown }).id if (typeof id === 'string' && id.trim()) ids.add(id.trim()) } return [...ids] }- return { baseUrl, modelIds: parseOpenAICompatibleModelIds(await response.json()) } + const modelIds = parseOpenAICompatibleModelIds(await response.json()) + if (!modelIds) { + errors.push(`${baseUrl} returned a non-OpenAI-compatible model payload`) + continue + } + + return { baseUrl, modelIds }Also applies to: 108-108
🤖 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 `@apps/web/app/lib/ollama.server.ts` around lines 71 - 83, The helper parseOpenAICompatibleModelIds currently returns an empty array for any non-{data: [...] } payload, which lets malformed 200 responses be treated as valid empty model lists; change parseOpenAICompatibleModelIds to return null (or throw) when the shape isn't the expected object with a data array (i.e. replace the initial guard to return null), and update every caller that currently treats an empty array as "valid" (the call site that validates endpoint model lists) to treat null as a validation failure and reject/save the endpoint as invalid instead of accepting zero models.apps/web/app/routes/settings.models.tsx (1)
552-560:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid Ollama setup guidance when filters caused the empty state.
This branch runs after search/provider/favorites filtering, so users with existing Ollama models can still see “Pull a model in Ollama” just by applying a non-matching filter. Only show this hint when the unfiltered Ollama list is empty; otherwise use a generic “adjust your search or filters” message.
🤖 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.
Outside diff comments:
In `@apps/web/app/lib/ollama.server.ts`:
- Around line 71-83: The helper parseOpenAICompatibleModelIds currently returns
an empty array for any non-{data: [...] } payload, which lets malformed 200
responses be treated as valid empty model lists; change
parseOpenAICompatibleModelIds to return null (or throw) when the shape isn't the
expected object with a data array (i.e. replace the initial guard to return
null), and update every caller that currently treats an empty array as "valid"
(the call site that validates endpoint model lists) to treat null as a
validation failure and reject/save the endpoint as invalid instead of accepting
zero models.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 568f3141-8091-4b2f-b163-54b2859c0494
📒 Files selected for processing (3)
apps/web/app/lib/ollama.server.tsapps/web/app/routes/settings.models.tsxapps/worker/src/utils.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
UI Improvements
Models
Database
Tests
Infrastructure