feat(report): add FX method-consistency and deferred-conversion notes#252
Merged
Conversation
Clarify how DeclaRenta handles foreign-currency (divisa) gains for foreign-currency stock accounts, after a community thread surfaced contradictory interpretations of the realize-vs-defer question. - report.fx_method_consistency (info): when FX disposals exist, explain the divisa gain is deferred until the currency is converted to euros (Art. 14.2.e LIRPF; DGT V0152-26) and warn not to mix methods across years, which can double-count or drop carried basis. - report.fx_deferred_no_conversion (info): when the user holds foreign-currency securities (non-EUR STK/FUND/BOND) but made no FCY->EUR conversion, confirm that 0 in casillas 1633/1637 is correct and that IBKR's "realized forex gain" uses the US criterion, not the Spanish one. - guide_rw.fx_note: rewrite in all 5 locales. The old text wrongly said FX only appears on manual currency trades, false since auto-convert (#239) and stock round-trips (#230). Both notes are info severity (not warnings) and excluded from the deprecated warnings/messages sync check, matching the existing report.competitor_reconciliation note.
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthrough
FX Messaging and Localization Update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #252 +/- ##
=======================================
Coverage 95.29% 95.30%
=======================================
Files 54 54
Lines 5185 5192 +7
Branches 1736 1739 +3
=======================================
+ Hits 4941 4948 +7
Misses 210 210
Partials 34 34
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two in-app info notes and fixes a stale guide string clarifying how DeclaRenta treats foreign-currency (divisa) gains for foreign-currency stock accounts. Motivated by a community thread where users got contradictory answers (realize-at-purchase vs. defer-to-conversion) for the "I trade US stocks in a USD account and never convert to EUR" case.
DeclaRenta already implements the legally-correct deferral method (carry-basis; Art. 14.2.e LIRPF; DGT V0152-26): buying a stock in USD does not realize an FX gain — only an actual USD→EUR conversion does. The engine is unchanged; this PR closes the communication gaps around it.
Changes
report.fx_method_consistency(info, fires when FX disposals exist): explains the divisa gain is deferred until conversion to euros, and warns not to mix methods across years (mixing defer with a realize-at-buy method can double-count or drop carried basis).report.fx_deferred_no_conversion(info, fires when there are 0 FX disposals but the user holds non-EUR STK/FUND/BOND securities): confirms0in casillas 1633/1637 is correct, and that IBKR's "realized forex gain" uses the US criterion, not the Spanish one. Scoped so it never fires on EUR-only or crypto-only files.guide_rw.fx_noterewritten in all 5 locales (es, en, ca, eu, gl): the old text wrongly said FX only appears on manual currency trades — false since auto-convert (ISSUE El motor de detección ifFxcon genera operaciones fantasma. #239) and stock round-trips (ISSUE. Las ventas de acciones con plus/minusvalía en FCY no están trasladando efectos a la Divisa #230).Both notes are
severity: "info"(not warnings), consistent with the project's "don't alarm users with non-critical info" posture, and are excluded from the deprecatedwarnings/messagessync check exactly like the existingreport.competitor_reconciliationnote.Testing
npm run typecheck,npm run typecheck:tests,npm run lint— cleannpm test— relevant suites green: FX integration (carry-basis, trace), generators (36/36 incl. the updated sync test), i18n + web + generators batch (433/433)should keep warnings and messages in synctest to exclude the two new info-only IDs (same precedent ascompetitor_reconciliation)Summary by CodeRabbit
New Features
Documentation
Tests