Skip to content

feat(review): enhance diagnostic propagation for negotiated failures - #221

Open
Juanjo-Ramirez wants to merge 2 commits into
Gentleman-Programming:mainfrom
Juanjo-Ramirez:main
Open

feat(review): enhance diagnostic propagation for negotiated failures#221
Juanjo-Ramirez wants to merge 2 commits into
Gentleman-Programming:mainfrom
Juanjo-Ramirez:main

Conversation

@Juanjo-Ramirez

@Juanjo-Ramirez Juanjo-Ramirez commented Jul 23, 2026

Copy link
Copy Markdown

Review diagnostic propagation handoff

The Pi adapter repair is proven: after verified package-local Gentle AI v2.1.11 provisioning, local provider inspection succeeds and an authorized fresh ordinary START returns a sanitized provider diagnostic. The raw negotiated failure payload is not exposed.

Review first

  1. Read patch.diff for the four-file repair.
  2. Run the test-only reproduction in reproduction.md from a source checkout.
  3. Check validation.md before treating the installed package as validated.

Impact and decision

Topic Maintainer conclusion
Impact A caller receives the operation-scoped, sanitized provider diagnostic needed to explain a failed negotiated START reconciliation.
Pi adapter conclusion Repaired and proven: the post-install ordinary START reports only the sanitized review/start diagnostic captured in observed-start-failure.json.
Remaining native root failure Gentle AI v2.1.11 emits only the generic non-zero / operation_outcome_unknown diagnostic. Native maintainers must add stage-level sanitized failure evidence.
Repair invariant For a negotiated unknown START reconciled to start with no lineage, expose only sanitized diagnostics for review/start; never expose native_failure or raw payload content.
Source base v1.2.0 at 113906a34c1b527ba2d65806b7638bff32b8e916.
Source branch fix/review-diagnostic-propagation at 4d5214b410d352712be20917e81f9ce5974d039a, with the repair present as working-tree changes.

Evidence boundary

Proven: the focused source regression passes; package-local Gentle AI v2.1.11 provisioning was verified; local provider inspection succeeds; and the authorized fresh ordinary START returned the sanitized diagnostic recorded in observed-start-failure.json.

Native escalation required: the remaining native failure is generic (non-zero / operation_outcome_unknown) and lacks stage-level sanitized failure evidence. Native maintainers must add that evidence before the root cause can be diagnosed.

Authority boundary: status reconciliation reports no lineage and action start. No replay is safe, and this evidence cannot authorize a commit. This bundle does not authorize creating, replaying, or inspecting a real review lineage.

Tracker paths

Safety statement

This bundle contains no private authority material, credentials, tokens, native receipts, real review snapshots, target values, repository identities, or user identities. patch.diff redacts a test-only sentinel value and excludes this handoff directory, so it is a review artifact rather than an apply-ready patch.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of native operation failures so sanitized process diagnostics are consistently captured and surfaced in responses.
    • Updated failure mapping to compute diagnostics earlier and avoid exposing raw integration failure details.
    • Ensured native START failure reconciliation preserves correct outcome/next-action values while redacting sensitive stderr content.
  • Tests
    • Added/updated integration and routing tests to verify diagnostics attachment timing, redaction behavior, and absence of raw failure fields in returned details.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7a39364d-b851-4b8c-88be-d66b6e656b42

📥 Commits

Reviewing files that changed from the base of the PR and between f8eae9d and 12b754a.

📒 Files selected for processing (2)
  • tests/native-review-integration-v1.test.ts
  • tests/review-controller-native-routing.test.ts

📝 Walkthrough

Walkthrough

Native integration failures now carry process diagnostics through both native CLI implementations. Controller routing recognizes these errors, sanitizes diagnostics, omits raw failure envelopes, and validates negotiated unknown START failure responses.

Changes

Native diagnostics propagation

Layer / File(s) Summary
Native integration diagnostics transport
lib/native-review-cli.ts, runtime/native-review-cli.mjs, tests/native-review-integration-v1.test.ts
NativeReviewIntegrationError stores diagnostics, negotiated native failures pass computed diagnostics into the error, and tests verify redacted stderr.
Controller diagnostics routing and validation
extensions/gentle-ai.ts, tests/review-controller-native-routing.test.ts
Controller mapping recognizes integration errors, routes diagnostics for native and pre-native START failures, removes raw failure-envelope output, and tests sanitized diagnostics.

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

Possibly related PRs

Suggested labels: type:feature

Suggested reviewers: alan-thegentleman

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 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: improved diagnostic propagation for negotiated review failures.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/review-controller-native-routing.test.ts`:
- Around line 1163-1174: Update the fixture around NativeReviewCliV216.invoke to
instantiate the actual NativeReviewIntegrationError rather than assigning its
name onto a plain Error. Add an adapter-level non-zero-envelope test that
verifies invoke attaches diagnostics before controller sanitization, and ensure
the test exercises the changed native transport path.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3f05779b-9a5e-45bc-864f-5b2c7b197bc7

📥 Commits

Reviewing files that changed from the base of the PR and between dfca69b and f8eae9d.

⛔ Files ignored due to path filters (8)
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/README.md is excluded by !openspec/changes/**
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/manifest.json is excluded by !openspec/changes/**
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/observed-start-failure.json is excluded by !openspec/changes/**
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/patch.diff is excluded by !openspec/changes/**
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/reference/failure.fixture.json is excluded by !openspec/changes/**
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/reference/failure.schema.json is excluded by !openspec/changes/**
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/reproduction.md is excluded by !openspec/changes/**
  • openspec/changes/consolidate-review-parity-runtime/upstream-trackers/handoff-122-review-diagnostic-propagation/validation.md is excluded by !openspec/changes/**
📒 Files selected for processing (4)
  • extensions/gentle-ai.ts
  • lib/native-review-cli.ts
  • runtime/native-review-cli.mjs
  • tests/review-controller-native-routing.test.ts

Comment thread tests/review-controller-native-routing.test.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant