Skip to content

fix(models): stop flagging built-in models as custom in /v1/models (base-red) - #10388

Closed
diegosouzapw wants to merge 1 commit into
release/v3.8.50from
fix/release-v3.8.50-basereds
Closed

fix(models): stop flagging built-in models as custom in /v1/models (base-red)#10388
diegosouzapw wants to merge 1 commit into
release/v3.8.50from
fix/release-v3.8.50-basereds

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Summary

Base-red fix for release/v3.8.50 (#9985): tests/unit/models-catalog-route.test.ts has been failing on the release tip, taking down the Unit Tests fast-path shard of every open PR — including third-party ones.

Root cause: #10248 rewrote the custom-model overlay merge in src/app/api/v1/models/catalog.ts to always write custom: true. That is correct when the custom row is the model, but wrong when it is only a metadata overlay on a model already present as built-in/synced (e.g. a token-limit override on openai/gpt-4o-2024-11-20) — those got reported as operator-defined in the public /v1/models response.

Fix: drop the hardcoded custom: true from the overlay object so existing.custom survives the merge. A row that already was custom stays custom; a built-in merely overlaid with custom metadata stays built-in. One file, ~12 lines including the explanatory comment.

Validation

  • tests/unit/models-catalog-route.test.ts — 44/44 (was 43/44 red on the tip).
  • Sibling sweep: all 66 test files touching getUnifiedModelsResponse / addCustomModel — 388/388.
  • check-open-sse-typecheck.mjs OK · typecheck:core exit 0 · prettier/eslint clean on the touched file.

The originating investigation started from a different hypothesis (inactive-provider leakage); instrumentation proved the activeAliases gate already works and pointed at the custom flag instead.

Refs #9985, #10248

PR #10248 rewrote the custom-model overlay merge to always write
`custom: true`, even when the custom row is only a metadata overlay on a
model already present in the catalog as built-in/synced (e.g. a token-limit
override on openai/gpt-4o-2024-11-20). That misclassified built-in models as
operator-defined in the public catalog response.

Drop the hardcoded `custom: true` from the overlay so `existing.custom` is
preserved: a row that already was custom stays custom, a built-in merely
overlaid with custom metadata stays built-in.

Restores the pre-existing contract asserted by
tests/unit/models-catalog-route.test.ts, which has been red on the release tip
and was failing every open PR's Unit Tests shard.

Refs #9985
@diegosouzapw

Copy link
Copy Markdown
Owner Author

Superseded by #10383 (0bd2be05e7), which landed on the release tip while this was in review.

Both PRs diagnosed the same red (models-catalog-route.test.ts, duplicate-builtin overlay) and reached the same code location — but resolved it in opposite directions: this PR treated custom: true on an overlay as a regression of #10248 and removed it; #10383 treated it as the intended new contract ("a custom row for an id that already exists is the operator-owned overlay, and the merged entry is flagged custom") and aligned the test expectation instead.

The maintainer decision is the one already on the branch, so this PR is closed without merging — no code change is needed. Verified: the release tip now passes that test, and the failure is gone from the open PRs once they merge the current base.

@diegosouzapw
diegosouzapw deleted the fix/release-v3.8.50-basereds branch August 14, 2026 15:42
diegosouzapw added a commit that referenced this pull request Aug 14, 2026
…#9985) (#10396)

`check:dead-code` reports 410 dead symbols against a 409 baseline on the
pristine `release/v3.8.50` tip, so every PR on the branch is born red on that
gate (#10386, #10393, #10390, #10388, #10382 all fail it).

Isolated the +1 by diffing knip 6.32 reports between the rebaseline commit
97aac6a (409) and the tip (410): `resolveOpencodeConfigDir` in
`src/shared/services/cliRuntime.ts`. #10246 moved the canonical resolvers into
`opencodeConfigPath.ts` and left this wrapper behind; the same commit removed
its last consumer.

The wrapper was not just unused, it was divergent: it returned
`path.dirname()` of the canonical value — `~/.config` rather than
`~/.config/opencode` — so any future caller reaching for it by name would have
written the OpenCode config one directory too high.

Removed the wrapper and its now-unused import. A new test pins the canonical
resolver's contract and asserts the divergent re-export stays gone; the guard
was mutation-validated (re-adding the wrapper fails it).

check:dead-code: 409 = baseline, PASS.
cliRuntime/opencode suites: 51 pass, 0 fail. New guard: 3 pass, 0 fail.
lint / typecheck:core / file-size / complexity-ratchets / test-discovery: green.

Co-authored-by: Xiangzhe <bakryun0718@proton.me>
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