Skip to content

feat: release history via get_release_changes - #52

Draft
fedorov wants to merge 1 commit into
mainfrom
feat/release-changes
Draft

fedorov wants to merge 1 commit into
mainfrom
feat/release-changes

Conversation

@fedorov

@fedorov fedorov commented Sep 29, 2026

Copy link
Copy Markdown
Member

Closes #40.

Summary

The served release already carries the full history of earlier releases, but nothing on the tool surface pointed at it. This PR turns that into a first-class capability and fixes the discoverability gaps the issue diagnosed.

  • New: GET /v3/releases/changes?version=N / MCP get_release_changes(version) — diff of release N vs N-1: series added / revised / removed (TB, patients affected), collections new / updated / removed, analysis results that gained series. Defaults to the served release; accepts 24 or "v24".
  • How: every series version IDC ever served is either the current row in index (valid from series_revised_idc_version) or a row in prior_versions_index (valid min_idc_version–max_idc_version). Comparing which versions span N vs N-1 gives an exact diff, including removed series, which the issue's query can't see.
  • Discoverability (recs 2–5): get_idc_version description points at release history; one-line INSTRUCTIONS touch + "Release history" section in idc://guide; whats_new MCP prompt; prior_versions_index table/column descriptions (fill-ins only where upstream is empty); notable_columns on list_tables entries.
  • Not done (rec 6): per-release announcement URL/DOI — no such data in the index; belongs upstream in idc-index-data. The user guide links the IDC release notes page instead.

Validation

For v24 the service reproduces the issue's numbers exactly (39,872 added, 5.66 TB, 15 new collections, 7 revised in acrin_nsclc_fdg_pet / anti_pd_1_lung) and additionally reports 1,034 series removed from bonemarrowwsi_pediatricleukemia. Query runs in ~0.08s.

New tests/test_releases.py: self-consistency invariants (totals = per-collection sums; added + revised = series with series_revised_idc_version = current), v1 has no predecessor, bad versions are clean errors (400 on REST), core == REST == MCP parity, and discoverability checks (tool description vocabulary, prompt, schema docs, notable columns exist). Full suite: 109 passed.

Known limitation

analysis_results for past releases counts only series still present today (prior_versions_index has no analysis_result_id); documented in the field description.

Docs: docs/user-guide.md (new Release history section, endpoint/tool tables), CHANGELOG.md under [Unreleased].

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
@fedorov
fedorov force-pushed the feat/release-changes branch from 7c7c804 to eb7b1ac Compare September 30, 2026 02:37

@fedorov fedorov left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: correct for the served release, wrong for past ones

For the served release (v24) the diff matches the issue's numbers. For earlier releases, two problems in the diff SQL cause wrong results. The tests only check the served release, which is the one case where neither problem can happen.

Fix before merge

  1. Bucket moves reported as revisions (releases.py:38). prior_versions_index splits one content version into two rows when only the storage bucket changes. Of the 2,843 series reported as revised in v20, 2,253 have the same crdc_series_uuid in v19: they were moved from public-datasets-idc to idc-open-data, not changed.
  2. Series that move between collections are misclassified (releases.py:32). At v5, apollo goes from 43 series to 0 and apollo_5_lscc from 0 to 43. The diff reports apollo_5_lscc as updated (43 revised) and doesn't list apollo at all.
  3. MCP version: int rejects "v23" (server.py:264). That is the format the tool itself returns in previous_version. The PR description says "v24" is accepted, but the core's handling of the v prefix can't be reached from either adapter.
  4. The new prior_versions_index docs describe one row per version (schema.py:155/172, _GUIDE, user guide). The data has 2,253 uuids with two rows each, so SQL written from these docs will repeat problem 1.

Also worth fixing: patients_affected isn't scoped to collection. The analysis_results section and the collections section can disagree. "Current version" is read from a different source than get_idc_version. The whats_new prompt normalizes the version in the adapter. isdigit() accepts characters like '²'. The query is recomputed on every call. There are also a few nits. Details are in the inline comments.

All numbers above were checked with DuckDB against idc-index-data 24.2.2.


🤖 Drafted with Claude Code (automated review, posted at my request).

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,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bucket moves are reported as content revisions. c.lo = N marks a series as revised whenever its current version row starts at N, but prior_versions_index also starts a new row when only the storage bucket changes. It contains 2,253 crdc_series_uuids with two contiguous rows each (e.g. 19–19 and 20–21).

At v20 this reports series_revised = 2,843. 2,253 of those have the same crdc_series_uuid in v19: they only moved from public-datasets-idc to idc-open-data (mostly cptac_ccrcc, cptac_ucec, cptac_pda and ccdi_mci). The real count is about 590, and those collections are wrongly shown as updated.

Fix: carry crdc_series_uuid in v and use WHEN c.crdc_series_uuid <> p.crdc_series_uuid THEN 'revised', or merge adjacent prior rows with the same uuid first.

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,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Series that move between collections get attributed to the new collection. COALESCE(c.collection_id, p.collection_id) assigns each joined row to its current collection, so the previous-release membership is credited to the new collection.

v5 example: apollo goes from 43 series to 0 and apollo_5_lscc from 0 to 43. This returns apollo_5_lscc as updated with 43 revised, leaves it out of new_collections, returns removed_collections=[], and has no row for apollo.

Fix: join on (SeriesInstanceUID, collection_id) (a move then counts as removed from one collection and added to the other), or compute collection presence for each release separately.

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

count(DISTINCT PatientID) isn't scoped to collection. PatientIDs are only unique within a collection (80 PatientIDs in the current index appear in more than one), so the top-level total merges different patients and can be less than the sum of the per-collection counts. Suggest count(DISTINCT (collection_id, PatientID)).

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 "

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section uses series_init_idc_version on current index rows, while the collections section uses version rows. So:

  • is_new = min(init) = N ignores analysis-result series that were later removed, so a result can be reported as new in a release after the one where it actually first appeared.
  • An analysis result whose series were only revised in N shows up as revisions in collections but is missing from analysis_results.

Deriving both sections from the same v/cur/prev sets would keep them consistent.


def release_changes(self, version: int | str | None = None) -> ReleaseChanges:
dates = self._release_dates()
current = max(dates)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Current version" comes from max(version_metadata_index.idc_version) here, but DiscoveryService.version() (what get_idc_version returns) derives it from the idc-index-data version. If an idc-index-data release ever ships a version_metadata_index that is out of step with its own version, current_version here will differ from get_idc_version. It is also bound as hi for every index row, so the default call would describe the wrong release. Consider reusing the same helper.

Comment thread src/idc_api/mcp/server.py

@mcp.tool()
@guard
def get_release_changes(version: int | None = None) -> dict:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

version: int | None makes pydantic reject "v23", which is the format this tool returns in idc_version / previous_version and the format get_idc_version returns. An agent that passes previous_version back in gets ToolError: Input should be a valid integer, and REST ?version=v23 returns 422. The core's v prefix handling (and its test) can't be reached from either adapter, even though the PR description says "v24" is accepted. Suggest int | str | None here and on the REST query param, and let _parse_version validate.

Comment thread src/idc_api/mcp/server.py
@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")

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This repeats the version normalization in the adapter with no validation (against the "adapters are thin" rule in CLAUDE.md), and pastes the raw text into the generated call. whats_new(version="latest") produces get_release_changes(version=latest). Suggest reusing the core _parse_version (made public) and falling back to the no-argument form on invalid input.

"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; "

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The data contradicts these descriptions. prior_versions_index is not one row per version: 2,253 crdc_series_uuids have two contiguous rows that differ only by bucket. So "the next release revised or removed it" is wrong for those rows (see the comment on releases.py:38). The same claim is in the table description (line 155), _GUIDE, and docs/user-guide.md. An LLM that follows these docs in run_sql (e.g. max_idc_version = 19 AND SeriesInstanceUID IN index meaning "revised in v20") will count those 2,253 bucket moves as revisions.

Also consider adding crdc_series_uuid to NOTABLE_COLUMNS for this table, since it's the column that tells the two cases apart.


class CollectionChange(BaseModel):
collection_id: str
status: str = Field(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: status only ever holds 'new' | 'removed' | 'updated'. Literal["new", "removed", "updated"] would validate it and put the enum in /v3/openapi.json and the MCP output schema.

Comment thread tests/test_releases.py
)


def test_latest_release_is_default_and_self_consistent(ctx):

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These invariants only run against the served release, which is the one case where neither the split-row problem nor the collection-move problem can happen, so both pass the suite. Suggest adding past-release checks, e.g.:

  • every series counted as revised in N has a different crdc_series_uuid in N-1 (catches the v20 bucket moves);
  • release_changes(5) lists apollo_5_lscc in new_collections and apollo in removed_collections.

@fedorov fedorov mentioned this pull request Oct 1, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve versioning discoverability

1 participant