Repository navigation
Conversation
- feat: create confirmation tool for user confirmation requests with input and output schemas. - feat: implement cytoscape tool for generating Cytoscape.js compatible graph data structures, including input validation and tracing. - feat: add downsample tool for downsampling numeric series using LTTB and other algorithms, with comprehensive input and output schemas. - feat: develop leaflet tool for generating Leaflet/GeoJSON compatible map data structures, including bounds clipping and marker generation. - feat: introduce resilient fetch tool for fetching data from URLs with automatic retries and rate limiting using axios and Bottleneck. - feat: create spatial index tool for building and querying RBush spatial indexes, with support for JSON serialization. - test: add integration tests for downsample tool to ensure correct downsampling behavior. - test: implement tests for spatial index tool to validate index building and searching functionality. - test: add integration tests for chartJsTool to verify decimation of large datasets. - chore: add TypeScript definitions for downsample-lttb module. - docs: create session summaries for continuity tracking and project analysis.
Learn moreAll Green is an AI agent that automatically: ✅ Addresses code review comments ✅ Fixes failing CI checks ✅ Resolves merge conflicts |
Reviewer's GuideIntroduces a new suite of graphing, mapping, downsampling, resilient fetch, spatial index, and confirmation tools, wires them into multiple agent networks, and enhances observability, validation, and typings across existing technical analysis and imaging tools plus adds docs and test scaffolding. Sequence diagram for chartjs-generator tool execution with downsampling and tracingsequenceDiagram
actor User
participant Agent as chartJsAgent
participant Tool as chartJsTool
participant Downsample as downsampleTool
participant Trace as tracingContext
participant Writer as writer
participant Logger as logToolExecution
User->>Agent: Request Chart.js config
Agent->>Tool: execute(input, context)
Tool->>Writer: custom data-tool-progress
Tool->>Trace: currentSpan.createChildSpan(TOOL_CALL)
Tool->>Logger: logToolExecution start
Tool->>Tool: build labels and datasets
alt series larger than threshold
Tool->>Downsample: execute(values, target, algorithm=lttb)
Downsample-->>Tool: indices, values
Tool->>Tool: apply indices to labels and datasets
else small series
Tool->>Tool: use original labels and datasets
end
Tool->>Tool: build Chart.js config
Tool->>Trace: toolSpan.update(output, metadata)
Tool->>Trace: toolSpan.end()
Tool->>Logger: logToolExecution success
Tool->>Writer: custom data-tool-progress done
Tool-->>Agent: { config }
Agent-->>User: Chart.js config rendered
Sequence diagram for confirmation-tool suspend and resume flow in a networksequenceDiagram
actor User
participant Network as AgentNetwork
participant Tool as confirmationTool
User->>Network: Request action requiring confirmation
Network->>Tool: execute({ action }, context)
Tool->>Tool: inspect context.agent.resumeData
alt resumeData.confirmed is not true
Tool-->>Network: suspend({ message, action })
Network-->>User: Ask for confirmation message
User->>Network: User responds confirmed true
Network->>Tool: execute({ action }, context with resumeData.confirmed=true)
Tool->>Tool: detect confirmed true
Tool-->>Network: { confirmed: true, action }
Network-->>User: Proceed with action
else already confirmed
Tool-->>Network: { confirmed: true, action }
Network-->>User: Proceed with action
end
Class diagram for new graphing and analysis agents and toolsclassDiagram
class ChartJsTool {
+id chartjs-generator
+execute(input, context)
+inputSchema
+outputSchema
}
class DownsampleTool {
+id downsample
+execute(input, context)
+inputSchema
+outputSchema
}
class LeafletTool {
+id leaflet-generator
+execute(input, context)
+inputSchema
+outputSchema
}
class CytoscapeTool {
+id cytoscape-generator
+execute(input, context)
+inputSchema
+outputSchema
}
class ResilientFetchTool {
+id resilient-fetch
+execute(input, context)
+inputSchema
+outputSchema
}
class SpatialIndexTool {
+id spatial-index
+execute(input, context)
+inputSchema
+outputSchema
}
class ConfirmationTool {
+id confirmation-tool
+execute(input, context)
+inputSchema
+outputSchema
+suspendSchema
+resumeSchema
}
class GraphSupervisorAgent {
+id graphSupervisorAgent
+tools
+inputProcessors
+outputProcessors
}
class TechnicalAnalysisAgent {
+id technicalAnalysisAgent
+tools
}
class ChartJsAgent {
+id chartJsAgent
+tools
}
class MappingAgent {
+id mappingAgent
+tools
}
class FetchAgent {
+id fetchAgent
+tools
}
class FinnhubAgent {
+id finnhubAgent
+tools
}
class CodeGraphAgent {
+id codeGraphAgent
+tools
}
class CodeMetricsAgent {
+id codeMetricsAgent
+tools
+defaultOptions
}
class QualityChecker {
+id quality-checker
+processOutputStep(args)
}
GraphSupervisorAgent ..> ChartJsTool : uses
GraphSupervisorAgent ..> DownsampleTool : uses
GraphSupervisorAgent ..> LeafletTool : uses
GraphSupervisorAgent ..> CytoscapeTool : uses
GraphSupervisorAgent ..> ResilientFetchTool : uses
TechnicalAnalysisAgent ..> ChartJsTool : via ChartJsAgent
TechnicalAnalysisAgent ..> SpatialIndexTool : can combine
ChartJsAgent ..> ChartJsTool : uses
ChartJsAgent ..> DownsampleTool : uses
MappingAgent ..> LeafletTool : uses
MappingAgent ..> CytoscapeTool : uses
FetchAgent ..> ResilientFetchTool : uses
FinnhubAgent ..> ResilientFetchTool : indirectly
CodeGraphAgent ..> CytoscapeTool : uses
CodeGraphAgent ..> ResilientFetchTool : uses
CodeMetricsAgent ..> ResilientFetchTool : uses
class CodingTeamNetwork {
+processors
+tools
}
CodingTeamNetwork ..> QualityChecker : outputProcessor
CodingTeamNetwork ..> ConfirmationTool : tool
class MastraCore {
<<framework>>
}
MastraCore <|-- GraphSupervisorAgent
MastraCore <|-- TechnicalAnalysisAgent
MastraCore <|-- ChartJsAgent
MastraCore <|-- MappingAgent
MastraCore <|-- FetchAgent
MastraCore <|-- FinnhubAgent
MastraCore <|-- CodeGraphAgent
MastraCore <|-- CodeMetricsAgent
MastraCore <|-- CodingTeamNetwork
Class diagram for Leaflet and Cytoscape tools with observabilityclassDiagram
class LeafletTool {
+id leaflet-generator
+execute(input, context)
+onInputStart(params)
+onInputDelta(params)
+onInputAvailable(params)
+onOutput(params)
+inputSchema
+outputSchema
}
class LeafletToolContext {
+userId string
+workspaceId string
}
class CytoscapeTool {
+id cytoscape-generator
+execute(input, context)
+onInputStart(params)
+onInputDelta(params)
+onInputAvailable(params)
+onOutput(params)
+inputSchema
+outputSchema
}
class CytoscapeContext {
+userId string
+workspaceId string
}
class TracingContext {
+currentSpan
+createChildSpan(options)
}
class Logger {
+logToolExecution(toolId, inputMeta, outputMeta)
+info(message, data)
+warn(message, data)
+error(message, data)
}
LeafletTool ..> LeafletToolContext : requestContext
CytoscapeTool ..> CytoscapeContext : requestContext
LeafletTool ..> TracingContext : TOOL_CALL span
CytoscapeTool ..> TracingContext : TOOL_CALL span
LeafletTool ..> Logger : logToolExecution
CytoscapeTool ..> Logger : logToolExecution
class Writer {
+custom(event)
}
LeafletTool ..> Writer : data-tool-progress
CytoscapeTool ..> Writer : data-tool-progress
Flow diagram for resilient-fetch tool with rate limiting and retriesflowchart TD
A["Start execute(input)"] --> B["Emit data-tool-progress
status in-progress"]
B --> C["Schedule request via Bottleneck limiter
priority from input"]
C --> D["Axios request with axios-retry
method, url, data, headers"]
D -->|Success| E["Emit data-tool-progress
status done with status text"]
E --> F["Return { status, data, headers }"]
D -->|Error after retries| G["Build errorMessage
from response or error"]
G --> H["log.error Resilient fetch failed"]
H --> I["Emit data-tool-progress
status done with error message"]
I --> J["Throw Error(Request failed: errorMessage)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
🤖 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 CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR introduces a suite of visualization and data-analysis tools (Chart.js, Cytoscape, Leaflet, spatial indexing, downsampling) integrated with tracing and observability, registers eight new graphing agents, adds confirmation tools to network configurations, updates dependencies for chart/map libraries, and removes legacy agent specifications. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Contributor
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 significantly expands the project's capabilities by introducing a suite of new tools focused on data visualization, spatial analysis, and robust data fetching. It also integrates these new tools into existing agent networks, enhancing their functionality and enabling more complex workflows. Alongside these new features, the PR improves the project's observability through detailed tracing attributes, refines existing tools, and bolsters the testing framework to ensure stability and correctness. The changes aim to provide a more versatile and reliable platform for building intelligent agents. 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. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- In
QualityChecker.evaluateQuality, the keyword lists overlap (e.g.,testappears as both a negative and part of positive phrases), which means text mentioning tests can be simultaneously penalized and rewarded; consider tightening the negative list (e.g., onlytodo,fixme,wip, etc.) or using more precise matching to avoid conflicting signals. - For consistency with other tools that emit tracing metadata, consider adding a TOOL_CALL span around
resilientFetchToolexecution (you already importSpanType) so fetch failures/successes and latencies are visible in the same observability pipeline as the new chart/graph/mapping tools.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `QualityChecker.evaluateQuality`, the keyword lists overlap (e.g., `test` appears as both a negative and part of positive phrases), which means text mentioning tests can be simultaneously penalized and rewarded; consider tightening the negative list (e.g., only `todo`, `fixme`, `wip`, etc.) or using more precise matching to avoid conflicting signals.
- For consistency with other tools that emit tracing metadata, consider adding a TOOL_CALL span around `resilientFetchTool` execution (you already import `SpanType`) so fetch failures/successes and latencies are visible in the same observability pipeline as the new chart/graph/mapping tools.
## Individual Comments
### Comment 1
<location> `src/mastra/tools/spatial-index.tool.ts:20-29` </location>
<code_context>
+ action: z.enum(['build', 'search', 'fromJSON', 'toJSON']).describe('Action to perform'),
</code_context>
<issue_to_address>
**issue (bug_risk):** The `toJSON` action is declared but never handled, which can lead to confusing no-op behavior.
Because `action: 'toJSON'` is allowed in the schema but not handled explicitly, requests with this action just hit the default `{ message: 'No-op' }` path. This is misleading for callers expecting serialization of an existing index. Either implement a proper `toJSON` branch (e.g., returning a normalized JSON representation) or remove `'toJSON'` from the enum so clients can’t depend on a no-op action.
</issue_to_address>
### Comment 2
<location> `src/mastra/tools/resilient-fetch.tool.ts:49-58` </location>
<code_context>
+ options: z.record(z.string(), z.unknown()),
+ }),
+ }),
+ execute: async (input, context) => {
+ const { data, indicators, chartType, title } = input
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Resilient fetch ignores `abortSignal`, so long-running requests cannot be cancelled.
`context?.abortSignal` isn’t passed through, so cancelled tool calls still execute the axios request and use resources. Please wire the signal into axios (e.g., via `AbortController` / `signal` or `cancelToken`), short-circuit if `abortSignal.aborted` is already true, and ensure queued jobs in Bottleneck are also cancelled if the signal fires while waiting.
Suggested implementation:
```typescript
}),
execute: async (input, context) => {
const abortSignal = context?.abortSignal
// Short-circuit if the tool call has already been cancelled
if (abortSignal?.aborted) {
// Use a standard Web-style AbortError so upstream callers
// can consistently detect cancellations.
throw new DOMException(
'The resilient fetch request was aborted before it started',
'AbortError',
)
}
const { data, indicators, chartType, title } = input
```
I can only see part of the file, so to fully implement your request you should also:
1. **Pass the abort signal into axios**
Wherever you build the axios request (likely something like `client({ url, method, ... })` or `client.request({ ... })`), ensure the signal is attached:
- If you see a call like:
```ts
const response = await client.request({
url,
method,
headers,
data,
// ...
})
```
change it to:
```ts
const response = await client.request({
url,
method,
headers,
data,
// ...
signal: context?.abortSignal,
})
```
- Do the same for any other axios calls inside this tool.
2. **Wire the abort signal into Bottleneck jobs so queued jobs are cancelled**
Look for where you call `limiter.schedule` / `limiter.wrap` for this tool, something like:
```ts
const response = await limiter.schedule(() => client.request({ ... }))
```
Refactor that to a helper that handles aborts, for example:
```ts
const scheduleWithAbort = <T>(
fn: () => Promise<T>,
signal?: AbortSignal,
): Promise<T> => {
if (!signal) return limiter.schedule(fn)
if (signal.aborted) {
throw new DOMException('The resilient fetch request was aborted', 'AbortError')
}
// If your Bottleneck version supports job IDs and cancellation:
// const jobId = `resilient-fetch-${Date.now()}-${Math.random().toString(36).slice(2)}`
// const promise = limiter.schedule({ id: jobId }, fn)
// const onAbort = () => limiter.cancel(jobId)
// signal.addEventListener('abort', onAbort, { once: true })
// return promise.finally(() => signal.removeEventListener('abort', onAbort))
// If not, at minimum ensure the function checks the abort signal again:
return limiter.schedule(async () => {
if (signal.aborted) {
throw new DOMException('The resilient fetch request was aborted', 'AbortError')
}
return fn()
})
}
```
Then replace direct `limiter.schedule(...)` calls with:
```ts
const response = await scheduleWithAbort(
() => client.request({ /* ...existing config..., */ signal: context?.abortSignal }),
context?.abortSignal,
)
```
3. **Ensure uniform cancellation semantics**
- Any error handling which currently treats axios/network errors should also treat the `AbortError` thrown above as a cancellation (e.g., not retried, surfaced as a cancelled tool call).
- If you have retry logic that wraps axios (beyond `axiosRetry`), ensure it does not retry once `abortSignal.aborted` is true.
You’ll need to integrate the `scheduleWithAbort` helper and the `signal` property into the existing code structure (wherever axios + Bottleneck are actually used) following your current style and imports.
</issue_to_address>
### Comment 3
<location> `src/mastra/tools/chartjs.tool.ts:47-50` </location>
<code_context>
+ .optional()
+ .default([])
+ .describe('Indicators to overlay'),
+ chartType: z
+ .enum(['line', 'bar', 'candlestick'])
+ .default('line')
+ .describe('Base chart type'),
+ title: z.string().optional(),
+ }),
</code_context>
<issue_to_address>
**issue (bug_risk):** Supporting `candlestick` in `chartType` without matching dataset shape may produce invalid Chart.js configs.
The enum allows `'candlestick'`, but the datasets you build are simple line/bar series (e.g., `closePrices`), not OHLC objects required by Chart.js financial charts. Choosing `candlestick` will likely produce an invalid or misleading chart. Either drop `'candlestick'` for now, or special-case it to build proper OHLC datasets from `open/high/low/close` in `input.data`.
</issue_to_address>
### Comment 4
<location> `src/mastra/tools/leaflet.tool.ts:127-136` </location>
<code_context>
+ const startTime = Date.now()
+ logToolExecution('cytoscape-generator', { nodes: nodes.length, edges: edges.length })
+
+ await writer?.custom({
+ type: 'data-tool-progress',
+ data: {
</code_context>
<issue_to_address>
**nitpick:** Error reporting in the progress message recomputes the message instead of using the normalized `err` instance.
Since you already normalize the error into `err` for logging/tracing, the progress payload should also use `err.message` instead of branching on the original `error`. This keeps formatting consistent and avoids subtle discrepancies between logs and progress reporting.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+20
to
+29
| action: z.enum(['build', 'search', 'fromJSON', 'toJSON']).describe('Action to perform'), | ||
| points: z | ||
| .array( | ||
| z.object({ | ||
| id: z.string(), | ||
| lat: z.number(), | ||
| lng: z.number(), | ||
| data: z.record(z.string(), z.unknown()).optional(), | ||
| }) | ||
| ) |
There was a problem hiding this comment.
issue (bug_risk): The toJSON action is declared but never handled, which can lead to confusing no-op behavior.
Because action: 'toJSON' is allowed in the schema but not handled explicitly, requests with this action just hit the default { message: 'No-op' } path. This is misleading for callers expecting serialization of an existing index. Either implement a proper toJSON branch (e.g., returning a normalized JSON representation) or remove 'toJSON' from the enum so clients can’t depend on a no-op action.
Comment on lines
+49
to
+58
| execute: async (input, context) => { | ||
| const { url, method, data, headers, priority } = input | ||
|
|
||
| await context?.writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'in-progress', | ||
| message: `Scheduling ${method} request to ${url}`, | ||
| stage: 'resilient-fetch', | ||
| }, |
There was a problem hiding this comment.
suggestion (bug_risk): Resilient fetch ignores abortSignal, so long-running requests cannot be cancelled.
context?.abortSignal isn’t passed through, so cancelled tool calls still execute the axios request and use resources. Please wire the signal into axios (e.g., via AbortController / signal or cancelToken), short-circuit if abortSignal.aborted is already true, and ensure queued jobs in Bottleneck are also cancelled if the signal fires while waiting.
Suggested implementation:
}),
execute: async (input, context) => {
const abortSignal = context?.abortSignal
// Short-circuit if the tool call has already been cancelled
if (abortSignal?.aborted) {
// Use a standard Web-style AbortError so upstream callers
// can consistently detect cancellations.
throw new DOMException(
'The resilient fetch request was aborted before it started',
'AbortError',
)
}
const { data, indicators, chartType, title } = inputI can only see part of the file, so to fully implement your request you should also:
-
Pass the abort signal into axios
Wherever you build the axios request (likely something likeclient({ url, method, ... })orclient.request({ ... })), ensure the signal is attached:- If you see a call like:
change it to:
const response = await client.request({ url, method, headers, data, // ... })
const response = await client.request({ url, method, headers, data, // ... signal: context?.abortSignal, })
- Do the same for any other axios calls inside this tool.
- If you see a call like:
-
Wire the abort signal into Bottleneck jobs so queued jobs are cancelled
Look for where you calllimiter.schedule/limiter.wrapfor this tool, something like:const response = await limiter.schedule(() => client.request({ ... }))
Refactor that to a helper that handles aborts, for example:
const scheduleWithAbort = <T>( fn: () => Promise<T>, signal?: AbortSignal, ): Promise<T> => { if (!signal) return limiter.schedule(fn) if (signal.aborted) { throw new DOMException('The resilient fetch request was aborted', 'AbortError') } // If your Bottleneck version supports job IDs and cancellation: // const jobId = `resilient-fetch-${Date.now()}-${Math.random().toString(36).slice(2)}` // const promise = limiter.schedule({ id: jobId }, fn) // const onAbort = () => limiter.cancel(jobId) // signal.addEventListener('abort', onAbort, { once: true }) // return promise.finally(() => signal.removeEventListener('abort', onAbort)) // If not, at minimum ensure the function checks the abort signal again: return limiter.schedule(async () => { if (signal.aborted) { throw new DOMException('The resilient fetch request was aborted', 'AbortError') } return fn() }) }
Then replace direct
limiter.schedule(...)calls with:const response = await scheduleWithAbort( () => client.request({ /* ...existing config..., */ signal: context?.abortSignal }), context?.abortSignal, )
-
Ensure uniform cancellation semantics
- Any error handling which currently treats axios/network errors should also treat the
AbortErrorthrown above as a cancellation (e.g., not retried, surfaced as a cancelled tool call). - If you have retry logic that wraps axios (beyond
axiosRetry), ensure it does not retry onceabortSignal.abortedis true.
- Any error handling which currently treats axios/network errors should also treat the
You’ll need to integrate the scheduleWithAbort helper and the signal property into the existing code structure (wherever axios + Bottleneck are actually used) following your current style and imports.
Comment on lines
+47
to
+50
| chartType: z | ||
| .enum(['line', 'bar', 'candlestick']) | ||
| .default('line') | ||
| .describe('Base chart type'), |
There was a problem hiding this comment.
issue (bug_risk): Supporting candlestick in chartType without matching dataset shape may produce invalid Chart.js configs.
The enum allows 'candlestick', but the datasets you build are simple line/bar series (e.g., closePrices), not OHLC objects required by Chart.js financial charts. Choosing candlestick will likely produce an invalid or misleading chart. Either drop 'candlestick' for now, or special-case it to build proper OHLC datasets from open/high/low/close in input.data.
Comment on lines
+127
to
+136
| await writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'in-progress', | ||
| message: `Generating map data for ${points.length} points`, | ||
| stage: 'leaflet-generator', | ||
| }, | ||
| id: 'leaflet-generator', | ||
| }) | ||
|
|
There was a problem hiding this comment.
nitpick: Error reporting in the progress message recomputes the message instead of using the normalized err instance.
Since you already normalize the error into err for logging/tracing, the progress payload should also use err.message instead of branching on the original error. This keeps formatting consistent and avoids subtle discrepancies between logs and progress reporting.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a significant number of new tools and agents, greatly expanding the capabilities of the system, particularly in graphing, mapping, and data analysis. The new tools are well-structured, include robust validation with Zod, and are instrumented for observability with tracing and progress reporting, which is excellent. The addition of documentation and integration tests for the new features is also a major plus.
I've identified a few critical issues related to TypeScript type exports that will likely cause build failures and should be addressed. I also found a risky type assertion that bypasses type safety. Additionally, there's a minor cleanup needed in one of the new documentation files. Overall, this is a substantial and valuable contribution.
| import type { RequestContext } from '@mastra/core/request-context' | ||
|
|
||
| export type UserTier = 'free' | 'pro' | 'enterprise' | ||
| type UserTier = 'free' | 'pro' | 'enterprise' |
Contributor
There was a problem hiding this comment.
The UserTier type is used by the exported interface ArxivRequestContext, but it is no longer exported itself. This will cause a build error in any module that imports ArxivRequestContext. Please re-add the export keyword.
Suggested change
| type UserTier = 'free' | 'pro' | 'enterprise' | |
| export type UserTier = 'free' | 'pro' | 'enterprise' |
| import { log } from '../config/logger' | ||
|
|
||
| export type UserTier = 'free' | 'pro' | 'enterprise' | ||
| type UserTier = 'free' | 'pro' | 'enterprise' |
Contributor
There was a problem hiding this comment.
The UserTier type is used by the exported interface BrowserRequestContext, but it is no longer exported itself. This will cause a build error in any module that imports BrowserRequestContext. Please re-add the export keyword.
Suggested change
| type UserTier = 'free' | 'pro' | 'enterprise' | |
| export type UserTier = 'free' | 'pro' | 'enterprise' |
| import { log } from '../config/logger' | ||
|
|
||
| export type UserTier = 'free' | 'pro' | 'enterprise' | ||
| type UserTier = 'free' | 'pro' | 'enterprise' |
Contributor
There was a problem hiding this comment.
The UserTier type is used by the exported interface CalculatorToolContext, but it is no longer exported itself. This will cause a build error in any module that imports CalculatorToolContext. Please re-add the export keyword.
Suggested change
| type UserTier = 'free' | 'pro' | 'enterprise' | |
| export type UserTier = 'free' | 'pro' | 'enterprise' |
|
|
||
| tools: { confirmationTool }, | ||
| options: {}, | ||
| defaultNetworkOptions: { autoResumeSuspendedTools: true } as unknown as any, |
Contributor
There was a problem hiding this comment.
The type assertion as unknown as any completely bypasses TypeScript's type safety and should be avoided. It can hide potential bugs and makes the code harder to maintain. Please define a proper type for defaultNetworkOptions or use a more specific type assertion if the type from the library is incorrect.
|
|
||
| --- | ||
|
|
||
| If you want, I can create the example API route and the client page in the repo now (including a small sample dataset and a Playwright test). Which location would you prefer for the example page: `app/examples/chart-viewer` or `app/chat/examples/chart-viewer`? No newline at end of file |
Contributor
There was a problem hiding this comment.
This final paragraph appears to be a conversational prompt from an AI assistant rather than part of the technical documentation. It should be removed to keep the document focused and professional.
| outputSchema: z.object({ | ||
| success: z.boolean(), | ||
| candles: z.any().optional(), | ||
| candles: z.array(z.object({ open: z.number(), high: z.number(), low: z.number(), close: z.number() })).optional(), |
Contributor
There was a problem hiding this comment.
The output schema for candles has been improved from z.any() to a specific object shape. This is a great change for type safety. However, the map function on line 1508 uses any for the candle object c. To fully leverage the improved schema, you should define an interface for the raw candle object from heikinashi if possible, or at least use a more specific type than any.
Comment on lines
+1866
to
+1887
| patterns.abandonedBaby = Boolean(abandonedbaby(inputData)) | ||
| patterns.doji = Boolean(doji(inputData)) | ||
| patterns.darkCloudCover = Boolean(darkcloudcover(inputData)) | ||
| patterns.downsideTasukiGap = Boolean(downsidetasukigap(inputData)) | ||
| patterns.dragonflyDoji = Boolean(dragonflydoji(inputData)) | ||
| patterns.eveningDojiStar = Boolean(eveningdojistar(inputData)) | ||
| patterns.eveningStar = Boolean(eveningstar(inputData)) | ||
| patterns.gravestoneDoji = Boolean(gravestonedoji(inputData)) | ||
| patterns.hangingMan = Boolean(hangingman(inputData)) | ||
| patterns.morningDojiStar = Boolean(morningdojistar(inputData)) | ||
| patterns.morningStar = Boolean(morningstar(inputData)) | ||
| patterns.piercingLine = Boolean(piercingline(inputData)) | ||
| patterns.bullishHarami = Boolean(bullishharami(inputData)) | ||
| patterns.bearishHarami = Boolean(bearishharami(inputData)) | ||
| patterns.bullishHaramiCross = Boolean(bullishharamicross(inputData)) | ||
| patterns.bearishHaramiCross = Boolean(bearishharamicross(inputData)) | ||
| patterns.bullishMarubozu = Boolean(bullishmarubozu(inputData)) | ||
| patterns.bearishMarubozu = Boolean(bearishmarubozu(inputData)) | ||
| patterns.bullishSpinningTop = Boolean(bullishspinningtop(inputData)) | ||
| patterns.bearishSpinningTop = Boolean(bearishspinningtop(inputData)) | ||
| patterns.threeBlackCrows = Boolean(threeblackcrows(inputData)) | ||
| patterns.threeWhiteSoldiers = Boolean(threewhitesoldiers(inputData)) |
Contributor
There was a problem hiding this comment.
Using Boolean() to convert the result of the pattern functions is a good simplification. However, this block of code has a lot of repetition. You could refactor this into a loop to make it more concise and maintainable. For example, you could create an array of pattern functions and their names, then iterate over it.
| outputSchema: z.object({ | ||
| success: z.boolean(), | ||
| results: z.record(z.string(), z.any()), | ||
| results: z.record(z.string(), z.unknown()), |
Contributor
There was a problem hiding this comment.
Changing the schema for results from z.record(z.string(), z.any()) to z.record(z.string(), z.unknown()) is a good improvement for type safety, as unknown is safer than any. This forces consumers of the data to perform type checks.
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds multiple new visualization and data processing tools to the AgentStack framework, along with improvements to existing tools and comprehensive test coverage.
Changes:
- Adds 7 new tools for data visualization, processing, and fetching (cytoscape, leaflet, chartjs, downsample, spatial-index, resilient-fetch, confirmation)
- Improves existing tools with better error handling, logging, and schema validation (technical-analysis, image-tool, serpapi tools)
- Integrates new graphing agents and confirmation tool across all networks
- Adds comprehensive test coverage and documentation for new features
Reviewed changes
Copilot reviewed 46 out of 48 changed files in this pull request and generated 32 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mastra/tools/confirmation.tool.ts | New tool for user confirmation requests with suspend/resume support |
| src/mastra/tools/cytoscape.tool.ts | New tool generating Cytoscape.js graph structures with validation |
| src/mastra/tools/downsample.tool.ts | New tool for downsampling time series using LTTB and other algorithms |
| src/mastra/tools/leaflet.tool.ts | New tool for generating Leaflet/GeoJSON map data with bounds clipping |
| src/mastra/tools/resilient-fetch.tool.ts | New tool for HTTP requests with retries and rate limiting |
| src/mastra/tools/spatial-index.tool.ts | New tool for RBush spatial indexing with JSON serialization |
| src/mastra/tools/chartjs.tool.ts | New tool generating Chart.js configs with technical indicators |
| src/mastra/tools/technical-analysis.tool.ts | Improved error handling, logging, and schema refinements |
| src/mastra/tools/image-tool.ts | Fixed unconditional flip bug and improved code formatting |
| src/mastra/agents/graphingAgents.ts | New agents for graph generation, technical analysis, and mapping |
| src/mastra/networks/* | Integrated confirmation tool across all networks |
| package.json | Added visualization and data processing dependencies |
| tests/* | Added comprehensive test coverage for new tools |
| docs/* | Added tracing examples and Chart.js integration documentation |
Comment on lines
+1
to
+36
| import { createTool } from "@mastra/core/tools"; | ||
| import { z } from "zod"; | ||
|
|
||
| const confirmationTool = createTool({ | ||
| id: "confirmation-tool", | ||
| description: "Requests user confirmation before proceeding", | ||
| inputSchema: z.object({ | ||
| action: z.string() | ||
| }), | ||
| outputSchema: z.object({ | ||
| confirmed: z.boolean(), | ||
| action: z.string() | ||
| }), | ||
| suspendSchema: z.object({ | ||
| message: z.string(), | ||
| action: z.string() | ||
| }), | ||
| resumeSchema: z.object({ | ||
| confirmed: z.boolean() | ||
| }), | ||
| execute: async ({ action }, context) => { | ||
| const { resumeData, suspend } = context?.agent ?? {}; | ||
|
|
||
| // Explicitly check for true to avoid nullable boolean in condition | ||
| if (resumeData?.confirmed !== true) { | ||
| return suspend?.({ | ||
| message: `Please confirm: ${action}`, | ||
| action | ||
| }); | ||
| } | ||
|
|
||
| return { confirmed: true, action }; | ||
| } | ||
| }); | ||
|
|
||
| export { confirmationTool }; |
There was a problem hiding this comment.
The confirmationTool is missing the standard Mastra tool patterns present in other tools: lifecycle hooks (onInputStart, onInputDelta, onInputAvailable, onOutput), tracing spans with SpanType.TOOL_CALL, RequestContext typing, and data-tool-progress events. This inconsistency could affect observability and debugging.
Comment on lines
+1
to
+108
| import { createTool } from '@mastra/core/tools' | ||
| import { z } from 'zod' | ||
| import RBush from 'rbush' | ||
| import { log } from '../config/logger' | ||
|
|
||
| // Each indexed item will be stored with a bbox: {minX,minY,maxX,maxY, id, data} | ||
| interface IndexedItem { | ||
| minX: number | ||
| minY: number | ||
| maxX: number | ||
| maxY: number | ||
| id: string | ||
| data?: Record<string, unknown> | ||
| } | ||
|
|
||
| export const spatialIndexTool = createTool({ | ||
| id: 'spatial-index', | ||
| description: 'Create and query RBush spatial index. Returns serialized index JSON for persistence.', | ||
| inputSchema: z.object({ | ||
| action: z.enum(['build', 'search', 'fromJSON', 'toJSON']).describe('Action to perform'), | ||
| points: z | ||
| .array( | ||
| z.object({ | ||
| id: z.string(), | ||
| lat: z.number(), | ||
| lng: z.number(), | ||
| data: z.record(z.string(), z.unknown()).optional(), | ||
| }) | ||
| ) | ||
| .optional(), | ||
| bounds: z | ||
| .object({ | ||
| southWest: z.tuple([z.number(), z.number()]), | ||
| northEast: z.tuple([z.number(), z.number()]), | ||
| }) | ||
| .optional(), | ||
| treeJSON: z.unknown().optional(), | ||
| }), | ||
| // Define a structured item schema for search results | ||
| outputSchema: z.object({ | ||
| treeJSON: z.unknown().optional(), | ||
| results: z | ||
| .array( | ||
| z.object({ | ||
| minX: z.number(), | ||
| minY: z.number(), | ||
| maxX: z.number(), | ||
| maxY: z.number(), | ||
| id: z.string(), | ||
| data: z.record(z.string(), z.unknown()).optional(), | ||
| }) | ||
| ) | ||
| .optional(), | ||
| message: z.string().optional(), | ||
| }), | ||
| execute: async (input) => { | ||
| const { action, points, bounds, treeJSON } = input | ||
|
|
||
| if (action === 'build') { | ||
| if (!points || points.length === 0) { | ||
| throw new Error('No points provided to build the spatial index') | ||
| } | ||
|
|
||
| const tree = new RBush<IndexedItem>() | ||
| const items = points.map((p) => ({ | ||
| minX: p.lng, | ||
| minY: p.lat, | ||
| maxX: p.lng, | ||
| maxY: p.lat, | ||
| id: p.id, | ||
| data: p.data, | ||
| })) | ||
| tree.load(items) | ||
| const json = tree.toJSON() | ||
| return { treeJSON: json } | ||
| } | ||
|
|
||
| if (action === 'fromJSON') { | ||
| if (treeJSON === undefined) { throw new Error('treeJSON is required for fromJSON action') } | ||
| const tree = new RBush<IndexedItem>() | ||
| tree.fromJSON(treeJSON) | ||
| return { message: 'Index loaded' } | ||
| } | ||
|
|
||
| if (action === 'search') { | ||
| if (treeJSON === undefined) { throw new Error('treeJSON is required for search action') } | ||
| if (bounds === undefined) { throw new Error('bounds is required for search action') } | ||
| const tree = new RBush<IndexedItem>() | ||
| tree.fromJSON(treeJSON as unknown) | ||
| const [swLat, swLng] = bounds.southWest | ||
| const [neLat, neLng] = bounds.northEast | ||
| const rawResults = tree.search({ minX: swLng, minY: swLat, maxX: neLng, maxY: neLat }) | ||
| const results = rawResults.map((r: IndexedItem) => ({ | ||
| minX: Number(r.minX), | ||
| minY: Number(r.minY), | ||
| maxX: Number(r.maxX), | ||
| maxY: Number(r.maxY), | ||
| id: String(r.id), | ||
| data: r.data, | ||
| })) | ||
| return { results } | ||
| } | ||
|
|
||
| return { message: 'No-op' } | ||
| }, | ||
| }) | ||
|
|
||
| export type SpatialIndexUITool = typeof spatialIndexTool |
There was a problem hiding this comment.
The spatial index tool is missing the standard Mastra tool patterns: RequestContext interface, tracing spans with SpanType.TOOL_CALL, abortSignal checks, lifecycle hooks, and data-tool-progress events. This is inconsistent with other tools in the codebase and reduces observability.
Comment on lines
+16
to
+40
| export class QualityChecker implements Processor { | ||
| id = "quality-checker"; | ||
|
|
||
| processOutputStep(args: ProcessOutputStepArgs<unknown>): ProcessorMessageResult { | ||
| const { text, abort, retryCount } = args; | ||
|
|
||
| // If there's no text to evaluate, do nothing. | ||
| if (typeof text !== 'string' || text.length === 0) { | ||
| return []; | ||
| } | ||
|
|
||
| // evaluateQuality may be asynchronous; return the promise directly to match the expected return type | ||
| return evaluateQuality(text).then((score) => { | ||
| if (score < 0.7 && (retryCount ?? 0) < 3) { | ||
| // Request retry with feedback for the LLM | ||
| abort("Response quality too low. Please be more specific.", { | ||
| retry: true, | ||
| metadata: { score }, | ||
| }); | ||
| } | ||
|
|
||
| return []; | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The QualityChecker processor implementation has a type mismatch. The processOutputStep method returns ProcessorMessageResult which should be an array or Promise resolving to an array, but the implementation wraps the evaluateQuality promise which resolves to void after calling abort. The return statement inside the then() returns an empty array, but this pattern is confusing and may not work as intended with the Processor interface.
Comment on lines
+85
to
+90
| if (action === 'search') { | ||
| if (treeJSON === undefined) { throw new Error('treeJSON is required for search action') } | ||
| if (bounds === undefined) { throw new Error('bounds is required for search action') } | ||
| const tree = new RBush<IndexedItem>() | ||
| tree.fromJSON(treeJSON as unknown) | ||
| const [swLat, swLng] = bounds.southWest |
There was a problem hiding this comment.
The spatial index tool lacks input validation for the treeJSON parameter when it's used. The tool checks if treeJSON === undefined but doesn't validate that it's a valid RBush JSON structure before calling tree.fromJSON(treeJSON), which could cause runtime errors with malformed input.
Comment on lines
+1
to
+140
| import { createTool } from '@mastra/core/tools' | ||
| import { SpanType } from '@mastra/core/observability' | ||
| import { z } from 'zod' | ||
| import { log } from '../config/logger' | ||
| import axios from 'axios' | ||
| import axiosRetry from 'axios-retry' | ||
| import Bottleneck from 'bottleneck' | ||
|
|
||
| // Create a shared limiter instance (could be configurable per tool instance if needed) | ||
| const limiter = new Bottleneck({ | ||
| minTime: 200, // 5 requests per second | ||
| maxConcurrent: 5, | ||
| }) | ||
|
|
||
| // Configure axios with retry | ||
| const client = axios.create() | ||
| axiosRetry(client, { | ||
| retries: 3, | ||
| retryDelay: axiosRetry.exponentialDelay, | ||
| retryCondition: (error) => { | ||
| return ( | ||
| axiosRetry.isNetworkOrIdempotentRequestError(error) || | ||
| error.response?.status === 429 | ||
| ) | ||
| }, | ||
| }) | ||
|
|
||
| export const resilientFetchTool = createTool({ | ||
| id: 'resilient-fetch', | ||
| description: | ||
| 'Fetches data from URLs with automatic retries and rate limiting', | ||
| inputSchema: z.object({ | ||
| url: z.string().url(), | ||
| method: z.enum(['GET', 'POST', 'PUT', 'DELETE']).default('GET'), | ||
| data: z.record(z.string(), z.unknown()).optional(), | ||
| headers: z.record(z.string(), z.string()).optional(), | ||
| priority: z | ||
| .number() | ||
| .min(0) | ||
| .max(9) | ||
| .default(5) | ||
| .describe('Priority (0-9, higher is more important)'), | ||
| }), | ||
| outputSchema: z.object({ | ||
| status: z.number(), | ||
| data: z.unknown(), | ||
| headers: z.record(z.string(), z.unknown()), | ||
| }), | ||
| execute: async (input, context) => { | ||
| const { url, method, data, headers, priority } = input | ||
|
|
||
| await context?.writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'in-progress', | ||
| message: `Scheduling ${method} request to ${url}`, | ||
| stage: 'resilient-fetch', | ||
| }, | ||
| id: 'resilient-fetch', | ||
| }) | ||
|
|
||
| try { | ||
| // Schedule the request through bottleneck | ||
| const response = await limiter.schedule({ priority }, () => | ||
| client.request({ | ||
| url, | ||
| method, | ||
| data, | ||
| headers, | ||
| }) | ||
| ) | ||
|
|
||
| await context?.writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'done', | ||
| message: `Request complete: ${response.status} ${response.statusText}`, | ||
| stage: 'resilient-fetch', | ||
| }, | ||
| id: 'resilient-fetch', | ||
| }) | ||
|
|
||
| return { | ||
| status: response.status, | ||
| data: response.data, | ||
| headers: response.headers as Record<string, any>, | ||
| } | ||
| } catch (error: any) { | ||
| const errorMessage = error.response | ||
| ? `${error.response.status} - ${JSON.stringify(error.response.data)}` | ||
| : error.message | ||
|
|
||
| log.error('Resilient fetch failed', { | ||
| url, | ||
| method, | ||
| error: errorMessage, | ||
| }) | ||
|
|
||
| await context?.writer?.custom({ | ||
| type: 'data-tool-progress', | ||
| data: { | ||
| status: 'done', | ||
| message: `Request failed: ${errorMessage}`, | ||
| stage: 'resilient-fetch', | ||
| }, | ||
| id: 'resilient-fetch', | ||
| }) | ||
|
|
||
| throw new Error(`Request failed: ${errorMessage}`) | ||
| } | ||
| }, | ||
| onInputStart: ({ toolCallId, messages, abortSignal }) => { | ||
| log.info('Resilient fetch tool input streaming started', { | ||
| toolCallId, | ||
| hook: 'onInputStart', | ||
| }) | ||
| }, | ||
| onInputDelta: ({ inputTextDelta, toolCallId, messages, abortSignal }) => { | ||
| log.info('Resilient fetch tool received input chunk', { | ||
| toolCallId, | ||
| inputTextDelta, | ||
| hook: 'onInputDelta', | ||
| }) | ||
| }, | ||
| onInputAvailable: ({ input, toolCallId, messages, abortSignal }) => { | ||
| log.info('Resilient fetch tool received input', { | ||
| toolCallId, | ||
| inputData: { url: input.url, method: input.method }, | ||
| hook: 'onInputAvailable', | ||
| }) | ||
| }, | ||
| onOutput: ({ output, toolCallId, toolName, abortSignal }) => { | ||
| log.info('Resilient fetch tool completed', { | ||
| toolCallId, | ||
| toolName, | ||
| outputData: { status: output.status }, | ||
| hook: 'onOutput', | ||
| }) | ||
| }, | ||
| }) |
There was a problem hiding this comment.
The resilient fetch tool is missing RequestContext typing and tracing spans with SpanType.TOOL_CALL. While it has lifecycle hooks, it lacks the tracing instrumentation pattern used in other tools for observability.
| it('downsamples to target points using lttb', async () => { | ||
| const values = Array.from({ length: 1000 }, (_, i) => Math.sin(i / 10)) | ||
| const target = 100 | ||
| const res = (await downsampleTool.execute({ values, target, algorithm: 'lttb' })) as any |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
| it('downsamples with min-max preserving peaks', async () => { | ||
| // create a signal with spikes | ||
| const values = Array.from({ length: 1000 }, (_, i) => (i % 50 === 0 ? 100 : Math.sin(i / 10))) | ||
| const res = (await downsampleTool.execute({ values, target: 100, algorithm: 'min-max' })) as any |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
| const treeJSON = build.treeJSON | ||
| const res = await spatialIndexTool.execute({ action: 'search', treeJSON, bounds: { southWest: [5, 5], northEast: [25, 25] } }) | ||
| expect(res.results).toBeDefined() | ||
| expect((res.results as any[]).some((r) => r.id === 'a')).toBe(true) |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
| const res = await spatialIndexTool.execute({ action: 'search', treeJSON, bounds: { southWest: [5, 5], northEast: [25, 25] } }) | ||
| expect(res.results).toBeDefined() | ||
| expect((res.results as any[]).some((r) => r.id === 'a')).toBe(true) | ||
| expect((res.results as any[]).some((r) => r.id === 'b')).toBe(true) |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
| expect(res.results).toBeDefined() | ||
| expect((res.results as any[]).some((r) => r.id === 'a')).toBe(true) | ||
| expect((res.results as any[]).some((r) => r.id === 'b')).toBe(true) | ||
| expect((res.results as any[]).some((r) => r.id === 'c')).toBe(false) |
There was a problem hiding this comment.
Unexpected any. Specify a different type.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 51
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
src/mastra/tools/calendar-tool.ts (3)
118-246: ReordercreateTool()properties: hooks should come afterexecute(per repo guidelines).
listEventscurrently definesonInput*/onOutputbeforeexecute, unlike the other tools in this file and contrary to the stated tool pattern.
122-125:listEventsadvertisesstartDatefiltering but ignores it (behavior/contract mismatch).Either (a) filter
eventsbased oninputData.startDate, or (b) removestartDatefrom the schema/description (or add anendDateand apply both).Proposed minimal fix (filter in JS after reading events)
try { const events = await reader.getEvents() + const startDateFilter = new Date(inputData.startDate) + const filteredEvents = Number.isNaN(startDateFilter.getTime()) + ? events + : events.filter((e) => e.startDate >= startDateFilter) - const formattedEvents = events.map((event) => ({ + const formattedEvents = filteredEvents.map((event) => ({ title: event.title || '', startDate: event.startDate.toISOString(), endDate: event.endDate.toISOString(), location: event.location, description: event.description, })) - log.info(`Found ${events.length} calendar events`) + log.info(`Found ${filteredEvents.length} calendar events`) ... return { content: JSON.stringify(formattedEvents, null, 2), events: formattedEvents, - count: events.length, + count: filteredEvents.length, } } catch (e) {Also applies to: 200-236
178-186: Tracing metadatatool.idshould match the actual tool id.Right now the tool id is
'listEvents', but the span metadata sets'tool.id': 'list-calendar-events', which can fragment traces/metrics.Proposed fix
const span = tracingContext?.currentSpan?.createChildSpan({ type: SpanType.TOOL_CALL, name: 'list-calendar-events', input: inputData, metadata: { - 'tool.id': 'list-calendar-events', + 'tool.id': 'listEvents', 'tool.input.startDate': inputData.startDate, 'user.id': requestCtx?.userId, }, })src/mastra/a2a/codingA2ACoordinator.ts (1)
10-23: Avoid mixed import styles from../tools/e2bin the same file.Now that you’re explicitly importing the tool symbols, consider consolidating to a single import pattern to reduce drift/confusion (namespace import vs explicit named imports).
package.json (1)
114-225: Supply chain risk: address unmaintaineddownsample-lttb@0.0.1before merge.
downsample-lttb@0.0.1has not been updated for ~10 years and is unmaintained. While no published security advisories exist, the lack of maintenance presents a supply chain risk. Either replace it with an actively maintained alternative, upgrade to a newer version if available, or explicitly document the risk and runnpm auditto verify no transitive vulnerabilities are introduced.
🤖 Fix all issues with AI agents
In @docs/components/chartjs-integration.md:
- Around line 23-29: Update the install instructions to separate required vs
optional packages: change the npm install line to "npm install chart.js
react-chartjs-2 chartjs-adapter-date-fns", move "chartjs-plugin-zoom" into an
optional install command, and remove "downsample-lttb" from the client install;
then update the bullets to mark chartjs-plugin-zoom as optional and add a short
note that "downsample" / "downsample-lttb" are server-side dependencies used by
the tool and do not need to be installed in the client application.
- Around line 230-231: Remove the draft conversational sentence starting "If you
want, I can create the example API route and the client page in the repo now..."
(the question about creating the example and choosing between
`app/examples/chart-viewer` or `app/chat/examples/chart-viewer`) from the docs
so the final content contains only the polished documentation text; ensure no
other editorial questions remain in that section.
- Around line 157-165: The example using
MastraClient.getTool('chartjs-generator') calls tool.execute(...) without error
handling or input validation; wrap the execution in a try/catch and validate
inputs before calling execute so unhandled rejections are prevented and errors
surface clearly. Specifically, before calling MastraClient.getTool and
tool.execute, ensure the args (data and indicators) are present and well-formed,
then call tool.execute inside a try block and log or surface the caught error
(e.g., via console.error or UI error handler) in the catch; reference
MastraClient, getTool, and tool.execute when applying the changes.
In @docs/opentelemetry-semantic-conventions.md:
- Around line 22-35: The "Tool-specific attributes (recommended)" section
conflates core TOOL_CALL attributes with domain-specific ones, which can mislead
implementers; update the section to first list core/general attributes for all
TOOL_CALL spans (e.g., tool.id, tool.duration_ms, tool.success, tool.error) and
then add a clearly labeled "Domain-specific examples" subsection that shows
visualization-specific attributes (tool.input.dataCount,
tool.input.indicatorsCount, tool.output.labels, tool.output.datasetCount) so
readers know those are examples for chart/visualization tools rather than
universal requirements for every TOOL_CALL.
In @next.config.ts:
- Around line 27-28: The dependency list in next.config.ts mixes single and
double quotes and crams multiple packages onto one line; update the array of
packages (the string list containing "ai", "html-to-image", "cytoscape",
"axios-retry", "bottleneck", "leaflet", "chart.js", "technicalindicators",
"simple-statistics") to use consistent double quotes, put one package per line,
and sort the entries alphabetically to improve consistency, maintainability, and
cleaner git diffs.
In @src/mastra/a2a/codingA2ACoordinator.ts:
- Around line 205-218: The tools object in codingA2ACoordinator.ts currently
exposes all tool functions (createSandbox, writeFile, writeFiles, listFiles,
deleteFile, createDirectory, getFileInfo, checkFileExists, getFileSize,
watchDirectory, runCommand, runCode) with no authorization or confirmation;
implement RBAC by checking the caller's userId/role (extracted for context)
against a permission matrix before any tool is invoked (e.g., require
"file:read", "file:write", "file:delete", "exec:run" scopes), enforce this check
inside the coordinator wrapper that returns the tools, and ensure
codingA2AMcpServer callers are validated; additionally add explicit confirmation
gates for destructive or high-risk actions (deleteFile, runCommand, runCode)
such as requiring a confirmed flag or a separate approveAction(userId, actionId)
call and reject if sandbox verification/approval is not present, and validate
each incoming request payload against the caller's allowed operations before
forwarding to the E2B Sandbox API.
In @src/mastra/agents/graphingAgents.ts:
- Line 24: The module-level log.info('Initializing Graphing Agents...') runs at
import time; move this call into a runtime initializer (e.g., create or export
an initializeGraphingAgents / initGraphingAgents function) or guard it behind a
conditional so it only executes when explicitly invoked; update references to
call that initializer where agents are actually started and remove the
standalone module-level log.info to avoid side effects during import.
- Around line 289-317: The instructions() function inside codeMetricsAgent
returns a malformed object: the template literal for the system content closes
but the surrounding returned object is not correctly closed, leaving an extra
standalone brace; fix the return value in codeMetricsAgent.instructions (the
function that accepts { requestContext }:
RequestContext<GraphingRuntimeContext>) by ensuring the returned object has
properly nested braces and commas (role and content keys inside a single
object), close the template literal and then the object literal correctly, and
match the object shape used by technicalAnalysisAgent so the function returns a
single valid object.
In @src/mastra/networks/codingTeamNetwork.ts:
- Around line 16-40: The QualityChecker class is implemented but not added to
the pipeline, so its processOutputStep never runs; open the module that defines
the outputProcessors array and insert the QualityChecker class into that array
(ensure you import or reference QualityChecker if in a different module) and
place it as the first entry in outputProcessors so quality checks run before
subsequent processors like token limiting and batching.
In @src/mastra/networks/devopsNetwork.ts:
- Line 10: The DevOps Network instructions lack guidance on when to use the
confirmationTool for high-risk operations; update the DevOps network instruction
block (the long instructions spanning lines ~18-152 and the later section around
line 165) to add a short documented policy that instructs agents to invoke
confirmationTool for sensitive actions (deployments, infra changes, rollbacks,
security config changes), specify required confirmation phrasing or scope (e.g.,
exact user approval and context), and include examples or a one-line rule for
when confirmation is mandatory; reference the confirmationTool symbol and the
DevOps Network instructions so reviewers can find and validate the added
guidance.
In @src/mastra/networks/financialIntelligenceNetwork.ts:
- Line 12: Add a short Guidelines entry documenting the confirmationTool usage
within the Financial Intelligence Network: explain when to invoke
confirmationTool (e.g., before executing transactions, recommending investments,
performing portfolio rebalancing, generating official reports, or any
high-risk/irreversible financial actions), what form the confirmation prompt
should take (clear summary, explicit yes/no, display key amounts/entities), and
note any exceptions (read-only queries, low-value test operations); update the
Financial Intelligence Network Guidelines (where confirmationTool is
imported/used in this module) and mirror the same guidance at the other
occurrence of the network to keep behavior consistent.
In @src/mastra/networks/index.ts:
- Line 44: The routing instructions for agentNetwork do not mention the newly
available confirmationTool; update the instructions block that constructs
agentNetwork to document confirmationTool's purpose and usage patterns by adding
a short paragraph describing when to call confirmationTool, expected
inputs/outputs, and examples of outcomes (e.g., confirm/cancel boolean or enum),
referencing the confirmationTool symbol and how the agent should invoke it in
routing logic; ensure the same documentation note is added to the parallel
instructions section where agentNetwork is created again (the other occurrence)
so both instances mention confirmationTool consistently.
In @src/mastra/networks/reportGenerationNetwork.ts:
- Line 15: Remove the unused confirmationTool: delete the import statement for
confirmationTool and remove it from the NetworkTools/ tools registration in
reportGenerationNetwork (where tools are configured, e.g., the array that
currently includes confirmationTool). Also inspect learningExtractionWorkflow
and other symbols in this file to confirm they do not reference confirmationTool
before removing, and run tests/build to ensure no other references remain.
In @src/mastra/networks/securityNetwork.ts:
- Line 176: Remove the unsafe "as unknown as any" double-cast on the
defaultNetworkOptions assignment; instead ensure the object literal matches the
expected type used by the Agent constructor (or update that type). Replace the
cast by either explicitly typing defaultNetworkOptions with the correct
interface/type (e.g., DefaultNetworkOptions) or adjust the Agent constructor's
parameter type to accept { autoResumeSuspendedTools: boolean }, so
defaultNetworkOptions: { autoResumeSuspendedTools: true } is typed correctly
without using any.
In @src/mastra/tools/browser-tool.ts:
- Line 10: The UserTier type is duplicated; extract a single exported type alias
(e.g., export type UserTier = 'free' | 'pro' | 'enterprise') into a
central/shared types module, remove all local UserTier declarations, and update
every file that currently defines UserTier to import that shared UserTier
instead of redefining it; after changing imports, run the TypeScript build/check
to ensure no remaining duplicates or import errors.
In @src/mastra/tools/calculator.tool.ts:
- Line 8: UserTier is duplicated across files; create a single exported type
definition (e.g., export type UserTier = 'free' | 'pro' | 'enterprise') in a
shared module (suggested symbol: src/mastra/types/common.ts) and replace local
definitions with an import of UserTier in src/mastra/tools/calculator.tool.ts,
src/mastra/agents/codingAgents.ts, src/mastra/tools/alpha-vantage.tool.ts (and
any other files defining it), ensuring each file removes its local type and
imports the shared UserTier to maintain a single source of truth.
In @src/mastra/tools/chartjs.tool.ts:
- Around line 219-260: The MACD datasets are assigned yAxisID 'y1' in the 'MACD'
case (datasets.push for 'MACD', 'MACD Signal', 'MACD Histogram') but the code
that adds the y1 scale only runs when RSI is present; update the scale-creation
logic to create/configure the 'y1' scale whenever MACD is present (or when
either RSI or MACD are requested), not just RSI. Locate the scale construction
(the code that conditionally adds the 'y1' scale) and change its condition to
check for MACD as well (e.g., presence of the 'MACD' indicator or a flag set
when handling the 'MACD' case), and ensure the y1 scale configuration uses
appropriate bounds/type for MACD (separate axis or dynamic bounds) rather than
assuming 0-100.
- Around line 320-347: The decimation catch block logs the failure via log.warn
but doesn't update the tracing span; inside the catch (the block catching e
where log.warn is called), add updates to the tracing span (e.g.,
span.setAttribute('chart.decimation.skipped', true), span.recordException(e)
and/or span.setStatus({ code: SpanStatusCode.ERROR, message: 'Decimation failed'
})) so that the trace records that decimation was skipped and captures the
exception; keep the existing log.warn and ensure span is in scope or passed into
the enclosing function that contains this decimation logic.
In @src/mastra/tools/confirmation.tool.ts:
- Around line 21-33: The execute function must emit progress events via
context?.writer?.custom() with type 'data-tool-progress'; add an "in-progress"
event at the start of execute including the input parameter action and stage
identifier equal to this tool's id, and add a "done" event before both return
paths (the suspend call and the confirmed return) with status 'done',
descriptive messages including action and the outcome (e.g., "awaiting
confirmation" or "confirmed"), using context?.writer?.custom({ type:
'data-tool-progress', status: 'in-progress'| 'done', message: string, stage:
<tool id> }) so callers can observe progress while preserving existing behavior
in execute.
- Around line 21-33: The execute function is missing tracing spans; wrap its
logic in a child span created via tracingContext?.currentSpan?.createChildSpan()
using type: SpanType.TOOL_CALL, a descriptive name (e.g.,
"confirmation.tool.execute"), and include input data and metadata with tool.id;
ensure you start the span before checking resumeData, add the input payload to
the span (action and resumeData), set metadata { "tool.id": tool.id }, and
always close/end the span in finally so it is closed on all code paths
(including when calling suspend or returning the confirmed result).
In @src/mastra/tools/cytoscape.tool.ts:
- Around line 207-233: Update the Cytoscape tool lifecycle hooks (onInputStart,
onInputDelta, onInputAvailable, onOutput) to match the Leaflet tool's richer
signature: include the messages array and abortSignal where applicable, and log
structured inputData and outputData fields instead of raw input; also ensure
onInputDelta and onInputAvailable include the full messages context and onOutput
logs outputData along with toolName and toolCallId for consistency.
- Around line 151-172: The edge mapping in edges.map is creating edges with
e.source and e.target without verifying those IDs exist in the nodeIds set;
update the logic inside edges.map to validate that both e.source and e.target
are present in the nodeIds collection (e.g., nodeIds.has(e.source) and
nodeIds.has(e.target)), and skip (or alternatively mark/log) any edges whose
source or target is missing so you don't produce broken Cytoscape edges; keep
the rest of the shape (group: 'edges', data id/default id generation using
`${e.source}-${e.target}-${idx}`, classes) intact when emitting only validated
edges.
- Around line 129-173: Wrap the elements generation (the nodes.map and edges.map
block that builds elements) in a try-catch so runtime errors during
transformation are captured; in the catch, record the exception on the current
tracing span (e.g., span.recordException(error)) and set the span status to
error (e.g., span.setStatus({ code: SpanStatusCode.ERROR, message: error.message
})), then rethrow the error so callers still receive it. Ensure you reference
the same span used for validation/tracing so the span is properly closed with
the error.
In @src/mastra/tools/downsample.tool.ts:
- Line 90: The type alias DownsampleUITool should follow the project pattern by
using the generic helper InferUITool; replace the direct typeof usage with
InferUITool<typeof downsampleTool> so it matches other aliases like
ChartJsUITool and LeafletUITool and ensures consistent typing across tools.
- Around line 37-41: LTTB x-values can round to duplicate indices; after
creating indices from decimated (the indices = decimated.map(...
Math.round(p[0])) step) deduplicate them while preserving order (e.g., filter by
first occurrence) before building sampledValues so consumers get unique indices;
also update the returned target (or document it) to reflect the deduplicated
count (use the deduped indices array instead of the original indices when
mapping sampledValues and when returning the target).
In @src/mastra/tools/image-tool.ts:
- Around line 352-367: The blur step runs even when ops.blur is undefined
because the code uses `ops.blur !== null`; change the guard to explicitly check
for presence (e.g., `ops.blur != null` or `typeof ops.blur === 'number'`) before
calling `pipeline.blur(ops.blur)` so `pipeline.blur` is only invoked when
`ops.blur` is provided and valid; update the check around the
`pipeline.blur(ops.blur)` call in image-tool.ts accordingly.
In @src/mastra/tools/leaflet.tool.ts:
- Around line 222-230: The progress event emitted in the error branch currently
sets status: 'done' which is misleading; update the writer?.custom call that
sends the data-tool-progress event (id 'leaflet-generator', stage
'leaflet-generator') to use a failure status such as 'error' or 'failed' and
keep the existing message and stage fields intact so consumers can detect
failures reliably.
- Around line 184-189: The code builds a result object with "zoom: zoom || 13"
but the zoom value already gets a default of 13 in the schema, so remove the
redundant fallback; update the result construction in the function that returns
geoJSON/center/zoom/markers to use "zoom" directly (or use the nullish
coalescing operator "zoom ?? 13" if you prefer explicit clarity) so the explicit
schema default is respected and no unreachable falsy fallback remains.
- Around line 61-103: The coordinate validation for rawPoints is outside the
try-catch and may bypass tracing/error logging; move the for-loop that checks
p.lat/p.lng into the main try block inside execute (or wrap it in its own try
that reports errors to tracingContext.currentSpan/toolSpan) so any thrown Error
is caught, logged via toolSpan/tracingContext and handled consistently with the
rest of the function (refer to execute, rawPoints, toolSpan and tracingContext
identifiers to locate where to place the validation).
In @src/mastra/tools/resilient-fetch.tool.ts:
- Around line 1-140: Summary: The resilientFetchTool has multiple critical
issues (retrying 429 for non-idempotent methods, inverted Bottleneck priority,
no abort/tracing, unsafe types, SSRF/timeout risks); update the tool
accordingly. Fix: in createTool's inputSchema (inputSchema) refine url to only
allow http/https and add a configurable timeout default (e.g., 30_000); change
axios client (client) request defaults to include timeout and response size
limits; update axiosRetry configuration (retryCondition) to only treat 429 as
retryable for idempotent methods (GET/HEAD/OPTIONS/PUT?) or require an explicit
idempotency key in input for POST/PUT/DELETE; in execute, check
abortSignal?.aborted early and pass abortSignal (as signal) into client.request
options; when scheduling with limiter.schedule({ priority }, ...) invert the
priority value (e.g., map schemaPriority => bottleneckPriority = 9 - priority)
to match the schema description or update the schema text consistently; add
tracing by creating a child span via
tracingContext?.currentSpan?.createChildSpan({ type: SpanType.TOOL_CALL, ... })
and ensure span.end() on success and span.error({ endSpan: true }) in error
paths; replace catch (error: any) with catch (error: unknown) and use type
guards to extract response/status/message safely, and change output headers type
to Record<string, string | string[]> (not any); finally add SSFR protection by
validating host/port if needed and ensure all newly added behaviors reference
resilientFetchTool, execute, client, limiter.schedule, retryCondition,
inputSchema, and tracing span usage so reviewers can find the changes.
In @src/mastra/tools/serpapi-news-trends.tool.ts:
- Line 128: The local variable requestContext in serpapi-news-trends.tool.ts is
cast to SerpApiNewsContext | undefined but never used; either remove the line
"const requestContext = context?.requestContext as SerpApiNewsContext |
undefined" to eliminate the unused variable, or if per-request context will be
needed later, add a short comment above the declaration explaining its purpose
and then use requestContext where appropriate (for example in fetch/query calls
inside the function), ensuring linting and type-checking no longer flag it as
unused.
In @src/mastra/tools/spatial-index.tool.ts:
- Line 89: The call uses an unnecessary double cast: remove the redundant "as
unknown" and pass treeJSON directly to tree.fromJSON (i.e., change
tree.fromJSON(treeJSON as unknown) to tree.fromJSON(treeJSON)); update any
surrounding type assumptions if the compiler complains by ensuring the schema
already types treeJSON as unknown or narrow its type before calling fromJSON,
referencing the tree.fromJSON invocation and the treeJSON variable.
- Around line 16-38: The input schema for spatialIndexTool declares an unhandled
'toJSON' action in the action enum and uses z.unknown() for treeJSON; either
remove 'toJSON' from the z.enum in spatialIndexTool or implement the handler
inside the tool's execute function to return the RBush serialized tree (e.g.,
call tree.toJSON() / RBush export) when action === 'toJSON'; also tighten
treeJSON's type from z.unknown() to a more specific schema such as
z.record(z.string(), z.unknown()) (or the exact shape returned by
RBush.toJSON()) so runtime validation will catch invalid input when handling
actions like 'fromJSON'.
- Around line 56-106: The execute handler in the spatial-index tool must add
tracing, progress events, abort checks, and standardized logging: create a child
span via tracingContext?.currentSpan?.createChildSpan(SpanType.TOOL_CALL) at the
start of execute and finish it in a finally block; call logToolExecution(...)
with the span and input/action before heavy work; before any long or blocking
operations (building/loading/searching the RBush in execute, and before
tree.load, tree.fromJSON, and tree.search) check abortSignal?.aborted and throw
or return early; emit progress updates via context?.writer?.custom({ type:
'data-tool-progress', progress: <number>, message: <string> }) at key steps
(start, after load/fromJSON, before/after search); apply these changes across
the action branches for 'build', 'fromJSON', and 'search' using the existing
symbols RBush, execute, tree.load, tree.fromJSON, tree.search and ensure the
child span is properly closed in all paths.
In @src/mastra/tools/technical-analysis.tool.ts:
- Around line 238-255: The error path for the "ichimoku-cloud" block is missing
duration logging; compute the elapsed time (e.g., const duration = Date.now() -
startTime or capture a start timestamp at the top of the tool if not present)
and include that duration in the error reporting by passing it to
toolSpan?.error (or ending the span with duration), adding it to the
logToolExecution call for 'ichimoku-cloud' (include { duration }), and include
duration in the writer?.custom payload so the error branch mirrors the
fibonacciTool duration logging.
In @src/mastra/tools/tests/chartjs.integration.test.ts:
- Around line 4-11: Add three additional tests around chartJsTool.execute to
cover non-decimation, decimation failure, and indicators preservation: (1)
create a dataset with fewer than 5000 points, call chartJsTool.execute({ data,
indicators: [], chartType: 'line' }) and assert the returned (res as
any).config.data.labels length equals input length (no downsampling); (2)
simulate the downsample tool failing (mock the internal downsample invocation or
inject a failure) then call chartJsTool.execute and assert it returns a valid
chart config without throwing and that labels length is handled gracefully
(fallback behavior); (3) include one or more indicator series in the input, call
chartJsTool.execute and assert that decimation (labels length reduced) is
applied consistently across main dataset and each indicator series in (res as
any).config.data.datasets. Use existing test structure and names
(chartJsTool.execute, config.data.labels, config.data.datasets) so the new tests
integrate with the current suite.
- Around line 8-9: The test currently casts res to any to access
config.data.labels; replace that unsafe cast by validating/typing the tool
output with the proper outputSchema or explicit result type: import or reference
the tool's output type (or call outputSchema.parse/validate on res) and then
access (validated).config.data.labels so labels is strongly typed (avoid any)
and keep the existing assertion on labels.length.
In @src/mastra/tools/tests/downsample.tool.test.ts:
- Around line 1-34: Remove the unsafe "as any" casts on the results of
downsampleTool.execute; instead declare res with a proper type such as
Awaited<ReturnType<typeof downsampleTool.execute>> (or a specific
DownsampleResult interface) or cast to unknown then narrow before accessing
.values/.indices, and update the two tests that use "as any" to use that typed
res so TypeScript checks properties without using any.
In @src/mastra/tools/tests/spatial-index.tool.test.ts:
- Around line 4-22: Add unit tests for spatialIndexTool to cover error paths and
additional actions: write tests that call spatialIndexTool.execute with action
'build' and an empty points array and assert it throws or returns an error; call
'search' with missing or malformed bounds and with invalid treeJSON and assert
proper error handling; add a test for the 'fromJSON' action using a known
treeJSON and verify it reconstructs the same tree and supports searches; include
edge-case tests for points on boundary coordinates and duplicate IDs to assert
expected inclusion/exclusion behavior; use spatialIndexTool.execute, checking
returned error/results shapes and any thrown exceptions to validate all cases.
- Around line 15-21: Replace the unsafe any[] assertions by using the tool's
declared output type from spatialIndexTool.execute (or the outputSchema/type
exported by the spatial index tool) so results are strongly typed; e.g., declare
const typedRes = res as Awaited<ReturnType<typeof spatialIndexTool.execute>> (or
import the tool's Result/Output type) and then use (typedRes.results as
YourResultType[]).some(r => r.id === 'a') etc., updating the three expect lines
to use the proper Result type instead of any[].
In @tests/test-results/test-results.json:
- Line 1: The committed test artifact tests/test-results/test-results.json
contains Windows absolute user paths (e.g., "C:/Users/ssdsk/..."); remove the
PII by either excluding this file from VCS (add
tests/test-results/test-results.json or tests/test-results/ to .gitignore and
remove the tracked file), or sanitize the JSON by replacing absolute paths with
repo-relative names (e.g., only keep test file basenames or repo-relative paths
in the "name" fields) and commit the sanitized file; ensure CI regenerates full
artifacts if needed rather than committing local machine paths.
In @thoughts/ledgers/CONTINUITY_ses_456e.md:
- Around line 46-47: The markdown has a fenced code block that lacks a blank
line above it; add a single blank line between the preceding paragraph/header
("**Pattern to implement** (from AGENTS.md):") and the opening ```typescript
fence so the block renders correctly and aligns with the project's Markdown
whitespace guidelines; ensure the blank line is added immediately before the
line containing ```typescript and that surrounding spacing follows the same
pattern elsewhere in the file.
Comment on lines
+23
to
+29
| ```bash | ||
| npm install chart.js react-chartjs-2 chartjs-adapter-date-fns chartjs-plugin-zoom downsample-lttb | ||
| ``` | ||
|
|
||
| - `chartjs-adapter-date-fns` provides date parsing for time axes | ||
| - `chartjs-plugin-zoom` enables client zoom/pan interactions when desired | ||
| - `downsample-lttb` is a server-side friendly LTTB downsampling helper (useful for very large datasets) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify which dependencies are required vs. optional.
The prerequisites section lists downsample-lttb in the install command alongside Chart.js core dependencies, but it's only needed for server-side downsampling of large datasets (which happens in the tool, not the client). This may confuse users about what's actually required for basic chart rendering.
♻️ Suggested clarification
```bash
-npm install chart.js react-chartjs-2 chartjs-adapter-date-fns chartjs-plugin-zoom downsample-lttb
+npm install chart.js react-chartjs-2 chartjs-adapter-date-fns
+
+# Optional: for interactive zoom/pan
+npm install chartjs-plugin-zoom
```
- `chartjs-adapter-date-fns` provides date parsing for time axes
-- `chartjs-plugin-zoom` enables client zoom/pan interactions when desired
-- `downsample-lttb` is a server-side friendly LTTB downsampling helper (useful for very large datasets)
+- `chartjs-plugin-zoom` (optional) enables client zoom/pan interactions
+Note: The server-side tool uses `downsample` and `downsample-lttb` internally for large dataset optimization. These are server dependencies only and do not need to be installed in your client application.🧰 Tools
🪛 LanguageTool
[style] ~29-~29: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ly LTTB downsampling helper (useful for very large datasets) 4. (Optional) Install zod ...
(EN_WEAK_ADJECTIVE)
🤖 Prompt for AI Agents
In @docs/components/chartjs-integration.md around lines 23 - 29, Update the
install instructions to separate required vs optional packages: change the npm
install line to "npm install chart.js react-chartjs-2 chartjs-adapter-date-fns",
move "chartjs-plugin-zoom" into an optional install command, and remove
"downsample-lttb" from the client install; then update the bullets to mark
chartjs-plugin-zoom as optional and add a short note that "downsample" /
"downsample-lttb" are server-side dependencies used by the tool and do not need
to be installed in the client application.
Comment on lines
+97
to
+145
| // app/examples/chart-viewer/page.tsx | ||
| 'use client' | ||
|
|
||
| import { useEffect, useState } from 'react' | ||
| import { Chart } from 'react-chartjs-2' | ||
| import type { ChartConfiguration } from 'chart.js' | ||
|
|
||
| export default function ChartViewer({ sampleData }) { | ||
| const [config, setConfig] = useState<ChartConfiguration | null>(null) | ||
| const [loading, setLoading] = useState(false) | ||
| const [error, setError] = useState<string | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| if (!sampleData) return | ||
|
|
||
| async function fetchConfig() { | ||
| setLoading(true) | ||
| setError(null) | ||
| try { | ||
| const res = await fetch('/api/chart-config', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(sampleData), | ||
| }) | ||
| const json = await res.json() | ||
| if (!res.ok) throw new Error(json?.error || 'Failed to generate chart config') | ||
|
|
||
| setConfig(json.config) | ||
| } catch (e: any) { | ||
| setError(e.message) | ||
| } finally { | ||
| setLoading(false) | ||
| } | ||
| } | ||
|
|
||
| fetchConfig() | ||
| }, [sampleData]) | ||
|
|
||
| if (loading) return <div>Generating chart…</div> | ||
| if (error) return <div>Error: {error}</div> | ||
| if (!config) return <div>No chart to show</div> | ||
|
|
||
| return ( | ||
| <div> | ||
| <Chart type={config.type as any} data={config.data as any} options={config.options as any} /> | ||
| </div> | ||
| ) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Missing Chart.js component registration will cause runtime errors.
The client component example uses react-chartjs-2 but doesn't register Chart.js components before rendering. Chart.js 3+ requires explicit registration of chart types, scales, and plugins. Without registration, the <Chart> component will throw errors at runtime.
🔧 Proposed fix with proper Chart.js registration
// app/examples/chart-viewer/page.tsx
'use client'
import { useEffect, useState } from 'react'
-import { Chart } from 'react-chartjs-2'
+import { Chart as ChartComponent } from 'react-chartjs-2'
import type { ChartConfiguration } from 'chart.js'
+import {
+ Chart,
+ CategoryScale,
+ LinearScale,
+ TimeScale,
+ LineElement,
+ BarElement,
+ PointElement,
+ LineController,
+ BarController,
+ Tooltip,
+ Legend,
+ Filler,
+} from 'chart.js'
+import 'chartjs-adapter-date-fns'
+
+// Register Chart.js components
+Chart.register(
+ CategoryScale,
+ LinearScale,
+ TimeScale,
+ LineElement,
+ BarElement,
+ PointElement,
+ LineController,
+ BarController,
+ Tooltip,
+ Legend,
+ Filler
+)
export default function ChartViewer({ sampleData }) {
const [config, setConfig] = useState<ChartConfiguration | null>(null)
// ... rest of component
return (
<div>
- <Chart type={config.type as any} data={config.data as any} options={config.options as any} />
+ <ChartComponent type={config.type} data={config.data} options={config.options} />
</div>
)
}Additionally, the as any type assertions are unnecessary if config is properly typed as ChartConfiguration.
Comment on lines
+157
to
+165
| Example direct call (not recommended for production): | ||
|
|
||
| ```ts | ||
| import { MastraClient } from '@mastra/client-js' | ||
| const mastraClient = new MastraClient({ baseUrl: process.env.NEXT_PUBLIC_MASTRA_API_URL }) | ||
| const tool = mastraClient.getTool('chartjs-generator') | ||
| const result = await tool.execute({ args: { data: ..., indicators: [...] } }) | ||
| // result.config -> render | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add error handling to direct client-side execution example.
The alternative client-side tool execution example lacks error handling and input validation, which could lead to unhandled promise rejections and unclear error states for users.
♻️ Enhanced example with error handling
Example direct call (not recommended for production):
```ts
import { MastraClient } from '@mastra/client-js'
+
const mastraClient = new MastraClient({ baseUrl: process.env.NEXT_PUBLIC_MASTRA_API_URL })
-const tool = mastraClient.getTool('chartjs-generator')
-const result = await tool.execute({ args: { data: ..., indicators: [...] } })
-// result.config -> render
+
+try {
+ const tool = mastraClient.getTool('chartjs-generator')
+ const result = await tool.execute({
+ args: {
+ data: /* your data */,
+ indicators: [/* your indicators */]
+ }
+ })
+ // result.config -> render in your component
+} catch (error) {
+ console.error('Failed to generate chart:', error)
+ // Handle error appropriately in your UI
+}</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @docs/components/chartjs-integration.md around lines 157 - 165, The example
using MastraClient.getTool('chartjs-generator') calls tool.execute(...) without
error handling or input validation; wrap the execution in a try/catch and
validate inputs before calling execute so unhandled rejections are prevented and
errors surface clearly. Specifically, before calling MastraClient.getTool and
tool.execute, ensure the args (data and indicators) are present and well-formed,
then call tool.execute inside a try block and log or surface the caught error
(e.g., via console.error or UI error handler) in the catch; reference
MastraClient, getTool, and tool.execute when applying the changes.
</details>
<!-- fingerprinting:phantom:poseidon:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
Comment on lines
+230
to
+231
|
|
||
| If you want, I can create the example API route and the client page in the repo now (including a small sample dataset and a Playwright test). Which location would you prefer for the example page: `app/examples/chart-viewer` or `app/chat/examples/chart-viewer`? No newline at end of file |
There was a problem hiding this comment.
Remove conversational question from final documentation.
Lines 230-231 contain a conversational question ("If you want, I can create the example API route...") that should be removed before merging. This reads like a draft or work-in-progress note, not final documentation.
📝 Proposed fix
-If you want, I can create the example API route and the client page in the repo now (including a small sample dataset and a Playwright test). Which location would you prefer for the example page: `app/examples/chart-viewer` or `app/chat/examples/chart-viewer`?
+See the example files in this repository for complete, working implementations of the patterns described above.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| If you want, I can create the example API route and the client page in the repo now (including a small sample dataset and a Playwright test). Which location would you prefer for the example page: `app/examples/chart-viewer` or `app/chat/examples/chart-viewer`? | |
| See the example files in this repository for complete, working implementations of the patterns described above. |
🧰 Tools
🪛 LanguageTool
[style] ~230-~230: This phrasing can be overused. Try elevating your writing with a more formal alternative.
Context: ...at fetches and renders the config --- If you want, I can create the example API route and...
(IF_YOU_WANT)
🤖 Prompt for AI Agents
In @docs/components/chartjs-integration.md around lines 230 - 231, Remove the
draft conversational sentence starting "If you want, I can create the example
API route and the client page in the repo now..." (the question about creating
the example and choosing between `app/examples/chart-viewer` or
`app/chat/examples/chart-viewer`) from the docs so the final content contains
only the polished documentation text; ensure no other editorial questions remain
in that section.
Comment on lines
+22
to
+35
| ### Tool-specific attributes (recommended) | ||
|
|
||
| When instrumenting tool execution (SpanType.TOOL_CALL), include the following attributes to make traces actionable and measurable: | ||
|
|
||
| • `tool.id` - Tool identifier (e.g., `chartjs-generator`) | ||
| • `tool.input.dataCount` - Numeric size of the input (rows, points, documents) | ||
| • `tool.input.indicatorsCount` - Example: number of indicators requested for chart tool | ||
| • `tool.output.labels` - Example: number of labels produced (visualization size) | ||
| • `tool.output.datasetCount` - Number of datasets produced | ||
| • `tool.duration_ms` - Duration of the tool execution (ms) | ||
| • `tool.success` - boolean (true/false) | ||
| • `tool.error` - Error message or code when `tool.success` is false | ||
|
|
||
| These attributes should be added to the TOOL_CALL span's `metadata` or `attributes` so they appear in trace exporters and dashboards. |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Tool-specific attributes appear domain-specific, not general.
The documented attributes (tool.input.indicatorsCount, tool.output.labels, tool.output.datasetCount) are specific to chart/visualization tools, but the section title "Tool-specific attributes (recommended)" implies these are general recommendations for all TOOL_CALL spans. This could mislead developers implementing other tool types (e.g., file operations, API calls, database queries) into thinking these specific attributes are universally applicable.
Consider one of these approaches:
- Clarify domain specificity:
-### Tool-specific attributes (recommended)
+### Tool-specific attributes (recommended examples for visualization tools)
-When instrumenting tool execution (SpanType.TOOL_CALL), include the following attributes to make traces actionable and measurable:
+When instrumenting visualization tool execution (SpanType.TOOL_CALL), consider including domain-specific attributes like:- Provide general attributes first, then domain-specific examples:
### Tool-specific attributes (recommended)
Core attributes for all tool calls:
• `tool.id` - Tool identifier (e.g., `chartjs-generator`)
• `tool.duration_ms` - Duration of the tool execution (ms)
• `tool.success` - boolean (true/false)
• `tool.error` - Error message or code when `tool.success` is false
Domain-specific examples (for visualization tools):
• `tool.input.dataCount` - Number of data points processed
• `tool.input.indicatorsCount` - Number of indicators requested
• `tool.output.labels` - Number of labels produced
• `tool.output.datasetCount` - Number of datasets produced🤖 Prompt for AI Agents
In @docs/opentelemetry-semantic-conventions.md around lines 22 - 35, The
"Tool-specific attributes (recommended)" section conflates core TOOL_CALL
attributes with domain-specific ones, which can mislead implementers; update the
section to first list core/general attributes for all TOOL_CALL spans (e.g.,
tool.id, tool.duration_ms, tool.success, tool.error) and then add a clearly
labeled "Domain-specific examples" subsection that shows visualization-specific
attributes (tool.input.dataCount, tool.input.indicatorsCount,
tool.output.labels, tool.output.datasetCount) so readers know those are examples
for chart/visualization tools rather than universal requirements for every
TOOL_CALL.
Comment on lines
+4
to
+22
| describe('spatialIndexTool', () => { | ||
| it('builds and searches index', async () => { | ||
| const points = [ | ||
| { id: 'a', lat: 10, lng: 10, data: { name: 'a' } }, | ||
| { id: 'b', lat: 20, lng: 20, data: { name: 'b' } }, | ||
| { id: 'c', lat: 30, lng: 30, data: { name: 'c' } }, | ||
| ] | ||
|
|
||
| const build = await spatialIndexTool.execute({ action: 'build', points }) | ||
| expect(build.treeJSON).toBeDefined() | ||
|
|
||
| const treeJSON = build.treeJSON | ||
| const res = await spatialIndexTool.execute({ action: 'search', treeJSON, bounds: { southWest: [5, 5], northEast: [25, 25] } }) | ||
| expect(res.results).toBeDefined() | ||
| expect((res.results as any[]).some((r) => r.id === 'a')).toBe(true) | ||
| expect((res.results as any[]).some((r) => r.id === 'b')).toBe(true) | ||
| expect((res.results as any[]).some((r) => r.id === 'c')).toBe(false) | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Expand test coverage for error cases and additional actions.
The test only covers the happy path. As per coding guidelines, tests should cover both success and error paths. Consider adding tests for:
- Error cases: empty points array, missing bounds, invalid treeJSON
- The
fromJSONaction (currently untested) - Edge cases: points at boundary limits, duplicate IDs
🤖 Prompt for AI Agents
In @src/mastra/tools/tests/spatial-index.tool.test.ts around lines 4 - 22, Add
unit tests for spatialIndexTool to cover error paths and additional actions:
write tests that call spatialIndexTool.execute with action 'build' and an empty
points array and assert it throws or returns an error; call 'search' with
missing or malformed bounds and with invalid treeJSON and assert proper error
handling; add a test for the 'fromJSON' action using a known treeJSON and verify
it reconstructs the same tree and supports searches; include edge-case tests for
points on boundary coordinates and duplicate IDs to assert expected
inclusion/exclusion behavior; use spatialIndexTool.execute, checking returned
error/results shapes and any thrown exceptions to validate all cases.
Comment on lines
+15
to
+21
| const treeJSON = build.treeJSON | ||
| const res = await spatialIndexTool.execute({ action: 'search', treeJSON, bounds: { southWest: [5, 5], northEast: [25, 25] } }) | ||
| expect(res.results).toBeDefined() | ||
| expect((res.results as any[]).some((r) => r.id === 'a')).toBe(true) | ||
| expect((res.results as any[]).some((r) => r.id === 'b')).toBe(true) | ||
| expect((res.results as any[]).some((r) => r.id === 'c')).toBe(false) | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use proper typing instead of any[] type assertion.
The type assertion to any[] (lines 18-20) weakens type safety. The tool's outputSchema defines a specific result type that should be used.
🔧 Proposed fix using proper types
const treeJSON = build.treeJSON
const res = await spatialIndexTool.execute({ action: 'search', treeJSON, bounds: { southWest: [5, 5], northEast: [25, 25] } })
expect(res.results).toBeDefined()
- expect((res.results as any[]).some((r) => r.id === 'a')).toBe(true)
- expect((res.results as any[]).some((r) => r.id === 'b')).toBe(true)
- expect((res.results as any[]).some((r) => r.id === 'c')).toBe(false)
+ expect(res.results?.some((r) => r.id === 'a')).toBe(true)
+ expect(res.results?.some((r) => r.id === 'b')).toBe(true)
+ expect(res.results?.some((r) => r.id === 'c')).toBe(false)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const treeJSON = build.treeJSON | |
| const res = await spatialIndexTool.execute({ action: 'search', treeJSON, bounds: { southWest: [5, 5], northEast: [25, 25] } }) | |
| expect(res.results).toBeDefined() | |
| expect((res.results as any[]).some((r) => r.id === 'a')).toBe(true) | |
| expect((res.results as any[]).some((r) => r.id === 'b')).toBe(true) | |
| expect((res.results as any[]).some((r) => r.id === 'c')).toBe(false) | |
| }) | |
| const treeJSON = build.treeJSON | |
| const res = await spatialIndexTool.execute({ action: 'search', treeJSON, bounds: { southWest: [5, 5], northEast: [25, 25] } }) | |
| expect(res.results).toBeDefined() | |
| expect(res.results?.some((r) => r.id === 'a')).toBe(true) | |
| expect(res.results?.some((r) => r.id === 'b')).toBe(true) | |
| expect(res.results?.some((r) => r.id === 'c')).toBe(false) | |
| }) |
🤖 Prompt for AI Agents
In @src/mastra/tools/tests/spatial-index.tool.test.ts around lines 15 - 21,
Replace the unsafe any[] assertions by using the tool's declared output type
from spatialIndexTool.execute (or the outputSchema/type exported by the spatial
index tool) so results are strongly typed; e.g., declare const typedRes = res as
Awaited<ReturnType<typeof spatialIndexTool.execute>> (or import the tool's
Result/Output type) and then use (typedRes.results as YourResultType[]).some(r
=> r.id === 'a') etc., updating the three expect lines to use the proper Result
type instead of any[].
Comment on lines
+1
to
+3
| declare module 'downsample-lttb' { | ||
| export default function downsample(points: Array<[number, number]>, threshold: number): Array<[number, number]>; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "package.json" -type f | head -20Repository: ssdeanx/AgentStack
Length of output: 76
🌐 Web query:
downsample-lttb npm package latest version current status
💡 Result:
Summary: The npm package downsample-lttb is at version 0.0.1 (published ~10 years ago). The package appears unmaintained (no recent publishes or releases); its source repo is github.com/pingec/downsample-lttb. [1][2]
Sources:
- npm package page for downsample-lttb (version 0.0.1, last publish ~10 years ago). [1]
- GitHub repository pingec/downsample-lttb (repo exists; no published releases). [2]
🌐 Web query:
LTTB largest triangle three buckets JavaScript TypeScript alternatives
💡 Result:
Short answer — alternatives (JS/TS implementations or approaches) to LTTB:
- MinMax (min/max per bin) — very fast, simple, often used as a first-pass preselection. [1][2]
- MinMaxLTTB (MinMax preselection + LTTB) — much faster at large scale while preserving LTTB quality. Implementations / paper available. [1][3]
- M4 (min, max, first, last per bin) — preserves extremes and edges, cheap to compute. [1][4]
- Every‑Nth / nth sampling — trivial, constant-time; useful when uniform sampling is acceptable. [1][5]
- ASAP (Automatic Smoothing for Attention Prioritization) — attention‑aware smoothing/downsampling algorithm. [6]
- SMA / simple moving average (aggregation) — cheap smoothing/decimation. [4]
- LTOB / LTD / other “largest-triangle” variants — related triangle-based variants available in some libs. [4][7]
- Reservoir / random sampling or stratified sampling — useful for streaming or statistically representative subsets (general technique; multiple libs). [8]
JS/TS libraries and tooling to look at:
- tsdownsample (high‑performance Rust backend with bindings; exposes MinMax, M4, LTTB, MinMaxLTTB). Good for large datasets. [1]
- downsample (npm) — collection exposing LTTB, LTOB, ASAP, SMA, etc. [4]
- plotly-resampler / Plotly-Resampler & HoloViews integration — supports MinMax, MinMax-LTTB, M4, nth and LTTB usage patterns. [5][2]
- Original flot-downsample / small JS LTTB ports on npm — useful reference implementations. [7][4]
Recommendations:
- For very large series: use MinMax or MinMaxLTTB (much faster than plain LTTB). [1][3]
- If you need strict preservation of peaks/valleys: M4 or MinMax. [1]
- For simplest/fastest: Every‑Nth or reservoir sampling. [5][8]
- If you want ready JS/TS packages: try “downsample” (npm) or call tsdownsample via bindings / WASM for performance. [4][1]
Sources:
[1] tsdownsample (GitHub repo / README).
[2] HoloViews downsample docs (lists algorithms).
[3] MinMaxLTTB paper / repo (arXiv / IEEE VIS).
[4] downsample (npm) / package readme.
[5] plotly-resampler docs (MinMaxLTTB mention).
[6] ASAP paper.
[7] flot-downsample (original LTTB).
[8] Reservoir sampling references (general).
🏁 Script executed:
cat package.json | grep -A 5 -B 5 "downsample"Repository: ssdeanx/AgentStack
Length of output: 410
Consider replacing downsample-lttb with the more mature alternative already in the project.
The type definition is correct, but downsample-lttb@0.0.1 has been unmaintained for ~10 years with no recent releases. The project already includes downsample@^1.4.0 in its dependencies, which provides LTTB downsampling along with other algorithms (LTOB, ASAP, etc.). Review whether downsample-lttb is still needed or if downsample can fully replace it, reducing dependency maintenance burden and improving stability.
| @@ -1 +1 @@ | |||
| {"numTotalTestSuites":2,"numPassedTestSuites":2,"numFailedTestSuites":0,"numPendingTestSuites":0,"numTotalTests":1,"numPassedTests":1,"numFailedTests":0,"numPendingTests":0,"numTodoTests":0,"snapshot":{"added":0,"failure":false,"filesAdded":0,"filesRemoved":0,"filesRemovedList":[],"filesUnmatched":0,"filesUpdated":0,"matched":0,"total":0,"unchecked":0,"uncheckedKeysByFile":[],"unmatched":0,"updated":0,"didUpdate":false},"startTime":1767908772739,"success":true,"testResults":[{"assertionResults":[{"ancestorTitles":["ForecastView"],"fullName":"ForecastView renders forecast rows","status":"passed","title":"renders forecast rows","duration":51.56680000000006,"failureMessages":[],"meta":{}}],"startTime":1767908773982,"endTime":1767908774033.567,"status":"passed","message":"","name":"C:/Users/ssdsk/AgentStack/src/components/ai-elements/tools/__tests__/weather-tool.test.tsx"}]} No newline at end of file | |||
| {"numTotalTestSuites":4,"numPassedTestSuites":4,"numFailedTestSuites":0,"numPendingTestSuites":0,"numTotalTests":5,"numPassedTests":5,"numFailedTests":0,"numPendingTests":0,"numTodoTests":0,"snapshot":{"added":0,"failure":false,"filesAdded":0,"filesRemoved":0,"filesRemovedList":[],"filesUnmatched":0,"filesUpdated":0,"matched":0,"total":0,"unchecked":0,"uncheckedKeysByFile":[],"unmatched":0,"updated":0,"didUpdate":false},"startTime":1768071920164,"success":true,"testResults":[{"assertionResults":[{"ancestorTitles":["downsampleTool"],"fullName":"downsampleTool returns full series when target >= length","status":"passed","title":"returns full series when target >= length","duration":3.6336999999998625,"failureMessages":[],"meta":{}},{"ancestorTitles":["downsampleTool"],"fullName":"downsampleTool downsamples to target points using lttb","status":"passed","title":"downsamples to target points using lttb","duration":7.829100000000153,"failureMessages":[],"meta":{}},{"ancestorTitles":["downsampleTool"],"fullName":"downsampleTool downsamples with min-max preserving peaks","status":"passed","title":"downsamples with min-max preserving peaks","duration":1.3289999999999509,"failureMessages":[],"meta":{}},{"ancestorTitles":["downsampleTool"],"fullName":"downsampleTool downsamples with m4 preserving edges","status":"passed","title":"downsamples with m4 preserving edges","duration":1.2346000000000004,"failureMessages":[],"meta":{}}],"startTime":1768072050289,"endTime":1768072050303.2346,"status":"passed","message":"","name":"C:/Users/ssdsk/AgentStack/src/mastra/tools/tests/downsample.tool.test.ts"},{"assertionResults":[{"ancestorTitles":["chartJsTool integration (decimation)"],"fullName":"chartJsTool integration (decimation) decimates large datasets via downsample tool","status":"passed","title":"decimates large datasets via downsample tool","duration":39.377700000000004,"failureMessages":[],"meta":{}}],"startTime":1768072050270,"endTime":1768072050309.3777,"status":"passed","message":"","name":"C:/Users/ssdsk/AgentStack/src/mastra/tools/tests/chartjs.integration.test.ts"}]} No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Confirm presence of absolute user paths in the committed artifact
rg -n --hidden --glob '!.git/**' '([A-Za-z]:/Users/|/home/|/Users/)' tests/test-results/test-results.jsonRepository: ssdeanx/AgentStack
Length of output: 2161
Remove absolute local paths containing usernames from committed test artifacts.
This file contains Windows absolute paths with the username C:/Users/ssdsk/..., which is personally identifiable information (PII) and violates the coding guidelines. Do not commit test artifacts with local machine paths. Either:
- Exclude the file from version control (add to .gitignore)
- Strip absolute paths and replace with repo-relative paths
- Generate this file only in CI artifacts, not in the repository
🤖 Prompt for AI Agents
In @tests/test-results/test-results.json at line 1, The committed test artifact
tests/test-results/test-results.json contains Windows absolute user paths (e.g.,
"C:/Users/ssdsk/..."); remove the PII by either excluding this file from VCS
(add tests/test-results/test-results.json or tests/test-results/ to .gitignore
and remove the tracked file), or sanitize the JSON by replacing absolute paths
with repo-relative names (e.g., only keep test file basenames or repo-relative
paths in the "name" fields) and commit the sanitized file; ensure CI regenerates
full artifacts if needed rather than committing local machine paths.
Comment on lines
+46
to
+47
| - **Pattern to implement** (from AGENTS.md): | ||
| ```typescript |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add blank line before fenced code block.
Markdown best practice requires blank lines around fenced code blocks for proper rendering and readability.
Based on coding guidelines, markdown formatting should include proper whitespace.
📝 Fix markdown formatting
## Critical Context
- **Pattern to implement** (from AGENTS.md):
+
```typescript
import type { RequestContext } from '@mastra/core/request-context'🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
47-47: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In @thoughts/ledgers/CONTINUITY_ses_456e.md around lines 46 - 47, The markdown
has a fenced code block that lacks a blank line above it; add a single blank
line between the preceding paragraph/header ("**Pattern to implement** (from
AGENTS.md):") and the opening ```typescript fence so the block renders correctly
and aligns with the project's Markdown whitespace guidelines; ensure the blank
line is added immediately before the line containing ```typescript and that
surrounding spacing follows the same pattern elsewhere in the file.
Comment on lines
+289
to
+317
| instructions: ({ requestContext }: { requestContext: RequestContext<GraphingRuntimeContext> }) => { | ||
| const lang = requestContext.get('language') ?? 'en' | ||
| return { | ||
| role: 'system', | ||
| content: `You are Code Metrics Analyst (lang=${lang}). Collect precise repository metrics using AST-based measurement when possible. Input may be { repoPath?, files?: [{path, content}] }. | ||
|
|
||
| Process: | ||
| 1) Collect files (git ls-files or provided list); respect ignore rules. | ||
| 2) Parse files (AST preferred) and compute: LOC, function counts, average function size, simple cyclomatic complexity estimate, nesting depth. | ||
| 3) Aggregate per-module and per-node metrics, normalize scores to a 0-100 scale, and flag top hotspots. | ||
|
|
||
| Output JSON schema: | ||
| { | ||
| "totalFiles": number, | ||
| "totalLOC": number, | ||
| "totalFunctions": number, | ||
| "averageFunctionSize": number, | ||
| "perModule": { "moduleName": { "files": number, "loc": number, "functions": number, "avgFuncSize": number } }, | ||
| "perNodeAnnotations": { "nodeId": { "loc": number, "functions": number, "cyclomatic": number, "nesting": number } }, | ||
| "hotspots": [ { "nodeId": "string", "metric": "string", "score": number, "reason": "string" } ] | ||
| } | ||
|
|
||
| Rules: | ||
| - Provide numeric outputs and top-N hotspots. Validate numbers and include timestamps and counts. | ||
| - Return only JSON by default. If diagnostic=true in the request, include tool call traces and validation failures. | ||
| `, | ||
|
|
||
| } | ||
| }, |
There was a problem hiding this comment.
Syntax issue in codeMetricsAgent instructions return statement.
The instructions function has a malformed return object. Line 315 ends the template literal, but Line 316 has a standalone } that doesn't properly close the return object structure. Compare with technicalAnalysisAgent (Lines 114-126) which has proper structure.
Proposed fix
instructions: ({ requestContext }: { requestContext: RequestContext<GraphingRuntimeContext> }) => {
const lang = requestContext.get('language') ?? 'en'
return {
role: 'system',
content: `You are Code Metrics Analyst (lang=${lang}). Collect precise repository metrics using AST-based measurement when possible. Input may be { repoPath?, files?: [{path, content}] }.
Process:
1) Collect files (git ls-files or provided list); respect ignore rules.
2) Parse files (AST preferred) and compute: LOC, function counts, average function size, simple cyclomatic complexity estimate, nesting depth.
3) Aggregate per-module and per-node metrics, normalize scores to a 0-100 scale, and flag top hotspots.
Output JSON schema:
{
"totalFiles": number,
"totalLOC": number,
"totalFunctions": number,
"averageFunctionSize": number,
"perModule": { "moduleName": { "files": number, "loc": number, "functions": number, "avgFuncSize": number } },
"perNodeAnnotations": { "nodeId": { "loc": number, "functions": number, "cyclomatic": number, "nesting": number } },
"hotspots": [ { "nodeId": "string", "metric": "string", "score": number, "reason": "string" } ]
}
Rules:
- Provide numeric outputs and top-N hotspots. Validate numbers and include timestamps and counts.
- Return only JSON by default. If diagnostic=true in the request, include tool call traces and validation failures.
`,
-
- }
- },
+ }
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| instructions: ({ requestContext }: { requestContext: RequestContext<GraphingRuntimeContext> }) => { | |
| const lang = requestContext.get('language') ?? 'en' | |
| return { | |
| role: 'system', | |
| content: `You are Code Metrics Analyst (lang=${lang}). Collect precise repository metrics using AST-based measurement when possible. Input may be { repoPath?, files?: [{path, content}] }. | |
| Process: | |
| 1) Collect files (git ls-files or provided list); respect ignore rules. | |
| 2) Parse files (AST preferred) and compute: LOC, function counts, average function size, simple cyclomatic complexity estimate, nesting depth. | |
| 3) Aggregate per-module and per-node metrics, normalize scores to a 0-100 scale, and flag top hotspots. | |
| Output JSON schema: | |
| { | |
| "totalFiles": number, | |
| "totalLOC": number, | |
| "totalFunctions": number, | |
| "averageFunctionSize": number, | |
| "perModule": { "moduleName": { "files": number, "loc": number, "functions": number, "avgFuncSize": number } }, | |
| "perNodeAnnotations": { "nodeId": { "loc": number, "functions": number, "cyclomatic": number, "nesting": number } }, | |
| "hotspots": [ { "nodeId": "string", "metric": "string", "score": number, "reason": "string" } ] | |
| } | |
| Rules: | |
| - Provide numeric outputs and top-N hotspots. Validate numbers and include timestamps and counts. | |
| - Return only JSON by default. If diagnostic=true in the request, include tool call traces and validation failures. | |
| `, | |
| } | |
| }, | |
| instructions: ({ requestContext }: { requestContext: RequestContext<GraphingRuntimeContext> }) => { | |
| const lang = requestContext.get('language') ?? 'en' | |
| return { | |
| role: 'system', | |
| content: `You are Code Metrics Analyst (lang=${lang}). Collect precise repository metrics using AST-based measurement when possible. Input may be { repoPath?, files?: [{path, content}] }. | |
| Process: | |
| 1) Collect files (git ls-files or provided list); respect ignore rules. | |
| 2) Parse files (AST preferred) and compute: LOC, function counts, average function size, simple cyclomatic complexity estimate, nesting depth. | |
| 3) Aggregate per-module and per-node metrics, normalize scores to a 0-100 scale, and flag top hotspots. | |
| Output JSON schema: | |
| { | |
| "totalFiles": number, | |
| "totalLOC": number, | |
| "totalFunctions": number, | |
| "averageFunctionSize": number, | |
| "perModule": { "moduleName": { "files": number, "loc": number, "functions": number, "avgFuncSize": number } }, | |
| "perNodeAnnotations": { "nodeId": { "loc": number, "functions": number, "cyclomatic": number, "nesting": number } }, | |
| "hotspots": [ { "nodeId": "string", "metric": "string", "score": number, "reason": "string" } ] | |
| } | |
| Rules: | |
| - Provide numeric outputs and top-N hotspots. Validate numbers and include timestamps and counts. | |
| - Return only JSON by default. If diagnostic=true in the request, include tool call traces and validation failures. | |
| `, | |
| } | |
| }, |
🤖 Prompt for AI Agents
In @src/mastra/agents/graphingAgents.ts around lines 289 - 317, The
instructions() function inside codeMetricsAgent returns a malformed object: the
template literal for the system content closes but the surrounding returned
object is not correctly closed, leaving an extra standalone brace; fix the
return value in codeMetricsAgent.instructions (the function that accepts {
requestContext }: RequestContext<GraphingRuntimeContext>) by ensuring the
returned object has properly nested braces and commas (role and content keys
inside a single object), close the template literal and then the object literal
correctly, and match the object shape used by technicalAnalysisAgent so the
function returns a single valid object.
Comment on lines
+219
to
+260
| case 'MACD': { | ||
| const macdResult = MACD.calculate({ | ||
| fastPeriod: ind.fastPeriod ?? 12, | ||
| slowPeriod: ind.slowPeriod ?? 26, | ||
| signalPeriod: ind.signalPeriod ?? 9, | ||
| values: closePrices, | ||
| SimpleMAOscillator: false, | ||
| SimpleMASignal: false | ||
| }) | ||
| const macd = macdResult.map((m) => m.MACD) | ||
| const signal = macdResult.map((m) => m.signal) | ||
| const histogram = macdResult.map((m) => m.histogram) | ||
| const padding = Array( | ||
| closePrices.length - macd.length | ||
| ).fill(null) | ||
|
|
||
| datasets.push({ | ||
| label: 'MACD', | ||
| data: padding.concat(macd), | ||
| borderColor: color, | ||
| borderWidth: 1.5, | ||
| pointRadius: 0, | ||
| fill: false, | ||
| yAxisID: 'y1', | ||
| }) | ||
| datasets.push({ | ||
| label: 'MACD Signal', | ||
| data: padding.concat(signal), | ||
| borderColor: '#00ff00', | ||
| borderWidth: 1.5, | ||
| pointRadius: 0, | ||
| fill: false, | ||
| yAxisID: 'y1', | ||
| }) | ||
| datasets.push({ | ||
| label: 'MACD Histogram', | ||
| data: padding.concat(histogram), | ||
| backgroundColor: color, | ||
| type: 'bar', | ||
| yAxisID: 'y1', | ||
| }) | ||
| break; |
There was a problem hiding this comment.
MACD datasets use y1 axis but the scale may not be rendered.
MACD datasets are assigned yAxisID: 'y1' (Lines 242, 251, 258), but the y1 scale is only conditionally rendered when RSI is present (Line 378). If a user requests only MACD without RSI, the chart will have datasets referencing a non-existent axis.
Fix the scale condition
// Only show y1 if RSI is used
- y1: indicators.some((i) => i.type === 'RSI')
+ y1: indicators.some((i) => i.type === 'RSI' || i.type === 'MACD')
? {
type: 'linear',
display: true,
position: 'right',
- title: { display: true, text: 'Oscillators' },
+ title: { display: true, text: 'RSI / MACD' },
min: 0,
max: 100,Note: MACD values aren't bounded 0-100 like RSI. You may need separate axes or dynamic bounds for MACD.
🤖 Prompt for AI Agents
In @src/mastra/tools/chartjs.tool.ts around lines 219 - 260, The MACD datasets
are assigned yAxisID 'y1' in the 'MACD' case (datasets.push for 'MACD', 'MACD
Signal', 'MACD Histogram') but the code that adds the y1 scale only runs when
RSI is present; update the scale-creation logic to create/configure the 'y1'
scale whenever MACD is present (or when either RSI or MACD are requested), not
just RSI. Locate the scale construction (the code that conditionally adds the
'y1' scale) and change its condition to check for MACD as well (e.g., presence
of the 'MACD' indicator or a flag set when handling the 'MACD' case), and ensure
the y1 scale configuration uses appropriate bounds/type for MACD (separate axis
or dynamic bounds) rather than assuming 0-100.
Comment on lines
+320
to
+347
| if (labels.length > DECIMATE_THRESHOLD) { | ||
| try { | ||
| // Use price series (first dataset) indices to decimate all datasets consistently | ||
| const baseSeries = datasets[0]?.data ?? closePrices | ||
| const baseSeriesArr = Array.isArray(baseSeries) ? (baseSeries as Array<number | null | undefined>) : closePrices | ||
| const valuesForDownsample = baseSeriesArr.map((v) => (v === null || v === undefined ? NaN : Number(v))) | ||
|
|
||
| // Use downsample tool (server-side) to decimate according to algorithm | ||
| const dec = (await downsampleTool.execute({ values: valuesForDownsample, target: DECIMATED_POINTS, algorithm: 'lttb' })) as { indices: number[]; values: number[]; originalLength: number; target: number } | ||
| const indices: number[] = dec.indices | ||
|
|
||
| // Build new labels and datasets sampled at the chosen indices | ||
| finalLabels = indices.map((i) => labels[i]) | ||
| finalDatasets = datasets.map((ds) => { | ||
| const dsData = (ds as { data?: unknown }).data | ||
| const dsArr = Array.isArray(dsData) ? (dsData as unknown[]) : [] | ||
| return { | ||
| ...ds, | ||
| data: indices.map((idx) => { | ||
| const v = dsArr[idx] | ||
| return typeof v === 'number' ? v : null | ||
| }), | ||
| } | ||
| }) | ||
| } catch (e) { | ||
| log.warn('Decimation failed, continuing with full dataset', { error: e }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Decimation failure silently falls back without notifying the span.
When decimation fails (Line 344-346), a warning is logged but the tracing span isn't updated to reflect that decimation was skipped. This could make debugging performance issues harder.
} catch (e) {
log.warn('Decimation failed, continuing with full dataset', { error: e })
+ toolSpan?.update({
+ metadata: { 'tool.decimation.failed': true, 'tool.decimation.error': String(e) },
+ })
}🤖 Prompt for AI Agents
In @src/mastra/tools/chartjs.tool.ts around lines 320 - 347, The decimation
catch block logs the failure via log.warn but doesn't update the tracing span;
inside the catch (the block catching e where log.warn is called), add updates to
the tracing span (e.g., span.setAttribute('chart.decimation.skipped', true),
span.recordException(e) and/or span.setStatus({ code: SpanStatusCode.ERROR,
message: 'Decimation failed' })) so that the trace records that decimation was
skipped and captures the exception; keep the existing log.warn and ensure span
is in scope or passed into the enclosing function that contains this decimation
logic.
Comment on lines
+129
to
+173
| const elements = [ | ||
| ...nodes.map((n) => { | ||
| const labelText = | ||
| typeof n.label === 'string' && n.label.trim() !== '' | ||
| ? n.label | ||
| : n.id | ||
| const nodeClasses = | ||
| typeof n.type === 'string' && n.type.trim() !== '' | ||
| ? [n.type] | ||
| : undefined | ||
| return { | ||
| group: 'nodes' as const, | ||
| data: { | ||
| id: n.id, | ||
| label: labelText, | ||
| type: n.type, | ||
| ...n.data, | ||
| }, | ||
| position: n.position, | ||
| classes: nodeClasses, | ||
| } | ||
| }), | ||
| ...edges.map((e, idx) => { | ||
| const edgeClasses = | ||
| typeof e.type === 'string' && e.type.trim() !== '' | ||
| ? [e.type] | ||
| : undefined | ||
| const edgeLabel = | ||
| typeof e.label === 'string' && e.label.trim() !== '' | ||
| ? e.label | ||
| : undefined | ||
| return { | ||
| group: 'edges' as const, | ||
| data: { | ||
| id: e.id ?? `${e.source}-${e.target}-${idx}`, | ||
| source: e.source, | ||
| target: e.target, | ||
| label: edgeLabel, | ||
| type: e.type ?? undefined, | ||
| ...e.data, | ||
| }, | ||
| classes: edgeClasses, | ||
| } | ||
| }), | ||
| ] |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Element generation logic lacks try-catch wrapper.
If an unexpected error occurs during the node/edge mapping (e.g., accessing undefined properties), the tracing span won't be properly closed with an error. The validation errors are handled, but runtime errors during transformation are not.
Suggested wrapper
+ try {
const elements = [
...nodes.map((n) => {
// ... mapping logic
}),
...edges.map((e, idx) => {
// ... mapping logic
}),
]
const duration = Date.now() - startTime
toolSpan?.update({
// ...
})
toolSpan?.end()
// ... rest
+ } catch (error) {
+ const err = error instanceof Error ? error : new Error(String(error))
+ toolSpan?.error({ error: err, endSpan: true })
+ log.error('cytoscape-generator failed', { error: err.message })
+ throw error
+ }🤖 Prompt for AI Agents
In @src/mastra/tools/cytoscape.tool.ts around lines 129 - 173, Wrap the elements
generation (the nodes.map and edges.map block that builds elements) in a
try-catch so runtime errors during transformation are captured; in the catch,
record the exception on the current tracing span (e.g.,
span.recordException(error)) and set the span status to error (e.g.,
span.setStatus({ code: SpanStatusCode.ERROR, message: error.message })), then
rethrow the error so callers still receive it. Ensure you reference the same
span used for validation/tracing so the span is properly closed with the error.
Comment on lines
+151
to
+172
| ...edges.map((e, idx) => { | ||
| const edgeClasses = | ||
| typeof e.type === 'string' && e.type.trim() !== '' | ||
| ? [e.type] | ||
| : undefined | ||
| const edgeLabel = | ||
| typeof e.label === 'string' && e.label.trim() !== '' | ||
| ? e.label | ||
| : undefined | ||
| return { | ||
| group: 'edges' as const, | ||
| data: { | ||
| id: e.id ?? `${e.source}-${e.target}-${idx}`, | ||
| source: e.source, | ||
| target: e.target, | ||
| label: edgeLabel, | ||
| type: e.type ?? undefined, | ||
| ...e.data, | ||
| }, | ||
| classes: edgeClasses, | ||
| } | ||
| }), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Edge validation: source/target nodes may not exist.
The code generates edges referencing source and target node IDs, but doesn't validate that these IDs exist in the nodeIds set. Invalid references would produce a broken Cytoscape graph.
Add edge validation
...edges.map((e, idx) => {
+ if (!nodeIds.has(e.source)) {
+ log.warn(`Edge references non-existent source node: ${e.source}`)
+ }
+ if (!nodeIds.has(e.target)) {
+ log.warn(`Edge references non-existent target node: ${e.target}`)
+ }
const edgeClasses =
typeof e.type === 'string' && e.type.trim() !== ''🤖 Prompt for AI Agents
In @src/mastra/tools/cytoscape.tool.ts around lines 151 - 172, The edge mapping
in edges.map is creating edges with e.source and e.target without verifying
those IDs exist in the nodeIds set; update the logic inside edges.map to
validate that both e.source and e.target are present in the nodeIds collection
(e.g., nodeIds.has(e.source) and nodeIds.has(e.target)), and skip (or
alternatively mark/log) any edges whose source or target is missing so you don't
produce broken Cytoscape edges; keep the rest of the shape (group: 'edges', data
id/default id generation using `${e.source}-${e.target}-${idx}`, classes) intact
when emitting only validated edges.
Comment on lines
+81
to
+85
| // Fallback: uniform sampling | ||
| const step = Math.max(1, Math.floor(originalLength / target)) | ||
| const indices = Array.from({ length: Math.min(target, originalLength) }, (_, i) => Math.min(originalLength - 1, i * step)) | ||
| const sampledValues = indices.map((i) => values[i]) | ||
| return { indices, values: sampledValues, originalLength, target } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Fallback uniform sampling may produce duplicate indices.
When step is 1 and target equals originalLength, i * step produces sequential indices, but the Math.min clamping could still cause edge issues. More importantly, this fallback doesn't deduplicate, unlike the min-max/m4 paths.
const step = Math.max(1, Math.floor(originalLength / target))
- const indices = Array.from({ length: Math.min(target, originalLength) }, (_, i) => Math.min(originalLength - 1, i * step))
+ const rawIndices = Array.from({ length: Math.min(target, originalLength) }, (_, i) => Math.min(originalLength - 1, i * step))
+ const indices = Array.from(new Set(rawIndices))
const sampledValues = indices.map((i) => values[i])| }, | ||
| }) | ||
|
|
||
| export type DownsampleUITool = typeof downsampleTool |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type alias should use InferUITool for consistency.
Other tools use InferUITool<typeof toolName> pattern. This uses typeof downsampleTool directly, which works but is inconsistent with ChartJsUITool, LeafletUITool, etc.
-export type DownsampleUITool = typeof downsampleTool
+import type { InferUITool } from '@mastra/core/tools'
+export type DownsampleUITool = InferUITool<typeof downsampleTool>🤖 Prompt for AI Agents
In @src/mastra/tools/downsample.tool.ts at line 90, The type alias
DownsampleUITool should follow the project pattern by using the generic helper
InferUITool; replace the direct typeof usage with InferUITool<typeof
downsampleTool> so it matches other aliases like ChartJsUITool and LeafletUITool
and ensures consistent typing across tools.
Comment on lines
+61
to
+103
| execute: async (input, context) => { | ||
| const writer = context?.writer | ||
| const abortSignal = context?.abortSignal | ||
| const tracingContext = context?.tracingContext | ||
| const requestCtx = context?.requestContext as | ||
| | LeafletToolContext | ||
| | undefined | ||
|
|
||
| const userId = requestCtx?.userId | ||
| const workspaceId = requestCtx?.workspaceId | ||
|
|
||
| if (abortSignal?.aborted ?? false) { | ||
| throw new Error('Tool call cancelled') | ||
| } | ||
|
|
||
| const toolSpan = tracingContext?.currentSpan?.createChildSpan({ | ||
| type: SpanType.TOOL_CALL, | ||
| name: 'leaflet-generator', | ||
| input, | ||
| metadata: { | ||
| 'tool.id': 'leaflet-generator', | ||
| 'tool.input.pointsCount': input.points?.length ?? 0, | ||
| 'tool.input.cluster': input.cluster ?? true, | ||
| 'user.id': userId, | ||
| 'workspace.id': workspaceId, | ||
| }, | ||
| }) | ||
| const startTime = Date.now() | ||
| logToolExecution('leaflet-generator', { pointsCount: input.points?.length ?? 0, cluster: input.cluster ?? true }) | ||
|
|
||
| const { points: rawPoints, bounds, center, zoom } = input | ||
|
|
||
| const MAX_MARKERS = 5000 | ||
|
|
||
| // basic validation | ||
| for (const p of rawPoints) { | ||
| if (p.lat < -90 || p.lat > 90 || p.lng < -180 || p.lng > 180) { | ||
| throw new Error( | ||
| `Invalid coordinates for point: lat=${p.lat}, lng=${p.lng}` | ||
| ) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Coordinate validation and bounds clipping occur outside the try-catch block.
If the coordinate validation (Lines 96-102) throws, the error won't be properly traced or logged via the established error handling pattern. Consider moving the validation inside the try block or wrapping it separately.
Suggested restructure
+ try {
// basic validation
for (const p of rawPoints) {
if (p.lat < -90 || p.lat > 90 || p.lng < -180 || p.lng > 180) {
throw new Error(
`Invalid coordinates for point: lat=${p.lat}, lng=${p.lng}`
)
}
}
// Clip to bounds if provided
let points = rawPoints
// ... rest of the code
- try {🤖 Prompt for AI Agents
In @src/mastra/tools/leaflet.tool.ts around lines 61 - 103, The coordinate
validation for rawPoints is outside the try-catch and may bypass tracing/error
logging; move the for-loop that checks p.lat/p.lng into the main try block
inside execute (or wrap it in its own try that reports errors to
tracingContext.currentSpan/toolSpan) so any thrown Error is caught, logged via
toolSpan/tracingContext and handled consistently with the rest of the function
(refer to execute, rawPoints, toolSpan and tracingContext identifiers to locate
where to place the validation).
Comment on lines
+56
to
+106
| execute: async (input) => { | ||
| const { action, points, bounds, treeJSON } = input | ||
|
|
||
| if (action === 'build') { | ||
| if (!points || points.length === 0) { | ||
| throw new Error('No points provided to build the spatial index') | ||
| } | ||
|
|
||
| const tree = new RBush<IndexedItem>() | ||
| const items = points.map((p) => ({ | ||
| minX: p.lng, | ||
| minY: p.lat, | ||
| maxX: p.lng, | ||
| maxY: p.lat, | ||
| id: p.id, | ||
| data: p.data, | ||
| })) | ||
| tree.load(items) | ||
| const json = tree.toJSON() | ||
| return { treeJSON: json } | ||
| } | ||
|
|
||
| if (action === 'fromJSON') { | ||
| if (treeJSON === undefined) { throw new Error('treeJSON is required for fromJSON action') } | ||
| const tree = new RBush<IndexedItem>() | ||
| tree.fromJSON(treeJSON) | ||
| return { message: 'Index loaded' } | ||
| } | ||
|
|
||
| if (action === 'search') { | ||
| if (treeJSON === undefined) { throw new Error('treeJSON is required for search action') } | ||
| if (bounds === undefined) { throw new Error('bounds is required for search action') } | ||
| const tree = new RBush<IndexedItem>() | ||
| tree.fromJSON(treeJSON as unknown) | ||
| const [swLat, swLng] = bounds.southWest | ||
| const [neLat, neLng] = bounds.northEast | ||
| const rawResults = tree.search({ minX: swLng, minY: swLat, maxX: neLng, maxY: neLat }) | ||
| const results = rawResults.map((r: IndexedItem) => ({ | ||
| minX: Number(r.minX), | ||
| minY: Number(r.minY), | ||
| maxX: Number(r.maxX), | ||
| maxY: Number(r.maxY), | ||
| id: String(r.id), | ||
| data: r.data, | ||
| })) | ||
| return { results } | ||
| } | ||
|
|
||
| return { message: 'No-op' } | ||
| }, | ||
| }) |
There was a problem hiding this comment.
Missing tracing and progress events required by coding guidelines.
Per the tool implementation guidelines, this tool should:
- Create a child span via
tracingContext?.currentSpan?.createChildSpan()withSpanType.TOOL_CALL - Emit progress events via
context?.writer?.custom()with'data-tool-progress'type - Check
abortSignal?.abortedbefore performing operations - Use
logToolExecution()for consistent logging
The other tools in this PR (leaflet, cytoscape, chartjs) all implement these patterns. As per coding guidelines for src/mastra/tools/**/*.ts.
Suggested pattern to add
- execute: async (input) => {
+ execute: async (input, context) => {
+ const tracingContext = context?.tracingContext
+ const abortSignal = context?.abortSignal
+ const writer = context?.writer
+
+ if (abortSignal?.aborted) {
+ throw new Error('Tool call cancelled')
+ }
+
+ const toolSpan = tracingContext?.currentSpan?.createChildSpan({
+ type: SpanType.TOOL_CALL,
+ name: 'spatial-index',
+ input,
+ metadata: { 'tool.id': 'spatial-index', 'tool.action': input.action },
+ })
+
+ try {
+ await writer?.custom({
+ type: 'data-tool-progress',
+ data: { status: 'in-progress', message: `Performing ${input.action}`, stage: 'spatial-index' },
+ id: 'spatial-index',
+ })
+ // ... existing logic ...
+ toolSpan?.end()
+ return result
+ } catch (error) {
+ toolSpan?.error({ error: error instanceof Error ? error : new Error(String(error)), endSpan: true })
+ throw error
+ }
const { action, points, bounds, treeJSON } = input🤖 Prompt for AI Agents
In @src/mastra/tools/spatial-index.tool.ts around lines 56 - 106, The execute
handler in the spatial-index tool must add tracing, progress events, abort
checks, and standardized logging: create a child span via
tracingContext?.currentSpan?.createChildSpan(SpanType.TOOL_CALL) at the start of
execute and finish it in a finally block; call logToolExecution(...) with the
span and input/action before heavy work; before any long or blocking operations
(building/loading/searching the RBush in execute, and before tree.load,
tree.fromJSON, and tree.search) check abortSignal?.aborted and throw or return
early; emit progress updates via context?.writer?.custom({ type:
'data-tool-progress', progress: <number>, message: <string> }) at key steps
(start, after load/fromJSON, before/after search); apply these changes across
the action branches for 'build', 'fromJSON', and 'search' using the existing
symbols RBush, execute, tree.load, tree.fromJSON, tree.search and ensure the
child span is properly closed in all paths.
Comment on lines
+78
to
+83
| if (action === 'fromJSON') { | ||
| if (treeJSON === undefined) { throw new Error('treeJSON is required for fromJSON action') } | ||
| const tree = new RBush<IndexedItem>() | ||
| tree.fromJSON(treeJSON) | ||
| return { message: 'Index loaded' } | ||
| } |
There was a problem hiding this comment.
fromJSON action is effectively a no-op.
The fromJSON handler reconstructs the RBush tree into a local variable, then immediately discards it by returning only { message: 'Index loaded' }. The reconstructed tree is never used or returned, making this action useless.
Either return the tree (serialized) for subsequent operations, or document that this action is only for validation purposes.
Possible fix: validate and return confirmation
if (action === 'fromJSON') {
if (treeJSON === undefined) { throw new Error('treeJSON is required for fromJSON action') }
const tree = new RBush<IndexedItem>()
tree.fromJSON(treeJSON)
- return { message: 'Index loaded' }
+ // Return the tree JSON back to confirm it was loaded/validated
+ return { treeJSON: tree.toJSON(), message: 'Index loaded and validated' }
}
This was referenced Jan 12, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce new graphing, charting, mapping, and resilient fetch tools, wire a confirmation tool into multiple agent networks, and enhance observability and technical analysis tooling.
New Features:
Enhancements:
Build:
Documentation:
Tests: