fix: polish SEP-1577 sampling handling (#765)
## Summary - reject empty sampling content in SamplingMessage and CreateMessageResult constructors - clarify sampling validation docs and emit an explicit error when tool_use is not followed by tool_result - return sampled text from the conformance test_sampling tool instead of content object strings Fixes #730 ## Testing - ./gradlew -Dorg.gradle.java.installations.paths="/Users/ubl/Documents/New project/oss-contributions/.jdks/jdk-21.0.11+10/Contents/Home" :kotlin-sdk-core:jvmTest :kotlin-sdk-server:jvmTest :conformance-test:compileKotlin Co-authored-by: Pavel Gorgulov <devcrocod@gmail.com>
This commit is contained in:
parent
d65c50735b
commit
75a944fb8c
5 changed files with 56 additions and 3 deletions
|
|
@ -179,7 +179,13 @@ fun Server.registerConformanceTools() {
|
||||||
),
|
),
|
||||||
),
|
),
|
||||||
)
|
)
|
||||||
CallToolResult(listOf(TextContent(result.content.joinToString("\n") { it.toString() })))
|
val sampledText = result.content.joinToString("\n") { content ->
|
||||||
|
when (content) {
|
||||||
|
is TextContent -> content.text
|
||||||
|
else -> "Non-text sampling content: ${content::class.simpleName}"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
CallToolResult(listOf(TextContent(sampledText)))
|
||||||
}
|
}
|
||||||
|
|
||||||
// 9. Elicitation
|
// 9. Elicitation
|
||||||
|
|
|
||||||
|
|
@ -91,6 +91,10 @@ public data class SamplingMessage(
|
||||||
@SerialName("_meta")
|
@SerialName("_meta")
|
||||||
val meta: JsonObject? = null,
|
val meta: JsonObject? = null,
|
||||||
) {
|
) {
|
||||||
|
init {
|
||||||
|
require(content.isNotEmpty()) { "content must contain at least one block" }
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Convenience constructor for a single-block message. Wraps [content] in a
|
* Convenience constructor for a single-block message. Wraps [content] in a
|
||||||
* singleton list so call sites can write `SamplingMessage(Role.User, TextContent("hi"))`
|
* singleton list so call sites can write `SamplingMessage(Role.User, TextContent("hi"))`
|
||||||
|
|
@ -273,6 +277,10 @@ public data class CreateMessageResult(
|
||||||
@SerialName("_meta")
|
@SerialName("_meta")
|
||||||
override val meta: JsonObject? = null,
|
override val meta: JsonObject? = null,
|
||||||
) : ClientResult {
|
) : ClientResult {
|
||||||
|
init {
|
||||||
|
require(content.isNotEmpty()) { "content must contain at least one block" }
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Convenience constructor for a single-block response. Wraps [content] in a
|
* Convenience constructor for a single-block response. Wraps [content] in a
|
||||||
* singleton list so call sites can write
|
* singleton list so call sites can write
|
||||||
|
|
|
||||||
|
|
@ -302,6 +302,13 @@ class SamplingTest {
|
||||||
(m.content[0] as TextContent).text shouldBe "hi"
|
(m.content[0] as TextContent).text shouldBe "hi"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `SamplingMessage rejects empty content`() {
|
||||||
|
assertFailsWith<IllegalArgumentException> {
|
||||||
|
SamplingMessage(role = Role.User, content = emptyList())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun `SamplingMessage single-element content serialises as single object`() {
|
fun `SamplingMessage single-element content serialises as single object`() {
|
||||||
val m = SamplingMessage(role = Role.User, content = listOf(TextContent("hi")))
|
val m = SamplingMessage(role = Role.User, content = listOf(TextContent("hi")))
|
||||||
|
|
@ -408,6 +415,17 @@ class SamplingTest {
|
||||||
(decoded.content[0] as TextContent).text shouldBe "hi"
|
(decoded.content[0] as TextContent).text shouldBe "hi"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `CreateMessageResult rejects empty content`() {
|
||||||
|
assertFailsWith<IllegalArgumentException> {
|
||||||
|
CreateMessageResult(
|
||||||
|
role = Role.Assistant,
|
||||||
|
content = emptyList(),
|
||||||
|
model = "test-model",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
// SamplingContentSerializer (single-or-array wire heuristic)
|
// SamplingContentSerializer (single-or-array wire heuristic)
|
||||||
// ============================================================================
|
// ============================================================================
|
||||||
|
|
|
||||||
|
|
@ -20,8 +20,8 @@ import io.modelcontextprotocol.kotlin.sdk.types.ToolUseContent
|
||||||
* 3. If the previous message contains `tool_use` blocks, the last message's
|
* 3. If the previous message contains `tool_use` blocks, the last message's
|
||||||
* `tool_result` ids MUST form exactly the same set.
|
* `tool_result` ids MUST form exactly the same set.
|
||||||
*
|
*
|
||||||
* On the first violation throws [IllegalArgumentException]. No-op when there are fewer
|
* On the first violation throws [IllegalArgumentException]. No-op when no
|
||||||
* than two messages or no tool_use / tool_result blocks are involved.
|
* tool_use / tool_result blocks are involved.
|
||||||
*/
|
*/
|
||||||
internal fun validateSamplingMessages(messages: List<SamplingMessage>) {
|
internal fun validateSamplingMessages(messages: List<SamplingMessage>) {
|
||||||
if (messages.isEmpty()) return
|
if (messages.isEmpty()) return
|
||||||
|
|
@ -44,6 +44,9 @@ internal fun validateSamplingMessages(messages: List<SamplingMessage>) {
|
||||||
if (hasPreviousToolUse) {
|
if (hasPreviousToolUse) {
|
||||||
val toolUseIds = previous.filterIsInstance<ToolUseContent>().map { it.id }.toSet()
|
val toolUseIds = previous.filterIsInstance<ToolUseContent>().map { it.id }.toSet()
|
||||||
val toolResultIds = last.filterIsInstance<ToolResultContent>().map { it.toolUseId }.toSet()
|
val toolResultIds = last.filterIsInstance<ToolResultContent>().map { it.toolUseId }.toSet()
|
||||||
|
require(toolResultIds.isNotEmpty()) {
|
||||||
|
"tool_use blocks from previous message must be followed by matching tool_result blocks"
|
||||||
|
}
|
||||||
require(toolUseIds == toolResultIds) {
|
require(toolUseIds == toolResultIds) {
|
||||||
"ids of tool_result blocks and tool_use blocks from previous message do not match"
|
"ids of tool_result blocks and tool_use blocks from previous message do not match"
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -8,6 +8,7 @@ import io.modelcontextprotocol.kotlin.sdk.types.ToolUseContent
|
||||||
import kotlinx.serialization.json.JsonObject
|
import kotlinx.serialization.json.JsonObject
|
||||||
import org.junit.jupiter.api.assertDoesNotThrow
|
import org.junit.jupiter.api.assertDoesNotThrow
|
||||||
import kotlin.test.Test
|
import kotlin.test.Test
|
||||||
|
import kotlin.test.assertEquals
|
||||||
import kotlin.test.assertFailsWith
|
import kotlin.test.assertFailsWith
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -83,4 +84,21 @@ class SamplingTest {
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun `validate tool_use requires explicit tool_result in last message`() {
|
||||||
|
val error = assertFailsWith<IllegalArgumentException> {
|
||||||
|
validateSamplingMessages(
|
||||||
|
listOf(
|
||||||
|
SamplingMessage(Role.Assistant, toolUse("c1")),
|
||||||
|
SamplingMessage(Role.User, TextContent("missing result")),
|
||||||
|
),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
"tool_use blocks from previous message must be followed by matching tool_result blocks",
|
||||||
|
error.message,
|
||||||
|
)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue