Repository navigation
Remove dropped ui_artifacts search columns - #94
Merged
Merged
Conversation
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
📝 WalkthroughWalkthroughThis PR removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
kentcdodds
changed the base branch from
cursor/ui-artifacts-unused-columns-c9f5
to
main
March 29, 2026 20:38
kentcdodds
marked this pull request as ready for review
March 29, 2026 20:39
Contributor
|
🔎 Preview deployed: https://kody-pr-94.kentcdodds.workers.dev Worker: Mocks:
|
![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)
packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts (1)
921-921: Use a code-only token in this search regression.This query still matches the title/description, so it would keep passing even if
row.codestopped contributing to app search. A unique term that exists only insidecodewould make the post-migration behavior explicit.📌 Suggested test tweak
- code: '<main><h1>Saved Searchable App Demo</h1></main>', + code: '<main><h1>Saved Searchable App Demo</h1><p>saved-app-code-token</p></main>', ... - query: 'searchable saved ui artifact demo', + query: 'saved-app-code-token',Also applies to: 938-939
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts` at line 921, The test inserts an app with code that currently contains the same text as the title/description so the search still matches even if row.code stops contributing; change the inserted object(s) where the code property is set (refer to row.code in the failing test cases around the test in mcp-server.mcp-e2e.test.ts) to include a unique, code-only token (e.g. a random or clearly unique string) that does not appear in title/description, and update the assertions to search for that unique token (also update the similar occurrences noted around lines 938-939) so the test will only pass if row.code is actually indexed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/worker/src/mcp/ui-artifacts-embed.ts`:
- Around line 38-47: The embed currently places raw source (input.code) before
structured metadata so slice(maxChars) can chop off runtime/parameterText;
update the construction in ui-artifacts-embed.ts so structured metadata
(input.title, input.description, input.runtime, 'mcp app', 'ui artifact',
parameterText) appear before the source, and then append input.code last (or
build structuredPart + codePart and, if truncated to maxChars, only trim the
codePart) so that runtime and parameterText are preserved in the final sliced
text; adjust variables text/structuredPart/codePart and the final return to
ensure parameterText and runtime are kept ahead of the code when enforcing
maxChars.
---
Nitpick comments:
In `@packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts`:
- Line 921: The test inserts an app with code that currently contains the same
text as the title/description so the search still matches even if row.code stops
contributing; change the inserted object(s) where the code property is set
(refer to row.code in the failing test cases around the test in
mcp-server.mcp-e2e.test.ts) to include a unique, code-only token (e.g. a random
or clearly unique string) that does not appear in title/description, and update
the assertions to search for that unique token (also update the similar
occurrences noted around lines 938-939) so the test will only pass if row.code
is actually indexed.
🪄 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: f3fc8f25-437b-4b05-b1b4-4ce8b12a328b
📒 Files selected for processing (13)
packages/worker/src/app/saved-ui-hosted-html.node.test.tspackages/worker/src/mcp/capabilities/apps/ui-get-app.tspackages/worker/src/mcp/capabilities/apps/ui-list-apps.tspackages/worker/src/mcp/capabilities/apps/ui-save-app.tspackages/worker/src/mcp/capabilities/apps/ui-update-app.tspackages/worker/src/mcp/capabilities/unified-search.workers.test.tspackages/worker/src/mcp/mcp-server.mcp-e2e.test.tspackages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.tspackages/worker/src/mcp/tools/search.node.test.tspackages/worker/src/mcp/ui-artifacts-embed.tspackages/worker/src/mcp/ui-artifacts-repo.tspackages/worker/src/mcp/ui-artifacts-search.tspackages/worker/src/mcp/ui-artifacts-types.ts
💤 Files with no reviewable changes (3)
- packages/worker/src/mcp/capabilities/unified-search.workers.test.ts
- packages/worker/src/app/saved-ui-hosted-html.node.test.ts
- packages/worker/src/mcp/ui-artifacts-types.ts
Comment on lines
38
to
47
| const text = [ | ||
| input.title, | ||
| input.description, | ||
| input.keywords.join(' '), | ||
| input.code, | ||
| input.runtime, | ||
| 'mcp app', | ||
| 'ui artifact', | ||
| ...(parameterText ? [parameterText] : []), | ||
| ...(input.searchText ? [input.searchText] : []), | ||
| ].join('\n') | ||
| return text.slice(0, maxChars) |
There was a problem hiding this comment.
Keep structured app metadata ahead of raw source in the embed budget.
Line 41 now inserts the full code before runtime and parameterText, then Line 47 truncates the whole document. For larger saved apps, parameter names/descriptions get cut off entirely, so searches for things like a runtime param or default value stop finding the app even though that metadata still exists in the row.
✂️ Suggested fix
- const text = [
- input.title,
- input.description,
- input.code,
- input.runtime,
- 'mcp app',
- 'ui artifact',
- ...(parameterText ? [parameterText] : []),
- ].join('\n')
- return text.slice(0, maxChars)
+ const metadataText = [
+ input.title,
+ input.description,
+ input.runtime,
+ 'mcp app',
+ 'ui artifact',
+ ...(parameterText ? [parameterText] : []),
+ ].join('\n')
+ const remainingChars = Math.max(0, maxChars - metadataText.length - 1)
+ const codeText = input.code.slice(0, remainingChars)
+ return [metadataText, codeText].filter(Boolean).join('\n')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const text = [ | |
| input.title, | |
| input.description, | |
| input.keywords.join(' '), | |
| input.code, | |
| input.runtime, | |
| 'mcp app', | |
| 'ui artifact', | |
| ...(parameterText ? [parameterText] : []), | |
| ...(input.searchText ? [input.searchText] : []), | |
| ].join('\n') | |
| return text.slice(0, maxChars) | |
| const metadataText = [ | |
| input.title, | |
| input.description, | |
| input.runtime, | |
| 'mcp app', | |
| 'ui artifact', | |
| ...(parameterText ? [parameterText] : []), | |
| ].join('\n') | |
| const remainingChars = Math.max(0, maxChars - metadataText.length - 1) | |
| const codeText = input.code.slice(0, remainingChars) | |
| return [metadataText, codeText].filter(Boolean).join('\n') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/worker/src/mcp/ui-artifacts-embed.ts` around lines 38 - 47, The
embed currently places raw source (input.code) before structured metadata so
slice(maxChars) can chop off runtime/parameterText; update the construction in
ui-artifacts-embed.ts so structured metadata (input.title, input.description,
input.runtime, 'mcp app', 'ui artifact', parameterText) appear before the
source, and then append input.code last (or build structuredPart + codePart and,
if truncated to maxChars, only trim the codePart) so that runtime and
parameterText are preserved in the final sliced text; adjust variables
text/structuredPart/codePart and the final return to ensure parameterText and
runtime are kept ahead of the code when enforcing maxChars.
This was referenced Mar 29, 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
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.
Summary
keywordsandsearch_textusage fromui_artifactsrepository queries and row typesTesting
npm run typechecknpm run test -- packages/worker/src/app/saved-ui-hosted-html.node.test.ts packages/worker/src/mcp/tools/search.node.test.ts packages/worker/src/mcp/capabilities/unified-search.workers.test.ts packages/worker/src/mcp/skills/infer-codemode-capabilities.node.test.tsnpm run test:mcp -- packages/worker/src/mcp/mcp-server.mcp-e2e.test.ts -t "mcp server executes ui_save_app via execute tool|mcp server saves app, search returns app hit, and open_generated_ui supports app_id|mcp server supports parameterized saved apps with resolved runtime params|generated ui sessions support secret storage, execute-time resolution, and scoped search visibility|mcp server deletes saved ui app artifacts|mcp server updates saved ui app artifacts"Summary by CodeRabbit