diff --git a/core/events/src/main/kotlin/com/correx/core/events/events/StaticAnalysisEvents.kt b/core/events/src/main/kotlin/com/correx/core/events/events/StaticAnalysisEvents.kt new file mode 100644 index 00000000..b5c1c5b8 --- /dev/null +++ b/core/events/src/main/kotlin/com/correx/core/events/events/StaticAnalysisEvents.kt @@ -0,0 +1,40 @@ +package com.correx.core.events.events + +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +@Serializable +enum class StaticFindingSeverity { ERROR, WARNING, INFO } + +/** + * One deterministic finding from a static-analysis tool (compiler, ktlint, detekt, …). + * [tool] is the tool name; [file]/[line]/[column] locate it; [ruleId] is the tool's rule + * identifier when it emits one (e.g. detekt's `[MagicNumber]`). + */ +@Serializable +data class StaticFinding( + val tool: String, + val file: String, + val line: Int, + val column: Int = 0, + val severity: StaticFindingSeverity, + val message: String, + val ruleId: String? = null, +) + +/** + * Deterministic static-analysis output recorded before review so the reviewer can be given a + * clean, static-issue-free context (BACKLOG §B-§5): the compile/lint/format findings a tool + * already caught are mechanically excluded from the reviewer's context, so the LLM reviewer + * spends its attention on semantic review rather than re-finding what the build already reports. + */ +@Serializable +@SerialName("StaticFindingsRecorded") +data class StaticFindingsRecordedEvent( + val sessionId: SessionId, + val stageId: StageId, + val tool: String, + val findings: List, +) : EventPayload 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 d9b17031..2c394076 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 @@ -49,6 +49,7 @@ import com.correx.core.events.events.RetryAttemptedEvent import com.correx.core.events.events.WorkspaceStateObservedEvent import com.correx.core.events.events.RiskAssessedEvent import com.correx.core.events.events.SourceFetchedEvent +import com.correx.core.events.events.StaticFindingsRecordedEvent import com.correx.core.events.events.StageCheckpointFailedEvent import com.correx.core.events.events.StageCheckpointPassedEvent import com.correx.core.events.events.StageCompletedEvent @@ -143,6 +144,7 @@ val eventModule = SerializersModule { subclass(SourceFetchedEvent::class) subclass(LowQualityExtractionEvent::class) subclass(EgressHostsGrantedEvent::class) + subclass(StaticFindingsRecordedEvent::class) } } diff --git a/core/events/src/test/kotlin/com/correx/core/events/serialization/StaticAnalysisEventSerializationTest.kt b/core/events/src/test/kotlin/com/correx/core/events/serialization/StaticAnalysisEventSerializationTest.kt new file mode 100644 index 00000000..b6140068 --- /dev/null +++ b/core/events/src/test/kotlin/com/correx/core/events/serialization/StaticAnalysisEventSerializationTest.kt @@ -0,0 +1,48 @@ +package com.correx.core.events.serialization + +import com.correx.core.events.events.EventPayload +import com.correx.core.events.events.StaticFinding +import com.correx.core.events.events.StaticFindingSeverity +import com.correx.core.events.events.StaticFindingsRecordedEvent +import com.correx.core.events.types.SessionId +import com.correx.core.events.types.StageId +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +class StaticAnalysisEventSerializationTest { + + @Test + fun `StaticFindingsRecordedEvent round-trips as polymorphic EventPayload`() { + val sample: EventPayload = StaticFindingsRecordedEvent( + sessionId = SessionId("s"), + stageId = StageId("static_check"), + tool = "detekt", + findings = listOf( + StaticFinding( + tool = "detekt", + file = "src/Main.kt", + line = 12, + column = 5, + severity = StaticFindingSeverity.WARNING, + message = "Magic number found", + ruleId = "MagicNumber", + ), + StaticFinding( + tool = "kotlinc", + file = "src/Other.kt", + line = 3, + severity = StaticFindingSeverity.ERROR, + message = "unresolved reference: foo", + ), + ), + ) + val encoded = eventJson.encodeToString(EventPayload.serializer(), sample) + assertTrue( + encoded.contains("\"type\":\"StaticFindingsRecorded\""), + "SerialName must be present: $encoded", + ) + val decoded = eventJson.decodeFromString(EventPayload.serializer(), encoded) + assertEquals(sample, decoded) + } +} diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt index 5112ecff..5fcab52e 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/ContextFeedback.kt @@ -10,6 +10,7 @@ import com.correx.core.context.model.ContextLayer import com.correx.core.context.model.EntryRole import com.correx.core.events.events.RefinementIterationEvent import com.correx.core.events.events.RetryAttemptedEvent +import com.correx.core.events.events.StaticFinding import com.correx.core.events.events.StoredEvent import com.correx.core.events.types.ArtifactId import com.correx.core.events.types.ContextEntryId @@ -85,6 +86,50 @@ fun buildRelevantFilesEntry(hits: List): ContextEntry { ) } +/** + * `sourceType`s whose entries merely restate deterministic static-analysis output. Entries tagged + * with any of these are static-derived by construction and dropped wholesale before review. + */ +private val STATIC_DERIVED_SOURCE_TYPES = setOf("staticAnalysis", "staticFindings", "lintFindings") + +/** + * Mechanically excludes context entries that merely restate static-analysis findings from the + * reviewer's context (BACKLOG §B-§5), so the LLM reviewer spends its attention on semantic review + * instead of re-finding compile/lint errors a deterministic tool already caught. + * + * The rule is concrete and deterministic, applied per entry: + * 1. Drop the entry outright if its [ContextEntry.sourceType] is static-analysis-derived + * (see [STATIC_DERIVED_SOURCE_TYPES]). + * 2. Otherwise drop it if its content references a `file:line` location that exactly matches a + * recorded [StaticFinding] (same `file` and `line`) — i.e. the entry is talking about an issue + * the static tool already reported at that exact site. + * + * Pure and identity-preserving: empty [staticFindings] returns [entries] unchanged. + */ +fun excludeStaticFindingsFromReview( + entries: List, + staticFindings: List, +): List { + if (staticFindings.isEmpty()) return entries + val findingSites: Set = staticFindings.map { siteKey(it.file, it.line) }.toSet() + return entries.filterNot { entry -> + entry.sourceType in STATIC_DERIVED_SOURCE_TYPES || referencesAnyFindingSite(entry.content, findingSites) + } +} + +/** Canonical `file:line` key. Trailing path segment match keeps it robust to path-prefix drift. */ +private fun siteKey(file: String, line: Int): String = "${file.substringAfterLast('/')}:$line" + +/** True if [content] mentions a `file:line` location present in [findingSites]. */ +private fun referencesAnyFindingSite(content: String, findingSites: Set): Boolean = + FILE_LINE_REFERENCE.findAll(content).any { match -> + val (file, line) = match.destructured + "${file.substringAfterLast('/')}:$line" in findingSites + } + +// A `path/to/File.ext:line` reference anywhere in free text (path may include directories). +private val FILE_LINE_REFERENCE = Regex("""([\w./\-]+\.\w+):(\d+)""") + fun buildProjectProfileEntry(profile: BoundProjectProfile): ContextEntry { val content = buildString { append("## Project profile\n") diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt index 3d3d946b..fdde54ec 100644 --- a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt @@ -28,6 +28,7 @@ import com.correx.core.events.events.RefinementIterationEvent import com.correx.core.events.events.ApprovalRequestedEvent import com.correx.core.events.events.ArtifactContentStoredEvent import com.correx.core.events.events.ArtifactCreatedEvent +import com.correx.core.events.events.StaticFindingsRecordedEvent import com.correx.core.events.events.BriefEchoMismatchEvent import com.correx.core.events.events.BriefGroundingCheckedEvent import com.correx.core.events.events.ContextTruncatedEvent @@ -402,10 +403,18 @@ abstract class SessionOrchestrator( val vocabularyEntries = artifactKindRegistry ?.takeIf { stageConfig.metadata["injectArtifactKinds"] == "true" } ?.let { listOf(buildArtifactKindVocabularyEntry(it.list())) } ?: emptyList() - var accumulatedEntries = + // Mechanically strip entries that merely restate static-analysis findings already recorded + // this session (BACKLOG §B-§5), so a downstream reviewer stage doesn't re-find compile/lint + // issues a deterministic tool caught. No-op until a static_check stage has emitted findings. + val recordedStaticFindings = sessionEvents + .mapNotNull { it.payload as? StaticFindingsRecordedEvent } + .flatMap { it.findings } + var accumulatedEntries = excludeStaticFindingsFromReview( systemPrompt + profileEntries + projectProfileEntries + journalEntries + repoMapEntries + needsEntries + schemaEntries + vocabularyEntries + promptEntries + steeringEntries + - clarificationEntries + retryFeedbackEntries + clarificationEntries + retryFeedbackEntries, + recordedStaticFindings, + ) val contextPack = contextPackBuilder.build( id = ContextPackId(UUID.randomUUID().toString()), sessionId = sessionId, diff --git a/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/StaticAnalysisRunner.kt b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/StaticAnalysisRunner.kt new file mode 100644 index 00000000..011279ae --- /dev/null +++ b/core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/StaticAnalysisRunner.kt @@ -0,0 +1,136 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.events.events.StaticFinding +import com.correx.core.events.events.StaticFindingSeverity + +/** Output of one external command run. */ +data class CommandOutput(val exitCode: Int, val stdout: String, val stderr: String) + +/** + * Minimal injectable seam for running an external static-analysis command. Kept as an interface + * (not a direct process spawn) so [StaticAnalysisRunner] stays pure and unit-testable: a fake + * runner returns canned output in tests; a real one shells out via the harness's sandbox. The + * established pattern in this package (see [RepoKnowledgeRetriever]) is a one-method `suspend` + * port. + */ +interface CommandRunner { + suspend fun run(argv: List): CommandOutput +} + +/** + * Runs a deterministic static-analysis tool and parses its console output into [StaticFinding]s + * (BACKLOG §B-§5). Pure of side effects beyond calling [runner]: no real process spawning lives + * here, so the parsing logic and the runner wiring are independently testable. + * + * Both stdout and stderr are parsed — compilers write diagnostics to stderr, ktlint/detekt to + * stdout — and parsing is lenient: lines that don't match any known format are ignored. + * + * TODO(wiring): a real `static_check` stage in role_pipeline.toml (between `implementer` and + * `reviewer`) would invoke this runner and emit the findings as a first-class event: + * + * val findings = StaticAnalysisRunner(commandRunner).analyze("detekt", argv) + * eventDispatcher.emit(StaticFindingsRecordedEvent(sessionId, stageId, "detekt", findings), sessionId) + * + * Once that event is on the journal, [excludeStaticFindingsFromReview] (already live-wired in + * SessionOrchestrator's context assembly) strips the matching entries from the reviewer's context. + * That stage is intentionally NOT added here: workflow stages are LLM-driven and emit typed + * artifacts, so a deterministic event-emitting stage needs a new stage-type / tool-execution seam + * that doesn't yet exist — adding it now would be broad plumbing. The runner + event + filter are + * complete; only the stage that calls them is deferred. + */ +class StaticAnalysisRunner(private val runner: CommandRunner) { + + suspend fun analyze(tool: String, argv: List): List { + val output = runner.run(argv) + val combined = sequenceOf(output.stdout, output.stderr) + .filter { it.isNotEmpty() } + .flatMap { it.lineSequence() } + return combined.mapNotNull { parseLine(tool, it) }.toList() + } + + companion object { + // Order matters: detekt's `[RuleId]` suffix is a superset of the generic compiler shape, + // so it is attempted first; the generic `path:line:col: severity: message` next; the + // bare `path:line: message` last. + private val DETEKT = Regex("""^\s*(\S.*?):(\d+):(\d+):\s*(.+?)\s*\[([A-Za-z0-9_.]+)]\s*$""") + private val COMPILER_COL = + Regex("""^\s*(\S.*?):(\d+):(\d+):\s*(error|warning|info)\s*:\s*(.+?)\s*$""", RegexOption.IGNORE_CASE) + private val COMPILER_SEV = + Regex("""^\s*(\S.*?):(\d+):\s*(error|warning|info)\s*:\s*(.+?)\s*$""", RegexOption.IGNORE_CASE) + private val PATH_LINE = Regex("""^\s*(\S.*?):(\d+):\s*(.+?)\s*$""") + + /** + * Parses a single console line into a [StaticFinding], or null if it matches no known + * format. Public + companion-scoped so each format is independently unit-testable. + */ + fun parseLine(tool: String, line: String): StaticFinding? { + detektFinding(tool, line)?.let { return it } + compilerWithColumn(tool, line)?.let { return it } + compilerWithSeverity(tool, line)?.let { return it } + return pathLine(tool, line) + } + + /** detekt console: `path:line:col: Message [RuleId]` — no explicit severity → WARNING. */ + private fun detektFinding(tool: String, line: String): StaticFinding? { + val m = DETEKT.matchEntire(line) ?: return null + val (file, ln, col, message, ruleId) = m.destructured + return StaticFinding( + tool = tool, + file = file, + line = ln.toInt(), + column = col.toInt(), + severity = StaticFindingSeverity.WARNING, + message = message, + ruleId = ruleId, + ) + } + + /** compiler / ktlint: `path:line:col: severity: message`. */ + private fun compilerWithColumn(tool: String, line: String): StaticFinding? { + val m = COMPILER_COL.matchEntire(line) ?: return null + val (file, ln, col, sev, message) = m.destructured + return StaticFinding( + tool = tool, + file = file, + line = ln.toInt(), + column = col.toInt(), + severity = severityOf(sev), + message = message, + ) + } + + /** compiler: `path:line: severity: message` (no column). */ + private fun compilerWithSeverity(tool: String, line: String): StaticFinding? { + val m = COMPILER_SEV.matchEntire(line) ?: return null + val (file, ln, sev, message) = m.destructured + return StaticFinding( + tool = tool, + file = file, + line = ln.toInt(), + severity = severityOf(sev), + message = message, + ) + } + + /** bare `path:line: message` — no severity token → WARNING. */ + private fun pathLine(tool: String, line: String): StaticFinding? { + val m = PATH_LINE.matchEntire(line) ?: return null + val (file, ln, message) = m.destructured + return StaticFinding( + tool = tool, + file = file, + line = ln.toInt(), + severity = StaticFindingSeverity.WARNING, + message = message, + ) + } + + /** Maps a severity token to the enum; anything unrecognised defaults to WARNING. */ + fun severityOf(token: String): StaticFindingSeverity = when (token.trim().lowercase()) { + "error" -> StaticFindingSeverity.ERROR + "warning" -> StaticFindingSeverity.WARNING + "info" -> StaticFindingSeverity.INFO + else -> StaticFindingSeverity.WARNING + } + } +} diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ExcludeStaticFindingsTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ExcludeStaticFindingsTest.kt new file mode 100644 index 00000000..8ad5d257 --- /dev/null +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ExcludeStaticFindingsTest.kt @@ -0,0 +1,74 @@ +package com.correx.core.kernel.orchestration + +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.StaticFinding +import com.correx.core.events.events.StaticFindingSeverity +import com.correx.core.events.types.ContextEntryId +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertSame +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test + +class ExcludeStaticFindingsTest { + + private fun entry(sourceType: String, content: String, id: String = sourceType): ContextEntry = + ContextEntry( + id = ContextEntryId(id), + layer = ContextLayer.L1, + content = content, + sourceType = sourceType, + sourceId = id, + tokenEstimate = content.length / 4, + role = EntryRole.SYSTEM, + ) + + private val finding = StaticFinding( + tool = "detekt", + file = "src/main/Foo.kt", + line = 12, + column = 5, + severity = StaticFindingSeverity.WARNING, + message = "Magic number found", + ruleId = "MagicNumber", + ) + + @Test + fun `drops a static-analysis-derived entry by sourceType`() { + val entries = listOf( + entry("staticFindings", "Foo.kt:12 Magic number found"), + entry("agentPrompt", "Review the patch against the requirements."), + ) + val result = excludeStaticFindingsFromReview(entries, listOf(finding)) + assertEquals(listOf("agentPrompt"), result.map { it.sourceType }) + } + + @Test + fun `drops an entry referencing a recorded finding site`() { + val entries = listOf( + entry("toolResult", "The build reported src/main/Foo.kt:12: magic number — please address."), + entry("agentPrompt", "Does the change satisfy each requirement?"), + ) + val result = excludeStaticFindingsFromReview(entries, listOf(finding)) + assertEquals(listOf("agentPrompt"), result.map { it.sourceType }) + } + + @Test + fun `keeps a semantic entry that references an unrelated site`() { + val semantic = entry("toolResult", "Consider extracting Bar.kt:99 into a helper for clarity.") + val result = excludeStaticFindingsFromReview(listOf(semantic), listOf(finding)) + assertEquals(1, result.size) + assertTrue(result.contains(semantic)) + } + + @Test + fun `empty findings is identity`() { + val entries = listOf( + entry("staticFindings", "Foo.kt:12 Magic number found"), + entry("agentPrompt", "Review the patch."), + ) + val result = excludeStaticFindingsFromReview(entries, emptyList()) + assertSame(entries, result) + } +} diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/StaticAnalysisRunnerTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/StaticAnalysisRunnerTest.kt new file mode 100644 index 00000000..88b6e99b --- /dev/null +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/StaticAnalysisRunnerTest.kt @@ -0,0 +1,103 @@ +package com.correx.core.kernel.orchestration + +import com.correx.core.events.events.StaticFinding +import com.correx.core.events.events.StaticFindingSeverity +import kotlinx.coroutines.runBlocking +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test + +class StaticAnalysisRunnerTest { + + @Test + fun `parses compiler file line col error message`() { + val finding = StaticAnalysisRunner.parseLine("kotlinc", "src/Main.kt:12:5: error: unresolved reference: foo") + assertEquals( + StaticFinding( + tool = "kotlinc", + file = "src/Main.kt", + line = 12, + column = 5, + severity = StaticFindingSeverity.ERROR, + message = "unresolved reference: foo", + ), + finding, + ) + } + + @Test + fun `parses ktlint style warning with column`() { + val finding = StaticAnalysisRunner.parseLine("ktlint", "src/Util.kt:3:1: warning: Unexpected blank line(s)") + assertEquals("src/Util.kt", finding?.file) + assertEquals(3, finding?.line) + assertEquals(1, finding?.column) + assertEquals(StaticFindingSeverity.WARNING, finding?.severity) + assertEquals("Unexpected blank line(s)", finding?.message) + assertNull(finding?.ruleId) + } + + @Test + fun `parses detekt console line with rule id`() { + val finding = StaticAnalysisRunner.parseLine("detekt", "src/Foo.kt:42:9: Magic number found [MagicNumber]") + assertEquals( + StaticFinding( + tool = "detekt", + file = "src/Foo.kt", + line = 42, + column = 9, + severity = StaticFindingSeverity.WARNING, + message = "Magic number found", + ruleId = "MagicNumber", + ), + finding, + ) + } + + @Test + fun `parses bare file line message without column or severity`() { + val finding = StaticAnalysisRunner.parseLine("tool", "src/Bar.kt:7: something is off") + assertEquals("src/Bar.kt", finding?.file) + assertEquals(7, finding?.line) + assertEquals(0, finding?.column) + assertEquals(StaticFindingSeverity.WARNING, finding?.severity) + assertEquals("something is off", finding?.message) + } + + @Test + fun `unparseable line is ignored`() { + assertNull(StaticAnalysisRunner.parseLine("tool", "BUILD SUCCESSFUL in 3s")) + assertNull(StaticAnalysisRunner.parseLine("tool", "")) + assertNull(StaticAnalysisRunner.parseLine("tool", "just some prose with no location")) + } + + @Test + fun `severity mapping covers error warning info and default`() { + assertEquals(StaticFindingSeverity.ERROR, StaticAnalysisRunner.severityOf("error")) + assertEquals(StaticFindingSeverity.WARNING, StaticAnalysisRunner.severityOf("WARNING")) + assertEquals(StaticFindingSeverity.INFO, StaticAnalysisRunner.severityOf("Info")) + assertEquals(StaticFindingSeverity.WARNING, StaticAnalysisRunner.severityOf("note")) + } + + @Test + fun `analyze parses canned multi-line output from both streams`() = runBlocking { + val runner = StaticAnalysisRunner( + object : CommandRunner { + override suspend fun run(argv: List): CommandOutput = CommandOutput( + exitCode = 1, + stdout = listOf( + "src/A.kt:10:4: warning: deprecated api use", + "src/B.kt:2:1: Magic number found [MagicNumber]", + "BUILD FAILED", + ).joinToString("\n"), + stderr = "src/C.kt:5:9: error: type mismatch", + ) + }, + ) + val findings = runner.analyze("static", listOf("./gradlew", "check")) + assertEquals(3, findings.size) + assertEquals(StaticFindingSeverity.WARNING, findings[0].severity) + assertEquals("MagicNumber", findings[1].ruleId) + assertEquals(StaticFindingSeverity.ERROR, findings[2].severity) + assertEquals("src/C.kt", findings[2].file) + } +}