fix(catalog): carry Together /models array fix onto dev2-go (#639) - #682
Conversation
* fix(catalog): accept Together top-level /models arrays Closes lidge-jun#617 * fix(catalog): keep models[] for probe only; array+data for discovery (cherry picked from commit 9d181fc)
…#639) Align FetchProviderModels with the TS oracle: accept top-level arrays and {data:[...]}, reject stray {models} for catalog discovery.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Wibias
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 549c4bcb92
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err := json.Unmarshal(trimmed, &items); err != nil { | ||
| return nil, false | ||
| } | ||
| return items, true |
There was a problem hiding this comment.
When a top-level /models array contains null, {}, or an entry whose id is blank, unmarshalling into []ProviderModelsAPIItem succeeds and this branch reports the payload as valid. FetchProviderModels then skips those rows, marks discovery successful, and caches an empty or partial authoritative catalog instead of retaining the stale/configured fallback; the TypeScript path rejects the entire malformed list. Validate every decoded item has a nonblank ID before returning success.
Useful? React with 👍 / 👎.
| const data = json !== null && typeof json === "object" && !Array.isArray(json) | ||
| ? (json as { data?: unknown }).data | ||
| : undefined; | ||
| const data = providerModelsListFromResponse(json); |
There was a problem hiding this comment.
The new Bun test exercises POST /api/providers/test, which calls providerModelsListFromProbeResponse; it never reaches the changed fetchProviderModels path or providerModelsListFromResponse. The TypeScript catalog behavior this change is intended to deliver can therefore regress while the added test remains green. Add a top-level-array case alongside the existing catalog discovery tests, exercising gatherRoutedModels or fetchProviderModels directly.
AGENTS.md reference: src/AGENTS.md:L22-L25
Useful? React with 👍 / 👎.
|
Merged. Cross-platform CI had known dig2-go flakes unrelated to this change ( Go decisionPorted now in this PR ( |
1 participant
Summary
dev2-go(TS catalog + connectivity probe + tests).9d181fc4ondev.FetchProviderModels.Go port
Ported now —
go/internal/codex/catalog_provider_fetch.goaccepts top-level/modelsarrays (Together AI) and{data:[...]}, matching the TS oracle. Catalog discovery still rejects stray{models:[...]}(probe-only on TS).Management probe on Go uses the injected
fetchModelspath (catalog fetch), so the Go catalog fix covers the #617 discovery failure mode.Test plan
9d181fc4appliedgo test ./internal/codex/ -run FetchProviderModels|ProviderModelsAPIItemsbun test ./tests/provider-connection-test.test.tsdev2-go