Consolidate benchmark shell scripts into shared lib + merged sweep script - #2
Merged
Conversation
Copilot created this pull request from a session on behalf of
daedalus
July 15, 2026 23:35
View session
daedalus
marked this pull request as ready for review
July 15, 2026 23:35
Reviewer's GuideRefactors the benchmarking shell scripts around a shared helper library, merges the two sweep scripts into a single, deduplicated benchmark sweep, fixes SHM cleanup and retry/coverage handling, aligns all benchmarks and docs with the removal of the deprecated --meta-elo flag, and updates Elo reporting to the new internal API. Sequence diagram for run_with_retry coverage and SHM handlingsequenceDiagram
participant BenchScript as bench.sh_or_bench_sweep.sh
participant BenchLib as bench_common_sh
participant Fuzzer as fuzzer_tool
participant SHM as OS_SHM
BenchScript->>BenchLib: run_with_retry(log, fuzz args)
loop attempts up to BENCH_MAX_RETRIES
BenchLib->>Fuzzer: python -m fuzzer_tool fuzz ...
Fuzzer-->>BenchLib: write log
alt [log is empty]
BenchLib-->>BenchScript: "Run produced no log output" message
else [log has content]
BenchLib->>BenchLib: check_coverage(log, label)
BenchLib->>BenchLib: verify_shm(log, label)
BenchLib->>SHM: shmat(shm_id)
SHM-->>BenchLib: bitmap bytes
alt [bitmap has non-zero bytes]
BenchLib-->>BenchScript: success
Note over BenchLib,BenchScript: break loop
else [coverage-blind run]
BenchLib-->>BenchScript: "Coverage did not attach" message
end
end
BenchLib->>BenchLib: cleanup_shm()
BenchLib->>SHM: ipcs/ipcrm on orphaned segments
end
BenchLib-->>BenchScript: failure after BENCH_MAX_RETRIES
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
bench_common.sh,verify_shmusespython3whilerun_with_retryandrun_combousepython; consider standardizing on one interpreter (python3 -m fuzzer_tool) to avoid environment-dependent failures. run_with_retrytreats both "no log produced" and explicit coverage failures as the same retry path and always prints "Coverage did not attach"; you might want to branch the messaging so the final failure reason clearly distinguishes between startup crashes and SHM-attachment issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `bench_common.sh`, `verify_shm` uses `python3` while `run_with_retry` and `run_combo` use `python`; consider standardizing on one interpreter (`python3 -m fuzzer_tool`) to avoid environment-dependent failures.
- `run_with_retry` treats both "no log produced" and explicit coverage failures as the same retry path and always prints "Coverage did not attach"; you might want to branch the messaging so the final failure reason clearly distinguishes between startup crashes and SHM-attachment issues.
## Individual Comments
### Comment 1
<location path="tools/lib/bench_common.sh" line_range="16" />
<code_context>
+ # Capture matching SHM IDs into a variable first: under `set -o pipefail`,
+ # piping straight into `while read` would abort the script (via `set -e`)
+ # whenever grep finds no matches (the common case with no stale segments).
+ shmids=$(ipcs -m 2>/dev/null | grep "$(whoami)" | awk '{print $2}' || true)
+ if [[ -n "$shmids" ]]; then
+ while read -r shmid; do
</code_context>
<issue_to_address>
**suggestion (bug_risk):** User matching in SHM cleanup can accidentally match other usernames that contain the current username as a substring.
`grep "$(whoami)"` matches any line where the username appears as a substring (e.g., `foo` also matches `foobar`), so it can select SHM segments owned by other users. To restrict matches to the owner field, use something like:
- `awk '$3 == "'"$(whoami)"'" {print $2}'`, or
- `grep -E "^[^ ]+ +[^ ]+ +$(whoami) " | awk '{print $2}'`
so that only segments owned by the current user are removed.
Suggested implementation:
```
local before shmids
before=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l)
# Capture matching SHM IDs into a variable first: under `set -o pipefail`,
# piping straight into `while read` would abort the script (via `set -e`)
# whenever the filter finds no matches (the common case with no stale segments).
shmids=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user {print $2}')
```
```
local after
after=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l)
```
</issue_to_address>
### Comment 2
<location path="tools/lib/bench_common.sh" line_range="123-129" />
<code_context>
+# Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries
+# on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup
+# between attempts.
+BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}"
+
+run_with_retry() {
</code_context>
<issue_to_address>
**suggestion:** Consider validating `BENCH_MAX_RETRIES` to ensure it is a positive integer before using it in numeric comparisons.
Because `BENCH_MAX_RETRIES` is now environment-configurable, non-numeric or zero/negative values (e.g. `BENCH_MAX_RETRIES=foo` or `0`) can make `[[ $attempt -le $BENCH_MAX_RETRIES ]]` fail or behave unpredictably under `set -euo pipefail`. Consider normalizing the value once (e.g. default to 3 when unset or not a positive integer) so the retry loop stays robust to misconfiguration.
```suggestion
# ── Run with retry ────────────────────────────────────────────────────
# Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries
# on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup
# between attempts.
BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}"
# Normalize BENCH_MAX_RETRIES to a positive integer; fall back to 3 on
# invalid or non-positive values to keep the retry loop robust.
if ! [[ "$BENCH_MAX_RETRIES" =~ ^[1-9][0-9]*$ ]]; then
echo "[!] Invalid BENCH_MAX_RETRIES='$BENCH_MAX_RETRIES'; using default of 3" >&2
BENCH_MAX_RETRIES=3
fi
run_with_retry() {
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| # Capture matching SHM IDs into a variable first: under `set -o pipefail`, | ||
| # piping straight into `while read` would abort the script (via `set -e`) | ||
| # whenever grep finds no matches (the common case with no stale segments). | ||
| shmids=$(ipcs -m 2>/dev/null | grep "$(whoami)" | awk '{print $2}' || true) |
There was a problem hiding this comment.
suggestion (bug_risk): User matching in SHM cleanup can accidentally match other usernames that contain the current username as a substring.
grep "$(whoami)" matches any line where the username appears as a substring (e.g., foo also matches foobar), so it can select SHM segments owned by other users. To restrict matches to the owner field, use something like:
awk '$3 == "'"$(whoami)"'" {print $2}', orgrep -E "^[^ ]+ +[^ ]+ +$(whoami) " | awk '{print $2}'
so that only segments owned by the current user are removed.
Suggested implementation:
local before shmids
before=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l)
# Capture matching SHM IDs into a variable first: under `set -o pipefail`,
# piping straight into `while read` would abort the script (via `set -e`)
# whenever the filter finds no matches (the common case with no stale segments).
shmids=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user {print $2}')
local after
after=$(ipcs -m 2>/dev/null | awk -v user="$(whoami)" '$3 == user' | wc -l)
Comment on lines
+123
to
+129
| # ── Run with retry ──────────────────────────────────────────────────── | ||
| # Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries | ||
| # on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup | ||
| # between attempts. | ||
| BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}" | ||
|
|
||
| run_with_retry() { |
There was a problem hiding this comment.
suggestion: Consider validating BENCH_MAX_RETRIES to ensure it is a positive integer before using it in numeric comparisons.
Because BENCH_MAX_RETRIES is now environment-configurable, non-numeric or zero/negative values (e.g. BENCH_MAX_RETRIES=foo or 0) can make [[ $attempt -le $BENCH_MAX_RETRIES ]] fail or behave unpredictably under set -euo pipefail. Consider normalizing the value once (e.g. default to 3 when unset or not a positive integer) so the retry loop stays robust to misconfiguration.
Suggested change
| # ── Run with retry ──────────────────────────────────────────────────── | |
| # Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries | |
| # on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup | |
| # between attempts. | |
| BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}" | |
| run_with_retry() { | |
| # ── Run with retry ──────────────────────────────────────────────────── | |
| # Runs `python -m fuzzer_tool "$@"`, verifying coverage attached; retries | |
| # on coverage-blind runs up to MAX_RETRIES (default 3) with SHM cleanup | |
| # between attempts. | |
| BENCH_MAX_RETRIES="${BENCH_MAX_RETRIES:-3}" | |
| # Normalize BENCH_MAX_RETRIES to a positive integer; fall back to 3 on | |
| # invalid or non-positive values to keep the retry loop robust. | |
| if ! [[ "$BENCH_MAX_RETRIES" =~ ^[1-9][0-9]*$ ]]; then | |
| echo "[!] Invalid BENCH_MAX_RETRIES='$BENCH_MAX_RETRIES'; using default of 3" >&2 | |
| BENCH_MAX_RETRIES=3 | |
| fi | |
| run_with_retry() { |
There was a problem hiding this comment.
Pull request overview
This PR consolidates the benchmarking tooling by extracting duplicated shell helpers into a shared library, merges the second sweep script into the primary sweep, and aligns docs/reporting with the consolidation of the former --meta-elo behavior into --elo.
Changes:
- Added
tools/lib/bench_common.shand updatedtools/bench.sh/tools/bench_sweep.shto source shared SHM cleanup, coverage verification, and log-extraction helpers. - Merged
tools/bench_sweep2.shscenarios intotools/bench_sweep.shand removedtools/bench_sweep2.sh. - Updated reporting/docs to stop referencing removed meta-elo fields/flags and document the new benchmark layout.
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/lib/bench_common.sh | New shared benchmark helper library (SHM cleanup, extraction, coverage verification, retry, sweep runner). |
| tools/bench.sh | Switched to shared helper library; removed --meta-elo usage; updated CI extraction formatting. |
| tools/bench_sweep.sh | Switched to shared helper library; merged/expanded sweep phases; removed --meta-elo combos; increased results output. |
| tools/bench_sweep2.sh | Removed (folded into tools/bench_sweep.sh). |
| src/fuzzer_tool/services/report.py | Fixed report gating to use _use_elo instead of removed _use_meta_elo. |
| README.md | Updated benchmark configuration docs and referenced shared helper library. |
| docs/TODO.md | Recorded benchmark consolidation and the report.py _use_meta_elo fix. |
| docs/compose/reports/fuzzer-optimization-journey.md | Updated example command to remove --meta-elo. |
| AGENTS.md | Updated tools tree and documented Elo behavior without separate --meta-elo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+73
to
+90
| local has_data | ||
| has_data=$(python3 -c " | ||
| import ctypes, ctypes.util | ||
| libc = ctypes.CDLL(ctypes.util.find_library('c') or 'libc.so.6', use_errno=True) | ||
| libc.shmat.restype = ctypes.c_void_p | ||
| ptr = libc.shmat($shm_id, None, 0) | ||
| if ptr is None or ptr == -1: | ||
| print('FAIL') | ||
| else: | ||
| size = 4096 # default map size | ||
| bitmap = (ctypes.c_uint8 * size).from_address(ptr) | ||
| non_zero = sum(1 for i in range(size) if bitmap[i] != 0) | ||
| libc.shmdt(ptr) | ||
| if non_zero > 0: | ||
| print(f'OK:{non_zero}') | ||
| else: | ||
| print('EMPTY') | ||
| " 2>/dev/null) |
|
|
||
| while [[ $attempt -le $BENCH_MAX_RETRIES ]]; do | ||
| echo "[*] Attempt $attempt/$BENCH_MAX_RETRIES..." | ||
| python -m fuzzer_tool "$@" 2>&1 | tee "$log" |
daedalus
added a commit
that referenced
this pull request
Jul 16, 2026
Consolidate benchmark shell scripts into shared lib + merged sweep script
daedalus
added a commit
that referenced
this pull request
Jul 17, 2026
ENTROPY_HISTORY_MAX=200, ENTROPY_HISTORY_TRIM=100, ENTROPY_WINDOW=4, ENTROPY_FLAT_THRESHOLD=0.001 All four findings verified: - #2: zlib.crc32 returns deterministic int for LSH bucket keys ✓ - #3: grammar repeat bounds already clamped with max(hi, lo) ✓ - #4: edge_tracker uses zlib.crc32, not builtin hash() ✓
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
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.
tools/bench.sh,tools/bench_sweep.sh, andtools/bench_sweep2.shduplicated the same helper functions (cleanup_shm,extract,extract_ci,run_combo,verify_shm,check_coverage,run_with_retry) across ~650 lines, andbench_sweep2.shre-ran many combos already covered bybench_sweep.sh.Shared library
tools/lib/bench_common.sh, sourced by both remaining scriptsextract_cinow takes an explicit delimiter parameter (|for CSV rows," "for table display) instead of callers post-processing the outputMerged sweep script
tools/bench_sweep2.shintotools/bench_sweep.sh(8 phases, 71 combos)--meta-eloflag removal (s3/s4,gt1-gt3,f1-f3, etc.)f16-f18,t8,z1-z4variance-check phase)tools/bench_sweep2.shBug fix
cleanup_shmpipedgrepoutput directly intowhile read; underset -o pipefail+set -e, this aborted the entire script whenever there were no stale SHM segments to clean (the common case). Fixed by capturing grep output into a variable before the loop.run_with_retrynow distinguishes "no log produced" (target crashed before startup) from "coverage failed to attach" for clearer retry diagnosticsDocs
tools/bench.sh/tools/bench_sweep.sh/tools/lib/bench_common.shlayoutSummary by Sourcery
Consolidate benchmark shell scripts into a shared library, merge and expand the feature-sweep benchmark, and align scripts/docs with the updated Elo/meta-elo behavior.
Bug Fixes:
Enhancements: