align documentation with the codebase for v0.0.28#172
Conversation
WalkthroughDocs and examples updated project-wide: proxy defaults set to Olla, load-balancer to least-connections, discovery examples use discovery.static.endpoints, Anthropic passthrough/backends expanded, response headers and internal system endpoint schemas revised, and many integration/example README and CI path filters adjusted. ChangesDocumentation and configuration refresh
Estimated code review effort: Possibly related PRs:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
…nspector defaults)
71566c1 to
f088269
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/content/concepts/model-unification.md (1)
10-25:⚠️ Potential issue | 🔴 CriticalFix model-unification docs: YAML example doesn’t match
OLLA_MODEL_UNIFIER_*environment variablesIn
docs/content/concepts/model-unification.md(lines 10–16), the “Default Configuration” block showsdiscovery.model_discovery(enabled/interval/concurrent_workers), but the listed environment variables are for model unification:
OLLA_MODEL_UNIFIER_ENABLED→model_registry.enable_unifier(and setsmodel_registry.unification.enabled)OLLA_MODEL_UNIFIER_CACHE_TTL→model_registry.unification.cache_ttl(duration; default is10m)Update the example block or the environment variable section so both reference the same config subtree.
🤖 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 `@docs/content/concepts/model-unification.md` around lines 10 - 25, The YAML example currently shows the discovery.model_discovery subtree (keys: discovery.model_discovery.enabled/interval/concurrent_workers) but the env var list references OLLA_MODEL_UNIFIER_* which map to model_registry.enable_unifier and model_registry.unification.* (specifically model_registry.unification.enabled and model_registry.unification.cache_ttl, default 10m); fix by making both parts consistent—either change the YAML block to show the model_registry.unification subtree (e.g., model_registry.enable_unifier and model_registry.unification.cache_ttl) or replace the two env vars with the correct discovery-related env vars, and ensure OLLA_MODEL_UNIFIER_ENABLED and OLLA_MODEL_UNIFIER_CACHE_TTL are documented next to model_registry.enable_unifier / model_registry.unification.cache_ttl.docs/content/configuration/examples.md (1)
1047-1056:⚠️ Potential issue | 🟠 MajorClarify OLLA_LOG_LEVEL vs OLLA_LOGGING_LEVEL
OLLA_LOG_LEVELis read when the logger is created inmain.go(beforeconfig.Load) and drives theslog.Defaultlogger, so it controls the actual emitted log level.OLLA_LOGGING_LEVELonly overridesconfig.Logging.Levelduring env parsing, but that config value isn’t used when initialising the runtime logger, so it doesn’t override YAMLlogging.leveloutput.
Update the docs’ inline comments/snippet to reflect this (point users toOLLA_LOG_LEVELfor changing runtime log output).🤖 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 `@docs/content/configuration/examples.md` around lines 1047 - 1056, Update the example and inline comment to accurately distinguish OLLA_LOG_LEVEL vs OLLA_LOGGING_LEVEL: state that OLLA_LOG_LEVEL is read in main.go when the logger is created (before config.Load) and controls slog.Default and the actual emitted runtime log level, whereas OLLA_LOGGING_LEVEL only sets config.Logging.Level during env parsing and does not affect the initial runtime logger; point users to use OLLA_LOG_LEVEL to change runtime log output and keep OLLA_LOGGING_LEVEL note limited to config overrides.
🧹 Nitpick comments (8)
docs/content/api-reference/ollama.md (1)
291-296: ⚡ Quick winInconsistent formatting in unsupported endpoints list.
The bullet points have inconsistent formatting. Line 292 includes the HTTP method prefix "POST /olla/ollama/api/pull", whilst lines 293–296 omit the method and use just the path component. For consistency, either include the method for all entries or omit it for all.
📝 Suggested fix for consistent formatting
-- **POST /olla/ollama/api/pull** - Download a model from the Ollama registry -- **POST /olla/ollama/api/push** - Upload a model to the Ollama registry +- **POST /olla/ollama/api/pull** - Download a model from the Ollama registry +- **POST /olla/ollama/api/push** - Upload a model to the Ollama registry - **POST /olla/ollama/api/copy** - Create a copy of a model - **DELETE /olla/ollama/api/delete** - Remove a model - **POST /olla/ollama/api/create** - Create a model from a Modelfile🤖 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 `@docs/content/api-reference/ollama.md` around lines 291 - 296, The list of unsupported endpoints is formatted inconsistently—only "POST /olla/ollama/api/pull" includes the HTTP method; update the other bullets to include their HTTP methods to match (e.g., change "/olla/ollama/api/push", "/copy", "/create" to "POST /olla/ollama/api/push", "POST /olla/ollama/api/copy", "POST /olla/ollama/api/create", change "/olla/ollama/api/delete" to "DELETE /olla/ollama/api/delete", and change "/olla/ollama/api/list" to "GET /olla/ollama/api/list") so all entries use the same "METHOD /olla/ollama/api/..." format.docs/content/concepts/load-balancing.md (1)
6-14: Correctness: runtime defaultproxy.load_balancerisleast-connections(docs match code)
internal/config/config.gosetsDefaultLoadBalancer = "least-connections"andDefaultConfig().Proxy.LoadBalancerto that value;OLLA_PROXY_LOAD_BALANCERonly overrides when explicitly set.internal/app/config.godefinesDefaultLoadBalancer = "priority", but it has no usages elsewhere, so it doesn’t affect the effective default.🤖 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 `@docs/content/concepts/load-balancing.md` around lines 6 - 14, Docs are correct but there's a confusing, unused default in internal/app/config.go; update the code to avoid confusion by making the runtime defaults consistent: change the symbol DefaultLoadBalancer in internal/app/config.go from "priority" to "least-connections" (or remove that unused constant) so it matches internal/config/config.go's DefaultLoadBalancer and DefaultConfig().Proxy.LoadBalancer; also ensure behavior remains that OLLA_PROXY_LOAD_BALANCER only overrides when explicitly set.docs/content/configuration/overview.md (2)
342-342: 💤 Low valueConsider adding comma for readability.
LanguageTool suggests adding a comma after "Currently" for improved readability: "Currently, only the Anthropic translator is supported".
🤖 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 `@docs/content/configuration/overview.md` at line 342, The sentence in the docs describing the translators section needs a comma after the introductory adverb for readability; update the line "Currently only the Anthropic translator is supported; it allows Claude-compatible clients to reach OpenAI-compatible backends." to "Currently, only the Anthropic translator is supported; it allows Claude-compatible clients to reach OpenAI-compatible backends." so the introductory phrase "Currently" is followed by a comma.
352-352: 💤 Low valueMinor spelling: "subsection" is typically one word.
The phrase "inspector sub-section" is flagged by LanguageTool. Consider using "inspector subsection" as a single word, which is the more common spelling in technical documentation.
🤖 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 `@docs/content/configuration/overview.md` at line 352, Update the phrasing in the sentence that reads "See [Configuration Reference](reference.md#translators-configuration) for the full field reference, including the inspector sub-section" by replacing "inspector sub-section" with "inspector subsection" so the document uses the single-word form; locate this text in overview.md and apply the replacement to that sentence.docs/content/getting-started/installation.md (2)
97-97: 💤 Low valueConsider rephrasing for clarity.
The phrase "Print version, build commit, and Go runtime, then exit" could be slightly clearer as "Print version, build commit, and Go runtime details, then exit" or "Print version, build commit hash, and Go runtime, then exit".
🤖 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 `@docs/content/getting-started/installation.md` at line 97, Rephrase the `--version` description to be clearer: update the text for the `--version` flag (and its equivalent `OLLA_SHOW_VERSION=true`) to say something like "Print version, build commit hash, and Go runtime details, then exit" so it explicitly conveys that commit hash and runtime details are printed.
101-101: 💤 Low valueMinor grammar improvement.
The phrase "Running
ollawithout arguments uses the YAML config" is grammatically valid but slightly awkward. Consider "Runningollawithout arguments will use the YAML config" for improved flow.🤖 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 `@docs/content/getting-started/installation.md` at line 101, Reword the sentence for smoother grammar: change "Running `olla` without arguments uses the YAML config from `-c`/`--config` if set, then `OLLA_CONFIG_FILE`, otherwise the built-in defaults." to "Running `olla` without arguments will use the YAML config from `-c`/`--config` if set, then `OLLA_CONFIG_FILE`, otherwise the built-in defaults." and keep the surrounding mention of `-h` / `--help` and the flag names (`-c`/`--config`, `OLLA_CONFIG_FILE`) unchanged.docs/content/integrations/frontend/crush-cli.md (1)
197-197: 💤 Low valueConsider clarifying the engine default more explicitly.
The comment "default; or: sherpa (simpler codebase, maintenance mode)" is helpful, but it might be clearer to state explicitly that
ollais now the recommended engine for production use, whilstsherpais provided for backwards compatibility and simpler deployments.📝 Suggested clarification
- engine: olla # default; or: sherpa (simpler codebase, maintenance mode) + engine: olla # recommended for production; sherpa available for backwards compatibility🤖 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 `@docs/content/integrations/frontend/crush-cli.md` at line 197, Update the inline comment next to "engine: olla" to explicitly state that "olla" is the recommended production engine and "sherpa" exists only for backward-compatibility and simpler/maintenance-mode deployments; modify the text around the "engine: olla" and the "sherpa" mention so it reads something like: "engine: olla # recommended for production; use sherpa only for backwards-compatibility or simpler/maintenance-mode deployments" so readers understand which engine to choose.docs/content/usage.md (1)
167-167: Update LM Studio endpointtypeformat (lm-studio) in docs
lm-studiois the canonical LM Studio profile/backend type in the code (ProfileLmStudio/ProviderTypeLMStudio). Olla also acceptslmstudio/lm_studiovia routing-prefix aliases, so endpoint discovery won’t fail just because a snippet usestype: lmstudio.
docs/content/usage.mdis already consistent at lines 167 and 175-176 (type: "lm-studio"), but there are still a few doc snippets usingtype: lmstudio(e.g.,docs/content/integrations/frontend/opencode.md,crush-cli.md,claude-code.md); standardise them tolm-studiofor consistency.🤖 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 `@docs/content/usage.md` at line 167, Several documentation snippets still use the non-canonical endpoint type values (e.g., type: lmstudio or type: lm_studio); update all occurrences to the canonical "lm-studio" string so docs match the code's ProfileLmStudio / ProviderTypeLMStudio naming and routing-alias behavior. Search the repo for "type: lmstudio" and "type: lm_studio" (noting examples in docs/content/integrations/frontend/opencode.md, crush-cli.md, claude-code.md) and replace each with type: "lm-studio", leaving surrounding formatting intact and keeping quotation style consistent with nearby examples.
🤖 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 `@docs/content/api-reference/anthropic.md`:
- Around line 672-677: The docs and example configs use the legacy
security.rate_limit block; update them to the actual config schema by replacing
legacy keys with server.rate_limits and the matching field names
(global_requests_per_minute, per_ip_requests_per_minute, burst_size) so they map
to ServerConfig.RateLimits / ServerRateLimits in Go; specifically, change any
occurrences of security: rate_limit: ... in example manifests (e.g., the example
Olla YAMLs) and the docs/content/api-reference/anthropic.md snippets to use
server.rate_limits with the same YAML tags and ensure examples validate against
the current config structs.
In `@docs/content/development/benchmarking.md`:
- Around line 121-126: The documented benchmark names (BenchmarkFactory,
BenchmarkPriority, BenchmarkRoundRobin, BenchmarkLeastConnections) don't match
actual benchmark functions in the balancer package; either update the docs to
use the real benchmark function names or add/rename benchmarks to match the
docs. Locate the benchmark functions in the internal/adapter/balancer package
(functions matching func BenchmarkXxx(*testing.B)) and then either change the
markdown list to the actual function names found, or implement new benchmark
functions named BenchmarkFactory, BenchmarkPriority, BenchmarkRoundRobin, and
BenchmarkLeastConnections (or export wrappers with those names) so the go test
-bench pattern in the docs matches reality.
In `@docs/content/development/circuit-breaker.md`:
- Around line 121-123: Docs claim "one request is allowed through as a test" but
implementation differs: the health-checker enforces a single probe
(internal/adapter/health/circuit_breaker.go — IsOpen() uses lastAttempt CAS and
RecordSuccess() sets isOpen=0) while the proxy half-open logic
(internal/adapter/proxy/olla/service.go — RecordSuccess(), cb.state,
cb.failures) does not prevent concurrent requests once state==2 and closes on
the first success. Update the circuit-breaker docs to state that the circuit
closes after 1 successful probe but clarify that only the health-checker
enforces a single in-flight probe; the proxy implementation may allow concurrent
probes during half-open and still closes on the first success.
- Around line 115-116: Update the docs to clearly distinguish the proxy-layer
circuit breaker behavior from the health-checker circuit breaker: state that the
proxy CB (where cb.RecordFailure() is called on RoundTrip/stream/request errors
and cb.RecordSuccess() is called on any successful backend response regardless
of resp.StatusCode) ignores HTTP 5xx, while the health-check circuit breaker
treats non-healthy HTTP statuses as failures and can be tripped by 5xx
responses; reference the proxy RoundTrip/stream/request paths and the
health-check logic and keep the link to issue `#144`.
In `@docs/content/integrations/frontend/crush-cli.md`:
- Around line 730-731: Update the documentation list of valid Crush provider
"type" values: remove "gemini", "azure", and "vertexai" from the "Valid `type`
values" sentence and ensure it only lists `openai-compat`, `openai`, and
`anthropic`; keep a brief note (if present nearby) that Gemini/Azure/VertexAI
are configured via their dedicated integrations/env vars rather than via
providers.<name>.type to avoid misleading readers and runtime misconfiguration.
In `@docs/content/integrations/frontend/opencode.md`:
- Around line 107-120: The fenced ASCII-art diagrams (the block beginning with
"┌──────────────┐" / "OpenCode" and the second similar block later) are missing
language specifiers; update each opening triple-backtick to include the language
identifier text (i.e., change ``` to ```text) so the diagrams render correctly,
and apply the same change to the other ASCII block referenced (the block
containing "Ollama :11434" / "vLLM :8000").
- Around line 227-231: Update the opencode docs to match actual config structs:
state that rate limiting is configured via server.rate_limits (backed by
config.ServerConfig.RateLimits) rather than any security.* key; document
endpoint health fields as flat fields on EndpointConfig (health_check_url,
check_interval, check_timeout) and note that only discovery.health_check exists
for initial_delay-style settings (no per-endpoint health_check: block); correct
translators to show translators.anthropic as the only translator (OpenAI is the
native format, not translators.openai); and remove any mention of
proxy.write_timeout — instruct users to use server.write_timeout (use 0s to
disable server write timeout for streaming) since ProxyConfig has no
write_timeout.
In `@readme.md`:
- Around line 158-161: The endpoints table is missing the backward-compatible
OpenAI-specific route; update the README table to either add a row for
`/olla/openai/` (description: OpenAI-compatible routes, e.g. `GET
/olla/openai/v1/models` and other `/olla/openai/v1/*` endpoints) or insert a
clear compatibility note next to the table stating that `/olla/openai/` is still
supported and maps to OpenAI-compatible paths; update the same table entries
that list `/olla/{provider}/` and `/olla/proxy/` so the new `/olla/openai/` row
or note is consistent with the existing examples.
---
Outside diff comments:
In `@docs/content/concepts/model-unification.md`:
- Around line 10-25: The YAML example currently shows the
discovery.model_discovery subtree (keys:
discovery.model_discovery.enabled/interval/concurrent_workers) but the env var
list references OLLA_MODEL_UNIFIER_* which map to model_registry.enable_unifier
and model_registry.unification.* (specifically
model_registry.unification.enabled and model_registry.unification.cache_ttl,
default 10m); fix by making both parts consistent—either change the YAML block
to show the model_registry.unification subtree (e.g.,
model_registry.enable_unifier and model_registry.unification.cache_ttl) or
replace the two env vars with the correct discovery-related env vars, and ensure
OLLA_MODEL_UNIFIER_ENABLED and OLLA_MODEL_UNIFIER_CACHE_TTL are documented next
to model_registry.enable_unifier / model_registry.unification.cache_ttl.
In `@docs/content/configuration/examples.md`:
- Around line 1047-1056: Update the example and inline comment to accurately
distinguish OLLA_LOG_LEVEL vs OLLA_LOGGING_LEVEL: state that OLLA_LOG_LEVEL is
read in main.go when the logger is created (before config.Load) and controls
slog.Default and the actual emitted runtime log level, whereas
OLLA_LOGGING_LEVEL only sets config.Logging.Level during env parsing and does
not affect the initial runtime logger; point users to use OLLA_LOG_LEVEL to
change runtime log output and keep OLLA_LOGGING_LEVEL note limited to config
overrides.
---
Nitpick comments:
In `@docs/content/api-reference/ollama.md`:
- Around line 291-296: The list of unsupported endpoints is formatted
inconsistently—only "POST /olla/ollama/api/pull" includes the HTTP method;
update the other bullets to include their HTTP methods to match (e.g., change
"/olla/ollama/api/push", "/copy", "/create" to "POST /olla/ollama/api/push",
"POST /olla/ollama/api/copy", "POST /olla/ollama/api/create", change
"/olla/ollama/api/delete" to "DELETE /olla/ollama/api/delete", and change
"/olla/ollama/api/list" to "GET /olla/ollama/api/list") so all entries use the
same "METHOD /olla/ollama/api/..." format.
In `@docs/content/concepts/load-balancing.md`:
- Around line 6-14: Docs are correct but there's a confusing, unused default in
internal/app/config.go; update the code to avoid confusion by making the runtime
defaults consistent: change the symbol DefaultLoadBalancer in
internal/app/config.go from "priority" to "least-connections" (or remove that
unused constant) so it matches internal/config/config.go's DefaultLoadBalancer
and DefaultConfig().Proxy.LoadBalancer; also ensure behavior remains that
OLLA_PROXY_LOAD_BALANCER only overrides when explicitly set.
In `@docs/content/configuration/overview.md`:
- Line 342: The sentence in the docs describing the translators section needs a
comma after the introductory adverb for readability; update the line "Currently
only the Anthropic translator is supported; it allows Claude-compatible clients
to reach OpenAI-compatible backends." to "Currently, only the Anthropic
translator is supported; it allows Claude-compatible clients to reach
OpenAI-compatible backends." so the introductory phrase "Currently" is followed
by a comma.
- Line 352: Update the phrasing in the sentence that reads "See [Configuration
Reference](reference.md#translators-configuration) for the full field reference,
including the inspector sub-section" by replacing "inspector sub-section" with
"inspector subsection" so the document uses the single-word form; locate this
text in overview.md and apply the replacement to that sentence.
In `@docs/content/getting-started/installation.md`:
- Line 97: Rephrase the `--version` description to be clearer: update the text
for the `--version` flag (and its equivalent `OLLA_SHOW_VERSION=true`) to say
something like "Print version, build commit hash, and Go runtime details, then
exit" so it explicitly conveys that commit hash and runtime details are printed.
- Line 101: Reword the sentence for smoother grammar: change "Running `olla`
without arguments uses the YAML config from `-c`/`--config` if set, then
`OLLA_CONFIG_FILE`, otherwise the built-in defaults." to "Running `olla` without
arguments will use the YAML config from `-c`/`--config` if set, then
`OLLA_CONFIG_FILE`, otherwise the built-in defaults." and keep the surrounding
mention of `-h` / `--help` and the flag names (`-c`/`--config`,
`OLLA_CONFIG_FILE`) unchanged.
In `@docs/content/integrations/frontend/crush-cli.md`:
- Line 197: Update the inline comment next to "engine: olla" to explicitly state
that "olla" is the recommended production engine and "sherpa" exists only for
backward-compatibility and simpler/maintenance-mode deployments; modify the text
around the "engine: olla" and the "sherpa" mention so it reads something like:
"engine: olla # recommended for production; use sherpa only for
backwards-compatibility or simpler/maintenance-mode deployments" so readers
understand which engine to choose.
In `@docs/content/usage.md`:
- Line 167: Several documentation snippets still use the non-canonical endpoint
type values (e.g., type: lmstudio or type: lm_studio); update all occurrences to
the canonical "lm-studio" string so docs match the code's ProfileLmStudio /
ProviderTypeLMStudio naming and routing-alias behavior. Search the repo for
"type: lmstudio" and "type: lm_studio" (noting examples in
docs/content/integrations/frontend/opencode.md, crush-cli.md, claude-code.md)
and replace each with type: "lm-studio", leaving surrounding formatting intact
and keeping quotation style consistent with nearby examples.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 18cbeb92-3fd5-4c1b-8818-484a75646cf9
📒 Files selected for processing (65)
CLAUDE.mddocs/content/api-reference/anthropic.mddocs/content/api-reference/docker-model-runner.mddocs/content/api-reference/lemonade.mddocs/content/api-reference/llamacpp.mddocs/content/api-reference/lmstudio.mddocs/content/api-reference/models.mddocs/content/api-reference/ollama.mddocs/content/api-reference/omlx.mddocs/content/api-reference/openai.mddocs/content/api-reference/overview.mddocs/content/api-reference/sglang.mddocs/content/api-reference/system.mddocs/content/api-reference/vllm-mlx.mddocs/content/api-reference/vllm.mddocs/content/compare/gpustack.mddocs/content/compare/integration-patterns.mddocs/content/compare/litellm.mddocs/content/compare/localai.mddocs/content/compare/overview.mddocs/content/concepts/api-translation.mddocs/content/concepts/health-checking.mddocs/content/concepts/load-balancing.mddocs/content/concepts/model-routing.mddocs/content/concepts/model-unification.mddocs/content/concepts/overview.mddocs/content/concepts/profile-system.mddocs/content/concepts/proxy-engines.mddocs/content/concepts/proxy-profiles.mddocs/content/concepts/sticky-sessions.mddocs/content/configuration/examples.mddocs/content/configuration/filters.mddocs/content/configuration/overview.mddocs/content/configuration/practices/configuration.mddocs/content/configuration/practices/monitoring.mddocs/content/configuration/practices/performance.mddocs/content/configuration/practices/security.mddocs/content/configuration/reference.mddocs/content/demo.mddocs/content/development/architecture.mddocs/content/development/benchmarking.mddocs/content/development/circuit-breaker.mddocs/content/development/overview.mddocs/content/development/patterns.mddocs/content/development/setup.mddocs/content/development/testing.mddocs/content/faq.mddocs/content/getting-started/installation.mddocs/content/index.mddocs/content/integrations/api-translation/anthropic.mddocs/content/integrations/backend/docker-model-runner.mddocs/content/integrations/backend/lemonade.mddocs/content/integrations/backend/llamacpp.mddocs/content/integrations/backend/ollama.mddocs/content/integrations/backend/vllm-mlx.mddocs/content/integrations/frontend/claude-code.mddocs/content/integrations/frontend/crush-cli.mddocs/content/integrations/frontend/opencode.mddocs/content/integrations/frontend/openwebui-openai.mddocs/content/integrations/frontend/openwebui.mddocs/content/notes/anthropic-inspector.mddocs/content/notes/overview.mddocs/content/troubleshooting/path-preservation.mddocs/content/usage.mdreadme.md
| # Test all balancer strategies (or use make bench-balancer) | ||
| go test -bench="BenchmarkFactory|BenchmarkPriority|BenchmarkRoundRobin|BenchmarkLeastConnections" \ | ||
| -benchmem ./internal/adapter/balancer/ | ||
|
|
||
| # Via make | ||
| make bench-balancer |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify benchmark function names in the balancer package
# Search for benchmark functions matching the documented names
rg -n -A 2 'func Benchmark(Factory|Priority|RoundRobin|LeastConnections)\(' internal/adapter/balancer/Repository: thushan/olla
Length of output: 38
Fix benchmark names in docs (or update tests/code)
No functions matching the documented benchmark names (BenchmarkFactory, BenchmarkPriority, BenchmarkRoundRobin, BenchmarkLeastConnections) were found under internal/adapter/balancer/ with the expected func Benchmark...( pattern, so the referenced names likely don’t exist as written.
🤖 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 `@docs/content/development/benchmarking.md` around lines 121 - 126, The
documented benchmark names (BenchmarkFactory, BenchmarkPriority,
BenchmarkRoundRobin, BenchmarkLeastConnections) don't match actual benchmark
functions in the balancer package; either update the docs to use the real
benchmark function names or add/rename benchmarks to match the docs. Locate the
benchmark functions in the internal/adapter/balancer package (functions matching
func BenchmarkXxx(*testing.B)) and then either change the markdown list to the
actual function names found, or implement new benchmark functions named
BenchmarkFactory, BenchmarkPriority, BenchmarkRoundRobin, and
BenchmarkLeastConnections (or export wrappers with those names) so the go test
-bench pattern in the docs matches reality.
This reverts commit d1f8df5.
…l discovery example
…penai-compat) and valid olla config
…r) and valid olla config
…n env block; fix olla rate-limit keys
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
examples/opencode-lmstudio/README.md (1)
21-24: 💤 Low valueAdd language identifier to the code block.
The endpoint URL code block should specify a language identifier (use
textorhttpfor plain URLs) to comply with Markdown linting standards.📝 Suggested fix
-``` +```text http://localhost:40114/olla/openai/v1</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@examples/opencode-lmstudio/README.mdaround lines 21 - 24, The Markdown code
block containing the endpoint URL "http://localhost:40114/olla/openai/v1" lacks
a language identifier; update that fenced code block to include a language tag
such as text or http (e.g., changetotext) so the block complies with
Markdown linting standards and renders correctly.</details> <!-- cr-comment:v1:d5b3d5f4aee50a89e46f035f --> </blockquote></details> <details> <summary>examples/crush-vllm/README.md (1)</summary><blockquote> `7-15`: _💤 Low value_ **Add language identifier to the code block.** The architecture diagram code block should specify a language identifier (or use `text` if no syntax highlighting is needed) to comply with Markdown linting standards. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text ┌─────────────┐ ┌──────────┐ ┌─────────────────┐ │ Crush CLI │───▶│ Olla │───▶│ vLLM Instance │ │ │ │(Port │ │ (Port 8000) │ ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@examples/crush-vllm/README.mdaround lines 7 - 15, The ASCII architecture
diagram code block in the README is missing a Markdown language identifier;
update the fenced code block that contains the ASCII art (the "Crush CLI → Olla
→ vLLM Instance" diagram) to start with a language tag such as ```text so
linters recognize it as a text block and the markdown passes linting.</details> <!-- cr-comment:v1:d4e1a308a95c385514903b88 --> </blockquote></details> <details> <summary>examples/ollama-openwebui/README.md (1)</summary><blockquote> `9-24`: _💤 Low value_ **Add language identifier to the code block.** The architecture diagram code block should specify a language identifier (or use `text` for ASCII diagrams) to comply with Markdown linting standards. <details> <summary>📝 Suggested fix</summary> ```diff -``` +```text ┌─────────────┐ ┌──────────────┐ ┌─────────────────┐ │ Open WebUI │───▶│ Olla │───▶│ Ollama Instance │ │ (Port 3000) │ │ (Port 40114) │ │ (External) │ ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@examples/ollama-openwebui/README.mdaround lines 9 - 24, The fenced ASCII
diagram in README.md is missing a language identifier which breaks Markdown
linting; update the triple-backtick fence that wraps the diagram (the ASCII
block in examples/ollama-openwebui/README.md) to include a language identifier
such as text (e.g., changetotext) so the diagram is treated as plain
text and satisfies the linter.</details> <!-- cr-comment:v1:572757ea83b39576d4aad4de --> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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@examples/ollama-openwebui/compose.yaml:
- Line 40: Replace the insecure hardcoded default for WEBUI_SECRET_KEY with a
safer setup: add a clear comment next to WEBUI_SECRET_KEY warning users to
change it and either (A) make the compose entry a placeholder that requires the
operator to supply the value (e.g., read from environment/.env) or (B) add a
one-time bootstrap script or Make target that generates a
cryptographically-random secret and writes it to .env, then reference that
variable in the compose file; update any service that reads WEBUI_SECRET_KEY
accordingly so deployments never run with the literal "change-this-secret-key".In
@examples/opencode-lmstudio/test.sh:
- Line 81: The if-statement assigns ENDPOINTS but never uses it; change the
condition so it invokes curl to check endpoint availability without capturing
output: remove the ENDPOINTS= assignment in the if condition that currently
calls curl (the line referencing ENDPOINTS and curl to
"http://${OLLA_HOST}:${OLLA_PORT}/internal/status/endpoints") and instead run
curl with -s -f and redirect its output to /dev/null so the script only tests
curl's exit status.
Nitpick comments:
In@examples/crush-vllm/README.md:
- Around line 7-15: The ASCII architecture diagram code block in the README is
missing a Markdown language identifier; update the fenced code block that
contains the ASCII art (the "Crush CLI → Olla → vLLM Instance" diagram) to start
with a language tag such as ```text so linters recognize it as a text block and
the markdown passes linting.In
@examples/ollama-openwebui/README.md:
- Around line 9-24: The fenced ASCII diagram in README.md is missing a language
identifier which breaks Markdown linting; update the triple-backtick fence that
wraps the diagram (the ASCII block in examples/ollama-openwebui/README.md) to
include a language identifier such as text (e.g., changetotext) so the
diagram is treated as plain text and satisfies the linter.In
@examples/opencode-lmstudio/README.md:
- Around line 21-24: The Markdown code block containing the endpoint URL
"http://localhost:40114/olla/openai/v1" lacks a language identifier; update that
fenced code block to include a language tag such as text or http (e.g., change
totext) so the block complies with Markdown linting standards and
renders correctly.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Path: .coderabbit.yaml **Review profile**: CHILL **Plan**: Pro **Run ID**: `d1f68bd7-daab-48dd-9830-c86b95df4b9f` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between f08826951d6efe6188c02a198e19d8af7042e07d and e4d562a1c36d3a0f86c7fd12e4ae6050610ab044. </details> <details> <summary>📒 Files selected for processing (55)</summary> * `.github/workflows/ci.yml` * `docs/content/api-reference/docker-model-runner.md` * `docs/content/api-reference/lmdeploy.md` * `docs/content/api-reference/openai.md` * `docs/content/api-reference/overview.md` * `docs/content/api-reference/sglang.md` * `docs/content/api-reference/system.md` * `docs/content/compare/gpustack.md` * `docs/content/compare/integration-patterns.md` * `docs/content/compare/localai.md` * `docs/content/concepts/health-checking.md` * `docs/content/concepts/load-balancing.md` * `docs/content/concepts/model-aliases.md` * `docs/content/concepts/profile-system.md` * `docs/content/configuration/examples.md` * `docs/content/configuration/filters.md` * `docs/content/configuration/practices/monitoring.md` * `docs/content/development/architecture.md` * `docs/content/development/circuit-breaker.md` * `docs/content/development/overview.md` * `docs/content/development/patterns.md` * `docs/content/development/setup.md` * `docs/content/getting-started/quickstart.md` * `docs/content/integrations/api-translation/anthropic.md` * `docs/content/integrations/backend/docker-model-runner.md` * `docs/content/integrations/backend/llamacpp.md` * `docs/content/integrations/backend/lmdeploy.md` * `docs/content/integrations/frontend/claude-code.md` * `docs/content/integrations/frontend/crush-cli.md` * `docs/content/integrations/frontend/opencode.md` * `docs/content/integrations/overview.md` * `docs/content/troubleshooting/path-preservation.md` * `docs/content/usage.md` * `examples/claude-code-llamacpp/README.md` * `examples/claude-code-llamacpp/compose.yaml` * `examples/claude-code-llamacpp/olla.yaml` * `examples/claude-code-llamacpp/test.sh` * `examples/claude-code-ollama/README.md` * `examples/claude-code-ollama/claude-code-config.example.json` * `examples/claude-code-ollama/compose.yaml` * `examples/claude-code-ollama/olla.yaml` * `examples/claude-code-ollama/test.sh` * `examples/crush-vllm/README.md` * `examples/crush-vllm/crush-config.example.json` * `examples/crush-vllm/olla.yaml` * `examples/crush-vllm/test.sh` * `examples/ollama-openwebui/README.md` * `examples/ollama-openwebui/compose.yaml` * `examples/ollama-openwebui/olla.yaml` * `examples/opencode-lmstudio/README.md` * `examples/opencode-lmstudio/compose.yaml` * `examples/opencode-lmstudio/olla.yaml` * `examples/opencode-lmstudio/opencode-config.example.json` * `examples/opencode-lmstudio/test.sh` * `readme.md` </details> <details> <summary>✅ Files skipped from review due to trivial changes (21)</summary> * examples/opencode-lmstudio/compose.yaml * docs/content/getting-started/quickstart.md * .github/workflows/ci.yml * docs/content/configuration/filters.md * docs/content/integrations/backend/lmdeploy.md * docs/content/integrations/backend/docker-model-runner.md * docs/content/api-reference/openai.md * docs/content/compare/gpustack.md * docs/content/usage.md * docs/content/concepts/model-aliases.md * docs/content/integrations/backend/llamacpp.md * docs/content/compare/localai.md * docs/content/concepts/profile-system.md * docs/content/configuration/practices/monitoring.md * examples/crush-vllm/test.sh * docs/content/integrations/frontend/claude-code.md * readme.md * docs/content/development/patterns.md * docs/content/api-reference/sglang.md * docs/content/troubleshooting/path-preservation.md * docs/content/development/setup.md </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (6)</summary> * docs/content/api-reference/docker-model-runner.md * docs/content/development/overview.md * docs/content/configuration/examples.md * docs/content/development/architecture.md * docs/content/compare/integration-patterns.md * docs/content/integrations/frontend/opencode.md </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| - WEBUI_NAME=Olla + Open WebUI | ||
| - WEBUI_URL=http://localhost:3000 | ||
| - WEBUI_SECRET_KEY=your-secret-key-change-this | ||
| - WEBUI_SECRET_KEY=change-this-secret-key |
There was a problem hiding this comment.
Insecure default secret key requires user action.
The WEBUI_SECRET_KEY is set to a well-known default value (change-this-secret-key). Users who deploy this example without reading the config will have a predictable secret, allowing session forgery.
🔐 Recommended improvement
Add a comment warning users to change this value, or consider generating a random key:
- - WEBUI_SECRET_KEY=change-this-secret-key
+ # IMPORTANT: Generate a unique secret key before deploying
+ # Example: openssl rand -hex 32
+ - WEBUI_SECRET_KEY=change-this-secret-key-before-deployingAlternatively, provide a script or make target that generates a random secret for first-time setup.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - WEBUI_SECRET_KEY=change-this-secret-key | |
| # IMPORTANT: Generate a unique secret key before deploying | |
| # Example: openssl rand -hex 32 | |
| - WEBUI_SECRET_KEY=change-this-secret-key-before-deploying |
🤖 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 `@examples/ollama-openwebui/compose.yaml` at line 40, Replace the insecure
hardcoded default for WEBUI_SECRET_KEY with a safer setup: add a clear comment
next to WEBUI_SECRET_KEY warning users to change it and either (A) make the
compose entry a placeholder that requires the operator to supply the value
(e.g., read from environment/.env) or (B) add a one-time bootstrap script or
Make target that generates a cryptographically-random secret and writes it to
.env, then reference that variable in the compose file; update any service that
reads WEBUI_SECRET_KEY accordingly so deployments never run with the literal
"change-this-secret-key".
| # Test 4: Check endpoint status | ||
| echo -e "${BLUE}[4/8]${NC} Checking LM Studio endpoint status in Olla..." | ||
| echo -e "${BLUE}[4/6]${NC} Checking LM Studio endpoint status in Olla..." | ||
| if ENDPOINTS=$(curl -s -f "http://${OLLA_HOST}:${OLLA_PORT}/internal/status/endpoints" 2>&1); then |
There was a problem hiding this comment.
Remove unused ENDPOINTS variable assignment.
The ENDPOINTS variable is assigned but never referenced. The endpoint status test now only checks retrieval success without parsing the response.
🧹 Proposed fix
-if ENDPOINTS=$(curl -s -f "http://${OLLA_HOST}:${OLLA_PORT}/internal/status/endpoints" 2>&1); then
+if curl -s -f "http://${OLLA_HOST}:${OLLA_PORT}/internal/status/endpoints" > /dev/null 2>&1; then
print_result 0 "Endpoint status retrieved"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ENDPOINTS=$(curl -s -f "http://${OLLA_HOST}:${OLLA_PORT}/internal/status/endpoints" 2>&1); then | |
| if curl -s -f "http://${OLLA_HOST}:${OLLA_PORT}/internal/status/endpoints" > /dev/null 2>&1; then |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 81-81: ENDPOINTS appears unused. Verify use (or export if used externally).
(SC2034)
🤖 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 `@examples/opencode-lmstudio/test.sh` at line 81, The if-statement assigns
ENDPOINTS but never uses it; change the condition so it invokes curl to check
endpoint availability without capturing output: remove the ENDPOINTS= assignment
in the if condition that currently calls curl (the line referencing ENDPOINTS
and curl to "http://${OLLA_HOST}:${OLLA_PORT}/internal/status/endpoints") and
instead run curl with -s -f and redirect its output to /dev/null so the script
only tests curl's exit status.
Source: Linters/SAST tools
Documentation refresh for v0.0.28 release preparation, squashes 3 months of updates :(
Summary by CodeRabbit