RFC: Pluggable backends — make the assembler backend-agnostic - #378
Conversation
PR SummaryLow Risk Overview The RFC argues backends should work like plugins (name/repo + manifest) instead of the closed Codegen is reframed from text-splicing Also covers incremental migration (ABI → audio pilot → sokol → open resolver), per-layer impact (gfx, assembler, engine, extracted Reviewed by Cursor Bugbot for commit 7f60ca9. Bugbot is set up for automated code reviews on this repo. Configure here. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an RFC proposing to make the assembler backend-agnostic by decoupling backends into separate packages and establishing a versioned runtime contract (ABI). The review feedback recommends aligning terminology with the Zig ecosystem by replacing 'crate' with 'package' or 'module', clarifying allocator ownership within the deinit() lifecycle contract, and ensuring Android target detection in build scripts is compatible with Zig 0.16.0.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ### NEW — `labelle-platform-abi` (a thin leaf crate) | ||
| Houses the four comptime contracts + the shared value types; almost no deps. | ||
| gfx, engine, and every backend depend on it. A backend author opens exactly one | ||
| crate to see "here's everything I must implement, here's the version I pin." |
There was a problem hiding this comment.
In Zig, the standard terminology is "package" or "module" rather than "crate" (which is Rust-specific). To maintain consistency with the Zig ecosystem and the rest of the codebase, it is recommended to replace "crate" with "package" or "module" throughout the RFC.
| ### NEW — `labelle-platform-abi` (a thin leaf crate) | |
| Houses the four comptime contracts + the shared value types; almost no deps. | |
| gfx, engine, and every backend depend on it. A backend author opens exactly one | |
| crate to see "here's everything I must implement, here's the version I pin." | |
| ### NEW — `labelle-platform-abi` (a thin leaf package) | |
| Houses the four comptime contracts + the shared value types; almost no deps. | |
| gfx, engine, and every backend depend on it. A backend author opens exactly one | |
| package to see "here's everything I must implement, here's the version I pin." |
| 2. **Where the contracts live + versioning** — gfx owns `render`; does it | ||
| relocate into the ABI crate, or stay in gfx and be re-exported? How does a |
There was a problem hiding this comment.
Replace the Rust-specific term "crate" with "package" or "module" to align with Zig's terminology.
| 2. **Where the contracts live + versioning** — gfx owns `render`; does it | |
| relocate into the ABI crate, or stay in gfx and be re-exported? How does a | |
| 2. **Where the contracts live + versioning** — gfx owns `render`; does it | |
| relocate into the ABI package, or stay in gfx and be re-exported? How does a |
| 4. **window** — the inversion-of-control crux. The window **owns the run loop** | ||
| and the per-frame render target: | ||
| ```zig | ||
| // required: init(cfg) → deinit() → shouldQuit() → beginFrame() *Target → endFrame() |
There was a problem hiding this comment.
When defining the deinit() contract, consider clarifying the allocator ownership model. In Zig (especially with 0.16), unmanaged data structures or certain allocation patterns require passing the allocator to deinit(allocator). Specifying whether the allocator is stored during init(cfg) or passed explicitly to lifecycle methods like deinit will prevent API mismatches for backend authors.
| 4. **Mobile/android + gamepad** (`android_gamepad`, `sdl_gamepad`, the | ||
| `templates/mobile.txt` path) — extra contract surface beyond the desktop four. |
There was a problem hiding this comment.
When extending the contract surface for Android/mobile support, keep in mind that in Zig 0.16.0, target.result.isAndroid() does not exist. Any platform-specific checks in the build scripts or backend code should use an explicit ABI check comparing target.result.abi against .android and .androideabi to properly support both 64-bit and 32-bit Android targets.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e55754fd-b771-4964-a10b-5e8add365501) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_3311b0c7-23c7-4431-b0f8-bf199a92795c) |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_e06c7106-5cdc-495f-84dd-e4e118252a49) |
|
Review findings:
The RFC defines render as draw/loadTexture and audio as play/loadSound slots, but current generated code also depends on backend module contracts for asset streaming:
The examples say
Raw backend GUIs are currently resolved by the closed backend enum, not by render/window/input slots. See
Zig lazy dependencies can avoid fetching unused transitive native deps, but only if those deps are not vendored into the fetched package and are modeled lazily at the right package boundary. The RFC states the result as guaranteed even though the current repo layout contains multiple backend subpackages plus shared in-tree packages. This should be reframed as a packaging requirement: official providers must keep heavyweight native deps as lazy external deps, and the root package must not reference unused provider deps during build graph construction. Verification: |
apotema
left a comment
apotema
left a comment
There was a problem hiding this comment.
RFC review (revision 4)
Solid RFC — the problem statement is accurate against the current tree, the incremental migration is credible, and revision 4’s per-contract composition + render-anchored cascade is the right framing. This is ready to guide implementation planning; a few gaps are worth closing before calling it “accepted.”
What checks out
- Problem diagnosis —
src/config.zig:50closed enum, ~4 GB bundled native deps (sokol 2.0G / bgfx 812M / wgpu 555M / raylib 335M on disk), and the plugin asymmetry (cache.resolvePluginvs enum switches) all match reality. - Audio as proof of independent contracts — bgfx
audio.zig(~790 LOC) + wgpuaudio.zig(~296 LOC) vs backends that already delegate to a device lib is exactly the duplication the RFC targets. Audio-first extraction (step 2) is the lowest-risk, highest-payoff starting point. - Codegen splice reframing (rev 2) — comparing
backends/sokol/templates/desktop.txt(callbackinit/frameexports) vsbackends/raylib/templates/desktop.txt(pub fn main+whileloop) makes the “text merge is impossible” argument concrete. TheGame-lifecycle hook model is the right successor. - Context stays package-private — keeping GPU context out of the cross-backend contract avoids the
*anyopaquetrap; POC v3 on #377 backs this up. - Monorepo + lazy deps (rev 3) — resolving open Q#3 this way is pragmatic; contract granularity ≠ repo count is an important distinction.
Suggestions before acceptance
-
Terminology — still uses “crate” in four places (lines 174, 177, 202, 265). Zig ecosystem convention is package; align with the rest of the toolkit docs.
-
AudioInterfacehome — RFC says labelle-core (correct:labelle-core/src/audio.zig), but every backend comment says “engineAudioInterface(Impl)”. Call out explicitly that engine re-exports core’s contract and that the ABI crate will be the single canonical import — this is the core-diamond story in miniature. -
Cascade rules need a spec sketch — the render→window→input defaulting is principled, but authors will need:
- where defaults are declared (per render provider manifest? a resolver table in assembler?)
- what happens on incompatible overrides (e.g.
.render = .bgfx+.window = .sokol_app) - platform-conditioned defaults (bgfx desktop ⇒ GLFW, bgfx Android ⇒ custom
android_apppath — not GLFW)
Even a short “resolver pseudocode” block would de-risk step 4.
-
project.labelletype shape — the per-contract struct examples are clear ergonomically, but worth one paragraph on how shorthand enum tags (.sokol) and full repo refs (.{ .repo = "github:…" }) unify in the config parser — this is the plugin-model mirror. -
Game-lifecycle ABI (open Q#1) — the sokol template already carries more than
frame:sokolEventforwarding, screenshot state, GUI event hooks. When pinning the hook surface, inventory whattemplates/{desktop,mobile}.txtinject today so nothing regresses (especially suspend/resume on mobile and context-loss on Android). -
Gamepad backends —
android_gamepad/sdl_gamepadlook like input extensions composed alongside the window provider rather than a fifth top-level contract. Worth stating explicitly so open Q#4 doesn’t sprawl the ABI. -
Stale wording in “The backends” — §“The backends” still says “one
labelle-backendsmonorepo, or per-backend repos” but open Q#3 is struck through as resolved. Tighten to match. -
Build splice (open Q#5) — the hardest remaining work. A minimal manifest sketch (what a provider ships:
build.zighook?zonfragment? lazy native dep list?) would help parallelize design from the runtime ABI work.
Verdict
Approve direction; keep as Draft until Q#1 (Game-lifecycle ABI) and Q#5 (build-graph manifest) get at least outline answers. The runtime contract + per-contract composition story is convincing; the POC on #377 credibly de-risks the window/render/context side. Audio extraction as step 2 is the right “prove the rails” move before touching sokol’s entry-point shape.
Nice iterative revisions — rev 2’s codegen insight and rev 4’s cascade are the two biggest leaps.
apotema
left a comment
apotema
left a comment
There was a problem hiding this comment.
Inline follow-ups on the review above.
|
|
||
| ## Per-layer changes | ||
|
|
||
| ### NEW — `labelle-platform-abi` (a thin leaf crate) |
There was a problem hiding this comment.
Nit: replace "crate" with "package" throughout (Zig terminology). Still present at 174/177/202/265.
|
|
||
| (The bare `.backend = .sokol` is accepted only as sugar for `.{ .render = .sokol }`.) | ||
|
|
||
| **`render` is the anchor and the other slots cascade**, because they aren't |
There was a problem hiding this comment.
Consider adding a short resolver spec: where defaults live, error on incompatible overrides, and platform-conditioned window defaults (bgfx Android ≠ GLFW).
| - The window / input / audio contracts (today implicit in the engine + the | ||
| generated game) get formalized and given a versioned home in the ABI crate. | ||
|
|
||
| ### The backends |
There was a problem hiding this comment.
Open Q#3 resolves to one monorepo, but this section still offers "or per-backend repos" — suggest aligning.
|
Addressed in revision 5 (7f60ca9). This was sharp review — thanks. Mapping: Findings
Suggestions — Kept as Draft per your verdict — Q#1 (lifecycle ABI) and Q#5 (build-graph/manifest) still need outline answers before "accepted." |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_d639007b-1211-4e7b-ba17-ef2d713cf154) |
There was a problem hiding this comment.
Pull request overview
Draft RFC proposing a design to make labelle-assembler backend-agnostic by treating backends as resolved-by-name packages implementing versioned comptime contracts (render/window/input/audio), plus a migration plan and open questions.
Changes:
- Adds
RFC-PLUGGABLE-BACKENDS.mddescribing the “backends as plugins” model and its ABI/contracts. - Defines the proposed contract decomposition and platform-qualified defaulting/cascades.
- Documents migration steps and remaining design unknowns (build-graph splice, lifecycle ABI, GUI bridge compatibility).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **(backend × platform) matrix** — the most platform-specific, most-hardcoded | ||
| thing in the assembler today: every backend ships | ||
| `templates/{desktop,mobile,android,wasm,headless}.txt`, plus the APK packaging | ||
| (`package_apk.sh`, generated `AndroidManifest.xml`, the NDK build) and the | ||
| wasm/emscripten shell. A `Platform` enum (`desktop`/`android`/`ios`/`wasm`) and | ||
| per-platform asset compression already exist. |
| - **Emit a context-free skeleton** (the `main()` the POC's `run` models) and | ||
| **splice** the backend's manifest-provided run-loop / build fragments *blind*. | ||
| (The codegen-splice — the hard part, see below.) |
Review — RFC mismatches its descriptionThe PR is described (and bot-reviewed) as docs-only, but the diff touches 5 non-doc files with substantive code deletions that appear to be leftover WIP from the audio-extraction pilot (RFC migration step 2), not RFC scaffolding. Mismatched code changesThe diff deletes ~145 LOC from
RFC content (the docs)Revision 5 is solid and the rev-4 review findings are well-addressed: the full asset-loader surface on render/audio, the platform-qualified cascade VerdictThe RFC doc is ready to merge as Draft. The bundled code deletions are not — they look like in-progress audio-extraction work that shouldn't ship inside an RFC PR. Recommend:
|
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis RFC documents a backend-agnostic assembler design with pluggable providers, manifest-driven wiring, versioned core contracts, and named GUI/input integration points. It also records remaining migration questions. ChangesPluggable backend RFC
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
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.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@RFC-PLUGGABLE-BACKENDS.md`:
- Around line 198-202: The migration plan uses two different ABI package names,
which makes the ownership model contradictory. In the RFC section that
introduces the thin leaf package and in the later status/goals wording, collapse
everything to a single ABI home by renaming the `labelle-platform-abi`
references to match the chosen package name used elsewhere (the `labelle-core`
path, if that is the intended home) and keep the surrounding text consistent
across the plan, goals, and dependency descriptions.
- Around line 1315-1343: The build hook contract is too late in the flow to
handle dynamic dependency setup like with_imgui, NDK sysroot selection, and
emccStep wiring. Move the hook integration earlier in the assembler flow so
provider logic can influence dependency construction before generic
module/artifact/system-lib wiring, and update the build.zig HookContext/wire()
contract to reflect that it can participate in graph setup rather than only
supplementing it. Use the existing wire(), HookContext, and build_hook sections
to relocate the hook to the point where backend-specific build graph decisions
are made.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b42d0173-11cd-4c14-9e17-3d498bbd169b
📒 Files selected for processing (1)
RFC-PLUGGABLE-BACKENDS.md
Two pre-Accept items — both localized, neither threatens the architectureRev 10 is in great shape — grounding every answer in file:line inventories of shipped code is exactly right. Two findings I'd want resolved before flipping Draft → Accepted. 1. The validation plan doesn't cover the residual it's staked on (Q#1)The RFC gates Accept-readiness on the audio-extraction pilot "validating the context-handoff story," and Suggestion: either reframe the pilot's claim ("validates extraction mechanics, not GPU context loss"), or add a second pilot on a GPU backend that actually cycles the surface. 2. The
|
Revision 11 — addresses all four findingsPushed in 1. Collapse 2. Split the build hook into 3. Reframe the Accept gate to the bgfx-Android pilot (apotema finding #1). The audio-extraction pilot validates extraction mechanics (contract home, versioning, build-graph wiring, lazy deps) but has zero GPU context — it cannot exercise 4. Fix the The |
Review — RFC: Pluggable backendsType: Documentation-only. 1 file, +1493/-0 ( VerdictMergeable as a Draft RFC, but I'd request changes on the doc-correctness issues below before flipping to Accepted. The design is high-quality, exceptionally well-grounded, and its hard problems (context handoff, Verified findings (checked against the code)High — Medium — versioning prose contradicts its own code sample. The generated check uses strict equality ( Medium — pilot/migration step is internally inconsistent. Migration step 3 says convert sokol (RFC:321); Q#1 calls the GPU-context Accept gate bgfx-Android while labeling it "step 3 — sokol conversion" (RFC:470). Sokol (the headless-screenshot-verifiable desktop backend) cannot exercise the Low — stale line count. Low — stale PR description. The PR body still references What's accurate (and it's a lot)Spot-checks held up exactly: Recommendation
No blockers for merging the document as Draft; the four content fixes should land before it's promoted to Accepted. |
Five findings from the rev-11 review: 1. High — audio-contract conflation. The RFC attributed the loader surface (decodeAudio/uploadSound/unloadSound(Sound)) to core's AudioInterface. It isn't there: core only contracts runtime playback (playSound/stopSound, optional loadSound(id)/music). The loader is a separate Backend(Impl) in labelle-engine/audio_backend. Split both the contract inventory (audio bullet) and the gfx Backend(Impl) section so the "already contracted" claim is scoped to playback only. 2. Medium — versioning summary contradicted its code sample. Changed the prose "asserts N <= M" to "N == M"; the generated check @compileerrors on both N > M and N < M (N <= M would permit the dominant old-backend-vs-new-core failure the t < p branch rejects). 3. Medium — pilot/migration inconsistency. Q#1 called the GPU-context Accept gate "step 3 — sokol conversion", but sokol-desktop can't exercise TERM_WINDOW/INIT_WINDOW. Split into step 3 (sokol-desktop, extraction mechanics) and a distinct step 4 (bgfx-Android GPU-context gate); renumbered resolver/extract to 5/6 and fixed all cross-refs. 4. Low — refreshed the build_zig.txt line count to 1142. 5. Low — last "crate" → "package"; PR description refreshed separately (labelle-core, six answered questions, bgfx-Android gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…377) Draft RFC synthesizing the design discussion + the runnable POC: promote backends from a closed enum to the open plugin model, behind four versioned comptime contracts (render/input/audio/window) in a new labelle-platform-abi crate, with the GPU context kept package-private at comptime. Captures the per-layer changes, the gfx impact, the codegen-splice as the core remaining work, an incremental migration, and the open questions.
…cycle hook contract POC'd open question #1 (#377): the run-loop splice isn't a text merge (which can't even span desktop-loop vs mobile-callback entry shapes) — the assembler emits a backend-blind Game (init/frame/deinit hooks); the backend owns its entry point and drives them, composing at the module level. The code splice is largely answered; the residuals are the build-splice (#5) and the full lifecycle ABI (input/resize/suspend-resume + context-loss, the new #1).
…worked example Correct the 'one package = four contracts' framing: the four contracts are independently pluggable (audio is the proof — AudioInterface is already in labelle-core, yet bgfx+wgpu each reimplement a WAV mixer because their render lib has no audio). A 'backend' is a composition of per-contract providers, declared as full-stack packages with per-contract overrides. Audio decomposes into a shared labelle-audio mixer + pluggable device sinks (sokol_audio/ miniaudio/...) and is the ideal first extraction (already contracted, zero context-sharing). Packaging: one labelle-backends monorepo + Zig lazy deps (contract granularity ≠ repo count); resolves the monorepo open question. Adds the audio pilot to the migration plan.
…anchored cascade
Make the per-contract struct the canonical declaration (self-documenting) and
demote bare `.backend = .sokol` to sugar for `.{ .render = .sokol }`. Document
the cascade: render is the anchor (render needs a compatible window → window
default), window→input (input ships with the window lib), audio independent
(sokol→sokol_audio else miniaudio). You pin render + any slot to override; the
defaulting is principled (render⇄window coupling), not a magic table.
…ackaging/manifest Folds in the review findings: - HIGH: render/audio contracts are thicker than draw/play — spell out the asset-loader surface (decodeImage/uploadTexture/unloadTexture/compressed/font, decodeAudio/uploadSound/unloadSound) so a backend can't conform-but-fail. - HIGH: the cascade is platform-qualified — resolver is (platform, render) -> window/input/audio (bgfx Android != GLFW); incompatible overrides are errors. - MED: lazy-deps reframed from a guarantee to a packaging requirement. - MED: new GUI-bridge open question (imgui bridges vs per-contract providers). - New 'Platform packaging & the manifest' section: the (backend x platform) matrix, the manifest schema sketch, window-entry vs shared platform-packager. - Suggestions: crate->package, AudioInterface home, gamepad-as-input-extension, lifecycle-ABI inventory note, stale 'monorepo or per-backend' wording fixed.
Answers all six open questions, grounded in inventories of the shipped codebase. Each answer cites the specific files + line counts it is verified against. Q#1 — the Game-lifecycle ABI (rev 6): the full hook surface is init/deinit/frame(dt)/running/event/suspend_/resume_/contextLost (last four @hasDecl/null-gated). Grounded in an inventory of all seven shipped templates. Per-frame work (screenshot, preview, GUI, setScreenSize) stays codegen, not lifecycle. Residual: contextLost semantics + the Event type shape — both gated on the audio-extraction pilot. Q#2 — where the contracts live + versioning (rev 8): the ABI package IS labelle-core (7 of 8 contracts already live there). Backend(Impl) + its value types relocate from gfx to core. Versioning: a contract_version integer on each contract + a targets_<contract>_version on each backend, asserted at comptime. Q#3 — monorepo (rev 3, resolved): one labelle-backends monorepo + Zig lazy deps per provider. Q#4 — gamepad as an input-extension (rev 9): gamepad is NOT a fifth contract. The three existing sources are packages composed alongside the input provider. The manifest input_extensions field replaces deps_linker.zig staging switches. Q#5 — the build-graph manifest (rev 7): the manifest build-side schema (.modules/.artifacts/.system_libs/.frameworks/.platforms/.build_hook), the core-diamond generalization (8 hand-coded sites to 1 generic walk), the build-hook escape hatch, and lazy native deps as a zon-level requirement. Q#6 — GUI-bridge compatibility (rev 10): bridges keyed by render provider name (not a closed enum). Two integration patterns: external C++ bridge (default) and in-backend adapter (via provider manifest build_options). The with_imgui/gui_enabled flags are replaced by manifest-declared options. render_interface GUIs are unaffected. All residuals are migration-gated, not design-blockers. The RFC is ready to move from Draft to Accepted pending the audio-extraction pilot validating the context-handoff story.
Four fixes from the rev-10 review: 1. Collapse labelle-platform-abi to labelle-core everywhere (CodeRabbit). The migration plan and per-layer-changes section used the old name even after Q#2 established that the ABI package IS labelle-core. Renamed all remaining references; the only historical mention is in Q#2 where the rename is explained. 2. Split the build hook into pre_wire/post_wire (CodeRabbit + apotema). The single wire()-after-generic-wiring contract was known-insufficient: sokol with_imgui is a shipped consumer that must set b.dependency options BEFORE the artifact is built. pre_wire returns DependencyOptions the assembler passes to b.dependency; post_wire supplements the graph after generic wiring (NDK sysroot, emcc shell-out, extra links). Removed the hook-ordering residual from Q#5 open-questions — it is now answered. 3. Reframe the Accept gate to the bgfx-Android pilot (apotema). The audio-extraction pilot (step 2) validates extraction mechanics (contract home, versioning, build-graph wiring, lazy deps) but has ZERO GPU context — it cannot exercise contextLost or the TERM_WINDOW+INIT_WINDOW surface-recreation cycle. The Accept gate is now on the bgfx-Android pilot (step 3), which already has the init_done one-shot guard for exactly this cycle. 4. Fix the contract_version check direction (apotema). The rev-8 check (targets > provided) only caught old-core+new-backend (rare). The dominant ecosystem failure is new-core+old-backend (targets < provided) — which fell through to the raw @CompileError("Backend must define 'foo'") the validator emits, defeating the purpose of versioning. Now gates on strict equality with direction-branched diagnostics: t > p = "upgrade core", t < p = "upgrade backend". Updated the third-party-pinning residual to match.
Five findings from the rev-11 review: 1. High — audio-contract conflation. The RFC attributed the loader surface (decodeAudio/uploadSound/unloadSound(Sound)) to core's AudioInterface. It isn't there: core only contracts runtime playback (playSound/stopSound, optional loadSound(id)/music). The loader is a separate Backend(Impl) in labelle-engine/audio_backend. Split both the contract inventory (audio bullet) and the gfx Backend(Impl) section so the "already contracted" claim is scoped to playback only. 2. Medium — versioning summary contradicted its code sample. Changed the prose "asserts N <= M" to "N == M"; the generated check @compileerrors on both N > M and N < M (N <= M would permit the dominant old-backend-vs-new-core failure the t < p branch rejects). 3. Medium — pilot/migration inconsistency. Q#1 called the GPU-context Accept gate "step 3 — sokol conversion", but sokol-desktop can't exercise TERM_WINDOW/INIT_WINDOW. Split into step 3 (sokol-desktop, extraction mechanics) and a distinct step 4 (bgfx-Android GPU-context gate); renumbered resolver/extract to 5/6 and fixed all cross-refs. 4. Low — refreshed the build_zig.txt line count to 1142. 5. Low — last "crate" → "package"; PR description refreshed separately (labelle-core, six answered questions, bgfx-Android gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
a9b6c02 to
c8b0c01
Compare
Seven design points from the rev-12 review: 1. Separate runtime vs asset-loader contracts. Audio was split in rev 12; this names render's two sub-surfaces — draw API (drawTriangle/...) vs asset-streaming/loader (decodeImage/uploadTexture/font decls) — making the split symmetric with audio. 2. Conformance suites per contract. New "Opening the ecosystem" section: each contract ships a shared conformance suite in labelle-core (next to mock_backend), parameterized over the provider Impl, checking behavior (round-trips, event mapping, surface-loss state preservation) — not just @hasDecl shape. A provider is conformant iff it passes the suite for every capability it advertises. 3. Explicit lifecycle. Replaced the single under-specified contextLost with surfaceLost/surfaceRestored, with the engine responding via gpuResourcesInvalidated -> reuploadAssets instead of overloading deinit/init for mobile surface recreation. Only the re-upload granularity stays pilot-gated (bgfx-Android, step 4). 4. Constrain build hooks. Manifest now ~95%, hook ~5%; HookContext/ DependencyOptions are versioned types, hook may read only documented ctx fields and construct build-graph nodes only — no arbitrary FS/ network/shell-out. Extend the manifest, not the hook. 5. Provider identity & collisions. Canonical <namespace>.<name> IDs; labelle.* reserved for the official monorepo and is what the enum shorthands resolve to; collision (or a third party claiming labelle.*) is a hard resolve-time error. The ID is the stable key for GUI bridges and capabilities. 6. Capability negotiation. Providers declare a .capabilities set; the assembler checks project-required capabilities (explicit .requires + derived from platform/target/GUI) before emitting the build graph, erroring with a project-level message instead of a deep @CompileError. 7. Migration pilots — already addressed in rev 12 (audio = extraction mechanics, sokol-desktop = full-stack, bgfx-Android = lifecycle/context gate); status/residual wording updated to match the new hook names. Added open-questions 7-9 (identity, capabilities, conformance) and bumped the status header to revision 13. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The "Audio — the worked example" section (and migration step 2) still described labelle-audio as "the AudioInterface impl (decode + mix)" and called audio "already contracted (core.AudioInterface)", conflating the two surfaces rev 12 split. Corrected: playback/mix IS AudioInterface (core); decode/upload is the separate audio-loader contract (Backend(Impl) in labelle-engine/audio_backend). "Already contracted" is now scoped to the playback half, with the loader half noted as having a ready home. PR description updated separately (revision 11 → 13; contextLost → surfaceLost/surfaceRestored; rev 12-13 additions summarized). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sakana reviewVerdict: Excellent RFC. Keep it Draft until the bgfx-Android surface-loss pilot validates the lifecycle story. Revision 13 addresses the major prior design concerns:
FindingsLow — build-hook constraints are policy, not enforcement, unless implemented. The RFC says hooks have no arbitrary filesystem/network/shell-out access outside documented surfaces. That is the right policy, but a Zig Low — historical These are understandable as historical notes, but they make the current model slightly harder to scan. Not blocking. RecommendationMerge/keep as Draft RFC. Promote to Accepted only after the bgfx-Android pilot proves — Sakana |
Phases 1 & 2 shipped (labelle-core #45 render contract; labelle-audio v0.3.0 shared Mixer(Sink)+DeviceSink i16+f32; bgfx/wgpu/sokol collapsed). Mark them DONE in the migration plan; correct the audio worked-example (raudio/sdl_audio are monolithic engines, NOT shared-mixer device sinks — raylib/sdl delegate decode+ mix and didn't collapse); add the composable-vs-monolithic provider distinction to the resolver (Phase 5); flag the WAV-shared/OGG-backend decode split + the writeAudioBackendWiring codegen as Phase-6 targets; record that audio proved the provider-composition mechanic + no-codegen-change for a module dep but NOT the run-loop splice (Phase 3 remains the gating crux).
A throwaway manifest-driven generation path for sokol-desktop produced byte-identical main.zig+build.zig with no =>.sokol branch in the splice logic, builds + runs (headless screenshot, negative-control verified). Verdict: the build splice is VIABLE; externalizing the embedded build_zig.txt sections was the easy part. Refines the model to 'manifest declarations + fixed assembler-computed params + capability-flag-keyed lifecycle block library' (NOT pure-data) and names the three things that stay code + the name->package registry that is the Phase-5 pluggability seam.
…ated) The surfaceLost/surfaceRestored GPU-context-loss gate — the one validation CI structurally can't run — passed on real hardware (Tab A7 / Adreno 610 / Android 12): 9 surface destroy/recreate cycles, hooks fire in order, 0 crashes, assets re-upload, GPU memory plateaus. That was the last gating residual, so the RFC moves Draft→Accepted. Remaining work is Phase-6 implementation, not design.
Draft RFC for #377 (revision 13). Synthesizes the design discussion + the runnable POC (window/render/context contracts) into a concrete proposal.
Thesis: backends are a closed enum (
config.zig:50) while plugins are open (resolved-by-name, manifest-driven). Close that gap — a backend becomes a resolved-by-name package implementing versioned comptime contracts (render = gfx's existingBackend, plus input/audio/window). The ABI home islabelle-coreitself (7 of 8 contracts already live there; the 8th, gfx'sBackend(Impl), relocates from gfx to core) — not a newlabelle-platform-abipackage, which earlier revisions proposed before the inventory showed core already holds the surface. The GPU context stays package-private at comptime (never in the contract — avoids*anyopaque).Covers: the runtime contract (POC-validated), the backend-as-package model, per-layer changes (core / gfx / assembler / engine / extracted backends), the codegen-splice as the core remaining work, the
pre_wire/post_wirebuild-hook escape hatch (constrained to a versioned, documentedHookContext; manifest ~95%, hook ~5%) for dynamic build-graph cases (e.g. sokol'swith_imgui, NDK/emcc setup), and an incremental migration (audio pilot → sokol-desktop conversion → bgfx-Android GPU-context pilot → open the resolver → extract the rest), keeping the enum as a resolver shorthand.Rev 12–13 add: the runtime-vs-loader split for both audio (playback
AudioInterfacevs thedecodeAudio/uploadSoundloader contract inlabelle-engine/audio_backend) and render (draw API vs asset-streaming sub-surface); an explicitsurfaceLost/surfaceRestoredlifecycle pair (engine responds viagpuResourcesInvalidated→reuploadAssets, nodeinit/initoverload); and an Opening the ecosystem section — provider identity (canonical<namespace>.<name>IDs,labelle.*reserved), capability negotiation (declared.capabilitieschecked at resolve time → early errors), and per-contract conformance suites (behavior, not just@hasDeclshape).All six open questions are answered (Q#1 lifecycle ABI, Q#2 contract home + versioning, Q#3 monorepo, Q#4 gamepad-as-input-extension, Q#5 build-graph manifest, Q#6 GUI-bridge compatibility), plus three ecosystem concerns added in rev 13 (provider identity, capability negotiation, conformance suites). Residuals are migration-gated, not design-blockers. The Accept-readiness gate is the bgfx-Android pilot (migration step 4) validating the
surfaceLost/surfaceRestoredre-upload story — the only pilot with a GPU context that can exercise theTERM_WINDOW+INIT_WINDOWcycle.Rendered at
RFC-PLUGGABLE-BACKENDS.md. Posting as a PR for review — not implementing. Stays Draft until the bgfx-Android pilot lands.Summary by CodeRabbit