[thomhurst]

Typed rows such as [Arguments<int[]>] now infer T = int for Run<T>(IEnumerable<T>). Source-generated and reflection discovery traverse implemented interfaces and base classes, including nested generic shapes. All matching constructions must agree on each binding; conflicting bindings produce no declared-row inference, independent of interface order. Repeated identical bindings and interface diamonds are accepted.

The analyzer closes array and constructed generic parameter types using the same inference before checking row compatibility. Added parity cases, analyzer positive/negative cases, a reviewed generator snapshot, and targeted execution regressions. Updated the earlier explicit-generic regression's comment to reflect the new inference.

Closes #7466

Validation on macOS arm64, SDK 10.0.403:

• Full core generator suite, net10.0: 265 passed, 1 skipped (includes 26 inference parity cases and the new snapshot/analyzer cases).
• Full analyzer suite, net10.0: 965 passed, 1 skipped.
• Targeted AssignableTypedRowTests: 2 passed in each of generated and reflection modes.
• Existing TypedCollectionRowWithGenerateGenericTest: 1 passed in each mode.
• Release Native AOT publish succeeded; published executable passed both new targeted tests.
• git diff --check passed. No public API changes.

Only .NET 10 is installed locally; other framework legs remain for CI. The engine test suite was not run per repository instructions. Publish emits existing unrelated trimming/AOT warnings; the core generator restore's known Pkcs vulnerability warning is tracked in #7412.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 8 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: baf4672a-66c1-480e-9dbf-08d5375c0d73

Commits

Reviewing files that changed from the base of the PR and between b5561fb and ebbd9f9.

Files selected for processing: 10

• src/TUnit.Analyzers/Extensions/TypeExtensions.cs
• src/TUnit.Analyzers/TestDataAnalyzer.cs
• src/TUnit.Core.SourceGenerator/Extensions/TypedDataSourceInference.cs
• src/TUnit.Engine/Discovery/ReflectionTestDataCollector.cs
• tests/TUnit.Core.SourceGenerator.Tests/AssignableTypedRowAnalyzerTests.cs
• tests/TUnit.Core.SourceGenerator.Tests/AssignableTypedRowTests.Interfaces_and_base_types_emit_closed_tests.verified.txt
• tests/TUnit.Core.SourceGenerator.Tests/AssignableTypedRowTests.cs
• tests/TUnit.Core.SourceGenerator.Tests/TypedRowInferenceParityTests.cs
• tests/TUnit.TestProject/Bugs/_7214/TypedArgumentsRowOnGenericMethodTests.cs
• tests/TUnit.TestProject/Bugs/_7466/AssignableTypedRowTests.cs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

CodeRabbit Verify

• Run CodeRabbit Verify to test the changes in this pull request.

[thomhurst]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-03T15:07:22.095855Z	ebbd9f9	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Review: PR 7508 (assignable typed-row inference)

I found no blocking issues. The change looks correct and well tested.

What I checked

• The source generator (MapTypeParameters) and reflection discovery (MapTypedRowParameters) use the same algorithm. Each walks the base-class chain and every implemented interface, and requires all matching constructions to produce the same binding. Both loops cover AllInterfaces and GetInterfaces, so the result doesn't depend on metadata order. The parity tests include Ambiguous and AmbiguousReversed for exactly this.
• Because the candidate has the same generic definition as the parameter, the type-argument counts match. The removed i < Length guards were therefore redundant, not a lost safety check.
• Diamonds and repeated identical bindings are accepted, and conflicts return null. That fits the existing "each to one type" contract.
• Test coverage is good: parity cases for nested shapes (IEnumerable<IEnumerable<T>> over List<int[]>), an analyzer positive/negative test, a reviewed generator snapshot, and execution tests in both modes.
• The reflection path uses the existing AssemblyReferenceCache.GetInterfaces. I saw no new AOT or trimming concerns in the diff. The PR says a Native AOT publish succeeded.

Non-blocking suggestions

1. The two MapMatchingType local functions duplicate the same "walk bases, then walk interfaces" structure in two files. A short comment cross-referencing each other would help keep them in sync. The existing doc comment on the generator side already mentions the reflection counterpart, and the parity test guards against drift.
2. The interface lookup calls AllInterfaces or GetInterfaces for every generic parameter that is mapped. This only runs during discovery, but if it ever shows up in profiles, you could compute the interface set once per row type per call and reuse it across parameters.
3. The Roslyn side compares type arguments with the default SymbolEqualityComparer, which ignores nullable annotations. A row such as List<string?> therefore binds the same as List<string>. That is probably what you want, but a parity case would pin it down.

Good to merge once CI is green on the other target frameworks.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f32e16da2

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

[thomhurst]

@codex review

Fixed the containing-type inference finding in 79f52e3. Full net10.0 generator suite: 269 passed, 1 skipped; analyzer suite: 965 passed, 1 skipped. No snapshot changes; all 30 generator/reflection parity cases pass.

Disposition of Claude’s non-blocking suggestions: the existing generator documentation already identifies the reflection counterpart, and parity tests enforce synchronization. Interface caching remains a profiling-driven optimization; no performance change is justified here. Nullable annotation equality intentionally follows runtime type identity (reflection does not retain nullable annotations in Type); no inference semantic change is needed.

The PR remains draft for fresh reviews of this head. Other framework validation remains for CI; no reflection execution path or public API changed in this fix.

[github-actions]

Review of #7508

I found no blocking issues. The change does what it describes.

What I checked

• Order independence. The old code matched only the first construction of a generic type. The new code visits every base type and every interface of the row type and requires all matches to agree. That makes Ambiguous and AmbiguousReversed both infer nothing, and the parity tests cover both orders. Diamonds and repeated identical bindings still succeed.
• Generator and reflection parity. MapMatchingType has the same structure in TypedDataSourceInference and in MapTypedRowParameters. The reflection side uses AssemblyReferenceCache.GetInterfaces, which suits the AOT and trimming constraints. The nested generic case is handled too: Roslyn keeps outer type arguments on ContainingType, while reflection folds them into GetGenericArguments.
• Arrays. int[] has no INamedTypeSymbol base type, but AllInterfaces supplies IEnumerable<int>, so Run<T>(IEnumerable<T>) infers T = int.
• Analyzer. In t is IArrayTypeSymbol or INamedTypeSymbol && ..., the or stays inside the pattern, so the && applies to the whole pattern as intended. Closing the method over the inferred types before the compatibility check fixes the false positive. The positive and negative analyzer cases cover it.
• Tests and snapshot. The snapshot is reviewed and committed as .verified.txt. The parity tests, the new engine tests and the update to the older Source-gen drops an [Arguments<T>] row on a generic test method when another row infers the same types; reflection runs it #7214 comment are consistent with the behaviour change.

Non-blocking suggestions

1. Duplicated algorithm. The two MapMatchingType implementations are near copies and the parity test is what keeps them aligned. That is acceptable given the two backends. If the inference rules grow further, consider one shared rule description or a table-driven test that runs both backends over the same cases, which the parity test already does for 26 cases.
2. Interface walk cost. AllInterfaces and GetInterfaces are walked for every generic parameter on every row. Discovery is a hot path, so a benchmark on a large typed-row suite would be worth running. Reflection mode already caches interfaces, so I expect the cost to be small.
3. Variance. Inference binds to the exact constructed type argument. For IEnumerable<T> a, T b with row (List<string>, object), this reports a conflict rather than inferring object through covariance. The conservative result is fine, but a short comment saying it is intentional would help.

Overall this is a clean, well-tested change. Approving from my side, pending CI on the other target frameworks.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 79f52e3b6e

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

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; the previous nested-type build failure is fixed.

Summary

Typed rows now infer method type arguments through interfaces and base classes in generated and reflection discovery.

• Typed rows infer method types through base classes and interfaces.
• The analyzer closes generic parameter types before checking typed rows.

The previous nested-type finding is fully addressed. No new actionable issues were found in the changes since that review.

_{Reviews (2) · Last reviewed commit: "Fix nested containing-type analyzer chec..." · Reviewed by Greptile}

[thomhurst]

@codex review

Fixed the nested containing-type analyzer finding in ebbd9f9. Both helper checks now recurse through containing types. The nested-base positive regression failed before the fix and passes afterward; nested generic positive and invalid-row cases also pass. Full net10.0 generator suite: 272 passed, 1 skipped; analyzer suite: 965 passed, 1 skipped. Snapshots unchanged; git diff --check passed. Other framework legs remain for CI.

Claude suggestions disposition: parity tests continue to guard the two backends; interface caching remains profiling-driven. Exact binding rather than covariance merging remains the existing conservative inference policy.

Remaining draft for fresh reviews of this head.

[github-actions]

Review

Reviewed the diff for #7508. It looks good and I found no blocking issues.

What I checked

• Source and reflection parity: both paths now walk the base chain and all implemented interfaces, and both require every matching construction to agree. Interface order therefore can't change the result. The TypedRowInferenceParityTests cases cover this, including Ambiguous and AmbiguousReversed.
• Nested generics: the source generator maps ContainingType separately, while reflection's GetGenericArguments already includes the outer arguments. The comment explains why, and the Outer<T>.Base cases exercise it.
• Arrays: int[] isn't an INamedTypeSymbol, so the base-type loop skips it. AllInterfaces still yields IEnumerable<int>, which is the case the PR targets.
• Analyzer: t is IArrayTypeSymbol or INamedTypeSymbol && … parses as t is (A or B) && …, as intended. The ContainingType additions to IsGenericDefinition and ReferencesMethodTypeParameter are consistent with the inference change.
• Tests: there are positive and negative analyzer cases, a reviewed snapshot and targeted execution regressions.

Suggestions (non-blocking)

1. The two MapMatchingType local functions repeat the same algorithm in Roslyn and reflection form. That's reasonable given the two type systems. A short comment in each pointing at the other would help keep them in sync.
2. AllInterfaces and GetInterfaces run on every generic parameter of every typed row. This only affects discovery and is probably negligible. If it ever shows up in profiles, short-circuit when the parameter type's definition is sealed or not an interface or base of the value type.

I couldn't run the tests here. Please confirm the multi-framework CI legs pass, since the PR notes only .NET 10 was tested locally.

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: ebbd9f94ca

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

[thomhurst]

Marked ready for review at unchanged head ebbd9f9 after confirming positive Claude and Codex verdicts and all review threads resolved. The latest non-blocking suggestions have the same disposition already recorded: existing backend documentation and parity tests guard synchronization; interface caching remains profiling-driven.

Waiting for the ready-for-review Claude run 41660232844 and multi-framework CI. Auto-merge has not been armed: the fresh Claude run must pass and its review must be read first.

[github-actions]

Review: PR 7508 (infer T from typed rows through interfaces and base types)

I read the source-generator inference in TypedDataSourceInference.cs and the reflection counterpart in ReflectionTestDataCollector.cs. I did not run the tests. I did not read the analyzer or test diffs line by line.

What looks good

• The two modes follow the same algorithm, so the AGENTS.md requirement of identical behaviour in both is met. Both walk the base-type chain and then the interfaces, and both require every matching construction to agree. Conflicting bindings give no inference, whatever order the interfaces come in. Repeated identical bindings, such as interface diamonds, are accepted.
• Handling for nested generics is explicit. Roslyn keeps the outer type arguments on ContainingType while reflection folds them into GetGenericArguments, and the code accounts for that.
• The change adds parity tests, analyzer positive and negative cases, a reviewed generator snapshot, and targeted execution tests in both modes. The PR description also reports a Native AOT publish check.

Suggestions (non-blocking)

1. In the source-generator version, MapMatchingType is a local function declared after its use inside the loop. It reads well enough, but extracting a private static helper that takes method, inferred and parameterNamed would drop the closure allocation. This is only worth doing if the code runs often during generation.
2. The two implementations are kept in sync by hand and by parity tests. If they diverge later, a one-line comment in each pointing at the other is the only safeguard. The comments mostly exist already ("As in source discovery..."). Keeping that cross-reference up to date is cheap insurance.
3. Optional: add a test where a row type implements the same generic interface twice with different arguments, for example IEnumerable<int> and IEnumerable<string>. The description says this case is covered, and I did not check that the tests include it.

No blocking issues found. Looks good to merge once CI is green.

[thomhurst]

The earlier blocker is cleared: run 41660232844 passed on ebbd9f9. I read its Claude review and all review surfaces; Codex also reports completed with no findings on this head. Both finding threads remain resolved with no later reviewer replies. All current CI legs, including multi-framework, engine and AOT jobs, passed.

Latest non-blocking suggestions: keep helper extraction and caching profiling-driven; existing backend cross-references and parity tests guard synchronization. The suggested conflicting-interface case already exists as Ambiguous and AmbiguousReversed implementing IMarker and IMarker, with null inference expected in both modes. No additional code changes are needed.