From 1456e642e32b1b1e065812e7a3ed871ab798ac04 Mon Sep 17 00:00:00 2001 From: m57 <41645758+Hy4ri@users.noreply.github.com> Date: Wed, 30 Sep 2026 00:40:24 +0300 Subject: [PATCH] refactor(#1375): type config.set, config.get, model.options and client.capabilities (#1393) --- .../hermescontrol/data/ws/HermesWsClient.kt | 8 +- .../data/ws/ModelOptionsRepository.kt | 15 +-- .../data/ws/contract/ConfigModel.kt | 41 +++++++ .../data/ws/contract/RpcMethod.kt | 36 ++++++ .../ui/chat/ChatModelSwitchDelegate.kt | 62 ++++------ .../hermescontrol/ui/chat/ChatViewModel.kt | 32 +++--- .../data/ws/ModelOptionsRepositoryTest.kt | 3 +- .../data/ws/RawRpcParamsRatchetTest.kt | 8 -- .../data/ws/contract/ConfigModelTypesTest.kt | 102 +++++++++++++++++ .../ui/chat/ChatModelSwitchDelegateTest.kt | 85 +++++++------- .../ui/chat/ChatViewModelTest.kt | 46 +++----- .../ui/chat/SlashCommandDispatchRpcTest.kt | 108 ++++++++---------- 12 files changed, 339 insertions(+), 207 deletions(-) create mode 100644 app/src/main/java/com/m57/hermescontrol/data/ws/contract/ConfigModel.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/ws/contract/ConfigModelTypesTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt index a9238ebc..4175356e 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/HermesWsClient.kt @@ -10,6 +10,7 @@ import com.m57.hermescontrol.data.remote.NetworkMonitor import com.m57.hermescontrol.data.remote.OkHttpProvider import com.m57.hermescontrol.data.remote.await import com.m57.hermescontrol.data.session.ActiveSessionHolder +import com.m57.hermescontrol.data.ws.contract.ClientCapabilitiesParams import com.m57.hermescontrol.data.ws.contract.PromptSubmitParams import com.m57.hermescontrol.data.ws.contract.RpcMethod import com.m57.hermescontrol.data.ws.contract.RpcMethods @@ -364,8 +365,11 @@ object HermesWsClient { // Older gateways may reject this method; that is harmless. runCatching { send( - WsMethods.CLIENT_CAPABILITIES, - mapOf("server_requests" to true), + RpcMethods.CLIENT_CAPABILITIES.name, + encodeParams( + RpcMethods.CLIENT_CAPABILITIES, + ClientCapabilitiesParams(serverRequests = true), + ), onSent = { id -> capabilityRequestIds.add(id) }, queueIfDisconnected = false, ) diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/ModelOptionsRepository.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/ModelOptionsRepository.kt index 90cd65b8..6509b24f 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/ModelOptionsRepository.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/ModelOptionsRepository.kt @@ -5,6 +5,8 @@ import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.NetworkResult import com.m57.hermescontrol.data.remote.OkHttpProvider import com.m57.hermescontrol.data.remote.safeApiCall +import com.m57.hermescontrol.data.ws.contract.ModelOptionsParams +import com.m57.hermescontrol.data.ws.contract.RpcMethods import kotlinx.coroutines.CancellationException import kotlinx.coroutines.currentCoroutineContext import kotlinx.coroutines.ensureActive @@ -16,13 +18,8 @@ class ModelOptionsRepository( private val connected: () -> Boolean = { HermesWsClient.connectionStatus.value == ConnectionStatus.CONNECTED }, - private val request: suspend (Map) -> Any? = { params -> - val deferred = HermesWsClient.request(WsMethods.MODEL_OPTIONS, params) - try { - deferred.await() - } finally { - if (!deferred.isCompleted) deferred.cancel() - } + private val request: suspend (ModelOptionsParams) -> JsonElement = { params -> + HermesWsClient.call(RpcMethods.MODEL_OPTIONS, params) }, private val rest: suspend (Boolean) -> NetworkResult = { refresh -> safeApiCall { ApiClient.hermesApi.getModelOptions(refresh = refresh, includeUnconfigured = false) } @@ -32,9 +29,7 @@ class ModelOptionsRepository( currentCoroutineContext().ensureActive() if (connected()) { try { - val result = request(mapOf("refresh" to refresh, "include_unconfigured" to false)) - // request() returns raw JsonElement, unlike the pushed-event Map representation. - val json = result as? JsonElement ?: error("Invalid model.options result") + val json = request(ModelOptionsParams(refresh = refresh, includeUnconfigured = false)) return NetworkResult.Success(OkHttpProvider.json.decodeFromJsonElement(json)) } catch (e: CancellationException) { // A disconnected socket may cancel the independent RPC deferred. Only fall back diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/ConfigModel.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/ConfigModel.kt new file mode 100644 index 00000000..c9721c89 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/ConfigModel.kt @@ -0,0 +1,41 @@ +package com.m57.hermescontrol.data.ws.contract + +import kotlinx.serialization.SerialName +import kotlinx.serialization.Serializable + +/** + * `config.set` params. [key] and [value] are always encoded (no defaults); the rest are omitted + * when null. `profile` is injected by WsProfileParams, so it is not a field here. + */ +@Serializable +data class ConfigSetParams( + @SerialName("key") val key: String, + @SerialName("value") val value: String, + @SerialName("session_id") val sessionId: String? = null, + @SerialName("scope") val scope: String? = null, + @SerialName("confirm_expensive_model") val confirmExpensiveModel: Boolean? = null, +) + +@Serializable +data class ConfigGetParams( + @SerialName("key") val key: String, + @SerialName("session_id") val sessionId: String? = null, + @SerialName("cwd") val cwd: String? = null, +) + +/** + * `model.options` params. Flags are `Boolean?` so an explicit `false` is still sent while an + * unset flag is omitted (the client Json has `encodeDefaults = false`). + */ +@Serializable +data class ModelOptionsParams( + @SerialName("session_id") val sessionId: String? = null, + @SerialName("explicit_only") val explicitOnly: Boolean? = null, + @SerialName("include_unconfigured") val includeUnconfigured: Boolean? = null, + @SerialName("refresh") val refresh: Boolean? = null, +) + +@Serializable +data class ClientCapabilitiesParams( + @SerialName("server_requests") val serverRequests: Boolean, +) diff --git a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt index c2020fb4..68bbc53e 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/ws/contract/RpcMethod.kt @@ -395,6 +395,38 @@ object RpcMethods { JsonElement.serializer(), ) + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val CONFIG_SET: RpcMethod = + RpcMethod( + WsMethods.CONFIG_SET, + ConfigSetParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val CONFIG_GET: RpcMethod = + RpcMethod( + WsMethods.CONFIG_GET, + ConfigGetParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val MODEL_OPTIONS: RpcMethod = + RpcMethod( + WsMethods.MODEL_OPTIONS, + ModelOptionsParams.serializer(), + JsonElement.serializer(), + ) + + // Passthrough JsonElement results: callers keep their existing legacy decoders. + val CLIENT_CAPABILITIES: RpcMethod = + RpcMethod( + WsMethods.CLIENT_CAPABILITIES, + ClientCapabilitiesParams.serializer(), + JsonElement.serializer(), + ) + /** Every typed method. GatewayContractTest iterates this; future migrations append here. */ val all: List> = listOf( @@ -446,5 +478,9 @@ object RpcMethods { PROJECTS_LIST, PROFILES_LIST, PROFILES_CONFIGURE, + CONFIG_SET, + CONFIG_GET, + MODEL_OPTIONS, + CLIENT_CAPABILITIES, ) } diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt index 092e8a9a..9840d544 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegate.kt @@ -12,7 +12,8 @@ import com.m57.hermescontrol.data.remote.ApiClient import com.m57.hermescontrol.data.remote.NetworkResult import com.m57.hermescontrol.data.remote.safeApiCall import com.m57.hermescontrol.data.ws.ModelCatalogStore -import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.ConfigSetParams +import com.m57.hermescontrol.data.ws.contract.RpcMethods import com.m57.hermescontrol.data.ws.toAny import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineDispatcher @@ -51,7 +52,7 @@ class ChatModelSwitchDelegate( private val ioDispatcher: CoroutineDispatcher, private val uiState: MutableStateFlow, private val runtimeSessionId: () -> String?, - private val wsSend: (method: String, params: Map, onSent: ((String) -> Unit)?) -> Unit, + private val wsSend: (params: ConfigSetParams, onSent: ((String) -> Unit)?) -> Unit, private val trackRequest: (id: String, method: String) -> Unit, private val addAssistantMessage: (text: String) -> Unit, private val handleSlashCommand: (command: String) -> Unit, @@ -64,7 +65,7 @@ class ChatModelSwitchDelegate( { refresh -> ModelCatalogStore.shared.ensureLoaded(forceRefresh = refresh) }, private val getPinnedModels: () -> List = { AuthManager.getPinnedModels() }, private val savePinnedModels: (List) -> Unit = { AuthManager.savePinnedModels(it) }, - private val wsRequest: (suspend (method: String, params: Map) -> Any?)? = null, + private val wsRequest: (suspend (params: ConfigSetParams) -> JsonElement)? = null, ) { private val pendingModelSwitchRequests = ConcurrentHashMap() private val pendingFastSwitchRequests = ConcurrentHashMap() @@ -300,20 +301,15 @@ class ChatModelSwitchDelegate( unconfirmedTargetModel = spec val switchSeq = ++modelSwitchSequence val params = - mutableMapOf( - "key" to "model", - "value" to spec, - "session_id" to sessionId, + ConfigSetParams( + key = "model", + value = spec, + sessionId = sessionId, + confirmExpensiveModel = true.takeIf { confirmExpensive }, ) - if (confirmExpensive) { - params["confirm_expensive_model"] = true - } scope.launch(ioDispatcher) { - wsSend( - WsMethods.CONFIG_SET, - params, - ) { id -> - trackRequest(id, WsMethods.CONFIG_SET) + wsSend(params) { id -> + trackRequest(id, RpcMethods.CONFIG_SET.name) pendingModelSwitchRequests[id] = ActiveModelSwitch(spec, previousModel, switchSeq) } } @@ -408,14 +404,13 @@ class ChatModelSwitchDelegate( uiState.update { it.copy(isFastModeChanging = true) } scope.launch(ioDispatcher) { wsSend( - WsMethods.CONFIG_SET, - mapOf( - "key" to "fast", - "value" to if (target) "fast" else "normal", - "session_id" to sessionId, + ConfigSetParams( + key = "fast", + value = if (target) "fast" else "normal", + sessionId = sessionId, ), ) { id -> - trackRequest(id, WsMethods.CONFIG_SET) + trackRequest(id, RpcMethods.CONFIG_SET.name) pendingFastSwitchRequests[id] = ActiveFastSwitch(target) } } @@ -539,18 +534,18 @@ class ChatModelSwitchDelegate( scope.launch(ioDispatcher) { try { val params = - mapOf( - "key" to "reasoning", - "value" to level, - "session_id" to sessionId, - "scope" to scopeName, + ConfigSetParams( + key = "reasoning", + value = level, + sessionId = sessionId, + scope = scopeName, ) val result = if (wsRequest != null) { - wsRequest.invoke(WsMethods.CONFIG_SET, params) + wsRequest.invoke(params) } else { - wsSend(WsMethods.CONFIG_SET, params) { reqId -> - trackRequest(reqId, WsMethods.CONFIG_SET) + wsSend(params) { reqId -> + trackRequest(reqId, RpcMethods.CONFIG_SET.name) } null } @@ -558,14 +553,7 @@ class ChatModelSwitchDelegate( if (opSeq == reasoningOpSequence && capturedScope == dataScopeFlow.value) { val ackLevel = if (wsRequest != null) { - // HermesWsClient.request() completes with the RAW JsonElement. - // Tests and a few injected callers may provide an already-decoded map. - val map = - when (result) { - is JsonElement -> result.toAny() as? Map<*, *> - is Map<*, *> -> result - else -> null - } + val map = result?.toAny() as? Map<*, *> val resKey = map?.get("key") as? String val resVal = map?.get("value") as? String if (resKey != "reasoning" || resVal.isNullOrEmpty()) { diff --git a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt index f1dffe6a..b2d7721d 100644 --- a/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt +++ b/app/src/main/java/com/m57/hermescontrol/ui/chat/ChatViewModel.kt @@ -37,6 +37,8 @@ import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.data.ws.JsonRpcError import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.ConfigGetParams +import com.m57.hermescontrol.data.ws.contract.ConfigSetParams import com.m57.hermescontrol.data.ws.contract.PromptBtwParams import com.m57.hermescontrol.data.ws.contract.RpcMethod import com.m57.hermescontrol.data.ws.contract.RpcMethods @@ -694,13 +696,13 @@ class ChatViewModel( ioDispatcher = ioDispatcher, uiState = _uiState, runtimeSessionId = { runtimeSessionId }, - wsSend = { method, params, onSent -> wsClient.send(method, params, onSent) }, + wsSend = { params, onSent -> wsClient.send(RpcMethods.CONFIG_SET, params, onSent) }, trackRequest = { id, method -> trackRequest(id, method) }, addAssistantMessage = { text -> addAssistantMessage(text) }, handleSlashCommand = { cmd -> handleSlashCommand(cmd) }, fetchContextUsage = { fetchContextUsage() }, onModelSwitchInitiated = { onModelSwitchInitiated() }, - wsRequest = { method, params -> wsClient.request(method, params).await() }, + wsRequest = { params -> wsClient.call(RpcMethods.CONFIG_SET, params) }, ).apply { attachScopeObserver(viewModelScope) } @@ -3846,14 +3848,10 @@ class ChatViewModel( viewModelScope.launch(ioDispatcher) { try { val result = - wsClient - .request( - WsMethods.CONFIG_GET, - mapOf( - "key" to "reasoning", - "session_id" to sessionId, - ), - ).await() + wsClient.call( + RpcMethods.CONFIG_GET, + ConfigGetParams(key = "reasoning", sessionId = sessionId), + ) val map = rpcResultMap(result) ?: error("Invalid reasoning config.get response") val value = (map["value"] as? String)?.takeIf { it.isNotBlank() } ?: "unknown" val display = (map["display"] as? String)?.takeIf { it.isNotBlank() } ?: "unknown" @@ -3901,13 +3899,13 @@ class ChatViewModel( viewModelScope.launch(ioDispatcher) { try { val params = - buildMap { - put("key", "reasoning") - put("value", parsed.value) - put("session_id", sessionId) - parsed.scopeName?.let { put("scope", it) } - } - val result = wsClient.request(WsMethods.CONFIG_SET, params).await() + ConfigSetParams( + key = "reasoning", + value = parsed.value, + sessionId = sessionId, + scope = parsed.scopeName, + ) + val result = wsClient.call(RpcMethods.CONFIG_SET, params) val map = rpcResultMap(result) ?: error("Invalid reasoning config.set response") val responseKey = map["key"] as? String val responseValue = map["value"] as? String diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/ModelOptionsRepositoryTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/ModelOptionsRepositoryTest.kt index 2bff3c55..06c5e2d4 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/ModelOptionsRepositoryTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/ModelOptionsRepositoryTest.kt @@ -2,6 +2,7 @@ package com.m57.hermescontrol.data.ws import com.m57.hermescontrol.data.model.ModelOptionsResponse import com.m57.hermescontrol.data.remote.NetworkResult +import com.m57.hermescontrol.data.ws.contract.ModelOptionsParams import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.async import kotlinx.coroutines.awaitCancellation @@ -21,7 +22,7 @@ class ModelOptionsRepositoryTest { ModelOptionsRepository( connected = { true }, request = { params -> - assertEquals(mapOf("refresh" to true, "include_unconfigured" to false), params) + assertEquals(ModelOptionsParams(refresh = true, includeUnconfigured = false), params) Json.parseToJsonElement("""{"providers":[{"slug":"test","name":"Test","models":["a"]}]}""") }, rest = { error("REST must not run") }, diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt index 64595925..0775f856 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/RawRpcParamsRatchetTest.kt @@ -316,17 +316,9 @@ class RawRpcParamsRatchetTest { */ val BASELINE: List = listOf( - "data/ws/HermesWsClient.kt::CLIENT_CAPABILITIES", - "data/ws/ModelOptionsRepository.kt::MODEL_OPTIONS", - "ui/chat/ChatModelSwitchDelegate.kt::CONFIG_SET", - "ui/chat/ChatModelSwitchDelegate.kt::CONFIG_SET", - "ui/chat/ChatModelSwitchDelegate.kt::CONFIG_SET", - "ui/chat/ChatModelSwitchDelegate.kt::CONFIG_SET", "ui/chat/ChatViewModel.kt::COMMANDS_CATALOG", "ui/chat/ChatViewModel.kt::COMMAND_DISPATCH", "ui/chat/ChatViewModel.kt::COMMAND_DISPATCH", - "ui/chat/ChatViewModel.kt::CONFIG_GET", - "ui/chat/ChatViewModel.kt::CONFIG_SET", "ui/chat/ChatViewModel.kt::FILE_ATTACH", "ui/chat/ChatViewModel.kt::IMAGE_ATTACH_BYTES", "ui/chat/ChatViewModel.kt::SLASH_EXEC", diff --git a/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ConfigModelTypesTest.kt b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ConfigModelTypesTest.kt new file mode 100644 index 00000000..8f786b28 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/ws/contract/ConfigModelTypesTest.kt @@ -0,0 +1,102 @@ +package com.m57.hermescontrol.data.ws.contract + +import com.m57.hermescontrol.data.remote.OkHttpProvider +import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class ConfigModelTypesTest { + private val json = OkHttpProvider.json + + @Test + fun configSetEncodesOnlyKeyAndValueByDefault() { + assertEquals( + buildJsonObject { + put("key", "fast") + put("value", "normal") + }, + json.encodeToJsonElement(ConfigSetParams.serializer(), ConfigSetParams(key = "fast", value = "normal")), + ) + } + + @Test + fun configSetEncodesSessionScopeAndConfirmWhenPresent() { + assertEquals( + buildJsonObject { + put("key", "model") + put("value", "gpt-4o --provider openai --session") + put("session_id", "s1") + put("scope", "global") + put("confirm_expensive_model", true) + }, + json.encodeToJsonElement( + ConfigSetParams.serializer(), + ConfigSetParams( + key = "model", + value = "gpt-4o --provider openai --session", + sessionId = "s1", + scope = "global", + confirmExpensiveModel = true, + ), + ), + ) + } + + @Test + fun configGetOmitsNullSessionAndCwd() { + assertEquals( + buildJsonObject { + put("key", "reasoning") + put("session_id", "s1") + }, + json.encodeToJsonElement( + ConfigGetParams.serializer(), + ConfigGetParams(key = "reasoning", sessionId = "s1"), + ), + ) + } + + @Test + fun modelOptionsKeepsExplicitFalseAndOmitsUnsetFlags() { + assertEquals( + buildJsonObject { + put("refresh", true) + put("include_unconfigured", false) + }, + json.encodeToJsonElement( + ModelOptionsParams.serializer(), + ModelOptionsParams(refresh = true, includeUnconfigured = false), + ), + ) + assertEquals( + buildJsonObject { }, + json.encodeToJsonElement(ModelOptionsParams.serializer(), ModelOptionsParams()), + ) + } + + @Test + fun clientCapabilitiesAlwaysEncodesServerRequests() { + assertEquals( + buildJsonObject { put("server_requests", true) }, + json.encodeToJsonElement( + ClientCapabilitiesParams.serializer(), + ClientCapabilitiesParams(serverRequests = true), + ), + ) + } + + @Test + fun descriptorsAreRegisteredWithContractNames() { + listOf( + RpcMethods.CONFIG_SET to "config.set", + RpcMethods.CONFIG_GET to "config.get", + RpcMethods.MODEL_OPTIONS to "model.options", + RpcMethods.CLIENT_CAPABILITIES to "client.capabilities", + ).forEach { (method, name) -> + assertTrue("$name not in RpcMethods.all", method in RpcMethods.all) + assertEquals(name, method.name) + } + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt index 94370290..0e4dacdc 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatModelSwitchDelegateTest.kt @@ -8,7 +8,7 @@ import com.m57.hermescontrol.data.model.ModelProvider import com.m57.hermescontrol.data.model.PinnedModel import com.m57.hermescontrol.data.remote.NetworkError import com.m57.hermescontrol.data.remote.NetworkResult -import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.ConfigSetParams import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow @@ -18,6 +18,8 @@ import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest +import kotlinx.serialization.json.JsonElement +import kotlinx.serialization.json.JsonNull import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertNull @@ -31,8 +33,7 @@ class ChatModelSwitchDelegateTest { private val uiState = MutableStateFlow(ChatUiState(currentSessionId = "sess-1", currentSessionModel = "anthropic/claude-3")) private var runtimeId: String? = "runtime-1" - private val sentMethods = mutableListOf() - private val sentParams = mutableListOf>() + private val sentParams = mutableListOf() private val slashCommands = mutableListOf() private val assistantMessages = mutableListOf() private var contextRefetched = 0 @@ -71,10 +72,9 @@ class ChatModelSwitchDelegateTest { ioDispatcher = testDispatcher, uiState = uiState, runtimeSessionId = { runtimeId }, - wsSend = { method, params, onSent -> - sentMethods.add(method) + wsSend = { params, onSent -> sentParams.add(params) - onSent?.invoke("req-${sentMethods.size}") + onSent?.invoke("req-${sentParams.size}") }, trackRequest = { _, _ -> }, addAssistantMessage = { assistantMessages.add(it) }, @@ -304,8 +304,9 @@ class ChatModelSwitchDelegateTest { delegate.handleModelSwitch("/MODEL gpt-4o --provider openai --session") advanceUntilIdle() - assertEquals(listOf(WsMethods.CONFIG_SET), sentMethods) - assertEquals("gpt-4o --provider openai --session", sentParams.first()["value"]) + assertEquals(1, sentParams.size) + assertEquals("model", sentParams.first().key) + assertEquals("gpt-4o --provider openai --session", sentParams.first().value) delegate.handleConfigSetResult( id = "req-1", @@ -320,8 +321,8 @@ class ChatModelSwitchDelegateTest { delegate.confirmModelSwitchExpensive() advanceUntilIdle() - assertEquals(2, sentMethods.size) - assertEquals(true, sentParams[1]["confirm_expensive_model"]) + assertEquals(2, sentParams.size) + assertEquals(true, sentParams[1].confirmExpensiveModel) } @Test @@ -366,7 +367,6 @@ class ChatModelSwitchDelegateTest { // Switch to model with fast capability delegate.sendSlashModel("openai", "gpt-4o") advanceUntilIdle() - sentMethods.clear() sentParams.clear() assertTrue(uiState.value.currentModelCapabilities?.fast == true) @@ -377,10 +377,10 @@ class ChatModelSwitchDelegateTest { advanceUntilIdle() assertTrue(uiState.value.isFastModeChanging) assertFalse(uiState.value.fastMode) // Still unconfirmed! - assertEquals(listOf(WsMethods.CONFIG_SET), sentMethods) - assertEquals("fast", sentParams.first()["key"]) - assertEquals("fast", sentParams.first()["value"]) - assertEquals("runtime-1", sentParams.first()["session_id"]) + assertEquals(1, sentParams.size) + assertEquals("fast", sentParams.first().key) + assertEquals("fast", sentParams.first().value) + assertEquals("runtime-1", sentParams.first().sessionId) // Backend acknowledges delegate.handleConfigSetResult("req-1", mapOf("key" to "fast", "value" to "fast")) @@ -391,7 +391,7 @@ class ChatModelSwitchDelegateTest { delegate.toggleFastMode() advanceUntilIdle() assertTrue(uiState.value.isFastModeChanging) - assertEquals("normal", sentParams.last()["value"]) + assertEquals("normal", sentParams.last().value) delegate.handleConfigSetResult("req-2", mapOf("key" to "fast", "value" to "normal")) assertFalse(uiState.value.fastMode) @@ -405,7 +405,6 @@ class ChatModelSwitchDelegateTest { advanceUntilIdle() delegate.sendSlashModel("openai", "gpt-4o") advanceUntilIdle() - sentMethods.clear() sentParams.clear() assertTrue(uiState.value.currentModelCapabilities?.fast == true) @@ -420,10 +419,10 @@ class ChatModelSwitchDelegateTest { assertEquals(false, uiState.value.currentModelCapabilities?.fast) // Further toggle attempts are no-ops - val sentCount = sentMethods.size + val sentCount = sentParams.size delegate.toggleFastMode() advanceUntilIdle() - assertEquals(sentCount, sentMethods.size) + assertEquals(sentCount, sentParams.size) } @Test @@ -470,9 +469,8 @@ class ChatModelSwitchDelegateTest { @Test fun setReasoningLevel_awaitsRequest_updatesOnAck_andPreventsConcurrentPick() = testScope.runTest { - val gate = CompletableDeferred() - val reqMethods = mutableListOf() - val reqParams = mutableListOf>() + val gate = CompletableDeferred() + val reqParams = mutableListOf() val customDelegate = ChatModelSwitchDelegate( @@ -480,14 +478,13 @@ class ChatModelSwitchDelegateTest { ioDispatcher = testDispatcher, uiState = uiState, runtimeSessionId = { runtimeId }, - wsSend = { _, _, _ -> }, + wsSend = { _, _ -> }, trackRequest = { _, _ -> }, addAssistantMessage = {}, handleSlashCommand = {}, fetchContextUsage = {}, dataScopeFlow = dataScope, - wsRequest = { method, params -> - reqMethods.add(method) + wsRequest = { params -> reqParams.add(params) gate.await() }, @@ -503,14 +500,15 @@ class ChatModelSwitchDelegateTest { assertEquals("ultra", uiState.value.reasoningLevel) assertEquals("high", uiState.value.pendingReasoningLevel) assertNull(uiState.value.reasoningWireLevel) - assertEquals(listOf(WsMethods.CONFIG_SET), reqMethods) - assertEquals("session", reqParams.first()["scope"]) - assertEquals("high", reqParams.first()["value"]) + assertEquals(1, reqParams.size) + assertEquals("reasoning", reqParams.first().key) + assertEquals("session", reqParams.first().scope) + assertEquals("high", reqParams.first().value) // Second pick while in-flight is rejected (serialized) customDelegate.setReasoningLevel("low") runCurrent() - assertEquals(1, reqMethods.size) + assertEquals(1, reqParams.size) // Simulate session.info winning the race and publishing the fresh // authoritative wire before the RPC ACK resumes the delegate. @@ -540,13 +538,13 @@ class ChatModelSwitchDelegateTest { ioDispatcher = testDispatcher, uiState = uiState, runtimeSessionId = { runtimeId }, - wsSend = { _, _, _ -> }, + wsSend = { _, _ -> }, trackRequest = { _, _ -> }, addAssistantMessage = {}, handleSlashCommand = {}, fetchContextUsage = {}, dataScopeFlow = dataScope, - wsRequest = { _, _ -> throw RuntimeException("4002 unknown reasoning value") }, + wsRequest = { throw RuntimeException("4002 unknown reasoning value") }, ) customDelegate.setReasoningLevel("ultra") @@ -560,7 +558,7 @@ class ChatModelSwitchDelegateTest { @Test fun setReasoningLevel_whenRuntimeSessionNullOrBlank_doesNotSend() = testScope.runTest { - val reqMethods = mutableListOf() + val reqParams = mutableListOf() runtimeId = null val customDelegate = @@ -569,22 +567,22 @@ class ChatModelSwitchDelegateTest { ioDispatcher = testDispatcher, uiState = uiState, runtimeSessionId = { runtimeId }, - wsSend = { _, _, _ -> }, + wsSend = { _, _ -> }, trackRequest = { _, _ -> }, addAssistantMessage = {}, handleSlashCommand = {}, fetchContextUsage = {}, dataScopeFlow = dataScope, - wsRequest = { method, _ -> - reqMethods.add(method) - null + wsRequest = { params -> + reqParams.add(params) + JsonNull }, ) customDelegate.setReasoningLevel("high") advanceUntilIdle() - assertTrue(reqMethods.isEmpty()) + assertTrue(reqParams.isEmpty()) assertNull(uiState.value.pendingReasoningLevel) assertNull(uiState.value.reasoningLevel) } @@ -592,20 +590,20 @@ class ChatModelSwitchDelegateTest { @Test fun setReasoningLevel_staleScopeOrReset_dropsLateAck() = testScope.runTest { - val gate = CompletableDeferred() + val gate = CompletableDeferred() val customDelegate = ChatModelSwitchDelegate( scope = testScope, ioDispatcher = testDispatcher, uiState = uiState, runtimeSessionId = { runtimeId }, - wsSend = { _, _, _ -> }, + wsSend = { _, _ -> }, trackRequest = { _, _ -> }, addAssistantMessage = {}, handleSlashCommand = {}, fetchContextUsage = {}, dataScopeFlow = dataScope, - wsRequest = { _, _ -> gate.await() }, + wsRequest = { gate.await() }, ) customDelegate.setReasoningLevel("high") @@ -617,7 +615,12 @@ class ChatModelSwitchDelegateTest { assertNull(uiState.value.pendingReasoningLevel) // Late RPC response arrives - gate.complete(mapOf("key" to "reasoning", "value" to "high")) + gate.complete( + kotlinx.serialization.json.buildJsonObject { + put("key", kotlinx.serialization.json.JsonPrimitive("reasoning")) + put("value", kotlinx.serialization.json.JsonPrimitive("high")) + }, + ) advanceUntilIdle() assertNull(uiState.value.reasoningLevel) diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt index a2a0be4b..10f52473 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/ChatViewModelTest.kt @@ -35,6 +35,8 @@ import com.m57.hermescontrol.data.ws.JsonRpcError import com.m57.hermescontrol.data.ws.ModelCatalogStore import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.ConfigSetParams +import com.m57.hermescontrol.data.ws.contract.RpcMethods import com.m57.hermescontrol.notification.TurnCorrelationTracker import com.m57.hermescontrol.ui.chat.fakes.FakeChatPersistenceRepository import com.m57.hermescontrol.ui.chat.fakes.FakeSlashUsageStore @@ -1295,15 +1297,9 @@ class ChatViewModelTest { // commands). NOT command.dispatch (4018s on /model), NOT prompt.submit // (LLM would treat it as text). Capture the config.set params. val modelCalls = mutableListOf>() - every { HermesWsClient.send(WsMethods.CONFIG_SET, any(), any()) } answers { - val params = arg>(1) - modelCalls.add( - Triple( - params["key"] as String, - params["value"] as String, - params["session_id"] as String, - ), - ) + every { HermesWsClient.send(RpcMethods.CONFIG_SET, any(), any()) } answers { + val params = arg(1) + modelCalls.add(Triple(params.key, params.value, params.sessionId.orEmpty())) "req-cfg-${modelCalls.size}" } @@ -1315,7 +1311,7 @@ class ChatViewModelTest { "openai/gpt-4o", viewModel.uiState.value.currentSessionModel, ) - verify { HermesWsClient.send(WsMethods.CONFIG_SET, any(), any()) } + verify { HermesWsClient.send(RpcMethods.CONFIG_SET, any(), any()) } val call = modelCalls.firstOrNull { it.first == "model" } assertNotNull("selection must route through config.set key=model", call) assertEquals("gpt-4o --provider openai --session", call!!.second) @@ -1340,7 +1336,7 @@ class ChatViewModelTest { // (key="model"), which the gateway routes to _apply_model_switch. NOT // command.dispatch (4018s on /model) and NOT prompt.submit (LLM would // treat it as text). - verify { HermesWsClient.send(WsMethods.CONFIG_SET, any(), any()) } + verify { HermesWsClient.send(RpcMethods.CONFIG_SET, any(), any()) } } @Test @@ -1353,15 +1349,9 @@ class ChatViewModelTest { // slash prefix must be stripped, or parse_model_flags on the // backend won't recognize it and the hot-swap silently fails. val modelCalls = mutableListOf>() - every { HermesWsClient.send(WsMethods.CONFIG_SET, any(), any()) } answers { - val params = arg>(1) - modelCalls.add( - Triple( - params["key"] as String, - params["value"] as String, - params["session_id"] as String, - ), - ) + every { HermesWsClient.send(RpcMethods.CONFIG_SET, any(), any()) } answers { + val params = arg(1) + modelCalls.add(Triple(params.key, params.value, params.sessionId.orEmpty())) "req-cfg-ci-${modelCalls.size}" } @@ -1388,9 +1378,9 @@ class ChatViewModelTest { val (viewModel, sessionId) = createViewModelWithSession() var lastReqId = "" - val capturedParams = mutableListOf>() - every { HermesWsClient.send(WsMethods.CONFIG_SET, any(), any()) } answers { - val params = arg>(1) + val capturedParams = mutableListOf() + every { HermesWsClient.send(RpcMethods.CONFIG_SET, any(), any()) } answers { + val params = arg(1) capturedParams.add(params) val id = "req-confirm-${capturedParams.size}" lastReqId = id @@ -1403,7 +1393,7 @@ class ChatViewModelTest { advanceUntilIdle() assertEquals(1, capturedParams.size) - assertNull(capturedParams[0]["confirm_expensive_model"]) + assertNull(capturedParams[0].confirmExpensiveModel) // Backend returns confirm_required = true mockEventsFlow.emit( @@ -1429,12 +1419,12 @@ class ChatViewModelTest { assertNull(viewModel.uiState.value.modelSwitchConfirmMessage) assertEquals(2, capturedParams.size) - assertEquals(true, capturedParams[1]["confirm_expensive_model"]) + assertEquals(true, capturedParams[1].confirmExpensiveModel) assertEquals( "muse-spark-1.3-contributor-free --provider opencode-free --session", - capturedParams[1]["value"], + capturedParams[1].value, ) - assertEquals(sessionId, capturedParams[1]["session_id"]) + assertEquals(sessionId, capturedParams[1].sessionId) } @Test @@ -1443,7 +1433,7 @@ class ChatViewModelTest { val (viewModel, _) = createViewModelWithSession() var lastReqId = "" - every { HermesWsClient.send(WsMethods.CONFIG_SET, any(), any()) } answers { + every { HermesWsClient.send(RpcMethods.CONFIG_SET, any(), any()) } answers { val id = "req-cfg-dismiss" lastReqId = id arg<((String) -> Unit)?>(2)?.invoke(id) diff --git a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt index 1447a65e..5c301f69 100644 --- a/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/ui/chat/SlashCommandDispatchRpcTest.kt @@ -12,6 +12,9 @@ import com.m57.hermescontrol.data.ws.ConnectionStatus import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.data.ws.WsEvent import com.m57.hermescontrol.data.ws.WsMethods +import com.m57.hermescontrol.data.ws.contract.ConfigGetParams +import com.m57.hermescontrol.data.ws.contract.ConfigSetParams +import com.m57.hermescontrol.data.ws.contract.RpcMethods import com.m57.hermescontrol.ui.chat.fakes.FakeChatPersistenceRepository import com.m57.hermescontrol.ui.chat.fakes.FakeSlashUsageStore import io.mockk.coEvery @@ -21,6 +24,7 @@ import io.mockk.mockkObject import io.mockk.mockkStatic import io.mockk.slot import io.mockk.unmockkAll +import io.mockk.verify import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -34,6 +38,7 @@ import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.setMain import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -271,35 +276,27 @@ class SlashCommandDispatchRpcTest { runTest { val (vm, sessionId) = createViewModelWithSession() - val methodCalls = mutableListOf() - val paramsCalls = mutableListOf>() - every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + val setCalls = mutableListOf() + coEvery { + HermesWsClient.call(RpcMethods.CONFIG_SET, capture(setCalls), any(), any()) } answers { - val result = - if (arg(0) == WsMethods.CONFIG_SET) { - kotlinx.serialization.json.buildJsonObject { - put("key", kotlinx.serialization.json.JsonPrimitive("reasoning")) - put("value", kotlinx.serialization.json.JsonPrimitive("high")) - } - } else { - kotlinx.serialization.json.JsonNull - } - CompletableDeferred(result) + kotlinx.serialization.json.buildJsonObject { + put("key", kotlinx.serialization.json.JsonPrimitive("reasoning")) + put("value", kotlinx.serialization.json.JsonPrimitive("high")) + } } vm.sendMessage("/reasoning high --global") advanceUntilIdle() - val configIndex = methodCalls.indexOf(WsMethods.CONFIG_SET) - assertTrue("expected CONFIG_SET, got $methodCalls", configIndex >= 0) - val params = paramsCalls[configIndex] - assertEquals("reasoning", params["key"]) - assertEquals("high", params["value"]) - assertEquals(sessionId, params["session_id"]) - assertEquals("global", params["scope"]) + assertEquals("expected exactly one config.set, got $setCalls", 1, setCalls.size) + val params = setCalls.single() + assertEquals("reasoning", params.key) + assertEquals("high", params.value) + assertEquals(sessionId, params.sessionId) + assertEquals("global", params.scope) assertEquals("high", vm.uiState.value.reasoningLevel) - assertTrue(WsMethods.COMMAND_DISPATCH !in methodCalls) + verify(exactly = 0) { HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) } } @Test @@ -307,33 +304,26 @@ class SlashCommandDispatchRpcTest { runTest { val (vm, sessionId) = createViewModelWithSession() - val methodCalls = mutableListOf() - val paramsCalls = mutableListOf>() - every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + val setCalls = mutableListOf() + coEvery { + HermesWsClient.call(RpcMethods.CONFIG_SET, capture(setCalls), any(), any()) } answers { - val result = - if (arg(0) == WsMethods.CONFIG_SET) { - kotlinx.serialization.json.buildJsonObject { - put("key", kotlinx.serialization.json.JsonPrimitive("reasoning")) - put("value", kotlinx.serialization.json.JsonPrimitive("show")) - } - } else { - kotlinx.serialization.json.JsonNull - } - CompletableDeferred(result) + kotlinx.serialization.json.buildJsonObject { + put("key", kotlinx.serialization.json.JsonPrimitive("reasoning")) + put("value", kotlinx.serialization.json.JsonPrimitive("show")) + } } vm.sendMessage("/reasoning show") advanceUntilIdle() - val configIndex = methodCalls.indexOf(WsMethods.CONFIG_SET) - assertTrue("expected CONFIG_SET, got $methodCalls", configIndex >= 0) - val params = paramsCalls[configIndex] - assertEquals("reasoning", params["key"]) - assertEquals("show", params["value"]) - assertEquals(sessionId, params["session_id"]) - assertTrue(WsMethods.COMMAND_DISPATCH !in methodCalls) + assertEquals("expected exactly one config.set, got $setCalls", 1, setCalls.size) + val params = setCalls.single() + assertEquals("reasoning", params.key) + assertEquals("show", params.value) + assertEquals(sessionId, params.sessionId) + assertNull(params.scope) + verify(exactly = 0) { HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) } assertEquals( "reasoning: show", vm.uiState.value.messages @@ -347,32 +337,24 @@ class SlashCommandDispatchRpcTest { runTest { val (vm, sessionId) = createViewModelWithSession() - val methodCalls = mutableListOf() - val paramsCalls = mutableListOf>() - every { - HermesWsClient.request(capture(methodCalls), capture(paramsCalls), any()) + val getCalls = mutableListOf() + coEvery { + HermesWsClient.call(RpcMethods.CONFIG_GET, capture(getCalls), any(), any()) } answers { - val result = - if (arg(0) == WsMethods.CONFIG_GET) { - kotlinx.serialization.json.buildJsonObject { - put("value", kotlinx.serialization.json.JsonPrimitive("ultra")) - put("display", kotlinx.serialization.json.JsonPrimitive("hide")) - } - } else { - kotlinx.serialization.json.JsonNull - } - CompletableDeferred(result) + kotlinx.serialization.json.buildJsonObject { + put("value", kotlinx.serialization.json.JsonPrimitive("ultra")) + put("display", kotlinx.serialization.json.JsonPrimitive("hide")) + } } vm.sendMessage("/reasoning") advanceUntilIdle() - val configIndex = methodCalls.indexOf(WsMethods.CONFIG_GET) - assertTrue("expected CONFIG_GET, got $methodCalls", configIndex >= 0) - val params = paramsCalls[configIndex] - assertEquals("reasoning", params["key"]) - assertEquals(sessionId, params["session_id"]) - assertTrue(WsMethods.COMMAND_DISPATCH !in methodCalls) + assertEquals("expected exactly one config.get, got $getCalls", 1, getCalls.size) + val params = getCalls.single() + assertEquals("reasoning", params.key) + assertEquals(sessionId, params.sessionId) + verify(exactly = 0) { HermesWsClient.send(WsMethods.COMMAND_DISPATCH, any(), any()) } assertEquals( "reasoning: ultra ยท display hide", vm.uiState.value.messages