feat(core): reasoning for all providers (auto fallback) by hqhq1025 · Pull Request #90 · OpenCoworkAI/open-codesign

hqhq1025 · 2026-04-19T05:40:52Z

Claude 4 仍然 explicit 'high'，其他 provider 改 'auto' 让 pi-ai 决定。让 OpenRouter elephant-alpha / OpenAI o-series / DeepSeek R1 等也能享受 reasoning。

github-actions

Findings

[Blocker] Silent fallback path introduced for most models — this conflicts with the project constraint that fallbacks/errors must be surfaced, and the new behavior now applies reasoning: 'auto' to all non-Claude-4 models while explicitly documenting that unsupported models are silently ignored. Evidence packages/core/src/index.ts:408, packages/core/src/index.ts:411, packages/providers/src/index.ts:16
Suggested fix:
```
function reasoningForModel(model: ModelRef): ReasoningLevel | undefined {
  if (model.provider === 'anthropic' && CLAUDE_4_MODEL_RE.test(model.modelId)) {
    return 'high';
  }
  if (isKnownReasoningModel(model)) {
    return 'auto';
  }
  return undefined;
}
```

Summary

Review mode: initial
1 blocker found in added/modified lines.
docs/VISION.md and docs/PRINCIPLES.md: Not found in repo/docs; policy validation used CLAUDE.md in this checkout.

Testing

Not run (automation)

github-actions · 2026-04-19T05:42:40Z

+  // Other reasoning-capable models (OpenAI o-series / GPT-5, DeepSeek R1,
+  // Qwen QwQ, OpenRouter pass-through, etc.) — let pi-ai's adapter decide
+  // based on the model capability flag. Non-reasoning models drop it.
+  return 'auto';


Applying return 'auto' for all non-Claude-4 models introduces a silent fallback path for unsupported models. The repo constraints require surfaced failures instead of silent behavior. Consider gating auto to known reasoning-capable models and returning undefined otherwise (or surfacing an explicit warning/error path).

… sentinel Codex review on PR #90 flagged 'auto' as a silent fallback: pi-ai's ThinkingLevel does not include 'auto', so OpenAI / OpenRouter non-reasoning models silently drop it while older Anthropic Claudes coerce it to 'high' and unintentionally enable extended thinking. Both behaviours violate the project's "no silent fallbacks" rule. Replace the sentinel with an explicit whitelist: - Anthropic Claude 4.x -> 'high' (extended thinking) - OpenAI o1/o3/o4 + GPT-5 -> 'high' - DeepSeek R1 -> 'high' - Qwen QwQ -> 'high' - everything else -> undefined (pi-ai sees no reasoning field) Also revert the providers ReasoningLevel union back to the named effort levels pi-ai actually understands.

github-actions

Findings

[Major] Over-broad reasoning-model detection can reintroduce silent fallback behavior — reasoningForModel currently enables reasoning based on substring matches regardless of provider, so pass-through model IDs (especially on openrouter) that happen to contain o1/o3/o4/gpt-5/r1/qwq can receive reasoning unintentionally, which conflicts with the no-silent-fallback constraint. Evidence packages/core/src/index.ts:412.
Suggested fix:

function reasoningForModel(model: ModelRef): ReasoningLevel | undefined {
  const id = model.modelId.toLowerCase();

  if (model.provider === 'anthropic' && /claude-(?:opus|sonnet)-4/.test(id)) return 'high';

  if (
    model.provider === 'openai' &&
    (/^gpt-5(?:$|[-:])/i.test(model.modelId) || /^o(?:1|3|4)(?:$|[-:])/i.test(model.modelId))
  ) {
    return 'high';
  }

  if (model.provider === 'groq' && /deepseek.*r1|r1.*deepseek/i.test(model.modelId)) return 'high';
  if (model.provider === 'qwen' && /\bqwq\b/i.test(model.modelId)) return 'high';

  return undefined;
}

Summary

Review mode: initial
1 issue found (Major) in reasoning capability detection. docs/VISION.md and docs/PRINCIPLES.md were not found in repo/docs in this checkout.

Testing

Not run (automation)

open-codesign Bot

github-actions · 2026-04-19T06:21:23Z

+  if (model.provider === 'anthropic' && CLAUDE_4_MODEL_RE.test(model.modelId)) {
+    return 'high';
+  }
+  if (/\b(o1|o3|o4|gpt-5)/i.test(model.modelId)) return 'high';


[Major] This regex is provider-agnostic and substring-based, so model IDs from pass-through providers can accidentally match and receive reasoning, reintroducing silent fallback behavior. Consider provider-scoped, anchored allowlists (e.g., openai-only for gpt-5/o1|o3|o4) and default undefined elsewhere.

Suggested fix:

if ( model.provider === 'openai' && (/^gpt-5(?:$|[-:])/i.test(model.modelId) || /^o(?:1|3|4)(?:$|[-:])/i.test(model.modelId)) ) { return 'high'; }

… sentinel Codex review on PR #90 flagged 'auto' as a silent fallback: pi-ai's ThinkingLevel does not include 'auto', so OpenAI / OpenRouter non-reasoning models silently drop it while older Anthropic Claudes coerce it to 'high' and unintentionally enable extended thinking. Both behaviours violate the project's "no silent fallbacks" rule. Replace the sentinel with an explicit whitelist: - Anthropic Claude 4.x -> 'high' (extended thinking) - OpenAI o1/o3/o4 + GPT-5 -> 'high' - DeepSeek R1 -> 'high' - Qwen QwQ -> 'high' - everything else -> undefined (pi-ai sees no reasoning field) Also revert the providers ReasoningLevel union back to the named effort levels pi-ai actually understands.

Codex flagged that the previous whitelist still ran substring checks (o1/o3/r1/qwq) against `model.modelId` for ANY provider. OpenRouter, Groq, Cerebras and other pass-through providers happily forward an arbitrary upstream id like `mystery-lab/o1-lookalike` or `anthropic/claude-sonnet-4` — so reasoning got silently enabled on models the upstream provider does not advertise as reasoning-capable. That re-introduced the silent-fallback behavior PR #90 set out to kill. Tighten the gate to a (provider, modelId) pair: - anthropic + claude-(opus|sonnet)-4 → 'high' - openai + ^(o1|o3|o4|gpt-5)(...)$ → 'high' - everything else (incl. all pass-through providers) → undefined Add tests for the OpenRouter pass-through cases and o3-mini / claude-opus-4-7 first-party cases.

hqhq1025 · 2026-04-19T06:42:27Z

Addressing Codex [Major] re: substring matching across all providers.

reasoningForModel previously fell back to running o1|o3|o4|gpt-5 / r1 / qwq substring checks against model.modelId regardless of provider. That meant any OpenRouter / Groq / Cerebras pass-through id containing one of those substrings (e.g. mystery-lab/o1-lookalike, anthropic/claude-sonnet-4, deepseek-r1-distill-llama-70b on Groq) would silently get reasoning: 'high' even though the upstream provider does not advertise the model as reasoning-capable — exactly the silent-fallback this PR was supposed to eliminate.

Tightened the gate to require both axes to match a first-party provider we trust:

provider === 'anthropic' AND claude-(opus|sonnet)-4 -> 'high'
provider === 'openai' AND ^(o1|o3|o4|gpt-5)(?:[-.].*)?$ -> 'high'
everything else (including all OpenRouter / Groq / Cerebras / xAI / Bedrock / Vercel-AI-Gateway pass-throughs) -> undefined

Decision rationale: pass-through providers serve arbitrary upstream model ids, so the model id alone is not a trustable capability signal. Users who want reasoning on a pass-through provider should configure the first-party provider directly (BYOK is already the model). Better to under-enable than to silently drop or mistranslate the reasoning knob.

Added tests for: openrouter anthropic/claude-sonnet-4 -> undefined, openrouter mystery-lab/o1-lookalike -> undefined, openrouter elephant/elephant-alpha -> undefined, groq deepseek-r1-distill-llama-70b -> undefined, anthropic claude-opus-4-7 -> 'high', openai o3-mini -> 'high'. Full vitest suite green (127 core, 288 desktop).

github-actions

Findings

None.

Summary

Review mode: follow-up after new commits
No issues found in added/modified lines of the latest PR diff.
Residual risk: docs/VISION.md and docs/PRINCIPLES.md were not found in this checkout (Not found in repo/docs), so principle-level validation was limited to CLAUDE.md plus diff evidence.

Testing

Not run (automation)

open-codesign Bot

github-actions Bot reviewed Apr 19, 2026

View reviewed changes

github-advanced-security AI found potential problems Apr 19, 2026

View reviewed changes

Comment thread packages/core/src/index.ts Fixed

github-actions Bot reviewed Apr 19, 2026

View reviewed changes

hqhq1025 added 3 commits April 19, 2026 14:40

feat(core): widen reasoning gate to all providers (let pi-ai decide)

fe42702

hqhq1025 force-pushed the wt/loop-feat-reasoning-all-providers branch from cf033d1 to 4469c23 Compare April 19, 2026 06:42

github-actions Bot reviewed Apr 19, 2026

View reviewed changes

hqhq1025 merged commit a0057a2 into main Apr 19, 2026
5 of 6 checks passed

hqhq1025 deleted the wt/loop-feat-reasoning-all-providers branch April 19, 2026 06:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): reasoning for all providers (auto fallback)#90

feat(core): reasoning for all providers (auto fallback)#90
hqhq1025 merged 3 commits intomainfrom
wt/loop-feat-reasoning-all-providers

hqhq1025 commented Apr 19, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Apr 19, 2026

Uh oh!

Uh oh!

github-actions Bot left a comment

Uh oh!

github-actions Bot Apr 19, 2026

Uh oh!

hqhq1025 commented Apr 19, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hqhq1025 commented Apr 19, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

hqhq1025 commented Apr 19, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants