Skip to content

feat(quality-gate): hung-reviewer retry policy with configurable constants#413

Merged
sansari merged 3 commits into
mainfrom
fix/408-reviewer-timeout-retry
May 20, 2026
Merged

feat(quality-gate): hung-reviewer retry policy with configurable constants#413
sansari merged 3 commits into
mainfrom
fix/408-reviewer-timeout-retry

Conversation

@sansari
Copy link
Copy Markdown
Contributor

@sansari sansari commented May 20, 2026

Goal

Closes #408 (also tracked as deepwork-frontend#87). Reviewers that complete with 0 tool uses — a signal of API overload or dropped connection — were previously silently passed or required manual mark_review_as_passed calls, wasting ~4–8 minutes per incident.

Changes

  • src/deepwork/jobs/mcp/quality_gate.py

    • Add REVIEWER_MAX_RETRIES = 1 and REVIEWER_FAST_FAIL_SECONDS = 30 as named module-level constants (not magic numbers)
    • Extend _build_review_guidance() with a "Handling Hung Reviewers" section instructing Claude to: detect 0-tool-use returns, retry once, and after exhausting retries call mark_review_as_passed with a "manual review recommended" skip note
    • Fast-fail path: if elapsed time < REVIEWER_FAST_FAIL_SECONDS the reviewer never got an API response — retry immediately
    • _build_review_guidance() now accepts max_retries and fast_fail_seconds kwargs for test overrides and future caller configuration
  • doc/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md

    • Add JOBS-REQ-004.9 (7 sub-requirements) covering the hung-reviewer retry policy
  • tests/unit/jobs/mcp/test_quality_gate.py

    • Import REVIEWER_MAX_RETRIES, REVIEWER_FAST_FAIL_SECONDS, and _build_review_guidance
    • Add TestHungReviewerRetryPolicy (9 tests) covering all JOBS-REQ-004.9 acceptance criteria
  • CHANGELOG.md — entry in [Unreleased]

Demo

$ uv run pytest tests/unit/jobs/mcp/test_quality_gate.py -v -k TestHungReviewerRetryPolicy
...
9 passed in 1.70s

$ uv run pytest tests/
...
1309 passed, 1 skipped, 1 xfailed in 7.84s

Acceptance criteria from #408:

  • ✅ Hung reviewers (0 tool uses) are retried up to REVIEWER_MAX_RETRIES times
  • ✅ Fast-fail detection for sub-REVIEWER_FAST_FAIL_SECONDS returns
  • ✅ After retries exhausted: skip with logged note, user informed — workflow does not block
  • ✅ Constants are named and configurable (not hardcoded)

🤖 Generated with Claude Code

…tants

Reviewers that return with 0 tool uses (API overload / dropped connection)
are now surfaced with explicit retry instructions rather than being silently
passed or requiring manual intervention.

- Add REVIEWER_MAX_RETRIES (default: 1) and REVIEWER_FAST_FAIL_SECONDS
  (default: 30) module-level constants to quality_gate.py
- Extend _build_review_guidance() with a "Handling Hung Reviewers" section
  that instructs Claude to detect 0-tool-use returns, retry once, and — after
  exhausting retries — call mark_review_as_passed with a skip note so the
  workflow proceeds without blocking indefinitely
- Fast-fail detection: if elapsed time < REVIEWER_FAST_FAIL_SECONDS the
  reviewer never got an API response; retry immediately
- _build_review_guidance() now accepts max_retries and fast_fail_seconds
  kwargs for test overriding and future caller configuration
- Add JOBS-REQ-004.9 to spec and 9 new tests covering all acceptance criteria

Closes #408

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

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

This PR updates the quality gate’s review-dispatch guidance and specification to handle “hung” reviewer agents (those returning with 0 tool uses) by retrying a configurable number of times and then skipping with a “manual review recommended” note, avoiding wasted wall time and manual intervention.

Changes:

  • Added module-level constants for hung-reviewer retry behavior and extended _build_review_guidance() to include a “Handling Hung Reviewers” section with configurable parameters.
  • Documented the hung-reviewer retry policy as JOBS-REQ-004.9 in the quality review system spec.
  • Added unit tests covering JOBS-REQ-004.9 and updated the changelog.

Reviewed changes

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

File Description
src/deepwork/jobs/mcp/quality_gate.py Adds retry-policy constants and injects hung-reviewer handling guidance into the quality gate output.
tests/unit/jobs/mcp/test_quality_gate.py Adds a new test class validating the hung-reviewer retry policy guidance and configurability.
doc/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md Specifies hung-reviewer retry requirements (JOBS-REQ-004.9).
CHANGELOG.md Notes the new hung-reviewer retry policy behavior under Unreleased additions.

Comment thread src/deepwork/jobs/mcp/quality_gate.py Outdated
Comment thread src/deepwork/jobs/mcp/quality_gate.py Outdated
Comment thread doc/specs/deepwork/jobs/JOBS-REQ-004-quality-review-system.md Outdated
sansari and others added 2 commits May 19, 2026 18:09
- Fix hardcoded "once"/"If the retry also" — now parameterized via
  retry_instruction/exhaust_condition locals so the prose stays correct
  for any value of max_retries
- Fix mark_review_as_passed guidance: the tool only accepts review_id;
  skip note is now communicated to the user separately, not as a phantom
  tool argument
- Add slow-hang path: guidance now explicitly describes both fast-fail
  (elapsed < REVIEWER_FAST_FAIL_SECONDS, retry immediately) and slow-hang
  (elapsed >= threshold, retry after brief pause)
- Fix JOBS-REQ-004.9 spec: moved to end of file (after 004.8), corrected
  requirement 4 and 5 to match implementation
- Strengthen test_guidance_specifies_max_retries: checks "max N retry/ies"
  not just presence of "retry"
- Fix test_build_review_guidance_accepts_override_params: checks specific
  formatted strings "max 3 retries" / "4 failed attempts" / "60"
- Add test_guidance_describes_slow_hang_path (REQ-004.9.5)
- Add test_guidance_instructs_separate_user_message_not_tool_comment
  (REQ-004.9.4)
- Add JOBS-REQ-004.8 and JOBS-REQ-004.9 to module docstring

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wrap two long ternary expressions in quality_gate.py and one long
assert expression in test_quality_gate.py into parenthesized
multi-line form per ruff line-length rules.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sansari sansari self-assigned this May 20, 2026
@sansari sansari requested review from ncrmro and nhorton May 20, 2026 01:28
@sansari sansari added this pull request to the merge queue May 20, 2026
Merged via the queue into main with commit cba737b May 20, 2026
5 checks passed
@sansari sansari deleted the fix/408-reviewer-timeout-retry branch May 20, 2026 17:45
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.

Category C: Hard timeout and retry for failed reviewer agents

3 participants