[#502] Profile page — writer stats and story portfolio#507
[#502] Profile page — writer stats and story portfolio#507realproject7 merged 4 commits intomainfrom
Conversation
- Writer stats section: total storylines, total plots, total donations received, and claimable royalties (own profile only via wallet) - Per-story metrics: title, plot count, token price, holder count, view count, genre, active/complete status, creation date - AI agent extras: average plots per story, genre distribution tags, model info from ERC-8004 agentURI - Enhanced empty state for addresses with no stories - Holder count derived from trade_history (net positive mint/burn) Fixes #502 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The story portfolio work is close, but the writer stats section still misses a required aggregate holder metric, and the per-story holder count logic is incorrect for multi-token trades.
Findings
-
[high] The writer stats block does not show the required total token holders aggregate. On other people's profiles it renders
Holders: —, and on your own profile it replaces the holder stat entirely withClaimable. Issue #502 requires both total token holders and claimable royalties when viewing your own profile.- File:
src/app/profile/[address]/page.tsx:323 - Suggestion: Compute and render aggregate holder count across the writer's stories, and keep claimable royalties as an additional stat instead of replacing holders.
- File:
-
[high] Per-story holder count is computed from event count, not token amount, so it produces false zero/non-zero balances whenever a user mints or burns multiple tokens in one trade. The trade indexer stores only
event_type, not the minted/burned amount intrade_history, somint => +1/burn => -1is not a valid holder balance reconstruction.- File:
src/app/profile/[address]/page.tsx:403 - File:
src/app/api/cron/trade-history/route.ts:172 - Suggestion: Derive holder counts from actual token balances or persist trade amounts needed for balance reconstruction; do not ship holder counts based on raw event tallies.
- File:
Decision
Requesting changes because the current implementation still misses a required top-level stat and can show materially incorrect holder counts in the story portfolio.
- Remove unused formatSupply import - Add aggregate holder count query across all writer's storylines (from trade_history net positions) - Always show Holders stat; add Claimable as 5th stat for own profile 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 top-level holder stat is now present and claimable royalties are shown as an additional stat, but the core holder-counting bug is still unresolved. Both the aggregate holder count and the per-story holder count are still derived from raw mint/burn event tallies rather than token amounts.
Findings
- [high] Holder counts are still materially incorrect for multi-token trades. The new aggregate
totalHoldersquery and the per-storyholderCountquery both domint => +1/burn => -1, but the trade indexer only persistsevent_typetotrade_historyeven though the source events includeamountMinted/amountBurned. A user who mints 10 tokens and later burns 1 token will be treated the same as someone who minted 1 and burned 1.- File:
src/app/profile/[address]/page.tsx:252 - File:
src/app/profile/[address]/page.tsx:405 - File:
src/app/api/cron/trade-history/route.ts:172 - Suggestion: Persist the traded token amount needed for balance reconstruction, or derive holder counts from actual token balances instead of event counts.
- File:
Decision
Requesting changes because the PR still displays holder counts that can be wrong in normal multi-token trading flows.
…istory Event-based holder counting (+1 mint / -1 burn) is unreliable because trade_history doesn't persist per-event token amounts. Replace with total_supply from the latest trade_history entry per storyline, which is accurate and already indexed. - Writer stats: "Token Supply" replaces "Holders" - Per-story row: shows supply from latest trade entry - Both aggregate and per-story queries now read total_supply field 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 invalid holder-count reconstruction is gone, but the PR now substitutes token supply where issue #502 explicitly requires token holders. That means both the top-level writer stat and the per-story metric still do not match the ticket.
Findings
- [high] The PR replaced the required holder metrics with token supply. Issue #502 calls for
total token holders across all storiesand per-storyholder count, but the current implementation now rendersToken Supplyfrom the latesttrade_history.total_supplyinstead. Token supply is a different metric and does not satisfy the acceptance criteria.- File:
src/app/profile/[address]/page.tsx:251 - File:
src/app/profile/[address]/page.tsx:327 - File:
src/app/profile/[address]/page.tsx:407 - Suggestion: Restore holder-focused metrics by deriving actual holder counts from a reliable source, rather than swapping the UI to token supply.
- File:
Decision
Requesting changes because the current implementation still does not deliver the holder metrics required by issue #502.
Replace trade_history total_supply (wrong metric) with actual holder counts derived from on-chain balanceOf multicalls: - Get unique trader addresses from trade_history - Multicall balanceOf for each (user, token) pair via browserClient - Count addresses with balance > 0 Applied to both aggregate writer stat and per-story holder count. 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: APPROVE
Summary
The holder metrics now match issue #502 again. The writer stats section includes the required aggregate numbers, and both the aggregate holder stat and per-story holder count are derived from on-chain balanceOf checks rather than placeholder or event-count approximations.
Findings
- None.
Decision
Approve because the stories tab now provides the required writer stats, per-story metrics, AI agent extras, and empty state in line with issue #502.
Summary
Test plan
Fixes #502
🤖 Generated with Claude Code