Knowledge base: Creation and deletion UI #28
Conversation
📝 WalkthroughWalkthroughAdds a Knowledge Base UI and supporting API proxy routes: collection list/create, collection get/delete, and job-status polling; consolidates date formatting into a shared utility and adds a Knowledge base item to the sidebar. All API routes forward X-API-KEY to the backend. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/Browser
participant APIRoute as API Route (Next.js)
participant Backend as Backend Service
participant Poller as Polling Component
Client->>APIRoute: POST /api/collections (X-API-KEY, body)
APIRoute->>Backend: POST /api/v1/collections (forward key & body)
Backend-->>APIRoute: 200 { job_id }
APIRoute-->>Client: 200 { job_id }
Client->>Poller: Start polling job_id
loop until job complete
Poller->>APIRoute: GET /api/collections/jobs/{job_id} (X-API-KEY)
APIRoute->>Backend: GET /api/v1/collections/jobs/{job_id}
Backend-->>APIRoute: job status
APIRoute-->>Poller: job status
Poller->>Client: update UI
end
Client->>APIRoute: GET /api/collections/{collection_id} (X-API-KEY)
APIRoute->>Backend: GET /api/v1/collections/{collection_id}?include_docs=true
Backend-->>APIRoute: collection + docs
APIRoute-->>Client: collection payload
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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 unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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: 2
🤖 Fix all issues with AI agents
In `@app/components/utils.ts`:
- Around line 10-21: The formatDate function currently assumes new
Date(dateString) is valid; update formatDate to guard against invalid dates by
checking Number.isNaN(date.getTime()) (or isNaN(date.getTime())) right after
creating const date = new Date(dateString) and before applying the IST offset
and formatting; if the check fails return 'N/A' (or the current fallback) so you
never format an Invalid Date and produce NaN values. Ensure you keep the
existing IST conversion and formatting logic in formatDate unchanged except for
this validity guard.
In `@app/knowledge-base/page.tsx`:
- Around line 170-188: The fetchCollections function assumes the API returns an
object with result.data, so when /api/collections returns a raw array the UI
shows zero items; change the parsing to accept both shapes by treating the
response as const collections = result.data || result || [] before
enriching—then continue to call enrichCollectionWithCache on that array and pass
the enrichedCollections to setCollections.
| export const formatDate = (dateString?: string): string => { | ||
| if (!dateString) return 'N/A'; | ||
| try { | ||
| const date = new Date(dateString); | ||
| const istDate = new Date(date.getTime() + (5.5 * 60 * 60 * 1000)); | ||
| const day = istDate.getDate(); | ||
| const month = istDate.toLocaleDateString('en-US', { month: 'short' }); | ||
| const year = istDate.getFullYear(); | ||
| const hours = String(istDate.getHours()).padStart(2, '0'); | ||
| const minutes = String(istDate.getMinutes()).padStart(2, '0'); | ||
| return `${day} ${month} ${year}, ${hours}:${minutes}`; | ||
| } catch { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "utils.ts" -path "*/app/components/*" -type fRepository: ProjectTech4DevAI/kaapi-frontend
Length of output: 101
🏁 Script executed:
cat -n app/components/utils.ts | head -30Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 1356
🏁 Script executed:
# Test JavaScript Date behavior with invalid input
node <<'EOF'
// Test invalid date behavior
const invalidDate = new Date("invalid-date-string");
console.log("Invalid date object:", invalidDate);
console.log("getTime() result:", invalidDate.getTime());
console.log("isNaN(getTime()):", Number.isNaN(invalidDate.getTime()));
console.log("getDate() result:", invalidDate.getDate());
console.log("getHours() result:", invalidDate.getHours());
// Test formatting with NaN values
const result = `${invalidDate.getDate()} ${invalidDate.getHours()}:00`;
console.log("Formatted with NaN:", result);
EOFRepository: ProjectTech4DevAI/kaapi-frontend
Length of output: 230
🏁 Script executed:
# Search for tests or usages of formatDate
rg "formatDate" --type ts --type tsx -B 2 -A 2Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 103
🏁 Script executed:
# Search for tests or usages of formatDate with correct file types
rg "formatDate" -B 2 -A 2Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 6249
🏁 Script executed:
# Check if there are test files
find . -name "*.test.ts" -o -name "*.spec.ts" | head -10Repository: ProjectTech4DevAI/kaapi-frontend
Length of output: 58
Guard against invalid date inputs. new Date() doesn't throw on invalid input—it returns an Invalid Date object with NaN values that render as "NaN NaN:00" in the output. The current try/catch won't prevent this. Add a check for Number.isNaN(date.getTime()) before formatting.
Proposed fix
export const formatDate = (dateString?: string): string => {
if (!dateString) return 'N/A';
try {
const date = new Date(dateString);
+ if (Number.isNaN(date.getTime())) {
+ return dateString;
+ }
const istDate = new Date(date.getTime() + (5.5 * 60 * 60 * 1000));
const day = istDate.getDate();
const month = istDate.toLocaleDateString('en-US', { month: 'short' });📝 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.
| export const formatDate = (dateString?: string): string => { | |
| if (!dateString) return 'N/A'; | |
| try { | |
| const date = new Date(dateString); | |
| const istDate = new Date(date.getTime() + (5.5 * 60 * 60 * 1000)); | |
| const day = istDate.getDate(); | |
| const month = istDate.toLocaleDateString('en-US', { month: 'short' }); | |
| const year = istDate.getFullYear(); | |
| const hours = String(istDate.getHours()).padStart(2, '0'); | |
| const minutes = String(istDate.getMinutes()).padStart(2, '0'); | |
| return `${day} ${month} ${year}, ${hours}:${minutes}`; | |
| } catch { | |
| export const formatDate = (dateString?: string): string => { | |
| if (!dateString) return 'N/A'; | |
| try { | |
| const date = new Date(dateString); | |
| if (Number.isNaN(date.getTime())) { | |
| return dateString; | |
| } | |
| const istDate = new Date(date.getTime() + (5.5 * 60 * 60 * 1000)); | |
| const day = istDate.getDate(); | |
| const month = istDate.toLocaleDateString('en-US', { month: 'short' }); | |
| const year = istDate.getFullYear(); | |
| const hours = String(istDate.getHours()).padStart(2, '0'); | |
| const minutes = String(istDate.getMinutes()).padStart(2, '0'); | |
| return `${day} ${month} ${year}, ${hours}:${minutes}`; | |
| } catch { |
🤖 Prompt for AI Agents
In `@app/components/utils.ts` around lines 10 - 21, The formatDate function
currently assumes new Date(dateString) is valid; update formatDate to guard
against invalid dates by checking Number.isNaN(date.getTime()) (or
isNaN(date.getTime())) right after creating const date = new Date(dateString)
and before applying the IST offset and formatting; if the check fails return
'N/A' (or the current fallback) so you never format an Invalid Date and produce
NaN values. Ensure you keep the existing IST conversion and formatting logic in
formatDate unchanged except for this validity guard.
| // Fetch collections | ||
| const fetchCollections = async () => { | ||
| if (!apiKey) return; | ||
|
|
||
| setIsLoading(true); | ||
| try { | ||
| const response = await fetch('/api/collections', { | ||
| headers: { 'X-API-KEY': apiKey.key }, | ||
| }); | ||
|
|
||
| if (response.ok) { | ||
| const result = await response.json(); | ||
| const collections = result.data || []; | ||
|
|
||
| // Enrich collections with cached names and live status | ||
| const enrichedCollections = await Promise.all( | ||
| collections.map(collection => enrichCollectionWithCache(collection)) | ||
| ); | ||
| setCollections(enrichedCollections); |
There was a problem hiding this comment.
Handle array responses from /api/collections. If the API returns a raw array (as the proxy route can), result.data is undefined and the UI will show zero collections. Mirror the document fetch pattern to accept both shapes.
Proposed fix
if (response.ok) {
const result = await response.json();
- const collections = result.data || [];
+ const collections = Array.isArray(result) ? result : (result.data || []);
// Enrich collections with cached names and live status
const enrichedCollections = await Promise.all(
collections.map(collection => enrichCollectionWithCache(collection))📝 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.
| // Fetch collections | |
| const fetchCollections = async () => { | |
| if (!apiKey) return; | |
| setIsLoading(true); | |
| try { | |
| const response = await fetch('/api/collections', { | |
| headers: { 'X-API-KEY': apiKey.key }, | |
| }); | |
| if (response.ok) { | |
| const result = await response.json(); | |
| const collections = result.data || []; | |
| // Enrich collections with cached names and live status | |
| const enrichedCollections = await Promise.all( | |
| collections.map(collection => enrichCollectionWithCache(collection)) | |
| ); | |
| setCollections(enrichedCollections); | |
| // Fetch collections | |
| const fetchCollections = async () => { | |
| if (!apiKey) return; | |
| setIsLoading(true); | |
| try { | |
| const response = await fetch('/api/collections', { | |
| headers: { 'X-API-KEY': apiKey.key }, | |
| }); | |
| if (response.ok) { | |
| const result = await response.json(); | |
| const collections = Array.isArray(result) ? result : (result.data || []); | |
| // Enrich collections with cached names and live status | |
| const enrichedCollections = await Promise.all( | |
| collections.map(collection => enrichCollectionWithCache(collection)) | |
| ); | |
| setCollections(enrichedCollections); |
🤖 Prompt for AI Agents
In `@app/knowledge-base/page.tsx` around lines 170 - 188, The fetchCollections
function assumes the API returns an object with result.data, so when
/api/collections returns a raw array the UI shows zero items; change the parsing
to accept both shapes by treating the response as const collections =
result.data || result || [] before enriching—then continue to call
enrichCollectionWithCache on that array and pass the enrichedCollections to
setCollections.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/knowledge-base/page.tsx`:
- Around line 598-609: The onChange handler for the batch size input can set
batchSize to NaN because parseInt(e.target.value) may return NaN; update the
handler used with setBatchSize so you first coerce/validate the parsed value
(e.g., const n = parseInt(e.target.value); const valid = Number.isNaN(n) ? 1 :
n) and then call setBatchSize(Math.max(1, valid)) — modify the inline onChange
arrow function that currently uses setBatchSize(Math.max(1,
parseInt(e.target.value) || 1)) to ensure NaN is handled explicitly before
Math.max.
- Around line 270-275: In handleConfirmCreate add an explicit null guard for
apiKey before using apiKey.key: check if apiKey is null (e.g., if (!apiKey) {
handle the error/return early }) and only call fetch('/api/collections', ...)
when apiKey is non-null; update any UI state (close modal or set an error) in
the early-return branch so TypeScript no longer complains about accessing
apiKey.key and future regressions are prevented.
🧹 Nitpick comments (4)
app/knowledge-base/page.tsx (4)
9-17: DuplicateDocumentinterface definition.This interface is identical to the one in
app/document/page.tsx. Consider importing it from there or extracting to a shared types file to avoid drift and reduce maintenance overhead.-export interface Document { - id: string; - fname: string; - object_store_url: string; - signed_url?: string; - file_size?: number; - inserted_at?: string; - updated_at?: string; -} +import { Document } from '@/app/document/page';
79-141: Sequential API calls inenrichCollectionWithCachemay cause slow loading.This function is called via
Promise.allon line 185-186, but internally it performs sequential API calls within loops (lines 99, 120). For users with many collections, this creates a waterfall of network requests during enrichment.Consider batching job status requests or fetching all job statuses in a single call if the backend supports it, then matching locally.
367-372: Missing dependencies inuseEffectmay cause stale closures.The effect depends on
apiKeybut callsfetchCollectionsandfetchDocumentswhich are recreated on each render. Either add them to the dependency array withuseCallback, or inline the fetch logic.+ const fetchCollections = useCallback(async () => { + // ... existing implementation + }, [apiKey]); + + const fetchDocuments = useCallback(async () => { + // ... existing implementation + }, [apiKey]); + useEffect(() => { if (apiKey) { fetchCollections(); fetchDocuments(); } - }, [apiKey]); + }, [apiKey, fetchCollections, fetchDocuments]);Alternatively, suppress the lint warning if the current behavior is intentional, but document why.
707-711: Use theme color instead of hardcoded hex value.The delete button uses hardcoded
#ef4444while the project hascolors.status.error(#dc2626) defined in the theme. Consider using the theme color for consistency.style={{ backgroundColor: 'transparent', - color: '#ef4444', - border: '1px solid `#ef4444`', + color: colors.status.error, + border: `1px solid ${colors.status.error}`, }}
| const response = await fetch('/api/collections', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'X-API-KEY': apiKey.key, | ||
| 'Content-Type': 'application/json', | ||
| }, |
There was a problem hiding this comment.
Add null guard for apiKey in handleConfirmCreate.
TypeScript will flag apiKey.key as potentially null since apiKey is typed as APIKey | null. Although the modal is only shown after handleCreateClick validates apiKey, adding an explicit guard improves type safety and prevents future regressions.
const handleConfirmCreate = async () => {
setShowConfirmCreate(false);
+ if (!apiKey) return;
setIsCreating(true);
try {
const response = await fetch('/api/collections', {🤖 Prompt for AI Agents
In `@app/knowledge-base/page.tsx` around lines 270 - 275, In handleConfirmCreate
add an explicit null guard for apiKey before using apiKey.key: check if apiKey
is null (e.g., if (!apiKey) { handle the error/return early }) and only call
fetch('/api/collections', ...) when apiKey is non-null; update any UI state
(close modal or set an error) in the early-return branch so TypeScript no longer
complains about accessing apiKey.key and future regressions are prevented.
| <input | ||
| type="number" | ||
| value={batchSize} | ||
| onChange={(e) => setBatchSize(Math.max(1, parseInt(e.target.value) || 1))} | ||
| min="1" | ||
| className="w-full px-4 py-2 rounded-md border text-sm" | ||
| style={{ | ||
| borderColor: colors.border, | ||
| backgroundColor: colors.bg.secondary, | ||
| color: colors.text.primary, | ||
| }} | ||
| /> |
There was a problem hiding this comment.
Handle NaN when parsing batch size input.
parseInt(e.target.value) returns NaN for non-numeric input, and Math.max(1, NaN) evaluates to NaN. This could result in an invalid batchSize being sent to the API.
- onChange={(e) => setBatchSize(Math.max(1, parseInt(e.target.value) || 1))}
+ onChange={(e) => {
+ const parsed = parseInt(e.target.value, 10);
+ setBatchSize(Number.isNaN(parsed) ? 1 : Math.max(1, parsed));
+ }}📝 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.
| <input | |
| type="number" | |
| value={batchSize} | |
| onChange={(e) => setBatchSize(Math.max(1, parseInt(e.target.value) || 1))} | |
| min="1" | |
| className="w-full px-4 py-2 rounded-md border text-sm" | |
| style={{ | |
| borderColor: colors.border, | |
| backgroundColor: colors.bg.secondary, | |
| color: colors.text.primary, | |
| }} | |
| /> | |
| <input | |
| type="number" | |
| value={batchSize} | |
| onChange={(e) => { | |
| const parsed = parseInt(e.target.value, 10); | |
| setBatchSize(Number.isNaN(parsed) ? 1 : Math.max(1, parsed)); | |
| }} | |
| min="1" | |
| className="w-full px-4 py-2 rounded-md border text-sm" | |
| style={{ | |
| borderColor: colors.border, | |
| backgroundColor: colors.bg.secondary, | |
| color: colors.text.primary, | |
| }} | |
| /> |
🤖 Prompt for AI Agents
In `@app/knowledge-base/page.tsx` around lines 598 - 609, The onChange handler for
the batch size input can set batchSize to NaN because parseInt(e.target.value)
may return NaN; update the handler used with setBatchSize so you first
coerce/validate the parsed value (e.g., const n = parseInt(e.target.value);
const valid = Number.isNaN(n) ? 1 : n) and then call setBatchSize(Math.max(1,
valid)) — modify the inline onChange arrow function that currently uses
setBatchSize(Math.max(1, parseInt(e.target.value) || 1)) to ensure NaN is
handled explicitly before Math.max.
nishika26
changed the title
This is how the UI looks like right now, please take a pull from this branch to try it out, also note that creation and deletion of knowledge base takes a little time to get updated in the UI, while the process already starts in the backend -
Summary
New Features
Improvements
Summary by CodeRabbit