Kotlin sdk catalog uri scheme#1398
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a streaming parser for A2UI (v0.8 and v0.9), enhances catalog pruning for messages and common types, and improves schema validation with detailed error reporting. New utilities like TopologyAnalyzer and SchemaInspector are introduced to manage component relationships and reachability. Reviewer feedback highlights critical performance bottlenecks in the parser's JSON processing and metadata sniffing, a potential argument swap in the AgentExtension constructor, and robustness issues in URI path resolution.
| val objBuffer = jsonBuffer.substring(startIdx) | ||
| if (objBuffer.startsWith("{") && objBuffer.endsWith("}")) { | ||
| try { | ||
| val obj = Json.parseToJsonElement(objBuffer) as? JsonObject |
There was a problem hiding this comment.
Parsing the entire objBuffer using Json.parseToJsonElement every time a closing brace } is encountered results in
| } | ||
|
|
||
| if (braceCount > 0 && char in listOf('"', ':', ',', '}', ']')) { | ||
| sniffMetadata() |
There was a problem hiding this comment.
Calling sniffMetadata() for every punctuation character in the JSON stream is extremely expensive due to the underlying regex searches in getLatestValue. This logic should be optimized to either use a more efficient state machine or to only sniff at specific intervals or boundaries.
| val searchStr = "\"$key\"" | ||
| while (true) { | ||
| idx = jsonBuffer.lastIndexOf(searchStr, idx - 1) | ||
| if (idx == -1) return null | ||
|
|
||
| val fragment = jsonBuffer.substring(idx) | ||
| val match = Regex("\"$key\"\\s*:\\s*\"([^\"]+)\"").find(fragment) | ||
| if (match != null && match.range.first == 0) { | ||
| return match.groupValues[1] | ||
| } | ||
| } |
There was a problem hiding this comment.
Creating a new Regex object inside getLatestValue is highly inefficient, especially since this function is called frequently during punctuation sniffing. This will lead to significant performance degradation as the JSON buffer grows.
| return AgentExtension( | ||
| A2UI_EXTENSION_URI, | ||
| "Provides agent driven UI using the A2UI JSON format.", | ||
| params, | ||
| isSupportRequired, | ||
| "Provides agent driven UI using the A2UI JSON format.", | ||
| A2UI_EXTENSION_URI, | ||
| ) |
There was a problem hiding this comment.
The arguments passed to the AgentExtension constructor appear to be swapped compared to the previous version. The description string is now the first argument, and A2UI_EXTENSION_URI is the last. Please verify if this change in parameter order is intentional and matches the AgentExtension class definition.
| if (inStr) { | ||
| val prefix = fixed.substring(0, lastQuoteIdx).trimEnd() | ||
| if (prefix.endsWith(":")) { | ||
| val keyMatch = Regex("\"([^\"]+)\"\\s*:\\s*$").find(prefix) |
There was a problem hiding this comment.
The regex Regex("\"([^\"]+)\"\\s*:\\s*$") is instantiated on every call to fixJson. It should be moved to a companion object as a pre-compiled constant to avoid redundant object allocation and compilation overhead.
References
- Best practices for performance in Kotlin suggest pre-compiling Regex instances used in loops or frequent function calls. (link)
| java.net.URI(catalogPath) | ||
| } catch (e: Exception) { | ||
| null | ||
| } |
There was a problem hiding this comment.
The java.net.URI constructor is strict and may throw URISyntaxException for valid local file paths that contain characters like spaces or colons (common on Windows). While the try-catch handles the exception, it's safer to check if the string looks like a URI before attempting to parse it, or use File(catalogPath).toURI() if a local path is intended.
jgindin
force-pushed
the
kotlin-sdk-catalog-uri-scheme
branch
3 times, most recently
from
2579b16 to
ed9072f
Compare
nan-yu
left a comment
nan-yu
left a comment
There was a problem hiding this comment.
Looks good to me on the URI scheme support in CatalogConfig commit.
jgindin
force-pushed
the
kotlin-sdk-catalog-uri-scheme
branch
5 times, most recently
from
1c0c628 to
a6116c6
Compare
Implements the full incremental A2UI v0.9 streaming parser suite in Kotlin, achieving SDK parity. Automatically incorporates critical subsequent fixes for robust real-time topology parsing and relative bindings. Port of Python SDK commit 8ba982a
Port of Python SDK commit 1ea689d
Implement coordinated component and message pruning via withPruning, propagating allowedMessages from A2uiSchemaManager down to Catalog. Add robust automated unit tests and enable full conformance verification. Port of Python SDK commit 0fd7240
Implements custom JSON-path validation error string construction for v0.9 payloads in Kotlin, matching Python exactly. Validates messages individually via sub-validators and iterates component arrays to construct precise path prefixes. Port of Python SDK commit 15ee789 � Conflicts: � agent_sdks/kotlin/src/main/kotlin/com/google/a2ui/core/parser/StreamingParser.kt � agent_sdks/kotlin/src/main/kotlin/com/google/a2ui/core/schema/Validator.kt
Implement equivalent transparent file:// URI parsing support in CatalogConfig.fromPath and BasicCatalog.getConfig. Unsupported schemes are correctly rejected, and robust unit tests are included. Port of Python SDK commit 90a0a19
jgindin
force-pushed
the
kotlin-sdk-catalog-uri-scheme
branch
from
a6116c6 to
1c78b7e
Compare
2 participants
Description
Implement equivalent transparent file:// URI parsing support in CatalogConfig.fromPath and BasicCatalog.getConfig. Unsupported schemes are correctly rejected, and robust unit tests are included.
Port of Python SDK commit 90a0a19
Pre-launch Checklist
If you need help, consider asking for advice on the discussion board.