fix(retry): charge gate budget on any repeated failure fingerprint (#460)
gateFailureFingerprints held one slot per gate, so the budget only charged when a failure repeated back to back. Whack-a-mole (fix A breaks B, fix B breaks A) alternates two fingerprints forever, every round read as progress, and the per-gate budget never triggered. Track the full set seen per gate and charge when the current fingerprint is a repeat. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+6
-3
@@ -19,9 +19,12 @@ data class OrchestrationState(
|
||||
// brief_grounding, brief_echo, contract, plan_compile, static_analysis, execution, review, ...)
|
||||
// exhausts its own budget rather than sharing retryCount session-wide. Attempts charged so far.
|
||||
val gateRetryBudgets: Map<String, Int> = emptyMap(),
|
||||
// Last-seen failure fingerprint per gate, used to tell a no-progress retry (same fingerprint,
|
||||
// charged) from a genuine-progress retry (changed fingerprint, free).
|
||||
val gateFailureFingerprints: Map<String, String> = emptyMap(),
|
||||
// Every failure fingerprint seen per gate, used to tell a no-progress retry (a fingerprint
|
||||
// already seen for this gate, charged) from a genuine-progress retry (an unseen fingerprint,
|
||||
// free). A set, not a single slot: whack-a-mole (fix A breaks B, fix B breaks A) alternates two
|
||||
// fingerprints forever and a single slot reads every round as progress, so nothing is ever
|
||||
// charged and the budget never triggers.
|
||||
val gateFailureFingerprints: Map<String, Set<String>> = emptyMap(),
|
||||
// Gates that have already spent their one hybrid-exhaustion salvage reset (review gate only,
|
||||
// see RetrySalvageDecidedEvent) — a second exhaustion for that gate is terminal.
|
||||
val gateSalvageUsed: Set<String> = emptySet(),
|
||||
|
||||
+2
-1
@@ -83,7 +83,8 @@ class DefaultOrchestrationReducer : OrchestrationReducer {
|
||||
} else {
|
||||
state.gateRetryBudgets
|
||||
},
|
||||
gateFailureFingerprints = state.gateFailureFingerprints + (p.gate to p.fingerprint),
|
||||
gateFailureFingerprints = state.gateFailureFingerprints +
|
||||
(p.gate to ((state.gateFailureFingerprints[p.gate] ?: emptySet()) + p.fingerprint)),
|
||||
)
|
||||
|
||||
is RetrySalvageDecidedEvent -> if (p.decision == SalvageDecision.CONTINUE) {
|
||||
|
||||
@@ -26,9 +26,10 @@ class DefaultRetryCoordinator(
|
||||
policy: RetryPolicy,
|
||||
): RetryDecision {
|
||||
val fingerprint = FailureFingerprint.of(failureReason)
|
||||
val prevFingerprint = state.gateFailureFingerprints[gate]
|
||||
val progressed = prevFingerprint != fingerprint
|
||||
val charged = !progressed
|
||||
// Charge when this failure has been seen before for this gate — a repeat means the round
|
||||
// trip produced no state the gate has not already rejected. Comparing against only the
|
||||
// previous fingerprint misses cycles: A/B/A alternation reads as progress forever.
|
||||
val charged = fingerprint in (state.gateFailureFingerprints[gate] ?: emptySet())
|
||||
val currentCount = state.gateRetryBudgets[gate] ?: 0
|
||||
val newCount = if (charged) currentCount + 1 else currentCount
|
||||
val maxAttempts = policy.maxAttemptsFor(gate)
|
||||
|
||||
@@ -179,7 +179,7 @@ class DefaultRetryCoordinatorTest {
|
||||
val policy = RetryPolicy(maxAttempts = 3, backoffMs = 0L)
|
||||
val state = OrchestrationState(
|
||||
gateRetryBudgets = mapOf("contract" to 1),
|
||||
gateFailureFingerprints = mapOf("contract" to FailureFingerprint.of("same-reason")),
|
||||
gateFailureFingerprints = mapOf("contract" to setOf(FailureFingerprint.of("same-reason"))),
|
||||
)
|
||||
|
||||
val decision = retryCoordinator.decide(
|
||||
@@ -200,7 +200,7 @@ class DefaultRetryCoordinatorTest {
|
||||
// changed fingerprint (progress) must still be free and retry.
|
||||
val state = OrchestrationState(
|
||||
gateRetryBudgets = mapOf("contract" to 1),
|
||||
gateFailureFingerprints = mapOf("contract" to "old-reason"),
|
||||
gateFailureFingerprints = mapOf("contract" to setOf("old-reason")),
|
||||
)
|
||||
|
||||
val decision = retryCoordinator.decide(
|
||||
@@ -218,7 +218,7 @@ class DefaultRetryCoordinatorTest {
|
||||
val policy = RetryPolicy(maxAttempts = 2, backoffMs = 0L)
|
||||
val state = OrchestrationState(
|
||||
gateRetryBudgets = mapOf("contract" to 2),
|
||||
gateFailureFingerprints = mapOf("contract" to FailureFingerprint.of("same-reason")),
|
||||
gateFailureFingerprints = mapOf("contract" to setOf(FailureFingerprint.of("same-reason"))),
|
||||
)
|
||||
|
||||
val decision = retryCoordinator.decide(
|
||||
@@ -234,7 +234,7 @@ class DefaultRetryCoordinatorTest {
|
||||
val policy = RetryPolicy(maxAttempts = 1, backoffMs = 0L)
|
||||
val state = OrchestrationState(
|
||||
gateRetryBudgets = mapOf("contract" to 1),
|
||||
gateFailureFingerprints = mapOf("contract" to FailureFingerprint.of("same-reason")),
|
||||
gateFailureFingerprints = mapOf("contract" to setOf(FailureFingerprint.of("same-reason"))),
|
||||
)
|
||||
|
||||
// "contract" is already exhausted at maxAttempts=1, but "review" has never charged.
|
||||
@@ -245,12 +245,33 @@ class DefaultRetryCoordinatorTest {
|
||||
assertEquals(RetryDecision.Retry, decision)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `decide charges a whack-a-mole repeat — fingerprint seen earlier but not last round`() = runTest {
|
||||
val policy = RetryPolicy(maxAttempts = 3, backoffMs = 0L)
|
||||
// A/B/A: round 3 repeats round 1's failure. The previous fingerprint is B, so a single-slot
|
||||
// comparison would read this as progress and never charge.
|
||||
val state = OrchestrationState(
|
||||
gateRetryBudgets = mapOf("contract" to 1),
|
||||
gateFailureFingerprints = mapOf(
|
||||
"contract" to setOf(FailureFingerprint.of("A"), FailureFingerprint.of("B")),
|
||||
),
|
||||
)
|
||||
|
||||
retryCoordinator.decide(
|
||||
sessionId, stageId, gate = "contract", failureReason = "A", state = state, policy = policy,
|
||||
)
|
||||
|
||||
val event = eventStore.read(sessionId).single().payload as RetryAttemptedEvent
|
||||
assertEquals(true, event.charged)
|
||||
assertEquals(2, event.attemptNumber)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `decide honours perGateMaxAttempts override`() = runTest {
|
||||
val policy = RetryPolicy(maxAttempts = 1, backoffMs = 0L, perGateMaxAttempts = mapOf("review" to 5))
|
||||
val state = OrchestrationState(
|
||||
gateRetryBudgets = mapOf("review" to 4),
|
||||
gateFailureFingerprints = mapOf("review" to "same-reason"),
|
||||
gateFailureFingerprints = mapOf("review" to setOf(FailureFingerprint.of("same-reason"))),
|
||||
)
|
||||
|
||||
val decision = retryCoordinator.decide(
|
||||
|
||||
@@ -167,7 +167,7 @@ class OrchestrationReducerTest {
|
||||
),
|
||||
)
|
||||
assertEquals(1, retried.gateRetryBudgets["contract"])
|
||||
assertEquals("fp1", retried.gateFailureFingerprints["contract"])
|
||||
assertEquals(setOf("fp1"), retried.gateFailureFingerprints["contract"])
|
||||
}
|
||||
|
||||
@Test
|
||||
@@ -191,7 +191,7 @@ class OrchestrationReducerTest {
|
||||
),
|
||||
)
|
||||
assertEquals(1, free.gateRetryBudgets["contract"])
|
||||
assertEquals("fp2", free.gateFailureFingerprints["contract"])
|
||||
assertEquals(setOf("fp1", "fp2"), free.gateFailureFingerprints["contract"])
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user