fix(formatter): omit null/undefined and indent array items in plain output#92
Conversation
…utput Plain format printed literal "undefined"/"null" for empty fields and only indented the first key of each array item, collapsing item boundaries. Items now use a "- " marker with aligned continuation lines, nested objects indent under their parent key, and empty fields are omitted (json format still keeps null for scripting).
The 58.9% figure was not reproducible; benchmarking real TaskView list output (10-200 rows) with o200k/cl100k tokenizers measures ~37% vs JSON, and ~6% on detail views dominated by long free text.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe plain-text output formatter was reworked to use indentation-aware helper functions, changing array rendering to use "- " markers and omitting object properties with undefined or null values. TOON documentation comments were updated with revised token-savings figures. Tests were added to cover the new formatting behavior. ChangesPlain Formatter Rewrite
Estimated code review effort: 2 (Simple) | ~12 minutes PoemA rabbit hopped through nested keys, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Code Review
This pull request refactors the plain text formatter in src/utils/formatter.ts to support proper indentation for nested objects and arrays, and to omit null and undefined values. It also updates the documentation and tests accordingly. The review feedback correctly identifies a bug where nested arrays are improperly formatted as objects because typeof item === 'object' evaluates to true for arrays, and suggests adding a corresponding test case to verify nested array formatting.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/utils/formatter.ts`:
- Around line 148-155: The nested array/object handling in formatter.ts
currently pushes the result of formatPlainArray and formatPlainObject even when
they return an empty string, creating stray blank lines after the key label.
Update the logic in the Array.isArray(value) and typeof value === 'object'
branches to check the formatted body before pushing it, so empty arrays/objects
only emit the key line and nothing else. Keep the fix localized around the
formatter flow that uses formatPlainArray, formatPlainObject, and keyLabel.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 04b3fda1-c4b5-4b11-a6ef-4fce69af92ba
📒 Files selected for processing (2)
src/utils/formatter.tstest/utils/formatter.test.ts
- handle nested arrays in plain format via Array.isArray branch (gemini: nested arrays were rendered as index-keyed objects) - skip empty array/object bodies to avoid stray blank lines (coderabbit/greptile: empty collections emitted a blank line) - add regression tests for both cases
|
/gemini review |
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/utils/formatter.ts">
<violation number="1" location="src/utils/formatter.ts:129">
P2: Nested arrays are incorrectly formatted as objects here. Since `typeof [] === 'object'` is `true` in JavaScript, any array element that is itself an array will be passed to `formatPlainObject`, which iterates over its numeric indices as keys. Add an `Array.isArray(item)` check before the object check to recurse correctly:
```ts
if (Array.isArray(item)) {
const body = formatPlainArray(item, colors, itemIndent)
return `${indent}- ${body.slice(itemIndent.length)}`
}
if (typeof item === 'object' && item !== null) {
...
}
```</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
Code Review
This pull request refactors the plain text formatting utility to support proper indentation for nested arrays and objects, prefix array items with a dash, and omit null or undefined values. It also updates the documentation and tests to reflect these changes. The reviewer suggested adding a test case to verify that multiline strings are correctly indented in plain format, which is a valid improvement for test integrity.
… output Continuation lines of multiline scalar values now align under their key (gemini review suggestion, with the behavior implemented alongside the requested regression test)
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the plain text formatter in src/utils/formatter.ts to support proper nested indentation, omit null/undefined values, and handle multiline strings. It also updates the documentation and tests regarding TOON format token savings. Feedback was provided regarding a missing indentation alignment for multiline strings when they are elements of an array.
Continuation lines of multiline strings in array items now align under the `- ` marker content, matching the object-value behavior (gemini review suggestion, with regression test)
|
/gemini review |
|
There was a problem hiding this comment.
Code Review
This pull request refactors the plain text formatting logic in src/utils/formatter.ts to support proper indentation, item markers, omission of null/undefined values, and multiline strings, along with adding comprehensive unit tests. The reviewer suggested adding a test case to ensure Date objects are correctly formatted as ISO strings in plain text format rather than being treated as empty objects.



Summary
Fixes the plain-text formatter to skip null/undefined fields, use "- " item markers with aligned indentation for arrays, and properly indent nested objects. Also corrects an inline comment about TOON token-savings from an unreproducible 58.9% claim to a measured ~37% (using o200k/cl100k tokenizers on TaskView-shaped list data; ~6% savings observed on long-text detail views).
Changes
fix(formatter): omit null/undefined fields in plain-text output; use "- " markers with aligned indentation for array items; fix nested object indentationdocs(formatter): correct TOON token-savings claim in comment to measured ~37% (was an unreproducible 58.9%)Related issue
None found — no issue linked.
Test Plan
bun test)bun run lint)Checklist
bun run test)bun run lint)BREAKING CHANGE:note is includedSummary by cubic
Fix the plain-text formatter to omit null/undefined fields, mark array items with "- " and indent nested structures, and align continuation lines for multiline strings (including in arrays). Also updates the TOON comment to ~37% token savings vs JSON (was 58.9%).
Written for commit b1284e7. Summary will update on new commits.
Summary by CodeRabbit
Documentation
Bug Fixes
Tests