feat(nano): Implement cross-platform action dispatch system#490
feat(nano): Implement cross-platform action dispatch system#490
Conversation
Implements GitHub issue #489 - Cross-platform action dispatch system for NanoRenderer. ## Changes ### Core Action System (xuiper-ui) - Add NanoActionHandler interface with methods for each action type: - handleStateMutation, handleNavigate, handleFetch, handleShowToast, handleSequence - handleCustomAction for user-defined actions (AddTask, DeleteTask, etc.) - Add ActionResult sealed class (Success, SuccessWithValue, Error, Pending) - Add NanoActionContext interface for state access with operator get/set - Add BaseNanoActionHandler abstract class with default implementations - Add CustomActionHandler typealias for user-defined action handlers ### State Management (xuiper-ui) - Extend NanoState to implement NanoActionContext - Add mutate() method supporting MutationOp (SET, ADD, SUBTRACT, APPEND, REMOVE) - Add nested path support for get/set (e.g., 'user.name') ### Render Context (xuiper-ui) - Add NanoRenderContext bundling state, actionHandler, and theme - Add static() factory for no-op rendering ### Compose Implementation (mpp-ui) - Add ComposeActionHandler with platform-specific implementations: - Navigate: Desktop.browse() or custom callback - Fetch: HttpClient with async support - ShowToast: JOptionPane or custom callback - Custom action registry ### React/TypeScript Implementation (mpp-vscode) - Add ReactActionHandler with browser-native implementations: - Navigate: window.open/location - Fetch: native fetch API - ShowToast: alert or custom callback - Add ReactStateContext for React useState integration - Add parseNanoAction helper for IR parsing ### HtmlRenderer Updates - Add data-actions attribute for action bindings - Add data-bindings attribute for two-way bindings - Update Button, Input, Checkbox, TextArea, Select components Closes #489
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughImplements a cross-platform action dispatch system for NanoRenderer: adds NanoActionHandler API and BaseNanoActionHandler, platform implementations for Compose Desktop and React, expands Navigate action, makes NanoState an actionable NanoActionContext with nested-path mutations, wires renderers and HTML output to emit data-actions/data-bindings, and adds navigation-focused DSL/tests and task-boundary hooks. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component (Compose/React)
participant Handler as NanoActionHandler (Base / Platform)
participant State as NanoState / NanoActionContext
participant Custom as Custom Action Registry
participant Effect as Effects (Nav/Toast/HTTP)
UI->>Handler: handleAction(NanoAction)
alt StateMutation
Handler->>State: mutate(path, op, value)
State-->>UI: emit state update
else Navigate
Handler->>Effect: buildUrl()/onNavigate(...)
else Fetch
Handler->>Handler: buildRequestBody(context)
Handler->>Effect: perform HTTP request
Effect-->>Handler: response/error
Handler->>State: set response bindings / update loading state
Handler->>UI: invoke onFetchComplete/onSuccess/onError
else ShowToast
Handler->>Effect: onToast(message) or platform fallback
else Sequence
loop for each nested action
Handler->>Handler: handleAction(nested)
end
else CustomAction
Handler->>Custom: lookup and invoke handler(name, payload)
Custom-->>Handler: ActionResult
end
Handler-->>UI: return ActionResult (Success/Error/Pending)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (17)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (9)
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/HtmlRenderer.kt (2)
285-309: Manual JSON building is fragile and may produce invalid output.The manual JSON construction doesn't escape special characters (quotes, backslashes) in keys or values. If
action.type, event names, or payload values contain"or\, the generated JSON will be malformed.Consider using
kotlinx.serialization.json(already imported forjsonPrimitive) for proper JSON encoding:+import kotlinx.serialization.json.buildJsonObject +import kotlinx.serialization.json.put +import kotlinx.serialization.json.putJsonObject private fun renderActionAttribute(ir: NanoIR): String { val actions = ir.actions ?: return "" if (actions.isEmpty()) return "" - val actionsJson = buildString { - append("{") - actions.entries.forEachIndexed { index, (event, action) -> - if (index > 0) append(",") - append("\"$event\":{") - append("\"type\":\"${action.type}\"") - action.payload?.let { payload -> - append(",\"payload\":{") - payload.entries.forEachIndexed { pIndex, (key, value) -> - if (pIndex > 0) append(",") - append("\"$key\":$value") - } - append("}") - } - append("}") - } - append("}") - } + val actionsJson = buildJsonObject { + actions.forEach { (event, action) -> + putJsonObject(event) { + put("type", action.type) + action.payload?.let { payload -> + putJsonObject("payload") { + payload.forEach { (key, value) -> + // value is already JsonElement + put(key, value) + } + } + } + } + } + }.toString() return " data-actions='$actionsJson'" }
316-333: Same escaping concern applies to binding attribute JSON.The
binding.expressionvalue (e.g.,state.new_task) is interpolated directly without escaping. While typical expressions may be safe, a malformed or malicious expression containing"could break the JSON.Apply the same
buildJsonObjectapproach for consistency and safety.xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.kt (1)
54-58: Silent failure when registering custom actions on incompatible handlers.
registerActionsilently does nothing if the action handler is not aBaseNanoActionHandler. This could lead to subtle bugs where users expect their custom actions to work but they're never registered.Consider returning a Boolean or throwing an exception for clarity:
- fun registerAction(name: String, handler: CustomActionHandler) { + fun registerAction(name: String, handler: CustomActionHandler): Boolean { if (actionHandler is cc.unitmesh.xuiper.action.BaseNanoActionHandler) { actionHandler.registerCustomAction(name, handler) + return true } + return false }xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.kt (1)
180-194: Numeric operations always produce Double, losing original type.
ADDandSUBTRACTconvert toDoublefor all numeric types. This means anIntvalue of5afterADD 1becomes6.0, which may break code expectingInt.Consider preserving the original numeric type:
MutationOp.ADD -> { when { currentValue is Int && value is Int -> currentValue + value currentValue is Long && value is Number -> currentValue + value.toLong() currentValue is Number && value is Number -> currentValue.toDouble() + value.toDouble() currentValue is String && value != null -> currentValue + value.toString() else -> value } }mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt (1)
78-92: Redundantelsebranch may hide futureHttpMethodvariants.If
HttpMethodis an enum or sealed class, theelsebranch prevents the compiler from warning about unhandled cases when new methods (e.g.,PATCH) are added.when (fetch.method) { HttpMethod.GET -> requestBuilder.GET() HttpMethod.POST -> { ... } HttpMethod.PUT -> { ... } HttpMethod.DELETE -> requestBuilder.DELETE() - else -> requestBuilder.GET() + HttpMethod.PATCH -> { + val body = buildRequestBody(fetch.body, context) + requestBuilder.method("PATCH", HttpRequest.BodyPublishers.ofString(body)) + requestBuilder.header("Content-Type", fetch.contentType.mimeType) + } }mpp-vscode/webview/src/components/nano/NanoActionHandler.ts (3)
254-296: Handle undefined values in mutation operations more explicitly.The
mutatemethod has edge cases whencurrentValueisundefined:
ADDwith undefined number falls through to set value directly (line 268)SUBTRACTwith undefined keeps undefined (line 275)These behaviors might be acceptable but could lead to unexpected results. Consider adding explicit handling or validation for undefined values.
Example improvement for numeric operations:
case 'ADD': - if (typeof currentValue === 'number' && typeof value === 'number') { + if (typeof value === 'number') { + newValue = (typeof currentValue === 'number' ? currentValue : 0) + value; + } else if (typeof currentValue === 'string') { - newValue = currentValue + value; - } else if (typeof currentValue === 'string') { - newValue = currentValue + String(value); - } else { + newValue = currentValue + String(value); + } else { newValue = value; } break; case 'SUBTRACT': - if (typeof currentValue === 'number' && typeof value === 'number') { + if (typeof currentValue === 'number' && typeof value === 'number') { newValue = currentValue - value; } else { - newValue = currentValue; + newValue = 0; } break;
324-349: Consider validating required fields in parseNanoAction.The parser uses empty string defaults for required fields like
path,to, andurl(lines 329, 334, 338), which could lead to runtime errors or unexpected behavior when these actions are executed. Additionally, unknown action types are silently treated as custom actions (line 347), which might mask typos.Consider either:
- Throwing an error for missing required fields
- Returning
nullwhen validation fails and handling it at the call site- Adding a validation step before execution
Example:
export function parseNanoAction(actionIR: { type: string; payload?: Record<string, any> }): NanoAction | null { switch (actionIR.type) { case 'StateMutation': if (!actionIR.payload?.path) return null; return { type: 'StateMutation', path: actionIR.payload.path, operation: (actionIR.payload.operation ?? 'SET') as MutationOp, value: actionIR.payload.value, }; case 'Navigate': if (!actionIR.payload?.to) return null; return { type: 'Navigate', to: actionIR.payload.to }; // ... similar for other required fields } }
303-317: Document nested object creation behavior in setNestedValue.The method creates an empty object when the intermediate path doesn't exist or isn't an object (lines 311-312). This silently overwrites non-object values, which might be surprising. Consider adding a comment explaining this behavior or validating the path structure.
Add a comment:
private setNestedValue(obj: Record<string, any>, parts: string[], value: any): void { if (parts.length === 0) return; const key = parts[0]; if (parts.length === 1) { obj[key] = value; } else { + // Create intermediate objects if they don't exist or aren't objects + // This will overwrite any non-object values in the path if (!(key in obj) || typeof obj[key] !== 'object') { obj[key] = {}; } obj[key] = { ...obj[key] }; this.setNestedValue(obj[key], parts.slice(1), value); } }xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.kt (1)
106-106: Document the Pending action result pattern.The
Pendingvariant with a callback is a useful pattern for async operations, but the callback parameteronComplete: (ActionResult) -> Unitisn't well documented. Consider adding a usage example or more detailed documentation about when and how this callback is invoked.Enhance the documentation:
/** * Action is pending (async operation in progress) - * The callback will be invoked when the action completes + * + * Use this when the action cannot complete synchronously. + * The callback will be invoked by the handler when the async operation completes. + * + * Example: + * ```kotlin + * ActionResult.Pending { result -> + * when (result) { + * is ActionResult.Success -> // handle success + * is ActionResult.Error -> // handle error + * } + * } + * ``` */ data class Pending(val onComplete: (ActionResult) -> Unit) : ActionResult()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt(1 hunks)mpp-vscode/webview/src/components/nano/NanoActionHandler.ts(1 hunks)mpp-vscode/webview/src/components/nano/index.ts(1 hunks)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.kt(1 hunks)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/HtmlRenderer.kt(3 hunks)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.kt(1 hunks)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.kt(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code in Kotlin Multiplatform projects
For @JsExport in Kotlin, use concrete classes (not interfaces) and Promise (not Flow) for JavaScript interoperability
Avoid emoji and UTF-8 characters in WASM code
Files:
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.ktxuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.ktmpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.ktxuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.ktxuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/HtmlRenderer.kt
**/*Compose*.kt
📄 CodeRabbit inference engine (AGENTS.md)
For Compose UI design, use
AutoDevColorsorMaterialTheme.colorScheme
Files:
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt
{**/Compose*.kt,mpp-ui/src/jsMain/typescript/**/*.ts}
📄 CodeRabbit inference engine (AGENTS.md)
Never use hardcoded colors - always use design tokens from the design system
Files:
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt
**/*Renderer.kt
📄 CodeRabbit inference engine (AGENTS.md)
When modifying
CodingAgentRendererin Kotlin, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer,JsRendererAdapter
Files:
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/HtmlRenderer.kt
🧠 Learnings (2)
📚 Learning: 2025-12-06T13:54:33.558Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T13:54:33.558Z
Learning: Applies to mpp-vscode/src/{bridge,providers}/**/*.ts : When modifying `CodingAgentRenderer` in VSCode, update implementations in `mpp-vscode/src/bridge/mpp-core.ts` and `mpp-vscode/src/providers/chat-view.ts`
Applied to files:
mpp-vscode/webview/src/components/nano/index.ts
📚 Learning: 2025-12-06T13:54:33.558Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T13:54:33.558Z
Learning: Applies to **/*Renderer.kt : When modifying `CodingAgentRenderer` in Kotlin, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, `JsRendererAdapter`
Applied to files:
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/HtmlRenderer.kt
🧬 Code graph analysis (3)
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.kt (2)
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.kt (3)
get(126-148)get(131-131)set(137-137)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoStateManager.kt (2)
get(54-54)set(59-61)
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt (3)
mpp-vscode/webview/src/components/nano/NanoActionHandler.ts (1)
handleAction(86-103)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.kt (3)
handleAction(16-81)handleAction(25-25)handleAction(181-189)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.kt (1)
handleAction(77-77)
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.kt (3)
mpp-vscode/webview/src/components/nano/NanoActionHandler.ts (6)
handleStateMutation(105-115)handleNavigate(117-136)handleFetch(138-167)handleShowToast(169-184)handleSequence(186-197)handleAction(86-103)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.kt (6)
handleStateMutation(78-78)handleNavigate(79-79)handleFetch(80-80)handleShowToast(81-81)handleSequence(82-82)handleAction(77-77)mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt (3)
handleNavigate(44-61)handleFetch(63-149)handleShowToast(151-166)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: Test mpp-core (KMP)
- GitHub Check: Build
🔇 Additional comments (9)
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/HtmlRenderer.kt (1)
145-161: LGTM!The button rendering correctly incorporates action and binding attributes with proper tag closure. The pattern is consistently applied across all interactive components.
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.kt (1)
76-84: LGTM!The
NoOpActionHandlercorrectly provides a no-op implementation for static rendering scenarios. All action types returnSuccess, which is appropriate for non-interactive contexts.mpp-vscode/webview/src/components/nano/index.ts (1)
20-33: LGTM!Clean organization of action handling exports. The separation of runtime exports (
ReactActionHandler,ReactStateContext,parseNanoAction) from type exports follows good TypeScript patterns for tree-shaking and API clarity.xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.kt (2)
147-172: Nested pathsetdoesn't support list index updates.The
getmethod supports accessing list elements via numeric path segments (e.g.,items.0), butsetalways creates nestedMaps. This asymmetry could be confusing — users can readstate["items.0"]but writing to it creates a map with key"0"instead of updating the list element.Is this limitation intentional? If list element updates are needed, consider documenting this behavior or adding list index support to
setNestedValue.
123-145: LGTM!The nested path traversal in
getcorrectly handles bothMapandListtypes, with proper bounds checking for list indices and gracefulnullreturns for invalid paths.mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt (1)
44-61: LGTM!The navigation handler provides sensible defaults: delegates to the callback if provided, otherwise opens HTTP(S) URLs in the system browser. Error handling wraps exceptions in
ActionResult.Error.mpp-vscode/webview/src/components/nano/NanoActionHandler.ts (1)
26-28: Verify cross-platform ActionResult consistency.The TypeScript
ActionResulttype uses a discriminated union withsuccess: boolean, while the Kotlin version uses a sealed class with four variants:Success,SuccessWithValue,Error, andPending. The KotlinPendingvariant is missing from the TypeScript implementation. Since TypeScript usesPromise<ActionResult>for async operations (which is idiomatic), this difference might be intentional, but it creates an API inconsistency across platforms.Confirm that the ActionResult design difference is intentional and document the rationale if needed. Consider adding a comment explaining why the Pending state isn't needed in TypeScript (async/await vs callback pattern).
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.kt (2)
163-230: LGTM! Well-designed base implementation.The
BaseNanoActionHandlerprovides a solid foundation with:
- Clean separation of concerns with dedicated handlers per action type
- Proper error handling with try-catch blocks
- Registry pattern for extensible custom actions
- Sequence handling with fail-fast behavior
The implementation follows good Kotlin practices and aligns well with the cross-platform design goals.
181-189: Verify the NanoAction sealed class includes all action types in the when expression.The
whenexpression in thehandleActionmethod covers five action types (StateMutation, Navigate, Fetch, ShowToast, Sequence), but the review suggests aCustomaction type exists. In Kotlin,whenexpressions on sealed classes are exhaustive—ifNanoAction.Customexists as a sealed class variant, this code would not compile without handling it. EitherCustomis intentionally excluded from the sealed hierarchy and handled elsewhere, or it should be added to thiswhenexpression. Confirm the sealed class definition includes only the five variants shown, and that custom actions follow a different dispatch pattern.
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt
Show resolved
Hide resolved
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt
Show resolved
Hide resolved
| // Simple JSON serialization | ||
| return buildString { | ||
| append("{") | ||
| resolvedBody.entries.forEachIndexed { index, (key, value) -> | ||
| if (index > 0) append(",") | ||
| append("\"$key\":\"$value\"") | ||
| } | ||
| append("}") | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Manual JSON serialization doesn't escape values and always produces strings.
All values are wrapped in quotes ("$value"), so numeric and boolean values become strings in the JSON. Additionally, values containing " or \ will produce malformed JSON.
Consider using kotlinx.serialization.json:
import kotlinx.serialization.json.buildJsonObject
import kotlinx.serialization.json.put
private fun buildRequestBody(
body: Map<String, BodyField>?,
context: NanoActionContext
): String {
if (body == null) return ""
return buildJsonObject {
body.forEach { (key, field) ->
val value = when (field) {
is BodyField.Literal -> field.value
is BodyField.StateBinding -> context.get(field.path)
}
// put handles proper JSON encoding and types
when (value) {
is String -> put(key, value)
is Number -> put(key, value)
is Boolean -> put(key, value)
null -> put(key, null as String?)
else -> put(key, value.toString())
}
}
}.toString()
}🤖 Prompt for AI Agents
In mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt
around lines 181-189, the manual JSON serialization wraps every value in quotes
and does not escape characters, breaking numeric/boolean types and producing
invalid JSON for values with quotes or backslashes; replace the hand-rolled
buildString with kotlinx.serialization.json's buildJsonObject (or JsonObject
builder) to construct the JSON properly: resolve each BodyField to its value,
then use put for String, Number, Boolean, and null (and fallback to toString()
for other types) so values are encoded with correct JSON types and escaping, and
return the resulting JsonObject.toString(); also keep the early return of empty
string when body is null.
| const response = await fetch(action.url, { | ||
| method: action.method || 'GET', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| ...action.headers, | ||
| }, | ||
| body: action.body ? JSON.stringify(action.body) : undefined, | ||
| }); |
There was a problem hiding this comment.
Avoid sending body with GET requests.
The code serializes the request body for all HTTP methods, including GET. HTTP GET requests should not have a body, as it's semantically incorrect and may be rejected by some servers or proxies.
Apply this diff to conditionally include the body:
- const response = await fetch(action.url, {
- method: action.method || 'GET',
- headers: {
- 'Content-Type': 'application/json',
- ...action.headers,
- },
- body: action.body ? JSON.stringify(action.body) : undefined,
- });
+ const method = action.method || 'GET';
+ const response = await fetch(action.url, {
+ method,
+ headers: {
+ 'Content-Type': 'application/json',
+ ...action.headers,
+ },
+ ...(method !== 'GET' && action.body ? { body: JSON.stringify(action.body) } : {}),
+ });🤖 Prompt for AI Agents
In mpp-vscode/webview/src/components/nano/NanoActionHandler.ts around lines 143
to 150, the fetch call always serializes and sends action.body even for GET
requests; change the request construction so body is only included when
action.body is present and the HTTP method is not GET (compare method uppercased
to 'GET' to be safe). Keep headers as-is but only set 'Content-Type':
'application/json' when a body will be sent. Build the options object
conditionally (or omit body when method === 'GET') and pass that to fetch.
| * Example: | ||
| * ```kotlin | ||
| * val context = NanoRenderContext( | ||
| * state = NanoState(mapOf("count" to 0)), | ||
| * actionHandler = ComposeActionHandler(), | ||
| * theme = NanoTheme.default() | ||
| * ) | ||
| * | ||
| * val html = HtmlRenderer(context).render(ir) | ||
| * ``` | ||
| */ |
There was a problem hiding this comment.
Documentation example has incorrect theme reference.
The example uses NanoTheme.default() but the actual default parameter is NanoTheme.Default (an object, not a function call).
* val context = NanoRenderContext(
* state = NanoState(mapOf("count" to 0)),
* actionHandler = ComposeActionHandler(),
- * theme = NanoTheme.default()
+ * theme = NanoTheme.Default
* )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Example: | |
| * ```kotlin | |
| * val context = NanoRenderContext( | |
| * state = NanoState(mapOf("count" to 0)), | |
| * actionHandler = ComposeActionHandler(), | |
| * theme = NanoTheme.default() | |
| * ) | |
| * | |
| * val html = HtmlRenderer(context).render(ir) | |
| * ``` | |
| */ | |
| * Example: | |
| * |
🤖 Prompt for AI Agents
In xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.kt
around lines 17 to 27 the documentation example incorrectly calls
NanoTheme.default() even though the default theme is an object named
NanoTheme.Default; update the example to reference NanoTheme.Default (no
parentheses) so it matches the actual API and compiles correctly.
| MutationOp.APPEND -> { | ||
| when (currentValue) { | ||
| is List<*> -> currentValue + value | ||
| is MutableList<*> -> { | ||
| @Suppress("UNCHECKED_CAST") | ||
| (currentValue as MutableList<Any?>).also { it.add(value) } | ||
| } | ||
| null -> listOf(value) | ||
| else -> listOf(currentValue, value) | ||
| } | ||
| } |
There was a problem hiding this comment.
APPEND mutates MutableList in-place, which may not trigger StateFlow updates.
When currentValue is a MutableList, you mutate it in-place with it.add(value) and return the same instance. Since StateFlow uses reference equality, setting the same object reference won't notify collectors. The List branch correctly creates a new list.
MutationOp.APPEND -> {
when (currentValue) {
is List<*> -> currentValue + value
- is MutableList<*> -> {
- @Suppress("UNCHECKED_CAST")
- (currentValue as MutableList<Any?>).also { it.add(value) }
- }
null -> listOf(value)
else -> listOf(currentValue, value)
}
}Alternatively, always create a new list to ensure change detection works consistently.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MutationOp.APPEND -> { | |
| when (currentValue) { | |
| is List<*> -> currentValue + value | |
| is MutableList<*> -> { | |
| @Suppress("UNCHECKED_CAST") | |
| (currentValue as MutableList<Any?>).also { it.add(value) } | |
| } | |
| null -> listOf(value) | |
| else -> listOf(currentValue, value) | |
| } | |
| } | |
| MutationOp.APPEND -> { | |
| when (currentValue) { | |
| is List<*> -> currentValue + value | |
| null -> listOf(value) | |
| else -> listOf(currentValue, value) | |
| } | |
| } |
🤖 Prompt for AI Agents
In xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.kt around lines
196 to 206, the APPEND branch mutates MutableList in-place which won't trigger
StateFlow updates because the same reference is returned; instead, always return
a new list instance so collectors see a change. Replace the in-place add with
creating a new list (e.g., currentValue.toMutableList().also { it.add(value) }
or use currentValue + value) and return that new list; do not return the
original mutable instance.
| MutationOp.REMOVE -> { | ||
| when (currentValue) { | ||
| is List<*> -> currentValue - value | ||
| is MutableList<*> -> { | ||
| @Suppress("UNCHECKED_CAST") | ||
| (currentValue as MutableList<Any?>).also { it.remove(value) } | ||
| } | ||
| else -> currentValue | ||
| } | ||
| } |
There was a problem hiding this comment.
Same in-place mutation issue in REMOVE operation.
The MutableList branch has the same problem as APPEND — mutating in place won't trigger StateFlow change notifications.
MutationOp.REMOVE -> {
when (currentValue) {
is List<*> -> currentValue - value
- is MutableList<*> -> {
- @Suppress("UNCHECKED_CAST")
- (currentValue as MutableList<Any?>).also { it.remove(value) }
- }
else -> currentValue
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| MutationOp.REMOVE -> { | |
| when (currentValue) { | |
| is List<*> -> currentValue - value | |
| is MutableList<*> -> { | |
| @Suppress("UNCHECKED_CAST") | |
| (currentValue as MutableList<Any?>).also { it.remove(value) } | |
| } | |
| else -> currentValue | |
| } | |
| } | |
| MutationOp.REMOVE -> { | |
| when (currentValue) { | |
| is List<*> -> currentValue - value | |
| is MutableList<*> -> { | |
| @Suppress("UNCHECKED_CAST") | |
| (currentValue as MutableList<Any?>).toMutableList().also { it.remove(value) } | |
| } | |
| else -> currentValue | |
| } | |
| } |
🤖 Prompt for AI Agents
In xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.kt around lines
208-217, the REMOVE branch currently mutates MutableList in place which won't
emit StateFlow updates; instead create and return a new collection with the
element removed (matching the immutable List branch) — e.g., cast to
MutableList<Any?>, convert to an immutable List or a new MutableList copy,
remove the value from that copy and return it rather than calling remove on the
original instance so StateFlow change notifications occur.
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive cross-platform action dispatch system for NanoUI, enabling components to respond to user interactions consistently across Compose/Desktop, React/TypeScript, and HTML renderers. The implementation follows the established component-specific method pattern from NanoRenderer for type-safety and adds sophisticated state management with nested path support and mutation operations.
Key Changes:
- Core action handler interface with dedicated methods for each action type (StateMutation, Navigate, Fetch, ShowToast, Sequence, Custom)
- Extended NanoState to support mutation operations (SET, ADD, SUBTRACT, APPEND, REMOVE) with nested path access
- Platform-specific implementations (ComposeActionHandler, ReactActionHandler) with HTTP client support, navigation, and toast notifications
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/action/NanoActionHandler.kt |
Core action handler interface and base implementation with routing logic and custom action registry |
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoState.kt |
Extended state management with NanoActionContext implementation, nested path support, and mutation operations |
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/NanoRenderContext.kt |
New render context bundling state, action handler, and theme with static factory method |
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/render/HtmlRenderer.kt |
Added data-actions and data-bindings attributes to interactive components (Button, Input, etc.) |
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/ComposeActionHandler.kt |
Compose/Desktop implementation with Java HTTP client, Desktop.browse() navigation, and JOptionPane toasts |
mpp-vscode/webview/src/components/nano/NanoActionHandler.ts |
React/TypeScript implementation with native fetch API, window navigation, and ReactStateContext for useState integration |
mpp-vscode/webview/src/components/nano/index.ts |
Added exports for action handling types and utilities |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| override fun handleStateMutation( | ||
| mutation: NanoAction.StateMutation, | ||
| context: NanoActionContext | ||
| ): ActionResult { | ||
| return try { | ||
| context.mutate(mutation.path, mutation.operation, mutation.value) | ||
| ActionResult.Success | ||
| } catch (e: Exception) { | ||
| ActionResult.Error("Failed to mutate state: ${e.message}", e) | ||
| } | ||
| } |
There was a problem hiding this comment.
Type mismatch in StateMutation handling: NanoAction.StateMutation.value is typed as String in NanoAction.kt line 19, but context.mutate() expects Any? as its third parameter. This will cause the mutation.value (a String) to be passed where Any? is expected, which could lead to incorrect behavior when the value needs to be interpreted as a number, boolean, or other type.
Consider either:
- Changing
NanoAction.StateMutation.valuetoAny?to match the interface, or - Adding type conversion logic here to parse the string value based on the mutation operation
| is List<*> -> currentValue - value | ||
| is MutableList<*> -> { | ||
| @Suppress("UNCHECKED_CAST") | ||
| (currentValue as MutableList<Any?>).also { it.remove(value) } |
There was a problem hiding this comment.
The REMOVE operation mutates a MutableList in place (line 213: it.remove(value)) and returns the same instance. This has the same immutability issue as the APPEND operation - the mutated list will still have the same reference, causing state change detection issues in reactive systems.
Recommendation: Create a copy before removing:
is MutableList<*> -> {
@Suppress("UNCHECKED_CAST")
(currentValue as MutableList<Any?>).toMutableList().also { it.remove(value) }
}| (currentValue as MutableList<Any?>).also { it.remove(value) } | |
| (currentValue as MutableList<Any?>).toMutableList().also { it.remove(value) } |
| append("{") | ||
| resolvedBody.entries.forEachIndexed { index, (key, value) -> | ||
| if (index > 0) append(",") | ||
| append("\"$key\":\"$value\"") |
There was a problem hiding this comment.
Incorrect JSON serialization: The buildRequestBody method always wraps values in quotes (line 186: append("\"$key\":\"$value\"")), treating all values as strings. This will produce invalid JSON for numeric, boolean, null, or object values.
For example, {"count": 5} will be serialized as {"count": "5"}, which is incorrect.
Recommendation: Use a proper JSON serialization library (like kotlinx.serialization) or at least check the value type before wrapping in quotes:
append("\"$key\":")
when (value) {
is String -> append("\"$value\"")
is Number, is Boolean -> append(value)
null -> append("null")
else -> append("\"$value\"")
}| append("\"$key\":\"$value\"") | |
| append("\"$key\":") | |
| when (value) { | |
| is String -> append("\"${value.replace("\"", "\\\"")}\"") | |
| is Number, is Boolean -> append(value) | |
| null -> append("null") | |
| else -> append("\"${value.toString().replace("\"", "\\\"")}\"") | |
| } |
| val newRootValue = setNestedValue( | ||
| when (rootValue) { | ||
| is Map<*, *> -> @Suppress("UNCHECKED_CAST") (rootValue as Map<String, Any?>).toMutableMap() | ||
| else -> mutableMapOf() |
There was a problem hiding this comment.
When setting a nested path (e.g., user.name), if the root value is not a Map, it's replaced with an empty Map (line 163: else -> mutableMapOf()). This silently discards the existing value without warning.
For example:
state.set("user", "John") // user = "John"
state.set("user.age", 25) // user = {"age": 25}, "John" is lostRecommendation: Either:
- Throw an exception when trying to set a nested property on a non-Map value, or
- Document this behavior clearly in the KDoc
| else -> mutableMapOf() | |
| null -> mutableMapOf() | |
| else -> throw IllegalStateException("Cannot set nested property on non-Map value at '$rootKey' (type: ${rootValue!!::class.simpleName})") |
| if (this.options.onNavigate) { | ||
| this.options.onNavigate(action.to); | ||
| } else { | ||
| // Default: use window.location for external URLs | ||
| if (action.to.startsWith('http://') || action.to.startsWith('https://')) { | ||
| window.open(action.to, '_blank'); | ||
| } else { | ||
| window.location.href = action.to; |
There was a problem hiding this comment.
Open redirect vulnerability: The navigation handler opens URLs without validation (lines 126-129). An attacker could craft a malicious action like Navigate(to="javascript:alert('XSS')") or use other dangerous protocols.
Recommendation: Validate the URL protocol before navigation:
const allowedProtocols = ['http:', 'https:', 'mailto:'];
const url = new URL(action.to, window.location.origin);
if (!allowedProtocols.includes(url.protocol)) {
return { success: false, error: `Unsafe protocol: ${url.protocol}` };
}This prevents XSS via javascript: URLs and other protocol-based attacks.
| if (this.options.onNavigate) { | |
| this.options.onNavigate(action.to); | |
| } else { | |
| // Default: use window.location for external URLs | |
| if (action.to.startsWith('http://') || action.to.startsWith('https://')) { | |
| window.open(action.to, '_blank'); | |
| } else { | |
| window.location.href = action.to; | |
| // Validate protocol before navigation | |
| const allowedProtocols = ['http:', 'https:', 'mailto:']; | |
| let url: URL; | |
| try { | |
| url = new URL(action.to, window.location.origin); | |
| } catch (e) { | |
| return { success: false, error: `Invalid URL: ${action.to}` }; | |
| } | |
| if (!allowedProtocols.includes(url.protocol)) { | |
| return { success: false, error: `Unsafe protocol: ${url.protocol}` }; | |
| } | |
| if (this.options.onNavigate) { | |
| this.options.onNavigate(action.to); | |
| } else { | |
| // Default: use window.location for external URLs | |
| if (url.protocol === 'http:' || url.protocol === 'https:') { | |
| window.open(url.href, '_blank'); | |
| } else { | |
| window.location.href = url.href; |
| async handleFetch( | ||
| action: Extract<NanoAction, { type: 'Fetch' }>, | ||
| _context: NanoActionContext | ||
| ): Promise<ActionResult> { | ||
| try { | ||
| const response = await fetch(action.url, { | ||
| method: action.method || 'GET', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| ...action.headers, | ||
| }, | ||
| body: action.body ? JSON.stringify(action.body) : undefined, | ||
| }); | ||
|
|
||
| const data = await response.json().catch(() => response.text()); | ||
|
|
||
| if (response.ok) { | ||
| this.options.onFetchComplete?.(action.url, true, data); | ||
| return { success: true, value: data }; | ||
| } else { | ||
| const error = `HTTP ${response.status}: ${data}`; | ||
| this.options.onFetchComplete?.(action.url, false, error); | ||
| return { success: false, error }; | ||
| } | ||
| } catch (e) { | ||
| const error = `Fetch failed: ${e}`; | ||
| this.options.onFetchComplete?.(action.url, false, error); | ||
| return { success: false, error }; | ||
| } | ||
| } |
There was a problem hiding this comment.
The TypeScript Fetch implementation doesn't support the advanced features present in the Kotlin NanoAction.Fetch definition. The Kotlin version includes:
loadingState: State path to bind loading stateresponseBinding: State path to bind response dataerrorBinding: State path to bind error messageonSuccess: Action to execute on successful responseonError: Action to execute on error
The TypeScript implementation (lines 138-167) only calls onFetchComplete but doesn't update state bindings or execute callback actions. This creates an inconsistency between platforms - the same NanoIR will behave differently in Compose vs React environments.
Recommendation: Add support for these fields in the TypeScript implementation to maintain cross-platform consistency.
| export type NanoAction = | ||
| | { type: 'StateMutation'; path: string; operation: MutationOp; value: any } | ||
| | { type: 'Navigate'; to: string } | ||
| | { type: 'Fetch'; url: string; method?: string; body?: Record<string, any>; headers?: Record<string, string> } |
There was a problem hiding this comment.
The TypeScript Fetch action type definition is incomplete compared to the Kotlin version. It's missing several optional fields that exist in NanoAction.Fetch:
params?: Record<string, string>- Query parametersonSuccess?: NanoAction- Action to execute on successonError?: NanoAction- Action to execute on errorloadingState?: string- State path for loading indicatorresponseBinding?: string- State path for response dataerrorBinding?: string- State path for error messages
This inconsistency means NanoIR generated by the Kotlin compiler cannot be fully processed by the TypeScript handler, limiting cross-platform functionality.
| | { type: 'Fetch'; url: string; method?: string; body?: Record<string, any>; headers?: Record<string, string> } | |
| | { type: 'Fetch'; url: string; method?: string; body?: Record<string, any>; headers?: Record<string, string>; params?: Record<string, string>; onSuccess?: NanoAction; onError?: NanoAction; loadingState?: string; responseBinding?: string; errorBinding?: string } |
| } | ||
| } | ||
|
|
||
| return ActionResult.Pending { /* async operation */ } |
There was a problem hiding this comment.
The ActionResult.Pending is returned with an empty callback (line 148: ActionResult.Pending { /* async operation */ }), but the callback is never invoked when the async fetch operation completes. This means code waiting on the Pending result will never be notified of completion.
Either:
- Store the callback and invoke it when the async operation completes (success or error), or
- If async completion notification isn't needed, document that Pending results don't support completion callbacks in this implementation
The same issue exists in the base class where Pending results aren't being properly tracked.
| case 'StateMutation': | ||
| return { | ||
| type: 'StateMutation', | ||
| path: actionIR.payload?.path ?? '', | ||
| operation: (actionIR.payload?.operation ?? 'SET') as MutationOp, | ||
| value: actionIR.payload?.value, | ||
| }; |
There was a problem hiding this comment.
Missing validation for MutationOp: The operation value is cast to MutationOp without validation (line 330: as MutationOp). If the payload contains an invalid operation string (e.g., "INVALID"), it will pass through and only fail later during execution.
Recommendation: Validate the operation before casting:
const validOps: MutationOp[] = ['SET', 'ADD', 'SUBTRACT', 'APPEND', 'REMOVE'];
const operation = actionIR.payload?.operation ?? 'SET';
if (!validOps.includes(operation as MutationOp)) {
console.warn(`Invalid mutation operation: ${operation}, defaulting to SET`);
return { type: 'StateMutation', path: actionIR.payload?.path ?? '', operation: 'SET', value: actionIR.payload?.value };
}| case 'StateMutation': | |
| return { | |
| type: 'StateMutation', | |
| path: actionIR.payload?.path ?? '', | |
| operation: (actionIR.payload?.operation ?? 'SET') as MutationOp, | |
| value: actionIR.payload?.value, | |
| }; | |
| case 'StateMutation': { | |
| const validOps: MutationOp[] = ['SET', 'ADD', 'SUBTRACT', 'APPEND', 'REMOVE']; | |
| const op = actionIR.payload?.operation ?? 'SET'; | |
| const operation: MutationOp = validOps.includes(op) ? (op as MutationOp) : 'SET'; | |
| if (!validOps.includes(op)) { | |
| console.warn(`Invalid mutation operation: ${op}, defaulting to SET`); | |
| } | |
| return { | |
| type: 'StateMutation', | |
| path: actionIR.payload?.path ?? '', | |
| operation, | |
| value: actionIR.payload?.value, | |
| }; | |
| } |
| if (parts.size == 1) { | ||
| map[key] = value | ||
| } else { | ||
| val nested = map[key] | ||
| val nestedMap = when (nested) { | ||
| is MutableMap<*, *> -> @Suppress("UNCHECKED_CAST") (nested as MutableMap<String, Any?>) | ||
| is Map<*, *> -> @Suppress("UNCHECKED_CAST") (nested as Map<String, Any?>).toMutableMap() | ||
| else -> mutableMapOf() | ||
| } | ||
| map[key] = setNestedValue(nestedMap, parts.drop(1), value) | ||
| } | ||
|
|
||
| return map | ||
| } |
There was a problem hiding this comment.
The setNestedValue method doesn't support setting values in nested lists, even though the get method supports reading from them (line 135-138). For example, state.get("items.0.name") works to read from a list, but state.set("items.0.name", "value") will fail because setNestedValue only handles Map types (lines 238-242).
This creates an inconsistency where you can read from nested list items but not update them using the same path syntax.
Recommendation: Add support for list index paths in setNestedValue, or document that nested list updates are not supported.
| if (parts.size == 1) { | |
| map[key] = value | |
| } else { | |
| val nested = map[key] | |
| val nestedMap = when (nested) { | |
| is MutableMap<*, *> -> @Suppress("UNCHECKED_CAST") (nested as MutableMap<String, Any?>) | |
| is Map<*, *> -> @Suppress("UNCHECKED_CAST") (nested as Map<String, Any?>).toMutableMap() | |
| else -> mutableMapOf() | |
| } | |
| map[key] = setNestedValue(nestedMap, parts.drop(1), value) | |
| } | |
| return map | |
| } | |
| // Check if key is an integer (list index) | |
| val index = key.toIntOrNull() | |
| if (index != null) { | |
| // This path segment is a list index | |
| // Find the parent list in the map (should only happen at the root or after a map key) | |
| val listKey = parts.dropLast(parts.size - 1).firstOrNull() // The key in the parent map | |
| val parentList = map[listKey] as? MutableList<Any?> ?: ( | |
| (map[listKey] as? List<Any?>)?.toMutableList() ?: mutableListOf() | |
| ) | |
| // Ensure the list is large enough | |
| while (parentList.size <= index) { | |
| parentList.add(null) | |
| } | |
| if (parts.size == 1) { | |
| parentList[index] = value | |
| } else { | |
| val nested = parentList[index] | |
| val nextKey = parts[1] | |
| val nextIndex = nextKey.toIntOrNull() | |
| if (nextIndex != null) { | |
| // Next is also a list index | |
| val nestedList = when (nested) { | |
| is MutableList<*> -> @Suppress("UNCHECKED_CAST") (nested as MutableList<Any?>) | |
| is List<*> -> @Suppress("UNCHECKED_CAST") (nested as List<Any?>).toMutableList() | |
| else -> mutableListOf<Any?>() | |
| } | |
| parentList[index] = setNestedValueList(nestedList, parts.drop(1), value) | |
| } else { | |
| // Next is a map key | |
| val nestedMap = when (nested) { | |
| is MutableMap<*, *> -> @Suppress("UNCHECKED_CAST") (nested as MutableMap<String, Any?>) | |
| is Map<*, *> -> @Suppress("UNCHECKED_CAST") (nested as Map<String, Any?>).toMutableMap() | |
| else -> mutableMapOf<String, Any?>() | |
| } | |
| parentList[index] = setNestedValue(nestedMap, parts.drop(1), value) | |
| } | |
| } | |
| // Update the parent map with the possibly new/expanded list | |
| map[listKey ?: key] = parentList | |
| } else { | |
| if (parts.size == 1) { | |
| map[key] = value | |
| } else { | |
| val nested = map[key] | |
| val nextKey = parts[1] | |
| val nextIndex = nextKey.toIntOrNull() | |
| if (nextIndex != null) { | |
| // Next is a list index | |
| val nestedList = when (nested) { | |
| is MutableList<*> -> @Suppress("UNCHECKED_CAST") (nested as MutableList<Any?>) | |
| is List<*> -> @Suppress("UNCHECKED_CAST") (nested as List<Any?>).toMutableList() | |
| else -> mutableListOf<Any?>() | |
| } | |
| map[key] = setNestedValueList(nestedList, parts.drop(1), value) | |
| } else { | |
| // Next is a map key | |
| val nestedMap = when (nested) { | |
| is MutableMap<*, *> -> @Suppress("UNCHECKED_CAST") (nested as MutableMap<String, Any?>) | |
| is Map<*, *> -> @Suppress("UNCHECKED_CAST") (nested as Map<String, Any?>).toMutableMap() | |
| else -> mutableMapOf<String, Any?>() | |
| } | |
| map[key] = setNestedValue(nestedMap, parts.drop(1), value) | |
| } | |
| } | |
| } | |
| return map | |
| } | |
| // Helper for setting nested value in a list | |
| private fun setNestedValueList( | |
| list: MutableList<Any?>, | |
| parts: List<String>, | |
| value: Any? | |
| ): MutableList<Any?> { | |
| if (parts.isEmpty()) return list | |
| val key = parts.first() | |
| val index = key.toIntOrNull() | |
| if (index == null) return list // Invalid path, ignore | |
| while (list.size <= index) { | |
| list.add(null) | |
| } | |
| if (parts.size == 1) { | |
| list[index] = value | |
| } else { | |
| val nested = list[index] | |
| val nextKey = parts[1] | |
| val nextIndex = nextKey.toIntOrNull() | |
| if (nextIndex != null) { | |
| // Next is a list index | |
| val nestedList = when (nested) { | |
| is MutableList<*> -> @Suppress("UNCHECKED_CAST") (nested as MutableList<Any?>) | |
| is List<*> -> @Suppress("UNCHECKED_CAST") (nested as List<Any?>).toMutableList() | |
| else -> mutableListOf<Any?>() | |
| } | |
| list[index] = setNestedValueList(nestedList, parts.drop(1), value) | |
| } else { | |
| // Next is a map key | |
| val nestedMap = when (nested) { | |
| is MutableMap<*, *> -> @Suppress("UNCHECKED_CAST") (nested as MutableMap<String, Any?>) | |
| is Map<*, *> -> @Suppress("UNCHECKED_CAST") (nested as Map<String, Any?>).toMutableMap() | |
| else -> mutableMapOf<String, Any?>() | |
| } | |
| list[index] = setNestedValue(nestedMap, parts.drop(1), value) | |
| } | |
| } | |
| return list | |
| } |
- Create StatefulNanoRenderer that manages state and handles actions - Update IndentParser to support << (subscribe) binding in parseArgs - Update IndentParser to parse Text(content << state.count) syntax - Update NanoDSLDemo to use StatefulNanoRenderer for live preview - Update COUNTER_DSL example with proper state and on_click actions - Enhance prompt template with Counter example for LLM guidance The StatefulNanoRenderer: - Initializes state from NanoIR state definitions - Handles stateMutation actions (ADD, SUBTRACT, SET) - Passes state values to components via bindings - Supports Text content binding from state (content << state.count) - Supports Input/Checkbox/TextArea/Select two-way bindings Closes #489
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.kt (1)
837-840: Bug: Two-way binding detection only works for the first:=in argsStr.The
.find()method returns only the first match. When multiple two-way bindings exist (e.g.,a := state.x, b := state.y), only the first binding's key is captured, causing subsequent:=bindings to lose their prefix.Apply this diff to detect two-way bindings correctly for each key:
- // Preserve the binding operator for value parsing - val operatorMatch = Regex("""(\w+)\s*(:=)""").find(argsStr) - val hasTwoWayBinding = operatorMatch?.groupValues?.get(1) == key - - result[key] = if (hasTwoWayBinding) ":= $rawValue" else rawValue + // Preserve the binding operator for value parsing + // Check if this specific key uses := operator in the original string + val hasTwoWayBinding = Regex("""\b${Regex.escape(key)}\s*:=""").containsMatchIn(argsStr) + + result[key] = if (hasTwoWayBinding) ":= $rawValue" else rawValue
🧹 Nitpick comments (1)
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt (1)
42-56: State initialization may cause brief flash of uninitialized values.
LaunchedEffect(ir)runs asynchronously after the first composition, sostateMapis empty during the initial render frame. Consider initializing state synchronously:- // Initialize state from IR - val stateMap = remember { mutableStateMapOf<String, Any>() } - - // Initialize state values from IR state definitions - LaunchedEffect(ir) { - ir.state?.variables?.forEach { (name, varDef) -> - val defaultValue = varDef.defaultValue - stateMap[name] = when (varDef.type) { - "int" -> defaultValue?.jsonPrimitive?.intOrNull ?: 0 - "float" -> defaultValue?.jsonPrimitive?.content?.toFloatOrNull() ?: 0f - "bool" -> defaultValue?.jsonPrimitive?.booleanOrNull ?: false - "str" -> defaultValue?.jsonPrimitive?.content ?: "" - else -> defaultValue?.jsonPrimitive?.content ?: "" - } - } - } + // Initialize state from IR synchronously + val stateMap = remember(ir) { + mutableStateMapOf<String, Any>().apply { + ir.state?.variables?.forEach { (name, varDef) -> + val defaultValue = varDef.defaultValue + this[name] = when (varDef.type) { + "int" -> defaultValue?.jsonPrimitive?.intOrNull ?: 0 + "float" -> defaultValue?.jsonPrimitive?.content?.toFloatOrNull() ?: 0f + "bool" -> defaultValue?.jsonPrimitive?.booleanOrNull ?: false + "str" -> defaultValue?.jsonPrimitive?.content ?: "" + else -> defaultValue?.jsonPrimitive?.content ?: "" + } + } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
mpp-idea/build.gradle.kts(1 hunks)mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt(0 hunks)mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/NanoDSLDemo.kt(2 hunks)mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt(1 hunks)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.kt(2 hunks)xuiper-ui/src/main/resources/prompts/standard.txt(1 hunks)
💤 Files with no reviewable changes (1)
- mpp-idea/src/test/kotlin/cc/unitmesh/devins/idea/renderer/JewelRendererTest.kt
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code in Kotlin Multiplatform projects
For @JsExport in Kotlin, use concrete classes (not interfaces) and Promise (not Flow) for JavaScript interoperability
Avoid emoji and UTF-8 characters in WASM code
Files:
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.ktxuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.ktmpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/NanoDSLDemo.kt
**/*Renderer.kt
📄 CodeRabbit inference engine (AGENTS.md)
When modifying
CodingAgentRendererin Kotlin, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer,JsRendererAdapter
Files:
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt
🧠 Learnings (3)
📚 Learning: 2025-12-06T13:54:33.558Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T13:54:33.558Z
Learning: Applies to **/*.kt : Use `expect`/`actual` for platform-specific code in Kotlin Multiplatform projects
Applied to files:
mpp-idea/build.gradle.kts
📚 Learning: 2025-12-06T13:54:33.558Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T13:54:33.558Z
Learning: Applies to **/*Renderer.kt : When modifying `CodingAgentRenderer` in Kotlin, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, `JsRendererAdapter`
Applied to files:
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.ktmpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/NanoDSLDemo.kt
📚 Learning: 2025-12-06T13:54:33.558Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T13:54:33.558Z
Learning: Applies to **/*{CliRenderer,ConsoleRenderer}.kt : When modifying `CodingAgentRenderer` for JVM CLI, update implementations in `CodingCliRenderer` and `ConsoleRenderer`
Applied to files:
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt
🧬 Code graph analysis (1)
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt (3)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/editor/IdeaInputListener.kt (1)
Button(9-12)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/state/NanoStateManager.kt (1)
onAction(82-84)mpp-vscode/webview/src/types/nano.ts (1)
NanoActionIR(28-31)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Test mpp-core (KMP)
- GitHub Check: Build
🔇 Additional comments (12)
mpp-idea/build.gradle.kts (1)
348-348: Good addition for Kotlin test framework support.The kotlin("test") dependency aligns with the PR goal of supporting tests for the new action dispatch system and mirrors the existing pattern in
mpp-idea-core(line 443). Kotlin 2.2.0 will automatically resolve a compatible version, and this integrates well with the existing JUnit 4 + useJUnitPlatform() test infrastructure.Please confirm that test files like
JewelRendererTest.kt(mentioned in the PR context) are properly using kotlin-test assertions to justify this dependency addition.xuiper-ui/src/main/resources/prompts/standard.txt (2)
35-37: State mutation clarifications align well with PR objectives.The added examples for increment (
+=), decrement (-=), and explicit set operations are clear and directly support the cross-platform action dispatch system. These clarifications help users understand how to use state mutations in actions.
42-57: Verify Text component parameter syntax for bindings (line 54).The Counter example is comprehensive and effectively demonstrates state management, bindings, and mutations. However, line 54 uses
Text(content << state.count, style="h1")with a namedcontentparameter, which differs from the documented syntax on line 12 that showsText("content", style="h1|h2|h3|body|caption")as a positional string argument.Please clarify:
- Is
contentthe correct parameter name for binding to the Text component's display value?- Should the syntax be
Text(value << state.count, style="h1")or another variant?- Ensure the example follows the same parameter conventions as documented in the Components section.
Otherwise, the example is well-structured and effectively showcases state-driven interactivity with proper indentation and component usage.
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.kt (2)
735-752: LGTM! Binding-aware Text node construction is well implemented.The logic correctly distinguishes between binding expressions (leaving content empty for runtime resolution) and static content, and appropriately filters out
Binding.Staticinstances to only preserve dynamic bindings.
817-832: Subscribe binding handling looks good.The precedence logic correctly processes
<<(subscribe) bindings first and skips them during the subsequent=/:=/:pass to avoid overwriting.mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/NanoDSLDemo.kt (2)
259-259: LGTM: Renderer switch to stateful implementation.The change from
ComposeNanoRenderertoStatefulNanoRendererenables the new action dispatch and state management features as intended by the PR.
306-322: LGTM: COUNTER_DSL updated with state-driven interactions.The DSL template now demonstrates the stateful counter pattern with proper state initialization and increment/decrement actions.
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt (5)
213-240: LGTM: Text rendering with binding support.The binding expression handling with
removePrefix("state.")aligns with the DSL syntax (state.count). The fallback to props when no binding exists is appropriate.
132-209: LGTM: Layout component renderers.The layout renderers (
VStack,HStack,Card,Form,Component) correctly map IR properties to Compose layout primitives with appropriate defaults.
312-404: LGTM: Input component implementations.The input components correctly synchronize local state with the shared state map via bidirectional bindings and dispatch appropriate
stateMutationactions.
457-475: LGTM: Utility extensions.The
toSpacing()andtoPadding()extensions provide clean token-to-dp mapping with sensible defaults.
443-455: LGTM: Unknown component fallback.Good defensive rendering with clear visual indication (red border and background) for unrecognized node types.
| val handleAction: (NanoActionIR) -> Unit = handleAction@{ action -> | ||
| when (action.type) { | ||
| "stateMutation" -> { | ||
| val payload = action.payload ?: return@handleAction | ||
| val path = payload["path"]?.jsonPrimitive?.content ?: return@handleAction | ||
| val operation = payload["operation"]?.jsonPrimitive?.content ?: "SET" | ||
| val valueStr = payload["value"]?.jsonPrimitive?.content ?: "" | ||
|
|
||
| val currentValue = stateMap[path] | ||
| val newValue = when (operation) { | ||
| "ADD" -> { | ||
| when (currentValue) { | ||
| is Int -> currentValue + (valueStr.toIntOrNull() ?: 1) | ||
| is Float -> currentValue + (valueStr.toFloatOrNull() ?: 1f) | ||
| else -> currentValue | ||
| } | ||
| } | ||
| "SUBTRACT" -> { | ||
| when (currentValue) { | ||
| is Int -> currentValue - (valueStr.toIntOrNull() ?: 1) | ||
| is Float -> currentValue - (valueStr.toFloatOrNull() ?: 1f) | ||
| else -> currentValue | ||
| } | ||
| } | ||
| "SET" -> { | ||
| when (currentValue) { | ||
| is Int -> valueStr.toIntOrNull() ?: 0 | ||
| is Float -> valueStr.toFloatOrNull() ?: 0f | ||
| is Boolean -> valueStr.toBooleanStrictOrNull() ?: false | ||
| else -> valueStr | ||
| } | ||
| } | ||
| else -> valueStr | ||
| } | ||
|
|
||
| if (newValue != null) { | ||
| stateMap[path] = newValue | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Action handler only processes stateMutation; other action types are silently ignored.
Per PR objectives, the action system should support Navigate, Fetch, ShowToast, Sequence, and custom actions. Currently, these actions are silently dropped. Consider adding handlers or at minimum logging unhandled actions for debugging:
if (newValue != null) {
stateMap[path] = newValue
}
}
+ "navigate", "fetch", "showToast", "sequence" -> {
+ // TODO: Integrate with ComposeActionHandler for platform actions
+ println("Action type '${action.type}' not yet implemented in StatefulNanoRenderer")
+ }
+ else -> {
+ println("Unknown action type: ${action.type}")
+ }
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| val handleAction: (NanoActionIR) -> Unit = handleAction@{ action -> | |
| when (action.type) { | |
| "stateMutation" -> { | |
| val payload = action.payload ?: return@handleAction | |
| val path = payload["path"]?.jsonPrimitive?.content ?: return@handleAction | |
| val operation = payload["operation"]?.jsonPrimitive?.content ?: "SET" | |
| val valueStr = payload["value"]?.jsonPrimitive?.content ?: "" | |
| val currentValue = stateMap[path] | |
| val newValue = when (operation) { | |
| "ADD" -> { | |
| when (currentValue) { | |
| is Int -> currentValue + (valueStr.toIntOrNull() ?: 1) | |
| is Float -> currentValue + (valueStr.toFloatOrNull() ?: 1f) | |
| else -> currentValue | |
| } | |
| } | |
| "SUBTRACT" -> { | |
| when (currentValue) { | |
| is Int -> currentValue - (valueStr.toIntOrNull() ?: 1) | |
| is Float -> currentValue - (valueStr.toFloatOrNull() ?: 1f) | |
| else -> currentValue | |
| } | |
| } | |
| "SET" -> { | |
| when (currentValue) { | |
| is Int -> valueStr.toIntOrNull() ?: 0 | |
| is Float -> valueStr.toFloatOrNull() ?: 0f | |
| is Boolean -> valueStr.toBooleanStrictOrNull() ?: false | |
| else -> valueStr | |
| } | |
| } | |
| else -> valueStr | |
| } | |
| if (newValue != null) { | |
| stateMap[path] = newValue | |
| } | |
| } | |
| } | |
| } | |
| val handleAction: (NanoActionIR) -> Unit = handleAction@{ action -> | |
| when (action.type) { | |
| "stateMutation" -> { | |
| val payload = action.payload ?: return@handleAction | |
| val path = payload["path"]?.jsonPrimitive?.content ?: return@handleAction | |
| val operation = payload["operation"]?.jsonPrimitive?.content ?: "SET" | |
| val valueStr = payload["value"]?.jsonPrimitive?.content ?: "" | |
| val currentValue = stateMap[path] | |
| val newValue = when (operation) { | |
| "ADD" -> { | |
| when (currentValue) { | |
| is Int -> currentValue + (valueStr.toIntOrNull() ?: 1) | |
| is Float -> currentValue + (valueStr.toFloatOrNull() ?: 1f) | |
| else -> currentValue | |
| } | |
| } | |
| "SUBTRACT" -> { | |
| when (currentValue) { | |
| is Int -> currentValue - (valueStr.toIntOrNull() ?: 1) | |
| is Float -> currentValue - (valueStr.toFloatOrNull() ?: 1f) | |
| else -> currentValue | |
| } | |
| } | |
| "SET" -> { | |
| when (currentValue) { | |
| is Int -> valueStr.toIntOrNull() ?: 0 | |
| is Float -> valueStr.toFloatOrNull() ?: 0f | |
| is Boolean -> valueStr.toBooleanStrictOrNull() ?: false | |
| else -> valueStr | |
| } | |
| } | |
| else -> valueStr | |
| } | |
| if (newValue != null) { | |
| stateMap[path] = newValue | |
| } | |
| } | |
| "navigate", "fetch", "showToast", "sequence" -> { | |
| // TODO: Integrate with ComposeActionHandler for platform actions | |
| println("Action type '${action.type}' not yet implemented in StatefulNanoRenderer") | |
| } | |
| else -> { | |
| println("Unknown action type: ${action.type}") | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt
around lines 59 to 99, the action handler only processes "stateMutation" and
silently drops other action types; update the handler to cover the required
action types (Navigate, Fetch, ShowToast, Sequence, and any custom actions) by
adding branches for each action type that either invoke the appropriate handler
functions (navigateTo(...), fetchResource(...), showToast(...),
executeSequence(...), or dispatch custom handlers) and map action.payload into
the handler params, and add a default branch that logs unhandled action types
(including action.type and payload) to aid debugging so no action is silently
ignored.
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt
Show resolved
Hide resolved
mpp-ui/src/jvmMain/kotlin/cc/unitmesh/devins/ui/nano/StatefulNanoRenderer.kt
Show resolved
Hide resolved
The parser was keeping the raw quotes around string default values, causing empty strings like to be rendered literally as "" in the UI. Changes: - Update parseStateBlock() to strip surrounding quotes from string values - Add tests for state variable parsing with empty and non-empty strings - Verify IR conversion preserves the unquoted string values This fixes the LoginForm rendering issue where email and password inputs showed "" instead of being empty.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.kt (1)
833-848: Bug: Two-way binding detection fails for multiple:=bindings.The
operatorMatchregex only finds the first:=occurrence inargsStr. For input likea := x, b := y, when processing keyb, it still matchesa :=, sohasTwoWayBindingbecomes false andbloses its binding prefix.Fix by capturing the operator in the main regex:
- // Then handle := (two-way binding) and = (assignment) patterns // Match patterns like: name="value", name=value, name := state.path, name: "value" - val argRegex = Regex("""(\w+)\s*(?::=|=|:)\s*(?:"([^"]*)"|([\w.]+))""") + val argRegex = Regex("""(\w+)\s*(:=|=|:)\s*(?:"([^"]*)"|([\w.]+))""") argRegex.findAll(argsStr).forEach { match -> val key = match.groupValues[1] // Skip if already handled by subscribe binding if (key in result) return@forEach - val rawValue = match.groupValues[2].ifEmpty { match.groupValues[3] } + val operator = match.groupValues[2] + val rawValue = match.groupValues[3].ifEmpty { match.groupValues[4] } - // Preserve the binding operator for value parsing - val operatorMatch = Regex("""(\w+)\s*(:=)""").find(argsStr) - val hasTwoWayBinding = operatorMatch?.groupValues?.get(1) == key - - result[key] = if (hasTwoWayBinding) ":= $rawValue" else rawValue + result[key] = if (operator == ":=") ":= $rawValue" else rawValue }
🧹 Nitpick comments (4)
xuiper-ui/src/test/kotlin/cc/unitmesh/xuiper/dsl/NanoDSLParserTest.kt (1)
395-439: Solid coverage for state default parsing (strings vs non-strings)This test nicely locks in the intended behavior: stripping quotes for string defaults (including empty strings) while preserving non‑string defaults as written. If you want to tighten it further, you could also assert the
typeforloadingVarandcountVar("bool"/"int") for symmetry withemailVar, but that’s optional.xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.kt (1)
197-208: Consider usingremoveSurrounding()for safer quote stripping.The current implementation could throw
StringIndexOutOfBoundsExceptionifrawValueis a single"character (though unlikely given the regex). UsingremoveSurroundingis both safer and more concise.- val rawValue = match.groupValues[3].trim() - // Strip quotes from string values (e.g., "" -> empty string, "hello" -> hello) - val defaultValue = if (rawValue.startsWith("\"") && rawValue.endsWith("\"")) { - rawValue.substring(1, rawValue.length - 1) - } else { - rawValue - } + val rawValue = match.groupValues[3].trim() + // Strip quotes from string values (e.g., "" -> empty string, "hello" -> hello) + val defaultValue = rawValue.removeSurrounding("\"")mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
87-90: Consider whether tasks should be reset with other renderer stateExposing
tasksas aStateFlow<List<TaskInfo>>is consistent with the rest of this renderer’s API. One open question: do you want task state to be cleared alongside timeline/processing state (e.g., inclearTimeline()/reset()), or intentionally persist across sessions? If the former, adding_tasks.value = emptyList()there would keep lifecycle semantics consistent.
341-370: Task update logic is correct; consider sharing with ComposeRenderer to avoid driftThe
updateTaskFromToolCallhelper correctly:
- Parses
taskName/status/summaryand maps toTaskStatus.- Uses
_tasks.update { ... }with a new list instance (viatoMutableList().apply { ... }orcurrentTasks + TaskInfo(...)), avoiding in-place mutation of the previousList.- Updates timestamps only on existing tasks, as expected.
Given there’s an almost identical helper in
ComposeRenderer, you might want to extract a shared utility (e.g., an extension onMutableStateFlow<List<TaskInfo>>or a small pure function taking the current list and params) to keep behavior and status handling identical across UIs and reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
mpp-idea/build.gradle.kts(1 hunks)mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt(5 hunks)xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.kt(3 hunks)xuiper-ui/src/test/kotlin/cc/unitmesh/xuiper/dsl/NanoDSLParserTest.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- mpp-idea/build.gradle.kts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Useexpect/actualfor platform-specific code in Kotlin Multiplatform projects
For @JsExport in Kotlin, use concrete classes (not interfaces) and Promise (not Flow) for JavaScript interoperability
Avoid emoji and UTF-8 characters in WASM code
Files:
xuiper-ui/src/test/kotlin/cc/unitmesh/xuiper/dsl/NanoDSLParserTest.ktxuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.ktmpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
**/*Renderer.kt
📄 CodeRabbit inference engine (AGENTS.md)
When modifying
CodingAgentRendererin Kotlin, update ALL implementations:DefaultCodingAgentRenderer,ComposeRenderer,JewelRenderer,ServerSideRenderer,JsRendererAdapter
Files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
🧠 Learnings (1)
📚 Learning: 2025-12-06T13:54:33.558Z
Learnt from: CR
Repo: phodal/auto-dev PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T13:54:33.558Z
Learning: Applies to **/*Renderer.kt : When modifying `CodingAgentRenderer` in Kotlin, update ALL implementations: `DefaultCodingAgentRenderer`, `ComposeRenderer`, `JewelRenderer`, `ServerSideRenderer`, `JsRendererAdapter`
Applied to files:
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
🧬 Code graph analysis (2)
xuiper-ui/src/test/kotlin/cc/unitmesh/xuiper/dsl/NanoDSLParserTest.kt (1)
mpp-core/src/commonMain/kotlin/cc/unitmesh/devins/test/DevInsTestCase.kt (2)
assertNotNull(54-59)assertEquals(45-49)
mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt (1)
updateTaskFromToolCall(248-281)mpp-vscode/webview/src/types/timeline.ts (1)
TaskInfo(34-39)
🔇 Additional comments (5)
xuiper-ui/src/test/kotlin/cc/unitmesh/xuiper/dsl/NanoDSLParserTest.kt (2)
3-10: Import ofjsonPrimitiveis appropriate and scopedThe added
jsonPrimitiveimport is used in the new IR test and keeps JSON‑specific logic localized to tests; no issues here.
441-463: IR-level assertion for empty-string state default is preciseVerifying
emailState.defaultValue?.jsonPrimitive?.contentis""ensures the IR encodes a real empty string rather than a quoted literal, which aligns with the parser-level expectations. This is a good end‑to‑end guard on the state serialization path.xuiper-ui/src/main/kotlin/cc/unitmesh/xuiper/parser/IndentParser.kt (1)
743-759: LGTM!The binding detection logic correctly handles subscribe (
<<) and two-way (:=) bindings, appropriately setting content to empty when it will come from a binding. The fallback toextractFirstArgmaintains backward compatibility for positional arguments.mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt (2)
9-10: Task model imports look correct
TaskInfo/TaskStatusimports align with the new task-tracking usage later in the file and match the render-side types already used elsewhere; no issues here.
192-196: Symmetric handling for parsed-params path looks goodThe
renderToolCallWithParamspath mirrors the string-basedrenderToolCallhandling by normalizing toMap<String, String>and invoking the same task-update helper; this keeps behavior consistent for both call sites.
| // Handle task-boundary tool - update task list | ||
| if (toolName == "task-boundary") { | ||
| updateTaskFromToolCall(params) | ||
| } | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all task-boundary references in Kotlin renderer files
echo "=== Task-boundary references ==="
rg -n "task-boundary" --type=kt
echo -e "\n=== Looking for all Renderer implementations ==="
# Find all *Renderer.kt files to identify the implementations
fd "Renderer\.kt$" --type f
echo -e "\n=== Searching for CodingAgentRenderer interface/class ==="
rg -n "class.*CodingAgentRenderer|interface.*CodingAgentRenderer" --type=kt -A 5Repository: phodal/auto-dev
Length of output: 4405
🏁 Script executed:
#!/bin/bash
# Search for task-boundary without file type filter
echo "=== Task-boundary references ==="
rg "task-boundary" -C 3
echo -e "\n=== CodingAgentRenderer interface ==="
cat mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/CodingAgentRenderer.kt
echo -e "\n=== Check DefaultCodingAgentRenderer for task-boundary ==="
rg "task-boundary" mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/DefaultCodingAgentRenderer.kt -C 3 || echo "Not found in DefaultCodingAgentRenderer"
echo -e "\n=== Check ComposeRenderer for task-boundary ==="
rg "task-boundary" mpp-ui/src/commonMain/kotlin/cc/unitmesh/devins/ui/compose/agent/ComposeRenderer.kt -C 3 || echo "Not found in ComposeRenderer"
echo -e "\n=== Check ServerSideRenderer for task-boundary ==="
rg "task-boundary" mpp-server/src/main/kotlin/cc/unitmesh/server/render/ServerSideRenderer.kt -C 3 || echo "Not found in ServerSideRenderer"
echo -e "\n=== Check JewelRenderer for task-boundary ==="
rg "task-boundary" mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt -C 3Repository: phodal/auto-dev
Length of output: 13916
🏁 Script executed:
#!/bin/bash
echo "=== DefaultCodingAgentRenderer full implementation ==="
cat mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/DefaultCodingAgentRenderer.kt
echo -e "\n=== ServerSideRenderer full implementation ==="
wc -l mpp-server/src/main/kotlin/cc/unitmesh/server/render/ServerSideRenderer.kt
head -100 mpp-server/src/main/kotlin/cc/unitmesh/server/render/ServerSideRenderer.kt
echo -e "\n=== Search for JsRendererAdapter ==="
fd "JsRendererAdapter" --type f
rg "JsRendererAdapter" -lRepository: phodal/auto-dev
Length of output: 7681
🏁 Script executed:
#!/bin/bash
echo "=== BaseRenderer implementation ==="
cat mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/BaseRenderer.kt
echo -e "\n=== RendererExports.kt (JS adapter) ==="
cat mpp-core/src/jsMain/kotlin/cc/unitmesh/agent/RendererExports.kt
echo -e "\n=== Check if task-boundary is in BaseRenderer ==="
rg "task-boundary" mpp-core/src/commonMain/kotlin/cc/unitmesh/agent/render/BaseRenderer.kt || echo "Not found"Repository: phodal/auto-dev
Length of output: 9537
Task-boundary handling is inconsistent across CodingAgentRenderer implementations
Only ComposeRenderer and JewelRenderer handle the task-boundary tool in their renderToolCall methods. The other three implementations—DefaultCodingAgentRenderer, ServerSideRenderer, and JsRendererAdapter—lack this handler. Since CodingAgentRenderer does not define task-boundary as a required interface method, either all implementations should add consistent task-boundary handling, or it should be removed from the two that currently have it to avoid platform discrepancies.
🤖 Prompt for AI Agents
In mpp-idea/src/main/kotlin/cc/unitmesh/devins/idea/renderer/JewelRenderer.kt
around lines 161-165 the file contains a task-boundary handler
(updateTaskFromToolCall(params)) but other renderer implementations lack it;
make handling consistent by adding a common hook in the CodingAgentRenderer
interface and wiring it into all renderers: add a default fun
handleTaskBoundary(params: Map<String, Any?>) = Unit to CodingAgentRenderer (or
an abstract method if you want it required), ensure updateTaskFromToolCall is
refactored into a shared/internal helper accessible from all renderers (or call
that helper from the new interface method), then call handleTaskBoundary(params)
from each renderer’s renderToolCall (update DefaultCodingAgentRenderer,
ServerSideRenderer, JsRendererAdapter, and keep ComposeRenderer/JewelRenderer
implementations delegating to the new method); alternatively, if you prefer not
to support task-boundary, remove the handling from ComposeRenderer and
JewelRenderer instead—but pick one approach and apply it across all renderer
implementations for consistency.
- Enhance Navigate action with params, query, and replace parameters - Add parseNavigateAction() and parseNavigateMap() to IndentParser - Fix component call vs property parsing priority in IndentParser - Update prompt templates with routing documentation - Add 4 navigation test cases (16-19) and integration tests - Update eval suite with NAVIGATION category
Summary
Implements GitHub issue #489 - Cross-platform action dispatch system for NanoRenderer.
This PR adds a complete action handling system that enables NanoUI components to respond to user interactions (clicks, input changes, etc.) across all platforms.
Changes
Core Action System (
xuiper-ui)handleStateMutation- State changeshandleNavigate- Page navigationhandleFetch- HTTP requestshandleShowToast- Toast notificationshandleSequence- Sequential actionshandleCustomAction- User-defined actions (AddTask, DeleteTask, etc.)State Management (
xuiper-ui)mutate()method supporting MutationOp (SET, ADD, SUBTRACT, APPEND, REMOVE)user.name)Render Context (
xuiper-ui)static()factory for no-op renderingCompose Implementation (
mpp-ui)Desktop.browse()or custom callbackReact/TypeScript Implementation (
mpp-vscode)window.open/locationparseNanoActionhelper for IR parsingHtmlRenderer Updates
data-actionsattribute for action bindingsdata-bindingsattribute for two-way bindingsExample Usage
Build Verification
./gradlew :xuiper-ui:compileKotlin- Success./gradlew :mpp-ui:compileKotlinJvm- Successcd mpp-vscode/webview && npm run build- SuccessCloses #489
Summary by CodeRabbit
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.