docs(core): milestone 5 phase 3 - close out NSubstitute integration docs - #31
docs(core): milestone 5 phase 3 - close out NSubstitute integration docs#31ncipollina wants to merge 3 commits into
Conversation
Adds the Providers public/internal split write-up and Compono.NSubstitute's real Owns list to docs/architecture.md, closing PLAN-0005's last open Phase 3 task. Auditing the rest of the doc set for this closeout surfaced a second round of staleness: PLAN-0004's Open Items, docs/architecture.md's Compono.XunitV3 entry, docs/mvp.md's Milestone 4 section, and docs/public-api.md's xUnit v3 Experience section all still described the CMP0003 interface/abstract/delegate-root gap as open, even though Phase 2 (merged in #30) already fixed it. All updated to point at ADR-0024's Amendment 2 instead of contradicting Milestone 5's own shipped fix. PLAN-0005 is now Status: Done - every phase (0-3) checked off. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![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: d8cadd1ea5
ℹ️ 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".
…ndled PR #31 review (Codex, P2): the Compono.NSubstitute Package Boundaries entry overstated NotHandled as unconditionally reaching stage-9's diagnostic - a sealed concrete class with a generated plan still composes fine at stage 8 despite the test-double provider declining it at stage 6. Reworded to say the diagnostic is reached only when nothing later in the pipeline can satisfy the request either. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a24951e82
ℹ️ 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".
PR #31 review (Codex, P2): the Open Architectural Decisions section still said Compono.NSubstitute (ADR-0025) was "design only, not yet implemented" and that stages 5/6 stay empty because no package registers into them - directly contradicting the Package Boundaries section a few hundred lines earlier in the same PR, which already documents NSubstituteProvider as implemented and test-covered. Updated that bullet and the Resolution Pipeline table's stage-6 row to say Compono.NSubstitute is shipped and populates stage 6 via UseNSubstitute() - only stage 5 (Compono.Bogus, Milestone 6) remains genuinely unimplemented. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0e8cef5301
ℹ️ 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".
| | 4 | Configuration rules | Ordered `ICompositionProvider` collection, per [ADR-0020](adr/0020-composition-configuration-rules.md) (implemented, Milestone 3 Phase 3). Renamed from "profile rules" (`PipelineStage.ConfigurationRule`): populated by type/member value rules compiled from `builder.For<T>()...`, whether reached directly or via a profile's `Configure` — a profile is a reusable application mechanism over this stage, not its owner ([ADR-0018](adr/0018-composition-profiles.md)). Collection-size configuration does **not** populate this stage — see Configuration Rules, below. | | ||
| | 5 | Semantic value providers | Ordered `ICompositionProvider` collection. The public registration surface (`builder.AddSemanticProvider(ICompositionValueProvider)`) is implemented as of Milestone 5 Phase 0 ([ADR-0024](adr/0024-public-provider-extensibility-model.md)) — but no `Compono`-shipped package registers anything into it by default, since `Compono.Bogus` (Milestone 6) doesn't exist yet. Empty in practice until then, populated in mechanism now. | | ||
| | 6 | Test-double providers | Ordered `ICompositionProvider` collection. Same status as stage 5: the public registration surface (`builder.AddTestDoubleProvider(ICompositionValueProvider)`) is implemented ([ADR-0024](adr/0024-public-provider-extensibility-model.md)), but empty in practice until `Compono.NSubstitute` (Milestone 5's own package, [ADR-0025](adr/0025-compono-nsubstitute-package-design.md)) ships — see [PLAN-0005](plans/0005-milestone-5-nsubstitute-integration.md) for its phase status. | | ||
| | 6 | Test-double providers | Ordered `ICompositionProvider` collection. The public registration surface (`builder.AddTestDoubleProvider(ICompositionValueProvider)`) is implemented ([ADR-0024](adr/0024-public-provider-extensibility-model.md)), and `Compono.NSubstitute` ([ADR-0025](adr/0025-compono-nsubstitute-package-design.md), implemented and test-covered — see [PLAN-0005](plans/0005-milestone-5-nsubstitute-integration.md)) is a real registrant via `UseNSubstitute()`. Still empty by default in any composition that doesn't opt into it — registration is per-`Composer`, not automatic just because the package is referenced. | |
There was a problem hiding this comment.
This row now correctly says Compono.NSubstitute is implemented, but the summary immediately below still says stages 5/6 remain empty until Compono.Bogus/Compono.NSubstitute—“which don't exist yet”—register on the consumer's behalf (lines 207–209). Fresh evidence after the earlier documentation fix is that this stale sentence remains in the current head, leaving the Resolution Pipeline internally contradictory while Phase 3 is marked done; update the summary to distinguish stage 5 from the now-available opt-in stage-6 integration.
AGENTS.md reference: AGENTS.md:L156-L157
Useful? React with 👍 / 👎.
1 participant
📋 Summary
Closes out PLAN-0005 Phase 3, the last open phase of Milestone 5. Adds the one genuinely missing piece (
docs/architecture.md's public/internal provider split write-up andCompono.NSubstitute's realOwnslist), then fixes a second round of doc staleness this closeout audit surfaced: several docs across Milestone 4 and Milestone 5 still described aCMP0003compile-time gap as open, even though PR #30 (Phase 2) already fixed it.📝 Changes
New content (the actual Phase 3 task)
docs/architecture.md— new "Public providers (stages 5/6)" subsection under Providers, documentingICompositionValueProvider/CompositionProviderRequest/CompositionProviderResult/PublicProviderAdapteralongside the existing internalICompositionProvidersketch.docs/architecture.md—Compono.NSubstitute's Package Boundaries entry now has a Design line, implementation-status note, and its realOwnslist (NSubstituteProvider/NSubstituteOptions/CompositionBuilderExtensions), matchingCompono.XunitV3's existing entry shape.Stale-doc correction (found while closing out Phase 3)
Compono.Generators' root-levelCMP0003check so an interface/abstract/delegate-typed[Compose]parameter compiles — but five places written or last touched before that fix still described the gap as open:docs/plans/0004-milestone-4-xunit-integration.md— Open Items entry marked resolved (struck through, not deleted) with a pointer to ADR-0024's Amendment 2.docs/architecture.md—Compono.XunitV3Package Boundaries entry.docs/mvp.md— Milestone 4 section, Example, and Exit Criteria.docs/public-api.md— xUnit v3 Experience section.docs/mvp.md— Milestone 5 section now says "This milestone is complete" instead of "Phase 3 still open."docs/plans/README.mdanddocs/plans/0005-milestone-5-nsubstitute-integration.md— PLAN-0005 status flipped toDone(all four phases now checked off).🧪 Validation
dotnet buildre-verified clean (0 warnings, 0 errors) as a sanity check.💬 Notes for Reviewers
No code changes. The stale-doc correction is larger than the nominal Phase 3 task list because tracing "who else still claims the CMP0003 gap is open" led into PLAN-0004's docs, not just PLAN-0005's — same pattern as the doc-staleness rounds PR #28/#29 caught, just spanning a different plan's docs this time since the fix landed in Phase 2 of this plan rather than a Phase 4 follow-up to PLAN-0004.