docs(fix-review): tighten weak language and add trigger phrase

[ralphbean]

Ran skillsaw (evaluating it per #188) against the repo's skills.
Adds a "Use when..." trigger phrase to the description, rewrites the
trigger_source field explanation as a positive instruction instead of
a "must be / do NOT" pair, and suppresses a false-positive
placeholder-text match on the phrase "TODO comments" in a checklist
item.

Related to #188

Assisted-by: Claude Opus 4.6 noreply@anthropic.com
Signed-off-by: Ralph Bean rbean@redhat.com

---

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[qodo-code-review]

PR Summary by Qodo

Tighten Fix Review skill wording and suppress a skillsaw false positive

📝 Documentation 🕐 Less than 10 minutes

AI Description

• Add a clear “Use when…” trigger phrase to the Fix Review skill description.
• Reword trigger_source guidance as a positive instruction while preserving validation details.
• Suppress a skillsaw placeholder-text false positive for the checklist phrase “TODO comments”.

Diagram

graph TD
  A["Skill author"] --> B["SKILL.md (fix-review)"] --> C["skillsaw"] --> D["Reviewers"]

Loading

High-Level Assessment

This is the right approach for a docs-focused skill: clarify intent with an explicit “Use when…” trigger and keep schema constraints explicit, while using the narrowest possible suppression to avoid a known false-positive on checklist wording.

Files changed (1) +7 / -5

Documentation (1) +7 / -5

SKILL.mdClarify trigger guidance and suppress checklist false-positive +7/-5

Clarify trigger guidance and suppress checklist false-positive

• Adds a “Use when…” sentence to make the skill’s intended usage explicit. Rewrites 'trigger_source' instructions as a positive directive while retaining the schema/enum constraints. Adds a targeted 'skillsaw-disable-next-line content-placeholder-text' comment to avoid misclassifying the phrase “TODO comments” as placeholder text.

skills/fix-review/SKILL.md

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:00 PM UTC · Completed 5:12 PM UTC

Commit: 808280a · View workflow run →

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (1)

1. Protected skills/ file modified 📜 Skill insight § Compliance

Description

This PR modifies skills/fix-review/SKILL.md, which is a protected governance/infrastructure path
and must not be auto-approved. The change requires explicit human review per the protected-path
compliance rule.

Code

skills/fix-review/SKILL.md[R7-8]

+  post-script. Use when implementing fixes for review comments left on
+  an open PR.

Relevance

●●● Strong

Protected skills/ path changes historically flagged as requiring explicit human approval;
compliance gating expected.

PR-#157
PR-#59

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The compliance checklist flags any modifications under protected paths (including skills/) as
requiring a finding to prevent auto-approval. The diff shows edits in skills/fix-review/SKILL.md.

skills/fix-review/SKILL.md[7-8]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR modifies a protected path (`skills/`), which requires explicit justification and human review; it must not be auto-approved.

## Issue Context
Protected governance/infrastructure paths require heightened scrutiny.

## Fix Focus Areas
- skills/fix-review/SKILL.md[7-8]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Comment-only skillsaw-disable directive 📘 Rule violation ⚙ Maintainability

Description

A new HTML comment disable-directive was added without any functional/non-comment change in the
associated adjacent lines. This violates the rule prohibiting comment-only modifications when the
documented behavior is otherwise unchanged.

Code

skills/fix-review/SKILL.md[331]

+<!-- skillsaw-disable-next-line content-placeholder-text -->

Relevance

●●● Strong

Repo has rejected standalone comment-only edits; this disable-directive is comment-only in its block
and will be asked to change.

PR-#631

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The rule requires comment changes to be paired with functional/non-comment changes in the same
associated block. The diff adds only the HTML comment disable directive immediately before an
existing checklist bullet line.

Rule 1540690: Do not change comments on lines without functional code changes in a diff
skills/fix-review/SKILL.md[331-332]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A comment-only change (`<!-- skillsaw-disable-next-line ... -->`) was introduced without an accompanying functional/content change in the same block, which violates the comment-only modification rule.

## Issue Context
The directive appears to be used to suppress a tooling false-positive. To comply, prefer changing the adjacent content so the suppression is unnecessary, or make an actual associated change in the documented block rather than only adding a comment.

## Fix Focus Areas
- skills/fix-review/SKILL.md[331-332]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context

✅ Compliance rules (platform): 55 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

Tip of the day

💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] skills/fix-review/SKILL.md — This file is under the skills/ protected path. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and provides rationale (skillsaw evaluation producing documentation improvements). Human approval is always required for protected-path changes, regardless of context.

Previous run

Review

Findings

Medium

• [protected-path] skills/fix-review/SKILL.md — This file is under the skills/ protected path. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and provides rationale (skillsaw evaluation producing documentation improvements). Human approval is always required for protected-path changes, regardless of context.

Low

• [documentation-tone] skills/fix-review/SKILL.md:429 — The rewritten trigger_source field documentation uses a different instructional tone ('Use the normalized trigger type... not the raw') from the established imperative voice used elsewhere in the file (e.g., 'must be strictly limited', 'Do NOT re-run').
  Remediation: Either revert to the imperative tone or apply the tone change consistently across all imperative instructions in the file.

Previous run (2)

Review

Findings

Medium

• [protected-path] skills/fix-review/SKILL.md — This file is under the skills/ protected path. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and provides rationale (skillsaw evaluation producing documentation improvements). Human approval is always required for protected-path changes, regardless of context.

Low

• [structure] skills/fix-review/SKILL.md:7 — The "Use when..." trigger phrase is placed at the end of the description. Among the 5 skills that include a "Use when" phrase (out of 10 total), all 5 place it at the beginning, not the end.
  Remediation: Move "Use when implementing fixes for review comments left on an open PR." to the beginning of the description to match the pattern used by the 5 other skills that include a trigger phrase.

Previous run (3)

Review

Findings

Medium

• [protected-path] skills/fix-review/SKILL.md — This file is under the skills/ protected path. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and provides rationale (skillsaw evaluation producing documentation improvements). Human approval is always required for protected-path changes, regardless of context.

Low

• [structure] skills/fix-review/SKILL.md:7 — The "Use when..." trigger phrase is placed at the end of the description. When SKILL.md descriptions include this phrase, the established convention (5 of 5 instances) places it at the beginning, not the end.
  Remediation: Move "Use when implementing fixes for review comments left on an open PR." to the beginning of the description to match the established pattern in autonomy-readiness, agent-scaffolding, finding-agent-runs, merge-queue, and retro-analysis skills.

Previous run (4)

Review

Findings

Medium

• [protected-path] skills/fix-review/SKILL.md — This file is under the skills/ protected path. The PR links to issue Evaluate skillsaw for linting agent instructions #188 and provides rationale (skillsaw evaluation producing documentation improvements). Human approval is always required for protected-path changes, regardless of context.

Low

• [structure] skills/fix-review/SKILL.md:7 — The "Use when..." trigger phrase is appended at the end of the existing description. Some skills (autonomy-readiness, merge-queue, agent-scaffolding, finding-agent-runs, retro-analysis) place "Use when..." at the beginning, but an equal number (code-review, docs-review, pr-review, issue-labels, code-implementation) do not use this pattern. The convention is not strongly established, but moving it to the start would align with the subset that does.
  Remediation: Optional — move "Use when implementing fixes for review comments left on an open PR." to the start of the description to match the subset of skills that use that convention.

---

Labels: PR modifies skill documentation (skills/fix-review/SKILL.md) with no code changes

[ralphbean]

/fs-fix address @waynesun09's findings.

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 8:47 PM UTC · Completed 8:52 PM UTC

Commit: 808280a · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 1 review finding from @waynesun09. Indented the skillsaw HTML comment to fix markdown list rendering without changing directive semantics.

Fixed (1):

1. skillsaw HTML comment between list items breaks markdown rendering (skills/fix-review/SKILL.md): Indented the skillsaw-disable-next-line HTML comment by 2 spaces so it becomes continuation content of the preceding list item, preserving the four-item checklist as a single
   on GitHub while keeping the disable-next-line directive correctly targeting the TODO comments bullet

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 8:53 PM UTC · Completed 9:02 PM UTC

Commit: cca982f · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:39 PM UTC · Completed 9:51 PM UTC

Commit: 6def28e · View workflow run →

[waynesun09]

Reviewed via 4-agent squad (claude-coder, claude-researcher, grok-review-agent, cursor-code-review) + verification pass at head 6def28e — no CRITICAL/HIGH/MEDIUM findings survived verification.

[ralphbean]

/fs-fix move the "Use when implementing" phrase to the beginning of the description

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 10:46 PM UTC · Completed 10:50 PM UTC

Commit: 6def28e · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Moved the 'Use when implementing' trigger phrase from the end of the skill description to the beginning, as requested by the human reviewer.

Fixed (1):

1. move trigger phrase to beginning of description (skills/fix-review/SKILL.md): Moved the 'Use when implementing fixes for review comments left on an open PR.' sentence from the end of the YAML frontmatter description to the beginning, per human instruction.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:51 PM UTC · Completed 11:02 PM UTC

Commit: 9097caf · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:38 PM UTC · Completed 3:51 PM UTC

Commit: 585c707 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 4:11 PM UTC · Completed 4:23 PM UTC

Commit: 585c707 · View workflow run →

[fullsend-ai-retro]

Retro: PR #751 — docs(fix-review): tighten weak language and add trigger phrase

A 1-file, 6-line documentation change to skills/fix-review/SKILL.md from the skillsaw evaluation effort (#188). The PR added a "Use when" trigger phrase, rewrote trigger_source guidance to positive phrasing, and suppressed a false-positive skillsaw lint match with an HTML comment.

Timeline

Phase	What happened
PR opened (Aug 11)	Review agent ran in 12 min, found Low structure issue ("Use when" phrase at end vs beginning) and Medium protected-path gate. qodo-code-review flagged protected-path compliance and a debatable "comment-only" finding.
Human review (Aug 11, +2h)	waynesun09 identified the only actual bug: the unindented <!-- skillsaw-disable-next-line --> HTML comment between list items broke the markdown into two separate <ul> elements. Verified via GitHub's markdown API.
Fix 1 (Aug 12)	Author triggered /fs-fix address @waynesun09's findings. Fix agent correctly indented the HTML comment by 2 spaces to preserve list continuity. Completed in 5 min — surgical and correct.
Fix 2 (Aug 13)	Author triggered /fs-fix move the "Use when implementing" phrase to the beginning of the description. Fix agent relocated the phrase as instructed. Completed in 4 min.
Approval (Aug 13)	waynesun09 approved after a 4-agent squad verification pass found no surviving CRITICAL/HIGH/MEDIUM findings.
Merge (Aug 14)	Merged after final rebase and review.

What went well

• Fix agent quality was strong. Both /fs-fix iterations produced surgical, correct changes with clear commit messages. The fix agent correctly parsed a human reviewer's inline comment (fix 1) and explicit instructions (fix 2).
• Protected-path gate worked as designed. The skills/ path triggered requires-manual-review, ensuring a human reviewed the change — which is exactly where the rendering bug was caught.
• The review agent's "Use when" placement finding was valid and led to fix iteration 2, improving consistency with the repo's skill description conventions.

What could go better

• The review agent missed the only actual bug across 3 pre-fix runs. The unindented HTML comment breaking markdown list rendering was detectable (it requires understanding GFM list continuation rules) and was in-diff. The correctness sub-agent did not surface it. This is the same class of bug that occurred on sibling PR docs(code-implementation): tighten weak language and add trigger phrase #748 from the same skillsaw batch.
• 5 review runs for a 6-line doc change is excessive. Runs 2 and 3 reproduced the same structure finding from run 1 verbatim. The protected-path finding was repeated in all 5 runs. Total: 7 agent runs (5 reviews + 2 fixes) for a change that could have been reviewed once.
• A non-obvious tone finding appeared only on run 4. Investigation showed this was diff-dependent, not LLM non-determinism — the structure finding was fixed by that point, so the agent surfaced a secondary observation. This is expected behavior but highlights that the review agent does not guarantee finding completeness on any single pass.

Evidence for existing issues (no new proposals needed)

• agents#802 (Correctness sub-agent should detect GFM list-breaking insertions): This PR provides direct corroborating evidence — the same rendering bug class was missed by the review agent on this sibling PR, confirming the gap is systematic, not a one-off on PR docs(code-implementation): tighten weak language and add trigger phrase #748.
• fullsend#5139, fullsend#4681, agents#108, agents#323: 5 review runs on a 6-line doc PR with runs 2–3 producing zero new information. This is a concrete cost data point for the review dedup effort.
• fullsend#2794: The protected-path finding was posted identically across all 5 runs with no change in status or context.
• agents#721: Stable finding identifiers would have enabled dedup of the repeated structure finding across runs 1–3.
• agents#373: All 6 review dimensions were dispatched for a single-file markdown edit. Security, cross-repo-contracts, and potentially style-conventions sub-agents produced no findings — a lighter review path for docs-only changes would have saved tokens.

No novel proposals are warranted. All identified improvement opportunities are already tracked by open issues with clear remediation paths.