Evaluation: UI/UX Revamp#142
Merged
Merged
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughRefactors UI to use shared Modal/Button primitives, extracts large evaluation and speech-to-text UIs into focused components, adds skeleton/icon assets, introduces CSV export utilities, and updates types and page loading behaviors. ChangesDesign system & shared primitives
Evaluations UI & CSV export
Speech-to-Text & Text-to-Speech UI decomposition
Sequence Diagram(s)sequenceDiagram
participant User
participant EvalPage
participant ExportHelpers
participant Browser
User->>EvalPage: Click "Export CSV"
EvalPage->>ExportHelpers: exportGroupedCSV(job, traces) or exportRowCSV(job, scoreObject, assistantConfig)
ExportHelpers->>ExportHelpers: Build CSV lines, escape fields, construct Blob
ExportHelpers->>Browser: createObjectURL + <a> click (download)
Browser-->>User: Prompt/download file (evaluation_*.csv)
ExportHelpers->>EvalPage: return row count
EvalPage-->>User: Show success toast with count
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Ayush8923
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/ConfigModal.tsx (1)
67-138:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIgnore stale fetch results after close or job switches.
Now that this modal stays mounted, an in-flight fetch can resolve after
isOpenflips false orjobchanges and overwrite state with stale config data. That will surface the wrong config details the next time the modal opens.Suggested fix
useEffect(() => { + let cancelled = false; + if (!isOpen || !job.config_id || !job.config_version) { setConfigVersionInfo(null); return; } const fetchConfigVersionInfo = async () => { - if (!isAuthenticated) return; + if (!isAuthenticated) { + if (!cancelled) setConfigVersionInfo(null); + return; + } const apiKey = activeKey?.key ?? ""; setIsLoadingConfig(true); try { const configData = await apiFetch<{ @@ - if (versionData.success && versionData.data) { + if (!cancelled && versionData.success && versionData.data) { const blob = versionData.data.config_blob; const params: CompletionParams = blob?.completion?.params || ({} as CompletionParams); @@ - setConfigVersionInfo({ - name: configName || "Unknown Config", - version: job.config_version!, - model: params.model, - instructions: params.instructions, - temperature: params.temperature, - tools: params.tools, - provider: blob?.completion?.provider, - type: blob?.completion?.type || "text", - knowledge_base_ids: - uniqueKbIds.length > 0 ? uniqueKbIds : undefined, - }); + setConfigVersionInfo({ + name: configName || "Unknown Config", + version: job.config_version!, + model: params.model, + instructions: params.instructions, + temperature: params.temperature, + tools: params.tools, + provider: blob?.completion?.provider, + type: blob?.completion?.type || "text", + knowledge_base_ids: + uniqueKbIds.length > 0 ? uniqueKbIds : undefined, + }); } } catch (error) { console.error("Error fetching config version info:", error); } finally { - setIsLoadingConfig(false); + if (!cancelled) setIsLoadingConfig(false); } }; fetchConfigVersionInfo(); + return () => { + cancelled = true; + }; }, [isOpen, job.config_id, job.config_version, activeKey, isAuthenticated]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/ConfigModal.tsx` around lines 67 - 138, The effect can let an in-flight fetch overwrite state after isOpen or job changes; in the useEffect/fetchConfigVersionInfo block, cancel or ignore stale responses by creating an AbortController (or a local requestId token) at the start of the effect, pass the signal to apiFetch calls, and in the cleanup set a flag/abort to mark the request stale; before calling setConfigVersionInfo or setIsLoadingConfig in the success/error/finally paths verify the request is still current (not aborted and job.config_id/version and isOpen still match) so results from a previous job or a closed modal are ignored.
🧹 Nitpick comments (6)
app/components/ResultsTableSkeleton.tsx (1)
11-19: ⚡ Quick winStaggered pulse delay is not taking effect.
animationDelayon Line 18 won’t apply unless the animated class is on each cell too. Right nowanimate-pulseis only on the wrapper (Line 11), so all cells pulse together.Proposed fix
- <div className="p-4 space-y-3 animate-pulse"> + <div className="p-4 space-y-3"> {Array.from({ length: rows }).map((_, r) => ( <div key={r} className="flex items-center gap-3"> {Array.from({ length: cols }).map((_, c) => ( <div key={c} - className="flex-1 h-12 bg-neutral-100 rounded" + className="flex-1 h-12 bg-neutral-100 rounded animate-pulse" style={{ animationDelay: `${(r + c) * 50}ms` }} /> ))} </div> ))} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/ResultsTableSkeleton.tsx` around lines 11 - 19, The staggered animationDelay on the cell divs isn't effective because the animated class is only on the outer wrapper; update ResultsTableSkeleton so the animation class is applied to each cell (the div with className "flex-1 h-12 bg-neutral-100 rounded" that also has style={{ animationDelay: ... }}), e.g. add "animate-pulse" to that inner div (and optionally remove it from the outer wrapper) so each cell respects its individual animationDelay.app/components/speech-to-text/AudioFileItem.tsx (1)
53-59: ⚡ Quick winSet explicit button type for the remove action.
This button should be
type="button"to avoid accidental form submission when rendered inside form trees.Proposed fix
<button + type="button" onClick={onRemove} className="p-1 rounded shrink-0 text-text-secondary" aria-label="Remove audio sample" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/AudioFileItem.tsx` around lines 53 - 59, In AudioFileItem.tsx update the remove button element (the one with onRemove and CloseIcon) to include an explicit type attribute: set type="button" so clicking the Remove action doesn't trigger form submission when this component is rendered inside a form; locate the button with onRemove in the AudioFileItem component and add the type attribute.app/components/evaluations/EvaluationsTab.tsx (1)
8-9: ⚡ Quick winUse the root alias for the new child component imports.
These moved components should follow the same import convention as the rest of the file to avoid mixing relative and project-root resolution.
Suggested fix
-import EvalRunsList from "./EvalRunsList"; -import RunEvaluationForm from "./RunEvaluationForm"; +import EvalRunsList from "@/app/components/evaluations/EvalRunsList"; +import RunEvaluationForm from "@/app/components/evaluations/RunEvaluationForm";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use the@/import alias configured in tsconfig.json to resolve imports from the project root.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/EvaluationsTab.tsx` around lines 8 - 9, Update the two relative imports so they use the project root alias instead of relative paths: replace the current imports of EvalRunsList and RunEvaluationForm with imports using the "@/..." alias (matching the project's tsconfig paths) so the symbols EvalRunsList and RunEvaluationForm are resolved from the root-based module path consistent with the rest of the file.app/components/evaluations/RunEvaluationForm.tsx (1)
8-10: ⚡ Quick winSwitch this block to the shared root imports.
The new relative import violates the repo convention, and this is also a good place to consume the shared
Tabtype instead of re-declaring the union locally.Suggested fix
-import EvalDatasetDescription from "./EvalDatasetDescription"; - -type Tab = "datasets" | "evaluations"; +import EvalDatasetDescription from "@/app/components/evaluations/EvalDatasetDescription"; +import type { Tab } from "@/app/lib/types/nav";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use the@/import alias configured in tsconfig.json to resolve imports from the project root.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/RunEvaluationForm.tsx` around lines 8 - 10, Replace the local relative import and the locally redeclared Tab union by using the repository root alias and the shared Tab type: change the import of EvalDatasetDescription to use the "@/components/..." root import and remove the local type declaration, importing the shared Tab type (named Tab) from its module instead of redeclaring it; update any references to Tab in RunEvaluationForm to use the imported symbol.app/components/evaluations/DatasetsTab.tsx (1)
10-13: ⚡ Quick winUse the project alias for these component imports.
These new relative imports drift from the repo convention and make the moved components harder to relocate safely later.
Suggested fix
-import DatasetCard from "./DatasetCard"; -import CreateDatasetForm from "./CreateDatasetForm"; -import ViewDatasetModal from "./ViewDatasetModal"; -import DeleteDatasetModal from "./DeleteDatasetModal"; +import DatasetCard from "@/app/components/evaluations/DatasetCard"; +import CreateDatasetForm from "@/app/components/evaluations/CreateDatasetForm"; +import ViewDatasetModal from "@/app/components/evaluations/ViewDatasetModal"; +import DeleteDatasetModal from "@/app/components/evaluations/DeleteDatasetModal";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use the@/import alias configured in tsconfig.json to resolve imports from the project root.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/DatasetsTab.tsx` around lines 10 - 13, The file DatasetsTab.tsx uses relative imports for DatasetCard, CreateDatasetForm, ViewDatasetModal, and DeleteDatasetModal which violate the repo convention; update these import statements to use the project alias (@"...") as configured in tsconfig.json (e.g. replace "./DatasetCard" etc. with the corresponding "@/components/..." or "@/components/evaluations/..." paths) so the components resolve from the repo root and follow the alias-based import pattern.app/components/speech-to-text/CreateSTTDatasetForm.tsx (1)
9-9: ⚡ Quick winUse the project alias for the new local import.
Please keep this component aligned with the repo import convention instead of introducing a relative path here.
Suggested fix
-import AudioFileItem from "./AudioFileItem"; +import AudioFileItem from "@/app/components/speech-to-text/AudioFileItem";As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Use the@/import alias configured in tsconfig.json to resolve imports from the project root.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx` at line 9, The import in CreateSTTDatasetForm.tsx currently uses a relative path for AudioFileItem; update the import to use the project alias (replace the "./AudioFileItem" import with the "@/components/speech-to-text/AudioFileItem" alias) so it follows the repo convention and tsconfig.json path mapping, and ensure the AudioFileItem symbol is still exported/imported correctly.
🤖 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/components/evaluations/CreateDatasetForm.tsx`:
- Around line 196-220: The upload surface in CreateDatasetForm is a
non-interactive div which blocks keyboard users; replace that div with a real
interactive element (preferably a <button> or a <label> tied to fileInputRef) so
keyboard activation opens the file chooser, keep existing props and handlers
(onClick -> forward to fileInputRef.current?.click or use htmlFor on label),
retain drag handlers (onDragOver/onDragLeave/onDrop -> handleDrop) and
isDragging visual state, and add keyboard handlers (onKeyDown handling
Enter/Space if you use a div-like element) plus appropriate accessible
attributes (tabIndex, aria-label/role if needed) to ensure CreateDatasetForm
remains fully keyboard-accessible while preserving fileInputRef and handleDrop
behavior.
In `@app/components/evaluations/DatasetsTab.tsx`:
- Around line 219-226: The DeleteDatasetModal is being closed immediately
because setConfirmDeleteId(null) is called right after starting
handleDeleteDataset, so the modal unmounts before its isDeleting state can
render and it disappears on failure; change the onConfirm flow to await the
deletion and only clear confirmDeleteId on success (or keep it if the deletion
fails). Concretely, make handleDeleteDataset return a Promise/boolean if it
doesn't already, then replace the inline onConfirm to call the async handler
(e.g., await handleDeleteDataset(confirmDeleteId) or
handleDeleteDataset(confirmDeleteId).then(...)) and only call
setConfirmDeleteId(null) when the deletion resolves successfully; keep using
deletingId === confirmDeleteId for the isDeleting prop so the modal remains open
and shows loading until the request settles.
- Around line 125-127: The current code splits csvText by "\n" then calls
parseCsvRow which breaks valid CSV cells containing embedded newlines; instead
parse the entire csvText with a proper CSV parser and derive headers and rows
from that parsed result. Replace the split + parseCsvRow flow by calling a CSV
parsing function on csvText (e.g., a library parseCsv/textToRows) to produce a
2D array, then set headers = parsed[0] (or [] if empty) and rows =
parsed.slice(1); update usages of lines, headers, rows, and parseCsvRow
accordingly to ensure quoted multiline cells are preserved.
In `@app/components/evaluations/DeleteDatasetModal.tsx`:
- Around line 37-39: The confirmation copy in DeleteDatasetModal.tsx renders the
optional datasetName directly which yields awkward text when undefined; update
the JSX that renders datasetName (the datasetName variable used inside the
<strong> tag) to use a fallback label (e.g., "this dataset" or "Unnamed
dataset") when datasetName is falsy so the sentence reads naturally even if no
name is provided.
In `@app/components/evaluations/ViewDatasetModal.tsx`:
- Around line 16-27: CSV export in ViewDatasetModal.tsx writes headers and cells
directly, which can allow spreadsheet formula injection. Update the csvLines
building logic to: sanitize both data.headers and each cell in data.rows by
converting null/undefined to empty strings, escaping inner double-quotes by
replacing " with "" and wrapping fields with commas/quotes/newlines in
double-quotes, and if a header or cell begins with =, +, -, or @ prefix it with
a single quote character to neutralize formulas. Ensure the same sanitation
function is used for headers (data.headers) and cells (data.rows) so all
untrusted values are consistently escaped before joining into csvLines.
In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx`:
- Around line 173-191: The empty-upload affordance is a non-interactive <div>
with onClick (when audioFiles.length === 0) so keyboard users cannot open the
picker; change it to an actual interactive element (replace the <div> or make it
act like a button) by using a <button> or adding tabIndex, role="button",
onKeyDown handler that calls triggerAudioUpload for Enter/Space, and proper
disabled handling when isAuthenticated is false (or aria-disabled="true") so
focus/keyboard and screen-reader users can activate it; update the element
around the MusicNoteIcon and text and ensure the accessible label (aria-label)
reflects "Upload audio samples" and that visual disabled styles remain when not
authenticated.
- Around line 241-245: The cancel/reset onClick handler hard-codes
setDatasetLanguageId(1), which can be invalid; change it to reset using the
available languages or an explicit default prop instead. In the
CreateSTTDatasetForm component update the onClick logic that currently calls
setDatasetLanguageId(1) to use a safe default derived from the languages array
(e.g., languages[0].id or find the language with isDefault) or a provided
defaultLanguageId prop; ensure you handle the case where languages is empty by
falling back to null/undefined and updating form validation accordingly.
In `@app/components/speech-to-text/DatasetsTab.tsx`:
- Around line 17-19: The imports in DatasetsTab.tsx for STTDatasetCard and
STTViewDatasetModal are incorrect and breaking the build; update the import
paths to use the project alias and correct filename casing (e.g., replace
relative "./STTDatasetCard" and "./STTViewDatasetModal" with the
"@/components/speech-to-text/..." alias that matches the actual filenames) so
CreateSTTDatasetForm, STTDatasetCard, and STTViewDatasetModal are all imported
via the "@/..." tsconfig alias and with exact case to resolve CI errors when
rendering the DatasetsTab component.
In `@app/components/TabNavigation.tsx`:
- Around line 22-29: The tab buttons in TabNavigation are missing an explicit
type and will submit enclosing forms; update the button element rendered for
each tab (the element using key={tab.id}, onClick={() => onTabChange(tab.id)}
and isActive for classes) to include type="button" so clicks do not trigger form
submission.
---
Outside diff comments:
In `@app/components/ConfigModal.tsx`:
- Around line 67-138: The effect can let an in-flight fetch overwrite state
after isOpen or job changes; in the useEffect/fetchConfigVersionInfo block,
cancel or ignore stale responses by creating an AbortController (or a local
requestId token) at the start of the effect, pass the signal to apiFetch calls,
and in the cleanup set a flag/abort to mark the request stale; before calling
setConfigVersionInfo or setIsLoadingConfig in the success/error/finally paths
verify the request is still current (not aborted and job.config_id/version and
isOpen still match) so results from a previous job or a closed modal are
ignored.
---
Nitpick comments:
In `@app/components/evaluations/DatasetsTab.tsx`:
- Around line 10-13: The file DatasetsTab.tsx uses relative imports for
DatasetCard, CreateDatasetForm, ViewDatasetModal, and DeleteDatasetModal which
violate the repo convention; update these import statements to use the project
alias (@"...") as configured in tsconfig.json (e.g. replace "./DatasetCard" etc.
with the corresponding "@/components/..." or "@/components/evaluations/..."
paths) so the components resolve from the repo root and follow the alias-based
import pattern.
In `@app/components/evaluations/EvaluationsTab.tsx`:
- Around line 8-9: Update the two relative imports so they use the project root
alias instead of relative paths: replace the current imports of EvalRunsList and
RunEvaluationForm with imports using the "@/..." alias (matching the project's
tsconfig paths) so the symbols EvalRunsList and RunEvaluationForm are resolved
from the root-based module path consistent with the rest of the file.
In `@app/components/evaluations/RunEvaluationForm.tsx`:
- Around line 8-10: Replace the local relative import and the locally redeclared
Tab union by using the repository root alias and the shared Tab type: change the
import of EvalDatasetDescription to use the "@/components/..." root import and
remove the local type declaration, importing the shared Tab type (named Tab)
from its module instead of redeclaring it; update any references to Tab in
RunEvaluationForm to use the imported symbol.
In `@app/components/ResultsTableSkeleton.tsx`:
- Around line 11-19: The staggered animationDelay on the cell divs isn't
effective because the animated class is only on the outer wrapper; update
ResultsTableSkeleton so the animation class is applied to each cell (the div
with className "flex-1 h-12 bg-neutral-100 rounded" that also has style={{
animationDelay: ... }}), e.g. add "animate-pulse" to that inner div (and
optionally remove it from the outer wrapper) so each cell respects its
individual animationDelay.
In `@app/components/speech-to-text/AudioFileItem.tsx`:
- Around line 53-59: In AudioFileItem.tsx update the remove button element (the
one with onRemove and CloseIcon) to include an explicit type attribute: set
type="button" so clicking the Remove action doesn't trigger form submission when
this component is rendered inside a form; locate the button with onRemove in the
AudioFileItem component and add the type attribute.
In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx`:
- Line 9: The import in CreateSTTDatasetForm.tsx currently uses a relative path
for AudioFileItem; update the import to use the project alias (replace the
"./AudioFileItem" import with the "@/components/speech-to-text/AudioFileItem"
alias) so it follows the repo convention and tsconfig.json path mapping, and
ensure the AudioFileItem symbol is still exported/imported correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6eed6132-079f-48e7-956a-5db5485703a0
📒 Files selected for processing (24)
app/components/ConfigModal.tsxapp/components/DatasetListSkeleton.tsxapp/components/ErrorModal.tsxapp/components/ResultsTableSkeleton.tsxapp/components/RunsListSkeleton.tsxapp/components/Select.tsxapp/components/TabNavigation.tsxapp/components/evaluations/CreateDatasetForm.tsxapp/components/evaluations/DatasetCard.tsxapp/components/evaluations/DatasetsTab.tsxapp/components/evaluations/DeleteDatasetModal.tsxapp/components/evaluations/EvalRunCard.tsxapp/components/evaluations/EvalRunsList.tsxapp/components/evaluations/EvaluationsTab.tsxapp/components/evaluations/RunEvaluationForm.tsxapp/components/evaluations/ViewDatasetModal.tsxapp/components/icons/common/CheckLineIcon.tsxapp/components/icons/common/MusicNoteIcon.tsxapp/components/icons/common/PlayIcon.tsxapp/components/icons/index.tsxapp/components/index.tsapp/components/speech-to-text/AudioFileItem.tsxapp/components/speech-to-text/CreateSTTDatasetForm.tsxapp/components/speech-to-text/DatasetsTab.tsx
vprashrex
approved these changes
May 4, 2026
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
README.md (1)
5-5:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInconsistent Discord invite URLs across the README.
Line 5 (badge) still uses the old Discord invite
BRYzSYha, while Line 173 (Chat With Us section) was updated tos7e2UBFku. This inconsistency will send users to different Discord servers depending on which link they click.Please ensure both locations reference the same Discord invite URL.
🔗 Proposed fix to align both Discord URLs
If the new URL is correct, update the badge on line 5:
-[](https://discord.gg/BRYzSYha) +[](https://discord.gg/s7e2UBFku)If the old URL is correct, revert line 173:
-- 💬 [Discord](https://discord.gg/s7e2UBFku) +- 💬 [Discord](https://discord.gg/BRYzSYha)Also applies to: 173-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 5, The README contains two different Discord invite IDs—BRYzSYha in the badge markdown and s7e2UBFku in the "Chat With Us" section—causing inconsistent links; pick the correct invite and make both occurrences identical by replacing the incorrect token (search for the strings "BRYzSYha" and "s7e2UBFku") so the badge markdown line that starts with the Discord shield and the "Chat With Us" section use the same invite ID.app/components/evaluations/DatasetsTab.tsx (1)
86-117:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
clientFetchfor these client-side requests.This tab is a client component, but both request paths still call
apiFetch, so expired sessions skip the standard 401 refresh/auth-expiry flow here.As per coding guidelines, "Use
clientFetch(endpoint, options)on the client-side for API requests, which handles token refresh on 401 errors and dispatches AUTH_EXPIRED_EVENT when refresh fails".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/DatasetsTab.tsx` around lines 86 - 117, Both handleDeleteDataset and handleViewDataset are using apiFetch which bypasses the client-side token refresh/401 handling; replace those calls with clientFetch so the AUTH_EXPIRED_EVENT and refresh flow run on 401. Update the call in handleDeleteDataset to call clientFetch(`/api/evaluations/datasets/${datasetId}`, { method: "DELETE", apiKey }) (or the project's clientFetch signature) and update the call in handleViewDataset to call clientFetch with the same endpoint and options used currently (include_signed_url and fetch_content query params and any apiKey/header option), and ensure you import clientFetch at top of the file; keep the existing try/catch/finally behavior unchanged.app/components/text-to-speech/DatasetsTab.tsx (1)
116-123:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
clientFetchhere too.This is another client component calling
apiFetch, so viewing a dataset bypasses the standard 401 refresh/auth-expiry handling that the rest of the client fetch path expects.As per coding guidelines, "Use
clientFetch(endpoint, options)on the client-side for API requests, which handles token refresh on 401 errors and dispatches AUTH_EXPIRED_EVENT when refresh fails".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/text-to-speech/DatasetsTab.tsx` around lines 116 - 123, The handleViewDataset function is calling apiFetch directly which bypasses client-side auth refresh logic; replace the apiFetch call in handleViewDataset with clientFetch so token refresh and AUTH_EXPIRED_EVENT handling occur, passing the same endpoint string and options (including the API key logic currently using apiKeys[0]?.key ?? "") and keep the rest of the flow (setViewingId, try/catch, response type TTSDatasetDetailResponse) unchanged; ensure the call signature matches clientFetch usage in other components.app/components/text-to-speech/EvaluationsTab.tsx (1)
72-84:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
is_correctis still being patched on score/comment-only edits.The new
undefinedbranch never fires with the current callers:app/components/text-to-speech/TTSResultRow.tsx:148-171,207still passesresult.is_correctfor score/comment commits. Those requests can still overwrite a newer correctness value with stale local state. The callers need to passundefinedwhen correctness is untouched.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/text-to-speech/EvaluationsTab.tsx` around lines 72 - 84, The updateFeedback call is still receiving stale correctness values (result.is_correct) from TTSResultRow, so score/comment-only edits can overwrite newer correctness; modify the callers in TTSResultRow (where updateFeedback is invoked for score/comment commits) to pass undefined for the isCorrect parameter when correctness was not changed (i.e., only pass a boolean when the user actually toggles correctness), and ensure updateFeedback(resultId, undefined, comment, score) is used for comment/score-only flows so the payload building in updateFeedback will omit is_correct entirely.
♻️ Duplicate comments (2)
app/components/evaluations/DatasetsTab.tsx (2)
125-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winQuoted multiline CSV cells are still being split into separate rows.
This still line-splits the raw CSV before parsing fields, so any valid quoted cell containing an embedded newline will shift the modal’s headers/rows out of alignment. The same issue was already flagged on the previous revision and is still present here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/DatasetsTab.tsx` around lines 125 - 127, Don't pre-split csvText on "\n" (the lines variable) because quoted CSV cells can contain embedded newlines; instead feed the entire csvText to a CSV-record-level parser that understands quoted fields (either update/rename parseCsvRow to parseCsvRecords or add a new parseCsvRecords function) and set headers = records[0] and rows = records.slice(1). Replace the current use of csvText.split("\n").filter(...) and parseCsvRow per-line logic so parsing is performed at the record level (using a proper CSV parser that handles quoted multiline cells) and then derive headers/rows from the returned records.
221-223:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the delete dialog open until the request settles.
setConfirmDeleteId(null)runs immediately after starting the delete, so the loading state never has a chance to render and the dialog also disappears on failure. This is the same lifecycle issue that was already called out earlier.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/DatasetsTab.tsx` around lines 221 - 223, The delete dialog is being closed immediately because setConfirmDeleteId(null) is called right after initiating deletion; change this so the dialog stays open until the delete request settles by moving the call into the post-request path: have handleDeleteDataset(confirmDeleteId) return a Promise (or accept a callback) and only call setConfirmDeleteId(null) after the promise resolves (or on successful delete) or clear it in the finally block if you want to close on both success/failure; reference the existing confirmDeleteId, handleDeleteDataset, and setConfirmDeleteId to implement this lifecycle fix.
🧹 Nitpick comments (13)
app/components/speech-to-text/STTDatasetCard.tsx (1)
29-33: 💤 Low valueRedundant optional chain on required
dataset_metadatafield.Per the
Datasettype (context snippet 1),dataset_metadata: DatasetMetadatais a required property, so?.will never short-circuit. Thesample_count?: numberoptional onDatasetMetadatais what actually necessitates the!== undefinedguard. The?.is harmless as a runtime defensive check, but it silently disagrees with the type contract — a future reader might incorrectly conclude the field can be absent.♻️ Proposed cleanup
- {dataset.dataset_metadata?.sample_count !== undefined && ( + {dataset.dataset_metadata.sample_count !== undefined && (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/STTDatasetCard.tsx` around lines 29 - 33, The JSX uses a redundant optional chain on dataset.dataset_metadata even though Dataset.type declares dataset_metadata as required; update the conditional and rendering in STTDatasetCard.tsx to remove the ?. on dataset_metadata while preserving the sample_count existence check (i.e., keep the sample_count !== undefined guard and render dataset.dataset_metadata.sample_count), so the code aligns with the Dataset type contract without changing runtime behavior.app/components/speech-to-text/STTViewDatasetModal.tsx (1)
130-132: ⚡ Quick win
onBlurunconditionally firesonUpdateSample, even when the value is unchanged.Every time the user focuses into and out of the textarea without editing, a PATCH request is dispatched. Consider tracking the "last saved" value and skipping the call when there is no diff.
♻️ Proposed fix
onBlur={(e) => { + const original = data.samples.find((s) => s.id === sample.id) + ?.ground_truth ?? ""; + if (e.target.value === original) return; onUpdateSample(sample.id, "ground_truth", e.target.value); }}Note:
datais already in scope from props, so no additional state is required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/STTViewDatasetModal.tsx` around lines 130 - 132, The onBlur handler currently always calls onUpdateSample causing redundant PATCHes; change it to compare the textarea value with the last saved ground truth before calling onUpdateSample. Specifically, in the onBlur for the textarea (where you currently call onUpdateSample(sample.id, "ground_truth", e.target.value)), first derive the saved value from props (e.g., locate the sample via data.samples.find(s => s.id === sample.id) or fallback to sample.ground_truth) and only call onUpdateSample if e.target.value !== savedValue.app/components/speech-to-text/CreateSTTDatasetForm.tsx (1)
8-8: 💤 Low valueUse
@/alias forAudioFileItemimport.Relative sibling imports bypass the configured path alias.
♻️ Proposed fix
-import AudioFileItem from "./AudioFileItem"; +import AudioFileItem from "@/app/components/speech-to-text/AudioFileItem";As per coding guidelines: "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx` at line 8, In CreateSTTDatasetForm.tsx replace the relative sibling import of AudioFileItem (import AudioFileItem from "./AudioFileItem") with the project-root alias import using "@/..." so the module resolves via the tsconfig path alias (i.e., import AudioFileItem from "@/.../AudioFileItem"); ensure the new import points to the same AudioFileItem export and update any editor/tsconfig settings if needed.app/lib/utils/evaluationExport.ts (1)
10-20: ⚡ Quick winMissing UTF-8 BOM may garble non-ASCII text when opened in Excel.
The
Blobis created without a BOM (\uFEFF). Excel auto-detects encoding from the BOM; without it, UTF-8 content (e.g., Hindi evaluation text) is often misinterpreted as a legacy codepage, rendering all non-ASCII characters as garbage. Adding the BOM is a one-character change and is safe for all other CSV consumers.🛠️ Proposed fix
- const blob = new Blob([csvContent], { type: "text/csv;charset=utf-8;" }); + const blob = new Blob(["\uFEFF", csvContent], { type: "text/csv;charset=utf-8;" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/utils/evaluationExport.ts` around lines 10 - 20, The downloadCSV function creates a Blob without a UTF-8 BOM, which causes Excel to misinterpret non-ASCII characters; fix it by prepending the BOM ('\uFEFF') to the csvContent before building the Blob (inside downloadCSV, where Blob is constructed) so the Blob contains the BOM and UTF-8 content is correctly detected when opened in Excel.app/components/speech-to-text/RunSTTEvaluationForm.tsx (1)
1-139: ⚡ Quick win
RunSTTEvaluationFormandRunTTSEvaluationFormare near-identical — extract a shared component.Both files (~140 lines each) share the exact same structure, props shape, and JSX. The only differences are the description string, name placeholder, and the single model option. Both datasets expose the same fields (
id,name,dataset_metadata?.sample_count). Keeping two copies means any future layout/UX change must be applied twice, and they will inevitably diverge.Consider extracting a shared
RunEvaluationFormaccepting amodelOptionsarray and a few string overrides:♻️ Sketch of unified interface
// e.g. app/components/shared/RunEvaluationForm.tsx interface BaseDataset { id: number; name: string; dataset_metadata?: { sample_count?: number }; } interface RunEvaluationFormProps<T extends BaseDataset> { title?: string; // default "Run New Evaluation" description: string; namePlaceholder: string; modelOptions: { value: string; label: string }[]; evaluationName: string; setEvaluationName: (name: string) => void; datasets: T[]; isLoadingDatasets: boolean; selectedDatasetId: number | null; setSelectedDatasetId: (id: number | null) => void; selectedDataset: T | undefined; selectedModel: string; setSelectedModel: (model: string) => void; isRunning: boolean; handleRunEvaluation: () => void; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/RunSTTEvaluationForm.tsx` around lines 1 - 139, RunSTTEvaluationForm and RunTTSEvaluationForm are duplicated; extract a shared RunEvaluationForm component and have both forms delegate to it. Create RunEvaluationForm(props) that accepts title, description, namePlaceholder, modelOptions and the existing state handlers (evaluationName, setEvaluationName, datasets, isLoadingDatasets, selectedDatasetId, setSelectedDatasetId, selectedDataset, selectedModel, setSelectedModel, isRunning, handleRunEvaluation) and move the JSX from RunSTTEvaluationForm into it; then update RunSTTEvaluationForm and RunTTSEvaluationForm to import RunEvaluationForm and call it with the appropriate modelOptions and text overrides. Ensure you keep the same prop names (e.g., evaluationName, setEvaluationName, selectedDatasetId, setSelectedDatasetId, selectedModel, setSelectedModel, handleRunEvaluation) so callers need minimal changes.app/components/evaluations/EvaluationsTab.tsx (1)
8-9: ⚡ Quick winUse the project alias for these extracted-component imports.
"./EvalRunsList"and"./RunEvaluationForm"should be imported through@/app/...to match the repository convention.As per coding guidelines, "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root (e.g.,import { apiClient } from '@/app/lib/apiClient')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/EvaluationsTab.tsx` around lines 8 - 9, Replace the relative imports in EvaluationsTab.tsx for the components EvalRunsList and RunEvaluationForm with the project alias import form (use the '@/app/...' alias) to match repository convention; update the two import lines that reference "./EvalRunsList" and "./RunEvaluationForm" so they import EvalRunsList and RunEvaluationForm via the '@/app/...' root alias instead of relative paths.app/components/speech-to-text/STTResultsTable.tsx (1)
6-7: ⚡ Quick winUse the project alias for these component imports.
"./STTResultRow"and"./STTScoreInfoTooltip"should follow the repo's@/import convention.As per coding guidelines, "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root (e.g.,import { apiClient } from '@/app/lib/apiClient')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/STTResultsTable.tsx` around lines 6 - 7, The imports for the child components use relative paths; replace them to use the project alias by importing STTResultRow and STTScoreInfoTooltip via the '@/...' alias (e.g., change "./STTResultRow" and "./STTScoreInfoTooltip" to their corresponding '@/...' paths) so the module resolution follows the tsconfig.json alias convention and keeps consistency across the codebase.app/components/speech-to-text/STTRunsList.tsx (2)
86-90: ⚡ Quick winAvoid embedding SVG markup in the
className.This data-URL arrow bypasses the repo's SVG handling rule and is harder to theme or reuse. Prefer a normal icon component or an asset referenced from
/public.As per coding guidelines, "SVGs follow Next.js defaults and should be imported as static assets via next/image or referenced from /public".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/STTRunsList.tsx` around lines 86 - 90, The select in STTRunsList.tsx embeds an SVG data-URL inside its className; remove the bg-[url(...)] and related bg-position/bg-no-repeat utilities from the select's className and replace the visual arrow with a proper asset or component: either import an SVG/Icon component and render it adjacent to the <select> (using a container with relative positioning and padding-right to keep layout), or reference a static SVG in /public and use CSS background-image via a class or inline style that points to that file. Update the select rendering in the STTRunsList component (the select with value={statusFilter} onChange={...}) to use the new icon approach and adjust spacing (e.g., padding) accordingly so the arrow is accessible and themeable.
11-12: ⚡ Quick winUse the project alias for these component imports.
"./STTRunCard"and"./STTResultsTable"should use@/app/...imports to match the repo convention.As per coding guidelines, "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root (e.g.,import { apiClient } from '@/app/lib/apiClient')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/STTRunsList.tsx` around lines 11 - 12, Replace the relative imports for the STTRunCard and STTResultsTable components with the project alias imports using the "@/app/..." alias: locate the import statements that reference STTRunCard and STTResultsTable (currently "./STTRunCard" and "./STTResultsTable") in STTRunsList and change them to use the "@/app/..." alias form consistent with repo convention (e.g., import STTRunCard and STTResultsTable via the "@/app/..." paths).app/components/text-to-speech/EvaluationsTab.tsx (1)
15-16: ⚡ Quick winUse the project alias for these local imports.
"./TTSRunsList"and"./RunTTSEvaluationForm"bypass the repo's import convention. Switch them to@/app/...imports so path resolution stays consistent across moves and refactors.As per coding guidelines, "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root (e.g.,import { apiClient } from '@/app/lib/apiClient')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/text-to-speech/EvaluationsTab.tsx` around lines 15 - 16, The imports in EvaluationsTab.tsx use relative paths ("./TTSRunsList" and "./RunTTSEvaluationForm") which violates the repo convention; replace them with the project alias form (e.g., import TTSRunsList from '@/app/components/text-to-speech/TTSRunsList' and import RunTTSEvaluationForm from '@/app/components/text-to-speech/RunTTSEvaluationForm') so TTSRunsList and RunTTSEvaluationForm resolve via the tsconfig "@/..." alias and remain stable across moves/refactors.app/components/evaluations/RunEvaluationForm.tsx (1)
8-8: ⚡ Quick winUse the project alias for this component import.
"./EvalDatasetDescription"should follow the repo's@/import convention for local modules.As per coding guidelines, "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root (e.g.,import { apiClient } from '@/app/lib/apiClient')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/evaluations/RunEvaluationForm.tsx` at line 8, The import in RunEvaluationForm.tsx uses a relative path for EvalDatasetDescription; change it to use the project alias (`@/`) per the repo convention (e.g., import EvalDatasetDescription from '@/...') so the component resolves from the project root; update the import statement that references EvalDatasetDescription to the correct '@/...' path that matches the module location in the project.app/components/speech-to-text/EvaluationsTab.tsx (1)
7-9: ⚡ Quick winUse the project alias for these local imports.
"./STTRunsList"and"./RunSTTEvaluationForm"should come through@/app/..., and the same applies to any local type/component import that can be resolved from the project root.As per coding guidelines, "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root (e.g.,import { apiClient } from '@/app/lib/apiClient')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/EvaluationsTab.tsx` around lines 7 - 9, The imports in EvaluationsTab.tsx use relative paths; update them to use the project root alias so local components/types resolve via "@/app/...". Replace the "./STTRunsList" and "./RunSTTEvaluationForm" imports with "@/app/components/speech-to-text/STTRunsList" and "@/app/components/speech-to-text/RunSTTEvaluationForm" (and any other local imports in this file) while leaving the Tab type import as "@/app/lib/types/evaluation" if not already aliased, ensuring the symbols STTRunsList and RunSTTEvaluationForm are imported via the "@/..." alias.app/components/speech-to-text/STTResultRow.tsx (1)
4-5: ⚡ Quick winUse the project alias for these local imports.
"./AudioPlayerFromUrl"and"./TranscriptionDiffViewer"should be imported through@/app/...instead of sibling relative paths.As per coding guidelines, "Use the
@/import alias configured in tsconfig.json to resolve imports from the project root (e.g.,import { apiClient } from '@/app/lib/apiClient')".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/speech-to-text/STTResultRow.tsx` around lines 4 - 5, Replace the sibling relative imports in STTResultRow.tsx with the project alias paths: change the imports for AudioPlayerFromUrl and computeWordDiff to use the '@/app/...' alias (referencing the AudioPlayerFromUrl component and the computeWordDiff export from TranscriptionDiffViewer) so they resolve from the project root instead of "./AudioPlayerFromUrl" and "./TranscriptionDiffViewer"; update the import statements to reference the correct alias-based locations for those modules.
🤖 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/`(main)/evaluations/[id]/page.tsx:
- Around line 77-80: Replace client-side uses of apiFetch with clientFetch for
the evaluation API calls: where you call
apiFetch<EvalJobApiResponse>(`/api/evaluations/${jobId}?export_format=${exportFormat}`,
apiKey) (and the analogous call in the resync path), switch to
clientFetch(endpoint, { method, headers, body, apiKey }) so the request goes
through the token-refresh/401 handling. Locate the two spots that reference
apiFetch and EvalJobApiResponse and update them to call clientFetch with the
same endpoint and options, preserving jobId and exportFormat variables and
existing headers/options.
- Around line 178-184: The code currently casts the API response into an EvalJob
without checking for a failed resync envelope; update the logic around apiFetch
and foundJob so you first check if data.success === false (or otherwise
indicates failure) and throw or handle the error using the response's
error/message before attempting to use data.data or casting to EvalJob;
specifically modify the block that declares data and foundJob (the apiFetch call
and the foundJob assignment) to validate success, surface the error (e.g., throw
new Error(data.error || data.message || 'Evaluation job not found')), and only
then set foundJob = data.data as EvalJob (or use the fallback cast) so a failed
resync envelope cannot replace the job object.
In `@app/components/evaluations/MetricsOverview.tsx`:
- Around line 70-74: In MetricsOverview (JSX span where job.total_items and
summary.total_pairs are used) avoid using the numeric short-circuit pattern;
replace the expression `job.total_items && summary.total_pairs < job.total_items
&& \`/${job.total_items}\`` with an explicit boolean check such as
`job.total_items > 0 && summary.total_pairs < job.total_items` (use this for
both the NUMERIC and CATEGORICAL blocks referenced around lines where
job.total_items and summary.total_pairs are rendered) so that when total_items
is 0 it doesn't render the literal "0" and only renders the
`/${job.total_items}` suffix when the condition truly holds.
In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx`:
- Around line 180-193: The button should be truly disabled when the user is
unauthenticated: set the button's disabled prop to !isAuthenticated, add
aria-disabled={String(!isAuthenticated)} and tabIndex={isAuthenticated ? 0 : -1}
so it is removed from keyboard focus, and keep/update the className to reflect
disabled styling (e.g., cursor-not-allowed/opacity) so users get visual
feedback; locate the button using isAuthenticated and triggerAudioUpload and
update usage of uploadedCount/audioFiles accordingly.
- Around line 55-57: isCreateDisabled currently only checks isCreating,
datasetName and audioFiles.length, so the "Create Dataset" button can be clicked
before uploads finish; update the logic used where isCreateDisabled is defined
(referencing isCreateDisabled, uploadedCount, audioFiles, and
handleCreateDataset) to also require that uploadedCount === audioFiles.length
(i.e., all files have a fileId) before enabling the button—this prevents calling
handleCreateDataset with undefined fileIds.
In `@app/components/speech-to-text/RunSTTEvaluationForm.tsx`:
- Around line 84-87: The onChange handler in RunSTTEvaluationForm.tsx uses
parseInt without a radix which can produce incorrect parsing; update the call in
the setSelectedDatasetId callback (the onChange arrow function that reads
e.target.value) to call parseInt(e.target.value, 10) (or Number(e.target.value)
if preferred) so the value is parsed explicitly in base 10 before passing to
setSelectedDatasetId.
In `@app/components/speech-to-text/STTResultRow.tsx`:
- Around line 250-255: The blur handler on the textarea currently calls
onCommitFeedback(result.is_correct ?? null, e.target.value), which
unintentionally resends/stales the correctness field; change the textarea onBlur
to omit or pass undefined for the correctness argument so only the comment is
committed (e.g., call onCommitFeedback(undefined, e.target.value) or the
single-arg comment form), updating usages of onCommitFeedback if needed to
accept an optional correctness param; reference the textarea onBlur,
onCommitFeedback, onLocalCommentChange, and the result.is_correct field to
locate and fix the call so it no longer PATCHes correctness on comment-only
blurs and respects the optional-field contract in EvaluationsTab.
In `@app/components/speech-to-text/STTViewDatasetModal.tsx`:
- Around line 101-122: The select for sample.language_id is controlled with
value={sample.language_id ?? ""} but no <option value=""> exists, causing React
to display the first language and warn; add a disabled placeholder option (e.g.,
<option value="" disabled>Select language</option>) at the top of the options
rendered by languages.map so when sample.language_id is null/undefined the
placeholder is shown; update the JSX around the select in STTViewDatasetModal
(the select using sample.language_id, onChange -> onUpdateSample, and
languages.map) to include that empty/placeholder option and ensure it's
selectable only when not disabled by isSaving.
- Around line 53-66: Add proper accessibility attributes: add scope="col" to
each <th> in the table header, give the first empty header a descriptive
aria-label (e.g., aria-label="row-select" or similar), and associate the form
controls by adding aria-label attributes to the <select> in the Language column
and the <textarea> in the Ground Truth column (or add visually hidden <label>
elements tied via id/for). Update these elements inside STTViewDatasetModal.tsx
(the table <th> elements, the Language <select>, and the Ground Truth
<textarea>) so assistive tech can identify column headers and controls.
In `@app/components/text-to-speech/CreateTTSDatasetForm.tsx`:
- Around line 139-149: The Add Sample button in CreateTTSDatasetForm currently
relies on conditional onClick (addTextSample) and remains focusable when
isAuthenticated is false; make it a real disabled control by adding the disabled
prop (disabled={!isAuthenticated}) to the button, always pass the handler as
onClick={addTextSample} (disabled will prevent activation), and update the
className conditional to reflect disabled styling (keep "cursor-not-allowed"
when !isAuthenticated and remove "cursor-pointer" for that case) so keyboard
users receive proper feedback; target the CreateTTSDatasetForm button element
and the addTextSample/isAuthenticated symbols when making the change.
In `@app/components/text-to-speech/RunTTSEvaluationForm.tsx`:
- Around line 89-92: The onChange handler in RunTTSEvaluationForm uses
parseInt(e.target.value) without a radix; update that call to include the radix
(e.g., parseInt(e.target.value, 10)) or replace it with a safer numeric
conversion (e.g., Number(e.target.value) or +e.target.value) before passing the
result to setSelectedDatasetId so the component's onChange uses an unambiguous
base when converting the string to an integer.
In `@app/components/text-to-speech/TTSResultRow.tsx`:
- Around line 141-173: The handlers currently call onCommitFeedback with only
the changed field (e.g., onCommitFeedback(result.is_correct, undefined, {
"Speech Naturalness": value })) which can cause the backend to overwrite the
whole score and lose other fields; update both select handlers (the one that
constructs newScore for "Speech Naturalness" and the one for "Pronunciation
Accuracy") to compute the merged TTSScore (const newScore = { ...(result.score
|| {}), "Speech Naturalness": value } or "Pronunciation Accuracy": value) and
pass that full newScore into onCommitFeedback instead of the single-field
object, while still calling onUpdateLocalScore(newScore).
In `@app/components/text-to-speech/TTSViewDatasetModal.tsx`:
- Around line 15-29: The CSV header line is built by raw join
(data.headers.join(",")) which fails to escape commas/quotes/newlines; update
the handleDownload function to reuse the same escaping logic used for rows:
create a helper (e.g., escapeCsvCell) or inline the same map/replace logic and
apply it to each header cell before joining, so headers are wrapped/quoted and
quotes escaped exactly like row cells when building csvLines.
In `@app/lib/utils/evaluationExport.ts`:
- Around line 30-36: The CSV header writes raw score names into CSV columns
which can break alignment if a name contains commas or quotes; update
exportGroupedCSV (the loop that builds csvContent with `LLM Answer ${i},Trace ID
${i}` and the `${name} (${i})` entries) to pass every header label through
sanitizeCSVCell (including the value column label and the comment label), and
likewise update exportRowCSV where headers are built (`${name},${name}
(comment)`) to sanitize both the score name and the comment label using
sanitizeCSVCell so all header cells are correctly escaped.
- Line 28: The code computing maxAnswers in exportGroupedCSV uses
Math.max(...traces.map(...)) which returns -Infinity for empty traces; fix by
ensuring a safe default and a clear output when traces is empty: set maxAnswers
= Math.max(0, ...traces.map(g => g.llm_answers.length)) (or use traces.length ?
Math.max(...) : 0) and if traces.length === 0, early-call downloadCSV with the
header plus a single row like "No traces available" (or otherwise return early)
so users aren't given a mysterious empty file; update references in
exportGroupedCSV where maxAnswers and downloadCSV are used.
---
Outside diff comments:
In `@app/components/evaluations/DatasetsTab.tsx`:
- Around line 86-117: Both handleDeleteDataset and handleViewDataset are using
apiFetch which bypasses the client-side token refresh/401 handling; replace
those calls with clientFetch so the AUTH_EXPIRED_EVENT and refresh flow run on
401. Update the call in handleDeleteDataset to call
clientFetch(`/api/evaluations/datasets/${datasetId}`, { method: "DELETE", apiKey
}) (or the project's clientFetch signature) and update the call in
handleViewDataset to call clientFetch with the same endpoint and options used
currently (include_signed_url and fetch_content query params and any
apiKey/header option), and ensure you import clientFetch at top of the file;
keep the existing try/catch/finally behavior unchanged.
In `@app/components/text-to-speech/DatasetsTab.tsx`:
- Around line 116-123: The handleViewDataset function is calling apiFetch
directly which bypasses client-side auth refresh logic; replace the apiFetch
call in handleViewDataset with clientFetch so token refresh and
AUTH_EXPIRED_EVENT handling occur, passing the same endpoint string and options
(including the API key logic currently using apiKeys[0]?.key ?? "") and keep the
rest of the flow (setViewingId, try/catch, response type
TTSDatasetDetailResponse) unchanged; ensure the call signature matches
clientFetch usage in other components.
In `@app/components/text-to-speech/EvaluationsTab.tsx`:
- Around line 72-84: The updateFeedback call is still receiving stale
correctness values (result.is_correct) from TTSResultRow, so score/comment-only
edits can overwrite newer correctness; modify the callers in TTSResultRow (where
updateFeedback is invoked for score/comment commits) to pass undefined for the
isCorrect parameter when correctness was not changed (i.e., only pass a boolean
when the user actually toggles correctness), and ensure updateFeedback(resultId,
undefined, comment, score) is used for comment/score-only flows so the payload
building in updateFeedback will omit is_correct entirely.
In `@README.md`:
- Line 5: The README contains two different Discord invite IDs—BRYzSYha in the
badge markdown and s7e2UBFku in the "Chat With Us" section—causing inconsistent
links; pick the correct invite and make both occurrences identical by replacing
the incorrect token (search for the strings "BRYzSYha" and "s7e2UBFku") so the
badge markdown line that starts with the Discord shield and the "Chat With Us"
section use the same invite ID.
---
Duplicate comments:
In `@app/components/evaluations/DatasetsTab.tsx`:
- Around line 125-127: Don't pre-split csvText on "\n" (the lines variable)
because quoted CSV cells can contain embedded newlines; instead feed the entire
csvText to a CSV-record-level parser that understands quoted fields (either
update/rename parseCsvRow to parseCsvRecords or add a new parseCsvRecords
function) and set headers = records[0] and rows = records.slice(1). Replace the
current use of csvText.split("\n").filter(...) and parseCsvRow per-line logic so
parsing is performed at the record level (using a proper CSV parser that handles
quoted multiline cells) and then derive headers/rows from the returned records.
- Around line 221-223: The delete dialog is being closed immediately because
setConfirmDeleteId(null) is called right after initiating deletion; change this
so the dialog stays open until the delete request settles by moving the call
into the post-request path: have handleDeleteDataset(confirmDeleteId) return a
Promise (or accept a callback) and only call setConfirmDeleteId(null) after the
promise resolves (or on successful delete) or clear it in the finally block if
you want to close on both success/failure; reference the existing
confirmDeleteId, handleDeleteDataset, and setConfirmDeleteId to implement this
lifecycle fix.
---
Nitpick comments:
In `@app/components/evaluations/EvaluationsTab.tsx`:
- Around line 8-9: Replace the relative imports in EvaluationsTab.tsx for the
components EvalRunsList and RunEvaluationForm with the project alias import form
(use the '@/app/...' alias) to match repository convention; update the two
import lines that reference "./EvalRunsList" and "./RunEvaluationForm" so they
import EvalRunsList and RunEvaluationForm via the '@/app/...' root alias instead
of relative paths.
In `@app/components/evaluations/RunEvaluationForm.tsx`:
- Line 8: The import in RunEvaluationForm.tsx uses a relative path for
EvalDatasetDescription; change it to use the project alias (`@/`) per the repo
convention (e.g., import EvalDatasetDescription from '@/...') so the component
resolves from the project root; update the import statement that references
EvalDatasetDescription to the correct '@/...' path that matches the module
location in the project.
In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx`:
- Line 8: In CreateSTTDatasetForm.tsx replace the relative sibling import of
AudioFileItem (import AudioFileItem from "./AudioFileItem") with the
project-root alias import using "@/..." so the module resolves via the tsconfig
path alias (i.e., import AudioFileItem from "@/.../AudioFileItem"); ensure the
new import points to the same AudioFileItem export and update any
editor/tsconfig settings if needed.
In `@app/components/speech-to-text/EvaluationsTab.tsx`:
- Around line 7-9: The imports in EvaluationsTab.tsx use relative paths; update
them to use the project root alias so local components/types resolve via
"@/app/...". Replace the "./STTRunsList" and "./RunSTTEvaluationForm" imports
with "@/app/components/speech-to-text/STTRunsList" and
"@/app/components/speech-to-text/RunSTTEvaluationForm" (and any other local
imports in this file) while leaving the Tab type import as
"@/app/lib/types/evaluation" if not already aliased, ensuring the symbols
STTRunsList and RunSTTEvaluationForm are imported via the "@/..." alias.
In `@app/components/speech-to-text/RunSTTEvaluationForm.tsx`:
- Around line 1-139: RunSTTEvaluationForm and RunTTSEvaluationForm are
duplicated; extract a shared RunEvaluationForm component and have both forms
delegate to it. Create RunEvaluationForm(props) that accepts title, description,
namePlaceholder, modelOptions and the existing state handlers (evaluationName,
setEvaluationName, datasets, isLoadingDatasets, selectedDatasetId,
setSelectedDatasetId, selectedDataset, selectedModel, setSelectedModel,
isRunning, handleRunEvaluation) and move the JSX from RunSTTEvaluationForm into
it; then update RunSTTEvaluationForm and RunTTSEvaluationForm to import
RunEvaluationForm and call it with the appropriate modelOptions and text
overrides. Ensure you keep the same prop names (e.g., evaluationName,
setEvaluationName, selectedDatasetId, setSelectedDatasetId, selectedModel,
setSelectedModel, handleRunEvaluation) so callers need minimal changes.
In `@app/components/speech-to-text/STTDatasetCard.tsx`:
- Around line 29-33: The JSX uses a redundant optional chain on
dataset.dataset_metadata even though Dataset.type declares dataset_metadata as
required; update the conditional and rendering in STTDatasetCard.tsx to remove
the ?. on dataset_metadata while preserving the sample_count existence check
(i.e., keep the sample_count !== undefined guard and render
dataset.dataset_metadata.sample_count), so the code aligns with the Dataset type
contract without changing runtime behavior.
In `@app/components/speech-to-text/STTResultRow.tsx`:
- Around line 4-5: Replace the sibling relative imports in STTResultRow.tsx with
the project alias paths: change the imports for AudioPlayerFromUrl and
computeWordDiff to use the '@/app/...' alias (referencing the AudioPlayerFromUrl
component and the computeWordDiff export from TranscriptionDiffViewer) so they
resolve from the project root instead of "./AudioPlayerFromUrl" and
"./TranscriptionDiffViewer"; update the import statements to reference the
correct alias-based locations for those modules.
In `@app/components/speech-to-text/STTResultsTable.tsx`:
- Around line 6-7: The imports for the child components use relative paths;
replace them to use the project alias by importing STTResultRow and
STTScoreInfoTooltip via the '@/...' alias (e.g., change "./STTResultRow" and
"./STTScoreInfoTooltip" to their corresponding '@/...' paths) so the module
resolution follows the tsconfig.json alias convention and keeps consistency
across the codebase.
In `@app/components/speech-to-text/STTRunsList.tsx`:
- Around line 86-90: The select in STTRunsList.tsx embeds an SVG data-URL inside
its className; remove the bg-[url(...)] and related bg-position/bg-no-repeat
utilities from the select's className and replace the visual arrow with a proper
asset or component: either import an SVG/Icon component and render it adjacent
to the <select> (using a container with relative positioning and padding-right
to keep layout), or reference a static SVG in /public and use CSS
background-image via a class or inline style that points to that file. Update
the select rendering in the STTRunsList component (the select with
value={statusFilter} onChange={...}) to use the new icon approach and adjust
spacing (e.g., padding) accordingly so the arrow is accessible and themeable.
- Around line 11-12: Replace the relative imports for the STTRunCard and
STTResultsTable components with the project alias imports using the "@/app/..."
alias: locate the import statements that reference STTRunCard and
STTResultsTable (currently "./STTRunCard" and "./STTResultsTable") in
STTRunsList and change them to use the "@/app/..." alias form consistent with
repo convention (e.g., import STTRunCard and STTResultsTable via the "@/app/..."
paths).
In `@app/components/speech-to-text/STTViewDatasetModal.tsx`:
- Around line 130-132: The onBlur handler currently always calls onUpdateSample
causing redundant PATCHes; change it to compare the textarea value with the last
saved ground truth before calling onUpdateSample. Specifically, in the onBlur
for the textarea (where you currently call onUpdateSample(sample.id,
"ground_truth", e.target.value)), first derive the saved value from props (e.g.,
locate the sample via data.samples.find(s => s.id === sample.id) or fallback to
sample.ground_truth) and only call onUpdateSample if e.target.value !==
savedValue.
In `@app/components/text-to-speech/EvaluationsTab.tsx`:
- Around line 15-16: The imports in EvaluationsTab.tsx use relative paths
("./TTSRunsList" and "./RunTTSEvaluationForm") which violates the repo
convention; replace them with the project alias form (e.g., import TTSRunsList
from '@/app/components/text-to-speech/TTSRunsList' and import
RunTTSEvaluationForm from
'@/app/components/text-to-speech/RunTTSEvaluationForm') so TTSRunsList and
RunTTSEvaluationForm resolve via the tsconfig "@/..." alias and remain stable
across moves/refactors.
In `@app/lib/utils/evaluationExport.ts`:
- Around line 10-20: The downloadCSV function creates a Blob without a UTF-8
BOM, which causes Excel to misinterpret non-ASCII characters; fix it by
prepending the BOM ('\uFEFF') to the csvContent before building the Blob (inside
downloadCSV, where Blob is constructed) so the Blob contains the BOM and UTF-8
content is correctly detected when opened in Excel.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73ae6983-0f4a-48be-8839-7611e9e3dc66
⛔ Files ignored due to path filters (1)
app/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (45)
README.mdapp/(main)/evaluations/[id]/page.tsxapp/(main)/evaluations/page.tsxapp/(main)/speech-to-text/page.tsxapp/(main)/text-to-speech/page.tsxapp/components/Button.tsxapp/components/Select.tsxapp/components/StatusBadge.tsxapp/components/evaluations/DatasetCard.tsxapp/components/evaluations/DatasetsTab.tsxapp/components/evaluations/DetailedResultsTable.tsxapp/components/evaluations/EvalRunCard.tsxapp/components/evaluations/EvaluationsTab.tsxapp/components/evaluations/GroupedResultsTable.tsxapp/components/evaluations/MetricsOverview.tsxapp/components/evaluations/RunEvaluationForm.tsxapp/components/speech-to-text/AudioFileItem.tsxapp/components/speech-to-text/CreateSTTDatasetForm.tsxapp/components/speech-to-text/DatasetsTab.tsxapp/components/speech-to-text/EvaluationsTab.tsxapp/components/speech-to-text/RunSTTEvaluationForm.tsxapp/components/speech-to-text/STTDatasetCard.tsxapp/components/speech-to-text/STTResultRow.tsxapp/components/speech-to-text/STTResultsTable.tsxapp/components/speech-to-text/STTRunCard.tsxapp/components/speech-to-text/STTRunsList.tsxapp/components/speech-to-text/STTScoreInfoTooltip.tsxapp/components/speech-to-text/STTViewDatasetModal.tsxapp/components/text-to-speech/CreateTTSDatasetForm.tsxapp/components/text-to-speech/DatasetsTab.tsxapp/components/text-to-speech/EvaluationsTab.tsxapp/components/text-to-speech/RunTTSEvaluationForm.tsxapp/components/text-to-speech/TTSDatasetCard.tsxapp/components/text-to-speech/TTSResultRow.tsxapp/components/text-to-speech/TTSResultsTable.tsxapp/components/text-to-speech/TTSRunCard.tsxapp/components/text-to-speech/TTSRunsList.tsxapp/components/text-to-speech/TTSScoreInfoTooltip.tsxapp/components/text-to-speech/TTSViewDatasetModal.tsxapp/lib/types/dataset.tsapp/lib/types/evaluation.tsapp/lib/types/nav.tsapp/lib/types/speechToText.tsapp/lib/types/textToSpeech.tsapp/lib/utils/evaluationExport.ts
💤 Files with no reviewable changes (1)
- app/components/StatusBadge.tsx
✅ Files skipped from review due to trivial changes (5)
- app/components/Select.tsx
- app/lib/types/dataset.ts
- app/components/speech-to-text/STTScoreInfoTooltip.tsx
- app/components/Button.tsx
- app/lib/types/speechToText.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- app/components/speech-to-text/DatasetsTab.tsx
Comment on lines
+77
to
80
| const data = await apiFetch<EvalJobApiResponse>( | ||
| `/api/evaluations/${jobId}?export_format=${exportFormat}`, | ||
| apiKey, | ||
| ); |
There was a problem hiding this comment.
Use clientFetch() for these client-side requests.
Both the report load and resync paths now call apiFetch() from a client page. That skips the repo's 401 refresh/auth-expiry handling, so expired sessions can surface as broken report fetches instead of the normal refresh flow.
As per coding guidelines, "Use clientFetch(endpoint, options) on the client-side for API requests, which handles token refresh on 401 errors and dispatches AUTH_EXPIRED_EVENT when refresh fails".
Also applies to: 178-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(main)/evaluations/[id]/page.tsx around lines 77 - 80, Replace
client-side uses of apiFetch with clientFetch for the evaluation API calls:
where you call
apiFetch<EvalJobApiResponse>(`/api/evaluations/${jobId}?export_format=${exportFormat}`,
apiKey) (and the analogous call in the resync path), switch to
clientFetch(endpoint, { method, headers, body, apiKey }) so the request goes
through the token-refresh/401 handling. Locate the two spots that reference
apiFetch and EvalJobApiResponse and update them to call clientFetch with the
same endpoint and options, preserving jobId and exportFormat variables and
existing headers/options.
Comment on lines
+178
to
184
| const data = await apiFetch<EvalJobApiResponse>( | ||
| `/api/evaluations/${jobId}?get_trace_info=true&resync_score=true&export_format=${exportFormat}`, | ||
| apiKey, | ||
| ); | ||
| const foundJob = data.data || data; | ||
| const foundJob: EvalJob | undefined = | ||
| data.data ?? (data as unknown as EvalJob); | ||
| if (!foundJob) throw new Error("Evaluation job not found"); |
There was a problem hiding this comment.
Handle failed resync envelopes before casting to EvalJob.
Unlike fetchJobDetails, this path never checks data.success === false. A failed resync response will fall through data.data ?? (data as EvalJob) and can replace job with the error envelope, which will break the next render.
Suggested guard
const data = await apiFetch<EvalJobApiResponse>(
`/api/evaluations/${jobId}?get_trace_info=true&resync_score=true&export_format=${exportFormat}`,
apiKey,
);
+ if (data.success === false) {
+ throw new Error(data.error || "Failed to resync evaluation job");
+ }
const foundJob: EvalJob | undefined =
data.data ?? (data as unknown as EvalJob);
if (!foundJob) throw new Error("Evaluation job not found");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(main)/evaluations/[id]/page.tsx around lines 178 - 184, The code
currently casts the API response into an EvalJob without checking for a failed
resync envelope; update the logic around apiFetch and foundJob so you first
check if data.success === false (or otherwise indicates failure) and throw or
handle the error using the response's error/message before attempting to use
data.data or casting to EvalJob; specifically modify the block that declares
data and foundJob (the apiFetch call and the foundJob assignment) to validate
success, surface the error (e.g., throw new Error(data.error || data.message ||
'Evaluation job not found')), and only then set foundJob = data.data as EvalJob
(or use the fallback cast) so a failed resync envelope cannot replace the job
object.
Comment on lines
+70
to
+74
| {job.total_items && | ||
| summary.total_pairs < job.total_items && | ||
| `/${job.total_items}`}{" "} | ||
| pairs | ||
| </span> |
There was a problem hiding this comment.
Numeric && short-circuit renders a literal "0" when total_items is 0.
job.total_items is typed as number (non-nullable). When it equals 0, the expression job.total_items && summary.total_pairs < job.total_items && \/${job.total_items}`short-circuits to the number0, and React renders "0"as text in the span — producing output like"5 0 pairs"`.
The same pattern is duplicated in both the NUMERIC (Line 70) and CATEGORICAL (Line 107) sections.
🐛 Proposed fix (both occurrences)
- {job.total_items &&
- summary.total_pairs < job.total_items &&
- `/${job.total_items}`}{" "}
+ {job.total_items > 0 &&
+ summary.total_pairs < job.total_items &&
+ `/${job.total_items}`}{" "}Apply this to both Line 70 and Line 107.
Also applies to: 107-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/evaluations/MetricsOverview.tsx` around lines 70 - 74, In
MetricsOverview (JSX span where job.total_items and summary.total_pairs are
used) avoid using the numeric short-circuit pattern; replace the expression
`job.total_items && summary.total_pairs < job.total_items &&
\`/${job.total_items}\`` with an explicit boolean check such as `job.total_items
> 0 && summary.total_pairs < job.total_items` (use this for both the NUMERIC and
CATEGORICAL blocks referenced around lines where job.total_items and
summary.total_pairs are rendered) so that when total_items is 0 it doesn't
render the literal "0" and only renders the `/${job.total_items}` suffix when
the condition truly holds.
Comment on lines
+55
to
+57
| const isCreateDisabled = | ||
| isCreating || !datasetName.trim() || audioFiles.length === 0; | ||
| const uploadedCount = audioFiles.filter((f) => f.fileId).length; |
There was a problem hiding this comment.
"Create Dataset" button is enabled before all uploads complete.
isCreateDisabled only checks whether audioFiles is non-empty, but uploadedCount (the files that actually have a fileId) is never checked. A user can select 10 files and click "Create Dataset" immediately while all uploads are still in-flight, sending the parent handleCreateDataset a payload with fileId: undefined for every file.
🐛 Proposed fix
const isCreateDisabled =
- isCreating || !datasetName.trim() || audioFiles.length === 0;
+ isCreating || !datasetName.trim() || audioFiles.length === 0 || uploadedCount < audioFiles.length;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const isCreateDisabled = | |
| isCreating || !datasetName.trim() || audioFiles.length === 0; | |
| const uploadedCount = audioFiles.filter((f) => f.fileId).length; | |
| const uploadedCount = audioFiles.filter((f) => f.fileId).length; | |
| const isCreateDisabled = | |
| isCreating || !datasetName.trim() || audioFiles.length === 0 || uploadedCount < audioFiles.length; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx` around lines 55 - 57,
isCreateDisabled currently only checks isCreating, datasetName and
audioFiles.length, so the "Create Dataset" button can be clicked before uploads
finish; update the logic used where isCreateDisabled is defined (referencing
isCreateDisabled, uploadedCount, audioFiles, and handleCreateDataset) to also
require that uploadedCount === audioFiles.length (i.e., all files have a fileId)
before enabling the button—this prevents calling handleCreateDataset with
undefined fileIds.
Comment on lines
+180
to
+193
| <button | ||
| onClick={isAuthenticated ? triggerAudioUpload : undefined} | ||
| className={`flex items-center gap-1 text-xs font-medium mt-2 ${ | ||
| isAuthenticated | ||
| ? "text-accent-primary cursor-pointer" | ||
| : "text-text-secondary cursor-not-allowed" | ||
| }`} | ||
| > | ||
| <PlusIcon className="w-3.5 h-3.5" /> | ||
| Add more samples | ||
| <span className="text-text-secondary"> | ||
| ({uploadedCount}/{audioFiles.length} uploaded) | ||
| </span> | ||
| </button> |
There was a problem hiding this comment.
"Add more samples" button is missing disabled when unauthenticated.
When !isAuthenticated the onClick is set to undefined, but the button has no disabled attribute. It remains focusable via keyboard; pressing Enter/Space fires nothing with no feedback — a silent trap for keyboard and screen-reader users.
🐛 Proposed fix
<button
- onClick={isAuthenticated ? triggerAudioUpload : undefined}
+ onClick={triggerAudioUpload}
+ disabled={!isAuthenticated}
className={`flex items-center gap-1 text-xs font-medium mt-2 ${
isAuthenticated
? "text-accent-primary cursor-pointer"
- : "text-text-secondary cursor-not-allowed"
+ : "text-text-secondary cursor-not-allowed opacity-50"
}`}
>📝 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
| <button | |
| onClick={isAuthenticated ? triggerAudioUpload : undefined} | |
| className={`flex items-center gap-1 text-xs font-medium mt-2 ${ | |
| isAuthenticated | |
| ? "text-accent-primary cursor-pointer" | |
| : "text-text-secondary cursor-not-allowed" | |
| }`} | |
| > | |
| <PlusIcon className="w-3.5 h-3.5" /> | |
| Add more samples | |
| <span className="text-text-secondary"> | |
| ({uploadedCount}/{audioFiles.length} uploaded) | |
| </span> | |
| </button> | |
| <button | |
| onClick={triggerAudioUpload} | |
| disabled={!isAuthenticated} | |
| className={`flex items-center gap-1 text-xs font-medium mt-2 ${ | |
| isAuthenticated | |
| ? "text-accent-primary cursor-pointer" | |
| : "text-text-secondary cursor-not-allowed opacity-50" | |
| }`} | |
| > | |
| <PlusIcon className="w-3.5 h-3.5" /> | |
| Add more samples | |
| <span className="text-text-secondary"> | |
| ({uploadedCount}/{audioFiles.length} uploaded) | |
| </span> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/speech-to-text/CreateSTTDatasetForm.tsx` around lines 180 -
193, The button should be truly disabled when the user is unauthenticated: set
the button's disabled prop to !isAuthenticated, add
aria-disabled={String(!isAuthenticated)} and tabIndex={isAuthenticated ? 0 : -1}
so it is removed from keyboard focus, and keep/update the className to reflect
disabled styling (e.g., cursor-not-allowed/opacity) so users get visual
feedback; locate the button using isAuthenticated and triggerAudioUpload and
update usage of uploadedCount/audioFiles accordingly.
Comment on lines
+89
to
+92
| onChange={(e) => | ||
| setSelectedDatasetId( | ||
| e.target.value ? parseInt(e.target.value) : null, | ||
| ) |
There was a problem hiding this comment.
parseInt is missing its radix argument.
parseInt(e.target.value) without a radix triggers the ESLint radix rule and is technically ambiguous for strings starting with "0".
🛠️ Proposed fix
- setSelectedDatasetId(
- e.target.value ? parseInt(e.target.value) : null,
- )
+ setSelectedDatasetId(
+ e.target.value ? parseInt(e.target.value, 10) : null,
+ )📝 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
| onChange={(e) => | |
| setSelectedDatasetId( | |
| e.target.value ? parseInt(e.target.value) : null, | |
| ) | |
| onChange={(e) => | |
| setSelectedDatasetId( | |
| e.target.value ? parseInt(e.target.value, 10) : null, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/text-to-speech/RunTTSEvaluationForm.tsx` around lines 89 - 92,
The onChange handler in RunTTSEvaluationForm uses parseInt(e.target.value)
without a radix; update that call to include the radix (e.g.,
parseInt(e.target.value, 10)) or replace it with a safer numeric conversion
(e.g., Number(e.target.value) or +e.target.value) before passing the result to
setSelectedDatasetId so the component's onChange uses an unambiguous base when
converting the string to an integer.
Comment on lines
+141
to
+173
| onChange={(e) => { | ||
| const value = e.target.value || null; | ||
| const newScore: TTSScore = { | ||
| ...(result.score || {}), | ||
| "Speech Naturalness": value, | ||
| }; | ||
| onUpdateLocalScore(newScore); | ||
| onCommitFeedback(result.is_correct, undefined, { | ||
| "Speech Naturalness": value, | ||
| }); | ||
| }} | ||
| disabled={!isSuccess} | ||
| className={`w-full px-2 py-1.5 border rounded text-xs font-medium ${snStyles.bg} ${snStyles.border} ${snStyles.text} ${disabledClass}`} | ||
| > | ||
| <option value="">-</option> | ||
| <option value="High">High</option> | ||
| <option value="Medium">Medium</option> | ||
| <option value="Low">Low</option> | ||
| </select> | ||
| </td> | ||
| <td className="px-3 py-3 text-sm align-top"> | ||
| <select | ||
| value={normalizedPa} | ||
| onChange={(e) => { | ||
| const value = e.target.value || null; | ||
| const newScore: TTSScore = { | ||
| ...(result.score || {}), | ||
| "Pronunciation Accuracy": value, | ||
| }; | ||
| onUpdateLocalScore(newScore); | ||
| onCommitFeedback(result.is_correct, undefined, { | ||
| "Pronunciation Accuracy": value, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the updateFeedback / onUpdateFeedback API implementation
rg -n --type=ts "updateFeedback\|onUpdateFeedback\|feedback" app/components/text-to-speech/EvaluationsTab.tsx -A 10 -B 2Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 58
🏁 Script executed:
# Find where onCommitFeedback is defined or used
rg -n "onCommitFeedback" --type=ts --type=tsx -B 3 -A 5Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 104
🏁 Script executed:
# Also find parent components of TTSResultRow to trace where onCommitFeedback comes from
rg -n "TTSResultRow" --type=ts --type=tsx -B 2 -A 5Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 104
🏁 Script executed:
# Search for any feedback-related API calls or endpoints
rg -n "feedback" --type=ts --type=tsx app/ -l | head -20Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 104
🏁 Script executed:
# Find onCommitFeedback definition without type filter
rg -n "onCommitFeedback" app/ -B 3 -A 5Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 9478
🏁 Script executed:
# Find TTSResultRow usage and parent components
rg -n "TTSResultRow" app/ -B 2 -A 5Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 3235
🏁 Script executed:
# Find where scores are committed/updated to API
rg -n "Speech Naturalness|Pronunciation Accuracy" app/ -B 2 -A 8Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 7030
🏁 Script executed:
# Find onUpdateFeedback definition
rg -n "onUpdateFeedback" app/ -B 5 -A 15 | head -100Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 8035
🏁 Script executed:
# Look for the actual API endpoint that handles the score update
rg -n "updateFeedback\|/feedback\|patch.*feedback" app/ -B 3 -A 10 | head -150Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 58
🏁 Script executed:
# Find where onUpdateFeedback is actually implemented (the arrow function/function definition)
rg -n "const onUpdateFeedback|function onUpdateFeedback" app/ -A 20Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 58
🏁 Script executed:
# Look in page/layout files for the evaluation components
fd -n "page\|layout" app/ -t f | grep -i "tts\|evaluation\|text.*speech"Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 307
🏁 Script executed:
# Search for API calls related to feedback or score updates
rg -n "clientFetch.*feedback\|POST.*feedback\|PATCH.*feedback" app/ -B 2 -A 5Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 58
🏁 Script executed:
# Fix fd syntax - search for page or layout files
fd "page|layout" app -type f | grep -i "tts\|text-to-speech\|evaluation"Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 247
🏁 Script executed:
# Look at TTSResultsTable.tsx more carefully for the full implementation
cat app/components/text-to-speech/TTSResultsTable.tsxRepository: ProjectTech4DevAI/kaapi-frontend
Length of output: 5982
🏁 Script executed:
# Search for any API calls that might update feedback/results
rg -n "apiClient\|clientFetch" app/components/text-to-speech/ -A 3Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 58
🏁 Script executed:
# Find where TTSRunsList is imported/used
rg -n "import.*TTSRunsList\|from.*TTSRunsList" app/ -B 2 -A 5Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 58
🏁 Script executed:
# Look for page files in app directory
find app -name "page.tsx" -o -name "page.ts" | grep -i "tts\|text\|evaluation"Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 298
🏁 Script executed:
# Search more broadly for updateFeedback with full context
rg -n "updateFeedback.*=" app/ -B 2 -A 25 | head -150Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 4504
🏁 Script executed:
# Check TTSFeedbackPayload type definition
rg -n "TTSFeedbackPayload" app/ -B 3 -A 5Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 2067
Pass the full merged score object to onCommitFeedback to prevent silent data loss.
onUpdateLocalScore correctly spreads the full existing score before patching ({ ...result.score, "Speech Naturalness": value }), keeping local state consistent. However, onCommitFeedback sends only the changed field to the API:
onCommitFeedback(result.is_correct, undefined, { "Speech Naturalness": value });
The API endpoint in EvaluationsTab.tsx (line 84) passes the score parameter directly to the PATCH payload: payload.score = score. If the backend replaces the entire score object rather than merging it, sequential updates will silently discard the previous field. The local state will appear correct until the next page load, at which point both fields won't be persisted.
Pass the full merged score to onCommitFeedback:
🛠️ Proposed fix — Speech Naturalness handler (same change applies to Pronunciation Accuracy)
onChange={(e) => {
const value = e.target.value || null;
const newScore: TTSScore = {
...(result.score || {}),
"Speech Naturalness": value,
};
onUpdateLocalScore(newScore);
- onCommitFeedback(result.is_correct, undefined, {
- "Speech Naturalness": value,
- });
+ onCommitFeedback(result.is_correct, undefined, newScore);
}}📝 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
| onChange={(e) => { | |
| const value = e.target.value || null; | |
| const newScore: TTSScore = { | |
| ...(result.score || {}), | |
| "Speech Naturalness": value, | |
| }; | |
| onUpdateLocalScore(newScore); | |
| onCommitFeedback(result.is_correct, undefined, { | |
| "Speech Naturalness": value, | |
| }); | |
| }} | |
| disabled={!isSuccess} | |
| className={`w-full px-2 py-1.5 border rounded text-xs font-medium ${snStyles.bg} ${snStyles.border} ${snStyles.text} ${disabledClass}`} | |
| > | |
| <option value="">-</option> | |
| <option value="High">High</option> | |
| <option value="Medium">Medium</option> | |
| <option value="Low">Low</option> | |
| </select> | |
| </td> | |
| <td className="px-3 py-3 text-sm align-top"> | |
| <select | |
| value={normalizedPa} | |
| onChange={(e) => { | |
| const value = e.target.value || null; | |
| const newScore: TTSScore = { | |
| ...(result.score || {}), | |
| "Pronunciation Accuracy": value, | |
| }; | |
| onUpdateLocalScore(newScore); | |
| onCommitFeedback(result.is_correct, undefined, { | |
| "Pronunciation Accuracy": value, | |
| }); | |
| onChange={(e) => { | |
| const value = e.target.value || null; | |
| const newScore: TTSScore = { | |
| ...(result.score || {}), | |
| "Speech Naturalness": value, | |
| }; | |
| onUpdateLocalScore(newScore); | |
| onCommitFeedback(result.is_correct, undefined, newScore); | |
| }} | |
| disabled={!isSuccess} | |
| className={`w-full px-2 py-1.5 border rounded text-xs font-medium ${snStyles.bg} ${snStyles.border} ${snStyles.text} ${disabledClass}`} | |
| > | |
| <option value="">-</option> | |
| <option value="High">High</option> | |
| <option value="Medium">Medium</option> | |
| <option value="Low">Low</option> | |
| </select> | |
| </td> | |
| <td className="px-3 py-3 text-sm align-top"> | |
| <select | |
| value={normalizedPa} | |
| onChange={(e) => { | |
| const value = e.target.value || null; | |
| const newScore: TTSScore = { | |
| ...(result.score || {}), | |
| "Pronunciation Accuracy": value, | |
| }; | |
| onUpdateLocalScore(newScore); | |
| onCommitFeedback(result.is_correct, undefined, { | |
| "Pronunciation Accuracy": value, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/text-to-speech/TTSResultRow.tsx` around lines 141 - 173, The
handlers currently call onCommitFeedback with only the changed field (e.g.,
onCommitFeedback(result.is_correct, undefined, { "Speech Naturalness": value }))
which can cause the backend to overwrite the whole score and lose other fields;
update both select handlers (the one that constructs newScore for "Speech
Naturalness" and the one for "Pronunciation Accuracy") to compute the merged
TTSScore (const newScore = { ...(result.score || {}), "Speech Naturalness":
value } or "Pronunciation Accuracy": value) and pass that full newScore into
onCommitFeedback instead of the single-field object, while still calling
onUpdateLocalScore(newScore).
Comment on lines
+15
to
+29
| const handleDownload = () => { | ||
| const csvLines = [data.headers.join(",")]; | ||
| data.rows.forEach((row) => { | ||
| csvLines.push( | ||
| row | ||
| .map((cell) => | ||
| cell.includes(",") || cell.includes('"') || cell.includes("\n") | ||
| ? `"${cell.replace(/"/g, '""')}"` | ||
| : cell, | ||
| ) | ||
| .join(","), | ||
| ); | ||
| }); | ||
| const blob = new Blob([csvLines.join("\n")], { | ||
| type: "text/csv;charset=utf-8;", |
There was a problem hiding this comment.
Escape header cells before building the CSV.
data.headers.join(",") writes raw header text, so a header containing a comma, quote, or newline will corrupt the downloaded file even though row cells are escaped correctly. Reuse the same escaping function for both headers and rows.
Suggested fix
const handleDownload = () => {
- const csvLines = [data.headers.join(",")];
+ const escapeCell = (cell: string) =>
+ cell.includes(",") || cell.includes('"') || cell.includes("\n")
+ ? `"${cell.replace(/"/g, '""')}"`
+ : cell;
+
+ const csvLines = [data.headers.map(escapeCell).join(",")];
data.rows.forEach((row) => {
- csvLines.push(
- row
- .map((cell) =>
- cell.includes(",") || cell.includes('"') || cell.includes("\n")
- ? `"${cell.replace(/"/g, '""')}"`
- : cell,
- )
- .join(","),
- );
+ csvLines.push(row.map(escapeCell).join(","));
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleDownload = () => { | |
| const csvLines = [data.headers.join(",")]; | |
| data.rows.forEach((row) => { | |
| csvLines.push( | |
| row | |
| .map((cell) => | |
| cell.includes(",") || cell.includes('"') || cell.includes("\n") | |
| ? `"${cell.replace(/"/g, '""')}"` | |
| : cell, | |
| ) | |
| .join(","), | |
| ); | |
| }); | |
| const blob = new Blob([csvLines.join("\n")], { | |
| type: "text/csv;charset=utf-8;", | |
| const handleDownload = () => { | |
| const escapeCell = (cell: string) => | |
| cell.includes(",") || cell.includes('"') || cell.includes("\n") | |
| ? `"${cell.replace(/"/g, '""')}"` | |
| : cell; | |
| const csvLines = [data.headers.map(escapeCell).join(",")]; | |
| data.rows.forEach((row) => { | |
| csvLines.push(row.map(escapeCell).join(",")); | |
| }); | |
| const blob = new Blob([csvLines.join("\n")], { | |
| type: "text/csv;charset=utf-8;", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/text-to-speech/TTSViewDatasetModal.tsx` around lines 15 - 29,
The CSV header line is built by raw join (data.headers.join(",")) which fails to
escape commas/quotes/newlines; update the handleDownload function to reuse the
same escaping logic used for rows: create a helper (e.g., escapeCsvCell) or
inline the same map/replace logic and apply it to each header cell before
joining, so headers are wrapped/quoted and quotes escaped exactly like row cells
when building csvLines.
| job: EvalJob, | ||
| traces: GroupedTraceItem[], | ||
| ): number => { | ||
| const maxAnswers = Math.max(...traces.map((g) => g.llm_answers.length)); |
There was a problem hiding this comment.
Math.max(...[]) returns -Infinity when traces is empty.
If exportGroupedCSV is called with an empty traces array, maxAnswers becomes -Infinity, the header loop doesn't run, and downloadCSV is invoked with only "Question ID,Question,Ground Truth\n" — an effectively empty file with no indication of the problem.
🛠️ Proposed fix
export const exportGroupedCSV = (
job: EvalJob,
traces: GroupedTraceItem[],
): number => {
+ if (traces.length === 0) {
+ throw new Error("No grouped trace data available to export");
+ }
const maxAnswers = Math.max(...traces.map((g) => g.llm_answers.length));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const maxAnswers = Math.max(...traces.map((g) => g.llm_answers.length)); | |
| export const exportGroupedCSV = ( | |
| job: EvalJob, | |
| traces: GroupedTraceItem[], | |
| ): number => { | |
| if (traces.length === 0) { | |
| throw new Error("No grouped trace data available to export"); | |
| } | |
| const maxAnswers = Math.max(...traces.map((g) => g.llm_answers.length)); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/lib/utils/evaluationExport.ts` at line 28, The code computing maxAnswers
in exportGroupedCSV uses Math.max(...traces.map(...)) which returns -Infinity
for empty traces; fix by ensuring a safe default and a clear output when traces
is empty: set maxAnswers = Math.max(0, ...traces.map(g => g.llm_answers.length))
(or use traces.length ? Math.max(...) : 0) and if traces.length === 0,
early-call downloadCSV with the header plus a single row like "No traces
available" (or otherwise return early) so users aren't given a mysterious empty
file; update references in exportGroupedCSV where maxAnswers and downloadCSV are
used.
Comment on lines
+30
to
+36
| let csvContent = "Question ID,Question,Ground Truth"; | ||
| for (let i = 1; i <= maxAnswers; i++) { | ||
| csvContent += `,LLM Answer ${i},Trace ID ${i}`; | ||
| scoreNames.forEach((name) => { | ||
| csvContent += `,${name} (${i}),${sanitizeCSVCell(`${name} (${i}) Comment`)}`; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Score names written directly into CSV headers without sanitization — could break column alignment.
Both exportGroupedCSV (line 33: ${name} (${i})) and exportRowCSV (line 82: `${name},${name} (comment)`) interpolate score names directly into the CSV header string. A score name containing a comma or double-quote would malform the header row, silently misaligning every data column for the affected score.
The comment column label is already passed through sanitizeCSVCell, but the value column label is not.
🛠️ Proposed fix for `exportGroupedCSV`
for (let i = 1; i <= maxAnswers; i++) {
csvContent += `,LLM Answer ${i},Trace ID ${i}`;
scoreNames.forEach((name) => {
- csvContent += `,${name} (${i}),${sanitizeCSVCell(`${name} (${i}) Comment`)}`;
+ csvContent += `,${sanitizeCSVCell(`${name} (${i})`)},${sanitizeCSVCell(`${name} (${i}) Comment`)}`;
});
}🛠️ Proposed fix for `exportRowCSV`
- csvContent +=
- scoreNames.map((name) => `${name},${name} (comment)`).join(",") + "\n";
+ csvContent +=
+ scoreNames.map((name) => `${sanitizeCSVCell(name)},${sanitizeCSVCell(`${name} (comment)`)}`).join(",") + "\n";Also applies to: 78-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/lib/utils/evaluationExport.ts` around lines 30 - 36, The CSV header
writes raw score names into CSV columns which can break alignment if a name
contains commas or quotes; update exportGroupedCSV (the loop that builds
csvContent with `LLM Answer ${i},Trace ID ${i}` and the `${name} (${i})`
entries) to pass every header label through sanitizeCSVCell (including the value
column label and the comment label), and likewise update exportRowCSV where
headers are built (`${name},${name} (comment)`) to sanitize both the score name
and the comment label using sanitizeCSVCell so all header cells are correctly
escaped.
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
2 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.
Issue: #140
Summary:
Summary by CodeRabbit
Release Notes
New Features
Improvements
UI/UX