Skip to content

switch to swift 6 -- prevent concurrency issues, fix concurrency issues#165

Merged
davidkoski merged 3 commits into
mainfrom
test-fixes
Apr 1, 2026
Merged

switch to swift 6 -- prevent concurrency issues, fix concurrency issues#165
davidkoski merged 3 commits into
mainfrom
test-fixes

Conversation

@davidkoski

Copy link
Copy Markdown
Collaborator

Proposed changes

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

- some concurrency issues snuck in for test support
- fix and prevent more from coming in by switching Package to swift 6
- depends on ml-explore/mlx-swift#379
@davidkoski davidkoski requested a review from angeloskath March 27, 2026 23:08
/// concurrently with an active ``respond(to:role:images:videos:)`` or
/// ``streamResponse(_:)`` call on the same session. To persist the cache
/// across process launches, use ``saveCache(to:)`` instead.
public func currentCache() async -> [KVCache]? {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was a little too permissive -- it is added for test support so we can just switch it to an internal scoped visitor.

let session = ChatSession(model(), generateParameters: generationParameters)
await session.withCache { cache in
XCTAssertNil(cache)
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is the pattern to fix the tests -- just use the visitor.


func testCurrentCacheAfterGeneration() async throws {
let session = ChatSession(model())
let session = ChatSession(model(), generateParameters: generationParameters)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

See #128, those changes were in flight while these calls were added. This just syncs everything up -- this is needed to make the random weight token generation stop.

import Testing

private let cacheCreators: [() -> any KVCache] = [
private let cacheCreators: [@Sendable () -> any KVCache] = [

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fix sendable issue

Comment thread Package.swift
@@ -1,4 +1,4 @@
// swift-tools-version: 5.12
// swift-tools-version: 6.1

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Switch to swift 6 -- prevent concurrency issues from making it past CI

Comment thread Package.swift
"README.md"
],
swiftSettings: [
.enableExperimentalFeature("StrictConcurrency")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No longer needed.

@angeloskath angeloskath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@davidkoski davidkoski merged commit 25b00d4 into main Apr 1, 2026
2 checks passed
@davidkoski davidkoski deleted the test-fixes branch April 1, 2026 20:12
jolexxa added a commit to hobbyfarm-ai/mlx-swift-lm that referenced this pull request Apr 20, 2026
Merges 48 upstream commits including:
- Gemma 4 text + vision model support (ml-explore#180, ml-explore#185, ml-explore#211, ml-explore#212)
- Swift 6 migration (ml-explore#165)
- Speculative decoding (ml-explore#173)
- Prompt cache serialization (ml-explore#155)
- KVCache copy() protocol requirement (ml-explore#158)
- Various model fixes and tool calling improvements

Conflicts resolved:
- KVCache.swift: kept both batch cache classes (ours) and
  serialization code (upstream), added copy() to BatchKVCache
  and BatchRotatingKVCache
- Load.swift: merged upstream's loadArraysAndMetadata + metadata
  parameter with our progress handler reporting

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

2 participants