fix(model-capabilities): harden runtime capability handling#2829
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@code-yeongyu follow-up PR for the six Cubic review fixes from #2826. This is just the post-merge delta from Verification:
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Confidence score: 4/5
- This PR is likely safe to merge, with a focused medium-risk gap rather than a broad stability concern.
- In
src/shared/model-capabilities.ts,readRuntimeModelThinkingSupportcurrently only readsruntimeModel.capabilities, so providers using root-levelthinking/supportsThinkingcan be misclassified and lose thinking-support behavior. - Given the issue is moderate severity (5/10) and isolated to capability detection logic, the overall merge risk looks limited but worth a follow-up fix.
- Pay close attention to
src/shared/model-capabilities.ts- root-level thinking flags may be ignored, causing incorrect provider capability detection.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/model-capabilities.ts">
<violation number="1" location="src/shared/model-capabilities.ts:273">
P2: readRuntimeModelThinkingSupport ignores root-level thinking/supportsThinking flags; it only checks runtimeModel.capabilities, so providers that set these at the root will be missed.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@code-yeongyu Cubic follow-up addressed in Additional fix:
Verification rerun:
@cubic-dev-ai please re-review. |
@RaviTharuma I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/model-capabilities.ts">
<violation number="1" location="src/shared/model-capabilities.ts:279">
P2: Custom agent: **Opencode Compatibility**
The fallback loop checking `runtimeCapabilities` is now unreachable dead code since `readRuntimeModelBoolean` already checks the `capabilities` object.</violation>
</file>
<file name="src/shared/model-capabilities.test.ts">
<violation number="1" location="src/shared/model-capabilities.test.ts:121">
P2: The new test is tautological: it uses a model whose snapshot already enables reasoning, so it does not actually verify root-level runtime `supportsThinking` handling.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
2 issues found across 8 files
Confidence score: 4/5
- This PR looks safe to merge overall: the reported issues are moderate-to-low severity (5/10 and 4/10) and don’t indicate a clear runtime breakage on their own.
- The most important risk is in
src/shared/model-capabilities.test.ts: the root-level thinking-flag test can pass via snapshot fallback (gpt-5.4withreasoning: true), so it may miss a real regression in runtime root-level parsing behavior. - In
src/shared/model-capabilities.ts, the extra loop appears to be redundant/dead code becausereadRuntimeModelBooleanalready handles those keys; this is mainly a maintainability/clarity concern rather than an immediate user-facing bug. - Pay close attention to
src/shared/model-capabilities.test.tsandsrc/shared/model-capabilities.ts- strengthen the test to verify actual runtime parsing and remove or refactor the unreachable return path.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/model-capabilities.ts">
<violation number="1" location="src/shared/model-capabilities.ts:289">
P2: Redundant dead code: this loop re-checks `runtimeCapabilities` keys already handled by `readRuntimeModelBoolean`, making its return path effectively unreachable.</violation>
</file>
<file name="src/shared/model-capabilities.test.ts">
<violation number="1" location="src/shared/model-capabilities.test.ts:87">
P2: The new root-level thinking-flag test can pass due to snapshot fallback (`gpt-5.4` has `reasoning: true`), so it does not actually prove runtime root-level parsing works.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@code-yeongyu I pushed a follow-up hardening commit on top of the current model-capabilities PR: This extends the earlier review-fix work with long-term maintenance guardrails so model capability drift does not silently accumulate:
Fresh verification on this head:
One maintainer decision I want to make explicit: the new scheduled workflow auto-opens a PR for bundled |
There was a problem hiding this comment.
3 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/shared/model-capability-aliases.ts">
<violation number="1" location="src/shared/model-capability-aliases.ts:45">
P2: Thinking-suffix alias matching fails for provider-prefixed Claude model IDs, causing alias resolution to be skipped.</violation>
<violation number="2" location="src/shared/model-capability-aliases.ts:56">
P2: Direct indexing into a plain object allows prototype keys (e.g. `constructor`) to bypass alias checks and return an invalid `exact-alias` resolution with undefined fields.</violation>
</file>
<file name=".github/workflows/refresh-model-capabilities.yml">
<violation number="1" location=".github/workflows/refresh-model-capabilities.yml:32">
P2: Automated PRs created by this workflow use the default `GITHUB_TOKEN`, which can prevent downstream `pull_request` CI workflows from auto-running.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| export function resolveModelIDAlias(modelID: string): ModelIDAliasResolution { | ||
| const normalizedModelID = normalizeLookupModelID(modelID) | ||
| const exactRule = EXACT_ALIAS_RULES[normalizedModelID] |
There was a problem hiding this comment.
P2: Direct indexing into a plain object allows prototype keys (e.g. constructor) to bypass alias checks and return an invalid exact-alias resolution with undefined fields.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/model-capability-aliases.ts, line 56:
<comment>Direct indexing into a plain object allows prototype keys (e.g. `constructor`) to bypass alias checks and return an invalid `exact-alias` resolution with undefined fields.</comment>
<file context>
@@ -0,0 +1,84 @@
+
+export function resolveModelIDAlias(modelID: string): ModelIDAliasResolution {
+ const normalizedModelID = normalizeLookupModelID(modelID)
+ const exactRule = EXACT_ALIAS_RULES[normalizedModelID]
+ if (exactRule) {
+ return {
</file context>
| const PATTERN_ALIAS_RULES: ReadonlyArray<PatternAliasRule> = [ | ||
| { | ||
| ruleID: "anthropic-thinking-suffix", | ||
| match: (normalizedModelID) => normalizedModelID.startsWith("claude-") && normalizedModelID.endsWith("-thinking"), |
There was a problem hiding this comment.
P2: Thinking-suffix alias matching fails for provider-prefixed Claude model IDs, causing alias resolution to be skipped.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/shared/model-capability-aliases.ts, line 45:
<comment>Thinking-suffix alias matching fails for provider-prefixed Claude model IDs, causing alias resolution to be skipped.</comment>
<file context>
@@ -0,0 +1,84 @@
+const PATTERN_ALIAS_RULES: ReadonlyArray<PatternAliasRule> = [
+ {
+ ruleID: "anthropic-thinking-suffix",
+ match: (normalizedModelID) => normalizedModelID.startsWith("claude-") && normalizedModelID.endsWith("-thinking"),
+ canonicalize: (normalizedModelID) => normalizedModelID.replace(/-thinking$/i, ""),
+ },
</file context>
| match: (normalizedModelID) => normalizedModelID.startsWith("claude-") && normalizedModelID.endsWith("-thinking"), | |
| match: (normalizedModelID) => /(?:^|\/)claude-.*-thinking$/i.test(normalizedModelID), |
| run: bun run build:model-capabilities | ||
|
|
||
| - name: Create refresh pull request | ||
| uses: peter-evans/create-pull-request@v7 |
There was a problem hiding this comment.
P2: Automated PRs created by this workflow use the default GITHUB_TOKEN, which can prevent downstream pull_request CI workflows from auto-running.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/refresh-model-capabilities.yml, line 32:
<comment>Automated PRs created by this workflow use the default `GITHUB_TOKEN`, which can prevent downstream `pull_request` CI workflows from auto-running.</comment>
<file context>
@@ -0,0 +1,43 @@
+ run: bun run build:model-capabilities
+
+ - name: Create refresh pull request
+ uses: peter-evans/create-pull-request@v7
+ with:
+ commit-message: "chore: refresh model capabilities snapshot"
</file context>
|
@code-yeongyu I verified the current red CI is not coming from this PR. I reproduced the same todo-sync failure on a clean upstream/dev checkout at 5e9231e, and the failing files have no diff from upstream/dev. I split that out into a separate base-branch fix here: #2832. So the current state is: #2829 remains the model-capabilities follow-up, and #2832 is the unrelated CI unblock for the existing todo-sync mismatch. |
2 participants
Summary
Why a follow-up PR
#2826 merged before these review fixes landed on the branch, so this PR carries the one remaining delta on top of current
dev.Verification
Summary by cubic
Hardens runtime model capability handling and adds guardrails to surface capability diagnostics and auto-refresh the bundled capabilities snapshot for accuracy over time.
New Features
omo doctorshows capability resolution mode and warns when models rely on alias/heuristic/unknown fallback.src/generated/model-capabilities.generated.jsonweekly and open a PR.Bug Fixes
runtimeModel.capabilitiesor root; keep variant arrays intact.thinking/supportsThinking) when providers don’t nest capabilities.modalitiesorlimitobjects; treat arrays as non-records.openai/o3-mini) in heuristics.max); compatibility downgrades requestedmaxtoxhigh.idis not a string.Written for commit a15f607. Summary will update on new commits.