Conversation
- Updated MainSidebar to include agent details and thread count in tooltips. - Integrated useRouter for navigation within MainSidebar. - Improved loading state handling for selected agent details. - Adjusted layout and spacing in LandingHero for better responsiveness and aesthetics. style: refine landing hero and tools list UI - Modified styles in LandingHero for improved visual hierarchy and spacing. - Adjusted button sizes and padding for consistency across different screen sizes. - Enhanced ToolsList component with better spacing and layout adjustments. fix: update navbar links and styles - Corrected navigation links in Navbar to include workflows. - Improved mobile menu styles for better usability. chore: clean up login and signup pages - Removed unnecessary hydration checks in Login and Signup pages. - Simplified effect dependencies for better performance. test: update weather tool tests - Enhanced weather tool test cases to include current weather data and additional forecast details.
Reviewer's GuideEnhances chat UI components (sidebar, tabs, navbar) and landing page layout while expanding tool typings, refining LibSQL configuration, simplifying auth flows, and enriching weather tool tests. Sequence diagram for MainSidebar agent dashboard tooltipsequenceDiagram
actor User
participant MainSidebar
participant useAgent
participant useThreads
participant Tooltip
User->>MainSidebar: Navigate to /chat/agents/{agentId}
MainSidebar->>useAgent: useAgent(selectedAgent)
MainSidebar->>useThreads: useThreads({ agentId: selectedAgent })
useAgent-->>MainSidebar: selectedAgentQuery (loading)
useThreads-->>MainSidebar: threadsResult (loading)
MainSidebar->>MainSidebar: selectedAgentSummary useMemo
MainSidebar-->>User: Render sidebar with dashboard button
useAgent-->>MainSidebar: selectedAgentQuery.data
useThreads-->>MainSidebar: threadsResult.data
MainSidebar->>MainSidebar: Recompute selectedAgentSummary
MainSidebar->>MainSidebar: threads = threadsResult.data
User->>Tooltip: Hover dashboard button
Tooltip->>MainSidebar: Read selectedAgentSummary and threads.length
MainSidebar-->>Tooltip: Agent name, provider, model, thread count
Tooltip-->>User: Show tooltip with agent summary
Class diagram for UITool type aliases and PolygonCryptoSnapshotsCardclassDiagram
class InferUITool {
<<generic type>>
}
class WeatherToolFn {
<<function type>>
}
class BinanceSpotMarketDataToolFn {
<<function type>>
}
class PolygonCryptoSnapshotsToolFn {
<<function type>>
}
class WeatherUITool {
<<type alias>>
}
class BinanceSpotMarketDataUITool {
<<type alias>>
}
class PolygonCryptoSnapshotsUITool {
<<type alias>>
}
WeatherUITool ..|> InferUITool : extends
BinanceSpotMarketDataUITool ..|> InferUITool : extends
PolygonCryptoSnapshotsUITool ..|> InferUITool : extends
WeatherUITool o-- WeatherToolFn : wraps tool type
BinanceSpotMarketDataUITool o-- BinanceSpotMarketDataToolFn : wraps tool type
PolygonCryptoSnapshotsUITool o-- PolygonCryptoSnapshotsToolFn : wraps tool type
class PolygonCryptoSnapshotsCard {
+props input
+props output
+props errorText
+render() ReactNode
}
PolygonCryptoSnapshotsCard ..> PolygonCryptoSnapshotsUITool : displays output of
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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.
|
🤖 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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🤖 I'm sorry @ssdeanx, but I was unable to process your request. Please see the logs for more details. |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| id: 'libsql-storage', | ||
| url: process.env.TURSO_DATABASE_URL ?? 'file:./database.db', | ||
| authToken: process.env.TURSO_AUTH_TOKEN, | ||
| url: 'file:./database.db', |
Contributor
There was a problem hiding this comment.
CRITICAL: Hardcoded database URL removes environment variable fallback, potentially breaking production deployments that rely on TURSO_DATABASE_URL
| export const libsqlvector = new LibSQLVector({ | ||
| id: 'libsql-vector', | ||
| url: process.env.TURSO_DATABASE_URL ?? 'file:./vectors.db', | ||
| url: 'file:./database.db', |
Contributor
There was a problem hiding this comment.
CRITICAL: Hardcoded vector database URL removes environment variable fallback, breaking cloud database support
| dimension: 3072, | ||
| metric: 'cosine', | ||
| }) | ||
| //await libsqlvector.createIndex({ |
Contributor
There was a problem hiding this comment.
WARNING: Commented out vector index creation may cause runtime errors if index is required for vector operations
Contributor
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)No additional issues found outside the diff. Files Reviewed (11 files)
Reviewed by grok-code-fast-1:optimized:free · 444,267 tokens |
![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 extensive UI refinements across the chat sidebar, landing hero, and tools list, primarily focusing on Tailwind CSS adjustments for improved responsiveness and styling. Functional updates include the addition of agent summaries in the main sidebar tooltip, the removal of hydration checks in the login and signup pages, and a significant expansion of available tool types. Feedback focuses on correcting navigation link mismatches in the navbar, restoring environment variable support for database and vector store configurations which were hardcoded, and re-adding symbol-specific context to the crypto snapshots UI components for better user clarity.
Comment on lines
+268
to
+269
| href="/chat/workflows" | ||
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/workflows' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} |
Contributor
There was a problem hiding this comment.
The navigation link for 'Admin Settings' (labeled on line 285) has been incorrectly updated to point to /chat/workflows. This creates a mismatch between the link's label and its destination. It should point to /chat/admin.
Suggested change
| href="/chat/workflows" | |
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/workflows' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} | |
| href="/chat/admin" | |
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/admin' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} |
| <Link | ||
| href="/chat/admin" | ||
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/admin' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} | ||
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/workflows' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} |
Contributor
There was a problem hiding this comment.
The active state check for this link is using /chat/workflows, but the link itself points to /chat/admin (line 294) and is labeled 'Workflows' (line 328). This link should point to /chat/workflows to match its label and active state check.
| id: 'libsql-storage', | ||
| url: process.env.TURSO_DATABASE_URL ?? 'file:./database.db', | ||
| authToken: process.env.TURSO_AUTH_TOKEN, | ||
| url: 'file:./database.db', |
Contributor
There was a problem hiding this comment.
The database URL is hardcoded to a local file, removing the environment variable configuration (process.env.TURSO_DATABASE_URL). This will break deployments that rely on remote databases. Please restore the environment variable check.
Suggested change
| url: 'file:./database.db', | |
| url: process.env.TURSO_DATABASE_URL ?? 'file:./database.db', |
| export const libsqlvector = new LibSQLVector({ | ||
| id: 'libsql-vector', | ||
| url: process.env.TURSO_DATABASE_URL ?? 'file:./vectors.db', | ||
| url: 'file:./database.db', |
Contributor
There was a problem hiding this comment.
The vector store URL is hardcoded and the environment variable check has been removed. Additionally, the default fallback was changed from file:./vectors.db to file:./database.db. If this consolidation is intentional, the environment variable support should still be preserved.
Suggested change
| url: 'file:./database.db', | |
| url: process.env.TURSO_DATABASE_URL ?? 'file:./database.db', |
| }) { | ||
| if (errorText) {return <ErrorCard title="Crypto Snapshots Failed" message={errorText} />} | ||
| if (!output) {return <LoadingCard title={`Fetching snapshots for ${(input as any).symbol}`} icon={<Loader2 className="size-4 animate-spin" />} />} | ||
| if (!output) {return <LoadingCard title="Fetching crypto snapshots" icon={<Loader2 className="size-4 animate-spin" />} />} |
Contributor
There was a problem hiding this comment.
The symbol name was removed from the loading state title. Including the symbol provides better context to the user while the data is being fetched.
Suggested change
| if (!output) {return <LoadingCard title="Fetching crypto snapshots" icon={<Loader2 className="size-4 animate-spin" />} />} | |
| if (!output) {return <LoadingCard title={`Fetching snapshots for ${(input as any).symbol}`} icon={<Loader2 className="size-4 animate-spin" />} />} |
Comment on lines
+396
to
+397
| <CardTitle className="flex items-center gap-2 text-sm"><FileText className="size-4 text-primary" /> Crypto Snapshots</CardTitle> | ||
| <div><Button size="sm" variant="ghost" onClick={() => downloadJSON('crypto-snapshots.json', data)}><Download className="size-4" /></Button></div> |
Contributor
There was a problem hiding this comment.
The symbol name has been removed from the card title and the download filename. This makes the UI and the exported file less descriptive. It is recommended to keep the symbol context.
Suggested change
| <CardTitle className="flex items-center gap-2 text-sm"><FileText className="size-4 text-primary" /> Crypto Snapshots</CardTitle> | |
| <div><Button size="sm" variant="ghost" onClick={() => downloadJSON('crypto-snapshots.json', data)}><Download className="size-4" /></Button></div> | |
| <CardTitle className="flex items-center gap-2 text-sm"><FileText className="size-4 text-primary" /> Crypto Snapshots: {(input as any).symbol}</CardTitle> | |
| <div><Button size="sm" variant="ghost" onClick={() => downloadJSON(`${(input as any).symbol}-snapshots.json`, data)}><Download className="size-4" /></Button></div> |
![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:
- The libsql config now hardcodes
url: 'file:./database.db'for bothLibSQLStoreandLibSQLVectorand removesprocess.env.TURSO_DATABASE_URL/vectors.db, which changes deployment behavior and eliminates configurability; consider keeping the env-based URLs (and separate vector DB if intentional) while still adding the new retry options. - In
Navbar, the second "Admin"/"Workflows" link still hashref="/chat/admin"but its active-state condition checkspathname === '/chat/workflows', so the visual state and destination can diverge—align the href and pathname checks to the same route. - For
PolygonCryptoSnapshotsCard, the loading/title text and downloaded filename no longer include the symbol, which makes the UI and exported data less informative when working with multiple symbols; consider incorporating the symbol back into the label and filename.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The libsql config now hardcodes `url: 'file:./database.db'` for both `LibSQLStore` and `LibSQLVector` and removes `process.env.TURSO_DATABASE_URL`/`vectors.db`, which changes deployment behavior and eliminates configurability; consider keeping the env-based URLs (and separate vector DB if intentional) while still adding the new retry options.
- In `Navbar`, the second "Admin"/"Workflows" link still has `href="/chat/admin"` but its active-state condition checks `pathname === '/chat/workflows'`, so the visual state and destination can diverge—align the href and pathname checks to the same route.
- For `PolygonCryptoSnapshotsCard`, the loading/title text and downloaded filename no longer include the symbol, which makes the UI and exported data less informative when working with multiple symbols; consider incorporating the symbol back into the label and filename.
## Individual Comments
### Comment 1
<location path="src/mastra/config/libsql.ts" line_range="13-17" />
<code_context>
id: 'libsql-storage',
- url: process.env.TURSO_DATABASE_URL ?? 'file:./database.db',
- authToken: process.env.TURSO_AUTH_TOKEN,
+ url: 'file:./database.db',
+ maxRetries: 5, // Optional retry configuration for transient errors
+ initialBackoffMs: 100, // Initial backoff for retries
+ //disableInit: process.env.DB_DISABLE_INIT === 'true', // Disable auto-init if specified
+ authToken: process.env.TURSO_AUTH_TOKEN, // Optional: for Turso cloud databases
+ // Additional options can be added here as needed
})
</code_context>
<issue_to_address>
**issue (bug_risk):** Using a hard-coded local DB URL ignores TURSO_DATABASE_URL and may break cloud deployments.
Previously the URL only fell back to `file:./database.db` when `TURSO_DATABASE_URL` was unset. Now it always uses the local file path even if the env var is provided, while still passing an auth token, so Turso cloud or other remote setups will silently hit the wrong DB. Please restore the `process.env.TURSO_DATABASE_URL ?? 'file:./database.db'` pattern (or similar) so remote deployments continue to work.
</issue_to_address>
### Comment 2
<location path="app/components/navbar.tsx" line_range="268-269" />
<code_context>
<Link
- href="/chat/admin"
- className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/admin' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`}
+ href="/chat/workflows"
+ className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/workflows' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`}
>
<svg
</code_context>
<issue_to_address>
**issue (bug_risk):** Updated link target to `/chat/workflows`, but the mobile/admin link below still points to `/chat/admin`.
On this mobile/admin link, the `href` still points to `/chat/admin` while the active state uses `pathname === '/chat/workflows'`. This will highlight for workflows but navigate to admin. Please make the `href` and pathname check consistent (likely both `/chat/workflows`).
</issue_to_address>
### Comment 3
<location path="src/components/ai-elements/tools/polygon-tools.tsx" line_range="389-396" />
<code_context>
+ if (!output) {return <LoadingCard title="Fetching crypto snapshots" icon={<Loader2 className="size-4 animate-spin" />} />}
</code_context>
<issue_to_address>
**suggestion:** Removing the symbol from the loading and title text may reduce clarity when multiple symbols are used.
Including the symbol in the loading title (as before with `(input as any).symbol`) helps users distinguish which asset a snapshot belongs to when multiple are requested. If the symbol is available here, consider keeping it in the title (e.g., `Crypto Snapshots: BTC-USD`) while still improving the wording, so each card is clearly tied to its symbol.
Suggested implementation:
```typescript
if (errorText) {return <ErrorCard title="Crypto Snapshots Failed" message={errorText} />}
if (!output) {
const symbol = (input as any)?.symbol
return (
<LoadingCard
title={symbol ? `Fetching Crypto Snapshots: ${symbol}` : 'Fetching Crypto Snapshots'}
icon={<Loader2 className="size-4 animate-spin" />}
/>
)
}
if ('error' in output) {return <ErrorCard title="Crypto Snapshots Error" message={(output as any).error ?? 'Unknown'} />}
```
If the variable holding the request context is not named `input` in this scope, replace `(input as any)?.symbol` with the correct variable reference that contains the symbol (e.g., `(toolInput as any)?.symbol`), consistent with how the symbol was previously accessed in this component.
</issue_to_address>
### Comment 4
<location path="src/components/ai-elements/tools/__tests__/weather-tool.test.tsx" line_range="15-24" />
<code_context>
input: { location: 'London', days: 3 },
output: {
data: {
+ current: {
+ temp_c: 20,
+ condition: {
+ text: 'Partly cloudy',
+ icon: '//cdn.weatherapi.com/icon.png',
+ },
+ humidity: 65,
+ wind_kph: 12,
+ },
+ location: {
+ name: 'London',
+ country: 'UK',
</code_context>
<issue_to_address>
**suggestion (testing):** Extend the test to assert that the newly added `current` and detailed forecast fields are actually rendered/used by `ForecastView`.
The fixture now includes `current` (temp, condition, humidity, wind), a richer `location`, and `mintemp_c`/`maxtemp_c` for each `forecastday`, but the test still only checks the basic forecast behavior. Please add assertions that `ForecastView` renders:
- Current temperature and condition text/icon
- Location name (and, if used, country / local time)
- Min/max temps per forecast day in addition to `avgtemp_c`
If the component has fallback behavior when these fields are missing, add a separate test with a minimal response to cover those edge cases as well.
Suggested implementation:
```typescript
input: { location: 'London', days: 3 },
output: {
data: {
current: {
temp_c: 20,
condition: {
text: 'Partly cloudy',
icon: '//cdn.weatherapi.com/icon.png',
},
humidity: 65,
wind_kph: 12,
},
location: {
name: 'London',
country: 'UK',
localtime: '2023-01-02 12:00',
},
forecast: {
forecastday: [
{
date: '2023-01-02',
day: {
avgtemp_c: 22,
mintemp_c: 16,
maxtemp_c: 24,
```
I can only see the fixture portion of the test file, not the tests themselves. Below is how you should extend the tests around this fixture to fully implement the requested behavior.
Assuming there is an existing test that verifies the basic forecast rendering (something like `"renders weather forecast"`), update it along these lines:
```ts
it('renders current conditions and detailed forecast for 3 days', async () => {
// Arrange: however you currently invoke ForecastView + tool call
render(<ForecastView /* props as in existing test */ />);
// Act: wait for the API-driven content to appear (reuse your existing waitFor/findBy...)
// e.g.:
const londonHeading = await screen.findByText(/London/i);
// Assert: location info
expect(londonHeading).toBeInTheDocument();
// If the component renders country/localtime, assert them as well:
expect(screen.getByText(/UK/)).toBeInTheDocument();
expect(screen.getByText(/2023-01-02 12:00/)).toBeInTheDocument();
// Assert: current conditions
expect(screen.getByText(/20\s*°?C/i)).toBeInTheDocument();
expect(screen.getByText(/Partly cloudy/i)).toBeInTheDocument();
// If the icon is rendered as <img>, assert its src/alt; adjust to actual implementation:
const iconImg = screen.getByRole('img', { name: /Partly cloudy/i });
expect(iconImg).toHaveAttribute('src', expect.stringContaining('cdn.weatherapi.com/icon.png'));
// If humidity and wind are shown:
expect(screen.getByText(/65\s*%/i)).toBeInTheDocument();
expect(screen.getByText(/12\s*(kph|km\/h)/i)).toBeInTheDocument();
// Assert: per-day forecast, including min/max + avg
// Adjust selectors to how dates/temps are rendered in your component.
expect(screen.getByText(/2023-01-02/)).toBeInTheDocument();
expect(screen.getByText(/22\s*°?C/i)).toBeInTheDocument(); // avg
expect(screen.getByText(/16\s*°?C/i)).toBeInTheDocument(); // min
expect(screen.getByText(/24\s*°?C/i)).toBeInTheDocument(); // max
// Repeat or generalize for other forecast days if present in the fixture.
});
```
Then add a separate test that covers fallback behavior when the new fields are missing. Reuse your existing test helpers/mocking approach:
```ts
it('gracefully handles minimal responses without current conditions and detailed temps', async () => {
// Arrange: mock the tool/API to return a minimal response:
mockWeatherApiResponse({
data: {
// no current
location: {
name: 'London',
},
forecast: {
forecastday: [
{
date: '2023-01-02',
day: {
// only avgtemp_c, no min/max
avgtemp_c: 22,
},
},
],
},
},
});
render(<ForecastView /* same props as main test */ />);
// Act
await screen.findByText(/London/i);
// Assert: still renders basic forecast
expect(screen.getByText(/2023-01-02/)).toBeInTheDocument();
expect(screen.getByText(/22\s*°?C/i)).toBeInTheDocument();
// Assert: does NOT break if current/min/max missing
// Depending on your fallback UI, this may be "N/A", hidden sections, or similar.
// Example if you hide them:
expect(screen.queryByText(/Partly cloudy/i)).toBeNull();
expect(screen.queryByText(/Min/i)).toBeNull();
expect(screen.queryByText(/Max/i)).toBeNull();
});
```
Concretely, you will need to:
1. Locate the main forecast-rendering test in `weather-tool.test.tsx` and:
- Update its description to reflect that it also checks current conditions and detailed forecast.
- Add assertions like the ones above for:
- Location name (and country/localtime if rendered).
- Current temperature, condition text, and icon.
- Min/max temps per forecast day, in addition to `avgtemp_c`.
2. Add a new test case in the same file that:
- Sets up a minimal mock response (no `current`, only `location.name`, and only `avgtemp_c` per day).
- Renders `ForecastView`.
- Asserts:
- The component still renders the location and basic forecast.
- The absence (or fallback UI) of current conditions and min/max temperatures without throwing errors.
3. Ensure your new test uses the same mocking helpers / patterns already in this file (e.g., whatever you use to stub the Weather API/tool response and to render `ForecastView`), and update import lists if you add new testing-library matchers (e.g., `toHaveAttribute` from `@testing-library/jest-dom` is likely already configured).
</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
+13
to
+17
| url: 'file:./database.db', | ||
| maxRetries: 5, // Optional retry configuration for transient errors | ||
| initialBackoffMs: 100, // Initial backoff for retries | ||
| //disableInit: process.env.DB_DISABLE_INIT === 'true', // Disable auto-init if specified | ||
| authToken: process.env.TURSO_AUTH_TOKEN, // Optional: for Turso cloud databases |
There was a problem hiding this comment.
issue (bug_risk): Using a hard-coded local DB URL ignores TURSO_DATABASE_URL and may break cloud deployments.
Previously the URL only fell back to file:./database.db when TURSO_DATABASE_URL was unset. Now it always uses the local file path even if the env var is provided, while still passing an auth token, so Turso cloud or other remote setups will silently hit the wrong DB. Please restore the process.env.TURSO_DATABASE_URL ?? 'file:./database.db' pattern (or similar) so remote deployments continue to work.
Comment on lines
+268
to
+269
| href="/chat/workflows" | ||
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/workflows' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} |
There was a problem hiding this comment.
issue (bug_risk): Updated link target to /chat/workflows, but the mobile/admin link below still points to /chat/admin.
On this mobile/admin link, the href still points to /chat/admin while the active state uses pathname === '/chat/workflows'. This will highlight for workflows but navigate to admin. Please make the href and pathname check consistent (likely both /chat/workflows).
Comment on lines
+389
to
+396
| if (!output) {return <LoadingCard title="Fetching crypto snapshots" icon={<Loader2 className="size-4 animate-spin" />} />} | ||
| if ('error' in output) {return <ErrorCard title="Crypto Snapshots Error" message={(output as any).error ?? 'Unknown'} />} | ||
|
|
||
| const data = (output as any).data ?? [] | ||
| return ( | ||
| <Card> | ||
| <CardHeader className="pb-3 flex items-center justify-between"> | ||
| <CardTitle className="flex items-center gap-2 text-sm"><FileText className="size-4 text-primary" /> Crypto Snapshots: {(input as any).symbol}</CardTitle> | ||
| <div><Button size="sm" variant="ghost" onClick={() => downloadJSON(`${(input as any).symbol}-snapshots.json`, data)}><Download className="size-4" /></Button></div> | ||
| <CardTitle className="flex items-center gap-2 text-sm"><FileText className="size-4 text-primary" /> Crypto Snapshots</CardTitle> |
There was a problem hiding this comment.
suggestion: Removing the symbol from the loading and title text may reduce clarity when multiple symbols are used.
Including the symbol in the loading title (as before with (input as any).symbol) helps users distinguish which asset a snapshot belongs to when multiple are requested. If the symbol is available here, consider keeping it in the title (e.g., Crypto Snapshots: BTC-USD) while still improving the wording, so each card is clearly tied to its symbol.
Suggested implementation:
if (errorText) {return <ErrorCard title="Crypto Snapshots Failed" message={errorText} />}
if (!output) {
const symbol = (input as any)?.symbol
return (
<LoadingCard
title={symbol ? `Fetching Crypto Snapshots: ${symbol}` : 'Fetching Crypto Snapshots'}
icon={<Loader2 className="size-4 animate-spin" />}
/>
)
}
if ('error' in output) {return <ErrorCard title="Crypto Snapshots Error" message={(output as any).error ?? 'Unknown'} />}If the variable holding the request context is not named input in this scope, replace (input as any)?.symbol with the correct variable reference that contains the symbol (e.g., (toolInput as any)?.symbol), consistent with how the symbol was previously accessed in this component.
Comment on lines
+15
to
+24
| current: { | ||
| temp_c: 20, | ||
| condition: { | ||
| text: 'Partly cloudy', | ||
| icon: '//cdn.weatherapi.com/icon.png', | ||
| }, | ||
| humidity: 65, | ||
| wind_kph: 12, | ||
| }, | ||
| location: { |
There was a problem hiding this comment.
suggestion (testing): Extend the test to assert that the newly added current and detailed forecast fields are actually rendered/used by ForecastView.
The fixture now includes current (temp, condition, humidity, wind), a richer location, and mintemp_c/maxtemp_c for each forecastday, but the test still only checks the basic forecast behavior. Please add assertions that ForecastView renders:
- Current temperature and condition text/icon
- Location name (and, if used, country / local time)
- Min/max temps per forecast day in addition to
avgtemp_c
If the component has fallback behavior when these fields are missing, add a separate test with a minimal response to cover those edge cases as well.
Suggested implementation:
input: { location: 'London', days: 3 },
output: {
data: {
current: {
temp_c: 20,
condition: {
text: 'Partly cloudy',
icon: '//cdn.weatherapi.com/icon.png',
},
humidity: 65,
wind_kph: 12,
},
location: {
name: 'London',
country: 'UK',
localtime: '2023-01-02 12:00',
},
forecast: {
forecastday: [
{
date: '2023-01-02',
day: {
avgtemp_c: 22,
mintemp_c: 16,
maxtemp_c: 24,I can only see the fixture portion of the test file, not the tests themselves. Below is how you should extend the tests around this fixture to fully implement the requested behavior.
Assuming there is an existing test that verifies the basic forecast rendering (something like "renders weather forecast"), update it along these lines:
it('renders current conditions and detailed forecast for 3 days', async () => {
// Arrange: however you currently invoke ForecastView + tool call
render(<ForecastView /* props as in existing test */ />);
// Act: wait for the API-driven content to appear (reuse your existing waitFor/findBy...)
// e.g.:
const londonHeading = await screen.findByText(/London/i);
// Assert: location info
expect(londonHeading).toBeInTheDocument();
// If the component renders country/localtime, assert them as well:
expect(screen.getByText(/UK/)).toBeInTheDocument();
expect(screen.getByText(/2023-01-02 12:00/)).toBeInTheDocument();
// Assert: current conditions
expect(screen.getByText(/20\s*°?C/i)).toBeInTheDocument();
expect(screen.getByText(/Partly cloudy/i)).toBeInTheDocument();
// If the icon is rendered as <img>, assert its src/alt; adjust to actual implementation:
const iconImg = screen.getByRole('img', { name: /Partly cloudy/i });
expect(iconImg).toHaveAttribute('src', expect.stringContaining('cdn.weatherapi.com/icon.png'));
// If humidity and wind are shown:
expect(screen.getByText(/65\s*%/i)).toBeInTheDocument();
expect(screen.getByText(/12\s*(kph|km\/h)/i)).toBeInTheDocument();
// Assert: per-day forecast, including min/max + avg
// Adjust selectors to how dates/temps are rendered in your component.
expect(screen.getByText(/2023-01-02/)).toBeInTheDocument();
expect(screen.getByText(/22\s*°?C/i)).toBeInTheDocument(); // avg
expect(screen.getByText(/16\s*°?C/i)).toBeInTheDocument(); // min
expect(screen.getByText(/24\s*°?C/i)).toBeInTheDocument(); // max
// Repeat or generalize for other forecast days if present in the fixture.
});Then add a separate test that covers fallback behavior when the new fields are missing. Reuse your existing test helpers/mocking approach:
it('gracefully handles minimal responses without current conditions and detailed temps', async () => {
// Arrange: mock the tool/API to return a minimal response:
mockWeatherApiResponse({
data: {
// no current
location: {
name: 'London',
},
forecast: {
forecastday: [
{
date: '2023-01-02',
day: {
// only avgtemp_c, no min/max
avgtemp_c: 22,
},
},
],
},
},
});
render(<ForecastView /* same props as main test */ />);
// Act
await screen.findByText(/London/i);
// Assert: still renders basic forecast
expect(screen.getByText(/2023-01-02/)).toBeInTheDocument();
expect(screen.getByText(/22\s*°?C/i)).toBeInTheDocument();
// Assert: does NOT break if current/min/max missing
// Depending on your fallback UI, this may be "N/A", hidden sections, or similar.
// Example if you hide them:
expect(screen.queryByText(/Partly cloudy/i)).toBeNull();
expect(screen.queryByText(/Min/i)).toBeNull();
expect(screen.queryByText(/Max/i)).toBeNull();
});Concretely, you will need to:
-
Locate the main forecast-rendering test in
weather-tool.test.tsxand:- Update its description to reflect that it also checks current conditions and detailed forecast.
- Add assertions like the ones above for:
- Location name (and country/localtime if rendered).
- Current temperature, condition text, and icon.
- Min/max temps per forecast day, in addition to
avgtemp_c.
-
Add a new test case in the same file that:
- Sets up a minimal mock response (no
current, onlylocation.name, and onlyavgtemp_cper day). - Renders
ForecastView. - Asserts:
- The component still renders the location and basic forecast.
- The absence (or fallback UI) of current conditions and min/max temperatures without throwing errors.
- Sets up a minimal mock response (no
-
Ensure your new test uses the same mocking helpers / patterns already in this file (e.g., whatever you use to stub the Weather API/tool response and to render
ForecastView), and update import lists if you add new testing-library matchers (e.g.,toHaveAttributefrom@testing-library/jest-domis likely already configured).
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 3 minutes and 47 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (12)
✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Enhances the chat UI/UX (sidebar tooltips, navigation, responsive landing components) while cleaning up auth pages and expanding tool-related types and tests.
Changes:
- Improved chat navigation and sidebar tooltip details (agent summary + thread count), plus updated Navbar product links/styles.
- Refined landing/marketing UI (LandingHero, ToolsList, ChatSidebar) for better responsiveness and spacing.
- Updated tool typings and weather tool tests; adjusted LibSQL config defaults.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test-results/test-results.json | Updates committed Jest results artifact (now with different suite/test counts). |
| src/mastra/config/libsql.ts | Changes LibSQLStore/LibSQLVector URLs and retry options; disables index creation. |
| src/components/ai-elements/tools/types.ts | Reorganizes GitHub tool type imports and adds many new tool UI types. |
| src/components/ai-elements/tools/polygon-tools.tsx | Adds crypto snapshots UI types and adjusts loading/title behaviors. |
| src/components/ai-elements/tools/tests/weather-tool.test.tsx | Expands mocked weather data to cover “current” and more forecast fields. |
| app/login/signup/page.tsx | Removes redundant hydration guards and simplifies effect deps. |
| app/login/page.tsx | Removes redundant hydration guards and simplifies effect deps. |
| app/components/tools-list.tsx | Refines spacing, sizing, and responsive layout for tools listing. |
| app/components/navbar.tsx | Updates Products menu links/styles (adds workflows) and improves mobile menu overflow. |
| app/components/landing-hero.tsx | Adjusts hero layout, typography, and button sizing for responsiveness. |
| app/chat/components/main-sidebar.tsx | Adds agent detail loading/summary in tooltip; switches to next/navigation router usage. |
| app/chat/components/chat-sidebar.tsx | Refines tab list layout for responsiveness (4 cols on small screens). |
| <Link | ||
| href="/chat/admin" | ||
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/admin' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} | ||
| className={`flex items-center gap-3 rounded-sm p-2 text-sm ${pathname === '/chat/workflows' ? 'bg-red-500/10 shadow-sm' : 'hover:bg-red-500/5'}`} |
Comment on lines
11
to
19
| export const libsqlstorage = new LibSQLStore({ | ||
| id: 'libsql-storage', | ||
| url: process.env.TURSO_DATABASE_URL ?? 'file:./database.db', | ||
| authToken: process.env.TURSO_AUTH_TOKEN, | ||
| url: 'file:./database.db', | ||
| maxRetries: 5, // Optional retry configuration for transient errors | ||
| initialBackoffMs: 100, // Initial backoff for retries | ||
| //disableInit: process.env.DB_DISABLE_INIT === 'true', // Disable auto-init if specified | ||
| authToken: process.env.TURSO_AUTH_TOKEN, // Optional: for Turso cloud databases | ||
| // Additional options can be added here as needed | ||
| }) |
Comment on lines
23
to
28
| export const libsqlvector = new LibSQLVector({ | ||
| id: 'libsql-vector', | ||
| url: process.env.TURSO_DATABASE_URL ?? 'file:./vectors.db', | ||
| url: 'file:./database.db', | ||
| // Optional: for Turso cloud databases | ||
| authToken: process.env.TURSO_AUTH_TOKEN, | ||
| syncInterval: 10000, // Sync every 10 seconds (optional) |
Comment on lines
+35
to
+39
| //await libsqlvector.createIndex({ | ||
| // indexName: 'memory_messages_3072', | ||
| // dimension: 3072, | ||
| // metric: 'cosine', | ||
| //}) |
| @@ -1 +1 @@ | |||
| {"numTotalTestSuites":2,"numPassedTestSuites":2,"numFailedTestSuites":0,"numPendingTestSuites":0,"numTotalTests":6,"numPassedTests":6,"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":1776210199450,"success":true,"testResults":[{"assertionResults":[{"ancestorTitles":["Supervisor scorers"],"fullName":"Supervisor scorers rewards a synthesized supervisor response that uses delegation without exposing routing chatter","status":"passed","title":"rewards a synthesized supervisor response that uses delegation without exposing routing chatter","duration":9.037199999999984,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["Supervisor scorers"],"fullName":"Supervisor scorers penalizes raw delegation chatter in the final supervisor response","status":"passed","title":"penalizes raw delegation chatter in the final supervisor response","duration":1.4966999999999189,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["Supervisor scorers"],"fullName":"Supervisor scorers rewards evidence grounded supervisor answers and penalizes unsupported summaries","status":"passed","title":"rewards evidence grounded supervisor answers and penalizes unsupported summaries","duration":2.5877000000000407,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["Supervisor scorers"],"fullName":"Supervisor scorers scores request coverage higher when the supervisor answers the key parts of the prompt","status":"passed","title":"scores request coverage higher when the supervisor answers the key parts of the prompt","duration":2.0133999999998196,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["Supervisor scorers"],"fullName":"Supervisor scorers rewards actionable supervisor recommendations and penalizes vague conclusions","status":"passed","title":"rewards actionable supervisor recommendations and penalizes vague conclusions","duration":1.822300000000041,"failureMessages":[],"meta":{},"tags":[]},{"ancestorTitles":["Supervisor scorers"],"fullName":"Supervisor scorers rewards uncertainty handling when the supervisor states caveats instead of overcommitting","status":"passed","title":"rewards uncertainty handling when the supervisor states caveats instead of overcommitting","duration":2.0205000000000837,"failureMessages":[],"meta":{},"tags":[]}],"startTime":1776210200826,"endTime":1776210200845.0205,"status":"passed","message":"","name":"C:/Users/ssdsk/AgentStack/src/mastra/scorers/supervisor-scorers.test.ts"}]} No newline at end of file | |||
| {"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":1776262021876,"success":true,"testResults":[{"assertionResults":[{"ancestorTitles":["ForecastView"],"fullName":"ForecastView renders forecast rows","status":"passed","title":"renders forecast rows","duration":77.82680000000028,"failureMessages":[],"location":{"line":9,"column":5},"meta":{},"tags":[]}],"startTime":1776262025182,"endTime":1776262025259.827,"status":"passed","message":"","name":"C:/Users/ssdsk/AgentStack/src/components/ai-elements/tools/__tests__/weather-tool.test.tsx"}]} No newline at end of file | |||
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.
style: refine landing hero and tools list UI
fix: update navbar links and styles
chore: clean up login and signup pages
test: update weather tool tests
Summary by Sourcery
Enhance chat and landing UI while expanding tool integrations and improving storage configuration and auth flows.
New Features:
Bug Fixes:
Enhancements:
Tests: