Skip to content

fix(setup): use /v1/models API only and support Cohere response shape#6

Closed
siddharthsambharia-portkey wants to merge 1 commit into
mainfrom
feat/remove-hardcoded-models
Closed

fix(setup): use /v1/models API only and support Cohere response shape#6
siddharthsambharia-portkey wants to merge 1 commit into
mainfrom
feat/remove-hardcoded-models

Conversation

@siddharthsambharia-portkey
Copy link
Copy Markdown
Collaborator

Summary

  • Parse both Portkey { data: [{ id }] } and provider-native { models: [{ name, endpoints }] } responses from GET /v1/models (fixes Cohere showing “no models” while curl works).
  • Remove hardcoded MODEL_DEFAULTS merged into the setup picker — the list now reflects only what the catalog API returns.
  • Show non-chat models with an inline badge (e.g. embed-v4.0 · embed) so embed/transcribe rows stay visible but are clearly not for chat.
  • portkey verify requires a configured model instead of a hardcoded fallback.

Test plan

  • Anthropic provider with N enabled models → picker shows exactly N (+ Other), no extra hardcoded names
  • Cohere provider (@cohere-*) → “Found 20 models”, embed rows show · embed
  • Empty /v1/models response → manual model ID entry
  • Claude Code setup → optional Sonnet/Opus/Haiku step; Yes → three model pickers
  • npm test passes

Made with Cursor

- Updated `fetchModels` to support additional model properties (name, endpoints) for better compatibility with various providers.
- Improved `testGatewayConnection` to require a model parameter, ensuring proper validation.
- Refactored `doSetup` to streamline model selection and user guidance, removing outdated fallback suggestions and enhancing error messages.
- Added tests for new model parsing logic in `fetchModels` to ensure correct functionality.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Portkey CLI setup/verification flow to rely exclusively on the /v1/models catalog endpoint, including support for provider-native model list shapes (notably Cohere), and removes hardcoded model suggestions so the picker reflects only what the API returns.

Changes:

  • Extend fetchModels() to parse both { data: [...] } and { models: [{ name, endpoints }] } shapes and surface a non-chat “hint” derived from endpoints.
  • Remove hardcoded model defaults from the interactive setup model picker and display non-chat models with an inline badge (e.g. · embed).
  • Make portkey verify require an explicitly configured model (no hardcoded fallback).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
tests/api.test.js Adds coverage for Cohere-style /v1/models response parsing and endpointshint mapping.
src/api.js Updates model catalog parsing logic and adds a required model parameter to the gateway connectivity test helper.
src/commands/claude-code/setup.js Removes curated model defaults and updates the model selection UX to use only catalog results + inline hints.
src/commands/claude-code/verify.js Removes the hardcoded test model and requires a configured model before verifying.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const existing = readExistingConfig();
const gateway = process.env.ANTHROPIC_BASE_URL || existing.gateway || PORTKEY_GATEWAY;
const testModel = existing.model || "claude-haiku-4-20250514";
const testModel = existing.model;
if (targetAgent === "codex" && mode === "config" && !String(model || "").trim()) {
if (args.yes) {
model = args.model || "gpt-4o";
model = args.model || "";
Comment on lines 450 to 453
const manualFirst = !permDenied && catalogUnavailable && merged.length > 0;
const skipSelect =
permDenied || (catalogUnavailable && merged.length === 0);
permDenied || (catalogUnavailable && merged.length === 0) || merged.length === 0;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants