docs(adr): 0014 source-pivot, feed-card refresh, post-avatar overlay

[CasparChou]

Summary

• ADR 0014 records six UX decisions, split into two phases:
  • Phase 1 (items 1-5): settings alignment, feed-card → article-card style + clickable title, RSSHub namespace autocomplete, /source/{name} pivot from articles to beats-by-source, avatar-stack collapsed to first + N
  • Phase 2 (item 6): new feeds.image_url auto-extracted from RSS channel <image>, beat-card uses post avatar as main with brand icon overlay
• Implementation blueprint splits Phase 1 / Phase 2 into two PRs, with file/line guidance and a Phase 2 validation gate (verify channel <image> semantics on Twitter/Mastodon/Bluesky before writing code)

[claude]

Claude finished @CasparChou's task —— View job

---

Reviewing PR #43: ADR 0014 Documentation

Todo List:

• Read and analyze ADR 0014 document
• Read and analyze implementation blueprint
• Review documentation quality and structure
• Check for potential issues or concerns
• Provide comprehensive feedback

Review Summary

This documentation PR presents a well-structured ADR and implementation blueprint for 6 UX improvements. The documentation quality is excellent, but I identified several areas for improvement.

Strengths

ADR Structure & Content

• Excellent organization: Clear separation of context, decisions, scope boundaries, and migration strategy
• Comprehensive rationale: Each UX decision includes solid reasoning (visual consistency, user workflow improvements, performance)
• Smart phasing: Phase 1/2 split appropriately balances risk (no schema changes vs. validation-gated features)
• Clear scope boundaries: Out-of-scope section prevents feature creep

Implementation Blueprint

• Detailed technical guidance: File paths, line numbers, specific code snippets
• Test coverage: Each major component includes test requirements
• Risk mitigation: Phase 2 validation gate prevents blind implementation

Areas for Improvement

1. Security Considerations ⚠️

The ADR lacks security analysis for:

• Avatar URL validation: External image URLs could be malicious (XSS via SVG, tracking pixels)
• CORS/CSP implications: New external image sources may conflict with content security policies
• Input sanitization: RSSHub namespace search accepts user input without documented validation

Recommendation: Add a security section covering URL validation, CSP updates, and input sanitization requirements.

2. Performance Impact Analysis

Missing performance considerations:

• Additional HTTP requests: Beat cards will now load 2 images (post + brand overlay) vs 1
• Database query complexity: WHERE EXISTS subquery in ListBeats could impact performance with large datasets
• Network latency: Direct linking to external avatar URLs (no CDN/caching strategy)

Recommendation: Add performance impact section with load testing requirements and optimization strategies.

3. Error Handling & Resilience

Insufficient error handling documentation:

• Image loading failures: What happens when auto-extracted image_url returns 404/timeout?
• Parser failures: How does the system handle malformed RSS <image> elements?
• Graceful degradation: Missing fallback strategies for various failure modes

Recommendation: Expand error handling specifications, especially for external image dependencies.

4. Migration Strategy Gaps

The migration approach has some concerns:

• No rollback testing: While rollback is mentioned as "revert the PR", no validation strategy for rollback safety
• Production deployment risk: Phase 2 changes parsing logic without staged rollout options
• Data consistency: No strategy for handling feeds that change their channel image URLs

Recommendation: Add rollback validation procedures and consider feature flags for Phase 2.

5. Technical Implementation Issues

Repository Pattern Breaking Change

pkg/repository/beat.go:625 - The signature change from positional parameters to ListBeatsOptions struct is a breaking change affecting all callers. The blueprint acknowledges this but doesn't provide migration strategy for existing code.

CSS Specificity Concerns

server/static/css/style.css - The proposed .feed-card inheriting .article-card styles could create specificity conflicts, especially in mobile responsive rules.

JavaScript Error Handling

The RSSHub autocomplete JavaScript lacks error handling for network failures and has potential race conditions with the 250ms debounce timer.

Minor Issues

1. Documentation Language Mixing: Blueprint uses Chinese (實作說明, 已停用) while ADR is in English - consider consistency
2. Version References: No mention of Go version compatibility beyond "1.24+" in CLAUDE.md
3. Icon Sources: Missing specification for where to source the action icons (pencil, power, refresh, trash)

Recommendations

High Priority

1. Add security analysis section to the ADR
2. Specify error handling requirements for image loading and parsing
3. Include performance impact assessment
4. Document breaking change migration strategy for ListBeats

Medium Priority

1. Add rollback validation procedures
2. Consider feature flags for Phase 2 deployment
3. Specify CSS organization to avoid specificity conflicts
4. Add JavaScript error handling requirements

Low Priority

1. Standardize documentation language
2. Specify icon asset sources
3. Add browser compatibility requirements

Conclusion

The ADR demonstrates thorough UX analysis and the implementation blueprint provides excellent technical guidance. The phased approach is well-reasoned. However, the documentation would benefit from security, performance, and error handling analysis before implementation begins.

The technical decisions are sound, but the breaking changes and external dependencies (avatar URLs) introduce risks that should be explicitly addressed in the documentation.

---