test: align file-preview tests with #524 and #526#527
Merged
Conversation
Two tests trailed the latest contract changes: - test-file-handlers.js: #526 dropped structuredContent.imageData and carries the image base64 only in structuredContent.content. Update the image/SVG assertions to read content instead of the removed field. - test-markdown-preview.js: #524 tags read_file calls with origin:'ui'. The Test 9 and Test 11 mocks asserted exact read_file args via deepStrictEqual; add origin:'ui' so the full-document reload and the failed-save resync paths match again. No source changes; both were stale tests, not regressions.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughTests and version metadata were updated for the new release. Preview assertions now expect Changesread_file contract updates and release version bump
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/test-file-handlers.js (1)
336-337: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the structured preview bytes match the image content bytes.
These checks now only prove
structuredContent.contentis present. Since the handler builds both fields from the same base64 payload, asserting equality withimageContentItem.data/svgContentItem.datawould catch contract drift instead of only emptiness.Diff to tighten the contract assertion
assert.strictEqual(typeof imageResult.structuredContent.content, 'string', 'Image structured payload should carry base64 in content'); assert.ok(imageResult.structuredContent.content.length > 0, 'Image structured payload should include non-empty content'); + assert.strictEqual(imageResult.structuredContent.content, imageContentItem.data, 'Image structured payload should mirror the image content bytes'); @@ assert.strictEqual(typeof svgResult.structuredContent.content, 'string', 'SVG structured payload should carry base64 in content'); assert.ok(svgResult.structuredContent.content.length > 0, 'SVG structured payload should include non-empty content'); + assert.strictEqual(svgResult.structuredContent.content, svgContentItem.data, 'SVG structured payload should mirror the image content bytes');Also applies to: 351-352
🤖 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 `@test/test-file-handlers.js` around lines 336 - 337, The current assertions in the image/SVG handler tests only verify that structuredContent.content exists and is non-empty, but they do not confirm it matches the source payload. Update the relevant checks in the image and SVG test cases to compare structuredContent.content against the corresponding content item data (imageContentItem.data and svgContentItem.data) so the test enforces the intended base64 contract in the handler output.
🤖 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.
Nitpick comments:
In `@test/test-file-handlers.js`:
- Around line 336-337: The current assertions in the image/SVG handler tests
only verify that structuredContent.content exists and is non-empty, but they do
not confirm it matches the source payload. Update the relevant checks in the
image and SVG test cases to compare structuredContent.content against the
corresponding content item data (imageContentItem.data and svgContentItem.data)
so the test enforces the intended base64 contract in the handler output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27e42643-d653-44f7-a77a-53e1a451008f
📒 Files selected for processing (2)
test/test-file-handlers.jstest/test-markdown-preview.js
edgarsskore
approved these changes
Jun 26, 2026
Automated release commit with version bump from 0.2.42 to 0.2.43
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
2 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.
What
Fixes the two failing tests in the suite (
test-file-handlers.js,test-markdown-preview.js). Both were stale tests trailing the two most recent merges — not MCP regressions.Why they failed
test-file-handlers.js(Test 9) — #526 (refactor(read_file): carry image base64 once) deliberately removedstructuredContent.imageDataand now carries the image base64 only instructuredContent.content. The test still assertedimageDatawas a string, so it gotundefined.test-markdown-preview.js(Test 9) — #524 (feat(telemetry): call_origin ui vs llm) appendsorigin: 'ui'to the controller'sread_filecall. The mock asserted exact args viadeepStrictEqual({ path, offset, length }); the extra key made it throw,loadFullDocument'scatchswallowed it, and the full-document reload silently aborted — leaving the baseline at the truncated# Intro. The same staleness was hiding behind it in Test 11 (testFailedSaveResyncsEditBaseline, the runner stops at the first failure).Changes
structuredContent.contentinstead of the removedimageData.origin: 'ui'to the expectedread_fileargs in Test 9 and Test 11.No source changes.
Verification
Full suite green locally: 42/42 passed.
Summary by CodeRabbit