Skip to content

Conversation

@jjshanks
Copy link
Contributor

@jjshanks jjshanks commented Dec 10, 2025

Fix typo in testing-skills-with-subagents.md checklist:

  • "ith" → "with" in "Updated description with violation symptoms"

Motivation and Context

Simple typo fix discovered during documentation review. The checklist item was missing a letter, reducing readability.

How Has This Been Tested?

Visual review of the documentation change.

Breaking Changes

None. Documentation-only change.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • My code follows the repository's style guidelines
  • I have added or updated documentation as needed

Note: The original PR had 4 fixes, but after merging with upstream/main:

  • sharing-skills/SKILL.md was deleted upstream (skill obsoleted)
  • systematic-debugging/SKILL.md changes superseded by upstream refactoring
  • DOT diagram fixes were in the deleted/refactored files

Only the typo fix remains.

@coderabbitai
Copy link

coderabbitai bot commented Dec 10, 2025

📝 Walkthrough

Walkthrough

A minor typo correction in documentation: the word "ith" was changed to "with" in the REFACTOR Phase checklist within a markdown file about testing skills. No functional, logical, or control flow changes are present.

Changes

Cohort / File(s) Change Summary
Documentation Typo Fix
skills/writing-skills/testing-skills-with-subagents.md
Fixed typo in REFACTOR Phase checklist description: "ith violation symptoms" → "with violation symptoms"

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~1 minute

Poem

A rabbit hops through words with care,
Spotting typos hiding there!
"With" not "ith"—a simple fix,
Attention to detail does the trick! 🐰✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title 'docs: fix documentation accuracy issues in skills' is accurate and relevant. However, based on the PR objectives, the actual changes being merged are narrower (HEREDOC fix, typo fix, and test clarification only—without the DOT diagram changes that were requested to be dropped), making the title somewhat broader than the final scope.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@obra
Copy link
Owner

obra commented Dec 10, 2025

The graphviz changes look like they're ditching the superpowers graphviz format that uses quoted strings as identifiers. Was that on purpose?

@obra
Copy link
Owner

obra commented Dec 24, 2025

Thanks for catching the HEREDOC syntax bug and the typo - those are real fixes!

However, we'd like you to drop the DOT diagram changes. Looking at them critically:

  1. Readability regression: The original uses descriptive quoted strings as node names ("Bug appears deep in stack?") which are self-documenting. The new version uses cryptic identifiers with separate labels (start [label="..."]) requiring cross-referencing.

  2. Unnecessary complexity: Adding nodes like notdeep and done for obvious cases.

  3. Meaning change: The second diagram originally has "NEVER fix just the symptom" as a dead end (hard stop warning). The new version adds a dashed edge back to retry, which dilutes the message. The original intentionally says "this is wrong, don't do it" - not "try again with logging."

If you can update the PR to keep just the HEREDOC fix, typo fix, and test clarification, we'd be happy to merge those.

— Claude (in consultation with Jesse)

- Fix broken commit message HEREDOC syntax in sharing-skills/SKILL.md
  - Move entire message inside command substitution for valid bash

- Fix contradictory test requirements in systematic-debugging/SKILL.md
  - Clarify automated tests are "strongly preferred"
  - One-off scripts should be documented for regression testing
  - Requirement is for "reproducible test" not escape clause

- Fix typo in testing-skills-with-subagents/SKILL.md
  - "ith" -> "with" in checklist item
@jjshanks jjshanks force-pushed the fix-documentation-issues branch from b051537 to d268aae Compare December 26, 2025 20:05
Resolve conflicts:
- Accept deletion of skills/sharing-skills/SKILL.md (obsoleted upstream)
- Keep upstream's simpler formatting in systematic-debugging/SKILL.md
@jjshanks
Copy link
Contributor Author

@obra pushed an updated version and fixed the conflicts

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