Turn the reviewer chain into a shared Provider/Model/Optional/Max-Iterations table

[atomantic]

Summary

Code Review Defaults rendered reviewers as a chip row plus a stack of panel-local per-backend model <select>s below it. That model-picking logic lived in CodeReviewDefaultsPanel.jsx, never in the shared ReviewerPicker.jsx — so the three other surfaces that reuse the picker (TaskAddForm.jsx, GlobalConfigControls.jsx, SlashDoRunDrawer.jsx) had no way to set a reviewer's model at all.

This moves the model picker into ReviewerPicker and lays each reviewer out as a row with all four of its controls as columns: Provider | Model | Optional | Max Iterations. Reorder / remove / ~opt / ~max all move into the row and keep working. Under sm the row collapses to a stacked labelled block rather than scrolling horizontally.

Closes #3133 (#3123 shipped in #3146, so the Max Iterations column is included rather than deferred).

The picker stays fetch-free. Callers pass resolved option lists via modelOptions, produced by the new useReviewerModelOptions hook (local-LLM ids from /api/local-llm/status, codex/claude tiers from /api/providers, with claude's list also spanning installed Ollama ids for an Ollama-backed CLI). All four surfaces therefore offer the same options. Passing nothing degrades every Model cell to free-text rather than hiding the column.

Model cell shape follows what's knowable. A local backend's installed list is authoritative, so it renders a closed <select>; a CLI reviewer renders text + <datalist>, because an Ollama-backed claude or a Bedrock-form id is environment-specific and a closed list would lock out valid ids. A pin the probe no longer lists still gets an <option> so the select can't render blank and read as unset.

The pin now travels per task, not just per install. reviewerModels is a token-keyed map alongside reviewerMaxRounds, with the same normalizer / resolver / task-over-default precedence and the same absent-vs-empty discipline (an explicit {} means "each reviewer's own default"; clearing a field DELETES the key rather than persisting '', which would emit a --model with no id). It is emitted as slashdo's [<model>] selector, placed between the slug and the ~opt/~max suffixes — slashdo's parser strips those suffixes from the right before reading the bracket, so the order is load-bearing. copilot/@login/lmstudio never get a bracket (no slashdo slug takes one).

Local-LLM pins ride the same map deliberately. POST /api/code-review/local resolves its default model from global settings and has never seen the task, so a per-task local pin would silently do nothing if dropped as a CLI-only concern. resolveReviewLoopOptions keeps it and the follow-up prompt names it in the request body instead.

Persisted encoding is unchanged. settings.codeReview keeps its <reviewer>Model scalars — that encoding crosses installs, so rewriting it to a map would need a migration for zero gain. client/src/lib/reviewerModels.js (mirroring reviewerModelsFromDefaults server-side) is the one adapter between the scalars and the map. No migration needed.

From the review rounds

• The run drawer's reviewer controls are now gated on isNext. Only the next branch of POST /tasks/slashdo reads the reviewer fields; every other /do:* body owns its own review/PR sequence. Rendering the picker there let a user choose a reviewer and have the run discard it — pre-existing for the three older controls, and the Model column would have been a fourth. The submit payload states the same contract rather than leaning on the picker being unrendered.
• A model id can no longer corrupt the emitted token. [, ], , and line breaks are structural in [<model>] with no escape — foo]~opt closes the selector early and leaves slashdo parsing the remainder as a suffix, and a comma splits one entry into two. Such an id is dropped server-side (the reviewer falls back to its own default rather than running against a corrupt list) and stripped as the user types. A space stays legal: agy[Gemini 3.5 Flash (High)] is a valid slashdo entry.
• The stored scalars are validated too. They previously accepted any string, so a hand-edited settings.json could hold an id the token builders then drop — the picker would display a pin no reviewer ever received. All three paths (map normalizer, scalars at write time, reviewerModelsFromDefaults on read) now share one normalizeReviewerModel check. pickCodeReviewDefaults stays a faithful mirror on purpose: those scalars also feed /api/code-review/local as a JSON body field where a delimiter is harmless.

Test plan

• cd client && npm test — 470 files / 5242 tests pass. ReviewerPicker.test.jsx grew from 29 to 46 tests: all 15 pre-existing reorder/remove/~opt/~max cases pass unmodified against the new table (the contract didn't change, only the layout), plus new coverage for the Model column — which reviewers get a cell, select-vs-free-text, an uninstalled pin staying visible, clear-deletes-rather-than-empty-string, whitespace-only treated as a clear, delimiter stripping, pruning on remove, the three per-row controls staying independent, and case-insensitive key reads. SlashDoRunDrawer.test.jsx adds two cases pinning the non-next gate.
• cd server && npm test — the reviewer-related suites (validation, cos routes, settings routes, codeReview, cosTaskGenerator, agentPromptBuilder, cleanupAgentWorktree) pass: 659 tests. New server coverage for normalizeReviewerModels / resolveReviewerModels / reviewerModelsFromDefaults (including the re-check on read), the [<model>] emission in both token builders (bracket-before-suffix ordering, the never-bracket cases, delimiter rejection), the sanitizer, and both schemas.
  • 60 files fail in a local full server run — all pre-existing and unrelated: they require PostgreSQL, which isn't provisioned in this environment. Verified by running a sample (pipeline/seriesStore/store, writersRoom/store, lib/dataRoot) on clean main, where they fail identically. CI, which does provision Postgres, is green.
• cd client && npm run lint clean; npm run build succeeds.
• All CI checks pass: lint, test (24.x), Client tests and build, DB tests and server smoke, Plan test impact, CI Gate.