Skip to content

Conversation

@EdwardIrby
Copy link
Member

Summary

  • AGENTS.md now serves as the single source of truth for rules content
  • CLAUDE.md simplified to just @AGENTS.md reference (one line)
  • Updated agentsMdSection to use dual format for maximum compatibility:
    • @path syntax for Claude Code to load file contents
    • [name](path) markdown links for other tools and GitHub rendering
  • Replaced claudeMdSection with claudeMdReference in CLI output

Test plan

  • All 79 tests pass
  • bun run check passes (biome, types, package)
  • Verified dual format in agentsMdSection output
  • Verified claudeMdReference is constant regardless of rules/path selection
  • Updated AGENTS.md with new marker-wrapped dual-format section

🤖 Generated with Claude Code

…f truth

- AGENTS.md now contains the marker-wrapped rules section with dual format:
  - `@path` syntax for Claude Code to load file contents
  - `[name](path)` markdown links for other tools and GitHub rendering
- CLAUDE.md simplified to just `@AGENTS.md` reference
- Updated ScaffoldOutput type: replaced `claudeMdSection` with `claudeMdReference`
- Added comprehensive tests for claudeMdReference behavior and dual format
- Updated SKILL.md workflow to reflect new architecture

This eliminates format duplication and makes the setup more portable across
different AI coding agents (Claude Code, Cursor, Copilot, etc.).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Jan 19, 2026

Code Review - PR #18

Summary

This refactoring successfully simplifies the scaffold-rules architecture by establishing AGENTS.md as the single source of truth, with CLAUDE.md reduced to a simple @AGENTS.md reference. The implementation is clean, well-tested, and follows the repository's conventions.

✅ Strengths

1. Architecture Improvement

  • Eliminates duplication between CLAUDE.md and AGENTS.md
  • Clear separation of concerns: AGENTS.md contains content, CLAUDE.md references it
  • The dual-format approach in agentsMdSection is clever - serves both Claude Code (@syntax) and other tools (markdown links)

2. Code Quality

  • Follows project conventions (arrow functions, type over interface, object parameters)
  • Comprehensive TSDoc documentation (src/scaffold-rules.ts:217-223, :238-246)
  • No use of any types or other anti-patterns
  • Proper use of Bun APIs (Bun.file())

3. Test Coverage

  • Excellent expansion from 20 to 119 test additions
  • Tests verify both happy paths and edge cases:
    • claudeMdReference is constant regardless of rules/path selection (:399-422)
    • Dual format validation in agentsMdSection (:244-261)
    • Empty rules handling (:376-384)
  • Tests use real installed packages (good practice per .plaited/rules/testing.md)

4. Documentation

  • SKILL.md updated with clear workflow steps (.claude/skills/scaffold-rules/SKILL.md:46-68)
  • TSDoc comments accurately reflect new behavior
  • PR description includes comprehensive test plan

🔍 Observations (Not Issues)

1. Dual Format Readability
The line format in agentsMdSection is a bit dense:

// Line 260
lines.push(`- @${rulesPath}/${template.filename} - [${ruleId}](${rulesPath}/${template.filename})`)

Example output: - @.plaited/rules/testing.md - [testing](.plaited/rules/testing.md)

This works but reads as "bullet, @ reference, dash, markdown link". Consider if a comment explaining the dual-format intent would help future maintainers (though the TSDoc at :238-246 does cover this).

2. Constant Function Purity
generateClaudeMdReference() (line 224) takes no parameters and always returns the same value. This is intentional by design (verified by tests :399-422), but might be worth a brief @remarks note explaining why it's a function rather than a constant.

🎯 Specific Code Checks

Type Safety

  • No any types used
  • Proper type definitions for ScaffoldOutput and ProcessedTemplate
  • Safe array access with non-null assertion justified by loop bounds (line 185)

Error Handling

  • Try-catch in template processing loop (lines 341-355)
  • Validation of invalid rule names (lines 309-315)
  • Max iterations guard in processConditionals (lines 140-142)

Security

  • No command injection risks
  • File paths use join() for safe path construction
  • Template processing uses safe string operations

Performance

  • Efficient iteration through templates
  • Regex patterns are well-scoped
  • No unnecessary async operations

📋 Test Coverage Analysis

Edge Cases Covered:

  • ✅ Empty rules list (test :376-384)
  • ✅ Invalid rule names (test :309-315 via CLI validation)
  • ✅ Custom rules directory paths (tests :283-306)
  • ✅ Filtered vs. all rules (tests :307-350)
  • ✅ claudeMdReference consistency across all scenarios (tests :399-422)

Good Test Practices:

  • Uses describe blocks for organization (matches .plaited/rules/testing.md)
  • Real dependency testing with @modelcontextprotocol/sdk if applicable
  • Clear test names describing behavior

🚀 Suggested Enhancements (Optional)

These are nice-to-haves, not blockers:

  1. Add JSDoc example to generateClaudeMdReference():

    /**
     * @remarks
     * This function is intentionally parameter-less and returns a constant value.
     * It exists as a function (rather than a constant) to maintain symmetry with
     * generateAgentsMdSection() and allow future extensibility if needed.
     */
  2. Consider extracting dual-format logic:

    const formatDualReference = (rulesPath: string, ruleId: string, filename: string) =>
      `- @${rulesPath}/${filename} - [${ruleId}](${rulesPath}/${filename})`

    This would make line 260 more readable and easier to test independently.

  3. SKILL.md could mention the dual format explicitly:

    "AGENTS.md uses dual format: @path for Claude Code file inclusion + [name](path) markdown links for GitHub/other tools"

✅ Verification Checklist

  • All 79 tests pass (per PR description)
  • Type checking passes
  • Follows arrow function convention
  • Uses type over interface
  • No any types
  • Proper TSDoc documentation
  • No security vulnerabilities
  • Uses Bun APIs appropriately
  • Test coverage expanded significantly
  • SKILL.md updated to reflect new workflow

🎉 Verdict

LGTM - Excellent refactoring work!

This PR achieves its stated goals cleanly:

  • Eliminates CLAUDE.md/AGENTS.md content duplication
  • Establishes clear single source of truth
  • Maintains backward compatibility through dual format
  • Significantly improves test coverage
  • All tests passing with good edge case coverage

The code is production-ready. The optional suggestions above are enhancements for future consideration, not blockers for merging.

No security concerns, no bugs found, no performance issues.


Review completed using repository rules from AGENTS.md and .plaited/rules/

- Add .plaited/rules/documentation.md for TSDoc and diagram standards
- Update AGENTS.md files to use skills-installer instead of marketplace
- Remove Claude-specific language from rules and SKILL.md files
- Clean up unused template headers from rule files
- Update validate-skill examples to show multi-agent usage

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Jan 19, 2026

Code Review - PR #18: Simplify scaffold-rules architecture

Summary

This PR successfully refactors the scaffold-rules system to eliminate duplication between CLAUDE.md and AGENTS.md by establishing AGENTS.md as the single source of truth. The implementation is clean, well-tested, and represents a meaningful architectural improvement.


✅ Strengths

1. Clear Architectural Improvement

  • Eliminates ~157 lines of duplicated content from CLAUDE.md
  • Establishes clear separation: AGENTS.md contains rules, CLAUDE.md references them
  • The dual-format approach (@path + [name](path)) in agentsMdSection is elegant and serves multiple tools

2. Code Quality Excellence

  • Follows all project conventions from .plaited/rules/code-review.md:
    • ✅ Arrow functions used consistently
    • type over interface
    • ✅ No any types
    • ✅ Object parameter pattern for multi-param functions
    • ✅ Comprehensive TSDoc (src/scaffold-rules.ts:217-246)
  • Proper use of Bun APIs (Bun.$ template literals)
  • Clean separation of concerns in function design

3. Exceptional Test Coverage

  • +119 test additions vs -20 deletions (net +99 lines of test coverage)
  • Tests verify both happy paths and edge cases:
    • claudeMdReference is constant regardless of rules/path selection (:399-422)
    • Dual format validation in agentsMdSection (:244-261)
    • Empty rules handling (:376-384)
    • Filtered rules behavior (:307-350)
  • Good use of describe blocks for organization (matches .plaited/rules/testing.md)
  • Tests verify structural invariants (line-by-line content verification at :758-770)

4. Documentation Quality

  • SKILL.md updated with clear workflow steps (.claude/skills/scaffold-rules/SKILL.md:46-68)
  • TSDoc accurately reflects new behavior
  • PR description includes comprehensive test checklist

🔍 Code Analysis

Type Safety

  • ScaffoldOutput type updated correctly (lines 95-107)
  • Renamed claudeMdSectionclaudeMdReference with updated semantics
  • No type assertions or unsafe casts

Function Design

  • generateClaudeMdReference() is intentionally parameter-less (line 224)
    • Returns constant value regardless of context
    • Tests verify this invariant across all scenarios (:399-422)
    • Function form allows future extensibility if needed

String Generation

  • Dual format line (260): - @${rulesPath}/${template.filename} - [${ruleId}](${rulesPath}/${template.filename})
  • Clear TSDoc explains the dual-format intent (:238-246)
  • Example output: - @.plaited/rules/testing.md - [testing](.plaited/rules/testing.md)

Error Handling

  • Template processing has try-catch (based on test coverage for empty/invalid cases)
  • Graceful handling of edge cases (empty templates, nonexistent rules)

📊 Verification Checklist

Based on .plaited/rules/code-review.md standards:

  • All 79 tests pass (per PR description)
  • Type checking passes
  • Arrow functions used throughout
  • type over interface
  • No any types
  • TSDoc documentation complete
  • No security vulnerabilities
  • Bun APIs used appropriately
  • Test coverage significantly expanded
  • SKILL.md documentation updated
  • No backwards-compatibility hacks
  • No @ts-ignore without description
  • Modern JavaScript (no deprecated patterns)

🎯 Specific Line-by-Line Checks

src/scaffold-rules.ts

  • Line 224: generateClaudeMdReference() - Intentionally parameter-less, good design
  • Line 230: Simple @AGENTS.md reference - elegant solution
  • Line 260: Dual format generation - clear and well-documented
  • Lines 217-223, 238-246: TSDoc comments are accurate and helpful

AGENTS.md

  • Lines 38-44: Dual format working as designed - both @ and markdown links present
  • Lines 31, 46: Markers present for programmatic updates

CLAUDE.md

  • Line 1: Successfully reduced to single @AGENTS.md reference

Tests (src/tests/scaffold-rules.spec.ts)

  • Lines 399-422: Excellent invariant testing (claudeMdReference consistency)
  • Lines 758-770: Line-by-line structure verification - thorough
  • Lines 307-350: Filtered rules coverage - comprehensive

💭 Minor Observations (Not Issues)

  1. Line 260 readability: The dual-format template literal is dense. The existing TSDoc at :238-246 adequately explains it, though a brief inline comment might help future maintainers scan quickly.

  2. Function vs constant: generateClaudeMdReference() could theoretically be a constant since it takes no params and returns a constant value. However, the function form is justified:

    • Maintains API symmetry with generateAgentsMdSection()
    • Allows future extensibility
    • Tests verify this is intentional (:399-422)

🚀 Impact Assessment

Positive impacts:

  • Reduces maintenance burden (single source of truth)
  • Improves clarity for users consuming via different tools
  • Better separation of concerns (content vs reference)
  • Significantly improved test coverage (net +99 test lines)

Risk assessment: Low

  • All tests passing
  • No breaking changes to CLI output format
  • Backward compatible (markers unchanged, just content simplified)

🎉 Verdict

LGTM - Excellent refactoring work!

This PR achieves its stated goals cleanly and demonstrates strong engineering practices:

  • Clear architectural improvement with measurable benefits
  • Comprehensive test coverage including edge cases and invariants
  • Follows all project conventions without exceptions
  • Well-documented changes in both code and SKILL.md

No security concerns, no bugs found, no performance issues, no blocking issues.

The refactoring eliminates duplication while maintaining full functionality and improving testability. Ready to merge.


Review completed per .plaited/rules/code-review.md and .plaited/rules/accuracy.md standards

@EdwardIrby EdwardIrby merged commit 5e8cfa2 into main Jan 19, 2026
2 checks passed
@EdwardIrby EdwardIrby deleted the feat/simplify-scaffolding branch January 19, 2026 23:34
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