anomalyco/opencode
1
0
Fork 0
mirror of https://github.com/anomalyco/opencode.git synced 2026-07-22 18:25:32 -04:00
Code Issues 5.9k Packages Projects Releases 19 Wiki Activity

Subtle bug: model.variants can be unintentionally overwritten when ProviderTransform.variants() returns {} #8540

New Issue
Open
opened 2026-02-16 18:10:14 -05:00 by yindo · 1 comment
yindo commented 2026-02-16 18:10:14 -05:00
Owner
Copy Link
Copy Source

Originally created by @monotykamary on GitHub (Feb 4, 2026).

Originally assigned to: @thdxr on GitHub.

Bug Description

There's a subtle bug in provider.ts where model.variants is unconditionally overwritten with the result of ProviderTransform.variants(model) before merging with user config variants.

The Issue

In the provider state initialization (around line 925), the code does:

model.variants = mapValues(ProviderTransform.variants(model), (v) => v)

// Then tries to merge with config
const configVariants = configProvider?.models?.[modelID]?.variants
if (configVariants && model.variants) {
  const merged = mergeDeep(model.variants, configVariants)
  model.variants = mapValues(
    pickBy(merged, (v) => !v.disabled),
    (v) => omit(v, ["disabled"]),
  )
}

The problem is that ProviderTransform.variants() returns {} for models like kimi, deepseek, minimax, etc. (lines 331-341 in transform.ts). This means:

  1. model.variants is first set to {}
  2. Then the code attempts to merge with configVariants

Why This Is Subtle

Currently, this doesn't cause visible issues because:

  • The merge with configVariants happens immediately after
  • For most models, ProviderTransform.variants() returns non-empty variants

However, the subtle bug emerges in this scenario:

If a user defines custom variants in config AND those variants happen to have the same name as a built-in variant that ProviderTransform.variants() would return (if it weren't empty), the merge order and the fact that we first overwrite then merge could lead to unexpected precedence issues.

More importantly, if configVariants lookup fails or is undefined (which can happen in certain config loading edge cases), the user-defined variants from an earlier processing step would be lost entirely.

Current Workaround

The code happens to work now because:

  1. For built-in providers, models go through initial processing where variants are set
  2. The second loop then overwrites and re-merges
  3. For kimi models, ProviderTransform returns {} so the merge just uses configVariants

Suggested Fix

Instead of overwriting then conditionally merging, we should merge directly:

const baseVariants = ProviderTransform.variants(model)
const mergedVariants = mergeDeep(baseVariants, model.variants ?? {})
model.variants = mapValues(
  pickBy(mergedVariants, (v) => !v.disabled),
  (v) => omit(v, ["disabled"]),
)

This ensures:

  1. User-defined variants from config always take precedence
  2. No data loss if ProviderTransform returns {}
  3. Cleaner, more predictable behavior

Impact

This is a low-severity bug that doesn't affect current functionality but could cause issues if:

  • Future changes modify the config loading order
  • Custom provider configs become more complex
  • Models start having overlapping variant names between built-in and user-defined

I've submitted PR #XXX with a fix.

Originally created by @monotykamary on GitHub (Feb 4, 2026). Originally assigned to: @thdxr on GitHub. ## Bug Description There's a subtle bug in `provider.ts` where `model.variants` is unconditionally overwritten with the result of `ProviderTransform.variants(model)` before merging with user config variants. ### The Issue In the provider state initialization (around line 925), the code does: ```typescript model.variants = mapValues(ProviderTransform.variants(model), (v) => v) // Then tries to merge with config const configVariants = configProvider?.models?.[modelID]?.variants if (configVariants && model.variants) { const merged = mergeDeep(model.variants, configVariants) model.variants = mapValues( pickBy(merged, (v) => !v.disabled), (v) => omit(v, ["disabled"]), ) } ``` The problem is that `ProviderTransform.variants()` returns `{}` for models like kimi, deepseek, minimax, etc. (lines 331-341 in transform.ts). This means: 1. `model.variants` is first set to `{}` 2. Then the code attempts to merge with `configVariants` ### Why This Is Subtle Currently, this doesn't cause visible issues because: - The merge with `configVariants` happens immediately after - For most models, `ProviderTransform.variants()` returns non-empty variants However, **the subtle bug emerges in this scenario**: If a user defines custom variants in config AND those variants happen to have the same name as a built-in variant that `ProviderTransform.variants()` would return (if it weren't empty), the merge order and the fact that we first overwrite then merge could lead to unexpected precedence issues. More importantly, if `configVariants` lookup fails or is undefined (which can happen in certain config loading edge cases), the user-defined variants from an earlier processing step would be lost entirely. ### Current Workaround The code happens to work now because: 1. For built-in providers, models go through initial processing where variants are set 2. The second loop then overwrites and re-merges 3. For kimi models, ProviderTransform returns {} so the merge just uses configVariants ### Suggested Fix Instead of overwriting then conditionally merging, we should merge directly: ```typescript const baseVariants = ProviderTransform.variants(model) const mergedVariants = mergeDeep(baseVariants, model.variants ?? {}) model.variants = mapValues( pickBy(mergedVariants, (v) => !v.disabled), (v) => omit(v, ["disabled"]), ) ``` This ensures: 1. User-defined variants from config always take precedence 2. No data loss if ProviderTransform returns {} 3. Cleaner, more predictable behavior ### Impact This is a low-severity bug that doesn't affect current functionality but could cause issues if: - Future changes modify the config loading order - Custom provider configs become more complex - Models start having overlapping variant names between built-in and user-defined I've submitted PR #XXX with a fix.
yindo commented 2026-02-16 18:10:14 -05:00
Author
Owner
Copy Link
Copy Source

@github-actions[bot] commented on GitHub (Feb 4, 2026):

This issue appears to be related to similar config merging and variant handling problems found in other issues. Please check:

  • #6806: Partial thinking config doesn't merge with defaults, causing AI_InvalidArgumentError - demonstrates similar config merging issues
  • #12155: missing reasoning variants for multiple providers - related to variant handling for models with empty variant returns

These issues share the same root cause pattern where partial or missing configurations don't properly merge with defaults/existing data, which aligns with the variant overwriting bug described here.

@github-actions[bot] commented on GitHub (Feb 4, 2026): This issue appears to be related to similar config merging and variant handling problems found in other issues. Please check: - #6806: Partial thinking config doesn't merge with defaults, causing AI_InvalidArgumentError - demonstrates similar config merging issues - #12155: missing reasoning variants for multiple providers - related to variant handling for models with empty variant returns These issues share the same root cause pattern where partial or missing configurations don't properly merge with defaults/existing data, which aligns with the variant overwriting bug described here.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
acp
bad
bug
discussion
docs
documentation
enhancement
extension
good first issue
growth
help-wanted
mcp
model-problem
needs:compliance
nix
opentui
perf
pull-request
Mirrored from GitHub Pull Request
 tui
web
windows
zen
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: anomalyco/opencode#8540
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?