fix(kernel): make the ACR steer-away hint genuinely one-shot per retry (#306)
Root cause: unconfirmedFixEntries() re-derived the hint on every context build from "the latest RetryAttemptedEvent for this stage," with no guard on whether that retry occurrence had already been delivered. A single contradicted retry (e.g. a workflow-routing dead end, gate="stage") therefore kept re-injecting its "Steer away from a known dead end" block on every subsequent stage retry/rebuild, even once the model had moved on to a materially different sub-problem — live evidence: session d734e1de, stage scaffold_frontend, injected on 7 consecutive turns. Fix (direction 1 from the three proposed): key delivery to the specific RetryAttemptedEvent occurrence and check whether it was already delivered by folding prior ContextAssembledEvent manifests (#307) for sourceType="unconfirmedFix"/sourceId=classKey recorded after that retry's own sessionSequence — no new mutable state, replay-safe (invariant #9). A fresh contradicted retry now injects exactly once; later rebuilds for the same occurrence stay silent; a NEW RetryAttemptedEvent of the same class (a real recurrence) earns one fresh injection. Direction 2 (match the current turn's own classKey, not "latest in stage") turned out to be already true by construction — classKey already comes from the single "latest" retry being matched — so the mismatch in the evidence was purely temporal/sticky, which direction 1 resolves. Implementing 2 separately would have been redundant; noted rather than silently dropped. Direction 3 (tighten the signature so routing and build failures can't collapse): already structurally true — routing dead-ends ("no transition condition matched...") only ever reach RetryAttemptedEvent via DefaultRetryCoordinator.shouldRetry(), which defaults gate="stage", while every other failure path uses .decide() with its real gate (build/contract/lint/...). Since classKey = "$gate: $normalizedSignature", the two families can never share a classKey. Added a regression test locking this in rather than changing the scheme. Docstring on unconfirmedFixEntries updated to describe the one-shot mechanism precisely instead of just asserting it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HgDL1v3GuQ9RZnYR6fDT95
This commit is contained in:
+56
-7
@@ -4,6 +4,7 @@ import com.correx.core.context.model.ContextEntry
|
||||
import com.correx.core.context.model.ContextLayer
|
||||
import com.correx.core.context.model.EntryRole
|
||||
import com.correx.core.events.events.ConceptPromotedEvent
|
||||
import com.correx.core.events.events.ContextAssembledEvent
|
||||
import com.correx.core.events.events.RetryAttemptedEvent
|
||||
import com.correx.core.events.events.StoredEvent
|
||||
import com.correx.core.events.types.ContextEntryId
|
||||
@@ -81,12 +82,24 @@ internal suspend fun SessionOrchestrator.promotedConceptEntries(stageConfig: Sta
|
||||
* `classKey` is in [state.promoted][com.correx.core.kernel.concept.ConceptCompilerState.promoted] the
|
||||
* hard-promoted delivery ([promotedConceptEntries]) already covers it, so this is skipped to avoid
|
||||
* double delivery.
|
||||
*
|
||||
* Genuinely one-shot per retry occurrence (#306): the hint is keyed to the LATEST
|
||||
* [RetryAttemptedEvent] for this stage, and is only injected while that retry hasn't yet been
|
||||
* delivered — derived by folding prior [ContextAssembledEvent] manifests (sourceType="unconfirmedFix",
|
||||
* sourceId=classKey) recorded AFTER that retry's own position in the log. So a fresh contradicted
|
||||
* retry injects the steer-away exactly once (the first context build following it); every later
|
||||
* rebuild for the SAME retry occurrence — whether more tool rounds in this attempt or a subsequent
|
||||
* stage retry that hasn't reproduced the class again — sees the prior delivery and stays silent. A
|
||||
* later, NEW `RetryAttemptedEvent` of the same classKey (the class recurred) advances "latest" past
|
||||
* that delivery and earns one fresh injection of its own. No new mutable state: purely a fold over
|
||||
* existing events (invariant #9).
|
||||
*/
|
||||
internal suspend fun SessionOrchestrator.unconfirmedFixEntries(
|
||||
sessionEvents: List<StoredEvent>,
|
||||
stageId: StageId,
|
||||
): List<ContextEntry> {
|
||||
val latest = sessionEvents.mapNotNull { it.payload as? RetryAttemptedEvent }.lastOrNull { it.stageId == stageId }
|
||||
val latestRetry = sessionEvents.lastOrNull { (it.payload as? RetryAttemptedEvent)?.stageId == stageId }
|
||||
val latest = latestRetry?.payload as? RetryAttemptedEvent
|
||||
val sig = latest?.failureReason?.lineSequence()?.firstOrNull()?.take(SIGNATURE_MAX)?.trim().orEmpty()
|
||||
val classKey = latest?.let { conceptClassKey(it.gate, sig) }
|
||||
val projection = ConceptCompilerProjection()
|
||||
@@ -104,19 +117,55 @@ internal suspend fun SessionOrchestrator.unconfirmedFixEntries(
|
||||
(cluster.fixPath?.let { " in `$it`" } ?: "") + " — worth trying first, but it hasn't " +
|
||||
"recurred enough times across sessions to be a certain fix here. Verify it actually applies."
|
||||
else -> null
|
||||
} ?: return emptyList()
|
||||
return listOf(
|
||||
}
|
||||
val deliverable = deliverableUnconfirmedFix(content, classKey, latestRetry, sessionEvents)
|
||||
val entry = deliverable?.let { (text, key) ->
|
||||
ContextEntry(
|
||||
id = ContextEntryId(UUID.randomUUID().toString()),
|
||||
layer = ContextLayer.L1,
|
||||
content = content,
|
||||
content = text,
|
||||
sourceType = "unconfirmedFix",
|
||||
sourceId = classKey.orEmpty(),
|
||||
tokenEstimate = estimateTokens(content),
|
||||
sourceId = key,
|
||||
tokenEstimate = estimateTokens(text),
|
||||
role = EntryRole.USER,
|
||||
),
|
||||
)
|
||||
}
|
||||
return listOfNotNull(entry)
|
||||
}
|
||||
|
||||
/**
|
||||
* (content, classKey) pair to deliver, or null if any of the one-shot preconditions fail: no
|
||||
* content derived, no classKey (no retry seen), no retry event to anchor the delivery check
|
||||
* against, or the classKey was already delivered for this retry occurrence. Split out of
|
||||
* [unconfirmedFixEntries] to keep that function's branching flat.
|
||||
*/
|
||||
private fun deliverableUnconfirmedFix(
|
||||
content: String?,
|
||||
classKey: String?,
|
||||
latestRetry: StoredEvent?,
|
||||
sessionEvents: List<StoredEvent>,
|
||||
): Pair<String, String>? = content?.let { text ->
|
||||
classKey?.let { key ->
|
||||
latestRetry
|
||||
?.takeUnless { unconfirmedFixAlreadyDelivered(sessionEvents, it.sessionSequence, key) }
|
||||
?.let { text to key }
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* True when a prior [ContextAssembledEvent] manifest already recorded delivery of the
|
||||
* "unconfirmedFix" hint for [classKey] AFTER [afterSequence] (the triggering retry's own
|
||||
* position in the session log). Pure fold over recorded events — see #306.
|
||||
*/
|
||||
internal fun unconfirmedFixAlreadyDelivered(
|
||||
sessionEvents: List<StoredEvent>,
|
||||
afterSequence: Long,
|
||||
classKey: String,
|
||||
): Boolean = sessionEvents.any { stored ->
|
||||
stored.sessionSequence > afterSequence &&
|
||||
(stored.payload as? ContextAssembledEvent)?.entries.orEmpty()
|
||||
.any { it.sourceType == "unconfirmedFix" && it.sourceId == classKey }
|
||||
}
|
||||
|
||||
private const val SIGNATURE_MAX = 200
|
||||
private const val MAX_PROMOTED_CONCEPTS = 3
|
||||
|
||||
+86
@@ -0,0 +1,86 @@
|
||||
package com.correx.core.kernel.orchestration
|
||||
|
||||
import com.correx.core.events.events.ContextAssembledEvent
|
||||
import com.correx.core.events.events.ContextManifestEntry
|
||||
import com.correx.core.events.events.EventMetadata
|
||||
import com.correx.core.events.events.EventPayload
|
||||
import com.correx.core.events.events.StoredEvent
|
||||
import com.correx.core.events.types.EventId
|
||||
import com.correx.core.events.types.SessionId
|
||||
import com.correx.core.events.types.StageId
|
||||
import com.correx.core.kernel.concept.conceptClassKey
|
||||
import kotlinx.datetime.Clock
|
||||
import org.junit.jupiter.api.Assertions.assertFalse
|
||||
import org.junit.jupiter.api.Assertions.assertNotEquals
|
||||
import org.junit.jupiter.api.Assertions.assertTrue
|
||||
import org.junit.jupiter.api.Test
|
||||
import java.util.UUID
|
||||
|
||||
/**
|
||||
* #306: the steer-away hint must fire once per retry occurrence, not on every context rebuild for
|
||||
* as long as the latest retry stays contradicted. [unconfirmedFixAlreadyDelivered] is the pure fold
|
||||
* that makes that "already delivered?" question replay-safe (folded over recorded
|
||||
* [ContextAssembledEvent] manifests, no new mutable state).
|
||||
*/
|
||||
class UnconfirmedFixDeliveryTest {
|
||||
|
||||
private val sessionId = SessionId("s1")
|
||||
|
||||
private fun stored(sessionSequence: Long, payload: EventPayload) = StoredEvent(
|
||||
metadata = EventMetadata(
|
||||
eventId = EventId(UUID.randomUUID().toString()),
|
||||
sessionId = sessionId,
|
||||
timestamp = Clock.System.now(),
|
||||
schemaVersion = 1,
|
||||
causationId = null,
|
||||
correlationId = null,
|
||||
),
|
||||
sequence = sessionSequence,
|
||||
sessionSequence = sessionSequence,
|
||||
payload = payload,
|
||||
)
|
||||
|
||||
private fun assembled(sessionSequence: Long, sourceType: String, sourceId: String) = stored(
|
||||
sessionSequence,
|
||||
ContextAssembledEvent(
|
||||
sessionId = sessionId,
|
||||
stageId = StageId("scaffold_frontend"),
|
||||
contextPackId = "pack-$sessionSequence",
|
||||
entries = listOf(
|
||||
ContextManifestEntry(sourceType, sourceId, tokenEstimate = 10, layer = "L1", role = "USER"),
|
||||
),
|
||||
timestampMs = 0L,
|
||||
),
|
||||
)
|
||||
|
||||
@Test
|
||||
fun `not yet delivered for a fresh retry`() {
|
||||
val events = listOf(assembled(1, "unconfirmedFix", "other-class"))
|
||||
assertFalse(unconfirmedFixAlreadyDelivered(events, afterSequence = 5, classKey = "stage:x"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `delivered once is not delivered again on the next rebuild for the same retry`() {
|
||||
// retry lands at seq 5; the hint is delivered in the ContextAssembledEvent at seq 6
|
||||
// (first context build after the retry). A LATER rebuild for that same retry (still the
|
||||
// latest one, unchanged) must see it already delivered.
|
||||
val events = listOf(assembled(6, "unconfirmedFix", "stage:x"))
|
||||
assertTrue(unconfirmedFixAlreadyDelivered(events, afterSequence = 5, classKey = "stage:x"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a fresh recurrence of the same class after a new retry earns one more delivery`() {
|
||||
// Prior delivery at seq 6 for the FIRST retry (afterSequence=5). A NEW retry of the same
|
||||
// class lands later (seq 20) — the delivery check for the new retry only looks after seq 20,
|
||||
// so the stale seq-6 delivery no longer counts.
|
||||
val events = listOf(assembled(6, "unconfirmedFix", "stage:x"))
|
||||
assertFalse(unconfirmedFixAlreadyDelivered(events, afterSequence = 20, classKey = "stage:x"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a routing dead-end and a build failure never collapse into one classKey`() {
|
||||
val routing = conceptClassKey("stage", "no transition condition matched from stage scaffold_frontend")
|
||||
val build = conceptClassKey("build", "no transition condition matched from stage scaffold_frontend")
|
||||
assertNotEquals(routing, build)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user