Skip to content

fix(review): preserve terminal escalation evidence - #1819

Merged
decode2 merged 1 commit into
mainfrom
fix/4553-terminal-escalation-decoder
Oct 6, 2026
Merged

decode2 merged 1 commit into
mainfrom
fix/4553-terminal-escalation-decoder

Conversation

@decode2

@decode2 decode2 commented Oct 6, 2026 •

Copy link
Copy Markdown
Member

Linked issue

Refs Gentleman-Programming/gentle-ai#4553 (status:approved, nonclosing).

PR type

  • Bug fix (type:bug)

Summary

  • Decode the native escalation diagnostic on terminal capture closures using the existing STATUS parser.
  • Preserve cause, finding IDs, and refuter evidence in Pi tool output. Escalated authority remains terminal, with no acknowledgement, retry, or new continuation.
  • Cover single-lens and four-lens completion, malformed evidence, and the actual registered-tool/native-provider boundary.

Review path

Area Change
lib/review-integration-v2.ts and generated runtime Reuse strict escalation decoding; retain compatibility with previously accepted closures that omit diagnostic evidence.
extensions/gentle-ai.ts Preserve native evidence in the mapped terminal closure.
Unit and devbinary tests Verify no replay/reconciliation after a successful terminal capture and matching persisted native authority.

Review the decoder and mapper first. Native lifecycle, finding-rationale retrieval, recovery, and the separate targeted-validator evidence compatibility issue are out of scope.

Verification

  • RED on pristine main, then GREEN on this candidate, including a registered Pi tool against the real native binary with a faux reviewer (no live model execution).
  • 98 focused tests: node --experimental-strip-types --test tests/review-last-event-closure.test.ts tests/review-integration-v2.test.ts tests/native-review-cli.test.ts.
  • New native E2E: both dev-binary environment variables set, then node --experimental-strip-types --test --test-name-pattern='terminal single-lens escalation' tests/devbinary/pi-host-relay.devtest.ts.
  • 24 comparisons of actual native 4.0.0/main closures through both TypeScript and generated runtime preserve state and diagnostics.
  • Type baseline has no regressions; generated runtime parity, provider-contract, runtime harness, and git diff --check pass.
  • Independent read-only candidate validation found no introduced blockers.
  • Full suites green: pnpm test reports 4,955 passed, 2 failed, 44 skipped. Pristine main reports the same two routing-test failures (4,951 passed, 2 failed, 44 skipped). The expanded devbinary file has the same two selected-untracked/mode-isolation failures in main and candidate; the new escalation E2E passes. These failures were not modified or waived.

Scope and rollback

244 changed lines across five files. Revert this commit as one unit to restore the previous decoder and generated runtime. No native authority, consent, delivery, or terminal-state policy changes.

Summary by CodeRabbit

  • New Features
    • Escalated review closures now include the escalation cause and related finding IDs, with optional refuter outcomes.
    • Escalation evidence is available directly in the completed review result, without requiring a follow-up status check.
  • Bug Fixes
    • Escalation data is accepted only for escalated closures and validated consistently. Historical escalated closures without evidence remain supported.

Reuse the existing STATUS escalation decoder for terminal capture closures and retain its native diagnostic evidence in tool output without adding a continuation.

Refs: Gentleman-Programming/gentle-ai#4553

Validation: 98 focused tests, real native single-lens committed-candidate E2E, type baseline without regressions, runtime parity, provider-contract and runtime harness. Full unit and expanded devbinary suites retain failures independently reproduced on pristine main.
@decode2 decode2 added the type:bug Bug fix label Oct 6, 2026
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: ebfe9f90-8e97-4839-9597-5af066977927
📥 Commits

Reviewing files that changed from the base of the PR and between c806cf3 and 84d824c.

📒 Files selected for processing (5)
  • extensions/gentle-ai.ts
  • lib/review-integration-v2.ts
  • runtime/review-integration-v2.mjs
  • tests/devbinary/pi-host-relay.devtest.ts
  • tests/review-last-event-closure.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Escalation data is now decoded for last-event closures and included in escalated closure output. Tests cover escalation evidence, closure capture behavior, and terminal status from the reviewer relay.

Changes

Escalation Closure Data

Layer / File(s) Summary
Shared escalation decoding
lib/review-integration-v2.ts, runtime/review-integration-v2.mjs
Both implementations use a shared decoder to validate escalation data. Status decoding now calls this decoder.
Escalation data in closures
lib/review-integration-v2.ts, runtime/review-integration-v2.mjs, extensions/gentle-ai.ts
Last-event closures accept escalation data only when the state is escalated. The closure mapper serializes finding IDs and optional refuter outcomes.
Closure and relay tests
tests/review-last-event-closure.test.ts, tests/devbinary/pi-host-relay.devtest.ts
Tests cover valid and invalid escalation data, closure capture without post-success STATUS requests, and terminal relay status with escalation evidence.

Priority: ➖ Normal

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

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 84d82

The escalation-evidence change is mergeable after normal checks; no unresolved issue attributable to this PR is established.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 13 functions across 5 files. 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 concisely describes the main change: preserving terminal escalation evidence in review output.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@decode2
decode2 merged commit 1384e72 into main Oct 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:bug Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant