Skip to content

fix(votes): persist upvote counts reliably across merge and hook dispatch - #565

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
m0Nst3r873:fix/upvote-vote-persistence
Sep 16, 2026
Merged

jeff-r2026 merged 1 commit into
Tencent:mainfrom
m0Nst3r873:fix/upvote-vote-persistence

Conversation

@m0Nst3r873

Copy link
Copy Markdown
Collaborator

Summary

Team-wide upvote counts were stuck at 0 while recall counts accumulated normally. Investigation across 20 members' vote data (3433 recalls, 19/20 members with upvoted_count=0) traced this to two independent defects in the vote persistence path. This PR fixes both.

1. mergeDeltas timestamp/count inconsistency (src/votes.ts)

mergeDeltas merged upvoted_count as remote + delta but last_upvoted_at as max(local, remote). When upvoted_delta === 0 (recall without upvote) and remote count was 0, the result was an orphan state: upvoted_count: 0 with a last_upvoted_at set. Confirmed against real team data (e.g. 11 such entries in one member's file).

  • After merging, clear last_upvoted_at when upvoted_count === 0 and reset last_recalled_at when recalled_count === 0.
  • recallFeedback now clears last_upvoted_at when a downvote brings the count to 0, closing the source of dirty data.

2. Hook dispatch short-circuit on malformed STDIN (src/hook-dispatch-cli.ts)

parseStdin returned null on unparseable STDIN, which made hookDispatchCli return early and skip every handler — including votes-sync. Under concurrent hook invocations some processes received truncated/corrupt STDIN, so vote sync silently never ran.

  • On parse failure, degrade to {} and continue dispatch; handlers that need STDIN fields (e.g. votes-sync requires transcript_path) self-skip, while STDIN-independent handlers still run.
  • Normalize valid-but-non-object JSON (null / number / array) so it also degrades cleanly instead of throwing.
  • Log a diagnostic preview (length + head/tail) of the unparseable payload.

Scope note

These fixes make accumulated vote data consistent and stop the dispatch from being silently dropped, but they do not by themselves start upvote accumulation on installs that predate the collection-path fix (4567e02, first in v0.22.0-beta.3). They also cannot recover the upvote from the specific invocation whose STDIN was corrupted. Teams on older CLIs still need to upgrade to >= v0.22.0-beta.3.

Test Plan

  • npx vitest run — 3081 passed, no regressions
  • npx tsc --noEmit — clean
  • New unit tests: mergeDeltas (upvote→0, recall→0, positive-count preservation), recallFeedback negative→0, parseStdin (malformed / empty / valid / non-object JSON)
  • Real-data verification: fed a real orphan entry from team vote data through the fixed mergeDeltas — dirty timestamp cleared, normal upvote path unaffected
  • Real dispatch verification: piped malformed JSON to hook-dispatch stop — exits 0, no crash, diagnostic logged, downstream handlers reached

🤖 Generated with Claude Code

…atch

Two independent defects caused team-wide upvote counts to stay at 0
while recall counts accumulated normally:

- mergeDeltas merged upvoted_count via remote+delta but last_upvoted_at
  via max(local,remote), producing orphan timestamps (count 0 with a
  last_upvoted_at set). Now clears the timestamp when the merged count
  is 0, and recallFeedback clears last_upvoted_at when a downvote brings
  the count to 0. Prevents the inconsistent state from propagating.

- parseStdin returned null on malformed STDIN, short-circuiting the
  entire hook dispatch so votes-sync (and other handlers) never ran when
  a concurrent hook delivered corrupt STDIN. Now degrades to {} and
  continues; handlers that need STDIN fields self-skip. Also normalizes
  valid-but-non-object JSON (null/number/array) and logs a diagnostic
  preview of the unparseable payload.

Adds unit tests for both paths.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jeff-r2026
jeff-r2026 self-requested a review September 15, 2026 08:49
@jeff-r2026
jeff-r2026 merged commit 46b7921 into Tencent:main Sep 16, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants