-
Notifications
You must be signed in to change notification settings - Fork 769
Proposal with simple interpolation #504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request refactors the string formatting and interpolation mechanism, replacing the flexible but complex string_format function with a more structured approach centered around a new formatString function and several other specific formatting helpers. The changes are significant and affect the protocol specification, evolution guide, and the standard component catalog. My review focuses on ensuring consistency and correctness across these changes. I've found a few critical issues in the new function definitions within standard_catalog.json, including incorrect examples and parameter schemas that would break data binding. I also noted some documentation inconsistencies, such as a function name mismatch in the evolution guide and an unclear description in the main protocol document. Addressing these points will improve the clarity and correctness of this new proposal.
| }, | ||
| { | ||
| "name": "formatNumber", | ||
| "description": "Formats a number with the specified grouping and decimal precision. Example: `formatNumber(1234.56, 1, true)` -> '1,234.6'.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example formatNumber(1234.56, 1, true) uses a positional argument syntax. This is inconsistent with the function's parameter definition which specifies named arguments (value, decimals, useGrouping) and the overall direction of this PR to move away from string-based expression parsing. The example should demonstrate the correct JSON object-based argument passing, for instance: with args {"value": 1234.56, ...} returns '1,234.6'. This issue is also present in the examples for formatCurrency (line 763), formatDate (line 782), and pluralize (line 801).
| "value": { | ||
| "type": "number", | ||
| "description": "The number to format." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value parameter for formatNumber is defined with type: "number". This is too restrictive, as it will fail validation when a data binding path is used (e.g., {"value": {"path": "/transaction/count"}}). The type constraint should be removed to allow for DynamicValues, consistent with how other functions like required are defined. This same issue applies to the value parameters in formatCurrency (line 770), formatDate (line 789), and pluralize (line 807). Furthermore, type: "date" on line 789 is not a valid JSON Schema type.
"value": {
"description": "The number to format."
}Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| } | ||
| }, | ||
| { | ||
| "name": "formatNumber", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer this lower-magic version of string interpolation function calling overall. But I still wonder if there is some middle ground where you need to explicitly pass arguments to stringFormat, but these specific formatting options for certain types are still built into the formatString function rather than separated out, which makes the "call sites" super complex.
E.g. trying to take inspiration from Python:
{
"id": "receipt",
"component": "Text",
"text": {
"call": "formatString",
"args": {
"template": "Bought ${quantity} ${item} on ${date:%m %d, %Y} for ${price.2f}.",
"args": {
"quantity": {path: "/transaction/count"},
"item": {path: "/transaction/name"},
"date": {path: "/transaction/timestamp"},
"price": {path: "/transaction/amount"},
}
}
}
}
At least this way, the core spec and data flow is still pretty simple and synchronous, but the standard catalog just happens to have a slightly magical function that can understand some fancy formatting placeholders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my original proposal, but I thought we wanted to do the dart / calling functions approach for string formatting. I'd actually prefer just a single string_format function if that what you prefer. @gspencergoog thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked this over with Jacob, and I still think that I'd rather use the more "modern" version of string interpolation that just takes a single string:
- It it just simpler to construct if the LLM needs to construct one string instead of a string and also a separate set of nested args.
- They both have equivalent functionality, and don't provide any extra validation, since any of these options need to have validation that involves validating the template or string against the args anyhow.
- Having a fixed set of built in formatting options is not as flexible in the long run, although I guess there's nothing saying we couldn't have some built in ones along with the ability to interpolate function outputs.
- Also, with a single string there's no chance of having a mismatch between the number of arguments and the substitution points.
So, how about this compromise: We keep the args in the string, but include some basic formatters that don't need function calls?
Like so:
{
"id": "receipt",
"component": "Text",
"text": {
"call": "formatString",
"args": {
"template": "Bought ${/transaction/quantity} ${/transaction/name} on ${/transaction/timestamp:%m %d, %Y} for ${/transaction/amount:.2f}.",
}
}
}In the end, I think the best solution will be whatever generates the best results from the various LLMs, and we kind of need to run an implementation through evals to know that definitively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wanted to add: if we remove mention of the DSL above from the core A2UI specification, and just include it in the definition for the stringFormat function in the standard catalog, then developers using custom catalogs would be completely free to implement your exact proposal, @wrenj .
So people using the standard catalog get a convenient string format syntax that is (hopefully!) simple enough for humans and LLMs to write directly. But Catalog creators have the hooks they need to innovate with other approaches, and they are not tethered to the DSL we are using in the standard catalog.
Re the actual DSL (sorry, not sure if it technically counts as a language), maybe we should try to stick more closely to the Python fstring format, e.g. by using {/dataModelReference} instead of ${/dataModelReference}, so the LLM can leverage its knowledge there. Though this might encourage it to hallucinate python code and formatting options. Just seems confusing to be quite close to Python, but then arbitrarily make some different design decisions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re the actual DSL (sorry, not sure if it technically counts as a language), maybe we should try to stick more closely to the Python fstring format,
It was intended to be close to TypeScript, JavaScript, Dart, Bash, Kotlin, PHP and Groovy. I figured that was a larger learning base than Python and Rust.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was intended to be close to TypeScript, JavaScript, Dart, Bash, Kotlin, PHP and Groovy. I figured that was a larger learning base than Python and Rust.
Oh that makes perfect sense. My language knowledge is not as broad as yours :-D. I was referring to the colon syntax in {/transaction/amount:.2f} which I suggested based on Python, but probably its the same as other languages.
Overall, I don't feel strongly other than borrowing familiar syntax ideas, which we already have!
|
This direction makes more sense to me than the original proposal. Moving formatting out of inline string templates avoids drifting into eval-like territory, reduces brittleness, and aligns better with the trust-boundary goals of the spec. Pre-formatting values before interpolation is a cleaner and more predictable model. I was going to suggest something similar conceptually, though I wonder if this could be taken one step further by separating concerns more explicitly. Instead of expanding a generic functions map, it may be cleaner to introduce a dedicated formatters (or format) catalog specifically for interpolation. That catalog could expose a predefined set of formatters (date, number, currency, pluralization, etc.) that operate over resolved values, rather than requiring the model or spec author to reason about low-level formatting details. In most cases these could defer to standard platform APIs and inferred context, for example locale detection: This keeps formatting deterministic, testable, and implementation-owned, rather than encoded in templates or model output. Users could still register custom formatters where needed, but the common cases would be handled consistently out of the box. I also think this structure makes it easier to support full i18n down the line. Once the template itself is treated as an i18n resource, with values passed in as already-formatted tokens, localization becomes a much more natural extension of the system rather than a retrofit. Overall, the move from:
to:
is the right abstraction boundary IMO. The remaining question is mostly about where those formatting primitives live and how strongly we want to constrain them for safety, portability, and localization. |
|
@derekdon Thanks for the comment. Our thinking was to put something super basic in here so its easy for open source renderers to implement and let clients add more advanced formatting like i18n in custom catalogs. We had looked at just wrapping MessageFormat syntax https://messageformat.github.io/messageformat/guide/ but didn't want each renderer to have to import the message format library. Based on Jacob's comment above and to keep this simple I think I'll just make this a simple string_format function with some basic python style formatters. |
| "call": "formatString", | ||
| "args": { | ||
| "value": "Hello, ${/user/firstName}! Welcome back to ${/appName}." | ||
| "template": "Bought ${quantity} ${item} on ${date} for ${price}.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you're going for here, but it seems like it's a lot more verbose than the previous design, and still requires parsing/validating of the template to interpolate the values, and escaping of the template markers. I realize that it's a simpler parser, but it's not that much simpler (and that's not really where we want to optimize complexity).
An equivalent string_format argument would be:
"Bought ${formatNumber(${/transaction/count}, 0)} ${pluralize(${/transaction/count}, 'apple', 'apples')} on ${formatDate(${/transaction/timestamp}, 'MMM d, yyyy')} for ${formatCurrency{${/transaction/amount}, 'USD')}"
To me, this seems easier/smaller to generate, since it doesn't require the extra argument names, "call", and "args" objects and eliminates much of the nesting. It has only marginally harder parsing complexity (which we'll put into a reusable SDK function anyhow).
| } | ||
| }, | ||
| { | ||
| "name": "formatNumber", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked this over with Jacob, and I still think that I'd rather use the more "modern" version of string interpolation that just takes a single string:
- It it just simpler to construct if the LLM needs to construct one string instead of a string and also a separate set of nested args.
- They both have equivalent functionality, and don't provide any extra validation, since any of these options need to have validation that involves validating the template or string against the args anyhow.
- Having a fixed set of built in formatting options is not as flexible in the long run, although I guess there's nothing saying we couldn't have some built in ones along with the ability to interpolate function outputs.
- Also, with a single string there's no chance of having a mismatch between the number of arguments and the substitution points.
So, how about this compromise: We keep the args in the string, but include some basic formatters that don't need function calls?
Like so:
{
"id": "receipt",
"component": "Text",
"text": {
"call": "formatString",
"args": {
"template": "Bought ${/transaction/quantity} ${/transaction/name} on ${/transaction/timestamp:%m %d, %Y} for ${/transaction/amount:.2f}.",
}
}
}In the end, I think the best solution will be whatever generates the best results from the various LLMs, and we kind of need to run an implementation through evals to know that definitively.
| } | ||
| ``` | ||
|
|
||
| #### Client-Side Functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gspencergoog this defines a DSL to call functions from within string templates. I think if we do decide to support a syntax like this, we should move this specification to the "standard catalog" section and make it specific to the string_format function etc. So maybe string_format happens to use some magical DSL and be able to refer to the data model and other client-side functions directly (we will expose the necessary rendering framework APIs to make this feasible to implement). A different catalog can invent some other magical DSL. But the core A2UI specification is more purely JSON-based.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this section should be removed. I agree that it should only be part of the string_format function's definition, and not general for all strings. (I thought I had already removed that when I added string_format).
| }, | ||
| "currencyCode": { | ||
| "$ref": "common_types.json#/$defs/DynamicString", | ||
| "description": "Optional. The ISO 4217 currency code (e.g., 'USD', 'EUR'). If omitted, uses the device's default currency." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How does the LLM know the device's default currency to be able to put in the relevant value?
|
chatted offline with greg, we'll keep the syntax as is and make sure any custom function can access the interpolation functionality as well as hook up databinding, probably using a callcontext that can resolve data model values or functions |
| }, | ||
| { | ||
| "name": "pluralize", | ||
| "description": "Returns the singular string if the count is 1, otherwise returns the plural string.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this definition is only sufficient for English text. Other languages will need more complex rules, and more arguments.
Proposal using just simple client functions instead of interpolation syntax