fix(agents-api): fix sort by updated_at query in list docs

[Ahmad-mtos]

User description

… functionality

---

PR Type

Enhancement, Tests

---

Description

• Add updated_at field to docs list query and sorting functionality

• Implement SORT_COLUMN_MAP to safely map sort keys to fully-qualified columns

• Use parameterized sort direction to prevent SQL injection vulnerabilities

• Add regression test for sorting by updated_at with descending order

---

Diagram Walkthrough

flowchart LR
  A["list_docs query"] -->|"add updated_at column"| B["base_docs_query"]
  C["sort_by parameter"] -->|"map via SORT_COLUMN_MAP"| D["order_column"]
  D -->|"combined with direction_sql"| E["ORDER BY clause"]
  F["Test: create 2 docs"] -->|"sort by updated_at desc"| G["Verify correct order"]

Loading

File Walkthrough

	Relevant files
Enhancement	list_docs.py Add updated_at field and safe sort column mapping                src/agents-api/agents_api/queries/docs/list_docs.py Added updated_at field to the base docs SELECT query Introduced SORT_COLUMN_MAP dictionary to map exposed sort keys to fully-qualified column names Updated query construction to use mapped column names and parameterized direction SQL Prevents SQL injection and join ambiguity by using explicit column references +13/-3
Tests	test_docs_queries.py Add regression test for updated_at sorting                              src/agents-api/tests/test_docs_queries.py Added asyncio import for sleep functionality Created new regression test for sorting by updated_at field Test verifies that documents are correctly ordered by their latest timestamps Uses metadata filter to isolate test documents and validates descending sort order +51/-0

---

---

Important

Enhance document listing by adding updated_at sorting and a regression test for safety and functionality.

• Enhancement:
  • Add updated_at field to list_docs.py for document sorting.
  • Implement SORT_COLUMN_MAP to map sort keys to column names safely.
  • Use parameterized sort direction to prevent SQL injection.
• Tests:
  • Add regression test in test_docs_queries.py for sorting by updated_at in descending order.

^{This description was created by for 5514730. You can customize this summary. It will automatically update as commits are pushed.}

[qodo-merge-for-open-source]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	SQL injection Description: The ORDER BY clause is built with f-strings using mapped column names and normalized direction, which is safe if and only if sort_by is strictly validated against SORT_COLUMN_MAP keys and direction against known values; otherwise, it could enable SQL injection—verify upstream validation enforces these constraints. list_docs.py [142-146] Referred Code order_column = SORT_COLUMN_MAP[sort_by] direction_sql = "ASC" if direction == "asc" else "DESC" query += ( f" ORDER BY {order_column} {direction_sql} LIMIT ${len(params) + 1} OFFSET ${len(params) + 2}" )
Ticket Compliance
⚪	🎫 No ticket provided - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr>
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
⚪	No custom compliance provided Follow the guide to enable custom compliance check.

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[github-actions]

Claude finished @Ahmad-mtos's task —— View job

---

Analyzing PR Changes for Changelog

Todo List:

• Read diff.patch to understand the changes
• Determine if changes are user-visible
• Create changelog entry for user-visible changes
• Stage, commit and push changes

Analysis:
The changes include:

1. User-visible: Added updated_at field to list docs API response
2. User-visible: Fixed sorting by updated_at field in list docs functionality
3. Internal: Added SORT_COLUMN_MAP for secure column mapping
4. Internal: Added regression test for updated_at sorting

This introduces user-visible changes that should be documented in the changelog.

[qodo-merge-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Make test more robust by explicitly updating a record To improve test robustness, replace the asyncio.sleep(0.01) call with an explicit SQL UPDATE on a document. This will deterministically set a more recent updated_at timestamp, removing the potential for flaky tests. src/agents-api/tests/test_docs_queries.py [369-397] # Ensure the second document has a greater updated_at timestamp await asyncio.sleep(0.01) second_doc = await create_doc( developer_id=developer.id, data=CreateDocRequest( title="Sort regression doc 2", content="content two", metadata=shared_metadata, embed_instruction="Embed", ), owner_type="user", owner_id=user.id, connection_pool=pool, ) +# Explicitly update the first document to make its updated_at the latest +await pool.execute("UPDATE docs SET updated_at = NOW() WHERE doc_id = $1", first_doc.id) + docs = await list_docs( developer_id=developer.id, owner_type="user", owner_id=user.id, connection_pool=pool, metadata_filter=shared_metadata, sort_by="updated_at", direction="desc", ) assert len(docs) >= 2 -assert docs[0].id == second_doc.id -assert docs[1].id == first_doc.id +# The first doc was updated last, so it should appear first +assert docs[0].id == first_doc.id +assert docs[1].id == second_doc.id Apply / Chat Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that using asyncio.sleep to ensure timestamp differences can lead to flaky tests and proposes a more robust solution by explicitly updating a record, which improves test reliability.	Medium
More

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to a91db86 in 1 minute and 51 seconds. Click for details.

• Reviewed 110 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/agents-api/agents_api/queries/docs/list_docs.py:142

• Draft comment:
  Validate the sort_by input explicitly. If sort_by is not in SORT_COLUMN_MAP, raise an HTTPException with detail "Invalid sort field" instead of risking a KeyError.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The Literal type combined with @beartype decorator on line 65 means this is already type-checked at runtime. The SORT_COLUMN_MAP keys exactly match the Literal type options. A KeyError is impossible unless there's a bug in the type checking, which is extremely unlikely. The validation would be redundant. The comment might have a point about defensive programming - what if the code is called from untyped code that bypasses the type checker? What if SORT_COLUMN_MAP is modified elsewhere? The @beartype decorator ensures runtime type checking even if called from untyped code. SORT_COLUMN_MAP is a module-level constant that matches the Literal type exactly. The suggested validation would be redundant with the existing type safety mechanisms. The code is already safe through the combination of Literal types and runtime type checking via @beartype.

2. src/agents-api/tests/test_docs_queries.py:350

• Draft comment:
  Good test coverage for sorting by 'updated_at'. It verifies that documents are ordered by the latest update timestamps.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_0aJUBjqxMU2W52rz

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Add explicit validation for the sort direction. Currently, any value other than 'asc' defaults to 'DESC', but tests expect an HTTPException with "Invalid sort direction" for invalid inputs.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 5514730 in 46 seconds. Click for details.

• Reviewed 15 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/agents-api/agents_api/queries/docs/list_docs.py:141

• Draft comment:
  The update consolidates the ORDER BY clause string into a single f-string for conciseness. The approach is safe because both the sort column and direction are controlled via a mapping and literals.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_187uYBSeIxsOGSQ7

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[claude]

Code Review for PR #1587: Fix sort by updated_at query in list docs

Thank you for this PR! I have reviewed the changes and have the following feedback:

✅ Strengths

1. Excellent SQL Injection Prevention: The introduction of SORT_COLUMN_MAP and parameterized direction handling is a solid security improvement that prevents SQL injection vulnerabilities.

2. Good Test Coverage: The regression test properly verifies the sorting functionality with a clear test case that creates two documents with different timestamps.

3. Proper AIDEV-NOTE: The comment explaining the purpose of SORT_COLUMN_MAP follows project conventions and provides clear context.

4. Correct Query Fix: Adding d.updated_at to both the SELECT and GROUP BY clauses properly fixes the sorting issue.

🔍 Areas for Improvement

1. Security Concern in Related Code

While reviewing your changes, I noticed that src/agents-api/agents_api/queries/files/list_files.py:83 has a similar pattern but lacks the same security improvements:

query += f" ORDER BY f.{sort_by} {direction} LIMIT ${param_index} OFFSET ${param_index + 1}"

This directly interpolates sort_by and direction into the SQL query. While these are validated by Literal types, the approach in your PR is more robust. Consider applying the same pattern there in a follow-up PR.

2. Test Reliability

The test uses await asyncio.sleep(0.01) to ensure different timestamps. While this works, consider:

• This might be flaky in heavily loaded CI environments
• Some databases might have timestamp precision that could make this unreliable

Alternative approaches:

• Update the first doc after creating the second to ensure updated_at changes
• Use a longer sleep or verify the timestamps are actually different with an assertion

3. Missing Edge Cases in Tests

Consider adding tests for:

• Sorting by updated_at in ascending order
• Documents with identical updated_at timestamps (to verify stable sorting)
• Combining updated_at sorting with other filters more comprehensively

4. Documentation

The function docstring in list_docs correctly lists the parameters, but consider documenting this security pattern in the directory AGENTS.md file for future reference.

💡 Suggestions

1. Extend SORT_COLUMN_MAP for Future Fields: If more sortable fields are added in the future, this pattern scales well. Consider documenting this as the preferred approach.

2. Consider a Shared Utility: Since multiple query functions need safe sorting, consider extracting this pattern into a shared utility function that could be reused across list_docs.py, list_files.py, and other similar queries.

✅ Verdict

This is a well-implemented fix that properly addresses the sorting issue while improving security. The code is clean, follows project conventions, and includes appropriate tests. With the minor improvements suggested above (particularly addressing the test reliability), this PR is ready to merge.

Great work on identifying and fixing this issue with a security-conscious approach! 🎉