Skip to content

Review task: don't update last_seen_review_id when LLM response fails #67

@ClaydeCode

Description

@ClaydeCode

Problem

When addressing a PR review, if the LLM invocation fails (e.g. due to an API overload/overloaded_error), the CLI backend returns an error JSON instead of raising `UsageLimitError`. The `parse_response` call then raises a `ValueError`, which is caught and logged, but the code continues and still updates `last_seen_review_id` to mark the review as seen. The review is effectively dropped.

This was observed for FreeshardBase/freeshard-controller#160 (PR #163):

Failed to parse address_review response JSON: LLM response failed validation for AddressReviewResponse: 1 validation error for AddressReviewResponse
  Field required [type=missing, input_value={'type': 'error', 'error'...}, input_type=dict]
Data: {'type': 'error', 'error': {'type': 'overloaded_error', 'message': 'Overloaded'}, 'request_id': 'req_011CZqrNqKPPVeYVfV5ZseM3'}

Despite the failure, the log then shows "Review comments addressed" — which is incorrect.

Root Cause

In tasks/review.py, the except ValueError block (lines 133–136) falls through to the state update that sets last_seen_review_id. This should not happen when the failure is due to an LLM/API error rather than a minor formatting issue.

Fix

In the except ValueError block, check if the error output looks like an API error (e.g. {'type': 'error', ...}). If so:

  • Log at error level
  • Return early without updating last_seen_review_id
  • Optionally set status to a retryable state (e.g. stay at pr_open)

This ensures that on the next turn, the review is picked up and addressed again.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions