Skip to content

docs(code-implementation): tighten weak language and add trigger phrase - #748

Merged
ralphbean merged 3 commits into
mainfrom
skillsaw/fix-code-implementation
Aug 14, 2026
Merged

docs(code-implementation): tighten weak language and add trigger phrase#748
ralphbean merged 3 commits into
mainfrom
skillsaw/fix-code-implementation

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Ran skillsaw (evaluating it per #188) against the repo's skills.
Adds a "Use when..." trigger phrase to the description so
description-based routing has a clear match condition, condenses a
duplicated directive about using the local target-branch ref, 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 CLIGive Feedback 💬

@ralphbean
ralphbean requested a review from a team as a code owner August 11, 2026 16:59
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Clarify code-implementation skill trigger phrase and reduce skillsaw false positives

📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Add a clear "Use when..." trigger phrase for description-based skill routing.
• Consolidate guidance to prefer local target-branch refs over origin refs.
• Suppress skillsaw placeholder-text false positive for a "TODO comments" checklist item.
Diagram

graph TD
  Agent([Implementation agent]) --> SKILL["skills/code-implementation/SKILL.md"] --> Router([Description router]) --> Reviewer([Review agent])
  Repo[(Repo)] --> Skillsaw([skillsaw linter]) --> Agent --> SKILL
  subgraph Legend
    direction LR
    _doc["Markdown doc"] ~~~ _proc(["Process"]) ~~~ _repo[("Repo")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add explicit routing metadata (separate from prose description)
  • ➕ Removes reliance on exact phrasing like "Use when..." for skill selection
  • ➕ Easier to validate and lint as structured data
  • ➕ Avoids future edits to prose breaking routing
  • ➖ Requires router changes and a schema decision for all skills
  • ➖ Bigger migration surface than a targeted doc tweak
2. Centralize skillsaw allowlist/ignore rules
  • ➕ Avoids inline suppression comments in the document
  • ➕ Applies consistently across skills with the same false positive
  • ➖ Harder to understand suppression rationale when reading the doc
  • ➖ May mask legitimate placeholder-text findings in other contexts

Recommendation: The PR’s approach is appropriate for the goal: it fixes routing clarity and a known false positive with minimal surface area. If routing accuracy becomes fragile or requires more trigger patterns, consider moving toward explicit, structured routing metadata.

Files changed (1) +10 / -10

Documentation (1) +10 / -10
SKILL.mdStrengthen description routing cue, refine branch-diff guidance, and suppress lint FP +10/-10

Strengthen description routing cue, refine branch-diff guidance, and suppress lint FP

• Adds a "Use when..." sentence to the skill description to support description-based routing. Rewrites the git-diff testing guidance to remove duplicated rationale while still emphasizing local target-branch refs. Adds an inline skillsaw suppression comment to avoid flagging "TODO comments" as placeholder text.

skills/code-implementation/SKILL.md

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

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

Commit: 44d771d · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Protected skills/ file modified 📜 Skill insight § Compliance
Description
This PR modifies skills/code-implementation/SKILL.md, which is in a protected
governance/infrastructure path and therefore must not be auto-approved and requires explicit human
review.
Code

skills/code-implementation/SKILL.md[R6-7]

+  tests and linters, and commits to a feature branch. Use when implementing
+  a triaged GitHub issue end-to-end into a committed, tested change.
Relevance

●●● Strong

Repo enforces protected-path checks; skills/ changes are treated as requiring explicit human review.

PR-#569

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1538392 lists skills/ as a protected path and requires raising a finding for any
modifications under it. The diff shows changes within skills/code-implementation/SKILL.md.

skills/code-implementation/SKILL.md[6-7]
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
This PR changes a protected path (`skills/`), which must never be auto-approved and requires explicit human review/approval.

## Issue Context
Protected governance/infrastructure paths require heightened review controls.

## Fix Focus Areas
- skills/code-implementation/SKILL.md[6-7]

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


Grey Divider

Context
✅ Compliance rules (platform): 55 rules
✅ Skills: 4 invoked
  code-review
  code-implementation
  pr-review
  docs-review

Grey Divider

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 ↗

Grey Divider

Qodo Logo

Comment thread skills/code-implementation/SKILL.md Outdated
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] skills/code-implementation/SKILL.md — This file is under the skills/ protected path. The PR is linked to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale for the changes (applying skillsaw linter findings to tighten weak language and add a trigger phrase). Human approval is always required for protected-path changes, regardless of context.
Previous run

Review

Findings

Medium

  • [protected-path] skills/code-implementation/SKILL.md — This file is under the skills/ protected path. The PR is linked to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale for the changes (applying skillsaw linter findings to tighten weak language and add a trigger phrase). Human approval is always required for protected-path changes, regardless of context.
Previous run (2)

Review

Findings

Medium

  • [protected-path] skills/code-implementation/SKILL.md — This file is under the skills/ protected path. The PR is linked to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale for the changes (applying skillsaw linter findings to tighten weak language and add a trigger phrase). Human approval is always required for protected-path changes, regardless of context.
Previous run (3)

Review

Findings

Medium

  • [protected-path] skills/code-implementation/SKILL.md — This file is under the skills/ protected path. The PR is linked to issue Evaluate skillsaw for linting agent instructions #188 and explains the rationale for the changes (evaluating skillsaw linter against instruction files). Human approval is always required for protected-path changes, regardless of context.

Labels: PR modifies skill documentation (SKILL.md)

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment documentation Improvements or additions to documentation labels Aug 11, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review pass (2 findings).

Comment thread skills/code-implementation/SKILL.md Outdated
Comment thread skills/code-implementation/SKILL.md Outdated
@ralphbean

Copy link
Copy Markdown
Member Author

/fs-fix address both of @waynesun09's findings

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🤖 Finished Fix · ❌ Failure · Started 9:37 PM UTC · Completed 9:42 PM UTC

Commit: 44d771d · View workflow run →

Base automatically changed from skillsaw/fix-autonomy-readiness to main August 13, 2026 21:38
Ran skillsaw (evaluating it per #188) against the repo's skills.
Adds a "Use when..." trigger phrase to the description so
description-based routing has a clear match condition, condenses a
duplicated directive about using the local target-branch ref, 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>
@ralphbean
ralphbean force-pushed the skillsaw/fix-code-implementation branch from 44d771d to 3ba23d5 Compare August 13, 2026 21:38
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 13, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 9:39 PM UTC · Ended 9:50 PM UTC

Commit: 3ba23d5 · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

⚠️ Post-fix script failed — Push rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/31746438481

Details:
To https://github.com/fullsend-ai/agents.git
! [rejected] skillsaw/fix-code-implementation -> skillsaw/fix-code-implementation (fetch first)
error: failed to push some refs to 'https://github.com/fullsend-ai/agents.git'
hint: Updates were rejected because the remote contains work that you do not
hint: have locally. This is usually caused by another repository pushing to
hint: the same ref. If you want to integrate the remote changes, use
hint: 'git pull' before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
To https://github.com/fullsend-ai/agents.git
! [rejected] skillsaw/fix-code-implementation -> skillsaw/fix-code-implementation (stale info)
error: failed to push some refs to 'https://github.com/fullsend-ai/agents.git'
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

@fullsend-ai-review

Copy link
Copy Markdown

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

Commit: 3ba23d5 · View workflow run →

@ralphbean

Copy link
Copy Markdown
Member Author

/fs-fix fix the trigger phrase placement. put "use when .." at the front of the description.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

🤖 Finished Fix · ✅ Success · Started 12:51 AM UTC · Completed 12:56 AM UTC

Commit: 3ba23d5 · View workflow run →

Place the "Use when ..." clause at the start of the skill description
so description-based routing matches on it immediately.

Addresses review feedback on #748
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

🔧 Fix agent — iteration 1 (human-triggered)

Moved the 'Use when ...' trigger phrase from the end of the skill description to the front, per human instruction.

Fixed (1):

  1. trigger phrase placement - put 'use when ..' at the front of the description: Moved the 'Use when implementing a triaged GitHub issue end-to-end into a committed, tested change.' sentence from the end of the description to the front, so description-based routing matches on the trigger phrase immediately.

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

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:57 AM UTC · Completed 1:06 AM UTC

Commit: b89aa07 · View workflow run →

@ralphbean

Copy link
Copy Markdown
Member Author

/fs-fix fix the Unindented skillsaw-disable comment

@fullsend-ai-coder

fullsend-ai-coder Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

🤖 Finished Fix · ✅ Success · Started 1:15 PM UTC · Completed 1:20 PM UTC

Commit: b89aa07 · View workflow run →

Indent the skillsaw-disable-next-line HTML comment by 2 spaces so it
sits inside the Markdown bullet list rather than breaking the list flow.

Addresses review feedback on #748
@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

🔧 Fix agent — iteration 2 (human-triggered)

Indented the skillsaw-disable HTML comment by 2 spaces to fix it within the Markdown list. Pre-existing test failures in post-retro-test.sh are unrelated to this documentation-only change.

Fixed (1):

  1. Unindented skillsaw-disable comment: Indented the HTML comment by 2 spaces so it sits properly within the Markdown bullet list context rather than breaking the list flow at column 0.

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

fullsend-ai-review Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:21 PM UTC · Completed 1:32 PM UTC

Commit: 3fd69a4 · View workflow run →

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both review findings resolved: the skillsaw-disable comment is now indented into the list (3fd69a4), and the trigger phrase is front-loaded (b89aa07). CI is green. Approving as the required human review for this protected skills/ path change.

@ralphbean
ralphbean added this pull request to the merge queue Aug 14, 2026
Merged via the queue into main with commit 6f21584 Aug 14, 2026
12 checks passed
@ralphbean
ralphbean deleted the skillsaw/fix-code-implementation branch August 14, 2026 15:36
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 14, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 3:39 PM UTC · Completed 3:50 PM UTC

Commit: 3fd69a4 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #748docs(code-implementation): tighten weak language and add trigger phrase

Human-authored PR by ralphbean modifying skills/code-implementation/SKILL.md (+12/−12 lines). Three changes: added a "Use when..." trigger phrase to the skill description, condensed a duplicated directive, and inserted a skillsaw-disable-next-line HTML comment to suppress a false-positive lint match. Related to #188 (skillsaw evaluation).

Timeline

  1. Aug 11 16:59 — PR created. Review agent and qodo bot both flag the protected-path governance finding (skills/ directory). No substantive content findings from any automated review.
  2. Aug 13 12:55 — Human reviewer (waynesun09) posts 2 findings: (HIGH) unindented HTML comment breaks the Markdown checklist into two <ul> elements on GitHub; (MEDIUM) trigger phrase appended at end violates repo convention where all other skills front-load "Use when..." as the first sentence.
  3. Aug 13 21:36/fs-fix address both findings triggers fix agent run 31746438481. Fails: ralphbean pushed to the branch ~54s after the fix agent checked out, causing a non-fast-forward rejection. --force-with-lease correctly prevented overwriting the human commit.
  4. Aug 14 00:50/fs-fix for trigger phrase placement. Fix agent succeeds, commit b89aa07.
  5. Aug 14 13:14/fs-fix for HTML comment indentation. Fix agent succeeds, commit 3fd69a4.
  6. Aug 14 13:26 — waynesun09 approves: both findings resolved, CI green, human review gate satisfied.
  7. Aug 14 15:36 — PR merged.

Key findings

Review quality gap: The review agent ran 4 times across the PR lifecycle and produced only the procedural protected-path finding each time. Both substantive issues — the GFM rendering breakage and the convention violation — were caught exclusively by the human reviewer. The style-conventions sub-agent's exploration budget likely prevented it from reading peer SKILL.md files in sibling directories to detect the trigger phrase placement convention. The correctness sub-agent lacked guidance on GFM list continuation semantics.

Fix agent resilience: The first fix attempt failed due to a push race condition, requiring two additional manual /fs-fix invocations split by finding. The race was handled safely (no data loss), but the human had to decompose and re-issue the fix request.

Evidence for existing issues

Proposals

Two proposals filed below targeting the style-conventions and correctness sub-agent definitions to close the gaps the human reviewer exposed.

Proposals filed

ggallen pushed a commit that referenced this pull request Aug 19, 2026
Place the "Use when ..." clause at the start of the skill description
so description-based routing matches on it immediately.

Addresses review feedback on #748
ggallen pushed a commit that referenced this pull request Aug 19, 2026
Indent the skillsaw-disable-next-line HTML comment by 2 spaces so it
sits inside the Markdown bullet list rather than breaking the list flow.

Addresses review feedback on #748
ggallen pushed a commit to ggallen/agents that referenced this pull request Aug 19, 2026
Place the "Use when ..." clause at the start of the skill description
so description-based routing matches on it immediately.

Addresses review feedback on fullsend-ai#748
ggallen pushed a commit to ggallen/agents that referenced this pull request Aug 19, 2026
Indent the skillsaw-disable-next-line HTML comment by 2 spaces so it
sits inside the Markdown bullet list rather than breaking the list flow.

Addresses review feedback on fullsend-ai#748
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation fullsend-fix requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants