From 2deecfe23645b5c3d509902911a1f29a2e664bf9 Mon Sep 17 00:00:00 2001 From: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Date: Wed, 8 Jul 2026 21:51:25 -0500 Subject: [PATCH] chore(lsp-pilot): retire the LSP pilot after Shell NO-GO (#839, #845) The LSP language-server pilot is a decided NO-GO for Shell/bash-language-server (#845; two live A/B runs: sonnet 1.26x / opus 1.05x nav-token ratio, well under the >=2x bar). Remove the ~3,800 LOC pilot footprint rather than carry it dormant. Removed (27 dedicated files): evals/lsp-pilot/*, scripts/lsp_pilot_*.sh, setup-lsp-pilot.sh, lib/lsp-verification.sh, .github/mcp/lsp.json, .github/workflows/lsp-pilot-run.yml, docs/lsp-pilot*.md, all tests/**/*lsp*. Surgically stripped the LSP hooks from 11 shared production files, preserving the Context7/general-MCP mechanism (REVIEW_MCP_CONFIG/REVIEW_MCP_ALLOWED_TOOLS, epic #676/#681) untouched: - review-one-pr.sh: drop lsp-verification source + apply_lsp_verification (deep + audit) + the LSP_PILOT_ENABLED emit block/trap - engine.sh: drop _lsp_pilot_active + the stream-json capture branches; the non-pilot path reverts to --output-format json parsed directly (unchanged) - token-metrics.sh: drop emit_verification_record + emit_lsp_coldstart_record (dead; no callers outside the deleted files) - prompts/deep-review.md + security-audit.md: drop the lsp_verification annotation block (dead instruction weight on every production review) - pr-review.yml: drop lsp_pilot_variant input + LSP_PILOT_ENABLED env + the LSP cache/setup steps - lint.yml / test-dev-lead.yml: drop registrations of the deleted lsp tests - stub-claude fixture + test_holdout_guard.bats + test_mcp_connectivity_check.bats: drop LSP-only cases (generic behavior stays covered) The reusable harness + per-language restart recipe + the Shell corpus are pinned in #1124 (rebuild ref f882266); the decision + numbers are on #845. A future per-language re-pilot (Python #1125 / TS-JS #1126) cherry-picks from that ref. Validation: orphan grep empty; shellcheck clean; YAML parses; Context7 plumbing intact; holdout-guard + mcp-connectivity + engine/token-metrics bats green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/mcp/lsp.json | 11 - .github/workflows/lint.yml | 2 - .github/workflows/lsp-pilot-run.yml | 94 ---- .github/workflows/pr-review.yml | 73 --- .github/workflows/test-dev-lead.yml | 14 - docs/lsp-pilot-report.md | 148 ------ docs/lsp-pilot.md | 257 ----------- evals/lsp-pilot/README.md | 90 ---- evals/lsp-pilot/dev/cases.jsonl | 2 - .../lsp-pilot/holdout/baseline-lsp-off.jsonl | 6 - evals/lsp-pilot/holdout/cases.jsonl | 6 - evals/lsp-pilot/runs/agent-lsp.jsonl | 6 - evals/lsp-pilot/runs/serena.jsonl | 6 - prompts/deep-review.md | 26 +- prompts/security-audit.md | 21 +- scripts/engine.sh | 55 +-- scripts/lib/lsp-verification.sh | 141 ------ scripts/lib/token-metrics.sh | 94 ---- scripts/lsp_pilot_compare.sh | 283 ------------ scripts/lsp_pilot_emit.sh | 173 ------- scripts/lsp_pilot_measure.sh | 238 ---------- scripts/lsp_pilot_report.sh | 436 ------------------ scripts/lsp_pilot_run.sh | 187 -------- scripts/review-one-pr.sh | 42 -- scripts/setup-lsp-pilot.sh | 302 ------------ tests/dev-lead/fixtures/engines/stub-claude | 28 -- .../test_pr_review_lsp_pilot_plumbing.py | 192 -------- .../dev-lead/unit/test_engine_lsp_pilot.bats | 160 ------- .../dev-lead/unit/test_lsp_coldstart_sla.bats | 160 ------- tests/dev-lead/unit/test_lsp_pilot.bats | 202 -------- tests/dev-lead/unit/test_lsp_pilot_emit.bats | 232 ---------- .../dev-lead/unit/test_lsp_pilot_measure.bats | 142 ------ tests/dev-lead/unit/test_lsp_pilot_run.bats | 43 -- .../dev-lead/unit/test_lsp_verification.bats | 223 --------- tests/lsp_pilot_compare.bats | 154 ------- tests/lsp_pilot_report.bats | 234 ---------- tests/test_holdout_guard.bats | 24 - tests/test_mcp_connectivity_check.bats | 4 +- 38 files changed, 7 insertions(+), 4504 deletions(-) delete mode 100644 .github/mcp/lsp.json delete mode 100644 .github/workflows/lsp-pilot-run.yml delete mode 100644 docs/lsp-pilot-report.md delete mode 100644 docs/lsp-pilot.md delete mode 100644 evals/lsp-pilot/README.md delete mode 100644 evals/lsp-pilot/dev/cases.jsonl delete mode 100644 evals/lsp-pilot/holdout/baseline-lsp-off.jsonl delete mode 100644 evals/lsp-pilot/holdout/cases.jsonl delete mode 100644 evals/lsp-pilot/runs/agent-lsp.jsonl delete mode 100644 evals/lsp-pilot/runs/serena.jsonl delete mode 100644 scripts/lib/lsp-verification.sh delete mode 100644 scripts/lsp_pilot_compare.sh delete mode 100644 scripts/lsp_pilot_emit.sh delete mode 100644 scripts/lsp_pilot_measure.sh delete mode 100644 scripts/lsp_pilot_report.sh delete mode 100644 scripts/lsp_pilot_run.sh delete mode 100755 scripts/setup-lsp-pilot.sh delete mode 100644 tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py delete mode 100644 tests/dev-lead/unit/test_engine_lsp_pilot.bats delete mode 100644 tests/dev-lead/unit/test_lsp_coldstart_sla.bats delete mode 100644 tests/dev-lead/unit/test_lsp_pilot.bats delete mode 100644 tests/dev-lead/unit/test_lsp_pilot_emit.bats delete mode 100644 tests/dev-lead/unit/test_lsp_pilot_measure.bats delete mode 100644 tests/dev-lead/unit/test_lsp_pilot_run.bats delete mode 100644 tests/dev-lead/unit/test_lsp_verification.bats delete mode 100644 tests/lsp_pilot_compare.bats delete mode 100644 tests/lsp_pilot_report.bats diff --git a/.github/mcp/lsp.json b/.github/mcp/lsp.json deleted file mode 100644 index 93444746d..000000000 --- a/.github/mcp/lsp.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "mcpServers": { - "lsp": { - "type": "stdio", - "command": "agent-lsp", - "args": [ - "bash:bash-language-server,start" - ] - } - } -} diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index cd09c64f7..27ce15898 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -51,8 +51,6 @@ jobs: tests/test_initiative_driver_canary.bats \ tests/test_mcp_connectivity_check.bats \ tests/token_report.bats \ - tests/lsp_pilot_compare.bats \ - tests/lsp_pilot_report.bats \ tests/auto_rebase_health.bats \ tests/model_pricing.bats \ tests/test_review_cycle.bats \ diff --git a/.github/workflows/lsp-pilot-run.yml b/.github/workflows/lsp-pilot-run.yml deleted file mode 100644 index ba85fece1..000000000 --- a/.github/workflows/lsp-pilot-run.yml +++ /dev/null @@ -1,94 +0,0 @@ -name: LSP Pilot — Comparative Run (#844) - -# Self-contained, on-demand driver for the LSP-pilot comparison (epic #839, story -# #844). NOT part of the PR-review duty and NOT a thin caller: it stands alone so it -# can be dispatched without touching the shared pr-review channel or the -# LSP_PILOT_ENABLED repo variable. -# -# For each input PR it runs the SAME navigation-heavy review twice — once LSP-off -# (Grep/Glob/Read/Bash) and once LSP-on (+ the pinned agent-lsp MCP navigation -# allowlist) — measures each run's transcript with scripts/lsp_pilot_measure.sh, and -# renders scripts/lsp_pilot_compare.sh into the job summary. Both sides are measured -# live on the same real PRs, so the A/B does not depend on the frozen synthetic -# baseline. See scripts/lsp_pilot_run.sh for the honest-scope notes (cost probe, not -# the full review pipeline). -# -# Cost: this spends real model credits (two reviews per PR). Keep the PR list small -# and the model modest (default claude-sonnet-4-6) for a smoke; bump deliberately. - -on: - workflow_dispatch: - inputs: - pr_numbers: - # Default smoke corpus: real merged .github-private PRs with genuine - # cross-file shell changes (good find-references targets) — #928 (churn - # breaker, 5 scripts) + #899 (self-review deadlock fix, 2 scripts). - description: "Comma-separated PR numbers to A/B" - required: false - default: "928,899" - type: string - model: - description: "Model id for both legs (keep modest to bound cost)" - required: false - default: "claude-sonnet-4-6" - type: string - candidate: - description: "LSP candidate server name (label only; install is agent-lsp)" - required: false - default: "agent-lsp" - type: string - -permissions: - contents: read - pull-requests: read - -concurrency: - group: lsp-pilot-run - cancel-in-progress: false - -jobs: - pilot: - runs-on: ubuntu-latest - env: - CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} - GH_TOKEN: ${{ secrets.DON_PETRY_BOT_GH_PAT_CLASSIC || secrets.DON_PETRY_BOT_GH_PAT || secrets.GITHUB_TOKEN }} - TOKEN_LOG_FILE: /tmp/token-usage-${{ github.run_id }}.jsonl - REPO_SLUG: ${{ github.repository }} - LSP_PILOT_MODEL: ${{ inputs.model }} - LSP_CANDIDATE: ${{ inputs.candidate }} - AGENT_LSP_VERSION: ${{ vars.AGENT_LSP_VERSION || 'v0.15.0' }} - BASH_LANGUAGE_SERVER_VERSION: ${{ vars.BASH_LANGUAGE_SERVER_VERSION || '5.6.0' }} - CLAUDE_CODE_VERSION: ${{ vars.CLAUDE_CODE_VERSION || 'latest' }} - LSP_PILOT_OUTDIR: lsp-pilot-out - steps: - - name: Checkout - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - - - name: Setup Node - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 - with: - node-version: "22" - - - name: Install Claude Code CLI - run: npm install -g "@anthropic-ai/claude-code@${CLAUDE_CODE_VERSION}" - - - name: Install LSP toolchain + record cold start - # setup-lsp-pilot.sh installs agent-lsp (gh release) + bash-language-server, - # times the bring-up, and writes the lsp_cold_start record to TOKEN_LOG_FILE. - # It "warns, never fails": if the toolchain is unavailable the LSP-on leg - # simply runs without the MCP server (degraded, not broken). - run: bash scripts/setup-lsp-pilot.sh - - - name: Run comparative pilot - run: bash scripts/lsp_pilot_run.sh "${{ inputs.pr_numbers }}" - - - name: Upload pilot artifacts - if: always() - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 - with: - name: lsp-pilot-out-${{ github.run_id }} - path: | - lsp-pilot-out/** - ${{ env.TOKEN_LOG_FILE }} - if-no-files-found: warn - retention-days: 14 diff --git a/.github/workflows/pr-review.yml b/.github/workflows/pr-review.yml index c42804f8d..a1b6f0158 100644 --- a/.github/workflows/pr-review.yml +++ b/.github/workflows/pr-review.yml @@ -32,18 +32,6 @@ on: type: string required: false default: "" - # LSP-MCP review pilot A/B leg selector (epic #839, story #844, issue #1031). - # Decouples pilot capture from LSP wiring so ONE production pipeline can emit - # both control legs: - # on → wire the LSP MCP (setup step) AND capture → lsp-on (B leg) - # off → capture only, skip LSP wiring → lsp-off (A control) - # none/'' → today's behaviour (defers to the legacy vars.LSP_PILOT_ENABLED) - # A trigger stub forwards this so the A/B can be driven on this repo's PRs. - lsp_pilot_variant: - description: "LSP pilot A/B leg: on (wire LSP + capture) | off (capture only) | none" - type: string - required: false - default: "" secrets: CLAUDE_CODE_OAUTH_TOKEN: required: false @@ -194,26 +182,6 @@ jobs: # server handshake (`--debug mcp`) and surface a ::notice:: for a healthy # connection. Off by default; never changes the review verdict. REVIEW_MCP_DEBUG: ${{ vars.REVIEW_MCP_DEBUG || '' }} - # ── LSP-MCP review pilot capture gate (epic #839, story #844, issue #1031) ─ - # engine.sh's _lsp_pilot_active() and review-one-pr.sh's emit trap gate on - # the ENV var LSP_PILOT_ENABLED — NOT the repo variable. Export it here so a - # production review actually captures the stream-json transcript and appends - # the kind:"lsp_pilot_run" record to TOKEN_LOG_FILE (Gap 1). On when the - # legacy repo variable is set OR a pilot A/B variant (on|off) is - # dispatched/defaulted. Empty (the default) → no capture, no records, no - # extra flags — byte-for-byte unchanged for the consumer repos (AC #4). - LSP_PILOT_ENABLED: >- - ${{ (vars.LSP_PILOT_ENABLED == 'true' - || inputs.lsp_pilot_variant == 'on' || inputs.lsp_pilot_variant == 'off' - || vars.LSP_PILOT_VARIANT == 'on' || vars.LSP_PILOT_VARIANT == 'off') - && 'true' || '' }} - # A/B leg selector that DECOUPLES capture from LSP wiring (Gap 2). 'on' wires - # the LSP MCP (the setup step below) → lsp-on (B); 'off' skips wiring → lsp-off - # (A control), capture only. lpe_variant() treats this as authoritative; - # empty/'none' preserves the legacy vars.LSP_PILOT_ENABLED behaviour (falls - # back to REVIEW_MCP_CONFIG detection). The dispatch input beats the repo-var - # default so the A/B can be driven per-run. - LSP_PILOT_VARIANT: ${{ inputs.lsp_pilot_variant || vars.LSP_PILOT_VARIANT || '' }} # Downstream-impact pass (epic #748 / discussion #653): annotate a review # with the consumer repos that pin a shared surface the PR changes. Enabled # by default here; set the repo variable DOWNSTREAM_IMPACT_ENABLED=false to @@ -390,47 +358,6 @@ jobs: ;; esac - # ── LSP-MCP review pilot (epic #839, story #842/#844) ──────────────── - # Opt-in ONLY: these WIRING steps run when the repo variable - # LSP_PILOT_ENABLED == 'true' OR the pilot variant resolves to 'on' (the B - # leg). The 'off' (A control) leg deliberately does NOT run them — it - # captures a review WITHOUT wiring the LSP MCP (issue #1031, Gap 2). - # When neither is set (the default) these steps are skipped entirely, - # REVIEW_MCP_CONFIG stays unset, and the review behaves byte-for-byte as - # today (the engine's .github/review-mcp.json default is untouched). When on, the - # script installs the pinned LSP-MCP candidate + bash-language-server and - # threads the deep/audit-only MCP knobs into $GITHUB_ENV. A missing tool - # degrades gracefully (::warning:: + skip) — it never fails the workflow. - # Pins/candidate are overridable via repo variables; see docs/lsp-pilot.md. - # Cache the LSP toolchain/index across ephemeral runs (story #846) so a warm - # run restores under the 30s cold-start SLA instead of re-installing. Keyed on - # OS + candidate + pinned tool versions + a hash of the indexed Shell sources - # (the "relevant lockfile" for a Shell-only index). SHA-pinned to the same - # actions/cache the Claude CLI cache above uses. Opt-in behind LSP_PILOT_ENABLED; - # its cache-hit output is threaded into the setup step for the cold-start metric. - - name: Cache LSP index - id: lsp-cache - # Wire the LSP toolchain for the legacy repo var OR the 'on' (B) leg only. - # The 'off' (A) control leg deliberately skips wiring — capture without LSP. - if: ${{ vars.LSP_PILOT_ENABLED == 'true' || (inputs.lsp_pilot_variant || vars.LSP_PILOT_VARIANT) == 'on' }} - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 - with: - path: ~/.local - key: lsp-pilot-${{ runner.os }}-${{ vars.LSP_CANDIDATE || 'agent-lsp' }}-${{ vars.AGENT_LSP_VERSION || 'v0.15.0' }}-bls${{ vars.BASH_LANGUAGE_SERVER_VERSION || '5.6.0' }}-${{ hashFiles('scripts/**/*.sh', '**/package-lock.json') }} - restore-keys: | - lsp-pilot-${{ runner.os }}-${{ vars.LSP_CANDIDATE || 'agent-lsp' }}-${{ vars.AGENT_LSP_VERSION || 'v0.15.0' }}-bls${{ vars.BASH_LANGUAGE_SERVER_VERSION || '5.6.0' }}- - - - name: Set up LSP pilot servers (opt-in) - # Wire the LSP MCP for the legacy repo var OR the 'on' (B) leg only; the - # 'off' (A) control leg captures WITHOUT wiring REVIEW_MCP_CONFIG. - if: ${{ vars.LSP_PILOT_ENABLED == 'true' || (inputs.lsp_pilot_variant || vars.LSP_PILOT_VARIANT) == 'on' }} - env: - LSP_CANDIDATE: ${{ vars.LSP_CANDIDATE || 'agent-lsp' }} - AGENT_LSP_VERSION: ${{ vars.AGENT_LSP_VERSION || 'v0.15.0' }} - BASH_LANGUAGE_SERVER_VERSION: ${{ vars.BASH_LANGUAGE_SERVER_VERSION || '5.6.0' }} - LSP_INDEX_CACHE_HIT: ${{ steps.lsp-cache.outputs.cache-hit }} - run: bash scripts/setup-lsp-pilot.sh - - name: Enumerate candidate PRs run: | set -euo pipefail diff --git a/.github/workflows/test-dev-lead.yml b/.github/workflows/test-dev-lead.yml index afa3a43de..e4ce2e4f7 100644 --- a/.github/workflows/test-dev-lead.yml +++ b/.github/workflows/test-dev-lead.yml @@ -12,7 +12,6 @@ on: - '.github/workflows/dependency-audit.yml' - '.github/workflows/auto-rebase.yml' - '.github/workflows/auto-rebase-retry.yml' - # LSP pilot capture/decoupling plumbing lives in the pr-review workflows (#1031). - '.github/workflows/pr-review.yml' - '.github/workflows/pr-review-trigger.yml' - 'tests/dev-lead/**' @@ -157,16 +156,3 @@ jobs: run: python3 -m pip install --quiet pyyaml - name: Validate auto-rebase.yml uses @auto-rebase/next (not a SHA or frozen tag) run: python3 tests/dev-lead/integration/test_auto_rebase_stub.py - - lsp-pilot-plumbing: - runs-on: ubuntu-latest - permissions: - contents: read - steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - with: - persist-credentials: false - - name: Install PyYAML - run: python3 -m pip install --quiet pyyaml - - name: Validate pr-review LSP-pilot capture gate + variant decoupling plumbing - run: python3 tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py diff --git a/docs/lsp-pilot-report.md b/docs/lsp-pilot-report.md deleted file mode 100644 index 4e3d4f782..000000000 --- a/docs/lsp-pilot-report.md +++ /dev/null @@ -1,148 +0,0 @@ - - -> **Status — structural / seed-data run.** The frozen corpus -> (`evals/lsp-pilot/holdout/cases.jsonl`) and the immutable LSP-off baseline -> (`evals/lsp-pilot/holdout/baseline-lsp-off.jsonl`) are still **synthetic, -> de-identified seeds** (see `evals/lsp-pilot/README.md`), and the candidate runs -> under `evals/lsp-pilot/runs/` are the matching seed captures. This report -> therefore exercises the full speed/quality/cost rendering and the go/no-go logic -> end-to-end, but it is **not yet the real go/no-go record**: it becomes the real -> input the moment the corpus + baseline are repinned to the real pilot PR set and -> the candidate servers are re-run. The pipeline, metric, and honesty annotations -> below are final; only the numbers are seed data. - -## Reproduce - -```bash -bash scripts/lsp_pilot_report.sh \ - evals/lsp-pilot/holdout/baseline-lsp-off.jsonl \ - evals/lsp-pilot/holdout/cases.jsonl \ - "agent-lsp=evals/lsp-pilot/runs/agent-lsp.jsonl" \ - "serena=evals/lsp-pilot/runs/serena.jsonl" -``` - ---- - -# 🔬 LSP pilot — comparative speed / quality / cost report - -_Each candidate LSP-on run scored against the **immutable LSP-off baseline** (`evals/lsp-pilot/holdout/baseline-lsp-off.jsonl`) over the frozen corpus (`evals/lsp-pilot/holdout/cases.jsonl`). The baseline is consumed, never re-derived._ - -## Success metric (verbatim, epic #839) - -> Go = on the frozen pilot PR corpus, LSP-on deep-tier review delivers a measurable -> navigation-token reduction (**target ≥2x fewer navigation tool-call tokens** vs the -> LSP-off control) AND **no regression in review quality** (false-positive / precision -> no worse than the frozen LSP-off baseline), achieved **within the cold-start SLA -> (≤30s P95)**. A win on tokens that costs precision is a _no-go_, not a win. - -## Candidate: `agent-lsp` - -# 🔬 LSP pilot comparison — `agent-lsp` vs frozen LSP-off baseline - -_Speed (cold-start, wall-time) · cost (nav tokens, tool calls, ET, USD) · quality (findings, false positives), LSP-on vs the immutable LSP-off control._ - -**Quality proxy:** a PR regresses when its **false-positive** count exceeds the frozen baseline's (precision dropped). A navigation-token win that costs precision is a no-go. - -## Per PR (off → on) - -| PR | Nav tokens | Nav reduction | Tool calls | Cold start | Wall time | ET | USD | Findings | False positives | Quality | -|---|---|---:|---|---:|---|---:|---:|---|---:|:--| -| `petry-projects/.github-private#701@0000000000000000000000000000000000000701` | 21,800 → 8,000 | 2.7x | 34 → 12 | N/A → 9.0s | 188.0s → 95.0s | 283,000 → 193,000 | $0.30 → $0.21 | 4 → 4 | 1 → 1 | ok | -| `petry-projects/.github-private#702@0000000000000000000000000000000000000702` | 15,400 → 6,000 | 2.6x | 26 → 11 | N/A → 8.0s | 151.0s → 78.0s | 242,000 → 154,500 | $0.26 → $0.17 | 3 → 3 | 1 → 1 | ok | -| `petry-projects/.github-private#703@0000000000000000000000000000000000000703` | 9,800 → 4,000 | 2.5x | 18 → 8 | N/A → 7.0s | 121.0s → 64.0s | 192,000 → 123,000 | $0.20 → $0.13 | 2 → 2 | 0 → 0 | ok | -| `petry-projects/.github-private#704@0000000000000000000000000000000000000704` | 7,600 → 3,200 | 2.4x | 14 → 7 | N/A → 7.0s | 104.0s → 55.0s | 162,500 → 106,000 | $0.17 → $0.12 | 2 → 2 | 0 → 0 | ok | -| `petry-projects/.github-private#705@0000000000000000000000000000000000000705` | 29,600 → 11,000 | 2.7x | 48 → 16 | N/A → 11.0s | 233.0s → 120.0s | 345,000 → 224,000 | $0.36 → $0.24 | 5 → 5 | 2 → 2 | ok | -| `petry-projects/.github-private#706@0000000000000000000000000000000000000706` | 1,200 → 1,200 | 1.0x | 3 → 3 | N/A → 6.0s | 58.0s → 40.0s | 91,500 → 91,500 | $0.10 → $0.10 | 1 → 1 | 0 → 0 | ok | - -## Aggregate - -- **Navigation tool-call tokens:** 85,400 (off) → 33,400 (on) — **2.6x** reduction -- **ET:** 1,316,000 (off) → 892,000 (on) -- **USD:** $1.39 (off) → $0.96 (on) -- **Mean LSP-on cold start:** 8.0s (baseline N/A — no server) -- **Findings:** 17 (off) → 17 (on) -- **False positives:** 4 (off) → 4 (on) -- **Quality verdict:** ✅ **PASS** — no PR exceeds the frozen baseline false-positive count. - -### Cold start vs the 30s P95 SLA - -- **P50:** 7.0s · **P95:** 11.0s · **max:** 11.0s over 6 sample(s) — ✅ within SLA (SLA 30s) - -### Coverage & skipped / degraded runs - -| Corpus PR | Status | -|---|---| -| `petry-projects/.github-private#701@0000000000000000000000000000000000000701` | COVERED | -| `petry-projects/.github-private#702@0000000000000000000000000000000000000702` | COVERED | -| `petry-projects/.github-private#703@0000000000000000000000000000000000000703` | COVERED | -| `petry-projects/.github-private#704@0000000000000000000000000000000000000704` | COVERED | -| `petry-projects/.github-private#705@0000000000000000000000000000000000000705` | COVERED | -| `petry-projects/.github-private#706@0000000000000000000000000000000000000706` | COVERED | - -- No SLA auto-skip or MCP degradation on this candidate. - -### Success-metric verdict - -- GO — token reduction 2.56x ≥ 2.0x target; no quality regression (false positives ≤ baseline); cold-start P95 11.0s within the 30s SLA - -## Candidate: `serena` - -# 🔬 LSP pilot comparison — `serena` vs frozen LSP-off baseline - -_Speed (cold-start, wall-time) · cost (nav tokens, tool calls, ET, USD) · quality (findings, false positives), LSP-on vs the immutable LSP-off control._ - -**Quality proxy:** a PR regresses when its **false-positive** count exceeds the frozen baseline's (precision dropped). A navigation-token win that costs precision is a no-go. - -## Per PR (off → on) - -| PR | Nav tokens | Nav reduction | Tool calls | Cold start | Wall time | ET | USD | Findings | False positives | Quality | -|---|---|---:|---|---:|---|---:|---:|---|---:|:--| -| `petry-projects/.github-private#701@0000000000000000000000000000000000000701` | 21,800 → 21,800 | 1.0x | 34 → 34 | N/A → 34.0s | 188.0s → 188.0s | 283,000 → 283,000 | $0.30 → $0.30 | 4 → 4 | 1 → 1 | ok | -| `petry-projects/.github-private#702@0000000000000000000000000000000000000702` | 15,400 → 7,000 | 2.2x | 26 → 12 | N/A → 14.0s | 151.0s → 95.0s | 242,000 → 159,500 | $0.26 → $0.17 | 3 → 3 | 1 → 1 | ok | -| `petry-projects/.github-private#703@0000000000000000000000000000000000000703` | 9,800 → 5,000 | 2.0x | 18 → 9 | N/A → 13.0s | 121.0s → 80.0s | 192,000 → 128,000 | $0.20 → $0.14 | 2 → 2 | 0 → 0 | ok | -| `petry-projects/.github-private#704@0000000000000000000000000000000000000704` | 7,600 → 7,600 | 1.0x | 14 → 14 | N/A → N/A | 104.0s → 104.0s | 162,500 → 162,500 | $0.17 → $0.17 | 2 → 2 | 0 → 0 | ok | -| `petry-projects/.github-private#705@0000000000000000000000000000000000000705` | 29,600 → 13,000 | 2.3x | 48 → 18 | N/A → 15.0s | 233.0s → 140.0s | 345,000 → 239,000 | $0.36 → $0.26 | 5 → 5 | 2 → 2 | ok | -| `petry-projects/.github-private#706@0000000000000000000000000000000000000706` | 1,200 → 1,200 | 1.0x | 3 → 3 | N/A → 9.0s | 58.0s → 50.0s | 91,500 → 91,500 | $0.10 → $0.10 | 1 → 1 | 0 → 0 | ok | - -## Aggregate - -- **Navigation tool-call tokens:** 85,400 (off) → 55,600 (on) — **1.5x** reduction -- **ET:** 1,316,000 (off) → 1,063,500 (on) -- **USD:** $1.39 (off) → $1.14 (on) -- **Mean LSP-on cold start:** 17.0s (baseline N/A — no server) -- **Findings:** 17 (off) → 17 (on) -- **False positives:** 4 (off) → 4 (on) -- **Quality verdict:** ✅ **PASS** — no PR exceeds the frozen baseline false-positive count. - -### Cold start vs the 30s P95 SLA - -- **P50:** 14.0s · **P95:** 34.0s · **max:** 34.0s over 5 sample(s) — ❌ SLA breach (SLA 30s) - -### Coverage & skipped / degraded runs - -| Corpus PR | Status | -|---|---| -| `petry-projects/.github-private#701@0000000000000000000000000000000000000701` | SKIPPED:sla-skip: cold-start 34.0s exceeded 30s P95 SLA — LSP step auto-skipped; review continued on base navigation | -| `petry-projects/.github-private#702@0000000000000000000000000000000000000702` | COVERED | -| `petry-projects/.github-private#703@0000000000000000000000000000000000000703` | COVERED | -| `petry-projects/.github-private#704@0000000000000000000000000000000000000704` | SKIPPED:mcp-degraded: MCP server init failed (degraded) — review continued on base capabilities | -| `petry-projects/.github-private#705@0000000000000000000000000000000000000705` | COVERED | -| `petry-projects/.github-private#706@0000000000000000000000000000000000000706` | COVERED | - -- Annotated skipped / degraded runs (not dropped): - - `petry-projects/.github-private#701@0000000000000000000000000000000000000701` — **sla-skip**: cold-start 34.0s exceeded 30s P95 SLA — LSP step auto-skipped; review continued on base navigation - - `petry-projects/.github-private#704@0000000000000000000000000000000000000704` — **mcp-degraded**: MCP server init failed (degraded) — review continued on base capabilities - -### Success-metric verdict - -- NO-GO — token reduction 1.54x below 2.0x target; no quality regression (false positives ≤ baseline); cold-start P95 34.0s breaches the 30s SLA - -## Cross-candidate summary - -| Candidate | Nav reduction | Cold-start P95 | Verdict | -|---|---:|---:|:--| -| `agent-lsp` | 2.56x | 11.0s | GO | -| `serena` | 1.54x | 34.0s | NO-GO | - -- **agent-lsp** is the only candidate meeting the success metric; no head-to-head pick is needed. diff --git a/docs/lsp-pilot.md b/docs/lsp-pilot.md deleted file mode 100644 index 5f9ee435b..000000000 --- a/docs/lsp-pilot.md +++ /dev/null @@ -1,257 +0,0 @@ -# LSP Pilot — Scoping Doc - -**Status:** Contract — the single agreed definition of done that every later story in -epic [#839](https://github.com/petry-projects/.github-private/issues/839) grounds in. -**Author:** dev-lead / Claude Code -**Date:** 2026-06-20 -**Story:** [#840](https://github.com/petry-projects/.github-private/issues/840) (Phase 1) -**Scope (confirmed from discussion [#578](https://github.com/petry-projects/.github-private/discussions/578)):** -Pilot Language Server Protocol (LSP) code intelligence as MCP tools in the **pr-review** -agent, **Shell** language, **this repo**, scoped to **finding-verification** in the -deep/audit tiers — measured against a frozen baseline for a human go/no-go. - -This doc is the contract, not a vendor survey: it fixes the pilot's language, the specific -agent actions it covers, the cold-start SLA, the success metric, the cost cap, and the -candidate-server shortlist so the downstream wiring and measurement stories -([#841](https://github.com/petry-projects/.github-private/issues/841)–[#846](https://github.com/petry-projects/.github-private/issues/846)) -share one unambiguous scope instead of each re-deciding it. It **reuses**, and does not -re-describe, the MCP mechanism that already shipped (epic -[#676](https://github.com/petry-projects/.github-private/issues/676); see §6). - ---- - -## 1. Scope — language, repo, tier - -| Dimension | Pilot decision | -|---|---| -| **Target language** | **Shell** (bash) | -| **Pilot repo** | **`.github-private`** (this repo) | -| **Agent surface** | **pr-review** only | -| **Review tiers** | **deep / audit** only (the agentic tiers); triage is never touched | - -**Rationale (tied to the discussion).** Shell in `.github-private` is the agent's own -substrate — the harness's biggest "language" is itself (`scripts/engine.sh`, -`scripts/review-batch.sh`, the audit scripts), so the pilot reviews code it already owns -with no cross-repo coordination. `bash-language-server` is lightweight (Tree-sitter based, -near-instant index for a shell-only repo), which keeps cold-start off the critical path — -the make-or-break operational risk for LSP on ephemeral CI runners. pr-review -finding-verification is the lowest-risk first surface: a bad verification only downgrades a -finding, whereas a bad code write breaks a build (so dev-lead code-writing is deferred — -see §2). - ---- - -## 2. In-scope vs out-of-scope agent actions - -**In scope — finding-verification in the deep/audit tiers.** LSP is used as a -*grounding/verification* step: before the reviewer posts a cross-file claim, it confirms the -claim against real semantic context via: - -- **find-references** (`textDocument/references`) — confirm "this breaks N callers" / - "this symbol is unused" against the actual reference set, not a textual `grep` match. -- **diagnostics** (`textDocument/publishDiagnostics`) — confirm "this is undefined / this is - a type or syntax error" against the language server's own diagnostics. - -This directly attacks the false-positive problem (textual `grep` matches a name in a string -or comment; LSP resolves the *semantic* reference), which is what erodes trust in an -auto-approving reviewer. - -**Explicitly OUT of scope for this pilot:** - -- **The triage tier.** Triage stays fast and restricted; it never receives MCP/LSP tools - (consistent with the existing per-tier gating — see §6). -- **dev-lead code-writing.** LSP-assisted code *authoring/fixing* (safe `rename`, - `definition`-on-first-pass, pre-push `diagnostics`) is **not** part of this pilot. It is - considered only after a positive go/no-go on the pr-review surface (epic Phase 3, - [#845](https://github.com/petry-projects/.github-private/issues/845)). - ---- - -## 3. Cold-start SLA & graceful degradation - -**SLA:** LSP cold-start (server launch + index to query-ready) must complete in -**<=30s P95** on the pilot repo. - -**Auto-skip rule (graceful degradation).** If cold-start exceeds the SLA, the LSP step is -**automatically skipped — never a workflow failure**. The review continues on the model's -base capabilities (and the existing `grep`/read navigation) and the run's verdict and exit -code are untouched. Exceeding the SLA degrades the *enrichment*, not the *review*. - -This is an **addition layered on** the MCP graceful-degradation path that already exists in -`scripts/engine.sh` (`_emit_mcp_failure_warning` emits a single `::warning::[mcp]` -annotation on a server connect/init failure and leaves the verdict and exit code intact — see -§6 and `docs/initiatives/mcp-powered-review.md` §4), not a replacement for it. The SLA -auto-skip is the same "Fail Loud, Never Fake" contract applied to the new cold-start budget: -skip visibly, continue, never abort. Cold-start measurement and the SLA enforcement / -index-caching live in story -[#846](https://github.com/petry-projects/.github-private/issues/846); this doc only fixes the -SLA number and the auto-skip policy. - ---- - -## 4. Success metric & cost cap - -These are transcribed verbatim from the epic -([#839](https://github.com/petry-projects/.github-private/issues/839)) body so the doc and -the epic stay one source of truth. If the epic changes, update this section to match. - -### Initiative success metric - -> Go = on the **frozen pilot PR corpus**, LSP-on deep-tier review delivers a **measurable -> navigation-token reduction (target >=2x fewer navigation tool-call tokens vs the LSP-off -> control)** AND **no regression in review quality** (false-positive / precision **no worse -> than the frozen LSP-off baseline**), achieved **within the cold-start SLA (<=30s P95)**. A -> win on tokens that costs precision is a *no-go*, not a win. - -The frozen corpus and the immutable LSP-off baseline are established in story -[#841](https://github.com/petry-projects/.github-private/issues/841). - -### Cost cap (explicit) - -> - LSP is gated to **deep/audit tiers only** (triage untouched); cold-start is -> **auto-skipped above the 30s P95 SLA** (graceful degradation, never a workflow failure). -> - The pilot is run-count bounded: **<=20 PRs in the corpus x <=2 candidate servers x <=3 -> runs each ~= <=120 deep-tier review runs total**, under the existing 60-min job cap. No -> fleet-wide rollout, and no dev-lead (code-writing) LSP, until the go/no-go decision -> record lands. - -The cost cap is structural (run-count and tier-gating), not a dollar ceiling. - ---- - -## 5. Candidate LSP-MCP servers & selection rule - -Three candidate LSP-MCP servers are compared in the pilot. **Final selection is decided by -the comparative pilot ([#844](https://github.com/petry-projects/.github-private/issues/844)) -on the frozen corpus — it is NOT pre-chosen here.** Per -[@don-petry](https://github.com/petry-projects/.github-private/discussions/578) the choice is -made from measured **speed, quality, and cost** with and without LSP, not from the vendor -narrative. - -| Candidate | Distribution | Notes (from research, [#578](https://github.com/petry-projects/.github-private/discussions/578)) | -|---|---|---| -| **agent-lsp** ([blackwell-systems/agent-lsp](https://github.com/blackwell-systems/agent-lsp)) | Single Go binary (pin a GitHub release tag) | Persistent daemon (~10s first index), `--non-interactive` + `--http` CI modes, GCF token compression, 30-language CI matrix. CI-first design. | -| **Serena** ([oraios/serena](https://github.com/oraios/serena)) | Python wheel via `uv` (pin an exact PyPI version) | Has the production benchmark (Project AEGIS); use its results as the bar to beat. | -| **lsp-mcp** ([owner/lsp-mcp](https://github.com/owner/lsp-mcp)) | Go binary (pin a GitHub release tag) | ~24 tools, CI-verified across several languages. | - -All three drive the same underlying Shell language server, **`bash-language-server`** (npm), -which is **also pinned** to an exact version for the pilot. - -### Version-pinning rule - -Each candidate (and `bash-language-server`) is a **third-party, non-org-owned dependency** -and **must be pinned to an exact version for a reproducible pilot** (epic untracked -prerequisite). The exact pins are **locked when the server is wired** (story -[#842](https://github.com/petry-projects/.github-private/issues/842)) and recorded back into -the table below — they are not invented in this scoping doc, consistent with the repo rule -that pins (SHAs / release tags) are looked up from the source of truth, never guessed -([CLAUDE.md](../CLAUDE.md), [AGENTS.md](../AGENTS.md)). - -| Dependency | Source of truth | Pinned version | Locked in | -|---|---|---|---| -| agent-lsp | GitHub releases — `blackwell-systems/agent-lsp` | **`v0.15.0`** | #842 | -| Serena | PyPI `serena-agent` / `oraios/serena` releases | **`1.5.3`** | #842 | -| lsp-mcp | GitHub releases — `owner/lsp-mcp` | *unresolved — repo not locatable; not wired* | #842 | -| bash-language-server | npm `bash-language-server` | **`5.6.0`** | #842 | - -Pins were looked up from each source of truth when the server was wired (#842), not invented -here. `owner/lsp-mcp` could not be resolved to a real repository, so it is **not wired**; the -run-count cost cap in §4 bounds the comparison to **<=2 candidate servers**, so the two -resolvable candidates (**agent-lsp**, **Serena**) cover the pilot. **agent-lsp** is the wired -default candidate (`scripts/setup-lsp-pilot.sh`): it is a single CI-first Go binary, which keeps -cold-start off the critical path on ephemeral runners (§3). Only the candidate that wins the -comparison (#844) is carried past Phase 2. - -### Wiring (story #842) — config, knobs, install - -The pilot rides the existing MCP knob (§6); #842 adds three committed pieces, all **inert until -explicitly enabled**: - -- **Config** — [`.github/mcp/lsp.json`](../.github/mcp/lsp.json) declares the candidate's LSP-MCP - server as a stdio binary (server name `lsp`) driving `bash-language-server`. To switch - candidates, point `REVIEW_MCP_CONFIG` at a different committed config (and set `LSP_CANDIDATE` - so the installer fetches the matching binary). This file is **not** the engine's conventional - auto-default path (`.github/review-mcp.json`), so merely committing it activates nothing - (byte-for-byte unchanged — the success criterion in §4 / story AC #4). - -- **Knobs** (set by the installer when the pilot is on): - - | Env var | Pilot value | - |---|---| - | `REVIEW_MCP_CONFIG` | `.github/mcp/lsp.json` | - | `REVIEW_MCP_ALLOWED_TOOLS` | the navigation allowlist below | - - With both set, `engine.sh` appends `--mcp-config .github/mcp/lsp.json --strict-mcp-config` and - merges the navigation tools into the deep/audit + rubber-duck `--allowed-tools` base - (`Bash,Read,Grep,Glob`). **Triage is never touched.** - -- **Navigation allowlist (AC #3).** Only a small read-only navigation set is exposed (target - 8–12 tools) to bound MCP tool-definition token overhead (~200 tokens/tool/turn). This is the - finding-verification surface from §2 (references + diagnostics) plus close navigation kin - (definition/hover/symbols) — **no** editing/refactoring tools (rename, apply_edit, insert, - delete, format, execute_command). The canonical list is the single source of truth in - `scripts/setup-lsp-pilot.sh` (`scripts/setup-lsp-pilot.sh print-allowed-tools`): - `mcp__lsp__find_references`, `mcp__lsp__go_to_definition`, `mcp__lsp__go_to_type_definition`, - `mcp__lsp__go_to_implementation`, `mcp__lsp__go_to_declaration`, `mcp__lsp__get_diagnostics`, - `mcp__lsp__inspect_symbol`, `mcp__lsp__list_symbols`, `mcp__lsp__find_symbol`, - `mcp__lsp__explore_symbol` (10 tools). - -- **Install (AC #2).** [`scripts/setup-lsp-pilot.sh`](../scripts/setup-lsp-pilot.sh) installs the - pinned candidate binary (verifying the release tarball's sha256 against `checksums.txt`) and the - pinned `bash-language-server`, then threads the knobs above into `$GITHUB_ENV`. It is gated in - `.github/workflows/pr-review.yml` behind the repo variable `LSP_PILOT_ENABLED == 'true'`. If a - pinned tool cannot be installed it emits a single `::warning::` and skips wiring the knobs — the - review continues on base capabilities and **the workflow never fails** (the same "Fail Loud, - Never Fake" contract as §3). - ---- - -## 6. How this rides the existing MCP plumbing (reuse, do not rebuild) - -LSP is **one more MCP server behind the knob that already shipped** — the pilot does not -rebuild the gating, the graceful-degradation path, or the Token Cost Observatory. - -- **Per-tier gating + config threading.** `scripts/engine.sh` already threads - `--mcp-config --strict-mcp-config` and the merged `--allowed-tools` into the - agentic tiers (`run_agentic` — deep/audit) and the rubber-duck tier (`run_duck`) via - `REVIEW_MCP_CONFIG` / `REVIEW_MCP_ALLOWED_TOOLS`; the fast triage tier is intentionally - excluded. This pilot scopes LSP *use* to the deep/audit finding-verification surface (§2). - The LSP server is added as another entry in the same MCP config and its tools as another - `REVIEW_MCP_ALLOWED_TOOLS` glob. See - [`docs/initiatives/mcp-powered-review.md`](./initiatives/mcp-powered-review.md) §3 for the - knob and [`.github/review-mcp.json`](../.github/review-mcp.json) for the live MCP config - (Context7 today). -- **Graceful degradation.** The MCP "Fail Loud, Never Fake" path - (`_emit_mcp_failure_warning` in `scripts/engine.sh`) already turns an unavailable server - into a single `::warning::[mcp]` annotation while the review completes on base - capabilities. The §3 SLA auto-skip is layered on top of this, not a parallel mechanism. - See [`docs/initiatives/mcp-powered-review.md`](./initiatives/mcp-powered-review.md) §4. -- **What is genuinely new in this initiative:** (a) the cold-start SLA + index-caching + - auto-skip on the LSP server's launch budget (#846), and (b) the finding-verification step - that calls find-references / diagnostics before posting a cross-file finding (#843 — - shipped: the deep/audit prompts ask the model to annotate each grounded finding with - `lsp_verification`, and [`scripts/lib/lsp-verification.sh`](../scripts/lib/lsp-verification.sh) - enforces it — downgrading + annotating `unverifiable` findings and emitting each outcome to - the Token Cost Observatory JSONL. It is inert when LSP is unwired or degraded). - ---- - -## 7. References - -- Epic [#839](https://github.com/petry-projects/.github-private/issues/839) — LSP pilot; - stories #840 (this doc), #841 (corpus + baseline), #842 (wire server), #843 - (finding-verification step), #844 (run comparison), #845 (go/no-go), #846 (cache + SLA). -- [`docs/lsp-pilot-report.md`](./lsp-pilot-report.md) — the rendered comparative - speed/quality/cost report (#844), produced by - [`scripts/lsp_pilot_report.sh`](../scripts/lsp_pilot_report.sh) from the frozen - baseline + the candidate runs under `evals/lsp-pilot/runs/`. -- Discussion [#578](https://github.com/petry-projects/.github-private/discussions/578) — - idea body, the LSP-MCP server-selection weekly update, and the enhancement comment whose - suggested `docs/lsp-pilot.md` this mirrors. -- [`docs/initiatives/mcp-powered-review.md`](./initiatives/mcp-powered-review.md) — the MCP - knob (§3) and graceful degradation (§4) this pilot reuses. -- [`scripts/engine.sh`](../scripts/engine.sh) — `REVIEW_MCP_CONFIG` / - `REVIEW_MCP_ALLOWED_TOOLS` threading and `_emit_mcp_failure_warning`. -- [`.github/review-mcp.json`](../.github/review-mcp.json) — live MCP config. -- [`.github/workflows/pr-review.yml`](../.github/workflows/pr-review.yml) — the review - workflow that invokes the engine. diff --git a/evals/lsp-pilot/README.md b/evals/lsp-pilot/README.md deleted file mode 100644 index 536d0b566..000000000 --- a/evals/lsp-pilot/README.md +++ /dev/null @@ -1,90 +0,0 @@ -# `evals/lsp-pilot/` — frozen comparative corpus + immutable LSP-off baseline - -This held-out set backs the LSP pilot (epic -[#839](https://github.com/petry-projects/.github-private/issues/839), scoping doc -[`docs/lsp-pilot.md`](../../docs/lsp-pilot.md)). It exists so candidate LSP-MCP -servers are compared against a **fixed target that cannot be tuned to** — the -go/no-go must reflect real generalization, not overfitting to a few PRs. - -It reuses the standard held-out machinery (see [`../README.md`](../README.md)): the -`dev/` (proposer-visible) vs `holdout/` (gate-scored) split, the directory -validator (`evals/validate-cases.py`), and the hard immutability gate -(`scripts/lib/holdout-guard.sh` + `.github/workflows/holdout-guard.yml`, keyed on -`HOLDOUT_GUARDED_PREFIXES` whose default `evals/` already covers this tree). No new -immutability mechanism is invented here. - -## Layout - -``` -evals/lsp-pilot/ - dev/cases.jsonl # proposer-visible smoke PRs - holdout/cases.jsonl # frozen scored corpus (the comparison target) - holdout/baseline-lsp-off.jsonl # immutable LSP-off control, captured ONCE - runs/.jsonl # captured LSP-on candidate runs (story #844 inputs) -``` - -`runs/` is not a held-out split — it is not validated by `evals/validate-cases.py` -(which only walks `dev/`/`holdout/`) and holds **candidate run captures**, one JSONL -per candidate LSP server, in the comparison-harness record shape (see -[`../../scripts/lsp_pilot_compare.sh`](../../scripts/lsp_pilot_compare.sh)). They are -the LSP-on inputs the comparative report -([`../../scripts/lsp_pilot_report.sh`](../../scripts/lsp_pilot_report.sh) → -[`../../docs/lsp-pilot-report.md`](../../docs/lsp-pilot-report.md)) scores against the -frozen baseline. A run may carry two optional honesty fields — `lsp_skipped` (the SLA -auto-skip fired) and `mcp_degraded` (the MCP server ran degraded), each with a -`skip_reason` — so a skipped/degraded run is reported, never silently dropped. Like the -corpus and baseline, the committed runs are **synthetic, de-identified seeds** until the -real pilot PR set is wired. - -## The frozen corpus (`cases.jsonl`) - -Each case pins one pilot PR by **immutable identifiers** — `repo` + `pr_number` + -`head_sha` — plus a unique `id` and a `description` of the navigation claim it -exercises (find-references or diagnostics, per the scoping doc §2). Pinning the -head SHA freezes *exactly which revision* is reviewed, so the corpus is a moving -target for nobody. - -> The identifiers committed here are **synthetic, de-identified seeds** (note the -> zero-padded `head_sha`s), consistent with the de-identification requirement in -> [`../README.md`](../README.md). They are replaced with the real pinned pilot PR -> set when the server is wired and the controls are run (story -> [#842](https://github.com/petry-projects/.github-private/issues/842)). The -> structure, schema, and immutability guarantee are what this story freezes. - -## The immutable LSP-off baseline (`baseline-lsp-off.jsonl`) - -The LSP-off control is captured **once** and committed as a frozen artifact, so -later stories compare against it rather than re-deriving it (a re-derived baseline -is a moving target — a reward-hacking vector). One JSON object per corpus PR: - -| field | meaning | -|---|---| -| `pr` | `repo#pr_number@head_sha` — the join key against a candidate run | -| `variant` / `candidate` | `lsp-off` / `baseline` | -| `nav_tokens` | navigation tool-call tokens (the headline cost metric) | -| `tool_calls` | navigation tool-call count | -| `findings` / `false_positives` | the review-quality proxy (see below) | -| `cold_start_s` | `null` — **N/A** for the LSP-off control (no server to launch) | -| `wall_time_s`, `model`, `*_tokens` | speed + cost inputs (ET/USD) | - -## The review-quality proxy (explicit, not ad hoc) - -Quality is `(findings, false_positives)` per PR. A candidate is a **quality -regression on a PR when its `false_positives` exceeds the frozen baseline's** — -i.e. precision got worse. This is the same definition the harness -([`scripts/lsp_pilot_compare.sh`](../../scripts/lsp_pilot_compare.sh)) renders and -enforces, so the go/no-go is reproducible. Per the success metric, a navigation- -token win that costs precision is a **no-go**, so this metric is mandatory, not -optional. - -## Comparing a candidate against the baseline - -```bash -bash scripts/lsp_pilot_compare.sh \ - evals/lsp-pilot/holdout/baseline-lsp-off.jsonl \ - .jsonl -``` - -The harness renders per-PR and aggregate speed/cost/quality deltas and **exits -non-zero if any candidate PR has no baseline counterpart** — a partial corpus can -never masquerade as a clean comparison. diff --git a/evals/lsp-pilot/dev/cases.jsonl b/evals/lsp-pilot/dev/cases.jsonl deleted file mode 100644 index f85ce551c..000000000 --- a/evals/lsp-pilot/dev/cases.jsonl +++ /dev/null @@ -1,2 +0,0 @@ -{"id":"lsp-dev-smoke-find-references","repo":"petry-projects/.github-private","pr_number":707,"head_sha":"0000000000000000000000000000000000000707","description":"Proposer-visible smoke case: a small cross-file shell change exercising find-references navigation end-to-end. Visible by design; never scored by the gate.","tags":["find-references","smoke","shell"]} -{"id":"lsp-dev-smoke-diagnostic","repo":"petry-projects/.github-private","pr_number":708,"head_sha":"0000000000000000000000000000000000000708","description":"Proposer-visible smoke case: a single-file shell change with a diagnostics-detectable issue, to smoke-test the publishDiagnostics path. Never scored by the gate.","tags":["diagnostics","smoke","shell"]} diff --git a/evals/lsp-pilot/holdout/baseline-lsp-off.jsonl b/evals/lsp-pilot/holdout/baseline-lsp-off.jsonl deleted file mode 100644 index 6d9b41c3f..000000000 --- a/evals/lsp-pilot/holdout/baseline-lsp-off.jsonl +++ /dev/null @@ -1,6 +0,0 @@ -{"pr":"petry-projects/.github-private#701@0000000000000000000000000000000000000701","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":42000,"cache_read_tokens":18000,"output_tokens":3200,"nav_tokens":21800,"tool_calls":34,"findings":4,"false_positives":1,"cold_start_s":null,"wall_time_s":188.0} -{"pr":"petry-projects/.github-private#702@0000000000000000000000000000000000000702","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":36500,"cache_read_tokens":15000,"output_tokens":2600,"nav_tokens":15400,"tool_calls":26,"findings":3,"false_positives":1,"cold_start_s":null,"wall_time_s":151.0} -{"pr":"petry-projects/.github-private#703@0000000000000000000000000000000000000703","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":28800,"cache_read_tokens":12000,"output_tokens":2100,"nav_tokens":9800,"tool_calls":18,"findings":2,"false_positives":0,"cold_start_s":null,"wall_time_s":121.0} -{"pr":"petry-projects/.github-private#704@0000000000000000000000000000000000000704","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":24300,"cache_read_tokens":10000,"output_tokens":1800,"nav_tokens":7600,"tool_calls":14,"findings":2,"false_positives":0,"cold_start_s":null,"wall_time_s":104.0} -{"pr":"petry-projects/.github-private#705@0000000000000000000000000000000000000705","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":51200,"cache_read_tokens":22000,"output_tokens":3900,"nav_tokens":29600,"tool_calls":48,"findings":5,"false_positives":2,"cold_start_s":null,"wall_time_s":233.0} -{"pr":"petry-projects/.github-private#706@0000000000000000000000000000000000000706","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":14100,"cache_read_tokens":6000,"output_tokens":900,"nav_tokens":1200,"tool_calls":3,"findings":1,"false_positives":0,"cold_start_s":null,"wall_time_s":58.0} diff --git a/evals/lsp-pilot/holdout/cases.jsonl b/evals/lsp-pilot/holdout/cases.jsonl deleted file mode 100644 index e121e5c74..000000000 --- a/evals/lsp-pilot/holdout/cases.jsonl +++ /dev/null @@ -1,6 +0,0 @@ -{"id":"lsp-corpus-find-references-callers","repo":"petry-projects/.github-private","pr_number":701,"head_sha":"0000000000000000000000000000000000000701","description":"Cross-file change to a shell helper whose review must claim 'this breaks N callers'; find-references is the navigation that LSP replaces a recursive grep with.","tags":["find-references","cross-file","shell"]} -{"id":"lsp-corpus-unused-symbol","repo":"petry-projects/.github-private","pr_number":702,"head_sha":"0000000000000000000000000000000000000702","description":"Removes a shell function the diff says is unused; the LSP-off control resolves 'unused' by textual grep, the kind of claim find-references verifies semantically.","tags":["find-references","unused-symbol","shell"]} -{"id":"lsp-corpus-undefined-var-diagnostic","repo":"petry-projects/.github-private","pr_number":703,"head_sha":"0000000000000000000000000000000000000703","description":"Introduces a reference to an undefined variable under set -u; the reviewer's 'this is undefined' claim is what textDocument/publishDiagnostics confirms.","tags":["diagnostics","undefined","shell"]} -{"id":"lsp-corpus-syntax-error-diagnostic","repo":"petry-projects/.github-private","pr_number":704,"head_sha":"0000000000000000000000000000000000000704","description":"Adds a shell snippet with a real syntax error; a finding-verification step asks the language server's diagnostics rather than eyeballing the diff.","tags":["diagnostics","syntax-error","shell"]} -{"id":"lsp-corpus-cross-file-rename-claim","repo":"petry-projects/.github-private","pr_number":705,"head_sha":"0000000000000000000000000000000000000705","description":"Renames an exported function across several scripts; the review must confirm every reference moved, a high navigation-token claim under grep.","tags":["find-references","rename","cross-file","shell"]} -{"id":"lsp-corpus-clean-no-nav-needed","repo":"petry-projects/.github-private","pr_number":706,"head_sha":"0000000000000000000000000000000000000706","description":"A self-contained docs/comment-only change that needs no semantic navigation; pins a low-navigation control so a token win is not an artefact of skipped work.","tags":["no-navigation","control","docs"]} diff --git a/evals/lsp-pilot/runs/agent-lsp.jsonl b/evals/lsp-pilot/runs/agent-lsp.jsonl deleted file mode 100644 index fbfafd67c..000000000 --- a/evals/lsp-pilot/runs/agent-lsp.jsonl +++ /dev/null @@ -1,6 +0,0 @@ -{"pr":"petry-projects/.github-private#701@0000000000000000000000000000000000000701","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":24000,"cache_read_tokens":18000,"output_tokens":3200,"nav_tokens":8000,"tool_calls":12,"findings":4,"false_positives":1,"cold_start_s":9.0,"wall_time_s":95.0} -{"pr":"petry-projects/.github-private#702@0000000000000000000000000000000000000702","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":19000,"cache_read_tokens":15000,"output_tokens":2600,"nav_tokens":6000,"tool_calls":11,"findings":3,"false_positives":1,"cold_start_s":8.0,"wall_time_s":78.0} -{"pr":"petry-projects/.github-private#703@0000000000000000000000000000000000000703","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":15000,"cache_read_tokens":12000,"output_tokens":2100,"nav_tokens":4000,"tool_calls":8,"findings":2,"false_positives":0,"cold_start_s":7.0,"wall_time_s":64.0} -{"pr":"petry-projects/.github-private#704@0000000000000000000000000000000000000704","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":13000,"cache_read_tokens":10000,"output_tokens":1800,"nav_tokens":3200,"tool_calls":7,"findings":2,"false_positives":0,"cold_start_s":7.0,"wall_time_s":55.0} -{"pr":"petry-projects/.github-private#705@0000000000000000000000000000000000000705","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":27000,"cache_read_tokens":22000,"output_tokens":3900,"nav_tokens":11000,"tool_calls":16,"findings":5,"false_positives":2,"cold_start_s":11.0,"wall_time_s":120.0} -{"pr":"petry-projects/.github-private#706@0000000000000000000000000000000000000706","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":14100,"cache_read_tokens":6000,"output_tokens":900,"nav_tokens":1200,"tool_calls":3,"findings":1,"false_positives":0,"cold_start_s":6.0,"wall_time_s":40.0} diff --git a/evals/lsp-pilot/runs/serena.jsonl b/evals/lsp-pilot/runs/serena.jsonl deleted file mode 100644 index 8fc37e5fc..000000000 --- a/evals/lsp-pilot/runs/serena.jsonl +++ /dev/null @@ -1,6 +0,0 @@ -{"pr":"petry-projects/.github-private#701@0000000000000000000000000000000000000701","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":42000,"cache_read_tokens":18000,"output_tokens":3200,"nav_tokens":21800,"tool_calls":34,"findings":4,"false_positives":1,"cold_start_s":34.0,"wall_time_s":188.0,"lsp_skipped":true,"skip_reason":"cold-start 34.0s exceeded 30s P95 SLA — LSP step auto-skipped; review continued on base navigation"} -{"pr":"petry-projects/.github-private#702@0000000000000000000000000000000000000702","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":20000,"cache_read_tokens":15000,"output_tokens":2600,"nav_tokens":7000,"tool_calls":12,"findings":3,"false_positives":1,"cold_start_s":14.0,"wall_time_s":95.0} -{"pr":"petry-projects/.github-private#703@0000000000000000000000000000000000000703","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":16000,"cache_read_tokens":12000,"output_tokens":2100,"nav_tokens":5000,"tool_calls":9,"findings":2,"false_positives":0,"cold_start_s":13.0,"wall_time_s":80.0} -{"pr":"petry-projects/.github-private#704@0000000000000000000000000000000000000704","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":24300,"cache_read_tokens":10000,"output_tokens":1800,"nav_tokens":7600,"tool_calls":14,"findings":2,"false_positives":0,"cold_start_s":null,"wall_time_s":104.0,"mcp_degraded":true,"skip_reason":"MCP server init failed (degraded) — review continued on base capabilities"} -{"pr":"petry-projects/.github-private#705@0000000000000000000000000000000000000705","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":30000,"cache_read_tokens":22000,"output_tokens":3900,"nav_tokens":13000,"tool_calls":18,"findings":5,"false_positives":2,"cold_start_s":15.0,"wall_time_s":140.0} -{"pr":"petry-projects/.github-private#706@0000000000000000000000000000000000000706","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":14100,"cache_read_tokens":6000,"output_tokens":900,"nav_tokens":1200,"tool_calls":3,"findings":1,"false_positives":0,"cold_start_s":9.0,"wall_time_s":50.0} diff --git a/prompts/deep-review.md b/prompts/deep-review.md index a9396b738..eed9f0994 100644 --- a/prompts/deep-review.md +++ b/prompts/deep-review.md @@ -80,26 +80,6 @@ enumeration. No actions on other PRs. review because MCP was unavailable, and never fabricate a scan result. 6. Fetch linked issues if any. 7. Check `statusCheckRollup` for CI status. -8. **LSP finding-verification (only when the `mcp__lsp__*` navigation tools are - available).** These tools are exposed only when the LSP pilot is enabled; if - they are not present, skip this step entirely and review as usual. When they - are present, before you report any finding that makes a **cross-file or - semantic claim** — e.g. "X is undefined", "this breaks N callers", "this - symbol is unused", "this is a type/syntax error" — ground the claim against - real semantic context instead of a textual `grep` match: - - use `mcp__lsp__find_references` to confirm a "breaks N callers" / "unused - symbol" claim against the actual reference set; - - use `mcp__lsp__get_diagnostics` to confirm an "undefined" / "type or syntax - error" claim against the language server's own diagnostics. - Then annotate that finding with an `"lsp_verification"` field: - - `"verified"` — LSP confirmed the claim; - - `"unverifiable"` — LSP could not ground the claim. **Do not drop it and do - not post it as confident**: lower its `severity` one level; the - verification step also tags it `[lsp: unverifiable]` so the outcome is - auditable. - Findings that make no cross-file/semantic claim (style, docs, etc.) need no - `lsp_verification` field. Never fail or block a review because an LSP tool was - unavailable, and never fabricate a verification result. ## Semantic safety checks (issue #305 — the LLM-only half) @@ -181,14 +161,10 @@ Write a JSON object to `$OUTPUT_FILE`: "category": "...", "message": "...", "file": "path or null", - "line": "number or null", - "lsp_verification": "verified|unverifiable (OMIT unless step 8 applied)" + "line": "number or null" } ] } ``` -Include `lsp_verification` **only** on a finding you grounded via the LSP tools -in step 8 (`verified` or `unverifiable`); omit it otherwise. - Write with `cat > "$OUTPUT_FILE" <<'JSON' ... JSON`. Ensure it parses with `jq`. diff --git a/prompts/security-audit.md b/prompts/security-audit.md index 15a659360..48fbba4f2 100644 --- a/prompts/security-audit.md +++ b/prompts/security-audit.md @@ -34,21 +34,6 @@ reviewer, called only for PRs with real concerns. 4. Fetch linked issues if any. 5. Read any CONTRIBUTING.md, AGENTS.md, CODEOWNERS in the repo to check standards compliance (fetch via `gh api`). -6. **LSP finding-verification (only when the `mcp__lsp__*` navigation tools are - available).** These tools are exposed only when the LSP pilot is enabled; if - they are not present, skip this step entirely and audit as usual. When they - are present, before you report any finding that makes a **cross-file or - semantic claim** — e.g. "X is undefined", "this breaks N callers", "this - symbol is unused", "this is a type/syntax error" — ground it against real - semantic context with `mcp__lsp__find_references` (reference set) or - `mcp__lsp__get_diagnostics` (language-server diagnostics) rather than a textual - `grep` match. Annotate that finding with an `"lsp_verification"` field: - `"verified"` if LSP confirmed it, or `"unverifiable"` if LSP could not ground - it — and for `unverifiable` lower its `severity` one level rather than dropping - it (the verification step also tags it `[lsp: unverifiable]` so the outcome is - auditable). Findings with no cross-file/semantic claim need no field. Never - fail the audit because an LSP tool was unavailable, and never fabricate a - verification result. ## Your focus @@ -86,8 +71,7 @@ Write a JSON object to `$OUTPUT_FILE`: "category": "...", "message": "...", "file": "path or null", - "line": "number or null", - "lsp_verification": "verified|unverifiable (OMIT unless step 6 applied)" + "line": "number or null" } ], "sonnet_findings_confirmed": [""], @@ -95,7 +79,4 @@ Write a JSON object to `$OUTPUT_FILE`: } ``` -Include `lsp_verification` **only** on a finding you grounded via the LSP tools -in step 6 (`verified` or `unverifiable`); omit it otherwise. - Write with `cat > "$OUTPUT_FILE" <<'JSON' ... JSON`. Ensure it parses with `jq`. diff --git a/scripts/engine.sh b/scripts/engine.sh index 119c080ec..1d530035b 100644 --- a/scripts/engine.sh +++ b/scripts/engine.sh @@ -113,16 +113,6 @@ REVIEW_MCP_DEBUG="${REVIEW_MCP_DEBUG:-}" # the export also covers any future invocation via a separate child shell. export REVIEW_MCP_CONFIG REVIEW_MCP_ALLOWED_TOOLS REVIEW_MCP_DEBUG -# LSP-pilot stream capture (epic #839, story #844, issue #960). -# The owner-gated A/B master switch. When LSP_PILOT_ENABLED=true, the capturing -# review tiers (deep/audit/single via run_agentic, and run_duck) record their -# per-tool-call stream-json transcript so a real pr-review can emit ONE -# pilot-schema record (see scripts/lsp_pilot_emit.sh). When unset/not "true", -# capture is fully inert: the claude tiers keep `--output-format json` and write -# no transcript — byte-for-byte unchanged for every consumer repo that never -# sets the flag. -_lsp_pilot_active() { [ "${LSP_PILOT_ENABLED:-}" = "true" ]; } - set_engine_config() { case "$REVIEW_ENGINE" in claude) @@ -698,23 +688,11 @@ _claude_chain_invoke() { # Gated by ENGINE_USAGE_JSON (default on) so it can be disabled without a code # change. Falls back to raw output if extraction yields nothing. declare -f reset_engine_usage >/dev/null 2>&1 && reset_engine_usage - local _usage_json=0 _stream_capture=0 fmt_args=() + local _usage_json=0 fmt_args=() if [ -n "${TOKEN_LOG_FILE:-}" ] && [ "${ENGINE_USAGE_JSON:-1}" != "0" ] \ && declare -f parse_engine_usage >/dev/null 2>&1; then _usage_json=1 fmt_args=(--output-format json) - # LSP pilot (issue #960): when the recording flag is on AND this caller is a - # capturing tier (run_agentic deep/audit/single, run_duck set - # _LSP_PILOT_CAPTURE=1), swap the aggregate json envelope for the per-tool- - # call stream-json transcript. The terminal `result` event still carries - # top-level .result + .usage, so usage/text parsing below is unchanged; the - # raw NDJSON is teed to a per-call file under $LSP_PILOT_STREAM_DIR for - # scripts/lsp_pilot_measure.sh. Off-pilot or non-capturing tiers keep - # `--output-format json` (no behaviour change). - if [ "${_LSP_PILOT_CAPTURE:-0}" = "1" ] && _lsp_pilot_active; then - _stream_capture=1 - fmt_args=(--output-format stream-json --verbose) - fi fi local saved_ifs="$IFS" @@ -807,28 +785,10 @@ _claude_chain_invoke() { # receive plain text); fall back to the raw payload if extraction is empty or # the call failed (preserves error/rate-limit text for downstream detection). if [ -n "$final_stdout" ]; then - # LSP pilot (issue #960): in stream-capture mode the buffer is NDJSON. Tee - # the raw transcript to a unique per-call file under the per-PR stream dir - # (unique name → concurrent deep+duck never interleave), then reduce it to - # the terminal `result` event so the claude usage/text parsers — which read - # top-level .usage/.result — keep working unchanged. - local _usage_src="$final_stdout" _result_evt="" - if [ "$_stream_capture" -eq 1 ]; then - if [ -n "${LSP_PILOT_STREAM_DIR:-}" ] && [ -d "${LSP_PILOT_STREAM_DIR}" ]; then - local _ps - _ps="$(mktemp "${LSP_PILOT_STREAM_DIR}/stream.XXXXXX" 2>/dev/null || true)" - [ -n "$_ps" ] && cat "$final_stdout" >> "$_ps" 2>/dev/null || true - fi - _result_evt="$(mktemp 2>/dev/null || true)" - if [ -n "$_result_evt" ]; then - jq -c 'select(.type=="result")' "$final_stdout" 2>/dev/null | tail -n1 > "$_result_evt" || true - [ -s "$_result_evt" ] && _usage_src="$_result_evt" - fi - fi if [ "$_usage_json" -eq 1 ] && [ "$final_rc" -eq 0 ]; then - parse_engine_usage claude "$_usage_src" || true + parse_engine_usage claude "$final_stdout" || true local _txt - _txt="$(extract_engine_text claude "$_usage_src")" + _txt="$(extract_engine_text claude "$final_stdout")" if [ -n "$_txt" ]; then printf '%s\n' "$_txt" else @@ -837,7 +797,6 @@ _claude_chain_invoke() { else cat "$final_stdout" fi - [ -n "$_result_evt" ] && rm -f "$_result_evt" rm -f "$final_stdout" fi if [ -n "$final_stderr" ]; then @@ -1065,11 +1024,6 @@ run_agentic() { fi # Thread the opt-in MCP config (no-op when REVIEW_MCP_CONFIG is unset). _mcp_review_flags "Bash,Read,Grep,Glob" - # LSP pilot (issue #960): opt the navigation-heavy review tiers into - # stream-json capture. Action (writer/synthesis) is excluded — it is not a - # navigation tier. Inert unless _lsp_pilot_active (gate inside the chain). - local _LSP_PILOT_CAPTURE=0 - case "$tier" in deep|audit|single) _LSP_PILOT_CAPTURE=1 ;; esac if [ -n "$_tok_tmp" ]; then _claude_chain_invoke "$_agentic_chain" "$prompt_file" "$DEEP_TIMEOUT_SEC" \ --permission-mode acceptEdits \ @@ -1501,9 +1455,6 @@ run_duck() { unset GEMINI_API_KEY 2>/dev/null || true # Thread the opt-in MCP config (no-op when REVIEW_MCP_CONFIG is unset). _mcp_review_flags "Bash,Read,Grep,Glob" - # LSP pilot (issue #960): the rubber-duck is a navigation tier — opt it into - # stream-json capture (inert unless _lsp_pilot_active). - local _LSP_PILOT_CAPTURE=1 # Route through the chain helper so real token usage (incl. cache) is captured # when logging is on; a single-model "chain" preserves prior behaviour. if [ -n "$_tok_tmp" ]; then diff --git a/scripts/lib/lsp-verification.sh b/scripts/lib/lsp-verification.sh deleted file mode 100644 index bb7081898..000000000 --- a/scripts/lib/lsp-verification.sh +++ /dev/null @@ -1,141 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# LSP finding-verification step — epic #839 (LSP pilot), story #843. -# Contract: docs/lsp-pilot.md §2 (find-references / diagnostics grounding). -# -# This is the one genuinely-new piece of the LSP pilot (docs/lsp-pilot.md §6): -# before a deep/audit finding that asserts a cross-file/semantic claim (e.g. -# "undefined", "breaks N callers", "unused symbol") is posted, the reviewer -# grounds it against the LSP navigation tools (find_references / get_diagnostics). -# Only the model can call the MCP `mcp__lsp__*` tools, so it annotates each such -# finding in its JSON output with an `lsp_verification` field ("verified" | -# "unverifiable"); this shell layer ENFORCES that annotation deterministically: -# -# - A finding LSP could not ground ("unverifiable") is DOWNGRADED one severity -# level and ANNOTATED `[lsp: unverifiable]` — never silently dropped and -# never posted as a confident finding (AC #2). -# - Each verified/unverifiable outcome is emitted to the Token Cost Observatory -# JSONL so the comparison harness can compute the false-positive-rate delta -# (AC #4) — via emit_verification_record in token-metrics.sh. -# - The whole step is INERT (findings file byte-for-byte unchanged, nothing -# emitted) when the LSP server is absent or degraded, so the review proceeds -# exactly as today (AC #3). This reuses the same MCP connect/degradation -# signal `_emit_mcp_failure_warning` keys on (scripts/engine.sh). -# -# Sourced by scripts/review-one-pr.sh after engine.sh and token-metrics.sh. - -# _lsp_failure_pattern -# The MCP connection/init-failure regex used to detect a degraded LSP server. -# Prefer engine.sh's single source of truth (_mcp_failure_pattern) when this lib -# is sourced alongside it; otherwise fall back to an equivalent local pattern so -# the lib stays usable (and unit-testable) on its own. -_lsp_failure_pattern() { - if declare -f _mcp_failure_pattern >/dev/null 2>&1; then - _mcp_failure_pattern - return - fi - local _pat - _pat='mcp server [^[:space:]]*[[:space:]]*("[^"]*"[[:space:]]*)?(failed|error)' - _pat="$_pat"'|failed to (connect|initialize|reconnect|start)[^.]*mcp' - _pat="$_pat"'|mcp[^.]*(connection|initializ)[^.]*(fail|error)' - _pat="$_pat"'|could not (connect to|start) mcp server' - printf '%s' "$_pat" -} - -# _lsp_downgrade_severity -# One step toward "info" on the info|minor|major|critical ladder. Bottoms out at -# info; unknown values collapse to info (safe — never escalates). -_lsp_downgrade_severity() { - case "$1" in - critical) printf 'major' ;; - major) printf 'minor' ;; - *) printf 'info' ;; - esac -} - -# lsp_verification_active [cli_output_file...] -# Returns 0 (active) iff the LSP navigation tools are wired AND not degraded: -# - REVIEW_MCP_CONFIG points at a readable file, -# - REVIEW_MCP_ALLOWED_TOOLS permits at least one mcp__lsp__ tool (so this is -# the LSP pilot, not e.g. a Context7-only run), -# - none of the given CLI-output files show an MCP connect/init failure. -# Returns 1 (inert) otherwise. The CLI-output scan is the degradation gate: a -# server that failed to connect means "skip verification, review as today". -lsp_verification_active() { - [ -n "${REVIEW_MCP_CONFIG:-}" ] || return 1 - [ -f "${REVIEW_MCP_CONFIG}" ] && [ -r "${REVIEW_MCP_CONFIG}" ] || return 1 - case "${REVIEW_MCP_ALLOWED_TOOLS:-}" in - *mcp__lsp__*) : ;; - *) return 1 ;; - esac - - if [ "$#" -gt 0 ]; then - local f present=() - for f in "$@"; do [ -f "$f" ] && present+=("$f"); done - if [ "${#present[@]}" -gt 0 ] \ - && grep -qiE "$(_lsp_failure_pattern)" "${present[@]}" 2>/dev/null; then - return 1 - fi - fi - return 0 -} - -# apply_lsp_verification [cli_output_file...] -# Enforces the LSP finding-verification contract on a review verdict file in -# place. No-op (file untouched, nothing emitted) when verification is inert — -# LSP unwired/degraded, jq missing, or the file is not a {findings:[...]} object. -apply_lsp_verification() { - local file="${1:-}" tier="${2:-deep}" - if [ "$#" -ge 2 ]; then shift 2; else shift "$#"; fi # remaining args: CLI output files - [ -f "$file" ] || return 0 - command -v jq >/dev/null 2>&1 || return 0 - jq -e 'type == "object" and (.findings | type == "array")' "$file" >/dev/null 2>&1 || return 0 - lsp_verification_active "$@" || return 0 - - local workflow="${TOKEN_WORKFLOW:-unknown}" context="${PR_URL:-}" - - # Emit one verification record per annotated finding (before rewriting, so the - # original severity is the recorded severity_before). - if declare -f emit_verification_record >/dev/null 2>&1; then - local idx outcome sev cat sev_after - while IFS=$'\t' read -r idx outcome sev cat; do - [ -n "$idx" ] || continue - if [ "$outcome" = "unverifiable" ]; then - sev_after="$(_lsp_downgrade_severity "$sev")" - else - sev_after="$sev" - fi - emit_verification_record "$workflow" "$tier" "$context" \ - "$idx" "$cat" "$sev" "$sev_after" "$outcome" || true - done < <(jq -r ' - .findings | to_entries[] - | (.value.lsp_verification // "") as $v - | select($v == "verified" or $v == "unverifiable") - | [ (.key | tostring), $v, (.value.severity // "info"), (.value.category // "") ] - | @tsv - ' "$file" 2>/dev/null) - fi - - # Downgrade + annotate every unverifiable finding (atomic rewrite). - local tmp; tmp="$(mktemp "${file}.lspv.XXXXXX")" || return 0 - if jq ' - def downgrade: - if . == "critical" then "major" - elif . == "major" then "minor" - else "info" end; - .findings |= map( - if (.lsp_verification // "") == "unverifiable" then - .severity = ((.severity // "info") | downgrade) - | .message = ((.message // "") - + (if (.message // "") | test("\\[lsp: unverifiable\\]") - then "" else " [lsp: unverifiable]" end)) - else . end - ) - ' "$file" > "$tmp" 2>/dev/null && [ -s "$tmp" ]; then - mv "$tmp" "$file" - else - rm -f "$tmp" - fi - return 0 -} diff --git a/scripts/lib/token-metrics.sh b/scripts/lib/token-metrics.sh index 3622f4fd5..181852ded 100644 --- a/scripts/lib/token-metrics.sh +++ b/scripts/lib/token-metrics.sh @@ -131,100 +131,6 @@ emit_token_record() { printf '%s\n' "$record" >> "$TOKEN_LOG_FILE" 2>/dev/null || true } -# emit_verification_record -# -# Appends one finding-verification record to TOKEN_LOG_FILE on the SAME JSONL -# channel as token records (story #843, epic #839). The record is discriminated -# by kind:"finding_verification" so the comparison harness (story 2) can compute -# the false-positive-rate delta and the cost report can skip it (it is not a -# priced token-usage call — see scripts/token_report.sh `annotate_records`). -# No-op when TOKEN_LOG_FILE is unset; swallows I/O errors so it never aborts a run. -emit_verification_record() { - [ -n "${TOKEN_LOG_FILE:-}" ] || return 0 - - local workflow="$1" tier="$2" context="$3" finding_index="$4" - local category="$5" severity_before="$6" severity_after="$7" outcome="$8" - - local ts run_id record - ts=$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo "") - run_id="${GITHUB_RUN_ID:-}" - - record=$(jq -cn \ - --arg ts "$ts" \ - --arg workflow "$workflow" \ - --arg tier "$tier" \ - --arg context "$context" \ - --arg finding_index "$finding_index" \ - --arg category "$category" \ - --arg severity_before "$severity_before" \ - --arg severity_after "$severity_after" \ - --arg outcome "$outcome" \ - --arg run_id "$run_id" \ - '{ - kind: "finding_verification", - ts: $ts, - workflow: $workflow, - tier: $tier, - context: $context, - finding_index: ($finding_index | tonumber? // $finding_index), - category: $category, - severity_before: $severity_before, - severity_after: $severity_after, - outcome: $outcome, - run_id: $run_id - }' 2>/dev/null) || return 0 - - printf '%s\n' "$record" >> "$TOKEN_LOG_FILE" 2>/dev/null || true -} - -# emit_lsp_coldstart_record -# [reason] -# Appends one LSP cold-start record to TOKEN_LOG_FILE on the SAME JSONL channel as -# token records (story #846, epic #839). Discriminated by kind:"lsp_cold_start" so -# story #844's comparison harness can compute cold-start P50/P95 against the SLA and -# the cost report skips it (token_report.sh keeps only (.kind // "token_usage") == -# "token_usage" — a cold-start record is not a priced token-usage call). -# cache — "hit" | "miss" | "unknown" (actions/cache outcome) -# skipped — "true" when LSP wiring was skipped (SLA exceeded or toolchain absent) -# No-op when TOKEN_LOG_FILE is unset; swallows I/O errors so it never aborts a run. -emit_lsp_coldstart_record() { - [ -n "${TOKEN_LOG_FILE:-}" ] || return 0 - - # set -u-safe defaults (this lib runs under set -euo pipefail): a short call - # degrades to a defaulted record rather than crashing on an unbound positional. - local workflow="${1:-}" candidate="${2:-}" cold_start_ms="${3:-0}" cache="${4:-unknown}" - local skipped="${5:-false}" sla_ms="${6:-0}" reason="${7:-}" - - local ts run_id record - ts=$(date -u +%Y-%m-%dT%H:%M:%SZ 2>/dev/null || echo "") - run_id="${GITHUB_RUN_ID:-}" - - record=$(jq -cn \ - --arg ts "$ts" \ - --arg workflow "$workflow" \ - --arg candidate "$candidate" \ - --arg cold_start_ms "$cold_start_ms" \ - --arg cache "$cache" \ - --arg skipped "$skipped" \ - --arg sla_ms "$sla_ms" \ - --arg reason "$reason" \ - --arg run_id "$run_id" \ - '{ - kind: "lsp_cold_start", - ts: $ts, - workflow: $workflow, - candidate: $candidate, - cold_start_ms: ($cold_start_ms | tonumber? // 0), - cache: $cache, - skipped: ($skipped == "true"), - sla_ms: ($sla_ms | tonumber? // 0), - reason: $reason, - run_id: $run_id - }' 2>/dev/null) || return 0 - - printf '%s\n' "$record" >> "$TOKEN_LOG_FILE" 2>/dev/null || true -} - # ───────────────────────────────────────────────────────────────────────────── # Real-usage capture (replaces the char/4 estimate when an engine reports usage) # diff --git a/scripts/lsp_pilot_compare.sh b/scripts/lsp_pilot_compare.sh deleted file mode 100644 index 26fc1bdd0..000000000 --- a/scripts/lsp_pilot_compare.sh +++ /dev/null @@ -1,283 +0,0 @@ -#!/usr/bin/env bash -# lsp_pilot_compare.sh — LSP-pilot comparison harness (#841, epic #839). -# -# Compares a candidate LSP-ON review run against the FROZEN LSP-off baseline -# captured once under evals/lsp-pilot/holdout/baseline-lsp-off.jsonl. It renders -# per-PR and aggregate deltas for: -# * speed — cold-start (N/A for the LSP-off control), wall-time -# * cost — navigation tool-call tokens, tool-call count, ET, USD -# * quality — findings and labelled false-positives (the explicit proxy below) -# -# It FAILS LOUD (non-zero) when a candidate PR has no baseline counterpart, so a -# partial corpus can never masquerade as a clean comparison (the success metric -# requires every candidate be scored against the fixed, frozen target). -# -# Review-quality proxy (explicit, not ad hoc): quality per PR is -# (findings, false_positives). A candidate REGRESSES on a PR when its -# false_positive count exceeds the frozen baseline's — i.e. precision dropped. A -# navigation-token win that costs precision is a no-go, so this is mandatory. -# -# Record shape (one JSON object per line, both baseline and candidate streams) — -# a Token Cost Observatory record (scripts/lib/token-metrics.sh) plus pilot fields: -# pr "#@" — the immutable join key -# variant "lsp-off" | "lsp-on" -# candidate "baseline" | "" -# model model id (priced via scripts/lib/model-pricing.tsv) -# input_tokens / cache_read_tokens / output_tokens -# nav_tokens navigation tool-call tokens -# tool_calls navigation tool-call count -# findings review findings count -# false_positives labelled false-positive count (quality proxy) -# cold_start_s server launch->query-ready seconds (null/N/A for lsp-off) -# wall_time_s end-to-end review seconds -# A candidate PR may appear on multiple lines (the corpus allows <=3 runs/PR); -# such runs are aggregated by mean. -# -# Layout (mirrors scripts/token_report.sh): the render_* / lp_* functions are -# PURE (no network) and unit-tested in tests/lsp_pilot_compare.bats; main() only -# resolves paths and prints. -# -# Usage: -# bash scripts/lsp_pilot_compare.sh [name] - -set -euo pipefail - -# Reuse the single USD/int formatters (AGENTS.md "Cost reporting") and the dated -# pricing helpers (cost_usd, et_multiplier_for). Sourcing token_report.sh does not -# run its main() (guarded by the BASH_SOURCE check at its foot). -_LP_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=scripts/token_report.sh -source "${_LP_DIR}/token_report.sh" - -# --------------------------------------------------------------------------- -# Pure helpers -# --------------------------------------------------------------------------- - -# _lp_prs — emit the sorted-unique set of pr identifiers in a stream. -_lp_prs() { - local jsonl="$1" - [ -f "$jsonl" ] || return 0 - jq -r 'select(type=="object") | .pr // empty' "$jsonl" 2>/dev/null | sort -u -} - -# lp_missing_baselines -# Prints every candidate pr that has no baseline counterpart (one per line). -# Returns 1 if any are missing, else 0. This is the AC5 coverage guard. -lp_missing_baselines() { - local baseline="$1" candidate="$2" missing - missing="$(comm -13 <(_lp_prs "$baseline") <(_lp_prs "$candidate"))" - if [ -n "$missing" ]; then - printf '%s\n' "$missing" - return 1 - fi - return 0 -} - -# _lp_aggregate — fold a stream to one mean row per pr. Pricing (ET, USD) -# is computed per record from the dated price table, then averaged. Columns: -# 1 pr 2 nav 3 tool_calls 4 findings 5 false_positives -# 6 cold_start(mean | "NA") 7 wall_time 8 et 9 usd 10 usd_unknown(1/0) -# Pure: reads only the JSONL and the local price table. -_lp_aggregate() { - local jsonl="$1" - [ -f "$jsonl" ] || return 0 - - local pr model inp ca out nav tool find fp cold wall - while IFS=$'\t' read -r pr model inp ca out nav tool find fp cold wall; do - [ -n "$pr" ] || continue - local mult usd known=1 - mult="$(et_multiplier_for "$model")" - [ -z "$mult" ] && mult=1.0 - usd="$(cost_usd "$model" "$inp" "$ca" "$out")" - if [ -z "$usd" ]; then usd=0; known=0; fi - printf '%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\t%s\n' \ - "$pr" "$nav" "$tool" "$find" "$fp" "$cold" "$wall" "$mult" "$inp" "$ca" "$out" "$usd" "$known" - done < <(jq -r 'select(type=="object") | [ - (.pr // "unknown"), (.model // "-"), - (.input_tokens // 0), (.cache_read_tokens // 0), (.output_tokens // 0), - (.nav_tokens // 0), (.tool_calls // 0), (.findings // 0), (.false_positives // 0), - (if (.cold_start_s == null) then "null" else (.cold_start_s|tostring) end), - (.wall_time_s // 0) - ] | @tsv' "$jsonl" 2>/dev/null) | awk -F'\t' ' - { - pr = $1; n[pr]++ - nav[pr] += $2; tool[pr] += $3; find[pr] += $4; fp[pr] += $5 - wall[pr] += $7 - mult = $8; inp = $9; ca = $10; out = $11; usd_val = $12; known_val = $13 - et_val = mult * (1.0 * inp + 0.1 * ca + 4.0 * out) - et[pr] += et_val; usd[pr] += usd_val - if (known_val == 0) unk[pr] = 1 - if ($6 != "null" && $6 != "" && $6 != "NA") { cold[pr] += $6; coldn[pr]++ } - } - END { - for (pr in n) { - c = n[pr] - coldval = (coldn[pr] > 0) ? sprintf("%.1f", cold[pr] / coldn[pr]) : "NA" - printf "%s\t%.1f\t%.1f\t%.2f\t%.2f\t%s\t%.1f\t%.4f\t%.6f\t%d\n", - pr, nav[pr]/c, tool[pr]/c, find[pr]/c, fp[pr]/c, coldval, - wall[pr]/c, et[pr]/c, usd[pr]/c, (unk[pr] ? 1 : 0) - } - }' | sort -} - -# _lp_ratio — baseline/candidate as "N.Nx" (— if candidate is 0). -_lp_ratio() { - awk -v b="$1" -v c="$2" 'BEGIN { if (c > 0) printf "%.1fx", b / c; else printf "—" }' -} - -# render_lsp_comparison [candidate_name] -# Writes the Markdown comparison to stdout. Returns non-zero (AC5) WITHOUT -# emitting a clean comparison when any candidate PR lacks a baseline counterpart. -render_lsp_comparison() { - local baseline="$1" candidate="$2" name="${3:-candidate}" - - if [ ! -f "$baseline" ] || [ ! -f "$candidate" ]; then - echo "[lsp-pilot] ERROR: baseline and candidate JSONL paths are both required" >&2 - return 2 - fi - - # AC5 — fail loud on any candidate PR with no frozen baseline counterpart. - local missing - if ! missing="$(lp_missing_baselines "$baseline" "$candidate")"; then - printf '# ❌ LSP pilot comparison — INCOMPLETE\n\n' - printf 'Candidate run has PR(s) with **no frozen LSP-off baseline counterpart**, so ' - printf 'this is not a clean comparison against the fixed target. Refusing to score.\n\n' - printf 'Missing baseline for:\n\n' - local pr - while IFS= read -r pr; do - [ -n "$pr" ] || continue - printf -- '- `%s`\n' "$pr" - done <<< "$missing" - return 1 - fi - - local bagg cagg - bagg="$(_lp_aggregate "$baseline")" - cagg="$(_lp_aggregate "$candidate")" - - printf '# 🔬 LSP pilot comparison — `%s` vs frozen LSP-off baseline\n\n' "$name" - printf '_Speed (cold-start, wall-time) · cost (nav tokens, tool calls, ET, USD) · ' - printf 'quality (findings, false positives), LSP-on vs the immutable LSP-off control._\n\n' - printf '**Quality proxy:** a PR regresses when its **false-positive** count exceeds the ' - printf "frozen baseline's (precision dropped). A navigation-token win that costs " - printf 'precision is a no-go.\n\n' - - awk -F'\t' ' - function fmt_int(n, neg, s, r) { - n = int(n + 0.5); s = ""; if (n < 0) { neg = 1; n = -n } - if (n == 0) s = "0" - while (n > 0) { - r = n % 1000; n = int(n / 1000); - s = (n > 0) ? sprintf("%03d", r) (s == "" ? "" : "," s) : r (s == "" ? "" : "," s) - } - return (neg ? "-" : "") s - } - - function fmt_usd(v) { - return sprintf("$%.2f", v) - } - - # First file: baseline aggregate - NR == FNR { - pr = $1 - b_nav[pr] = $2; b_tool[pr] = $3; b_find[pr] = $4; b_fp[pr] = $5 - b_cold[pr] = $6; b_wall[pr] = $7; b_et[pr] = $8; b_usd[pr] = $9 - next - } - - # Print header on transition to candidate aggregate - NR > FNR && !candidate_header_printed { - print "## Per PR (off → on)\n" - print "| PR | Nav tokens | Nav reduction | Tool calls | Cold start | Wall time | ET | USD | Findings | False positives | Quality |" - print "|---|---|---:|---|---:|---|---:|---:|---|---:|:--|" - candidate_header_printed = 1 - } - - # Second file: candidate aggregate - { - pr = $1 - nav = $2; tool = $3; find = $4; fp = $5; cold = $6; wall = $7; et = $8; usd = $9 - - bnav = b_nav[pr] - btool = b_tool[pr] - bfind = b_find[pr] - bfp = b_fp[pr] - bwall = b_wall[pr] - bet = b_et[pr] - busd = b_usd[pr] - - if (nav > 0) ratio = sprintf("%.1fx", bnav / nav); else ratio = "—" - - if (fp > bfp) { - quality = "⚠️ REGRESSION" - any_regression = 1 - } else { - quality = "ok" - } - - if (cold == "NA") { - coldcell = "N/A → N/A" - } else { - coldcell = "N/A → " cold "s" - cold_sum += cold - cold_n++ - } - - printf "| \`%s\` | %s → %s | %s | %s → %s | %s | %ss → %ss | %s → %s | %s → %s | %s → %s | %s → %s | %s |\n", - pr, - fmt_int(bnav), fmt_int(nav), ratio, - fmt_int(btool), fmt_int(tool), - coldcell, - bwall, wall, - fmt_int(bet), fmt_int(et), - fmt_usd(busd), fmt_usd(usd), - fmt_int(bfind), fmt_int(find), - fmt_int(bfp), fmt_int(fp), - quality - - tot_bnav += bnav; tot_cnav += nav - tot_bet += bet; tot_cet += et - tot_busd += busd; tot_cusd += usd - tot_bfind += bfind; tot_cfind += find - tot_bfp += bfp; tot_cfp += fp - } - - END { - if (tot_cnav > 0) overall_ratio = sprintf("%.1fx", tot_bnav / tot_cnav); else overall_ratio = "—" - if (cold_n > 0) mean_cold = sprintf("%.1fs", cold_sum / cold_n); else mean_cold = "N/A" - - print "\n## Aggregate\n" - printf "- **Navigation tool-call tokens:** %s (off) → %s (on) — **%s** reduction\n", fmt_int(tot_bnav), fmt_int(tot_cnav), overall_ratio - printf "- **ET:** %s (off) → %s (on)\n", fmt_int(tot_bet), fmt_int(tot_cet) - printf "- **USD:** %s (off) → %s (on)\n", fmt_usd(tot_busd), fmt_usd(tot_cusd) - printf "- **Mean LSP-on cold start:** %s (baseline N/A — no server)\n", mean_cold - printf "- **Findings:** %s (off) → %s (on)\n", fmt_int(tot_bfind), fmt_int(tot_cfind) - printf "- **False positives:** %s (off) → %s (on)\n", fmt_int(tot_bfp), fmt_int(tot_cfp) - - if (any_regression == 1) { - print "- **Quality verdict:** ⚠️ **REGRESSION** — at least one PR has more false positives than the frozen baseline." - } else { - print "- **Quality verdict:** ✅ **PASS** — no PR exceeds the frozen baseline false-positive count." - } - } - ' <(echo "$bagg") <(echo "$cagg") - - return 0 -} - -# --------------------------------------------------------------------------- -# main -# --------------------------------------------------------------------------- - -main() { - if [ "$#" -lt 2 ]; then - echo "usage: $0 [candidate-name]" >&2 - exit 2 - fi - render_lsp_comparison "$@" -} - -# Only run main when executed directly (not when sourced by tests). -if [ "${BASH_SOURCE[0]}" = "${0}" ]; then - main "$@" -fi diff --git a/scripts/lsp_pilot_emit.sh b/scripts/lsp_pilot_emit.sh deleted file mode 100644 index c114b4ed1..000000000 --- a/scripts/lsp_pilot_emit.sh +++ /dev/null @@ -1,173 +0,0 @@ -#!/usr/bin/env bash -# lsp_pilot_emit.sh — turn ONE real pr-review (running under the pilot) into ONE -# pilot-schema record on TOKEN_LOG_FILE (#844, epic #839, issue #960). -# -# Where #952's standalone lsp_pilot_run.sh re-runs a SIMPLIFIED review to produce -# pilot records, this rides the PRODUCTION review: scripts/engine.sh captures the -# deep/audit/rubber-duck claude tiers' stream-json transcripts (gated on -# LSP_PILOT_ENABLED) into a per-PR stream dir, and scripts/review-one-pr.sh calls -# emit_record here when the review exits. So every auto-review under the flag is a -# free LSP-off ("A") sample, and flipping the LSP MCP on yields the matching -# LSP-on ("B") sample through the SAME pipeline — apples-to-apples, no synthetic -# input. -# -# Contract (mirrors lsp_pilot_run.sh's run_variant semantics so both sides join -# and render via scripts/lsp_pilot_compare.sh UNCHANGED): -# variant the explicit LSP_PILOT_VARIANT (on|off, #1031) when set, else -# lsp-on when the LSP MCP is wired (REVIEW_MCP_CONFIG → readable -# file, as setup-lsp-pilot.sh sets it), else lsp-off. -# candidate agent-lsp (override: LSP_CANDIDATE) on the lsp-on leg; baseline -# on the lsp-off leg. -# pr "#@" — the immutable join key. -# -# The record is produced by scripts/lsp_pilot_measure.sh (its schema is reused -# verbatim — NOT modified here) and then tagged with kind:"lsp_pilot_run" so -# scripts/token_report.sh keeps excluding it from cost aggregation (its filter is -# (.kind // "token_usage") == "token_usage"). emit_record is non-fatal and a -# strict no-op off-pilot: the five consumer repos that never set -# LSP_PILOT_ENABLED are wholly unaffected. -# -# Pure helpers (lpe_*) are unit-tested in tests/dev-lead/unit/test_lsp_pilot_emit.bats. - -set -euo pipefail - -_LPE_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - -# lpe_pilot_active — true (0) when the pilot recording flag is on. The master -# gate: off → emit_record is a no-op and no consumer repo is affected. -lpe_pilot_active() { [ "${LSP_PILOT_ENABLED:-}" = "true" ]; } - -# lpe_variant — the A/B leg label. When the pipeline explicitly selects a leg via -# LSP_PILOT_VARIANT (on|off, issue #1031) that is AUTHORITATIVE — this is what -# DECOUPLES capture from LSP wiring, so a production review can emit the lsp-off -# (A) control leg even here, where engine.sh auto-defaults REVIEW_MCP_CONFIG to -# the committed .github/review-mcp.json (the Context7 MCP) and -# review-one-pr.sh sources engine.sh — which would otherwise mislabel A as lsp-on. -# When unset/"none", fall back to detecting the wired MCP (REVIEW_MCP_CONFIG → -# readable file, the exact activation condition engine.sh's _mcp_review_flags -# uses), preserving the legacy vars.LSP_PILOT_ENABLED path and lsp_pilot_run.sh's -# run_variant() semantics. -lpe_variant() { - case "${LSP_PILOT_VARIANT:-}" in - on) printf 'lsp-on\n'; return 0 ;; - off) printf 'lsp-off\n'; return 0 ;; - esac - if [ -n "${REVIEW_MCP_CONFIG:-}" ] && [ -f "${REVIEW_MCP_CONFIG}" ] && [ -r "${REVIEW_MCP_CONFIG}" ]; then - printf 'lsp-on\n' - else - printf 'lsp-off\n' - fi -} - -# lpe_candidate — "baseline" on the lsp-off control leg; the candidate server name -# (LSP_CANDIDATE, default agent-lsp) on the lsp-on leg — matching run_variant(). -lpe_candidate() { - if [ "$(lpe_variant)" = "lsp-on" ]; then - printf '%s\n' "${LSP_CANDIDATE:-agent-lsp}" - else - printf 'baseline\n' - fi -} - -# lpe_pr_key — assemble the "#@" join -# key from a PR HTML URL (https://github.com///pull/). -lpe_pr_key() { - local url="${1:-}" sha="${2:-}" repo num - repo="$(printf '%s' "$url" | sed -E 's#^https?://github\.com/([^/]+/[^/]+)/pull/[0-9].*#\1#')" - num="$(printf '%s' "$url" | sed -E 's#.*/pull/([0-9]+).*#\1#')" - printf '%s#%s@%s\n' "$repo" "$num" "$sha" -} - -# lpe_wall_seconds — float seconds, 1 dp, clamped -# >=0. Non-numeric input (e.g. %N unsupported) → 0.0; never crashes. -lpe_wall_seconds() { - local a="${1:-0}" b="${2:-0}" - awk -v a="$a" -v b="$b" 'BEGIN { - if (a !~ /^[0-9]+$/ || b !~ /^[0-9]+$/ || a == 0 || b == 0) { print "0.0"; exit } - d = (b - a) / 1000000000.0; if (d < 0) d = 0; printf "%.1f", d - }' -} - -# lpe_emit_record [token_log] -# Append exactly ONE pilot record (kind:"lsp_pilot_run") to TOKEN_LOG_FILE from -# the captured stream transcripts. Strict no-op (return 0) when: the pilot is off, -# jq is absent, TOKEN_LOG_FILE is unset, or no transcript was captured (e.g. the -# review took a skip/noop path with no claude tier call). Never aborts the review. -lpe_emit_record() { - local stream_dir="${1:-}" pr_url="${2:-}" head_sha="${3:-}" wall="${4:-0}" - local token_log="${5:-${TOKEN_LOG_FILE:-}}" - - lpe_pilot_active || return 0 - [ -n "$token_log" ] || return 0 - command -v jq >/dev/null 2>&1 || return 0 - [ -n "$stream_dir" ] && [ -d "$stream_dir" ] || return 0 - - # Fold every per-call transcript (deep/audit/duck) into one stream. - # Nav tool counts are summed across all transcripts; usage is synthesized into - # exactly ONE terminal result event (sum of all per-call result usages) so - # lsp_pilot_measure.sh's lpm_usage_from_stream always reads a single, stable - # result event instead of whichever stream.XXXXXX filename sorts last. - local combined found=0 f - combined="$(mktemp 2>/dev/null || true)" - [ -n "$combined" ] || return 0 - for f in "$stream_dir"/*; do - [ -f "$f" ] || continue - cat "$f" >> "$combined" 2>/dev/null && found=1 - done - if [ "$found" -ne 1 ] || [ ! -s "$combined" ]; then - rm -f "$combined" - return 0 - fi - - # Re-write combined: keep all non-result events for nav counting; replace the - # multiple per-call result events with one synthesized event that sums all - # usage fields and takes the model from the last result event in the stream. - local _synth - _synth="$(mktemp 2>/dev/null || true)" - if [ -n "$_synth" ] && jq -cs ' - [.[] | select(type=="object")] as $all | - ($all[] | select(.type != "result")), - ([$all[] | select(.type=="result")] as $rs | - if ($rs | length) == 0 then empty else - $rs[-1] | .usage = { - input_tokens: ([$rs[].usage.input_tokens // 0] | add), - cache_read_input_tokens: ([$rs[].usage.cache_read_input_tokens // 0] | add), - cache_creation_input_tokens: ([$rs[].usage.cache_creation_input_tokens // 0] | add), - output_tokens: ([$rs[].usage.output_tokens // 0] | add) - } - end) - ' "$combined" > "$_synth" 2>/dev/null; then - mv "$_synth" "$combined" - else - rm -f "$_synth" - fi - - local variant candidate pr_key record - variant="$(lpe_variant)" - candidate="$(lpe_candidate)" - pr_key="$(lpe_pr_key "$pr_url" "$head_sha")" - - # Reuse #952's extractor verbatim (its record schema is out of scope to change). - # --token-log feeds it the #843 finding-verification + #846 cold-start records - # that share TOKEN_LOG_FILE. Model is read from the stream's result event. - if ! record="$(bash "${_LPE_DIR}/lsp_pilot_measure.sh" "$combined" \ - --pr "$pr_key" --variant "$variant" --candidate "$candidate" \ - --wall-time-s "$wall" --token-log "${token_log:-/dev/null}" 2>/dev/null)"; then - rm -f "$combined" - return 0 - fi - rm -f "$combined" - [ -n "$record" ] || return 0 - - # Tag with kind so token_report.sh keeps excluding it from cost aggregation. - # This is added to the EMITTED line only — the extractor's schema is untouched. - jq -c '. + {kind: "lsp_pilot_run"}' <<< "$record" >> "$token_log" 2>/dev/null || true - return 0 -} - -# Sourced for its helpers (review-one-pr.sh, unit tests). Direct execution is a -# usage error — this script has no standalone CLI. -if [ "${BASH_SOURCE[0]:-$0}" = "$0" ]; then - echo "lsp_pilot_emit.sh is a sourced helper library; it has no CLI entry point." >&2 - exit 2 -fi diff --git a/scripts/lsp_pilot_measure.sh b/scripts/lsp_pilot_measure.sh deleted file mode 100644 index aa985b65a..000000000 --- a/scripts/lsp_pilot_measure.sh +++ /dev/null @@ -1,238 +0,0 @@ -#!/usr/bin/env bash -# lsp_pilot_measure.sh — turn ONE review run into ONE pilot-schema JSONL record -# (#844, epic #839). The companion to scripts/lsp_pilot_compare.sh: compare renders -# baseline-vs-candidate; measure PRODUCES the per-run records compare consumes. -# -# Why this exists: the review engine (scripts/engine.sh) emits aggregate token -# records + finding-verification records, but NOT the pilot's headline navigation -# fields (nav_tokens, tool_calls). Those are only observable in the model's -# tool-call transcript. So the pilot runner invokes claude with -# `--output-format stream-json --verbose`, tees the transcript, and hands it here. -# -# What it measures, and how (honest + documented — no field is invented): -# tool_calls COUNT of navigation tool_use events in the transcript (a tool is -# "navigation" when its name starts with one of NAV_TOOL_PREFIXES). -# This is symmetric across variants: LSP-off navigates with -# Grep/Glob/Read/Bash; LSP-on navigates with mcp__lsp__*. Both are -# counted by the same rule so the comparison is apples-to-apples. -# nav_tokens ESTIMATED tokens those navigation calls cost the context: for each -# navigation tool_use, length(input)/4 + length(matching tool_result -# content)/4 (the char/4 proxy the repo already uses in -# estimate_tokens_from_file). It is the read+write context churn of -# navigation — exactly the cost the LSP index is meant to cut. Marked -# an estimate; the *_tokens / usage fields below are the engine's -# real reported usage, not estimates. -# findings / From kind:"finding_verification" records on TOKEN_LOG_FILE (story -# false_pos. #843): findings = verification records; false_positives = those -# whose outcome downgraded/removed the finding. Populated only when -# the verification step ran (the LSP path); 0 otherwise — the quality -# proxy is asymmetric by construction and documented as such. -# cold_start_s From the kind:"lsp_cold_start" record (story #846): cold_start_ms/1000. -# null for lsp-off (no server to launch) — matches the frozen baseline. -# input/cache/ The run's REAL reported usage, read from the stream's terminal -# output_tokens `result` event (.usage), falling back to the last assistant usage. -# model From --model, else the transcript's reported model. -# wall_time_s Passed in (the runner times the claude call end-to-end). -# -# Output: exactly one JSON object (the pilot record) on stdout, joinable against the -# baseline by its `pr` key and renderable by lsp_pilot_compare.sh. -# -# Pure/Testable: the lpm_* helpers read only their file/string args (no network), -# and main() only parses flags and prints — mirrors token_report.sh / the compare -# harness. Unit-tested in tests/dev-lead/unit/test_lsp_pilot_measure.bats. -# -# Usage: -# lsp_pilot_measure.sh \ -# --pr "#@" --variant lsp-on --candidate agent-lsp \ -# [--model claude-opus-4-8] [--wall-time-s 173.0] \ -# [--cold-start-ms 8200] [--token-log ] [--nav-prefixes "a,b,c"] - -set -euo pipefail - -# Navigation tool-name prefixes (a tool counts as navigation when its name starts -# with any of these). Override via NAV_TOOL_PREFIXES for a different toolset. The -# default set is symmetric across variants on purpose (see header). -NAV_TOOL_PREFIXES_DEFAULT="mcp__lsp__,Grep,Glob,Read,Bash" - -# --------------------------------------------------------------------------- -# Pure helpers -# --------------------------------------------------------------------------- - -# _lpm_have_jq — jq is required for transcript parsing; fail loud if absent. -_lpm_have_jq() { command -v jq >/dev/null 2>&1; } - -# lpm_nav_from_stream -# Emits "\t" — the count of navigation tool_use events and -# the estimated tokens they cost (input + matching tool_result, char/4). A missing -# or unparseable transcript yields "0\t0" (Fail Loud upstream, soft here). -lpm_nav_from_stream() { - local stream="$1" prefixes="${2:-$NAV_TOOL_PREFIXES_DEFAULT}" - if [ ! -f "$stream" ] || ! _lpm_have_jq; then - printf '0\t0\n'; return 0 - fi - # Slurp the NDJSON: gather tool_use {id,name,input} and tool_result {id,content}, - # join on id, keep navigation names, sum char/4 of input + result content. - jq -rs --arg prefixes "$prefixes" ' - ($prefixes | split(",") | map(select(length > 0))) as $pre | - def is_nav($n): ($n != null) and ([ $pre[] | . as $p | ($n | startswith($p)) ] | any); - # content can be a string or an array of {type,text}; normalize to a string. - def text_of($c): - if ($c | type) == "string" then $c - elif ($c | type) == "array" then ([ $c[]? | (.text // (. | tostring)) ] | join("")) - elif $c == null then "" - else ($c | tostring) end; - [ .[] | select(type=="object") ] as $events | - [ $events[] | select(.type=="assistant") | (.message.content // [])[]? - | select(.type=="tool_use") | {id, name, input: (.input // {})} ] as $uses | - ( [ $events[] | select(.type=="user") | (.message.content // [])[]? - | select(.type=="tool_result") - | {id: .tool_use_id, content: text_of(.content)} ] - | map({(.id): .content}) | add // {} ) as $results | - ( [ $uses[] | select(is_nav(.name)) ] ) as $nav | - ( $nav | length ) as $calls | - ( [ $nav[] - | ((.input | tostring | length) / 4) - + (((($results[.id]) // "") | length) / 4) - ] | add // 0 ) as $tokens | - "\($calls)\t\($tokens | floor)" - ' "$stream" 2>/dev/null || printf '0\t0\n' -} - -# lpm_usage_from_stream -# Emits "\t\t\t" from the terminal `result` event's -# .usage, falling back to the last assistant message usage. Zeros + "-" when absent. -lpm_usage_from_stream() { - local stream="$1" - if [ ! -f "$stream" ] || ! _lpm_have_jq; then - printf '0\t0\t0\t-\n'; return 0 - fi - jq -rs ' - [ .[] | select(type=="object") ] as $e | - ( [ $e[] | select(.type=="result") ] | last ) as $r | - ( [ $e[] | select(.type=="assistant") ] | last ) as $a | - ( $r.usage // $a.message.usage // {} ) as $u | - ( $r.model // $a.message.model // "-" ) as $m | - "\(($u.input_tokens // 0))\t\(($u.cache_read_input_tokens // 0))\t\(($u.output_tokens // 0))\t\($m)" - ' "$stream" 2>/dev/null || printf '0\t0\t0\t-\n' -} - -# lpm_quality_from_log -# Emits "\t" from kind:"finding_verification" records. -# false_positives = records whose outcome marks the finding downgraded/removed -# (outcome matching removed|downgrade|false_positive|refuted, case-insensitive). -# "0\t0" when the log is absent or has no verification records. -lpm_quality_from_log() { - local log="$1" - if [ ! -f "$log" ] || ! _lpm_have_jq; then - printf '0\t0\n'; return 0 - fi - jq -rs ' - [ .[] | select(type=="object") | select((.kind // "") == "finding_verification") ] as $v | - ( $v | length ) as $findings | - ( [ $v[] | (.outcome // "" | ascii_downcase) - | select(test("removed|downgrade|false[_ ]?positive|refuted")) ] | length ) as $fp | - "\($findings)\t\($fp)" - ' "$log" 2>/dev/null || printf '0\t0\n' -} - -# lpm_coldstart_from_log -# Emits the cold-start seconds from the (last) kind:"lsp_cold_start" record, or the -# literal "null" when none exists (the lsp-off control, matching the frozen baseline). -lpm_coldstart_from_log() { - local log="$1" - if [ ! -f "$log" ] || ! _lpm_have_jq; then - printf 'null\n'; return 0 - fi - jq -rs ' - ( [ .[] | select(type=="object") | select((.kind // "") == "lsp_cold_start") ] | last ) as $c | - if $c == null then "null" else (($c.cold_start_ms // 0) / 1000) end - ' "$log" 2>/dev/null || printf 'null\n' -} - -# lpm_record — assemble the pilot JSON record from already-extracted scalars. -# Args (all by flagless position): pr variant candidate model input cache output -# nav_tokens tool_calls findings false_positives cold_start_s wall_time_s -lpm_record() { - local pr="$1" variant="$2" candidate="$3" model="$4" input="$5" cache="$6" output="$7" - local nav_tokens="$8" tool_calls="$9" findings="${10}" false_positives="${11}" - local cold_start_s="${12}" wall_time_s="${13}" - _lpm_have_jq || { echo "[lsp-pilot-measure] ERROR: jq required" >&2; return 2; } - jq -cn \ - --arg pr "$pr" --arg variant "$variant" --arg candidate "$candidate" --arg model "$model" \ - --argjson input "${input:-0}" --argjson cache "${cache:-0}" --argjson output "${output:-0}" \ - --argjson nav_tokens "${nav_tokens:-0}" --argjson tool_calls "${tool_calls:-0}" \ - --argjson findings "${findings:-0}" --argjson false_positives "${false_positives:-0}" \ - --argjson cold_start_s "${cold_start_s:-null}" --argjson wall_time_s "${wall_time_s:-0}" \ - '{ - pr: $pr, variant: $variant, candidate: $candidate, model: $model, - input_tokens: $input, cache_read_tokens: $cache, output_tokens: $output, - nav_tokens: $nav_tokens, tool_calls: $tool_calls, - findings: $findings, false_positives: $false_positives, - cold_start_s: $cold_start_s, wall_time_s: $wall_time_s - }' -} - -# --------------------------------------------------------------------------- -# main -# --------------------------------------------------------------------------- - -main() { - # Pre-flight: jq drives every parser; fail loud + early rather than letting the - # helpers silently return zeros on a box without it. - _lpm_have_jq || { echo "[lsp-pilot-measure] ERROR: jq is required but not installed" >&2; exit 1; } - - local stream="" pr="" variant="lsp-on" candidate="candidate" model="" - local wall_time_s="0" cold_start_ms="" token_log="" - local nav_prefixes="${NAV_TOOL_PREFIXES:-$NAV_TOOL_PREFIXES_DEFAULT}" - - # ${2?…} makes a value-flag with no argument fail loud under set -u (rather than - # crashing on an unbound $2); shift 2 is then only reached when $2 exists. - while [ "$#" -gt 0 ]; do - case "$1" in - --pr) pr="${2?--pr requires an argument}"; shift 2 ;; - --variant) variant="${2?--variant requires an argument}"; shift 2 ;; - --candidate) candidate="${2?--candidate requires an argument}"; shift 2 ;; - --model) model="${2?--model requires an argument}"; shift 2 ;; - --wall-time-s) wall_time_s="${2?--wall-time-s requires an argument}"; shift 2 ;; - --cold-start-ms) cold_start_ms="${2?--cold-start-ms requires an argument}"; shift 2 ;; - --token-log) token_log="${2?--token-log requires an argument}"; shift 2 ;; - --nav-prefixes) nav_prefixes="${2?--nav-prefixes requires an argument}"; shift 2 ;; - -h|--help) sed -n '1,40p' "$0"; exit 0 ;; - --*) echo "[lsp-pilot-measure] unknown flag: $1" >&2; exit 2 ;; - *) stream="$1"; shift ;; - esac - done - - if [ -z "$stream" ] || [ -z "$pr" ]; then - echo "usage: $0 --pr [--variant ..] [--candidate ..] [--model ..] [--wall-time-s ..] [--cold-start-ms ..] [--token-log ..]" >&2 - exit 2 - fi - - local calls nav_tokens input cache output stream_model findings fp cold_start_s - IFS=$'\t' read -r calls nav_tokens < <(lpm_nav_from_stream "$stream" "$nav_prefixes") - IFS=$'\t' read -r input cache output stream_model < <(lpm_usage_from_stream "$stream") - IFS=$'\t' read -r findings fp < <(lpm_quality_from_log "$token_log") - - # cold-start: an explicit --cold-start-ms wins (the runner knows it directly); - # else read it from the token log; else null for the lsp-off control. - if [ -n "$cold_start_ms" ]; then - case "$cold_start_ms" in - ''|*[!0-9]*) cold_start_s="null" ;; - *) cold_start_s="$(awk "BEGIN { printf \"%.1f\", $cold_start_ms / 1000 }")" ;; - esac - else - cold_start_s="$(lpm_coldstart_from_log "$token_log")" - fi - [ "$variant" = "lsp-off" ] && cold_start_s="null" - - [ -n "$model" ] || model="$stream_model" - - lpm_record "$pr" "$variant" "$candidate" "$model" \ - "$input" "$cache" "$output" "$nav_tokens" "$calls" \ - "$findings" "$fp" "$cold_start_s" "$wall_time_s" -} - -# Run main only when executed directly (sourcing for unit tests must not run it). -if [ "${BASH_SOURCE[0]:-$0}" = "$0" ]; then - main "$@" -fi diff --git a/scripts/lsp_pilot_report.sh b/scripts/lsp_pilot_report.sh deleted file mode 100644 index b2d87dcae..000000000 --- a/scripts/lsp_pilot_report.sh +++ /dev/null @@ -1,436 +0,0 @@ -#!/usr/bin/env bash -# lsp_pilot_report.sh — LSP-pilot comparative report renderer (#844, epic #839). -# -# Phase 3 of the LSP pilot. It does NOT introduce any new review-engine behaviour: -# it CONSUMES the story-2 comparison harness (scripts/lsp_pilot_compare.sh) and -# renders ONE report across every candidate LSP server against the FROZEN LSP-off -# baseline (evals/lsp-pilot/holdout/baseline-lsp-off.jsonl). The baseline is read, -# never re-derived or mutated — re-deriving it would void the overfitting guard. -# -# On top of the per-candidate harness (which gives per-PR + aggregate nav tokens, -# tool calls, ET, USD, findings, false-positives and a quality verdict) this layer -# adds the three things the epic success metric needs that the harness alone does -# not produce: -# -# * cold-start P50/P95 vs the 30s P95 SLA, per candidate (AC2); -# * a per-candidate success-metric GO/NO-GO verdict — a candidate is a GO only -# when it delivers a >=2x navigation-token reduction AND no review-quality -# (false-positive) regression AND a cold-start P95 within the 30s SLA. A token -# win that costs precision, or that breaches the SLA, is a no-go (AC3). The -# data-favored candidate is named ONLY when >=2 candidates are competitive -# (both GO) — never on a token win alone (AC3); -# * honest annotation of SLA auto-skipped / degraded-MCP runs and a corpus -# coverage smoke check, so a partial corpus can never read as a clean sweep -# ("No silent caps" — AC4). -# -# Candidate-run record shape: a story-2 pilot record (see scripts/lsp_pilot_compare.sh) -# with two OPTIONAL honesty fields a run may carry: -# lsp_skipped true when the SLA auto-skip fired for this run (cold-start > SLA; -# the review fell back to base navigation, so nav_tokens ~ baseline) -# mcp_degraded true when the MCP/LSP server failed to connect/init for this run -# (the existing "Fail Loud, Never Fake" degradation path) -# skip_reason human-readable reason carried alongside either flag -# -# Layout (mirrors scripts/lsp_pilot_compare.sh): the lpr_* / render_* functions are -# PURE (no network) and unit-tested in tests/lsp_pilot_report.bats; main() only -# resolves paths and prints. -# -# Usage: -# bash scripts/lsp_pilot_report.sh \ -# [ ...] - -set -euo pipefail - -# Reuse the story-2 harness (render_lsp_comparison, _lp_aggregate, _lp_prs) and, -# transitively, the dated pricing helpers and USD/int formatters. Sourcing it does -# not run its main() (guarded by the BASH_SOURCE check at its foot). -_LPR_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -# shellcheck source=scripts/lsp_pilot_compare.sh -source "${_LPR_DIR}/lsp_pilot_compare.sh" - -# The cold-start P95 SLA in seconds (docs/lsp-pilot.md §3). Overridable for tests. -LSP_SLA_SECONDS="${LSP_SLA_SECONDS:-30}" -# The success-metric navigation-token reduction target (docs/lsp-pilot.md §4). -LSP_TOKEN_TARGET="${LSP_TOKEN_TARGET:-2.0}" - -# --------------------------------------------------------------------------- -# Pure helpers -# --------------------------------------------------------------------------- - -# lpr_coldstart_stats -# Emits one TSV line "p50p95maxn" over the non-null cold_start_s -# values in a candidate run (nearest-rank percentiles, one decimal). When there is -# no cold-start data (e.g. the LSP-off baseline) it emits "NANANA0". -# Pure: reads only the JSONL. -lpr_coldstart_stats() { - if [ "$#" -lt 1 ]; then - echo "[lsp-pilot] ERROR: lpr_coldstart_stats requires a candidate JSONL file path" >&2 - return 2 - fi - local jsonl="$1" - if [ ! -f "$jsonl" ]; then - printf 'NA\tNA\tNA\t0\n' - return 0 - fi - jq -r 'select(type=="object") | select((.kind // "token_usage") == "token_usage") - | .cold_start_s | select(. != null)' "$jsonl" 2>/dev/null \ - | sort -n \ - | awk ' - { v[NR] = $1 } - END { - n = NR - if (n == 0) { printf "NA\tNA\tNA\t0\n"; exit 0 } - # Nearest-rank: rank = ceil(p * n), 1-based, clamped to [1, n]. - p50 = int(0.50 * n); if (p50 < 0.50 * n) p50++ - p95 = int(0.95 * n); if (p95 < 0.95 * n) p95++ - if (p50 < 1) p50 = 1; if (p50 > n) p50 = n - if (p95 < 1) p95 = 1; if (p95 > n) p95 = n - printf "%.1f\t%.1f\t%.1f\t%d\n", v[p50], v[p95], v[n], n - }' -} - -# lpr_within_sla [sla_seconds] -# Returns 0 when the cold-start P95 is within the SLA, 1 when it breaches the SLA, -# and 2 when there is no cold-start data to judge (a coverage gap, not a pass). -lpr_within_sla() { - if [ "$#" -lt 1 ]; then - echo "[lsp-pilot] ERROR: lpr_within_sla requires a candidate JSONL file path" >&2 - return 2 - fi - local jsonl="$1" sla="${2:-$LSP_SLA_SECONDS}" stats p95 n - stats="$(lpr_coldstart_stats "$jsonl")" - p95="$(printf '%s' "$stats" | cut -f2)" - n="$(printf '%s' "$stats" | cut -f4)" - [ "${n:-0}" -gt 0 ] || return 2 - awk -v p="$p95" -v s="$sla" 'BEGIN { exit !(p <= s) }' -} - -# lpr_skip_annotations -# Emits "prtypereason" for every run that auto-skipped the LSP step -# (SLA breach) or ran with a degraded MCP server, so a coverage gap is never -# silently dropped (AC4). type is "sla-skip" or "mcp-degraded". Pure. -lpr_skip_annotations() { - if [ "$#" -lt 1 ]; then - echo "[lsp-pilot] ERROR: lpr_skip_annotations requires a candidate JSONL file path" >&2 - return 2 - fi - local jsonl="$1" - [ -f "$jsonl" ] || return 0 - jq -r 'select(type=="object") | select((.kind // "token_usage") == "token_usage") - | select((.lsp_skipped == true) or (.mcp_degraded == true)) - | [ (.pr // "unknown"), - (if (.lsp_skipped == true) then "sla-skip" else "mcp-degraded" end), - (.skip_reason // "(no reason given)") ] | @tsv' "$jsonl" 2>/dev/null \ - | sort -u -} - -# lpr_coverage -# Smoke check (AC4): for every PR in the frozen corpus, emit "prstatus" where -# status is COVERED, "SKIPPED:" (an SLA-skip / degraded run — present but -# without LSP enrichment), or MISSING (no run captured at all). Returns 1 if any -# corpus PR is MISSING, so a partial corpus is loud rather than silent. -lpr_coverage() { - if [ "$#" -lt 2 ]; then - echo "[lsp-pilot] ERROR: lpr_coverage requires corpus and candidate arguments" >&2 - return 2 - fi - local corpus="$1" candidate="$2" - [ -f "$corpus" ] || { echo "[lsp-pilot] ERROR: corpus cases file not found: $corpus" >&2; return 2; } - - local annotations corpus_prs cand_prs pr status missing=0 - annotations="$(lpr_skip_annotations "$candidate")" - corpus_prs="$(jq -r 'select(type=="object") - | "\(.repo)#\(.pr_number)@\(.head_sha)"' "$corpus" 2>/dev/null | sort -u)" - cand_prs="$(_lp_prs "$candidate")" - - while IFS= read -r pr; do - [ -n "$pr" ] || continue - if ! printf '%s\n' "$cand_prs" | grep -Fxq "$pr"; then - printf '%s\tMISSING\n' "$pr" - missing=1 - continue - fi - local ann - ann="$(printf '%s\n' "$annotations" | awk -F'\t' -v p="$pr" '$1 == p { print $2": "$3; exit }')" - if [ -n "$ann" ]; then - status="SKIPPED:${ann}" - else - status="COVERED" - fi - printf '%s\t%s\n' "$pr" "$status" - done <<< "$corpus_prs" - - return "$missing" -} - -# _lpr_total_nav -# Prints "baseline_navcandidate_nav" summed over the PRs the candidate covers -# (reusing the harness aggregate so the numbers match render_lsp_comparison). Pure. -_lpr_total_nav() { - if [ "$#" -lt 2 ]; then - echo "[lsp-pilot] ERROR: _lpr_total_nav requires baseline and candidate arguments" >&2 - return 2 - fi - local baseline="$1" candidate="$2" bagg cagg - bagg="$(_lp_aggregate "$baseline")" - cagg="$(_lp_aggregate "$candidate")" - awk -F'\t' ' - NR == FNR { bnav[$1] = $2; next } - { pr = $1; if (pr in bnav) { tb += bnav[pr]; tc += $2 } } - END { printf "%.1f\t%.1f\n", tb + 0, tc + 0 } - ' <(printf '%s\n' "$bagg") <(printf '%s\n' "$cagg") -} - -# lpr_token_ratio -# Prints the aggregate navigation-token reduction ratio baseline/candidate to two -# decimals (0.00 when the candidate's nav total is 0). Pure. -lpr_token_ratio() { - if [ "$#" -lt 2 ]; then - echo "[lsp-pilot] ERROR: lpr_token_ratio requires baseline and candidate arguments" >&2 - return 2 - fi - local nav; nav="$(_lpr_total_nav "$1" "$2")" - awk -F'\t' '{ if ($2 > 0) printf "%.2f\n", $1 / $2; else printf "0.00\n" }' <<< "$nav" -} - -# lpr_quality_regressed -# Returns 1 when the candidate regresses review quality on any covered PR (its -# mean false-positive count exceeds the frozen baseline's — precision dropped), -# else 0. This is the same precision proxy the story-2 harness enforces. Pure. -lpr_quality_regressed() { - if [ "$#" -lt 2 ]; then - echo "[lsp-pilot] ERROR: lpr_quality_regressed requires baseline and candidate arguments" >&2 - return 2 - fi - local baseline="$1" candidate="$2" bagg cagg - bagg="$(_lp_aggregate "$baseline")" - cagg="$(_lp_aggregate "$candidate")" - # _lp_aggregate column 5 is mean false_positives. - awk -F'\t' ' - NR == FNR { bfp[$1] = $5; next } - { pr = $1; if ((pr in bfp) && ($5 > bfp[pr] + 1e-9)) regressed = 1 } - END { exit (regressed ? 1 : 0) } - ' <(printf '%s\n' "$bagg") <(printf '%s\n' "$cagg") -} - -# lpr_candidate_verdict [name] [sla_seconds] -# Prints the success-metric verdict on its first word — "GO" or "NO-GO" — followed -# by the deciding reasons, and returns 0 for GO / 1 for NO-GO (AC3). A GO requires -# ALL of: nav-token reduction >= the target, no quality regression, cold-start P95 -# within the SLA. Pure. -lpr_candidate_verdict() { - if [ "$#" -lt 2 ]; then - echo "[lsp-pilot] ERROR: lpr_candidate_verdict requires baseline and candidate arguments" >&2 - return 2 - fi - local baseline="$1" candidate="$2" name="${3:-candidate}" sla="${4:-$LSP_SLA_SECONDS}" - local ratio target="$LSP_TOKEN_TARGET" reasons=() go=1 - - ratio="$(lpr_token_ratio "$baseline" "$candidate")" - if awk -v r="$ratio" -v t="$target" 'BEGIN { exit !(r + 0 >= t + 0) }'; then - reasons+=("token reduction ${ratio}x ≥ ${target}x target") - else - go=0 - reasons+=("token reduction ${ratio}x below ${target}x target") - fi - - if lpr_quality_regressed "$baseline" "$candidate"; then - reasons+=("no quality regression (false positives ≤ baseline)") - else - go=0 - reasons+=("quality REGRESSION (more false positives than baseline)") - fi - - local stats p95 n - stats="$(lpr_coldstart_stats "$candidate")" - p95="$(printf '%s' "$stats" | cut -f2)" - n="$(printf '%s' "$stats" | cut -f4)" - if [ "${n:-0}" -le 0 ]; then - go=0 - reasons+=("no cold-start data to verify the ${sla}s P95 SLA") - elif awk -v p="$p95" -v s="$sla" 'BEGIN { exit !(p + 0 <= s + 0) }'; then - reasons+=("cold-start P95 ${p95}s within the ${sla}s SLA") - else - go=0 - reasons+=("cold-start P95 ${p95}s breaches the ${sla}s SLA") - fi - - local verdict joined="" r - verdict=$([ "$go" -eq 1 ] && echo "GO" || echo "NO-GO") - for r in "${reasons[@]}"; do - if [ -n "$joined" ]; then joined="$joined; $r"; else joined="$r"; fi - done - printf '%s — %s\n' "$verdict" "$joined" - return $((1 - go)) -} - -# --------------------------------------------------------------------------- -# render_pilot_report — the single multi-candidate comparison report -# --------------------------------------------------------------------------- - -# render_pilot_report [...] -# Writes the full Markdown report to stdout. Pure (no network). Returns 0. -render_pilot_report() { - if [ "$#" -lt 3 ]; then - echo "[lsp-pilot] ERROR: render_pilot_report requires baseline, corpus, and at least one candidate" >&2 - return 2 - fi - local baseline="$1" corpus="$2"; shift 2 - - printf '# 🔬 LSP pilot — comparative speed / quality / cost report\n\n' - printf '_Each candidate LSP-on run scored against the **immutable LSP-off baseline** ' - printf '(`evals/lsp-pilot/holdout/baseline-lsp-off.jsonl`) over the frozen corpus ' - printf '(`evals/lsp-pilot/holdout/cases.jsonl`). The baseline is consumed, never ' - printf 're-derived._\n\n' - - printf '## Success metric (verbatim, epic #839)\n\n' - printf '> Go = on the frozen pilot PR corpus, LSP-on deep-tier review delivers a measurable\n' - printf '> navigation-token reduction (**target ≥2x fewer navigation tool-call tokens** vs the\n' - printf '> LSP-off control) AND **no regression in review quality** (false-positive / precision\n' - printf '> no worse than the frozen LSP-off baseline), achieved **within the cold-start SLA\n' - printf '> (≤30s P95)**. A win on tokens that costs precision is a _no-go_, not a win.\n\n' - - # Accumulate per-candidate verdict facts for the cross-candidate summary. - local names=() ratios=() verdicts=() p95s=() go_names=() - local spec name run - - for spec in "$@"; do - name="${spec%%=*}" - run="${spec#*=}" - names+=("$name") - - printf '## Candidate: `%s`\n\n' "$name" - - if [ ! -f "$run" ]; then - printf '⚠️ **No run artifact found** at `%s` — candidate not scored.\n\n' "$run" - ratios+=("0.00"); p95s+=("NA"); verdicts+=("NO-GO") - continue - fi - - # 1) Speed / cost / quality deltas — straight from the story-2 harness. - local cmp rc=0 - cmp="$(render_lsp_comparison "$baseline" "$run" "$name")" || rc=$? - printf '%s\n\n' "$cmp" - if [ "$rc" -ne 0 ]; then - # The harness already printed an INCOMPLETE banner; record a NO-GO and move on. - ratios+=("0.00"); p95s+=("NA"); verdicts+=("NO-GO") - continue - fi - - # 2) Cold-start vs the SLA (AC2). - local stats p50 p95 cmax cn - stats="$(lpr_coldstart_stats "$run")" - IFS=$'\t' read -r p50 p95 cmax cn <<< "$stats" - printf '### Cold start vs the %ss P95 SLA\n\n' "$LSP_SLA_SECONDS" - if [ "${cn:-0}" -le 0 ]; then - printf -- '- No cold-start samples captured (no server launched on any run).\n\n' - else - local sla_word - if lpr_within_sla "$run"; then sla_word="✅ within SLA"; else sla_word="❌ SLA breach"; fi - printf -- '- **P50:** %ss · **P95:** %ss · **max:** %ss over %s sample(s) — %s (SLA %ss)\n\n' \ - "$p50" "$p95" "$cmax" "$cn" "$sla_word" "$LSP_SLA_SECONDS" - fi - - # 3) Coverage + skipped/degraded honesty (AC4). - printf '### Coverage & skipped / degraded runs\n\n' - local cov cov_rc=0 - cov="$(lpr_coverage "$corpus" "$run")" || cov_rc=$? - printf '| Corpus PR | Status |\n|---|---|\n' - while IFS=$'\t' read -r pr status; do - [ -n "$pr" ] || continue - printf '| `%s` | %s |\n' "$pr" "$status" - done <<< "$cov" - printf '\n' - if [ "$cov_rc" -ne 0 ]; then - printf -- '- ⚠️ **Coverage gap:** at least one corpus PR has no captured run (MISSING above). ' - printf 'A partial corpus is not a clean sweep.\n\n' - fi - local skips - skips="$(lpr_skip_annotations "$run")" - if [ -n "$skips" ]; then - printf -- '- Annotated skipped / degraded runs (not dropped):\n' - while IFS=$'\t' read -r pr typ reason; do - [ -n "$pr" ] || continue - printf -- ' - `%s` — **%s**: %s\n' "$pr" "$typ" "$reason" - done <<< "$skips" - printf '\n' - else - printf -- '- No SLA auto-skip or MCP degradation on this candidate.\n\n' - fi - - # 4) Success-metric verdict (AC3). - local verdict vrc=0 ratio - verdict="$(lpr_candidate_verdict "$baseline" "$run" "$name")" || vrc=$? - ratio="$(lpr_token_ratio "$baseline" "$run")" - printf '### Success-metric verdict\n\n' - printf -- '- %s\n\n' "$verdict" - - ratios+=("$ratio"); p95s+=("$p95") - if [ "$vrc" -eq 0 ]; then - verdicts+=("GO"); go_names+=("$name") - else - verdicts+=("NO-GO") - fi - done - - # Cross-candidate summary + data-favored pick (only when >=2 candidates are GO). - printf '## Cross-candidate summary\n\n' - printf '| Candidate | Nav reduction | Cold-start P95 | Verdict |\n|---|---:|---:|:--|\n' - local i - for i in "${!names[@]}"; do - local p95cell="${p95s[$i]}" - [ "$p95cell" = "NA" ] || p95cell="${p95cell}s" - printf '| `%s` | %sx | %s | %s |\n' \ - "${names[$i]}" "${ratios[$i]}" "$p95cell" "${verdicts[$i]}" - done - printf '\n' - - if [ "${#go_names[@]}" -eq 0 ]; then - printf -- '- **No candidate meets the success metric** — no go on token reduction, quality, and SLA together.\n' - elif [ "${#go_names[@]}" -eq 1 ]; then - printf -- '- **%s** is the only candidate meeting the success metric; no head-to-head pick is needed.\n' \ - "${go_names[0]}" - else - # Two or more are genuinely competitive — name the data-favored one by the - # largest nav-token reduction (tie-break: lower cold-start P95). - local fav="" fav_ratio="-1" fav_p95="999999" idx - for idx in "${!names[@]}"; do - [ "${verdicts[$idx]}" = "GO" ] || continue - local r="${ratios[$idx]}" p="${p95s[$idx]}" - [ "$p" = "NA" ] && p="999999" - if awk -v r="$r" -v fr="$fav_ratio" -v p="$p" -v fp="$fav_p95" \ - 'BEGIN { exit !(r > fr + 1e-9 || (r > fr - 1e-9 && p < fp)) }'; then - fav="${names[$idx]}"; fav_ratio="$r"; fav_p95="$p" - fi - done - printf -- '- %s candidates are competitive (both meet the metric). **Data favored: %s** — largest navigation-token reduction (%sx), tie-broken on cold-start P95.\n' \ - "${#go_names[@]}" "$fav" "$fav_ratio" - fi - printf '\n' - - return 0 -} - -# --------------------------------------------------------------------------- -# main -# --------------------------------------------------------------------------- - -main() { - for cmd in jq awk; do - if ! command -v "$cmd" >/dev/null 2>&1; then - echo "[lsp-pilot] ERROR: Required dependency '$cmd' is not installed." >&2 - exit 1 - fi - done - - if [ "$#" -lt 3 ]; then - echo "usage: $0 [ ...]" >&2 - exit 2 - fi - render_pilot_report "$@" -} - -# Only run main when executed directly (not when sourced by tests). -if [ "${BASH_SOURCE[0]}" = "${0}" ]; then - main "$@" -fi diff --git a/scripts/lsp_pilot_run.sh b/scripts/lsp_pilot_run.sh deleted file mode 100644 index a3f03c06f..000000000 --- a/scripts/lsp_pilot_run.sh +++ /dev/null @@ -1,187 +0,0 @@ -#!/usr/bin/env bash -# lsp_pilot_run.sh — execute the LSP pilot A/B on real PRs and render the comparison -# (#844, epic #839). The CI-side driver that ties the pieces together: -# -# for each PR: fetch diff ─┬─► claude review LSP-OFF ─► lsp_pilot_measure ─► baseline.jsonl -# └─► claude review LSP-ON ─► lsp_pilot_measure ─► candidate.jsonl -# then: lsp_pilot_compare.sh baseline.jsonl candidate.jsonl ─► $GITHUB_STEP_SUMMARY -# -# Both sides are measured LIVE on the same real PRs with the same prompt and model — -# only the toolset differs (LSP-off navigates with Grep/Glob/Read/Bash; LSP-on adds -# the mcp__lsp__* navigation allowlist + the pinned MCP config). Generating both -# sides live means the comparison does NOT depend on the frozen synthetic baseline: -# it is a real, self-contained A/B. -# -# Honest scope: this is a navigation-COST probe (nav_tokens, tool_calls, cold-start, -# ET/USD), not the full review pipeline. The quality proxy (findings/false_positives) -# is only populated when the verification step runs (scripts/review-one-pr.sh, story -# #843); a bare claude invocation emits no finding_verification records, so those -# fields are 0 here and the cost metrics are the signal. -# -# Requires (provided by the CI job): claude (authed via CLAUDE_CODE_OAUTH_TOKEN), gh -# (authed via GH_TOKEN), jq, and — for the LSP-ON leg — agent-lsp + bash-language- -# server already installed by scripts/setup-lsp-pilot.sh (which also wrote the -# lsp_cold_start record to TOKEN_LOG_FILE). -# -# Pure helpers (lpr_*) are unit-tested in tests/dev-lead/unit/test_lsp_pilot_run.bats; -# the claude/gh calls live only in run_variant()/main() and are exercised in CI. - -set -euo pipefail - -_LPR_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -REPO_SLUG="${REPO_SLUG:-petry-projects/.github-private}" -MODEL="${LSP_PILOT_MODEL:-claude-sonnet-4-6}" # cheaper tier by default to bound smoke cost -CANDIDATE="${LSP_CANDIDATE:-agent-lsp}" -LSP_MCP_CONFIG="${LSP_PILOT_CONFIG:-.github/mcp/lsp.json}" -DEEP_TIMEOUT_SEC="${LSP_PILOT_TIMEOUT_SEC:-600}" -OUTDIR="${LSP_PILOT_OUTDIR:-lsp-pilot-out}" - -# Navigation allowlists. Off = the textual-navigation baseline; On = base + the -# pilot's read-only LSP navigation tools (sourced from setup-lsp-pilot.sh so the two -# stay in lockstep — never hand-duplicated). -LPR_BASE_TOOLS="Bash,Read,Grep,Glob" - -# --------------------------------------------------------------------------- -# Pure helpers -# --------------------------------------------------------------------------- - -# lpr_on_tools — base navigation tools + the pilot LSP allowlist (comma-joined). -lpr_on_tools() { - local nav - nav="$(bash "${_LPR_DIR}/setup-lsp-pilot.sh" print-allowed-tools 2>/dev/null | tr -d '[:space:]')" - if [ -n "$nav" ]; then printf '%s,%s' "$LPR_BASE_TOOLS" "$nav"; else printf '%s' "$LPR_BASE_TOOLS"; fi -} - -# lpr_build_prompt — the navigation-heavy review prompt. -# Deliberately steers the model to VERIFY cross-file claims (the work LSP nav -# replaces grep for), so the run actually exercises navigation. -lpr_build_prompt() { - local pr="$1" diff_file="$2" - cat </dev/null) ------ END DIFF ----- -PROMPT -} - -# lpr_wall_seconds — float seconds, 1 dp, clamped >=0. -# Falls back gracefully if nanoseconds are unavailable (the literal N survives). -lpr_wall_seconds() { - local a="${1:-0}" b="${2:-0}" - awk -v a="$a" -v b="$b" 'BEGIN { - if (a !~ /^[0-9]+$/ || b !~ /^[0-9]+$/) { print "0.0"; exit } - d = (b - a) / 1000000000.0; if (d < 0) d = 0; printf "%.1f", d - }' -} - -# lpr_now_ns — epoch nanoseconds (mawk-safe; %N intact ⇒ "0" tail is harmless to the -# clamp). Isolated so tests can stub timing. -lpr_now_ns() { date +%s%N 2>/dev/null || echo 0; } - -# --------------------------------------------------------------------------- -# Impure: one variant of one PR (claude + gh). CI-only. -# --------------------------------------------------------------------------- - -# run_variant -# Runs one claude review (stream-json), measures it, and APPENDS the pilot record to -# /.jsonl. Echoes the record path. -run_variant() { - local pr="$1" pr_key="$2" variant="$3" diff_file="$4" outdir="$5" - local stream prompt tools record t0 t1 wall rc=0 - stream="${outdir}/${pr}.${variant}.stream.jsonl" - prompt="${outdir}/${pr}.${variant}.prompt.txt" - lpr_build_prompt "$pr" "$diff_file" > "$prompt" - - local -a mcp_flags=() - if [ "$variant" = "lsp-on" ]; then - tools="$(lpr_on_tools)" - mcp_flags=(--mcp-config "$LSP_MCP_CONFIG" --strict-mcp-config) - else - tools="$LPR_BASE_TOOLS" - fi - - t0="$(lpr_now_ns)" - timeout "$DEEP_TIMEOUT_SEC" claude --print --model "$MODEL" \ - --output-format stream-json --verbose \ - --permission-mode acceptEdits \ - --allowed-tools "$tools" \ - ${mcp_flags[@]+"${mcp_flags[@]}"} \ - < "$prompt" > "$stream" 2>"${stream}.err" || rc=$? - t1="$(lpr_now_ns)" - wall="$(lpr_wall_seconds "$t0" "$t1")" - if [ "$rc" -ne 0 ]; then - echo "::warning::[lsp-pilot] claude ${variant} for PR #${pr} exited ${rc} (see ${stream}.err) — measuring partial transcript" >&2 - fi - - local out_file cand - if [ "$variant" = "lsp-on" ]; then out_file="${outdir}/candidate.jsonl"; cand="$CANDIDATE" - else out_file="${outdir}/baseline.jsonl"; cand="baseline"; fi - - record="$(bash "${_LPR_DIR}/lsp_pilot_measure.sh" "$stream" \ - --pr "$pr_key" --variant "$variant" --candidate "$cand" --model "$MODEL" \ - --wall-time-s "$wall" --token-log "${TOKEN_LOG_FILE:-/dev/null}")" - printf '%s\n' "$record" >> "$out_file" - printf '%s\n' "$out_file" -} - -# --------------------------------------------------------------------------- -# main -# --------------------------------------------------------------------------- - -main() { - local prs="${1:-${LSP_PILOT_PRS:-}}" - if [ -z "$prs" ]; then - echo "usage: $0 (or set LSP_PILOT_PRS)" >&2 - exit 2 - fi - command -v claude >/dev/null 2>&1 || { echo "::error::claude CLI not found" >&2; exit 1; } - command -v gh >/dev/null 2>&1 || { echo "::error::gh CLI not found" >&2; exit 1; } - - mkdir -p "$OUTDIR" - : > "${OUTDIR}/baseline.jsonl" - : > "${OUTDIR}/candidate.jsonl" - - local pr head_sha pr_key diff_file - IFS=',' read -ra _prs <<< "$prs" - for pr in "${_prs[@]}"; do - pr="$(printf '%s' "$pr" | tr -d '[:space:]')" - [ -n "$pr" ] || continue - echo "[lsp-pilot] === PR #${pr} ===" - head_sha="$(gh pr view "$pr" --repo "$REPO_SLUG" --json headRefOid -q .headRefOid 2>/dev/null || echo "")" - [ -n "$head_sha" ] || { echo "::warning::[lsp-pilot] could not resolve head SHA for PR #${pr} — skipping" >&2; continue; } - pr_key="${REPO_SLUG}#${pr}@${head_sha}" - diff_file="${OUTDIR}/${pr}.diff" - if ! gh pr diff "$pr" --repo "$REPO_SLUG" > "$diff_file" 2>/dev/null; then - echo "::warning::[lsp-pilot] could not fetch diff for PR #${pr} — skipping" >&2; continue - fi - run_variant "$pr" "$pr_key" "lsp-off" "$diff_file" "$OUTDIR" >/dev/null - run_variant "$pr" "$pr_key" "lsp-on" "$diff_file" "$OUTDIR" >/dev/null - done - - echo "[lsp-pilot] rendering comparison" - # Capture the harness's exit code instead of masking it with `|| true`: the - # comparison FAILS LOUD (non-zero) when a candidate PR has no baseline counterpart, - # and the CI step must surface that — while still writing the summary either way. - local report compare_rc=0 - report="$(bash "${_LPR_DIR}/lsp_pilot_compare.sh" "${OUTDIR}/baseline.jsonl" "${OUTDIR}/candidate.jsonl" "$CANDIDATE")" || compare_rc=$? - printf '%s\n' "$report" - if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then printf '%s\n' "$report" >> "$GITHUB_STEP_SUMMARY"; fi - return "$compare_rc" -} - -if [ "${BASH_SOURCE[0]:-$0}" = "$0" ]; then - main "$@" -fi diff --git a/scripts/review-one-pr.sh b/scripts/review-one-pr.sh index 1394d2a88..3a6c1a72c 100755 --- a/scripts/review-one-pr.sh +++ b/scripts/review-one-pr.sh @@ -51,12 +51,6 @@ source "$SCRIPT_DIR/lib/downstream-impact.sh" # Gated behind SAFETY_CHECKS_ENABLED (default ON; off => byte-identical prompt). # shellcheck source=lib/safety-checks.sh source "$SCRIPT_DIR/lib/safety-checks.sh" -# LSP finding-verification (epic #839, story #843): apply_lsp_verification grounds -# deep/audit cross-file findings against LSP nav tools, downgrading/annotating the -# ones LSP cannot ground and emitting each outcome to the token JSONL. Inert when -# the LSP MCP server is unwired or degraded (review unchanged). -# shellcheck source=lib/lsp-verification.sh -source "$SCRIPT_DIR/lib/lsp-verification.sh" PR_URL="${1:?usage: review-one-pr.sh }" export PR_URL @@ -109,29 +103,6 @@ PR_HEAD_SHA=$(echo "$PR_SNAPSHOT" | jq -r '.headRefOid') export PR_HEAD_SHA echo " head SHA: $PR_HEAD_SHA" -# LSP pilot (issue #960, story #844): when the recording flag is on, capture this -# real production review as ONE pilot record. engine.sh tees the deep/audit/duck -# stream-json transcripts into a per-PR stream dir; on exit (any path), the EXIT -# trap folds them into one kind:"lsp_pilot_run" record on TOKEN_LOG_FILE via -# lpe_emit_record. Strict no-op off-pilot — the consumer repos never set the flag. -if [ "${LSP_PILOT_ENABLED:-}" = "true" ]; then - # shellcheck source=lsp_pilot_emit.sh - source "$SCRIPT_DIR/lsp_pilot_emit.sh" - LSP_PILOT_STREAM_DIR="$(mktemp -d 2>/dev/null || true)" - export LSP_PILOT_STREAM_DIR - _LSP_PILOT_T0="$(date +%s%N 2>/dev/null || echo 0)" - [[ "$_LSP_PILOT_T0" == *N ]] && _LSP_PILOT_T0="${_LSP_PILOT_T0%N}000000000" - _lsp_pilot_on_exit() { - local _t1 _wall - _t1="$(date +%s%N 2>/dev/null || echo 0)" - [[ "$_t1" == *N ]] && _t1="${_t1%N}000000000" - _wall="$(lpe_wall_seconds "${_LSP_PILOT_T0:-0}" "$_t1")" - lpe_emit_record "${LSP_PILOT_STREAM_DIR:-}" "$PR_URL" "${PR_HEAD_SHA:-}" "$_wall" || true - [ -n "${LSP_PILOT_STREAM_DIR:-}" ] && rm -rf "$LSP_PILOT_STREAM_DIR" 2>/dev/null || true - } - trap _lsp_pilot_on_exit EXIT -fi - CI_STATUS=$(compute_ci_status "$(jq '.statusCheckRollup' <<< "$PR_SNAPSHOT")") echo " CI status: $CI_STATUS" @@ -936,13 +907,6 @@ fi # Wait for duck to finish (deep succeeded) [ -n "$DUCK_PID" ] && wait $DUCK_PID || true -# LSP finding-verification (story #843): ground the deep tier's cross-file -# findings before they are synthesized/posted. Inert (no change) unless the LSP -# MCP server is wired and connected; the deep CLI output is scanned so a degraded -# server skips verification. Operates in place on deep.json. -apply_lsp_verification "$OUTPUT_FILE" "deep" \ - /tmp/cascade/deep-stdout.txt /tmp/cascade/deep.log || true - DEEP_DECISION=$(jq -r '.decision' "$OUTPUT_FILE") DEEP_RISK=$(jq -r '.risk' "$OUTPUT_FILE") echo " [tier2] deep: decision=$DEEP_DECISION risk=$DEEP_RISK" @@ -1072,12 +1036,6 @@ if [ ! -s "$OUTPUT_FILE" ] || ! jq empty "$OUTPUT_FILE" 2>/dev/null; then exit 1 fi -# LSP finding-verification (story #843): ground the audit tier's cross-file -# findings before the final verdict is posted. Inert unless the LSP MCP server is -# wired and connected (audit CLI output scanned for a degraded server). -apply_lsp_verification "$OUTPUT_FILE" "audit" \ - /tmp/cascade/audit-stdout.txt /tmp/cascade/audit.log || true - AUDIT_DECISION=$(jq -r '.decision' "$OUTPUT_FILE") AUDIT_RISK=$(jq -r '.risk' "$OUTPUT_FILE") echo " [tier3] decision=$AUDIT_DECISION risk=$AUDIT_RISK" diff --git a/scripts/setup-lsp-pilot.sh b/scripts/setup-lsp-pilot.sh deleted file mode 100755 index 67d9cf9cb..000000000 --- a/scripts/setup-lsp-pilot.sh +++ /dev/null @@ -1,302 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail -# setup-lsp-pilot.sh — opt-in runner setup for the LSP-MCP review pilot. -# Epic #839 (LSP pilot), story #842. Contract: docs/lsp-pilot.md. -# -# Installs the pinned LSP-MCP server binary for the selected candidate plus the -# pinned `bash-language-server` (the pilot's Shell language server), then threads -# the opt-in MCP knobs into the job environment so engine.sh enriches the -# deep/audit (and rubber-duck) review tiers — triage is never touched (see -# scripts/engine.sh `_mcp_review_flags`). -# -# Fail Loud, Never Fake: if a pinned tool cannot be installed, this emits a -# single `::warning::` and SKIPS wiring the knobs (the review continues on the -# model's base capabilities). It never exits non-zero — a missing tool must not -# fail the workflow (AC #2). The committed config (.github/mcp/lsp.json) stays -# inert until REVIEW_MCP_CONFIG references it, so an off-pilot job is byte-for- -# byte unchanged (AC #4). -# -# Usage: -# setup-lsp-pilot.sh install + wire (the CI entry point) -# setup-lsp-pilot.sh print-allowed-tools print the navigation allowlist, exit -# -# Pins are LOOKED UP from the source of truth and recorded in docs/lsp-pilot.md -# §5 — never guessed (CLAUDE.md / AGENTS.md). Override via the env vars below for -# a candidate bump without editing this script. - -# Server name in .github/mcp/lsp.json is "lsp", so its tools are mcp__lsp__*. -# Canonical navigation/read-only allowlist for the pilot (AC #3): kept to 8-12 -# tools to bound MCP tool-definition token overhead (~200 tokens/tool/turn). -# These are exactly the finding-verification surface from docs/lsp-pilot.md §2 -# (references + diagnostics) and close navigation kin (definition/hover/symbols). -# Deliberately read-only: NO editing/refactoring tools (rename, apply_edit, -# insert_*, safe_delete, replace_symbol, format_*, execute_command) are exposed — -# the pilot verifies findings, it does not mutate code. -LSP_PILOT_ALLOWED_TOOLS="\ -mcp__lsp__find_references,\ -mcp__lsp__go_to_definition,\ -mcp__lsp__go_to_type_definition,\ -mcp__lsp__go_to_implementation,\ -mcp__lsp__go_to_declaration,\ -mcp__lsp__get_diagnostics,\ -mcp__lsp__inspect_symbol,\ -mcp__lsp__list_symbols,\ -mcp__lsp__find_symbol,\ -mcp__lsp__explore_symbol" - -# Committed, candidate-switchable MCP config (relative to the repo root the -# review job checks out). Point REVIEW_MCP_CONFIG here to enable the pilot. -LSP_PILOT_CONFIG="${LSP_PILOT_CONFIG:-.github/mcp/lsp.json}" - -# Selected candidate server (docs/lsp-pilot.md §5). Only the candidate that wins -# the comparison (#844) is carried past Phase 2; the cost cap bounds the pilot to -# <=2 candidate servers. agent-lsp is the wired default. -LSP_CANDIDATE="${LSP_CANDIDATE:-agent-lsp}" - -# Pinned versions (locked in #842; recorded in docs/lsp-pilot.md §5). -AGENT_LSP_VERSION="${AGENT_LSP_VERSION:-v0.15.0}" -BASH_LANGUAGE_SERVER_VERSION="${BASH_LANGUAGE_SERVER_VERSION:-5.6.0}" -AGENT_LSP_REPO="${AGENT_LSP_REPO:-blackwell-systems/agent-lsp}" - -# Where downloaded binaries land. On the runner this is added to PATH for later -# steps via $GITHUB_PATH. -INSTALL_BIN="${INSTALL_BIN:-$HOME/.local/bin}" - -# Cold-start SLA (story #846): toolchain bring-up (install / actions/cache restore -# → ready) must complete within this budget; over it the LSP wiring is auto-skipped -# (graceful degradation — never a workflow failure, docs/lsp-pilot.md §3). The -# index cache is what keeps a warm run under budget. Overridable for a tuning bump. -LSP_COLD_START_SLA_MS="${LSP_COLD_START_SLA_MS:-30000}" - -# Best-effort: the cold-start metric channel (Token Cost Observatory). The review -# job always has the repo checked out, so this resolves; if absent (odd invocation) -# emit_lsp_coldstart_record falls back to a no-op below so the script stays -# self-contained. -if [ -f "$(dirname "${BASH_SOURCE[0]}")/lib/token-metrics.sh" ]; then - # shellcheck source=scripts/lib/token-metrics.sh - source "$(dirname "${BASH_SOURCE[0]}")/lib/token-metrics.sh" -fi -if ! declare -f emit_lsp_coldstart_record >/dev/null 2>&1; then - emit_lsp_coldstart_record() { :; } -fi - -warn() { echo "::warning::[lsp-pilot] $*" >&2; } -note() { echo "[lsp-pilot] $*"; } - -# _lsp_now_ms — current epoch in milliseconds. Falls back to second resolution on -# platforms where `date +%N` is unsupported (the literal `N` is left intact there). -_lsp_now_ms() { - local ns - ns="$(date +%s%N 2>/dev/null || echo '')" - case "$ns" in - # %.0f, not %d: epoch ms (~1.75e12) overflows awk's 32-bit %d conversion on - # common awks (mawk) and would yield a garbage cold-start measurement. - ''|*[!0-9]*) date +%s 2>/dev/null | awk '{ printf "%.0f", $1 * 1000 }' ;; - *) awk "BEGIN { printf \"%.0f\", $ns / 1000000 }" ;; - esac -} - -# _lsp_elapsed_ms — non-negative integer delta. Any non-numeric -# input or a backwards clock yields 0 (a glitch must not look like a slow start). -_lsp_elapsed_ms() { - local a="${1:-0}" b="${2:-0}" d - case "$a$b" in ''|*[!0-9]*) echo 0; return 0 ;; esac - d=$(( b - a )) - [ "$d" -lt 0 ] && d=0 - echo "$d" -} - -# _lsp_sla_exceeded — true (0) when cold-start is over the -# SLA. Pure, for unit coverage. A non-numeric cold-start (clock glitch) is the safe -# default: NOT exceeded, so a measurement error never spuriously skips LSP. -_lsp_sla_exceeded() { - local cold="${1:-}" sla="${2:-}" - case "$cold" in ''|*[!0-9]*) return 1 ;; esac - # sla may be negative (tests use -1 to force the skip path); strip a leading - # minus, then require the remainder to be all digits. - case "${sla#-}" in ''|*[!0-9]*) return 1 ;; esac - [ "$cold" -gt "$sla" ] -} - -# _lsp_cache_status — normalize an actions/cache `cache-hit` output to -# hit/miss/unknown (true→hit, false→miss, unset/other→unknown). -_lsp_cache_status() { - case "${1:-}" in - true|hit) echo "hit" ;; - false|miss) echo "miss" ;; - *) echo "unknown" ;; - esac -} - -print_allowed_tools() { printf '%s\n' "$LSP_PILOT_ALLOWED_TOOLS"; } - -# install_bash_language_server — npm-global install of the pinned server. -# Returns 0 when the pinned binary is available (cache hit or fresh install). -install_bash_language_server() { - if command -v bash-language-server >/dev/null 2>&1; then - note "bash-language-server present: $(bash-language-server --version 2>/dev/null | head -1 || true)" - return 0 - fi - note "installing bash-language-server@${BASH_LANGUAGE_SERVER_VERSION}" - if ! command -v npm >/dev/null 2>&1; then - warn "npm unavailable — cannot install bash-language-server" - return 1 - fi - if npm install -g --prefix "$(dirname "$INSTALL_BIN")" "bash-language-server@${BASH_LANGUAGE_SERVER_VERSION}" >/dev/null 2>&1; then - command -v bash-language-server >/dev/null 2>&1 && return 0 - fi - warn "bash-language-server@${BASH_LANGUAGE_SERVER_VERSION} install failed" - return 1 -} - -# install_agent_lsp — download the pinned release tarball, verify its sha256 -# against the release checksums.txt, and extract the binary onto PATH. -install_agent_lsp() { - if command -v agent-lsp >/dev/null 2>&1; then - note "agent-lsp present: $(agent-lsp --version 2>/dev/null | head -1 || true)" - return 0 - fi - if ! command -v gh >/dev/null 2>&1; then - warn "gh unavailable — cannot download ${AGENT_LSP_REPO}@${AGENT_LSP_VERSION}" - return 1 - fi - - local arch asset workdir - case "$(uname -m)" in - x86_64|amd64) arch="amd64" ;; - aarch64|arm64) arch="arm64" ;; - *) warn "unsupported arch '$(uname -m)' for agent-lsp"; return 1 ;; - esac - asset="agent-lsp_linux_${arch}.tar.gz" - - workdir="$(mktemp -d)" - note "downloading ${AGENT_LSP_REPO}@${AGENT_LSP_VERSION} (${asset})" - if ! gh release download "$AGENT_LSP_VERSION" \ - --repo "$AGENT_LSP_REPO" \ - --pattern "$asset" \ - --pattern "checksums.txt" \ - --dir "$workdir" >/dev/null 2>&1; then - warn "release download failed for ${AGENT_LSP_REPO}@${AGENT_LSP_VERSION}" - rm -rf "$workdir" - return 1 - fi - - # Checksum verification (pins the artifact, not just the tag). - if [ -f "$workdir/checksums.txt" ] && command -v sha256sum >/dev/null 2>&1; then - if ! ( cd "$workdir" && grep -E "[ *]${asset}\$" checksums.txt | sha256sum -c - >/dev/null 2>&1 ); then - warn "checksum verification failed for ${asset} — refusing to install" - rm -rf "$workdir" - return 1 - fi - note "checksum verified for ${asset}" - else - warn "checksums.txt or sha256sum missing — cannot verify ${asset}, refusing to install" - rm -rf "$workdir" - return 1 - fi - - mkdir -p "$INSTALL_BIN" - if ! tar -xzf "$workdir/$asset" -C "$workdir" agent-lsp 2>/dev/null; then - # Some archives nest the binary; fall back to extracting all then locating it. - tar -xzf "$workdir/$asset" -C "$workdir" >/dev/null 2>&1 || true - fi - local bin - bin="$(find "$workdir" -type f -name agent-lsp -perm -u+x 2>/dev/null | head -1)" - if [ -z "$bin" ]; then - bin="$(find "$workdir" -type f -name agent-lsp 2>/dev/null | head -1)" - fi - if [ -z "$bin" ]; then - warn "agent-lsp binary not found in ${asset}" - rm -rf "$workdir" - return 1 - fi - install -m 0755 "$bin" "$INSTALL_BIN/agent-lsp" - rm -rf "$workdir" - command -v agent-lsp >/dev/null 2>&1 || export PATH="$INSTALL_BIN:$PATH" - [ -x "$INSTALL_BIN/agent-lsp" ] && return 0 - warn "agent-lsp install did not produce an executable" - return 1 -} - -# install_candidate — dispatch to the selected candidate's installer. -install_candidate() { - case "$LSP_CANDIDATE" in - agent-lsp) install_agent_lsp ;; - *) - warn "candidate '$LSP_CANDIDATE' is not wired in this pilot phase — skipping (see docs/lsp-pilot.md §5)" - return 1 - ;; - esac -} - -main() { - if [ ! -f "$LSP_PILOT_CONFIG" ]; then - warn "MCP config '$LSP_PILOT_CONFIG' not found — skipping LSP pilot setup" - return 0 - fi - - mkdir -p "$INSTALL_BIN" - if [ -n "${GITHUB_PATH:-}" ]; then - echo "$INSTALL_BIN" >> "$GITHUB_PATH" - fi - export PATH="$INSTALL_BIN:$PATH" - - # Time the toolchain bring-up (install / actions/cache restore → ready). This is - # the honest, shell-observable cold-start proxy: the language server's first-query - # indexing happens inside the claude subprocess and isn't visible here, but the - # bring-up window is exactly what the index cache (story #846) eliminates. The - # cache outcome is threaded in from the workflow's actions/cache step. - local cache_status t0 t1 cold_ms - cache_status="$(_lsp_cache_status "${LSP_INDEX_CACHE_HIT:-}")" - t0="$(_lsp_now_ms)" - local server_ok=1 bls_ok=1 - install_candidate || server_ok=0 - install_bash_language_server || bls_ok=0 - t1="$(_lsp_now_ms)" - cold_ms="$(_lsp_elapsed_ms "$t0" "$t1")" - - if [ "$server_ok" -ne 1 ] || [ "$bls_ok" -ne 1 ]; then - emit_lsp_coldstart_record "pr-review" "$LSP_CANDIDATE" "$cold_ms" "$cache_status" \ - "true" "$LSP_COLD_START_SLA_MS" "toolchain-unavailable" - warn "LSP toolchain unavailable (candidate=${LSP_CANDIDATE}) — review continues on base capabilities, LSP enrichment skipped" - return 0 - fi - - # Cold-start SLA auto-skip (AC #3): an over-budget bring-up degrades the - # enrichment, not the review — same "warn, don't fail" contract as a missing - # tool. The review proceeds on base capabilities; the workflow never fails. - if _lsp_sla_exceeded "$cold_ms" "$LSP_COLD_START_SLA_MS"; then - emit_lsp_coldstart_record "pr-review" "$LSP_CANDIDATE" "$cold_ms" "$cache_status" \ - "true" "$LSP_COLD_START_SLA_MS" "sla-exceeded" - warn "LSP cold-start ${cold_ms}ms exceeded SLA ${LSP_COLD_START_SLA_MS}ms (cache=${cache_status}) — skipping LSP wiring; review continues on base capabilities" - return 0 - fi - - emit_lsp_coldstart_record "pr-review" "$LSP_CANDIDATE" "$cold_ms" "$cache_status" \ - "false" "$LSP_COLD_START_SLA_MS" "ok" - note "LSP cold-start ${cold_ms}ms within SLA ${LSP_COLD_START_SLA_MS}ms (cache=${cache_status})" - - # Thread the opt-in knobs into the job env for the subsequent review step. - # engine.sh reads these; with them set it appends --mcp-config/--strict-mcp-config - # and merges the navigation tools into the deep/duck --allowed-tools (triage - # untouched). - if [ -n "${GITHUB_ENV:-}" ]; then - { - echo "REVIEW_MCP_CONFIG=${LSP_PILOT_CONFIG}" - echo "REVIEW_MCP_ALLOWED_TOOLS=${LSP_PILOT_ALLOWED_TOOLS}" - } >> "$GITHUB_ENV" - note "LSP pilot enabled: REVIEW_MCP_CONFIG=${LSP_PILOT_CONFIG}" - else - note "GITHUB_ENV unset — install complete; export REVIEW_MCP_CONFIG=${LSP_PILOT_CONFIG} to enable" - fi -} - -# Run the dispatcher only when executed directly — sourcing (e.g. the unit tests -# for the pure cold-start/SLA helpers) must not trigger install + wiring. -if [ "${BASH_SOURCE[0]:-$0}" = "$0" ]; then - case "${1:-}" in - print-allowed-tools) print_allowed_tools ;; - ""|setup) main ;; - *) echo "usage: $(basename "$0") [print-allowed-tools|setup]" >&2; exit 2 ;; - esac -fi diff --git a/tests/dev-lead/fixtures/engines/stub-claude b/tests/dev-lead/fixtures/engines/stub-claude index 3c9044bec..6cd4f091d 100755 --- a/tests/dev-lead/fixtures/engines/stub-claude +++ b/tests/dev-lead/fixtures/engines/stub-claude @@ -64,34 +64,6 @@ if [ "$output_format" = "json" ]; then printf '{"type":"result","result":"%s","usage":{"input_tokens":%s,"cache_read_input_tokens":%s,"cache_creation_input_tokens":%s,"output_tokens":%s}}\n' \ "$response" "${_in:-0}" "${_cr:-0}" "${_cw:-0}" "${_out:-0}" fi -elif [ "$output_format" = "stream-json" ]; then - # Mirror `claude --print --output-format stream-json --verbose`: an NDJSON - # transcript — one navigation tool_use + its tool_result, then a terminal - # `result` event carrying top-level .result + .usage (parsed by the engine and - # by lsp_pilot_measure.sh). Usage from STUB_CLAUDE_USAGE (same default as json). - read -r _in _cr _cw _out <<< "${STUB_CLAUDE_USAGE:-100 20 30 40}" - if command -v jq >/dev/null 2>&1; then - jq -cn '{type:"assistant", message:{content:[{type:"tool_use", id:"t1", name:"Grep", input:{pattern:"foo"}}]}}' - jq -cn '{type:"user", message:{content:[{type:"tool_result", tool_use_id:"t1", content:"scripts/x.sh:1:foo"}]}}' - jq -cn --arg result "$response" --arg model "${model:-claude-stub}" \ - --argjson i "${_in:-0}" --argjson cr "${_cr:-0}" \ - --argjson cw "${_cw:-0}" --argjson o "${_out:-0}" \ - '{type:"result", subtype:"success", is_error:false, result:$result, model:$model, - usage:{input_tokens:$i, cache_read_input_tokens:$cr, - cache_creation_input_tokens:$cw, output_tokens:$o}}' - else - _resp_json="${response//\\/\\\\}" - _resp_json="${_resp_json//\"/\\\"}" - _resp_json="${_resp_json//$'\n'/\\n}" - _model_json="${model:-claude-stub}" - _model_json="${_model_json//\\/\\\\}" - _model_json="${_model_json//\"/\\\"}" - _model_json="${_model_json//$'\n'/\\n}" - printf '{"type":"assistant","message":{"content":[{"type":"tool_use","id":"t1","name":"Grep","input":{"pattern":"foo"}}]}}\n' - printf '{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"t1","content":"scripts/x.sh:1:foo"}]}}\n' - printf '{"type":"result","subtype":"success","is_error":false,"result":"%s","model":"%s","usage":{"input_tokens":%s,"cache_read_input_tokens":%s,"cache_creation_input_tokens":%s,"output_tokens":%s}}\n' \ - "$_resp_json" "$_model_json" "${_in:-0}" "${_cr:-0}" "${_cw:-0}" "${_out:-0}" - fi else printf '%s\n' "$response" fi diff --git a/tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py b/tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py deleted file mode 100644 index 3d115de1d..000000000 --- a/tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py +++ /dev/null @@ -1,192 +0,0 @@ -#!/usr/bin/env python3 -"""Validate the LSP-pilot capture/decoupling plumbing in the pr-review workflows. - -Issue #1031 (story #844, epic #839) closes two gaps that left the LSP pilot inert -in real CI: - - * Gap 1 — the capture gate is never plumbed. engine.sh's `_lsp_pilot_active()` - and review-one-pr.sh's emit trap gate on the ENV var `LSP_PILOT_ENABLED`, but - the workflow only referenced the repo variable `vars.LSP_PILOT_ENABLED` - (never exported to the job env). This guards that pr-review.yml exports - `LSP_PILOT_ENABLED` to the review job so a production run actually captures a - transcript and emits a `lsp_pilot_run` record. - - * Gap 2 — capture and LSP wiring can't be decoupled. This guards that an - independent `lsp_pilot_variant` (on|off|none) control exists: `on` wires the - LSP MCP (lsp-on/B leg), `off` captures without wiring (lsp-off/A control leg), - and the two LSP-wiring steps are gated so `off` skips them. - - * AC #5 (CORRECTED after incident 2026-07-03 / PR #1048) — the ring-0 trigger - stub pr-review-trigger.yml pins the reusable at `@pr-review/next`. Forwarding - `lsp_pilot_variant` to that channel BEFORE it declares the input makes every - review `startup_fail` (an undeclared `with:` key is an invalid workflow). So - the stub must NOT declare/forward the input until `pr-review/next` is advanced - (via cut-release.sh) to a commit that declares it. This guard now enforces that - channel-safety invariant (stub must not forward ahead of the pinned channel); - the original "must forward" form (#1034) is what took production down. - -Off-pilot (neither var nor input set) must stay byte-for-byte unchanged (AC #4): -the exported env resolves to empty and the wiring steps stay skipped. - -Re-enable procedure for the stub-driven A/B: (1) advance `pr-review/next` to a -commit that declares the reusable `lsp_pilot_variant` input, (2) re-add the stub -dispatch input + forwarding, and (3) flip check_trigger below back to requiring -them (see git history at #1034 for the requiring form). -""" -from __future__ import annotations - -import sys - -try: - import yaml -except ImportError: - yaml = None - -REUSABLE = ".github/workflows/pr-review.yml" -TRIGGER = ".github/workflows/pr-review-trigger.yml" -INPUT = "lsp_pilot_variant" - - -def _load(path: str): - with open(path, encoding="utf-8") as fh: - return yaml.safe_load(fh) - - -def _on(doc: dict | None): - if not doc: - return {} - # PyYAML parses the bare `on:` key as the boolean True (YAML 1.1). - return doc.get("on", doc.get(True)) or {} - - -def _review_job(doc: dict | None) -> dict: - jobs = (doc or {}).get("jobs") or {} - job = jobs.get("review") - return job if isinstance(job, dict) else {} - - -def _steps(job: dict) -> list: - steps = job.get("steps") or [] - return [s for s in steps if isinstance(s, dict)] - - -def check_reusable(fails: list[str]) -> None: - doc = _load(REUSABLE) - if not doc: - fails.append(f"{REUSABLE}: failed to parse YAML (document is None/empty)") - return - on = _on(doc) - - # workflow_call input declared so a trigger stub can forward it. - wc_inputs = ((on.get("workflow_call") or {}).get("inputs")) or {} - if INPUT not in wc_inputs: - fails.append(f"{REUSABLE}: workflow_call.inputs.{INPUT} is not declared") - - job = _review_job(doc) - env = job.get("env") or {} - - enabled = str(env.get("LSP_PILOT_ENABLED", "")) - if not enabled: - fails.append(f"{REUSABLE}: review job env does not export LSP_PILOT_ENABLED (Gap 1)") - else: - # The capture gate must turn on for the legacy repo var AND for a - # dispatched/defaulted on|off variant. - for needle in ("vars.LSP_PILOT_ENABLED", "lsp_pilot_variant", "'on'", "'off'"): - if needle not in enabled: - fails.append( - f"{REUSABLE}: LSP_PILOT_ENABLED expression is missing {needle!r} " - f"(got: {enabled!r})" - ) - - variant = str(env.get("LSP_PILOT_VARIANT", "")) - if not variant: - fails.append(f"{REUSABLE}: review job env does not export LSP_PILOT_VARIANT (Gap 2)") - else: - if "lsp_pilot_variant" not in variant: - fails.append( - f"{REUSABLE}: LSP_PILOT_VARIANT must resolve from inputs.{INPUT} " - f"(got: {variant!r})" - ) - - # The two LSP-wiring steps must run only for the legacy var OR variant 'on' - # — never for the 'off' control leg (that's the decoupling). - wiring_ids = {"lsp-cache"} - wiring_names = {"Set up LSP pilot servers (opt-in)"} - found = 0 - for step in _steps(job): - is_wiring = step.get("id") in wiring_ids or step.get("name") in wiring_names - if not is_wiring: - continue - found += 1 - cond = str(step.get("if", "")) - if "== 'on'" not in cond: - fails.append( - f"{REUSABLE}: LSP-wiring step {step.get('name') or step.get('id')!r} " - f"'if' does not gate on the 'on' variant (got: {cond!r})" - ) - if "vars.LSP_PILOT_ENABLED == 'true'" not in cond: - fails.append( - f"{REUSABLE}: LSP-wiring step {step.get('name') or step.get('id')!r} " - f"'if' dropped the legacy vars.LSP_PILOT_ENABLED gate (got: {cond!r})" - ) - if found < 2: - fails.append( - f"{REUSABLE}: expected both LSP-wiring steps (cache + setup), found {found}" - ) - - -def check_trigger(fails: list[str]) -> None: - # CHANNEL-SKEW SAFETY (incident 2026-07-03, PR #1048). The ring-0 stub pins the - # reusable at `@pr-review/next`. A `with:` key the pinned reusable doesn't - # declare is an invalid workflow → every review `startup_fail`s. The stub must - # therefore NOT declare or forward `lsp_pilot_variant` until that channel is - # advanced to a commit that declares it (then re-add both and flip this guard - # back — see the module docstring's re-enable procedure). - doc = _load(TRIGGER) - if not doc: - fails.append(f"{TRIGGER}: failed to parse YAML (document is None/empty)") - return - on = _on(doc) - - wd_inputs = ((on.get("workflow_dispatch") or {}).get("inputs")) or {} - if INPUT in wd_inputs: - fails.append( - f"{TRIGGER}: workflow_dispatch.inputs.{INPUT} must NOT be declared while the " - f"stub pins a channel that lacks it (channel-skew guard — see #1048)" - ) - - job = _review_job(doc) - with_block = job.get("with") or {} - if INPUT in with_block: - fails.append( - f"{TRIGGER}: review job must NOT forward {INPUT} to the pinned reusable — " - f"forwarding an input @pr-review/next does not declare startup_fails every " - f"review (channel-skew guard — see #1048)" - ) - - -def main() -> int: - if yaml is None: - print("FAIL: PyYAML is required (pip install pyyaml)", file=sys.stderr) - return 2 - - fails: list[str] = [] - for path, fn in ((REUSABLE, check_reusable), (TRIGGER, check_trigger)): - try: - fn(fails) - except FileNotFoundError: - fails.append(f"FAIL: {path} not found") - - if fails: - print("FAIL: LSP-pilot plumbing checks failed:") - for msg in fails: - print(f" - {msg}") - return 1 - - print("PASS: pr-review.yml exports the capture gate + variant and gates LSP wiring; " - "pr-review-trigger.yml does not forward lsp_pilot_variant ahead of the pinned channel") - return 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/tests/dev-lead/unit/test_engine_lsp_pilot.bats b/tests/dev-lead/unit/test_engine_lsp_pilot.bats deleted file mode 100644 index a43aa9050..000000000 --- a/tests/dev-lead/unit/test_engine_lsp_pilot.bats +++ /dev/null @@ -1,160 +0,0 @@ -#!/usr/bin/env bats -# Unit tests for engine.sh — gated LSP-pilot stream-json capture (issue #960, -# story #844, epic #839). -# -# When the pilot recording flag (LSP_PILOT_ENABLED=true) is on AND a capturing -# tier runs (deep/audit/single via run_agentic, or run_duck), the claude call -# must switch from `--output-format json` to `--output-format stream-json -# --verbose` and append the raw transcript to a per-call file under -# LSP_PILOT_STREAM_DIR — while still recording real token usage (parsed from the -# stream's terminal `result` event). When the flag is OFF, behaviour is -# byte-for-byte unchanged: `--output-format json`, no stream capture. The triage -# and action tiers never capture, even with the flag on. - -SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" -ENGINE_SCRIPT="$SCRIPT_DIR/scripts/engine.sh" -STUB_ENGINES_DIR="$SCRIPT_DIR/tests/dev-lead/fixtures/engines" - -setup() { - export GITHUB_ENV="$(mktemp)" - unset CLAUDE_TRIAGE_MODEL_CHAIN CLAUDE_DEEP_MODEL_CHAIN CLAUDE_AUDIT_MODEL_CHAIN - unset CLAUDE_ACTION_MODEL_CHAIN CLAUDE_SINGLE_MODEL_CHAIN - unset REVIEW_MCP_CONFIG REVIEW_MCP_ALLOWED_TOOLS REVIEW_MCP_CONFIG_DEFAULT_PATH REVIEW_MCP_DEBUG - unset LSP_PILOT_ENABLED LSP_PILOT_STREAM_DIR - - STUB_BIN_DIR="$(mktemp -d)" - cp "$STUB_ENGINES_DIR/stub-claude" "$STUB_BIN_DIR/claude" - chmod +x "$STUB_BIN_DIR/claude" - export PATH="$STUB_BIN_DIR:$PATH" - - TEST_PROMPT="$(mktemp)"; echo "test prompt content" > "$TEST_PROMPT" - ARGS_RECORD="$(mktemp)"; export STUB_ENGINE_RECORD_ARGS="$ARGS_RECORD" - TOKEN_LOG="$(mktemp)"; : > "$TOKEN_LOG" - STREAM_DIR="$(mktemp -d)" - - export DEV_LEAD_DRY_RUN="false" STUB_ENGINE_EXIT=0 -} - -teardown() { - rm -f "$GITHUB_ENV" "$TEST_PROMPT" "$ARGS_RECORD" "$TOKEN_LOG" - rm -rf "$STUB_BIN_DIR" "$STREAM_DIR" - unset STUB_ENGINE_RECORD_ARGS STUB_ENGINE_EXIT LSP_PILOT_ENABLED LSP_PILOT_STREAM_DIR -} - -_source_engine() { - export REVIEW_ENGINE="claude" - source "$ENGINE_SCRIPT" -} - -# ── Pilot OFF: byte-for-byte unchanged ─────────────────────────────────────── - -@test "off-pilot: deep tier + token log → --output-format json, no stream-json, no capture" { - _source_engine - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_STREAM_DIR="$STREAM_DIR" # set, but flag is off → ignored - run run_agentic "$TEST_PROMPT" "claude-opus-4-8" "deep" - [ "$status" -eq 0 ] - grep -q -- "--output-format json" "$ARGS_RECORD" - ! grep -q -- "stream-json" "$ARGS_RECORD" - # No transcript captured when the pilot flag is off. - [ -z "$(ls -A "$STREAM_DIR")" ] -} - -@test "off-pilot: no TOKEN_LOG_FILE → no output-format flag at all (legacy text path)" { - _source_engine - unset TOKEN_LOG_FILE - run run_agentic "$TEST_PROMPT" "claude-opus-4-8" "deep" - [ "$status" -eq 0 ] - ! grep -q -- "--output-format" "$ARGS_RECORD" -} - -# ── Pilot ON: capturing tiers switch to stream-json + write a transcript ────── - -@test "pilot deep: stream-json --verbose threaded, transcript captured, usage still recorded" { - _source_engine - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_ENABLED=true - export LSP_PILOT_STREAM_DIR="$STREAM_DIR" - export STUB_ENGINE_RESPONSE="pilot deep response" - export STUB_CLAUDE_USAGE="1500 400 0 90" - run run_agentic "$TEST_PROMPT" "claude-opus-4-8" "deep" - [ "$status" -eq 0 ] - [ "$output" = "pilot deep response" ] - grep -q -- "--output-format stream-json --verbose" "$ARGS_RECORD" - ! grep -q -- "--output-format json" "$ARGS_RECORD" - # A per-call transcript file was written under the stream dir. - [ -n "$(ls -A "$STREAM_DIR")" ] - # Real usage parsed from the stream's result event lands in the token record. - run jq -r 'select(.tier=="deep") | .input_tokens' "$TOKEN_LOG" - [ "$output" = "1500" ] -} - -@test "pilot duck: stream-json --verbose threaded and transcript captured" { - _source_engine - export DUCK_ENGINE="claude" - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_ENABLED=true - export LSP_PILOT_STREAM_DIR="$STREAM_DIR" - run run_duck "$TEST_PROMPT" "claude-sonnet-4-6" - [ "$status" -eq 0 ] - grep -q -- "--output-format stream-json --verbose" "$ARGS_RECORD" - [ -n "$(ls -A "$STREAM_DIR")" ] - # The duck's existing --max-turns flag must survive. - grep -q -- "--max-turns 25" "$ARGS_RECORD" -} - -@test "pilot audit: capturing tier switches to stream-json" { - _source_engine - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_ENABLED=true - export LSP_PILOT_STREAM_DIR="$STREAM_DIR" - run run_agentic "$TEST_PROMPT" "claude-fable-5" "audit" - [ "$status" -eq 0 ] - grep -q -- "--output-format stream-json --verbose" "$ARGS_RECORD" - [ -n "$(ls -A "$STREAM_DIR")" ] -} - -@test "pilot single: capturing tier switches to stream-json" { - _source_engine - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_ENABLED=true - export LSP_PILOT_STREAM_DIR="$STREAM_DIR" - run run_agentic "$TEST_PROMPT" "claude-sonnet-4-6" "single" - [ "$status" -eq 0 ] - grep -q -- "--output-format stream-json --verbose" "$ARGS_RECORD" - [ -n "$(ls -A "$STREAM_DIR")" ] -} - -# ── Pilot ON but non-capturing tiers stay on the unchanged path ────────────── - -@test "pilot triage: never captures (no stream-json, no transcript)" { - _source_engine - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_ENABLED=true - export LSP_PILOT_STREAM_DIR="$STREAM_DIR" - run run_triage "$TEST_PROMPT" - [ "$status" -eq 0 ] - ! grep -q -- "stream-json" "$ARGS_RECORD" - [ -z "$(ls -A "$STREAM_DIR")" ] -} - -@test "pilot action: writer/synthesis tier never captures (out of pilot scope)" { - _source_engine - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_ENABLED=true - export LSP_PILOT_STREAM_DIR="$STREAM_DIR" - run run_agentic "$TEST_PROMPT" "claude-sonnet-4-6" "action" - [ "$status" -eq 0 ] - ! grep -q -- "stream-json" "$ARGS_RECORD" - [ -z "$(ls -A "$STREAM_DIR")" ] -} - -@test "pilot deep but stream dir unset → stream-json still chosen, no crash" { - _source_engine - export TOKEN_LOG_FILE="$TOKEN_LOG" - export LSP_PILOT_ENABLED=true - unset LSP_PILOT_STREAM_DIR - run run_agentic "$TEST_PROMPT" "claude-opus-4-8" "deep" - [ "$status" -eq 0 ] - grep -q -- "--output-format stream-json --verbose" "$ARGS_RECORD" -} diff --git a/tests/dev-lead/unit/test_lsp_coldstart_sla.bats b/tests/dev-lead/unit/test_lsp_coldstart_sla.bats deleted file mode 100644 index 1fb201788..000000000 --- a/tests/dev-lead/unit/test_lsp_coldstart_sla.bats +++ /dev/null @@ -1,160 +0,0 @@ -#!/usr/bin/env bats -# Unit tests for the LSP cold-start SLA + index-cache wiring (epic #839, story #846). -# -# Builds on story #842's setup wiring — covers only what #846 adds on top: -# AC #1 — pr-review.yml caches the LSP toolchain/index (SHA-pinned actions/cache) -# and threads its cache-hit into the setup step. -# AC #2 — emit_lsp_coldstart_record writes a discriminated lsp_cold_start JSONL -# record (cold_start_ms, cache, skipped, sla_ms) to the Token Cost -# Observatory; cache status is normalized to hit/miss/unknown. -# AC #3 — an over-budget cold-start auto-skips LSP wiring via the "warn, don't -# fail" contract (no knobs, exit 0); a missing tool degrades the same way. - -SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" -SETUP_SCRIPT="$SCRIPT_DIR/scripts/setup-lsp-pilot.sh" -WORKFLOW="$SCRIPT_DIR/.github/workflows/pr-review.yml" - -# Source the script in a clean subshell so its `set -euo pipefail` never leaks into -# the bats test shell, and the source-guard keeps main() from running. -_call() { run bash -c 'source "$1"; shift; "$@"' bash "$SETUP_SCRIPT" "$@"; } - -# ── AC #3: pure SLA decision helper ─────────────────────────────────────────── - -@test "_lsp_sla_exceeded: over budget → exceeded (0)" { - _call _lsp_sla_exceeded 31000 30000 - [ "$status" -eq 0 ] -} - -@test "_lsp_sla_exceeded: at budget → not exceeded (1)" { - _call _lsp_sla_exceeded 30000 30000 - [ "$status" -eq 1 ] -} - -@test "_lsp_sla_exceeded: under budget → not exceeded (1)" { - _call _lsp_sla_exceeded 12000 30000 - [ "$status" -eq 1 ] -} - -@test "_lsp_sla_exceeded: non-numeric cold-start → safe default, not exceeded (1)" { - _call _lsp_sla_exceeded "oops" 30000 - [ "$status" -eq 1 ] -} - -@test "_lsp_sla_exceeded: negative SLA (test seam) is exceeded by any cold-start (0)" { - _call _lsp_sla_exceeded 0 -1 - [ "$status" -eq 0 ] -} - -@test "_lsp_sla_exceeded: non-numeric SLA → safe default, not exceeded (1)" { - _call _lsp_sla_exceeded 5 abc - [ "$status" -eq 1 ] -} - -# ── AC #2: cache-status normalization + elapsed ─────────────────────────────── - -@test "_lsp_cache_status: true→hit, false→miss, unset→unknown" { - _call _lsp_cache_status true; [ "$output" = "hit" ] - _call _lsp_cache_status false; [ "$output" = "miss" ] - _call _lsp_cache_status ""; [ "$output" = "unknown" ] -} - -@test "_lsp_elapsed_ms: forward delta, and a backwards clock clamps to 0" { - _call _lsp_elapsed_ms 1000 1500; [ "$output" = "500" ] - _call _lsp_elapsed_ms 1500 1000; [ "$output" = "0" ] -} - -# ── AC #2: the cold-start metric record ─────────────────────────────────────── - -@test "emit_lsp_coldstart_record: writes a well-formed lsp_cold_start record" { - local log; log="$(mktemp)" - run bash -c 'source "$1"; TOKEN_LOG_FILE="$2" emit_lsp_coldstart_record pr-review agent-lsp 1234 hit false 30000 ok' \ - bash "$SETUP_SCRIPT" "$log" - [ "$status" -eq 0 ] - run jq -e '.kind=="lsp_cold_start" and .cold_start_ms==1234 and .cache=="hit" and .skipped==false and .sla_ms==30000 and .reason=="ok"' "$log" - [ "$status" -eq 0 ] - rm -f "$log" -} - -@test "emit_lsp_coldstart_record: no-op when TOKEN_LOG_FILE unset" { - run bash -c 'source "$1"; unset TOKEN_LOG_FILE; emit_lsp_coldstart_record pr-review agent-lsp 1 hit false 30000 ok; echo done' \ - bash "$SETUP_SCRIPT" - [ "$status" -eq 0 ] - [ "$output" = "done" ] -} - -# ── AC #2/#3: end-to-end setup paths ────────────────────────────────────────── - -_make_setup_env() { - SANDBOX_BIN="$(mktemp -d)" - export GITHUB_ENV="$(mktemp)" - export TOKEN_LOG_FILE="$(mktemp)" - export PATH="$SANDBOX_BIN:$PATH" -} - -_teardown_setup_env() { - rm -rf "$SANDBOX_BIN" - rm -f "$GITHUB_ENV" "$TOKEN_LOG_FILE" - unset GITHUB_ENV TOKEN_LOG_FILE SANDBOX_BIN -} - -_stub_tools_present() { - printf '#!/usr/bin/env bash\necho agent-lsp 0.15.0\n' > "$SANDBOX_BIN/agent-lsp" - printf '#!/usr/bin/env bash\necho 5.6.0\n' > "$SANDBOX_BIN/bash-language-server" - chmod +x "$SANDBOX_BIN/agent-lsp" "$SANDBOX_BIN/bash-language-server" -} - -@test "setup: cold-start under SLA → knobs wired + record skipped:false" { - _make_setup_env - _stub_tools_present - export LSP_INDEX_CACHE_HIT=true - LSP_COLD_START_SLA_MS=30000 run bash "$SETUP_SCRIPT" - [ "$status" -eq 0 ] - grep -q "^REVIEW_MCP_CONFIG=.github/mcp/lsp.json$" "$GITHUB_ENV" - grep -q "^REVIEW_MCP_ALLOWED_TOOLS=mcp__lsp__" "$GITHUB_ENV" - run jq -e 'select(.kind=="lsp_cold_start") | .skipped==false and .reason=="ok" and .cache=="hit"' "$TOKEN_LOG_FILE" - [ "$status" -eq 0 ] - unset LSP_INDEX_CACHE_HIT - _teardown_setup_env -} - -@test "setup: cold-start over SLA → ::warning:: + no knobs + exit 0 + record skipped:true" { - _make_setup_env - _stub_tools_present - # SLA of -1ms is exceeded by any non-negative cold-start → deterministic skip. - LSP_COLD_START_SLA_MS=-1 run bash "$SETUP_SCRIPT" - [ "$status" -eq 0 ] - [[ "$output" == *"::warning::"* ]] - [[ "$output" == *"exceeded SLA"* ]] - ! grep -q "^REVIEW_MCP_CONFIG=" "$GITHUB_ENV" - ! grep -q "^REVIEW_MCP_ALLOWED_TOOLS=" "$GITHUB_ENV" - run jq -e 'select(.kind=="lsp_cold_start") | .skipped==true and .reason=="sla-exceeded"' "$TOKEN_LOG_FILE" - [ "$status" -eq 0 ] - _teardown_setup_env -} - -@test "setup: toolchain uninstallable → record skipped:true (toolchain-unavailable) + exit 0, no knobs" { - _make_setup_env - # No agent-lsp / bash-language-server on PATH; stub the installers to fail so - # nothing installs and no network is touched. - for cmd in npm gh curl uv; do - printf '#!/usr/bin/env bash\nexit 1\n' > "$SANDBOX_BIN/$cmd" - chmod +x "$SANDBOX_BIN/$cmd" - done - run bash "$SETUP_SCRIPT" - [ "$status" -eq 0 ] - [[ "$output" == *"::warning::"* ]] - ! grep -q "^REVIEW_MCP_CONFIG=" "$GITHUB_ENV" - run jq -e 'select(.kind=="lsp_cold_start") | .skipped==true and .reason=="toolchain-unavailable"' "$TOKEN_LOG_FILE" - [ "$status" -eq 0 ] - _teardown_setup_env -} - -# ── AC #1: workflow wiring ──────────────────────────────────────────────────── - -@test "workflow: pr-review.yml caches the LSP index (SHA-pinned) and threads cache-hit" { - # SHA-pinned actions/cache (the same SHA the Claude CLI cache uses), a Cache LSP - # index step, and the cache-hit output threaded into the setup step's env. - grep -q "name: Cache LSP index" "$WORKFLOW" - grep -q "actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae" "$WORKFLOW" - grep -q "LSP_INDEX_CACHE_HIT: \${{ steps.lsp-cache.outputs.cache-hit }}" "$WORKFLOW" -} diff --git a/tests/dev-lead/unit/test_lsp_pilot.bats b/tests/dev-lead/unit/test_lsp_pilot.bats deleted file mode 100644 index 286dbe050..000000000 --- a/tests/dev-lead/unit/test_lsp_pilot.bats +++ /dev/null @@ -1,202 +0,0 @@ -#!/usr/bin/env bats -# Unit tests for the LSP-MCP pilot wiring (epic #839, story #842). -# -# This pilot reuses the MCP plumbing that already shipped (#677/#678/#679 — see -# tests/dev-lead/unit/test_engine_mcp.bats). These tests cover only what #842 -# adds on top of it: -# AC #1 — a committed, candidate-switchable MCP config (.github/mcp/lsp.json) -# that engine.sh threads into deep/duck only (triage untouched). -# AC #2 — runner setup (scripts/setup-lsp-pilot.sh) installs the pinned tools -# and degrades gracefully (warning + skip) when a tool is absent. -# AC #3 — only a small (8-12) read-only navigation tool set is exposed. -# AC #4 — with the LSP config NOT referenced by REVIEW_MCP_CONFIG, the review -# is byte-for-byte unchanged (no LSP MCP flags threaded). - -SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" -ENGINE_SCRIPT="$SCRIPT_DIR/scripts/engine.sh" -LSP_CONFIG="$SCRIPT_DIR/.github/mcp/lsp.json" -SETUP_SCRIPT="$SCRIPT_DIR/scripts/setup-lsp-pilot.sh" -STUB_ENGINES_DIR="$SCRIPT_DIR/tests/dev-lead/fixtures/engines" - -setup() { - export GITHUB_ENV="$(mktemp)" - export GITHUB_OUTPUT="$(mktemp)" - - unset CLAUDE_TRIAGE_MODEL_CHAIN CLAUDE_DEEP_MODEL_CHAIN CLAUDE_AUDIT_MODEL_CHAIN - unset CLAUDE_ACTION_MODEL_CHAIN CLAUDE_SINGLE_MODEL_CHAIN - unset REVIEW_MCP_CONFIG REVIEW_MCP_ALLOWED_TOOLS REVIEW_MCP_CONFIG_DEFAULT_PATH - - STUB_BIN_DIR="$(mktemp -d)" - cp "$STUB_ENGINES_DIR/stub-claude" "$STUB_BIN_DIR/claude" - cp "$STUB_ENGINES_DIR/stub-gemini" "$STUB_BIN_DIR/gemini" - chmod +x "$STUB_BIN_DIR/claude" "$STUB_BIN_DIR/gemini" - export PATH="$STUB_BIN_DIR:$PATH" - export STUB_BIN_DIR - - TEST_PROMPT="$(mktemp)" - echo "test prompt content" > "$TEST_PROMPT" - export TEST_PROMPT - - ARGS_RECORD="$(mktemp)" - export STUB_ENGINE_RECORD_ARGS="$ARGS_RECORD" - - export DEV_LEAD_DRY_RUN="false" - export STUB_ENGINE_EXIT=0 -} - -teardown() { - rm -f "$GITHUB_ENV" "$GITHUB_OUTPUT" "$TEST_PROMPT" "$ARGS_RECORD" - rm -rf "$STUB_BIN_DIR" - unset STUB_ENGINE_RECORD_ARGS STUB_ENGINE_EXIT - unset REVIEW_MCP_CONFIG REVIEW_MCP_ALLOWED_TOOLS REVIEW_MCP_CONFIG_DEFAULT_PATH -} - -_source_engine() { - local engine="${1:-claude}" - export REVIEW_ENGINE="$engine" - source "$ENGINE_SCRIPT" -} - -# ── AC #1: the committed config is a valid, correctly-shaped LSP MCP config ──── - -@test "config: .github/mcp/lsp.json exists and is valid JSON" { - [ -f "$LSP_CONFIG" ] - run jq empty "$LSP_CONFIG" - [ "$status" -eq 0 ] -} - -@test "config: declares an 'lsp' stdio server launched via the agent-lsp binary" { - run jq -e '.mcpServers.lsp.type == "stdio"' "$LSP_CONFIG" - [ "$status" -eq 0 ] - run jq -r '.mcpServers.lsp.command' "$LSP_CONFIG" - [ "$status" -eq 0 ] - [ "$output" = "agent-lsp" ] -} - -@test "config: drives bash-language-server (the pilot's Shell language server)" { - run jq -e '[.mcpServers.lsp.args[]] | any(test("bash-language-server"))' "$LSP_CONFIG" - [ "$status" -eq 0 ] -} - -@test "config: carries no auth headers/secret (stdio binary, no smuggled token)" { - run jq -e '.mcpServers.lsp | has("headers") | not' "$LSP_CONFIG" - [ "$status" -eq 0 ] -} - -# ── AC #3: only a small (8-12) read-only navigation tool set is exposed ─────── - -@test "allowed-tools: setup script prints the canonical navigation allowlist" { - run bash "$SETUP_SCRIPT" print-allowed-tools - [ "$status" -eq 0 ] - [ -n "$output" ] - # Every entry must be an lsp MCP tool. - run bash -c "bash '$SETUP_SCRIPT' print-allowed-tools | tr ',' '\n' | grep -vc '^mcp__lsp__'" - [ "$output" = "0" ] -} - -@test "allowed-tools: navigation set is 8-12 tools (token-overhead bound)" { - local count - count="$(bash "$SETUP_SCRIPT" print-allowed-tools | tr ',' '\n' | grep -c .)" - [ "$count" -ge 8 ] - [ "$count" -le 12 ] -} - -@test "allowed-tools: navigation set is read-only — no editing/refactor tools" { - # Finding-verification is read-only (references/diagnostics/hover/symbols). - # No mutating tools (rename/apply_edit/insert/delete/format/execute_command). - run bash -c "bash '$SETUP_SCRIPT' print-allowed-tools | grep -Eic 'rename|apply_edit|insert_|safe_delete|replace_symbol|format_|execute_command'" - [ "$output" = "0" ] -} - -@test "allowed-tools: includes the in-scope find-references and diagnostics tools" { - run bash "$SETUP_SCRIPT" print-allowed-tools - [[ "$output" == *"mcp__lsp__find_references"* ]] - [[ "$output" == *"mcp__lsp__get_diagnostics"* ]] -} - -# ── AC #1: engine threads the lsp config into deep/duck, never triage ───────── - -@test "engine: lsp config referenced → deep tier threads its MCP flags" { - _source_engine "claude" - export REVIEW_MCP_CONFIG="$LSP_CONFIG" - export REVIEW_MCP_ALLOWED_TOOLS="$(bash "$SETUP_SCRIPT" print-allowed-tools)" - run run_agentic "$TEST_PROMPT" "claude-opus-4-8" "deep" - [ "$status" -eq 0 ] - grep -q -- "--mcp-config $LSP_CONFIG" "$ARGS_RECORD" - grep -q -- "--strict-mcp-config" "$ARGS_RECORD" - grep -q -- "mcp__lsp__find_references" "$ARGS_RECORD" -} - -@test "engine: lsp config referenced → triage tier still gets NO MCP flags" { - _source_engine "claude" - export REVIEW_MCP_CONFIG="$LSP_CONFIG" - export REVIEW_MCP_ALLOWED_TOOLS="$(bash "$SETUP_SCRIPT" print-allowed-tools)" - run run_triage "$TEST_PROMPT" - [ "$status" -eq 0 ] - ! grep -q -- "--mcp-config" "$ARGS_RECORD" - ! grep -q -- "--strict-mcp-config" "$ARGS_RECORD" - ! grep -q -- "--allowed-tools" "$ARGS_RECORD" - grep -q -- "--disallowed-tools" "$ARGS_RECORD" -} - -# ── AC #4: committing lsp.json is inert until explicitly referenced ─────────── - -@test "inert: the conventional auto-default path is NOT the lsp config" { - # engine.sh auto-defaults REVIEW_MCP_CONFIG only to .github/review-mcp.json, - # never to the LSP config — so merely committing lsp.json activates nothing. - _source_engine "claude" - [ "$REVIEW_MCP_CONFIG_DEFAULT_PATH" != "$LSP_CONFIG" ] - [ "$REVIEW_MCP_CONFIG_DEFAULT_PATH" = ".github/review-mcp.json" ] -} - -@test "inert: lsp config present but unreferenced → no LSP MCP flags threaded" { - # Point the conventional default away from any committed config and leave - # REVIEW_MCP_CONFIG unset: the deep call must be byte-for-byte the legacy shape. - export REVIEW_MCP_CONFIG_DEFAULT_PATH="$(mktemp -u)" - _source_engine "claude" - run run_agentic "$TEST_PROMPT" "claude-opus-4-8" "deep" - [ "$status" -eq 0 ] - ! grep -q -- "--mcp-config" "$ARGS_RECORD" - ! grep -q -- "mcp__lsp__" "$ARGS_RECORD" - grep -q -- "--allowed-tools Bash,Read,Grep,Glob" "$ARGS_RECORD" -} - -# ── AC #2: runner setup installs pinned tools, degrades gracefully ──────────── - -_make_setup_env() { - # Hermetic sandbox: an isolated bin dir on PATH + a fresh GITHUB_ENV file. - SANDBOX_BIN="$(mktemp -d)" - export GITHUB_ENV="$(mktemp)" - export PATH="$SANDBOX_BIN:$PATH" -} - -@test "setup: tools present → threads REVIEW_MCP_CONFIG + navigation knob into GITHUB_ENV" { - _make_setup_env - # Simulate the pinned tools already being installed (cache hit) so no network. - printf '#!/usr/bin/env bash\necho agent-lsp 0.15.0\n' > "$SANDBOX_BIN/agent-lsp" - printf '#!/usr/bin/env bash\necho 5.6.0\n' > "$SANDBOX_BIN/bash-language-server" - chmod +x "$SANDBOX_BIN/agent-lsp" "$SANDBOX_BIN/bash-language-server" - run bash "$SETUP_SCRIPT" - [ "$status" -eq 0 ] - grep -q "^REVIEW_MCP_CONFIG=.github/mcp/lsp.json$" "$GITHUB_ENV" - grep -q "^REVIEW_MCP_ALLOWED_TOOLS=mcp__lsp__" "$GITHUB_ENV" - rm -rf "$SANDBOX_BIN" -} - -@test "setup: a tool cannot be installed → ::warning:: + no knobs + exit 0 (graceful)" { - _make_setup_env - # No agent-lsp / bash-language-server on PATH; stub their installers to fail so - # nothing can be installed and no real network is touched. - for cmd in npm gh curl uv; do - printf '#!/usr/bin/env bash\nexit 1\n' > "$SANDBOX_BIN/$cmd" - chmod +x "$SANDBOX_BIN/$cmd" - done - run bash "$SETUP_SCRIPT" - # AC #2: never fails the workflow. - [ "$status" -eq 0 ] - [[ "$output" == *"::warning::"* ]] - # AC #2/#4: nothing wired when degraded → review stays on base capabilities. - ! grep -q "^REVIEW_MCP_CONFIG=" "$GITHUB_ENV" - ! grep -q "^REVIEW_MCP_ALLOWED_TOOLS=" "$GITHUB_ENV" - rm -rf "$SANDBOX_BIN" -} diff --git a/tests/dev-lead/unit/test_lsp_pilot_emit.bats b/tests/dev-lead/unit/test_lsp_pilot_emit.bats deleted file mode 100644 index fce4e39cd..000000000 --- a/tests/dev-lead/unit/test_lsp_pilot_emit.bats +++ /dev/null @@ -1,232 +0,0 @@ -#!/usr/bin/env bats -# Unit tests for scripts/lsp_pilot_emit.sh — the production-review pilot record -# emitter (epic #839, story #844, issue #960). It is the glue that turns a real -# pr-review (under the pilot flag) into ONE pilot-schema record on TOKEN_LOG_FILE: -# * the gate (LSP_PILOT_ENABLED), variant/candidate resolution, and pr join key, -# * emit_record: run lsp_pilot_measure.sh over the captured stream, tag the -# output with kind:"lsp_pilot_run" so token_report.sh keeps excluding it. -# The off-pilot path must be a strict no-op (the five consumer repos rely on it). - -SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" -EMIT="$SCRIPT_DIR/scripts/lsp_pilot_emit.sh" - -_call() { run bash -c 'source "$1"; shift; "$@"' bash "$EMIT" "$@"; } - -setup() { - unset LSP_PILOT_ENABLED LSP_PILOT_VARIANT REVIEW_MCP_CONFIG LSP_CANDIDATE TOKEN_LOG_FILE - STREAM_DIR="$(mktemp -d)" - TOKEN_LOG="$(mktemp)" - : > "$TOKEN_LOG" -} - -teardown() { - rm -rf "$STREAM_DIR" - rm -f "$TOKEN_LOG" -} - -# A minimal stream-json transcript: one navigation tool_use (Grep) + its result, -# then a terminal `result` event carrying .result text and real .usage. -_write_stream() { - local dest="$1" - cat > "$dest" <<'JSON' -{"type":"assistant","message":{"content":[{"type":"tool_use","id":"t1","name":"Grep","input":{"pattern":"foo"}}]}} -{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"t1","content":"scripts/x.sh:1:foo"}]}} -{"type":"result","subtype":"success","is_error":false,"result":"verdict","model":"claude-opus-4-8","usage":{"input_tokens":1200,"cache_read_input_tokens":300,"output_tokens":80}} -JSON -} - -# ── Gate ───────────────────────────────────────────────────────────────────── - -@test "lpe_pilot_active: true only when LSP_PILOT_ENABLED=true" { - LSP_PILOT_ENABLED=true _call lpe_pilot_active - [ "$status" -eq 0 ] -} - -@test "lpe_pilot_active: false when flag unset" { - _call lpe_pilot_active - [ "$status" -ne 0 ] -} - -@test "lpe_pilot_active: false for any non-true value" { - LSP_PILOT_ENABLED=1 _call lpe_pilot_active - [ "$status" -ne 0 ] - LSP_PILOT_ENABLED=yes _call lpe_pilot_active - [ "$status" -ne 0 ] -} - -# ── Variant / candidate ────────────────────────────────────────────────────── - -@test "lpe_variant: lsp-off when REVIEW_MCP_CONFIG unset" { - _call lpe_variant - [ "$output" = "lsp-off" ] -} - -@test "lpe_variant: lsp-on when REVIEW_MCP_CONFIG points at a readable file" { - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - REVIEW_MCP_CONFIG="$cfg" _call lpe_variant - [ "$output" = "lsp-on" ] -} - -@test "lpe_variant: lsp-off when REVIEW_MCP_CONFIG path is missing" { - REVIEW_MCP_CONFIG="/no/such/file" _call lpe_variant - [ "$output" = "lsp-off" ] -} - -@test "lpe_candidate: baseline for lsp-off, agent-lsp for lsp-on" { - _call lpe_candidate - [ "$output" = "baseline" ] - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - REVIEW_MCP_CONFIG="$cfg" _call lpe_candidate - [ "$output" = "agent-lsp" ] -} - -@test "lpe_candidate: honors LSP_CANDIDATE override on the lsp-on leg" { - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - REVIEW_MCP_CONFIG="$cfg" LSP_CANDIDATE="agent-lsp-next" _call lpe_candidate - [ "$output" = "agent-lsp-next" ] -} - -# ── Decoupling: explicit LSP_PILOT_VARIANT is authoritative (issue #1031) ───── -# The A/B legs must be selectable independently of REVIEW_MCP_CONFIG, because -# engine.sh auto-defaults REVIEW_MCP_CONFIG to the committed .github/review-mcp.json -# (the Context7 MCP) — so the lsp-off (A) control leg would otherwise be -# mislabelled lsp-on. LSP_PILOT_VARIANT=off|on overrides the detection. - -@test "lpe_variant: LSP_PILOT_VARIANT=off forces lsp-off even with a readable REVIEW_MCP_CONFIG" { - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - REVIEW_MCP_CONFIG="$cfg" LSP_PILOT_VARIANT=off _call lpe_variant - [ "$output" = "lsp-off" ] -} - -@test "lpe_variant: LSP_PILOT_VARIANT=on forces lsp-on even when REVIEW_MCP_CONFIG is unset" { - LSP_PILOT_VARIANT=on _call lpe_variant - [ "$output" = "lsp-on" ] -} - -@test "lpe_variant: LSP_PILOT_VARIANT=none/empty falls back to REVIEW_MCP_CONFIG detection" { - LSP_PILOT_VARIANT=none _call lpe_variant - [ "$output" = "lsp-off" ] - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - REVIEW_MCP_CONFIG="$cfg" LSP_PILOT_VARIANT=none _call lpe_variant - [ "$output" = "lsp-on" ] -} - -@test "lpe_candidate: follows the explicit variant (off→baseline, on→agent-lsp)" { - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - REVIEW_MCP_CONFIG="$cfg" LSP_PILOT_VARIANT=off _call lpe_candidate - [ "$output" = "baseline" ] - LSP_PILOT_VARIANT=on _call lpe_candidate - [ "$output" = "agent-lsp" ] -} - -# ── pr join key ────────────────────────────────────────────────────────────── - -@test "lpe_pr_key: builds #@ from a PR URL" { - _call lpe_pr_key "https://github.com/petry-projects/.github-private/pull/960" "deadbeef" - [ "$status" -eq 0 ] - [ "$output" = "petry-projects/.github-private#960@deadbeef" ] -} - -# ── wall seconds ───────────────────────────────────────────────────────────── - -@test "lpe_wall_seconds: positive delta → seconds (1dp); backwards → 0.0" { - _call lpe_wall_seconds 1000000000 4500000000 - [ "$output" = "3.5" ] - _call lpe_wall_seconds 5000000000 1000000000 - [ "$output" = "0.0" ] -} - -# ── emit_record ────────────────────────────────────────────────────────────── - -@test "lpe_emit_record: off-pilot is a strict no-op (no record appended)" { - _write_stream "$STREAM_DIR/stream.aaa" - unset LSP_PILOT_ENABLED - run bash -c 'source "$1"; shift; TOKEN_LOG_FILE="$1"; shift; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" \ - "$STREAM_DIR" "https://github.com/o/r/pull/5" "sha5" "12.0" - [ "$status" -eq 0 ] - [ ! -s "$TOKEN_LOG" ] -} - -@test "lpe_emit_record: active + transcript → exactly one kind:lsp_pilot_run record" { - _write_stream "$STREAM_DIR/stream.aaa" - run bash -c 'source "$1"; shift; export LSP_PILOT_ENABLED=true TOKEN_LOG_FILE="$1"; shift; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" \ - "$STREAM_DIR" "https://github.com/petry-projects/.github-private/pull/960" "abc123" "17.5" - [ "$status" -eq 0 ] - # exactly one line - [ "$(wc -l < "$TOKEN_LOG")" -eq 1 ] - run jq -r '.kind' "$TOKEN_LOG"; [ "$output" = "lsp_pilot_run" ] - run jq -r '.pr' "$TOKEN_LOG"; [ "$output" = "petry-projects/.github-private#960@abc123" ] - run jq -r '.variant' "$TOKEN_LOG"; [ "$output" = "lsp-off" ] - run jq -r '.candidate' "$TOKEN_LOG"; [ "$output" = "baseline" ] - run jq -r '.wall_time_s' "$TOKEN_LOG"; [ "$output" = "17.5" ] - run jq -r '.tool_calls' "$TOKEN_LOG"; [ "$output" = "1" ] - run jq -r '.input_tokens' "$TOKEN_LOG"; [ "$output" = "1200" ] -} - -@test "lpe_emit_record: lsp-on variant uses REVIEW_MCP_CONFIG + agent-lsp candidate" { - _write_stream "$STREAM_DIR/stream.aaa" - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - run bash -c 'source "$1"; shift; export LSP_PILOT_ENABLED=true TOKEN_LOG_FILE="$1" REVIEW_MCP_CONFIG="$2"; shift 2; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" "$cfg" \ - "$STREAM_DIR" "https://github.com/o/r/pull/7" "sha7" "9.0" - [ "$status" -eq 0 ] - run jq -r '.variant' "$TOKEN_LOG"; [ "$output" = "lsp-on" ] - run jq -r '.candidate' "$TOKEN_LOG"; [ "$output" = "agent-lsp" ] -} - -@test "lpe_emit_record: LSP_PILOT_VARIANT=off yields an lsp-off record even with REVIEW_MCP_CONFIG set (A-leg decoupling)" { - _write_stream "$STREAM_DIR/stream.aaa" - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - run bash -c 'source "$1"; shift; export LSP_PILOT_ENABLED=true LSP_PILOT_VARIANT=off TOKEN_LOG_FILE="$1" REVIEW_MCP_CONFIG="$2"; shift 2; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" "$cfg" \ - "$STREAM_DIR" "https://github.com/petry-projects/.github-private/pull/1031" "shaA" "6.0" - [ "$status" -eq 0 ] - [ "$(wc -l < "$TOKEN_LOG")" -eq 1 ] - run jq -r '.pr' "$TOKEN_LOG"; [ "$output" = "petry-projects/.github-private#1031@shaA" ] - run jq -r '.variant' "$TOKEN_LOG"; [ "$output" = "lsp-off" ] - run jq -r '.candidate' "$TOKEN_LOG"; [ "$output" = "baseline" ] -} - -@test "lpe_emit_record: LSP_PILOT_VARIANT=on yields an lsp-on/agent-lsp record (B leg, same PR key)" { - _write_stream "$STREAM_DIR/stream.aaa" - local cfg; cfg="$(mktemp "$STREAM_DIR/cfg.XXXXXX")"; echo '{}' > "$cfg" - run bash -c 'source "$1"; shift; export LSP_PILOT_ENABLED=true LSP_PILOT_VARIANT=on TOKEN_LOG_FILE="$1" REVIEW_MCP_CONFIG="$2"; shift 2; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" "$cfg" \ - "$STREAM_DIR" "https://github.com/petry-projects/.github-private/pull/1031" "shaA" "6.0" - [ "$status" -eq 0 ] - run jq -r '.pr' "$TOKEN_LOG"; [ "$output" = "petry-projects/.github-private#1031@shaA" ] - run jq -r '.variant' "$TOKEN_LOG"; [ "$output" = "lsp-on" ] - run jq -r '.candidate' "$TOKEN_LOG"; [ "$output" = "agent-lsp" ] -} - -@test "lpe_emit_record: empty stream dir (no review transcript) → no record" { - run bash -c 'source "$1"; shift; export LSP_PILOT_ENABLED=true TOKEN_LOG_FILE="$1"; shift; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" \ - "$STREAM_DIR" "https://github.com/o/r/pull/8" "sha8" "1.0" - [ "$status" -eq 0 ] - [ ! -s "$TOKEN_LOG" ] -} - -@test "lpe_emit_record: concatenates multiple per-call transcripts (sums nav tool_calls)" { - _write_stream "$STREAM_DIR/stream.aaa" - _write_stream "$STREAM_DIR/stream.bbb" - run bash -c 'source "$1"; shift; export LSP_PILOT_ENABLED=true TOKEN_LOG_FILE="$1"; shift; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" \ - "$STREAM_DIR" "https://github.com/o/r/pull/9" "sha9" "2.0" - [ "$status" -eq 0 ] - [ "$(wc -l < "$TOKEN_LOG")" -eq 1 ] - run jq -r '.tool_calls' "$TOKEN_LOG"; [ "$output" = "2" ] -} - -@test "lpe_emit_record: emitted record is excluded by token_report's kind filter" { - _write_stream "$STREAM_DIR/stream.aaa" - run bash -c 'source "$1"; shift; export LSP_PILOT_ENABLED=true TOKEN_LOG_FILE="$1"; shift; lpe_emit_record "$@"' \ - bash "$EMIT" "$TOKEN_LOG" \ - "$STREAM_DIR" "https://github.com/o/r/pull/10" "sha10" "3.0" - [ "$status" -eq 0 ] - # token_report.sh keeps only (.kind // "token_usage") == "token_usage" - run jq -r 'select((.kind // "token_usage") == "token_usage")' "$TOKEN_LOG" - [ -z "$output" ] -} diff --git a/tests/dev-lead/unit/test_lsp_pilot_measure.bats b/tests/dev-lead/unit/test_lsp_pilot_measure.bats deleted file mode 100644 index 6cf93358d..000000000 --- a/tests/dev-lead/unit/test_lsp_pilot_measure.bats +++ /dev/null @@ -1,142 +0,0 @@ -#!/usr/bin/env bats -# Unit tests for the LSP-pilot measurement extractor (epic #839, story #844). -# -# scripts/lsp_pilot_measure.sh turns one review run's stream-json transcript (+ the -# Token Cost Observatory log) into one pilot-schema JSONL record that -# scripts/lsp_pilot_compare.sh can render. These tests pin the pure extractors: -# lpm_nav_from_stream — navigation tool_use count + estimated nav tokens -# lpm_usage_from_stream — real reported usage from the terminal result event -# lpm_quality_from_log — findings / false_positives from verification records -# lpm_coldstart_from_log — cold_start_s from the lsp_cold_start record (null off) -# main — end-to-end record assembly, lsp-off cold_start=null - -SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" -MEASURE="$SCRIPT_DIR/scripts/lsp_pilot_measure.sh" - -# Source the script in a clean subshell so its `set -euo pipefail` never leaks and -# the source-guard keeps main() from running. -_call() { run bash -c 'source "$1"; shift; "$@"' bash "$MEASURE" "$@"; } - -setup() { - STREAM="$BATS_TEST_TMPDIR/stream.jsonl" - LOG="$BATS_TEST_TMPDIR/token.jsonl" - - # A realistic LSP-ON transcript: two navigation calls (one mcp__lsp__, one Grep), - # one non-navigation call (Write), each with a matching tool_result, then the - # terminal result event carrying real usage. (printf, not heredoc — bats 1.10's - # preprocessor mis-parses heredocs inside setup().) - printf '%s\n' \ - '{"type":"system","subtype":"init","model":"claude-opus-4-8"}' \ - '{"type":"assistant","message":{"model":"claude-opus-4-8","content":[{"type":"text","text":"checking callers"},{"type":"tool_use","id":"t1","name":"mcp__lsp__find_references","input":{"symbol":"emit_token_record"}}],"usage":{"input_tokens":10,"output_tokens":5}}}' \ - '{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"t1","content":"ref1\nref2\nref3\nref4"}]}}' \ - '{"type":"assistant","message":{"content":[{"type":"tool_use","id":"t2","name":"Grep","input":{"pattern":"foo"}}]}}' \ - '{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"t2","content":[{"type":"text","text":"scripts/a.sh:12:foo"}]}]}}' \ - '{"type":"assistant","message":{"content":[{"type":"tool_use","id":"t3","name":"Write","input":{"file":"x"}}]}}' \ - '{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"t3","content":"ok"}]}}' \ - '{"type":"result","subtype":"success","model":"claude-opus-4-8","result":"done","usage":{"input_tokens":1200,"cache_read_input_tokens":800,"output_tokens":300}}' \ - > "$STREAM" - - printf '%s\n' \ - '{"kind":"lsp_cold_start","candidate":"agent-lsp","cold_start_ms":8200,"cache":"miss","skipped":false,"sla_ms":30000,"reason":"ok"}' \ - '{"kind":"finding_verification","finding_index":0,"category":"cross-file","outcome":"confirmed"}' \ - '{"kind":"finding_verification","finding_index":1,"category":"unused-symbol","outcome":"removed"}' \ - '{"ts":"2026-06-26T00:00:00Z","tier":"deep","input_tokens":1200,"output_tokens":300}' \ - > "$LOG" -} - -# ── lpm_nav_from_stream: count + estimated tokens ──────────────────────────── - -@test "lpm_nav_from_stream: counts only navigation tool_use (mcp__lsp__ + Grep, not Write)" { - _call lpm_nav_from_stream "$STREAM" "mcp__lsp__,Grep,Glob,Read,Bash" - [ "$status" -eq 0 ] - # t1 (mcp__lsp__find_references) + t2 (Grep) = 2; t3 (Write) excluded. - calls="$(printf '%s' "$output" | cut -f1)" - [ "$calls" -eq 2 ] -} - -@test "lpm_nav_from_stream: nav tokens are a positive char/4 estimate of input+result" { - _call lpm_nav_from_stream "$STREAM" "mcp__lsp__,Grep,Glob,Read,Bash" - tokens="$(printf '%s' "$output" | cut -f2)" - [ "$tokens" -gt 0 ] -} - -@test "lpm_nav_from_stream: a restricted prefix set counts only matching tools" { - # Only mcp__lsp__ counts → just t1. - _call lpm_nav_from_stream "$STREAM" "mcp__lsp__" - [ "$(printf '%s' "$output" | cut -f1)" -eq 1 ] -} - -@test "lpm_nav_from_stream: missing transcript → 0 calls, 0 tokens (soft)" { - _call lpm_nav_from_stream "$BATS_TEST_TMPDIR/nope.jsonl" "Grep" - [ "$output" = "$(printf '0\t0')" ] -} - -# ── lpm_usage_from_stream: real reported usage ─────────────────────────────── - -@test "lpm_usage_from_stream: reads input/cache/output/model from the result event" { - _call lpm_usage_from_stream "$STREAM" - [ "$output" = "$(printf '1200\t800\t300\tclaude-opus-4-8')" ] -} - -# ── lpm_quality_from_log: findings / false positives ───────────────────────── - -@test "lpm_quality_from_log: findings = verification records, fp = downgraded/removed" { - _call lpm_quality_from_log "$LOG" - # 2 finding_verification records; one outcome 'removed' → 1 false positive. - [ "$output" = "$(printf '2\t1')" ] -} - -@test "lpm_quality_from_log: no log → 0 findings, 0 fp" { - _call lpm_quality_from_log "$BATS_TEST_TMPDIR/none.jsonl" - [ "$output" = "$(printf '0\t0')" ] -} - -# ── lpm_coldstart_from_log: seconds or null ────────────────────────────────── - -@test "lpm_coldstart_from_log: derives seconds from cold_start_ms" { - _call lpm_coldstart_from_log "$LOG" - [ "$output" = "8.2" ] -} - -@test "lpm_coldstart_from_log: no record → null" { - printf '%s\n' '{"ts":"x","tier":"deep"}' > "$BATS_TEST_TMPDIR/nocold.jsonl" - _call lpm_coldstart_from_log "$BATS_TEST_TMPDIR/nocold.jsonl" - [ "$output" = "null" ] -} - -# ── main: end-to-end record assembly ───────────────────────────────────────── - -@test "main: assembles a valid pilot record for an lsp-on run" { - run bash "$MEASURE" "$STREAM" --pr "petry-projects/.github-private#1@abc" \ - --variant lsp-on --candidate agent-lsp --model claude-opus-4-8 \ - --wall-time-s 173.0 --token-log "$LOG" - [ "$status" -eq 0 ] - echo "$output" | jq -e '.pr == "petry-projects/.github-private#1@abc"' >/dev/null - echo "$output" | jq -e '.variant == "lsp-on" and .candidate == "agent-lsp"' >/dev/null - echo "$output" | jq -e '.tool_calls == 2' >/dev/null - echo "$output" | jq -e '.nav_tokens > 0' >/dev/null - echo "$output" | jq -e '.input_tokens == 1200 and .cache_read_tokens == 800 and .output_tokens == 300' >/dev/null - echo "$output" | jq -e '.findings == 2 and .false_positives == 1' >/dev/null - echo "$output" | jq -e '.cold_start_s == 8.2' >/dev/null - echo "$output" | jq -e '.wall_time_s == 173.0' >/dev/null -} - -@test "main: lsp-off run forces cold_start_s to null (matches the frozen baseline)" { - run bash "$MEASURE" "$STREAM" --pr "petry-projects/.github-private#1@abc" \ - --variant lsp-off --candidate baseline --model claude-opus-4-8 \ - --wall-time-s 188.0 --cold-start-ms 8200 --token-log "$LOG" - [ "$status" -eq 0 ] - echo "$output" | jq -e '.cold_start_s == null' >/dev/null - echo "$output" | jq -e '.variant == "lsp-off"' >/dev/null -} - -@test "main: missing required --pr fails loud (exit 2)" { - run bash "$MEASURE" "$STREAM" - [ "$status" -eq 2 ] -} - -@test "main: a value-flag with no argument fails loud, not an unbound-var crash" { - run bash "$MEASURE" "$STREAM" --pr "x" --variant - [ "$status" -ne 0 ] - [[ "$output" == *"--variant requires an argument"* ]] -} diff --git a/tests/dev-lead/unit/test_lsp_pilot_run.bats b/tests/dev-lead/unit/test_lsp_pilot_run.bats deleted file mode 100644 index 497efcf56..000000000 --- a/tests/dev-lead/unit/test_lsp_pilot_run.bats +++ /dev/null @@ -1,43 +0,0 @@ -#!/usr/bin/env bats -# Unit tests for the LSP-pilot run driver's PURE helpers (epic #839, story #844). -# The claude/gh calls in run_variant()/main() are CI-only; these pin the helpers -# that shape the run: wall-time math, the navigation-heavy prompt, and the on-leg -# tool allowlist (base + the pilot's read-only LSP nav tools). - -SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" -RUN="$SCRIPT_DIR/scripts/lsp_pilot_run.sh" - -_call() { run bash -c 'source "$1"; shift; "$@"' bash "$RUN" "$@"; } - -@test "lpr_wall_seconds: positive delta → seconds (1dp)" { - _call lpr_wall_seconds 1000000000 4500000000 - [ "$status" -eq 0 ] - [ "$output" = "3.5" ] -} - -@test "lpr_wall_seconds: backwards clock → clamped to 0.0" { - _call lpr_wall_seconds 5000000000 1000000000 - [ "$output" = "0.0" ] -} - -@test "lpr_wall_seconds: non-numeric (e.g. %N unsupported) → 0.0, never crashes" { - _call lpr_wall_seconds "1234N" "5678N" - [ "$output" = "0.0" ] -} - -@test "lpr_build_prompt: embeds the PR number, verify instruction, and the diff body" { - printf '%s\n' '+++ b/scripts/x.sh' '+echo CANARY_DIFF_LINE' > "$BATS_TEST_TMPDIR/d.diff" - _call lpr_build_prompt 917 "$BATS_TEST_TMPDIR/d.diff" - [ "$status" -eq 0 ] - [[ "$output" == *"pull request #917"* ]] - [[ "$output" == *"verify every CROSS-FILE claim"* ]] - [[ "$output" == *"CANARY_DIFF_LINE"* ]] -} - -@test "lpr_on_tools: includes the base tools AND the pilot LSP navigation allowlist" { - _call lpr_on_tools - [ "$status" -eq 0 ] - [[ "$output" == *"Grep"* ]] - [[ "$output" == *"mcp__lsp__find_references"* ]] - [[ "$output" == *"mcp__lsp__get_diagnostics"* ]] -} diff --git a/tests/dev-lead/unit/test_lsp_verification.bats b/tests/dev-lead/unit/test_lsp_verification.bats deleted file mode 100644 index a95e9f900..000000000 --- a/tests/dev-lead/unit/test_lsp_verification.bats +++ /dev/null @@ -1,223 +0,0 @@ -#!/usr/bin/env bats -# Unit tests for the LSP finding-verification step (epic #839, story #843). -# -# The step grounds deep/audit cross-file/semantic findings against LSP nav tools -# before posting. The model annotates each such finding with an `lsp_verification` -# field (`verified`|`unverifiable`); this shell layer ENFORCES the annotation: -# - inert when LSP is not wired or is degraded (review byte-for-byte unchanged) — AC #3 -# - `unverifiable` → downgrade severity one step + annotate (never dropped) — AC #2 -# - each verified/unverifiable outcome → one JSONL record on TOKEN_LOG_FILE — AC #4 -# It reuses the MCP gating/degradation signal already shipped (#677/#678). - -SCRIPT_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")/../../.." && pwd)" -ENGINE_SCRIPT="$SCRIPT_DIR/scripts/engine.sh" -LSP_LIB="$SCRIPT_DIR/scripts/lib/lsp-verification.sh" -TOKEN_LIB="$SCRIPT_DIR/scripts/lib/token-metrics.sh" -LSP_ALLOWED="mcp__lsp__find_references,mcp__lsp__get_diagnostics" - -setup() { - unset REVIEW_MCP_CONFIG REVIEW_MCP_ALLOWED_TOOLS - # The degradation scan reuses engine.sh's _mcp_failure_pattern when present. - # shellcheck source=/dev/null - source "$ENGINE_SCRIPT" >/dev/null 2>&1 || true - # shellcheck source=/dev/null - source "$TOKEN_LIB" - # shellcheck source=/dev/null - source "$LSP_LIB" - - WORKDIR="$(mktemp -d)" - export TOKEN_LOG_FILE="$WORKDIR/tokens.jsonl" - export TOKEN_WORKFLOW="pr-review" - export PR_URL="https://github.com/petry-projects/x/pull/7" - - MCP_CONFIG_FILE="$WORKDIR/lsp.json" - echo '{"mcpServers":{"lsp":{"type":"stdio","command":"agent-lsp"}}}' > "$MCP_CONFIG_FILE" - - FINDINGS="$WORKDIR/deep.json" -} - -teardown() { - rm -rf "$WORKDIR" - unset REVIEW_MCP_CONFIG REVIEW_MCP_ALLOWED_TOOLS TOKEN_LOG_FILE TOKEN_WORKFLOW PR_URL -} - -_write_findings() { - cat > "$FINDINGS" -} - -_activate_lsp() { - export REVIEW_MCP_CONFIG="$MCP_CONFIG_FILE" - export REVIEW_MCP_ALLOWED_TOOLS="$LSP_ALLOWED" -} - -# ── AC #3: inert when LSP is not wired ──────────────────────────────────────── - -@test "inert: no MCP knob → findings file byte-for-byte unchanged, no JSONL" { - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"critical","category":"correctness","message":"foo() is undefined","lsp_verification":"unverifiable"}]} -JSON - local before; before="$(cat "$FINDINGS")" - run apply_lsp_verification "$FINDINGS" "deep" - [ "$status" -eq 0 ] - [ "$(cat "$FINDINGS")" = "$before" ] - [ ! -s "$TOKEN_LOG_FILE" ] -} - -@test "inert: MCP wired but no lsp tools (e.g. context7) → unchanged, no JSONL" { - export REVIEW_MCP_CONFIG="$MCP_CONFIG_FILE" - export REVIEW_MCP_ALLOWED_TOOLS="mcp__context7__*" - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"critical","category":"correctness","message":"foo() is undefined","lsp_verification":"unverifiable"}]} -JSON - local before; before="$(cat "$FINDINGS")" - run apply_lsp_verification "$FINDINGS" "deep" - [ "$status" -eq 0 ] - [ "$(cat "$FINDINGS")" = "$before" ] - [ ! -s "$TOKEN_LOG_FILE" ] -} - -@test "inert: lsp wired but CLI output shows MCP failure (degraded) → unchanged" { - _activate_lsp - local cli="$WORKDIR/deep.log" - printf 'MCP server "lsp" failed to connect after 3 attempts\n' > "$cli" - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"critical","category":"correctness","message":"foo() is undefined","lsp_verification":"unverifiable"}]} -JSON - local before; before="$(cat "$FINDINGS")" - run apply_lsp_verification "$FINDINGS" "deep" "$cli" - [ "$status" -eq 0 ] - [ "$(cat "$FINDINGS")" = "$before" ] - [ ! -s "$TOKEN_LOG_FILE" ] -} - -# ── AC #2: downgrade + annotate the unverifiable finding (never drop) ────────── - -@test "downgrade: unverifiable finding is downgraded one level and annotated" { - _activate_lsp - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"critical","category":"correctness","message":"foo() is undefined","lsp_verification":"unverifiable"}]} -JSON - run apply_lsp_verification "$FINDINGS" "deep" - [ "$status" -eq 0 ] - # Finding is NOT dropped. - [ "$(jq '.findings | length' "$FINDINGS")" = "1" ] - # critical → major. - [ "$(jq -r '.findings[0].severity' "$FINDINGS")" = "major" ] - # Annotated so the downgrade is visible/auditable in the posted body. - run jq -r '.findings[0].message' "$FINDINGS" - [[ "$output" == *"[lsp: unverifiable]"* ]] -} - -@test "downgrade: ladder bottoms out at info (minor → info, info → info)" { - _activate_lsp - _write_findings <<'JSON' -{"tier":"audit","findings":[ - {"severity":"minor","category":"x","message":"a","lsp_verification":"unverifiable"}, - {"severity":"info","category":"y","message":"b","lsp_verification":"unverifiable"} -]} -JSON - run apply_lsp_verification "$FINDINGS" "audit" - [ "$status" -eq 0 ] - [ "$(jq -r '.findings[0].severity' "$FINDINGS")" = "info" ] - [ "$(jq -r '.findings[1].severity' "$FINDINGS")" = "info" ] -} - -@test "verified: finding is left intact (no downgrade, no annotation)" { - _activate_lsp - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"major","category":"correctness","message":"breaks 3 callers","lsp_verification":"verified"}]} -JSON - run apply_lsp_verification "$FINDINGS" "deep" - [ "$status" -eq 0 ] - [ "$(jq -r '.findings[0].severity' "$FINDINGS")" = "major" ] - run jq -r '.findings[0].message' "$FINDINGS" - [ "$output" = "breaks 3 callers" ] -} - -@test "untouched: a finding with no lsp_verification annotation is not changed" { - _activate_lsp - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"critical","category":"style","message":"prefer printf"}]} -JSON - run apply_lsp_verification "$FINDINGS" "deep" - [ "$status" -eq 0 ] - [ "$(jq -r '.findings[0].severity' "$FINDINGS")" = "critical" ] - # No verification record for an un-annotated finding. - [ ! -s "$TOKEN_LOG_FILE" ] -} - -# ── AC #4: per-finding verification outcome on the Token Observatory JSONL ───── - -@test "jsonl: one finding_verification record per verified/unverifiable finding" { - _activate_lsp - _write_findings <<'JSON' -{"tier":"deep","findings":[ - {"severity":"critical","category":"correctness","message":"undefined sym","lsp_verification":"unverifiable"}, - {"severity":"major","category":"correctness","message":"breaks 2 callers","lsp_verification":"verified"}, - {"severity":"minor","category":"style","message":"nit"} -]} -JSON - run apply_lsp_verification "$FINDINGS" "deep" - [ "$status" -eq 0 ] - # Two records (the un-annotated finding produces none). - [ "$(grep -c . "$TOKEN_LOG_FILE")" = "2" ] - # Every record is discriminated as a finding_verification record. - run jq -rs 'map(select(.kind == "finding_verification")) | length' "$TOKEN_LOG_FILE" - [ "$output" = "2" ] -} - -@test "jsonl: record carries tier, outcome, and before/after severities" { - _activate_lsp - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"critical","category":"correctness","message":"undefined","lsp_verification":"unverifiable"}]} -JSON - run apply_lsp_verification "$FINDINGS" "deep" - [ "$status" -eq 0 ] - run jq -r '[.kind,.tier,.outcome,.severity_before,.severity_after,.workflow] | @tsv' "$TOKEN_LOG_FILE" - [ "$output" = "$(printf 'finding_verification\tdeep\tunverifiable\tcritical\tmajor\tpr-review')" ] -} - -@test "jsonl: verified outcome keeps severity_after == severity_before" { - _activate_lsp - _write_findings <<'JSON' -{"tier":"audit","findings":[{"severity":"major","category":"correctness","message":"ok","lsp_verification":"verified"}]} -JSON - run apply_lsp_verification "$FINDINGS" "audit" - [ "$status" -eq 0 ] - run jq -r '[.outcome,.severity_before,.severity_after] | @tsv' "$TOKEN_LOG_FILE" - [ "$output" = "$(printf 'verified\tmajor\tmajor')" ] -} - -# ── emit_verification_record: no-op safety ──────────────────────────────────── - -@test "emit_verification_record: no-op when TOKEN_LOG_FILE is unset" { - unset TOKEN_LOG_FILE - run emit_verification_record "pr-review" "deep" "ctx" "0" "correctness" "critical" "major" "unverifiable" - [ "$status" -eq 0 ] -} - -@test "emit_verification_record: appends one valid JSONL object" { - emit_verification_record "pr-review" "deep" "ctx" "0" "correctness" "critical" "major" "unverifiable" - [ "$(grep -c . "$TOKEN_LOG_FILE")" = "1" ] - run jq -e '.kind == "finding_verification" and .finding_index == 0' "$TOKEN_LOG_FILE" - [ "$status" -eq 0 ] -} - -# ── apply_lsp_verification: positional-parameter edge cases ────────────────── - -@test "shift-guard: called with 1 arg (no tier) and findings contain MCP-failure text → still processes, not inert" { - # Regression for: shift 2 failing when $# < 2 leaves $@ unchanged, causing - # lsp_verification_active to scan the findings file for failure patterns. - _activate_lsp - # The findings file itself contains text matching the MCP-failure pattern. - _write_findings <<'JSON' -{"tier":"deep","findings":[{"severity":"critical","category":"correctness","message":"mcp server lsp failed to connect","lsp_verification":"unverifiable"}]} -JSON - # Called with only the findings file (no tier arg) — the risky case. - run apply_lsp_verification "$FINDINGS" - [ "$status" -eq 0 ] - # Verification must be active: the unverifiable finding must be downgraded. - [ "$(jq -r '.findings[0].severity' "$FINDINGS")" = "major" ] - run jq -r '.findings[0].message' "$FINDINGS" - [[ "$output" == *"[lsp: unverifiable]"* ]] -} diff --git a/tests/lsp_pilot_compare.bats b/tests/lsp_pilot_compare.bats deleted file mode 100644 index f517590b1..000000000 --- a/tests/lsp_pilot_compare.bats +++ /dev/null @@ -1,154 +0,0 @@ -#!/usr/bin/env bats -# Tests for scripts/lsp_pilot_compare.sh — the LSP-pilot comparison harness (#841). -# -# The harness reads two Token Cost Observatory JSONL streams in the pilot record -# shape: a FROZEN LSP-off baseline (captured once, evals/lsp-pilot/holdout/) and a -# candidate LSP-on run. It renders per-PR and aggregate speed (cold-start, -# wall-time), cost (tokens, ET, USD) and quality (findings, false-positive) deltas -# against the frozen baseline, and FAILS LOUD (non-zero) if a candidate PR has no -# baseline counterpart so a partial corpus cannot masquerade as a clean comparison. -# -# All functions are PURE (no network); these tests exercise them directly. -# Run locally: bats tests/lsp_pilot_compare.bats - -setup() { - # shellcheck source=scripts/lsp_pilot_compare.sh - source "${BATS_TEST_DIRNAME}/../scripts/lsp_pilot_compare.sh" - - BASELINE="$(mktemp)" - CANDIDATE="$(mktemp)" - - # Frozen LSP-off baseline: two PRs pinned by repo#number@sha. - cat > "$BASELINE" <<'JSONL' -{"pr":"petry-projects/.github-private#811@aaaaaaa","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":1000,"cache_read_tokens":0,"output_tokens":200,"nav_tokens":20000,"tool_calls":40,"findings":5,"false_positives":1,"cold_start_s":null,"wall_time_s":60.0} -{"pr":"petry-projects/.github-private#812@bbbbbbb","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":800,"cache_read_tokens":0,"output_tokens":100,"nav_tokens":10000,"tool_calls":20,"findings":3,"false_positives":0,"cold_start_s":null,"wall_time_s":45.0} -JSONL - - # Candidate LSP-on run (agent-lsp): same two PRs, fewer navigation tokens. - cat > "$CANDIDATE" <<'JSONL' -{"pr":"petry-projects/.github-private#811@aaaaaaa","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":1000,"cache_read_tokens":0,"output_tokens":200,"nav_tokens":8000,"tool_calls":12,"findings":5,"false_positives":1,"cold_start_s":8.0,"wall_time_s":42.0} -{"pr":"petry-projects/.github-private#812@bbbbbbb","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":800,"cache_read_tokens":0,"output_tokens":100,"nav_tokens":5000,"tool_calls":10,"findings":3,"false_positives":0,"cold_start_s":6.0,"wall_time_s":30.0} -JSONL -} - -teardown() { - rm -f "$BASELINE" "$CANDIDATE" -} - -# --------------------------------------------------------------------------- -# lp_missing_baselines — coverage check (AC5) -# --------------------------------------------------------------------------- - -@test "lp_missing_baselines: passes when every candidate PR has a baseline" { - run lp_missing_baselines "$BASELINE" "$CANDIDATE" - [ "$status" -eq 0 ] - [ -z "$output" ] -} - -@test "lp_missing_baselines: names the orphan PR and fails when a baseline is absent" { - # Add a third candidate PR with no baseline counterpart. - printf '%s\n' '{"pr":"petry-projects/.github-private#813@ccccccc","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":500,"cache_read_tokens":0,"output_tokens":50,"nav_tokens":3000,"tool_calls":6,"findings":2,"false_positives":0,"cold_start_s":5.0,"wall_time_s":20.0}' >> "$CANDIDATE" - run lp_missing_baselines "$BASELINE" "$CANDIDATE" - [ "$status" -ne 0 ] - [[ "$output" == *"#813@ccccccc"* ]] -} - -# --------------------------------------------------------------------------- -# render_lsp_comparison — per-PR + aggregate render (AC4) -# --------------------------------------------------------------------------- - -@test "render_lsp_comparison: clean comparison renders and exits 0" { - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [ "$status" -eq 0 ] - [[ "$output" == *"#811@aaaaaaa"* ]] - [[ "$output" == *"#812@bbbbbbb"* ]] -} - -@test "render_lsp_comparison: surfaces the navigation-token reduction ratio (>=2x)" { - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [ "$status" -eq 0 ] - # PR #811: 20000 / 8000 = 2.5x reduction in navigation tool-call tokens. - [[ "$output" == *"2.5"* ]] - # PR #812: 10000 / 5000 = 2.0x. - [[ "$output" == *"2.0"* ]] -} - -@test "render_lsp_comparison: renders speed, cost and quality columns" { - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [[ "$output" == *"Nav tokens"* ]] - [[ "$output" == *"Tool calls"* ]] - [[ "$output" == *"Cold start"* ]] - [[ "$output" == *"ET"* ]] - [[ "$output" == *"USD"* ]] - [[ "$output" == *"Findings"* ]] - [[ "$output" == *"False positives"* ]] -} - -@test "render_lsp_comparison: cold start is N/A for the LSP-off baseline" { - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [[ "$output" == *"N/A"* ]] -} - -@test "render_lsp_comparison: documents the explicit quality proxy" { - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - # The proxy is defined IN the harness output so the go/no-go is reproducible. - [[ "$output" == *"false-positive"* ]] || [[ "$output" == *"false positive"* ]] -} - -@test "render_lsp_comparison: USD is rendered to cents (org cost-reporting standard)" { - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [[ "$output" == *'$'* ]] -} - -# --------------------------------------------------------------------------- -# Quality proxy — false-positive regression is visible (AC4: a token win that -# drops precision must show as a regression, not be hidden) -# --------------------------------------------------------------------------- - -@test "render_lsp_comparison: clean run reports no quality regression" { - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [ "$status" -eq 0 ] - [[ "$output" == *"PASS"* ]] -} - -@test "render_lsp_comparison: a candidate with MORE false positives is flagged as a regression" { - # Candidate #811 introduces an extra false positive (1 -> 3) — a precision loss - # that a token win must never hide. - cat > "$CANDIDATE" <<'JSONL' -{"pr":"petry-projects/.github-private#811@aaaaaaa","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":1000,"cache_read_tokens":0,"output_tokens":200,"nav_tokens":8000,"tool_calls":12,"findings":7,"false_positives":3,"cold_start_s":8.0,"wall_time_s":42.0} -{"pr":"petry-projects/.github-private#812@bbbbbbb","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":800,"cache_read_tokens":0,"output_tokens":100,"nav_tokens":5000,"tool_calls":10,"findings":3,"false_positives":0,"cold_start_s":6.0,"wall_time_s":30.0} -JSONL - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [ "$status" -eq 0 ] - [[ "$output" == *"REGRESSION"* ]] -} - -# --------------------------------------------------------------------------- -# Multi-run candidate aggregation (corpus allows <=3 runs per PR) -# --------------------------------------------------------------------------- - -@test "render_lsp_comparison: averages multiple candidate runs of the same PR" { - # PR #811 run twice (nav 8000 and 12000 -> mean 10000); 20000/10000 = 2.0x. - cat > "$CANDIDATE" <<'JSONL' -{"pr":"petry-projects/.github-private#811@aaaaaaa","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":1000,"cache_read_tokens":0,"output_tokens":200,"nav_tokens":8000,"tool_calls":12,"findings":5,"false_positives":1,"cold_start_s":8.0,"wall_time_s":42.0} -{"pr":"petry-projects/.github-private#811@aaaaaaa","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":1000,"cache_read_tokens":0,"output_tokens":200,"nav_tokens":12000,"tool_calls":16,"findings":5,"false_positives":1,"cold_start_s":10.0,"wall_time_s":50.0} -{"pr":"petry-projects/.github-private#812@bbbbbbb","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":800,"cache_read_tokens":0,"output_tokens":100,"nav_tokens":5000,"tool_calls":10,"findings":3,"false_positives":0,"cold_start_s":6.0,"wall_time_s":30.0} -JSONL - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [ "$status" -eq 0 ] - # Mean nav 10000 for #811 -> 2.0x (not 2.5x from the single 8000 run). - [[ "$output" == *"2.0"* ]] -} - -# --------------------------------------------------------------------------- -# AC5 — render itself fails loud on a missing baseline counterpart -# --------------------------------------------------------------------------- - -@test "render_lsp_comparison: fails loud (non-zero) when a candidate PR has no baseline" { - printf '%s\n' '{"pr":"petry-projects/.github-private#999@fffffff","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":500,"cache_read_tokens":0,"output_tokens":50,"nav_tokens":3000,"tool_calls":6,"findings":2,"false_positives":0,"cold_start_s":5.0,"wall_time_s":20.0}' >> "$CANDIDATE" - run render_lsp_comparison "$BASELINE" "$CANDIDATE" - [ "$status" -ne 0 ] - [[ "$output" == *"#999@fffffff"* ]] - # It must not pretend the comparison is clean. - [[ "$output" != *"no quality regression"* ]] -} diff --git a/tests/lsp_pilot_report.bats b/tests/lsp_pilot_report.bats deleted file mode 100644 index a0afca7b6..000000000 --- a/tests/lsp_pilot_report.bats +++ /dev/null @@ -1,234 +0,0 @@ -#!/usr/bin/env bats -# Tests for scripts/lsp_pilot_report.sh — the LSP-pilot report renderer (#844). -# -# This story (Phase 3) consumes the story-2 comparison harness -# (scripts/lsp_pilot_compare.sh) and renders ONE comparison report across every -# candidate LSP server against the FROZEN LSP-off baseline. On top of the -# per-candidate harness it adds the three things the success metric needs that the -# harness alone does not produce: -# * cold-start P50/P95 vs the 30s SLA (AC2), -# * a per-candidate success-metric GO/NO-GO verdict — >=2x navigation-token -# reduction AND no quality regression AND P95 <= 30s — with a data-favored -# pick only when >=2 candidates are competitive (AC3), -# * honest annotation of SLA auto-skip / degraded-MCP runs and a corpus-coverage -# smoke check so a partial corpus can never read as a clean sweep (AC4). -# -# All functions are PURE (no network); these tests exercise them directly. -# Run locally: bats tests/lsp_pilot_report.bats - -setup() { - # shellcheck source=scripts/lsp_pilot_report.sh - source "${BATS_TEST_DIRNAME}/../scripts/lsp_pilot_report.sh" - - BASELINE="$(mktemp)" || { echo "Failed to create temp file"; exit 1; } - CORPUS="$(mktemp)" || { echo "Failed to create temp file"; exit 1; } - CAND_GOOD="$(mktemp)" || { echo "Failed to create temp file"; exit 1; } - CAND_BAD="$(mktemp)" || { echo "Failed to create temp file"; exit 1; } - - # Frozen LSP-off baseline: three corpus PRs pinned by repo#number@sha. - cat > "$BASELINE" <<'JSONL' -{"pr":"petry-projects/.github-private#801@aaa","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":40000,"cache_read_tokens":0,"output_tokens":3000,"nav_tokens":20000,"tool_calls":40,"findings":5,"false_positives":1,"cold_start_s":null,"wall_time_s":180.0} -{"pr":"petry-projects/.github-private#802@bbb","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":20000,"cache_read_tokens":0,"output_tokens":1500,"nav_tokens":10000,"tool_calls":20,"findings":3,"false_positives":0,"cold_start_s":null,"wall_time_s":120.0} -{"pr":"petry-projects/.github-private#803@ccc","variant":"lsp-off","candidate":"baseline","model":"claude-opus-4-7","input_tokens":10000,"cache_read_tokens":0,"output_tokens":800,"nav_tokens":5000,"tool_calls":10,"findings":2,"false_positives":0,"cold_start_s":null,"wall_time_s":60.0} -JSONL - - # The frozen corpus (cases.jsonl shape) — repo + pr_number + head_sha. - cat > "$CORPUS" <<'JSONL' -{"id":"c-801","repo":"petry-projects/.github-private","pr_number":801,"head_sha":"aaa","description":"x","tags":[]} -{"id":"c-802","repo":"petry-projects/.github-private","pr_number":802,"head_sha":"bbb","description":"x","tags":[]} -{"id":"c-803","repo":"petry-projects/.github-private","pr_number":803,"head_sha":"ccc","description":"x","tags":[]} -JSONL - - # GOOD candidate: >=2x nav reduction, no FP regression, cold start within SLA. - cat > "$CAND_GOOD" <<'JSONL' -{"pr":"petry-projects/.github-private#801@aaa","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":18000,"cache_read_tokens":0,"output_tokens":3000,"nav_tokens":8000,"tool_calls":12,"findings":5,"false_positives":1,"cold_start_s":8.0,"wall_time_s":90.0} -{"pr":"petry-projects/.github-private#802@bbb","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":9000,"cache_read_tokens":0,"output_tokens":1500,"nav_tokens":4000,"tool_calls":8,"findings":3,"false_positives":0,"cold_start_s":9.0,"wall_time_s":60.0} -{"pr":"petry-projects/.github-private#803@ccc","variant":"lsp-on","candidate":"agent-lsp","model":"claude-opus-4-7","input_tokens":5000,"cache_read_tokens":0,"output_tokens":800,"nav_tokens":2000,"tool_calls":5,"findings":2,"false_positives":0,"cold_start_s":7.0,"wall_time_s":30.0} -JSONL - - # BAD candidate: one PR SLA-skipped (cold start 34s > 30), one PR MCP-degraded, - # net nav reduction below 2x and a P95 SLA breach. - cat > "$CAND_BAD" <<'JSONL' -{"pr":"petry-projects/.github-private#801@aaa","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":38000,"cache_read_tokens":0,"output_tokens":3000,"nav_tokens":20000,"tool_calls":40,"findings":5,"false_positives":1,"cold_start_s":34.0,"wall_time_s":180.0,"lsp_skipped":true,"skip_reason":"cold-start 34.0s exceeded 30s P95 SLA"} -{"pr":"petry-projects/.github-private#802@bbb","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":9000,"cache_read_tokens":0,"output_tokens":1500,"nav_tokens":5000,"tool_calls":10,"findings":3,"false_positives":0,"cold_start_s":12.0,"wall_time_s":70.0} -{"pr":"petry-projects/.github-private#803@ccc","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":10000,"cache_read_tokens":0,"output_tokens":800,"nav_tokens":5000,"tool_calls":10,"findings":2,"false_positives":0,"cold_start_s":null,"wall_time_s":60.0,"mcp_degraded":true,"skip_reason":"MCP server init failed; review continued on base capabilities"} -JSONL -} - -teardown() { - rm -f "$BASELINE" "$CORPUS" "$CAND_GOOD" "$CAND_BAD" -} - -# --------------------------------------------------------------------------- -# lpr_coldstart_stats — cold-start P50/P95/max over a candidate run (AC2) -# --------------------------------------------------------------------------- - -@test "lpr_coldstart_stats: computes P50/P95/max over non-null cold starts" { - run lpr_coldstart_stats "$CAND_GOOD" - [ "$status" -eq 0 ] - # cold starts 7,8,9 -> p50=8.0 p95=9.0 max=9.0 n=3 - [[ "$output" == "8.0 9.0 9.0 3" ]] -} - -@test "lpr_coldstart_stats: emits NA columns when no cold-start data" { - run lpr_coldstart_stats "$BASELINE" - [ "$status" -eq 0 ] - [[ "$output" == "NA NA NA 0" ]] -} - -# --------------------------------------------------------------------------- -# lpr_within_sla — P95 cold-start vs the 30s SLA (AC2) -# --------------------------------------------------------------------------- - -@test "lpr_within_sla: passes when P95 is within the 30s SLA" { - run lpr_within_sla "$CAND_GOOD" - [ "$status" -eq 0 ] -} - -@test "lpr_within_sla: fails (non-zero) when P95 exceeds the 30s SLA" { - run lpr_within_sla "$CAND_BAD" - [ "$status" -ne 0 ] -} - -# --------------------------------------------------------------------------- -# lpr_skip_annotations — SLA-skip / MCP-degraded runs are surfaced (AC4) -# --------------------------------------------------------------------------- - -@test "lpr_skip_annotations: surfaces SLA-skip and MCP-degraded runs with reasons" { - run lpr_skip_annotations "$CAND_BAD" - [ "$status" -eq 0 ] - [[ "$output" == *"#801@aaa"* ]] - [[ "$output" == *"sla-skip"* ]] - [[ "$output" == *"#803@ccc"* ]] - [[ "$output" == *"mcp-degraded"* ]] -} - -@test "lpr_skip_annotations: emits nothing for a clean run" { - run lpr_skip_annotations "$CAND_GOOD" - [ "$status" -eq 0 ] - [ -z "$output" ] -} - -# --------------------------------------------------------------------------- -# lpr_coverage — every corpus PR covered or an explicit skip reason (AC4 smoke) -# --------------------------------------------------------------------------- - -@test "lpr_coverage: reports a clean candidate as fully covered" { - run lpr_coverage "$CORPUS" "$CAND_GOOD" - [ "$status" -eq 0 ] - [[ "$output" == *"#801@aaa"*"COVERED"* ]] -} - -@test "lpr_coverage: a skipped/degraded run is annotated, not dropped" { - run lpr_coverage "$CORPUS" "$CAND_BAD" - [ "$status" -eq 0 ] - # The SLA-skip and the MCP-degraded PRs must be visible with a reason. - [[ "$output" == *"SKIPPED"* ]] -} - -@test "lpr_coverage: a corpus PR with no candidate run is flagged MISSING and fails" { - # Drop PR #803 from the candidate so a corpus PR is uncovered. - grep -v '#803@ccc' "$CAND_GOOD" > "${CAND_GOOD}.partial" - run lpr_coverage "$CORPUS" "${CAND_GOOD}.partial" - rm -f "${CAND_GOOD}.partial" - [ "$status" -ne 0 ] - [[ "$output" == *"#803@ccc"*"MISSING"* ]] -} - -# --------------------------------------------------------------------------- -# lpr_token_ratio — overall navigation-token reduction vs the frozen baseline -# --------------------------------------------------------------------------- - -@test "lpr_token_ratio: reports the aggregate nav-token reduction ratio" { - run lpr_token_ratio "$BASELINE" "$CAND_GOOD" - [ "$status" -eq 0 ] - # baseline nav 35000 / candidate nav 14000 = 2.50 - [[ "$output" == "2.50" ]] -} - -# --------------------------------------------------------------------------- -# lpr_quality_regressed — false-positive precision proxy vs the frozen baseline -# --------------------------------------------------------------------------- - -@test "lpr_quality_regressed: clean run does not regress quality" { - run lpr_quality_regressed "$BASELINE" "$CAND_GOOD" - [ "$status" -eq 0 ] -} - -@test "lpr_quality_regressed: a run with more false positives regresses" { - printf '%s\n' '{"pr":"petry-projects/.github-private#801@aaa","variant":"lsp-on","candidate":"x","model":"claude-opus-4-7","input_tokens":18000,"cache_read_tokens":0,"output_tokens":3000,"nav_tokens":8000,"tool_calls":12,"findings":7,"false_positives":3,"cold_start_s":8.0,"wall_time_s":90.0}' > "$CAND_GOOD" - run lpr_quality_regressed "$BASELINE" "$CAND_GOOD" - [ "$status" -ne 0 ] -} - -# --------------------------------------------------------------------------- -# lpr_candidate_verdict — the success-metric GO/NO-GO (AC3) -# --------------------------------------------------------------------------- - -@test "lpr_candidate_verdict: GO when token target met, quality held, within SLA" { - run lpr_candidate_verdict "$BASELINE" "$CAND_GOOD" agent-lsp - [ "$status" -eq 0 ] - [[ "$output" == "GO"* ]] -} - -@test "lpr_candidate_verdict: NO-GO on an SLA breach even with a token win" { - run lpr_candidate_verdict "$BASELINE" "$CAND_BAD" serena - [ "$status" -ne 0 ] - [[ "$output" == "NO-GO"* ]] - [[ "$output" == *"SLA"* ]] -} - -# --------------------------------------------------------------------------- -# render_pilot_report — the single multi-candidate report (AC2/AC3/AC4) -# --------------------------------------------------------------------------- - -@test "render_pilot_report: renders each candidate against the frozen baseline" { - run render_pilot_report "$BASELINE" "$CORPUS" "agent-lsp=$CAND_GOOD" "serena=$CAND_BAD" - [ "$status" -eq 0 ] - [[ "$output" == *"agent-lsp"* ]] - [[ "$output" == *"serena"* ]] -} - -@test "render_pilot_report: states the success metric and the 30s SLA verbatim target" { - run render_pilot_report "$BASELINE" "$CORPUS" "agent-lsp=$CAND_GOOD" - [[ "$output" == *"2x"* || "$output" == *"2×"* ]] - [[ "$output" == *"30s"* ]] -} - -@test "render_pilot_report: surfaces cold-start P50/P95 per candidate" { - run render_pilot_report "$BASELINE" "$CORPUS" "agent-lsp=$CAND_GOOD" - [[ "$output" == *"P50"* ]] - [[ "$output" == *"P95"* ]] -} - -@test "render_pilot_report: reports a per-candidate GO/NO-GO verdict" { - run render_pilot_report "$BASELINE" "$CORPUS" "agent-lsp=$CAND_GOOD" "serena=$CAND_BAD" - [[ "$output" == *"GO"* ]] - [[ "$output" == *"NO-GO"* ]] -} - -@test "render_pilot_report: annotates SLA-skipped and degraded runs (no silent omission)" { - run render_pilot_report "$BASELINE" "$CORPUS" "serena=$CAND_BAD" - [[ "$output" == *"sla-skip"* || "$output" == *"SLA"* ]] - [[ "$output" == *"degraded"* ]] -} - -@test "render_pilot_report: does NOT pick a favored candidate when only one is GO" { - # agent-lsp GO, serena NO-GO -> a single passing candidate, no head-to-head pick. - run render_pilot_report "$BASELINE" "$CORPUS" "agent-lsp=$CAND_GOOD" "serena=$CAND_BAD" - [[ "$output" != *"Data favored:"* ]] -} - -@test "render_pilot_report: picks the data-favored candidate only when two are competitive" { - # A second GO candidate with a smaller token win than agent-lsp. - local cand2; cand2="$(mktemp)" || { echo "Failed to create temp file"; exit 1; } - cat > "$cand2" <<'JSONL' -{"pr":"petry-projects/.github-private#801@aaa","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":20000,"cache_read_tokens":0,"output_tokens":3000,"nav_tokens":9000,"tool_calls":14,"findings":5,"false_positives":1,"cold_start_s":10.0,"wall_time_s":95.0} -{"pr":"petry-projects/.github-private#802@bbb","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":10000,"cache_read_tokens":0,"output_tokens":1500,"nav_tokens":4500,"tool_calls":9,"findings":3,"false_positives":0,"cold_start_s":11.0,"wall_time_s":65.0} -{"pr":"petry-projects/.github-private#803@ccc","variant":"lsp-on","candidate":"serena","model":"claude-opus-4-7","input_tokens":5000,"cache_read_tokens":0,"output_tokens":800,"nav_tokens":2200,"tool_calls":6,"findings":2,"false_positives":0,"cold_start_s":9.0,"wall_time_s":32.0} -JSONL - run render_pilot_report "$BASELINE" "$CORPUS" "agent-lsp=$CAND_GOOD" "serena=$cand2" - rm -f "$cand2" - [ "$status" -eq 0 ] - [[ "$output" == *"Data favored: agent-lsp"* ]] -} diff --git a/tests/test_holdout_guard.bats b/tests/test_holdout_guard.bats index 068f4a9a0..6892b7294 100644 --- a/tests/test_holdout_guard.bats +++ b/tests/test_holdout_guard.bats @@ -76,18 +76,6 @@ setup() { [ "$status" -ne 0 ] } -@test "hg_path_is_guarded covers the lsp-pilot holdout corpus (AC2, #841)" { - # The frozen LSP-pilot corpus lives under evals/lsp-pilot/holdout/, so the - # default evals/ prefix must already guard it — no new mechanism is invented. - run hg_path_is_guarded "evals/lsp-pilot/holdout/cases.jsonl" - [ "$status" -eq 0 ] -} - -@test "hg_path_is_guarded covers the frozen lsp-off baseline artifact (AC2/AC3, #841)" { - run hg_path_is_guarded "evals/lsp-pilot/holdout/baseline-lsp-off.jsonl" - [ "$status" -eq 0 ] -} - @test "hg_path_is_guarded honours a configurable prefix list" { export HOLDOUT_GUARDED_PREFIXES="evals/ secret-set/" run hg_path_is_guarded "secret-set/holdout.jsonl" @@ -121,18 +109,6 @@ setup() { [ "$status" -eq 0 ] } -@test "FAIL: proposer mutating the frozen lsp-pilot scored corpus (AC2, #841)" { - run hg_evaluate "github-actions[bot]" <<< "evals/lsp-pilot/holdout/cases.jsonl" - [ "$status" -ne 0 ] - [[ "$output" == *"evals/lsp-pilot/holdout/cases.jsonl"* ]] -} - -@test "FAIL: proposer mutating the frozen lsp-off baseline artifact (AC2/AC3, #841)" { - run hg_evaluate "github-actions[bot]" <<< "evals/lsp-pilot/holdout/baseline-lsp-off.jsonl" - [ "$status" -ne 0 ] - [[ "$output" == *"evals/lsp-pilot/holdout/baseline-lsp-off.jsonl"* ]] -} - @test "FAIL: proposer with a mix of evals and non-evals paths" { run hg_evaluate "github-actions[bot]" <<< "$(printf 'prompts/triage.md\nevals/triage/cases.jsonl\n')" [ "$status" -ne 0 ] diff --git a/tests/test_mcp_connectivity_check.bats b/tests/test_mcp_connectivity_check.bats index aebceec8f..473a0a2a3 100644 --- a/tests/test_mcp_connectivity_check.bats +++ b/tests/test_mcp_connectivity_check.bats @@ -93,12 +93,12 @@ setup() { @test "derive_allowed_tools: multiple servers are comma-joined" { cfg="$(mktemp)" - echo '{"mcpServers":{"context7":{},"lsp":{}}}' > "$cfg" + echo '{"mcpServers":{"context7":{},"github":{}}}' > "$cfg" run derive_allowed_tools "$cfg" rm -f "$cfg" [ "$status" -eq 0 ] [[ "$output" == *"mcp__context7__*"* ]] - [[ "$output" == *"mcp__lsp__*"* ]] + [[ "$output" == *"mcp__github__*"* ]] } @test "derive_allowed_tools: a config with no servers yields empty output" {