2.0.0-beta.3: Feature Complete - Clone Implementation & Issue Resolution

- Clone Feature (Issue #29): Complete workspace-based workflow with ADR-007
  - Pull Preflight (Issue #30): Prevents cache pollution from gated/private repos
  - Lenient MLX Detection (Issue #31): Framework detection beyond mlx-community
  - Multi-shard Health (Issue #27): Strict completeness validation
  - Full JSON API 0.1.4: Complete schema for all 10 commands
  - Test Suite: 254/254 passed, comprehensive validation

  See CHANGELOG.md fnd TESTING.md or technical implementation details.
This commit is contained in:
The BROKE Cluster Team
2025-09-18 14:09:32 +02:00
parent 57bf6d86be
commit 9261bc0c4e
29 changed files with 3378 additions and 341 deletions
@@ -0,0 +1,250 @@
# ADR-005: mlxk2 clone Implementation for 2.0.0-beta.3
## Status
**DEPRECATED** - 2025-09-15
**Superseded by:** ADR-006 (Clone Implementation - Revised Strategy)
**Reason for Deprecation:** Critical findings revealed that HuggingFace Hub's `local_dir` parameter does not provide reliable cache isolation and can corrupt existing cache entries. The assumptions about cache isolation in this ADR were incorrect.
## Context
GitHub Issue #29 requests Ollama CLI-like "push" functionality for MLX Knife. The push function was successfully implemented in 2.0.0-alpha.1, but analysis revealed a critical workflow gap: there is no `clone` command to create writable workspaces from HuggingFace models.
### Current Workflow Limitations
**Missing Link in Author Workflow:**
```bash
# Desired workflow - currently incomplete
mlxk2 clone org/model@revision ./workspace # ❌ Missing
mlxk2 health ./workspace # ✅ Exists
mlxk2 push ./workspace org/my-model --private # ✅ Exists
```
**Two Key Use Cases Identified:**
1. **Fork-Modify-Push:** `clone` existing HF model → edit → `push` to new repo
2. **Author-Generated Models:** Native MLX training → workspace → `health``push`
### Technical Analysis Results
**MLX Model Compatibility:** ✅ No additional work needed
- Native MLX models use identical structure to HuggingFace models (config.json + .safetensors)
- Existing `_analyze_workspace()` in push.py already validates MLX-native models correctly
- No .npz/.mlx extensions - MLX uses .safetensors with metadata={"format": "mlx"}
**Implementation Effort:** Very Low (~2 hours)
- Can reuse 90% of existing `pull.py` logic (snapshot_download)
- Only difference: download to custom local_dir instead of HF cache
- Test patterns already established for push (21 tests with offline/online/spec coverage)
### JSON API Schema Impact
**Required Changes for JSON API 0.1.4:**
- Schema update: Add "clone" to command enum in `docs/json-api-schema.json:9`
- Version bump: `mlxk2/spec.py``JSON_API_SPEC_VERSION = "0.1.4"`
- Documentation update: `docs/json-api-specification.md` → Version 0.1.4
- **No new schema definition needed** - clone reuses existing pull schema
## Decision
We will implement `mlxk2 clone` for 2.0.0-beta.3 to complete the GitHub Issue #29 feature request and provide a comprehensive workspace-based workflow, including full JSON API 0.1.4 compliance.
## Implementation Plan
### Phase 1: Core Implementation + JSON API (Session 1)
- **Time Estimate:** 1-1.5 hours (simplified - no new schema needed)
- **Files to Modify:**
- `mlxk2/operations/clone.py` - New file, ~80 lines (reuse pull.py patterns)
- `mlxk2/cli.py` - Add clone command integration
- `mlxk2/spec.py` - Version bump to 0.1.4
- `docs/json-api-schema.json` - Add "clone" to command enum only
- `docs/json-api-specification.md` - Version update + clone documentation
- Basic test coverage: CLI args, validation, JSON output schema
### Phase 2: Complete Test Suite (Session 2)
- **Time Estimate:** 1-2 hours
- **Test Structure:** Mirror existing push test patterns from TESTING.md
- Offline tests: target directory validation, CLI argument parsing
- Online tests: live clone with opt-in env vars (MLXK2_LIVE_CLONE=1)
- Spec tests: JSON schema validation for clone command output (JSON API 0.1.4)
- Integration: Add to existing test matrix in TESTING.md
### Phase 3: Issue #29 Feedback
- Request user testing from feynon (Swift/iOS porting use case)
- Validate workflow completeness for both identified use cases
## Implementation Details
### API Signature
```bash
mlxk2 clone <org>/<repo>[@<revision>] <target_dir> [options]
```
**Options:**
- `--branch <branch>` - Clone specific branch/revision
- `--json` - JSON output mode
- `--quiet` - Suppress progress output
### Critical Cache Behavior Requirements
**IMPORTANT:** Session 1 initial implementation used `snapshot_download(local_dir=target)` which creates symlinks to HF cache. This violates the core requirements below.
**Required Implementation:**
```python
snapshot_download(
repo_id=model_name,
local_dir=str(target_path),
local_dir_use_symlinks=False # CRITICAL: Force actual file copies
)
```
**Cache Isolation Validation:**
- Clone target must contain real files, not symlinks
- HF cache (`~/.cache/huggingface/hub/`) must not be populated during clone
- Target directory should be completely self-contained workspace
### JSON Response Schema (API 0.1.4)
```json
{
"status": "success|error",
"command": "clone",
"data": {
"model": "org/repo",
"download_status": "completed",
"message": "Cloned successfully to ./workspace",
"target_dir": "/abs/path/to/workspace"
},
"error": null
}
```
**Note:** Clone reuses the existing `pull` schema. The `additionalProperties: true` allows `target_dir` field. Only schema change: command enum addition.
### Code Reuse Strategy
- Leverage `pull.py:snapshot_download()` logic
- Reuse `push.py:_analyze_workspace()` for post-clone health validation
- Maintain consistent error handling patterns with existing operations
## JSON API Schema Updates
### Required Schema Changes (docs/json-api-schema.json)
**1. Command Enum Update (Line 9):**
```json
"command": {"type": "string", "enum": ["list", "show", "health", "pull", "rm", "version", "push", "run", "clone"]}
```
**2. No new schema definition needed:**
- Clone reuses existing `pull` schema (lines 180-202)
- `"additionalProperties": true` allows `target_dir` field
- `"required": ["download_status", "message"]` covers clone requirements
- Schema validation works automatically for clone commands
### Specification Documentation Update
**Version:** 0.1.4 (minimal bump for command enum change)
**New Section:** Clone Command documentation with examples
## Testing Strategy
**Test Categories (following TESTING.md patterns):**
- **Offline Tests:** ~10 tests (CLI validation, error handling, schema compliance)
- **Online Tests:** ~3 opt-in tests with live HF repos (MLXK2_LIVE_CLONE=1)
- **Spec Tests:** ~3 JSON schema validation tests (JSON API 0.1.4)
**Environment Variables:**
- `MLXK2_ENABLE_EXPERIMENTAL_CLONE=1` - Enable clone tests in CI
- `MLXK2_LIVE_CLONE=1` - Enable live network tests (opt-in)
**Schema Validation Testing:**
- All clone responses validate against updated JSON schema 0.1.4
- Test both success and error response structures
- Verify backward compatibility with existing commands
## Benefits
1. **Completes Issue #29:** Provides full workspace-based model management workflow
2. **Swift/iOS Friendly:** Clean JSON API suitable for cross-platform porting
3. **Low Risk:** Reuses battle-tested components (snapshot_download, workspace analysis)
4. **Fast Implementation:** Can be completed in 1-2 Claude sessions
5. **Test Coverage:** Follows established patterns from push implementation
6. **JSON API Compliance:** Full schema validation and version management
## Security Classification and Risk Analysis
### Clone vs Push: Fundamental Safety Difference
**Clone Operation: LOW RISK**
- **Read-only operation:** Downloads existing HF content to local workspace
- **No publication risk:** Cannot create/modify remote repositories
- **Local-only impact:** Only affects specified target directory
- **Cache isolation:** Bypasses HF cache entirely - direct download to target
- **Validation safeguards:** Refuses to overwrite non-empty directories
- **Risk profile:** Similar to `pull` operation - safe for general use
**Push Operation: HIGH RISK**
- **Write operation:** Publishes content to HuggingFace Hub
- **Publication risk:** Can accidentally expose private/sensitive data
- **Global impact:** Creates permanent public records
- **Requires authentication:** Uses HF_TOKEN with write permissions
- **Experimental status:** Hidden behind `MLXK2_ENABLE_EXPERIMENTAL_PUSH=1`
### Implementation Implications
**Clone does NOT require experimental gating:**
- No `MLXK2_ENABLE_EXPERIMENTAL_CLONE=1` flag needed
- Can be enabled by default in 2.0.0-beta.3
- Standard test integration (not opt-in only)
- Live tests follow normal marker patterns (like `list`, `pull`)
**Clone workspace isolation guarantees:**
1. **No cache pollution:** Downloads directly to target_dir with `local_dir_use_symlinks=False`, never touches HF_HOME
2. **No overwrite risk:** Validation ensures target directory is empty or non-existent
3. **Explicit targeting:** User must specify exact target path
4. **Atomic operation:** Either succeeds completely or fails cleanly
5. **Real file copies:** Target contains actual files, not symlinks to cache (validated in tests)
## Risks and Mitigations
**Risk:** Directory conflicts and overwrite behavior
**Mitigation:** Require explicit target directory, validate empty/non-existent before download
**Risk:** Large model download interruption
**Mitigation:** Leverage huggingface_hub's built-in resume_download=True
**Risk:** Disk space exhaustion
**Mitigation:** Pre-flight disk space check, clear error messages
**Risk:** JSON API version compatibility
**Mitigation:**
- Follow established versioning patterns from existing commands
- Complete schema validation test coverage
- Document breaking changes clearly
**Risk:** Test suite complexity
**Mitigation:** Standard test integration (not experimental opt-in), proven patterns from pull tests
## Timeline
**Target:** 2.0.0-beta.3 release within 24 hours
- Session 1: Core implementation + minimal schema update + basic tests (1.5-2 hours)
- Session 2: Complete test suite + documentation (1-2 hours)
- Issue #29 feedback request: Immediate after implementation
## Success Criteria
1. ✅ Complete workflow: `clone``health``push`
2. ✅ Both use cases supported (fork-modify-push + author-generated)
3. ✅ JSON API 0.1.4 compliance with full schema validation
4. ✅ Test coverage matches push patterns (~15 total tests)
5. ✅ Schema backwards compatibility maintained
6. ✅ feynon feedback positive for Swift porting use case
## References
- GitHub Issue #29: https://github.com/ml-explore/mlx-knife/issues/29
- TESTING.md: Push test patterns (21 tests, offline/online/spec structure)
- ADR-001: JSON-first architecture principles
- mlxk2/operations/push.py: Workspace analysis and health check patterns
- docs/json-api-schema.json: Current schema definition (0.1.3)
- docs/json-api-specification.md: Current specification (0.1.3)
@@ -0,0 +1,209 @@
# ADR-006: mlxk2 clone Implementation - Revised Strategy
## Status
**Accepted** - 2025-09-15
**Supersedes:** ADR-005 (deprecated due to incorrect HuggingFace cache assumptions)
## Context
GitHub Issue #29 requests clone functionality for MLX Knife 2.0. After implementing ADR-005, critical findings revealed that HuggingFace Hub's `local_dir` parameter does not provide true cache isolation and can corrupt existing cache entries.
### Key Findings from ADR-005 Implementation
**Problem: HuggingFace Cache Behavior is Unreliable**
1. `snapshot_download(local_dir=target, local_dir_use_symlinks=False)` still interacts with global cache
2. Global cache corruption observed (models showing 0.0 KB after clone operations)
3. `local_dir_use_symlinks` parameter is deprecated but behavior remains unclear
4. Documentation promises cache isolation but implementation differs
**Evidence:**
```bash
# Before clone: Phi-3-mini shows 4.3 GB in cache
mlxk list --health # Shows healthy model
# After clone with local_dir: Cache corrupted
mlxk list --health # Shows 0.0 KB - corrupted cache entry
```
### Revised Strategy: Pull + APFS Copy + Cleanup
**Core Insight:** Instead of fighting HuggingFace Hub's undocumented cache behavior, leverage it robustly:
1. **Pull to Cache** (battle-tested, reliable)
2. **Copy Cache → Workspace** (APFS copy-on-write optimization)
3. **Delete Cache Entry** (automatic cleanup)
## Decision
Implement `mlxk2 clone` using a **Pull + Copy + Cleanup** strategy that provides robust workspace creation without relying on HuggingFace Hub's unreliable `local_dir` behavior.
## Implementation Strategy
### Core Workflow
```
1. Hidden Pull → Download to cache (existing reliable logic)
2. Optional Health → Validate model integrity before copy
3. APFS Copy → Copy cache → workspace (copy-on-write efficient)
4. Cache Cleanup → Delete cache entry (no user prompt needed)
```
### APFS Volume Optimization
**Key Advantage:** On APFS volumes (standard on macOS), file copies use copy-on-write:
- Initial copy: **No additional disk space** (metadata references only)
- Space usage: Only when files are modified in workspace
- Copy speed: Near-instantaneous for large models
**Volume Detection:**
```python
def is_same_apfs_volume(cache_path, workspace_path):
# Check if both paths are on same APFS volume
# Optimize copy strategy accordingly
```
### API Signature (Unchanged)
```bash
mlxk2 clone <org>/<repo>[@<revision>] <target_dir> [options]
```
**Options:**
- `--branch <branch>` - Clone specific branch/revision
- `--json` - JSON output mode
- `--quiet` - Suppress progress output
- `--no-health-check` - Skip optional health validation
### JSON Response Schema (API 0.1.4 - Unchanged)
```json
{
"status": "success|error",
"command": "clone",
"data": {
"model": "org/repo",
"clone_status": "completed",
"message": "Cloned successfully to ./workspace",
"target_dir": "/abs/path/to/workspace",
"cache_cleanup": true,
"health_check": true
},
"error": null
}
```
## Implementation Details
### Phase 1: Core Clone Logic
```python
def clone_operation(model_spec, target_dir, health_check=True):
# 1. Standard pull to cache
pull_result = pull_operation(model_spec)
if pull_result["status"] != "success":
return error_response("Pull failed", pull_result["error"])
# 2. Optional health check
if health_check:
health_result = health_check_cache(model_spec)
if not health_result["healthy"]:
return error_response("Model unhealthy", health_result)
# 3. Copy cache to workspace
cache_path = resolve_cache_path(model_spec)
copy_result = apfs_optimized_copy(cache_path, target_dir)
if not copy_result["success"]:
return error_response("Copy failed", copy_result["error"])
# 4. Cleanup cache entry
cleanup_result = remove_cache_entry(model_spec)
return success_response(copy_result, cleanup_result)
```
### Phase 2: APFS Optimization
```python
def apfs_optimized_copy(source_path, target_path):
"""Copy with APFS copy-on-write optimization where possible."""
if is_same_apfs_volume(source_path, target_path):
# Use APFS-optimized copy (clonefile on macOS)
return apfs_clone_files(source_path, target_path)
else:
# Fall back to standard file copy
return standard_copy(source_path, target_path)
```
### Phase 3: Cache Management
```python
def remove_cache_entry(model_spec):
"""Remove cache entry after successful workspace creation."""
cache_path = hf_to_cache_dir(model_spec)
if cache_path.exists():
shutil.rmtree(cache_path)
return {"cache_cleanup": True, "path": str(cache_path)}
```
## Benefits
1. **Robust Behavior:** Uses proven pull logic, avoids HF cache edge cases
2. **APFS Efficient:** No duplicate storage on same volume (copy-on-write)
3. **Clean Workspaces:** No cache artifacts (.cache folders, symlinks)
4. **Predictable:** No undocumented HF behavior dependencies
5. **Testable:** Each phase can be tested independently
## Security Classification
**Clone Operation: LOW RISK** (unchanged)
- Read-only operation with local file manipulation only
- No remote publication risk
- Workspace isolation maintained through file copying
## Risk Analysis
### Mitigated Risks (from ADR-005)
-**Cache Corruption:** Eliminated by using standard pull path
-**Undocumented Behavior:** No reliance on HF `local_dir` edge cases
-**Symlink Issues:** Pure file copying, no symlinks
### New Risks and Mitigations
**Risk:** Double storage usage during copy process
**Mitigation:** APFS copy-on-write optimization, volume detection
**Risk:** Cache cleanup removes model unexpectedly
**Mitigation:** Only cleanup after successful workspace creation
**Risk:** Interrupted copy leaves partial workspace
**Mitigation:** Atomic operations, rollback on failure
## Testing Strategy
### Test Categories
1. **Pull Integration:** Verify pull-phase works correctly
2. **Copy Operations:** Test APFS vs standard copying
3. **Cache Management:** Validate cleanup behavior
4. **Error Handling:** Test failure modes at each phase
5. **JSON Schema:** API 0.1.4 compliance validation
### Environment Variables
- `MLXK2_ENABLE_EXPERIMENTAL_CLONE=1` - Enable clone tests in CI
- `MLXK2_LIVE_CLONE=1` - Enable live network tests (opt-in)
## Timeline
**Target:** Complete within current session
- Implementation: 1-2 hours (reuse existing pull logic)
- Testing: 1 hour (focused on copy + cleanup logic)
- Documentation: 30 minutes
## Success Criteria
1.**Reliable Clone:** No cache corruption, predictable behavior
2.**APFS Optimized:** Minimal storage overhead on macOS
3.**Clean Workspaces:** No cache artifacts in target directories
4.**JSON API Compliance:** Full 0.1.4 schema validation
5.**Robust Error Handling:** Graceful failure at each phase
## References
- **Supersedes:** ADR-005 (retained for historical reference)
- GitHub Issue #29: Clone functionality request
- HuggingFace Hub Documentation: `snapshot_download` behavior analysis
- APFS Technical Reference: Copy-on-write filesystem optimization
@@ -0,0 +1,534 @@
# ADR-007: Clone Implementation Fixed Strategy
**Status:** Accepted
**Date:** 2025-01-16
**Supersedes:** ADR-006 (Clone Implementation Revised)
## Context
The clone implementation following ADR-006 has critical bugs that make it unsuitable for release:
1. **Destructive Cache Cleanup:** Always deletes user cache after copy, even when model pre-existed
2. **Commit Hash Mismatch:** Can copy outdated snapshots when remote HEAD differs from cached version
3. **Inconsistent Behavior:** User unexpectedly loses models from cache
Analysis revealed that the Pull+Copy+Cleanup strategy is fundamentally flawed for a "clone" operation, which should be non-destructive by nature.
## Decision
### Phased Implementation Strategy
**Core Principle:** Cache must be APFS (for optimization), workspace flexibility increases per phase.
### Phase 1: Same-Volume APFS (2.0.0-beta.3)
**Constraints:**
- Cache: APFS required
- Workspace: APFS required, same volume as cache
- Optimization: Direct APFS copy-on-write
**Workflow:**
```
1. Validate cache and workspace both on same APFS volume
2. Create isolated temp cache on same volume as workspace
3. Pull model to temp cache (isolated from user cache)
4. APFS clone temp cache → workspace (instant, zero space initially)
5. Delete temp cache (cleanup)
```
### Phase 2: Cross-Filesystem Support (eventually, when clone and push is non-Alpha)
**Constraints:**
- Cache: APFS required (for temp cache optimization)
- Workspace: Any filesystem supported
- Optimization: APFS CoW for temp cache, standard copy to workspace
**Workflow:**
```
1. Validate cache on APFS (workspace can be any filesystem)
2. Create isolated temp cache on APFS volume (cache volume)
3. Pull model to temp cache via APFS optimization
4. Copy temp cache → workspace (standard copy if cross-filesystem)
5. Delete temp cache (cleanup)
```
## Implementation Matrix
### Filesystem Compatibility Table
| Cache FS | Workspace FS | Same Volume | Phase 1 Support | Phase 2 Support | Copy Method | Performance |
|----------|--------------|-------------|------------------|------------------|-------------|-------------|
| APFS | APFS | Yes | ✅ Supported | ✅ Supported | APFS CoW Direct | ⚡ Instant |
| APFS | APFS | No | ❌ Error | ✅ Supported | Temp+Standard | 🐌 2x Copy |
| APFS | HFS+ | No | ❌ Error | ✅ Supported | Temp+Standard | 🐌 2x Copy |
| APFS | ExFAT | No | ❌ Error | ✅ Supported | Temp+Standard | 🐌 2x Copy |
| APFS | NFS | No | ❌ Error | ⚠️ Warning | Temp+Network | 🐌🐌 Slow |
| APFS | SMB/CIFS | No | ❌ Error | ⚠️ Warning | Temp+Network | 🐌🐌 Slow |
| HFS+ | Any | Any | ❌ Error | ❌ Error | N/A | N/A |
| NFS | Any | Any | ❌ Error | ❌ Error | N/A | N/A |
| SMB/CIFS | Any | Any | ❌ Error | ❌ Error | N/A | N/A |
### Data Flow Scenarios
#### Scenario A: Phase 1 Optimal (Same APFS Volume)
```
User Cache (APFS): /Users/me/.cache/huggingface/hub/
Target Workspace: /Users/me/projects/mymodel/
Temp Cache: /Users/me/.mlxk2_temp_12345/
Flow:
[Remote] --pull--> [Temp Cache] --APFS CoW--> [Workspace]
↑ ↓
Zero space Instant copy
```
#### Scenario B: Phase 2 Cross-Filesystem
```
User Cache (APFS): /Users/me/.cache/huggingface/hub/
Target Workspace: /Volumes/ProjectSSD/myapp/models/
Temp Cache: /Users/me/.mlxk2_temp_12345/
Flow:
[Remote] --pull--> [Temp Cache] --Standard Copy--> [Workspace]
↑ ↓
APFS CoW Full copy
```
#### Scenario C: Phase 2 Network Workspace (NFS/SMB)
```
User Cache (APFS): /Users/me/.cache/huggingface/hub/
Target Workspace: /Volumes/NetworkShare/models/ (NFS or SMB/CIFS)
Temp Cache: /Users/me/.mlxk2_temp_12345/
Flow:
[Remote] --pull--> [Temp Cache] --Network Copy--> [Network Workspace]
↑ ↓
Fast local Slow network
```
### Response Matrix (Phase 1 Implementation)
| Function | APFS Check Timing | Non-APFS Response | Response Type | JSON Example |
|----------|------------------|-------------------|---------------|--------------|
| `serve` | Never | Normal operation | Success | `{"status": "success", "command": "serve", ...}` |
| `list` | Never | Normal operation | Success | `{"status": "success", "command": "list", ...}` |
| `show` | Never | Normal operation | Success | `{"status": "success", "command": "show", ...}` |
| `health` | Never | Normal operation | Success | `{"status": "success", "command": "health", ...}` |
| `pull` | Never | Normal operation | Success | `{"status": "success", "command": "pull", ...}` |
| **`push`** | On success (Alpha only) | **Add APFS hint to message** | ⚠️ Success + Warning | `{"status": "success", "data": {"message": "Push successful. Clone operations require APFS filesystem."}}` |
| **`clone`** | On demand (lazy) | **Hard error, abort** | ❌ Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` |
### Error Handling Matrix
#### Phase 1 Error Matrix (Same-Volume APFS Only)
| Scenario | Cache FS | Workspace FS | Same Volume | Error Type | Behavior | JSON Error | User Action |
|----------|----------|--------------|-------------|------------|----------|------------|-------------|
| ✅ **Supported** | APFS | APFS | Yes | None | Success | N/A | None |
| ❌ **Cache Requirement** | HFS+ | Any | Any | CacheFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Migrate cache to APFS |
| ❌ **Cache Requirement** | ExFAT | Any | Any | CacheFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Migrate cache to APFS |
| ❌ **Cache Requirement** | NFS/SMB | Any | Any | CacheFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Use local APFS cache |
| ❌ **Workspace Requirement** | APFS | HFS+ | No | WorkspaceFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Use APFS workspace |
| ❌ **Workspace Requirement** | APFS | ExFAT | No | WorkspaceFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Use APFS workspace |
| ❌ **Workspace Requirement** | APFS | NFS/SMB | No | WorkspaceFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Use APFS workspace |
| ❌ **Volume Requirement** | APFS | APFS | No | VolumeError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Move workspace to cache volume |
#### Phase 2 Error Matrix (Cross-Filesystem Support)
| Scenario | Cache FS | Workspace FS | Same Volume | Error Type | Behavior | JSON Response | User Action |
|----------|----------|--------------|-------------|------------|----------|---------------|-------------|
| ✅ **Optimal** | APFS | APFS | Yes | None | Success (CoW) | `{"status": "success", "data": {"clone_status": "success", "copy_method": "apfs_cow"}}` | None |
| ✅ **Standard** | APFS | APFS | No | None | Success (Standard) | `{"status": "success", "data": {"clone_status": "success", "copy_method": "standard_copy"}}` | None |
| ✅ **Standard** | APFS | HFS+ | No | None | Success (Standard) | `{"status": "success", "data": {"clone_status": "success", "copy_method": "standard_copy"}}` | None |
| ✅ **Standard** | APFS | ExFAT | No | None | Success (Standard) | `{"status": "success", "data": {"clone_status": "success", "copy_method": "standard_copy"}}` | None |
| ⚠️ **Network Warning** | APFS | NFS | No | NetworkWarning | Warning + Proceed | `{"status": "success", "data": {"clone_status": "success", "copy_method": "network_copy", "warning": "Network filesystem detected. Copy will be slower."}}` | Expect slower performance |
| ⚠️ **Network Warning** | APFS | SMB/CIFS | No | NetworkWarning | Warning + Proceed | `{"status": "success", "data": {"clone_status": "success", "copy_method": "network_copy", "warning": "Network filesystem detected. Copy will be slower."}}` | Expect slower performance |
| ❌ **Cache Requirement** | HFS+ | Any | Any | CacheFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Migrate cache to APFS |
| ❌ **Cache Requirement** | ExFAT | Any | Any | CacheFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Migrate cache to APFS |
| ❌ **Cache Requirement** | NFS/SMB | Any | Any | CacheFilesystemError | Hard Error | `{"status": "error", "data": {"clone_status": "filesystem_error"}}` | Use local APFS cache |
#### Error Message Examples
**⚠️ JSON Protocol Disclaimer:**
> All JSON response examples are provisional and based on specification v0.1.4. Field contents (e.g., `clone_status` values) and response structure may evolve during Phase 1 and Phase 2 implementation.
**Phase 1 Errors:**
```json
{
"status": "error",
"command": "clone",
"data": {
"clone_status": "filesystem_error",
"target_dir": "/some/workspace"
},
"error": {
"type": "FilesystemError",
"message": "APFS cache required for clone operations."
}
}
```
**Phase 2 Network Warnings:**
```json
{
"status": "success",
"command": "clone",
"data": {
"model": "microsoft/DialoGPT-small",
"target_dir": "/Volumes/NASShare/models/dialog",
"clone_status": "success",
"message": "Cloned to /Volumes/NASShare/models/dialog",
"expanded_name": "microsoft/DialoGPT-small"
},
"error": null
}
```
### Performance Characteristics
#### Phase 1 (Same APFS Volume)
```
4GB Model Clone Performance:
- Temp cache creation: ~30 seconds (network download)
- APFS CoW copy: ~0.1 seconds (metadata only)
- Temp cleanup: ~0.5 seconds
- Total time: ~30.6 seconds
- Total space: ~4GB (only in workspace after CoW)
```
#### Phase 2 (Cross-Filesystem)
```
4GB Model Clone Performance:
- Temp cache creation: ~30 seconds (network download)
- Standard copy: ~60 seconds (4GB copy)
- Temp cleanup: ~0.5 seconds
- Total time: ~90.5 seconds
- Peak space: ~8GB (temp + workspace during copy)
```
## Migration Strategy Between Phases
### Phase 1 → Phase 2 Upgrade
- **Breaking Change:** None (Phase 1 scenarios still work optimally)
- **New Capability:** Cross-filesystem support added
- **User Impact:** More flexible workspace placement
- **Performance:** Same for existing use cases, degraded for new cross-FS cases
### Implementation Flags
```python
# Alpha feature gate (existing)
MLXK2_ENABLE_ALPHA_FEATURES=1 # Required for clone and push operations
# Future Phase 2 flags (if needed)
# MLXK2_CLONE_ALLOW_CROSS_FILESYSTEM=1
```
## Implementation Details
### 1. Volume-Aware Temp Cache Creation
```python
def create_temp_cache_same_volume(target_workspace: Path) -> Path:
"""Create temp cache on same APFS volume as target for CoW optimization."""
# Get target volume mount point via st_dev
target_volume = get_volume_mount_point(target_workspace)
# Create temp cache on same volume
temp_cache = target_volume / f".mlxk2_temp_{os.getpid()}_{random.randint(1000,9999)}"
temp_cache.mkdir(parents=True)
# SAFETY: Create sentinel file to prevent accidental user cache deletion
sentinel = temp_cache / ".mlxk2_temp_cache_sentinel"
sentinel.write_text(f"mlxk2_temp_cache_created_{int(time.time())}")
return temp_cache
def cleanup_temp_cache_safe(temp_cache: Path) -> bool:
"""Safely delete temp cache only if sentinel exists."""
# SAFETY: Only delete if sentinel exists
sentinel = temp_cache / ".mlxk2_temp_cache_sentinel"
if not sentinel.exists():
logger.warning(f"Refusing to delete {temp_cache} - no sentinel found")
return False
shutil.rmtree(temp_cache, ignore_errors=True)
return True
def get_volume_mount_point(path: Path) -> Path:
"""Find mount point (volume root) for given path via st_dev changes."""
abs_path = path.resolve()
current = abs_path
while current != current.parent:
try:
parent_stat = current.parent.stat()
current_stat = current.stat()
# Different st_dev = mount boundary
if parent_stat.st_dev != current_stat.st_dev:
return current
except (OSError, PermissionError):
pass
current = current.parent
return current # Filesystem root
```
### 2. Shared APFS Filesystem Check
```python
def is_apfs_filesystem(path: Path) -> bool:
"""Simple APFS check - returns True/False only.
Used by both clone (validation) and push (conditional warning).
"""
try:
import subprocess
result = subprocess.run(['stat', '-f', '-c', '%T', str(path)],
capture_output=True, text=True)
return result.stdout.strip() == 'apfs'
except subprocess.CalledProcessError:
return False # Safe fallback
def validate_apfs_filesystem(path: Path) -> None:
"""Validate APFS requirement for clone operations.
Called lazily - only on first clone operation, not at CLI startup.
"""
if not is_apfs_filesystem(path):
raise FilesystemError(
f"APFS required for clone operations. "
f"Path: {path}\n"
f"Solution: Use APFS volume or external APFS SSD."
)
```
### 3. Clone Operation Implementation
```python
def clone_operation(model_spec: str, target_dir: str) -> Dict[str, Any]:
"""Clone with isolated temp cache strategy."""
target_path = Path(target_dir).resolve()
# 1. Validate APFS requirement
validate_apfs_filesystem(target_path.parent)
# 2. Create temp cache on same volume as target
temp_cache = create_temp_cache_same_volume(target_path)
try:
# 3. Pull to isolated temp cache
with patch_hf_home(temp_cache):
pull_result = pull_operation(model_spec)
if pull_result["status"] != "success":
return handle_pull_error(pull_result)
# 4. Resolve temp cache snapshot path
resolved_model = pull_result["data"]["model"]
temp_snapshot = resolve_latest_snapshot(temp_cache, resolved_model)
# 5. APFS clone to workspace (instant, CoW)
target_path.mkdir(parents=True, exist_ok=True)
clone_success = apfs_clone_directory(temp_snapshot, target_path)
if not clone_success:
return handle_clone_error()
# 6. Success - temp cache auto-cleanup via context manager
return {
"status": "success",
"command": "clone",
"data": {
"model": resolved_model,
"target_dir": str(target_path),
"clone_status": "completed",
"cache_preserved": True, # User cache never touched
"copy_method": "apfs_cow"
}
}
finally:
# Cleanup temp cache
shutil.rmtree(temp_cache, ignore_errors=True)
```
### 4. User Experience: Push Workflow Warning
```python
def push_operation(...) -> Dict[str, Any]:
# ... normal push logic ...
# Conditional APFS hint based on cache filesystem
if not is_apfs_filesystem(get_hf_cache_dir()):
message = "Push successful. Clone operations require APFS filesystem."
else:
message = "Push successful."
result = {
"status": "success",
"command": "push",
"data": {
"repo_id": repo_id,
"branch": branch,
"message": message,
# ... existing fields ...
}
}
return result
```
### 5. APFS Copy-on-Write Implementation
```python
def apfs_clone_directory(source: Path, target: Path) -> bool:
"""Clone directory using APFS copy-on-write via clonefile."""
try:
for item in source.rglob("*"):
if item.is_file():
relative_path = item.relative_to(source)
target_file = target / relative_path
target_file.parent.mkdir(parents=True, exist_ok=True)
# Use cp -c for clonefile (APFS CoW)
subprocess.run(['cp', '-c', str(item), str(target_file)],
check=True, capture_output=True)
return True
except subprocess.CalledProcessError as e:
logger.error(f"APFS clone failed: {e}")
return False
```
## Pros and Cons
### Pros
1. **User Cache Preservation:** Never touches existing user cache
2. **Consistency:** Always gets latest/specified model version
3. **Performance:** APFS CoW provides instant copy with minimal space
4. **Isolation:** Temp cache prevents pollution of user environment
5. **Predictable:** Clone behaves like standard file copy operation
6. **Robust:** Clear filesystem requirements with early validation
### Cons
1. **APFS Requirement:** Users on non-APFS setups need migration
2. **Temporary Disk Usage:** Brief full model copy in temp cache before CoW
3. **Implementation Complexity:** Volume detection and temp cache management
4. **Platform Specific:** Relies on macOS/iOS APFS features
## Migration from ADR-006
### Breaking Changes
1. **Cache Behavior:** User cache is preserved (not deleted after clone)
2. **Filesystem Requirements:** APFS validation added
3. **Performance Profile:** May use more temporary disk space
### User Migration
**Before (ADR-006):**
```bash
mlxk2 clone org/model ./workspace # Deleted model from cache
```
**After (ADR-007):**
```bash
mlxk2 clone org/model ./workspace # Preserves model in cache
# User cache remains intact for other operations
```
### Error Handling
**Non-APFS Cache:**
```
Error: Filesystem 'nfs' not supported
MLX-Knife requires APFS for clone operations.
Current path: /Volumes/NetworkShare/cache
Solution: Use APFS volume:
export HF_HOME="/Users/you/.cache/huggingface"
```
## Testing Strategy
### Unit Tests
1. **Volume Detection:** Verify mount point resolution across scenarios
2. **APFS Validation:** Test filesystem detection and error handling
3. **Temp Cache Creation:** Validate same-volume placement
4. **Copy-on-Write:** Test clonefile success and fallback behavior
### Integration Tests
1. **Cross-Volume Scenarios:** Cache on external APFS, workspace on internal
2. **Large Model Performance:** Verify CoW benefits with multi-GB models
3. **Error Recovery:** Temp cache cleanup on failures
4. **Concurrent Access:** Multiple clone operations
### Real-World Validation
1. **External APFS SSDs:** Thunderbolt/USB-C attached storage
2. **iOS Simulator:** Validate iOS filesystem assumptions
3. **Network Limitations:** Ensure clear errors for unsupported setups
## Implementation Timeline
### Phase 1: Same-Volume APFS (2.0.0-beta.3)
**Target:** Stable clone functionality with strict constraints
- ✅ Volume detection utilities (`get_volume_mount_point`, `is_same_volume`)
- ✅ APFS validation framework (`validate_apfs_filesystem`)
- ✅ Temp cache management on same volume
- ✅ APFS copy-on-write implementation (`apfs_clone_directory`)
- ✅ Error handling for unsupported scenarios
- ✅ Performance optimization for direct CoW path
**Success Criteria:**
- Clone works reliably when cache and workspace on same APFS volume
- Clear error messages for unsupported filesystem combinations
- Performance benchmarks show near-instant copy for large models
### Phase 2: Cross-Filesystem Support (eventually, when clone and push is non-Alpha)
**Target:** Flexible workspace placement with graceful degradation
- 🔄 Cross-filesystem copy implementation
- 🔄 Performance monitoring for different copy methods
- 🔄 Network filesystem handling and warnings
- 🔄 User experience improvements for mixed scenarios
- 🔄 Configuration flags for behavior control
**Success Criteria:**
- Clone works across all supported filesystem combinations
- Performance degradation is predictable and documented
- User guidance for optimal setup configurations
### Phase 3: Advanced Features (future, no version commitment)
**Target:** Production hardening and edge case handling
**Status:** Nice-to-have features, implement based on user demand
- 🔄 Incremental clone support (delta updates)
- 🔄 Resume capability for interrupted operations
- 🔄 Bandwidth limiting for network operations
- 🔄 Comprehensive logging and diagnostics
- 🔄 Advanced caching strategies
## Decision Rationale
This strategy addresses the fundamental flaws in ADR-006 while leveraging the strengths of the Apple Silicon ecosystem. By requiring APFS and using isolated temp caches, we achieve:
- **Correctness:** No data loss or inconsistent states
- **Performance:** Copy-on-write optimization
- **Simplicity:** Clear requirements and predictable behavior
The APFS requirement is justified given MLX's Apple Silicon dependency and the target use case focus on iOS development.
## Status
- **Implementation:** To be started
- **Testing:** Required before release
- **Documentation:** Needs update for filesystem requirements
- **Release:** Blocks 2.0.0-beta.3 until complete
+4 -1
View File
@@ -10,8 +10,11 @@ This directory contains Architecture Decision Records (ADRs) that document signi
|-----|-------|--------|------|
| [ADR-001](ADR-001-json-api-strategy.md) | JSON API Strategy & 2.0 Migration Path | Accepted | 2025-08-28 |
| [ADR-002](ADR-002-edge-cases.md) | Edge Cases from 1.x Test Suite | Accepted | 2025-08-28 |
| [ADR-003](ADR-003-Server-Run-Port-to-2.0.md) | Server and Run Functionality Port from 1.x to 2.0 | Proposed | 2025-09-10 |
| [ADR-003](ADR-003-Server-Run-Port-to-2.0.md) | Server and Run Functionality Port from 1.x to 2.0 | Accepted | 2025-09-10 |
| [ADR-004](ADR-004-Enhanced-Error-Logging.md) | Enhanced Error Handling & Logging | Proposal (post-beta.3) | 2025-09-14 |
| [ADR-005](ADR-005-Clone-Implementation-Beta3.md) | Clone Implementation Beta3 | Superseded by ADR-007 | 2025-01-15 |
| [ADR-006](ADR-006-Clone-Implementation-Revised.md) | Clone Implementation Revised | Superseded by ADR-007 | 2025-01-16 |
| [ADR-007](ADR-007-Clone-Implementation-Fixed.md) | Clone Implementation Fixed Strategy | Accepted | 2025-01-16 |
## ADR Format
+25 -1
View File
@@ -6,7 +6,7 @@
"additionalProperties": false,
"properties": {
"status": {"type": "string", "enum": ["success", "error"]},
"command": {"type": "string", "enum": ["list", "show", "health", "pull", "rm", "version", "push", "run"]},
"command": {"type": "string", "enum": ["list", "show", "health", "pull", "rm", "clone", "version", "push", "run", "server"]},
"api_version": {"type": "string", "pattern": "^json-[0-9]+\\.[0-9]+\\.[0-9]+$"},
"data": {"type": ["object", "null"]},
"error": {
@@ -250,6 +250,30 @@
}
},
"else": {}
},
{
"if": {
"allOf": [
{"properties": {"status": {"const": "success"}}},
{"properties": {"command": {"const": "run"}}}
]
},
"then": {
"properties": {
"data": {
"type": "object",
"additionalProperties": true,
"properties": {
"model": {"type": "string"},
"prompt": {"type": "string"},
"response": {"type": "string"},
"tokens_generated": {"type": "integer"},
"generation_time_s": {"type": "number"}
}
}
}
},
"else": {}
}
]
}
+116 -23
View File
@@ -1,6 +1,6 @@
# MLX-Knife 2.0 JSON API Specification
**Specification Version:** 0.1.3
**Specification Version:** 0.1.4
**Status:** Alpha - Subject to change
**Target:** MLX-Knife 2.0.0
@@ -15,16 +15,10 @@ MLX Knife is promoted as a "scriptable" tool, but formatted terminal output make
All commands require the `--json` flag for JSON output:
```bash
mlxk-json list --json # JSON output (2.0.0-alpha+)
mlxk list --json # JSON output (2.0.0+)
mlxk list # Human-readable output (2.0.0+)
mlxk2 list --json # JSON output
mlxk2 list # Human-readable output
```
**Version Support:**
- **2.0.0-alpha:** Only `mlxk-json --json` available (JSON-only implementation)
- **2.0.0+:** Both `mlxk --json` and `mlxk-json --json` for JSON output
- **2.0.0+:** `mlxk` without `--json` for human-readable output
### Version Reporting
- CLI version (human):
@@ -57,7 +51,7 @@ All commands support consistent JSON output with standardized error handling and
```jsonc
{
"status": "success" | "error",
"command": "list" | "show" | "health" | "pull" | "rm" | "version",
"command": "list" | "show" | "health" | "pull" | "rm" | "clone" | "version" | "push" | "run" | "server",
"data": { /* command-specific data */ },
"error": null | { "type": "string", "message": "string" }
}
@@ -84,16 +78,19 @@ Notes:
### Supported Commands
| Command | Description | JSON-Only in 2.0 |
|---------|-------------|------------------|
| `list` | List models with metadata and hash codes | ✅ |
| `show` | Detailed model inspection with files/config | ✅ |
| `health` | Check model integrity and corruption | ✅ |
| `pull` | Download models from HuggingFace | ✅ |
| `rm` | Delete models from cache | ✅ |
| `push` | Upload a local folder to Hugging Face (experimental) | ✅ |
| `run` | Execute model inference | ❌ Not in 2.0 |
| `server` | OpenAI-compatible API server | ❌ Not in 2.0 |
| Command | Description | JSON-Only in 2.0 | Alpha Feature |
|---------|-------------|------------------|---------------|
| `list` | List models with metadata and hash codes | ✅ | - |
| `show` | Detailed model inspection with files/config | ✅ | - |
| `health` | Check model integrity and corruption | ✅ | - |
| `pull` | Download models from HuggingFace | ✅ | - |
| `rm` | Delete models from cache | ✅ | - |
| `clone` | Clone models to workspace directory | ✅ | `MLXK2_ENABLE_ALPHA_FEATURES=1` |
| `push` | Upload a local folder to Hugging Face (experimental) | ✅ | `MLXK2_ENABLE_ALPHA_FEATURES=1` |
| `run` | Execute model inference | ✅ | - |
| `serve`/`server` | OpenAI-compatible API server | ✅ | - |
**Note:** Commands marked with Alpha Feature require `MLXK2_ENABLE_ALPHA_FEATURES=1` environment variable to be available.
## Model Discovery & Metadata
@@ -603,9 +600,104 @@ mlxk-json rm "locked-model" --json # Error: requires --force due t
}
```
### `mlxk-json clone <model> <target_dir> --json`
**Requires:** `MLXK2_ENABLE_ALPHA_FEATURES=1`
**Usage:**
```bash
mlxk-json clone "Phi-3-mini" ./workspace --json # Clone to workspace directory
mlxk-json clone "mlx-community/Phi-3-mini" ./my-model --json # Full name to custom directory
mlxk-json clone "microsoft/DialoGPT-small" ./workspace --json # Non-MLX model
```
**Successful Clone:**
```json
{
"status": "success",
"command": "clone",
"data": {
"model": "mlx-community/Phi-3-mini-4k-instruct-4bit",
"clone_status": "success",
"message": "Cloned to ./workspace",
"target_dir": "./workspace",
"expanded_name": "mlx-community/Phi-3-mini-4k-instruct-4bit"
},
"error": null
}
```
**Target Directory Not Empty:**
```json
{
"status": "error",
"command": "clone",
"data": {
"model": null,
"clone_status": "error",
"target_dir": "./workspace"
},
"error": {
"type": "ValidationError",
"message": "Target directory './workspace' already exists and is not empty"
}
}
```
**Clone Failed:**
```json
{
"status": "error",
"command": "clone",
"data": {
"model": "nonexistent/model",
"clone_status": "failed",
"target_dir": "./workspace"
},
"error": {
"type": "clone_failed",
"message": "Repository not found for url: https://huggingface.co/api/models/nonexistent/model"
}
}
```
**Access Denied:**
```json
{
"status": "error",
"command": "clone",
"data": {
"model": "gated/model",
"clone_status": "access_denied",
"target_dir": "./workspace"
},
"error": {
"type": "access_denied",
"message": "Access denied: gated/private model 'gated/model'. Accept terms and set HF_TOKEN."
}
}
```
**APFS Filesystem Error:**
```json
{
"status": "error",
"command": "clone",
"data": {
"model": "org/model",
"clone_status": "filesystem_error",
"target_dir": "./workspace"
},
"error": {
"type": "FilesystemError",
"message": "APFS required for clone operations."
}
}
```
### `mlxk-json push <dir> <org/model> [--create] [--private] [--branch <b>] [--commit "..."] [--verbose] [--check-only] --json`
Status: experimental (M0: upload-only; no validation, no filters)
**Requires:** `MLXK2_ENABLE_ALPHA_FEATURES=1`
Behavior:
- Requires `HF_TOKEN` env.
@@ -631,7 +723,7 @@ Successful Upload (with changes):
"no_changes": false,
"created_repo": false,
"change_summary": {"added": 1, "modified": 2, "deleted": 0},
"message": "Committed 3 files (+1 ~2 -0).",
"message": "Push successful. Clone operations require APFS filesystem.",
"experimental": true,
"disclaimer": "Experimental feature (M0: upload only). No validation/filters; review on the Hub."
},
@@ -866,4 +958,5 @@ All commands use consistent exit codes for scripting:
## Version History
- **2.0.0-alpha:** JSON-only implementation with `mlxk-json --json`
- **2.0.0:** Full implementation with both JSON and human-readable output
- **2.0.0-alphha.1:** Full implementation with both JSON and human-readable output
- **2.0.0-alphha.2:** Push function protocol extension (json-0.1.3)