feat: implement issue #960 — feat(#844): instrument the real pr-review to emit LSP-pilot records (gated stream-json capture)

[don-petry]

Closes #960

Implemented by dev-lead agent. Please review.

Summary by CodeRabbit

• New Features
  • Added an optional “LSP pilot” mode that captures selected review runs using a streaming transcript and records a single transcript-derived usage entry per run.
  • Updated the Claude stub fixture to support a stream-json output format for testing.
• Bug Fixes
  • Ensured pilot capture only affects enabled runs and only the intended review tiers; non-capturing tiers remain on the normal output format.
  • Automatic cleanup of temporary capture artifacts after processing.
• Tests
  • Added unit and engine coverage for pilot capture behavior and transcript-based usage extraction.

[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]

📝 Walkthrough

Walkthrough

Adds opt-in LSP pilot capture for selected claude tiers, emits one tagged pilot record per reviewed PR, and adds helper scripts plus Bats coverage for the capture and emission flow.

Changes

LSP Pilot Stream Capture and Record Emission

Layer / File(s)	Summary
engine.sh stream-json capture and tier wiring scripts/engine.sh	Adds the pilot gate, switches selected claude calls to stream-json --verbose, parses the final result event for existing usage/text handling, and enables capture for deep, audit, single, and duck tiers.
lsp_pilot_emit.sh helper library scripts/lsp_pilot_emit.sh	Adds the sourced emission helper with pilot gating, variant/candidate/key/time helpers, transcript aggregation, pilot record extraction, and kind: lsp_pilot_run tagging.
review-one-pr.sh lifecycle trap scripts/review-one-pr.sh	Sources the emit helper under LSP_PILOT_ENABLED=true, creates a temp stream directory, and registers an EXIT trap that emits the pilot record and removes the temp directory.
stub-claude stream-json mode and engine pilot tests tests/dev-lead/fixtures/engines/stub-claude, tests/dev-lead/unit/test_engine_lsp_pilot.bats	Adds stream-json fixture output and Bats coverage for pilot-off behavior, pilot-on capture for each tier, transcript creation, usage parsing, and unset stream-dir handling.
lsp_pilot_emit.sh unit tests tests/dev-lead/unit/test_lsp_pilot_emit.bats	Adds Bats coverage for helper functions, record emission paths, multi-transcript aggregation, and token-report filtering.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• petry-projects/.github-private#334: Also changes scripts/engine.sh tier execution and Claude output handling in the review pipeline.
• petry-projects/.github-private#713: Also modifies the same run_agentic/run_duck execution path in scripts/engine.sh.

Suggested labels

needs-human-review

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 57.89% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding gated stream-json LSP-pilot records to the real pr-review pipeline.
Linked Issues check	✅ Passed	The changes align with the issue: pilot-gated stream capture, review-time emission, kind-tagging, and tests for on/off behavior are all present.
Out of Scope Changes check	✅ Passed	The reported changes stay within the pilot instrumentation scope and appear additive, with no unrelated features or schema changes introduced.

✨ 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-960-20260627-1347

---

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.}

[gemini-code-assist]

Code Review

This pull request introduces LSP-pilot stream capture functionality to record production PR reviews as pilot-schema records. It updates scripts/engine.sh to capture stream-json transcripts for navigation-heavy tiers, adds scripts/lsp_pilot_emit.sh to aggregate and measure these transcripts, and integrates this process into scripts/review-one-pr.sh via an exit trap. Feedback on these changes highlights a portability issue on macOS where nanosecond date formatting fails, a potential math bug when calculating wall time with fallback values, and instances where the token_log parameter override is ignored in guard checks and output redirection.

[don-petry]

Dev-Lead — review-changes (applied)

Changes committed and pushed.

[don-petry]

Dev-Lead — review-changes (applied)

Changes committed and pushed.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

**Files analyzed:**
- scripts/engine.sh
- scripts/lsp_pilot_emit.sh
- scripts/review-one-pr.sh
- tests/dev-lead/fixtures/engines/stub-claude
- tests/dev-lead/unit/test_engine_lsp_pilot.bats
- tests/dev-lead/unit/test_lsp_pilot_emit.bats
- tests/test_cut_release.bats
- tests/test_cut_release_cross_repo.bats
No actionable issues found in the SonarCloud analysis. The PR is ready to merge from a code quality perspective.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: 48c13f83ece288e60849fe5ea406156f69c36bf9
Review mode: triage-approved (single reviewer)

Summary

Gated LSP-pilot stream-json capture for the production PR-review (closes #960, story #844, epic #839). Adds scripts/lsp_pilot_emit.sh, threads stream-json capture through engine.sh's claude chain for navigation tiers (deep/audit/single/duck), and wires an EXIT-trap emitter into review-one-pr.sh. The whole feature is gated on LSP_PILOT_ENABLED; when unset the off-pilot path is byte-for-byte unchanged (verified by tests). +662/-3 across 8 files, with substantial new bats coverage.

Linked issue analysis

#960 asks to instrument the real pr-review to emit LSP-pilot records via gated stream-json capture. The PR implements exactly that: a master switch (LSP_PILOT_ENABLED), per-capturing-tier opt-in (_LSP_PILOT_CAPTURE), per-PR transcript dir, and a non-fatal emitter that reuses #952's lsp_pilot_measure.sh schema verbatim and tags records kind:"lsp_pilot_run" so token_report.sh keeps excluding them from cost aggregation. Substantively addressed.

Findings

Gemini's earlier review (on first commit 88605df) raised 4 MEDIUM items, all fixed in the current head (48c13f) and verified:

• macOS/BSD date +%s%N portability: review-one-pr.sh now strips a trailing N and pads with 9 zeros, so timestamps degrade to 1s granularity instead of breaking. Fixed.
• Wall-time math with zero fallback: lpe_wall_seconds awk guard adds a == 0 || b == 0 -> returns 0.0 instead of a ~1.6e9s delta. Fixed.
• token_log 5th-arg ignored in guard: now guards on [ -n "$token_log" ]. Fixed.
• token_log not honored on final append: now jq -c ... <<< "$record" >> "$token_log" (here-string, correct target). Fixed.
  coderabbitai APPROVED the current head. Off-pilot inertness is asserted by tests (no output-format change, no transcript). No new issues found in this triage-confirmation pass. Secret-scanning MCP tool was unavailable; relying on the gitleaks CI check (passed).

CI status

All substantive checks green: CodeQL (actions/python), ShellCheck/shellcheck, CI Lint, bats, unit-tests, Test Dev-Lead suite, gitleaks secret scan, AgentShield, Holdout Guard, Test-Deletion Guard, SonarCloud (Quality Gate passed: 0 new issues). The two CANCELLED entries (dev-lead/dispatch, dev-lead/ci-relay) are concurrency supersessions with a parallel SUCCESS dev-lead/dispatch run; SKIPPED entries are conditional no-ops (dependabot, language-specific audits). mergeStateStatus is BEHIND (needs a rebase before merge) but mergeable=MERGEABLE; not a review blocker.

---

Reviewed automatically by the PR-review agent (claude-opus-4-8). Reply if you need a human review.

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

Issues addressed: 0 (Quality Gate Passed)
- 0 New issues reported
- 0 Security Hotspots
- 0 Accepted issues
Prior Findings: All Gemini-reported issues already fixed and verified in current HEAD
Files changed: No action needed
Skipped (informational): 0
```
No changes required — the PR passes all quality gates and reviews.

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/lsp_pilot_emit.sh`:
- Around line 93-118: The combined-stream approach in lsp_pilot_emit.sh is
producing an arbitrary “last result” because lsp_pilot_measure.sh expects a
single transcript with one terminal result event. Update the folding logic
around the combined file creation and the lsp_pilot_measure.sh call so
usage/model are aggregated deterministically per call instead of concatenating
random stream.XXXXXX files, and keep the one-record-per-review output stable by
summing per-call result events or preserving a fixed call order before emission.

In `@tests/dev-lead/fixtures/engines/stub-claude`:
- Around line 82-86: The no-jq fallback in stub-claude is emitting raw response
and model values into the result JSON, so quotes, backslashes, or newlines can
break the NDJSON stream. Update the fallback printf that builds the
{"type":"result"...} payload to serialize/escape both $response and
${model:-claude-stub} safely before insertion, keeping the fixture valid for
parser tests.

In `@tests/dev-lead/unit/test_engine_lsp_pilot.bats`:
- Around line 104-113: Add test coverage for the new single capture tier in the
existing pilot audit suite. Extend the test setup around run_agentic,
_source_engine, and LSP_PILOT_ENABLED to exercise the single mode path that now
sets _LSP_PILOT_CAPTURE, then assert the expected stream-json capture behavior
via ARGS_RECORD and STREAM_DIR. Keep the new test alongside the current pilot
audit test so regressions in the deep|audit|single branching are caught.
- Around line 73-88: The pilot deep test currently verifies args, transcripts,
and token usage, but not the user-facing output from run_agentic. Update this
test to assert that _claude_chain_invoke still returns plain text to stdout by
checking run’s output for the final .result text and ensuring it does not
contain raw NDJSON or stream-json event payloads. Use the existing run_agentic
and _claude_chain_invoke flow in this test so any regression in terminal output
is caught.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4c698c66-1e59-4f9c-a621-3e50c8ad1157

📥 Commits

Reviewing files that changed from the base of the PR and between f3b24ed and e97cb17.

📒 Files selected for processing (6)

• scripts/engine.sh
• scripts/lsp_pilot_emit.sh
• scripts/review-one-pr.sh
• tests/dev-lead/fixtures/engines/stub-claude
• tests/dev-lead/unit/test_engine_lsp_pilot.bats
• tests/dev-lead/unit/test_lsp_pilot_emit.bats

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[don-petry]

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

No changes were needed for this PR.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: b7d590f7135a2c23b452a65d6a2e36af0121a3d0
Review mode: triage-approved (single reviewer)

Summary

Implements issue #960: instruments the production pr-review to emit one LSP-pilot record per reviewed PR via gated stream-json capture. engine.sh tees the deep/audit/single/duck claude transcripts into a per-PR stream dir (only when LSP_PILOT_ENABLED=true and the tier is a navigation tier); lsp_pilot_emit.sh folds them into one synthesized result event and appends a kind:"lsp_pilot_run" record to TOKEN_LOG_FILE; review-one-pr.sh wires an EXIT trap to emit on any exit path. Strictly opt-in and byte-for-byte inert when the flag is unset, so the consumer repos are unaffected.

Linked issue analysis

Linked issue #960 (Story #844, Epic #839) is substantively addressed. It asked to upgrade the LSP pilot from the standalone #952 probe to riding the real production review, the blocker being that engine.sh runs claude --output-format json (aggregate usage only) and never captures the per-tool-call transcript needed for nav_tokens/tool_calls. The PR adds exactly that gated stream-json capture path plus the emit helper, stub fixture, and unit/engine test coverage.

Findings

No blocking findings.

• All advisory review threads (8) are RESOLVED: gemini-code-assist (BSD date %N fallback, T0=0 guard, token_log honoring) and CodeRabbit (transcript-collapse data-integrity, no-jq fallback escaping, plain-text-return assertion, single-tier coverage).
• CodeRabbit's final review state is APPROVED (2026-06-27T15:15:58Z), superseding its earlier CHANGES_REQUESTED; Gemini's is COMMENTED only.
• Security sanity scan of the new shell found no eval/injection/destructive patterns; the URL parse in lpe_pr_key uses trusted PR metadata, and all I/O is defensively guarded (mktemp, || true, no-op returns).
• Note: GitHub Secret Protection MCP (run_secret_scanning) was not available in this environment; the gitleaks CI secret-scan check passed.
• Prior single-reviewer pass approved an earlier commit (48c13f8, MEDIUM) and was dismissed by subsequent main merges; head b7d590f is a no-change merge-forward per dev-lead.

CI status

All required checks green or skipped. Notable SUCCESS: shellcheck/ShellCheck, unit/unit-tests/bats, validate-fixtures, CodeQL, Analyze (actions/python), Secret scan (gitleaks), AgentShield, Agent Security Scan, SonarCloud, Lint, gh-aw-compile. dependency-audit ecosystem legs SKIPPED (no matching ecosystems). mergeStateStatus is BLOCKED only on REVIEW_REQUIRED (this approval).

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.