feat: add complex arbitrary filtering on all objects by dr-frmr · Pull Request #140 · plastic-labs/honcho

dr-frmr · 2025-06-24T16:45:08Z

Summary by CodeRabbit

New Features
- Introduced advanced recursive filtering for API queries with logical (AND, OR, NOT) and comparison operators, plus wildcard support.
- Enhanced filtering capabilities on metadata fields for workspaces, peers, sessions, messages, and documents.
- Standardized pagination across multiple API endpoints for improved consistency.
- Removed the session active state attribute for clearer session status representation.
Bug Fixes
- Improved accuracy and consistency of filter application in API endpoints.
Tests
- Expanded test coverage for advanced filtering, including logical operators, comparison filters, wildcards, and edge cases.
- Added tests for logical NOT filtering and ensured backward compatibility with filter formats.
- Updated existing tests to reflect new filter structures and behaviors.

coderabbitai · 2025-06-24T16:45:14Z

"""

Walkthrough

A comprehensive, recursive filtering system for SQLAlchemy queries was introduced, supporting logical and comparison operators, nested JSONB metadata, and wildcards. All query functions now use this centralized filter logic. Pagination imports and usages were updated to use apaginate. Test suites were expanded and updated to validate advanced filter handling and new filter formats. Additionally, the is_active attribute was removed from SessionGet.

Changes

File(s)	Change Summary
`src/crud.py`	Replaced explicit metadata filter checks with centralized `apply_filter` for all queries; removed `is_active` parameters in session queries.
`src/utils/filter.py`	Added `apply_filter` function supporting logical/comparison operators, nested JSONB filtering, wildcards, and field mappings.
`src/routers/messages.py`, `src/routers/peers.py`, `src/routers/sessions.py`, `src/routers/workspaces.py`	Changed pagination import and usage from `paginate` to `apaginate` throughout all router endpoints; removed `is_active` handling in session-related routers.
`tests/routes/test_messages.py`	Updated filter structure in tests to use `"metadata"` key; added/expanded tests for logical operators (`AND`, `OR`) in filters.
`tests/routes/test_peers.py`	Updated filter structure to use `"metadata"` key; added test for logical `NOT` operator in peer filters; removed `is_active` session test.
`tests/routes/test_sessions.py`	Changed filter structure in session listing test to nest under `"metadata"`; tightened assertion on result count; removed `is_active` session test; updated inactive session filter usage.
`tests/routes/test_workspaces.py`	Changed filter structure in workspace listing test to nest under `"metadata"`; tightened assertion on result count.
`tests/test_advanced_filters.py`	Added new test module for advanced filtering: logical/comparison operators, wildcards, nested filters, edge cases, and compatibility.
`src/schemas.py`	Removed `is_active` attribute from `SessionGet`; minor formatting changes in other Pydantic models.
`src/exceptions.py`	Added new `FilterError` exception class for invalid filter configurations with HTTP 422 status.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API_Router
    participant CRUD
    participant DB

    Client->>API_Router: Request with filter (possibly complex)
    API_Router->>CRUD: Call query function with filter
    CRUD->>CRUD: apply_filter(stmt, filter)
    CRUD->>CRUD: Recursively build conditions (logical, comparison, metadata)
    CRUD->>DB: Execute filtered SQLAlchemy query
    DB-->>CRUD: Return results
    CRUD-->>API_Router: Return results
    API_Router-->>Client: Paginated response (via apaginate)

Suggested reviewers

VVoruganti

Poem

🐇
Filters now dance with logic and flair,
ANDs and ORs mingle in the spring air.
Metadata’s nested, wildcards take flight,
Pagination’s apace, all queries just right.
With tests all expanded, the future looks bright—
A rabbit’s delight in the filtering night!
✨
"""

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 82c3c8b and d3cb9fe.

📒 Files selected for processing (6)

src/crud.py (14 hunks)
src/routers/messages.py (2 hunks)
src/routers/peers.py (5 hunks)
src/routers/sessions.py (4 hunks)
src/routers/workspaces.py (3 hunks)
src/utils/filter.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

`src/routers/*.py`: Define API endpoints in these files.

src/routers/*.py: Define API endpoints in these files.

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

src/routers/messages.py
src/routers/peers.py
src/routers/workspaces.py
src/routers/sessions.py

`src/crud.py`: Implement database operations in this file.

src/crud.py: Implement database operations in this file.

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

src/crud.py

🧠 Learnings (5)

src/routers/messages.py (1)

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: tests/routes/test_messages.py:7-39
Timestamp: 2025-06-18T15:47:44.327Z
Learning: In FastAPI tests, it's common to use @pytest.mark.asyncio with a synchronous TestClient. The async marker is used when tests contain async database operations (like await db_session.commit()), but the HTTP client calls (client.post(), client.get()) should NOT be awaited since TestClient is synchronous. Only the actual async operations like database commits need awaiting.

src/routers/peers.py (5)

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:36-77
Timestamp: 2025-06-19T14:07:32.309Z
Learning: The migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` properly handles foreign key constraints during table updates by explicitly dropping foreign key constraints before making schema changes, preventing constraint violations during the migration process.

Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T22:29:51.942Z
Learning: Define global exception handlers in the FastAPI app entrypoint (main.py) for consistent error responses.

Learnt from: dr-frmr
PR: plastic-labs/honcho#131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The `get_or_create_session` function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.

src/routers/sessions.py (2)

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:918-942
Timestamp: 2025-06-19T14:32:02.934Z
Learning: The queue and active_queue_sessions tables in the migration script `migrations/versions/d429de0e5338_adopt_peer_paradigm.py` are expected to remain small, so per-row updates are acceptable and performance optimizations like set-based updates are not necessary.

Learnt from: dr-frmr
PR: plastic-labs/honcho#131
File: src/routers/sessions.py:206-213
Timestamp: 2025-06-18T20:42:06.458Z
Learning: The `get_or_create_session` function in this codebase is designed to handle both session creation and adding peers to existing sessions. When called with peers, it will add those peers to an existing session rather than creating a duplicate session.

src/utils/filter.py (3)

Learnt from: VVoruganti
PR: plastic-labs/honcho#140
File: src/utils/filter.py:51-53
Timestamp: 2025-06-26T15:20:35.408Z
Learning: When renaming parameters to avoid shadowing built-ins, prefer `filters` over `filter_dict` for filter/filtering parameters as it's cleaner and more concise.

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.

Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T22:29:51.942Z
Learning: Use JSONB metadata fields in the database for extensibility and flexible data storage.

src/crud.py (1)

Learnt from: VVoruganti
PR: plastic-labs/honcho#140
File: src/utils/filter.py:51-53
Timestamp: 2025-06-26T15:20:35.408Z
Learning: When renaming parameters to avoid shadowing built-ins, prefer `filters` over `filter_dict` for filter/filtering parameters as it's cleaner and more concise.

🧬 Code Graph Analysis (5)

src/routers/messages.py (1)

src/crud.py (1)

get_messages (1248-1328)

src/routers/peers.py (1)

src/crud.py (3)

get_peers (240-250)

get_sessions_for_peer (290-321)

get_messages_for_peer (1382-1402)

src/routers/workspaces.py (1)

src/crud.py (1)

get_all_workspaces (88-101)

src/routers/sessions.py (1)

src/crud.py (1)

get_sessions (329-342)

src/utils/filter.py (1)

src/exceptions.py (1)

FilterError (69-73)

🪛 Pylint (3.3.7)

src/routers/messages.py

[error] 6-6: Unable to import 'fastapi_pagination.ext.sqlalchemy'

(E0401)

src/routers/peers.py

[error] 14-14: Unable to import 'fastapi_pagination.ext.sqlalchemy'

(E0401)

src/routers/workspaces.py

[error] 5-5: Unable to import 'fastapi_pagination.ext.sqlalchemy'

(E0401)

src/routers/sessions.py

[error] 5-5: Unable to import 'fastapi_pagination.ext.sqlalchemy'

(E0401)

src/utils/filter.py

[convention] 199-199: Line too long (106/100)

(C0301)

[convention] 215-215: Line too long (103/100)

(C0301)

[convention] 221-221: Line too long (106/100)

(C0301)

[convention] 276-276: Line too long (101/100)

(C0301)

[convention] 280-280: Line too long (127/100)

(C0301)

[convention] 327-327: Line too long (123/100)

(C0301)

[convention] 337-337: Line too long (134/100)

(C0301)

[convention] 463-463: Line too long (110/100)

(C0301)

[convention] 480-480: Line too long (102/100)

(C0301)

[convention] 503-503: Line too long (138/100)

(C0301)

[convention] 1-1: Missing module docstring

(C0114)

[error] 6-6: Unable to import 'sqlalchemy'

(E0401)

[error] 7-7: Unable to import 'sqlalchemy.types'

(E0401)

[refactor] 170-175: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 100-100: Too many branches (22/12)

(R0912)

[refactor] 217-225: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 222-225: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 227-230: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 261-281: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 274-281: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 330-338: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 406-411: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 481-500: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 435-517: Too many nested blocks (8/5)

(R1702)

[refactor] 435-517: Too many nested blocks (6/5)

(R1702)

[refactor] 520-525: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 414-414: Too many branches (32/12)

(R0912)

[refactor] 414-414: Too many statements (61/50)

(R0915)

🪛 Ruff (0.11.9)

src/routers/sessions.py

94-94: Trailing comma missing

Add trailing comma

(COM812)

src/utils/filter.py

51-51: Trailing comma missing

Add trailing comma

(COM812)

100-100: _build_filter_conditions is too complex (22 > 10)

(C901)

101-101: Trailing comma missing

Add trailing comma

(COM812)

118-120: Avoid specifying long messages outside the exception class

(TRY003)

119-119: Trailing comma missing

Add trailing comma

(COM812)

131-133: Avoid specifying long messages outside the exception class

(TRY003)

132-132: Trailing comma missing

Add trailing comma

(COM812)

144-144: Avoid specifying long messages outside the exception class

(TRY003)

146-148: Avoid specifying long messages outside the exception class

(TRY003)

147-147: Trailing comma missing

Add trailing comma

(COM812)

154-154: Trailing comma missing

Add trailing comma

(COM812)

172-172: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

179-179: Dynamically typed expressions (typing.Any) are disallowed in value

(ANN401)

179-179: Trailing comma missing

Add trailing comma

(COM812)

198-200: Avoid specifying long messages outside the exception class

(TRY003)

199-199: Trailing comma missing

Add trailing comma

(COM812)

204-204: Avoid specifying long messages outside the exception class

(TRY003)

219-219: Unnecessary else after return statement

Remove unnecessary else

(RET505)

224-224: Unnecessary else after return statement

Remove unnecessary else

(RET505)

229-229: Unnecessary else after return statement

Remove unnecessary else

(RET505)

234-234: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

234-234: Trailing comma missing

Add trailing comma

(COM812)

263-263: Unnecessary else after return statement

Remove unnecessary else

(RET505)

268-268: Consider moving this statement to an else block

(TRY300)

272-272: Consider moving this statement to an else block

(TRY300)

278-278: Unnecessary else after return statement

Remove unnecessary else

(RET505)

279-281: Avoid specifying long messages outside the exception class

(TRY003)

280-280: Trailing comma missing

Add trailing comma

(COM812)

283-285: Avoid specifying long messages outside the exception class

(TRY003)

284-284: Use explicit conversion flag

Replace with conversion flag

(RUF010)

284-284: Trailing comma missing

Add trailing comma

(COM812)

289-289: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

289-289: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

289-289: Trailing comma missing

Add trailing comma

(COM812)

305-305: Avoid specifying long messages outside the exception class

(TRY003)

326-328: Avoid specifying long messages outside the exception class

(TRY003)

327-327: Use explicit conversion flag

Replace with conversion flag

(RUF010)

327-327: Trailing comma missing

Add trailing comma

(COM812)

335-335: Unnecessary else after return statement

Remove unnecessary else

(RET505)

336-338: Avoid specifying long messages outside the exception class

(TRY003)

337-337: Trailing comma missing

Add trailing comma

(COM812)

346-346: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

346-346: Trailing comma missing

Add trailing comma

(COM812)

381-381: Trailing comma missing

Add trailing comma

(COM812)

408-408: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

414-414: _build_comparison_conditions is too complex (27 > 10)

(C901)

415-415: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

415-415: Trailing comma missing

Add trailing comma

(COM812)

432-432: Trailing comma missing

Add trailing comma

(COM812)

438-438: Avoid specifying long messages outside the exception class

(TRY003)

452-452: Avoid specifying long messages outside the exception class

(TRY003)

462-464: Avoid specifying long messages outside the exception class

(TRY003)

463-463: Trailing comma missing

Add trailing comma

(COM812)

483-483: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

491-493: Avoid specifying long messages outside the exception class

(TRY003)

492-492: Trailing comma missing

Add trailing comma

(COM812)

502-504: Avoid specifying long messages outside the exception class

(TRY003)

503-503: Trailing comma missing

Add trailing comma

(COM812)

522-522: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

557-557: Naive datetime constructed using datetime.datetime.strptime() without %z

(DTZ007)

🔇 Additional comments (29)

src/routers/workspaces.py (3)

5-5: LGTM! Consistent pagination function update.

The import change from paginate to apaginate aligns with the async pagination refactoring across all router modules.

68-71: LGTM! Consistent parameter naming and pagination updates.

The changes properly align with the centralized filtering refactor:

Parameter name changed from filter to filters to avoid shadowing the built-in

Function call updated to use apaginate for async operations

Correctly passes filters to the CRUD layer

107-107: LGTM! Consistent pagination function usage.

The function call correctly uses apaginate consistent with other router updates.

src/routers/messages.py (2)

6-6: LGTM! Consistent pagination function update.

The import change from paginate to apaginate aligns with the async pagination refactoring across all router modules.

324-337: LGTM! Proper variable naming and consistent parameter updates.

The changes properly implement the filtering refactor:

Variable renamed from filter to filters to avoid shadowing the built-in function

Parameter correctly passed to crud.get_messages as filters

Function call updated to use apaginate for async operations

These changes align with the centralized filtering system and async pagination updates.

src/routers/peers.py (5)

14-14: LGTM! Consistent pagination function update.

The import change from paginate to apaginate aligns with the async pagination refactoring.

54-57: LGTM! Consistent parameter naming and pagination updates.

The changes properly align with the centralized filtering refactor:

Parameter name changed from filter to filters to avoid shadowing the built-in

Function call updated to use apaginate for async operations

132-144: LGTM! Simplified filter handling with removal of deprecated is_active.

The changes correctly:

Remove the deprecated is_active parameter handling (consistent with schema updates)

Rename variable from filter to filters to avoid shadowing built-ins

Use apaginate for async pagination

Pass filters parameter to the CRUD layer

These changes align with the centralized filtering architecture and removal of session active-state filtering.

272-285: LGTM! Consistent variable naming and pagination updates.

The changes properly implement the filtering refactor with variable renamed to filters and apaginate usage.

335-335: LGTM! Consistent pagination function usage.

The function call correctly uses apaginate consistent with other router updates.

src/routers/sessions.py (3)

5-5: LGTM! Consistent pagination function update.

The import change from paginate to apaginate aligns with the async pagination refactoring.

355-355: LGTM! Consistent pagination function usage.

The function call correctly uses apaginate consistent with other router updates.

462-462: LGTM! Consistent pagination function usage.

The function call correctly uses apaginate consistent with other router updates.

src/utils/filter.py (7)

1-98: LGTM! Comprehensive filtering utility with excellent design.

This is a well-architected filtering system that provides:

✅ Robust functionality: Supports logical operators (AND, OR, NOT), comparison operators, and wildcards
✅ Type safety: Proper handling of JSONB fields with safe type casting
✅ Security: Datetime validation to prevent SQL injection
✅ Flexibility: Column mapping for user-facing vs internal field names
✅ Error handling: Clear error messages with FilterError exceptions

The main apply_filter function provides a clean interface and the modular design supports complex filtering scenarios while maintaining backward compatibility.

100-176: LGTM! Well-structured recursive filter condition building.

The logical operator handling is correctly implemented:

✅ AND/OR operators: Properly validate list types and recursively build conditions
✅ NOT operator: Correctly applies negation to individual conditions then combines with AND
✅ Error handling: Clear validation with descriptive error messages
✅ Recursion: Safely handles nested logical structures

The complexity flagged by static analysis is justified for the comprehensive logical operator support.

178-231: LGTM! Robust field condition building with proper column mapping.

The implementation correctly:

✅ Column mapping: Handles different mappings for Message vs other models
✅ Security: Validates allowed columns and prevents unauthorized filtering
✅ JSONB support: Properly handles metadata and configuration fields
✅ Wildcards: Correctly treats * as no-op filters
✅ Type detection: Distinguishes between comparison dicts and regular values

The column existence validation prevents runtime errors.

233-286: LGTM! Safe numeric casting with comprehensive type handling.

The _safe_numeric_cast function provides excellent type safety:

✅ Boolean handling: Correctly converts to PostgreSQL JSONB string representation
✅ Safe casting: Uses CASE WHEN to handle empty strings and NULL values gracefully
✅ Type coercion: Attempts int, then float, then string comparison
✅ Error handling: Clear error messages for invalid numeric values
✅ Fallback logic: Graceful degradation to string comparison for non-numeric values

This addresses the complex reality of JSONB type handling in PostgreSQL.

288-343: LGTM! Comprehensive JSONB field comparison handling.

The function correctly implements:

✅ Operator validation: Ensures only supported operators are used
✅ Numeric operations: Leverages safe casting for proper type comparisons
✅ IN operator: Properly handles iterables with wildcard support
✅ Text operations: Case-insensitive contains for string matching
✅ Wildcard support: Correctly handles * in various contexts

The error handling provides clear feedback for invalid configurations.

345-412: LGTM! Sophisticated nested metadata condition building.

The implementation handles complex JSONB nested filtering:

✅ Recursive structure: Properly detects and handles comparison operators in nested fields
✅ JSONB contains: Uses appropriate PostgreSQL JSONB operators for object matching
✅ Condition combination: Correctly combines field conditions with AND logic
✅ Wildcard support: Handles wildcards in nested contexts

This enables powerful filtering on complex JSONB structures while maintaining type safety.

528-569: LGTM! Robust datetime validation preventing SQL injection.

The _validate_datetime_string function provides excellent security:

✅ Multiple formats: Supports common datetime formats including ISO and UTC variants
✅ Fallback parsing: Uses fromisoformat as additional parsing option
✅ Safe validation: Returns None for invalid inputs, letting caller handle errors
✅ Security focus: Prevents SQL injection by validating before allowing in queries

This addresses the datetime validation concerns raised in previous reviews.

src/crud.py (9)

20-20: LGTM: Clean import addition for centralized filtering.

The import of apply_filter supports the refactoring to centralize filtering logic across all CRUD operations.

89-89: LGTM: Parameter renamed to avoid shadowing built-in.

Renaming filter to filters correctly avoids shadowing Python's built-in filter function, improving code clarity and preventing potential issues.

Also applies to: 96-96

99-99: LGTM: Centralized filtering implementation.

The use of apply_filter centralizes complex filtering logic and replaces previous custom implementations, improving maintainability.

242-242: LGTM: Consistent parameter renaming and filtering.

The changes maintain consistency with the filtering refactoring pattern used throughout the file.

Also applies to: 246-246

293-293: LGTM: Session filtering refactored correctly.

The function signature correctly removes the is_active parameter (aligning with schema changes) and consistently applies the new filtering approach.

Also applies to: 301-301, 317-317

331-331: LGTM: Consistent session filtering updates.

The changes maintain the same pattern as other functions, removing is_active filtering and using centralized filter logic.

Also applies to: 338-338

1252-1252: LGTM: Message filtering consistently implemented.

The filtering is correctly applied at multiple points in the function logic, maintaining proper order with limit operations and ensuring consistent behavior across different execution paths.

Also applies to: 1266-1266, 1282-1282, 1312-1312, 1322-1322

1386-1386: LGTM: Peer message filtering updated correctly.

The function maintains the same filtering pattern used throughout the codebase refactoring.

Also applies to: 1395-1395

1518-1518: LGTM: Document query filtering integrated properly.

The filtering is applied at the correct position in the query chain, after the base conditions but before ordering and limiting operations.

Also applies to: 1535-1535

✨ Finishing Touches

📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/crud.py (2)
81-139: Consider the complexity and performance implications of deeply nested filters.

While the recursive implementation correctly handles logical operators, the function complexity (18 branches) could impact performance with deeply nested filters. Consider:

Adding a maximum recursion depth check to prevent stack overflow

Monitoring query performance with complex filters

Adding query plan analysis for JSONB operations

Can you verify the maximum expected nesting depth in production use cases?

414-414: Consistent filter application across all query functions.

All query functions have been properly updated to use the new apply_filter function. However, consider adding filter structure validation to prevent malformed filters from causing runtime errors.

Consider adding a filter validation function:
def validate_filter_structure(filter_dict: dict[str, Any]) -> None:
    """Validate filter structure before applying."""
    allowed_keys = {"AND", "OR", "NOT"}
    comparison_ops = {"gte", "lte", "gt", "lt", "ne", "in", "contains", "icontains"}
    
    # Add validation logic here
    pass
Can you verify if there's any input validation on the API layer for these filters?

Also applies to: 561-561, 637-637, 662-662, 1601-1601, 1631-1631, 1641-1641, 1714-1714, 1854-1854

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a67fb3d and d1310e2.

📒 Files selected for processing (10)

src/crud.py (10 hunks)
src/routers/messages.py (2 hunks)
src/routers/peers.py (5 hunks)
src/routers/sessions.py (4 hunks)
src/routers/workspaces.py (3 hunks)
tests/routes/test_messages.py (2 hunks)
tests/routes/test_peers.py (1 hunks)
tests/routes/test_sessions.py (1 hunks)
tests/routes/test_workspaces.py (1 hunks)
tests/test_advanced_filters.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

`src/routers/*.py`: API endpoints should be implemented in the src/routers/ directory, with each resource (workspaces, peers, sessions, messages, keys) having its own module.

src/routers/*.py: API endpoints should be implemented in the src/routers/ directory, with each resource (workspaces, peers, sessions, messages, keys) having its own module.

src/routers/sessions.py
src/routers/workspaces.py
src/routers/peers.py
src/routers/messages.py

🪛 Pylint (3.3.7)

src/crud.py

[refactor] 133-138: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 81-81: Too many branches (18/12)

(R0912)

[refactor] 181-189: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 186-189: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 192-195: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 141-141: Too many return statements (7/6)

(R0911)

[refactor] 273-273: Consider merging these comparisons with 'in' by using 'operator in ('contains', 'icontains')'. Use a set instead if elements are hashable.

(R1714)

[refactor] 289-294: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 198-198: Too many branches (30/12)

(R0912)

[refactor] 198-198: Too many statements (55/50)

(R0915)

[refactor] 333-336: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 352-357: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 297-297: Too many branches (19/12)

(R0912)

🔇 Additional comments (22)

src/crud.py (3)

37-79: Well-structured filter application function with clear documentation!

The apply_filter function provides a clean interface for applying complex filters to SQLAlchemy queries. The comprehensive documentation with examples is particularly helpful.

141-196: Robust field condition builder with proper type handling.

The function correctly handles the metadata field mapping and distinguishes between comparison operators and regular values.

297-358: Clean comparison condition builder.

The function properly handles all comparison operators with appropriate special cases for JSONB columns and wildcard values.

src/routers/workspaces.py (1)

6-6: Correct migration to async pagination.

The import and function calls have been properly updated to use apaginate for async pagination support.

Also applies to: 68-68, 107-107

src/routers/messages.py (1)

7-7: Consistent async pagination update.

The pagination function has been correctly updated to use apaginate.

Also applies to: 336-336

src/routers/sessions.py (1)

6-6: Proper async pagination implementation.

All pagination calls have been correctly updated to use apaginate.

Also applies to: 97-97, 368-368, 475-475

src/routers/peers.py (1)

16-16: Complete async pagination migration for peer endpoints.

All pagination calls have been properly updated to use apaginate.

Also applies to: 54-54, 141-141, 289-289, 339-339

tests/routes/test_workspaces.py (1)

91-97: LGTM: Filter structure updated correctly

The changes properly update the filter structure to nest criteria under the "metadata" key, which aligns with the new advanced filtering system. The assertion is also improved from a loose > 0 check to a precise == 1 check, making the test more robust.

tests/routes/test_sessions.py (1)

173-180: LGTM: Consistent filter structure update

The filter structure is correctly updated to match the new advanced filtering format, and the assertion is appropriately tightened to verify exact expected results rather than just checking for non-empty results.

tests/routes/test_peers.py (2)

104-113: LGTM: Well-documented backward compatibility

The comment clearly indicates backward compatibility considerations, and the filter structure is correctly updated to the new nested format. The assertion is appropriately tightened to expect exactly 2 results.

115-126: LGTM: NOT operator test logic is correct

The NOT operator test correctly filters out peers with peer_key="peer_value2" and expects 3 results (2 peers with "peer_value" + 1 sample peer with empty metadata). The test logic properly validates the new logical operator functionality.

tests/routes/test_messages.py (2)

357-367: LGTM: Filter structure consistently updated

The filter is correctly nested under the "metadata" key and the assertion is appropriately tightened to expect exactly one matching message.

409-453: LGTM: Comprehensive logical operator testing

The new tests provide excellent coverage of the advanced filtering capabilities:

Backward compatibility test maintains the old filter format

AND operator test with precise assertion (== 2 messages)

OR operator test correctly expects all 3 messages to match the criteria

The test logic is sound and validates the new filtering functionality thoroughly.
tests/test_advanced_filters.py (9)

1-13: LGTM: Excellent comprehensive test module

This new test module provides thorough coverage of the advanced filtering functionality with well-organized test cases covering logical operators, comparison operators, wildcards, and edge cases.

14-101: LGTM: Logical operators test coverage is comprehensive

The test correctly validates AND, OR, and NOT operators with appropriate test data setup and precise assertions. The logic for each operator is sound:

AND: correctly expects 1 match (admin AND engineering)

OR: correctly expects 3 matches (admin OR engineering covers all test peers)

NOT: correctly excludes admin peers and validates specific exclusions

102-219: LGTM: Comparison operators thoroughly tested

The comparison operator tests are mathematically correct and comprehensive:

gte/lte/gt/lt tests with numeric values have correct expected counts

in/contains/icontains tests properly validate text and array operations

Test data setup creates appropriate scenarios for each operator

273-361: LGTM: Complex nested filter logic is correct

The complex nested filter test properly validates the logical combination:

(urgent OR normal priority) AND open status AND NOT assigned to charlie

Expected result of 2 messages is mathematically correct

Content verification ensures the right messages are returned

362-433: LGTM: Cross-model consistency well validated

The tests appropriately verify that the filtering system works consistently across workspaces, sessions, and other models, ensuring the filtering logic is properly centralized and reusable.

434-484: LGTM: Edge cases comprehensively covered

The edge case tests cover important scenarios like empty logical operators, non-existent columns, and mixed wildcard/regular value combinations. This helps ensure the filtering system is robust and handles unexpected inputs gracefully.

485-522: LGTM: Backward compatibility properly maintained

The backward compatibility tests ensure that existing simple filter formats continue to work, which is crucial for maintaining API compatibility during the transition to advanced filtering.

523-594: LGTM: Date range queries properly tested

The date range filtering tests provide good coverage for temporal queries, including both individual date ranges and combined date/numeric filters. The test logic correctly validates the expected filtering behavior.

258-271: Verify wildcard behavior in comparison operators

The comment suggests that wildcards in comparison operators should match all items, but the implementation of {"name": {"in": ["*"]}} may not behave as expected. Wildcards within array comparisons might be treated as literal asterisk characters rather than match-all patterns.

Please verify that this wildcard behavior works as intended:
#!/bin/bash
# Search for wildcard handling in the filtering logic
rg -A 10 -B 5 "wildcard\|\\*" src/

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (7)

src/crud.py (7)

133-138: Simplify the condition combination logic.

The static analysis correctly identifies that the elif/else chain can be simplified since early returns are used.

-    # Combine all conditions with AND
-    if len(conditions) == 0:
-        return None
-    elif len(conditions) == 1:
-        return conditions[0]
-    else:
-        return and_(*conditions)
+    # Combine all conditions with AND
+    if len(conditions) == 0:
+        return None
+    if len(conditions) == 1:
+        return conditions[0]
+    return and_(*conditions)

180-195: Remove unnecessary else statements to improve readability.

Multiple else statements after returns can be simplified for better code flow.

     if isinstance(value, dict):
         # Check if this is a comparison operators dict by looking for known operators
         comparison_operators = {
             "gte",
             "lte",
             "gt",
             "lt",
             "ne",
             "in",
             "contains",
             "icontains",
         }
         is_comparison_dict = any(key in comparison_operators for key in value)

         if is_comparison_dict:
             return _build_comparison_conditions(column, column_name, value)
-        else:
-            # This is a regular value that happens to be a dict
-            # For JSONB metadata, check if it contains nested comparison operators
-            if column_name == "h_metadata":
-                return _build_nested_metadata_conditions(column, value, model_class)
-            else:
-                return column == value
-    else:
-        # Simple equality or contains for JSONB
-        if column_name == "h_metadata":
-            return column.contains(value)
-        else:
-            return column == value
+        # This is a regular value that happens to be a dict
+        # For JSONB metadata, check if it contains nested comparison operators
+        if column_name == "h_metadata":
+            return _build_nested_metadata_conditions(column, value, model_class)
+        return column == value
+    # Simple equality or contains for JSONB
+    if column_name == "h_metadata":
+        return column.contains(value)
+    return column == value

259-265: Simplify the conditional chain.

The elif statements after return can be simplified for better readability.

         return operator_map[operator](safe_accessor, safe_value)
-    elif operator == "in":
+    if operator == "in":
         if isinstance(op_value, list):
             return field_accessor.in_([str(v) for v in op_value])
-    elif operator in ("contains", "icontains"):
+    if operator in ("contains", "icontains"):
         return field_accessor.ilike(f"%{op_value}%")

     return None

283-292: Extract comparison operators to reduce code duplication.

The comparison operators set is duplicated across multiple functions. Consider extracting it to a module-level constant.

Add at the module level (after imports):

+# Comparison operators supported by the filter system
+COMPARISON_OPERATORS = {
+    "gte",
+    "lte", 
+    "gt",
+    "lt",
+    "ne",
+    "in",
+    "contains",
+    "icontains",
+}

Then replace the local set definition:

-    comparison_operators = {
-        "gte",
-        "lte",
-        "gt",
-        "lt",
-        "ne",
-        "in",
-        "contains",
-        "icontains",
-    }

And update the usage:

         if isinstance(field_value, dict) and any(
-            op in comparison_operators for op in field_value
+            op in COMPARISON_OPERATORS for op in field_value
         ):

331-336: Simplify the conditional logic.

Remove unnecessary elif after return for cleaner code flow.

     if not conditions:
         return None
-    elif len(conditions) == 1:
+    if len(conditions) == 1:
         return conditions[0]
-    else:
-        return and_(*conditions)
+    return and_(*conditions)

374-378: Remove unnecessary else after continue.

Simplify the control flow as suggested by static analysis.

                 # Check if the list contains wildcard - if so, skip condition (match all)
                 if "*" in op_value:
                     continue
-                else:
-                    condition = column.in_(op_value)
+                condition = column.in_(op_value)

394-399: Consider operator mapping for better maintainability.

As an alternative to the current if-elif chain, consider using a mapping approach similar to _build_comparison_condition to reduce complexity and improve maintainability.

+    # Operator mapping for cleaner code organization
+    def build_gte_condition(column, op_value):
+        return column >= op_value
+    
+    def build_lte_condition(column, op_value):
+        return column <= op_value
+    
+    # ... other operator functions
+    
+    operator_map = {
+        "gte": build_gte_condition,
+        "lte": build_lte_condition,
+        # ... other operators
+    }

This would reduce the current branching complexity and make adding new operators easier.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d1310e2 and dc2730d.

📒 Files selected for processing (1)

src/crud.py (10 hunks)

🧰 Additional context used

🪛 Pylint (3.3.7)

src/crud.py

[refactor] 133-138: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 81-81: Too many branches (18/12)

(R0912)

[refactor] 181-189: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 186-189: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 192-195: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 141-141: Too many return statements (7/6)

(R0911)

[refactor] 249-263: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 331-336: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 375-378: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 394-399: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 339-339: Too many branches (19/12)

(R0912)

🔇 Additional comments (3)

src/crud.py (3)

37-78: Well-implemented entry point for the filtering system.

The function is properly documented with comprehensive examples, handles edge cases (None filter), and follows a clean separation of concerns by delegating to helper functions.

198-223: Excellent improvement addressing previous safety concerns.

This function properly addresses the unsafe numeric casting issue raised in the previous review by implementing try-catch blocks with fallback to string comparison and appropriate debug logging.

456-456: Excellent centralization of filtering logic.

The replacement of direct .where(models.X.h_metadata.contains(filter)) calls with apply_filter(stmt, filter) creates a consistent and powerful filtering interface across all query functions while maintaining backward compatibility.

Also applies to: 603-603, 679-679, 704-704, 1643-1643, 1673-1673, 1683-1683, 1756-1756, 1896-1896

VVoruganti

Move helper functions into a util/filter.py file or something similar and add support for reconciling internal and external field names (peer_name vs peer_id)

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (9)

src/utils/filter.py (8)

59-117: Consider refactoring to reduce complexity and improve code style.

The function has high cyclomatic complexity (18 branches) which could impact maintainability. Additionally, the elif on lines 113-114 can be replaced with if since the previous condition returns.

Apply this diff to improve code style:

     # Combine all conditions with AND
     if len(conditions) == 0:
         return None
-    elif len(conditions) == 1:
+    if len(conditions) == 1:
         return conditions[0]
-    else:
-        return and_(*conditions)
+    return and_(*conditions)

Consider extracting the logical operator handling into separate methods to reduce complexity:

def _handle_and_operator(filter_dict: dict[str, Any], model_class) -> Any:
    """Handle AND logical operator."""
    and_conditions = []
    for sub_filter in filter_dict["AND"]:
        sub_condition = _build_filter_conditions(sub_filter, model_class)
        if sub_condition is not None:
            and_conditions.append(sub_condition)
    return and_(*and_conditions) if and_conditions else None

169-194: Simplify code by removing unnecessary else blocks.

The nested else blocks after return statements can be removed to improve code readability.

Apply this diff to simplify the code structure:

         if is_comparison_dict:
             return _build_comparison_conditions(column, column_name, value)
-        else:
-            # This is a regular value that happens to be a dict
-            # For JSONB fields (metadata, configuration), check if it contains nested comparison operators
-            # Check if this column is a JSONB column by checking the column type
-            is_jsonb_column = (
-                hasattr(column.type, "python_type")
-                and hasattr(column.type, "impl")
-                and "JSONB" in str(column.type)
-            )
-            if column_name in ("h_metadata", "configuration") or is_jsonb_column:
-                return _build_nested_metadata_conditions(column, value)
-            else:
-                return column == value
-    else:
-        # Simple equality or contains for JSONB
+        
+        # This is a regular value that happens to be a dict
+        # For JSONB fields (metadata, configuration), check if it contains nested comparison operators
+        # Check if this column is a JSONB column by checking the column type
         is_jsonb_column = (
             hasattr(column.type, "python_type")
             and hasattr(column.type, "impl")
             and "JSONB" in str(column.type)
         )
         if column_name in ("h_metadata", "configuration") or is_jsonb_column:
-            return column.contains(value)
-        else:
-            return column == value
+            return _build_nested_metadata_conditions(column, value)
+        return column == value
+    
+    # Simple equality or contains for JSONB
+    is_jsonb_column = (
+        hasattr(column.type, "python_type")
+        and hasattr(column.type, "impl")
+        and "JSONB" in str(column.type)
+    )
+    if column_name in ("h_metadata", "configuration") or is_jsonb_column:
+        return column.contains(value)
+    return column == value

209-226: Minor code style improvement for readability.

The elif can be replaced with if after the return statement.

     if isinstance(op_value, bool):
         # For boolean values, compare with the string representation
         # PostgreSQL JSONB stores booleans as "true"/"false" strings when extracted with ->>
         return column_accessor, str(op_value).lower()
-    elif isinstance(op_value, (int, float)):
+    if isinstance(op_value, (int, float)):
         try:
             # Cast to numeric for proper numeric comparison
             numeric_accessor = cast(column_accessor, Numeric)
             return numeric_accessor, op_value
         except Exception:
             # Fall back to string comparison if casting fails
             logger.debug(
                 "Failed to cast JSONB value to numeric, using string comparison"
             )
             return column_accessor, str(op_value)
-    else:
-        # Non-numeric, non-boolean value, use string comparison
-        return column_accessor, str(op_value)
+    # Non-numeric, non-boolean value, use string comparison
+    return column_accessor, str(op_value)

252-266: Simplify conditional logic.

The elif blocks can be simplified after return statements.

     if operator in numeric_operators:
         safe_accessor, safe_value = _safe_numeric_cast(field_accessor, op_value)
         operator_map = {
             "gte": lambda a, v: a >= v,
             "lte": lambda a, v: a <= v,
             "gt": lambda a, v: a > v,
             "lt": lambda a, v: a < v,
             "ne": lambda a, v: a != v,
         }
         return operator_map[operator](safe_accessor, safe_value)
-    elif operator == "in":
+    if operator == "in":
         if isinstance(op_value, list):
             return field_accessor.in_([str(v) for v in op_value])
-    elif operator in ("contains", "icontains"):
+    if operator in ("contains", "icontains"):
         return field_accessor.ilike(f"%{op_value}%")

331-336: Remove unnecessary elif.

     if not conditions:
         return None
-    elif len(conditions) == 1:
+    if len(conditions) == 1:
         return conditions[0]
-    else:
-        return and_(*conditions)
+    return and_(*conditions)

339-422: Consider refactoring to reduce complexity.

This function has high cyclomatic complexity (22 branches) and could benefit from splitting into smaller, focused functions.

Consider extracting operator handling into a separate mapping:

def _get_comparison_operator_map():
    """Return a mapping of operator names to condition builders."""
    return {
        "gte": lambda col, val: col >= val,
        "lte": lambda col, val: col <= val,
        "gt": lambda col, val: col > val,
        "lt": lambda col, val: col < val,
        "ne": lambda col, val: col != val,
    }

This would simplify the main function and make it easier to maintain.

389-400: Remove unnecessary else after continue.

             if isinstance(op_value, list):
                 # Check if the list contains wildcard - if so, skip condition (match all)
                 if "*" in op_value:
                     continue
-                else:
-                    if is_datetime_column:
-                        # Cast each datetime string value
-                        casted_values = [
-                            text(f"'{val}'::timestamp") if isinstance(val, str) else val
-                            for val in op_value
-                        ]
-                        condition = column.in_(casted_values)
-                    else:
-                        condition = column.in_(op_value)
+                if is_datetime_column:
+                    # Cast each datetime string value
+                    casted_values = [
+                        text(f"'{val}'::timestamp") if isinstance(val, str) else val
+                        for val in op_value
+                    ]
+                    condition = column.in_(casted_values)
+                else:
+                    condition = column.in_(op_value)

416-421: Simplify return logic.

     # Combine all conditions for this field with AND
     if len(conditions) == 0:
         return None
-    elif len(conditions) == 1:
+    if len(conditions) == 1:
         return conditions[0]
-    else:
-        return and_(*conditions)
+    return and_(*conditions)

tests/test_advanced_filters.py (1)

875-1008: Consider splitting this test function for better maintainability.

The function exceeds the recommended statement limit (52/50). Consider splitting it into smaller, focused test functions.

Consider splitting into separate test functions:

test_message_id_filtering

test_message_foreign_key_filtering

test_message_content_filtering

test_message_metadata_filtering

test_message_numeric_field_filtering

This would improve readability and make it easier to identify which specific aspect fails if a test breaks.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc2730d and 9680c59.

📒 Files selected for processing (3)

src/crud.py (10 hunks)
src/utils/filter.py (1 hunks)
tests/test_advanced_filters.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

src/crud.py

🧰 Additional context used

🪛 Pylint (3.3.7)

src/utils/filter.py

[refactor] 111-116: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 59-59: Too many branches (18/12)

(R0912)

[refactor] 169-183: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 180-183: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 191-194: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 119-119: Too many return statements (7/6)

(R0911)

[refactor] 209-226: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 252-266: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 331-336: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 389-400: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 416-421: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 339-339: Too many branches (22/12)

(R0912)

tests/test_advanced_filters.py

[refactor] 875-875: Too many statements (52/50)

(R0915)

🔇 Additional comments (2)

src/utils/filter.py (1)

11-57: Well-structured entry point for the filtering system.

The function provides a clean interface with comprehensive documentation and examples. The implementation correctly extracts the model class and delegates condition building appropriately.

tests/test_advanced_filters.py (1)

1-1375: Excellent test coverage for the filtering system!

The test suite comprehensively covers all filtering functionality including logical operators, comparison operators, wildcards, edge cases, field interpolation, and backward compatibility. The test data setup is well-structured using nanoid for unique identifiers.

coderabbitai

Actionable comments posted: 4

🧹 Nitpick comments (5)

src/utils/filter.py (5)

59-117: Refactor to reduce complexity and improve control flow.

The function works correctly but has high cyclomatic complexity and could benefit from cleaner control flow patterns.

Apply these changes to improve readability:

-    # Combine all conditions with AND
-    if len(conditions) == 0:
-        return None
-    elif len(conditions) == 1:
-        return conditions[0]
-    else:
-        return and_(*conditions)
+    # Combine all conditions with AND
+    if len(conditions) == 0:
+        return None
+    if len(conditions) == 1:
+        return conditions[0]
+    return and_(*conditions)

131-140: Consider using a mapping dictionary for field name transformations.

The current if-elif chain for field mapping could be more maintainable and extensible.

Replace the field mapping logic with a more structured approach:

-    # Map 'metadata' to 'h_metadata' for all models
-    column_name = "h_metadata" if key == "metadata" else key
-    if column_name == "id":
-        # For Message model, id maps to public_id, for others it maps to name
-        column_name = "public_id" if model_class.__name__ == "Message" else "name"
-    # Only apply _id to _name mapping for fields that are actually foreign keys
-    # and not already mapped to public_id
-    elif column_name.endswith("_id"):
-        column_name = column_name[:-3] + "_name"
+    # Field mapping configurations
+    FIELD_MAPPINGS = {
+        "metadata": "h_metadata",
+    }
+    
+    column_name = FIELD_MAPPINGS.get(key, key)
+    
+    if column_name == "id":
+        # For Message model, id maps to public_id, for others it maps to name
+        column_name = "public_id" if model_class.__name__ == "Message" else "name"
+    elif column_name.endswith("_id"):
+        # Only apply _id to _name mapping for fields that are actually foreign keys
+        column_name = column_name[:-3] + "_name"

155-195: Simplify control flow by removing unnecessary else clauses.

The nested if-else structure can be flattened for better readability.

-        if is_comparison_dict:
-            return _build_comparison_conditions(column, column_name, value)
-        else:
-            # This is a regular value that happens to be a dict
-            # For JSONB fields (metadata, configuration), check if it contains nested comparison operators
-            # Check if this column is a JSONB column by checking the column type
-            is_jsonb_column = (
-                hasattr(column.type, "python_type")
-                and hasattr(column.type, "impl")
-                and "JSONB" in str(column.type)
-            )
-            if column_name in ("h_metadata", "configuration") or is_jsonb_column:
-                return _build_nested_metadata_conditions(column, value)
-            else:
-                return column == value
+        if is_comparison_dict:
+            return _build_comparison_conditions(column, column_name, value)
+        
+        # This is a regular value that happens to be a dict
+        # For JSONB fields (metadata, configuration), check if it contains nested comparison operators
+        is_jsonb_column = (
+            hasattr(column.type, "python_type")
+            and hasattr(column.type, "impl")
+            and "JSONB" in str(column.type)
+        )
+        if column_name in ("h_metadata", "configuration") or is_jsonb_column:
+            return _build_nested_metadata_conditions(column, value)
+        return column == value

353-353: Move import to module level for better performance.

Importing within a function that could be called frequently is inefficient.

+from sqlalchemy import Select, and_, cast, not_, or_, text
-from sqlalchemy import Select, and_, cast, not_, or_

And remove the local import:

-    from sqlalchemy import text

388-400: Simplify wildcard handling in list operations.

The control flow can be simplified by removing the unnecessary else clause.

-                # Check if the list contains wildcard - if so, skip condition (match all)
-                if "*" in op_value:
-                    continue
-                else:
-                    if is_datetime_column:
-                        # Cast each datetime string value
-                        casted_values = [
-                            text(f"'{val}'::timestamp") if isinstance(val, str) else val
-                            for val in op_value
-                        ]
-                        condition = column.in_(casted_values)
-                    else:
-                        condition = column.in_(op_value)
+                # Check if the list contains wildcard - if so, skip condition (match all)
+                if "*" in op_value:
+                    continue
+                
+                if is_datetime_column:
+                    # Cast each datetime string value
+                    casted_values = [
+                        text(f"'{val}'::timestamp") if isinstance(val, str) else val
+                        for val in op_value
+                    ]
+                    condition = column.in_(casted_values)
+                else:
+                    condition = column.in_(op_value)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9680c59 and 2a3f3d9.

📒 Files selected for processing (2)

src/crud.py (10 hunks)
src/utils/filter.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

src/crud.py

🧰 Additional context used

🪛 Pylint (3.3.7)

src/utils/filter.py

[refactor] 111-116: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 59-59: Too many branches (18/12)

(R0912)

[refactor] 169-183: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 180-183: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 191-194: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 119-119: Too many return statements (7/6)

(R0911)

[refactor] 209-226: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 252-266: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 331-336: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 389-400: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 416-421: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 339-339: Too many branches (22/12)

(R0912)

🔇 Additional comments (3)

src/utils/filter.py (3)

1-8: LGTM: Clean imports and logger setup.

The imports are well-organized and the logger initialization follows best practices.

11-57: LGTM: Well-documented main function with clear API.

The function signature, documentation, and examples are comprehensive. The logic is straightforward and handles the null case appropriately.

197-227: LGTM: Robust type casting with proper fallback handling.

The safe numeric casting function handles different data types appropriately and includes proper error handling with logging.

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (5)

src/utils/filter.py (5)

154-160: Simplify the conditional logic.

The static analysis correctly identifies unnecessary elif statements that can be simplified.

-    # Combine all conditions with AND
-    if len(conditions) == 0:
-        return None
-    elif len(conditions) == 1:
-        return conditions[0]
-    else:
-        return and_(*conditions)
+    # Combine all conditions with AND
+    if len(conditions) == 0:
+        return None
+    if len(conditions) == 1:
+        return conditions[0]
+    return and_(*conditions)

199-220: Simplify nested conditionals.

The nested if-else structure can be flattened to improve readability.

     # Handle comparison operators vs regular values
     if isinstance(value, dict):
         # Check if this is a comparison operators dict by looking for known operators
         is_comparison_dict = any(key in COMPARISON_OPERATORS for key in value)
 
         if is_comparison_dict:
             return _build_comparison_conditions(column, column_name, value)
-        else:
-            # This is a regular value that happens to be a dict
-            # For JSONB fields (metadata, configuration), check if it contains nested comparison operators
-            # Check if this column is a JSONB column by checking the column type
-            is_jsonb_column = _is_jsonb_column(column)
-            if column_name in ("h_metadata", "configuration") or is_jsonb_column:
-                return _build_nested_metadata_conditions(column, value)
-            else:
-                return column == value
-    else:
-        # Simple equality or contains for JSONB
-        is_jsonb_column = _is_jsonb_column(column)
-        if column_name in ("h_metadata", "configuration") or is_jsonb_column:
-            return column.contains(value)
-        else:
-            return column == value
+        
+        # This is a regular value that happens to be a dict
+        # For JSONB fields (metadata, configuration), check if it contains nested comparison operators
+        # Check if this column is a JSONB column by checking the column type
+        is_jsonb_column = _is_jsonb_column(column)
+        if column_name in ("h_metadata", "configuration") or is_jsonb_column:
+            return _build_nested_metadata_conditions(column, value)
+        return column == value
+    
+    # Simple equality or contains for JSONB
+    is_jsonb_column = _is_jsonb_column(column)
+    if column_name in ("h_metadata", "configuration") or is_jsonb_column:
+        return column.contains(value)
+    return column == value

235-252: Simplify return logic in numeric casting function.

Remove unnecessary elif statements for cleaner flow.

     if isinstance(op_value, bool):
         # For boolean values, compare with the string representation
         # PostgreSQL JSONB stores booleans as "true"/"false" strings when extracted with ->>
         return column_accessor, str(op_value).lower()
-    elif isinstance(op_value, (int, float)):
+    if isinstance(op_value, (int, float)):
         try:
             # Cast to numeric for proper numeric comparison
             numeric_accessor = cast(column_accessor, Numeric)
             return numeric_accessor, op_value
         except Exception:
             # Fall back to string comparison if casting fails
             logger.debug(
                 "Failed to cast JSONB value to numeric, using string comparison"
             )
             return column_accessor, str(op_value)
-    else:
-        # Non-numeric, non-boolean value, use string comparison
-        return column_accessor, str(op_value)
+    
+    # Non-numeric, non-boolean value, use string comparison
+    return column_accessor, str(op_value)

406-427: Simplify the else-after-continue pattern.

The static analysis correctly identifies that the else clause after continue is unnecessary.

             if isinstance(op_value, list):
                 # Check if the list contains wildcard - if so, skip condition (match all)
                 if "*" in op_value:
                     continue
-                else:
-                    if is_datetime_column:
-                        # Validate and cast each datetime string value
-                        casted_values = []
-                        for val in op_value:
-                            if isinstance(val, str):
-                                validated_datetime = _validate_datetime_string(val)
-                                if validated_datetime is None:
-                                    logger.warning(
-                                        f"Skipping invalid datetime value in list: {val}"
-                                    )
-                                    continue
-                                casted_values.append(validated_datetime)
-                            else:
-                                casted_values.append(val)
-                        if casted_values:
-                            condition = column.in_(casted_values)
-                    else:
-                        condition = column.in_(op_value)
+                
+                if is_datetime_column:
+                    # Validate and cast each datetime string value
+                    casted_values = []
+                    for val in op_value:
+                        if isinstance(val, str):
+                            validated_datetime = _validate_datetime_string(val)
+                            if validated_datetime is None:
+                                logger.warning(
+                                    f"Skipping invalid datetime value in list: {val}"
+                                )
+                                continue
+                            casted_values.append(validated_datetime)
+                        else:
+                            casted_values.append(val)
+                    if casted_values:
+                        condition = column.in_(casted_values)
+                else:
+                    condition = column.in_(op_value)

443-448: Simplify final condition combination logic.

Remove unnecessary elif for consistent style.

     # Combine all conditions for this field with AND
     if len(conditions) == 0:
         return None
-    elif len(conditions) == 1:
+    if len(conditions) == 1:
         return conditions[0]
-    else:
-        return and_(*conditions)
+    return and_(*conditions)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee7c4c9 and 52b59aa.

📒 Files selected for processing (4)

src/crud.py (10 hunks)
src/routers/workspaces.py (3 hunks)
src/schemas.py (1 hunks)
src/utils/filter.py (1 hunks)

✅ Files skipped from review due to trivial changes (1)

src/routers/workspaces.py

🚧 Files skipped from review as they are similar to previous changes (2)

src/schemas.py
src/crud.py

🧰 Additional context used

🪛 Pylint (3.3.7)

src/utils/filter.py

[refactor] 155-160: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 103-103: Too many branches (18/12)

(R0912)

[refactor] 203-213: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 210-213: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 217-220: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 163-163: Too many return statements (7/6)

(R0911)

[refactor] 235-252: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 276-290: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 345-350: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 407-427: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 374-440: Too many nested blocks (8/5)

(R1702)

[refactor] 374-440: Too many nested blocks (6/5)

(R1702)

[refactor] 443-448: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 353-353: Too many branches (28/12)

(R0912)

[refactor] 353-353: Too many statements (54/50)

(R0915)

🔇 Additional comments (4)

src/utils/filter.py (4)

1-24: LGTM! Module structure and constants are well-organized.

The imports are appropriate and the module-level constants for comparison and numeric operators follow best practices, addressing previous review feedback about avoiding duplication.

26-52: LGTM! JSONB detection is now robust and addresses previous concerns.

The improved JSONB detection logic properly checks against the actual JSONB type class and includes a fallback for edge cases, which addresses the previous review comment about unreliable string-based detection.

55-100: LGTM! Main filter function has comprehensive documentation and clean logic.

The function signature, docstring with examples, and implementation are well-structured. The examples clearly demonstrate the supported filter patterns.

451-494: LGTM! Comprehensive datetime validation addresses security concerns.

The datetime validation function properly addresses the SQL injection concerns from previous reviews. The multiple format support and malicious content detection provide robust protection while maintaining usability.

VVoruganti · 2025-06-25T15:03:15Z

+    if JSONB is None:
+        # Fallback to string-based detection if JSONB type is not available
+        return (
+            hasattr(column.type, "python_type")
+            and hasattr(column.type, "impl")
+            and "JSONB" in str(column.type)
+        )


This should never be the case. We are just importing a type directly?

VVoruganti · 2025-06-25T15:07:09Z

+    column_name = "h_metadata" if key == "metadata" else key
+    if column_name == "id":
+        # For Message model, id maps to public_id, for others it maps to name
+        column_name = "public_id" if model_class.__name__ == "Message" else "name"
+    # Only apply _id to _name mapping for fields that are actually foreign keys
+    # and not already mapped to public_id
+    elif column_name.endswith("_id"):
+        column_name = column_name[:-3] + "_name"


Still shaky on this would prefer if we had a more direct mapping with the contracts in code. Maybe add a TODO to optimize in the future

VVoruganti · 2025-06-25T15:09:18Z

+    value = value.strip()
+    if not value or "'" in value or ";" in value or "--" in value:
+        logger.warning(f"Potentially malicious datetime string rejected: {value}")
+        return None


This feels brittle, I think SQLAlchemy has some built-in sql injection prevention techniques but would double check that.

Claude:

Lines 395-403: The SQL injection "prevention" is still wrong Even with JSONB context, this is bad practice. SQLAlchemy's parameterization handles this correctly, and you're blocking legitimate datetime strings.

VVoruganti · 2025-06-25T15:10:45Z

+            if column_name == "h_metadata":
+                # For JSONB columns, use JSONB contains
+                condition = column.contains(op_value)


Claude Comment:

JSONB contains assumptions: The code assumes column.contains(value) will work correctly for all JSONB scenarios, but PostgreSQL's JSONB contains operator has specific semantics that might not match user expectations for nested objects.

VVoruganti · 2025-06-25T15:17:46Z

 class SessionGet(SessionBase):
    filter: dict | None = None
-    is_active: bool = False
+    is_active: bool | None = None


Let's consolidate all Get Request Models to have any filters such as is_active to be a part of the filter itself. This does not include stuff like reverse which is more of a data transformation.

VVoruganti · 2025-06-25T15:24:50Z

Want to make sure we do a thorough job of testing all edge cases here. Worked with claude to analyze the code and test cases and have it list untested capabilities:

Missing Error Handling Tests: Invalid datetime strings Malformed filter dictionaries Type mismatches in comparisons Null/None value handling SQL injection attempts Missing Type Casting Edge Cases: Boolean comparisons in JSONB (true/false vs "true"/"false") Large number handling Float precision issues Mixed type comparisons (string vs number) Missing JSONB Behavior Tests: Nested object filtering Array handling in JSONB fields Complex JSONB contains scenarios Multiple operators on same JSONB field Missing Real DateTime Column Tests: The tests only use date-like strings in metadata, not actual datetime columns No validation of datetime parsing edge cases

…dge cases with more tests

coderabbitai

Actionable comments posted: 4

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ab8f33b and 29ad171.

📒 Files selected for processing (9)

src/crud.py (9 hunks)
src/exceptions.py (1 hunks)
src/routers/peers.py (5 hunks)
src/routers/sessions.py (4 hunks)
src/schemas.py (3 hunks)
src/utils/filter.py (1 hunks)
tests/routes/test_peers.py (1 hunks)
tests/routes/test_sessions.py (2 hunks)
tests/test_advanced_filters.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (4)

`src/exceptions.py`: Define custom exception types here. Custom exceptions should be defined here and used throughout the codebase for error handling.

src/exceptions.py: Define custom exception types here.
Custom exceptions should be defined here and used throughout the codebase for error handling.

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

src/exceptions.py

`src/routers/*.py`: Define API endpoints in these files.

src/routers/*.py: Define API endpoints in these files.

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

src/routers/peers.py
src/routers/sessions.py

`src/schemas.py`: Define Pydantic validation schemas for API.

src/schemas.py: Define Pydantic validation schemas for API.

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

src/schemas.py

`src/crud.py`: Implement database operations in this file.

src/crud.py: Implement database operations in this file.

📄 Source: CodeRabbit Inference Engine (CLAUDE.md)

List of files the instruction was applied to:

src/crud.py

🪛 Pylint (3.3.7)

src/routers/peers.py

[error] 14-14: Unable to import 'fastapi_pagination.ext.sqlalchemy'

(E0401)

src/schemas.py

[refactor] 256-256: Too few public methods (0/2)

(R0903)

[refactor] 288-288: Too few public methods (0/2)

(R0903)

[refactor] 294-294: Too few public methods (0/2)

(R0903)

src/routers/sessions.py

[error] 5-5: Unable to import 'fastapi_pagination.ext.sqlalchemy'

(E0401)

src/utils/filter.py

[convention] 214-214: Line too long (106/100)

(C0301)

[convention] 265-265: Line too long (123/100)

(C0301)

[convention] 313-313: Line too long (134/100)

(C0301)

[convention] 431-431: Line too long (110/100)

(C0301)

[convention] 448-448: Line too long (102/100)

(C0301)

[convention] 471-471: Line too long (138/100)

(C0301)

[warning] 44-44: # TODO any others?

(W0511)

[convention] 1-1: Missing module docstring

(C0114)

[error] 5-5: Unable to import 'sqlalchemy'

(E0401)

[error] 6-6: Unable to import 'sqlalchemy.types'

(E0401)

[warning] 49-49: Redefining built-in 'filter'

(W0622)

[refactor] 167-172: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 99-99: Too many branches (22/12)

(R0912)

[refactor] 210-218: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 215-218: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 220-223: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 238-266: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 259-266: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 295-316: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 306-314: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 374-379: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 449-468: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 403-485: Too many nested blocks (8/5)

(R1702)

[refactor] 403-485: Too many nested blocks (6/5)

(R1702)

[refactor] 488-493: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 382-382: Too many branches (32/12)

(R0912)

[refactor] 382-382: Too many statements (61/50)

(R0915)

tests/test_advanced_filters.py

[convention] 1397-1397: Line too long (105/100)

(C0301)

[convention] 1398-1398: Line too long (104/100)

(C0301)

[convention] 1417-1417: Line too long (118/100)

(C0301)

[convention] 1-1: Too many lines in module (2122/1000)

(C0302)

[error] 9-9: Unable to import 'fastapi.testclient'

(E0401)

[error] 10-10: Unable to import 'nanoid'

(E0401)

[error] 11-11: Unable to import 'sqlalchemy.ext.asyncio'

(E0401)

[warning] 18-18: Unused argument 'db_session'

(W0613)

[warning] 21-21: Unused variable 'test_peer'

(W0612)

[warning] 106-106: Unused argument 'db_session'

(W0613)

[warning] 224-224: Unused argument 'db_session'

(W0613)

[warning] 227-227: Unused variable 'test_peer'

(W0612)

[warning] 277-277: Unused argument 'db_session'

(W0613)

[warning] 366-366: Unused argument 'db_session'

(W0613)

[warning] 438-438: Unused argument 'db_session'

(W0613)

[warning] 489-489: Unused argument 'db_session'

(W0613)

[warning] 492-492: Unused variable 'test_peer'

(W0612)

[warning] 527-527: Unused argument 'db_session'

(W0613)

[warning] 687-687: Unused argument 'db_session'

(W0613)

[warning] 690-690: Unused variable 'test_peer'

(W0612)

[warning] 777-777: Unused argument 'db_session'

(W0613)

[refactor] 877-877: Too many statements (52/50)

(R0915)

[warning] 878-878: Unused argument 'db_session'

(W0613)

[warning] 1014-1014: Unused argument 'db_session'

(W0613)

[warning] 1017-1017: Unused variable 'test_workspace'

(W0612)

[warning] 1017-1017: Unused variable 'test_peer'

(W0612)

[warning] 1106-1106: Unused argument 'db_session'

(W0613)

[warning] 1317-1317: Unused argument 'db_session'

(W0613)

[warning] 1320-1320: Unused variable 'test_peer'

(W0612)

[warning] 1367-1367: Unused argument 'db_session'

(W0613)

[warning] 1370-1370: Unused variable 'test_peer'

(W0612)

[warning] 1427-1427: Unused argument 'db_session'

(W0613)

[convention] 1516-1516: Import outside toplevel (time)

(C0415)

[warning] 1501-1501: Unused argument 'db_session'

(W0613)

[warning] 1504-1504: Unused variable 'test_peer'

(W0612)

[warning] 1551-1551: Unused argument 'db_session'

(W0613)

[warning] 1554-1554: Unused variable 'test_peer'

(W0612)

[warning] 1574-1574: Unused argument 'db_session'

(W0613)

[warning] 1627-1627: Unused argument 'db_session'

(W0613)

[warning] 1680-1680: Unused argument 'db_session'

(W0613)

[warning] 1683-1683: Unused variable 'test_peer'

(W0612)

[warning] 1710-1710: Unused argument 'db_session'

(W0613)

[warning] 1741-1741: Unused argument 'db_session'

(W0613)

[warning] 1744-1744: Unused variable 'test_peer'

(W0612)

[warning] 1789-1789: Unused argument 'db_session'

(W0613)

[warning] 1792-1792: Unused variable 'test_peer'

(W0612)

[warning] 1815-1815: Unused argument 'db_session'

(W0613)

[warning] 1818-1818: Unused variable 'test_peer'

(W0612)

[warning] 1865-1865: Unused argument 'db_session'

(W0613)

[warning] 1868-1868: Unused variable 'test_peer'

(W0612)

[warning] 1931-1931: Unused argument 'db_session'

(W0613)

[warning] 1984-1984: Unused argument 'db_session'

(W0613)

[warning] 1987-1987: Unused variable 'test_peer'

(W0612)

[warning] 2043-2043: Unused argument 'db_session'

(W0613)

[warning] 2046-2046: Unused variable 'test_peer'

(W0612)

[warning] 2064-2064: Unused argument 'db_session'

(W0613)

🪛 Ruff (0.11.9)

src/routers/sessions.py

94-94: Trailing comma missing

Add trailing comma

(COM812)

src/utils/filter.py

49-49: Missing type annotation for function argument model_class

(ANN001)

49-49: Trailing comma missing

Add trailing comma

(COM812)

99-99: _build_filter_conditions is too complex (22 > 10)

(C901)

99-99: Missing type annotation for function argument model_class

(ANN001)

99-99: Dynamically typed expressions (typing.Any) are disallowed in _build_filter_conditions

(ANN401)

115-117: Avoid specifying long messages outside the exception class

(TRY003)

116-116: Trailing comma missing

Add trailing comma

(COM812)

128-130: Avoid specifying long messages outside the exception class

(TRY003)

129-129: Trailing comma missing

Add trailing comma

(COM812)

141-141: Avoid specifying long messages outside the exception class

(TRY003)

143-145: Avoid specifying long messages outside the exception class

(TRY003)

144-144: Trailing comma missing

Add trailing comma

(COM812)

151-151: Trailing comma missing

Add trailing comma

(COM812)

169-169: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

175-175: Dynamically typed expressions (typing.Any) are disallowed in value

(ANN401)

175-175: Missing type annotation for function argument model_class

(ANN001)

175-175: Dynamically typed expressions (typing.Any) are disallowed in _build_field_condition

(ANN401)

193-193: Avoid specifying long messages outside the exception class

(TRY003)

197-197: Avoid specifying long messages outside the exception class

(TRY003)

212-212: Unnecessary else after return statement

Remove unnecessary else

(RET505)

217-217: Unnecessary else after return statement

Remove unnecessary else

(RET505)

222-222: Unnecessary else after return statement

Remove unnecessary else

(RET505)

226-226: Missing return type annotation for private function _safe_numeric_cast

(ANN202)

226-226: Missing type annotation for function argument column_accessor

(ANN001)

226-226: Missing type annotation for function argument op_value

(ANN001)

242-242: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

252-252: Consider moving this statement to an else block

(TRY300)

257-257: Consider moving this statement to an else block

(TRY300)

263-263: Unnecessary else after return statement

Remove unnecessary else

(RET505)

264-266: Avoid specifying long messages outside the exception class

(TRY003)

265-265: Trailing comma missing

Add trailing comma

(COM812)

270-270: Missing type annotation for function argument column

(ANN001)

270-270: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

270-270: Trailing comma missing

Add trailing comma

(COM812)

271-271: Dynamically typed expressions (typing.Any) are disallowed in _build_comparison_condition

(ANN401)

286-286: Avoid specifying long messages outside the exception class

(TRY003)

305-305: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

311-311: Unnecessary else after return statement

Remove unnecessary else

(RET505)

312-314: Avoid specifying long messages outside the exception class

(TRY003)

313-313: Trailing comma missing

Add trailing comma

(COM812)

321-321: Missing type annotation for function argument column

(ANN001)

321-321: Dynamically typed expressions (typing.Any) are disallowed in _build_nested_metadata_conditions

(ANN401)

342-342: Trailing comma missing

Add trailing comma

(COM812)

351-351: Trailing comma missing

Add trailing comma

(COM812)

364-364: Dynamically typed expressions (typing.Any) are disallowed in _combine_conditions_with_and

(ANN401)

376-376: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

382-382: _build_comparison_conditions is too complex (27 > 10)

(C901)

383-383: Missing type annotation for function argument column

(ANN001)

383-383: Trailing comma missing

Add trailing comma

(COM812)

384-384: Dynamically typed expressions (typing.Any) are disallowed in _build_comparison_conditions

(ANN401)

400-400: Trailing comma missing

Add trailing comma

(COM812)

406-406: Avoid specifying long messages outside the exception class

(TRY003)

420-420: Avoid specifying long messages outside the exception class

(TRY003)

430-432: Avoid specifying long messages outside the exception class

(TRY003)

431-431: Trailing comma missing

Add trailing comma

(COM812)

451-451: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

459-461: Avoid specifying long messages outside the exception class

(TRY003)

460-460: Trailing comma missing

Add trailing comma

(COM812)

470-472: Avoid specifying long messages outside the exception class

(TRY003)

471-471: Trailing comma missing

Add trailing comma

(COM812)

490-490: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

525-525: Naive datetime constructed using datetime.datetime.strptime() without %z

(DTZ007)

tests/test_advanced_filters.py

17-17: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

18-18: Unused function argument: db_session

(ARG001)

18-18: Trailing comma missing

Add trailing comma

(COM812)

66-66: Trailing comma missing

Add trailing comma

(COM812)

67-67: Trailing comma missing

Add trailing comma

(COM812)

83-83: Trailing comma missing

Add trailing comma

(COM812)

84-84: Trailing comma missing

Add trailing comma

(COM812)

105-105: Missing return type annotation for public function test_comparison_operators_filters

Add return type annotation: None

(ANN201)

106-106: Unused function argument: db_session

(ARG001)

106-106: Trailing comma missing

Add trailing comma

(COM812)

143-143: Trailing comma missing

Add trailing comma

(COM812)

223-223: Missing return type annotation for public function test_wildcard_filters

Add return type annotation: None

(ANN201)

224-224: Unused function argument: db_session

(ARG001)

224-224: Trailing comma missing

Add trailing comma

(COM812)

250-250: Trailing comma missing

Add trailing comma

(COM812)

251-251: Trailing comma missing

Add trailing comma

(COM812)

265-265: Trailing comma missing

Add trailing comma

(COM812)

266-266: Trailing comma missing

Add trailing comma

(COM812)

276-276: Missing return type annotation for public function test_complex_nested_filters

Add return type annotation: None

(ANN201)

277-277: Unused function argument: db_session

(ARG001)

277-277: Trailing comma missing

Add trailing comma

(COM812)

330-330: Trailing comma missing

Add trailing comma

(COM812)

344-344: Trailing comma missing

Add trailing comma

(COM812)

348-348: Trailing comma missing

Add trailing comma

(COM812)

349-349: Trailing comma missing

Add trailing comma

(COM812)

365-365: Missing return type annotation for public function test_filters_across_different_models

Add return type annotation: None

(ANN201)

366-366: Unused function argument: db_session

(ARG001)

366-366: Trailing comma missing

Add trailing comma

(COM812)

388-388: Trailing comma missing

Add trailing comma

(COM812)

389-389: Trailing comma missing

Add trailing comma

(COM812)

425-425: Trailing comma missing

Add trailing comma

(COM812)

426-426: Trailing comma missing

Add trailing comma

(COM812)

437-437: Missing return type annotation for public function test_filter_edge_cases

Add return type annotation: None

(ANN201)

438-438: Unused function argument: db_session

(ARG001)

438-438: Trailing comma missing

Add trailing comma

(COM812)

448-448: Trailing comma missing

Add trailing comma

(COM812)

449-449: Trailing comma missing

Add trailing comma

(COM812)

476-476: Trailing comma missing

Add trailing comma

(COM812)

477-477: Trailing comma missing

Add trailing comma

(COM812)

488-488: Missing return type annotation for public function test_backward_compatibility

Add return type annotation: None

(ANN201)

489-489: Unused function argument: db_session

(ARG001)

489-489: Trailing comma missing

Add trailing comma

(COM812)

526-526: Missing return type annotation for public function test_range_queries_with_dates

Add return type annotation: None

(ANN201)

527-527: Unused function argument: db_session

(ARG001)

527-527: Trailing comma missing

Add trailing comma

(COM812)

567-567: Trailing comma missing

Add trailing comma

(COM812)

568-568: Trailing comma missing

Add trailing comma

(COM812)

586-586: Trailing comma missing

Add trailing comma

(COM812)

587-587: Trailing comma missing

Add trailing comma

(COM812)

599-599: Missing return type annotation for public function test_all_workspace_columns_filtering

Add return type annotation: None

(ANN201)

686-686: Missing return type annotation for public function test_all_peer_columns_filtering

Add return type annotation: None

(ANN201)

687-687: Unused function argument: db_session

(ARG001)

687-687: Trailing comma missing

Add trailing comma

(COM812)

776-776: Missing return type annotation for public function test_all_session_columns_filtering

Add return type annotation: None

(ANN201)

777-777: Unused function argument: db_session

(ARG001)

777-777: Trailing comma missing

Add trailing comma

(COM812)

877-877: Missing return type annotation for public function test_all_message_columns_filtering

Add return type annotation: None

(ANN201)

878-878: Unused function argument: db_session

(ARG001)

878-878: Trailing comma missing

Add trailing comma

(COM812)

911-911: Trailing comma missing

Add trailing comma

(COM812)

1013-1013: Missing return type annotation for public function test_id_field_interpolation_consistency

Add return type annotation: None

(ANN201)

1014-1014: Unused function argument: db_session

(ARG001)

1014-1014: Trailing comma missing

Add trailing comma

(COM812)

1055-1055: Trailing comma missing

Add trailing comma

(COM812)

1056-1056: Trailing comma missing

Add trailing comma

(COM812)

1105-1105: Missing return type annotation for public function test_foreign_key_field_interpolation

Add return type annotation: None

(ANN201)

1106-1106: Unused function argument: db_session

(ARG001)

1106-1106: Trailing comma missing

Add trailing comma

(COM812)

1138-1138: Trailing comma missing

Add trailing comma

(COM812)

1139-1139: Trailing comma missing

Add trailing comma

(COM812)

1193-1193: Missing return type annotation for public function test_metadata_field_interpolation

Add return type annotation: None

(ANN201)

1237-1237: Trailing comma missing

Add trailing comma

(COM812)

1238-1238: Trailing comma missing

Add trailing comma

(COM812)

1316-1316: Missing return type annotation for public function test_nonexistent_columns_ignored_gracefully

Add return type annotation: None

(ANN201)

1317-1317: Unused function argument: db_session

(ARG001)

1317-1317: Trailing comma missing

Add trailing comma

(COM812)

1344-1344: Trailing comma missing

Add trailing comma

(COM812)

1345-1345: Trailing comma missing

Add trailing comma

(COM812)

1358-1358: Trailing comma missing

Add trailing comma

(COM812)

1359-1359: Trailing comma missing

Add trailing comma

(COM812)

1366-1366: Missing return type annotation for public function test_not_logic_correctness

Add return type annotation: None

(ANN201)

1367-1367: Unused function argument: db_session

(ARG001)

1367-1367: Trailing comma missing

Add trailing comma

(COM812)

1406-1406: Trailing comma missing

Add trailing comma

(COM812)

1407-1407: Trailing comma missing

Add trailing comma

(COM812)

1426-1426: Missing return type annotation for public function test_jsonb_type_casting_edge_cases

Add return type annotation: None

(ANN201)

1427-1427: Unused function argument: db_session

(ARG001)

1427-1427: Trailing comma missing

Add trailing comma

(COM812)

1463-1463: Trailing comma missing

Add trailing comma

(COM812)

1500-1500: Missing return type annotation for public function test_real_datetime_column_filtering

Add return type annotation: None

(ANN201)

1501-1501: Unused function argument: db_session

(ARG001)

1501-1501: Trailing comma missing

Add trailing comma

(COM812)

1518-1518: Async functions should not call time.sleep

(ASYNC251)

1550-1550: Missing return type annotation for public function test_invalid_datetime_handling

Add return type annotation: None

(ANN201)

1551-1551: Unused function argument: db_session

(ARG001)

1551-1551: Trailing comma missing

Add trailing comma

(COM812)

1573-1573: Missing return type annotation for public function test_nested_jsonb_filtering

Add return type annotation: None

(ANN201)

1574-1574: Unused function argument: db_session

(ARG001)

1574-1574: Trailing comma missing

Add trailing comma

(COM812)

1610-1610: Trailing comma missing

Add trailing comma

(COM812)

1626-1626: Missing return type annotation for public function test_multiple_operators_same_field

Add return type annotation: None

(ANN201)

1627-1627: Unused function argument: db_session

(ARG001)

1627-1627: Trailing comma missing

Add trailing comma

(COM812)

1663-1663: Trailing comma missing

Add trailing comma

(COM812)

1679-1679: Missing return type annotation for public function test_empty_and_null_filter_handling

Add return type annotation: None

(ANN201)

1680-1680: Unused function argument: db_session

(ARG001)

1680-1680: Trailing comma missing

Add trailing comma

(COM812)

1687-1687: Trailing comma missing

Add trailing comma

(COM812)

1696-1696: Trailing comma missing

Add trailing comma

(COM812)

1709-1709: Missing return type annotation for public function test_unicode_and_special_characters

Add return type annotation: None

(ANN201)

1710-1710: Unused function argument: db_session

(ARG001)

1710-1710: Trailing comma missing

Add trailing comma

(COM812)

1740-1740: Missing return type annotation for public function test_case_sensitivity_edge_cases

Add return type annotation: None

(ANN201)

1741-1741: Unused function argument: db_session

(ARG001)

1741-1741: Trailing comma missing

Add trailing comma

(COM812)

1788-1788: Missing return type annotation for public function test_malformed_filter_structures

Add return type annotation: None

(ANN201)

1789-1789: Unused function argument: db_session

(ARG001)

1789-1789: Trailing comma missing

Add trailing comma

(COM812)

1814-1814: Missing return type annotation for public function test_performance_with_complex_filters

Add return type annotation: None

(ANN201)

1815-1815: Unused function argument: db_session

(ARG001)

1815-1815: Trailing comma missing

Add trailing comma

(COM812)

1828-1828: Trailing comma missing

Add trailing comma

(COM812)

1836-1836: Trailing comma missing

Add trailing comma

(COM812)

1837-1837: Trailing comma missing

Add trailing comma

(COM812)

1838-1838: Trailing comma missing

Add trailing comma

(COM812)

1847-1847: Trailing comma missing

Add trailing comma

(COM812)

1849-1849: Trailing comma missing

Add trailing comma

(COM812)

1851-1851: Trailing comma missing

Add trailing comma

(COM812)

1864-1864: Missing return type annotation for public function test_filter_precedence_and_grouping

Add return type annotation: None

(ANN201)

1865-1865: Unused function argument: db_session

(ARG001)

1865-1865: Trailing comma missing

Add trailing comma

(COM812)

1904-1904: Trailing comma missing

Add trailing comma

(COM812)

1910-1910: Trailing comma missing

Add trailing comma

(COM812)

1912-1912: Trailing comma missing

Add trailing comma

(COM812)

1913-1913: Trailing comma missing

Add trailing comma

(COM812)

1930-1930: Missing return type annotation for public function test_jsonb_contains_vs_equality_semantics

Add return type annotation: None

(ANN201)

1931-1931: Unused function argument: db_session

(ARG001)

1931-1931: Trailing comma missing

Add trailing comma

(COM812)

1965-1965: Trailing comma missing

Add trailing comma

(COM812)

1983-1983: Missing return type annotation for public function test_wildcard_edge_cases_comprehensive

Add return type annotation: None

(ANN201)

1984-1984: Unused function argument: db_session

(ARG001)

1984-1984: Trailing comma missing

Add trailing comma

(COM812)

1999-1999: Trailing comma missing

Add trailing comma

(COM812)

2012-2012: Trailing comma missing

Add trailing comma

(COM812)

2013-2013: Trailing comma missing

Add trailing comma

(COM812)

2030-2030: Trailing comma missing

Add trailing comma

(COM812)

2031-2031: Trailing comma missing

Add trailing comma

(COM812)

2042-2042: Missing return type annotation for public function test_error_logging_and_debugging

Add return type annotation: None

(ANN201)

2043-2043: Unused function argument: db_session

(ARG001)

2043-2043: Trailing comma missing

Add trailing comma

(COM812)

2063-2063: Missing return type annotation for public function test_boundary_conditions_numeric

Add return type annotation: None

(ANN201)

2064-2064: Unused function argument: db_session

(ARG001)

2064-2064: Trailing comma missing

Add trailing comma

(COM812)

2098-2098: Trailing comma missing

Add trailing comma

(COM812)

🪛 GitHub Check: basedpyright

src/crud.py

[warning] 21-21:
Type of "apply_filter" is partially unknown
Type of "apply_filter" is "(stmt: Select[Unknown], model_class: Unknown, filter: dict[str, Any] | None = None) -> Select[Unknown]" (reportUnknownVariableType)

[warning] 102-102:
Type of "stmt" is partially unknown
Type of "stmt" is "Select[Unknown]" (reportUnknownVariableType)

[warning] 249-249:
Type of "stmt" is partially unknown
Type of "stmt" is "Select[Unknown]" (reportUnknownVariableType)

[warning] 320-320:
Type of "stmt" is partially unknown
Type of "stmt" is "Select[Unknown]" (reportUnknownVariableType)

[warning] 341-341:
Type of "stmt" is partially unknown
Type of "stmt" is "Select[Unknown]" (reportUnknownVariableType)

[warning] 1282-1282:
Type of "stmt" is partially unknown
Type of "stmt" is "Select[Unknown]" (reportUnknownVariableType)

src/utils/filter.py

[failure] 364-364:
Expected type arguments for generic class "list" (reportMissingTypeArgument)

[failure] 50-50:
Expected type arguments for generic class "Select" (reportMissingTypeArgument)

[failure] 49-49:
Expected type arguments for generic class "Select" (reportMissingTypeArgument)

🪛 GitHub Actions: Static Analysis

src/crud.py

[warning] 21-21: Type of "apply_filter" is partially unknown (reportUnknownVariableType)

[warning] 102-102: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 249-249: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 251-251: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 253-253: Return type, "Select[Unknown]", is partially unknown (reportUnknownVariableType)

[warning] 320-320: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 341-341: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 343-343: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 345-345: Return type, "Select[Unknown]", is partially unknown (reportUnknownVariableType)

[warning] 1282-1282: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1285-1285: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1289-1289: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1291-1291: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1312-1312: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1316-1316: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1318-1318: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1322-1322: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1324-1324: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1326-1326: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1328-1328: Return type, "Select[Unknown]", is partially unknown (reportUnknownVariableType)

[warning] 1395-1395: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1398-1398: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1400-1400: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1402-1402: Return type, "Select[Unknown]", is partially unknown (reportUnknownVariableType)

[warning] 1535-1535: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1536-1536: Type of "stmt" is partially unknown (reportUnknownVariableType)

[warning] 1539-1539: Type of "result" is partially unknown (reportUnknownVariableType)

[warning] 1539-1539: Argument type is partially unknown for parameter "statement" in function "execute" (reportUnknownArgumentType)

[warning] 1540-1540: Return type, "Sequence[Unknown]", is partially unknown (reportUnknownVariableType)

src/utils/filter.py

[warning] 48-48: Return type, "Select[Unknown]", is partially unknown (reportUnknownParameterType)

[warning] 49-49: Type of parameter "stmt" is partially unknown (reportUnknownParameterType)

[error] 49-49: Expected type arguments for generic class "Select" (reportMissingTypeArgument)

🪛 Flake8 (7.2.0)

src/utils/filter.py

[error] 44-44: too many leading '#' for block comment

(E266)

🔇 Additional comments (21)

src/exceptions.py (1)

68-73: LGTM! Well-structured exception for filtering validation.

The new FilterError exception follows the established patterns in the codebase with appropriate status code (422) for validation errors and a clear detail message. This aligns perfectly with the PR's filtering enhancement objectives.

src/schemas.py (1)

250-250: LGTM! Improved code formatting.

The addition of blank lines before field declarations enhances readability of the Pydantic model definitions.

Also applies to: 258-258, 268-268, 282-282, 290-290, 296-296

src/routers/peers.py (4)

14-14: LGTM! Updated to async pagination.

The import change from paginate to apaginate aligns with the async pagination improvements across the codebase.

54-57: LGTM! Consistent async pagination usage.

The function correctly uses apaginate for async pagination with proper filter parameter handling.

132-144: LGTM! Simplified filter handling aligns with new filtering architecture.

The removal of is_active parameter handling and the simplified filter extraction aligns well with the centralized filtering approach introduced in this PR. The filter parameter is now handled consistently through the new apply_filter function.

285-285: LGTM! Consistent async pagination adoption.

All pagination calls have been properly updated to use apaginate for async operations.

Also applies to: 335-335

src/routers/sessions.py (2)

5-5: LGTM! Updated to async pagination import.

The import change supports the async pagination improvements throughout the application.

355-355: LGTM! Consistent async pagination adoption.

All pagination endpoints have been properly updated to use apaginate.

Also applies to: 462-462

tests/routes/test_sessions.py (3)

189-189: LGTM! Updated filter structure aligns with new filtering architecture.

The change from flat filter {"test_key": "test_value"} to nested metadata filter {"metadata": {"test_key": "test_value"}} correctly reflects the new centralized filtering structure introduced in this PR.

194-194: LGTM! More precise assertion.

The change from > 0 to == 1 provides a more exact and reliable test assertion.

359-359: LGTM! Consistent filter structure for session deletion test.

The updated filter structure properly wraps the is_active filter in the expected format, maintaining consistency with the new filtering paradigm.

tests/routes/test_peers.py (2)

113-122: LGTM: Filter structure updated for new filtering system.

The changes correctly implement the new nested filter structure under the "metadata" key, which aligns with the centralized filtering system introduced in this PR. The assertion change from >= 2 to exactly == 2 makes the test more precise and deterministic.

124-134: LGTM: Well-implemented NOT operator test.

The new test correctly validates the NOT operator functionality by:

Creating peers with different metadata values ("peer_value" and "peer_value2")

Using NOT filter to exclude peers with "peer_value2"

Expecting exactly 3 results (2 peers with "peer_value" + 1 sample peer with empty metadata)

The test logic is sound and properly validates the advanced filtering capabilities.

src/crud.py (5)

21-21: LGTM: Centralized filtering import added.

The import of apply_filter from src.utils.filter supports the centralized filtering refactoring implemented across all query functions in this file.

102-102: LGTM: Consistent centralized filtering implementation.

The refactoring successfully replaces custom filtering logic with the centralized apply_filter function across workspace, peer, and session queries. This approach:

Standardizes filtering: All queries now use the same filtering mechanism

Supports advanced features: Enables logical operators, comparison operators, and wildcards

Improves maintainability: Centralizes filtering logic in one place

The function calls are consistent: apply_filter(stmt, ModelClass, filter)

Also applies to: 249-249, 320-320, 341-341

293-296: LGTM: Function signatures updated to remove deprecated is_active parameter.

The removal of the is_active parameter from get_sessions_for_peer and get_sessions functions aligns with the broader refactoring that deprecates is_active filtering in favor of the more flexible centralized filtering system.

Also applies to: 332-334

1282-1282: LGTM: Message filtering consistently refactored.

All message query functions (get_messages, get_messages_for_peer) now use the centralized filtering approach. The placement of apply_filter calls is appropriate - applied after base conditions but before ordering and pagination logic.

Also applies to: 1312-1312, 1322-1322, 1395-1395

1535-1535: LGTM: Document filtering integrated with centralized system.

The query_documents function correctly incorporates the centralized filtering approach, maintaining consistency with other query functions throughout the codebase.
src/utils/filter.py (2)

496-537: Excellent datetime validation implementation!

This function properly addresses SQL injection concerns by validating datetime strings before they're used in queries. The support for multiple formats is comprehensive.

139-154: Fix the NOT operator logic to match user expectations.

The current NOT implementation applies NOT to each condition individually then ANDs them together. This results in NOT(A) AND NOT(B) when users likely expect NOT(A OR B) which equals NOT(A) AND NOT(B) by De Morgan's law. However, without the implicit OR, the current behavior may be confusing.

Consider this more intuitive implementation:
-    if "NOT" in filter_dict:
-        if filter_dict["NOT"] is None:
-            raise FilterError("NOT operator cannot be None")
-        if not isinstance(filter_dict["NOT"], list):
-            raise FilterError(
-                f"NOT operator must contain a list, got {type(filter_dict['NOT']).__name__}"
-            )
-        not_conditions = []
-        for sub_filter in filter_dict["NOT"]:
-            sub_condition = _build_filter_conditions(sub_filter, model_class)
-            if sub_condition is not None:
-                not_conditions.append(
-                    not_(sub_condition)
-                )  # Apply NOT to each condition individually
-        if not_conditions:
-            conditions.append(and_(*not_conditions))  # Then AND them together
+    if "NOT" in filter_dict:
+        if filter_dict["NOT"] is None:
+            raise FilterError("NOT operator cannot be None")
+        if not isinstance(filter_dict["NOT"], list):
+            raise FilterError(
+                f"NOT operator must contain a list, got {type(filter_dict['NOT']).__name__}"
+            )
+        # Build OR of all sub-conditions first, then apply NOT
+        or_conditions = []
+        for sub_filter in filter_dict["NOT"]:
+            sub_condition = _build_filter_conditions(sub_filter, model_class)
+            if sub_condition is not None:
+                or_conditions.append(sub_condition)
+        if or_conditions:
+            # Apply NOT to the OR of all conditions: NOT(A OR B OR ...)
+            conditions.append(not_(or_(*or_conditions)))
This change makes NOT: [{A}, {B}] equivalent to "exclude items matching A OR B", which is more intuitive than the current "exclude items matching A AND B".

Likely an incorrect or invalid review comment.
tests/test_advanced_filters.py (1)

1365-1423: This test confirms the NOT operator logic issue.

The test correctly identifies that the current NOT implementation doesn't match user expectations. When users specify NOT: [A, B], they expect to exclude items matching A OR B, but the current implementation excludes items matching A AND B.

This test will pass once the NOT operator logic in src/utils/filter.py is fixed as suggested in the review comment.

coderabbitai · 2025-06-25T17:15:09Z

+def _build_comparison_conditions(
+    column, column_name: str, comparisons: dict[str, Any]
+) -> Any:
+    """
+    Build comparison conditions for a single column.
+
+    Args:
+        column: SQLAlchemy column object
+        column_name: Name of the column
+        comparisons: Dictionary of comparison operators and values
+
+    Returns:
+        Combined SQLAlchemy condition object or None
+    """
+    conditions = []
+
+    # Check if this is a datetime column
+    is_datetime_column = hasattr(column.type, "python_type") and issubclass(
+        column.type.python_type, datetime.datetime
+    )
+
+    for operator, op_value in comparisons.items():
+        # Validate that the operator is supported
+        if operator not in COMPARISON_OPERATORS:
+            raise FilterError(f"Unsupported comparison operator: {operator}")
+
+        # Handle wildcard - matches everything, so no condition needed
+        if op_value == "*":
+            continue
+
+        condition = None
+
+        # For datetime columns, cast string values to timestamp
+        if is_datetime_column and isinstance(op_value, str):
+            # Validate datetime string to prevent SQL injection
+            validated_datetime = _validate_datetime_string(op_value)
+            if validated_datetime is None:
+                # Raise error if datetime validation fails
+                raise FilterError(f"Invalid datetime value: {op_value}")
+
+            # Use the validated datetime object directly instead of string interpolation
+            casted_value = validated_datetime
+        else:
+            # if the operator is a numeric operator, the value must cast to a number
+            if operator in NUMERIC_OPERATORS:
+                try:
+                    casted_value = float(op_value)
+                except ValueError:
+                    raise FilterError(
+                        f"Invalid numeric value: {op_value}. Expected a number, got {type(op_value).__name__}"
+                    ) from None
+            else:
+                casted_value = op_value
+
+        if operator == "gte":
+            condition = column >= casted_value
+        elif operator == "lte":
+            condition = column <= casted_value
+        elif operator == "gt":
+            condition = column > casted_value
+        elif operator == "lt":
+            condition = column < casted_value
+        elif operator == "ne":
+            condition = column != casted_value
+        elif operator == "in":
+            if hasattr(op_value, "__iter__") and not isinstance(op_value, str | bytes):
+                # Handle wildcard in iterable - if present, matches everything, so no condition needed
+                if "*" in op_value:
+                    continue
+                else:
+                    if is_datetime_column:
+                        # Validate and cast each datetime string value
+                        casted_values = []
+                        for val in op_value:
+                            if isinstance(val, str):
+                                validated_datetime = _validate_datetime_string(val)
+                                if validated_datetime is None:
+                                    raise FilterError(
+                                        f"Invalid datetime value in list: {val}"
+                                    )
+                                casted_values.append(validated_datetime)
+                            else:
+                                casted_values.append(val)
+                        if casted_values:
+                            condition = column.in_(casted_values)
+                    else:
+                        condition = column.in_(list(op_value))
+            else:
+                raise FilterError(
+                    f"Invalid value for 'in' operator: {op_value}. Expected an iterable (list, tuple, set), got {type(op_value).__name__}"
+                )
+        elif operator == "contains":
+            if column_name == "h_metadata":
+                # For JSONB columns, use JSONB contains
+                condition = column.contains(op_value)
+            else:
+                # For text columns, use ILIKE
+                condition = column.ilike(f"%{op_value}%")
+        elif operator == "icontains":
+            # Case-insensitive contains for text columns
+            condition = column.ilike(f"%{op_value}%")
+
+        if condition is not None:
+            conditions.append(condition)
+
+    # Combine all conditions for this field with AND
+    if len(conditions) == 0:
+        return None
+    elif len(conditions) == 1:
+        return conditions[0]
+    else:
+        return and_(*conditions)
+


🧹 Nitpick (assertive)

Consider refactoring this complex function for better maintainability.

While the logic is correct, this function has very high complexity (32 branches, 61 statements). Consider extracting operator-specific logic into helper functions.

Example refactoring approach:

def _build_datetime_comparison(column, operator: str, value: Any) -> Any: """Build comparison for datetime columns.""" if isinstance(value, str): validated = _validate_datetime_string(value) if validated is None: raise FilterError(f"Invalid datetime value: {value}") value = validated return _apply_operator(column, operator, value) def _build_numeric_comparison(column, operator: str, value: Any) -> Any: """Build comparison for numeric values.""" if operator in NUMERIC_OPERATORS: try: value = float(value) except ValueError: raise FilterError( f"Invalid numeric value: {value}. Expected a number, got {type(value).__name__}" ) return _apply_operator(column, operator, value) def _apply_operator(column, operator: str, value: Any) -> Any: """Apply the comparison operator.""" operator_map = { "gte": lambda c, v: c >= v, "lte": lambda c, v: c <= v, "gt": lambda c, v: c > v, "lt": lambda c, v: c < v, "ne": lambda c, v: c != v, } if operator in operator_map: return operator_map[operator](column, value) # Handle other operators...

🧰 Tools

🪛 Ruff (0.11.9)

382-382: _build_comparison_conditions is too complex (27 > 10)

(C901)

383-383: Missing type annotation for function argument column

(ANN001)

383-383: Trailing comma missing

Add trailing comma

(COM812)

384-384: Dynamically typed expressions (typing.Any) are disallowed in _build_comparison_conditions

(ANN401)

400-400: Trailing comma missing

Add trailing comma

(COM812)

406-406: Avoid specifying long messages outside the exception class

(TRY003)

420-420: Avoid specifying long messages outside the exception class

(TRY003)

430-432: Avoid specifying long messages outside the exception class

(TRY003)

431-431: Trailing comma missing

Add trailing comma

(COM812)

451-451: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

459-461: Avoid specifying long messages outside the exception class

(TRY003)

460-460: Trailing comma missing

Add trailing comma

(COM812)

470-472: Avoid specifying long messages outside the exception class

(TRY003)

471-471: Trailing comma missing

Add trailing comma

(COM812)

490-490: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

🪛 Pylint (3.3.7)

[convention] 431-431: Line too long (110/100)

(C0301)

[convention] 448-448: Line too long (102/100)

(C0301)

[convention] 471-471: Line too long (138/100)

(C0301)

[refactor] 449-468: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 403-485: Too many nested blocks (8/5)

(R1702)

[refactor] 403-485: Too many nested blocks (6/5)

(R1702)

[refactor] 488-493: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 382-382: Too many branches (32/12)

(R0912)

[refactor] 382-382: Too many statements (61/50)

(R0915)

🤖 Prompt for AI Agents

In src/utils/filter.py between lines 382 and 494, the _build_comparison_conditions function is overly complex with many branches and statements, making it hard to maintain. Refactor by extracting operator-specific logic into separate helper functions, such as one for datetime comparisons that validates and casts datetime strings, another for numeric comparisons that casts values to float and handles errors, and a general function to apply operators to columns. This modular approach will simplify the main function and improve readability and maintainability.

coderabbitai · 2025-06-25T17:15:09Z

+
+
+def apply_filter(
+    stmt: Select, model_class, filter: dict[str, Any] | None = None


🧹 Nitpick (assertive)

Add missing type annotations for better type safety.

The model_class parameter is missing a type annotation, and the Select type is missing type arguments.

-def apply_filter( - stmt: Select, model_class, filter: dict[str, Any] | None = None -) -> Select: +from typing import Type, TypeVar +from sqlalchemy.orm import DeclarativeBase + +T = TypeVar('T', bound=DeclarativeBase) + +def apply_filter( + stmt: Select[tuple[T]], model_class: Type[T], filter: dict[str, Any] | None = None +) -> Select[tuple[T]]:

📝 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.

Suggested change

stmt: Select, model_class, filter: dict[str, Any] | None = None

# at the top of src/utils/filter.py, add the new imports and TypeVar

from typing import Type, TypeVar

from sqlalchemy.orm import DeclarativeBase

T = TypeVar('T', bound=DeclarativeBase)

# Update the apply_filter signature

def apply_filter(

stmt: Select[tuple[T]], model_class: Type[T], filter: dict[str, Any] | None = None

) -> Select[tuple[T]]:

# … existing implementation …

...

🧰 Tools

🪛 GitHub Check: basedpyright

[failure] 49-49:
Expected type arguments for generic class "Select" (reportMissingTypeArgument)

🪛 Ruff (0.11.9)

49-49: Missing type annotation for function argument model_class

(ANN001)

49-49: Trailing comma missing

Add trailing comma

(COM812)

🪛 Pylint (3.3.7)

[warning] 49-49: Redefining built-in 'filter'

(W0622)

🪛 GitHub Actions: Static Analysis

[warning] 49-49: Type of parameter "stmt" is partially unknown (reportUnknownParameterType)

[error] 49-49: Expected type arguments for generic class "Select" (reportMissingTypeArgument)

🤖 Prompt for AI Agents

In src/utils/filter.py at line 49, add explicit type annotations for the model_class parameter and specify the type arguments for the Select type. For example, annotate model_class with the appropriate class type or Type[Model] and provide the model class as a generic parameter to Select to improve type safety and clarity.

coderabbitai · 2025-06-25T17:15:10Z

+    import time
+
+    time.sleep(0.1)


⚠️ Potential issue

Replace blocking time.sleep with async asyncio.sleep.

Using time.sleep in an async function blocks the event loop. Use the async equivalent instead.

- # Wait a bit to ensure different timestamps - import time - - time.sleep(0.1) + # Wait a bit to ensure different timestamps + import asyncio + + await asyncio.sleep(0.1)

📝 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.

Suggested change

import time

time.sleep(0.1)

# Wait a bit to ensure different timestamps

import asyncio

await asyncio.sleep(0.1)

🧰 Tools

🪛 Ruff (0.11.9)

1518-1518: Async functions should not call time.sleep

(ASYNC251)

🪛 Pylint (3.3.7)

[convention] 1516-1516: Import outside toplevel (time)

(C0415)

🤖 Prompt for AI Agents

In tests/test_advanced_filters.py around lines 1516 to 1518, replace the blocking call to time.sleep(0.1) with the non-blocking async equivalent asyncio.sleep(0.1). Import asyncio if not already imported, and make sure to await asyncio.sleep(0.1) inside the async function to avoid blocking the event loop.

coderabbitai

Actionable comments posted: 3

♻️ Duplicate comments (1)

tests/test_advanced_filters.py (1)
1514-1516: Replace blocking time.sleep with async asyncio.sleep.

Using time.sleep in an async function blocks the event loop. Use the async equivalent instead.
    # Wait a bit to ensure different timestamps
-    import time
-
-    time.sleep(0.1)
+    import asyncio
+
+    await asyncio.sleep(0.1)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 29ad171 and 52fbc8f.

📒 Files selected for processing (2)

src/utils/filter.py (1 hunks)
tests/test_advanced_filters.py (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

src/utils/filter.py (1)

src/exceptions.py (1)

FilterError (69-73)

🪛 Ruff (0.11.9)

src/utils/filter.py

54-54: Trailing comma missing

Add trailing comma

(COM812)

104-104: _build_filter_conditions is too complex (22 > 10)

(C901)

105-105: Trailing comma missing

Add trailing comma

(COM812)

122-124: Avoid specifying long messages outside the exception class

(TRY003)

123-123: Trailing comma missing

Add trailing comma

(COM812)

135-137: Avoid specifying long messages outside the exception class

(TRY003)

136-136: Trailing comma missing

Add trailing comma

(COM812)

148-148: Avoid specifying long messages outside the exception class

(TRY003)

150-152: Avoid specifying long messages outside the exception class

(TRY003)

151-151: Trailing comma missing

Add trailing comma

(COM812)

158-158: Trailing comma missing

Add trailing comma

(COM812)

176-176: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

183-183: Dynamically typed expressions (typing.Any) are disallowed in value

(ANN401)

183-183: Trailing comma missing

Add trailing comma

(COM812)

202-202: Avoid specifying long messages outside the exception class

(TRY003)

206-206: Avoid specifying long messages outside the exception class

(TRY003)

221-221: Unnecessary else after return statement

Remove unnecessary else

(RET505)

226-226: Unnecessary else after return statement

Remove unnecessary else

(RET505)

231-231: Unnecessary else after return statement

Remove unnecessary else

(RET505)

235-235: Dynamically typed expressions (typing.Any) are disallowed in column_accessor

(ANN401)

235-235: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

251-251: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

261-261: Consider moving this statement to an else block

(TRY300)

266-266: Consider moving this statement to an else block

(TRY300)

272-272: Unnecessary else after return statement

Remove unnecessary else

(RET505)

273-275: Avoid specifying long messages outside the exception class

(TRY003)

274-274: Trailing comma missing

Add trailing comma

(COM812)

279-279: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

279-279: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

279-279: Trailing comma missing

Add trailing comma

(COM812)

295-295: Avoid specifying long messages outside the exception class

(TRY003)

314-314: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

320-320: Unnecessary else after return statement

Remove unnecessary else

(RET505)

321-323: Avoid specifying long messages outside the exception class

(TRY003)

322-322: Trailing comma missing

Add trailing comma

(COM812)

331-331: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

331-331: Trailing comma missing

Add trailing comma

(COM812)

353-353: Trailing comma missing

Add trailing comma

(COM812)

362-362: Trailing comma missing

Add trailing comma

(COM812)

389-389: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

395-395: _build_comparison_conditions is too complex (27 > 10)

(C901)

396-396: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

396-396: Trailing comma missing

Add trailing comma

(COM812)

413-413: Trailing comma missing

Add trailing comma

(COM812)

419-419: Avoid specifying long messages outside the exception class

(TRY003)

433-433: Avoid specifying long messages outside the exception class

(TRY003)

443-445: Avoid specifying long messages outside the exception class

(TRY003)

444-444: Trailing comma missing

Add trailing comma

(COM812)

464-464: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

472-474: Avoid specifying long messages outside the exception class

(TRY003)

473-473: Trailing comma missing

Add trailing comma

(COM812)

483-485: Avoid specifying long messages outside the exception class

(TRY003)

484-484: Trailing comma missing

Add trailing comma

(COM812)

503-503: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

538-538: Naive datetime constructed using datetime.datetime.strptime() without %z

(DTZ007)

tests/test_advanced_filters.py

17-17: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

18-18: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

18-18: Trailing comma missing

Add trailing comma

(COM812)

66-66: Trailing comma missing

Add trailing comma

(COM812)

67-67: Trailing comma missing

Add trailing comma

(COM812)

83-83: Trailing comma missing

Add trailing comma

(COM812)

84-84: Trailing comma missing

Add trailing comma

(COM812)

105-105: Missing return type annotation for public function test_comparison_operators_filters

Add return type annotation: None

(ANN201)

106-106: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

106-106: Trailing comma missing

Add trailing comma

(COM812)

143-143: Trailing comma missing

Add trailing comma

(COM812)

223-223: Missing return type annotation for public function test_wildcard_filters

Add return type annotation: None

(ANN201)

224-224: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

224-224: Trailing comma missing

Add trailing comma

(COM812)

250-250: Trailing comma missing

Add trailing comma

(COM812)

251-251: Trailing comma missing

Add trailing comma

(COM812)

265-265: Trailing comma missing

Add trailing comma

(COM812)

266-266: Trailing comma missing

Add trailing comma

(COM812)

276-276: Missing return type annotation for public function test_complex_nested_filters

Add return type annotation: None

(ANN201)

277-277: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

277-277: Trailing comma missing

Add trailing comma

(COM812)

330-330: Trailing comma missing

Add trailing comma

(COM812)

344-344: Trailing comma missing

Add trailing comma

(COM812)

348-348: Trailing comma missing

Add trailing comma

(COM812)

349-349: Trailing comma missing

Add trailing comma

(COM812)

365-365: Missing return type annotation for public function test_filters_across_different_models

Add return type annotation: None

(ANN201)

366-366: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

366-366: Trailing comma missing

Add trailing comma

(COM812)

388-388: Trailing comma missing

Add trailing comma

(COM812)

389-389: Trailing comma missing

Add trailing comma

(COM812)

425-425: Trailing comma missing

Add trailing comma

(COM812)

426-426: Trailing comma missing

Add trailing comma

(COM812)

437-437: Missing return type annotation for public function test_filter_edge_cases

Add return type annotation: None

(ANN201)

438-438: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

438-438: Trailing comma missing

Add trailing comma

(COM812)

448-448: Trailing comma missing

Add trailing comma

(COM812)

449-449: Trailing comma missing

Add trailing comma

(COM812)

476-476: Trailing comma missing

Add trailing comma

(COM812)

477-477: Trailing comma missing

Add trailing comma

(COM812)

488-488: Missing return type annotation for public function test_backward_compatibility

Add return type annotation: None

(ANN201)

489-489: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

489-489: Trailing comma missing

Add trailing comma

(COM812)

526-526: Missing return type annotation for public function test_range_queries_with_dates

Add return type annotation: None

(ANN201)

527-527: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

527-527: Trailing comma missing

Add trailing comma

(COM812)

567-567: Trailing comma missing

Add trailing comma

(COM812)

568-568: Trailing comma missing

Add trailing comma

(COM812)

586-586: Trailing comma missing

Add trailing comma

(COM812)

587-587: Trailing comma missing

Add trailing comma

(COM812)

599-599: Missing return type annotation for public function test_all_workspace_columns_filtering

Add return type annotation: None

(ANN201)

686-686: Missing return type annotation for public function test_all_peer_columns_filtering

Add return type annotation: None

(ANN201)

687-687: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

687-687: Trailing comma missing

Add trailing comma

(COM812)

776-776: Missing return type annotation for public function test_all_session_columns_filtering

Add return type annotation: None

(ANN201)

777-777: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

777-777: Trailing comma missing

Add trailing comma

(COM812)

877-877: Missing return type annotation for public function test_all_message_columns_filtering

Add return type annotation: None

(ANN201)

878-878: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

878-878: Trailing comma missing

Add trailing comma

(COM812)

911-911: Trailing comma missing

Add trailing comma

(COM812)

1013-1013: Missing return type annotation for public function test_id_field_interpolation_consistency

Add return type annotation: None

(ANN201)

1014-1014: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1014-1014: Trailing comma missing

Add trailing comma

(COM812)

1053-1053: Trailing comma missing

Add trailing comma

(COM812)

1054-1054: Trailing comma missing

Add trailing comma

(COM812)

1103-1103: Missing return type annotation for public function test_foreign_key_field_interpolation

Add return type annotation: None

(ANN201)

1104-1104: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1104-1104: Trailing comma missing

Add trailing comma

(COM812)

1136-1136: Trailing comma missing

Add trailing comma

(COM812)

1137-1137: Trailing comma missing

Add trailing comma

(COM812)

1191-1191: Missing return type annotation for public function test_metadata_field_interpolation

Add return type annotation: None

(ANN201)

1235-1235: Trailing comma missing

Add trailing comma

(COM812)

1236-1236: Trailing comma missing

Add trailing comma

(COM812)

1314-1314: Missing return type annotation for public function test_nonexistent_columns_ignored_gracefully

Add return type annotation: None

(ANN201)

1315-1315: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1315-1315: Trailing comma missing

Add trailing comma

(COM812)

1342-1342: Trailing comma missing

Add trailing comma

(COM812)

1343-1343: Trailing comma missing

Add trailing comma

(COM812)

1356-1356: Trailing comma missing

Add trailing comma

(COM812)

1357-1357: Trailing comma missing

Add trailing comma

(COM812)

1364-1364: Missing return type annotation for public function test_not_logic_correctness

Add return type annotation: None

(ANN201)

1365-1365: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1365-1365: Trailing comma missing

Add trailing comma

(COM812)

1404-1404: Trailing comma missing

Add trailing comma

(COM812)

1405-1405: Trailing comma missing

Add trailing comma

(COM812)

1424-1424: Missing return type annotation for public function test_jsonb_type_casting_edge_cases

Add return type annotation: None

(ANN201)

1425-1425: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1425-1425: Trailing comma missing

Add trailing comma

(COM812)

1461-1461: Trailing comma missing

Add trailing comma

(COM812)

1498-1498: Missing return type annotation for public function test_real_datetime_column_filtering

Add return type annotation: None

(ANN201)

1499-1499: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1499-1499: Trailing comma missing

Add trailing comma

(COM812)

1516-1516: Async functions should not call time.sleep

(ASYNC251)

1548-1548: Missing return type annotation for public function test_invalid_datetime_handling

Add return type annotation: None

(ANN201)

1549-1549: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1549-1549: Trailing comma missing

Add trailing comma

(COM812)

1571-1571: Missing return type annotation for public function test_nested_jsonb_filtering

Add return type annotation: None

(ANN201)

1572-1572: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1572-1572: Trailing comma missing

Add trailing comma

(COM812)

1608-1608: Trailing comma missing

Add trailing comma

(COM812)

1624-1624: Missing return type annotation for public function test_multiple_operators_same_field

Add return type annotation: None

(ANN201)

1625-1625: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1625-1625: Trailing comma missing

Add trailing comma

(COM812)

1661-1661: Trailing comma missing

Add trailing comma

(COM812)

1677-1677: Missing return type annotation for public function test_empty_and_null_filter_handling

Add return type annotation: None

(ANN201)

1678-1678: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1678-1678: Trailing comma missing

Add trailing comma

(COM812)

1685-1685: Trailing comma missing

Add trailing comma

(COM812)

1694-1694: Trailing comma missing

Add trailing comma

(COM812)

1707-1707: Missing return type annotation for public function test_unicode_and_special_characters

Add return type annotation: None

(ANN201)

1708-1708: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1708-1708: Trailing comma missing

Add trailing comma

(COM812)

1738-1738: Missing return type annotation for public function test_case_sensitivity_edge_cases

Add return type annotation: None

(ANN201)

1739-1739: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1739-1739: Trailing comma missing

Add trailing comma

(COM812)

1786-1786: Missing return type annotation for public function test_malformed_filter_structures

Add return type annotation: None

(ANN201)

1787-1787: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1787-1787: Trailing comma missing

Add trailing comma

(COM812)

1812-1812: Missing return type annotation for public function test_performance_with_complex_filters

Add return type annotation: None

(ANN201)

1813-1813: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1813-1813: Trailing comma missing

Add trailing comma

(COM812)

1826-1826: Trailing comma missing

Add trailing comma

(COM812)

1834-1834: Trailing comma missing

Add trailing comma

(COM812)

1835-1835: Trailing comma missing

Add trailing comma

(COM812)

1836-1836: Trailing comma missing

Add trailing comma

(COM812)

1845-1845: Trailing comma missing

Add trailing comma

(COM812)

1847-1847: Trailing comma missing

Add trailing comma

(COM812)

1849-1849: Trailing comma missing

Add trailing comma

(COM812)

1862-1862: Missing return type annotation for public function test_filter_precedence_and_grouping

Add return type annotation: None

(ANN201)

1863-1863: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1863-1863: Trailing comma missing

Add trailing comma

(COM812)

1902-1902: Trailing comma missing

Add trailing comma

(COM812)

1908-1908: Trailing comma missing

Add trailing comma

(COM812)

1910-1910: Trailing comma missing

Add trailing comma

(COM812)

1911-1911: Trailing comma missing

Add trailing comma

(COM812)

1928-1928: Missing return type annotation for public function test_jsonb_contains_vs_equality_semantics

Add return type annotation: None

(ANN201)

1929-1929: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1929-1929: Trailing comma missing

Add trailing comma

(COM812)

1963-1963: Trailing comma missing

Add trailing comma

(COM812)

1981-1981: Missing return type annotation for public function test_wildcard_edge_cases_comprehensive

Add return type annotation: None

(ANN201)

1982-1982: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

1982-1982: Trailing comma missing

Add trailing comma

(COM812)

1997-1997: Trailing comma missing

Add trailing comma

(COM812)

2010-2010: Trailing comma missing

Add trailing comma

(COM812)

2011-2011: Trailing comma missing

Add trailing comma

(COM812)

2028-2028: Trailing comma missing

Add trailing comma

(COM812)

2029-2029: Trailing comma missing

Add trailing comma

(COM812)

2040-2040: Missing return type annotation for public function test_error_logging_and_debugging

Add return type annotation: None

(ANN201)

2041-2041: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

2041-2041: Trailing comma missing

Add trailing comma

(COM812)

2061-2061: Missing return type annotation for public function test_boundary_conditions_numeric

Add return type annotation: None

(ANN201)

2062-2062: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

2062-2062: Trailing comma missing

Add trailing comma

(COM812)

2096-2096: Trailing comma missing

Add trailing comma

(COM812)

🪛 GitHub Check: basedpyright

src/utils/filter.py

[warning] 253-253:
Declaration "numeric_accessor" is obscured by a declaration of the same name (reportRedeclaration)

[warning] 225-225:
Argument type is partially unknown
Argument corresponds to parameter "metadata_dict" in function "_build_nested_metadata_conditions"
Argument type is "dict[Unknown, Unknown]" (reportUnknownArgumentType)

[warning] 220-220:
Argument type is partially unknown
Argument corresponds to parameter "comparisons" in function "_build_comparison_conditions"
Argument type is "dict[Unknown, Unknown]" (reportUnknownArgumentType)

[warning] 217-217:
Type of "op_key" is unknown (reportUnknownVariableType)

[warning] 155-155:
Argument type is unknown
Argument corresponds to parameter "filter_dict" in function "_build_filter_conditions" (reportUnknownArgumentType)

[warning] 154-154:
Type of "sub_filter" is unknown (reportUnknownVariableType)

[warning] 140-140:
Argument type is unknown
Argument corresponds to parameter "filter_dict" in function "_build_filter_conditions" (reportUnknownArgumentType)

[warning] 139-139:
Type of "sub_filter" is unknown (reportUnknownVariableType)

[warning] 127-127:
Argument type is unknown
Argument corresponds to parameter "filter_dict" in function "_build_filter_conditions" (reportUnknownArgumentType)

[warning] 126-126:
Type of "sub_filter" is unknown (reportUnknownVariableType)

🪛 Flake8 (7.2.0)

src/utils/filter.py

[error] 49-49: too many leading '#' for block comment

(E266)

🪛 Pylint (3.3.7)

src/utils/filter.py

[convention] 223-223: Line too long (106/100)

(C0301)

[convention] 274-274: Line too long (123/100)

(C0301)

[convention] 322-322: Line too long (134/100)

(C0301)

[convention] 444-444: Line too long (110/100)

(C0301)

[convention] 461-461: Line too long (102/100)

(C0301)

[convention] 484-484: Line too long (138/100)

(C0301)

[warning] 49-49: # TODO any others?

(W0511)

[convention] 1-1: Missing module docstring

(C0114)

[error] 6-6: Unable to import 'sqlalchemy'

(E0401)

[error] 7-7: Unable to import 'sqlalchemy.sql.elements'

(E0401)

[error] 8-8: Unable to import 'sqlalchemy.types'

(E0401)

[warning] 54-54: Redefining built-in 'filter'

(W0622)

[refactor] 174-179: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 104-104: Too many branches (22/12)

(R0912)

[refactor] 219-227: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 224-227: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 229-232: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 247-275: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 268-275: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 304-325: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 315-323: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 387-392: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 462-481: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 416-498: Too many nested blocks (8/5)

(R1702)

[refactor] 416-498: Too many nested blocks (6/5)

(R1702)

[refactor] 501-506: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 395-395: Too many branches (32/12)

(R0912)

[refactor] 395-395: Too many statements (61/50)

(R0915)

tests/test_advanced_filters.py

[convention] 1395-1395: Line too long (105/100)

(C0301)

[convention] 1396-1396: Line too long (104/100)

(C0301)

[convention] 1415-1415: Line too long (118/100)

(C0301)

[convention] 1-1: Too many lines in module (2120/1000)

(C0302)

[error] 9-9: Unable to import 'fastapi.testclient'

(E0401)

[error] 10-10: Unable to import 'nanoid'

(E0401)

[error] 11-11: Unable to import 'sqlalchemy.ext.asyncio'

(E0401)

[refactor] 877-877: Too many statements (52/50)

(R0915)

[convention] 1514-1514: Import outside toplevel (time)

(C0415)

🪛 GitHub Actions: Static Analysis

src/utils/filter.py

[warning] 126-126: Type of "sub_filter" is unknown (reportUnknownVariableType)

[warning] 127-127: Argument type is unknown for parameter "filter_dict" in function "_build_filter_conditions" (reportUnknownArgumentType)

[warning] 139-139: Type of "sub_filter" is unknown (reportUnknownVariableType)

[warning] 140-140: Argument type is unknown for parameter "filter_dict" in function "_build_filter_conditions" (reportUnknownArgumentType)

[warning] 154-154: Type of "sub_filter" is unknown (reportUnknownVariableType)

[warning] 155-155: Argument type is unknown for parameter "filter_dict" in function "_build_filter_conditions" (reportUnknownArgumentType)

[warning] 217-217: Type of "op_key" is unknown (reportUnknownVariableType)

[warning] 220-220: Argument type is partially unknown for parameter "comparisons" in function "_build_comparison_conditions"; argument type is "dict[Unknown, Unknown]" (reportUnknownArgumentType)

[warning] 225-225: Argument type is partially unknown for parameter "metadata_dict" in function "_build_nested_metadata_conditions"; argument type is "dict[Unknown, Unknown]" (reportUnknownArgumentType)

[warning] 253-253: Declaration "numeric_accessor" is obscured by a declaration of the same name (reportRedeclaration)

[warning] 260-260: Declaration "numeric_accessor" is obscured by a declaration of the same name (reportRedeclaration)

[warning] 347-347: Type of "op" is unknown (reportUnknownVariableType)

[warning] 351-351: Type of "operator" is unknown (reportUnknownVariableType)

[warning] 351-351: Type of "op_value" is unknown (reportUnknownVariableType)

[warning] 353-353: Argument type is unknown for parameter "operator" in function "_build_comparison_condition" (reportUnknownArgumentType)

🪛 GitHub Actions: FastAPI Tests with PostgreSQL and uv

tests/test_advanced_filters.py

[error] 16-16: Pytest fixture '_db_session' not found. The test 'test_logical_operators_and_filters' failed setup due to missing fixture.

🔇 Additional comments (2)

src/utils/filter.py (1)

509-549: Well-implemented datetime validation.

The datetime validation function properly prevents SQL injection by parsing datetime strings before use and supports multiple common formats.

tests/test_advanced_filters.py (1)

1367-1368: Verify the NOT logic test comment.

The comment states "THIS WILL LIKELY FAIL due to broken NOT logic", but the implementation in filter.py appears correct. The NOT operator correctly applies De Morgan's law: NOT([A, B]) becomes NOT(A) AND NOT(B). Please verify if this comment is still accurate or if it needs to be updated.

coderabbitai · 2025-06-25T18:32:03Z

+@pytest.mark.asyncio
+async def test_logical_operators_and_filters(
+    client: TestClient, _db_session: AsyncSession, sample_data: tuple[Workspace, Peer]
+):


⚠️ Potential issue

🧩 Verification correct

Fix missing _db_session fixture causing pipeline failure.

The pipeline indicates that the _db_session fixture is not found, causing test setup failures. Ensure this fixture is properly defined in your test configuration or conftest.py file.

🏁 Script executed:

#!/bin/bash # Description: Search for _db_session fixture definition in test files # Search for fixture definition echo "Searching for _db_session fixture definition:" rg -A 5 "@pytest.fixture.*_db_session" tests/ # Check conftest.py files echo -e "\nChecking conftest.py files:" fd conftest.py tests/ --exec cat {}

Length of output: 15749

Fix missing _db_session fixture causing pipeline failure.

The pipeline indicates that the _db_session fixture is not found, causing test setup failures. Ensure this fixture is properly defined in your test configuration or conftest.py file.

#!/bin/bash # Description: Search for _db_session fixture definition in test files # Search for fixture definition echo "Searching for _db_session fixture definition:" rg -A 5 "@pytest.fixture.*_db_session" tests/ # Check conftest.py files echo -e "\nChecking conftest.py files:" fd conftest.py tests/ --exec cat {}

🧰 Tools

🪛 Ruff (0.11.9)

17-17: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

18-18: Fixture _db_session without value is injected as parameter, use @pytest.mark.usefixtures instead

(PT019)

18-18: Trailing comma missing

Add trailing comma

(COM812)

🪛 GitHub Actions: FastAPI Tests with PostgreSQL and uv

[error] 16-16: Pytest fixture '_db_session' not found. The test 'test_logical_operators_and_filters' failed setup due to missing fixture.

🤖 Prompt for AI Agents

In tests/test_advanced_filters.py around lines 16 to 19, the test function uses a fixture named _db_session which is not found, causing pipeline failures. To fix this, define the _db_session fixture properly either in the test file itself or in a shared conftest.py file within the tests directory. Ensure the fixture provides a valid AsyncSession instance for database interactions during tests.

cheat: sprinkle in some pyright: ignore in filter.py

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (3)

src/utils/filter.py (2)

49-49: Address or remove the TODO comment.

The TODO asks about additional disallowed internal columns. Please either add any missing internal columns that should be disallowed or remove this comment if the list is complete.

247-273: Fix variable redeclaration warnings.

The variable numeric_accessor is declared multiple times in different branches. Consider using unique variable names or restructuring to avoid redeclaration warnings.
tests/test_advanced_filters.py (1)
1510-1514: Replace blocking time.sleep with async asyncio.sleep.

Using time.sleep in an async function blocks the event loop. Use the async equivalent instead.
-    # Wait a bit to ensure different timestamps
-    import time
-
-    time.sleep(0.1)
+    # Wait a bit to ensure different timestamps
+    import asyncio
+
+    await asyncio.sleep(0.1)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52fbc8f and d65b32c.

📒 Files selected for processing (2)

src/utils/filter.py (1 hunks)
tests/test_advanced_filters.py (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

src/utils/filter.py (1)

src/exceptions.py (1)

FilterError (69-73)

🪛 Ruff (0.11.9)

src/utils/filter.py

54-54: Trailing comma missing

Add trailing comma

(COM812)

104-104: _build_filter_conditions is too complex (22 > 10)

(C901)

105-105: Trailing comma missing

Add trailing comma

(COM812)

122-124: Avoid specifying long messages outside the exception class

(TRY003)

123-123: Trailing comma missing

Add trailing comma

(COM812)

135-137: Avoid specifying long messages outside the exception class

(TRY003)

136-136: Trailing comma missing

Add trailing comma

(COM812)

148-148: Avoid specifying long messages outside the exception class

(TRY003)

150-152: Avoid specifying long messages outside the exception class

(TRY003)

151-151: Trailing comma missing

Add trailing comma

(COM812)

158-158: Trailing comma missing

Add trailing comma

(COM812)

176-176: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

183-183: Dynamically typed expressions (typing.Any) are disallowed in value

(ANN401)

183-183: Trailing comma missing

Add trailing comma

(COM812)

202-202: Avoid specifying long messages outside the exception class

(TRY003)

206-206: Avoid specifying long messages outside the exception class

(TRY003)

221-221: Unnecessary else after return statement

Remove unnecessary else

(RET505)

226-226: Unnecessary else after return statement

Remove unnecessary else

(RET505)

231-231: Unnecessary else after return statement

Remove unnecessary else

(RET505)

235-235: Dynamically typed expressions (typing.Any) are disallowed in column_accessor

(ANN401)

235-235: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

251-251: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

269-269: Unnecessary else after return statement

Remove unnecessary else

(RET505)

270-272: Avoid specifying long messages outside the exception class

(TRY003)

271-271: Trailing comma missing

Add trailing comma

(COM812)

276-276: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

276-276: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

276-276: Trailing comma missing

Add trailing comma

(COM812)

292-292: Avoid specifying long messages outside the exception class

(TRY003)

311-311: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

317-317: Unnecessary else after return statement

Remove unnecessary else

(RET505)

318-320: Avoid specifying long messages outside the exception class

(TRY003)

319-319: Trailing comma missing

Add trailing comma

(COM812)

328-328: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

328-328: Trailing comma missing

Add trailing comma

(COM812)

363-363: Trailing comma missing

Add trailing comma

(COM812)

390-390: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

396-396: _build_comparison_conditions is too complex (27 > 10)

(C901)

397-397: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

397-397: Trailing comma missing

Add trailing comma

(COM812)

414-414: Trailing comma missing

Add trailing comma

(COM812)

420-420: Avoid specifying long messages outside the exception class

(TRY003)

434-434: Avoid specifying long messages outside the exception class

(TRY003)

444-446: Avoid specifying long messages outside the exception class

(TRY003)

445-445: Trailing comma missing

Add trailing comma

(COM812)

465-465: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

473-475: Avoid specifying long messages outside the exception class

(TRY003)

474-474: Trailing comma missing

Add trailing comma

(COM812)

484-486: Avoid specifying long messages outside the exception class

(TRY003)

485-485: Trailing comma missing

Add trailing comma

(COM812)

504-504: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

539-539: Naive datetime constructed using datetime.datetime.strptime() without %z

(DTZ007)

tests/test_advanced_filters.py

16-16: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

17-17: Trailing comma missing

Add trailing comma

(COM812)

65-65: Trailing comma missing

Add trailing comma

(COM812)

66-66: Trailing comma missing

Add trailing comma

(COM812)

82-82: Trailing comma missing

Add trailing comma

(COM812)

83-83: Trailing comma missing

Add trailing comma

(COM812)

104-104: Missing return type annotation for public function test_comparison_operators_filters

Add return type annotation: None

(ANN201)

105-105: Trailing comma missing

Add trailing comma

(COM812)

142-142: Trailing comma missing

Add trailing comma

(COM812)

222-222: Missing return type annotation for public function test_wildcard_filters

Add return type annotation: None

(ANN201)

223-223: Trailing comma missing

Add trailing comma

(COM812)

249-249: Trailing comma missing

Add trailing comma

(COM812)

250-250: Trailing comma missing

Add trailing comma

(COM812)

264-264: Trailing comma missing

Add trailing comma

(COM812)

265-265: Trailing comma missing

Add trailing comma

(COM812)

275-275: Missing return type annotation for public function test_complex_nested_filters

Add return type annotation: None

(ANN201)

276-276: Trailing comma missing

Add trailing comma

(COM812)

329-329: Trailing comma missing

Add trailing comma

(COM812)

343-343: Trailing comma missing

Add trailing comma

(COM812)

347-347: Trailing comma missing

Add trailing comma

(COM812)

348-348: Trailing comma missing

Add trailing comma

(COM812)

364-364: Missing return type annotation for public function test_filters_across_different_models

Add return type annotation: None

(ANN201)

365-365: Trailing comma missing

Add trailing comma

(COM812)

387-387: Trailing comma missing

Add trailing comma

(COM812)

388-388: Trailing comma missing

Add trailing comma

(COM812)

424-424: Trailing comma missing

Add trailing comma

(COM812)

425-425: Trailing comma missing

Add trailing comma

(COM812)

436-436: Missing return type annotation for public function test_filter_edge_cases

Add return type annotation: None

(ANN201)

437-437: Trailing comma missing

Add trailing comma

(COM812)

447-447: Trailing comma missing

Add trailing comma

(COM812)

448-448: Trailing comma missing

Add trailing comma

(COM812)

475-475: Trailing comma missing

Add trailing comma

(COM812)

476-476: Trailing comma missing

Add trailing comma

(COM812)

487-487: Missing return type annotation for public function test_backward_compatibility

Add return type annotation: None

(ANN201)

488-488: Trailing comma missing

Add trailing comma

(COM812)

525-525: Missing return type annotation for public function test_range_queries_with_dates

Add return type annotation: None

(ANN201)

526-526: Trailing comma missing

Add trailing comma

(COM812)

566-566: Trailing comma missing

Add trailing comma

(COM812)

567-567: Trailing comma missing

Add trailing comma

(COM812)

585-585: Trailing comma missing

Add trailing comma

(COM812)

586-586: Trailing comma missing

Add trailing comma

(COM812)

598-598: Missing return type annotation for public function test_all_workspace_columns_filtering

Add return type annotation: None

(ANN201)

685-685: Missing return type annotation for public function test_all_peer_columns_filtering

Add return type annotation: None

(ANN201)

686-686: Trailing comma missing

Add trailing comma

(COM812)

775-775: Missing return type annotation for public function test_all_session_columns_filtering

Add return type annotation: None

(ANN201)

776-776: Trailing comma missing

Add trailing comma

(COM812)

876-876: Missing return type annotation for public function test_all_message_columns_filtering

Add return type annotation: None

(ANN201)

877-877: Trailing comma missing

Add trailing comma

(COM812)

910-910: Trailing comma missing

Add trailing comma

(COM812)

1012-1012: Missing return type annotation for public function test_id_field_interpolation_consistency

Add return type annotation: None

(ANN201)

1050-1050: Trailing comma missing

Add trailing comma

(COM812)

1051-1051: Trailing comma missing

Add trailing comma

(COM812)

1100-1100: Missing return type annotation for public function test_foreign_key_field_interpolation

Add return type annotation: None

(ANN201)

1101-1101: Trailing comma missing

Add trailing comma

(COM812)

1133-1133: Trailing comma missing

Add trailing comma

(COM812)

1134-1134: Trailing comma missing

Add trailing comma

(COM812)

1188-1188: Missing return type annotation for public function test_metadata_field_interpolation

Add return type annotation: None

(ANN201)

1232-1232: Trailing comma missing

Add trailing comma

(COM812)

1233-1233: Trailing comma missing

Add trailing comma

(COM812)

1311-1311: Missing return type annotation for public function test_nonexistent_columns_ignored_gracefully

Add return type annotation: None

(ANN201)

1312-1312: Trailing comma missing

Add trailing comma

(COM812)

1339-1339: Trailing comma missing

Add trailing comma

(COM812)

1340-1340: Trailing comma missing

Add trailing comma

(COM812)

1353-1353: Trailing comma missing

Add trailing comma

(COM812)

1354-1354: Trailing comma missing

Add trailing comma

(COM812)

1361-1361: Missing return type annotation for public function test_not_logic_correctness

Add return type annotation: None

(ANN201)

1362-1362: Trailing comma missing

Add trailing comma

(COM812)

1401-1401: Trailing comma missing

Add trailing comma

(COM812)

1402-1402: Trailing comma missing

Add trailing comma

(COM812)

1421-1421: Missing return type annotation for public function test_jsonb_type_casting_edge_cases

Add return type annotation: None

(ANN201)

1422-1422: Trailing comma missing

Add trailing comma

(COM812)

1458-1458: Trailing comma missing

Add trailing comma

(COM812)

1495-1495: Missing return type annotation for public function test_real_datetime_column_filtering

Add return type annotation: None

(ANN201)

1496-1496: Trailing comma missing

Add trailing comma

(COM812)

1513-1513: Async functions should not call time.sleep

(ASYNC251)

1545-1545: Missing return type annotation for public function test_invalid_datetime_handling

Add return type annotation: None

(ANN201)

1546-1546: Trailing comma missing

Add trailing comma

(COM812)

1568-1568: Missing return type annotation for public function test_nested_jsonb_filtering

Add return type annotation: None

(ANN201)

1569-1569: Trailing comma missing

Add trailing comma

(COM812)

1605-1605: Trailing comma missing

Add trailing comma

(COM812)

1621-1621: Missing return type annotation for public function test_multiple_operators_same_field

Add return type annotation: None

(ANN201)

1622-1622: Trailing comma missing

Add trailing comma

(COM812)

1658-1658: Trailing comma missing

Add trailing comma

(COM812)

1674-1674: Missing return type annotation for public function test_empty_and_null_filter_handling

Add return type annotation: None

(ANN201)

1675-1675: Trailing comma missing

Add trailing comma

(COM812)

1682-1682: Trailing comma missing

Add trailing comma

(COM812)

1691-1691: Trailing comma missing

Add trailing comma

(COM812)

1704-1704: Missing return type annotation for public function test_unicode_and_special_characters

Add return type annotation: None

(ANN201)

1705-1705: Trailing comma missing

Add trailing comma

(COM812)

1735-1735: Missing return type annotation for public function test_case_sensitivity_edge_cases

Add return type annotation: None

(ANN201)

1736-1736: Trailing comma missing

Add trailing comma

(COM812)

1783-1783: Missing return type annotation for public function test_malformed_filter_structures

Add return type annotation: None

(ANN201)

1784-1784: Trailing comma missing

Add trailing comma

(COM812)

1809-1809: Missing return type annotation for public function test_performance_with_complex_filters

Add return type annotation: None

(ANN201)

1810-1810: Trailing comma missing

Add trailing comma

(COM812)

1823-1823: Trailing comma missing

Add trailing comma

(COM812)

1831-1831: Trailing comma missing

Add trailing comma

(COM812)

1832-1832: Trailing comma missing

Add trailing comma

(COM812)

1833-1833: Trailing comma missing

Add trailing comma

(COM812)

1842-1842: Trailing comma missing

Add trailing comma

(COM812)

1844-1844: Trailing comma missing

Add trailing comma

(COM812)

1846-1846: Trailing comma missing

Add trailing comma

(COM812)

1859-1859: Missing return type annotation for public function test_filter_precedence_and_grouping

Add return type annotation: None

(ANN201)

1860-1860: Trailing comma missing

Add trailing comma

(COM812)

1899-1899: Trailing comma missing

Add trailing comma

(COM812)

1905-1905: Trailing comma missing

Add trailing comma

(COM812)

1907-1907: Trailing comma missing

Add trailing comma

(COM812)

1908-1908: Trailing comma missing

Add trailing comma

(COM812)

1925-1925: Missing return type annotation for public function test_jsonb_contains_vs_equality_semantics

Add return type annotation: None

(ANN201)

1926-1926: Trailing comma missing

Add trailing comma

(COM812)

1960-1960: Trailing comma missing

Add trailing comma

(COM812)

1978-1978: Missing return type annotation for public function test_wildcard_edge_cases_comprehensive

Add return type annotation: None

(ANN201)

1979-1979: Trailing comma missing

Add trailing comma

(COM812)

1994-1994: Trailing comma missing

Add trailing comma

(COM812)

2007-2007: Trailing comma missing

Add trailing comma

(COM812)

2008-2008: Trailing comma missing

Add trailing comma

(COM812)

2025-2025: Trailing comma missing

Add trailing comma

(COM812)

2026-2026: Trailing comma missing

Add trailing comma

(COM812)

2037-2037: Missing return type annotation for public function test_error_logging_and_debugging

Add return type annotation: None

(ANN201)

2038-2038: Trailing comma missing

Add trailing comma

(COM812)

2058-2058: Missing return type annotation for public function test_boundary_conditions_numeric

Add return type annotation: None

(ANN201)

2059-2059: Trailing comma missing

Add trailing comma

(COM812)

2093-2093: Trailing comma missing

Add trailing comma

(COM812)

🪛 Flake8 (7.2.0)

src/utils/filter.py

[error] 49-49: too many leading '#' for block comment

(E266)

🪛 Pylint (3.3.7)

src/utils/filter.py

[convention] 217-217: Line too long (103/100)

(C0301)

[convention] 223-223: Line too long (106/100)

(C0301)

[convention] 271-271: Line too long (123/100)

(C0301)

[convention] 319-319: Line too long (134/100)

(C0301)

[convention] 445-445: Line too long (110/100)

(C0301)

[convention] 462-462: Line too long (102/100)

(C0301)

[convention] 485-485: Line too long (138/100)

(C0301)

[warning] 49-49: # TODO any others?

(W0511)

[convention] 1-1: Missing module docstring

(C0114)

[error] 6-6: Unable to import 'sqlalchemy'

(E0401)

[error] 7-7: Unable to import 'sqlalchemy.sql.elements'

(E0401)

[error] 8-8: Unable to import 'sqlalchemy.types'

(E0401)

[warning] 54-54: Redefining built-in 'filter'

(W0622)

[refactor] 174-179: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 104-104: Too many branches (22/12)

(R0912)

[refactor] 219-227: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 224-227: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 229-232: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 247-272: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 265-272: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 301-322: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 312-320: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 388-393: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 463-482: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 417-499: Too many nested blocks (8/5)

(R1702)

[refactor] 417-499: Too many nested blocks (6/5)

(R1702)

[refactor] 502-507: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 396-396: Too many branches (32/12)

(R0912)

[refactor] 396-396: Too many statements (61/50)

(R0915)

tests/test_advanced_filters.py

[convention] 1392-1392: Line too long (105/100)

(C0301)

[convention] 1393-1393: Line too long (104/100)

(C0301)

[convention] 1412-1412: Line too long (118/100)

(C0301)

[convention] 1-1: Too many lines in module (2117/1000)

(C0302)

[error] 9-9: Unable to import 'fastapi.testclient'

(E0401)

[error] 10-10: Unable to import 'nanoid'

(E0401)

[refactor] 876-876: Too many statements (52/50)

(R0915)

[convention] 1511-1511: Import outside toplevel (time)

(C0415)

🔇 Additional comments (2)

src/utils/filter.py (1)

510-551: Secure datetime validation implementation.

The _validate_datetime_string function effectively prevents SQL injection by parsing and validating datetime strings before use. Good security practice!

tests/test_advanced_filters.py (1)

1-2118: Comprehensive test coverage!

The test suite thoroughly covers all filtering features including:

Logical operators (AND, OR, NOT)

Comparison operators

Wildcard filtering

Edge cases and error handling

Field interpolation across models

JSONB and datetime handling

Excellent test organization and coverage.

…or anything internal

coderabbitai

Actionable comments posted: 2

♻️ Duplicate comments (3)

src/utils/filter.py (2)
51-53: Rename filter parameter to avoid shadowing built-in.

The parameter name filter shadows Python's built-in filter function. Consider renaming to filter_dict or filters.
 def apply_filter(
-    stmt: Select[tuple[T]], model_class: type[T], filter: dict[str, Any] | None = None
+    stmt: Select[tuple[T]], model_class: type[T], filter_dict: dict[str, Any] | None = None
 ) -> Select[tuple[T]]:
Update all references within the function:
-    if filter is None:
+    if filter_dict is None:
         return stmt

-    conditions = _build_filter_conditions(filter, model_class)
+    conditions = _build_filter_conditions(filter_dict, model_class)
395-506: Consider refactoring this complex function for better maintainability.

While the logic is correct, this function has very high complexity (32 branches, 61 statements). The implementation works well for the complex filtering requirements, but consider extracting operator-specific logic into helper functions if maintainability becomes an issue in the future.

The current implementation correctly handles:

Datetime validation and casting

Numeric type conversion with proper error handling

All comparison operators with appropriate SQL generation

Wildcard support throughout
tests/test_advanced_filters.py (1)

1439-1441: Replace blocking time.sleep with async asyncio.sleep

This issue has already been identified in the previous review.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d65b32c and 85b004c.

📒 Files selected for processing (4)

src/utils/filter.py (1 hunks)
tests/routes/test_scoped_api.py (1 hunks)
tests/routes/test_sessions.py (3 hunks)
tests/test_advanced_filters.py (1 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (1)

src/utils/filter.py (1)

src/exceptions.py (1)

FilterError (69-73)

🪛 Ruff (0.11.9)

src/utils/filter.py

52-52: Trailing comma missing

Add trailing comma

(COM812)

101-101: _build_filter_conditions is too complex (22 > 10)

(C901)

102-102: Trailing comma missing

Add trailing comma

(COM812)

119-121: Avoid specifying long messages outside the exception class

(TRY003)

120-120: Trailing comma missing

Add trailing comma

(COM812)

132-134: Avoid specifying long messages outside the exception class

(TRY003)

133-133: Trailing comma missing

Add trailing comma

(COM812)

145-145: Avoid specifying long messages outside the exception class

(TRY003)

147-149: Avoid specifying long messages outside the exception class

(TRY003)

148-148: Trailing comma missing

Add trailing comma

(COM812)

155-155: Trailing comma missing

Add trailing comma

(COM812)

173-173: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

180-180: Dynamically typed expressions (typing.Any) are disallowed in value

(ANN401)

180-180: Trailing comma missing

Add trailing comma

(COM812)

199-201: Avoid specifying long messages outside the exception class

(TRY003)

200-200: Trailing comma missing

Add trailing comma

(COM812)

205-205: Avoid specifying long messages outside the exception class

(TRY003)

220-220: Unnecessary else after return statement

Remove unnecessary else

(RET505)

225-225: Unnecessary else after return statement

Remove unnecessary else

(RET505)

230-230: Unnecessary else after return statement

Remove unnecessary else

(RET505)

234-234: Dynamically typed expressions (typing.Any) are disallowed in column_accessor

(ANN401)

234-234: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

250-250: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

268-268: Unnecessary else after return statement

Remove unnecessary else

(RET505)

269-271: Avoid specifying long messages outside the exception class

(TRY003)

270-270: Trailing comma missing

Add trailing comma

(COM812)

275-275: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

275-275: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

275-275: Trailing comma missing

Add trailing comma

(COM812)

291-291: Avoid specifying long messages outside the exception class

(TRY003)

310-310: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

316-316: Unnecessary else after return statement

Remove unnecessary else

(RET505)

317-319: Avoid specifying long messages outside the exception class

(TRY003)

318-318: Trailing comma missing

Add trailing comma

(COM812)

327-327: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

327-327: Trailing comma missing

Add trailing comma

(COM812)

362-362: Trailing comma missing

Add trailing comma

(COM812)

389-389: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

395-395: _build_comparison_conditions is too complex (27 > 10)

(C901)

396-396: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

396-396: Trailing comma missing

Add trailing comma

(COM812)

413-413: Trailing comma missing

Add trailing comma

(COM812)

419-419: Avoid specifying long messages outside the exception class

(TRY003)

433-433: Avoid specifying long messages outside the exception class

(TRY003)

443-445: Avoid specifying long messages outside the exception class

(TRY003)

444-444: Trailing comma missing

Add trailing comma

(COM812)

464-464: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

472-474: Avoid specifying long messages outside the exception class

(TRY003)

473-473: Trailing comma missing

Add trailing comma

(COM812)

483-485: Avoid specifying long messages outside the exception class

(TRY003)

484-484: Trailing comma missing

Add trailing comma

(COM812)

503-503: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

538-538: Naive datetime constructed using datetime.datetime.strptime() without %z

(DTZ007)

tests/test_advanced_filters.py

16-16: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

17-17: Trailing comma missing

Add trailing comma

(COM812)

65-65: Trailing comma missing

Add trailing comma

(COM812)

66-66: Trailing comma missing

Add trailing comma

(COM812)

82-82: Trailing comma missing

Add trailing comma

(COM812)

83-83: Trailing comma missing

Add trailing comma

(COM812)

104-104: Missing return type annotation for public function test_comparison_operators_filters

Add return type annotation: None

(ANN201)

105-105: Trailing comma missing

Add trailing comma

(COM812)

142-142: Trailing comma missing

Add trailing comma

(COM812)

203-203: Missing return type annotation for public function test_wildcard_filters

Add return type annotation: None

(ANN201)

204-204: Trailing comma missing

Add trailing comma

(COM812)

230-230: Trailing comma missing

Add trailing comma

(COM812)

231-231: Trailing comma missing

Add trailing comma

(COM812)

245-245: Trailing comma missing

Add trailing comma

(COM812)

246-246: Trailing comma missing

Add trailing comma

(COM812)

256-256: Missing return type annotation for public function test_complex_nested_filters

Add return type annotation: None

(ANN201)

257-257: Trailing comma missing

Add trailing comma

(COM812)

310-310: Trailing comma missing

Add trailing comma

(COM812)

324-324: Trailing comma missing

Add trailing comma

(COM812)

328-328: Trailing comma missing

Add trailing comma

(COM812)

329-329: Trailing comma missing

Add trailing comma

(COM812)

345-345: Missing return type annotation for public function test_filters_across_different_models

Add return type annotation: None

(ANN201)

346-346: Trailing comma missing

Add trailing comma

(COM812)

368-368: Trailing comma missing

Add trailing comma

(COM812)

369-369: Trailing comma missing

Add trailing comma

(COM812)

405-405: Trailing comma missing

Add trailing comma

(COM812)

406-406: Trailing comma missing

Add trailing comma

(COM812)

417-417: Missing return type annotation for public function test_filter_edge_cases

Add return type annotation: None

(ANN201)

418-418: Trailing comma missing

Add trailing comma

(COM812)

428-428: Trailing comma missing

Add trailing comma

(COM812)

429-429: Trailing comma missing

Add trailing comma

(COM812)

456-456: Trailing comma missing

Add trailing comma

(COM812)

457-457: Trailing comma missing

Add trailing comma

(COM812)

468-468: Missing return type annotation for public function test_backward_compatibility

Add return type annotation: None

(ANN201)

469-469: Trailing comma missing

Add trailing comma

(COM812)

506-506: Missing return type annotation for public function test_range_queries_with_dates

Add return type annotation: None

(ANN201)

507-507: Trailing comma missing

Add trailing comma

(COM812)

547-547: Trailing comma missing

Add trailing comma

(COM812)

548-548: Trailing comma missing

Add trailing comma

(COM812)

566-566: Trailing comma missing

Add trailing comma

(COM812)

567-567: Trailing comma missing

Add trailing comma

(COM812)

579-579: Missing return type annotation for public function test_all_workspace_columns_filtering

Add return type annotation: None

(ANN201)

655-655: Missing return type annotation for public function test_all_peer_columns_filtering

Add return type annotation: None

(ANN201)

656-656: Trailing comma missing

Add trailing comma

(COM812)

734-734: Missing return type annotation for public function test_all_session_columns_filtering

Add return type annotation: None

(ANN201)

735-735: Trailing comma missing

Add trailing comma

(COM812)

825-825: Missing return type annotation for public function test_all_message_columns_filtering

Add return type annotation: None

(ANN201)

826-826: Trailing comma missing

Add trailing comma

(COM812)

859-859: Trailing comma missing

Add trailing comma

(COM812)

943-943: Missing return type annotation for public function test_id_field_interpolation_consistency

Add return type annotation: None

(ANN201)

981-981: Trailing comma missing

Add trailing comma

(COM812)

982-982: Trailing comma missing

Add trailing comma

(COM812)

1028-1028: Missing return type annotation for public function test_foreign_key_field_interpolation

Add return type annotation: None

(ANN201)

1029-1029: Trailing comma missing

Add trailing comma

(COM812)

1061-1061: Trailing comma missing

Add trailing comma

(COM812)

1062-1062: Trailing comma missing

Add trailing comma

(COM812)

1116-1116: Missing return type annotation for public function test_metadata_field_interpolation

Add return type annotation: None

(ANN201)

1160-1160: Trailing comma missing

Add trailing comma

(COM812)

1161-1161: Trailing comma missing

Add trailing comma

(COM812)

1239-1239: Missing return type annotation for public function test_nonexistent_columns_ignored_gracefully

Add return type annotation: None

(ANN201)

1240-1240: Trailing comma missing

Add trailing comma

(COM812)

1267-1267: Trailing comma missing

Add trailing comma

(COM812)

1268-1268: Trailing comma missing

Add trailing comma

(COM812)

1281-1281: Trailing comma missing

Add trailing comma

(COM812)

1282-1282: Trailing comma missing

Add trailing comma

(COM812)

1289-1289: Missing return type annotation for public function test_not_logic_correctness

Add return type annotation: None

(ANN201)

1290-1290: Trailing comma missing

Add trailing comma

(COM812)

1329-1329: Trailing comma missing

Add trailing comma

(COM812)

1330-1330: Trailing comma missing

Add trailing comma

(COM812)

1349-1349: Missing return type annotation for public function test_jsonb_type_casting_edge_cases

Add return type annotation: None

(ANN201)

1350-1350: Trailing comma missing

Add trailing comma

(COM812)

1386-1386: Trailing comma missing

Add trailing comma

(COM812)

1423-1423: Missing return type annotation for public function test_real_datetime_column_filtering

Add return type annotation: None

(ANN201)

1424-1424: Trailing comma missing

Add trailing comma

(COM812)

1441-1441: Async functions should not call time.sleep

(ASYNC251)

1473-1473: Missing return type annotation for public function test_invalid_datetime_handling

Add return type annotation: None

(ANN201)

1474-1474: Trailing comma missing

Add trailing comma

(COM812)

1496-1496: Missing return type annotation for public function test_nested_jsonb_filtering

Add return type annotation: None

(ANN201)

1497-1497: Trailing comma missing

Add trailing comma

(COM812)

1533-1533: Trailing comma missing

Add trailing comma

(COM812)

1549-1549: Missing return type annotation for public function test_multiple_operators_same_field

Add return type annotation: None

(ANN201)

1550-1550: Trailing comma missing

Add trailing comma

(COM812)

1586-1586: Trailing comma missing

Add trailing comma

(COM812)

1602-1602: Missing return type annotation for public function test_empty_and_null_filter_handling

Add return type annotation: None

(ANN201)

1603-1603: Trailing comma missing

Add trailing comma

(COM812)

1610-1610: Trailing comma missing

Add trailing comma

(COM812)

1619-1619: Trailing comma missing

Add trailing comma

(COM812)

1632-1632: Missing return type annotation for public function test_unicode_and_special_characters

Add return type annotation: None

(ANN201)

1633-1633: Trailing comma missing

Add trailing comma

(COM812)

1663-1663: Missing return type annotation for public function test_case_sensitivity_edge_cases

Add return type annotation: None

(ANN201)

1664-1664: Trailing comma missing

Add trailing comma

(COM812)

1711-1711: Missing return type annotation for public function test_malformed_filter_structures

Add return type annotation: None

(ANN201)

1712-1712: Trailing comma missing

Add trailing comma

(COM812)

1737-1737: Missing return type annotation for public function test_performance_with_complex_filters

Add return type annotation: None

(ANN201)

1738-1738: Trailing comma missing

Add trailing comma

(COM812)

1751-1751: Trailing comma missing

Add trailing comma

(COM812)

1759-1759: Trailing comma missing

Add trailing comma

(COM812)

1760-1760: Trailing comma missing

Add trailing comma

(COM812)

1761-1761: Trailing comma missing

Add trailing comma

(COM812)

1770-1770: Trailing comma missing

Add trailing comma

(COM812)

1772-1772: Trailing comma missing

Add trailing comma

(COM812)

1774-1774: Trailing comma missing

Add trailing comma

(COM812)

1787-1787: Missing return type annotation for public function test_filter_precedence_and_grouping

Add return type annotation: None

(ANN201)

1788-1788: Trailing comma missing

Add trailing comma

(COM812)

1827-1827: Trailing comma missing

Add trailing comma

(COM812)

1833-1833: Trailing comma missing

Add trailing comma

(COM812)

1835-1835: Trailing comma missing

Add trailing comma

(COM812)

1836-1836: Trailing comma missing

Add trailing comma

(COM812)

1853-1853: Missing return type annotation for public function test_jsonb_contains_vs_equality_semantics

Add return type annotation: None

(ANN201)

1854-1854: Trailing comma missing

Add trailing comma

(COM812)

1888-1888: Trailing comma missing

Add trailing comma

(COM812)

1906-1906: Missing return type annotation for public function test_wildcard_edge_cases_comprehensive

Add return type annotation: None

(ANN201)

1907-1907: Trailing comma missing

Add trailing comma

(COM812)

1922-1922: Trailing comma missing

Add trailing comma

(COM812)

1935-1935: Trailing comma missing

Add trailing comma

(COM812)

1936-1936: Trailing comma missing

Add trailing comma

(COM812)

1953-1953: Trailing comma missing

Add trailing comma

(COM812)

1954-1954: Trailing comma missing

Add trailing comma

(COM812)

1965-1965: Missing return type annotation for public function test_error_logging_and_debugging

Add return type annotation: None

(ANN201)

1966-1966: Trailing comma missing

Add trailing comma

(COM812)

1986-1986: Missing return type annotation for public function test_boundary_conditions_numeric

Add return type annotation: None

(ANN201)

1987-1987: Trailing comma missing

Add trailing comma

(COM812)

2021-2021: Trailing comma missing

Add trailing comma

(COM812)

🪛 Pylint (3.3.7)

src/utils/filter.py

[convention] 200-200: Line too long (106/100)

(C0301)

[convention] 216-216: Line too long (103/100)

(C0301)

[convention] 222-222: Line too long (106/100)

(C0301)

[convention] 270-270: Line too long (123/100)

(C0301)

[convention] 318-318: Line too long (134/100)

(C0301)

[convention] 444-444: Line too long (110/100)

(C0301)

[convention] 461-461: Line too long (102/100)

(C0301)

[convention] 484-484: Line too long (138/100)

(C0301)

[convention] 1-1: Missing module docstring

(C0114)

[error] 6-6: Unable to import 'sqlalchemy'

(E0401)

[error] 7-7: Unable to import 'sqlalchemy.sql.elements'

(E0401)

[error] 8-8: Unable to import 'sqlalchemy.types'

(E0401)

[warning] 52-52: Redefining built-in 'filter'

(W0622)

[refactor] 171-176: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 101-101: Too many branches (22/12)

(R0912)

[refactor] 218-226: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 223-226: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 228-231: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 246-271: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 264-271: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 300-321: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 311-319: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 387-392: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 462-481: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 416-498: Too many nested blocks (8/5)

(R1702)

[refactor] 416-498: Too many nested blocks (6/5)

(R1702)

[refactor] 501-506: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 395-395: Too many branches (32/12)

(R0912)

[refactor] 395-395: Too many statements (61/50)

(R0915)

tests/test_advanced_filters.py

[convention] 1320-1320: Line too long (105/100)

(C0301)

[convention] 1321-1321: Line too long (104/100)

(C0301)

[convention] 1340-1340: Line too long (118/100)

(C0301)

[convention] 1-1: Too many lines in module (2045/1000)

(C0302)

[error] 9-9: Unable to import 'fastapi.testclient'

(E0401)

[error] 10-10: Unable to import 'nanoid'

(E0401)

[convention] 1439-1439: Import outside toplevel (time)

(C0415)

🔇 Additional comments (13)

tests/routes/test_scoped_api.py (1)

171-171: LGTM! Filter key updated to match new filtering conventions.

The change from "name" to "id" correctly aligns with the new filtering system where external column names are mapped to internal model fields.

tests/routes/test_sessions.py (4)

149-149: LGTM! Filter structure updated for new filtering system.

The change to filter by "id" instead of "name" correctly adapts to the new filtering conventions where external IDs are used.

189-189: LGTM! Metadata filter properly nested.

The nesting of metadata filters under the "metadata" key correctly follows the new structured filtering approach for JSONB fields.

194-194: LGTM! More precise assertion for exact match.

Changing from >= 1 to == 1 provides a more precise test assertion, ensuring the filter returns exactly the expected number of sessions.

359-359: LGTM! is_active filter properly structured.

The nesting of the is_active filter inside the "filter" dictionary correctly adapts to the new filtering structure while preserving the test functionality.

src/utils/filter.py (7)

91-98: LGTM! Well-structured main entry point with proper null handling.

The function provides a clean interface with appropriate null checking and clear separation of concerns by delegating to helper functions.

114-176: LGTM! Comprehensive logical operator handling.

The recursive implementation correctly handles AND, OR, and NOT operators with proper validation and error handling. The type checking for list requirements and null handling are well implemented.

193-232: LGTM! Robust field condition building with proper validation.

The function correctly handles column mapping, existence validation, wildcard support, and differentiation between comparison operators and regular values. The special handling for JSONB fields is well implemented.

234-271: LGTM! Safe type casting implementation.

The _safe_numeric_cast function provides robust type conversion with proper fallback handling and clear error messages for invalid values.

274-323: LGTM! Comprehensive comparison condition building.

The function correctly handles all supported operators with proper validation, wildcard support, and appropriate casting for different data types.

326-372: LGTM! Well-implemented nested metadata filtering.

The function correctly handles complex nested filtering scenarios for JSONB fields with proper operator detection and condition combination.

509-549: LGTM! Robust datetime validation with comprehensive format support.

The function provides excellent security by validating datetime strings before use in SQL queries, supporting multiple common formats, and returning None for invalid inputs to allow proper error handling by callers.
tests/test_advanced_filters.py (1)

1292-1292: ```shell
#!/bin/bash
set -e

List top-level function definitions to locate the filtering entrypoint

rg -n "^def " src/utils/filter.py

Search for handling of the 'NOT' key in dynamic filter logic

rg -n "NOT" src/utils/filter.py

Show 5 lines of context around each 'NOT' mention to inspect its implementation

rg -C5 "NOT" src/utils/filter.py
</details>

</blockquote></details>

</details>

coderabbitai

Actionable comments posted: 3

♻️ Duplicate comments (3)

src/utils/filter.py (1)
50-52: Rename filter parameter to avoid shadowing built-in.

The parameter name filter shadows Python's built-in filter function. Based on previous discussions, consider renaming to filters.
 def apply_filter(
-    stmt: Select[tuple[T]], model_class: type[T], filter: dict[str, Any] | None = None
+    stmt: Select[tuple[T]], model_class: type[T], filters: dict[str, Any] | None = None
 ) -> Select[tuple[T]]:
And update references within the function:
-    if filter is None:
+    if filters is None:
         return stmt

-    conditions = _build_filter_conditions(filter, model_class)
+    conditions = _build_filter_conditions(filters, model_class)
tests/test_advanced_filters.py (2)
1-4: Consider splitting this large test module

With 2458 lines, this module significantly exceeds typical size guidelines. Consider organizing tests into multiple focused modules (e.g., test_logical_operators.py, test_comparison_operators.py, test_edge_cases.py) for better maintainability.

1473-1476: Replace blocking time.sleep with async asyncio.sleep

Using time.sleep in an async function blocks the event loop. Use the async equivalent instead.
-    # Wait a bit to ensure different timestamps
-    import time
-
-    time.sleep(0.1)
+    # Wait a bit to ensure different timestamps
+    import asyncio
+
+    await asyncio.sleep(0.1)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 85b004c and 35a7f9a.

📒 Files selected for processing (2)

src/utils/filter.py (1 hunks)
tests/test_advanced_filters.py (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

src/utils/filter.py (3)

Learnt from: VVoruganti
PR: plastic-labs/honcho#140
File: src/utils/filter.py:51-53
Timestamp: 2025-06-26T15:20:35.408Z
Learning: When renaming parameters to avoid shadowing built-ins, prefer `filters` over `filter_dict` for filter/filtering parameters as it's cleaner and more concise.

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.

Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T22:29:51.942Z
Learning: Use JSONB metadata fields in the database for extensibility and flexible data storage.

tests/test_advanced_filters.py (1)

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: tests/routes/test_messages.py:7-39
Timestamp: 2025-06-18T15:47:44.327Z
Learning: In FastAPI tests, it's common to use @pytest.mark.asyncio with a synchronous TestClient. The async marker is used when tests contain async database operations (like await db_session.commit()), but the HTTP client calls (client.post(), client.get()) should NOT be awaited since TestClient is synchronous. Only the actual async operations like database commits need awaiting.

🧬 Code Graph Analysis (1)

src/utils/filter.py (1)

src/exceptions.py (1)

FilterError (69-73)

🪛 GitHub Check: basedpyright

src/utils/filter.py

[warning] 278-278:
Argument type is partially unknown
Argument corresponds to parameter "else_" in function "case"
Argument type is "Cast[Unknown]" (reportUnknownArgumentType)

[warning] 268-268:
Argument type is partially unknown
Argument corresponds to parameter "else_" in function "case"
Argument type is "Cast[Unknown]" (reportUnknownArgumentType)

[warning] 256-256:
Argument type is partially unknown
Argument corresponds to parameter "else_" in function "case"
Argument type is "Cast[Unknown]" (reportUnknownArgumentType)

tests/test_advanced_filters.py

[failure] 163-163:
Expected type arguments for generic class "dict" (reportMissingTypeArgument)

[warning] 163-163:
Type of parameter "filter_config" is partially unknown
Parameter type is "dict[Unknown, Unknown]" (reportUnknownParameterType)

[warning] 90-90:
Type of "combined_filter" is partially unknown
Type of "combined_filter" is "dict[str, list[dict[Unknown, Unknown] | dict[str, dict[str, list[str]]]]]" (reportUnknownVariableType)

[failure] 53-53:
Expected type arguments for generic class "dict" (reportMissingTypeArgument)

[warning] 53-53:
Type of parameter "filter_config" is partially unknown
Parameter type is "dict[Unknown, Unknown]" (reportUnknownParameterType)

🪛 Ruff (0.11.9)

src/utils/filter.py

51-51: Trailing comma missing

Add trailing comma

(COM812)

100-100: _build_filter_conditions is too complex (22 > 10)

(C901)

101-101: Trailing comma missing

Add trailing comma

(COM812)

118-120: Avoid specifying long messages outside the exception class

(TRY003)

119-119: Trailing comma missing

Add trailing comma

(COM812)

131-133: Avoid specifying long messages outside the exception class

(TRY003)

132-132: Trailing comma missing

Add trailing comma

(COM812)

144-144: Avoid specifying long messages outside the exception class

(TRY003)

146-148: Avoid specifying long messages outside the exception class

(TRY003)

147-147: Trailing comma missing

Add trailing comma

(COM812)

154-154: Trailing comma missing

Add trailing comma

(COM812)

172-172: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

179-179: Dynamically typed expressions (typing.Any) are disallowed in value

(ANN401)

179-179: Trailing comma missing

Add trailing comma

(COM812)

198-200: Avoid specifying long messages outside the exception class

(TRY003)

199-199: Trailing comma missing

Add trailing comma

(COM812)

204-204: Avoid specifying long messages outside the exception class

(TRY003)

219-219: Unnecessary else after return statement

Remove unnecessary else

(RET505)

224-224: Unnecessary else after return statement

Remove unnecessary else

(RET505)

229-229: Unnecessary else after return statement

Remove unnecessary else

(RET505)

233-233: Dynamically typed expressions (typing.Any) are disallowed in column_accessor

(ANN401)

233-233: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

250-250: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

270-270: Consider moving this statement to an else block

(TRY300)

280-280: Consider moving this statement to an else block

(TRY300)

286-286: Unnecessary else after return statement

Remove unnecessary else

(RET505)

287-289: Avoid specifying long messages outside the exception class

(TRY003)

288-288: Trailing comma missing

Add trailing comma

(COM812)

291-293: Avoid specifying long messages outside the exception class

(TRY003)

292-292: Use explicit conversion flag

Replace with conversion flag

(RUF010)

292-292: Trailing comma missing

Add trailing comma

(COM812)

297-297: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

297-297: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

297-297: Trailing comma missing

Add trailing comma

(COM812)

313-313: Avoid specifying long messages outside the exception class

(TRY003)

334-336: Avoid specifying long messages outside the exception class

(TRY003)

335-335: Use explicit conversion flag

Replace with conversion flag

(RUF010)

335-335: Trailing comma missing

Add trailing comma

(COM812)

343-343: Unnecessary else after return statement

Remove unnecessary else

(RET505)

344-346: Avoid specifying long messages outside the exception class

(TRY003)

345-345: Trailing comma missing

Add trailing comma

(COM812)

354-354: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

354-354: Trailing comma missing

Add trailing comma

(COM812)

389-389: Trailing comma missing

Add trailing comma

(COM812)

416-416: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

422-422: _build_comparison_conditions is too complex (27 > 10)

(C901)

423-423: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

423-423: Trailing comma missing

Add trailing comma

(COM812)

440-440: Trailing comma missing

Add trailing comma

(COM812)

446-446: Avoid specifying long messages outside the exception class

(TRY003)

460-460: Avoid specifying long messages outside the exception class

(TRY003)

470-472: Avoid specifying long messages outside the exception class

(TRY003)

471-471: Trailing comma missing

Add trailing comma

(COM812)

491-491: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

499-501: Avoid specifying long messages outside the exception class

(TRY003)

500-500: Trailing comma missing

Add trailing comma

(COM812)

510-512: Avoid specifying long messages outside the exception class

(TRY003)

511-511: Trailing comma missing

Add trailing comma

(COM812)

530-530: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

565-565: Naive datetime constructed using datetime.datetime.strptime() without %z

(DTZ007)

tests/test_advanced_filters.py

16-16: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple

Use a tuple for the first argument

(PT006)

23-23: Trailing comma missing

Add trailing comma

(COM812)

33-33: Trailing comma missing

Add trailing comma

(COM812)

50-50: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

94-94: Trailing comma missing

Add trailing comma

(COM812)

125-125: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple

Use a tuple for the first argument

(PT006)

160-160: Missing return type annotation for public function test_comparison_operators_filters

Add return type annotation: None

(ANN201)

238-238: Missing return type annotation for public function test_wildcard_filters

Add return type annotation: None

(ANN201)

239-239: Trailing comma missing

Add trailing comma

(COM812)

265-265: Trailing comma missing

Add trailing comma

(COM812)

266-266: Trailing comma missing

Add trailing comma

(COM812)

280-280: Trailing comma missing

Add trailing comma

(COM812)

281-281: Trailing comma missing

Add trailing comma

(COM812)

291-291: Missing return type annotation for public function test_complex_nested_filters

Add return type annotation: None

(ANN201)

292-292: Trailing comma missing

Add trailing comma

(COM812)

345-345: Trailing comma missing

Add trailing comma

(COM812)

359-359: Trailing comma missing

Add trailing comma

(COM812)

363-363: Trailing comma missing

Add trailing comma

(COM812)

364-364: Trailing comma missing

Add trailing comma

(COM812)

380-380: Missing return type annotation for public function test_filters_across_different_models

Add return type annotation: None

(ANN201)

381-381: Trailing comma missing

Add trailing comma

(COM812)

403-403: Trailing comma missing

Add trailing comma

(COM812)

404-404: Trailing comma missing

Add trailing comma

(COM812)

440-440: Trailing comma missing

Add trailing comma

(COM812)

441-441: Trailing comma missing

Add trailing comma

(COM812)

452-452: Missing return type annotation for public function test_filter_edge_cases

Add return type annotation: None

(ANN201)

453-453: Trailing comma missing

Add trailing comma

(COM812)

463-463: Trailing comma missing

Add trailing comma

(COM812)

464-464: Trailing comma missing

Add trailing comma

(COM812)

491-491: Trailing comma missing

Add trailing comma

(COM812)

492-492: Trailing comma missing

Add trailing comma

(COM812)

503-503: Missing return type annotation for public function test_backward_compatibility

Add return type annotation: None

(ANN201)

504-504: Trailing comma missing

Add trailing comma

(COM812)

541-541: Missing return type annotation for public function test_range_queries_with_dates

Add return type annotation: None

(ANN201)

542-542: Trailing comma missing

Add trailing comma

(COM812)

582-582: Trailing comma missing

Add trailing comma

(COM812)

583-583: Trailing comma missing

Add trailing comma

(COM812)

601-601: Trailing comma missing

Add trailing comma

(COM812)

602-602: Trailing comma missing

Add trailing comma

(COM812)

614-614: Missing return type annotation for public function test_all_workspace_columns_filtering

Add return type annotation: None

(ANN201)

690-690: Missing return type annotation for public function test_all_peer_columns_filtering

Add return type annotation: None

(ANN201)

691-691: Trailing comma missing

Add trailing comma

(COM812)

769-769: Missing return type annotation for public function test_all_session_columns_filtering

Add return type annotation: None

(ANN201)

770-770: Trailing comma missing

Add trailing comma

(COM812)

860-860: Missing return type annotation for public function test_all_message_columns_filtering

Add return type annotation: None

(ANN201)

861-861: Trailing comma missing

Add trailing comma

(COM812)

894-894: Trailing comma missing

Add trailing comma

(COM812)

978-978: Missing return type annotation for public function test_id_field_interpolation_consistency

Add return type annotation: None

(ANN201)

1016-1016: Trailing comma missing

Add trailing comma

(COM812)

1017-1017: Trailing comma missing

Add trailing comma

(COM812)

1063-1063: Missing return type annotation for public function test_foreign_key_field_interpolation

Add return type annotation: None

(ANN201)

1064-1064: Trailing comma missing

Add trailing comma

(COM812)

1096-1096: Trailing comma missing

Add trailing comma

(COM812)

1097-1097: Trailing comma missing

Add trailing comma

(COM812)

1151-1151: Missing return type annotation for public function test_metadata_field_interpolation

Add return type annotation: None

(ANN201)

1195-1195: Trailing comma missing

Add trailing comma

(COM812)

1196-1196: Trailing comma missing

Add trailing comma

(COM812)

1274-1274: Missing return type annotation for public function test_nonexistent_columns_ignored_gracefully

Add return type annotation: None

(ANN201)

1275-1275: Trailing comma missing

Add trailing comma

(COM812)

1302-1302: Trailing comma missing

Add trailing comma

(COM812)

1303-1303: Trailing comma missing

Add trailing comma

(COM812)

1316-1316: Trailing comma missing

Add trailing comma

(COM812)

1317-1317: Trailing comma missing

Add trailing comma

(COM812)

1324-1324: Missing return type annotation for public function test_not_logic_correctness

Add return type annotation: None

(ANN201)

1325-1325: Trailing comma missing

Add trailing comma

(COM812)

1364-1364: Trailing comma missing

Add trailing comma

(COM812)

1365-1365: Trailing comma missing

Add trailing comma

(COM812)

1384-1384: Missing return type annotation for public function test_jsonb_type_casting_edge_cases

Add return type annotation: None

(ANN201)

1385-1385: Trailing comma missing

Add trailing comma

(COM812)

1421-1421: Trailing comma missing

Add trailing comma

(COM812)

1458-1458: Missing return type annotation for public function test_real_datetime_column_filtering

Add return type annotation: None

(ANN201)

1459-1459: Trailing comma missing

Add trailing comma

(COM812)

1476-1476: Async functions should not call time.sleep

(ASYNC251)

1508-1508: Missing return type annotation for public function test_invalid_datetime_handling

Add return type annotation: None

(ANN201)

1509-1509: Trailing comma missing

Add trailing comma

(COM812)

1531-1531: Missing return type annotation for public function test_nested_jsonb_filtering

Add return type annotation: None

(ANN201)

1532-1532: Trailing comma missing

Add trailing comma

(COM812)

1568-1568: Trailing comma missing

Add trailing comma

(COM812)

1584-1584: Missing return type annotation for public function test_multiple_operators_same_field

Add return type annotation: None

(ANN201)

1585-1585: Trailing comma missing

Add trailing comma

(COM812)

1621-1621: Trailing comma missing

Add trailing comma

(COM812)

1637-1637: Missing return type annotation for public function test_empty_and_null_filter_handling

Add return type annotation: None

(ANN201)

1638-1638: Trailing comma missing

Add trailing comma

(COM812)

1645-1645: Trailing comma missing

Add trailing comma

(COM812)

1654-1654: Trailing comma missing

Add trailing comma

(COM812)

1667-1667: Missing return type annotation for public function test_unicode_and_special_characters

Add return type annotation: None

(ANN201)

1668-1668: Trailing comma missing

Add trailing comma

(COM812)

1698-1698: Missing return type annotation for public function test_case_sensitivity_edge_cases

Add return type annotation: None

(ANN201)

1699-1699: Trailing comma missing

Add trailing comma

(COM812)

1746-1746: Missing return type annotation for public function test_malformed_filter_structures

Add return type annotation: None

(ANN201)

1747-1747: Trailing comma missing

Add trailing comma

(COM812)

1772-1772: Missing return type annotation for public function test_performance_with_complex_filters

Add return type annotation: None

(ANN201)

1773-1773: Trailing comma missing

Add trailing comma

(COM812)

1786-1786: Trailing comma missing

Add trailing comma

(COM812)

1794-1794: Trailing comma missing

Add trailing comma

(COM812)

1795-1795: Trailing comma missing

Add trailing comma

(COM812)

1796-1796: Trailing comma missing

Add trailing comma

(COM812)

1805-1805: Trailing comma missing

Add trailing comma

(COM812)

1807-1807: Trailing comma missing

Add trailing comma

(COM812)

1809-1809: Trailing comma missing

Add trailing comma

(COM812)

1822-1822: Missing return type annotation for public function test_filter_precedence_and_grouping

Add return type annotation: None

(ANN201)

1823-1823: Trailing comma missing

Add trailing comma

(COM812)

1862-1862: Trailing comma missing

Add trailing comma

(COM812)

1868-1868: Trailing comma missing

Add trailing comma

(COM812)

1870-1870: Trailing comma missing

Add trailing comma

(COM812)

1871-1871: Trailing comma missing

Add trailing comma

(COM812)

1888-1888: Missing return type annotation for public function test_jsonb_contains_vs_equality_semantics

Add return type annotation: None

(ANN201)

1889-1889: Trailing comma missing

Add trailing comma

(COM812)

1923-1923: Trailing comma missing

Add trailing comma

(COM812)

1941-1941: Missing return type annotation for public function test_wildcard_edge_cases_comprehensive

Add return type annotation: None

(ANN201)

1942-1942: Trailing comma missing

Add trailing comma

(COM812)

1957-1957: Trailing comma missing

Add trailing comma

(COM812)

1970-1970: Trailing comma missing

Add trailing comma

(COM812)

1971-1971: Trailing comma missing

Add trailing comma

(COM812)

1988-1988: Trailing comma missing

Add trailing comma

(COM812)

1989-1989: Trailing comma missing

Add trailing comma

(COM812)

2000-2000: Missing return type annotation for public function test_error_logging_and_debugging

Add return type annotation: None

(ANN201)

2001-2001: Trailing comma missing

Add trailing comma

(COM812)

2021-2021: Missing return type annotation for public function test_boundary_conditions_numeric

Add return type annotation: None

(ANN201)

2022-2022: Trailing comma missing

Add trailing comma

(COM812)

2056-2056: Trailing comma missing

Add trailing comma

(COM812)

2084-2084: Missing return type annotation for public function test_float_precision_edge_cases

Add return type annotation: None

(ANN201)

2096-2096: Trailing comma missing

Add trailing comma

(COM812)

2159-2159: Trailing comma missing

Add trailing comma

(COM812)

2185-2185: Trailing comma missing

Add trailing comma

(COM812)

2186-2186: Trailing comma missing

Add trailing comma

(COM812)

2249-2249: Missing return type annotation for public function test_mixed_type_comparisons

Add return type annotation: None

(ANN201)

2261-2261: Trailing comma missing

Add trailing comma

(COM812)

2330-2330: Trailing comma missing

Add trailing comma

(COM812)

🪛 Pylint (3.3.7)

src/utils/filter.py

[convention] 199-199: Line too long (106/100)

(C0301)

[convention] 215-215: Line too long (103/100)

(C0301)

[convention] 221-221: Line too long (106/100)

(C0301)

[convention] 284-284: Line too long (101/100)

(C0301)

[convention] 288-288: Line too long (127/100)

(C0301)

[convention] 335-335: Line too long (123/100)

(C0301)

[convention] 345-345: Line too long (134/100)

(C0301)

[convention] 471-471: Line too long (110/100)

(C0301)

[convention] 488-488: Line too long (102/100)

(C0301)

[convention] 511-511: Line too long (138/100)

(C0301)

[convention] 1-1: Missing module docstring

(C0114)

[error] 6-6: Unable to import 'sqlalchemy'

(E0401)

[error] 7-7: Unable to import 'sqlalchemy.types'

(E0401)

[warning] 51-51: Redefining built-in 'filter'

(W0622)

[refactor] 170-175: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 100-100: Too many branches (22/12)

(R0912)

[refactor] 217-225: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 222-225: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 227-230: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 246-289: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 282-289: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 338-346: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 414-419: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 489-508: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 443-525: Too many nested blocks (8/5)

(R1702)

[refactor] 443-525: Too many nested blocks (6/5)

(R1702)

[refactor] 528-533: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 422-422: Too many branches (32/12)

(R0912)

[refactor] 422-422: Too many statements (61/50)

(R0915)

tests/test_advanced_filters.py

[convention] 233-233: Line too long (102/100)

(C0301)

[convention] 1355-1355: Line too long (105/100)

(C0301)

[convention] 1356-1356: Line too long (104/100)

(C0301)

[convention] 1375-1375: Line too long (118/100)

(C0301)

[convention] 1-1: Too many lines in module (2458/1000)

(C0302)

[error] 9-9: Unable to import 'fastapi.testclient'

(E0401)

[error] 10-10: Unable to import 'nanoid'

(E0401)

[refactor] 50-50: Too many local variables (18/15)

(R0914)

[refactor] 160-160: Too many local variables (18/15)

(R0914)

[convention] 1474-1474: Import outside toplevel (time)

(C0415)

[refactor] 2249-2249: Too many statements (68/50)

(R0915)

🪛 GitHub Actions: Static Analysis

src/utils/filter.py

[warning] 256-256: Argument type is partially unknown. Argument corresponds to parameter "else_" in function "case". Argument type is "Cast[Unknown]" (reportUnknownArgumentType).

[warning] 268-268: Argument type is partially unknown. Argument corresponds to parameter "else_" in function "case". Argument type is "Cast[Unknown]" (reportUnknownArgumentType).

[warning] 278-278: Argument type is partially unknown. Argument corresponds to parameter "else_" in function "case". Argument type is "Cast[Unknown]" (reportUnknownArgumentType).

tests/test_advanced_filters.py

[warning] 53-53: Type of parameter "filter_config" is partially unknown. Parameter type is "dict[Unknown, Unknown]" (reportUnknownParameterType).

[error] 53-53: Expected type arguments for generic class "dict" (reportMissingTypeArgument).

🔇 Additional comments (8)

src/utils/filter.py (6)

1-48: Well-structured module setup with clear constants and mappings.

The module constants and column mappings are properly organized at the module level, following best practices. The separation between general models and message-specific mappings is a good design choice.

100-176: Robust implementation of logical operators with proper error handling.

The recursive handling of AND, OR, and NOT operators is well-implemented with appropriate validation and error messages. The complexity is justified by the requirements.

233-294: Excellent safe casting implementation for JSONB comparisons.

The use of CASE WHEN statements to handle empty strings and NULL values before casting is a robust approach that prevents runtime errors. The comprehensive type handling for booleans, numbers, and strings ensures reliable comparisons.

536-577: Secure datetime validation with comprehensive format support.

The validation function effectively prevents SQL injection by parsing datetime strings before use. Supporting multiple common formats improves usability.

514-516: Verify JSONB contains operator behavior matches requirements.

The JSONB contains operator has specific PostgreSQL semantics - it checks if the left JSON value contains all key/value pairs of the right JSON value at the top level. For nested objects, this might not match user expectations.

For example:

{"a": {"b": 1}} contains {"a": {}} returns true

{"a": {"b": 1}} contains {"b": 1} returns false

Please confirm this behavior aligns with your filtering requirements.

1-577: Comprehensive filtering implementation with robust error handling.

This filtering utility provides a well-designed abstraction for complex SQLAlchemy queries. The recursive handling of logical operators, safe type casting for comparisons, and proper validation make this a solid foundation for the application's filtering needs.

The code demonstrates good practices:

Clear separation of concerns with focused helper functions

Comprehensive error handling with informative messages

Security-conscious datetime validation

Flexible support for various filter formats

tests/test_advanced_filters.py (2)

2020-2246: Excellent coverage of numeric boundary conditions and floating point precision

These tests thoroughly cover important edge cases including:

Zero and negative values

Large integers (32-bit and 64-bit boundaries)

Floating point precision issues (0.1 + 0.2)

Scientific notation

Repeating decimals

This comprehensive testing helps ensure the filtering system handles numeric edge cases correctly.

1507-1528: Good security testing for SQL injection attempts

Excellent practice testing malicious datetime strings that attempt SQL injection. The tests correctly verify that these inputs are rejected with 422 status codes.

coderabbitai · 2025-06-26T15:33:44Z

+async def test_not_logic_correctness(
+    client: TestClient, sample_data: tuple[Workspace, Peer]
+):
+    """Test that NOT logic works correctly - THIS WILL LIKELY FAIL due to broken NOT logic"""
+    test_workspace, _test_peer = sample_data
+
+    # Create peers with specific metadata
+    peer1_name = str(generate_nanoid())
+    peer2_name = str(generate_nanoid())
+    peer3_name = str(generate_nanoid())
+
+    client.post(
+        f"/v2/workspaces/{test_workspace.name}/peers",
+        json={
+            "name": peer1_name,
+            "metadata": {"role": "admin", "department": "engineering"},
+        },
+    )
+    client.post(
+        f"/v2/workspaces/{test_workspace.name}/peers",
+        json={
+            "name": peer2_name,
+            "metadata": {"role": "user", "department": "engineering"},
+        },
+    )
+    client.post(
+        f"/v2/workspaces/{test_workspace.name}/peers",
+        json={"name": peer3_name, "metadata": {"role": "admin", "department": "sales"}},
+    )
+
+    # Test multiple NOT conditions - this exposes the bug
+    # User expectation: NOT admin AND NOT engineering = exclude admin users AND exclude engineering users
+    # Current broken code: NOT(admin AND engineering) = exclude users who are BOTH admin AND engineering
+    response = client.post(
+        f"/v2/workspaces/{test_workspace.name}/peers/list",
+        json={
+            "filter": {
+                "NOT": [
+                    {"metadata": {"role": "admin"}},
+                    {"metadata": {"department": "engineering"}},
+                ]
+            }
+        },
+    )
+    assert response.status_code == 200
+    data = response.json()
+    found_names = [item["id"] for item in data["items"]]
+
+    # This assertion will FAIL with current broken logic
+    # We expect only peers that are NOT admin AND NOT in engineering
+    # With broken logic, we get peers that are not (admin AND engineering) = peer2 and peer3
+    # With correct logic, we should get only peers that are (NOT admin) AND (NOT engineering) = none of our test peers
+    assert peer1_name not in found_names  # admin + engineering - should be excluded
+    assert (
+        peer2_name not in found_names
+    )  # user + engineering - should be excluded (engineering)
+    assert peer3_name not in found_names  # admin + sales - should be excluded (admin)
+


🧹 Nitpick (assertive)

Test correctly identifies NOT logic bug

This test properly documents the broken NOT logic where multiple conditions are incorrectly interpreted as NOT(A AND B) instead of (NOT A) AND (NOT B). The test will fail until the underlying logic is fixed.

Would you like me to create an issue to track this NOT logic bug?

🧰 Tools

🪛 Ruff (0.11.9)

1324-1324: Missing return type annotation for public function test_not_logic_correctness

Add return type annotation: None

(ANN201)

1325-1325: Trailing comma missing

Add trailing comma

(COM812)

1364-1364: Trailing comma missing

Add trailing comma

(COM812)

1365-1365: Trailing comma missing

Add trailing comma

(COM812)

🪛 Pylint (3.3.7)

[convention] 1355-1355: Line too long (105/100)

(C0301)

[convention] 1356-1356: Line too long (104/100)

(C0301)

[convention] 1375-1375: Line too long (118/100)

(C0301)

🤖 Prompt for AI Agents

In tests/test_advanced_filters.py around lines 1324 to 1381, the test correctly identifies that the current NOT logic implementation treats multiple NOT conditions as NOT(A AND B) instead of the correct (NOT A) AND (NOT B). To fix this, update the filtering logic in the application code to handle multiple NOT conditions by applying De Morgan's laws, ensuring each NOT condition is individually negated and combined with AND rather than grouping them inside a single NOT with AND. This test should then pass once the underlying NOT logic is corrected.

coderabbitai · 2025-06-26T15:33:44Z

+@pytest.mark.asyncio
+async def test_mixed_type_comparisons(client: TestClient):
+    """Test comparisons between different data types within JSONB fields"""
+    # Create workspace and peer through API to ensure they're properly committed
+    workspace_name = str(generate_nanoid())
+    peer_name = str(generate_nanoid())
+
+    # Create workspace
+    response = client.post("/v2/workspaces", json={"name": workspace_name})
+    assert response.status_code == 200
+
+    # Create peer
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers", json={"name": peer_name}
+    )
+    assert response.status_code == 200
+
+    # Create messages with mixed data types for the same logical field
+    messages_response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages",
+        json={
+            "messages": [
+                {
+                    "content": "String number five",
+                    "peer_id": peer_name,
+                    "metadata": {
+                        "priority": "5",  # String
+                        "score": "10.5",  # String float
+                        "active": "true",  # String boolean
+                        "count": "0",  # String zero
+                    },
+                },
+                {
+                    "content": "Integer five",
+                    "peer_id": peer_name,
+                    "metadata": {
+                        "priority": 5,  # Integer
+                        "score": 10.5,  # Float
+                        "active": True,  # Boolean
+                        "count": 0,  # Integer zero
+                    },
+                },
+                {
+                    "content": "Float five",
+                    "peer_id": peer_name,
+                    "metadata": {
+                        "priority": 5.0,  # Float that equals integer
+                        "score": 10,  # Integer that could be float
+                        "active": False,  # Boolean false
+                        "count": None,  # Null value
+                    },
+                },
+                {
+                    "content": "String vs numeric comparison",
+                    "peer_id": peer_name,
+                    "metadata": {
+                        "priority": "10",  # String > numeric 5?
+                        "score": "2",  # String < numeric 10?
+                        "active": "false",  # String false vs boolean
+                        "count": "null",  # String null vs null
+                    },
+                },
+                {
+                    "content": "Leading zeros and formats",
+                    "peer_id": peer_name,
+                    "metadata": {
+                        "priority": "05",  # Leading zero
+                        "score": "010.50",  # Leading zeros in float
+                        "active": "TRUE",  # Uppercase boolean
+                        "count": "00",  # Leading zeros
+                    },
+                },
+                {
+                    "content": "Edge case values",
+                    "peer_id": peer_name,
+                    "metadata": {
+                        "priority": "",  # Empty string
+                        "score": "NaN",  # Not a number string
+                        "active": 1,  # Numeric truthy
+                        "count": "infinity",  # Infinity string
+                    },
+                },
+            ]
+        },
+    )
+    assert messages_response.status_code == 200
+
+    # Test string vs numeric equality
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"priority": 5}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    # Should match integer 5 and float 5.0, behavior with string "5" depends on JSONB casting
+    assert "Integer five" in contents
+    assert "Float five" in contents
+
+    # Test string number comparison with numeric operator
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"priority": {"gte": 5}}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    # Behavior depends on how JSONB handles string-to-number conversion
+    # Should definitely include numeric values >= 5
+
+    # Test string boolean vs actual boolean
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"active": True}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "Integer five" in contents  # Has boolean True
+    # String "true" behavior depends on JSONB casting
+
+    # Test explicit string matching
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"priority": "5"}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "String number five" in contents
+
+    # Test numeric comparison with string numbers
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"score": {"gt": 10}}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    # Should include 10.5 (both string and float versions)
+
+    # Test zero comparisons (string "0" vs integer 0)
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"count": 0}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "Integer five" in contents  # Has integer 0
+
+    # Test null vs string "null"
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"count": None}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "Float five" in contents  # Has actual null
+
+    # Test leading zeros handling
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"priority": "05"}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "Leading zeros and formats" in contents
+
+    # Test case sensitivity for string booleans
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"active": "TRUE"}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "Leading zeros and formats" in contents
+
+    # Test empty string vs other falsy values
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"priority": ""}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "Edge case values" in contents
+
+    # Test special string values
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"score": "NaN"}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    assert "Edge case values" in contents
+
+    # Test mixed type in operator
+    response = client.post(
+        f"/v2/workspaces/{workspace_name}/peers/{peer_name}/messages/list",
+        json={"filter": {"metadata": {"priority": {"in": [5, "5", 5.0]}}}},
+    )
+    assert response.status_code == 200
+    data = response.json()
+    contents = [item["content"] for item in data["items"]]
+    # Should match various representations of 5
+    assert len(contents) >= 2


🧹 Nitpick (assertive)

Consider splitting this large test function

With 68 statements, this function exceeds complexity guidelines. Consider splitting it into focused test functions for better readability and maintainability.

Example refactor approach:

async def test_string_vs_numeric_equality(client: TestClient) -> None: """Test equality comparisons between string and numeric values.""" # Test cases for string "5" vs numeric 5 async def test_boolean_type_comparisons(client: TestClient) -> None: """Test comparisons with boolean values in different formats.""" # Test cases for true/false vs "true"/"false" vs 1/0 async def test_null_and_empty_value_handling(client: TestClient) -> None: """Test handling of null, empty string, and special values.""" # Test cases for null vs "null", empty strings, NaN, infinity

🧰 Tools

🪛 Ruff (0.11.9)

2249-2249: Missing return type annotation for public function test_mixed_type_comparisons

Add return type annotation: None

(ANN201)

2261-2261: Trailing comma missing

Add trailing comma

(COM812)

2330-2330: Trailing comma missing

Add trailing comma

(COM812)

🪛 Pylint (3.3.7)

[refactor] 2249-2249: Too many statements (68/50)

(R0915)

🤖 Prompt for AI Agents

In tests/test_advanced_filters.py between lines 2248 and 2458, the test_mixed_type_comparisons function is too large and complex with 68 statements. To fix this, split this single large test into multiple smaller, focused async test functions each targeting a specific aspect such as string vs numeric equality, boolean comparisons, and null/empty value handling. Move relevant test cases into these new functions with clear descriptive names and docstrings to improve readability and maintainability.

coderabbitai · 2025-06-26T15:33:45Z

+@pytest.mark.asyncio
+async def test_logical_operators_and_filters(
+    client: TestClient,
+    sample_data: tuple[Workspace, Peer],
+    filter_config: dict,
+    expected_peer_indices: list[int],
+    description: str,
+):


🧹 Nitpick (assertive)

Add return type annotation

Add return type annotation for better type safety and documentation.

@pytest.mark.asyncio -async def test_logical_operators_and_filters( +async def test_logical_operators_and_filters( client: TestClient, sample_data: tuple[Workspace, Peer], filter_config: dict, expected_peer_indices: list[int], description: str, -): +) -> None:

📝 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.

Suggested change

@pytest.mark.asyncio

async def test_logical_operators_and_filters(

client: TestClient,

sample_data: tuple[Workspace, Peer],

filter_config: dict,

expected_peer_indices: list[int],

description: str,

):

@pytest.mark.asyncio

async def test_logical_operators_and_filters(

client: TestClient,

sample_data: tuple[Workspace, Peer],

filter_config: dict,

expected_peer_indices: list[int],

description: str,

) -> None:

# existing test body follows...

🧰 Tools

🪛 GitHub Check: basedpyright

[failure] 53-53:
Expected type arguments for generic class "dict" (reportMissingTypeArgument)

[warning] 53-53:
Type of parameter "filter_config" is partially unknown
Parameter type is "dict[Unknown, Unknown]" (reportUnknownParameterType)

🪛 Ruff (0.11.9)

50-50: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

🪛 Pylint (3.3.7)

[refactor] 50-50: Too many local variables (18/15)

(R0914)

🪛 GitHub Actions: Static Analysis

[warning] 53-53: Type of parameter "filter_config" is partially unknown. Parameter type is "dict[Unknown, Unknown]" (reportUnknownParameterType).

[error] 53-53: Expected type arguments for generic class "dict" (reportMissingTypeArgument).

🤖 Prompt for AI Agents

In tests/test_advanced_filters.py around lines 49 to 56, the test_logical_operators_and_filters function lacks a return type annotation. Add an appropriate return type annotation, such as '-> None', to the function definition to improve type safety and documentation.

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (3)

src/utils/filter.py (1)
50-52: Rename filter parameter to filters to avoid shadowing built-in.

Based on previous feedback, the parameter should be renamed to filters instead of filter_dict as it's cleaner and more concise.
 def apply_filter(
-    stmt: Select[tuple[T]], model_class: type[T], filter: dict[str, Any] | None = None
+    stmt: Select[tuple[T]], model_class: type[T], filters: dict[str, Any] | None = None
 ) -> Select[tuple[T]]:
Update all references within the function:
-    if filter is None:
+    if filters is None:
         return stmt

-    conditions = _build_filter_conditions(filter, model_class)
+    conditions = _build_filter_conditions(filters, model_class)
tests/test_advanced_filters.py (2)
1475-1477: Replace blocking sleep with async sleep

Using time.sleep in an async function blocks the event loop.

50-57: Add return type annotation

Add return type annotation for consistency and type safety.

Apply this diff:
 @pytest.mark.asyncio
 async def test_logical_operators_and_filters(
     client: TestClient,
     sample_data: tuple[Workspace, Peer],
     filter_config: dict[str, Any],
     expected_peer_indices: list[int],
     description: str,
-):
+) -> None:

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 35a7f9a and 82c3c8b.

📒 Files selected for processing (2)

src/utils/filter.py (1 hunks)
tests/test_advanced_filters.py (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

src/utils/filter.py (3)

Learnt from: VVoruganti
PR: plastic-labs/honcho#140
File: src/utils/filter.py:51-53
Timestamp: 2025-06-26T15:20:35.408Z
Learning: When renaming parameters to avoid shadowing built-ins, prefer `filters` over `filter_dict` for filter/filtering parameters as it's cleaner and more concise.

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: migrations/versions/d429de0e5338_adopt_peer_paradigm.py:188-194
Timestamp: 2025-06-18T15:58:51.202Z
Learning: In the migration file `migrations/versions/d429de0e5338_adopt_peer_paradigm.py`, the team has explicitly decided to accept SQL injection risks from f-string interpolation of schema names, despite the security concerns around DDL injection via the DATABASE_SCHEMA environment variable.

Learnt from: CR
PR: plastic-labs/honcho#0
File: CLAUDE.md:0-0
Timestamp: 2025-06-24T22:29:51.942Z
Learning: Use JSONB metadata fields in the database for extensibility and flexible data storage.

tests/test_advanced_filters.py (1)

Learnt from: Rajat-Ahuja1997
PR: plastic-labs/honcho#131
File: tests/routes/test_messages.py:7-39
Timestamp: 2025-06-18T15:47:44.327Z
Learning: In FastAPI tests, it's common to use @pytest.mark.asyncio with a synchronous TestClient. The async marker is used when tests contain async database operations (like await db_session.commit()), but the HTTP client calls (client.post(), client.get()) should NOT be awaited since TestClient is synchronous. Only the actual async operations like database commits need awaiting.

🧬 Code Graph Analysis (1)

src/utils/filter.py (1)

src/exceptions.py (1)

FilterError (69-73)

🪛 Ruff (0.11.9)

src/utils/filter.py

51-51: Trailing comma missing

Add trailing comma

(COM812)

100-100: _build_filter_conditions is too complex (22 > 10)

(C901)

101-101: Trailing comma missing

Add trailing comma

(COM812)

118-120: Avoid specifying long messages outside the exception class

(TRY003)

119-119: Trailing comma missing

Add trailing comma

(COM812)

131-133: Avoid specifying long messages outside the exception class

(TRY003)

132-132: Trailing comma missing

Add trailing comma

(COM812)

144-144: Avoid specifying long messages outside the exception class

(TRY003)

146-148: Avoid specifying long messages outside the exception class

(TRY003)

147-147: Trailing comma missing

Add trailing comma

(COM812)

154-154: Trailing comma missing

Add trailing comma

(COM812)

172-172: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

179-179: Dynamically typed expressions (typing.Any) are disallowed in value

(ANN401)

179-179: Trailing comma missing

Add trailing comma

(COM812)

198-200: Avoid specifying long messages outside the exception class

(TRY003)

199-199: Trailing comma missing

Add trailing comma

(COM812)

204-204: Avoid specifying long messages outside the exception class

(TRY003)

219-219: Unnecessary else after return statement

Remove unnecessary else

(RET505)

224-224: Unnecessary else after return statement

Remove unnecessary else

(RET505)

229-229: Unnecessary else after return statement

Remove unnecessary else

(RET505)

234-234: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

234-234: Trailing comma missing

Add trailing comma

(COM812)

263-263: Unnecessary else after return statement

Remove unnecessary else

(RET505)

268-268: Consider moving this statement to an else block

(TRY300)

272-272: Consider moving this statement to an else block

(TRY300)

278-278: Unnecessary else after return statement

Remove unnecessary else

(RET505)

279-281: Avoid specifying long messages outside the exception class

(TRY003)

280-280: Trailing comma missing

Add trailing comma

(COM812)

283-285: Avoid specifying long messages outside the exception class

(TRY003)

284-284: Use explicit conversion flag

Replace with conversion flag

(RUF010)

284-284: Trailing comma missing

Add trailing comma

(COM812)

289-289: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

289-289: Dynamically typed expressions (typing.Any) are disallowed in op_value

(ANN401)

289-289: Trailing comma missing

Add trailing comma

(COM812)

305-305: Avoid specifying long messages outside the exception class

(TRY003)

326-328: Avoid specifying long messages outside the exception class

(TRY003)

327-327: Use explicit conversion flag

Replace with conversion flag

(RUF010)

327-327: Trailing comma missing

Add trailing comma

(COM812)

335-335: Unnecessary else after return statement

Remove unnecessary else

(RET505)

336-338: Avoid specifying long messages outside the exception class

(TRY003)

337-337: Trailing comma missing

Add trailing comma

(COM812)

346-346: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

346-346: Trailing comma missing

Add trailing comma

(COM812)

381-381: Trailing comma missing

Add trailing comma

(COM812)

408-408: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

414-414: _build_comparison_conditions is too complex (27 > 10)

(C901)

415-415: Dynamically typed expressions (typing.Any) are disallowed in column

(ANN401)

415-415: Trailing comma missing

Add trailing comma

(COM812)

432-432: Trailing comma missing

Add trailing comma

(COM812)

438-438: Avoid specifying long messages outside the exception class

(TRY003)

452-452: Avoid specifying long messages outside the exception class

(TRY003)

462-464: Avoid specifying long messages outside the exception class

(TRY003)

463-463: Trailing comma missing

Add trailing comma

(COM812)

483-483: Unnecessary else after continue statement

Remove unnecessary else

(RET507)

491-493: Avoid specifying long messages outside the exception class

(TRY003)

492-492: Trailing comma missing

Add trailing comma

(COM812)

502-504: Avoid specifying long messages outside the exception class

(TRY003)

503-503: Trailing comma missing

Add trailing comma

(COM812)

522-522: Unnecessary elif after return statement

Remove unnecessary elif

(RET505)

557-557: Naive datetime constructed using datetime.datetime.strptime() without %z

(DTZ007)

tests/test_advanced_filters.py

17-17: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple

Use a tuple for the first argument

(PT006)

24-24: Trailing comma missing

Add trailing comma

(COM812)

34-34: Trailing comma missing

Add trailing comma

(COM812)

51-51: Missing return type annotation for public function test_logical_operators_and_filters

Add return type annotation: None

(ANN201)

95-95: Trailing comma missing

Add trailing comma

(COM812)

126-126: Wrong type passed to first argument of pytest.mark.parametrize; expected tuple

Use a tuple for the first argument

(PT006)

161-161: Missing return type annotation for public function test_comparison_operators_filters

Add return type annotation: None

(ANN201)

239-239: Missing return type annotation for public function test_wildcard_filters

Add return type annotation: None

(ANN201)

240-240: Trailing comma missing

Add trailing comma

(COM812)

266-266: Trailing comma missing

Add trailing comma

(COM812)

267-267: Trailing comma missing

Add trailing comma

(COM812)

281-281: Trailing comma missing

Add trailing comma

(COM812)

282-282: Trailing comma missing

Add trailing comma

(COM812)

292-292: Missing return type annotation for public function test_complex_nested_filters

Add return type annotation: None

(ANN201)

293-293: Trailing comma missing

Add trailing comma

(COM812)

346-346: Trailing comma missing

Add trailing comma

(COM812)

360-360: Trailing comma missing

Add trailing comma

(COM812)

364-364: Trailing comma missing

Add trailing comma

(COM812)

365-365: Trailing comma missing

Add trailing comma

(COM812)

381-381: Missing return type annotation for public function test_filters_across_different_models

Add return type annotation: None

(ANN201)

382-382: Trailing comma missing

Add trailing comma

(COM812)

404-404: Trailing comma missing

Add trailing comma

(COM812)

405-405: Trailing comma missing

Add trailing comma

(COM812)

441-441: Trailing comma missing

Add trailing comma

(COM812)

442-442: Trailing comma missing

Add trailing comma

(COM812)

453-453: Missing return type annotation for public function test_filter_edge_cases

Add return type annotation: None

(ANN201)

454-454: Trailing comma missing

Add trailing comma

(COM812)

464-464: Trailing comma missing

Add trailing comma

(COM812)

465-465: Trailing comma missing

Add trailing comma

(COM812)

492-492: Trailing comma missing

Add trailing comma

(COM812)

493-493: Trailing comma missing

Add trailing comma

(COM812)

504-504: Missing return type annotation for public function test_backward_compatibility

Add return type annotation: None

(ANN201)

505-505: Trailing comma missing

Add trailing comma

(COM812)

542-542: Missing return type annotation for public function test_range_queries_with_dates

Add return type annotation: None

(ANN201)

543-543: Trailing comma missing

Add trailing comma

(COM812)

583-583: Trailing comma missing

Add trailing comma

(COM812)

584-584: Trailing comma missing

Add trailing comma

(COM812)

602-602: Trailing comma missing

Add trailing comma

(COM812)

603-603: Trailing comma missing

Add trailing comma

(COM812)

615-615: Missing return type annotation for public function test_all_workspace_columns_filtering

Add return type annotation: None

(ANN201)

691-691: Missing return type annotation for public function test_all_peer_columns_filtering

Add return type annotation: None

(ANN201)

692-692: Trailing comma missing

Add trailing comma

(COM812)

770-770: Missing return type annotation for public function test_all_session_columns_filtering

Add return type annotation: None

(ANN201)

771-771: Trailing comma missing

Add trailing comma

(COM812)

861-861: Missing return type annotation for public function test_all_message_columns_filtering

Add return type annotation: None

(ANN201)

862-862: Trailing comma missing

Add trailing comma

(COM812)

895-895: Trailing comma missing

Add trailing comma

(COM812)

979-979: Missing return type annotation for public function test_id_field_interpolation_consistency

Add return type annotation: None

(ANN201)

1017-1017: Trailing comma missing

Add trailing comma

(COM812)

1018-1018: Trailing comma missing

Add trailing comma

(COM812)

1064-1064: Missing return type annotation for public function test_foreign_key_field_interpolation

Add return type annotation: None

(ANN201)

1065-1065: Trailing comma missing

Add trailing comma

(COM812)

1097-1097: Trailing comma missing

Add trailing comma

(COM812)

1098-1098: Trailing comma missing

Add trailing comma

(COM812)

1152-1152: Missing return type annotation for public function test_metadata_field_interpolation

Add return type annotation: None

(ANN201)

1196-1196: Trailing comma missing

Add trailing comma

(COM812)

1197-1197: Trailing comma missing

Add trailing comma

(COM812)

1275-1275: Missing return type annotation for public function test_nonexistent_columns_ignored_gracefully

Add return type annotation: None

(ANN201)

1276-1276: Trailing comma missing

Add trailing comma

(COM812)

1303-1303: Trailing comma missing

Add trailing comma

(COM812)

1304-1304: Trailing comma missing

Add trailing comma

(COM812)

1317-1317: Trailing comma missing

Add trailing comma

(COM812)

1318-1318: Trailing comma missing

Add trailing comma

(COM812)

1325-1325: Missing return type annotation for public function test_not_logic_correctness

Add return type annotation: None

(ANN201)

1326-1326: Trailing comma missing

Add trailing comma

(COM812)

1365-1365: Trailing comma missing

Add trailing comma

(COM812)

1366-1366: Trailing comma missing

Add trailing comma

(COM812)

1385-1385: Missing return type annotation for public function test_jsonb_type_casting_edge_cases

Add return type annotation: None

(ANN201)

1386-1386: Trailing comma missing

Add trailing comma

(COM812)

1422-1422: Trailing comma missing

Add trailing comma

(COM812)

1459-1459: Missing return type annotation for public function test_real_datetime_column_filtering

Add return type annotation: None

(ANN201)

1460-1460: Trailing comma missing

Add trailing comma

(COM812)

1477-1477: Async functions should not call time.sleep

(ASYNC251)

1509-1509: Missing return type annotation for public function test_invalid_datetime_handling

Add return type annotation: None

(ANN201)

1510-1510: Trailing comma missing

Add trailing comma

(COM812)

1532-1532: Missing return type annotation for public function test_nested_jsonb_filtering

Add return type annotation: None

(ANN201)

1533-1533: Trailing comma missing

Add trailing comma

(COM812)

1569-1569: Trailing comma missing

Add trailing comma

(COM812)

1585-1585: Missing return type annotation for public function test_multiple_operators_same_field

Add return type annotation: None

(ANN201)

1586-1586: Trailing comma missing

Add trailing comma

(COM812)

1622-1622: Trailing comma missing

Add trailing comma

(COM812)

1638-1638: Missing return type annotation for public function test_empty_and_null_filter_handling

Add return type annotation: None

(ANN201)

1639-1639: Trailing comma missing

Add trailing comma

(COM812)

1646-1646: Trailing comma missing

Add trailing comma

(COM812)

1655-1655: Trailing comma missing

Add trailing comma

(COM812)

1668-1668: Missing return type annotation for public function test_unicode_and_special_characters

Add return type annotation: None

(ANN201)

1669-1669: Trailing comma missing

Add trailing comma

(COM812)

1699-1699: Missing return type annotation for public function test_case_sensitivity_edge_cases

Add return type annotation: None

(ANN201)

1700-1700: Trailing comma missing

Add trailing comma

(COM812)

1747-1747: Missing return type annotation for public function test_malformed_filter_structures

Add return type annotation: None

(ANN201)

1748-1748: Trailing comma missing

Add trailing comma

(COM812)

1773-1773: Missing return type annotation for public function test_performance_with_complex_filters

Add return type annotation: None

(ANN201)

1774-1774: Trailing comma missing

Add trailing comma

(COM812)

1787-1787: Trailing comma missing

Add trailing comma

(COM812)

1795-1795: Trailing comma missing

Add trailing comma

(COM812)

1796-1796: Trailing comma missing

Add trailing comma

(COM812)

1797-1797: Trailing comma missing

Add trailing comma

(COM812)

1806-1806: Trailing comma missing

Add trailing comma

(COM812)

1808-1808: Trailing comma missing

Add trailing comma

(COM812)

1810-1810: Trailing comma missing

Add trailing comma

(COM812)

1823-1823: Missing return type annotation for public function test_filter_precedence_and_grouping

Add return type annotation: None

(ANN201)

1824-1824: Trailing comma missing

Add trailing comma

(COM812)

1863-1863: Trailing comma missing

Add trailing comma

(COM812)

1869-1869: Trailing comma missing

Add trailing comma

(COM812)

1871-1871: Trailing comma missing

Add trailing comma

(COM812)

1872-1872: Trailing comma missing

Add trailing comma

(COM812)

1889-1889: Missing return type annotation for public function test_jsonb_contains_vs_equality_semantics

Add return type annotation: None

(ANN201)

1890-1890: Trailing comma missing

Add trailing comma

(COM812)

1924-1924: Trailing comma missing

Add trailing comma

(COM812)

1942-1942: Missing return type annotation for public function test_wildcard_edge_cases_comprehensive

Add return type annotation: None

(ANN201)

1943-1943: Trailing comma missing

Add trailing comma

(COM812)

1958-1958: Trailing comma missing

Add trailing comma

(COM812)

1971-1971: Trailing comma missing

Add trailing comma

(COM812)

1972-1972: Trailing comma missing

Add trailing comma

(COM812)

1989-1989: Trailing comma missing

Add trailing comma

(COM812)

1990-1990: Trailing comma missing

Add trailing comma

(COM812)

2001-2001: Missing return type annotation for public function test_error_logging_and_debugging

Add return type annotation: None

(ANN201)

2002-2002: Trailing comma missing

Add trailing comma

(COM812)

2022-2022: Missing return type annotation for public function test_boundary_conditions_numeric

Add return type annotation: None

(ANN201)

2023-2023: Trailing comma missing

Add trailing comma

(COM812)

2057-2057: Trailing comma missing

Add trailing comma

(COM812)

2085-2085: Missing return type annotation for public function test_float_precision_edge_cases

Add return type annotation: None

(ANN201)

2097-2097: Trailing comma missing

Add trailing comma

(COM812)

2160-2160: Trailing comma missing

Add trailing comma

(COM812)

2186-2186: Trailing comma missing

Add trailing comma

(COM812)

2187-2187: Trailing comma missing

Add trailing comma

(COM812)

2250-2250: Missing return type annotation for public function test_mixed_type_comparisons

Add return type annotation: None

(ANN201)

2262-2262: Trailing comma missing

Add trailing comma

(COM812)

2331-2331: Trailing comma missing

Add trailing comma

(COM812)

🪛 Pylint (3.3.7)

src/utils/filter.py

[convention] 199-199: Line too long (106/100)

(C0301)

[convention] 215-215: Line too long (103/100)

(C0301)

[convention] 221-221: Line too long (106/100)

(C0301)

[convention] 276-276: Line too long (101/100)

(C0301)

[convention] 280-280: Line too long (127/100)

(C0301)

[convention] 327-327: Line too long (123/100)

(C0301)

[convention] 337-337: Line too long (134/100)

(C0301)

[convention] 463-463: Line too long (110/100)

(C0301)

[convention] 480-480: Line too long (102/100)

(C0301)

[convention] 503-503: Line too long (138/100)

(C0301)

[convention] 1-1: Missing module docstring

(C0114)

[error] 6-6: Unable to import 'sqlalchemy'

(E0401)

[error] 7-7: Unable to import 'sqlalchemy.types'

(E0401)

[warning] 51-51: Redefining built-in 'filter'

(W0622)

[refactor] 170-175: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 100-100: Too many branches (22/12)

(R0912)

[refactor] 217-225: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 222-225: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 227-230: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 261-281: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 274-281: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 330-338: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it

(R1705)

[refactor] 406-411: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 481-500: Unnecessary "else" after "continue", remove the "else" and de-indent the code inside it

(R1724)

[refactor] 435-517: Too many nested blocks (8/5)

(R1702)

[refactor] 435-517: Too many nested blocks (6/5)

(R1702)

[refactor] 520-525: Unnecessary "elif" after "return", remove the leading "el" from "elif"

(R1705)

[refactor] 414-414: Too many branches (32/12)

(R0912)

[refactor] 414-414: Too many statements (61/50)

(R0915)

tests/test_advanced_filters.py

[convention] 234-234: Line too long (102/100)

(C0301)

[convention] 1356-1356: Line too long (105/100)

(C0301)

[convention] 1357-1357: Line too long (104/100)

(C0301)

[convention] 1376-1376: Line too long (118/100)

(C0301)

[convention] 1-1: Too many lines in module (2459/1000)

(C0302)

[error] 10-10: Unable to import 'fastapi.testclient'

(E0401)

[error] 11-11: Unable to import 'nanoid'

(E0401)

[refactor] 51-51: Too many local variables (18/15)

(R0914)

[refactor] 161-161: Too many local variables (18/15)

(R0914)

[convention] 1475-1475: Import outside toplevel (time)

(C0415)

[refactor] 2250-2250: Too many statements (68/50)

(R0915)

🔇 Additional comments (9)

src/utils/filter.py (7)

100-176: Function complexity is acceptable for logical operator handling.

While static analysis flags this function as complex (22 branches), the logic for handling AND, OR, and NOT operators with proper error checking is necessary and correctly implemented.

178-231: Well-structured field condition logic.

The column mapping, wildcard handling, and distinction between comparison operators and regular values is correctly implemented. The error handling for invalid columns is appropriate.

233-286: Excellent safe casting implementation.

The use of SQLAlchemy's CASE WHEN for safe numeric casting and proper handling of different data types (boolean, numeric, string) effectively prevents SQL injection while maintaining functionality.

288-342: Solid comparison condition logic.

The operator validation, wildcard handling, and integration with safe numeric casting provides a robust foundation for JSONB field comparisons. The error handling appropriately catches invalid operators and values.

345-392: Effective nested metadata filtering implementation.

The logic for handling nested JSONB fields with comparison operators is well-structured. The consistent wildcard handling and proper use of JSONB contains for nested object matching provides flexible filtering capabilities.

414-526: Complex but comprehensive comparison handling.

This function handles a wide range of comparison scenarios correctly, including datetime validation, numeric casting, and various operators. While the complexity is high (32 branches), the functionality appears complete and secure.

528-568: Robust datetime validation for security.

The comprehensive datetime format validation effectively prevents SQL injection by ensuring only valid datetime strings are processed. The multiple format attempts and fromisoformat fallback provide good compatibility.
tests/test_advanced_filters.py (2)

2085-2247: Excellent floating point precision testing!

This test comprehensively covers floating point precision edge cases including:

The classic 0.1 + 0.2 problem

Epsilon-based comparisons for proper float equality

Scientific notation handling

Large number precision

Repeating decimals

Great attention to detail in testing these tricky edge cases.

820-829: Verify is_active field still exists

This test filters by is_active field, but the AI summary indicates this attribute was removed from SessionGet. Please verify if this field is still valid for filtering.
#!/bin/bash
# Description: Check if is_active field exists in Session model and filtering logic

# Search for is_active in Session model definition
echo "Checking Session model for is_active field:"
ast-grep --pattern 'class Session {
  $$$
  is_active $$$
  $$$
}'

# Search for is_active references in filter logic
echo -e "\nChecking filter logic for is_active handling:"
rg -A 3 "is_active" src/utils/filter.py src/crud.py

# Check if is_active is in allowed columns for sessions
echo -e "\nChecking allowed columns for session filtering:"
rg -A 10 "session.*columns|ALLOWED.*SESSION" src/

coderabbitai · 2025-06-26T15:44:58Z

+"""
+Tests for advanced filter functionality including logical operators,
+comparison operators, and wildcards across multiple models.
+"""


🛠️ Refactor suggestion

Consider splitting this large test module

With 2459 lines, this file significantly exceeds maintainability guidelines. Consider splitting it into focused modules like:

test_logical_operators.py

test_comparison_operators.py

test_edge_cases.py

test_type_handling.py

🧰 Tools

🪛 Pylint (3.3.7)

[convention] 1-1: Too many lines in module (2459/1000)

(C0302)

🤖 Prompt for AI Agents

In tests/test_advanced_filters.py at the beginning of the file, the test module is too large with 2459 lines, which harms maintainability. Refactor by splitting this file into smaller, focused test modules such as test_logical_operators.py, test_comparison_operators.py, test_edge_cases.py, and test_type_handling.py. Move related tests into these new files to improve organization and readability.

feat: add complex arbitrary filtering on all objects

d1310e2

coderabbitai Bot reviewed Jun 24, 2025

View reviewed changes

Comment thread src/crud.py Outdated

fix: safe numeric casting and application of comparators

dc2730d

coderabbitai Bot reviewed Jun 24, 2025

View reviewed changes

VVoruganti requested changes Jun 24, 2025

View reviewed changes

fix: add way more tests, fix bugs with filter parsing

9680c59

coderabbitai Bot reviewed Jun 24, 2025

View reviewed changes

chore: pass model_class as arugment to apply_filter

2a3f3d9

coderabbitai Bot reviewed Jun 24, 2025

View reviewed changes

Comment thread src/utils/filter.py

Comment thread src/utils/filter.py Outdated

Comment thread src/utils/filter.py Outdated

Comment thread src/utils/filter.py Outdated

dr-frmr and others added 3 commits June 24, 2025 16:23

fix: default to not caring about is_active in get_sessions_for_peer

ee7c4c9

fix: address coderabbit complaints (valid)

1cd1c08

Merge branch 'main' into ben/dev-922

52b59aa

coderabbitai Bot reviewed Jun 24, 2025

View reviewed changes

Comment thread src/utils/filter.py

VVoruganti requested changes Jun 25, 2025

View reviewed changes

dr-frmr added 2 commits June 25, 2025 11:52

Merge branch 'main' into ben/dev-922

ab8f33b

fix: throw filter errors when necessary, validate inputs and handle e…

29ad171

…dge cases with more tests

coderabbitai Bot reviewed Jun 25, 2025

View reviewed changes

fix: remove all type errors and most type warnings

52fbc8f

coderabbitai Bot reviewed Jun 25, 2025

View reviewed changes

fix: don't use db in tests that don't need it

d65b32c

cheat: sprinkle in some pyright: ignore in filter.py

coderabbitai Bot reviewed Jun 25, 2025

View reviewed changes

Comment thread src/utils/filter.py

fix: allowlist for filtering -- no filtering by content, message id, …

85b004c

…or anything internal

coderabbitai Bot reviewed Jun 25, 2025

View reviewed changes

Comment thread tests/test_advanced_filters.py

fix: handle mixed types in metadata, add tests

35a7f9a

coderabbitai Bot reviewed Jun 26, 2025

View reviewed changes

chore: refine types

82c3c8b

coderabbitai Bot reviewed Jun 26, 2025

View reviewed changes

dr-frmr added 2 commits June 26, 2025 12:18

Merge branch 'main' into ben/dev-922

4fe0cbb

chore: rename fiter param everywhere

d3cb9fe

VVoruganti approved these changes Jun 26, 2025

View reviewed changes

VVoruganti merged commit 41bf5ad into main Jun 26, 2025
0 checks passed

VVoruganti deleted the ben/dev-922 branch June 26, 2025 16:25

This was referenced Jul 21, 2025

feat: add filter params everywhere #164

Merged

Add Pre-commit Hooks #165

Merged

This was referenced Aug 6, 2025

Ben/search rrf #179

Merged

feat: add support for custom message timestamps in API #183

Closed

coderabbitai Bot mentioned this pull request Oct 28, 2025

feat: add observation endpoints, make session deletion endpoint real #248

Merged



		def apply_filter(
		stmt: Select, model_class, filter: dict[str, Any] \| None = None

-    stmt: Select, model_class, filter: dict[str, Any] | None = None
+# at the top of src/utils/filter.py, add the new imports and TypeVar
+from typing import Type, TypeVar
+from sqlalchemy.orm import DeclarativeBase
+T = TypeVar('T', bound=DeclarativeBase)
+# Update the apply_filter signature
+def apply_filter(
+    stmt: Select[tuple[T]], model_class: Type[T], filter: dict[str, Any] | None = None
+) -> Select[tuple[T]]:
+    # … existing implementation …
+    ...

-    import time
-    time.sleep(0.1)
+    # Wait a bit to ensure different timestamps
+    import asyncio
+    await asyncio.sleep(0.1)

Conversation

dr-frmr commented Jun 24, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Suggested reviewers

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

VVoruganti left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

VVoruganti Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

VVoruganti Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

VVoruganti Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

VVoruganti Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

VVoruganti Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

VVoruganti Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

VVoruganti Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jun 25, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

dr-frmr commented Jun 24, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 24, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)