-
Notifications
You must be signed in to change notification settings - Fork 4
Switch to pure Swift implementation (swift-tiktoken) #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Switch to pure Swift implementation (swift-tiktoken) #2
Conversation
- Add DePasqualeOrg/swift-tiktoken as package dependency - Create shim file re-exporting SwiftTiktoken as TiktokenSwift - Add typealias CoreBpe -> CoreBPE for API compatibility - Update platform requirements to match swift-tiktoken (macOS 14+, iOS 16+) Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
- Delete Sources/TiktokenFFI/ directory with static libraries and headers - Delete TiktokenFFI.xcframework/ with prebuilt binaries - Remove UniFFI-generated Swift bindings (TiktokenFFI.swift) - Remove custom encoding loaders (replaced by swift-tiktoken's EncodingLoader) - Delete test helpers that depended on old implementation - Reduces package size by ~50MB Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Auto code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Review SummaryPR Scope: Migration from FFI-based Rust implementation to pure Swift implementation using . Changes Reviewed:
Validation:
Notes:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e690c3dc69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /// Helper utilities for using Tiktoken | ||
| public struct TiktokenHelper { | ||
| /// Creates a test encoder with a small vocabulary | ||
| /// - Note: This is for testing only. In production, load real encoding data. | ||
| public static func createTestEncoder() throws -> CoreBpe { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep TiktokenHelper or adjust callers
Removing TiktokenHelper breaks the example app (and any external code) that still calls TiktokenHelper.createTestEncoder() as a fallback when downloads fail. Since TiktokenViewModel in the Example target still references this helper, the Example target will no longer compile after this deletion. If you intend to drop the helper, the example (and any documented fallback) needs to be updated to use the new SwiftTiktoken API instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
Summary
This PR replaces the FFI-based Rust implementation with a pure Swift implementation from DePasqualeOrg/swift-tiktoken.
Motivation
The previous implementation used UniFFI to bridge to a Rust core, which caused:
Changes
Added
Removed
Updated
Benefits
Testing
All 13 tests pass with the new implementation:
Migration Notes
For users of this package:
Trade-offs
Ultraworked with Sisyphus
Co-authored-by: Sisyphus clio-agent@sisyphuslabs.ai