Skip to content

feat: implement issue #846 — [Phase 2] Cache the LSP index and enforce the 30s cold-start SLA with auto-skip - #860

Closed
don-petry wants to merge 379 commits into
mainfrom
dev-lead/issue-846-20260621-0727
Closed

don-petry wants to merge 379 commits into
mainfrom
dev-lead/issue-846-20260621-0727

Conversation

@don-petry

@don-petry don-petry commented Jun 21, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #846

Implemented by dev-lead agent. Please review.

Summary by CodeRabbit

  • New Features

    • Added LSP cold-start caching and startup timing checks to improve setup reliability and skip enrichment when startup exceeds the allowed budget.
    • Added new telemetry for cold-start performance, including cache status and whether LSP setup was skipped.
  • Bug Fixes

    • Simplified review and audit flows by removing LSP-based finding verification, reducing unexpected severity changes and output fields.

@don-petry
don-petry requested a review from a team as a code owner June 21, 2026 07:45
@coderabbitai

coderabbitai Bot commented Jun 21, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@don-petry, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 47 minutes and 38 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b2d25dba-f6e7-485c-8ae9-6c275efb3f2e

📥 Commits

Reviewing files that changed from the base of the PR and between d42a879 and 55e2cb0.

📒 Files selected for processing (1)
  • tests/dev-lead/unit/test_lsp_coldstart_sla.bats
📝 Walkthrough

Walkthrough

The PR removes LSP finding-verification references from prompts and review scripts, replaces verification JSONL with cold-start telemetry, adds LSP index caching plus SLA-based auto-skip in pilot setup, and trims review-automation, engine, and Dependabot cleanup.

Changes

LSP cold-start SLA and verification removal

Layer / File(s) Summary
Remove finding-verification prompts and call sites
prompts/deep-review.md, prompts/security-audit.md, scripts/review-one-pr.sh, scripts/lib/lsp-verification.sh
deep-review.md and security-audit.md drop the LSP finding-verification step and lsp_verification field, review-one-pr.sh stops invoking apply_lsp_verification, and the helper module is removed.
Cold-start metric emission
scripts/lib/token-metrics.sh, scripts/token_report.sh, tests/token_report.bats
emit_lsp_coldstart_record replaces the verification JSONL shape, token_report.sh filters rows by null .metric, and the token-report test drops the old finding_verification case.
Cache-aware cold-start setup
docs/lsp-pilot.md, scripts/setup-lsp-pilot.sh, .github/workflows/pr-review.yml
The LSP pilot docs shift to cold-start SLA, caching, auto-skip, and Phase 2 wording; setup-lsp-pilot.sh adds install/probe timing, JSON-RPC initialize probing, SLA gating, telemetry, and a sourced-vs-executed entrypoint guard; and pr-review.yml adds the actions/cache LSP index step plus LSP_INDEX_CACHE_HIT wiring.
Cold-start test coverage
tests/dev-lead/unit/test_lsp_coldstart_sla.bats, tests/dev-lead/unit/test_lsp_pilot.bats
The Bats coverage checks telemetry fields, SLA thresholds, cache status, under- and over-budget runs, install failure, workflow wiring, and the richer agent-lsp stub.

Review automation and engine cleanup

Layer / File(s) Summary
Review marker and health cleanup
scripts/dev-lead-fix-reviews.sh, scripts/pr_review_health.sh
REVIEWS_MARKER_PREFIX initialization changes, a duplicated helper block is removed, and pr_review_health.sh drops failure-rate, percentile, and overall calculations.
Engine defaults and config cleanup
scripts/engine.sh, .github/dependabot.yml
scripts/engine.sh removes the early COPILOT_API_MODEL default and updates related comments, and .github/dependabot.yml removes its top-level version field.

Sequence Diagram(s)

sequenceDiagram
  participant prReview as pr-review.yml
  participant cacheStep as actions/cache
  participant setupLsp as scripts/setup-lsp-pilot.sh
  participant bashLs as bash-language-server
  participant agentLsp as agent-lsp
  participant tokenMetrics as scripts/lib/token-metrics.sh
  prReview->>cacheStep: restore LSP index cache when enabled
  cacheStep-->>prReview: cache-hit output
  prReview->>setupLsp: LSP_INDEX_CACHE_HIT env
  setupLsp->>bashLs: install pinned version
  setupLsp->>agentLsp: send initialize request
  setupLsp->>tokenMetrics: emit_lsp_coldstart_record
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

needs-human-review

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR also removes the LSP finding-verification pipeline, related tests, and docs/prompt updates that are not required by issue #846. Keep the change focused on cache, telemetry, and SLA auto-skip; move verification-pipeline removals to a separate follow-up if needed.
Docstring Coverage ⚠️ Warning Docstring coverage is 46.88% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 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: LSP index caching plus 30s cold-start auto-skip for Phase 2.
Linked Issues check ✅ Passed The PR adds LSP index caching, cold-start telemetry, and SLA-based auto-skip behavior that matches issue #846's acceptance criteria.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-lead/issue-846-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

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

@don-petry

Copy link
Copy Markdown
Collaborator Author

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

No changes were needed for this PR.

@don-petry
don-petry enabled auto-merge (squash) June 21, 2026 07:46
@don-petry
don-petry disabled auto-merge June 21, 2026 07:47
@donpetry-bot

Copy link
Copy Markdown
Contributor

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

@don-petry

Copy link
Copy Markdown
Collaborator Author

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

Agent reasoning
Issues addressed: 0
Files changed: none
Skipped (informational): 0
✓ No action required — Quality Gate passed with no issues.
```

@don-petry
don-petry enabled auto-merge (squash) June 21, 2026 07:47

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces cold-start SLA tracking and index-cache instrumentation for the LSP pilot. It adds logging of cold-start metrics to the Token Cost Observatory, implements timing and SLA checks to auto-skip LSP if bring-up exceeds the 30-second SLA, and adds comprehensive BATS unit tests. A review comment identifies a resource leak in the unit tests where temporary files and directories are not properly cleaned up in the teardown function, particularly when assertions fail.

Comment thread tests/dev-lead/unit/test_lsp_coldstart_sla.bats
@don-petry
don-petry disabled auto-merge June 21, 2026 07:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d3396a720

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread scripts/lib/token-metrics.sh
Comment thread scripts/setup-lsp-pilot.sh
Comment thread tests/dev-lead/unit/test_lsp_coldstart_sla.bats
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 21, 2026
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

@chatgpt-codex-connector

Copy link
Copy Markdown

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.

@don-petry
don-petry disabled auto-merge June 21, 2026 07:54
@don-petry

Copy link
Copy Markdown
Collaborator Author

Dev-Lead — review-changes (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) June 21, 2026 08:04
@chatgpt-codex-connector

Copy link
Copy Markdown

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.

@don-petry
don-petry disabled auto-merge June 21, 2026 08:05
@donpetry-bot

Copy link
Copy Markdown
Contributor

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

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jun 21, 2026
@don-petry

Copy link
Copy Markdown
Collaborator Author

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

Changes committed and pushed.

@chatgpt-codex-connector

Copy link
Copy Markdown

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.

@don-petry
don-petry disabled auto-merge June 21, 2026 08:10
@don-petry
don-petry enabled auto-merge (squash) June 21, 2026 08:12
@chatgpt-codex-connector

Copy link
Copy Markdown

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ack-test-deletion Acknowledge intentional test deletion (bypasses test-deletion-guard)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Phase 2] Cache the LSP index and enforce the 30s cold-start SLA with auto-skip

2 participants