feat: add server-side search and filtering for agents list #71
Conversation
WalkthroughServer-side search, filtering, sorting, and pagination were added to ListAgents. Controllers parse query params into a new AgentFilter model, services accept that filter, repositories implement DB-level filtering and return total counts, and the frontend sends debounced search queries instead of client-side filtering. Changes
sequenceDiagram
participant User
participant Frontend as AgentsList
participant API as AgentController
participant Service as AgentManager
participant Repo as AgentRepository
participant DB
User->>Frontend: Type search/filter
Frontend->>Frontend: Debounce (300ms)
Frontend->>API: GET /agents?search=...&sortBy=...&sortOrder=...
API->>Service: ListAgents(ctx, org, proj, AgentFilter)
Service->>Repo: ListAgentsWithFilter(ctx, orgId, projId, AgentFilter)
Repo->>DB: SELECT COUNT(*) WHERE (search/type)
DB-->>Repo: totalCount
Repo->>DB: SELECT ... WHERE (search/type) ORDER BY ... LIMIT/OFFSET
DB-->>Repo: agents + details
Repo-->>Service: agents, totalCount
Service->>API: AgentResponse[], totalCount
API-->>Frontend: {agents, total}
Frontend-->>User: Render filtered list
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 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 |
Kavirubc
force-pushed
the
feat/agent-search-filter
branch
from
5678e71 to
8c1344d
Compare
![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)
console/workspaces/libs/types/src/api/agents.ts (1)
65-70: LGTM with optional type strengthening suggestion.The interface correctly extends ListQuery with optional filter fields. The
sortOrderfield is well-typed as a union type.Optionally, consider adding union types for
sortByandprovisioningTypeto match thesortOrderpattern:🔎 View suggested type improvements:
export interface ListAgentsQuery extends ListQuery { search?: string; - provisioningType?: string; - sortBy?: string; + provisioningType?: 'internal' | 'external'; + sortBy?: 'name' | 'createdAt' | 'updatedAt'; sortOrder?: 'asc' | 'desc'; }This would provide better type safety and IDE autocomplete support on the frontend.
agent-manager-service/repositories/agent.go (1)
67-75: Clarify that wildcards in user input should be escaped for correct pattern matching behavior.The ILIKE query is safe from SQL injection via GORM's parameterization, but wildcards (% and _) in
filter.Searchare not escaped and will be treated as pattern wildcards. A user entering%would return all records. Consider escaping these characters using PostgreSQL's ESCAPE clause or sanitizing the input before concatenation.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
agent-manager-service/controllers/agent_controller.go(2 hunks)agent-manager-service/models/agent_filter.go(1 hunks)agent-manager-service/repositories/agent.go(2 hunks)agent-manager-service/services/agent_manager.go(3 hunks)console/workspaces/libs/types/src/api/agents.ts(1 hunks)console/workspaces/pages/overview/src/AgentsList/AgentsList.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-18T16:41:56.601Z
Learnt from: hanzjk
Repo: wso2/ai-agent-management-platform PR: 70
File: agent-manager-service/services/agent_manager.go:978-980
Timestamp: 2025-12-18T16:41:56.601Z
Learning: In the agent-manager-service, enforce that external agents cannot have a SubType; only internal/managed agents may have SubType (e.g., chat-api, custom-api). When converting external agents to AgentResponse, the Type payload should include only the Type field and should not expose SubType. Implement this in the conversion/serialization path (likely in services/agent_manager.go) and update related DTO mappings or serializers. Add unit tests to verify internal vs external agent handling and ensure SubType is omitted for external agents.
Applied to files:
agent-manager-service/services/agent_manager.goagent-manager-service/models/agent_filter.goagent-manager-service/controllers/agent_controller.goagent-manager-service/repositories/agent.go
🧬 Code graph analysis (5)
console/workspaces/libs/types/src/api/agents.ts (1)
console/workspaces/libs/types/src/api/common.ts (1)
ListQuery(27-30)
agent-manager-service/services/agent_manager.go (4)
agent-manager-service/models/agent_filter.go (1)
AgentFilter(20-27)agent-manager-service/models/agent.go (1)
AgentResponse(27-37)agent-manager-service/spec/model_agent_response.go (1)
AgentResponse(22-33)agent-manager-service/repositories/agent.go (1)
AgentRepository(30-39)
agent-manager-service/models/agent_filter.go (1)
console/workspaces/libs/types/src/api/agents.ts (1)
ProvisioningType(38-38)
agent-manager-service/controllers/agent_controller.go (2)
agent-manager-service/models/agent_filter.go (1)
AgentFilter(20-27)agent-manager-service/middleware/jwtassertion/auth.go (1)
GetTokenClaims(80-86)
agent-manager-service/repositories/agent.go (4)
agent-manager-service/models/agent_filter.go (1)
AgentFilter(20-27)agent-manager-service/models/agent.go (1)
Agent(58-70)agent-manager-service/db/tx_utils.go (1)
DB(31-46)console/workspaces/libs/types/src/api/agents.ts (1)
ProvisioningType(38-38)
🔇 Additional comments (5)
console/workspaces/pages/overview/src/AgentsList/AgentsList.tsx (3)
103-112: LGTM! Clean debounced search implementation.The debounced search with a 300ms delay is well-implemented with proper cleanup. This will reduce API calls while providing a responsive user experience.
130-133: Backend search integration correctly implemented.The search parameter is properly passed to the API with
debouncedSearch || undefined, ensuring empty strings are not sent as query parameters.
172-189: Redirect guard correctly prevents navigation during search.The addition of
!debouncedSearchat line 179 prevents automatic redirect to the new agent page when search returns no results, showing the empty state instead. This is the correct UX behavior.agent-manager-service/services/agent_manager.go (1)
129-165: LGTM! Service layer correctly delegates filtering to repository.The signature change from separate limit/offset parameters to a single filter object simplifies the API and allows for future filter extensions without changing signatures. The implementation correctly delegates to
ListAgentsWithFilterand returns both results and total count.Note: This service method assumes the filter has been validated by the controller. Ensure the controller at agent-manager-service/controllers/agent_controller.go validates filter parameters before calling this method (see my comment on that file).
agent-manager-service/repositories/agent.go (1)
101-106: Pagination logic applies Limit even when zero.Lines 101-102 apply
Limit(filter.Limit)even whenfilter.Limitis 0. In GORM,Limit(0)returns no results. The check should beif filter.Limit > 0to match the Offset check pattern and align with typical pagination behavior where Limit=0 might mean "no limit."🔎 Apply this diff:
// Apply pagination - if filter.Limit > 0 { + if filter.Limit > 0 { query = query.Limit(filter.Limit) } if filter.Offset > 0 { query = query.Offset(filter.Offset) }Wait, the code already checks
> 0. Let me re-read...Actually, the existing code at line 101 already checks
if filter.Limit > 0, so this is correct. The Offset check at line 104 also uses> 0, which means Offset=0 is the default (start from beginning).This is actually fine. Let me reconsider - this is correct as-is.
Actually, upon closer inspection, the existing logic at lines 101-106 is correct. My initial assessment was wrong.
| // AgentFilter holds filter options for listing agents | ||
| type AgentFilter struct { | ||
| Search string // search in name, displayName, description | ||
| ProvisioningType string // "internal", "external" | ||
| SortBy string // "name", "createdAt", "updatedAt" | ||
| SortOrder string // "asc", "desc" | ||
| Limit int | ||
| Offset int | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consider adding validation or using constants for filter values.
The AgentFilter struct accepts string values for SortBy, SortOrder, and ProvisioningType without validation. Invalid values will be silently ignored or cause unexpected behavior in the repository layer.
Consider:
- Define constants for valid values (e.g.,
SortByName,SortByCreatedAt,SortOrderAsc,ProvisioningTypeInternal) - Add a
Validate()method to the AgentFilter type to check field values before database queries - Return validation errors early in the controller layer
This will improve type safety and provide better error messages to API consumers.
🤖 Prompt for AI Agents
In agent-manager-service/models/agent_filter.go around lines 19 to 27, the
AgentFilter struct accepts free-form strings for SortBy, SortOrder, and
ProvisioningType which can lead to silent failures; define typed constants for
allowed values (e.g., SortByName, SortByCreatedAt, SortOrderAsc/Desc,
ProvisioningTypeInternal/External), add a Validate() method on AgentFilter that
checks these fields (and limits/offset bounds) returning a descriptive error
when a value is invalid, and update callers (controllers) to call Validate() and
return the validation error to clients before proceeding to repository queries.
Naming inconsistency between default value and expected format.
The comment on line 23 documents SortBy values as camelCase ("name", "createdAt", "updatedAt"), but DefaultAgentFilter() at line 32 uses snake_case "created_at". The repository at lines 84-92 of agent-manager-service/repositories/agent.go validates against camelCase values and maps them to snake_case column names.
This mismatch will cause the default filter to skip the switch statement and fall through to "created_at" being used directly as the sort column, which works accidentally but is inconsistent with the documented API contract.
🔎 Apply this diff to align the default with the expected format:
func DefaultAgentFilter() AgentFilter {
return AgentFilter{
- SortBy: "created_at",
+ SortBy: "createdAt",
SortOrder: "desc",
Limit: 20,
Offset: 0,
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In agent-manager-service/models/agent_filter.go around lines 19 to 27 (and
DefaultAgentFilter at line 32), the SortBy default uses snake_case
("created_at") while the documented/validated values are camelCase ("name",
"createdAt", "updatedAt"); update the default SortBy to "createdAt" (or adjust
the comment/validation to match) so the default follows the API contract and
will be handled by the repository's switch that maps camelCase to snake_case
column names.
- Add validation constants for SortBy, SortOrder, ProvisioningType - Add IsValidSortBy, IsValidSortOrder, IsValidProvisioningType helpers - Fix DefaultAgentFilter to use camelCase 'createdAt' (was snake_case) - Fix SQL injection risk by using constants in ORDER BY clause - Add controller validation with proper error responses Addresses CodeRabbit review feedback on PR wso2#71
- Add AgentSortBy type for sortBy field - Use ProvisioningType for provisioningType field - Improves type safety and IDE autocomplete Addresses CodeRabbit nitpick on PR wso2#71
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
agent-manager-service/repositories/agent.go (1)
82-99: Consider using GORM's Order method directly for clarity.While the current implementation is safe (sortColumn is validated via switch, sortOrder via constant), using
fmt.Sprintffor SQL construction is less clear than using GORM's methods directly.🔎 Apply this diff for clearer sorting:
// Apply sorting - map camelCase API values to snake_case DB columns sortColumn := "created_at" switch filter.SortBy { case models.SortByName: sortColumn = "name" case models.SortByUpdatedAt: sortColumn = "updated_at" case models.SortByCreatedAt: sortColumn = "created_at" } - // Validate sortOrder to prevent SQL injection - only allow known values - sortOrder := "DESC" - if filter.SortOrder == models.SortOrderAsc { - sortOrder = "ASC" + // Apply sort order using GORM's Order method + if filter.SortOrder == models.SortOrderAsc { + query = query.Order(sortColumn + " ASC") + } else { + query = query.Order(sortColumn + " DESC") } - - query = query.Order(fmt.Sprintf("%s %s", sortColumn, sortOrder))This approach avoids string formatting for SQL and makes the intent clearer, as suggested in the previous review.
🧹 Nitpick comments (1)
agent-manager-service/models/agent_filter.go (1)
35-43: Consider adding a Validate() method to the AgentFilter struct.While validation helpers exist (lines 55-68), encapsulating validation in a method on AgentFilter would improve the API and ensure consistent validation across all callers.
🔎 Add a Validate method:
+// Validate checks if all filter fields have valid values +func (f AgentFilter) Validate() error { + if !IsValidSortBy(f.SortBy) { + return fmt.Errorf("invalid sortBy value: %s (expected: name, createdAt, or updatedAt)", f.SortBy) + } + if !IsValidSortOrder(f.SortOrder) { + return fmt.Errorf("invalid sortOrder value: %s (expected: asc or desc)", f.SortOrder) + } + if !IsValidProvisioningType(f.ProvisioningType) { + return fmt.Errorf("invalid provisioningType value: %s (expected: internal or external)", f.ProvisioningType) + } + if f.Limit < 0 { + return fmt.Errorf("limit must be non-negative") + } + if f.Offset < 0 { + return fmt.Errorf("offset must be non-negative") + } + return nil +}Then update the controller to call
filter.Validate()and return the error to API consumers, providing better error messages.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
agent-manager-service/controllers/agent_controller.go(2 hunks)agent-manager-service/models/agent_filter.go(1 hunks)agent-manager-service/repositories/agent.go(2 hunks)console/workspaces/libs/types/src/api/agents.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- agent-manager-service/controllers/agent_controller.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-18T16:41:56.601Z
Learnt from: hanzjk
Repo: wso2/ai-agent-management-platform PR: 70
File: agent-manager-service/services/agent_manager.go:978-980
Timestamp: 2025-12-18T16:41:56.601Z
Learning: In the agent-manager-service, enforce that external agents cannot have a SubType; only internal/managed agents may have SubType (e.g., chat-api, custom-api). When converting external agents to AgentResponse, the Type payload should include only the Type field and should not expose SubType. Implement this in the conversion/serialization path (likely in services/agent_manager.go) and update related DTO mappings or serializers. Add unit tests to verify internal vs external agent handling and ensure SubType is omitted for external agents.
Applied to files:
agent-manager-service/repositories/agent.goagent-manager-service/models/agent_filter.go
🧬 Code graph analysis (3)
agent-manager-service/repositories/agent.go (4)
agent-manager-service/models/agent_filter.go (4)
AgentFilter(36-43)SortByUpdatedAt(24-24)SortByCreatedAt(23-23)SortOrderAsc(27-27)agent-manager-service/models/agent.go (1)
Agent(58-70)agent-manager-service/db/tx_utils.go (1)
DB(31-46)console/workspaces/libs/types/src/api/agents.ts (1)
ProvisioningType(38-38)
console/workspaces/libs/types/src/api/agents.ts (1)
console/workspaces/libs/types/src/api/common.ts (1)
ListQuery(27-30)
agent-manager-service/models/agent_filter.go (1)
console/workspaces/libs/types/src/api/agents.ts (1)
ProvisioningType(38-38)
🔇 Additional comments (9)
console/workspaces/libs/types/src/api/agents.ts (1)
64-73: LGTM! Clean type definitions aligned with backend.The interface properly extends ListQuery and adds optional filter fields that match the backend AgentFilter model. The AgentSortBy type and inline union types ensure type safety for valid filter values.
agent-manager-service/repositories/agent.go (5)
32-32: LGTM! Appropriate method signature.The new method signature properly extends the interface to support filtering while maintaining consistency with existing methods and returning the total count needed for pagination.
66-75: LGTM! Safe parameterized queries.The search and provisioning type filters correctly use parameterized queries to prevent SQL injection. The ILIKE operator provides case-insensitive search across the specified fields.
Note: ILIKE is PostgreSQL-specific. This is acceptable given the PR summary mentions PostgreSQL, but ensure the database requirement is documented.
77-80: LGTM! Correct count query placement.The total count is correctly calculated after filters but before pagination, providing accurate pagination metadata.
101-107: LGTM! Correct pagination implementation.Pagination is correctly applied after counting and sorting, with appropriate conditional checks.
109-115: LGTM! Proper query execution and error handling.The method correctly preloads the AgentDetails relation and returns both the filtered agents and total count with appropriate error handling.
agent-manager-service/models/agent_filter.go (3)
19-33: LGTM! Well-defined constants aligned with frontend.The constants provide type-safe values for filter options and align perfectly with the frontend TypeScript types, ensuring API contract consistency.
45-53: LGTM! Sensible defaults with constants.The default filter uses the proper constants (SortByCreatedAt, SortOrderDesc) and provides reasonable pagination defaults. The past review issue regarding naming inconsistency has been resolved.
55-68: LGTM! Correct validation helpers.The validation functions properly check against defined constants and correctly allow empty strings for optional filters. They provide the building blocks for validation, though a Validate() method on the struct (as suggested earlier) would provide better encapsulation.
1 participant
Summary
Changes
Backend (amp-api)
AgentFiltermodel with search, sorting, and pagination optionsListAgentsWithFilterrepository method with case-insensitive searchFrontend (console)
ListAgentsQuerytype with search parametersAgentsListcomponentDatabase Schema Requirements
Required for filtering (this PR)
The
provisioning_typecolumn must exist on theagentstable:Required for internal agent creation
The
internal_agentstable must exist with proper defaults:Note: New databases using the full migration set (migrations 005 and 007) will have these tables and columns correctly configured.
API Changes
New query parameters for
GET /orgs/{org}/projects/{project}/agents:searchprovisioningTypesortBysortOrderTest Plan
provisioningTypefilter works via APICloses #68
Summary by CodeRabbit
New Features
Behavior Changes
✏️ Tip: You can customize this high-level summary in your review settings.