Fix top backlog items: FacetSort enum, include-field semantics, docs

CHANGELOG.md

@@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `FacetSort` is now exported from the `visor` package root.
+
+### Changed
+
+- `FacetSort` converted from a `Literal` type alias to a `str, Enum`
+  (`COUNT`, `COUNT_DESC`, `METRIC`, `METRIC_DESC`), matching the `SortOrder` pattern used by
+  `ListingsFilter`. Raw string values (e.g. `"-count"`) continue to validate through Pydantic's
+  lax coercion.
+- `FacetsFilter.sort` default changed from the string literal `"-count"` to `FacetSort.COUNT_DESC`.
+
+### Fixed
+
+- `FacetsFilter.to_params()` now serializes `sort` via `.value` (was relying on implicit
+  `str` coercion; behaviour is identical but now explicit and consistent with `ListingsFilter`).
+
 ## [0.1.1] - 2026-06-13
 
 ### Fixed

README.md

@@ -159,6 +159,40 @@ filter = ListingsFilter(
 
 All responses — `ListingsPage`, `ListingDetail`, `VinDetail`, etc. — are Pydantic v2 models. Access fields as attributes and use standard Pydantic methods (`.model_dump()`, `.model_json_schema()`, etc.) as needed.
 
+### Optional include fields
+
+Some fields are only populated when you request them via `include=`. The API uses three distinct states that the SDK preserves:
+
+| Value | Meaning |
+|---|---|
+| `None` | Section not requested — `include=` was not passed, or the API omitted the field |
+| `[]` | Section requested but no data exists |
+| `[...]` | Section requested and populated |
+
+Fields that follow this pattern:
+
+- `ListingDetail.price_history` — pass `include=["price_history"]` to `get_listing()`
+- `ListingSnapshot.price_history` — pass `include=["price_history"]` to `lookup_vin()`
+- `VehicleBuild.options` — pass `include=["options"]` to `get_listing()` or `lookup_vin()`
+
+```python
+listing = client.get_listing(listing_id, include=["price_history"])
+
+if listing.price_history is None:
+    print("price history was not requested")
+elif listing.price_history == []:
+    print("requested but no price changes recorded")
+else:
+    for entry in listing.price_history:
+        print(entry.date, entry.price)
+```
+
+`ListingSummary.price_history` and `ListingSummary.options` are also populated via
+`ListingsFilter(include=...)`, but they default to `[]` when the API omits the key.
+For summary listings, `[]` can mean either "not present in the response" or
+"requested and empty"; use detail/VIN lookups if you need the three-state
+`None` / `[]` / populated distinction.
+
 ## Error handling
 
 All methods raise typed exceptions from `visor.exceptions`. The SDK does not retry automatically — `RateLimitError.retry_after` gives you the hint to build your own retry logic.

src/visor/__init__.py

@@ -31,7 +31,13 @@
     DealersPage,
     DealerSummary,
 )
-from visor.models.facets import FacetBucket, FacetsData, FacetsFilter, FacetsResponse
+from visor.models.facets import (
+    FacetBucket,
+    FacetsData,
+    FacetsFilter,
+    FacetSort,
+    FacetsResponse,
+)
 from visor.models.listings import (
     ListingDetail,
     ListingsFilter,
@@ -79,6 +85,7 @@
     "FacetsData",
     "FacetsFilter",
     "FacetsResponse",
+    "FacetSort",
     # listings
     "ListingDetail",
     "ListingsFilter",

src/visor/_pagination.py

@@ -29,6 +29,7 @@ async def paginate_listings(
     *,
     max_pages: int | None = None,
 ) -> AsyncGenerator[ListingSummary, None]:
+    # Async generators cannot use `yield from`, so items are yielded one by one.
     if max_pages is not None and max_pages <= 0:
         return
     current = filter if filter is not None else ListingsFilter()
@@ -51,6 +52,7 @@ async def paginate_dealers(
     *,
     max_pages: int | None = None,
 ) -> AsyncGenerator[DealerSummary, None]:
+    # Async generators cannot use `yield from`, so items are yielded one by one.
     if max_pages is not None and max_pages <= 0:
         return
     current = filter if filter is not None else DealerFilter()

src/visor/models/facets.py

@@ -1,4 +1,4 @@
-from typing import Literal, TypeAlias
+from enum import Enum
 
 from pydantic import Field, field_validator, model_validator
 
@@ -37,7 +37,13 @@
     "days_on_market",
 }
 
-FacetSort: TypeAlias = Literal["count", "-count", "metric", "-metric"]
+
+class FacetSort(str, Enum):
+    COUNT = "count"
+    COUNT_DESC = "-count"
+    METRIC = "metric"
+    METRIC_DESC = "-metric"
+
 
 # Facets that return range histograms; all others are categorical (bucket lists).
 RANGE_FACET_NAMES = {"price", "msrp", "miles", "days_on_market"}
@@ -50,7 +56,7 @@ class FacetsFilter(ListingsFilterBase):
     facets: list[str]
     facet_value_limit: int | None = None
     metric: str | None = None
-    sort: FacetSort = "-count"
+    sort: FacetSort = FacetSort.COUNT_DESC
 
     @field_validator("facets")
     @classmethod
@@ -85,7 +91,7 @@ def to_params(self) -> dict[str, str]:
             params["facet_value_limit"] = str(self.facet_value_limit)
         if self.metric is not None:
             params["metric"] = self.metric
-        params["sort"] = self.sort
+        params["sort"] = self.sort.value
         return params
 
 

tests/integration/test_int_listings.py

@@ -95,3 +95,65 @@ def test_get_listing_with_price_history(
 def test_get_listing_with_options(client: VisorClient, sample_listing_id: str) -> None:
     listing = client.get_listing(sample_listing_id, include=["options"])
     assert isinstance(listing.vehicle.build.options, list)
+
+
+# ---------------------------------------------------------------------------
+# ListingSummary include-field raw-shape guards
+#
+# Background: ListingSummary.price_history and ListingSummary.options use
+# list[...] with default_factory=list.  That type does NOT accept null — if
+# the API ever sends null for these fields without include=, Pydantic will
+# raise a ValidationError at parse time.
+#
+# We verified via live probe (2026-06-17) that the API omits both fields
+# entirely from the raw JSON when include= is not passed.  Pydantic's
+# default_factory=list kicks in, so listing.price_history == [] means
+# "field was absent from response", not "requested and empty".
+#
+# These tests re-verify that invariant on every release-gate run.  If either
+# assertion fails it means the API started sending null (model type is wrong)
+# or [] (model type is technically fine but semantics become ambiguous —
+# revisit whether to make the field Optional so callers can distinguish
+# "not requested" from "requested but empty").
+# ---------------------------------------------------------------------------
+
+_SENTINEL = object()
+
+
+def _check_include_field_raw(client: VisorClient, field: str) -> None:
+    """Assert field is absent (not null, not []) in raw listings without include=."""
+    raw = client._transport.get("/listings", {"limit": "5"})
+    items: list[dict] = raw.get("data", [])
+    assert items, "No listings in response"
+    for item in items:
+        val = item.get(field, _SENTINEL)
+        assert val is _SENTINEL, (
+            f"ListingSummary.{field} unexpectedly present in raw response "
+            f"without include=: {val!r}. "
+            f"If null: change type to list[...] | None. "
+            f"If []: field is now always sent; consider Optional for caller clarity."
+        )
+
+
+@pytest.mark.integration
+@pytest.mark.release_gate
+def test_listing_summary_price_history_absent_without_include(
+    client: VisorClient,
+) -> None:
+    """price_history must be absent from raw JSON when include= is not passed.
+
+    Verified live 2026-06-17: API omits the key entirely; Pydantic default []
+    applies.  Failure here means the API changed shape — re-evaluate the model.
+    """
+    _check_include_field_raw(client, "price_history")
+
+
+@pytest.mark.integration
+@pytest.mark.release_gate
+def test_listing_summary_options_absent_without_include(client: VisorClient) -> None:
+    """options must be absent from raw JSON when include=options is not passed.
+
+    Verified live 2026-06-17: API omits the key entirely; Pydantic default []
+    applies.  Failure here means the API changed shape — re-evaluate the model.
+    """
+    _check_include_field_raw(client, "options")

tests/test_exports.py

@@ -45,6 +45,7 @@
     "FacetsResponse",
     "FacetsData",
     "FacetBucket",
+    "FacetSort",
     # shared models
     "Pagination",
     "DealerRef",

tests/test_models.py

@@ -20,6 +20,8 @@
 )
 from visor.models.facets import (
     FacetsData,
+    FacetsFilter,
+    FacetSort,
     FacetsResponse,
 )
 from visor.models.listings import (
@@ -162,6 +164,92 @@ def test_vehicle_build_options_null() -> None:
     assert build.options is None
 
 
+# ---------------------------------------------------------------------------
+# Include-field semantics: None / [] / [...]
+#
+# Fields backed by include= follow a three-way contract:
+#   None  — section not requested; API omitted or sent null
+#   []    — section requested but empty
+#   [...] — section requested and populated
+# ---------------------------------------------------------------------------
+
+
+def test_listing_detail_price_history_none_when_not_requested() -> None:
+    data = {
+        "id": "a",
+        "vin": "VIN1",
+        "status": "active",
+        "inventory_type": "new",
+        "dealer": DEALER_REF_DATA,
+        "vehicle": VEHICLE_RECORD_DATA,
+    }
+    ld = ListingDetail.model_validate(data)
+    assert ld.price_history is None
+
+
+def test_listing_detail_price_history_empty_list_when_requested_but_empty() -> None:
+    data = {
+        "id": "a",
+        "vin": "VIN1",
+        "status": "active",
+        "inventory_type": "new",
+        "dealer": DEALER_REF_DATA,
+        "vehicle": VEHICLE_RECORD_DATA,
+        "price_history": [],
+    }
+    ld = ListingDetail.model_validate(data)
+    assert ld.price_history == []
+
+
+def test_listing_detail_price_history_populated() -> None:
+    data = {
+        "id": "a",
+        "vin": "VIN1",
+        "status": "active",
+        "inventory_type": "new",
+        "dealer": DEALER_REF_DATA,
+        "vehicle": VEHICLE_RECORD_DATA,
+        "price_history": [{"date": "2026-01-01", "price": 35000}],
+    }
+    ld = ListingDetail.model_validate(data)
+    assert ld.price_history is not None
+    assert len(ld.price_history) == 1
+    assert ld.price_history[0].price == 35000
+
+
+def test_listing_snapshot_price_history_empty_list_when_requested_but_empty() -> None:
+    snap = ListingSnapshot.model_validate(
+        {
+            "id": "s1",
+            "inventory_type": "new",
+            "dealer": DEALER_REF_DATA,
+            "price_history": [],
+        }
+    )
+    assert snap.price_history == []
+
+
+def test_vehicle_build_options_empty_list_when_requested_but_empty() -> None:
+    build = VehicleBuild.model_validate(
+        {"year": 2026, "make": "Toyota", "model": "Camry", "options": []}
+    )
+    assert build.options == []
+
+
+def test_vehicle_build_options_populated() -> None:
+    build = VehicleBuild.model_validate(
+        {
+            "year": 2026,
+            "make": "Toyota",
+            "model": "Camry",
+            "options": [{"code": "X1", "name": "Sunroof", "msrp": 1200.0}],
+        }
+    )
+    assert build.options is not None
+    assert len(build.options) == 1
+    assert build.options[0].code == "X1"
+
+
 # ---------------------------------------------------------------------------
 # ListingsPage
 # ---------------------------------------------------------------------------
@@ -302,6 +390,41 @@ def test_facets_data_defaults() -> None:
     assert fd.stats == {}
 
 
+# ---------------------------------------------------------------------------
+# FacetSort enum
+# ---------------------------------------------------------------------------
+
+
+def test_facet_sort_default_is_count_desc() -> None:
+    f = FacetsFilter(facets=["make"])
+    assert f.sort is FacetSort.COUNT_DESC
+    assert f.sort.value == "-count"
+
+
+def test_facet_sort_explicit_enum_accepted() -> None:
+    f = FacetsFilter(facets=["make"], sort=FacetSort.COUNT)
+    assert f.sort is FacetSort.COUNT
+    assert f.to_params()["sort"] == "count"
+
+
+def test_facet_sort_raw_string_coerces() -> None:
+    # Pydantic coerces raw strings matching enum values in lax mode.
+    f = FacetsFilter(facets=["make"], sort="-count")  # type: ignore[arg-type]
+    assert f.sort is FacetSort.COUNT_DESC
+
+
+def test_facet_sort_all_wire_values() -> None:
+    expected = {
+        FacetSort.COUNT: "count",
+        FacetSort.COUNT_DESC: "-count",
+        FacetSort.METRIC: "metric",
+        FacetSort.METRIC_DESC: "-metric",
+    }
+    for member, wire in expected.items():
+        f = FacetsFilter(facets=["make"], sort=member)
+        assert f.to_params()["sort"] == wire
+
+
 # ---------------------------------------------------------------------------
 # DealerSummary / DealerDetail / DealersPage
 # ---------------------------------------------------------------------------
@@ -429,9 +552,6 @@ def test_dealer_filter_dealer_id_100_ok() -> None:
 # ---------------------------------------------------------------------------
 
 
-from visor.models.facets import FacetsFilter  # noqa: E402
-
-
 def test_facets_filter_value_limit_max_100() -> None:
     with pytest.raises(ValidationError, match="facet_value_limit maximum is 100"):
         FacetsFilter(facets=["make"], facet_value_limit=101)