Round mirrored GIVeconomy sync percentages#2310
Conversation
Normalize mirrored GIVeconomy boosting percentages to v5 precision so reverse sync no longer stalls on high-precision payloads. Made-with: Cursor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 26 minutes and 55 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe changes implement percentage normalization for GIVeconomy boosting updates by introducing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
🧹 Nitpick comments (2)
src/services/giveconomyPowerSyncService.ts (1)
29-39: Consider sharing precision logic withpowerBoostingRepository.
src/repositories/powerBoostingRepository.tsalready readsGIVPOWER_BOOSTING_PERCENTAGE_PRECISIONand defines aformatPercentagehelper with the same+p.toFixed(precision)logic. Duplicating it here risks the two layers drifting (e.g., different defaults, or one reading env at module load vs. per-call). ExportingformatPercentage(or a shared util) and reusing it would keep the rounding contract single-sourced.Also note a subtle behavioral difference: the repository captures
PERCENTAGE_PRECISIONonce at module load, whileroundSyncedPercentagere-readsprocess.envon every call. This is what makes the test’s per-test env assignment work for the service but not for the repository — worth being aware of if you ever need runtime reconfiguration consistently across both.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/giveconomyPowerSyncService.ts` around lines 29 - 39, roundSyncedPercentage duplicates the precision/rounding logic already implemented in powerBoostingRepository.formatPercentage (and they differ in when they read process.env), so export and reuse that single implementation instead of duplicating it: remove roundSyncedPercentage and import formatPercentage from powerBoostingRepository (or move the logic into a shared util and export it), then call formatPercentage(percentage) from the service; ensure you preserve the desired env-read behavior (choose module-load capture or per-call read) consistently across both modules when consolidating.src/services/giveconomyPowerSyncService.test.ts (1)
43-119: Good coverage of the rounding + zero-drop cases.The fixture exercises both high-precision rounding (
18.6789→18.68,14.754→14.75) and the round-down-to-zero drop (0.004) alongside an explicit0, and asserts the exactprojectIds/percentagespassed tosetMultipleBoosting. This locks in the behavior described in the PR objectives.One optional addition: a case where the value rounds up to a non-zero (e.g.,
0.006→0.01) would explicitly pin down the boundary between "dropped" and "kept", but it’s not required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/giveconomyPowerSyncService.test.ts` around lines 43 - 119, Add an extra boosting fixture that rounds up from a tiny non-zero value (e.g., add { projectId: 3000, percentage: 0.006, ... } in the axios stub payload) and update the assertions for setMultipleBoosting (in the test 'rounds mirrored percentages...') to expect projectIds to include 3000 and percentages to include 0.01; keep the existing checks that 0.004 and 0 are dropped and that flags (allowZeroTotal, allowPartialTotal, allowExceedProjectLimit, emitOutboxEvent, beforeSave) and callCount remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/services/giveconomyPowerSyncService.test.ts`:
- Around line 43-119: Add an extra boosting fixture that rounds up from a tiny
non-zero value (e.g., add { projectId: 3000, percentage: 0.006, ... } in the
axios stub payload) and update the assertions for setMultipleBoosting (in the
test 'rounds mirrored percentages...') to expect projectIds to include 3000 and
percentages to include 0.01; keep the existing checks that 0.004 and 0 are
dropped and that flags (allowZeroTotal, allowPartialTotal,
allowExceedProjectLimit, emitOutboxEvent, beforeSave) and callCount remain
unchanged.
In `@src/services/giveconomyPowerSyncService.ts`:
- Around line 29-39: roundSyncedPercentage duplicates the precision/rounding
logic already implemented in powerBoostingRepository.formatPercentage (and they
differ in when they read process.env), so export and reuse that single
implementation instead of duplicating it: remove roundSyncedPercentage and
import formatPercentage from powerBoostingRepository (or move the logic into a
shared util and export it), then call formatPercentage(percentage) from the
service; ensure you preserve the desired env-read behavior (choose module-load
capture or per-call read) consistently across both modules when consolidating.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e7bd02b-0ce9-4ebd-86f6-157d9372a061
📒 Files selected for processing (2)
src/services/giveconomyPowerSyncService.test.tssrc/services/giveconomyPowerSyncService.ts
Use the validated cause input to derive the recipient address network so resolver CI does not insert a null networkId. Add a repository regression assertion to cover the created recipient address. Made-with: Cursor
Create unique projects and reactions in the resolver tests instead of relying on staging snapshot titles and reaction ids. Also assert slug history against the previous slug so the updateProject test matches current behavior. Made-with: Cursor
Issue
Summary
Round mirrored GIVeconomy boosting percentages to v5 precision before applying them in impact-graph so reverse sync no longer stalls on high-precision payloads. This keeps the v6 -> v5 sync cursor moving when the upstream event feed contains percentages that do not fit v5's
numeric(5,2)storage.Changes
setMultipleBoostingHow to Test
NODE_ENV=test npx mocha ./src/services/giveconomyPowerSyncService.test.ts.18.6789are written as18.68and tiny values like0.004are dropped.power_sync_cursorrow forsourceSystem = 'giveconomy'advances past previously blocked events and new mirrored boosting updates start landing on v5 again.Made with Cursor
Summary by CodeRabbit
Release Notes