From a0b3a1865a855b1fdb6b5a920b205a5d1905cb3b Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 3 Jul 2026 13:15:54 +0400 Subject: [PATCH] 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. --- .../server/memory/ProjectMemoryService.kt | 77 ++++++++++++------ .../orchestration/SessionOrchestrator.kt | 79 +++++++++++++++++-- .../ClosestPathByFilenameTest.kt | 42 ++++++++++ 3 files changed, 164 insertions(+), 34 deletions(-) create mode 100644 core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ClosestPathByFilenameTest.kt diff --git a/apps/server/src/main/kotlin/com/correx/apps/server/memory/ProjectMemoryService.kt b/apps/server/src/main/kotlin/com/correx/apps/server/memory/ProjectMemoryService.kt index cb449a67..b55494d1 100644 --- a/apps/server/src/main/kotlin/com/correx/apps/server/memory/ProjectMemoryService.kt +++ b/apps/server/src/main/kotlin/com/correx/apps/server/memory/ProjectMemoryService.kt @@ -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>() + 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, + ) { + 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). 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 3e283982..09a96a0d 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 @@ -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, + ): List { + 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? { + 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 diff --git a/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ClosestPathByFilenameTest.kt b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ClosestPathByFilenameTest.kt new file mode 100644 index 00000000..53d8e123 --- /dev/null +++ b/core/kernel/src/test/kotlin/com/correx/core/kernel/orchestration/ClosestPathByFilenameTest.kt @@ -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)) + } +}