[#33] Add writer type filter and agent badge#124
Conversation
Fixes #33 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The new/filter-completed paths are correct, and CI passed, but the trending/rising writer filter is implemented after ranking the global candidate set rather than in the underlying Supabase candidate query. That means writer=agent or writer=human can miss valid results and return an under-filled or incorrect top list.
Findings
- [medium] Trending/rising writer filtering happens too late, so filtered rankings are not actually correct
- File:
src/app/discover/page.tsx:113 - Suggestion: Thread the writer filter into the ranking candidate fetch in
lib/ranking.tsso the Supabase storyline query itself is constrained bywriter_typebefore scoring/sorting. Filtering aftergetTrendingStorylines(..., 50)/getRisingStorylines(..., 50)only considers the global top 50 candidates, not the top writer-filtered set.
- File:
Decision
Request changes because issue #33 explicitly calls for filtering by writer_type, and the current trending/rising implementation can drop valid stories for the selected writer type.
Move writer_type filtering from post-filter (after ranking) into the Supabase candidate query so trending/rising tabs rank the full pool of matching stories instead of filtering a pre-ranked global top-50. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
T2b Review: REQUEST CHANGES
Agree with T2a — the post-filter on trending/rising is the blocking issue.
Blocking:
The filterByWriter(all, writer).slice(0, 20) pattern fetches 50 pre-ranked results then filters. If only 3 of 50 match the writer type, users see 3 results instead of 20, and valid stories ranked 51+ are invisible. Thread writerType into fetchCandidatesAndRatings in lib/ranking.ts so .eq("writer_type", ...) is applied before .limit(50).
Non-blocking:
filterByWriterJSDoc says "client-side" but it runs in a Server Component — minor but misleading.
Everything else looks good:
- AgentBadge: clean, properly typed, reusable
- Query param handling: whitelist-validated, no injection risk
- WriterFilter + TabNav integration: correct URL construction, filter persists across tab switches
- Import paths all correct
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up fix resolves the correctness issue in the writer filter implementation by applying writer_type in the ranking candidate query before scoring and sorting. The discover-page filter, badge extraction, and query-param persistence now align with issue #33, and CI passed.
Findings
- None.
Decision
Approve because the implementation now satisfies the acceptance criteria end-to-end, stays focused, and the lint-and-typecheck check passed.
project7-interns
left a comment
There was a problem hiding this comment.
T2b Re-review: APPROVE
Fix is correct — writerType now threaded into fetchCandidatesAndRatings with .eq("writer_type", ...) applied before .limit(50). Post-filter removed entirely. Same pattern applied to new/completed tabs in queryTab. All 4 tabs now filter at the DB level. Ship it.
Summary
?writer=all|human|agentquery paramAgentBadgecomponentwriter_type(0 = human, 1 = agent) on new/completed tabs; post-filter trending/rising resultsTabNavnow preserves extra query params (writer filter persists across tab switches)Fixes #33
Test plan
/discover— writer filter shows below tabs with "All" active by default?tab=new&writer=human, onlywriter_type=0stories shown?tab=new&writer=agent, onlywriter_type=1stories shown🤖 Generated with Claude Code