Fix payout double-count flash after voting#3330
Conversation
The SDK's post-broadcast cache write replaces the voter's active_votes
record with a bare { rshares, voter }, dropping the optimistic amount
marker. The recent-vote override then re-added the full vote value on
top of a payout that already included it, briefly showing
base + 2x vote value until the deferred refetch reconciled.
Skip re-applying the recent vote when the entry already carries the
voter's record without an amount marker: that data already includes
the vote (server-indexed or SDK-written) and its old contribution is
unknown, so re-adding is always an overcount.
Also pass estimated: 0 on downvotes: the SDK adds estimated unsigned,
so a positive value briefly showed the payout increased by the
downvote's value now that the override no longer re-touches it.
Greptile SummaryThis PR fixes the payout flash after voting. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (4): Last reviewed commit: "Merge branch 'development' into fix/vote..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58e313d1c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (recentVote.status !== 'DELETED' && Array.isArray(entry.active_votes)) { | ||
| const existingVote = entry.active_votes.find((v: any) => v?.voter === recentVote.voter); | ||
| if (existingVote && typeof existingVote.amount !== 'number') { | ||
| return entry; |
There was a problem hiding this comment.
When the recent vote is a downvote, this guard also skips the override as soon as the SDK success write replaces the vote with a bare { voter, rshares } record. In that same path the mutation now passes estimated: 0, and the SDK computes the entry payout from the unchanged base payout plus estimated, so the post jumps back to the pre-downvote payout until the deferred refetch arrives instead of continuing to show the optimistic reduction. Please avoid applying this bare-record skip to downvotes, or otherwise reapply the negative payout delta for that SDK-written shape.
Useful? React with 👍 / 👎.
Sending 0 kept the SDK's post-broadcast write at the base payout, briefly reverting the at-press reduction until the deferred refetch. The signed delta, clamped to the pre-vote payout, makes that write land on the same reduced value the optimistic layer already shows.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpdates downvote mutation payout estimation, adds a guard in recent vote cache override handling to avoid double-counting, and expands unit tests for the updated cache behavior. ChangesDownvote payout double-count fix
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant UpvotePopover
participant voteMutation
participant voteCacheUtils
participant QueryCache
UpvotePopover->>voteMutation: mutateAsync(negative estimated)
voteMutation->>QueryCache: optimistic payout update
voteMutation-->>voteCacheUtils: recentVote cache write
voteCacheUtils->>voteCacheUtils: inspect active_votes for voter without amount
alt voter already present without amount
voteCacheUtils-->>QueryCache: return entry unchanged
else
voteCacheUtils->>voteCacheUtils: applyVoteToPost(entry, recentVote)
voteCacheUtils-->>QueryCache: updated entry
end
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
What
Voting on the post page briefly showed the payout increased by twice the vote value (e.g. payout 1 + vote 2 showed 3, then flashed 5, then settled at 3).
Why
Two optimistic layers stack on the same cached entry:
updateVoteInQueryCachesbumps the payout and stores the voter's record with anamountfield, whichapplyRecentVoteOverrideToEntryuses to stay idempotent.useVoteonSuccess rewrites the same["posts", "entry", ...]cache entry: payout is re-derived from the untouched numericpayoutfield (correct), but the voter's record is replaced with a bare{ rshares, voter }— droppingamount.How
applyRecentVoteOverrideToEntrynow skips re-applying when the entry already carries the voter's record without a numericamountmarker: that data already includes the vote (server-indexed or SDK-written) and re-adding is always an overcount. Unvotes (DELETED) are unaffected.estimatedunsigned, so the positive value briefly showed the payout increased by the downvote's value once the override stopped compensating for it. The clamped negative delta makes the SDK's write land on the same reduced value the at-press update shows.Complementary SDK-side fix (skips the stacked write entirely): ecency/vision-next#1096. This change fixes the visible flash on its own with the currently pinned SDK; the SDK bump can follow the usual release order.
Tests
7 new jest cases in
voteCacheUtils.test.tscovering the skip (up/down), lagging-refetch re-apply, amount-marker idempotency,amount: 0semantics, TTL expiry, and the unvote path. Verified the guard test fails against the pre-fix code with the exact reported values (5.000 vs 3.000).Summary by CodeRabbit