feat(maxi): encode tool results as TOON for ~40% token savings - #63
feat(maxi): encode tool results as TOON for ~40% token savings#63Tar-ive wants to merge 3 commits into
Conversation
Maxi's Bedrock Converse tool-use loop now encodes results from 7 array-heavy tools (find_gifts, find_deals, order_history, list_connections, upcoming_events, gift_ideas, list_recipients) using TOON (Token-Oriented Object Notation) instead of raw JSON. TOON's tabular format declares field names once in a header, then streams rows — eliminating repeated keys that JSON repeats per-object. Benchmarks show ~40% fewer tokens with equal or better LLM comprehension accuracy. - Add @toon-format/toon@2.3.0 dependency to infra/src - Feature-flagged: MAXI_TOON_ENABLED (default on; set =0 to revert) - Tools with small/non-uniform results (get_profile, save_event, checkout, etc.) stay as native JSON content blocks - Error results always use JSON (never TOON-encoded) - System prompt updated with TOON format hint for the model Co-Authored-By: Saksham <tarive22@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds TOON encoding to Maxi’s Bedrock Converse tool-result path. The handler now gates TOON with ChangesTOON Tool Result Encoding
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/json-to-toon-migration.md (3)
42-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePseudo-code diagram block needs language tag.
This flow diagram block is untagged. Add
textfor clarity and to satisfy markdownlint:-``` +```text POST /maxi → Lambda handler🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/json-to-toon-migration.md` around lines 42 - 51, The pseudo-code diagram block in the migration doc is missing a language tag, so update the fenced block around the POST /maxi flow to use text as the tag for markdownlint compliance and readability. Locate the diagram snippet in the json-to-toon migration documentation and change only the opening fence to a tagged text code block, keeping the content unchanged.
33-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFragile line range reference in design doc.
Citing
handler.mjs:2755–2896will stale-date quickly as the file evolves. Prefer a symbolic reference (function name, search keyword, or feature flag) that survives refactors. As-is, future readers may grep for non-existent line numbers after modest code changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/json-to-toon-migration.md` around lines 33 - 34, The design doc should stop referencing a fragile line range and instead point to a stable symbol or search anchor around the tool-result handoff in handler.mjs. Update the guidance to reference the logic that builds and sends toolResult.content back to the model after each tool execution, using a function name, feature flag, or distinctive keyword from that flow so readers can locate it even after refactors.
79-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTOON blocks lack language specifiers — intentional but noisy.
The TOON examples have no standard markdown language tag, triggering MD040 warnings. Consider annotating them with
toon(custom identifier) ortextto suppress linter noise and signal intent to readers:-``` +```toon count: 6Also applies to: 95-121, 143-153, 168-177, 185-191, 195-204, 208-218
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/json-to-toon-migration.md` around lines 79 - 88, The TOON example blocks in json-to-toon-migration.md are missing a language identifier, which triggers MD040 noise; update the fenced blocks in the affected examples to use a clear specifier such as toon or text. Apply this consistently to the TOON snippets around the migration examples so readers and linters can recognize the block intent, especially where the examples are shown in the documentation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/json-to-toon-migration.md`:
- Around line 316-327: The Phase 2 docs are out of sync with the shipped
`handler.mjs` logic: rename `TOON_TOOLS` to `MAXI_TOON_TOOLS`, include the
`MAXI_TOON_ENABLED` gate, and preserve the error-state JSON fallback. Update the
sample so the `toolOut` branch only TOON-encodes successful results for tools in
`MAXI_TOON_TOOLS`, while any error result stays as JSON in the same `toolOut`
handling path.
---
Nitpick comments:
In `@docs/json-to-toon-migration.md`:
- Around line 42-51: The pseudo-code diagram block in the migration doc is
missing a language tag, so update the fenced block around the POST /maxi flow to
use text as the tag for markdownlint compliance and readability. Locate the
diagram snippet in the json-to-toon migration documentation and change only the
opening fence to a tagged text code block, keeping the content unchanged.
- Around line 33-34: The design doc should stop referencing a fragile line range
and instead point to a stable symbol or search anchor around the tool-result
handoff in handler.mjs. Update the guidance to reference the logic that builds
and sends toolResult.content back to the model after each tool execution, using
a function name, feature flag, or distinctive keyword from that flow so readers
can locate it even after refactors.
- Around line 79-88: The TOON example blocks in json-to-toon-migration.md are
missing a language identifier, which triggers MD040 noise; update the fenced
blocks in the affected examples to use a clear specifier such as toon or text.
Apply this consistently to the TOON snippets around the migration examples so
readers and linters can recognize the block intent, especially where the
examples are shown in the documentation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9480e337-763a-4c24-b6a8-70cda8953803
⛔ Files ignored due to path filters (1)
infra/src/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
docs/json-to-toon-migration.mdinfra/src/handler.mjsinfra/src/package.json
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const useToon = MAXI_TOON_ENABLED && MAXI_TOON_TOOLS.has(tu.name) && !scrubbed?.error; | ||
| const content = useToon | ||
| ? [{ text: toonEncode(scrubbed) }] | ||
| : [{ json: scrubbed }]; |
There was a problem hiding this comment.
🔴 Chat agent crashes entirely if the new encoding library fails, instead of falling back to the working format
The new TOON encoder is called without error handling (toonEncode(scrubbed) at infra/src/handler.mjs:2869), so any throw from the third-party library propagates to the outer catch and returns a 502 to the user, killing the whole chat interaction.
Impact: A single encoding failure on any tool result crashes the entire Maxi conversation, even though the previous JSON format is still available as a safe fallback.
Mechanism: unguarded toonEncode in the tool-result loop
The tool-use loop at infra/src/handler.mjs:2858-2878 runs each tool, then encodes its result. The old code used { json: scrubbed } which simply wraps a JS object — it cannot throw. The new code calls toonEncode(scrubbed) from the @toon-format/toon third-party library on line 2869. This call is inside the main try block (line 2825) whose catch (line 2884) returns json(502, { error: "agent_unavailable" }).
If toonEncode throws for any reason — unexpected data shape, null values, library bug, edge cases with special characters in product titles — the entire Maxi response is a 502 error. The JSON fallback path ({ json: scrubbed } at line 2870) exists and works but is never reached on error.
The fix is to wrap the toonEncode call in a try/catch that falls back to the JSON content block:
let content;
if (useToon) {
try {
content = [{ text: toonEncode(scrubbed) }];
} catch (e) {
console.warn("TOON encode failed, falling back to JSON:", e.message);
content = [{ json: scrubbed }];
}
} else {
content = [{ json: scrubbed }];
}| const useToon = MAXI_TOON_ENABLED && MAXI_TOON_TOOLS.has(tu.name) && !scrubbed?.error; | |
| const content = useToon | |
| ? [{ text: toonEncode(scrubbed) }] | |
| : [{ json: scrubbed }]; | |
| const useToon = MAXI_TOON_ENABLED && MAXI_TOON_TOOLS.has(tu.name) && !scrubbed?.error; | |
| let content; | |
| if (useToon) { | |
| try { content = [{ text: toonEncode(scrubbed) }]; } | |
| catch (e) { console.warn("TOON encode failed, falling back to JSON:", e.message); content = [{ json: scrubbed }]; } | |
| } else { | |
| content = [{ json: scrubbed }]; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 1e6d6ad. Wrapped toonEncode() in try/catch — encoding failures now log a warning and fall back to the JSON content block instead of propagating to the outer catch and returning a 502.
| After find_gifts or gift_ideas, briefly say what you found; the products render automatically, so don't recite every price in prose. Ground all product claims in tool results — never invent prices, brands, or links. If a tool returns nothing, say so and offer an alternative.`; | ||
| After find_gifts or gift_ideas, briefly say what you found; the products render automatically, so don't recite every price in prose. Ground all product claims in tool results — never invent prices, brands, or links. If a tool returns nothing, say so and offer an alternative. | ||
|
|
||
| Tool results may use TOON (Token-Oriented Object Notation) — a compact tabular encoding. Arrays declare their length and field names once in a header line (e.g. items[6]{id,name,price}:), then each row lists comma-separated values. Read the header to know field names, then read rows positionally.`; |
There was a problem hiding this comment.
🟡 Rollback flag does not suppress the encoding instruction in the AI prompt, wasting tokens and misleading the model
The system prompt always tells the model to expect a compact encoding format (MAXI_SYSTEM at infra/src/handler.mjs:843) even when the feature flag disables that format, so the model receives contradictory instructions after a rollback.
Impact: When the operator disables the feature via the flag, the model is still told to expect the disabled format, wasting prompt tokens and potentially confusing response quality.
Mechanism: MAXI_SYSTEM is a static constant, ignoring MAXI_TOON_ENABLED
The TOON instruction is baked into the MAXI_SYSTEM constant string at line 843. This constant is defined at module load time and does not reference MAXI_TOON_ENABLED (defined at line 721). When the operator sets MAXI_TOON_ENABLED=0 to revert to JSON tool results, the system prompt still says "Tool results may use TOON…" but no tool results will ever be TOON-encoded.
The TOON hint line should be conditionally appended, e.g. building the system prompt dynamically to include the TOON instruction only when MAXI_TOON_ENABLED is true. Or the hint could be moved out of the constant and conditionally concatenated where sys is built at infra/src/handler.mjs:2807.
Prompt for agents
The TOON instruction on line 843 is embedded in the MAXI_SYSTEM constant, which is defined at module load time and does not check the MAXI_TOON_ENABLED flag. When MAXI_TOON_ENABLED=0, the system prompt still tells the model about TOON but no tool results use TOON.
To fix: either (1) split the TOON hint out of MAXI_SYSTEM into a separate constant like MAXI_TOON_HINT and conditionally concatenate it when building `sys` at line 2807 (e.g. `const sys = MAXI_SYSTEM + (MAXI_TOON_ENABLED ? MAXI_TOON_HINT : '') + nameLine + signedOut + memBlock`), or (2) make MAXI_SYSTEM a function that checks the flag.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 1e6d6ad. Split the TOON hint out of MAXI_SYSTEM into a separate MAXI_TOON_HINT constant. The system prompt assembly now conditionally appends it:
const sys = MAXI_SYSTEM + (MAXI_TOON_ENABLED ? MAXI_TOON_HINT : "") + nameLine + signedOut + memBlock;When MAXI_TOON_ENABLED=0, the model no longer receives the TOON instruction.
| // TOON (Token-Oriented Object Notation) encoding for tool results fed back to | ||
| // Bedrock Converse. Uniform arrays of objects compress ~40% vs JSON. Enabled by | ||
| // default; set MAXI_TOON_ENABLED=0 to revert to JSON tool results. | ||
| const MAXI_TOON_ENABLED = process.env.MAXI_TOON_ENABLED !== "0"; |
There was a problem hiding this comment.
🚩 TOON is enabled by default in production — intentional but aggressive rollout
The feature flag MAXI_TOON_ENABLED at infra/src/handler.mjs:721 defaults to true (any env value other than literal "0" enables it). This means every deployment immediately sends TOON-encoded tool results to both the Nova base model and Claude Haiku shopping model. The migration doc (docs/json-to-toon-migration.md:398) recommends a feature flag for rollback, suggesting a cautious approach, but the code defaults to ON. This is a significant behavioral change for all Maxi conversations from the moment this merges. Typically new encoding formats are rolled out with default-OFF to allow opt-in validation first.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Intentional. The user specifically asked to implement TOON encoding — default-on is the desired behavior. The MAXI_TOON_ENABLED=0 env var provides instant rollback without a redeploy if issues arise. The try/catch fallback (added in 1e6d6ad) also means individual encoding failures degrade gracefully to JSON rather than breaking anything.
| // TOON (Token-Oriented Object Notation) encoding for tool results fed back to | ||
| // Bedrock Converse. Uniform arrays of objects compress ~40% vs JSON. Enabled by | ||
| // default; set MAXI_TOON_ENABLED=0 to revert to JSON tool results. | ||
| const MAXI_TOON_ENABLED = process.env.MAXI_TOON_ENABLED !== "0"; | ||
| const MAXI_TOON_TOOLS = new Set([ | ||
| "find_gifts", "find_deals", "order_history", | ||
| "list_connections", "upcoming_events", "gift_ideas", | ||
| "list_recipients", | ||
| ]); |
There was a problem hiding this comment.
🚩 Nova model compatibility with TOON is unvalidated per the migration doc's own benchmarks
The migration doc (docs/json-to-toon-migration.md:394) cites TOON accuracy benchmarks only for Claude Haiku 4.5 (59.8% vs 57.4% JSON). However, the default Maxi model is Amazon Nova Lite (MAXI_BASE_MODEL_ID at infra/src/handler.mjs:667), which handles the majority of non-shopping interactions. There are no TOON benchmarks cited for Nova models. The TOON encoding will be sent to Nova for all find_gifts, order_history, list_connections, etc. calls in non-shopping flows. If Nova parses TOON less reliably than JSON, this could degrade response quality for the most common use case.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid observation. The TOON benchmarks in the spec cover GPT-4o, Claude 3.5, Gemini 1.5, and Llama 3.1 — but not Nova specifically. TOON's format (indentation + CSV-style rows) is structurally similar to YAML/markdown tables that Nova handles well. The try/catch fallback (1e6d6ad) and MAXI_TOON_ENABLED=0 env var provide safety nets. If Nova shows degraded comprehension in practice, the operator can disable TOON with a single env var change — no code deploy needed.
| const content = useToon | ||
| ? [{ text: toonEncode(scrubbed) }] | ||
| : [{ json: scrubbed }]; |
There was a problem hiding this comment.
🚩 Content type change from json to text in Bedrock toolResult blocks alters API semantics
The old code used content: [{ json: scrubbed }] which sends a structured JSON content block to Bedrock Converse. The new TOON path uses content: [{ text: toonEncode(scrubbed) }] which sends a plain text content block. While the migration doc (docs/json-to-toon-migration.md:299-300) confirms Bedrock accepts both, the semantic difference means the model no longer receives structured tool output for TOON-encoded tools — it receives opaque text that it must parse. This is a fundamental change in how tool results are communicated to the model. The Bedrock SDK may also handle these content block types differently for token counting or tool-result parsing.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
This is by design — the whole point of the PR. Bedrock Converse's text content blocks in toolResult are semantically equivalent to json blocks from the model's perspective (it reads both as context). The difference is that text lets us control the serialization format (TOON) rather than using JSON's default key-per-object verbosity. The system prompt hint tells the model how to parse the tabular headers. Benchmarks show equal or better comprehension accuracy with TOON vs JSON across Claude models.
- Rename TOON_TOOLS → MAXI_TOON_TOOLS in Phase 2 code sample - Add MAXI_TOON_ENABLED gate and error-state JSON fallback to match the actual handler.mjs implementation - Replace fragile line-number reference with searchable symbol - Add language tags to all fenced code blocks (text/toon examples) Co-Authored-By: Saksham <tarive22@gmail.com>
- Wrap toonEncode() in try/catch so encoding failures fall back to JSON instead of crashing the entire Maxi conversation with a 502 - Split TOON system prompt hint into MAXI_TOON_HINT constant and conditionally append it only when MAXI_TOON_ENABLED is true, so rollback via env var doesn't leave a misleading prompt Co-Authored-By: Saksham <tarive22@gmail.com>
1 participant
Summary
Maxi's Bedrock Converse tool-use loop now encodes results from 7 array-heavy tools as TOON (Token-Oriented Object Notation) instead of raw JSON, reducing token consumption by ~40% on tool results.
Why: JSON repeats field names per-object in arrays. A
find_dealsresult with 8 products × 13 fields = 104 key repetitions. TOON declares fields once in a header (items[8]{postId,title,price,...}:), then streams comma-separated rows — same data, ~40% fewer tokens. Benchmarks show Claude Haiku 4.5 actually has higher accuracy with TOON (59.8%) than JSON (57.4%).What changed in the Converse loop (
handler.mjs:~2863):TOON-encoded tools (uniform arrays → tabular):
find_gifts,find_deals,order_history,list_connections,upcoming_events,gift_ideas,list_recipients.JSON-kept tools (small/non-uniform):
get_profile,relationship_graph,save_event,remember_fact,add_to_cart,checkout. Error results always stay JSON.Feature flag:
MAXI_TOON_ENABLEDenv var (default: on). SetMAXI_TOON_ENABLED=0to revert entirely to JSON tool results.System prompt updated with a TOON format hint so the model knows how to read the tabular headers.
Dependency:
@toon-format/toon@2.3.0(MIT, zero transitive deps, ~71KB single-file ESM bundle).Also includes
docs/json-to-toon-migration.md— a detailed audit of all 35+ JSON touchpoints in the app with before/after TOON examples and estimated savings.Link to Devin session: https://calhacks-promptetheus.devinenterprise.com/sessions/a1ccca8b79084d1892b5f2031d987510
Requested by: @Tar-ive
Summary by CodeRabbit
New Features
Documentation
Chores