Skip to content

Avoid reevaluating duplicate QuickCheck shrinks - #4150

Closed
mizchi wants to merge 2 commits into
moonbitlang:mainfrom
mizchi:quickcheck-deduplicate-shrink-candidates
Closed

Avoid reevaluating duplicate QuickCheck shrinks#4150
mizchi wants to merge 2 commits into
moonbitlang:mainfrom
mizchi:quickcheck-deduplicate-shrink-candidates

Conversation

@mizchi

@mizchi mizchi commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Adds an optional shrink_equal equivalence function to report and check. When supplied, the shrink driver tracks candidates it has already examined and skips repeated property/filter evaluation.

Duplicate candidates still count toward max_shrinks, preserving termination for infinite shrink streams. The default comparator treats candidates as distinct, so existing callers and types without Eq or Hash remain unaffected.

Tests cover both a self-repeating shrinker and an A -> B -> A cycle.

Validation: moon test --target all, moon check --target all, and moon info.

Copilot AI lite review requested due to automatic review settings August 24, 2026 18:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in mechanism to avoid re-evaluating duplicate shrink candidates during QuickCheck shrinking by introducing a shrink_equal equivalence function and tracking previously seen candidates in the shrink driver.

Changes:

  • Extend @quickcheck.report and @quickcheck.check with an optional shrink_equal : (A, A) -> Bool parameter.
  • Update the shrink driver to skip property/filter evaluation for candidates considered equivalent to any previously examined candidate.
  • Add regression tests covering a self-repeating shrinker and a 2-state shrink cycle.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
quickcheck/pkg.generated.mbti Updates the public interface signatures for report/check to include shrink_equal.
quickcheck/driver.mbt Implements deduplication in shrink_failure and threads shrink_equal through report and check (plus docs).
quickcheck/driver_test.mbt Adds tests ensuring equivalent/cyclic shrink candidates are not re-evaluated.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread quickcheck/driver.mbt Outdated
@bobzhang
bobzhang force-pushed the quickcheck-deduplicate-shrink-candidates branch from 585bbc7 to 7a4baa3 Compare August 26, 2026 07:39
@bobzhang

Copy link
Copy Markdown
Contributor

Thanks for the PR — the implementation is clean and well-tested. Before growing the check/report API surface, though, could you share a real-world case where this is measurably beneficial (e.g. a property where duplicate shrink evaluations cost noticeable time, with numbers)?

Our hesitation is that the benefit looks marginal in typical usage:

  • Shrinking only runs after a property is falsified, so it's a rare, terminal path — and duplicates still count toward max_shrinks, so dedup only skips evaluations rather than finding better counterexamples.
  • For cheap properties, the seen.any linear scan (plus retaining up to max_shrinks candidates) can cost more than the duplicate evaluations it avoids.
  • The A→B→A cycle case is arguably a bug in the Shrink instance — well-formed shrinkers produce strictly smaller candidates, which rules out cycles by construction.
  • Established implementations (Haskell QuickCheck, Hedgehog) have shipped without shrink dedup for years, which suggests duplicates aren't a significant cost in practice.

There's also a correctness footgun: a shrink_equal coarser than behavioral equivalence silently skips genuine smaller counterexamples.

If the motivating case is expensive (integration-style) properties with heavily overlapping shrink streams, that's worth supporting — but then a keyed design (shrink_key with O(1) lookup) would likely be the better API shape than an equivalence oracle, which forces the O(n²) scan. A concrete example would help us decide.

@bobzhang bobzhang added the question Further information is requested label Aug 26, 2026
@bobzhang bobzhang closed this Aug 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants