feat: project scaffold with module graph and CLI stub#5
Merged
Conversation
- build.zig.zon: package manifest (Zig >= 0.16.0) - build.zig: 7-module graph with compile-enforced dependency direction core (leaf), mlxbridge (leaf), chat->core, model->core+mlxbridge, engine->core+mlxbridge+model (NOT chat), registry->core, http->core+chat+model+engine+registry - src/cli/main.zig: stub subcommand dispatch (serve/run/pull/list) - Per-module test stubs under zig build test - Empty subdirs with .gitkeep for future expansion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
tlkahn
commented
Jul 5, 2026
tlkahn
commented
Jul 5, 2026
tlkahn
commented
Jul 5, 2026
tlkahn
commented
Jul 5, 2026
tlkahn
commented
Jul 5, 2026
tlkahn
commented
Jul 5, 2026
Unimplemented subcommands now write their stub message to stderr and exit 1 instead of claiming success. writeAll surfaces write failures via error.WriteFailed with errno-based handling (EINTR retried), and every caller already returns 1, so a failed write can never mask a non-zero exit. The duplicated command-name/message pairs collapse into a single name list with a runtime loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016qUaL3Er38EYQCRoExKwzA
The cli root module joins the test_modules loop so its inline tests run under 'zig build test'. A shared build_options module carries the new -Dgpu-tests flag so Metal-dependent tests can gate themselves via 'if (!build_options.gpu_tests) return error.SkipZigTest'. The 'itest' step documented in CLAUDE.md now exists, backed by a stub tests/itest.zig that skips without -Dmodel and fails honestly with IntegrationTestsNotYetImplemented when one is given. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016qUaL3Er38EYQCRoExKwzA
All seven modules now map their @import name to src/<name>/root.zig. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016qUaL3Er38EYQCRoExKwzA
tlkahn
added a commit
that referenced
this pull request
Jul 5, 2026
… 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
tlkahn
added a commit
that referenced
this pull request
Jul 5, 2026
* feat: attach native deps - vendored jinja_cpp and brew-gated mlx-c 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 * fix(chat): wrap jinja C API in renderTemplate, document mlxbridge ABI 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 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
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
core,mlxbridgeas leaves;engineimportscore+mlxbridge+modelbut NOTchat;httpat the top)serve/run/pull/list) with usage on bad inputzig build test.gitkeep) for futurespecdecode,constrain,handlers,cmd,fixturesCloses #2
Test plan
zig buildproduceszig-out/bin/mlxd./zig-out/bin/mlxd serve/run/pull/listeach exit 0./zig-out/bin/mlxd(no args) and unknown commands exit 1zig build testpasses all 7 module tests@import("chat")toengine/root.zigfails withno module named 'chat' available within module 'engine'🤖 Generated with Claude Code
https://claude.ai/code/session_018JYKbyZSBQmhyVyKpPEXpH
Learning tip: why
writeAllinsrc/cli/main.ziglooks like raw syscallsfn writeAll(fd: std.posix.fd_t, bytes: []const u8) error{WriteFailed}!voidloops onstd.posix.system.writeand switches onstd.posix.errno(rc)directly, which reads more like C than idiomatic Zig. This is deliberate, not an oversight, and follows from how Zig 0.16 restructured I/O ("Writergate"):std.debug.printstill exists but its doc comment says it "writes to stderr, ignoring errors" and calls itself a low-level debugging primitive. Review finding M0 Skeleton: build system, module graph, vendored deps, CI #1 on this PR was specifically about swallowing write errors, so an API that swallows them by design can't satisfy it - it's also meant for debugging output, not user-facing CLI output.std.Iointerface is the idiomatic 0.16 way to do buffered, error-propagating I/O (there's no more ambientstd.io.getStdOut().writer()). But anIoinstance is only available ifmainaccepts the fullstd.process.Init. This scaffold'smainintentionally takesstd.process.Init.Minimal(just args/environ, no I/O runtime) to keep the stub binary free of runtime machinery it doesn't need yet.std.posix.write, the mid-level wrapper that used to retry onEINTRfor you, was removed in 0.16. What's left at that layer is the rawstd.posix.system.writesyscall plusstd.posix.errnofor classifying the result.Given
Init.Minimalplus the "don't swallow write errors" requirement, the currentwriteAllis essentially reimplementing the ~8 linesstd.posix.writeused to provide. It's expected to be temporary: once the CLI does real work (serve,run),mainwill need the fullstd.process.Initanyway, and switching toinit.iowith properstd.Iowriters at that point should let this helper be deleted entirely.