feat(kernel): reviewer static-first infra (B§5)

- StaticFinding/StaticFindingSeverity + StaticFindingsRecordedEvent (registered + test) - StaticAnalysisRunner over an injectable CommandRunner; parses compiler/ktlint/detekt finding formats (lenient) into StaticFindings - excludeStaticFindingsFromReview pure filter, live-wired into SessionOrchestrator context assembly so static-tool findings are kept out of the reviewer's context - static_check pipeline stage left as a documented TODO (needs an event-emitting deterministic stage-type seam that doesn't exist yet) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-20 15:36:06 +00:00
parent eb9b1aee65
commit 447fc7a43e
8 changed files with 459 additions and 2 deletions
@@ -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<RepoKnowledgeHit>): 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<ContextEntry>,
+    staticFindings: List<StaticFinding>,
+): List<ContextEntry> {
+    if (staticFindings.isEmpty()) return entries
+    val findingSites: Set<String> = 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<String>): 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")
@@ -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,
@@ -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<String>): 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<String>): List<StaticFinding> {
+        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
+        }
+    }
+}
@@ -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)
+    }
+}
@@ -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<String>): 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)
+    }
+}