Evals UI: Filter text eval jobs for eval jobs page#52
Closed
Evals UI: Filter text eval jobs for eval jobs page#52
Conversation
📝 WalkthroughWalkthroughAPI routes now filter backend payloads to expose only relevant dataset/evaluation item types (text or stt) and surface backend errors; the evaluations UI renames tabs and components (Upload → Datasets, Results → Evaluations) and updates navigation, labels, and redirects accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Prajna1999
approved these changes
Mar 5, 2026
AkhileshNegi
requested changes
Mar 5, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
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/stt/datasets/route.ts (1)
12-19:⚠️ Potential issue | 🟠 MajorFail fast on missing
X-API-KEYin GET.GET currently forwards an empty API key instead of returning 401, unlike POST. This creates inconsistent auth behavior and avoidable backend traffic.
Proposed fix
const apiKey = request.headers.get('X-API-KEY'); + if (!apiKey) { + return NextResponse.json( + { error: 'Missing X-API-KEY header' }, + { status: 401 } + ); + } try { const response = await fetch(`${backendUrl}/api/v1/evaluations/stt/datasets`, { headers: { - '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/datasets/route.ts` around lines 12 - 19, The GET handler currently reads const apiKey = request.headers.get('X-API-KEY') then forwards an empty string to fetch; instead, fail fast when apiKey is missing by returning a 401 response immediately. Update the logic around the apiKey constant (before the fetch to `${backendUrl}/api/v1/evaluations/stt/datasets`) to check if apiKey is falsy and return an appropriate 401 response (matching the POST behavior) rather than calling fetch with an empty 'X-API-KEY' header.
🤖 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/route.ts`:
- Around line 43-45: The code assumes data.data is an array before calling
.filter; update the conditional in the datasets response assignment to guard
with Array.isArray(data.data) so you only call data.data.filter when
Array.isArray(data.data) is true (otherwise leave data unchanged); locate the
expression that builds the final result (the ternary using data.data ? {
...data, data: data.data.filter(...) } : data) and add the Array.isArray check
to prevent runtime errors.
- Around line 40-47: The endpoint in route.ts is hard-filtering datasets to type
=== 'text' (see the filteredData construction using data.filter and
data.data.filter), which hides non-text datasets for other consumers; revert
this global filter and instead implement conditional filtering based on a query
parameter (e.g. check req.nextUrl.searchParams.get('type') === 'text' before
applying the filter) or create a separate text-only endpoint used by
app/evaluations/page.tsx; update the logic that builds filteredData to only
filter when the type query is set (leave the default behavior to return all
dataset types) and ensure the variable names filteredData and data are reused
accordingly.
In `@app/api/evaluations/route.ts`:
- Around line 27-32: The handler currently filters evaluations into the local
variable items but still returns the original data, so clients receive
unfiltered mixed types; update the response in the route handler (where data is
returned) to use the filtered items variable (or construct a response object
that replaces data with items) so the API returns text-only evaluations; ensure
you reference the items variable and the same data structure shape used
elsewhere in this file (e.g., where items is populated) so pagination/metadata
(if any) is preserved or reattached when returning the filtered payload.
In `@app/api/evaluations/stt/datasets/route.ts`:
- Around line 27-32: The STT filtering logic in the route uses filteredData and
currently only handles top-level arrays and data.data, which misses the
data.datasets shape and calls .filter without array guards; update the
filteredData computation to check for Array.isArray on data, data.data, and
data.datasets and apply dataset => dataset.type === 'stt' only when each is an
array (preserve non-array branches unchanged), referencing the filteredData
variable and the dataset.type predicate so all response shapes (data, data.data,
data.datasets) are safely filtered for 'stt'.
---
Outside diff comments:
In `@app/api/evaluations/stt/datasets/route.ts`:
- Around line 12-19: The GET handler currently reads const apiKey =
request.headers.get('X-API-KEY') then forwards an empty string to fetch;
instead, fail fast when apiKey is missing by returning a 401 response
immediately. Update the logic around the apiKey constant (before the fetch to
`${backendUrl}/api/v1/evaluations/stt/datasets`) to check if apiKey is falsy and
return an appropriate 401 response (matching the POST behavior) rather than
calling fetch with an empty 'X-API-KEY' header.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9cbe595-7fb4-40d1-b4aa-afcca5517f6f
📒 Files selected for processing (4)
app/api/evaluations/datasets/route.tsapp/api/evaluations/route.tsapp/api/evaluations/stt/datasets/route.tsapp/evaluations/page.tsx
Comment on lines
+40
to
+47
| // Filter to only return text datasets | ||
| const filteredData = Array.isArray(data) | ||
| ? data.filter((dataset: any) => dataset.type === 'text') | ||
| : data.data | ||
| ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') } | ||
| : data; | ||
|
|
||
| return NextResponse.json(filteredData, { status: 200 }); |
There was a problem hiding this comment.
Avoid hard-filtering this shared datasets endpoint.
This route is consumed by both app/evaluations/page.tsx (text eval flow) and app/datasets/page.tsx (datasets management). Applying type === 'text' here makes non-text datasets silently disappear in other pages. Prefer a dedicated text-only endpoint (or a ?type=text query) instead of changing the default behavior globally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/evaluations/datasets/route.ts` around lines 40 - 47, The endpoint in
route.ts is hard-filtering datasets to type === 'text' (see the filteredData
construction using data.filter and data.data.filter), which hides non-text
datasets for other consumers; revert this global filter and instead implement
conditional filtering based on a query parameter (e.g. check
req.nextUrl.searchParams.get('type') === 'text' before applying the filter) or
create a separate text-only endpoint used by app/evaluations/page.tsx; update
the logic that builds filteredData to only filter when the type query is set
(leave the default behavior to return all dataset types) and ensure the variable
names filteredData and data are reused accordingly.
Comment on lines
+43
to
+45
| : data.data | ||
| ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') } | ||
| : data; |
There was a problem hiding this comment.
Guard nested payload shape before calling .filter().
If data.data is present but not an array, Line 44 throws at runtime. Add an Array.isArray(data.data) check before filtering.
Proposed fix
-const filteredData = Array.isArray(data)
- ? data.filter((dataset: any) => dataset.type === 'text')
- : data.data
- ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') }
- : data;
+const filteredData = Array.isArray(data)
+ ? data.filter((dataset: any) => dataset.type === 'text')
+ : Array.isArray(data?.data)
+ ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') }
+ : data;📝 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
| : data.data | |
| ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') } | |
| : data; | |
| const filteredData = Array.isArray(data) | |
| ? data.filter((dataset: any) => dataset.type === 'text') | |
| : Array.isArray(data?.data) | |
| ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') } | |
| : data; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/evaluations/datasets/route.ts` around lines 43 - 45, The code assumes
data.data is an array before calling .filter; update the conditional in the
datasets response assignment to guard with Array.isArray(data.data) so you only
call data.data.filter when Array.isArray(data.data) is true (otherwise leave
data unchanged); locate the expression that builds the final result (the ternary
using data.data ? { ...data, data: data.data.filter(...) } : data) and add the
Array.isArray check to prevent runtime errors.
Comment on lines
+27
to
+32
| // Filter to only return STT datasets (additional safety check) | ||
| const filteredData = Array.isArray(data) | ||
| ? data.filter((dataset: any) => dataset.type === 'stt') | ||
| : data.data | ||
| ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'stt') } | ||
| : data; |
There was a problem hiding this comment.
STT filtering misses the data.datasets shape and lacks array guards.
This code only filters top-level arrays and data.data. But the STT page also handles data.datasets; in that response shape, filtering is skipped entirely. Also, calling .filter without Array.isArray checks risks runtime errors.
Proposed fix
- const filteredData = Array.isArray(data)
- ? data.filter((dataset: any) => dataset.type === 'stt')
- : data.data
- ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'stt') }
- : data;
+ const filteredData = Array.isArray(data)
+ ? data.filter((dataset: any) => dataset.type === 'stt')
+ : Array.isArray(data?.datasets)
+ ? { ...data, datasets: data.datasets.filter((dataset: any) => dataset.type === 'stt') }
+ : Array.isArray(data?.data)
+ ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'stt') }
+ : data;📝 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
| // Filter to only return STT datasets (additional safety check) | |
| const filteredData = Array.isArray(data) | |
| ? data.filter((dataset: any) => dataset.type === 'stt') | |
| : data.data | |
| ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'stt') } | |
| : data; | |
| // Filter to only return STT datasets (additional safety check) | |
| const filteredData = Array.isArray(data) | |
| ? data.filter((dataset: any) => dataset.type === 'stt') | |
| : Array.isArray(data?.datasets) | |
| ? { ...data, datasets: data.datasets.filter((dataset: any) => dataset.type === 'stt') } | |
| : Array.isArray(data?.data) | |
| ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'stt') } | |
| : data; |
🤖 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 27 - 32, The STT
filtering logic in the route uses filteredData and currently only handles
top-level arrays and data.data, which misses the data.datasets shape and calls
.filter without array guards; update the filteredData computation to check for
Array.isArray on data, data.data, and data.datasets and apply dataset =>
dataset.type === 'stt' only when each is an array (preserve non-array branches
unchanged), referencing the filteredData variable and the dataset.type predicate
so all response shapes (data, data.data, data.datasets) are safely filtered for
'stt'.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
app/api/evaluations/datasets/route.ts (2)
43-45:⚠️ Potential issue | 🟠 MajorGuard nested payload shape before calling
.filter().Line 43 only checks truthiness of
data.data. If backend returns a non-array (e.g. object), Line 44 throws at runtime. UseArray.isArray(data?.data)in that branch.Proposed fix
-const filteredData = Array.isArray(data) - ? data.filter((dataset: any) => dataset.type === 'text') - : data.data - ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') } - : data; +const filteredData = Array.isArray(data) + ? data.filter((dataset: any) => dataset.type === 'text') + : Array.isArray(data?.data) + ? { ...data, data: data.data.filter((dataset: any) => dataset.type === 'text') } + : data;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/datasets/route.ts` around lines 43 - 45, The current conditional uses truthiness of data.data and then calls .filter, which will throw if data.data is not an array; update the branch that maps datasets (in app/api/evaluations/datasets/route.ts where you compute the filtered result from variable data) to first guard with Array.isArray(data?.data) and only call .filter when that check is true, otherwise fall back to the original value so non-array payloads are handled safely.
40-45:⚠️ Potential issue | 🟠 MajorAvoid unconditional text filtering on this route.
Line 40 through Line 45 hard-filter to
type === 'text'for every request. If this endpoint is still used as a general datasets source anywhere, non-text datasets will disappear silently. Prefer conditional filtering (e.g. query-param gated) or keep this route generic and move text-only behavior to a dedicated endpoint.#!/bin/bash # Verify whether this route is consumed as a generic datasets endpoint. # Expected: if consumers require mixed dataset types, unconditional filtering is unsafe. rg -n -C3 "/api/evaluations/datasets" --type=ts --type=tsx rg -n -C3 "evaluations/datasets" --type=ts --type=tsx🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/evaluations/datasets/route.ts` around lines 40 - 45, The current route unconditionally filters datasets to text-only by assigning to filteredData using dataset.type === 'text', which can silently remove non-text datasets; update the route handler to apply this filter conditionally (e.g., check a query parameter like "type=text" or "textOnly=true" from the incoming request) and only perform the data.filter(...) when that param is present, otherwise return the original data as-is; alternatively, if you prefer separation of concerns, move the text-only logic into a dedicated endpoint and keep this route generic (refer to the filteredData assignment and the route handler that reads "data" to implement the conditional gating).
🤖 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/route.ts`:
- Around line 43-45: The current conditional uses truthiness of data.data and
then calls .filter, which will throw if data.data is not an array; update the
branch that maps datasets (in app/api/evaluations/datasets/route.ts where you
compute the filtered result from variable data) to first guard with
Array.isArray(data?.data) and only call .filter when that check is true,
otherwise fall back to the original value so non-array payloads are handled
safely.
- Around line 40-45: The current route unconditionally filters datasets to
text-only by assigning to filteredData using dataset.type === 'text', which can
silently remove non-text datasets; update the route handler to apply this filter
conditionally (e.g., check a query parameter like "type=text" or "textOnly=true"
from the incoming request) and only perform the data.filter(...) when that param
is present, otherwise return the original data as-is; alternatively, if you
prefer separation of concerns, move the text-only logic into a dedicated
endpoint and keep this route generic (refer to the filteredData assignment and
the route handler that reads "data" to implement the conditional gating).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fca1ac70-c18f-4abf-a4f8-a864f72b26d3
📒 Files selected for processing (2)
app/api/evaluations/datasets/route.tsapp/api/evaluations/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/api/evaluations/route.ts
2 tasks
Contributor
Author
|
we fixed this issue from backend instead of frontend, where evaluation apis being used for text evaluation will only return text evaluation response, and same for stt evaluation |
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
3 participants
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.
Target issue is #53
Summary by CodeRabbit
Bug Fixes
Refactor
Summary by CodeRabbit
New Features
Bug Fixes