Skip to content

Commit a460c4e

Browse files
jpicklykclaude
jpicklyk
and
claude
authored
perf: parallelize sequential findByRole queries in GetContextTool (#45)
* perf: parallelize sequential findByRole queries in GetContextTool Health-check mode (3 queries: WORK, REVIEW, BLOCKED) and session-resume mode (2 queries: WORK, REVIEW) now launch findByRole calls concurrently via coroutineScope/async instead of sequentially, reducing latency when the database supports concurrent reads. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: add Result.getOrElse and simplify unwrap patterns Add getOrElse(default) extension to Result sealed class, replacing verbose when-expression unwrapping with one-liner calls. Applied to all 6 instances in GetContextTool (5 from parallelization + 1 bonus). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test: add ResultTest covering all Result utility methods Adds direct unit tests for getOrElse, getOrNull, map, isSuccess, isError, onSuccess, and onError — previously only exercised indirectly through tool integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 42c5342 commit a460c4e

File tree

4 files changed

+212
-25
lines changed

4 files changed

+212
-25
lines changed

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/GetContextTool.kt‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import io.github.jpicklyk.mcptask.current.domain.model.Role
55
import io.github.jpicklyk.mcptask.current.domain.repository.Result
66
import io.modelcontextprotocol.kotlin.sdk.types.ToolAnnotations
77
import io.modelcontextprotocol.kotlin.sdk.types.ToolSchema
8+
import kotlinx.coroutines.async
9+
import kotlinx.coroutines.coroutineScope
810
import kotlinx.serialization.json.*
911

1012
/**
@@ -193,22 +195,21 @@ Parameters:
193195
): JsonElement {
194196
val workItemRepo = context.workItemRepository()
195197

196-
// Fetch work and review items (two calls, merge results)
197-
val workItems = when (val r = workItemRepo.findByRole(Role.WORK, limit = 200)) {
198-
is Result.Success -> r.data
199-
is Result.Error -> emptyList()
200-
}
201-
val reviewItems = when (val r = workItemRepo.findByRole(Role.REVIEW, limit = 200)) {
202-
is Result.Success -> r.data
203-
is Result.Error -> emptyList()
198+
// Fetch work and review items in parallel, merge results
199+
val (workItems, reviewItems) = coroutineScope {
200+
val workDeferred = async { workItemRepo.findByRole(Role.WORK, limit = 200) }
201+
val reviewDeferred = async { workItemRepo.findByRole(Role.REVIEW, limit = 200) }
202+
203+
Pair(
204+
workDeferred.await().getOrElse(emptyList()),
205+
reviewDeferred.await().getOrElse(emptyList())
206+
)
204207
}
205-
val activeItems = (workItems + reviewItems)
208+
val activeItems = workItems + reviewItems
206209

207210
// Recent transitions since the given timestamp
208-
val recentTransitions = when (val r = context.roleTransitionRepository().findSince(since, limit = transitionLimit)) {
209-
is Result.Success -> r.data
210-
is Result.Error -> emptyList()
211-
}
211+
val recentTransitions = context.roleTransitionRepository().findSince(since, limit = transitionLimit)
212+
.getOrElse(emptyList())
212213

213214
// Stalled items: active items with missing required notes
214215
val stalledItems = findStalledItems(activeItems, context)
@@ -266,20 +267,20 @@ Parameters:
266267
private suspend fun executeHealthCheckMode(context: ToolExecutionContext, includeAncestors: Boolean): JsonElement {
267268
val workItemRepo = context.workItemRepository()
268269

269-
val workItems = when (val r = workItemRepo.findByRole(Role.WORK, limit = 200)) {
270-
is Result.Success -> r.data
271-
is Result.Error -> emptyList()
272-
}
273-
val reviewItems = when (val r = workItemRepo.findByRole(Role.REVIEW, limit = 200)) {
274-
is Result.Success -> r.data
275-
is Result.Error -> emptyList()
276-
}
277-
val blockedItems = when (val r = workItemRepo.findByRole(Role.BLOCKED, limit = 200)) {
278-
is Result.Success -> r.data
279-
is Result.Error -> emptyList()
270+
// Fetch work, review, and blocked items in parallel
271+
val (workItems, reviewItems, blockedItems) = coroutineScope {
272+
val workDeferred = async { workItemRepo.findByRole(Role.WORK, limit = 200) }
273+
val reviewDeferred = async { workItemRepo.findByRole(Role.REVIEW, limit = 200) }
274+
val blockedDeferred = async { workItemRepo.findByRole(Role.BLOCKED, limit = 200) }
275+
276+
Triple(
277+
workDeferred.await().getOrElse(emptyList()),
278+
reviewDeferred.await().getOrElse(emptyList()),
279+
blockedDeferred.await().getOrElse(emptyList())
280+
)
280281
}
281282

282-
val activeItems = (workItems + reviewItems)
283+
val activeItems = workItems + reviewItems
283284
val stalledItems = findStalledItems(activeItems, context)
284285

285286
// Resolve ancestor chains once for all items if requested

‎current/src/main/kotlin/io/github/jpicklyk/mcptask/current/domain/repository/Result.kt‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ sealed class Result<out T> {
6262
}
6363
return this
6464
}
65+
66+
/**
67+
* Returns the data if this is a success result, otherwise returns the provided default value.
68+
*/
69+
fun getOrElse(default: @UnsafeVariance T): T = when (this) {
70+
is Success -> data
71+
is Error -> default
72+
}
6573
}
6674

6775
/**

‎current/src/test/kotlin/io/github/jpicklyk/mcptask/current/application/tools/workflow/GetContextToolTest.kt‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,43 @@ class GetContextToolTest {
588588
assertEquals("Root", childAncestors[0].jsonObject["title"]!!.jsonPrimitive.content)
589589
}
590590

591+
// ──────────────────────────────────────────────
592+
// Parallel query tests
593+
// ──────────────────────────────────────────────
594+
595+
@Test
596+
fun `health check parallel queries return correct combined results`(): Unit = runBlocking {
597+
val workItem = makeItem(title = "Work Task", role = Role.WORK)
598+
val reviewItem = makeItem(title = "Review Task", role = Role.REVIEW)
599+
val blockedItem = makeItem(title = "Blocked Task", role = Role.BLOCKED)
600+
601+
coEvery { workItemRepo.findByRole(Role.WORK, any()) } returns Result.Success(listOf(workItem))
602+
coEvery { workItemRepo.findByRole(Role.REVIEW, any()) } returns Result.Success(listOf(reviewItem))
603+
coEvery { workItemRepo.findByRole(Role.BLOCKED, any()) } returns Result.Success(listOf(blockedItem))
604+
605+
val result = tool.execute(params(), context)
606+
607+
val data = extractData(result)
608+
assertEquals("health-check", data["mode"]!!.jsonPrimitive.content)
609+
610+
// Verify active items contain both work and review items
611+
val activeItems = data["activeItems"]!!.jsonArray
612+
assertEquals(2, activeItems.size)
613+
val activeTitles = activeItems.map { it.jsonObject["title"]!!.jsonPrimitive.content }.toSet()
614+
assertTrue("Work Task" in activeTitles, "Expected 'Work Task' in active items")
615+
assertTrue("Review Task" in activeTitles, "Expected 'Review Task' in active items")
616+
617+
// Verify blocked items section
618+
val blockedItems = data["blockedItems"]!!.jsonArray
619+
assertEquals(1, blockedItems.size)
620+
assertEquals("Blocked Task", blockedItems[0].jsonObject["title"]!!.jsonPrimitive.content)
621+
622+
// Verify all three findByRole calls were made
623+
coVerify(exactly = 1) { workItemRepo.findByRole(Role.WORK, any()) }
624+
coVerify(exactly = 1) { workItemRepo.findByRole(Role.REVIEW, any()) }
625+
coVerify(exactly = 1) { workItemRepo.findByRole(Role.BLOCKED, any()) }
626+
}
627+
591628
// ──────────────────────────────────────────────
592629
// guidancePointer tests
593630
// ──────────────────────────────────────────────

‎current/src/test/kotlin/io/github/jpicklyk/mcptask/current/domain/repository/ResultTest.kt‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
package io.github.jpicklyk.mcptask.current.domain.repository
2+
3+
import org.junit.jupiter.api.Assertions.*
4+
import org.junit.jupiter.api.Test
5+
import java.util.UUID
6+
7+
class ResultTest {
8+
9+
private val testError = RepositoryError.NotFound(UUID.randomUUID(), "Not found")
10+
11+
// ──────────────────────────────────────────────
12+
// getOrElse
13+
// ──────────────────────────────────────────────
14+
15+
@Test
16+
fun `getOrElse returns data on success`() {
17+
val result: Result<String> = Result.Success("hello")
18+
assertEquals("hello", result.getOrElse("default"))
19+
}
20+
21+
@Test
22+
fun `getOrElse returns default on error`() {
23+
val result: Result<String> = Result.Error(testError)
24+
assertEquals("default", result.getOrElse("default"))
25+
}
26+
27+
@Test
28+
fun `getOrElse returns empty list on error for list types`() {
29+
val result: Result<List<Int>> = Result.Error(testError)
30+
assertEquals(emptyList<Int>(), result.getOrElse(emptyList()))
31+
}
32+
33+
@Test
34+
fun `getOrElse returns populated list on success`() {
35+
val result: Result<List<Int>> = Result.Success(listOf(1, 2, 3))
36+
assertEquals(listOf(1, 2, 3), result.getOrElse(emptyList()))
37+
}
38+
39+
// ──────────────────────────────────────────────
40+
// getOrNull
41+
// ──────────────────────────────────────────────
42+
43+
@Test
44+
fun `getOrNull returns data on success`() {
45+
val result: Result<String> = Result.Success("hello")
46+
assertEquals("hello", result.getOrNull())
47+
}
48+
49+
@Test
50+
fun `getOrNull returns null on error`() {
51+
val result: Result<String> = Result.Error(testError)
52+
assertNull(result.getOrNull())
53+
}
54+
55+
// ──────────────────────────────────────────────
56+
// map
57+
// ──────────────────────────────────────────────
58+
59+
@Test
60+
fun `map transforms success data`() {
61+
val result: Result<Int> = Result.Success(5)
62+
val mapped = result.map { it * 2 }
63+
assertEquals(10, (mapped as Result.Success).data)
64+
}
65+
66+
@Test
67+
fun `map passes error through unchanged`() {
68+
val result: Result<Int> = Result.Error(testError)
69+
val mapped = result.map { it * 2 }
70+
assertTrue(mapped.isError())
71+
assertEquals(testError, (mapped as Result.Error).error)
72+
}
73+
74+
// ──────────────────────────────────────────────
75+
// isSuccess / isError
76+
// ──────────────────────────────────────────────
77+
78+
@Test
79+
fun `isSuccess returns true for success`() {
80+
assertTrue(Result.Success("data").isSuccess())
81+
}
82+
83+
@Test
84+
fun `isSuccess returns false for error`() {
85+
assertFalse(Result.Error(testError).isSuccess())
86+
}
87+
88+
@Test
89+
fun `isError returns true for error`() {
90+
assertTrue(Result.Error(testError).isError())
91+
}
92+
93+
@Test
94+
fun `isError returns false for success`() {
95+
assertFalse(Result.Success("data").isError())
96+
}
97+
98+
// ──────────────────────────────────────────────
99+
// onSuccess / onError callbacks
100+
// ──────────────────────────────────────────────
101+
102+
@Test
103+
fun `onSuccess executes block for success`() {
104+
var captured: String? = null
105+
Result.Success("hello").onSuccess { captured = it }
106+
assertEquals("hello", captured)
107+
}
108+
109+
@Test
110+
fun `onSuccess does not execute block for error`() {
111+
var executed = false
112+
Result.Error(testError).onSuccess { executed = true }
113+
assertFalse(executed)
114+
}
115+
116+
@Test
117+
fun `onError executes block for error`() {
118+
var captured: RepositoryError? = null
119+
Result.Error(testError).onError { captured = it }
120+
assertEquals(testError, captured)
121+
}
122+
123+
@Test
124+
fun `onError does not execute block for success`() {
125+
var executed = false
126+
Result.Success("hello").onError { executed = true }
127+
assertFalse(executed)
128+
}
129+
130+
@Test
131+
fun `onSuccess returns same result for chaining`() {
132+
val result = Result.Success("hello")
133+
assertSame(result, result.onSuccess { })
134+
}
135+
136+
@Test
137+
fun `onError returns same result for chaining`() {
138+
val result: Result<String> = Result.Error(testError)
139+
assertSame(result, result.onError { })
140+
}
141+
}

0 commit comments

Comments
 (0)