✨ Add `circular` option to `fc.letrec` #6040

TomerAberbach · 2025-07-07T03:21:50Z

Description

Closes #6039

Checklist — Don't delete this checklist and make sure you do the following before opening the PR

The name of my PR follows gitmoji specification
My PR references one of several related issues (if any)
- New features or breaking changes must come with an associated Issue or Discussion
- My PR does not add any new dependency without an associated Issue or Discussion
My PR includes bumps details, please run pnpm run bump and flag the impacts properly
My PR adds relevant tests and they would have failed without my PR (when applicable)

Advanced

Category: ✨ Introduce new feature
Impacts: N/A

Open questions:

Is this really the best place/way to implement this? (see my comments on Add circular: true option to fc.letrec #6039)
See // TODO: Do we need to clone here? in the PR code. If I do need to clone, then what should I be using to do that? Answer: I think we don't need cloning here as long as we implement shrinking correctly
I have not been able to figure out how to implement the logic that throws if we accidentally don't replace some placeholderSymbol. I tried tracking them in a local set from nat().map(...) and then deleting them later when we replace, and asserting we have zero after, but this was causing problems, so I'm probably missing something. This seems like a critical feature in order to ship this
Neither assertProduceValuesShrinkableWithoutContext nor assertShrinkProducesSameValueWithoutInitialContext pass for circular: true. Does that make sense? Should they pass? If so, then what am I doing wrong?

changeset-bot · 2025-07-07T03:21:53Z

⚠️ No Changeset found

Latest commit: 2f7caab

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

pkg-pr-new · 2025-07-07T03:24:29Z

@fast-check/examples

@fast-check/ava

npm i https://pkg.pr.new/@fast-check/ava@6040

@fast-check/expect-type

npm i https://pkg.pr.new/@fast-check/expect-type@6040

fast-check

npm i https://pkg.pr.new/fast-check@6040

@fast-check/jest

npm i https://pkg.pr.new/@fast-check/jest@6040

@fast-check/packaged

npm i https://pkg.pr.new/@fast-check/packaged@6040

@fast-check/poisoning

npm i https://pkg.pr.new/@fast-check/poisoning@6040

@fast-check/vitest

npm i https://pkg.pr.new/@fast-check/vitest@6040

@fast-check/worker

npm i https://pkg.pr.new/@fast-check/worker@6040

commit: 2f7caab

codecov · 2025-07-07T03:28:38Z

Codecov Report

❌ Patch coverage is 92.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.95%. Comparing base (fc3afba) to head (2f7caab).

Files with missing lines	Patch %	Lines
packages/fast-check/src/arbitrary/letrec.ts	94.59%	6 Missing ⚠️
...trary/_internals/helpers/MaxLengthFromMinLength.ts	71.42%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6040      +/-   ##
==========================================
- Coverage   95.98%   95.95%   -0.04%     
==========================================
  Files         212      212              
  Lines       10194    10299     +105     
  Branches     2765     2798      +33     
==========================================
+ Hits         9785     9882      +97     
- Misses        409      417       +8

Flag	Coverage Δ
tests	`95.95% <92.00%> (-0.04%)`	⬇️
tests-against-22.x-on-macOS	`95.95% <92.00%> (-0.04%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

packages/fast-check/src/arbitrary/letrec.ts

TomerAberbach · 2025-07-13T01:57:46Z

@dubzzz would you also be able to take a look at my "open questions" in my PR description, and give your thoughts? #6040 (comment)

dubzzz · 2025-07-26T12:34:47Z

I'm not forgetting this PR. I just have a very very limited bandwidth at the moment. I promise I'll try to review and if needed provide you with feedback as soon as I find the time for it 🤗

dubzzz · 2025-07-26T12:37:51Z

In the meantime, you can install it via:

npm i https://pkg.pr.new/fast-check@6040

dubzzz

Awesome work! Thanks a lot for the time you spent on this PR. I'd have a few requests before merging it:

Drop any unmap code for now

Currently you provided an unmap code via poolsArb.map but none for nat().map, so it won't be enough. Let do it in a second time, it would probably help focusing the PR on the core feature.

Move derefPools into a dedicated file and put a small test on it
Factorizing letrecWithCycles and letrecWithoutCycles would be nice

I have the feeling that "without" one could stop at return strictArbs; while the "with" would have to add its pools.

packages/fast-check/test/unit/arbitrary/__test-helpers__/ArbitraryAssertions.ts

packages/fast-check/test/unit/arbitrary/letrec.spec.ts

packages/fast-check/src/arbitrary/letrec.ts

dubzzz · 2025-08-07T19:29:50Z

packages/fast-check/src/arbitrary/letrec.ts

+  for (const key in strictArbs) {
+    if (!safeHasOwnProperty(strictArbs, key)) {
+      // Prevents accidental iteration over properties inherited from an object’s prototype
+      continue;
+    }
+    const lazyAtKey: LazyArbitrary<unknown> | undefined = lazyArbs[key];
+    const lazyArb = lazyAtKey !== undefined ? lazyAtKey : new LazyArbitrary(key);
+    lazyArb.underlying = noShrink(nat().map((index) => ({ [placeholderSymbol]: { key, index } })));
+    lazyArbs[key] = lazyArb;
+    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
+    poolArbs[key] = array(strictArbs[key]!, poolConstraints);
+  }


Personal note: Close to the non-cycle one. The difference is that non-cycle one was doing lazyArb.underlying = strictArbs[key]. Plus the pool.

I fell that up to this loop we could have the two codes being the same. The "with cycles" one could just be a follow up code not returning immediately but doing the extra stuff with pools and cie.

packages/fast-check/src/arbitrary/letrec.ts

TomerAberbach · 2025-08-30T00:46:54Z

@dubzzz I think I've addressed things now (sorry, was busy for a while)

The one thing we might want to think about is catching the scenario where we we accidentally don't replace some placeholderSymbol. I tried tracking them in a local set from nat().map(...) and then deleting them later when we replace, and asserting we have zero after, but this was causing problems, so I'm probably missing something. Do you have any ideas around how this should work?

TomerAberbach · 2025-09-28T21:50:11Z

@gruhn since you seem to be around, curious if you have any thoughts on my comment above? I got a bit stuck on this point

TomerAberbach added 2 commits July 5, 2025 22:53

chore: update onlyBuiltDependencies

b797107

feat: letrec circular option

b924c68

TomerAberbach requested a review from dubzzz as a code owner July 7, 2025 03:21

test: add assertProduceCorrectValues

1568369

dubzzz reviewed Jul 9, 2025

View reviewed changes

packages/fast-check/src/arbitrary/letrec.ts Outdated Show resolved Hide resolved

dubzzz reviewed Jul 9, 2025

View reviewed changes

packages/fast-check/src/arbitrary/letrec.ts Show resolved Hide resolved

TomerAberbach added 3 commits July 12, 2025 21:44

fix: ignore built dependencies

6231a35

Merge branch 'dubzzz:main' into main

f7cdd37

Merge branch 'main' into feat/letrec-circular

a766dff

TomerAberbach requested a review from dubzzz July 13, 2025 02:11

TomerAberbach added 6 commits July 20, 2025 13:37

feat: shrinking improvements

6b98ec7

Merge branch 'main' into feat/letrec-circular

d57b84c

chore: get rid of commented out return

7d0427b

feat: frequency option

224ba73

chore: some more testing

bc21ce1

chore: remove unused var

c2e0ad9

dubzzz reviewed Aug 7, 2025

View reviewed changes

TomerAberbach added 3 commits August 29, 2025 20:15

chore: address most comments

25d6ea6

chore: test derefPools

1cda443

Merge branch 'main' into feat/letrec-circular

2f7caab

TomerAberbach requested a review from dubzzz September 28, 2025 20:27

Uh oh!

✨ Add circular option to fc.letrec #6040

Are you sure you want to change the base?

✨ Add circular option to fc.letrec #6040

Uh oh!

Conversation

TomerAberbach commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

pkg-pr-new bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

TomerAberbach commented Jul 13, 2025

Uh oh!

dubzzz commented Jul 26, 2025

Uh oh!

dubzzz commented Jul 26, 2025

Uh oh!

dubzzz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dubzzz Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

dubzzz Aug 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TomerAberbach commented Aug 30, 2025

Uh oh!

TomerAberbach commented Sep 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

✨ Add `circular` option to `fc.letrec` #6040

✨ Add `circular` option to `fc.letrec` #6040

TomerAberbach commented Jul 7, 2025 •

edited

Loading

changeset-bot bot commented Jul 7, 2025 •

edited

Loading

pkg-pr-new bot commented Jul 7, 2025 •

edited

Loading

codecov bot commented Jul 7, 2025 •

edited

Loading