feat(api): cohort filters can exclude or isolate analysis results

CHANGELOG.md

@@ -19,6 +19,16 @@ Refactors, CI, and formatting land in the git history, not here.
   statement through DuckDB's prepared-statement mechanism. A caller no longer has to quote or
   escape a value it did not author — an identifier taken from a URL, a name a user typed — and
   such a value can no longer change the shape of the query.
+- Cohort filters take `analysis_results`: `include` (default), `exclude` (original data only)
+  or `only` (analysis results only), as in idc-index's `download_from_selection`. It applies to
+  `cohort/counts`, `cohort/manifest`, `cohort/manifest.txt`, `citations`, `licenses` and the
+  matching MCP tools, and is echoed in `filters_applied`. A collection filter returns that
+  collection's analysis results too, and until now there was no way to drop them short of SQL
+  ([#43](https://github.com/ImagingDataCommons/IDC-REST-MCP/issues/43)).
+- The cohort download payload includes `idc download <analysis_result_id>` when the filter is only
+  `analysis_result_id` values, as it already did for `collection_id`.
+- `get_attribute_values` / `GET /v3/attributes/{attribute}/values` return `null_count`, the
+  number of series with no value, which `values` does not list.
 
 ### Changed
 

CLAUDE.md

@@ -40,6 +40,12 @@ These are the rules the design depends on. Full detail and walkthroughs are in
 7. **Errors:** raise an `IDCAPIError` subclass from services. REST maps it to
    `{status, code, message}`; the MCP `guard` decorator converts it to a clean `ToolError`.
    Never leak tracebacks.
+8. **The text manifest is an idc-index input format.** `POST /v3/cohort/manifest.txt` and
+   `get_cohort_urls` return exactly one `s3://<bucket>/<crdc_series_uuid>/*` URL per line and
+   nothing else, because `idc download` / `idc download-from-manifest` parse each line as a
+   single CSV column (an added field crashes them). Extra per-series fields belong in the JSON
+   `cohort/manifest` rows or in SQL, never on these lines. `tests/test_manifest_contract.py`
+   enforces it.
 
 Adding a capability touches five places (model → service → REST route → MCP tool → parity
 test). See the walkthrough in [dev/developer_guide.md](dev/developer_guide.md).

dev/developer_guide.md

@@ -118,6 +118,13 @@ tests/                   # pytest suite
 7. **Errors:** raise an [`IDCAPIError`](../src/idc_api/core/errors.py) subclass from services.
    REST maps it to `{status, code, message}`; the MCP `guard` decorator converts it to a clean
    `ToolError`. Never leak tracebacks.
+8. **The text manifest is an idc-index input format.** `POST /v3/cohort/manifest.txt` and
+   `get_cohort_urls` return exactly one `s3://<bucket>/<crdc_series_uuid>/*` URL per line and
+   nothing else. `idc download` and `idc download-from-manifest` read each line as a single CSV
+   column, so an added field (a series number, a licence) makes them fail with a pandas
+   `Length mismatch` error. Extra per-series fields belong in the JSON `cohort/manifest` rows or
+   in SQL. [test_manifest_contract.py](../tests/test_manifest_contract.py) checks the line format
+   and feeds the output through idc-index's own manifest parsing.
 
 ## Walkthrough: add a new capability
 

docs/user-guide.md

@@ -35,6 +35,12 @@ of an analysis result's series (and vice versa). Filter on whichever axis you ac
 (`analysis_results_index` lists each result's source collections in its plural `collections`
 field.)
 
+An analysis result's series carry the `collection_id` of the images they analyze, so a
+`collection_id` filter returns the collection's original images *and* any analysis results on
+them — in NLST, about two thirds of the series are analysis results (IDC v25). Original data has no
+`analysis_result_id` (it is NULL), so set the cohort filter's `analysis_results` to `exclude` for
+original data only, or `only` for analysis results only.
+
 The main queryable table is **`index`** — one row per **series**, the unit you filter, count,
 and download. IDC is large (~100+ TB total), so always check counts/size before downloading.
 
@@ -298,6 +304,9 @@ request/response example for each.
 curl -s 'localhost:8000/v3/attributes/Modality/values?limit=10'
 ```
 
+Series with no value for the attribute are not listed; `null_count` says how many there are.
+For `analysis_result_id` that is all of the original data.
+
 **Other read-only lookups (GET)** — no body, just the URL:
 
 ```bash
@@ -337,6 +346,18 @@ must be numbers; bounds on `StudyDate` / `SeriesDate` must be dates as `"YYYY-MM
 `"YYYYMMDD"` is accepted and normalized). Any other bound is rejected with a 400 rather than
 silently matching nothing.
 
+`analysis_results` keeps or drops the series that belong to analysis results: `include`
+(default), `exclude` (original data only) or `only` (analysis results only) — the same option
+`idc download-from-selection` takes. It applies on top of `terms` and `ranges` but is not a
+predicate on its own, so a manifest still needs at least one of those; combining `exclude` with an
+`analysis_result_id` term is a 400. NLST's original images only:
+
+```bash
+curl -s localhost:8000/v3/cohort/counts \
+  -H 'content-type: application/json' \
+  -d '{"filters": {"terms": {"collection_id": ["nlst"]}, "analysis_results": "exclude"}}'
+```
+
 **Get the full manifest as plain text** (for `idc download-from-manifest` / `s5cmd`):
 
 ```bash
@@ -466,10 +487,18 @@ the API server (the server never moves bytes; retrieval always means URLs/manife
 the same against the hosted or a local instance, and scales to whole collections. Two ways to
 do it:
 
-1. **Whole collection** — the simplest path; `cohort/manifest`'s download payload emits it for
-   you when your filter is a single `collection_id`:
+1. **Whole collection or analysis result** — the simplest path; `cohort/manifest`'s download
+   payload emits it for you when your filter is only `collection_id` values, or only
+   `analysis_result_id` values (an analysis result is downloaded across all the collections it
+   spans):
    ```bash
    idc download nlst --download-dir ./idc-data
+   idc download nlstseg --download-dir ./idc-data
+   ```
+   That fetches the collection's analysis results too. With `analysis_results` set to
+   `exclude` or `only`, the payload emits the split form instead:
+   ```bash
+   idc download-from-selection --collection-id nlst --analysis-results exclude --download-dir ./idc-data
    ```
 2. **A manifest** — save the `manifest.txt` output (or any SQL result's `series_aws_url`
    column) and feed it to the CLI:

src/idc_api/core/filters.py

@@ -99,13 +99,26 @@ def compile_filters(filters: CohortFilters) -> CompiledFilters:
             params.append(rng.lte)
         applied_ranges[attr] = rng
 
+    mode = filters.analysis_results
+    if mode == "exclude" and "analysis_result_id" in applied_terms:
+        # Always empty. Refused rather than answered with a zero that reads as "no such data".
+        raise InvalidQueryError(
+            "analysis_results='exclude' keeps original data only, so it cannot be combined with "
+            "an analysis_result_id term, which selects analysis results. Drop one of the two."
+        )
+    # Analysis result series are the ones with an analysis_result_id; original data has NULL.
+    if mode == "exclude":
+        clauses.append('"analysis_result_id" IS NULL')
+    elif mode == "only":
+        clauses.append('"analysis_result_id" IS NOT NULL')
+
     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),
+        applied=CohortFilters(terms=applied_terms, ranges=applied_ranges, analysis_results=mode),
         warnings=warnings,
     )
 
@@ -164,10 +177,17 @@ def require_filter(compiled: CompiledFilters, action: str) -> None:
     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.
+
+    ``analysis_results`` alone does not count: either side of that split is roughly half the
+    archive (about half a million series).
     """
     if compiled.applied.terms or compiled.applied.ranges:
         return
     dropped = [w for w in compiled.warnings if w != UNFILTERED_WARNING]
+    if compiled.applied.analysis_results != "include":
+        dropped.append(
+            "analysis_results on its own is not a predicate: either side of it is about half of IDC."
+        )
     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 "

src/idc_api/core/models.py

@@ -2,10 +2,13 @@
 
 from __future__ import annotations
 
-from typing import Any
+from typing import Any, Literal
 
 from pydantic import BaseModel, ConfigDict, Field
 
+# Same name and values as idc-index's download_from_selection(analysis_results=...).
+AnalysisResultsMode = Literal["include", "exclude", "only"]
+
 # --- discovery ----------------------------------------------------------------------------
 
 
@@ -95,6 +98,11 @@ class AttributeValues(BaseModel):
     attribute: str
     values: list[AttributeValue]
     truncated: bool = False
+    null_count: int = Field(
+        0,
+        description="Series with no value (NULL) for this attribute. They are not listed in "
+        "`values`, and a term filter cannot select them.",
+    )
     note: str = Field("", description="Semantic caveat about this attribute, when one applies.")
 
 
@@ -158,8 +166,9 @@ class NumericRange(BaseModel):
 
 class CohortFilters(BaseModel):
     """Structured filters over the main `index` table. ``terms`` does equality/IN (OR within
-    an attribute, AND across attributes); ``ranges`` does gte/lte. Discover valid attribute
-    names with ``list_attributes`` and valid values with ``get_attribute_values``."""
+    an attribute, AND across attributes); ``ranges`` does gte/lte; ``analysis_results`` keeps
+    or drops the series that belong to analysis results. Discover valid attribute 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
@@ -182,6 +191,15 @@ class CohortFilters(BaseModel):
         examples=[{"collection_id": ["nlst"], "Modality": ["CT"]}],
     )
     ranges: dict[str, NumericRange] = Field(default_factory=dict)
+    analysis_results: AnalysisResultsMode = Field(
+        "include",
+        description="Whether series that belong to analysis results (derived data: "
+        "segmentations, annotations, radiomics) are in the cohort: 'include' keeps them "
+        "(default), 'exclude' keeps original data only, 'only' keeps analysis results only. "
+        "Analysis result series carry the collection_id of the images they analyze, so a "
+        "collection_id filter alone returns both. Applied on top of terms and ranges; not a "
+        "predicate on its own, so it does not satisfy the at-least-one-predicate rule.",
+    )
 
 
 # Shared by every response built from a filter, so the same two field names mean the same thing

src/idc_api/core/schema.py

@@ -241,7 +241,16 @@ def table_schema(table: str) -> dict:
 
 FILTERABLE_ATTRIBUTES: list[dict] = [
     {"name": "collection_id", "kind": "term", "categorical": True},
-    {"name": "analysis_result_id", "kind": "term", "categorical": True},
+    {
+        "name": "analysis_result_id",
+        "kind": "term",
+        "categorical": True,
+        "note": (
+            "A term filter can only select analysis results by id. To keep original data only "
+            "(analysis_result_id NULL), or analysis results only, set the cohort filter's "
+            "analysis_results to 'exclude' or 'only'."
+        ),
+    },
     {"name": "PatientID", "kind": "term", "categorical": False},
     {"name": "StudyInstanceUID", "kind": "term", "categorical": False},
     {"name": "SeriesInstanceUID", "kind": "term", "categorical": False},

src/idc_api/core/services/discovery.py

@@ -154,9 +154,15 @@ def get_attribute_values(self, attribute: str, limit: int = 100) -> AttributeVal
         )
         truncated = len(res.rows) > limit
         rows = res.rows[:limit]
+        # The values above leave out NULLs; count them so that a mostly-NULL column (e.g.
+        # analysis_result_id, NULL for all original data) doesn't read as fully enumerated.
+        null_count = self.backend.query(
+            f'SELECT count(*) AS n FROM index WHERE "{attribute}" IS NULL'  # nosec B608
+        ).rows[0]["n"]
         return AttributeValues(
             attribute=attribute,
             values=[AttributeValue(value=r["value"], count=r["count"]) for r in rows],
             truncated=truncated,
+            null_count=null_count,
             note=schema.attribute_note(attribute),
         )

src/idc_api/core/services/manifest.py

@@ -4,6 +4,8 @@
 
 from __future__ import annotations
 
+import shlex
+
 from ..backend.base import QueryBackend
 from ..filters import compile_filters, require_filter
 from ..models import CohortFilters, DownloadInfo
@@ -36,7 +38,14 @@ def __init__(self, backend: QueryBackend, settings):
         self.settings = settings
 
     def _series_urls(self, where: str, params: list, limit: int) -> tuple[list[str], bool]:
-        """Return up to ``limit`` ``s3://`` series URLs plus a truncation flag."""
+        """Return up to ``limit`` ``s3://`` series URLs plus a truncation flag.
+
+        These URLs are the lines of the text manifest (``manifest_lines`` / ``manifest_text``),
+        which ``idc download`` / ``idc download-from-manifest`` read as one CSV column. Keep each
+        line a bare ``s3://<bucket>/<crdc_series_uuid>/*``: any added field crashes the CLI.
+        Extra per-series fields go in the JSON manifest rows (``SeriesManifestRow``). Enforced by
+        tests/test_manifest_contract.py.
+        """
         # `where` is compile_filters output: allow-listed columns, values bound below.
         rows = self.backend.query(
             f"SELECT series_aws_url FROM index WHERE {where} "  # nosec B608
@@ -74,18 +83,27 @@ def download_info(
         truncated = total_series > self.settings.manifest_hard_cap
 
         commands: list[str] = []
-        # Whole-collection selection -> the simplest idc command.
-        terms = filters.terms or {}
-        only_collection = (
-            list(terms.keys()) == ["collection_id"]
-            and not (filters.ranges or {})
-            and len(terms["collection_id"]) >= 1
-        )
-        if only_collection:
-            for cid in terms["collection_id"]:
-                commands.append(f"idc download {cid} --download-dir ./idc-data")
+        # A selection that is whole collections, or whole analysis results, has a one-line idc
+        # command (`idc download <value>` resolves either kind of id). Read from what was
+        # applied, so a predicate the compiler dropped doesn't hide or fake that shape.
+        terms = f.applied.terms
+        mode = f.applied.analysis_results
+        whole = next(iter(terms)) if len(terms) == 1 and not f.applied.ranges else None
+        if whole in ("collection_id", "analysis_result_id"):
+            for value in terms[whole]:
+                # Values come from the caller and land in a shell command; quote them.
+                value = shlex.quote(value)
+                if whole == "collection_id" and mode != "include":
+                    # `idc download` takes everything in the collection; this one can split it.
+                    commands.append(
+                        f"idc download-from-selection --collection-id {value} "
+                        f"--analysis-results {mode} --download-dir ./idc-data"
+                    )
+                else:
+                    commands.append(f"idc download {value} --download-dir ./idc-data")
         commands.append(
-            "# Or save the full manifest (see the manifest endpoint/tool), then:\n"
+            ("# Or save" if commands else "# Save")
+            + " the full manifest (see the manifest endpoint/tool), then:\n"
             "idc download-from-manifest idc_manifest.txt --download-dir ./idc-data"
         )