diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 436eab2..be50317 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -21,7 +21,7 @@ 'name': 'JVM' 'uses': 'JorisJonkers-dev/github-workflows/.github/workflows/jvm-ci.yml@v0.8.1' 'with': - 'lint-gradle-args': ':api:detekt :api:ktlintCheck' + 'lint-gradle-args': ':api:detektMain :api:detektTest :api:detektIntegrationTest :api:ktlintCheck' 'test-gradle-args': ':api:test :api:integrationTest :api:jacocoTestCoverageVerification :api:exportOpenApiSpec' 'secrets': 'packages-token': '${{ secrets.GITHUB_TOKEN }}' diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/HealthIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/HealthIntegrationTest.kt index 2025239..4b2803b 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/HealthIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/HealthIntegrationTest.kt @@ -18,33 +18,37 @@ import org.springframework.web.context.WebApplicationContext * HealthIntegrationTest but without security (knowledge-api 4a doesn't * wire in spring-security yet). */ -class HealthIntegrationTest : IntegrationTestBase() { +class HealthIntegrationTest @Autowired - private lateinit var context: WebApplicationContext - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = MockMvcBuilders.webAppContextSetup(context).build() - } - - @Test - fun `composite health returns 200 with status UP and a db contributor`() { - val result = mockMvc.get("/api/actuator/health").andReturn() - - assertThat(result.response.status) - .describedAs("composite /api/actuator/health body=%s", result.response.contentAsString) - .isEqualTo(200) - - val body = objectMapper.readTree(result.response.contentAsString) - assertThat(body["status"].asText()).isEqualTo("UP") - - val components = body["components"] ?: body["details"] ?: error("no components: $body") - assertThat(components.fieldNames().asSequence().toList()) - .describedAs("expected core infra contributors to be present: $body") - .contains("db", "ping") + constructor( + private val context: WebApplicationContext, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + + private val objectMapper = jacksonObjectMapper() + + @BeforeEach + fun setUp() { + mockMvc = MockMvcBuilders.webAppContextSetup(context).build() + } + + @Test + fun compositeHealthReturnsOkWithStatusUPAndADbContributor() { + val result = mockMvc.get("/api/actuator/health").andReturn() + + assertThat(result.response.status) + .describedAs("composite /api/actuator/health body=%s", result.response.contentAsString) + .isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + + val body = objectMapper.readTree(result.response.contentAsString) + assertThat(body["status"].asText()).isEqualTo("UP") + + val components = body["components"] ?: body["details"] ?: error("no components: $body") + assertThat(components.fieldNames().asSequence().toList()) + .describedAs("expected core infra contributors to be present: $body") + .contains("db", "ping") + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/IntegrationTestBase.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/IntegrationTestBase.kt index 0c69ab7..40f98e5 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/IntegrationTestBase.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/IntegrationTestBase.kt @@ -14,12 +14,11 @@ import org.testcontainers.rabbitmq.RabbitMQContainer @Tag("integration") @SpringBootTest @Testcontainers -abstract class IntegrationTestBase { - @Autowired - private lateinit var dsl: DSLContext - +open class IntegrationTestBase { @AfterEach - fun resetSharedState() { + fun resetSharedState( + @Autowired dsl: DSLContext, + ) { dsl.execute( "TRUNCATE TABLE kb_notes, kb_note_tags, kb_relations, kb_audit " + "RESTART IDENTITY CASCADE", diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/audit/AuditFlowIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/audit/AuditFlowIntegrationTest.kt index 9667b99..05e48a6 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/audit/AuditFlowIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/audit/AuditFlowIntegrationTest.kt @@ -3,6 +3,7 @@ package com.jorisjonkers.personalstack.knowledge.audit import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.jorisjonkers.personalstack.knowledge.IntegrationTestBase import com.jorisjonkers.personalstack.knowledge.auth.McpBearerFilter +import com.jorisjonkers.personalstack.knowledge.repo.AuditRecordRequest import com.jorisjonkers.personalstack.knowledge.repo.AuditRepository import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach @@ -30,157 +31,163 @@ import java.time.Instant "knowledge.mcp.tokens.ws=test-token-ws", ], ) -class AuditFlowIntegrationTest : IntegrationTestBase() { +class AuditFlowIntegrationTest @Autowired - private lateinit var context: WebApplicationContext - - @Autowired - private lateinit var mcpBearerFilter: McpBearerFilter - - @Autowired - private lateinit var auditRepository: AuditRepository - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .webAppContextSetup(context) - .addFilters(mcpBearerFilter) - .build() - } - - private fun seed( - actor: String, - action: String, - targetId: String? = null, - targetKind: String? = null, - beforeJson: String? = null, - afterJson: String? = null, - at: Instant = Instant.now(), - ) = auditRepository.record( - actor = actor, - action = action, - targetId = targetId, - targetKind = targetKind, - beforeJson = beforeJson, - afterJson = afterJson, - now = at, - ) - - @Test - fun `list_audit returns most-recent-first by default`() { - val older = seed(actor = "kb-curator-resolver", action = "drop_relation") - Thread.sleep(LIST_SLEEP_MS) - val newer = seed(actor = "kb-curator-resolver", action = "resolve_relation") - - val rows = toolResult(call("knowledge.list_audit", mapOf("limit" to 10)))["rows"] - val ids = rows.map { it["id"].asText() } - - // ULIDs sort lex by time — `ORDER BY id DESC` is recency-first. - assertThat(ids).containsExactly(newer.id, older.id) - } - - @Test - fun `list_audit filters by actor`() { - val first = seed(actor = "mcp:workstation", action = "add_topic") - seed(actor = "kb-renormalise-titles", action = "rename_title") - - val rows = toolResult(call("knowledge.list_audit", mapOf("actor" to "mcp:workstation")))["rows"] - val ids = rows.map { it["id"].asText() } - - assertThat(ids).containsExactly(first.id) + constructor( + private val context: WebApplicationContext, + private val mcpBearerFilter: McpBearerFilter, + private val auditRepository: AuditRepository, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + + private val objectMapper = jacksonObjectMapper() + + @BeforeEach + fun setUp() { + mockMvc = + MockMvcBuilders + .webAppContextSetup(context) + .addFilters(mcpBearerFilter) + .build() + } + + private fun seed(seed: AuditSeed) = auditRepository.record(seed.toRequest()) + + @Test + fun listAuditReturnsMostRecentFirstByDefault() { + val older = seed(AuditSeed(actor = "kb-curator-resolver", action = "drop_relation")) + Thread.sleep(LIST_SLEEP_MS) + val newer = seed(AuditSeed(actor = "kb-curator-resolver", action = "resolve_relation")) + + val rows = toolResult(call("knowledge.list_audit", mapOf("limit" to 10)))["rows"] + val ids = rows.map { it["id"].asText() } + + // ULIDs sort lex by time — `ORDER BY id DESC` is recency-first. + assertThat(ids).containsExactly(newer.id, older.id) + } + + @Test + fun listAuditFiltersByActor() { + val first = seed(AuditSeed(actor = "mcp:workstation", action = "add_topic")) + seed(AuditSeed(actor = "kb-renormalise-titles", action = "rename_title")) + + val rows = toolResult(call("knowledge.list_audit", mapOf("actor" to "mcp:workstation")))["rows"] + val ids = rows.map { it["id"].asText() } + + assertThat(ids).containsExactly(first.id) + } + + @Test + fun listAuditFiltersByAction() { + val first = seed(AuditSeed(actor = "kb-curator-resolver", action = "resolve_relation")) + seed(AuditSeed(actor = "kb-curator-resolver", action = "drop_relation")) + + val rows = toolResult(call("knowledge.list_audit", mapOf("action" to "resolve_relation")))["rows"] + val ids = rows.map { it["id"].asText() } + + assertThat(ids).containsExactly(first.id) + } + + @Test + fun listAuditFiltersByTargetId() { + seed(AuditSeed(actor = "a", action = "rename_title", targetId = "01HXNOTE000000000000000001")) + val match = + seed(AuditSeed(actor = "a", action = "rename_title", targetId = "01HXNOTE000000000000000002")) + + val rows = + toolResult(call("knowledge.list_audit", mapOf("target_id" to "01HXNOTE000000000000000002")))["rows"] + val ids = rows.map { it["id"].asText() } + + assertThat(ids).containsExactly(match.id) + } + + @Test + fun listAuditClampsToASinceWindowWhenAnIso8601TimestampIsPassed() { + val cutoff = Instant.parse("2026-05-19T12:00:00Z") + seed(AuditSeed(actor = "a", action = "drop_relation", at = Instant.parse("2026-05-19T11:00:00Z"))) + val newer = + seed(AuditSeed(actor = "a", action = "drop_relation", at = Instant.parse("2026-05-19T13:00:00Z"))) + + val rows = toolResult(call("knowledge.list_audit", mapOf("since" to cutoff.toString())))["rows"] + val ids = rows.map { it["id"].asText() } + + assertThat(ids).containsExactly(newer.id) + } + + @Test + fun listAuditSurfacesBeforeJsonAndAfterJsonVerbatim() { + seed( + AuditSeed( + actor = "kb-renormalise-titles", + action = "rename_title", + targetId = "01HXNOTE000000000000000099", + beforeJson = """{"title":"a very long title that nobody can scan"}""", + afterJson = """{"title":"scannable claim"}""", + ), + ) + + val rows = toolResult(call("knowledge.list_audit", mapOf("limit" to 1)))["rows"] + assertThat(rows[0]["before_json"].asText()) + .isEqualTo("""{"title":"a very long title that nobody can scan"}""") + assertThat(rows[0]["after_json"].asText()).isEqualTo("""{"title":"scannable claim"}""") + } + + private fun call( + name: String, + arguments: Map, + ): String = rpcRaw("tools/call", mapOf("name" to name, "arguments" to arguments)) + + private fun toolResult(rawJson: String) = objectMapper.readTree(rawJson)["result"]["structuredContent"] + + private fun rpcRaw( + method: String, + params: Map?, + ): String { + val body = + buildMap { + put("jsonrpc", "2.0") + put("id", 1) + put("method", method) + if (params != null) put("params", params) + } + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = objectMapper.writeValueAsBytes(body) + }.andReturn() + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + return result.response.contentAsString + } + + companion object { + // ULIDs sort lex by capture time; a small sleep separates + // two consecutive seeds so the `most-recent-first` assertion + // is deterministic. + private const val LIST_SLEEP_MS = 3L + } } - @Test - fun `list_audit filters by action`() { - val first = seed(actor = "kb-curator-resolver", action = "resolve_relation") - seed(actor = "kb-curator-resolver", action = "drop_relation") - - val rows = toolResult(call("knowledge.list_audit", mapOf("action" to "resolve_relation")))["rows"] - val ids = rows.map { it["id"].asText() } - - assertThat(ids).containsExactly(first.id) - } - - @Test - fun `list_audit filters by target_id`() { - seed(actor = "a", action = "rename_title", targetId = "01HXNOTE000000000000000001", targetKind = "note") - val match = - seed(actor = "a", action = "rename_title", targetId = "01HXNOTE000000000000000002", targetKind = "note") - - val rows = - toolResult(call("knowledge.list_audit", mapOf("target_id" to "01HXNOTE000000000000000002")))["rows"] - val ids = rows.map { it["id"].asText() } - - assertThat(ids).containsExactly(match.id) - } - - @Test - fun `list_audit clamps to a since window when an ISO-8601 timestamp is passed`() { - val cutoff = Instant.parse("2026-05-19T12:00:00Z") - seed(actor = "a", action = "drop_relation", at = Instant.parse("2026-05-19T11:00:00Z")) - val newer = seed(actor = "a", action = "drop_relation", at = Instant.parse("2026-05-19T13:00:00Z")) - - val rows = toolResult(call("knowledge.list_audit", mapOf("since" to cutoff.toString())))["rows"] - val ids = rows.map { it["id"].asText() } - - assertThat(ids).containsExactly(newer.id) - } - - @Test - fun `list_audit surfaces before_json and after_json verbatim`() { - seed( - actor = "kb-renormalise-titles", - action = "rename_title", - targetId = "01HXNOTE000000000000000099", - targetKind = "note", - beforeJson = """{"title":"a very long title that nobody can scan"}""", - afterJson = """{"title":"scannable claim"}""", +private data class AuditSeed( + val actor: String, + val action: String, + val targetId: String? = null, + val beforeJson: String? = null, + val afterJson: String? = null, + val at: Instant = Instant.now(), +) { + fun toRequest(): AuditRecordRequest = + AuditRecordRequest( + actor = actor, + action = action, + targetId = targetId, + targetKind = targetId?.let { "note" }, + beforeJson = beforeJson, + afterJson = afterJson, + now = at, ) - - val rows = toolResult(call("knowledge.list_audit", mapOf("limit" to 1)))["rows"] - assertThat(rows[0]["before_json"].asText()) - .isEqualTo("""{"title":"a very long title that nobody can scan"}""") - assertThat(rows[0]["after_json"].asText()).isEqualTo("""{"title":"scannable claim"}""") - } - - private fun call( - name: String, - arguments: Map, - ): String = rpcRaw("tools/call", mapOf("name" to name, "arguments" to arguments)) - - private fun toolResult(rawJson: String) = objectMapper.readTree(rawJson)["result"]["structuredContent"] - - private fun rpcRaw( - method: String, - params: Map?, - ): String { - val body = - buildMap { - put("jsonrpc", "2.0") - put("id", 1) - put("method", method) - if (params != null) put("params", params) - } - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = objectMapper.writeValueAsBytes(body) - }.andReturn() - assertThat(result.response.status).isEqualTo(200) - return result.response.contentAsString - } - - companion object { - // ULIDs sort lex by capture time; a small sleep separates - // two consecutive seeds so the `most-recent-first` assertion - // is deterministic. - private const val LIST_SLEEP_MS = 3L - } } diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/capture/CaptureFlowIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/capture/CaptureFlowIntegrationTest.kt index 073dc94..fde8db7 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/capture/CaptureFlowIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/capture/CaptureFlowIntegrationTest.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") // Jackson 3 asText() — see McpTools file header. package com.jorisjonkers.personalstack.knowledge.capture @@ -27,201 +26,212 @@ import java.time.Duration "knowledge.mcp.tokens.ws=test-token-ws", ], ) -class CaptureFlowIntegrationTest : IntegrationTestBase() { +class CaptureFlowIntegrationTest @Autowired - private lateinit var context: WebApplicationContext - - @Autowired - private lateinit var mcpBearerFilter: McpBearerFilter - - @Autowired - private lateinit var noteRepository: NoteRepository - - @Autowired - private lateinit var rabbitTemplate: RabbitTemplate + constructor( + private val context: WebApplicationContext, + private val mcpBearerFilter: McpBearerFilter, + private val noteRepository: NoteRepository, + private val rabbitTemplate: RabbitTemplate, + private val messageConverter: MessageConverter, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + + private val objectMapper = jacksonObjectMapper() + + @BeforeEach + fun setUp() { + mockMvc = + MockMvcBuilders + .webAppContextSetup(context) + .addFilters(mcpBearerFilter) + .build() + // Drain the bound queue so earlier tests don't bleed messages + // into the assertions below. `receive(queue, timeout)` is the + // public Spring-AMQP API for a non-blocking poll; the + // setter-style `receiveTimeout` is package-private in 4.x. + while (rabbitTemplate.receive(IngestQueueConfig.QUEUE, DRAIN_TIMEOUT_MS) != null) Unit + } - @Autowired - private lateinit var messageConverter: MessageConverter - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .webAppContextSetup(context) - .addFilters(mcpBearerFilter) - .build() - // Drain the bound queue so earlier tests don't bleed messages - // into the assertions below. `receive(queue, timeout)` is the - // public Spring-AMQP API for a non-blocking poll; the - // setter-style `receiveTimeout` is package-private in 4.x. - while (rabbitTemplate.receive(IngestQueueConfig.QUEUE, DRAIN_TIMEOUT_MS) != null) Unit - } + @Test + fun toolsListAdvertisesTheThreeCaptureTools() { + val body = postRpc(mapOf("jsonrpc" to "2.0", "id" to "test-request", "method" to "tools/list")) + val tools = objectMapper.readTree(body)["result"]["tools"] + assertThat(tools.isArray).isTrue + val names = tools.map { it["name"].asText() } + assertThat(names) + .contains( + "knowledge.capture_lesson", + "knowledge.capture_decision", + "knowledge.ingest_note", + ) + } - @Test - fun `tools list advertises the three capture tools`() { - val body = postRpc(mapOf("jsonrpc" to "2.0", "id" to 1, "method" to "tools/list")) - val tools = objectMapper.readTree(body)["result"]["tools"] - assertThat(tools.isArray).isTrue - val names = tools.map { it["name"].asText() } - assertThat(names) - .contains( - "knowledge.capture_lesson", - "knowledge.capture_decision", - "knowledge.ingest_note", - ) - } + @Test + fun captureLessonPersistsAKbNotesRowAndPublishesToKnowledgeLesson() { + val before = noteRepository.rowCount() - @Test - fun `capture_lesson persists a kb_notes row and publishes to knowledge_lesson`() { - val before = noteRepository.rowCount() + val response = + postRpc( + mapOf( + "jsonrpc" to "2.0", + "id" to "test-request", + "method" to "tools/call", + "params" to + mapOf( + "name" to "knowledge.capture_lesson", + "arguments" to + mapOf( + "scope" to "personal", + "title" to "lesson title", + "body" to "lesson body", + "tags" to listOf("kotlin", "mcp"), + ), + ), + ), + ) + + // Unwrap the MCP `CallToolResult` envelope (spec 2025-06-18): + // every `tools/call` result is `{content, structuredContent, + // isError}`. The capture payload (id / type / scope / title / + // captured_at / vault_path) sits under `structuredContent`. + val result = objectMapper.readTree(response)["result"]["structuredContent"] + val id = result["id"].asText() + assertThat(id).matches("[0-9A-HJKMNP-TV-Z]{26}") + assertThat(result["type"].asText()).isEqualTo("lesson") + assertThat(result["scope"].asText()).isEqualTo("personal") + + assertThat(noteRepository.rowCount()).isEqualTo(before + 1) + val row = checkNotNull(noteRepository.findById(id)) { "expected persisted note $id" } + assertThat(row.title).isEqualTo("lesson title") + assertThat(row.type.wire).isEqualTo("lesson") + assertThat(row.tags).containsExactlyInAnyOrder("kotlin", "mcp") + + val (routingKey, payload) = await1QueueMessage() + assertThat(routingKey).isEqualTo(IngestQueueConfig.ROUTING_LESSON) + assertThat(payload["id"]).isEqualTo(id) + assertThat(payload["type"]).isEqualTo("lesson") + assertThat(payload["tags"].stringList()).containsExactlyInAnyOrder("kotlin", "mcp") + } - val response = + @Test + fun captureDecisionRoutesToKnowledgeDecisionAndInsertsTypeDecision() { postRpc( mapOf( "jsonrpc" to "2.0", - "id" to 7, + "id" to "test-request", "method" to "tools/call", "params" to mapOf( - "name" to "knowledge.capture_lesson", + "name" to "knowledge.capture_decision", "arguments" to mapOf( - "scope" to "personal", - "title" to "lesson title", - "body" to "lesson body", - "tags" to listOf("kotlin", "mcp"), + "scope" to "project:personal-stack", + "title" to "decision title", + "body" to "decision body", ), ), ), ) + val (routingKey, payload) = await1QueueMessage() + assertThat(routingKey).isEqualTo(IngestQueueConfig.ROUTING_DECISION) + assertThat(payload["type"]).isEqualTo("decision") + val row = checkNotNull(noteRepository.findById(payload["id"] as String)) { "expected decision note" } + assertThat(row.type.wire).isEqualTo("decision") + assertThat(row.scope).isEqualTo("project:personal-stack") + } - // Unwrap the MCP `CallToolResult` envelope (spec 2025-06-18): - // every `tools/call` result is `{content, structuredContent, - // isError}`. The capture payload (id / type / scope / title / - // captured_at / vault_path) sits under `structuredContent`. - val result = objectMapper.readTree(response)["result"]["structuredContent"] - val id = result["id"].asText() - assertThat(id).matches("[0-9A-HJKMNP-TV-Z]{26}") - assertThat(result["type"].asText()).isEqualTo("lesson") - assertThat(result["scope"].asText()).isEqualTo("personal") - - assertThat(noteRepository.rowCount()).isEqualTo(before + 1) - val row = noteRepository.findById(id)!! - assertThat(row.title).isEqualTo("lesson title") - assertThat(row.type.wire).isEqualTo("lesson") - assertThat(row.tags).containsExactlyInAnyOrder("kotlin", "mcp") - - val (routingKey, payload) = await1QueueMessage() - assertThat(routingKey).isEqualTo(IngestQueueConfig.ROUTING_LESSON) - assertThat(payload["id"]).isEqualTo(id) - assertThat(payload["type"]).isEqualTo("lesson") - @Suppress("UNCHECKED_CAST") - assertThat(payload["tags"] as List).containsExactlyInAnyOrder("kotlin", "mcp") - } - - @Test - fun `capture_decision routes to knowledge_decision and inserts type=decision`() { - postRpc( - mapOf( - "jsonrpc" to "2.0", - "id" to 8, - "method" to "tools/call", - "params" to - mapOf( - "name" to "knowledge.capture_decision", - "arguments" to - mapOf( - "scope" to "project:personal-stack", - "title" to "decision title", - "body" to "decision body", - ), - ), - ), - ) - val (routingKey, payload) = await1QueueMessage() - assertThat(routingKey).isEqualTo(IngestQueueConfig.ROUTING_DECISION) - assertThat(payload["type"]).isEqualTo("decision") - val row = noteRepository.findById(payload["id"] as String)!! - assertThat(row.type.wire).isEqualTo("decision") - assertThat(row.scope).isEqualTo("project:personal-stack") - } - - @Test - fun `ingest_note honours the requested type and routes to knowledge_ingest`() { - postRpc( - mapOf( - "jsonrpc" to "2.0", - "id" to 9, - "method" to "tools/call", - "params" to - mapOf( - "name" to "knowledge.ingest_note", - "arguments" to - mapOf( - "scope" to "agent:claude", - "title" to "URL clip", - "body" to "https://example.com", - "type" to "fact", - ), - ), - ), - ) - val (routingKey, payload) = await1QueueMessage() - assertThat(routingKey).isEqualTo(IngestQueueConfig.ROUTING_INGEST) - assertThat(payload["type"]).isEqualTo("fact") - } - - @Test - fun `tools_call with an unknown name returns method_not_found`() { - val response = + @Test + fun ingestNoteHonoursTheRequestedTypeAndRoutesToKnowledgeIngest() { postRpc( mapOf( "jsonrpc" to "2.0", - "id" to 10, + "id" to "test-request", "method" to "tools/call", "params" to mapOf( - "name" to "knowledge.no_such_tool", - "arguments" to mapOf(), + "name" to "knowledge.ingest_note", + "arguments" to + mapOf( + "scope" to "agent:claude", + "title" to "URL clip", + "body" to "https://example.com", + "type" to "fact", + ), ), ), ) - val error = objectMapper.readTree(response)["error"] - assertThat(error["code"].asInt()).isEqualTo(-32601) - } + val (routingKey, payload) = await1QueueMessage() + assertThat(routingKey).isEqualTo(IngestQueueConfig.ROUTING_INGEST) + assertThat(payload["type"]).isEqualTo("fact") + } - private fun postRpc(envelope: Map): String { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = objectMapper.writeValueAsBytes(envelope) - }.andReturn() - assertThat(result.response.status).isEqualTo(200) - return result.response.contentAsString - } + @Test + fun toolsCallWithAnUnknownNameReturnsMethodNotFound() { + val response = + postRpc( + mapOf( + "jsonrpc" to "2.0", + "id" to "test-request", + "method" to "tools/call", + "params" to + mapOf( + "name" to "knowledge.no_such_tool", + "arguments" to mapOf(), + ), + ), + ) + val error = objectMapper.readTree(response)["error"] + assertThat(error["code"].asInt()).isEqualTo(METHOD_NOT_FOUND_CODE) + } - private fun await1QueueMessage(): Pair> { - val deadline = System.nanoTime() + Duration.ofSeconds(5).toNanos() - var msg: Message? = null - while (msg == null && System.nanoTime() < deadline) { - msg = rabbitTemplate.receive(IngestQueueConfig.QUEUE) - if (msg == null) Thread.sleep(POLL_MS) + private fun postRpc(envelope: Map): String { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = objectMapper.writeValueAsBytes(envelope) + }.andReturn() + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + return result.response.contentAsString } - check(msg != null) { "no message arrived on ${IngestQueueConfig.QUEUE} within 5s" } - val deserialized = messageConverter.fromMessage(msg) - val routingKey = msg.messageProperties.receivedRoutingKey ?: error("no routing key on received message") - @Suppress("UNCHECKED_CAST") - return routingKey to (deserialized as Map) - } - companion object { - private const val POLL_MS = 50L - private const val DRAIN_TIMEOUT_MS = 50L + private fun await1QueueMessage(): Pair> { + val deadline = System.nanoTime() + Duration.ofSeconds(5).toNanos() + var msg: Message? = null + while (msg == null && System.nanoTime() < deadline) { + msg = rabbitTemplate.receive(IngestQueueConfig.QUEUE) + if (msg == null) Thread.sleep(POLL_MS) + } + val received = checkNotNull(msg) { "no message arrived on ${IngestQueueConfig.QUEUE} within 5s" } + val deserialized = messageConverter.fromMessage(received) + val routingKey = + received.messageProperties.receivedRoutingKey ?: error("no routing key on received message") + return routingKey to deserialized.stringKeyMap() + } + + private fun Any?.stringKeyMap(): Map { + val raw = this as? Map<*, *> ?: error("expected message payload to be a map") + return raw.entries.associate { (key, value) -> + check(key is String) { "expected string payload key but got $key" } + key to value + } + } + + private fun Any?.stringList(): List { + val raw = this as? List<*> ?: error("expected payload value to be a list") + return raw.map { value -> + value as? String ?: error("expected string list item but got $value") + } + } + + companion object { + private const val POLL_MS = 50L + private const val DRAIN_TIMEOUT_MS = 50L + private const val METHOD_NOT_FOUND_CODE = -32601 + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/contract/OpenApiSpecExportTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/contract/OpenApiSpecExportTest.kt index b938125..3f4a0fe 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/contract/OpenApiSpecExportTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/contract/OpenApiSpecExportTest.kt @@ -60,44 +60,45 @@ import java.nio.file.Paths KnowledgeReviewController::class, ], ) -class OpenApiSpecExportTest { +class OpenApiSpecExportTest @Autowired - private lateinit var mockMvc: MockMvc - - @Test - fun `export OpenAPI spec to repo root`() { - OpenApiSliceExporter.writeJson(mockMvc, resolveOpenApiSpecPath(), "/api/v1/api-docs") - } + constructor( + private val mockMvc: MockMvc, + ) { + @Test + fun exportOpenApiSpecToRepoRoot() { + OpenApiSliceExporter.writeJson(mockMvc, resolveOpenApiSpecPath(), "/api/v1/api-docs") + } - private fun resolveOpenApiSpecPath(): Path { - // The Gradle task sets `openapi.spec.output` to the canonical - // committed location. Fallback to `/knowledge-api.json` when run - // directly from the IDE so a one-off invocation still works. - val override = System.getProperty("openapi.spec.output") - if (override != null) { - return Paths.get(override) + private fun resolveOpenApiSpecPath(): Path { + // The Gradle task sets `openapi.spec.output` to the canonical + // committed location. Fallback to `/knowledge-api.json` when run + // directly from the IDE so a one-off invocation still works. + val override = System.getProperty("openapi.spec.output") + if (override != null) { + return Paths.get(override) + } + return Paths.get(System.getProperty("user.dir")).resolve("knowledge-api.json") } - return Paths.get(System.getProperty("user.dir")).resolve("knowledge-api.json") - } - @SpringBootConfiguration - class Application + @SpringBootConfiguration + class Application - @TestConfiguration(proxyBeanMethods = false) - class Collaborators { - @Bean - fun auditService(): AuditService = mockk(relaxed = true) + @TestConfiguration(proxyBeanMethods = false) + class Collaborators { + @Bean + fun auditService(): AuditService = mockk(relaxed = true) - @Bean - fun discoveryService(): DiscoveryService = mockk(relaxed = true) + @Bean + fun discoveryService(): DiscoveryService = mockk(relaxed = true) - @Bean - fun recallService(): RecallService = mockk(relaxed = true) + @Bean + fun recallService(): RecallService = mockk(relaxed = true) - @Bean - fun reviewService(): ReviewService = mockk(relaxed = true) + @Bean + fun reviewService(): ReviewService = mockk(relaxed = true) - @Bean - fun tagClusterService(): TagClusterService = mockk(relaxed = true) + @Bean + fun tagClusterService(): TagClusterService = mockk(relaxed = true) + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryFlowIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryFlowIntegrationTest.kt index d13a285..28db689 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryFlowIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryFlowIntegrationTest.kt @@ -33,191 +33,190 @@ import java.time.Instant "knowledge.mcp.tokens.ws=test-token-ws", ], ) -class DiscoveryFlowIntegrationTest : IntegrationTestBase() { +class DiscoveryFlowIntegrationTest @Autowired - private lateinit var context: WebApplicationContext - - @Autowired - private lateinit var mcpBearerFilter: McpBearerFilter - - @Autowired - private lateinit var noteRepository: NoteRepository - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .webAppContextSetup(context) - .addFilters(mcpBearerFilter) - .build() - } - - private fun seed( - title: String, - scope: String, - type: KbNoteType = KbNoteType.LESSON, - source: String = "test", - tags: Set = emptySet(), - ): KbNote { - val note = - KbNote( - id = Ulid.generate(), - type = type, - scope = scope, - source = source, - capturedAt = Instant.now(), - sessionId = null, - confidence = 0.4, - title = title, - body = "body for $title", - vaultPath = "$scope/${type.wire}/${title.replace(' ', '-')}.md", - vaultCommit = null, - tags = tags, + constructor( + private val context: WebApplicationContext, + private val mcpBearerFilter: McpBearerFilter, + private val noteRepository: NoteRepository, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + + private val objectMapper = jacksonObjectMapper() + + @BeforeEach + fun setUp() { + mockMvc = + MockMvcBuilders + .webAppContextSetup(context) + .addFilters(mcpBearerFilter) + .build() + } + + private fun seed( + title: String, + scope: String, + type: KbNoteType = KbNoteType.LESSON, + source: String = "test", + tags: Set = emptySet(), + ): KbNote { + val note = + KbNote( + id = Ulid.generate(), + type = type, + scope = scope, + source = source, + capturedAt = Instant.now(), + sessionId = null, + confidence = 0.4, + title = title, + body = "body for $title", + vaultPath = "$scope/${type.wire}/${title.replace(' ', '-')}.md", + vaultCommit = null, + tags = tags, + ) + return noteRepository.create(note) + } + + @Test + fun listTopicsReturnsSlugsWithTheTopicPrefixStripped() { + seed("kotlin one", scope = "topic:kotlin") + seed("kotlin two", scope = "topic:kotlin") + seed("postgres one", scope = "topic:postgres") + seed("project note", scope = "project:foo") // must not surface as a topic + + val out = discoveryToolResult(objectMapper, call("knowledge.list_topics", mapOf("limit" to 10))) + val topics = out["topics"].map { it["slug"].asText() to it["note_count"].asInt() } + assertThat(topics).containsExactlyInAnyOrder( + "kotlin" to 2, + "postgres" to 1, ) - return noteRepository.create(note) - } - - @Test - fun `list_topics returns slugs with the topic prefix stripped`() { - seed("kotlin one", scope = "topic:kotlin") - seed("kotlin two", scope = "topic:kotlin") - seed("postgres one", scope = "topic:postgres") - seed("project note", scope = "project:foo") // must not surface as a topic - - val out = toolResult(call("knowledge.list_topics", mapOf("limit" to 10))) - - @Suppress("UNCHECKED_CAST") - val topics = out["topics"].map { it["slug"].asText() to it["note_count"].asInt() } - assertThat(topics).containsExactlyInAnyOrder( - "kotlin" to 2, - "postgres" to 1, - ) - } - - @Test - fun `list_tags counts each tag and honours the scope filter`() { - seed("a", scope = "project:p1", tags = setOf("kotlin", "spring")) - seed("b", scope = "project:p1", tags = setOf("kotlin")) - seed("c", scope = "project:p2", tags = setOf("kotlin", "vue")) - - val unscoped = toolResult(call("knowledge.list_tags", mapOf("limit" to 10))) - - @Suppress("UNCHECKED_CAST") - val all = unscoped["tags"].associate { it["tag"].asText() to it["count"].asInt() } - assertThat(all["kotlin"]).isEqualTo(3) - assertThat(all["spring"]).isEqualTo(1) - assertThat(all["vue"]).isEqualTo(1) - - val scoped = - toolResult(call("knowledge.list_tags", mapOf("scope" to "project:p1", "limit" to 10))) - - @Suppress("UNCHECKED_CAST") - val p1 = scoped["tags"].associate { it["tag"].asText() to it["count"].asInt() } - assertThat(p1["kotlin"]).isEqualTo(2) - assertThat(p1).doesNotContainKey("vue") - } - - @Test - fun `list_scopes returns every distinct scope ordered by note_count desc`() { - seed("a", scope = "project:foo") - seed("b", scope = "project:foo") - seed("c", scope = "personal") - - val out = toolResult(call("knowledge.list_scopes", mapOf("limit" to 10))) - - @Suppress("UNCHECKED_CAST") - val scopes = out["scopes"].map { it["scope"].asText() to it["note_count"].asInt() } - // project:foo (2) sorts before personal (1). - assertThat(scopes).containsExactly( - "project:foo" to 2, - "personal" to 1, - ) - } - - @Test - fun `list_sources rolls up provenance markers`() { - seed("a", scope = "personal", source = "claude-code") - seed("b", scope = "personal", source = "claude-code") - seed("c", scope = "personal", source = "codex") - - val out = toolResult(call("knowledge.list_sources", mapOf("limit" to 10))) - - @Suppress("UNCHECKED_CAST") - val sources = out["sources"].associate { it["source"].asText() to it["count"].asInt() } - assertThat(sources["claude-code"]).isEqualTo(2) - assertThat(sources["codex"]).isEqualTo(1) - } - - @Test - fun `topic_stats returns aggregates including type breakdown and top tags`() { - seed("a", scope = "topic:kotlin", type = KbNoteType.LESSON, tags = setOf("spring", "jOOQ")) - seed("b", scope = "topic:kotlin", type = KbNoteType.LESSON, tags = setOf("spring")) - seed("c", scope = "topic:kotlin", type = KbNoteType.DECISION, tags = setOf("jOOQ")) - seed("d", scope = "topic:postgres", type = KbNoteType.LESSON, tags = setOf("pgvector")) - - val out = toolResult(call("knowledge.topic_stats", mapOf("slug" to "kotlin"))) - - val stats = out["stats"] - assertThat(stats["slug"].asText()).isEqualTo("kotlin") - assertThat(stats["note_count"].asInt()).isEqualTo(3) - val breakdown = stats["type_breakdown"] - assertThat(breakdown["lesson"].asInt()).isEqualTo(2) - assertThat(breakdown["decision"].asInt()).isEqualTo(1) - - val topTags = stats["top_tags"].map { it["tag"].asText() to it["count"].asInt() } - // spring + jOOQ both have count 2 — assert containment, not order, - // because the SQL tiebreak on the join goes by tag name. - assertThat(topTags).contains("spring" to 2, "jOOQ" to 2) - } - - @Test - fun `topic_stats returns null stats when the slug has no notes`() { - val out = toolResult(call("knowledge.topic_stats", mapOf("slug" to "nonexistent"))) - assertThat(out["stats"].isNull).isTrue - } - - @Test - fun `list_inbox returns notes that still hold the _inbox sentinel scope`() { - val pending = seed("pending classification", scope = "_inbox") - seed("already classified", scope = "topic:kotlin") - - val out = toolResult(call("knowledge.list_inbox", mapOf("limit" to 10))) + } + + @Test + fun listTagsCountsEachTagAndHonoursTheScopeFilter() { + seed("a", scope = "project:p1", tags = setOf("kotlin", "spring")) + seed("b", scope = "project:p1", tags = setOf("kotlin")) + seed("c", scope = "project:p2", tags = setOf("kotlin", "vue")) + + val unscoped = discoveryToolResult(objectMapper, call("knowledge.list_tags", mapOf("limit" to 10))) + val all = unscoped["tags"].associate { it["tag"].asText() to it["count"].asInt() } + assertThat(all["kotlin"]).isEqualTo(KOTLIN_TAG_COUNT) + assertThat(all["spring"]).isEqualTo(1) + assertThat(all["vue"]).isEqualTo(1) + + val scoped = + discoveryToolResult( + objectMapper, + call("knowledge.list_tags", mapOf("scope" to "project:p1", "limit" to 10)), + ) + val p1 = scoped["tags"].associate { it["tag"].asText() to it["count"].asInt() } + assertThat(p1["kotlin"]).isEqualTo(2) + assertThat(p1).doesNotContainKey("vue") + } + + @Test + fun listScopesReturnsEveryDistinctScopeOrderedByNoteCountDesc() { + seed("a", scope = "project:foo") + seed("b", scope = "project:foo") + seed("c", scope = "personal") + + val out = discoveryToolResult(objectMapper, call("knowledge.list_scopes", mapOf("limit" to 10))) + val scopes = out["scopes"].map { it["scope"].asText() to it["note_count"].asInt() } + // project:foo (2) sorts before personal (1). + assertThat(scopes).containsExactly( + "project:foo" to 2, + "personal" to 1, + ) + } + + @Test + fun listSourcesRollsUpProvenanceMarkers() { + seed("a", scope = "personal", source = "claude-code") + seed("b", scope = "personal", source = "claude-code") + seed("c", scope = "personal", source = "codex") + + val out = discoveryToolResult(objectMapper, call("knowledge.list_sources", mapOf("limit" to 10))) + val sources = out["sources"].associate { it["source"].asText() to it["count"].asInt() } + assertThat(sources["claude-code"]).isEqualTo(2) + assertThat(sources["codex"]).isEqualTo(1) + } + + @Test + fun topicStatsReturnsAggregatesIncludingTypeBreakdownAndTopTags() { + seed("a", scope = "topic:kotlin", type = KbNoteType.LESSON, tags = setOf("spring", "jOOQ")) + seed("b", scope = "topic:kotlin", type = KbNoteType.LESSON, tags = setOf("spring")) + seed("c", scope = "topic:kotlin", type = KbNoteType.DECISION, tags = setOf("jOOQ")) + seed("d", scope = "topic:postgres", type = KbNoteType.LESSON, tags = setOf("pgvector")) + + val out = discoveryToolResult(objectMapper, call("knowledge.topic_stats", mapOf("slug" to "kotlin"))) + + val stats = out["stats"] + assertThat(stats["slug"].asText()).isEqualTo("kotlin") + assertThat(stats["note_count"].asInt()).isEqualTo(KOTLIN_TOPIC_NOTE_COUNT) + val breakdown = stats["type_breakdown"] + assertThat(breakdown["lesson"].asInt()).isEqualTo(2) + assertThat(breakdown["decision"].asInt()).isEqualTo(1) + + val topTags = stats["top_tags"].map { it["tag"].asText() to it["count"].asInt() } + // spring + jOOQ both have count 2 — assert containment, not order, + // because the SQL tiebreak on the join goes by tag name. + assertThat(topTags).contains("spring" to 2, "jOOQ" to 2) + } + + @Test + fun topicStatsReturnsNullStatsWhenTheSlugHasNoNotes() { + val out = discoveryToolResult(objectMapper, call("knowledge.topic_stats", mapOf("slug" to "nonexistent"))) + assertThat(out["stats"].isNull).isTrue + } + + @Test + fun listInboxReturnsNotesThatStillHoldTheInboxSentinelScope() { + val pending = seed("pending classification", scope = "_inbox") + seed("already classified", scope = "topic:kotlin") + + val out = discoveryToolResult(objectMapper, call("knowledge.list_inbox", mapOf("limit" to 10))) + val ids = out["notes"].map { it["id"].asText() } + assertThat(ids).containsExactly(pending.id) + } + + private fun call( + name: String, + arguments: Map, + ): String = rpcRaw("tools/call", mapOf("name" to name, "arguments" to arguments)) + + private fun rpcRaw( + method: String, + params: Map?, + ): String { + val body = + buildMap { + put("jsonrpc", "2.0") + put("id", 1) + put("method", method) + if (params != null) put("params", params) + } + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = objectMapper.writeValueAsBytes(body) + }.andReturn() + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + return result.response.contentAsString + } - @Suppress("UNCHECKED_CAST") - val ids = out["notes"].map { it["id"].asText() } - assertThat(ids).containsExactly(pending.id) + private companion object { + private const val KOTLIN_TAG_COUNT = 3 + private const val KOTLIN_TOPIC_NOTE_COUNT = 3 + } } - private fun call( - name: String, - arguments: Map, - ): String = rpcRaw("tools/call", mapOf("name" to name, "arguments" to arguments)) - - private fun toolResult(rawJson: String) = objectMapper.readTree(rawJson)["result"]["structuredContent"] - - private fun rpcRaw( - method: String, - params: Map?, - ): String { - val body = - buildMap { - put("jsonrpc", "2.0") - put("id", 1) - put("method", method) - if (params != null) put("params", params) - } - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = objectMapper.writeValueAsBytes(body) - }.andReturn() - assertThat(result.response.status).isEqualTo(200) - return result.response.contentAsString - } -} +private fun discoveryToolResult( + objectMapper: com.fasterxml.jackson.databind.ObjectMapper, + rawJson: String, +) = objectMapper.readTree(rawJson)["result"]["structuredContent"] diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/SuggestionsRepositoryIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/SuggestionsRepositoryIntegrationTest.kt index 9fd7246..6460083 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/SuggestionsRepositoryIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/SuggestionsRepositoryIntegrationTest.kt @@ -19,125 +19,136 @@ import java.time.Instant * Hand-crafted embeddings live in known scopes so the centroid + * threshold logic surfaces unambiguously. */ -class SuggestionsRepositoryIntegrationTest : IntegrationTestBase() { +class SuggestionsRepositoryIntegrationTest @Autowired - private lateinit var noteRepository: NoteRepository - - @Autowired - private lateinit var embeddingRepository: EmbeddingRepository - - @Autowired - private lateinit var dsl: DSLContext - - @Test - fun `suggestTopic ranks closer topics first by their centroid`() { - // topic:kotlin = three notes clustered around the [1, 0, …] axis; - // topic:postgres = three notes around the [0, 1, …] axis. - seedEmbedded("k1", padded(1.0f, 0.0f), scope = "topic:kotlin") - seedEmbedded("k2", padded(0.9f, 0.1f), scope = "topic:kotlin") - seedEmbedded("k3", padded(0.95f, 0.05f), scope = "topic:kotlin") - seedEmbedded("p1", padded(0.0f, 1.0f), scope = "topic:postgres") - seedEmbedded("p2", padded(0.1f, 0.9f), scope = "topic:postgres") - - val query = padded(1.0f, 0.0f) - val suggestions = embeddingRepository.suggestTopic(query, limit = 5) - - assertThat(suggestions.map { it.slug }).containsExactly("kotlin", "postgres") - // Closer topic scores higher than the further one. - assertThat(suggestions.first().score).isGreaterThan(suggestions.last().score) - } - - @Test - fun `findDuplicates returns rows above the cosine threshold and excludes the source`() { - val origin = seedEmbedded("source", padded(1.0f, 0.0f), scope = "personal") - seedEmbedded("near", padded(0.95f, 0.31f), scope = "personal") - seedEmbedded("far", padded(0.0f, 1.0f), scope = "personal") - - // threshold=0.85 cosine → distance ≤ 0.15. The `near` row should - // survive; `far` (orthogonal) and `origin` (excluded by id) drop. - val dupes = - embeddingRepository.findDuplicates( - queryEmbedding = padded(1.0f, 0.0f), - threshold = 0.85, - limit = 10, - excludeId = origin.id, + constructor( + private val noteRepository: NoteRepository, + private val embeddingRepository: EmbeddingRepository, + private val dsl: DSLContext, + ) : IntegrationTestBase() { + @Test + fun suggesttopicRanksCloserTopicsFirstByTheirCentroid() { + // topic:kotlin = three notes clustered around the [1, 0, …] axis; + // topic:postgres = three notes around the [0, 1, …] axis. + seedEmbedded("k1", padded(KOTLIN_X, ZERO), scope = "topic:kotlin") + seedEmbedded("k2", padded(KOTLIN_NEAR_X, KOTLIN_NEAR_Y), scope = "topic:kotlin") + seedEmbedded("k3", padded(KOTLIN_MID_X, KOTLIN_MID_Y), scope = "topic:kotlin") + seedEmbedded("p1", padded(ZERO, POSTGRES_Y), scope = "topic:postgres") + seedEmbedded("p2", padded(POSTGRES_NEAR_X, POSTGRES_NEAR_Y), scope = "topic:postgres") + + val query = padded(KOTLIN_X, ZERO) + val suggestions = embeddingRepository.suggestTopic(query, limit = 5) + + assertThat(suggestions.map { it.slug }).containsExactly("kotlin", "postgres") + // Closer topic scores higher than the further one. + assertThat(suggestions.first().score).isGreaterThan(suggestions.last().score) + } + + @Test + fun findduplicatesReturnsRowsAboveTheCosineThresholdAndExcludesTheSource() { + val origin = seedEmbedded("source", padded(KOTLIN_X, ZERO), scope = "personal") + seedEmbedded("near", padded(DUPE_NEAR_X, DUPE_NEAR_Y), scope = "personal") + seedEmbedded("far", padded(ZERO, POSTGRES_Y), scope = "personal") + + // threshold=0.85 cosine → distance ≤ 0.15. The `near` row should + // survive; `far` (orthogonal) and `origin` (excluded by id) drop. + val dupes = + embeddingRepository.findDuplicates( + queryEmbedding = padded(KOTLIN_X, ZERO), + threshold = DUPLICATE_THRESHOLD, + limit = 10, + excludeId = origin.id, + ) + + assertThat(dupes.map { it.title }).containsExactly("near") + assertThat(dupes.first().score).isGreaterThanOrEqualTo(DUPLICATE_THRESHOLD) + } + + @Test + fun embeddingforRoundTripsAPersistedVector() { + val note = seedEmbedded("round-trip", padded(HALF, HALF), scope = "personal") + val vector = embeddingRepository.embeddingFor(note.id) ?: error("expected non-null") + + assertThat(vector.size).isEqualTo(EMBEDDING_DIM) + assertThat(vector[0]).isCloseTo(HALF, within(EPSILON)) + assertThat(vector[1]).isCloseTo(HALF, within(EPSILON)) + } + + @Test + fun embeddingforReturnsNullWhenTheRowHasNoEmbedding() { + val note = + noteRepository.create( + KbNote( + id = Ulid.generate(), + type = KbNoteType.LESSON, + scope = "personal", + source = "test", + capturedAt = Instant.now(), + sessionId = null, + confidence = 0.4, + title = "unembedded", + body = "no vector here", + vaultPath = "personal/lesson/unembedded.md", + vaultCommit = null, + tags = emptySet(), + ), + ) + + assertThat(embeddingRepository.embeddingFor(note.id)).isNull() + } + + private fun padded(vararg head: Float): FloatArray { + val out = FloatArray(EMBEDDING_DIM) + head.forEachIndexed { i, v -> out[i] = v } + return out + } + + private fun seedEmbedded( + title: String, + embedding: FloatArray, + scope: String, + ): KbNote { + val note = + noteRepository.create( + KbNote( + id = Ulid.generate(), + type = KbNoteType.LESSON, + scope = scope, + source = "test", + capturedAt = Instant.now(), + sessionId = null, + confidence = 0.4, + title = title, + body = "body of $title", + vaultPath = "$scope/lesson/${title.replace(' ', '-')}.md", + vaultCommit = null, + tags = emptySet(), + ), + ) + val literal = embedding.joinToString(",", "[", "]") { it.toString() } + dsl.execute( + "UPDATE kb_notes SET embedding = ?::vector, embedding_model = 'test', embedded_at = NOW() WHERE id = ?", + literal, + note.id, ) - - assertThat(dupes.map { it.title }).containsExactly("near") - assertThat(dupes.first().score).isGreaterThanOrEqualTo(0.85) - } - - @Test - fun `embeddingFor round-trips a persisted vector`() { - val note = seedEmbedded("round-trip", padded(0.5f, 0.5f), scope = "personal") - val vector = embeddingRepository.embeddingFor(note.id) ?: error("expected non-null") - - assertThat(vector.size).isEqualTo(EMBEDDING_DIM) - assertThat(vector[0]).isCloseTo(0.5f, within(1e-3f)) - assertThat(vector[1]).isCloseTo(0.5f, within(1e-3f)) - } - - @Test - fun `embeddingFor returns null when the row has no embedding`() { - val note = - noteRepository.create( - KbNote( - id = Ulid.generate(), - type = KbNoteType.LESSON, - scope = "personal", - source = "test", - capturedAt = Instant.now(), - sessionId = null, - confidence = 0.4, - title = "unembedded", - body = "no vector here", - vaultPath = "personal/lesson/unembedded.md", - vaultCommit = null, - tags = emptySet(), - ), - ) - - assertThat(embeddingRepository.embeddingFor(note.id)).isNull() - } - - private fun padded(vararg head: Float): FloatArray { - val out = FloatArray(EMBEDDING_DIM) - head.forEachIndexed { i, v -> out[i] = v } - return out - } - - private fun seedEmbedded( - title: String, - embedding: FloatArray, - scope: String, - ): KbNote { - val note = - noteRepository.create( - KbNote( - id = Ulid.generate(), - type = KbNoteType.LESSON, - scope = scope, - source = "test", - capturedAt = Instant.now(), - sessionId = null, - confidence = 0.4, - title = title, - body = "body of $title", - vaultPath = "$scope/lesson/${title.replace(' ', '-')}.md", - vaultCommit = null, - tags = emptySet(), - ), - ) - val literal = embedding.joinToString(",", "[", "]") { it.toString() } - dsl.execute( - "UPDATE kb_notes SET embedding = ?::vector, embedding_model = 'test', embedded_at = NOW() WHERE id = ?", - literal, - note.id, - ) - return note - } - - companion object { - private const val EMBEDDING_DIM = 1024 + return note + } + + companion object { + private const val EMBEDDING_DIM = 1024 + private const val ZERO = 0.0f + private const val HALF = 0.5f + private const val KOTLIN_X = 1.0f + private const val KOTLIN_NEAR_X = 0.9f + private const val KOTLIN_NEAR_Y = 0.1f + private const val KOTLIN_MID_X = 0.95f + private const val KOTLIN_MID_Y = 0.05f + private const val POSTGRES_Y = 1.0f + private const val POSTGRES_NEAR_X = 0.1f + private const val POSTGRES_NEAR_Y = 0.9f + private const val DUPE_NEAR_X = 0.95f + private const val DUPE_NEAR_Y = 0.31f + private const val DUPLICATE_THRESHOLD = 0.85 + private const val EPSILON = 1e-3f + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/installer/InstallerControllerIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/installer/InstallerControllerIntegrationTest.kt index 35d78d6..59e85d5 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/installer/InstallerControllerIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/installer/InstallerControllerIntegrationTest.kt @@ -28,70 +28,77 @@ import org.springframework.web.context.WebApplicationContext "knowledge.installer.kb-url=https://kb.example.test", ], ) -class InstallerControllerIntegrationTest : IntegrationTestBase() { +class InstallerControllerIntegrationTest @Autowired - private lateinit var context: WebApplicationContext + constructor( + private val context: WebApplicationContext, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc - private lateinit var mockMvc: MockMvc + @BeforeEach + fun setUp() { + mockMvc = MockMvcBuilders.webAppContextSetup(context).build() + } - @BeforeEach - fun setUp() { - mockMvc = MockMvcBuilders.webAppContextSetup(context).build() - } - - @Test - fun `install_sh serves a bash script templated with the configured kb url`() { - val result = mockMvc.get("/install.sh").andReturn() + @Test + fun installShServesABashScriptTemplatedWithTheConfiguredKbUrl() { + val result = mockMvc.get("/install.sh").andReturn() - assertThat(result.response.status).isEqualTo(200) - assertThat(result.response.contentType).startsWith("text/x-shellscript") + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + assertThat(result.response.contentType).startsWith("text/x-shellscript") - val body = result.response.contentAsString - // Shebang lets `curl … | bash` pipes work, and lets the user - // also save + chmod the file as a standalone executable. - assertThat(body).startsWith("#!/usr/bin/env bash") - // The kb-url token gets substituted into the script body in - // multiple places (KB_URL constant + help text). One match is - // enough to prove the substitution ran. - assertThat(body).contains("https://kb.example.test") - // Negative assertion: every placeholder is gone. A leftover - // `@KB_URL@` or `@VERSION@` is a sign the templater missed a - // site. - assertThat(body).doesNotContain("@KB_URL@") - assertThat(body).doesNotContain("@VERSION@") - // The managed paths must be present so the operator's uninstall - // path knows what to remove. The install script references them - // via `${SKILLS_DIR}//SKILL.md` shell expansion, so we - // assert on the trailing path segment only. - assertThat(body).contains("user-prompt-submit-recall.sh") - assertThat(body).contains("topics/SKILL.md") - assertThat(body).contains("audit/SKILL.md") - assertThat(body).contains("kb-first/SKILL.md") - assertThat(body).contains("token-economy/SKILL.md") - assertThat(body).contains("agent-session-bootstrap/SKILL.md") - assertThat(body).contains("KB_RECALL_HOOK_MODE") - assertThat(body).contains("KB_DIGEST_MAX_CHARS") - assertThat(body).contains("KB_DIGEST_DEDUPE_SCORE") - assertThat(body).contains("KB_MCP_URL") - } + val body = result.response.contentAsString + // Shebang lets `curl … | bash` pipes work, and lets the user + // also save + chmod the file as a standalone executable. + assertThat(body).startsWith("#!/usr/bin/env bash") + // The kb-url token gets substituted into the script body in + // multiple places (KB_URL constant + help text). One match is + // enough to prove the substitution ran. + assertThat(body).contains("https://kb.example.test") + // Negative assertion: every placeholder is gone. A leftover + // `@KB_URL@` or `@VERSION@` is a sign the templater missed a + // site. + assertThat(body).doesNotContain("@KB_URL@") + assertThat(body).doesNotContain("@VERSION@") + // The managed paths must be present so the operator's uninstall + // path knows what to remove. The install script references them + // via `${SKILLS_DIR}//SKILL.md` shell expansion, so we + // assert on the trailing path segment only. + assertThat(body).contains("user-prompt-submit-recall.sh") + assertThat(body).contains("topics/SKILL.md") + assertThat(body).contains("audit/SKILL.md") + assertThat(body).contains("kb-first/SKILL.md") + assertThat(body).contains("token-economy/SKILL.md") + assertThat(body).contains("agent-session-bootstrap/SKILL.md") + assertThat(body).contains("KB_RECALL_HOOK_MODE") + assertThat(body).contains("KB_DIGEST_MAX_CHARS") + assertThat(body).contains("KB_DIGEST_DEDUPE_SCORE") + assertThat(body).contains("KB_MCP_URL") + } - @Test - fun `install-agents_sh serves a bash script templated with the configured kb url`() { - val result = mockMvc.get("/install-agents.sh").andReturn() + @Test + fun installAgentsShServesABashScriptTemplatedWithTheConfiguredKbUrl() { + val result = mockMvc.get("/install-agents.sh").andReturn() - assertThat(result.response.status).isEqualTo(200) - assertThat(result.response.contentType).startsWith("text/x-shellscript") + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + assertThat(result.response.contentType).startsWith("text/x-shellscript") - val body = result.response.contentAsString - assertThat(body).startsWith("#!/usr/bin/env bash") - assertThat(body).contains("https://kb.example.test") - assertThat(body).doesNotContain("@KB_URL@") - assertThat(body).doesNotContain("@VERSION@") - // This installer is a thin wrapper that delegates the base - // install by fetching the sibling install.sh from the same KB. - assertThat(body).contains("/install.sh") - // The new capability over the base installer: registering the - // knowledge MCP server with each agent. - assertThat(body).contains("mcp_servers.knowledge") + val body = result.response.contentAsString + assertThat(body).startsWith("#!/usr/bin/env bash") + assertThat(body).contains("https://kb.example.test") + assertThat(body).doesNotContain("@KB_URL@") + assertThat(body).doesNotContain("@VERSION@") + // This installer is a thin wrapper that delegates the base + // install by fetching the sibling install.sh from the same KB. + assertThat(body).contains("/install.sh") + // The new capability over the base installer: registering the + // knowledge MCP server with each agent. + assertThat(body).contains("mcp_servers.knowledge") + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminFlowIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminFlowIntegrationTest.kt index daa0ca6..09393ae 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminFlowIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminFlowIntegrationTest.kt @@ -9,6 +9,7 @@ import com.jorisjonkers.personalstack.knowledge.domain.Ulid import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository import com.jorisjonkers.personalstack.knowledge.repo.TopicRepository import org.assertj.core.api.Assertions.assertThat +import org.jooq.DSLContext import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired @@ -34,251 +35,271 @@ import java.time.Instant "knowledge.mcp.admin-tokens[0]=admin", ], ) -class AdminFlowIntegrationTest : IntegrationTestBase() { +class AdminFlowIntegrationTest @Autowired - private lateinit var context: WebApplicationContext - - @Autowired - private lateinit var mcpBearerFilter: McpBearerFilter - - @Autowired - private lateinit var noteRepository: NoteRepository - - @Autowired - private lateinit var topicRepository: TopicRepository - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .webAppContextSetup(context) - .addFilters(mcpBearerFilter) - .build() - } - - @Test - fun `add_topic refuses a non-admin bearer with -32001`() { - val response = - callRaw( - bearer = "user-token", - name = "knowledge.add_topic", - args = mapOf("slug" to "rust", "description" to "Rust ecosystem"), + constructor( + private val context: WebApplicationContext, + private val mcpBearerFilter: McpBearerFilter, + private val noteRepository: NoteRepository, + private val topicRepository: TopicRepository, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + + private val objectMapper = jacksonObjectMapper() + + /** + * Clean topics created by earlier tests before each test so ordering does not bleed + * state into siblings. Flyway-seeded topics (created_by = 'seed') survive; test-created + * topics (tool calls → 'mcp:admin', seedTopic → 'test-fixture') are removed. + */ + @BeforeEach + fun setUp( + @Autowired dsl: DSLContext, + ) { + dsl.execute( + "DELETE FROM kb_topic_aliases WHERE slug IN " + + "(SELECT slug FROM kb_topics WHERE created_by != 'seed')", ) - val error = objectMapper.readTree(response)["error"] - assertThat(error["code"].asInt()).isEqualTo(-32001) - assertThat(topicRepository.findBySlug("rust")).isNull() - } - - @Test - fun `add_topic with an admin bearer inserts the slug and surfaces it via list_topics`() { - val response = - callRaw( - bearer = "admin-token", - name = "knowledge.add_topic", - args = - mapOf( - "slug" to "rust", - "description" to "Rust ecosystem", - "aliases" to listOf("rs", "Rust"), - ), + dsl.execute("DELETE FROM kb_topics WHERE created_by != 'seed'") + mockMvc = + MockMvcBuilders + .webAppContextSetup(context) + .addFilters(mcpBearerFilter) + .build() + } + + @Test + fun addTopicRefusesANonAdminBearer() { + val response = + callRaw( + bearer = "user-token", + name = "knowledge.add_topic", + args = mapOf("slug" to "rust", "description" to "Rust ecosystem"), + ) + val error = objectMapper.readTree(response)["error"] + assertThat(error["code"].asInt()).isEqualTo(UNAUTHORIZED_CODE) + assertThat(topicRepository.findBySlug("rust")).isNull() + } + + @Test + fun addTopicWithAnAdminBearerInsertsTheSlugAndSurfacesItViaListTopics() { + val response = + callRaw( + bearer = "admin-token", + name = "knowledge.add_topic", + args = + mapOf( + "slug" to "rust", + "description" to "Rust ecosystem", + "aliases" to listOf("rs", "Rust"), + ), + ) + val structured = objectMapper.readTree(response)["result"]["structuredContent"] + assertThat(structured["topic"]["slug"].asText()).isEqualTo("rust") + assertThat(structured["topic"]["is_active"].asBoolean()).isTrue + val aliases = structured["topic"]["aliases"].map { it.asText() } + // The slug doubles as its own alias by convention; passed + // aliases are lowercased on insert. + assertThat(aliases).contains("rust", "rs") + } + + @Test + fun updateTopicFlipsIsActiveAndReplacesAliasesWholesale() { + seedTopic(slug = "ephemeral", description = "to be retired") + + val response = + callRaw( + bearer = "admin-token", + name = "knowledge.update_topic", + args = + mapOf( + "slug" to "ephemeral", + "description" to "retired", + "aliases" to listOf("old-name"), + "is_active" to false, + ), + ) + + val topic = objectMapper.readTree(response)["result"]["structuredContent"]["topic"] + assertThat(topic["is_active"].asBoolean()).isFalse + assertThat(topic["description"].asText()).isEqualTo("retired") + } + + @Test + fun mergeTopicsMovesEveryScopedNoteOverAndSoftDeactivatesTheSource() { + seedTopic(slug = "old-slug", description = "legacy") + seedTopic(slug = "new-slug", description = "consolidated home") + val a = seedNote(scope = "topic:old-slug") + val b = seedNote(scope = "topic:old-slug") + val c = seedNote(scope = "topic:new-slug") + + val response = + callRaw( + bearer = "admin-token", + name = "knowledge.merge_topics", + args = mapOf("from_slug" to "old-slug", "into_slug" to "new-slug"), + ) + + val structured = objectMapper.readTree(response)["result"]["structuredContent"] + assertThat(structured["notes_moved"].asInt()).isEqualTo(2) + assertThat(structured["actor"].asText()).isEqualTo("mcp:admin") + + // Source slug is now soft-deactivated; both notes carry the + // new scope; the destination's existing note is unaffected. + assertThat(topicRepository.findBySlug("old-slug")?.isActive).isFalse + assertThat(noteRepository.findById(a.id)?.scope).isEqualTo("topic:new-slug") + assertThat(noteRepository.findById(b.id)?.scope).isEqualTo("topic:new-slug") + assertThat(noteRepository.findById(c.id)?.scope).isEqualTo("topic:new-slug") + } + + @Test + fun renameTagUpdatesEveryKbNoteTagsRowTouchingTheSourceTag() { + seedNote(tags = setOf("kt", "spring")) + seedNote(tags = setOf("kt")) + seedNote(tags = setOf("vue")) + + val response = + callRaw( + bearer = "admin-token", + name = "knowledge.rename_tag", + args = mapOf("from" to "kt", "to" to "kotlin"), + ) + + val structured = objectMapper.readTree(response)["result"]["structuredContent"] + assertThat(structured["rows_touched"].asInt()).isEqualTo(2) + assertThat(structured["from"].asText()).isEqualTo("kt") + assertThat(structured["to"].asText()).isEqualTo("kotlin") + + val auditRows = auditRows("rename_tag") + assertThat(auditRows.size()).isEqualTo(1) + val audit = auditRows[0] + assertThat(audit["id"].asText()).isEqualTo(structured["audit_id"].asText()) + assertThat(audit["actor"].asText()).isEqualTo("mcp:admin") + assertThat(audit["target_kind"].asText()).isEqualTo("tag") + assertThat(audit["before_json"].asText()).isEqualTo("""{"tag":"kt"}""") + assertThat(audit["after_json"].asText()).isEqualTo("""{"tag":"kotlin","rows_touched":2}""") + } + + @Test + fun mergeTagsFoldsDuplicateAndSourceRowsIntoTheCanonicalTagIdempotently() { + val duplicate = seedNote(tags = setOf("kt", "kotlin")) + val renamed = seedNote(tags = setOf("kt", "spring")) + val renamedSecondSource = seedNote(tags = setOf("kts", "gradle")) + val unrelated = seedNote(tags = setOf("vue")) + + val response = + callRaw( + bearer = "admin-token", + name = "knowledge.merge_tags", + args = mapOf("from" to listOf("kt", "kts"), "into" to "kotlin"), + ) + + val structured = objectMapper.readTree(response)["result"]["structuredContent"] + assertThat(structured["from"].map { it.asText() }).containsExactly("kt", "kts") + assertThat(structured["into"].asText()).isEqualTo("kotlin") + assertThat(structured["rows_renamed"].asInt()).isEqualTo(2) + assertThat(structured["rows_dropped_as_dupes"].asInt()).isEqualTo(1) + assertThat(structured["actor"].asText()).isEqualTo("mcp:admin") + + assertThat(noteRepository.findById(duplicate.id)?.tags).containsExactlyInAnyOrder("kotlin") + assertThat(noteRepository.findById(renamed.id)?.tags).containsExactlyInAnyOrder("kotlin", "spring") + assertThat(noteRepository.findById(renamedSecondSource.id)?.tags) + .containsExactlyInAnyOrder("kotlin", "gradle") + assertThat(noteRepository.findById(unrelated.id)?.tags).containsExactlyInAnyOrder("vue") + + val secondResponse = + callRaw( + bearer = "admin-token", + name = "knowledge.merge_tags", + args = mapOf("from" to listOf("kt", "kts"), "into" to "kotlin"), + ) + + val secondStructured = objectMapper.readTree(secondResponse)["result"]["structuredContent"] + assertThat(secondStructured["rows_renamed"].asInt()).isEqualTo(0) + assertThat(secondStructured["rows_dropped_as_dupes"].asInt()).isEqualTo(0) + assertThat(secondStructured.has("audit_id")).isFalse() + + val auditRows = auditRows("merge_tags") + assertThat(auditRows.size()).isEqualTo(1) + val audit = auditRows[0] + assertThat(audit["id"].asText()).isEqualTo(structured["audit_id"].asText()) + assertThat(audit["actor"].asText()).isEqualTo("mcp:admin") + assertThat(audit["target_kind"].asText()).isEqualTo("tag") + assertThat(audit["before_json"].asText()).isEqualTo("""{"from":["kt","kts"]}""") + assertThat(audit["after_json"].asText()) + .isEqualTo("""{"into":"kotlin","rows_renamed":2,"rows_dropped_as_dupes":1}""") + } + + private fun seedTopic( + slug: String, + description: String, + ) = topicRepository.insert( + slug = slug, + description = description, + aliases = emptySet(), + createdBy = "test-fixture", + ) + + private fun seedNote( + scope: String = "personal", + tags: Set = emptySet(), + ): KbNote { + val note = + KbNote( + id = Ulid.generate(), + type = KbNoteType.LESSON, + scope = scope, + source = "test", + capturedAt = Instant.now(), + sessionId = null, + confidence = 0.4, + title = "title", + body = "body", + vaultPath = "$scope/draft.md", + vaultCommit = null, + tags = tags, + ) + return noteRepository.create(note) + } + + private fun callRaw( + bearer: String, + name: String, + args: Map, + ): String { + val body = + mapOf( + "jsonrpc" to "2.0", + "id" to "test-request", + "method" to "tools/call", + "params" to mapOf("name" to name, "arguments" to args), + ) + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer $bearer") + content = objectMapper.writeValueAsBytes(body) + }.andReturn() + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), ) - val structured = objectMapper.readTree(response)["result"]["structuredContent"] - assertThat(structured["topic"]["slug"].asText()).isEqualTo("rust") - assertThat(structured["topic"]["is_active"].asBoolean()).isTrue - val aliases = structured["topic"]["aliases"].map { it.asText() } - // The slug doubles as its own alias by convention; passed - // aliases are lowercased on insert. - assertThat(aliases).contains("rust", "rs") - } - - @Test - fun `update_topic flips is_active and replaces aliases wholesale`() { - seedTopic(slug = "ephemeral", description = "to be retired") - - val response = - callRaw( - bearer = "admin-token", - name = "knowledge.update_topic", - args = - mapOf( - "slug" to "ephemeral", - "description" to "retired", - "aliases" to listOf("old-name"), - "is_active" to false, + return result.response.contentAsString + } + + private fun auditRows(action: String) = + objectMapper + .readTree( + callRaw( + bearer = "admin-token", + name = "knowledge.list_audit", + args = mapOf("action" to action, "limit" to AUDIT_LIMIT), ), - ) + )["result"]["structuredContent"]["rows"] - val topic = objectMapper.readTree(response)["result"]["structuredContent"]["topic"] - assertThat(topic["is_active"].asBoolean()).isFalse - assertThat(topic["description"].asText()).isEqualTo("retired") + private companion object { + private const val UNAUTHORIZED_CODE = -32001 + private const val AUDIT_LIMIT = 10 + } } - - @Test - fun `merge_topics moves every scoped note over and soft-deactivates the source`() { - seedTopic(slug = "old-slug", description = "legacy") - seedTopic(slug = "new-slug", description = "consolidated home") - val a = seedNote(scope = "topic:old-slug") - val b = seedNote(scope = "topic:old-slug") - val c = seedNote(scope = "topic:new-slug") - - val response = - callRaw( - bearer = "admin-token", - name = "knowledge.merge_topics", - args = mapOf("from_slug" to "old-slug", "into_slug" to "new-slug"), - ) - - val structured = objectMapper.readTree(response)["result"]["structuredContent"] - assertThat(structured["notes_moved"].asInt()).isEqualTo(2) - assertThat(structured["actor"].asText()).isEqualTo("mcp:admin") - - // Source slug is now soft-deactivated; both notes carry the - // new scope; the destination's existing note is unaffected. - assertThat(topicRepository.findBySlug("old-slug")?.isActive).isFalse - assertThat(noteRepository.findById(a.id)?.scope).isEqualTo("topic:new-slug") - assertThat(noteRepository.findById(b.id)?.scope).isEqualTo("topic:new-slug") - assertThat(noteRepository.findById(c.id)?.scope).isEqualTo("topic:new-slug") - } - - @Test - fun `rename_tag updates every kb_note_tags row touching the source tag`() { - seedNote(tags = setOf("kt", "spring")) - seedNote(tags = setOf("kt")) - seedNote(tags = setOf("vue")) - - val response = - callRaw( - bearer = "admin-token", - name = "knowledge.rename_tag", - args = mapOf("from" to "kt", "to" to "kotlin"), - ) - - val structured = objectMapper.readTree(response)["result"]["structuredContent"] - assertThat(structured["rows_touched"].asInt()).isEqualTo(2) - assertThat(structured["from"].asText()).isEqualTo("kt") - assertThat(structured["to"].asText()).isEqualTo("kotlin") - - val auditRows = auditRows("rename_tag") - assertThat(auditRows.size()).isEqualTo(1) - val audit = auditRows[0] - assertThat(audit["id"].asText()).isEqualTo(structured["audit_id"].asText()) - assertThat(audit["actor"].asText()).isEqualTo("mcp:admin") - assertThat(audit["target_kind"].asText()).isEqualTo("tag") - assertThat(audit["before_json"].asText()).isEqualTo("""{"tag":"kt"}""") - assertThat(audit["after_json"].asText()).isEqualTo("""{"tag":"kotlin","rows_touched":2}""") - } - - @Test - fun `merge_tags folds duplicate and source rows into the canonical tag idempotently`() { - val duplicate = seedNote(tags = setOf("kt", "kotlin")) - val renamed = seedNote(tags = setOf("kt", "spring")) - val renamedSecondSource = seedNote(tags = setOf("kts", "gradle")) - val unrelated = seedNote(tags = setOf("vue")) - - val response = - callRaw( - bearer = "admin-token", - name = "knowledge.merge_tags", - args = mapOf("from" to listOf("kt", "kts"), "into" to "kotlin"), - ) - - val structured = objectMapper.readTree(response)["result"]["structuredContent"] - assertThat(structured["from"].map { it.asText() }).containsExactly("kt", "kts") - assertThat(structured["into"].asText()).isEqualTo("kotlin") - assertThat(structured["rows_renamed"].asInt()).isEqualTo(2) - assertThat(structured["rows_dropped_as_dupes"].asInt()).isEqualTo(1) - assertThat(structured["actor"].asText()).isEqualTo("mcp:admin") - - assertThat(noteRepository.findById(duplicate.id)?.tags).containsExactlyInAnyOrder("kotlin") - assertThat(noteRepository.findById(renamed.id)?.tags).containsExactlyInAnyOrder("kotlin", "spring") - assertThat(noteRepository.findById(renamedSecondSource.id)?.tags) - .containsExactlyInAnyOrder("kotlin", "gradle") - assertThat(noteRepository.findById(unrelated.id)?.tags).containsExactlyInAnyOrder("vue") - - val secondResponse = - callRaw( - bearer = "admin-token", - name = "knowledge.merge_tags", - args = mapOf("from" to listOf("kt", "kts"), "into" to "kotlin"), - ) - - val secondStructured = objectMapper.readTree(secondResponse)["result"]["structuredContent"] - assertThat(secondStructured["rows_renamed"].asInt()).isEqualTo(0) - assertThat(secondStructured["rows_dropped_as_dupes"].asInt()).isEqualTo(0) - assertThat(secondStructured.has("audit_id")).isFalse() - - val auditRows = auditRows("merge_tags") - assertThat(auditRows.size()).isEqualTo(1) - val audit = auditRows[0] - assertThat(audit["id"].asText()).isEqualTo(structured["audit_id"].asText()) - assertThat(audit["actor"].asText()).isEqualTo("mcp:admin") - assertThat(audit["target_kind"].asText()).isEqualTo("tag") - assertThat(audit["before_json"].asText()).isEqualTo("""{"from":["kt","kts"]}""") - assertThat(audit["after_json"].asText()) - .isEqualTo("""{"into":"kotlin","rows_renamed":2,"rows_dropped_as_dupes":1}""") - } - - private fun seedTopic( - slug: String, - description: String, - ) = topicRepository.insert(slug = slug, description = description, aliases = emptySet(), createdBy = "seed") - - private fun seedNote( - scope: String = "personal", - tags: Set = emptySet(), - ): KbNote { - val note = - KbNote( - id = Ulid.generate(), - type = KbNoteType.LESSON, - scope = scope, - source = "test", - capturedAt = Instant.now(), - sessionId = null, - confidence = 0.4, - title = "title", - body = "body", - vaultPath = "$scope/draft.md", - vaultCommit = null, - tags = tags, - ) - return noteRepository.create(note) - } - - private fun callRaw( - bearer: String, - name: String, - args: Map, - ): String { - val body = - mapOf( - "jsonrpc" to "2.0", - "id" to 1, - "method" to "tools/call", - "params" to mapOf("name" to name, "arguments" to args), - ) - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer $bearer") - content = objectMapper.writeValueAsBytes(body) - }.andReturn() - assertThat(result.response.status).isEqualTo(200) - return result.response.contentAsString - } - - private fun auditRows(action: String) = - objectMapper - .readTree( - callRaw( - bearer = "admin-token", - name = "knowledge.list_audit", - args = mapOf("action" to action, "limit" to 10), - ), - )["result"]["structuredContent"]["rows"] -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerIntegrationTest.kt index e43eeb6..5c88319 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerIntegrationTest.kt @@ -20,204 +20,238 @@ import org.springframework.web.context.WebApplicationContext "knowledge.mcp.tokens.laptop=test-token-lap", ], ) -class McpControllerIntegrationTest : IntegrationTestBase() { +class McpControllerIntegrationTest @Autowired - private lateinit var context: WebApplicationContext + constructor( + private val context: WebApplicationContext, + private val mcpBearerFilter: McpBearerFilter, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc - @Autowired - private lateinit var mcpBearerFilter: McpBearerFilter - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - // `MockMvcBuilders.webAppContextSetup` only wires Spring MVC - // dispatch — it does *not* auto-mount servlet filters even - // when those beans are present in the context. Without an - // explicit `.addFilter(...)`, every /mcp request bypasses - // McpBearerFilter entirely and would return 200, masking - // every authn assertion below as silently green. - mockMvc = - MockMvcBuilders - .webAppContextSetup(context) - .addFilters(mcpBearerFilter) - .build() - } + private val objectMapper = jacksonObjectMapper() - @Test - fun `mcp without authorization header returns 401 with json-rpc error body`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - content = """{"jsonrpc":"2.0","id":1,"method":"ping"}""" - }.andReturn() - - assertThat(result.response.status).isEqualTo(401) - assertThat(result.response.getHeader("WWW-Authenticate")).contains("Bearer") - val body = objectMapper.readTree(result.response.contentAsString) - assertThat(body["error"]["code"].asInt()).isEqualTo(-32001) - } + @BeforeEach + fun setUp() { + // `MockMvcBuilders.webAppContextSetup` only wires Spring MVC + // dispatch — it does *not* auto-mount servlet filters even + // when those beans are present in the context. Without an + // explicit `.addFilter(...)`, every /mcp request bypasses + // McpBearerFilter entirely and would return 200, masking + // every authn assertion below as silently green. + mockMvc = + MockMvcBuilders + .webAppContextSetup(context) + .addFilters(mcpBearerFilter) + .build() + } - @Test - fun `mcp with wrong bearer token returns 401`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer nope") - content = """{"jsonrpc":"2.0","id":1,"method":"ping"}""" - }.andReturn() - - assertThat(result.response.status).isEqualTo(401) - } + @Test + fun mcpWithoutAuthorizationHeaderReturnsUnauthorizedWithJsonRpcErrorBody() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + content = """{"jsonrpc":"2.0","id":1,"method":"ping"}""" + }.andReturn() - @Test - fun `mcp ping with valid bearer returns empty result and sets X-User-Id`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = """{"jsonrpc":"2.0","id":42,"method":"ping"}""" - }.andReturn() - - assertThat(result.response.status).isEqualTo(200) - assertThat(result.response.getHeader("X-User-Id")).isEqualTo("mcp:workstation") - val body = objectMapper.readTree(result.response.contentAsString) - assertThat(body["jsonrpc"].asText()).isEqualTo("2.0") - assertThat(body["id"].asInt()).isEqualTo(42) - assertThat(body["result"]).isNotNull - // `error` is omitted entirely on success thanks to - // `@JsonInclude(NON_NULL)` on JsonRpcResponse. - assertThat(body.has("error")).isFalse - } + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.UNAUTHORIZED + .value(), + ) + assertThat(result.response.getHeader("WWW-Authenticate")).contains("Bearer") + val body = objectMapper.readTree(result.response.contentAsString) + assertThat(body["error"]["code"].asInt()).isEqualTo(UNAUTHORIZED_CODE) + } - @Test - fun `mcp initialize advertises tools and prompts capabilities plus the protocol version`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-lap") - content = """{"jsonrpc":"2.0","id":1,"method":"initialize"}""" - }.andReturn() - - assertThat(result.response.status).isEqualTo(200) - val result0 = objectMapper.readTree(result.response.contentAsString)["result"] - assertThat(result0["protocolVersion"].asText()).isEqualTo("2025-06-18") - assertThat(result0["serverInfo"]["name"].asText()).isEqualTo("knowledge-api") - assertThat(result0["capabilities"]["tools"]["listChanged"].asBoolean()).isFalse - assertThat(result0["capabilities"]["prompts"]["listChanged"].asBoolean()).isFalse - } + @Test + fun mcpWithWrongBearerTokenReturnsUnauthorized() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer nope") + content = """{"jsonrpc":"2.0","id":1,"method":"ping"}""" + }.andReturn() - @Test - fun `mcp tools list advertises the registered tools as a JSON-RPC result array`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = """{"jsonrpc":"2.0","id":2,"method":"tools/list"}""" - }.andReturn() - - assertThat(result.response.status).isEqualTo(200) - // Assert on the contract — `tools` is a JSON array of objects - // with `name` set — rather than pinning a count. The capture - // suite (CaptureFlowIntegrationTest) already pins the specific - // tool names; adding new tools should not regress this test. - val tools = objectMapper.readTree(result.response.contentAsString)["result"]["tools"] - assertThat(tools.isArray).isTrue - assertThat(tools.size()).isGreaterThan(0) - tools.forEach { assertThat(it["name"].asText()).isNotBlank } - } + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.UNAUTHORIZED + .value(), + ) + } - @Test - fun `mcp prompts list returns the registered prompts with their argument metadata`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = """{"jsonrpc":"2.0","id":10,"method":"prompts/list"}""" - }.andReturn() - - assertThat(result.response.status).isEqualTo(200) - val prompts = objectMapper.readTree(result.response.contentAsString)["result"]["prompts"] - assertThat(prompts.isArray).isTrue - val names = prompts.map { it["name"].asText() } - assertThat(names).contains("recall_for_task", "capture_lesson_about", "topics_audit") - } + @Test + fun mcpPingWithValidBearerReturnsEmptyResultAndSetsXUserId() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = """{"jsonrpc":"2.0","id":"test-request","method":"ping"}""" + }.andReturn() - @Test - fun `mcp prompts get renders the recall_for_task message with the supplied task verbatim`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = - """ - {"jsonrpc":"2.0","id":11,"method":"prompts/get","params": - {"name":"recall_for_task","arguments":{"task":"add a Vue component"}}} - """.trimIndent() - }.andReturn() - - assertThat(result.response.status).isEqualTo(200) - val out = objectMapper.readTree(result.response.contentAsString)["result"] - assertThat(out["messages"].size()).isEqualTo(1) - val msg = out["messages"][0] - assertThat(msg["role"].asText()).isEqualTo("user") - assertThat(msg["content"]["type"].asText()).isEqualTo("text") - val text = msg["content"]["text"].asText() - assertThat(text).contains("add a Vue component") - assertThat(text).contains("knowledge.recall") - assertThat(text).contains("knowledge.list_topics") - } + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + assertThat(result.response.getHeader("X-User-Id")).isEqualTo("mcp:workstation") + val body = objectMapper.readTree(result.response.contentAsString) + assertThat(body["jsonrpc"].asText()).isEqualTo("2.0") + assertThat(body["id"].asText()).isEqualTo("test-request") + assertThat(body["result"]).isNotNull + // `error` is omitted entirely on success thanks to + // `@JsonInclude(NON_NULL)` on JsonRpcResponse. + assertThat(body.has("error")).isFalse + } - @Test - fun `mcp prompts get on an unknown name returns method_not_found`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = - """ - {"jsonrpc":"2.0","id":12,"method":"prompts/get", - "params":{"name":"nonexistent"}} - """.trimIndent() - }.andReturn() - - assertThat(result.response.status).isEqualTo(200) - val error = objectMapper.readTree(result.response.contentAsString)["error"] - assertThat(error["code"].asInt()).isEqualTo(-32601) - } + @Test + fun mcpInitializeAdvertisesToolsAndPromptsCapabilitiesPlusTheProtocolVersion() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-lap") + content = """{"jsonrpc":"2.0","id":1,"method":"initialize"}""" + }.andReturn() - @Test - fun `mcp unknown method returns method_not_found error`() { - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = """{"jsonrpc":"2.0","id":3,"method":"resources/list"}""" - }.andReturn() - - assertThat(result.response.status).isEqualTo(200) - val error = objectMapper.readTree(result.response.contentAsString)["error"] - assertThat(error["code"].asInt()).isEqualTo(-32601) - } + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + val result0 = objectMapper.readTree(result.response.contentAsString)["result"] + assertThat(result0["protocolVersion"].asText()).isEqualTo("2025-06-18") + assertThat(result0["serverInfo"]["name"].asText()).isEqualTo("knowledge-api") + assertThat(result0["capabilities"]["tools"]["listChanged"].asBoolean()).isFalse + assertThat(result0["capabilities"]["prompts"]["listChanged"].asBoolean()).isFalse + } + + @Test + fun mcpToolsListAdvertisesTheRegisteredToolsAsAJsonRpcResultArray() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = """{"jsonrpc":"2.0","id":2,"method":"tools/list"}""" + }.andReturn() + + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + // Assert on the contract — `tools` is a JSON array of objects + // with `name` set — rather than pinning a count. The capture + // suite (CaptureFlowIntegrationTest) already pins the specific + // tool names; adding new tools should not regress this test. + val tools = objectMapper.readTree(result.response.contentAsString)["result"]["tools"] + assertThat(tools.isArray).isTrue + assertThat(tools.size()).isGreaterThan(0) + tools.forEach { assertThat(it["name"].asText()).isNotBlank } + } + + @Test + fun mcpPromptsListReturnsTheRegisteredPromptsWithTheirArgumentMetadata() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = """{"jsonrpc":"2.0","id":10,"method":"prompts/list"}""" + }.andReturn() + + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + val prompts = objectMapper.readTree(result.response.contentAsString)["result"]["prompts"] + assertThat(prompts.isArray).isTrue + val names = prompts.map { it["name"].asText() } + assertThat(names).contains("recall_for_task", "capture_lesson_about", "topics_audit") + } + + @Test + fun mcpPromptsGetRendersTheRecallForTaskMessageWithTheSuppliedTaskVerbatim() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = + """ + {"jsonrpc":"2.0","id":11,"method":"prompts/get","params": + {"name":"recall_for_task","arguments":{"task":"add a Vue component"}}} + """.trimIndent() + }.andReturn() + + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + val out = objectMapper.readTree(result.response.contentAsString)["result"] + assertThat(out["messages"].size()).isEqualTo(1) + val msg = out["messages"][0] + assertThat(msg["role"].asText()).isEqualTo("user") + assertThat(msg["content"]["type"].asText()).isEqualTo("text") + val text = msg["content"]["text"].asText() + assertThat(text).contains("add a Vue component") + assertThat(text).contains("knowledge.recall") + assertThat(text).contains("knowledge.list_topics") + } + + @Test + fun mcpPromptsGetOnAnUnknownNameReturnsMethodNotFound() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = + """ + {"jsonrpc":"2.0","id":12,"method":"prompts/get", + "params":{"name":"nonexistent"}} + """.trimIndent() + }.andReturn() + + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + val error = objectMapper.readTree(result.response.contentAsString)["error"] + assertThat(error["code"].asInt()).isEqualTo(METHOD_NOT_FOUND_CODE) + } + + @Test + fun mcpUnknownMethodReturnsMethodNotFoundError() { + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = """{"jsonrpc":"2.0","id":3,"method":"resources/list"}""" + }.andReturn() + + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + val error = objectMapper.readTree(result.response.contentAsString)["error"] + assertThat(error["code"].asInt()).isEqualTo(METHOD_NOT_FOUND_CODE) + } + + @Test + fun nonMcpPathsAreUnaffectedByTheBearerFilter() { + // Sanity: actuator/health is reachable from auth-less local + // calls (the integration-test profile doesn't ship security), + // and the McpBearerFilter must not interfere with anything + // outside `/mcp**`. + val result = mockMvc.post("/api/actuator/health") { contentType = MediaType.APPLICATION_JSON }.andReturn() + assertThat(result.response.status).isNotEqualTo( + org.springframework.http.HttpStatus.UNAUTHORIZED + .value(), + ) + } - @Test - fun `non-mcp paths are unaffected by the bearer filter`() { - // Sanity: actuator/health is reachable from auth-less local - // calls (the integration-test profile doesn't ship security), - // and the McpBearerFilter must not interfere with anything - // outside `/mcp**`. - val result = mockMvc.post("/api/actuator/health") { contentType = MediaType.APPLICATION_JSON }.andReturn() - assertThat(result.response.status).isNotEqualTo(401) + private companion object { + private const val UNAUTHORIZED_CODE = -32001 + private const val METHOD_NOT_FOUND_CODE = -32601 + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/EmbeddingRepositoryIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/EmbeddingRepositoryIntegrationTest.kt index cb25d78..fda9091 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/EmbeddingRepositoryIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/EmbeddingRepositoryIntegrationTest.kt @@ -22,105 +22,107 @@ import java.time.Instant * running Ollama — the JDBC path is the only thing under test. PR-2 * adds the curator-side embed-on-promote integration test. */ -class EmbeddingRepositoryIntegrationTest : IntegrationTestBase() { +class EmbeddingRepositoryIntegrationTest @Autowired - private lateinit var noteRepository: NoteRepository - - @Autowired - private lateinit var embeddingRepository: EmbeddingRepository - - @Autowired - private lateinit var dsl: DSLContext - - @Test - fun `recallVector orders rows by cosine similarity`() { - // Three notes; the embedding column is 1024-dim per the V9 schema, - // so we pad the meaningful prefix with zeros. The vectors differ - // only on the first three dimensions, which is enough to drive - // distinct cosine distances. - seedEmbedded("near", padded(1.0f, 0.0f, 0.0f)) - seedEmbedded("middle", padded(0.6f, 0.6f, 0.0f)) - seedEmbedded("far", padded(0.0f, 0.0f, 1.0f)) - - val query = padded(1.0f, 0.0f, 0.0f) - val hits = embeddingRepository.recallVector(query, scope = "personal", limit = 3) - - assertThat(hits.map { it.title }).containsExactly("near", "middle", "far") - // Score is `1 - cosine_distance`, so the perfect-match row scores 1. - assertThat(hits.first().score).isCloseTo(1.0, within(1e-6)) - } - - @Test - fun `recallVector skips rows whose embedding is NULL`() { - // Real-world rows during the rollout window have no embedding - // yet; the vector leg must treat them as invisible so the FTS - // leg owns those hits via RRF. - seedEmbedded("embedded", padded(1.0f, 0.0f, 0.0f)) - seedUnembedded("not-yet") - - val hits = embeddingRepository.recallVector(padded(1.0f, 0.0f, 0.0f), scope = "personal", limit = 10) - - assertThat(hits.map { it.title }).containsExactly("embedded") - } - - @Test - fun `recallVector scope=null applies the curated default filter`() { - seedEmbedded("public", padded(1.0f, 0.0f, 0.0f), scope = "personal") - seedEmbedded("inbox", padded(1.0f, 0.0f, 0.0f), scope = "_inbox") - seedEmbedded("private-agent", padded(1.0f, 0.0f, 0.0f), scope = "agent:claude") - seedEmbedded("shared-agent", padded(1.0f, 0.0f, 0.0f), scope = "agent:_shared") - - val hits = embeddingRepository.recallVector(padded(1.0f, 0.0f, 0.0f), scope = null, limit = 10) - - // _inbox and agent: filtered out; agent:_shared stays. - assertThat(hits.map { it.title }).containsExactlyInAnyOrder("public", "shared-agent") - } - - private fun padded(vararg head: Float): FloatArray { - val out = FloatArray(EMBEDDING_DIM) - head.forEachIndexed { i, v -> out[i] = v } - return out - } - - private fun seedEmbedded( - title: String, - embedding: FloatArray, - scope: String = "personal", - ): KbNote { - val note = seedRaw(title = title, scope = scope) - val literal = embedding.joinToString(",", "[", "]") { it.toString() } - dsl.execute( - "UPDATE kb_notes SET embedding = ?::vector, embedding_model = 'test', embedded_at = NOW() WHERE id = ?", - literal, - note.id, - ) - return note - } - - private fun seedUnembedded(title: String): KbNote = seedRaw(title = title) - - private fun seedRaw( - title: String, - scope: String = "personal", - ): KbNote = - noteRepository.create( - KbNote( - id = Ulid.generate(), - type = KbNoteType.LESSON, - scope = scope, - source = "test", - capturedAt = Instant.now(), - sessionId = null, - confidence = 0.4, - title = title, - body = "body of $title", - vaultPath = "$scope/lesson/${title.replace(' ', '-')}.md", - vaultCommit = null, - tags = emptySet(), - ), - ) - - companion object { - private const val EMBEDDING_DIM = 1024 + constructor( + private val noteRepository: NoteRepository, + private val embeddingRepository: EmbeddingRepository, + private val dsl: DSLContext, + ) : IntegrationTestBase() { + @Test + fun recallvectorOrdersRowsByCosineSimilarity() { + // Three notes; the embedding column is 1024-dim per the V9 schema, + // so we pad the meaningful prefix with zeros. The vectors differ + // only on the first three dimensions, which is enough to drive + // distinct cosine distances. + seedEmbedded("near", padded(AXIS_MATCH, ZERO, ZERO)) + seedEmbedded("middle", padded(MIDDLE_COMPONENT, MIDDLE_COMPONENT, ZERO)) + seedEmbedded("far", padded(ZERO, ZERO, AXIS_MATCH)) + + val query = padded(AXIS_MATCH, ZERO, ZERO) + val hits = embeddingRepository.recallVector(query, scope = "personal", limit = 3) + + assertThat(hits.map { it.title }).containsExactly("near", "middle", "far") + // Score is `1 - cosine_distance`, so the perfect-match row scores 1. + assertThat(hits.first().score).isCloseTo(PERFECT_SCORE, within(SCORE_EPSILON)) + } + + @Test + fun recallvectorSkipsRowsWhoseEmbeddingIsNULL() { + // Real-world rows during the rollout window have no embedding + // yet; the vector leg must treat them as invisible so the FTS + // leg owns those hits via RRF. + seedEmbedded("embedded", padded(1.0f, 0.0f, 0.0f)) + seedUnembedded("not-yet") + + val hits = embeddingRepository.recallVector(padded(1.0f, 0.0f, 0.0f), scope = "personal", limit = 10) + + assertThat(hits.map { it.title }).containsExactly("embedded") + } + + @Test + fun recallvectorScopeNullAppliesTheCuratedDefaultFilter() { + seedEmbedded("public", padded(1.0f, 0.0f, 0.0f), scope = "personal") + seedEmbedded("inbox", padded(1.0f, 0.0f, 0.0f), scope = "_inbox") + seedEmbedded("private-agent", padded(1.0f, 0.0f, 0.0f), scope = "agent:claude") + seedEmbedded("shared-agent", padded(1.0f, 0.0f, 0.0f), scope = "agent:_shared") + + val hits = embeddingRepository.recallVector(padded(1.0f, 0.0f, 0.0f), scope = null, limit = 10) + + // _inbox and agent: filtered out; agent:_shared stays. + assertThat(hits.map { it.title }).containsExactlyInAnyOrder("public", "shared-agent") + } + + private fun padded(vararg head: Float): FloatArray { + val out = FloatArray(EMBEDDING_DIM) + head.forEachIndexed { i, v -> out[i] = v } + return out + } + + private fun seedEmbedded( + title: String, + embedding: FloatArray, + scope: String = "personal", + ): KbNote { + val note = seedRaw(title = title, scope = scope) + val literal = embedding.joinToString(",", "[", "]") { it.toString() } + dsl.execute( + "UPDATE kb_notes SET embedding = ?::vector, embedding_model = 'test', embedded_at = NOW() WHERE id = ?", + literal, + note.id, + ) + return note + } + + private fun seedUnembedded(title: String): KbNote = seedRaw(title = title) + + private fun seedRaw( + title: String, + scope: String = "personal", + ): KbNote = + noteRepository.create( + KbNote( + id = Ulid.generate(), + type = KbNoteType.LESSON, + scope = scope, + source = "test", + capturedAt = Instant.now(), + sessionId = null, + confidence = 0.4, + title = title, + body = "body of $title", + vaultPath = "$scope/lesson/${title.replace(' ', '-')}.md", + vaultCommit = null, + tags = emptySet(), + ), + ) + + companion object { + private const val EMBEDDING_DIM = 1024 + private const val ZERO = 0.0f + private const val AXIS_MATCH = 1.0f + private const val MIDDLE_COMPONENT = 0.6f + private const val PERFECT_SCORE = 1.0 + private const val SCORE_EPSILON = 1e-6 + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallFlowIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallFlowIntegrationTest.kt index 2d6ea39..0322b13 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallFlowIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallFlowIntegrationTest.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") // Jackson 3 asText() — see McpTools file header. package com.jorisjonkers.personalstack.knowledge.recall @@ -27,264 +26,320 @@ import java.time.Instant "knowledge.mcp.tokens.ws=test-token-ws", ], ) -class RecallFlowIntegrationTest : IntegrationTestBase() { +class RecallFlowIntegrationTest @Autowired - private lateinit var context: WebApplicationContext - - @Autowired - private lateinit var mcpBearerFilter: McpBearerFilter - - @Autowired - private lateinit var noteRepository: NoteRepository - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .webAppContextSetup(context) - .addFilters(mcpBearerFilter) - .build() - } - - private fun seed( - title: String, - body: String, - scope: String = "personal", - type: KbNoteType = KbNoteType.LESSON, - tags: Set = emptySet(), - ): KbNote { - val note = - KbNote( - id = Ulid.generate(), - type = type, - scope = scope, - source = "test", - capturedAt = Instant.now(), - sessionId = null, - confidence = 0.4, - title = title, - body = body, - vaultPath = "$scope/${type.wire}/draft.md", - vaultCommit = null, - tags = tags, + constructor( + private val context: WebApplicationContext, + private val mcpBearerFilter: McpBearerFilter, + private val noteRepository: NoteRepository, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + + private val objectMapper = jacksonObjectMapper() + + @BeforeEach + fun setUp() { + mockMvc = + MockMvcBuilders + .webAppContextSetup(context) + .addFilters(mcpBearerFilter) + .build() + } + + @Test + fun getNoteReturnsTheSeededRowWithItsTags() { + val note = noteRepository.seedRecallNote("title", "body", tags = setOf("kotlin", "mcp")) + + val result = recallCall(mockMvc, objectMapper, "knowledge.get_note", mapOf("id" to note.id)) + val returned = recallToolResult(objectMapper, result)["note"] + + assertThat(returned["id"].asText()).isEqualTo(note.id) + assertThat(returned["title"].asText()).isEqualTo("title") + val tags = returned["tags"].map { it.asText() } + assertThat(tags).containsExactlyInAnyOrder("kotlin", "mcp") + } + + @Test + fun getNoteReturnsNullWhenNoRowMatches() { + val result = recallCall(mockMvc, objectMapper, "knowledge.get_note", mapOf("id" to Ulid.generate())) + val note = recallToolResult(objectMapper, result)["note"] + assertThat(note.isNull).isTrue + } + + @Test + fun listRecentReturnsRowsInULIDDescendingOrder() { + val a = noteRepository.seedRecallNote("oldest", "x") + Thread.sleep(LIST_SLEEP_MS) + val b = noteRepository.seedRecallNote("middle", "y") + Thread.sleep(LIST_SLEEP_MS) + val c = noteRepository.seedRecallNote("newest", "z") + + val result = + recallCall( + mockMvc, + objectMapper, + "knowledge.list_recent", + mapOf("limit" to RECENT_LIMIT, "scope" to "personal"), + ) + val ids = recallToolResult(objectMapper, result)["notes"].map { it["id"].asText() } + + assertThat(ids).containsExactly(c.id, b.id, a.id) + } + + @Test + fun listRecentHonoursTheTypeFilter() { + noteRepository.seedRecallNote("lesson1", "body", type = KbNoteType.LESSON) + noteRepository.seedRecallNote("decision1", "body", type = KbNoteType.DECISION) + noteRepository.seedRecallNote("lesson2", "body", type = KbNoteType.LESSON) + + val result = + recallCall( + mockMvc, + objectMapper, + "knowledge.list_recent", + mapOf("type" to "decision", "limit" to MAX_LIMIT), + ) + val titles = recallToolResult(objectMapper, result)["notes"].map { it["title"].asText() } + assertThat(titles).containsExactly("decision1") + } + + @Test + fun recallRanksTheTermRichRowHigherAndIncludesASnippet() { + val rocket = + noteRepository.seedRecallNote("rocket launch", "the rocket launched at dawn over the rocket field") + val unrelated = noteRepository.seedRecallNote("kitchen tips", "boil water and add salt to taste") + noteRepository.seedRecallNote("misc", "the dawn was beautiful, no rockets here") // sneaky single match + + val result = + recallCall( + mockMvc, + objectMapper, + "knowledge.recall", + mapOf("query" to "rocket", "limit" to RECALL_LIMIT), + ) + val hits = recallToolResult(objectMapper, result)["hits"] + val ids = hits.map { it["id"].asText() } + + // The "rocket"-heavy row wins; the kitchen-tips row mustn't appear. + assertThat(ids).contains(rocket.id) + assertThat(ids).doesNotContain(unrelated.id) + assertThat(hits[0]["id"].asText()).isEqualTo(rocket.id) + assertThat(hits[0]["snippet"].asText()).contains("rocket") + assertThat(hits[0]["score"].asDouble()).isGreaterThan(0.0) + } + + @Test + fun recallScopeFilterRestrictsResults() { + noteRepository.seedRecallNote("personal note", "rocket science", scope = "personal") + val work = noteRepository.seedRecallNote("work note", "rocket science", scope = "work") + + val result = + recallCall( + mockMvc, + objectMapper, + "knowledge.recall", + mapOf("query" to "rocket", "scope" to "work"), + ) + val ids = recallToolResult(objectMapper, result)["hits"].map { it["id"].asText() } + assertThat(ids).containsExactly(work.id) + } + + @Test + fun recallWithABlankQueryReturnsNoHitsWithoutErroring() { + noteRepository.seedRecallNote("anything", "rocket") + val result = recallCall(mockMvc, objectMapper, "knowledge.recall", mapOf("query" to " ")) + val hits = recallToolResult(objectMapper, result)["hits"] + assertThat(hits.size()).isZero + } + + @Test + fun findConflictsReturnsSupersedesPlusContradictsButSkipsMentions() { + val newer = noteRepository.seedRecallNote("v2", "v2 body") + val older = noteRepository.seedRecallNote("v1", "v1 body") + val unrelated = noteRepository.seedRecallNote("loose link", "body") + noteRepository.insertRelation( + KbRelation( + subjectId = newer.id, + predicate = "supersedes", + objectId = older.id, + props = emptyMap(), + createdAt = Instant.now(), + ), + ) + noteRepository.insertRelation( + KbRelation( + subjectId = older.id, + predicate = "contradicts", + objectId = unrelated.id, + props = mapOf("confidence_delta" to CONFIDENCE_DELTA), + createdAt = Instant.now(), + ), + ) + noteRepository.insertRelation( + KbRelation( + subjectId = newer.id, + predicate = "mentions", + objectId = unrelated.id, + props = emptyMap(), + createdAt = Instant.now(), + ), ) - return noteRepository.create(note) - } - - @Test - fun `get_note returns the seeded row with its tags`() { - val note = seed("title", "body", tags = setOf("kotlin", "mcp")) - - val result = call("knowledge.get_note", mapOf("id" to note.id)) - val returned = toolResult(result)["note"] - - assertThat(returned["id"].asText()).isEqualTo(note.id) - assertThat(returned["title"].asText()).isEqualTo("title") - val tags = returned["tags"].map { it.asText() } - assertThat(tags).containsExactlyInAnyOrder("kotlin", "mcp") - } - - @Test - fun `get_note returns null when no row matches`() { - val result = call("knowledge.get_note", mapOf("id" to Ulid.generate())) - val note = toolResult(result)["note"] - assertThat(note.isNull).isTrue - } - - @Test - fun `list_recent returns rows in ULID-descending order`() { - val a = seed("oldest", "x") - Thread.sleep(LIST_SLEEP_MS) - val b = seed("middle", "y") - Thread.sleep(LIST_SLEEP_MS) - val c = seed("newest", "z") - - val result = call("knowledge.list_recent", mapOf("limit" to 3, "scope" to "personal")) - val ids = toolResult(result)["notes"].map { it["id"].asText() } - - assertThat(ids).containsExactly(c.id, b.id, a.id) - } - - @Test - fun `list_recent honours the type filter`() { - seed("lesson1", "body", type = KbNoteType.LESSON) - seed("decision1", "body", type = KbNoteType.DECISION) - seed("lesson2", "body", type = KbNoteType.LESSON) - - val result = call("knowledge.list_recent", mapOf("type" to "decision", "limit" to 10)) - val titles = toolResult(result)["notes"].map { it["title"].asText() } - assertThat(titles).containsExactly("decision1") - } - - @Test - fun `recall ranks the term-rich row higher and includes a snippet`() { - val rocket = seed("rocket launch", "the rocket launched at dawn over the rocket field") - val unrelated = seed("kitchen tips", "boil water and add salt to taste") - seed("misc", "the dawn was beautiful, no rockets here") // sneaky single match - - val result = call("knowledge.recall", mapOf("query" to "rocket", "limit" to 5)) - val hits = toolResult(result)["hits"] - val ids = hits.map { it["id"].asText() } - - // The "rocket"-heavy row wins; the kitchen-tips row mustn't appear. - assertThat(ids).contains(rocket.id) - assertThat(ids).doesNotContain(unrelated.id) - assertThat(hits[0]["id"].asText()).isEqualTo(rocket.id) - assertThat(hits[0]["snippet"].asText()).contains("rocket") - assertThat(hits[0]["score"].asDouble()).isGreaterThan(0.0) - } - - @Test - fun `recall scope filter restricts results`() { - seed("personal note", "rocket science", scope = "personal") - val work = seed("work note", "rocket science", scope = "work") - - val result = call("knowledge.recall", mapOf("query" to "rocket", "scope" to "work")) - val ids = toolResult(result)["hits"].map { it["id"].asText() } - assertThat(ids).containsExactly(work.id) - } - - @Test - fun `recall with a blank query returns no hits without erroring`() { - seed("anything", "rocket") - val result = call("knowledge.recall", mapOf("query" to " ")) - val hits = toolResult(result)["hits"] - assertThat(hits.size()).isZero - } - - @Test - fun `find_conflicts returns supersedes plus contradicts but skips mentions`() { - val newer = seed("v2", "v2 body") - val older = seed("v1", "v1 body") - val unrelated = seed("loose link", "body") - noteRepository.insertRelation( - KbRelation( - subjectId = newer.id, - predicate = "supersedes", - objectId = older.id, - props = emptyMap(), - createdAt = Instant.now(), - ), - ) - noteRepository.insertRelation( - KbRelation( - subjectId = older.id, - predicate = "contradicts", - objectId = unrelated.id, - props = mapOf("confidence_delta" to -0.3), - createdAt = Instant.now(), - ), - ) - noteRepository.insertRelation( - KbRelation( - subjectId = newer.id, - predicate = "mentions", - objectId = unrelated.id, - props = emptyMap(), - createdAt = Instant.now(), - ), - ) - - val result = call("knowledge.find_conflicts", mapOf("id" to older.id)) - val rels = toolResult(result)["relations"] - val predicates = rels.map { it["predicate"].asText() }.toSet() - // older shows up twice — once as object of supersedes, once as - // subject of contradicts. mentions is filtered out. - assertThat(predicates).containsExactlyInAnyOrder("supersedes", "contradicts") - } + val result = recallCall(mockMvc, objectMapper, "knowledge.find_conflicts", mapOf("id" to older.id)) + val rels = recallToolResult(objectMapper, result)["relations"] + val predicates = rels.map { it["predicate"].asText() }.toSet() + + // older shows up twice — once as object of supersedes, once as + // subject of contradicts. mentions is filtered out. + assertThat(predicates).containsExactlyInAnyOrder("supersedes", "contradicts") + } + + @Test + fun toolsListNowAdvertisesTheFourReadToolsAlongsideTheCaptures() { + // Exercise one read tool first to confirm Spring composed the + // read-side registry; assert on tools/list separately. + recallCall(mockMvc, objectMapper, "knowledge.recall", mapOf("query" to "x")) + val names = + objectMapper.readTree(recallRpcRaw(mockMvc, objectMapper, "tools/list", null))["result"]["tools"].map { + it["name"].asText() + } + assertThat(names).contains( + "knowledge.recall", + "knowledge.get_note", + "knowledge.list_recent", + "knowledge.find_conflicts", + "knowledge.relations", + ) + } + + @Test + fun relationsWalksTheKbRelationsGraphUpToTheRequestedDepth() { + val a = noteRepository.seedRecallNote("root", "a") + val b = noteRepository.seedRecallNote("hop1", "b") + val c = noteRepository.seedRecallNote("hop2", "c") + noteRepository.insertRelation( + KbRelation( + subjectId = a.id, + predicate = "see_also", + objectId = b.id, + props = emptyMap(), + createdAt = Instant.now(), + ), + ) + noteRepository.insertRelation( + KbRelation( + subjectId = b.id, + predicate = "supersedes", + objectId = c.id, + props = emptyMap(), + createdAt = Instant.now(), + ), + ) - @Test - fun `tools_list now advertises the four read tools alongside the captures`() { - // Exercise one read tool first to confirm Spring composed the - // read-side registry; assert on tools/list separately. - call("knowledge.recall", mapOf("query" to "x")) - val names = - objectMapper.readTree(rpcRaw("tools/list", null))["result"]["tools"].map { - it["name"].asText() - } - assertThat(names).contains( - "knowledge.recall", - "knowledge.get_note", - "knowledge.list_recent", - "knowledge.find_conflicts", - "knowledge.relations", - ) + // depth=1 reaches the direct neighbour only. + val depth1 = + recallToolResult( + objectMapper, + recallCall( + mockMvc, + objectMapper, + "knowledge.relations", + mapOf( + "id" to a.id, + "depth" to 1, + ), + ), + ) + val one = depth1["relations"].map { it["object_id"].asText() }.toSet() + assertThat(one).containsExactly(b.id) + + // depth=2 sees the b → c hop too. + val depth2 = + recallToolResult( + objectMapper, + recallCall( + mockMvc, + objectMapper, + "knowledge.relations", + mapOf( + "id" to a.id, + "depth" to 2, + ), + ), + ) + val two = depth2["relations"].map { it["object_id"].asText() }.toSet() + assertThat(two).containsExactlyInAnyOrder(b.id, c.id) + } + + companion object { + private const val LIST_SLEEP_MS = 3L + private const val RECENT_LIMIT = 3 + private const val MAX_LIMIT = 10 + private const val RECALL_LIMIT = 5 + private const val CONFIDENCE_DELTA = -0.3 + } } - @Test - fun `relations walks the kb_relations graph up to the requested depth`() { - val a = seed("root", "a") - val b = seed("hop1", "b") - val c = seed("hop2", "c") - noteRepository.insertRelation( - KbRelation( - subjectId = a.id, - predicate = "see_also", - objectId = b.id, - props = emptyMap(), - createdAt = Instant.now(), - ), - ) - noteRepository.insertRelation( - KbRelation( - subjectId = b.id, - predicate = "supersedes", - objectId = c.id, - props = emptyMap(), - createdAt = Instant.now(), - ), +private fun NoteRepository.seedRecallNote( + title: String, + body: String, + scope: String = "personal", + type: KbNoteType = KbNoteType.LESSON, + tags: Set = emptySet(), +): KbNote { + val note = + KbNote( + id = Ulid.generate(), + type = type, + scope = scope, + source = "test", + capturedAt = Instant.now(), + sessionId = null, + confidence = 0.4, + title = title, + body = body, + vaultPath = "$scope/${type.wire}/draft.md", + vaultCommit = null, + tags = tags, ) + return create(note) +} - // depth=1 reaches the direct neighbour only. - val depth1 = toolResult(call("knowledge.relations", mapOf("id" to a.id, "depth" to 1))) - val one = depth1["relations"].map { it["object_id"].asText() }.toSet() - assertThat(one).containsExactly(b.id) - - // depth=2 sees the b → c hop too. - val depth2 = toolResult(call("knowledge.relations", mapOf("id" to a.id, "depth" to 2))) - val two = depth2["relations"].map { it["object_id"].asText() }.toSet() - assertThat(two).containsExactlyInAnyOrder(b.id, c.id) - } - - private fun call( - name: String, - arguments: Map, - ): String = rpcRaw("tools/call", mapOf("name" to name, "arguments" to arguments)) - - // Unwrap the MCP `tools/call` envelope: every result is now a - // `CallToolResult` with `{content, structuredContent, isError}` per - // MCP spec 2025-06-18. Tests assert on the domain shape sitting in - // `structuredContent`. - private fun toolResult(rawJson: String) = objectMapper.readTree(rawJson)["result"]["structuredContent"] - - private fun rpcRaw( - method: String, - params: Map?, - ): String { - val body = - buildMap { - put("jsonrpc", "2.0") - put("id", 1) - put("method", method) - if (params != null) put("params", params) - } - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = objectMapper.writeValueAsBytes(body) - }.andReturn() - assertThat(result.response.status).isEqualTo(200) - return result.response.contentAsString - } - - companion object { - private const val LIST_SLEEP_MS = 3L - } +private fun recallCall( + mockMvc: MockMvc, + objectMapper: com.fasterxml.jackson.databind.ObjectMapper, + name: String, + arguments: Map, +): String = recallRpcRaw(mockMvc, objectMapper, "tools/call", mapOf("name" to name, "arguments" to arguments)) + +private fun recallToolResult( + objectMapper: com.fasterxml.jackson.databind.ObjectMapper, + rawJson: String, +) = objectMapper.readTree(rawJson)["result"]["structuredContent"] + +private fun recallRpcRaw( + mockMvc: MockMvc, + objectMapper: com.fasterxml.jackson.databind.ObjectMapper, + method: String, + params: Map?, +): String { + val body = + buildMap { + put("jsonrpc", "2.0") + put("id", "test-request") + put("method", method) + if (params != null) put("params", params) + } + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = objectMapper.writeValueAsBytes(body) + }.andReturn() + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), + ) + return result.response.contentAsString } diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallMigrationIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallMigrationIntegrationTest.kt index fafc4b6..f0b5a2b 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallMigrationIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallMigrationIntegrationTest.kt @@ -1,5 +1,3 @@ -@file:Suppress("VarCouldBeVal") - package com.jorisjonkers.personalstack.knowledge.recall import com.jorisjonkers.personalstack.knowledge.IntegrationTestBase @@ -8,15 +6,16 @@ import org.jooq.DSLContext import org.junit.jupiter.api.Test import org.springframework.beans.factory.annotation.Autowired -class RecallMigrationIntegrationTest : IntegrationTestBase() { +class RecallMigrationIntegrationTest @Autowired - private lateinit var dsl: DSLContext - - @Test - fun postgresMigrationCreatesRecallFtsIndex() { - val indexName = - dsl.fetchValue("SELECT to_regclass('public.kb_notes_fts_english_idx')::text", String::class.java) + constructor( + private val dsl: DSLContext, + ) : IntegrationTestBase() { + @Test + fun postgresMigrationCreatesRecallFtsIndex() { + val indexName = + dsl.fetchValue("SELECT to_regclass('public.kb_notes_fts_english_idx')::text", String::class.java) - assertThat(indexName).isEqualTo("kb_notes_fts_english_idx") + assertThat(indexName).isEqualTo("kb_notes_fts_english_idx") + } } -} diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/review/ReviewFlowIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/review/ReviewFlowIntegrationTest.kt index 75127f1..613db1a 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/review/ReviewFlowIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/review/ReviewFlowIntegrationTest.kt @@ -28,147 +28,154 @@ import java.time.ZoneOffset "knowledge.mcp.tokens.ws=test-token-ws", ], ) -class ReviewFlowIntegrationTest : IntegrationTestBase() { +class ReviewFlowIntegrationTest @Autowired - private lateinit var context: WebApplicationContext - - @Autowired - private lateinit var mcpBearerFilter: McpBearerFilter - - @Autowired - private lateinit var noteRepository: NoteRepository - - @Autowired - private lateinit var dsl: DSLContext - - private lateinit var mockMvc: MockMvc - - private val objectMapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = - MockMvcBuilders - .webAppContextSetup(context) - .addFilters(mcpBearerFilter) - .build() - } - - @Test - fun `review_summary returns bounded governance buckets and advisory suggestions`() { - val inbox = seed("plain inbox", scope = "_inbox", vaultPath = "_inbox/2026-06-04/plain.md") - val needsReview = seed("needs review", scope = "_inbox", vaultPath = "_inbox/_needs-review/note.md") - val wildcardLookalike = seed("wildcard lookalike", scope = "_inbox", vaultPath = "xinbox/xneeds-review/note.md") - seed("recent auto", scope = "project:personal-stack", source = "assistant-ui:auto-capture:s1") - seed( - "stale old note", - scope = "project:personal-stack", - capturedAt = Instant.parse("2026-01-01T00:00:00Z"), - ) - val lowConfidence = + constructor( + private val context: WebApplicationContext, + private val mcpBearerFilter: McpBearerFilter, + private val noteRepository: NoteRepository, + private val dsl: DSLContext, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + + private val objectMapper = jacksonObjectMapper() + + @BeforeEach + fun setUp() { + mockMvc = + MockMvcBuilders + .webAppContextSetup(context) + .addFilters(mcpBearerFilter) + .build() + } + + @Test + fun reviewSummaryReturnsBoundedGovernanceBucketsAndAdvisorySuggestions() { + val inbox = + seed(ReviewSeed("plain inbox", scope = "_inbox", vaultPath = "_inbox/2026-06-04/plain.md")) + val needsReview = + seed(ReviewSeed("needs review", scope = "_inbox", vaultPath = "_inbox/_needs-review/note.md")) + val wildcardLookalike = + seed(ReviewSeed("wildcard lookalike", scope = "_inbox", vaultPath = "xinbox/xneeds-review/note.md")) + seed(ReviewSeed("recent auto", scope = "project:personal-stack", source = "assistant-ui:auto-capture:s1")) seed( - "low confidence useful note", - scope = "project:personal-stack", - confidence = 0.4, - ) - bumpRecall(lowConfidence.id, count = 5) - - val out = - toolResult( - call( - "knowledge.review_summary", - mapOf( - "limit" to 10, - "stale_days" to 30, - "high_recall_min" to 3, - "tag_max_tags" to 1, - ), + ReviewSeed( + "stale old note", + scope = "project:personal-stack", + capturedAt = Instant.parse("2026-01-01T00:00:00Z"), ), ) - - val summary = out["summary"] - assertThat(summary["needs_review"]["total"].asInt()).isEqualTo(1) - val needsReviewIds = summary["needs_review"]["items"].map { it["id"].asText() } - assertThat(needsReviewIds).contains(needsReview.id) - assertThat(needsReviewIds).doesNotContain(wildcardLookalike.id) - assertThat(summary["inbox"]["total"].asInt()).isEqualTo(2) - val inboxIds = summary["inbox"]["items"].map { it["id"].asText() } - assertThat(inboxIds).contains(inbox.id, wildcardLookalike.id) - assertThat(inboxIds).doesNotContain(needsReview.id) - - val autoSources = summary["recent_auto_captures"]["items"].map { it["source"].asText() } - assertThat(autoSources).contains("assistant-ui:auto-capture:s1") - - val lowConfidenceIds = summary["low_confidence_high_recall"]["items"].map { it["id"].asText() } - assertThat(lowConfidenceIds).contains(lowConfidence.id) - - val suggestionKinds = summary["suggestions"].map { it["kind"].asText() } - assertThat(suggestionKinds).contains("needs_review", "low_confidence_high_recall") - } - - private fun seed( - title: String, - scope: String, - source: String = "test", - confidence: Double = 0.8, - capturedAt: Instant = Instant.now(), - vaultPath: String = "$scope/lesson/${title.replace(' ', '-')}.md", - ): KbNote { - val note = - KbNote( - id = Ulid.generate(), - type = KbNoteType.LESSON, - scope = scope, - source = source, - capturedAt = capturedAt, - sessionId = null, - confidence = confidence, - title = title, - body = "body for $title", - vaultPath = vaultPath, - vaultCommit = null, + val lowConfidence = + seed( + ReviewSeed( + "low confidence useful note", + scope = "project:personal-stack", + confidence = 0.4, + ), + ) + bumpRecall(lowConfidence.id, count = 5) + + val out = + toolResult( + call( + "knowledge.review_summary", + mapOf( + "limit" to 10, + "stale_days" to 30, + "high_recall_min" to 3, + "tag_max_tags" to 1, + ), + ), + ) + + val summary = out["summary"] + assertThat(summary["needs_review"]["total"].asInt()).isEqualTo(1) + val needsReviewIds = summary["needs_review"]["items"].map { it["id"].asText() } + assertThat(needsReviewIds).contains(needsReview.id) + assertThat(needsReviewIds).doesNotContain(wildcardLookalike.id) + assertThat(summary["inbox"]["total"].asInt()).isEqualTo(2) + val inboxIds = summary["inbox"]["items"].map { it["id"].asText() } + assertThat(inboxIds).contains(inbox.id, wildcardLookalike.id) + assertThat(inboxIds).doesNotContain(needsReview.id) + + val autoSources = summary["recent_auto_captures"]["items"].map { it["source"].asText() } + assertThat(autoSources).contains("assistant-ui:auto-capture:s1") + + val lowConfidenceIds = summary["low_confidence_high_recall"]["items"].map { it["id"].asText() } + assertThat(lowConfidenceIds).contains(lowConfidence.id) + + val suggestionKinds = summary["suggestions"].map { it["kind"].asText() } + assertThat(suggestionKinds).contains("needs_review", "low_confidence_high_recall") + } + + private fun seed(seed: ReviewSeed): KbNote { + val note = + KbNote( + id = Ulid.generate(), + type = KbNoteType.LESSON, + scope = seed.scope, + source = seed.source, + capturedAt = seed.capturedAt, + sessionId = null, + confidence = seed.confidence, + title = seed.title, + body = "body for ${seed.title}", + vaultPath = seed.vaultPath, + vaultCommit = null, + ) + return noteRepository.create(note) + } + + private fun bumpRecall( + id: String, + count: Int, + ) { + dsl + .update(KB_NOTES) + .set(KB_NOTES.RECALL_COUNT, count) + .set(KB_NOTES.LAST_RECALLED_AT, LocalDateTime.ofInstant(Instant.now(), ZoneOffset.UTC)) + .where(KB_NOTES.ID.eq(id)) + .execute() + } + + private fun call( + name: String, + arguments: Map, + ): String = rpcRaw("tools/call", mapOf("name" to name, "arguments" to arguments)) + + private fun toolResult(rawJson: String) = objectMapper.readTree(rawJson)["result"]["structuredContent"] + + private fun rpcRaw( + method: String, + params: Map?, + ): String { + val body = + buildMap { + put("jsonrpc", "2.0") + put("id", 1) + put("method", method) + if (params != null) put("params", params) + } + val result = + mockMvc + .post("/mcp") { + contentType = MediaType.APPLICATION_JSON + header("Authorization", "Bearer test-token-ws") + content = objectMapper.writeValueAsBytes(body) + }.andReturn() + assertThat(result.response.status).isEqualTo( + org.springframework.http.HttpStatus.OK + .value(), ) - return noteRepository.create(note) - } - - private fun bumpRecall( - id: String, - count: Int, - ) { - dsl - .update(KB_NOTES) - .set(KB_NOTES.RECALL_COUNT, count) - .set(KB_NOTES.LAST_RECALLED_AT, LocalDateTime.ofInstant(Instant.now(), ZoneOffset.UTC)) - .where(KB_NOTES.ID.eq(id)) - .execute() + return result.response.contentAsString + } } - private fun call( - name: String, - arguments: Map, - ): String = rpcRaw("tools/call", mapOf("name" to name, "arguments" to arguments)) - - private fun toolResult(rawJson: String) = objectMapper.readTree(rawJson)["result"]["structuredContent"] - - private fun rpcRaw( - method: String, - params: Map?, - ): String { - val body = - buildMap { - put("jsonrpc", "2.0") - put("id", 1) - put("method", method) - if (params != null) put("params", params) - } - val result = - mockMvc - .post("/mcp") { - contentType = MediaType.APPLICATION_JSON - header("Authorization", "Bearer test-token-ws") - content = objectMapper.writeValueAsBytes(body) - }.andReturn() - assertThat(result.response.status).isEqualTo(200) - return result.response.contentAsString - } -} +private data class ReviewSeed( + val title: String, + val scope: String, + val source: String = "test", + val confidence: Double = 0.8, + val capturedAt: Instant = Instant.now(), + val vaultPath: String = "$scope/lesson/${title.replace(' ', '-')}.md", +) diff --git a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeRestIntegrationTest.kt b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeRestIntegrationTest.kt index 4b4328d..2c04e93 100644 --- a/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeRestIntegrationTest.kt +++ b/api/src/integrationTest/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeRestIntegrationTest.kt @@ -5,6 +5,7 @@ import com.jorisjonkers.personalstack.knowledge.IntegrationTestBase import com.jorisjonkers.personalstack.knowledge.domain.KbNote import com.jorisjonkers.personalstack.knowledge.domain.KbNoteType import com.jorisjonkers.personalstack.knowledge.domain.Ulid +import com.jorisjonkers.personalstack.knowledge.repo.AuditRecordRequest import com.jorisjonkers.personalstack.knowledge.repo.AuditRepository import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository import org.assertj.core.api.Assertions.assertThat @@ -28,168 +29,172 @@ import java.time.Instant * any path outside `/mcp`), so a MockMvc setup without the filter * mirrors the real reachability surface. */ -class KnowledgeRestIntegrationTest : IntegrationTestBase() { +class KnowledgeRestIntegrationTest @Autowired - private lateinit var context: WebApplicationContext + constructor( + private val context: WebApplicationContext, + private val noteRepository: NoteRepository, + private val auditRepository: AuditRepository, + ) : IntegrationTestBase() { + private lateinit var mockMvc: MockMvc + private val mapper = jacksonObjectMapper() + + @BeforeEach + fun setUp() { + mockMvc = MockMvcBuilders.webAppContextSetup(context).build() + } + + private fun seed( + title: String, + scope: String, + type: KbNoteType = KbNoteType.LESSON, + tags: Set = emptySet(), + ): KbNote { + val note = + KbNote( + id = Ulid.generate(), + type = type, + scope = scope, + source = "test", + capturedAt = Instant.now(), + sessionId = null, + confidence = 0.4, + title = title, + body = "body for $title", + vaultPath = "$scope/${type.wire}/${title.replace(' ', '-')}.md", + vaultCommit = null, + tags = tags, + ) + return noteRepository.create(note) + } + + @Test + fun getRecentReturnsNotesOrderedMostRecentFirstWithTheSnakeCaseWireShape() { + seed("oldest", scope = "personal") + seed("middle", scope = "personal") + seed("newest", scope = "personal") + + val raw = + mockMvc + .get("/api/v1/knowledge/recent?limit=10") + .andReturn() + .response + .contentAsString + val tree = mapper.readTree(raw) + val titles = tree["notes"].map { it["title"].asText() } + assertThat(titles).containsExactly("newest", "middle", "oldest") + val first = tree["notes"][0] + // snake_case keys preserved across the projection + assertThat(first.has("captured_at")).isTrue() + assertThat(first.has("vault_path")).isTrue() + assertThat(first.has("session_id")).isTrue() + } + + @Test + fun getNoteByIdReturnsNotFoundOnMissAndOkWithTheNotePayloadOnHit() { + val note = seed("present", scope = "personal", tags = setOf("kotlin", "mcp")) - @Autowired - private lateinit var noteRepository: NoteRepository - - @Autowired - private lateinit var auditRepository: AuditRepository - - private lateinit var mockMvc: MockMvc - private val mapper = jacksonObjectMapper() - - @BeforeEach - fun setUp() { - mockMvc = MockMvcBuilders.webAppContextSetup(context).build() - } - - private fun seed( - title: String, - scope: String, - type: KbNoteType = KbNoteType.LESSON, - tags: Set = emptySet(), - ): KbNote { - val note = - KbNote( - id = Ulid.generate(), - type = type, - scope = scope, - source = "test", - capturedAt = Instant.now(), - sessionId = null, - confidence = 0.4, - title = title, - body = "body for $title", - vaultPath = "$scope/${type.wire}/${title.replace(' ', '-')}.md", - vaultCommit = null, - tags = tags, - ) - return noteRepository.create(note) - } - - @Test - fun `GET recent returns notes ordered most-recent-first with the snake_case wire shape`() { - seed("oldest", scope = "personal") - seed("middle", scope = "personal") - seed("newest", scope = "personal") - - val raw = mockMvc - .get("/api/v1/knowledge/recent?limit=10") + .get("/api/v1/knowledge/notes/missing-id") .andReturn() .response - .contentAsString - val tree = mapper.readTree(raw) - val titles = tree["notes"].map { it["title"].asText() } - assertThat(titles).containsExactly("newest", "middle", "oldest") - val first = tree["notes"][0] - // snake_case keys preserved across the projection - assertThat(first.has("captured_at")).isTrue() - assertThat(first.has("vault_path")).isTrue() - assertThat(first.has("session_id")).isTrue() - } - - @Test - fun `GET note by id returns 404 on miss and 200 with the note payload on hit`() { - val note = seed("present", scope = "personal", tags = setOf("kotlin", "mcp")) - - mockMvc - .get("/api/v1/knowledge/notes/missing-id") - .andReturn() - .response - .status - .let { assertThat(it).isEqualTo(404) } - - val raw = - mockMvc - .get("/api/v1/knowledge/notes/${note.id}") - .andReturn() - .response - .contentAsString - val tree = mapper.readTree(raw) - assertThat(tree["id"].asText()).isEqualTo(note.id) - assertThat(tree["tags"].map { it.asText() }).containsExactly("kotlin", "mcp") - } - - @Test - fun `GET recall surfaces FTS hits and reports the resolved mode`() { - seed("rocket launch", scope = "personal") - seed("kitchen tips", scope = "personal") - - val raw = - mockMvc - .get("/api/v1/knowledge/recall?query=rocket&limit=5") - .andReturn() - .response - .contentAsString - val tree = mapper.readTree(raw) - assertThat(tree["mode"].asText()).isIn("fast", "hybrid", "deep") - val titles = tree["hits"].map { it["title"].asText() } - assertThat(titles).contains("rocket launch") - assertThat(titles).doesNotContain("kitchen tips") - } - - @Test - fun `GET inbox surfaces unclassified notes only`() { - seed("inboxed", scope = "_inbox") - seed("promoted", scope = "topic:kotlin") - - val raw = - mockMvc - .get("/api/v1/knowledge/inbox?limit=10") - .andReturn() - .response - .contentAsString - val tree = mapper.readTree(raw) - val titles = tree["notes"].map { it["title"].asText() } - assertThat(titles).containsExactly("inboxed") - } - - @Test - fun `GET topics returns slugs in use with counts`() { - seed("k1", scope = "topic:kotlin") - seed("k2", scope = "topic:kotlin") - seed("p1", scope = "topic:postgres") - - val raw = - mockMvc - .get("/api/v1/knowledge/topics?limit=10") - .andReturn() - .response - .contentAsString - val tree = mapper.readTree(raw) - val slugs = tree["topics"].map { it["slug"].asText() } - assertThat(slugs).contains("kotlin", "postgres") - } + .status + .let { + assertThat(it).isEqualTo( + org.springframework.http.HttpStatus.NOT_FOUND + .value(), + ) + } + + val raw = + mockMvc + .get("/api/v1/knowledge/notes/${note.id}") + .andReturn() + .response + .contentAsString + val tree = mapper.readTree(raw) + assertThat(tree["id"].asText()).isEqualTo(note.id) + assertThat(tree["tags"].map { it.asText() }).containsExactly("kotlin", "mcp") + } + + @Test + fun getRecallSurfacesFTSHitsAndReportsTheResolvedMode() { + seed("rocket launch", scope = "personal") + seed("kitchen tips", scope = "personal") + + val raw = + mockMvc + .get("/api/v1/knowledge/recall?query=rocket&limit=5") + .andReturn() + .response + .contentAsString + val tree = mapper.readTree(raw) + assertThat(tree["mode"].asText()).isIn("fast", "hybrid", "deep") + val titles = tree["hits"].map { it["title"].asText() } + assertThat(titles).contains("rocket launch") + assertThat(titles).doesNotContain("kitchen tips") + } + + @Test + fun getInboxSurfacesUnclassifiedNotesOnly() { + seed("inboxed", scope = "_inbox") + seed("promoted", scope = "topic:kotlin") + + val raw = + mockMvc + .get("/api/v1/knowledge/inbox?limit=10") + .andReturn() + .response + .contentAsString + val tree = mapper.readTree(raw) + val titles = tree["notes"].map { it["title"].asText() } + assertThat(titles).containsExactly("inboxed") + } + + @Test + fun getTopicsReturnsSlugsInUseWithCounts() { + seed("k1", scope = "topic:kotlin") + seed("k2", scope = "topic:kotlin") + seed("p1", scope = "topic:postgres") + + val raw = + mockMvc + .get("/api/v1/knowledge/topics?limit=10") + .andReturn() + .response + .contentAsString + val tree = mapper.readTree(raw) + val slugs = tree["topics"].map { it["slug"].asText() } + assertThat(slugs).contains("kotlin", "postgres") + } + + @Test + fun getAuditListsRowsWithSnakeCaseKeys() { + auditRepository.record( + AuditRecordRequest( + actor = "kb-test", + action = "test_action", + targetId = "target-1", + targetKind = "note", + beforeJson = """{"a":1}""", + afterJson = """{"a":2}""", + ), + ) - @Test - fun `GET audit lists rows with snake_case keys`() { - auditRepository.record( - actor = "kb-test", - action = "test_action", - targetId = "target-1", - targetKind = "note", - beforeJson = """{"a":1}""", - afterJson = """{"a":2}""", - ) - - val raw = - mockMvc - .get("/api/v1/knowledge/audit?actor=kb-test&limit=10") - .andReturn() - .response - .contentAsString - val tree = mapper.readTree(raw) - val rows = tree["rows"] - assertThat(rows).isNotEmpty - val row = rows[0] - assertThat(row["actor"].asText()).isEqualTo("kb-test") - assertThat(row["target_id"].asText()).isEqualTo("target-1") - assertThat(row["target_kind"].asText()).isEqualTo("note") - assertThat(row.has("before_json")).isTrue() - assertThat(row.has("after_json")).isTrue() + val raw = + mockMvc + .get("/api/v1/knowledge/audit?actor=kb-test&limit=10") + .andReturn() + .response + .contentAsString + val tree = mapper.readTree(raw) + val rows = tree["rows"] + assertThat(rows).isNotEmpty + val row = rows[0] + assertThat(row["actor"].asText()).isEqualTo("kb-test") + assertThat(row["target_id"].asText()).isEqualTo("target-1") + assertThat(row["target_kind"].asText()).isEqualTo("note") + assertThat(row.has("before_json")).isTrue() + assertThat(row.has("after_json")).isTrue() + } } -} diff --git a/api/src/main/java/com/jorisjonkers/personalstack/knowledge/repo/JooqQueryBindsSupport.java b/api/src/main/java/com/jorisjonkers/personalstack/knowledge/repo/JooqQueryBindsSupport.java new file mode 100644 index 0000000..43fc680 --- /dev/null +++ b/api/src/main/java/com/jorisjonkers/personalstack/knowledge/repo/JooqQueryBindsSupport.java @@ -0,0 +1,18 @@ +package com.jorisjonkers.personalstack.knowledge.repo; + +import java.util.List; +import org.jooq.DSLContext; +import org.jooq.ResultQuery; + +final class JooqQueryBindsSupport { + private JooqQueryBindsSupport() { + } + + static ResultQuery resultQueryWithBinds( + DSLContext dsl, + String sql, + List binds + ) { + return dsl.resultQuery(sql, binds.toArray()); + } +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/KnowledgeApiApplication.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/KnowledgeApiApplication.kt index 6c29efe..d42e439 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/KnowledgeApiApplication.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/KnowledgeApiApplication.kt @@ -3,9 +3,9 @@ package com.jorisjonkers.personalstack.knowledge import com.jorisjonkers.personalstack.knowledge.auth.McpBearerFilter import com.jorisjonkers.personalstack.knowledge.auth.McpBearerProperties import com.jorisjonkers.personalstack.knowledge.mcp.KnowledgeModeProperties +import org.springframework.boot.SpringApplication import org.springframework.boot.autoconfigure.SpringBootApplication import org.springframework.boot.context.properties.EnableConfigurationProperties -import org.springframework.boot.runApplication import org.springframework.boot.web.servlet.FilterRegistrationBean import org.springframework.context.annotation.Bean import org.springframework.scheduling.annotation.EnableScheduling @@ -49,5 +49,5 @@ class KnowledgeApiApplication { } fun main(args: Array) { - runApplication(*args) + SpringApplication.run(arrayOf(KnowledgeApiApplication::class.java), args) } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/auth/AdminAuthorization.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/auth/AdminAuthorization.kt index 9afd87d..3b3ed90 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/auth/AdminAuthorization.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/auth/AdminAuthorization.kt @@ -49,8 +49,7 @@ class AdminAuthorization( ?: denied("admin tools require a resolved bearer token") } - @Suppress("NOTHING_TO_INLINE") - private inline fun denied(message: String): Nothing = throw McpAuthorizationError(message) + private fun denied(message: String): Nothing = throw McpAuthorizationError(message) } /** diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestService.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestService.kt index 38f8e5e..e055a7e 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestService.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestService.kt @@ -5,6 +5,9 @@ import com.jorisjonkers.personalstack.knowledge.repo.DiscoveryRepository import com.jorisjonkers.personalstack.knowledge.repo.TopicRepository import org.slf4j.LoggerFactory import org.springframework.stereotype.Service +import org.springframework.web.client.RestClientException +import tools.jackson.core.JacksonException +import tools.jackson.databind.JsonNode /** * Reflexion-style session-digest service. Given a session @@ -38,38 +41,61 @@ class DigestService( ) { private val log = LoggerFactory.getLogger(DigestService::class.java) - @Suppress("ReturnCount", "TooGenericExceptionCaught") fun digest( transcript: String, maxCandidates: Int = DEFAULT_MAX_CANDIDATES, minConfidence: Double = DEFAULT_MIN_CONFIDENCE, + ): List = + if (transcript.isBlank()) { + emptyList() + } else { + digestTranscript(transcript, maxCandidates, minConfidence) + } + + private fun digestTranscript( + transcript: String, + maxCandidates: Int, + minConfidence: Double, ): List { - if (transcript.isBlank()) return emptyList() val cappedMax = maxCandidates.coerceIn(1, ABSOLUTE_MAX_CANDIDATES) val topicSlugs = topicRepository.listActive().map { it.slug } val knownTags = discoveryRepository.listTags(scope = null, limit = TAG_VOCAB_FOR_PROMPT).map { it.tag } - val rawJson = - try { - ollama.chatJson( - systemPrompt = systemPrompt(topicSlugs, knownTags, cappedMax), - userPrompt = userPrompt(transcript), - responseSchema = responseSchema(cappedMax), - ) - } catch (exc: RuntimeException) { - log.warn("digest.ollama_failed", exc) - return emptyList() - } - val parsed = - rawJson.path("candidates").takeIf { it.isArray } - ?: rawJson.takeIf { it.isArray } - ?: return emptyList() + val parsed = requestDigest(transcript, topicSlugs, knownTags, cappedMax)?.candidateArray() return parsed - .mapNotNull(::parseCandidate) - .filter { it.confidence >= minConfidence } - .take(cappedMax) + ?.mapNotNull(::parseCandidate) + ?.filter { it.confidence >= minConfidence } + ?.take(cappedMax) + .orEmpty() } - private fun parseCandidate(node: tools.jackson.databind.JsonNode): DigestCandidate? { + private fun requestDigest( + transcript: String, + topicSlugs: List, + knownTags: List, + cappedMax: Int, + ): JsonNode? = + try { + ollama.chatJson( + systemPrompt = systemPrompt(topicSlugs, knownTags, cappedMax), + userPrompt = userPrompt(transcript), + responseSchema = responseSchema(cappedMax), + ) + } catch (exc: RestClientException) { + log.warn("digest.ollama_failed", exc) + null + } catch (exc: JacksonException) { + log.warn("digest.ollama_invalid_json", exc) + null + } catch (exc: IllegalStateException) { + log.warn("digest.ollama_incomplete_response", exc) + null + } + + private fun JsonNode.candidateArray(): JsonNode? = + path("candidates").takeIf { it.isArray } + ?: takeIf { it.isArray } + + private fun parseCandidate(node: JsonNode): DigestCandidate? { val kind = node .path("kind") @@ -94,7 +120,7 @@ class DigestService( } private fun parseStringArray( - node: tools.jackson.databind.JsonNode, + node: JsonNode, field: String, cap: Int, ): List = diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryService.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryService.kt index d9bee82..043901c 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryService.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/DiscoveryService.kt @@ -9,9 +9,11 @@ import com.jorisjonkers.personalstack.knowledge.domain.TagSummary import com.jorisjonkers.personalstack.knowledge.domain.TopicStats import com.jorisjonkers.personalstack.knowledge.domain.TopicSummary import com.jorisjonkers.personalstack.knowledge.recall.QueryEmbedder +import com.jorisjonkers.personalstack.knowledge.recall.QueryEmbeddingException import com.jorisjonkers.personalstack.knowledge.repo.DiscoveryRepository import com.jorisjonkers.personalstack.knowledge.repo.EmbeddingRepository import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository +import org.jooq.exception.DataAccessException import org.slf4j.LoggerFactory import org.springframework.stereotype.Service @@ -68,7 +70,6 @@ class DiscoveryService( * no graceful FTS fallback for this tool because the FTS path * doesn't compute topic centroids. */ - @Suppress("TooGenericExceptionCaught") fun suggestTopic( text: String, limit: Int, @@ -77,13 +78,20 @@ class DiscoveryService( return try { val embedding = queryEmbedder.embed(text) embeddingRepository.suggestTopic(embedding, limit) - } catch (ex: RuntimeException) { + } catch (ex: QueryEmbeddingException) { log.warn( "suggest_topic degraded: embedder failed (text.len={})", text.length, ex, ) emptyList() + } catch (ex: DataAccessException) { + log.warn( + "suggest_topic degraded: repository failed (text.len={})", + text.length, + ex, + ) + emptyList() } } @@ -94,7 +102,6 @@ class DiscoveryService( * note id (post-capture audit) — the id path uses the row's * persisted embedding to skip re-embedding. */ - @Suppress("TooGenericExceptionCaught") fun findDuplicates( text: String, threshold: Double, @@ -104,9 +111,12 @@ class DiscoveryService( return try { val embedding = queryEmbedder.embed(text) embeddingRepository.findDuplicates(embedding, threshold, limit) - } catch (ex: RuntimeException) { + } catch (ex: QueryEmbeddingException) { log.warn("find_duplicates degraded: embedder failed (text.len={})", text.length, ex) emptyList() + } catch (ex: DataAccessException) { + log.warn("find_duplicates degraded: repository failed (text.len={})", text.length, ex) + emptyList() } } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/TagClusterService.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/TagClusterService.kt index 768d865..cb7dbb6 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/TagClusterService.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/discovery/TagClusterService.kt @@ -3,6 +3,7 @@ package com.jorisjonkers.personalstack.knowledge.discovery import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateCluster import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateMember import com.jorisjonkers.personalstack.knowledge.recall.QueryEmbedder +import com.jorisjonkers.personalstack.knowledge.recall.QueryEmbeddingException import com.jorisjonkers.personalstack.knowledge.repo.DiscoveryRepository import org.slf4j.LoggerFactory import org.springframework.stereotype.Service @@ -33,28 +34,39 @@ class TagClusterService( ) { private val log = LoggerFactory.getLogger(TagClusterService::class.java) - @Suppress("ReturnCount", "TooGenericExceptionCaught") fun listTagCandidates( minCount: Int, threshold: Double, maxTags: Int, - ): List { - val rawTags = - discoveryRepository - .listTags(scope = null, limit = maxTags) - .filter { it.count >= minCount } - if (rawTags.size < 2) return emptyList() + ): List = + candidateTags(minCount, maxTags) + .takeIf { it.size >= MIN_CLUSTER_SIZE } + ?.let { rawTags -> + val embeddings = embeddingsFor(rawTags.map { it.tag }) + val countByTag = rawTags.associate { it.tag to it.count } + embeddings + .takeIf { it.isNotEmpty() } + ?.let { cluster(it, countByTag, threshold) } + .orEmpty() + }.orEmpty() - val embeddings: Map = - try { - embedAll(rawTags.map { it.tag }) - } catch (ex: RuntimeException) { - log.warn("list_tag_candidates degraded: embedder failed", ex) - return emptyList() - } - val countByTag = rawTags.associate { it.tag to it.count } - return cluster(embeddings, countByTag, threshold) - } + private fun candidateTags( + minCount: Int, + maxTags: Int, + ) = discoveryRepository + .listTags(scope = null, limit = maxTags) + .filter { it.count >= minCount } + + private fun embeddingsFor(tags: List): Map = + try { + embedAll(tags) + } catch (ex: QueryEmbeddingException) { + log.warn("list_tag_candidates degraded: embedder failed", ex) + emptyMap() + } catch (ex: IllegalStateException) { + log.warn("list_tag_candidates degraded: embedder returned an invalid state", ex) + emptyMap() + } private fun embedAll(tags: List): Map { val out = LinkedHashMap(tags.size) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/domain/Ulid.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/domain/Ulid.kt index a85944a..8210f12 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/domain/Ulid.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/domain/Ulid.kt @@ -49,7 +49,6 @@ object Ulid { sb.append(chars) } - @Suppress("MagicNumber") // Byte-index ranges below match the documented hi/lo split. private fun encodeRandom(sb: StringBuilder) { val bytes = ByteArray(RANDOM_BYTES) random.nextBytes(bytes) @@ -59,9 +58,9 @@ object Ulid { // hi = bytes[0..3] → 32 bits // lo = bytes[4..9] → 48 bits var hi = 0L - for (i in 0..3) hi = (hi shl BYTE_BITS) or (bytes[i].toLong() and BYTE_MASK) + for (i in HI_BYTE_RANGE) hi = (hi shl BYTE_BITS) or (bytes[i].toLong() and BYTE_MASK) var lo = 0L - for (i in 4..9) lo = (lo shl BYTE_BITS) or (bytes[i].toLong() and BYTE_MASK) + for (i in LO_BYTE_RANGE) lo = (lo shl BYTE_BITS) or (bytes[i].toLong() and BYTE_MASK) val chars = CharArray(RANDOM_CHARS) // Lower 10 chars come from `lo` (50 bits — 48 source bits + // 2 zero-padded; padding only widens the output, doesn't bias @@ -86,3 +85,5 @@ private const val RANDOM_BYTES = 10 private const val BYTE_BITS = 8 private const val BYTE_MASK = 0xFFL private const val LO_CHARS = 10 +private val HI_BYTE_RANGE = 0..3 +private val LO_BYTE_RANGE = 4..9 diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminMcpTools.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminMcpTools.kt index ea38b27..f07f344 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminMcpTools.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/AdminMcpTools.kt @@ -2,6 +2,7 @@ package com.jorisjonkers.personalstack.knowledge.mcp import com.jorisjonkers.personalstack.knowledge.auth.AdminAuthorization import com.jorisjonkers.personalstack.knowledge.domain.Topic +import com.jorisjonkers.personalstack.knowledge.repo.AuditRecordRequest import com.jorisjonkers.personalstack.knowledge.repo.AuditRepository import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository import com.jorisjonkers.personalstack.knowledge.repo.TopicRepository @@ -31,10 +32,6 @@ import tools.jackson.databind.JsonNode * verbose — the MCP `tools/list` payload is the agent-facing API, * and the description text is part of that contract. */ -@Suppress( - "TooManyFunctions", - "LargeClass", -) // Admin tool surface lives here by design — one descriptor + handler per tool. @Component class AdminMcpTools( private val topicRepository: TopicRepository, @@ -46,37 +43,16 @@ class AdminMcpTools( fun tools(): List = listOf( - McpTool(addTopicDescriptor(), ::addTopicHandler), - McpTool(updateTopicDescriptor(), ::updateTopicHandler), - McpTool(mergeTopicsDescriptor(), ::mergeTopicsHandler), - McpTool(renameTagDescriptor(), ::renameTagHandler), - McpTool(mergeTagsDescriptor(), ::mergeTagsHandler), - McpTool(reclassifyNoteDescriptor(), ::reclassifyNoteHandler), + McpTool(AdminMcpDescriptors.addTopic(), ::addTopicHandler), + McpTool(AdminMcpDescriptors.updateTopic(), ::updateTopicHandler), + McpTool(AdminMcpDescriptors.mergeTopics(), ::mergeTopicsHandler), + McpTool(AdminMcpDescriptors.renameTag(), ::renameTagHandler), + McpTool(AdminMcpDescriptors.mergeTags(), ::mergeTagsHandler), + McpTool(AdminMcpDescriptors.reclassifyNote(), ::reclassifyNoteHandler), ) // -------- add_topic -------- - private fun addTopicDescriptor(): Map = - toolDescriptor( - name = "knowledge.add_topic", - description = - "Insert a new topic into `kb_topics`. `slug` is required, " + - "`description` and `aliases` are optional. Aliases are " + - "lower-cased before insert; the slug doubles as its own " + - "alias. Admin-only — non-admin callers get -32001.", - required = listOf("slug"), - properties = - mapOf( - "slug" to mapOf("type" to "string"), - "description" to mapOf("type" to "string"), - "aliases" to - mapOf( - "type" to "array", - "items" to mapOf("type" to "string"), - ), - ), - ) - private fun addTopicHandler(args: JsonNode): Map { val actor = adminAuthorization.requireAdmin() val slug = JsonArguments.requireString(args, "slug") @@ -86,7 +62,7 @@ class AdminMcpTools( topicRepository.insert(slug, description, aliases, createdBy = actor) } catch (exc: IntegrityConstraintViolationException) { log.info("admin.add_topic.duplicate", exc) - throw IllegalStateException( + error( "topic `$slug` already exists or one of its aliases collides — " + "use update_topic to modify, or merge_topics to consolidate", ) @@ -99,28 +75,6 @@ class AdminMcpTools( // -------- update_topic -------- - private fun updateTopicDescriptor(): Map = - toolDescriptor( - name = "knowledge.update_topic", - description = - "Replace `description` / `aliases` / `is_active` on an existing " + - "topic. Null fields leave the corresponding column untouched. " + - "`aliases`, when present, replaces the alias set wholesale " + - "(pass the full intended set). Admin-only.", - required = listOf("slug"), - properties = - mapOf( - "slug" to mapOf("type" to "string"), - "description" to mapOf("type" to "string"), - "aliases" to - mapOf( - "type" to "array", - "items" to mapOf("type" to "string"), - ), - "is_active" to mapOf("type" to "boolean"), - ), - ) - private fun updateTopicHandler(args: JsonNode): Map { val actor = adminAuthorization.requireAdmin() val slug = JsonArguments.requireString(args, "slug") @@ -129,7 +83,7 @@ class AdminMcpTools( if (args.has("aliases")) JsonArguments.optionalStringArray(args, "aliases").toSet() else null val isActive = JsonArguments.optionalBoolean(args, "is_active") val updated = topicRepository.update(slug, description, aliases, isActive) - if (!updated) throw IllegalStateException("topic `$slug` does not exist") + if (!updated) error("topic `$slug` does not exist") val refreshed = topicRepository.findBySlug(slug) ?: error("topic `$slug` disappeared after update") @@ -138,33 +92,15 @@ class AdminMcpTools( // -------- merge_topics -------- - private fun mergeTopicsDescriptor(): Map = - toolDescriptor( - name = "knowledge.merge_topics", - description = - "Move every `kb_notes` row scoped `topic:` over to " + - "`topic:` and soft-deactivate the source slug. " + - "Returns the number of notes that moved. Vault-path " + - "rewrites for already-promoted notes are not handled here " + - "— surface the count and run a vault sweep separately. " + - "Admin-only.", - required = listOf("from_slug", "into_slug"), - properties = - mapOf( - "from_slug" to mapOf("type" to "string"), - "into_slug" to mapOf("type" to "string"), - ), - ) - private fun mergeTopicsHandler(args: JsonNode): Map { val actor = adminAuthorization.requireAdmin() val fromSlug = JsonArguments.requireString(args, "from_slug") val intoSlug = JsonArguments.requireString(args, "into_slug") if (fromSlug == intoSlug) { - throw IllegalStateException("merge_topics: from_slug and into_slug must differ") + error("merge_topics: from_slug and into_slug must differ") } if (topicRepository.findBySlug(intoSlug) == null) { - throw IllegalStateException("merge_topics: into_slug `$intoSlug` is not defined") + error("merge_topics: into_slug `$intoSlug` is not defined") } val moved = topicRepository.mergeInto(fromSlug, intoSlug) return mapOf( @@ -177,38 +113,23 @@ class AdminMcpTools( // -------- rename_tag -------- - private fun renameTagDescriptor(): Map = - toolDescriptor( - name = "knowledge.rename_tag", - description = - "Rename a tag everywhere it appears in `kb_note_tags`. Returns " + - "the number of rows touched. The PK on `kb_note_tags` is " + - "`(note_id, tag)`, so an UPDATE fails loudly if some note " + - "already carries both the old and the new tag — dedupe " + - "first via `list_tags` or `find_duplicates`. Admin-only.", - required = listOf("from", "to"), - properties = - mapOf( - "from" to mapOf("type" to "string"), - "to" to mapOf("type" to "string"), - ), - ) - private fun renameTagHandler(args: JsonNode): Map { val actor = adminAuthorization.requireAdmin() val fromTag = JsonArguments.requireString(args, "from") val toTag = JsonArguments.requireString(args, "to") - if (fromTag == toTag) throw IllegalStateException("rename_tag: from and to must differ") + if (fromTag == toTag) error("rename_tag: from and to must differ") val touched = topicRepository.renameTag(fromTag, toTag) val auditId = if (touched > 0) { auditRepository .record( - actor = actor, - action = "rename_tag", - targetKind = "tag", - beforeJson = renameTagBeforePayload(fromTag), - afterJson = renameTagAfterPayload(toTag, touched), + AuditRecordRequest( + actor = actor, + action = "rename_tag", + targetKind = "tag", + beforeJson = renameTagBeforePayload(fromTag), + afterJson = renameTagAfterPayload(toTag, touched), + ), ).id } else { null @@ -224,29 +145,6 @@ class AdminMcpTools( // -------- merge_tags -------- - private fun mergeTagsDescriptor(): Map = - toolDescriptor( - name = "knowledge.merge_tags", - description = - "Merge multiple near-duplicate tags into a single canonical tag. `from` " + - "is the list of tags to fold; `into` is the survivor. Idempotent — " + - "a re-run with the same arguments is a no-op. Notes carrying both a " + - "source tag and the destination drop the source row rather than " + - "duplicate the PK. Use after `list_tag_candidates` surfaces a cluster " + - "for review. Admin-only.", - required = listOf("from", "into"), - properties = - mapOf( - "from" to - mapOf( - "type" to "array", - "items" to mapOf("type" to "string"), - "minItems" to 1, - ), - "into" to mapOf("type" to "string"), - ), - ) - private fun mergeTagsHandler(args: JsonNode): Map { val actor = adminAuthorization.requireAdmin() val fromTags = JsonArguments.optionalStringArray(args, "from") @@ -267,52 +165,23 @@ class AdminMcpTools( // -------- reclassify_note -------- - private fun reclassifyNoteDescriptor(): Map = - toolDescriptor( - name = "knowledge.reclassify_note", - description = - "Mark a previously-promoted note for reclassification: flip its scope " + - "back to `_inbox` so the next curator pass picks it up alongside the " + - "fresh inbox files. Optional `guidance` is persisted to `kb_audit` " + - "(`action=reclassify_requested`) and the curator's classifier prompt " + - "folds it into the LLM input for the next pass. Use when the operator " + - "(or an agent acting on their behalf) thinks a note ended up in the " + - "wrong topic or carries wrong tags. Admin-only.", - required = listOf("id"), - properties = - mapOf( - "id" to - mapOf( - "type" to "string", - "description" to "ULID of the note to reclassify.", - ), - "guidance" to - mapOf( - "type" to "string", - "description" to - "Operator hint for the classifier (e.g. \"this should be " + - "under topic:postgres, not topic:databases\"). " + - "Persisted to kb_audit; the curator reads the latest " + - "such row when re-classifying.", - ), - ), - ) - private fun reclassifyNoteHandler(args: JsonNode): Map { val actor = adminAuthorization.requireAdmin() val id = JsonArguments.requireString(args, "id") val guidance = JsonArguments.optionalString(args, "guidance") val touched = noteRepository.markForReclassify(id) if (touched == 0) { - throw IllegalStateException("reclassify_note: note `$id` does not exist") + error("reclassify_note: note `$id` does not exist") } val audit = auditRepository.record( - actor = actor, - action = "reclassify_requested", - targetId = id, - targetKind = "note", - afterJson = reclassifyAuditPayload(guidance), + AuditRecordRequest( + actor = actor, + action = "reclassify_requested", + targetId = id, + targetKind = "note", + afterJson = reclassifyAuditPayload(guidance), + ), ) log.info( "reclassify_note queued: id={} actor={} guidance_len={}", @@ -328,22 +197,6 @@ class AdminMcpTools( ) } - private fun reclassifyAuditPayload(guidance: String?): String = - if (guidance != null) { - """{"guidance":${jsonStringLiteral(guidance)}}""" - } else { - "{}" - } - - private fun renameTagBeforePayload(fromTag: String): String = """{"tag":${jsonStringLiteral(fromTag)}}""" - - private fun renameTagAfterPayload( - toTag: String, - rowsTouched: Int, - ): String = """{"tag":${jsonStringLiteral(toTag)},"rows_touched":$rowsTouched}""" - - private fun mergeTagsBeforePayload(fromTags: List): String = """{"from":${jsonStringArray(fromTags)}}""" - private fun recordMergeTagsAudit( actor: String, fromTags: List, @@ -353,71 +206,218 @@ class AdminMcpTools( if (result.rowsRenamed + result.rowsDeletedAsDupes == 0) return null return auditRepository .record( - actor = actor, - action = "merge_tags", - targetKind = "tag", - beforeJson = mergeTagsBeforePayload(fromTags), - afterJson = mergeTagsAfterPayload(intoTag, result), + AuditRecordRequest( + actor = actor, + action = "merge_tags", + targetKind = "tag", + beforeJson = mergeTagsBeforePayload(fromTags), + afterJson = mergeTagsAfterPayload(intoTag, result), + ), ).id } +} - private fun projectMergeTagsResult( - fromTags: List, - intoTag: String, - result: TopicRepository.MergeTagsResult, - actor: String, - auditId: String?, - ): Map = - buildMap { - put("from", fromTags) - put("into", intoTag) - put("rows_renamed", result.rowsRenamed) - put("rows_dropped_as_dupes", result.rowsDeletedAsDupes) - put("actor", actor) - if (auditId != null) put("audit_id", auditId) - } +private object AdminMcpDescriptors { + fun addTopic(): Map = + toolDescriptor( + name = "knowledge.add_topic", + description = + "Insert a new topic into `kb_topics`. `slug` is required, " + + "`description` and `aliases` are optional. Aliases are " + + "lower-cased before insert; the slug doubles as its own " + + "alias. Admin-only — non-admin callers get -32001.", + required = listOf("slug"), + properties = + mapOf( + "slug" to mapOf("type" to "string"), + "description" to mapOf("type" to "string"), + "aliases" to + mapOf( + "type" to "array", + "items" to mapOf("type" to "string"), + ), + ), + ) - private fun mergeTagsAfterPayload( - intoTag: String, - result: TopicRepository.MergeTagsResult, - ): String = - listOf( - """"into":${jsonStringLiteral(intoTag)}""", - """"rows_renamed":${result.rowsRenamed}""", - """"rows_dropped_as_dupes":${result.rowsDeletedAsDupes}""", - ).joinToString(separator = ",", prefix = "{", postfix = "}") + fun updateTopic(): Map = + toolDescriptor( + name = "knowledge.update_topic", + description = + "Replace `description` / `aliases` / `is_active` on an existing " + + "topic. Null fields leave the corresponding column untouched. " + + "`aliases`, when present, replaces the alias set wholesale " + + "(pass the full intended set). Admin-only.", + required = listOf("slug"), + properties = + mapOf( + "slug" to mapOf("type" to "string"), + "description" to mapOf("type" to "string"), + "aliases" to + mapOf( + "type" to "array", + "items" to mapOf("type" to "string"), + ), + "is_active" to mapOf("type" to "boolean"), + ), + ) - /** - * Minimal JSON-string encoder for the audit `afterJson` payload. - * The guidance arrives as untrusted operator text, so quotes / - * backslashes / control characters need escaping before they're - * embedded in a JSON literal. Kept inline rather than pulling - * in the Jackson mapper just for this one field. - */ - private fun jsonStringLiteral(text: String): String { - val escaped = - text - .replace("\\", "\\\\") - .replace("\"", "\\\"") - .replace("\n", "\\n") - .replace("\r", "\\r") - .replace("\t", "\\t") - return "\"$escaped\"" - } + fun mergeTopics(): Map = + toolDescriptor( + name = "knowledge.merge_topics", + description = + "Move every `kb_notes` row scoped `topic:` over to " + + "`topic:` and soft-deactivate the source slug. " + + "Returns the number of notes that moved. Vault-path " + + "rewrites for already-promoted notes are not handled here " + + "— surface the count and run a vault sweep separately. " + + "Admin-only.", + required = listOf("from_slug", "into_slug"), + properties = + mapOf( + "from_slug" to mapOf("type" to "string"), + "into_slug" to mapOf("type" to "string"), + ), + ) - private fun jsonStringArray(values: List): String = - values.joinToString(separator = ",", prefix = "[", postfix = "]") { jsonStringLiteral(it) } + fun renameTag(): Map = + toolDescriptor( + name = "knowledge.rename_tag", + description = + "Rename a tag everywhere it appears in `kb_note_tags`. Returns " + + "the number of rows touched. The PK on `kb_note_tags` is " + + "`(note_id, tag)`, so an UPDATE fails loudly if some note " + + "already carries both the old and the new tag — dedupe " + + "first via `list_tags` or `find_duplicates`. Admin-only.", + required = listOf("from", "to"), + properties = + mapOf( + "from" to mapOf("type" to "string"), + "to" to mapOf("type" to "string"), + ), + ) - // -------- shared projection -------- + fun mergeTags(): Map = + toolDescriptor( + name = "knowledge.merge_tags", + description = + "Merge multiple near-duplicate tags into a single canonical tag. `from` " + + "is the list of tags to fold; `into` is the survivor. Idempotent — " + + "a re-run with the same arguments is a no-op. Notes carrying both a " + + "source tag and the destination drop the source row rather than " + + "duplicate the PK. Use after `list_tag_candidates` surfaces a cluster " + + "for review. Admin-only.", + required = listOf("from", "into"), + properties = + mapOf( + "from" to + mapOf( + "type" to "array", + "items" to mapOf("type" to "string"), + "minItems" to 1, + ), + "into" to mapOf("type" to "string"), + ), + ) - private fun projectTopic(topic: Topic): Map = - mapOf( - "slug" to topic.slug, - "description" to topic.description, - "aliases" to topic.aliases.toList().sorted(), - "created_at" to topic.createdAt.toString(), - "created_by" to topic.createdBy, - "updated_at" to topic.updatedAt.toString(), - "is_active" to topic.isActive, + fun reclassifyNote(): Map = + toolDescriptor( + name = "knowledge.reclassify_note", + description = + "Mark a previously-promoted note for reclassification: flip its scope " + + "back to `_inbox` so the next curator pass picks it up alongside the " + + "fresh inbox files. Optional `guidance` is persisted to `kb_audit` " + + "(`action=reclassify_requested`) and the curator's classifier prompt " + + "folds it into the LLM input for the next pass. Use when the operator " + + "(or an agent acting on their behalf) thinks a note ended up in the " + + "wrong topic or carries wrong tags. Admin-only.", + required = listOf("id"), + properties = + mapOf( + "id" to + mapOf( + "type" to "string", + "description" to "ULID of the note to reclassify.", + ), + "guidance" to + mapOf( + "type" to "string", + "description" to + "Operator hint for the classifier (e.g. \"this should be " + + "under topic:postgres, not topic:databases\"). " + + "Persisted to kb_audit; the curator reads the latest " + + "such row when re-classifying.", + ), + ), ) } + +private fun reclassifyAuditPayload(guidance: String?): String = + if (guidance != null) { + """{"guidance":${jsonStringLiteral(guidance)}}""" + } else { + "{}" + } + +private fun renameTagBeforePayload(fromTag: String): String = """{"tag":${jsonStringLiteral(fromTag)}}""" + +private fun renameTagAfterPayload( + toTag: String, + rowsTouched: Int, +): String = """{"tag":${jsonStringLiteral(toTag)},"rows_touched":$rowsTouched}""" + +private fun mergeTagsBeforePayload(fromTags: List): String = """{"from":${jsonStringArray(fromTags)}}""" + +private fun projectMergeTagsResult( + fromTags: List, + intoTag: String, + result: TopicRepository.MergeTagsResult, + actor: String, + auditId: String?, +): Map = + buildMap { + put("from", fromTags) + put("into", intoTag) + put("rows_renamed", result.rowsRenamed) + put("rows_dropped_as_dupes", result.rowsDeletedAsDupes) + put("actor", actor) + if (auditId != null) put("audit_id", auditId) + } + +private fun mergeTagsAfterPayload( + intoTag: String, + result: TopicRepository.MergeTagsResult, +): String = + listOf( + """"into":${jsonStringLiteral(intoTag)}""", + """"rows_renamed":${result.rowsRenamed}""", + """"rows_dropped_as_dupes":${result.rowsDeletedAsDupes}""", + ).joinToString(separator = ",", prefix = "{", postfix = "}") + +/** + * Minimal JSON-string encoder for audit payloads. Operator text can contain + * quotes, backslashes, or control characters, so escape it before embedding. + */ +private fun jsonStringLiteral(text: String): String { + val escaped = + text + .replace("\\", "\\\\") + .replace("\"", "\\\"") + .replace("\n", "\\n") + .replace("\r", "\\r") + .replace("\t", "\\t") + return "\"$escaped\"" +} + +private fun jsonStringArray(values: List): String = + values.joinToString(separator = ",", prefix = "[", postfix = "]") { jsonStringLiteral(it) } + +private fun projectTopic(topic: Topic): Map = + mapOf( + "slug" to topic.slug, + "description" to topic.description, + "aliases" to topic.aliases.toList().sorted(), + "created_at" to topic.createdAt.toString(), + "created_by" to topic.createdBy, + "updated_at" to topic.updatedAt.toString(), + "is_active" to topic.isActive, + ) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/CaptureMcpTools.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/CaptureMcpTools.kt index 529604d..091b1f2 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/CaptureMcpTools.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/CaptureMcpTools.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") // Jackson 3 deprecated asText()/isTextual. package com.jorisjonkers.personalstack.knowledge.mcp diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpProjections.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpProjections.kt new file mode 100644 index 0000000..24dfd0e --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpProjections.kt @@ -0,0 +1,90 @@ +package com.jorisjonkers.personalstack.knowledge.mcp + +import com.jorisjonkers.personalstack.knowledge.domain.DuplicateMatch +import com.jorisjonkers.personalstack.knowledge.domain.KbNote +import com.jorisjonkers.personalstack.knowledge.domain.ScopeSummary +import com.jorisjonkers.personalstack.knowledge.domain.SourceSummary +import com.jorisjonkers.personalstack.knowledge.domain.SuggestedTopic +import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateCluster +import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateMember +import com.jorisjonkers.personalstack.knowledge.domain.TagSummary +import com.jorisjonkers.personalstack.knowledge.domain.TopicStats +import com.jorisjonkers.personalstack.knowledge.domain.TopicSummary + +internal fun projectTopic(summary: TopicSummary): Map = + mapOf( + "slug" to summary.slug, + "note_count" to summary.noteCount, + "last_captured_at" to summary.lastCapturedAt?.toString(), + "description" to summary.description, + ) + +internal fun projectTag(summary: TagSummary): Map = + mapOf( + "tag" to summary.tag, + "count" to summary.count, + "last_used_at" to summary.lastUsedAt?.toString(), + ) + +internal fun projectScope(summary: ScopeSummary): Map = + mapOf( + "scope" to summary.scope, + "note_count" to summary.noteCount, + "last_captured_at" to summary.lastCapturedAt?.toString(), + ) + +internal fun projectSource(summary: SourceSummary): Map = + mapOf( + "source" to summary.source, + "count" to summary.count, + ) + +internal fun projectTopicStats(stats: TopicStats): Map = + mapOf( + "slug" to stats.slug, + "note_count" to stats.noteCount, + "first_captured_at" to stats.firstCapturedAt?.toString(), + "last_captured_at" to stats.lastCapturedAt?.toString(), + "type_breakdown" to stats.typeBreakdown, + "top_tags" to stats.topTags.map(::projectTag), + ) + +internal fun projectInboxNote(note: KbNote): Map = + mapOf( + "id" to note.id, + "type" to note.type.wire, + "scope" to note.scope, + "source" to note.source, + "captured_at" to note.capturedAt.toString(), + "title" to note.title, + "vault_path" to note.vaultPath, + "tags" to note.tags.toList().sorted(), + ) + +internal fun projectSuggestedTopic(suggestion: SuggestedTopic): Map = + mapOf( + "slug" to suggestion.slug, + "score" to suggestion.score, + "note_count" to suggestion.noteCount, + ) + +internal fun projectDuplicate(match: DuplicateMatch): Map = + mapOf( + "id" to match.id, + "title" to match.title, + "scope" to match.scope, + "score" to match.score, + ) + +internal fun projectTagCluster(cluster: TagCandidateCluster): Map = + mapOf( + "members" to cluster.members.map(::projectTagMember), + "suggested_canonical" to cluster.suggestedCanonical, + "average_similarity" to cluster.averageSimilarity, + ) + +internal fun projectTagMember(member: TagCandidateMember): Map = + mapOf( + "tag" to member.tag, + "count" to member.count, + ) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpSupport.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpSupport.kt new file mode 100644 index 0000000..54b1067 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpSupport.kt @@ -0,0 +1,123 @@ +package com.jorisjonkers.personalstack.knowledge.mcp + +import com.jorisjonkers.personalstack.knowledge.discovery.DiscoveryService +import com.jorisjonkers.personalstack.knowledge.discovery.TagClusterService + +internal fun findDuplicatesDescriptor() = + toolDescriptor( + name = "knowledge.find_duplicates", + description = FIND_DUPLICATES_DESCRIPTION, + required = emptyList(), + properties = + mapOf( + "text" to mapOf("type" to "string"), + "id" to mapOf("type" to "string"), + "threshold" to + mapOf( + "type" to "number", + "minimum" to 0.0, + "maximum" to 1.0, + "default" to DEFAULT_DUP_THRESHOLD, + ), + "limit" to limitSchema(DEFAULT_DUP_LIMIT), + ), + ) + +internal fun findDuplicatesHandler( + args: tools.jackson.databind.JsonNode, + discoveryService: DiscoveryService, +): Map { + val text = JsonArguments.optionalString(args, "text") + val id = JsonArguments.optionalString(args, "id") + val threshold = JsonArguments.optionalDouble(args, "threshold") ?: DEFAULT_DUP_THRESHOLD + val limit = JsonArguments.optionalInt(args, "limit") ?: DEFAULT_DUP_LIMIT + val matches = + when { + id != null -> discoveryService.findDuplicatesOf(id, threshold, limit) + text != null -> discoveryService.findDuplicates(text, threshold, limit) + else -> error("find_duplicates requires either `text` or `id` — neither was supplied") + } + return mapOf("matches" to matches.map(::projectDuplicate)) +} + +internal fun listTagCandidatesDescriptor(): Map = + toolDescriptor( + name = "knowledge.list_tag_candidates", + description = LIST_TAG_CANDIDATES_DESCRIPTION, + required = emptyList(), + properties = + mapOf( + "min_count" to + mapOf( + "type" to "integer", + "minimum" to 1, + "default" to DEFAULT_TAG_CANDIDATE_MIN_COUNT, + ), + "threshold" to + mapOf( + "type" to "number", + "minimum" to 0.0, + "maximum" to 1.0, + "default" to DEFAULT_TAG_CANDIDATE_THRESHOLD, + ), + "max_tags" to + mapOf( + "type" to "integer", + "minimum" to 1, + "maximum" to MAX_TAG_CANDIDATE_LIMIT, + "default" to DEFAULT_TAG_CANDIDATE_MAX_TAGS, + ), + ), + ) + +internal fun listTagCandidatesHandler( + args: tools.jackson.databind.JsonNode, + tagClusterService: TagClusterService, +): Map { + val minCount = + JsonArguments.optionalInt(args, "min_count") + ?: DEFAULT_TAG_CANDIDATE_MIN_COUNT + val threshold = + JsonArguments.optionalDouble(args, "threshold") + ?: DEFAULT_TAG_CANDIDATE_THRESHOLD + val maxTags = + (JsonArguments.optionalInt(args, "max_tags") ?: DEFAULT_TAG_CANDIDATE_MAX_TAGS) + .coerceIn(1, MAX_TAG_CANDIDATE_LIMIT) + val clusters = tagClusterService.listTagCandidates(minCount, threshold, maxTags) + return mapOf("clusters" to clusters.map(::projectTagCluster)) +} + +internal fun limitSchema(default: Int) = + mapOf( + "type" to "integer", + "minimum" to 1, + "maximum" to DISCOVERY_MAX_LIMIT, + "default" to default, + ) + +internal const val DEFAULT_LIST_LIMIT = 50 +internal const val DEFAULT_TAG_LIMIT = 100 +internal const val DEFAULT_INBOX_LIMIT = 20 +internal const val DEFAULT_TOP_TAGS = 10 +internal const val MAX_TOP_TAGS = 50 +internal const val DISCOVERY_MAX_LIMIT = 200 +internal const val DEFAULT_SUGGEST_LIMIT = 5 +internal const val DEFAULT_DUP_LIMIT = 5 +internal const val DEFAULT_DUP_THRESHOLD = 0.85 +internal const val DEFAULT_TAG_CANDIDATE_MIN_COUNT = 1 +internal const val DEFAULT_TAG_CANDIDATE_THRESHOLD = 0.85 +internal const val DEFAULT_TAG_CANDIDATE_MAX_TAGS = 200 +internal const val MAX_TAG_CANDIDATE_LIMIT = 500 +internal const val FIND_DUPLICATES_DESCRIPTION = + "Vector-backed near-duplicate detection. Embeds `text` (or pulls the persisted " + + "embedding when `id` is provided) and returns rows whose cosine similarity " + + "is at or above `threshold` (default 0.85). Use pre-capture to avoid " + + "re-stating an existing lesson, or post-capture to audit possible dupes." +internal const val LIST_TAG_CANDIDATES_DESCRIPTION = + "Surface near-duplicate tag clusters for hygiene review. Each cluster contains " + + "tags whose pairwise cosine similarity is at or above `threshold` (default " + + "0.85), alongside the highest-count member as the suggested canonical. Use " + + "before `knowledge.merge_tags` so the merge target is a deliberate operator " + + "choice and the source list is complete. Embeds every distinct tag on the " + + "fly — at the current scale (≤200 tags) this completes in seconds; degrades " + + "to an empty list when the embedder is unreachable." diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpTools.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpTools.kt index b0f61ef..12fb865 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpTools.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpTools.kt @@ -2,16 +2,6 @@ package com.jorisjonkers.personalstack.knowledge.mcp import com.jorisjonkers.personalstack.knowledge.discovery.DiscoveryService import com.jorisjonkers.personalstack.knowledge.discovery.TagClusterService -import com.jorisjonkers.personalstack.knowledge.domain.DuplicateMatch -import com.jorisjonkers.personalstack.knowledge.domain.KbNote -import com.jorisjonkers.personalstack.knowledge.domain.ScopeSummary -import com.jorisjonkers.personalstack.knowledge.domain.SourceSummary -import com.jorisjonkers.personalstack.knowledge.domain.SuggestedTopic -import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateCluster -import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateMember -import com.jorisjonkers.personalstack.knowledge.domain.TagSummary -import com.jorisjonkers.personalstack.knowledge.domain.TopicStats -import com.jorisjonkers.personalstack.knowledge.domain.TopicSummary import org.springframework.stereotype.Component /** @@ -36,7 +26,6 @@ import org.springframework.stereotype.Component * and ship as follow-up PRs once the dynamic-topic schema is in * place. */ -@Suppress("TooManyFunctions", "LargeClass") // Discovery surface lives here by design. @Component class DiscoveryMcpTools( private val discoveryService: DiscoveryService, @@ -231,207 +220,12 @@ class DiscoveryMcpTools( private fun findDuplicatesTool() = McpTool( descriptor = findDuplicatesDescriptor(), - handler = ::findDuplicatesHandler, + handler = { args -> findDuplicatesHandler(args, discoveryService) }, ) - private fun findDuplicatesDescriptor() = - toolDescriptor( - name = "knowledge.find_duplicates", - description = FIND_DUPLICATES_DESCRIPTION, - required = emptyList(), - properties = - mapOf( - "text" to mapOf("type" to "string"), - "id" to mapOf("type" to "string"), - "threshold" to - mapOf( - "type" to "number", - "minimum" to 0.0, - "maximum" to 1.0, - "default" to DEFAULT_DUP_THRESHOLD, - ), - "limit" to limitSchema(DEFAULT_DUP_LIMIT), - ), - ) - - private fun findDuplicatesHandler(args: tools.jackson.databind.JsonNode): Map { - val text = JsonArguments.optionalString(args, "text") - val id = JsonArguments.optionalString(args, "id") - val threshold = JsonArguments.optionalDouble(args, "threshold") ?: DEFAULT_DUP_THRESHOLD - val limit = JsonArguments.optionalInt(args, "limit") ?: DEFAULT_DUP_LIMIT - val matches = - when { - id != null -> discoveryService.findDuplicatesOf(id, threshold, limit) - text != null -> discoveryService.findDuplicates(text, threshold, limit) - else -> error("find_duplicates requires either `text` or `id` — neither was supplied") - } - return mapOf("matches" to matches.map(::projectDuplicate)) - } - private fun listTagCandidatesTool() = McpTool( descriptor = listTagCandidatesDescriptor(), - handler = ::listTagCandidatesHandler, - ) - - private fun listTagCandidatesDescriptor(): Map = - toolDescriptor( - name = "knowledge.list_tag_candidates", - description = LIST_TAG_CANDIDATES_DESCRIPTION, - required = emptyList(), - properties = - mapOf( - "min_count" to - mapOf( - "type" to "integer", - "minimum" to 1, - "default" to DEFAULT_TAG_CANDIDATE_MIN_COUNT, - ), - "threshold" to - mapOf( - "type" to "number", - "minimum" to 0.0, - "maximum" to 1.0, - "default" to DEFAULT_TAG_CANDIDATE_THRESHOLD, - ), - "max_tags" to - mapOf( - "type" to "integer", - "minimum" to 1, - "maximum" to MAX_TAG_CANDIDATE_LIMIT, - "default" to DEFAULT_TAG_CANDIDATE_MAX_TAGS, - ), - ), - ) - - private fun listTagCandidatesHandler(args: tools.jackson.databind.JsonNode): Map { - val minCount = - JsonArguments.optionalInt(args, "min_count") - ?: DEFAULT_TAG_CANDIDATE_MIN_COUNT - val threshold = - JsonArguments.optionalDouble(args, "threshold") - ?: DEFAULT_TAG_CANDIDATE_THRESHOLD - val maxTags = - (JsonArguments.optionalInt(args, "max_tags") ?: DEFAULT_TAG_CANDIDATE_MAX_TAGS) - .coerceIn(1, MAX_TAG_CANDIDATE_LIMIT) - val clusters = tagClusterService.listTagCandidates(minCount, threshold, maxTags) - return mapOf("clusters" to clusters.map(::projectTagCluster)) - } - - // -------- projections -------- - - private fun projectTopic(summary: TopicSummary): Map = - mapOf( - "slug" to summary.slug, - "note_count" to summary.noteCount, - "last_captured_at" to summary.lastCapturedAt?.toString(), - "description" to summary.description, - ) - - private fun projectTag(summary: TagSummary): Map = - mapOf( - "tag" to summary.tag, - "count" to summary.count, - "last_used_at" to summary.lastUsedAt?.toString(), - ) - - private fun projectScope(summary: ScopeSummary): Map = - mapOf( - "scope" to summary.scope, - "note_count" to summary.noteCount, - "last_captured_at" to summary.lastCapturedAt?.toString(), - ) - - private fun projectSource(summary: SourceSummary): Map = - mapOf( - "source" to summary.source, - "count" to summary.count, - ) - - private fun projectTopicStats(stats: TopicStats): Map = - mapOf( - "slug" to stats.slug, - "note_count" to stats.noteCount, - "first_captured_at" to stats.firstCapturedAt?.toString(), - "last_captured_at" to stats.lastCapturedAt?.toString(), - "type_breakdown" to stats.typeBreakdown, - "top_tags" to stats.topTags.map(::projectTag), - ) - - private fun projectInboxNote(note: KbNote): Map = - mapOf( - "id" to note.id, - "type" to note.type.wire, - "scope" to note.scope, - "source" to note.source, - "captured_at" to note.capturedAt.toString(), - "title" to note.title, - "vault_path" to note.vaultPath, - "tags" to note.tags.toList().sorted(), - ) - - private fun projectSuggestedTopic(suggestion: SuggestedTopic): Map = - mapOf( - "slug" to suggestion.slug, - "score" to suggestion.score, - "note_count" to suggestion.noteCount, - ) - - private fun projectDuplicate(match: DuplicateMatch): Map = - mapOf( - "id" to match.id, - "title" to match.title, - "scope" to match.scope, - "score" to match.score, - ) - - private fun projectTagCluster(cluster: TagCandidateCluster): Map = - mapOf( - "members" to cluster.members.map(::projectTagMember), - "suggested_canonical" to cluster.suggestedCanonical, - "average_similarity" to cluster.averageSimilarity, + handler = { args -> listTagCandidatesHandler(args, tagClusterService) }, ) - - private fun projectTagMember(member: TagCandidateMember): Map = - mapOf( - "tag" to member.tag, - "count" to member.count, - ) - - private fun limitSchema(default: Int) = - mapOf( - "type" to "integer", - "minimum" to 1, - "maximum" to MAX_LIMIT, - "default" to default, - ) - - private companion object { - const val DEFAULT_LIST_LIMIT = 50 - const val DEFAULT_TAG_LIMIT = 100 - const val DEFAULT_INBOX_LIMIT = 20 - const val DEFAULT_TOP_TAGS = 10 - const val MAX_TOP_TAGS = 50 - const val MAX_LIMIT = 200 - const val DEFAULT_SUGGEST_LIMIT = 5 - const val DEFAULT_DUP_LIMIT = 5 - const val DEFAULT_DUP_THRESHOLD = 0.85 - const val DEFAULT_TAG_CANDIDATE_MIN_COUNT = 1 - const val DEFAULT_TAG_CANDIDATE_THRESHOLD = 0.85 - const val DEFAULT_TAG_CANDIDATE_MAX_TAGS = 200 - const val MAX_TAG_CANDIDATE_LIMIT = 500 - const val FIND_DUPLICATES_DESCRIPTION = - "Vector-backed near-duplicate detection. Embeds `text` (or pulls the persisted " + - "embedding when `id` is provided) and returns rows whose cosine similarity " + - "is at or above `threshold` (default 0.85). Use pre-capture to avoid " + - "re-stating an existing lesson, or post-capture to audit possible dupes." - const val LIST_TAG_CANDIDATES_DESCRIPTION = - "Surface near-duplicate tag clusters for hygiene review. Each cluster contains " + - "tags whose pairwise cosine similarity is at or above `threshold` (default " + - "0.85), alongside the highest-count member as the suggested canonical. Use " + - "before `knowledge.merge_tags` so the merge target is a deliberate operator " + - "choice and the source list is complete. Embeds every distinct tag on the " + - "fly — at the current scale (≤200 tags) this completes in seconds; degrades " + - "to an empty list when the embedder is unreachable." - } } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/JsonArguments.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/JsonArguments.kt index 78054ab..5073985 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/JsonArguments.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/JsonArguments.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") // Jackson 3 deprecated asText()/isTextual. package com.jorisjonkers.personalstack.knowledge.mcp @@ -26,8 +25,8 @@ internal object JsonArguments { node .get(field) ?.takeUnless { it.isNull } - ?.takeIf { it.isTextual } - ?.asText() + ?.takeIf { it.isString } + ?.asString() ?.takeIf { it.isNotBlank() } fun rawText( @@ -37,8 +36,8 @@ internal object JsonArguments { node .get(field) ?.takeUnless { it.isNull } - ?.takeIf { it.isTextual } - ?.asText() + ?.takeIf { it.isString } + ?.asString() fun optionalDouble( node: JsonNode, @@ -76,7 +75,7 @@ internal object JsonArguments { ): List { val arr = node.get(field) return if (arr?.isArray == true) { - arr.mapNotNull { if (it.isTextual) it.asText() else null } + arr.mapNotNull { if (it.isString) it.asString() else null } } else { emptyList() } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpController.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpController.kt index caec688..9509fad 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpController.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpController.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") // Jackson 3 deprecated JsonNode.asText() in favour of asString(); // keeping the JsonNode shape across the codebase until a coordinated migration lands. package com.jorisjonkers.personalstack.knowledge.mcp @@ -60,7 +59,7 @@ class McpController( val name = request.params ?.get("name") - ?.asText() + ?.asString() .orEmpty() val arguments = request.params?.get("arguments") return when { @@ -73,41 +72,32 @@ class McpController( } } - private fun projectPromptResult(resolved: PromptResult): Map = - mapOf( - "description" to resolved.description, - "messages" to - resolved.messages.map { msg -> - mapOf( - "role" to msg.role, - "content" to mapOf("type" to "text", "text" to msg.text), - ) - }, - ) - - // ReturnCount(4): each return signals a distinct failure mode of - // the JSON-RPC dispatch (blank name, unauthorized, unknown tool, - // success). Collapsing them via a sealed-result type adds more - // code than it removes and obscures the per-failure error code, - // so suppress the rule here rather than refactor for refactor's - // sake. - @Suppress("ReturnCount") private fun handleToolsCall(request: JsonRpcRequest): JsonRpcResponse { val name = request.params ?.get("name") - ?.asText() + ?.asString() .orEmpty() - if (name.isBlank()) return invalidParamsResponse(request.id, "tools/call: missing required string 'name'") + return if (name.isBlank()) { + invalidParamsResponse(request.id, "tools/call: missing required string 'name'") + } else { + dispatchToolCall(request, name) + } + } + + private fun dispatchToolCall( + request: JsonRpcRequest, + name: String, + ): JsonRpcResponse { val arguments = request.params?.get("arguments") - val result = - try { - tools.call(name, arguments) - } catch (exc: McpAuthorizationError) { - return unauthorizedResponse(request.id, exc.message) - } - ?: return methodNotFoundResponse(request.id, "tools/call:$name") - return JsonRpcResponse(id = request.id, result = wrapToolResult(result)) + return try { + tools + .call(name, arguments) + ?.let { JsonRpcResponse(id = request.id, result = wrapToolResult(it)) } + ?: methodNotFoundResponse(request.id, "tools/call:$name") + } catch (exc: McpAuthorizationError) { + unauthorizedResponse(request.id, exc.message) + } } private fun invalidParamsResponse( @@ -200,3 +190,15 @@ class McpController( private val MAPPER: JsonMapper = JsonMapper.builder().build() } } + +private fun projectPromptResult(resolved: PromptResult): Map = + mapOf( + "description" to resolved.description, + "messages" to + resolved.messages.map { msg -> + mapOf( + "role" to msg.role, + "content" to mapOf("type" to "text", "text" to msg.text), + ) + }, + ) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpPrompts.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpPrompts.kt index d75f50f..c1e0b9d 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpPrompts.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpPrompts.kt @@ -177,9 +177,8 @@ internal object TopicsAuditPrompt : PromptDefinition { "thin, slugs that look like duplicates, and notes that haven't been classified yet." override val arguments: List = emptyList() - @Suppress("UNUSED_PARAMETER") override fun build(arguments: JsonNode?): PromptResult { - val text = + val baseText = """ Run a knowledge-base vocabulary audit: @@ -189,6 +188,12 @@ internal object TopicsAuditPrompt : PromptDefinition { Report findings as three short sections (thin topics, duplicate-looking tags, pending inbox). Propose specific `knowledge.merge_topics` / `knowledge.rename_tag` calls for the candidates rather than just naming them. Don't actually run the mutations. """.trimIndent() + val text = + if (arguments != null && !arguments.isNull && arguments.size() > 0) { + "$baseText\n\nIgnore supplied arguments; this prompt does not define parameters." + } else { + baseText + } return PromptResult( description = "Audit pass over topics + tags + pending inbox.", messages = listOf(PromptMessage(role = "user", text = text)), diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpTools.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpTools.kt index bcf2e4c..bdb7e80 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpTools.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpTools.kt @@ -11,28 +11,18 @@ import tools.jackson.databind.json.JsonMapper */ @Component class McpTools( - captureTools: CaptureMcpTools, - readTools: ReadMcpTools, - discoveryTools: DiscoveryMcpTools, - adminTools: AdminMcpTools, - digestTools: DigestMcpTools, - auditTools: AuditMcpTools, - reviewTools: ReviewMcpTools, + coreTools: CoreMcpToolSet, + fullTools: FullMcpToolSet, modeProperties: KnowledgeModeProperties = KnowledgeModeProperties(), ) { private val tools: Map = buildList { // Core retrieval + capture surface — always registered. - addAll(captureTools.tools()) - addAll(readTools.tools()) + addAll(coreTools.tools()) // Curator-governance surface — only in full mode. `lite` keeps a // thin recall+capture service (the lightweight-memory target). if (modeProperties.mode == KnowledgeMode.FULL) { - addAll(discoveryTools.tools()) - addAll(adminTools.tools()) - addAll(digestTools.tools()) - addAll(auditTools.tools()) - addAll(reviewTools.tools()) + addAll(fullTools.tools()) } }.associateBy { it.name } @@ -47,3 +37,27 @@ class McpTools( private val NULL_NODE: JsonNode = JsonMapper.builder().build().createObjectNode() } } + +@Component +class CoreMcpToolSet( + private val captureTools: CaptureMcpTools, + private val readTools: ReadMcpTools, +) { + fun tools(): List = captureTools.tools() + readTools.tools() +} + +@Component +class FullMcpToolSet( + private val discoveryTools: DiscoveryMcpTools, + private val adminTools: AdminMcpTools, + private val digestTools: DigestMcpTools, + private val auditTools: AuditMcpTools, + private val reviewTools: ReviewMcpTools, +) { + fun tools(): List = + discoveryTools.tools() + + adminTools.tools() + + digestTools.tools() + + auditTools.tools() + + reviewTools.tools() +} diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReadMcpTools.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReadMcpTools.kt index b0e7fd7..30f7870 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReadMcpTools.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReadMcpTools.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") // Jackson 3 deprecated asText()/isTextual. package com.jorisjonkers.personalstack.knowledge.mcp @@ -19,7 +18,6 @@ import tools.jackson.databind.JsonNode * planned follow-up, not the current deep implementation. */ @Component -@Suppress("TooManyFunctions") // Read-path descriptors, parsers, and projections stay co-located. class ReadMcpTools( private val recallService: RecallService, ) { @@ -123,26 +121,6 @@ class ReadMcpTools( ), ) - private fun recallModeProperty() = - mapOf( - "type" to "string", - "enum" to RecallMode.entries.map { it.wire }, - "description" to - "fast = FTS only; hybrid = FTS + vector + RRF; " + - "deep = hybrid + listwise reranker (LightRAG graph traversal is a planned follow-up).", - ) - - private fun recallScopeProperty() = - mapOf( - "type" to "string", - "description" to - "Restrict to a single scope (`topic:` / `project:` / " + - "`agent:`). Pass `all` to include every scope including " + - "untriaged `_inbox`. Omit for the curated default — every " + - "scope except `_inbox` and assistant-private agent scopes " + - "(`agent:_shared` stays visible).", - ) - private fun recallHandler(args: JsonNode): Map { // `query` is "required" by the schema, but a whitespace-only value // should produce zero hits, not a 500 — agents sometimes forward @@ -204,62 +182,78 @@ class ReadMcpTools( ) return mapOf("notes" to notes.map(::projectNote)) } +} - // -------- projections -------- +private fun recallModeProperty() = + mapOf( + "type" to "string", + "enum" to RecallMode.entries.map { it.wire }, + "description" to + "fast = FTS only; hybrid = FTS + vector + RRF; " + + "deep = hybrid + listwise reranker (LightRAG graph traversal is a planned follow-up).", + ) - private fun projectNote(note: KbNote): Map = - mapOf( - "id" to note.id, - "type" to note.type.wire, - "scope" to note.scope, - "source" to note.source, - "captured_at" to note.capturedAt.toString(), - "session_id" to note.sessionId, - "confidence" to note.confidence, - "title" to note.title, - "body" to note.body, - "vault_path" to note.vaultPath, - "vault_commit" to note.vaultCommit, - "tags" to note.tags.toList().sorted(), - ) +private fun recallScopeProperty() = + mapOf( + "type" to "string", + "description" to + "Restrict to a single scope (`topic:` / `project:` / " + + "`agent:`). Pass `all` to include every scope including " + + "untriaged `_inbox`. Omit for the curated default — every " + + "scope except `_inbox` and assistant-private agent scopes " + + "(`agent:_shared` stays visible).", + ) - private fun projectHit(hit: RecallHit): Map = - mapOf( - "id" to hit.id, - "type" to hit.type, - "scope" to hit.scope, - "title" to hit.title, - "snippet" to hit.snippet, - "score" to hit.score, - "tags" to hit.tags.toList().sorted(), - ) +private fun projectNote(note: KbNote): Map = + mapOf( + "id" to note.id, + "type" to note.type.wire, + "scope" to note.scope, + "source" to note.source, + "captured_at" to note.capturedAt.toString(), + "session_id" to note.sessionId, + "confidence" to note.confidence, + "title" to note.title, + "body" to note.body, + "vault_path" to note.vaultPath, + "vault_commit" to note.vaultCommit, + "tags" to note.tags.toList().sorted(), + ) - private fun projectRelation(rel: KbRelation): Map = - mapOf( - "subject_id" to rel.subjectId, - "predicate" to rel.predicate, - "object_id" to rel.objectId, - "props" to rel.props, - "created_at" to rel.createdAt.toString(), - ) +private fun projectHit(hit: RecallHit): Map = + mapOf( + "id" to hit.id, + "type" to hit.type, + "scope" to hit.scope, + "title" to hit.title, + "snippet" to hit.snippet, + "score" to hit.score, + "tags" to hit.tags.toList().sorted(), + ) - companion object { - private const val DEFAULT_RECALL_LIMIT = 10 - private const val DEFAULT_RECENT_LIMIT = 20 - private const val MAX_LIMIT = 100 - private const val DEFAULT_RELATION_DEPTH = 1 +private fun projectRelation(rel: KbRelation): Map = + mapOf( + "subject_id" to rel.subjectId, + "predicate" to rel.predicate, + "object_id" to rel.objectId, + "props" to rel.props, + "created_at" to rel.createdAt.toString(), + ) - private const val RECALL_TOOL_DESCRIPTION = - "Layered recall over kb_notes. `mode=fast` is single-leg Postgres FTS (~50 ms p50). " + - "`mode=hybrid` adds the pgvector ANN leg and fuses with Reciprocal Rank Fusion " + - "(~100-300 ms once Ollama is warm). `mode=deep` runs hybrid retrieval, optionally " + - "fuses LightRAG graph context when `knowledge.recall.graph.enabled=true`, then " + - "applies a listwise reranker. Server-side default is " + - "configurable (`knowledge.recall.default-mode`); when omitted, the server's choice applies." +private const val DEFAULT_RECALL_LIMIT = 10 +private const val DEFAULT_RECENT_LIMIT = 20 +private const val MAX_LIMIT = 100 +private const val DEFAULT_RELATION_DEPTH = 1 - // Hard ceiling for the agent-facing depth; the repo enforces - // its own (private) ceiling underneath, so this just keeps the - // tool's input shape honest. - private const val MAX_RELATION_DEPTH = 4 - } -} +private const val RECALL_TOOL_DESCRIPTION = + "Layered recall over kb_notes. `mode=fast` is single-leg Postgres FTS (~50 ms p50). " + + "`mode=hybrid` adds the pgvector ANN leg and fuses with Reciprocal Rank Fusion " + + "(~100-300 ms once Ollama is warm). `mode=deep` runs hybrid retrieval, optionally " + + "fuses LightRAG graph context when `knowledge.recall.graph.enabled=true`, then " + + "applies a listwise reranker. Server-side default is " + + "configurable (`knowledge.recall.default-mode`); when omitted, the server's choice applies." + +// Hard ceiling for the agent-facing depth; the repo enforces its own +// private ceiling underneath, so this just keeps the tool's input +// shape honest. +private const val MAX_RELATION_DEPTH = 4 diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpSupport.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpSupport.kt new file mode 100644 index 0000000..0176920 --- /dev/null +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpSupport.kt @@ -0,0 +1,107 @@ +package com.jorisjonkers.personalstack.knowledge.mcp + +import com.jorisjonkers.personalstack.knowledge.domain.KbAuditRow +import com.jorisjonkers.personalstack.knowledge.domain.ReviewBucket +import com.jorisjonkers.personalstack.knowledge.domain.ReviewNote +import com.jorisjonkers.personalstack.knowledge.domain.ReviewSuggestion +import com.jorisjonkers.personalstack.knowledge.domain.ReviewSummary +import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateCluster +import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateMember + +internal fun projectReviewSummary(summary: ReviewSummary): Map = + mapOf( + "generated_at" to summary.generatedAt.toString(), + "inbox" to projectReviewBucket(summary.inbox, ::projectReviewNote), + "needs_review" to projectReviewBucket(summary.needsReview, ::projectReviewNote), + "recent_auto_captures" to projectReviewBucket(summary.recentAutoCaptures, ::projectReviewNote), + "stale_unused_notes" to projectReviewBucket(summary.staleUnusedNotes, ::projectReviewNote), + "low_confidence_high_recall" to projectReviewBucket(summary.lowConfidenceHighRecall, ::projectReviewNote), + "tag_candidate_clusters" to projectReviewBucket(summary.tagCandidateClusters, ::projectReviewTagCluster), + "recent_audit" to projectReviewBucket(summary.recentAudit, ::projectReviewAuditRow), + "suggestions" to summary.suggestions.map(::projectReviewSuggestion), + ) + +internal fun projectReviewBucket( + bucket: ReviewBucket, + project: (T) -> Map, +): Map = + mapOf( + "total" to bucket.total, + "items" to bucket.items.map(project), + ) + +internal fun projectReviewNote(note: ReviewNote): Map = + mapOf( + "id" to note.id, + "type" to note.type, + "scope" to note.scope, + "source" to note.source, + "captured_at" to note.capturedAt.toString(), + "confidence" to note.confidence, + "title" to note.title, + "vault_path" to note.vaultPath, + "tags" to note.tags.toList().sorted(), + "recall_count" to note.recallCount, + "last_recalled_at" to note.lastRecalledAt?.toString(), + ) + +internal fun projectReviewTagCluster(cluster: TagCandidateCluster): Map = + mapOf( + "members" to cluster.members.map(::projectReviewTagMember), + "suggested_canonical" to cluster.suggestedCanonical, + "average_similarity" to cluster.averageSimilarity, + ) + +internal fun projectReviewTagMember(member: TagCandidateMember): Map = + mapOf( + "tag" to member.tag, + "count" to member.count, + ) + +internal fun projectReviewAuditRow(row: KbAuditRow): Map = + mapOf( + "id" to row.id, + "actor" to row.actor, + "action" to row.action, + "target_id" to row.targetId, + "target_kind" to row.targetKind, + "before_json" to row.beforeJson, + "after_json" to row.afterJson, + "at" to row.at.toString(), + ) + +internal fun projectReviewSuggestion(suggestion: ReviewSuggestion): Map = + mapOf( + "kind" to suggestion.kind, + "severity" to suggestion.severity, + "message" to suggestion.message, + "suggested_tool" to suggestion.suggestedTool, + "target_id" to suggestion.targetId, + "target_kind" to suggestion.targetKind, + "details" to suggestion.details, + ) + +internal fun reviewIntSchema( + default: Int, + maximum: Int, +): Map = + mapOf( + "type" to "integer", + "minimum" to 1, + "maximum" to maximum, + "default" to default, + ) + +internal fun boundedReviewNumberSchema(default: Double): Map = + mapOf( + "type" to "number", + "minimum" to 0.0, + "maximum" to 1.0, + "default" to default, + ) + +internal const val REVIEW_MAX_LIMIT = 50 +internal const val REVIEW_MAX_STALE_DAYS = 365 +internal const val REVIEW_MAX_RECALL_COUNT = 1000 +internal const val REVIEW_MAX_TAG_COUNT = 1000 +internal const val REVIEW_MAX_TAGS = 500 diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpTools.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpTools.kt index 78e4741..b73090b 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpTools.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpTools.kt @@ -1,19 +1,11 @@ package com.jorisjonkers.personalstack.knowledge.mcp -import com.jorisjonkers.personalstack.knowledge.domain.KbAuditRow -import com.jorisjonkers.personalstack.knowledge.domain.ReviewBucket -import com.jorisjonkers.personalstack.knowledge.domain.ReviewNote -import com.jorisjonkers.personalstack.knowledge.domain.ReviewSuggestion -import com.jorisjonkers.personalstack.knowledge.domain.ReviewSummary -import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateCluster -import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateMember import com.jorisjonkers.personalstack.knowledge.review.ReviewService import com.jorisjonkers.personalstack.knowledge.review.ReviewSummaryRequest import org.springframework.stereotype.Component import tools.jackson.databind.JsonNode @Component -@Suppress("TooManyFunctions") // MCP descriptor, parser, and projection helpers stay co-located. class ReviewMcpTools( private val reviewService: ReviewService, ) { @@ -35,153 +27,69 @@ class ReviewMcpTools( private fun reviewSummaryProperties(): Map = mapOf( - "limit" to intSchema(default = ReviewSummaryRequest.DEFAULT_LIMIT, maximum = MAX_LIMIT), - "stale_days" to intSchema(default = ReviewSummaryRequest.DEFAULT_STALE_DAYS, maximum = MAX_STALE_DAYS), - "low_confidence_max" to boundedNumberSchema(ReviewSummaryRequest.DEFAULT_LOW_CONFIDENCE_MAX), + "limit" to reviewIntSchema(default = ReviewSummaryRequest.DEFAULT_LIMIT, maximum = REVIEW_MAX_LIMIT), + "stale_days" to + reviewIntSchema( + default = ReviewSummaryRequest.DEFAULT_STALE_DAYS, + maximum = REVIEW_MAX_STALE_DAYS, + ), + "low_confidence_max" to boundedReviewNumberSchema(ReviewSummaryRequest.DEFAULT_LOW_CONFIDENCE_MAX), "high_recall_min" to - intSchema( + reviewIntSchema( default = ReviewSummaryRequest.DEFAULT_HIGH_RECALL_MIN, - maximum = MAX_RECALL_COUNT, + maximum = REVIEW_MAX_RECALL_COUNT, + ), + "tag_min_count" to + reviewIntSchema( + default = ReviewSummaryRequest.DEFAULT_TAG_MIN_COUNT, + maximum = REVIEW_MAX_TAG_COUNT, + ), + "tag_threshold" to boundedReviewNumberSchema(ReviewSummaryRequest.DEFAULT_TAG_THRESHOLD), + "tag_max_tags" to + reviewIntSchema( + default = ReviewSummaryRequest.DEFAULT_TAG_MAX_TAGS, + maximum = REVIEW_MAX_TAGS, ), - "tag_min_count" to intSchema(default = ReviewSummaryRequest.DEFAULT_TAG_MIN_COUNT, maximum = MAX_TAG_COUNT), - "tag_threshold" to boundedNumberSchema(ReviewSummaryRequest.DEFAULT_TAG_THRESHOLD), - "tag_max_tags" to intSchema(default = ReviewSummaryRequest.DEFAULT_TAG_MAX_TAGS, maximum = MAX_TAGS), ) private fun reviewSummaryHandler(args: JsonNode): Map { val request = requestFrom(args) - return mapOf("summary" to projectSummary(reviewService.summary(request))) - } - - private fun requestFrom(args: JsonNode): ReviewSummaryRequest = - ReviewSummaryRequest( - limit = intArg(args, "limit", ReviewSummaryRequest.DEFAULT_LIMIT).coerceIn(1, MAX_LIMIT), - staleDays = intArg(args, "stale_days", ReviewSummaryRequest.DEFAULT_STALE_DAYS).coerceIn(1, MAX_STALE_DAYS), - lowConfidenceMax = - doubleArg( - args, - "low_confidence_max", - ReviewSummaryRequest.DEFAULT_LOW_CONFIDENCE_MAX, - ).coerceIn(0.0, 1.0), - highRecallMin = - intArg(args, "high_recall_min", ReviewSummaryRequest.DEFAULT_HIGH_RECALL_MIN) - .coerceAtLeast(1), - tagMinCount = intArg(args, "tag_min_count", ReviewSummaryRequest.DEFAULT_TAG_MIN_COUNT).coerceAtLeast(1), - tagThreshold = - doubleArg(args, "tag_threshold", ReviewSummaryRequest.DEFAULT_TAG_THRESHOLD) - .coerceIn(0.0, 1.0), - tagMaxTags = intArg(args, "tag_max_tags", ReviewSummaryRequest.DEFAULT_TAG_MAX_TAGS).coerceIn(1, MAX_TAGS), - ) - - private fun intArg( - args: JsonNode, - name: String, - default: Int, - ): Int = JsonArguments.optionalInt(args, name) ?: default - - private fun doubleArg( - args: JsonNode, - name: String, - default: Double, - ): Double = JsonArguments.optionalDouble(args, name) ?: default - - private fun projectSummary(summary: ReviewSummary): Map = - mapOf( - "generated_at" to summary.generatedAt.toString(), - "inbox" to projectBucket(summary.inbox, ::projectNote), - "needs_review" to projectBucket(summary.needsReview, ::projectNote), - "recent_auto_captures" to projectBucket(summary.recentAutoCaptures, ::projectNote), - "stale_unused_notes" to projectBucket(summary.staleUnusedNotes, ::projectNote), - "low_confidence_high_recall" to projectBucket(summary.lowConfidenceHighRecall, ::projectNote), - "tag_candidate_clusters" to projectBucket(summary.tagCandidateClusters, ::projectTagCluster), - "recent_audit" to projectBucket(summary.recentAudit, ::projectAuditRow), - "suggestions" to summary.suggestions.map(::projectSuggestion), - ) - - private fun projectBucket( - bucket: ReviewBucket, - project: (T) -> Map, - ): Map = - mapOf( - "total" to bucket.total, - "items" to bucket.items.map(project), - ) - - private fun projectNote(note: ReviewNote): Map = - mapOf( - "id" to note.id, - "type" to note.type, - "scope" to note.scope, - "source" to note.source, - "captured_at" to note.capturedAt.toString(), - "confidence" to note.confidence, - "title" to note.title, - "vault_path" to note.vaultPath, - "tags" to note.tags.toList().sorted(), - "recall_count" to note.recallCount, - "last_recalled_at" to note.lastRecalledAt?.toString(), - ) - - private fun projectTagCluster(cluster: TagCandidateCluster): Map = - mapOf( - "members" to cluster.members.map(::projectTagMember), - "suggested_canonical" to cluster.suggestedCanonical, - "average_similarity" to cluster.averageSimilarity, - ) - - private fun projectTagMember(member: TagCandidateMember): Map = - mapOf( - "tag" to member.tag, - "count" to member.count, - ) - - private fun projectAuditRow(row: KbAuditRow): Map = - mapOf( - "id" to row.id, - "actor" to row.actor, - "action" to row.action, - "target_id" to row.targetId, - "target_kind" to row.targetKind, - "before_json" to row.beforeJson, - "after_json" to row.afterJson, - "at" to row.at.toString(), - ) - - private fun projectSuggestion(suggestion: ReviewSuggestion): Map = - mapOf( - "kind" to suggestion.kind, - "severity" to suggestion.severity, - "message" to suggestion.message, - "suggested_tool" to suggestion.suggestedTool, - "target_id" to suggestion.targetId, - "target_kind" to suggestion.targetKind, - "details" to suggestion.details, - ) - - private fun intSchema( - default: Int, - maximum: Int, - ): Map = - mapOf( - "type" to "integer", - "minimum" to 1, - "maximum" to maximum, - "default" to default, - ) - - private fun boundedNumberSchema(default: Double): Map = - mapOf( - "type" to "number", - "minimum" to 0.0, - "maximum" to 1.0, - "default" to default, - ) - - private companion object { - const val MAX_LIMIT = 50 - const val MAX_STALE_DAYS = 365 - const val MAX_RECALL_COUNT = 1000 - const val MAX_TAG_COUNT = 1000 - const val MAX_TAGS = 500 + return mapOf("summary" to projectReviewSummary(reviewService.summary(request))) } } + +private fun requestFrom(args: JsonNode): ReviewSummaryRequest = + ReviewSummaryRequest( + limit = intArg(args, "limit", ReviewSummaryRequest.DEFAULT_LIMIT).coerceIn(1, REVIEW_MAX_LIMIT), + staleDays = + intArg(args, "stale_days", ReviewSummaryRequest.DEFAULT_STALE_DAYS) + .coerceIn(1, REVIEW_MAX_STALE_DAYS), + lowConfidenceMax = + doubleArg( + args, + "low_confidence_max", + ReviewSummaryRequest.DEFAULT_LOW_CONFIDENCE_MAX, + ).coerceIn(0.0, 1.0), + highRecallMin = + intArg(args, "high_recall_min", ReviewSummaryRequest.DEFAULT_HIGH_RECALL_MIN) + .coerceAtLeast(1), + tagMinCount = intArg(args, "tag_min_count", ReviewSummaryRequest.DEFAULT_TAG_MIN_COUNT).coerceAtLeast(1), + tagThreshold = + doubleArg(args, "tag_threshold", ReviewSummaryRequest.DEFAULT_TAG_THRESHOLD) + .coerceIn(0.0, 1.0), + tagMaxTags = + intArg(args, "tag_max_tags", ReviewSummaryRequest.DEFAULT_TAG_MAX_TAGS) + .coerceIn(1, REVIEW_MAX_TAGS), + ) + +private fun intArg( + args: JsonNode, + name: String, + default: Int, +): Int = JsonArguments.optionalInt(args, name) ?: default + +private fun doubleArg( + args: JsonNode, + name: String, + default: Double, +): Double = JsonArguments.optionalDouble(args, name) ?: default diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/queue/IngestQueueConfig.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/queue/IngestQueueConfig.kt index 04a5a05..3033e0f 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/queue/IngestQueueConfig.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/queue/IngestQueueConfig.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") package com.jorisjonkers.personalstack.knowledge.queue diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/LightRagGraphRetriever.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/LightRagGraphRetriever.kt index 032ef8b..be73b25 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/LightRagGraphRetriever.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/LightRagGraphRetriever.kt @@ -7,12 +7,15 @@ import org.springframework.http.MediaType import org.springframework.http.client.JdkClientHttpRequestFactory import org.springframework.stereotype.Component import org.springframework.web.client.RestClient +import org.springframework.web.client.RestClientException import org.springframework.web.client.body +import tools.jackson.core.JacksonException import tools.jackson.databind.json.JsonMapper import tools.jackson.module.kotlin.KotlinModule import java.net.http.HttpClient import java.security.MessageDigest import java.time.Duration +import java.util.Locale /** * LightRAG graph/context leg for `knowledge.recall(mode=deep)`. @@ -64,16 +67,24 @@ class LightRagGraphRetriever( private fun shouldQuery(query: String): Boolean = enabled && lightragUrl.isNotBlank() && query.isNotBlank() - @Suppress("TooGenericExceptionCaught") private fun queryLightRagSafely( query: String, limit: Int, ): String = try { queryLightRag(query, limit) - } catch (ex: RuntimeException) { + } catch (ex: RestClientException) { log.warn("lightrag graph recall failed; deep recall will use hybrid candidates only", ex) "" + } catch (ex: JacksonException) { + log.warn("lightrag graph recall returned invalid JSON; deep recall will use hybrid candidates only", ex) + "" + } catch (ex: IllegalStateException) { + log.warn( + "lightrag graph recall returned an incomplete response; deep recall will use hybrid candidates only", + ex, + ) + "" } private fun queryLightRag( @@ -106,7 +117,7 @@ class LightRagGraphRetriever( MessageDigest .getInstance("SHA-256") .digest(value.toByteArray()) - .joinToString("") { "%02x".format(it.toInt() and BYTE_MASK) } + .joinToString("") { "%02x".format(Locale.ROOT, it.toInt() and BYTE_MASK) } private companion object { private const val ID_HASH_CHARS = 16 diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaListwiseReranker.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaListwiseReranker.kt index f4dfa57..9c8554a 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaListwiseReranker.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaListwiseReranker.kt @@ -6,7 +6,9 @@ import org.springframework.beans.factory.annotation.Value import org.springframework.http.MediaType import org.springframework.stereotype.Component import org.springframework.web.client.RestClient +import org.springframework.web.client.RestClientException import org.springframework.web.client.body +import tools.jackson.core.JacksonException import tools.jackson.databind.json.JsonMapper import tools.jackson.module.kotlin.KotlinModule @@ -32,8 +34,6 @@ class OllamaListwiseReranker( private val baseUrl: String, @param:Value("\${knowledge.ollama.reranker-model:qwen3-reranker:0.6b}") private val rerankerModel: String, - @param:Value("\${knowledge.recall.rerank-timeout-seconds:10}") - private val timeoutSeconds: Long, ) : Reranker { private val log = LoggerFactory.getLogger(OllamaListwiseReranker::class.java) @@ -42,28 +42,36 @@ class OllamaListwiseReranker( private val client: RestClient = RestClient.builder().baseUrl(baseUrl).build() - @Suppress("TooGenericExceptionCaught", "ReturnCount") override fun rerank( query: String, candidates: List, keep: Int, - ): List { - if (candidates.isEmpty()) return emptyList() - if (candidates.size == 1) return candidates.take(keep) + ): List = + when { + candidates.isEmpty() -> emptyList() + candidates.size == 1 -> candidates.take(keep) + else -> rerankOrNull(query, candidates)?.take(keep) ?: candidates.take(keep) + } + private fun rerankOrNull( + query: String, + candidates: List, + ): List? { val raw = try { callOllama(query, candidates) - } catch (ex: RuntimeException) { + } catch (ex: RestClientException) { log.warn("rerank: ollama call failed, falling back to pre-rerank order", ex) - return candidates.take(keep) + null + } catch (ex: IllegalStateException) { + log.warn("rerank: ollama response was incomplete, falling back to pre-rerank order", ex) + null } - val ordered = parseAndValidate(raw, candidates) - if (ordered == null) { + val ordered = raw?.let { parseAndValidate(it, candidates) } + if (raw != null && ordered == null) { log.warn("rerank: response failed validation, falling back to pre-rerank order") - return candidates.take(keep) } - return ordered.take(keep) + return ordered } private fun callOllama( @@ -96,10 +104,20 @@ class OllamaListwiseReranker( * ids) of the input candidates' ids. Returns the ordered hit list * or null on any validation failure. */ - @Suppress("ReturnCount") private fun parseAndValidate( raw: String, candidates: List, + ): List? = + try { + parseAndValidateJson(raw, candidates) + } catch (ex: JacksonException) { + log.warn("rerank: response was invalid JSON, falling back to pre-rerank order", ex) + null + } + + private fun parseAndValidateJson( + raw: String, + candidates: List, ): List? { val node = mapper.readTree(raw) val content = @@ -109,17 +127,19 @@ class OllamaListwiseReranker( ?.path("message") ?.path("content") ?.asString() - ?: return null - val tokenized = - content - .lines() - .map { it.trim() } - .filter { it.isNotEmpty() } - .map { it.removeSuffix(",").removePrefix("-").trim() } - val byId = candidates.associateBy { it.id } - val ordered = tokenized.mapNotNull { byId[it] } - if (ordered.isEmpty()) return null - return ordered + .orEmpty() + return if (content.isBlank()) { + null + } else { + val tokenized = + content + .lines() + .map { it.trim() } + .filter { it.isNotEmpty() } + .map { it.removeSuffix(",").removePrefix("-").trim() } + val byId = candidates.associateBy { it.id } + tokenized.mapNotNull { byId[it] }.takeIf { it.isNotEmpty() } + } } private fun userPrompt( diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaQueryEmbedder.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaQueryEmbedder.kt index 896f258..34ad307 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaQueryEmbedder.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/OllamaQueryEmbedder.kt @@ -4,7 +4,9 @@ import org.springframework.beans.factory.annotation.Value import org.springframework.http.MediaType import org.springframework.stereotype.Component import org.springframework.web.client.RestClient +import org.springframework.web.client.RestClientException import org.springframework.web.client.body +import tools.jackson.core.JacksonException import tools.jackson.databind.json.JsonMapper import tools.jackson.module.kotlin.KotlinModule @@ -38,7 +40,18 @@ class OllamaQueryEmbedder( .baseUrl(baseUrl) .build() - override fun embed(query: String): FloatArray { + override fun embed(query: String): FloatArray = + try { + embedOrThrow(query) + } catch (ex: RestClientException) { + throw QueryEmbeddingException("Ollama embedding request failed", ex) + } catch (ex: JacksonException) { + throw QueryEmbeddingException("Ollama embedding response was not valid JSON", ex) + } catch (ex: IllegalStateException) { + throw QueryEmbeddingException("Ollama embedding response was incomplete", ex) + } + + private fun embedOrThrow(query: String): FloatArray { val raw = client .post() @@ -58,3 +71,8 @@ class OllamaQueryEmbedder( return FloatArray(embedding.size()) { i -> embedding.get(i).asDouble().toFloat() } } } + +class QueryEmbeddingException( + message: String, + cause: Throwable, +) : RuntimeException(message, cause) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallService.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallService.kt index 55c861b..b9693aa 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallService.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallService.kt @@ -8,8 +8,10 @@ import com.jorisjonkers.personalstack.knowledge.repo.EmbeddingRepository import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository import com.jorisjonkers.personalstack.knowledge.repo.RecallRepository import io.micrometer.observation.ObservationRegistry +import org.jooq.exception.DataAccessException import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Value +import org.springframework.stereotype.Component import org.springframework.stereotype.Service /** @@ -31,12 +33,8 @@ import org.springframework.stereotype.Service */ @Service class RecallService( - private val noteRepository: NoteRepository, - private val recallRepository: RecallRepository, - private val embeddingRepository: EmbeddingRepository, - private val queryEmbedder: QueryEmbedder, - private val graphRetriever: GraphRetriever, - private val reranker: Reranker, + private val stores: RecallStores, + private val enhancers: RecallEnhancers, private val observationRegistry: ObservationRegistry, @param:Value("\${knowledge.recall.default-mode:fast}") private val defaultModeWire: String, @@ -82,12 +80,11 @@ class RecallService( * indexed-lookup, sub-millisecond at our scale. A failure here * never bubbles up; the recall response already serialised. */ - @Suppress("TooGenericExceptionCaught") private fun bumpRecallStats(hits: List) { if (hits.isEmpty()) return try { - noteRepository.bumpRecallStats(hits.map { it.id }) - } catch (ex: RuntimeException) { + stores.noteRepository.bumpRecallStats(hits.map { it.id }) + } catch (ex: DataAccessException) { log.warn("recall: usage-stats bump failed (count={})", hits.size, ex) } } @@ -118,26 +115,26 @@ class RecallService( observation.lowCardinalityKeyValue("recall.rerank_used", "false") return candidates.take(limit) } - val reranked = reranker.rerank(query, candidates, keep = limit) + val reranked = enhancers.reranker.rerank(query, candidates, keep = limit) observation.lowCardinalityKeyValue("recall.rerank_used", "true") observation.highCardinalityKeyValue("recall.reranked_hits", reranked.size.toString()) return reranked } - fun getNote(id: String): KbNote? = noteRepository.findById(id) + fun getNote(id: String): KbNote? = stores.noteRepository.findById(id) fun listRecent( scope: String?, type: KbNoteType?, limit: Int, - ): List = noteRepository.listRecent(scope, type, limit) + ): List = stores.noteRepository.listRecent(scope, type, limit) - fun findConflicts(id: String): List = noteRepository.findConflicts(id) + fun findConflicts(id: String): List = stores.noteRepository.findConflicts(id) fun walkRelations( id: String, depth: Int, - ): List = noteRepository.walkRelations(id, depth) + ): List = stores.noteRepository.walkRelations(id, depth) // -------- mode implementations -------- @@ -147,13 +144,12 @@ class RecallService( limit: Int, observation: io.micrometer.observation.Observation, ): List { - val hits = recallRepository.recall(query, scope, limit) + val hits = stores.recallRepository.recall(query, scope, limit) observation.highCardinalityKeyValue("recall.fts_hits", hits.size.toString()) observation.highCardinalityKeyValue("recall.vector_hits", "0") return hits } - @Suppress("TooGenericExceptionCaught") private fun recallHybrid( query: String, scope: String?, @@ -162,19 +158,25 @@ class RecallService( ): List { // The FTS leg is the floor — if it explodes there is nothing to // fall back to, so let the exception propagate. - val ftsHits = recallRepository.recall(query, scope, limit * VECTOR_OVERFETCH_FACTOR) + val ftsHits = stores.recallRepository.recall(query, scope, limit * VECTOR_OVERFETCH_FACTOR) observation.highCardinalityKeyValue("recall.fts_hits", ftsHits.size.toString()) val vectorHits = try { - val embedding = queryEmbedder.embed(query) - embeddingRepository.recallVector(embedding, scope, limit * VECTOR_OVERFETCH_FACTOR) - } catch (ex: RuntimeException) { + val embedding = enhancers.queryEmbedder.embed(query) + stores.embeddingRepository.recallVector(embedding, scope, limit * VECTOR_OVERFETCH_FACTOR) + } catch (ex: QueryEmbeddingException) { // Embedder or vector repo failed — log + count, don't 500 // the read path. The FTS leg alone still produces an answer. - // Narrow to RuntimeException so genuine programming errors - // (Errors, ThreadDeath, …) still propagate; RestClient and - // JDBC both wrap their failures as RuntimeException subtypes. + log.warn( + "vector leg degraded; falling back to FTS-only (mode=hybrid, scope={}, query.len={})", + scope, + query.length, + ex, + ) + observation.lowCardinalityKeyValue("recall.degraded", "vector") + emptyList() + } catch (ex: DataAccessException) { log.warn( "vector leg degraded; falling back to FTS-only (mode=hybrid, scope={}, query.len={})", scope, @@ -196,7 +198,6 @@ class RecallService( return fused } - @Suppress("TooGenericExceptionCaught") private fun recallGraph( query: String, scope: String?, @@ -204,8 +205,8 @@ class RecallService( observation: io.micrometer.observation.Observation, ): List = try { - graphRetriever.retrieve(query, scope, limit) - } catch (ex: RuntimeException) { + enhancers.graphRetriever.retrieve(query, scope, limit) + } catch (ex: DataAccessException) { log.warn("graph leg degraded; falling back to hybrid-only deep recall", ex) observation.lowCardinalityKeyValue("recall.graph_degraded", "true") emptyList() @@ -234,3 +235,17 @@ class RecallService( private const val RERANK_OVERFETCH_FACTOR = 2 } } + +@Component +class RecallStores( + val noteRepository: NoteRepository, + val recallRepository: RecallRepository, + val embeddingRepository: EmbeddingRepository, +) + +@Component +class RecallEnhancers( + val queryEmbedder: QueryEmbedder, + val graphRetriever: GraphRetriever, + val reranker: Reranker, +) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/AuditRepository.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/AuditRepository.kt index 3205c9e..196f8e0 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/AuditRepository.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/AuditRepository.kt @@ -33,37 +33,29 @@ class AuditRepository( * wants to log "wrote audit " doesn't need a follow-up * SELECT. */ - fun record( - actor: String, - action: String, - targetId: String? = null, - targetKind: String? = null, - beforeJson: String? = null, - afterJson: String? = null, - now: Instant = Instant.now(), - ): KbAuditRow { + fun record(request: AuditRecordRequest): KbAuditRow { val id = Ulid.generate() - val ts = now.atOffset(ZoneOffset.UTC).toLocalDateTime() + val ts = request.now.atOffset(ZoneOffset.UTC).toLocalDateTime() dsl .insertInto(KB_AUDIT) .set(KB_AUDIT.ID, id) - .set(KB_AUDIT.ACTOR, actor) - .set(KB_AUDIT.ACTION, action) - .set(KB_AUDIT.TARGET_ID, targetId) - .set(KB_AUDIT.TARGET_KIND, targetKind) - .set(KB_AUDIT.BEFORE_JSON, beforeJson) - .set(KB_AUDIT.AFTER_JSON, afterJson) + .set(KB_AUDIT.ACTOR, request.actor) + .set(KB_AUDIT.ACTION, request.action) + .set(KB_AUDIT.TARGET_ID, request.targetId) + .set(KB_AUDIT.TARGET_KIND, request.targetKind) + .set(KB_AUDIT.BEFORE_JSON, request.beforeJson) + .set(KB_AUDIT.AFTER_JSON, request.afterJson) .set(KB_AUDIT.AT, ts) .execute() return KbAuditRow( id = id, - actor = actor, - action = action, - targetId = targetId, - targetKind = targetKind, - beforeJson = beforeJson, - afterJson = afterJson, - at = now, + actor = request.actor, + action = request.action, + targetId = request.targetId, + targetKind = request.targetKind, + beforeJson = request.beforeJson, + afterJson = request.afterJson, + at = request.now, ) } @@ -114,3 +106,13 @@ class AuditRepository( at = record.at?.toInstant(ZoneOffset.UTC) ?: Instant.EPOCH, ) } + +data class AuditRecordRequest( + val actor: String, + val action: String, + val targetId: String? = null, + val targetKind: String? = null, + val beforeJson: String? = null, + val afterJson: String? = null, + val now: Instant = Instant.now(), +) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/JooqQueryBinds.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/JooqQueryBinds.kt index 2dda7d6..338afb1 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/JooqQueryBinds.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/JooqQueryBinds.kt @@ -3,8 +3,7 @@ package com.jorisjonkers.personalstack.knowledge.repo import org.jooq.DSLContext import org.jooq.ResultQuery -@Suppress("SpreadOperator") internal fun DSLContext.resultQueryWithBinds( sql: String, binds: List, -): ResultQuery = resultQuery(sql, *binds.toTypedArray()) +): ResultQuery = JooqQueryBindsSupport.resultQueryWithBinds(this, sql, binds) diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/NoteRepository.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/NoteRepository.kt index 3c2b1a0..550a8ef 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/NoteRepository.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/NoteRepository.kt @@ -8,14 +8,15 @@ import com.jorisjonkers.personalstack.knowledge.jooq.tables.KbNotes.KB_NOTES import com.jorisjonkers.personalstack.knowledge.jooq.tables.KbRelations.KB_RELATIONS import com.jorisjonkers.personalstack.knowledge.jooq.tables.records.KbNotesRecord import org.jooq.DSLContext +import org.slf4j.LoggerFactory import org.springframework.stereotype.Repository +import tools.jackson.core.JacksonException import tools.jackson.databind.json.JsonMapper import tools.jackson.module.kotlin.KotlinModule import java.time.Instant import java.time.ZoneOffset @Repository -@Suppress("TooManyFunctions") // Repository methods intentionally mirror MCP/read-model operations. class NoteRepository( private val dsl: DSLContext, ) { @@ -100,7 +101,7 @@ class NoteRepository( // Bulk-load tags for the returned ids in one query rather than // N+1. val ids = records.map { it.id ?: "" }.filter { it.isNotBlank() } - val tagMap = bulkTags(ids) + val tagMap = bulkTags(dsl, ids) return records.map { it.toDomain(tagMap[it.id].orEmpty()) } } @@ -154,43 +155,13 @@ class NoteRepository( val collected = mutableListOf() repeat(effectiveDepth) { if (frontier.isNotEmpty() && collected.size < MAX_ROWS) { - val edges = fetchEdgesTouching(frontier) + val edges = fetchEdgesTouching(dsl, frontier) frontier = absorbEdges(edges, collected, visited) } } return collected } - private fun fetchEdgesTouching(ids: Set): List = - dsl - .selectFrom(KB_RELATIONS) - .where(KB_RELATIONS.SUBJECT_ID.`in`(ids).or(KB_RELATIONS.OBJECT_ID.`in`(ids))) - .fetch() - .map { record -> - KbRelation( - subjectId = record.subjectId ?: "", - predicate = record.predicate ?: "", - objectId = record.objectId ?: "", - props = parseProps(record.props), - createdAt = record.createdAt?.toInstant(ZoneOffset.UTC) ?: Instant.EPOCH, - ) - } - - private fun absorbEdges( - edges: List, - collected: MutableList, - visited: MutableSet, - ): Set { - val nextFrontier = mutableSetOf() - for (edge in edges) { - if (collected.size >= MAX_ROWS) break - collected += edge - if (visited.add(edge.subjectId)) nextFrontier += edge.subjectId - if (visited.add(edge.objectId)) nextFrontier += edge.objectId - } - return nextFrontier - } - /** * Insert helper for tests + the soon-to-arrive `link` tool. Keeps * `kb_relations.props` as a JSON-encoded TEXT column (the V1 schema @@ -211,35 +182,6 @@ class NoteRepository( .execute() } - // -------- helpers -------- - - private fun bulkTags(ids: List): Map> { - if (ids.isEmpty()) return emptyMap() - return dsl - .select(KB_NOTE_TAGS.NOTE_ID, KB_NOTE_TAGS.TAG) - .from(KB_NOTE_TAGS) - .where(KB_NOTE_TAGS.NOTE_ID.`in`(ids)) - .fetch() - .groupBy({ it.value1() ?: "" }, { it.value2() ?: "" }) - .mapValues { (_, tags) -> tags.filter { it.isNotBlank() }.toSet() } - } - - private fun KbNotesRecord.toDomain(tags: Set): KbNote = - KbNote( - id = id ?: error("kb_notes row missing id"), - type = KbNoteType.fromWire(type ?: error("kb_notes row missing type")), - scope = scope ?: "", - source = source ?: "", - capturedAt = capturedAt?.toInstant(ZoneOffset.UTC) ?: Instant.EPOCH, - sessionId = sessionId, - confidence = confidence?.toDouble() ?: 0.0, - title = title ?: "", - body = body ?: "", - vaultPath = vaultPath ?: "", - vaultCommit = vaultCommit, - tags = tags, - ) - /** * Bump `recall_count` + `last_recalled_at` on one or more rows. * Used by the recall path as a fire-and-forget UPDATE so the @@ -282,25 +224,92 @@ class NoteRepository( "WHERE id = ?", id, ) +} - @Suppress("UNCHECKED_CAST", "SwallowedException") - private fun parseProps(raw: String?): Map { - if (raw.isNullOrBlank()) return emptyMap() - return try { - jsonMapper.readValue(raw, Map::class.java) as Map - } catch (_: Exception) { - emptyMap() +private val noteRepositoryLog = LoggerFactory.getLogger(NoteRepository::class.java) +private val relationPropsJsonMapper: JsonMapper = + JsonMapper.builder().addModule(KotlinModule.Builder().build()).build() + +private val CONFLICT_PREDICATES = listOf("supersedes", "contradicts") + +// Bounds for `walkRelations`. The depth cap stops a runaway graph +// walk; the row cap keeps the agent-facing response from blowing the +// context budget on a hub note with thousands of incoming edges. +private const val MAX_DEPTH = 4 +private const val MAX_ROWS = 200 + +private fun fetchEdgesTouching( + dsl: DSLContext, + ids: Set, +): List = + dsl + .selectFrom(KB_RELATIONS) + .where(KB_RELATIONS.SUBJECT_ID.`in`(ids).or(KB_RELATIONS.OBJECT_ID.`in`(ids))) + .fetch() + .map { record -> + KbRelation( + subjectId = record.subjectId ?: "", + predicate = record.predicate ?: "", + objectId = record.objectId ?: "", + props = parseProps(record.props), + createdAt = record.createdAt?.toInstant(ZoneOffset.UTC) ?: Instant.EPOCH, + ) } + +private fun absorbEdges( + edges: List, + collected: MutableList, + visited: MutableSet, +): Set { + val nextFrontier = mutableSetOf() + for (edge in edges) { + if (collected.size >= MAX_ROWS) break + collected += edge + if (visited.add(edge.subjectId)) nextFrontier += edge.subjectId + if (visited.add(edge.objectId)) nextFrontier += edge.objectId } + return nextFrontier +} + +private fun bulkTags( + dsl: DSLContext, + ids: List, +): Map> { + if (ids.isEmpty()) return emptyMap() + return dsl + .select(KB_NOTE_TAGS.NOTE_ID, KB_NOTE_TAGS.TAG) + .from(KB_NOTE_TAGS) + .where(KB_NOTE_TAGS.NOTE_ID.`in`(ids)) + .fetch() + .groupBy({ it.value1() ?: "" }, { it.value2() ?: "" }) + .mapValues { (_, tags) -> tags.filter { it.isNotBlank() }.toSet() } +} - companion object { - private val CONFLICT_PREDICATES = listOf("supersedes", "contradicts") +private fun KbNotesRecord.toDomain(tags: Set): KbNote = + KbNote( + id = id ?: error("kb_notes row missing id"), + type = KbNoteType.fromWire(type ?: error("kb_notes row missing type")), + scope = scope ?: "", + source = source ?: "", + capturedAt = capturedAt?.toInstant(ZoneOffset.UTC) ?: Instant.EPOCH, + sessionId = sessionId, + confidence = confidence?.toDouble() ?: 0.0, + title = title ?: "", + body = body ?: "", + vaultPath = vaultPath ?: "", + vaultCommit = vaultCommit, + tags = tags, + ) - // Bounds for `walkRelations`. The depth cap stops a runaway - // graph walk; the row cap keeps the agent-facing response - // from blowing the context budget on a hub note with - // thousands of incoming edges. - private const val MAX_DEPTH = 4 - private const val MAX_ROWS = 200 +private fun parseProps(raw: String?): Map { + if (raw.isNullOrBlank()) return emptyMap() + return try { + relationPropsJsonMapper + .readValue(raw, Map::class.java) + .entries + .associate { (key, value) -> key.toString() to value } + } catch (ex: JacksonException) { + noteRepositoryLog.warn("kb_relation props were not valid JSON", ex) + emptyMap() } } diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/ReviewRepository.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/ReviewRepository.kt index aa5052b..0a40aa9 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/ReviewRepository.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/repo/ReviewRepository.kt @@ -14,12 +14,12 @@ import java.time.Instant import java.time.ZoneOffset @Repository -@Suppress("TooManyFunctions") // Review buckets are a single query surface for the governance summary. class ReviewRepository( private val dsl: DSLContext, ) { fun listInbox(limit: Int): ReviewBucket = - listNotes( + listReviewNotes( + dsl = dsl, condition = KB_NOTES.SCOPE .eq(INBOX_SCOPE) @@ -29,7 +29,8 @@ class ReviewRepository( ) fun listNeedsReview(limit: Int): ReviewBucket = - listNotes( + listReviewNotes( + dsl = dsl, condition = KB_NOTES.SCOPE .eq(INBOX_SCOPE) @@ -39,7 +40,8 @@ class ReviewRepository( ) fun listRecentAutoCaptures(limit: Int): ReviewBucket = - listNotes( + listReviewNotes( + dsl = dsl, condition = autoCaptureCondition(), limit = limit, orderBy = listOf(KB_NOTES.ID.desc()), @@ -49,7 +51,8 @@ class ReviewRepository( olderThan: Instant, limit: Int, ): ReviewBucket = - listNotes( + listReviewNotes( + dsl = dsl, condition = curatedCondition() .and(KB_NOTES.RECALL_COUNT.eq(0)) @@ -64,7 +67,8 @@ class ReviewRepository( minRecallCount: Int, limit: Int, ): ReviewBucket = - listNotes( + listReviewNotes( + dsl = dsl, condition = curatedCondition() .and(KB_NOTES.CONFIDENCE.le(maxConfidence.toFloat())) @@ -72,89 +76,91 @@ class ReviewRepository( limit = limit, orderBy = listOf(KB_NOTES.RECALL_COUNT.desc(), KB_NOTES.ID.desc()), ) +} - private fun listNotes( - condition: Condition, - limit: Int, - orderBy: List>, - ): ReviewBucket { - val total = - dsl - .selectCount() - .from(KB_NOTES) - .where(condition) - .fetchOne(0, Int::class.java) ?: 0 - val records = - dsl - .selectFrom(KB_NOTES) - .where(condition) - .orderBy(orderBy) - .limit(limit) - .fetch() - val ids = records.mapNotNull { it.id }.filter { it.isNotBlank() } - val tagMap = bulkTags(ids) - return ReviewBucket(total = total, items = records.map { it.toReviewNote(tagMap[it.id].orEmpty()) }) - } - - private fun bulkTags(ids: List): Map> { - if (ids.isEmpty()) return emptyMap() - return dsl - .select(KB_NOTE_TAGS.NOTE_ID, KB_NOTE_TAGS.TAG) - .from(KB_NOTE_TAGS) - .where(KB_NOTE_TAGS.NOTE_ID.`in`(ids)) +private fun listReviewNotes( + dsl: DSLContext, + condition: Condition, + limit: Int, + orderBy: List>, +): ReviewBucket { + val total = + dsl + .selectCount() + .from(KB_NOTES) + .where(condition) + .fetchOne(0, Int::class.java) ?: 0 + val records = + dsl + .selectFrom(KB_NOTES) + .where(condition) + .orderBy(orderBy) + .limit(limit) .fetch() - .groupBy({ it.value1() ?: "" }, { it.value2() ?: "" }) - .mapValues { (_, tags) -> tags.filter { it.isNotBlank() }.toSet() } - } + val ids = records.mapNotNull { it.id }.filter { it.isNotBlank() } + val tagMap = bulkReviewTags(dsl, ids) + return ReviewBucket(total = total, items = records.map { it.toReviewNote(tagMap[it.id].orEmpty()) }) +} - private fun autoCaptureCondition(): Condition { - val autoTag = - DSL - .selectOne() - .from(KB_NOTE_TAGS) - .where(KB_NOTE_TAGS.NOTE_ID.eq(KB_NOTES.ID)) - .and(KB_NOTE_TAGS.TAG.`in`(AUTO_CAPTURE_TAGS)) - return KB_NOTES.SOURCE - .like("%auto%") - .or(DSL.exists(autoTag)) - } +private fun bulkReviewTags( + dsl: DSLContext, + ids: List, +): Map> { + if (ids.isEmpty()) return emptyMap() + return dsl + .select(KB_NOTE_TAGS.NOTE_ID, KB_NOTE_TAGS.TAG) + .from(KB_NOTE_TAGS) + .where(KB_NOTE_TAGS.NOTE_ID.`in`(ids)) + .fetch() + .groupBy({ it.value1() ?: "" }, { it.value2() ?: "" }) + .mapValues { (_, tags) -> tags.filter { it.isNotBlank() }.toSet() } +} - private fun curatedCondition(): Condition = - KB_NOTES.SCOPE - .ne(INBOX_SCOPE) - .and(KB_NOTES.SCOPE.ne(NEEDS_REVIEW_SCOPE)) +private fun autoCaptureCondition(): Condition { + val autoTag = + DSL + .selectOne() + .from(KB_NOTE_TAGS) + .where(KB_NOTE_TAGS.NOTE_ID.eq(KB_NOTES.ID)) + .and(KB_NOTE_TAGS.TAG.`in`(AUTO_CAPTURE_TAGS)) + return KB_NOTES.SOURCE + .like("%auto%") + .or(DSL.exists(autoTag)) +} - private fun needsReviewPathCondition(): Condition = - KB_NOTES - .VAULT_PATH - .like("${likeLiteral(NEEDS_REVIEW_PREFIX)}%", LIKE_ESCAPE) +private fun curatedCondition(): Condition = + KB_NOTES.SCOPE + .ne(INBOX_SCOPE) + .and(KB_NOTES.SCOPE.ne(NEEDS_REVIEW_SCOPE)) - private fun likeLiteral(value: String): String = - value - .replace(LIKE_ESCAPE.toString(), "$LIKE_ESCAPE$LIKE_ESCAPE") - .replace("%", "$LIKE_ESCAPE%") - .replace("_", "${LIKE_ESCAPE}_") +private fun needsReviewPathCondition(): Condition = + KB_NOTES + .VAULT_PATH + .like("${likeLiteral(NEEDS_REVIEW_PREFIX)}%", LIKE_ESCAPE) - private fun KbNotesRecord.toReviewNote(tags: Set): ReviewNote = - ReviewNote( - id = id ?: error("kb_notes row missing id"), - type = type ?: "", - scope = scope ?: "", - source = source ?: "", - capturedAt = capturedAt?.toInstant(ZoneOffset.UTC) ?: Instant.EPOCH, - confidence = confidence?.toDouble() ?: 0.0, - title = title ?: "", - vaultPath = vaultPath ?: "", - tags = tags, - recallCount = recallCount ?: 0, - lastRecalledAt = lastRecalledAt?.toInstant(ZoneOffset.UTC), - ) +private fun likeLiteral(value: String): String = + value + .replace(LIKE_ESCAPE.toString(), "$LIKE_ESCAPE$LIKE_ESCAPE") + .replace("%", "$LIKE_ESCAPE%") + .replace("_", "${LIKE_ESCAPE}_") - private companion object { - const val INBOX_SCOPE = "_inbox" - const val NEEDS_REVIEW_SCOPE = "_needs-review" - const val NEEDS_REVIEW_PREFIX = "_inbox/_needs-review/" - const val LIKE_ESCAPE = '!' - val AUTO_CAPTURE_TAGS = listOf("auto-capture", "auto-memory", "auto-digest") - } -} +private fun KbNotesRecord.toReviewNote(tags: Set): ReviewNote = + ReviewNote( + id = id ?: error("kb_notes row missing id"), + type = type ?: "", + scope = scope ?: "", + source = source ?: "", + capturedAt = capturedAt?.toInstant(ZoneOffset.UTC) ?: Instant.EPOCH, + confidence = confidence?.toDouble() ?: 0.0, + title = title ?: "", + vaultPath = vaultPath ?: "", + tags = tags, + recallCount = recallCount ?: 0, + lastRecalledAt = lastRecalledAt?.toInstant(ZoneOffset.UTC), + ) + +private const val INBOX_SCOPE = "_inbox" +private const val NEEDS_REVIEW_SCOPE = "_needs-review" +private const val NEEDS_REVIEW_PREFIX = "_inbox/_needs-review/" +private const val LIKE_ESCAPE = '!' +private val AUTO_CAPTURE_TAGS = listOf("auto-capture", "auto-memory", "auto-digest") diff --git a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeReviewController.kt b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeReviewController.kt index 7954f5d..6509e7f 100644 --- a/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeReviewController.kt +++ b/api/src/main/kotlin/com/jorisjonkers/personalstack/knowledge/web/KnowledgeReviewController.kt @@ -2,9 +2,12 @@ package com.jorisjonkers.personalstack.knowledge.web import com.jorisjonkers.personalstack.knowledge.review.ReviewService import com.jorisjonkers.personalstack.knowledge.review.ReviewSummaryRequest +import io.swagger.v3.oas.annotations.Operation +import io.swagger.v3.oas.annotations.Parameter +import io.swagger.v3.oas.annotations.media.Schema +import jakarta.servlet.http.HttpServletRequest import org.springframework.web.bind.annotation.GetMapping import org.springframework.web.bind.annotation.RequestMapping -import org.springframework.web.bind.annotation.RequestParam import org.springframework.web.bind.annotation.RestController /** @@ -18,44 +21,77 @@ class KnowledgeReviewController( private val reviewService: ReviewService, ) { @GetMapping("/summary") - fun summary( - @RequestParam("limit", defaultValue = "${ReviewSummaryRequest.DEFAULT_LIMIT}") limit: Int, - @RequestParam("stale_days", defaultValue = "${ReviewSummaryRequest.DEFAULT_STALE_DAYS}") staleDays: Int, - @RequestParam( - "low_confidence_max", - defaultValue = "${ReviewSummaryRequest.DEFAULT_LOW_CONFIDENCE_MAX}", - ) lowConfidenceMax: Double, - @RequestParam( - "high_recall_min", - defaultValue = "${ReviewSummaryRequest.DEFAULT_HIGH_RECALL_MIN}", - ) highRecallMin: Int, - @RequestParam( - "tag_min_count", - defaultValue = "${ReviewSummaryRequest.DEFAULT_TAG_MIN_COUNT}", - ) tagMinCount: Int, - @RequestParam( - "tag_threshold", - defaultValue = "${ReviewSummaryRequest.DEFAULT_TAG_THRESHOLD}", - ) tagThreshold: Double, - @RequestParam( - "tag_max_tags", - defaultValue = "${ReviewSummaryRequest.DEFAULT_TAG_MAX_TAGS}", - ) tagMaxTags: Int, - ): ReviewSummaryResponse = + @Operation( + parameters = [ + Parameter( + name = "limit", + schema = Schema(type = "integer", format = "int32", defaultValue = "10"), + ), + Parameter( + name = "stale_days", + schema = Schema(type = "integer", format = "int32", defaultValue = "60"), + ), + Parameter( + name = "low_confidence_max", + schema = Schema(type = "number", format = "double", defaultValue = "0.6"), + ), + Parameter( + name = "high_recall_min", + schema = Schema(type = "integer", format = "int32", defaultValue = "3"), + ), + Parameter( + name = "tag_min_count", + schema = Schema(type = "integer", format = "int32", defaultValue = "1"), + ), + Parameter( + name = "tag_threshold", + schema = Schema(type = "number", format = "double", defaultValue = "0.85"), + ), + Parameter( + name = "tag_max_tags", + schema = Schema(type = "integer", format = "int32", defaultValue = "200"), + ), + ], + ) + fun summary(request: HttpServletRequest): ReviewSummaryResponse = ReviewSummaryResponse.from( reviewService.summary( ReviewSummaryRequest( - limit = limit.coerceIn(1, MAX_LIMIT), - staleDays = staleDays.coerceIn(1, MAX_STALE_DAYS), - lowConfidenceMax = lowConfidenceMax.coerceIn(0.0, 1.0), - highRecallMin = highRecallMin.coerceAtLeast(1), - tagMinCount = tagMinCount.coerceAtLeast(1), - tagThreshold = tagThreshold.coerceIn(0.0, 1.0), - tagMaxTags = tagMaxTags.coerceIn(1, MAX_TAGS), + limit = intParam(request, "limit", ReviewSummaryRequest.DEFAULT_LIMIT).coerceIn(1, MAX_LIMIT), + staleDays = + intParam(request, "stale_days", ReviewSummaryRequest.DEFAULT_STALE_DAYS) + .coerceIn(1, MAX_STALE_DAYS), + lowConfidenceMax = + doubleParam(request, "low_confidence_max", ReviewSummaryRequest.DEFAULT_LOW_CONFIDENCE_MAX) + .coerceIn(0.0, 1.0), + highRecallMin = + intParam(request, "high_recall_min", ReviewSummaryRequest.DEFAULT_HIGH_RECALL_MIN) + .coerceAtLeast(1), + tagMinCount = + intParam(request, "tag_min_count", ReviewSummaryRequest.DEFAULT_TAG_MIN_COUNT) + .coerceAtLeast(1), + tagThreshold = + doubleParam(request, "tag_threshold", ReviewSummaryRequest.DEFAULT_TAG_THRESHOLD) + .coerceIn(0.0, 1.0), + tagMaxTags = + intParam(request, "tag_max_tags", ReviewSummaryRequest.DEFAULT_TAG_MAX_TAGS) + .coerceIn(1, MAX_TAGS), ), ), ) + private fun intParam( + request: HttpServletRequest, + name: String, + default: Int, + ): Int = request.getParameter(name)?.toIntOrNull() ?: default + + private fun doubleParam( + request: HttpServletRequest, + name: String, + default: Double, + ): Double = request.getParameter(name)?.toDoubleOrNull() ?: default + private companion object { const val MAX_LIMIT = 50 const val MAX_STALE_DAYS = 365 diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestServiceTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestServiceTest.kt index 38d4f9f..faabdb3 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestServiceTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/digest/DigestServiceTest.kt @@ -126,7 +126,7 @@ class DigestServiceTest { @Test fun `digest tolerates an Ollama call failure and returns empty`() { fixture() - every { ollama.chatJson(any(), any(), any()) } throws RuntimeException("connect timeout") + every { ollama.chatJson(any(), any(), any()) } throws IllegalStateException("connect timeout") val out = service.digest("transcript", 5, 0.0) assertThat(out).isEmpty() } diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpToolsTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpToolsTest.kt new file mode 100644 index 0000000..99d4fce --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/DiscoveryMcpToolsTest.kt @@ -0,0 +1,216 @@ +package com.jorisjonkers.personalstack.knowledge.mcp + +import com.jorisjonkers.personalstack.knowledge.audit.AuditService +import com.jorisjonkers.personalstack.knowledge.auth.AdminAuthorization +import com.jorisjonkers.personalstack.knowledge.capture.CaptureService +import com.jorisjonkers.personalstack.knowledge.digest.DigestService +import com.jorisjonkers.personalstack.knowledge.discovery.DiscoveryService +import com.jorisjonkers.personalstack.knowledge.discovery.TagClusterService +import com.jorisjonkers.personalstack.knowledge.domain.KbNote +import com.jorisjonkers.personalstack.knowledge.domain.KbNoteType +import com.jorisjonkers.personalstack.knowledge.domain.ScopeSummary +import com.jorisjonkers.personalstack.knowledge.domain.SourceSummary +import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateCluster +import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateMember +import com.jorisjonkers.personalstack.knowledge.domain.TagSummary +import com.jorisjonkers.personalstack.knowledge.domain.TopicStats +import com.jorisjonkers.personalstack.knowledge.domain.TopicSummary +import com.jorisjonkers.personalstack.knowledge.recall.RecallService +import com.jorisjonkers.personalstack.knowledge.repo.AuditRepository +import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository +import com.jorisjonkers.personalstack.knowledge.repo.TopicRepository +import com.jorisjonkers.personalstack.knowledge.review.ReviewService +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import tools.jackson.databind.json.JsonMapper +import tools.jackson.module.kotlin.KotlinModule +import java.time.Instant + +class DiscoveryMcpToolsTest { + private val discoveryService = mockk(relaxed = true) + private val tagClusterService = mockk(relaxed = true) + private val tools = discoveryTools(discoveryService, tagClusterService) + private val mapper: JsonMapper = JsonMapper.builder().addModule(KotlinModule.Builder().build()).build() + + private val stubNote = + KbNote( + id = "01HXYZ00000000000000000000", + type = KbNoteType.LESSON, + scope = "personal", + source = "claude-code", + capturedAt = Instant.parse("2026-05-13T12:00:00Z"), + sessionId = null, + confidence = 0.4, + title = "title", + body = "body", + vaultPath = "personal/lesson/draft.md", + vaultCommit = null, + ) + + @Test + fun `list_tag_candidates projects each cluster with its members and canonical`() { + every { tagClusterService.listTagCandidates(any(), any(), any()) } returns + listOf( + TagCandidateCluster( + members = listOf(TagCandidateMember("kotlin", 10), TagCandidateMember("Kotlin", 3)), + suggestedCanonical = "kotlin", + averageSimilarity = 0.97, + ), + ) + + val out = + tools.call( + "knowledge.list_tag_candidates", + mapper.readTree("""{"min_count":2,"threshold":0.9,"max_tags":50}"""), + )!! + + val clusters = out["clusters"].stringKeyMapList() + assertThat(clusters).hasSize(1) + assertThat(clusters[0]["suggested_canonical"]).isEqualTo("kotlin") + assertThat(clusters[0]["average_similarity"]).isEqualTo(0.97) + + val members = clusters[0]["members"].stringKeyMapList() + assertThat(members.map { it["tag"] }).containsExactly("kotlin", "Kotlin") + assertThat(members.map { it["count"] }).containsExactly(10, 3) + } + + @Test + fun `list_topics projects each summary with the bare slug`() { + every { discoveryService.listTopics(5) } returns + listOf( + TopicSummary( + slug = "kotlin", + noteCount = 12, + lastCapturedAt = Instant.parse("2026-05-13T12:00:00Z"), + ), + TopicSummary(slug = "postgres", noteCount = 3, lastCapturedAt = null), + ) + + val out = tools.call("knowledge.list_topics", mapper.readTree("""{"limit":5}"""))!! + + val rows = out["topics"].stringKeyMapList() + assertThat(rows).hasSize(2) + assertThat(rows[0]["slug"]).isEqualTo("kotlin") + assertThat(rows[0]["note_count"]).isEqualTo(12) + assertThat(rows[0]["last_captured_at"]).isEqualTo("2026-05-13T12:00:00Z") + assertThat(rows[1]["last_captured_at"]).isNull() + } + + @Test + fun `list_tags forwards the optional scope filter`() { + every { discoveryService.listTags("project:personal-stack", 50) } returns + listOf( + TagSummary( + tag = "kotlin", + count = 7, + lastUsedAt = Instant.parse("2026-05-13T12:00:00Z"), + ), + ) + tools.call( + "knowledge.list_tags", + mapper.readTree("""{"scope":"project:personal-stack","limit":50}"""), + ) + verify(exactly = 1) { discoveryService.listTags("project:personal-stack", 50) } + } + + @Test + fun `list_scopes returns counts ordered by note_count desc`() { + every { discoveryService.listScopes(any()) } returns + listOf( + ScopeSummary(scope = "project:a", noteCount = 5, lastCapturedAt = null), + ScopeSummary(scope = "personal", noteCount = 1, lastCapturedAt = null), + ) + val out = tools.call("knowledge.list_scopes", mapper.readTree("""{}"""))!! + + val rows = out["scopes"].stringKeyMapList() + assertThat(rows.map { it["scope"] }).containsExactly("project:a", "personal") + } + + @Test + fun `list_sources projects the source frequency`() { + every { discoveryService.listSources(any()) } returns + listOf(SourceSummary(source = "claude-code", count = 42)) + val out = tools.call("knowledge.list_sources", mapper.readTree("""{}"""))!! + val rows = out["sources"].stringKeyMapList() + assertThat(rows).hasSize(1) + assertThat(rows[0]["source"]).isEqualTo("claude-code") + assertThat(rows[0]["count"]).isEqualTo(42) + } + + @Test + fun `topic_stats returns null when the slug has no notes`() { + every { discoveryService.topicStats("nonexistent", any()) } returns null + val out = tools.call("knowledge.topic_stats", mapper.readTree("""{"slug":"nonexistent"}"""))!! + assertThat(out["stats"]).isNull() + } + + @Test + fun `topic_stats projects the breakdowns when the slug has notes`() { + every { discoveryService.topicStats("kotlin", 10) } returns topicStats() + + val out = tools.call("knowledge.topic_stats", mapper.readTree("""{"slug":"kotlin"}"""))!! + val stats = out["stats"].stringKeyMap() + assertThat(stats["slug"]).isEqualTo("kotlin") + assertThat(stats["note_count"]).isEqualTo(12) + assertThat(stats["type_breakdown"].stringKeyMap()["lesson"]).isEqualTo(8) + val topTags = stats["top_tags"].stringKeyMapList() + assertThat(topTags).hasSize(1) + assertThat(topTags[0]["tag"]).isEqualTo("spring") + } + + @Test + fun `list_inbox surfaces vault_path so the operator can find the file`() { + every { discoveryService.listInbox(20) } returns + listOf(stubNote.copy(vaultPath = "_inbox/2026-05-13/hello.md", scope = "_inbox")) + val out = tools.call("knowledge.list_inbox", mapper.readTree("""{}"""))!! + val notes = out["notes"].stringKeyMapList() + assertThat(notes).hasSize(1) + assertThat(notes[0]["vault_path"]).isEqualTo("_inbox/2026-05-13/hello.md") + assertThat(notes[0]["scope"]).isEqualTo("_inbox") + } + + private fun topicStats(): TopicStats = + TopicStats( + slug = "kotlin", + noteCount = 12, + firstCapturedAt = Instant.parse("2026-05-10T12:00:00Z"), + lastCapturedAt = Instant.parse("2026-05-13T12:00:00Z"), + typeBreakdown = mapOf("lesson" to 8, "decision" to 4), + topTags = + listOf( + TagSummary( + tag = "spring", + count = 5, + lastUsedAt = Instant.parse("2026-05-13T12:00:00Z"), + ), + ), + ) + + private fun discoveryTools( + discoveryService: DiscoveryService, + tagClusterService: TagClusterService, + ): McpTools = + McpTools( + coreTools = + CoreMcpToolSet( + CaptureMcpTools(mockk(relaxed = true)), + ReadMcpTools(mockk(relaxed = true)), + ), + fullTools = + FullMcpToolSet( + DiscoveryMcpTools(discoveryService, tagClusterService), + AdminMcpTools( + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + ), + DigestMcpTools(mockk(relaxed = true)), + AuditMcpTools(mockk(relaxed = true)), + ReviewMcpTools(mockk(relaxed = true)), + ), + ) +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerTest.kt index f497f41..8075657 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpControllerTest.kt @@ -58,8 +58,7 @@ class McpControllerTest { val body = response.body!! assertThat(body.id).isEqualTo(request.id) assertThat(body.error).isNull() - @Suppress("UNCHECKED_CAST") - val result = body.result as Map + val result = body.result.stringKeyMap() assertThat(result["tools"]).isEqualTo(emptyList()) } @@ -78,4 +77,12 @@ class McpControllerTest { val body = response.body!! assertThat(body.error?.code).isEqualTo(JsonRpcErrorCodes.INVALID_REQUEST) } + + private fun Any?.stringKeyMap(): Map { + val raw = this as? Map<*, *> ?: error("expected JSON-RPC result map") + return raw.entries.associate { (key, value) -> + check(key is String) { "expected string result key but got $key" } + key to value + } + } } diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpToolTestPayloads.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpToolTestPayloads.kt new file mode 100644 index 0000000..f2e2606 --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpToolTestPayloads.kt @@ -0,0 +1,21 @@ +package com.jorisjonkers.personalstack.knowledge.mcp + +internal fun Any?.stringKeyMap(): Map { + val raw = this as? Map<*, *> ?: error("expected map value") + return raw.entries.associate { (key, value) -> + check(key is String) { "expected string map key but got $key" } + key to value + } +} + +internal fun Any?.stringKeyMapList(): List> { + val raw = this as? List<*> ?: error("expected list value") + return raw.map { it.stringKeyMap() } +} + +internal fun Any?.stringList(): List { + val raw = this as? List<*> ?: error("expected string list") + return raw.map { value -> + value as? String ?: error("expected string list item but got $value") + } +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpToolsTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpToolsTest.kt index eb29fa0..0e036a1 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpToolsTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/McpToolsTest.kt @@ -1,4 +1,3 @@ -@file:Suppress("DEPRECATION") // Jackson 3 deprecated asText(). package com.jorisjonkers.personalstack.knowledge.mcp @@ -15,23 +14,12 @@ import com.jorisjonkers.personalstack.knowledge.domain.KbNote import com.jorisjonkers.personalstack.knowledge.domain.KbNoteType import com.jorisjonkers.personalstack.knowledge.domain.KbRelation import com.jorisjonkers.personalstack.knowledge.domain.RecallHit -import com.jorisjonkers.personalstack.knowledge.domain.ReviewBucket -import com.jorisjonkers.personalstack.knowledge.domain.ReviewNote -import com.jorisjonkers.personalstack.knowledge.domain.ReviewSuggestion -import com.jorisjonkers.personalstack.knowledge.domain.ReviewSummary -import com.jorisjonkers.personalstack.knowledge.domain.ScopeSummary -import com.jorisjonkers.personalstack.knowledge.domain.SourceSummary -import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateCluster -import com.jorisjonkers.personalstack.knowledge.domain.TagCandidateMember -import com.jorisjonkers.personalstack.knowledge.domain.TagSummary -import com.jorisjonkers.personalstack.knowledge.domain.TopicStats -import com.jorisjonkers.personalstack.knowledge.domain.TopicSummary import com.jorisjonkers.personalstack.knowledge.recall.RecallService +import com.jorisjonkers.personalstack.knowledge.repo.AuditRecordRequest import com.jorisjonkers.personalstack.knowledge.repo.AuditRepository import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository import com.jorisjonkers.personalstack.knowledge.repo.TopicRepository import com.jorisjonkers.personalstack.knowledge.review.ReviewService -import com.jorisjonkers.personalstack.knowledge.review.ReviewSummaryRequest import io.mockk.every import io.mockk.mockk import io.mockk.slot @@ -42,7 +30,6 @@ import tools.jackson.databind.json.JsonMapper import tools.jackson.module.kotlin.KotlinModule import java.time.Instant -@Suppress("LargeClass") class McpToolsTest { private val captureService = mockk() private val recallService = mockk(relaxed = true) @@ -62,13 +49,15 @@ class McpToolsTest { // builders + handler argument parsing. private val tools = McpTools( - CaptureMcpTools(captureService), - ReadMcpTools(recallService), - DiscoveryMcpTools(discoveryService, tagClusterService), - AdminMcpTools(topicRepository, noteRepository, auditRepository, adminAuthorization), - DigestMcpTools(digestService), - AuditMcpTools(auditService), - ReviewMcpTools(reviewService), + coreTools = CoreMcpToolSet(CaptureMcpTools(captureService), ReadMcpTools(recallService)), + fullTools = + FullMcpToolSet( + DiscoveryMcpTools(discoveryService, tagClusterService), + AdminMcpTools(topicRepository, noteRepository, auditRepository, adminAuthorization), + DigestMcpTools(digestService), + AuditMcpTools(auditService), + ReviewMcpTools(reviewService), + ), ) private val mapper: JsonMapper = JsonMapper.builder().addModule(KotlinModule.Builder().build()).build() @@ -101,14 +90,16 @@ class McpToolsTest { private fun liteTools() = McpTools( - CaptureMcpTools(captureService), - ReadMcpTools(recallService), - DiscoveryMcpTools(discoveryService, tagClusterService), - AdminMcpTools(topicRepository, noteRepository, auditRepository, adminAuthorization), - DigestMcpTools(digestService), - AuditMcpTools(auditService), - ReviewMcpTools(reviewService), - KnowledgeModeProperties(mode = KnowledgeMode.LITE), + coreTools = CoreMcpToolSet(CaptureMcpTools(captureService), ReadMcpTools(recallService)), + fullTools = + FullMcpToolSet( + DiscoveryMcpTools(discoveryService, tagClusterService), + AdminMcpTools(topicRepository, noteRepository, auditRepository, adminAuthorization), + DigestMcpTools(digestService), + AuditMcpTools(auditService), + ReviewMcpTools(reviewService), + ), + modeProperties = KnowledgeModeProperties(mode = KnowledgeMode.LITE), ) @Test @@ -165,91 +156,6 @@ class McpToolsTest { ) } - @Test - @Suppress("LongMethod") - fun `review_summary forwards bounded request and projects governance buckets`() { - val request = slot() - every { reviewService.summary(capture(request)) } returns - ReviewSummary( - generatedAt = Instant.parse("2026-06-04T16:00:00Z"), - inbox = - ReviewBucket( - total = 1, - items = - listOf( - ReviewNote( - id = "01HXREVIEW0000000000000000", - type = "lesson", - scope = "_inbox", - source = "assistant-ui:auto-capture:s1", - capturedAt = Instant.parse("2026-06-04T15:00:00Z"), - confidence = 0.52, - title = "pending note", - vaultPath = "_inbox/2026-06-04/pending.md", - tags = setOf("auto-capture"), - recallCount = 4, - lastRecalledAt = Instant.parse("2026-06-04T15:30:00Z"), - ), - ), - ), - needsReview = ReviewBucket(total = 0, items = emptyList()), - recentAutoCaptures = ReviewBucket(total = 0, items = emptyList()), - staleUnusedNotes = ReviewBucket(total = 0, items = emptyList()), - lowConfidenceHighRecall = ReviewBucket(total = 0, items = emptyList()), - tagCandidateClusters = ReviewBucket(total = 0, items = emptyList()), - recentAudit = ReviewBucket(total = 0, items = emptyList()), - suggestions = - listOf( - ReviewSuggestion( - kind = "needs_review", - severity = "high", - message = "review pending memory", - suggestedTool = "knowledge.reclassify_note", - targetId = "01HXREVIEW0000000000000000", - targetKind = "note", - details = mapOf("total" to 1), - ), - ), - ) - - val out = - tools.call( - "knowledge.review_summary", - mapper.readTree( - """ - { - "limit": 5, - "stale_days": 30, - "low_confidence_max": 0.5, - "high_recall_min": 2, - "tag_threshold": 0.9 - } - """.trimIndent(), - ), - )!! - - assertThat(request.captured.limit).isEqualTo(5) - assertThat(request.captured.staleDays).isEqualTo(30) - assertThat(request.captured.lowConfidenceMax).isEqualTo(0.5) - assertThat(request.captured.highRecallMin).isEqualTo(2) - assertThat(request.captured.tagThreshold).isEqualTo(0.9) - - @Suppress("UNCHECKED_CAST") - val summary = out["summary"] as Map - assertThat(summary["generated_at"]).isEqualTo("2026-06-04T16:00:00Z") - @Suppress("UNCHECKED_CAST") - val inbox = summary["inbox"] as Map - assertThat(inbox["total"]).isEqualTo(1) - @Suppress("UNCHECKED_CAST") - val inboxItems = inbox["items"] as List> - assertThat(inboxItems[0]["source"]).isEqualTo("assistant-ui:auto-capture:s1") - assertThat(inboxItems[0]["recall_count"]).isEqualTo(4) - assertThat(inboxItems[0]["last_recalled_at"]).isEqualTo("2026-06-04T15:30:00Z") - @Suppress("UNCHECKED_CAST") - val suggestions = summary["suggestions"] as List> - assertThat(suggestions[0]["suggested_tool"]).isEqualTo("knowledge.reclassify_note") - } - @Test fun `list_audit projects each row including target metadata and timestamps`() { every { auditService.list(any(), any(), any(), any(), any()) } returns @@ -271,9 +177,7 @@ class McpToolsTest { "knowledge.list_audit", mapper.readTree("""{"actor":"kb-renormalise-titles","limit":10}"""), )!! - - @Suppress("UNCHECKED_CAST") - val rows = out["rows"] as List> + val rows = out["rows"].stringKeyMapList() assertThat(rows).hasSize(1) assertThat(rows[0]["action"]).isEqualTo("rename_title") assertThat(rows[0]["target_kind"]).isEqualTo("note") @@ -281,41 +185,12 @@ class McpToolsTest { assertThat(rows[0]["at"]).isEqualTo("2026-05-19T13:00:00Z") } - @Test - fun `list_tag_candidates projects each cluster with its members and canonical`() { - every { tagClusterService.listTagCandidates(any(), any(), any()) } returns - listOf( - TagCandidateCluster( - members = listOf(TagCandidateMember("kotlin", 10), TagCandidateMember("Kotlin", 3)), - suggestedCanonical = "kotlin", - averageSimilarity = 0.97, - ), - ) - - val out = - tools.call( - "knowledge.list_tag_candidates", - mapper.readTree("""{"min_count":2,"threshold":0.9,"max_tags":50}"""), - )!! - - @Suppress("UNCHECKED_CAST") - val clusters = out["clusters"] as List> - assertThat(clusters).hasSize(1) - assertThat(clusters[0]["suggested_canonical"]).isEqualTo("kotlin") - assertThat(clusters[0]["average_similarity"]).isEqualTo(0.97) - - @Suppress("UNCHECKED_CAST") - val members = clusters[0]["members"] as List> - assertThat(members.map { it["tag"] }).containsExactly("kotlin", "Kotlin") - assertThat(members.map { it["count"] }).containsExactly(10, 3) - } - @Test fun `merge_tags forwards source tags and projects idempotent merge counts`() { every { adminAuthorization.requireAdmin() } returns "mcp:admin" every { topicRepository.mergeTags(listOf("kt", "kts"), "kotlin") } returns TopicRepository.MergeTagsResult(rowsRenamed = 2, rowsDeletedAsDupes = 1) - every { auditRepository.record(any(), any(), any(), any(), any(), any(), any()) } returns + every { auditRepository.record(any()) } returns auditRow(id = "01HXAUDMERGETAGS0000000000") val out = @@ -333,13 +208,18 @@ class McpToolsTest { verify(exactly = 1) { topicRepository.mergeTags(listOf("kt", "kts"), "kotlin") } verify(exactly = 1) { auditRepository.record( - actor = "mcp:admin", - action = "merge_tags", - targetId = null, - targetKind = "tag", - beforeJson = """{"from":["kt","kts"]}""", - afterJson = """{"into":"kotlin","rows_renamed":2,"rows_dropped_as_dupes":1}""", - now = any(), + match { + it == + AuditRecordRequest( + actor = "mcp:admin", + action = "merge_tags", + targetId = null, + targetKind = "tag", + beforeJson = """{"from":["kt","kts"]}""", + afterJson = """{"into":"kotlin","rows_renamed":2,"rows_dropped_as_dupes":1}""", + now = it.now, + ) + }, ) } } @@ -357,14 +237,14 @@ class McpToolsTest { )!! assertThat(out).doesNotContainKey("audit_id") - verify(exactly = 0) { auditRepository.record(any(), any(), any(), any(), any(), any(), any()) } + verify(exactly = 0) { auditRepository.record(any()) } } @Test fun `rename_tag writes an audit row when rows are touched`() { every { adminAuthorization.requireAdmin() } returns "mcp:admin" every { topicRepository.renameTag("kt", "kotlin") } returns 2 - every { auditRepository.record(any(), any(), any(), any(), any(), any(), any()) } returns + every { auditRepository.record(any()) } returns auditRow(id = "01HXAUDRENAMETAG000000000") val out = @@ -377,13 +257,18 @@ class McpToolsTest { assertThat(out["audit_id"]).isEqualTo("01HXAUDRENAMETAG000000000") verify(exactly = 1) { auditRepository.record( - actor = "mcp:admin", - action = "rename_tag", - targetId = null, - targetKind = "tag", - beforeJson = """{"tag":"kt"}""", - afterJson = """{"tag":"kotlin","rows_touched":2}""", - now = any(), + match { + it == + AuditRecordRequest( + actor = "mcp:admin", + action = "rename_tag", + targetId = null, + targetKind = "tag", + beforeJson = """{"tag":"kt"}""", + afterJson = """{"tag":"kotlin","rows_touched":2}""", + now = it.now, + ) + }, ) } } @@ -410,9 +295,7 @@ class McpToolsTest { "knowledge.digest_transcript", mapper.readTree("""{"transcript":"... session text ...","max_candidates":3,"min_confidence":0.5}"""), )!! - - @Suppress("UNCHECKED_CAST") - val candidates = out["candidates"] as List> + val candidates = out["candidates"].stringKeyMapList() assertThat(candidates).hasSize(1) assertThat(candidates[0]["kind"]).isEqualTo("lesson") assertThat(candidates[0]["confidence"]).isEqualTo(0.82) @@ -426,14 +309,12 @@ class McpToolsTest { tools.describe().filter { it["name"] in captureNames }.forEach { descriptor -> val schema = descriptor["inputSchema"] as Map<*, *> assertThat(schema["type"]).isEqualTo("object") - @Suppress("UNCHECKED_CAST") - val required = schema["required"] as List + val required = schema["required"].stringList() // `scope` is optional now: omitted captures default to // `_inbox` and the curator agent assigns the final scope // during the classify-and-promote pass. assertThat(required).containsExactlyInAnyOrder("title", "body") - @Suppress("UNCHECKED_CAST") - val properties = schema["properties"] as Map + val properties = schema["properties"].stringKeyMap() assertThat(properties.keys) .contains( "scope", @@ -450,16 +331,12 @@ class McpToolsTest { @Test fun `read tool descriptors expose their own required and properties shape`() { val recall = tools.describe().single { it["name"] == "knowledge.recall" } - - @Suppress("UNCHECKED_CAST") - val recallSchema = recall["inputSchema"] as Map + val recallSchema = recall["inputSchema"].stringKeyMap() assertThat(recallSchema["required"] as List<*>).containsExactly("query") assertThat((recallSchema["properties"] as Map<*, *>).keys).contains("query", "scope", "limit") val findConflicts = tools.describe().single { it["name"] == "knowledge.find_conflicts" } - - @Suppress("UNCHECKED_CAST") - val fcSchema = findConflicts["inputSchema"] as Map + val fcSchema = findConflicts["inputSchema"].stringKeyMap() assertThat(fcSchema["required"] as List<*>).containsExactly("id") } @@ -482,9 +359,7 @@ class McpToolsTest { "knowledge.recall", mapper.readTree("""{"query":"rockets","scope":"personal","limit":5}"""), )!! - - @Suppress("UNCHECKED_CAST") - val hits = out["hits"] as List> + val hits = out["hits"].stringKeyMapList() assertThat(hits).hasSize(1) assertThat(hits[0]["id"]).isEqualTo("01HXY") assertThat(hits[0]["score"]).isEqualTo(0.42) @@ -520,9 +395,7 @@ class McpToolsTest { ), ) val out = tools.call("knowledge.find_conflicts", mapper.readTree("""{"id":"01HXY"}"""))!! - - @Suppress("UNCHECKED_CAST") - val rels = out["relations"] as List> + val rels = out["relations"].stringKeyMapList() assertThat(rels).hasSize(1) assertThat(rels[0]["predicate"]).isEqualTo("supersedes") assertThat(rels[0]["object_id"]).isEqualTo("01ABC") @@ -549,9 +422,7 @@ class McpToolsTest { ) val out = tools.call("knowledge.relations", mapper.readTree("""{"id":"01HXY","depth":2}"""))!! - - @Suppress("UNCHECKED_CAST") - val rels = out["relations"] as List> + val rels = out["relations"].stringKeyMapList() assertThat(rels).hasSize(2) assertThat(rels.map { it["object_id"] }).containsExactly("01DEF", "01OLD") verify(exactly = 1) { recallService.walkRelations("01HXY", 2) } @@ -671,126 +542,4 @@ class McpToolsTest { ) assertThat(captured.captured.confidence).isEqualTo(0.85) } - - // -------- discovery tools -------- - - @Test - fun `list_topics projects each summary with the bare slug`() { - every { discoveryService.listTopics(5) } returns - listOf( - TopicSummary( - slug = "kotlin", - noteCount = 12, - lastCapturedAt = Instant.parse("2026-05-13T12:00:00Z"), - ), - TopicSummary(slug = "postgres", noteCount = 3, lastCapturedAt = null), - ) - - val out = tools.call("knowledge.list_topics", mapper.readTree("""{"limit":5}"""))!! - - @Suppress("UNCHECKED_CAST") - val rows = out["topics"] as List> - assertThat(rows).hasSize(2) - assertThat(rows[0]["slug"]).isEqualTo("kotlin") - assertThat(rows[0]["note_count"]).isEqualTo(12) - assertThat(rows[0]["last_captured_at"]).isEqualTo("2026-05-13T12:00:00Z") - assertThat(rows[1]["last_captured_at"]).isNull() - } - - @Test - fun `list_tags forwards the optional scope filter`() { - every { discoveryService.listTags("project:personal-stack", 50) } returns - listOf( - TagSummary( - tag = "kotlin", - count = 7, - lastUsedAt = Instant.parse("2026-05-13T12:00:00Z"), - ), - ) - tools.call( - "knowledge.list_tags", - mapper.readTree("""{"scope":"project:personal-stack","limit":50}"""), - ) - verify(exactly = 1) { discoveryService.listTags("project:personal-stack", 50) } - } - - @Test - fun `list_scopes returns counts ordered by note_count desc`() { - every { discoveryService.listScopes(any()) } returns - listOf( - ScopeSummary(scope = "project:a", noteCount = 5, lastCapturedAt = null), - ScopeSummary(scope = "personal", noteCount = 1, lastCapturedAt = null), - ) - val out = tools.call("knowledge.list_scopes", mapper.readTree("""{}"""))!! - - @Suppress("UNCHECKED_CAST") - val rows = out["scopes"] as List> - assertThat(rows.map { it["scope"] }).containsExactly("project:a", "personal") - } - - @Test - fun `list_sources projects the source frequency`() { - every { discoveryService.listSources(any()) } returns - listOf(SourceSummary(source = "claude-code", count = 42)) - val out = tools.call("knowledge.list_sources", mapper.readTree("""{}"""))!! - - @Suppress("UNCHECKED_CAST") - val rows = out["sources"] as List> - assertThat(rows).hasSize(1) - assertThat(rows[0]["source"]).isEqualTo("claude-code") - assertThat(rows[0]["count"]).isEqualTo(42) - } - - @Test - fun `topic_stats returns null when the slug has no notes`() { - every { discoveryService.topicStats("nonexistent", any()) } returns null - val out = tools.call("knowledge.topic_stats", mapper.readTree("""{"slug":"nonexistent"}"""))!! - assertThat(out["stats"]).isNull() - } - - @Test - fun `topic_stats projects the breakdowns when the slug has notes`() { - every { discoveryService.topicStats("kotlin", 10) } returns - TopicStats( - slug = "kotlin", - noteCount = 12, - firstCapturedAt = Instant.parse("2026-05-10T12:00:00Z"), - lastCapturedAt = Instant.parse("2026-05-13T12:00:00Z"), - typeBreakdown = mapOf("lesson" to 8, "decision" to 4), - topTags = - listOf( - TagSummary( - tag = "spring", - count = 5, - lastUsedAt = Instant.parse("2026-05-13T12:00:00Z"), - ), - ), - ) - - val out = tools.call("knowledge.topic_stats", mapper.readTree("""{"slug":"kotlin"}"""))!! - - @Suppress("UNCHECKED_CAST") - val stats = out["stats"] as Map - assertThat(stats["slug"]).isEqualTo("kotlin") - assertThat(stats["note_count"]).isEqualTo(12) - assertThat((stats["type_breakdown"] as Map<*, *>)["lesson"]).isEqualTo(8) - - @Suppress("UNCHECKED_CAST") - val topTags = stats["top_tags"] as List> - assertThat(topTags).hasSize(1) - assertThat(topTags[0]["tag"]).isEqualTo("spring") - } - - @Test - fun `list_inbox surfaces vault_path so the operator can find the file`() { - every { discoveryService.listInbox(20) } returns - listOf(stubNote.copy(vaultPath = "_inbox/2026-05-13/hello.md", scope = "_inbox")) - val out = tools.call("knowledge.list_inbox", mapper.readTree("""{}"""))!! - - @Suppress("UNCHECKED_CAST") - val notes = out["notes"] as List> - assertThat(notes).hasSize(1) - assertThat(notes[0]["vault_path"]).isEqualTo("_inbox/2026-05-13/hello.md") - assertThat(notes[0]["scope"]).isEqualTo("_inbox") - } } diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpToolsTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpToolsTest.kt new file mode 100644 index 0000000..09d3d67 --- /dev/null +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/mcp/ReviewMcpToolsTest.kt @@ -0,0 +1,141 @@ +package com.jorisjonkers.personalstack.knowledge.mcp + +import com.jorisjonkers.personalstack.knowledge.audit.AuditService +import com.jorisjonkers.personalstack.knowledge.auth.AdminAuthorization +import com.jorisjonkers.personalstack.knowledge.capture.CaptureService +import com.jorisjonkers.personalstack.knowledge.digest.DigestService +import com.jorisjonkers.personalstack.knowledge.discovery.DiscoveryService +import com.jorisjonkers.personalstack.knowledge.discovery.TagClusterService +import com.jorisjonkers.personalstack.knowledge.domain.ReviewBucket +import com.jorisjonkers.personalstack.knowledge.domain.ReviewNote +import com.jorisjonkers.personalstack.knowledge.domain.ReviewSuggestion +import com.jorisjonkers.personalstack.knowledge.domain.ReviewSummary +import com.jorisjonkers.personalstack.knowledge.recall.RecallService +import com.jorisjonkers.personalstack.knowledge.repo.AuditRepository +import com.jorisjonkers.personalstack.knowledge.repo.NoteRepository +import com.jorisjonkers.personalstack.knowledge.repo.TopicRepository +import com.jorisjonkers.personalstack.knowledge.review.ReviewService +import com.jorisjonkers.personalstack.knowledge.review.ReviewSummaryRequest +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.Test +import tools.jackson.databind.json.JsonMapper +import tools.jackson.module.kotlin.KotlinModule +import java.time.Instant + +class ReviewMcpToolsTest { + private val reviewService = mockk(relaxed = true) + private val tools = reviewTools(reviewService) + private val mapper: JsonMapper = JsonMapper.builder().addModule(KotlinModule.Builder().build()).build() + + @Test + fun `review_summary forwards bounded request and projects governance buckets`() { + val request = slot() + every { reviewService.summary(capture(request)) } returns reviewSummary() + + val out = + tools.call( + "knowledge.review_summary", + mapper.readTree( + """ + { + "limit": 5, + "stale_days": 30, + "low_confidence_max": 0.5, + "high_recall_min": 2, + "tag_threshold": 0.9 + } + """.trimIndent(), + ), + )!! + + assertReviewRequest(request.captured) + assertReviewSummary(out["summary"].stringKeyMap()) + } + + private fun assertReviewRequest(request: ReviewSummaryRequest) { + assertThat(request.limit).isEqualTo(5) + assertThat(request.staleDays).isEqualTo(30) + assertThat(request.lowConfidenceMax).isEqualTo(0.5) + assertThat(request.highRecallMin).isEqualTo(2) + assertThat(request.tagThreshold).isEqualTo(0.9) + } + + private fun assertReviewSummary(summary: Map) { + assertThat(summary["generated_at"]).isEqualTo("2026-06-04T16:00:00Z") + val inbox = summary["inbox"].stringKeyMap() + assertThat(inbox["total"]).isEqualTo(1) + val inboxItems = inbox["items"].stringKeyMapList() + assertThat(inboxItems[0]["source"]).isEqualTo("assistant-ui:auto-capture:s1") + assertThat(inboxItems[0]["recall_count"]).isEqualTo(4) + assertThat(inboxItems[0]["last_recalled_at"]).isEqualTo("2026-06-04T15:30:00Z") + val suggestions = summary["suggestions"].stringKeyMapList() + assertThat(suggestions[0]["suggested_tool"]).isEqualTo("knowledge.reclassify_note") + } + + private fun reviewSummary(): ReviewSummary = + ReviewSummary( + generatedAt = Instant.parse("2026-06-04T16:00:00Z"), + inbox = ReviewBucket(total = 1, items = listOf(reviewNote())), + needsReview = ReviewBucket(total = 0, items = emptyList()), + recentAutoCaptures = ReviewBucket(total = 0, items = emptyList()), + staleUnusedNotes = ReviewBucket(total = 0, items = emptyList()), + lowConfidenceHighRecall = ReviewBucket(total = 0, items = emptyList()), + tagCandidateClusters = ReviewBucket(total = 0, items = emptyList()), + recentAudit = ReviewBucket(total = 0, items = emptyList()), + suggestions = listOf(reviewSuggestion()), + ) + + private fun reviewNote(): ReviewNote = + ReviewNote( + id = "01HXREVIEW0000000000000000", + type = "lesson", + scope = "_inbox", + source = "assistant-ui:auto-capture:s1", + capturedAt = Instant.parse("2026-06-04T15:00:00Z"), + confidence = 0.52, + title = "pending note", + vaultPath = "_inbox/2026-06-04/pending.md", + tags = setOf("auto-capture"), + recallCount = 4, + lastRecalledAt = Instant.parse("2026-06-04T15:30:00Z"), + ) + + private fun reviewSuggestion(): ReviewSuggestion = + ReviewSuggestion( + kind = "needs_review", + severity = "high", + message = "review pending memory", + suggestedTool = "knowledge.reclassify_note", + targetId = "01HXREVIEW0000000000000000", + targetKind = "note", + details = mapOf("total" to 1), + ) + + private fun reviewTools(reviewService: ReviewService): McpTools = + McpTools( + coreTools = + CoreMcpToolSet( + CaptureMcpTools(mockk(relaxed = true)), + ReadMcpTools(mockk(relaxed = true)), + ), + fullTools = + FullMcpToolSet( + DiscoveryMcpTools( + mockk(relaxed = true), + mockk(relaxed = true), + ), + AdminMcpTools( + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + mockk(relaxed = true), + ), + DigestMcpTools(mockk(relaxed = true)), + AuditMcpTools(mockk(relaxed = true)), + ReviewMcpTools(reviewService), + ), + ) +} diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallEvalFixtureTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallEvalFixtureTest.kt index 659eac8..7903965 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallEvalFixtureTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallEvalFixtureTest.kt @@ -94,12 +94,8 @@ class RecallEvalFixtureTest { val service = RecallService( - noteRepository = noteRepository, - recallRepository = recallRepository, - embeddingRepository = embeddingRepository, - queryEmbedder = queryEmbedder, - graphRetriever = graphRetriever, - reranker = reranker, + stores = RecallStores(noteRepository, recallRepository, embeddingRepository), + enhancers = RecallEnhancers(queryEmbedder, graphRetriever, reranker), observationRegistry = ObservationRegistry.NOOP, defaultModeWire = evalCase.mode, rrfK = 60, diff --git a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallServiceTest.kt b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallServiceTest.kt index 298e53b..d49a779 100644 --- a/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallServiceTest.kt +++ b/api/src/test/kotlin/com/jorisjonkers/personalstack/knowledge/recall/RecallServiceTest.kt @@ -10,6 +10,7 @@ import io.mockk.every import io.mockk.mockk import io.mockk.verify import org.assertj.core.api.Assertions.assertThat +import org.jooq.exception.DataAccessException import org.junit.jupiter.api.Test class RecallServiceTest { @@ -23,12 +24,8 @@ class RecallServiceTest { private fun service(defaultModeWire: String = "fast"): RecallService { every { graphRetriever.retrieve(any(), any(), any()) } returns emptyList() return RecallService( - noteRepository = noteRepository, - recallRepository = recallRepository, - embeddingRepository = embeddingRepository, - queryEmbedder = queryEmbedder, - graphRetriever = graphRetriever, - reranker = reranker, + stores = RecallStores(noteRepository, recallRepository, embeddingRepository), + enhancers = RecallEnhancers(queryEmbedder, graphRetriever, reranker), observationRegistry = ObservationRegistry.NOOP, defaultModeWire = defaultModeWire, rrfK = 60, @@ -92,7 +89,8 @@ class RecallServiceTest { fun `hybrid mode degrades gracefully when the embedder throws`() { every { recallRepository.recall("rockets", "personal", 15) } returns listOf(hit("ID-A"), hit("ID-B")) - every { queryEmbedder.embed("rockets") } throws RuntimeException("ollama down") + every { queryEmbedder.embed("rockets") } throws + QueryEmbeddingException("ollama down", IllegalStateException("ollama down")) val hits = service().recall("rockets", "personal", 5, RecallMode.HYBRID) @@ -140,8 +138,8 @@ class RecallServiceTest { listOf(hit("GRAPH-A", score = 0.55)) every { reranker.rerank("rockets", any(), 5) } answers { val input = secondArg>() - assertThat(input.map { it.id }).contains("GRAPH-A") - input.sortedByDescending { if (it.id == "GRAPH-A") 1 else 0 } + assertThat(input.map { hit -> hit.id }).contains("GRAPH-A") + input.sortedByDescending { hit -> if (hit.id == "GRAPH-A") 1 else 0 } } val hits = recallService.recall("rockets", "personal", 5, RecallMode.DEEP) @@ -188,7 +186,7 @@ class RecallServiceTest { @Test fun `recall swallows a usage-stats bump failure rather than 500ing`() { every { recallRepository.recall("rockets", "personal", 5) } returns listOf(hit("ID-A")) - every { noteRepository.bumpRecallStats(any()) } throws RuntimeException("db down") + every { noteRepository.bumpRecallStats(any()) } throws DataAccessException("db down") val hits = service().recall("rockets", "personal", 5, RecallMode.FAST) diff --git a/client-spec/openapi/knowledge-api.json b/client-spec/openapi/knowledge-api.json index d47352a..80787fb 100644 --- a/client-spec/openapi/knowledge-api.json +++ b/client-spec/openapi/knowledge-api.json @@ -328,7 +328,6 @@ "parameters" : [ { "in" : "query", "name" : "limit", - "required" : false, "schema" : { "type" : "integer", "format" : "int32", @@ -337,7 +336,6 @@ }, { "in" : "query", "name" : "stale_days", - "required" : false, "schema" : { "type" : "integer", "format" : "int32", @@ -346,7 +344,6 @@ }, { "in" : "query", "name" : "low_confidence_max", - "required" : false, "schema" : { "type" : "number", "format" : "double", @@ -355,7 +352,6 @@ }, { "in" : "query", "name" : "high_recall_min", - "required" : false, "schema" : { "type" : "integer", "format" : "int32", @@ -364,7 +360,6 @@ }, { "in" : "query", "name" : "tag_min_count", - "required" : false, "schema" : { "type" : "integer", "format" : "int32", @@ -373,7 +368,6 @@ }, { "in" : "query", "name" : "tag_threshold", - "required" : false, "schema" : { "type" : "number", "format" : "double", @@ -382,7 +376,6 @@ }, { "in" : "query", "name" : "tag_max_tags", - "required" : false, "schema" : { "type" : "integer", "format" : "int32",