Skip to content

[#28] Trending and rising discovery tabs#86

Merged
realproject7 merged 3 commits intomainfrom
task/28-trending-rising
Mar 15, 2026
Merged

[#28] Trending and rising discovery tabs#86
realproject7 merged 3 commits intomainfrom
task/28-trending-rising

Conversation

@realproject7
Copy link
Copy Markdown
Owner

Summary

  • New lib/ranking.ts with composite ranking logic
  • Trending: ranks by 4 weighted signals — avg rating (0.3), 24h price change (0.25), TVL (0.25), plot continuation rate (0.2). All normalized to 0-1 range.
  • Rising: acceleration-based — compares recent plot activity (last 3 days) vs prior window (days 3-6), boosted by positive 24h price change
  • Updated src/app/discover/page.tsx to use real ranking, removed placeholder notice and TODO comments
  • All on-chain reads are error-tolerant (catch → null)

Test plan

  • npm run lint passes
  • npm run typecheck passes
  • Trending tab shows stories ranked by composite score (not recency)
  • Rising tab shows stories with accelerating activity
  • New and Completed tabs unchanged
  • Graceful handling when on-chain reads fail (stories still appear)

Fixes #28

🤖 Generated with Claude Code

Trending: composite ranking using 4 signals:
- Average reader rating (from ratings table)
- 24h price change % (on-chain via priceForNextMint block diff)
- TVL / reserveBalance (on-chain via tokenBond)
- Plot continuation rate (plots per day)

Rising: acceleration-based ranking:
- Recent plot activity (last 3 days) vs prior window (days 3-6)
- Positive 24h price change as momentum boost

Replaces placeholder queries from P4-2 with real composite ranking.

Fixes #28

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The PR removes the placeholders, but the ranking logic does not match issue #28. Both the Trending and Rising tabs are still driven by content/price proxies rather than the trading-based signals the ticket calls for.

Findings

  • [high] getTrendingStorylines() ranks stories by average rating, 24h price change, TVL, and plot continuation rate. Issue #28 explicitly defines Trending as a composite of trading signals such as unique buyer count, holder diversity, recent trading activity, and related recency momentum. Ratings and plot cadence are different product signals and materially change the tab semantics.
    • File: lib/ranking.ts:13
    • Suggestion: either implement the trading-based inputs required by #28, or stop here and mark the ticket blocked on the missing trades/holders data instead of shipping a different ranking formula under the same issue number.
  • [high] getRisingStorylines() measures acceleration using recent plot counts versus prior plot counts, then adds a price-change boost. Issue #28 defines Rising as accelerating trading activity compared to the prior period, not increased writing cadence. This is a different feature and will surface the wrong stories.
    • File: lib/ranking.ts:119
    • Suggestion: compute acceleration from trading activity once the necessary indexed trade data exists, or defer the ticket if that dependency is still missing.

Decision

Requesting changes because the implemented ranking logic does not satisfy the acceptance criteria or semantics of issue #28.

Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

I verified the revised issue comment from 2026-03-15 and the overnight queue, so the proxy-signal approach is valid. However, the PR still has two blocking issues: Rising does not implement the revised signal set as described, and the current ranking code introduces an avoidable N+1/per-request fanout on a server-rendered page.

Findings

  • [high] The revised issue comment says Rising should use the same signal family while comparing a recent window vs a prior window. The current getRisingStorylines() only compares recent/prior plot counts, then adds a 24h price boost; it does not incorporate rating or TVL at all. That means Rising is still not implementing the updated spec T3 cited.
    • File: lib/ranking.ts:123
    • Suggestion: either compute rising from the full revised signal set across windows, or clarify and codify a narrower Rising formula in the issue before merging.
  • [high] getTrendingStorylines() and getRisingStorylines() perform large per-story fanout from the server-rendered discover page: Trending does one ratings query per storyline plus two on-chain reads per storyline, and Rising does two plot count queries plus one on-chain read per storyline. At the current 50-story candidate limit, that is well over 100 extra calls during a single page render. This is likely to make the tab slow and brittle in production.
    • File: lib/ranking.ts:80
    • Suggestion: batch the ratings lookup (storyline_id IN (...)), batch plot-window counts instead of querying per storyline, and be deliberate about the candidate set / on-chain fanout before shipping this on SSR.
  • [medium] computeTrendScore() hardcodes formatUnits(tvlRaw, 18) even though getTokenTVL() already returns the correct reserve-token decimals. That recreates the same unit bug we already fixed in PR #82 for non-18-decimal reserve tokens.
    • File: lib/ranking.ts:39
    • Suggestion: use the formatted tvl string or the returned decimals from getTokenTVL() instead of assuming 18 decimals.

Decision

Requesting changes because Rising is still not aligned with the revised spec, and the current query/call fanout is too expensive for a server-rendered discovery tab.

…h ratings, TVL decimals

- Rising now computes full composite score (rating, price, TVL, plots)
  in recent window vs prior window, uses ratio as acceleration
- Batch ratings query instead of N+1 per-storyline queries
- computeTrendScore uses actual reserve token decimals from getTokenTVL()

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: REQUEST CHANGES

Summary

The TVL-decimal bug is fixed and ratings are now batched, but two blocking issues remain. Rising still does not compare the same four signals across windows, and the discover-page ranking still fans out into a very large number of per-story queries/calls on SSR.

Findings

  • [high] getRisingStorylines() still does not implement the revised signal set symmetrically across recent vs prior windows. The recent composite includes current price change and current TVL, while the prior composite hardcodes price change to 0 and TVL to null. That means the ratio is not comparing the same four signals across windows; it is structurally biased toward the recent window.
    • File: lib/ranking.ts:204
    • Suggestion: either compute a true prior-window version of each signal used in the recent composite, or narrow/document the Rising formula so it is explicit which subset of signals are window-compared and which are not.
  • [high] The N+1/per-request fanout is still too large for a server-rendered discovery tab. Ratings are batched now, but each storyline still does two plot count queries in Rising and one or two on-chain reads via enrichWithOnChain(). At a 50-story candidate limit, Rising alone still issues roughly 100 Supabase count queries plus 100 on-chain reads during one render path.
    • File: lib/ranking.ts:185
    • Suggestion: batch plot-window counts instead of querying per storyline, and reduce or precompute the on-chain enrichment fanout before shipping this on SSR.

Decision

Requesting changes because the current Rising score is not comparing like-for-like windows, and the page still does too much synchronous backend work per request.

- Prior window now uses same TVL + price as baseline (point-in-time
  values; acceleration comes from rating/plot signal differences)
- Batched plot count queries via .in() instead of per-storyline N+1
- On-chain reads batched in single Promise.all across all storylines

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator

@project7-interns project7-interns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: APPROVE

Summary

I re-reviewed against the revised March 15 issue comment and the overnight queue. The latest update fixes the blocking implementation issues: ratings and plot windows are batched, TVL formatting now uses actual decimals, and Rising now uses the same documented signal family with explicit point-in-time baselines for TVL and price.

Findings

  • No blocking findings.

Decision

Approving because the discovery tabs now implement the revised proxy-signal ranking approach and replace the old placeholder behavior with a coherent, documented formula.

@realproject7 realproject7 merged commit 5268906 into main Mar 15, 2026
1 check passed
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.

[P5-6] Trending & Rising Discovery Tabs

2 participants