test: add vitest coverage for tool render modules#123
Merged
Conversation
Add per-renderer vitest coverage with XSS escaping assertions, a shared test helper, registry-driven smoke tests, and raised coverage thresholds.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (20)
🚧 Files skipped from review as they are similar to previous changes (16)
📝 WalkthroughWalkthroughThis PR adds shared renderer test helpers, registry consistency and smoke tests, broad Vitest coverage for ChangesRenderer Behavioral Test Coverage
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 (1)
static/js/render/tool_result/file_edit.test.js (1)
1-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a shared test factory for summary-only renderers.
This file and
plan.test.jsfollow an identical three-test pattern (render field, missing field defaults, XSS escape) for renderers that just build a"<Label>: <field>"summary viafinishToolResult. A small parametrized helper (e.g.,testSummaryOnlyRenderer(name, renderFn, field, label)) could reduce duplication if more such renderers are added later.🤖 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 `@static/js/render/tool_result/file_edit.test.js` around lines 1 - 29, The tests in renderFileEditResult duplicate the same three-case pattern used by other summary-only renderers, so extract a shared parametrized test helper for this shape. Create a reusable factory (for example, around mountToolResult, expectNoRawHtml, and the renderer under test) that accepts the render function, field name, and label, then use it from file_edit.test.js and plan.test.js to cover summary, missing-field default, and XSS escaping consistently.
🤖 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 `@static/js/render/tool_result/task.test.js`:
- Around line 6-19: The completed-task rendering test is asserting a
locale-specific token string, which can vary across environments. Update the
`renderTaskResult` test to avoid hard-coding the formatted token value in the
`renders completed task with duration and token stats` case, and instead compare
against a locale-agnostic expectation or derive the expected text using
`toLocaleString()` from the same numeric value.
---
Nitpick comments:
In `@static/js/render/tool_result/file_edit.test.js`:
- Around line 1-29: The tests in renderFileEditResult duplicate the same
three-case pattern used by other summary-only renderers, so extract a shared
parametrized test helper for this shape. Create a reusable factory (for example,
around mountToolResult, expectNoRawHtml, and the renderer under test) that
accepts the render function, field name, and label, then use it from
file_edit.test.js and plan.test.js to cover summary, missing-field default, and
XSS escaping consistently.
🪄 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
Run ID: 759aa381-9776-4d07-9d03-ffacba6ed9a2
📒 Files selected for processing (23)
static/js/render/registry.test.jsstatic/js/render/test_helpers.jsstatic/js/render/tool_result/bash.test.jsstatic/js/render/tool_result/fallback.test.jsstatic/js/render/tool_result/file_edit.test.jsstatic/js/render/tool_result/file_write.test.jsstatic/js/render/tool_result/glob.test.jsstatic/js/render/tool_result/grep.test.jsstatic/js/render/tool_result/plan.test.jsstatic/js/render/tool_result/task.test.jsstatic/js/render/tool_result/todo_write.test.jsstatic/js/render/tool_result/user_input.test.jsstatic/js/render/tool_result/web_fetch.test.jsstatic/js/render/tool_result/web_search.test.jsstatic/js/render/tool_use/ask_user_question.test.jsstatic/js/render/tool_use/fallback.test.jsstatic/js/render/tool_use/glob.test.jsstatic/js/render/tool_use/grep.test.jsstatic/js/render/tool_use/read.test.jsstatic/js/render/tool_use/task.test.jsstatic/js/render/tool_use/todo_write.test.jsstatic/js/render/tool_use/write.test.jsvitest.config.js
timon0305
requested changes
Jul 8, 2026
timon0305
approved these changes
Jul 8, 2026
wpak-ai
approved these changes
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #115
Summary
Adds vitest files for the remaining registered renderers in
tool_use/andtool_result/. We had five test files covering a small slice of 32 render modules; now every registered renderer has at least one behavioral test.test_helpers.jswrapsrenderToolUse/renderToolResultand gives a consistent way to check that raw HTML does not leak through. Most tests also cover empty or malformed input.registry.test.jsruns representative fixtures through every entry inTOOL_USE_RENDERERSandTOOL_RESULT_RENDERERS.Coverage thresholds in
vitest.config.jsmove from 80/70/50 to 85/75/55.Test plan
npm test(156 passing, up from 93)npm run test:coverage(tool_result/andtool_use/both around 99%)Week 28 item #3. Wednesday PR 2 (adversarial dispatch fixtures) is separate.
Summary by CodeRabbit
Bug Fixes
Tests