[no-ticket] Add review prompt eval set and dry run mode

.github/workflows/biggiepockets-review.yml

@@ -46,6 +46,17 @@ on:
         required: false
         default: main
         type: string
+      dry_run:
+        description: >-
+          Run the review without submitting it. The verdict is uploaded as a
+          `review-verdict-pr<pr>` artifact instead of being posted, which is what the
+          prompt evals in the registry's evals/ use to score the prompts against pull
+          requests whose correct outcome is already known. Everything upstream of the
+          submit step is unchanged on purpose: an eval that runs a different pipeline
+          measures a different pipeline.
+        required: false
+        default: false
+        type: boolean
 
 jobs:
   # Codex and Claude intentionally run in separate jobs. GitHub can only re-run a
@@ -319,6 +330,18 @@ jobs:
       # Stage 2: Claude verifies Codex's findings, reviews independently, and decides.
       # This is the only Claude pass, running whichever arm's prompt was assigned to
       # this PR. Its verdict is the review.
+      # Allocated here rather than inside the Datadog step below, because the eval
+      # metrics the prompt evals attach to this review have to join on the same span ids
+      # this trace is submitted under, and that job runs after this one.
+      - name: Allocate LLM Obs trace ids
+        run: |
+          {
+            echo "TRACE_ID=$(openssl rand -hex 16)"
+            echo "ROOT_SPAN_ID=$(openssl rand -hex 8)"
+            echo "CODEX_SPAN_ID=$(openssl rand -hex 8)"
+            echo "CLAUDE_SPAN_ID=$(openssl rand -hex 8)"
+          } >> "$GITHUB_ENV"
+
       - name: Record Claude start time
         run: echo "CLAUDE_START_NS=$(date +%s%N)" >> "$GITHUB_ENV"
 
@@ -341,7 +364,53 @@ jobs:
       # Submit the review under the arm this PR was assigned. The arm is not disclosed
       # in the comment: a reviewer who knows which prompt wrote a summary can't rate it
       # blind, and the whole point of the split is an unbiased read on the two prompts.
+      # Evals path: keep the verdict as an artifact rather than posting it. VERDICT is
+      # still exported so the Datadog reporting below records a dry run the same way it
+      # records a real one.
+      - name: Record verdict without submitting
+        if: ${{ inputs.dry_run }}
+        run: |
+          if [ ! -f verdict.json ]; then
+            echo "::error::Claude review run did not produce verdict.json" >&2
+            exit 1
+          fi
+          echo "VERDICT=$(jq -r '.verdict' verdict.json)" >> "$GITHUB_ENV"
+          echo "Dry run: verdict is $(jq -r '.verdict' verdict.json), not submitting"
+
+          # What the eval run needs to publish its result against this review: the prompt
+          # that produced it, and the span to hang the eval metric on. ml_app matches the
+          # trace submission below so the metric lands on the same application.
+          jq -n \
+            --arg pr "$PR" \
+            --arg ml_app "biggiepockets-review" \
+            --arg arm "${{ steps.resolve.outputs.assigned_arm }}" \
+            --arg prompt_name "${{ steps.resolve.outputs.arm_prompt_name }}" \
+            --arg prompt_version "${{ steps.resolve.outputs.arm_prompt_version }}" \
+            --arg registry_ref "${{ inputs.registry_ref }}" \
+            --arg registry_sha "${{ needs.codex.outputs.registry_sha }}" \
+            --arg trace_id "$TRACE_ID" \
+            --arg claude_span_id "$CLAUDE_SPAN_ID" \
+            '{pr: $pr, ml_app: $ml_app, arm: $arm, prompt_name: $prompt_name,
+              prompt_version: $prompt_version, registry_ref: $registry_ref,
+              registry_sha: $registry_sha, trace_id: $trace_id,
+              claude_span_id: $claude_span_id}' > review-meta.json
+
+      # Uploaded even on failure: a missing or malformed verdict is itself an eval
+      # result, and the scorer treats "no verdict" as a failed case rather than a pass.
+      - name: Upload verdict for eval scoring
+        if: ${{ always() && inputs.dry_run }}
+        uses: actions/upload-artifact@v4
+        with:
+          name: review-verdict-pr${{ inputs.pr }}
+          path: |
+            verdict.json
+            review-meta.json
+          if-no-files-found: warn
+          retention-days: 7
+          overwrite: true
+
       - name: Submit review as BiggiePockets
+        if: ${{ !inputs.dry_run }}
         id: submit
         env:
           # PAT authenticating as the BiggiePockets service account. A bot cannot
@@ -482,10 +551,10 @@ jobs:
 
           run_id="$GITHUB_REPOSITORY-pr$PR-${{ github.run_id }}"
 
-          trace_id=$(openssl rand -hex 16)
-          root_span_id=$(openssl rand -hex 8)
-          codex_span_id=$(openssl rand -hex 8)
-          claude_span_id=$(openssl rand -hex 8)
+          trace_id="$TRACE_ID"
+          root_span_id="$ROOT_SPAN_ID"
+          codex_span_id="$CODEX_SPAN_ID"
+          claude_span_id="$CLAUDE_SPAN_ID"
 
           spans=$(jq -n \
             --arg trace_id "$trace_id" \

evals/README.md

@@ -0,0 +1,55 @@
+# Review prompt evals
+
+A regression set for the BiggiePockets review prompts in `prompts/`. Each case is a real
+pull request whose correct review outcome is known, usually because the review that
+actually ran got it wrong and the defect was found later. Running the set answers one
+question: would today's prompts catch it?
+
+The runner lives in the repository whose pull requests the cases point at
+(`.github/workflows/eval-review-prompts.yml` in `BiggerPockets/biggerpockets`), because
+the reusable review workflow reviews the caller's own repo. It calls the same review
+workflow that posts real reviews, with `dry_run: true` so nothing is submitted, and
+scores each resulting `verdict.json` with `evals/score.sh`.
+
+Run it against a prompt branch before merging a prompt change:
+
+```
+gh workflow run eval-review-prompts.yml --repo BiggerPockets/biggerpockets \
+  -f registry_ref=<your prompt branch>
+```
+
+Every case costs one Codex pass plus one Claude pass, so the set is run on demand rather
+than on every prompt push.
+
+## Adding a case
+
+Write `evals/cases/<id>.json`:
+
+```json
+{
+  "id": "short-kebab-case-id",
+  "pr": 12345,
+  "why": "What the diff does, what is wrong with it, and what the real review did.",
+  "expect_verdict": "request_changes",
+  "must_match": [["pull-requests", "permission scope"], ["403", "permission"]],
+  "notes": "Anything a later reader needs to judge whether the assertions are fair."
+}
+```
+
+`must_match` rows are ANDed and the entries within a row are ORed, matched
+case-insensitively against the review summary. The point of a row is to separate "found
+this bug" from "found some other bug and requested changes anyway" — a case whose only
+assertion is the verdict proves very little on its own. Matching cannot judge whether the
+reasoning was sound, so keep rows broad enough to survive different phrasing and put
+anything a reader needs to re-judge the case in `notes`.
+
+A case needs a stable pull request to point at. A merged PR works: the runner diffs
+`refs/pull/<n>/head` against the merge base, so the diff stays what it was. Two things to
+watch:
+
+- Do not point a case at a PR whose discussion contains the answer. The prompts weigh PR
+  comments, so a thread that explains the defect turns the case into a reading exercise.
+  When the original PR has been annotated after the fact, open a replay PR at the original
+  head and close it, and point the case at the replay.
+- Keep at least one case whose expected verdict is `approve`. Without one, a prompt that
+  requests changes on everything scores perfectly.

evals/cases/review-app-sweeper-permission-fix.json

@@ -0,0 +1,8 @@
+{
+  "id": "review-app-sweeper-permission-fix",
+  "pr": 30502,
+  "why": "The fix for the case above: adds `pull-requests: read` and captures the HTTP status so a non-200 is surfaced as a warning annotation instead of being swallowed. A correct review approves this. It is the negative case that stops a prompt from scoring well by requesting changes on everything.",
+  "expect_verdict": "approve",
+  "must_match": [],
+  "notes": "Scored on verdict alone. Kept deliberately: an eval set of defect cases only rewards a reviewer that never approves."
+}

evals/cases/review-app-sweeper-permission-scope.json

@@ -0,0 +1,28 @@
+{
+  "id": "review-app-sweeper-permission-scope",
+  "pr": 30503,
+  "why": "Replay of BiggerPockets/biggerpockets#30384 at its original head. The workflow it adds declares only `issues: read`, but its /cwa check reads comments on a PR-backed issue, which also needs `pull-requests: read`. The request 403s, `curl -sf` swallows it into an empty string, and the fail-closed guard keeps every app, so the sweeper can never expire anything. The real review approved this and recorded the scope as correctly declared.",
+  "expect_verdict": "request_changes",
+  "must_match": [
+    [
+      "pull-requests",
+      "pull_requests",
+      "pull request read",
+      "permission scope",
+      "too narrow"
+    ],
+    [
+      "403",
+      "permission",
+      "scope"
+    ],
+    [
+      "expire",
+      "sweep",
+      "never",
+      "no-op",
+      "skip"
+    ]
+  ],
+  "notes": "The blocking finding must be the permission scope. A review that requests changes only over the missing pagination on the comments fetch, or only over the swallowed DELETE status, has not found this bug \u2014 the must_match rows are what separate those."
+}

evals/publish.sh

@@ -0,0 +1,130 @@
+#!/usr/bin/env bash
+# publish.sh — publish one scored eval case to Datadog.
+#
+# Usage: DD_API_KEY=... evals/publish.sh <case.json> <verdict.json> <review-meta.json> <0|1>
+#   the final argument is the score: 1 when the case passed, 0 when it failed.
+#
+# Publishes the result twice, on purpose, because the two answer different questions:
+#
+#   1. An LLM Obs evaluation metric joined to the review's Claude span. This puts the
+#      case result on the same trace as the review that produced it, so a case can be
+#      read next to the summary it scored and grouped by prompt_version alongside every
+#      other review the pipeline reports.
+#   2. A custom metric series, biggiepockets.review_eval.case_passed. This is what a
+#      dashboard or a monitor can query — evaluation metrics are not metrics and cannot
+#      be alerted on.
+#
+# Failures warn rather than fail the run: a publishing outage should not turn a passing
+# eval set red. Each request's HTTP status and body are printed, because a silently
+# dropped submission looks exactly like a submission nobody made.
+#
+# The span join needs span_id in DECIMAL (trace_id may be decimal or 32-char hex), while
+# the review workflow allocates both as hex. Converting is the whole reason this script
+# is not two inline curls.
+
+set -euo pipefail
+
+CASE_FILE="${1:?usage: publish.sh <case.json> <verdict.json> <review-meta.json> <0|1>}"
+VERDICT_FILE="${2:?missing verdict.json}"
+META_FILE="${3:?missing review-meta.json}"
+PASSED="${4:?missing score (0|1)}"
+
+DD_SITE="${DD_SITE:-datadoghq.com}"
+
+if [ -z "${DD_API_KEY:-}" ]; then
+  echo "Skipping Datadog publish: DD_API_KEY not set"
+  exit 0
+fi
+
+CASE_ID=$(jq -r '.id' "$CASE_FILE")
+EXPECTED=$(jq -r '.expect_verdict' "$CASE_FILE")
+ACTUAL=$(jq -r '.verdict // "none"' "$VERDICT_FILE")
+
+# A review that died before writing review-meta.json has no span to join to. The custom
+# metric still goes out — a case that could not run is a result worth graphing — but say
+# so rather than posting an eval metric with an invented join key.
+if [ -f "$META_FILE" ]; then
+  ML_APP=$(jq -r '.ml_app' "$META_FILE")
+  ARM=$(jq -r '.arm // "unknown"' "$META_FILE")
+  PROMPT_NAME=$(jq -r '.prompt_name // "unknown"' "$META_FILE")
+  PROMPT_VERSION=$(jq -r '.prompt_version // "unknown"' "$META_FILE")
+  REGISTRY_REF=$(jq -r '.registry_ref // "unknown"' "$META_FILE")
+  PR=$(jq -r '.pr' "$META_FILE")
+  TRACE_HEX=$(jq -r '.trace_id // empty' "$META_FILE")
+  SPAN_HEX=$(jq -r '.claude_span_id // empty' "$META_FILE")
+else
+  echo "::warning::$CASE_ID has no review-meta.json; publishing the metric without a span join"
+  ML_APP="biggiepockets-review"
+  ARM="unknown"; PROMPT_NAME="unknown"; PROMPT_VERSION="unknown"
+  REGISTRY_REF="${REGISTRY_REF:-unknown}"
+  PR=$(jq -r '.pr' "$CASE_FILE")
+  TRACE_HEX=""; SPAN_HEX=""
+fi
+
+TAGS=(
+  "case:$CASE_ID"
+  "pr:$PR"
+  "expected_verdict:$EXPECTED"
+  "actual_verdict:$ACTUAL"
+  "arm:$ARM"
+  "prompt_name:$PROMPT_NAME"
+  "prompt_version:$PROMPT_VERSION"
+  "registry_ref:$REGISTRY_REF"
+)
+tags_json=$(printf '%s\n' "${TAGS[@]}" | jq -R . | jq -s .)
+
+post() { # post <url> <payload> <what>
+  local url="$1" payload="$2" what="$3" code
+  code=$(curl -sS -o /tmp/dd-publish-body.txt -w '%{http_code}' \
+    -X POST "$url" \
+    -H "Content-Type: application/json" \
+    -H "DD-API-KEY: $DD_API_KEY" \
+    -d "$payload") || code="000"
+  if [ "$code" = "000" ] || [ "$code" -ge 300 ] 2>/dev/null; then
+    echo "::warning::$what failed for $CASE_ID (HTTP $code): $(cat /tmp/dd-publish-body.txt 2>/dev/null)"
+    return 0
+  fi
+  echo "$what accepted for $CASE_ID (HTTP $code)"
+}
+
+now_ms=$(( $(date +%s) * 1000 ))
+passed_bool=false
+[ "$PASSED" = "1" ] && passed_bool=true
+
+if [ -n "$TRACE_HEX" ] && [ -n "$SPAN_HEX" ]; then
+  # 64-bit span ids exceed what bash arithmetic can hold, so convert in python.
+  span_dec=$(python3 -c "import sys; print(int(sys.argv[1], 16))" "$SPAN_HEX")
+  eval_payload=$(jq -n \
+    --arg ml_app "$ML_APP" \
+    --arg trace_id "$TRACE_HEX" \
+    --arg span_id "$span_dec" \
+    --arg label_pass "review_eval.case_passed" \
+    --arg label_case "review_eval.case" \
+    --arg case_id "$CASE_ID" \
+    --argjson passed "$passed_bool" \
+    --argjson ts "$now_ms" \
+    --argjson tags "$tags_json" \
+    '{data: {type: "evaluation_metric", attributes: {metrics: [
+        {eval_scope: "span",
+         join_on: {span: {span_id: $span_id, trace_id: $trace_id}},
+         ml_app: $ml_app, timestamp_ms: $ts,
+         metric_type: "boolean", label: $label_pass,
+         boolean_value: $passed, tags: $tags},
+        {eval_scope: "span",
+         join_on: {span: {span_id: $span_id, trace_id: $trace_id}},
+         ml_app: $ml_app, timestamp_ms: $ts,
+         metric_type: "categorical", label: $label_case,
+         categorical_value: $case_id, tags: $tags}
+      ]}}}')
+  post "https://api.$DD_SITE/api/intake/llm-obs/v2/eval-metric" "$eval_payload" "LLM Obs eval metric"
+fi
+
+series_payload=$(jq -n \
+  --argjson value "$PASSED" \
+  --argjson ts "$(date +%s)" \
+  --argjson tags "$tags_json" \
+  '{series: [{metric: "biggiepockets.review_eval.case_passed",
+              type: 3,
+              points: [{timestamp: $ts, value: $value}],
+              tags: $tags}]}')
+post "https://api.$DD_SITE/api/v2/series" "$series_payload" "Custom metric"