diff --git a/.gitignore b/.gitignore index f410c73..69a69b4 100644 --- a/.gitignore +++ b/.gitignore @@ -21,8 +21,8 @@ CLAUDE.md TODO_REAL_TESTS.md server.log install_*.log -openwebui311/bin/ .gitignore +docs/ISSUES/ # Test artifacts (generated reports) *_report.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 719c911..3c7f50e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,243 @@ # Changelog +## [2.0.4-beta.6] - 2026-01-07 + +### Highlights + +**Complete Local Development Workflow:** Beta.6 completes the workspace story started in beta.5. Full local development cycle without HuggingFace round-trips: +1. Clone a model to a local workspace (`mlxk clone`) +2. Repair or modify it locally (`mlxk convert --repair-index` fixes index/shard mismatches) +3. Use it directly without pushing: + - Run inference: `mlxk run ./workspace "prompt"` + - Inspect metadata: `mlxk show ./workspace` + - Start a dev server: `mlxk server --model ./workspace` + +Enables model experimentation, repair (mlx-vlm #624 affected models), and testing before publishing to HuggingFace. + +**Vision Batch Processing:** Production-ready vision processing with automatic chunking for large image collections. Default: process one image at a time for maximum stability (prevents Metal OOM + model-specific hallucination). Server now accepts unlimited images with safe chunking. Complete Vision→Text pipe workflow support. Metadata now appears before model output (vision models can reference filenames, GPS, dates in their analysis). + +**Enhanced Reliability:** Resumable clone operations with `--force-resume`, deterministic temp cache naming for better debugging, and improved test architecture (umbrella marker convention) for multi-Python compatibility (3.9-3.14). 161 Wet Umbrella tests passing (includes new Vision→Geo pipe integration tests). + +--- + +### Added + +- **Workspace Operation Support (ADR-018 Phase 0c):** + - Complete local workflow: `clone → convert → run/show/server` without HF push + - `mlxk run ./workspace "prompt"` - Direct inference on workspace paths + - `mlxk show ./workspace` - Metadata inspection for workspaces + - `mlxk server --model ./workspace` - Local dev server with workspace models + - Server `/v1/models` endpoint lists workspaces with `"owned_by": "workspace"` + - MLXRunner + VisionRunner now support workspace paths + - Central implementation via `resolve_model_for_operation()` + `is_workspace_path()` + - Files changed: 11 files (~250 LOC) + - See ADR-018.md Phase 0c for details + +- **Path Resolution Convention (README.md):** + - `model-name` → Cache resolution (expands to `mlx-community/...`) + - `./model-name` → Workspace path (local directory) + - `/abs/path` → Workspace path (absolute) + - `.` or `..` → Workspace path (current/parent directory) + - Explicit path prefixes required to treat local directories as workspaces + - Prevents ambiguity when workspace name matches cache model name + +- **Health command workspace support:** + - `mlxk health ./workspace` - Integrity checks for workspace directories + - Returns minimal format per JSON API Schema 0.1.5: `{name, status, reason}` + - Uses same integrity checks as list/show commands (consistency) + +- **Resumable Clone with --force-resume (ADR-018 Phase 0b):** + - `mlxk clone --force-resume` - Skip confirmation prompt for partial downloads + - Deterministic temp cache naming: `.mlxk2_temp_{hash(model+target)}` (SHA256-based, replaces PID random names) + - Resume detection: Verifies health + download completion marker before resuming + - Conditional cleanup: Keeps complete downloads on failure (debugging/retry), cleanup incomplete + - Helper functions: `_get_deterministic_temp_cache_name()`, `_create_temp_cache_same_volume()` + - Files: `operations/clone.py` (~100 LOC) + +- **Portfolio Discovery Blacklist (Testing Infrastructure):** + - `KNOWN_BROKEN_MODELS` config in `tests_2.0/live/test_utils.py` + - Filters models with upstream runtime bugs (pass static health checks, fail at runtime) + - Policy: Add only when static health ✅ but runtime initialization ❌ due to verified upstream bug + - First entry: `mlx-community/MiMo-VL-7B-RL-bf16` (mlx-vlm NoneType iteration bug) + - Portfolio Discovery now filters against blacklist (27 → 26 models) + - Files: `tests_2.0/live/test_utils.py` (+29 LOC blacklist config) + +- **Vision Batch Processing (ADR-012 Phase 1c, #47):** + - Automatic chunking for large image collections (prevents Metal OOM crashes) + - CLI: `mlxk run model --image *.jpg --chunk N` (N = images per batch) + - Server: `POST /v1/chat/completions {"chunk": N, ...}` + `mlxk serve --chunk N` + - Model loaded once per chunk (isolation guarantees no state leakage between batches) + - Global image numbering preserved (Image 1..N across all chunks) + - Batch context markers in prompt: `[Processing batch 1/2: Images 1-4 (chunk_size=4, 8 total)]` + - Collapsible batch info in output: `📸 Batch 1/2: Images 1-4` (WebUI-friendly) + - Environment variable: `MLXK2_VISION_BATCH_SIZE` sets default chunk size + - Server unlimited images: Removed MAX_IMAGES_PER_REQUEST limit, added MAX_SAFE_CHUNK_SIZE=5 validation + - Chunk size validation: CLI + Server validate against MAX_SAFE_CHUNK_SIZE (HTTP 400 if chunk>5) + - Files: `cli.py`, `operations/run.py`, `operations/serve.py`, `core/server_base.py`, `core/vision_runner.py`, `tools/vision_adapter.py` (~200 LOC) + +- **Vision Chunk Isolation (Prevents State Leakage):** + - Fresh VisionRunner created per chunk (no KV-cache or model state persistence) + - Fixes chunk hallucination bug where model "remembered" images from previous batches + - Trade-off: Model load overhead (~2-3s per chunk) vs guaranteed isolation + - Alternative approach deferred: Model reuse + cache clearing (2.0.5-beta, requires mlx-vlm API support) + +- **Flexible Prompt Argument Order (UX Improvement):** + - Added `--prompt` flag as alternative to positional argument + - Use case: Test different prompts with same parameters: `mlxk run model --image *.jpg --chunk 5 --prompt "prompt"` + - Solves argparse limitation where `--image` with `nargs='+'` consumes following arguments + - Both forms supported: `mlxk run model "prompt" --image file.jpg` OR `mlxk run model --image file.jpg --prompt "prompt"` + - Backward compatible: Positional prompt still works when placed before flags + - Pipe mode integration: `mlx-run pixtral ... | mlx-run qwen - --prompt "..."` combines stdin with additional prompt + - File: `cli.py` (+10 LOC) + +- **Vision→Geo Pipe Integration Tests (Wet Umbrella Phase 4):** + - Comprehensive smoke tests for Vision→Text pipe workflows + - Test suite: `tests_2.0/live/test_pipe_vision_geo.py` (3 tests, marker: `live_vision_pipe`) + - **Test 1:** Vision batch processing with `--chunk 1` (validates ADR-012 Phase 1c) + - **Test 2:** Complete Vision→Geo pipe (validates stdin + --prompt fix) + - **Test 3:** Chunk isolation (validates no state leakage between chunks) + - Test assets: `tests_2.0/assets/geo-test/` (9 publishable Stockholm photos with EXIF) + - Model selection: Hardcoded pixtral (vision) + Portfolio discovery for text model (largest eligible) + - Wet Umbrella integration: 161 total tests passed (Phase 1: 152, Phase 2: 3, Phase 3: 3, Phase 4: 3) + - Graceful skip: Tests skip cleanly if vision/text models unavailable + - **NOT a quality benchmark:** Pure workflow validation (pass = exit 0, no GOLD reference) + +- **Test Infrastructure Improvements (Umbrella Marker Convention):** + - Added `pytest.mark.live` umbrella marker for scalable test exclusion + - All 11 live test files decorated with umbrella + specific markers + - `test-multi-python.sh` simplified: `pytest -m "not live"` (future-proof) + - Documentation: TESTING-DETAILS.md "Writing New Live Tests: Umbrella Marker Convention" + - Rationale: Blacklist pattern (excluding individual markers) doesn't scale with new test categories + +### Changed + +- **BREAKING: Vision processing now defaults to processing one image at a time (ADR-012 Phase 1c, #47)** + - Previous: All images processed in single batch (could OOM crash with many images) + - New: Process one image at a time by default (maximum safety on all systems) + - Override with `--chunk N` (CLI) or `"chunk": N` (API) for faster batching when system can handle it + - Rationale: Safety over speed - prevents Metal OOM + model-specific hallucination with large batches + - **Important:** Chunk isolation (fresh VisionRunner per chunk) prevents state leakage but adds ~2-3s model load overhead per chunk + - Migration: Users with large image workflows should use `--chunk 5` or higher for old behavior + - Environment variable: `MLXK2_VISION_BATCH_SIZE=N` sets default (e.g., export `MLXK2_VISION_BATCH_SIZE=5`) + - **Model-specific hallucination triggers:** + - Larger chunks with global context hints (e.g., "8 total images" when batch only has 4) + - Plural prompts mismatched to actual count (e.g., "describe these images" with chunk=1) + - Default chunk=1 with singular prompts provides maximum robustness + +- **Model metadata `cached` field:** Now `false` for workspace paths, `true` for cache models + - Workspace paths are NOT in HuggingFace cache → `cached: false` + - Cache models remain `cached: true` + - Semantic distinction: cache-managed vs user-managed models + +### Fixed + +- **Vision model detection (False Negatives):** + - Fixed: Models with `vision_config` dict but `skip_vision: true` incorrectly marked as non-vision + - Root cause: `skip_vision` flag misinterpreted as "no vision support" (actually means "optional for text-only") + - Solution: Presence of `vision_config` dict now reliably indicates vision capability + - Impact: Mistral-Small 3.1, DeepSeek-OCR now correctly detected as vision models + - File: `operations/common.py:detect_vision_capability()` + +- **Health check improvements:** + - **processor_config.json support:** Vision models can use EITHER `preprocessor_config.json` OR `processor_config.json` + - Different models use different naming conventions (e.g., DeepSeek-OCR) + - Previously: Only `preprocessor_config.json` accepted → false "unhealthy" status + - Impact: More vision models pass health checks + - **mlx-vlm #624 specific error message:** Index/shard mismatch now distinguished from incomplete downloads + - Message: "Index/shard mismatch (mlx-vlm #624). Index references N shards but found M different files. Fix: mlxk convert --repair-index" + - Previously: Generic "Missing weight shards" message without fix guidance + - Impact: Users get actionable repair instructions + - **File Integrity Definition documented:** 5-point definition in `_check_snapshot_health()` docstring + - Clarifies: Required files, weight completeness, corruption markers, auxiliary assets + - Emphasizes: Integrity (health) vs Runtime (runtime_compatible) separation + - Ensures: Consistent checks across list/show/health commands + - File: `operations/health.py` + +- **Workspace health check bug:** `build_model_object()` now uses `health_check_workspace()` for workspace paths + - Previously: Workspace paths triggered cache-based health check → "Model not in cache" + - Now: Correct health reasons for workspaces (e.g., "Missing weight shards") + - Impact: `mlxk show ./workspace` displays accurate health status + +- **Path resolution priority:** Explicit path detection prevents cache bypass + - Previously: Any local directory matching model name would bypass cache resolution + - Now: Only paths with `./ ../ /` prefixes or `.` `..` are treated as workspaces + - Impact: `mlxk show Mistral-Small` tries cache first (even if local dir exists) + +- **Sentinel version fix:** Workspace metadata now uses dynamic version + - Previously: Hardcoded `"mlxk_version": "2.0.4"` in clone/convert operations + - Now: Dynamic import from `mlxk2.__init__.__version__` + - Impact: Sentinel metadata automatically tracks current version + - Files: `operations/clone.py`, `operations/convert.py`, `operations/workspace.py` + +- **Clone operation: Ctrl-C now preserves temp cache for resume:** + - Previously: KeyboardInterrupt during `mlxk clone` deleted partial downloads → resume impossible + - Root cause: Cleanup logic checked `result["status"] == "success"` first (initial value) + - Fix: `cancelled_by_user` flag + inner/outer KeyboardInterrupt handlers + correct cleanup order + - Impact: Ctrl-C during clone preserves temp cache, user can resume operation + - Files: `operations/clone.py` (lines 59, 139-143, 242-246, 279-302) + +- **Clone operation: Partial downloads now resumable automatically:** + - Previously: `--force-resume` only worked for "complete but unhealthy" models, not after Ctrl-C + - Root cause: Partial downloads (no `.mlxk2_download_complete` marker) treated as non-resumable + - Fix: `_check_temp_cache_resume()` now treats partial downloads as resumable + - Impact: `mlxk clone` automatically resumes after Ctrl-C (HuggingFace Hub native resume) + - Files: `operations/clone.py` (lines 393-424, 447-469) + - Note: Resume validation overhead (checksum) takes 5-10 min for large models (HF Hub behavior) + +- **Vision server scalability:** + - Server now accepts unlimited images (removed 5-image hardcoded limit from beta.5) + - Added safe chunk size validation (max 5 images per chunk for Metal stability) + - Impact: Large image collections work with server API + - Files: `tools/vision_adapter.py`, `operations/serve.py`, `core/server_base.py` + +- **Vision metadata positioning:** + - Metadata table now appears BEFORE model output (was: after in beta.5) + - Key benefit: Vision model can reference metadata in its analysis (filename, GPS, date visible in prompt context) + - Secondary benefit: Clearer association with output when processing multiple chunks + - File: `core/vision_runner.py` + +### Testing + +- **Unit tests:** 550 passed, 56 skipped (0 failures) + - New: `tests_2.0/test_model_resolution_workspace.py` (9 tests - workspace path resolution) + - Extended: `tests_2.0/test_workspace_sentinel.py` (+32 tests - sentinel version, health checks) + - Extended: `tests_2.0/test_clone_operation.py` (+10 tests - resumable clone, deterministic naming) + - Coverage: workspace detection, path resolution, health checks, resumable clone, sentinel metadata + +- **Vision pipe integration tests:** 3 new smoke tests for Vision→Text workflows + - Test suite: `tests_2.0/live/test_pipe_vision_geo.py` (marker: `live_vision_pipe`) + - Test assets: 9 publishable Stockholm photos with EXIF in `tests_2.0/assets/geo-test/` + - Coverage: Batch processing (`--chunk 1`), complete pipe workflow, chunk isolation + - Model selection: Hardcoded pixtral + portfolio discovery for text model + - **Note:** Smoke tests only (exit 0 = pass), not quality benchmarks + +- **Wet Umbrella test integration:** 161 tests total across 4 phases + - Phase 1: 152 tests (live_e2e marker) + - Phase 2: 3 tests (live_pull marker) + - Phase 3: 3 tests (live_clone marker) + - Phase 4: 3 tests (live_vision_pipe marker) + - Single entry point: `scripts/test-wet-umbrella.sh` + - RAM requirements: 64GB recommended (M2 Max tested), 32GB untested + +- **Test marker architecture improvements:** + - Added umbrella marker `pytest.mark.live` to all 11 live test files + - Simplified exclusion: `pytest -m "not live"` (future-proof, scalable) + - Old approach: Blacklist individual markers (doesn't scale with new test categories) + - Updated: `test-multi-python.sh` uses umbrella marker for clean isolation + - Documentation: TESTING-DETAILS.md "Writing New Live Tests: Umbrella Marker Convention" + +- **Test marker refactoring:** `live_resumable` → `live_pull` for consistency + - Renamed marker to match `live_push`, `live_list` pattern + - Updated Wet Umbrella script: 4-Phase separation (wet, live_pull, live_clone, live_vision_pipe) + - Files: `pytest.ini`, `conftest.py`, `scripts/test-wet-umbrella.sh`, `TESTING-DETAILS.md` + +### Documentation + +- **README.md:** Model reference path conventions documented +- **operations/health.py:** File Integrity Definition documented in `_check_snapshot_health()` docstring + +--- + ## [2.0.4-beta.5] - 2025-12-31 ### Added diff --git a/README.md b/README.md index cfbf802..5dfed30 100644 --- a/README.md +++ b/README.md @@ -4,11 +4,11 @@ MLX Knife Demo

-**Current Version: 2.0.4-beta.5** (Stable: 2.0.3) +**Current Version: 2.0.4-beta.6** (Stable: 2.0.3) -[![GitHub Release](https://img.shields.io/badge/version-2.0.4--beta.5-blue.svg)](https://github.com/mzau/mlx-knife/releases) +[![GitHub Release](https://img.shields.io/badge/version-2.0.4--beta.6-blue.svg)](https://github.com/mzau/mlx-knife/releases) [![License: Apache 2.0](https://img.shields.io/badge/License-Apache%202.0-blue.svg)](https://www.apache.org/licenses/LICENSE-2.0) -[![Python 3.9+](https://img.shields.io/badge/python-3.9+-blue.svg)](https://www.python.org/downloads/) +[![Python 3.9+](https://img.shields.io/badge/python-3.10+(3.9)-blue.svg)](https://www.python.org/downloads/) [![Apple Silicon](https://img.shields.io/badge/Apple%20Silicon-green.svg)](https://support.apple.com/en-us/HT211814) [![MLX](https://img.shields.io/badge/MLX-Latest-orange.svg)](https://github.com/ml-explore/mlx) @@ -17,13 +17,20 @@ ## Features +### What's New in 2.0.4 (Coming Soon - Currently Beta) +- **Vision Models with EXIF Metadata** - Image analysis + automatic GPS/date/camera extraction visible to the model +- **Unix Pipe Integration** - Chain models without temp files (`vision → text` workflows) +- **Local Development Workflow** - Clone → Repair → Test models without HuggingFace round-trips +- **Community Model Repair Tool** - Fix broken mlx-vlm models with `--repair-index` +- **Resumable Downloads** - Interrupted clone/pull operations continue automatically +- **Safe Vision Batch Processing** - Automatic chunking prevents Metal OOM crashes +- **Workspace Path Support** - Run/show/server commands work with local directories + ### Core Functionality - **List & Manage Models**: Browse your HuggingFace cache with MLX-specific filtering - **Model Information**: Detailed model metadata including quantization info - **Download Models**: Pull models from HuggingFace with progress tracking - **Run Models**: Native MLX execution with streaming and chat modes -- **Vision Models**: Image analysis (Python 3.10+, beta) -- **Unix Pipes**: Chain models via stdin/stdout - no temp files (beta) - **Health Checks**: Verify model integrity and MLX runtime compatibility - **Cache Management**: Clean up and organize your model storage - **Privacy & Network**: No background network or telemetry; only explicit Hugging Face interactions when you run pull or the experimental push. @@ -99,17 +106,17 @@ mlxk --version # → mlxk 2.0.3 (latest stable on PyPI) ### Via GitHub (Latest Beta) ```bash -# Install 2.0.4-beta.5 (Community repair tools + BPE fix) -pip install "git+https://github.com/mzau/mlx-knife.git@v2.0.4-beta.5" +# Install 2.0.4-beta.6 (Workspace operations + Resumable clone) +pip install "git+https://github.com/mzau/mlx-knife.git@v2.0.4-beta.6" # With Vision support (Python 3.10+ required) -pip install "git+https://github.com/mzau/mlx-knife.git@v2.0.4-beta.5#egg=mlx-knife[vision]" +pip install "git+https://github.com/mzau/mlx-knife.git@v2.0.4-beta.6#egg=mlx-knife[vision]" # Verify installation -mlxk --version # → mlxk 2.0.4b5 +mlxk --version # → mlxk 2.0.4b6 ``` -**Beta.5 note:** Uses mlx-vlm commit c536165df2b3b4aece3a795b2e414349f935e750 (includes Pixtral text-only fix). The `[vision]` extra automatically installs the correct version. +**Beta.6 note:** Uses mlx-vlm commit c536165df2b3b4aece3a795b2e414349f935e750 (includes Pixtral text-only fix). The `[vision]` extra automatically installs the correct version. **For production use:** Wait for 2.0.4 stable on PyPI (requires mlx-vlm 0.3.10 release). @@ -169,6 +176,83 @@ mlxk run "Phi-3-mini" -c mlxk serve --port 8080 ``` +## Model References + +MLX-Knife supports multiple ways to reference models: + +### HuggingFace Models (Cache) + +| Format | Example | Description | +|--------|---------|-------------| +| Full name | `mlx-community/Phi-4-4bit` | Exact HuggingFace repo ID | +| Short name | `Phi-4` | Fuzzy match against cache | +| With hash | `Phi-4@e96f3b2` | Specific commit/version | + +```bash +mlxk run "mlx-community/Phi-4-4bit" "Hello" +mlxk run "Phi-4" "Hello" # Fuzzy match +mlxk show "Qwen3@e96" --json # Specific version +``` + +### Local Paths (2.0.4-beta.6+) + +| Format | Example | +|--------|---------| +| Relative | `./my-workspace` | +| Absolute | `/Volumes/External/model` | +| Home | `~/models/fine-tuned` | + +```bash +# Clone → Run +mlxk clone org/model ./workspace +mlxk run ./workspace "Hello" + +# Convert → Run +mlxk convert ./broken ./fixed --repair-index +mlxk run ./fixed "Test" +``` + +**Disambiguating paths vs cache names:** When a local directory exists with the same name as a cached model, use `./` prefix to force workspace resolution. Otherwise, cache lookup is attempted first. + +--- + +## Workspace Development Workflow (2.0.4-beta.6+) + +**Complete local development cycle** for model experimentation, repair, and testing without HuggingFace round-trips: + +```bash +export MLXK2_ENABLE_ALPHA_FEATURES=1 + +# Clone → Repair → Test → Publish (optional) +mlxk clone "model" ./workspace +mlxk convert ./workspace ./fixed --repair-index # Fix mlx-vlm #624 models +mlxk run ./fixed "test prompt" # Local inference +mlxk server --model ./fixed # Dev server +mlxk push ./fixed "your-org/model" # Optional publish +``` + +**Key capabilities:** +- **Model repair:** Fix index/shard mismatches from mlx-vlm #624 +- **Local testing:** Run/server/show without pushing to HuggingFace +- **Side-by-side comparison:** Multiple workspaces with parallel servers +- **Rapid iteration:** Clone → Modify → Test loop + +### Workspace Commands Reference + +| Command | Workspace Support | Example | +|---------|-------------------|---------| +| `run` | ✅ Yes | `mlxk run ./workspace "prompt"` | +| `show` | ✅ Yes | `mlxk show ./workspace --files` | +| `health` | ✅ Yes | `mlxk health ./workspace` | +| `server` | ✅ Yes | `mlxk server --model ./workspace` | +| `clone` | ✅ Creates | `mlxk clone model ./workspace` | +| `convert` | ✅ Yes | `mlxk convert ./in ./out --repair-index` | +| `push` | ✅ Yes | `mlxk push ./workspace "org/name"` | +| `pull` | ❌ Cache only | Downloads to HuggingFace cache | +| `list` | ❌ Cache only | Lists cached models only | + +--- + ## Web Interface For a web-based chat UI, use **[nChat](https://github.com/mzau/broke-nchat)** - a lightweight web interface for the BROKE ecosystem: @@ -182,7 +266,7 @@ cd broke-nchat mlxk serve # Open web UI: -open index.html +open webui/index.html ``` **On-Prem:** Pure HTML/CSS/JS - runs entirely locally, zero dependencies. @@ -240,24 +324,73 @@ mlxk run vision-model --image cat.jpg mlxk run "mlx-community/Llama-3.2-11B-Vision-Instruct-4bit" "What is 2+2?" ``` +#### Batch Processing + +**Breaking Change (2.0.4-beta.6):** Vision processing now defaults to processing **one image at a time** for maximum stability on all systems. Use `--chunk N` to process multiple images per batch when your system can handle it. + +```bash +# Default: one image at a time (most robust, automatic chunking) +mlxk run pixtral "Describe image" --image photos/*.jpg + +# Faster: 5 images per batch (requires more RAM, may trigger model-specific issues) +mlxk run pixtral "Describe images" --chunk 5 --image photos/*.jpg + +# Alternative: Use --prompt flag (useful when experimenting with different prompts) +mlxk run pixtral --chunk 5 --image photos/*.jpg --prompt "Describe images" + +# Set default batch size via environment variable +export MLXK2_VISION_BATCH_SIZE=3 +mlxk run pixtral "Describe images" --image photos/*.jpg +``` + +**Why chunking?** +- **Safety:** Prevents Metal OOM crashes with large image batches +- **Isolation:** Fresh model load per chunk prevents state leakage between batches +- **Trade-off:** ~2-3s model load overhead per chunk vs guaranteed isolation + +**⚠️ Important:** Some vision models may hallucinate details about non-existent images when: +- Processing larger chunks (model sees global context like "8 total images" but only has 4 in current batch) +- Prompts use plural forms that don't match actual image count (e.g., "describe these images" when chunk=1) + +Default chunk=1 with singular prompts provides maximum robustness. + +**Server API:** +```bash +curl -X POST http://localhost:8000/v1/chat/completions \ + -H "Content-Type: application/json" \ + -d '{"model": "pixtral", "chunk": 3, "messages": [...]}' +``` + #### Metadata Output Format -When processing images, MLX Knife automatically appends metadata in a **collapsible table** (collapsed by default): +When processing images, MLX Knife automatically prepends metadata in a **collapsible table** (collapsed by default) **before** the model output: ``` -A beach with palm trees and clear blue water. -
-📸 Image Metadata (2 images) +📸 Batch 1/3: Images 1-4 | Image | Filename | Original | Location | Date | Camera | |-------|----------|----------|----------|------|--------| | 1 | image_abc123.jpeg | beach.jpg | 📍 32.79°N, 16.92°W | 📅 2023-12-06 12:19 | 📷 Apple iPhone SE | | 2 | image_def456.jpeg | mountain.jpg | 📍 32.87°N, 17.17°W | 📅 2023-12-10 15:42 | 📷 Apple iPhone SE | +| 3 | image_xyz789.jpeg | sunset.jpg | 📍 32.82°N, 17.05°W | 📅 2023-12-08 18:30 | 📷 Apple iPhone SE | +| 4 | image_uvw456.jpeg | forest.jpg | 📍 32.88°N, 17.12°W | 📅 2023-12-09 10:15 | 📷 Apple iPhone SE |
+ +A beach with palm trees and clear blue water. A mountain landscape with snow-capped peaks... ``` +**Batch information in summary:** +- Shows current batch and total batches (e.g., "Batch 1/3") +- Shows image range in current batch (e.g., "Images 1-4") +- Helps track progress in WebUI and prevents confusion about which images are being described +- **Important:** Batch context prevents hallucination by making scope clear to both model and user + +**Why metadata comes first:** +- Vision models can **reference metadata in their analysis** (filename, GPS coordinates, date visible in prompt context) +- Clearer association with output when processing multiple chunks + **Metadata includes:** - **Image ID** → **Filename mapping** (identify which description belongs to which file) - **GPS coordinates** (latitude/longitude, if available in EXIF) @@ -278,7 +411,11 @@ mlxk run vision-model --image photo.jpg "describe" #### Limitations - **Non-streaming:** Vision runs always use batch mode (no streaming output) -- **Image limits:** 5 images max per request, 20 MB per image, 50 MB total +- **Image limits:** Model-dependent due to Metal buffer constraints (~41.7GB on Apple Silicon) + - **pixtral-12b-8bit:** Up to 5 images tested on M2 Max 64GB (multi-image capable) + - **Llama-3.2-11B / Other models:** Single-image only + - **Larger models (24B+):** Limited to 1-2 images on 64GB RAM + - **Per-image:** 20 MB max, 50 MB total per request #### Server API @@ -297,6 +434,36 @@ curl http://localhost:8000/v1/chat/completions -H "Content-Type: application/jso }' ``` +#### Model Compatibility + +**⚠️ Important:** Vision support relies on mlx-vlm (upstream), which has known stability issues. While `mlxk health` verifies file integrity, **runtime failures may occur** with certain model architectures due to upstream bugs. + +**✅ Tested & Working Models** (mlx-knife v2.0.4-beta.6): + +| Model | Size | Notes | +|-------|------|-------| +| `mlx-community/pixtral-12b-8bit` | ~13.5GB | **⭐ Recommended:** Excellent text recognition, multi-image support (5+ images on M2 Max 64GB); beta.4+ for text-only | +| `mlx-community/Llama-3.2-11B-Vision-Instruct-4bit` | ~6.5GB | Reliable, good quality; single-image only | +| `Devstral-Small-2-24B-Instruct-2512-6bit` | ~14GB | Single-image only on 64GB RAM; requires `--repair-index` (mlx-vlm #624); better for Mac Studio/Ultra | + +**❌ Known Issues** (as of 2026-01-03): + +| Model | Issue | Workaround | +|-------|-------|------------| +| `Mistral-Small-3.1-24B-Instruct-2503-4bit` | Vision feature mismatch (5476 positions ≠ 1369 features). **Note:** `--repair-index` fixes mlx-vlm #624 but NOT this bug | Use alternative models (pixtral-12b-8bit, Llama-3.2-11B) | +| `MiMo-VL-7B-RL-bf16` | NoneType iteration error | mlx-vlm processor bug, no workaround | +| `DeepSeek-OCR-8bit` | Runs but hallucinates details | Quality issue, not recommended | + +**Models affected by mlx-vlm #624** (index/shard mismatch) can be repaired: + +```bash +mlxk convert --repair-index +``` + +**Recommendation:** Test models with real images before production use. The vision ecosystem is evolving rapidly - this list will be updated as mlx-vlm matures. + +**Reporting Issues:** If you encounter vision model failures, please report with model name and error message to help improve compatibility tracking. + ## JSON API @@ -408,16 +575,6 @@ mlxk show "Phi-3-mini" --json --files } ``` -### Hash Syntax Support - -All commands support `@hash` syntax for specific model versions: - -```bash -mlxk health "Qwen3@e96" --json # Check specific hash -mlxk show "model@3df9bfd" --json # Short hash matching -mlxk rm "Phi-3@e967" --json --force # Delete specific version -``` - ### Integration Examples #### Broke-Cluster Integration @@ -674,7 +831,26 @@ Control server behavior without command-line flags: | `MLXK2_MAX_TOKENS` | Override default max_tokens for all requests | (auto) | 2.0.4 | | `MLXK2_RELOAD` | Enable Uvicorn auto-reload (development only) | `0` (disabled) | 2.0.0 | +### Vision Processing + +Control vision model behavior (Python 3.10+, beta): + +| Variable | Description | Default | Since | +|----------|-------------|---------|-------| +| `MLXK2_VISION_BATCH_SIZE` | Default chunk size for vision image processing | `1` | 2.0.4-beta.6 | + **Examples:** +```bash +# Process 3 images per batch instead of 1 (faster but requires more RAM) +export MLXK2_VISION_BATCH_SIZE=3 +mlxk run pixtral --image photos/*.jpg "Describe images" + +# CLI flag overrides environment variable +mlxk run pixtral --chunk 5 --image photos/*.jpg "Describe images" # Uses 5, not 3 +``` + +### Server Configuration Examples + ```bash # Custom host/port binding MLXK2_HOST=0.0.0.0 MLXK2_PORT=9000 mlxk serve @@ -1003,7 +1179,7 @@ Apache License 2.0 — see `LICENSE` (root) and `mlxk2/NOTICE`.

Made with ❤️ by The BROKE team BROKE Logo
- Version 2.0.4-beta.5 | December 2025
+ Version 2.0.4-beta.6 | January 2026
💬 Web UI: nChat - lightweight chat interface🔮 Multi-node: BROKE Cluster

diff --git a/TESTING-DETAILS.md b/TESTING-DETAILS.md index f886d4e..f657612 100644 --- a/TESTING-DETAILS.md +++ b/TESTING-DETAILS.md @@ -4,9 +4,32 @@ This document contains version-specific details, complete file listings, and imp ## Current Status -✅ **2.0.4-beta.5 (WIP for stable)** — Probe/Policy architecture complete; Vision support Phase 1-3 (CLI + Server); Pipes/Memory-Aware; EXIF metadata; **Test Portfolio Separation complete**; Workspace Infrastructure (ADR-018 Phase 0a); Convert Operation (ADR-018 Phase 1). -✅ **Test Suite:** **528 unit tests passing** (Py3.10: 528 passed/60 skipped, baseline without HF_HOME); Live E2E: 144+ passed/21 skipped -✅ **Test environment:** macOS 26.2 (Tahoe), M2 Max, Python 3.9-3.14 +✅ **2.0.4-beta.6** — Probe/Policy architecture complete; Vision support Phase 1-3 (CLI + Server); Pipes/Memory-Aware; EXIF metadata; **Test Portfolio Separation complete**; Workspace Infrastructure (ADR-018 Phase 0a+0b+0c); Convert Operation (ADR-018 Phase 1); Resumable Clone. + +### Test Results (Official Reference) + +**Standard Unit Tests:** +``` +Platform: macOS 26.2 (Tahoe), M2 Max, 64GB RAM +Python: 3.9-3.14 (Multi-Python verified) +Results: 550 passed, 56 skipped +Note: Default suite works on 16GB. Wet-umbrella: 64GB recommended (M1 Max 32GB untested) +``` + +**Live E2E Tests:** +``` +Results: 144+ passed, 21 skipped +``` + +**Wet Umbrella (4-Phase Integration):** +``` +Phase 1 (wet marker): 152 passed, 34 skipped, 579 deselected +Phase 2 (live_pull): 3 passed, 630 deselected +Phase 3 (live_clone): 3 passed, 630 deselected +Phase 4 (live_vision_pipe): 3 passed (requires vision+text models, skips if unavailable) +Total: 161 passed across all phases +``` + ✅ **Production verified & reported:** M1, M1 Max, M2 Max in real-world use ✅ **License:** Apache 2.0 (was MIT in 1.x) ✅ **Isolated test system** - user cache stays pristine with temp cache isolation @@ -57,6 +80,17 @@ This document contains version-specific details, complete file listings, and imp - `mlx-run` wrapper entrypoint argv injection. - Tests added: `tests_2.0/test_cli_run_exit_codes.py` (pipe/JSON/SIGPIPE/BrokenPipe), `tests_2.0/test_cli_run_wrapper.py`, `tests_2.0/live/test_vision_e2e_live.py` (5 vision CLI E2E tests), `tests_2.0/test_server_vision.py` (17 vision server unit tests), `tests_2.0/live/test_vision_server_e2e.py` (3 vision server E2E tests), `tests_2.0/test_portfolio_discovery.py` (10 tests), `tests_2.0/test_ram_calculation.py` (11 tests), `tests_2.0/live/test_portfolio_fixtures.py` (7 validation tests), `tests_2.0/show_portfolios.py` (diagnostic script). +**New coverage in 2.0.4-beta.6:** +- **Vision Batch Processing (ADR-012 Phase 1c):** `--chunk N` flag for processing images in isolated batches + - Default: `--chunk 1` (incremental output, fresh VisionRunner per chunk) + - Server support: Unlimited images with safe chunking (MAX_SAFE_CHUNK_SIZE=5) + - Context-line in prompt: Batch info visible to model and user +- **Vision→Geo Pipe Integration Tests:** Smoke tests for complete pipeline (marker: `live_vision_pipe`) + - **3 tests** in `tests_2.0/live/test_pipe_vision_geo.py`: Vision batch processing, complete pipe workflow, chunk isolation + - Validates: Sessions 72-75 fixes (chunk isolation, pipe stdin + `--prompt`, server chunking) + - Uses: `tests_2.0/assets/geo-test/` (9 JPEGs with EXIF metadata) + - PASSED criteria: Process exits 0, output not empty, mentions expected terms (smoke test only, no quality metrics) + For complete test file structure, see [Appendix](#complete-test-file-structure-201). --- @@ -69,7 +103,8 @@ For complete test file structure, see [Appendix](#complete-test-file-structure-2 | Spec only | `pytest -m spec -v` | `spec` | Schema/contract tests, version sync, docs example validation | No | | Exclude spec | `pytest -m "not spec" -v` | `not spec` | Everything except spec/schema checks | No | | Push offline | `pytest -k push -v` | — | Push offline tests (tests alpha feature: `--check-only`, `--dry-run`, error handling); no network, no credentials needed | No | -| Live pipe mode | `pytest -m live_e2e tests_2.0/live/test_cli_pipe_live.py -v` | `live_e2e`; Env: `HF_HOME`, `MLXK2_ENABLE_PIPES=1` | Stdin `-`, pipe auto-batch, JSON interactive error path, list→run pipe; first eligible model from portfolio discovery | No (uses local cache) | +| Live pipe mode | `MLXK2_ENABLE_PIPES=1 pytest -m live_e2e tests_2.0/live/test_cli_pipe_live.py -v` | `live_e2e`; Env: `HF_HOME`, `MLXK2_ENABLE_PIPES=1` | Stdin `-`, pipe auto-batch, JSON interactive error path, list→run pipe; first eligible model from portfolio discovery | No (uses local cache) | +| Vision→Geo pipe | `MLXK2_ENABLE_PIPES=1 pytest -m live_vision_pipe -v` | `live_vision_pipe` (new marker); Env: `HF_HOME` (requires vision + text models), `MLXK2_ENABLE_PIPES=1`; Optional: `MLXK2_VISION_BATCH_SIZE=N` (default: 1) | **Smoke test for complete Vision→Geo pipeline.** Validates: Vision batch processing (`--chunk 1`), chunk isolation (no state leakage), pipe stdin + `--prompt` combination, geo inference. **PASSED criteria:** Process exits 0, output not empty, output mentions expected terms. Uses `tests_2.0/assets/geo-test/` (9 JPEGs with EXIF). | No (uses local cache) | | Live push | `MLXK2_ENABLE_ALPHA_FEATURES=1 pytest -m live_push -v` | `live_push` (subset of `wet`) + Env: `MLXK2_ENABLE_ALPHA_FEATURES=1`, `MLXK2_LIVE_PUSH=1`, `HF_TOKEN`, `MLXK2_LIVE_REPO`, `MLXK2_LIVE_WORKSPACE` | JSON push against the real Hub; on errors the test SKIPs (diagnostic) | Yes | | Live list | `pytest -m live_list -v` | `live_list` (subset of `wet`) + Env: `HF_HOME` (user cache with models) | Tests list/health against user cache models | No (uses local cache) | | Clone offline | `pytest -k clone -v` | — | Clone offline tests (tests alpha feature: APFS validation, temp cache, CoW workflow); no network needed | No | @@ -79,7 +114,7 @@ For complete test file structure, see [Appendix](#complete-test-file-structure-2 | Live E2E (ADR-011) | `HF_HOME=/path/to/cache pytest -m live_e2e -v` | `live_e2e` (required) + Env: `HF_HOME` (optional, enables Portfolio Discovery); Requires: `httpx` installed | **✅ Working:** Server/HTTP/CLI validation with real models. Portfolio Discovery auto-discovers all MLX chat models via `mlxk list --json` (filter: MLX+healthy+runtime+chat), parametrized tests (one server per model), RAM-aware skip. | No (uses local cache) | | Vision CLI E2E (ADR-012) | `HF_HOME=/path/to/cache pytest -m live_e2e tests_2.0/live/test_vision_e2e_live.py -v` | `live_e2e` (required) + Env: `HF_HOME` (vision model in cache, e.g., pixtral-12b-8bit or Llama-3.2-Vision); Requires: `mlx-vlm` installed (Python 3.10+) | **✅ Working:** Deterministic vision queries validate actual image understanding (not hallucination). Tests: chess position reading (e6=black king), OCR text extraction (contract name), color recognition (blue mug), chart label reading (Y-axis), large image support (2.7MB). | No (uses local cache) | | Vision Server E2E (ADR-012 Phase 3) | `HF_HOME=/path/to/cache pytest -m live_e2e tests_2.0/live/test_vision_server_e2e.py -v` | `live_e2e` (required) + Env: `HF_HOME` (vision model in cache); Requires: `mlx-vlm` installed (Python 3.10+), `httpx` | **✅ Working:** Vision API over HTTP. Tests: Base64 image chat completion, streaming graceful degradation (SSE emulation), text request on vision model server. | No (uses local cache) | -| Resumable Pull | `MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_resumable tests_2.0/test_resumable_pull.py -v` | `live_resumable` (required) + Env: `MLXK2_TEST_RESUMABLE_DOWNLOAD=1` (opt-in for network test) | **✅ Working:** Real network download with controlled interruption (45s timer). Tests unhealthy detection → `requires_confirmation` status → resume with `force_resume=True` → final health check. Validates resumable pull feature (interrupted downloads can be resumed). Uses isolated cache (no impact on user cache). | Yes (HuggingFace download) | +| Resumable Pull | `MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_pull tests_2.0/test_resumable_pull.py -v` | `live_pull` (required) + Env: `MLXK2_TEST_RESUMABLE_DOWNLOAD=1` (opt-in for network test) | **✅ Working:** Real network download with controlled interruption (45s timer). Tests unhealthy detection → `requires_confirmation` status → resume with `force_resume=True` → final health check. Validates resumable pull feature (interrupted downloads can be resumed). Uses isolated cache (no impact on user cache). | Yes (HuggingFace download) | | Show E2E portfolios | `HF_HOME=/path/to/cache python tests_2.0/show_portfolios.py` OR `pytest -m show_model_portfolio -s` | Env: `HF_HOME` | Displays TEXT and VISION portfolios separately. Shows model keys (text_XX, vision_XX), RAM requirements, and test/skip status. Diagnostic tool for understanding portfolio separation. Use script for detailed output, or pytest marker for quick check. | No (uses local cache) | | Manual debug mode | `mlxk run "test prompt" --verbose` | Manual CLI usage with `--verbose` flag | Shows token generation details including multiple EOS token warnings. Use this for manual debugging of model quality issues. Output includes `[DEBUG] Token generation analysis` and `⚠️ WARNING: Multiple EOS tokens detected` for broken models. | No (uses local cache) | | Issue #27 real-model | `pytest -m issue27 tests_2.0/test_issue_27.py -v` | Marker: `issue27`; Env (required): `MLXK2_USER_HF_HOME` or `HF_HOME` (user cache, read-only). Env (optional): `MLXK2_ISSUE27_MODEL`, `MLXK2_ISSUE27_INDEX_MODEL`, `MLXK2_SUBSET_COUNT=0`. | Copies real models from user cache into isolated test cache; validates strict health policy on index-based models (no network) | No (uses local cache) | @@ -118,7 +153,7 @@ HF_HOME=/path/to/user/cache pytest -m live_run -v HF_HOME=/path/to/user/cache pytest -m live_e2e -v # See model list: pytest tests_2.0/live/test_server_e2e.py::TestChatCompletionsBatch --collect-only -q # Resumable Pull only (separate run - uses isolated cache) -MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_resumable tests_2.0/test_resumable_pull.py -v +MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_pull tests_2.0/test_resumable_pull.py -v # Empirical Mapping only (model benchmarking - excluded from wet due to RAM) pytest -m live_stop_tokens tests_2.0/test_stop_tokens_live.py::TestStopTokensEmpiricalMapping -v @@ -128,6 +163,12 @@ MLXK2_USER_HF_HOME=/path/to/user/cache pytest -m issue27 tests_2.0/test_issue_27 # All live tests (umbrella) MLXK2_ENABLE_ALPHA_FEATURES=1 pytest -m wet -v + +# Vision→Geo pipe only (ADR-012 Phase 1c + Pipe integration) +MLXK2_ENABLE_PIPES=1 pytest -m live_vision_pipe -v + +# With custom batch size (optional) +MLXK2_ENABLE_PIPES=1 MLXK2_VISION_BATCH_SIZE=3 pytest -m live_vision_pipe -v ``` --- @@ -294,9 +335,10 @@ Portfolio Discovery fixtures (`vision_portfolio`, `text_portfolio`) use module s | Portfolio Discovery | User READ | vision/text_portfolio | ✅ (source) | live_e2e | wet | | Stop Token Validation | User READ | vision/text_portfolio | ✅ (uses it) | live_stop_tokens| wet | | User Cache Read | User READ | isolated_cache (copy) | ✅ (no conflict) | live_run/list | wet | -| Workspace Operations | N/A | tmp_path | ✅ (cache-agnostic) | live_push/clone | wet | +| Workspace Operations | N/A | tmp_path | ✅ (cache-agnostic) | live_push | wet | | Issue Reproduction | User→Iso | isolated_cache + copy | ✅ (no conflict) | issue27 | wet | -| Isolated Cache Write | Isolated | isolated_cache (fresh) | ❌ (import conflict) | live_resumable | separate | +| Isolated Cache Write (Pull) | Isolated | isolated_cache (fresh) | ❌ (import conflict) | live_pull | separate | +| Isolated Cache Write (Clone) | Isolated + tmp_path | temp cache (fresh) | ❌ (import conflict) | live_clone | separate | **Run Groups:** - `wet`: Can run together in one pytest invocation @@ -316,14 +358,29 @@ When writing a new test, follow this decision tree: └─ NO → Continue to Question 2 **Question 2:** Does your test need fresh downloads (Isolated Cache WRITE)? -├─ YES → Marker: `live_resumable` → Run group: **separate** ⚠️ +├─ YES (Pull) → Marker: `live_pull` → Run group: **separate** ⚠️ +├─ YES (Clone with internal pull) → Marker: `live_clone` → Run group: **separate** ⚠️ └─ NO → Continue to Question 3 **Question 3:** What does your test use? ├─ User Cache (READ only) → Markers: `live_run`, `live_list`, `issue27` → Run group: **wet** ✅ -├─ Workspace (tmp_path) → Markers: `live_push`, `live_clone` → Run group: **wet** ✅ +├─ Workspace (tmp_path only) → Marker: `live_push` → Run group: **wet** ✅ └─ Isolated Cache (copy/mock) → Standard markers → Unit test (not live) +### Writing New Live Tests: Umbrella Marker Convention + +**CRITICAL:** All live tests (real models/network/user cache) MUST include `pytest.mark.live`: + +```python +# tests_2.0/live/test_my_new_live_test.py +pytestmark = [pytest.mark.live, pytest.mark.live_e2e] # Umbrella + specific marker + +def test_my_feature(text_portfolio): + pass +``` + +**Why:** Default test run excludes ALL `live` tests via `pytest -m "not live"` (used in `test-multi-python.sh`). New live tests are automatically excluded without script changes. + ### Compatibility Rule (Technical Background) **Why separate runs?** @@ -339,7 +396,7 @@ Portfolio Discovery fixtures manipulate import state via subprocesses: - Tests with Workspace (cache-agnostic) **Incompatible tests:** Require clean import state -- `live_resumable`: HuggingFace Hub needs clean imports for symlink creation +- `live_pull`: HuggingFace Hub needs clean imports for symlink creation - Fresh downloads fail with polluted `sys.modules` **Solution:** Separate pytest runs ensure clean import state for incompatible tests. @@ -396,7 +453,7 @@ tests_2.0/ ``` **Verification:** -- Test in `tests_2.0/test_resumable_pull.py` with marker `live_resumable` should collect as 1 test (NOT parametrized) +- Test in `tests_2.0/test_resumable_pull.py` with marker `live_pull` should collect as 1 test (NOT parametrized) - Test in `tests_2.0/live/test_server_e2e.py` with `text_model_key` should parametrize across portfolio ### Test Categories by Cache Strategy @@ -1180,7 +1237,6 @@ LIVE_MARKERS_FOR_WET = { # Workspace operations "live_push", # Workspace only (tmp_path) - "live_clone", # Workspace only (tmp_path) } def pytest_collection_modifyitems(config, items): @@ -1192,8 +1248,8 @@ def pytest_collection_modifyitems(config, items): # Wet marker for compatible tests if (test_markers & LIVE_MARKERS_FOR_WET) or is_in_live_dir: - # EXCLUDE live_resumable (incompatible) - if "live_resumable" not in test_markers: + # EXCLUDE Isolated Cache WRITE tests (incompatible) + if "live_pull" not in test_markers and "live_clone" not in test_markers: item.add_marker(pytest.mark.wet) ``` @@ -1212,7 +1268,7 @@ def vision_portfolio(): # Subprocess imports pollute pytest process sys.modules ``` -**Why live_resumable breaks:** +**Why live_pull breaks:** ```python # test_resumable_pull.py:190 pull_operation(model, force_resume=True) # In pytest process @@ -1270,7 +1326,7 @@ These variables enable optional live tests that interact with real models or ext | `MLXK2_ISSUE27_INDEX_MODEL` | Index-based model for Issue #27 | `pytest -m issue27` | | `MLXK2_SUBSET_COUNT` | Limit Issue #27 test count | `pytest -m issue27` | | `MLXK2_BOOTSTRAP_INDEX` | Auto-download model for Issue #27 | `pytest -m issue27` | -| `MLXK2_TEST_RESUMABLE_DOWNLOAD` | Enable resumable pull tests (requires network) | `pytest -m live_resumable tests_2.0/test_resumable_pull.py` | +| `MLXK2_TEST_RESUMABLE_DOWNLOAD` | Enable resumable pull tests (requires network) | `pytest -m live_pull tests_2.0/test_resumable_pull.py` | **Example:** ```bash @@ -1325,6 +1381,7 @@ tests_2.0/ │ ├── test_cli_pipe_live.py # Pipe-mode E2E (stdin '-', JSON interactive error, list→run pipe) using first eligible model │ ├── test_clone_live.py # Live clone flow (requires MLXK2_LIVE_CLONE, HF_TOKEN) │ ├── test_list_human_live.py # Live list/health against user cache (requires HF_HOME) +│ ├── test_pipe_vision_geo.py # Vision→Geo pipe integration tests (marker: live_vision_pipe, 3 smoke tests: batch processing, complete pipe, chunk isolation) │ ├── test_portfolio_fixtures.py # Portfolio separation validation tests (7 tests: fixture behavior, disjoint check) │ ├── test_push_live.py # Live push flow (requires MLXK2_LIVE_PUSH, HF_TOKEN) │ ├── test_server_e2e.py # Server E2E tests with TEXT models (ADR-011 + Portfolio Separation, parametrized: text_XX) diff --git a/TESTING.md b/TESTING.md index 4f7a24a..ff3bce4 100644 --- a/TESTING.md +++ b/TESTING.md @@ -196,7 +196,7 @@ See [TESTING-DETAILS.md → Truth Table](TESTING-DETAILS.md#truth-table-cache-ty **Benefits:** - Fast test runs (seconds instead of minutes) -- Low RAM usage (16GB sufficient) +- Low RAM usage (default suite: 16GB sufficient) - No model downloads required - Deterministic behavior @@ -231,14 +231,24 @@ pytest -k "test_name" -v # Run specific test ### Required Setup 1. **Apple Silicon Mac (M1/M2/M3)** - Required (MLX uses Metal) 2. **Python 3.9 or newer** -3. **16GB RAM minimum** -4. **~10-20MB disk space** for test temp files +3. **RAM Requirements:** + - **Default suite:** 16GB minimum (isolated tests, mock models) + - **Live E2E tests:** 32GB minimum (real models, Portfolio Discovery) + - **Full suite (wet-umbrella):** **64GB recommended** + - Wet umbrella Phase 4 (Vision→Geo pipe): ~29GB peak observed (M2 Max) + - Sequential loading: Vision unloads before text model loads (not parallel) + - Portfolio Discovery selects largest eligible models for quality + - **Tested:** M2 Max 64GB (comfortable headroom) + - **Untested:** M1 Max 32GB (theoretically viable but Metal limits unknown) + - **Note:** Metal memory limits may vary by chip generation +4. **~10-20MB disk space** for test temp files (default suite) 5. **Test dependencies:** ```bash pip install -e .[test] ``` -**That's it!** Default tests use mock models - no HF cache or downloads needed. +**Default suite (16GB):** Mock models, fast, no downloads needed. +**Full suite (64GB):** Real models, comprehensive validation, recommended for development. ### Optional Setup (Live Tests) @@ -374,22 +384,14 @@ pytest path/to/test.py::test_name -v -s ## Contributing Tests -When submitting PRs with test changes, please include: - -1. **Test environment:** - - macOS version - - Apple Silicon chip (M1/M2/M3/M4/M5) - - Python version - -2. **Test results** (example): - ``` - Platform: macOS 26.2 (Tahoe), M2 Max - Python: 3.10.x - Results: 528 passed, 60 skipped - ``` +When submitting PRs with test changes, please document in the PR description: +1. **Test environment** (macOS version, Apple Silicon chip, Python version) +2. **Test results** (passed/skipped/failed counts) 3. **Any issues encountered** and resolutions +See [TESTING-DETAILS.md](TESTING-DETAILS.md#current-status) for the current official test environment and results as an example. + ## Development Workflow Before committing: diff --git a/docs/json-api-specification.md b/docs/json-api-specification.md index 8bc0592..ab8c073 100644 --- a/docs/json-api-specification.md +++ b/docs/json-api-specification.md @@ -149,14 +149,15 @@ All commands that return model information use the same minimal model object. - `health`: "healthy" | "unhealthy" (always present). - `runtime_compatible`: `true` | `false` (0.1.5+, always present). - `reason`: `string | null` (0.1.5+, describes first problem found, null when both checks pass). -- `cached`: true. +- `cached`: `true` for cache-managed models (HuggingFace cache), `false` for workspace paths (user-managed local directories). Notes: - No human-readable `size` field; only `size_bytes`. - No human-readable "modified" field; `last_modified` is authoritative. -- No absolute filesystem paths are exposed. +- No absolute filesystem paths are exposed (except for workspace paths where `name` is the full path). - `runtime_compatible` and `reason` fields added in spec version 0.1.5 (Issue #36). - `vision` capability added in 0.1.5 as a backward-compatible enum extension (ADR-012 Phase 1a). +- `cached` field distinguishes cache-managed models (`true`) from workspace paths (`false`). Workspace support added in ADR-018 Phase 0c. ### Supported Commands @@ -168,11 +169,14 @@ Notes: | `pull` | Download models from HuggingFace | ✅ | - | | `rm` | Delete models from cache | ✅ | - | | `clone` | Clone models to workspace directory | ✅ | `MLXK2_ENABLE_ALPHA_FEATURES=1` | +| `convert` | Repair vision model index files (--repair-index) | ✅ | `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. +**Notes:** +- Commands marked with Alpha Feature require `MLXK2_ENABLE_ALPHA_FEATURES=1` environment variable to be available. +- **Workspace Path Support (ADR-018 Phase 0c):** Commands `show`, `run`, `serve`/`server`, and `health` now accept workspace paths (e.g., `./workspace` or `/absolute/path`) in addition to HuggingFace model IDs. Models in workspaces return `"cached": false` to distinguish them from cache-managed models. ## Model Discovery & Metadata @@ -218,6 +222,34 @@ Notes: } ``` +**Workspace Path Example (Phase 0c, ADR-018):** +```json +{ + "status": "success", + "command": "show", + "data": { + "model": { + "name": "/Users/dev/my-workspace", + "hash": null, + "size_bytes": 4613734656, + "last_modified": "2025-12-29T14:30:00Z", + "framework": "MLX", + "model_type": "chat", + "capabilities": ["text-generation", "chat"], + "health": "healthy", + "runtime_compatible": true, + "reason": null, + "cached": false + }, + "metadata": { + "model_type": "llama", + "quantization": "4bit" + } + }, + "error": null +} +``` + ### `mlxk-json list [pattern] --json` **Basic Usage:** diff --git a/mlxk2/__init__.py b/mlxk2/__init__.py index b04b84c..2084fc6 100644 --- a/mlxk2/__init__.py +++ b/mlxk2/__init__.py @@ -7,4 +7,4 @@ import warnings # Issue parity with 1.1.0 (Issue #22) warnings.filterwarnings('ignore', message='urllib3 v2 only supports OpenSSL 1.1.1+') -__version__ = "2.0.4b5" +__version__ = "2.0.4b6" diff --git a/mlxk2/cli.py b/mlxk2/cli.py index 1fb7fe7..48bf06d 100644 --- a/mlxk2/cli.py +++ b/mlxk2/cli.py @@ -214,6 +214,11 @@ def main(): nargs="*", help="Input prompt (optional - interactive if omitted). Use '-' for stdin (requires MLXK2_ENABLE_PIPES=1).", ) + run_parser.add_argument( + "--prompt", + dest="prompt_flag", + help="Input prompt (alternative to positional argument). Useful when prompt comes after --image flag.", + ) run_parser.add_argument( "--image", nargs='+', @@ -221,6 +226,13 @@ def main(): metavar="FILE", help="Attach image file(s) for vision models. Accepts multiple files per flag or use multiple flags.", ) + run_parser.add_argument( + "--chunk", + type=int, + default=1, + metavar="N", + help="Process images in batches of N (default: 1 for maximum safety)", + ) run_parser.add_argument("--max-tokens", type=int, help="Maximum tokens to generate") run_parser.add_argument("--temperature", type=float, default=0.7, help="Sampling temperature (default: 0.7)") run_parser.add_argument("--top-p", type=float, default=0.9, help="Top-p sampling parameter (default: 0.9)") @@ -240,6 +252,7 @@ def main(): serve_parser.add_argument("--reload", action="store_true", help="Enable auto-reload for development") serve_parser.add_argument("--log-level", default="info", help="Logging level (debug/info/warning/error, default: info)") serve_parser.add_argument("--log-json", action="store_true", help="Output logs in JSON format (for log aggregation)") + serve_parser.add_argument("--chunk", type=int, default=1, metavar="N", help="Default batch size for vision requests (default: 1 for maximum safety, max: 5)") serve_parser.add_argument("--verbose", action="store_true", help="Show detailed output") serve_parser.add_argument("--json", action="store_true", help="Output startup info in JSON format") @@ -374,7 +387,8 @@ def main(): result = clone_operation( model_spec=model_spec, target_dir=args.target_dir, - health_check=not getattr(args, "no_health_check", False) + health_check=not getattr(args, "no_health_check", False), + force_resume=getattr(args, "force_resume", False) ) print_result(result, render_clone, args.json, quiet=getattr(args, "quiet", False)) @@ -402,25 +416,42 @@ def main(): result = rm_operation(args.model, args.force) print_result(result, render_rm, args.json) elif args.command == "run": - prompt_parts = args.prompt if isinstance(args.prompt, list) else ([args.prompt] if args.prompt is not None else []) + # Support both positional prompt and --prompt flag (UX improvement) + # IMPORTANT: Check for stdin ("-") FIRST before applying prompt_flag precedence prompt_value = None pipes_enabled = bool(os.getenv("MLXK2_ENABLE_PIPES")) - if prompt_parts: - first_part = prompt_parts[0] - additional_text = " ".join(prompt_parts[1:]) if len(prompt_parts) > 1 else None + # Normalize positional args + positional_prompt = args.prompt if isinstance(args.prompt, list) else ([args.prompt] if args.prompt is not None else []) - if first_part == "-": - if not pipes_enabled: - result = handle_error("CommandError", "Pipe mode requires MLXK2_ENABLE_PIPES=1") - print_result(result, None, True if args.json else False) - sys.exit(1) - stdin_content = sys.stdin.read() - prompt_value = stdin_content - if additional_text: - prompt_value = f"{stdin_content}\n\n{additional_text}" + # Check if stdin ("-") is in positional args + has_stdin = "-" in positional_prompt if positional_prompt else False + + if has_stdin: + # Stdin mode: Read from pipe + if not pipes_enabled: + result = handle_error("CommandError", "Pipe mode requires MLXK2_ENABLE_PIPES=1") + print_result(result, None, True if args.json else False) + sys.exit(1) + stdin_content = sys.stdin.read() + + # Combine stdin with --prompt flag if both present + if hasattr(args, 'prompt_flag') and args.prompt_flag: + # "- --prompt text" → combine stdin + flag + prompt_value = f"{stdin_content}\n\n{args.prompt_flag}" else: - prompt_value = " ".join(prompt_parts) + # "- additional text" → combine stdin + positional + additional_parts = [p for p in positional_prompt if p != "-"] + if additional_parts: + prompt_value = f"{stdin_content}\n\n{' '.join(additional_parts)}" + else: + prompt_value = stdin_content + elif hasattr(args, 'prompt_flag') and args.prompt_flag: + # --prompt flag (no stdin) + prompt_value = args.prompt_flag + elif positional_prompt: + # Positional prompt (no stdin, no flag) + prompt_value = " ".join(positional_prompt) image_inputs = [] images = getattr(args, "image", None) or [] @@ -457,6 +488,7 @@ def main(): model_spec=args.model, prompt=prompt_value, # Can be None for interactive mode images=image_inputs if images else None, + chunk=args.chunk, stream=stream_mode, max_tokens=getattr(args, "max_tokens", None), temperature=args.temperature, @@ -533,6 +565,7 @@ def main(): max_tokens=getattr(args, "max_tokens", None), reload=getattr(args, "reload", False), log_level=getattr(args, "log_level", "info"), + chunk=getattr(args, "chunk", 1), verbose=getattr(args, "verbose", False), supervise=True ) diff --git a/mlxk2/core/model_resolution.py b/mlxk2/core/model_resolution.py index 4d6838f..b37281f 100644 --- a/mlxk2/core/model_resolution.py +++ b/mlxk2/core/model_resolution.py @@ -3,6 +3,7 @@ from pathlib import Path from typing import Tuple, Optional, List from .cache import get_current_model_cache, hf_to_cache_dir, cache_dir_to_hf +from ..operations.workspace import is_workspace_path def expand_model_name(model_name: str) -> str: @@ -80,15 +81,31 @@ def find_model_by_hash(pattern: str, commit_hash: str) -> Optional[Tuple[Path, s def resolve_model_for_operation(model_spec: str) -> Tuple[Optional[str], Optional[str], Optional[List[str]]]: """Resolve model specification for operations. - + + Supports both HuggingFace model IDs and local workspace paths. + Returns: (resolved_name, commit_hash, ambiguous_matches) - + Examples: 'Phi-3-mini' → ('mlx-community/Phi-3-mini-4k-instruct-4bit', None, None) - 'Qwen3@e96' → ('Qwen/Qwen3-Coder-480B-A35B-Instruct', 'e96', None) + 'Qwen3@e96' → ('Qwen/Qwen3-Coder-480B-A35B-Instruct', 'e96', None) + './workspace' → ('/abs/path/to/workspace', None, None) + '/abs/path/workspace' → ('/abs/path/workspace', None, None) + 'Mistral-Small' → cache resolution (NOT workspace, even if local dir exists) 'ambig' → (None, None, ['model1', 'model2']) """ + # NEW: Check if model_spec is an EXPLICIT workspace path (ADR-018 Phase 0c) + # Only paths starting with ./ ../ / or being . or .. are treated as workspace paths + # This ensures "model-name" goes through cache resolution even if a local dir exists + is_explicit_path = ( + model_spec.startswith(('./', '../', '/')) or + model_spec in ('.', '..') + ) + if is_explicit_path and is_workspace_path(model_spec): + # Explicit workspace path - return absolute path, skip cache logic + return (str(Path(model_spec).resolve()), None, None) + model_name, commit_hash = parse_model_spec(model_spec) # For @hash syntax, find by pattern + hash verification diff --git a/mlxk2/core/runner/__init__.py b/mlxk2/core/runner/__init__.py index afff00d..b722f86 100644 --- a/mlxk2/core/runner/__init__.py +++ b/mlxk2/core/runner/__init__.py @@ -16,6 +16,7 @@ from typing import Optional from ..cache import get_current_model_cache, hf_to_cache_dir from ..model_resolution import resolve_model_for_operation from ..reasoning import ReasoningExtractor, StreamingReasoningParser +from ...operations.workspace import is_workspace_path from .token_limits import get_model_context_length, calculate_dynamic_max_tokens from .chat_format import apply_user_prompt, format_conversation as _format_conversation_helper from .reasoning_format import format_reasoning_response as _format_reasoning_helper @@ -176,7 +177,12 @@ class MLXRunner: # Fallback to provided spec (tests may patch load() to accept any path) resolved_name = str(self.model_spec) - if is_path_like: + # NEW: Check if resolved_name is a workspace path + if is_workspace_path(resolved_name): + # Workspace path - use directly + model_path = Path(resolved_name) + elif is_path_like: + # Cache model - existing logic model_cache_dir = (Path(model_cache) if not isinstance(model_cache, Path) else model_cache) / hf_to_cache_dir(resolved_name) if commit_hash: model_path = model_cache_dir / "snapshots" / commit_hash diff --git a/mlxk2/core/server_base.py b/mlxk2/core/server_base.py index be7db39..ce229a4 100644 --- a/mlxk2/core/server_base.py +++ b/mlxk2/core/server_base.py @@ -4,11 +4,13 @@ Provides REST endpoints for text generation with MLX backend. """ import json +import os import threading import time import uuid from collections.abc import AsyncGenerator from contextlib import asynccontextmanager +from pathlib import Path from typing import Any, Dict, List, Optional, Union from fastapi import FastAPI, HTTPException, Request @@ -69,6 +71,7 @@ class ChatCompletionRequest(BaseModel): stream: Optional[bool] = False stop: Optional[Union[str, List[str]]] = None repetition_penalty: Optional[float] = 1.1 + chunk: Optional[int] = 1 # Vision batch processing (default: 1 for maximum safety) class CompletionResponse(BaseModel): @@ -143,30 +146,62 @@ def get_or_load_model(model_spec: str, verbose: bool = False) -> Any: model_path = None resolved_name = None try: + from ..operations.workspace import is_workspace_path + resolved_name, _, _ = resolve_model_for_operation(model_spec) - cache_root = get_current_model_cache() - cache_dir = cache_root / hf_to_cache_dir(resolved_name or model_spec) - # Debug logging for preload path - logger.debug( - f"Preload path: resolved_name={resolved_name}, cache_dir={cache_dir}", - model=model_spec - ) + # Check if resolved_name is a workspace path (ADR-018 Phase 0c) + if resolved_name and is_workspace_path(resolved_name): + # Workspace path - use directly + model_path = Path(resolved_name) - # ARCHITECTURE.md Principle #6: HTTP 404 = "Model not found in cache" - if not cache_dir.exists(): - raise HTTPException( - status_code=404, - detail=f"Model not found in cache: {model_spec}" + # Debug logging for workspace path + logger.debug( + f"Preload path (workspace): resolved_name={resolved_name}, model_path={model_path}", + model=model_spec ) - # Get actual snapshot path - snapshots_dir = cache_dir / "snapshots" - model_path = cache_dir - if snapshots_dir.exists(): - snapshots = [d for d in snapshots_dir.iterdir() if d.is_dir()] - if snapshots: - model_path = max(snapshots, key=lambda x: x.stat().st_mtime) + # Check workspace exists + if not model_path.exists(): + raise HTTPException( + status_code=404, + detail=f"Workspace not found: {model_spec}" + ) + elif resolved_name: + # Cache model found - existing logic + cache_root = get_current_model_cache() + cache_dir = cache_root / hf_to_cache_dir(resolved_name) + + # Debug logging for preload path + logger.debug( + f"Preload path (cache): resolved_name={resolved_name}, cache_dir={cache_dir}", + model=model_spec + ) + + # Get actual snapshot path + snapshots_dir = cache_dir / "snapshots" + model_path = cache_dir + if snapshots_dir.exists(): + snapshots = [d for d in snapshots_dir.iterdir() if d.is_dir()] + if snapshots: + model_path = max(snapshots, key=lambda x: x.stat().st_mtime) + else: + # Resolution failed - check if local directory exists as fallback + if is_workspace_path(model_spec): + # Local workspace exists - use it + model_path = Path(model_spec).resolve() + resolved_name = str(model_path) + + logger.debug( + f"Preload path (fallback workspace): model_spec={model_spec}, model_path={model_path}", + model=model_spec + ) + else: + # Not found in cache and no local workspace + raise HTTPException( + status_code=404, + detail=f"Model not found in cache: {model_spec}" + ) logger.debug( f"Preload path: model_path={model_path}", @@ -214,8 +249,8 @@ def get_or_load_model(model_spec: str, verbose: bool = False) -> Any: logger.info(f"Loading vision model: {model_spec}", model=model_spec, backend="mlx_vlm") runner = VisionRunner(model_path, resolved_name or model_spec, verbose=verbose) else: - # Text model - use MLXRunner - runner = MLXRunner(model_spec, verbose=verbose, install_signal_handlers=False) + # Text model - use MLXRunner (use resolved_name for workspace support) + runner = MLXRunner(resolved_name or model_spec, verbose=verbose, install_signal_handlers=False) # If shutdown was requested, abort before expensive load if _shutdown_event.is_set(): @@ -816,8 +851,30 @@ async def list_models(): logger.warning(f"Skipping model {model_name} from /v1/models: {e}") continue - # Sort: preloaded model first, then alphabetically by id + # Add preloaded workspace if present and not already in list global _preload_model + if _preload_model: + # Check if it's a workspace path + from ..operations.workspace import is_workspace_path + if is_workspace_path(_preload_model): + # Check if already in list (avoid duplicates) + if not any(m.id == _preload_model for m in model_list): + # Get context length + context_length = None + try: + from .runner import get_model_context_length + context_length = get_model_context_length(_preload_model) + except Exception: + pass + + model_list.append(ModelInfo( + id=_preload_model, # Original path string + object="model", + owned_by="workspace", + context_length=context_length + )) + + # Sort: preloaded model first, then alphabetically by id if _preload_model: def sort_key(model: ModelInfo): # Preloaded model gets priority (0), others sorted alphabetically @@ -1071,6 +1128,62 @@ async def _handle_text_chat_completion(request: ChatCompletionRequest, runner: A ) +def _process_vision_chunks_server( + model_path, + model_name: str, + prompt: str, + images: List[tuple], + chunk_size: int, + image_id_map: Dict[str, int], + max_tokens: Optional[int], + temperature: float, + top_p: float, + repetition_penalty: float, +) -> str: + """Process vision images in batches with isolated model instances per chunk. + + Each chunk creates a fresh VisionRunner to prevent state leakage between batches. + Similar to CLI _process_images_in_chunks but server-optimized + (no verbose output to stderr, uses existing image_id_map). + + Args: + model_path: Path to model snapshot directory + model_name: Model name for VisionRunner + prompt: User prompt + images: Full list of (filename, bytes) tuples + chunk_size: Images per batch + image_id_map: Pre-computed global image IDs (from conversation history) + max_tokens, temperature, top_p, repetition_penalty: Generation params + + Returns: + Combined text with merged filename mappings + """ + from .vision_runner import VisionRunner + + # Split into chunks + chunks = [images[i:i+chunk_size] for i in range(0, len(images), chunk_size)] + + # Process each chunk with fresh runner (prevents state leakage) + all_results = [] + for chunk in chunks: + # Fresh runner per chunk to prevent KV-cache/state accumulation + with VisionRunner(model_path, model_name, verbose=False) as runner: + chunk_result = runner.generate( + prompt=prompt, + images=chunk, + max_tokens=max_tokens, + temperature=temperature, + top_p=top_p, + repetition_penalty=repetition_penalty, + image_id_map=image_id_map, # Global numbering from conversation + total_images=len(images), # Enable chunk context line + ) + all_results.append(chunk_result) + + # Concatenate results + return "\n\n".join(all_results) + + async def _handle_vision_chat_completion(request: ChatCompletionRequest, runner: Any = None) -> ChatCompletionResponse: """Handle vision chat completion with images (ADR-012 Phase 3). @@ -1140,15 +1253,50 @@ async def _handle_vision_chat_completion(request: ChatCompletionRequest, runner: completion_id = f"chatcmpl-{uuid.uuid4()}" created = int(time.time()) - generated_text = runner.generate( - prompt=prompt, - images=images, - max_tokens=get_effective_max_tokens_vision(runner, request.max_tokens), - temperature=0.0, # Experiment: greedy sampling to reduce hallucinations - top_p=request.top_p or 0.9, - repetition_penalty=request.repetition_penalty or 1.0, - image_id_map=image_id_map, - ) + # Get chunk size (with env var override) + chunk_size = request.chunk if request.chunk != 1 else int(os.environ.get("MLXK2_VISION_BATCH_SIZE", "1")) + + # Validate chunk size for Metal API stability + from ..tools.vision_adapter import MAX_SAFE_CHUNK_SIZE + if chunk_size < 1: + raise HTTPException( + status_code=400, + detail=f"chunk size must be at least 1 (got: {chunk_size})." + ) + if chunk_size > MAX_SAFE_CHUNK_SIZE: + raise HTTPException( + status_code=400, + detail=( + f"chunk size too large (max: {MAX_SAFE_CHUNK_SIZE} for Metal API stability). " + f"This limit is based on empirically tested performance." + ) + ) + + if len(images) <= chunk_size: + # Single batch (no chunking) + generated_text = runner.generate( + prompt=prompt, + images=images, + max_tokens=get_effective_max_tokens_vision(runner, request.max_tokens), + temperature=0.0, # Experiment: greedy sampling to reduce hallucinations + top_p=request.top_p or 0.9, + repetition_penalty=request.repetition_penalty or 1.0, + image_id_map=image_id_map, + ) + else: + # Multi-batch chunking - creates fresh runner per chunk + generated_text = _process_vision_chunks_server( + model_path=runner.model_path, + model_name=runner.model_name, + prompt=prompt, + images=images, + chunk_size=chunk_size, + image_id_map=image_id_map, + max_tokens=get_effective_max_tokens_vision(runner, request.max_tokens), + temperature=0.0, + top_p=request.top_p or 0.9, + repetition_penalty=request.repetition_penalty or 1.0, + ) logger.info( f"Vision generation complete: {len(generated_text)} chars", diff --git a/mlxk2/core/vision_runner.py b/mlxk2/core/vision_runner.py index e41409d..4de67ff 100644 --- a/mlxk2/core/vision_runner.py +++ b/mlxk2/core/vision_runner.py @@ -16,6 +16,8 @@ from datetime import datetime from pathlib import Path from typing import Dict, Iterable, Optional, Sequence, Tuple +from ..operations.workspace import is_workspace_path + @dataclass class ExifData: @@ -96,9 +98,17 @@ class VisionRunner: if self._load is None or self._generate is None: raise RuntimeError("mlx-vlm is missing load()/generate() API") - # mlx-vlm expects HF repo_id, not local path - # local_files_only=True: Use mlx-knife's cache only, never download (pull's responsibility) - loaded = self._load(self.model_name, local_files_only=True) + # Check if model_path is a workspace directory + if is_workspace_path(self.model_path): + # Workspace path - use model_path directly + model_ref = str(self.model_path) + loaded = self._load(model_ref) # No local_files_only needed for direct path + else: + # HF repo_id - use model_name with local_files_only + # local_files_only=True: Use mlx-knife's cache only, never download (pull's responsibility) + model_ref = self.model_name + loaded = self._load(model_ref, local_files_only=True) + if isinstance(loaded, tuple): # Common pattern: (model, processor) self.model = loaded[0] if len(loaded) > 0 else None @@ -112,8 +122,8 @@ class VisionRunner: if self.model is None: raise RuntimeError("mlx-vlm load() returned no model") - # Load config for chat template (local cache only) - self.config = self._load_config(self.model_name, local_files_only=True) + # Load config for chat template (use same model_ref) + self.config = self._load_config(model_ref, local_files_only=(model_ref == self.model_name)) def _prepare_images(self, images: Sequence[Tuple[str, bytes]] | None): """ @@ -148,6 +158,7 @@ class VisionRunner: top_p: float = 0.9, repetition_penalty: float = 1.0, image_id_map: Optional[Dict[str, int]] = None, + total_images: Optional[int] = None, ) -> str: """Generate a response with optional images. Non-streaming. @@ -160,15 +171,21 @@ class VisionRunner: repetition_penalty: Repetition penalty image_id_map: Optional mapping of content_hash -> image_id for stable numbering across requests. If None, uses request-scoped IDs. + total_images: Total number of images in full batch (for chunking context) """ # Prepare image file paths image_paths = self._prepare_images(images) try: + # Augment prompt with metadata context (GPS, datetime, camera, chunk info) + augmented_prompt = self._augment_prompt_with_metadata( + prompt, images, image_id_map, total_images + ) + # Apply chat template (required for vision models) num_images = len(image_paths) if image_paths else 0 formatted_prompt = self._apply_chat_template( - self.processor, self.config, prompt, num_images=num_images + self.processor, self.config, augmented_prompt, num_images=num_images ) # Build generation kwargs @@ -196,7 +213,7 @@ class VisionRunner: # Add filename mapping (even for single images - enables cross-model workflows) if images: - normalized = self._add_filename_mapping(normalized, images, image_id_map) + normalized = self._add_filename_mapping(normalized, images, image_id_map, total_images) return normalized except Exception as e: @@ -205,6 +222,104 @@ class VisionRunner: # Clean up temp files after generation (success or error) self._cleanup_temp_files() + @staticmethod + def _augment_prompt_with_metadata( + prompt: str, + images: Sequence[Tuple[str, bytes]], + image_id_map: Optional[Dict[str, int]], + total_images: Optional[int], + ) -> str: + """Augment user prompt with image metadata context for better responses. + + Prepends metadata (GPS coordinates, datetime, camera) to the user prompt + so the model can use this information in its response. + + Feature flag: MLXK2_VISION_METADATA_CONTEXT=0 to disable (default: enabled) + + Args: + prompt: User prompt + images: List of (filename, bytes) tuples + image_id_map: Mapping of content_hash -> image_id for stable numbering + total_images: Total images in full batch (for chunking context) + + Returns: + Augmented prompt with metadata context prepended + """ + # Feature flag: Can be disabled + if os.environ.get("MLXK2_VISION_METADATA_CONTEXT") == "0": + return prompt + + if not images: + return prompt + + # Extract EXIF for all images + metadata_lines = [] + + # Per-image metadata + for idx, (filename, img_bytes) in enumerate(images, 1): + # Determine image ID + if image_id_map: + content_hash = hashlib.sha256(img_bytes).hexdigest()[:8] + img_id = image_id_map.get(content_hash, idx) + else: + img_id = idx + + # Add chunk context line before first image (if chunking active) + if idx == 1 and total_images and total_images > len(images): + # Calculate chunk info + chunk_size = len(images) + if image_id_map: + # Find all IDs in current chunk to determine range + chunk_ids = [] + for fn, ib in images: + ch = hashlib.sha256(ib).hexdigest()[:8] + if ch in image_id_map: + chunk_ids.append(image_id_map[ch]) + + if chunk_ids: + start_id = min(chunk_ids) + end_id = max(chunk_ids) + batch_num = (start_id - 1) // chunk_size + 1 + total_batches = (total_images + chunk_size - 1) // chunk_size + metadata_lines.append( + f"[Processing batch {batch_num}/{total_batches}: " + f"Images {start_id}-{end_id} (chunk_size={chunk_size}, {total_images} total)]" + ) + + # Extract EXIF + exif = VisionRunner._extract_exif(img_bytes) + + # Build metadata string for this image + meta_parts = [f"Image {img_id}"] + + if exif: + if exif.gps_lat is not None and exif.gps_lon is not None: + # Format GPS coordinates + lat_dir = "N" if exif.gps_lat >= 0 else "S" + lon_dir = "E" if exif.gps_lon >= 0 else "W" + meta_parts.append( + f"GPS: {abs(exif.gps_lat):.2f}°{lat_dir}, {abs(exif.gps_lon):.2f}°{lon_dir}" + ) + + if exif.datetime: + # Format datetime (just date for brevity) + meta_parts.append(f"Date: {exif.datetime[:10]}") + + if exif.camera: + # Shorten camera name for token efficiency + camera = exif.camera.replace("(", "").replace(")", "").replace("generation", "gen") + meta_parts.append(f"Camera: {camera}") + + if len(meta_parts) > 1: # Only add if we have metadata beyond image ID + metadata_lines.append("[" + " | ".join(meta_parts) + "]") + + # Prepend metadata to prompt + if metadata_lines: + metadata_context = "\n".join(metadata_lines) + return f"{metadata_context}\n\n{prompt}" + else: + return prompt + @staticmethod def _extract_exif(image_bytes: bytes) -> Optional[ExifData]: """ @@ -315,11 +430,12 @@ class VisionRunner: result: str, images: Sequence[Tuple[str, bytes]], image_id_map: Optional[Dict[str, int]] = None, + total_images: Optional[int] = None, ) -> str: - """Add filename mapping footer for multiple images (deterministic). + """Add filename mapping header for multiple images (deterministic). Vision models reference images by position (Image 1, Image 2, etc.). - This footer helps users map positions back to original filenames. + This header helps users map positions back to original filenames. The mapping is formatted as a Markdown table with an HTML comment marker ''. This makes it: @@ -340,7 +456,7 @@ class VisionRunner: numbering. If None, uses request-scoped sequential IDs. Returns: - Result with appended filename mapping + Result with prepended filename mapping (metadata before model output) """ # Extract EXIF data (optional, controlled by feature flag) exif_enabled = os.environ.get("MLXK2_EXIF_METADATA") != "0" @@ -414,12 +530,35 @@ class VisionRunner: # Build collapsible HTML details (collapsed by default) # The marker comment is preserved for backwards compatibility count = len(images) - mapping = "\n\n
\n" - mapping += f"📸 Image Metadata ({count} image{'s' if count != 1 else ''})\n\n" + mapping = "
\n" # No leading newlines - this goes at beginning of output + + # Determine summary text (with chunk info if chunking is active) + if total_images and total_images > count and image_id_map: + # Chunking is active - show batch info + chunk_ids = [] + for _, raw_bytes in images: + content_hash = hashlib.sha256(raw_bytes).hexdigest()[:8] + if content_hash in image_id_map: + chunk_ids.append(image_id_map[content_hash]) + + if chunk_ids: + start_id = min(chunk_ids) + end_id = max(chunk_ids) + chunk_size = len(images) + batch_num = (start_id - 1) // chunk_size + 1 + total_batches = (total_images + chunk_size - 1) // chunk_size + mapping += f"📸 Batch {batch_num}/{total_batches}: Images {start_id}-{end_id}\n\n" + else: + # Fallback if chunk_ids calculation fails + mapping += f"📸 Image Metadata ({count} image{'s' if count != 1 else ''})\n\n" + else: + # No chunking - standard summary + mapping += f"📸 Image Metadata ({count} image{'s' if count != 1 else ''})\n\n" mapping += "\n" mapping += f"{header}\n" mapping += f"{separator}\n" mapping += "\n".join(rows) + "\n" - mapping += "\n
\n" + mapping += "\n
\n\n" # Spacing after metadata table - return result + mapping + # Metadata goes BEFORE model output (matches input order, clearer UX in chunking) + return mapping + result diff --git a/mlxk2/operations/clone.py b/mlxk2/operations/clone.py index d1b2d98..3ef6e5b 100644 --- a/mlxk2/operations/clone.py +++ b/mlxk2/operations/clone.py @@ -12,30 +12,31 @@ This implementation: User cache is NEVER touched - only temp cache is used and cleaned up. """ +import hashlib import logging -import os -import random import re import shutil import subprocess import time from pathlib import Path -from typing import Optional, Dict, Any +from typing import Optional, Dict, Any, Tuple from .pull import pull_to_cache from .workspace import write_workspace_sentinel from ..core.cache import hf_to_cache_dir, get_current_cache_root +from mlxk2 import __version__ logger = logging.getLogger(__name__) -def clone_operation(model_spec: str, target_dir: str, health_check: bool = True) -> Dict[str, Any]: +def clone_operation(model_spec: str, target_dir: str, health_check: bool = True, force_resume: bool = False) -> Dict[str, Any]: """Clone operation following ADR-007 Phase 1: Same-Volume APFS strategy. Args: model_spec: Model specification (org/repo[@revision]) target_dir: Target directory for workspace health_check: Whether to run health check before copy (default: True) + force_resume: If True, skip unhealthy check and resume partial download (default: False) Returns: JSON response following API 0.1.4 schema @@ -54,6 +55,7 @@ def clone_operation(model_spec: str, target_dir: str, health_check: bool = True) } temp_cache = None # Initialize for cleanup in finally block + target_created_by_us = False # Track if we created target dir (for cleanup on failure) try: # Validate target directory @@ -105,14 +107,38 @@ def clone_operation(model_spec: str, target_dir: str, health_check: bool = True) result["data"]["clone_status"] = "filesystem_error" return result - # Phase 2: Create temp cache on same volume as workspace + # Phase 2: Create or resume temp cache on same volume as workspace (ADR-018 Phase 0b) result["data"]["clone_status"] = "preparing" - temp_cache = _create_temp_cache_same_volume(target_path) + temp_cache, should_download = _create_temp_cache_same_volume( + target_path, model_spec, force_resume + ) + + # Extract resolved model name early for health check + resolved_model = model_spec # Will be updated from pull_result if download happens try: - # Phase 3: Pull to isolated temp cache (no HF_HOME patching needed) - result["data"]["clone_status"] = "pulling" - pull_result = pull_to_cache(model_spec, temp_cache) + # Phase 3: Pull to isolated temp cache (conditional, ADR-018 Phase 0b) + try: + if should_download: + result["data"]["clone_status"] = "pulling" + pull_result = pull_to_cache(model_spec, temp_cache) + else: + # Resuming healthy existing download - skip pull + result["data"]["clone_status"] = "resuming" + logger.info("Skipping download - resuming healthy temp cache") + # Create minimal pull_result for continuation + pull_result = { + "status": "success", + "data": { + "model": model_spec, + "commit_hash": None # Unknown for resumed cache + } + } + + except KeyboardInterrupt: + # User cancelled - set status BEFORE finally block runs + result["data"]["clone_status"] = "cancelled" + raise # Re-raise to outer handler if pull_result["status"] != "success": result["status"] = "error" @@ -159,6 +185,7 @@ def clone_operation(model_spec: str, target_dir: str, health_check: bool = True) # Phase 6: APFS clone temp cache → workspace (instant, CoW) result["data"]["clone_status"] = "cloning" target_path.mkdir(parents=True, exist_ok=True) + target_created_by_us = True # Track for cleanup on failure clone_success = _apfs_clone_directory(temp_snapshot, target_path) if not clone_success: @@ -178,7 +205,7 @@ def clone_operation(model_spec: str, target_dir: str, health_check: bool = True) commit_hash = pull_result["data"].get("commit_hash") metadata = { - "mlxk_version": "2.0.4", # TODO: Read from __version__ + "mlxk_version": __version__, "created_at": datetime.utcnow().isoformat() + "Z", "source_repo": resolved_model, "source_revision": commit_hash, @@ -200,13 +227,80 @@ def clone_operation(model_spec: str, target_dir: str, health_check: bool = True) result["data"]["message"] = f"Cloned to {target_dir}" finally: - # Phase 7: Cleanup temp cache (always) - with safety check - # TODO (ADR-018 Phase 0b): Make cleanup conditional based on: - # - .mlxk2_download_complete marker exists AND - # - health status (healthy → cleanup, unhealthy → keep for debugging/repair) - # - user choice (prompt if unhealthy: "Keep temp cache for inspection?") + # Phase 7: Conditional cleanup (ADR-018 Phase 0b) + # Cleanup strategy: + # 1. Success (clone complete) → always cleanup (workspace created, temp no longer needed) + # 2. User cancelled (Ctrl-C) → keep for resume (partial download, resumable) + # 3. Failure with complete download → keep for debugging/repair (user can retry) + # 4. Failure with incomplete download → cleanup (partial download, unusable - non-resumable error) if temp_cache and temp_cache.exists(): - _cleanup_temp_cache_safe(temp_cache) + should_cleanup = False + + # Check clone_status FIRST (set by inner except before finally runs) + if result["data"]["clone_status"] == "cancelled": + # User cancelled - preserve for resume + should_cleanup = False + logger.info(f"Preserving partial download for resume: {temp_cache}") + logger.info("Retry with same model+target to resume download") + elif result["status"] == "success": + # Clone succeeded - always cleanup + should_cleanup = True + cleanup_reason = "clone succeeded" + else: + # Clone failed - check if download was complete + download_marker = temp_cache / ".mlxk2_download_complete" + if download_marker.exists(): + # Complete download failed - keep for debugging + should_cleanup = False + logger.info(f"Keeping temp cache for inspection: {temp_cache}") + logger.info("Retry with same target to resume, or use --force-resume") + else: + # Incomplete download - cleanup + should_cleanup = True + cleanup_reason = "incomplete download" + + if should_cleanup: + logger.debug(f"Cleaning up temp cache ({cleanup_reason}): {temp_cache}") + _cleanup_temp_cache_safe(temp_cache) + + # Phase 8: Cleanup partial target directory on failure (defensive) + # If clone failed after we created target_path but before success, + # remove it so retries can proceed cleanly + if target_created_by_us and result["status"] != "success": + if target_path.exists() and not any(target_path.iterdir()): + # Only remove if empty (safety check - don't delete user data) + try: + target_path.rmdir() + logger.debug(f"Cleaned up empty target directory: {target_path}") + except OSError as e: + logger.warning(f"Failed to cleanup target directory: {e}") + + except KeyboardInterrupt: + # User cancelled - set status (clone_status may already be set by inner except) + result["status"] = "error" + if result["data"]["clone_status"] != "cancelled": + # KeyboardInterrupt before inner try/except - set clone_status now + result["data"]["clone_status"] = "cancelled" + + # Prepare error message (check temp_cache state) + if temp_cache and temp_cache.exists(): + result["error"] = { + "type": "UserCancelledError", + "message": ( + "Operation cancelled by user.\n" + f"Partial download preserved at: {temp_cache}\n" + f"To resume: Run the same command again (same model + target).\n" + f"To delete: rm -rf {temp_cache}" + ) + } + logger.info("Operation cancelled - temp cache preserved for resume") + else: + result["error"] = { + "type": "UserCancelledError", + "message": "Operation cancelled by user." + } + logger.info("Operation cancelled - no temp cache to preserve") + # Don't re-raise - return error result for clean CLI output except Exception as e: result["status"] = "error" @@ -277,29 +371,109 @@ def _is_apfs_filesystem(path: Path) -> bool: return False # Safe fallback -def _create_temp_cache_same_volume(target_workspace: Path) -> Path: - """Create temp cache on same APFS volume as target for CoW optimization. +def _get_deterministic_temp_cache_name(model_spec: str, target_workspace: Path) -> str: + """Generate deterministic temp cache name for resumable clone (ADR-018 Phase 0b). - TODO (ADR-018 Phase 0b - Resumable Clone): - Change naming to deterministic for resume support: - OLD: f".mlxk2_temp_{os.getpid()}_{random.randint(...)}" # ephemeral - NEW: f".mlxk2_temp_{hash(model_spec + target_workspace)}" # deterministic + Args: + model_spec: Model specification (org/repo[@revision]) + target_workspace: Target workspace path - This allows detection of existing partial downloads for resume prompts. + Returns: + Deterministic temp cache directory name (e.g., ".mlxk2_temp_a1b2c3d4e5f6g7h8") + """ + # Deterministic hash: model_spec + target_workspace absolute path + # Use first 16 hex chars for readability (64-bit hash, collision unlikely) + hash_input = f"{model_spec}:{target_workspace.resolve()}" + hash_hex = hashlib.sha256(hash_input.encode()).hexdigest()[:16] + return f".mlxk2_temp_{hash_hex}" + + +def _check_temp_cache_resume(temp_cache: Path, model_spec: str) -> Tuple[bool, str, bool]: + """Check if temp cache can be resumed (ADR-018 Phase 0b). + + Args: + temp_cache: Temp cache directory path + model_spec: Model specification for health check + + Returns: + Tuple of (can_resume, reason, is_healthy) + - can_resume: True if temp cache exists (partial or complete) + - reason: Human-readable explanation + - is_healthy: True if download is complete AND passed health check + """ + # Check 1: Does temp cache exist? + if not temp_cache.exists(): + return False, "No existing download", False + + # Check 2: Is download complete? + download_marker = temp_cache / ".mlxk2_download_complete" + if not download_marker.exists(): + # Partial download (e.g., Ctrl-C) - resumable via HuggingFace + # No health check needed for partial downloads + return True, "Partial download (resumable via HuggingFace)", False + + # Check 3: Health check on completed download + from .health import health_from_cache + healthy, health_message = health_from_cache(model_spec, temp_cache) + + if healthy: + return True, "Complete and healthy", True + else: + return True, f"Complete but unhealthy: {health_message}", False + + +def _create_temp_cache_same_volume(target_workspace: Path, model_spec: str, force_resume: bool = False) -> Tuple[Path, bool]: + """Create or resume temp cache on same APFS volume (ADR-018 Phase 0b). + + Args: + target_workspace: Target workspace path + model_spec: Model specification (for deterministic naming) + force_resume: If True, skip unhealthy check and resume + + Returns: + Tuple of (temp_cache_path, should_download) + - temp_cache_path: Path to temp cache directory + - should_download: True if download needed, False if resuming healthy cache """ # Get target volume mount point via st_dev target_volume = _get_volume_mount_point(target_workspace) - # Create temp cache on same volume - # NOTE: PID-based (ephemeral) - will become deterministic in Phase 0b - temp_cache = target_volume / f".mlxk2_temp_{os.getpid()}_{random.randint(1000,9999)}" - temp_cache.mkdir(parents=True) + # Deterministic naming (ADR-018 Phase 0b) + temp_cache_name = _get_deterministic_temp_cache_name(model_spec, target_workspace) + temp_cache = target_volume / temp_cache_name + + # Check if we can resume existing temp cache + can_resume, resume_reason, is_healthy = _check_temp_cache_resume(temp_cache, model_spec) + + if can_resume: + download_marker = temp_cache / ".mlxk2_download_complete" + + if not download_marker.exists(): + # Partial download (Ctrl-C) - resume via HuggingFace snapshot_download + logger.info(f"Resuming partial download: {resume_reason}") + return temp_cache, True # should_download=True to call pull_to_cache (resumes automatically) + elif is_healthy: + # Complete + Healthy - skip download entirely, use existing + logger.info(f"Using existing healthy download: {resume_reason}") + return temp_cache, False # should_download=False + elif force_resume: + # Complete + Unhealthy + force - use broken model without re-downloading + logger.warning(f"Force resuming unhealthy download: {resume_reason}") + return temp_cache, False # should_download=False + else: + # Complete + Unhealthy + no force - delete and restart fresh + logger.warning(f"Deleting unhealthy temp cache: {resume_reason}") + shutil.rmtree(temp_cache) + # Fall through to create new + + # Create new temp cache (either no existing or deleted unhealthy) + temp_cache.mkdir(parents=True, exist_ok=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())}") + sentinel.write_text(f"mlxk2_temp_cache_created_{int(time.time())}\n") - return temp_cache + return temp_cache, True def _get_volume_mount_point(path: Path) -> Path: diff --git a/mlxk2/operations/common.py b/mlxk2/operations/common.py index 1b16d5a..3812132 100644 --- a/mlxk2/operations/common.py +++ b/mlxk2/operations/common.py @@ -213,9 +213,18 @@ def detect_vision_capability(probe: Path, config: Optional[Dict[str, Any]]) -> b Video models (AutoVideoProcessor) are excluded as they require PyTorch/Torchvision. mlx-vlm only supports image vision models (AutoImageProcessor). + + Note: skip_vision flag indicates vision components can be skipped for text-only + inference, but does NOT mean the model lacks vision capabilities. """ try: if isinstance(config, dict): + # Check for vision_config presence (Mistral-Small 3.1 has vision_config with skip_vision) + vision_config = config.get("vision_config") + if isinstance(vision_config, dict): + # Vision config present = vision model (even if skip_vision=true) + return True + mt = config.get("model_type") if isinstance(mt, str) and mt.lower() in VISION_MODEL_TYPES: return True @@ -330,8 +339,11 @@ def build_model_object(hf_name: str, model_root: Path, selected_path: Optional[P selected_path: points at the chosen snapshot directory when available; otherwise may be the model_root. Commit hash is taken from selected_path.name if it looks like a 40-char hex string, else None. + + ADR-018 Phase 0c: Supports workspace paths (hf_name can be absolute path). """ - from ..operations.health import is_model_healthy, check_runtime_compatibility # local import to avoid cycle + from ..operations.health import is_model_healthy, check_runtime_compatibility, health_check_workspace + from ..operations.workspace import is_workspace_path # Compute commit hash if selected path is a snapshot dir commit_hash: Optional[str] = None @@ -351,8 +363,13 @@ def build_model_object(hf_name: str, model_root: Path, selected_path: Optional[P capabilities = detect_capabilities(model_type, hf_name, tok, config, probe) has_vision = "vision" in capabilities - # Health: rely on existing operation (name-based) - healthy, health_reason = is_model_healthy(hf_name) + # Health: workspace-aware (ADR-018 Phase 0c) + if is_workspace_path(hf_name): + # Workspace path - use workspace health check directly + healthy, health_reason, _ = health_check_workspace(Path(hf_name)) + else: + # Cache model - use name-based health check + healthy, health_reason = is_model_healthy(hf_name) # Runtime compatibility: ALWAYS computed (gate logic applies) # Gate 1: File integrity must be healthy @@ -376,6 +393,10 @@ def build_model_object(hf_name: str, model_root: Path, selected_path: Optional[P # Size/Modified computed from selected path (snapshot preferred) base = selected_path if selected_path is not None else model_root + + # Cached flag: True for cache models, False for workspace paths (ADR-018 Phase 0c) + cached = not is_workspace_path(hf_name) + model_obj = { "name": hf_name, "hash": commit_hash, @@ -387,6 +408,6 @@ def build_model_object(hf_name: str, model_root: Path, selected_path: Optional[P "health": "healthy" if healthy else "unhealthy", "runtime_compatible": runtime_compatible, "reason": reason, - "cached": True, + "cached": cached, } return model_obj diff --git a/mlxk2/operations/convert.py b/mlxk2/operations/convert.py index fba516e..31e6240 100644 --- a/mlxk2/operations/convert.py +++ b/mlxk2/operations/convert.py @@ -28,6 +28,7 @@ from typing import Dict, Any from .workspace import write_workspace_sentinel, is_managed_workspace, read_workspace_metadata from .health import health_check_workspace from ..core.cache import get_current_cache_root +from mlxk2 import __version__ logger = logging.getLogger(__name__) @@ -251,7 +252,7 @@ def convert_operation( src_metadata = read_workspace_metadata(src) target_metadata = { - "mlxk_version": "2.0.4", # TODO: Read from __version__ + "mlxk_version": __version__, "created_at": datetime.utcnow().isoformat() + "Z", "source_repo": src_metadata.get("source_repo", str(src)), "source_revision": src_metadata.get("source_revision"), diff --git a/mlxk2/operations/health.py b/mlxk2/operations/health.py index 37f189a..f21ff13 100644 --- a/mlxk2/operations/health.py +++ b/mlxk2/operations/health.py @@ -4,6 +4,7 @@ from pathlib import Path from typing import Tuple, Optional from ..core.cache import get_current_model_cache, hf_to_cache_dir, cache_dir_to_hf from ..core.model_resolution import resolve_model_for_operation +from .workspace import is_workspace_path def is_model_healthy(model_spec): @@ -57,18 +58,25 @@ def _check_auxiliary_assets(model_path, config_data): is_vision = detect_vision_capability(model_path, config_data) if is_vision: - # Vision models require preprocessor_config.json for mlx-vlm + # Vision models require preprocessor_config.json OR processor_config.json for mlx-vlm + # Different models use different naming conventions (DeepSeek-OCR uses processor_config.json) preprocessor_path = model_path / "preprocessor_config.json" - if not preprocessor_path.exists(): - return False, "Vision model missing preprocessor_config.json" + processor_path = model_path / "processor_config.json" + + if not preprocessor_path.exists() and not processor_path.exists(): + return False, "Vision model missing preprocessor_config.json or processor_config.json" + + # Use whichever file exists + config_path = preprocessor_path if preprocessor_path.exists() else processor_path + config_name = config_path.name try: - with open(preprocessor_path) as f: + with open(config_path) as f: preprocessor_data = json.load(f) if not isinstance(preprocessor_data, dict): - return False, "preprocessor_config.json invalid" + return False, f"{config_name} invalid" except (OSError, json.JSONDecodeError): - return False, "preprocessor_config.json invalid JSON" + return False, f"{config_name} invalid JSON" # Chat models benefit from tokenizer assets (not strict requirement for base models) # Check tokenizer_config.json for chat template support @@ -93,12 +101,41 @@ def _check_auxiliary_assets(model_path, config_data): def _check_snapshot_health(model_path): """Check health of a specific snapshot directory. + FILE INTEGRITY DEFINITION (used consistently across list/show/health commands): + + A model is HEALTHY (integrity OK) if ALL of the following are true: + + 1. REQUIRED FILES PRESENT: + - config.json exists, is valid JSON, non-empty dict + - At least one weight file (.safetensors/.bin/.gguf) present + + 2. WEIGHT FILES COMPLETE (Index-based models): + - If model.safetensors.index.json exists: + * ALL referenced shards exist (no missing files) + * ALL shards are non-empty (size > 0) + * NO shards are LFS pointers (git-lfs markers) + * Index/shard mismatch (mlx-vlm #624) → unhealthy + + 3. WEIGHT FILES COMPLETE (Pattern-based, no index): + - If model-XXXXX-of-YYYYY.safetensors pattern detected: + * ALL shards 1 to YYYYY must exist (no gaps) + * ALL shards are non-empty + * Sharded models WITHOUT index → unhealthy + + 4. NO CORRUPTION MARKERS: + - No LFS pointers (git-lfs placeholders) + - No partial download markers (.partial, .tmp) + + 5. AUXILIARY ASSETS (model-type specific): + - Vision models: preprocessor_config.json OR processor_config.json required + - Chat models: if tokenizer_config.json exists, tokenizer.json required + + IMPORTANT: This is INTEGRITY only - does NOT check runtime compatibility. + A PyTorch model can be "healthy" (files intact) but not runtime_compatible (mlx-lm can't run it). + Rules (Issue #27 parity): - - If a multi-file safetensors index exists (model.safetensors.index.json), - ALL referenced shard files must exist and be non-empty, and none may be LFS pointers. - A subset must NOT be marked healthy. - - Without an index, require at least one weight file present and non-empty, - and ensure none are LFS pointers. + - Multi-file models: ALL shards required (no partial sets) + - mlx-vlm #624: Index/shard mismatch detected as integrity issue ADR-012 Phase 2: Auxiliary asset validation - Vision models require preprocessor_config.json (for image processing) @@ -143,7 +180,15 @@ def _check_snapshot_health(model_path): referenced_files = sorted(set(weight_map.values())) missing = [rf for rf in referenced_files if not (model_path / rf).exists()] if missing: - return False, f"Missing weight shards: {', '.join(missing)}" + # Check if this is mlx-vlm #624 (index/shard mismatch) vs incomplete download + # mlx-vlm #624: Index references wrong files, but other shards exist + actual_shards = list(model_path.glob("*.safetensors")) + if actual_shards: + # Shards exist but don't match index - mlx-vlm #624 (INTEGRITY ISSUE) + return False, f"Index/shard mismatch (mlx-vlm #624). Index references {len(referenced_files)} shards but found {len(actual_shards)} different files. Fix: mlxk convert --repair-index" + else: + # No shards at all - incomplete download + return False, f"Missing weight shards: {', '.join(missing)}" empty = [rf for rf in referenced_files if (model_path / rf).stat().st_size == 0] if empty: return False, f"Empty weight shards: {', '.join(empty)}" @@ -481,6 +526,10 @@ def health_check_operation(model_pattern=None): - Cache model names: "microsoft/phi-2", "phi-2", "1.3b" - Workspace paths: "./workspace", "/path/to/model" - No pattern: Check all cached models + + Returns minimal format per JSON API Schema 0.1.5: + - healthy/unhealthy arrays contain: {name, status, reason} + - Uses same integrity checks as list/show (_check_snapshot_health) """ result = { "status": "success", @@ -499,28 +548,27 @@ def health_check_operation(model_pattern=None): try: # Check if model_pattern is a workspace path - if model_pattern: - workspace_path = Path(model_pattern) - if workspace_path.exists() and (workspace_path / "config.json").exists(): - # This is a workspace directory - use workspace health check - healthy, reason, managed = health_check_workspace(workspace_path) + if model_pattern and is_workspace_path(model_pattern): + # This is a workspace directory - use workspace health check + workspace_path = Path(model_pattern).resolve() + healthy, reason, managed = health_check_workspace(workspace_path) - model_info = { - "name": str(workspace_path), - "status": "healthy" if healthy else "unhealthy", - "reason": reason, - "managed": managed - } + model_info = { + "name": str(workspace_path), + "status": "healthy" if healthy else "unhealthy", + "reason": reason, + "managed": managed + } - result["data"]["summary"]["total"] = 1 - if healthy: - result["data"]["healthy"].append(model_info) - result["data"]["summary"]["healthy_count"] = 1 - else: - result["data"]["unhealthy"].append(model_info) - result["data"]["summary"]["unhealthy_count"] = 1 + result["data"]["summary"]["total"] = 1 + if healthy: + result["data"]["healthy"].append(model_info) + result["data"]["summary"]["healthy_count"] = 1 + else: + result["data"]["unhealthy"].append(model_info) + result["data"]["summary"]["unhealthy_count"] = 1 - return result + return result # Not a workspace - proceed with cache-based health check model_cache = get_current_model_cache() @@ -531,7 +579,7 @@ def health_check_operation(model_pattern=None): # Use model resolution if specific pattern provided if model_pattern: resolved_name, commit_hash, ambiguous_matches = resolve_model_for_operation(model_pattern) - + if ambiguous_matches: # Multiple matches - let user choose result["status"] = "error" @@ -555,33 +603,37 @@ def health_check_operation(model_pattern=None): else: # No pattern - check all models models_to_check = [d for d in model_cache.iterdir() if d.name.startswith("models--")] - + result["data"]["summary"]["total"] = len(models_to_check) - + for model_dir in sorted(models_to_check, key=lambda x: x.name): hf_name = cache_dir_to_hf(model_dir.name) - - # Use the new flexible health check + + # Hide test sentinel directories from listings + if "TEST-CACHE-SENTINEL" in hf_name: + continue + + # Use same integrity check as list/show (CONSISTENCY) healthy, reason = is_model_healthy(hf_name) - + model_info = { "name": hf_name, - "status": "healthy" if healthy else "unhealthy", + "status": "healthy" if healthy else "unhealthy", "reason": reason } - + if healthy: result["data"]["healthy"].append(model_info) result["data"]["summary"]["healthy_count"] += 1 else: result["data"]["unhealthy"].append(model_info) result["data"]["summary"]["unhealthy_count"] += 1 - + except Exception as e: result["status"] = "error" result["error"] = { "type": "health_check_failed", "message": str(e) } - + return result diff --git a/mlxk2/operations/run.py b/mlxk2/operations/run.py index 0b8a49d..92169f1 100644 --- a/mlxk2/operations/run.py +++ b/mlxk2/operations/run.py @@ -3,10 +3,13 @@ Run operation for 2.0 implementation. Ported from 1.x with 2.0 architecture integration. """ +import hashlib import json +import os import subprocess import sys -from typing import Optional, Sequence, Tuple +from pathlib import Path +from typing import List, Optional, Sequence, Tuple from ..core.runner import MLXRunner from ..core.cache import get_current_model_cache, hf_to_cache_dir @@ -147,10 +150,98 @@ def check_memory_for_server( return None +def _process_images_in_chunks( + model_path: str, + model_name: str, + prompt: str, + images: List[Tuple[str, bytes]], + chunk_size: int, + max_tokens: Optional[int], + temperature: float, + top_p: float, + repetition_penalty: float, + verbose: bool, + json_output: bool = False, +) -> str: + """Process images in batches with isolated model instances per chunk. + + Each chunk creates a fresh VisionRunner to prevent state leakage between batches. + This ensures each image is processed independently without context from previous images. + + Args: + model_path: Path to model snapshot directory + model_name: Model name for VisionRunner + prompt: User prompt + images: Full list of (filename, bytes) tuples + chunk_size: Images per batch + max_tokens, temperature, top_p, repetition_penalty: Generation params + verbose: Show chunk progress + json_output: If True, suppress incremental output and return full result + + Returns: + Combined text with merged filename mappings (or empty if printed incrementally) + """ + # Lazy import: only load VisionRunner when needed (Python 3.10+ required) + from ..core.vision_runner import VisionRunner + + # Pre-assign global image IDs (1..N) before chunking + # IMPORTANT: Use [:8] to match vision_runner.py hash length (line 367, 373) + image_id_map = {} + for idx, (filename, image_bytes) in enumerate(images, start=1): + content_hash = hashlib.sha256(image_bytes).hexdigest()[:8] + image_id_map[content_hash] = idx + + # Split into chunks + chunks = [images[i:i+chunk_size] for i in range(0, len(images), chunk_size)] + + # Process each chunk with fresh runner (prevents state leakage) + all_results = [] + for chunk_idx, chunk in enumerate(chunks, start=1): + if verbose: + start_img = (chunk_idx - 1) * chunk_size + 1 + end_img = min(chunk_idx * chunk_size, len(images)) + print( + f"Processing images {start_img}-{end_img} (chunk {chunk_idx}/{len(chunks)})...", + file=sys.stderr + ) + + # Fresh runner per chunk to prevent KV-cache/state accumulation + with VisionRunner(model_path, model_name, verbose=verbose) as runner: + chunk_result = runner.generate( + prompt=prompt, + images=chunk, + max_tokens=max_tokens, + temperature=temperature, + top_p=top_p, + repetition_penalty=repetition_penalty, + image_id_map=image_id_map, # Global numbering preserved + total_images=len(images), # Enable chunk context line + ) + + # Incremental output for better UX (show results as they come) + if not json_output: + try: + print(chunk_result) + print() # Blank line between chunks + sys.stdout.flush() # Ensure immediate output + except BrokenPipeError: + sys.stderr.close() + + all_results.append(chunk_result) + + # Return combined results for json_output mode + # For non-json mode, return empty since we already printed incrementally + if json_output: + return "\n\n".join(all_results) + else: + return "" # Already printed incrementally, avoid duplicate output + + def run_model( model_spec: str, prompt: Optional[str] = None, images: Optional[Sequence[Tuple[str, bytes]]] = None, + chunk: int = 1, stream: bool = True, max_tokens: Optional[int] = None, temperature: float = 0.7, @@ -196,32 +287,52 @@ def run_model( # Only perform compatibility check if model is actually in cache is_vision_model = False model_path = None + model_cache_dir = None cfg = None if resolved_name: - model_cache = get_current_model_cache() - model_cache_dir = model_cache / hf_to_cache_dir(resolved_name) + from .workspace import is_workspace_path - if model_cache_dir.exists(): - snapshots_dir = model_cache_dir / "snapshots" - if snapshots_dir.exists(): - # Resolve snapshot path (commit-pinned or latest) - if commit_hash: - model_path = snapshots_dir / commit_hash - else: - snapshots = [d for d in snapshots_dir.iterdir() if d.is_dir()] - if snapshots: - model_path = max(snapshots, key=lambda x: x.stat().st_mtime) + # Check if resolved_name is a workspace path (ADR-018 Phase 0c) + if is_workspace_path(resolved_name): + # Workspace path - use directly + model_path = Path(resolved_name) + model_cache_dir = model_path.parent # For detect_framework compatibility - # Detect vision capability to select backend - cfg_path = model_path / "config.json" if model_path else None - if cfg_path and cfg_path.exists(): - try: - cfg = json.loads(cfg_path.read_text(encoding="utf-8", errors="ignore")) - except Exception: - cfg = None + # Detect vision capability from workspace + cfg_path = model_path / "config.json" + if cfg_path.exists(): + try: + cfg = json.loads(cfg_path.read_text(encoding="utf-8", errors="ignore")) + except Exception: + cfg = None - if model_path is not None: - is_vision_model = detect_vision_capability(model_path, cfg) + is_vision_model = detect_vision_capability(model_path, cfg) + else: + # Cache model - existing logic + model_cache = get_current_model_cache() + model_cache_dir = model_cache / hf_to_cache_dir(resolved_name) + + if model_cache_dir.exists(): + snapshots_dir = model_cache_dir / "snapshots" + if snapshots_dir.exists(): + # Resolve snapshot path (commit-pinned or latest) + if commit_hash: + model_path = snapshots_dir / commit_hash + else: + snapshots = [d for d in snapshots_dir.iterdir() if d.is_dir()] + if snapshots: + model_path = max(snapshots, key=lambda x: x.stat().st_mtime) + + # Detect vision capability to select backend + cfg_path = model_path / "config.json" if model_path else None + if cfg_path and cfg_path.exists(): + try: + cfg = json.loads(cfg_path.read_text(encoding="utf-8", errors="ignore")) + except Exception: + cfg = None + + if model_path is not None: + is_vision_model = detect_vision_capability(model_path, cfg) # If images are provided but model is not vision-capable, fail fast if images and not is_vision_model: @@ -302,17 +413,56 @@ def run_model( return error_result try: - # Lazy import: only load VisionRunner when needed (Python 3.10+ required) - from ..core.vision_runner import VisionRunner + # Get chunk size (with env var override) + chunk_size = chunk if chunk != 1 else int(os.environ.get("MLXK2_VISION_BATCH_SIZE", "1")) - with VisionRunner(model_path, resolved_name or model_spec, verbose=verbose) as runner: - result = runner.generate( + # Validate chunk size for Metal API stability + from ..tools.vision_adapter import MAX_SAFE_CHUNK_SIZE + if chunk_size < 1: + error_result = ( + f"Error: chunk size must be at least 1 (got: {chunk_size})." + ) + if not json_output: + print(error_result, file=sys.stderr) + return error_result + if chunk_size > MAX_SAFE_CHUNK_SIZE: + error_result = ( + f"Error: chunk size too large (max: {MAX_SAFE_CHUNK_SIZE} for Metal API stability). " + f"This limit is based on empirically tested performance." + ) + if not json_output: + print(error_result, file=sys.stderr) + return error_result + + images_list = list(images or []) + + if len(images_list) <= chunk_size: + # Single batch (no chunking needed) - use single runner instance + from ..core.vision_runner import VisionRunner + + with VisionRunner(model_path, resolved_name or model_spec, verbose=verbose) as runner: + result = runner.generate( + prompt=prompt, + images=images_list, + max_tokens=max_tokens, + temperature=temperature, + top_p=top_p, + repetition_penalty=repetition_penalty, + ) + else: + # Multi-batch chunking - creates fresh runner per chunk + result = _process_images_in_chunks( + model_path=model_path, + model_name=resolved_name or model_spec, prompt=prompt, - images=list(images or []), + images=images_list, + chunk_size=chunk_size, max_tokens=max_tokens, temperature=temperature, top_p=top_p, repetition_penalty=repetition_penalty, + verbose=verbose, + json_output=json_output, ) except Exception as e: error_result = f"Error: {e}" @@ -532,6 +682,7 @@ def run_model_enhanced( model_spec: str, prompt: Optional[str], images: Optional[Sequence[Tuple[str, bytes]]] = None, + chunk: int = 1, stream: bool = True, max_tokens: Optional[int] = None, temperature: float = 0.7, @@ -576,6 +727,7 @@ def run_model_enhanced( model_spec=model_spec, prompt=prompt, images=images, + chunk=chunk, stream=stream, max_tokens=max_tokens, temperature=temperature, diff --git a/mlxk2/operations/serve.py b/mlxk2/operations/serve.py index 8790a58..0e4e5d6 100644 --- a/mlxk2/operations/serve.py +++ b/mlxk2/operations/serve.py @@ -91,6 +91,7 @@ def start_server( max_tokens: Optional[int] = None, reload: bool = False, log_level: str = "info", + chunk: int = 1, verbose: bool = False, supervise: bool = True, ) -> None: @@ -105,24 +106,43 @@ def start_server( max_tokens: Default maximum tokens for generation reload: Enable auto-reload for development log_level: Logging level + chunk: Default batch size for vision requests (default: 1, max: 5) verbose: Show detailed output supervise: Run uvicorn in a supervised subprocess for instant Ctrl-C """ + # Validate chunk size + from ..tools.vision_adapter import MAX_SAFE_CHUNK_SIZE + if chunk < 1: + raise ValueError( + f"chunk size must be at least 1 (got: {chunk})." + ) + if chunk > MAX_SAFE_CHUNK_SIZE: + raise ValueError( + f"chunk size too large (max: {MAX_SAFE_CHUNK_SIZE} for Metal API stability). " + f"This limit is based on empirically tested performance." + ) + + # Set environment variables for server configuration + # These apply to both supervised and non-supervised modes + os.environ["MLXK2_LOG_LEVEL"] = log_level + if model: + os.environ["MLXK2_PRELOAD_MODEL"] = model + if chunk != 1: + os.environ["MLXK2_VISION_BATCH_SIZE"] = str(chunk) + if verbose: print("Starting MLX Knife Server 2.0...") if model: print(f"Pre-loading model: {model}") print(f"Server will bind to: http://{host}:{port}") + if chunk != 1: + print(f"Vision batch size: {chunk}") # Pre-load validation happens in server_base.py lifespan hook # via environment variable MLXK2_PRELOAD_MODEL if supervise: - # Pass log_level and model via environment to subprocess (ADR-004) - os.environ["MLXK2_LOG_LEVEL"] = log_level - if model: - os.environ["MLXK2_PRELOAD_MODEL"] = model - # Delegate to subprocess-managed uvicorn + # Delegate to subprocess-managed uvicorn (env vars already set above) _ = _run_supervised_uvicorn(host=host, port=port, log_level=log_level, reload=reload) return diff --git a/mlxk2/operations/show.py b/mlxk2/operations/show.py index 66a91e2..555b432 100644 --- a/mlxk2/operations/show.py +++ b/mlxk2/operations/show.py @@ -1,11 +1,13 @@ """Show model operation for MLX-Knife 2.0.""" import json +from pathlib import Path from typing import Dict, Any from ..core.cache import get_current_model_cache, hf_to_cache_dir from ..core.model_resolution import resolve_model_for_operation from .common import build_model_object +from .workspace import is_workspace_path def get_file_type(file_name): @@ -141,42 +143,48 @@ def show_model_operation(model_pattern: str, include_files: bool = False, includ "message": f"No model found matching '{model_pattern}'" } return result - - # Get model directory - model_cache_dir = get_current_model_cache() / hf_to_cache_dir(resolved_name) - if not model_cache_dir.exists(): - result["status"] = "error" - result["error"] = { - "type": "model_not_cached", - "message": f"Model '{resolved_name}' not found in cache" - } - return result - - # Find the correct snapshot - snapshots_dir = model_cache_dir / "snapshots" - model_path = None - - if commit_hash and snapshots_dir.exists(): - # Specific hash requested - hash_path = snapshots_dir / commit_hash - if hash_path.exists(): - model_path = hash_path - else: + + # Check if resolved_name is a workspace path + if is_workspace_path(resolved_name): + # Workspace path - use directly + model_path = Path(resolved_name) + model_cache_dir = model_path.parent + else: + # Cache model - existing logic + model_cache_dir = get_current_model_cache() / hf_to_cache_dir(resolved_name) + if not model_cache_dir.exists(): result["status"] = "error" result["error"] = { - "type": "hash_not_found", - "message": f"Hash '{commit_hash}' not found for model '{resolved_name}'" + "type": "model_not_cached", + "message": f"Model '{resolved_name}' not found in cache" } return result - elif snapshots_dir.exists(): - # Use latest snapshot - snapshots = [d for d in snapshots_dir.iterdir() if d.is_dir()] - if snapshots: - model_path = max(snapshots, key=lambda x: x.stat().st_mtime) - commit_hash = model_path.name - - if not model_path: - model_path = model_cache_dir + + # Find the correct snapshot + snapshots_dir = model_cache_dir / "snapshots" + model_path = None + + if commit_hash and snapshots_dir.exists(): + # Specific hash requested + hash_path = snapshots_dir / commit_hash + if hash_path.exists(): + model_path = hash_path + else: + result["status"] = "error" + result["error"] = { + "type": "hash_not_found", + "message": f"Hash '{commit_hash}' not found for model '{resolved_name}'" + } + return result + elif snapshots_dir.exists(): + # Use latest snapshot + snapshots = [d for d in snapshots_dir.iterdir() if d.is_dir()] + if snapshots: + model_path = max(snapshots, key=lambda x: x.stat().st_mtime) + commit_hash = model_path.name + + if not model_path: + model_path = model_cache_dir # Build unified model object model_obj = build_model_object(resolved_name, model_cache_dir, model_path) diff --git a/mlxk2/operations/workspace.py b/mlxk2/operations/workspace.py index 3f1a3cf..364a7b7 100644 --- a/mlxk2/operations/workspace.py +++ b/mlxk2/operations/workspace.py @@ -10,7 +10,7 @@ Managed workspaces contain a `.mlxk_workspace.json` sentinel file that enables: Sentinel format: { - "mlxk_version": "2.0.4", + "mlxk_version": "", // e.g., "2.0.4b6" "created_at": "2025-12-29T10:30:00Z", "source_repo": "mlx-community/Llama-3.2-3B", "source_revision": "abc123def456", @@ -140,3 +140,31 @@ def read_workspace_metadata(workspace_path: Path) -> Dict[str, Any]: except (json.JSONDecodeError, OSError) as e: logger.debug(f"Failed to read sentinel in {workspace_path}: {e}") return {} + + +def is_workspace_path(path) -> bool: + """Check if path points to a workspace directory (managed or unmanaged). + + A workspace is any directory containing a config.json file (MLX model structure). + This includes both managed workspaces (with .mlxk_workspace.json) and + unmanaged workspaces (3rd-party model directories). + + Args: + path: Path-like object (str, Path) to check + + Returns: + True if path exists and contains config.json, False otherwise + + Examples: + >>> is_workspace_path("./my-workspace") + True + >>> is_workspace_path("/path/to/model") + True + >>> is_workspace_path("mlx-community/Phi-3-mini") + False # HF model ID, not a path + """ + try: + p = Path(path) + return p.exists() and (p / "config.json").exists() + except (TypeError, OSError): + return False diff --git a/mlxk2/tools/vision_adapter.py b/mlxk2/tools/vision_adapter.py index f6c16c1..999dbbe 100644 --- a/mlxk2/tools/vision_adapter.py +++ b/mlxk2/tools/vision_adapter.py @@ -15,11 +15,11 @@ from typing import Any, Dict, List, Optional, Tuple # No imports needed - use standard Python exceptions # Limits for vision requests (safety and resource management) -# Conservative limits to prevent Metal OOM crashes (ADR-012 Phase 3) -# Phase 1c (future) will use batching for large collections -MAX_IMAGES_PER_REQUEST = 5 # Tested stable with 5 images -MAX_IMAGE_SIZE_BYTES = 20 * 1024 * 1024 # 20 MB per image -MAX_TOTAL_IMAGE_BYTES = 50 * 1024 * 1024 # 50 MB total (critical for Metal API) +# Per-image size limit prevents Metal OOM crashes (ADR-012 Phase 3) +# Total image count is unlimited - chunking (ADR-012 Phase 1c) handles batch safety +# chunk_size controls memory usage: default=1 (safest), can be increased via request param +MAX_IMAGE_SIZE_BYTES = 20 * 1024 * 1024 # 20 MB per image (Metal API limit) +MAX_SAFE_CHUNK_SIZE = 5 # Empirically tested stable (5 images @ ~50MB total) SUPPORTED_MIME_TYPES = frozenset({"jpeg", "jpg", "png", "gif", "webp"}) @@ -138,25 +138,12 @@ class VisionHTTPAdapter: "image_url.url cannot be empty" ) - # Decode base64 image + # Decode base64 image (validates size per image) filename, raw_bytes = VisionHTTPAdapter.decode_base64_image(url) images.append((filename, raw_bytes)) - # Enforce image count limit - if len(images) > MAX_IMAGES_PER_REQUEST: - raise ValueError( - f"Too many images in request (max: {MAX_IMAGES_PER_REQUEST})" - ) - - # Enforce total size limit (critical for Metal API) - total_size = sum(len(img[1]) for img in images) - if total_size > MAX_TOTAL_IMAGE_BYTES: - size_mb = total_size / (1024 * 1024) - limit_mb = MAX_TOTAL_IMAGE_BYTES / (1024 * 1024) - raise ValueError( - f"Total image size ({size_mb:.1f} MB) exceeds limit ({limit_mb:.0f} MB). " - f"Try fewer or smaller images." - ) + # Note: Total image count/size is unlimited - chunking handles batch safety + # chunk_size (default=1) controls memory usage per batch # Stop after processing first (most recent) user message break diff --git a/pytest.ini b/pytest.ini index 48b8d8b..7174926 100644 --- a/pytest.ini +++ b/pytest.ini @@ -5,14 +5,16 @@ python_classes = Test* python_functions = test_* markers = spec: JSON API contract tests (current spec only) + live: Umbrella marker for ALL tests requiring real models/network (excluded from default run) wet: Umbrella for Portfolio Discovery compatible tests (see TESTING-DETAILS.md) live_push: Alias for wet; push live tests (require env) live_list: Alias for wet; list human live tests (require env) - live_clone: Alias for wet; clone live tests (require env, ADR-007 Phase 1) + live_pull: Isolated Cache WRITE - pull tests (separate run required) + live_clone: Isolated Cache WRITE - clone tests (separate run required, ADR-007 Phase 1 + ADR-018 Phase 0b) live_run: Opt-in run command tests with real models (require user cache model) live_stop_tokens: Opt-in stop token tests with real models (Issue #32, ADR-009) live_e2e: E2E tests with real models and server (require HF_HOME, ADR-011) - live_resumable: Isolated Cache WRITE tests (separate run required) + live_vision_pipe: Vision→Geo pipe integration smoke tests (Sessions 72-75, require vision+text models) show_model_portfolio: Display E2E test portfolio (convenience, no testing) issue27: Real-model health policy tests (opt-in; read-only user cache) slow: Tests that take >1 minute to run diff --git a/scripts/test-wet-umbrella.sh b/scripts/test-wet-umbrella.sh index 1c5ddb1..1d017be 100755 --- a/scripts/test-wet-umbrella.sh +++ b/scripts/test-wet-umbrella.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Run all "real tests" (wet umbrella + resumable) +# Run all "real tests" (wet umbrella + isolated cache tests) # Memory-optimized for large test suites (154+ tests) set -e @@ -8,17 +8,31 @@ echo "🌂 Wet Umbrella: Running all real tests..." # Memory-saving pytest options: # --tb=no: No tracebacks (routine runs expect all PASSED, debug failures individually) # --capture=sys: System-level capture only (less buffering than --capture=fd) +# For verbose output with portfolio info, run with: pytest -s ... PYTEST_OPTS="--tb=no --capture=sys" -# Run 1: Compatible live tests (User Cache READ) +# Run 1: Compatible live tests (User Cache READ + Workspace) echo "" echo "📦 Phase 1: User Cache READ tests (wet umbrella)..." MLXK2_ENABLE_ALPHA_FEATURES=1 pytest -m wet -v $PYTEST_OPTS -# Run 2: Isolated Cache WRITE tests (incompatible with Portfolio) +# Run 2: Isolated Cache WRITE - Pull (incompatible with Portfolio) echo "" -echo "📥 Phase 2: Isolated Cache WRITE tests (resumable)..." -MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_resumable -v $PYTEST_OPTS +echo "📥 Phase 2: Isolated Cache WRITE - Pull tests..." +MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_pull -v $PYTEST_OPTS + +# Run 3: Isolated Cache WRITE - Clone (incompatible with Portfolio) +echo "" +echo "🔄 Phase 3: Isolated Cache WRITE - Clone tests..." +# Note: live_clone tests are opt-in (require env vars), will skip if not configured +MLXK2_ENABLE_ALPHA_FEATURES=1 pytest -m live_clone -v $PYTEST_OPTS + +# Run 4: Vision→Geo Pipe Integration +echo "" +echo "🖼️ Phase 4: Vision→Geo Pipe tests..." +# Note: Requires vision model (e.g., pixtral) + text model (e.g., Qwen3-Next) +# Will skip if models not found in cache (graceful degradation) +MLXK2_ENABLE_PIPES=1 pytest -m live_vision_pipe -v $PYTEST_OPTS echo "" echo "✅ All real tests completed!" diff --git a/test-multi-python.sh b/test-multi-python.sh index 863b614..8953665 100755 --- a/test-multi-python.sh +++ b/test-multi-python.sh @@ -80,10 +80,10 @@ test_python_version() { if python -m mlxk2.cli --version --json > /dev/null 2>&1; then echo -e "${GREEN}✅ CLI test (version) passed${NC}" - # Run complete test suite + # Run complete test suite (exclude ALL live tests requiring models/network) echo "🧪 Running 2.0 test suite..." local test_log="test_results_${version_name//./_}.log" - if python -m pytest tests_2.0/ -v --tb=short > "$test_log" 2>&1; then + if python -m pytest tests_2.0/ -m "not live" -v --tb=short > "$test_log" 2>&1; then local passed_count=$(grep -c "PASSED" "$test_log" 2>/dev/null) local failed_count=$(grep -c "FAILED" "$test_log" 2>/dev/null) passed_count=${passed_count:-0} diff --git a/tests_2.0/assets/geo-test/coll2_1.jpeg b/tests_2.0/assets/geo-test/coll2_1.jpeg new file mode 100644 index 0000000..6651ef6 Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_1.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_2.jpeg b/tests_2.0/assets/geo-test/coll2_2.jpeg new file mode 100644 index 0000000..d8a7fab Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_2.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_3.jpeg b/tests_2.0/assets/geo-test/coll2_3.jpeg new file mode 100644 index 0000000..674b4ae Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_3.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_4.jpeg b/tests_2.0/assets/geo-test/coll2_4.jpeg new file mode 100644 index 0000000..b2100d2 Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_4.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_5.jpeg b/tests_2.0/assets/geo-test/coll2_5.jpeg new file mode 100644 index 0000000..33e13bc Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_5.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_6.jpeg b/tests_2.0/assets/geo-test/coll2_6.jpeg new file mode 100644 index 0000000..c8c6fdd Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_6.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_7.jpeg b/tests_2.0/assets/geo-test/coll2_7.jpeg new file mode 100644 index 0000000..1739ab1 Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_7.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_8.jpeg b/tests_2.0/assets/geo-test/coll2_8.jpeg new file mode 100644 index 0000000..ef42806 Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_8.jpeg differ diff --git a/tests_2.0/assets/geo-test/coll2_9.jpeg b/tests_2.0/assets/geo-test/coll2_9.jpeg new file mode 100644 index 0000000..dabfa73 Binary files /dev/null and b/tests_2.0/assets/geo-test/coll2_9.jpeg differ diff --git a/tests_2.0/conftest.py b/tests_2.0/conftest.py index 1fd451f..0ef3014 100644 --- a/tests_2.0/conftest.py +++ b/tests_2.0/conftest.py @@ -1171,11 +1171,12 @@ def pytest_collection_modifyitems(config, items): Wet Umbrella groups tests that can run together in one pytest invocation: - User Cache READ tests (live_e2e, live_stop_tokens, live_run, live_list) - - Workspace operations (live_push, live_clone) + - Workspace operations (live_push) - Issue reproduction (issue27) - Excluded from wet: - - live_resumable (Isolated Cache WRITE - requires clean import state) + Excluded from wet (Isolated Cache WRITE - requires clean import state): + - live_pull (resumable pull tests) + - live_clone (clone tests with internal pull) - Model validation tests (memory-intensive, belongs in separate benchmark suite) See: TESTING-DETAILS.md → Extended Truth Table @@ -1187,7 +1188,6 @@ def pytest_collection_modifyitems(config, items): "live_run", # User Cache READ "live_list", # User Cache READ "live_push", # Workspace (not Cache) - "live_clone", # Workspace (not Cache) "issue27", # User Cache READ } @@ -1209,8 +1209,8 @@ def pytest_collection_modifyitems(config, items): # Wet marker for compatible tests if (test_markers & LIVE_MARKERS_FOR_WET) or is_in_live_dir: - # EXCLUDE live_resumable (Isolated Cache WRITE - incompatible!) - if "live_resumable" not in test_markers: + # EXCLUDE Isolated Cache WRITE tests (incompatible with Portfolio Discovery!) + if "live_pull" not in test_markers and "live_clone" not in test_markers: item.add_marker(pytest.mark.wet) diff --git a/tests_2.0/live/test_cli_e2e.py b/tests_2.0/live/test_cli_e2e.py index b844b35..c1bdb41 100644 --- a/tests_2.0/live/test_cli_e2e.py +++ b/tests_2.0/live/test_cli_e2e.py @@ -40,7 +40,7 @@ from .test_utils import ( # portfolio_models fixture is provided by conftest.py # Opt-in markers -pytestmark = [pytest.mark.live_e2e, pytest.mark.slow] +pytestmark = [pytest.mark.live, pytest.mark.live_e2e, pytest.mark.slow] def _run_mlxk_subprocess(args: list[str], timeout: int = 60) -> tuple[str, str, int]: diff --git a/tests_2.0/live/test_cli_pipe_live.py b/tests_2.0/live/test_cli_pipe_live.py index 478f085..d34e8a2 100644 --- a/tests_2.0/live/test_cli_pipe_live.py +++ b/tests_2.0/live/test_cli_pipe_live.py @@ -16,7 +16,7 @@ import pytest from .test_utils import should_skip_model, MAX_TOKENS, TEST_TEMPERATURE -pytestmark = [pytest.mark.live_e2e, pytest.mark.slow] +pytestmark = [pytest.mark.live, pytest.mark.live_e2e, pytest.mark.slow] def _pick_first_eligible_model(portfolio_models: Dict[str, Dict[str, Any]]) -> Dict[str, Any]: diff --git a/tests_2.0/live/test_clone_live.py b/tests_2.0/live/test_clone_live.py index a01ae56..2946f48 100644 --- a/tests_2.0/live/test_clone_live.py +++ b/tests_2.0/live/test_clone_live.py @@ -12,7 +12,9 @@ Enable with ALL required env vars: Run: - pytest -m live_clone -v -- or umbrella: pytest -m wet -v +- or umbrella Phase 3: scripts/test-wet-umbrella.sh (isolated run) + +NOT part of wet marker (incompatible with Portfolio Discovery - does fresh HF download). ADR-007 Phase 1 Requirements: - Same volume: workspace and HF_HOME cache must be on same volume @@ -39,7 +41,7 @@ model = os.environ.get("MLXK2_LIVE_CLONE_MODEL") workspace = os.environ.get("MLXK2_LIVE_CLONE_WORKSPACE") pytestmark = [ - pytest.mark.wet, + pytest.mark.live, pytest.mark.live_clone, pytest.mark.skipif( not (alpha_enabled and live_enabled and hf_token_present and model and workspace), diff --git a/tests_2.0/live/test_list_human_live.py b/tests_2.0/live/test_list_human_live.py index ad99193..424d750 100644 --- a/tests_2.0/live/test_list_human_live.py +++ b/tests_2.0/live/test_list_human_live.py @@ -16,7 +16,7 @@ from typing import List, Dict import pytest -pytestmark = [pytest.mark.wet, pytest.mark.live_list] +pytestmark = [pytest.mark.live, pytest.mark.wet, pytest.mark.live_list] def _run_cli(argv: List[str], capsys) -> str: diff --git a/tests_2.0/live/test_pipe_vision_geo.py b/tests_2.0/live/test_pipe_vision_geo.py new file mode 100644 index 0000000..ad69477 --- /dev/null +++ b/tests_2.0/live/test_pipe_vision_geo.py @@ -0,0 +1,241 @@ +"""Vision→Geo pipe integration test (Session 72-75 validation). + +Simple smoke test for the complete pipeline: +- Vision model with chunking (--chunk 1) for geo-test images +- Pipe to text model for geo-location inference + +PASSED criteria (minimal): +- Both phases exit 0 (no crash) +- Output not empty +- Output contains geo-related terms (heuristic) + +FAILED criteria: +- Process crash (non-zero exit) +- Empty output +- Import/model errors + +Opt-in: pytest -m live_vision_pipe -v +Requires: HF_HOME with vision+text models, MLXK2_ENABLE_PIPES=1 + +See: TESTING-DETAILS.md for test strategy +""" + +from __future__ import annotations + +import subprocess +import sys +import os +from pathlib import Path +from typing import Dict, Any + +import pytest + +from .test_utils import should_skip_model + +pytestmark = [pytest.mark.live, pytest.mark.live_vision_pipe, pytest.mark.slow] + +# Test images (9 JPEGs in geo-test collection) +GEO_TEST_DIR = Path(__file__).parent.parent / "assets" / "geo-test" +GEO_IMAGES = sorted(GEO_TEST_DIR.glob("coll2_*.jpeg")) + + +def _pick_best_eligible_text_model(text_portfolio: Dict[str, Dict[str, Any]]) -> Dict[str, Any]: + """Select the best text model for geo-inference (RAM-aware, task-appropriate). + + Rationale: After vision model unloads (~12GB), we want the best available + text model for geo-inference. Prefer general-purpose chat models over + specialized models (coder, math) which lack geographic knowledge. + + Portfolio structure: dict of dicts with keys like 'text_00', 'text_01', etc. + Each value has keys: id, ram_needed_gb, description, expected_issue + """ + # Blacklist patterns for specialized models (not good for geo-inference) + SPECIALIZED_PATTERNS = ["coder", "code", "math", "medical", "legal"] + + eligible = [] + # Portfolio is a dict - iterate over items + for key, info in text_portfolio.items(): + should_skip, _ = should_skip_model(key, text_portfolio) + if should_skip: + continue + + model_id = info.get("id", "").lower() + # Skip specialized models (coder/math/etc - poor geographic knowledge) + if any(pattern in model_id for pattern in SPECIALIZED_PATTERNS): + continue + + eligible.append((key, info)) + + if not eligible: + pytest.skip("No suitable general-purpose text models found in portfolio (RAM gating)") + + # Sort by RAM requirement (DESC) - larger general-purpose models = better geo knowledge + # Use ram_needed_gb (from portfolio, not ram_mb!) + eligible.sort(key=lambda x: x[1].get("ram_needed_gb", 0), reverse=True) + + return eligible[0][1] # Return largest general-purpose model info dict + + +def _run_cli(args: list[str], stdin: str | None = None, timeout: int = 600) -> tuple[str, str, int]: + """Run mlxk CLI as subprocess.""" + result = subprocess.run( + [sys.executable, "-m", "mlxk2.cli"] + args, + input=stdin, + text=True, + capture_output=True, + timeout=timeout, + env={**os.environ, "MLXK2_ENABLE_PIPES": "1"}, + ) + return result.stdout, result.stderr, result.returncode + + +class TestVisionGeoPipeline: + """Integration test for Vision→Geo pipeline (Sessions 72-75).""" + + @pytest.fixture(scope="class") + def vision_model_id(self): + """Get vision model (hardcoded for now - pixtral only viable model).""" + # TODO: Use vision_portfolio when more vision models are viable + # Currently only pixtral works reliably (blacklist filters others) + return "pixtral" + + @pytest.fixture(scope="class") + def text_model_id(self, text_portfolio): + """Get best (largest) eligible text model from portfolio (RAM-aware). + + Sequential loading strategy (Session 73): Vision model unloads first + (~12GB freed), then text model loads. Pick largest available for quality. + """ + model = _pick_best_eligible_text_model(text_portfolio) + model_id = model.get("id") # Portfolio uses 'id', not 'model_id' + ram_gb = model.get("ram_needed_gb", "unknown") + + # Standard print (works with -s flag like all other tests) + print(f"\n🌍 Vision→Geo Pipe: Selected text model: {model_id} (~{ram_gb:.1f}GB)") + + return model_id + + @pytest.fixture(scope="class") + def check_prerequisites(self): + """Check if pipe mode is enabled and images exist.""" + if not os.getenv("MLXK2_ENABLE_PIPES"): + pytest.skip("Pipe mode gated by MLXK2_ENABLE_PIPES=1") + + if not GEO_IMAGES: + pytest.skip(f"No geo-test images found in {GEO_TEST_DIR}") + + assert len(GEO_IMAGES) == 9, f"Expected 9 images, found {len(GEO_IMAGES)}" + + def test_vision_batch_processing_chunk_1(self, check_prerequisites, vision_model_id): + """Test vision batch processing with chunk=1 (incremental output). + + Validates: ADR-012 Phase 1c, Sessions 73-75 fixes + PASSED: Process succeeds, output not empty, multiple images mentioned + """ + image_paths = [str(p) for p in GEO_IMAGES] + + args = [ + "run", + vision_model_id, + "--image", *image_paths, + "--chunk", "1", + "--max-tokens", "12000", + "--prompt", ( + "Describe each image in best possible detail. " + "Don't repeat unimportant camera information. " + "Number images according to metadata image number." + ), + ] + + stdout, stderr, code = _run_cli(args, timeout=600) + + # Minimal criteria: Process succeeds and produces output + assert code == 0, f"Vision phase failed: exit={code}\nstderr={stderr}" + assert stdout.strip(), "Vision output is empty" + + # Heuristic: Output should mention multiple images (smoke test) + image_mentions = sum(1 for i in range(1, 10) if f"Image {i}" in stdout or f"image {i}" in stdout.lower()) + assert image_mentions >= 5, f"Only {image_mentions}/9 images mentioned (expected most/all)" + + def test_vision_to_geo_pipe(self, check_prerequisites, vision_model_id, text_model_id): + """Test complete Vision→Geo pipeline. + + Validates: Session 73 pipe stdin + --prompt, complete integration + PASSED: Both phases succeed, geo output mentions location concepts + """ + image_paths = [str(p) for p in GEO_IMAGES] + + # Phase 1: Vision descriptions + vision_args = [ + "run", + vision_model_id, + "--image", *image_paths, + "--chunk", "1", + "--max-tokens", "12000", + "--prompt", ( + "Describe each image in best possible detail. " + "Don't repeat unimportant camera information. " + "Number images according to metadata image number." + ), + ] + + vision_stdout, vision_stderr, vision_code = _run_cli(vision_args, timeout=600) + + assert vision_code == 0, f"Vision phase failed: {vision_stderr}" + assert vision_stdout.strip(), "Vision output is empty" + + # Phase 2: Geo inference via pipe + geo_args = [ + "run", + text_model_id, + "-", + "--prompt", ( + "According to the location information - " + "tell me the area where all the images have been made." + ), + "--max-tokens", "500", + ] + + geo_stdout, geo_stderr, geo_code = _run_cli(geo_args, stdin=vision_stdout, timeout=300) + + assert geo_code == 0, f"Geo phase failed: exit={geo_code}\nstderr={geo_stderr}" + assert geo_stdout.strip(), "Geo output is empty" + + # Heuristic: Output should mention location-related concepts (smoke test) + # NOTE: We don't verify accuracy (no GOLD), just that pipe workflow functions + geo_lower = geo_stdout.lower() + has_location_terms = any(term in geo_lower for term in [ + "location", "area", "region", "place", "city", "country", + "latitude", "longitude", "coordinates", "gps" + ]) + + assert has_location_terms, f"Geo output lacks location terms (pipe may have failed):\n{geo_stdout[:300]}" + + def test_vision_chunk_isolation_no_hallucination(self, check_prerequisites, vision_model_id): + """Test chunk isolation with chunk=1 (Session 73 regression test). + + Validates: Fresh VisionRunner per chunk, no state leakage + PASSED: Process succeeds, both images mentioned separately + """ + # Test with only 2 images, chunk=1 (minimal isolation test) + image_paths = [str(p) for p in GEO_IMAGES[:2]] + + args = [ + "run", + vision_model_id, + "--image", *image_paths, + "--chunk", "1", + "--max-tokens", "800", + "--prompt", "Describe this image briefly.", + ] + + stdout, stderr, code = _run_cli(args, timeout=240) + + # Minimal criteria: Process succeeds, output not empty, both batches present + assert code == 0, f"exit={code}\nstderr={stderr}" + assert stdout.strip(), "Output is empty" + + # Smoke test: Both batches should be visible (chunk workflow functioning) + # NOTE: We don't verify isolation quality - just that 2 batches were processed + assert "batch 1/2" in stdout.lower(), "Batch 1/2 not found (chunking failed?)" + assert "batch 2/2" in stdout.lower(), "Batch 2/2 not found (chunking failed?)" diff --git a/tests_2.0/live/test_portfolio_fixtures.py b/tests_2.0/live/test_portfolio_fixtures.py index 746a0a8..4aae028 100644 --- a/tests_2.0/live/test_portfolio_fixtures.py +++ b/tests_2.0/live/test_portfolio_fixtures.py @@ -5,8 +5,9 @@ These tests verify that text_portfolio and vision_portfolio fixtures work correc import pytest +pytestmark = [pytest.mark.live, pytest.mark.live_e2e] + -@pytest.mark.live_e2e def test_text_portfolio_contains_only_text_models(text_portfolio): """Verify that text_portfolio contains no vision models.""" # Should have at least one text model (or be empty if no HF_HOME) diff --git a/tests_2.0/live/test_push_live.py b/tests_2.0/live/test_push_live.py index 80648da..e0b8eda 100644 --- a/tests_2.0/live/test_push_live.py +++ b/tests_2.0/live/test_push_live.py @@ -28,6 +28,7 @@ repo = os.environ.get("MLXK2_LIVE_REPO") workspace = os.environ.get("MLXK2_LIVE_WORKSPACE") pytestmark = [ + pytest.mark.live, pytest.mark.wet, pytest.mark.live_push, pytest.mark.skipif( diff --git a/tests_2.0/live/test_server_e2e.py b/tests_2.0/live/test_server_e2e.py index 5803ee6..61354f7 100644 --- a/tests_2.0/live/test_server_e2e.py +++ b/tests_2.0/live/test_server_e2e.py @@ -47,6 +47,7 @@ MODEL_LIST_TIMEOUT = 20.0 # Opt-in markers pytestmark = [ + pytest.mark.live, pytest.mark.live_e2e, pytest.mark.slow, pytest.mark.skipif( diff --git a/tests_2.0/live/test_show_portfolio.py b/tests_2.0/live/test_show_portfolio.py index afcbcee..8e6e149 100644 --- a/tests_2.0/live/test_show_portfolio.py +++ b/tests_2.0/live/test_show_portfolio.py @@ -6,9 +6,9 @@ Usage: pytest -m show_model_portfolio -s from __future__ import annotations import pytest +pytestmark = [pytest.mark.live, pytest.mark.live_e2e, pytest.mark.show_model_portfolio] + -@pytest.mark.show_model_portfolio -@pytest.mark.live_e2e def test_show_portfolio(portfolio_models): """Display E2E test portfolio models (no actual testing). diff --git a/tests_2.0/live/test_streaming_parity.py b/tests_2.0/live/test_streaming_parity.py index 7aa5dac..4500e42 100644 --- a/tests_2.0/live/test_streaming_parity.py +++ b/tests_2.0/live/test_streaming_parity.py @@ -43,6 +43,7 @@ from .test_utils import ( # Opt-in markers pytestmark = [ + pytest.mark.live, pytest.mark.live_e2e, pytest.mark.slow, pytest.mark.skipif( diff --git a/tests_2.0/live/test_utils.py b/tests_2.0/live/test_utils.py index c6f2141..0ed5bc9 100644 --- a/tests_2.0/live/test_utils.py +++ b/tests_2.0/live/test_utils.py @@ -29,6 +29,46 @@ finally: sys.path.remove(str(_parent_dir)) +# ============================================================================= +# KNOWN BROKEN MODELS - Upstream Runtime Bugs +# ============================================================================= +# These models pass static health checks (files present, config valid) but fail +# at runtime initialization due to upstream mlx-lm/mlx-vlm bugs. They are +# excluded from portfolio discovery to prevent spurious test failures. +# +# Policy: Add models here ONLY when: +# 1. Static health check passes (healthy files) +# 2. Runtime initialization fails consistently +# 3. Root cause is verified upstream bug (not mlx-knife bug) +# 4. Issue is documented (session notes, upstream issue tracker) +# +# Format: Full HuggingFace model ID (org/name) +# ============================================================================= + +KNOWN_BROKEN_MODELS = { + # transformers 5.0 video processor bug: "argument of type 'NoneType' is not iterable" + # Root Cause: transformers sets video_processor=None when torchvision unavailable + # Upstream: https://github.com/Blaizzy/mlx-vlm/issues/640 + # Details: docs/ISSUES/transformers-5.0-video-processor-bug.md + # Test: `mlxk run mlx-community/MiMo-VL-7B-RL-bf16 "test" --image foo.jpg` → Error + # Strategy: Waiting for mlx-vlm Issue #640 resolution + "mlx-community/MiMo-VL-7B-RL-bf16", + + # transformers 5.0 video processor bug (same as MiMo-VL above) + # Upstream: https://github.com/Blaizzy/mlx-vlm/issues/640 + # Details: docs/ISSUES/transformers-5.0-video-processor-bug.md + # Test: `mlxk run mlx-community/Qwen2-VL-7B-Instruct-4bit "test" --image foo.jpg` → Error + # Note: Image-only processing works, video processing broken + "mlx-community/Qwen2-VL-7B-Instruct-4bit", + + # mlx-vlm vision feature mismatch: Image token positions (5476) ≠ features (1369) + # Status: Upstream mlx-vlm vision encoder/model compatibility bug (separate from #624) + # Test: `mlxk run ./Mistral-Small-3.1-24B-Instruct-2503-FIXED-4bit "test" --image foo.jpg` → Error + # Note: --repair-index fixes #624 (index mismatch) but NOT this vision feature bug + "mlx-community/Mistral-Small-3.1-24B-Instruct-2503-4bit", +} + + # RAM calculation utilities (modularized for different model types) def calculate_text_model_ram_gb(size_bytes: int) -> float: diff --git a/tests_2.0/live/test_vision_e2e_live.py b/tests_2.0/live/test_vision_e2e_live.py index 2cb08e0..1f88e4b 100644 --- a/tests_2.0/live/test_vision_e2e_live.py +++ b/tests_2.0/live/test_vision_e2e_live.py @@ -21,6 +21,7 @@ from pathlib import Path # Vision support requires Python 3.10+ (mlx-vlm requirement) pytestmark = [ + pytest.mark.live, pytest.mark.live_e2e, pytest.mark.skipif( sys.version_info < (3, 10), diff --git a/tests_2.0/live/test_vision_server_e2e.py b/tests_2.0/live/test_vision_server_e2e.py index 0d93c93..bbf2340 100644 --- a/tests_2.0/live/test_vision_server_e2e.py +++ b/tests_2.0/live/test_vision_server_e2e.py @@ -30,6 +30,7 @@ from .test_utils import should_skip_model # Skip entire module if httpx not installed pytestmark = [ + pytest.mark.live, pytest.mark.skipif(httpx is None, reason="httpx required for E2E tests"), pytest.mark.live_e2e, ] diff --git a/tests_2.0/test_clone_operation.py b/tests_2.0/test_clone_operation.py index 2c5d22a..cee483c 100644 --- a/tests_2.0/test_clone_operation.py +++ b/tests_2.0/test_clone_operation.py @@ -27,6 +27,8 @@ from mlxk2.operations.clone import ( _validate_apfs_filesystem, _is_apfs_filesystem, _create_temp_cache_same_volume, + _get_deterministic_temp_cache_name, + _check_temp_cache_resume, _get_volume_mount_point, _resolve_latest_snapshot, _apfs_clone_directory, @@ -244,16 +246,18 @@ class TestTempCacheCreation: def test_create_temp_cache_same_volume(self, tmp_path): """Test temp cache creation on same volume.""" target_workspace = tmp_path / "workspace" + model_spec = "test/model" with patch('mlxk2.operations.clone._get_volume_mount_point') as mock_volume: mock_volume.return_value = tmp_path - temp_cache = _create_temp_cache_same_volume(target_workspace) + temp_cache, should_download = _create_temp_cache_same_volume(target_workspace, model_spec, False) # Verify temp cache is on same volume assert temp_cache.parent == tmp_path assert temp_cache.exists() assert temp_cache.is_dir() + assert should_download is True # New temp cache, should download # Verify sentinel file exists sentinel = temp_cache / ".mlxk2_temp_cache_sentinel" @@ -263,37 +267,8 @@ class TestTempCacheCreation: # Cleanup shutil.rmtree(temp_cache) - def test_create_temp_cache_unique_names(self, tmp_path): - """Test temp cache gets unique names.""" - target_workspace = tmp_path / "workspace" - - with patch('mlxk2.operations.clone._get_volume_mount_point') as mock_volume: - mock_volume.return_value = tmp_path - - cache1 = _create_temp_cache_same_volume(target_workspace) - cache2 = _create_temp_cache_same_volume(target_workspace) - - assert cache1 != cache2 - assert cache1.exists() - assert cache2.exists() - - # Cleanup - shutil.rmtree(cache1) - shutil.rmtree(cache2) - - def test_create_temp_cache_includes_pid(self, tmp_path): - """Test temp cache name includes process ID.""" - target_workspace = tmp_path / "workspace" - - with patch('mlxk2.operations.clone._get_volume_mount_point') as mock_volume: - mock_volume.return_value = tmp_path - - temp_cache = _create_temp_cache_same_volume(target_workspace) - - assert str(os.getpid()) in temp_cache.name - - # Cleanup - shutil.rmtree(temp_cache) + # Note: test_create_temp_cache_unique_names and test_create_temp_cache_includes_pid + # removed in ADR-018 Phase 0b - replaced by deterministic naming (tested in TestResumableClone) class TestSentinelSafetyMechanism: @@ -504,7 +479,7 @@ class TestCloneOperationIntegration: patch('mlxk2.operations.clone._apfs_clone_directory') as mock_clone: # Use real temp cache - mock_create_cache.return_value = real_temp_cache + mock_create_cache.return_value = (real_temp_cache, True) # (temp_cache, should_download) mock_health.return_value = (True, "Model is healthy") mock_pull.return_value = { @@ -573,7 +548,7 @@ class TestCloneOperationIntegration: patch('mlxk2.operations.clone._create_temp_cache_same_volume') as mock_create_cache, \ patch('mlxk2.operations.clone.pull_to_cache') as mock_pull: - mock_create_cache.return_value = real_temp_cache + mock_create_cache.return_value = (real_temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = { "status": "error", @@ -610,7 +585,7 @@ class TestCloneOperationIntegration: patch('mlxk2.operations.clone._resolve_latest_snapshot') as mock_resolve, \ patch('mlxk2.operations.health.health_from_cache') as mock_health: - mock_create_cache.return_value = real_temp_cache + mock_create_cache.return_value = (real_temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = {"status": "success", "data": {"model": model_spec}} @@ -681,7 +656,7 @@ class TestCloneOperationIntegration: patch('mlxk2.operations.health.health_from_cache') as mock_health, \ patch('mlxk2.operations.clone._apfs_clone_directory') as mock_clone: - mock_create_cache.return_value = real_temp_cache + mock_create_cache.return_value = (real_temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = {"status": "success", "data": {"model": model_spec}} @@ -698,8 +673,10 @@ class TestCloneOperationIntegration: assert result["data"]["clone_status"] == "filesystem_error" assert "APFS clone operation failed" in result["error"]["message"] - # Verify real cleanup happened - assert not real_temp_cache.exists() + # ADR-018 Phase 0b: Temp cache is KEPT on failure with complete download (for debugging/retry) + assert real_temp_cache.exists() + # Download marker should exist (indicating complete download) + assert (real_temp_cache / ".mlxk2_download_complete").exists() @pytest.mark.spec @@ -725,7 +702,7 @@ class TestCloneJSONAPICompliance: patch('mlxk2.operations.health.health_from_cache') as mock_health, \ patch('mlxk2.operations.clone._apfs_clone_directory'): - mock_create_cache.return_value = real_temp_cache + mock_create_cache.return_value = (real_temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = {"status": "success", "data": {"model": model_spec}} mock_health.return_value = (True, "Model is healthy") @@ -840,7 +817,7 @@ class TestCloneCoreFeatures: temp_cache1.mkdir() # Create directory so .exists() returns True temp_cache2 = tmp_path / "temp_cache_2" temp_cache2.mkdir() # Create directory so .exists() returns True - mock_create_cache.side_effect = [temp_cache1, temp_cache2] + mock_create_cache.side_effect = [(temp_cache1, True), (temp_cache2, True)] # (temp_cache, should_download) mock_health.return_value = (True, "Model is healthy") # Setup side effects for both clones @@ -899,7 +876,7 @@ class TestCloneCoreFeatures: # Different temp cache (not user cache) temp_cache = tmp_path / "temp_cache" temp_cache.mkdir() # Create directory so .exists() returns True - mock_create_cache.return_value = temp_cache + mock_create_cache.return_value = (temp_cache, True) # (temp_cache, should_download) mock_health.return_value = (True, "Model is healthy") mock_snapshot = MagicMock() @@ -942,7 +919,7 @@ class TestCloneEdgeCases: patch('mlxk2.operations.clone._apfs_clone_directory') as mock_clone, \ patch('mlxk2.operations.clone._cleanup_temp_cache_safe'): - mock_create_cache.return_value = temp_cache + mock_create_cache.return_value = (temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = {"status": "success", "data": {"model": model_spec}} # Mock snapshot resolution @@ -977,7 +954,7 @@ class TestCloneEdgeCases: patch('mlxk2.operations.clone.pull_to_cache') as mock_pull, \ patch('mlxk2.operations.clone._resolve_latest_snapshot') as mock_resolve: - mock_create_cache.return_value = real_temp_cache + mock_create_cache.return_value = (real_temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = {"status": "success", "data": {"model": model_spec}} mock_resolve.return_value = None # Snapshot not found @@ -988,8 +965,9 @@ class TestCloneEdgeCases: assert result["data"]["clone_status"] == "cache_not_found" assert "Temp cache snapshot not found" in result["error"]["message"] - # Verify real cleanup happened - assert not real_temp_cache.exists() + # ADR-018 Phase 0b: Temp cache is KEPT on failure with complete download + assert real_temp_cache.exists() + assert (real_temp_cache / ".mlxk2_download_complete").exists() def test_clone_operation_target_existing_empty(self, tmp_path): """Test clone operation with existing empty target directory.""" @@ -1009,7 +987,7 @@ class TestCloneEdgeCases: patch('mlxk2.operations.clone._apfs_clone_directory') as mock_clone, \ patch('mlxk2.operations.clone._cleanup_temp_cache_safe'): - mock_create_cache.return_value = temp_cache + mock_create_cache.return_value = (temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = {"status": "success", "data": {"model": "test/model"}} mock_health.return_value = (True, "Model is healthy") @@ -1071,7 +1049,7 @@ class TestUnhealthyModelClone: # Setup mocks temp_cache = tmp_path / "temp" temp_cache.mkdir() - mock_temp_cache.return_value = temp_cache + mock_temp_cache.return_value = (temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = { "status": "success", @@ -1125,7 +1103,7 @@ class TestUnhealthyModelClone: # Setup mocks temp_cache = tmp_path / "temp" temp_cache.mkdir() - mock_temp_cache.return_value = temp_cache + mock_temp_cache.return_value = (temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = { "status": "success", @@ -1172,7 +1150,7 @@ class TestUnhealthyModelClone: # Setup mocks temp_cache = tmp_path / "temp" temp_cache.mkdir() - mock_temp_cache.return_value = temp_cache + mock_temp_cache.return_value = (temp_cache, True) # (temp_cache, should_download) mock_pull.return_value = { "status": "success", @@ -1194,4 +1172,228 @@ class TestUnhealthyModelClone: # Health status should NOT be in result assert "health" not in result["data"] - assert "health_reason" not in result["data"] \ No newline at end of file + assert "health_reason" not in result["data"] + +class TestResumableClone: + """Test suite for ADR-018 Phase 0b: Resumable Clone functionality.""" + + def test_deterministic_temp_cache_name_consistency(self, tmp_path): + """Test that temp cache name is deterministic for same inputs.""" + model_spec = "mlx-community/Llama-3.2-3B" + target = tmp_path / "workspace" + + # Same inputs should produce same name + name1 = _get_deterministic_temp_cache_name(model_spec, target) + name2 = _get_deterministic_temp_cache_name(model_spec, target) + + assert name1 == name2 + assert name1.startswith(".mlxk2_temp_") + assert len(name1) == 28 # ".mlxk2_temp_" + 16 hex chars + + def test_deterministic_temp_cache_name_different_inputs(self, tmp_path): + """Test that different inputs produce different names.""" + target = tmp_path / "workspace" + + name1 = _get_deterministic_temp_cache_name("model-a", target) + name2 = _get_deterministic_temp_cache_name("model-b", target) + name3 = _get_deterministic_temp_cache_name("model-a", tmp_path / "other") + + assert name1 != name2 # Different model + assert name1 != name3 # Different target + assert name2 != name3 + + def test_check_temp_cache_resume_no_existing(self, tmp_path): + """Test resume check when temp cache doesn't exist.""" + temp_cache = tmp_path / "nonexistent" + + can_resume, reason, is_healthy = _check_temp_cache_resume(temp_cache, "model") + + assert can_resume is False + assert "No existing download" in reason + assert is_healthy is False + + def test_check_temp_cache_resume_incomplete_download(self, tmp_path): + """Test resume check when download is incomplete (no marker) - should be resumable.""" + temp_cache = tmp_path / "temp" + temp_cache.mkdir() + + # Create some files but no completion marker (simulates Ctrl-C) + (temp_cache / "partial.bin").write_text("data") + + can_resume, reason, is_healthy = _check_temp_cache_resume(temp_cache, "model") + + # Partial downloads are resumable via HuggingFace snapshot_download + assert can_resume is True + assert "Partial download" in reason + assert "resumable" in reason.lower() + assert is_healthy is False # Not complete yet + + def test_check_temp_cache_resume_complete_healthy(self, tmp_path): + """Test resume check when download is complete and healthy.""" + temp_cache = tmp_path / "temp" + temp_cache.mkdir() + + # Mark as complete + (temp_cache / ".mlxk2_download_complete").write_text("completed") + + # Mock healthy check + with patch('mlxk2.operations.health.health_from_cache', return_value=(True, "OK")): + can_resume, reason, is_healthy = _check_temp_cache_resume(temp_cache, "model") + + assert can_resume is True + assert "healthy" in reason.lower() + assert is_healthy is True + + def test_check_temp_cache_resume_complete_unhealthy(self, tmp_path): + """Test resume check when download is complete but unhealthy.""" + temp_cache = tmp_path / "temp" + temp_cache.mkdir() + + # Mark as complete + (temp_cache / ".mlxk2_download_complete").write_text("completed") + + # Mock unhealthy check + with patch('mlxk2.operations.health.health_from_cache', return_value=(False, "Missing weights")): + can_resume, reason, is_healthy = _check_temp_cache_resume(temp_cache, "model") + + assert can_resume is True + assert "unhealthy" in reason.lower() + assert is_healthy is False + + def test_create_temp_cache_new(self, tmp_path): + """Test creating new temp cache when none exists.""" + target = tmp_path / "workspace" + model_spec = "test/model" + + with patch('mlxk2.operations.clone._get_volume_mount_point', return_value=tmp_path): + temp_cache, should_download = _create_temp_cache_same_volume(target, model_spec, False) + + assert temp_cache.exists() + assert should_download is True + assert (temp_cache / ".mlxk2_temp_cache_sentinel").exists() + + def test_create_temp_cache_resume_healthy(self, tmp_path): + """Test resuming healthy existing temp cache.""" + target = tmp_path / "workspace" + model_spec = "test/model" + + # Create existing temp cache with deterministic name + with patch('mlxk2.operations.clone._get_volume_mount_point', return_value=tmp_path): + temp_name = _get_deterministic_temp_cache_name(model_spec, target) + temp_cache = tmp_path / temp_name + temp_cache.mkdir() + (temp_cache / ".mlxk2_download_complete").write_text("completed") + + # Mock healthy + with patch('mlxk2.operations.health.health_from_cache', return_value=(True, "OK")): + result_cache, should_download = _create_temp_cache_same_volume(target, model_spec, False) + + assert result_cache == temp_cache + assert should_download is False # Skip download - healthy resume + + def test_create_temp_cache_resume_unhealthy_force(self, tmp_path): + """Test force resuming unhealthy temp cache.""" + target = tmp_path / "workspace" + model_spec = "test/model" + + with patch('mlxk2.operations.clone._get_volume_mount_point', return_value=tmp_path): + temp_name = _get_deterministic_temp_cache_name(model_spec, target) + temp_cache = tmp_path / temp_name + temp_cache.mkdir() + (temp_cache / ".mlxk2_download_complete").write_text("completed") + + # Mock unhealthy + with patch('mlxk2.operations.health.health_from_cache', return_value=(False, "Broken")): + result_cache, should_download = _create_temp_cache_same_volume(target, model_spec, force_resume=True) + + assert result_cache == temp_cache + assert should_download is False # Skip download - forced resume + + def test_create_temp_cache_resume_unhealthy_no_force(self, tmp_path): + """Test deleting unhealthy temp cache when not forcing.""" + target = tmp_path / "workspace" + model_spec = "test/model" + + with patch('mlxk2.operations.clone._get_volume_mount_point', return_value=tmp_path): + temp_name = _get_deterministic_temp_cache_name(model_spec, target) + temp_cache = tmp_path / temp_name + temp_cache.mkdir() + (temp_cache / ".mlxk2_download_complete").write_text("completed") + (temp_cache / "data.bin").write_text("old data") + + # Mock unhealthy + with patch('mlxk2.operations.health.health_from_cache', return_value=(False, "Broken")): + result_cache, should_download = _create_temp_cache_same_volume(target, model_spec, force_resume=False) + + # Should have deleted and recreated + assert result_cache.exists() + assert should_download is True + assert not (result_cache / "data.bin").exists() # Old data gone + + def test_create_temp_cache_resume_partial_download(self, tmp_path): + """Test resuming partial download (no completion marker) - should call pull_to_cache.""" + target = tmp_path / "workspace" + model_spec = "test/model" + + with patch('mlxk2.operations.clone._get_volume_mount_point', return_value=tmp_path): + temp_name = _get_deterministic_temp_cache_name(model_spec, target) + temp_cache = tmp_path / temp_name + temp_cache.mkdir() + # No completion marker - simulates Ctrl-C during download + (temp_cache / "partial.bin").write_text("partial data") + + result_cache, should_download = _create_temp_cache_same_volume(target, model_spec, force_resume=False) + + # Should resume partial download via HuggingFace + assert result_cache == temp_cache + assert should_download is True # Call pull_to_cache to resume + assert (result_cache / "partial.bin").exists() # Partial data preserved + + def test_keyboard_interrupt_preserves_temp_cache(self, tmp_path): + """Test that KeyboardInterrupt (Ctrl-C) preserves temp cache for resume.""" + target = tmp_path / "workspace" + model_spec = "test/model" + + with patch('mlxk2.operations.clone._validate_apfs_filesystem'): + with patch('mlxk2.operations.clone._validate_same_volume'): + with patch('mlxk2.operations.clone._get_volume_mount_point', return_value=tmp_path): + # Mock pull_to_cache to raise KeyboardInterrupt + with patch('mlxk2.operations.clone.pull_to_cache', side_effect=KeyboardInterrupt()): + result = clone_operation(model_spec, str(target)) + + # Check result structure + assert result["status"] == "error" + assert result["error"]["type"] == "UserCancelledError" + assert result["data"]["clone_status"] == "cancelled" + + # Check error message has resume instructions + error_msg = result["error"]["message"] + assert "Operation cancelled by user" in error_msg + assert "Partial download preserved" in error_msg + assert "To resume: Run the same command again" in error_msg + + # Verify temp cache exists and was NOT deleted + temp_name = _get_deterministic_temp_cache_name(model_spec, target) + temp_cache = tmp_path / temp_name + assert temp_cache.exists(), "Temp cache should be preserved after Ctrl-C" + assert (temp_cache / ".mlxk2_temp_cache_sentinel").exists() + + def test_keyboard_interrupt_early_no_temp_cache(self, tmp_path): + """Test KeyboardInterrupt before temp cache creation.""" + target = tmp_path / "workspace" + model_spec = "test/model" + + with patch('mlxk2.operations.clone._validate_apfs_filesystem'): + with patch('mlxk2.operations.clone._validate_same_volume'): + # Raise KeyboardInterrupt during volume mount check + with patch('mlxk2.operations.clone._get_volume_mount_point', side_effect=KeyboardInterrupt()): + result = clone_operation(model_spec, str(target)) + + # Should handle gracefully even without temp cache + assert result["status"] == "error" + assert result["error"]["type"] == "UserCancelledError" + assert result["data"]["clone_status"] == "cancelled" + + # Error message should be simple (no temp cache to preserve) + error_msg = result["error"]["message"] + assert error_msg == "Operation cancelled by user." diff --git a/tests_2.0/test_model_resolution_workspace.py b/tests_2.0/test_model_resolution_workspace.py new file mode 100644 index 0000000..c9b957a --- /dev/null +++ b/tests_2.0/test_model_resolution_workspace.py @@ -0,0 +1,138 @@ +"""Tests for model resolution workspace path support (ADR-018 Phase 0c). + +Tests that resolve_model_for_operation() correctly handles: +- Local workspace paths (managed and unmanaged) +- HF model IDs (existing cache behavior) +- Edge cases and error handling +""" + +import pytest +from pathlib import Path + +from mlxk2.core.model_resolution import resolve_model_for_operation + + +class TestResolveModelForOperationWorkspace: + """Test workspace path resolution (ADR-018 Phase 0c).""" + + def test_resolve_workspace_path_absolute(self, tmp_path): + """Test resolves absolute workspace path.""" + (tmp_path / "config.json").write_text('{"model_type": "llama"}') + + resolved_name, commit_hash, ambiguous = resolve_model_for_operation(str(tmp_path)) + + assert resolved_name == str(tmp_path.resolve()) + assert commit_hash is None + assert ambiguous is None + + def test_resolve_workspace_path_relative(self, tmp_path, monkeypatch): + """Test resolves relative workspace path with ./ prefix.""" + workspace = tmp_path / "my-workspace" + workspace.mkdir() + (workspace / "config.json").write_text('{"model_type": "llama"}') + + # Change to parent dir, then use relative path with ./ + monkeypatch.chdir(tmp_path) + + resolved_name, commit_hash, ambiguous = resolve_model_for_operation("./my-workspace") + + assert resolved_name == str(workspace.resolve()) + assert commit_hash is None + assert ambiguous is None + + def test_resolve_workspace_path_with_dot(self, tmp_path, monkeypatch): + """Test resolves ./workspace notation.""" + (tmp_path / "config.json").write_text('{"model_type": "llama"}') + + monkeypatch.chdir(tmp_path) + + resolved_name, commit_hash, ambiguous = resolve_model_for_operation(".") + + assert resolved_name == str(tmp_path.resolve()) + assert commit_hash is None + assert ambiguous is None + + def test_resolve_nonexistent_explicit_path_returns_none(self): + """Test that nonexistent explicit paths (./) fall through to cache logic.""" + resolved_name, commit_hash, ambiguous = resolve_model_for_operation("./nonexistent") + + # Explicit path ./nonexistent doesn't exist, doesn't match workspace check + # Falls through to cache logic, which tries to expand it + assert resolved_name is None + assert ambiguous == [] + + def test_resolve_name_without_prefix_uses_cache_logic(self): + """Test that names without ./ prefix go through cache resolution.""" + # Even if a local directory exists, without ./ it should try cache first + resolved_name, commit_hash, ambiguous = resolve_model_for_operation("some-model") + + # Should use cache logic (not find it, return None) + assert resolved_name is None or "/" in resolved_name # Either not found or cache path + assert ambiguous is not None # Cache logic sets this + + def test_resolve_explicit_path_without_config_falls_back(self, tmp_path): + """Test explicit path (./) without config.json falls back to cache logic.""" + # Directory exists but no config.json + # Use ./ to make it an explicit path + import os + cwd = os.getcwd() + try: + os.chdir(tmp_path.parent) + resolved_name, commit_hash, ambiguous = resolve_model_for_operation(f"./{tmp_path.name}") + + # Explicit path exists but no config.json → not a workspace, falls to cache logic + assert resolved_name is None or ambiguous is not None + finally: + os.chdir(cwd) + + def test_resolve_workspace_ignores_at_hash_syntax(self, tmp_path): + """Test that @hash syntax doesn't affect workspace paths.""" + # Edge case: workspace path with @ in it (unlikely but should handle) + workspace = tmp_path / "workspace" + workspace.mkdir() + (workspace / "config.json").write_text('{"model_type": "llama"}') + + # Even with @something, if it's a valid workspace path, use it + resolved_name, commit_hash, ambiguous = resolve_model_for_operation(str(workspace)) + + assert str(workspace.resolve()) in resolved_name + assert commit_hash is None + + +class TestResolveModelOperationBackwardCompatibility: + """Test that existing cache-based resolution still works.""" + + def test_resolve_hf_model_id_still_works(self, tmp_path, monkeypatch): + """Test HF model IDs are not treated as workspace paths.""" + # Mock cache to return expected result for cache-based resolution + from mlxk2.core import cache + monkeypatch.setattr(cache, "get_current_model_cache", lambda: tmp_path) + + # HF model ID (not a path) should go through cache logic + resolved_name, commit_hash, ambiguous = resolve_model_for_operation("mlx-community/Phi-3-mini") + + # Cache logic: exact match fails, returns (None, None, []) + # The key assertion: it's NOT treated as workspace path (would return absolute path) + assert resolved_name is None or not Path(resolved_name).exists() + assert ambiguous == [] or ambiguous is None # Empty list or None for not found + + def test_resolve_short_name_expansion_still_works(self, tmp_path, monkeypatch): + """Test short name expansion (existing behavior) still works.""" + from mlxk2.core import cache + + # Create mock cache structure + cache_dir = tmp_path / "cache" + cache_dir.mkdir() + model_dir = cache_dir / "models--mlx-community--Phi-3-mini-4k-instruct-4bit" + model_dir.mkdir(parents=True) + snapshot_dir = model_dir / "snapshots" / "abc123" + snapshot_dir.mkdir(parents=True) + (snapshot_dir / "config.json").write_text('{}') + + monkeypatch.setattr(cache, "get_current_model_cache", lambda: cache_dir) + + # Short name "phi-3" should expand via cache logic + resolved_name, commit_hash, ambiguous = resolve_model_for_operation("Phi-3") + + # Either finds the model or returns ambiguous/not found + assert resolved_name is None or ambiguous is not None or "Phi-3" in str(resolved_name) diff --git a/tests_2.0/test_resumable_pull.py b/tests_2.0/test_resumable_pull.py index d0919e1..ec19a53 100644 --- a/tests_2.0/test_resumable_pull.py +++ b/tests_2.0/test_resumable_pull.py @@ -12,7 +12,7 @@ IMPORTANT: This test MUST stay in tests_2.0/ (NOT tests_2.0/live/). The live/ directory has pytest hooks for Portfolio Discovery that interfere with the isolated_cache fixture, causing the test to fail. -IMPORTANT: This test uses live_resumable marker (not live_e2e) because +IMPORTANT: This test uses live_pull marker (not live_e2e) because module-scoped fixtures from live/conftest.py (_use_real_mlx_modules, vision_portfolio, text_portfolio) interfere with HuggingFace Hub's symlink creation mechanism during resume. These fixtures manipulate sys.path and @@ -20,7 +20,7 @@ run subprocesses, causing import resolution issues. Must run independently. Opt-in: Requires MLXK2_TEST_RESUMABLE_DOWNLOAD=1 (network test) -Run: MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_resumable tests_2.0/test_resumable_pull.py -v +Run: MLXK2_TEST_RESUMABLE_DOWNLOAD=1 pytest -m live_pull tests_2.0/test_resumable_pull.py -v """ import os @@ -30,8 +30,8 @@ import subprocess import pytest from pathlib import Path -# Mark as live_resumable (isolated from live_e2e module fixtures) -pytestmark = [pytest.mark.live_resumable] +# Mark as live_pull (isolated from live_e2e module fixtures) +pytestmark = [pytest.mark.live_pull] @pytest.mark.skipif( diff --git a/tests_2.0/test_run_vision.py b/tests_2.0/test_run_vision.py index 229e9ed..48bb890 100644 --- a/tests_2.0/test_run_vision.py +++ b/tests_2.0/test_run_vision.py @@ -159,10 +159,9 @@ def test_vision_adds_filename_mapping_for_multiple_images(): result = VisionRunner._add_filename_mapping("Images 1 and 3 show motorboats.", images) # Updated for ADR-017 Phase 1: Collapsible HTML table with EXIF columns (enabled by default) + # Session 75: Metadata moved to BEGINNING for better UX in chunking scenarios # Hashes: sha256(b"data1")[:8] = 5b41362b, etc. - expected = """Images 1 and 3 show motorboats. - -
+ expected = """
📸 Image Metadata (3 images) @@ -173,7 +172,8 @@ def test_vision_adds_filename_mapping_for_multiple_images(): | 3 | image_f60f2d65.jpeg | vacation3.jpg | - | - | - |
-""" + +Images 1 and 3 show motorboats.""" assert result == expected @@ -185,10 +185,9 @@ def test_vision_no_mapping_for_single_image(): result = VisionRunner._add_filename_mapping("A dog.", images) # Updated for ADR-017 Phase 1: Collapsible HTML table with EXIF columns (enabled by default) + # Session 75: Metadata moved to BEGINNING for better UX in chunking scenarios # Hash: sha256(b"data")[:8] = 3a6eb079 - expected = """A dog. - -
+ expected = """
📸 Image Metadata (1 image) @@ -197,5 +196,6 @@ def test_vision_no_mapping_for_single_image(): | 1 | image_3a6eb079.jpeg | single.jpg | - | - | - |
-""" + +A dog.""" assert result == expected diff --git a/tests_2.0/test_stop_tokens_live.py b/tests_2.0/test_stop_tokens_live.py index 0f9bd54..0dc9229 100644 --- a/tests_2.0/test_stop_tokens_live.py +++ b/tests_2.0/test_stop_tokens_live.py @@ -181,9 +181,10 @@ def discover_mlx_models_in_user_cache() -> List[Dict[str, Any]]: Filters for: - Framework: MLX only (not GGUF/PyTorch) - - Health: healthy only + - Health: healthy only (static file integrity) - Runtime: runtime_compatible only (mlx-lm/mlx-vlm can load) - Type: chat models (TEXT + VISION, includes all model_type="chat") + - Exclusions: KNOWN_BROKEN_MODELS (upstream runtime bugs) Note: Returns BOTH text and vision models. Caller must filter by capabilities if needed (e.g., portfolio_models fixture filters to TEXT-only). @@ -197,6 +198,15 @@ def discover_mlx_models_in_user_cache() -> List[Dict[str, Any]]: from mlxk2.core.model_resolution import resolve_model_for_operation from mlxk2.core.cache import get_current_model_cache, hf_to_cache_dir + # Import blacklist (local import to avoid circular dependency) + # KNOWN_BROKEN_MODELS is defined in tests_2.0/live/test_utils.py + try: + sys.path.insert(0, str(Path(__file__).parent / "live")) + from test_utils import KNOWN_BROKEN_MODELS + sys.path.pop(0) + except ImportError: + KNOWN_BROKEN_MODELS = set() # Fallback if import fails + # Check HF_HOME is set (required for mlxk list) env = os.environ.copy() if not env.get("HF_HOME"): @@ -248,6 +258,10 @@ def discover_mlx_models_in_user_cache() -> List[Dict[str, Any]]: except Exception: pass + # FILTER: Exclude known broken models (upstream runtime bugs) + if model_name in KNOWN_BROKEN_MODELS: + continue + # Ensure cache directory exists (defensive against stale listings) try: cache_dir = get_current_model_cache() / hf_to_cache_dir(model_name) diff --git a/tests_2.0/test_vision_adapter.py b/tests_2.0/test_vision_adapter.py index def3048..b832783 100644 --- a/tests_2.0/test_vision_adapter.py +++ b/tests_2.0/test_vision_adapter.py @@ -10,9 +10,8 @@ import pytest # No MLXKError import needed - using standard ValueError from mlxk2.tools.vision_adapter import ( VisionHTTPAdapter, - MAX_IMAGES_PER_REQUEST, + MAX_SAFE_CHUNK_SIZE, MAX_IMAGE_SIZE_BYTES, - MAX_TOTAL_IMAGE_BYTES, ) @@ -301,48 +300,9 @@ class TestParseOpenAIMessages: assert "url cannot be empty" in str(exc.value).lower() - def test_too_many_images_raises_error(self): - """Test that exceeding image count limit raises validation error.""" - # Create more than MAX_IMAGES_PER_REQUEST images - content = [{"type": "text", "text": "Many images"}] - for _ in range(MAX_IMAGES_PER_REQUEST + 1): - content.append({ - "type": "image_url", - "image_url": {"url": f"data:image/jpeg;base64,{VALID_JPEG_B64}"} - }) - - messages = [{"role": "user", "content": content}] - - with pytest.raises(ValueError) as exc: - VisionHTTPAdapter.parse_openai_messages(messages) - - assert "Too many images" in str(exc.value) - assert str(MAX_IMAGES_PER_REQUEST) in str(exc.value) - - def test_total_image_size_limit_enforced(self): - """Test that total image size limit is enforced (critical for Metal API).""" - # Create images that individually pass but collectively exceed total limit - # Each image ~3 MB → 5 images would be ~15 MB (under 50 MB limit) - # But we'll create larger images to trigger the limit - large_data = "A" * (12 * 1024 * 1024) # 12 MB per image - large_b64 = base64.b64encode(large_data.encode()).decode() - - content = [{"type": "text", "text": "Many large images"}] - # 5 images × 12 MB = 60 MB → exceeds 50 MB limit - for _ in range(5): - content.append({ - "type": "image_url", - "image_url": {"url": f"data:image/jpeg;base64,{large_b64}"} - }) - - messages = [{"role": "user", "content": content}] - - with pytest.raises(ValueError) as exc: - VisionHTTPAdapter.parse_openai_messages(messages) - - assert "Total image size" in str(exc.value) - assert "exceeds limit" in str(exc.value) - assert "Try fewer or smaller images" in str(exc.value) + # Note: Image count/size limits removed in Session 74 (ADR-012 Phase 1c) + # Chunking now handles batch safety - unlimited images allowed in adapter + # Validation happens at request level (run.py/server_base.py) not adapter level class TestSequentialImageExtraction: diff --git a/tests_2.0/test_workspace_sentinel.py b/tests_2.0/test_workspace_sentinel.py index 97d399f..c23f1a6 100644 --- a/tests_2.0/test_workspace_sentinel.py +++ b/tests_2.0/test_workspace_sentinel.py @@ -12,22 +12,60 @@ from pathlib import Path import pytest +from mlxk2 import __version__ from mlxk2.operations.workspace import ( write_workspace_sentinel, is_managed_workspace, + is_workspace_path, read_workspace_metadata, SENTINEL_FILENAME ) from mlxk2.operations.health import health_check_workspace +class TestIsWorkspacePath: + """Test is_workspace_path() helper function (ADR-018 Phase 0c).""" + + def test_is_workspace_path_valid_workspace(self, tmp_path): + """Test detects valid workspace with config.json.""" + (tmp_path / "config.json").write_text('{"model_type": "llama"}') + assert is_workspace_path(tmp_path) is True + assert is_workspace_path(str(tmp_path)) is True # String path + + def test_is_workspace_path_no_config(self, tmp_path): + """Test returns False if config.json missing.""" + (tmp_path / "other.json").write_text('{}') + assert is_workspace_path(tmp_path) is False + + def test_is_workspace_path_nonexistent(self, tmp_path): + """Test returns False for nonexistent path.""" + assert is_workspace_path(tmp_path / "nonexistent") is False + + def test_is_workspace_path_hf_model_id(self, tmp_path): + """Test returns False for HF model IDs (not paths).""" + assert is_workspace_path("mlx-community/Phi-3-mini") is False + assert is_workspace_path("microsoft/phi-2") is False + + def test_is_workspace_path_file_not_directory(self, tmp_path): + """Test returns False if path is a file, not directory.""" + config_file = tmp_path / "config.json" + config_file.write_text('{}') + assert is_workspace_path(config_file) is False + + def test_is_workspace_path_invalid_input(self): + """Test handles invalid input gracefully.""" + assert is_workspace_path(None) is False + assert is_workspace_path(123) is False + assert is_workspace_path([]) is False + + class TestWorkspaceSentinel: """Test workspace sentinel write/read primitives.""" def test_write_sentinel_atomic(self, tmp_path): """Test atomic write with rename.""" metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "source_repo": "mlx-community/Llama-3.2-3B", "source_revision": "abc123", @@ -43,14 +81,14 @@ class TestWorkspaceSentinel: # Verify JSON is valid and matches input data = json.loads(sentinel.read_text()) assert data["managed"] is True - assert data["mlxk_version"] == "2.0.4" + assert data["mlxk_version"] == __version__ assert data["operation"] == "clone" assert data["source_repo"] == "mlx-community/Llama-3.2-3B" def test_write_sentinel_creates_valid_json(self, tmp_path): """Test sentinel is well-formed JSON.""" metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "managed": True, "operation": "convert" @@ -91,7 +129,7 @@ class TestWorkspaceSentinel: def test_write_sentinel_allows_additional_fields(self, tmp_path): """Test forward compatibility: extra fields allowed.""" metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "managed": True, "operation": "clone", @@ -112,7 +150,7 @@ class TestManagedWorkspaceDetection: def test_is_managed_workspace_true(self, tmp_path): """Test managed workspace detection (has valid sentinel).""" metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "managed": True, "operation": "clone" @@ -129,7 +167,7 @@ class TestManagedWorkspaceDetection: def test_is_managed_workspace_false_managed_field_false(self, tmp_path): """Test sentinel exists but managed=False.""" metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "managed": False, # Explicitly unmanaged "operation": "manual" @@ -153,7 +191,7 @@ class TestManagedWorkspaceDetection: def test_read_workspace_metadata_valid(self, tmp_path): """Test reading sentinel metadata.""" metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "source_repo": "mlx-community/Model", "managed": True, @@ -162,7 +200,7 @@ class TestManagedWorkspaceDetection: write_workspace_sentinel(tmp_path, metadata) read_data = read_workspace_metadata(tmp_path) - assert read_data["mlxk_version"] == "2.0.4" + assert read_data["mlxk_version"] == __version__ assert read_data["source_repo"] == "mlx-community/Model" def test_read_workspace_metadata_no_sentinel(self, tmp_path): @@ -187,7 +225,7 @@ class TestWorkspaceHealthCheck: # Create minimal valid workspace (tmp_path / "config.json").write_text('{"model_type": "llama"}') metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "managed": True, "operation": "clone" @@ -211,7 +249,7 @@ class TestWorkspaceHealthCheck: """Test workspace without config.json is unhealthy.""" # Empty directory metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "managed": True, "operation": "clone" @@ -248,7 +286,7 @@ class TestHealthCheckOperationWorkspaceIntegration: (tmp_path / "model.safetensors").write_text("fake weights") metadata = { - "mlxk_version": "2.0.4", + "mlxk_version": __version__, "created_at": "2025-12-29T10:30:00Z", "managed": True, "operation": "clone"