feat: implement issue #1031 — fix(#844): plumb LSP_PILOT_ENABLED env + decouple capture from LSP wiring so the pilot A/B runs in CI

[don-petry]

Closes #1031

Implemented by dev-lead agent. Please review.

Summary by CodeRabbit

• New Features

  • Added a manual A/B variant option for the PR review flow, allowing runs to be marked as on, off, or legacy behavior.
  • Review runs now carry the selected variant through the workflow and emitted pilot records.

• Bug Fixes

  • Improved variant selection so explicit inputs take priority over existing configuration.
  • Added validation to ensure both workflow wiring and variant handling behave consistently across trigger types.

• Tests

  • Expanded coverage for pilot variant routing, record generation, and workflow input forwarding.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ae0bf98b-bdfe-4d43-b59c-afe1e29f2ae0

📥 Commits

Reviewing files that changed from the base of the PR and between c76eb4d and d4a1411.

📒 Files selected for processing (6)

• .github/workflows/pr-review-trigger.yml
• .github/workflows/pr-review.yml
• .github/workflows/test-dev-lead.yml
• scripts/lsp_pilot_emit.sh
• tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py
• tests/dev-lead/unit/test_lsp_pilot_emit.bats

---

📝 Walkthrough

Walkthrough

This PR adds an lsp_pilot_variant A/B selector (none/off/on) to pr-review.yml and pr-review-trigger.yml, decoupling LSP-MCP wiring from pilot capture. It updates lsp_pilot_emit.sh to prioritize LSP_PILOT_VARIANT, adds an integration test validating the workflow plumbing, extends bats unit tests, and adds a CI job.

Changes

LSP Pilot A/B Plumbing

Layer / File(s)	Summary
Reusable workflow variant input and gating .github/workflows/pr-review.yml	Adds lsp_pilot_variant workflow_call input, computes LSP_PILOT_ENABLED/LSP_PILOT_VARIANT job env from legacy repo var and new variant, and broadens LSP wiring step conditions to gate on on.
Trigger workflow forwarding .github/workflows/pr-review-trigger.yml	Adds lsp_pilot_variant workflow_dispatch choice input and forwards it into the reusable pr-review.yml call.
Emit script variant resolution scripts/lsp_pilot_emit.sh	Updates lpe_variant() to honor an explicit LSP_PILOT_VARIANT before falling back to REVIEW_MCP_CONFIG wiring detection.
Bats unit tests and CI path wiring tests/dev-lead/unit/test_lsp_pilot_emit.bats, .github/workflows/test-dev-lead.yml	Adds decoupling test coverage for variant selection/candidate/record emission and includes the pr-review workflows in path filters.
Integration test for workflow plumbing tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py, .github/workflows/test-dev-lead.yml	New static YAML-parsing test validates the reusable workflow input/env/step gating and trigger forwarding, wired into a new lsp-pilot-plumbing CI job.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Trigger as pr-review-trigger.yml
  participant Reusable as pr-review.yml
  participant EmitScript as lsp_pilot_emit.sh
  participant Log as TOKEN_LOG_FILE

  Trigger->>Reusable: forward lsp_pilot_variant input
  Reusable->>Reusable: resolve LSP_PILOT_ENABLED / LSP_PILOT_VARIANT
  alt variant is on
    Reusable->>Reusable: wire LSP MCP (REVIEW_MCP_CONFIG)
  else variant is off
    Reusable->>Reusable: skip LSP wiring
  end
  Reusable->>EmitScript: run capture with LSP_PILOT_VARIANT env
  EmitScript->>EmitScript: lpe_variant() resolves lsp-on/lsp-off
  EmitScript->>Log: emit lsp_pilot_run record

Loading

Possibly related PRs

• petry-projects/.github-private#961: Initial implementation of lsp_pilot_emit.sh and pilot record emission that this PR extends with variant decoupling logic.

Suggested labels: needs-human-review

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev-lead/issue-1031-20260703-0238

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[gemini-code-assist]

Code Review

This pull request decouples the LSP pilot capture from the LSP wiring by introducing an explicit LSP_PILOT_VARIANT (on|off) variable in scripts/lsp_pilot_emit.sh, and adds corresponding integration and unit tests. The review feedback suggests improving the BATS tests by creating temporary files inside a setup-created, teardown-managed directory instead of using bare mktemp to ensure automatic cleanup. Additionally, it recommends adding defensive checks in the Python test script to handle cases where the parsed YAML document is None.

[Copilot]

Pull request overview

Implements issue #1031 by wiring the LSP pilot capture gate into CI and introducing a decoupled A/B “variant” control so the same pr-review pipeline can emit comparable lsp-off (control) and lsp-on (treatment) lsp_pilot_run records.

Changes:

• Plumbs LSP_PILOT_ENABLED / LSP_PILOT_VARIANT into pr-review.yml job env: and gates the LSP wiring steps on the on variant (while keeping the legacy vars.LSP_PILOT_ENABLED path).
• Updates scripts/lsp_pilot_emit.sh to treat LSP_PILOT_VARIANT=on|off as authoritative for labeling (lsp-on/lsp-off), decoupling capture from whether REVIEW_MCP_CONFIG is readable.
• Adds unit + integration coverage (including a workflow-plumbing guard) and wires the new integration check into test-dev-lead.yml.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/dev-lead/unit/test_lsp_pilot_emit.bats	Adds unit tests asserting explicit LSP_PILOT_VARIANT overrides legacy MCP-config-based detection and yields correct emitted records.
tests/dev-lead/integration/test_pr_review_lsp_pilot_plumbing.py	Adds an integration check that validates workflow inputs, env plumbing, and step gating for the pilot decoupling.
scripts/lsp_pilot_emit.sh	Makes `LSP_PILOT_VARIANT=on
.github/workflows/test-dev-lead.yml	Extends path filters and adds a new job to run the integration plumbing test.
.github/workflows/pr-review.yml	Adds workflow_call input lsp_pilot_variant, exports LSP_PILOT_ENABLED/LSP_PILOT_VARIANT, and gates LSP wiring steps on on.
.github/workflows/pr-review-trigger.yml	Adds a workflow_dispatch choice input and forwards it into the reusable workflow for ring-0 A/B driving.

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

[don-petry]

Dev-Lead — review-changes (applied)

Changes committed and pushed.

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[don-petry]

Dev-Lead — fix-bot-comment (applied)

Changes committed and pushed.