Conversation
📝 WalkthroughWalkthroughRefactors many API route handlers to use a centralized Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant NextRoute as "Next API Route"
participant ApiClient as "apiClient / apiFetch"
participant Backend as "Backend API"
Client->>NextRoute: HTTP request (includes cookies/headers)
NextRoute->>ApiClient: apiClient(request, endpoint, options)
ApiClient->>Backend: Forwarded request (backend URL, headers, body)
Backend-->>ApiClient: { status, data } / signed URL or CSV
ApiClient-->>NextRoute: { status, data }
NextRoute-->>Client: NextResponse.json(data, { status })
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…jectTech4DevAI/kaapi-frontend into feat/refactor-routes-phase-2
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/evaluations/tts/datasets/`[dataset_id]/route.ts:
- Around line 27-47: The handler currently dereferences signed_url even when the
upstream fetch failed; update the fetch_content branch to short-circuit on the
upstream response status before reading data.data.signed_url (or
data.signed_url): if the upstream response is non-2xx, return a NextResponse
with the upstream status and error/body instead of proceeding to check
signed_url; only after confirming a successful upstream response should you
attempt to read signed_url and fetch the CSV (refer to variables fetchContent,
data, signed_url, and NextResponse.json to implement this).
- Around line 66-71: The delete handler currently always calls NextResponse.json
even when apiClient returns status 204 and data is null; change the logic in the
route (the block that calls
apiClient(`/api/v1/evaluations/tts/datasets/${dataset_id}`, { method: "DELETE"
})) to check if status === 204 and, if so, return a bare NextResponse with no
JSON body (e.g., new NextResponse(null, { status })) instead of
NextResponse.json; otherwise keep returning NextResponse.json(data, { status }).
In `@app/api/evaluations/tts/runs/`[run_id]/route.ts:
- Around line 12-14: The endpoint string concatenation uses the raw run_id which
can contain reserved characters; change the construction of endpoint in route.ts
to use encodeURIComponent(run_id) (e.g., encodeURIComponent(run_id)) when
building the path so the upstream URL cannot be rewritten by dot-segments or
reserved chars; apply the same encoding safeguard to other dynamic ID routes in
this PR that build URLs from params.
In `@app/hooks/useConfigs.ts`:
- Around line 103-164: Fast-path cache and pendingFetch are not scoped to the
request inputs (apiKey, pageSize), so cached inMemory/localStorage entries and
configState.pendingFetch can be reused across different apiKey/pageSize
combinations; namespace the cache and pending promise by those inputs or clear
them on change. Concretely: change configState.inMemoryCache and the object
persisted by loadCache() to include apiKey and pageSize (e.g., { apiKey,
pageSize, partialFetch, configs, ... }) and only reuse when cached.apiKey ===
current apiKey && cached.pageSize === current pageSize; similarly attach the
same key to configState.pendingFetch (or store pendingFetch map keyed by
`${apiKey}|${pageSize}`) and ensure you clear/replace pendingFetch and
inMemoryCache when apiKey or pageSize changes; apply the same scoping for the
localStorage usage and for scheduleBackgroundValidation usage (see the
fast-paths around configState.inMemoryCache and loadCache() and the subsequent
block at ~169-205).
- Around line 92-93: The memoized callbacks in useConfigs.ts (fetchConfigs,
loadVersionsForConfig, loadSingleVersion, loadMoreConfigs) close over auth and
can capture a null apiKey; update their dependency arrays to include apiKey (or
activeKey where appropriate) so they re-create when auth hydrates: set
fetchConfigs deps to [apiKey, pageSize], loadVersionsForConfig to [apiKey],
loadSingleVersion to [apiKey, configs], and loadMoreConfigs to [apiKey, configs,
pageSize] (or substitute activeKey consistently) to prevent stale closures.
- Around line 97-100: useConfigs is surfacing "No API key found" prematurely
because useAuth() initializes activeKey=null and hydration from localStorage
happens asynchronously; add an isHydrated (or isReady) boolean to AuthContext
that flips true after the mount effect finishes reading localStorage, expose it
from useAuth(), then update fetchConfigs (and its useEffect) to wait for
isHydrated before checking apiKey and setting the error—i.e., only run the
!apiKey branch when isHydrated is true (or when apiKey is explicitly null after
hydration) so the initial null does not trigger a false error.
🪄 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: 00940a48-7ae8-4498-9e52-c1e7efd30849
📒 Files selected for processing (13)
app/api/credentials/[provider]/route.tsapp/api/credentials/route.tsapp/api/evaluations/stt/files/route.tsapp/api/evaluations/tts/datasets/[dataset_id]/route.tsapp/api/evaluations/tts/datasets/route.tsapp/api/evaluations/tts/results/[result_id]/route.tsapp/api/evaluations/tts/runs/[run_id]/route.tsapp/api/evaluations/tts/runs/route.tsapp/api/languages/route.tsapp/hooks/useConfigs.tsapp/lib/apiClient.tsapp/speech-to-text/page.tsxapp/text-to-speech/page.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/lib/context/AuthContext.tsx (1)
32-36:⚠️ Potential issue | 🟡 MinorValidate parsed localStorage payload before
setApiKeys.
JSON.parsecan return non-array JSON, which would later break array operations onapiKeys. Guard the shape before updating state.Suggested fix
useEffect(() => { try { const stored = localStorage.getItem(STORAGE_KEY); // eslint-disable-next-line react-hooks/set-state-in-effect - if (stored) setApiKeys(JSON.parse(stored)); + if (stored) { + const parsed = JSON.parse(stored); + if (Array.isArray(parsed)) { + setApiKeys(parsed as APIKey[]); + } else { + localStorage.removeItem(STORAGE_KEY); + } + } } catch { /* ignore malformed data */ } setIsHydrated(true); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/context/AuthContext.tsx` around lines 32 - 36, The code reads localStorage.getItem(STORAGE_KEY) and JSON.parse(...) then unconditionally calls setApiKeys, but JSON.parse may return non-array values; update the try block in AuthContext (where STORAGE_KEY and setApiKeys are used) to parse the stored value, verify it's an array (and optionally validate expected item shape), and only call setApiKeys(parsed) when Array.isArray(parsed) (and items match the expected shape), otherwise ignore or clear the stored value to avoid breaking apiKeys consumers.
♻️ Duplicate comments (2)
app/hooks/useConfigs.ts (2)
106-176:⚠️ Potential issue | 🟠 MajorCache and in-flight fetch state are still not scoped by request shape.
configState.inMemoryCache,configState.pendingFetch, andconfigState.pendingLoadMoreare reused globally without{apiKey, pageSize}identity, so a key/page-size switch can reuse stale data or pending work from a different request context.Suggested direction
+const requestScope = `${apiKey}|${pageSize ?? "all"}`; - if (configState.inMemoryCache) { ...reuse... } + if (configState.inMemoryCache?.scope === requestScope) { ...reuse... } - configState.pendingFetch = (async () => { ... })() + configState.pendingFetchByScope[requestScope] = (async () => { ... })() - saveCache(newCache); + saveCache({ ...newCache, scope: requestScope });Also apply the same scope check to localStorage cache restore and
pendingLoadMore.Also applies to: 180-226, 409-433
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/hooks/useConfigs.ts` around lines 106 - 176, The cache/pending state (configState.inMemoryCache, configState.pendingFetch, configState.pendingLoadMore) is global and not keyed by request identity, causing cross-request reuse; update the code to namespace these entries by a request key derived from the request shape (e.g., `${apiKey}|pageSize=${pageSize}`) and use that key whenever reading/writing the in-memory cache, restoring localStorage (loadCache), scheduling background validation (scheduleBackgroundValidation), and checking/setting pendingFetch/pendingLoadMore; ensure the same keying is applied where localStorage is restored and where pendingLoadMore is used so loadMore operations and cached results are only reused for the matching {apiKey,pageSize} request context.
304-382:⚠️ Potential issue | 🟠 MajorAdd missing auth dependencies to memoized callbacks.
loadSingleVersionusesapiKey(Line 315) but excludes it from deps, andloadMoreConfigsreads auth-derived key (Line 391) without auth deps. These can hold stale credentials after hydration/key switches.Suggested fix
- const loadSingleVersion = useCallback( + const loadSingleVersion = useCallback( async (config_id: string, version: number): Promise<SavedConfig | null> => { ... }, - [configs], + [configs, apiKey], ); const loadMoreConfigs = useCallback(async () => { if (!configState.allConfigMeta || configState.allConfigMeta.length === 0) return; - const apiKey = activeKey?.key; if (!apiKey) return; ... - }, [configs, pageSize]); + }, [configs, pageSize, apiKey]);Also applies to: 388-450
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/hooks/useConfigs.ts` around lines 304 - 382, The memoized callback loadSingleVersion currently closes over apiKey (and other auth-derived values) but doesn't list them in its dependency array, which can lead to stale credentials after hydration or key changes; update the useCallback dependency array for loadSingleVersion to include apiKey and any other auth-derived values it reads, and likewise update loadMoreConfigs and any other memoized functions that read the auth-derived key to include that key (e.g., apiKey or auth.token) and related auth/context variables in their deps so the callbacks are recreated when credentials change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@app/lib/context/AuthContext.tsx`:
- Around line 32-36: The code reads localStorage.getItem(STORAGE_KEY) and
JSON.parse(...) then unconditionally calls setApiKeys, but JSON.parse may return
non-array values; update the try block in AuthContext (where STORAGE_KEY and
setApiKeys are used) to parse the stored value, verify it's an array (and
optionally validate expected item shape), and only call setApiKeys(parsed) when
Array.isArray(parsed) (and items match the expected shape), otherwise ignore or
clear the stored value to avoid breaking apiKeys consumers.
---
Duplicate comments:
In `@app/hooks/useConfigs.ts`:
- Around line 106-176: The cache/pending state (configState.inMemoryCache,
configState.pendingFetch, configState.pendingLoadMore) is global and not keyed
by request identity, causing cross-request reuse; update the code to namespace
these entries by a request key derived from the request shape (e.g.,
`${apiKey}|pageSize=${pageSize}`) and use that key whenever reading/writing the
in-memory cache, restoring localStorage (loadCache), scheduling background
validation (scheduleBackgroundValidation), and checking/setting
pendingFetch/pendingLoadMore; ensure the same keying is applied where
localStorage is restored and where pendingLoadMore is used so loadMore
operations and cached results are only reused for the matching {apiKey,pageSize}
request context.
- Around line 304-382: The memoized callback loadSingleVersion currently closes
over apiKey (and other auth-derived values) but doesn't list them in its
dependency array, which can lead to stale credentials after hydration or key
changes; update the useCallback dependency array for loadSingleVersion to
include apiKey and any other auth-derived values it reads, and likewise update
loadMoreConfigs and any other memoized functions that read the auth-derived key
to include that key (e.g., apiKey or auth.token) and related auth/context
variables in their deps so the callbacks are recreated when credentials change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 32af3b65-7588-42a9-9a02-008e6899ff77
📒 Files selected for processing (2)
app/hooks/useConfigs.tsapp/lib/context/AuthContext.tsx
Isssue: #71
Summary:
apiClientfunction in theroute.tsfile.AuthContext.tsfile, and fix the hydration issues.Summary by CodeRabbit
Refactor
Bug Fixes
UX / Data Loading