fix: address copilot review + windows webhook-receiver test (PR #25)

CI fix
- `tests/test_webhooks_receiver.py::test_receiver_404s_on_unknown_path`
  was failing on windows-latest with `httpx.ReadError WinError 10053`.
  The receiver's do_POST returned a 404 before draining the request body
  on unknown paths. On Linux/macOS that's absorbed silently; on Windows
  the abrupt socket close abort the still-pending request. do_POST now
  drains the body first, then branches on path.

Behavior changes
- `_post` / `_patch` now raise `TangoValidationError` when both
  `json_data=` and `json=` are passed instead of silently preferring
  one. Single-kwarg usage is unchanged.
- `create_webhook_endpoint(name=...)` is now **required** (was a
  `DeprecationWarning` in 0.7.0 — never publicly released). The server
  enforces unique(user, name), so omitting `name` returned a 400
  anyway. Raising client-side gives a clear error and avoids the
  wasted round-trip. Updated `test_create_endpoint_without_name_warns`
  → `..._raises`.

Docs/example fixes
- `get_psc_metrics` / `get_naics_metrics` / `get_entity_metrics`
  docstrings — `period_grouping` values are "month"/"quarter"/"year"
  (path-segment values), not "monthly"/"quarterly".
- `docs/API_REFERENCE.md#get_agency` — consistent `get_agency("GSA")`
  example + note that the parameter accepts CGAC / FPDS / short code /
  abbreviation / canonical name.
- `README.md` Quick Start — `agency.name` instead of `agency['name']`;
  `get_agency()` returns an `Agency` dataclass.
- `scripts/smoke_api_parity.py` — `list_business_types(limit=1)` is
  now wrapped in the `run(...)` helper so a failure there records FAIL
  instead of crashing the whole script.

CHANGELOG updated to reflect the behavior changes.

Closes the open Copilot comments on PR #25.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: infinityplusone

CHANGELOG.md

@@ -23,7 +23,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 - `ordering` parameter on `list_forecasts`, `list_grants`, `list_subawards`, `list_gsa_elibrary_contracts`, and `list_opportunities`. Prefix with `-` for descending. Closes a parity gap with the API surface (these endpoints all accept `?ordering=` server-side).
-- `create_webhook_endpoint` accepts `name=` (keyword-only). Required by the Tango API since multi-endpoint support landed; omitting it now emits a `DeprecationWarning` and will become an error in a future major version.
+- `create_webhook_endpoint` accepts `name=` (keyword-only) and now **requires** it. The Tango API enforces unique `(user, name)` on endpoints; omitting `name` returns a 400 server-side, so the SDK raises `TangoValidationError` client-side instead of round-tripping. (0.7.0 — never publicly released — emitted a `DeprecationWarning` instead.)
 - `update_webhook_endpoint` accepts `name=` for renaming an endpoint.
 - Webhook alerts (filter subscriptions): `list_webhook_alerts`, `get_webhook_alert`, `create_webhook_alert`, `update_webhook_alert`, `delete_webhook_alert` — the canonical write surface over `/api/webhooks/alerts/`. New `WebhookAlert` dataclass exported from the top-level package.
 - `resolve(name, target_type, ...)` — POST `/api/resolve/` to rank entity / organization candidates from a free-text name. Returns `ResolveResult` with `ResolveCandidate` entries (both exported).
@@ -48,7 +48,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `ordering` kwarg from `list_notices` and `list_protests`. The notices and protests viewsets reject every `?ordering=` value at runtime (tango#2254); the kwarg silently sent unsupported values. Other five list methods retain `ordering`.
 
 ### Fixed
-- `TangoClient._post()` and `_patch()` now accept both `json_data=` (positional) and `json=` (keyword) for backward compatibility. Internal callers and docs examples that use `json=` no longer fail with `TypeError`.
+- `TangoClient._post()` and `_patch()` accept both `json_data=` (positional) and `json=` (keyword) for backward compatibility. Internal callers and docs examples that use `json=` no longer fail with `TypeError`. Passing **both** now raises `TangoValidationError` rather than silently preferring one — that ambiguity would hide caller bugs.
+- `get_psc_metrics` / `get_naics_metrics` / `get_entity_metrics` docstrings — `period_grouping` values are `"month"` / `"quarter"` / `"year"` (the path-segment values the API accepts), not `"monthly"` / `"quarterly"`.
+- `docs/API_REFERENCE.md#get_agency` — example uses `client.get_agency("GSA")` consistently and notes the parameter accepts CGAC / FPDS / short code / abbreviation / canonical name.
+- `README.md` Quick Start — `get_agency()` returns an `Agency` dataclass, so the example uses attribute access (`agency.name`) instead of `agency['name']` which would `TypeError`.
+- `scripts/smoke_api_parity.py` — `list_business_types(limit=1)` is now wrapped in the `run(...)` helper so a failure on that call records FAIL instead of aborting the smoke run.
 - `tango webhooks endpoints create` CLI now accepts and requires `--name` (passed through to `create_webhook_endpoint(name=...)`). Previously the option was absent, meaning the CLI could never set a custom endpoint name and every call would 400 server-side (the server enforces `unique(user, name)`).
 - `WebhookAlert.query_type` and `WebhookAlert.filters` tightened from `Optional` to non-optional (`str` and `dict[str, Any]` respectively). Legacy nullable rows were purged by the tango#2275 migration; the server model and serializer guarantee non-null values for all current data. `WebhookAlert.status` narrowed from `str` to `Literal["active", "paused"]` — the server serializer produces exactly those two values.
 - **Shape validator agrees with server on `naics(...)` / `psc(...)` expansions.** The client-side `ShapeParser.validate()` previously rejected the canonical `shape=naics(code,description)` form (which the server has always accepted) and also rejected the alias `shape=naics_code(code,description)`. The parser now mirrors the server's `_EXPAND_ALIASES` (introduced in Tango PR makegov/tango#2259) and rewrites `naics_code(...)` / `psc_code(...)` to their canonical `naics(...)` / `psc(...)` form at parse time. Bare scalar leaves (`shape=naics_code` / `shape=psc_code`) are left untouched and still return the raw column value, matching the server. Schemas for `Contract`, `Forecast`, `Opportunity`, `Notice`, and `Vehicle` gained explicit `naics` / `psc` expand entries backed by the existing `CodeDescription` nested model. Fixes makegov/tango#2266.

README.md

@@ -39,7 +39,7 @@ print(f"Found {agencies.count} agencies")
 
 # Get specific agency
 agency = client.get_agency("GSA")
-print(f"Agency: {agency['name']}")
+print(f"Agency: {agency.name}")
 
 # Search contracts
 contracts = client.list_contracts(

docs/API_REFERENCE.md

@@ -86,17 +86,17 @@ for agency in agencies.results:
 Get a specific agency by code.
 
 ```python
-agency = client.get_agency(code="GSA")
+agency = client.get_agency("GSA")
 ```
 
 **Parameters:**
-- `code` (str): Agency code (e.g., "GSA", "DOD", "HHS")
+- `code` (str): Agency identifier. Accepts CGAC ("097"), FPDS code ("4712"), short code ("GSA"), abbreviation, or canonical name. See [Federal agency hierarchy](https://docs.makegov.com/api-reference/concepts/federal-agency-hierarchy/) for code semantics.
 
 **Returns:** `Agency` dataclass with agency details
 
 **Example:**
 ```python
-gsa = client.get_agency("4712")
+gsa = client.get_agency("GSA")
 print(f"Name: {gsa.name}")
 print(f"Abbreviation: {gsa.abbreviation or 'N/A'}")
 if gsa.department:

scripts/smoke_api_parity.py

@@ -96,9 +96,9 @@ def run(label: str, fn: Callable[[], Any], *, skip_if_blank: str | None = None)
     lambda: client.get_naics_metrics(NAICS_CODE, 12, "month"),
 )
 
-bt_pager = client.list_business_types(limit=1)
+bt_pager = run("list_business_types(limit=1)", lambda: client.list_business_types(limit=1))
 bt_code = ""
-if bt_pager.results:
+if bt_pager and bt_pager.results:
     bt_code = str(bt_pager.results[0].code or "")
 run(
     f"get_business_type({bt_code!r})",

tango/client.py

@@ -216,7 +216,13 @@ def _post(
 
         Accepts either ``json_data`` (positional) or ``json=`` (keyword) for
         backward compatibility with internal callers and docs examples.
+        Passing both raises ``TangoValidationError`` rather than silently
+        picking one — that ambiguity would hide caller bugs.
         """
+        if json_data is not None and json is not None:
+            raise TangoValidationError(
+                "_post: pass `json_data` or `json`, not both."
+            )
         body = json_data if json_data is not None else json
         if body is None:
             body = {}
@@ -233,7 +239,13 @@ def _patch(
 
         Accepts either ``json_data`` (positional) or ``json=`` (keyword) for
         backward compatibility with internal callers and docs examples.
+        Passing both raises ``TangoValidationError`` rather than silently
+        picking one — that ambiguity would hide caller bugs.
         """
+        if json_data is not None and json is not None:
+            raise TangoValidationError(
+                "_patch: pass `json_data` or `json`, not both."
+            )
         body = json_data if json_data is not None else json
         if body is None:
             body = {}
@@ -2502,18 +2514,18 @@ def create_webhook_endpoint(
         if not callback_url:
             raise TangoValidationError("Webhook callback_url is required")
         if name is None:
-            warnings.warn(
-                "create_webhook_endpoint() called without name=; the Tango "
-                "API requires `name` and this call will fail server-side. "
-                "Pass name='your-endpoint-name'. This will become a required "
-                "argument in a future major version.",
-                DeprecationWarning,
-                stacklevel=2,
+            # `name` was a deprecation warning in 0.7.0 (never publicly
+            # released). 1.0.0 makes it a hard error since the server
+            # enforces unique(user, name) and the warn-then-400 path
+            # was a worse DX than just raising client-side.
+            raise TangoValidationError(
+                "create_webhook_endpoint(): `name=` is required. "
+                "The Tango API enforces unique(user, name) on endpoints; "
+                "omitting it would return a 400 server-side. "
+                "Pass name='your-endpoint-name'."
             )
 
-        body: dict[str, Any] = {"callback_url": callback_url, "is_active": is_active}
-        if name is not None:
-            body["name"] = name
+        body: dict[str, Any] = {"callback_url": callback_url, "is_active": is_active, "name": name}
 
         data = self._post("/api/webhooks/endpoints/", body)
         return WebhookEndpoint(
@@ -2901,7 +2913,8 @@ def get_psc_metrics(self, code: str, months: int, period_grouping: str) -> dict[
         Args:
             code: PSC code.
             months: Window size in months (e.g. 6, 12, 24, 36).
-            period_grouping: ``monthly``, ``quarterly``, etc.
+            period_grouping: ``"month"``, ``"quarter"``, or ``"year"`` (the
+                values the API accepts on the path segment).
         """
         if not code:
             raise TangoValidationError("PSC code is required")

tango/webhooks/receiver.py

@@ -113,11 +113,18 @@ def log_message(self, format: str, *args: Any) -> None:  # noqa: A002
                 return
 
             def do_POST(self) -> None:  # noqa: N802 (stdlib API)
+                # Always drain the request body before responding, even on
+                # error paths. Windows surfaces an undrained body as
+                # `WinError 10053` (connection aborted by the host) when
+                # the server closes the socket mid-request; Linux/macOS
+                # absorb it silently. Reading first keeps the response
+                # cycle clean across platforms.
+                length = int(self.headers.get("Content-Length", "0") or 0)
+                body = self.rfile.read(length) if length > 0 else b""
+
                 if self.path != receiver.path:
                     self._write_json(404, {"ok": False, "error": "not_found"})
                     return
-                length = int(self.headers.get("Content-Length", "0") or 0)
-                body = self.rfile.read(length) if length > 0 else b""
                 signature = self.headers.get(SIGNATURE_HEADER)
                 verified = bool(receiver.secret) and verify_signature(
                     body, receiver.secret, signature

tests/test_api_parity.py

@@ -253,20 +253,17 @@ def test_create_endpoint_passes_name(self, mock_request: Mock) -> None:
         assert body["name"] == "primary"
         assert body["callback_url"] == "https://x/"
 
-    def test_create_endpoint_without_name_warns(self, mock_request: Mock) -> None:
-        mock_request.return_value = _mock_response(
-            {
-                "id": "ep-1",
-                "name": "",
-                "callback_url": "https://x/",
-                "is_active": True,
-                "created_at": "2026-01-01",
-                "updated_at": "2026-01-01",
-            }
-        )
-        client = TangoClient(api_key="x", base_url="https://t.example")
-        with pytest.warns(DeprecationWarning, match="name"):
+    def test_create_endpoint_without_name_raises(self, mock_request: Mock) -> None:
+        # 1.0.0 turned the 0.7.0 DeprecationWarning into a hard error: the
+        # server enforces unique(user, name), so omitting name would 400
+        # anyway. Raising client-side gives a better error message and
+        # avoids the wasted round-trip.
+        mock_request.return_value = _mock_response({})
+        client = TangoClient(api_key="x", base_url="https://t.example")
+        with pytest.raises(TangoValidationError, match="name"):
             client.create_webhook_endpoint("https://x/")
+        # And the request never went out.
+        mock_request.assert_not_called()
 
     def test_update_endpoint_passes_name(self, mock_request: Mock) -> None:
         mock_request.return_value = _mock_response(