Linked repository document
Bunny Mark original review — 2026-09-12
examples/bunny_mark/eval/reviews/2026-09-12-original.md
This document retains its design, example or evidence context. References outside this guide show their repository paths.
Assistant review of baseline 7601c55, retained before fixes. Scope: source,
pure subsystem tests and offline eval; no engine process or visual inspection.
User request: “Review the other evals as well, fix when needed.”
- B1 — blocker: the native per-node arm draws no bunnies.
project/main.gd:223–238instantiates bareBunnyobjects; the authored base isNode2D(declarations/bunny.odin:27), with no drawing operation, texture, or Sprite2D child. The arm can PASS while rendering an empty play area. Correction must use the same textured geometry and scales as the other arms, and the runtime gate must inspect actual drawable instances. - B2 — blocker: the comparison changes the workload.
The staged arm uses a different PRNG (
subsystem/bunny.odin:115–140versusproject/gd_bunnies.gd:42–55) and folds an ever-accelerating gravity parabola (subsystem/bunny.odin:216–233) instead of integrating gravity after elastic bounces. Folding gravity reverses acceleration on alternate intervals; it is not the same physical bounce claimed in README. GD also scales both velocity components (gd_bunnies.gd:75), while native scales only x (bunny.odin:143–144). The staged 14×14 quad differs from the sprite's natural dimensions. Use common seeded identities, geometry and explicit step rules; check all modes against the same replay state outside timing. - B3 — should-fix: the producer and oracle bypass simulation state.
field_write_layoutregenerates parameters and positions instead of reading the SoA (subsystem/bunny.odin:314–323), andoracle_instancecalls the same writers (333–347). Incorrect x/y updates do not affect the submitted layout or its oracle. Every frame also repeats hashing/trigonometry and trajectory calculation. Pack real retained simulation state and compare it with an independent reference integrator; include corruption rejection. - B4 — should-fix: destruction reads freed memory.
field_destroyfrees its own Field (subsystem/bunny.odin:201–209), but its comment promises safe repeated destruction and the test invokes it twice (subsystem/bunny_test.odin:280–286). The secondfield.liveread is a use after free. Clear the owning pointer and test repeated owner cleanup safely. - B5 — should-fix: setup failure and ordinary exit leak staging.
bunny_field_setup:144–145returns after population allocation failure without retiring the newly created local staging owner. Earlier failures after the retained get_multimesh result can skip release (110–121).main.gdhas no exit cleanup; only successful field eval explicitly calls shutdown (142), while deinit deliberately cannot free staging (bunny_field.odin:63–73). Cover acquisition rollback and every scene exit with symmetric cleanup. - B6 — should-fix: resize rollback and state claims are false.
bunny_field_set_count:255–279reallocates server storage before retiring the old owner. Its “resize undone” error branch does not undo server allocation, and the new population starts at frame zero without resetting submitted-frame state. Retain new owners immediately, make failures explicit, and expose consistent post-resize count/frame/verification state. - B7 — should-fix: errors and controls can leave a lying UI.
main.gd:218,236–238,265,286,289discard native error results. Switching to a newly set-up field resets its profile to canonical without applying current sliders (210–220). Returning sliders to canonical labels the old mixed history oracle-verifiable (303), although native verify returns -30. The interactive staged arm advances 1/60 per display frame while others use delta. Check calls, apply active settings on transitions, label actual verification applicability, and use a common simulation clock. - B8 — should-fix: eval automatically overwrites failed historical evidence.
test.sh:49–50compares then always writes the tracked receipt. A mismatch merely prints DIFFER and exits zero (eval/main.odin:228–235); section matching searches the entire text (238–243). Invalid flags/numbers are silently ignored or clamped (38–49), production return values are dropped (99–110), and PASS precedes comparison. Preserve the receipt, reject bad arguments and failures, assert deterministic replay and workload-matched digest equality. - B9 — should-fix: runtime campaign does not gate the claimed ownership or
visible workload.
test.sh:95suppresses import failure and119–120checks only a workload sentinel, not engine diagnostics or zero-owner Host shutdown. No timeout, one-process guard or durable per-run provenance exists. The timed bracket excludes native no-op_processdispatch and engine rendering; README calls costs flat in count (CPU or GPU), while the recorded data grows ~10×. Retain runtime logs, reject errors/leaks, measure actual frame intervals separately from driver CPU, and bound claims to the measured workload. - B10 — should-fix: unearned reload and verification claims. README's reload journey claims stored-property restoration although these authored structs declare no stored Godot properties and no reload workload runs. The field's oracle compares float equality, not the promised bitwise equality. State the actual non-reloading demo contract and execute any claimed byte-level checks.
Baseline evidence: pinned Odin test .../subsystem -o:speed completed 12 tests
successfully. Read-only odin run .../eval -o:speed -- --count=10000 --frames=1200
printed EVAL_PASS and RECEIPT_MATCH (26.910 / 327.548 µs average producer time).
These results did not establish comparative or visible correctness. An Address
Sanitizer test build failed to link ___asan_version_mismatch_check_v8; no ASAN
runtime result is claimed. Exact dev5 source archive spot-check confirmed the
eight-float 2D layout and getter mapping at the cited mesh_storage.cpp lines.
The buffer submission code extends past line 2161; the caller-consumption claim
requires following buffer_update, not inferring lifetime from its name.