fix(DAT-121): make property_v2 limit a total cap and warn on silent truncation

[zshamroukh]

Fixes DAT-121. Surfaced by PAR-100 (Central Michigan University). Breaking changes deliberately carved out to DAT-122.

The bug

Passing any explicit limit to property_v2.search.retrieve silently disabled auto-pagination:

if limit == 0 or limit is None:
    return max_limit, True       # omitted => fetch everything
return limit, False              # ANY explicit limit => one page, rest discarded

So limit=1000 meant "one page of 1,000, silently discard the other 107,295." No error, no warning, a populated DataFrame that looked complete. Omitting limit was the only way to get complete data — the opposite of what a caller would guess.

An academic customer built a 50-CBSA / 6-year research panel this way and received ~0.4% of the data. He only noticed because the row counts felt low.

Why this is a redesign rather than another patched condition

This was the third attempt at this logic — CHANGELOG v1.15.2 "fix limit logic bug", v1.16.1 "Fix bug in auto pagination logic" — and the multi-page path had no test coverage at all. test_fetch_post_pagination built two mock pages, then called _fetch_post with pagination disabled and asserted a single request, so the second page was never consumed. Two further tests asserted the buggy behaviour as correct. All three are rewritten here.

Worth noting: limit-as-a-total-cap is what the README has documented all along —

Use the limit parameter to specify the number of matched properties to return. If limit is not provided, all matched properties will be returned.

The implementation had silently diverged from its own docs. This restores the documented intent rather than inventing new semantics.

Changes

• limit is now a cap on total properties returned. Pagination is an internal detail of satisfying it. Page size is derived as min(limit, 50_000).
• limit above 50,000 paginates instead of failing. The schema's le=50000 bound rejected these in Pydantic before the pagination logic ran at all. Removing it does not weaken the server contract — every individual request still sends limit <= 50000. (History check: the constant was 100,000 while auto_paginate was public, and was lowered to 50,000 in the same commit that removed it — it tracked the per-request ceiling, never a product cap. limit=None has always returned unbounded data, so 50,000 was never a volume guardrail.)
• ParclLabsTruncationWarning when limit withholds matching data, reporting returned vs available counts. Once per session, so per-market loops stay usable.
• Failed pages are retried (3 attempts, exponential backoff) and then reported via ParclLabsIncompleteResultWarning + metadata["incomplete_pages"], instead of being printed and skipped as if the result were complete. There was previously no retry logic anywhere in the SDK, and 1.17.2 added a 90s read timeout — so timeouts on 50,000-property pages are a live failure mode. NYC alone is 54 pages.
• Pagination integrity check warns if assembled pages do not yield the expected number of distinct properties. Offset pagination is only safe while the server applies a stable sort; this surfaces a regression here rather than in a customer's dataset.
• Stop leaking the internal auto_paginate flag into the request query string (every call was shipping ?limit=…&auto_paginate=False).
• Stop duplicating limit in paginated page URLs (?limit=50000&offset=50000&limit=50000).
• _get_metadata deep-copies, so it no longer mutates the caller's raw first-page response through a shallow copy.

Warnings use warnings.warn rather than print so callers can filter or escalate them — print is exactly what failed this customer.

Not changed

limit=N for N <= 50000 issues an identical HTTP request and returns an identical result. The behavioural fix for PAR-100 is that partial results now announce themselves.

Verification

Live against prod — NYC (parcl_id 2900187), Jan 2024, total_available 108,295:

Call	Properties	Warning
limit=1000 (the customer's setting)	1,000	"Returned 1,000 of 108,295…"
limit=60000	60,000 over 2 pages	truncation
limit omitted	108,295 (= total_available)	none

limit=60000 previously returned HTTP 422. The integrity check passed silently in all three cases, confirming pagination stays lossless at 108,295.

94 tests pass; ruff check and ruff format --check clean. Version bumped to 1.18.0 with CHANGELOG.

Test coverage added

Multi-page walk, stop-at-cap, final-page trimming, truncation warning fires once per session, no warning when complete, retry recovery, incomplete-pages warning on every affected call, metadata surfacing, no-mutation guard, integrity mismatch, non-positive limit rejection, above-ceiling acceptance.

Note for reviewers

While verifying, ge=1 on the schema turned out to already reject limit=0 and negatives — so DAT-122's first item ("limit <= 0 should raise") was based on a misreading of mine and is closed there. DAT-122 is now down to one genuinely breaking change (parcl_property_ids + limit).

🤖 Generated with Claude Code

---

Note

High Risk
Changes core property search pagination and limit semantics (previously silent data loss); behavior shifts for large limits and partial failures, though ≤50k limits keep the same HTTP shape when complete.

Overview
property_v2.search.retrieve now treats limit as a total cap on properties returned, with internal pagination to honor it (including values above the API’s 50k per-request ceiling). Explicit limits no longer disable pagination, fixing silent one-page truncation when callers expected a sample cap, not a single page.

Pagination is reworked: per-page fetches use retries with backoff, failed offsets surface in metadata["incomplete_pages"], and ParclLabsTruncationWarning / ParclLabsIncompleteResultWarning (new parcllabs.warnings) replace silent print behavior. A post-assembly integrity check warns when distinct parcl_property_id counts don’t match reported returned_count. The internal auto_paginate flag is no longer sent on requests; PropertyV2RetrieveParams.limit drops the 50k schema cap; _get_metadata deep-copies so callers’ raw responses aren’t mutated.

Docs and CHANGELOG describe credits-per-property vs event-level DataFrames; version 1.18.0; tests cover multi-page fetch, caps, warnings, retries, and metadata.

^{Reviewed by Cursor Bugbot for commit e01624c. Configure here.}

[zshamroukh]

bugbot review

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit e01624c. Configure here.}