Repository navigation
feat(#461): manifest-v2 PR1 — v2 types + header-first version parse - #462
Merged
Merged
Conversation
…(PR 1)
Add the v2 build-graph manifest type surface and a header-first, bounded
version parse, fully additive alongside the live v1 splice/enum path. No
codegen wiring (PR 2/3); types + parsing + tests only.
New module src/codegen/manifest_v2.zig:
- SUPPORTED_MANIFEST_VERSION = 2.
- ManifestHeader { manifest_version: u8 = 1 } — parsed FIRST with
ignore_unknown_fields so it reads the version off ANY manifest shape
(v1/field-less reads as 1).
- BackendManifestV2 — full v2 schema per design §3: id + capabilities
(retained, not dropped), modules (with root_alias), per-platform
system_libs/frameworks/loop_style/link_libc/pic/artifacts, platforms
matrix (entry template + Target .native/.triple/.resolved + Package
recipe + extra_modules + dep_options + root_build_deps with resolution),
build_hook (dedicated backend.hook.zig path).
- DepOption { name, value } declarative — value is a closed ValueSource
predicate (gui_is_imgui/gamepad_enabled/gamepad_hidapi/true_literal/
false_literal). No DependencyOptions type, no pre_wire.
- HookContext + HOOK_ABI_VERSION = 2 (types only; not invoked).
android_target_sdk is ?u32 and REQUIRED for Android (no silent default).
- parseManifest: header-first bounded parse — reject <1 or >SUPPORTED with
BackendManifestUnknownVersion; dispatch <=1 to v1 BackendManifest,
2..SUPPORTED to BackendManifestV2 (mirrors plugin_manifest.zig discipline).
Tests: synthetic v2 full-surface parse; v1-passthrough (field-less v1
routes to the v1 path unchanged); header reads version off both v1 and v2
shapes; version >SUPPORTED and <1 rejected; malformed → parse error; hook
ABI type surface. Registered in root.zig test discovery.
zig build test and zig build both exit 0; generation output byte-unchanged
(the only production change is a test{}-block import).
Claude-Session: https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughWalkthroughAdds a new manifest v2 module implementing a header-first, version-gated parsing scheme for build-graph manifests, including new schema types (BackendManifestV2), a hook ABI (HookContext), dispatch logic between v1 and v2 formats, and comprehensive unit tests. Wires the module into the root test suite. ChangesManifest v2 schema and parsing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant parseHeader
participant parseManifest
participant SpliceV1
participant BackendManifestV2
Caller->>parseHeader: raw_z manifest text
parseHeader-->>Caller: ManifestHeader (manifest_version)
Caller->>parseManifest: raw_z manifest text
parseManifest->>parseManifest: validate manifest_version range
alt manifest_version <= 1
parseManifest->>SpliceV1: parse as BackendManifest
SpliceV1-->>parseManifest: v1 manifest
else within v2 range
parseManifest->>BackendManifestV2: parse with ignore_unknown_fields
BackendManifestV2-->>parseManifest: v2 manifest
else out of range
parseManifest-->>Caller: error.BackendManifestUnknownVersion
end
parseManifest-->>Caller: ParsedManifest union
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/codegen/manifest_v2.zig (2)
287-289: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename
HookContext.manifest_versionto reflect it holds the hook ABI version.The doc comment says this field is asserted
== HOOK_ABI_VERSION, yet it's namedmanifest_version— the same name as the schema field onBackendManifestV2. BothHOOK_ABI_VERSIONandSUPPORTED_MANIFEST_VERSIONare currently2, so wiringctx.manifest_version = header.manifest_versionwould pass the assertion today by coincidence and silently break the moment either version diverges. A clearer name (e.g.abi_version) removes the footgun before the hook is wired in PR 2/3.♻️ Proposed rename
pub const HookContext = struct { /// Asserted `== HOOK_ABI_VERSION` before the hook is called. - manifest_version: u8, + abi_version: u8,Update the test on Line 645-647 accordingly if the field is referenced there in later PRs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codegen/manifest_v2.zig` around lines 287 - 289, Rename HookContext.manifest_version to a name that clearly indicates it stores HOOK_ABI_VERSION, such as abi_version, and update the struct doc/comment and any references in manifest_v2.zig accordingly. Make sure the field is no longer confused with BackendManifestV2.manifest_version so future wiring in hook call sites uses the ABI version explicitly. If any tests or later references in the file mention the old field name, update them to the new symbol as well.
345-389: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve the underlying ZON error for diagnostics.
Both
parseHeaderandparseManifestcollapse every parse failure into a bareerror.BackendManifestParseErrorwith no detail. The v1manifest_splice.loadManifestlogs the concrete ZON error (and path) viastd.log.warn(..., {any}, err)before returning the same generic error, so a backend author gets an actionable message. Mirroring that here keeps the two paths consistent and avoids a silent "parse failed" with no cause.♻️ Log the cause before mapping the error
pub fn parseManifest(allocator: std.mem.Allocator, raw_z: [:0]const u8) !ParsedManifest { ... if (v <= 1) { const m = std.zon.parse.fromSliceAlloc(splice.BackendManifest, allocator, raw_z, null, .{ .ignore_unknown_fields = true, - }) catch return error.BackendManifestParseError; + }) catch |err| { + std.log.warn("labelle-assembler: failed to parse v1 backend.manifest.zon: {any}", .{err}); + return error.BackendManifestParseError; + }; return .{ .v1 = m }; } const m = std.zon.parse.fromSliceAlloc(BackendManifestV2, allocator, raw_z, null, .{ .ignore_unknown_fields = true, - }) catch return error.BackendManifestParseError; + }) catch |err| { + std.log.warn("labelle-assembler: failed to parse v2 backend.manifest.zon: {any}", .{err}); + return error.BackendManifestParseError; + }; return .{ .v2 = m }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codegen/manifest_v2.zig` around lines 345 - 389, Both parseHeader and parseManifest currently erase the underlying ZON parse failure by immediately mapping it to error.BackendManifestParseError, so update these paths to retain and log the concrete parse error before returning the generic error. In parseHeader and both fromSliceAlloc calls inside parseManifest, capture the caught error into a symbol, emit a warning with the raw ZON error details using std.log.warn (mirroring manifest_splice.loadManifest), then return the same BackendManifestParseError so callers still see the existing API while gaining actionable diagnostics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/codegen/manifest_v2.zig`:
- Around line 287-289: Rename HookContext.manifest_version to a name that
clearly indicates it stores HOOK_ABI_VERSION, such as abi_version, and update
the struct doc/comment and any references in manifest_v2.zig accordingly. Make
sure the field is no longer confused with BackendManifestV2.manifest_version so
future wiring in hook call sites uses the ABI version explicitly. If any tests
or later references in the file mention the old field name, update them to the
new symbol as well.
- Around line 345-389: Both parseHeader and parseManifest currently erase the
underlying ZON parse failure by immediately mapping it to
error.BackendManifestParseError, so update these paths to retain and log the
concrete parse error before returning the generic error. In parseHeader and both
fromSliceAlloc calls inside parseManifest, capture the caught error into a
symbol, emit a warning with the raw ZON error details using std.log.warn
(mirroring manifest_splice.loadManifest), then return the same
BackendManifestParseError so callers still see the existing API while gaining
actionable diagnostics.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 264e3866-e405-4c9c-82ac-7dbb4097bfbf
📒 Files selected for processing (2)
src/codegen/manifest_v2.zigsrc/root.zig
This was referenced Jul 3, 2026
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.
PR 1 of the manifest-v2 epic (#461, #453 item 3). The v2 type foundation + header-first version parse. Fully additive — no codegen wiring (that's PR 2/3); v1 path untouched.
New module
src/codegen/manifest_v2.zigSUPPORTED_MANIFEST_VERSION = 2ManifestHeader { manifest_version = 1 }— parsed first (ignore_unknown_fields) to read the version off any manifest shape; absent ⇒ 1.BackendManifestV2— full §3 schema:id+capabilitiesretained (v2 keeps capability/identity negotiation),modules(each withroot_alias, forbackend_app), base + per-platformdep_options, per-OSsystem_libs/frameworks,platformsmatrix (entry,loop_style,Target = .native|.triple|.resolved,pic,link_libc,artifacts,extra_modules,root_build_depswith a requiredResolutionunion,Packagerecipe),build_hook(dedicatedbackend.hook.zigpath — not the provider's build.zig).DepOption { name, value }wherevalueis the closedValueSourceenum (declarative — nopre_wire/DependencyOptions; the comptime-options correction from the design review).HookContext+HOOK_ABI_VERSION = 2(types only;android_target_sdkdocumented assembler-required for Android, no silent default).Header-first parse
parseManifest: readManifestHeader→ reject<1 or >SUPPORTED(error.BackendManifestUnknownVersion) → dispatch<=1to the existing v1BackendManifest(unchanged) /2toBackendManifestV2. Returns a taggedParsedManifestwithfree. v1-passthrough preserved (the retainedbackends/sokolv1 manifest parses exactly as today).Tests
Synthetic v2 full-surface parse; v1-passthrough; header reads version off both shapes;
>SUPPORTED(99) +<1(0) rejection; malformed → parse error; hook ABI surface.Generation is byte-unchanged (only a test-block
@importinroot.zig, comptime-stripped from non-test builds).zig build test+zig buildexit 0.Ref #461, #453.
https://claude.ai/code/session_017pW3ifKf9wgxNg4viy6okw
Summary by CodeRabbit