Add proper server-side query support for instances #26

abrookins · 2025-12-13T01:44:37Z

In RAAE-1176, we moved instance data from serialized JSON in strings (yikes) to hashes. However, we left all the querying logic in place... which was basically "filter this list of dictionaries." This PR exposes more query abilities related to instances and uses proper RedisVL queries to serve them.

Copilot

Pull request overview

This PR implements proper server-side query support for Redis instances, replacing the previous client-side filtering approach with RedisVL queries. This follows the migration from serialized JSON strings to Redis hashes (RAAE-1176), now leveraging RediSearch capabilities for efficient filtering and pagination.

Key Changes:

Implemented query_instances() function with support for filtering by environment, usage, status, instance_type, user_id, and text search on instance names
Added pagination support with limit/offset parameters and total count tracking
Updated all API endpoints and UI components to use the new query-based approach with structured response format

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`redis_sre_agent/core/instances.py`	Added `query_instances()` with RedisVL filter expressions and pagination; refactored `get_instance_by_id()` and `get_instance_by_name()` to use direct lookups
`redis_sre_agent/api/instances.py`	Updated `/instances` endpoint to accept query parameters and return paginated response with `InstanceListResponse` model
`redis_sre_agent/mcp_server/server.py`	Enhanced `redis_sre_list_instances()` tool with filter parameters and updated documentation
`ui/src/services/sreAgentApi.ts`	Added `ListInstancesParams` and `InstanceListResponse` interfaces; updated `listInstances()` to build query parameters from filter options
`ui/src/pages/Instances.tsx`	Refactored to use server-side filtering with `buildFilterParams()` and handle paginated response structure
`ui/src/pages/Dashboard.tsx`	Updated to access `response.instances` from new response structure
`ui/src/pages/Triage.tsx`	Updated to access `response.instances` from new response structure
`ui/src/pages/Schedules.tsx`	Updated to access `response.instances` from new response structure
`tests/unit/api/test_instances_api.py`	Updated tests to mock `query_instances()` instead of `get_instances()` and added test for filter parameters
`tests/unit/mcp_server/test_mcp_server.py`	Updated tests to use `InstanceQueryResult` and added test coverage for filter parameters

Comments suppressed due to low confidence (1)

ui/src/pages/Instances.tsx:1037

The client-side filtering is redundant and adds unnecessary complexity. Since server-side filtering is now implemented, consider removing the client-side filter logic. If you need it as a "safety net," this suggests a lack of confidence in the server-side implementation and could mask bugs. The comment also indicates that in a race condition, the client might show incorrect results (stale filtered data before the new server response arrives).

  // Filtering is now done server-side, but we keep client-side filtering
  // as a safety net in case filters change before the API response arrives.
  // In most cases, instances will already be filtered correctly from the server.
  const filteredInstances = instances.filter((instance) => {
    const environmentMatch =
      selectedEnvironment === "all" ||
      instance.environment === selectedEnvironment;
    const usageMatch =
      selectedUsage === "all" || instance.usage === selectedUsage;
    return environmentMatch && usageMatch;
  });

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-13T01:52:16Z

redis_sre_agent/api/instances.py

+        description="Filter by type (oss_single, oss_cluster, redis_enterprise, redis_cloud)",
+    ),
+    user_id: Optional[str] = Query(None, description="Filter by user ID"),
+    search: Optional[str] = Query(None, description="Search by instance name"),


The new search parameter is not covered by tests. Consider adding test cases for the search functionality to verify text search behavior works as expected, especially for partial matches and edge cases like empty strings or special characters.

Copilot · 2025-12-13T01:52:17Z

redis_sre_agent/core/instances.py

+        # Note: name is indexed as TEXT, so we search for the exact term
+        fq = FilterQuery(
+            return_fields=["data"],
+            num_results=1,
+        )
+        fq.set_filter(Text("name") % instance_name)


The comment states "Use exact text match on name field" but the implementation at line 796 uses the % operator which performs fuzzy text search in RediSearch, not exact matching. This comment is misleading and should be corrected to reflect the actual behavior (fuzzy/prefix matching) or the implementation should be changed to perform exact matching.

Suggested change

# Note: name is indexed as TEXT, so we search for the exact term

fq = FilterQuery(

return_fields=["data"],

num_results=1,

)

fq.set_filter(Text("name") % instance_name)

# Note: name is indexed as TEXT, so we search for the exact term using ==

fq = FilterQuery(

return_fields=["data"],

num_results=1,

)

fq.set_filter(Text("name") == instance_name)

Yes, this should be a TAG field with a TAG filter.

Copilot · 2025-12-13T01:52:17Z

redis_sre_agent/core/instances.py

+            return_fields=["data"],
+            num_results=1,
+        )
+        fq.set_filter(Text("name") % instance_name)


The text search implementation is inconsistent between search parameter (line 418) and get_instance_by_name. The search parameter uses wildcard matching with Text("name") % f"*{search}*" for partial matches, but get_instance_by_name uses Text("name") % instance_name without wildcards. This means get_instance_by_name will perform a partial match search when it should do an exact match lookup. Consider using Tag field for exact name matching or explicitly handle exact vs partial matching differently.

Suggested change

fq.set_filter(Text("name") % instance_name)

fq.set_filter(Text("name") % f'"{instance_name}"')

instance_name should be a TAG field with a TAG filter here

Copilot · 2025-12-13T01:52:17Z

redis_sre_agent/core/instances.py

+            # Text search on name field (supports prefix matching)
+            name_filter = Text("name") % f"*{search}*"


The wildcard pattern f"*{search}*" is being passed to Text field search. In RediSearch, this pattern syntax may not work as intended for TEXT fields. Text fields typically use word-based tokenization and prefix matching. Consider using proper RediSearch query syntax for fuzzy/wildcard text searches, or ensure this pattern is compatible with how the name field is indexed.

Suggested change

# Text search on name field (supports prefix matching)

name_filter = Text("name") % f"*{search}*"

# Text search on name field (supports prefix matching only)

name_filter = Text("name") % f"{search}*"

Copilot · 2025-12-13T01:52:17Z

redis_sre_agent/core/instances.py

+        fq = FilterQuery(
+            return_fields=["data"],
+            num_results=limit,
+        ).sort_by("updated_at", asc=False)
+
+        if filter_expr is not None:
+            fq.set_filter(filter_expr)
+
+        fq.paging(offset, limit)


The paging call uses positional arguments fq.paging(offset, limit), but this appears to be called after setting num_results. This could lead to confusion about which parameter controls pagination. Verify that paging() correctly overrides the num_results set on line 439, or consider removing the redundant num_results parameter from the FilterQuery constructor.

Copilot · 2025-12-13T01:52:17Z

ui/src/pages/Instances.tsx


 const Instances = () => {
  const [instances, setInstances] = useState<RedisInstance[]>([]);
+  const [totalCount, setTotalCount] = useState(0);


Unused variable totalCount.

Suggested change

const [totalCount, setTotalCount] = useState(0);

Copilot · 2025-12-13T01:52:18Z

redis_sre_agent/core/instances.py

 from datetime import datetime, timezone
 from enum import Enum
-from typing import Any, Dict, List, Optional
+from typing import Any, Dict, List, Optional, Tuple


Import of 'Tuple' is not used.

Suggested change

from typing import Any, Dict, List, Optional, Tuple

from typing import Any, Dict, List, Optional

Add proper server-side query support for instances

f0dc299

abrookins requested a review from Copilot December 13, 2025 01:47

Copilot started reviewing on behalf of abrookins December 13, 2025 01:47 View session

Copilot AI reviewed Dec 13, 2025

View reviewed changes

abrookins added 3 commits December 12, 2025 19:27

lint

786587e

Use a TAG field for instance names

5f39caf

Note why we are using FilterExpression

df9d7c4

abrookins merged commit c04f906 into main Dec 16, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add proper server-side query support for instances #26

Add proper server-side query support for instances #26

Uh oh!

abrookins commented Dec 13, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

abrookins Dec 15, 2025

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

abrookins Dec 15, 2025

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

Copilot AI Dec 13, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	fq.set_filter(Text("name") % instance_name)
	fq.set_filter(Text("name") % f'"{instance_name}"')

		# Text search on name field (supports prefix matching)
		name_filter = Text("name") % f"{search}"

	from typing import Any, Dict, List, Optional, Tuple
	from typing import Any, Dict, List, Optional

Add proper server-side query support for instances #26

Add proper server-side query support for instances #26

Uh oh!

Conversation

abrookins commented Dec 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

abrookins Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

abrookins Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abrookins commented Dec 13, 2025 •

edited

Loading