fix(api)!: one filter body shape, no silently unfiltered results

CHANGELOG.md

@@ -13,8 +13,36 @@ Refactors, CI, and formatting land in the git history, not here.
 
 ## [Unreleased]
 
+### Changed
+
+- **Breaking (beta): `POST /v3/cohort/counts` and `POST /v3/licenses` now take the filter under
+  `filters`**, like every other filter endpoint: `{"filters": {"terms": …}}`. The old bare body
+  (`{"terms": …}`) returns `422` naming the fix. The shapes used to differ per endpoint, which is
+  what made the bug below possible.
+- **Breaking (beta): the series-enumerating surfaces require at least one filter predicate** —
+  `POST /v3/cohort/manifest`, `POST /v3/cohort/manifest.txt`, and the MCP `build_cohort` /
+  `get_cohort_urls`. Unfiltered they returned a download payload for the whole archive; they now
+  return `400`. The aggregate surfaces (`cohort/counts`, `licenses`) still answer an unfiltered
+  filter, with a warning.
+
+### Added
+
+- Responses built from a cohort filter now carry `filters_applied` (the predicates that reached
+  SQL) and `warnings` (predicates dropped, whether nothing was filtered, and — when a cohort
+  matches nothing only because of letter case — the casing that does exist): `POST /v3/cohort/counts`,
+  `POST /v3/licenses`, `POST /v3/citations`, the `counts` object in `POST /v3/cohort/manifest`,
+  and the MCP `build_cohort` / `get_licenses` / `get_citations` results.
+- A "Limits" section in the user guide: the per-request caps (SQL timeout and row ceiling,
+  manifest cap, page size, memory) and the fact that there is no per-caller rate limit or `429`.
+
 ### Fixed
 
+- A mis-shaped filter body no longer returns all of IDC at HTTP 200. Unrecognized keys in a
+  filter body (`{"term": …}`, a range bound misspelled `{"min": …}`) and malformed MCP filter
+  arguments are now errors rather than silently dropped predicates.
+- `POST /v3/citations` / `get_citations` resolve DOIs in batches via DataCite instead of one
+  request each — a cohort spanning every DOI in IDC took 237 serial round-trips, now 5. Any DOI
+  the batch doesn't cover still falls back to a per-DOI resolve, so citations stay complete.
 - The `/v3/viewer-url` OpenAPI examples (the values Swagger UI's "Try it out" pre-fills) used a StudyInstanceUID and SeriesInstanceUID that are not present in IDC, so running the example returned a `not_found` error instead of a viewer link. Both now use resolvable UIDs.
 
 ## [3.0.0b2] — 2026-07-14

docs/user-guide.md

@@ -114,6 +114,16 @@ from S3/GCS; see [§4](#4-getting-the-data)) — and **be a good citizen** — c
 > default and flags truncation, so a peek stays cheap; large unfiltered results mostly just waste
 > the agent's context.
 >
+> **Knowing your filter was applied.** Every filtered response echoes `filters_applied` — the
+> predicates the server actually used — alongside a `warnings` list. Empty `filters_applied`
+> means *nothing* was filtered and the counts cover the entire archive; `warnings` says so in
+> words. A mis-shaped filter body is a `422`, never a silently unfiltered `200`; a predicate that
+> can't constrain anything (an empty value list, a range with neither bound) is dropped and named
+> in `warnings`; and the surfaces that *enumerate* series (`cohort/manifest`,
+> `cohort/manifest.txt`, `build_cohort`, `get_cohort_urls`) refuse an unfiltered request outright.
+> Values match **case-sensitively**, and a cohort that matches nothing tells you when case is the
+> only reason — `Modality: ["mr"]` comes back with zero counts *and* a warning that `MR` exists.
+>
 > **Knowing you got it all.** Size-capped responses include a `truncated` boolean:
 > `truncated: false` means the result is complete; `true` means raise the limit and re-check (or
 > narrow/aggregate). `run_sql`'s `max_rows` is clamped to a server ceiling (`SQL_MAX_ROWS_CAP`),
@@ -239,6 +249,27 @@ uv run idc-api          # http://127.0.0.1:8000  — Swagger UI at /v3/docs
 | `POST /v3/citations` | Citations for a cohort |
 | `POST /v3/licenses` | License breakdown for a cohort |
 
+### Filter bodies: one shape
+
+**Every** filter-taking endpoint — `cohort/counts`, `cohort/manifest`, `cohort/manifest.txt`,
+`citations`, `licenses` — takes the filter object under `filters`, alongside that endpoint's own
+options:
+
+```json
+{"filters": {"terms": {"collection_id": ["nlst"]}, "ranges": {"instanceCount": {"gte": 100}}}}
+```
+
+Sending the filter bare (`{"terms": …}` at the top level) is a `422` naming the fix, and so is any
+unrecognized key inside a filter body (`{"term": …}`, a range bound misspelled `{"min": …}`) —
+an ignored key is a dropped predicate, and a dropped predicate silently widens the selection to
+the whole archive. Each response reports what was actually applied; see *Knowing your filter was
+applied* in [§1](#recommended-workflow).
+
+An empty filter is answered by the aggregate endpoints (`cohort/counts`, `licenses`) with a
+warning — "how big is IDC" is a legitimate question. The endpoints that *enumerate series*
+(`cohort/manifest`, `cohort/manifest.txt`) refuse it with a `400`: an unfiltered manifest is a
+download payload for 100+ TB.
+
 ### Worked examples
 
 Every endpoint below is also documented interactively at `/v3/docs` (Swagger UI), with a filled-in
@@ -266,15 +297,15 @@ curl -s localhost:8000/v3/clinical/tables/nlst_canc                     # clinic
 curl -s 'localhost:8000/v3/clinical/tables/nlst_canc/rows?max_rows=100' # clinical rows (capped)
 ```
 
-**Cheap size check** — the `counts` body is the filter object directly:
+**Cheap size check** — the filter goes under `filters`, as it does on every filter endpoint:
 
 ```bash
 curl -s localhost:8000/v3/cohort/counts \
   -H 'content-type: application/json' \
-  -d '{"terms": {"Modality": ["MR"], "BodyPartExamined": ["BREAST"]}}'
+  -d '{"filters": {"terms": {"Modality": ["MR"], "BodyPartExamined": ["BREAST"]}}}'
 ```
 
-**Build a cohort** — `manifest` wraps the filter in a request with paging:
+**Build a cohort** — `manifest` adds paging to the same filter body:
 
 ```bash
 curl -s localhost:8000/v3/cohort/manifest \
@@ -302,15 +333,15 @@ curl -s localhost:8000/v3/sql \
   -d '{"sql": "SELECT Modality, count(*) n FROM index GROUP BY 1 ORDER BY n DESC", "max_rows": 20}'
 ```
 
-**License check** — like `counts`, the body is the filter object directly:
+**License check:**
 
 ```bash
 curl -s localhost:8000/v3/licenses \
   -H 'content-type: application/json' \
-  -d '{"terms": {"collection_id": ["nlst"]}}'
+  -d '{"filters": {"terms": {"collection_id": ["nlst"]}}}'
 ```
 
-**Citations for a cohort** — body wraps the filter, like `manifest`:
+**Citations for a cohort:**
 
 ```bash
 curl -s localhost:8000/v3/citations \
@@ -447,6 +478,28 @@ Values interpolated into curated (non-SQL) queries are always passed as bound pa
 ([OWASP](https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.html)).
 See [`dev/api_v3_plan.md`](../dev/api_v3_plan.md) for the full threat model.
 
+### Limits
+
+The public deployment is unauthenticated and needs no API key, and there is **no per-caller rate
+limit or quota** — nothing to budget against, and no `429`. What *is* bounded is each individual
+request:
+
+| Limit | Default | Applies to |
+|---|---|---|
+| SQL statement timeout | 30 s | `POST /v3/sql`, `run_sql` |
+| SQL rows returned | 5 000, hard ceiling 10 000 | `POST /v3/sql`, `run_sql` (`truncated` flags it) |
+| Manifest series enumerated | 100 000 | `cohort/manifest*`, `get_cohort_urls` |
+| Page size | 5 000 | `cohort/manifest` |
+| Query memory | 4 GB | every query |
+| At least one filter predicate | required | the series-enumerating endpoints (see above) |
+
+All of these are configurable per deployment ([§7](#7-configuration)). Beyond them, a burst is
+absorbed by Cloud Run autoscaling and surfaces as slower responses or a `503` — back off and
+retry rather than treating it as a permanent failure. Please keep automated use reasonable; if
+you need sustained heavy access, query the [`idc-index`](https://github.com/ImagingDataCommons/idc-index)
+Parquet or IDC's BigQuery tables directly instead of driving this API hard, and note that data
+transfer never goes through this server anyway ([§4](#4-getting-the-data)).
+
 ---
 
 ## 6. Licenses & citations

src/idc_api/core/filters.py

@@ -3,21 +3,49 @@
 Attribute *names* are validated against a fixed allow-list (they can't be parameterized, so
 we whitelist + double-quote them); attribute *values* are always passed as bound parameters
 — the OWASP-recommended primary defense for the inputs we control.
+
+Compilation also reports what it *dropped*. A filter that compiles to no predicates selects
+every series in IDC, which is a plausible-looking answer rather than an error, so the compiler
+returns the predicates it actually applied plus caller-facing warnings and every response built
+from a filter echoes them (see ``CohortCounts.filters_applied`` / ``.warnings``).
 """
 
 from __future__ import annotations
 
-from typing import Any
+from typing import Any, NamedTuple
 
 from . import schema
 from .errors import InvalidQueryError
-from .models import CohortFilters
+from .models import CohortFilters, NumericRange
+
+UNFILTERED_WARNING = (
+    "No filter predicates were applied, so this result describes the ENTIRE IDC archive, not a "
+    "cohort. If you meant to filter, the filter did not arrive in a usable shape — compare "
+    "`filters_applied` with what you sent, then re-send it."
+)
+
+
+class CompiledFilters(NamedTuple):
+    """The compiled filter, plus what the caller needs to see to trust it.
 
+    ``where`` is ``TRUE`` when nothing survived compilation — hence ``applied`` and ``warnings``,
+    which let a caller tell "no data matched" apart from "my filter was silently dropped".
+    """
 
-def compile_filters(filters: CohortFilters) -> tuple[str, list[Any]]:
-    """Return ``(where_sql, params)``. ``where_sql`` is ``TRUE`` when no filters are given."""
+    where: str
+    params: list[Any]
+    applied: CohortFilters  # only the predicates that made it into `where`
+    warnings: list[str]
+
+
+def compile_filters(filters: CohortFilters) -> CompiledFilters:
+    """Compile ``filters`` into a WHERE clause with bound params, the effective filter, and
+    warnings for anything dropped along the way."""
     clauses: list[str] = []
     params: list[Any] = []
+    applied_terms: dict[str, list[str]] = {}
+    applied_ranges: dict[str, NumericRange] = {}
+    warnings: list[str] = []
 
     for attr, values in (filters.terms or {}).items():
         if attr not in schema.TERM_ATTRIBUTES:
@@ -27,23 +55,61 @@ def compile_filters(filters: CohortFilters) -> tuple[str, list[Any]]:
             )
         values = [v for v in (values or []) if v is not None]
         if not values:
+            warnings.append(
+                f"Term filter {attr!r} was ignored because its value list is empty; it "
+                "constrains nothing."
+            )
             continue
         placeholders = ", ".join(["?"] * len(values))
         clauses.append(f'"{attr}" IN ({placeholders})')
         params.extend(values)
+        applied_terms[attr] = values
 
     for attr, rng in (filters.ranges or {}).items():
         if attr not in schema.RANGE_ATTRIBUTES:
             raise InvalidQueryError(
                 f"Unknown or non-range filter attribute: {attr!r}. "
                 "Use list_attributes to see valid attributes."
             )
+        if rng.gte is None and rng.lte is None:
+            warnings.append(
+                f"Range filter {attr!r} was ignored because neither 'gte' nor 'lte' was set; it "
+                "constrains nothing."
+            )
+            continue
         if rng.gte is not None:
             clauses.append(f'"{attr}" >= ?')
             params.append(rng.gte)
         if rng.lte is not None:
             clauses.append(f'"{attr}" <= ?')
             params.append(rng.lte)
+        applied_ranges[attr] = rng
+
+    if not clauses:
+        warnings.append(UNFILTERED_WARNING)
+
+    return CompiledFilters(
+        where=" AND ".join(clauses) if clauses else "TRUE",
+        params=params,
+        applied=CohortFilters(terms=applied_terms, ranges=applied_ranges),
+        warnings=warnings,
+    )
+
+
+def require_filter(compiled: CompiledFilters, action: str) -> None:
+    """Refuse ``action`` when no predicate survived compilation.
 
-    where = " AND ".join(clauses) if clauses else "TRUE"
-    return where, params
+    The aggregate surfaces (counts, licenses) answer an unfiltered filter honestly — "how big is
+    IDC" is a real question, and it costs one query. The surfaces that enumerate *per series* do
+    not: an unfiltered manifest is a download payload for the whole archive, which no caller
+    means to ask for. Those raise instead, with the dropped-predicate warnings attached so the
+    caller can see *why* their filter came out empty.
+    """
+    if compiled.applied.terms or compiled.applied.ranges:
+        return
+    dropped = [w for w in compiled.warnings if w != UNFILTERED_WARNING]
+    raise InvalidQueryError(
+        f"At least one filter predicate is required to {action}: unfiltered, that is every series "
+        "in IDC (100+ TB). Use the stats surface for archive-wide totals, or cohort counts to "
+        "size a filter first." + ("".join(f" {w}" for w in dropped))
+    )

src/idc_api/core/models.py

@@ -148,6 +148,10 @@ class ClinicalTableList(BaseModel):
 
 
 class NumericRange(BaseModel):
+    # Unknown keys are rejected rather than ignored: {"min": 5} would otherwise compile to a
+    # range that constrains nothing (see CohortFilters).
+    model_config = ConfigDict(extra="forbid")
+
     gte: float | str | None = None
     lte: float | str | None = None
 
@@ -158,14 +162,19 @@ class CohortFilters(BaseModel):
     names with ``list_attributes`` and valid values with ``get_attribute_values``."""
 
     model_config = ConfigDict(
+        # An unrecognized key is a hard error, never silently ignored. Ignoring one meant a
+        # mis-shaped body dropped every predicate and returned all of IDC at HTTP 200 — a wrong
+        # answer that looks like a right one. Failing loud beats forward-compatibility for a
+        # request object this small and this consequential.
+        extra="forbid",
         json_schema_extra={
             "examples": [
                 {
                     "terms": {"collection_id": ["nlst"], "Modality": ["CT"]},
                     "ranges": {},
                 }
             ]
-        }
+        },
     )
 
     terms: dict[str, list[str]] = Field(
@@ -175,12 +184,26 @@ class CohortFilters(BaseModel):
     ranges: dict[str, NumericRange] = Field(default_factory=dict)
 
 
+# Shared by every response built from a filter, so the same two field names mean the same thing
+# everywhere a cohort is described.
+_APPLIED_DESC = (
+    "The filter predicates the server actually applied. Compare it with what you sent: if it is "
+    "empty, nothing was filtered and these numbers cover all of IDC."
+)
+_WARNINGS_DESC = (
+    "Non-fatal problems with the request — above all, filter predicates that were dropped. "
+    "Empty when the request was honored in full."
+)
+
+
 class CohortCounts(BaseModel):
     patients: int
     studies: int
     series: int
     instances: int
     size_TB: float
+    filters_applied: CohortFilters = Field(default_factory=CohortFilters, description=_APPLIED_DESC)
+    warnings: list[str] = Field(default_factory=list, description=_WARNINGS_DESC)
 
 
 class SeriesManifestRow(BaseModel):
@@ -252,7 +275,11 @@ class CitationsResult(BaseModel):
         "In addition to the per-dataset citations, always acknowledge IDC itself by citing "
         "Fedorov et al., https://doi.org/10.1148/rg.230180 (see idc_acknowledgment)."
     )
+    filters_applied: CohortFilters = Field(default_factory=CohortFilters, description=_APPLIED_DESC)
+    warnings: list[str] = Field(default_factory=list, description=_WARNINGS_DESC)
 
 
 class LicensesResult(BaseModel):
     licenses: list[LicenseItem]
+    filters_applied: CohortFilters = Field(default_factory=CohortFilters, description=_APPLIED_DESC)
+    warnings: list[str] = Field(default_factory=list, description=_WARNINGS_DESC)