Merged
Conversation
📝 WalkthroughWalkthroughThis pull request refactors the application's state management by introducing two new React context providers ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/document/page.tsx (1)
33-45:⚠️ Potential issue | 🟠 MajorClear document state on API-key loss/switch.
With context-driven keys, previous documents/preview can remain visible after key removal or key change. Reset local document state before/when key becomes unavailable.
💡 Suggested fix
useEffect(() => { - if (apiKey) { - fetchDocuments(); - } + setCurrentPage(1); + setSelectedDocument(null); + setDocuments([]); + + if (!apiKey) { + setError('No API key found. Please add an API key in the Keystore.'); + return; + } + + fetchDocuments(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [apiKey]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/document/page.tsx` around lines 33 - 45, The page currently leaves previous documents/preview visible when the apiKey is removed or changed; update the useEffect that watches apiKey so that when apiKey is falsy or changes you clear local state first (call setDocuments([]), setPreviewDocument(null) and reset setCurrentPage(1)), and when apiKey becomes truthy call fetchDocuments() (for a key switch, clear state then fetch). Locate the useEffect and the state variables (apiKey from useAuth, fetchDocuments, setDocuments, setPreviewDocument, setCurrentPage) and add the else/branch or conditional handling to perform the resets before/when fetching.app/lib/useConfigs.ts (1)
195-209:⚠️ Potential issue | 🔴 CriticalScope cache by active key to prevent cross-key data bleed.
After switching
activeKey, the hook can still return cached configs from the previous key (sharedinMemoryCache/CACHE_KEY) before refetch. This risks showing another key’s data in the current session.💡 Suggested fix (invalidate cache on key switch)
export function useConfigs(): UseConfigsResult { const { activeKey } = useAuth(); + const lastKeyIdRef = useRef<string | null>(null); const [configs, setConfigs] = useState<SavedConfig[]>([]); @@ const fetchConfigs = useCallback(async (force: boolean = false) => { // Prevent concurrent fetches if (fetchInProgress.current) return; + const currentKeyId = activeKey?.id ?? null; + if (lastKeyIdRef.current !== currentKeyId) { + inMemoryCache = null; + if (typeof window !== 'undefined') { + localStorage.removeItem(CACHE_KEY); + } + lastKeyIdRef.current = currentKeyId; + } + const apiKey = activeKey?.key ?? null;Also applies to: 347-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/useConfigs.ts` around lines 195 - 209, The cached configs are shared across different active keys causing cross-key data bleed; update the cache logic used by useConfigs by scoping the inMemoryCache/CACHE_KEY to the activeKey (or by clearing the cached entry whenever activeKey changes) so fetchConfigs and any cache reads/writes use a unique key per apiKey (reference useAuth.activeKey, useConfigs.fetchConfigs, inMemoryCache and CACHE_KEY) and ensure the effect that reacts to activeKey invalidates or refetches (and resets isCached) to avoid showing another key’s data.
🧹 Nitpick comments (3)
app/keystore/page.tsx (1)
22-22: RedundantSTORAGE_KEYexport.This constant is now defined in
AuthContext.tsx. Consider removing this export to avoid duplication and potential drift. If other modules depend on it, they should import from the canonical source or the constant should be moved to a shared utility file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/keystore/page.tsx` at line 22, Remove the redundant STORAGE_KEY export from page.tsx and update callers to import the canonical constant from AuthContext.tsx (or move STORAGE_KEY into a shared utility and import from there); locate STORAGE_KEY in app/keystore/page.tsx and replace any local references or imports to use the single source of truth defined in AuthContext.tsx (or the new shared file) to avoid duplication and potential drift.app/lib/context/AppContext.tsx (1)
13-22: Sidebar collapsed state is not persisted to localStorage.The context manages
sidebarCollapsedstate but doesn't persist it. Users will lose their sidebar preference on page refresh.Based on learnings, sidebar state should be persisted using the
sidebar-expanded-menuslocalStorage key.♻️ Proposed fix to add localStorage persistence
+const SIDEBAR_STORAGE_KEY = 'sidebar-expanded-menus'; + export function AppProvider({ children }: { children: React.ReactNode }) { - const [sidebarCollapsed, setSidebarCollapsed] = useState(false); + const [sidebarCollapsed, setSidebarCollapsed] = useState(() => { + if (typeof window === 'undefined') return false; + try { + const stored = localStorage.getItem(SIDEBAR_STORAGE_KEY); + return stored ? JSON.parse(stored) : false; + } catch { + return false; + } + }); + + const updateSidebarCollapsed = (collapsed: boolean) => { + setSidebarCollapsed(collapsed); + localStorage.setItem(SIDEBAR_STORAGE_KEY, JSON.stringify(collapsed)); + }; + - const toggleSidebar = () => setSidebarCollapsed(prev => !prev); + const toggleSidebar = () => { + setSidebarCollapsed(prev => { + const newValue = !prev; + localStorage.setItem(SIDEBAR_STORAGE_KEY, JSON.stringify(newValue)); + return newValue; + }); + }; return ( - <AppContext.Provider value={{ sidebarCollapsed, setSidebarCollapsed, toggleSidebar }}> + <AppContext.Provider value={{ sidebarCollapsed, setSidebarCollapsed: updateSidebarCollapsed, toggleSidebar }}> {children} </AppContext.Provider> ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/lib/context/AppContext.tsx` around lines 13 - 22, The AppProvider currently holds sidebarCollapsed but doesn't persist it; update AppProvider to read the initial boolean from localStorage key "sidebar-expanded-menus" (fallback to false) when initializing sidebarCollapsed, and whenever setSidebarCollapsed or toggleSidebar changes the value also write the updated boolean back to that same localStorage key; reference the AppProvider function and the state setters sidebarCollapsed, setSidebarCollapsed and toggleSidebar so the initialization and update logic are added there (use JSON.stringify/JSON.parse or string "true"/"false" consistently).app/configurations/prompt-editor/page.tsx (1)
212-220: Dependency array has redundant entries.The dependencies
provider,temperature, andtoolsat lines 216-218 are not directly referenced in the effect body. Changes to these values would already be reflected throughcurrentConfigBlob(line 215), which is already a dependency. These extra entries are harmless but add noise.♻️ Optional: Remove redundant dependencies
}, [ selectedConfigId, currentContent, currentConfigBlob, - provider, - temperature, - tools, savedConfigs, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/configurations/prompt-editor/page.tsx` around lines 212 - 220, The effect dependency array contains redundant entries: remove provider, temperature, and tools from the dependencies in the useEffect that currently lists selectedConfigId, currentContent, currentConfigBlob, provider, temperature, tools, savedConfigs; those values are already represented via currentConfigBlob (and the other listed deps) so update the dependency array to only include selectedConfigId, currentContent, currentConfigBlob, and savedConfigs to eliminate noise while preserving correct reactivity for the effect.
🤖 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/configurations/page.tsx`:
- Around line 35-39: The handler that loads evaluations (the block using
activeKey and apiFetch in the component) never clears evaluationCounts when
activeKey is missing or changes; update the early return for if (!activeKey) to
reset evaluationCounts (e.g., call the state setter used for evaluationCounts)
before returning, and also reset evaluationCounts at the start of the fetch path
when activeKey changes so stale counts are cleared; reference the
evaluationCounts state and the function that fetches evaluations (the code
around apiFetch('/api/evaluations', activeKey.key) and the jobs variable) and
ensure the same reset is applied in the other similar load path noted in the
comment.
In `@app/evaluations/`[id]/page.tsx:
- Around line 68-73: The useEffect in page.tsx that initializes selectedKeyId
(watching [apiKeys, selectedKeyId]) doesn't handle removed/rotated keys, leaving
selectedKeyId stale; update that effect to also check whether the current
selectedKeyId exists in apiKeys and if not setSelectedKeyId to a valid fallback
(e.g., apiKeys[0].id or null/undefined when apiKeys is empty) so fetches resume;
reference the useEffect block, the selectedKeyId state, apiKeys array, and
setSelectedKeyId updater when implementing this change.
In `@app/lib/context/AuthContext.tsx`:
- Around line 37-38: The addKey/removeKey callbacks close over the rendered
apiKeys value and can overwrite concurrent updates; change them to use
functional updates via persist so they read the latest state (e.g. in addKey
call persist(prev => [...prev, key]) and in removeKey call persist(prev =>
prev.filter(k => k.id !== id))). Also update the useCallback dependencies to
remove apiKeys (keep persist) so addKey and removeKey do not capture stale
apiKeys.
- Around line 19-26: Replace the client-only localStorage access inside the
useState initializer to avoid SSR hydration mismatches: initialize apiKeys with
useState<APIKey[]>([]) and move the localStorage read into a useEffect that runs
on mount; inside that effect, try to read localStorage.getItem(STORAGE_KEY) and
if present JSON.parse it and call setApiKeys(...), catching errors for malformed
data—update the AuthContext.tsx code that currently defines apiKeys and
setApiKeys to use this pattern.
---
Outside diff comments:
In `@app/document/page.tsx`:
- Around line 33-45: The page currently leaves previous documents/preview
visible when the apiKey is removed or changed; update the useEffect that watches
apiKey so that when apiKey is falsy or changes you clear local state first (call
setDocuments([]), setPreviewDocument(null) and reset setCurrentPage(1)), and
when apiKey becomes truthy call fetchDocuments() (for a key switch, clear state
then fetch). Locate the useEffect and the state variables (apiKey from useAuth,
fetchDocuments, setDocuments, setPreviewDocument, setCurrentPage) and add the
else/branch or conditional handling to perform the resets before/when fetching.
In `@app/lib/useConfigs.ts`:
- Around line 195-209: The cached configs are shared across different active
keys causing cross-key data bleed; update the cache logic used by useConfigs by
scoping the inMemoryCache/CACHE_KEY to the activeKey (or by clearing the cached
entry whenever activeKey changes) so fetchConfigs and any cache reads/writes use
a unique key per apiKey (reference useAuth.activeKey, useConfigs.fetchConfigs,
inMemoryCache and CACHE_KEY) and ensure the effect that reacts to activeKey
invalidates or refetches (and resets isCached) to avoid showing another key’s
data.
---
Nitpick comments:
In `@app/configurations/prompt-editor/page.tsx`:
- Around line 212-220: The effect dependency array contains redundant entries:
remove provider, temperature, and tools from the dependencies in the useEffect
that currently lists selectedConfigId, currentContent, currentConfigBlob,
provider, temperature, tools, savedConfigs; those values are already represented
via currentConfigBlob (and the other listed deps) so update the dependency array
to only include selectedConfigId, currentContent, currentConfigBlob, and
savedConfigs to eliminate noise while preserving correct reactivity for the
effect.
In `@app/keystore/page.tsx`:
- Line 22: Remove the redundant STORAGE_KEY export from page.tsx and update
callers to import the canonical constant from AuthContext.tsx (or move
STORAGE_KEY into a shared utility and import from there); locate STORAGE_KEY in
app/keystore/page.tsx and replace any local references or imports to use the
single source of truth defined in AuthContext.tsx (or the new shared file) to
avoid duplication and potential drift.
In `@app/lib/context/AppContext.tsx`:
- Around line 13-22: The AppProvider currently holds sidebarCollapsed but
doesn't persist it; update AppProvider to read the initial boolean from
localStorage key "sidebar-expanded-menus" (fallback to false) when initializing
sidebarCollapsed, and whenever setSidebarCollapsed or toggleSidebar changes the
value also write the updated boolean back to that same localStorage key;
reference the AppProvider function and the state setters sidebarCollapsed,
setSidebarCollapsed and toggleSidebar so the initialization and update logic are
added there (use JSON.stringify/JSON.parse or string "true"/"false"
consistently).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d1884eb9-a451-46e8-a640-9c8d89834a58
📒 Files selected for processing (17)
app/components/ConfigModal.tsxapp/configurations/page.tsxapp/configurations/prompt-editor/page.tsxapp/datasets/page.tsxapp/document/page.tsxapp/evaluations/[id]/page.tsxapp/evaluations/page.tsxapp/keystore/page.tsxapp/knowledge-base/page.tsxapp/layout.tsxapp/lib/configTypes.tsapp/lib/context/AppContext.tsxapp/lib/context/AuthContext.tsxapp/lib/useConfigs.tsapp/settings/credentials/page.tsxapp/speech-to-text/page.tsxapp/text-to-speech/page.tsx
💤 Files with no reviewable changes (1)
- app/lib/configTypes.ts
There was a problem hiding this comment.
♻️ Duplicate comments (1)
app/lib/context/AuthContext.tsx (1)
48-55:⚠️ Potential issue | 🟠 MajorStale closure bug in
addKeyandremoveKeystill present.These callbacks capture
apiKeysat render time. If called in rapid succession before re-render, later calls operate on stale state and overwrite previous updates, causing data loss.Use functional updates to access the latest state:
🐛 Proposed fix using functional setState
- const addKey = useCallback( - (key: APIKey) => persist([...apiKeys, key]), - [apiKeys, persist], - ); - const removeKey = useCallback( - (id: string) => persist(apiKeys.filter((k) => k.id !== id)), - [apiKeys, persist], - ); + const addKey = useCallback((key: APIKey) => { + setApiKeys((prev) => { + const updated = [...prev, key]; + localStorage.setItem(STORAGE_KEY, JSON.stringify(updated)); + return updated; + }); + }, []); + + const removeKey = useCallback((id: string) => { + setApiKeys((prev) => { + const updated = prev.filter((k) => k.id !== id); + if (updated.length > 0) { + localStorage.setItem(STORAGE_KEY, JSON.stringify(updated)); + } else { + localStorage.removeItem(STORAGE_KEY); + } + return updated; + }); + }, []);🤖 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 48 - 55, addKey and removeKey capture apiKeys in their closure and can overwrite concurrent updates; change them to use functional updates so they always operate on the latest state by calling persist with an updater function (i.e., replace persist([...apiKeys, key]) with persist(prev => [...prev, key]) in addKey and replace persist(apiKeys.filter(...)) with persist(prev => prev.filter(k => k.id !== id)) in removeKey), keeping the same useCallback wrappers referencing addKey, removeKey, apiKeys, and persist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@app/lib/context/AuthContext.tsx`:
- Around line 48-55: addKey and removeKey capture apiKeys in their closure and
can overwrite concurrent updates; change them to use functional updates so they
always operate on the latest state by calling persist with an updater function
(i.e., replace persist([...apiKeys, key]) with persist(prev => [...prev, key])
in addKey and replace persist(apiKeys.filter(...)) with persist(prev =>
prev.filter(k => k.id !== id)) in removeKey), keeping the same useCallback
wrappers referencing addKey, removeKey, apiKeys, and persist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4e09e51e-2a35-48f6-9bb4-17e2f8e5b7ff
📒 Files selected for processing (1)
app/lib/context/AuthContext.tsx
vprashrex
approved these changes
Mar 25, 2026
…ntend into feat/api-management-centralize-storage
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/utils.ts`:
- Around line 13-18: The current logic that appends "Z" to date strings only
checks for "+" and therefore misses negative timezone offsets; update the check
in the function (the one that constructs `date` from `dateStr`—same area as
`formatRelativeTime`) to detect either "+" or "-" offsets (e.g. check for a sign
after the time separator or use a regex that matches timezone offsets like
/[+-]\d{2}:\d{2}$/) so strings like "2024-01-15T10:30:00-05:00" are recognized
as having an offset and do not get "Z" appended before calling `new Date()` and
`formatDistanceToNow`.
🪄 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: 76b8b98e-dfc3-4ff8-a947-094ac33b505a
📒 Files selected for processing (3)
app/configurations/page.tsxapp/configurations/prompt-editor/page.tsxapp/lib/utils.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/configurations/prompt-editor/page.tsx
- app/configurations/page.tsx
This was referenced Mar 26, 2026
Merged
This was referenced Mar 28, 2026
Merged
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: #86
Summary:
apiClient,apiFetchhelper function in theroute.tsandpage.tsxfiles.Summary by CodeRabbit
Release Notes
New Features
Improvements