Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #251 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 41 41
Lines 6115 6213 +98
=========================================
+ Hits 6115 6213 +98
🚀 New features to boost your workflow:
|
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
04e65e2 to
38b6e2e
Compare
dfed
changed the title
|
@codex-review |
Review Result (
|
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
38b6e2e to
39504d3
Compare
|
@codex review |
![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: 39504d3767
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
2 times, most recently
from
55dbb37 to
a609395
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a60939595e
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
a609395 to
f7a7b54
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7a7b54fae
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
f7a7b54 to
f91cd57
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f91cd579c7
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
f91cd57 to
c4ad163
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
c4ad163 to
23bddf7
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23bddf7aab
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
23bddf7 to
b665ab9
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b665ab92de
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
b665ab9 to
aa1a9b4
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa1a9b48c9
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa1a9b48c9
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
aa1a9b4 to
378ab0e
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 378ab0e347
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
378ab0e to
030d747
Compare
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 030d747f48
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
030d747 to
fff3115
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fff311596f
ℹ️ 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".
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
fff3115 to
5f90597
Compare
|
@codex review |
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
5f90597 to
325e303
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 325e303964
ℹ️ 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".
…tials MockParameterNode.builderClosureType used concreteType for the closure return type, causing type mismatches when the property type differs from the concrete type (e.g., protocol fulfilled by concrete, or type-erased wrapper like AnyService). Changes: - builderClosureType now uses instantiatedTypeDescription (property type) so SafeDIOverrides closures match what the parent init expects - For erasedToConcreteExistential, the mock body splits override vs default paths: overrides return the wrapper type directly, defaults wrap the concrete builder result (e.g., AnyService(ConcreteService())) - Helper function return types updated from concrete to property type - Instantiator construction no longer double-wraps erased existentials Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dfed
force-pushed
the
dfed/fix-builderClosureType-property-type
branch
from
325e303 to
17cf0a5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17cf0a5834
ℹ️ 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".
…nit (#270) ## Summary Capture convention/mechanism knowledge that wasn't load-bearing in the code but has tripped up recent work. All additions are docs + one small helper extraction — no behavior change. ## What's documented **Cycle mechanisms table** (CLAUDE.md). Four related things — `isPropertyCycle`, `cycleEdges`, `throwIfInvalidCycle`, `validateMockRootScopeForCycles` — each doing a different job with overlapping names. Table names them, says where each is set, and what it does. Also explicit: if `throwIfInvalidCycle` accepts a cycle, every hop is non-constant. **`erasedToConcreteExistential` × property-type matrix** (CLAUDE.md Common Pitfalls). The flag doesn't imply a constant property type — it's orthogonal. Four rows cover all combinations and whether each can appear in an accepted cycle. This mistake cost a round trip in #259 when defensive branches were removed on a wrong "erased = constant" assumption. **Two-layer mock return type** (CLAUDE.md Mock generation specifics). Inner closure type (`builderClosureType`, #251) vs outer `mock()` return type (#266) are distinct concerns when a `customMock` returns a type other than `Self`. Both layers have to agree. **"Selected construction initializer"** (CLAUDE.md). SafeDI picks a single canonical initializer per type. Validation that only matters for the path SafeDI uses (e.g., `Self.*`-in-default-expression check) should gate on this, not walk every initializer. Was #268's P1 bug class. **"Structurally unreachable" claims need a failing test** (CLAUDE.md Testing Philosophy). Codifies the discipline we're already trying to hold — structural arguments are easy to get wrong. **Inline /// comments**: - `MockParameterNode.isPropertyCycle` cross-references `cycleEdges` (per-root syntactic vs global feedback arc set). - `MockContext.cycleEdges` cross-references `isPropertyCycle` (opposite direction). - `throwIfInvalidCycle` docstring expanded with explicit rejection classes and acceptance condition. ## Code change Add `InstantiableVisitor.canonicalConstructionInitializer` computed property. Route the two existing `initializers.first(where: { $0.isValid(forFulfilling: dependencies) })` call sites through it: - `InstantiableVisitor.swift:377` (building `Instantiable.initializer`) - `InstantiableMacro.swift:259` (the `hasMemberwiseInitializerForInjectableProperties` guard) Removes drift risk — previously the macro recomputed the selector separately. ## Test plan - [x] `swift test --traits sourceBuild` — 885 tests pass - [x] `./CLI/lint.sh` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sts (#276) ## Summary Five mock-generation tests carried `skipCompileVerification: true` FIXMEs pointing at generator bugs that have since been fixed. Removing the opt-outs confirms the fixes hold end-to-end (fresh parse → generated code → actual swiftc typecheck). ## Changes Removed the FIXME comments and `skipCompileVerification: true` flag on: | Test | Bug | Fixed in | |------|-----|----------| | `mock_usesMockOnly_whenProductionHasNoMockAndBothFulfillSameAdditionalType` | mockOnly not coerced to fulfilled-slot type | #251 | | `mock_sendableClosureDependencyProducesValidIdentifier` | `@escaping` dropped on `@Sendable` closure on mock() | #267 | | `mock_generatedForFullyLazyInstantiationCycle` | Fully-lazy cycles produced self-referential `SafeDIMockConfiguration` defaults | #259 | | `mock_extensionUserMockReturningAdditionalTypeUsedForProtocolProperty` | customMock return type disagreement between root call site and type's own `mock()` | #266 | | `mock_extensionUserMockReturningAdditionalTypeNotUsedForConcreteProperty` | Same as above — sibling test | #266 | ## Test plan - [x] `swift test --traits sourceBuild` — 901 tests pass (up from 901; no tests added, but all five now exercise the compile-verification path) - [x] `./CLI/lint.sh` 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
MockParameterNode.builderClosureTypeusedconcreteTypefor the closure return type, causing type mismatches when the property type differs (e.g.,NetworkServicefulfilled byDefaultNetworkService, orAnyServicewrappingConcreteService)instantiatedTypeDescription(property type) soSafeDIOverridesclosures match the parent init signatureerasedToConcreteExistential, the mock body now splits override vs default paths: overrides return the wrapper type directly, defaults wrap the concrete builder resultdefaultBuilderCall(arguments:)to generate direct calls (e.g.,ConcreteService(helper: helper)) instead of curried function references (e.g.,ConcreteService.init(helper:)(helper)) in erased default paths — faster for the compiler to type-checkTest plan
🤖 Generated with Claude Code