-
Notifications
You must be signed in to change notification settings - Fork 5
feat: added eReputation weighted calculation #459
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: added eReputation weighted calculation #459
Conversation
WalkthroughAdds eReputation-weighted voting across frontend and backend: UI shows voting-weight badges in lists, details, and creation flows; create flow prevents private+ereputation; backend enforces validation and uses raw reputation scores; VoteReputationResult.group nullable with migration; webhook/mappings adjusted; client refresh uses immediate fetch with retries. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Client UI
participant API as eVoting API
participant DB as Database
rect rgba(200,230,255,0.25)
Note right of UI: Poll creation with mutual-excl.
User->>UI: Fill form (visibility, votingWeight)
UI->>UI: enforce mutual-exclusivity (private ↔ no ereputation)
UI->>API: POST /polls
API->>API: validate (no private + ereputation)
API->>DB: insert poll
DB-->>API: ok
API-->>UI: created
end
rect rgba(220,255,220,0.18)
Note right of UI: Vote signing & immediate results fetch with retries
User->>UI: Sign vote
UI->>API: submit signature
API-->>UI: signed
UI->>API: GET /polls/:id/results (immediate)
alt success
API-->>UI: results (includes ereputation totals)
else transient failure
loop up to 3 times
UI-->>API: retry fetch (1s delay)
end
API-->>UI: results or error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platforms/evoting-api/src/controllers/WebhookController.ts (1)
210-218: ApplymappingDb.getLocalId()to pollId like groupId to resolve global-to-local ID mapping.The pollId is extracted from webhook data without conversion (line 217), but groupId correctly uses
mappingDb.getLocalId()(line 227). Since both come from the same webhook system with identical format ("table(ID)"), pollId should also be mapped. Currently,getPollById()at line 280 receives a global ID and searches the local database, causing a null result. The code continues regardless and assigns the unmapped global ID to vote/result entities (lines 256, 324), violating the foreign key constraint on the polls table.// Lines 212-218: Apply mapping like groupId does let pollId: string | null = null; if (local.data.pollId && typeof local.data.pollId === "string") { const pollIdValue = local.data.pollId.includes("(") ? local.data.pollId.split("(")[1].split(")")[0] : local.data.pollId; pollId = await this.adapter.mappingDb.getLocalId(pollIdValue); // ADD MAPPING }
🧹 Nitpick comments (5)
platforms/eVoting/src/app/(app)/page.tsx (1)
166-170: Consider adding sort functionality for consistency.The "Voting Weight" header lacks the
onClickhandler present on other column headers. Consider adding sorting capability for consistency with other columns.Apply this diff to add sorting:
<th className="text-left py-3 px-4 font-medium text-gray-700 cursor-pointer hover:bg-gray-50" + onClick={() => handleSort("votingWeight")} > - Voting Weight + Voting Weight {getSortIcon("votingWeight")} </th>platforms/evoting-api/src/services/VoteService.ts (1)
45-64: HardengetReputationScoreagainst bad data and noisy PII-heavy logs
memberRep.scoreis trusted blindly; if upstream ever sends0,<0,>5, orNaN, it will directly skew results. Consider clamping to[1,5]and falling back to1.0for invalid values.- The logs here print the
enameand, when missing, the full list of enames fromreputationResults. Combined with other logs, this is quite PII-heavy and will be very noisy for large groups.Recommend:
- Clamp/validate
memberRep.scorebefore returning.- Gate these logs behind a proper logger with debug level or an env-guard, and avoid dumping the entire ename list in production.
platforms/eVoting/src/app/(app)/[id]/page.tsx (3)
578-687: Unreachable “You voted” branch and duplicated vote parsing logicThe ternary starting at Line 378:
selectedPoll.visibility === "private" ? (...) : (selectedPoll.visibility as string) !== "private" ? (...) : (<> ... "You voted" ... </>)makes the final
elsebranch (containing the new “You voted: …” summary and per-option highlighting in this block) effectively unreachable:
- If
visibility === "private", we take the first arm.- Otherwise
visibility !== "private"is true, so we always take the second arm.- The third arm can never be hit.
At the same time, this block introduces yet another variant of parsing
voteStatus?.vote?.data(especially for rank mode) that differs from the other “Your Vote Details” sections further down.Recommendations:
- Simplify this conditional to clear
if/elseblocks and ensure the “You voted” UI lives in a branch that’s actually rendered (likely in thehasVoted+ active‑poll path, which you already have).- Extract small helpers for “summarizeVoteForHeader” and “isUserChoice/userChoiceDetails” so all these blocks (ended + active, public + private) share a single consistent interpretation of
vote.datafor normal/point/rank modes.
588-605: Rank vote summary logic is inconsistent with other rank handlersIn the new “You voted” summary:
} else if (voteData.mode === "rank" && Array.isArray(voteData.data)) { const rankData = voteData.data; const sortedRanks = [...rankData].sort((a, b) => a.points - b.points); const topChoice = selectedPoll.options[parseInt(sortedRanks[0]?.option || "0")]; return `ranked options (${topChoice} as 1st choice)`; }you assume
voteData.datais an array of{ option, points }.Elsewhere in this same file (e.g. the detailed rank sections) you assume:
const rankData = voteData.data[0]?.points; if (rankData && typeof rankData === "object") { // rankData is an object: { "0": 1, "1": 2, ... } }The backend IRV code also documents a stored format like
[{ option: "ranks", points: { "0": 1, ... } }].These competing assumptions mean one of these paths will misinterpret rank votes (or silently show nonsense) depending on which format is actually produced.
Given this is a newly touched area, I’d recommend:
- Decide on a single canonical stored shape for rank votes.
- Implement a single parser (e.g.
parseRankVoteData(voteData)that handles both legacy and new formats if needed) and reuse it in all UI blocks.- Update this header summary to use that helper instead of directly sorting
voteData.data.Also applies to: 627-660
1283-1311:fetchWithRetrycurrently doesn’t actually provide retriesInside
onSigningComplete:
fetchWithRetrywrapsfetchPoll()andfetchVoteData()in atry/catchloop with retries.- But both
fetchPollandfetchVoteDataalready have their owntry/catchblocks that swallow errors (theyconsole.errorbut do not rethrow), so fromfetchWithRetry’s perspective these calls almost never throw.- As a result, the loop will break on the first iteration even if the underlying API calls failed, and the retry logic is effectively dead.
Either:
- Keep it simple and just call
await fetchPoll(); await fetchVoteData();here, or- Move the retry around the raw
pollApicalls (and let them throw) so retries really kick in.Also,
voteIdis unused in this callback — either wire it into some confirmation UI/logging or drop it from the parameter list if not needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
platforms/eVoting/src/app/(app)/[id]/page.tsx(6 hunks)platforms/eVoting/src/app/(app)/create/page.tsx(2 hunks)platforms/eVoting/src/app/(app)/page.tsx(3 hunks)platforms/eVoting/src/lib/pollApi.ts(1 hunks)platforms/evoting-api/src/controllers/WebhookController.ts(3 hunks)platforms/evoting-api/src/database/entities/VoteReputationResult.ts(1 hunks)platforms/evoting-api/src/database/migrations/1763745645194-migration.ts(1 hunks)platforms/evoting-api/src/services/PollService.ts(1 hunks)platforms/evoting-api/src/services/VoteService.ts(5 hunks)platforms/evoting-api/src/web3adapter/mappings/vote-reputation-result.mapping.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-20T12:00:02.259Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 453
File: platforms/eReputation-api/src/controllers/WebhookController.ts:6-7
Timestamp: 2025-11-20T12:00:02.259Z
Learning: In platforms/eReputation-api/src/controllers/WebhookController.ts, the eager instantiation of VotingReputationService in the constructor (which requires OPENAI_API_KEY at startup) is intentional behavior and works as intended.
Applied to files:
platforms/evoting-api/src/services/PollService.tsplatforms/evoting-api/src/services/VoteService.tsplatforms/evoting-api/src/controllers/WebhookController.ts
🧬 Code graph analysis (1)
platforms/evoting-api/src/database/migrations/1763745645194-migration.ts (3)
platforms/eReputation-api/src/database/migrations/1763622721610-migration.ts (1)
Migration1763622721610(3-22)platforms/evoting-api/src/database/migrations/1763584613393-migration.ts (1)
Migration1763584613393(3-22)platforms/evoting-api/src/database/migrations/1755186562660-migration.ts (1)
Migration1755186562660(3-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (11)
platforms/eVoting/src/lib/pollApi.ts (1)
119-119: LGTM!The addition of
"ereputation"to thePollResults.modetype union properly extends support for eReputation-weighted voting results.platforms/eVoting/src/app/(app)/page.tsx (2)
4-4: LGTM!The new icon imports support the Voting Weight column display.
219-227: LGTM!The voting weight badge display is well-implemented with appropriate icons and styling.
platforms/evoting-api/src/services/PollService.ts (1)
219-222: LGTM!The validation correctly prevents combining blind voting (private visibility) with eReputation weighted voting. The error message is clear and aligns with the frontend enforcement in
platforms/eVoting/src/app/(app)/create/page.tsx.platforms/evoting-api/src/controllers/WebhookController.ts (2)
234-236: LGTM!The validation now correctly checks only for
pollId, which aligns with makinggroupIdnullable in the schema.
255-259: LGTM!Setting
groupIdtogroupId || nullproperly handles the nullable column requirement.platforms/eVoting/src/app/(app)/create/page.tsx (1)
595-595: LGTM!Removing the redundant "(eReputation Weighted)" text improves clarity since the heading already states "eReputation Weighted".
platforms/evoting-api/src/database/entities/VoteReputationResult.ts (1)
31-36: LGTM!The nullable
grouprelation andgroupIdcolumn are properly declared with both TypeORM decorators and TypeScript types. This aligns correctly with the migration inplatforms/evoting-api/src/database/migrations/1763745645194-migration.ts.platforms/evoting-api/src/web3adapter/mappings/vote-reputation-result.mapping.json (1)
5-5: No issues found. The review comment is based on incorrect assumptions about the data flow.The mapping transformation and local ID lookup happen in the
fromGlobal()function (infrastructure/web3-adapter/src/mapper/mapper.ts), not in the controller. Here's how the format"polls(pollId),pollId"works:
In fromGlobal() transformation (lines 188-203):
- Extracts
tableRef = "polls"from the mapping- Extracts
pathRef = "pollId"(the data field name)- Gets the value from incoming data
- Calls
mappingStore.getLocalId()to resolve the global ID to a local ID- Returns the value formatted as
"polls(localId)"In WebhookController (lines 212-218):
- Receives
local.data.pollIdalready containing"polls(localId)"- Correctly parses it by extracting the ID from parentheses
The adapter's mapping resolution logic properly handles the composite format, and the controller correctly processes the already-transformed data. The pollId uses a local ID lookup—it happens upstream in the mapping transformation, not in the controller itself.
platforms/evoting-api/src/services/VoteService.ts (1)
212-223: Clarify what “ereputation calculation complete” means and align with data shapeThe new guard
if (isWeighted && !reputationResults) throw ...is good to prevent silently unweighted results, but today:
- A
VoteReputationResultwith an empty or missingresultsarray will be treated as “complete”, and all voters will silently get weight1.0.- For normal-mode ereputation polls, this means the UI will show a “weighted” poll that is effectively unweighted.
Consider:
- Treating
!reputationResults?.results?.lengthas “not ready yet” and throwing the same error, or at least logging a warning and surfacing a clear status to the client.- Explicitly documenting in the model or comments what state indicates “calculation finished” vs “in progress”.
Would you double‑check with whoever owns the eReputation pipeline whether an empty
resultsarray should also block results as “not yet complete”?platforms/eVoting/src/app/(app)/[id]/page.tsx (1)
4-16: Header badge UI for visibility + voting weight looks consistentThe ChartLine/CircleUser imports and the dual Badge row correctly distinguish public/private and ereputation vs 1P1V weighting without changing behavior elsewhere. No issues spotted here.
Also applies to: 313-348
platforms/evoting-api/src/database/migrations/1763745645194-migration.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
platforms/eVoting/src/app/(app)/[id]/page.tsx (1)
496-526: Critical bug: eReputation-normal polls will render 0 points and mark all options as winners.This is the same issue flagged in the previous review. Line 513 causes all ereputation polls to enter the points-based branch, even when
result.totalPointsis undefined for ereputation-normal polls. This results in:
- All options showing "0 points"
- All options marked as winners (since
Math.max(...0s) === 0)- All percentages showing "0%"
Apply this fix to correctly handle ereputation-normal vs ereputation-point polls:
- if (resultsData.mode === "rank") { + const hasPoints = typeof result.totalPoints === "number"; + if (resultsData.mode === "rank") { // Rank-based voting: show winner status instead of misleading vote counts if (result.isTied) { displayValue = "🏆 Tied Winner"; } else if (result.isWinner) { displayValue = "🏆 Winner"; } else { displayValue = "Eliminated"; } isWinner = result.isWinner || result.isTied || false; percentage = result.percentage || 0; if (resultsData.irvDetails && resultsData.irvDetails.rounds.length > 1) { console.log(`[IRV Debug] Poll had ${resultsData.irvDetails.rounds.length} rounds, check console for tie warnings`); } - } else if (resultsData.mode === "point" || resultsData.mode === "ereputation") { - // Point-based voting: show total points and average - // Check for eReputation weighted points-based voting (mode: "ereputation") + } else if (resultsData.mode === "point" || (resultsData.mode === "ereputation" && hasPoints) || hasPoints) { + // Point-based voting (including ereputation-point): show total points and average const totalPoints = result.totalPoints || 0; displayValue = `${totalPoints} points${result.averagePoints !== undefined ? ` (avg: ${result.averagePoints})` : ''}`; isWinner = totalPoints === Math.max(...resultsData.results.map(r => r.totalPoints || 0)); const totalAllPoints = resultsData.results.reduce((sum, r) => sum + (r.totalPoints || 0), 0); percentage = totalAllPoints > 0 ? (totalPoints / totalAllPoints) * 100 : 0; } else { - // Normal voting: show votes and percentage + // Normal voting (including ereputation-normal): show votes and percentage displayValue = `${result.votes} votes`; isWinner = result.votes === Math.max(...resultsData.results.map(r => r.votes || 0)); - percentage = resultsData.totalVotes > 0 ? (result.votes / resultsData.totalVotes) * 100 : 0; + const totalBase = + typeof resultsData.totalWeightedVotes === "number" && resultsData.mode === "ereputation" + ? resultsData.totalWeightedVotes + : resultsData.totalVotes ?? 0; + percentage = totalBase > 0 ? (result.votes / totalBase) * 100 : 0; }This ensures:
- eReputation-point polls (with
totalPoints) use the points-style path- eReputation-normal polls (without
totalPoints) use the votes-style path with weighted totals
🧹 Nitpick comments (1)
platforms/eVoting/src/app/(app)/[id]/page.tsx (1)
1283-1311: Good improvement: async callback with retry logic.The retry mechanism with immediate fetch followed by delayed retries is more responsive and reliable than a fixed delay. The implementation correctly breaks out of the retry loop on success.
Optional: Consider exponential backoff for retries to reduce server load:
- const fetchWithRetry = async (retries = 3, delay = 1000) => { + const fetchWithRetry = async (retries = 3, baseDelay = 1000) => { for (let i = 0; i < retries; i++) { try { await fetchPoll(); await fetchVoteData(); - // If successful, break out of retry loop break; } catch (error) { console.error(`Error during data refresh (attempt ${i + 1}/${retries}):`, error); if (i < retries - 1) { - // Wait before retrying - await new Promise(resolve => setTimeout(resolve, delay)); + const delay = baseDelay * Math.pow(2, i); // Exponential: 1s, 2s, 4s + await new Promise(resolve => setTimeout(resolve, delay)); } } } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platforms/eVoting/src/app/(app)/[id]/page.tsx(7 hunks)platforms/evoting-api/src/services/VoteService.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/evoting-api/src/services/VoteService.ts
🔇 Additional comments (3)
platforms/eVoting/src/app/(app)/[id]/page.tsx (3)
14-15: LGTM!The new icon imports support the voting weight badge UI and are used appropriately.
313-348: LGTM!The dual-badge layout clearly communicates both poll visibility and voting weight. The conditional rendering and icon selection are implemented correctly.
588-660: The vote data structure is correctly implemented.The code correctly accesses the nested structure. The
Voteinterface definesdata: VoteData, whereVoteDatais an object withmodeanddataproperties. The backend returns this exact structure, and the frontend unpacks it as:voteStatus.vote.data // ← VoteData object with { mode, data } propertiesAccessing
voteData.modeandvoteData.datais valid and matches both the type definitions and backend response structure. No changes are needed.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
platforms/eVoting/src/app/(app)/create/page.tsx (1)
144-153: This redundant reciprocal logic was already flagged in a previous review.Both branches check the same condition (
private+ereputation) but apply different corrections, creating unpredictable behavior. This issue remains unresolved.As suggested in the previous review, simplify to one-directional enforcement:
- // Prevent blind voting (private visibility) + eReputation weighted combination React.useEffect(() => { + // Prevent blind voting (private visibility) + eReputation weighted combination if (watchedVisibility === "private" && watchedVotingWeight === "ereputation") { - // If private visibility is selected and user tries to select eReputation, force to 1p1v setValue("votingWeight", "1p1v"); - } else if (watchedVotingWeight === "ereputation" && watchedVisibility === "private") { - // If eReputation is selected and user tries to select private visibility, force to public - setValue("visibility", "public"); } }, [watchedVisibility, watchedVotingWeight, setValue]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
platforms/eVoting/src/app/(app)/[id]/page.tsx(7 hunks)platforms/eVoting/src/app/(app)/create/page.tsx(5 hunks)platforms/eVoting/src/lib/pollApi.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/eVoting/src/app/(app)/[id]/page.tsx
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 453
File: platforms/eReputation-api/src/controllers/WebhookController.ts:6-7
Timestamp: 2025-11-20T12:00:02.259Z
Learning: In platforms/eReputation-api/src/controllers/WebhookController.ts, the eager instantiation of VotingReputationService in the constructor (which requires OPENAI_API_KEY at startup) is intentional behavior and works as intended.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
platforms/eVoting/src/lib/pollApi.ts (1)
117-117: LGTM!The
totalWeightedVotesfield appropriately captures the sum of weighted votes for eReputation-based polls.platforms/eVoting/src/app/(app)/create/page.tsx (1)
566-597: LGTM! UI constraints correctly enforce the business rules.The disabled states and conditional messaging appropriately prevent combining:
- eReputation weighting with private visibility
- eReputation weighting with rank-based voting
The user-facing text clearly communicates these constraints.
Description of change
Fix eVoting results
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.