Skip to content

fix(dummies): gate distinct collections by the effective element domain - #199

Merged
Reefact merged 7 commits into
mainfrom
claude/issue-188-discussion-nxi3ja
Jul 19, 2026
Merged

fix(dummies): gate distinct collections by the effective element domain#199
Reefact merged 7 commits into
mainfrom
claude/issue-188-discussion-nxi3ja

Conversation

@Reefact

@Reefact Reefact commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

Fix issue #188: a distinct collection (SetOf, ListOf(...).Distinct(), dictionary keys) gated its element count against the element generator's cardinality alone, so it rejected satisfiable requests whose Containing(...) values lie outside that domain — e.g. Any.SetOf(Any.Int32().OneOf(1, 2)).Containing(3).WithCount(3), where {1, 2, 3} is reachable. This models the effective distinct domain instead, and completes the "hold the eager promise everywhere" work started in ADR-0013 by bringing every finite-domain generator into the same cardinality contract.

Type of change

  • Bug fix
  • New feature — extends eager cardinality gating to the decimal, floating-point and 128-bit finite generators
  • Breaking change — this PR is not an API break; the eager extension turns a few generation-time shortfalls into declaration-time conflicts
  • Refactoring — merge ICardinalityHint + IDomainMembership into one interface
  • Analyzer / diagnostic change
  • Tests
  • Documentation

Changes

  • Fix the false rejection (Dummies: Distinct collection cardinality rejects valid contained values outside the item domain #188). CollectionState now counts only the elements that must be drawn from the generator: it subtracts the Containing(...) values proven outside the domain (and each opaque ContainingAny(...) draw) from the required count, and raises the resolution cap by the same out-of-domain values. The check is written subtractively so it cannot overflow for a near-long.MaxValue domain, and membership is decided under the generator's own equality so it stays a sound upper bound under a custom comparer. A contained value already inside the domain still does not inflate capacity, so genuinely impossible requests keep failing eagerly; an unprovable overlap defers to the bounded generation-time draw rather than a false conflict.

  • Unify cardinality and membership into one interface. ICardinalityHint becomes generic ICardinalityHint<T>, carrying both DistinctCardinality and Contains(T). A finite-cardinality generator is now compiler-forced to answer membership, so the pairing cannot silently drift out of the eager perimeter. The interim IDomainMembership<T> is removed.

  • Hold the promise across the whole knowable perimeter. The six generators that silently sat outside it are brought in — AnyDecimal, AnyDouble, AnySingle, AnyHalf, AnyInt128, AnyUInt128. Their specs (DecimalIntervalSpec, ContinuousIntervalSpec, WideIntervalSpec) now advertise a finite cardinality (allow-lists, and narrow 128-bit ranges) and answer membership; continuum ranges and strings stay out, deferring to the bounded draw as before. OrdinalIntervalSpec gains the matching Contains.

  • Tests in AnyCollectionTests: out-of-domain values extending the effective domain; in-domain values not inflating it; order-independence of Distinct/Containing/count; near-maximum cardinality without overflow; conservative ContainingAny deferral; a merging comparer collapsing an out-of-domain value back into the domain; and the eager perimeter now reaching the decimal, floating-point and 128-bit generators.

  • Documentation: ICardinalityHint and CollectionState XML docs updated to the unified contract and the effective-domain model; README.nuget.md wording clarified.

Testing

  • dotnet build FirstClassErrors.sln
  • dotnet test FirstClassErrors.sln
  • Analyzer tests pass (FirstClassErrors.Analyzers.UnitTests)

All green — 0 warnings; Dummies.UnitTests 162 passed, and the rest of the solution (analyzers 85, core 423, request-binder, gendoc 169, cli 64, …) with 0 failures.

Documentation

  • Public API / error documentation updated — XML docs on the unified ICardinalityHint and CollectionState
  • README / doc/ updated — README.nuget.md; ADR-0013 refined and accepted
  • French translation (doc/handwritten/for-users/README.fr.md) updated if user-facing behavior changed — n/a: that user page does not cover distinct collections; the ADR-0013 French mirror is kept in sync
  • No documentation change required

Architecture decisions

  • No architectural decision in this pull request
  • New decision recorded — ADR drafted as Proposed: ADR-____
  • Supersedes an existing ADR — successor proposed, status not flipped: ADR-____
  • ⚠️ Conflicts with an existing ADR — flagged for the maintainer: ADR-____

ADR-0013 (gate distinct collections by cardinality, otherwise by a bounded draw) is refined and accepted in this PR: the rationale is corrected — the earlier draft claimed the eager check "never rejects a satisfiable request", which #188 disproved — and the maintainer accepted it here (Status: Accepted).

ADR-0023 (prune the exotic-width numeric generators — 128-bit and Half) was drafted during this work as a Proposed question, but the maintainer decided to keep Any.Int128/Any.UInt128/Any.Half. The draft has been withdrawn; no generator code changed.

Related issues

Closes #188

claude added 3 commits July 19, 2026 21:07
A distinct collection (SetOf, ListOf(...).Distinct(), dictionary keys)
gated its element count against the element generator's cardinality
alone, so it rejected satisfiable requests whose Containing(...) values
lie outside that domain -- e.g. Any.SetOf(Any.Int32().OneOf(1, 2))
.Containing(3).WithCount(3), where {1, 2, 3} is reachable.

Model the effective distinct domain instead. A new internal
IDomainMembership<T> lets a generator answer whether a value is one it
could produce; every finite-cardinality generator now implements it.
CollectionState counts only the elements that must come from the
generator: it subtracts the contained values proven outside the domain
(and each opaque ContainingAny draw) from the required count, and raises
the resolution cap by the same out-of-domain values. A value already
inside the domain still does not inflate capacity, so genuinely
impossible requests keep failing eagerly; an unprovable overlap defers
to the bounded generation-time draw rather than a false conflict.

The check is written in subtractive form so it cannot overflow for a
near-long.MaxValue element domain, and membership is decided under the
generator's own equality so it stays a sound upper bound under a custom
comparer.

Refine the still-Proposed ADR-0013 (its context and the rationale claim
that the eager check "never rejects a satisfiable request", which this
case disproved) and the user-facing wording, and add tests across SetOf,
ListOf().Distinct(), custom comparers and order-independent declaration.

Refs: #188

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L1JGj1wj5C55UE9vxQFWoF
The comment claimed the out-of-domain scenario was exercised "for
dictionary-shaped keys", but no dictionary is constructed and none can
be: AnyDictionary exposes no Containing surface, so its keys are gated
purely by count. Say instead that dictionaries share the same
CollectionState path and are therefore covered transitively.

Refs: #188

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L1JGj1wj5C55UE9vxQFWoF
A distinct collection (SetOf, ListOf(...).Distinct(), dictionary keys)
gated its element count against the element generator's cardinality
alone, so it rejected satisfiable requests whose Containing(...) values
lie outside that domain -- e.g. Any.SetOf(Any.Int32().OneOf(1, 2))
.Containing(3).WithCount(3), where {1, 2, 3} is reachable.

Model the effective distinct domain instead. A generator now answers
both "how many distinct values can I produce" and "is this one of them"
through a single unified interface ICardinalityHint<T> (cardinality plus
membership), so a fixed value proven outside the domain extends the
effective cardinality while one already inside it does not.
CollectionState counts only the elements that must come from the
generator: it subtracts the contained values proven outside the domain
(and each opaque ContainingAny draw) from the required count, and raises
the resolution cap by the same out-of-domain values. Genuinely
impossible requests still fail eagerly; an unprovable overlap defers to
the bounded generation-time draw rather than a false conflict. The check
is subtractive so it cannot overflow for a near-long.MaxValue domain,
and membership is decided under the generator's own equality so it stays
a sound upper bound under a custom comparer.

Merging cardinality and membership into one interface makes the pairing
compiler-enforced: a finite-cardinality generator cannot drift out of
the eager perimeter without failing to compile. Hold that promise across
the whole knowable perimeter by bringing every finite generator into it,
including the six that silently sat outside before -- Int128, UInt128,
Decimal, Double, Single and Half -- whose specs now advertise a finite
cardinality (allow-lists, and narrow 128-bit ranges) and answer
membership; continuum ranges and strings stay out, deferring to the
bounded draw as before.

Refine the still-Proposed ADR-0013 to the unified capability, and add
ADR-0023 (Proposed) asking whether the exotic-width generators
(Int128/UInt128 and their dedicated engine, and Half) should remain in a
dummies library at all -- a breaking product decision left to the
maintainer. Add tests across SetOf, ListOf().Distinct(), custom
comparers, order-independent declaration, near-maximum cardinality, and
the newly-gated decimal, floating-point and 128-bit generators.

Refs: #188

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L1JGj1wj5C55UE9vxQFWoF
@Reefact
Reefact force-pushed the claude/issue-188-discussion-nxi3ja branch from 2876bca to 5794263 Compare July 19, 2026 21:14
@Reefact Reefact changed the title feat(dummies): extend cardinality perimeter to floating-point and 128-bit allow-lists fix(dummies): gate distinct collections by the effective element domain Jul 19, 2026
@Reefact

Reefact commented Jul 19, 2026

Copy link
Copy Markdown
Owner Author

@codex Please perform an in-depth review of the code introduced or modified by this pull request, which implements a new feature.

The objective is to determine whether the feature is correctly designed, correctly implemented, sufficiently tested, and consistently integrated into the existing project.

Review scope

Start by reading:

  1. the pull request description;
  2. any referenced issues, specifications, or ADRs;
  3. the relevant functional and technical documentation;
  4. the repository conventions, including CLAUDE.md, AGENTS.md, contribution guides, and applicable architectural rules;
  5. the complete pull request diff;
  6. any existing code required to understand how the feature integrates with the rest of the system.

Do not review the diff in isolation. Verify that the changes are consistent with the abstractions, behaviors, contracts, and conventions already established in the project.

Review areas

1. Requirements and expected behavior

Verify that the implementation actually satisfies the described requirements.

Look specifically for:

  • missing or only partially implemented requirements;
  • behaviors introduced without being specified;
  • ambiguities resolved in a questionable way;
  • unhandled edge cases;
  • discrepancies between the expected behavior, documentation, tests, and implementation.

Do not treat a behavior as incorrect merely because a different design would also have been possible. Report an issue only when there is a demonstrable contradiction, omission, defect, or concrete risk.

2. Design and architecture

Evaluate:

  • consistency with the existing architecture;
  • separation of responsibilities;
  • newly introduced dependencies;
  • coupling and cohesion;
  • the quality and stability of public abstractions;
  • enforcement of domain invariants;
  • error handling;
  • consistency with existing patterns;
  • meaningful duplication;
  • premature abstractions;
  • classes or components carrying too many responsibilities.

Also verify that the feature does not introduce a second competing way to achieve an existing operation without an explicit and justified reason.

3. Public API and developer experience

For every public API, fluent API, extension method, overload, or generic abstraction added or modified, review:

  • naming clarity;
  • consistency with existing APIs;
  • generic type inference;
  • generic constraints;
  • nullability;
  • overload resolution and ambiguity risks;
  • compilation failures for otherwise legitimate usages;
  • IntelliSense discoverability;
  • error-message quality;
  • source and binary compatibility;
  • surprising or difficult-to-predict behavior.

Mentally validate several realistic usage scenarios, including edge cases and invalid usages.

4. Code correctness

Look for concrete defects, including:

  • logical errors;
  • incorrect conditional branches;
  • incorrectly propagated values;
  • incomplete null handling;
  • unexpected exceptions;
  • type conversion, variance, or generic inference issues;
  • incorrect behavior for empty collections;
  • execution-order problems;
  • uncontrolled mutable state;
  • unintended non-deterministic behavior;
  • invalid assumptions about inputs;
  • concurrency or thread-safety issues, where relevant;
  • significant performance or allocation problems.

Do not report micro-optimizations without a realistic impact.

5. Tests

Assess whether the tests genuinely demonstrate the intended behavior of the feature.

Verify coverage of:

  • nominal scenarios;
  • expected failures;
  • edge cases;
  • likely regressions;
  • null values and empty collections;
  • relevant generic variants;
  • deterministic behavior;
  • interactions between new components and existing code;
  • backward-compatibility guarantees.

Identify tests that may pass without actually proving the announced behavior, as well as tests that are excessively coupled to implementation details.

Do not mechanically request a test for every line or method. Recommend tests only when they protect an important behavior or an identified risk.

6. Documentation

Verify that:

  • the documentation describes the behavior actually implemented;
  • examples are conceptually compilable and use valid APIs;
  • signatures and names are up to date;
  • French and English documentation remain consistent when both are affected;
  • important limitations and design choices are documented;
  • no misleading, obsolete, or technically incorrect statement has been introduced.

Do not criticize writing style unless it creates a real technical ambiguity or communicates incorrect behavior.

7. Regressions and integration

Analyze the impact on existing code, including:

  • source or binary compatibility breaks;
  • unintended changes to existing behavior;
  • semantic changes to an existing API;
  • impact on downstream consumers;
  • interactions with analyzers, generators, renderers, publishers, or other affected modules;
  • newly introduced compiler warnings;
  • nullability regressions;
  • performance or memory consequences;
  • inconsistencies between packages or modules.

Do not perform a general audit of the entire repository. Inspect existing code only when it is necessary to evaluate the changes introduced by this pull request.

Expected output

Start with a concise summary containing:

  • the overall assessment;

  • the confidence level in the implementation;

  • the main identified risks, if any;

  • an explicit conclusion using one of the following:

    • ready to merge;
    • ready after minor corrections;
    • significant corrections required.

Then report only genuinely actionable findings, classified using these severity levels:

  • Blocking: bug, contract violation, security issue, data loss, confirmed regression, or substantially incorrect functionality;
  • Important: a real issue that should be corrected before merge but does not make the entire feature unusable;
  • Useful improvement: a justified improvement that is not required before merge.

For each finding, include:

  1. the affected file and lines;
  2. the current behavior;
  3. the concrete problem;
  4. a scenario demonstrating or explaining the risk;
  5. the recommended correction;
  6. the test that should protect the correction, when relevant.

Group findings that share the same root cause instead of reporting multiple variations of the same issue.

After the findings, add a separate section for open questions. Do not present a question as a defect unless the answer establishes that an actual issue exists.

Finish with:

  • aspects that were particularly well implemented;
  • requirements that were correctly implemented and covered;
  • residual risks that cannot be confirmed through static code review alone.

Review rules

  • Do not invent problems merely to produce a longer review.
  • Do not recommend a complete rewrite when a local correction is sufficient.
  • Do not criticize stylistic choices already accepted by the repository.
  • Do not request refactoring without a concrete benefit for this feature.
  • Do not present personal preferences as project requirements.
  • Do not report hypothetical risks without explaining precisely how they could occur.
  • Verify every finding before reporting it.
  • Prefer a small number of strong, actionable findings over a long list of minor remarks.
  • Do not modify any files. This task is strictly a code review and analysis of the pull request.
  • Use the highest available reasoning depth.
  • Do not conclude before reviewing the complete diff and the main execution paths affected by the feature.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5794263d6f

ℹ️ 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".

Comment thread Dummies/ContinuousIntervalSpec.cs Outdated
Comment thread Dummies/DecimalIntervalSpec.cs Outdated
claude added 4 commits July 19, 2026 21:29
DecimalIntervalSpec and ContinuousIntervalSpec only reported a finite
cardinality for their allow-list branch, so a validated pin such as
Any.Decimal().Zero() or Any.Double().Between(1, 1) -- a singleton domain
of exactly one value -- was left outside the eager perimeter:
Any.SetOf(Any.Decimal().Zero()).WithCount(2) passed declaration and
failed only later during the bounded draw. Report 1 for a validated pin
(_min == _max) so the fully knowable contradiction conflicts eagerly,
and cover it with a distinct-collection test.

Refs: #188

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L1JGj1wj5C55UE9vxQFWoF
Codex noted that finite floating-point ranges (a narrow Half/Single
sub-range, Half's ~63k full domain) sit outside the eager perimeter.
That is intentional (Option A): counting a narrow float type's
representable values in a range is type-specific bit-arithmetic
disproportionate to the dummy use case, the case is pathological, and
the bounded seed-named draw already covers it. Make the boundary
explicit in ADR-0013's two-groups paragraph (EN + FR): pools, narrow
integer/time ranges, allow-lists and single-value pins are counted
cheaply and gated eagerly; a floating-point range defers, so a decimal
or floating-point generator is gated only through an allow-list or a pin.

Refs: #188

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L1JGj1wj5C55UE9vxQFWoF
Flip ADR-0013 (gate distinct collections by cardinality, otherwise by a
bounded draw) from Proposed to Accepted at the maintainer's decision, and
set its date to the day it reached that status, per the index convention.
Update the ADR index row to match.

Refs: #188

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L1JGj1wj5C55UE9vxQFWoF
The maintainer decided to keep Any.Int128, Any.UInt128, and Any.Half
rather than prune them, preserving the "Any covers every numeric
primitive" orthogonality. The proposal never reached main, so remove the
never-accepted draft and its index row; no generator code changes.

Refs: #188

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L1JGj1wj5C55UE9vxQFWoF
@Reefact
Reefact merged commit 30430da into main Jul 19, 2026
15 checks passed
@Reefact
Reefact deleted the claude/issue-188-discussion-nxi3ja branch July 19, 2026 22:06
Reefact added a commit that referenced this pull request Jul 19, 2026
Rebased onto main after PR #199 merged a substantive rewrite of this
ADR's decision content; folds that content into the editorial trim so
neither PR's work is lost.
Reefact added a commit that referenced this pull request Jul 19, 2026
Rebased onto main after PR #199 merged a substantive rewrite of this
ADR's decision content; folds that content into the editorial trim so
neither PR's work is lost.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dummies: Distinct collection cardinality rejects valid contained values outside the item domain

2 participants