Add backend prefix registry#25
Merged
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 19 minutes and 23 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughIntroduces two new Ollama backend plugins ( ChangesOllama backend plugin, BackendPrefixes contract, frontend route-select, and minor fixes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🤖 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 `@internal/infra/modelcatalog/modelsdev/normalize_test.go`:
- Around line 46-59: Add a new table-driven test function
TestParseModelIDs_EdgeCases that tests the error handling paths of the
modelsdev.ParseModelIDs function. Create a test with multiple cases that verify
behavior for empty payloads (should error), null root JSON (should error), empty
objects (should return empty slice), and providers missing the models field.
Each test case should check whether an error is returned based on the wantErr
flag to ensure all edge cases handled by the implementation are properly
verified.
In `@internal/infra/modelcatalog/modelsdev/normalize.go`:
- Around line 73-112: The ParseModelIDs function duplicates approximately 90% of
the payload validation and provider iteration logic already present in
ParseSnapshot. Extract a shared helper function (for example, decodeProviderMap)
that handles the common logic of validating empty/null payloads, unmarshaling
the root map, iterating through providers, and decoding wireProvider/wireModel
structures. This helper should accept a callback function to handle provider and
model processing differently for each caller. Update both ParseModelIDs and
ParseSnapshot to use this shared helper, passing their respective callback logic
to handle building the ID list and catalog facts map respectively.
In `@internal/pluginreg/backend_prefix_inventory_test.go`:
- Around line 20-22: Remove the conditional block that skips the "bedrock"
backend in the inventory test loop. The if statement checking if id equals
"bedrock" and the corresponding continue statement are outdated and should be
deleted entirely. Bedrock now properly implements BackendPrefixes in its
NewWithContext factory method, making it compatible with this test. The existing
test configuration with region us-east-1 is sufficient to construct a backend
with populated prefixes, so bedrock should participate in the inventory test
alongside other backends.
In `@internal/plugins/backends/bedrock/prefix_test.go`:
- Around line 11-14: The test code creates and immediately cancels a context
before passing it to NewWithContext to intentionally trigger the error path
during backend construction. Add a clarifying comment above the
context.WithCancel and cancel() calls that explains this test strategy,
specifically noting that the canceled context is used to verify that
BackendPrefixes is properly set even when newRuntimeClient fails due to the
canceled context.
In `@internal/plugins/backends/ollama/inventory_test.go`:
- Around line 30-34: The cloudHits and localHits variables are accessed from
multiple goroutines (HTTP handler callbacks and the test goroutine) without
synchronization, causing a data race. Replace the plain int declarations for
cloudHits and localHits with atomic.Int64 types, then update all increment
operations from cloudHits++ to cloudHits.Add(1), and all read operations where
cloudHits or localHits are used in assertions to use the .Load() method instead
of direct access. Apply this pattern consistently across all occurrences at the
declaration point (around line 30), the cloudHits assertions (lines 84-88), the
localHits assertions (lines 125-128), and the subsequent assertions (lines
180-183).
In `@internal/plugins/backends/ollama/inventory.go`:
- Around line 296-314: The probeCapabilities function creates a new capsByID map
and replaces p.capsByID entirely, which can discard concurrent capability
updates from ProbeCapsForNative. Instead of replacing the entire map at the end
of probeCapabilities (where p.capsByID = capsByID is assigned), merge the newly
probed capabilities into the existing p.capsByID map while holding the p.capsMu
lock. Iterate through the capsByID map built from the probed models and assign
each capability to the existing p.capsByID map rather than doing a wholesale
replacement, ensuring that any capabilities written by ProbeCapsForNative
concurrently are preserved.
In `@internal/plugins/backends/ollama/version_test.go`:
- Around line 9-24: The test function TestNativeRootFromBaseURL currently only
tests lowercase /v1 paths but does not cover the uppercase /V1 variant. To
verify that NativeRootFromBaseURL correctly handles case-insensitive path
matching, add one or more test cases to the tests slice that include uppercase
/V1 suffixes (e.g., http://localhost:11434/V1 and http://localhost:11434/V1/)
with their corresponding expected root URLs to ensure the function works
correctly with all case variants.
In `@internal/plugins/backends/ollama/version.go`:
- Around line 34-35: The check for `len(parts) == 0` in the version parsing
logic is unreachable dead code because strings.Split always returns at least one
element even for empty strings, and empty strings are already handled earlier in
the function. Remove the entire `if len(parts) == 0` condition block that
returns the error, as this case cannot occur.
- Around line 17-23: In the NativeRootFromBaseURL function, the code checks for
a /v1 suffix using a lowercased version of the string but then attempts to trim
the suffix from the original uncased string, which will fail for uppercase or
mixed-case variants like /V1. Fix this by trimming based on the string length
instead of the literal suffix string, so that the original casing is properly
removed. When the lowercased version has the /v1 suffix, use string slicing to
remove the last 3 characters from the original string u before returning it.
In `@internal/plugins/frontends/routeselect/routeselect.go`:
- Around line 1-44: Add package-level documentation at the beginning of the
routeselect package file explaining the purpose and the route selector concept.
Add doc comments above the InlineOrDefault function describing what it does, its
parameters (model and defaultRoute), and what it returns. Add doc comments above
the FromModelOrDefault function describing what it does, its parameters (body
and defaultRoute), and what it returns. Ensure all doc comments follow Go
conventions by starting with the function/package name.
- Around line 8-21: The inlineRoutePrefixes map includes the "stub" prefix but
there is no corresponding backend plugin registered with that ID; "stub" only
appears in test mocks. Either remove "stub" from the inlineRoutePrefixes map if
it is not needed for production functionality, or add a clear comment above the
map entry explaining why it is included for test purposes. Additionally,
consider whether the entire inlineRoutePrefixes map should be derived from
registered backend IDs at runtime or protected by an archtest to prevent future
sync issues when new backends are added or removed.
In `@scripts/ollama-text-smoke.ps1`:
- Line 283: In the if condition that checks script:proc.ExitCode, reorder the
null comparison to follow PowerShell style guidelines. Change the current
comparison `$script:proc.ExitCode -ne $null` to place `$null` on the left side
as `$null -ne $script:proc.ExitCode`. This idiomatic comparison order prevents
accidental assignment operators and improves code readability according to
PowerShell best practices.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: dbfa79b3-fc5f-40ab-83f7-c51f0d1e28a4
📒 Files selected for processing (75)
cmd/lipstd/testdata/dogfood-local-stub/inventory.golden.jsoncmd/lipstd/testdata/dogfood-local-stub/routes.golden.jsonconfig/config.yamlconfig/examples/anthropic-stub.yamlconfig/examples/dogfood-local-stub.yamlconfig/examples/gemini-stub.yamlconfig/examples/openai-legacy-stub.yamlconfig/examples/openai-responses-stub.yamldocs/capability-catalogs.mddocs/plugin-authoring.mdinternal/archtest/backend_lifecycle_contract_test.gointernal/core/execbackend/backend.gointernal/core/modelregistry/prefix_test.gointernal/core/modelregistry/registry.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime.gointernal/core/modelregistry/runtime_test.gointernal/core/runtime/attempt_accounting.gointernal/core/runtime/attempt_accounting_test.gointernal/infra/modelcatalog/modelsdev/normalize.gointernal/infra/modelcatalog/modelsdev/normalize_test.gointernal/infra/runtimebundle/build.gointernal/infra/runtimebundle/dual_backend_test.gointernal/infra/runtimebundle/exclusive_registry_test.gointernal/infra/runtimebundle/modelregistry_build_test.gointernal/infra/runtimebundle/security_policy_test.gointernal/infra/runtimebundle/token_accounting_build_test.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/pluginreg/backends_install.gointernal/pluginreg/ollama_build_test.gointernal/pluginreg/spec_bundle_standard_inventory_test.gointernal/pluginreg/standard_table.gointernal/plugins/backends/acp/plugin.gointernal/plugins/backends/anthropic/plugin.gointernal/plugins/backends/bedrock/plugin.gointernal/plugins/backends/bedrock/prefix_test.gointernal/plugins/backends/gemini/plugin.gointernal/plugins/backends/localstub/plugin.gointernal/plugins/backends/ollama/caps.gointernal/plugins/backends/ollama/caps_test.gointernal/plugins/backends/ollama/config.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/ollama/doc.gointernal/plugins/backends/ollama/integration_test.gointernal/plugins/backends/ollama/inventory.gointernal/plugins/backends/ollama/inventory_test.gointernal/plugins/backends/ollama/mode.gointernal/plugins/backends/ollama/mode_test.gointernal/plugins/backends/ollama/payload_mutate.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/ollama/plugin.gointernal/plugins/backends/ollama/resolve.gointernal/plugins/backends/ollama/resolve_test.gointernal/plugins/backends/ollama/version.gointernal/plugins/backends/ollama/version_test.gointernal/plugins/backends/openaicompat/backend.gointernal/plugins/backends/openailegacy/plugin.gointernal/plugins/backends/openairesponses/plugin.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/refbackend/ollama/backend_test.gointernal/refbackend/ollama/doc.gointernal/refbackend/ollama/server.gointernal/stdhttp/testdata/dogfood_gemini_dual_stub_failover.yamlpkg/lipsdk/standard_bundle.gopkg/lipsdk/standard_bundle_ollama_test.goscripts/ollama-text-smoke.ps1
💤 Files with no reviewable changes (1)
- internal/infra/runtimebundle/dual_backend_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: qa
🧰 Additional context used
📓 Path-based instructions (13)
internal/plugins/backends/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Provider SDKs may only be imported in backend plugin adapters
Files:
internal/plugins/backends/ollama/payload_mutate.gointernal/plugins/backends/ollama/doc.gointernal/plugins/backends/ollama/mode_test.gointernal/plugins/backends/ollama/mode.gointernal/plugins/backends/localstub/plugin.gointernal/plugins/backends/ollama/caps.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/acp/plugin.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/bedrock/prefix_test.gointernal/plugins/backends/ollama/caps_test.gointernal/plugins/backends/anthropic/plugin.gointernal/plugins/backends/ollama/config.gointernal/plugins/backends/gemini/plugin.gointernal/plugins/backends/openairesponses/plugin.gointernal/plugins/backends/openaicompat/backend.gointernal/plugins/backends/openailegacy/plugin.gointernal/plugins/backends/ollama/resolve.gointernal/plugins/backends/ollama/version_test.gointernal/plugins/backends/ollama/version.gointernal/plugins/backends/bedrock/plugin.gointernal/plugins/backends/ollama/resolve_test.gointernal/plugins/backends/ollama/plugin.gointernal/plugins/backends/ollama/integration_test.gointernal/plugins/backends/ollama/inventory_test.gointernal/plugins/backends/ollama/inventory.go
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use small interfaces defined where they are consumed
Every I/O boundary takescontext.Context
Do not use Java-style interface prefixes; use idiomatic Go names such asStore,Router,Clock
Avoidanyunless unavoidable at a protocol boundary
Wrap errors with%wand preserve classification metadata
Avoid circular imports by design
Prefer the standard library unless a dependency clearly reduces complexity
Use small interfaces defined where they are consumed
Slices in JSON and returned values should use explicit empty initialization (s := []T{}ormake) to preventnullin JSON
Establish explicit ownership for goroutines, channels, buffers, and cancellation
Do not use Go's nativepluginpackage in v1
Add package docs where the boundary is non-obvious
Line length ~120+ characters; break at semantic boundaries. Single long lines preferred for embedded JSON/SSE to preserve stream integrityFor server, CLI, worker, or network code, check that context.Context is propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely
Files:
internal/plugins/backends/ollama/payload_mutate.gointernal/plugins/backends/ollama/doc.gointernal/plugins/backends/ollama/mode_test.gointernal/plugins/backends/ollama/mode.gointernal/plugins/frontends/gemini/integration_test.gopkg/lipsdk/standard_bundle.gointernal/pluginreg/spec_bundle_standard_inventory_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/backends/localstub/plugin.gointernal/core/runtime/attempt_accounting.gointernal/plugins/frontends/anthropic/integration_test.gointernal/infra/runtimebundle/exclusive_registry_test.gointernal/plugins/backends/ollama/caps.gointernal/infra/modelcatalog/modelsdev/normalize_test.gopkg/lipsdk/standard_bundle_ollama_test.gointernal/refbackend/ollama/doc.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/acp/plugin.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/bedrock/prefix_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/infra/modelcatalog/modelsdev/normalize.gointernal/plugins/backends/ollama/caps_test.gointernal/core/modelregistry/runtime.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/anthropic/plugin.gointernal/plugins/backends/ollama/config.gointernal/plugins/backends/gemini/plugin.gointernal/plugins/backends/openairesponses/plugin.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/pluginreg/ollama_build_test.gointernal/plugins/backends/openaicompat/backend.gointernal/infra/runtimebundle/security_policy_test.gointernal/core/runtime/attempt_accounting_test.gointernal/plugins/backends/openailegacy/plugin.gointernal/plugins/backends/ollama/resolve.gointernal/plugins/backends/ollama/version_test.gointernal/plugins/backends/ollama/version.gointernal/plugins/frontends/gemini/handler.gointernal/infra/runtimebundle/build.gointernal/plugins/frontends/openairesponses/handler.gointernal/pluginreg/standard_table.gointernal/core/modelregistry/prefix_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/anthropic/handler.gointernal/core/execbackend/backend.gointernal/infra/runtimebundle/token_accounting_build_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/backends/bedrock/plugin.gointernal/plugins/backends/ollama/resolve_test.gointernal/plugins/backends/ollama/plugin.gointernal/refbackend/ollama/server.gointernal/core/modelregistry/registry.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime_test.gointernal/infra/runtimebundle/modelregistry_build_test.gointernal/pluginreg/backends_install.gointernal/plugins/backends/ollama/integration_test.gointernal/refbackend/ollama/backend_test.gointernal/plugins/backends/ollama/inventory_test.gointernal/plugins/backends/ollama/inventory.go
⚙️ CodeRabbit configuration file
**/*.go: Review as production Go code. Prioritize correctness, race conditions, goroutine leaks, context cancellation, timeout handling, error wrapping, nil-pointer risks, resource cleanup, defer placement, API compatibility, interface design, dependency boundaries, and testability. Avoid generic style comments when gofmt/golangci-lint already covers the issue.
Files:
internal/plugins/backends/ollama/payload_mutate.gointernal/plugins/backends/ollama/doc.gointernal/plugins/backends/ollama/mode_test.gointernal/plugins/backends/ollama/mode.gointernal/plugins/frontends/gemini/integration_test.gopkg/lipsdk/standard_bundle.gointernal/pluginreg/spec_bundle_standard_inventory_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/backends/localstub/plugin.gointernal/core/runtime/attempt_accounting.gointernal/plugins/frontends/anthropic/integration_test.gointernal/infra/runtimebundle/exclusive_registry_test.gointernal/plugins/backends/ollama/caps.gointernal/infra/modelcatalog/modelsdev/normalize_test.gopkg/lipsdk/standard_bundle_ollama_test.gointernal/refbackend/ollama/doc.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/acp/plugin.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/bedrock/prefix_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/infra/modelcatalog/modelsdev/normalize.gointernal/plugins/backends/ollama/caps_test.gointernal/core/modelregistry/runtime.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/anthropic/plugin.gointernal/plugins/backends/ollama/config.gointernal/plugins/backends/gemini/plugin.gointernal/plugins/backends/openairesponses/plugin.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/pluginreg/ollama_build_test.gointernal/plugins/backends/openaicompat/backend.gointernal/infra/runtimebundle/security_policy_test.gointernal/core/runtime/attempt_accounting_test.gointernal/plugins/backends/openailegacy/plugin.gointernal/plugins/backends/ollama/resolve.gointernal/plugins/backends/ollama/version_test.gointernal/plugins/backends/ollama/version.gointernal/plugins/frontends/gemini/handler.gointernal/infra/runtimebundle/build.gointernal/plugins/frontends/openairesponses/handler.gointernal/pluginreg/standard_table.gointernal/core/modelregistry/prefix_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/anthropic/handler.gointernal/core/execbackend/backend.gointernal/infra/runtimebundle/token_accounting_build_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/backends/bedrock/plugin.gointernal/plugins/backends/ollama/resolve_test.gointernal/plugins/backends/ollama/plugin.gointernal/refbackend/ollama/server.gointernal/core/modelregistry/registry.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime_test.gointernal/infra/runtimebundle/modelregistry_build_test.gointernal/pluginreg/backends_install.gointernal/plugins/backends/ollama/integration_test.gointernal/refbackend/ollama/backend_test.gointernal/plugins/backends/ollama/inventory_test.gointernal/plugins/backends/ollama/inventory.go
internal/plugins/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/plugins/**/*.go: Keep provider-specific payload types inside adapters and plugins
No pairwise protocol translators; only protocol <-> canonical adapters
Files:
internal/plugins/backends/ollama/payload_mutate.gointernal/plugins/backends/ollama/doc.gointernal/plugins/backends/ollama/mode_test.gointernal/plugins/backends/ollama/mode.gointernal/plugins/frontends/gemini/integration_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/backends/localstub/plugin.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/backends/ollama/caps.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/acp/plugin.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/bedrock/prefix_test.gointernal/plugins/backends/ollama/caps_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/anthropic/plugin.gointernal/plugins/backends/ollama/config.gointernal/plugins/backends/gemini/plugin.gointernal/plugins/backends/openairesponses/plugin.gointernal/plugins/backends/openaicompat/backend.gointernal/plugins/backends/openailegacy/plugin.gointernal/plugins/backends/ollama/resolve.gointernal/plugins/backends/ollama/version_test.gointernal/plugins/backends/ollama/version.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/backends/bedrock/plugin.gointernal/plugins/backends/ollama/resolve_test.gointernal/plugins/backends/ollama/plugin.gointernal/plugins/backends/ollama/integration_test.gointernal/plugins/backends/ollama/inventory_test.gointernal/plugins/backends/ollama/inventory.go
internal/**
⚙️ CodeRabbit configuration file
internal/**: Focus on package boundaries, hidden coupling, unexported API design, concurrency safety, deterministic behavior, and whether logic belongs in this internal package.
Files:
internal/plugins/backends/ollama/payload_mutate.gointernal/plugins/backends/ollama/doc.gointernal/plugins/backends/ollama/mode_test.gointernal/plugins/backends/ollama/mode.gointernal/plugins/frontends/gemini/integration_test.gointernal/stdhttp/testdata/dogfood_gemini_dual_stub_failover.yamlinternal/pluginreg/spec_bundle_standard_inventory_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/backends/localstub/plugin.gointernal/core/runtime/attempt_accounting.gointernal/plugins/frontends/anthropic/integration_test.gointernal/infra/runtimebundle/exclusive_registry_test.gointernal/plugins/backends/ollama/caps.gointernal/infra/modelcatalog/modelsdev/normalize_test.gointernal/refbackend/ollama/doc.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/acp/plugin.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/bedrock/prefix_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/infra/modelcatalog/modelsdev/normalize.gointernal/plugins/backends/ollama/caps_test.gointernal/core/modelregistry/runtime.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/backends/anthropic/plugin.gointernal/plugins/backends/ollama/config.gointernal/plugins/backends/gemini/plugin.gointernal/plugins/backends/openairesponses/plugin.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/pluginreg/ollama_build_test.gointernal/plugins/backends/openaicompat/backend.gointernal/infra/runtimebundle/security_policy_test.gointernal/core/runtime/attempt_accounting_test.gointernal/plugins/backends/openailegacy/plugin.gointernal/plugins/backends/ollama/resolve.gointernal/plugins/backends/ollama/version_test.gointernal/plugins/backends/ollama/version.gointernal/plugins/frontends/gemini/handler.gointernal/infra/runtimebundle/build.gointernal/plugins/frontends/openairesponses/handler.gointernal/pluginreg/standard_table.gointernal/core/modelregistry/prefix_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/anthropic/handler.gointernal/core/execbackend/backend.gointernal/infra/runtimebundle/token_accounting_build_test.gointernal/plugins/frontends/openailegacy/handler.gointernal/plugins/backends/bedrock/plugin.gointernal/plugins/backends/ollama/resolve_test.gointernal/plugins/backends/ollama/plugin.gointernal/refbackend/ollama/server.gointernal/core/modelregistry/registry.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime_test.gointernal/infra/runtimebundle/modelregistry_build_test.gointernal/pluginreg/backends_install.gointernal/plugins/backends/ollama/integration_test.gointernal/refbackend/ollama/backend_test.gointernal/plugins/backends/ollama/inventory_test.gointernal/plugins/backends/ollama/inventory.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Prefer testdata/ for very large wire dumps and protocol fixtures instead of megastrings in test code
Keep tests near the package they validate unless a cross-package integration test is required
Tests are behavior contracts, not implementation snapshots
Files:
internal/plugins/backends/ollama/mode_test.gointernal/plugins/frontends/gemini/integration_test.gointernal/pluginreg/spec_bundle_standard_inventory_test.gointernal/plugins/frontends/anthropic/integration_test.gointernal/infra/runtimebundle/exclusive_registry_test.gointernal/infra/modelcatalog/modelsdev/normalize_test.gopkg/lipsdk/standard_bundle_ollama_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/bedrock/prefix_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/plugins/backends/ollama/caps_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/pluginreg/ollama_build_test.gointernal/infra/runtimebundle/security_policy_test.gointernal/core/runtime/attempt_accounting_test.gointernal/plugins/backends/ollama/version_test.gointernal/core/modelregistry/prefix_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/infra/runtimebundle/token_accounting_build_test.gointernal/plugins/backends/ollama/resolve_test.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime_test.gointernal/infra/runtimebundle/modelregistry_build_test.gointernal/plugins/backends/ollama/integration_test.gointernal/refbackend/ollama/backend_test.gointernal/plugins/backends/ollama/inventory_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Review tests for meaningful assertions, table-driven coverage, race-prone tests, t.Parallel misuse, nondeterminism, leaked goroutines, real network or filesystem dependencies, fragile sleeps, and missing edge cases. Prefer testing observable behavior over implementation details.
Files:
internal/plugins/backends/ollama/mode_test.gointernal/plugins/frontends/gemini/integration_test.gointernal/pluginreg/spec_bundle_standard_inventory_test.gointernal/plugins/frontends/anthropic/integration_test.gointernal/infra/runtimebundle/exclusive_registry_test.gointernal/infra/modelcatalog/modelsdev/normalize_test.gopkg/lipsdk/standard_bundle_ollama_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/backends/ollama/config_test.gointernal/plugins/backends/ollama/payload_mutate_test.gointernal/plugins/backends/bedrock/prefix_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/plugins/backends/ollama/caps_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/pluginreg/ollama_build_test.gointernal/infra/runtimebundle/security_policy_test.gointernal/core/runtime/attempt_accounting_test.gointernal/plugins/backends/ollama/version_test.gointernal/core/modelregistry/prefix_test.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/infra/runtimebundle/token_accounting_build_test.gointernal/plugins/backends/ollama/resolve_test.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime_test.gointernal/infra/runtimebundle/modelregistry_build_test.gointernal/plugins/backends/ollama/integration_test.gointernal/refbackend/ollama/backend_test.gointernal/plugins/backends/ollama/inventory_test.go
internal/plugins/frontends/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/plugins/frontends/**/*.go: Do not addgo(goroutine) in request handlers, frontend encoders, or per-call hot paths; prefer long-lived workers and stream-scoped pumps
Frontends are responsible for mapping internal errors to protocol-specific error shapes
Files:
internal/plugins/frontends/gemini/integration_test.gointernal/plugins/frontends/routeselect/routeselect.gointernal/plugins/frontends/anthropic/integration_test.gointernal/plugins/frontends/openairesponses/integration_test.gointernal/plugins/frontends/routeselect/routeselect_test.gointernal/plugins/frontends/gemini/handler.gointernal/plugins/frontends/openairesponses/handler.gointernal/plugins/frontends/openailegacy/integration_test.gointernal/plugins/frontends/anthropic/handler.gointernal/plugins/frontends/openailegacy/handler.go
cmd/**
⚙️ CodeRabbit configuration file
cmd/**: Focus on CLI/service entrypoint behavior: signal handling, graceful shutdown, context propagation, configuration parsing, environment variables, logging setup, exit codes, and backwards compatibility of flags or command output.
Files:
cmd/lipstd/testdata/dogfood-local-stub/inventory.golden.jsoncmd/lipstd/testdata/dogfood-local-stub/routes.golden.json
pkg/lipsdk/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Public contracts in
pkg/lipapiandpkg/lipsdkmust be versionable, documented, and minimal
Files:
pkg/lipsdk/standard_bundle.gopkg/lipsdk/standard_bundle_ollama_test.go
pkg/**/*.go
📄 CodeRabbit inference engine (Custom checks)
For changes under pkg/** or exported Go identifiers, warn if the PR changes exported types, function signatures, error behavior, JSON fields, CLI flags, config keys, or documented behavior without clearly explaining the compatibility impact
Files:
pkg/lipsdk/standard_bundle.gopkg/lipsdk/standard_bundle_ollama_test.go
pkg/**
⚙️ CodeRabbit configuration file
pkg/**: Treat exported identifiers as public API. Flag breaking changes, ambiguous contracts, missing error semantics, poor interface boundaries, and changes that make downstream usage harder.
Files:
pkg/lipsdk/standard_bundle.gopkg/lipsdk/standard_bundle_ollama_test.go
internal/core/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/core/**/*.go: Core packages must not import official provider SDKs
Core packages must not import concrete plugins
Return errors, do not panic in request paths
Keep core files small and cohesive
Do not mix frontend codec logic, routing policy, and backend invocation in one package
Prefer explicit construction and registration over DI containers, reflection, or global registries
No package-level mutable global state in core code
Capability mismatches must fail explicitly; never silently drop required semantics
Advanced request/response mutation belongs behind hook interfaces, not inside core business logic
Narrow interfaces, small files, simple control flow (keep the core boring)
Files:
internal/core/runtime/attempt_accounting.gointernal/core/modelregistry/runtime.gointernal/core/runtime/attempt_accounting_test.gointernal/core/modelregistry/prefix_test.gointernal/core/execbackend/backend.gointernal/core/modelregistry/registry.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime_test.go
internal/core/runtime/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/core/runtime/**/*.go: No transparent retry or failover after the first downstream content event is emitted
Streaming is the primary path; non-streaming is collected from the streaming path
Preserve ordering guarantees for canonical event streams
Files:
internal/core/runtime/attempt_accounting.gointernal/core/runtime/attempt_accounting_test.go
internal/core/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/core/**/*_test.go: Add regression tests for every bug fix in routing, translation, or streaming behavior
Decoder and selector parsers should gain fuzz tests when practical
Files:
internal/core/runtime/attempt_accounting_test.gointernal/core/modelregistry/prefix_test.gointernal/core/modelregistry/registry_test.gointernal/core/modelregistry/runtime_test.go
🪛 PSScriptAnalyzer (1.25.0)
scripts/ollama-text-smoke.ps1
[warning] 283-283: $null should be on the left side of equality comparisons.
Suggested fix: Use $null on the left hand side for safe comparison with $null.
(PSPossibleIncorrectComparisonWithNull)
[warning] 9-9: The parameter 'StartupTimeoutSeconds' has been declared but not used.
(PSReviewUnusedParameter)
[warning] 10-10: The parameter 'RequestTimeoutSeconds' has been declared but not used.
(PSReviewUnusedParameter)
[warning] 22-22: Function 'New-FreeLoopbackAddress' has verb that could change system state. Therefore, the function has to support 'ShouldProcess'.
(PSUseShouldProcessForStateChangingFunctions)
🔇 Additional comments (100)
internal/core/runtime/attempt_accounting.go (1)
94-96: LGTM!internal/core/runtime/attempt_accounting_test.go (1)
54-70: LGTM!internal/plugins/backends/ollama/doc.go (1)
1-8: LGTM!internal/pluginreg/standard_table.go (2)
14-14: LGTM!
178-183: LGTM!pkg/lipsdk/standard_bundle.go (1)
19-20: LGTM!pkg/lipsdk/standard_bundle_ollama_test.go (1)
1-21: LGTM!internal/pluginreg/ollama_build_test.go (5)
17-29: LGTM!
31-49: LGTM!
51-104: LGTM!
106-124: LGTM!
126-171: LGTM!internal/pluginreg/spec_bundle_standard_inventory_test.go (1)
37-38: LGTM!config/config.yaml (1)
279-300: LGTM!config/examples/anthropic-stub.yaml (1)
63-68: LGTM!internal/plugins/backends/ollama/config.go (4)
10-39: LGTM!
41-58: LGTM!
60-85: LGTM!
87-92: LGTM!internal/plugins/backends/ollama/mode.go (4)
5-10: LGTM!
12-23: LGTM!
25-36: LGTM!
38-54: LGTM!internal/plugins/backends/ollama/payload_mutate.go (1)
8-17: LGTM!internal/plugins/backends/ollama/config_test.go (3)
10-20: LGTM!
22-52: LGTM!
54-61: LGTM!internal/plugins/backends/ollama/mode_test.go (2)
5-58: LGTM!
60-113: LGTM!internal/plugins/backends/ollama/payload_mutate_test.go (3)
9-16: LGTM!
18-28: LGTM!
30-40: LGTM!internal/refbackend/ollama/backend_test.go (6)
14-40: LGTM!
42-80: LGTM!
82-122: LGTM!
124-143: LGTM!
145-164: LGTM!
166-404: LGTM!scripts/ollama-text-smoke.ps1 (6)
1-21: LGTM!
22-31: LGTM!
33-83: LGTM!
85-102: LGTM!
104-140: LGTM!
142-282: LGTM!Also applies to: 284-287
internal/plugins/backends/ollama/plugin.go (1)
24-183: LGTM!internal/plugins/backends/ollama/integration_test.go (1)
1-670: LGTM!config/examples/openai-responses-stub.yaml (1)
64-69: LGTM!internal/stdhttp/testdata/dogfood_gemini_dual_stub_failover.yaml (1)
62-67: LGTM!cmd/lipstd/testdata/dogfood-local-stub/inventory.golden.json (1)
65-74: LGTM!internal/plugins/backends/ollama/caps.go (1)
7-26: LGTM!internal/plugins/backends/ollama/caps_test.go (1)
10-45: LGTM!internal/plugins/backends/ollama/resolve.go (1)
12-98: LGTM!internal/plugins/backends/ollama/resolve_test.go (1)
10-64: LGTM!internal/plugins/backends/ollama/inventory.go (4)
63-135: LGTM!
137-198: LGTM!
200-294: LGTM!
316-448: LGTM!cmd/lipstd/testdata/dogfood-local-stub/routes.golden.json (1)
44-53: LGTM!internal/plugins/backends/ollama/inventory_test.go (1)
520-577: LGTM!Also applies to: 579-627
internal/refbackend/ollama/doc.go (1)
1-4: LGTM!internal/refbackend/ollama/server.go (1)
1-333: LGTM!internal/pluginreg/backends_install.go (1)
363-435: LGTM!config/examples/dogfood-local-stub.yaml (1)
66-71: LGTM!config/examples/gemini-stub.yaml (1)
63-68: LGTM!config/examples/openai-legacy-stub.yaml (1)
63-68: LGTM!internal/core/execbackend/backend.go (1)
27-30: LGTM!internal/core/modelregistry/registry.go (3)
16-29: LGTM!
57-60: LGTM!Also applies to: 96-98, 121-121
150-213: LGTM!Also applies to: 227-229
internal/core/modelregistry/runtime.go (1)
200-204: LGTM!internal/infra/runtimebundle/build.go (1)
126-130: LGTM!docs/plugin-authoring.md (1)
64-69: LGTM!internal/plugins/backends/openairesponses/plugin.go (1)
52-53: LGTM!Also applies to: 125-127
internal/infra/runtimebundle/modelregistry_build_test.go (2)
34-35: LGTM!Also applies to: 58-59, 109-111, 153-155, 189-191, 216-218, 257-259
295-298: LGTM!Also applies to: 381-385
docs/capability-catalogs.md (1)
23-27: LGTM!internal/core/modelregistry/prefix_test.go (1)
1-151: LGTM!internal/core/modelregistry/registry_test.go (1)
19-29: LGTM!Also applies to: 59-72, 91-94, 110-112, 142-144, 181-214
internal/plugins/backends/bedrock/plugin.go (1)
61-63: LGTM!Also applies to: 74-76
internal/plugins/frontends/openailegacy/handler.go (1)
110-112: LGTM!internal/plugins/frontends/openairesponses/handler.go (1)
154-156: LGTM!internal/core/modelregistry/runtime_test.go (1)
32-35: LGTM!Also applies to: 74-77, 96-135, 147-150, 180-183, 225-228, 264-267, 296-299, 335-338, 377-380, 408-411, 446-449
internal/plugins/backends/acp/plugin.go (1)
55-57: LGTM!Also applies to: 74-75
internal/plugins/backends/anthropic/plugin.go (1)
62-62: LGTM!Also applies to: 122-124
internal/infra/runtimebundle/security_policy_test.go (1)
26-28: LGTM!Also applies to: 48-50
internal/plugins/frontends/anthropic/integration_test.go (1)
395-428: LGTM!internal/plugins/frontends/gemini/integration_test.go (1)
381-416: LGTM!internal/plugins/frontends/openailegacy/integration_test.go (1)
371-405: LGTM!Also applies to: 407-440
internal/plugins/frontends/openairesponses/integration_test.go (1)
398-431: LGTM!internal/plugins/backends/gemini/plugin.go (1)
58-59: LGTM!Also applies to: 121-123
internal/plugins/backends/localstub/plugin.go (1)
33-34: LGTM!internal/plugins/backends/openaicompat/backend.go (1)
48-58: LGTM!Also applies to: 124-129
internal/infra/runtimebundle/exclusive_registry_test.go (1)
30-32: LGTM!internal/plugins/backends/openailegacy/plugin.go (2)
51-109: LGTM!
123-135: LGTM!internal/infra/runtimebundle/token_accounting_build_test.go (1)
26-28: LGTM!Also applies to: 114-116, 159-161, 217-219, 257-259
internal/archtest/backend_lifecycle_contract_test.go (1)
14-19: LGTM!internal/plugins/frontends/routeselect/routeselect_test.go (1)
7-117: LGTM!internal/plugins/frontends/anthropic/handler.go (1)
15-15: LGTM!Also applies to: 99-116
internal/plugins/frontends/gemini/handler.go (1)
15-15: LGTM!Also applies to: 111-113
Comment on lines
+73
to
+112
| // ParseModelIDs decodes a models.dev-style provider map and returns sorted canonical provider/model IDs. | ||
| func ParseModelIDs(raw []byte) ([]string, error) { | ||
| if len(raw) == 0 { | ||
| return nil, errors.New("modelsdev: empty payload") | ||
| } | ||
| var root map[string]json.RawMessage | ||
| if err := json.Unmarshal(raw, &root); err != nil { | ||
| return nil, fmt.Errorf("modelsdev: decode root: %w", err) | ||
| } | ||
| if root == nil { | ||
| return nil, errors.New("modelsdev: unsupported schema: null root") | ||
| } | ||
| ids := []string{} | ||
| for providerKey, providerRaw := range root { | ||
| providerKey = strings.TrimSpace(providerKey) | ||
| if providerKey == "" { | ||
| continue | ||
| } | ||
| var wp wireProvider | ||
| if err := json.Unmarshal(providerRaw, &wp); err != nil { | ||
| return nil, fmt.Errorf("modelsdev: provider %q: %w", providerKey, err) | ||
| } | ||
| if len(wp.Models) == 0 || string(wp.Models) == "null" { | ||
| continue | ||
| } | ||
| var models []wireModel | ||
| if err := json.Unmarshal(wp.Models, &models); err != nil { | ||
| return nil, fmt.Errorf("modelsdev: provider %q models: %w", providerKey, err) | ||
| } | ||
| for _, wm := range models { | ||
| mid := strings.TrimSpace(wm.ID) | ||
| if mid != "" { | ||
| ids = append(ids, providerKey+"/"+mid) | ||
| } | ||
| } | ||
| } | ||
| slices.Sort(ids) | ||
| return ids, nil | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Consider extracting shared JSON decoding logic.
ParseModelIDs duplicates ~90% of the provider map decoding logic from ParseSnapshot (lines 21-71). Both functions:
- Validate empty/null payload identically (lines 23-32 vs 75-84)
- Decode and iterate providers identically (lines 35-50 vs 86-101)
- Only differ in what they build (catalog facts map vs ID list)
Extracting a shared helper (e.g., func iterateProviders(raw []byte, fn func(providerKey string, model wireModel))) would reduce duplication and make schema changes easier to maintain.
♻️ Example refactoring approach
// Shared helper
func decodeProviderMap(raw []byte, fn func(providerKey, modelID string)) error {
// Common validation and iteration logic
// ...
for _, wm := range models {
mid := strings.TrimSpace(wm.ID)
if mid != "" {
fn(providerKey, mid)
}
}
return nil
}
// ParseModelIDs becomes:
func ParseModelIDs(raw []byte) ([]string, error) {
var ids []string
err := decodeProviderMap(raw, func(pk, mid string) {
ids = append(ids, pk+"/"+mid)
})
if err != nil {
return nil, err
}
slices.Sort(ids)
return ids, nil
}🤖 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 `@internal/infra/modelcatalog/modelsdev/normalize.go` around lines 73 - 112,
The ParseModelIDs function duplicates approximately 90% of the payload
validation and provider iteration logic already present in ParseSnapshot.
Extract a shared helper function (for example, decodeProviderMap) that handles
the common logic of validating empty/null payloads, unmarshaling the root map,
iterating through providers, and decoding wireProvider/wireModel structures.
This helper should accept a callback function to handle provider and model
processing differently for each caller. Update both ParseModelIDs and
ParseSnapshot to use this shared helper, passing their respective callback logic
to handle building the ID list and catalog facts map respectively.
This was referenced Jun 24, 2026
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
Tests