-
Notifications
You must be signed in to change notification settings - Fork 37
feat: Add pagination and search for projects and sessions lists #428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Backend: Implement parallel SSAR checks for projects listing - Backend: Add pagination and search support to ListProjects and ListSessions - Frontend: Add pagination controls and search input to projects page - Frontend: Add pagination controls and search input to sessions section - Add PaginationParams and PaginatedResponse types
The Next.js API routes for projects and sessions were not forwarding query parameters to the backend, breaking pagination and search. - Forward query string in /api/projects route - Forward query string in /api/projects/[name]/agentic-sessions route
- Backend: Implement parallel SSAR checks using worker pool for faster project listing - Backend: Add server-side search filtering for projects and sessions - Backend: Add offset-based pagination for both endpoints - Frontend: Add search input with debouncing for projects and sessions pages - Frontend: Add pagination controls (Previous/Next) with page info - Frontend: Fix Next.js API routes to forward query parameters to backend - Add shared pagination types in backend (PaginationParams, PaginatedResponse) - Add corresponding frontend types and React Query hooks Performance improvement: SSAR checks now run in parallel with configurable worker pool (default 10 workers), significantly reducing project list load times.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM but first fix GoLint
- Fix context cancellation in SSAR worker pool: workers now report cancellation so caller can detect timeouts and log warnings for partial results - Replace bubble sort O(n²) with sort.Slice O(n log n) for both projects and sessions sorting - significant performance improvement for large lists - Fix gofmt formatting in types/common.go
Claude Code ReviewSummaryThis PR adds pagination and search functionality to projects and sessions lists, addressing slow response times with large datasets. The implementation includes parallel SSAR checks, server-side search filtering, and offset-based pagination. Overall Assessment: Strong implementation with good performance improvements. A few critical issues need attention before merge. Issues by Severity🚫 Blocker IssuesNone - No blocking issues found. 🔴 Critical Issues1. Type-Unsafe Unstructured Access (sessions.go:330)Location: session.Metadata = item.Object["metadata"].(map[string]interface{})Issue: Direct type assertion without checking violates CLAUDE.md Critical Rule #4. This will panic if the type is wrong. Required Fix (per .claude/patterns/error-handling.md): if metadata, ok := item.Object["metadata"].(map[string]interface{}); ok {
session.Metadata = metadata
} else {
log.Printf("Warning: invalid metadata type for session %v", item.GetName())
session.Metadata = make(map[string]interface{})
}Reference: CLAUDE.md lines 362-365, backend-development.md lines 69-85 2. Goroutine Leak Risk in Worker Pool (projects.go:286-289)Location: for range workChan {
resultChan <- accessCheckResult{cancelled: true}
}Issue: When context is cancelled, the worker drains the entire work channel, but this can block if the channel is large and no other workers are consuming. This could cause goroutine leaks. Recommended Fix: select {
case <-ctx.Done():
resultChan <- accessCheckResult{
namespace: ns,
cancelled: true,
}
return // Exit immediately, rely on wg.Wait() and close(resultChan)
default:
}The sender will handle unconsumed work items by the channel closing. 🟡 Major Issues3. Missing User Token for SSAR ChecksLocation: Issue: The parallel SSAR checks use Recommended: if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}Already present at line 165, but the function signature of 4. Generic Error Messages Expose Internal Details (sessions.go:321)Location: c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list agentic sessions"})Issue: While this follows the pattern, the log message at line 320 is correct. Good adherence to error-handling.md. No action needed - Just noting for completeness. 5. Frontend: Deprecated
|
## Summary This PR fixes a critical performance issue discovered after merging #428 (pagination and parallel SSAR checks). ### Problem The default Kubernetes client rate limits (QPS=5, Burst=10) were causing significant throttling during parallel SSAR checks: ``` "Waited before sending request" delay="1.997446758s" reason="client-side throttling" ``` ### Solution Increase rate limits: - QPS: 5 → 100 - Burst: 10 → 200 ### Performance Impact Tested with 51 namespaces: - **Before**: 8.2 seconds - **After**: 50 milliseconds - **Improvement**: ~164x faster ### Testing Verified on dev cluster with 51 managed namespaces. Timing logs confirmed SSAR checks completed in 30ms with the increased rate limits.
Summary
This PR addresses slow backend response times for the project list and session list pages by implementing:
Backend Changes
?search=query parameter for filtering?limit=and?offset=parameters with paginated responsesFrontend Changes
totalPages > 1)API Response Format
Endpoints now return:
{ "items": [...], "totalCount": 100, "limit": 20, "offset": 0, "hasMore": true, "nextOffset": 20 }Performance Impact
Testing
totalPages > 1