Open
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds multiple Next.js App Router API route handlers under app/api/evaluations/stt/ that proxy requests to a backend (NEXT_PUBLIC_BACKEND_URL), enforce X-API-KEY for write operations, remove several legacy STT/audio endpoints, and make error handlers more robust by stringifying non-Error exceptions in Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant NextAPI as Next.js API (app/api/...)
participant Backend as Backend service (NEXT_PUBLIC_BACKEND_URL)
Client->>NextAPI: HTTP GET/POST (may include X-API-KEY)
NextAPI->>Backend: Forward request to /api/v1/... (preserve headers/body)
Backend-->>NextAPI: Response (status + JSON)
NextAPI-->>Client: Mirror backend status and JSON
alt fetch/forward error
NextAPI-->>Client: 500 with { success:false, error: ..., details: String(error), data: null }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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: 10
🤖 Fix all issues with AI agents
In `@app/api/evaluations/stt/datasets/`[dataset_id]/route.ts:
- Around line 20-24: The catch block currently returns an inaccurate error
message ("Failed to fetch config"); update the response in the catch inside
route.ts so the NextResponse.json call uses a message referencing the dataset
(e.g., "Failed to fetch dataset" or include the dataset_id) instead of "config".
Locate the catch block that calls NextResponse.json and replace the error string
to clearly reference the dataset (and optionally include dataset_id or error
details if available) so the returned error message matches the operation
performed.
In `@app/api/evaluations/stt/datasets/route.ts`:
- Around line 56-61: The catch block in the POST handler currently assumes error
has a .message property (console.error and NextResponse.json using
error.message); update the POST catch to normalize the error into a safe string
(e.g., const msg = error instanceof Error ? error.message : String(error)) and
use that safe value in both the console.error and the NextResponse.json details
so we don't access .message on unknown types; look for the catch in the POST
route handler in route.ts and replace direct error.message usage with this
normalized message before returning the response.
- Around line 18-22: The catch block in the route handler currently assumes
`error.message` exists; update the error handling in the catch clause to narrow
the unknown `error` with `if (error instanceof Error)` and use `error.message`
when true, otherwise provide a safe fallback string (e.g., "Unknown error") when
building the NextResponse JSON; locate and update the catch in the evaluation
STT datasets route (the block that returns NextResponse.json({ success: false,
error: error.message, ... })) to implement this instanceof Error narrowing and
fallback.
- Around line 16-17: The handler currently returns NextResponse.json(data) but
does not forward the upstream HTTP status (same bug as runs/route.ts); change
the return to preserve the original response status (and optionally headers) by
returning NextResponse.json(data, { status: response.status /*, headers:
response.headers */ }); locate the variables response and data in route.ts and
update the NextResponse.json call accordingly.
In `@app/api/evaluations/stt/results/`[result_id]/route.ts:
- Line 8: The code uses a NEXT_PUBLIC_ prefixed env var for server-only route
handlers, which leaks to the client; update the backendUrl constant to read a
non-public variable (e.g., process.env.BACKEND_URL) with the existing fallback
('http://localhost:8000') and replace occurrences in this file's backendUrl
declaration (and the same change in the other five route files in this PR) so
the backend URL is read from a server-only env var instead of
NEXT_PUBLIC_BACKEND_URL.
- Around line 20-24: The catch block returns an inaccurate error message
("Failed to fetch config") — update the NextResponse.json call in the route's
catch handler to return a correct message like "Failed to fetch result" (keep
success:false, data:null and status:500) so the error reflects the endpoint's
purpose; apply the same fix to the analogous catch handlers in the
datasets/[dataset_id] and runs/[run_id] routes, locating the places by searching
for NextResponse.json(...) in the catch blocks of those route handlers.
In `@app/api/evaluations/stt/runs/`[run_id]/route.ts:
- Around line 20-24: The catch block in route.ts returns an inaccurate error
message; update the NextResponse.json call inside the catch (where it currently
returns { success: false, error: 'Failed to fetch config', ... }) to reference
the run (e.g., 'Failed to fetch run' or include the run_id) so the error
correctly describes the failed operation; modify the error string passed to
NextResponse.json in that catch block (the code around NextResponse.json)
accordingly.
In `@app/api/evaluations/stt/runs/route.ts`:
- Around line 18-22: The catch blocks currently access error.message on an
unknown-typed catch variable (seen where NextResponse.json is called in the
catch), which breaks TypeScript strict checks; update each catch (including in
app/api/evaluations/stt/runs/route.ts, datasets/route.ts and runs/route.ts) to
safely narrow the error before reading message—e.g. determine const msg = error
instanceof Error ? error.message : String(error) (or JSON.stringify when
appropriate) and pass msg into NextResponse.json instead of error.message so the
code compiles under strict/useUnknownInCatchVariables.
- Around line 56-61: The catch block in the POST handler currently uses
error.message which will throw if error isn't an Error; update the POST catch
block (the try/catch around the request forward) to safely extract a message by
checking if error is an instance of Error and using error.message, otherwise
convert the error to a string (e.g., String(error) or JSON.stringify fallback)
and include that safe message in the NextResponse.json details and the
console.error call so unknown error shapes don't cause a secondary failure.
- Around line 16-17: The GET handler currently returns the backend JSON body but
drops the original HTTP status; update the response to forward the backend
status by returning NextResponse.json(data, { status: response.status }) instead
of NextResponse.json(data). Apply the same change to the analogous GET handler
in datasets/route.ts so both app/api/evaluations/stt/runs/route.ts and
datasets/route.ts forward response.status via NextResponse.json.
🧹 Nitpick comments (6)
app/api/evaluations/stt/results/[result_id]/route.ts (1)
1-1:NextRequestis imported but unused.The
requestparameter is typed asRequest, notNextRequest. Either useNextRequestas the parameter type or remove the unused import.Proposed fix
-import { NextResponse, NextRequest } from 'next/server'; +import { NextResponse } from 'next/server';app/api/evaluations/stt/datasets/[dataset_id]/route.ts (1)
1-1: UnusedNextRequestimport — same as the other route files.app/api/evaluations/stt/runs/[run_id]/route.ts (1)
1-1: UnusedNextRequestimport.app/api/evaluations/stt/files/route.ts (3)
37-41: Redundant branch — both paths return the identical response.The
if (!response.ok)check returns the exact same expression as the else branch. Remove the conditional.Proposed fix
- // Return the response with the same status code - if (!response.ok) { - return NextResponse.json(data, { status: response.status }); - } - - return NextResponse.json(data, { status: response.status }); + return NextResponse.json(data, { status: response.status });
42-42: Avoid explicitany— use proper type narrowing instead.Typing
errorasanysuppresses useful type safety. Prefer theunknown+instanceofpattern used elsewhere or at minimum keep itunknown.Proposed fix
- } catch (error: any) { + } catch (error) { console.error('Proxy error:', error); return NextResponse.json( - { error: 'Failed to forward request to backend', details: error.message }, + { error: 'Failed to forward request to backend', details: error instanceof Error ? error.message : 'Unknown error' }, { status: 500 } ); }
1-49: Consider extracting a shared proxy utility to reduce duplication across all six route files.All six route handlers repeat the same pattern: read env var, forward API key, fetch backend, return JSON, catch errors. A small shared helper (e.g.,
lib/proxy.ts) accepting the path and options would eliminate ~80% of the boilerplate and make error handling/status forwarding consistent in one place.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/api/evaluations/datasets/route.ts (1)
32-32:⚠️ Potential issue | 🟡 Minor
response.json()in GET/POST can throw on non-JSON responsesUnlike the DELETE handler in
[dataset_id]/route.ts(which wraps the JSON parse in a nested try-catch), the GET and POST handlers callawait response.json()without a guard. If the backend returns a non-JSON body (e.g., a gateway timeout HTML page), this propagates an unhandled exception instead of a clean 500.🛡️ Proposed fix
- // Get the response data - const data = await response.json(); + // Get the response data + let data: unknown; + try { + data = await response.json(); + } catch { + data = { success: !response.ok ? false : true }; + }Apply the same guard to the POST handler (around line 84).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/datasets/route.ts` at line 32, The GET and POST handlers call await response.json() directly which can throw on non-JSON responses; wrap the response.json() call in a try-catch (same pattern used in the DELETE handler in [dataset_id]/route.ts) inside both the GET and POST handlers, capture parse errors, log the error/context, and return a clean 500 response (or set data to null/empty and handle accordingly) instead of letting the exception propagate; locate the usages of response.json() in the GET and POST handler functions and add the nested try-catch around those calls.
🧹 Nitpick comments (1)
app/api/evaluations/datasets/route.ts (1)
21-21:NEXT_PUBLIC_BACKEND_URLleaks the backend origin into the client bundleThe
NEXT_PUBLIC_prefix causes the variable to be statically inlined into the browser-side JavaScript at build time. Route handlers run exclusively server-side and don't need a public env var; use a privateBACKEND_URLinstead to avoid exposing the backend host in the client bundle.♻️ Proposed refactor
- const backendUrl = process.env.NEXT_PUBLIC_BACKEND_URL || 'http://localhost:8000'; + const backendUrl = process.env.BACKEND_URL || 'http://localhost:8000';Apply the same change to line 71 in this file and every other route handler that uses
NEXT_PUBLIC_BACKEND_URL.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/datasets/route.ts` at line 21, Replace usage of the public env var by reading the private BACKEND_URL instead: change the const backendUrl assignment that currently uses process.env.NEXT_PUBLIC_BACKEND_URL to use process.env.BACKEND_URL with the same fallback ('http://localhost:8000'), and update the same pattern at the other occurrence (the backendUrl declaration around line 71) and in any other server-only route handler files that reference NEXT_PUBLIC_BACKEND_URL so the backend origin is not exposed to the client bundle; keep the variable name backendUrl and behavior identical except for the env var key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/evaluations/datasets/`[dataset_id]/route.ts:
- Around line 51-57: The catch block that returns NextResponse.json({ error:
'Failed to forward request to backend', details: error }, { status: 500 }) will
serialize error as {}; change the returned details to a serializable
string/object (e.g. use error.message or a safe serializer like (error
instanceof Error ? error.message : JSON.stringify(error))) so NextResponse.json
receives serializable data; update the catch handling around NextResponse.json
in this route (the catch block referencing error and NextResponse.json) to
include the serialized error details and preserve existing status/message.
In `@app/api/evaluations/datasets/route.ts`:
- Around line 40-46: The catch blocks currently return NextResponse.json({
error: 'Failed to forward request to backend', details: error }, ...) which
serializes Error objects to {} — change those to include the error message (and
fallback) instead, e.g. use details: error?.message ?? String(error) (or
JSON.stringify({ name: error?.name, message: error?.message, stack: error?.stack
}) if you need more info); update both catch blocks that call NextResponse.json
so they reference the improved details expression rather than the raw error
object.
In `@app/components/Sidebar.tsx`:
- Line 2: Fix the header comment typo: change the word "Sidebad" to "Sidebar" in
the top-of-file comment in app/components/Sidebar.tsx so the component
description reads "Sidebar - Navigation sidebar with collapse/expand
functionality"; update only the comment text (no code logic) near the component
declaration/export to keep the header accurate.
---
Outside diff comments:
In `@app/api/evaluations/datasets/route.ts`:
- Line 32: The GET and POST handlers call await response.json() directly which
can throw on non-JSON responses; wrap the response.json() call in a try-catch
(same pattern used in the DELETE handler in [dataset_id]/route.ts) inside both
the GET and POST handlers, capture parse errors, log the error/context, and
return a clean 500 response (or set data to null/empty and handle accordingly)
instead of letting the exception propagate; locate the usages of response.json()
in the GET and POST handler functions and add the nested try-catch around those
calls.
---
Nitpick comments:
In `@app/api/evaluations/datasets/route.ts`:
- Line 21: Replace usage of the public env var by reading the private
BACKEND_URL instead: change the const backendUrl assignment that currently uses
process.env.NEXT_PUBLIC_BACKEND_URL to use process.env.BACKEND_URL with the same
fallback ('http://localhost:8000'), and update the same pattern at the other
occurrence (the backendUrl declaration around line 71) and in any other
server-only route handler files that reference NEXT_PUBLIC_BACKEND_URL so the
backend origin is not exposed to the client bundle; keep the variable name
backendUrl and behavior identical except for the env var key.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
app/api/evaluations/datasets/[dataset_id]/route.tsapp/api/evaluations/datasets/route.tsapp/api/evaluations/stt/datasets/[dataset_id]/route.tsapp/api/evaluations/stt/datasets/route.tsapp/api/evaluations/stt/files/route.tsapp/api/evaluations/stt/results/[result_id]/route.tsapp/api/evaluations/stt/runs/[run_id]/route.tsapp/api/evaluations/stt/runs/route.tsapp/api/speech-to-text/evaluate/route.tsapp/api/speech-to-text/parse-csv/route.tsapp/api/v1/audio/transcriptions/route.tsapp/api/v1/evaluations/stt/wer/route.tsapp/components/Sidebar.tsxapp/speech-to-text/page.tsx
💤 Files with no reviewable changes (4)
- app/api/v1/audio/transcriptions/route.ts
- app/api/speech-to-text/parse-csv/route.ts
- app/api/v1/evaluations/stt/wer/route.ts
- app/api/speech-to-text/evaluate/route.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- app/api/evaluations/stt/results/[result_id]/route.ts
- app/api/evaluations/stt/runs/[run_id]/route.ts
- app/api/evaluations/stt/runs/route.ts
- app/api/evaluations/stt/datasets/[dataset_id]/route.ts
- app/api/evaluations/stt/files/route.ts
- app/api/evaluations/stt/datasets/route.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/api/evaluations/stt/results/[result_id]/route.ts (1)
8-8:NEXT_PUBLIC_BACKEND_URLleaks the backend URL to the client bundle.Both the GET (line 8) and PATCH (line 33) handlers read
NEXT_PUBLIC_BACKEND_URL. Variables with theNEXT_PUBLIC_prefix are inlined into the client-side bundle by Next.js; since these are server-only route handlers, a non-public variable (e.g.BACKEND_URL) should be used instead.Also applies to: 33-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/results/`[result_id]/route.ts at line 8, The code defines backendUrl using process.env.NEXT_PUBLIC_BACKEND_URL which leaks into the client bundle; change the environment variable access in this file’s server handlers (the backendUrl constant used by the GET and PATCH handlers in route.ts) to a server-only variable such as process.env.BACKEND_URL (keeping the same localhost fallback if desired) so the backend URL is not exported to the client; update both occurrences (the backendUrl declaration and any other reads in the GET/PATCH handlers) to use process.env.BACKEND_URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/evaluations/stt/results/`[result_id]/route.ts:
- Around line 28-46: The PATCH handler reads the API key into the apiKey
variable but does not validate it before proxying; update the PATCH function to
check that apiKey is present and non-empty (from
request.headers.get('X-API-KEY')) and immediately return a 401 Unauthorized
response when missing, mirroring the write-operation pattern used in the
datasets route; ensure this early-return happens before calling fetch to the
backend so you never forward an empty 'X-API-KEY'.
---
Duplicate comments:
In `@app/api/evaluations/stt/results/`[result_id]/route.ts:
- Line 8: The code defines backendUrl using process.env.NEXT_PUBLIC_BACKEND_URL
which leaks into the client bundle; change the environment variable access in
this file’s server handlers (the backendUrl constant used by the GET and PATCH
handlers in route.ts) to a server-only variable such as process.env.BACKEND_URL
(keeping the same localhost fallback if desired) so the backend URL is not
exported to the client; update both occurrences (the backendUrl declaration and
any other reads in the GET/PATCH handlers) to use process.env.BACKEND_URL.
There was a problem hiding this comment.
♻️ Duplicate comments (8)
app/components/Sidebar.tsx (1)
2-2: Residual double space in header comment.The previous "Sidebad" typo is now corrected, but an extra space remains:
"Sidebar - Navigation...". Minor nit.✏️ Proposed fix
- * Sidebar - Navigation sidebar with collapse/expand functionality + * Sidebar - Navigation sidebar with collapse/expand functionality🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/Sidebar.tsx` at line 2, Remove the residual double space in the file header comment for the Sidebar component: change the comment string "Sidebar - Navigation sidebar with collapse/expand functionality" to use a single space ("Sidebar - Navigation...") so the header is clean; locate the top-of-file comment near the Sidebar component definition (symbol: Sidebar) and update that comment text accordingly.app/api/evaluations/stt/runs/route.ts (2)
6-7:⚠️ Potential issue | 🟠 Major
NEXT_PUBLIC_BACKEND_URL— same client-bundle exposure asstt/datasets/route.ts.Use
process.env.BACKEND_URL(server-only) instead. See fix proposed instt/datasets/route.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/runs/route.ts` around lines 6 - 7, The backend URL is using the client-exposed env var NEXT_PUBLIC_BACKEND_URL when it should use the server-only var; update the backendUrl initialization (the backendUrl constant in route.ts) to read from process.env.BACKEND_URL with a fallback like 'http://localhost:8000' instead of process.env.NEXT_PUBLIC_BACKEND_URL so the server-only secret is not exposed to the client.
18-23:⚠️ Potential issue | 🟠 Major
error: errorin GET catch still serializes to{}— same unresolved issue asstt/datasets/route.ts.This was marked as ✅ addressed in a prior commit, but the current code still passes the raw
Errorobject directly.JSON.stringify(new Error(...))returns"{}". The POST catch in the same file uses proper narrowing; the GET catch must be aligned.🐛 Proposed fix
} catch (error) { return NextResponse.json( - { success: false, error: error, data: null }, + { success: false, error: error instanceof Error ? error.message : String(error), data: null }, { status: 500 } ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/runs/route.ts` around lines 18 - 23, The GET handler's catch block currently returns the raw Error object (error: error) which serializes to {}; update the NextResponse.json call in the GET catch to mirror the POST catch error narrowing by replacing error: error with a serialized string/object (e.g., error: error instanceof Error ? error.message : String(error) or include stack via { message: error.message, stack: error.stack } when error is Error) so the response contains useful error text; locate the catch in route.ts around the GET handler and change the NextResponse.json payload accordingly.app/api/evaluations/stt/datasets/route.ts (2)
19-24:⚠️ Potential issue | 🟠 Major
error: errorin GET catch block serializes to{}— error detail is lost.
JSON.stringify(new Error('msg'))produces"{}". Theerrorfield in the 500 response will always be an empty object, giving callers no actionable information. The POST catch block in this same file correctly usesinstanceof Errornarrowing; the GET catch block should match.🐛 Proposed fix
} catch (error) { return NextResponse.json( - { success: false, error: error, data: null }, + { success: false, error: error instanceof Error ? error.message : String(error), data: null }, { status: 500 } ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/datasets/route.ts` around lines 19 - 24, The GET handler's catch currently returns the raw error object which JSON serializes to {}—update the catch in app/api/evaluations/stt/datasets/route.ts (the GET error handler) to narrow the error like the POST handler: check if error instanceof Error and return error: error.message (or include both message and stack if desired), otherwise use String(error) so callers receive meaningful error text instead of an empty object.
7-8:⚠️ Potential issue | 🟠 Major
NEXT_PUBLIC_BACKEND_URLexposes the backend URL in the client-side JavaScript bundle.
NEXT_PUBLIC_variables are designed to expose values to the browser, and they are inlined into the JavaScript bundle duringnext build. Since this is a server-only route handler, the backend URL should be read from a non-prefixed variable (e.g.,BACKEND_URL) that remains server-side only. This same issue is present instt/runs/route.tsandstt/results/[result_id]/route.ts.🔒 Proposed fix (applies to all six proxy routes)
- const backendUrl = process.env.NEXT_PUBLIC_BACKEND_URL || 'http://localhost:8000'; + const backendUrl = process.env.BACKEND_URL || 'http://localhost:8000';Update
.env*files accordingly:- NEXT_PUBLIC_BACKEND_URL=http://your-backend-host + BACKEND_URL=http://your-backend-host🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/datasets/route.ts` around lines 7 - 8, The server-only route handlers currently set backendUrl using process.env.NEXT_PUBLIC_BACKEND_URL which leaks into the client bundle; change the constant assignment (backendUrl = process.env.NEXT_PUBLIC_BACKEND_URL || 'http://localhost:8000') in the route handler files that define backendUrl to use a non-public env var (process.env.BACKEND_URL || 'http://localhost:8000') and update any other route modules that use the same pattern (e.g., the STT runs and results route handlers that declare backendUrl) to the same non-prefixed variable; also update your environment files to define BACKEND_URL.app/api/evaluations/route.ts (1)
50-56: Error serialization fix correctly applied in both GET and POST handlers.Also applies to: 84-90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/route.ts` around lines 50 - 56, Update both error-handling catch blocks in app/api/evaluations/route.ts (the catch in the GET handler and the catch in the POST handler) to serialize the caught error before returning JSON: replace any direct use of the error object with details: error instanceof Error ? error.message : String(error) and include a descriptive message in processLogger/console.error as appropriate (e.g., console.error('Proxy error:', error)). Ensure both handlers return NextResponse.json({ error: 'Failed to forward request to backend', details: error instanceof Error ? error.message : String(error) }, { status: 500 }).app/api/evaluations/stt/results/[result_id]/route.ts (1)
8-8:⚠️ Potential issue | 🟠 Major
NEXT_PUBLIC_BACKEND_URL— same client-bundle exposure in both GET and PATCH.Unresolved from prior review. Replace with
process.env.BACKEND_URLfor both occurrences in this file. See fix proposed instt/datasets/route.ts.Also applies to: 34-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/results/`[result_id]/route.ts at line 8, Replace the client-exposed env var with the server-only one: update the backendUrl constant(s) in this file to use process.env.BACKEND_URL instead of process.env.NEXT_PUBLIC_BACKEND_URL (keep the existing '|| "http://localhost:8000"' fallback). Change the occurrences referenced by the backendUrl symbol used by the route handlers (GET and PATCH) so both use process.env.BACKEND_URL.app/api/evaluations/datasets/[dataset_id]/route.ts (1)
51-57: Error serialization fix correctly applied.
details: error instanceof Error ? error.message : String(error)resolves the previous{}serialization issue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/datasets/`[dataset_id]/route.ts around lines 51 - 57, The catch block now properly serializes the thrown error by using the pattern "error instanceof Error ? error.message : String(error)" before returning via NextResponse.json; ensure the catch handling in route.ts (the catch (error: any) block that calls console.error and NextResponse.json) uses that exact serialization pattern for the details field (and continue to log the raw error with console.error or processLogger) so the response no longer returns an empty object for errors while preserving useful error text.
🧹 Nitpick comments (2)
app/api/evaluations/stt/results/[result_id]/route.ts (1)
50-50: Redundant|| ''fallback is dead code after the null-guard.
apiKeyis guaranteed non-null after the early-return at Line 37, soapiKey || ''can never fall back to the empty string.♻️ Proposed fix
- 'X-API-KEY': apiKey || '', + 'X-API-KEY': apiKey,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/results/`[result_id]/route.ts at line 50, The header assembly uses 'X-API-KEY': apiKey || '' which is dead code because apiKey is already guaranteed non-null by the earlier guard; remove the redundant fallback and set the header directly to apiKey (e.g., 'X-API-KEY': apiKey) so the intent is clear and no unreachable fallback remains.app/api/evaluations/stt/datasets/route.ts (1)
5-6: Malformed function signature split across two lines.The
GETfunction signature is split mid-parameter-list (request:\n Request), likely a formatting artifact. It compiles, but hurts readability and deviates from the style used across every other route in this PR.♻️ Proposed fix
-export async function GET(request: - Request) { +export async function GET(request: Request) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/stt/datasets/route.ts` around lines 5 - 6, The GET handler's function signature is split across two lines ("export async function GET(request: \n Request)"), harming readability and style consistency; fix by collapsing the signature into a single line (export async function GET(request: Request) { ... }) in the GET function inside route.ts so the parameter and its type appear on one line, matching the other route handlers and preserving existing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/api/evaluations/datasets/`[dataset_id]/route.ts:
- Around line 51-57: The catch block now properly serializes the thrown error by
using the pattern "error instanceof Error ? error.message : String(error)"
before returning via NextResponse.json; ensure the catch handling in route.ts
(the catch (error: any) block that calls console.error and NextResponse.json)
uses that exact serialization pattern for the details field (and continue to log
the raw error with console.error or processLogger) so the response no longer
returns an empty object for errors while preserving useful error text.
In `@app/api/evaluations/route.ts`:
- Around line 50-56: Update both error-handling catch blocks in
app/api/evaluations/route.ts (the catch in the GET handler and the catch in the
POST handler) to serialize the caught error before returning JSON: replace any
direct use of the error object with details: error instanceof Error ?
error.message : String(error) and include a descriptive message in
processLogger/console.error as appropriate (e.g., console.error('Proxy error:',
error)). Ensure both handlers return NextResponse.json({ error: 'Failed to
forward request to backend', details: error instanceof Error ? error.message :
String(error) }, { status: 500 }).
In `@app/api/evaluations/stt/datasets/route.ts`:
- Around line 19-24: The GET handler's catch currently returns the raw error
object which JSON serializes to {}—update the catch in
app/api/evaluations/stt/datasets/route.ts (the GET error handler) to narrow the
error like the POST handler: check if error instanceof Error and return error:
error.message (or include both message and stack if desired), otherwise use
String(error) so callers receive meaningful error text instead of an empty
object.
- Around line 7-8: The server-only route handlers currently set backendUrl using
process.env.NEXT_PUBLIC_BACKEND_URL which leaks into the client bundle; change
the constant assignment (backendUrl = process.env.NEXT_PUBLIC_BACKEND_URL ||
'http://localhost:8000') in the route handler files that define backendUrl to
use a non-public env var (process.env.BACKEND_URL || 'http://localhost:8000')
and update any other route modules that use the same pattern (e.g., the STT runs
and results route handlers that declare backendUrl) to the same non-prefixed
variable; also update your environment files to define BACKEND_URL.
In `@app/api/evaluations/stt/results/`[result_id]/route.ts:
- Line 8: Replace the client-exposed env var with the server-only one: update
the backendUrl constant(s) in this file to use process.env.BACKEND_URL instead
of process.env.NEXT_PUBLIC_BACKEND_URL (keep the existing '||
"http://localhost:8000"' fallback). Change the occurrences referenced by the
backendUrl symbol used by the route handlers (GET and PATCH) so both use
process.env.BACKEND_URL.
In `@app/api/evaluations/stt/runs/route.ts`:
- Around line 6-7: The backend URL is using the client-exposed env var
NEXT_PUBLIC_BACKEND_URL when it should use the server-only var; update the
backendUrl initialization (the backendUrl constant in route.ts) to read from
process.env.BACKEND_URL with a fallback like 'http://localhost:8000' instead of
process.env.NEXT_PUBLIC_BACKEND_URL so the server-only secret is not exposed to
the client.
- Around line 18-23: The GET handler's catch block currently returns the raw
Error object (error: error) which serializes to {}; update the NextResponse.json
call in the GET catch to mirror the POST catch error narrowing by replacing
error: error with a serialized string/object (e.g., error: error instanceof
Error ? error.message : String(error) or include stack via { message:
error.message, stack: error.stack } when error is Error) so the response
contains useful error text; locate the catch in route.ts around the GET handler
and change the NextResponse.json payload accordingly.
In `@app/components/Sidebar.tsx`:
- Line 2: Remove the residual double space in the file header comment for the
Sidebar component: change the comment string "Sidebar - Navigation sidebar with
collapse/expand functionality" to use a single space ("Sidebar - Navigation...")
so the header is clean; locate the top-of-file comment near the Sidebar
component definition (symbol: Sidebar) and update that comment text accordingly.
---
Nitpick comments:
In `@app/api/evaluations/stt/datasets/route.ts`:
- Around line 5-6: The GET handler's function signature is split across two
lines ("export async function GET(request: \n Request)"), harming readability
and style consistency; fix by collapsing the signature into a single line
(export async function GET(request: Request) { ... }) in the GET function inside
route.ts so the parameter and its type appear on one line, matching the other
route handlers and preserving existing behavior.
In `@app/api/evaluations/stt/results/`[result_id]/route.ts:
- Line 50: The header assembly uses 'X-API-KEY': apiKey || '' which is dead code
because apiKey is already guaranteed non-null by the earlier guard; remove the
redundant fallback and set the header directly to apiKey (e.g., 'X-API-KEY':
apiKey) so the intent is clear and no unreachable fallback remains.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/api/evaluations/datasets/[dataset_id]/route.tsapp/api/evaluations/datasets/route.tsapp/api/evaluations/route.tsapp/api/evaluations/stt/datasets/route.tsapp/api/evaluations/stt/files/route.tsapp/api/evaluations/stt/results/[result_id]/route.tsapp/api/evaluations/stt/runs/route.tsapp/components/Sidebar.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- app/api/evaluations/stt/files/route.ts
- app/api/evaluations/datasets/route.ts
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.
Targer issue is #42
Summary by CodeRabbit
New Features
Security
Removed
UI
Bug Fixes