ci: default large Lean OCR scout to MiniMax - #2138
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_926feeeb-32e9-4b61-9550-380629421b88) |
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
OpenCodeReview first-pass review
Status: success · Mode: config-docs · 3 finding(s) · 4 files · 187771 tokens · 22 tool calls
✅ Posted 3 inline comment(s).
OCR pilot metrics
- Routing: config-docs (router-v4)
- Changed files: 4 supported / 4 total; Lean 0, trust docs 0, workflow/scripts 4, contracts 0, docs 0
- Changed lines: 168 supported; thresholds large Lean >=3 files or >800 lines
- OCR: status success; comments 3; files 4; tokens 187771; tool calls 22; warnings 0; duration 652s
- Largest changed files: .github/scripts/ocr-router.js (+80/-11), .github/scripts/test-ocr-routing.js (+63/-0), .github/ocr/README.md (+8/-1), .github/workflows/ocr-review.yml (+4/-1)
Pilot mode: advisory only. Codex Review remains the merge gate.
|
|
||
| - `OCR_SCOUT_ENABLED`: optional, defaults to `true`; set to `false` to disable only the large Lean scout call. | ||
| - `OCR_SCOUT_LLM_URL`: optional; defaults to `OCR_LLM_URL` when the same sandboxed endpoint supports model selection. | ||
| - `OCR_SCOUT_LLM_KEY`: optional; defaults to `OCR_LLM_KEY` when the same sandboxed key can route both models. |
There was a problem hiding this comment.
OpenCodeReview [documentation · low]
The README documents OCR_SCOUT_LLM_KEY as falling back to OCR_LLM_KEY, but the code (resolveScoutConfig) has an additional third-level fallback to OCR_LLM_TOKEN: env.OCR_SCOUT_LLM_KEY || env.OCR_LLM_KEY || env.OCR_LLM_TOKEN || ''. This fallback is exercised in the workflow YAML where OCR_LLM_TOKEN is separately set. The documentation should mention this third fallback for completeness, so operators debugging key resolution are not confused.
Suggested change:
- `OCR_SCOUT_LLM_KEY`: optional; defaults to `OCR_LLM_KEY`, then `OCR_LLM_TOKEN`, when the same sandboxed key can route both models.
| assert.strictEqual(config.model, 'MiniMax-M3'); | ||
| const ocrReviewerModel = 'reviewer'; | ||
| assert.strictEqual(ocrReviewerModel, 'reviewer'); | ||
| } |
There was a problem hiding this comment.
OpenCodeReview [test · high]
This assertion is dead code: ocrReviewerModel is a local constant assigned 'reviewer' and then compared against itself. It does not verify any behavior from resolveScoutConfig or the router. This gives a false sense of test coverage — if the strong OCR path model ever changes, this test would still pass. Either remove these two lines, or assert against the actual model the router uses for the strong OCR path (e.g., via a router export or decision field).
Suggested change:
assert.strictEqual(config.model, 'MiniMax-M3');
// Remove the dead assertion, or replace with a real check against router state, e.g.:
// assert.strictEqual(router.STRONG_OCR_MODEL, 'reviewer');
}
| .replace(/<think>[\s\S]*?<\/think>/gi, '') | ||
| .replace(/^```(?:json)?\s*/i, '') | ||
| .replace(/\s*```$/i, ''); | ||
| } |
There was a problem hiding this comment.
OpenCodeReview [bug · medium]
The markdown fence-stripping regexes /^```(?:json)?\s*/i and /\s*```$/i use ^/$ anchors without the m flag, so they only match at the absolute start/end of the string. However, stripThinking is called BEFORE .trim() in parseScoutJson. When MiniMax wraps output with <mm:think> tags followed by a fenced JSON block (the exact use case this function was added for), removing the thinking tags leaves a leading \n before the opening fence and a trailing \n after the closing fence. This prevents both regexes from matching, so the fences survive into JSON.parse(), which throws, and the code falls through to extractJsonObject() every time. The fallback works, but it silently masks a broken primary path and adds unnecessary parsing overhead. Fix: call .trim() on the input at the start of stripThinking, or remove the anchors and use non-anchored patterns.
Suggested change:
function stripThinking(text) {
return text
.replace(/<mm:think>[\s\S]*?<\/mm:think>/gi, '')
.replace(/<think>[\s\S]*?<\/think>/gi, '')
.trim()
.replace(/^```(?:json)?\s*/i, '')
.replace(/\s*```$/i, '');
}
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05b66bb3ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| enabled, | ||
| url: env.OCR_SCOUT_LLM_URL || env.OCR_LLM_URL || '', | ||
| key: env.OCR_SCOUT_LLM_KEY || env.OCR_LLM_KEY || env.OCR_LLM_TOKEN || '', | ||
| model: env.OCR_SCOUT_LLM_MODEL || DEFAULT_SCOUT_MODEL, |
There was a problem hiding this comment.
When only OCR_SCOUT_LLM_URL is set for a separate scout provider and OCR_SCOUT_LLM_KEY is omitted, this default model makes the scout config complete while the key fallback on the previous line pulls in OCR_LLM_KEY/OCR_LLM_TOKEN; large-Lean routing then POSTs the primary reviewer credential to the scout URL. Before this default, the missing model kept the scout disabled in that configuration. Require an explicit scout key whenever the scout URL is overridden, or only fall back the URL and key together.
Useful? React with 👍 / 👎.
1 participant
Summary
MiniMax-M3.OCR_SCOUT_*secrets/vars when present, or fall back to the existingOCR_LLM_URL/OCR_LLM_KEYendpoint/key with model selection.reviewerfor small Lean/config/docs paths.Validation
node --check .github/scripts/ocr-router.jsnode --check .github/scripts/post-ocr-review.jsnode --check .github/scripts/test-ocr-routing.jsnode .github/scripts/test-ocr-routing.jsbash -n .github/scripts/ocr-git-wrapper.shpython3 -m json.tool .github/ocr/rules.json.github/workflows/ocr-review.ymlactionlint .github/workflows/ocr-review.ymllarge-lean-hotspots,MiniMax-M3, 7 packets, OCR not attempted; local env has no OCR secrets, so production must verify the live scout call.Production test plan
After checks and Codex review are clean, merge to
main, then trigger/ocr reviewon #2128. The expected production result islarge-lean-hotspotswith scout status showing a live MiniMax-M3 attempt/success or a concrete provider/model error. The run must not invoke full OCR over the entire large Lean diff.Note
Medium Risk
Changes CI routing and external LLM calls for large Lean PRs; failures are designed to fall back to deterministic ranking, but production scout behavior should be verified on a real large diff.
Overview
Large Lean hotspot routing now turns on the scout by default and targets
MiniMax-M3, with sharedOCR_LLM_*credentials when dedicated scout secrets are omitted.ocr-router.js(bumped torouter-v5) centralizes scout env resolution:OCR_SCOUT_ENABLEDcan disable only the scout call; URL/key/model fall back throughOCR_SCOUT_*→OCR_LLM_*; scout metrics distinguish configured, disabled, and not_configured. MiniMax-style responses are normalized viaparseScoutJson(thinking tags, fenced JSON, brace extraction) so malformed output falls back to deterministic packet ranking withinvalid_json.The OCR workflow passes
OCR_LLM_URL/OCR_LLM_KEYinto the route step, defaultsOCR_SCOUT_ENABLEDto true andOCR_SCOUT_LLM_MODELtoMiniMax-M3. Full OCR on small/medium paths still uses modelreviewer.README documents the scout knobs and default-on behavior; routing tests cover config defaults, disable switch, thinking-wrapper parsing, and malformed JSON fallback.
Reviewed by Cursor Bugbot for commit d5c173f. Bugbot is set up for automated code reviews on this repo. Configure here.