Skip to content

feat(chat-api): persist review results and skip re-review for reviewed reports#304

Closed
willvelida wants to merge 0 commit intomainfrom
feat/chat-api-review-persistence
Closed

feat(chat-api): persist review results and skip re-review for reviewed reports#304
willvelida wants to merge 0 commit intomainfrom
feat/chat-api-review-persistence

Conversation

@willvelida
Copy link
Copy Markdown
Owner

Summary

Updates Chat.Api's A2AReportTool to persist review results back to Reporting.Api and skip Claude re-review for already-reviewed reports. This is PR 2 of Issue #269 (Phase 2 of the ReportReviewerService fail-closed fix).

After this change, the full review persistence flow works end-to-end:

  1. User asks for report status → GET /api/reports/{jobId} returns "generated"
  2. Chat.Api calls Claude reviewer → persists result via PUT /api/reports/{jobId}/review
  3. Next status check → GET returns "reviewed" → Chat.Api presents persisted review data without calling Claude

Changes

A2AReportTool.cs

  • Split status handling: "generated"ReviewPersistAndPresentAsync (review + PUT + present), "reviewed"PresentPersistedReview (skip Claude, present persisted data)
  • Fire-and-forget PUT: After successful Claude review, persists result via PUT /api/reports/{jobId}/review. Failure is non-blocking (logs warning, still presents review to user) per ASI08.
  • New PresentPersistedReview method: Reads ReviewApproved, ReviewConcerns, ReviewValidatedSummary from GET metadata. Approved reports show validated summary; flagged reports show concerns list.
  • Updated ReportMetadataDto: Added 4 review fields (ReviewApproved, ReviewConcerns, ReviewValidatedSummary, ReviewedAt) to deserialize persisted review data.

Tests (6 new)

  • PersistReviewResult_WhenStatusIsGenerated_AndReviewCompleted — verifies PUT called
  • SkipPersistence_WhenStatusIsGenerated_AndReviewNotCompleted — verifies PUT NOT called
  • PresentPersistedReview_WhenStatusIsReviewed_AndApproved — verifies reviewer NOT called
  • PresentPersistedConcerns_WhenStatusIsReviewed_AndNotApproved — verifies concerns in output
  • HandlePutFailureGracefully_WhenPersistenceFails — PUT throws, result still returned
  • HandlePut409Gracefully_WhenAlreadyReviewed — PUT returns 409, result still returned

Validation

  • 188 tests pass (170 unit + 15 integration + 3 evaluation), 0 failures
  • 82.9% line coverage (threshold: 70%)
  • GetReportStatusTool (obsolete) was NOT modified — only A2AReportTool was updated

Context

Copilot AI review requested due to automatic review settings April 20, 2026 07:26
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates Biotrackr.Chat.Api’s A2AReportTool to persist Claude review results back to Reporting.Api and to skip re-review when a report is already in the "reviewed" state, presenting persisted review metadata instead.

Changes:

  • Split "generated" vs "reviewed" status handling to either (a) review+persist+present or (b) present persisted review directly.
  • Added best-effort PUT /api/reports/{jobId}/review persistence after a completed review.
  • Extended ReportMetadataDto and added unit tests for persistence + skip behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/Biotrackr.Chat.Api/Biotrackr.Chat.Api/Tools/A2AReportTool.cs Adds review persistence PUT and introduces a fast-path to present persisted review data for "reviewed" reports.
src/Biotrackr.Chat.Api/Biotrackr.Chat.Api.UnitTests/Tools/A2AReportToolShould.cs Adds unit tests covering PUT persistence, skip persistence, skip reviewer on reviewed status, and graceful handling of PUT failures/409.

Comment thread src/Biotrackr.Chat.Api/Biotrackr.Chat.Api/Tools/A2AReportTool.cs Outdated
Comment thread src/Biotrackr.Chat.Api/Biotrackr.Chat.Api/Tools/A2AReportTool.cs Outdated
Comment thread src/Biotrackr.Chat.Api/Biotrackr.Chat.Api.UnitTests/Tools/A2AReportToolShould.cs Outdated
@github-actions
Copy link
Copy Markdown

Code Coverage

Package Line Rate Branch Rate Health
Biotrackr.Chat.Api 83% 65%
Summary 83% (763 / 923) 65% (127 / 196)

Minimum allowed line rate is 70%

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.

2 participants