Skip to content

reverse sign of amount for sell and buy in app/models/binance_account/processor.rb#1451

Open
IamTaoChen wants to merge 1 commit intowe-promise:mainfrom
IamTaoChen:binance-import
Open

reverse sign of amount for sell and buy in app/models/binance_account/processor.rb#1451
IamTaoChen wants to merge 1 commit intowe-promise:mainfrom
IamTaoChen:binance-import

Conversation

@IamTaoChen
Copy link
Copy Markdown

@IamTaoChen IamTaoChen commented Apr 12, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Fixed the sign convention for USD amounts in spot trade records. Buyer entries now correctly record positive USD amounts, while seller entries correctly record negative USD amounts. This ensures greater accuracy in your trade history, financial reports, and overall account balance tracking.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 12, 2026

📝 Walkthrough

Walkthrough

Updated USD amount sign handling for Binance spot trades in the processor module. Buyer entries now record positive amounts while seller entries record negative amounts, reversing the previous sign convention. Other trade-processing logic remains unchanged.

Changes

Cohort / File(s) Summary
Spot Trade Amount Sign Correction
app/models/binance_account/processor.rb
Inverted USD amount signs for spot trades: buyer entries changed from -amount_usd to amount_usd; seller entries changed from amount_usd to -amount_usd.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A flip of the sign, oh what delight,
Buyers gain positive, sellers go slight,
Two tiny lines changed with care,
The ledger now balanced, perfectly fair! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: reversing the sign of amount for sell and buy transactions in the processor file, which matches the summary of swapping positive/negative amounts between buyer and seller entries.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/models/binance_account/processor.rb (1)

175-196: ⚠️ Potential issue | 🟠 Major

Backfill existing Binance spot entries before flipping the sign.

This only fixes newly created rows. Previously imported spot trades keep the old convention because fetch_new_trades only requests ids after the cached max trade id, and external_id skips any existing entry. Existing Binance users will therefore keep inverted historical inflow/outflow and balance data unless this ships with a reimport or data backfill path.


ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 45e462af-d2ad-4c0a-bee1-32418dd4b091

📥 Commits

Reviewing files that changed from the base of the PR and between 11f2131 and 4c36198.

📒 Files selected for processing (1)
  • app/models/binance_account/processor.rb

@jjmata
Copy link
Copy Markdown
Collaborator

jjmata commented Apr 12, 2026

Can you review @boul2gom? 🙏

@jjmata jjmata self-requested a review April 12, 2026 22:52
@jjmata jjmata added this to the v0.7.0 milestone Apr 12, 2026
@IamTaoChen IamTaoChen changed the title reverse sign of amount for sell and buy reverse sign of amount for sell and buy in app/models/binance_account/processor.rb Apr 13, 2026
@sure-admin sure-admin modified the milestones: v0.7.0, v0.7.1 Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants