M0 Stage 2: Native dependencies#6
Merged
Merged
Conversation
Stage 2 of the M0 skeleton: attach the two native dependencies to their owning modules so consumers inherit transitively. - vendor/jinja_cpp: llama.cpp's in-tree Jinja engine (MIT, from ggml-org/llama.cpp#18462) plus mlx-serve's C wrapper, sources only, built from source with -std=c++17 and attached to the chat module. Provenance and vendoring recipe recorded in README.md. - Homebrew version gate on macOS: mlx >= 0.31.2, mlx-c >= 0.6.0, actionable install/upgrade message and exit(1) when unmet. - mlxbridge links mlxc + Metal/Foundation/IOKit/CoreFoundation with brew prefix resolved dynamically. - Smoke tests: chat renders a trivial jinja template end-to-end; mlxbridge calls mlx_version behind -Dgpu-tests (symbol resolution is proven at link time even when skipped). Closes #3 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017z9pjMWW4Jdhd244PSHHVV
… policy Addresses Stage 2 review findings 1-6: - #1: C-owned render output is copied and freed inside renderTemplate; the malloc'd pointer never escapes src/chat/jinja.zig - #2: jinja_last_error() is read immediately on the failing call, same thread; externs are non-pub so the pair cannot be split - #3: mlxbridge file-header comment + CLAUDE.md convention establishing the ABI-drift policy (unchecked extern signatures, gated smoke tests, -Dgpu-tests run required on mlx-c upgrades) - #4: error-path tests for invalid templates, with and without a Diagnostics out-param - #5: inputs are dupeZ'd for NUL termination and interior NULs are rejected with error.InteriorNul before any C call - #6: null-with-no-error (malloc failure in the wrapper) falls back to "unknown jinja render error" instead of reading a null message Error messages surface via a std.json-style Diagnostics out-param. Wrapper takes raw JSON slices for now; typed DTOs deferred to M1. Unlike the old mlx-serve chat.zig, render failure returns an error instead of silently falling back to a generic template. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017z9pjMWW4Jdhd244PSHHVV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stage 2 of the M0 skeleton (epic #1): attaches the two native dependencies to their owning modules so consumers inherit transitively. No other module touches native deps directly.
Phase 2.1: Vendored jinja_cpp
vendor/jinja_cpp/: llama.cpp's in-tree Jinja engine (common/jinja/, from implement new jinja template engine ggml-org/llama.cpp#18462, the PR that replaced minja) plus mlx-serve's 3-function C wrapper. Sources only - nolibjinja.a, noobj/.vendor/jinja_cpp/README.md.build.zig(-std=c++17 -O2 -DNDEBUG, Zig's bundled clang++/libc++) and attached to thechatmodule only;http/cliinherit the link transitively.src/chat/root.zig:jinja_last_error()null check, plus an end-to-end render of a trivial template proving the whole engine compiles and links.Phase 2.2: Homebrew mlx gates
verifyBrewDeps(ported from mlx-serve): requiresmlx >= 0.31.2andmlx-c >= 0.6.0, strips_revsuffixes, fails configuration with an actionablebrew install/brew upgrademessage.mlxbridgelinksmlxc+ Metal/Foundation/IOKit/CoreFoundation, brew prefix resolved dynamically viabrew --prefix;model/engine/http/cliinherit transitively.src/mlxbridge/root.zig:mlx_versionbehind-Dgpu-tests; symbol resolution is proven at link time even when the test is skipped.Verification
zig buildfails with[mlxd] missing Homebrew dependency 'mlx' (>= 0.31.2). Install with: brew install mlx-cbrew install mlx-c(mlx 0.31.2, mlx-c 0.6.0_2 - exercises the_revstrip):zig buildsucceedszig build test: 9 pass, 1 skip (the gated mlx test)zig build test -Dgpu-tests: all 10 pass,mlx_versionreturns 0 with a non-empty versionvendor/(find vendor -name "*.a" -o -name "*.o"empty)@import("chat")in engine root fails to compile (reverted)Closes #3
🤖 Generated with Claude Code
https://claude.ai/code/session_017z9pjMWW4Jdhd244PSHHVV