feat(backends): add Hugging Face connector#34
Conversation
|
Warning Review limit reached
More reviews will be available in 14 minutes and 53 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 review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling 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 (4)
📝 WalkthroughWalkthroughThis PR adds a Hugging Face OpenAI-compatible backend, registers it in the standard backend bundle, extends upstream API-key resolution and custom-prefix validation for Hugging Face, and updates supporting tests, config, docs, and inventory tooling. ChangesHugging Face backend wiring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
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: 6
🤖 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/pluginreg/custom_backends.go`:
- Around line 204-218: The reservation check in standardBackendPrefixSet is
using backend IDs instead of the actual BackendPrefixes contract, so alias
prefixes can slip through and collide later. Update standardBackendPrefixSet to
build the reserved set from each built backend’s BackendPrefixes (matching
backend_prefix_inventory_test behavior) while still skipping custom-compatible
kinds via IsCustomCompatibleBackendKind, and keep
isReservedStandardBackendPrefix using that derived set.
In `@internal/pluginreg/hosted_backend_build_test.go`:
- Around line 60-77: The test in
TestBuildBackend_huggingface_envDefaultsWhenYAMLHasNoKeys only checks that
BuildBackend returns a backend, so it does not verify the Hugging Face env-key
fallback behavior. Update the test to exercise b.Open against an httptest.Server
and assert the outgoing request includes authorization using one of the
UpstreamAPIKeys.HuggingFace values injected via InstallStandardBackendsOn, so
the test proves credentials are wired when the YAML omits api_key/api_keys.
In `@internal/plugins/backends/huggingface/integration_test.go`:
- Around line 59-75: Avoid calling t.Fatal* inside the httptest handler in the
HuggingFace integration test; instead, have the handler record any request
mismatches and assert them from the main test goroutine after be.Open completes.
Update the handler around the srv setup in integration_test.go to capture
path/model/stream validation failures, so regressions surface as test assertions
rather than generic transport errors.
- Around line 30-45: The drainStream test helper can hang indefinitely because
it uses context.Background() for es.Recv and waits forever if EOF never arrives.
Update drainStream to create a bounded context with context.WithTimeout, pass
that context into es.Recv, and fail the test when the deadline expires. Keep the
change localized to drainStream in integration_test.go so all callers benefit
from the timeout.
In `@internal/plugins/backends/huggingface/inventory_test.go`:
- Around line 14-18: The httptest handler in the inventory test is calling
t.Fatalf inside the server goroutine, which can fail the test from the wrong
goroutine. Update the anonymous handler passed to httptest.NewServer in
inventory_test.go to avoid fatal assertions there; use t.Errorf or capture the
path mismatch in a variable, then perform the final assertion in the test body
after LoadModels returns. Keep the check associated with the handler logic so
the expected /v1/models path is still validated, but move the test failure
handling out of the handler goroutine.
In `@README.md`:
- Around line 87-88: README has inconsistent documentation for
ResolveUpstreamAPIKeysFromEnv because the earlier startup-flow bullet still
omits the Hugging Face key family. Update that existing bullet to mention
HUGGINGFACE_API_KEY and its numbered suffixes, matching the later Composition
roots note, so both descriptions list the same supported env key families and
ordering rules.
🪄 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: da83a2a3-6aed-4014-8b91-4fa43fd95324
📒 Files selected for processing (20)
README.mdcmd/model-inventory-proof/main.goconfig/config.yamldocs/backend-adapter-boundaries.mdinternal/archtest/backend_lifecycle_contract_test.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/pluginreg/backends_misc.gointernal/pluginreg/custom_backend_prefix_test.gointernal/pluginreg/custom_backends.gointernal/pluginreg/hosted_backend_build_test.gointernal/pluginreg/keys.gointernal/pluginreg/resolve_upstream_api_keys_test.gointernal/pluginreg/spec_bundle_standard_inventory_test.gointernal/pluginreg/standard_bundle_posture_test.gointernal/pluginreg/standard_table.gointernal/plugins/backends/huggingface/doc.gointernal/plugins/backends/huggingface/integration_test.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin.gointernal/plugins/backends/huggingface/plugin_config_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: qa
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (Custom checks)
For server, CLI, worker, or network code, check that context.Context is propagated correctly, cancellation is respected, and new goroutines cannot leak indefinitely
**/*.go: Avoidanyunless unavoidable at a protocol boundary
Use small interfaces defined where they are consumed
Do not use Java-style interface prefixes. Use idiomatic Go names such asStore,Router,Clock
Every I/O boundary takescontext.Context
Establish explicit ownership for goroutines, channels, buffers, and cancellation
Prefer simple push/pull stream abstractions over ad hoc channel webs
Return errors, do not panic in request paths
Wrap errors with%wand preserve classification metadata
Keep config structs typed and explicit
Avoid circular imports by design
Do not mix frontend codec logic, routing policy, and backend invocation in one package
Add package docs where the boundary is non-obvious
Line length ~120+: break at semantic boundaries where practical. When splitting embedded JSON/SSE in tests or emulators, re-check brace matching
Slices in JSON and returned values: prefer explicit empty initialization (s := []T{}ormake) sonullnever appears in JSON for 'empty list'
Short-lived append-only local buffers may usevar s []Tandappendwhen the value never escapes
JSON presence vs null: when a wire shape must preserve 'field absent' vs explicitnullvs empty containers, reuseinternal/core/jsonpresencepatterns
Files:
internal/pluginreg/spec_bundle_standard_inventory_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/plugins/backends/huggingface/doc.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin_config_test.gointernal/plugins/backends/huggingface/plugin.gointernal/pluginreg/standard_bundle_posture_test.gointernal/pluginreg/standard_table.gointernal/pluginreg/custom_backend_prefix_test.gointernal/pluginreg/backends_misc.gocmd/model-inventory-proof/main.gointernal/pluginreg/hosted_backend_build_test.gointernal/pluginreg/keys.gointernal/pluginreg/resolve_upstream_api_keys_test.gointernal/pluginreg/custom_backends.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/plugins/backends/huggingface/integration_test.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/pluginreg/spec_bundle_standard_inventory_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/plugins/backends/huggingface/doc.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin_config_test.gointernal/plugins/backends/huggingface/plugin.gointernal/pluginreg/standard_bundle_posture_test.gointernal/pluginreg/standard_table.gointernal/pluginreg/custom_backend_prefix_test.gointernal/pluginreg/backends_misc.gocmd/model-inventory-proof/main.gointernal/pluginreg/hosted_backend_build_test.gointernal/pluginreg/keys.gointernal/pluginreg/resolve_upstream_api_keys_test.gointernal/pluginreg/custom_backends.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/plugins/backends/huggingface/integration_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Prefer locking invariants (routing, streaming, B2BUA, capability mismatch, no-retry-after-output) through tests
Tests are behavior contracts, not implementation snapshots
Decoder and selector parsers should gain fuzz tests when practical
Keep tests near the package they validate unless a cross-package integration test is required
Files:
internal/pluginreg/spec_bundle_standard_inventory_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin_config_test.gointernal/pluginreg/standard_bundle_posture_test.gointernal/pluginreg/custom_backend_prefix_test.gointernal/pluginreg/hosted_backend_build_test.gointernal/pluginreg/resolve_upstream_api_keys_test.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/plugins/backends/huggingface/integration_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/pluginreg/spec_bundle_standard_inventory_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin_config_test.gointernal/pluginreg/standard_bundle_posture_test.gointernal/pluginreg/custom_backend_prefix_test.gointernal/pluginreg/hosted_backend_build_test.gointernal/pluginreg/resolve_upstream_api_keys_test.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/plugins/backends/huggingface/integration_test.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/pluginreg/spec_bundle_standard_inventory_test.gointernal/archtest/backend_lifecycle_contract_test.gointernal/plugins/backends/huggingface/doc.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin_config_test.gointernal/plugins/backends/huggingface/plugin.gointernal/pluginreg/standard_bundle_posture_test.gointernal/pluginreg/standard_table.gointernal/pluginreg/custom_backend_prefix_test.gointernal/pluginreg/backends_misc.gointernal/pluginreg/hosted_backend_build_test.gointernal/pluginreg/keys.gointernal/pluginreg/resolve_upstream_api_keys_test.gointernal/pluginreg/custom_backends.gointernal/pluginreg/backend_prefix_inventory_test.gointernal/plugins/backends/huggingface/integration_test.go
internal/plugins/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Keep provider-specific payload types inside adapters/plugins
Files:
internal/plugins/backends/huggingface/doc.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin_config_test.gointernal/plugins/backends/huggingface/plugin.gointernal/plugins/backends/huggingface/integration_test.go
internal/plugins/backends/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Use official vendor Go SDKs only inside backend plugins (OpenAI:
openai-go); do not import provider SDKs frominternal/core,pkg/lipapi, orpkg/lipsdk
Files:
internal/plugins/backends/huggingface/doc.gointernal/plugins/backends/huggingface/inventory_test.gointernal/plugins/backends/huggingface/plugin_config_test.gointernal/plugins/backends/huggingface/plugin.gointernal/plugins/backends/huggingface/integration_test.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/model-inventory-proof/main.go
🔇 Additional comments (12)
cmd/model-inventory-proof/main.go (1)
18-18: LGTM!Also applies to: 101-127
internal/pluginreg/standard_table.go (1)
12-12: LGTM!Also applies to: 187-189
internal/archtest/backend_lifecycle_contract_test.go (1)
23-23: LGTM!internal/pluginreg/spec_bundle_standard_inventory_test.go (1)
38-38: LGTM!internal/pluginreg/standard_bundle_posture_test.go (1)
3-12: LGTM!internal/pluginreg/keys.go (1)
22-22: LGTM!Also applies to: 66-68, 77-77, 113-115
internal/pluginreg/resolve_upstream_api_keys_test.go (1)
20-20: LGTM!Also applies to: 180-200
internal/pluginreg/backends_misc.go (1)
12-12: LGTM!Also applies to: 98-113
internal/plugins/backends/huggingface/plugin_config_test.go (1)
13-53: LGTM!internal/pluginreg/custom_backend_prefix_test.go (1)
29-29: LGTM!internal/pluginreg/backend_prefix_inventory_test.go (1)
71-71: LGTM!Also applies to: 92-92
config/config.yaml (1)
279-283: LGTM!Also applies to: 360-360
| - Composition roots: [`pluginreg.InstallStandardBundleOn`](internal/pluginreg/standard_table.go) / [`InstallStandardBackendsOn`](internal/pluginreg/standard_table.go) take [`pluginreg.UpstreamAPIKeys`](internal/pluginreg/keys.go) (per-family **ordered slices**; use [`ResolveUpstreamAPIKeysFromEnv`](internal/pluginreg/keys.go) in `main`, or `UpstreamAPIKeys{}` in tests). Env resolution includes numbered keys (`OPENAI_API_KEY_2`, … **contiguous** suffixes only: scanning stops at the first missing or empty `_N`, so a gap like `_2` unset while `_3` is set will not load `_3`; same pattern for Anthropic/Gemini/OpenRouter/NVIDIA/Hugging Face). Hosted backend YAML accepts optional `api_keys` alongside `api_key` (see [`config/config.multi-instance.example.yaml`](config/config.multi-instance.example.yaml)). [`runtime.New`](internal/core/runtime/app.go), [`runtimebundle.Build`](internal/infra/runtimebundle/build.go), [`stdhttp.Run`](internal/stdhttp/server.go), and [`stdhttp.RunWithRuntime`](internal/stdhttp/server.go) require a **non-nil** `*slog.Logger`. [`pluginreg.(*Registry).RegisterBackend`](internal/pluginreg/reg.go) factories return [`execbackend.Backend`](internal/core/execbackend/backend.go) directly (not `any`). [`sqlitestore.New`](internal/core/continuity/sqlitestore/store.go) accepts an existing `*sql.DB` for tests. | ||
| - **Hosted provider backends (multi-key pools)** — The bundled `openai-responses`, `openai-legacy`, `anthropic`, `gemini`, `openrouter`, `nvidia`, and `huggingface` backends keep ordered credentials per instance, classify pre-output 401/429 from the official SDKs, and may return [`lipapi.RecoverablePreOutputError`](pkg/lipapi/upstream.go) from [`execbackend.Backend.Open`](internal/core/execbackend/backend.go) when no key is usable before the first canonical stream event (including single-key rate limit or auth failure). They do not return an [`lipapi.EventStream`](pkg/lipapi/events.go) that fails only during [`lipapi.Collect`](pkg/lipapi/events.go) for that case. **401 handling:** HTTP 401 from the hosted upstream is treated as **permanently invalid** for that credential inside the process (the pool marks it unusable until restart); this matches static API keys but is not suitable for short-lived tokens that might recover without a restart. **429 / Gemini:** OpenAI and Anthropic read `Retry-After` from the SDK error where available; the genai client does not attach response headers to [`genai.APIError`](https://pkg.go.dev/google.golang.org/genai#APIError), so the Gemini adapter also reads [`google.rpc.RetryInfo`](https://cloud.google.com/apis/design/errors#error_details) from error JSON `details` when present, otherwise it uses a conservative fixed cooldown fallback. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
README now disagrees with itself about supported env key families.
These lines add Hugging Face correctly, but the earlier cmd/lipstd startup-flow bullet in this same README still omits HUGGINGFACE_API_KEY / numbered suffixes. That leaves two conflicting descriptions of what ResolveUpstreamAPIKeysFromEnv loads. Please update the earlier bullet too so operators do not miss the new credential family.
🤖 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 `@README.md` around lines 87 - 88, README has inconsistent documentation for
ResolveUpstreamAPIKeysFromEnv because the earlier startup-flow bullet still
omits the Hugging Face key family. Update that existing bullet to mention
HUGGINGFACE_API_KEY and its numbered suffixes, matching the later Composition
roots note, so both descriptions list the same supported env key families and
ordering rules.
1 participant
Summary
Verification
Notes