chore: docs and benchmark - #26
Merged
Merged
Conversation
The shields.io docker/v badge sorts tags by push date by default, and the buildcache tag (re-pushed on every release build) is always the newest, so the badge read buildcache instead of the version. Add sort=semver so it picks the highest semantic version tag; verified it now renders v0.7.8.
…adata Bring the load-test harness to metric parity with the Iggy benchmark dashboard. Replace the reservoir sample (which under-represents the deep tail, so P99.9/P99.99 were noise) with an inline HDR-style log-linear histogram: accurate percentiles including the deep tail, bounded memory, no sampling error. Add stddev via Welford's online variance, extend the reported percentiles to P99.9 and P99.99, and add a meta block to --json (exact command, git ref, malachi version, hardware) so a stored result is self-describing. A --self-test validates the histogram offline against a brute-force reference. No new dependencies.
Malachi.Test.PollingHelper (wait_until/2, wait_until!/2) was unused dead code, while broker_server_test.exs and broker_server_streaming_test.exs each defined their own local defp wait_until. Import the shared helper in both and drop the duplicates. The call sites become wait_until! (raise on timeout), preserving the old fail-on-timeout behavior (a false-returning assert and a flunk respectively).
Add a self-contained results page (benchmark/dashboard/) that renders one loadtest.js --json result: the throughput headline, a latency percentile curve (P50 to P99.99) drawn as inline SVG, and the reproduce metadata (command, git ref, version, hardware). No build step and no dependencies; it reads data.json next to it, theme-aware via prefers-color-scheme. The Pages workflow copies the bundle into doc/benchmarks so it publishes at /benchmarks/. data.json is a real sample run, labeled as such.
…en rings, and resumability
📝 WalkthroughWalkthroughThe PR adds a benchmark dashboard and automated publication, replaces load-test reservoir sampling with histogram statistics, documents wire protocol compatibility, and expands broker and cluster property-test coverage. ChangesBenchmark reporting and publication
Wire protocol documentation
Test coverage updates
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
coderabbitai
Bot
added
ci/cd
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
benchmark/dashboard/index.html (2)
145-145: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGive the chart an accessible name.
The
svgelement declaresrole="img"but has no accessible name. A screen reader announces an unlabeled image. Addaria-label.♿ Proposed fix
- const svg = svgEl("svg", { viewBox: `0 0 ${W} ${H}`, width: "100%", height: "auto", role: "img" }); + const svg = svgEl("svg", { + viewBox: `0 0 ${W} ${H}`, + width: "100%", + height: "auto", + role: "img", + "aria-label": "Latency percentiles from P50 to P99.99, in milliseconds", + });🤖 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 `@benchmark/dashboard/index.html` at line 145, Add an aria-label to the SVG created in the svgEl call so the role="img" chart has a clear accessible name for screen readers.
215-218: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueUse
textContentfor the error message.
e.messageis inserted intoinnerHTML. A JSON parse failure can place document fragments into that message. Build the node instead of parsing HTML.🔒 Proposed fix
.catch((e) => { - document.getElementById("subtitle").innerHTML = - '<span class="err">Could not load data.json (' + e.message + "). Serve this directory over HTTP.</span>"; + const span = el("span", { + class: "err", + text: `Could not load data.json (${e.message}). Serve this directory over HTTP.`, + }); + const subtitle = document.getElementById("subtitle"); + subtitle.textContent = ""; + subtitle.appendChild(span); });🤖 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 `@benchmark/dashboard/index.html` around lines 215 - 218, Update the data-loading catch handler to avoid assigning the error message through innerHTML. In the catch callback, build the error display using a text-safe DOM node or textContent while preserving the existing error text and subtitle target.Source: Linters/SAST tools
🤖 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 `@benchmark/dashboard/data.json`:
- Line 5: Regenerate benchmark/dashboard/data.json, including its git_ref
metadata, from a clean working tree at the committed sample revision so the
value no longer has the “-dirty” suffix. Preserve the generated result and
associated metadata for the self-describing benchmark output.
In `@benchmark/dashboard/README.md`:
- Around line 20-22: Update the benchmark regeneration command in the README to
include the `--record-size 256` option, matching the workload configuration
recorded in benchmark/dashboard/data.json.
In `@lib/malachi/wire.ex`:
- Around line 36-39: Update the protocol evolution guidance near
decode_auth_req/1, decode_fetch_req/1, and decode_commit_req/1 to match actual
compatibility behavior: assign a new api_key to any extended payload unless the
decoders are first changed to support an explicit version or capability
mechanism and safely ignore appended fields. Do not document same-key field
appends as backward-compatible while these decoders require an empty remainder.
In `@scripts/loadtest.js`:
- Around line 361-378: Update buildMeta in scripts/loadtest.js to record the
script path relative to the repository root while preserving the remaining
command-line arguments, instead of exposing the absolute process.argv path.
Regenerate benchmark/dashboard/data.json after this change so its command field
no longer contains the operator’s home-directory path.
- Around line 544-546: Update the load-test broker operation paths in main,
including produce, fetch, subscribe, and streaming acknowledgements, to call the
shared withRetry helper with the existing transient-error predicates. Ensure
:migrating and :not_owner failures are retried consistently so they are not
counted as benchmark errors and do not terminate streaming runs.
---
Nitpick comments:
In `@benchmark/dashboard/index.html`:
- Line 145: Add an aria-label to the SVG created in the svgEl call so the
role="img" chart has a clear accessible name for screen readers.
- Around line 215-218: Update the data-loading catch handler to avoid assigning
the error message through innerHTML. In the catch callback, build the error
display using a text-safe DOM node or textContent while preserving the existing
error text and subtitle target.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 58fc924e-1450-4af2-9e5b-81ba93eaba0a
📒 Files selected for processing (11)
.github/workflows/pages.ymlREADME.mdbenchmark/dashboard/README.mdbenchmark/dashboard/data.jsonbenchmark/dashboard/index.htmllib/malachi/wire.exscripts/loadtest.jstest/malachi/broker_server_streaming_test.exstest/malachi/broker_server_test.exstest/malachi/cluster/placement_test.exstest/malachi/cluster/reshard_plan_test.exs
…ayload append fix(loadtest): relative command path in meta and retry transient reshard errors
…ADME command ci(bench): workflow to regenerate the dashboard sample on a stable runner
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/bench-dashboard.yml:
- Around line 47-48: Update the checkout step using actions/checkout@v4 to set
persist-credentials to false. Before the later git push command, explicitly
configure Git authentication using the workflow token, while leaving gh pr
create to use GH_TOKEN as currently configured.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0de632e9-a339-4532-b87e-7daadd9c2da8
📒 Files selected for processing (3)
.github/workflows/bench-dashboard.ymlbenchmark/dashboard/README.mdbenchmark/dashboard/data.json
🚧 Files skipped from review as they are similar to previous changes (2)
- benchmark/dashboard/README.md
- benchmark/dashboard/data.json
- ci(bench-dashboard): checkout with persist-credentials false so the write token is not left in the local git config while the load test (repo code) runs; the push authenticates explicitly with the token, and gh pr create uses GH_TOKEN - bench(dashboard): give the latency chart svg an aria-label; render the data.json load error via textContent instead of innerHTML Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
HectorIFC
marked this pull request as ready for review
August 10, 2026 16:18
Contributor
✅ PR Validation Summary
Next Steps
This comment was automatically generated by the PR validation workflow |
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.
📝 Description
🔖 Type of Change
✅ Checklist
🧪 How to Test
📸 Screenshots (if applicable)
🔗 Related Issues
Note about versioning:
patch,minorormajoron the PR[major],[minor]in the titlepatch(0.0.1)Summary by CodeRabbit
New Features
/benchmarks/alongside the documentation.Documentation
Improvements
Tests