Repository navigation
feat(#846): cache the LSP index + enforce the 30s cold-start SLA with auto-skip (clean redo of #860) - #922
Conversation
…th auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 57 minutes and 42 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 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-25T03:57:43Z. |
There was a problem hiding this comment.
Code Review
This pull request implements LSP cold-start SLA tracking and index-cache wiring, adding helper functions to measure elapsed time, check SLA compliance, and log cold-start metrics, along with a comprehensive suite of unit tests. The review feedback suggests several robustness improvements: validating the number of positional parameters in emit_lsp_coldstart_record to prevent unbound variable crashes under set -u, using %.0f in awk to avoid integer overflow when formatting epoch milliseconds, and replacing a fragile integer validation check with a cleaner case statement pattern.
…u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr
Dev-Lead — waiting on PR blockers (intent: fix-reviews)PR: #922 |
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 173cac17b08a260de94f66a9c22a868e18121d1b
Review mode: triage-approved (single reviewer)
Summary
Additive Phase-2 LSP-pilot change (story #846, epic #839): adds index caching + a 30s cold-start SLA with graceful auto-skip. 4 files, +323/-5; the 5 deletions are just the dispatcher case being wrapped in a source-guard (no behavior removed). New work is a SHA-pinned actions/cache step (reusing the already-vetted 27d5ce7…v5.0.5 SHA also used at pr-review.yml:211), a set -u-safe emit_lsp_coldstart_record on the existing JSONL channel, pure timing/SLA helpers, and a 12+-case bats suite. Off-pilot runs are byte-for-byte unchanged (everything gated behind LSP_PILOT_ENABLED). Triage tier's low-risk assessment confirmed.
Linked issue analysis
Closes #846. Substantively addressed: AC #1 (pr-review.yml SHA-pinned actions/cache for ~/.local, keyed on OS+candidate+pinned tool versions+hashFiles, threads cache-hit into setup), AC #2 (discriminated kind:"lsp_cold_start" record so #844 can compute P50/P95; token_report.sh's (.kind // "token_usage")=="token_usage" filter excludes it from cost aggregation), AC #3 (over-SLA bring-up auto-skips LSP wiring via the existing warn-don't-fail contract, exit 0). Deliberately scoped clean of #860's accidental deletions — #843's verification step and dev-lead-fix-reviews.sh are untouched; no ack-test-deletion needed.
Findings
No blocking findings.
- Workflow security: new actions/cache is SHA-pinned to the same vetted SHA already in use at pr-review.yml:211 (not a guessed pin), gated behind LSP_PILOT_ENABLED, scoped to ~/.local. No untrusted action, no injection surface. GitHub cache scoping isolates per-branch caches, so cross-PR cache poisoning is mitigated.
- Robustness (advisory): all three gemini-code-assist comments are resolved in commit 8836b8e — awk %.0f avoids 32-bit epoch-ms overflow; emit_lsp_coldstart_record is set -u-safe (positional defaults rather than gemini's suggested arg-count guard — equivalent, matches emit_token_record house style); _lsp_sla_exceeded uses a clean case check. SonarCloud quality gate passed (0 new issues).
- Downstream: pr-review.yml is consumed by 5 repos, but the change is purely additive + opt-in, so consumers not setting LSP_PILOT_ENABLED=true are unaffected.
- Secret scan: run_secret_scanning MCP tool not available in this environment; gitleaks CI check passed. No secrets in diff.
CI status
All required checks green. Lint/ShellCheck/bats/unit-tests/CodeQL(actions,python)/SonarCloud/AgentShield/Holdout-Guard/Test-Deletion-Guard/Secret-scan(gitleaks) all SUCCESS; dependency-audit ecosystem jobs and dependabot SKIPPED (expected). Advisory bots: CodeRabbit APPROVED at head 173cac1; gemini comments addressed; Codex/CodeRabbit earlier rate-limit notices were on older SHAs. mergeStateStatus BLOCKED only due to the org-leads human-review gate (REVIEW_REQUIRED), not a failing check.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #922 |
|
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. |
|
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-06-25T06:57:58Z. |
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>
… auto-skip (clean redo of #860) (#922) * feat(#846): cache the LSP index and enforce the 30s cold-start SLA with auto-skip Phase 2 of the LSP pilot (epic #839). Additive only — builds on story #842's setup wiring; does not touch story #843's finding-verification step or the existing tier-gating. - scripts/lib/token-metrics.sh: add emit_lsp_coldstart_record — a discriminated kind:"lsp_cold_start" JSONL record (cold_start_ms, cache, skipped, sla_ms, reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. token_report.sh already keeps only (.kind // "token_usage") == "token_usage", so the record is excluded from cost aggregation untouched. - scripts/setup-lsp-pilot.sh: time the toolchain bring-up, normalize the actions/cache outcome (hit/miss/unknown), emit the cold-start record, and auto-skip wiring the MCP knobs when cold-start > LSP_COLD_START_SLA_MS (default 30000) via the existing "warn, don't fail" contract — the review proceeds on base capabilities and the workflow never fails. Pure helpers (_lsp_sla_exceeded, _lsp_cache_status, _lsp_elapsed_ms, _lsp_now_ms) are unit-tested; the dispatcher is source-guarded so sourcing doesn't run main. - .github/workflows/pr-review.yml: SHA-pinned actions/cache (reusing the vetted v5.0.5 SHA) for ~/.local, keyed on OS + candidate + pinned tool versions + hashFiles of the Shell sources; threads cache-hit into the setup step. Opt-in behind LSP_PILOT_ENABLED; off-pilot runs are byte-for-byte unchanged. - tests: new test_lsp_coldstart_sla.bats (12 cases) — SLA decision, cache-status, elapsed clamp, metric record + no-op, end-to-end warm/auto-skip/degraded paths, and the workflow cache wiring. Existing #842 and token-metrics suites stay green. Closes #846 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr * fix(#846): address gemini review — awk %.0f, cleaner int check, set -u-safe metric - scripts/setup-lsp-pilot.sh (_lsp_now_ms): format epoch ms with awk %.0f, not %d — epoch ms (~1.75e12) overflows the 32-bit %d conversion on common awks (mawk), which would yield a garbage cold-start measurement (the very signal #844 needs). - scripts/setup-lsp-pilot.sh (_lsp_sla_exceeded): validate the SLA with an explicit `case "${sla#-}"` (strip one leading minus, require all-digits) instead of the `[ "$sla" -eq "$sla" ]` self-compare; negatives still work as the test seam. - scripts/lib/token-metrics.sh (emit_lsp_coldstart_record): set -u-safe positional defaults so a short call degrades to a defaulted record rather than crashing (matches emit_token_record's house style). - tests: +2 cases (negative SLA seam, non-numeric SLA). Suite 14/14; #842 + token -metrics regressions 56/56; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016Y3xW3cnB5wscv4SmjSNEr --------- Co-authored-by: Claude <noreply@anthropic.com>



Closes #846. Clean redo of #860, which went off the rails (377 commits, net −982 lines) and deleted unrelated/merged work; see the close note on #860.
Why a fresh PR
#860 was supposed to be a small, additive caching + SLA story but had drifted into deleting story #843's already-merged finding-verification step (
scripts/lib/lsp-verification.sh+ tests + thelsp_verificationprompt wiring) and an unrelated 317-line script (scripts/dev-lead-fix-reviews.sh), carrying anack-test-deletionlabel. Rather than untangle 377 commits, this re-implements only the in-scope #846 work on a fresh branch off currentmain.What this does (additive only — 157 insertions, 5 deletions, 4 files)
scripts/lib/token-metrics.shemit_lsp_coldstart_record— a discriminatedkind:"lsp_cold_start"JSONL record (cold_start_ms,cache,skipped,sla_ms,reason) on the Token Cost Observatory channel so #844 can compute cold-start P50/P95. AC #2scripts/setup-lsp-pilot.shactions/cacheoutcome to hit/miss/unknown, emit the record, and auto-skip wiring the MCP knobs when cold-start >LSP_COLD_START_SLA_MS(default 30000) via the existing "warn, don't fail" contract. Pure helpers (_lsp_sla_exceeded,_lsp_cache_status,_lsp_elapsed_ms,_lsp_now_ms); dispatcher source-guarded for unit testing. AC #2/#3.github/workflows/pr-review.ymlactions/cache(reusing the vettedv5.0.5SHA) for~/.local, keyed on OS + candidate + pinned tool versions +hashFilesof the Shell sources; threadscache-hitinto the setup step. Opt-in behindLSP_PILOT_ENABLED. AC #1tests/dev-lead/unit/test_lsp_coldstart_sla.batsScope guarantees
lsp-verification.sh, its tests, anddev-lead-fix-reviews.share untouched. Noack-test-deletionneeded.LSP_PILOT_ENABLED, andtoken_report.shalready filters to(.kind // "token_usage") == "token_usage", so the new record is excluded from cost aggregation with no change totoken_report.sh.claudesubprocess); it's exactly what the index cache eliminates. Documented inline.Tests
tests/dev-lead/unit/test_lsp_coldstart_sla.bats— 12/12 green.test_lsp_pilot.bats([Phase 2] Wire candidate LSP-MCP server(s) into the pr-review runtime — config, install, tier gating #842) 14/14,test_token_metrics.bats42/42.shellcheck --severity=warning -xclean;pr-review.ymlvalidates as YAML.🤖 Generated with Claude Code
Generated by Claude Code