Skip to content

Raise list_instruments default limit to 500; warn when truncated - #77

Open
Apurv-Salunke wants to merge 1 commit into
mainfrom
feat/list-instruments-limit-warn
Open

Apurv-Salunke wants to merge 1 commit into
mainfrom
feat/list-instruments-limit-warn

Conversation

@Apurv-Salunke

@Apurv-Salunke Apurv-Salunke commented Aug 3, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Raise list_instruments default limit from 100 → 500 (DEFAULT_LIST_LIMIT).
  • Log a WARNING (instruments.list.truncated) when len(results) == limit so a capped page is not mistaken for a full result set; pass limit=None for an unbounded scan.
  • Changelog entry under [Unreleased]; integration tests for warn / no-warn paths.

Test plan

  • make ci locally (ruff, mypy, 351 tests, coverage ≥64%)
  • tests/integration/test_instrument_store_queries.py — truncation warning + under-limit / limit=None
  • Manual: setup_logging(level="WARNING") then store.list_instruments(..., limit=2) and confirm WARNING; limit=None silent

Risk notes

  • Callers that assumed an implicit max of 100 rows will now receive up to 500 by default (still capped). Behaviour for explicit limit= / limit=None is unchanged aside from the new warning when the page is full.

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Instrument listings now return up to 500 results by default.
    • Set the limit to None to perform an unlimited scan.
    • Listings that reach the configured limit now generate a truncation warning.
  • Documentation

    • Updated public documentation and changelog to describe default limits, unlimited scans, and truncation warnings.
  • Tests

    • Added coverage for warnings when results reach the limit and for cases where results remain below it.

Silent limit=100 made full scans look complete in the REPL; log when
the page is full so callers know to pass limit=None.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: bba49872-58a6-4af0-b40d-0e6aeaffa5c7

📥 Commits

Reviewing files that changed from the base of the PR and between 15eb5cb and ec71e41.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • tests/integration/test_instrument_store_queries.py
  • tt_connect/core/store/queries.py
  • tt_connect/core/store/store.py

📝 Walkthrough

Walkthrough

list_instruments now defaults to 500 results, supports limit=None for unbounded scans, and logs instruments.list.truncated when bounded results reach the limit. Integration tests cover warning and no-warning cases.

Changes

Instrument listing limits

Layer / File(s) Summary
Listing limit contract
tt_connect/core/store/queries.py, tt_connect/core/store/store.py
Both store interfaces and InstrumentQueries.list_instruments use DEFAULT_LIST_LIMIT = 500. Their documentation describes unbounded scans and truncation warnings.
Truncation detection and validation
tt_connect/core/store/queries.py, tests/integration/test_instrument_store_queries.py, CHANGELOG.md
Query branches share result processing and log instruments.list.truncated when a bounded result count reaches its limit. Integration tests cover warning and no-warning cases. The changelog records the behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant InstrumentStore
  participant InstrumentQueries
  participant Logger
  InstrumentStore->>InstrumentQueries: list_instruments(limit)
  InstrumentQueries->>InstrumentQueries: collect results
  InstrumentQueries->>Logger: warn when count equals finite limit
  InstrumentQueries-->>InstrumentStore: return results
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the change and tests, but it omits required Release Impact, Test Evidence outputs, rollback guidance, and Checklist sections. Add the missing template sections and include command outputs, API and migration impact, rollback steps, and completed checklist items.
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the two main changes: increasing the default limit and warning when results are truncated.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/list-instruments-limit-warn

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

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant