Skip to content

feat: configurable retry policy as a top-level retry: root#44

Closed
uipreliga wants to merge 3 commits into
mainfrom
feat/configurable-retry-policy
Closed

feat: configurable retry policy as a top-level retry: root#44
uipreliga wants to merge 3 commits into
mainfrom
feat/configurable-retry-policy

Conversation

@uipreliga

Copy link
Copy Markdown
Collaborator

Summary

Split out of #34, addressing @akshaylive's review on that PR:

The weight 0 fix LGTM. The configurable retry policy should ideally be a separate PR. Additionally the retry policy configuration shouldn't be in "run_limits" (retry policy is not really a limit, so a separate top level field is better). I haven't looked too deep into the impl but would this clear the working directory between retries?

All three points are taken:

Comment Resolution
Should be a separate PR This PR. #34 is now the weight-0 fix alone.
Shouldn't live under run_limits Moved to a top-level retry: block and its own -D root — see below.
Does it clear the working directory between retries? No. Answered in detail below and documented in the guide.

Stacked on fix/weight-zero-gating-and-retry-policy (#34) — retarget to main once that merges.

Configurable retry policy

  • New RetryPolicy (models/retry.py): max_retries / initial_delay / backoff_multiplier, all optional.
  • It lives at the task's top level (retry:), not under run_limits. Per review: run_limits holds caps that abort a run, whereas a retry policy is how hard the run tries to survive a transient error.
  • retry is a first-class 4th -D root alongside agent/run_limits/sandbox: -D retry.max_retries=0 to fail fast while debugging. It merges through the same resolve_root engine at every layer (experiment defaults → task → variant → CLI), so the unification invariant holds — two new tests/test_merge_unification.py cases pin it.
  • All retry decisions resolve through one seam, errors/retry.py::resolve_retry_config. A category that is non-retryable in the built-in table (auth/billing) is never made retryable by a per-run override — retryability is a property of the error, not the run.
  • Persisted error_details.is_retryable / retry_delay_seconds in task.json now reflect the run's actual policy instead of the built-in defaults.

Review question: does this clear the working directory between retries?

No — and the guide now says so explicitly. A retried communicate() re-sends the prompt into the same sandbox and the same agent session; the working directory is not reset, so files the failed attempt already wrote survive into the retry. The crashed attempt's partial turn is also preserved on the result (deliberate, for crash forensics), which means trajectory-observing criteria (command_executed, commands_efficiency, skill_triggered) see both attempts.

I chose to document this rather than add a reset seam: resetting mid-task would discard work the agent already did and budget already spent. retry.max_retries: 0 is the escape hatch when you need a clean single-attempt trajectory. Happy to add the reset if reviewers prefer it.

Test plan

  • make verify green — 3450 passed, 2 skipped, ruff + pyright + custom CE lint clean.
  • New tests: retry-policy resolution + fail-fast, error_details policy fidelity, -D retry.* override + field-merge + did-you-mean on an unknown key, and merge-unification parity for the new root.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @uipreliga's task in 1m 25s —— View job


Code Review in Progress

Review Tasks:

  • Read .github/code_review.md for review standards
  • Read CLAUDE.md for project conventions
  • Run git diff to see full changes
  • Review all 16 changed files with full context
  • Check cross-file consistency
  • Analyze "what's missing"
  • Provide design-level feedback
  • Complete final review summary

Starting review now...


- weight: 0 now makes a criterion purely informational — excluded from
  both the weighted score AND the pass/fail gate (previously it could
  still flip a task to FAILURE). New BaseSuccessCriterion.is_gating drives
  the single-source gate (all_criteria_passed) and the `coder-eval
  evaluate` exit code. A validator rejects weight:0 combined with
  stop_when or suite_thresholds (arming a non-gating criterion for a
  pass/fail gate is incoherent).
- reports.py failure-reason sampling uses pass_threshold, not score < 1.0.
- Doc drift: dropped the non-existent `rephrase` mutation; corrected the
  stale "max_memory_mb NOT enforced" claim (docker --memory/--cpus/--pids-limit).

Split out per review: the configurable retry policy moved to #44, and the
evalboard OSS-edition change is dropped entirely (see @bai-uipath's review
comment — the panels need core-produced fields that do not exist yet).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@uipreliga
uipreliga force-pushed the fix/weight-zero-gating-and-retry-policy branch from c746087 to 8c02dc6 Compare July 22, 2026 21:40
@uipreliga
uipreliga force-pushed the feat/configurable-retry-policy branch from 863f7dd to cc54a8b Compare July 22, 2026 21:41
uipreliga and others added 2 commits July 22, 2026 14:54
Closes the follow-up deferred in #34. `final_status` and both exit codes
already ignored weight:0 criteria, but the display sites still labeled a
below-threshold informational criterion as FAILED — a header-vs-list
contradiction where the task reads SUCCESS while a row reads FAIL.

The fix is one carried marker rather than four independent re-derivations:

- `CriterionResult.gating` (default True) mirrors `BaseSuccessCriterion.
  is_gating`, stamped by `SuccessChecker` alongside `pass_threshold` on all
  three construction paths. Defaulting True means task.json files written
  before this field read back as gating.
- checker log line says "below threshold (informational)" / "errored
  (informational)" instead of FAILED.
- reports_html: rows tagged "informational — not gated (weight: 0)"; the
  "(n/m passed)" header counts gating criteria only and appends
  "+ k informational".
- reports: suite failure-reason sampling skips non-gating criteria — an
  informational miss must not explain why a row failed.
- evalboard: DTO carries `gating` + `passThreshold`; the pill renders INFO
  (gray) for non-gating criteria, and the criteria header counts them
  separately. Drive-by: the pill compared `score === 1`, so a fractional
  criterion passing at 0.95 with threshold 0.9 rendered FAIL — it now
  compares against the criterion's own threshold.

Tests: gating is stamped from the criterion; results default to gating on
read-back; the HTML header excludes informational criteria and labels the
row; suite sampling omits an informational miss (verified failing without
the fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split out of #34 per review, with the placement feedback applied.

- New RetryPolicy (models/retry.py): max_retries / initial_delay /
  backoff_multiplier, all optional. It lives at the task's TOP LEVEL
  (`retry:`), not under `run_limits` — `run_limits` holds caps that abort a
  run, while a retry policy is how hard the run tries to survive a transient
  error.
- `retry` is a first-class 4th `-D` root alongside agent/run_limits/sandbox:
  `-D retry.max_retries=0` to fail fast while debugging. It merges through
  the SAME resolve_root engine at every layer (experiment defaults, task,
  variant, CLI), so the unification invariant holds — covered by new
  test_merge_unification cases.
- All retry decisions resolve through one seam, errors/retry.py::
  resolve_retry_config. A category that is non-retryable in the built-in
  table (auth/billing) is never made retryable by a per-run override —
  retryability is a property of the error, not the run.
- Persisted error_details.is_retryable / retry_delay_seconds now reflect the
  run's actual policy instead of the built-in defaults.
- Docs: TASK_DEFINITION_GUIDE gains a `retry` section that documents the
  placement rationale AND the in-place retry semantics raised in review —
  a retried communicate() re-sends into the same sandbox and session (the
  working directory is NOT reset), and the crashed attempt's partial turn is
  preserved, so trajectory-observing criteria see both attempts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@uipreliga
uipreliga force-pushed the feat/configurable-retry-policy branch from cc54a8b to 3ec8a05 Compare July 22, 2026 22:03
uipreliga added a commit that referenced this pull request Jul 22, 2026
* fix: weight:0 un-gates criteria (informational criteria)

- weight: 0 now makes a criterion purely informational — excluded from
  both the weighted score AND the pass/fail gate (previously it could
  still flip a task to FAILURE). New BaseSuccessCriterion.is_gating drives
  the single-source gate (all_criteria_passed) and the `coder-eval
  evaluate` exit code. A validator rejects weight:0 combined with
  stop_when or suite_thresholds (arming a non-gating criterion for a
  pass/fail gate is incoherent).
- reports.py failure-reason sampling uses pass_threshold, not score < 1.0.
- Doc drift: dropped the non-existent `rephrase` mutation; corrected the
  stale "max_memory_mb NOT enforced" claim (docker --memory/--cpus/--pids-limit).

Split out per review: the configurable retry policy moved to #44, and the
evalboard OSS-edition change is dropped entirely (see @bai-uipath's review
comment — the panels need core-produced fields that do not exist yet).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: render weight:0 criteria as informational on every display surface

Closes the follow-up deferred in #34. `final_status` and both exit codes
already ignored weight:0 criteria, but the display sites still labeled a
below-threshold informational criterion as FAILED — a header-vs-list
contradiction where the task reads SUCCESS while a row reads FAIL.

The fix is one carried marker rather than four independent re-derivations:

- `CriterionResult.gating` (default True) mirrors `BaseSuccessCriterion.
  is_gating`, stamped by `SuccessChecker` alongside `pass_threshold` on all
  three construction paths. Defaulting True means task.json files written
  before this field read back as gating.
- checker log line says "below threshold (informational)" / "errored
  (informational)" instead of FAILED.
- reports_html: rows tagged "informational — not gated (weight: 0)"; the
  "(n/m passed)" header counts gating criteria only and appends
  "+ k informational".
- reports: suite failure-reason sampling skips non-gating criteria — an
  informational miss must not explain why a row failed.
- evalboard: DTO carries `gating` + `passThreshold`; the pill renders INFO
  (gray) for non-gating criteria, and the criteria header counts them
  separately. Drive-by: the pill compared `score === 1`, so a fractional
  criterion passing at 0.95 with threshold 0.9 rendered FAIL — it now
  compares against the criterion's own threshold.

Tests: gating is stamped from the criterion; results default to gating on
read-back; the HTML header excludes informational criteria and labels the
row; suite sampling omits an informational miss (verified failing without
the fix).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Base automatically changed from fix/weight-zero-gating-and-retry-policy to main July 22, 2026 23:25
@uipreliga uipreliga closed this Jul 23, 2026
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