Fix 5 single-line confirmed bugs from bugreport - #7
Merged
Conversation
Co-authored-by: daedalus <115175+daedalus@users.noreply.github.com>
Co-authored-by: daedalus <115175+daedalus@users.noreply.github.com>
Copilotcreated this pull request from a session on behalf of daedalusAugust 24, 2026 21:46 View session
Reviewer's GuideFixes five confirmed single-line bugs across persistent execution, GF(2^n) arithmetic, numeric generation, JPEG generation, and test-execution positioning; updates the audit documentation and adds deterministic regression tests covering each corrected behavior. Sequence diagram for resumed persistent executionsequenceDiagram
participant Runner
participant PersistentTarget
Runner->>PersistentTarget: run_one(data)
PersistentTarget-->>PersistentTarget: Read return code from SIGSTOP branch
PersistentTarget->>PersistentTarget: os.kill(pid, SIGCONT)
PersistentTarget-->>Runner: return returncode
Flow diagram for corrected bug-report behaviorsflowchart LR
A[GF2 pow with zero base] --> B[Preserve exponent]
C[Decimal number generation] --> D[Append generated digits]
E[Small JPEG max_len] --> F[Clamp scan length lower bound]
G[TE position selection] --> H[Choose highest edge-hit weight]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
daedalus
marked this pull request as ready for review
August 24, 2026 21:46
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="src/fuzzer_tool/services/te_position.py"line_range="46-49" />
<code_context>
if not byte_edges:
return None
- best_pos = max(byte_edges.keys())
+ best_pos = max(byte_edges, key=lambda pos: sum(byte_edges[pos].values()))
return best_pos if best_pos < input_length else None
</code_context>
<issue_to_address>
**issue (bug_risk):** When the highest-weight entry in `byte_edges` has a position at or beyond `input_length`, `max(...)` selects it first and the function returns `None` instead of selecting a lower-weight position that is valid for the current input.
**Triggers:** When stale TE data contains positions from longer inputs and the current input is shorter than the highest-weight position.
**Suggested fix:** Filter `byte_edges` to positions below `input_length` before applying `max`, returning `None` only when no valid positions remain.
```suggestion valid_positions = [pos for pos in byte_edges if pos < input_length] if not valid_positions: return None return max(valid_positions, key=lambda pos: sum(byte_edges[pos].values()))```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: src/fuzzer_tool/services/te_position.py:49
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
46
to
49
| if not byte_edges: | ||
| return None | ||
| best_pos = max(byte_edges.keys()) | ||
| best_pos = max(byte_edges, key=lambda pos: sum(byte_edges[pos].values())) | ||
| return best_pos if best_pos < input_length else None |
There was a problem hiding this comment.
issue (bug_risk): When the highest-weight entry in byte_edges has a position at or beyond input_length, max(...) selects it first and the function returns None instead of selecting a lower-weight position that is valid for the current input.
Triggers: When stale TE data contains positions from longer inputs and the current input is shorter than the highest-weight position.
Suggested fix: Filter byte_edges to positions below input_length before applying max, returning None only when no valid positions remain.
Suggested change
| ifnotbyte_edges: | |
| returnNone | |
| best_pos=max(byte_edges.keys()) | |
| best_pos=max(byte_edges, key=lambdapos: sum(byte_edges[pos].values())) | |
| returnbest_posifbest_pos<input_lengthelseNone | |
| valid_positions= [posforposinbyte_edgesifpos<input_length] | |
| ifnotvalid_positions: | |
| returnNone | |
| returnmax(valid_positions, key=lambdapos: sum(byte_edges[pos].values())) |
There was a problem hiding this comment.
🟡 Changes recommended
The new SIGCONT send in PersistentRunner.run_one() can raise ProcessLookupError without handling, potentially crashing the fuzzer loop.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes five audited single-line bugs across the fuzzer’s persistent runner, GF(2^n) math, versifier number emission, JPEG generation, and TE-based mutation targeting, and adds regression tests to lock in the corrected behavior.
Changes:
- Add a regression test suite covering all five bugreport findings (with deterministic versifier output).
- Fix TE position selection to use weighted edge influence rather than max byte offset.
- Fix one-line bugs in persistent SIGCONT resume, GF2 pow(0, e) exponent handling, JPEG scan length clamping, and versifier base-10 emission.
File summaries
| File | Description |
|---|---|
| tests/test_regression_bugreport_easy_fixes.py | New regression tests for the five confirmed single-line fixes (incl. deterministic versifier case). |
| src/fuzzer_tool/services/te_position.py | Select TE mutation position by highest summed edge influence weight. |
| src/fuzzer_tool/core/mutations/jpeg.py | Clamp JPEG scan-length randint upper bound to avoid invalid ranges on small max_len. |
| src/fuzzer_tool/core/mutations/generic.py | Ensure _NumNode.Generate appends digits for base-10 numbers. |
| src/fuzzer_tool/core/gf2_common.py | Fix conditional exponent reduction so pow(0, e) doesn’t incorrectly reduce e. |
| src/fuzzer_tool/adapters/persistent.py | Resume persistent targets by sending SIGCONT after reading return code on SIGSTOP. |
| docs/bugreport_2026-08-21_merged.md | Mark the five confirmed findings as fixed with brief fix notes. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+162
to
163
| os.kill(self.pid, signal.SIGCONT) | ||
| return returncode, "" |
daedalus
commented
Aug 24, 2026
Owner
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
daedalus added a commit
that referenced
this pull request
Aug 29, 2026
Each of these carried a status table where most rows had closed, which buried the handful of open ones. Same treatment in all three: the closed rows are named in one line so nobody re-surveys them, and the per-row justification goes to git history. `six_source_technique_port.md` — nine of sixteen rows were "already done", six of them by machinery better than what the source proposed. Pruned to a name-only list. Kept the note that four of them left a narrow residue that is genuinely NOT covered, which is R5 and still open, and the "Read this first" framing, which is the point of the document: it was drafted from sources alone and the audit against live code killed most of it. `tigerbeetle_four_fuzzers_port.md` — P1-3 (scheduler convergence), P1-5 (exhaustive enumeration) and P2-6 (negative space) are done, and their whole sections are gone. What each of them FOUND is not lost: all three have a learnings note (`docs/learnings/2026-08-21-scheduler-convergence.md`, `2026-08-22-exhaustive-pool-p1-5.md`, `2026-08-22-count-class-exhaustive.md`) and the status block now points at those instead of restating them. The 20 remaining `rng.random() < 0.5` coin-flip sites are promoted out of the P1-5 prose into their own open item, since that is a P1-5 follow-up rather than unfinished P1-5 and was easy to misread as the latter. "Suggested sequence" was a seven-step plan whose first three steps are done; rewritten to the five that remain. `web_research_port_candidates_2026-08.md` — Tier 1 is closed in its entirety and pruned. Four rows landed 2026-08-24; #4 trace-div/trace-gep and #5 n-gram edge coverage shipped after this doc was last touched and were still tabled here as `L`-effort candidates. #6 (Zest validity channel) and #7 (SGFuzz enum states) shipped as mechanism — `--reject-code` and `__sfuzz_state` both exist — but each left one open design question, and those two questions are already tracked in `docs/TODO.md` under Scheduling, so tracking them here as well was the duplication. Tier 2/3 are untouched and still unstarted; the note that their effort estimates are unaudited guesses now cites Tier 1 as the evidence for why, since two of its seven rows turned out to be near-free. Status of every pruned row was checked against live source, not against the row's own marker.
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 freeto 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.
Fixes 5 confirmed single-line bugs identified in bugreport_2026-08-21_merged.md, with a follow-up commit making the versifier regression test deterministic per code review feedback.
Summary by Sourcery
Fix five confirmed single-line bugs and add deterministic regression coverage for their corrected behavior.
Bug Fixes:
Documentation:
Tests: