Add subtree-population crossover to grammar TreeMutator - #6
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:09 View session
Reviewer's GuideIntroduces grammar-aware subtree-population crossover by maintaining bounded, reservoir-sampled per-rule donor pools harvested incrementally from the corpus, then using compatible cloned subtrees during tree mutation with productive fallback behavior. Documentation and regression tests cover reservoir sampling, splice dispatch, operator lifecycle, and edge cases. Sequence diagram for incremental subtree-population crossoversequenceDiagram
participant Operator as GrammarTreeOperator
participant Mutator as TreeMutator
participant Population as SubtreePopulation
participant Corpus as Corpus
GrammarTreeOperator->>TreeMutator: parse(buf, chunk_size)
GrammarTreeOperator->>Corpus: read corpus[next_idx:]
loop newly added corpus entries
GrammarTreeOperator->>TreeMutator: parse(seed)
TreeMutator-->>GrammarTreeOperator: donor_tree
GrammarTreeOperator->>SubtreePopulation: add(donor_tree, rng)
end
GrammarTreeOperator->>TreeMutator: mutate_tree(tree, max_len, rng, population)
TreeMutator->>SubtreePopulation: sample(target.rule, rng)
SubtreePopulation-->>TreeMutator: donor subtree
TreeMutator->>TreeMutator: _clone_tree(donor)
TreeMutator-->>GrammarTreeOperator: serialized mutated tree
Flow diagram for grammar tree mutation with donor fallbackflowchart TD
A[Parse input tree] --> B[Harvest new corpus trees]
B --> C[mutate_tree]
C --> D{Subtree splice selected?}
D -- No --> E[Run selected mutation]
D -- Yes --> F{Matching donor available?}
F -- Yes --> G[Clone and replace same-rule subtree]
F -- No --> H[_tree_swap]
G --> I[Serialize and enforce max_len]
H --> I
E --> I
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:12
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="src/fuzzer_tool/services/operators.py"line_range="1920-1925" />
<code_context>
+ # docs/web_research_port_candidates_2026-08.md #8) instead of+ # reparsing the whole corpus on every call.+ corpus = getattr(f, "corpus", None) or []+ next_idx = f._subtree_pop_next_idx+ if next_idx > len(corpus):+ next_idx = 0 # corpus was replaced/shrunk — restart harvesting+ for seed in corpus[next_idx:]:+ donor_tree = f._tree_mutator.parse(bytes(seed))+ f._subtree_population.add(donor_tree, rng=rng)+ f._subtree_pop_next_idx = len(corpus)+
</code_context>
<issue_to_address>
**issue (broader_impact):** When the corpus is replaced with a different corpus of the same length, `_subtree_pop_next_idx` remains equal to that length, so no new entries are harvested and the population continues supplying subtrees from the old corpus. When the corpus shrinks, resetting the index still leaves the old pools intact, so stale donors from removed seeds remain eligible indefinitely.
**Triggers:** When corpus minimization, seed transformation, or corpus synchronization replaces or removes entries without increasing the list length.
**Suggested fix:** Track the corpus identity/content generation and clear or rebuild the population whenever entries are replaced or removed, rather than using only the list length as the change detector.
</issue_to_address>
### Comment 2
<locationpath="src/fuzzer_tool/core/grammar.py"line_range="540-565" />
<code_context>
+ bounded, per-rule reservoir of interior nodes so ``TreeMutator``
</code_context>
<issue_to_address>
**issue (bug_risk):** The population stores no donor provenance, so `_tree_splice` can select a subtree harvested from the same corpus entry as the target. With a one-entry corpus whose input is being mutated, the matching root donor is the same serialized tree and operation 3 returns the original bytes instead of performing a crossover or using the productive fallback.
**Triggers:** When the corpus contains only one entry, or when the current input is also present in the corpus and the reservoir selects its subtrees.
**Suggested fix:** Store the source corpus entry with each donor and exclude donors from the current entry; fall back to `_tree_swap` when no donor from a different entry is available.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: src/fuzzer_tool/services/operators.py:1925, src/fuzzer_tool/core/grammar.py:565
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1920
to
+1925
| next_idx = f._subtree_pop_next_idx | ||
| if next_idx > len(corpus): | ||
| next_idx = 0 # corpus was replaced/shrunk — restart harvesting | ||
| for seed in corpus[next_idx:]: | ||
| donor_tree = f._tree_mutator.parse(bytes(seed)) | ||
| f._subtree_population.add(donor_tree, rng=rng) |
There was a problem hiding this comment.
issue (broader_impact): When the corpus is replaced with a different corpus of the same length, _subtree_pop_next_idx remains equal to that length, so no new entries are harvested and the population continues supplying subtrees from the old corpus. When the corpus shrinks, resetting the index still leaves the old pools intact, so stale donors from removed seeds remain eligible indefinitely.
Triggers: When corpus minimization, seed transformation, or corpus synchronization replaces or removes entries without increasing the list length.
Suggested fix: Track the corpus identity/content generation and clear or rebuild the population whenever entries are replaced or removed, rather than using only the list length as the change detector.
Comment on lines
+540
to
+565
| bounded, per-rule reservoir of interior nodes so ``TreeMutator`` | ||
| can splice in subtrees seen elsewhere in the corpus. | ||
| Reservoir sampling (Algorithm R) bounds memory to ``max_per_rule`` | ||
| nodes per rule regardless of corpus size, while still giving every | ||
| harvested node an equal chance of ending up in the pool. | ||
| """ | ||
| def __init__(self, max_per_rule: int = 64): | ||
| self.max_per_rule = max_per_rule | ||
| self._pools: dict[str, list[TreeNode]] = {} | ||
| self._seen: dict[str, int] = {} | ||
| def add(self, tree: TreeNode, rng=None) -> None: | ||
| """Harvest every interior node of *tree* into the population.""" | ||
| rand = rng or random | ||
| for node in tree.collect_interior(): | ||
| pool = self._pools.setdefault(node.rule, []) | ||
| seen = self._seen.get(node.rule, 0) | ||
| self._seen[node.rule] = seen + 1 | ||
| if len(pool) < self.max_per_rule: | ||
| pool.append(node) | ||
| continue | ||
| j = rand.randint(0, seen) | ||
| if j < self.max_per_rule: | ||
| pool[j] = node |
There was a problem hiding this comment.
issue (bug_risk): The population stores no donor provenance, so _tree_splice can select a subtree harvested from the same corpus entry as the target. With a one-entry corpus whose input is being mutated, the matching root donor is the same serialized tree and operation 3 returns the original bytes instead of performing a crossover or using the productive fallback.
Triggers: When the corpus contains only one entry, or when the current input is also present in the corpus and the reservoir selects its subtrees.
Suggested fix: Store the source corpus entry with each donor and exclude donors from the current entry; fall back to _tree_swap when no donor from a different entry is available.
There was a problem hiding this comment.
🟡 Changes recommended
Fix root replacement fallback and donor-cache invalidation before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds grammar-aware subtree-population crossover using corpus-derived donor trees.
Changes:
- Adds bounded subtree reservoirs and splice mutation.
- Integrates incremental corpus harvesting.
- Adds tests and documentation.
File summaries
| File | Summary |
|---|---|
tests/test_subtree_population_crossover.py | Adds crossover, reservoir, and integration tests. |
src/fuzzer_tool/services/operators.py | Integrates corpus harvesting; donor cache invalidation needs correction. |
src/fuzzer_tool/core/grammar.py | Implements reservoirs and splicing; root-only fallback can silently no-op. |
docs/web_research_port_candidates_2026-08.md | Updates research status. |
docs/DEEP_DIVE.md | Documents the feature. |
Review details
Suppressed comments (3)
src/fuzzer_tool/core/grammar.py:835
donor is targetcannot enforce the documented cross-entry splice: the target is parsed from the parent buffer, while donors are parsed separately fromf.corpus, so even the current seed's matching node is a different object. When the parent is itself in the corpus, op 3 can therefore select its own subtree and return an unchanged clone instead of crossing over; retain donor-seed identity (or exclude the current seed) when sampling.
donor = population.sample(target.rule, rng=rng)
if donor is None or donor is target:
src/fuzzer_tool/services/operators.py:1925
- This harvest includes the current parent seed, but the pool stores no donor provenance and
_tree_splicecan only compare object identity with the target. Becausefuzz_onepasses a corpus entry asdata, op 3 can select that entry's reparsed subtree and return an unchanged mutation, contrary to the documented cross-entry crossover. Track the source and exclude the current parent when sampling, including entries already cached.
for seed in corpus[next_idx:]:
donor_tree = f._tree_mutator.parse(bytes(seed))
f._subtree_population.add(donor_tree, rng=rng)
src/fuzzer_tool/services/operators.py:1925
- This newly added corpus-wide parse has no failure boundary.
TreeMutator.parserecursively descends nested braced input, so one sufficiently deep corpus seed can raiseRecursionErrorwhile an unrelated parent is being mutated and abort the fuzzing iteration. Harvest donor trees with a bounded or exception-safe parse and skip an unparseable donor.
for seed in corpus[next_idx:]:
donor_tree = f._tree_mutator.parse(bytes(seed))
f._subtree_population.add(donor_tree, rng=rng)
- 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.
Comment on lines
+822
to
+823
| if population is None or not len(population): | ||
| return self._tree_swap(tree, max_len) |
Comment on lines
+1920
to
+1922
| next_idx = f._subtree_pop_next_idx | ||
| if next_idx > len(corpus): | ||
| next_idx = 0 # corpus was replaced/shrunk — restart harvesting |
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.
Summary
Adds subtree-population crossover to the grammar
TreeMutator.Changes
Summary by Sourcery
Enable grammar tree mutation to exchange compatible subtrees across corpus entries.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: