From f783eed4be6893711c17fc6bf4f89f6a0b08ae27 Mon Sep 17 00:00:00 2001 From: kami Date: Sat, 20 Jun 2026 07:45:23 +0000 Subject: [PATCH] feat(critique): structured CritiqueFinding + per-model calibration projection - CritiqueFinding/CritiqueSeverity/CritiqueRole shared type (BACKLOG B6) - CritiqueOutcomeCorrelatedEvent + serialization registration (BACKLOG C-A3) - core/critique module: CriticCalibration state/reducer/projector, keyed by (modelHash, role) - schema+projection only; live reviewer-loop producer wiring deferred Co-Authored-By: Claude Opus 4.8 --- core/critique/build.gradle | 11 ++ .../critique/CriticCalibrationProjector.kt | 13 ++ .../core/critique/CriticCalibrationReducer.kt | 8 + .../DefaultCriticCalibrationReducer.kt | 22 +++ .../critique/model/CriticCalibrationState.kt | 33 ++++ .../DefaultCriticCalibrationReducerTest.kt | 156 ++++++++++++++++++ .../events/CritiqueCalibrationEvents.kt | 33 ++++ .../core/events/events/CritiqueFinding.kt | 34 ++++ .../events/serialization/Serialization.kt | 2 + ...itiqueCalibrationEventSerializationTest.kt | 33 ++++ settings.gradle | 1 + 11 files changed, 346 insertions(+) create mode 100644 core/critique/build.gradle create mode 100644 core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationProjector.kt create mode 100644 core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationReducer.kt create mode 100644 core/critique/src/main/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducer.kt create mode 100644 core/critique/src/main/kotlin/com/correx/core/critique/model/CriticCalibrationState.kt create mode 100644 core/critique/src/test/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducerTest.kt create mode 100644 core/events/src/main/kotlin/com/correx/core/events/events/CritiqueCalibrationEvents.kt create mode 100644 core/events/src/main/kotlin/com/correx/core/events/events/CritiqueFinding.kt create mode 100644 core/events/src/test/kotlin/com/correx/core/events/serialization/CritiqueCalibrationEventSerializationTest.kt diff --git a/core/critique/build.gradle b/core/critique/build.gradle new file mode 100644 index 00000000..4e8d68f9 --- /dev/null +++ b/core/critique/build.gradle @@ -0,0 +1,11 @@ +plugins { + id 'java-library' + id 'org.jetbrains.kotlin.jvm' + id 'org.jetbrains.kotlin.plugin.serialization' +} + +dependencies { + implementation(project(":core:events")) + testImplementation "org.jetbrains.kotlin:kotlin-test" +} +tasks.named("koverVerify").configure { enabled = false } diff --git a/core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationProjector.kt b/core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationProjector.kt new file mode 100644 index 00000000..d0919260 --- /dev/null +++ b/core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationProjector.kt @@ -0,0 +1,13 @@ +package com.correx.core.critique + +import com.correx.core.critique.model.CriticCalibrationState +import com.correx.core.events.events.StoredEvent +import com.correx.core.sessions.projections.Projection + +class CriticCalibrationProjector( + private val reducer: CriticCalibrationReducer, +) : Projection { + override fun initial(): CriticCalibrationState = CriticCalibrationState() + override fun apply(state: CriticCalibrationState, event: StoredEvent): CriticCalibrationState = + reducer.reduce(state, event) +} diff --git a/core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationReducer.kt b/core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationReducer.kt new file mode 100644 index 00000000..493d6632 --- /dev/null +++ b/core/critique/src/main/kotlin/com/correx/core/critique/CriticCalibrationReducer.kt @@ -0,0 +1,8 @@ +package com.correx.core.critique + +import com.correx.core.critique.model.CriticCalibrationState +import com.correx.core.events.events.StoredEvent + +interface CriticCalibrationReducer { + fun reduce(state: CriticCalibrationState, event: StoredEvent): CriticCalibrationState +} diff --git a/core/critique/src/main/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducer.kt b/core/critique/src/main/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducer.kt new file mode 100644 index 00000000..3630d8a2 --- /dev/null +++ b/core/critique/src/main/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducer.kt @@ -0,0 +1,22 @@ +package com.correx.core.critique + +import com.correx.core.critique.model.CalibrationKey +import com.correx.core.critique.model.CalibrationStats +import com.correx.core.critique.model.CriticCalibrationState +import com.correx.core.events.events.CritiqueOutcome +import com.correx.core.events.events.CritiqueOutcomeCorrelatedEvent +import com.correx.core.events.events.StoredEvent + +class DefaultCriticCalibrationReducer : CriticCalibrationReducer { + override fun reduce(state: CriticCalibrationState, event: StoredEvent): CriticCalibrationState { + val payload = event.payload as? CritiqueOutcomeCorrelatedEvent ?: return state + val key = CalibrationKey(payload.modelHash, payload.role) + val current = state.byKey[key] ?: CalibrationStats() + val updated = when (payload.outcome) { + CritiqueOutcome.UPHELD -> current.copy(upheld = current.upheld + 1) + CritiqueOutcome.DISMISSED -> current.copy(dismissed = current.dismissed + 1) + CritiqueOutcome.INCONCLUSIVE -> current.copy(inconclusive = current.inconclusive + 1) + } + return state.copy(byKey = state.byKey + (key to updated)) + } +} diff --git a/core/critique/src/main/kotlin/com/correx/core/critique/model/CriticCalibrationState.kt b/core/critique/src/main/kotlin/com/correx/core/critique/model/CriticCalibrationState.kt new file mode 100644 index 00000000..7b4ddf50 --- /dev/null +++ b/core/critique/src/main/kotlin/com/correx/core/critique/model/CriticCalibrationState.kt @@ -0,0 +1,33 @@ +package com.correx.core.critique.model + +import com.correx.core.events.events.CritiqueRole +import kotlinx.serialization.Serializable + +/** Composite key: critic precision is tracked per model and per role. */ +@Serializable +data class CalibrationKey(val modelHash: String, val role: CritiqueRole) + +/** + * Outcome tallies for one [CalibrationKey]. [precision] = upheld / (upheld + dismissed) — the + * share of *decisive* findings that held up — or null when no decisive findings exist yet. + * INCONCLUSIVE outcomes count toward [total] but never toward precision. + */ +@Serializable +data class CalibrationStats( + val upheld: Int = 0, + val dismissed: Int = 0, + val inconclusive: Int = 0, +) { + val total: Int get() = upheld + dismissed + inconclusive + val decisive: Int get() = upheld + dismissed + val precision: Double? get() = if (decisive == 0) null else upheld.toDouble() / decisive +} + +/** + * Replay-built view of how each (model, role) critic has performed. Rebuilt from + * [com.correx.core.events.events.CritiqueOutcomeCorrelatedEvent]s; never persisted directly. + */ +@Serializable +data class CriticCalibrationState( + val byKey: Map = emptyMap(), +) diff --git a/core/critique/src/test/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducerTest.kt b/core/critique/src/test/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducerTest.kt new file mode 100644 index 00000000..a5fdc2e1 --- /dev/null +++ b/core/critique/src/test/kotlin/com/correx/core/critique/DefaultCriticCalibrationReducerTest.kt @@ -0,0 +1,156 @@ +package com.correx.core.critique + +import com.correx.core.critique.model.CalibrationKey +import com.correx.core.critique.model.CriticCalibrationState +import com.correx.core.events.events.CritiqueOutcome +import com.correx.core.events.events.CritiqueOutcomeCorrelatedEvent +import com.correx.core.events.events.CritiqueRole +import com.correx.core.events.events.CritiqueSeverity +import com.correx.core.events.events.EventMetadata +import com.correx.core.events.events.EventPayload +import com.correx.core.events.events.StageCompletedEvent +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.events.types.TransitionId +import kotlinx.datetime.Instant +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertTrue + +class DefaultCriticCalibrationReducerTest { + + private val reducer = DefaultCriticCalibrationReducer() + private val projector = CriticCalibrationProjector(reducer) + private var seq = 0L + + private fun stored(payload: EventPayload): StoredEvent { + seq += 1 + return StoredEvent( + metadata = EventMetadata( + eventId = EventId("e-$seq"), + sessionId = SessionId("s1"), + timestamp = Instant.parse("2026-01-01T00:00:00Z"), + schemaVersion = 1, + causationId = null, + correlationId = null, + ), + sequence = seq, + sessionSequence = seq, + payload = payload, + ) + } + + private fun outcome( + outcome: CritiqueOutcome, + modelHash: String = "m1", + role: CritiqueRole = CritiqueRole.CODE_REVIEWER, + severity: CritiqueSeverity = CritiqueSeverity.MAJOR, + ) = stored( + CritiqueOutcomeCorrelatedEvent( + sessionId = SessionId("s1"), + findingId = "f-$seq", + role = role, + modelHash = modelHash, + severity = severity, + outcome = outcome, + ), + ) + + private fun fold(events: List): CriticCalibrationState = + events.fold(projector.initial(), projector::apply) + + @Test + fun `initial state is empty`() { + assertTrue(projector.initial().byKey.isEmpty()) + } + + @Test + fun `single UPHELD increments upheld and yields precision 1`() { + val state = fold(listOf(outcome(CritiqueOutcome.UPHELD))) + val stats = state.byKey.getValue(CalibrationKey("m1", CritiqueRole.CODE_REVIEWER)) + assertEquals(1, stats.upheld) + assertEquals(0, stats.dismissed) + assertEquals(1, stats.total) + assertEquals(1.0, stats.precision) + } + + @Test + fun `single DISMISSED increments dismissed and yields precision 0`() { + val state = fold(listOf(outcome(CritiqueOutcome.DISMISSED))) + val stats = state.byKey.getValue(CalibrationKey("m1", CritiqueRole.CODE_REVIEWER)) + assertEquals(1, stats.dismissed) + assertEquals(0.0, stats.precision) + } + + @Test + fun `INCONCLUSIVE counts toward total but not precision`() { + val state = fold(listOf(outcome(CritiqueOutcome.INCONCLUSIVE))) + val stats = state.byKey.getValue(CalibrationKey("m1", CritiqueRole.CODE_REVIEWER)) + assertEquals(1, stats.inconclusive) + assertEquals(1, stats.total) + assertEquals(0, stats.decisive) + assertNull(stats.precision, "precision is undefined with no decisive findings") + } + + @Test + fun `distinct model-role keys are tracked independently`() { + val state = fold( + listOf( + outcome(CritiqueOutcome.UPHELD, modelHash = "m1", role = CritiqueRole.CODE_REVIEWER), + outcome(CritiqueOutcome.DISMISSED, modelHash = "m2", role = CritiqueRole.CODE_REVIEWER), + outcome(CritiqueOutcome.UPHELD, modelHash = "m1", role = CritiqueRole.PLAN_CRITIC), + ), + ) + assertEquals(3, state.byKey.size) + assertEquals(1.0, state.byKey.getValue(CalibrationKey("m1", CritiqueRole.CODE_REVIEWER)).precision) + assertEquals(0.0, state.byKey.getValue(CalibrationKey("m2", CritiqueRole.CODE_REVIEWER)).precision) + assertEquals(1.0, state.byKey.getValue(CalibrationKey("m1", CritiqueRole.PLAN_CRITIC)).precision) + } + + @Test + fun `precision is computed over a mix of outcomes for one key`() { + val state = fold( + listOf( + outcome(CritiqueOutcome.UPHELD), + outcome(CritiqueOutcome.UPHELD), + outcome(CritiqueOutcome.UPHELD), + outcome(CritiqueOutcome.DISMISSED), + outcome(CritiqueOutcome.INCONCLUSIVE), + ), + ) + val stats = state.byKey.getValue(CalibrationKey("m1", CritiqueRole.CODE_REVIEWER)) + assertEquals(3, stats.upheld) + assertEquals(1, stats.dismissed) + assertEquals(1, stats.inconclusive) + assertEquals(5, stats.total) + assertEquals(0.75, stats.precision) // 3 / (3 + 1) + } + + @Test + fun `unrelated events pass through unchanged`() { + val state = fold( + listOf( + stored( + StageCompletedEvent( + sessionId = SessionId("s1"), + stageId = StageId("plan"), + transitionId = TransitionId("t1"), + ), + ), + outcome(CritiqueOutcome.UPHELD), + stored( + StageCompletedEvent( + sessionId = SessionId("s1"), + stageId = StageId("review"), + transitionId = TransitionId("t2"), + ), + ), + ), + ) + assertEquals(1, state.byKey.size) + assertEquals(1, state.byKey.getValue(CalibrationKey("m1", CritiqueRole.CODE_REVIEWER)).upheld) + } +} diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/CritiqueCalibrationEvents.kt b/core/events/src/main/kotlin/com/correx/core/events/events/CritiqueCalibrationEvents.kt new file mode 100644 index 00000000..908f202e --- /dev/null +++ b/core/events/src/main/kotlin/com/correx/core/events/events/CritiqueCalibrationEvents.kt @@ -0,0 +1,33 @@ +package com.correx.core.events.events + +import com.correx.core.events.types.SessionId +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** + * How a prior [CritiqueFinding] actually turned out once the surrounding loop resolved. + * UPHELD = the finding was confirmed/actioned; DISMISSED = rejected as a false positive; + * INCONCLUSIVE = no signal either way. + */ +@Serializable +enum class CritiqueOutcome { UPHELD, DISMISSED, INCONCLUSIVE } + +/** + * Correlates one prior [CritiqueFinding] (by [findingId]) with how it actually turned out, so a + * critic's precision can be tracked per model and per role over time (the calibration projection + * in `:core:critique`). [modelHash] identifies the model that produced the finding; [severity] + * is carried so calibration can be sliced by severity band without re-reading the finding. + * + * This is the recording half only — who decides UPHELD/DISMISSED lives in the reviewer-loop + * runtime and is wired separately. + */ +@Serializable +@SerialName("CritiqueOutcomeCorrelated") +data class CritiqueOutcomeCorrelatedEvent( + val sessionId: SessionId, + val findingId: String, + val role: CritiqueRole, + val modelHash: String, + val severity: CritiqueSeverity, + val outcome: CritiqueOutcome, +) : EventPayload diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/CritiqueFinding.kt b/core/events/src/main/kotlin/com/correx/core/events/events/CritiqueFinding.kt new file mode 100644 index 00000000..1a4e5839 --- /dev/null +++ b/core/events/src/main/kotlin/com/correx/core/events/events/CritiqueFinding.kt @@ -0,0 +1,34 @@ +package com.correx.core.events.events + +import kotlinx.serialization.Serializable + +/** Which critic role produced a [CritiqueFinding]. */ +@Serializable +enum class CritiqueRole { PLAN_CRITIC, CODE_REVIEWER } + +/** Severity of a [CritiqueFinding], highest-impact first. */ +@Serializable +enum class CritiqueSeverity { BLOCKER, MAJOR, MINOR, NIT } + +/** + * One structured finding emitted by a critic (plan critic) or reviewer (code reviewer), + * replacing the free-text verdict prose those roles produced before. Shared by both roles so + * findings can be counted, ranked, and calibrated uniformly. + * + * [id] is a stable identifier for the finding, used to correlate it with its eventual outcome + * (see [CritiqueOutcomeCorrelatedEvent]). [category] is a free-form classification + * (e.g. "correctness", "missing_requirement", "style"). [location] optionally points at a + * `file:line` or an artifact/stage reference the finding concerns. + * + * Distinct from `core.validation.ValidationIssue`, which models *structural* validation + * (graph/schema) rather than a critic's judgement. + */ +@Serializable +data class CritiqueFinding( + val id: String, + val role: CritiqueRole, + val severity: CritiqueSeverity, + val category: String, + val message: String, + val location: String? = null, +) diff --git a/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt b/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt index cbf6cf76..20d6ed08 100644 --- a/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt +++ b/core/events/src/main/kotlin/com/correx/core/events/serialization/Serialization.kt @@ -13,6 +13,7 @@ import com.correx.core.events.events.ChatSessionStartedEvent import com.correx.core.events.events.ClarificationAnsweredEvent import com.correx.core.events.events.ClarificationRequestedEvent import com.correx.core.events.events.ChatTurnEvent +import com.correx.core.events.events.CritiqueOutcomeCorrelatedEvent import com.correx.core.events.events.RouterNarrationEvent import com.correx.core.events.events.OperatorProfileBoundEvent import com.correx.core.events.events.ProjectProfileBoundEvent @@ -127,6 +128,7 @@ val eventModule = SerializersModule { subclass(WorkflowProposedEvent::class) subclass(IdeaCapturedEvent::class) subclass(IdeaDiscardedEvent::class) + subclass(CritiqueOutcomeCorrelatedEvent::class) } } diff --git a/core/events/src/test/kotlin/com/correx/core/events/serialization/CritiqueCalibrationEventSerializationTest.kt b/core/events/src/test/kotlin/com/correx/core/events/serialization/CritiqueCalibrationEventSerializationTest.kt new file mode 100644 index 00000000..7b1bcc98 --- /dev/null +++ b/core/events/src/test/kotlin/com/correx/core/events/serialization/CritiqueCalibrationEventSerializationTest.kt @@ -0,0 +1,33 @@ +package com.correx.core.events.serialization + +import com.correx.core.events.events.CritiqueOutcome +import com.correx.core.events.events.CritiqueOutcomeCorrelatedEvent +import com.correx.core.events.events.CritiqueRole +import com.correx.core.events.events.CritiqueSeverity +import com.correx.core.events.events.EventPayload +import com.correx.core.events.types.SessionId +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class CritiqueCalibrationEventSerializationTest { + + @Test + fun `CritiqueOutcomeCorrelatedEvent round-trips as polymorphic EventPayload`() { + val sample: EventPayload = CritiqueOutcomeCorrelatedEvent( + sessionId = SessionId("s"), + findingId = "f-1", + role = CritiqueRole.CODE_REVIEWER, + modelHash = "sha256:abc", + severity = CritiqueSeverity.MAJOR, + outcome = CritiqueOutcome.UPHELD, + ) + val encoded = eventJson.encodeToString(EventPayload.serializer(), sample) + assertTrue( + encoded.contains("\"type\":\"CritiqueOutcomeCorrelated\""), + "SerialName must be present: $encoded", + ) + val decoded = eventJson.decodeFromString(EventPayload.serializer(), encoded) + assertEquals(sample, decoded) + } +} diff --git a/settings.gradle b/settings.gradle index f10a760e..b98ee139 100644 --- a/settings.gradle +++ b/settings.gradle @@ -28,6 +28,7 @@ include ':core:sessions' include ':core:config' include ':core:risk' include ':core:journal' +include ':core:critique' include ':infrastructure:persistence' include ':infrastructure:inference'