Conversation
Carry the legacy AbortSignal and threading.Event contracts into the v2 bindings so callers can stop native downloads instead of receiving progress-only APIs. Preserve completion races and callback exceptions while mapping signal-driven JavaScript cancellation to AbortError. Files changed: - sdk_v2/js/native/src/model.cc: bridge AbortSignal state into the native progress callback - sdk_v2/js/src and test: restore overloads and verify cancellation semantics - sdk_v2/python/src and test: restore cancel_event and preserve callback errors - sdk_v2/python/README.md: document cooperative download cancellation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d2c76d2-0c86-4672-b91b-3fcb54ed7906
Keep optional AbortSignal and callback-or-undefined variables accepted while rejecting duplicate signals, and make the Python cancellation example demonstrate cancellation.\n\nFiles changed:\n- sdk_v2/js/src/imodel.ts\n- sdk_v2/js/src/model.ts\n- sdk_v2/js/test/model-download.types.ts\n- sdk_v2/python/README.md\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: 2d2c76d2-0c86-4672-b91b-3fcb54ed7906
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
Cross-language cancellation, native threading, and JavaScript reference cleanup warrant final human validation.
Review effort: Balanced
Findings: None
What changed in this PR
Restores cooperative model-download cancellation across the JavaScript and Python v2 SDKs.
Changes:
- Adds JavaScript
AbortSignalsupport with native progress-callback bridging andAbortErrormapping. - Adds Python
threading.Eventcancellation while preserving callback exceptions. - Adds runtime, type-level, lifecycle, and unit coverage.
| File | Description |
|---|---|
sdk_v2/python/test/unit/test_model_download_cancellation.py |
Tests Python cancellation and callback exceptions. |
sdk_v2/python/src/foundry_local_sdk/imodel.py |
Implements event-driven cancellation. |
sdk_v2/python/README.md |
Documents Python cancellation usage. |
sdk_v2/js/tsconfig.types.json |
Includes new type tests. |
sdk_v2/js/test/model.test.ts |
Tests active and pre-start cancellation. |
sdk_v2/js/test/model-lifecycle.test.ts |
Tests cached-model cancellation races. |
sdk_v2/js/test/model-download.types.ts |
Verifies supported overloads. |
sdk_v2/js/src/model.ts |
Validates signals and exposes overloads. |
sdk_v2/js/src/imodel.ts |
Updates the public interface. |
sdk_v2/js/src/detail/native.ts |
Extends the native binding contract. |
sdk_v2/js/native/src/model.cc |
Bridges abort events into native cancellation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Keep AbortSignal as an optional second argument so existing IModel implementations remain source-compatible while model downloads still support cancellation. Files changed: - sdk_v2/js/src/imodel.ts - sdk_v2/js/src/model.ts - sdk_v2/js/test/model-download.types.ts - sdk_v2/js/test/model-lifecycle.test.ts - sdk_v2/js/test/model.test.ts Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d2c76d2-0c86-4672-b91b-3fcb54ed7906
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.
Summary
AbortSignal, including progress-callback bridging and late-abort-safeAbortErrormappingthreading.Eventwhile preserving user progress-callback exceptionsValidation
FOUNDRY_TEST_DATA_DIR)git diff --checkThe full Biome formatter check on the Windows checkout reports pre-existing CRLF normalization differences; focused lint passes without suppressions.
Real-download cancellation stress
Each API used its own empty cache and the real
qwen2.5-0.5bregistry download. The matrix included two pre-start cancellations and eight active cancellations at thresholds from 0% through 3%, verifying progress occurred and the model remained uncached after every cancellation.AbortSignalthreading.EventThe successful recovery downloads reused the same partially downloaded cache after all cancellation attempts, validating cleanup/resume behavior. WSL cancellation used separate empty caches; full recovery was not repeated there to avoid two additional full model downloads.