diff --git a/CHANGELOG.md b/CHANGELOG.md index d188e8a..9e7430a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +### v1.18.0 +- **`property_v2.search.retrieve`: `limit` is now a cap on the total number of properties returned, not a page size.** Pagination is handled internally to satisfy it. Previously, passing *any* explicit `limit` silently disabled auto-pagination, so `limit=1000` returned one page of 1,000 and discarded every remaining match with no error or warning. Calls with `limit <= 50000` are unaffected — same request, same results. +- **`limit` above 50,000 now paginates instead of failing.** Previously the request was rejected by the API with `422 limit input should be less than or equal to 50000`. +- **Partial results now warn instead of passing silently.** When `limit` withholds matching data, a `ParclLabsTruncationWarning` reports how many properties were returned versus how many matched (emitted once per session). Note credits are charged per *property* returned, not per event, and because the returned DataFrame is event-level, `len(df)` is not bounded by `limit`. +- **Failed pages during pagination are now retried and reported.** Pages are retried up to 3 times with exponential backoff; if any still fail the result is returned with a `ParclLabsIncompleteResultWarning` and the failed offsets are listed in `metadata["incomplete_pages"]`. Previously a failed page was printed and skipped, returning short data indistinguishable from complete data. +- Added a pagination integrity check that warns if the assembled pages do not yield the expected number of distinct properties. +- New warning categories in `parcllabs.warnings` (`ParclLabsWarning`, `ParclLabsTruncationWarning`, `ParclLabsIncompleteResultWarning`) so callers can silence or escalate these via standard `warnings` filters. +- Fixed an internal `auto_paginate` flag leaking into the request query string. +- Fixed `_get_metadata` mutating the caller's raw first-page response via a shallow copy. + ### v1.17.2 - Added configurable request timeout to `ParclLabsClient`. Defaults to 10s connect / 90s read. Customizable via the `timeout` parameter on client instantiation. diff --git a/README.md b/README.md index 9d5cb49..897a587 100644 --- a/README.md +++ b/README.md @@ -480,6 +480,46 @@ Gets a list of unique properties and their associated metadata and events based **NOTE:** Use the `limit` parameter to specify the number of matched properties to return. If `limit` is not provided, all matched properties will be returned. Conceptually, you should set the `limit` to retrieve a sample of properties, and then if you want to retrieve all properties, make the same request again without the `limit` parameter. +`limit` is a cap on the total number of **properties** returned, and pagination is handled for you — values larger than the API's 50,000 per-request maximum are fetched across multiple pages rather than rejected. Two things to keep in mind: + +- **Credits are charged per property returned, not per event.** +- The returned DataFrame is event-level, so `len(df)` is *not* bounded by `limit` — a single property can contribute many rows. + +If `limit` caps the result below the number of matching properties, a `ParclLabsTruncationWarning` is emitted (once per session) and both counts are available in the returned metadata. If any page fails after retries, the data is still returned but a `ParclLabsIncompleteResultWarning` is raised and the failed offsets are listed in `metadata["incomplete_pages"]`. + +Both conditions are worth checking programmatically, especially in a loop over many markets where a warning is easy to miss: + +```python +results, metadata = client.property_v2.search.retrieve(parcl_ids=[2900187], limit=5) + +counts = metadata["results"] +if counts["returned_count"] < counts["total_available"]: + print( + f"Truncated: got {counts['returned_count']:,} of " + f"{counts['total_available']:,} properties. Raise `limit`, or omit it entirely." + ) + +if metadata.get("incomplete_pages"): + print(f"Incomplete: pages failed at offsets {metadata['incomplete_pages']}") +``` + +If a short result should be fatal for your pipeline, make those checks `assert`s or raise your own exception — treat a non-empty `incomplete_pages` as an incomplete dataset either way. Both warning types live in `parcllabs.warnings` and can be silenced or escalated with standard `warnings` filters: + +```python +import warnings + +from parcllabs.warnings import ParclLabsIncompleteResultWarning, ParclLabsTruncationWarning + +with warnings.catch_warnings(): + # Intentionally sampling? Silence the truncation notice. + warnings.filterwarnings("ignore", category=ParclLabsTruncationWarning) + + # Never accept a partial page silently -- make it raise instead. + warnings.filterwarnings("error", category=ParclLabsIncompleteResultWarning) + + results, metadata = client.property_v2.search.retrieve(parcl_ids=[2900187], limit=5) +``` + Example request, note that only one of `parcl_ids`, `parcl_property_ids`, or `geo_coordinates` can be provided per request: diff --git a/parcllabs/__version__.py b/parcllabs/__version__.py index 99f794b..4c0681d 100644 --- a/parcllabs/__version__.py +++ b/parcllabs/__version__.py @@ -1 +1 @@ -VERSION = "1.17.2" +VERSION = "1.18.0" diff --git a/parcllabs/schemas/schemas.py b/parcllabs/schemas/schemas.py index 11aca06..8512764 100644 --- a/parcllabs/schemas/schemas.py +++ b/parcllabs/schemas/schemas.py @@ -142,11 +142,21 @@ class PropertyV2RetrieveParams(BaseModel): ) # Pagination + # + # No upper bound: `limit` is a cap on the total number of properties returned, + # and values above the API's per-request ceiling + # (RequestLimits.PROPERTY_V2_MAX) are satisfied by paginating rather than + # rejected. Omit to retrieve every matching property. limit: int | None = Field( default=None, ge=1, - le=RequestLimits.PROPERTY_V2_MAX.value, - description=f"Number of results to return (max: {RequestLimits.PROPERTY_V2_MAX.value})", + description=( + "Maximum number of properties to return in total. Values above " + f"{RequestLimits.PROPERTY_V2_MAX.value} are fetched across multiple pages. " + "Omit to retrieve all matching properties. Credits are charged per property " + "returned; the returned DataFrame is event-level, so len(df) is not bounded " + "by this value." + ), ) # Additional parameters diff --git a/parcllabs/services/properties/property_v2.py b/parcllabs/services/properties/property_v2.py index 74877c6..7b4ada8 100644 --- a/parcllabs/services/properties/property_v2.py +++ b/parcllabs/services/properties/property_v2.py @@ -1,3 +1,4 @@ +import copy import time from collections.abc import Mapping from concurrent.futures import ThreadPoolExecutor, as_completed @@ -10,6 +11,15 @@ from parcllabs.schemas.schemas import PropertyV2RetrieveParamCategories, PropertyV2RetrieveParams from parcllabs.services.parcllabs_service import ParclLabsService from parcllabs.services.validators import Validators +from parcllabs.warnings import ( + warn_incomplete_pages, + warn_integrity_mismatch, + warn_truncation, +) + +# Transient page failures are retried before a page is abandoned. +PAGE_FETCH_ATTEMPTS = 3 +PAGE_FETCH_BACKOFF_SECONDS = 1.0 class PropertyV2Service(ParclLabsService): @@ -26,55 +36,127 @@ def _raise_http_error(chunk_num: int, status_code: int, response_preview: str) - def _raise_empty_response_error(chunk_num: int) -> None: raise RuntimeError(f"Chunk {chunk_num} failed: Empty response from API") - def _fetch_post(self, params: dict[str, Any], data: dict[str, Any]) -> list[dict]: - """Fetch data using POST request with pagination support.""" + def _fetch_page( + self, + data: dict[str, Any], + params: dict[str, Any], + offset: int, + limit: int, + ) -> dict: + """Fetch a single page, retrying transient failures with exponential backoff. + + Raises the last exception if every attempt fails. + """ + page_params = dict(params) + page_params["limit"] = limit + page_params["offset"] = offset + + last_exc: Exception | None = None + for attempt in range(PAGE_FETCH_ATTEMPTS): + try: + return self._post(url=self.full_post_url, data=data, params=page_params).json() + except Exception as exc: # retried below, then re-raised + last_exc = exc + if attempt < PAGE_FETCH_ATTEMPTS - 1: + time.sleep(PAGE_FETCH_BACKOFF_SECONDS * (2**attempt)) + raise last_exc # type: ignore[misc] + + def _fetch_post( + self, + params: dict[str, Any], + data: dict[str, Any], + max_results: int | None = None, + ) -> list[dict]: + """Fetch data using POST, paginating until ``max_results`` is satisfied. + + Args: + params: Request params. ``params["limit"]`` is the page size. + data: POST body containing the search criteria and filters. + max_results: Maximum number of properties to return in total. ``None`` + retrieves every matching property. + + Returns: + List of raw page payloads. Failed pages are omitted and reported via a + ``ParclLabsIncompleteResultWarning``. + """ + params = dict(params) response = self._post(url=self.full_post_url, data=data, params=params) result = response.json() - all_data = [result] - if params["auto_paginate"] is False: + pagination = result.get("pagination") or {} + results_meta = (result.get("metadata") or {}).get("results") or {} + total_available = results_meta.get("total_available", 0) + retrieved = results_meta.get("returned_count", 0) + + # How many properties do we actually want? Never more than exist. + target = total_available if max_results is None else min(max_results, total_available) + + if retrieved >= target or not pagination.get("has_more"): + # Gate on whether a cap actually withheld data (`target`), but report the + # count actually returned (`retrieved`). Gating on `retrieved` instead would + # also fire when no cap was set and the server merely ended pagination early, + # blaming `limit` for a shortfall it did not cause -- and burning the + # once-per-session budget on advice the caller cannot act on. + if target < total_available: + warn_truncation(retrieved, total_available) return all_data - # If we need to paginate, use concurrent requests - pagination = result.get("pagination") - if pagination.get("has_more"): - print("More pages to fetch, paginating additional pages...") - - limit = pagination.get("limit") - offset = pagination.get("offset") - metadata = result.get("metadata") - total_available = metadata.get("results", {}).get("total_available", 0) - - # Calculate how many more pages we need - remaining_pages = (total_available - limit) // limit - if (total_available - limit) % limit > 0: - remaining_pages += 1 - - # Generate all the URLs we need to fetch - urls = [] - current_offset = offset + limit - for _ in range(remaining_pages): - urls.append(f"{self.full_post_url}?limit={limit}&offset={current_offset}") - current_offset += limit - - # Use ThreadPoolExecutor to make concurrent requests - with ThreadPoolExecutor(max_workers=self.client.num_workers) as executor: - future_to_url = { - executor.submit(self._post, url=url, data=data, params=params): url - for url in urls - } - - for future in as_completed(future_to_url): - try: - response = future.result() - page_result = response.json() - all_data.append(page_result) - except Exception as exc: - print(f"Request failed: {exc}") + page_size = pagination.get("limit") or params.get("limit") or retrieved + offset = pagination.get("offset", 0) + + # Request exactly the pages needed to reach `target` -- the final page is + # trimmed so an explicit `limit` is honoured precisely rather than overshot. + pages: list[tuple[int, int]] = [] + current_offset = offset + retrieved + remaining = target - retrieved + while remaining > 0: + this_limit = min(page_size, remaining) + pages.append((current_offset, this_limit)) + current_offset += this_limit + remaining -= this_limit + + failed_offsets: list[int] = [] + with ThreadPoolExecutor(max_workers=self.client.num_workers) as executor: + future_to_offset = { + executor.submit( + self._fetch_page, data, params, page_offset, page_limit + ): page_offset + for page_offset, page_limit in pages + } + for future in as_completed(future_to_offset): + page_offset = future_to_offset[future] + try: + all_data.append(future.result()) + except Exception: # surfaced as a warning below + failed_offsets.append(page_offset) + + actually_retrieved = self._total_returned(all_data) + + if failed_offsets: + # Report the shortfall and stop. Deliberately no truncation warning here: + # we did NOT return `target`, so claiming we did would contradict this + # warning, and the truncation notice is once-per-session -- burning it on a + # misleading message would suppress a legitimate one later in the run. + # `total_available` is included so capping information is not lost. + failed_offsets.sort() + warn_incomplete_pages(failed_offsets, target, actually_retrieved, total_available) + all_data[0].setdefault("_parcllabs", {})["incomplete_pages"] = failed_offsets + return all_data + + if target < total_available: + warn_truncation(actually_retrieved, total_available) return all_data + @staticmethod + def _total_returned(pages: list[dict]) -> int: + """Sum the properties actually returned across assembled pages.""" + return sum( + (page.get("metadata") or {}).get("results", {}).get("returned_count", 0) + for page in pages + ) + def _fetch_post_parcl_property_ids( self, params: dict[str, Any], @@ -241,8 +323,9 @@ def _get_metadata(self, results: list[Mapping[str, Any]]) -> dict[str, Any]: if not results: return {} - # Start with a copy of the first result's metadata - metadata = results[0].get("metadata", {}).copy() + # Deep copy: a shallow .copy() leaves metadata["results"] aliased to the raw + # first page, so assigning returned_count below would mutate the response. + metadata = copy.deepcopy(results[0].get("metadata", {})) # Calculate total returned_count total_returned = sum( @@ -252,6 +335,11 @@ def _get_metadata(self, results: list[Mapping[str, Any]]) -> dict[str, Any]: if "results" in metadata: metadata["results"]["returned_count"] = total_returned + # Surface any pages that could not be fetched (see _fetch_post). + incomplete = (results[0].get("_parcllabs") or {}).get("incomplete_pages") + if incomplete: + metadata["incomplete_pages"] = incomplete + return metadata def _build_search_criteria( @@ -430,20 +518,30 @@ def _build_owner_filters(self, params: PropertyV2RetrieveParams) -> dict[str, An return owner_filters - def _set_limit_pagination(self, limit: int | None) -> tuple[int, bool]: - """Validate and set limit and auto pagination.""" + def _set_limit_pagination(self, limit: int | None) -> tuple[int, int | None]: + """Resolve the caller's ``limit`` into a page size and a total cap. + + ``limit`` is the maximum number of *properties* to return in total, not a + page size. Pagination is an internal detail of satisfying it. + + Args: + limit: Maximum properties to return. ``None`` (or ``0``) means no cap. + + Returns: + ``(page_size, max_results)`` where ``max_results`` is ``None`` when + unbounded. + """ max_limit = RequestLimits.PROPERTY_V2_MAX.value - # If no limit is provided, use maximum limit and auto paginate + # `0` is treated as "no cap" defensively only. Callers cannot reach this via + # retrieve(): PropertyV2RetrieveParams enforces ge=1, so 0 and negatives are + # already rejected at validation time. if limit == 0 or limit is None: - auto_paginate = True - print(f"""No limit provided. Using max limit of {max_limit}. - Auto pagination is {auto_paginate}""") - return max_limit, auto_paginate + return max_limit, None - auto_paginate = False - print(f"Limit is set at {limit}. Auto pagiation is {auto_paginate}") - return limit, auto_paginate + # A limit above the API's per-request ceiling is satisfied by paginating, + # rather than by letting the server reject the request outright. + return min(limit, max_limit), limit def _build_param_categories( self, params: PropertyV2RetrieveParams @@ -539,7 +637,16 @@ def retrieve( current_entity_owner_name: Current entity owner name to filter by. include_events: Whether to include events in the response. include_full_event_history: Whether to include full event history in the response. - limit: Number of results to return. + limit: Maximum number of *properties* to return in total. Pagination is + handled internally to satisfy it, so values above the API's + per-request ceiling (50,000) are fetched across several pages rather + than rejected. Omit (or pass None) to retrieve every matching + property. Two things to note: credits are charged per property + returned, not per event; and because the returned DataFrame is + event-level, ``len(df)`` is NOT bounded by ``limit`` -- one property + may contribute many rows. If ``limit`` withholds matching data, a + ParclLabsTruncationWarning is emitted and + ``metadata["results"]`` reports both counts. params: Additional parameters to pass to the request. Returns: A tuple containing (pandas DataFrame, metadata dictionary). @@ -604,19 +711,22 @@ def retrieve( # Update data with categories data.update(param_categories.model_dump(exclude_none=True)) - # Set limit + # Set limit. `auto_paginate` is deliberately NOT placed in request_params -- + # it is an internal concern and was previously leaking into the query string. request_params = input_params.params.copy() - request_params["auto_paginate"] = False # auto_paginate is False by default # Make request with params if data.get(PARCL_PROPERTY_IDS): + # Querying by explicit property IDs: the ID list bounds the result, so a + # single page of PARCL_PROPERTY_IDS_LIMIT can always hold it, and >that + # many IDs are chunked in _fetch_post_parcl_property_ids. The caller's + # `limit` is not honoured on this path (breaking change -> DAT-122). request_params["limit"] = PARCL_PROPERTY_IDS_LIMIT results = self._fetch_post_parcl_property_ids(params=request_params, data=data) else: - request_params["limit"], request_params["auto_paginate"] = self._set_limit_pagination( - input_params.limit - ) - results = self._fetch_post(params=request_params, data=data) + page_size, max_results = self._set_limit_pagination(input_params.limit) + request_params["limit"] = page_size + results = self._fetch_post(params=request_params, data=data, max_results=max_results) # Get metadata from results metadata = self._get_metadata(results) @@ -624,4 +734,28 @@ def retrieve( # Process results final_df = self._as_pd_dataframe(results) + self._check_pagination_integrity(final_df, metadata) + return final_df, metadata + + @staticmethod + def _check_pagination_integrity(final_df: pd.DataFrame, metadata: dict[str, Any]) -> None: + """Warn if assembled pages did not yield the expected property count. + + Offset pagination is only safe while the server applies a stable sort. This + is a cheap guard so an upstream ordering change surfaces here rather than as + silently duplicated or missing rows in a customer's dataset. + """ + if final_df.empty or "parcl_property_id" not in final_df.columns: + return + # Pages that failed outright are already reported; don't double-warn. + if metadata.get("incomplete_pages"): + return + + expected = (metadata.get("results") or {}).get("returned_count") + if not expected: + return + + unique_properties = final_df["parcl_property_id"].nunique() + if unique_properties != expected: + warn_integrity_mismatch(unique_properties, expected) diff --git a/parcllabs/warnings.py b/parcllabs/warnings.py new file mode 100644 index 0000000..8f374cf --- /dev/null +++ b/parcllabs/warnings.py @@ -0,0 +1,107 @@ +"""Runtime warnings emitted by the ParclLabs SDK. + +These are warnings rather than prints so that callers can filter, capture, or +escalate them: + + import warnings + from parcllabs.warnings import ParclLabsTruncationWarning + + # silence intentional truncation + warnings.filterwarnings("ignore", category=ParclLabsTruncationWarning) + + # or turn incomplete results into a hard failure + warnings.filterwarnings("error", category=ParclLabsIncompleteResultWarning) +""" + +import warnings + + +class ParclLabsWarning(UserWarning): + """Base class for all ParclLabs SDK runtime warnings.""" + + +class ParclLabsTruncationWarning(ParclLabsWarning): + """More data matched the query than was returned. + + Emitted when an explicit ``limit`` capped the result below the number of + matching properties. The returned data is correct, just partial -- this is + what the caller asked for. Emitted once per session to stay usable inside + per-market loops. + """ + + +class ParclLabsIncompleteResultWarning(ParclLabsWarning): + """One or more pages could not be fetched, so the result is short. + + Unlike truncation, this is a failure rather than a request: the caller asked + for data the SDK was unable to retrieve. Emitted on every affected call, and + the failed page offsets are recorded in ``metadata["incomplete_pages"]``. + """ + + +# Truncation is a requested outcome, so it is announced once per process rather +# than on every call. A module-level flag is used deliberately: mutating the +# global `warnings` filters from library code would clobber caller configuration. +_truncation_warned = False + + +def warn_truncation(returned_count: int, total_available: int, stacklevel: int = 4) -> None: + """Warn once per session that an explicit ``limit`` withheld matching data.""" + global _truncation_warned # noqa: PLW0603 + if _truncation_warned: + return + _truncation_warned = True + warnings.warn( + f"Returned {returned_count:,} of {total_available:,} matching properties because " + f"`limit` capped the result. Raise `limit` to retrieve more, or omit it entirely " + f"to retrieve all {total_available:,}. Credits are charged per property returned. " + f"Compare metadata['results']['returned_count'] against " + f"metadata['results']['total_available'] to detect this programmatically. " + f"(This warning is shown once per session.)", + ParclLabsTruncationWarning, + stacklevel=stacklevel, + ) + + +def warn_incomplete_pages( + failed_offsets: list[int], + expected: int, + retrieved: int, + total_available: int | None = None, + stacklevel: int = 4, +) -> None: + """Warn that pagination could not fetch every page. Fires on every occurrence. + + This supersedes the truncation warning when both would apply: the result is short + because pages failed, not because the caller asked for less. + """ + matched = "" + if total_available is not None and total_available > expected: + matched = f" ({total_available:,} properties matched the query in total.)" + warnings.warn( + f"Incomplete result: {len(failed_offsets)} page(s) failed after retries, so " + f"{retrieved:,} of an expected {expected:,} properties were retrieved. Failed " + f"offsets are listed in metadata['incomplete_pages']. Re-run those offsets or " + f"retry the query before treating this data as complete.{matched} Failed " + f"offsets: {failed_offsets}", + ParclLabsIncompleteResultWarning, + stacklevel=stacklevel, + ) + + +def warn_integrity_mismatch(unique_properties: int, expected: int, stacklevel: int = 4) -> None: + """Warn that assembled pages did not yield the expected number of properties.""" + warnings.warn( + f"Pagination integrity check failed: assembled {unique_properties:,} unique " + f"properties but expected {expected:,}. This can indicate overlapping or skipped " + f"pages (offset pagination relies on a stable server-side sort). The data is " + f"returned as-is; verify before use and report this to team@parcllabs.com.", + ParclLabsIncompleteResultWarning, + stacklevel=stacklevel, + ) + + +def _reset_truncation_warning() -> None: + """Reset the once-per-session truncation flag. Intended for tests.""" + global _truncation_warned # noqa: PLW0603 + _truncation_warned = False diff --git a/tests/test_property_v2.py b/tests/test_property_v2.py index b06e7d4..0ded210 100644 --- a/tests/test_property_v2.py +++ b/tests/test_property_v2.py @@ -1,7 +1,11 @@ +import warnings from unittest.mock import MagicMock, Mock, patch +import pandas as pd import pytest +from requests.exceptions import RequestException +from parcllabs import warnings as parcllabs_warnings from parcllabs.common import PARCL_PROPERTY_IDS from parcllabs.enums import RequestLimits from parcllabs.schemas.schemas import GeoCoordinates, PropertyV2RetrieveParams @@ -209,12 +213,70 @@ def test_schema_with_none_values() -> None: assert params.params == {} -def test_validate_limit(property_v2_service: PropertyV2Service) -> None: - assert property_v2_service._set_limit_pagination(limit=None) == ( +def test_set_limit_pagination_resolves_page_size_and_cap( + property_v2_service: PropertyV2Service, +) -> None: + max_limit = RequestLimits.PROPERTY_V2_MAX.value + + # No limit -> max page size, no cap (retrieve everything). + assert property_v2_service._set_limit_pagination(limit=None) == (max_limit, None) + + # An explicit limit is a TOTAL CAP; page size matches it while under the ceiling. + assert property_v2_service._set_limit_pagination(limit=100) == (100, 100) + + # A limit above the API ceiling is satisfied by paginating, not by erroring. + assert property_v2_service._set_limit_pagination(limit=120_000) == (max_limit, 120_000) + + +def test_limit_zero_treated_as_unbounded_defensively( + property_v2_service: PropertyV2Service, +) -> None: + """0 falls back to 'no cap' in the helper, but callers cannot reach it.""" + assert property_v2_service._set_limit_pagination(limit=0) == ( RequestLimits.PROPERTY_V2_MAX.value, - True, + None, ) - assert property_v2_service._set_limit_pagination(limit=100) == (100, False) + + +@pytest.mark.parametrize("bad_limit", [0, -1, -5000]) +def test_non_positive_limit_rejected_at_validation(bad_limit: int) -> None: + """The schema rejects 0 and negatives, so a computed zero cannot trigger an + unbounded pull.""" + with pytest.raises(ValueError, match="greater than or equal to 1"): + PropertyV2RetrieveParams(limit=bad_limit) + + +def test_limit_above_api_ceiling_is_accepted() -> None: + """Values above the per-request ceiling must validate; pagination satisfies them.""" + assert PropertyV2RetrieveParams(limit=120_000).limit == 120_000 + + +def _page( + parcl_property_id: int, + *, + total_available: int, + returned_count: int = 1, + limit: int = 1, + offset: int = 0, + has_more: bool = False, +) -> Mock: + """Build a mock page response.""" + response = Mock() + response.json.return_value = { + "data": [{"parcl_property_id": parcl_property_id}], + "metadata": { + "results": {"total_available": total_available, "returned_count": returned_count} + }, + "pagination": {"limit": limit, "offset": offset, "has_more": has_more}, + "account_info": {"credits_used": returned_count, "credits_remaining": 999}, + } + return response + + +@pytest.fixture(autouse=True) +def _reset_truncation_flag() -> None: + """Truncation warns once per session; reset between tests.""" + parcllabs_warnings._reset_truncation_warning() @patch.object(PropertyV2Service, "_post") @@ -222,7 +284,7 @@ def test_fetch_post_single_page( mock_post: Mock, property_v2_service: PropertyV2Service, mock_response: Mock ) -> None: mock_post.return_value = mock_response - result = property_v2_service._fetch_post(params={"auto_paginate": False}, data={}) + result = property_v2_service._fetch_post(params={"limit": 100}, data={}) assert len(result) == 1 assert result[0] == mock_response.json() @@ -231,32 +293,232 @@ def test_fetch_post_single_page( @patch.object(PropertyV2Service, "_post") def test_fetch_post_pagination(mock_post: Mock, property_v2_service: PropertyV2Service) -> None: - # First response with pagination - first_response = Mock() - first_response.json.return_value = { - "data": [{"parcl_id": 123}], - "metadata": {"results": {"total_available": 2, "returned_count": 1}}, - "pagination": {"limit": 1, "offset": 0, "has_more": True}, - "account_info": {"credits_used": 1, "credits_remaining": 999}, - } + """Unbounded fetch must actually walk every page and merge the results.""" + mock_post.side_effect = [ + _page(123, total_available=2, limit=1, offset=0, has_more=True), + _page(456, total_available=2, limit=1, offset=1, has_more=False), + ] - # Second response for pagination - second_response = Mock() - second_response.json.return_value = { - "data": [{"parcl_id": 456}], - "metadata": {"results": {"total_available": 2, "returned_count": 1}}, - "pagination": {"limit": 1, "offset": 1, "has_more": False}, - "account_info": {"credits_used": 1, "credits_remaining": 998}, - } + result = property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=None) - # Set up the mock to return different responses - mock_post.side_effect = [first_response, second_response] + assert mock_post.call_count == 2 + assert [page["data"][0]["parcl_property_id"] for page in result] == [123, 456] - result = property_v2_service._fetch_post(params={"limit": 1, "auto_paginate": False}, data={}) - assert len(result) == 1 - assert result[0]["data"][0]["parcl_id"] == 123 +@patch.object(PropertyV2Service, "_post") +def test_fetch_post_stops_at_max_results( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + """An explicit cap must not keep paginating toward total_available.""" + mock_post.return_value = _page(123, total_available=100, limit=1, offset=0, has_more=True) + + result = property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=1) + assert mock_post.call_count == 1 + assert len(result) == 1 + + +@patch.object(PropertyV2Service, "_post") +def test_fetch_post_trims_final_page_to_cap( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + """A cap that is not a multiple of the page size must not overshoot.""" + mock_post.side_effect = [ + _page(1, total_available=100, returned_count=10, limit=10, offset=0, has_more=True), + _page(2, total_available=100, returned_count=5, limit=5, offset=10, has_more=True), + ] + + property_v2_service._fetch_post(params={"limit": 10}, data={}, max_results=15) + + assert mock_post.call_count == 2 + # Second call asks for only the outstanding 5, at the correct offset. + second_params = mock_post.call_args_list[1][1]["params"] + assert second_params["limit"] == 5 + assert second_params["offset"] == 10 + + +@patch.object(PropertyV2Service, "_post") +def test_truncation_warning_fires_once_per_session( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + mock_post.return_value = _page(1, total_available=108_288, limit=1, offset=0, has_more=True) + + with pytest.warns(parcllabs_warnings.ParclLabsTruncationWarning, match="108,288"): + property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=1) + + # Second identical call is silent -- once per session. + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=1) + assert not [w for w in caught if w.category is parcllabs_warnings.ParclLabsTruncationWarning] + + +@patch.object(PropertyV2Service, "_post") +def test_no_truncation_warning_when_complete( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + mock_post.return_value = _page(1, total_available=1, limit=1, offset=0, has_more=False) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=1) + + assert not [w for w in caught if w.category is parcllabs_warnings.ParclLabsTruncationWarning] + + +@patch.object(PropertyV2Service, "_post") +def test_page_retry_recovers_from_transient_failure( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + mock_post.side_effect = [ + _page(1, total_available=2, limit=1, offset=0, has_more=True), + RequestException("boom"), + _page(2, total_available=2, limit=1, offset=1, has_more=False), + ] + + with patch("parcllabs.services.properties.property_v2.time.sleep"): + result = property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=None) + + assert mock_post.call_count == 3 + assert len(result) == 2 + + +@patch.object(PropertyV2Service, "_post") +def test_incomplete_pages_warn_every_call( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + """Exhausted retries must warn on EVERY call and record the failed offsets.""" + first = _page(1, total_available=2, limit=1, offset=0, has_more=True) + + def responses() -> list[object]: + return [first, *[RequestException("boom")] * 3] + + for _ in range(2): + mock_post.reset_mock() + mock_post.side_effect = responses() + with ( + patch("parcllabs.services.properties.property_v2.time.sleep"), + pytest.warns(parcllabs_warnings.ParclLabsIncompleteResultWarning), + ): + result = property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=None) + assert result[0]["_parcllabs"]["incomplete_pages"] == [1] + + +@patch.object(PropertyV2Service, "_post") +def test_failed_pages_suppress_contradictory_truncation_warning( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + """A capped request whose pages fail must not also claim the cap was met. + + Regression: _fetch_post emitted the incomplete warning with the real retrieved + count and *then* warned truncation with `target`, claiming the full cap was + returned. The two messages contradicted each other, and the truncation notice -- + which fires only once per session -- was burned on the misleading one. + """ + first = _page(1, total_available=108_295, limit=1, offset=0, has_more=True) + mock_post.side_effect = [first, *[RequestException("boom")] * 3] + + with ( + patch("parcllabs.services.properties.property_v2.time.sleep"), + warnings.catch_warnings(record=True) as caught, + ): + warnings.simplefilter("always") + property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=2) + + incomplete = [ + w for w in caught if w.category is parcllabs_warnings.ParclLabsIncompleteResultWarning + ] + truncation = [w for w in caught if w.category is parcllabs_warnings.ParclLabsTruncationWarning] + + assert len(incomplete) == 1 + assert "1 of an expected 2" in str(incomplete[0].message) + # Capping information is preserved in the incomplete message instead. + assert "108,295" in str(incomplete[0].message) + assert not truncation, "truncation warning must not contradict the incomplete warning" + + # The once-per-session budget must be intact for a later, legitimate truncation. + assert parcllabs_warnings._truncation_warned is False + + +@patch.object(PropertyV2Service, "_post") +def test_no_truncation_warning_when_no_limit_was_set( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + """A shortfall with no cap in play must not be blamed on `limit`. + + Regression: the early-return path gated on `retrieved < total_available`, so an + uncapped request whose server response ended pagination early (has_more=False + while more properties matched) warned "because `limit` capped the result" -- advice + the caller cannot act on, and it consumed the once-per-session budget. + """ + mock_post.return_value = _page(1, total_available=100, returned_count=50, has_more=False) + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + property_v2_service._fetch_post(params={"limit": 1000}, data={}, max_results=None) + + assert not [w for w in caught if w.category is parcllabs_warnings.ParclLabsTruncationWarning] + assert parcllabs_warnings._truncation_warned is False + + +@patch.object(PropertyV2Service, "_post") +def test_truncation_reports_actual_not_requested_count( + mock_post: Mock, property_v2_service: PropertyV2Service +) -> None: + """The truncation message must state what was returned, not what was asked for.""" + mock_post.return_value = _page(1, total_available=500, limit=1, offset=0, has_more=True) + + with pytest.warns(parcllabs_warnings.ParclLabsTruncationWarning) as caught: + property_v2_service._fetch_post(params={"limit": 1}, data={}, max_results=1) + + assert "Returned 1 of 500" in str(caught[0].message) + + +def test_incomplete_pages_surfaced_in_metadata(property_v2_service: PropertyV2Service) -> None: + results = [ + { + "metadata": {"results": {"returned_count": 1, "total_available": 2}}, + "_parcllabs": {"incomplete_pages": [1]}, + } + ] + assert property_v2_service._get_metadata(results)["incomplete_pages"] == [1] + + +def test_get_metadata_does_not_mutate_response(property_v2_service: PropertyV2Service) -> None: + """A shallow copy would rewrite returned_count on the caller's raw page.""" + results = [ + {"metadata": {"results": {"returned_count": 2, "total_available": 5}}}, + {"metadata": {"results": {"returned_count": 3, "total_available": 5}}}, + ] + + metadata = property_v2_service._get_metadata(results) + + assert metadata["results"]["returned_count"] == 5 + assert results[0]["metadata"]["results"]["returned_count"] == 2 + + +def test_integrity_check_warns_on_property_count_mismatch( + property_v2_service: PropertyV2Service, +) -> None: + # Two rows but only one distinct property, against a reported count of 2. + final_df = pd.DataFrame({"parcl_property_id": [1, 1]}) + metadata = {"results": {"returned_count": 2, "total_available": 2}} + + with pytest.warns(parcllabs_warnings.ParclLabsIncompleteResultWarning, match="integrity"): + property_v2_service._check_pagination_integrity(final_df, metadata) + + +def test_integrity_check_silent_when_counts_agree( + property_v2_service: PropertyV2Service, +) -> None: + final_df = pd.DataFrame({"parcl_property_id": [1, 1, 2]}) + metadata = {"results": {"returned_count": 2, "total_available": 2}} + + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + property_v2_service._check_pagination_integrity(final_df, metadata) + + assert not caught def test_as_pd_dataframe(property_v2_service: PropertyV2Service, mock_response: Mock) -> None: @@ -310,9 +572,12 @@ def test_retrieve( # check that the metadata is returned assert metadata == mock_response.json()["metadata"] - # check that the correct data was passed to _fetch_post + # `limit` is the page size here; `auto_paginate` must NOT leak into the query + # string (it previously did, and was asserted as correct). call_args = mock_fetch_post.call_args[1] - assert call_args["params"] == {"limit": 10, "auto_paginate": False} + assert call_args["params"] == {"limit": 10} + assert "auto_paginate" not in call_args["params"] + assert call_args["max_results"] == 10 data = call_args["data"] assert data["parcl_ids"] == [123]