Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Add document filter dropdown (All/Enabled/Disabled)
  • Add "Select all" for bulk operations across all documents
  • Migrate tag definitions hooks to React Query
  • Consolidate formatting utilities (formatRelativeTime, formatAbsoluteDate, formatDuration)
  • Add token hover indicator in chunk editor
  • Disable save buttons when no changes made
  • Clean up dead code and unnecessary re-exports

Type of Change

  • Improvement (non-breaking enhancement)

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Jan 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 23, 2026 3:21am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 23, 2026

Greptile Summary

  • Adds document filtering dropdown (All/Enabled/Disabled) and select-all functionality for bulk operations across all documents in knowledge base management
  • Migrates tag definitions hooks to React Query for improved caching and state management, replacing manual fetch logic with centralized query patterns
  • Consolidates formatting utilities (formatRelativeTime, formatAbsoluteDate, formatDuration) into /lib/core/utils/formatting to eliminate code duplication across components

Important Files Changed

Filename Overview
apps/sim/lib/knowledge/documents/service.ts Refactored to support new filtering with enabledFilter parameter and added bulkDocumentOperationByFilter for select-all operations
apps/sim/app/api/knowledge/[id]/documents/route.ts Enhanced API endpoint with flexible filtering, select-all bulk operations, and updated schema validation for new functionality
apps/sim/hooks/queries/knowledge.ts Added comprehensive React Query hooks for tag definitions with proper caching, invalidation, and error handling patterns
apps/sim/app/workspace/[workspaceId]/knowledge/[id]/base.tsx Implemented document filtering UI, select-all mode state management, and migrated to centralized formatting utilities
apps/sim/lib/core/utils/formatting.ts Consolidated formatting utilities from multiple components with enhanced functionality and consistent interfaces

Confidence score: 4/5

  • This PR requires careful review due to complex state management changes and API modifications that could affect bulk operations
  • Score reflects well-structured React Query migration and proper consolidation of utilities, but lowered due to complex filtering logic and potential edge cases in select-all functionality
  • Pay close attention to bulk operation files and the new filtering logic in the service layer to ensure proper handling of edge cases

Sequence Diagram

sequenceDiagram
    participant User
    participant API as "GET /api/knowledge/[id]/documents"
    participant Auth as "getSession"
    participant Access as "checkKnowledgeBaseAccess"
    participant Service as "getDocuments"
    participant DB as "Database"

    User->>API: "GET with enabledFilter param"
    API->>Auth: "getSession()"
    Auth-->>API: "session"
    
    alt session invalid
        API-->>User: "401 Unauthorized"
    else session valid
        API->>Access: "checkKnowledgeBaseAccess(id, userId)"
        Access->>DB: "verify access"
        DB-->>Access: "access result"
        
        alt no access
            Access-->>API: "hasAccess: false"
            API-->>User: "401/404 error"
        else has access
            Access-->>API: "hasAccess: true, knowledgeBase"
            API->>Service: "getDocuments(id, filters, requestId)"
            Service->>DB: "query with enabledFilter"
            DB-->>Service: "documents + pagination"
            Service-->>API: "documents data"
            API-->>User: "200 success with filtered documents"
        end
    end
Loading

@waleedlatif1
Copy link
Collaborator Author

@greptile

cursor[bot]

This comment was marked as outdated.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

26 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 7f22628 into staging Jan 23, 2026
7 checks passed
@waleedlatif1 waleedlatif1 deleted the improvement/kb branch January 23, 2026 03:25
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

async (newPage: number) => {
if (newPage < 1 || newPage > totalPages) return
(newPage: number): boolean => {
return newPage >= 1 && newPage <= totalPages
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goToPage function no longer performs navigation

High Severity

The goToPage function in useDocumentChunks was changed from an async function that performs navigation to a synchronous function that only validates page numbers. However, the consuming code still calls await initialGoToPage(page) expecting it to perform navigation. This breaks chunk pagination functionality entirely.

Additional Locations (1)

Fix in Cursor Fix in Web

{renderSortableHeader('chunkCount', 'Chunks', 'w-[8%]')}
{renderSortableHeader('uploadedAt', 'Uploaded', 'w-[11%]')}
{renderSortableHeader('processingStatus', 'Status', 'w-[10%]')}
{renderSortableHeader('enabled', 'Status', 'w-[10%]')}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Status column sorts by enabled instead of processing status

Medium Severity

The Status column header sorts by enabled field but displays processing status badges (Pending, Processing, Failed, Enabled/Disabled). This creates a mismatch where clicking the Status column header sorts by enabled/disabled boolean instead of the displayed processing status, producing unexpected and confusing sort results.

Additional Locations (1)

Fix in Cursor Fix in Web

setSelectedDocuments(new Set(documents.map((doc) => doc.id)))
} else {
setSelectedDocuments(new Set())
setIsSelectAllMode(false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Select-all mode not cleared when deselecting documents

High Severity

When in select-all mode (selecting all documents across all pages), manually deselecting an individual document via checkbox doesn't clear isSelectAllMode. This causes bulk operations to still affect all documents, including ones the user explicitly deselected, leading to data loss or unintended modifications.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants