feat(api)!: remove the local-download surface (POST /v3/download, download_cohort)#24
Merged
Conversation
…nload_cohort) Both only worked when the server ran on the caller's own machine and errored everywhere else — on the hosted deployment (the common case) the tool's mere presence misled agents into calling it, and every doc surface spent its words steering away from it. Downloading through the server is also never the right path: all IDC buckets are public, so direct S3/GCS transfer (idc CLI / s5cmd) is strictly more efficient. Retrieval is now manifests/URLs only on every surface: get_cohort_urls / POST /v3/cohort/manifest.txt, or the ready-to-run idc commands already in the build_cohort response. Removes DownloadService, the enable_local_download setting (IDC_API_ENABLE_LOCAL_DOWNLOAD), and the now-unused UnsupportedOperationError; tests guard against reintroduction. Beta contract change (3.0.0b1), recorded in the changelog; api_v3_plan.md carries a superseded note on the original decision. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR removes the “local download” API/tooling surface area from both REST and MCP, aligning retrieval behavior across all deployment modes to be manifests/URLs only (direct public S3/GCS transfers via idc CLI / s5cmd), and updates tests + docs to prevent accidental reintroduction.
Changes:
- Removed
POST /v3/download, its request model, and the MCPdownload_cohorttool, along with the associated coreDownloadService, setting flag, andUnsupportedOperationError. - Standardized retrieval guidance across MCP resources and documentation to “URLs/manifests only” in all modes.
- Updated tests to assert the REST endpoint returns 404 and the MCP tool is absent.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_rest.py | Replaces the prior 501 behavior assertion with a 404 assertion for the removed /v3/download endpoint. |
| tests/test_mcp.py | Removes download_cohort from the expected tool set and asserts it is not registered. |
| src/idc_api/settings.py | Removes the enable_local_download deployment-mode setting. |
| src/idc_api/rest/app.py | Deletes the DownloadRequest model and the /v3/download route. |
| src/idc_api/mcp/server.py | Removes the download_cohort tool and updates transport/retrieval guidance and guide text. |
| src/idc_api/core/services/manifest.py | Updates the module docstring to reflect manifests/URLs-only retrieval. |
| src/idc_api/core/services/download.py | Deletes the local-only download service implementation. |
| src/idc_api/core/services/init.py | Stops exporting DownloadService. |
| src/idc_api/core/errors.py | Removes UnsupportedOperationError (501) since the capability is removed. |
| src/idc_api/core/context.py | Removes DownloadService wiring from the application context. |
| README.md | Updates MCP launch text to remove local-download claims. |
| docs/user-guide.md | Removes REST/MCP local-download documentation and simplifies retrieval guidance. |
| dev/developer_guide.md | Removes the ENABLE_LOCAL_DOWNLOAD environment variable documentation. |
| dev/architecture.md | Updates architecture notes and deployment-mode table to reflect manifests/URLs-only retrieval. |
| dev/api_v3_plan.md | Marks the local-download surface as superseded during beta and removes planned usage references. |
| CHANGELOG.md | Adds an Unreleased “Removed” entry documenting the breaking (beta) removal and guidance alternatives. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+3
| """Manifest generation: public series URLs (AWS + GCS), s5cmd-style manifest text, and | ||
| ready-to-run ``idc`` CLI commands for a cohort. The service never moves bytes — local | ||
| download lives in ``download.py`` (MCP local mode only).""" | ||
| ready-to-run ``idc`` CLI commands for a cohort. The service never moves bytes — callers | ||
| download directly from the public S3/GCS buckets (`idc` CLI / s5cmd).""" |
Member
Author
There was a problem hiding this comment.
Fixed in 51f9da3 — the docstring now uses reST double-backticks throughout.
Addresses Copilot review on #24. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The local-download surface only worked when the server ran on the caller's own machine and errored everywhere else. That made it an attractive nuisance:
download_cohortexisted in the tool schema but could only ever return an error. Agents select tools by schema presence first, so some sessions burned a turn calling it and recovering.download_diris a path on the server's filesystem, which is only coherent when caller == server host.idcCLI / s5cmd) is strictly more efficient, and every doc surface already steered callers there.This is a beta contract change (
3.0.0b1), made now while the contract can still move.What changed
POST /v3/download+DownloadRequest(REST), thedownload_cohortMCP tool,DownloadService, theenable_local_downloadsetting (IDC_API_ENABLE_LOCAL_DOWNLOAD), and the now-unusedUnsupportedOperationError(501).get_cohort_urls/POST /v3/cohort/manifest.txt, or the ready-to-runidccommands already in thebuild_cohortresponse. Thedownloadpayload in the cohort response (DownloadInfo) is untouched.INSTRUCTIONSrule 3 and theidc://guideRetrieval family / workflow step 4 lost their local-vs-hosted caveats; the server now plainly never moves files./v3/downloadasserted 404,download_cohortasserted absent from the tool list — accidental reintroduction fails CI.dev/architecture.mddeployment-modes table now shows identical retrieval in every mode;dev/api_v3_plan.mdkeeps its original decision text with an explicit superseded during beta note; CHANGELOG entry under[Unreleased]→Removed.Release prep (added after review started)
This PR now also cuts 3.0.0b2 (
chore(release): 3.0.0b2): version bump inpyproject.toml+uv.lock, and the[Unreleased]changelog entries folded under## [3.0.0b2] — 2026-07-14. Merging this is step 1 ("bump and curate") of the release runbook indev/deployment.md— after merge: promote to test, verify/v3/versionreports3.0.0b2, then tagv3.0.0b2. Follow-up commits also add apip install idc-indexhint to the agent-facing download notes.Notes for deploy
The production MCP connector keeps advertising
download_cohortuntil redeployed; clients pick up the new tool list per session after that.Verification
pytest tests -q— 74 passedruff check/ruff format --check— clean🤖 Generated with Claude Code