From 0d9e9b6127b3fae3aaf383eadbe46da16aeb539d Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Tue, 29 Sep 2026 15:42:44 -0400 Subject: [PATCH 1/6] feat: release history via get_release_changes (#40) Add GET /v3/releases/changes and the MCP tool get_release_changes, which diff IDC release N against N-1 (series added/revised/removed, new/updated/ removed collections, analysis results) from index + prior_versions_index. Improve discoverability of version history: get_idc_version description, a whats_new MCP prompt, an INSTRUCTIONS line and idc://guide section, descriptions for prior_versions_index, and notable_columns in list_tables. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 14 +++ docs/user-guide.md | 40 +++++- src/idc_api/core/context.py | 2 + src/idc_api/core/models.py | 62 ++++++++++ src/idc_api/core/schema.py | 50 +++++++- src/idc_api/core/services/__init__.py | 2 + src/idc_api/core/services/query.py | 2 + src/idc_api/core/services/releases.py | 167 ++++++++++++++++++++++++++ src/idc_api/mcp/server.py | 58 ++++++++- src/idc_api/rest/app.py | 19 +++ tests/test_releases.py | 115 ++++++++++++++++++ 11 files changed, 525 insertions(+), 6 deletions(-) create mode 100644 src/idc_api/core/services/releases.py create mode 100644 tests/test_releases.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fa8994..dedf8c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,15 @@ Refactors, CI, and formatting land in the git history, not here. `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. +- **Release history: `GET /v3/releases/changes?version=N` and the MCP tool + `get_release_changes(version)`** — what changed in an IDC release versus the previous one: + series added / revised / removed (with size in TB and patients affected), collections that are + new / updated / removed, and analysis results that gained series. Any past release can be + described, computed from the served release's `index` + `prior_versions_index`; `version` + defaults to the served release. +- MCP prompt **`whats_new`** (optional `version`) — a ready-made "summarize this release" request. +- `GET /v3/tables` / `list_tables`: each table now carries **`notable_columns`**, a few columns + worth knowing before writing SQL (e.g. `series_init_idc_version` on `index`). ### Changed @@ -39,6 +48,11 @@ Refactors, CI, and formatting land in the git history, not here. `origin_patient`, `ImageOrientationPatient` and `slice_spacing_mm`. `GET /v3/version` and `get_idc_version` report `idc_index_data_version` `25.0.4`; collection, series, and size totals are unchanged. +- `prior_versions_index` is now documented: a table description and descriptions for + `min_idc_version`, `max_idc_version`, `crdc_series_uuid`, and `series_size_MB` (upstream ships + them empty). +- The `get_idc_version` tool description no longer implies the server can only speak to one + release; it points at `get_release_changes` and the version-history columns. ### Fixed diff --git a/docs/user-guide.md b/docs/user-guide.md index 3cc4b5c..4ae9f64 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -60,7 +60,7 @@ understanding, because picking the right one makes everything else easy: | Surface | Answers | REST | MCP tools | |---|---|---|---| -| **Discovery** | "What exists? What can I filter on?" | `GET /v3/version`, `/v3/stats`, `/v3/collections`, `/v3/collections/{id}`, `/v3/analysis_results`, `/v3/attributes`, `/v3/attributes/{attr}/values` | `get_idc_version`, `get_stats`, `list_collections`, `get_collection`, `list_analysis_results`, `list_attributes`, `get_attribute_values` | +| **Discovery** | "What exists? What can I filter on? What changed in a release?" | `GET /v3/version`, `/v3/releases/changes`, `/v3/stats`, `/v3/collections`, `/v3/collections/{id}`, `/v3/analysis_results`, `/v3/attributes`, `/v3/attributes/{attr}/values` | `get_idc_version`, `get_release_changes`, `get_stats`, `list_collections`, `get_collection`, `list_analysis_results`, `list_attributes`, `get_attribute_values` | | **Cohort** | "How big is *my* selection, and what's in it?" | `POST /v3/cohort/counts`, `POST /v3/cohort/manifest` | `build_cohort` | | **Retrieval** | "Give me the download links" | `POST /v3/cohort/manifest.txt` | `get_cohort_urls` | | **SQL** | "Run my custom query" + schema | `GET /v3/tables`, `/v3/tables/{table}`, `POST /v3/sql` | `list_tables`, `get_table_schema`, `run_sql` | @@ -154,7 +154,7 @@ per-collection, keyed by `collection_id`). | `index` | one row per **series** — the main table | | `collections_index` | one row per collection (curated metadata) | | `analysis_results_index` | one row per analysis result | -| `version_metadata_index` / `prior_versions_index` | IDC release versions / removed series | +| `version_metadata_index` / `prior_versions_index` | IDC release dates / superseded or removed series versions (`min_idc_version`–`max_idc_version`) — see [Release history](#release-history) | | `seg_index`, `ann_index`, `ann_group_index`, `rtstruct_index` | segmentations / annotations / RT structures: **what was segmented** (`SegmentedPropertyType_CodeMeanings` — `BodyPartExamined` reflects the source acquisition, not this) and the **reference** to the image series they derive from (`segmented_SeriesInstanceUID` / `referenced_SeriesInstanceUID`) | | `ct_index`, `mr_index`, `pt_index` | per-modality acquisition parameters (slice thickness, kVp, TE/TR, injected dose…) | | `sm_index`, `sm_instance_index` | slide-microscopy (pathology) series / instance metadata | @@ -238,6 +238,34 @@ WHERE i.collection_id = 'nlst' AND i.Modality = 'CT' > `IDC_API_INCLUDE_INDICES=all` includes it; the clinical tools return a clear "not included" > error otherwise). +### Release history + +The server serves **one** IDC data release (see `GET /v3/version` / `get_idc_version`), but that +release carries the history of every earlier one, so "what's new in v24" or "what changed since +v20" are answerable without external release notes: + +- On `index`, `series_init_idc_version` is the release a series first appeared in, and + `series_revised_idc_version` the release its current content dates from. +- `prior_versions_index` holds every **superseded or removed version** of a series — one row per + old `crdc_series_uuid`, valid from `min_idc_version` through `max_idc_version`. If its + `SeriesInstanceUID` is still in `index` the series was revised; otherwise it was removed. +- `version_metadata_index` dates each release. + +`GET /v3/releases/changes?version=N` / `get_release_changes(version=N)` does the diff of release +N against N-1 for you: series **added / revised / removed** (with TB and patients affected), +collections that are **new / updated / removed**, and the analysis results that gained series. +Omit `version` for the served release. For per-series detail, query the columns above with SQL — +e.g. the series of a collection that changed since v20: + +```sql +SELECT SeriesInstanceUID, series_init_idc_version, series_revised_idc_version +FROM index +WHERE collection_id = 'nlst' AND series_revised_idc_version > 20 +``` + +The diff tells you *what* changed, not *why*; for the narrative, see the +[IDC release notes](https://learn.canceridc.dev/data/data-release-notes). + --- ## 2. Using the REST API @@ -253,13 +281,14 @@ uv run idc-api # http://127.0.0.1:8000 — Swagger UI at /v3/docs | Method & path | Purpose | |---|---| | `GET /v3/version` | IDC data release served (e.g. `v25`) + pinned index version, **and** this server's own software version (`api_version`, plus `build` if the deploy stamped one) | +| `GET /v3/releases/changes?version=` | What changed in an IDC release vs. the previous one: series added / revised / removed, new / updated / removed collections, analysis results (default: the served release) | | `GET /v3/stats` | Headline totals (collections, patients, studies, series, size_TB) | | `GET /v3/collections` | List collections (datasets) | | `GET /v3/collections/{id}` | Collection detail: counts, modalities, license breakdown | | `GET /v3/analysis_results` | Derived datasets (segmentations/annotations) | | `GET /v3/attributes` | Filterable attributes (name, type, term/range, categorical) | | `GET /v3/attributes/{attr}/values?limit=` | Distinct values + counts for an attribute, plus a `note` caveat when one applies (e.g. `BodyPartExamined` ≠ segmented anatomy) | -| `GET /v3/tables` | Tables available to SQL | +| `GET /v3/tables` | Tables available to SQL, each with a few `notable_columns` | | `GET /v3/tables/{table}` | Column schema for a table | | `GET /v3/clinical/tables?collection_id=` | Per-collection clinical tables (optionally one collection) | | `GET /v3/clinical/tables/{table}` | Clinical table columns + human-readable labels | @@ -311,6 +340,8 @@ For `analysis_result_id` that is all of the original data. ```bash curl -s localhost:8000/v3/version # data release + this server's build +curl -s localhost:8000/v3/releases/changes # what's new in the served release +curl -s 'localhost:8000/v3/releases/changes?version=23' # …or in any earlier one curl -s localhost:8000/v3/stats # headline totals curl -s localhost:8000/v3/collections # list datasets curl -s localhost:8000/v3/collections/nlst # one collection's detail @@ -409,6 +440,7 @@ uv run idc-mcp --http --host 0.0.0.0 --port 8080 # hosted/shared - **Discovery:** `get_idc_version`, `get_stats`, `list_collections`, `get_collection`, `list_analysis_results`, `list_attributes`, `get_attribute_values` +- **Release history:** `get_release_changes` - **Schema (for SQL):** `list_tables`, `get_table_schema` - **Clinical data:** `list_clinical_tables`, `get_clinical_table_schema`, `get_clinical_table` - **Cohort / query:** `build_cohort`, `run_sql` @@ -416,6 +448,8 @@ uv run idc-mcp --http --host 0.0.0.0 --port 8080 # hosted/shared `get_citations`, `get_licenses` - **Resources:** `idc://guide` (data model + recommended workflow), `idc://tables`, `idc://schema/{table}` +- **Prompts:** `whats_new` (optional `version`) — a ready-made "summarize what's new in this + release" request for clients that show MCP prompts Tool descriptions are prescriptive about *when* to call each one, and the server ships an `idc://guide` resource with the same conceptual model as this document — so a capable agent can diff --git a/src/idc_api/core/context.py b/src/idc_api/core/context.py index ee8fa54..13f4838 100644 --- a/src/idc_api/core/context.py +++ b/src/idc_api/core/context.py @@ -13,6 +13,7 @@ LicenseService, ManifestService, QueryService, + ReleaseService, ViewerService, ) @@ -22,6 +23,7 @@ def __init__(self, settings: Settings | None = None): self.settings = settings or get_settings() self.backend = DuckDBBackend(self.settings) self.discovery = DiscoveryService(self.backend) + self.releases = ReleaseService(self.backend) self.cohort = CohortService(self.backend, self.settings) self.manifest = ManifestService(self.backend, self.settings) self.query = QueryService(self.backend, self.settings) diff --git a/src/idc_api/core/models.py b/src/idc_api/core/models.py index d475b26..13f498b 100644 --- a/src/idc_api/core/models.py +++ b/src/idc_api/core/models.py @@ -39,6 +39,63 @@ class Stats(BaseModel): size_TB: float +class CollectionChange(BaseModel): + collection_id: str + status: str = Field( + ..., + description="'new' (no series in the previous release), 'removed' (no series in this " + "release), or 'updated' (present in both, with series added/revised/removed).", + ) + series_added: int + series_revised: int + series_removed: int + size_TB_added: float + size_TB_revised: float + size_TB_removed: float + patients_affected: int = Field( + ..., description="Distinct patients with at least one added, revised, or removed series." + ) + + +class AnalysisResultChange(BaseModel): + analysis_result_id: str + is_new: bool = Field(..., description="True if this analysis result first appeared here.") + series_added: int + + +class ReleaseChanges(BaseModel): + """What changed in one IDC release relative to the one before it.""" + + idc_version: str = Field(..., description="The release described, e.g. 'v24'.") + release_date: str | None = None + previous_version: str | None = Field(None, description="The release compared against.") + previous_release_date: str | None = None + current_version: str = Field(..., description="The release this server serves.") + series_added: int + series_revised: int + series_removed: int + size_TB_added: float + size_TB_revised: float + size_TB_removed: float + patients_affected: int + new_collections: list[str] = Field( + default_factory=list, description="Collections that first appeared in this release." + ) + removed_collections: list[str] = Field( + default_factory=list, description="Collections with no series left in this release." + ) + collections: list[CollectionChange] = Field( + default_factory=list, description="Per-collection change, largest additions first." + ) + analysis_results: list[AnalysisResultChange] = Field( + default_factory=list, + description="Analysis results that gained series in this release. Counted from series " + "still present in the current release, so for an older release it omits series that were " + "later removed.", + ) + note: str = "" + + class CollectionSummary(BaseModel): collection_id: str collection_name: str | None = None @@ -125,6 +182,11 @@ class TableInfo(BaseModel): name: str description: str = "" column_count: int + notable_columns: list[str] = Field( + default_factory=list, + description="A few columns worth knowing about before writing SQL (not the full list — " + "get_table_schema has every column with its description).", + ) class TableList(BaseModel): diff --git a/src/idc_api/core/schema.py b/src/idc_api/core/schema.py index ffab3f2..09003b2 100644 --- a/src/idc_api/core/schema.py +++ b/src/idc_api/core/schema.py @@ -191,6 +191,50 @@ def _column_type(c: dict, struct_types: dict[str, str] | None = None) -> str: "(join to index on dicom_patient_id = index.PatientID). Use this table's column_label and " "value mappings to interpret their often-cryptic coded columns." ), + # Upstream ships this table with no description at all. + "prior_versions_index": ( + "Series versions IDC served in an earlier release but no longer serves: one row per " + "superseded or removed version of a series (identified by crdc_series_uuid), valid from " + "min_idc_version through max_idc_version. A SeriesInstanceUID that is also in `index` was " + "revised (its current version starts at index.series_revised_idc_version); one that is " + "not was removed. Together with `index` this reconstructs the content of any past release " + "— get_release_changes does that diff for you." + ), +} + +# Fill-ins for upstream column descriptions that are empty. Applied only where upstream has no +# text, so an upstream description takes over automatically once it lands. +COLUMN_DESCRIPTION_FILLINS: dict[str, dict[str, str]] = { + "prior_versions_index": { + "crdc_series_uuid": "Identifier of this specific version of the series (changes on " + "every revision); never equal to a crdc_series_uuid in `index`.", + "min_idc_version": "First IDC release (integer) that served this version of the series.", + "max_idc_version": "Last IDC release (integer) that served this version of the series; " + "the next release revised or removed it.", + "series_size_MB": "Size of this version of the series, in MB.", + }, +} + +# A handful of columns per table surfaced by list_tables, so a caller learns they exist without +# having to call get_table_schema first (a caller that guesses the obvious columns right never +# does). Not a full listing; names absent from a table's schema are dropped. +NOTABLE_COLUMNS: dict[str, list[str]] = { + "index": [ + "collection_id", + "analysis_result_id", + "PatientID", + "SeriesInstanceUID", + "Modality", + "BodyPartExamined", + "SeriesDescription", + "license_short_name", + "series_size_MB", + "series_init_idc_version", + "series_revised_idc_version", + ], + "version_metadata_index": ["idc_version", "version_timestamp"], + "prior_versions_index": ["SeriesInstanceUID", "min_idc_version", "max_idc_version"], + "seg_index": ["segmented_SeriesInstanceUID", "SegmentedPropertyType_CodeMeanings"], } @@ -198,7 +242,8 @@ def _column_type(c: dict, struct_types: dict[str, str] | None = None) -> str: def table_schema(table: str) -> dict: """Return ``{name, description, columns:[{name,type,description}]}`` for a table, sourced from the idc-index schema JSON shipped in INDEX_METADATA (table descriptions may be - repointed inward via ``TABLE_DESCRIPTION_OVERRIDES``).""" + repointed inward via ``TABLE_DESCRIPTION_OVERRIDES``; empty column descriptions are filled + from ``COLUMN_DESCRIPTION_FILLINS``).""" meta = idc_index_data.INDEX_METADATA[metadata_key(table)] schema = meta.get("schema", {}) or {} cols = schema.get("columns", []) @@ -206,11 +251,12 @@ def table_schema(table: str) -> dict: struct_types = ( _parquet_column_types(table) if any(c.get("type") == "RECORD" for c in cols) else {} ) + fillins = COLUMN_DESCRIPTION_FILLINS.get(table, {}) columns = [ { "name": c["name"], "type": _column_type(c, struct_types), - "description": c.get("description", "") or "", + "description": c.get("description", "") or fillins.get(c["name"], ""), } for c in cols ] diff --git a/src/idc_api/core/services/__init__.py b/src/idc_api/core/services/__init__.py index 69643e1..1785462 100644 --- a/src/idc_api/core/services/__init__.py +++ b/src/idc_api/core/services/__init__.py @@ -11,6 +11,7 @@ from .licenses import LicenseService from .manifest import ManifestService from .query import QueryService +from .releases import ReleaseService from .viewer import ViewerService __all__ = [ @@ -21,5 +22,6 @@ "LicenseService", "ManifestService", "QueryService", + "ReleaseService", "ViewerService", ] diff --git a/src/idc_api/core/services/query.py b/src/idc_api/core/services/query.py index 1260f6e..de73a15 100644 --- a/src/idc_api/core/services/query.py +++ b/src/idc_api/core/services/query.py @@ -17,11 +17,13 @@ def list_tables(self) -> TableList: tables = [] for name in self.backend.list_tables(): sch = schema.table_schema(name) + names = {c["name"] for c in sch["columns"]} tables.append( TableInfo( name=name, description=sch["description"], column_count=len(sch["columns"]), + notable_columns=[c for c in schema.NOTABLE_COLUMNS.get(name, []) if c in names], ) ) return TableList(tables=tables) diff --git a/src/idc_api/core/services/releases.py b/src/idc_api/core/services/releases.py new file mode 100644 index 0000000..e20c714 --- /dev/null +++ b/src/idc_api/core/services/releases.py @@ -0,0 +1,167 @@ +"""Release history: what changed in a given IDC data release. + +Computed entirely from the current release's bundled tables. Every *series version* IDC has +ever served is either the current row in ``index`` (valid from ``series_revised_idc_version`` +to now) or a row in ``prior_versions_index`` (a superseded or removed version, valid from +``min_idc_version`` to ``max_idc_version``). A series is in release N if one of its versions +spans N, so comparing release N with N-1 gives added / revised / removed series exactly. +""" + +from __future__ import annotations + +from ..backend.base import QueryBackend +from ..errors import InvalidQueryError +from ..models import AnalysisResultChange, CollectionChange, ReleaseChanges + +_MB_PER_TB = 1_000_000 + +# Params: current, N, N, N-1, N-1, N. +_DIFF_SQL = """ +WITH v AS ( + SELECT SeriesInstanceUID, collection_id, PatientID, series_size_MB, + series_revised_idc_version AS lo, ? AS hi + FROM index + UNION ALL + SELECT SeriesInstanceUID, collection_id, PatientID, series_size_MB, + min_idc_version AS lo, max_idc_version AS hi + FROM prior_versions_index +), +cur AS (SELECT * FROM v WHERE lo <= ? AND hi >= ?), +prev AS (SELECT * FROM v WHERE lo <= ? AND hi >= ?), +s AS ( + SELECT COALESCE(c.collection_id, p.collection_id) AS collection_id, + COALESCE(c.PatientID, p.PatientID) AS PatientID, + c.SeriesInstanceUID IS NOT NULL AS in_cur, + p.SeriesInstanceUID IS NOT NULL AS in_prev, + CASE WHEN p.SeriesInstanceUID IS NULL THEN 'added' + WHEN c.SeriesInstanceUID IS NULL THEN 'removed' + WHEN c.lo = ? THEN 'revised' END AS change, + c.series_size_MB AS cur_mb, + p.series_size_MB AS prev_mb + FROM cur c FULL OUTER JOIN prev p ON c.SeriesInstanceUID = p.SeriesInstanceUID +) +SELECT GROUPING(collection_id) AS is_total, + collection_id, + count(*) FILTER (WHERE in_prev) AS prev_series, + count(*) FILTER (WHERE in_cur) AS cur_series, + count(*) FILTER (WHERE change = 'added') AS series_added, + count(*) FILTER (WHERE change = 'revised') AS series_revised, + count(*) FILTER (WHERE change = 'removed') AS series_removed, + COALESCE(sum(cur_mb) FILTER (WHERE change = 'added'), 0) AS added_mb, + COALESCE(sum(cur_mb) FILTER (WHERE change = 'revised'), 0) AS revised_mb, + COALESCE(sum(prev_mb) FILTER (WHERE change = 'removed'), 0) AS removed_mb, + count(DISTINCT PatientID) FILTER (WHERE change IS NOT NULL) AS patients_affected +FROM s +GROUP BY GROUPING SETS ((collection_id), ()) +HAVING count(*) FILTER (WHERE change IS NOT NULL) > 0 OR GROUPING(collection_id) = 1 +""" + +_NOTE = ( + "Computed from the current release: series in `index` plus superseded/removed series " + "versions in `prior_versions_index`. 'revised' means the series is in both releases but its " + "content changed (new crdc_series_uuid). For per-series detail, query those tables with " + "run_sql (series_init_idc_version / series_revised_idc_version on `index`; min_idc_version / " + "max_idc_version on `prior_versions_index`)." +) + + +def _tb(mb: float) -> float: + return round(mb / _MB_PER_TB, 3) + + +def _parse_version(version: int | str) -> int: + s = str(version).strip().lower().removeprefix("v") + if not s.isdigit(): + raise InvalidQueryError(f"Invalid IDC version: {version!r}. Use an integer such as 24.") + return int(s) + + +class ReleaseService: + def __init__(self, backend: QueryBackend): + self.backend = backend + + def _release_dates(self) -> dict[int, str | None]: + rows = self.backend.query( + "SELECT idc_version, version_timestamp FROM version_metadata_index" + ).rows + return { + int(r["idc_version"]): (str(r["version_timestamp"]) if r["version_timestamp"] else None) + for r in rows + } + + def release_changes(self, version: int | str | None = None) -> ReleaseChanges: + dates = self._release_dates() + current = max(dates) + n = current if version is None else _parse_version(version) + if n not in dates: + raise InvalidQueryError( + f"Unknown IDC version: {version!r}. Releases available: v{min(dates)}–v{current}." + ) + + rows = self.backend.query(_DIFF_SQL, [current, n, n, n - 1, n - 1, n]).rows + total = next(r for r in rows if r["is_total"]) + per_coll = [r for r in rows if not r["is_total"]] + + def status(r: dict) -> str: + if r["prev_series"] == 0: + return "new" + if r["cur_series"] == 0: + return "removed" + return "updated" + + collections = sorted( + ( + CollectionChange( + collection_id=r["collection_id"], + status=status(r), + series_added=r["series_added"], + series_revised=r["series_revised"], + series_removed=r["series_removed"], + size_TB_added=_tb(r["added_mb"]), + size_TB_revised=_tb(r["revised_mb"]), + size_TB_removed=_tb(r["removed_mb"]), + patients_affected=r["patients_affected"], + ) + for r in per_coll + ), + key=lambda c: (-c.size_TB_added, -c.series_added, c.collection_id), + ) + + analysis = self.backend.query( + "SELECT analysis_result_id, " + "count(*) FILTER (WHERE series_init_idc_version = ?) AS series_added, " + "min(series_init_idc_version) = ? AS is_new " + "FROM index WHERE analysis_result_id IS NOT NULL " + "GROUP BY 1 HAVING series_added > 0 ORDER BY series_added DESC, 1", + [n, n], + ).rows + + prev = n - 1 if (n - 1) in dates else None + return ReleaseChanges( + idc_version=f"v{n}", + release_date=dates[n], + previous_version=f"v{prev}" if prev is not None else None, + previous_release_date=dates.get(prev) if prev is not None else None, + current_version=f"v{current}", + series_added=total["series_added"], + series_revised=total["series_revised"], + series_removed=total["series_removed"], + size_TB_added=_tb(total["added_mb"]), + size_TB_revised=_tb(total["revised_mb"]), + size_TB_removed=_tb(total["removed_mb"]), + patients_affected=total["patients_affected"], + new_collections=sorted(c.collection_id for c in collections if c.status == "new"), + removed_collections=sorted( + c.collection_id for c in collections if c.status == "removed" + ), + collections=collections, + analysis_results=[ + AnalysisResultChange( + analysis_result_id=r["analysis_result_id"], + is_new=bool(r["is_new"]), + series_added=r["series_added"], + ) + for r in analysis + ], + note=_NOTE, + ) diff --git a/src/idc_api/mcp/server.py b/src/idc_api/mcp/server.py index f163d4f..aff9070 100644 --- a/src/idc_api/mcp/server.py +++ b/src/idc_api/mcp/server.py @@ -64,6 +64,8 @@ buckets, no server involved. 4. Trust `filters_applied`, not your intent: surface any `warnings` a result carries, and treat an empty `filters_applied` as the whole archive rather than a cohort. +5. For "what's new / what changed in release N", call get_release_changes — release history is + queryable even though one release is served. Cite with get_citations (per-dataset citations plus the IDC paper to acknowledge IDC itself); respect get_licenses (CC-BY vs CC-BY-NC). See `idc://guide` for the data model, the full tool list, and join examples.""" @@ -246,7 +248,10 @@ def get_idc_version() -> dict: """Return the IDC data release served (e.g. 'v25') and pinned idc-index version, plus this server's own software version (`api_version`, and `build` if the deploy stamped one). Call this to confirm which IDC version your answers are based on — and which build of the server - produced them.""" + produced them. The server serves one release but is NOT limited to it: every series records + the release it first appeared in and was last revised in, and superseded/removed series are + kept in `prior_versions_index`, so past releases and what changed between them are + queryable — use get_release_changes for "what's new in vN".""" return ctx.discovery.version().model_dump(mode="json") @@ -258,6 +263,21 @@ def get_stats() -> dict: return ctx.discovery.stats().model_dump(mode="json") +@mcp.tool() +@guard +def get_release_changes(version: int | None = None) -> dict: + """What's new in an IDC data release — what changed since the previous version. Returns the + series added, revised, and removed (with size in TB and patients affected), the collections + that were newly added, updated, or removed, and the analysis results that gained series. + `version` is the release number (e.g. 24 for v24); omit it for the release this server + serves. Call this whenever the user asks what is new, added, changed, updated, or removed in + IDC, in a given release, or since a version — do not conclude that history is unavailable + because the server serves a single release. For per-series detail beyond this summary, use + run_sql on `index` (series_init_idc_version / series_revised_idc_version) and + `prior_versions_index` (min_idc_version / max_idc_version).""" + return ctx.releases.release_changes(version).model_dump(mode="json") + + @mcp.tool() @guard def list_collections() -> list[dict]: @@ -513,6 +533,29 @@ def get_licenses( return ctx.licenses.get_licenses(f).model_dump(mode="json") +# --- prompts ------------------------------------------------------------------------------ + + +@mcp.prompt() +def whats_new(version: str = "") -> str: + """Summarize what is new in an IDC data release (the latest one by default).""" + n = version.strip().lower().removeprefix("v") + target, call = ( + (f"IDC release v{n}", f"get_release_changes(version={n})") + if n + else ( + "the latest IDC release", + "get_release_changes()", + ) + ) + return ( + f"Summarize what changed in {target}. Call {call}, then report: the release date; the " + "headline totals (series added / revised / removed, TB added, patients affected); the new " + "collections, each with a one-line description from get_collection; notable updates or " + "removals in existing collections; and any new analysis results." + ) + + # --- resources ---------------------------------------------------------------------------- _GUIDE = """\ @@ -531,6 +574,8 @@ def get_licenses( - *Discovery* (`get_stats`, `list_collections`, `get_collection`, `list_analysis_results`, `list_attributes`, `get_attribute_values`) — what exists, and the *vocabulary* (attribute names + valid values) you filter on. +- *Release history* (`get_idc_version`, `get_release_changes`) — which release is served, and + what each release added, revised, or removed (see *Release history* below). - *Cohort* (`build_cohort`) — turn a chosen combination of that vocabulary into distinct counts + a sample of series + a download payload. - *Retrieval* (`get_cohort_urls`) — the download half: public URLs for direct S3/GCS transfer. @@ -623,6 +668,17 @@ def get_licenses( `index JOIN clinical.nlst_canc ON index.PatientID = clinical.nlst_canc.dicom_patient_id` filtered on the relevant staging column. Use `get_clinical_table` to read a whole small table. These tables are present only when `clinical_index` is included in the build. + +**Release history.** The server serves one IDC release, but that release carries the history of +all earlier ones. On `index`, `series_init_idc_version` is the release a series first appeared in +and `series_revised_idc_version` the release its current content dates from. +`prior_versions_index` holds every superseded or removed *version* of a series (one row per old +`crdc_series_uuid`, valid from `min_idc_version` through `max_idc_version`); a SeriesInstanceUID +also in `index` was revised, one that isn't was removed. `version_metadata_index` dates each +release. `get_release_changes(version=N)` diffs release N against N-1 from these tables — series +added / revised / removed, new and removed collections, per-collection size — so start there for +"what's new in vN"; drop to `run_sql` on those columns for per-series detail (e.g. "which of my +cohort's series changed since v20": `series_revised_idc_version > 20`). """ diff --git a/src/idc_api/rest/app.py b/src/idc_api/rest/app.py index fd8f329..d1b46d4 100644 --- a/src/idc_api/rest/app.py +++ b/src/idc_api/rest/app.py @@ -32,6 +32,7 @@ CollectionSummary, LicensesResult, ManifestResponse, + ReleaseChanges, SqlResult, Stats, TableList, @@ -440,6 +441,24 @@ def stats(): patients, studies, series, and instances, plus the total size in TB.""" return C().discovery.stats() + @app.get( + f"{API_PREFIX}/releases/changes", + response_model=ReleaseChanges, + tags=["discovery"], + summary="What changed in a release", + ) + def release_changes( + version: int | None = Query( + None, ge=1, description="IDC release number, e.g. 24; omit for the served release." + ), + ): + """What's new in an IDC data release relative to the previous one: series added, + revised, and removed (with size in TB and patients affected), collections that were + newly added, updated, or removed, and analysis results that gained series. Computed from + the served release's own tables (`index` plus `prior_versions_index`), so any past + release can be described, not just the latest.""" + return C().releases.release_changes(version) + @app.get( f"{API_PREFIX}/collections", response_model=list[CollectionSummary], diff --git a/tests/test_releases.py b/tests/test_releases.py new file mode 100644 index 0000000..7f887b7 --- /dev/null +++ b/tests/test_releases.py @@ -0,0 +1,115 @@ +"""Release history: get_release_changes diffs release N against N-1, and the version-history +tables/columns are discoverable from the tool surface (issue #40).""" + +from __future__ import annotations + +import pytest + +from idc_api.core import schema +from idc_api.core.errors import InvalidQueryError +from idc_api.mcp.server import mcp + + +def _current(ctx) -> int: + return int( + ctx.backend.query("SELECT max(idc_version) v FROM version_metadata_index").rows[0]["v"] + ) + + +def test_latest_release_is_default_and_self_consistent(ctx): + cur = _current(ctx) + rc = ctx.releases.release_changes() + assert rc.idc_version == rc.current_version == f"v{cur}" + assert rc.previous_version == f"v{cur - 1}" + assert rc.release_date and rc.previous_release_date + # Totals are the sum of the per-collection rows. + assert rc.series_added == sum(c.series_added for c in rc.collections) + assert rc.series_revised == sum(c.series_revised for c in rc.collections) + assert rc.series_removed == sum(c.series_removed for c in rc.collections) + assert rc.series_added > 0 + # In the served release, a series whose current content dates from this release was either + # added (absent before) or revised (present before) — nothing else. + n = ctx.backend.query( + "SELECT count(*) n FROM index WHERE series_revised_idc_version = ?", [cur] + ).rows[0]["n"] + assert rc.series_added + rc.series_revised == n + revised = ctx.backend.query( + "SELECT count(*) n FROM index WHERE series_init_idc_version < ? " + "AND series_revised_idc_version = ?", + [cur, cur], + ).rows[0]["n"] + assert rc.series_revised == revised + by_id = {c.collection_id: c for c in rc.collections} + for cid in rc.new_collections: + c = by_id[cid] + assert c.status == "new" and c.series_added > 0 and c.series_removed == 0 + + +def test_first_release_has_no_predecessor(ctx): + rc = ctx.releases.release_changes(1) + assert rc.idc_version == "v1" and rc.previous_version is None + assert rc.series_revised == rc.series_removed == 0 + assert rc.series_added > 0 + assert all(c.status == "new" for c in rc.collections) + + +def test_version_accepts_v_prefix(ctx): + cur = _current(ctx) + assert ctx.releases.release_changes(f"v{cur}") == ctx.releases.release_changes(cur) + + +@pytest.mark.parametrize("bad", [0, 9999, "latest", "v"]) +def test_unknown_version_is_a_clean_error(ctx, bad): + with pytest.raises(InvalidQueryError): + ctx.releases.release_changes(bad) + + +async def test_release_changes_parity(ctx, client, parse_mcp): + v = _current(ctx) - 1 + core = ctx.releases.release_changes(v).model_dump(mode="json") + rest = client.get("/v3/releases/changes", params={"version": v}).json() + mcp_out = parse_mcp(await mcp.call_tool("get_release_changes", {"version": v})) + assert core == rest == mcp_out + + +def test_rest_unknown_version_is_400(client): + r = client.get("/v3/releases/changes", params={"version": 9999}) + assert r.status_code == 400 + + +# --- discoverability ------------------------------------------------------------------------ + + +async def test_version_tool_points_at_release_history(): + tools = {t.name: t for t in await mcp.list_tools()} + assert "get_release_changes" in tools + assert "get_release_changes" in tools["get_idc_version"].description + desc = " ".join(tools["get_release_changes"].description.split()).lower() + for word in ("new", "changed", "added", "removed", "since"): + assert word in desc + + +async def test_whats_new_prompt(): + prompts = {p.name for p in await mcp.list_prompts()} + assert "whats_new" in prompts + msg = await mcp.get_prompt("whats_new", {"version": "v24"}) + assert "get_release_changes(version=24)" in msg.messages[0].content.text + + +def test_prior_versions_index_is_documented(): + sch = schema.table_schema("prior_versions_index") + assert sch["description"] + cols = {c["name"]: c["description"] for c in sch["columns"]} + assert cols["min_idc_version"] and cols["max_idc_version"] + + +def test_notable_columns_exist_in_schema(): + # list_tables drops unknown names silently, so catch curation drift here. + for table, cols in schema.NOTABLE_COLUMNS.items(): + names = {c["name"] for c in schema.table_schema(table)["columns"]} + assert set(cols) <= names, (table, set(cols) - names) + + +def test_list_tables_surfaces_version_columns(ctx): + tables = {t.name: t for t in ctx.query.list_tables().tables} + assert "series_init_idc_version" in tables["index"].notable_columns From 77fad01e34514502c9efbf225569a2520540e0a1 Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Mon, 19 Oct 2026 19:54:05 -0400 Subject: [PATCH 2/6] fix(releases): correct get_release_changes for past releases Address the review of #52. The served release was right; earlier ones were not. - Revised means a different crdc_series_uuid, not a prior_versions_index row that starts at N: that table also starts a row when a version only moved storage bucket (11,338 versions at v19->v20), which the old logic reported as revisions (v20: 11,928 -> 590). - Collapse each release to one row per series before joining; 48 v23 versions have two overlapping rows. - Collection moves: per collection a moved series is removed from one and added to the other (v5: apollo removed, apollo_5_lscc new); the totals join on the series alone, so a move is not an addition to IDC. - Analysis results come from the same per-release sets as collections and report revised/removed series too. - patients_affected counts (collection_id, PatientID) pairs. - The served release comes from the idc-index-data version, as for get_idc_version; versions are validated against 1..served. - REST and MCP accept "v23" (the form previous_version returns); the core parser rejects non-ASCII digits; whats_new reuses it and falls back to the served release on bad input. - Results are memoized per version; the diff takes ~1-1.5 s cold. - v1/v2 carry a warning: prior_versions_index has no v1 rows (its build drops the original idc-open-idc bucket). - prior_versions_index descriptions are fill-ins matching the text added upstream in idc-index-data, used only while upstream is empty, and say the table is not a record of what is new. - Shared mb_to_tb() / served_idc_release() helpers replace four copies. Tests cover past releases (v20 bucket moves, v5 collection move), both adapters accepting "v23", the prompt fallback, warnings and the memo. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 26 ++- docs/user-guide.md | 32 ++-- src/idc_api/core/models.py | 37 ++-- src/idc_api/core/schema.py | 68 ++++++-- src/idc_api/core/services/_common.py | 19 ++ src/idc_api/core/services/cohort.py | 4 +- src/idc_api/core/services/discovery.py | 12 +- src/idc_api/core/services/licenses.py | 5 +- src/idc_api/core/services/releases.py | 230 +++++++++++++++---------- src/idc_api/mcp/server.py | 49 +++--- src/idc_api/rest/app.py | 6 +- tests/test_releases.py | 98 ++++++++++- 12 files changed, 392 insertions(+), 194 deletions(-) create mode 100644 src/idc_api/core/services/_common.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dedf8c0..cbe2301 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,15 +29,15 @@ Refactors, CI, and formatting land in the git history, not here. `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. -- **Release history: `GET /v3/releases/changes?version=N` and the MCP tool - `get_release_changes(version)`** — what changed in an IDC release versus the previous one: - series added / revised / removed (with size in TB and patients affected), collections that are - new / updated / removed, and analysis results that gained series. Any past release can be - described, computed from the served release's `index` + `prior_versions_index`; `version` - defaults to the served release. -- MCP prompt **`whats_new`** (optional `version`) — a ready-made "summarize this release" request. -- `GET /v3/tables` / `list_tables`: each table now carries **`notable_columns`**, a few columns - worth knowing before writing SQL (e.g. `series_init_idc_version` on `index`). +- `GET /v3/releases/changes?version=N` and the MCP tool `get_release_changes(version)` describe + what changed in IDC release N (`24` or `v24`; default the served release) against N-1: series + added, revised and removed with TB and patients affected, new, updated and removed collections, + and analysis results with changed series. Any past release is covered, computed from the served + release's tables; v1 and v2 carry a `warnings` entry, since v1 is only partly recorded + ([#40](https://github.com/ImagingDataCommons/IDC-REST-MCP/issues/40)). +- MCP prompt `whats_new` (optional `version`) asks for a summary of a release. +- `GET /v3/tables` / `list_tables` give each table a few `notable_columns`, such as + `series_init_idc_version` on `index`. ### Changed @@ -48,11 +48,9 @@ Refactors, CI, and formatting land in the git history, not here. `origin_patient`, `ImageOrientationPatient` and `slice_spacing_mm`. `GET /v3/version` and `get_idc_version` report `idc_index_data_version` `25.0.4`; collection, series, and size totals are unchanged. -- `prior_versions_index` is now documented: a table description and descriptions for - `min_idc_version`, `max_idc_version`, `crdc_series_uuid`, and `series_size_MB` (upstream ships - them empty). -- The `get_idc_version` tool description no longer implies the server can only speak to one - release; it points at `get_release_changes` and the version-history columns. +- `get_table_schema` / `list_tables` describe `prior_versions_index` and all its columns, which + upstream ships empty through `idc-index-data` 25.0.4. The description says it is not a record + of what is new: a row also ends when a version only moved storage bucket. ### Fixed diff --git a/docs/user-guide.md b/docs/user-guide.md index 4ae9f64..8541401 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -154,7 +154,7 @@ per-collection, keyed by `collection_id`). | `index` | one row per **series** — the main table | | `collections_index` | one row per collection (curated metadata) | | `analysis_results_index` | one row per analysis result | -| `version_metadata_index` / `prior_versions_index` | IDC release dates / superseded or removed series versions (`min_idc_version`–`max_idc_version`) — see [Release history](#release-history) | +| `version_metadata_index` / `prior_versions_index` | IDC release dates / series versions no longer served (not a list of what is new) — see [Release history](#release-history) | | `seg_index`, `ann_index`, `ann_group_index`, `rtstruct_index` | segmentations / annotations / RT structures: **what was segmented** (`SegmentedPropertyType_CodeMeanings` — `BodyPartExamined` reflects the source acquisition, not this) and the **reference** to the image series they derive from (`segmented_SeriesInstanceUID` / `referenced_SeriesInstanceUID`) | | `ct_index`, `mr_index`, `pt_index` | per-modality acquisition parameters (slice thickness, kVp, TE/TR, injected dose…) | | `sm_index`, `sm_instance_index` | slide-microscopy (pathology) series / instance metadata | @@ -242,20 +242,19 @@ WHERE i.collection_id = 'nlst' AND i.Modality = 'CT' The server serves **one** IDC data release (see `GET /v3/version` / `get_idc_version`), but that release carries the history of every earlier one, so "what's new in v24" or "what changed since -v20" are answerable without external release notes: +v20" are answerable without external release notes. -- On `index`, `series_init_idc_version` is the release a series first appeared in, and - `series_revised_idc_version` the release its current content dates from. -- `prior_versions_index` holds every **superseded or removed version** of a series — one row per - old `crdc_series_uuid`, valid from `min_idc_version` through `max_idc_version`. If its - `SeriesInstanceUID` is still in `index` the series was revised; otherwise it was removed. -- `version_metadata_index` dates each release. +`GET /v3/releases/changes?version=N` / `get_release_changes(version=N)` diffs release N against +N-1: series **added / revised / removed** (with TB and patients affected), collections that are +**new / updated / removed**, and the analysis results with changed series. `version` takes `24` or +`v24`; omit it for the served release. A series that moved to another collection counts as removed +from one and added to the other in the per-collection rows, but not in the totals. Results for v1 +and v2 carry a `warnings` entry: series versions served in v1 and replaced later were never +recorded, so those two diffs are incomplete. -`GET /v3/releases/changes?version=N` / `get_release_changes(version=N)` does the diff of release -N against N-1 for you: series **added / revised / removed** (with TB and patients affected), -collections that are **new / updated / removed**, and the analysis results that gained series. -Omit `version` for the served release. For per-series detail, query the columns above with SQL — -e.g. the series of a collection that changed since v20: +For per-series detail, query `index` with SQL: `series_init_idc_version` is the release a series +first appeared in, and `series_revised_idc_version` the release its current content dates from. +For example, the series of a collection that changed since v20: ```sql SELECT SeriesInstanceUID, series_init_idc_version, series_revised_idc_version @@ -263,6 +262,13 @@ FROM index WHERE collection_id = 'nlst' AND series_revised_idc_version > 20 ``` +`prior_versions_index` lists the series versions IDC no longer serves, mainly so manifests from +earlier releases can still be resolved. It is **not** a record of what is new: it never lists +added series, and a row also ends when a version only moved storage bucket, so counting rows by +`max_idc_version` overstates revisions (for v20 it would count 11,928, of which 11,338 were +bucket moves). +`get_release_changes` accounts for this. + The diff tells you *what* changed, not *why*; for the narrative, see the [IDC release notes](https://learn.canceridc.dev/data/data-release-notes). diff --git a/src/idc_api/core/models.py b/src/idc_api/core/models.py index 13f498b..7fdb712 100644 --- a/src/idc_api/core/models.py +++ b/src/idc_api/core/models.py @@ -41,10 +41,11 @@ class Stats(BaseModel): class CollectionChange(BaseModel): collection_id: str - status: str = Field( + status: Literal["new", "removed", "updated"] = Field( ..., description="'new' (no series in the previous release), 'removed' (no series in this " - "release), or 'updated' (present in both, with series added/revised/removed).", + "release), or 'updated' (present in both, with series added/revised/removed). A series " + "that moved to another collection counts as removed from one and added to the other.", ) series_added: int series_revised: int @@ -59,8 +60,12 @@ class CollectionChange(BaseModel): class AnalysisResultChange(BaseModel): analysis_result_id: str - is_new: bool = Field(..., description="True if this analysis result first appeared here.") + is_new: bool = Field( + ..., description="True if this analysis result had no series in the previous release." + ) series_added: int + series_revised: int + series_removed: int class ReleaseChanges(BaseModel): @@ -71,13 +76,20 @@ class ReleaseChanges(BaseModel): previous_version: str | None = Field(None, description="The release compared against.") previous_release_date: str | None = None current_version: str = Field(..., description="The release this server serves.") - series_added: int - series_revised: int - series_removed: int + series_added: int = Field(..., description="Series new to IDC in this release.") + series_revised: int = Field( + ..., + description="Series in both releases whose content changed (a new crdc_series_uuid).", + ) + series_removed: int = Field(..., description="Series IDC no longer serves in this release.") size_TB_added: float size_TB_revised: float size_TB_removed: float - patients_affected: int + patients_affected: int = Field( + ..., + description="Distinct patients (PatientID within a collection) with at least one added, " + "revised, or removed series.", + ) new_collections: list[str] = Field( default_factory=list, description="Collections that first appeared in this release." ) @@ -89,9 +101,14 @@ class ReleaseChanges(BaseModel): ) analysis_results: list[AnalysisResultChange] = Field( default_factory=list, - description="Analysis results that gained series in this release. Counted from series " - "still present in the current release, so for an older release it omits series that were " - "later removed.", + description="Analysis results with series added, revised, or removed in this release. A " + "series no longer served today cannot be attributed to its analysis result, so for an " + "older release these counts omit series that were later removed.", + ) + warnings: list[str] = Field( + default_factory=list, + description="Caveats about how complete this release's numbers are. Empty when the " + "served data records the release in full.", ) note: str = "" diff --git a/src/idc_api/core/schema.py b/src/idc_api/core/schema.py index 09003b2..38209ae 100644 --- a/src/idc_api/core/schema.py +++ b/src/idc_api/core/schema.py @@ -191,27 +191,52 @@ def _column_type(c: dict, struct_types: dict[str, str] | None = None) -> str: "(join to index on dicom_patient_id = index.PatientID). Use this table's column_label and " "value mappings to interpret their often-cryptic coded columns." ), - # Upstream ships this table with no description at all. +} + +# Fill-ins for upstream descriptions that are empty, applied only where upstream has no text, so +# the upstream description takes over on the pin bump that brings it. The prior_versions_index +# text matches what idc-index-data adds for its next release (upstream ships it empty through +# 25.0.4), with the main index named `index` as this server's SQL calls it. +TABLE_DESCRIPTION_FILLINS: dict[str, str] = { "prior_versions_index": ( - "Series versions IDC served in an earlier release but no longer serves: one row per " - "superseded or removed version of a series (identified by crdc_series_uuid), valid from " - "min_idc_version through max_idc_version. A SeriesInstanceUID that is also in `index` was " - "revised (its current version starts at index.series_revised_idc_version); one that is " - "not was removed. Together with `index` this reconstructs the content of any past release " - "— get_release_changes does that diff for you." + "Versions of IDC series that an earlier IDC release served and the current release no " + "longer serves, each with the range of releases that served it. A version is identified " + "by crdc_series_uuid. A SeriesInstanceUID that is also in `index` was revised (its " + "current version is in `index`); one that is not was removed from IDC. A version can " + "span more than one row, since a new row starts whenever its storage bucket changed " + "between releases, even though its content did not. This table is not a record of what " + "is new in a release: it never lists added series, and a row ending at a release does " + "not mean that release changed the series. To find the series added or revised in a " + "release, use series_init_idc_version and series_revised_idc_version in `index`, or " + "get_release_changes for a whole release. The main use of this table is to resolve and " + "download series listed in manifests made from earlier releases." ), } -# Fill-ins for upstream column descriptions that are empty. Applied only where upstream has no -# text, so an upstream description takes over automatically once it lands. COLUMN_DESCRIPTION_FILLINS: dict[str, dict[str, str]] = { "prior_versions_index": { - "crdc_series_uuid": "Identifier of this specific version of the series (changes on " - "every revision); never equal to a crdc_series_uuid in `index`.", - "min_idc_version": "First IDC release (integer) that served this version of the series.", - "max_idc_version": "Last IDC release (integer) that served this version of the series; " - "the next release revised or removed it.", - "series_size_MB": "Size of this version of the series, in MB.", + "collection_id": "unique identifier of the collection the series belonged to", + "PatientID": "identifier of the patient within the collection (DICOM attribute PatientID)", + "SeriesInstanceUID": "DICOM SeriesInstanceUID of the series; the same for every version " + "of a series, so it also matches the current version in `index` if the series was " + "revised rather than removed", + "StudyInstanceUID": "DICOM StudyInstanceUID of the study the series belongs to", + "Modality": "DICOM Modality of the series", + "gcs_bucket": "name of the Google Cloud Storage bucket that held this version of the " + "series in the releases min_idc_version through max_idc_version", + "crdc_series_uuid": "unique identifier of this version of the series within IDC; " + "changes whenever the content of the series changes, so it never matches a " + "crdc_series_uuid in `index`. The same value on several rows is one version that moved " + "between storage buckets", + "series_size_MB": "total size of this version of the series in megabytes", + "series_aws_url": "public AWS S3 URL to download this version of the series in bulk " + "(each instance is a separate file)", + "aws_bucket": "name of the AWS S3 bucket that contains this version of the series", + "min_idc_version": "first IDC release (as an integer) in this row's range: this " + "version, in this storage bucket, was served from this release on", + "max_idc_version": "last IDC release (as an integer) in this row's range. The next " + "release revised the series, removed it, or moved this version to another storage " + "bucket (which starts a new row)", }, } @@ -233,7 +258,12 @@ def _column_type(c: dict, struct_types: dict[str, str] | None = None) -> str: "series_revised_idc_version", ], "version_metadata_index": ["idc_version", "version_timestamp"], - "prior_versions_index": ["SeriesInstanceUID", "min_idc_version", "max_idc_version"], + "prior_versions_index": [ + "SeriesInstanceUID", + "crdc_series_uuid", + "min_idc_version", + "max_idc_version", + ], "seg_index": ["segmented_SeriesInstanceUID", "SegmentedPropertyType_CodeMeanings"], } @@ -242,8 +272,8 @@ def _column_type(c: dict, struct_types: dict[str, str] | None = None) -> str: def table_schema(table: str) -> dict: """Return ``{name, description, columns:[{name,type,description}]}`` for a table, sourced from the idc-index schema JSON shipped in INDEX_METADATA (table descriptions may be - repointed inward via ``TABLE_DESCRIPTION_OVERRIDES``; empty column descriptions are filled - from ``COLUMN_DESCRIPTION_FILLINS``).""" + repointed inward via ``TABLE_DESCRIPTION_OVERRIDES``; empty descriptions are filled from + ``TABLE_DESCRIPTION_FILLINS`` / ``COLUMN_DESCRIPTION_FILLINS``).""" meta = idc_index_data.INDEX_METADATA[metadata_key(table)] schema = meta.get("schema", {}) or {} cols = schema.get("columns", []) @@ -264,7 +294,7 @@ def table_schema(table: str) -> dict: "name": table, "description": TABLE_DESCRIPTION_OVERRIDES.get(table) or schema.get("table_description", "") - or "", + or TABLE_DESCRIPTION_FILLINS.get(table, ""), "columns": columns, } diff --git a/src/idc_api/core/services/_common.py b/src/idc_api/core/services/_common.py new file mode 100644 index 0000000..cc316f4 --- /dev/null +++ b/src/idc_api/core/services/_common.py @@ -0,0 +1,19 @@ +"""Small helpers shared by several services, so every endpoint reports the same thing the same +way.""" + +from __future__ import annotations + +import idc_index_data + +_MB_PER_TB = 1_000_000 + + +def mb_to_tb(mb: float) -> float: + """Megabytes (as ``series_size_MB`` sums are) to the ``size_TB`` every response reports.""" + return round(mb / _MB_PER_TB, 3) + + +def served_idc_release() -> int: + """The IDC data release this server serves: the major version of the pinned + ``idc-index-data`` (25.0.4 → 25).""" + return int(idc_index_data.__version__.split(".")[0]) diff --git a/src/idc_api/core/services/cohort.py b/src/idc_api/core/services/cohort.py index 396f6a3..e893e82 100644 --- a/src/idc_api/core/services/cohort.py +++ b/src/idc_api/core/services/cohort.py @@ -12,8 +12,8 @@ SeriesManifestRow, ) from .manifest import ManifestService +from ._common import mb_to_tb -_MB_PER_TB = 1_000_000 _ROW_COLUMNS = [ "collection_id", @@ -55,7 +55,7 @@ def counts(self, filters: CohortFilters) -> CohortCounts: studies=row["studies"], series=row["series"], instances=int(row["instances"]), - size_TB=round(row["size_mb"] / _MB_PER_TB, 3), + size_TB=mb_to_tb(row["size_mb"]), # Echo the effective filter so a caller can tell an empty cohort apart from a # dropped filter without guessing from the magnitude of the numbers. filters_applied=f.applied, diff --git a/src/idc_api/core/services/discovery.py b/src/idc_api/core/services/discovery.py index 283e958..660037e 100644 --- a/src/idc_api/core/services/discovery.py +++ b/src/idc_api/core/services/discovery.py @@ -19,13 +19,11 @@ VersionInfo, ) from ..version import build_stamp, package_version - -_MB_PER_TB = 1_000_000 +from ._common import mb_to_tb, served_idc_release def _idc_version() -> str: - major = idc_index_data.__version__.split(".")[0] - return f"v{major}" + return f"v{served_idc_release()}" class DiscoveryService: @@ -58,7 +56,7 @@ def stats(self) -> Stats: studies=agg["studies"], series=agg["series"], instances=int(agg["instances"]), - size_TB=round(agg["size_mb"] / _MB_PER_TB, 3), + size_TB=mb_to_tb(agg["size_mb"]), ) def list_collections(self) -> list[CollectionSummary]: @@ -105,7 +103,7 @@ def get_collection(self, collection_id: str) -> CollectionDetail: studies=agg["studies"], series=agg["series"], instances=int(agg["instances"]), - size_TB=round(agg["size_mb"] / _MB_PER_TB, 3), + size_TB=mb_to_tb(agg["size_mb"]), modalities=modalities, licenses=licenses, ) @@ -124,7 +122,7 @@ def _licenses_for(self, where: list[str], params: list) -> list[LicenseItem]: LicenseItem( license_short_name=r["license_short_name"], series=r["series"], - size_TB=round(r["size_mb"] / _MB_PER_TB, 3), + size_TB=mb_to_tb(r["size_mb"]), ) for r in rows ] diff --git a/src/idc_api/core/services/licenses.py b/src/idc_api/core/services/licenses.py index c6515eb..d335285 100644 --- a/src/idc_api/core/services/licenses.py +++ b/src/idc_api/core/services/licenses.py @@ -5,8 +5,7 @@ from ..backend.base import QueryBackend from ..filters import compile_filters from ..models import CohortFilters, LicenseItem, LicensesResult - -_MB_PER_TB = 1_000_000 +from ._common import mb_to_tb class LicenseService: @@ -27,7 +26,7 @@ def get_licenses(self, filters: CohortFilters) -> LicensesResult: LicenseItem( license_short_name=r["license_short_name"], series=r["series"], - size_TB=round(r["size_mb"] / _MB_PER_TB, 3), + size_TB=mb_to_tb(r["size_mb"]), ) for r in rows ], diff --git a/src/idc_api/core/services/releases.py b/src/idc_api/core/services/releases.py index e20c714..9f769c4 100644 --- a/src/idc_api/core/services/releases.py +++ b/src/idc_api/core/services/releases.py @@ -2,9 +2,14 @@ Computed entirely from the current release's bundled tables. Every *series version* IDC has ever served is either the current row in ``index`` (valid from ``series_revised_idc_version`` -to now) or a row in ``prior_versions_index`` (a superseded or removed version, valid from +to now) or a row in ``prior_versions_index`` (a version no longer served, valid from ``min_idc_version`` to ``max_idc_version``). A series is in release N if one of its versions spans N, so comparing release N with N-1 gives added / revised / removed series exactly. + +Two properties of ``prior_versions_index`` shape the SQL. It starts a new row whenever a +version only moved storage bucket, so a row ending at N-1 does not mean N changed the series: +"revised" is decided by ``crdc_series_uuid``, never by where a row starts. And a few versions +have two overlapping rows, so each release is collapsed to one row per series before joining. """ from __future__ import annotations @@ -12,73 +17,112 @@ from ..backend.base import QueryBackend from ..errors import InvalidQueryError from ..models import AnalysisResultChange, CollectionChange, ReleaseChanges +from ._common import mb_to_tb, served_idc_release + +# prior_versions_index has no row starting at v1 (its build drops the original idc-open-idc +# bucket), so a v1 version that was later replaced is invisible. +_EARLY_RELEASE_WARNING = ( + "IDC v1 is only partly recorded: series versions served in v1 and replaced later are not in " + "prior_versions_index. The v1 counts include only series still served, and the v2 counts may " + "report as added some series that were revised from v1." +) -_MB_PER_TB = 1_000_000 +# Aggregates over a joined set of (current, previous) series rows. Shared verbatim by the total, +# per-collection, and per-analysis-result groupings below; contains no caller values. +_AGGS = """ + count(*) FILTER (WHERE in_prev) AS prev_series, + count(*) FILTER (WHERE change = 'added') AS series_added, + count(*) FILTER (WHERE change = 'revised') AS series_revised, + count(*) FILTER (WHERE change = 'removed') AS series_removed, + COALESCE(sum(cur_mb) FILTER (WHERE change = 'added'), 0) AS added_mb, + COALESCE(sum(cur_mb) FILTER (WHERE change = 'revised'), 0) AS revised_mb, + COALESCE(sum(prev_mb) FILTER (WHERE change = 'removed'), 0) AS removed_mb, + count(*) FILTER (WHERE in_cur) AS cur_series, + count(DISTINCT [collection_id, PatientID]) FILTER (WHERE change IS NOT NULL) + AS patients_affected""" -# Params: current, N, N, N-1, N-1, N. -_DIFF_SQL = """ -WITH v AS ( - SELECT SeriesInstanceUID, collection_id, PatientID, series_size_MB, - series_revised_idc_version AS lo, ? AS hi - FROM index - UNION ALL - SELECT SeriesInstanceUID, collection_id, PatientID, series_size_MB, - min_idc_version AS lo, max_idc_version AS hi - FROM prior_versions_index -), -cur AS (SELECT * FROM v WHERE lo <= ? AND hi >= ?), -prev AS (SELECT * FROM v WHERE lo <= ? AND hi >= ?), -s AS ( +# PatientID is unique only within a collection, hence the [collection_id, PatientID] pairs. +_JOINED = """ SELECT COALESCE(c.collection_id, p.collection_id) AS collection_id, COALESCE(c.PatientID, p.PatientID) AS PatientID, + COALESCE(c.analysis_result_id, p.analysis_result_id) AS analysis_result_id, c.SeriesInstanceUID IS NOT NULL AS in_cur, p.SeriesInstanceUID IS NOT NULL AS in_prev, CASE WHEN p.SeriesInstanceUID IS NULL THEN 'added' WHEN c.SeriesInstanceUID IS NULL THEN 'removed' - WHEN c.lo = ? THEN 'revised' END AS change, - c.series_size_MB AS cur_mb, - p.series_size_MB AS prev_mb - FROM cur c FULL OUTER JOIN prev p ON c.SeriesInstanceUID = p.SeriesInstanceUID + WHEN c.crdc_series_uuid <> p.crdc_series_uuid THEN 'revised' END AS change, + c.mb AS cur_mb, + p.mb AS prev_mb + FROM cur c FULL OUTER JOIN prev p ON c.SeriesInstanceUID = p.SeriesInstanceUID""" + +# The only placeholders are the two in `params`, bound once. Rows of `index` are already one +# per series; only prior_versions_index needs collapsing to one row per series and release (its +# rows for one release share a crdc_series_uuid). `analysis_result_id` of a version no longer +# served comes from the series' current row, so it is NULL for removed series. +_DIFF_SQL = f""" +WITH params AS (SELECT ?::INTEGER AS served, ?::INTEGER AS n), +rel AS (SELECT n AS r FROM params UNION ALL SELECT n - 1 FROM params), +versions AS ( + SELECT r, SeriesInstanceUID, collection_id, PatientID, analysis_result_id, crdc_series_uuid, + series_size_MB AS mb + FROM index JOIN rel ON series_revised_idc_version <= r + UNION ALL + SELECT r, pv.SeriesInstanceUID, pv.collection_id, pv.PatientID, i.analysis_result_id, + pv.crdc_series_uuid, max(pv.series_size_MB) + FROM prior_versions_index pv + JOIN rel ON pv.min_idc_version <= r AND pv.max_idc_version >= r + LEFT JOIN index i USING (SeriesInstanceUID) + GROUP BY ALL +), +cur AS (SELECT * FROM versions WHERE r = (SELECT n FROM params)), +prev AS (SELECT * FROM versions WHERE r = (SELECT n FROM params) - 1), +-- Series level, for the totals: a series that moved collection is neither added nor removed. +by_series AS ({_JOINED} +), +-- Collection level: a series that moved is removed from one collection and added to the other. +by_collection AS ({_JOINED} AND c.collection_id = p.collection_id ) -SELECT GROUPING(collection_id) AS is_total, - collection_id, - count(*) FILTER (WHERE in_prev) AS prev_series, - count(*) FILTER (WHERE in_cur) AS cur_series, - count(*) FILTER (WHERE change = 'added') AS series_added, - count(*) FILTER (WHERE change = 'revised') AS series_revised, - count(*) FILTER (WHERE change = 'removed') AS series_removed, - COALESCE(sum(cur_mb) FILTER (WHERE change = 'added'), 0) AS added_mb, - COALESCE(sum(cur_mb) FILTER (WHERE change = 'revised'), 0) AS revised_mb, - COALESCE(sum(prev_mb) FILTER (WHERE change = 'removed'), 0) AS removed_mb, - count(DISTINCT PatientID) FILTER (WHERE change IS NOT NULL) AS patients_affected -FROM s -GROUP BY GROUPING SETS ((collection_id), ()) -HAVING count(*) FILTER (WHERE change IS NOT NULL) > 0 OR GROUPING(collection_id) = 1 +SELECT 'total' AS level, NULL AS id, {_AGGS} FROM by_series +UNION ALL +SELECT 'collection', collection_id, {_AGGS} FROM by_collection GROUP BY collection_id +UNION ALL +SELECT 'analysis_result', analysis_result_id, {_AGGS} FROM by_collection +WHERE analysis_result_id IS NOT NULL GROUP BY analysis_result_id """ _NOTE = ( - "Computed from the current release: series in `index` plus superseded/removed series " - "versions in `prior_versions_index`. 'revised' means the series is in both releases but its " - "content changed (new crdc_series_uuid). For per-series detail, query those tables with " - "run_sql (series_init_idc_version / series_revised_idc_version on `index`; min_idc_version / " - "max_idc_version on `prior_versions_index`)." + "Computed from the current release: series in `index` plus the series versions no longer " + "served in `prior_versions_index`. 'revised' means the series is in both releases with " + "different content (a new crdc_series_uuid). Do not derive changes from " + "prior_versions_index row ranges: a row also ends when a version only moved storage bucket. " + "For per-series detail, use series_init_idc_version / series_revised_idc_version on `index`." ) -def _tb(mb: float) -> float: - return round(mb / _MB_PER_TB, 3) - - -def _parse_version(version: int | str) -> int: +def parse_idc_version(version: int | str) -> int: + """An IDC release number from ``24``, ``"24"`` or ``"v24"``; raises InvalidQueryError + otherwise.""" s = str(version).strip().lower().removeprefix("v") - if not s.isdigit(): - raise InvalidQueryError(f"Invalid IDC version: {version!r}. Use an integer such as 24.") + if not (s.isascii() and s.isdigit()): + raise InvalidQueryError(f"Invalid IDC version: {version!r}. Use a release such as 24.") return int(s) +def _status(r: dict) -> str: + if r["prev_series"] == 0: + return "new" + if r["cur_series"] == 0: + return "removed" + return "updated" + + class ReleaseService: + """Stateless apart from a per-version memo: the data is fixed for the life of the + process, and the diff scans every series version.""" + def __init__(self, backend: QueryBackend): self.backend = backend + self._memo: dict[int, ReleaseChanges] = {} def _release_dates(self) -> dict[int, str | None]: rows = self.backend.query( @@ -90,78 +134,80 @@ def _release_dates(self) -> dict[int, str | None]: } def release_changes(self, version: int | str | None = None) -> ReleaseChanges: - dates = self._release_dates() - current = max(dates) - n = current if version is None else _parse_version(version) - if n not in dates: + served = served_idc_release() + n = served if version is None else parse_idc_version(version) + if not 1 <= n <= served: raise InvalidQueryError( - f"Unknown IDC version: {version!r}. Releases available: v{min(dates)}–v{current}." + f"Unknown IDC version: {version!r}. Releases available: v1–v{served}." ) + if n not in self._memo: + self._memo[n] = self._compute(n, served) + return self._memo[n] - rows = self.backend.query(_DIFF_SQL, [current, n, n, n - 1, n - 1, n]).rows - total = next(r for r in rows if r["is_total"]) - per_coll = [r for r in rows if not r["is_total"]] - - def status(r: dict) -> str: - if r["prev_series"] == 0: - return "new" - if r["cur_series"] == 0: - return "removed" - return "updated" + def _compute(self, n: int, served: int) -> ReleaseChanges: + dates = self._release_dates() + rows = self.backend.query(_DIFF_SQL, [served, n]).rows + total = next(r for r in rows if r["level"] == "total") + changed = [ + r + for r in rows + if r["level"] != "total" + and (r["series_added"] or r["series_revised"] or r["series_removed"]) + ] collections = sorted( ( CollectionChange( - collection_id=r["collection_id"], - status=status(r), + collection_id=r["id"], + status=_status(r), series_added=r["series_added"], series_revised=r["series_revised"], series_removed=r["series_removed"], - size_TB_added=_tb(r["added_mb"]), - size_TB_revised=_tb(r["revised_mb"]), - size_TB_removed=_tb(r["removed_mb"]), + size_TB_added=mb_to_tb(r["added_mb"]), + size_TB_revised=mb_to_tb(r["revised_mb"]), + size_TB_removed=mb_to_tb(r["removed_mb"]), patients_affected=r["patients_affected"], ) - for r in per_coll + for r in changed + if r["level"] == "collection" ), key=lambda c: (-c.size_TB_added, -c.series_added, c.collection_id), ) + analysis_results = sorted( + ( + AnalysisResultChange( + analysis_result_id=r["id"], + is_new=r["prev_series"] == 0, + series_added=r["series_added"], + series_revised=r["series_revised"], + series_removed=r["series_removed"], + ) + for r in changed + if r["level"] == "analysis_result" + ), + key=lambda a: (-a.series_added, a.analysis_result_id), + ) - analysis = self.backend.query( - "SELECT analysis_result_id, " - "count(*) FILTER (WHERE series_init_idc_version = ?) AS series_added, " - "min(series_init_idc_version) = ? AS is_new " - "FROM index WHERE analysis_result_id IS NOT NULL " - "GROUP BY 1 HAVING series_added > 0 ORDER BY series_added DESC, 1", - [n, n], - ).rows - - prev = n - 1 if (n - 1) in dates else None + prev = n - 1 if n > 1 else None return ReleaseChanges( idc_version=f"v{n}", - release_date=dates[n], - previous_version=f"v{prev}" if prev is not None else None, - previous_release_date=dates.get(prev) if prev is not None else None, - current_version=f"v{current}", + release_date=dates.get(n), + previous_version=f"v{prev}" if prev else None, + previous_release_date=dates.get(prev), + current_version=f"v{served}", series_added=total["series_added"], series_revised=total["series_revised"], series_removed=total["series_removed"], - size_TB_added=_tb(total["added_mb"]), - size_TB_revised=_tb(total["revised_mb"]), - size_TB_removed=_tb(total["removed_mb"]), + size_TB_added=mb_to_tb(total["added_mb"]), + size_TB_revised=mb_to_tb(total["revised_mb"]), + size_TB_removed=mb_to_tb(total["removed_mb"]), patients_affected=total["patients_affected"], new_collections=sorted(c.collection_id for c in collections if c.status == "new"), removed_collections=sorted( c.collection_id for c in collections if c.status == "removed" ), collections=collections, - analysis_results=[ - AnalysisResultChange( - analysis_result_id=r["analysis_result_id"], - is_new=bool(r["is_new"]), - series_added=r["series_added"], - ) - for r in analysis - ], + analysis_results=analysis_results, + warnings=[_EARLY_RELEASE_WARNING] if n <= 2 else [], note=_NOTE, ) diff --git a/src/idc_api/mcp/server.py b/src/idc_api/mcp/server.py index aff9070..f88aaaf 100644 --- a/src/idc_api/mcp/server.py +++ b/src/idc_api/mcp/server.py @@ -37,6 +37,7 @@ from ..core.context import AppContext from ..core.errors import IDCAPIError, InvalidQueryError from ..core.models import AnalysisResultsMode, CohortFilters, NumericRange +from ..core.services.releases import parse_idc_version from ..http_headers import HSTSMiddleware from ..settings import get_settings @@ -249,9 +250,8 @@ def get_idc_version() -> dict: server's own software version (`api_version`, and `build` if the deploy stamped one). Call this to confirm which IDC version your answers are based on — and which build of the server produced them. The server serves one release but is NOT limited to it: every series records - the release it first appeared in and was last revised in, and superseded/removed series are - kept in `prior_versions_index`, so past releases and what changed between them are - queryable — use get_release_changes for "what's new in vN".""" + the release it first appeared in and was last revised in, so past releases and what changed + between them are queryable — use get_release_changes for "what's new in vN".""" return ctx.discovery.version().model_dump(mode="json") @@ -265,16 +265,17 @@ def get_stats() -> dict: @mcp.tool() @guard -def get_release_changes(version: int | None = None) -> dict: +def get_release_changes(version: int | str | None = None) -> dict: """What's new in an IDC data release — what changed since the previous version. Returns the series added, revised, and removed (with size in TB and patients affected), the collections that were newly added, updated, or removed, and the analysis results that gained series. - `version` is the release number (e.g. 24 for v24); omit it for the release this server - serves. Call this whenever the user asks what is new, added, changed, updated, or removed in - IDC, in a given release, or since a version — do not conclude that history is unavailable - because the server serves a single release. For per-series detail beyond this summary, use - run_sql on `index` (series_init_idc_version / series_revised_idc_version) and - `prior_versions_index` (min_idc_version / max_idc_version).""" + `version` is the release, as 24 or "v24" (the form get_idc_version and `previous_version` + use); omit it for the release this server serves. Call this whenever the user asks what is + new, added, changed, updated, or removed in IDC, in a given release, or since a version — do + not conclude that history is unavailable because the server serves a single release, and do + not work it out from `prior_versions_index` row ranges (a row also ends when a series only + moved storage bucket). Surface any `warnings`. For per-series detail beyond this summary, + use run_sql on `index` (series_init_idc_version / series_revised_idc_version).""" return ctx.releases.release_changes(version).model_dump(mode="json") @@ -539,14 +540,14 @@ def get_licenses( @mcp.prompt() def whats_new(version: str = "") -> str: """Summarize what is new in an IDC data release (the latest one by default).""" - n = version.strip().lower().removeprefix("v") + try: + n = parse_idc_version(version) if version.strip() else None + except InvalidQueryError: + n = None # fall back to the served release rather than paste the raw text into a call target, call = ( (f"IDC release v{n}", f"get_release_changes(version={n})") - if n - else ( - "the latest IDC release", - "get_release_changes()", - ) + if n is not None + else ("the latest IDC release", "get_release_changes()") ) return ( f"Summarize what changed in {target}. Call {call}, then report: the release date; the " @@ -671,14 +672,14 @@ def whats_new(version: str = "") -> str: **Release history.** The server serves one IDC release, but that release carries the history of all earlier ones. On `index`, `series_init_idc_version` is the release a series first appeared in -and `series_revised_idc_version` the release its current content dates from. -`prior_versions_index` holds every superseded or removed *version* of a series (one row per old -`crdc_series_uuid`, valid from `min_idc_version` through `max_idc_version`); a SeriesInstanceUID -also in `index` was revised, one that isn't was removed. `version_metadata_index` dates each -release. `get_release_changes(version=N)` diffs release N against N-1 from these tables — series -added / revised / removed, new and removed collections, per-collection size — so start there for -"what's new in vN"; drop to `run_sql` on those columns for per-series detail (e.g. "which of my -cohort's series changed since v20": `series_revised_idc_version > 20`). +and `series_revised_idc_version` the release its current content dates from; `version_metadata_index` +dates each release. For "what's new in vN", call `get_release_changes(version=N)`: it diffs +release N against N-1 — series added / revised / removed, new and removed collections, +per-collection size. For per-series detail use `run_sql` on the two `index` columns (e.g. "which +of my cohort's series changed since v20": `series_revised_idc_version > 20`). +`prior_versions_index` lists the series versions no longer served (for resolving old manifests). +It is not a record of what is new: it never lists added series, and a row also ends when a +version only moved storage bucket, so do not count changes from its `max_idc_version`. """ diff --git a/src/idc_api/rest/app.py b/src/idc_api/rest/app.py index d1b46d4..01602fe 100644 --- a/src/idc_api/rest/app.py +++ b/src/idc_api/rest/app.py @@ -448,8 +448,10 @@ def stats(): summary="What changed in a release", ) def release_changes( - version: int | None = Query( - None, ge=1, description="IDC release number, e.g. 24; omit for the served release." + version: str | None = Query( + None, + description="IDC release, as 24 or v24; omit for the served release.", + examples=["v24"], ), ): """What's new in an IDC data release relative to the previous one: series added, diff --git a/tests/test_releases.py b/tests/test_releases.py index 7f887b7..d894485 100644 --- a/tests/test_releases.py +++ b/tests/test_releases.py @@ -11,9 +11,7 @@ def _current(ctx) -> int: - return int( - ctx.backend.query("SELECT max(idc_version) v FROM version_metadata_index").rows[0]["v"] - ) + return int(ctx.discovery.version().idc_version.removeprefix("v")) def test_latest_release_is_default_and_self_consistent(ctx): @@ -53,12 +51,81 @@ def test_first_release_has_no_predecessor(ctx): assert all(c.status == "new" for c in rc.collections) +def test_early_releases_warn_that_v1_is_partly_recorded(ctx): + assert ctx.releases.release_changes(1).warnings + assert ctx.releases.release_changes(2).warnings + assert ctx.releases.release_changes(3).warnings == [] + + +# --- past releases: the cases the served release cannot exercise --------------------------- + + +def test_bucket_moves_are_not_revisions(ctx): + """prior_versions_index starts a new row when a version only changed storage bucket, which + v19→v20 did for thousands of series. Revised means a different crdc_series_uuid.""" + rc = ctx.releases.release_changes(20) + # Independent formulation: series in both releases whose sets of version ids differ. + expected = ctx.backend.query( + """ + WITH v AS ( + SELECT SeriesInstanceUID s, crdc_series_uuid u, series_revised_idc_version lo, + ? AS hi FROM index + UNION ALL + SELECT SeriesInstanceUID, crdc_series_uuid, min_idc_version, max_idc_version + FROM prior_versions_index + ), + c AS (SELECT s, list(DISTINCT u) us FROM v WHERE lo <= 20 AND hi >= 20 GROUP BY s), + p AS (SELECT s, list(DISTINCT u) us FROM v WHERE lo <= 19 AND hi >= 19 GROUP BY s) + SELECT count(*) n FROM c JOIN p USING (s) WHERE c.us <> p.us + """, + [_current(ctx)], + ).rows[0]["n"] + assert rc.series_revised == expected + rows_ending = ctx.backend.query( + "SELECT count(DISTINCT SeriesInstanceUID) n FROM prior_versions_index " + "WHERE max_idc_version = 19 AND min_idc_version <= 19 " + "AND SeriesInstanceUID IN (SELECT SeriesInstanceUID FROM index)" + ).rows[0]["n"] + assert rc.series_revised < rows_ending # the naive reading overcounts + + +def test_collection_move_is_removed_then_new(ctx): + """At v5 the 43 `apollo` series moved to `apollo_5_lscc`.""" + rc = ctx.releases.release_changes(5) + assert "apollo_5_lscc" in rc.new_collections + assert "apollo" in rc.removed_collections + by_id = {c.collection_id: c for c in rc.collections} + assert by_id["apollo"].series_removed == by_id["apollo_5_lscc"].series_added == 43 + # In the totals the move is neither an addition to IDC nor a removal from it. + assert rc.series_added == sum(c.series_added for c in rc.collections) - 43 + assert rc.series_removed == sum(c.series_removed for c in rc.collections) - 43 + + +def test_analysis_results_are_consistent_with_collections(ctx): + for v in (_current(ctx), 20): + rc = ctx.releases.release_changes(v) + for a in rc.analysis_results: + assert a.series_added + a.series_revised + a.series_removed > 0 + if a.is_new: + assert a.series_revised == a.series_removed == 0 + assert sum(a.series_added for a in rc.analysis_results) <= rc.series_added + + +def test_patients_are_counted_per_collection(ctx): + rc = ctx.releases.release_changes(_current(ctx)) + assert rc.patients_affected <= sum(c.patients_affected for c in rc.collections) + + +def test_results_are_memoized(ctx): + assert ctx.releases.release_changes(20) is ctx.releases.release_changes("v20") + + def test_version_accepts_v_prefix(ctx): cur = _current(ctx) assert ctx.releases.release_changes(f"v{cur}") == ctx.releases.release_changes(cur) -@pytest.mark.parametrize("bad", [0, 9999, "latest", "v"]) +@pytest.mark.parametrize("bad", [0, 9999, "latest", "v", "²", "v-1"]) def test_unknown_version_is_a_clean_error(ctx, bad): with pytest.raises(InvalidQueryError): ctx.releases.release_changes(bad) @@ -72,8 +139,18 @@ async def test_release_changes_parity(ctx, client, parse_mcp): assert core == rest == mcp_out -def test_rest_unknown_version_is_400(client): - r = client.get("/v3/releases/changes", params={"version": 9999}) +async def test_both_adapters_accept_the_version_they_return(ctx, client, parse_mcp): + """previous_version / get_idc_version use "v23"; passing it back must work.""" + prev = ctx.releases.release_changes().previous_version + rest = client.get("/v3/releases/changes", params={"version": prev}) + assert rest.status_code == 200 and rest.json()["idc_version"] == prev + mcp_out = parse_mcp(await mcp.call_tool("get_release_changes", {"version": prev})) + assert mcp_out["idc_version"] == prev + + +@pytest.mark.parametrize("bad", [9999, "latest", "²"]) +def test_rest_bad_version_is_400(client, bad): + r = client.get("/v3/releases/changes", params={"version": bad}) assert r.status_code == 400 @@ -94,13 +171,18 @@ async def test_whats_new_prompt(): assert "whats_new" in prompts msg = await mcp.get_prompt("whats_new", {"version": "v24"}) assert "get_release_changes(version=24)" in msg.messages[0].content.text + # Unparseable input falls back to the served release instead of reaching the call. + msg = await mcp.get_prompt("whats_new", {"version": "latest"}) + assert "get_release_changes()" in msg.messages[0].content.text def test_prior_versions_index_is_documented(): sch = schema.table_schema("prior_versions_index") - assert sch["description"] + # It must steer callers away from reading it as a list of what is new. + assert "not a record of what is new" in sch["description"] cols = {c["name"]: c["description"] for c in sch["columns"]} - assert cols["min_idc_version"] and cols["max_idc_version"] + assert all(cols.values()), [n for n, d in cols.items() if not d] + assert "bucket" in cols["max_idc_version"] and "bucket" in cols["crdc_series_uuid"] def test_notable_columns_exist_in_schema(): From b300d4efb77904b279ca33c951791e3696a5c24e Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Mon, 19 Oct 2026 20:05:17 -0400 Subject: [PATCH 3/6] fix(releases): mark the diff SQL's constant interpolation for bandit Bandit's B608 flagged _DIFF_SQL as string-built SQL. It interpolates only module-level SQL fragments and binds its two values; mark it nosec on the first line of the literal, as the other services do. Co-Authored-By: Claude Opus 5.5 --- src/idc_api/core/services/releases.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/idc_api/core/services/releases.py b/src/idc_api/core/services/releases.py index 9f769c4..fbe5218 100644 --- a/src/idc_api/core/services/releases.py +++ b/src/idc_api/core/services/releases.py @@ -59,8 +59,10 @@ # per series; only prior_versions_index needs collapsing to one row per series and release (its # rows for one release share a crdc_series_uuid). `analysis_result_id` of a version no longer # served comes from the series' current row, so it is NULL for removed series. -_DIFF_SQL = f""" -WITH params AS (SELECT ?::INTEGER AS served, ?::INTEGER AS n), +# Interpolates only the constant fragments above; the two values are bound. +_DIFF_SQL = ( + "WITH params AS (SELECT ?::INTEGER AS served, ?::INTEGER AS n)," # nosec B608 + f""" rel AS (SELECT n AS r FROM params UNION ALL SELECT n - 1 FROM params), versions AS ( SELECT r, SeriesInstanceUID, collection_id, PatientID, analysis_result_id, crdc_series_uuid, @@ -89,6 +91,7 @@ SELECT 'analysis_result', analysis_result_id, {_AGGS} FROM by_collection WHERE analysis_result_id IS NOT NULL GROUP BY analysis_result_id """ +) _NOTE = ( "Computed from the current release: series in `index` plus the series versions no longer " From 21d4e7fbb06271dce941ec29c0f57f03b56d96ac Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Mon, 19 Oct 2026 22:29:23 -0400 Subject: [PATCH 4/6] build(deps): bump idc-index 0.13.3 -> 0.13.4 (idc-index-data 25.0.5) idc-index-data 25.0.5 ships descriptions for prior_versions_index and its columns (ImagingDataCommons/idc-index-data#189), so drop the local fill-ins this branch carried for them, and the fill-in mechanism with them. The data is unchanged: 179 collections, 1,044,191 series, 99.9 TB, and the same prior_versions_index rows. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 11 +++--- dev/deployment.md | 2 +- pyproject.toml | 4 +- src/idc_api/core/schema.py | 55 ++-------------------------- src/idc_api/core/services/_common.py | 2 +- src/idc_api/rest/app.py | 2 +- uv.lock | 14 +++---- 7 files changed, 20 insertions(+), 70 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cbe2301..13d77fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,16 +41,15 @@ Refactors, CI, and formatting land in the git history, not here. ### Changed -- **`idc-index` 0.13.3 / `idc-index-data` 25.0.4** (still IDC v25): a missing value in +- **`idc-index` 0.13.4 / `idc-index-data` 25.0.5** (still IDC v25): a missing value in `index.PatientAge`, `index.SeriesDescription`, `pt_index.SeriesType`, `collections_index.supporting_data` or `clinical_index.column_label` is now NULL, never an empty string, so SQL that tested `= ''` must test `IS NULL`. `volume_geometry_index` adds `origin_patient`, `ImageOrientationPatient` and `slice_spacing_mm`. `GET /v3/version` and - `get_idc_version` report `idc_index_data_version` `25.0.4`; collection, series, and size totals - are unchanged. -- `get_table_schema` / `list_tables` describe `prior_versions_index` and all its columns, which - upstream ships empty through `idc-index-data` 25.0.4. The description says it is not a record - of what is new: a row also ends when a version only moved storage bucket. + `get_idc_version` report `idc_index_data_version` `25.0.5`; collection, series, and size totals + are unchanged. `prior_versions_index` now has a table description and column descriptions, + which say it is not a record of what is new: a row also ends when a version only moved storage + bucket. ### Fixed diff --git a/dev/deployment.md b/dev/deployment.md index f56b346..69ea34a 100644 --- a/dev/deployment.md +++ b/dev/deployment.md @@ -160,7 +160,7 @@ python3 dev/smoke_openapi_examples.py "$URL" The image installs dependencies from [uv.lock](../uv.lock) (see [Dockerfile](../Dockerfile)), so it bakes exactly the `idc-index-data` the lock records and a rebuild of the same commit is deterministic. To move IDC versions, bump the exact `idc-index` pin in -[pyproject.toml](../pyproject.toml) (e.g. `idc-index==0.13.3`, which pulls a specific +[pyproject.toml](../pyproject.toml) (e.g. `idc-index==0.13.4`, which pulls a specific `idc-index-data`), run `uv lock` and commit both files — CI's `uv sync --locked` fails if the lock is stale — then **rebuild and redeploy** (steps 2–3). The running version is always reported at `/v3/version`. diff --git a/pyproject.toml b/pyproject.toml index 0732a7d..84408b2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,12 +11,12 @@ keywords = ["IDC", "DICOM", "cancer imaging", "MCP", "REST", "idc-index"] dependencies = [ # Query engine + data. # idc-index is pinned EXACTLY, unlike every other dependency here, because it transitively - # pins idc-index-data (0.13.3 -> 25.0.4) — which *is* the IDC data release this server + # pins idc-index-data (0.13.4 -> 25.0.5) — which *is* the IDC data release this server # advertises at /v3/version and documents in the changelog. The image installs from uv.lock, # so the lock alone already fixes it; the exact pin keeps the served IDC release explicit # here and stops a routine `uv lock --upgrade` from silently moving it. Bump this pin (then # `uv lock`) to move IDC versions — see dev/deployment.md § Updating for a new IDC release. - "idc-index==0.13.3", + "idc-index==0.13.4", "duckdb>=1.5.5", "pyarrow>=25.0.0", # Shared models / settings diff --git a/src/idc_api/core/schema.py b/src/idc_api/core/schema.py index 38209ae..ac61dad 100644 --- a/src/idc_api/core/schema.py +++ b/src/idc_api/core/schema.py @@ -193,53 +193,6 @@ def _column_type(c: dict, struct_types: dict[str, str] | None = None) -> str: ), } -# Fill-ins for upstream descriptions that are empty, applied only where upstream has no text, so -# the upstream description takes over on the pin bump that brings it. The prior_versions_index -# text matches what idc-index-data adds for its next release (upstream ships it empty through -# 25.0.4), with the main index named `index` as this server's SQL calls it. -TABLE_DESCRIPTION_FILLINS: dict[str, str] = { - "prior_versions_index": ( - "Versions of IDC series that an earlier IDC release served and the current release no " - "longer serves, each with the range of releases that served it. A version is identified " - "by crdc_series_uuid. A SeriesInstanceUID that is also in `index` was revised (its " - "current version is in `index`); one that is not was removed from IDC. A version can " - "span more than one row, since a new row starts whenever its storage bucket changed " - "between releases, even though its content did not. This table is not a record of what " - "is new in a release: it never lists added series, and a row ending at a release does " - "not mean that release changed the series. To find the series added or revised in a " - "release, use series_init_idc_version and series_revised_idc_version in `index`, or " - "get_release_changes for a whole release. The main use of this table is to resolve and " - "download series listed in manifests made from earlier releases." - ), -} - -COLUMN_DESCRIPTION_FILLINS: dict[str, dict[str, str]] = { - "prior_versions_index": { - "collection_id": "unique identifier of the collection the series belonged to", - "PatientID": "identifier of the patient within the collection (DICOM attribute PatientID)", - "SeriesInstanceUID": "DICOM SeriesInstanceUID of the series; the same for every version " - "of a series, so it also matches the current version in `index` if the series was " - "revised rather than removed", - "StudyInstanceUID": "DICOM StudyInstanceUID of the study the series belongs to", - "Modality": "DICOM Modality of the series", - "gcs_bucket": "name of the Google Cloud Storage bucket that held this version of the " - "series in the releases min_idc_version through max_idc_version", - "crdc_series_uuid": "unique identifier of this version of the series within IDC; " - "changes whenever the content of the series changes, so it never matches a " - "crdc_series_uuid in `index`. The same value on several rows is one version that moved " - "between storage buckets", - "series_size_MB": "total size of this version of the series in megabytes", - "series_aws_url": "public AWS S3 URL to download this version of the series in bulk " - "(each instance is a separate file)", - "aws_bucket": "name of the AWS S3 bucket that contains this version of the series", - "min_idc_version": "first IDC release (as an integer) in this row's range: this " - "version, in this storage bucket, was served from this release on", - "max_idc_version": "last IDC release (as an integer) in this row's range. The next " - "release revised the series, removed it, or moved this version to another storage " - "bucket (which starts a new row)", - }, -} - # A handful of columns per table surfaced by list_tables, so a caller learns they exist without # having to call get_table_schema first (a caller that guesses the obvious columns right never # does). Not a full listing; names absent from a table's schema are dropped. @@ -272,8 +225,7 @@ def _column_type(c: dict, struct_types: dict[str, str] | None = None) -> str: def table_schema(table: str) -> dict: """Return ``{name, description, columns:[{name,type,description}]}`` for a table, sourced from the idc-index schema JSON shipped in INDEX_METADATA (table descriptions may be - repointed inward via ``TABLE_DESCRIPTION_OVERRIDES``; empty descriptions are filled from - ``TABLE_DESCRIPTION_FILLINS`` / ``COLUMN_DESCRIPTION_FILLINS``).""" + repointed inward via ``TABLE_DESCRIPTION_OVERRIDES``).""" meta = idc_index_data.INDEX_METADATA[metadata_key(table)] schema = meta.get("schema", {}) or {} cols = schema.get("columns", []) @@ -281,12 +233,11 @@ def table_schema(table: str) -> dict: struct_types = ( _parquet_column_types(table) if any(c.get("type") == "RECORD" for c in cols) else {} ) - fillins = COLUMN_DESCRIPTION_FILLINS.get(table, {}) columns = [ { "name": c["name"], "type": _column_type(c, struct_types), - "description": c.get("description", "") or fillins.get(c["name"], ""), + "description": c.get("description", "") or "", } for c in cols ] @@ -294,7 +245,7 @@ def table_schema(table: str) -> dict: "name": table, "description": TABLE_DESCRIPTION_OVERRIDES.get(table) or schema.get("table_description", "") - or TABLE_DESCRIPTION_FILLINS.get(table, ""), + or "", "columns": columns, } diff --git a/src/idc_api/core/services/_common.py b/src/idc_api/core/services/_common.py index cc316f4..a25a1f3 100644 --- a/src/idc_api/core/services/_common.py +++ b/src/idc_api/core/services/_common.py @@ -15,5 +15,5 @@ def mb_to_tb(mb: float) -> float: def served_idc_release() -> int: """The IDC data release this server serves: the major version of the pinned - ``idc-index-data`` (25.0.4 → 25).""" + ``idc-index-data`` (25.0.5 → 25).""" return int(idc_index_data.__version__.split(".")[0]) diff --git a/src/idc_api/rest/app.py b/src/idc_api/rest/app.py index 01602fe..a4f1a04 100644 --- a/src/idc_api/rest/app.py +++ b/src/idc_api/rest/app.py @@ -389,7 +389,7 @@ def health(): "summary": "Illustrative — not live values", "value": { "idc_version": "v25", - "idc_index_data_version": "25.0.4", + "idc_index_data_version": "25.0.5", "api_version": "3.0.0", "build": "a1b2c3d", }, diff --git a/uv.lock b/uv.lock index 212e649..80879be 100644 --- a/uv.lock +++ b/uv.lock @@ -543,7 +543,7 @@ requires-dist = [ { name = "duckdb", specifier = ">=1.5.5" }, { name = "fastapi", specifier = ">=0.141.1" }, { name = "httpx", marker = "extra == 'dev'", specifier = ">=0.27" }, - { name = "idc-index", specifier = "==0.13.3" }, + { name = "idc-index", specifier = "==0.13.4" }, { name = "mcp", specifier = ">=1.29.0,<2" }, { name = "pip-audit", marker = "extra == 'dev'", specifier = ">=2.7" }, { name = "pyarrow", specifier = ">=25.0.0" }, @@ -558,7 +558,7 @@ provides-extras = ["dev"] [[package]] name = "idc-index" -version = "0.13.3" +version = "0.13.4" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "click" }, @@ -573,18 +573,18 @@ dependencies = [ { name = "s5cmd" }, { name = "tqdm" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/c9/00/b15559ff69a11dc38ebd7b0c1bb433a1949b0c5f5472e60a7dec7f5b505b/idc_index-0.13.3.tar.gz", hash = "sha256:ea5c420ab65c026ff298f0c1c1234c2a56f40edff6f35094b772fa13a70b7180", size = 65167, upload-time = "2026-10-19T20:25:42.885Z" } +sdist = { url = "https://files.pythonhosted.org/packages/c9/b1/5502b2f13928b92ab420d23931d78b52f7d9fb0e466672d31b3b2ee54aa6/idc_index-0.13.4.tar.gz", hash = "sha256:1f606b663a45fb1125a5ae56228375c37f62d323b0e51b3aa85713174c3b6d98", size = 65869, upload-time = "2026-10-20T02:24:25.451Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/71/09/aefa662061248a462f5dfbaa9a690b5e2ae223052ed4888c323ee34fe2b9/idc_index-0.13.3-py3-none-any.whl", hash = "sha256:13132f53788c7a28f65c041679c50cf3460532eb460f780b2974e5d43358dd91", size = 31219, upload-time = "2026-10-19T20:25:41.501Z" }, + { url = "https://files.pythonhosted.org/packages/74/fd/066cf77cf1740d037538eead0066f2a5577d6c2b8b51358565641517a6b2/idc_index-0.13.4-py3-none-any.whl", hash = "sha256:dce52b945e1874c06c50ec22b86888332173cf6b84975e17b555237a87abb2f2", size = 31220, upload-time = "2026-10-20T02:24:24.14Z" }, ] [[package]] name = "idc-index-data" -version = "25.0.4" +version = "25.0.5" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/d4/f5/2c8de62a18c1217da9658706705d05f971429390e6e72461045823f37f4e/idc_index_data-25.0.4.tar.gz", hash = "sha256:4e67517d30074ed77d30ba2cc09c8ca26a56d985b3a8487ad828e445d7ecc04b", size = 79260358, upload-time = "2026-10-19T19:13:20.401Z" } +sdist = { url = "https://files.pythonhosted.org/packages/26/61/87147ca0cfff3bb1ba006dd19b27b63ea37844d77785ecf23eba8fb3e2ad/idc_index_data-25.0.5.tar.gz", hash = "sha256:a7ac0f81f995dc3880cc9c9825afeb4b26afa0e2c32290b3f0255cc1d855ae4a", size = 79225036, upload-time = "2026-10-20T01:28:42.187Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/8a/b1/85df4c3c6a2360090b7c39455f0647e17960ed35d8b8685177847d2ccecf/idc_index_data-25.0.4-py3-none-any.whl", hash = "sha256:37292b2225b3c96feaed84dec0957ce094d30ed3b0d3a290f6e09d8f7b37eab3", size = 79222881, upload-time = "2026-10-19T19:13:15.165Z" }, + { url = "https://files.pythonhosted.org/packages/bd/10/c6391b44d1b2f6c744e1cb4018250cb1a5d80947d47ed945172d9165a3b4/idc_index_data-25.0.5-py3-none-any.whl", hash = "sha256:1ea4ec812688759b71798249b6488eb2bce7178aabb3dccb00366b7a555de25c", size = 79186941, upload-time = "2026-10-20T01:28:38.372Z" }, ] [[package]] From 69e6fe7f03e0e5f879811b9166ff717fcd41dbf7 Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Mon, 19 Oct 2026 22:37:18 -0400 Subject: [PATCH 5/6] docs(releases): say analysis results with changed series, not gained Since analysis results report revised and removed series too, the tool and route descriptions understated them. Also spell Unparsable as codespell expects. Co-Authored-By: Claude Opus 5.5 --- src/idc_api/mcp/server.py | 2 +- src/idc_api/rest/app.py | 2 +- tests/test_releases.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/idc_api/mcp/server.py b/src/idc_api/mcp/server.py index f88aaaf..9c8feaf 100644 --- a/src/idc_api/mcp/server.py +++ b/src/idc_api/mcp/server.py @@ -268,7 +268,7 @@ def get_stats() -> dict: def get_release_changes(version: int | str | None = None) -> dict: """What's new in an IDC data release — what changed since the previous version. Returns the series added, revised, and removed (with size in TB and patients affected), the collections - that were newly added, updated, or removed, and the analysis results that gained series. + that were newly added, updated, or removed, and the analysis results with changed series. `version` is the release, as 24 or "v24" (the form get_idc_version and `previous_version` use); omit it for the release this server serves. Call this whenever the user asks what is new, added, changed, updated, or removed in IDC, in a given release, or since a version — do diff --git a/src/idc_api/rest/app.py b/src/idc_api/rest/app.py index a4f1a04..9da4a1c 100644 --- a/src/idc_api/rest/app.py +++ b/src/idc_api/rest/app.py @@ -456,7 +456,7 @@ def release_changes( ): """What's new in an IDC data release relative to the previous one: series added, revised, and removed (with size in TB and patients affected), collections that were - newly added, updated, or removed, and analysis results that gained series. Computed from + newly added, updated, or removed, and analysis results with changed series. Computed from the served release's own tables (`index` plus `prior_versions_index`), so any past release can be described, not just the latest.""" return C().releases.release_changes(version) diff --git a/tests/test_releases.py b/tests/test_releases.py index d894485..919b1b2 100644 --- a/tests/test_releases.py +++ b/tests/test_releases.py @@ -171,7 +171,7 @@ async def test_whats_new_prompt(): assert "whats_new" in prompts msg = await mcp.get_prompt("whats_new", {"version": "v24"}) assert "get_release_changes(version=24)" in msg.messages[0].content.text - # Unparseable input falls back to the served release instead of reaching the call. + # Unparsable input falls back to the served release instead of reaching the call. msg = await mcp.get_prompt("whats_new", {"version": "latest"}) assert "get_release_changes()" in msg.messages[0].content.text From b6fc0ef78b1c7450be279742e5cc96d68227934f Mon Sep 17 00:00:00 2001 From: Andrey Fedorov Date: Mon, 19 Oct 2026 22:42:46 -0400 Subject: [PATCH 6/6] fix(releases): reject overlong versions; qualify per-series guidance Copilot review of #52: - parse_idc_version caps the digit count at 4. A digit string past Python's int() conversion limit (4,300 digits) raised ValueError, so REST returned 500 rather than 400 and the whats_new fallback missed it. - "Use index for per-series detail" covered only series still served. The tool description, _NOTE, idc://guide and the user guide now say removed series and earlier versions of revised ones are only in prior_versions_index, and how to compare two releases there: build each from both tables, then compare membership and crdc_series_uuid. Co-Authored-By: Claude Opus 5.5 --- docs/user-guide.md | 16 +++++++++++----- src/idc_api/core/services/releases.py | 11 +++++++---- src/idc_api/mcp/server.py | 24 +++++++++++++++--------- tests/test_releases.py | 7 +++++-- 4 files changed, 38 insertions(+), 20 deletions(-) diff --git a/docs/user-guide.md b/docs/user-guide.md index 8541401..19ad3f9 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -252,9 +252,10 @@ from one and added to the other in the per-collection rows, but not in the total and v2 carry a `warnings` entry: series versions served in v1 and replaced later were never recorded, so those two diffs are incomplete. -For per-series detail, query `index` with SQL: `series_init_idc_version` is the release a series -first appeared in, and `series_revised_idc_version` the release its current content dates from. -For example, the series of a collection that changed since v20: +For per-series detail of the series still served, query `index` with SQL: +`series_init_idc_version` is the release a series first appeared in, and +`series_revised_idc_version` the release its current content dates from. For example, the series +of a collection that changed since v20: ```sql SELECT SeriesInstanceUID, series_init_idc_version, series_revised_idc_version @@ -266,8 +267,13 @@ WHERE collection_id = 'nlst' AND series_revised_idc_version > 20 earlier releases can still be resolved. It is **not** a record of what is new: it never lists added series, and a row also ends when a version only moved storage bucket, so counting rows by `max_idc_version` overstates revisions (for v20 it would count 11,928, of which 11,338 were -bucket moves). -`get_release_changes` accounts for this. +bucket moves). `get_release_changes` accounts for this. + +Removed series, and earlier versions of revised ones, are only in `prior_versions_index`. To +compare two releases series by series, build each release from both tables (an `index` row is in +release R when `series_revised_idc_version <= R`, a `prior_versions_index` row when +`min_idc_version <= R <= max_idc_version`), then compare which series are present and, for those +in both, their `crdc_series_uuid`. The diff tells you *what* changed, not *why*; for the narrative, see the [IDC release notes](https://learn.canceridc.dev/data/data-release-notes). diff --git a/src/idc_api/core/services/releases.py b/src/idc_api/core/services/releases.py index fbe5218..cea477a 100644 --- a/src/idc_api/core/services/releases.py +++ b/src/idc_api/core/services/releases.py @@ -96,9 +96,11 @@ _NOTE = ( "Computed from the current release: series in `index` plus the series versions no longer " "served in `prior_versions_index`. 'revised' means the series is in both releases with " - "different content (a new crdc_series_uuid). Do not derive changes from " - "prior_versions_index row ranges: a row also ends when a version only moved storage bucket. " - "For per-series detail, use series_init_idc_version / series_revised_idc_version on `index`." + "different content (a new crdc_series_uuid). For per-series detail, series_init_idc_version " + "/ series_revised_idc_version on `index` cover the series still served; removed series and " + "earlier versions of revised ones are only in prior_versions_index. Compare releases there " + "by membership and crdc_series_uuid, not by row ranges: a row also ends when a version only " + "moved storage bucket." ) @@ -106,7 +108,8 @@ def parse_idc_version(version: int | str) -> int: """An IDC release number from ``24``, ``"24"`` or ``"v24"``; raises InvalidQueryError otherwise.""" s = str(version).strip().lower().removeprefix("v") - if not (s.isascii() and s.isdigit()): + # The length cap keeps int() off digit strings it refuses to convert (over 4,300 digits). + if not (s.isascii() and s.isdigit() and len(s) <= 4): raise InvalidQueryError(f"Invalid IDC version: {version!r}. Use a release such as 24.") return int(s) diff --git a/src/idc_api/mcp/server.py b/src/idc_api/mcp/server.py index 9c8feaf..c7dd516 100644 --- a/src/idc_api/mcp/server.py +++ b/src/idc_api/mcp/server.py @@ -272,10 +272,12 @@ def get_release_changes(version: int | str | None = None) -> dict: `version` is the release, as 24 or "v24" (the form get_idc_version and `previous_version` use); omit it for the release this server serves. Call this whenever the user asks what is new, added, changed, updated, or removed in IDC, in a given release, or since a version — do - not conclude that history is unavailable because the server serves a single release, and do - not work it out from `prior_versions_index` row ranges (a row also ends when a series only - moved storage bucket). Surface any `warnings`. For per-series detail beyond this summary, - use run_sql on `index` (series_init_idc_version / series_revised_idc_version).""" + not conclude that history is unavailable because the server serves a single release. + Surface any `warnings`. For per-series detail beyond this summary, use run_sql: `index` + (series_init_idc_version / series_revised_idc_version) covers the series still served; + removed series and earlier versions of revised ones are only in `prior_versions_index`. + Compare releases there by membership and crdc_series_uuid, never by its row ranges (a row + also ends when a series only moved storage bucket).""" return ctx.releases.release_changes(version).model_dump(mode="json") @@ -675,11 +677,15 @@ def whats_new(version: str = "") -> str: and `series_revised_idc_version` the release its current content dates from; `version_metadata_index` dates each release. For "what's new in vN", call `get_release_changes(version=N)`: it diffs release N against N-1 — series added / revised / removed, new and removed collections, -per-collection size. For per-series detail use `run_sql` on the two `index` columns (e.g. "which -of my cohort's series changed since v20": `series_revised_idc_version > 20`). -`prior_versions_index` lists the series versions no longer served (for resolving old manifests). -It is not a record of what is new: it never lists added series, and a row also ends when a -version only moved storage bucket, so do not count changes from its `max_idc_version`. +per-collection size. For per-series detail of the series still served, use `run_sql` on the two +`index` columns (e.g. "which of my cohort's series changed since v20": +`series_revised_idc_version > 20`). Removed series and earlier versions of revised ones are only +in `prior_versions_index`, which lists the series versions no longer served (it also resolves old +manifests). It is not a record of what is new: it never lists added series, and a row also ends +when a version only moved storage bucket, so do not count changes from its `max_idc_version`. To +compare releases N-1 and N per series, build each from both tables (an `index` row is in release +R when `series_revised_idc_version <= R`, a `prior_versions_index` row when +`min_idc_version <= R <= max_idc_version`) and compare membership and `crdc_series_uuid`. """ diff --git a/tests/test_releases.py b/tests/test_releases.py index 919b1b2..c1bc3d2 100644 --- a/tests/test_releases.py +++ b/tests/test_releases.py @@ -125,7 +125,8 @@ def test_version_accepts_v_prefix(ctx): assert ctx.releases.release_changes(f"v{cur}") == ctx.releases.release_changes(cur) -@pytest.mark.parametrize("bad", [0, 9999, "latest", "v", "²", "v-1"]) +# "9" * 5000 is past the digit count int() will convert from a string. +@pytest.mark.parametrize("bad", [0, 9999, "latest", "v", "²", "v-1", "9" * 5000]) def test_unknown_version_is_a_clean_error(ctx, bad): with pytest.raises(InvalidQueryError): ctx.releases.release_changes(bad) @@ -148,7 +149,7 @@ async def test_both_adapters_accept_the_version_they_return(ctx, client, parse_m assert mcp_out["idc_version"] == prev -@pytest.mark.parametrize("bad", [9999, "latest", "²"]) +@pytest.mark.parametrize("bad", [9999, "latest", "²", "9" * 5000]) def test_rest_bad_version_is_400(client, bad): r = client.get("/v3/releases/changes", params={"version": bad}) assert r.status_code == 400 @@ -174,6 +175,8 @@ async def test_whats_new_prompt(): # Unparsable input falls back to the served release instead of reaching the call. msg = await mcp.get_prompt("whats_new", {"version": "latest"}) assert "get_release_changes()" in msg.messages[0].content.text + msg = await mcp.get_prompt("whats_new", {"version": "9" * 5000}) + assert "get_release_changes()" in msg.messages[0].content.text def test_prior_versions_index_is_documented():