fix(tdd): post-merge Copilot round-4 fixes — class-invariant rewrite + fixture clarity#8
Merged
Merged
Conversation
- SKILL.md: add flow-note linking Verify Regression Invariant prose to diagram node (verify_green → verify_invariant → refactor) - SKILL.md: fix class-invariant revert-and-restore guidance — revert only the previously-broken method so its row fails and correct siblings still pass; restore to make full table green - scenario.md: clarify Sum spec — 0-on-empty is correct; actual bug is 0-on-single-element (off-by-one) - old-flow.md: reframe "returns 0 for empty" as correct behavior, not a symptom; describe the panic as a regression from correct to crashing - new-flow.md: indent Edge-case sweep under Class invariant heuristic check (introduced by colon; was at wrong indentation level) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the test-driven-development skill documentation and fixtures to correct previously incorrect class-invariant regression guidance and to clarify the narrow-regression fixture narrative (what’s “correct behavior” vs the actual bug/regression).
Changes:
- Link “Verify Regression Invariant” prose to the corresponding diagram node/path (
verify_green → verify_invariant → refactor). - Correct the class-invariant “revert-and-restore” instruction to reflect expected table-driven test behavior (only the reverted method’s row fails; correct siblings pass).
- Clarify the narrow-regression fixture story: empty input returning
0is correct; the regression is an empty-slice panic, and the original bug is single-element returning0.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| skills/test-driven-development/SKILL.md | Fixes/clarifies Verify Regression Invariant + class-invariant revert/restore guidance and ties prose to the diagram node. |
| skills/test-driven-development/test-fixtures/narrow-regression/scenario.md | Clarifies the intended spec vs the actual off-by-one bug in the fixture scenario. |
| skills/test-driven-development/test-fixtures/narrow-regression/old-flow.md | Reframes the “hidden problem” as a regression from correct empty handling to a crash. |
| skills/test-driven-development/test-fixtures/narrow-regression/new-flow.md | Fixes list indentation so Edge-case sweep sits under the intended heuristic section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Cherry-pick of commit 911c62e, which was not captured in the PR #5 squash merge (f1baa9a). Contains 4 files changed across the test-driven-development skill:
verify_green → verify_invariant → refactor); fix class-invariant revert-and-restore guidance (Important finding: PR skill: test-driven-development — Verify Regression Invariant + class invariant coverage #5 shipped wrong instruction)0on empty input is correct; the actual bug is0on single-element input (off-by-one in loop)Key fix — class-invariant guidance (Important)
PR #5 shipped this in SKILL.md:
That is wrong — "both must fail" implies ALL sibling rows fail when you revert ONE method. The correct behavior is:
Old (wrong):
New (correct):
Runtime launch transcript
Step 1b trigger check: documentation-only PR (skill prose + fixture markdown). No build config, deployment config, version pins, startup config, migrations, or plugin loading paths touched. Step 1b: NOT triggered.
Step 1c trigger check: no version pins in diff. Step 1c: NOT triggered.
Verdict: PASS (doc-only; neither runtime-launch nor version-skew steps apply)
Verify Regression Invariant
Not applicable — documentation-only, no code paths. Prose correctness verified by reading old vs. new:
🤖 Generated with Claude Code