Repository navigation
fix(zig): use the Io awake clock in the bench harness (Zig 0.16) - #354
Merged
Merged
Conversation
Zig 0.16's std.Io.Clock has real, awake and boot; .monotonic does not exist, so zig build failed with 13 errors in src/bench.zig and the E2E and Bench jobs on main were red. .awake is the monotonic clock that excludes suspend time, which is what a latency benchmark wants. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019j8She9eTFx54r6aL6sCHP Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (17)
|
| Layer / File(s) | Summary |
|---|---|
Benchmark clock selection ffi/zig/src/bench.zig |
The comment and both timestamp calls now use .awake instead of .monotonic. |
Priority: ⬇️ Low
Estimated code review effort: 2 (Simple) | ~5 minutes
Change: Bug fix
Merge Risk: ⚪ Minimal · up to 74e83
The benchmark timing change is limited to the intended clock selection; no merge-blocking risk is evident.
Architecture Summary
Architecture risk: 🔵 Low · up to 74e83
The change affects 1 system.
Changed systems: ffi
Architecture concerns
No architecture-level concerns identified.
Review details
Systems and components
- observed — ffi (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
- observed — Modified behavior in ffi/zig/src/bench.zig: The clock comment now identifies
.awakeas the replacement, and both timestamps use.awakeinstead of.monotonic.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and concisely states the Zig benchmark clock change and the Zig 0.16 context. |
| Description check | ✅ Passed | The description covers the purpose, changes, testing results and applicable checklist items. The omitted screenshots section is not applicable to this code change. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
- Commit to this branch
- Create a new PR
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks the clock at dawn
The awake ticks carry timing on
Warmups hop along unchanged
The measured loops keep their range
Then off the rabbit bounds to play
Comment @coderabbitai help to get the list of available commands.
🏁 path-claims benchCommit NumbersHost-dependent — compare deltas across commits, not absolute values. |
🔍 Hypatia Security ScanFindings: 113 issues detected
View findings[
{
"reason": "Job `sonarqube` in build.yml has no `timeout-minutes:` declaration. Default is 6 hours — a stuck codeload fetch or runner hang can burn budget. Add `timeout-minutes: 10` (or proportional).",
"type": "missing_timeout_minutes",
"file": ".github/workflows/build.yml",
"action": "flag",
"rule_module": "workflow_audit",
"severity": "medium",
"recipe_id": "recipe-add-workflow-timeout-minutes",
"job": "sonarqube"
},
{
"reason": "Job `triage` in label-triage.yml has no `timeout-minutes:` declaration. Default is 6 hours — a stuck codeload fetch or runner hang can burn budget. Add `timeout-minutes: 10` (or proportional).",
"type": "missing_timeout_minutes",
"file": ".github/workflows/label-triage.yml",
"action": "flag",
"rule_module": "workflow_audit",
"severity": "medium",
"recipe_id": "recipe-add-workflow-timeout-minutes",
"job": "triage"
},
{
"reason": "Job `sync` in labels.yml has no `timeout-minutes:` declaration. Default is 6 hours — a stuck codeload fetch or runner hang can burn budget. Add `timeout-minutes: 10` (or proportional).",
"type": "missing_timeout_minutes",
"file": ".github/workflows/labels.yml",
"action": "flag",
"rule_module": "workflow_audit",
"severity": "medium",
"recipe_id": "recipe-add-workflow-timeout-minutes",
"job": "sync"
},
{
"reason": "Job `deploy` in pages-deploy.yml has no `timeout-minutes:` declaration. Default is 6 hours — a stuck codeload fetch or runner hang can burn budget. Add `timeout-minutes: 10` (or proportional).",
"type": "missing_timeout_minutes",
"file": ".github/workflows/pages-deploy.yml",
"action": "flag",
"rule_module": "workflow_audit",
"severity": "medium",
"recipe_id": "recipe-add-workflow-timeout-minutes",
"job": "deploy"
},
{
"reason": "Step uses `peter-evans/repository-dispatch` with `token: ${{ secrets.FARM_DISPATCH_TOKEN }}` but has no `if: secrets.FARM_DISPATCH_TOKEN != ''` gate. On repos where the secret hasn't been propagated the action fails on every push, red-maining the repo. Add the step-level gate (or env+if pattern) so the missing-secret path is a clean skip instead of a red.",
"type": "secret_action_without_presence_gate",
"file": ".github/workflows/instant-sync.yml",
"action": "peter-evans/repository-dispatch",
"rule_module": "workflow_audit",
"severity": "high",
"fix_recipe": "add_secret_presence_gate"
},
{
"reason": "codeql.yml does not list `language: actions` in its matrix, but the repo has workflow files. CodeQL's `actions` language scans workflow YAML for injection and other CI/CD-specific weaknesses — every repo with workflows benefits. Add an entry to `matrix.include` with `language: actions` + `build-mode: none`.",
"type": "codeql_missing_actions_language",
"file": ".github/workflows/codeql.yml",
"action": "flag",
"rule_module": "workflow_audit",
"severity": "medium",
"fix_recipe": "add_codeql_actions_language"
},
{
"line": 39,
"reason": "job in .github/workflows/labels.yml references `secrets.*` but does not install `step-security/harden-runner` — review outbound-egress monitoring",
"type": "RE001",
"file": ".github/workflows/labels.yml",
"action": "report",
"rule_module": "research_extensions",
"severity": "medium"
},
{
"line": 46,
"reason": "job in .github/workflows/push-email-notify.yml references `secrets.*` but does not install `step-security/harden-runner` — review outbound-egress monitoring",
"type": "RE001",
"file": ".github/workflows/push-email-notify.yml",
"action": "report",
"rule_module": "research_extensions",
"severity": "medium"
},
{
"line": 32,
"reason": "job in .github/workflows/build.yml references `secrets.*` but does not install `step-security/harden-runner` — review outbound-egress monitoring",
"type": "RE001",
"file": ".github/workflows/build.yml",
"action": "report",
"rule_module": "research_extensions",
"severity": "medium"
},
{
"line": 44,
"reason": "job in .github/workflows/container-publish.yml references `secrets.*` but does not install `step-security/harden-runner` — review outbound-egress monitoring",
"type": "RE001",
"file": ".github/workflows/container-publish.yml",
"action": "report",
"rule_module": "research_extensions",
"severity": "medium"
}
]Powered by Hypatia Neurosymbolic CI/CD Intelligence |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
9 of 14 tasks
hyperpolymath
added a commit
that referenced
this pull request
Oct 7, 2026
## Summary Follow-up to #354 (the bench). Zig 0.16 also removed `std.heap.GeneralPurposeAllocator` and `std.process.argsAlloc`. Because of that, `zig build invoke` fails with `src/boj_invoke_cli.zig:45:23: error: root source file struct 'heap' has no member named 'GeneralPurposeAllocator'`. That takes down **E2E — Full REST + MCP Bridge**, which is red on main, and the **Catalogue truthfulness** `--probe` job. Plain `zig build` never builds the `invoke` step, which is why the local check on #354 missed it. ## Changes - `ffi/zig/src/boj_invoke_cli.zig`: `main` now takes `std.process.Init` and uses `init.gpa`, `init.io`, and `init.minimal.args.toSlice(arena)`. stdout/stderr are `std.Io.File`, written with `writeStreamingAll(io, …)`. `emitJson` threads `io` through. - Docstrings added on `main` and `emitJson` (§5d). - `zig fmt` applied. The file was already failing `zig fmt --check` on main. ## Type of change Bug fix (build break under the pinned Zig 0.16.0 toolchain). No behaviour change: the verbs, JSON output and exit codes are the same. ## 📌 New pins Head SHA: **cfe598951808d75f8268f122e275cb7a31efc6e5**. No action, lockfile or container pins are added or changed. ## RSR Quality Checklist ### Required - [x] Tests pass: `zig build test --summary all` reports 41/41 steps succeeded and 316/316 tests passed. - [x] Code is formatted: `zig fmt --check ffi/zig/src/boj_invoke_cli.zig` rc 0. - [x] Linter is clean: zig compile has no warnings. The repo has no separate Zig linter. - [x] No banned language patterns: Zig only. - [x] No `unsafe` blocks without `// SAFETY:` comments. Not applicable: Zig, and no new unsafe constructs. - [x] No banned functions. - [x] SPDX license headers: the existing MPL-2.0 header is unchanged. - [x] No secrets, credentials, or `.env` files. ### As Applicable - [ ] `.machine_readable/*` updates. Not applicable: no project-state change. A2ML is retired estate-wide (D308). - [ ] Documentation. Not applicable: the CLI usage is unchanged. - [ ] `TOPOLOGY.md`. Not applicable. - [ ] CHANGELOG. Not applicable: an internal build fix, covered by the v0.5.0 release notes. - [ ] New dependencies. Not applicable: none. - [x] ABI/FFI consistency: the five-symbol ADR-0006 calls are unchanged. The fixture cartridge probe passes. ## Testing Run locally with Zig 0.16.0: - `zig build invoke`: rc 0 (it failed on main). - `boj-invoke libfeedback_mcp.so probe` printed `{"ok":true,"name":"feedback-mcp","version":"0.1.0"}` with rc 0. The error paths were also checked: no args gives rc 2, a missing .so gives `FileNotFound` with rc 3, and an unknown verb gives rc 2. - `bash tests/truthfulness_check.sh --probe`: `truthfulness: OK — 23 cartridges checked (probe=1)`. - `zig build test`: 316/316 tests pass. ## Pre-existing red checks (deferred, not introduced here) - `SonarQube`: red on main, tracked in #338. - `governance / UUID v7 conformance`: red on main, tracked in #347. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019j8She9eTFx54r6aL6sCHP Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
8 of 12 tasks
hyperpolymath
added a commit
that referenced
this pull request
Oct 7, 2026
…-empty (#356) ## Summary This is the last red check on the Zig 0.16 path (#354 bench, #355 invoke CLI). With #355 merged, **E2E — Full REST + MCP Bridge** builds and runs: 38 of 39 pass. The one failure is `teranga tier has cartridges (expected '1', got '0')`. The bundled `cartridges/` tree was retired, so the E2E catalogue is now `tests/fixtures/cartridges`, and all 23 of its manifests declare `"tier": "Ayo"`. The assertion can never pass against that catalogue, so this is a stale test rather than a defect in the menu. ## Changes - `tests/e2e_full.sh` Step 3: the "Teranga is non-empty" check is replaced by: - for each tier (teranga, shield, ayo), the menu count equals the number of manifests under `$BOJ_CARTRIDGES_PATH` declaring that tier (case-folded, matching `router.ex`); - `tier_teranga + tier_shield + tier_ayo == summary.total`, so no cartridge can silently fall outside the three tiers. - The new assertions don't assume any tier is non-empty. Tier assignments are being re-audited under the trust-tier ruling, so adding a fake Teranga fixture would have pre-empted that work. ## Type of change Test fix. No production code changes. ## 📌 New pins Head SHA: **7c192f068f9b20d5e70fd722ebe6bc3d1f41f98b**. No action, lockfile or container pins are added or changed. ## RSR Quality Checklist ### Required - [x] Tests pass: `bash tests/e2e_full.sh` locally reports `Results: 42 passed, 0 failed, 0 skipped (of 42)` with rc 0. - [x] Code is formatted: `bash -n tests/e2e_full.sh` reports no syntax errors. The repo has no shell formatter gate. - [x] Linter is clean: no new warnings. - [x] No banned language patterns: shell and jq only. - [x] No `unsafe` blocks. Not applicable. - [x] No banned functions. - [x] SPDX headers: the existing header is unchanged. - [x] No secrets. ### As Applicable - [ ] `.machine_readable/*`. Not applicable: A2ML is retired (D308). - [ ] Documentation, TOPOLOGY, CHANGELOG. Not applicable: test-only change. - [ ] New dependencies. Not applicable: `jq` is already installed by the workflow. - [ ] ABI/FFI. Not applicable. ## Testing - Full suite locally on Zig 0.16.0 and the Elixir backend: 42/42 pass. The new checks report teranga 0, shield 0, ayo 23, total 23. - Mutant controls, run on the new block in isolation against the real fixtures: - correct menu: 4/4 pass; - one cartridge dropped from `tier_ayo`: 2 fail (the ayo count and the total); - a cartridge in the wrong tier: 2 fail (the teranga count and the ayo count). ## Pre-existing red checks (deferred, not introduced here) - `SonarQube`: red on main, tracked in #338. - `governance / UUID v7 conformance`: red on main, tracked in #347. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_019j8She9eTFx54r6aL6sCHP Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com> Co-authored-by: Claude Opus 5.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
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.
Summary
zig buildfailed onmainunder the pinned Zig 0.16.0 with 13 errors. All 13 were the same error inffi/zig/src/bench.zig:27:enum 'Io.Clock' has no member named 'monotonic'. This turned the E2E and Bench jobs red. Zig 0.16'sstd.Io.Clockhasreal,awakeandboot.awakeis the monotonic clock that excludes suspend time, which is the correct clock for a latency benchmark.Type of change
Bug fix (build break). No behaviour change outside the benchmark harness.
📌 New pins
Head SHA:
74e839d9c583d822227e8afd535aa223740e43cf. This PR adds or changes no pins: no actionuses:,actions.lock, lockfile or container digest changes.Changes
ffi/zig/src/bench.zig:std.Io.Clock.Timestamp.now(shim.io(), .monotonic)→.awake, at both call sites. The comment now names the clock.RSR Quality Checklist
Required
zig build test --summary allreports "41/41 steps succeeded; 316/316 tests passed". The onefailed command:line in its output is the runner echoing a test that prints timing figures to stderr, not a failure.zig fmt --check src/bench.zigexits 0.zig buildrc 0, with no warnings.unsafeblocks without// SAFETY:comments: N/A, no new unsafe code..envfiles included.As Applicable
.machine_readable/*.a2mlupdated: N/A. A2ML is retired (D308), and project state is otherwise unchanged.TOPOLOGY.mdupdated: N/A, architecture unchanged.CHANGELOGor release notes updated: N/A; the release notes for v0.5.0 come with the tag.Testing
Run locally with Zig 0.16.0 in
ffi/zig/:zig build→ rc 0. Before the change: 13 errors, allbench.zig:27:60.zig build bench(the E2E workflow's bench step) → rc 0, ending with "Benchmark complete.".zig build test --summary all→ 316/316 tests passed.docs/AFFIRMATION.adocis deliberately left untouched. A fixed bench is a new state, so it gets a fresh affirmation against the post-merge anchor, drafted for the owner to sign.🤖 Generated with Claude Code
https://claude.ai/code/session_019j8She9eTFx54r6aL6sCHP