fix: preserve Bedrock DeepSeek model ids - #34441
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: Potential Duplicate FoundPR #18948: fix(provider): prevent Bedrock from prefixing ARN and deepseek.v3.2 model IDs This appears to be directly related to the current PR #34441. Both PRs address the same issue of preserving Bedrock DeepSeek model IDs (preventing prefixing/normalization that shouldn't happen). You should check if PR #18948 was previously closed or if this is addressing a regression or incomplete fix. |
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
|
Updated after rechecking the Bedrock DeepSeek R1 profile behavior. This PR now preserves the intended split:
This is related to #18948, but this PR also covers the current Verified locally:
Known unrelated local blockers while testing broader gates:
|
Robin1987China
left a comment
There was a problem hiding this comment.
LGTM. The narrowing from "deepseek" to "deepseek.r1" correctly distinguishes cross-region inference profiles (R1) from on-demand models (V3.2). The arn: guard is a solid defensive addition — foundation-model ARNs should never be region-prefixed.
One non-blocking note: if Bedrock adds DeepSeek R2 models that also require cross-region profiles, the requiresPrefix list will need updating. The narrow match is the safer choice for now.
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
|
Can this be re-opened and merged? It was already reviewed, but now closed. It is still a problem if you are stuck with AWS and need to use bedrock. |
Upstream v1.18.29 (1674747, merged at 18a4845) -> v1.18.30 (3104c14), 28 commits / 105 files. Staged delta vs pre-merge fork HEAD is identical to the upstream tag-to-tag diff (105 files, +1676/-403), confirming a clean adoption. Functional changes: - feat(opencode): port Astra system prompt from v2 (anomalyco#48057) — new session/prompt/gpt-astra.txt wired through system.ts; fork's system transforms live in the openchamber plugin, no overlap. - fix: preserve Bedrock DeepSeek model ids (anomalyco#34441) — provider.ts Bedrock loader passes arn: model ids through unmodified and tightens the cross-region prefix match from "deepseek" to "deepseek.r1"; inside the custom Azure/Bedrock loader region, disjoint from the fork snapshot architecture. - fix(provider): preserve explicit OpenAI service tiers (anomalyco#47671). - fix(opencode): add GitLab reasoning variants (anomalyco#47306) — transform.ts reasoningEffort now routes gitlab gpt-family to reasoningEffort and claude-family to adaptive thinking; disjoint from the fork GLM/ZhiPu normalization block, which remains intact (+77 fork-only lines). - SDK bumps: @ai-sdk/openai 3.0.88, @ai-sdk/azure 3.0.93, gitlab-ai-provider 6.15.0 (8 packages installed via bun install). - console: oauth client metadata document (anomalyco#47737), Chatwoot enterprise form (anomalyco#47488), openai usage normalization + tier threshold (anomalyco#47342), localized refund FAQs, zen model table updates (muse spark); go.mdx client session compatibility docs incl. zh translation. Conflicts resolved: 29 package.json version bumps (rebranded 1.18.30-sscity with a version-generic pattern after the v1.18.27 incident), bun.lock (took upstream, rebranded workspace entries via bun install). Fork deltas verified intact vs v1.18.30: ProviderSnapshot architecture (+492 in provider.ts), deferred snapshot init, GLM normalization (+77 in transform.ts), config getFresh/commitFresh (+14). Tests: provider, transform, bedrock, config, v2, azure plugin, session retry/llm — 1058 pass / 0 fail across 15 files (22 new upstream tests included); typecheck clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ck ARN/DeepSeek ids Two small upstream fixes to the opencode/provider/provider.ts "custom" loaders and shared wrapSSE helper, both completing partial ports that this fork's core-package copies already received but this file's duplicated logic missed: 1. wrapSSE(): the timeout path called `void reader.cancel(err)`, leaving an unhandled promise rejection when the underlying stream had already errored. core/aisdk.ts's copy of this function was already fixed (commit ac3b15b); this completes the port for the near-duplicate copy here. Ported from upstream ac1758c -> no, from upstream 69c172e ("fix(provider): handle SSE reader cancel rejections (anomalyco#44944)"). 2. amazon-bedrock custom loader's getModel(): double-prefixed full Bedrock ARNs (`arn:...`, which Bedrock rejects with a prefix) and over-matched "deepseek" (prefixing non-R1 DeepSeek models that don't support cross-region inference). The core Bedrock plugin (packages/core/src/plugin/provider/amazon-bedrock.ts) already got this fix (commit d66cefe); this completes the port for the legacy "custom" loader's own copy of the same logic. Ported from upstream ac1758c ("fix: preserve Bedrock DeepSeek model ids (anomalyco#34441)"). Verified via manual `tsgo --noEmit` on packages/core and packages/opencode (clean) and oxlint on the changed file (0 errors, pre-existing warnings only, none on changed lines). Full monorepo `bun turbo typecheck` cannot run in this sandbox because three unrelated packages (console-support, console-app, stats-app) depend on `@solidjs/start` pinned to a pkg.pr.new preview build blocked by this environment's network policy — a known, previously-documented sandbox limitation (see fork-expert skill's upstream-sync.md), not a regression from this change. The equivalent "typecheck" GitHub Actions check passed clean on PR #234, which hit the same sandbox limitation for its own real-code changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SHQKcGH7ayRe8UHJ9CAPQJ
Port upstream opencode ac1758c0e6 (anomalyco/opencode#34441), provider.ts hunks only: pass arn: model references through untouched, and narrow the us-region cross-region prefix list from bare "deepseek" to "deepseek.r1" so non-r1 DeepSeek ids (e.g. deepseek.v3.2) are no longer over-prefixed into us.deepseek.v3.2. Upstream's second landing site (core/src/plugin/provider/amazon-bedrock.ts resolveModelID) does not exist in this fork; the fork's mantle path (selectBedrockMantleLanguageModel) does no region prefixing, so no mantle-side change applies. Adds the upstream test matrix (bare v3.2, r1 prefixing, pre-prefixed profile id, arn passthrough) adapted to the fork's WithInstance/AppRuntime test shape. Upstream-Sha: ac1758c0e6
Co-authored-by: yeqisong <yeqisong@authing.com>
Issue for this PR
Closes #34412
Related #18812
Type of change
What does this PR do?
Bedrock DeepSeek model IDs were being treated like generic cross-region model IDs. That changed
deepseek.v3.2intous.deepseek.v3.2, which Bedrock rejects for the on-demand model.This PR keeps explicit Bedrock model identifiers intact:
deepseek.v3.2staysdeepseek.v3.2arn:aws:bedrock:...:foundation-model/...stays unchangedus.deepseek.r1-v1:0still pass through unchangedHow did you verify your code works?
Tested:
cd packages/opencode && bun test test/provider/amazon-bedrock.test.tscd packages/core && bun typecheckgit diff --checkReviewer can confirm by configuring
amazon-bedrock/deepseek.v3.2and checking that the provider receivesdeepseek.v3.2, notus.deepseek.v3.2.Local note:
cd packages/core && bun test test/plugin/provider-amazon-bedrock.test.tscurrently fails before this PR assertions withCannot access 'AmazonBedrockPlugin' before initialization.Screenshots / recordings
N/A, provider behavior only.
Checklist
If you do not follow this template your PR will be automatically rejected.