fix(kernel): deterministic repo-map floor + 0.0-hit filter + closest-path suggestion

RepoMapComputed recorded per session (in-memory scan cache keeps the walk-once guarantee); zero-score embedder hits filtered before reaching the agent, falling back to the deterministic repo map; REFERENCE_EXISTS surfaces a closest-path hint via closestPathByFilename. Breaks the hallucinated-package-path read loop even with a noop embedder.
2026-07-03 13:15:54 +04:00
parent 2698971082
commit a0b3a1865a
3 changed files with 164 additions and 34 deletions
@@ -4,6 +4,7 @@ import com.correx.core.config.ProjectConfig
 import com.correx.core.events.events.EventMetadata
 import com.correx.core.events.events.NewEvent
 import com.correx.core.events.events.RepoMapComputedEvent
+import com.correx.core.events.events.RepoMapEntry
 import com.correx.core.events.events.SteeringNoteAddedEvent
 import com.correx.core.events.events.WorkspaceStateObservedEvent
 import com.correx.core.events.stores.EventStore
@@ -44,6 +45,11 @@ class ProjectMemoryService(
    private val indexer: RepoMapIndexerPort = RepoMapIndexer(),
    private val workspaceStateProbe: WorkspaceStateProbe = RealWorkspaceStateProbe(),
 ) {
+    // repoRoot+stateKey -> scanned entries, so a second session on the same workspace state reuses
+    // the walk instead of re-scanning the FS (preserves the one-walk-per-state guarantee while still
+    // emitting a per-session RepoMapComputedEvent). Per-process; cold after a restart (one walk then).
+    private val entryCache = java.util.concurrent.ConcurrentHashMap<String, List<RepoMapEntry>>()
+
    private fun tag(repoRoot: String) = "project:$repoRoot"

    /** Resolved repo-root key: configured [ProjectConfig.root], else the working dir. */
@@ -59,12 +65,17 @@ class ProjectMemoryService(
        val stateKey = eventStore.read(sessionId)
            .mapNotNull { it.payload as? WorkspaceStateObservedEvent }
            .lastOrNull()?.stateKey
-        if (stateKey != null && l3MemoryStore.existsByTurnIdPrefix("repomap:$repoRoot:$stateKey")) {
-            log.debug("repo-map already indexed for {}@{} — skipping", repoRoot, stateKey)
-            return
-        }
        runCatching {
-            val entries = indexer.index(java.nio.file.Path.of(repoRoot))
+            // Record the map into THIS session's log unconditionally. buildRepoMapEntries and replay
+            // are session-scoped (invariant #9), so the old walk-once-and-skip left every session after
+            // the first with no deterministic repo grounding. To keep the walk-once guarantee we reuse
+            // the scanned entries from a per-stateKey in-memory cache (no re-walk), and only the
+            // expensive L3 embed/store below stays guarded by the persistent L3 presence check.
+            val cacheKey = stateKey?.let { "$repoRoot::$it" }
+            val entries = cacheKey?.let { entryCache[it] }
+                ?: indexer.index(java.nio.file.Path.of(repoRoot)).also {
+                    if (cacheKey != null && it.isNotEmpty()) entryCache[cacheKey] = it
+                }
            if (entries.isEmpty()) return
            eventStore.append(
                NewEvent(
@@ -85,33 +96,47 @@ class ProjectMemoryService(
                    ),
                ),
            )
-            // Trailing ':' delimiter so a repoRoot prefix can't collide with a longer sibling
-            // (/repo vs /repo2) under the retriever's startsWith filter.
-            val tag = if (stateKey != null) "repomap:$repoRoot:$stateKey" else "repomap:$repoRoot:"
-            entries.forEach { entry ->
-                runCatching {
-                    val text = entry.path + if (entry.symbols.isEmpty()) "" else ": ${entry.symbols.joinToString(", ")}"
-                    l3MemoryStore.store(
-                        L3MemoryEntry(
-                            id = UUID.randomUUID().toString(),
-                            sessionId = sessionId,
-                            turnId = tag,
-                            text = text,
-                            vector = embedder.embed(text),
-                            timestampMs = Clock.System.now().toEpochMilliseconds(),
-                        ),
-                    )
-                }.exceptionOrNull()?.let { e ->
-                    if (e is CancellationException) throw e
-                    log.warn("repo-map embedding failed for {}: {}", entry.path, e.message)
-                }
-            }
+            embedRepoMapIntoL3(sessionId, repoRoot, stateKey, entries)
        }.exceptionOrNull()?.let { e ->
            if (e is CancellationException) throw e
            log.warn("repo-map indexing failed for {}: {}", repoRoot, e.message)
        }
    }

+    /** Embed the scanned repo-map entries into L3 once per workspace state (skips if already stored). */
+    private suspend fun embedRepoMapIntoL3(
+        sessionId: SessionId,
+        repoRoot: String,
+        stateKey: String?,
+        entries: List<RepoMapEntry>,
+    ) {
+        if (stateKey != null && l3MemoryStore.existsByTurnIdPrefix("repomap:$repoRoot:$stateKey")) {
+            log.debug("repo-map already embedded for {}@{} — skipping L3 store", repoRoot, stateKey)
+            return
+        }
+        // Trailing ':' delimiter so a repoRoot prefix can't collide with a longer sibling
+        // (/repo vs /repo2) under the retriever's startsWith filter.
+        val tag = if (stateKey != null) "repomap:$repoRoot:$stateKey" else "repomap:$repoRoot:"
+        entries.forEach { entry ->
+            runCatching {
+                val text = entry.path + if (entry.symbols.isEmpty()) "" else ": ${entry.symbols.joinToString(", ")}"
+                l3MemoryStore.store(
+                    L3MemoryEntry(
+                        id = UUID.randomUUID().toString(),
+                        sessionId = sessionId,
+                        turnId = tag,
+                        text = text,
+                        vector = embedder.embed(text),
+                        timestampMs = Clock.System.now().toEpochMilliseconds(),
+                    ),
+                )
+            }.exceptionOrNull()?.let { e ->
+                if (e is CancellationException) throw e
+                log.warn("repo-map embedding failed for {}: {}", entry.path, e.message)
+            }
+        }
+    }
+
    /**
     * Observe the workspace content state and record it as a [WorkspaceStateObservedEvent]
     * (invariant #9: environment observations are recorded as events, never re-observed).
@@ -48,6 +48,7 @@ import com.correx.core.events.events.ArtifactValidatingEvent
 import com.correx.core.events.events.EventMetadata
 import com.correx.core.events.events.EventPayload
 import com.correx.core.events.events.InferenceCompletedEvent
+import com.correx.core.events.events.RepoKnowledgeHit
 import com.correx.core.events.events.RepoKnowledgeRetrievedEvent
 import com.correx.core.events.events.RepoMapComputedEvent
 import com.correx.core.events.events.InferenceFailedEvent
@@ -819,6 +820,18 @@ abstract class SessionOrchestrator(
                effectiveManifest,
            )?.let { assessment ->
                if (assessment.recommendedAction == RiskAction.BLOCK) {
+                    val rationale = assessment.rationale.joinToString("; ")
+                    // On a bad-path block, point the model at the closest real file so it fixes the
+                    // path instead of retrying the same wrong guess (deep package paths are easy to
+                    // misremember). Only fires when we can name a concrete match from the repo map.
+                    val suggestion = if (assessment.rationale.any { it.contains(REFERENCE_EXISTS_CODE) }) {
+                        (parameters["path"] as? String)
+                            ?.let { suggestClosestPath(sessionId, it) }
+                            ?.let { " Did you mean: $it ?" }
+                            .orEmpty()
+                    } else {
+                        ""
+                    }
                    emit(
                        sessionId,
                        ToolExecutionRejectedEvent(
@@ -828,8 +841,7 @@ abstract class SessionOrchestrator(
                            tier = tier,
                            // Record the specific rule rationale, not a generic string, so the audit
                            // trail (and task history) shows why a call was blocked.
-                            reason = assessment.rationale.joinToString("; ")
-                                .ifBlank { "blocked by tool-call policy" },
+                            reason = rationale.ifBlank { "blocked by tool-call policy" },
                        ),
                    )
                    val sourceId = toolCall.id ?: invocationId.value
@@ -848,8 +860,8 @@ abstract class SessionOrchestrator(
                            layer = ContextLayer.L2,
                            sourceType = "toolResult",
                            sourceId = sourceId,
-                            content = "BLOCKED: ${assessment.rationale.joinToString("; ")}",
-                            tokenEstimate = estimateTokens(assessment.rationale.joinToString("; ")),
+                            content = "BLOCKED: $rationale$suggestion",
+                            tokenEstimate = estimateTokens(rationale + suggestion),
                            role = EntryRole.TOOL,
                        ),
                    )
@@ -1366,6 +1378,19 @@ abstract class SessionOrchestrator(
        )
    }

+    /**
+     * Best real path in the repo map that shares [guessed]'s filename, ranked by shared trailing
+     * path segments — so a wrong package prefix (e.g. `com/correx/server/...` for the real
+     * `com/correx/apps/server/...`) still resolves. Null when nothing shares the filename, so we
+     * only ever suggest a concrete, existing file. Reads the recorded map (replay-safe).
+     */
+    private fun suggestClosestPath(sessionId: SessionId, guessed: String): String? {
+        val map = eventStore.read(sessionId)
+            .mapNotNull { it.payload as? RepoMapComputedEvent }
+            .lastOrNull() ?: return null
+        return closestPathByFilename(guessed, map.entries.map { it.path })
+    }
+
    private suspend fun buildContextualRepoEntries(
        sessionId: SessionId,
        stageId: StageId,
@@ -1374,7 +1399,7 @@ abstract class SessionOrchestrator(
        val recorded = eventStore.read(sessionId)
            .mapNotNull { it.payload as? RepoKnowledgeRetrievedEvent }
            .lastOrNull { it.stageId == stageId }
-        if (recorded != null) return listOf(buildRelevantFilesEntry(recorded.hits))
+        if (recorded != null) return repoEntriesOrMapFloor(sessionId, recorded.hits)

        val retriever = repoKnowledgeRetriever ?: return buildRepoMapEntries(sessionId)
        val query = (stageConfig.metadata["promptInline"] ?: stageConfig.metadata["prompt"] ?: stageId.value)
@@ -1385,9 +1410,23 @@ abstract class SessionOrchestrator(
                log.warn("repo-knowledge retrieval failed for stage {}: {}", stageId.value, e.message)
                emptyList()
            }
-        if (hits.isEmpty()) return buildRepoMapEntries(sessionId)
-        emit(sessionId, RepoKnowledgeRetrievedEvent(sessionId, stageId, query, hits))
-        return listOf(buildRelevantFilesEntry(hits))
+        // Record the raw retrieval for audit (a degenerate all-zero result is itself the signal that
+        // the embedder is noop / the index is empty), but only useful hits ever reach the agent.
+        if (hits.isNotEmpty()) emit(sessionId, RepoKnowledgeRetrievedEvent(sessionId, stageId, query, hits))
+        return repoEntriesOrMapFloor(sessionId, hits)
+    }
+
+    /**
+     * Zero-score hits carry no ranking signal (a noop embedder scores everything 0.0), so injecting
+     * them just feeds the agent arbitrary files and invites hallucinated paths. Drop them; when nothing
+     * useful survives, fall back to the deterministic repo map so the stage still has real grounding.
+     */
+    private suspend fun repoEntriesOrMapFloor(
+        sessionId: SessionId,
+        hits: List<RepoKnowledgeHit>,
+    ): List<ContextEntry> {
+        val useful = hits.filter { it.score > 0f }
+        return if (useful.isEmpty()) buildRepoMapEntries(sessionId) else listOf(buildRelevantFilesEntry(useful))
    }

    private suspend fun buildNeedsArtifactEntries(
@@ -2425,6 +2464,30 @@ private fun buildDiffString(path: String, existingContent: String?, proposedCont
    }.trimEnd()
 }

+/**
+ * The candidate in [paths] that shares [guessed]'s filename and the most trailing path segments,
+ * or null when nothing shares the filename. Lets a wrong package/dir prefix
+ * (`com/correx/server/routes/SessionRoutes.kt`) still resolve to the real file
+ * (`.../com/correx/apps/server/routes/SessionRoutes.kt`) without ever inventing a non-existent path.
+ */
+internal fun closestPathByFilename(guessed: String, paths: List<String>): String? {
+    val guessedName = guessed.substringAfterLast('/')
+    if (guessedName.isBlank()) return null
+    val guessedSegs = guessed.split('/').filter { it.isNotBlank() }
+    return paths.asSequence()
+        .filter { it.substringAfterLast('/') == guessedName }
+        .maxByOrNull { candidate ->
+            val segs = candidate.split('/').filter { it.isNotBlank() }
+            var shared = 0
+            var i = segs.size - 1
+            var j = guessedSegs.size - 1
+            while (i >= 0 && j >= 0 && segs[i] == guessedSegs[j]) {
+                shared++; i--; j--
+            }
+            shared
+        }
+}
+
 internal sealed interface InferenceResult {
    data class Success(val response: InferenceResponse, val responseArtifactId: ArtifactId? = null) : InferenceResult
    data class Failed(val reason: String) : InferenceResult
@@ -0,0 +1,42 @@
+package com.correx.core.kernel.orchestration
+
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertNull
+import org.junit.jupiter.api.Test
+
+class ClosestPathByFilenameTest {
+
+    private val repo = listOf(
+        "apps/server/src/main/kotlin/com/correx/apps/server/routes/SessionRoutes.kt",
+        "apps/server/src/main/kotlin/com/correx/apps/server/ws/SessionStreamHandler.kt",
+        "core/kernel/src/main/kotlin/com/correx/core/kernel/orchestration/SessionOrchestrator.kt",
+    )
+
+    @Test
+    fun `a wrong package prefix resolves to the real file by shared trailing segments`() {
+        // The model dropped the `apps` segment: com/correx/server/routes vs com/correx/apps/server/routes.
+        val guess = "apps/server/src/main/kotlin/com/correx/server/routes/SessionRoutes.kt"
+        assertEquals(
+            "apps/server/src/main/kotlin/com/correx/apps/server/routes/SessionRoutes.kt",
+            closestPathByFilename(guess, repo),
+        )
+    }
+
+    @Test
+    fun `a bare filename resolves to its only owner`() {
+        assertEquals(
+            "apps/server/src/main/kotlin/com/correx/apps/server/ws/SessionStreamHandler.kt",
+            closestPathByFilename("SessionStreamHandler.kt", repo),
+        )
+    }
+
+    @Test
+    fun `no filename match yields null rather than a wild guess`() {
+        assertNull(closestPathByFilename("com/correx/apps/server/routes/GhostRoutes.kt", repo))
+    }
+
+    @Test
+    fun `a directory path (no filename) yields null`() {
+        assertNull(closestPathByFilename("apps/server/src/main/kotlin/com/correx/server/routes/", repo))
+    }
+}