Skip to content

Review 4545

Cindy Zhang edited this page Aug 28, 2026 · 1 revision

Review 4545 — feat(vibe-tests): add MiniMax design judge provider

PR

#4545 feat(vibe-tests): add MiniMax design judge provider by octo-patch (bucket: contributor)

HEAD REVIEWED

5a159f312d69797c0122073e8ccdb642c48ed5e6

This head includes a conflict-free current-main merge pushed to the contributor branch before review.

VERSIONS

LOOP VERSION: 1.6.0 AUDIT RUBRIC: 1.13

LANE

LANE: full WHY: external fork, new optional judge provider/CLI flags, environment-secret and provider-network behavior, and focused CI evidence needed; not fast-lane eligible.

UNTRUSTED PR GUARD

Changed paths: internal/vibe-tests/src/design-judge.ts, internal/vibe-tests/src/design-judge.test.ts.

No package script, dependency, lockfile, workflow, or hook changes. The diff adds runtime provider env/network code, so no live provider command was run; the only executed provider path was a mocked Vitest test.

PROBLEM

WHY 1: the design judge could score with the existing providers, but not with MiniMax-M3. WHY 2: vibe-test maintainers could not compare MiniMax output inside the same scoring workflow. WHY 3: the benchmark is only useful when each provider is measured through the same repeatable pipeline.

USER-FACING PROBLEM: a vibe-test maintainer trying to evaluate MiniMax output cannot use the existing design-score workflow for that provider. PROBLEM SEVERITY: missing capability — the workflow has no supported MiniMax provider path.

VERDICT: clear

SOLUTION

The CLI now chooses a judge provider and region, fills default model/region values, and records them in the score output. The MiniMax path sends the same image+prompt payload to the selected region endpoint with Bearer auth, then reuses the shared JSON extraction and weighted-score recomputation. The new tests stub fetch, set a fake key, and verify MiniMax request/response behavior without network egress.

SOLUTION (2 decisions · ~287 runtime lines of 492)

  1. Add MiniMax as an optional vision provider with global_en and cn_zh endpoints, MINIMAX_API_KEY, and Anthropic-shaped payload/response handling.
  2. Add --provider/--region parsing, defaults, validation, and score-output traceability.

BURDEN: medium — no dependency/script/workflow change, no React Effects, one new external provider request path per pass, one new test file. BURDEN MATCH: proportionate — the capability is specifically to route the existing judge to a second provider and keep scoring shared.

VERDICT: clear

ARCHITECTURE

OWNER: internal/vibe-tests design judge CLI. TIER 1: none — internal evaluation tool. TIER 2: none. SEAMS: CLI args, provider helper, design-scores JSON consumed by universal-aggregate.ts. BEHAVIOR UNIT: pure helpers + provider function — request construction and score normalization are testable without the CLI.

seam driven result
MiniMax provider helper focused Vitest stubs fetch and verifies global/cn_zh endpoints, Bearer auth, base64 image blocks, response parsing, missing-key and non-2xx errors
CLI parse/dry-run --provider minimax --region cn_zh --dry-run prints the selected provider/region and completes without API calls
aggregation existing aggregator reads summary/results/passCount; added provider/region fields are ignored-compatible

VERDICT: clear

IMPACT

The change reaches vibe-test maintainers who opt into --provider minimax; the default Anthropic path stays the default. The new provider can score the same screenshots through MiniMax and records provider/region in the artifact, so later comparisons know which judge produced them.

VERDICT: clear

API

Internal CLI/API change only.

change public? class doc'd? verdict
+ `--provider anthropic minimaxdefaulting toanthropic` internal CLI existing CLI option pattern file header usage updated
+ `--region global_en cn_zhdefaulting toglobal_en`, validated against selected provider internal CLI provider-region selector file header usage updated
+ DesignScores.provider?, DesignScores.region? internal JSON artifact traceability metadata output writer updated ok
+ exported callMiniMaxVisionJudge(...) internal module function provider helper covered by focused test ok

OSSIFICATION: internal-only, not package surface; the CLI validates provider and region before any provider call.

VERDICT: clear

THEMING

No component styling, theme target, token, or CSS change.

VERDICT: clear

BREAKING

BEHAVIOR: default provider remains Anthropic; MiniMax only runs when selected. API: no public package API change; internal CLI grows optional flags. VISUAL: no rendered component or docs output changes. THEME: no theme target/token/override changes.

VERDICT: clear

PERFORMANCE & RESOURCES

EFFECTS: zero.

RENDER: n/a — non-React internal CLI. LISTENERS/OBSERVERS: none. LAYOUT: none. BUNDLE: no dependency or lockfile change. NETWORK: MiniMax adds one external fetch per judge pass when explicitly selected, using the same sequential pass loop and 500ms inter-pass delay as the existing design judge. No live provider request was made during review.

VERDICT: clear

VISUAL EVIDENCE

VISUAL CHECK: not applicable WHY: only internal CLI/test files changed; no component, style, DOM, Storybook, docsite render, or visual artifact output changed.

VERDICT: clear

REMEDY SEARCH

REMEDY SEARCH: not triggered — no proven visual defect

A11Y & I18N

No user-facing component strings, ARIA, keyboard, focus, locale, or RTL behavior changed. CLI messages and provider errors are internal-tool output.

VERDICT: clear

JUDGEMENT

slot verdict
PROBLEM clear
SOLUTION clear
ARCHITECTURE clear
IMPACT clear
API clear
THEMING clear
BREAKING clear
PERFORMANCE clear
VISUAL clear
A11Y & I18N clear

GOAL: met — mocked MiniMax tests cover endpoint selection, auth scheme, image payload, JSON extraction, weighted score recompute, missing key, and non-2xx error handling; valid MiniMax dry-run completes without a provider request.

DISPOSITION: explicit any types in the new test file → blocks now; focused ESLint fails on @typescript-eslint/no-explicit-any at lines 50, 66, and 70.

ADVICE: bounded outcome criteria — make the touched-file ESLint invocation clean without loosening the lint rule.

AUTHOR CAN PROCEED: yes — replace the three any annotations with concrete or unknown types and rerun lint.

WORST OUTCOME: “the repo checks won't pass as written” → request changes.

JUDGEMENT NEEDED: none — internal provider capability; the only blocker is mechanical lint failure.

request changes

  1. [BLOCKS] the new MiniMax test file has three explicit any annotations → the next CI lint run fails before the provider change can merge · internal/vibe-tests/src/design-judge.test.ts:50, :66, :70

REVIEW

Thanks for adding this. One blocking thing before this can land: the new test file currently fails ESLint on three explicit anys, so the repo checks won't pass as written. Could you type the recorded request body and fetch input with concrete/unknown types and rerun lint? If you'd rather talk it through with someone, we're in Discord.

[Reviewed by Robohands]

INLINE (0–3)

  • internal/vibe-tests/src/design-judge.test.ts:50 — This any makes the new test file fail ESLint; could it be unknown?
  • internal/vibe-tests/src/design-judge.test.ts:66 — Could this use the fetch input type instead of any?
  • internal/vibe-tests/src/design-judge.test.ts:70 — Could this be unknown until after the parse?

CRITIC PASSES

Draft 1: request-changes on focused ESLint failure. Critique failed nothing material: verdict matched evidence, block had two confirmations (ESLint output + anchored lines), review stayed under the request-changes word cap, and the Discord clause was present for an outside contributor.

Draft 2: shortened the public wording and kept the same single blocker. Critique passed: no rubric IDs or private control fields in the public text, no visual claim without frames, author can proceed, and every anchor was re-read at the reviewed head.

EVIDENCE I DID NOT SPEND

  • No prior GitHub reviews or review comments on the PR.
  • git diff --check is clean.
  • Current main did not change internal/vibe-tests/src/design-judge.ts while this PR aged; the required merge was conflict-free.

TIME

TIME total 18m setup/rules 6m — loop/rubric/wiki authority and PR guard refresh 2m — current-main merge and non-force push reading 4m — PR body, diff, source, existing judge seams, review history checks 4m — focused mocked Vitest, dry-run CLI, focused ESLint writing/wiki 2m — draft, critique, final record waste 0m — no listener or Storybook run

WHAT I COULD NOT VERIFY

  • GitHub Actions are still action_required on the fork head, so full CI has not run.
  • I did not make any live Anthropic or MiniMax provider request.

What changed before posting

Not posted to GitHub by instruction; drafted as request-changes.

Clone this wiki locally