fix(skill): correct TestDoubles matching guidance - #113
Conversation
![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: 513950747f
ℹ️ 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: caae88c4a1
ℹ️ 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".
Match and UseGeneratedTestDoubles live in the Compono namespace, not Compono.TestDoubles (the package name). 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: e68a5db0a9
ℹ️ 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".
Snapshot skills/compono/references/testdoubles.md at 644a5ad (pre-fix) and rerun evals 28-31 against it, per skill-creator's paired-run requirement for skill-improvement benchmarks. old_skill passes 1/4 (eval 29 only) vs with_skill's 4/4, confirming the fix corrects real regressions rather than restating already-passing behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@codex review — please focus on the latest commit (04d5d46): the new old_skill baseline added for the AWS Secrets Manager Provider TestDoubles benchmark (skills/compono-evals/benchmarks/2026-08-27/benchmark.json, benchmark.md, outputs/old_skill/eval-28..31-old-skill.md). Check that the paired with_skill/old_skill comparison is accurate, the pass/fail evidence for each eval matches its stated expectations, and the benchmark.json stays valid/consistent with benchmark.md. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04d5d46426
ℹ️ 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 flagged that with_skill and old_skill runs weren't launched together, so they don't satisfy skill-creator's paired-run requirement. Rerun all 8 combinations (evals 28-31 x with_skill/ old_skill) in a single batch against the current skill and the 644a5ad pre-fix snapshot. Also verified and refuted the accompanying claim that only testdoubles.md was snapshotted: the snapshot always captured the full skills/compono directory, including the pre-fix SKILL.md (5139507 also added a guardrail section there). Net effect: old_skill now fails all 4 evals (0/4, down from 1/4) since eval 29's prior old_skill pass was a run-to-run fluke, not a stable result - stronger, cleaner evidence the fix corrects real regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
@codex review — please check the latest commit (77ead53), which reruns the AWS Secrets Manager Provider TestDoubles benchmark as a single same-turn batch (skills/compono-evals/benchmarks/2026-08-27/benchmark.json, benchmark.md, outputs/eval-28..31-with-skill.md, outputs/old_skill/eval-28..31-old-skill.md). Focus on: whether the with_skill/old_skill pairing and pass/fail evidence now hold up, whether benchmark.json stays consistent with benchmark.md, and whether the snapshot-scope and run-ordering concerns from the prior review are actually resolved. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77ead53074
ℹ️ 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".
The prior wording said the earlier baseline "snapshotted only testdoubles.md" then immediately said it "captured the full directory" - contradictory. Clarify that only the benchmark's description of the earlier baseline was wrong, not the snapshot itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 participant
Summary
Fixes stale Compono agent-skill and package documentation that still described
Compono.TestDoublesas lacking argument matching and argument-filtered verification. The updated guidance now reflects the shippedMatch<T>surface and clearly separates ordinary matching/filtering from true capture or invocation-aware callback behavior.Changes
Skill guidance
skills/compono/SKILL.mdwith a high-priority TestDoubles matching guardrail.skills/compono/references/testdoubles.mdto document current supported behavior:Configure()/Verify()Match.Any<T>()Match.Is<T>(predicate)Never()/Once()/Exactly(n)Arg.Is,Arg.Any,Received, andDidNotReceive.Skill evals and evidence
skills/compono-evals/evals.json:IConfigurationBuilderargument-filtered verificationIAmazonSecretsManagerargument-matched configurationskills/compono-evals/benchmarks/2026-08-27/.docs/research/0012-aws-secrets-manager-provider-compono-skill-regression.mdrecording the AWS migration evidence and the abstract-class classification as existing project-local test architecture, not a roadmap item.Product documentation
docs/packages/compono-testdoubles.mdto remove stale current-state recommendations to useCompono.NSubstitutefor argument matching/call verification that TestDoubles now supports.CallVerifierXML docs and regenerated the affected API reference pages soCallVerifieris described as the terminal count assertion after generated member/argument filtering has already been applied.Validation
python3 -m json.tool skills/compono-evals/evals.jsonpython3 -m json.tool skills/compono-evals/benchmarks/2026-08-27/benchmark.jsonnpx skills add ./skills -l --full-depth— found exactlycomponodotnet tool restoredotnet build src/Compono/Compono.csproj -c Release -f net10.0.github/scripts/generate-api-reference.shdotnet build— 0 warnings / 0 errorsdotnet test— 2592 passed, 0 failedNote: a full historical Claude Code with/without-skill eval benchmark could not be rerun locally because Claude Code returned the local weekly-limit message. Existing eval definitions 1–27 were left unchanged; the focused regression run covers only the new AWS scenarios.
Notes for Reviewers
This PR intentionally makes no runtime
Compono.TestDoublesbehavior changes. It also does not add abstract-class doubles to the roadmap: the AWSTestConfigurationProvider : ConfigurationProvidercase was pre-existing project-local test architecture, not evidence of a NSubstitute-can/TestDoubles-cannot replacement case.