[contrasam]

Roadmap item from the Boudin review: per-call retries and timeouts. Until now Catalyst retried only the whole task, and a hung provider call had no limit.

What

Catalyst.builder(dir).callPolicy(CallPolicy.retries(3, Duration.ofMillis(200)).timeout(Duration.ofSeconds(30)))
ctx.call(lookupTool, orderId, CallPolicy.retries(5, Duration.ofSeconds(1)));
ctx.model(CallPolicy.timeoutAfter(Duration.ofSeconds(60))).complete(request);

• Retries run inside the attempt.
  • The backoff doubles each time, capped by maxBackoff (30s by default).
  • retryOn(predicate) chooses which failures are retried.
  • A runtime-wide default (Builder.callPolicy) applies to any call that names no policy. Per call, use ctx.call(tool, input, policy) and ctx.model(policy).
• Only the outcome is recorded: one request, then its result or its last failure. Each retry leaves an informational CallRetried (a new, additive event), which the timeline and OTel show. A replay substitutes the outcome, so a call that needed three tries replays like one that needed one.
• Timeout: each attempt runs on its own virtual thread. When it times out, the thread is interrupted and CallTimeoutException is thrown, which the policy can retry. A provider may still have accepted the call, and that's documented.
• Streams are retried only before the first chunk; re-delivering text would double what the task assembled. They get no timeout, so the sink stays on the task thread.
• Stopping:
  • A cancel during the retries surfaces as the cancel, so the execution ends CANCELLED rather than FAILED. The tool's real last failure is still recorded.
  • A shutdown surfaces the last failure to the existing drain decision, so a model request is closed rather than left in doubt.
• No change to the boundary queue or to replay. The default is CallPolicy.none(): one attempt, no timeout.

Tests

• CallPolicyTest (9 tests):
  • a tool is retried in place, with one request, one result and replay-exact;
  • retries run out and the last failure is recorded;
  • the predicate;
  • the runtime default;
  • model retries under one request;
  • timeout then retry, and timeout alone;
  • a stream retried only before the first chunk;
  • a cancel stops a 5s backoff promptly and ends CANCELLED.
• CallPolicyUnitTest (5 tests): attempts, backoff doubling and cap, the predicate, validation.
• CallsAcceptanceTest + Demo calls, added as a CI gate. A provider answers 503 twice and a tool hangs once (cut at 1s). The task runs in one attempt with 0 whole-task retries, and replay makes 0 calls.
• Mutation-checked: each of these fails a test:
  • never retrying;
  • no CallRetried;
  • no timeout;
  • a stream retried after chunks;
  • a cancel not surfaced;
  • recording the signal's text instead of the real failure.

🤖 Generated with Claude Code

[claude]

The in-doubt RETRY path records the cancel signal's text, not the tool's real failure.

This line now goes through applyWithPolicy. That means invokeWithPolicy → stoppedRetrying can throw a CancellationSignal here, with the real failure attached as a suppressed exception. runToolLive unwraps that with lastRealFailure(ex), but the catch block below still records String.valueOf(ex) (line 671: append(new ToolCompleted(now(), null, String.valueOf(ex), latencyMillis))).

Scenario:

1. InDoubtPolicy.RETRY is set, and the runtime has a callPolicy(CallPolicy.retries(5, 5s)).
2. A resume finds a dangling ToolRequested.
3. The retried call fails once, then cancel(id) lands during the backoff.
4. The log gets ToolCompleted(error = "…CancellationSignal: …") instead of the tool's real error.

That is the exact case the PR's mutation check ("recording the signal's text instead of the real failure") guards, but only on the runToolLive path. The recorded error is what inspect, the timeline and OTel show, and it is what a replay substitutes as RecordedToolFailureException.

Fix: use String.valueOf(lastRealFailure(ex)) at line 671, as runToolLive does.

(Minor: the execution still ends CANCELLED. Only the recorded failure text is wrong.)

[claude]

Review of c24b666 (CallPolicy: per-call retries and timeouts)

Files read:

• CallPolicy.java
• ReplayingContext.java: seed, the model boundary and in-doubt completion, invokeAndRecord, the tool boundary, answerFromRecording, runToolLive, handleInDoubt, and the new invokeWithPolicy / stoppedRetrying / waitBackoff / withTimeout
• Reducer.java (timeline fold)
• TrajectoryDiff.java
• AutoCapture.java
• Agent.java (engine-signal handling)
• CatalystRuntime.java: the callPolicy wiring in runAttempt and runBranch, and recordedToolResults
• CallPolicyTest.java, CallsDemo.java
• The diffs to CatalystEvent, Context, TimelineStep and CatalystTracer

Checked and found sound:

• seed ignores CallRetried (it hits the default branch). So the boundary queue and in-doubt detection are unchanged, including a crash between CallRetried and the outcome.
• Branch tool pairing (recordedToolResults) and the reducer's TOOL fold are unaffected.
• TrajectoryDiff compares only work steps, so the new CALL_RETRIED timeline step does not disturb diffs.
• The event change is additive.
• Stopping: a stop is checked before CallRetried is appended. Abandon never interrupts. A cancel surfaces as CancellationSignal, and Agent rethrows it before isRecordedToolFailure.
• AutoCapture uses a plain ThreadLocal, so the timeout worker thread shares no capture binding.

Confirmed findings: 1

1. Minor (ReplayingContext.handleInDoubt, in-doubt RETRY): a cancel during per-call retries records the CancellationSignal's text as the tool's error. The real failure is lost because this path lacks the lastRealFailure(ex) unwrap that runToolLive got. The execution still ends CANCELLED; only the recorded error text is wrong.

Verdict: looks correct apart from the one minor fix above. I did not build or run anything; this review is from reading the code only.

[claude]

A graceful shutdown during a per-call backoff fails the execution for good.

Scenario: callPolicy(CallPolicy.retries(3, 1s)) with the default RetryPolicy.none(). A tool fails once with a transient error, and close() lands during the backoff (or right after the failure, since stopRequested() is checked first).

1. waitBackoff returns false, and this path returns the raw failure.
2. runToolLive records ToolCompleted(error) and rethrows.
3. In runAttempt, raisedAbandon() is false and the throwable isn't an ExecutionAbandonedSignal, so stoppedByAbandon is false and control reaches the retry decision (CatalystRuntime ~L1714). With RetryPolicy.none(), nextBackoff is empty, so it calls recordFailed and writes a terminal ExecutionFailed.

The model path behaves the same way, writing ExecutionFailed after the open CompletionRequested.

This goes against Deploy-4 ("close() abandons running attempts instead of failing them"). The call's own policy would have retried, so this failure was still in progress, not final. A routine deploy now permanently fails every execution that happens to be in a call-level backoff, and recovery can't undo that.

The tool path already records the request's outcome. It could record the real failure, set raisedAbandon, and throw ExecutionAbandonedSignal. On restart the recorded failure would be substituted, though, so it also needs a durable "this failure gets retried" verdict. One option is a RetryRequested naming the ToolCompleted seq (which seed() already drops). That way the next start re-runs the call live. The model path can use the same verdict to close its request.

[claude]

With a timeout and no retries, a cancel ends the execution FAILED instead of CANCELLED.

Scenario: ctx.call(tool, in, CallPolicy.timeoutAfter(30s)), or ctx.model(timeoutAfter(...)), followed by runtime.cancel(id) while the call is running.

1. tripCancel interrupts the task thread, which is blocked in result.get.
2. That throws RuntimeException("… was interrupted") from here.
3. In invokeWithPolicy, policy.retries(1, failure) is false because maxAttempts == 1, so it rethrows the plain failure and stoppedRetrying never runs.
4. runToolLive records the error, and runAttempt sees no CancellationSignal or raisedCancellation. It then reaches backoff.isEmpty() || token.isCancelled() and calls recordFailed, which writes ExecutionFailed.

Without a timeout, a tool that ignores the interrupt finishes, and the next boundary's requireAppendable turns the cancel into CANCELLED. So adding a timeout alone turns cancels into failures. The PR's "a cancel … ends CANCELLED" test covers only the retrying case.

Suggested fix: when cancellation != null && cancellation.isCancelled() here, set raisedCancellation and throw a CancellationSignal. Alternatively, have invokeWithPolicy send stop-time failures through stoppedRetrying even when the policy wouldn't retry.

[claude]

Review of d59000e (CallPolicy: per-call retries and timeouts)

Files read:

• CLAUDE.md
• the PR diff for Context.java, CallPolicy.java, CallTimeoutException.java, Reducer.java, ReplayingContext.java, TimelineStep.java, CatalystEvent.java, CatalystTracer.java and CatalystRuntime.java
• in the checkout:
  • ReplayingContext.seed()
  • CatalystRuntime.recordedToolResults, the runAttempt failure / stop-rule path and tripCancel
  • the engine-signal handling in Agent.java

Confirmed findings: 2 (posted inline)

1. A shutdown during a per-call backoff fails the execution for good. stoppedRetrying hands the raw failure to runAttempt. With RetryPolicy.none() that leads to recordFailed, which writes ExecutionFailed for a failure the call policy would still have retried. This goes against Deploy-4's rule that close() abandons attempts rather than failing them.
2. With a timeout and no retries, a cancel ends FAILED instead of CANCELLED. The cancel's interrupt reaches withTimeout's wait and comes out as a plain RuntimeException. Because maxAttempts == 1, stoppedRetrying never runs, so the cancel is recorded as a failure.

Checked, no problem found:

• CallRetried between a request and its result is ignored by seed() (falls to default) and by recordedToolResults pairing, so replay, in-doubt detection and branch tool replay are unaffected.
• The event is additive.
• A cancel that cuts retries short records the real failure, and Agent still rethrows the CancellationSignal before isRecordedToolFailure.

Verdict: changes requested. Replay and log correctness look intact, but both findings change an execution's terminal state on the stop paths.

I did not build or run anything; this review comes from reading the code only.

[claude]

A shutdown during a per-call backoff uses up the task's whole-task RetryPolicy budget.

This RetryRequested folds through Reducer (case RetryRequested rr -> retries++). The runtime then compares that counter with the task's policy: policy.nextBackoff(state.retries(), t) in CatalystRuntime.runAttempt.

Scenario:

• Settings: RetryPolicy.maxRetries(1, …) plus CallPolicy.retries(3, 5s).
• A tool fails once, and close() lands during the 5s backoff.
• This line writes RetryRequested(failedSeq), so retries == 1. The task was never retried as a whole.
• After restart, the resumed attempt succeeds at this call and later hits a genuine transient failure.
• maxRetries(1) sees retriesSoFar == 1 and returns empty, so the execution ends FAILED where it should have got its one whole-task retry.

CLAUDE.md does say the restart "counts one retries". But the effect on the RetryPolicy budget isn't stated, and it contradicts the PR's framing that per-call retries are separate from whole-task retries. The TimelineReport / OTel retries figure is inflated the same way.

Possible fixes:

• Mark these (e.g. a negative backoffMillis, or a distinct cause prefix) and have the reducer skip the budget bump while keeping seed's drop and request-closing behaviour.
• Or document the budget cost explicitly.

[claude]

Review of 682cf56: per-call retries and timeouts (CallPolicy)

I read the code only. I did not build it or run any tests or demos.

Files read

• The full PR diff (gh pr diff 54).
• In the checkout:
  • ReplayingContext.java: seed, the model and tool boundaries, invokeWithPolicy, stoppedRetrying, recordRetryOnRestart, withTimeout.
  • CatalystRuntime.java: runAttempt's stop rule and retry decision, isRetryable, recordedToolResults.
  • Reducer.java: pairing of tool steps, and the RetryRequested fold.
  • RetryPolicy.java, AutoCapture.java (thread-local binding), CatalystTracer.java (request/completion pairing), Agent.java (engine-signal rethrow order).
  • CallPolicyTest, CallPolicyUnitTest, and the CLAUDE.md additions.

Confirmed findings: 1

1. ReplayingContext.recordRetryOnRestart (L1394): if a shutdown lands during a per-call backoff, the RetryRequested it writes adds one to the folded retries counter. The runtime gives that counter to the task's RetryPolicy.nextBackoff. So with RetryPolicy.maxRetries(1), a genuine failure after the restart ends FAILED instead of getting its one whole-task retry. Posted inline. The counter bump is acknowledged in CLAUDE.md, but the effect on the budget is not.

Checked, nothing wrong found

• CallRetried between the request and its outcome: seed ignores it, and seed, the reducer (lastToolStepIndex), OTel and eval's recordedToolResults all pair by type, not by adjacency. The boundary queue and strict replay are unchanged.
• A shutdown that cuts a model call's retries writes RetryRequested(-1), which closes the dangling CompletionRequested. Resume re-issues the call instead of treating it as in doubt.
• On the tool path, the failed ToolCompleted is dropped by failedSeq, both on resume and in eval branches.
• A cancel surfaces as CancellationSignal with raisedCancellation set, so the execution ends CANCELLED. Agent rethrows signals before its isRecordedToolFailure check, even though the signal is now lastToolFailureException.
• Auto-capture suppression for timed-out tools still holds on the worker thread, because applyUnchecked runs there and the state is thread-local.
• The CallRetried event is an additive schema change.

Verdict: no correctness blocker in the replay or recovery paths. One low-to-medium semantic issue: a shutdown during per-call retries uses up the whole-task retry budget. Worth fixing or documenting explicitly before merge.

[claude]

Review of d59f848: no confirmed findings.

I did not build or run anything. This review comes from reading the code only.

Files read

• The full PR diff.
• catalyst-core/.../engine/ReplayingContext.java: seed, fork, requireAppendable, append, and the model, tool and in-doubt paths.
• catalyst-runtime/.../CatalystRuntime.java: runAttempt's stop rule, the abandon/cancel/retry decision, and recordedToolResults.
• catalyst-core/.../engine/Reducer.java: the TOOL step pairing by index.
• catalyst-core/.../engine/TrajectoryDiff.java: isWork.
• catalyst-otel/.../CatalystTracer.java: tool/model span pairing.
• catalyst-agents/.../Agent.java: the signal-vs-recorded-failure check.
• catalyst-core/.../capture/AutoCapture.java: a plain ThreadLocal, so a timeout worker thread does not inherit the binding.
• CallPolicyTest.java, CallsDemo.java, CallPolicyUnitTest.java, EventCodecTest.java, and the CLAUDE.md section.

What I checked

• CallRetried between request and outcome. seed pairs by type and ignores the event (default case). The reducer attaches ToolCompleted by lastToolStepIndex, the OTel tracer by pendingTool, and recordedToolResults by its pending request. TrajectoryDiff filters CALL_RETRIED out as non-work. Boundary queues, replay and M2 diffs are unaffected.
• Shutdown mid-backoff.
  • Tool: ToolCompleted(error) with the real failure, then RetryRequested(failedSeq, callRetry=true). On restart, seed drops that failure and clears the pending request, so the call re-runs live.
  • Model: RetryRequested(-1) clears pendingCompletionRequest, so the request is not in doubt on restart.
  • runAttempt then takes the abandon branch and appends nothing more. callRetry is excluded from retries, so the task's RetryPolicy budget is untouched.
• Lease-lost abandon. This also goes through stoppedRetrying. A takeover that already appended fences these writes through the cursor (StaleWriterException, then the fencedOut() exit).
• Cancel during backoff or timeout wait. It surfaces as CancellationSignal with raisedCancellation set, so the execution ends CANCELLED. The agent checks isEngineSignal before isRecordedToolFailure, so a signal stored as lastToolFailureException is never fed to the model.
• Schema. RetryRequested.callRetry is additive (NON_DEFAULT) and the 4-arg constructor is kept. No record-deconstruction patterns on RetryRequested exist in the repo.

Not defects, noted for awareness

• A crash during a model call's backoff leaves CompletionRequested plus CallRetried with no verdict, so the restart treats it as in doubt. That is conservative: from the log alone it can't be told apart from a crash mid-retry.
• CallPolicy.retries(Integer.MAX_VALUE, …) overflows maxAttempts and silently means no retries. This only bites at extreme values.

Verdict: looks correct. No inline comments.