Add typed error response#88
Merged
wpak-ai merged 6 commits intoMay 15, 2026
Merged
Conversation
added 2 commits
May 15, 2026 07:13
📝 WalkthroughWalkthroughAdds a Zod-validated ChangesTyped Error Response System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #88 +/- ##
=======================================
Coverage ? 75.02%
=======================================
Files ? 33
Lines ? 1165
Branches ? 376
=======================================
Hits ? 874
Misses ? 291
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/types.ts (1)
86-91:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow
ListNamespacesResponseto success-only to match the migrated error handling.
ListNamespacesResponsestill definesstatus: 'success' | 'error'andmessage?: string, butlist_namespacesnow usesjsonErrorResponse()for failures, which returns aTextPayloadwithisError: truerather than aListNamespacesResponseobject. The type should be narrowed tostatus: 'success'only, matchingQueryResponseand reflecting the actual behavior of error responses in the MCP protocol.🤖 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 `@src/types.ts` around lines 86 - 91, Update the ListNamespacesResponse interface to represent only successful responses (matching QueryResponse and the MCP protocol): change status to the literal 'success' (remove the union with 'error'), drop the optional message field (and any other error-related properties), and ensure namespaces?: string[] and count?: number remain; this aligns ListNamespacesResponse with the fact that failed cases are returned by jsonErrorResponse() and not as ListNamespacesResponse objects (refer to ListNamespacesResponse and list_namespaces).
🧹 Nitpick comments (1)
src/server/tools/generate-urls-tool.test.ts (1)
21-21: ⚡ Quick winVerify the error message in addition to the code.
The test only checks
codebut does not verify the errormessage. According to the acceptance criteria, PINECONE_ERROR responses must include a descriptive message. Other PINECONE_ERROR tests in this PR verify both fields (e.g.,list-namespaces-tool.test.tsline 70,suggest-query-params-tool.test.tsline 110).✅ Recommended enhancement to verify message
- expect(assertToolError(raw).code).toBe('PINECONE_ERROR'); + const err = assertToolError(raw); + expect(err.code).toBe('PINECONE_ERROR'); + expect(err.message).toBe('Failed to generate URLs');🤖 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 `@src/server/tools/generate-urls-tool.test.ts` at line 21, The test currently only asserts the error code from assertToolError(raw); add an assertion that also verifies the error message (access assertToolError(raw).message) equals the expected descriptive PINECONE_ERROR message (or uses toMatch with the same text/regex used in the other PINECONE_ERROR tests such as the ones referenced for list-namespaces and suggest-query-params) so the test validates both code and message per acceptance criteria.
🤖 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/server/tools/guided-query-tool.ts`:
- Around line 100-116: Normalize and validate inputNamespace before deciding
error type: if inputNamespace is provided (inputNamespace !== undefined) trim it
and if the trimmed string is empty/only whitespace return a JSON validation
error for the namespace field (VALIDATION, field "namespace") instead of the
current backend/pinecone error; otherwise proceed to use the existing namespace
resolution (namespace = inputNamespace ?? ranked[0]?.namespace) and only return
the pinecone-style recoverable error when no routable namespace exists and no
explicit namespace was supplied. Reference symbols: inputNamespace, namespace,
ranked, jsonErrorResponse, and the pinecone-style error path currently using
pineconeToolError.
In `@src/server/tools/test-helpers.ts`:
- Around line 43-47: The helper assertToolError currently parses the payload
with parseToolJson and validates the body shape with toolErrorSchema but does
not enforce the envelope flag; update assertToolError to explicitly check that
the parsed raw value has isError === true (e.g. throw an error if raw?.isError
!== true) before calling toolErrorSchema.parse, or incorporate the isError
constraint into the validation step so that assertToolError always rejects
non-error envelopes; reference the function assertToolError, the parseToolJson
call, and toolErrorSchema when making the change.
---
Outside diff comments:
In `@src/types.ts`:
- Around line 86-91: Update the ListNamespacesResponse interface to represent
only successful responses (matching QueryResponse and the MCP protocol): change
status to the literal 'success' (remove the union with 'error'), drop the
optional message field (and any other error-related properties), and ensure
namespaces?: string[] and count?: number remain; this aligns
ListNamespacesResponse with the fact that failed cases are returned by
jsonErrorResponse() and not as ListNamespacesResponse objects (refer to
ListNamespacesResponse and list_namespaces).
---
Nitpick comments:
In `@src/server/tools/generate-urls-tool.test.ts`:
- Line 21: The test currently only asserts the error code from
assertToolError(raw); add an assertion that also verifies the error message
(access assertToolError(raw).message) equals the expected descriptive
PINECONE_ERROR message (or uses toMatch with the same text/regex used in the
other PINECONE_ERROR tests such as the ones referenced for list-namespaces and
suggest-query-params) so the test validates both code and message per acceptance
criteria.
🪄 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: 84e0bb6e-33a7-490b-a682-7c5743dde576
📒 Files selected for processing (28)
CHANGELOG.mdREADME.mdsrc/server.tssrc/server/metadata-filter.test.tssrc/server/metadata-filter.tssrc/server/tool-error.test.tssrc/server/tool-error.tssrc/server/tool-response.tssrc/server/tools/count-tool.test.tssrc/server/tools/count-tool.tssrc/server/tools/generate-urls-tool.test.tssrc/server/tools/generate-urls-tool.tssrc/server/tools/guided-query-tool.test.tssrc/server/tools/guided-query-tool.tssrc/server/tools/keyword-search-tool.test.tssrc/server/tools/keyword-search-tool.tssrc/server/tools/list-namespaces-tool.test.tssrc/server/tools/list-namespaces-tool.tssrc/server/tools/namespace-router-tool.test.tssrc/server/tools/namespace-router-tool.tssrc/server/tools/query-documents-tool.test.tssrc/server/tools/query-documents-tool.tssrc/server/tools/query-tool.test.tssrc/server/tools/query-tool.tssrc/server/tools/suggest-query-params-tool.test.tssrc/server/tools/suggest-query-params-tool.tssrc/server/tools/test-helpers.tssrc/types.ts
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/server/tools/test-helpers.ts`:
- Around line 34-38: parseToolJson was made error-only by checking
envelope.isError and throwing, which breaks success-path tests; revert
parseToolJson to be envelope-agnostic by removing the isError check so it simply
casts/returns the payload as a Record<string, unknown>, and move/enforce any
isError assertions into assertToolError (e.g., check isError: true inside
assertToolError instead). Update parseToolJson to not throw based on isError and
ensure assertToolError performs the error-specific validation.
🪄 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: fa756d10-970e-4e73-9047-d0dd64fce75c
📒 Files selected for processing (2)
src/server/tools/guided-query-tool.tssrc/server/tools/test-helpers.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/server/tools/guided-query-tool.ts
Collaborator
Author
|
@coderabbitai full review again |
|
✅ Actions performedFull review triggered. |
wpak-ai
approved these changes
May 15, 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.
Pull Request
Core Changes
Tools (all 9)
Validation: empty inputs, metadata filters → validationToolError with field; guided “namespace not in cache” → field: 'namespace'; empty index / no namespace → pineconeToolError with recoverable: true + suggestion (per plan comment).
Types
Package surface
Docs & tests
Related Issue
close #84
Summary by CodeRabbit
New Features
Changed
Documentation