Repository navigation
Conversation
A review avoided every model the PR's record named, so once Sol and Opus had both written a PR no listed reviewer was left, and agents picked an unlisted model themselves. It now avoids only the model of the record's last Implementer line, and each session moves its line last whenever it pushes, so that line names the latest writer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The body's and kix's sessions comment's last Implementer lines can name different models, and which pushed last cannot be told from them, so the running harness, usually the session that just pushed, is avoided then. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 7 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: 3003714
Compared with base: f3a9069
[P2] Reconcile iterate writer selection with the reordered record — src/github/models.ts:61-65
Moving an existing first Implementer: line to the end changes more than reviewer selection: assets/AGENTS.md's Coordination section still instructs agents to dispatch /quest-iterate to the model on the first line, and README's “Route work by size” section and quest models --writer help retain the same rule. For a body whose sessions are [Opus, Sol], an additional push by the original Opus session now produces [Sol, Opus], so the next iterate is dispatched to Sol rather than staying on the PR's writer as those instructions require. This affects the mixed-writer PRs this change explicitly supports.
Preserve the stable writer separately from recency, or update those routing instructions and help to an explicitly chosen writer-selection rule. Cover the [A, B] → A-push case so latest-writer tracking does not silently change iterate ownership.
The latest-writer review exclusion itself follows the stated direction: earlier writers remain eligible, the two record locations are kept separate, ambiguous records fall back to the running harness, and explicit exclusions retain precedence.
Validation: static review through the GitHub connector of the changed code, surrounding record/publication callers, documentation, and updated tests. I did not run builds or tests, and have not independently verified the PR body's reported passing checks. Submitted as a COMMENT, not an approval.
(Written by review (OpenAI))
Problem
quest reviewavoided every model the PR's## Modelsrecord named. Once Sol and Opus had both written a PR, as happens when an iterate runs on the other harness, no listed reviewer was left. The error saidpass --harness and --model, so agents picked an unlisted model themselves (Claude Fable 5.1 reviewed moq-dev/rind#455 and moq-dev/rind#483).Approach
src/cli/review.ts,src/cli/review-record.ts): a review avoids only the latest writer, the model of the record's lastImplementer:line. Earlier writers may review. The body's and kix's sessions comment's last lines are kept apart: when they name different models, which pushed last cannot be told, so the running harness, usually the session that just pushed, is taken to be the latest writer, as it is with no record or anunknownlast model.--harnessthen avoids every model they name.--exclude-modelstill names every model to avoid.src/github/models.ts):withModelsmoves a session's line last when it pushes again instead of leaving it where it was, so the last line names the latest writer. kix's sessions comment uses it throughwithSession. A redelivered PR body (cloudflare/src/github/service.ts) keeps earlier sessions with the delivering session's own lines last.quest review --help, README, getting-started, and the model-routing epic's Plan, which records the decision.Impact
is avoidedinstead ofwrote the code.Decisions
Maintainer:item instead of the hint to pass a model; add a third reviewer to each size; name fallback reviewers in a repository's AGENTS.md).Validation
just checkpasses.just testpasses on the final head: 1184 tests, 5 skipped, plus the Worker's 540. The review skill's stubs were regenerated withquest init.Models
Size: S
Implementer: Claude Code, claude-opus-5-5, high effort
(Written by Claude Opus 5.5 with high)
🤖 Generated with Claude Code
Review
Rewritten by
quest reviewafter each round.01a1b8ae-f07c-7fa0-a383-3e65b2cafe56, round 2The updated selection fixes the reported self-review scenario, and the incremental changes introduce no findings.src/cli/review.ts:191:Resolve the latest writer across both record locations;src/cli/review-record.ts now preserves each record's latest model separately, and authored() in src/cli/review.ts avoids the running harness when those models disagree, preventing the local Sol session from selecting itself in the reported scenario.