Separate crypto, new consolidated test data, tsv file support#161
Separate crypto, new consolidated test data, tsv file support#161
Conversation
sfraczek
added
enhancement
sfraczek
force-pushed
the
sfraczek/separate-crypto
branch
from
1c7f18b to
eb4bb1e
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR separates crypto transactions from stock sales in the transaction accumulator and adds a new consolidated test fixture (TSV format) containing crypto transactions. The changes enable proper differentiation between stock and cryptocurrency investment data.
Key Changes:
- Refactored
TransactionAccumulatorto separate stock and crypto transactions into distinctInvestmentTransactionsstructures - Updated
parse_investment_transaction_datesto accept multiple candidate column names for flexible date parsing - Added support for TSV file format alongside CSV
Reviewed Changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/ecb.rs | Removed debug println statement |
| src/csvparser.rs | Restructured transaction accumulator to separate stock/crypto, added TSV support, enhanced date column flexibility, and added comprehensive test for consolidated crypto statements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sfraczek
changed the title
sfraczek
force-pushed
the
sfraczek/separate-crypto
branch
from
33c4cd8 to
55e166c
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sfraczek
force-pushed
the
sfraczek/separate-crypto
branch
2 times, most recently
from
e87eeee to
468aff4
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let original_delimiter: u8 = if csvtoparse.ends_with(".tsv") { | ||
| b'\t' | ||
| } else { | ||
| b',' | ||
| }; |
There was a problem hiding this comment.
[nitpick] The delimiter detection based on file extension is fragile. Consider detecting the delimiter by analyzing the file content (e.g., counting occurrences of tabs vs commas in the first few lines) or accepting it as a parameter to make the function more robust.
| fn test_parse_revolut_transactions_consolidated_crypto_tsv() -> Result<(), String> { | ||
| // This test verifies that the consolidated TSV with crypto transactions | ||
| // is parsed and that crypto cost basis and gross proceeds are counted | ||
| // according to the summary present in the file (Wpływy brutto = 7,95$, Podstawa kosztowa = 0$). |
There was a problem hiding this comment.
Please use english language for comments
sfraczek
force-pushed
the
sfraczek/separate-crypto
branch
from
468aff4 to
27eb115
Compare
Separating crypto from stock sales and adding example consolidated statement for testing that contains crypto.