Skip to content

Fix MiniMax Direct reasoning split in Chrome extension#354

Open
vincent-peng wants to merge 1 commit into
steipete:mainfrom
vincent-peng:codex/minimax-direct-reasoning-split
Open

Fix MiniMax Direct reasoning split in Chrome extension#354
vincent-peng wants to merge 1 commit into
steipete:mainfrom
vincent-peng:codex/minimax-direct-reasoning-split

Conversation

@vincent-peng

@vincent-peng vincent-peng commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add MiniMax-specific reasoning_split: true to Chrome extension OpenAI-compatible Direct requests.
  • Add direct-provider regression coverage proving MiniMax separated reasoning fields are ignored while visible delta.content is preserved.
  • Assert the flag is not sent to the regular OpenAI-compatible path.

Root cause

Fixes #349.

MiniMax's OpenAI-compatible API keeps thinking in the content field with <think>...</think> tags unless reasoning_split is enabled. The CLI/daemon MiniMax path already requests separated reasoning, but the Chrome extension Direct provider routes MiniMax through the generic OpenAI-compatible adapter and did not include that MiniMax-specific request field.

Validation

  • pnpm -C apps/chrome-extension exec playwright test -c playwright.config.ts --project=chromium tests/direct-provider.spec.ts --grep "OpenAI-compatible streaming text|separated MiniMax reasoning"
  • pnpm -C apps/chrome-extension exec playwright test -c playwright.config.ts --project=chromium tests/direct-provider.spec.ts - 12 passed
  • pnpm -C apps/chrome-extension build
  • pnpm -s format:check
  • pnpm -s lint
  • pnpm -s typecheck
  • pnpm -s check - 547 passed / 29 skipped test files, 2940 passed / 43 skipped tests
  • GitHub CI on this PR: test (24), extension-e2e, extension-firefox-smoke, and GitGuardian all passed.

Chrome extension validation note: the CI extension-e2e job runs the Chrome extension build, Chromium native-messaging E2E, HTTP-transport extension build, and the full Chromium Playwright suite for the extension. A local macOS pnpm -C apps/chrome-extension test:chrome run previously reproduced an unrelated tests/sidepanel.browser-slides.spec.ts activeTab/<all_urls> permission failure, but the equivalent CI extension suite passed cleanly on this PR.

Live MiniMax proof

Pending: no live MiniMax credential is configured in this validation environment, so no live-provider proof is claimed yet. The current proof covers the exact Chrome extension Direct request body and stream parsing behavior. Once a MiniMax key is available, the remaining proof should be a redacted Chrome extension Direct MiniMax run showing that the request includes reasoning_split: true, visible output does not contain <think>/reasoning fields, and normal summary text still streams.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 10, 2026, 5:16 AM ET / 09:16 UTC.

Summary
The PR adds MiniMax-only reasoning_split: true to Chrome extension Direct requests and regression coverage for provider scoping and visible streamed content.

Reproducibility: yes. at source level: current main omits the established MiniMax request option in Chrome Direct, while the linked report shows reasoning tags in extension output. This review did not run a credential-backed live provider reproduction.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 2 files, 41 additions, 0 deletions. The fix is isolated to one request option and focused direct-provider regression coverage.
  • Reported required checks: 4 checks passed. The test matrix, extension E2E, Firefox smoke, and GitGuardian checks are green, leaving live behavior proof as the distinct unresolved gate.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #349
Summary: This PR is the candidate fix for the canonical Chrome extension MiniMax Direct reasoning-leak report.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Post a redacted live Chrome Direct MiniMax run showing reasoning_split: true, no visible reasoning tags, and preserved streamed summary text.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Live MiniMax Chrome extension proof is explicitly pending; add a redacted recording, network/request diagnostics, live output, or runtime logs showing the field and clean streamed summary, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review.

Risk before merge

  • [P1] A real MiniMax Chrome Direct request has not yet confirmed endpoint acceptance of the field and clean streamed visible output in the contributor’s setup.

Maintainer options:

  1. Decide the mitigation before merge
    Retain this narrow provider-specific request change and regression coverage, then merge after a redacted live Chrome Direct MiniMax run confirms separated reasoning and preserved summary output.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] The remaining blocker is contributor-owned real behavior proof from a MiniMax-enabled environment, not a code or test repair ClawSweeper can safely generate.

Security
Cleared: The diff introduces no dependency, permission, secret-handling, workflow, package-resolution, artifact-download, or other supply-chain change.

Review details

Best possible solution:

Retain this narrow provider-specific request change and regression coverage, then merge after a redacted live Chrome Direct MiniMax run confirms separated reasoning and preserved summary output.

Do we have a high-confidence way to reproduce the issue?

Yes, at source level: current main omits the established MiniMax request option in Chrome Direct, while the linked report shows reasoning tags in extension output. This review did not run a credential-backed live provider reproduction.

Is this the best way to solve the issue?

Yes. Applying the existing MiniMax request contract at the shared Chrome Direct request constructor is the narrowest maintainable fix and avoids a second, extension-only reasoning-tag stripping implementation.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 206a6f844764.

Label changes

Label justifications:

  • P2: The PR addresses a reproducible provider-specific Chrome extension output defect with limited blast radius.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Live MiniMax Chrome extension proof is explicitly pending; add a redacted recording, network/request diagnostics, live output, or runtime logs showing the field and clean streamed summary, then update the PR body to trigger re-review or ask a maintainer to comment @clawsweeper re-review.
Evidence reviewed

What I checked:

Likely related people:

  • steipete: The owner-authored Chrome Direct providers work introduced the affected adapter, and current file provenance remains primarily tied to that work. (role: feature introducer and recent area contributor; confidence: high; commits: 5b0b1eb4c657, 288bec056698; files: apps/chrome-extension/src/lib/direct-provider/openai-compatible.ts, apps/chrome-extension/tests/direct-provider.spec.ts)
  • neeravmakwana: The merged native MiniMax implementation established the separated-reasoning request behavior that this PR carries into Chrome Direct. (role: MiniMax feature contributor; confidence: high; commits: 3c24df0e9de8; files: src/llm/providers/minimax.ts, tests/llm.minimax.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (3 earlier review cycles)
  • reviewed 2026-07-09T01:32:04.027Z sha 9157f1f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-09T02:03:33.939Z sha 9157f1f :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-09T02:50:16.423Z sha 9157f1f :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jul 9, 2026
@vincent-peng

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

CI is now green and the PR body has been updated to explicitly state that live MiniMax proof is still pending because no MiniMax API key is configured in this validation environment. Please re-review based on the updated validation/proof notes.

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Browser extension doesn't strip reasoning-tag when using direct mode with MiniMax provider

1 participant