Skip to content

feat(skills): add public visual-qa skill for web UI review - #338

Closed
fabiolavillatoro2003 wants to merge 1 commit into
kunchenguid:mainfrom
fabiolavillatoro2003:fm/visual-qa-skill
Closed

fabiolavillatoro2003 wants to merge 1 commit into
kunchenguid:mainfrom
fabiolavillatoro2003:fm/visual-qa-skill

Conversation

@fabiolavillatoro2003

Copy link
Copy Markdown

Intent

The developer (acting as a firstmate crewmate on firstmate's own repo) tasked the agent with creating a new public, installer-facing skill at skills/visual-qa/SKILL.md that distills UI-review knowledge currently duplicated in two lower-fidelity places: an exhortation in the global CLAUDE.md and a concrete checklist in trashtalknyc-website's docs/agent-protocol.md. Requirements: base the work on an existing scout audit report (candidate #2) as rationale/content source; include the breakpoint matrix (1440/1280/768/375px), an interactive/hover/empty/success state checklist, the "passing tests does not imply visual quality" framing, a subjective-feedback translation table, and a DX-report closing format; keep it project-agnostic (not trashtalknyc-specific) and placed in public skills/ (not agent-only .agents/skills/), matching the skills/stow precedent, with no "internal" metadata and a clear trigger/description. The developer explicitly said not to touch the trashtalknyc-website repo/clone from this worktree — that doc-shrinking follow-up should just be noted for a separate task. Work had to follow firstmate-coding-guidelines (knowledge placement, shellcheck, one-sentence-per-line markdown, plain dash, no co-author), be committed on a dedicated branch, and pushed/validated through the no-mistakes pipeline; a GitHub push-permission blocker was resolved by the developer configuring a fork, after which they asked the agent to rerun the pipeline from the failed push step.

What Changed

  • Add skills/visual-qa/SKILL.md, a new public, project-agnostic skill for reviewing web UI changes: a breakpoint matrix (1440/1280/768/375px), an interactive/hover/empty/success state checklist, a "passing tests does not imply visual quality" framing, a subjective-feedback translation table, and a structured closing report format.
  • Update README.md to reference the new skill alongside existing public skills.

Risk Assessment

✅ Low: The change is documentation-only (a new public, installer-facing SKILL.md plus a two-line README update), closely mirrors the existing skills/stow precedent in frontmatter shape, style, and public/agent-only placement, stays project-agnostic as required, and does not touch any project clone or executable code.

Testing

The baseline shell test suite (47 tests/*.test.sh files covering bin/ scripts) already passed and is unaffected by this change, since the diff only touches README.md and a new public skill document. Since this is a documentation/content change with no runtime or rendered UI surface of its own, I verified the actual deliverable directly: the SKILL.md frontmatter parses correctly and matches the skills/stow public-skill precedent (no internal metadata, user-invocable), and the body contains every required element from the intent (breakpoint matrix, state checklist, "passing tests does not imply visual quality" framing, subjective-feedback translation table, closing report format), is project-agnostic, and correctly lives in public skills/ rather than .agents/skills/. Confirmed trashtalknyc-website was not touched and the working tree is clean. No issues found.

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"
  • git diff 2061c37..e77af1c --stat — confirmed only README.md and skills/visual-qa/SKILL.md changed; projects/trashtalknyc-website untouched as required
  • Parsed the SKILL.md YAML frontmatter — valid, matches the skills/stow public-skill precedent exactly (name, description, user-invocable: true, no metadata.internal)
  • Grepped file content for required elements: breakpoint matrix (1440/1280/768/375px), the 'passing tests does not imply visual quality' framing, hover/interactive/empty/success state checklist items, the subjective-feedback translation table, and the 4-part closing report format — all present
  • Confirmed the skill is project-agnostic (no trashtalknyc references) and correctly placed under public skills/ rather than agent-only .agents/skills/
  • Checked README.md diff for consistency with the new skill and one-sentence-per-line markdown style
  • bash tests/*.test.sh baseline suite (already run per task setup) — unrelated to this diff since no bin/ scripts changed
  • git status --porcelain — working tree clean, no stray artifacts
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Distills the UI-review knowledge previously living only as an
exhortation in the captain's global instructions and as a concrete
checklist inside one project's agent-protocol doc into a standalone,
installer-facing skill: breakpoint matrix, interactive/empty/success
state checklist, subjective-feedback translation table, and a closing
report format for human verification.

Documented alongside skills/stow in README's two-tier skill layout.
@fabiolavillatoro2003

Copy link
Copy Markdown
Author

Closing without merging - this is an internal-only change for our own firstmate installation, not an upstream contribution. Landing it directly on our local main instead.

@AdamTerhaerdt

Copy link
Copy Markdown

@coderabbitai review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants