🐛 Fix Storybook skip filtering via vizzly-skip tag (#215)

[Robdel12]

Summary

• add support for tag-based skip filtering via vizzly-skip during story discovery
• keep parameters.vizzly.skip support for backwards compatibility
• add debug logs to show why a story is filtered out
• add crawler tests for tag-only and mixed tag+parameter skip cases
• update Storybook README and example story to recommend tags: [vizzly-skip]

Test plan

• npm test -- tests/crawler.test.js (from clients/storybook)
• Verify against a real Storybook 7+ build with tagged stories

Closes #215

[claude]

Code Review

Summary

This PR successfully addresses issue #215 by adding support for the vizzly-skip tag to filter out stories in Storybook 7+, where custom parameters are stripped from index.json. The implementation is clean, well-tested, and maintains backward compatibility.

✅ Strengths

1. Correct Problem Resolution

• Properly addresses the root cause: Storybook 7+ strips custom parameters from index.json
• Uses tags which ARE preserved in the build output
• Maintains backward compatibility by keeping parameters.vizzly.skip support

2. Code Quality

• Clean, focused implementation with good separation of concerns (hasVizzlySkipTag helper function)
• Proper JSDoc documentation for the new helper
• Consistent code style with existing patterns
• Array.isArray() check prevents runtime errors if tags is malformed

3. Test Coverage

• Three new test cases comprehensively cover the new functionality:
  • Tag-only skip filtering
  • Both tag and parameters present (edge case)
  • Integration with existing include patterns
• Tests verify stories are correctly excluded from results
• Existing tests ensure no regressions

4. User Experience

• Debug logging helps users understand why stories are filtered (via VIZZLY_LOG_LEVEL=debug)
• Updated documentation clearly explains the new recommended approach
• Migration path is smooth: old code continues working

5. Documentation

• README updated to recommend tags as primary method
• Example story updated to demonstrate best practice
• Backward compatibility explicitly documented

---

🔍 Observations & Suggestions

1. Filter Order (Minor)
The current implementation checks tags first, then parameters. This is fine, but if both are present, only the tag reason is logged. The test at crawler.test.js:181-195 verifies this works correctly. The current behavior (tag wins) makes sense since it's the recommended approach.

Recommendation: This is working as intended. The early return pattern is clear and efficient.

2. Debug Logging to stderr (Question)
Debug logs use console.error() rather than console.log(). While stderr is often used for logging/diagnostics, this might be intentional to avoid polluting stdout.

Recommendation: If this matches the project's logging conventions elsewhere, keep it.

3. Test Assertion Style (Nitpick)
Tests use assert.equal(filtered.find(...), undefined) to verify stories are excluded. This works and is consistent with existing tests.

4. Edge Case: Empty Tags Array
The hasVizzlySkipTag() function correctly handles missing tags, non-array tags, and empty arrays. Good defensive programming!

5. Documentation Clarity
The README update is clear, but you might consider adding a note about why tags are now preferred (Storybook 7+ strips custom parameters).

Recommendation: Optional enhancement for future clarity.

---

🚀 Performance & Security

Performance: ✅ No concerns

• Filtering happens during discovery (before browser work), so this is optimal
• Tag check is O(n) on tag array length, which is typically tiny (1-5 tags)
• No async operations or I/O added

Security: ✅ No concerns

• Pure data filtering with no external inputs
• No injection vectors or unsafe operations
• Array.isArray() prevents type confusion attacks

---

🧪 Test Coverage

Test coverage for the new functionality is excellent:

• hasVizzlySkipTag() implicitly tested via filterStories() tests
• Tag-only filtering (line 171-179)
• Tag + parameters combination (line 181-195)
• Integration with include/exclude patterns

---

✅ Final Verdict

This PR is ready to merge. It's a well-executed fix that:

• ✅ Solves the reported issue completely
• ✅ Maintains backward compatibility
• ✅ Has comprehensive test coverage
• ✅ Updates documentation appropriately
• ✅ Follows project conventions
• ✅ No security or performance concerns
• ✅ No breaking changes

The observations above are minor suggestions for potential future enhancements, not blockers.

---

📋 Checklist

• ✅ Code follows CLAUDE.md conventions (gitmoji prefix in title, descriptive body)
• ✅ All changes in diff are covered by PR description
• ✅ Tests added for new functionality
• ✅ Documentation updated
• ✅ Backward compatibility maintained
• ✅ No security concerns
• ✅ No performance regressions

Great work! 🎉

[vizzly-testing]

Vizzly - Visual Test Results

CLI Reporter - Processing...

Build in progress...

CLI TUI - Approved

5 comparisons, no changes detected.

View build

---

_{fix/storybook-vizzly-skip-tag-215 · f45403ec}