Conversation
ssdeanx
commented
Dec 10, 2025
ssdeanx
commented
- Refactored indexDocuments and queryRAG functions to use listWorkflows and createRun for better performance.
- Updated getFileContent to ensure size is treated as a number.
- Removed unused RequestContext import in jwt-auth.tool.ts.
- Simplified context handling in serpapi-search and serpapi-shopping tools.
- Changed input parameter naming from 'input' to 'inputData' in multiple tools for consistency.
- Enhanced error handling in repo-ingestion-workflow to include ZodError validation.
- Added new schemas for analysis output and report output in financial-report-workflow.
- Refactored indexDocuments and queryRAG functions to use listWorkflows and createRun for better performance. - Updated getFileContent to ensure size is treated as a number. - Removed unused RequestContext import in jwt-auth.tool.ts. - Simplified context handling in serpapi-search and serpapi-shopping tools. - Changed input parameter naming from 'input' to 'inputData' in multiple tools for consistency. - Enhanced error handling in repo-ingestion-workflow to include ZodError validation. - Added new schemas for analysis output and report output in financial-report-workflow.
|
Keep this PR in a mergeable state → Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
🤖 Hi @ssdeanx, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR updates Mastra API method calls in the CLI (getWorkflows → listWorkflows, createRunAsync → createRun), standardizes parameter naming in shopping tools (input → inputData), enhances error handling in the repo-ingestion workflow with ZodError detection, restructures schema declarations in the financial-report workflow, refactors context access patterns in search tools, and applies minor type refinements and import optimizations across tool files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
Summary of ChangesHello @ssdeanx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the robustness, consistency, and functionality of various tools and workflows within the system. It streamlines workflow execution, standardizes API input parameters for better maintainability, and significantly improves error handling with Zod validation. Additionally, it refines data schemas for financial reporting, ensuring more structured and comprehensive output. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
🤖 I'm sorry @ssdeanx, but I was unable to process your request. Please see the logs for more details. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a series of refactorings and improvements across various workflows and tools. Key changes include updating workflow creation calls, renaming input parameters for consistency, and significantly enhancing error handling, particularly by adding validation for ZodError. The removal of unused imports and simplification of context handling are also welcome cleanups.
My review focuses on the robustness of the new error handling and type safety. While the error handling for mdocumentChunker is a great improvement, I've pointed out a couple of areas where type assertions (as) could be replaced with safer type guards to prevent potential runtime errors and improve the clarity of error messages. These suggestions aim to make the code even more robust and maintainable.
| const successResult = result as { success: boolean; content?: string; encoding?: string; sha?: string; size?: number; error?: string }; | ||
| if (!successResult.success || !successResult.content) { | ||
| throw new Error(`Failed to fetch file from GitHub: ${successResult.error}`); | ||
| } | ||
| content = result.content; | ||
| content = successResult.content; |
There was a problem hiding this comment.
Using a type assertion with as can be unsafe if the result object's shape is not what's expected at runtime. This can also lead to incorrect error handling. For example, if success is true but content is missing, successResult.error will be undefined, resulting in an unhelpful error message.
A more robust approach is to use type guards to safely access properties. This avoids potential runtime errors and is consistent with the safer error handling pattern used for mdocumentChunker.execute later in this file.
| const successResult = result as { success: boolean; content?: string; encoding?: string; sha?: string; size?: number; error?: string }; | |
| if (!successResult.success || !successResult.content) { | |
| throw new Error(`Failed to fetch file from GitHub: ${successResult.error}`); | |
| } | |
| content = result.content; | |
| content = successResult.content; | |
| if (result && 'success' in result && result.success && typeof result.content === 'string') { | |
| content = result.content; | |
| } else { | |
| const errorMessage = (result && 'error' in result && typeof result.error === 'string') ? result.error : 'Unknown error'; | |
| throw new Error(`Failed to fetch file from GitHub: ${errorMessage}`); | |
| } |
| span.setAttributes({ | ||
| 'tool.output.success': true, | ||
| 'tool.output.size': data.size | ||
| 'tool.output.size': data.size as number |
There was a problem hiding this comment.
Using as number is an unsafe type assertion. If data.size is not a number at runtime, this could cause issues with telemetry. Additionally, the tool's output schema defines size as optional (z.number().optional()), but casting to number doesn't account for cases where size might be missing. It would be safer to validate the type of data.size before using it, for example: const size = typeof data.size === 'number' ? data.size : undefined; and then using size conditionally.
Greptile OverviewGreptile SummaryThis PR refactors workflow and tool code for improved consistency, type safety, and maintainability across the Mastra framework. Key Changes:
Minor Issue:
Confidence Score: 5/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant CLI as CLI (index.ts)
participant Mastra as Mastra Framework
participant Workflow as Workflow Instance
participant Tools as Tools/Services
Note over CLI,Tools: Workflow Invocation Pattern (Updated)
CLI->>Mastra: listWorkflows()['governed-rag-index']
Mastra-->>CLI: workflow instance
CLI->>Workflow: createRun()
Workflow-->>CLI: run instance
CLI->>Workflow: run.start({ inputData })
Note over Workflow,Tools: Tool Execution Pattern (Standardized)
Workflow->>Tools: tool.execute(inputData, context)
alt GitHub File Fetch
Tools->>Tools: getFileContent.execute(params, context)
Tools->>Tools: Check result type (ZodError?)
alt Success
Tools-->>Workflow: { success: true, content }
else ZodError
Tools-->>Workflow: throw Error(validation message)
end
end
alt Document Chunking
Tools->>Tools: mdocumentChunker.execute(inputData, context)
Tools->>Tools: Validate with enhanced error handling
alt Success
Tools-->>Workflow: { success: true, chunkCount }
else Error
Tools-->>Workflow: throw appropriate Error
end
end
Workflow-->>CLI: execution result
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (result && typeof result === 'object' && 'success' in result && result.success) { | ||
| processedFiles++; | ||
| totalChunks += result.chunkCount; | ||
| } | ||
| else if (result instanceof ZodError) { | ||
| throw new Error(result.message); | ||
| } | ||
| else if (result instanceof Error) { | ||
| throw result; | ||
| } else if (result && typeof result === 'object' && 'error' in result) { | ||
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | ||
| } else { | ||
| throw new Error('Unknown error in chunking'); | ||
| } |
There was a problem hiding this comment.
style: Inconsistent indentation in error handling block (excessive leading whitespace)
| if (result && typeof result === 'object' && 'success' in result && result.success) { | |
| processedFiles++; | |
| totalChunks += result.chunkCount; | |
| } | |
| else if (result instanceof ZodError) { | |
| throw new Error(result.message); | |
| } | |
| else if (result instanceof Error) { | |
| throw result; | |
| } else if (result && typeof result === 'object' && 'error' in result) { | |
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | |
| } else { | |
| throw new Error('Unknown error in chunking'); | |
| } | |
| if (result && typeof result === 'object' && 'success' in result && result.success) { | |
| processedFiles++; | |
| totalChunks += result.chunkCount; | |
| } | |
| else if (result instanceof ZodError) { | |
| throw new Error(result.message); | |
| } | |
| else if (result instanceof Error) { | |
| throw result; | |
| } else if (result && typeof result === 'object' && 'error' in result) { | |
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | |
| } else { | |
| throw new Error('Unknown error in chunking'); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mastra/workflows/repo-ingestion-workflow.ts
Line: 189:202
Comment:
**style:** Inconsistent indentation in error handling block (excessive leading whitespace)
```suggestion
if (result && typeof result === 'object' && 'success' in result && result.success) {
processedFiles++;
totalChunks += result.chunkCount;
}
else if (result instanceof ZodError) {
throw new Error(result.message);
}
else if (result instanceof Error) {
throw result;
} else if (result && typeof result === 'object' && 'error' in result) {
throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking');
} else {
throw new Error('Unknown error in chunking');
}
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/mastra/workflows/repo-ingestion-workflow.ts (1)
11-15: Add TSDoc comments for exported types.Per coding guidelines, exported interfaces and types should have TSDoc comments explaining their purpose and usage context.
+/** + * User subscription tier determining ingestion limits. + */ export type UserTier = 'free' | 'pro' | 'enterprise'; +/** + * Runtime context shape for repository ingestion workflow. + * @property user-tier - The user's subscription tier affecting file scan limits + */ export interface IngestionRuntimeContext { + /** User subscription tier - determines maximum files that can be scanned */ 'user-tier': UserTier; }src/mastra/tools/serpapi-search.tool.ts (1)
89-188: googleSearchTool: undefinederrorMessagein catch blockIn the
catchblock you computeerror instanceof Error ? error.message : String(error)but never assign it, then useerrorMessagemultiple times. This will fail type‑checking (and at runtime if it slips through).Apply something like this:
- } catch (error) { - error instanceof Error ? error.message : String(error) + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : String(error); await writer?.custom({ type: 'data-tool-progress', data: { message: `❌ Search failed: ${errorMessage}` } }); searchSpan.recordException(new Error(errorMessage)); searchSpan.setStatus({ code: SpanStatusCode.ERROR, message: errorMessage }); searchSpan.end(); log.error('Google search failed', { query: input.query, error: errorMessage, }) throw new Error(`Google search failed: ${errorMessage}`) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
src/cli/index.ts(2 hunks)src/mastra/tools/code-chunking.ts(1 hunks)src/mastra/tools/github.ts(1 hunks)src/mastra/tools/jwt-auth.tool.ts(0 hunks)src/mastra/tools/serpapi-search.tool.ts(2 hunks)src/mastra/tools/serpapi-shopping.tool.ts(12 hunks)src/mastra/workflows/financial-report-workflow.ts(1 hunks)src/mastra/workflows/repo-ingestion-workflow.ts(6 hunks)
💤 Files with no reviewable changes (1)
- src/mastra/tools/jwt-auth.tool.ts
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/next-js.instructions.md)
**/*.{js,jsx,ts,tsx}: Usenext/dynamicfor dynamic imports to load components only when needed, improving initial load time.
Usenext/imagecomponent for automatic image optimization, including lazy loading and responsive images.
Use React.memo to prevent unnecessary re-renders of components.
Use the<Link prefetch>tag to prefetch pages that are likely to be visited.
Use getServerSideProps, getStaticProps, or server components for fetching data on the server-side.
Use SWR or React Query for client-side data fetching and caching.
Use CSS Modules, Styled Components, or Tailwind CSS for component-level styling. Prefer Tailwind CSS for rapid development.
Use React Context, Zustand, Jotai, or Recoil for managing global state. Avoid Redux unless necessary.
Usereact-hook-formfor managing forms and validation.
Only fetch the data that is needed by the component to avoid over-fetching.
Avoid long-running synchronous operations in the main thread to prevent blocking.
Always usesetStateor hooks to update state instead of mutating state directly.
Include a complete dependency array inuseEffecthooks to prevent unexpected behavior.
Avoid writing server-side code in client components to prevent exposing secrets or causing unexpected behavior.
Usetry...catchblocks for handling errors in asynchronous operations.
Implement error boundary components usinggetDerivedStateFromErrororcomponentDidCatchlifecycle methods.
Sanitize user input to prevent Cross-Site Scripting (XSS) attacks. Be especially careful when rendering HTML directly from user input.
Store authentication tokens in HTTP-only cookies or local storage securely.
Implement role-based access control to restrict access to sensitive resources.
Clean up event listeners and timers inuseEffecthooks to avoid memory leaks.
Only update state when necessary to reduce the number of re-renders and improve performance.
Use immutable data structures and avoid mutating data directly to prevent unexpected...
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/cli/index.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/instructions/next-js.instructions.md)
Use parameterized queries or an ORM to prevent SQL injection attacks.
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/cli/index.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
src/mastra/tools/**/*.ts
📄 CodeRabbit inference engine (src/mastra/AGENTS.md)
src/mastra/tools/**/*.ts: Use thecreateToolpattern with Zod schemas when adding new tools undersrc/mastra/tools
Use explicit Zod schemas for every tool input/output
src/mastra/tools/**/*.ts: Use Zod schemas for strict validation of tool inputs and outputs in all Mastra tools
Implement tools using the createTool({ id, inputSchema, outputSchema, execute }) pattern
Organize tools into categories: Financial (Polygon, Finnhub, AlphaVantage), Research (SerpAPI, ArXiv), Data (CSV, JSON), RAG (chunking, embeddings)
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/code-chunking.ts
src/mastra/{tools,workflows}/**/*.ts
📄 CodeRabbit inference engine (src/mastra/AGENTS.md)
Use
RuntimeContextto enforce access control in tools and workflows
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
src/mastra/tools/**/*.tool.ts
📄 CodeRabbit inference engine (src/mastra/tools/AGENTS.md)
src/mastra/tools/**/*.tool.ts: Define Zod schema for inputs and outputs when creating tools
Tool IDs should follow the 'namespace:toolName' format (e.g., 'alpha-vantage:stockTool')
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/serpapi-shopping.tool.ts
src/mastra/tools/**/*.{ts,tsx}
📄 CodeRabbit inference engine (src/mastra/tools/AGENTS.md)
src/mastra/tools/**/*.{ts,tsx}: All tools must include comprehensive error handling with try-catch blocks, clear error messages, and retry logic for API calls
Sanitize inputs, mask secrets in logs, and implement rate-limiting in tools
Implement caching, stream large data, and set timeouts for external API calls in tools
Add Arize spans on all tool execute functions for observability
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/tools/code-chunking.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never commit API keys or secrets to the repository; use maskSensitiveMessageData() helper from src/mastra/config/pg-storage.ts when logging
**/*.{ts,tsx}: Document interface and type definitions with TSDoc comments explaining their purpose and usage context
Document interface properties with /** */ comments explaining each field's purpose and constraints
Document generic type parameters with @template tags explaining what each type parameter represents
Use type guards with comments explaining the runtime validation logic being performed
Document advanced/complex TypeScript types with explanatory comments about their purpose and use cases
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/cli/index.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Run eslint with --max-warnings=0 on src/**/*.{ts,tsx} to enforce linting standards
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/cli/index.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
**/*.{ts,tsx,js,jsx,py,java,cs,rb,go,rs,cpp,c,h,hpp,swift,kotlin,php,scala,clj,groovy,lua,sh,bash}
📄 CodeRabbit inference engine (.github/instructions/self-explanatory-code-commenting.instructions.md)
**/*.{ts,tsx,js,jsx,py,java,cs,rb,go,rs,cpp,c,h,hpp,swift,kotlin,php,scala,clj,groovy,lua,sh,bash}: Write code that speaks for itself. Comment only when necessary to explain WHY, not WHAT. Avoid obvious comments that state what the code literally does.
Avoid redundant comments that simply repeat what the code is doing
Keep comments accurate and up-to-date with code changes. Remove or update outdated comments that no longer match the implementation.
Write comments for complex business logic that explain the WHY behind specific calculations or business rules
Document non-obvious algorithms with comments explaining the algorithm choice and its reasoning
Add comments explaining what regex patterns match, especially for complex patterns
Document API constraints, rate limits, gotchas, and external dependencies with explanatory comments
Avoid commenting out dead code. Use version control instead of maintaining commented code blocks.
Do not maintain code change history or modification logs as comments. Rely on git history and commit messages instead.
Avoid decorative divider comments (e.g., lines of equals signs or asterisks) for section separation
Ensure comments are placed appropriately above or adjacent to the code they describe
Write comments using proper grammar, spelling, and professional language
Prefer self-documenting code with clear variable/function names over adding comments to explain unclear code
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/cli/index.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/instructions/self-explanatory-code-commenting.instructions.md)
**/*.{ts,tsx,js,jsx}: Document public APIs with TSDoc/JSDoc comments including parameter descriptions, return types, examples, and thrown exceptions
Add TSDoc comments to configuration constants and environment variables explaining their source, reasoning, or constraints
Use TSDoc annotation tags (TODO, FIXME, HACK, NOTE, WARNING, PERF, SECURITY, BUG, REFACTOR, DEPRECATED) to mark special comments
Include file headers with @fileoverview, @author, @copyright, and @license tags to document file purpose and ownership
Document function parameters with @param tags, return values with @returns tags, and exceptions with @throws tags in TSDoc comments
Use @see tags in TSDoc comments to reference related functions, methods, or documentation
Include @example tags in public API documentation with code examples showing typical usage
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/cli/index.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
src/mastra/**/*
📄 CodeRabbit inference engine (src/AGENTS.md)
mastramodules can import fromutils, but must not import fromapporcli(excepttypes)
Files:
src/mastra/tools/serpapi-search.tool.tssrc/mastra/tools/github.tssrc/mastra/tools/serpapi-shopping.tool.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/tools/code-chunking.tssrc/mastra/workflows/financial-report-workflow.ts
src/cli/**/*
📄 CodeRabbit inference engine (src/AGENTS.md)
climodule can import frommastraandutilsmodules, but must not import fromapp
Files:
src/cli/index.ts
src/mastra/workflows/**/*.ts
📄 CodeRabbit inference engine (src/mastra/AGENTS.md)
Add new workflow definitions under
src/mastra/workflowsto orchestrate multi-step flowsUse Mastra DSL for multi-step workflow definitions in src/mastra/workflows
Files:
src/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/financial-report-workflow.ts
🧬 Code graph analysis (3)
src/mastra/tools/serpapi-shopping.tool.ts (2)
src/mastra/tools/serpapi-config.ts (1)
validateSerpApiKey(35-42)src/mastra/config/logger.ts (1)
log(14-18)
src/mastra/workflows/repo-ingestion-workflow.ts (1)
src/mastra/tools/github.ts (1)
getFileContent(523-597)
src/mastra/workflows/financial-report-workflow.ts (1)
src/mastra/config/agent-schemas.ts (1)
reportOutputSchema(136-138)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: Codacy Security Scan
🔇 Additional comments (12)
src/mastra/workflows/repo-ingestion-workflow.ts (3)
6-6: LGTM!The
ZodErrorimport is correctly added to support the enhanced error handling below.
51-52: LGTM!Good separation of
limitinto a mutable variable to support the tier-based limiting logic that follows.
88-89: LGTM!The explicit
Boolean(r)check is functionally equivalent to the implicit truthy check but makes the intent clearer.src/mastra/workflows/financial-report-workflow.ts (1)
116-134: Confirm downstream compatibility for the tightenedfinalReportSchema.datashape
finalReportSchema.data.stocksand.analysisare now defined inline with explicit Zod object shapes (includingrecommendations), instead of relying on more loosely referenced shapes. This matches whatgenerateReportStepactually returns, so from this workflow’s perspective the schema is consistent and more precise.However, because
financialReportWorkflowusesfinalReportSchemaas itsoutputSchema, this becomes the public contract for any external consumers. Please double-check that:
- Any callers deserializing or validating the workflow output are expecting
data.stocksanddata.analysisin exactly this form, and- There isn’t a separate, older schema (e.g., in shared config) that still describes a different
datastructure.If everything downstream already assumes this shape, then this change looks good and makes the API clearer.
src/mastra/tools/github.ts (1)
571-574: Telemetry size attribute cast looks goodCasting
data.sizetonumberfor the span attribute aligns with the output schema and avoids type‑mismatch issues with the OpenTelemetry API.src/mastra/tools/code-chunking.ts (1)
36-42:ext ?? ''change is fine; please double‑check execute signatureUsing
ext ?? ''instead ofext || ''is a safe, slightly more precise nullish check for the extension. Separately, this tool’sexecute: async ({ context }) => { ... }shape differs from the(inputData, context)pattern used in your other tools; please confirm this is intentional and matches your currentcreateToolAPI expectations.src/mastra/tools/serpapi-search.tool.ts (1)
240-309: Optional writer access in googleAiOverviewTool is appropriateSwitching to
const writer = context?.writer;and usingwriter?.customkeeps progress reporting while safely handling cases where no writer is provided.src/cli/index.ts (1)
95-103: Confirmmastra.listWorkflows()API shape andcreateRunavailabilityThe switch to
mastra.listWorkflows()andworkflow.createRun()keeps the control flow intact, but it assumes:
mastra.listWorkflows()exists on yourmastrainstance and returns a synchronousRecord<string, Workflow>.workflow.createRun()is the correct (non‑deprecated) entrypoint here.Please confirm these assumptions against the Mastra version wired into
../mastra/indexand adjust (e.g., addawaitor fall back togetWorkflows()) if the runtime API differs.Also applies to: 169-173
src/mastra/tools/serpapi-shopping.tool.ts (4)
41-138: amazonSearchTool: input → inputData rename is consistentThe execute signature and all references (params, logs, span attributes) have been updated to
inputDataconsistently; behavior should remain unchanged.
164-245: walmartSearchTool: parameter rename is clean and non‑breaking
inputDatais used consistently for query, sorting, price filters, logging, and tracing, with no apparent regressions.
273-359: ebaySearchTool: updated input naming remains correctAll usages (query, condition mapping, Buy‑It‑Now flag, sort mapping, logging, and spans) now correctly reference
inputDataand preserve the previous behavior.
385-459: homeDepotSearchTool: renamed input parameter wired through correctlyThe
inputDatarename is applied consistently to query, sorting, in‑stock filter, logging, and tracing; no functional changes beyond naming.
| const analysisOutputSchema = z.object({ | ||
| recommendations: z.array(z.object({ | ||
| symbol: z.string(), | ||
| action: z.enum(['buy', 'hold', 'sell']), | ||
| reason: z.string(), | ||
| })), | ||
| }); | ||
|
|
||
| const reportOutputSchema = z.object({ | ||
| summary: z.string(), | ||
| report: z.string(), | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Wire new analysisOutputSchema / reportOutputSchema into agent calls or remove/export them
analysisOutputSchema and reportOutputSchema are currently defined but never used in this file. The agent calls in analyzeDataStep and generateReportStep still inline their own Zod schemas for recommendations and { summary, report }, which duplicates these new definitions and risks the shapes drifting over time. There is also an existing reportOutputSchema exported from src/mastra/config/agent-schemas.ts with a different shape ({ report: string }), so having another, slightly different reportOutputSchema here is easy to misinterpret.
Consider one of:
- Reusing these schemas in the agent
.streamcalls (e.g.,output: analysisOutputSchema/output: reportOutputSchema) and exporting them if callers are meant to depend on them, or - Consolidating on the canonical schema in
agent-schemas.ts(updating it if necessary), or - Removing these locals if they’re not intended to be part of the public surface.
| interface GitTreeItem { | ||
| path: string; | ||
| mode?: string; | ||
| type: string; | ||
| sha?: string; | ||
| size?: number; | ||
| url?: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting shared interface.
Defining GitTreeItem locally works, but if this type is also used in github.ts, consider exporting it from there to avoid drift between the actual API response type and this local definition.
🤖 Prompt for AI Agents
In src/mastra/workflows/repo-ingestion-workflow.ts around lines 73 to 80, the
local GitTreeItem interface duplicates a type also used in github.ts; extract
and export the shared GitTreeItem interface (or a suitably named common type)
from github.ts and import it here instead of redefining it to ensure a single
source of truth and avoid drift between definitions; update the import
statements and remove the local declaration so all code references the exported
type.
| if (result instanceof ZodError) { | ||
| throw new Error(`Validation error fetching file: ${result.message}`); | ||
| } |
There was a problem hiding this comment.
result instanceof ZodError check is dead code.
Tool execute methods return the outputSchema type on success or throw on failure. They never return a ZodError instance directly. If schema validation fails, Zod throws a ZodError that would be caught by the outer try-catch block on line 204, not returned as the result.
This check will never evaluate to true.
Remove the dead code:
- if (result instanceof ZodError) {
- throw new Error(`Validation error fetching file: ${result.message}`);
- }
-If you want to handle ZodError specifically, catch it in the outer catch block:
} catch (error) {
if (error instanceof ZodError) {
// Handle validation error specifically
}
// ... rest of error handling
}🤖 Prompt for AI Agents
In src/mastra/workflows/repo-ingestion-workflow.ts around lines 159 to 161,
remove the dead check that tests "if (result instanceof ZodError)" because the
execute methods never return ZodError — they throw it; instead, delete that
branch and, if you need to handle validation errors specifically, add an
instanceof ZodError branch in the outer catch block (around the existing catch
at ~line 204) to handle ZodError cases before the general error handling.
| if (result && typeof result === 'object' && 'success' in result && result.success) { | ||
| processedFiles++; | ||
| totalChunks += result.chunkCount; | ||
| } | ||
| else if (result instanceof ZodError) { | ||
| throw new Error(result.message); | ||
| } | ||
| else if (result instanceof Error) { | ||
| throw result; | ||
| } else if (result && typeof result === 'object' && 'error' in result) { | ||
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | ||
| } else { | ||
| throw new Error('Unknown error in chunking'); | ||
| } |
There was a problem hiding this comment.
Dead code and inconsistent indentation in error handling.
Two issues:
-
Dead code:
result instanceof ZodError(line 193) andresult instanceof Error(line 196) will never betruebecause toolexecutemethods return objects matching the output schema, not Error instances. -
Inconsistent indentation: The if-else branches have irregular spacing that harms readability.
Apply this diff to remove dead code and fix indentation:
- if (result && typeof result === 'object' && 'success' in result && result.success) {
- processedFiles++;
- totalChunks += result.chunkCount;
- }
- else if (result instanceof ZodError) {
- throw new Error(result.message);
- }
- else if (result instanceof Error) {
- throw result;
- } else if (result && typeof result === 'object' && 'error' in result) {
- throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking');
- } else {
- throw new Error('Unknown error in chunking');
- }
+ if (result && typeof result === 'object' && 'success' in result && result.success) {
+ processedFiles++;
+ totalChunks += result.chunkCount;
+ } else if (result && typeof result === 'object' && 'error' in result) {
+ throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking');
+ } else {
+ throw new Error('Unknown error in chunking');
+ }📝 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.
| if (result && typeof result === 'object' && 'success' in result && result.success) { | |
| processedFiles++; | |
| totalChunks += result.chunkCount; | |
| } | |
| else if (result instanceof ZodError) { | |
| throw new Error(result.message); | |
| } | |
| else if (result instanceof Error) { | |
| throw result; | |
| } else if (result && typeof result === 'object' && 'error' in result) { | |
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | |
| } else { | |
| throw new Error('Unknown error in chunking'); | |
| } | |
| if (result && typeof result === 'object' && 'success' in result && result.success) { | |
| processedFiles++; | |
| totalChunks += result.chunkCount; | |
| } else if (result && typeof result === 'object' && 'error' in result) { | |
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | |
| } else { | |
| throw new Error('Unknown error in chunking'); | |
| } |
🤖 Prompt for AI Agents
In src/mastra/workflows/repo-ingestion-workflow.ts around lines 189 to 202,
remove the dead branches that check for result instanceof ZodError and result
instanceof Error (these never occur because execute returns schema-matching
objects) and reformat the remaining if/else chain to consistent indentation and
spacing; keep the successful branch that increments processedFiles and
totalChunks, then handle the case where result has an error property by throwing
a new Error with the string or a default message, and finally throw a generic
'Unknown error in chunking' Error for any other unexpected result, ensuring all
branches are uniformly indented.
There was a problem hiding this comment.
Pull request overview
This PR refactors workflows and tools to improve code consistency and functionality. The main changes include standardizing parameter naming (input → inputData), simplifying context handling in tools, updating workflow APIs (getWorkflows → listWorkflows, createRunAsync → createRun), and attempting to enhance error handling with ZodError checks. The PR also adds new schema definitions and removes unused imports.
Key changes:
- Standardized tool execute parameter naming from
inputtoinputDataacross serpapi-shopping tools - Updated workflow execution API calls in CLI from deprecated methods to current ones
- Added ZodError handling in repo-ingestion-workflow (though implementation has issues)
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mastra/workflows/repo-ingestion-workflow.ts | Enhanced error handling with ZodError checks, updated tool invocation patterns, changed type to interface, and restructured variable destructuring |
| src/mastra/workflows/financial-report-workflow.ts | Added new schema definitions (analysisOutputSchema, reportOutputSchema) and inlined previously reused schema shapes in finalReportSchema |
| src/mastra/tools/serpapi-shopping.tool.ts | Renamed execute function parameter from input to inputData for consistency across all shopping tools (Amazon, Walmart, eBay, Home Depot) |
| src/mastra/tools/serpapi-search.tool.ts | Simplified context destructuring by extracting only the writer property instead of full object destructuring |
| src/mastra/tools/jwt-auth.tool.ts | Removed unused RequestContext import |
| src/mastra/tools/github.ts | Added explicit type assertion for data.size to ensure it's treated as a number |
| src/mastra/tools/code-chunking.ts | Changed from logical OR operator to nullish coalescing operator for safer default value handling |
| src/cli/index.ts | Updated workflow API calls from getWorkflows() to listWorkflows() and createRunAsync() to createRun() |
Comments suppressed due to low confidence (2)
src/mastra/workflows/financial-report-workflow.ts:97
- Unused variable analysisOutputSchema.
const analysisOutputSchema = z.object({
src/mastra/workflows/financial-report-workflow.ts:105
- Unused variable reportOutputSchema.
const reportOutputSchema = z.object({
| processedFiles++; | ||
| totalChunks += result.chunkCount; | ||
| } | ||
| else if (result instanceof ZodError) { | ||
| throw new Error(result.message); | ||
| } | ||
| else if (result instanceof Error) { | ||
| throw result; | ||
| } else if (result && typeof result === 'object' && 'error' in result) { | ||
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | ||
| } else { | ||
| throw new Error('Unknown error in chunking'); | ||
| } |
There was a problem hiding this comment.
The indentation is inconsistent in this error handling block. Lines 190-202 have excessive leading whitespace that doesn't align with the surrounding code structure. This makes the code harder to read and maintain.
Recommendation: Remove the extra indentation to align with the if/else chain pattern at the same level.
| processedFiles++; | |
| totalChunks += result.chunkCount; | |
| } | |
| else if (result instanceof ZodError) { | |
| throw new Error(result.message); | |
| } | |
| else if (result instanceof Error) { | |
| throw result; | |
| } else if (result && typeof result === 'object' && 'error' in result) { | |
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | |
| } else { | |
| throw new Error('Unknown error in chunking'); | |
| } | |
| processedFiles++; | |
| totalChunks += result.chunkCount; | |
| } | |
| else if (result instanceof ZodError) { | |
| throw new Error(result.message); | |
| } | |
| else if (result instanceof Error) { | |
| throw result; | |
| } else if (result && typeof result === 'object' && 'error' in result) { | |
| throw new Error(typeof result.error === 'string' ? result.error : 'Unknown error in chunking'); | |
| } else { | |
| throw new Error('Unknown error in chunking'); | |
| } |
| if (result instanceof ZodError) { | ||
| throw new Error(`Validation error fetching file: ${result.message}`); | ||
| } | ||
|
|
There was a problem hiding this comment.
The result instanceof ZodError check is incorrect here. The getFileContent tool returns an object matching its output schema ({ success: boolean; content?: string; ... }), not a ZodError instance. Zod validation errors would be thrown, not returned. This error handling will never be triggered and creates confusion about the tool's error model.
Recommendation: Remove this check and rely on the standard success field check on line 164, or wrap the execute call in a try-catch if you need to handle validation errors.
| if (result instanceof ZodError) { | |
| throw new Error(`Validation error fetching file: ${result.message}`); | |
| } |
| else if (result instanceof ZodError) { | ||
| throw new Error(result.message); | ||
| } |
There was a problem hiding this comment.
Similar to the getFileContent issue, the result instanceof ZodError check is incorrect. The mdocumentChunker tool returns an object matching its output schema, not a ZodError instance. Zod validation errors would be thrown during schema validation, not returned as the result. This condition will never be true and creates confusion about error handling patterns.
Recommendation: Remove this check or wrap the execute call in a try-catch to handle thrown validation errors.
| else if (result instanceof ZodError) { | |
| throw new Error(result.message); | |
| } |
| stocks: z.array(z.object({ | ||
| symbol: z.string(), | ||
| price: priceDataSchema, | ||
| metrics: companyMetricsSchema, | ||
| sentiment: newsSentimentSchema, | ||
| })), | ||
| analysis: z.object({ | ||
| topPerformers: z.array(z.string()), | ||
| worstPerformers: z.array(z.string()), | ||
| bullishStocks: z.array(z.string()), | ||
| bearishStocks: z.array(z.string()), | ||
| averageChange: z.number(), | ||
| marketTrend: z.enum(['bullish', 'bearish', 'neutral']), | ||
| recommendations: z.array(z.object({ | ||
| symbol: z.string(), | ||
| action: z.enum(['buy', 'hold', 'sell']), | ||
| reason: z.string(), | ||
| })), | ||
| }), |
There was a problem hiding this comment.
The schema definition is duplicated. Lines 116-134 redefine the exact same structure that was already defined in analysisResultSchema.shape.stocks and analysisResultSchema.shape.analysis (lines 73-93). This creates maintenance overhead - if the structure needs to change, it must be updated in multiple places.
Recommendation: Reuse the existing schema definitions: stocks: analysisResultSchema.shape.stocks and analysis: analysisResultSchema.shape.analysis as it was before this change.
| stocks: z.array(z.object({ | |
| symbol: z.string(), | |
| price: priceDataSchema, | |
| metrics: companyMetricsSchema, | |
| sentiment: newsSentimentSchema, | |
| })), | |
| analysis: z.object({ | |
| topPerformers: z.array(z.string()), | |
| worstPerformers: z.array(z.string()), | |
| bullishStocks: z.array(z.string()), | |
| bearishStocks: z.array(z.string()), | |
| averageChange: z.number(), | |
| marketTrend: z.enum(['bullish', 'bearish', 'neutral']), | |
| recommendations: z.array(z.object({ | |
| symbol: z.string(), | |
| action: z.enum(['buy', 'hold', 'sell']), | |
| reason: z.string(), | |
| })), | |
| }), | |
| stocks: analysisResultSchema.shape.stocks, | |
| analysis: analysisResultSchema.shape.analysis, |
| interface GitTreeItem { | ||
| path: string; | ||
| mode?: string; | ||
| type: string; | ||
| sha?: string; | ||
| size?: number; | ||
| url?: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
[nitpick] Changing from type to interface for a simple object type is a matter of style, but type is generally preferred for simple object shapes in TypeScript, especially when it's a local definition used only once. The interface keyword is typically reserved for cases where you need declaration merging or are defining an API contract that may be extended. For local, single-use type definitions like this, type is more conventional.
Note: This is a minor style preference and doesn't affect functionality.
| @@ -1,5 +1,4 @@ | |||
| import { trace } from "@opentelemetry/api"; | |||
There was a problem hiding this comment.
'trace' is defined but never used.
| import { trace } from "@opentelemetry/api"; |
| const analysisOutputSchema = z.object({ | ||
| recommendations: z.array(z.object({ | ||
| symbol: z.string(), | ||
| action: z.enum(['buy', 'hold', 'sell']), | ||
| reason: z.string(), | ||
| })), | ||
| }); | ||
|
|
There was a problem hiding this comment.
'analysisOutputSchema' is assigned a value but never used.
| const analysisOutputSchema = z.object({ | |
| recommendations: z.array(z.object({ | |
| symbol: z.string(), | |
| action: z.enum(['buy', 'hold', 'sell']), | |
| reason: z.string(), | |
| })), | |
| }); |
| const reportOutputSchema = z.object({ | ||
| summary: z.string(), | ||
| report: z.string(), | ||
| }); | ||
|
|
There was a problem hiding this comment.
'reportOutputSchema' is assigned a value but never used.
| const reportOutputSchema = z.object({ | |
| summary: z.string(), | |
| report: z.string(), | |
| }); |
| @@ -85,7 +86,7 @@ const scanStep = createStep({ | |||
|
|
|||
| // Type guard to narrow to the success shape | |||
| const isScanSuccess = (r: any): r is { success: boolean; tree?: GitTreeItem[]; error?: string } => | |||
There was a problem hiding this comment.
Unexpected any. Specify a different type.
| const isScanSuccess = (r: any): r is { success: boolean; tree?: GitTreeItem[]; error?: string } => | |
| const isScanSuccess = (r: unknown): r is { success: boolean; tree?: GitTreeItem[]; error?: string } => |
| (Boolean(r)) && typeof r === 'object' && 'success' in r; | ||
|
|
||
| if (!isScanSuccess(scanResultRaw) || !scanResultRaw.success || !scanResultRaw.tree) { | ||
| throw new Error(`Failed to scan GitHub repo: ${(scanResultRaw as any)?.error ?? 'unknown error'}`); |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
| throw new Error(`Failed to scan GitHub repo: ${(scanResultRaw as any)?.error ?? 'unknown error'}`); | |
| throw new Error(`Failed to scan GitHub repo: ${typeof scanResultRaw === 'object' && scanResultRaw && 'error' in scanResultRaw ? (scanResultRaw as { error?: string }).error ?? 'unknown error' : 'unknown error'}`); |
