Skip to content

Let CoS tasks cap review rounds per reviewer via slashdo's ~max=<n> suffix #3123

Description

@atomantic

Problem / Goal

slashdo v3.25.0 (the pinned submodule at lib/slashdo, commit 1f70b7e) added a
per-reviewer iteration cap: any --review-with slot may be suffixed ~max=<n>
--review-with claude~max=2,ollama~max=1,codex~max=3 — budgeting how many
review → fix → re-review cycles that one reviewer runs. Before this, the local
agents (codex/claude/agy/grok) and ollama had hardcoded 3-iteration caps
with no way to move them, which makes a slow local model expensive to include in a
CoS Review Loop.

PortOS emits --review-with for every CoS agent task but has no way to express a
cap, so every local reviewer still burns up to 3 full cycles. The goal: expose a
per-reviewer max-rounds control in the task-add form (and the surfaces that share
the same picker) and thread it into the emitted flag.

Context

The ~opt (non-blocking) suffix — shipped in the same slashdo grammar slot — is the
exact precedent to mirror; ~max=<n> is a sibling suffix that chains with it in
either order (ollama~opt~max=1ollama~max=1~opt), and slashdo's canonical
storage order is ~opt first, then ~max=<n>.

Existing ~opt plumbing, end to end:

  • server/lib/cosValidation.js:117 normalizeOptionalReviewers — normalizes the
    token list (keyed slug or @user), aliases geminiantigravity, dedupes.
  • server/lib/cosValidation.js:149 resolveOptionalReviewers — task-over-default
    precedence (an explicitly empty array overrides the Code Review Defaults).
  • server/lib/cosValidation.js:163 markOptional — appends the suffix to a token.
  • server/lib/cosValidation.js:221 buildReviewersCsv — prompt {reviewers} CSV.
  • server/lib/cosValidation.js:244 buildReviewWithArgs — the flag string.
  • server/lib/cosValidation.js codeReviewSettingsSchema (optionalReviewers
    field) and the task-metadata sanitizer (optionalReviewers branch, which KEEPS an
    explicitly empty array).
  • Consumers: server/services/agentPromptBuilder.js (:400, :1053, :1352, :1475,
    :1580, :1750), server/services/agentWorktreeCleanup.js (:46, :232, :298, :320),
    server/services/cosTaskGenerator.js (:332, :369, :2202), server/services/codeReview.js
    (:70, :132, :145), server/routes/cosTaskRoutes.js:130, server/services/prWatcher.js:332.
  • Client: client/src/components/cos/ReviewerPicker.jsx:82 (renderOptToggle, the
    per-chip badge), client/src/components/cos/TaskAddForm.jsx (:26, :307, :556),
    client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx,
    client/src/components/providers/CodeReviewDefaultsPanel.jsx,
    client/src/hooks/useCodeReviewDefaults.jsx, client/src/components/apps/SlashDoRunDrawer.jsx.

The keyed-map storage precedent already in the codebase is
metadata.reviewLoopReviewerModels (server/services/agentPromptBuilder.js:413) —
a reviewer-keyed object threaded into the review-loop prompt.

PortOS does not use slashdo's --review-iterations flag anywhere (grep is empty), so
~max=<n> is the only cap surface that needs building.

Proposed approach

Mirror the ~opt plumbing with a per-reviewer numeric cap, stored as a
token-keyed map rather than a list.

  1. Schema/storage — add reviewerMaxRounds: Record<token, number> where token
    is an emitted --review-with token (a REVIEWER_VALUES slug or @user,
    matching normalizeOptionalReviewers' identity rule). Add a
    normalizeReviewerMaxRounds(map) + resolveReviewerMaxRounds(taskMap, defaultMap)
    pair in server/lib/cosValidation.js next to their optionalReviewers siblings:
    drop unknown tokens, coerce to a non-negative integer, drop non-integers /
    negatives, and drop entries with no cap so absent ≠ 0 (slashdo reads 0 as
    "loop until clean", so a dropped key and 0 must not collapse).
  2. Emission — extend markOptional into a markSuffixes(token, optSet, maxMap)
    that appends ~opt then ~max=<n> in slashdo's canonical order, and use it from
    both buildReviewersCsv and buildReviewWithArgs.
  3. Settings — add the field to codeReviewSettingsSchema (.strict(), so it
    must be declared) and to the task-metadata sanitizer, keeping the same
    "explicitly empty overrides the default" semantics optionalReviewers has.
  4. Threading — pass reviewerMaxRounds through every call site that already
    carries optionalReviewers (list above), including the review-loop follow-up
    spawner in agentWorktreeCleanup.js and codeReview.js's resolver.
  5. UI — in ReviewerPicker.jsx, render a compact numeric cap control on each
    reviewer/username chip beside the ~opt badge (blank = slashdo's built-in
    default; 0 = loop until clean, labeled as such). Since the picker is shared, the
    control lands in TaskAddForm, GlobalConfigControls, CodeReviewDefaultsPanel,
    and SlashDoRunDrawer at once. Clean up the map entry when a reviewer/username is
    removed, exactly as withoutToken does for optionalReviewers.
  6. Tests — extend server/lib/validation.test.js, server/routes/cos.test.js,
    server/services/agentPromptBuilder.test.js, server/services/cosTaskGenerator.test.js,
    server/services/codeReview.test.js, and client/src/components/cos/ReviewerPicker.test.jsx
    alongside their existing optionalReviewers cases.

Acceptance criteria

  • A CoS task created from the task-add form can set a max-rounds cap per keyed
    reviewer and per @username reviewer, persisted in task metadata.
  • buildReviewWithArgs emits <token>~max=<n>, and <token>~opt~max=<n> when
    both are set (~opt first), for exactly the tokens that carry a cap.
  • buildReviewersCsv emits the same suffixes so the {reviewers} prompt
    placeholder matches the flag string.
  • A cap of 0 round-trips as ~max=0 and is distinguishable from "no cap set"
    (which emits no suffix).
  • Non-integer, negative, and unknown-token entries are dropped by the normalizer
    rather than reaching the flag string.
  • Code Review Defaults can set caps, and a task-level map (including an
    explicitly empty one) overrides them — same precedence as optionalReviewers.
  • Removing a reviewer or username from the picker drops its cap entry.
  • Server + client tests cover normalization, emission ordering, precedence, and
    the picker control; cd server && npm test and cd client && npm test pass.

Out of scope

  • Surfacing slashdo's --review-iterations flag (PortOS uses none of it today; the
    per-entry suffix supersedes it for every reviewer type).
  • Rendering slashdo's new capped status or its Iterations column back into the
    PortOS Review Loop UI — a separate follow-up if wanted.
  • Fixing the pre-existing quirk that PortOS emits lmstudio (and antigravity
    rather than agy) into --review-with, which are outside slashdo's accepted slug
    set. A ~max on those tokens is inert for the same pre-existing reason.
  • --review-mode parallel, where slashdo ignores ~max entirely.

Metadata

Metadata

Assignees

Labels

area:cos-agentsChief-of-Staff autonomous agentsenhancementNew feature or request

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions