fix(han): correct the three findings in feedback issue #148 - #206
Merged
Conversation
Records the evidence, root cause, and correction plan for the three findings in #148: automated-test-planning runs its full pipeline regardless of question scope, the test analysis agents can only argue a deferral from coverage location, and the readability editor does not check its own insertions against the voice blocklist. Carries 20 evidence items and 11 adversarial validation findings. Four of those findings corrected the plan before implementation.
…sked The skill classified nothing, so a narrow yes/no question ran the same pipeline as a whole-branch analysis: up to seven agent dispatches, a nine-section document, and two reviewers over it. Adds a size band with the same shape code-review and iterative-plan-review already use. Small defaults to focused mode: one agent, no conditional specialists, no reviewers, and a prose answer instead of the template. Medium and large are unchanged. Classification reads the user's request before Step 1's file list, because Step 1 falls back to the whole changed-files list when no scope is named, and that describes the branch rather than the question. Step 3's behavioral, prerequisite, and YAGNI sweeps run in both modes, so a focused answer is shorter without being less filtered. Registers the skill in docs/sizing.md alongside the other sizing-aware skills.
test-engineer could justify skipping a proposed test only by pointing at coverage elsewhere or at brittleness risk. Neither survives a reader who grants the path is reachable and asks what the new assertion would add. edge-case-explorer had the same gap, with "sufficient" left undefined. test-engineer gains a fifth evaluation axis: name a specific weakening of the code under test, predict which existing tests fail under it, and defer only when an existing test already catches everything the candidate would. Adds a Redundant Kill Set anti-pattern, a required Discriminating power field on every deferred item, and a rule against deferring on coverage location alone. edge-case-explorer now defines sufficient: a test is sufficient for an edge case only when it fails under a change that breaks that case. Neither agent has a test runner, so both state that the answer is a prediction from reading assertions and must name the test and assertion it came from. The canonical YAGNI rule is left untouched. It is written for committed items generally and is vendored byte-identically into five plugins, while this reasoning is test-specific.
The editor's only post-rewrite step confirmed facts survived. Its blocklist criterion governed the draft it audited, never the sentences it wrote, so the rewrite was the one place a fresh voice violation could originate and the one place with no voice check. Three planning skills tell the caller not to walk the self-check over the editor's output, and one skill runs no independent check at all, so a violation the editor introduces can reach the reader. Adds a step 4 that re-reads only the sentences the editor rewrote or inserted against the vocabulary blocklist and the em-dash positions, plus a line in the returned report. The pass corrects named violations only and leaves compliant wording alone, so a legal appositive em-dash survives it.
mxriverlynn
force-pushed
the
gh-148-automated-test-planning
branch
from
September 9, 2026 14:25
b0dd64a to
6327462
Compare
mxriverlynn
marked this pull request as ready for review
September 9, 2026 14:26
This was referenced Sep 9, 2026
Draft
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
1 participant
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
automated-test-planninga size band so a narrow question runs one agent and returns prose, instead of up to seven dispatches and a nine-section document.test-engineerandedge-case-explorera discriminating-power test, so a deferral is argued from what an assertion would catch rather than from where coverage happens to sit.readability-editorcheck the sentences it writes against the voice blocklist it applies.manual-test-planninguntouched on purpose; both exclusions are argued in the investigation doc.Why
Closes #148, a Han feedback report. It scored the run 5/5 on output accuracy and evidence discipline, and 2/5 on output length against decision count. The analysis quality was never in question; the cost and the durability of the reasoning were.
Each fix copies a mechanism the suite already runs. The size band is
code-review's classifier plusiterative-plan-review's lightweight mode, and the suite's own agent-economics guidance already says not to delegate work a skill can finish in a few tool calls, so that change is a conformance repair rather than a new idea.One part of the report is stale. The absolute em-dash ban it quotes was replaced six days after filing by a rule legalizing two positions, so two of its three flagged instances may no longer be violations. The structural gap behind it is unaffected and is what this PR fixes.
Full evidence, root cause, and adversarial validation:
docs/plans/test-planning-feedback-issue-148/feedback-investigation.md(20 evidence items, 11 validation findings).How to verify
npm run linthan-coding/skills/automated-test-planning/SKILL.mdStep 1.5 and confirmarguments: sizeis in the frontmatter. Without that key there is no$sizeto bind, which validation caught in the first draft./automated-test-planningwith a narrow question on a branch with many changed files. It should announce a small band and answer in prose. Classification reads the request before the file list precisely so this case works./automated-test-planning largeand confirm the full roster and template still run.readability-editorover a draft containing a legal appositive em-dash and confirm the new pass leaves it alone.Risk / rollback
Low. Markdown instruction files only, no scripts and no behavior outside a Claude Code session. Medium and large runs of
automated-test-planningare unchanged, so the blast radius is the small band and the two agents' deferral rationale. Revert withgit revert; the four commits are independently revertable, one per correction plus the investigation doc.Three open risks are recorded in the investigation doc: the editor's new pass is unvalidated against false positives,
manual-test-planningkeeps an unconditional two-agent pipeline, and the discriminating-power answer is a prediction rather than an executed mutation test.