docs(claude): review-pr lanes + cost metering, and correct the host policy

docs/claude/commands.md

@@ -202,15 +202,48 @@ Review one or more GitHub Pull Requests. Fetches the diff, detects prior reviews
 
 **What it does:**
 
-1. **Detects re-reviews** — checks if anything has changed since your last review; skips if not
-2. **Classifies sensitivity** — auto-detects auth/RBAC/CI code and recommends Strict mode
-3. **Asks strictness** — Quick, Standard, Thorough, or Strict
-4. **Analyzes the diff** — runs in parallel sub-agents for batch mode; looks for bugs, null-safety issues, SQL parity, test coverage gaps, and more
-5. **Posts inline comments** — one finding per comment, severity marked with 🔴/🟡/🟢; never bundles multiple findings in one comment
-6. **Offers local testing (optional)** — when the PR touches frontend or backend code, asks whether to verify the changes locally; locates or clones the target repo, checks Docker is running (starts it or asks the user to), maps detected layers to applicable `/test-*` skills, builds a test plan, gets your approval, then executes — any new issues join the existing findings before the verdict
-7. **Checks CI** — blocks APPROVE if required CI checks are failing
-8. **Submits formal review** — APPROVE (no blockers) or REQUEST_CHANGES (one or more 🔴 findings)
-9. **Resolves addressed threads** — replies "✅ Resolved in {sha}" to previously raised comments now fixed, and marks threads closed
+1. **Checks batch scope first** — when given several PRs, detects whether one PR's head already contains another's commits (a retargeted stacked branch) and asks how to scope before staging anything. Reads `baseRefName` from the API, never the PR body, which goes stale the moment a branch is retargeted
+2. **Classifies each PR into a lane** — `settled` / `delta` / `full`, before any diff, gate, guide, clone or sub-agent runs. See *Lanes* below
+3. **Classifies sensitivity** — auto-detects auth/RBAC/CI code and recommends Strict mode
+4. **Asks strictness** — Quick, Standard, Thorough, or Strict. Always asked; never auto-selected
+5. **Consumes CI instead of repeating it** — when the repo's own gate job is green on *exactly* the PR's head SHA, that is taken as the evidence. A red gate check still gets a local run
+6. **Analyzes the diff** — parallel sub-agents in batch mode; Quick/delta PRs from one repo may share a single agent (max 4). Sub-agents work to a stated tool-call and report-length budget per strictness mode
+7. **Posts inline comments** — one finding per comment, severity marked with 🔴/🟡/🟢; never bundles multiple findings in one comment
+8. **Offers local testing (optional)** — when the PR touches frontend or backend code, asks whether to verify the changes locally; locates or clones the target repo, checks Docker is running (starts it or asks the user to), maps detected layers to applicable `/test-*` skills, builds a test plan, gets your approval, then executes — any new issues join the existing findings before the verdict
+9. **Checks CI** — blocks APPROVE if required CI checks are failing
+10. **Submits formal review** — APPROVE (no blockers) or REQUEST_CHANGES (one or more 🔴 findings)
+11. **Handles prior threads by author** — resolves threads *you* opened that are now addressed; on a co-reviewer's thread it posts an acknowledgment reply and leaves the thread for its author to close. Resolving someone else's comment misrepresents their agreement
+12. **Records what the run cost** — one line appended to `~/.claude/metrics/skill-runs.jsonl`, outside any repo. See *Measuring cost* below
+
+**Lanes** — decided per PR before any expensive step:
+
+| Lane | When | What runs |
+| ---- | ---- | --------- |
+| `settled` | Merged/closed **and** no commits since your last review | Nothing. One row in the summary table, nothing posted. A merged PR already carrying your APPROVE never gets a second one |
+| `delta` | Re-review with new commits, or a non-open PR that moved | Analysis scoped to `compare(lastReviewedSha, headSha)` — not the full PR diff |
+| `full` | Open PR you have not reviewed before | The whole pipeline |
+
+The lane is why a third review round costs a fraction of the first: the rest of the PR was already reviewed, and re-reading it is what made every round cost the same.
+
+**Measuring cost:**
+
+```bash
+# what a run cost (after the review; --prs makes batches comparable)
+python3 .claude/skills/review-pr/scripts/skill-metrics.py record --skill review-pr --prs 3 --mode Standard
+
+# compare runs, grouped by the skill's git SHA
+python3 .claude/skills/review-pr/scripts/skill-metrics.py report --skill review-pr
+
+# measure a skill EDIT without running a review at all
+python3 .claude/skills/review-pr/scripts/skill-metrics.py footprint
+
+# reconstruct past runs from transcripts already on disk
+python3 .claude/skills/review-pr/scripts/skill-metrics.py baseline --skill review-pr --all-projects
+```
+
+Claude Code tags every assistant message with `attributionSkill`, so the runtime meter reads real usage rather than estimating it. Sub-agent-internal tokens are **not** measured — sub-agents write no local transcript — so the meter records agent count, model and tool-result volume as proxies instead of inventing a number. Method and caveats: [hydra/.claude/skills/review-pr/references/metrics.md](https://github.com/ConductionNL/hydra/blob/main/.claude/skills/review-pr/references/metrics.md).
+
+The measurement data lives outside the repo on purpose (per machine, per operator, churns every run). The tooling is in the repo; the measurements are not.
 
 **Model:** Requires Sonnet or Opus — stops immediately on Haiku. Batch mode lets you choose the model for parallel analysis agents (Sonnet default, Opus for security-sensitive batches).
 

docs/claude/skill-evals.md

@@ -114,6 +114,38 @@ The previous `local-mods.patch` mechanism is gone — the merge replaces it. If
 
 ---
 
+## Evals measure quality, not cost
+
+An eval suite answers "is the skill still good?". It does not answer "what does the
+skill cost to run?" — and a change that makes a skill cheaper can quietly make it
+worse, which is exactly the failure an eval exists to catch.
+
+Treat the two as a pair. `review-pr` carries the reference implementation:
+
+```bash
+# cost, from real runs — reads Claude Code's own transcripts
+python3 .claude/skills/<skill>/scripts/skill-metrics.py record --skill <skill>
+python3 .claude/skills/<skill>/scripts/skill-metrics.py report --skill <skill>
+
+# cost of an EDIT, without running the skill at all
+python3 .claude/skills/<skill>/scripts/skill-metrics.py footprint --skill-dir .claude/skills/<skill>
+```
+
+The meter reads `attributionSkill`, which Claude Code writes on every assistant
+message, so it measures rather than estimates. It does **not** measure sub-agent
+internals — sub-agents write no local transcript — and it records agent count, model
+and tool-result volume as proxies instead of inventing a figure.
+
+**The rule: never accept a cost reduction without re-running the evals.** Record the
+cost delta and the eval pass rate in the same commit message, so a later reader can
+see both halves of the trade.
+
+⚠️ **`timing.json` in `workspace/` is not a cost baseline.** In the `review-pr` suite
+several of these files are byte-identical across *different* evals — hand-entered, not
+measured. Use the metrics store for cost; use the eval workspace for quality.
+
+See `hydra/.claude/skills/review-pr/references/metrics.md` for the full method.
+
 ## `baseline_score` — Regression Detection
 
 Even running evals manually (no CI), `baseline_score` is useful: it's the with-skill pass rate from the most recent successful eval run, recorded in `evals/evals.json` next to `last_validated`. When you re-run evals later, compare the new pass rate against `baseline_score`:

global-settings/tests/test-block-polling.sh

@@ -1,4 +1,5 @@
 #!/usr/bin/env bash
+# shellcheck disable=SC2016,SC2088 # literal $VAR / ~ tokens are intentional test fixtures fed to the hook as data
 # test-block-polling.sh — synthetic test harness for block-polling.sh.
 #
 # Feeds PreToolUse envelopes to the hook and asserts exit 2 (deny) or 0 (allow).

hydra-gates/scripts/lib/test_gate_mainline_push_audits_everything.sh

@@ -78,13 +78,23 @@ _out="${_tmp}/run.txt"
 (
     cd "${_app}" || exit 1
     # HYDRA_GATE_PUSH_BEFORE deliberately unset — this is the unresolvable case.
+    # GITHUB_EVENT_NAME=push is what makes it a PUSH whose tip is unresolvable,
+    # rather than a caller who simply passed a --base equal to HEAD. The runner
+    # distinguishes the two deliberately (see run-hydra-gates.sh, "the fallback
+    # is now reserved for the situation it was written for"), and only the push
+    # gets the full-tree fallback. Without this variable the test was asserting
+    # #183's guarantee against a context that never triggers it.
     unset HYDRA_GATE_PUSH_BEFORE
-    HYDRA_GATE_LOG_DIR="${_logs}" bash "${_runner}" \
+    GITHUB_EVENT_NAME=push HYDRA_GATE_LOG_DIR="${_logs}" bash "${_runner}" \
         --scope-to-diff --base development . > "${_out}" 2>&1
 )
 _rc=$?
 
-_lines=$(grep -cE '^\[gate-[0-9]+\]' "${_out}" 2>/dev/null || echo 0)
+# `grep -c` already prints 0 when it matches nothing AND exits 1, so a
+# `|| echo 0` appends a SECOND zero and `[ "0\n0" -gt 0 ]` dies with
+# "integer expression expected" — the assertion below then never ran.
+_lines=$(grep -cE '^\[gate-[0-9]+\]' "${_out}" 2>/dev/null || true)
+_lines=${_lines:-0}
 
 if [ "${_rc}" -eq 99 ]; then
     _bad "the runner exited 99 on a mainline push — it gated nothing (#183)"
@@ -138,6 +148,40 @@ else
     _ok "a resolvable base is still scoped narrowly (the fallback did not swallow it)"
 fi
 
+# THE OTHER HALF OF THE SAME DECISION. The full-tree fallback is reserved for a
+# push whose previous tip is unresolvable. With NO push context at all, a --base
+# that equals HEAD is a caller mistake with two honest readings, and the runner
+# refuses rather than silently auditing everything. That refusal is deliberate
+# and was previously untested — so removing it would have turned this file green
+# by widening the fallback, which is the failure this whole suite exists to catch.
+_out3="${_tmp}/run3.txt"
+_logs3="${_tmp}/logs3"
+mkdir -p "${_logs3}"
+(
+    cd "${_app}" || exit 1
+    unset HYDRA_GATE_PUSH_BEFORE
+    unset GITHUB_EVENT_NAME
+    HYDRA_GATE_LOG_DIR="${_logs3}" bash "${_runner}" \
+        --scope-to-diff --base development . > "${_out3}" 2>&1
+)
+_rc3=$?
+# 99 alone is not enough: the runner exits 99 for setup failures too (no git, no
+# app dir, unreadable tree), so asserting only the code would keep this green if
+# the refusal were deleted and something else broke instead. Require the refusal
+# to NAME itself.
+if [ "${_rc3}" -eq 99 ] && grep -q "resolves to HEAD" "${_out3}"; then
+    _ok "no push context + base == HEAD is refused (99) and says why, not silently widened"
+elif [ "${_rc3}" -eq 99 ]; then
+    _bad "exited 99 without the refusal message — that is a setup failure wearing the refusal's exit code"
+else
+    _bad "base == HEAD with no push context exited ${_rc3} — the refusal is gone, and a caller mistake now reads as a full audit"
+fi
+if grep -q 'FULL-TREE AUDIT' "${_out3}"; then
+    _bad "the refusal path still fell back to a full-tree audit — the two cases are no longer distinguished"
+else
+    _ok "the refusal names the mistake instead of substituting an audit nobody asked for"
+fi
+
 echo
 if [ "${_failures}" -eq 0 ]; then
     echo "test_gate_mainline_push_audits_everything.sh: ALL PASS"