Skip to content

atlas-search(service): bound the results-paging loop — deadline + cursor-progress guard (#5424) - #5437

Merged
dezren39 merged 1 commit into
mainfrom
queue/9435-5424-atlas-paging-guard
Nov 4, 2026
Merged

dezren39 merged 1 commit into
mainfrom
queue/9435-5424-atlas-paging-guard

Conversation

@dezren39

@dezren39 dezren39 commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Fixes #5424 (queue task 9435).

The via-service client's results-paging loop in service.rs search() had no deadline and no cursor-progress guard: a well-formed but non-advancing page stream (repeating cursor, or empty pages that keep returning a cursor) looped forever, hanging the CLI instead of returning the Err that triggers the documented loud standalone fallback.

  • The paging loop now checks the same 330 s client deadline (deadline, already computed for the poll loop) on every iteration and returns Err when it expires mid-paging.
  • A next_cursor equal to the cursor just used, or an empty page that still returns a cursor, is a malformed response: Err, not another iteration.
  • search() is split into search() (token resolution) + search_with_token() so the flow is unit-testable without env mutation.
  • Stub-service unit tests (in-process TCP stub: submit → done → results):
    • search_repeating_cursor_returns_err_promptly — repeating cursor returns Err ("did not advance") promptly.
    • search_empty_page_with_cursor_returns_err — empty page with a cursor returns Err.
    • search_multi_page_happy_path_unchanged — two-page happy path still splices both pages.

Tests: cargo test -p atlas-search green on the laptop (full suite exit 0; the 3 new tests pass by name); cargo fmt -p atlas-search applied.


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

Summary by CodeRabbit

  • Bug Fixes
    • Search results now stop paging when the request deadline is reached, a page is empty despite having a next cursor, or a cursor repeats.
    • Searches continue across pages until the requested number of results is collected or no further cursor is available.

The paging loop in search() now honors the same 330s client deadline as the poll loop, and a next_cursor that does not advance (or an empty page that still returns a cursor) is a malformed response returning Err, so the caller falls back to standalone loudly instead of hanging forever.

Adds stub-service unit tests: repeating cursor returns Err promptly, empty page with cursor returns Err, multi-page happy path unchanged. cargo test -p atlas-search green.
@dezren39
dezren39 requested a review from a team as a code owner November 4, 2026 12:23
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T12:25:17.926447Z e24fbd5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Explain Change →

Caution

Review failed

The pull request is closed.

Walkthrough

The search client now resolves its token before entering a private helper. The results-paging loop checks the job deadline and rejects repeated cursors or empty pages that provide a next cursor. Tests cover these error cases and successful multi-page results.

Changes: Atlas search results paging

Layer / File(s) Summary
Paging deadline, cursor validation, and tests
pkgs/atlas-search/crates/atlas-search/src/service.rs
search resolves the client token before calling search_with_token. Paging returns an error when the 330-second deadline expires, a page is empty but has a next cursor, or the next cursor repeats the cursor just used. Stub tests cover both error cases and a successful multi-page response.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

Possibly related PRs

Merge Risk: Low · up to e24fb

The paging guards improve malformed-response handling. The change is mergeable with bounded follow-up to test deadline expiry and verify ordered multi-page results; no newly introduced production failure is established.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant