feat(providers): add AI/ML API preset#402
Conversation
|
hey thanks for making pr @D1m7asis |
There was a problem hiding this comment.
Pull request overview
Adds AI/ML API as a first-class, OpenAI-compatible provider preset in Zero, including setup documentation, catalog placement, runtime/model-discovery attribution headers, and regression tests across CLI + TUI flows.
Changes:
- Adds
aimlapiprovider descriptor (base URLhttps://api.aimlapi.com/v1, envAIMLAPI_API_KEY, default modelopenai/gpt-5-chat) and updates catalog ordering expectations. - Injects AI/ML API attribution headers for both runtime provider requests and
/modelsdiscovery requests (scoped to catalog idaimlapi). - Documents direct setup and updates/extends tests for catalog onboarding, provider factory, model discovery, and the TUI provider wizard.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents AIMLAPI_API_KEY and zero providers setup aimlapi --set-active. |
| internal/tui/provider_wizard_test.go | Makes wizard selection tests robust to catalog order changes by selecting providers by ID. |
| internal/providers/providerio/headers.go | Adds catalog-scoped header injection helper for AIMLAPI attribution headers. |
| internal/providers/factory.go | Applies AIMLAPI attribution headers to OpenAI/OpenAI-compatible provider construction. |
| internal/providers/factory_test.go | Adds regression test asserting AIMLAPI attribution headers on runtime requests. |
| internal/providermodeldiscovery/discovery.go | Applies AIMLAPI attribution headers to OpenAI-compatible model discovery requests. |
| internal/providermodeldiscovery/discovery_test.go | Adds regression test asserting AIMLAPI attribution headers on discovery requests. |
| internal/providercatalog/catalog.go | Adds aimlapi preset immediately after the recommended OpenGateway entry. |
| internal/providercatalog/catalog_test.go | Updates expected catalog IDs and ordering; verifies AIMLAPI descriptor defaults. |
| internal/cli/provider_catalog_onboarding_test.go | Verifies onboarding output includes AIMLAPI requirements and setup hint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func HeadersForCatalog(catalogID string, headers map[string]string) map[string]string { | ||
| copied := CopyHeaders(headers) | ||
| if !strings.EqualFold(strings.TrimSpace(catalogID), "aimlapi") { | ||
| return copied | ||
| } | ||
| if copied == nil { | ||
| copied = map[string]string{} | ||
| } | ||
| copied["X-AIMLAPI-Partner-ID"] = "Gitlawb" | ||
| copied["X-AIMLAPI-Integration-Repo"] = "Gitlawb/zero" | ||
| copied["X-AIMLAPI-Integration-Version"] = "1.0.0" | ||
| return copied | ||
| } |
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
There was a problem hiding this comment.
Verdict: Approve ✅ — correct, well-scoped, and it doesn't touch other providers
Reviewed the header plumbing carefully (that's the part that could regress every provider, not just this one). It's clean.
Header injection is correctly scoped
HeadersForCatalog(catalogID, headers) only adds the X-AIMLAPI-* attribution headers when catalogID == "aimlapi"; every other provider gets CopyHeaders(headers) unchanged. I checked CopyHeaders(nil) == nil, so non-aimlapi providers keep identical behavior — and now get a defensive copy of their custom headers instead of the aliased map, which is actually a small safety win. It's applied consistently to both runtime (factory.go) and discovery (discovery.go), so the attribution can't diverge between the two paths.
Catalog + tests are solid
- Descriptor is a standard
openAICompatentry;expectedCatalogIDsand theTransportOpenAICompatorder list are both updated to match the insertion point. - Good coverage: descriptor fields, onboarding auth hint, factory header injection (asserting the real endpoint +
Authorization+ attribution + a user's ownX-Tracesurvive), and discovery header injection. - Nice test-hygiene call: the two wizard tests were refactored from brittle "press Down N times" navigation to
providerWizardProviderIndex(...)(id-based) — exactly right, since inserting a provider shifts the list and would otherwise silently break those tests. Verified the helper is defined and compiles.
Local: go build ./..., go vet, gofmt clean; providers, providercatalog, providermodeldiscovery, providerio, tui (wizard), and cli (catalog onboarding) all pass.
Two things for your conscious call (non-blocking)
- Placement.
aimlapiis inserted as the 2nd catalog entry — right after the recommended OpenGateway and above first-partyopenai/anthropic/google. So a third-party aggregator sits near the top of the provider picker. That's a product/ordering decision, not a correctness issue — if you'd rather it not outrank the first-party providers, moving it down next to the other aggregators (openrouter/groq) is a one-line reorder (plus the two test order-lists). - Default model
openai/gpt-5-chat. I didn't independently confirm this id against live AI/ML API — a wrong default 400s on first run. Worth a quick check that it's a currently-served id (their model names are provider-prefixed, so the shape is right). Not blocking per the "live ids lag the manifest" norm.
Also fine: the attribution headers hardcode Partner-ID: Gitlawb / Gitlawb/zero / version 1.0.0 — that's the project's own partner attribution (like OpenRouter's X-Title), which is the intended behavior for a first-class preset.
Approving. Thanks @D1m7asis — clean, well-tested integration.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new built-in ChangesAI/ML API provider preset
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/providers/providerio/headers.go (1)
66-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the AIMLAPI catalog ID into a shared constant
internal/providers/providerio/headers.goandinternal/providercatalog/catalog.goboth hardcode"aimlapi", so a rename or alias change would silently stop adding the attribution headers.provideriocan importprovidercataloghere without a cycle, so a shared exported constant would make the coupling explicit.- Add a negative test for
HeadersForCatalogwith a different catalog ID (for example"groq") to lock in the match behavior.🤖 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/providers/providerio/headers.go` around lines 66 - 78, The AIMLAPI catalog ID is hardcoded in both HeadersForCatalog and the catalog definition, so extract it into a shared exported constant from providercatalog and use that in providerio instead of duplicating "aimlapi". Update HeadersForCatalog to reference the shared symbol so the attribution-header check stays in sync with catalog changes. Add a negative test for HeadersForCatalog using a non-AIMLAPI catalog ID such as "groq" to verify headers are not added for other providers.
🤖 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.
Nitpick comments:
In `@internal/providers/providerio/headers.go`:
- Around line 66-78: The AIMLAPI catalog ID is hardcoded in both
HeadersForCatalog and the catalog definition, so extract it into a shared
exported constant from providercatalog and use that in providerio instead of
duplicating "aimlapi". Update HeadersForCatalog to reference the shared symbol
so the attribution-header check stays in sync with catalog changes. Add a
negative test for HeadersForCatalog using a non-AIMLAPI catalog ID such as
"groq" to verify headers are not added for other providers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a9aeaa8e-ec1b-4e48-8d9c-bd3f7fff5c40
📒 Files selected for processing (10)
README.mdinternal/cli/provider_catalog_onboarding_test.gointernal/providercatalog/catalog.gointernal/providercatalog/catalog_test.gointernal/providermodeldiscovery/discovery.gointernal/providermodeldiscovery/discovery_test.gointernal/providers/factory.gointernal/providers/factory_test.gointernal/providers/providerio/headers.gointernal/tui/provider_wizard_test.go
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Approving.
Genuinely additive — no existing descriptor, ID, base URL, or order entry is mutated. The one shared-plumbing touch (routing all providers through HeadersForCatalog) is proven behavior-preserving: for any non-aimlapi catalog ID it returns CopyHeaders(headers), and CopyHeaders(nil) == nil, so nil-header providers are byte-identical to before. Verified build / vet / test green across the touched packages.
Two non-blocking nits (both from the bots): the "aimlapi" literal is duplicated between the header match and the catalog descriptor — worth a shared const + a negative test so a future rename can't silently drop the attribution header — and the map is copied even for non-aimlapi providers (micro-allocation). Optional polish, not blockers.
|
hello @D1m7asis kindly rebase to main and fix conflicts please |
…ider # Conflicts: # README.md # internal/cli/provider_catalog_onboarding_test.go # internal/providercatalog/catalog_test.go
|
Updated the branch by merging current upstream Also applied the non-blocking review polish:
Validation:
The only local failure remains the unrelated flaky @kevincodex1 conflicts are resolved and the branch is ready for re-review. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/providers/providerio/headers_test.go (1)
28-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using the shared catalog ID constant instead of a literal.
" AIMLAPI "is hardcoded here rather than referencingprovidercatalog.AIMLAPIID. Since the PR introduces that exported constant specifically for this purpose, using it (with whitespace/case variations appended) would keep the test coupled to the actual ID rather than a duplicated literal.♻️ Proposed tweak
- got := HeadersForCatalog(" AIMLAPI ", headers) + got := HeadersForCatalog(" "+strings.ToUpper(providercatalog.AIMLAPIID)+" ", headers)🤖 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/providers/providerio/headers_test.go` at line 28, The test for HeadersForCatalog is hardcoding the catalog ID string instead of using the shared constant. Update the assertion setup in headers_test.go so it references providercatalog.AIMLAPIID and applies the whitespace/case variations to that value, keeping the test aligned with the exported catalog identifier used by HeadersForCatalog.
🤖 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.
Nitpick comments:
In `@internal/providers/providerio/headers_test.go`:
- Line 28: The test for HeadersForCatalog is hardcoding the catalog ID string
instead of using the shared constant. Update the assertion setup in
headers_test.go so it references providercatalog.AIMLAPIID and applies the
whitespace/case variations to that value, keeping the test aligned with the
exported catalog identifier used by HeadersForCatalog.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4712540b-6bc3-4820-8c41-1d56c91b1339
📒 Files selected for processing (7)
README.mdinternal/cli/provider_catalog_onboarding_test.gointernal/providercatalog/catalog.gointernal/providercatalog/catalog_test.gointernal/providers/providerio/headers.gointernal/providers/providerio/headers_test.gointernal/tui/provider_wizard_test.go
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (5)
- internal/providercatalog/catalog.go
- internal/providers/providerio/headers.go
- internal/cli/provider_catalog_onboarding_test.go
- internal/tui/provider_wizard_test.go
- internal/providercatalog/catalog_test.go
|
Addressed the remaining CodeRabbit nit in 34ab360: the case/whitespace normalization test now derives its input from the shared providercatalog.AIMLAPIID constant instead of duplicating the catalog ID. Targeted providerio and providercatalog tests pass. Ready for re-review. |
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
VERDICT: approve
REGRESSION RISK: low
Re-review of new commits since my prior review (c844cbbb → 34ab3606). The new commits merge main and reuse the aimlapi catalog id consistently (addressing the prior review's finding about catalog ID consistency). The PR also rebased onto main, picking up unrelated changes. The catalog ID fix is correct — the provider now uses a single consistent id throughout.
BUILD / TEST
- CI all green (macOS, Ubuntu, Windows, Smoke, Security, Zero Review)
FINDINGS
None. The catalog ID consistency issue from the prior review has been addressed.
BOTTOM LINE
The catalog ID consistency fix addresses the prior review's finding. Safe to merge.
Summary
AIMLAPI_API_KEY, the fixedhttps://api.aimlapi.com/v1endpoint, andopenai/gpt-5-chatas the starter modelLinked issue
Fixes #401
Testing
go build ./...go vet ./...git diff --checkgo test ./...(all changed packages pass; existing OAuth callback timing tests ininternal/cliandinternal/mcptime out on this Windows environment)Checklist
issue-approvedlabel. (approval requested)go build ./...,go vet ./..., andgo test ./...pass locally. (build/vet and changed packages pass; unrelated OAuth timing tests time out)gofmtclean.Summary by CodeRabbit
AIMLAPI_API_KEY, plus added a one-command setup option to activate aimlapi.