feat(packs): examples/packs-demo + CI wall probes (#498 PR 6) - #551
Conversation
The epic's example ask: a two-pack game (citizens provides a component, an event, a script, and a queries.zig exposing exactly one verb; production declares depends_on and consumes it) that builds + runs headless on the null backend. The citizens script comptime-proves the Registry bridge resolves its own namespaced name to the same type its relative import reaches. CI (examples-integration) builds the positive fixture, asserts the generated shapes (pack module createModule, Registry bridge, surface narrowing), runs it for 10 null frames, then drives four negative probes that each must FAIL with the wall's message: - sibling relative import → file exists in modules … - @import("game") → no module named 'game' … - non-exposed verb → no member named 'internal_reset' (comptime decl ref — an uncalled generic fn is never analyzed, which the first probe draft learned the hard way) - foreign Registry name → Component … is not visible to this pack (the engine ComponentView message, via production's own component) Baseline is rebuilt green after the probe cycle. Closes the #498 PR train (PRs 1-6). Claude-Session: https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds a new ChangesPacks-demo end-to-end fixture
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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 the packs-demo example to demonstrate the pack module wall isolation end-to-end, featuring a providing pack (citizens) and a consuming pack (production). The feedback highlights a compilation issue in the consuming script and an error in the README: the generated surface module flattens exposed queries directly into the root namespace, meaning find_idle must be accessed directly (e.g., citizens.find_idle) rather than through a nested .queries namespace.
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.
|
|
||
| pub fn tick(game: anytype, dt: f32) void { | ||
| _ = dt; | ||
| _ = citizens.queries.find_idle(game); |
There was a problem hiding this comment.
The generated surface module (pack_surface___mod) flattens the exposed queries and commands directly into the root of the module namespace rather than nesting them under a 'queries' or 'commands' namespace (as verified by renderSurface in test/pack_scan_tests.zig). Therefore, accessing citizens.queries.find_idle will result in a compilation error. It should be accessed directly as citizens.find_idle.
_ = citizens.find_idle(game);
There was a problem hiding this comment.
Refuted by both code and CI: renderSurface nests the re-exports — it emits pub const queries = struct { pub const @"find_idle" = pack.queries.@"find_idle"; }; (the test asserts those lines INSIDE the pub const queries = struct { block, not at module root). Empirically: this very PR's Examples-integration job compiled and ran the demo calling citizens.queries.find_idle(game) — green.
| - **citizens** ships a component, an event, a per-state script, and a | ||
| verb surface (`queries.zig`) exposing exactly one query. | ||
| - **production** declares `depends_on = .{"citizens"}` and calls the | ||
| exposed query through `@import("citizens").queries.find_idle(game)`. |
There was a problem hiding this comment.
Update the README to reflect that the exposed query is accessed directly on the imported module root (e.g., @import("citizens").find_idle(game)) rather than through a nested .queries namespace, as the generated surface module flattens these declarations.
There was a problem hiding this comment.
Same false premise as the sibling comment — the surface nests under queries/commands; the README matches the shipped shape (and the CI run proves the call form compiles).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
482-494: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRestore-on-failure gap in
probe().If a probe unexpectedly fails (RC=0, or the needle isn't matched), the function
exit 1s without runninggit checkoutto restore20_consume.zigfirst. This is cosmetic given the job fails outright either way, but leaves the working tree dirty for any subsequent debugging/artifact upload steps that might run on failure.🤖 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 @.github/workflows/ci.yml around lines 482 - 494, The probe() helper in the CI workflow leaves 20_consume.zig modified on early failure because the restore checkout only happens after the success checks; move the restore step to run on every exit path, including the RC=0 and missing-needle branches, so the working tree is cleaned before any fail/exit 1. Use the existing probe() shell function and the git checkout of examples/packs-demo/packs/production/scripts/playing/20_consume.zig as the unique anchors when updating the control flow.
🤖 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 @.github/workflows/ci.yml:
- Around line 465-509: The packs-demo workflow in the CI job resets
build.zig.zon whenever $ASM generate --project-root . runs, so the fingerprint
replacement currently only happens once before the first build. Update the
probe() flow and the final baseline rebuild to reuse the same fingerprint
rewrite logic after every regenerate, specifically around the generate/build
steps in the packs-demo shell block. Keep the rewrite tied to the existing
BAD/GOOD extraction and apply it before each zig build so later probes don’t
fail on invalid fingerprint instead of the wall assertion.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 482-494: The probe() helper in the CI workflow leaves
20_consume.zig modified on early failure because the restore checkout only
happens after the success checks; move the restore step to run on every exit
path, including the RC=0 and missing-needle branches, so the working tree is
cleaned before any fail/exit 1. Use the existing probe() shell function and the
git checkout of
examples/packs-demo/packs/production/scripts/playing/20_consume.zig as the
unique anchors when updating the control flow.
🪄 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: 55490d1a-dfb7-4310-a439-906c51b084dd
📒 Files selected for processing (13)
.github/workflows/ci.ymlexamples/packs-demo/.gitignoreexamples/packs-demo/README.mdexamples/packs-demo/packs/citizens/components/counter.zigexamples/packs-demo/packs/citizens/events/counted.zigexamples/packs-demo/packs/citizens/pack.labelleexamples/packs-demo/packs/citizens/queries.zigexamples/packs-demo/packs/citizens/scripts/playing/10_count.zigexamples/packs-demo/packs/production/components/recipe.zigexamples/packs-demo/packs/production/pack.labelleexamples/packs-demo/packs/production/scripts/playing/20_consume.zigexamples/packs-demo/project.labelleexamples/packs-demo/scenes/main.jsonc
| cd .labelle/null_desktop | ||
| set +e | ||
| ERR=$(zig build 2>&1) | ||
| set -e | ||
| BAD=$(printf '%s\n' "$ERR" | grep -oE 'invalid fingerprint: 0x[0-9a-f]+' | head -1 | awk '{print $3}') | ||
| GOOD=$(printf '%s\n' "$ERR" | grep -oE 'use this value: 0x[0-9a-f]+' | head -1 | awk '{print $4}') | ||
| if [ -n "$BAD" ] && [ -n "$GOOD" ]; then | ||
| sed -i "s/${BAD}/${GOOD}/" build.zig.zon | ||
| fi | ||
| zig build | ||
| LABELLE_NULL_FRAMES=10 ./zig-out/bin/packs_demo | ||
| echo "PASS: packs-demo built + ran headless (wall positive)" | ||
| cd ../.. | ||
|
|
||
| # Negative probes: append an escape to the consuming script, | ||
| # regenerate, and require the build to FAIL with the wall's | ||
| # message. `git checkout` restores between probes. | ||
| probe() { | ||
| local snippet="$1" needle="$2" label="$3" | ||
| printf '%s\n' "$snippet" >> packs/production/scripts/playing/20_consume.zig | ||
| $ASM generate --project-root . > /dev/null | ||
| set +e | ||
| OUT=$(cd .labelle/null_desktop && zig build 2>&1) | ||
| RC=$? | ||
| set -e | ||
| if [ $RC -eq 0 ]; then echo "FAIL: probe '$label' compiled"; exit 1; fi | ||
| printf '%s\n' "$OUT" | grep -q "$needle" || { echo "FAIL: probe '$label' died without the wall message ($needle)"; printf '%s\n' "$OUT" | tail -20; exit 1; } | ||
| git -C "$GITHUB_WORKSPACE/labelle-assembler" checkout -- examples/packs-demo/packs/production/scripts/playing/20_consume.zig | ||
| echo "PASS: probe '$label' blocked" | ||
| } | ||
|
|
||
| probe 'const stolen = @import("../../../citizens/components/counter.zig"); comptime { _ = stolen; }' \ | ||
| "file exists in modules" "sibling relative import" | ||
| probe 'const shim = @import("game"); comptime { _ = shim; }' \ | ||
| "no module named .game." "game shim from a pack" | ||
| probe 'comptime { _ = @import("citizens").queries.internal_reset; }' \ | ||
| "no member named .internal_reset." "non-exposed verb" | ||
| probe 'comptime { _ = @import("pack").Registry.getType("citizens__Counter"); }' \ | ||
| "is not visible to this pack" "foreign name through the Registry bridge" | ||
|
|
||
| # Baseline still green after the probe cycle. | ||
| $ASM generate --project-root . > /dev/null | ||
| (cd .labelle/null_desktop && zig build) | ||
| echo "PASS: packs-demo wall — 1 positive + 4 blocked probes" | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "fingerprint" src/main.zig src/codegen -C3
rg -n "build.zig.zon" src/codegen -lRepository: labelle-toolkit/labelle-assembler
Length of output: 778
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/main.zig around fingerprint note =="
sed -n '260,285p' src/main.zig
echo
echo "== ci workflow around the relevant step =="
sed -n '450,515p' .github/workflows/ci.yml
echo
echo "== other fixFingerprint / generate uses in workflow =="
rg -n "fixFingerprint|generate --project-root|invalid fingerprint|use this value" .github/workflows/ci.yml -n -C2Repository: labelle-toolkit/labelle-assembler
Length of output: 10540
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate fixFingerprint and generator write path =="
rg -n "fixFingerprint|fingerprint|build\.zig\.zon|runner\.fixFingerprint|post-generate" src/main.zig src -C3
echo
echo "== main.zig around any fixFingerprint implementation =="
awk 'NR>=1 && NR<=420 {print NR ":" $0}' src/main.zig | sed -n '1,420p' | rg -n "fixFingerprint|fingerprint|build\.zig\.zon|runner\.fixFingerprint|post-generate" -C6Repository: labelle-toolkit/labelle-assembler
Length of output: 29355
Reapply the fingerprint rewrite after every regenerate
$ASM generate --project-root . resets build.zig.zon to the template fingerprint, so the zig build inside probe() and the final baseline rebuild can stop on invalid fingerprint before they ever reach the wall assertion. Call the same rewrite helper after each regenerate.
🤖 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 @.github/workflows/ci.yml around lines 465 - 509, The packs-demo workflow in
the CI job resets build.zig.zon whenever $ASM generate --project-root . runs, so
the fingerprint replacement currently only happens once before the first build.
Update the probe() flow and the final baseline rebuild to reuse the same
fingerprint rewrite logic after every regenerate, specifically around the
generate/build steps in the packs-demo shell block. Keep the rewrite tied to the
existing BAD/GOOD extraction and apply it before each zig build so later probes
don’t fail on invalid fingerprint instead of the wall assertion.
There was a problem hiding this comment.
Refuted by the fingerprint derivation: generateBuildZigZon computes a DETERMINISTIC value — high half = Crc32 of the fixed zon package name, low half = Wyhash of cfg.name (build_zig_zon.zig:115-135, 'regenerating yields a stable value per project'). Every regenerate rewrites the same valid fingerprint, so no re-dance is needed. Empirically: this PR's CI ran generate→build five times (baseline + 4 probes + final baseline) and passed; the one-time dance at the top matches the other example steps' belt-and-suspenders.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -g '*build_zig_zon.zig*'Length of output: 212
🏁 Script executed:
#!/bin/bash
sed -n '1,160p' src/build_files/build_zig_zon.zigLength of output: 9063
@apotema, thanks for the detailed rebuttal — verified against build_zig_zon.zig (Lines ~115-135): the fingerprint is indeed (Crc32.hash("generated_game") << 32) | fork_id, with fork_id a deterministic Wyhash of cfg.name. Since neither input changes across generate calls for the same project, every regenerate reproduces the identical fingerprint, so the one-time fix at the top is sufficient and the re-dance I suggested isn't needed. Combined with the empirical 5x generate→build pass, I'm withdrawing this concern.
1 participant
PR 6 — the last of the #498 train: the example + e2e fixture the epic asked for.
examples/packs-demo
Two light packs on the headless null backend (flows-smoke layout): citizens ships a component, an event, a per-state script, and a
queries.zigexposing exactly one verb; production declaresdepends_on = .{"citizens"}and consumes@import("citizens").queries.find_idle(game). The citizens script comptime-proves the Registry bridge (@import("pack").Registry.getType("citizens__Counter") == Counter). The scene referencescitizens__Counterby its namespaced key — scenes are data, documented as outside the wall.CI: 1 positive + 4 blocked probes
The examples-integration job builds the fixture, asserts the generated shapes (
pack__citizens_mod, the Registry bridge, the narrowed surface), runs it for 10 null frames, then appends four escapes to the consuming script — each must FAIL with the wall's message, restoring between probes:file exists in modules …@import("game")no module named 'game' …no member named 'internal_reset'@import("pack").RegistryComponent … is not visible to this pack(the engineComponentViewmessage, routed through production's own component)Baseline is rebuilt green after the cycle, so a probe can't leave the example broken.
All five commands + probes were dry-run locally against the exact CI recipe (including the fingerprint dance and
LABELLE_NULL_FRAMES=10execution). One probe lesson encoded in the step: an uncalled generic fn is never analyzed by Zig, so the non-exposed-verb probe uses a comptime decl reference.With this, every item in #498's plan is landed: PR 1
PackViewemission → PR 2 module graph → PR 3 Registry bridge → PR 4 exposes/depends_on → PR 5 lint demotion/truth-up → PR 6 example + e2e. (The >1000-line splits the plan bundled into PR 5 landed independently as #539–#549.)Closes #498
https://claude.ai/code/session_01P7YLw4hXFCCaY2LAUt4G1j
Summary by CodeRabbit
New Features
Tests
Documentation