API: Remove the Traling Slashes#150
Open
Ayush8923 wants to merge 11 commits into
Open
Conversation
📝 WalkthroughWalkthroughRemove trailing slashes from backend proxy endpoints in route handlers and introduce UI loading/validation states plus layout/styling tweaks in keystore and knowledge-base components. ChangesAPI Endpoint Path Normalization
UI & Component Behavior Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
…ectTech4DevAI/kaapi-frontend into api/remove-trailing-slashes
…ntend into api/remove-trailing-slashes
Ayush8923
requested review from
Prajna1999 and
vprashrex
and removed request for
Prajna1999 and
vprashrex
vprashrex
approved these changes
May 12, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
app/components/keystore/AddKeyModal.tsx (1)
89-94: ⚡ Quick winConsider allowing cancellation during validation.
Both the Cancel and Add buttons are disabled while
isValidatingis true. Users cannot abort a validation request once started, which may be frustrating if the request hangs or takes unexpectedly long.Consider keeping the Cancel button enabled and handling cleanup of the in-flight request in the parent component.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/keystore/AddKeyModal.tsx` around lines 89 - 94, In AddKeyModal, keep the Cancel button enabled during validation by removing or changing the isValidating check on the Cancel button (currently using onClose and disabled={isValidating}), so Cancel always calls onClose; ensure the parent component (where onAddKey and validation request are managed) implements cancellation/cleanup of the in-flight validation (e.g., aborting fetch/Promise or ignoring results) when onClose is invoked so validation can be aborted safely while onAddKey still respects isValidating/isDisabled.app/components/knowledge-base/DocumentPreviewModal.tsx (1)
83-89: ⚡ Quick winAdd error handling for iframe load failures.
If the iframe fails to load due to network errors, invalid URLs, or CORS issues, the loading state will persist for the full 6-second timeout. Adding an
onErrorhandler would provide immediate feedback and better UX.⚡ Proposed fix
{previewDoc?.signed_url ? ( <iframe key={previewDoc.id} src={previewDoc.signed_url} title={previewDoc.fname} onLoad={() => setIsFrameLoading(false)} + onError={() => setIsFrameLoading(false)} className="w-full h-full border-none" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/knowledge-base/DocumentPreviewModal.tsx` around lines 83 - 89, The iframe in DocumentPreviewModal (rendering previewDoc.signed_url with onLoad => setIsFrameLoading(false)) lacks error handling, so failures leave the spinner until timeout; add an onError handler on the iframe that clears the loading state (call setIsFrameLoading(false)) and sets an error flag/state (e.g., setFrameError or setHasPreviewError) so the component can render a fallback UI/message and optionally log the previewDoc.id and URL for debugging; ensure the same error state is reset when previewDoc changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/`(main)/keystore/page.tsx:
- Around line 49-71: The isValidating flag may remain true if an exception
occurs after API validation; wrap the whole add-key flow in a try { ... }
finally { setIsValidating(false); } (or add a finally to the existing try-catch)
so setIsValidating(false) always runs; ensure you include the block that calls
apiFetch("/api/apikeys/verify", ...), constructs newKey, and calls addKey,
resetForm, setIsModalOpen and toast.success inside the try so cleanup always
happens in the finally.
In `@app/`(main)/knowledge-base/page.tsx:
- Around line 104-125: Both handlers (handlePreviewDocument and
handleSelectPreviewDoc) suffer from a race where out-of-order fetches overwrite
the preview; fix by adding a per-component request tracker (e.g., a
useRef<number> latestPreviewRequestId) that you increment before calling
fetchAndPreviewDoc, capture the current id in the async closure, and only call
setPreviewDoc(enriched) (and setIsPreviewLoading(false) behaviorally) if the
captured id matches latestPreviewRequestId.current; update
setShowDocPreviewModal as before and keep using fetchAndPreviewDoc (or switch to
AbortController if supported) so only the most recent selection updates the UI.
In `@app/components/knowledge-base/DocumentPreviewModal.tsx`:
- Around line 28-34: The effect and iframe load logic have a race when switching
documents: track the active document ID with a ref (e.g., currentPreviewIdRef)
inside the useEffect that watches previewDoc?.id and previewDoc?.signed_url, set
the ref to previewDoc.id when starting load, clear any prior timeout/timers, and
only call setIsFrameLoading(false) from the iframe onLoad or fallback timer if
the ref still matches the previewDoc.id; also ensure timers are cleared on
unmount and when previewDoc changes to avoid stale callbacks interfering with
setIsFrameLoading.
- Line 32: The inline comment for the fallback timer in DocumentPreviewModal is
truncated; update the comment above the const timer = setTimeout(() =>
setIsFrameLoading(false), 6000); to fully explain why we need a 6-second
fallback (e.g., iframe onLoad may not fire reliably due to cross-origin content,
cached responses, browser quirks or sandboxing/CSP preventing load events), and
note that the timeout ensures the loading spinner is cleared if onLoad never
fires; keep the comment concise and reference the iframe onLoad handler and the
setIsFrameLoading fallback logic so future maintainers understand the rationale.
---
Nitpick comments:
In `@app/components/keystore/AddKeyModal.tsx`:
- Around line 89-94: In AddKeyModal, keep the Cancel button enabled during
validation by removing or changing the isValidating check on the Cancel button
(currently using onClose and disabled={isValidating}), so Cancel always calls
onClose; ensure the parent component (where onAddKey and validation request are
managed) implements cancellation/cleanup of the in-flight validation (e.g.,
aborting fetch/Promise or ignoring results) when onClose is invoked so
validation can be aborted safely while onAddKey still respects
isValidating/isDisabled.
In `@app/components/knowledge-base/DocumentPreviewModal.tsx`:
- Around line 83-89: The iframe in DocumentPreviewModal (rendering
previewDoc.signed_url with onLoad => setIsFrameLoading(false)) lacks error
handling, so failures leave the spinner until timeout; add an onError handler on
the iframe that clears the loading state (call setIsFrameLoading(false)) and
sets an error flag/state (e.g., setFrameError or setHasPreviewError) so the
component can render a fallback UI/message and optionally log the previewDoc.id
and URL for debugging; ensure the same error state is reset when previewDoc
changes.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d1e7d9d-5f02-4bd5-887e-0a2885823777
📒 Files selected for processing (7)
app/(main)/keystore/page.tsxapp/(main)/knowledge-base/page.tsxapp/components/Sidebar.tsxapp/components/keystore/AddKeyModal.tsxapp/components/keystore/KeysCard.tsxapp/components/knowledge-base/CollectionDetail.tsxapp/components/knowledge-base/DocumentPreviewModal.tsx
✅ Files skipped from review due to trivial changes (2)
- app/components/keystore/KeysCard.tsx
- app/components/Sidebar.tsx
Comment on lines
+49
to
71
| try { | ||
| await apiFetch("/api/apikeys/verify", trimmedKey); | ||
| } catch (err) { | ||
| console.error("API key validation failed:", err); | ||
| toast.error("Invalid API key. Please check the key and try again."); | ||
| setIsValidating(false); | ||
| return; | ||
| } | ||
|
|
||
| const newKey: APIKey = { | ||
| id: Date.now().toString(), | ||
| label: newKeyLabel.trim(), | ||
| key: newKeyValue.trim(), | ||
| key: trimmedKey, | ||
| provider: newKeyProvider, | ||
| createdAt: new Date().toISOString(), | ||
| }; | ||
|
|
||
| addKey(newKey); | ||
| resetForm(); | ||
| setIsModalOpen(false); | ||
| setIsValidating(false); | ||
| toast.success("API key added successfully"); | ||
| }; |
There was a problem hiding this comment.
Add a finally block to guarantee isValidating state cleanup.
If an unexpected error occurs after the validation succeeds (lines 58–70)—for example, if addKey throws or an unforeseen exception arises—isValidating will remain true, permanently disabling the modal buttons.
Wrap the entire operation in a try-finally or extend the existing try-catch with a finally block to reset the state reliably.
🛡️ Proposed fix to add finally block
- const trimmedKey = newKeyValue.trim();
- setIsValidating(true);
-
- try {
- await apiFetch("/api/apikeys/verify", trimmedKey);
- } catch (err) {
- console.error("API key validation failed:", err);
- toast.error("Invalid API key. Please check the key and try again.");
- setIsValidating(false);
- return;
- }
-
- const newKey: APIKey = {
- id: Date.now().toString(),
- label: newKeyLabel.trim(),
- key: trimmedKey,
- provider: newKeyProvider,
- createdAt: new Date().toISOString(),
- };
-
- addKey(newKey);
- resetForm();
- setIsModalOpen(false);
- setIsValidating(false);
- toast.success("API key added successfully");
+ const trimmedKey = newKeyValue.trim();
+ setIsValidating(true);
+
+ try {
+ await apiFetch("/api/apikeys/verify", trimmedKey);
+
+ const newKey: APIKey = {
+ id: Date.now().toString(),
+ label: newKeyLabel.trim(),
+ key: trimmedKey,
+ provider: newKeyProvider,
+ createdAt: new Date().toISOString(),
+ };
+
+ addKey(newKey);
+ resetForm();
+ setIsModalOpen(false);
+ toast.success("API key added successfully");
+ } catch (err) {
+ console.error("API key validation failed:", err);
+ toast.error("Invalid API key. Please check the key and try again.");
+ } finally {
+ setIsValidating(false);
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(main)/keystore/page.tsx around lines 49 - 71, The isValidating flag may
remain true if an exception occurs after API validation; wrap the whole add-key
flow in a try { ... } finally { setIsValidating(false); } (or add a finally to
the existing try-catch) so setIsValidating(false) always runs; ensure you
include the block that calls apiFetch("/api/apikeys/verify", ...), constructs
newKey, and calls addKey, resetForm, setIsModalOpen and toast.success inside the
try so cleanup always happens in the finally.
Comment on lines
104
to
125
| const handlePreviewDocument = async (firstDocument: Document) => { | ||
| setShowDocPreviewModal(true); | ||
| setPreviewDoc(firstDocument); | ||
| const enriched = await fetchAndPreviewDoc(firstDocument); | ||
| setPreviewDoc(enriched); | ||
| setIsPreviewLoading(true); | ||
| try { | ||
| const enriched = await fetchAndPreviewDoc(firstDocument); | ||
| setPreviewDoc(enriched); | ||
| } finally { | ||
| setIsPreviewLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const handleSelectPreviewDoc = async (doc: Document) => { | ||
| setPreviewDoc(doc); | ||
| const enriched = await fetchAndPreviewDoc(doc); | ||
| setPreviewDoc(enriched); | ||
| setIsPreviewLoading(true); | ||
| try { | ||
| const enriched = await fetchAndPreviewDoc(doc); | ||
| setPreviewDoc(enriched); | ||
| } finally { | ||
| setIsPreviewLoading(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Race condition: Rapid document selection can display stale previews.
If a user clicks document A, then quickly clicks document B before A's preview loads, the requests can complete out of order. When A finishes after B, setPreviewDoc(enrichedA) will overwrite B, showing the wrong document to the user.
🔒 Proposed fix using request ID tracking
export default function KnowledgeBasePage() {
...
const [previewDoc, setPreviewDoc] = useState<Document | null>(null);
const [isPreviewLoading, setIsPreviewLoading] = useState(false);
+ const previewRequestIdRef = useRef(0);
const handlePreviewDocument = async (firstDocument: Document) => {
setShowDocPreviewModal(true);
setPreviewDoc(firstDocument);
+ const requestId = ++previewRequestIdRef.current;
setIsPreviewLoading(true);
try {
const enriched = await fetchAndPreviewDoc(firstDocument);
- setPreviewDoc(enriched);
+ if (requestId === previewRequestIdRef.current) {
+ setPreviewDoc(enriched);
+ }
} finally {
- setIsPreviewLoading(false);
+ if (requestId === previewRequestIdRef.current) {
+ setIsPreviewLoading(false);
+ }
}
};
const handleSelectPreviewDoc = async (doc: Document) => {
setPreviewDoc(doc);
+ const requestId = ++previewRequestIdRef.current;
setIsPreviewLoading(true);
try {
const enriched = await fetchAndPreviewDoc(doc);
- setPreviewDoc(enriched);
+ if (requestId === previewRequestIdRef.current) {
+ setPreviewDoc(enriched);
+ }
} finally {
- setIsPreviewLoading(false);
+ if (requestId === previewRequestIdRef.current) {
+ setIsPreviewLoading(false);
+ }
}
};Don't forget to add the import:
-import { useState } from "react";
+import { useState, useRef } from "react";📝 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 handlePreviewDocument = async (firstDocument: Document) => { | |
| setShowDocPreviewModal(true); | |
| setPreviewDoc(firstDocument); | |
| const enriched = await fetchAndPreviewDoc(firstDocument); | |
| setPreviewDoc(enriched); | |
| setIsPreviewLoading(true); | |
| try { | |
| const enriched = await fetchAndPreviewDoc(firstDocument); | |
| setPreviewDoc(enriched); | |
| } finally { | |
| setIsPreviewLoading(false); | |
| } | |
| }; | |
| const handleSelectPreviewDoc = async (doc: Document) => { | |
| setPreviewDoc(doc); | |
| const enriched = await fetchAndPreviewDoc(doc); | |
| setPreviewDoc(enriched); | |
| setIsPreviewLoading(true); | |
| try { | |
| const enriched = await fetchAndPreviewDoc(doc); | |
| setPreviewDoc(enriched); | |
| } finally { | |
| setIsPreviewLoading(false); | |
| } | |
| }; | |
| const handlePreviewDocument = async (firstDocument: Document) => { | |
| setShowDocPreviewModal(true); | |
| setPreviewDoc(firstDocument); | |
| const requestId = ++previewRequestIdRef.current; | |
| setIsPreviewLoading(true); | |
| try { | |
| const enriched = await fetchAndPreviewDoc(firstDocument); | |
| if (requestId === previewRequestIdRef.current) { | |
| setPreviewDoc(enriched); | |
| } | |
| } finally { | |
| if (requestId === previewRequestIdRef.current) { | |
| setIsPreviewLoading(false); | |
| } | |
| } | |
| }; | |
| const handleSelectPreviewDoc = async (doc: Document) => { | |
| setPreviewDoc(doc); | |
| const requestId = ++previewRequestIdRef.current; | |
| setIsPreviewLoading(true); | |
| try { | |
| const enriched = await fetchAndPreviewDoc(doc); | |
| if (requestId === previewRequestIdRef.current) { | |
| setPreviewDoc(enriched); | |
| } | |
| } finally { | |
| if (requestId === previewRequestIdRef.current) { | |
| setIsPreviewLoading(false); | |
| } | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/`(main)/knowledge-base/page.tsx around lines 104 - 125, Both handlers
(handlePreviewDocument and handleSelectPreviewDoc) suffer from a race where
out-of-order fetches overwrite the preview; fix by adding a per-component
request tracker (e.g., a useRef<number> latestPreviewRequestId) that you
increment before calling fetchAndPreviewDoc, capture the current id in the async
closure, and only call setPreviewDoc(enriched) (and setIsPreviewLoading(false)
behaviorally) if the captured id matches latestPreviewRequestId.current; update
setShowDocPreviewModal as before and keep using fetchAndPreviewDoc (or switch to
AbortController if supported) so only the most recent selection updates the UI.
Comment on lines
+28
to
+34
| useEffect(() => { | ||
| if (!previewDoc?.signed_url) return; | ||
| setIsFrameLoading(true); | ||
| // Fallback: iframe `onLoad` doesn't fire reliably when the browser | ||
| const timer = setTimeout(() => setIsFrameLoading(false), 6000); | ||
| return () => clearTimeout(timer); | ||
| }, [previewDoc?.id, previewDoc?.signed_url]); |
There was a problem hiding this comment.
Race condition when switching documents rapidly.
If a user clicks multiple documents in quick succession, the iframe onLoad handler from the previous document can fire after the new document starts loading, causing isFrameLoading to be set to false prematurely. The current implementation doesn't track which iframe's load event corresponds to which document.
🔒 Proposed fix using a ref to track the current document ID
export default function DocumentPreviewModal({
...
}: DocumentPreviewModalProps) {
const [isFrameLoading, setIsFrameLoading] = useState(false);
+ const loadingDocIdRef = useRef<string | null>(null);
useEffect(() => {
if (!previewDoc?.signed_url) return;
setIsFrameLoading(true);
+ loadingDocIdRef.current = previewDoc.id;
// Fallback: iframe `onLoad` doesn't fire reliably when the browser
const timer = setTimeout(() => setIsFrameLoading(false), 6000);
return () => clearTimeout(timer);
}, [previewDoc?.id, previewDoc?.signed_url]);
const showLoader = isLoading || isFrameLoading;
return (
<Modal ...>
...
<iframe
key={previewDoc.id}
src={previewDoc.signed_url}
title={previewDoc.fname}
- onLoad={() => setIsFrameLoading(false)}
+ onLoad={() => {
+ if (loadingDocIdRef.current === previewDoc.id) {
+ setIsFrameLoading(false);
+ }
+ }}
className="w-full h-full border-none"
/>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/components/knowledge-base/DocumentPreviewModal.tsx` around lines 28 - 34,
The effect and iframe load logic have a race when switching documents: track the
active document ID with a ref (e.g., currentPreviewIdRef) inside the useEffect
that watches previewDoc?.id and previewDoc?.signed_url, set the ref to
previewDoc.id when starting load, clear any prior timeout/timers, and only call
setIsFrameLoading(false) from the iframe onLoad or fallback timer if the ref
still matches the previewDoc.id; also ensure timers are cleared on unmount and
when previewDoc changes to avoid stale callbacks interfering with
setIsFrameLoading.
| if (!previewDoc?.signed_url) return; | ||
| setIsFrameLoading(true); | ||
| // Fallback: iframe `onLoad` doesn't fire reliably when the browser | ||
| const timer = setTimeout(() => setIsFrameLoading(false), 6000); |
There was a problem hiding this comment.
Complete the comment explaining the fallback timer.
The comment is truncated mid-sentence and doesn't explain why the iframe onLoad event might not fire reliably. This context is important for future maintainers understanding the 6-second timeout.
📝 Suggested completion
- // Fallback: iframe `onLoad` doesn't fire reliably when the browser
+ // Fallback: iframe `onLoad` doesn't fire reliably for all document types or when
+ // the browser blocks certain content, so we clear loading state after 6 seconds🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/components/knowledge-base/DocumentPreviewModal.tsx` at line 32, The
inline comment for the fallback timer in DocumentPreviewModal is truncated;
update the comment above the const timer = setTimeout(() =>
setIsFrameLoading(false), 6000); to fully explain why we need a 6-second
fallback (e.g., iframe onLoad may not fire reliably due to cross-origin content,
cached responses, browser quirks or sandboxing/CSP preventing load events), and
note that the timeout ensures the loading spinner is cleared if onLoad never
fires; keep the comment concise and reference the iframe onLoad handler and the
setIsFrameLoading fallback logic so future maintainers understand the rationale.
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: #152
Summary:
documentandknowledge-base.Summary by CodeRabbit
New Features
UI Improvements
Chores