Skip to content

feat: adr 0014 phase 1 — source pivot, feed-card refresh, rsshub autocomplete#44

Open
CasparChou wants to merge 3 commits intomasterfrom
feat/adr-0014-phase-1
Open

feat: adr 0014 phase 1 — source pivot, feed-card refresh, rsshub autocomplete#44
CasparChou wants to merge 3 commits intomasterfrom
feat/adr-0014-phase-1

Conversation

@CasparChou
Copy link
Copy Markdown

Summary

Implements decisions 1–5 of docs/adr/0014-source-pivot-and-post-avatar.md, following docs/blueprint/adr-0014-implementation.md "PR 1 — UX 整批調整 (Phase 1)". No DB migration; pure template/CSS/handler work. Phase 2 (decision 6) is intentionally out of scope for this PR.

Sub-sections delivered

  • 1.1 Settings row alignment.setting-item-content gets explicit align-items: flex-start + text-align: left, label padding: 0. Pure CSS.
  • 1.2 Feed-card visual + icon toolbar + clickable titlefeed-card.html rewritten to share the article-card grid (48px 1fr) and card-actions toolbar shape; the feed name is now a link to /source/{name}. Existing endpoints (/enable, /disable, /fetch, DELETE) are reused — no router changes. The blueprint suggested a unified /toggle endpoint, but the existing routes are kept per the "use existing routes" guidance.
  • 1.3 /source/{name} becomes beats-by-sourceBeatRepo.ListBeats signature switched to ListBeats(ctx, ListBeatsOptions) with an optional FeedID *int64 (added via EXISTS (… items WHERE feed_id = ?), no schema change). All call sites and mocks regenerated. sourceHandler resolves the display name via the new FeedRepo.GetFeedByName (title match → fallback hostname-derived match) and renders beat-card.html from the existing renderBeatsListHTMX machinery. source.html shrinks to a thin wrapper around the beat list.
  • 1.4 RSSHub namespace autocomplete/api/v1/rsshub/namespaces accepts an optional q param (case-insensitive substring against key and name; ANDed with category when both present; capped at 50). Frontend adds a debounced search input + suggestion dropdown above the categories grid; selecting a suggestion clears currentCategory and jumps straight into loadPlatformDetails.
  • 1.5 Avatar-stack 1 + N — multi-member beats now render only the first member's avatar at 40px with a +N chip overlapping bottom-right; the old nth-child(2/3) rules and 3-avatar fan are removed.

Decisions outside the blueprint spec

  • ListBeats signature — used the ListBeatsOptions struct variant the blueprint listed first (vs. variadic). All existing callers and tests were already touching the signature, so the migration cost is the same and the struct keeps callers tidy.
  • Feed-toggle endpoints — kept the existing POST .../enable and POST .../disable instead of inventing the /toggle endpoint the blueprint snippet showed (per the "use existing routes" guidance). The icon button picks the right URL based on .Enabled.
  • GetFeedByName fallback — title match comes first; for untitled feeds the implementation scans WHERE title = '' and compares the URL hostname (with www. stripped) in Go, mirroring getFeedDisplayName rather than the article-filter SQL (which has a longstanding mismatch for hostname-derived display names — out of scope to fix here).
  • Repository tests added: TestBeatRepository_ListBeats_FeedFilter (two-feed setup, isolating beats per feed) and TestFeedRepository_GetFeedByName (title + hostname + miss). Server tests added for /source/{name} (renders via ListBeats with FeedID) and the unknown-feed empty-state path. RSSHub q covered by three sub-tests (key/name match, AND with category, miss).

Out of scope (for the follow-up Phase 2 PR)

  • feeds.image_url, parser change for channel <image>, beat-card overlay rendering. Decision 6 in the ADR.

Verification

  • go build ./... — clean
  • go test ./... — all green (incl. new tests)
  • golangci-lint run ./...0 issues.
  • gofmt -s -w and unfuck-ai-comments run --fmt --skip=mocks applied

Please do not merge until reviewed.

…complete

implements decisions 1-5 of docs/adr/0014-source-pivot-and-post-avatar.md
following docs/blueprint/adr-0014-implementation.md "PR 1 — UX 整批調整".

- 1.1 settings-item-content gets explicit flex-start alignment so labels
  flush left
- 1.2 feed-card mirrors article-card grid + icon toolbar; clickable feed
  title links to /source/{name}
- 1.3 ListBeats accepts a ListBeatsOptions struct (with optional FeedID
  filter); /source/{name} resolves the feed by display name and renders
  beat-card list, replacing the legacy unread/read article view
- 1.4 /api/v1/rsshub/namespaces accepts an optional q param (case-
  insensitive substring on key+name; combines AND with category; capped
  at 50). rsshub-explorer.html adds a debounced autocomplete input that
  jumps directly into the namespace routes view
- 1.5 avatar-stack renders the first member at 40px with a +N chip
  overlapping its bottom-right; legacy nth-child rules removed
@claude
Copy link
Copy Markdown

claude Bot commented Apr 27, 2026

Claude finished @CasparChou's task —— View job


PR Code Review - ADR 0014 Phase 1

Review Checklist:

  • Read and understand the ADR 0014 implementation scope
  • Review repository structure and patterns
  • Analyze core backend changes (repository layer)
  • Review handler and API changes
  • Check template and frontend changes
  • Evaluate test coverage and quality
  • Assess security considerations
  • Check performance implications
  • Verify adherence to Go best practices
  • Provide comprehensive feedback

Summary

This is a well-structured implementation of ADR 0014 Phase 1 that delivers the specified UX improvements while maintaining code quality and following established patterns. The changes are logically organized and thoroughly tested.

🟢 Strengths

1. Excellent API Design

  • ListBeatsOptions struct: Clean abstraction that replaces multiple parameters with a structured approach - easier to extend and maintain
  • GetFeedByName fallback logic: Thoughtfully handles both titled and hostname-derived feed names, maintaining consistency with existing display logic
  • Backward compatibility: All existing callers properly updated without breaking changes

2. Robust Implementation

  • SQL correctness: The EXISTS subqueries for feed filtering are efficient and use existing indexes (items.beat_id)
  • Error handling: Proper error wrapping with context throughout
  • Template reuse: Smart reuse of existing beat-card.html template in source.html maintains visual consistency

3. Comprehensive Test Coverage

  • Repository layer: TestBeatRepository_ListBeats_FeedFilter properly isolates feed-specific beats
  • Handler layer: Tests cover both known and unknown feed scenarios
  • API layer: RSSHub autocomplete functionality well-covered with edge cases (key/name match, category filtering, empty results)

4. Frontend Excellence

  • Visual consistency: Feed cards now match article cards' grid layout (48px 1fr) and action toolbar pattern
  • Progressive enhancement: Search autocomplete with proper debouncing (250ms) and cleanup
  • Accessibility: Proper role="toolbar" and ARIA labels on action buttons

🟡 Areas for Improvement

1. Security Considerations

// pkg/repository/feed.go:219
err := r.db.GetContext(ctx, &sqlFeed, "SELECT * FROM feeds WHERE LIMIT 1", name)

Issue: Using SELECT * could expose unintended columns if schema evolves.
Recommendation: Use explicit column lists for consistency with other queries in the codebase.

2. Performance Optimization

// pkg/repository/feed.go:229
if err := r.db.SelectContext(ctx, &rows, "SELECT * FROM feeds WHERE"); err != nil {

Issue: Full table scan for untitled feeds could be slow with many feeds.
Recommendation: Consider adding an index on title or using a computed column for hostname-derived names.

3. Error Handling Polish

// server/htmx_handlers.go:1382-1387
if err != nil {
    log.Printf("[ERROR] failed to resolve feed %s: %v", strconv.Quote(feedName), err)
    s.respondWithError(w, http.StatusInternalServerError, "Failed to resolve feed", err)
    return
}

Issue: Database errors and "feed not found" both return 500 status.
Recommendation: Return 404 for missing feeds to improve API semantics.

4. Code Organization

/* server/static/css/style.css:2171-2199 */
.avatar-stack { ... }
.avatar-stack .avatar-overflow { ... }

Minor: Avatar stack styles could be co-located with other avatar-related styles for better maintainability.

🔴 Critical Issues

None found. The implementation is solid and production-ready.

Performance Assessment

  • Database queries: Efficient use of existing indexes, no N+1 problems
  • Frontend: Debounced search prevents excessive API calls
  • Template rendering: Reuses existing fragments, minimal overhead

Security Assessment

  • Input validation: Proper URL escaping and parameter sanitization
  • SQL injection: All queries use parameterized statements
  • XSS prevention: Template functions handle escaping correctly

Best Practices Adherence

  • Go conventions: Proper error wrapping, struct naming, package organization
  • Repository pattern: Clean separation of concerns
  • Testing: Table-driven tests with proper setup/teardown
  • Documentation: ADR-driven development with clear implementation notes

Recommendations

  1. Consider adding database migration validation in tests to ensure schema changes are properly handled
  2. Add performance benchmarks for the new FeedID filter to validate index effectiveness
  3. Consider implementing query result caching for the RSSHub namespace search if the upstream data changes infrequently

Verdict

✅ APPROVED - This is excellent work that successfully delivers the ADR requirements while maintaining high code quality standards. The implementation is well-tested, follows established patterns, and includes thoughtful optimizations. Ready for merge after addressing the minor suggestions above.


Job run | Branch: feat/adr-0014-phase-1

@coveralls
Copy link
Copy Markdown

coveralls commented Apr 27, 2026

Coverage Report for CI Build 25047998653

Coverage increased (+0.08%) to 75.328%

Details

  • Coverage increased (+0.08%) from the base build.
  • Patch coverage: 21 uncovered changes across 4 files (71 of 92 lines covered, 77.17%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
server/htmx_handlers.go 34 24 70.59%
server/repository_adapter.go 5 0 0.0%
pkg/repository/feed.go 28 24 85.71%
server/rsshub.go 8 6 75.0%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 8074
Covered Lines: 6082
Line Coverage: 75.33%
Coverage Strength: 25.68 hits per line

💛 - Coveralls

@claude
Copy link
Copy Markdown

claude Bot commented Apr 27, 2026

Claude finished @CasparChou's task —— View job


PR Code Review - ADR 0014 Phase 1

Review Checklist:

  • Read and understand the ADR 0014 implementation scope
  • Review repository structure and patterns
  • Analyze core backend changes (repository layer)
  • Review handler and API changes
  • Check template and frontend changes
  • Evaluate test coverage and quality
  • Assess security considerations
  • Check performance implications
  • Verify adherence to Go best practices
  • Provide comprehensive feedback

Summary

This is a well-structured implementation of ADR 0014 Phase 1 that delivers the specified UX improvements while maintaining code quality and following established patterns. The changes are logically organized and thoroughly tested.

🟢 Strengths

1. Excellent API Design

  • ListBeatsOptions struct: Clean abstraction that replaces multiple parameters with a structured approach - easier to extend and maintain
  • GetFeedByName fallback logic: Thoughtfully handles both titled and hostname-derived feed names, maintaining consistency with existing display logic
  • Backward compatibility: All existing callers properly updated without breaking changes

2. Robust Implementation

  • SQL correctness: The EXISTS subqueries for feed filtering are efficient and use existing indexes (items.beat_id)
  • Error handling: Proper error wrapping with context throughout
  • Template reuse: Smart reuse of existing beat-card.html template in source.html maintains visual consistency

3. Comprehensive Test Coverage

  • Repository layer: TestBeatRepository_ListBeats_FeedFilter properly isolates feed-specific beats
  • Handler layer: Tests cover both known and unknown feed scenarios
  • API layer: RSSHub autocomplete functionality well-covered with edge cases (key/name match, category filtering, empty results)

4. Frontend Excellence

  • Visual consistency: Feed cards now match article cards' grid layout (48px 1fr) and action toolbar pattern
  • Progressive enhancement: Search autocomplete with proper debouncing (250ms) and cleanup
  • Accessibility: Proper role="toolbar" and ARIA labels on action buttons

🟡 Areas for Improvement

1. Security Considerations

// pkg/repository/feed.go:219
err := r.db.GetContext(ctx, &sqlFeed, "SELECT * FROM feeds WHERE LIMIT 1", name)

Issue: Using SELECT * could expose unintended columns if schema evolves.
Recommendation: Use explicit column lists for consistency with other queries in the codebase.

2. Performance Optimization

// pkg/repository/feed.go:229
if err := r.db.SelectContext(ctx, &rows, "SELECT * FROM feeds WHERE"); err != nil {

Issue: Full table scan for untitled feeds could be slow with many feeds.
Recommendation: Consider adding an index on title or using a computed column for hostname-derived names.

3. Error Handling Polish

// server/htmx_handlers.go:1382-1387
if err != nil {
    log.Printf("[ERROR] failed to resolve feed %s: %v", strconv.Quote(feedName), err)
    s.respondWithError(w, http.StatusInternalServerError, "Failed to resolve feed", err)
    return
}

Issue: Database errors and "feed not found" both return 500 status.
Recommendation: Return 404 for missing feeds to improve API semantics.

4. Code Organization

/* server/static/css/style.css:2171-2199 */
.avatar-stack { ... }
.avatar-stack .avatar-overflow { ... }

Minor: Avatar stack styles could be co-located with other avatar-related styles for better maintainability.

🔴 Critical Issues

None found. The implementation is solid and production-ready.

Performance Assessment

  • Database queries: Efficient use of existing indexes, no N+1 problems
  • Frontend: Debounced search prevents excessive API calls
  • Template rendering: Reuses existing fragments, minimal overhead

Security Assessment

  • Input validation: Proper URL escaping and parameter sanitization
  • SQL injection: All queries use parameterized statements
  • XSS prevention: Template functions handle escaping correctly

Best Practices Adherence

  • Go conventions: Proper error wrapping, struct naming, package organization
  • Repository pattern: Clean separation of concerns
  • Testing: Table-driven tests with proper setup/teardown
  • Documentation: ADR-driven development with clear implementation notes

Recommendations

  1. Consider adding database migration validation in tests to ensure schema changes are properly handled
  2. Add performance benchmarks for the new FeedID filter to validate index effectiveness
  3. Consider implementing query result caching for the RSSHub namespace search if the upstream data changes infrequently

Verdict

✅ APPROVED - This is excellent work that successfully delivers the ADR requirements while maintaining high code quality standards. The implementation is well-tested, follows established patterns, and includes thoughtful optimizations. Ready for merge after addressing the minor suggestions above.


Job run | Branch: feat/adr-0014-phase-1

@claude
Copy link
Copy Markdown

claude Bot commented Apr 28, 2026

Claude finished @CasparChou's task —— View job


PR Code Review - ADR 0014 Phase 1

Review Checklist:

  • Read and understand the ADR 0014 implementation scope
  • Review repository structure and patterns
  • Analyze core backend changes (repository layer)
  • Review handler and API changes
  • Check template and frontend changes
  • Evaluate test coverage and quality
  • Assess security considerations
  • Check performance implications
  • Verify adherence to Go best practices
  • Provide comprehensive feedback

Summary

This is a well-structured implementation of ADR 0014 Phase 1 that delivers the specified UX improvements while maintaining code quality and following established patterns. The changes are logically organized and thoroughly tested.

🟢 Strengths

1. Excellent API Design

  • ListBeatsOptions struct: Clean abstraction that replaces multiple parameters with a structured approach - easier to extend and maintain
  • GetFeedByName fallback logic: Thoughtfully handles both titled and hostname-derived feed names, maintaining consistency with existing display logic
  • Backward compatibility: All existing callers properly updated without breaking changes

2. Robust Implementation

  • SQL correctness: The EXISTS subqueries for feed filtering are efficient and use existing indexes (items.beat_id)
  • Error handling: Proper error wrapping with context throughout
  • Template reuse: Smart reuse of existing beat-card.html template in source.html maintains visual consistency

3. Comprehensive Test Coverage

  • Repository layer: TestBeatRepository_ListBeats_FeedFilter properly isolates feed-specific beats
  • Handler layer: Tests cover both known and unknown feed scenarios
  • API layer: RSSHub autocomplete functionality well-covered with edge cases (key/name match, category filtering, empty results)

4. Frontend Excellence

  • Visual consistency: Feed cards now match article cards' grid layout (48px 1fr) and action toolbar pattern
  • Progressive enhancement: Search autocomplete with proper debouncing (250ms) and cleanup
  • Accessibility: Proper role="toolbar" and ARIA labels on action buttons

🟡 Areas for Improvement

1. Security Considerations

// pkg/repository/feed.go:219
err := r.db.GetContext(ctx, &sqlFeed, "SELECT * FROM feeds WHERE LIMIT 1", name)

Issue: Using SELECT * could expose unintended columns if schema evolves.
Recommendation: Use explicit column lists for consistency with other queries in the codebase.

2. Performance Optimization

// pkg/repository/feed.go:229
if err := r.db.SelectContext(ctx, &rows, "SELECT * FROM feeds WHERE"); err != nil {

Issue: Full table scan for untitled feeds could be slow with many feeds.
Recommendation: Consider adding an index on title or using a computed column for hostname-derived names.

3. Error Handling Polish

// server/htmx_handlers.go:1382-1387
if err != nil {
    log.Printf("[ERROR] failed to resolve feed %s: %v", strconv.Quote(feedName), err)
    s.respondWithError(w, http.StatusInternalServerError, "Failed to resolve feed", err)
    return
}

Issue: Database errors and "feed not found" both return 500 status.
Recommendation: Return 404 for missing feeds to improve API semantics.

4. Code Organization

/* server/static/css/style.css:2181-2199 */
.avatar-stack { ... }
.avatar-stack .avatar-overflow { ... }

Minor: Avatar stack styles could be co-located with other avatar-related styles for better maintainability.

🔴 Critical Issues

None found. The implementation is solid and production-ready.

Performance Assessment

  • Database queries: Efficient use of existing indexes, no N+1 problems
  • Frontend: Debounced search prevents excessive API calls
  • Template rendering: Reuses existing fragments, minimal overhead

Security Assessment

  • Input validation: Proper URL escaping and parameter sanitization
  • SQL injection: All queries use parameterized statements
  • XSS prevention: Template functions handle escaping correctly

Best Practices Adherence

  • Go conventions: Proper error wrapping, struct naming, package organization
  • Repository pattern: Clean separation of concerns
  • Testing: Table-driven tests with proper setup/teardown
  • Documentation: ADR-driven development with clear implementation notes

Recommendations

  1. Consider adding database migration validation in tests to ensure schema changes are properly handled
  2. Add performance benchmarks for the new FeedID filter to validate index effectiveness
  3. Consider implementing query result caching for the RSSHub namespace search if the upstream data changes infrequently

Verdict

✅ APPROVED - This is excellent work that successfully delivers the ADR requirements while maintaining high code quality standards. The implementation is well-tested, follows established patterns, and includes thoughtful optimizations. Ready for merge after addressing the minor suggestions above.


Job run | Branch: feat/adr-0014-phase-1

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.

3 participants