feat: ollama ai provider, design updates#124
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds repository-wide camelCase naming enforcement with targeted overrides; introduces Ollama/OpenRouter provider support and envs; implements AssistantChat and AssistantSidebar and integrates assistant into the dashboard; refactors many internal constants to camelCase; updates UI components (cursor/shadows/typography), Next/Image usage, theme tokens, and related docs/tests/workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant AssistantSidebar
participant AssistantChat
participant API as /api/ai/chat
participant AIService
User->>Browser: Open dashboard
Browser->>AssistantSidebar: mount (sheet or sidebar)
AssistantSidebar->>AssistantChat: mount & load suggestions
User->>AssistantChat: submit message
AssistantChat->>API: POST /api/ai/chat { provider, model, message, tools }
API->>AIService: forward request to Ollama or OpenRouter
AIService-->>API: stream tokens / tool calls
API-->>AssistantChat: stream response & tool outputs
AssistantChat->>Browser: render streaming message and tool outputs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (8)
apps/fastify/test/utils/db.ts (1)
82-90: Use camelCasetablesinstead of PascalCaseTables.The rename from
TABLEScorrectly removes UPPER_SNAKE_CASE per coding guidelines, but the replacement should use camelCase (tables) rather than PascalCase (Tables).♻️ Suggested fix
/** Tables to truncate (order respects FK: users referenced by others) */ -const Tables = [ +const tables = [ 'api_keys', 'account', 'sessions', 'wallet_identities', 'web3_nonce', 'users', 'verification', ] as constAnd update usage on line 99:
- await pgLiteInstance.exec(`TRUNCATE ${Tables.join(', ')} RESTART IDENTITY CASCADE`) + await pgLiteInstance.exec(`TRUNCATE ${tables.join(', ')} RESTART IDENTITY CASCADE`)As per coding guidelines: "Enforces camelCase for variables/consts; disallows UPPER_SNAKE_CASE for constants except environment keys."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/fastify/test/utils/db.ts` around lines 82 - 90, Rename the constant currently declared as Tables to camelCase tables (e.g., change "const Tables = [...] as const" to "const tables = [...] as const") and update every usage/reference in this file (and any exports) from Tables to tables so callers and internal code (e.g., any map/filter/includes calls) use the new identifier; ensure any exported symbol names or tests that referenced Tables are adjusted accordingly to avoid undefined identifier errors.apps/fastify/src/routes/reference/template.ts (1)
1-1: Use camelCase for the constant name.The constant should be
cssinstead ofCssto follow camelCase naming conventions for variables and constants. PascalCase is typically reserved for classes and type names.♻️ Proposed fix
-const Css = ` +const css = `And at Line 256:
- <style>${Css} + <style>${css}As per coding guidelines: "Enforces camelCase for variables/consts; disallows UPPER_SNAKE_CASE for constants except environment keys."
Also applies to: 256-256
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/fastify/src/routes/reference/template.ts` at line 1, Rename the top-level template string constant named "Css" to use camelCase ("css") everywhere it's declared and referenced; update the declaration (const Css = `...`) to const css = `...` and replace all usages of "Css" within this module (including the occurrence around line 256) to "css" so identifiers remain consistent with camelCase conventions.scripts/setup-database.mjs (1)
7-68: Consider using camelCase for constant names per coding guidelines.The refactoring from
UPPER_SNAKE_CASEtoPascalCaseis an improvement, but the coding guidelines specify camelCase for variables and constants. The current names (Docker,DockerCompose,Supabase) should ideally bedocker,dockerCompose,supabase.That said, PascalCase is a common JavaScript convention for configuration objects representing entities/namespaces, so this is a reasonable stylistic choice if the team prefers it.
📝 Suggested change to align with camelCase guideline
-const Docker = { +const docker = { name: 'Docker', ... } -const DockerCompose = { +const dockerCompose = { name: 'Docker Compose', ... } -const Supabase = { +const supabase = { name: 'Supabase CLI', ... }And update all references accordingly (e.g.,
Docker[os]→docker[os]).As per coding guidelines: "Enforces camelCase for variables/consts; disallows UPPER_SNAKE_CASE for constants except environment keys."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/setup-database.mjs` around lines 7 - 68, The constants Docker, DockerCompose, and Supabase should be renamed to camelCase (docker, dockerCompose, supabase) to follow the project's variable naming guideline; update the declarations (const Docker → const docker, etc.) and then update every usage/reference in the repo or this module accordingly (e.g., replace Docker[os] with docker[os], DockerCompose.checkCommand with dockerCompose.checkCommand, Supabase.getDownloadUrl with supabase.getDownloadUrl) to preserve behavior and avoid breaking references.biome.json (1)
149-176: Consider narrowing the override scope.The override disables
useNamingConventionfor broad paths including entire packages (**/packages/ui/**,**/packages/email/**,**/packages/notif/**). This effectively exempts these packages from the naming convention enforcement entirely.If the intent is to allow specific patterns (e.g., component exports, route handlers), consider using more targeted path patterns rather than disabling for the whole package directory.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@biome.json` around lines 149 - 176, The override currently turns off the linter rule "useNamingConvention" for broad include globs like "**/packages/ui/**", "**/packages/email/**", and "**/packages/notif/**", which over-exempts those packages; restrict the scope by replacing those package-wide globs with targeted patterns (for example only component directories or specific file types such as components/exports or route handlers) so "useNamingConvention" remains enabled elsewhere—update the "includes" entries that reference "**/packages/ui/**", "**/packages/email/**", and "**/packages/notif/**" to more specific paths (e.g., component or handler subfolders and file extensions) rather than the entire package directories.packages/ui/src/components/chart.tsx (1)
12-12: Consider using camelCase for consistency with coding guidelines.The constant is renamed from
THEMEStoThemes(PascalCase). The coding guidelines specify camelCase for variables/constants, which would bethemes. PascalCase is acceptable for enum-like objects in TypeScript, so this is a stylistic preference.♻️ Optional: Use camelCase
-const Themes = { light: '', dark: '.dark' } as const +const themes = { light: '', dark: '.dark' } as constNote: If applying this change, update all references (
keyof typeof themes,Object.entries(themes)).As per coding guidelines: "Enforces camelCase for variables/consts."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui/src/components/chart.tsx` at line 12, Rename the exported constant Themes to camelCase "themes" to comply with coding guidelines; update all usages and types that reference it (e.g., "keyof typeof themes", "Object.entries(themes)", any imports/exports or destructuring) and ensure the const definition remains "const themes = { light: '', dark: '.dark' } as const" so typings are preserved and no runtime behavior changes..cursor/rules/frontend/design.mdc (1)
47-47: Consider breaking down the Typography bullet for clarity.Line 47 contains multiple distinct concepts in a single bullet point:
- General typography principles (distinctive fonts, font pairing)
- A crypto-dashboard-specific exception (Inter + Poppins)
- Detailed usage patterns (font-sans, font-heading, numeric, font-mono)
While the information is accurate, consider splitting this into separate bullets or sub-bullets for improved scannability and maintainability, especially since the crypto dashboard exception may evolve independently of general typography principles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.cursor/rules/frontend/design.mdc at line 47, Split the long "Typography" bullet into multiple clearer bullets/sub-bullets: keep one bullet for general typography principles (distinctive fonts, pair display + body, reference @.cursor/skills/frontend-design), a separate bullet for the crypto-dashboard exception (explicitly note Inter for UI + Poppins for headings and mark it as an exception), and another bullet listing detailed usage patterns (use `font-sans` for UI text, `font-heading` / `font-heading font-semibold` for titles and large stats, `numeric`/`tabular-nums` for prices, `font-mono` for code/API keys) so each concept is scannable and independently editable.scripts/block-secret-files.mjs (1)
6-6: Use camelCase for these constants.
BlockedPatternsandAllowedPatternsare regular value bindings, so PascalCase here reads like class names and still misses the repo’s camelCase rule. Rename them toblockedPatterns/allowedPatternsand keep the loop references aligned.♻️ Proposed rename
-const BlockedPatterns = [ +const blockedPatterns = [ // ... ] -const AllowedPatterns = [ +const allowedPatterns = [ // ... ] - for (const allowed of AllowedPatterns) { + for (const allowed of allowedPatterns) { if (allowed.test(filename)) { return false } } - for (const blocked of BlockedPatterns) { + for (const blocked of blockedPatterns) { if (blocked.test(filename)) { return true } }As per coding guidelines, "Enforces camelCase for variables/consts; disallows UPPER_SNAKE_CASE for constants except environment keys."
Also applies to: 29-29, 56-56, 62-62
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/block-secret-files.mjs` at line 6, Rename the PascalCase constants BlockedPatterns and AllowedPatterns to camelCase blockedPatterns and allowedPatterns throughout the file (including their declarations and every usage such as loop iterations or references like "for (const p of BlockedPatterns)" → "for (const p of blockedPatterns)"); update any other identifiers that reference those exact names (imports/exports, function params, or tests) so all occurrences match the new camelCase names and preserve original behavior.apps/next/components/assistant/assistant-chat.tsx (1)
142-145: Add defensive guard for tool output serialization before calling.length.Currently the
output != nullcheck (line 138) prevents theundefinedcase, and the single existing tool returns safe values. However,JSON.stringifycan throw on circular structures andBigInt, and returnsundefinedfor certain values. As new tools are added, consider wrapping serialization in a try-catch or explicitly handling edge cases to prevent render crashes.💭 Suggested approach for future-proofing
- const str = typeof output === 'string' ? output : JSON.stringify(output) + let str: string + if (typeof output === 'string') { + str = output + } else { + try { + str = JSON.stringify(output) ?? '' + } catch { + str = String(output) + } + } if (str.length > 0) hasContent = true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/next/components/assistant/assistant-chat.tsx` around lines 142 - 145, The render code in assistant-chat.tsx currently assumes JSON.stringify(output) succeeds and that the resulting str is a string before calling .length; wrap the serialization of output (the output variable used to build str for MessageResponse) in a defensive block: attempt JSON.stringify in a try/catch, fall back to safe string conversions for BigInt/circular structures (e.g., String(output) or a placeholder like "[unserializable output]"), ensure str is explicitly a string (or empty string) before checking str.length and before pushing <MessageResponse key={`${message.id}-tool-${i}`}>{str}</MessageResponse>, and set hasContent only when str is a non-empty string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/rules/frontend/design.mdc:
- Line 17: The spacing guidance conflicts because gap-3 is shown as an md-level
standard while the rule forbids odd values like p-3/p-5/px-5; update the rule
text to explicitly allow gap-3 and gap-4 as the permitted mid-range pair used
for component spacing (while still discouraging other odd paddings like p-3,
p-5, px-5 except for micro-adjustments), and ensure the md-level recommendations
(where gap-3 is listed) remain consistent with that wording.
In `@apps/fastify/src/routes/ai/chat.ts`:
- Around line 35-39: The schema and handler allow an empty string for the model
which results in calling getOpenRouter().chat('') — normalize blank model names
by treating '' (and whitespace-only) as unset: update the ChatRequestSchema
handling or the route handler to coerce model = (model && model.trim().length >
0) ? model : defaultAiModel before invoking getOpenRouter().chat; alternatively
enforce non-empty via schema (e.g., disallow empty string) so the code paths
that call getOpenRouter().chat or any logic around model (references:
ChatRequestSchema, defaultAiModel, and getOpenRouter().chat) never pass an empty
string.
In `@apps/fastify/test/playwright-global-setup.ts`:
- Around line 3-4: Rename the two PascalCase constants PollTimeoutMs and
PollIntervalMs to camelCase pollTimeoutMs and pollIntervalMs and update all
their usages within the file (e.g., where PollTimeoutMs and PollIntervalMs are
referenced) to the new names; ensure any imports/exports or tests referencing
these identifiers are updated accordingly so the file matches the camelCase
convention used in other e2e setup files like pollTimeoutMs/pollIntervalMs.
In `@apps/fastify/test/swagger-login.e2e.spec.ts`:
- Around line 3-4: Rename the PascalCase constants TestEmail and ApiUrl to
camelCase (testEmail and apiUrl) and update every usage within the file to
match; locate the declarations and references to TestEmail and ApiUrl in
apps/fastify/test/swagger-login.e2e.spec.ts and replace them with testEmail and
apiUrl respectively so names align with apps/next/e2e/auth-helpers.ts and follow
the camelCase guideline.
In `@apps/next/app/`(dashboard)/(news)/news-list.tsx:
- Around line 43-56: The thumbnail anchor currently uses href={a.url ?? '#'}
which creates a useless target="_blank" link when a.url is missing; update the
JSX in news-list.tsx to conditionally render: if a.url is truthy render the
existing <a ... href={a.url} target="_blank" rel="noopener noreferrer" ...>
wrapper, otherwise render a non-interactive element (e.g., <div> or <span>) with
the same className and inner <Image ... /> so the visual layout and
accessibility (alt text) remain, but avoid linking to '#'; locate the code
around the anchor and Image for article variable a and apply the conditional
rendering there.
In `@apps/next/app/`(dashboard)/(news)/page.tsx:
- Around line 15-17: The NewsAPI key is being passed in the URL string in the
fetch call (using newsQuery and key); remove the apiKey query param from the URL
and instead add the API key to the fetch options headers as "X-Api-Key": key so
authentication uses a header; keep the existing next: { revalidate: 300 } option
and ensure the constructed URL uses only q, pageSize, sortBy, language, etc.,
and that the fetch call still references newsQuery and key.
In `@apps/next/app/`(dashboard)/markets/markets-table.tsx:
- Around line 47-60: Change24hBadge currently treats null/undefined as zero by
using (value ?? 0), causing missing 24h data to render as positive; update the
logic in Change24hBadge to detect missing values (value === null || value ===
undefined) and apply a neutral style/class for that case (instead of the
positive or negative classes) while only using positive classes for value > 0
and negative classes for value < 0; make the same change for the other identical
badge usage at the second occurrence (the same component/JSX block around lines
176-183) so that missing data renders a neutral “—” style rather than a gain.
In `@apps/next/app/`(dashboard)/page-title.tsx:
- Around line 5-10: Update the pageTitles map in page-title.tsx so the root key
'/' reflects the new topic-based everything feed instead of "Top headlines";
locate the exported pageTitles Record and change the value for '/' to a label
like "Everything" (or the agreed feed name) to keep the shared header accurate.
In `@apps/next/app/`(dashboard)/sidebar.tsx:
- Around line 86-97: The logout link in SidebarFooter uses SidebarMenuButton
asChild wrapping a Next Link to "/auth/logout", which causes Next.js to prefetch
and may trigger the GET logout side-effect; replace the Link-based GET
navigation with an explicit POST form or client action: change the
SidebarMenuButton (used in SidebarMenuItem) to render a <form
action="/auth/logout" method="post"> with a submit button (or call a client-side
logout action) so the logout is performed only on explicit submit/click and not
via prefetch; ensure the tooltip and icon (LogOut, <span>Sign out</span>) remain
inside the button and remove the Link to prevent automatic prefetching.
In `@apps/next/lib/env.ts`:
- Around line 12-15: The NEXT_PUBLIC_NODE_ENV env schema currently defaults to
'development' and is optional, which enables debug rendering when env is
missing; update the zod schema for NEXT_PUBLIC_NODE_ENV (the
NEXT_PUBLIC_NODE_ENV enum declaration) to be required and not default to
'development' — either remove .optional() and .default('development') and leave
it required, or change the default to 'production' (e.g., set
.default('production') and remove .optional()) so the app will fail closed and
error pages won't expose raw debug details; after changing the schema,
regenerate any derived types/usages that rely on NEXT_PUBLIC_NODE_ENV.
In `@apps/next/next.config.mjs`:
- Around line 8-9: Rename the two config constants ApiProjectName and TeamSlug
to camelCase (apiProjectName and teamSlug), and update every usage/interpolation
that references them (including the string/template interpolation currently
using ApiProjectName and TeamSlug) to the new names; ensure any exports or
imports in next.config.mjs reference apiProjectName/teamSlug to satisfy the repo
camelCase rule and keep behavior unchanged.
In `@packages/ui/src/components/alert-dialog.tsx`:
- Around line 17-20: The trigger always applies 'cursor-pointer' which makes a
disabled AlertDialogPrimitive.Trigger appear interactive; update the className
computation in the AlertDialogPrimitive.Trigger usage so 'cursor-pointer' is
only added when the trigger is not disabled (check props.disabled or
aria-disabled), and add a disabled cursor override such as 'cursor-not-allowed'
or 'cursor-default' when disabled; adjust the cn(...) call around
AlertDialogPrimitive.Trigger and ensure className and {...props} are preserved.
In `@packages/ui/src/components/chart.tsx`:
- Line 212: The span element currently includes conflicting font classes
`font-heading` and `font-mono`; remove `font-mono` so only `font-heading` sets
the font-family (locate the span with className containing "text-foreground
font-heading font-mono font-semibold tabular-nums" in the Chart component in
packages/ui/src/components/chart.tsx and delete the `font-mono` token).
In `@packages/ui/src/components/menubar.tsx`:
- Line 45: The MenubarTrigger and MenubarSubTrigger className currently includes
'cursor-pointer' which makes disabled items look interactive; update their
className strings (the className used in MenubarTrigger and MenubarSubTrigger
declarations) to include data-[disabled]:pointer-events-none
data-[disabled]:opacity-50 data-[disabled]:cursor-default so Radix's
data-disabled state visually and functionally disables entries while keeping
existing styles for the enabled state.
- Line 207: The menubar class string uses the Tailwind utility "outline-none"
which must be replaced with "outline-hidden" for consistency with other
components (MenubarTrigger, MenubarItem, MenubarRadioItem, MenubarCheckboxItem)
and Tailwind v4 accessibility guidelines; locate the class array/string in the
Menubar component (the item containing 'focus:bg-accent
focus:text-accent-foreground data-[state=open]:bg-accent
data-[state=open]:text-accent-foreground flex cursor-pointer items-center
rounded-sm px-2 py-1.5 text-sm outline-none select-none data-[inset]:pl-8') and
change "outline-none" to "outline-hidden". Ensure no other occurrences in this
component remain as "outline-none" so the pattern matches the other menubar
components.
In `@packages/ui/src/components/table.tsx`:
- Around line 12-15: The container toggles between 'overflow-hidden' and
'overflow-x-auto' based on the fluid prop (data-slot="table-container"), which
clips wide tables because headers/cells still force whitespace-nowrap; instead
always keep horizontal scrolling on the container (always use overflow-x-auto)
and implement the "fluid" presentation by changing table/cell sizing (e.g.,
adjust table layout or cell wrapping classes) rather than hiding overflow;
update the JSX around the className conditional that references fluid in the
component (the div with data-slot="table-container") to remove the
overflow-hidden branch and ensure overflow-x-auto remains the default.
In `@packages/ui/src/styles/globals.css`:
- Line 83: The CSS custom property --font-mono is self-referential and must be
fixed: remove the var(--font-mono) reference from its definition so --font-mono
is defined only as the fallback stack (ui-monospace, monospace) or delete the
property entirely if you intend Tailwind/Next.js to inject and reference
--font-mono directly; update the declaration that currently reads the
--font-mono variable (line with --font-mono) accordingly to avoid the
var(--font-mono) self-cycle.
In `@packages/utils/src/web3/index.ts`:
- Around line 220-222: The numeric fallback currently uses Number(chainId) which
accepts non-canonical forms like '0x1' or '1e0'; restrict this by only allowing
decimal digit-only inputs before indexing evmChains. Update the logic around
chainId / numericId so that you only compute numericId and return
evmChains[numericId] when either chainId is a number type or chainId (when a
string) matches a digits-only regex like /^\d+$/; otherwise skip the numeric
fallback and fall through to the non-numeric resolution path. Keep references to
the existing variables chainId, numericId and evmChains so the change is
localized.
---
Nitpick comments:
In @.cursor/rules/frontend/design.mdc:
- Line 47: Split the long "Typography" bullet into multiple clearer
bullets/sub-bullets: keep one bullet for general typography principles
(distinctive fonts, pair display + body, reference
@.cursor/skills/frontend-design), a separate bullet for the crypto-dashboard
exception (explicitly note Inter for UI + Poppins for headings and mark it as an
exception), and another bullet listing detailed usage patterns (use `font-sans`
for UI text, `font-heading` / `font-heading font-semibold` for titles and large
stats, `numeric`/`tabular-nums` for prices, `font-mono` for code/API keys) so
each concept is scannable and independently editable.
In `@apps/fastify/src/routes/reference/template.ts`:
- Line 1: Rename the top-level template string constant named "Css" to use
camelCase ("css") everywhere it's declared and referenced; update the
declaration (const Css = `...`) to const css = `...` and replace all usages of
"Css" within this module (including the occurrence around line 256) to "css" so
identifiers remain consistent with camelCase conventions.
In `@apps/fastify/test/utils/db.ts`:
- Around line 82-90: Rename the constant currently declared as Tables to
camelCase tables (e.g., change "const Tables = [...] as const" to "const tables
= [...] as const") and update every usage/reference in this file (and any
exports) from Tables to tables so callers and internal code (e.g., any
map/filter/includes calls) use the new identifier; ensure any exported symbol
names or tests that referenced Tables are adjusted accordingly to avoid
undefined identifier errors.
In `@apps/next/components/assistant/assistant-chat.tsx`:
- Around line 142-145: The render code in assistant-chat.tsx currently assumes
JSON.stringify(output) succeeds and that the resulting str is a string before
calling .length; wrap the serialization of output (the output variable used to
build str for MessageResponse) in a defensive block: attempt JSON.stringify in a
try/catch, fall back to safe string conversions for BigInt/circular structures
(e.g., String(output) or a placeholder like "[unserializable output]"), ensure
str is explicitly a string (or empty string) before checking str.length and
before pushing <MessageResponse
key={`${message.id}-tool-${i}`}>{str}</MessageResponse>, and set hasContent only
when str is a non-empty string.
In `@biome.json`:
- Around line 149-176: The override currently turns off the linter rule
"useNamingConvention" for broad include globs like "**/packages/ui/**",
"**/packages/email/**", and "**/packages/notif/**", which over-exempts those
packages; restrict the scope by replacing those package-wide globs with targeted
patterns (for example only component directories or specific file types such as
components/exports or route handlers) so "useNamingConvention" remains enabled
elsewhere—update the "includes" entries that reference "**/packages/ui/**",
"**/packages/email/**", and "**/packages/notif/**" to more specific paths (e.g.,
component or handler subfolders and file extensions) rather than the entire
package directories.
In `@packages/ui/src/components/chart.tsx`:
- Line 12: Rename the exported constant Themes to camelCase "themes" to comply
with coding guidelines; update all usages and types that reference it (e.g.,
"keyof typeof themes", "Object.entries(themes)", any imports/exports or
destructuring) and ensure the const definition remains "const themes = { light:
'', dark: '.dark' } as const" so typings are preserved and no runtime behavior
changes.
In `@scripts/block-secret-files.mjs`:
- Line 6: Rename the PascalCase constants BlockedPatterns and AllowedPatterns to
camelCase blockedPatterns and allowedPatterns throughout the file (including
their declarations and every usage such as loop iterations or references like
"for (const p of BlockedPatterns)" → "for (const p of blockedPatterns)"); update
any other identifiers that reference those exact names (imports/exports,
function params, or tests) so all occurrences match the new camelCase names and
preserve original behavior.
In `@scripts/setup-database.mjs`:
- Around line 7-68: The constants Docker, DockerCompose, and Supabase should be
renamed to camelCase (docker, dockerCompose, supabase) to follow the project's
variable naming guideline; update the declarations (const Docker → const docker,
etc.) and then update every usage/reference in the repo or this module
accordingly (e.g., replace Docker[os] with docker[os],
DockerCompose.checkCommand with dockerCompose.checkCommand,
Supabase.getDownloadUrl with supabase.getDownloadUrl) to preserve behavior and
avoid breaking references.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b57f83c8-e285-4f96-80bf-4b2529682204
📒 Files selected for processing (93)
.cursor/rules/base/naming.mdc.cursor/rules/frontend/design.mdcapps/docu/app/docs/[[...slug]]/page.tsxapps/fastify/src/db/account.tsapps/fastify/src/db/health.tsapps/fastify/src/lib/api-key-auth.tsapps/fastify/src/lib/api-keys.tsapps/fastify/src/lib/crypto.tsapps/fastify/src/lib/env.tsapps/fastify/src/lib/web3-verify.tsapps/fastify/src/plugins/error-handler.tsapps/fastify/src/routes/account/link/wallet/unlink.test.tsapps/fastify/src/routes/account/link/wallet/verify.test.tsapps/fastify/src/routes/account/link/wallet/verify.tsapps/fastify/src/routes/ai/chat.tsapps/fastify/src/routes/auth/oauth/github/authorize-url.tsapps/fastify/src/routes/auth/oauth/github/authorize.tsapps/fastify/src/routes/auth/oauth/github/exchange.tsapps/fastify/src/routes/auth/web3/eip155/nonce.tsapps/fastify/src/routes/auth/web3/eip155/verify.test.tsapps/fastify/src/routes/auth/web3/eip155/verify.tsapps/fastify/src/routes/auth/web3/nonce.test.tsapps/fastify/src/routes/auth/web3/nonce.tsapps/fastify/src/routes/auth/web3/solana/nonce.test.tsapps/fastify/src/routes/auth/web3/solana/nonce.tsapps/fastify/src/routes/auth/web3/solana/verify.tsapps/fastify/src/routes/auth/web3/validate-address.tsapps/fastify/src/routes/reference/template.tsapps/fastify/test/playwright-global-setup.tsapps/fastify/test/swagger-login.e2e.spec.tsapps/fastify/test/utils/db.tsapps/next/app/(dashboard)/(news)/news-list.tsxapps/next/app/(dashboard)/(news)/page.tsxapps/next/app/(dashboard)/layout.tsxapps/next/app/(dashboard)/markets/markets-table.tsxapps/next/app/(dashboard)/markets/page.tsxapps/next/app/(dashboard)/page-title.tsxapps/next/app/(dashboard)/settings/(profile)/page.tsxapps/next/app/(dashboard)/settings/(profile)/profile-section.tsxapps/next/app/(dashboard)/settings/security/api-keys-card.tsxapps/next/app/(dashboard)/settings/security/passkeys-card.tsxapps/next/app/(dashboard)/settings/security/security-section.tsxapps/next/app/(dashboard)/settings/security/totp-card.tsxapps/next/app/(dashboard)/sidebar.tsxapps/next/app/error.tsxapps/next/app/layout.tsxapps/next/components/assistant/assistant-chat.tsxapps/next/components/assistant/assistant-sidebar.tsxapps/next/components/assistant/index.tsxapps/next/components/shared/error-boundary.tsxapps/next/e2e/auth-helpers.tsapps/next/e2e/chat-assistant.spec.tsapps/next/e2e/magic-link-auth.spec.tsapps/next/e2e/playwright-global-setup.tsapps/next/lib/auth/auth-error-messages.tsapps/next/lib/env.tsapps/next/next.config.mjsbiome.jsonpackages/ui/src/components/accordion.tsxpackages/ui/src/components/alert-dialog.tsxpackages/ui/src/components/breadcrumb.tsxpackages/ui/src/components/button.tsxpackages/ui/src/components/card.tsxpackages/ui/src/components/chart.tsxpackages/ui/src/components/checkbox.tsxpackages/ui/src/components/collapsible.tsxpackages/ui/src/components/command.tsxpackages/ui/src/components/context-menu.tsxpackages/ui/src/components/dialog.tsxpackages/ui/src/components/drawer.tsxpackages/ui/src/components/dropdown-menu.tsxpackages/ui/src/components/hover-card.tsxpackages/ui/src/components/menubar.tsxpackages/ui/src/components/navigation-menu.tsxpackages/ui/src/components/popover.tsxpackages/ui/src/components/radio-group.tsxpackages/ui/src/components/select.tsxpackages/ui/src/components/sheet.tsxpackages/ui/src/components/sidebar.tsxpackages/ui/src/components/switch.tsxpackages/ui/src/components/table.tsxpackages/ui/src/components/tabs.tsxpackages/ui/src/components/tooltip.tsxpackages/ui/src/hooks/use-mobile.tspackages/ui/src/styles/globals.csspackages/utils/src/web3/alchemy.tspackages/utils/src/web3/index.tsscripts/block-secret-files.mjsscripts/ensure-tool.mjsscripts/setup-database.mjsscripts/setup-gitleaks.mjsscripts/setup-osv-scanner.mjstools/eslint/base.js
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/next/e2e/magic-link-auth.spec.ts`:
- Line 13: The test's Sign out locator (signOutButton created via
page.getByRole('link', { name: 'Sign out' }).first()) is brittle because
multiple "Sign out" links exist; scope the locator to the intended container or
add a disambiguating test id. Update the locator by narrowing it to the
sidebar/footer container (e.g.,
page.locator('<sidebar-or-footer-selector>').getByRole('link', { name: 'Sign
out' })) or change the app code to add a data-testid on the specific sidebar
Sign out link and use page.getByTestId('sidebar-signout') to locate it in the
magic-link-auth.spec.ts test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2976878d-fac9-4343-8c33-7d142abddc67
📒 Files selected for processing (1)
apps/next/e2e/magic-link-auth.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/utils/src/web3/index.ts (1)
220-224:⚠️ Potential issue | 🟡 MinorThis fallback still accepts zero-padded chain ID aliases.
/^\d+$/blocks hex/scientific notation, but it still lets values like'01'and'0008453'resolve to supported EVM chains. Since Line 209 already serves canonical string IDs, this branch only widens the accepted input surface for malformed identifiers.✂️ Suggested fix
- // Try parsing as decimal digits-only string (reject '0x1', '1e0', etc.) - if (isString(chainId) && /^\d+$/.test(chainId)) { - const numericId = Number(chainId) - return evmChains[numericId] - }This snippet shows the remaining aliasing behavior:
#!/bin/bash node - <<'NODE' const evmChains = { } const chainRegistry = new Map( Object.keys(evmChains).map(id => [id, evmChains[id]]) ) function current(chainId) { const byChainId = chainRegistry.get(chainId) if (byChainId) return byChainId if (/^\d+$/.test(chainId)) { const numericId = Number(chainId) return evmChains[numericId] } return undefined } for (const input of ['1', '01', '8453', '0008453']) { console.log(input, '=>', current(input)?.name ?? 'undefined') } NODE🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/utils/src/web3/index.ts` around lines 220 - 224, The numeric-string fallback in the current function (the branch using isString(chainId) && /^\d+$/.test(chainId)) still accepts zero-padded aliases like "01"; change it to only accept canonical decimal strings without leading zeros (allow "0" or non-zero-leading numbers). Replace the /^\d+$/ test with a stricter check such as /^(?:0|[1-9]\d*)$/ or equivalently verify String(Number(chainId)) === chainId before converting to Number(chainId), then return evmChains[numericId] as before.
🧹 Nitpick comments (2)
apps/next/app/(dashboard)/layout.tsx (2)
43-43: Hardcoded height creates implicit coupling with header.The
calc(100dvh - 3.5rem)is tightly coupled to the header'sh-14(3.5rem). If the header height changes, this calculation will break silently.Consider using CSS custom properties or a shared constant for maintainability:
♻️ Alternative using CSS variable
- <div className="flex min-h-0 flex-1" style={{ height: 'calc(100dvh - 3.5rem)' }}> + <div className="flex min-h-0 flex-1" style={{ height: 'calc(100dvh - var(--header-height, 3.5rem))' }}>Then define
--header-heightin the header or a parent element.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/next/app/`(dashboard)/layout.tsx at line 43, Replace the hardcoded inline style height "calc(100dvh - 3.5rem)" on the div with a reference to a shared header height value so changes to the header don't break layout: expose the header height as a CSS custom property (e.g., --header-height) on the header/root and update the div (the element with class "flex min-h-0 flex-1") to use calc(100dvh - var(--header-height)); alternatively, read a shared JS/TS constant (headerHeight) and interpolate it into the style for that same div so the value is single-sourced and maintainable.
14-14: Unnecessaryasynckeyword.The layout function is marked
asyncbut contains noawaitstatements. While harmless, removing it clarifies intent.♻️ Suggested change
-export default async function Layout({ children }: Readonly<{ children: React.ReactNode }>) { +export default function Layout({ children }: Readonly<{ children: React.ReactNode }>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/next/app/`(dashboard)/layout.tsx at line 14, The Layout function is declared async but contains no awaits; remove the unnecessary async modifier from the export default async function Layout(...) declaration so it becomes a plain synchronous export (update the signature of Layout to export default function Layout({ children }: Readonly<{ children: React.ReactNode }>) and ensure no other code relies on it returning a Promise).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/next/app/`(dashboard)/markets/markets-table.tsx:
- Around line 151-166: The coin name span in the desktop row (inside the div
with className "min-w-0") doesn't honor flex width so long names overflow;
change the inline <span className="truncate font-medium"> used for the coin name
to a block-level element (e.g., <p> or <div>) so truncation works inside the
flex container, and ensure the sibling symbol element remains a separate
block/inline-block (or also use <p className="ml-1 shrink-0
text-muted-foreground text-sm uppercase">) so layout and ellipsis behave like
the mobile layout; update the JSX around the coin name/symbol (the elements
currently rendering c.name and c.symbol) accordingly.
In `@packages/utils/src/web3/index.ts`:
- Around line 213-214: Remove the unsafe plain-object fallback lookup using
solanaClusters[chainId]; instead rely solely on the canonical chainRegistry
lookup that was populated earlier (chainRegistry). Replace the two-line fallback
(const bySolanaCluster = solanaClusters[chainId]; if (bySolanaCluster) return
bySolanaCluster) with a single check against chainRegistry (e.g., retrieve the
cluster via chainRegistry lookup for chainId and return it if present), or
simply omit the fallback entirely so only chainRegistry is used—this eliminates
the prototype-pollution risk and ensures the function returns only registered
clusters.
---
Duplicate comments:
In `@packages/utils/src/web3/index.ts`:
- Around line 220-224: The numeric-string fallback in the current function (the
branch using isString(chainId) && /^\d+$/.test(chainId)) still accepts
zero-padded aliases like "01"; change it to only accept canonical decimal
strings without leading zeros (allow "0" or non-zero-leading numbers). Replace
the /^\d+$/ test with a stricter check such as /^(?:0|[1-9]\d*)$/ or
equivalently verify String(Number(chainId)) === chainId before converting to
Number(chainId), then return evmChains[numericId] as before.
---
Nitpick comments:
In `@apps/next/app/`(dashboard)/layout.tsx:
- Line 43: Replace the hardcoded inline style height "calc(100dvh - 3.5rem)" on
the div with a reference to a shared header height value so changes to the
header don't break layout: expose the header height as a CSS custom property
(e.g., --header-height) on the header/root and update the div (the element with
class "flex min-h-0 flex-1") to use calc(100dvh - var(--header-height));
alternatively, read a shared JS/TS constant (headerHeight) and interpolate it
into the style for that same div so the value is single-sourced and
maintainable.
- Line 14: The Layout function is declared async but contains no awaits; remove
the unnecessary async modifier from the export default async function
Layout(...) declaration so it becomes a plain synchronous export (update the
signature of Layout to export default function Layout({ children }: Readonly<{
children: React.ReactNode }>) and ensure no other code relies on it returning a
Promise).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47e31fbf-f7fd-462b-84a8-49996d2fe19f
📒 Files selected for processing (22)
.cursor/rules/frontend/design.mdcapps/fastify/src/routes/ai/chat.tsapps/fastify/src/routes/reference/template.tsapps/fastify/test/playwright-global-setup.tsapps/fastify/test/swagger-login.e2e.spec.tsapps/fastify/test/utils/db.tsapps/next/app/(dashboard)/(news)/news-list.tsxapps/next/app/(dashboard)/(news)/page.tsxapps/next/app/(dashboard)/layout.tsxapps/next/app/(dashboard)/markets/markets-table.tsxapps/next/app/(dashboard)/page-title.tsxapps/next/app/(dashboard)/sidebar.tsxapps/next/components/assistant/assistant-chat.tsxapps/next/lib/env.tsapps/next/next.config.mjspackages/ui/src/components/alert-dialog.tsxpackages/ui/src/components/chart.tsxpackages/ui/src/components/menubar.tsxpackages/ui/src/styles/globals.csspackages/utils/src/web3/index.tsscripts/block-secret-files.mjsscripts/setup-database.mjs
🚧 Files skipped from review as they are similar to previous changes (10)
- apps/fastify/test/swagger-login.e2e.spec.ts
- apps/next/components/assistant/assistant-chat.tsx
- apps/next/app/(dashboard)/(news)/news-list.tsx
- apps/next/app/(dashboard)/(news)/page.tsx
- apps/next/app/(dashboard)/sidebar.tsx
- packages/ui/src/components/chart.tsx
- apps/next/lib/env.ts
- apps/fastify/src/routes/reference/template.ts
- apps/next/next.config.mjs
- packages/ui/src/components/alert-dialog.tsx
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/api-e2e.yml:
- Line 26: The workflow currently hard-codes OLLAMA_BASE_URL to a public
endpoint, which causes flaky PR runs and allows untrusted PR code to call
external services; change the job to stop using that live URL by either pointing
OLLAMA_BASE_URL to a local/mock service (e.g., http://localhost:<port> provided
by a test container) or by gating use of the real provider behind a trusted-only
path (e.g., only set the real OLLAMA_BASE_URL from a secret when
github.event_name == 'push' or another trusted condition). Update the api-e2e
workflow so the OLLAMA_BASE_URL environment variable is set from the mock
service started in the job or from a repository secret conditional on trusted
runs, and remove the hard-coded https://ollama.gaboesquivel.com value.
In @.github/workflows/next-e2e.yml:
- Line 30: The workflow is exposing a shared personal Ollama host via the
OLLAMA_BASE_URL environment key in next-e2e.yml; remove or replace that
hard-coded URL and either point OLLAMA_BASE_URL to a local/mock provider (e.g.,
localhost or a test stub) or gate the networked AI path so the job only runs for
trusted triggers (restrict to workflow_dispatch or specific branches in the
workflow triggers or wrap steps that use OLLAMA_BASE_URL with a conditional like
github.event_name == 'workflow_dispatch' or branch checks); update the
OLLAMA_BASE_URL reference in the job to read from a sanitized secret or the mock
endpoint and adjust any steps that call the Ollama client to respect the new
conditional.
In `@apps/docu/content/docs/deployment/self-hosted-llm.mdx`:
- Line 255: The markdown link referencing the internal setup uses the
placeholder org "your-org"; update the URL for the link text
"__dev/ollama-setup.md" so the GitHub path points to the real organization
(replace "your-org" with the repository owner/organization name used for this
project) ensuring the link becomes
https://github.com/<actual-org>/basilic/blob/main/__dev/ollama-setup.md.
In `@apps/fastify/src/lib/env.ts`:
- Around line 57-59: The OLLAMA_BASE_URL environment schema currently defaults
to a personal domain; update the zod schema for OLLAMA_BASE_URL in env.ts
(symbol: OLLAMA_BASE_URL) to avoid leaking requests: either remove the
.default(...) so the value is optional/required and must be explicitly provided,
or change the default to a local development endpoint such as
'http://localhost:11434' to be safe for local runs; ensure the type remains
z.string().url().optional() (or adjust optionality per your chosen approach) and
update any docs or startup checks that assume a default.
In `@apps/fastify/src/routes/ai/chat.ts`:
- Around line 74-97: The getProvider function is hiding an SDK mismatch by
casting responses to LanguageModel; upgrade `@ai-sdk/openai` (and OpenRouter SDK)
to a compatible major (match ai@6.x, e.g., ^5.x or ^6.x) so createOpenAI(...)
and createOpenRouter(...) return the current LanguageModelV2/LanguageModelV3
types, then remove the unsafe casts (remove "as unknown as LanguageModel" in
getProvider) and update any callsites to use the new model interface; ensure
types for modelId construction (openRouterModelAliases/effective) remain
compatible and preserve API key handling (env.OPEN_ROUTER_API_KEY) after the
dependency bump.
In `@apps/fastify/vitest.setup.ts`:
- Around line 14-15: Update the vitest setup comment that currently says tests
require OLLAMA_BASE_URL or OPEN_ROUTER_API_KEY in .env.test to clarify the
source can be any environment (local or CI): change the wording to something
like "via environment (e.g., .env.test or CI env)" so it reads that the provider
config can come from environment variables rather than only .env.test; keep
references to OLLAMA_BASE_URL, OPEN_ROUTER_API_KEY and the note about
AI_DEFAULT_MODEL unchanged.
In `@turbo.json`:
- Around line 42-54: Remove the duplicate "AI_DEFAULT_MODEL" entry from the env
array in turbo.json (the env list used by the lint:eslint task); locate the env
array that currently contains "AI_DEFAULT_MODEL" twice and delete the redundant
occurrence so each environment variable appears only once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f33aaccc-b5ed-49c6-b0d9-02ee4c0198f5
⛔ Files ignored due to path filters (4)
apps/fastify/.env-sampleis excluded by!**/.env*apps/fastify/.env.test.exampleis excluded by!**/.env*packages/core/src/gen/sdk.gen.tsis excluded by!**/gen/**,!**/gen/**,!**/*.gen.tspnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (18)
.github/workflows/api-e2e.yml.github/workflows/lint.yml.github/workflows/next-e2e.yml.github/workflows/packages-test.ymlapps/docu/content/docs/deployment/index.mdxapps/docu/content/docs/deployment/meta.jsonapps/docu/content/docs/deployment/self-hosted-llm.mdxapps/docu/content/docs/deployment/vercel.mdxapps/docu/content/docs/testing/e2e-testing.mdxapps/fastify/openapi/openapi.jsonapps/fastify/package.jsonapps/fastify/scripts/generate-openapi.tsapps/fastify/src/lib/env.tsapps/fastify/src/routes/ai/chat.test.tsapps/fastify/src/routes/ai/chat.tsapps/fastify/vitest.config.tsapps/fastify/vitest.setup.tsturbo.json
✅ Files skipped from review due to trivial changes (4)
- apps/fastify/src/routes/ai/chat.test.ts
- apps/docu/content/docs/deployment/index.mdx
- apps/docu/content/docs/deployment/vercel.mdx
- .github/workflows/packages-test.yml
|
|
||
| - [Vercel Deployment](/docs/deployment/vercel) — Frontend and API deployment | ||
| - [Portability Strategy](/docs/architecture/portability) — Migration paths | ||
| - [__dev/ollama-setup.md](https://github.com/your-org/basilic/blob/main/__dev/ollama-setup.md) — Full server setup reference (internal) |
There was a problem hiding this comment.
Placeholder organization in link.
The internal reference link contains your-org which should be updated to the actual organization name.
Suggested fix
-- [__dev/ollama-setup.md](https://github.com/your-org/basilic/blob/main/__dev/ollama-setup.md) — Full server setup reference (internal)
+- [__dev/ollama-setup.md](https://github.com/blockmatic/basilic/blob/main/__dev/ollama-setup.md) — Full server setup reference (internal)📝 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
| - [__dev/ollama-setup.md](https://github.com/your-org/basilic/blob/main/__dev/ollama-setup.md) — Full server setup reference (internal) | |
| - [__dev/ollama-setup.md](https://github.com/blockmatic/basilic/blob/main/__dev/ollama-setup.md) — Full server setup reference (internal) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/docu/content/docs/deployment/self-hosted-llm.mdx` at line 255, The
markdown link referencing the internal setup uses the placeholder org
"your-org"; update the URL for the link text "__dev/ollama-setup.md" so the
GitHub path points to the real organization (replace "your-org" with the
repository owner/organization name used for this project) ensuring the link
becomes https://github.com/<actual-org>/basilic/blob/main/__dev/ollama-setup.md.
Comment on lines
+74
to
97
| function getProvider(provider: ResolvedProvider, modelParam?: string): LanguageModel { | ||
| if (provider === 'ollama') { | ||
| const defaultModel = env.AI_DEFAULT_MODEL ?? defaultOllamaModel | ||
| const m = (modelParam?.trim().length ?? 0) > 0 ? modelParam?.trim() : undefined | ||
| const useDefault = | ||
| m === undefined || m === defaultOllamaModel || m === 'aurora-alpha' || m === 'default' | ||
| const modelId = useDefault ? defaultModel : (m ?? defaultModel) | ||
| const openai = createOpenAI({ | ||
| baseURL: `${env.OLLAMA_BASE_URL}/v1`, | ||
| apiKey: 'ollama', | ||
| }) | ||
| return openai.chat(modelId) as unknown as LanguageModel | ||
| } | ||
| const m = (modelParam?.trim().length ?? 0) > 0 ? modelParam?.trim() : undefined | ||
| const defaultModel = env.AI_DEFAULT_MODEL ?? defaultOpenRouterModel | ||
| const useRuntimeDefault = m === undefined || m === defaultOpenRouterModel || m === 'aurora-alpha' | ||
| const effective = useRuntimeDefault ? defaultModel : (m ?? defaultModel) | ||
| const modelId = | ||
| openRouterModelAliases[effective] ?? | ||
| (effective.startsWith('gpt') ? `openai/${effective}` : effective) | ||
| const apiKey = env.OPEN_ROUTER_API_KEY | ||
| if (!apiKey) throw new Error('OPEN_ROUTER_API_KEY required for Open Router') | ||
| return createOpenRouter({ apiKey }).chat(modelId) as unknown as LanguageModel | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does @ai-sdk/openai version 1.0.0 return types compatible with ai package LanguageModel?
💡 Result:
No—not with current ai.
@ai-sdk/openai@1.0.0 is from the pre–AI SDK 5 provider/spec era, while modern ai versions use the newer V2/V3 model specs (LanguageModelV2 / LanguageModelV3) rather than the old LanguageModel type. AI SDK 5 explicitly introduced updated V2 specifications across providers/models, and current ai type definitions expose LanguageModelV2/LanguageModelV3 (not the old LanguageModel). [1] [2]
What to do:
- If you’re on a modern
ai(AI SDK 5+), upgrade@ai-sdk/openaito a matching major version and use it as shown in current docs. [3] - If you must stay on
@ai-sdk/openai@1.0.0, you’ll likely need to downgradeaito the older major that still used the oldLanguageModeltype.
Sources
[1] (ai-sdk.dev)
[2] (app.unpkg.com)
[3] (ai-sdk.dev)
Citations:
- 1: https://ai-sdk.dev/docs/announcing-ai-sdk-5-alpha
- 2: https://app.unpkg.com/ai%406.0.41/files/dist/index.d.ts
- 3: https://ai-sdk.dev/docs/getting-started/nodejs
🏁 Script executed:
# Check the package.json for SDK versions
cd apps/fastify && cat -n package.json | grep -E '"(`@ai-sdk/openai`|@openrouter/ai-sdk-provider|"ai")"|"dependencies"|"devDependencies"' -A 20Repository: blockmatic/basilic
Length of output: 2018
Upgrade @ai-sdk/openai to match ai@6.x major version.
Lines 85 and 96 cast to as unknown as LanguageModel to mask a critical SDK version incompatibility. @ai-sdk/openai@1.0.0 is pre–AI SDK 5 and returns the old LanguageModel type, while ai@6.0.86 expects LanguageModelV2 or LanguageModelV3. These versions are fundamentally incompatible.
Upgrade @ai-sdk/openai to ^5.x or ^6.x (matching the major version of ai), and remove the type casts. The OpenRouter provider should be similarly aligned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/fastify/src/routes/ai/chat.ts` around lines 74 - 97, The getProvider
function is hiding an SDK mismatch by casting responses to LanguageModel;
upgrade `@ai-sdk/openai` (and OpenRouter SDK) to a compatible major (match ai@6.x,
e.g., ^5.x or ^6.x) so createOpenAI(...) and createOpenRouter(...) return the
current LanguageModelV2/LanguageModelV3 types, then remove the unsafe casts
(remove "as unknown as LanguageModel" in getProvider) and update any callsites
to use the new model interface; ensure types for modelId construction
(openRouterModelAliases/effective) remain compatible and preserve API key
handling (env.OPEN_ROUTER_API_KEY) after the dependency bump.
Comment on lines
42
to
54
| "env": [ | ||
| "AUTH_COOKIE_NAME", | ||
| "AI_DEFAULT_MODEL", | ||
| "NEWSAPI_KEY", | ||
| "NODE_ENV", | ||
| "CI", | ||
| "DATABASE_URL", | ||
| "PGLITE", | ||
| "RUN_PG_MIGRATE", | ||
| "OLLAMA_BASE_URL", | ||
| "OPEN_ROUTER_API_KEY", | ||
| "AI_DEFAULT_MODEL", | ||
| "VITEST", |
There was a problem hiding this comment.
Duplicate AI_DEFAULT_MODEL entry in env array.
AI_DEFAULT_MODEL appears twice in the lint:eslint task's env list (lines 44 and 53). This is harmless but indicates a copy-paste oversight.
Suggested fix
"lint:eslint": {
"env": [
"AUTH_COOKIE_NAME",
- "AI_DEFAULT_MODEL",
"NEWSAPI_KEY",
"NODE_ENV",
"CI",
"DATABASE_URL",
"PGLITE",
"RUN_PG_MIGRATE",
"OLLAMA_BASE_URL",
"OPEN_ROUTER_API_KEY",
"AI_DEFAULT_MODEL",📝 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
| "env": [ | |
| "AUTH_COOKIE_NAME", | |
| "AI_DEFAULT_MODEL", | |
| "NEWSAPI_KEY", | |
| "NODE_ENV", | |
| "CI", | |
| "DATABASE_URL", | |
| "PGLITE", | |
| "RUN_PG_MIGRATE", | |
| "OLLAMA_BASE_URL", | |
| "OPEN_ROUTER_API_KEY", | |
| "AI_DEFAULT_MODEL", | |
| "VITEST", | |
| "env": [ | |
| "AUTH_COOKIE_NAME", | |
| "NEWSAPI_KEY", | |
| "NODE_ENV", | |
| "CI", | |
| "DATABASE_URL", | |
| "PGLITE", | |
| "RUN_PG_MIGRATE", | |
| "OLLAMA_BASE_URL", | |
| "OPEN_ROUTER_API_KEY", | |
| "AI_DEFAULT_MODEL", | |
| "VITEST", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@turbo.json` around lines 42 - 54, Remove the duplicate "AI_DEFAULT_MODEL"
entry from the env array in turbo.json (the env list used by the lint:eslint
task); locate the env array that currently contains "AI_DEFAULT_MODEL" twice and
delete the redundant occurrence so each environment variable appears only once.
- Upgrade @ai-sdk/openai from v1 to v3 (fixes UnsupportedModelVersionError) - Use ollama(modelId) instead of openai.chat(modelId) for v3 API - Enforce Ollama at ollama.gaboesquivel.com in vitest.setup.ts - Add skipIfProviderUnavailable and throttle between AI chat tests - Update .env.test.example and .env-sample with Ollama defaults
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/fastify/vitest.setup.ts`:
- Line 23: The test task in turbo.json needs AI_PROVIDER added to its env list
because apps/fastify/vitest.setup.ts sets process.env.AI_PROVIDER = 'ollama';
update the "test" task's env array to include "AI_PROVIDER" alongside
"OLLAMA_BASE_URL" so Turbo can invalidate the cache when the provider changes;
locate the "test" task entry named "test" and add the "AI_PROVIDER" key to its
env list so the environment dependency matches the vitest setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6458e44e-b4c3-4f9f-a52d-367ca79650bf
⛔ Files ignored due to path filters (3)
apps/fastify/.env-sampleis excluded by!**/.env*apps/fastify/.env.test.exampleis excluded by!**/.env*pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (6)
apps/docu/content/docs/deployment/self-hosted-llm.mdxapps/fastify/package.jsonapps/fastify/src/routes/ai/chat.test.tsapps/fastify/src/routes/ai/chat.tsapps/fastify/src/routes/auth/web3/eip155/verify.tsapps/fastify/vitest.setup.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/fastify/package.json
gaboesquivel
changed the title
gaboesquivel
changed the title
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
1 participant
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 CodeRabbit
New Features
Improvements
Style
Documentation