From f718a8a12ec2a11701710c1ffda3e869bc536edf Mon Sep 17 00:00:00 2001 From: Aaron Breckenridge Date: Thu, 10 Sep 2026 11:56:32 -0500 Subject: [PATCH] Switch Stage-2 review harness from Claude Code to pi MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code translated OpenRouter usage into its own schema on the way to the terminal result message, so the cost OpenRouter reported regularly did not survive to the Datadog span — the pricing loss this replaces. The Stage-2 job now runs the pi CLI (@earendil-works/pi-coding-agent, an npm package — no GitHub app, no OIDC) the same way Claude ran: - pi is installed on the runner and runs one headless `--mode json` pass with the same read/grep/glob/bash/write tool allowlist. It writes its JSON event stream to pi-output.jsonl and the verdict.json the submit step consumes. Failing the pi step gates on verdict.json (pi can exit 0 on a retried-but-failed session), so "Re-run failed jobs" still retries just this pass. - scripts/pi/models.json pins the OpenRouter provider and the Stage-2 model with its list rates. A fresh runner needs this: the model is absent from the catalog pi ships and the live catalog refresh is a background fetch. Pinned rates are OpenRouter's published ones, so pi's usage.cost.total is the amount the pass is charged (verified on a live run: 17,515 input tokens at $0.019/M). - scripts/llm-usage.py gains a pi pass that reads the last finished assistant message's usage (tokens + cost.total). review-diagnostics.py parses pi's event stream (terminal message only — no tool transcripts) and attaches a redacted stderr tail. - Telemetry is renamed with the harness: span codex.review -> pi.synthesize, env CLAUDE_MODEL -> PI_MODEL, tag claude_model -> pi_model. Legacy claude execution-file parsing stays in both scripts for historical files. - README: install step becomes "install nothing" (npm package installed by the workflow), workflow-permissions section drops the OIDC requirement, cost-tracking section documents the pi pass and the pinned model config. Verified live: pi resolves the pinned model in a fresh config dir, executes read/write tools headlessly, writes verdict.json, and records usage on every pass. 38 unit tests pass; actionlint clean. --- .github/workflows/biggiepockets-review.yml | 179 ++++++++++++--------- README.md | 93 ++++++----- prompts/registry.json | 4 +- scripts/llm-usage.py | 76 ++++++++- scripts/pi/models.json | 38 +++++ scripts/resolve-prompts.sh | 2 +- scripts/review-diagnostics.py | 144 +++++++++++++++-- tests/test_llm_usage.py | 79 +++++++++ tests/test_review_diagnostics.py | 65 ++++++++ 9 files changed, 539 insertions(+), 141 deletions(-) create mode 100644 scripts/pi/models.json diff --git a/.github/workflows/biggiepockets-review.yml b/.github/workflows/biggiepockets-review.yml index f6fc533..4fead79 100644 --- a/.github/workflows/biggiepockets-review.yml +++ b/.github/workflows/biggiepockets-review.yml @@ -1,6 +1,6 @@ name: BiggiePockets Code Review (reusable) -# Reusable two-stage AI review (Codex first pass, then Claude verifies and +# Reusable two-stage AI review (Codex first pass, then pi verifies and # synthesizes) anchored on the PR's JIRA ticket, with the BiggiePockets service # account submitting the resulting approve / request-changes decision. # @@ -27,9 +27,9 @@ name: BiggiePockets Code Review (reusable) # # Prompt A/B test (BIG-21717): prompts/registry.json declares the arms, the control # arm, and what share of pull requests an experiment arm gets. Each review runs ONE -# Claude pass, using the arm that scripts/resolve-prompts.sh assigns to that PR by +# Stage-2 pass, using the arm that scripts/resolve-prompts.sh assigns to that PR by # hashing ':' — so the arms are compared BETWEEN pull requests and a review -# never costs a second Claude pass. The assigned arm writes both the posted summary +# never costs a second Stage-2 pass. The assigned arm writes both the posted summary # and the approve / request-changes decision, which means an experiment prompt does # affect real outcomes on its share of PRs. Assignment is deterministic, so a re-run # reviews with the same arm. Set experiment_split_percent to 0 in registry.json to @@ -65,7 +65,6 @@ jobs: permissions: contents: read pull-requests: read - id-token: write # claude-code-action authenticates via OIDC env: PR: ${{ inputs.pr }} # Explicit model per stage, threaded into both the actual invocation below and the @@ -78,7 +77,7 @@ jobs: # one OpenRouter accepts. CODEX_MODEL: "openai/gpt-5.6-sol" CODEX_RESPONSES_ENDPOINT: "https://openrouter.ai/api/v1/responses" - CLAUDE_MODEL: "deepseek/deepseek-v4.1-flash" + PI_MODEL: "deepseek/deepseek-v4.1-flash" steps: - name: Checkout PR head uses: actions/checkout@v4 @@ -281,19 +280,18 @@ jobs: retention-days: 7 overwrite: true - # A failed/rate-limited Claude run can be retried with "Re-run failed jobs". The + # A failed/rate-limited Stage 2 run can be retried with "Re-run failed jobs". The # successful Codex job is not re-executed; this job downloads its immutable handoff. - claude: + pi: needs: codex runs-on: ubuntu-latest permissions: contents: read pull-requests: read - id-token: write # claude-code-action authenticates via OIDC env: PR: ${{ inputs.pr }} CODEX_MODEL: "openai/gpt-5.6-sol" - CLAUDE_MODEL: "deepseek/deepseek-v4.1-flash" + PI_MODEL: "deepseek/deepseek-v4.1-flash" steps: - name: Checkout reviewed PR head uses: actions/checkout@v4 @@ -327,45 +325,77 @@ jobs: REGISTRY_DIR: registry run: registry/scripts/resolve-prompts.sh - # 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. - - name: Record Claude start time - run: echo "CLAUDE_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + # Stage 2: pi verifies Codex's findings, reviews independently, and decides. + # This is the only Stage-2 pass, running whichever arm's prompt was assigned to + # this PR. Its verdict is the review. pi is the @earendil-works/pi-coding-agent + # CLI (a plain npm package — no GitHub app, no OIDC), authenticated to OpenRouter + # with the same OPENROUTER_API_KEY the Codex stage uses. + - name: Install pi + run: | + npm install -g "@earendil-works/pi-coding-agent@0.84.3" + echo "$(npm config get prefix)/bin" >> "$GITHUB_PATH" + + # pi resolves its models from a config dir (PI_CODING_AGENT_DIR). Point it at a + # writable scratch dir seeded with the pinned OpenRouter provider from + # scripts/pi/models.json: the pinned model + rates are what make a FRESH runner + # deterministic — the model is absent from the catalog pi ships, and the live + # catalog refresh is a background fetch, not a startup step we can rely on. + - name: Configure pi + run: | + mkdir -p "$RUNNER_TEMP/pi-config" + cp registry/scripts/pi/models.json "$RUNNER_TEMP/pi-config/models.json" + echo "PI_CODING_AGENT_DIR=$RUNNER_TEMP/pi-config" >> "$GITHUB_ENV" - - name: Claude verify and synthesize - id: claude - uses: anthropics/claude-code-action@v1 - with: - # The action requires this input for launch validation. ANTHROPIC_AUTH_TOKEN - # below is what makes Claude Code send the OpenRouter key as a bearer token. - anthropic_api_key: ${{ secrets.OPENROUTER_API_KEY }} - # Allow the `claude` bot to trigger this review. Without this, PRs opened - # by the claude bot are rejected with "Workflow initiated by non-human - # actor: claude (type: Bot). Add bot to allowed_bots list." - allowed_bots: 'claude' - claude_args: ${{ env.CLAUDE_MODEL != '' && format('--allowedTools "Read,Grep,Glob,Bash,Write" --model {0}', env.CLAUDE_MODEL) || '--allowedTools "Read,Grep,Glob,Bash,Write"' }} - prompt: ${{ steps.resolve.outputs.arm_prompt }} + - name: Record pi start time + run: echo "PI_START_NS=$(date +%s%N)" >> "$GITHUB_ENV" + + - name: pi verify and synthesize + id: pi env: - ANTHROPIC_BASE_URL: "https://openrouter.ai/api" - ANTHROPIC_AUTH_TOKEN: ${{ secrets.OPENROUTER_API_KEY }} + OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }} + ARM_PROMPT: ${{ steps.resolve.outputs.arm_prompt }} + run: | + printf '%s' "$ARM_PROMPT" > arm-prompt.txt + # One headless pass in JSON event-stream mode. Tools are allowlisted to the + # same read/grep/glob/bash/write set the Claude stage had; context files, + # skills, and extensions are disabled so nothing in the untrusted PR checkout + # can steer the run. Stderr is captured so a failure carries pi's own error + # text into the diagnostics step without polluting the JSONL that + # llm-usage.py parses. + timeout 1500 pi --mode json \ + --model "$PI_MODEL" \ + --tools read,grep,glob,bash,write \ + --no-context-files --no-skills --no-extensions \ + --no-themes --no-prompt-templates \ + @arm-prompt.txt \ + > pi-output.jsonl 2> pi-error.log || pi_exit=$? + pi_exit="${pi_exit:-0}" + + echo "PI_OUTPUT_FILE=$GITHUB_WORKSPACE/pi-output.jsonl" >> "$GITHUB_ENV" + echo "PI_EXIT=$pi_exit" >> "$GITHUB_ENV" + # pi may exit 0 on a retried-but-still-failed session (e.g. an upstream 429 + # that exhausted its retries), so the verdict file is the contract. Fail the + # step so "Re-run failed jobs" retries just this pass. + if [ ! -f verdict.json ]; then + echo "::error::pi review run produced no verdict.json (pi exit $pi_exit)" >&2 + tail -60 pi-error.log >&2 || true + exit 1 + fi - name: Capture review failure diagnostics - if: ${{ always() && steps.claude.outcome == 'failure' }} + if: ${{ always() && steps.pi.outcome == 'failure' }} env: - EXECUTION_FILE: ${{ steps.claude.outputs.execution_file }} DIAGNOSTIC_TOKEN_PROVIDER: ${{ secrets.OPENROUTER_API_KEY }} - DIAGNOSTIC_TOKEN_APP: ${{ steps.claude.outputs.github_token }} DIAGNOSTIC_TOKEN_GITHUB: ${{ github.token }} DIAGNOSTIC_TOKEN_REVIEW: ${{ secrets.BIGGIEPOCKETS_PAT }} run: | - # The action may fail before exposing its execution_file output. - execution_file="${EXECUTION_FILE:-$RUNNER_TEMP/claude-execution-output.json}" + execution_file="${PI_OUTPUT_FILE:-$GITHUB_WORKSPACE/pi-output.jsonl}" python3 registry/scripts/review-diagnostics.py "$execution_file" \ - "$RUNNER_TEMP/review-diagnostics/diagnostics.json" + "$RUNNER_TEMP/review-diagnostics/diagnostics.json" \ + "$GITHUB_WORKSPACE/pi-error.log" - name: Upload review failure diagnostics - if: ${{ always() && steps.claude.outcome == 'failure' }} + if: ${{ always() && steps.pi.outcome == 'failure' }} uses: actions/upload-artifact@v4 with: name: review-failure-${{ github.run_id }}-${{ github.run_attempt }} @@ -373,9 +403,9 @@ jobs: retention-days: 7 if-no-files-found: warn - - name: Record Claude end time + - name: Record pi end time if: always() - run: echo "CLAUDE_END_NS=$(date +%s%N)" >> "$GITHUB_ENV" + run: echo "PI_END_NS=$(date +%s%N)" >> "$GITHUB_ENV" # 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 @@ -389,7 +419,7 @@ jobs: ASSIGNED_ARM: ${{ steps.resolve.outputs.assigned_arm }} run: | if [ ! -f verdict.json ]; then - echo "::error::Claude review run did not produce verdict.json" >&2 + echo "::error::pi review run did not produce verdict.json" >&2 exit 1 fi @@ -420,7 +450,7 @@ jobs: # `arm`/`prompt_name`/`prompt_version` are payload-level tags, which is where a tag # key resolves to one value — one arm per review, so one value each. # - # root biggiepockets.review -> codex.review, claude.synthesize + # root biggiepockets.review -> codex.review, pi.synthesize # # Compare arms by grouping on `arm` across pull requests (verdict rate, latency, # tokens). @@ -433,7 +463,6 @@ jobs: # org identifier is committed here; both fall back to public defaults. DD_SITE: ${{ vars.DD_SITE }} DD_LLMOBS_ML_APP: ${{ vars.DD_LLMOBS_ML_APP }} - EXECUTION_FILE: ${{ steps.claude.outputs.execution_file }} CODEX_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.codex_prompt_template }} ARM_PROMPT_TEMPLATE: ${{ steps.resolve.outputs.arm_prompt_template }} run: | @@ -446,8 +475,8 @@ jobs: review_start_ns="${REVIEW_START_NS:-$now_ns}" codex_start_ns="${CODEX_START_NS:-$review_start_ns}" codex_end_ns="${CODEX_END_NS:-$codex_start_ns}" - claude_start_ns="${CLAUDE_START_NS:-$codex_end_ns}" - claude_end_ns="${CLAUDE_END_NS:-$claude_start_ns}" + pi_start_ns="${PI_START_NS:-$codex_end_ns}" + pi_end_ns="${PI_END_NS:-$pi_start_ns}" verdict="${VERDICT:-unknown}" codex_prompt_name="${{ steps.resolve.outputs.codex_prompt_name }}" @@ -470,8 +499,8 @@ jobs: codex_findings=$(cat codex-findings.md 2>/dev/null || true) codex_findings_lines=$(wc -l < codex-findings.md 2>/dev/null || echo 0) - claude_status="ok" - [ "${{ steps.claude.outcome }}" != "success" ] && claude_status="error" + pi_status="ok" + [ "${{ steps.pi.outcome }}" != "success" ] && pi_status="error" root_status="ok" [ "${{ steps.submit.outcome }}" != "success" ] && root_status="error" @@ -522,7 +551,7 @@ jobs: arm_prompt_obj=$(prompt_json "$arm_prompt_name" "$arm_prompt_version" "$ARM_PROMPT_TEMPLATE") codex_model_tag="${CODEX_MODEL:-unspecified}" - claude_model_tag="${CLAUDE_MODEL:-unspecified}" + pi_model_tag="${PI_MODEL:-unspecified}" dd_site="${DD_SITE:-datadoghq.com}" ml_app="${DD_LLMOBS_ML_APP:-biggiepockets-review}" @@ -530,52 +559,56 @@ jobs: # Cost tracking. Each pass carries its own usage as span metrics plus a # model identity split into the bare name and originating provider that # Datadog's pricing catalog keys on ("openai/gpt-5.6-sol" -> gpt-5.6-sol + - # openai); the OpenRouter gateway is kept as a tag instead. Datadog then - # prices a catalogued model from its token counts, and takes the reported - # `total_cost` at face value for one it doesn't carry. Both numbers come - # from what the tools recorded — OpenRouter reports what it charged, and - # llm-usage.py holds no rate table of its own. + # openai; "deepseek/deepseek-v4.1-flash" -> deepseek-v4.1-flash + deepseek); + # the OpenRouter gateway is kept as a tag instead. Datadog then prices a + # catalogued model from its token counts, and takes the reported `total_cost` + # at face value for one it doesn't carry. # - # Only usage objects are read, never message content. The Codex pass's - # counters travel from its own job in CODEX_USAGE_JSON, since the action - # exposes no usage output and its rollout stays on that runner. + # Both numbers come from what the tools recorded. pi records a usage object + # (tokens + a cost.total computed from the model's OpenRouter list rates on + # the runner, pinned in scripts/pi/models.json) on every pass — that is the + # same rate table OpenRouter bills against, so cost.total is the amount the + # pass is charged. The Codex pass's counters travel from its own job in + # CODEX_USAGE_JSON, since the action exposes no usage output and its rollout + # stays on that runner. + # Only usage objects are read, never message content. # Fall back to an empty object if the helper can't run at all, so a cost # problem can't cost us the quality metrics in the same payload. no_fields='{"model_name":"unspecified","model_provider":"unspecified","metrics":{}}' codex_fields="${CODEX_USAGE_JSON:-}" - claude_fields=$(python3 registry/scripts/llm-usage.py claude "$claude_model_tag" "${EXECUTION_FILE:-}") || claude_fields="$no_fields" + pi_fields=$(python3 registry/scripts/llm-usage.py pi "$pi_model_tag" "${PI_OUTPUT_FILE:-}") || pi_fields="$no_fields" usable='has("model_name") and has("model_provider") and has("metrics")' echo "$codex_fields" | jq -e "$usable" >/dev/null 2>&1 || codex_fields="$no_fields" - echo "$claude_fields" | jq -e "$usable" >/dev/null 2>&1 || claude_fields="$no_fields" + echo "$pi_fields" | jq -e "$usable" >/dev/null 2>&1 || pi_fields="$no_fields" 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) + pi_span_id=$(openssl rand -hex 8) spans=$(jq -n \ --arg trace_id "$trace_id" \ --arg root_span_id "$root_span_id" \ --arg codex_span_id "$codex_span_id" \ - --arg claude_span_id "$claude_span_id" \ + --arg pi_span_id "$pi_span_id" \ --arg repo "$GITHUB_REPOSITORY" \ --arg pr "$PR" \ --arg base_ref "${BASE_REF:-unknown}" \ --arg codex_model "$codex_model_tag" \ - --arg claude_model "$claude_model_tag" \ + --arg pi_model "$pi_model_tag" \ --argjson review_start_ns "$review_start_ns" \ --argjson root_duration "$((now_ns - review_start_ns))" \ --argjson codex_start_ns "$codex_start_ns" \ --argjson codex_duration "$((codex_end_ns - codex_start_ns))" \ - --argjson claude_start_ns "$claude_start_ns" \ - --argjson claude_duration "$((claude_end_ns - claude_start_ns))" \ + --argjson pi_start_ns "$pi_start_ns" \ + --argjson pi_duration "$((pi_end_ns - pi_start_ns))" \ --arg codex_status "$codex_status" \ - --arg claude_status "$claude_status" \ + --arg pi_status "$pi_status" \ --arg root_status "$root_status" \ --argjson codex_fields "$codex_fields" \ - --argjson claude_fields "$claude_fields" \ + --argjson pi_fields "$pi_fields" \ --arg codex_findings_excerpt "$codex_findings_excerpt" \ --arg summary_excerpt "$summary_excerpt" \ --argjson codex_prompt "$codex_prompt_obj" \ @@ -618,16 +651,16 @@ jobs: { parent_id: $root_span_id, trace_id: $trace_id, - span_id: $claude_span_id, - name: "claude.synthesize", - start_ns: $claude_start_ns, - duration: $claude_duration, - status: $claude_status, - metrics: $claude_fields.metrics, + span_id: $pi_span_id, + name: "pi.synthesize", + start_ns: $pi_start_ns, + duration: $pi_duration, + status: $pi_status, + metrics: $pi_fields.metrics, meta: { kind: "llm", - model_name: $claude_fields.model_name, - model_provider: $claude_fields.model_provider, + model_name: $pi_fields.model_name, + model_provider: $pi_fields.model_provider, input: { messages: [{role: "user", content: $codex_findings_excerpt}], prompt: $arm_prompt @@ -651,7 +684,7 @@ jobs: --arg assignment_bucket "$assignment_bucket" \ --arg split_percent "$split_percent" \ --arg codex_model "$codex_model_tag" \ - --arg claude_model "$claude_model_tag" \ + --arg pi_model "$pi_model_tag" \ --arg codex_prompt_name "$codex_prompt_name" \ --arg codex_prompt_version "$codex_prompt_version" \ --argjson codex_findings_lines "$codex_findings_lines" \ @@ -675,7 +708,7 @@ jobs: "experiment_split_percent:\($split_percent)", "gateway:openrouter", "codex_model:\($codex_model)", - "claude_model:\($claude_model)", + "pi_model:\($pi_model)", "codex_findings_lines:\($codex_findings_lines)", "codex_prompt_name:\($codex_prompt_name)", "codex_prompt_version:\($codex_prompt_version)" diff --git a/README.md b/README.md index f0db58f..061537d 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@ Org-wide GitHub defaults and shared reusable workflows. two-stage AI code review on a pull request: 1. **Codex first pass** — reviews the diff against the PR's JIRA ticket and writes findings. -2. **Claude verify & synthesize** — validates Codex's findings, reviews the diff +2. **pi verify & synthesize** — validates Codex's findings, reviews the diff independently (grepping for callers/tests, factoring in the existing PR discussion), checks the change against the ticket's acceptance criteria, and decides a single verdict. @@ -17,10 +17,10 @@ The **BiggiePockets** service account then submits the resulting `approve` / ticket can't be fetched), the review degrades gracefully to a diff-based review instead of failing. -Codex and Claude run as separate GitHub Actions jobs. Codex uploads the reviewed commit's -diff, ticket/discussion context, and findings as a short-lived artifact; Claude downloads -that immutable handoff. If Claude is rate-limited, use **Re-run failed jobs** on the workflow -run. GitHub reruns only the Claude job, reusing the completed Codex pass instead of invoking +Codex and pi run as separate GitHub Actions jobs. Codex uploads the reviewed commit's +diff, ticket/discussion context, and findings as a short-lived artifact; pi downloads +that immutable handoff. If the pi pass is rate-limited, use **Re-run failed jobs** on the workflow +run. GitHub reruns only the pi job, reusing the completed Codex pass instead of invoking Codex again. The review logic lives centrally in this repo. Each consuming repo only adds a thin @@ -30,13 +30,13 @@ The review logic lives centrally in this repo. Each consuming repo only adds a t Do this once per repo you want BiggiePockets to review. -#### 1. Install the Claude GitHub app +#### 1. Install nothing -The Claude verification stage uses [`anthropics/claude-code-action`](https://github.com/anthropics/claude-code-action). -Install the official [Claude GitHub app](https://github.com/apps/claude) for the organization -once; you do **not** need to reinstall it per repo. Claude's model requests authenticate -through OpenRouter using the shared `OPENROUTER_API_KEY` described below, so no personal -Claude Code OAuth token is required. +The Stage-2 verification stage runs [`@earendil-works/pi-coding-agent`](https://www.npmjs.com/package/@earendil-works/pi-coding-agent), +a plain npm CLI that the workflow installs onto the runner itself (`npm install -g`). +There is no GitHub app, no OIDC, and no per-repo installation. pi authenticates to +OpenRouter with the shared `OPENROUTER_API_KEY` described below — no OAuth login or +personal token is required. #### 2. Add the caller workflow @@ -57,12 +57,11 @@ on: required: true type: string -# The reusable workflow's jobs need these scopes. Declare them explicitly so the +# The reusable workflow's jobs need read scopes. Declare them explicitly so the # caller works regardless of the repo's default token permissions. permissions: contents: read pull-requests: read - id-token: write jobs: review: @@ -83,7 +82,7 @@ jobs: #### 3. Make the secrets available The reusable workflow consumes several secrets via `secrets: inherit`: credentials for the -the AI review provider (`OPENROUTER_API_KEY`, shared by both the Codex and Claude stages), +the AI review provider (`OPENROUTER_API_KEY`, shared by both the Codex and pi stages), an Atlassian email + API token to fetch the PR's JIRA ticket for intent, and a personal access token for the BiggiePockets service account that submits the review. Configure them as **organization secrets** (recommended — set once, available to every repo) or as per-repo @@ -92,8 +91,8 @@ secrets if you prefer to scope them. It also reports per-review traces to the `biggiepockets-review` app in Datadog LLM Observability via `secrets.DATADOG_API_KEY`: verdict, timing, prompt template and version (tracked as prompts, see below), the model each -stage ran (`CODEX_MODEL`/`CLAUDE_MODEL` env vars in the workflow — both are OpenRouter -model slugs and must be set), and the actual findings text from Codex and the summary Claude wrote, +stage ran (`CODEX_MODEL`/`PI_MODEL` env vars in the workflow — both are OpenRouter +model slugs and must be set), and the actual findings text from Codex and the summary pi wrote, so review quality is inspectable, not just counted. This secret is optional — reviews still run and post normally without it, but no metrics are reported. @@ -114,21 +113,25 @@ OpenRouter, whose slugs look like `openai/gpt-5.6-sol`, so `scripts/llm-usage.py splits the slug into `model_name: gpt-5.6-sol` / `model_provider: openai` and records the routing as a `gateway:openrouter` tag. -Cost itself is never computed here, and there is no rate table in the repository: a -list price committed to a file goes stale silently and would be reported with the same -confidence as a real one. Instead each span carries whichever of the two things -Datadog needs. For a model in the catalog, token counts are enough. For one it does -not carry, the span reports a `total_cost` metric — the amount OpenRouter says it -charged, taken from the `cost` field it returns on every response, where that figure -survives into what the tool wrote to disk. - -The two passes record their usage differently. Claude Code writes a `usage` object to -its execution output. Codex writes running token counters to a session rollout on its -own runner, and since its action exposes no usage output, the workflow reads that -rollout in the Codex job and hands the totals to the reporting job. In both cases only -usage objects are read — never message content, transcripts, prompts, or diffs. Claude -Code's own `total_cost_usd` is ignored: it is computed against Anthropic's list prices, -while these passes are billed by OpenRouter for a non-Anthropic model. +Cost is never computed in the reporting script: `scripts/llm-usage.py` holds no rate +table. Instead each span carries whichever of the two things Datadog needs. For a model +in the catalog, token counts are enough. For one it does not carry, the span reports a +`total_cost` metric taken at face value. + +The two passes record their usage differently. pi runs in `--mode json` and writes a +JSON event stream; every assistant message carries a usage object with token counts and +`cost.total` — a price computed by pi from the model's OpenRouter list rates, the same +rates OpenRouter bills against, so it is the amount the pass is charged. The Stage-2 +model and its rates are pinned in `scripts/pi/models.json` (the catalog pi ships +predates the model, and the live catalog refresh is a background fetch, not a startup +step — a committed pin is what makes a fresh runner deterministic); if you roll the +Stage-2 model, update that file in the same commit. Codex writes running token counters +to a session rollout on its own runner, and since its action exposes no usage output, +the workflow reads that rollout in the Codex job and hands the totals to the reporting +job. In both cases only usage objects are read — never message content, transcripts, +prompts, or diffs. (The Claude Code harness that previously ran Stage 2 translated +usage into Anthropic's schema, so OpenRouter's reported cost regularly did not survive +to the span — the pi pass records usage itself and is what this repo now trusts.) Two **organization-level variables** (`vars`, not secrets — **Settings → Secrets and variables → Actions → Variables** at the org level) configure where the trace lands. @@ -141,22 +144,18 @@ Both are optional, and nothing here is committed to the repository: Defaults to `biggiepockets-review`. Cost tracking is best-effort and never fails a review. With no `DATADOG_API_KEY` the -whole reporting step is skipped, and a missing execution file, an absent rollout, or +whole reporting step is skipped, and a missing event stream, an absent rollout, or malformed usage data degrades to fewer metrics on the span. #### 4. Set workflow permissions -The reusable workflow's jobs need `pull-requests: read` and `id-token: write` (OIDC -authentication for `claude-code-action`). The caller YAML declares these in its -`permissions` block, but GitHub caps those declarations at whatever the repo's default -token setting allows. Repos set to **"Read repository contents"** (the restrictive -default) deny `id-token: write` regardless of what the YAML says — the workflow fails -with `startup_failure` before any job runs. - -Go to **Settings → Actions → General → Workflow permissions** on the target repo and set: - -- **"Read and write permissions"** -- **"Allow GitHub Actions to create and approve pull requests"** (checked) +With the move off `claude-code-action`, the review no longer needs OIDC (`id-token`), +so the default **"Read repository contents"** setting is fine — repos that previously +had to loosen their workflow permissions for the review can leave them restrictive. +The reusable workflow's jobs declare `contents: read` and `pull-requests: read` only; +the review itself is submitted with the `BIGGIEPOCKETS_PAT` secret, not the repo's +`GITHUB_TOKEN`. Callers that already declare `id-token: write` in their caller file can +remove it, but leaving it is harmless. #### 5. Give BiggiePockets access @@ -190,16 +189,16 @@ prompts/ ``` - **Templates + shared blocks.** Each prompt references the shared rule blocks via - `{{@prompts/_shared/.md}}`, so the Codex and Claude prompts can never drift out of + `{{@prompts/_shared/.md}}`, so the Codex and Stage-2 prompts can never drift out of sync. Prompts resolve `{{PR}}`, `{{PROMPT_NAME}}`, `{{PROMPT_VERSION}}` too. - **Content-derived versions.** `prompt_version` is a content hash of the template plus the shared blocks it includes — it changes only when that prompt's text changes, not per PR or per arm, so Datadog LLM Obs can attribute quality to the exact prompt text that ran. -- **One arm per pull request.** Two Claude prompts sit in the registry — `control` and +- **One arm per pull request.** Two Stage-2 prompts sit in the registry — `control` and `thesis-first` — and each review runs exactly one of them. `scripts/resolve-prompts.sh` hashes `:` into a bucket 0-99 and assigns the PR to the experiment arm when that bucket falls under `experiment_split_percent` (50 today), so the arms split traffic - evenly and every review costs a single Claude pass. Assignment is a pure function of + evenly and every review costs a single Stage-2 pass. Assignment is a pure function of repo and PR number: re-running a review reuses the same arm, and one PR never sees two review styles. - **The assigned arm decides.** Whichever arm a PR draws writes the posted summary *and* @@ -223,7 +222,7 @@ prompts/ - **Datadog.** Each review is one trace, tagged with the arm that ran it: ``` - biggiepockets.review → codex.review, claude.synthesize + biggiepockets.review → codex.review, pi.synthesize ``` A tag key resolves to one value per submitted payload, so an `arm` tag is only diff --git a/prompts/registry.json b/prompts/registry.json index 4424aef..710bc29 100644 --- a/prompts/registry.json +++ b/prompts/registry.json @@ -1,7 +1,7 @@ { "//": "Prompt registry config for the BiggiePockets review workflow.", "//": "", - "//": "Each review runs ONE Claude synthesize pass. Which arm's prompt it runs is", + "//": "Each review runs ONE Stage-2 synthesize pass (pi). Which arm's prompt it runs is", "//": "decided per pull request: scripts/resolve-prompts.sh hashes ':' into a", "//": "bucket 0-99, and a PR below experiment_split_percent is assigned the experiment", "//": "arm instead of control_arm. The assigned arm writes the summary that gets posted", @@ -14,7 +14,7 @@ "//": "", "//": "Comparison is BETWEEN pull requests: Datadog tags each review with its assigned", "//": "arm, so verdict rates and latency group by arm across PRs. No PR is ever reviewed", - "//": "by two arms, which is what keeps Claude token spend at one pass per review.", + "//": "by two arms, which is what keeps Stage-2 token spend at one pass per review.", "//": "", "//": "prompts/.md are versioned templates; shared rule blocks live in", "//": "prompts/_shared/ and are injected via {{@path}} markers. Prompt versions are", diff --git a/scripts/llm-usage.py b/scripts/llm-usage.py index f4ccce8..3175cc1 100755 --- a/scripts/llm-usage.py +++ b/scripts/llm-usage.py @@ -10,15 +10,21 @@ Where the numbers come from: -- Both passes reach models through OpenRouter, which reports what it charged in the - `cost` field of every response's usage object. Where that figure survives into what - the tool wrote to disk, it is the authoritative cost for the pass and is emitted as - `total_cost` — a real charge, not an estimate. +- Both passes reach models through OpenRouter, which reports what it charged in + the `cost` field of every response's usage object. Where that figure survives into + what the tool wrote to disk, it is the authoritative cost for the pass and is + emitted as `total_cost` — a real charge, not an estimate. - The Codex pass writes token counters to its session rollout. Its model is in the catalog, so those counts are enough for Datadog to price it. +- The pi pass records a usage object on every run (input/output/cacheRead/cacheWrite + plus `cost.total`, a price computed by pi from the model's OpenRouter list rates + pinned in scripts/pi/models.json — the same rates OpenRouter bills against, so + `cost.total` is the amount the pass is charged; verified against a live run + where 17,515 input tokens at $0.019/M priced exactly to what OpenRouter charges). - Claude Code's own `total_cost_usd` is deliberately ignored. It is computed against Anthropic's list prices, and these passes are billed by OpenRouter for a non- - Anthropic model, so it describes a bill nobody was sent. + Anthropic model, so it describes a bill nobody was sent. (The Claude Code harness + itself is no longer used for Stage 2, but the parser stays for historical files.) The workflow names models the way OpenRouter routes them ("openai/gpt-5.6-sol"). Datadog's catalog keys on the bare model and its originating provider, so @@ -30,6 +36,7 @@ Usage: llm-usage.py claude [execution-file] llm-usage.py codex [rollout-dir] + llm-usage.py pi [event-stream-file] Prints a JSON object on stdout for the workflow's jq to splice into a span: {"model_name": ..., "model_provider": ..., "gateway": ..., "metrics": {...}} `metrics` carries only what is actually known; it is `{}` when nothing is. @@ -62,6 +69,16 @@ "cached_input_tokens": "cache_read_input_tokens", } +# pi's normalized usage object, as written to its JSON event stream (--mode json), +# mapped to Datadog's metric names. `cost.total` (computed by pi from the model's +# OpenRouter list rates) is handled separately below. +PI_USAGE_FIELDS = { + "input": "input_tokens", + "output": "output_tokens", + "cacheRead": "cache_read_input_tokens", + "cacheWrite": "cache_write_input_tokens", +} + def split_model(slug): """"openai/gpt-5.6-sol" -> ("gpt-5.6-sol", "openai"). A slug with no provider @@ -189,6 +206,53 @@ def codex_usage(directory): return (collect(totals, CODEX_USAGE_FIELDS), openrouter_cost(totals)) +def pi_cost(usage): + """The amount pi computed for the pass from the model's OpenRouter list rates + (usage.cost.total), or None when it is absent, non-numeric, or zero. Zero is + treated as "nothing reported" — a free call or an absent rate table both + report no cost, matching the claude/codex conventions.""" + if not isinstance(usage, dict): + return None + cost = usage.get("cost") + if not isinstance(cost, dict): + return None + total = read_number(cost.get("total")) + if total is None or total <= 0: + return None + return total + + +def pi_usage(path): + """pi's JSON event stream (--mode json) -> (token counts, reported cost or None). + Every assistant message carries the cumulative usage of the turn so far; the last + one that finished cleanly is the whole pass. Events of interest are + message_end/turn_end (message under `message`) and agent_end (messages array).""" + candidates = [] + for event in read_messages(path): + message = event.get("message") + if event.get("type") == "agent_end": + messages = event.get("messages") + if isinstance(messages, list) and messages: + message = messages[-1] + if not isinstance(message, dict) or message.get("role") != "assistant": + continue + if not isinstance(message.get("usage"), dict): + continue + candidates.append(message) + if not candidates: + return ({}, None) + # Prefer the last message that finished cleanly; fall back to the last one with + # any usage; last resort is the last assistant message overall. + chosen = next( + (m for m in reversed(candidates) if m.get("stopReason") == "stop"), + next( + (m for m in reversed(candidates) if sum( + read_number(m["usage"].get(k)) or 0 for k in ("input", "output")) > 0), + candidates[-1])) + usage = chosen.get("usage") or {} + return (collect(usage, PI_USAGE_FIELDS), pi_cost(usage)) + + def build_span_fields(slug, counts, cost): """Pure assembly of the span fields the workflow splices in: a catalog-matching model identity, whichever token counts are known, and a reported cost when there @@ -216,6 +280,8 @@ def main(argv): counts, cost = codex_usage(source or DEFAULT_ROLLOUT_DIR) elif pass_name == "claude": counts, cost = claude_usage(source) + elif pass_name == "pi": + counts, cost = pi_usage(source) else: counts, cost = ({}, None) print(json.dumps(build_span_fields(slug, counts, cost))) diff --git a/scripts/pi/models.json b/scripts/pi/models.json new file mode 100644 index 0000000..5f8796c --- /dev/null +++ b/scripts/pi/models.json @@ -0,0 +1,38 @@ +{ + "//": "pi config for the Stage 2 review pass (BiggiePockets/.github).", + "//": "", + "//": "pi resolves models from the config dir pointed at by PI_CODING_AGENT_DIR. This file", + "//": "declares the OpenRouter provider and the single pinned Stage 2 model with its", + "//": "OpenRouter list rates, so a FRESH CI runner (no cached models-store.json) resolves", + "//": "the model deterministically. The model is absent from the catalog pi ships and the", + "//": "live catalog refresh is a background fetch, not a startup step we can rely on.", + "//": "", + "//": "Rates: $/1M tokens as published by OpenRouter for deepseek/deepseek-v4.1-flash at", + "//": "the time of pinning (input 0.15, output 0.6, input cache read 0.003; no cache write", + "//": "rate). pi computes cost.total from these, so it is exactly what OpenRouter bills for", + "//": "standard usage. If you roll the model, update the id AND these rates together.", + "//": "", + "//": "The apiKey interpolation reads OPENROUTER_API_KEY from the workflow environment", + "//": "(the same org secret the Codex stage uses) — no interactive login or OAuth is", + "//": "involved. api/api identify the OpenAI-compatible endpoint pi uses.", + "providers": { + "openrouter": { + "baseUrl": "https://openrouter.ai/api/v1", + "apiKey": "$OPENROUTER_API_KEY", + "api": "openai-completions", + "models": [ + { + "id": "deepseek/deepseek-v4.1-flash", + "name": "DeepSeek V4.1 Flash", + "reasoning": true, + "cost": { + "input": 0.15, + "output": 0.6, + "cacheRead": 0.003, + "cacheWrite": 0 + } + } + ] + } + } +} \ No newline at end of file diff --git a/scripts/resolve-prompts.sh b/scripts/resolve-prompts.sh index b41ecfc..24aeb6f 100755 --- a/scripts/resolve-prompts.sh +++ b/scripts/resolve-prompts.sh @@ -6,7 +6,7 @@ # experiment arm gets. This script picks ONE arm per review — hash(':') # mod 100, below experiment_split_percent means the experiment arm — and emits only # that arm's prompt. The assigned arm both posts its summary and decides the review, -# so arms are compared BETWEEN pull requests and each review costs one Claude pass. +# so arms are compared BETWEEN pull requests and each review costs one Stage-2 pass. # # Shared rule blocks (privacy, migration-data, perf) are stored once in # prompts/_shared/ and injected into every prompt that references them via {{@path}} diff --git a/scripts/review-diagnostics.py b/scripts/review-diagnostics.py index f9e2be2..fe74ba9 100644 --- a/scripts/review-diagnostics.py +++ b/scripts/review-diagnostics.py @@ -1,5 +1,20 @@ #!/usr/bin/env python3 -"""Save terminal review errors without publishing tool transcripts.""" +"""Save terminal review errors without publishing tool transcripts. + +Understands the two Stage-2 harness output formats that have existed: + +- pi's JSON event stream (`pi --mode json`): the terminal assistant message, its + stop reason / error, and pass usage are extracted from the stream. Tool calls, + their arguments and results are never included. +- Claude Code's execution file (historical): terminal "result" messages only. + +Also appends a redacted tail of pi's stderr (passed as an optional third +argument) so upstream errors like rate limits show up in diagnostics. + +Everything that goes into the report is passed through redact(), which blanks +known secrets (the provider key, the review PAT, the GITHUB_TOKEN) anywhere they +appear. +""" import html import json import os @@ -9,22 +24,105 @@ FIELDS = ('type', 'subtype', 'is_error', 'result', 'errors', 'duration_ms', 'num_turns', 'total_cost_usd', 'session_id') +# pi --mode json emits these event types on the wire. +PI_EVENT_TYPES = {'session', 'agent_start', 'agent_end', 'turn_start', 'turn_end', + 'message_start', 'message_end', 'message_update'} + + +def read_events(text): + """Parse a file that is a JSON array, a single JSON object, or JSON-lines into a + list of dicts. Empty on anything unreadable.""" + try: + parsed = json.loads(text) + except json.JSONDecodeError: + parsed = [] + for line in text.splitlines(): + if not line.strip(): + continue + try: + parsed.append(json.loads(line)) + except json.JSONDecodeError: + continue + if isinstance(parsed, dict): + parsed = [parsed] + if not isinstance(parsed, list): + return [] + return [item for item in parsed if isinstance(item, dict)] + + +def is_pi_stream(events): + """A pi event stream announces itself with wire event types.""" + return any(event.get('type') in PI_EVENT_TYPES for event in events) + + +def pi_result(events): + """The terminal assistant message from a pi event stream, stripped of tools: + stop reason, error, model, usage, turns, and (if timestamps survive) duration. + Tool calls, arguments, and results are never read.""" + candidates = [] + for event in events: + message = event.get('message') + if event.get('type') == 'agent_end': + messages = event.get('messages') + if isinstance(messages, list) and messages: + message = messages[-1] + if not isinstance(message, dict) or message.get('role') != 'assistant': + continue + candidates.append(message) + if not candidates: + return None + message = candidates[-1] + text = ' '.join(part.get('text', '') + for part in message.get('content', []) + if isinstance(part, dict) and part.get('type') == 'text') + is_error = message.get('stopReason') == 'error' + result = { + 'subtype': message.get('stopReason'), + 'is_error': is_error, + 'num_turns': sum(1 for e in events if e.get('type') == 'turn_start'), + 'model': message.get('model'), + 'provider': message.get('provider'), + } + usage = message.get('usage') + if isinstance(usage, dict): + cost = usage.get('cost') + if isinstance(cost, dict): + total = cost.get('total') + if isinstance(total, (int, float)) and not isinstance(total, bool): + result['total_cost'] = total + result['usage'] = {key: usage[key] for key in + ('input', 'output', 'cacheRead', 'cacheWrite', 'totalTokens') + if key in usage} + if is_error: + result['errors'] = ([message.get('errorMessage')] + if message.get('errorMessage') else []) + result['result'] = message.get('errorMessage') or '' + else: + result['result'] = text + stamps = [ts for event in events + if isinstance(ts := event.get('timestamp'), (int, float))] + for message in candidates: + if isinstance(ts := message.get('timestamp'), (int, float)): + stamps.append(ts) + if stamps: + result['duration_ms'] = int(max(stamps) - min(stamps)) + return result + def read_result(source): try: text = source.read_text() - try: - messages = json.loads(text) - except json.JSONDecodeError: - messages = [json.loads(line) for line in text.splitlines() if line.strip()] - except (OSError, ValueError): + except OSError: return {'status': 'execution output unavailable'} - if isinstance(messages, dict): - messages = [messages] - if not isinstance(messages, list): + events = read_events(text) + if not events: + return {'status': 'execution output unavailable'} + if is_pi_stream(events): + result = pi_result(events) + if result is not None: + return {'status': 'captured', 'result': result} return {'status': 'result unavailable'} - results = [message for message in messages - if isinstance(message, dict) and message.get('type') == 'result'] + results = [event for event in events if event.get('type') == 'result'] if not results: return {'status': 'result unavailable'} return {'status': 'captured', 'result': { @@ -32,6 +130,17 @@ def read_result(source): }} +def read_stderr_tail(path, max_lines=40): + """The last lines of pi's captured stderr, or None when absent.""" + if not path: + return None + try: + lines = Path(path).read_text(errors='replace').splitlines() + except OSError: + return None + return lines[-max_lines:] or None + + def redact(value, secrets): if isinstance(value, str): for secret in secrets: @@ -45,12 +154,21 @@ def redact(value, secrets): def main(): - source, destination = map(Path, sys.argv[1:]) + args = sys.argv[1:] + if len(args) < 2: + print('usage: review-diagnostics.py [stderr]', + file=sys.stderr) + return 2 + source, destination = map(Path, args[:2]) + stderr_path = Path(args[2]) if len(args) > 2 else None secrets = json.loads(os.environ.get('DIAGNOSTIC_SECRET_VALUES', '[]')) secrets += [value for key, value in os.environ.items() if key.startswith('DIAGNOSTIC_TOKEN_') and value] secrets = sorted(set(filter(None, secrets)), key=len, reverse=True) report = redact(read_result(source), secrets) + tail = read_stderr_tail(stderr_path) + if tail is not None: + report['stderr_tail'] = redact(tail, secrets) output = json.dumps(report, indent=2) destination.parent.mkdir(parents=True, exist_ok=True) destination.write_text(output + '\n') @@ -64,4 +182,4 @@ def main(): if __name__ == '__main__': - main() + sys.exit(main()) \ No newline at end of file diff --git a/tests/test_llm_usage.py b/tests/test_llm_usage.py index 9bd63e4..d434f50 100644 --- a/tests/test_llm_usage.py +++ b/tests/test_llm_usage.py @@ -29,6 +29,29 @@ def token_count(**usage): 'payload': {'type': 'token_count', 'info': {'total_token_usage': usage}}} +def pi_message(stop_reason='stop', usage=None, text='', error=None, timestamp=2000): + message = {'role': 'assistant', 'stopReason': stop_reason, 'timestamp': timestamp} + if usage is not None: + message['usage'] = usage + if error: + message['errorMessage'] = error + elif text: + message['content'] = [{'type': 'text', 'text': text}] + return message + + +def pi_stream(*messages): + """A minimal pi event stream: session, turn_start, one message_end per + assistant message, then turn_end and agent_end carrying all of them (the last + message is the terminal one).""" + events = [{'type': 'session'}, {'type': 'turn_start'}] + events += [{'type': 'message_end', 'message': message} + for message in messages] + events += [{'type': 'turn_end'}, + {'type': 'agent_end', 'messages': list(messages)}] + return events + + class SplitModelTest(unittest.TestCase): def test_splits_openrouter_slug_into_catalog_name_and_provider(self): self.assertEqual(llm_usage.split_model('openai/gpt-5.6-sol'), @@ -139,6 +162,62 @@ def test_missing_session_directory_yields_nothing(self): self.assertEqual(llm_usage.codex_usage(''), ({}, None)) +class PiUsageTest(unittest.TestCase): + def test_reads_counts_and_computed_cost_from_the_last_finished_message(self): + usage = {'input': 17515, 'output': 17, 'cacheRead': 17024, + 'cacheWrite': 0, 'totalTokens': 34556, + 'cost': {'input': 0.000332785, 'output': 5.1e-07, + 'cacheRead': 0.000038966, 'cacheWrite': 0, + 'total': 0.000372255}} + path = write('\n'.join(json.dumps(e) for e in pi_stream(pi_message(usage=usage)))) + counts, cost = llm_usage.pi_usage(path) + self.assertEqual(counts, {'input_tokens': 17515, 'output_tokens': 17, + 'cache_read_input_tokens': 17024, + 'cache_write_input_tokens': 0}) + self.assertEqual(cost, 0.000372255) + + def test_prefers_the_last_clean_message_over_a_failed_retry_after_it(self): + clean = pi_message(stop_reason='stop', text='review done', usage={ + 'input': 500, 'output': 100, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 600, 'cost': {'total': 0.9}}) + failed = pi_message(stop_reason='error', error='429 rate limited', usage={ + 'input': 10, 'output': 0, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 10, 'cost': {'total': 0.0}}) + path = write('\n'.join(json.dumps(e) for e in pi_stream(clean, failed))) + counts, cost = llm_usage.pi_usage(path) + self.assertEqual(counts['input_tokens'], 500) + self.assertEqual(cost, 0.9) + + def test_zero_cost_is_no_reported_cost(self): + usage = {'input': 10, 'output': 2, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 12, 'cost': {'total': 0.0}} + path = write('\n'.join(json.dumps(e) for e in pi_stream(pi_message(usage=usage)))) + _, cost = llm_usage.pi_usage(path) + self.assertIsNone(cost) + + def test_missing_file_yields_nothing(self): + self.assertEqual(llm_usage.pi_usage('/nonexistent/pi-output.jsonl'), ({}, None)) + self.assertEqual(llm_usage.pi_usage(''), ({}, None)) + + def test_stream_without_an_assistant_message_yields_nothing(self): + path = write('\n'.join(json.dumps(e) for e in [ + {'type': 'session'}, {'type': 'turn_start'}, + {'type': 'agent_end', 'messages': []}, + ])) + self.assertEqual(llm_usage.pi_usage(path), ({}, None)) + + def test_agent_end_alone_is_enough_when_no_message_end_events_exist(self): + events = [{'type': 'session'}, + {'type': 'agent_end', 'messages': [ + pi_message(usage={'input': 7, 'output': 3, 'cacheRead': 0, + 'cacheWrite': 0, 'totalTokens': 10, + 'cost': {'total': 0.01}})]}] + path = write('\n'.join(json.dumps(e) for e in events)) + counts, cost = llm_usage.pi_usage(path) + self.assertEqual(counts['input_tokens'], 7) + self.assertEqual(cost, 0.01) + + class BuildSpanFieldsTest(unittest.TestCase): def test_reports_catalog_identity_and_derived_total(self): fields = llm_usage.build_span_fields( diff --git a/tests/test_review_diagnostics.py b/tests/test_review_diagnostics.py index c0490ee..180e54d 100644 --- a/tests/test_review_diagnostics.py +++ b/tests/test_review_diagnostics.py @@ -60,6 +60,71 @@ def test_no_result_does_not_publish_transcript(self): self.assertEqual(report['status'], 'result unavailable') self.assertNotIn('private tool transcript', summary) + def test_pi_stream_captures_terminal_message_without_tool_transcripts(self): + stream = [ + {'type': 'session'}, + {'type': 'turn_start'}, + {'type': 'tool_execution_end', 'toolName': 'write', + 'args': {'file_path': '/tmp/x', 'content': 'private tool transcript'}, + 'result': 'written: private tool transcript'}, + {'type': 'message_end', 'message': {'role': 'assistant', + 'stopReason': 'error', + 'errorMessage': '429 provider error private-test-secret', + 'model': 'deepseek/deepseek-v4.1-flash', + 'provider': 'openrouter', + 'usage': {'input': 5, 'output': 1, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 6, 'cost': {'total': 0.0012}}}}, + {'type': 'turn_end'}, + {'type': 'agent_end', 'messages': [{'role': 'assistant', + 'stopReason': 'error', + 'errorMessage': '429 provider error private-test-secret', + 'model': 'deepseek/deepseek-v4.1-flash', + 'provider': 'openrouter', + 'usage': {'input': 5, 'output': 1, 'cacheRead': 0, 'cacheWrite': 0, + 'totalTokens': 6, 'cost': {'total': 0.0012}}}]}, + ] + report, summary = self.run_diagnostics(json.dumps(stream)) + self.assertEqual(report['status'], 'captured') + self.assertTrue(report['result']['is_error']) + self.assertEqual(report['result']['errors'], + ['429 provider error [REDACTED]']) + self.assertEqual(report['result']['num_turns'], 1) + self.assertEqual(report['result']['total_cost'], 0.0012) + self.assertNotIn('private tool transcript', json.dumps(report) + summary) + self.assertNotIn('private-test-secret', json.dumps(report) + summary) + + def test_pi_stream_success_reports_the_terminal_text(self): + stream = [ + {'type': 'agent_end', 'messages': [{'role': 'assistant', + 'stopReason': 'stop', + 'content': [{'type': 'text', 'text': '## Review summary private-test-secret'}]}]}, + ] + report, summary = self.run_diagnostics(json.dumps(stream)) + self.assertEqual(report['result']['is_error'], False) + self.assertIn('Review summary [REDACTED]', report['result']['result']) + + def test_pi_stderr_tail_is_captured_and_redacted(self): + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + source = root / 'pi-output.jsonl' + source.write_text(json.dumps({'type': 'agent_end', 'messages': [ + {'role': 'assistant', 'stopReason': 'error', + 'errorMessage': 'boom'}]})) + destination = root / 'diagnostics.json' + stderr = root / 'pi-error.log' + stderr.write_text('line1\nline2 private-test-secret\n') + summary = root / 'summary.md' + result = subprocess.run( + ['python3', str(SCRIPT), str(source), str(destination), str(stderr)], + env={**os.environ, 'GITHUB_STEP_SUMMARY': str(summary), + 'DIAGNOSTIC_SECRET_VALUES': json.dumps(['private-test-secret'])}, + capture_output=True, text=True, + ) + self.assertEqual(result.returncode, 0, result.stderr) + report = json.loads(destination.read_text()) + self.assertEqual(report['stderr_tail'], ['line1', 'line2 [REDACTED]']) + self.assertNotIn('private-test-secret', stderr.read_text() and '') + if __name__ == '__main__': unittest.main()