feat: implement issue #843 — [Phase 2] Add the LSP finding-verification step to the deep/audit pr-review tiers

[don-petry]

Closes #843

Implemented by dev-lead agent. Please review.

Summary by CodeRabbit

• New Features

  • Added LSP-based verification step for cross-file findings in deep reviews and security audits.
  • Unverifiable findings are automatically downgraded one severity level.
  • Finding verification outcomes are logged for token cost tracking.

• Documentation

  • Updated guidance on the new LSP finding-verification process and behavior when LSP tools are unavailable.

• Tests

  • Added comprehensive test coverage for verification behaviors, including handling of verified/unverifiable findings and token logging.

[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

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 54bd1dc3-09d9-4b2a-86e8-ae7bc4bb588a

📥 Commits

Reviewing files that changed from the base of the PR and between b31e518 and 1380a02.

📒 Files selected for processing (9)

• docs/lsp-pilot.md
• prompts/deep-review.md
• prompts/security-audit.md
• scripts/lib/lsp-verification.sh
• scripts/lib/token-metrics.sh
• scripts/review-one-pr.sh
• scripts/token_report.sh
• tests/dev-lead/unit/test_lsp_verification.bats
• tests/token_report.bats

---

📝 Walkthrough

Walkthrough

Implements LSP-backed finding verification for the deep and audit review tiers. New prompts steps define the lsp_verification annotation contract; a new Bash library (lsp-verification.sh) enforces severity downgrading and atomic JSON rewriting; token-metrics.sh gains emit_verification_record for JSONL emission; token_report.sh filters those records from cost aggregates; and review-one-pr.sh wires the verification calls for both tiers.

Changes

LSP Finding-Verification Step

Layer / File(s)	Summary
Verification contract in deep/audit prompts prompts/deep-review.md, prompts/security-audit.md	Adds LSP finding-verification step to both prompt documents: cross-file claims must be grounded via mcp__lsp__* tools, each finding annotated with lsp_verification (verified/unverifiable), severity reduced one level when unverifiable, step skipped when tools absent. Output JSON schema updated with the optional lsp_verification field.
lsp-verification.sh library scripts/lib/lsp-verification.sh	New Bash library exporting _lsp_failure_pattern, _lsp_downgrade_severity, lsp_verification_active (gates on REVIEW_MCP_CONFIG, allowed tools, and CLI-output failure signals), and apply_lsp_verification (atomic jq rewrite downgrading and annotating unverifiable findings, with optional per-finding record emission).
Token Observatory emission and kind filtering scripts/lib/token-metrics.sh, scripts/token_report.sh	Adds emit_verification_record to write finding_verification JSONL entries into TOKEN_LOG_FILE (no-op when unset). Updates annotate_records in token_report.sh to filter to kind=token_usage only, preventing finding_verification rows from polluting cost aggregates.
Pipeline integration scripts/review-one-pr.sh	Sources lsp-verification.sh and invokes apply_lsp_verification on deep.json after deep tier completion and on audit.json before audit result extraction, both tolerating failure with || true.
Bats tests and docs tests/dev-lead/unit/test_lsp_verification.bats, tests/token_report.bats, docs/lsp-pilot.md	New Bats suite covering inert mode (no LSP, wrong tools, MCP failure), unverifiable downgrade with floor clamping, verified/untouched finding preservation, Token Observatory JSONL field assertions, emit_verification_record unit tests, and positional-parameter regression. token_report.bats adds a kind-filtering guard. lsp-pilot.md documents the new step.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• petry-projects/.github-private#334: Directly extends the same scripts/lib/token-metrics.sh JSONL logging framework by adding emit_verification_record to the shared TOKEN_LOG_FILE channel.
• petry-projects/.github-private#456: Both PRs modify scripts/token_report.sh and tests/token_report.bats to control which JSONL record kinds are included in cost aggregates.
• petry-projects/.github-private#848: The docs/lsp-pilot.md additions in this PR directly extend the LSP pilot scoping contract introduced in PR #848.

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 63.64% 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 and specifically summarizes the main change: implementing issue #843 to add the LSP finding-verification step to deep/audit PR-review tiers, which directly matches the PR's primary objective.
Linked Issues check	✅ Passed	The PR fully implements all acceptance criteria from issue #843: verification contract (prompts updated), inert degradation (lsp-verification.sh), JSONL instrumentation (emit_verification_record), and bats test coverage for both inert and downgrade paths.
Out of Scope Changes check	✅ Passed	All changes directly support the LSP finding-verification feature: prompt updates define the verification step, lsp-verification.sh enforces it, token-metrics.sh emits records, review-one-pr.sh integrates it, token_report.sh filters verification records, and tests validate the behavior. No unrelated changes detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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-843-20260621-0727

---

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 and usage tips.}

[don-petry]

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

No changes were needed for this PR.

[gemini-code-assist]

Code Review

This pull request introduces the LSP finding-verification step to ground cross-file or semantic claims made during deep reviews and security audits against LSP navigation tools. It includes updates to prompts, a new Bash library (scripts/lib/lsp-verification.sh) to enforce annotations and handle downgrades of unverifiable findings, logging of verification outcomes to the Token Cost Observatory JSONL, and corresponding unit tests. Feedback on the changes suggests a robust handling of positional parameters in apply_lsp_verification to prevent the findings file from being scanned as a CLI output file when fewer than two arguments are provided.

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

[don-petry]

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

No changes were needed for this PR.

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-21T08:56:07Z.

[donpetry-bot]

Automated review — APPROVED ✓

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

Summary

Implements issue #843 (Phase 2 of the LSP pilot, epic #839): an LSP finding-verification step for the deep/audit PR-review tiers. The deep-review and security-audit prompts now ask the model to annotate each cross-file/semantic finding with an lsp_verification field; a new scripts/lib/lsp-verification.sh deterministically enforces it — downgrading + tagging unverifiable findings one severity level (never dropping them) and emitting each outcome to the Token Cost Observatory JSONL via emit_verification_record. token_report.sh is updated to exclude the new finding_verification record kind from cost pricing. The whole step is inert (findings byte-for-byte unchanged) when the LSP MCP server is unwired or degraded. +496/-4 across 9 files.

Linked issue analysis

Closes #843. The PR substantively addresses the story: (a) prompt-level instruction to ground cross-file/semantic claims via mcp__lsp__find_references / mcp__lsp__get_diagnostics (AC #1), (b) deterministic downgrade+annotate of unverifiable findings, never dropped (AC #2), (c) inert behavior when LSP is unwired/degraded, reusing the existing MCP failure-pattern signal (AC #3), and (d) per-finding verification records on the token JSONL for the FPR-delta harness (AC #4). Coverage is solid: 223 lines of new bats tests exercising all four ACs plus edge cases.

Findings

No blocking issues.

• Severity downgrade is bounded (one level, bottoms out at info), annotated [lsp: unverifiable], and audited — findings are never silently dropped, so the change cannot weaken a review by hiding a finding.
• The prior gemini-code-assist review flagged a positional-parameter risk (shift 2 when fewer than two args could cause the findings file to be scanned as a CLI-output file); this is resolved by the 'if [ "$#" -ge 2 ]; then shift 2; else shift "$#"; fi' guard in apply_lsp_verification and locked in by the dedicated shift-guard regression test.
• Net effect on current reviews is zero: the step is inert until the LSP pilot is wired, so the blast radius is contained.
  Note: mcp__github__run_secret_scanning was not available in this environment; the gitleaks CI secret-scan check passed.

CI status

All checks green. Lint/ShellCheck/bats, unit-tests, CodeQL (actions+python), SonarCloud (Quality Gate passed, 0 new issues), gitleaks secret scan, AgentShield, and the dev-lead/test suites all SUCCESS. One 'review / review' run shows CANCELLED, superseded by a later SUCCESS run of the same check (concurrency cancellation), not a failure. mergeStateStatus is BLOCKED pending this required review.

---

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

[don-petry]

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #861
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-06-21T11:22:52Z

[don-petry]

Note

@don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically.
Next attempt after: 2026-06-21T11:22:52Z

[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

[donpetry-bot]

Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-21T11:58:28Z.