Fix Trade Caravan

.claude/skills/pr-contribution-handler/SKILL.md

@@ -42,11 +42,13 @@ This skill lands contributor work, but **only after it meets the maintainer's ba
 
 7. **Never auto-enqueue a batch on the strength of a first-pass review.** Enqueue is effectively irreversible under the merge queue. Bring the maintainer the per-PR evidence (change summary, blast radius, regression/perf findings, test-discrimination result, architecture verdict, and large-refactor value verdict when applicable) and confirm authority. When authorized, enqueue only PRs that clear the full bar above — and improve the ones that fall short *first*.
 
-### Gemini review handling
+### Independent review pass
 
-- If a PR has **no Gemini review**, trigger one (`@gemini-code-assist review`) AND run a local `/code-review` — the bot's findings and the codebase-specific review are complementary.
-- If Gemini reports **daily quota exhaustion** (a `> [!WARNING] You have reached your daily quota limit` comment), stop triggering Gemini for the rest of the session and fall back to local `/code-review` only. Detect it by reading the Gemini comment body, not just by its presence.
-- A Gemini review may be **stale** (filed against an earlier commit). Confirm/refute each finding against the PR *head*, not the review timestamp — findings are routinely already fixed in later commits.
+The bar wants a second, independent look on top of `review-impl` — historically an external bot supplied it for free. **Gemini Code Assist has been sunset**: never post `@gemini-code-assist` (or any Gemini) triggers. The bot no longer answers, so summoning it spends a round-trip and posts dead-noise comments while adding zero coverage.
+
+- **CodeRabbit** is the free external reviewer for this public repo, and it reviews automatically on push — do not trigger it manually. When it has posted a review, treat its findings as the independent pass: confirm-or-refute each against the PR *head* (not the review timestamp — findings are routinely already fixed in later commits) and fold confirmed ones into the gate.
+- When **no external review exists** (CodeRabbit not yet run, or not installed) AND the PR trips a risk trigger — hot/shared path, new machinery, or the large-refactor value gate — run a local `/code-review` as the independent pass. Skip it for low-risk PRs an external review already covers: a local pass spends tokens, so scale it to blast radius rather than running it blanket.
+- Any external review — bot or human — may be **stale** (filed against an earlier commit). Always confirm/refute against the current head, never the review timestamp.
 
 ### Re-auditing merged PRs
 
@@ -67,7 +69,7 @@ Do not paraphrase these from memory. Re-read them each time because they are the
 A non-stop fleet exhausts GitHub's **5,000-req/hr REST `core`** bucket long before the separate GraphQL bucket. Minimize `core` reads:
 
 - **Scan diffs locally, not via API.** After fetching the PR head ref (needed for checkout anyway), use `git diff origin/main...pr/<N>` — never `gh pr diff <N>`, which spends one `core` request per PR and returns a truncatable payload. The local diff is free and carries the full patch (gitlink modes, binary flags, deletion counts).
-- **Comments: comprehensive via GraphQL for the gate; a windowed sweep only for triage.** Most feedback here is **top-level** — review bodies (Gemini's summary, human reviews) and issue comments — which have no resolved-flag, so the "all blocking comments resolved" gate must read every one. Fetch `reviews` + `comments` + inline `reviewThreads` comprehensively in one GraphQL call per PR (idle bucket), paginating every connection whose `hasNextPage` is true — never a time-windowed slice, which can drop an old unaddressed blocker (see `pr-review-comment-resolver.md` §2). A repo-wide `since=` REST sweep is acceptable **only** for lightweight "what's new across the batch" triage: derive `since` from the run window, dedup by comment `id` (`since` is inclusive), never a per-PR un-`since`'d `--paginate` walk (the top `core` drain). Do NOT persist a shared global high-water-mark file: concurrent fleet agents race on it and skip each other's new comments.
+- **Comments: comprehensive via GraphQL for the gate; a windowed sweep only for triage.** Most feedback here is **top-level** — review bodies (a bot's summary, human reviews) and issue comments — which have no resolved-flag, so the "all blocking comments resolved" gate must read every one. Fetch `reviews` + `comments` + inline `reviewThreads` comprehensively in one GraphQL call per PR (idle bucket), paginating every connection whose `hasNextPage` is true — never a time-windowed slice, which can drop an old unaddressed blocker (see `pr-review-comment-resolver.md` §2). A repo-wide `since=` REST sweep is acceptable **only** for lightweight "what's new across the batch" triage: derive `since` from the run window, dedup by comment `id` (`since` is inclusive), never a per-PR un-`since`'d `--paginate` walk (the top `core` drain). Do NOT persist a shared global high-water-mark file: concurrent fleet agents race on it and skip each other's new comments.
 - **Prefer GraphQL builders for state.** `gh pr view --json` / `gh pr list --json` / `gh pr checks` are GraphQL (the idle bucket) — they do not relieve `core`, but they are the right place to read PR state and check rollups.
 - **Back-pressure off response headers.** Watch `x-ratelimit-remaining` on calls you already make; consult `gh api rate_limit` at most once per batch (it counts against the *secondary* limit). When `core` runs low, pause **only at PR boundaries** — never inside the approve → label → enqueue → verify sequence (that critical section must not be interrupted; a half-enqueued PR is worse than waiting).
 
@@ -310,7 +312,7 @@ This expiry path is only for contributors not addressing requested changes. It m
 
 **Operational — posting and fetching.** Post every PR comment, review body, and final report through a temp file (`gh pr comment <N> --body-file /tmp/body.md`, `gh pr review <N> --body-file /tmp/review.md`), **never** an inline `--body "…"` string. Inline bodies are mangled by the shell — zsh strips backticked identifiers and code spans, which has silently corrupted the technical claims of a *blocking* review. Write the body to a file, then pass `--body-file`.
 
-**Fetching — gate vs triage.** For per-PR comment **resolution** (the gate), fetch `reviews` + `comments` + `reviewThreads` comprehensively via one GraphQL call per PR (the idle bucket — cheap even at fleet scale; query and pagination rules in `pr-review-comment-resolver.md` §2). Most feedback here is top-level review bodies and issue comments, which carry no resolved-flag — read every one's body for findings (Gemini posts its review as `COMMENTED` with `reviewDecision: null`, so do NOT gate on review `state`; treat `CHANGES_REQUESTED` as an additional hard block). The repo-wide `since=` REST sweep below is **only** for lightweight cross-batch triage of what changed recently — never the resolution gate, and never a per-PR un-`since`'d `--paginate` walk (that drains the 5,000/hr `core` bucket at fleet scale):
+**Fetching — gate vs triage.** For per-PR comment **resolution** (the gate), fetch `reviews` + `comments` + `reviewThreads` comprehensively via one GraphQL call per PR (the idle bucket — cheap even at fleet scale; query and pagination rules in `pr-review-comment-resolver.md` §2). Most feedback here is top-level review bodies and issue comments, which carry no resolved-flag — read every one's body for findings (review bots typically post as `COMMENTED` with `reviewDecision: null`, so do NOT gate on review `state`; treat `CHANGES_REQUESTED` as an additional hard block). The repo-wide `since=` REST sweep below is **only** for lightweight cross-batch triage of what changed recently — never the resolution gate, and never a per-PR un-`since`'d `--paginate` walk (that drains the 5,000/hr `core` bucket at fleet scale):
 
 ```bash
 gh api --paginate 'repos/phase-rs/phase/pulls/comments?since=<ISO8601>&per_page=100'   # inline review comments
@@ -552,7 +554,7 @@ Every item must be satisfied before running `gh pr merge`. Failing any item mean
 - [ ] **Adversarial second pass clean** for hot/shared-path or new-machinery PRs — "would a principal engineer merge this, or request changes?" answered, findings resolved.
 - [ ] **Architecture Review came back clean** (or all findings were resolved inline). No outstanding `class-of-cases-vs-special-case`, `building-block-reuse`, `CR-annotation-correctness`, or `engine/frontend boundary` issues left open.
 - [ ] **All blocking review comments resolved.** Author/reviewer comments tagged as required changes are addressed in commits; non-blocking nits may be deferred.
-- [ ] **No open finding more severe than the verdict.** If Gemini, another bot, or a reviewer raised an issue you would rate higher than your own verdict, you must confirm-or-refute it against the head with code evidence and resolve it before enqueue. A green "approve" never overrides an unaddressed higher-severity finding from another reviewer — reconcile it explicitly, do not enqueue past it.
+- [ ] **No open finding more severe than the verdict.** If a bot or a reviewer raised an issue you would rate higher than your own verdict, you must confirm-or-refute it against the head with code evidence and resolve it before enqueue. A green "approve" never overrides an unaddressed higher-severity finding from another reviewer — reconcile it explicitly, do not enqueue past it.
 - [ ] **Verification passed.** `cargo fmt` + the relevant Tilt resources (or fallback equivalents) reported green. If the PR touches engine/parser, `card-data` was included.
 - [ ] **No textual merge conflicts with `origin/main`.** Either the PR was already an ancestor descendant, or you merged main in cleanly. The queue can't speculate a rebase through textual conflicts.
 - [ ] **No explicit deferral was left that should have been finished in-PR.** Per "Explicit Deferrals" section ROI calibration — if a Frontier-tier PR left a deferral that ROI says you should have finished, finish it before enqueuing or report and stop.

.claude/skills/review-impl/SKILL.md

@@ -12,7 +12,7 @@ Review for gaps: things that are missing or wrong. Do not spend findings on styl
 1. Identify the changed surface from the diff, commit, or named files.
 2. Classify the surface area: engine logic, parser, frontend/UI, multiplayer/transport, AI heuristics, deck/format/feeds, build/CI/release, or docs.
 3. Apply only the relevant lenses below.
-4. If the scope is a PR, fetch existing bot/human review comments (Gemini, CodeRabbit, reviewers) and confirm-or-refute each against the current head with code evidence. Fold confirmed findings into your own. Never review in a vacuum — silently omitting a finding another reviewer already raised, or returning a verdict less severe than an open, unrefuted finding from another reviewer, is itself a defect.
+4. If the scope is a PR, fetch whatever external review comments exist (CodeRabbit, human reviewers) and confirm-or-refute each against the current head with code evidence, folding confirmed findings into your own. **Assume none exist by default** — Gemini Code Assist has been sunset, so no bot is guaranteed to have pre-screened this PR. Your own lenses are the complete review, not a supplement to a bot's; do not under-invest expecting a backstop. Where an external finding *does* exist, silently omitting it — or returning a verdict less severe than an open, unrefuted finding from another reviewer — is itself a defect.
 5. If the scope is a PR touching engine/parser source, the parse-diff sticky comment (marker `<!-- coverage-parse-diff -->`) is required evidence: fetch its full body and confront the card-level diff against the PR's claimed scope. Unexplained gained/lost/changed cards are findings (unintended parser blast radius). A *Baseline pending* body means the diff is unavailable — flag it so the handler brings the branch current to regenerate it; an absent comment despite changed engine source means CI evidence is missing for the current head.
 6. Report findings only. Silence means LGTM.
 
@@ -63,7 +63,7 @@ Two gates lead every review; apply them before the rest.
 ### Engine Logic
 
 - Verify every new or moved `// CR <rule>` by checking `docs/MagicCompRules.txt`; the cited rule must actually describe the code.
-- Compound CR annotations are the project's documented convention, **not** format violations: `CR X + CR Y` for interacting rules, `CR X / CR Y` for alternatives, and range/subpart forms like `CR 702.45a/b` (see CLAUDE.md "MTG Comprehensive Rules Annotations"). Do not flag the `+` / `/` / range forms as malformed or as regex violations — Gemini routinely raises these and they should be refuted, not echoed. Only flag a CR citation whose base number does not resolve in `docs/MagicCompRules.txt`, or one that does not describe the annotated code.
+- Compound CR annotations are the project's documented convention, **not** format violations: `CR X + CR Y` for interacting rules, `CR X / CR Y` for alternatives, and range/subpart forms like `CR 702.45a/b` (see CLAUDE.md "MTG Comprehensive Rules Annotations"). Do not flag the `+` / `/` / range forms as malformed or as regex violations — external reviewers routinely raise these and they should be refuted, not echoed. Only flag a CR citation whose base number does not resolve in `docs/MagicCompRules.txt`, or one that does not describe the annotated code.
 - Check reuse of building blocks in `parser/oracle_nom/`, `parser/oracle_util.rs`, `game/filter.rs`, `game/quantity.rs`, `game/ability_utils.rs`, `game/keywords.rs`, `game/zones.rs`, and `game/targeting.rs`.
 - Keep game logic in the engine. If player-visible state was added, verify multiplayer filtering.
 - For non-battlefield zones, player-scoped queries usually use `owner`, not `controller`.