fix(dummies): qualify exhaustion replay hint for foreign generators#278
Merged
Conversation
A distinct collection whose element generator is foreign (carries no IHasRandomSource) falls back to the ambient source for its count and layout, but the foreign generator's own draws ignore that seed. The exhaustion message nonetheless appended the faithful "reproduce this run with Any.Reproducibly(seed, ...)" hint, so a user who replayed the seed got different elements — and not even the same shortfall — the exact trust failure the seed reporting exists to prevent. Thread the failing generator into CollectionState.Exhausted and key the hint on AnyDerivation.SourceOf(culprit): when it is null — a foreign IAny, or an As/Combine derivation built over one — emit a qualified hint that scopes the promise to the count and layout. The discriminator is the null source, not an "is IHasRandomSource" test: DerivedAny carries a null source over a foreign operand and must be qualified too. Both exhaustion sites are covered — the element generator (FillDistinct) and a ContainingAny(...) generator (DrawFresh) — keyed on that specific generator's source. The qualified phrasing lives on RandomSource (PartialReplayHint, a twin of ReplayHint) so it names Any.WithSeed(seed) when the collection's own source is fixed, never the inapplicable Any.Reproducibly, staying consistent with #211. Library-sourced and As-over-library generators are unchanged: they stay faithful and keep the full-replay hint. The dead null-guard the issue also flagged was already removed by #211. Add four tests covering the foreign, As-over-foreign, foreign-ContainingAny-over-fixed-source, and unchanged-library cases. Refs: #212 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tME7DBWLTD7rSa5TWtodi
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aea665b375
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A Combine derivation with at least one foreign operand keeps a sourced operand's non-null source through the SourceOf(first) ?? ... collapse, so the previous discriminator — a null source — classified it as faithful and still promised a full Any.Reproducibly replay, even though the composed value follows the foreign draw. This is the same over-promise one level up, surfaced in review of #212. Add AnyDerivation.IsReproducible: a generator is reproducible when it draws only from its reported source — a library generator with a source, or a derivation whose operands are all themselves reproducible. Record it on DerivedAny (IReproducibilityHint) at construction from the operands, computed in As, Combine (arities 2-8) and OrNull. A non-null source to name and full reproducibility are now distinct signals. Route both replay sites through the predicate: distinct-collection exhaustion (CollectionState.Exhausted) and composer/factory failures (AnyDerivation.Invoke). Generalize the qualified hint on RandomSource (PartialReplayHint) so it phrases both the collection-element and the composed-value case, per source (Any.Reproducibly or Any.WithSeed). Add tests for the exhaustion and composer-failure paths over a Combine that mixes a foreign operand with a library one. Refs: #212 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018tME7DBWLTD7rSa5TWtodi
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
2 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.
Summary
When a distinct collection is driven to exhaustion and its element generator is foreign (carries no
IHasRandomSource), the failure message no longer promises a fullAny.Reproduciblyreplay of the elements. The collection falls back to the ambient source for its count and layout, but a foreign generator's own draws ignore that seed — so replaying it reproduced neither the elements nor the shortfall. The hint is now scoped to the count and layout instead, honoring ADR-0013's "fails explicitly and reproducibly."Type of change
Changes
RandomSource: addPartialReplayHint(seed)(abstract + ambient/fixed implementations). It states the count and layout are reproducible but the elements are not, phrased per source — the ambient source namesAny.Reproducibly(seed, ...), a fixed context namesAny.WithSeed(seed).CollectionState.Exhausted: take the generator that exhausted and key the hint onAnyDerivation.SourceOf(culprit) is null. When it is null — a foreignIAny, or anAs/Combinederivation built over one (DerivedAnycarries a null source there) — emit the qualified hint; otherwise keep the faithful full-replay hint unchanged. The discriminator is the null source, not anis IHasRandomSourcetype test.FillDistinct→_item) and aContainingAny(...)generator (DrawFresh→ that operand).AnyCollectionTests— foreign element generator,As-over-foreign, foreignContainingAnyover a fixedAny.WithSeed(...)source (assertsAny.WithSeedis named, never the inapplicableAny.Reproducibly), and the unchanged library case — plus aForeignPairtest double.Note: the dead
seed is not nullguard the issue also flagged was already removed by #211; the scope here is the over-promise only. See the issue comment for the two acceptance-criteria refinements (theAs-derived criterion and the addedContainingAnysite).Testing
dotnet build FirstClassErrors.sln— 0 warnings, 0 errors.dotnet test FirstClassErrors.sln— not run as a whole; randotnet test Dummies.UnitTests→ 380 passed, 0 failed (including the 4 new tests, confirmed via filter).FirstClassErrors.Analyzers.UnitTests) — not run; analyzers are untouched.Documentation
doc/updateddoc/handwritten/for-users/README.fr.md) updated if user-facing behavior changedArbitraryTestValues), which this change aligns the message with. No README behavior changed, so the French translation is unaffected.Architecture decisions
Proposed: ADR-____Related issues
Closes #212
Generated by Claude Code