feat(snowflake-cortex): close model-coverage gap vs Snowflake docs (#851)#866
Conversation
) Closes #851. altimate-code hardcodes the Snowflake Cortex model list in `provider.ts` (it doesn't auto-discover via `/v1/models` — Snowflake's endpoint has non-standard semantics that break standard clients). The list had drifted behind Snowflake's regional availability matrix, so several models that work on Cortex couldn't be selected from the picker — most visibly `claude-opus-4-7`, which prompted the report. Adds the 8 missing models per Snowflake's current regional availability matrix: - Claude: `claude-opus-4-7` (toolcall: true) - OpenAI: `openai-gpt-5.1`, `openai-gpt-5.2` (toolcall: true) - Llama: `llama4-scout`, `llama3.3-70b`, `snowflake-llama-3.1-405b` (toolcall: false) - Mistral: `mixtral-8x7b` (toolcall: false) - Gemini: `gemini-3.1-pro` (toolcall: false — tool calling unverified on Cortex; conservative default per the rule that only Claude+OpenAI reliably support tools today) Also updates `TOOLCALL_MODELS` in `snowflake.ts` to include the new Claude and OpenAI entries so request transform doesn't strip tools. Documents the config-extension escape hatch for the next drift cycle: users can add a model under `provider['snowflake-cortex'].models` in `opencode.json` and it merges into the built-in list (mechanism wired at provider.ts:1320-1399). New section in `docs/docs/configure/providers.md` shows the pattern so users don't need to fork or wait for a release when Snowflake adds a model. Tests: - `models added per Snowflake regional availability docs (issue #851)` asserts every newly-added model is defined with the right toolcall capability. - `user can register a model not in the hardcoded list via opencode.json` exercises the config-extension path end-to-end. 50 → 52 tests pass in `test/provider/snowflake.test.ts`; typecheck clean. Longer-term, true auto-discovery (a Snowflake-specific shim around `POST /api/v2/cortex/v1/models` or the GET-with-body variant) would eliminate drift entirely — flagged on the issue for maintainer input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates the Snowflake Cortex model catalog, derives a runtime tool-capable set from provider.models, changes transformSnowflakeBody to accept that set and wires it into the Snowflake plugin, updates docs for local model registration, and expands unit and E2E tests. ChangesSnowflake Cortex Model Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/opencode/src/provider/provider.ts`:
- Around line 1053-1060: The Snowflake Cortex token limits for the models
registered with makeSnowflakeModel are incorrect: update the entries for
"openai-gpt-5.1" and "openai-gpt-5.2" so their context and output values match
Snowflake documentation (context 272000, output 8192) instead of the current
1047576 and 32768; locate the two invocations of makeSnowflakeModel for the keys
"openai-gpt-5.1" and "openai-gpt-5.2" in provider.ts and change the numeric
parameters to the documented values (or add a concise inline comment explaining
an intentional divergence if you must keep the current numbers).
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8e57d713-612d-4fcc-af34-19c8a7427773
📒 Files selected for processing (4)
docs/docs/configure/providers.mdpackages/opencode/src/altimate/plugin/snowflake.tspackages/opencode/src/provider/provider.tspackages/opencode/test/provider/snowflake.test.ts
There was a problem hiding this comment.
2 issues found across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Addresses the consensus code review on PR #866. Two critical bugs found by independent verification against Snowflake's regional availability docs (https://docs.snowflake.com/en/user-guide/snowflake-cortex/aisql-regional-availability): 1. **Correct context/output limits on every newly-added model.** The initial commit inferred limits from sibling models; cross-check against the doc showed most were wrong, and `snowflake-llama-3.1-405b` was off by 16× on context (would silently fail any prompt above 8k tokens). Updated values: - `claude-opus-4-7`: 200000/32000 → 1000000/128000 - `openai-gpt-5.1`: 1047576/32768 → 272000/8192 - `openai-gpt-5.2`: 1047576/32768 → 272000/8192 (not in restrictions table; using gpt-5 family defaults with a comment) - `llama4-scout`: 1048576/4096 → 128000/8192 - `llama3.3-70b`: 128000/4096 → 128000/8192 - `snowflake-llama-3.1-405b`: 128000/4096 → 8000/8192 - `mixtral-8x7b`: 32000/4096 → 32000/8192 - `gemini-3.1-pro`: 1048576/8192 → 1000000/64000 2. **Fix the documented opencode.json escape hatch for tool-capable user-added models.** The `TOOLCALL_MODELS` hardcoded set in `snowflake.ts` was a second source of truth that the picker capability had to be kept in sync with. Worse, the docs change in the first commit told users they could mark a custom model `"tool_call": true` — but the request transform consulted only the static set, so user-added models had `tools` silently stripped at request time even though the picker advertised them as tool-capable. Replaced the hardcoded set with `buildToolCapableSet(models)` — the loader walks `provider.models` (which already includes any models the user registered via `opencode.json`) and derives the allowlist from `capabilities.toolcall`. Single source of truth; the picker and the transform can't drift; the escape hatch works. `transformSnowflakeBody` now takes an explicit allowlist parameter; the loader passes the dynamically-built set. Existing call sites updated to pass a shared `TOOLCAPABLE_FIXTURE`. Also folded into this commit: - Comments distinguishing `llama3.3-70b` (upstream Meta) from `snowflake-llama-3.3-70b` (Snowflake-hosted variant). - Doc note clarifying the `tool_call` snake_case field maps to `capabilities.toolcall`. - The "models added per Snowflake regional availability docs (issue #851)" regression test now asserts `limit.context` and `limit.output` per model in addition to identity and toolcall capability. - New tests: - `buildToolCapableSet derives the allowlist from provider model capabilities` — parity between picker and transform. - `escape-hatch model with tool_call: true retains tools through transformSnowflakeBody` — end-to-end check the escape hatch actually works for tool-capable user models. - `escape-hatch model with tool_call: false has tools stripped through transformSnowflakeBody` — the inverse, mirroring the built-in Llama/Mistral behavior for user-registered non-tool models. Deferred (per maintainer guidance pending @anandgupta42's input on the long-term auto-discovery question): - Removing the legacy `claude-3-7-sonnet`, `claude-3-5-sonnet`, `openai-gpt-5-chat` entries — kept per the original PR description. - Auto-discovery shim around Snowflake's `/api/v2/cortex/v1/models` — out of scope until maintainer green. Tests: typecheck clean across all 5 packages; 55/55 in `test/provider/snowflake.test.ts` (was 52, +3 net for new coverage). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/opencode/test/provider/snowflake.test.ts (1)
608-611: ⚡ Quick winUse
tmpdir({ config })instead of writingopencode.jsoninsideinit.These cases are only seeding project config, so the dedicated
configoption is the simpler and preferred test setup path here.As per coding guidelines, "Use the
configoption to write anopencode.jsonconfiguration file when tests need project configuration".Also applies to: 655-658, 684-701, 734-750
🤖 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 `@packages/opencode/test/provider/snowflake.test.ts` around lines 608 - 611, The test currently seeds an opencode.json by calling tmpdir({ init: async (dir) => Bun.write(path.join(dir, "opencode.json"), ... ) })—replace this pattern with the tmpdir helper's built-in config option: call tmpdir({ config: { $schema: "https://altimate.ai/config.json" } }) (and similarly for the other occurrences noted) so the framework writes opencode.json for you instead of using the init callback; update the uses of tmpdir, remove the init write logic, and keep the tmp variable and await using pattern intact.
🤖 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 `@packages/opencode/test/provider/snowflake.test.ts`:
- Line 126: The tests around transformSnowflakeBody using TOOLCAPABLE_FIXTURE
currently rely on generic ids like "llama3.3-70b" / "snowflake-llama-3.3-70b"
which can pass even when the wrong catalog id is used; update the test
input/fixture so the model id is a provider-backed Snowflake catalog id instead
of the generic name. Locate where transformSnowflakeBody is called with
TOOLCAPABLE_FIXTURE (and any other occurrences around lines 265) and replace the
model id in the fixture or test input with the real provider-backed Snowflake
model id constant or literal from your catalog (the Snowflake model entry),
ensuring the test exercises the actual Snowflake model entry rather than an
unknown fallback.
---
Nitpick comments:
In `@packages/opencode/test/provider/snowflake.test.ts`:
- Around line 608-611: The test currently seeds an opencode.json by calling
tmpdir({ init: async (dir) => Bun.write(path.join(dir, "opencode.json"), ... )
})—replace this pattern with the tmpdir helper's built-in config option: call
tmpdir({ config: { $schema: "https://altimate.ai/config.json" } }) (and
similarly for the other occurrences noted) so the framework writes opencode.json
for you instead of using the init callback; update the uses of tmpdir, remove
the init write logic, and keep the tmp variable and await using pattern intact.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3dbccef4-a8c4-42d9-83d1-cd74e1f68b76
📒 Files selected for processing (5)
docs/docs/configure/providers.mdpackages/opencode/src/altimate/plugin/snowflake.tspackages/opencode/src/provider/provider.tspackages/opencode/test/altimate/cortex-snowflake-e2e.test.tspackages/opencode/test/provider/snowflake.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- docs/docs/configure/providers.md
- packages/opencode/src/provider/provider.ts
Reviewer caught my providers.md addition referring to `opencode.json` (and `.opencode/opencode.json`). Both names are accepted by the config loader (config.ts:1541 candidates: `altimate-code.json`, `opencode.jsonc`, `opencode.json`, `config.json`) but the rest of the docs standardize on `altimate-code.json` — `docs/docs/configure/config.md` uses the canonical name 9× and `opencode.json` 0×. My section was the inconsistent one. Root cause: I used `opencode.json` because that's the upstream project's name and what the snowflake tests happen to use. But this is the altimate-code fork; user-facing docs use the fork's canonical name. Updates `providers.md` "Adding a model not in the list" section and the three `altimate_change`-introduced comments in `snowflake.ts` to use `altimate-code.json` consistently. Tests left using `opencode.json` literals — they're exercising the loader (which accepts both) and swapping them would be churn without behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Bot review nit from CodeRabbit on the second review pass: tests added
in this PR were calling `tmpdir({ init: async (dir) => Bun.write(...) })`
to seed a project config, but the fixture already exposes a dedicated
`config` option that does exactly that. Switching to the helper is
shorter, matches the codebase guideline, and centralizes the
`$schema` boilerplate.
Affects the 5 tests added by this PR:
- models added per Snowflake regional availability docs (issue #851)
- buildToolCapableSet derives the allowlist from provider model
capabilities
- user can register a model not in the hardcoded list via
altimate-code.json (also renamed from `via opencode.json` for
canonical consistency with the docs)
- escape-hatch model with tool_call: true retains tools through
transformSnowflakeBody
- escape-hatch model with tool_call: false has tools stripped through
transformSnowflakeBody
Pre-existing tests in the same file follow the old pattern; left
out of scope to avoid churn in unrelated code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
dev-punia-altimate
left a comment
There was a problem hiding this comment.
Multi-Persona Review — Verdict: ship
The PR successfully updates Snowflake Cortex model support to match regional availability, introduces a secure config-driven escape hatch for future model additions, and fixes tool-calling consistency by replacing a hardcoded allowlist with a dynamic configuration-based system. All changes are well-tested, aligned with industry best practices, and validated by adversarial testing and web research. No critical or high-severity issues were found.
14/14 agents completed · 233s · 4 findings (0 critical, 0 high, 2 medium)
Medium
- [web-researcher] PR implements dynamic tool-call detection via config, mitigating CVE-2026-12345 risk from static allowlists in AI proxies. →
packages/opencode/src/altimate/plugin/snowflake.ts:113- 💡 Ensure tool_call flag validation is enforced at config load time to prevent runtime injection.
- [web-researcher, adversarial-tester] Snowflake's /v1/models endpoint has non-standard semantics, justifying the decision to avoid auto-discovery and use config-driven model list. →
packages/opencode/src/provider/provider.ts:1050- 💡 Document this limitation in the code comments to guide future maintainers.
Low
- [web-researcher] Using config-driven model list (altimate-code.json) aligns with industry best practices for extensible AI plugins. →
docs/docs/configure/providers.md:280- 💡 Consider adding a schema validation step for user-provided model configs to prevent malformed entries.
- [pr-hygiene] Large PR size (361 additions) but all changes are cohesive and interdependent; splitting would risk breaking consistency.
- 💡 Future similar updates could be broken into smaller PRs if model additions become more frequent.
Multi-Persona Review · vllm:qwen3-next-80b (waves) + vllm-fallback (synth) ·
| @@ -97,6 +113,14 @@ export async function SnowflakeCortexAuthPlugin(_input: PluginInput): Promise<Ho | |||
| model.cost = { input: 0, output: 0, cache: { read: 0, write: 0 } } | |||
There was a problem hiding this comment.
[MEDIUM · web-researcher] PR implements dynamic tool-call detection via config, mitigating CVE-2026-12345 risk from static allowlists in AI proxies.
💡 Suggestion: Ensure tool_call flag validation is enforced at config load time to prevent runtime injection.
Confidence: 90/100
| @@ -280,15 +280,39 @@ Billing flows through your Snowflake credits — no per-token costs. | |||
|
|
|||
There was a problem hiding this comment.
[LOW · web-researcher] Using config-driven model list (altimate-code.json) aligns with industry best practices for extensible AI plugins.
💡 Suggestion: Consider adding a schema validation step for user-provided model configs to prevent malformed entries.
Confidence: 85/100
| @@ -1049,6 +1050,16 @@ export namespace Provider { | |||
| }), | |||
There was a problem hiding this comment.
[MEDIUM · web-researcher, adversarial-tester] Snowflake's /v1/models endpoint has non-standard semantics, justifying the decision to avoid auto-discovery and use config-driven model list.
💡 Suggestion: Document this limitation in the code comments to guide future maintainers.
Confidence: 95/100
Three valid findings from cubic + a human reviewer's auto-discovery
documentation request, addressed in one commit:
1. **`snowflake-llama-3.1-405b` output > context (cubic P2 on
provider.ts:1110).** Snowflake's docs list output=8192 for this
model, but its context is 8000. Output can never physically exceed
the total token budget, so a max_tokens=8192 request would either
be rejected or implicitly capped by the gateway. Capped at 4096
(the original sibling default), with a comment explaining the
apparent doc inconsistency.
2. **Aliased models bypass the tool-call allowlist (cubic P2 on
snowflake.ts:19).** `buildToolCapableSet` indexed by the picker map
key, but the request body sends `model: <api.id>`. A user
registering an alias like:
"my-claude-alias": { "id": "claude-opus-4-7", "tool_call": true }
would have tools silently stripped because "my-claude-alias" !=
"claude-opus-4-7" in the set. Updated the helper to also include
`m.id` and `m.api?.id` so the lookup matches whichever form the
transform sees. Added a regression test that exercises the alias
path end-to-end (config registration → loader-built set → request
transform with the api.id form).
3. **Document why auto-discovery isn't used (web-researcher
suggestion on provider.ts:1050).** Added a comment near the
snowflake-cortex registration explaining that Cortex's models
endpoint (`GET /api/v2/cortex/v1/models`) has non-standard
semantics (requires JSON body on GET) that break standard
OpenAI-compatible clients — so the hardcoded list is the source of
truth until a probe-able shim exists.
Skipped:
- "CVE-2026-12345" comment from the web-researcher reviewer — the
CVE id is a placeholder/fake (no such advisory exists), suggestion
body is vague. Not actioned.
- Schema-validation hardening for user-provided model configs —
reasonable but broader than this PR's scope.
CI status: the one TypeScript failure on this branch
(`S27: sql_analyze with parse error`) is pre-existing on `main`
HEAD `6ad8b47` — unrelated to snowflake-cortex.
Tests: 55 → 56 in `test/provider/snowflake.test.ts`; typecheck clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
2 similar comments
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
1 similar comment
|
👋 This PR was automatically closed by our quality checks. Common reasons:
If you believe this was a mistake, please open an issue explaining your intended contribution and a maintainer will help you. |
Summary
Closes #851. The Snowflake Cortex model list in
provider.tsis hardcoded (we don't auto-discover via/v1/models— Snowflake's endpoint has non-standard request semantics that break standard clients) and had drifted behind Snowflake's regional availability matrix. Models that work on Cortex couldn't be selected in the picker — most visiblyclaude-opus-4-7, which prompted the report.Models added
claude-opus-4-7openai-gpt-5.1,openai-gpt-5.2llama4-scout,llama3.3-70b,snowflake-llama-3.1-405bmixtral-8x7bgemini-3.1-proTOOLCALL_MODELSinsnowflake.tsupdated to include the new Claude/OpenAI entries so request transform doesn't strip tools.Config-extension escape hatch (documented for the next drift)
The mechanism at
provider.ts:1320-1399already mergesprovider['snowflake-cortex'].modelsfromopencode.jsoninto the built-in list — it was just undocumented. Added a section indocs/docs/configure/providers.mdshowing the pattern so users can add a model without forking or waiting for a release when Snowflake ships something new:{ "provider": { "snowflake-cortex": { "models": { "your-new-model-id": { "name": "Your New Model", "limit": { "context": 200000, "output": 32000 }, "tool_call": true } } } } }Out of scope (deferred)
True auto-discovery — a Snowflake-specific shim around
POST /api/v2/cortex/v1/modelsor the GET-with-body variant — would eliminate drift permanently but is a larger refactor. Flagged on issue #851 for @anandgupta42's input.Test plan
bun run typecheck— 5/5 packages cleanbun test test/provider/snowflake.test.ts— 52/52 pass (was 50; +2 regression cases for newly-added models and the config-extension path)claude-opus-4-7in the extension picker against a Snowflake account that has Opus 4.7 enabled, send a request, verify it routes to the Cortex Chat Completions endpointSummary by cubic
Updates the
snowflake-cortexmodel list to match Snowflake’s regional availability so all supported models appear in the picker and tool calling behaves correctly. Closes #851; builds the tool-capable set from picker capabilities (works with aliases and user-added models) and fixes limits.New Features
claude-opus-4-7(tools on), OpenAIopenai-gpt-5.1,openai-gpt-5.2(tools on), Llamallama4-scout,llama3.3-70b,snowflake-llama-3.1-405b, Mistralmixtral-8x7b, Geminigemini-3.1-pro(tools off).provider["snowflake-cortex"].modelsinaltimate-code.json(entries merge;tool_callmaps tocapabilities.toolcall).Bug Fixes
buildToolCapableSet(provider.models);transformSnowflakeBodynow takes this set and matches both the picker key andid/api.id.snowflake-llama-3.1-405boutput to 4096 since its context is 8000.Written for commit aa1951b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests