fix(requesting-code-review): correct fixture line refs (post-merge Copilot finding)#6
Merged
Merged
Conversation
…pilot finding)
Hunk @@ -10,6 +10,12 @@ places the Create function declaration at line 10
(shown as @@ context label), so content starts at line 11:
line 11: return d.client.Send("create", ...)
line 12: "kind": d.kind ← Create's kind ref (was 11)
...
line 18: func Inspect(...) ← Inspect location (was 17)
...
line 27: "kind": d.kind ← Delete's kind ref (was 26)
Fixes three stale refs in expected-new-result.md: bug-class scan
citation (line 10), Finding 1 Location (line 30), and Finding 1
What's wrong (line 31).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Corrects post-merge line-reference inaccuracies inside the expected-new-result.md fixture for the sibling-asymmetry requesting-code-review skill, keeping the fixture’s citations consistent with the diff.patch hunk line arithmetic.
Changes:
- Update bug-class scan text to reference
Create"kind": d.kindat line 12 andDelete"kind": d.kindat line 27. - Update Finding 1’s
Locationand “What’s wrong” text to referenceInspectstarting atlib/dispatcher.go:18and align sibling line references accordingly.
💡 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 1 out of 1 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.
intel352
added a commit
that referenced
this pull request
Apr 25, 2026
…der syntax + team-lead graph label
- Switch {N}/{team-name} curly-brace placeholders to <N>/<team-name> angle-bracket
style in implementer-prompt.md and spec-reviewer-prompt.md, consistent with
all other skill templates
- Update DOT graph node label from "Team Lead" to "team-lead" to match the
SendMessage recipient identifier used throughout the workflow text
- Add fixture invariant #6 + verify command for curly-brace placeholder detection
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352
added a commit
that referenced
this pull request
Apr 25, 2026
* skill(subagent-driven-development): DRY team conventions to agents/team-conventions.md
Extract repeated boilerplate from implementer/spec-reviewer/code-reviewer
prompts into a single shared file. Each prompt template now references
agents/team-conventions.md instead of inlining TDD discipline, self-review
checklist, review protocol, and sanitization rules.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* skill(subagent-driven-development): fixture — DRY conventions before/after
Documents the before/after of extracting team conventions from inline
prompt boilerplate to agents/team-conventions.md.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address 5 Copilot round-1 findings
A: conventions.md code-reviewer — bug-class checklist + verdict vocabulary
now reference skills/requesting-code-review/SKILL.md explicitly.
B: conventions.md implementer — version-skew audit now references
skills/finishing-a-development-branch/SKILL.md Step 1c.
C: implementer-prompt.md — same version-skew audit Step 1c reference.
D: code-quality-reviewer-prompt.md — split adversarial framing from
bug-class checklist + verdict vocabulary; latter now point to
skills/requesting-code-review/SKILL.md.
E: conventions.md code-reviewer — adversarial framing exception clause
preserved: if fewer than 3 issues found, document every bug-class
check run rather than manufacturing issues.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address 2 Copilot round-2 findings
A: implementer-prompt.md — restore explicit "DM spec-reviewer when ready"
step so the spec-compliance gate isn't stalled after implementation.
B: SKILL.md — fix stale "checklist below" reference in implementer workflow
step 4; now points to agents/team-conventions.md.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address 2 Copilot round-3 findings
A: implementer-prompt.md — DM team-lead with branch/commit + "ready for
merge" (not PR link; PR is created by team-lead via finishing-a-branch
after all tasks complete, not by the implementer).
B: SKILL.md code-reviewer section — split bug-class checklist + verdict
vocabulary to their canonical source (skills/requesting-code-review/
SKILL.md); team-conventions only hosts adversarial framing + output format.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address round-4 finding + tighten fixture
SKILL.md implementer Team Conventions block: replace enumeration that
implied "self-review checklist" is inline with explicit "all rules are
defined in agents/team-conventions.md, not repeated here"; also sync
version-skew audit pointer to finishing-a-development-branch Step 1c.
Fixture: add 5 invariants the "after" state must hold, including:
- no "checklist below" / "see below" / "inlined here" language
- every version-skew audit reference points to Step 1c
- every bug-class / verdict reference points to requesting-code-review
- adversarial-framing exception clause preserved verbatim
Verify command excludes test-fixtures dir to avoid self-matching.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address 2 Copilot round-5 findings
A: fixture verify command — switch to grep -rEn with ERE; replace \b
(non-portable) with ([[:space:][:punct:]]|$) for reliable Self-Review
boundary match across grep implementations.
B: team-conventions.md sanitization rule — tighten "no version references"
to "no specific internal project/company/product-version/incident
references"; explicitly allow dependency/runtime/tooling version numbers
needed for accurate technical guidance (version-skew audit, pin refs).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address round-6 finding + fixture ref check
code-quality-reviewer-prompt.md: fix broken path "requesting-code-review/
code-reviewer.md" → "skills/requesting-code-review/code-reviewer.md"
(missing skills/ prefix; file exists at the correct path).
Fixture: add 2 new invariant checks:
- Dead skill file refs: grep all skills/*.md references, verify each
resolves to an existing file (catches missing-prefix bugs like this one).
- Bare requesting-code-review refs: grep for refs without skills/ prefix
(both checks exclude test-fixtures dir to avoid self-matching).
All three invariant checks PASS on current tree.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): normalize team-lead spelling across all role prompts
Round-7 fix: use hyphenated team-lead consistently with agents/team-conventions.md
and implementer-prompt.md usage pattern, avoiding ambiguity with the Agent Teams
role identifier.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address round-9 findings — placeholder syntax + team-lead graph label
- Switch {N}/{team-name} curly-brace placeholders to <N>/<team-name> angle-bracket
style in implementer-prompt.md and spec-reviewer-prompt.md, consistent with
all other skill templates
- Update DOT graph node label from "Team Lead" to "team-lead" to match the
SendMessage recipient identifier used throughout the workflow text
- Add fixture invariant #6 + verify command for curly-brace placeholder detection
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): fix fixture grep to catch start-of-line bare refs
Round-10 finding: [^/]requesting-code-review/ requires a preceding character,
so a bare ref at the start of a line would be missed. Switch to
(^|[^/])requesting-code-review/ to cover both cases.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): remove parenthetical rule enumeration — point to team-conventions only
Round-11 findings: both implementer-prompt.md and the embedded implementer
prompt in SKILL.md enumerate convention details (TDD, regression-invariant,
etc.) that are already centralized in agents/team-conventions.md. This
re-inlining contradicts the DRY goal of the refactor. Trim to a simple
pointer; the single source of truth is the conventions file.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(subagent-driven-development): address round-12 findings — PLAN_REFERENCE field + grammar
- Add PLAN_REFERENCE field to code-quality-reviewer dispatch template to match
the {PLAN_REFERENCE} placeholder in skills/requesting-code-review/code-reviewer.md
(distinct from PLAN_OR_REQUIREMENTS used in the instructions section)
- Fix grammar in agents/team-conventions.md: "Shared rules every team agent"
→ "Shared rules that every team agent"
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352
added a commit
that referenced
this pull request
Apr 25, 2026
… plan+design Finding #1 (plan:395): verification command pipes to grep, which exits 1 when no matches — looks like failure. Fix: add || true, update expected output description. Finding #2 (design:142): described markers as "HTML-comment-shaped" but the syntax is angle-bracket tags (not HTML comments). Fix: "angle-bracket tag syntax that most markdown renderers treat as unknown HTML tags". Finding #3 (design:178): prose said OpenCode/Cursor "get blank entries" but table uses sentinel value host-pass-through. Fix: prose now matches sentinel. Finding #4 (plan:1232): "Two distinct clean Copilot review timestamps" hard gate has no anchor policy doc. Fix: downgrade to "recommended practice" phrasing. Finding #5 (plan:65): skill-content-grep.expected.txt listed as a created file and in ALLOWED_FILES but the script never reads it — dead artifact. Fix: drop the file from the task entirely. Finding #6 (plan:141): guard script ran grep twice per token, reported line numbers from the stripped (AWK-processed) string rather than the original file. Fix: single-pass AWK that emits "LINENO:content" preserving original line numbers; single grep pass with || true. Finding #7 (plan:132): skip-regex <host: *claude-code *> only matched the exact single-host tag form, not comma-separated lists like <host: codex, claude-code>. Fix: AWK rule checks if /claude-code/ appears anywhere in the <host:> line, handling any comma-separated host list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
9 tasks
intel352
added a commit
that referenced
this pull request
Apr 25, 2026
… plan+design Finding #1 (plan:395): verification command pipes to grep, which exits 1 when no matches — looks like failure. Fix: add || true, update expected output description. Finding #2 (design:142): described markers as "HTML-comment-shaped" but the syntax is angle-bracket tags (not HTML comments). Fix: "angle-bracket tag syntax that most markdown renderers treat as unknown HTML tags". Finding #3 (design:178): prose said OpenCode/Cursor "get blank entries" but table uses sentinel value host-pass-through. Fix: prose now matches sentinel. Finding #4 (plan:1232): "Two distinct clean Copilot review timestamps" hard gate has no anchor policy doc. Fix: downgrade to "recommended practice" phrasing. Finding #5 (plan:65): skill-content-grep.expected.txt listed as a created file and in ALLOWED_FILES but the script never reads it — dead artifact. Fix: drop the file from the task entirely. Finding #6 (plan:141): guard script ran grep twice per token, reported line numbers from the stripped (AWK-processed) string rather than the original file. Fix: single-pass AWK that emits "LINENO:content" preserving original line numbers; single grep pass with || true. Finding #7 (plan:132): skip-regex <host: *claude-code *> only matched the exact single-host tag form, not comma-separated lists like <host: codex, claude-code>. Fix: AWK rule checks if /claude-code/ appears anywhere in the <host:> line, handling any comma-separated host list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
intel352
added a commit
that referenced
this pull request
Apr 25, 2026
…ign docs (#13) * fix(pr12-round2): address 7 Copilot findings on cross-LLM portability plan+design Finding #1 (plan:395): verification command pipes to grep, which exits 1 when no matches — looks like failure. Fix: add || true, update expected output description. Finding #2 (design:142): described markers as "HTML-comment-shaped" but the syntax is angle-bracket tags (not HTML comments). Fix: "angle-bracket tag syntax that most markdown renderers treat as unknown HTML tags". Finding #3 (design:178): prose said OpenCode/Cursor "get blank entries" but table uses sentinel value host-pass-through. Fix: prose now matches sentinel. Finding #4 (plan:1232): "Two distinct clean Copilot review timestamps" hard gate has no anchor policy doc. Fix: downgrade to "recommended practice" phrasing. Finding #5 (plan:65): skill-content-grep.expected.txt listed as a created file and in ALLOWED_FILES but the script never reads it — dead artifact. Fix: drop the file from the task entirely. Finding #6 (plan:141): guard script ran grep twice per token, reported line numbers from the stripped (AWK-processed) string rather than the original file. Fix: single-pass AWK that emits "LINENO:content" preserving original line numbers; single grep pass with || true. Finding #7 (plan:132): skip-regex <host: *claude-code *> only matched the exact single-host tag form, not comma-separated lists like <host: codex, claude-code>. Fix: AWK rule checks if /claude-code/ appears anywhere in the <host:> line, handling any comma-separated host list. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(cross-llm-plan): address 14 Copilot round-1 findings on plan+design Plan doc (cross-llm-portability.md): - Add lowercase sonnet/opus/haiku to forbidden-tokens list and explain why - Add rationale for not yet including AskUserQuestion/Agent (PRs B-D) - Remove tests/skill-content-grep.sh from ALLOWED_FILES (tests/ not scanned) - Fix AWK skip to match only exclusive <host: claude-code> blocks, not multi-host tags; update inline comment accordingly - Remove dead fail=1 inside while-pipe subshell; rely solely on $tmp check - Add || true to all 7 skill-scoped grep verification commands (lines 443, 517, 626, 694, 764, 844, 888) — grep exits 1 on no-match (PASS case) Design doc (cross-llm-portability-design.md): - Fix broken path: subagent-driven-development.md → skills/subagent-driven-development/SKILL.md - Remove false gpt-5.x claim from grep guard description; clarify that only Claude-Code brand names are guarded, Codex names use <host:> markers - Replace "Open questions deferred to implementation" with resolved decisions: AWK guard approach, angle-bracket marker syntax, OpenCode tool mapping, Cursor support depth scope Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(cross-llm-plan): address 7 Copilot round-2 findings on plan + design docs - set -euo pipefail (was set -u only) - Fix stray || true in step-title prose and GHA step name - Verification commands capture guard output before grepping to avoid masking execution errors - Fix prose claiming gpt-5.x tokens are guarded (guard only covers Claude brand names) - Design doc: clarify angle-bracket marker advantage is readability in rendered Markdown * fix(cross-llm-plan): add end-anchor to AWK skip regex so multi-host blocks are never skipped Agent-Logs-Url: https://github.com/GoCodeAlone/claude-superpowers/sessions/24d5a30f-f7b3-43e7-83dd-60c470d0f4a4 Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> * Update docs/plans/2026-04-25-cross-llm-portability.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: intel352 <77607+intel352@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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
Copilot surfaced two line-ref errors in the
expected-new-result.mdfixture after PR #4 was already merged. This PR corrects them.The diff hunk is
@@ -10,6 +10,12 @@ func (d *Dispatcher) Create(req Request) error {. TheCreatefunction declaration appears as the@@context label at line 10. Actual hunk content starts at line 11, so:Create's"kind": d.kindInspectmethod start (Finding 1 Location)lib/dispatcher.go:17lib/dispatcher.go:18Delete's"kind": d.kindThree locations updated: bug-class scan citation, Finding 1 Location field, Finding 1 What's wrong text.
Verify Regression Invariant
This is a documentation-only fixture change — no production code path. Revert-and-restore proof applies to the correctness of the line numbers themselves:
diff.patchhunk+10,12counts"kind": d.kindin Create at new-file line 11 (wrong — the declaration is line 10, body starts at 11, "kind" is at 12)."kind": d.kindin Create correctly cited as line 12, matching the hunk arithmetic.Test plan
diff.patchhunk verified manually:@@ -10,6 +10,12 @@→ line 10 =func Createdeclaration → line 12 ="kind": d.kindin Create body → line 18 =func Inspect→ line 27 ="kind": d.kindin Deleteexpected-new-result.mdupdated and verifiedgit diff origin/main..HEADeyeballed line-by-line before push🤖 Generated with Claude Code