Conversation
- Added `eslint-plugin-react-hooks` and `eslint-plugin-react-refresh` to package.json for improved linting support. - Refactored `document-processing-workflow.ts`: - Updated stage identifiers from "documentProcessingAgent" to "convert-pdf-to-markdown" for clarity. - Changed error handling to use "data-tool-progress" type instead of "data-workflow-step-error". - Refactored `repo-ingestion-workflow.ts`: - Introduced type guards for result validation to improve error handling. - Added `embeddingModel` and `embeddingBatchSize` parameters to the `mdocumentChunker.execute` call. - Refactored `research-synthesis-workflow.ts`: - Updated stage identifiers to be more descriptive (e.g., "research-topic-item", "synthesize-research"). - Changed error handling to use "data-tool-progress" type. - Refactored `stock-analysis-workflow.ts`: - Updated stage identifiers to be more descriptive (e.g., "fetch-stock-data", "get-company-news", "run-analysis", "generate-report"). - Changed error handling to use "data-tool-progress" type.
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
|
🤖 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. WalkthroughThe PR introduces ESLint plugin configurations for React Hooks and Refresh, adds a new utility function for generating thread links with UUIDs, and standardizes workflow progress messaging by replacing generic stage indicators with step-specific names and updating payload types to use data-tool-progress. It also adds embedding configuration parameters to document chunking. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 primarily focuses on improving code quality, consistency, and robustness across several workflow files and the ESLint configuration. It introduces new linting plugins for React, standardizes workflow stage naming for better clarity, and refines error reporting mechanisms. Additionally, the repository ingestion process gains more precise control over embedding generation and enhanced result validation. 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
|
Reviewer's GuideUpdates linting configuration and dependency set while standardizing workflow telemetry and error reporting across multiple workflows, and improves repo ingestion chunking result handling plus adds a small URL helper. Sequence diagram for repo ingestion chunking with result type guardssequenceDiagram
participant RepoIngestionWorkflow
participant MDocumentChunker
participant ZodValidator
participant ErrorHandler
RepoIngestionWorkflow->>MDocumentChunker: execute(documentContent, documentMetadata, chunkingStrategy, indexName, generateEmbeddings, embeddingModel, embeddingBatchSize, chunkSize, chunkOverlap, chunkSeparator, writer, requestContext)
MDocumentChunker-->>RepoIngestionWorkflow: result
alt result isSuccessResult
RepoIngestionWorkflow->>RepoIngestionWorkflow: increment processedFiles
RepoIngestionWorkflow->>RepoIngestionWorkflow: add result.chunkCount to totalChunks
else result is ZodError
RepoIngestionWorkflow->>ZodValidator: inspect result
ZodValidator-->>RepoIngestionWorkflow: validationErrorMessage
RepoIngestionWorkflow->>ErrorHandler: throw Error(validationErrorMessage)
else result is Error
RepoIngestionWorkflow->>ErrorHandler: throw result
else result isErrorObject
RepoIngestionWorkflow->>ErrorHandler: throw Error(errorStringOrUnknown)
else unknown result shape
RepoIngestionWorkflow->>ErrorHandler: throw Error(Unknown error in chunking)
end
Note over RepoIngestionWorkflow,ErrorHandler: Errors are caught by surrounding try catch and logged, preserving workflow behavior while improving type safety
Class diagram for updated repo ingestion and utilitiesclassDiagram
class RepoIngestionWorkflow {
+ingestStep(documentContent: string, filePath: string, githubRepo: boolean, strategy: string, writer: unknown, requestContext: unknown) Promise~void~
-processedFiles: number
-totalChunks: number
-isSuccessResult(result: unknown) boolean
-isErrorObject(result: unknown) boolean
}
class MDocumentChunker {
+execute(documentContent: string, documentMetadata: object, chunkingStrategy: string, indexName: string, generateEmbeddings: boolean, embeddingModel: string, embeddingBatchSize: number, chunkSize: number, chunkOverlap: number, chunkSeparator: string, writer: unknown, requestContext: unknown) Promise~ChunkResultUnion~
}
class ChunkResultSuccess {
+success: true
+chunkCount: number
}
class ChunkResultErrorObject {
+error: unknown
}
class ZodError {
+message: string
}
class Error {
+message: string
}
class Utils {
+cn(inputs: ClassValue[]) string
+newThreadLink(rootPath: string, agentId: string) string
}
class UUID {
+v4() string
}
RepoIngestionWorkflow --> MDocumentChunker : uses execute
MDocumentChunker --> ChunkResultSuccess : may return
MDocumentChunker --> ChunkResultErrorObject : may return
MDocumentChunker --> ZodError : may throw
RepoIngestionWorkflow --> ZodError : checks instance
RepoIngestionWorkflow --> Error : throws on failure
Utils ..> UUID : uses v4 for newThreadLink
class ClassValue {
}
Utils --> ClassValue : cn parameter
class ChunkResultUnion {
}
ChunkResultUnion <|-- ChunkResultSuccess
ChunkResultUnion <|-- ChunkResultErrorObject
Flow diagram for standardized workflow tool progress telemetryflowchart TD
A[Start workflow step
e.g. fetch-stock-data,
research-topic-item,
convert-pdf-to-markdown] --> B[Send writer.custom
type data-tool-progress
status in-progress
stage step-specific
message descriptive]
B --> C[Execute core step logic
e.g. call API,
run AI model,
process document]
C --> D{Step succeeds?}
D -- Yes --> E[Send writer.custom
type data-tool-progress
status done
stage step-specific
message completion]
D -- No --> F[Catch error
normalize to Error]
F --> G[Send writer.custom
type data-tool-progress
status done
stage step-specific
error message]
E --> H[End step
logStepEnd
finish span]
G --> H
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
🤖 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 valuable dependency updates for ESLint and performs a significant and consistent refactoring across multiple workflow files. The changes to use more descriptive stage identifiers and standardize the error and progress reporting mechanism are excellent improvements for clarity and maintainability. The introduction of type guards in repo-ingestion-workflow.ts also enhances type safety. Overall, this is a high-quality refactoring effort. I've found one minor area for improvement regarding configuration redundancy.
|
|
||
| export default [ | ||
| { ignores }, | ||
| globalIgnores(["dist", "node_modules"]), |
There was a problem hiding this comment.
The use of globalIgnores here seems redundant. The ignores array defined on line 10 already includes patterns for dist/** and node_modules/**, and it's applied globally on line 52. To avoid redundancy and keep the configuration clean, you can remove this line.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In the various
data-tool-progresserror paths (e.g.,fetchStockDataStep,getCompanyNewsStep,runAnalysisStep,generateReportStep,synthesizeResearchStep,generateResearchReportStep,convertPdfToMarkdownStep), you’re settingstatus: 'done'even when reporting an error; consider using a dedicated error status or keepingstatusconsistent with the non-error states so consumers can reliably distinguish failures. - In
research-synthesis-workflow.ts, theinitializeResearchStepstill usesstage: 'researchAgent'while subsequent steps use the more descriptive per-step identifiers (e.g.,research-topic-item,synthesize-research); you may want to align this stage name with the new naming convention for consistency. - The ESLint flat config currently applies
globals.browserto all**/*.tsand**/*.tsxfiles; if you have Node/server-side TypeScript, consider splitting the config by directory or file pattern so that browser globals are only enabled for client-side code.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the various `data-tool-progress` error paths (e.g., `fetchStockDataStep`, `getCompanyNewsStep`, `runAnalysisStep`, `generateReportStep`, `synthesizeResearchStep`, `generateResearchReportStep`, `convertPdfToMarkdownStep`), you’re setting `status: 'done'` even when reporting an error; consider using a dedicated error status or keeping `status` consistent with the non-error states so consumers can reliably distinguish failures.
- In `research-synthesis-workflow.ts`, the `initializeResearchStep` still uses `stage: 'researchAgent'` while subsequent steps use the more descriptive per-step identifiers (e.g., `research-topic-item`, `synthesize-research`); you may want to align this stage name with the new naming convention for consistency.
- The ESLint flat config currently applies `globals.browser` to all `**/*.ts` and `**/*.tsx` files; if you have Node/server-side TypeScript, consider splitting the config by directory or file pattern so that browser globals are only enabled for client-side code.
## Individual Comments
### Comment 1
<location> `lib/utils.ts:8-12` </location>
<code_context>
export function cn(...inputs: ClassValue[]) {
return twMerge(clsx(inputs))
}
+export function newThreadLink(
+ rootPath: string,
+ agentId: string,
+): `/${string}/${string}/chat/${string}?new=true` {
+ return `/${rootPath}/${agentId}/chat/${uuid()}?new=true`;
+}
</code_context>
<issue_to_address>
**suggestion:** Consider normalizing `rootPath` and `agentId` to avoid accidental double slashes in the generated URL.
If callers pass `rootPath` with or without a leading slash, or an `agentId` that already includes slashes, this helper can generate malformed URLs like `//agents//id/chat/...`. Normalizing inputs (e.g., trimming leading/trailing slashes) within this function would make it safer and more predictable to use.
</issue_to_address>
### Comment 2
<location> `eslint.config.js:63-65` </location>
<code_context>
},
languageOptions: {
parser: tsparser,
+ globals: globals.browser,
parserOptions: {
ecmaVersion: 2022,
</code_context>
<issue_to_address>
**issue (bug_risk):** Applying `globals.browser` to all `*.ts` / `*.tsx` files may hide issues in Node-only code.
Because this config applies `globals.browser` to all TS/TSX files, browser globals like `window` will appear valid even in server-side/Node-only modules, potentially hiding real bugs. Consider limiting `globals.browser` to known client/React files (e.g., narrower `files` patterns or a separate override) and using appropriate Node globals for backend code instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| export function newThreadLink( | ||
| rootPath: string, | ||
| agentId: string, | ||
| ): `/${string}/${string}/chat/${string}?new=true` { | ||
| return `/${rootPath}/${agentId}/chat/${uuid()}?new=true`; |
There was a problem hiding this comment.
suggestion: Consider normalizing rootPath and agentId to avoid accidental double slashes in the generated URL.
If callers pass rootPath with or without a leading slash, or an agentId that already includes slashes, this helper can generate malformed URLs like //agents//id/chat/.... Normalizing inputs (e.g., trimming leading/trailing slashes) within this function would make it safer and more predictable to use.
| languageOptions: { | ||
| parser: tsparser, | ||
| globals: globals.browser, |
There was a problem hiding this comment.
issue (bug_risk): Applying globals.browser to all *.ts / *.tsx files may hide issues in Node-only code.
Because this config applies globals.browser to all TS/TSX files, browser globals like window will appear valid even in server-side/Node-only modules, potentially hiding real bugs. Consider limiting globals.browser to known client/React files (e.g., narrower files patterns or a separate override) and using appropriate Node globals for backend code instead.
Greptile OverviewGreptile SummaryThis PR refactors workflow progress reporting and adds ESLint plugins for React development. The workflow changes standardize stage identifiers and error handling patterns across multiple workflows. Key Changes:
Issues Found:
Confidence Score: 3/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant ESLint as ESLint Config
participant Workflows as Workflow Files
participant Utils as lib/utils.ts
participant Deps as Dependencies
Dev->>Deps: Add eslint-plugin-react-hooks
Dev->>Deps: Add eslint-plugin-react-refresh
Deps-->>ESLint: New plugins available
Dev->>ESLint: Import react plugins
Dev->>ESLint: Add globalIgnores import
Dev->>ESLint: Configure flat config with plugins
Note over ESLint: Added reactHooks.configs.flat.recommended<br/>Added reactRefresh.configs.next<br/>Added globals.browser
Dev->>Workflows: Update document-processing-workflow.ts
Note over Workflows: Changed stage IDs:<br/>"documentProcessingAgent" → "convert-pdf-to-markdown"<br/>Changed error type:<br/>"data-workflow-step-error" → "data-tool-progress"
Dev->>Workflows: Update repo-ingestion-workflow.ts
Note over Workflows: Added type guards for validation<br/>Added embeddingModel parameter<br/>Added embeddingBatchSize parameter
Dev->>Workflows: Update research-synthesis-workflow.ts
Note over Workflows: Updated stage IDs:<br/>"workflow" → "research-topic-item"<br/>"researchAgent" → descriptive stages<br/>Standardized error handling
Dev->>Workflows: Update stock-analysis-workflow.ts
Note over Workflows: Updated stage IDs:<br/>"workflow" → specific stages<br/>Fixed typo: "data-workflow-p" → "data-tool-progress"<br/>Standardized error handling
Dev->>Utils: Add newThreadLink function
Utils->>Deps: Import @lukeed/uuid
Note over Utils,Deps: ⚠️ Missing dependency!<br/>@lukeed/uuid not in package.json
Note over Dev: Workflow Progress Event Pattern:<br/>type: 'data-tool-progress'<br/>data: { status, message, stage }<br/>Error events use status: 'done'
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (1)
-
src/mastra/workflows/stock-analysis-workflow.ts, line 135-143 (link)style: Setting
status: 'done'in error case may be misleading - considerstatus: 'error'to better indicate failure state
7 files reviewed, 6 comments
| @@ -1,6 +1,13 @@ | |||
| import { type ClassValue, clsx } from "clsx" | |||
| import { twMerge } from "tailwind-merge" | |||
| import { v4 as uuid } from "@lukeed/uuid"; | |||
There was a problem hiding this comment.
syntax: Missing dependency: @lukeed/uuid is imported but not present in package.json
| import { v4 as uuid } from "@lukeed/uuid"; | |
| import { v4 as uuid } from "uuid"; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/utils.ts
Line: 3:3
Comment:
**syntax:** Missing dependency: `@lukeed/uuid` is imported but not present in `package.json`
```suggestion
import { v4 as uuid } from "uuid";
```
How can I resolve this? If you propose a fix, please make it concise.| await writer?.custom({ | ||
| type: 'data-workflow-step-error', | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| stepId: 'synthesize-research', | ||
| stage: 'synthesize-research', | ||
| error: error instanceof Error ? error.message : 'Unknown error', | ||
| status: 'done', | ||
| }, |
There was a problem hiding this comment.
style: Setting status: 'done' in error case may be misleading - consider status: 'error' to better indicate failure state
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/mastra/workflows/research-synthesis-workflow.ts
Line: 391:397
Comment:
**style:** Setting `status: 'done'` in error case may be misleading - consider `status: 'error'` to better indicate failure state
How can I resolve this? If you propose a fix, please make it concise.| import tseslint from '@typescript-eslint/eslint-plugin' | ||
| import tsparser from '@typescript-eslint/parser' | ||
| import prettierConfig from 'eslint-config-prettier' | ||
| import { globalIgnores } from "eslint/config"; |
There was a problem hiding this comment.
style: Verify that globalIgnores export exists in eslint/config - this import path may not be valid for ESLint
Prompt To Fix With AI
This is a comment left during a code review.
Path: eslint.config.js
Line: 8:8
Comment:
**style:** Verify that `globalIgnores` export exists in `eslint/config` - this import path may not be valid for ESLint
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| export default [ | ||
| { ignores }, | ||
| globalIgnores(["dist", "node_modules"]), |
There was a problem hiding this comment.
style: This globalIgnores call is redundant - dist and node_modules are already in the ignores array above (lines 14-15). Consider removing this line.
Prompt To Fix With AI
This is a comment left during a code review.
Path: eslint.config.js
Line: 53:53
Comment:
**style:** This `globalIgnores` call is redundant - `dist` and `node_modules` are already in the `ignores` array above (lines 14-15). Consider removing this line.
How can I resolve this? If you propose a fix, please make it concise.| globalIgnores(["dist", "node_modules"]), | ||
| js.configs.recommended, | ||
| reactHooks.configs.flat.recommended, | ||
| reactRefresh.configs.next, |
There was a problem hiding this comment.
style: Verify that reactRefresh.configs.next exists - the standard config is typically reactRefresh.configs.recommended or a custom flat config object
Prompt To Fix With AI
This is a comment left during a code review.
Path: eslint.config.js
Line: 56:56
Comment:
**style:** Verify that `reactRefresh.configs.next` exists - the standard config is typically `reactRefresh.configs.recommended` or a custom flat config object
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
eslint.config.js (1)
63-70: Consider adding Node.js globals for server-side code.Setting
globals: globals.browseronly provides browser globals. In a Next.js project with server components and API routes, Node.js globals likeprocessmay be needed. Consider combining both:languageOptions: { parser: tsparser, - globals: globals.browser, + globals: { + ...globals.browser, + ...globals.node, + }, parserOptions: {src/mastra/workflows/document-processing-workflow.ts (3)
237-244: Inconsistent error handling: still usingdata-workflow-step-error.This step's error handler uses
data-workflow-step-errorwithstepId, whileconvertPdfToMarkdownStepwas updated to usedata-tool-progresswithstage. The PR objective is to standardize error handling, but this step wasn't updated.Apply consistent error handling:
await writer?.custom({ - type: 'data-workflow-step-error', + type: 'data-tool-progress', data: { - stepId: 'load-document', + stage: 'load-document', + status: 'done', error: error instanceof Error ? error.message : 'Unknown error', }, id: "load-document", });The same inconsistency exists in
indexChunksStep(lines 720-727).
219-227: Inconsistent payload field:stepIdvsstagein success messages.Success payloads use
stepIdhere, butconvertPdfToMarkdownStep(line 348) usesstage. For consistency with the error handling changes, consider standardizing all progress payloads to usestage.
405-413: Broken template literal in message string.The message uses double quotes but contains template literal syntax
${inputData.content}which won't be interpolated. It also has a stray backtick and comma inside the string.await writer?.custom({ type: 'data-tool-progress', data: { status: "done", - message: "Text passed through successfully ${inputData.content}`,", + message: `Text passed through successfully`, stepId: 'pass-text-through', }, id: "pass-text-through", });Note: Logging full
inputData.contentin progress messages could expose sensitive document data; consider omitting it or using a truncated preview.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
eslint.config.js(3 hunks)lib/utils.ts(1 hunks)package.json(2 hunks)src/mastra/workflows/document-processing-workflow.ts(4 hunks)src/mastra/workflows/repo-ingestion-workflow.ts(1 hunks)src/mastra/workflows/research-synthesis-workflow.ts(16 hunks)src/mastra/workflows/stock-analysis-workflow.ts(20 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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:
lib/utils.tssrc/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.tseslint.config.js
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/instructions/next-js.instructions.md)
Use parameterized queries or an ORM to prevent SQL injection attacks.
Files:
lib/utils.tssrc/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.tseslint.config.js
**/*.{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:
lib/utils.tssrc/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.tseslint.config.js
**/*.{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
**/*.{ts,tsx,js,jsx}: Use Mastra mcp tools (#mastradocs,#mastraChanges,#mastraexamples,#mastraBlog) for Mastra framework development to stay updated with latest features and best practices
When working with Next.js projects, always utilize thenext-devtools-mcpserver for all Next.js related queries
Files:
lib/utils.tssrc/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.tseslint.config.js
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/instructions/self-explanatory-code-commenting.instructions.md)
**/*.{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:
lib/utils.tssrc/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.ts
**/*.{css,tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS 4 with oklch color variables for styling
Files:
lib/utils.tssrc/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.ts
src/mastra/**/*
📄 CodeRabbit inference engine (src/AGENTS.md)
mastramodules can import fromutils, but must not import fromapporcli(excepttypes)
Files:
src/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.ts
src/mastra/workflows/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Mastra DSL for multi-step workflow definitions in src/mastra/workflows
src/mastra/workflows/**/*.ts: Define workflows using Mastra DSL patterns with propercreateWorkflowandcreateStepfrom @mastra/core/workflows
Use tools and agents as building blocks in workflows; prefer composition over duplication
Define input and output schemas for each step using Zod schema validation
Implement streaming support by piping agent responses to the step writer using.pipeTo(writer)for text chunk streaming
Use.then()for sequential workflow steps
Use.parallel()for concurrent multi-step execution (e.g., fetching from multiple APIs simultaneously)
Use.branch()for conditional logic based on input data conditions
Use.dowhile()for iterative refinement loops with conditional exit criteria
Use.foreach()for concurrent iteration over collections
Usesuspend()andresume()for human-in-the-loop approval workflows
Call.commit()at the end of workflow definitions to finalize the workflow
Use custom writer events with types like 'data-tool-progress' to stream step progress and status updatesAdd workflows under
src/mastra/workflowsto orchestrate multi-step flows
Files:
src/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.ts
src/mastra/{tools,agents,workflows}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Zod schemas for strict input/output validation in tools, agents, and workflows
Files:
src/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.ts
src/mastra/{tools,workflows}/**/*.ts
📄 CodeRabbit inference engine (src/mastra/AGENTS.md)
Use
RequestContext(per-request) or agent-specific runtime context types (e.g.EditorRuntimeContext) to enforce access control in tools and workflows
Files:
src/mastra/workflows/document-processing-workflow.tssrc/mastra/workflows/stock-analysis-workflow.tssrc/mastra/workflows/repo-ingestion-workflow.tssrc/mastra/workflows/research-synthesis-workflow.ts
⏰ 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). (5)
- GitHub Check: Sourcery review
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Codacy Security Scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
src/mastra/workflows/repo-ingestion-workflow.ts (1)
191-209: Well-structured type guards for result validation.The type guards
isSuccessResultandisErrorObjectprovide good runtime type narrowing. The error handling cascade (ZodError → Error → error object → unknown) is thorough.Minor suggestion: The
isSuccessResultguard could be slightly more defensive by also checking thatris not an array:const isSuccessResult = (r: unknown): r is { success: true; chunkCount?: number } => typeof r === 'object' && r !== null && !Array.isArray(r) && 'success' in r && (r as { success: unknown }).success === true;src/mastra/workflows/stock-analysis-workflow.ts (1)
135-143: Good standardization of error handling todata-tool-progress.The error payload now correctly uses
data-tool-progresstype withstageandstatus: 'done'fields, consistent with the PR's objective to standardize workflow progress messaging.src/mastra/workflows/research-synthesis-workflow.ts (1)
391-399: Good: Error handling standardized todata-tool-progress.The error handler correctly uses
data-tool-progresswithstageandstatus: 'done'fields, maintaining consistency with the updated workflow messaging pattern.
| { ignores }, | ||
| globalIgnores(["dist", "node_modules"]), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant globalIgnores call.
The globalIgnores(["dist", "node_modules"]) duplicates entries already present in the ignores array (lines 13-14: 'node_modules/**' and 'dist/**'). This redundancy adds no value and may cause confusion.
{ ignores },
- globalIgnores(["dist", "node_modules"]),
js.configs.recommended,📝 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.
| { ignores }, | |
| globalIgnores(["dist", "node_modules"]), | |
| { ignores }, | |
| js.configs.recommended, |
🤖 Prompt for AI Agents
In eslint.config.js around lines 52 to 53, remove the redundant
globalIgnores(["dist", "node_modules"]) call because those patterns are already
present in the ignores array (lines ~13-14 as 'node_modules/**' and 'dist/**');
simply delete the globalIgnores(...) entry so ignores are defined in a single
place to avoid duplication and confusion.
| export function newThreadLink( | ||
| rootPath: string, | ||
| agentId: string, | ||
| ): `/${string}/${string}/chat/${string}?new=true` { | ||
| return `/${rootPath}/${agentId}/chat/${uuid()}?new=true`; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add TSDoc documentation and consider input validation.
The function lacks TSDoc documentation. Per coding guidelines, public APIs should include parameter descriptions, return types, and examples. Additionally, consider validating/sanitizing rootPath and agentId to prevent malformed URLs (e.g., if they contain slashes or special characters).
+/**
+ * Generates a new chat thread URL path with a unique identifier.
+ * @param rootPath - The root path segment (e.g., 'app', 'dashboard')
+ * @param agentId - The agent identifier
+ * @returns A URL path in the format `/{rootPath}/{agentId}/chat/{uuid}?new=true`
+ * @example
+ * newThreadLink('app', 'agent-123')
+ * // => '/app/agent-123/chat/550e8400-e29b-41d4-a716-446655440000?new=true'
+ */
export function newThreadLink(
rootPath: string,
agentId: string,
): `/${string}/${string}/chat/${string}?new=true` {
return `/${rootPath}/${agentId}/chat/${uuid()}?new=true`;
}🤖 Prompt for AI Agents
In lib/utils.ts around lines 8–13, add a TSDoc block for newThreadLink
describing parameters (rootPath, agentId), the template-literal return type, and
an example usage; then validate/sanitize inputs by trimming and removing
leading/trailing slashes from rootPath and agentId, ensure they are non-empty
(throw or return a clear error), and URL-encode each segment (e.g., with
encodeURIComponent) before composing the final
`/${rootPath}/${agentId}/chat/${uuid()}?new=true` string so the generated URL is
well-formed and safe.
| "eslint-plugin-react-hooks": "^7.0.1", | ||
| "eslint-plugin-react-refresh": "^0.4.23", |
There was a problem hiding this comment.
Move ESLint plugins to devDependencies.
ESLint plugins are development-time tools used for linting, not runtime dependencies. They should be in devDependencies to avoid unnecessary installation in production environments.
Move these entries from dependencies to devDependencies:
"devDependencies": {
"@mdx-js/loader": "^3.1.1",
...
+ "eslint-plugin-react-hooks": "^7.0.1",
+ "eslint-plugin-react-refresh": "^0.4.23",
...
}And remove from dependencies.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In package.json around lines 122-123, the ESLint plugins
"eslint-plugin-react-hooks" and "eslint-plugin-react-refresh" are incorrectly
listed under dependencies; move these two entries into devDependencies and
remove them from dependencies (update package.json accordingly or run npm
install --save-dev eslint-plugin-react-hooks@^7.0.1
eslint-plugin-react-refresh@^0.4.23 and remove the entries from dependencies),
then run an install to update lockfile.
| message: 'Converting PDF to markdown `...', | ||
| stage: "documentProcessingAgent", | ||
| stage: "convert-pdf-to-markdown", |
There was a problem hiding this comment.
Fix typo: stray backtick in message string.
The message contains an errant backtick that appears unintentional.
status: 'in-progress',
- message: 'Converting PDF to markdown `...',
+ message: 'Converting PDF to markdown...',
stage: "convert-pdf-to-markdown",📝 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.
| message: 'Converting PDF to markdown `...', | |
| stage: "documentProcessingAgent", | |
| stage: "convert-pdf-to-markdown", | |
| status: 'in-progress', | |
| message: 'Converting PDF to markdown...', | |
| stage: "convert-pdf-to-markdown", |
🤖 Prompt for AI Agents
In src/mastra/workflows/document-processing-workflow.ts around lines 271-272,
the log/message string "Converting PDF to markdown `..." contains a stray
backtick; remove the backtick so the message reads "Converting PDF to
markdown..." (ensure punctuation and string quoting remain valid after edit).
| embeddingModel: 'google/gemini-embedding-001', | ||
| embeddingBatchSize: 50, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider making embedding configuration parameters configurable.
The embedding model and batch size are hardcoded. For flexibility, consider adding these as optional inputs to the workflow schema with sensible defaults.
// In scanInputSchema, add optional embedding config:
embeddingModel: z.string().default('google/gemini-embedding-001'),
embeddingBatchSize: z.number().default(50),🤖 Prompt for AI Agents
In src/mastra/workflows/repo-ingestion-workflow.ts around lines 184-185, the
embeddingModel and embeddingBatchSize are hardcoded; update the scan input
schema to accept optional embeddingModel (default 'google/gemini-embedding-001')
and embeddingBatchSize (default 50), then replace the hardcoded values by
reading those fields from the validated input (falling back to the defaults for
backward compatibility) so the workflow can be configured per-run without
breaking existing callers.
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: "in-progress", | ||
| message: `Researching topic: ${inputData.topics}...`, | ||
| stage: "researchAgent", | ||
| status: "in-progress", | ||
| message: `Researching topic: ${inputData.topics}...`, | ||
| stage: "researchAgent", | ||
| }, | ||
| id: 'initialize-research', | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Inconsistent stage naming in initializeResearchStep.
This step uses stage: "researchAgent" while other steps in this file use their step IDs (e.g., 'research-topic-item', 'synthesize-research'). For consistency, consider updating to stage: 'initialize-research'.
await writer?.custom({
type: 'data-tool-progress',
data: {
status: "in-progress",
message: `Researching topic: ${inputData.topics}...`,
- stage: "researchAgent",
+ stage: "initialize-research",
},
id: 'initialize-research',
});🤖 Prompt for AI Agents
In src/mastra/workflows/research-synthesis-workflow.ts around lines 72 to 80,
the progress event sets stage: "researchAgent" which is inconsistent with other
steps; update the stage value to 'initialize-research' so it matches the step ID
(and keeps stage naming consistent across the file), leaving the rest of the
writer.custom payload unchanged.
|
|
||
| logStepEnd('fetch-stock-data', { symbol: result.symbol, price: result.currentPrice }, Date.now() - startTime); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Extra blank line.
There's an extra blank line that appears to be a formatting artifact. Same issue at lines 258, 407, and 559.
🤖 Prompt for AI Agents
In src/mastra/workflows/stock-analysis-workflow.ts around lines 126-127 (and
similarly at lines 258, 407, and 559), there are stray extra blank lines
introduced as formatting artifacts; remove those blank lines so each statement
is directly adjacent to the next (e.g., delete the empty line after the
logStepEnd call) and run the formatter/linter to ensure consistent spacing
across the file.
There was a problem hiding this comment.
Pull request overview
This PR refactors workflow error handling and enhances development tooling by adding ESLint plugins for React development. The main changes standardize error reporting across workflows by transitioning from data-workflow-step-error to data-tool-progress with more descriptive stage identifiers. Additionally, new ESLint plugins are configured to improve code quality for React components.
Key Changes
- Error handling standardization: Updated error handling across multiple workflows to use
data-tool-progresstype withstageandstatusfields instead ofdata-workflow-step-errorwithstepId - Stage identifier improvements: Changed generic "workflow" stage identifiers to more descriptive names like "fetch-stock-data", "research-topic-item", and "convert-pdf-to-markdown"
- React linting support: Added
eslint-plugin-react-hooksandeslint-plugin-react-refreshdependencies with corresponding ESLint configuration
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added eslint-plugin-react-hooks and eslint-plugin-react-refresh dependencies for improved React linting |
| package-lock.json | Updated lock file with new plugin dependencies and transitive dependencies |
| eslint.config.js | Configured new React plugins and added global browser environment support |
| src/mastra/workflows/stock-analysis-workflow.ts | Refactored to use descriptive stage identifiers and standardized error handling with data-tool-progress type |
| src/mastra/workflows/research-synthesis-workflow.ts | Updated stage names and error handling to match new pattern, improved consistency |
| src/mastra/workflows/repo-ingestion-workflow.ts | Added type guards for result validation and included embedding configuration parameters |
| src/mastra/workflows/document-processing-workflow.ts | Changed stage identifier and error type for consistency with other workflows |
| lib/utils.ts | Added newThreadLink utility function for generating thread URLs with UUID |
| stepId: 'run-analysis', | ||
| stage: 'run-analysis', | ||
| error: error instanceof Error ? error.message : 'Unknown error', | ||
| status: 'done' |
There was a problem hiding this comment.
The error data structure is missing the 'message' field that is present in other progress updates. For consistency with the in-progress and done status messages, consider adding a message field like "Analysis failed" or similar to provide better context in error scenarios.
| status: 'done' | |
| status: 'done', | |
| message: 'Analysis failed' |
| stepId: 'convert-pdf-to-markdown', | ||
| stage: 'convert-pdf-to-markdown', | ||
| status: "done", | ||
| error: error instanceof Error ? error.message : 'Unknown error', |
There was a problem hiding this comment.
The error data structure is inconsistent with other progress updates. Similar to other error handlers in this file, consider adding a 'message' field to provide better context. The current structure only includes 'stage', 'status', and 'error', while success cases include descriptive messages.
| error: error instanceof Error ? error.message : 'Unknown error', | |
| error: error instanceof Error ? error.message : 'Unknown error', | |
| message: `Error in convert-pdf-to-markdown: ${error instanceof Error ? error.message : 'Unknown error'}`, |
| import tseslint from '@typescript-eslint/eslint-plugin' | ||
| import tsparser from '@typescript-eslint/parser' | ||
| import prettierConfig from 'eslint-config-prettier' | ||
| import { globalIgnores } from "eslint/config"; |
There was a problem hiding this comment.
The import { globalIgnores } from "eslint/config" appears to be incorrect. ESLint does not export a globalIgnores function from "eslint/config". The standard way to define ignores in flat config is to use the ignores property in configuration objects. The ignores array already defined on line 10 should be sufficient. This import and its usage on line 53 should be removed.
| import { globalIgnores } from "eslint/config"; |
|
|
||
| export default [ | ||
| { ignores }, | ||
| globalIgnores(["dist", "node_modules"]), |
There was a problem hiding this comment.
This usage of globalIgnores(["dist", "node_modules"]) is incorrect and will cause a runtime error. The globalIgnores function does not exist in ESLint's API. Additionally, these paths are already included in the ignores array defined earlier (lines 13-14). This line should be removed.
| globalIgnores(["dist", "node_modules"]), | ||
| js.configs.recommended, | ||
| reactHooks.configs.flat.recommended, | ||
| reactRefresh.configs.next, |
There was a problem hiding this comment.
The configuration reference reactRefresh.configs.next appears to be incorrect. The eslint-plugin-react-refresh package typically exports configs.recommended or configs.vite, not configs.next. This should be verified against the actual plugin documentation and corrected to use a valid configuration export.
| reactRefresh.configs.next, | |
| reactRefresh.configs.recommended, |
| export function newThreadLink( | ||
| rootPath: string, | ||
| agentId: string, | ||
| ): `/${string}/${string}/chat/${string}?new=true` { |
There was a problem hiding this comment.
The function newThreadLink has a return type that is too specific. The template literal type /${string}/${string}/chat/${string}?new=true doesn't accurately represent the actual return value since uuid() returns a specific string format, not just any string. Consider using a more appropriate return type like string or a branded type if type safety for URLs is important.
| ): `/${string}/${string}/chat/${string}?new=true` { | |
| ): string { |
eslint-plugin-react-hooksandeslint-plugin-react-refreshto package.json for improved linting support.document-processing-workflow.ts:repo-ingestion-workflow.ts:embeddingModelandembeddingBatchSizeparameters to themdocumentChunker.executecall.research-synthesis-workflow.ts:stock-analysis-workflow.ts:Summary by Sourcery
Align workflow telemetry and error reporting across document processing, research synthesis, stock analysis, and repo ingestion, while tightening linting configuration and adding a small routing utility.
New Features:
Enhancements:
Build:
Chores: