Fix easy confirmed bugs from bugreport_2026-08-21_merged.md - #8
Conversation
Co-authored-by: daedalus <115175+daedalus@users.noreply.github.com>
Co-authored-by: daedalus <115175+daedalus@users.noreply.github.com>
Reviewer's GuideFixes three confirmed reproducibility and seed-generation bugs: short maximum lengths no longer produce invalid random ranges, radamsa numeric sign selection honors injected RNGs, and grammar generation uses its injected RNG for deterministic output; documentation and independent regression coverage were added. Sequence diagram for deterministic injected-RNG mutationsequenceDiagram
participant Caller
participant Radamsa as radamsa_mutate_num
participant RNG as InjectedRNG
Caller->>Radamsa: radamsa_mutate_num(val, rng)
Radamsa->>RNG: randint(1, 128)
Radamsa->>RNG: random()
RNG-->>Radamsa: deterministic sign decision
Radamsa-->>Caller: mutated integer
Sequence diagram for deterministic grammar generationsequenceDiagram
participant Caller
participant Grammar
participant RNG as InjectedRNG
Caller->>Grammar: mutate(..., rng)
Grammar->>Grammar: _expand_rule(name, depth)
Grammar->>RNG: choice(alts)
Grammar->>Grammar: _expand_tokens(tokens, depth)
Grammar->>RNG: randint(lo, hi)
RNG-->>Grammar: deterministic choices
Grammar-->>Caller: generated bytes
Flow diagram for safe short-length generic seed generationflowchart TD
A["Generic seed generation"] --> B["Read f.max_len"]
B --> C["Compute lower bound min(4, f.max_len)"]
C --> D["Compute upper bound min(64, f.max_len)"]
D --> E["rng.randint(lower bound, upper bound)"]
E --> F["Create random buffer of selected length"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
🟡 Changes recommended
The two new RNG regression tests can still pass nondeterministically on regression and should be hardened to fail deterministically (e.g., by patching module-global random.* to raise).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes a few confirmed “easy” bugreport findings affecting seed generation for small --max-len values and RNG determinism (in radamsa numeric mutation and grammar expansion), and adds regressions + notes in the merged bugreport doc.
Changes:
- Clamp generic seed length bounds to avoid
randint()invalid ranges whenmax_len < 4. - Ensure injected RNG is used for
radamsa_mutate_numop==9 sign selection and forGrammarrule/token expansion. - Add/extend regression tests and mark the corresponding bugreport items as fixed.
File summaries
| File | Description |
|---|---|
| tests/test_regression_bugreport_easy_fixes.py | Adds regression coverage for the fixed RNG leaks and short max_len seed generation. |
| src/fuzzer_tool/services/seed_picker.py | Fixes invalid randint() bounds in generic seed generation when max_len is small. |
| src/fuzzer_tool/core/mutations/generic.py | Uses injected RNG for radamsa op==9 sign selection to preserve -s determinism. |
| src/fuzzer_tool/core/grammar.py | Uses self._rng (set by mutate()) instead of module-global random during expansion. |
| docs/bugreport_2026-08-21_merged.md | Marks specific findings as fixed and links to regression tests. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # op=9 (random scaling): randint(0,9)->9, randint(1,128)->9, | ||
| # random()->0.9 selects the "val - n" branch. If the sign draw | ||
| # silently fell back to the global `random` module instead of this | ||
| # fake, it would not consume the sentinel and the call would raise | ||
| # StopIteration on the second next(). | ||
| n = _log2_ceil(9) | ||
| rng = _FakeRng(randints=[9, 9], randoms=[0.9]) | ||
| assert radamsa_mutate_num(100, rng=rng) == 100 - n |
| g._rng = _FakeRng() | ||
| out = g.generate("root") | ||
| assert out == b"a" | ||
| assert g._rng.choice_calls == 1 |
daedalus
commented
Aug 24, 2026
@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. |
Four documents all answered "what should we port next", each with its own ranking scheme: the six-external-sources survey (`R1`-`R6`), the TigerBeetle "Tale Of Four Fuzzers" port (`P0-1`..`P2-8`), the 2026-08-25 internet survey (29 tiered candidates) and the 2026-08-28 GitHub + FFmpeg survey (`I.*`, `II.*`). They cross-referenced each other, deferred to each other, and repeated each other. Merged into `docs/port-backlog.md`. The merge is the point, not the line count. Split across four files it was not visible that: - **Four separate proposals solve the same structure-aware-generation gap** — Grimoire generalization, Gramatron FSA walks, the tree-sitter category and FormatFuzzer templates. They are now section A, ranked by plumbing already in place, which puts Grimoire first (it reuses the colorization executor loop almost verbatim) rather than in the middle of a tier table. That is not the order any single source gave. - **Six items overlap on "which bytes should we mutate"** — FairFuzz masks, WindRanger probing, IDFuzz gradients, DataFlowTrace taint, NestFuzz repair and identifier canonicalization. Section B. FairFuzz carries the warning it always had, now next to its competitors: it may simply overlap the rarity bonus we already ship, so measure before committing. - **Two solver items and five substrate items** were spread across three files with three different effort scales. Sections G and H. Also merged rather than dropped: - **The four "not worth porting" lists into one "Rejected" section.** These are the highest-value part of any survey and the easiest to lose — "absent from the doc" and "considered and declined" are different states, and only one of them should be re-proposable. Nine rejections with their reasoning, plus the watch-list. - **The source-quality accounting.** fitzgen's is the only source with proper statistics and its conclusions can be trusted; PowerFuzz reports single numbers per cell with no variance and a 10x throughput tax invisible in every plot, so take nothing on its numbers. Anyone reading a candidate's claimed win needs that next to the candidate, not in a different file. - **The open questions that gate the work**, including the two the 2026-08-28 survey surfaced and nobody has acted on: colorization should probably be opt-out, and Entropic and our Chao2 rewrite come from the same STADS framework and have never been reasoned about together. The FFmpeg harness work (`II.*`) is section J and keeps its 7-commit sequence intact — it is the one block here that is fully specified and ready to start. Its status table was re-grepped: every mechanism it lists is still absent. Ten source and test citations re-pointed. The distinction matters and drove where each went: citations for items that **shipped** (`P1-5` exhaustive enumeration, `P2-6` negative space, `#8` subtree crossover) now point at the learnings note that records what building them actually found, or are made self-contained — pointing them at a backlog of *open* work would be wrong. Citations for items still **open** (`P1-4` minimal interface, `P0-1` seed discipline) point at the backlog.
Summary
Fixes easy confirmed bugs from
bugreport_2026-08-21_merged.md, with a regression test hardened per code review to derive its expected value independently rather than duplicating the implementation's formula.Changes
0045b90— Fix easy confirmed bugs from bugreport_2026-08-21_merged.mdf5d599a— Address code review: derive expected value independently in radamsa test (uses_log2_ceil(9)instead of hardcoding the expected result)Summary by Sourcery
Fix confirmed seed-generation and random-number reproducibility bugs, with regression coverage for the corrected behavior.
Bug Fixes:
Documentation:
Tests: