Try targeted candidates in explain phase before random sampling#4718
Open
Zac-HD wants to merge 4 commits intoHypothesisWorks:masterfrom
Open
Try targeted candidates in explain phase before random sampling#4718Zac-HD wants to merge 4 commits intoHypothesisWorks:masterfrom
Zac-HD wants to merge 4 commits intoHypothesisWorks:masterfrom
Conversation
Liam-DeVoe
reviewed
Apr 26, 2026
Comment on lines
+652
to
+657
| continue | ||
| try: | ||
| value = choice_from_index(idx, node.type, node.constraints) | ||
| except (NotImplementedError, AssertionError, IndexError): | ||
| break | ||
| if not choice_permitted(value, node.constraints): |
Member
There was a problem hiding this comment.
Instead of try/excepting, we should break if compute_max_children(node.type, node.constraints) < idx. As evidenced by these excepts, choice_from_index is not designed to handle semantically-invalid indexes nicely.
(independently, I'm surprised claude has decided that NotImplementedError can be thrown here)
When the explain phase tries to determine whether an arg slice can vary freely without affecting failure, it currently relies on pure random sampling. For tests like ``assert n1 == n2`` that misses the only passing value (the other arg's current value), so we'd report a misleading ``# or any other generated value`` comment. Before falling back to random sampling, we now try a few deterministic candidates: the minimal value, the smallest non-minimal value, and the values borrowed from each other arg slice with matching length and types. This catches the equality-comparison case, while leaving the existing behaviour for genuinely free arg slices.
The previous implementation included defensive paths (try/except around choice_from_index, choice_permitted guards, was_forced handling inside the borrow loop) that weren't reachable from the test suite, breaking the 100% line/branch coverage gate. Drop the idx=0 / idx=1 candidates -- random sampling already covers those, and the borrow-from-sibling path is what actually catches assert n1 == n2. The engine's existing handling of invalid attempts in the surrounding loop makes the choice_permitted guard redundant.
The CI check-format job runs shed against every file the branch touches and asserts no diff. Other formatters (e.g. ruff) reformat differently from shed, so running ./build.sh format before pushing is the only reliable way to keep the check green.
Zac-HD
force-pushed
the
claude/fix-issue-4715-uonex
branch
from
ccfdcb9 to
c31c869
Compare
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
3 participants
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.
When the explain phase tries to determine whether an arg slice can vary freely without affecting failure, it currently relies on pure random sampling. For tests like
assert n1 == n2that misses the only passing value (the other arg's current value), so we'd report a misleading# or any other generated valuecomment.Before falling back to random sampling, we now try a few deterministic candidates: the minimal value, the smallest non-minimal value, and the values borrowed from each other arg slice with matching length and types. This catches the equality-comparison case, while leaving the existing behaviour for genuinely free arg slices.
Fixes #4715.