From ac2a25261f214ac1a691ba0698ccbb6caa6260de Mon Sep 17 00:00:00 2001 From: Nikolay Metchev Date: Mon, 31 Aug 2026 00:47:40 +0300 Subject: [PATCH 1/6] feat(csv): assemble trades from row groups and add the Binance CSV strategy Binance's transaction-history export splits one trade across several rows that share a timestamp (one row per partial fill per leg), which the CSV engine could not model: a trade needed its credited leg on the same row. Two new, generic config primitives close that gap: - TradeGroupConfig buckets a strategy's trade legs by timestamp and folds each bucket whose debits name one asset and whose credits name one other into a single same-account trade. A bucket that does not resolve is left alone and its rows import as ordinary transfers, so no row is ever dropped. - ConversionConfig.sideAmountColumn classifies a conversion leg by the sign of an amount column, for sources that give both legs one operation name (Binance's dust sweeps). TradeGroupConfig takes the same option for the same reason. Both export fields are @EncodeDefault(NEVER), so no existing strategy's canonical catalog hash changes. The Binance CSV strategy imports the modern 7-column export. Deposits, withdrawals, Earn subscriptions and rewards route to the accounts the Binance API strategy also creates, so whichever source imports second reconciles against the first; staking, BNB Vault, Launchpool, Launchpad, commission, liquidity farming and dual savings have no API endpoint and are the reason to import the file at all. Dust sweeps go through ConversionConfig rather than trade assembly: nothing in the export attributes a credited BNB amount to a debited asset. Legacy 6-column exports are deliberately not supported - they use an older Operation vocabulary that would book every event a second time under a different description. A content rule on User_ID rejects them, so they report as skipped. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UijVfE192UWTLiAjx3mvVL --- .../csvimporter/CsvImportApplier.kt | 60 ++++- .../csvimporter/CsvTradeGroups.kt | 134 +++++++++ .../csvimporter/CsvTransferMapper.kt | 78 +++++- .../csvimporter/BinanceCsvMapperTest.kt | 207 ++++++++++++++ .../csvimporter/CsvTradeGroupsTest.kt | 226 ++++++++++++++++ .../csvimporter/StrategySelectorTest.kt | 49 ++++ .../service/CsvStrategyExportService.kt | 1 + .../database/json/FieldMappingJsonCodec.kt | 5 + .../CsvImportStrategyReadRepositoryImpl.kt | 2 + .../csvImportStrategy/CsvImportStrategy.sq | 1 + .../CsvImportStrategyWriteRepositoryImpl.kt | 1 + .../database/write/CsvImportStrategyInsert.kt | 1 + .../CsvImportStrategyWrite.sq | 6 +- .../model/csvstrategy/ConversionConfig.kt | 12 + .../model/csvstrategy/CsvImportStrategy.kt | 5 + .../model/csvstrategy/TradeGroupConfig.kt | 62 +++++ .../csvstrategy/export/CsvStrategyExport.kt | 6 + .../export/CsvStrategyExportMapper.kt | 1 + .../builtin/BuiltInCsvStrategies.kt | 255 ++++++++++++++++++ .../editor/CsvStrategyEditorFields.kt | 1 + .../editor/CsvStrategyEditorState.kt | 5 + 21 files changed, 1104 insertions(+), 14 deletions(-) create mode 100644 app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTradeGroups.kt create mode 100644 app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt create mode 100644 app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt create mode 100644 app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt index df279380d..fb11eaaa5 100644 --- a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt @@ -344,8 +344,16 @@ suspend fun applyStagedCsv( val stagedImport = restageXlsxForStrategy(csvImport, strategy, csvImportRepository, importEngine) val allRows = csvImportRepository.getImportRows(stagedImport.id, limit = stagedImport.rowCount.coerceAtLeast(1), offset = 0) - val rows = allRows.filter { it.importStatus == null || it.importStatus == ImportStatus.ERROR } - if (rows.isEmpty()) { + val unprocessedRows = allRows.filter { it.importStatus == null || it.importStatus == ImportStatus.ERROR } + // A strategy that assembles a trade from a whole row group must see the group whole: mapping only + // the unprocessed rows would turn a group with one errored leg into a one-legged group and assemble + // it wrongly (or not at all). Re-mapping the settled legs costs a pass and changes nothing — the + // trade's exact-tuple idempotency makes re-emitting the group a no-op that resolves to DUPLICATE. + // Transfers are still emitted only for the unprocessed rows (see [unprocessedRowIndexes] below), so + // widening the mapped set changes what groups see and nothing else. + val rows = if (strategy.tradeGroupConfig != null) allRows else unprocessedRows + val unprocessedRowIndexes: Set = unprocessedRows.mapTo(mutableSetOf()) { it.rowIndex } + if (unprocessedRows.isEmpty()) { // A genuinely empty file (a header-only export with no data rows) has nothing to import, but // must still be marked applied or it reappears in the Unimported tab on every "Import all". // Record the strategy application once (the lastAppliedAt guard keeps repeated runs a no-op). @@ -405,6 +413,7 @@ suspend fun applyStagedCsv( onProgress = onProgress, engineBatchSize = engineBatchSize, attributeAccountMatchers = attributeAccountMatchers, + unprocessedRowIndexes = unprocessedRowIndexes, ) } @@ -666,6 +675,10 @@ suspend fun runCsvImport( onProgress: (suspend (ImportProgress) -> Unit)? = null, engineBatchSize: Int = Int.MAX_VALUE, attributeAccountMatchers: Map = emptyMap(), + // Rows still awaiting import. Equals every row index in [rows] except when the strategy assembles + // trades from row groups, where [rows] is widened to the whole file so a group is never seen with + // some of its legs missing; transfers are then still emitted only for the rows named here. + unprocessedRowIndexes: Set = rows.mapTo(mutableSetOf()) { it.rowIndex }, ): CsvImportResult { logger.info { "Starting CSV import with ${basePrep.validTransfers.size} valid transfers" } @@ -794,12 +807,26 @@ suspend fun runCsvImport( null } + // Sources that split one trade across several rows (Binance stamps every partial fill of both legs + // with the same second) are assembled here: each resolvable group folds into one trade and its legs + // drop out of the transfer list. A group that does not resolve assembles to null and its rows stay + // ordinary transfers, so nothing is ever dropped for want of a clean pairing. + val assembledTrades = + strategy.tradeGroupConfig?.let { config -> + groupTradeLegs(finalPrep.validTransfers, config).mapNotNull { it.assemble(config) } + }.orEmpty() + val assembledRowIndexes: Set = assembledTrades.flatMapTo(mutableSetOf()) { it.group.rowIndexes } + val assembledTradeRowIndexes: Map> = + assembledTrades.associate { assembled -> + assembled.tradeKey(csvImport.id) to assembled.group.rowIndexes + } + // Rows carrying a credited leg (Currency != To Currency) are cross-asset conversions → trades. val importTrades = finalPrep.validTransfers.mapNotNull { row -> val credit = row.tradeTo ?: return@mapNotNull null ImportTradeIntent( - key = LocalTradeKey("csv-${csvImport.id.id}-${row.rowIndex}"), + key = LocalTradeKey("$CSV_TRADE_KEY_PREFIX${csvImport.id.id}-${row.rowIndex}"), source = Source.Csv(csvImport.id), timestamp = row.transfer.timestamp, description = row.transfer.description, @@ -808,7 +835,19 @@ suspend fun runCsvImport( toAccountId = row.transfer.targetAccountId, toAmount = credit, ) - } + } + + assembledTrades.map { assembled -> + ImportTradeIntent( + key = assembled.tradeKey(csvImport.id), + source = Source.Csv(csvImport.id), + timestamp = assembled.timestamp, + description = assembled.description, + fromAccountId = assembled.ownerAccountId, + fromAmount = assembled.fromAmount, + toAccountId = assembled.ownerAccountId, + toAmount = assembled.toAmount, + ) + } // A trade carries no fee field, so a conversion row that also has a fee would otherwise drop it. // Emit each such fee as its own standalone movement (source account -> " Fees") so the @@ -859,7 +898,9 @@ suspend fun runCsvImport( } val importTransfers = - finalPrep.validTransfers.filter { it.tradeTo == null }.map { row -> + finalPrep.validTransfers + .filter { it.tradeTo == null && it.rowIndex !in assembledRowIndexes && it.rowIndex in unprocessedRowIndexes } + .map { row -> val uniqueKey = if (uniqueIdTypeNames.isEmpty()) { null @@ -1049,16 +1090,21 @@ suspend fun runCsvImport( // same event arriving from another export) — and clear their errors too, otherwise a converted row // keeps a stale ERROR status and gets reprocessed on the next import. The key is // "csv--". - val tradeKeyPrefix = "csv-${csvImport.id.id}-" + val tradeKeyPrefix = "$CSV_TRADE_KEY_PREFIX${csvImport.id.id}-" fun LocalTradeKey.rowIndexOrNull(): Long? = if (value.startsWith(tradeKeyPrefix)) value.removePrefix(tradeKeyPrefix).toLongOrNull() else null + // An assembled trade speaks for every leg of its group, not just the row its key names, so all of + // them take the trade's outcome. A single-row conversion has no group and falls back to its own row. + fun LocalTradeKey.rowIndexes(): List = + assembledTradeRowIndexes[this] ?: listOfNotNull(rowIndexOrNull()) + // The row records the trade's transaction id, exactly as a transfer row records its transfer's: // it is what links the row to what it produced, and what lets a re-import find the trade again. val tradeRowsByStatus = importResult.createdTradeIds.entries - .mapNotNull { (key, tradeId) -> key.rowIndexOrNull()?.let { Triple(key, it, tradeId) } } + .flatMap { (key, tradeId) -> key.rowIndexes().map { Triple(key, it, tradeId) } } .groupBy( keySelector = { (key, _, _) -> if (key in importResult.dedupedTradeKeys) ImportStatus.DUPLICATE else ImportStatus.IMPORTED diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTradeGroups.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTradeGroups.kt new file mode 100644 index 000000000..1036d8a05 --- /dev/null +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTradeGroups.kt @@ -0,0 +1,134 @@ +package com.moneymanager.csvimporter + +import com.moneymanager.bigdecimal.BigInteger +import com.moneymanager.domain.model.AccountId +import com.moneymanager.domain.model.CsvImportId +import com.moneymanager.domain.model.Money +import com.moneymanager.domain.model.csvstrategy.TradeGroupConfig +import com.moneymanager.importengineapi.LocalTradeKey +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * Prefix of the [LocalTradeKey] a CSV row (or row group) produces. The remainder is the import id and + * the row index, so a key decodes back to the row whose outcome must be written. + */ +const val CSV_TRADE_KEY_PREFIX = "csv-" + +/** + * Rows a source split across one trade, bucketed by timestamp (see [TradeGroupConfig]). A group holds + * only rows the mapper flagged as trade legs; fee rows are deliberately not part of it. + */ +data class TradeGroup( + val debits: List, + val credits: List, +) { + val rows: List get() = debits + credits + + /** Row indexes of every leg, for writing the assembled trade's outcome back to all of them. */ + val rowIndexes: List get() = rows.map { it.rowIndex } +} + +/** + * A [TradeGroup] that resolved into a single trade: one asset out, one other asset in, both on the + * same owner account. + * + * @property timestamp The group's earliest row timestamp, so re-imports and different files that + * contain the same event agree on one instant. + */ +data class AssembledTrade( + val group: TradeGroup, + val ownerAccountId: AccountId, + val timestamp: Instant, + val fromAmount: Money, + val toAmount: Money, + val description: String, +) + +/** + * The batch key for this assembled trade. Keyed on the group's lowest row index so the key is stable + * across re-imports of the same file and decodes back to a real row for status write-back. + */ +fun AssembledTrade.tradeKey(csvImportId: CsvImportId): LocalTradeKey = + LocalTradeKey("$CSV_TRADE_KEY_PREFIX${csvImportId.id}-${group.rowIndexes.min()}") + +/** + * Splits [rows] into trade groups. Rows the mapper did not flag as a trade leg are ignored; the caller + * still imports them as ordinary transfers. + * + * Grouping is a greedy chain over time: a leg joins the current group while it is within + * [TradeGroupConfig.groupingWindowSeconds] of the group's **last** leg, so a group whose legs straddle + * a second boundary still holds together while genuinely separate events (minutes or hours apart) stay + * separate. A zero window means legs must share the exact instant. + */ +fun groupTradeLegs( + rows: List, + config: TradeGroupConfig, +): List { + val legs = + rows + .filter { it.tradeLeg != null } + .sortedWith(compareBy({ it.transfer.timestamp }, { it.rowIndex })) + if (legs.isEmpty()) return emptyList() + + val window = config.groupingWindowSeconds.seconds + val groups = mutableListOf>() + var current = mutableListOf(legs.first()) + for (leg in legs.drop(1)) { + val previous = current.last().transfer.timestamp + if (leg.transfer.timestamp - previous <= window) { + current += leg + } else { + groups += current + current = mutableListOf(leg) + } + } + groups += current + + return groups.map { group -> + TradeGroup( + debits = group.filter { it.tradeLeg?.side == TradeLegSide.DEBIT }, + credits = group.filter { it.tradeLeg?.side == TradeLegSide.CREDIT }, + ) + } +} + +/** + * Folds a group into one trade, or returns null when it does not resolve — no legs on a side, more + * than one asset on a side, a zero total, or legs that disagree about which account they belong to. + * A null is not an error: the caller leaves the group's rows to import as ordinary transfers, so the + * residue lands somewhere visible instead of being silently reshaped or dropped. + * + * The owner account is read off the legs themselves: with the usual `flipAccountsOnPositive` mapping a + * debit leg has the owner as its source and a credit leg has it as its target. + */ +@Suppress("ReturnCount") +fun TradeGroup.assemble(config: TradeGroupConfig): AssembledTrade? { + if (debits.isEmpty() || credits.isEmpty()) return null + + val ownerAccountIds = + (debits.map { it.transfer.sourceAccountId } + credits.map { it.transfer.targetAccountId }).toSet() + val ownerAccountId = ownerAccountIds.singleOrNull() ?: return null + + val fromAsset = debits.map { it.transfer.amount.asset }.distinctBy { it.id }.singleOrNull() ?: return null + val toAsset = credits.map { it.transfer.amount.asset }.distinctBy { it.id }.singleOrNull() ?: return null + if (fromAsset.id == toAsset.id) return null + + // Leg amounts are already absolute (the mapper takes abs() and encodes direction in the accounts), + // so summing each side gives the two totals of the one conversion the group describes. + val fromAmount = debits.map { it.transfer.amount }.reduce(Money::plus) + val toAmount = credits.map { it.transfer.amount }.reduce(Money::plus) + if (fromAmount.amount == BigInteger.ZERO || toAmount.amount == BigInteger.ZERO) return null + + return AssembledTrade( + group = this, + ownerAccountId = ownerAccountId, + timestamp = rows.minOf { it.transfer.timestamp }, + fromAmount = fromAmount, + toAmount = toAmount, + description = + config.descriptionTemplate + .replace("{from}", fromAsset.code) + .replace("{to}", toAsset.code), + ) +} diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTransferMapper.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTransferMapper.kt index e18855f73..e36ac5859 100644 --- a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTransferMapper.kt +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvTransferMapper.kt @@ -97,6 +97,8 @@ sealed interface MappingResult { val passThrough: CsvPassThrough? = null, /** Set when the row is one leg of an asset conversion (see `ConversionConfig`); null otherwise. */ val conversionLeg: ConversionLegInfo? = null, + /** Set when the row is one leg of a row-group trade (see `TradeGroupConfig`); null otherwise. */ + val tradeLeg: TradeLegInfo? = null, /** Raw funding value from [CsvImportStrategy.fundingAttributeMatch]'s column; null when unset/blank. */ val fundingMatchValue: String? = null, /** @@ -177,6 +179,19 @@ data class ConversionLegInfo( val pairingKey: String, ) +/** Which leg of a row-group trade a row represents (see `TradeGroupConfig`). */ +enum class TradeLegSide { DEBIT, CREDIT } + +/** + * Marks a mapped row as one leg of a trade the source split across several rows (see + * `TradeGroupConfig`). The applier buckets legs sharing a timestamp and folds each resolvable bucket + * into one `trade`; a bucket that does not resolve leaves its rows to import as ordinary transfers, + * so this marker never causes a row to be dropped. + */ +data class TradeLegInfo( + val side: TradeLegSide, +) + /** * A transfer with its associated attributes extracted from CSV. * Uses attribute type names (not IDs) since types may need to be created. @@ -205,6 +220,8 @@ data class CsvTransferWithAttributes( val passThrough: CsvPassThrough? = null, /** Set when the row is one leg of an asset conversion (see `ConversionConfig`); null otherwise. */ val conversionLeg: ConversionLegInfo? = null, + /** Set when the row is one leg of a row-group trade (see `TradeGroupConfig`); null otherwise. */ + val tradeLeg: TradeLegInfo? = null, /** * Raw value of the strategy's [CsvImportStrategy.fundingAttributeMatch] column for this row (e.g. a * card's last-4 like "7721"); null when the strategy declares no funding match or the cell is blank. @@ -340,6 +357,12 @@ class CsvTransferMapper( .orEmpty() .map { Regex(it.pattern, RegexOption.IGNORE_CASE) to it } + // Precompiled row-group trade detection (null when the strategy declares no tradeGroupConfig). + private val tradeDebitRegex: Regex? = + strategy.tradeGroupConfig?.let { Regex(it.debitPattern, RegexOption.IGNORE_CASE) } + private val tradeCreditRegex: Regex? = + strategy.tradeGroupConfig?.let { Regex(it.creditPattern, RegexOption.IGNORE_CASE) } + // Extract unique identifier column names from strategy private val uniqueIdentifierColumns: List = strategy.attributeMappings.filter { it.isUniqueIdentifier }.map { it.columnName } @@ -393,6 +416,7 @@ class CsvTransferMapper( personalCounterpartyName = result.personalCounterpartyName, passThrough = result.passThrough, conversionLeg = result.conversionLeg, + tradeLeg = result.tradeLeg, fundingMatchValue = result.fundingMatchValue, unidentifiedCounterpartyAccountId = result.unidentifiedCounterpartyAccountId, ), @@ -714,6 +738,7 @@ class CsvTransferMapper( passThrough = passThrough, conversionLeg = conversionDetection?.let { ConversionLegInfo(side = it.side, pairingKey = it.pairingKey) }, + tradeLeg = detectTradeLeg(originalValues), fundingMatchValue = strategy.fundingAttributeMatch?.let { getColumnValueOrNull(it.column, originalValues)?.trim()?.takeIf { v -> v.isNotBlank() } @@ -773,11 +798,31 @@ class CsvTransferMapper( val config = strategy.conversionConfig ?: return null val signal = getColumnValueOrNull(config.signalColumn, values)?.trim().orEmpty() if (signal.isEmpty()) return null + val matchesFamily = + conversionDebitRegex?.containsMatchIn(signal) == true || + conversionCreditRegex?.containsMatchIn(signal) == true + if (!matchesFamily) return null + // A source that names both legs identically (Binance's "Small Assets Exchange BNB" labels the + // swept asset and the BNB received the same) can only be told apart by the sign of its amount. val side = - when { - conversionDebitRegex?.containsMatchIn(signal) == true -> ConversionSide.DEBIT - conversionCreditRegex?.containsMatchIn(signal) == true -> ConversionSide.CREDIT - else -> return null + when (val sideColumn = config.sideAmountColumn) { + null -> + when { + conversionDebitRegex?.containsMatchIn(signal) == true -> ConversionSide.DEBIT + else -> ConversionSide.CREDIT + } + else -> { + val amount = + getColumnValueOrNull(sideColumn, values) + ?.takeIf { it.isNotBlank() } + ?.let { runCatching { parseBigDecimal(it) }.getOrNull() } + ?: return null + when { + amount < BigDecimal.ZERO -> ConversionSide.DEBIT + amount > BigDecimal.ZERO -> ConversionSide.CREDIT + else -> return null + } + } } val accountName = conversionAccountRuleRegexes @@ -797,6 +842,31 @@ class CsvTransferMapper( return ConversionDetection(side, accountName, "$base$PAIRING_KEY_SEPARATOR$extra") } + /** + * Detects whether [values] is a leg of a row-group trade per [CsvImportStrategy.tradeGroupConfig]. + * Returns null when the strategy declares no trade-group config or the signal column matches + * neither the debit nor the credit pattern — including for a fee row, which the config leaves out + * on purpose so it imports as its own transfer. + */ + private fun detectTradeLeg(values: List): TradeLegInfo? { + val config = strategy.tradeGroupConfig ?: return null + val signal = getColumnValueOrNull(config.signalColumn, values)?.trim().orEmpty() + if (signal.isEmpty()) return null + val isDebitPattern = tradeDebitRegex?.containsMatchIn(signal) == true + if (!isDebitPattern && tradeCreditRegex?.containsMatchIn(signal) != true) return null + val sideColumn = config.sideAmountColumn ?: return TradeLegInfo(if (isDebitPattern) TradeLegSide.DEBIT else TradeLegSide.CREDIT) + val amount = + getColumnValueOrNull(sideColumn, values) + ?.takeIf { it.isNotBlank() } + ?.let { runCatching { parseBigDecimal(it) }.getOrNull() } + ?: return null + return when { + amount < BigDecimal.ZERO -> TradeLegInfo(TradeLegSide.DEBIT) + amount > BigDecimal.ZERO -> TradeLegInfo(TradeLegSide.CREDIT) + else -> null + } + } + private fun parseAmount( amountMapping: FieldMapping, values: List, diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt new file mode 100644 index 000000000..29d89e34a --- /dev/null +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt @@ -0,0 +1,207 @@ +package com.moneymanager.csvimporter + +import com.moneymanager.builtin.BuiltInCsvStrategies +import com.moneymanager.domain.model.Account +import com.moneymanager.domain.model.AccountId +import com.moneymanager.domain.model.CryptoAsset +import com.moneymanager.domain.model.CryptoId +import com.moneymanager.domain.model.Currency +import com.moneymanager.domain.model.CurrencyId +import com.moneymanager.domain.model.CurrencyScaleFactors +import com.moneymanager.domain.model.csv.CsvColumn +import com.moneymanager.domain.model.csv.CsvColumnId +import com.moneymanager.domain.model.csv.CsvRow +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertIs +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import kotlin.time.Clock +import kotlin.uuid.Uuid + +/** + * Covers the built-in Binance CSV strategy's row-level behaviour: which counterparty account each + * `Operation` routes to, how the sign of `Change` sets the direction, and how trade and dust legs are + * classified. Group assembly itself is covered by [CsvTradeGroupsTest]. + */ +class BinanceCsvMapperTest { + private val now = Clock.System.now() + private val strategy = BuiltInCsvStrategies.buildBinanceCsvStrategy(now) + + // Production seeds every currency at the same 18-decimal scale as crypto, which is what lets + // Binance's 8-decimal fiat amounts (e.g. "186.70374170" GBP) be represented exactly. + private val gbp = + Currency( + id = CurrencyId(1), + code = "GBP", + name = "Pound Sterling", + scaleFactor = CurrencyScaleFactors.DEFAULT_SCALE_FACTOR, + ) + private val bnb = CryptoAsset(id = CryptoId(2), code = "BNB", name = "BNB") + + private val binance = Account(id = AccountId(1), name = "Binance", openingDate = now) + + private val columns = + listOf("User_ID", "UTC_Time", "Account", "Operation", "Coin", "Change", "Remark") + .mapIndexed { index, name -> CsvColumn(CsvColumnId(Uuid.random()), index, name) } + + private fun mapper() = + CsvTransferMapper( + strategy = strategy, + columns = columns, + existingAccounts = mapOf(binance.name to binance), + existingCurrencies = mapOf(gbp.id to gbp), + existingCurrenciesByCode = mapOf(gbp.code to gbp), + existingCryptoByCode = mapOf(bnb.code to bnb), + ) + + private fun row( + operation: String, + coin: String, + change: String, + remark: String = "", + rowIndex: Long = 1, + time: String = "2023-01-02 03:04:05", + ) = CsvRow(rowIndex = rowIndex, values = listOf("53064551", time, "Spot", operation, coin, change, remark)) + + private fun map(row: CsvRow) = assertIs(mapper().mapRow(row), "mapping failed") + + /** The counterparty account name, whichever side of the transfer it landed on. */ + private fun counterpartyName(result: MappingResult.Success): String? = result.newAccounts.firstOrNull()?.name + + @Test + fun negativeChange_leavesTheBinanceAccount() { + val r = map(row("Simple Earn Flexible Subscription", "GBP", "-100.00", remark = "Binance Earn")) + assertEquals(binance.id, r.transfer.sourceAccountId, "a negative Change leaves Binance") + assertEquals("Binance Earn", counterpartyName(r)) + } + + @Test + fun positiveChange_flipsSoBinanceReceives() { + val r = map(row("Simple Earn Flexible Interest", "GBP", "0.12", remark = "Binance Earn")) + assertEquals(binance.id, r.transfer.targetAccountId, "a positive Change arrives into Binance") + assertEquals("Binance Earn Rewards", counterpartyName(r)) + } + + @Test + fun operationRouting_sendsEachProductToItsOwnAccount() { + val expected = + mapOf( + "Staking Purchase" to "Binance Staking", + "Staking Redemption" to "Binance Staking", + "Staking Rewards" to "Binance Staking Rewards", + "BNB Vault Rewards" to "Binance Vault Rewards", + "Launchpool Subscription/Redemption" to "Binance Launchpool", + "Launchpool Earnings Withdrawal" to "Binance Launchpool Rewards", + "Launchpool Interest" to "Binance Launchpool Rewards", + "Launchpad Subscribe" to "Binance Launchpad", + "Launchpad Token Distribution" to "Binance Launchpad", + "Distribution" to "Binance Distribution", + "Commission History" to "Binance Commission", + "Commission Rebate" to "Binance Commission", + "Liquid Swap Add" to "Binance Liquid Swap", + "Liquidity Farming Remove" to "Binance Liquid Swap", + "Dual Savings Purchase" to "Binance Dual Savings", + "Dual Savings Settlement" to "Binance Dual Savings", + "Simple Earn Locked Subscription" to "Binance Earn", + "Simple Earn Locked Rewards" to "Binance Earn Rewards", + "Simple Earn Flexible Airdrop" to "Binance Earn Rewards", + "Fee" to "Binance Fees", + "Transaction Fee" to "Binance Fees", + ) + for ((operation, account) in expected) { + assertEquals(account, counterpartyName(map(row(operation, "GBP", "-1.00"))), "routing for '$operation'") + } + } + + @Test + fun depositAndWithdrawal_splitFiatFromCryptoFunding() { + // The API books crypto funding against "Binance Funding" and fiat against "Binance Bank"; the + // CSV has to make the same split or the two sources' versions of one movement never reconcile. + assertEquals("Binance Funding", counterpartyName(map(row("Deposit", "BNB", "4.82096423")))) + assertEquals("Binance Funding", counterpartyName(map(row("Withdraw", "BNB", "-1.0")))) + assertEquals("Binance Bank", counterpartyName(map(row("Deposit", "GBP", "500.00")))) + assertEquals("Binance Bank", counterpartyName(map(row("Withdraw", "GBP", "-500.00")))) + assertEquals("Binance Bank", counterpartyName(map(row("Fiat Deposit", "GBP", "500.00")))) + assertEquals("Binance Bank", counterpartyName(map(row("Fiat Withdrawal", "GBP", "-500.00")))) + } + + @Test + fun patternsAreAnchored_soNoOperationSwallowsAnother() { + // RegexRule matching is containsMatchIn: an unanchored "Deposit" would also claim "Fiat Deposit" + // and an unanchored "Buy" would claim "Transaction Buy". + assertEquals("Binance Bank", counterpartyName(map(row("Fiat Deposit", "GBP", "1.00")))) + assertEquals(TradeLegSide.CREDIT, map(row("Transaction Buy", "GBP", "1.00")).tradeLeg?.side) + assertEquals(TradeLegSide.DEBIT, map(row("Transaction Sold", "GBP", "-1.00")).tradeLeg?.side) + } + + @Test + fun tradeLegs_areClassifiedBySignForTheAmbiguousOperation() { + // "Transaction Related" is the older name for *either* leg, so only the sign distinguishes them. + assertEquals(TradeLegSide.DEBIT, map(row("Transaction Related", "GBP", "-99.97")).tradeLeg?.side) + assertEquals(TradeLegSide.CREDIT, map(row("Transaction Related", "BNB", "0.008196")).tradeLeg?.side) + } + + @Test + fun feeRows_areNotTradeLegs() { + // A trade row carries no fee field, so fee rows stay out of the group and import as transfers. + assertNull(map(row("Fee", "BNB", "-0.00012823")).tradeLeg) + assertNull(map(row("Transaction Fee", "BNB", "-0.0010")).tradeLeg) + } + + @Test + fun nonTradeRows_haveNoTradeLeg() { + assertNull(map(row("Deposit", "GBP", "100.00")).tradeLeg) + assertNull(map(row("Staking Rewards", "GBP", "0.01")).tradeLeg) + } + + @Test + fun dustLegs_areClassifiedBySignBecauseBothShareOneOperation() { + val debit = map(row("Small Assets Exchange BNB (Spot)", "BNB", "-90.89657258")) + val credit = map(row("Small Assets Exchange BNB (Spot)", "BNB", "0.03251993")) + assertEquals(ConversionSide.DEBIT, debit.conversionLeg?.side) + assertEquals(ConversionSide.CREDIT, credit.conversionLeg?.side) + assertTrue( + debit.newAccounts.any { it.name == "Binance Conversions" }, + "both legs route through the shared conversion account", + ) + assertEquals(debit.conversionLeg?.pairingKey, credit.conversionLeg?.pairingKey) + } + + @Test + fun dustLegs_areNotAlsoTradeLegs() { + // A dust sweep's credits cannot be attributed to its debits, so it must never be assembled into + // a trade; it goes through ConversionConfig instead. + assertNull(map(row("Small Assets Exchange BNB (Spot)", "BNB", "-90.89657258")).tradeLeg) + } + + @Test + fun scientificNotationChangeParses() { + // Binance writes very small amounts as "2.5E-7"; the mapper must not choke or read them as zero. + // Crypto assets hold 18 decimals, so such a value is representable exactly. + val r = map(row("Staking Rewards", "BNB", "2.5E-7")) + assertNotNull(r.transfer) + assertEquals(binance.id, r.transfer.targetAccountId, "it is still a positive (incoming) amount") + } + + @Test + fun timestampIsParsedAsUtc() { + val r = map(row("Deposit", "GBP", "1.00", time = "2023-01-02 03:04:05")) + assertEquals("2023-01-02T03:04:05Z", r.transfer.timestamp.toString()) + } + + @Test + fun attributesCarryTheExportsOwnColumns() { + val r = map(row("Staking Rewards", "GBP", "0.01", remark = "STAKING")) + val attributes = r.attributes.toMap() + assertEquals("53064551", attributes["binance-user-id"]) + assertEquals("Staking Rewards", attributes["binance-operation"]) + assertEquals("STAKING", attributes["binance-remark"]) + } + + @Test + fun anUnknownOperationLandsInTheSuspenseAccountRatherThanOneNamedAfterIt() { + assertEquals("Binance Trading", counterpartyName(map(row("Some Future Product", "GBP", "-1.00")))) + } +} diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt new file mode 100644 index 000000000..1d30aa45a --- /dev/null +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt @@ -0,0 +1,226 @@ +package com.moneymanager.csvimporter + +import com.moneymanager.bigdecimal.BigDecimal +import com.moneymanager.domain.model.AccountId +import com.moneymanager.domain.model.Asset +import com.moneymanager.domain.model.CryptoAsset +import com.moneymanager.domain.model.CryptoId +import com.moneymanager.domain.model.Currency +import com.moneymanager.domain.model.CurrencyId +import com.moneymanager.domain.model.CurrencyScaleFactors +import com.moneymanager.domain.model.Money +import com.moneymanager.domain.model.Transfer +import com.moneymanager.domain.model.TransferId +import com.moneymanager.domain.model.csvstrategy.TradeGroupConfig +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.time.Instant + +/** + * Covers assembling a trade out of the several rows a source splits it across (see [TradeGroupConfig]). + * The cases are the real shapes a Binance export produces: a plain 1-fill swap, a many-fill order with + * unequal counts per side, and the shapes that must NOT assemble. + */ +class CsvTradeGroupsTest { + private val config = + TradeGroupConfig( + signalColumn = "Operation", + debitPattern = "^(Sell|Transaction (Spend|Sold))$", + creditPattern = "^(Buy|Transaction (Buy|Revenue))$", + sideAmountColumn = "Change", + groupingWindowSeconds = 0, + descriptionTemplate = "Buy {to}/{from}", + ) + + private val binance = AccountId(1) + private val trading = AccountId(2) + + // Production seeds every currency at the same 18-decimal scale as crypto, which is what lets + // Binance's 8-decimal fiat amounts (e.g. "186.70374170" GBP) be represented exactly. + private val gbp = + Currency( + id = CurrencyId(1), + code = "GBP", + name = "Pound Sterling", + scaleFactor = CurrencyScaleFactors.DEFAULT_SCALE_FACTOR, + ) + private val btc = CryptoAsset(id = CryptoId(10), code = "BTC", name = "Bitcoin") + private val eth = CryptoAsset(id = CryptoId(11), code = "ETH", name = "Ethereum") + + private var nextRowIndex = 0L + + private fun money( + display: String, + asset: Asset, + ) = Money.fromDisplayValue(BigDecimal(display), asset) + + /** + * A mapped leg as the strategy produces it: the amount is absolute and the direction lives in the + * accounts — a debit leaves the owner account, a credit arrives into it. + */ + private fun leg( + side: TradeLegSide, + display: String, + asset: Asset, + at: String = "2022-11-14T20:32:54Z", + owner: AccountId = binance, + ): CsvTransferWithAttributes { + val amount = money(display, asset) + val timestamp = Instant.parse(at) + return CsvTransferWithAttributes( + transfer = + Transfer( + id = TransferId(0), + timestamp = timestamp, + description = "leg", + sourceAccountId = if (side == TradeLegSide.DEBIT) owner else trading, + targetAccountId = if (side == TradeLegSide.DEBIT) trading else owner, + amount = amount, + ), + attributes = emptyList(), + rowIndex = nextRowIndex++, + tradeLeg = TradeLegInfo(side), + ) + } + + /** A row the strategy did not flag as a trade leg (a fee, a deposit). */ + private fun nonLeg(display: String = "0.001"): CsvTransferWithAttributes = + leg(TradeLegSide.DEBIT, display, btc).copy(tradeLeg = null) + + @Test + fun oneFillPerSide_assemblesASingleTrade() { + val rows = + listOf( + leg(TradeLegSide.DEBIT, "4.0", eth), + leg(TradeLegSide.CREDIT, "0.128228", btc), + ) + val groups = groupTradeLegs(rows, config) + assertEquals(1, groups.size) + val trade = assertNotNull(groups.single().assemble(config)) + assertEquals(binance, trade.ownerAccountId, "both legs of the trade sit on the owner account") + assertEquals(money("4.0", eth), trade.fromAmount) + assertEquals(money("0.128228", btc), trade.toAmount) + assertEquals("Buy BTC/ETH", trade.description) + } + + @Test + fun manyPartialFills_sumIntoOneTradeEvenWithUnequalCountsPerSide() { + // The real 2022-11-14 20:32:54 group: six BTC sold rows and six GBP revenue rows. The API + // reports the same event as six per-fill trades, so the CSV's totals must be their sums. + val btcFills = listOf("0.04382", "0.90716", "0.00441", "0.00312", "0.01351", "0.02798") + val gbpFills = + listOf("186.70374170", "43.12694880", "12536.54297800", "60.96551580", "605.57925400", "386.67128880") + val rows = + btcFills.map { leg(TradeLegSide.DEBIT, it, btc) } + gbpFills.map { leg(TradeLegSide.CREDIT, it, gbp) } + + val trade = assertNotNull(groupTradeLegs(rows, config).single().assemble(config)) + assertEquals(money("1.00000000", btc), trade.fromAmount, "the six BTC fills sum to exactly 1 BTC") + assertEquals(money("13819.5897271", gbp), trade.toAmount) + assertEquals(12, trade.group.rowIndexes.size, "every leg belongs to the group, for status write-back") + } + + @Test + fun distinctTimestamps_makeDistinctGroups() { + val rows = + listOf( + leg(TradeLegSide.DEBIT, "1.0", eth, at = "2022-11-14T20:32:54Z"), + leg(TradeLegSide.CREDIT, "0.03", btc, at = "2022-11-14T20:32:54Z"), + leg(TradeLegSide.DEBIT, "2.0", eth, at = "2022-11-14T20:39:53Z"), + leg(TradeLegSide.CREDIT, "0.06", btc, at = "2022-11-14T20:39:53Z"), + ) + val groups = groupTradeLegs(rows, config) + assertEquals(2, groups.size, "orders seconds apart are separate trades, not one aggregate") + assertEquals(money("1.0", eth), assertNotNull(groups[0].assemble(config)).fromAmount) + assertEquals(money("2.0", eth), assertNotNull(groups[1].assemble(config)).fromAmount) + } + + @Test + fun aGroupingWindow_holdsTogetherLegsThatStraddleASecondBoundary() { + val windowed = config.copy(groupingWindowSeconds = 2) + val rows = + listOf( + leg(TradeLegSide.DEBIT, "1.0", eth, at = "2021-01-01T09:43:33Z"), + leg(TradeLegSide.CREDIT, "0.03", btc, at = "2021-01-01T09:43:34Z"), + ) + assertEquals(1, groupTradeLegs(rows, windowed).size) + assertEquals(2, groupTradeLegs(rows, config).size, "with a zero window the same rows are two groups") + } + + @Test + fun theTradeTakesTheGroupsEarliestTimestamp() { + val windowed = config.copy(groupingWindowSeconds = 2) + val rows = + listOf( + leg(TradeLegSide.CREDIT, "0.03", btc, at = "2021-01-01T09:43:34Z"), + leg(TradeLegSide.DEBIT, "1.0", eth, at = "2021-01-01T09:43:33Z"), + ) + val trade = assertNotNull(groupTradeLegs(rows, windowed).single().assemble(windowed)) + assertEquals(Instant.parse("2021-01-01T09:43:33Z"), trade.timestamp) + } + + @Test + fun aOneSidedGroupDoesNotAssemble() { + // A boundary spill or a truncated export: the rows stay ordinary transfers rather than becoming + // a trade with an invented other side. + val rows = listOf(leg(TradeLegSide.DEBIT, "1.0", eth)) + assertNull(groupTradeLegs(rows, config).single().assemble(config)) + } + + @Test + fun aGroupWithTwoAssetsOnOneSideDoesNotAssemble() { + // Guards the property the assembly relies on: a real trade group names exactly one asset per + // side. Anything else cannot be folded into one trade without inventing a pairing. + val rows = + listOf( + leg(TradeLegSide.DEBIT, "1.0", eth), + leg(TradeLegSide.DEBIT, "0.5", btc), + leg(TradeLegSide.CREDIT, "100.0", gbp), + ) + assertNull(groupTradeLegs(rows, config).single().assemble(config)) + } + + @Test + fun aGroupWhoseSidesNameTheSameAssetDoesNotAssemble() { + val rows = + listOf( + leg(TradeLegSide.DEBIT, "1.0", eth), + leg(TradeLegSide.CREDIT, "1.0", eth), + ) + assertNull(groupTradeLegs(rows, config).single().assemble(config)) + } + + @Test + fun aGroupSummingToZeroDoesNotAssemble() { + // Binance writes vanishing amounts as "0E-8"; a group of nothing but those is not a trade. + val rows = + listOf( + leg(TradeLegSide.DEBIT, "0E-8", eth), + leg(TradeLegSide.CREDIT, "0E-8", btc), + ) + assertNull(groupTradeLegs(rows, config).single().assemble(config)) + } + + @Test + fun legsDisagreeingAboutTheOwnerAccountDoNotAssemble() { + val rows = + listOf( + leg(TradeLegSide.DEBIT, "1.0", eth, owner = binance), + leg(TradeLegSide.CREDIT, "0.03", btc, owner = AccountId(99)), + ) + assertNull(groupTradeLegs(rows, config).single().assemble(config)) + } + + @Test + fun rowsThatAreNotTradeLegsAreIgnoredEntirely() { + val rows = listOf(nonLeg(), leg(TradeLegSide.DEBIT, "1.0", eth), leg(TradeLegSide.CREDIT, "0.03", btc), nonLeg()) + val group = groupTradeLegs(rows, config).single() + assertEquals(2, group.rows.size, "fee and other rows stay out of the group and import as transfers") + } + + @Test + fun noLegsMeansNoGroups() { + assertEquals(emptyList(), groupTradeLegs(listOf(nonLeg(), nonLeg()), config)) + } +} diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt index 7561aacf9..b9721ac93 100644 --- a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt @@ -281,4 +281,53 @@ class StrategySelectorTest { val selected = builtIns.selectForCsv("transaction-history.csv", wiseColumns, listOf(wiseRow)) assertEquals("Wise CSV", selected?.name) } + + private val binanceHeaders = listOf("User_ID", "UTC_Time", "Account", "Operation", "Coin", "Change", "Remark") + + private fun binanceColumns(headers: List) = + headers.mapIndexed { i, name -> CsvColumn(CsvColumnId(Uuid.random()), i, name) } + + private fun binanceRow( + index: Long, + values: List, + ) = CsvRow(rowIndex = index, values = values) + + @Test + fun `a modern Binance export selects the Binance CSV strategy`() { + val builtIns = BuiltInCsvStrategies.builtInCsvStrategies(Clock.System.now()) + val rows = + List(4) { i -> + binanceRow( + i.toLong(), + listOf("53064551", "2021-01-01 00:58:16", "Spot", "BNB Vault Rewards", "ASR", "0.00294372", ""), + ) + } + // Binance names its exports with bare UUIDs, so only the columns + content can identify one. + val selected = builtIns.selectForCsv("bc9a136a-8713-11ee-8edb-06655da838d5-1.csv", binanceColumns(binanceHeaders), rows) + assertEquals("Binance CSV", selected?.name) + } + + @Test + fun `a legacy Binance export without User_ID selects no strategy`() { + val builtIns = BuiltInCsvStrategies.builtInCsvStrategies(Clock.System.now()) + val legacyHeaders = binanceHeaders.drop(1) + val rows = + List(4) { i -> + binanceRow(i.toLong(), listOf("2020-10-11 09:31:32", "Spot", "Savings purchase", "BNB", "-4.82096423", "")) + } + // The legacy columns are a strict subset of the modern ones, so the tolerant fallback does make + // the Binance strategy a candidate - the content rule on User_ID is what rejects it. Legacy + // exports use a different Operation vocabulary and would double-book every event. + assertNull(builtIns.selectForCsv("20210424.csv", binanceColumns(legacyHeaders), rows)) + } + + @Test + fun `a Monzo export still selects the Monzo CSV strategy alongside Binance`() { + val builtIns = BuiltInCsvStrategies.builtInCsvStrategies(Clock.System.now()) + val monzo = builtIns.single { it.name == "Monzo CSV" } + val monzoColumns = + monzo.identificationColumns.toList().mapIndexed { i, name -> CsvColumn(CsvColumnId(Uuid.random()), i, name) } + val monzoRow = CsvRow(rowIndex = 1L, values = monzo.identificationColumns.map { "" }) + assertEquals("Monzo CSV", builtIns.selectForCsv("monzo.csv", monzoColumns, listOf(monzoRow))?.name) + } } diff --git a/app/db/core/src/commonMain/kotlin/com/moneymanager/database/service/CsvStrategyExportService.kt b/app/db/core/src/commonMain/kotlin/com/moneymanager/database/service/CsvStrategyExportService.kt index 431acc375..4beadd0a3 100644 --- a/app/db/core/src/commonMain/kotlin/com/moneymanager/database/service/CsvStrategyExportService.kt +++ b/app/db/core/src/commonMain/kotlin/com/moneymanager/database/service/CsvStrategyExportService.kt @@ -416,6 +416,7 @@ class CsvStrategyExportService( fileNamePattern = export.fileNamePattern, crossSourceReconcileWindowSeconds = export.crossSourceReconcileWindowSeconds, conversionConfig = export.conversionConfig, + tradeGroupConfig = export.tradeGroupConfig, fundingAttributeMatch = export.fundingAttributeMatch, worksheetName = export.worksheetName, createdAt = now, diff --git a/app/db/read/src/commonMain/kotlin/com/moneymanager/database/json/FieldMappingJsonCodec.kt b/app/db/read/src/commonMain/kotlin/com/moneymanager/database/json/FieldMappingJsonCodec.kt index cbf0b1695..4c8b03ec3 100644 --- a/app/db/read/src/commonMain/kotlin/com/moneymanager/database/json/FieldMappingJsonCodec.kt +++ b/app/db/read/src/commonMain/kotlin/com/moneymanager/database/json/FieldMappingJsonCodec.kt @@ -7,6 +7,7 @@ import com.moneymanager.domain.model.csvstrategy.ContentMatchRule import com.moneymanager.domain.model.csvstrategy.ConversionConfig import com.moneymanager.domain.model.csvstrategy.FieldMapping import com.moneymanager.domain.model.csvstrategy.RowPreprocessingRule +import com.moneymanager.domain.model.csvstrategy.TradeGroupConfig import com.moneymanager.domain.model.csvstrategy.TransferField import com.moneymanager.domain.serialization.UuidSerializersModule import kotlinx.serialization.json.Json @@ -52,6 +53,10 @@ object FieldMappingJsonCodec { fun decodeConversionConfig(jsonString: String?): ConversionConfig? = jsonString?.let { json.decodeFromString(it) } + fun encodeTradeGroupConfig(config: TradeGroupConfig?): String? = config?.let { json.encodeToString(it) } + + fun decodeTradeGroupConfig(jsonString: String?): TradeGroupConfig? = jsonString?.let { json.decodeFromString(it) } + fun encodeAttributeAccountMatch(match: AttributeAccountMatch?): String? = match?.let { json.encodeToString(it) } fun decodeAttributeAccountMatch(jsonString: String?): AttributeAccountMatch? = jsonString?.let { json.decodeFromString(it) } diff --git a/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/CsvImportStrategyReadRepositoryImpl.kt b/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/CsvImportStrategyReadRepositoryImpl.kt index 429ac8821..e2deb6d3d 100644 --- a/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/CsvImportStrategyReadRepositoryImpl.kt +++ b/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/CsvImportStrategyReadRepositoryImpl.kt @@ -54,6 +54,7 @@ class CsvImportStrategyReadRepositoryImpl( fileNamePattern: String?, crossSourceReconcileWindowSeconds: Long?, conversionConfigJson: String?, + tradeGroupConfigJson: String?, fundingAttributeMatchJson: String?, createdAt: Long, updatedAt: Long, @@ -71,6 +72,7 @@ class CsvImportStrategyReadRepositoryImpl( fileNamePattern = fileNamePattern, crossSourceReconcileWindowSeconds = crossSourceReconcileWindowSeconds, conversionConfig = FieldMappingJsonCodec.decodeConversionConfig(conversionConfigJson), + tradeGroupConfig = FieldMappingJsonCodec.decodeTradeGroupConfig(tradeGroupConfigJson), fundingAttributeMatch = FieldMappingJsonCodec.decodeAttributeAccountMatch(fundingAttributeMatchJson), worksheetName = worksheetName, createdAt = Instant.fromEpochMilliseconds(createdAt), diff --git a/app/db/schema/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategy.sq b/app/db/schema/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategy.sq index 65ad101f4..7e8516006 100644 --- a/app/db/schema/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategy.sq +++ b/app/db/schema/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategy.sq @@ -12,6 +12,7 @@ CREATE TABLE csv_import_strategy ( file_name_pattern TEXT, cross_source_reconcile_window_seconds INTEGER, conversion_config_json TEXT, + trade_group_config_json TEXT, funding_attribute_match_json TEXT, created_at INTEGER NOT NULL DEFAULT (CAST(strftime('%s', 'now') AS INTEGER) * 1000), updated_at INTEGER NOT NULL DEFAULT (CAST(strftime('%s', 'now') AS INTEGER) * 1000) diff --git a/app/db/write/src/commonMain/kotlin/com/moneymanager/database/repository/write/CsvImportStrategyWriteRepositoryImpl.kt b/app/db/write/src/commonMain/kotlin/com/moneymanager/database/repository/write/CsvImportStrategyWriteRepositoryImpl.kt index 3cea90e6a..ddc4860c0 100644 --- a/app/db/write/src/commonMain/kotlin/com/moneymanager/database/repository/write/CsvImportStrategyWriteRepositoryImpl.kt +++ b/app/db/write/src/commonMain/kotlin/com/moneymanager/database/repository/write/CsvImportStrategyWriteRepositoryImpl.kt @@ -62,6 +62,7 @@ class CsvImportStrategyWriteRepositoryImpl( file_name_pattern = strategy.fileNamePattern, cross_source_reconcile_window_seconds = strategy.crossSourceReconcileWindowSeconds, conversion_config_json = FieldMappingJsonCodec.encodeConversionConfig(strategy.conversionConfig), + trade_group_config_json = FieldMappingJsonCodec.encodeTradeGroupConfig(strategy.tradeGroupConfig), funding_attribute_match_json = FieldMappingJsonCodec.encodeAttributeAccountMatch(strategy.fundingAttributeMatch), updated_at = now.toEpochMilliseconds(), id = strategy.id.id.toString(), diff --git a/app/db/write/src/commonMain/kotlin/com/moneymanager/database/write/CsvImportStrategyInsert.kt b/app/db/write/src/commonMain/kotlin/com/moneymanager/database/write/CsvImportStrategyInsert.kt index 65d53f4a0..0b7211e0e 100644 --- a/app/db/write/src/commonMain/kotlin/com/moneymanager/database/write/CsvImportStrategyInsert.kt +++ b/app/db/write/src/commonMain/kotlin/com/moneymanager/database/write/CsvImportStrategyInsert.kt @@ -23,6 +23,7 @@ fun CsvImportStrategyWriteQueries.insertStrategy(strategy: CsvImportStrategy) { file_name_pattern = strategy.fileNamePattern, cross_source_reconcile_window_seconds = strategy.crossSourceReconcileWindowSeconds, conversion_config_json = FieldMappingJsonCodec.encodeConversionConfig(strategy.conversionConfig), + trade_group_config_json = FieldMappingJsonCodec.encodeTradeGroupConfig(strategy.tradeGroupConfig), funding_attribute_match_json = FieldMappingJsonCodec.encodeAttributeAccountMatch(strategy.fundingAttributeMatch), ) strategy.worksheetName?.let { worksheetName -> diff --git a/app/db/write/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategyWrite.sq b/app/db/write/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategyWrite.sq index 209f6f787..f394b408e 100644 --- a/app/db/write/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategyWrite.sq +++ b/app/db/write/src/commonMain/sqldelight/com/moneymanager/database/sql/csvImportStrategy/CsvImportStrategyWrite.sq @@ -1,12 +1,12 @@ -- Insert a new strategy. created_at/updated_at are filled by their column DEFAULTs (current time). insert: -INSERT INTO csv_import_strategy (id, name, identification_columns_json, field_mappings_json, attribute_mappings_json, row_rules_json, companion_rules_json, content_match_rules_json, file_name_pattern, cross_source_reconcile_window_seconds, conversion_config_json, funding_attribute_match_json) -VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); +INSERT INTO csv_import_strategy (id, name, identification_columns_json, field_mappings_json, attribute_mappings_json, row_rules_json, companion_rules_json, content_match_rules_json, file_name_pattern, cross_source_reconcile_window_seconds, conversion_config_json, trade_group_config_json, funding_attribute_match_json) +VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?); -- Update an existing strategy update: UPDATE csv_import_strategy -SET revision_id = revision_id + 1, name = ?, identification_columns_json = ?, field_mappings_json = ?, attribute_mappings_json = ?, row_rules_json = ?, companion_rules_json = ?, content_match_rules_json = ?, file_name_pattern = ?, cross_source_reconcile_window_seconds = ?, conversion_config_json = ?, funding_attribute_match_json = ?, updated_at = ? +SET revision_id = revision_id + 1, name = ?, identification_columns_json = ?, field_mappings_json = ?, attribute_mappings_json = ?, row_rules_json = ?, companion_rules_json = ?, content_match_rules_json = ?, file_name_pattern = ?, cross_source_reconcile_window_seconds = ?, conversion_config_json = ?, trade_group_config_json = ?, funding_attribute_match_json = ?, updated_at = ? WHERE id = ?; -- Delete a strategy by ID diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt index 7e4b9759f..1094772f2 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt @@ -1,5 +1,6 @@ package com.moneymanager.domain.model.csvstrategy +import kotlinx.serialization.EncodeDefault import kotlinx.serialization.Serializable /** @@ -41,6 +42,13 @@ import kotlinx.serialization.Serializable * distinct events (typically far apart in time) separate. * @property relationshipTypeName Relationship type name linking each debit leg to its credit leg * (resolved get-or-create, so already-populated databases self-heal). + * @property sideAmountColumn Optional column whose sign decides the side, for sources that give both + * legs of a conversion the **same** [signalColumn] value (Binance's + * "Small Assets Exchange BNB" names the swept asset and the BNB received + * identically). When set, a row matching [debitPattern] or [creditPattern] + * is a DEBIT if this column parses negative and a CREDIT if positive; a + * row that parses to zero or unparseably is not a conversion leg. When + * null the patterns alone decide, as before. */ @Serializable data class ConversionConfig( @@ -56,6 +64,10 @@ data class ConversionConfig( val pairingKeyColumns: List = emptyList(), val pairingWindowSeconds: Long, val relationshipTypeName: String, + // Omitted from JSON when null (@EncodeDefault NEVER on the export field) so adding this does not + // change the canonical hash of every existing strategy - only one that actually sets it rehashes. + @EncodeDefault(EncodeDefault.Mode.NEVER) + val sideAmountColumn: String? = null, ) { init { require(conversionAccountName != null || conversionAccountRules.isNotEmpty()) { diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/CsvImportStrategy.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/CsvImportStrategy.kt index d778c6d99..4a5a09194 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/CsvImportStrategy.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/CsvImportStrategy.kt @@ -66,6 +66,10 @@ data class AttributeAccountMatch( * separate debited/credited rows; the importer routes the legs through a * shared counterparty account and links each debit to its credit (see * [ConversionConfig]). Null when the source has no such conversions. + * @property tradeGroupConfig When set, describes how this source splits one trade across several rows + * sharing a timestamp; the importer assembles each such group into a single + * `trade` on the owner account (see [TradeGroupConfig]). Null when every + * cross-asset movement already arrives on one row. * @property fundingAttributeMatch When set, resolves each row's hidden funding account by matching a * CSV column against an account-attribute type (see [AttributeAccountMatch]; * e.g. Curve's "Funding Card Last 4 Digits" column against the `card-last4` @@ -93,6 +97,7 @@ data class CsvImportStrategy( val fileNamePattern: String? = null, val crossSourceReconcileWindowSeconds: Long? = null, val conversionConfig: ConversionConfig? = null, + val tradeGroupConfig: TradeGroupConfig? = null, val fundingAttributeMatch: AttributeAccountMatch? = null, val worksheetName: String? = null, val createdAt: Instant, diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt new file mode 100644 index 000000000..9303c8716 --- /dev/null +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt @@ -0,0 +1,62 @@ +package com.moneymanager.domain.model.csvstrategy + +import kotlinx.serialization.Serializable + +/** + * Declares how a CSV source expresses a **trade** that arrives as several separate rows sharing one + * timestamp — one row per partial fill per leg — rather than as a single cross-asset row. + * + * This is the trade-shaped sibling of [ConversionConfig], and a strategy may declare both. They solve + * different problems: + * - [ConversionConfig] handles conversions whose legs cannot be attributed to each other (a + * many-assets-in / one-asset-out dust sweep, where no column says which credited amount came from + * which debited asset). It keeps every leg a single-asset transfer routed through a shared + * conversion account, so balances stay exact without inventing a pairing. + * - [TradeGroupConfig] handles the case where a group's legs *are* attributable: every debit row in + * the group names one asset and every credit row names one other asset, so their sums are exactly + * the two sides of one conversion and can be booked as a real `trade` on the owner account. + * + * When set on a [CsvImportStrategy], the importer buckets matching rows by timestamp (widened by + * [groupingWindowSeconds]), and for each bucket whose debits name exactly one asset and whose credits + * name exactly one other asset emits a single trade — owner account on both sides, debit sum out, + * credit sum in. Fee rows in the bucket become their own transfers to [feeAccountName], because a + * `trade` row carries no fee field. A bucket that does not resolve — no credits, an empty side, or + * more than one asset on a side — is left alone and its rows import as ordinary transfers to whatever + * account the strategy's mappings chose, so no row is ever dropped and the residue is visible. + * + * A `trade` row carries no fee field, so fee rows are deliberately **not** part of this config: leave + * them out of both patterns and let the strategy's ordinary account routing book them as their own + * transfer to a fee account (naming the account another source already uses keeps one balance). + * + * Both patterns are matched with [Regex.containsMatchIn], so anchor them (`^…$`) unless a prefix + * match is genuinely wanted — an unanchored `Buy` would also claim `Transaction Buy`. + * + * @property signalColumn Column examined to classify a row as a trade leg (e.g. "Operation"). + * @property debitPattern Regex identifying a DEBIT leg — the asset leaving the owner account. + * @property creditPattern Regex identifying a CREDIT leg — the asset received into the owner account. + * @property sideAmountColumn Optional column whose sign decides the side, for sources that use one + * [signalColumn] value on both legs (Binance labels both sides of an older + * fill `Transaction Related`). When set, the two patterns together only say + * which rows are legs at all, and a leg is a DEBIT when this column parses + * negative and a CREDIT when positive; a row parsing to zero or unparseably + * is not a leg. When null the patterns alone decide, debit tested first. + * @property groupingWindowSeconds Seconds of timestamp jitter tolerated between consecutive rows of + * one group. Zero when the source stamps every leg of a fill with the + * identical time. + * @property descriptionTemplate Description given to the assembled trade. `{from}` and `{to}` are + * substituted with the debited and credited asset codes. Cosmetic only: + * a trade's identity never includes its description. + */ +@Serializable +data class TradeGroupConfig( + val signalColumn: String, + val debitPattern: String, + val creditPattern: String, + val sideAmountColumn: String? = null, + val groupingWindowSeconds: Long = 0, + val descriptionTemplate: String = "Buy {to}/{from}", +) { + init { + require(groupingWindowSeconds >= 0) { "TradeGroupConfig.groupingWindowSeconds must not be negative" } + } +} diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExport.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExport.kt index f13544ea7..870351869 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExport.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExport.kt @@ -15,6 +15,7 @@ import com.moneymanager.domain.model.csvstrategy.RowPreprocessingRule import com.moneymanager.domain.model.csvstrategy.SortedCompanionTransactionRuleListSerializer import com.moneymanager.domain.model.csvstrategy.SortedContentMatchRuleListSerializer import com.moneymanager.domain.model.csvstrategy.SortedRowConditionListSerializer +import com.moneymanager.domain.model.csvstrategy.TradeGroupConfig import com.moneymanager.domain.model.csvstrategy.TransferField import com.moneymanager.domain.model.serialization.SortedStringSetSerializer import kotlinx.serialization.EncodeDefault @@ -43,6 +44,8 @@ import kotlinx.serialization.Serializable * (see [com.moneymanager.domain.model.csvstrategy.CsvImportStrategy.fundingAttributeMatch]) * @property worksheetName When set, this is an Excel strategy targeting this worksheet * (see [com.moneymanager.domain.model.csvstrategy.CsvImportStrategy.worksheetName]) + * @property tradeGroupConfig Row-group trade assembly configuration (already portable, no IDs) + * (see [com.moneymanager.domain.model.csvstrategy.CsvImportStrategy.tradeGroupConfig]) */ @Serializable data class CsvStrategyExport( @@ -79,6 +82,9 @@ data class CsvStrategyExport( // worksheet name (XLSX strategies) rehash when this field is added. @EncodeDefault(EncodeDefault.Mode.NEVER) val worksheetName: String? = null, + // Same NEVER-encode rationale again: only a strategy that assembles trades from row groups rehashes. + @EncodeDefault(EncodeDefault.Mode.NEVER) + val tradeGroupConfig: TradeGroupConfig? = null, ) /** diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExportMapper.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExportMapper.kt index ccd3973dd..a74f0b85a 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExportMapper.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/export/CsvStrategyExportMapper.kt @@ -52,6 +52,7 @@ object CsvStrategyExportMapper { conversionConfig = strategy.conversionConfig, fundingAttributeMatch = strategy.fundingAttributeMatch, worksheetName = strategy.worksheetName, + tradeGroupConfig = strategy.tradeGroupConfig, ) private fun FieldMapping.toExport( diff --git a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt index 2014a3d98..1a5703e77 100644 --- a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt +++ b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt @@ -27,6 +27,7 @@ import com.moneymanager.domain.model.csvstrategy.RowConditionOperator import com.moneymanager.domain.model.csvstrategy.RowPreprocessingRule import com.moneymanager.domain.model.csvstrategy.TemplateAccountMapping import com.moneymanager.domain.model.csvstrategy.TransferField +import com.moneymanager.domain.model.csvstrategy.TradeGroupConfig import com.moneymanager.domain.model.qif.QifColumns import kotlin.time.Instant import kotlin.uuid.Uuid @@ -43,6 +44,7 @@ object BuiltInCsvStrategies { val cryptoComCryptoStrategyId: Uuid = Uuid.parse("00000000-0000-0000-0000-000000000009") val curveCsvStrategyId: Uuid = Uuid.parse("00000000-0000-0000-0000-00000000000a") val cryptoComCardXlsxStrategyId: Uuid = Uuid.parse("00000000-0000-0000-0000-00000000000b") + val binanceCsvStrategyId: Uuid = Uuid.parse("00000000-0000-0000-0000-00000000000c") /** Fixed account names shared by the crypto.com Card and Fiat strategies, so both files resolve the same accounts. */ private const val CRYPTO_COM_CARD_ACCOUNT = "Crypto.com Card" @@ -128,6 +130,72 @@ object BuiltInCsvStrategies { */ private const val CURVE_RECONCILE_WINDOW_SECONDS = 172_800L + /** + * The single Binance Spot account, holding one balance per asset. Matches the Binance API + * strategy's synthetic account name, so a movement both sources record resolves to the same + * account on both sides and cross-source reconciliation links it instead of double-counting. + */ + private const val BINANCE_ACCOUNT = "Binance" + + /** + * Binance counterparty accounts the API strategy also creates (`ApiDataEndpoint`'s + * `counterpartyAccountName`). Naming them identically is what makes the CSV's and the API's + * versions of the same deposit/withdrawal/earn movement reconcilable. + */ + private const val BINANCE_FEES_ACCOUNT = "Binance Fees" + private const val BINANCE_FUNDING_ACCOUNT = "Binance Funding" + private const val BINANCE_BANK_ACCOUNT = "Binance Bank" + private const val BINANCE_EARN_ACCOUNT = "Binance Earn" + private const val BINANCE_EARN_REWARDS_ACCOUNT = "Binance Earn Rewards" + private const val BINANCE_DISTRIBUTION_ACCOUNT = "Binance Distribution" + + /** + * Binance counterparty accounts with no API equivalent — Binance publishes no endpoint for staking, + * BNB Vault, Launchpool, Launchpad, commission, liquidity farming or dual savings, so these + * products exist only in the CSV export. Principal pools are kept apart from the income they throw + * off (and from [BINANCE_EARN_ACCOUNT]) so each balance answers one question: what is staked, and + * what has it paid out. + */ + private const val BINANCE_STAKING_ACCOUNT = "Binance Staking" + private const val BINANCE_STAKING_REWARDS_ACCOUNT = "Binance Staking Rewards" + private const val BINANCE_VAULT_REWARDS_ACCOUNT = "Binance Vault Rewards" + private const val BINANCE_LAUNCHPOOL_ACCOUNT = "Binance Launchpool" + private const val BINANCE_LAUNCHPOOL_REWARDS_ACCOUNT = "Binance Launchpool Rewards" + private const val BINANCE_LAUNCHPAD_ACCOUNT = "Binance Launchpad" + private const val BINANCE_COMMISSION_ACCOUNT = "Binance Commission" + private const val BINANCE_LIQUID_SWAP_ACCOUNT = "Binance Liquid Swap" + private const val BINANCE_DUAL_SAVINGS_ACCOUNT = "Binance Dual Savings" + + /** + * Counterparty for Binance's small-assets (dust) sweeps, which arrive as several debited rows and + * several credited BNB rows that no column attributes to each other. Routing every leg through one + * account keeps each asset balance exact and isolates the mixed-asset residual, exactly as + * [CRYPTO_COM_CONVERSIONS_ACCOUNT] does. See [ConversionConfig]. + */ + private const val BINANCE_CONVERSIONS_ACCOUNT = "Binance Conversions" + + /** + * Suspense counterparty for a trade leg whose group did not resolve into a trade — a one-sided + * group, or one naming more than one asset on a side. Reached only in that case (an assembled + * group's legs become the trade and never a transfer), so a non-zero balance here is a visible + * signal that an export had a shape the strategy does not model, rather than a silent loss. + */ + private const val BINANCE_TRADING_ACCOUNT = "Binance Trading" + + /** + * Cross-source reconciliation window for the Binance CSV strategy. The CSV and the API describe the + * same event with the same UTC second and differ only in sub-second precision, so this needs to be + * small; and Binance pays `Staking Rewards` in the same amount every day, so a wide window would + * start pairing genuinely distinct reward rows. Five minutes is far below that daily cadence. + */ + private const val BINANCE_RECONCILE_WINDOW_SECONDS = 300L + + /** + * Window for pairing a dust sweep's debit legs to its credit legs. Binance stamps a sweep's rows + * within the same second or the next one, while distinct sweeps are hours apart. + */ + private const val BINANCE_CONVERSION_PAIRING_WINDOW_SECONDS = 2L + /** * All built-in CSV import strategies seeded into a fresh database. [qifCurrencyId] is the fixed * currency the QIF strategies carry (the default the QIF import dialog pre-selects); it is resolved @@ -147,6 +215,7 @@ object BuiltInCsvStrategies { buildCryptoComCryptoStrategy(now), buildCurveCsvStrategy(now), buildCryptoComCardXlsxStrategy(now), + buildBinanceCsvStrategy(now), ) /** @@ -1426,4 +1495,190 @@ object BuiltInCsvStrategies { updatedAt = now, ) } + + /** + * Built-in strategy for Binance's "transaction history" export — one signed row per movement: + * `User_ID, UTC_Time, Account, Operation, Coin, Change, Remark`. + * + * **Modern exports only.** Binance's pre-2022 exports carry the same columns minus `User_ID`, and + * an older `Operation` vocabulary (`Savings purchase` for `Simple Earn Flexible Subscription`, + * `POS savings interest` for `Staking Rewards`, `Super BNB Mining` for `BNB Vault Rewards`, …) plus + * `LD*` mirror rows the modern format dropped. Importing both would book the same event twice under + * two different descriptions, so [contentMatchRules] requires the `User_ID` column: a legacy file + * scores zero and, because this strategy carries content rules, is also excluded from the + * no-signals fallback, so it resolves to no strategy and is reported skipped rather than misread. + * Re-export the same period from Binance to import it. + * + * `Operation` drives everything. Deposits, withdrawals, Earn subscriptions and rewards route to the + * accounts the Binance API strategy also creates, so whichever source imports second reconciles + * against the first. Staking, BNB Vault, Launchpool, Launchpad, commission, liquidity farming and + * dual savings have no API endpoint at all and are the reason to import this file. + * + * Trades are split across rows: Binance stamps every partial fill of both legs with the same second + * (a single order can produce a dozen `Transaction Sold`/`Transaction Revenue` rows). [tradeGroupConfig] + * folds each such group into one `trade`. Fee rows stay out of the group on purpose — a `trade` row + * has no fee field — and route to [BINANCE_FEES_ACCOUNT] as their own transfers, as the API does. + * + * Dust sweeps are the one conversion that cannot be assembled: a sweep debits several assets and + * credits several BNB amounts, and nothing in the file says which credit came from which debit + * (their order does not correspond, and the credited amount is net of Binance's service charge + * while the debited amount is gross). They go through [conversionConfig] instead, which keeps every + * balance exact without inventing a pairing. Both legs share one `Operation`, so + * [ConversionConfig.sideAmountColumn] classifies them by the sign of `Change`. + */ + @Suppress("LongMethod") + fun buildBinanceCsvStrategy(now: Instant): CsvImportStrategy { + // Every pattern is anchored: RegexRule matching is containsMatchIn, so a bare "Deposit" would + // also claim "Fiat Deposit" and a bare "Buy" would claim "Transaction Buy". + val targetAccountRules = + listOf( + // Fiat funding is the API's fiat/orders endpoints (Binance Bank); crypto funding is + // capital/deposit|withdraw (Binance Funding). The Coin column decides which, so the + // fiat rules match on it and the crypto rules catch the rest. + RegexRule(pattern = "^(Fiat Deposit|Fiat Withdrawal)$", accountName = BINANCE_BANK_ACCOUNT), + RegexRule(pattern = "^(Deposit|Withdraw)$", accountName = BINANCE_FUNDING_ACCOUNT), + RegexRule( + pattern = "^Simple Earn (Flexible|Locked) (Subscription|Redemption)$", + accountName = BINANCE_EARN_ACCOUNT, + ), + RegexRule( + pattern = "^Simple Earn (Flexible (Interest|Airdrop)|Locked Rewards)$", + accountName = BINANCE_EARN_REWARDS_ACCOUNT, + ), + RegexRule(pattern = "^Staking (Purchase|Redemption)$", accountName = BINANCE_STAKING_ACCOUNT), + RegexRule(pattern = "^Staking Rewards$", accountName = BINANCE_STAKING_REWARDS_ACCOUNT), + RegexRule(pattern = "^BNB Vault Rewards$", accountName = BINANCE_VAULT_REWARDS_ACCOUNT), + RegexRule( + pattern = "^Launchpool Subscription/Redemption$", + accountName = BINANCE_LAUNCHPOOL_ACCOUNT, + ), + RegexRule( + pattern = "^Launchpool (Interest|Earnings Withdrawal)$", + accountName = BINANCE_LAUNCHPOOL_REWARDS_ACCOUNT, + ), + RegexRule( + pattern = "^Launchpad (Subscribe|Token Distribution)$", + accountName = BINANCE_LAUNCHPAD_ACCOUNT, + ), + RegexRule( + pattern = "^(Distribution|Rewards Distribution)$", + accountName = BINANCE_DISTRIBUTION_ACCOUNT, + ), + RegexRule(pattern = "^Commission (History|Rebate)$", accountName = BINANCE_COMMISSION_ACCOUNT), + RegexRule( + pattern = "^(Liquid Swap Add|Liquidity Farming Remove)$", + accountName = BINANCE_LIQUID_SWAP_ACCOUNT, + ), + RegexRule( + pattern = "^Dual Savings (Purchase|Settlement)$", + accountName = BINANCE_DUAL_SAVINGS_ACCOUNT, + ), + RegexRule(pattern = "^(Fee|Transaction Fee)$", accountName = BINANCE_FEES_ACCOUNT), + // Dust legs are re-routed to the conversion account by conversionConfig; this rule is + // the home for a leg that somehow escapes detection (a zero Change). + RegexRule( + pattern = "^Small Assets Exchange BNB( \\(Spot\\))?$", + accountName = BINANCE_CONVERSIONS_ACCOUNT, + ), + // Trade legs only reach here when their group did not resolve; and the trailing + // catch-all keeps a future unknown Operation from minting an account named after it. + RegexRule(pattern = "^", accountName = BINANCE_TRADING_ACCOUNT), + ) + val fieldMappings = + mapOf( + // Account is "Spot" on every row; keying the rule off it rather than hard-coding an id + // keeps the mapping portable and leaves room for a future wallet column value. + TransferField.SOURCE_ACCOUNT to + RegexAccountMapping( + fieldType = TransferField.SOURCE_ACCOUNT, + columnName = "Account", + rules = listOf(RegexRule(pattern = "^", accountName = BINANCE_ACCOUNT)), + ), + TransferField.TARGET_ACCOUNT to + ConditionalAccountMapping( + fieldType = TransferField.TARGET_ACCOUNT, + conditions = listOf(RowCondition("Coin", RowConditionOperator.EQUALS_VALUE, value = "GBP")), + whenTrue = + RegexAccountMapping( + fieldType = TransferField.TARGET_ACCOUNT, + columnName = "Operation", + rules = + listOf( + RegexRule(pattern = "^(Deposit|Withdraw)$", accountName = BINANCE_BANK_ACCOUNT), + ) + targetAccountRules, + ), + whenFalse = + RegexAccountMapping( + fieldType = TransferField.TARGET_ACCOUNT, + columnName = "Operation", + rules = targetAccountRules, + ), + ), + TransferField.TIMESTAMP to + DateTimeParsingMapping( + fieldType = TransferField.TIMESTAMP, + dateColumnName = "UTC_Time", + dateFormat = "yyyy-MM-dd", + dateTimeFormat = "yyyy-MM-dd HH:mm:ss", + ), + TransferField.DESCRIPTION to + DirectColumnMapping(fieldType = TransferField.DESCRIPTION, columnName = "Operation"), + // Change carries the direction: negative leaves the Binance account, positive arrives. + TransferField.AMOUNT to + AmountParsingMapping( + fieldType = TransferField.AMOUNT, + mode = AmountMode.SINGLE_COLUMN, + amountColumnName = "Change", + flipAccountsOnPositive = true, + ), + TransferField.CURRENCY to + CurrencyLookupMapping(fieldType = TransferField.CURRENCY, columnName = "Coin"), + TransferField.TIMEZONE to + HardCodedTimezoneMapping(fieldType = TransferField.TIMEZONE, timezoneId = "UTC"), + ) + val attributeMappings = + listOf( + AttributeColumnMapping("User_ID", "binance-user-id"), + AttributeColumnMapping("Operation", "binance-operation"), + AttributeColumnMapping("Remark", "binance-remark"), + ) + return CsvImportStrategy( + id = CsvImportStrategyId(binanceCsvStrategyId), + name = "Binance CSV", + identificationColumns = + setOf("User_ID", "UTC_Time", "Account", "Operation", "Coin", "Change", "Remark"), + fieldMappings = fieldMappings, + attributeMappings = attributeMappings, + // Binance names its exports with bare UUIDs, so there is no filename signal to use - and a + // filename match would win outright over content scoring and let a legacy file through. + contentMatchRules = listOf(ContentMatchRule(columnName = "User_ID", pattern = "^\\s*\\d+\\s*$")), + crossSourceReconcileWindowSeconds = BINANCE_RECONCILE_WINDOW_SECONDS, + conversionConfig = + ConversionConfig( + signalColumn = "Operation", + debitPattern = "^Small Assets Exchange BNB( \\(Spot\\))?$", + creditPattern = "^Small Assets Exchange BNB( \\(Spot\\))?$", + sideAmountColumn = "Change", + conversionAccountName = BINANCE_CONVERSIONS_ACCOUNT, + pairingWindowSeconds = BINANCE_CONVERSION_PAIRING_WINDOW_SECONDS, + relationshipTypeName = "conversion", + ), + tradeGroupConfig = + TradeGroupConfig( + signalColumn = "Operation", + debitPattern = "^(Sell|Transaction (Spend|Sold))$", + creditPattern = "^(Buy|Transaction (Buy|Revenue)|Binance Convert|Transaction Related)$", + // "Transaction Related" is the older name for *either* leg of a fill, so the sign of + // Change - not the operation name - has to decide which side each row is. + sideAmountColumn = "Change", + // Every leg of one fill carries the identical second, and distinct orders are + // seconds-to-days apart, so no jitter needs tolerating. + groupingWindowSeconds = 0L, + // Matches the API importer's "Buy BASE/QUOTE" wording for the same conversion. + descriptionTemplate = "Buy {to}/{from}", + ), + createdAt = now, + updatedAt = now, + ) + } } diff --git a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorFields.kt b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorFields.kt index 093ce9480..64534d625 100644 --- a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorFields.kt +++ b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorFields.kt @@ -336,6 +336,7 @@ internal fun buildStrategyFromEditorState( ?.takeIf { it.isNotBlank() } ?.let { AttributeAccountMatch(column = it, attributeTypeName = state.fundingMatchAttributeTypeName) }, conversionConfig = state.conversionConfig, + tradeGroupConfig = state.tradeGroupConfig, createdAt = createdAt, updatedAt = updatedAt, ) diff --git a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorState.kt b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorState.kt index 5676336c9..256f0f1e5 100644 --- a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorState.kt +++ b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csvstrategy/editor/CsvStrategyEditorState.kt @@ -231,6 +231,11 @@ internal class CsvStrategyEditorState( // Edited via ConversionConfigEditor (Advanced tab); null when the source has no such conversions. var conversionConfig by mutableStateOf(strategy?.conversionConfig) + // Carried through verbatim, like conversionConfig above, but with no editor of its own yet: row-group + // trade assembly is configured only by built-in strategies. Held here so editing such a strategy in + // the UI round-trips it instead of silently dropping the trades it assembles. + val tradeGroupConfig = strategy?.tradeGroupConfig + // Initial primary columns, used to avoid clobbering saved fallbacks on edit-mode load. val initialTargetAccountColumnName: String? = targetAccountColumnName val initialDescriptionColumnName: String? = descriptionColumnName From f6a6bd3da06f293f29f58557b84021a6b03294fd Mon Sep 17 00:00:00 2001 From: Nikolay Metchev Date: Mon, 31 Aug 2026 00:53:17 +0300 Subject: [PATCH 2/6] test(csv): cover the Binance CSV strategy end to end Adds the DB round trip for the new tradeGroupConfig/sideAmountColumn fields and an E2E suite over the shapes a real export contains: reward operations landing in their own accounts, the fiat/crypto funding split, a multi-fill order folding into one trade plus a fee transfer, a dust sweep staying as linked conversion legs rather than fabricated trades, re-import idempotency, and a legacy 6-column export being skipped. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UijVfE192UWTLiAjx3mvVL --- .../database/BuiltInCsvStrategyInstallTest.kt | 48 +++ .../database/csv/BinanceCsvE2ETest.kt | 289 ++++++++++++++++++ .../builtin/BuiltInCsvStrategies.kt | 4 +- 3 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt diff --git a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt index e7bc9df43..730efbf2f 100644 --- a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt +++ b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt @@ -14,6 +14,7 @@ import kotlinx.coroutines.test.runTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertIs +import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -141,4 +142,51 @@ class BuiltInCsvStrategyInstallTest : DbTest() { val idMapping = strategy.attributeMappings.single { it.columnName == "Transaction ID" } assertTrue(idMapping.isUniqueIdentifier) } + + @Test + fun `installing the built-in Binance CSV strategy round-trips through the database`() = + runTest { + repositories.installBuiltInCsvStrategies() + val strategy = + repositories.csvImportStrategyRepository + .getAllStrategies() + .first() + .single { it.name == "Binance CSV" } + + // The modern export's header, and only it: the legacy one lacks User_ID. + assertTrue( + strategy.matchesColumns(setOf("User_ID", "UTC_Time", "Account", "Operation", "Coin", "Change", "Remark")), + ) + assertTrue( + !strategy.matchesColumns(setOf("UTC_Time", "Account", "Operation", "Coin", "Change", "Remark")), + "the legacy 6-column header is not an exact match", + ) + // The content rule is what keeps a legacy file out via the tolerant subset path. + assertEquals("User_ID", strategy.contentMatchRules.single().columnName) + + // Trade-group assembly survives the round trip - without it the export's trade rows would + // import as suspense transfers instead of trades. + val tradeGroup = assertNotNull(strategy.tradeGroupConfig) + assertEquals("Operation", tradeGroup.signalColumn) + assertEquals("Change", tradeGroup.sideAmountColumn, "the ambiguous leg name is resolved by sign") + assertEquals(0L, tradeGroup.groupingWindowSeconds) + + // So does the dust conversion config, including the sign-based side classification. + val conversion = assertNotNull(strategy.conversionConfig) + assertEquals("Binance Conversions", conversion.conversionAccountName) + assertEquals("Change", conversion.sideAmountColumn) + assertEquals(conversion.debitPattern, conversion.creditPattern, "both dust legs share one Operation") + + // Fiat and crypto funding split to the two accounts the API strategy also creates. + val target = strategy.fieldMappings[TransferField.TARGET_ACCOUNT] + assertIs(target) + + val amount = strategy.fieldMappings[TransferField.AMOUNT] + assertIs(amount) + assertTrue(amount.flipAccountsOnPositive, "a positive Change arrives into the Binance account") + + val timestamp = strategy.fieldMappings[TransferField.TIMESTAMP] + assertIs(timestamp) + assertEquals("yyyy-MM-dd HH:mm:ss", timestamp.dateTimeFormat) + } } diff --git a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt new file mode 100644 index 000000000..02b10411f --- /dev/null +++ b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt @@ -0,0 +1,289 @@ +@file:OptIn(kotlin.time.ExperimentalTime::class, kotlin.uuid.ExperimentalUuidApi::class) + +package com.moneymanager.database.csv + +import com.moneymanager.csvimporter.bulkApplyCsv +import com.moneymanager.domain.Maintenance +import com.moneymanager.domain.model.csv.CsvImport +import com.moneymanager.test.database.DbTest +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue +import kotlin.time.Clock +import kotlin.time.Duration + +/** + * End-to-end cover for the built-in Binance CSV strategy: a staged export goes through the real + * engine and must produce the right accounts, balances and trades. + * + * The row data is taken from the shapes a real Binance export contains — a multi-fill order whose + * legs all carry one timestamp, a dust sweep whose credits cannot be attributed to its debits, and + * the reward operations no Binance API endpoint exposes. + */ +class BinanceCsvE2ETest : DbTest() { + override val installBuiltInStrategies: Boolean = true + + private val now = Clock.System.now() + + private val headers = listOf("User_ID", "UTC_Time", "Account", "Operation", "Coin", "Change", "Remark") + + private val maintenance = + object : Maintenance { + override suspend fun reindex(): Duration = Duration.ZERO + + override suspend fun vacuum(): Duration = Duration.ZERO + + override suspend fun analyze(): Duration = Duration.ZERO + + override suspend fun refreshMaterializedViews(): Duration = Duration.ZERO + + override suspend fun fullRefreshMaterializedViews(): Duration = Duration.ZERO + } + + private fun row( + time: String, + operation: String, + coin: String, + change: String, + remark: String = "", + ): List = listOf("53064551", time, "Spot", operation, coin, change, remark) + + private suspend fun stage( + fileName: String, + rows: List>, + ): CsvImport { + val id = + repositories.csvImportRepository.createImport( + fileName = fileName, + headers = headers, + rows = rows, + fileChecksum = "checksum-$fileName", + fileLastModified = now, + ) + return repositories.csvImportRepository.getImport(id).first()!! + } + + private suspend fun applyAll(imports: List) = + bulkApplyCsv( + imports = imports, + sourceAccountOverride = null, + strategies = repositories.csvImportStrategyRepository.getAllStrategies().first(), + currencies = repositories.currencyRepository.getAllCurrencies().first(), + accountMappingRepository = repositories.accountMappingRepository, + accountRepository = repositories.accountRepository, + csvImportRepository = repositories.csvImportRepository, + maintenance = maintenance, + importEngine = repositories.importEngine, + onProgress = { }, + cryptoRepository = repositories.cryptoRepository, + ) + + private suspend fun accountByName(name: String) = + repositories.accountRepository + .getAllAccounts() + .first() + .firstOrNull { it.name == name } + + private suspend fun balanceOf( + accountName: String, + assetCode: String, + ): String? { + repositories.maintenanceService.refreshMaterializedViews() + val account = assertNotNull(accountByName(accountName), "account '$accountName' exists") + return repositories.transactionRepository + .getAccountBalances() + .first() + .firstOrNull { it.accountId == account.id && it.balance.asset.code == assetCode } + ?.balance + ?.toDisplayValue() + ?.toString() + } + + @Test + fun rewardOperations_landInTheirOwnAccountsAndCreateCryptoOnDemand() = + runTest { + val file = + stage( + "0452506e-8714-11ee-9934-06655da838d5-1.csv", + listOf( + row("2023-01-02 03:04:05", "Staking Rewards", "DOT", "1.5"), + row("2023-01-03 03:04:05", "BNB Vault Rewards", "BNB", "0.002"), + row("2023-01-04 03:04:05", "Launchpool Earnings Withdrawal", "BNB", "0.5", "Binance Launchpool"), + row("2023-01-05 03:04:05", "Commission History", "BNB", "0.01"), + row("2023-01-06 03:04:05", "Simple Earn Flexible Subscription", "BNB", "-2.0", "Binance Earn"), + row("2023-01-07 03:04:05", "Simple Earn Flexible Interest", "BNB", "0.03", "Binance Earn"), + ), + ) + val result = applyAll(listOf(file)) + assertEquals(1, result.filesImported) + assertEquals(0, result.filesSkippedNoStrategy) + + // DOT is created on demand, exactly as the API importer would. + assertNotNull(repositories.cryptoRepository.getCryptoAssetByCode("DOT").first()) + + // Each product's income has its own account rather than being pooled into Earn. + assertEquals("-1.5", balanceOf("Binance Staking Rewards", "DOT")) + assertEquals("-0.002", balanceOf("Binance Vault Rewards", "BNB")) + assertEquals("-0.5", balanceOf("Binance Launchpool Rewards", "BNB")) + assertEquals("-0.01", balanceOf("Binance Commission", "BNB")) + // Earn holds the subscribed principal, its rewards account the interest. + assertEquals("2", balanceOf("Binance Earn", "BNB")) + assertEquals("-0.03", balanceOf("Binance Earn Rewards", "BNB")) + + // Binance nets: -2 subscribed +0.002 +0.5 +0.01 +0.03 = -1.458 BNB + assertEquals("-1.458", balanceOf("Binance", "BNB")) + assertEquals("1.5", balanceOf("Binance", "DOT")) + } + + @Test + fun depositAndWithdrawal_splitFiatFromCryptoFundingLikeTheApiDoes() = + runTest { + val file = + stage( + "deposits.csv", + listOf( + row("2023-01-02 03:04:05", "Deposit", "GBP", "500.00"), + row("2023-01-03 03:04:05", "Deposit", "BTC", "0.5"), + row("2023-01-04 03:04:05", "Withdraw", "BTC", "-0.1", "Withdraw fee is included"), + ), + ) + applyAll(listOf(file)) + + assertEquals("-500", balanceOf("Binance Bank", "GBP"), "fiat funding books against the bank account") + assertEquals("-0.4", balanceOf("Binance Funding", "BTC"), "crypto funding books against the funding account") + assertEquals("500", balanceOf("Binance", "GBP")) + assertEquals("0.4", balanceOf("Binance", "BTC")) + } + + @Test + fun aMultiFillOrder_becomesOneTradeAndOneFeeTransfer() = + runTest { + // The real 2022-11-14 20:32:54 shape, trimmed: several fills per leg all stamped with the + // same second, plus a fee in a third asset. + val file = + stage( + "trades.csv", + listOf( + row("2022-11-14 20:32:54", "Transaction Sold", "BTC", "-0.04382"), + row("2022-11-14 20:32:54", "Transaction Revenue", "GBP", "605.57925400"), + row("2022-11-14 20:32:54", "Transaction Sold", "BTC", "-0.90716"), + row("2022-11-14 20:32:54", "Transaction Revenue", "GBP", "12536.54297800"), + row("2022-11-14 20:32:54", "Transaction Fee", "BNB", "-0.00090052"), + ), + ) + applyAll(listOf(file)) + + val binance = assertNotNull(accountByName("Binance")) + val trades = repositories.tradeRepository.getTradesByAccount(binance.id).first() + assertEquals(1, trades.size, "the four legs fold into a single trade, not four") + val trade = trades.single() + assertEquals("BTC", trade.from.asset.code) + assertEquals("0.95098", trade.from.toDisplayValue().toString(), "the BTC fills sum") + assertEquals("GBP", trade.to.asset.code) + assertEquals("13142.122232", trade.to.toDisplayValue().toString(), "the GBP fills sum") + + // The fee is its own transfer, because a trade row has no fee field. It goes to the same + // account the Binance API strategy books fees to. + assertEquals("0.00090052", balanceOf("Binance Fees", "BNB")) + + // The suspense account is created while the legs are being mapped (assembly happens after), + // but every leg became the trade, so it holds nothing. A non-zero balance here would mean a + // group failed to resolve. + assertNull(balanceOf("Binance Trading", "BTC"), "no leg fell through to the suspense account") + assertNull(balanceOf("Binance Trading", "GBP"), "no leg fell through to the suspense account") + } + + @Test + fun aDustSweep_becomesLinkedConversionLegsRatherThanFabricatedTrades() = + runTest { + // A real sweep: three assets swept, three BNB credits, and nothing in the file saying which + // credit came from which debit. Assembling trades here would have to invent the pairing. + val file = + stage( + "dust.csv", + listOf( + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "REEF", "-90.89657258"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "BNB", "0.00062058"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "BNB", "0.03251993"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "BNB", "0.00050172"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "PSG", "-0.00187671"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "ASR", "-0.00294372"), + ), + ) + applyAll(listOf(file)) + + val binance = assertNotNull(accountByName("Binance")) + assertEquals( + 0, + repositories.tradeRepository + .getTradesByAccount(binance.id) + .first() + .size, + "a dust sweep never assembles into trades", + ) + + // Every swept asset leaves the Binance account in full and the BNB arrives in full, so the + // per-asset balances stay exact; the conversion account holds the mixed-asset residual. + assertEquals("-90.89657258", balanceOf("Binance", "REEF")) + assertEquals("0.03364223", balanceOf("Binance", "BNB")) + assertEquals("90.89657258", balanceOf("Binance Conversions", "REEF")) + assertEquals("-0.03364223", balanceOf("Binance Conversions", "BNB")) + } + + @Test + fun reimportingTheSameFileCreatesNothingNew() = + runTest { + val rows = + listOf( + row("2022-11-14 20:32:54", "Transaction Sold", "BTC", "-0.04382"), + row("2022-11-14 20:32:54", "Transaction Revenue", "GBP", "605.57925400"), + row("2022-11-14 20:32:54", "Transaction Fee", "BNB", "-0.00090052"), + row("2023-01-02 03:04:05", "Staking Rewards", "DOT", "1.5"), + ) + val file = stage("idempotent.csv", rows) + applyAll(listOf(file)) + + val binance = assertNotNull(accountByName("Binance")) + val tradesAfterFirst = repositories.tradeRepository.getTradesByAccount(binance.id).first().size + val btcAfterFirst = balanceOf("Binance", "BTC") + + applyAll(repositories.csvImportRepository.getAllImports().first()) + + assertEquals( + tradesAfterFirst, + repositories.tradeRepository + .getTradesByAccount(binance.id) + .first() + .size, + "a second pass over the same file books no second trade", + ) + assertEquals(btcAfterFirst, balanceOf("Binance", "BTC"), "and no second transfer") + } + + @Test + fun aLegacySixColumnExportIsSkippedRatherThanImported() = + runTest { + // Legacy exports use an older Operation vocabulary ("Savings purchase" for what the modern + // file calls "Simple Earn Flexible Subscription"), so importing both would book each event + // twice under two descriptions. + val legacyHeaders = headers.drop(1) + val id = + repositories.csvImportRepository.createImport( + fileName = "20210424.csv", + headers = legacyHeaders, + rows = listOf(listOf("2020-10-11 09:31:32", "Spot", "Savings purchase", "BNB", "-4.82096423", "")), + fileChecksum = "checksum-legacy", + fileLastModified = now, + ) + val legacy = repositories.csvImportRepository.getImport(id).first()!! + + val result = applyAll(listOf(legacy)) + assertEquals(0, result.filesImported) + assertEquals(1, result.filesSkippedNoStrategy, "no strategy claims a legacy export") + assertTrue(repositories.accountRepository.getAllAccounts().first().none { it.name == "Binance" }) + } +} diff --git a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt index 1a5703e77..ec570afbf 100644 --- a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt +++ b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt @@ -178,7 +178,9 @@ object BuiltInCsvStrategies { * Suspense counterparty for a trade leg whose group did not resolve into a trade — a one-sided * group, or one naming more than one asset on a side. Reached only in that case (an assembled * group's legs become the trade and never a transfer), so a non-zero balance here is a visible - * signal that an export had a shape the strategy does not model, rather than a silent loss. + * signal that an export had a shape the strategy does not model, rather than a silent loss. The + * account itself is created whenever the file has any trade rows — accounts are resolved while the + * rows are mapped, before groups are assembled — so an empty one is the normal, healthy state. */ private const val BINANCE_TRADING_ACCOUNT = "Binance Trading" From 2fee7226f2ddfffd6be1ddc7ceec56d377876273 Mon Sep 17 00:00:00 2001 From: Nikolay Metchev Date: Mon, 31 Aug 2026 01:34:38 +0300 Subject: [PATCH 3/6] feat(import): reconcile trades across sources so one movement is booked once The Binance CSV export and the Binance API describe many of the same trades, and the writer's exact-tuple match cannot see it: the API stamps milliseconds where the export stamps whole seconds, and the API reports each partial fill where the export reports only their total. Without this, importing the export on top of an API import double-counts every overlapping conversion. A trade cannot be tagged excluded and linked as reconciled the way a transfer is - transfer_attribute and transfer_relationship both reference transfer(id) - so a match instead suppresses the write and reports the existing trade's id, which is what createTrade's own idempotency already does and surfaces the row as a duplicate of that trade. - TradeDedupePolicy.Fuzzy + TradeReconciler: a windowed match on the asset pair and accounts, either against a single trade or against the whole in-window candidate set whose amounts sum to the incoming one. No subset search: a partial overlap is genuinely ambiguous and is left to book rather than guessed at. - ConversionGroupReconciler: dust sweeps arrive as conversion transfers, not trades, so they are matched as whole groups on their debited legs alone. The credited side cannot be compared - Binance's API reports the BNB received gross while the export reports it net of the service charge - and a partial match must suppress nothing, or the balances it protects would be corrupted. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UijVfE192UWTLiAjx3mvVL --- .../csvimporter/ConversionGroupReconciler.kt | 144 +++++++++++++ .../csvimporter/CsvImportApplier.kt | 195 ++++++++++------- .../moneymanager/csvimporter/CsvReimport.kt | 1 + .../csvimporter/CsvTradeGroupsTest.kt | 3 +- .../csvimporter/StrategySelectorTest.kt | 3 +- .../database/csv/BinanceCsvE2ETest.kt | 198 +++++++++++++++++- .../database/sql/trade/TradeSelect.sq | 31 +++ .../repository/TradeReadRepositoryImpl.kt | 28 +++ .../importengineapi/ImportBatch.kt | 1 + .../importengineapi/TradeDedupePolicy.kt | 42 ++++ .../moneymanager/importer/ImportEngineImpl.kt | 30 +++ .../moneymanager/importer/TradeReconciler.kt | 122 +++++++++++ .../importer/TradeReconcilerTest.kt | 196 +++++++++++++++++ .../domain/repository/TradeReadRepository.kt | 13 ++ .../builtin/BuiltInCsvStrategies.kt | 2 +- .../ui/screens/csv/CsvImportAllDialog.kt | 3 + .../ui/screens/csv/CsvImportsScreen.kt | 1 + 17 files changed, 925 insertions(+), 88 deletions(-) create mode 100644 app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt create mode 100644 app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/TradeDedupePolicy.kt create mode 100644 app/importer/src/commonMain/kotlin/com/moneymanager/importer/TradeReconciler.kt create mode 100644 app/importer/src/commonTest/kotlin/com/moneymanager/importer/TradeReconcilerTest.kt diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt new file mode 100644 index 000000000..1207e3569 --- /dev/null +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt @@ -0,0 +1,144 @@ +package com.moneymanager.csvimporter + +import com.moneymanager.domain.model.Trade +import com.moneymanager.domain.model.TradeId +import com.moneymanager.domain.model.csvstrategy.CsvImportStrategy +import com.moneymanager.domain.repository.TradeReadRepository +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds + +/** + * Decides which asset-conversion groups the database already holds as trades, so a sweep another + * source recorded is not counted twice. + * + * A conversion group is emitted as transfers, not a trade, precisely because its credited amounts + * cannot be attributed to its debited assets (see `ConversionConfig`). So it cannot be matched by the + * engine's trade reconciler, and it has to be matched **as a whole**: suppressing only the debit legs + * would leave the credits behind, double-counting the received asset and stranding a balance in the + * conversion account. + * + * The debited legs are what makes a match possible. A source that reports the conversion as a trade + * (Binance's dust API) agrees with the export exactly on the asset given up, and disagrees on the + * asset received — the API reports it gross and the export net of the service charge. So a group + * matches when **every** one of its debit legs finds a distinct existing trade with the same account, + * the same debited asset and the same debited amount inside the window; the credited side is never + * compared, because it cannot be. + */ +class ConversionGroupReconciler( + private val window: Duration, + existing: List, +) { + private data class LegKey( + val accountId: Long, + val assetId: Long, + val amount: String, + ) + + private val byDebitLeg: Map> = + existing.groupBy { LegKey(it.fromAccountId.id, it.from.asset.id.id, it.from.amount.toString()) } + + private val claimed = mutableSetOf() + + /** + * The trades this group duplicates, or null when it is not already recorded and must be imported. + * On a match every trade involved is claimed, so two groups never both match the same one. + */ + fun match(debits: List): List? { + if (debits.isEmpty()) return null + val matched = mutableListOf() + val claimedHere = mutableSetOf() + for (debit in debits) { + val key = + LegKey( + debit.transfer.sourceAccountId.id, + debit.transfer.amount.asset.id.id, + debit.transfer.amount.amount + .toString(), + ) + val candidate = + byDebitLeg[key] + ?.firstOrNull { + it.id !in claimed && + it.id !in claimedHere && + (it.timestamp - debit.transfer.timestamp).absoluteValue <= window + } + // One unmatched leg means the group is not the one already recorded, so the whole + // group imports. Partial suppression would corrupt the balances it is meant to protect. + ?: return null + claimedHere += candidate.id + matched += candidate.id + } + claimed += claimedHere + return matched + } +} + +/** + * Row index -> the trade it duplicates, for every leg of every conversion group another source already + * recorded. Empty unless the strategy declares both a conversion config and a reconcile window and a + * [TradeReadRepository] is available — reconciliation is opt-in, exactly like the transfer path's. + */ +suspend fun reconcileConversionGroups( + strategy: CsvImportStrategy, + rows: List, + tradeRepository: TradeReadRepository?, +): Map { + val conversionConfig = strategy.conversionConfig ?: return emptyMap() + val window = strategy.crossSourceReconcileWindowSeconds?.seconds ?: return emptyMap() + val repository = tradeRepository ?: return emptyMap() + + val groups = conversionGroups(rows, conversionConfig.pairingWindowSeconds.seconds) + if (groups.isEmpty()) return emptyMap() + + val legs = groups.flatten() + val accountIds = + legs.flatMapTo(mutableSetOf()) { listOf(it.transfer.sourceAccountId, it.transfer.targetAccountId) } + val existing = + repository.getTradesByAccountsAndDateRange( + accountIds = accountIds, + minTimestamp = legs.minOf { it.transfer.timestamp } - window, + maxTimestamp = legs.maxOf { it.transfer.timestamp } + window, + ) + if (existing.isEmpty()) return emptyMap() + + val reconciler = ConversionGroupReconciler(window, existing) + val reconciled = mutableMapOf() + for (group in groups) { + val debits = group.filter { it.conversionLeg?.side == ConversionSide.DEBIT } + val matched = reconciler.match(debits) ?: continue + // Every leg of the group - debits and the credits paired with them - records the trade it + // duplicates, so the rows read as duplicates of something rather than as silently missing. + group.forEach { reconciled[it.rowIndex] = matched.first() } + } + return reconciled +} + +/** + * Groups a file's conversion legs into events: each debit paired with the credits nearest it in time, + * mirroring how the applier links them. Returns one entry per debit-bearing event, with every leg that + * belongs to it, so a caller can accept or reject the event as a unit. + */ +fun conversionGroups( + rows: List, + pairingWindow: Duration, +): List> { + val legs = + rows + .filter { it.conversionLeg != null } + .sortedWith(compareBy({ it.transfer.timestamp }, { it.rowIndex })) + if (legs.isEmpty()) return emptyList() + + val groups = mutableListOf>() + var current = mutableListOf(legs.first()) + for (leg in legs.drop(1)) { + val previous = current.last().transfer.timestamp + if (leg.transfer.timestamp - previous <= pairingWindow) { + current += leg + } else { + groups += current + current = mutableListOf(leg) + } + } + groups += current + return groups +} diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt index fb11eaaa5..50dcc0ec3 100644 --- a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt @@ -11,6 +11,7 @@ import com.moneymanager.domain.model.Currency import com.moneymanager.domain.model.NewAttribute import com.moneymanager.domain.model.RelationshipTypeId import com.moneymanager.domain.model.Source +import com.moneymanager.domain.model.TradeId import com.moneymanager.domain.model.TransferId import com.moneymanager.domain.model.WellKnownIds import com.moneymanager.domain.model.accountmapping.AccountMapping @@ -27,6 +28,7 @@ import com.moneymanager.domain.repository.AccountMappingReadRepository import com.moneymanager.domain.repository.AccountReadRepository import com.moneymanager.domain.repository.CryptoReadRepository import com.moneymanager.domain.repository.CsvImportReadRepository +import com.moneymanager.domain.repository.TradeReadRepository import com.moneymanager.importengineapi.AccountRef import com.moneymanager.importengineapi.BatchRelationship import com.moneymanager.importengineapi.CsvImportMutation @@ -46,6 +48,7 @@ import com.moneymanager.importengineapi.LocalPersonKey import com.moneymanager.importengineapi.LocalTradeKey import com.moneymanager.importengineapi.PassThroughDetector import com.moneymanager.importengineapi.PersonMatchKey +import com.moneymanager.importengineapi.TradeDedupePolicy import com.moneymanager.importengineapi.applyCsvImportMutations import com.moneymanager.importengineapi.createAccount import com.moneymanager.importengineapi.createAccountMapping @@ -248,6 +251,7 @@ suspend fun bulkApplyCsv( cryptoRepository: CryptoReadRepository? = null, attributeAccountMatchers: Map = emptyMap(), directoryAccounts: Map = emptyMap(), + tradeRepository: TradeReadRepository? = null, ): CsvBulkResult { var filesImported = 0 var transfers = 0 @@ -291,6 +295,7 @@ suspend fun bulkApplyCsv( engineBatchSize = BULK_ENGINE_BATCH_SIZE, cryptoRepository = cryptoRepository, attributeAccountMatchers = attributeAccountMatchers, + tradeRepository = tradeRepository, ) ?: return@forEachIndexed filesImported++ transfers += result.successCount @@ -337,6 +342,7 @@ suspend fun applyStagedCsv( engineBatchSize: Int = Int.MAX_VALUE, cryptoRepository: CryptoReadRepository? = null, attributeAccountMatchers: Map = emptyMap(), + tradeRepository: TradeReadRepository? = null, ): CsvImportResult? { // An Excel import is initially staged from the workbook's FIRST worksheet, but the matched strategy // may target a different sheet. Re-extract + re-stage the correct sheet before reading rows so the @@ -414,6 +420,7 @@ suspend fun applyStagedCsv( engineBatchSize = engineBatchSize, attributeAccountMatchers = attributeAccountMatchers, unprocessedRowIndexes = unprocessedRowIndexes, + tradeRepository = tradeRepository, ) } @@ -679,6 +686,7 @@ suspend fun runCsvImport( // trades from row groups, where [rows] is widened to the whole file so a group is never seen with // some of its legs missing; transfers are then still emitted only for the rows named here. unprocessedRowIndexes: Set = rows.mapTo(mutableSetOf()) { it.rowIndex }, + tradeRepository: TradeReadRepository? = null, ): CsvImportResult { logger.info { "Starting CSV import with ${basePrep.validTransfers.size} valid transfers" } @@ -812,10 +820,18 @@ suspend fun runCsvImport( // drop out of the transfer list. A group that does not resolve assembles to null and its rows stay // ordinary transfers, so nothing is ever dropped for want of a clean pairing. val assembledTrades = - strategy.tradeGroupConfig?.let { config -> - groupTradeLegs(finalPrep.validTransfers, config).mapNotNull { it.assemble(config) } - }.orEmpty() + strategy.tradeGroupConfig + ?.let { config -> + groupTradeLegs(finalPrep.validTransfers, config).mapNotNull { it.assemble(config) } + }.orEmpty() val assembledRowIndexes: Set = assembledTrades.flatMapTo(mutableSetOf()) { it.group.rowIndexes } + + // Conversion groups another source already recorded as trades (Binance's dust API reports the same + // sweep the export splits into unpairable legs). They are matched and dropped as whole groups: see + // ConversionGroupReconciler for why only the debited legs can be compared, and why a partial match + // must not suppress anything. + val reconciledConversionRows: Map = + reconcileConversionGroups(strategy, finalPrep.validTransfers, tradeRepository) val assembledTradeRowIndexes: Map> = assembledTrades.associate { assembled -> assembled.tradeKey(csvImport.id) to assembled.group.rowIndexes @@ -899,86 +915,90 @@ suspend fun runCsvImport( val importTransfers = finalPrep.validTransfers - .filter { it.tradeTo == null && it.rowIndex !in assembledRowIndexes && it.rowIndex in unprocessedRowIndexes } - .map { row -> - val uniqueKey = - if (uniqueIdTypeNames.isEmpty()) { - null - } else { - row.attributes - .filter { (name, _) -> name in uniqueIdTypeNames } - .associate { (name, value) -> name to value } - } - val fee = - row.feeAmount?.let { feeMoney -> - ImportFee( - source = AccountRef.Existing(row.transfer.sourceAccountId), - target = AccountRef.Existing(feeAccountId!!), - amount = feeMoney, - description = "Fee", - relationshipTypeId = RelationshipTypeId(WellKnownIds.FEE_RELATIONSHIP_TYPE_ID), - ) - } - // Pass-through (conduit) row: the mapper already routed the transfer's conduit side — the - // target for an outgoing charge, the source for an incoming refund/cancellation; that side - // is the chain's first conduit. Resolve the remaining chain conduits + the merchant account - // the mapper created and let the engine add the spend legs (C1 -> C2, …, Cn -> merchant, or - // reversed when incoming). accountsByName carries the created conduit + merchant ids. - val passThrough = - row.passThrough?.let { pt -> - val merchantId = pt.merchantAccountId ?: accountsByName[pt.merchantName]?.id - val firstConduitId = if (pt.incoming) row.transfer.sourceAccountId else row.transfer.targetAccountId - val innerConduitIds = pt.conduitNames.drop(1).map { accountsByName[it]?.id } - if (merchantId == null || innerConduitIds.any { it == null }) { + .filter { + it.tradeTo == null && + it.rowIndex !in assembledRowIndexes && + it.rowIndex !in reconciledConversionRows && + it.rowIndex in unprocessedRowIndexes + }.map { row -> + val uniqueKey = + if (uniqueIdTypeNames.isEmpty()) { null } else { - val nodes = listOf(firstConduitId) + innerConduitIds.filterNotNull() + merchantId - collapsePassThroughChain(nodes, pt.spendDescriptions)?.let { (keptNodes, keptDescriptions) -> - ImportPassThrough( - conduits = keptNodes.dropLast(1).map { AccountRef.Existing(it) }, - merchantTarget = AccountRef.Existing(keptNodes.last()), - amount = row.transfer.amount, - spendDescriptions = keptDescriptions, - relationshipTypeId = RelationshipTypeId(pt.relationshipTypeId), - incoming = pt.incoming, - ) + row.attributes + .filter { (name, _) -> name in uniqueIdTypeNames } + .associate { (name, value) -> name to value } + } + val fee = + row.feeAmount?.let { feeMoney -> + ImportFee( + source = AccountRef.Existing(row.transfer.sourceAccountId), + target = AccountRef.Existing(feeAccountId!!), + amount = feeMoney, + description = "Fee", + relationshipTypeId = RelationshipTypeId(WellKnownIds.FEE_RELATIONSHIP_TYPE_ID), + ) + } + // Pass-through (conduit) row: the mapper already routed the transfer's conduit side — the + // target for an outgoing charge, the source for an incoming refund/cancellation; that side + // is the chain's first conduit. Resolve the remaining chain conduits + the merchant account + // the mapper created and let the engine add the spend legs (C1 -> C2, …, Cn -> merchant, or + // reversed when incoming). accountsByName carries the created conduit + merchant ids. + val passThrough = + row.passThrough?.let { pt -> + val merchantId = pt.merchantAccountId ?: accountsByName[pt.merchantName]?.id + val firstConduitId = if (pt.incoming) row.transfer.sourceAccountId else row.transfer.targetAccountId + val innerConduitIds = pt.conduitNames.drop(1).map { accountsByName[it]?.id } + if (merchantId == null || innerConduitIds.any { it == null }) { + null + } else { + val nodes = listOf(firstConduitId) + innerConduitIds.filterNotNull() + merchantId + collapsePassThroughChain(nodes, pt.spendDescriptions)?.let { (keptNodes, keptDescriptions) -> + ImportPassThrough( + conduits = keptNodes.dropLast(1).map { AccountRef.Existing(it) }, + merchantTarget = AccountRef.Existing(keptNodes.last()), + amount = row.transfer.amount, + spendDescriptions = keptDescriptions, + relationshipTypeId = RelationshipTypeId(pt.relationshipTypeId), + incoming = pt.incoming, + ) + } } } - } - // Funding reconcile hint: match the row's funding value against the strategy's funding - // attribute type to find the account that must hold the matching funding leg (e.g. Curve's - // "7721" -> the Crypto.com Card account, via the `card-last4` attribute regexes). Never point - // at the row's own source conduit (a self-reconcile makes no sense). - val fundingMatcher = strategy.fundingAttributeMatch?.let { attributeAccountMatchers[it.attributeTypeName] } - val fundingAccountId = - row.fundingMatchValue - ?.let { fundingMatcher?.match(it) } - ?.takeIf { it != row.transfer.sourceAccountId } - ImportTransfer( - rowKey = ImportRowKey.CsvRow(row.rowIndex), - fromAccount = AccountRef.Existing(row.transfer.sourceAccountId), - toAccount = AccountRef.Existing(row.transfer.targetAccountId), - source = Source.Csv(csvImport.id), - timestamp = row.transfer.timestamp, - description = row.transfer.description, - amount = row.transfer.amount, - attributes = - attributesFor(row.attributes) + - listOfNotNull( - // Marks the persisted leg as the placeholder record, so a later import that - // does name both ends knows which of the two to exclude. - unidentifiedCounterpartyTypeId - ?.takeIf { row.unidentifiedCounterpartyAccountId != null } - ?.let { NewAttribute(it, "true") }, - ), - uniqueKey = uniqueKey, - fee = fee, - passThrough = passThrough, - batchRelationships = listOfNotNull(conversionLinkByRow[row.rowIndex]), - reconcileFundingAccountId = fundingAccountId, - unidentifiedCounterpartyAccountId = row.unidentifiedCounterpartyAccountId, - ) - } + // Funding reconcile hint: match the row's funding value against the strategy's funding + // attribute type to find the account that must hold the matching funding leg (e.g. Curve's + // "7721" -> the Crypto.com Card account, via the `card-last4` attribute regexes). Never point + // at the row's own source conduit (a self-reconcile makes no sense). + val fundingMatcher = strategy.fundingAttributeMatch?.let { attributeAccountMatchers[it.attributeTypeName] } + val fundingAccountId = + row.fundingMatchValue + ?.let { fundingMatcher?.match(it) } + ?.takeIf { it != row.transfer.sourceAccountId } + ImportTransfer( + rowKey = ImportRowKey.CsvRow(row.rowIndex), + fromAccount = AccountRef.Existing(row.transfer.sourceAccountId), + toAccount = AccountRef.Existing(row.transfer.targetAccountId), + source = Source.Csv(csvImport.id), + timestamp = row.transfer.timestamp, + description = row.transfer.description, + amount = row.transfer.amount, + attributes = + attributesFor(row.attributes) + + listOfNotNull( + // Marks the persisted leg as the placeholder record, so a later import that + // does name both ends knows which of the two to exclude. + unidentifiedCounterpartyTypeId + ?.takeIf { row.unidentifiedCounterpartyAccountId != null } + ?.let { NewAttribute(it, "true") }, + ), + uniqueKey = uniqueKey, + fee = fee, + passThrough = passThrough, + batchRelationships = listOfNotNull(conversionLinkByRow[row.rowIndex]), + reconcileFundingAccountId = fundingAccountId, + unidentifiedCounterpartyAccountId = row.unidentifiedCounterpartyAccountId, + ) + } // Personal counterparties (resolved via person-flagged strategy rules, e.g. a RegexRule with // counterpartyIsPerson) become People with an ownership link to their counterparty account, in @@ -1029,6 +1049,14 @@ suspend fun runCsvImport( peopleToCreate = peopleToCreate, ownerships = personOwnerships, trades = importTrades, + // Same window, same meaning as the transfer reconcile below: how far this source's clock may + // disagree with another's about one movement. Aggregation is what lets a group assembled from + // several fills match the per-fill trades an API import already booked for the same second. + tradeDedupePolicy = + strategy.crossSourceReconcileWindowSeconds + ?.takeIf { strategy.tradeGroupConfig != null } + ?.let { TradeDedupePolicy.Fuzzy(window = it.seconds, allowAggregation = true) } + ?: TradeDedupePolicy.ExactTupleOnly, dedupePolicy = if (uniqueIdTypeNames.isEmpty()) { // Cross-source reconciliation is opt-in per strategy: rows recording a movement @@ -1097,8 +1125,7 @@ suspend fun runCsvImport( // An assembled trade speaks for every leg of its group, not just the row its key names, so all of // them take the trade's outcome. A single-row conversion has no group and falls back to its own row. - fun LocalTradeKey.rowIndexes(): List = - assembledTradeRowIndexes[this] ?: listOfNotNull(rowIndexOrNull()) + fun LocalTradeKey.rowIndexes(): List = assembledTradeRowIndexes[this] ?: listOfNotNull(rowIndexOrNull()) // The row records the trade's transaction id, exactly as a transfer row records its transfer's: // it is what links the row to what it produced, and what lets a re-import find the trade again. @@ -1112,6 +1139,12 @@ suspend fun runCsvImport( valueTransform = { (_, rowIndex, tradeId) -> rowIndex to TransferId(tradeId.id) }, ) + // Conversion legs suppressed as already-recorded record the trade they duplicate, so the rows read + // as duplicates of something rather than as rows that quietly produced nothing. + for ((rowIndex, tradeId) in reconciledConversionRows) { + duplicateStatuses[rowIndex] = TransferId(tradeId.id) + } + val statusMutations = mutableListOf() if (importedStatuses.isNotEmpty()) { statusMutations += CsvImportMutation.UpdateRowStatuses(csvImport.id, ImportStatus.IMPORTED.name, importedStatuses) diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvReimport.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvReimport.kt index dd12e86fd..ac5bfeac1 100644 --- a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvReimport.kt +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvReimport.kt @@ -1534,6 +1534,7 @@ suspend fun executeCsvReimport( onProgress = onProgress, attributeAccountMatchers = attributeAccountMatchers, engineBatchSize = REIMPORT_ENGINE_BATCH_SIZE, + tradeRepository = tradeRepository, ) onProgress?.invoke(ImportProgress("Cleaning up empty accounts")) diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt index 1d30aa45a..978dc1d8e 100644 --- a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt @@ -86,8 +86,7 @@ class CsvTradeGroupsTest { } /** A row the strategy did not flag as a trade leg (a fee, a deposit). */ - private fun nonLeg(display: String = "0.001"): CsvTransferWithAttributes = - leg(TradeLegSide.DEBIT, display, btc).copy(tradeLeg = null) + private fun nonLeg(display: String = "0.001"): CsvTransferWithAttributes = leg(TradeLegSide.DEBIT, display, btc).copy(tradeLeg = null) @Test fun oneFillPerSide_assemblesASingleTrade() { diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt index b9721ac93..566a99a8e 100644 --- a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/StrategySelectorTest.kt @@ -284,8 +284,7 @@ class StrategySelectorTest { private val binanceHeaders = listOf("User_ID", "UTC_Time", "Account", "Operation", "Coin", "Change", "Remark") - private fun binanceColumns(headers: List) = - headers.mapIndexed { i, name -> CsvColumn(CsvColumnId(Uuid.random()), i, name) } + private fun binanceColumns(headers: List) = headers.mapIndexed { i, name -> CsvColumn(CsvColumnId(Uuid.random()), i, name) } private fun binanceRow( index: Long, diff --git a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt index 02b10411f..b5dc81348 100644 --- a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt +++ b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt @@ -2,9 +2,17 @@ package com.moneymanager.database.csv +import com.moneymanager.bigdecimal.BigDecimal import com.moneymanager.csvimporter.bulkApplyCsv import com.moneymanager.domain.Maintenance +import com.moneymanager.domain.model.Account +import com.moneymanager.domain.model.AccountId +import com.moneymanager.domain.model.Money +import com.moneymanager.domain.model.Source import com.moneymanager.domain.model.csv.CsvImport +import com.moneymanager.importengineapi.createAccount +import com.moneymanager.importengineapi.createCrypto +import com.moneymanager.importengineapi.createTrade import com.moneymanager.test.database.DbTest import kotlinx.coroutines.flow.first import kotlinx.coroutines.test.runTest @@ -15,6 +23,7 @@ import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.time.Clock import kotlin.time.Duration +import kotlin.time.Instant /** * End-to-end cover for the built-in Binance CSV strategy: a staged export goes through the real @@ -80,8 +89,11 @@ class BinanceCsvE2ETest : DbTest() { importEngine = repositories.importEngine, onProgress = { }, cryptoRepository = repositories.cryptoRepository, + tradeRepository = repositories.tradeRepository, ) + private fun account(name: String) = Account(id = AccountId(0), name = name, openingDate = now) + private suspend fun accountByName(name: String) = repositories.accountRepository .getAllAccounts() @@ -234,6 +246,179 @@ class BinanceCsvE2ETest : DbTest() { assertEquals("-0.03364223", balanceOf("Binance Conversions", "BNB")) } + @Test + fun aTradeTheApiImportAlreadyBookedIsNotBookedTwice() = + runTest { + // Stand in for the API import: the per-fill trades api/v3/myTrades returns for one order, + // stamped to the millisecond, on the single Binance account. + val binanceId = repositories.importEngine.createAccount(account("Binance"), Source.Manual) + val btc = repositories.importEngine.createCrypto("BTC", "Bitcoin", Source.Manual) + val btcAsset = assertNotNull(repositories.cryptoRepository.getCryptoAssetByCode("BTC").first()) + val gbpAsset = + repositories.currencyRepository + .getAllCurrencies() + .first() + .first { it.code == "GBP" } + assertNotNull(btc) + val fills = listOf("0.04382" to "605.57925400", "0.90716" to "12536.54297800") + for ((from, to) in fills) { + repositories.importEngine.createTrade( + timestamp = Instant.parse("2022-11-14T20:32:54.462Z"), + description = "Sell BTC/GBP", + fromAccountId = binanceId, + fromAmount = Money.fromDisplayValue(BigDecimal(from), btcAsset), + toAccountId = binanceId, + toAmount = Money.fromDisplayValue(BigDecimal(to), gbpAsset), + ) + } + val tradesBefore = + repositories.tradeRepository + .getTradesByAccount(binanceId) + .first() + .size + assertEquals(2, tradesBefore) + + // The same order as the CSV records it: one row per fill, all stamped to the second, which + // the strategy folds into a single aggregate trade. + val file = + stage( + "overlap.csv", + listOf( + row("2022-11-14 20:32:54", "Transaction Sold", "BTC", "-0.04382"), + row("2022-11-14 20:32:54", "Transaction Revenue", "GBP", "605.57925400"), + row("2022-11-14 20:32:54", "Transaction Sold", "BTC", "-0.90716"), + row("2022-11-14 20:32:54", "Transaction Revenue", "GBP", "12536.54297800"), + ), + ) + applyAll(listOf(file)) + + assertEquals( + tradesBefore, + repositories.tradeRepository + .getTradesByAccount(binanceId) + .first() + .size, + "the CSV's aggregate matches the API's fills and is not booked a second time", + ) + // And the balances are the API import's alone - the CSV added nothing. + assertEquals("-0.95098", balanceOf("Binance", "BTC")) + assertEquals("13142.122232", balanceOf("Binance", "GBP")) + } + + @Test + fun aTradeTheApiImportDoesNotHaveIsStillBooked() = + runTest { + // The 2022 Convert conversions Binance's API window limit hides: nothing to match, so they + // must import in full. This is the case the reconcile must not over-suppress. + val file = + stage( + "new-trades.csv", + listOf( + row("2022-05-28 05:33:35", "Transaction Sold", "ADA", "-9373.0"), + row("2022-05-28 05:33:35", "Transaction Revenue", "BTC", "0.14921816"), + ), + ) + applyAll(listOf(file)) + + val binance = assertNotNull(accountByName("Binance")) + val trades = repositories.tradeRepository.getTradesByAccount(binance.id).first() + assertEquals(1, trades.size) + assertEquals( + "9373", + trades + .single() + .from + .toDisplayValue() + .toString(), + ) + } + + @Test + fun aDustSweepTheApiImportAlreadyBookedIsNotBookedTwice() = + runTest { + // Stand in for the API's asset/dribblet import: one trade per swept asset, its BNB leg + // reported GROSS. The CSV credits are net of Binance's 2% service charge, so only the + // debited legs can be compared — which is exactly what the group reconcile does. + val binanceId = repositories.importEngine.createAccount(account("Binance"), Source.Manual) + for (code in listOf("REEF", "PSG", "ASR", "BNB")) { + repositories.importEngine.createCrypto(code, code, Source.Manual) + } + val assets = + repositories.cryptoRepository + .getAllCryptoAssets() + .first() + .associateBy { it.code } + val sweep = + listOf( + Triple("REEF", "90.89657258", "0.03318361"), + Triple("PSG", "0.00187671", "0.00063325"), + Triple("ASR", "0.00294372", "0.00051960"), + ) + for ((code, from, to) in sweep) { + repositories.importEngine.createTrade( + timestamp = Instant.parse("2021-01-01T09:43:33Z"), + description = "Buy BNB/$code", + fromAccountId = binanceId, + fromAmount = Money.fromDisplayValue(BigDecimal(from), assets.getValue(code)), + toAccountId = binanceId, + toAmount = Money.fromDisplayValue(BigDecimal(to), assets.getValue("BNB")), + ) + } + val reefBefore = balanceOf("Binance", "REEF") + + val file = + stage( + "dust-overlap.csv", + listOf( + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "REEF", "-90.89657258"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "BNB", "0.00062058"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "BNB", "0.03251993"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "BNB", "0.00050172"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "PSG", "-0.00187671"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "ASR", "-0.00294372"), + ), + ) + applyAll(listOf(file)) + + assertEquals(reefBefore, balanceOf("Binance", "REEF"), "the sweep was already recorded; nothing moved") + assertNull(balanceOf("Binance Conversions", "REEF"), "and nothing was stranded in the conversion account") + assertNull(balanceOf("Binance Conversions", "BNB")) + } + + @Test + fun aDustSweepTheApiImportDoesNotHaveIsStillImported() = + runTest { + // A near-miss must not suppress: one leg differs, so the group is genuinely a different + // sweep and has to import in full. + val binanceId = repositories.importEngine.createAccount(account("Binance"), Source.Manual) + for (code in listOf("REEF", "BNB")) repositories.importEngine.createCrypto(code, code, Source.Manual) + val assets = + repositories.cryptoRepository + .getAllCryptoAssets() + .first() + .associateBy { it.code } + repositories.importEngine.createTrade( + timestamp = Instant.parse("2021-01-01T09:43:33Z"), + description = "Buy BNB/REEF", + fromAccountId = binanceId, + fromAmount = Money.fromDisplayValue(BigDecimal("11.0"), assets.getValue("REEF")), + toAccountId = binanceId, + toAmount = Money.fromDisplayValue(BigDecimal("0.004"), assets.getValue("BNB")), + ) + + val file = + stage( + "dust-new.csv", + listOf( + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "REEF", "-90.89657258"), + row("2021-01-01 09:43:33", "Small Assets Exchange BNB (Spot)", "BNB", "0.03251993"), + ), + ) + applyAll(listOf(file)) + + assertEquals("-101.89657258", balanceOf("Binance", "REEF"), "the unmatched sweep imported in full") + } + @Test fun reimportingTheSameFileCreatesNothingNew() = runTest { @@ -248,7 +433,11 @@ class BinanceCsvE2ETest : DbTest() { applyAll(listOf(file)) val binance = assertNotNull(accountByName("Binance")) - val tradesAfterFirst = repositories.tradeRepository.getTradesByAccount(binance.id).first().size + val tradesAfterFirst = + repositories.tradeRepository + .getTradesByAccount(binance.id) + .first() + .size val btcAfterFirst = balanceOf("Binance", "BTC") applyAll(repositories.csvImportRepository.getAllImports().first()) @@ -284,6 +473,11 @@ class BinanceCsvE2ETest : DbTest() { val result = applyAll(listOf(legacy)) assertEquals(0, result.filesImported) assertEquals(1, result.filesSkippedNoStrategy, "no strategy claims a legacy export") - assertTrue(repositories.accountRepository.getAllAccounts().first().none { it.name == "Binance" }) + assertTrue( + repositories.accountRepository + .getAllAccounts() + .first() + .none { it.name == "Binance" }, + ) } } diff --git a/app/db/read/src/commonMain/sqldelight/com/moneymanager/database/sql/trade/TradeSelect.sq b/app/db/read/src/commonMain/sqldelight/com/moneymanager/database/sql/trade/TradeSelect.sq index 7391074c7..24d4a938b 100644 --- a/app/db/read/src/commonMain/sqldelight/com/moneymanager/database/sql/trade/TradeSelect.sq +++ b/app/db/read/src/commonMain/sqldelight/com/moneymanager/database/sql/trade/TradeSelect.sq @@ -72,6 +72,37 @@ JOIN asset_details ta ON trade.to_asset_id = ta.id WHERE trade.from_account_id = ? OR trade.to_account_id = ? ORDER BY trade.timestamp DESC; +-- Trades on the given accounts within a time range, for fuzzy cross-source reconciliation: a CSV +-- export stamps a trade to the second while an API feed stamps it to the millisecond, and a CSV +-- aggregates the fills an API reports individually, so the exact-tuple match above cannot see that +-- the two describe one movement. The caller loads one window per batch and matches in memory. +selectByAccountsAndDateRange: +SELECT + trade.id, + trade.revision_id, + trade.timestamp, + trade.description, + trade.from_account_id, + trade.from_asset_id, + trade.from_amount, + fa.code AS from_asset_code, + fa.name AS from_asset_name, + fa.scale_factor AS from_asset_scale_factor, + fa.kind AS from_asset_kind, + trade.to_account_id, + trade.to_asset_id, + trade.to_amount, + ta.code AS to_asset_code, + ta.name AS to_asset_name, + ta.scale_factor AS to_asset_scale_factor, + ta.kind AS to_asset_kind +FROM trade +JOIN asset_details fa ON trade.from_asset_id = fa.id +JOIN asset_details ta ON trade.to_asset_id = ta.id +WHERE trade.timestamp BETWEEN :minTimestamp AND :maxTimestamp + AND (trade.from_account_id IN :accountIds OR trade.to_account_id IN :accountIds) +ORDER BY trade.timestamp, trade.id; + countByAccount: SELECT COUNT(*) FROM trade diff --git a/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/TradeReadRepositoryImpl.kt b/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/TradeReadRepositoryImpl.kt index 77c271755..66d8ed9df 100644 --- a/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/TradeReadRepositoryImpl.kt +++ b/app/db/repository/src/commonMain/kotlin/com/moneymanager/database/repository/TradeReadRepositoryImpl.kt @@ -12,6 +12,7 @@ import com.moneymanager.domain.repository.TradeReadRepository import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.withContext +import kotlin.time.Instant class TradeReadRepositoryImpl( database: MoneyManagerDatabase, @@ -46,4 +47,31 @@ class TradeReadRepositoryImpl( selectQueries.selectAccountsWithTrades(chunk).executeAsList() }.mapTo(mutableSetOf(), ::AccountId) } + + override suspend fun getTradesByAccountsAndDateRange( + accountIds: Collection, + minTimestamp: Instant, + maxTimestamp: Instant, + ): List = + withContext(Dispatchers.Default) { + val ids = accountIds.map { it.id }.distinct() + if (ids.isEmpty()) { + emptyList() + } else { + // Chunked like accountsWithTrades: the query names the id list twice (one leg each), so + // it is bound by the same parameter limit. Distinct because the chunks can overlap on a + // trade whose two legs fall in different chunks. + ids + .chunked(MAX_IDS_PER_TWO_SIDED_QUERY) + .flatMap { chunk -> + selectQueries + .selectByAccountsAndDateRange( + minTimestamp = minTimestamp.toEpochMilliseconds(), + maxTimestamp = maxTimestamp.toEpochMilliseconds(), + accountIds = chunk, + mapper = TradeMapper::mapRaw, + ).executeAsList() + }.distinctBy { it.id } + } + } } diff --git a/app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/ImportBatch.kt b/app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/ImportBatch.kt index 44170215b..91c11a0ed 100644 --- a/app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/ImportBatch.kt +++ b/app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/ImportBatch.kt @@ -524,6 +524,7 @@ data class ImportBatch( val currencies: List = emptyList(), val cryptoAssets: List = emptyList(), val trades: List = emptyList(), + val tradeDedupePolicy: TradeDedupePolicy = TradeDedupePolicy.ExactTupleOnly, val orders: List = emptyList(), val csvStrategyMutations: List = emptyList(), val apiStrategyMutations: List = emptyList(), diff --git a/app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/TradeDedupePolicy.kt b/app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/TradeDedupePolicy.kt new file mode 100644 index 000000000..ab05086bd --- /dev/null +++ b/app/importengineapi/src/commonMain/kotlin/com/moneymanager/importengineapi/TradeDedupePolicy.kt @@ -0,0 +1,42 @@ +package com.moneymanager.importengineapi + +import kotlin.time.Duration + +/** + * How the engine decides an incoming [ImportTradeIntent] is a trade the database already holds. + * + * Unlike a transfer, a matched trade cannot be "imported but tagged excluded and linked as + * reconciled": `transfer_attribute` and `transfer_relationship` both reference `transfer(id)`, so a + * trade can carry neither. The only mechanism available — and the one the exact-tuple path already + * uses — is to suppress the write and report the existing trade's id, which surfaces the source row + * as a duplicate pointing at the trade it duplicates. + */ +sealed interface TradeDedupePolicy { + /** + * Only the writer's exact-tuple match (timestamp to the millisecond, both accounts, both assets, + * both amounts). Enough for re-importing the same file, and the default. + */ + data object ExactTupleOnly : TradeDedupePolicy + + /** + * Also matches a trade another source already recorded slightly differently. Two sources describing + * one movement rarely agree on the exact tuple: an export stamps the second where an API stamps the + * millisecond, and an export aggregates the fills an API reports one by one. + * + * @property window How far apart the two sources' timestamps for one movement may be. + * @property allowAggregation Match one incoming trade against a *set* of existing trades whose + * amounts sum to it — the per-fill case. The whole in-window candidate + * set is tried as a unit; no subset search is attempted, so a partial + * overlap deliberately does not match. + * @property matchFromLegOnly Compare only the debited leg (account, asset and amount), ignoring the + * credited amount. For sources whose credited amount is net of a charge + * the other source reports gross — Binance's dust sweeps, where the CSV + * credit is exactly 98% of the API trade's. Never enable it where the + * debited leg alone could describe two genuinely different trades. + */ + data class Fuzzy( + val window: Duration, + val allowAggregation: Boolean = true, + val matchFromLegOnly: Boolean = false, + ) : TradeDedupePolicy +} diff --git a/app/importer/src/commonMain/kotlin/com/moneymanager/importer/ImportEngineImpl.kt b/app/importer/src/commonMain/kotlin/com/moneymanager/importer/ImportEngineImpl.kt index 25acc3a8b..cbb8ffebc 100644 --- a/app/importer/src/commonMain/kotlin/com/moneymanager/importer/ImportEngineImpl.kt +++ b/app/importer/src/commonMain/kotlin/com/moneymanager/importer/ImportEngineImpl.kt @@ -85,6 +85,7 @@ import com.moneymanager.importengineapi.PassThroughMutation import com.moneymanager.importengineapi.PersonMatchKey import com.moneymanager.importengineapi.QifImportMutation import com.moneymanager.importengineapi.RowOutcome +import com.moneymanager.importengineapi.TradeDedupePolicy import com.moneymanager.importengineapi.WriteIntent import com.moneymanager.importengineapi.bankKeyFromExternalId import com.moneymanager.importengineapi.bankKeysFrom @@ -198,7 +199,18 @@ class ImportEngineImpl( val toAmount: Money?, ) val tradeOccurrences = mutableMapOf() + // Cross-source reconcile (opt-in): two sources describing one conversion rarely agree on the + // exact tuple the writer matches on, so load the window they could disagree within and match in + // memory. A hit suppresses the write and reports the existing trade, which is the only way a + // trade can be linked to its duplicate (see TradeDedupePolicy). + val tradeReconciler = buildTradeReconciler(batch) for (intent in batch.trades.creates()) { + val reconciledId = tradeReconciler?.match(intent) + if (reconciledId != null) { + createdTradeIds[intent.key] = reconciledId + dedupedTradeKeys += intent.key + continue + } val tupleKey = TradeTupleKey( intent.timestamp?.toEpochMilliseconds(), @@ -1212,6 +1224,24 @@ class ImportEngineImpl( .mapTo(mutableSetOf()) { it.id2 } } + /** + * A [TradeReconciler] over the trades the batch's own trades could be duplicating, or null when the + * batch opts out, has no trades, or nothing exists to match. The window is loaded once and widened + * by the policy's tolerance, mirroring [loadExisting]'s slack: an existing trade stamped just + * outside the batch's own span is exactly the one a second-vs-millisecond disagreement produces. + */ + private suspend fun buildTradeReconciler(batch: ImportBatch): TradeReconciler? { + val policy = batch.tradeDedupePolicy as? TradeDedupePolicy.Fuzzy ?: return null + val creates = batch.trades.creates().filter { it.timestamp != null } + if (creates.isEmpty()) return null + val accountIds = creates.flatMap { listOfNotNull(it.fromAccountId, it.toAccountId) }.toSet() + if (accountIds.isEmpty()) return null + val minTs = creates.minOf { requireNotNull(it.timestamp) } - policy.window + val maxTs = creates.maxOf { requireNotNull(it.timestamp) } + policy.window + val existing = tradeRepository.getTradesByAccountsAndDateRange(accountIds, minTs, maxTs) + return if (existing.isEmpty()) null else TradeReconciler(policy, existing) + } + private suspend fun loadExisting( transfers: List, batch: ImportBatch, diff --git a/app/importer/src/commonMain/kotlin/com/moneymanager/importer/TradeReconciler.kt b/app/importer/src/commonMain/kotlin/com/moneymanager/importer/TradeReconciler.kt new file mode 100644 index 000000000..6ce7aa5e4 --- /dev/null +++ b/app/importer/src/commonMain/kotlin/com/moneymanager/importer/TradeReconciler.kt @@ -0,0 +1,122 @@ +package com.moneymanager.importer + +import com.moneymanager.domain.model.AccountId +import com.moneymanager.domain.model.AssetId +import com.moneymanager.domain.model.Money +import com.moneymanager.domain.model.Trade +import com.moneymanager.domain.model.TradeId +import com.moneymanager.importengineapi.ImportTradeIntent +import com.moneymanager.importengineapi.TradeDedupePolicy +import kotlin.time.Instant + +/** + * Matches incoming trades against trades another source already recorded, so one movement described + * by two sources is booked once. + * + * The exact-tuple match the writer performs cannot see these: an export stamps a trade to the second + * where an API stamps it to the millisecond, and an export aggregates into one row the partial fills + * an API reports individually. This reconciler closes that gap in memory over one preloaded window. + * + * A match **suppresses the write** and reports the existing trade's id. That is the only mechanism + * available — a trade can carry neither an `excluded` attribute nor a `reconciled` relationship, since + * both tables reference `transfer(id)` — and it is the same one the writer's own idempotency uses, so + * the source row surfaces as a duplicate pointing at the trade it duplicates. + * + * Matched trades are **claimed**, so two incoming trades never both match the same existing one. + * Claims live for one reconciler (one import), which is weaker than the transfer path's persisted + * reconcile links: across separate files an existing trade could be claimed twice. That direction is + * the safe one — over-suppression cannot invent money — and amount equality bounds the other. + */ +class TradeReconciler( + private val policy: TradeDedupePolicy.Fuzzy, + existing: List, +) { + private data class BucketKey( + val fromAccountId: AccountId, + val toAccountId: AccountId, + val fromAssetId: AssetId, + val toAssetId: AssetId, + ) + + /** Candidates grouped by the fields any match requires, then ordered by time within a bucket. */ + private val buckets: Map> = + existing + .groupBy { BucketKey(it.fromAccountId, it.toAccountId, it.from.asset.id, it.to.asset.id) } + .mapValues { (_, trades) -> trades.sortedBy { it.timestamp } } + + private val claimed = mutableSetOf() + + /** + * The id of an existing trade (or the earliest of an existing set) that [intent] duplicates, or + * null when nothing matches and the trade should be written. + */ + fun match(intent: ImportTradeIntent): TradeId? { + val fromAccountId = intent.fromAccountId ?: return null + val toAccountId = intent.toAccountId ?: return null + val fromAmount = intent.fromAmount ?: return null + val toAmount = intent.toAmount ?: return null + val timestamp = intent.timestamp ?: return null + + val key = BucketKey(fromAccountId, toAccountId, fromAmount.asset.id, toAmount.asset.id) + // The overwhelmingly common case is a movement no other source recorded: one hash miss and out, + // before any window arithmetic. + val candidates = buckets[key] ?: return null + + val inWindow = candidates.filter { it.id !in claimed && withinWindow(it.timestamp, timestamp) } + if (inWindow.isEmpty()) return null + + matchOne(inWindow, fromAmount, toAmount, timestamp)?.let { return claim(listOf(it)) } + if (policy.allowAggregation) { + matchSet(inWindow, fromAmount, toAmount)?.let { return claim(it) } + } + return null + } + + private fun withinWindow( + candidate: Instant, + incoming: Instant, + ): Boolean = (candidate - incoming).absoluteValue <= policy.window + + /** A single existing trade describing the same movement; the nearest in time wins. */ + private fun matchOne( + candidates: List, + fromAmount: Money, + toAmount: Money, + timestamp: Instant, + ): Trade? = + candidates + .filter { amountsMatch(it.from, it.to, fromAmount, toAmount) } + .minByOrNull { (it.timestamp - timestamp).absoluteValue } + + /** + * The per-fill case: this source aggregated what the other reported one fill at a time. The whole + * in-window candidate set is tried as a unit — because both sides derive from the same fills, their + * totals agree exactly when they describe the same event. No subset search is attempted: it is + * exponential, and a partial overlap is genuinely ambiguous, so it is left to book separately + * rather than guessed at. + */ + private fun matchSet( + candidates: List, + fromAmount: Money, + toAmount: Money, + ): List? { + if (candidates.size < 2) return null + val fromTotal = candidates.map { it.from }.reduce(Money::plus) + val toTotal = candidates.map { it.to }.reduce(Money::plus) + return candidates.takeIf { amountsMatch(fromTotal, toTotal, fromAmount, toAmount) } + } + + private fun amountsMatch( + candidateFrom: Money, + candidateTo: Money, + fromAmount: Money, + toAmount: Money, + ): Boolean = + candidateFrom == fromAmount && + (policy.matchFromLegOnly || candidateTo == toAmount) + + private fun claim(trades: List): TradeId { + trades.forEach { claimed += it.id } + return trades.minOf { it.id.id }.let(::TradeId) + } +} diff --git a/app/importer/src/commonTest/kotlin/com/moneymanager/importer/TradeReconcilerTest.kt b/app/importer/src/commonTest/kotlin/com/moneymanager/importer/TradeReconcilerTest.kt new file mode 100644 index 000000000..346e2b619 --- /dev/null +++ b/app/importer/src/commonTest/kotlin/com/moneymanager/importer/TradeReconcilerTest.kt @@ -0,0 +1,196 @@ +package com.moneymanager.importer + +import com.moneymanager.bigdecimal.BigDecimal +import com.moneymanager.domain.model.AccountId +import com.moneymanager.domain.model.Asset +import com.moneymanager.domain.model.CryptoAsset +import com.moneymanager.domain.model.CryptoId +import com.moneymanager.domain.model.Currency +import com.moneymanager.domain.model.CurrencyId +import com.moneymanager.domain.model.CurrencyScaleFactors +import com.moneymanager.domain.model.Money +import com.moneymanager.domain.model.Source +import com.moneymanager.domain.model.Trade +import com.moneymanager.domain.model.TradeId +import com.moneymanager.importengineapi.ImportTradeIntent +import com.moneymanager.importengineapi.LocalTradeKey +import com.moneymanager.importengineapi.TradeDedupePolicy +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * Covers matching a trade a CSV export describes against the trades an API import already booked for + * the same movement. The cases are the two real disagreements between the sources: the API stamps + * milliseconds where the export stamps whole seconds, and the API reports each partial fill where the + * export reports only their total. + */ +class TradeReconcilerTest { + private val binance = AccountId(1) + private val other = AccountId(2) + + private val gbp = + Currency( + id = CurrencyId(1), + code = "GBP", + name = "Pound Sterling", + scaleFactor = CurrencyScaleFactors.DEFAULT_SCALE_FACTOR, + ) + private val btc = CryptoAsset(id = CryptoId(10), code = "BTC", name = "Bitcoin") + private val eth = CryptoAsset(id = CryptoId(11), code = "ETH", name = "Ethereum") + + private var nextTradeId = 1L + + private fun money( + display: String, + asset: Asset, + ) = Money.fromDisplayValue(BigDecimal(display), asset) + + private fun existing( + at: String, + from: String, + fromAsset: Asset, + to: String, + toAsset: Asset, + account: AccountId = binance, + ) = Trade( + id = TradeId(nextTradeId++), + timestamp = Instant.parse(at), + description = "api trade", + fromAccountId = account, + from = money(from, fromAsset), + toAccountId = account, + to = money(to, toAsset), + ) + + private fun incoming( + at: String, + from: String, + fromAsset: Asset, + to: String, + toAsset: Asset, + account: AccountId = binance, + ) = ImportTradeIntent( + key = LocalTradeKey("csv-1-1"), + source = Source.Manual, + timestamp = Instant.parse(at), + description = "csv trade", + fromAccountId = account, + fromAmount = money(from, fromAsset), + toAccountId = account, + toAmount = money(to, toAsset), + ) + + private fun reconciler( + existing: List, + window: Long = 300, + allowAggregation: Boolean = true, + matchFromLegOnly: Boolean = false, + ) = TradeReconciler( + TradeDedupePolicy.Fuzzy( + window = window.seconds, + allowAggregation = allowAggregation, + matchFromLegOnly = matchFromLegOnly, + ), + existing, + ) + + @Test + fun matchesAcrossASubSecondTimestampDisagreement() { + // api/v3/myTrades stamps 20:32:54.462; the CSV export only records the second. + val api = existing("2022-11-14T20:32:54.462Z", "1.0", btc, "13819.5897271", gbp) + val matched = reconciler(listOf(api)).match(incoming("2022-11-14T20:32:54Z", "1.0", btc, "13819.5897271", gbp)) + assertEquals(api.id, matched) + } + + @Test + fun doesNotMatchOutsideTheWindow() { + val api = existing("2022-11-14T20:32:54Z", "1.0", btc, "13819.5897271", gbp) + val matched = + reconciler(listOf(api), window = 60) + .match(incoming("2022-11-14T20:40:00Z", "1.0", btc, "13819.5897271", gbp)) + assertNull(matched) + } + + @Test + fun aggregatesPerFillTradesWhoseTotalsEqualTheIncomingTrade() { + // The real 2022-11-14 group: six API fills, one CSV row group summing to them. + val btcFills = listOf("0.04382", "0.90716", "0.00441", "0.00312", "0.01351", "0.02798") + val gbpFills = + listOf("605.57925400", "12536.54297800", "60.96551580", "43.12694880", "186.70374170", "386.67128880") + val fills = btcFills.zip(gbpFills).map { (b, g) -> existing("2022-11-14T20:32:54.462Z", b, btc, g, gbp) } + + val matched = + reconciler(fills).match(incoming("2022-11-14T20:32:54Z", "1.00000000", btc, "13819.5897271", gbp)) + assertEquals(fills.first().id, matched, "the earliest of the matched set is reported") + } + + @Test + fun aggregationIsRefusedWhenTheTotalsDisagree() { + // A partial overlap - some fills already imported, some not - is genuinely ambiguous, so the + // incoming trade is written rather than guessed at. + val fills = + listOf("0.04382", "0.90716").zip(listOf("605.57925400", "12536.54297800")).map { (b, g) -> + existing("2022-11-14T20:32:54.462Z", b, btc, g, gbp) + } + assertNull(reconciler(fills).match(incoming("2022-11-14T20:32:54Z", "1.00000000", btc, "13819.5897271", gbp))) + } + + @Test + fun aggregationCanBeTurnedOff() { + val fills = + listOf("0.5", "0.5").map { existing("2022-11-14T20:32:54.462Z", it, btc, "6909.79486355", gbp) } + assertNull( + reconciler(fills, allowAggregation = false) + .match(incoming("2022-11-14T20:32:54Z", "1.0", btc, "13819.5897271", gbp)), + ) + } + + @Test + fun anExistingTradeIsClaimedByAtMostOneIncomingTrade() { + val api = existing("2022-11-14T20:32:54Z", "1.0", btc, "13819.5897271", gbp) + val subject = reconciler(listOf(api)) + assertNotNull(subject.match(incoming("2022-11-14T20:32:54Z", "1.0", btc, "13819.5897271", gbp))) + assertNull( + subject.match(incoming("2022-11-14T20:32:54Z", "1.0", btc, "13819.5897271", gbp)), + "a genuinely repeated identical trade is still written", + ) + } + + @Test + fun theNearestCandidateInTimeWins() { + val near = existing("2022-11-14T20:32:54Z", "1.0", btc, "100.0", gbp) + val far = existing("2022-11-14T20:35:00Z", "1.0", btc, "100.0", gbp) + assertEquals(near.id, reconciler(listOf(far, near)).match(incoming("2022-11-14T20:32:55Z", "1.0", btc, "100.0", gbp))) + } + + @Test + fun aDifferentAssetPairIsNotAMatch() { + val api = existing("2022-11-14T20:32:54Z", "1.0", eth, "100.0", gbp) + assertNull(reconciler(listOf(api)).match(incoming("2022-11-14T20:32:54Z", "1.0", btc, "100.0", gbp))) + } + + @Test + fun aDifferentAccountIsNotAMatch() { + val api = existing("2022-11-14T20:32:54Z", "1.0", btc, "100.0", gbp, account = other) + assertNull(reconciler(listOf(api)).match(incoming("2022-11-14T20:32:54Z", "1.0", btc, "100.0", gbp))) + } + + @Test + fun matchFromLegOnly_ignoresACreditedAmountTheOtherSourceReportsGross() { + // Binance's dust API reports the BNB received before its 2% service charge; the CSV reports it + // after. The debited leg is identical, and is the only side that can be compared. + val api = existing("2021-01-01T09:43:33Z", "90.89657258", eth, "0.03318361", btc) + val csv = incoming("2021-01-01T09:43:33Z", "90.89657258", eth, "0.03251993", btc) + assertNull(reconciler(listOf(api)).match(csv), "comparing both legs cannot match a net-vs-gross pair") + assertEquals(api.id, reconciler(listOf(api), matchFromLegOnly = true).match(csv)) + } + + @Test + fun anEmptyCandidateSetMatchesNothing() { + assertNull(reconciler(emptyList()).match(incoming("2022-11-14T20:32:54Z", "1.0", btc, "100.0", gbp))) + } +} diff --git a/app/model/repository/read/src/commonMain/kotlin/com/moneymanager/domain/repository/TradeReadRepository.kt b/app/model/repository/read/src/commonMain/kotlin/com/moneymanager/domain/repository/TradeReadRepository.kt index 7684d1853..5c519ee9e 100644 --- a/app/model/repository/read/src/commonMain/kotlin/com/moneymanager/domain/repository/TradeReadRepository.kt +++ b/app/model/repository/read/src/commonMain/kotlin/com/moneymanager/domain/repository/TradeReadRepository.kt @@ -4,6 +4,7 @@ import com.moneymanager.domain.model.AccountId import com.moneymanager.domain.model.Trade import com.moneymanager.domain.model.TradeId import kotlinx.coroutines.flow.Flow +import kotlin.time.Instant interface TradeReadRepository { fun getTradeById(id: TradeId): Flow @@ -15,4 +16,16 @@ interface TradeReadRepository { /** Which of [accountIds] appear on either leg of any trade (batch emptiness check). */ suspend fun accountsWithTrades(accountIds: Collection): Set + + /** + * Trades touching any of [accountIds] whose timestamp falls in `[minTimestamp, maxTimestamp]`. + * Loaded in one window per import so a fuzzy trade reconcile can match in memory: an export that + * stamps to the second, or aggregates the fills another source reports individually, cannot be + * matched by the exact-tuple lookup the writer uses. + */ + suspend fun getTradesByAccountsAndDateRange( + accountIds: Collection, + minTimestamp: Instant, + maxTimestamp: Instant, + ): List } diff --git a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt index ec570afbf..63afdce1a 100644 --- a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt +++ b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt @@ -26,8 +26,8 @@ import com.moneymanager.domain.model.csvstrategy.RowCondition import com.moneymanager.domain.model.csvstrategy.RowConditionOperator import com.moneymanager.domain.model.csvstrategy.RowPreprocessingRule import com.moneymanager.domain.model.csvstrategy.TemplateAccountMapping -import com.moneymanager.domain.model.csvstrategy.TransferField import com.moneymanager.domain.model.csvstrategy.TradeGroupConfig +import com.moneymanager.domain.model.csvstrategy.TransferField import com.moneymanager.domain.model.qif.QifColumns import kotlin.time.Instant import kotlin.uuid.Uuid diff --git a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportAllDialog.kt b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportAllDialog.kt index f4ba74b2e..583b2bfa9 100644 --- a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportAllDialog.kt +++ b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportAllDialog.kt @@ -38,6 +38,7 @@ import com.moneymanager.domain.repository.CurrencyReadRepository import com.moneymanager.domain.repository.ImportDirectoryReadRepository import com.moneymanager.domain.repository.PassThroughAccountReadRepository import com.moneymanager.domain.repository.PersonReadRepository +import com.moneymanager.domain.repository.TradeReadRepository import com.moneymanager.importengineapi.ImportEngine import com.moneymanager.ui.components.AccountPicker import com.moneymanager.ui.components.LoadingTextButton @@ -65,6 +66,7 @@ fun CsvImportAllDialog( categoryRepository: CategoryReadRepository, currencyRepository: CurrencyReadRepository, cryptoRepository: CryptoReadRepository, + tradeRepository: TradeReadRepository, personRepository: PersonReadRepository, passThroughAccountRepository: PassThroughAccountReadRepository, csvImportRepository: CsvImportReadRepository, @@ -190,6 +192,7 @@ fun CsvImportAllDialog( onProgress = { progress = it }, passThroughAccounts = passThroughAccounts, cryptoRepository = cryptoRepository, + tradeRepository = tradeRepository, attributeAccountMatchers = AttributeAccountMatcher.registry(accountAttributes), directoryAccounts = directoryAccounts, ) diff --git a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportsScreen.kt b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportsScreen.kt index 3db344b07..3498377ec 100644 --- a/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportsScreen.kt +++ b/app/ui/imports/csv/src/commonMain/kotlin/com/moneymanager/ui/screens/csv/CsvImportsScreen.kt @@ -322,6 +322,7 @@ fun CsvImportsScreen( categoryRepository = categoryRepository, currencyRepository = currencyRepository, cryptoRepository = cryptoRepository, + tradeRepository = tradeRepository, personRepository = personRepository, passThroughAccountRepository = passThroughAccountRepository, csvImportRepository = csvImportRepository, From 45a01ecddc56727d1a65372f86d578948cf279fc Mon Sep 17 00:00:00 2001 From: Nikolay Metchev Date: Mon, 31 Aug 2026 02:08:44 +0300 Subject: [PATCH 4/6] fix(binance): book deposits against a placeholder and split the reconcile windows Both changes come from running the strategy against a copy of the real database. Deposits and withdrawals were routed by coin - fiat to "Binance Bank", crypto to "Binance Funding" - on the assumption that the API's fiat endpoints book fiat to the bank account. They do not: in practice the API puts every GBP deposit and withdrawal on Binance Funding, and books crypto deposits against the on-chain address they came from. Neither matched what the CSV produced, so ~GBP 20k of deposits imported twice. Deposit/Withdraw now book against one placeholder, and - because the export never names the other side at all - the counterparty is marked unidentified, which is what lets the engine reconcile a row against the API's record of the same movement whatever counterparty that record names. The trade and transfer reconciles also needed separate windows. Transfers need about an hour: the API records when Binance credited a fiat deposit and the export when it was initiated, a gap of seconds to minutes (an hour recovers 19 of 25 such rows; a day recovers no more, while raising identical-amount reward collisions from 65 to 4,088). Trades need seconds: aggregation matches a group against the whole in-window candidate set, so an hour would drag a later order's fills in and stop the sums matching at all. Documents one limitation the export cannot fix: a withdrawal remarked "Withdraw fee is included" is gross while the API books it net plus a fee, so the two never reconcile. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UijVfE192UWTLiAjx3mvVL --- .../csvimporter/ConversionGroupReconciler.kt | 2 +- .../csvimporter/CsvImportApplier.kt | 10 +-- .../csvimporter/BinanceCsvMapperTest.kt | 23 +++-- .../database/BuiltInCsvStrategyInstallTest.kt | 12 ++- .../database/csv/BinanceCsvE2ETest.kt | 64 +++++++++++++- .../model/csvstrategy/ConversionConfig.kt | 9 ++ .../model/csvstrategy/TradeGroupConfig.kt | 9 ++ .../builtin/BuiltInCsvStrategies.kt | 83 +++++++++++++------ 8 files changed, 169 insertions(+), 43 deletions(-) diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt index 1207e3569..6c5057846 100644 --- a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/ConversionGroupReconciler.kt @@ -84,7 +84,7 @@ suspend fun reconcileConversionGroups( tradeRepository: TradeReadRepository?, ): Map { val conversionConfig = strategy.conversionConfig ?: return emptyMap() - val window = strategy.crossSourceReconcileWindowSeconds?.seconds ?: return emptyMap() + val window = conversionConfig.reconcileWindowSeconds?.seconds ?: return emptyMap() val repository = tradeRepository ?: return emptyMap() val groups = conversionGroups(rows, conversionConfig.pairingWindowSeconds.seconds) diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt index 50dcc0ec3..bf17ba397 100644 --- a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt @@ -1049,12 +1049,12 @@ suspend fun runCsvImport( peopleToCreate = peopleToCreate, ownerships = personOwnerships, trades = importTrades, - // Same window, same meaning as the transfer reconcile below: how far this source's clock may - // disagree with another's about one movement. Aggregation is what lets a group assembled from - // several fills match the per-fill trades an API import already booked for the same second. + // Trades get their own, much tighter window than the transfer reconcile below: aggregation + // matches a group against the WHOLE in-window candidate set, so a wide window would drag a + // later order's fills in and stop the sums matching at all. tradeDedupePolicy = - strategy.crossSourceReconcileWindowSeconds - ?.takeIf { strategy.tradeGroupConfig != null } + strategy.tradeGroupConfig + ?.reconcileWindowSeconds ?.let { TradeDedupePolicy.Fuzzy(window = it.seconds, allowAggregation = true) } ?: TradeDedupePolicy.ExactTupleOnly, dedupePolicy = diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt index 29d89e34a..1fa601271 100644 --- a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt @@ -116,17 +116,30 @@ class BinanceCsvMapperTest { } @Test - fun depositAndWithdrawal_splitFiatFromCryptoFunding() { - // The API books crypto funding against "Binance Funding" and fiat against "Binance Bank"; the - // CSV has to make the same split or the two sources' versions of one movement never reconcile. + fun depositAndWithdrawal_bookAgainstAPlaceholderCounterparty() { + // The export never names the other side, so Deposit/Withdraw go to one placeholder whatever the + // coin — the same name the API falls back to when it has no address. Only the explicitly fiat + // operations, which the API books via its fiat endpoints, use the bank placeholder. assertEquals("Binance Funding", counterpartyName(map(row("Deposit", "BNB", "4.82096423")))) assertEquals("Binance Funding", counterpartyName(map(row("Withdraw", "BNB", "-1.0")))) - assertEquals("Binance Bank", counterpartyName(map(row("Deposit", "GBP", "500.00")))) - assertEquals("Binance Bank", counterpartyName(map(row("Withdraw", "GBP", "-500.00")))) + assertEquals("Binance Funding", counterpartyName(map(row("Deposit", "GBP", "500.00")))) + assertEquals("Binance Funding", counterpartyName(map(row("Withdraw", "GBP", "-500.00")))) assertEquals("Binance Bank", counterpartyName(map(row("Fiat Deposit", "GBP", "500.00")))) assertEquals("Binance Bank", counterpartyName(map(row("Fiat Withdrawal", "GBP", "-500.00")))) } + @Test + fun depositAndWithdrawalCounterpartiesAreMarkedUnidentified() { + // This is what lets a deposit reconcile against the API's record of it, which names the on-chain + // address the CSV cannot see. + assertNotNull(map(row("Deposit", "BNB", "1.0")).unidentifiedCounterpartyAccountId) + assertNotNull(map(row("Withdraw", "BNB", "-1.0")).unidentifiedCounterpartyAccountId) + assertNull( + map(row("Staking Rewards", "BNB", "0.01")).unidentifiedCounterpartyAccountId, + "a product account is a real identity, not a placeholder", + ) + } + @Test fun patternsAreAnchored_soNoOperationSwallowsAnother() { // RegexRule matching is containsMatchIn: an unanchored "Deposit" would also claim "Fiat Deposit" diff --git a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt index 730efbf2f..faacf7c6a 100644 --- a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt +++ b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/BuiltInCsvStrategyInstallTest.kt @@ -5,6 +5,7 @@ package com.moneymanager.database import com.moneymanager.domain.model.csvstrategy.AmountParsingMapping import com.moneymanager.domain.model.csvstrategy.ConditionalAccountMapping import com.moneymanager.domain.model.csvstrategy.DateTimeParsingMapping +import com.moneymanager.domain.model.csvstrategy.RegexAccountMapping import com.moneymanager.domain.model.csvstrategy.TemplateAccountMapping import com.moneymanager.domain.model.csvstrategy.TransferField import com.moneymanager.test.database.DbTest @@ -177,9 +178,16 @@ class BuiltInCsvStrategyInstallTest : DbTest() { assertEquals("Change", conversion.sideAmountColumn) assertEquals(conversion.debitPattern, conversion.creditPattern, "both dust legs share one Operation") - // Fiat and crypto funding split to the two accounts the API strategy also creates. + // The Operation column routes every row's counterparty, and the funding rules survive as + // unidentified placeholders — which is what lets a deposit reconcile against the API's + // record of it, since the API names the on-chain address the export cannot. val target = strategy.fieldMappings[TransferField.TARGET_ACCOUNT] - assertIs(target) + assertIs(target) + assertEquals("Operation", target.columnName) + assertTrue( + target.rules.first { it.accountName == "Binance Funding" }.counterpartyIsUnidentified, + "a deposit/withdrawal counterparty is a placeholder, not an identity", + ) val amount = strategy.fieldMappings[TransferField.AMOUNT] assertIs(amount) diff --git a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt index b5dc81348..d2a481b44 100644 --- a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt +++ b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt @@ -152,8 +152,10 @@ class BinanceCsvE2ETest : DbTest() { } @Test - fun depositAndWithdrawal_splitFiatFromCryptoFundingLikeTheApiDoes() = + fun depositAndWithdrawal_bookAgainstThePlaceholderTheApiAlsoFallsBackTo() = runTest { + // The export never says whose account the money came from, so both fiat and crypto funding + // land on "Binance Funding" — the same name the API uses when it has no address either. val file = stage( "deposits.csv", @@ -165,12 +167,68 @@ class BinanceCsvE2ETest : DbTest() { ) applyAll(listOf(file)) - assertEquals("-500", balanceOf("Binance Bank", "GBP"), "fiat funding books against the bank account") - assertEquals("-0.4", balanceOf("Binance Funding", "BTC"), "crypto funding books against the funding account") + assertEquals("-500", balanceOf("Binance Funding", "GBP")) + assertEquals("-0.4", balanceOf("Binance Funding", "BTC")) assertEquals("500", balanceOf("Binance", "GBP")) assertEquals("0.4", balanceOf("Binance", "BTC")) } + @Test + fun theExplicitlyFiatOperationsUseTheBankPlaceholder() = + runTest { + val file = + stage( + "fiat.csv", + listOf( + row("2023-01-02 03:04:05", "Fiat Deposit", "GBP", "500.00"), + row("2023-01-05 03:04:05", "Fiat Withdrawal", "GBP", "-100.00"), + ), + ) + applyAll(listOf(file)) + + assertEquals("-400", balanceOf("Binance Bank", "GBP")) + assertEquals("400", balanceOf("Binance", "GBP")) + } + + @Test + fun aDepositTheApiAlreadyRecordedAgainstAWalletIsNotCountedTwice() = + runTest { + // The API books a crypto deposit against the on-chain address it came from. The CSV cannot + // name that address, so its counterparty is a placeholder — and being marked unidentified is + // what lets the engine reconcile the two instead of adding a second 0.5 BTC. + val binanceId = repositories.importEngine.createAccount(account("Binance"), Source.Manual) + val walletId = repositories.importEngine.createAccount(account("BTC:39pm1RoWPkVuSyd2gNGRz"), Source.Manual) + repositories.importEngine.createCrypto("BTC", "Bitcoin", Source.Manual) + val btc = assertNotNull(repositories.cryptoRepository.getCryptoAssetByCode("BTC").first()) + repositories.importEngine.import( + com.moneymanager.importengineapi.ImportBatch( + transfers = + listOf( + com.moneymanager.importengineapi.ImportTransfer( + rowKey = + com.moneymanager.importengineapi.ImportRowKey + .Manual(1), + fromAccount = + com.moneymanager.importengineapi.AccountRef + .Existing(walletId), + toAccount = + com.moneymanager.importengineapi.AccountRef + .Existing(binanceId), + source = Source.Manual, + timestamp = Instant.parse("2023-01-03T03:04:05Z"), + description = "Deposit BTC", + amount = Money.fromDisplayValue(BigDecimal("0.5"), btc), + ), + ), + ), + ) + + val file = stage("dup-deposit.csv", listOf(row("2023-01-03 03:04:05", "Deposit", "BTC", "0.5"))) + applyAll(listOf(file)) + + assertEquals("0.5", balanceOf("Binance", "BTC"), "the deposit is counted once, not twice") + } + @Test fun aMultiFillOrder_becomesOneTradeAndOneFeeTransfer() = runTest { diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt index 1094772f2..e9349875e 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt @@ -49,6 +49,13 @@ import kotlinx.serialization.Serializable * is a DEBIT if this column parses negative and a CREDIT if positive; a * row that parses to zero or unparseably is not a conversion leg. When * null the patterns alone decide, as before. + * @property reconcileWindowSeconds When set, a conversion group another source already recorded as + * trades is matched on its debit legs and not imported again. Kept + * separate from the strategy's `crossSourceReconcileWindowSeconds` + * for the same reason as `TradeGroupConfig.reconcileWindowSeconds`: + * that window must tolerate a bank's settlement lag, while two + * sources agree about a conversion's instant to within seconds. + * Null disables it. */ @Serializable data class ConversionConfig( @@ -68,6 +75,8 @@ data class ConversionConfig( // change the canonical hash of every existing strategy - only one that actually sets it rehashes. @EncodeDefault(EncodeDefault.Mode.NEVER) val sideAmountColumn: String? = null, + @EncodeDefault(EncodeDefault.Mode.NEVER) + val reconcileWindowSeconds: Long? = null, ) { init { require(conversionAccountName != null || conversionAccountRules.isNotEmpty()) { diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt index 9303c8716..cabef4e91 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt @@ -46,6 +46,14 @@ import kotlinx.serialization.Serializable * @property descriptionTemplate Description given to the assembled trade. `{from}` and `{to}` are * substituted with the debited and credited asset codes. Cosmetic only: * a trade's identity never includes its description. + * @property reconcileWindowSeconds When set, an assembled trade that another source already recorded — + * as one trade, or as the individual fills this group aggregates — is + * not booked again. Deliberately **not** the strategy's + * `crossSourceReconcileWindowSeconds`: that window has to be wide + * enough for a bank's settlement lag, and a wide window here would + * pull a later order's fills into the candidate set and stop the sums + * matching at all. Sources disagree about a trade's instant only by + * sub-second rounding, so keep this to a few seconds. Null disables it. */ @Serializable data class TradeGroupConfig( @@ -55,6 +63,7 @@ data class TradeGroupConfig( val sideAmountColumn: String? = null, val groupingWindowSeconds: Long = 0, val descriptionTemplate: String = "Buy {to}/{from}", + val reconcileWindowSeconds: Long? = null, ) { init { require(groupingWindowSeconds >= 0) { "TradeGroupConfig.groupingWindowSeconds must not be negative" } diff --git a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt index 63afdce1a..dce9a15fb 100644 --- a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt +++ b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt @@ -143,7 +143,16 @@ object BuiltInCsvStrategies { * versions of the same deposit/withdrawal/earn movement reconcilable. */ private const val BINANCE_FEES_ACCOUNT = "Binance Fees" + + /** + * Placeholder counterparty for a deposit or withdrawal. The export records that money arrived or + * left but never whose account it was, so this account is not an assertion — it is where an + * unidentified movement waits to be reconciled against a source that does name the counterparty + * (the API books a crypto deposit against the on-chain address, falling back to this same name). + */ private const val BINANCE_FUNDING_ACCOUNT = "Binance Funding" + + /** Same placeholder role as [BINANCE_FUNDING_ACCOUNT], for the explicitly fiat operations. */ private const val BINANCE_BANK_ACCOUNT = "Binance Bank" private const val BINANCE_EARN_ACCOUNT = "Binance Earn" private const val BINANCE_EARN_REWARDS_ACCOUNT = "Binance Earn Rewards" @@ -185,12 +194,26 @@ object BuiltInCsvStrategies { private const val BINANCE_TRADING_ACCOUNT = "Binance Trading" /** - * Cross-source reconciliation window for the Binance CSV strategy. The CSV and the API describe the - * same event with the same UTC second and differ only in sub-second precision, so this needs to be - * small; and Binance pays `Staking Rewards` in the same amount every day, so a wide window would - * start pairing genuinely distinct reward rows. Five minutes is far below that daily cadence. + * Cross-source reconciliation window for the Binance CSV strategy's **transfers**. Sized by fiat + * funding, which is the only thing the two sources disagree about by more than a moment: the API + * records when Binance credited the deposit, the export when the transfer was initiated, and in + * this user's data that gap runs from 1 second to about 7 minutes (an hour recovers 19 of 25 such + * rows; widening to a day recovers no more). + * + * It is not wider than that because Binance pays `Staking Rewards` in the same amount every day: + * at an hour, 65 of 9,303 reward rows have an identical-amount sibling in range, but at a day + * 4,088 do — a window that size would start pairing genuinely distinct rewards. */ - private const val BINANCE_RECONCILE_WINDOW_SECONDS = 300L + private const val BINANCE_RECONCILE_WINDOW_SECONDS = 3600L + + /** + * Reconciliation window for Binance **trades and dust sweeps**, which is a different question from + * the transfer window above: the two sources agree about a conversion's instant to the second and + * differ only in sub-second rounding. It stays tight because trade reconciliation matches a group + * against the whole in-window candidate set — at an hour, a later order's fills would be dragged in + * and the sums would stop matching. + */ + private const val BINANCE_TRADE_RECONCILE_WINDOW_SECONDS = 5L /** * Window for pairing a dust sweep's debit legs to its credit legs. Binance stamps a sweep's rows @@ -1521,6 +1544,13 @@ object BuiltInCsvStrategies { * folds each such group into one `trade`. Fee rows stay out of the group on purpose — a `trade` row * has no fee field — and route to [BINANCE_FEES_ACCOUNT] as their own transfers, as the API does. * + * **Known limitation — withdrawals do not reconcile.** A `Withdraw` row remarked + * "Withdraw fee is included" is the **gross** amount, while the API records the withdrawal net and + * books the fee as its own transfer. Cross-source reconciliation matches on the amount, so the two + * never pair and such a withdrawal is counted twice if both sources are imported. The export gives + * no way to recover the fee, so nothing here can fix it; the affected rows are the ones carrying + * that remark, plus `Fiat Withdrawal`. + * * Dust sweeps are the one conversion that cannot be assembled: a sweep debits several assets and * credits several BNB amounts, and nothing in the file says which credit came from which debit * (their order does not correspond, and the credited amount is net of Binance's service charge @@ -1534,11 +1564,22 @@ object BuiltInCsvStrategies { // also claim "Fiat Deposit" and a bare "Buy" would claim "Transaction Buy". val targetAccountRules = listOf( - // Fiat funding is the API's fiat/orders endpoints (Binance Bank); crypto funding is - // capital/deposit|withdraw (Binance Funding). The Coin column decides which, so the - // fiat rules match on it and the crypto rules catch the rest. - RegexRule(pattern = "^(Fiat Deposit|Fiat Withdrawal)$", accountName = BINANCE_BANK_ACCOUNT), - RegexRule(pattern = "^(Deposit|Withdraw)$", accountName = BINANCE_FUNDING_ACCOUNT), + // The export says money arrived or left, never whose account it was: a deposit row names + // no bank and no wallet. The API does know — it books a crypto deposit against the + // on-chain address it came from ("ETH:0x9b4f…") and only falls back to Binance Funding + // when there is none. So the counterparty here is a *placeholder*: marking it + // unidentified lets the engine reconcile the row against the API's record of the same + // movement whatever counterparty that record names, instead of double-counting it. + RegexRule( + pattern = "^(Fiat Deposit|Fiat Withdrawal)$", + accountName = BINANCE_BANK_ACCOUNT, + counterpartyIsUnidentified = true, + ), + RegexRule( + pattern = "^(Deposit|Withdraw)$", + accountName = BINANCE_FUNDING_ACCOUNT, + counterpartyIsUnidentified = true, + ), RegexRule( pattern = "^Simple Earn (Flexible|Locked) (Subscription|Redemption)$", accountName = BINANCE_EARN_ACCOUNT, @@ -1597,24 +1638,10 @@ object BuiltInCsvStrategies { rules = listOf(RegexRule(pattern = "^", accountName = BINANCE_ACCOUNT)), ), TransferField.TARGET_ACCOUNT to - ConditionalAccountMapping( + RegexAccountMapping( fieldType = TransferField.TARGET_ACCOUNT, - conditions = listOf(RowCondition("Coin", RowConditionOperator.EQUALS_VALUE, value = "GBP")), - whenTrue = - RegexAccountMapping( - fieldType = TransferField.TARGET_ACCOUNT, - columnName = "Operation", - rules = - listOf( - RegexRule(pattern = "^(Deposit|Withdraw)$", accountName = BINANCE_BANK_ACCOUNT), - ) + targetAccountRules, - ), - whenFalse = - RegexAccountMapping( - fieldType = TransferField.TARGET_ACCOUNT, - columnName = "Operation", - rules = targetAccountRules, - ), + columnName = "Operation", + rules = targetAccountRules, ), TransferField.TIMESTAMP to DateTimeParsingMapping( @@ -1664,6 +1691,7 @@ object BuiltInCsvStrategies { conversionAccountName = BINANCE_CONVERSIONS_ACCOUNT, pairingWindowSeconds = BINANCE_CONVERSION_PAIRING_WINDOW_SECONDS, relationshipTypeName = "conversion", + reconcileWindowSeconds = BINANCE_TRADE_RECONCILE_WINDOW_SECONDS, ), tradeGroupConfig = TradeGroupConfig( @@ -1678,6 +1706,7 @@ object BuiltInCsvStrategies { groupingWindowSeconds = 0L, // Matches the API importer's "Buy BASE/QUOTE" wording for the same conversion. descriptionTemplate = "Buy {to}/{from}", + reconcileWindowSeconds = BINANCE_TRADE_RECONCILE_WINDOW_SECONDS, ), createdAt = now, updatedAt = now, From 43991f2545510cb612b16b914a71c3a8c56794e6 Mon Sep 17 00:00:00 2001 From: Nikolay Metchev Date: Mon, 31 Aug 2026 12:12:18 +0300 Subject: [PATCH 5/6] style: fix the Qodana findings on the Binance strategy Four unresolved KDoc links (a reference to a config field that was dropped during review, and three CsvImportStrategy properties named from a function's KDoc where they are not in scope) and eight arguments that just restate a default. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UijVfE192UWTLiAjx3mvVL --- .../moneymanager/csvimporter/CsvImportApplier.kt | 2 +- .../csvimporter/BinanceCsvMapperTest.kt | 4 ++-- .../csvimporter/CsvTradeGroupsTest.kt | 8 ++++---- .../domain/model/csvstrategy/TradeGroupConfig.kt | 7 +++---- .../moneymanager/builtin/BuiltInCsvStrategies.kt | 16 ++++++++-------- 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt index bf17ba397..6e13109d0 100644 --- a/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt +++ b/app/csvimporter/src/commonMain/kotlin/com/moneymanager/csvimporter/CsvImportApplier.kt @@ -1055,7 +1055,7 @@ suspend fun runCsvImport( tradeDedupePolicy = strategy.tradeGroupConfig ?.reconcileWindowSeconds - ?.let { TradeDedupePolicy.Fuzzy(window = it.seconds, allowAggregation = true) } + ?.let { TradeDedupePolicy.Fuzzy(window = it.seconds) } ?: TradeDedupePolicy.ExactTupleOnly, dedupePolicy = if (uniqueIdTypeNames.isEmpty()) { diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt index 1fa601271..9ad4c444f 100644 --- a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/BinanceCsvMapperTest.kt @@ -200,8 +200,8 @@ class BinanceCsvMapperTest { @Test fun timestampIsParsedAsUtc() { - val r = map(row("Deposit", "GBP", "1.00", time = "2023-01-02 03:04:05")) - assertEquals("2023-01-02T03:04:05Z", r.transfer.timestamp.toString()) + val r = map(row("Deposit", "GBP", "1.00", time = "2021-06-14 23:59:58")) + assertEquals("2021-06-14T23:59:58Z", r.transfer.timestamp.toString()) } @Test diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt index 978dc1d8e..e0ab57c96 100644 --- a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt @@ -30,8 +30,8 @@ class CsvTradeGroupsTest { debitPattern = "^(Sell|Transaction (Spend|Sold))$", creditPattern = "^(Buy|Transaction (Buy|Revenue))$", sideAmountColumn = "Change", - groupingWindowSeconds = 0, - descriptionTemplate = "Buy {to}/{from}", + // groupingWindowSeconds and descriptionTemplate keep their defaults (0 / "Buy {to}/{from}"), + // which is what the Binance strategy uses. ) private val binance = AccountId(1) @@ -124,8 +124,8 @@ class CsvTradeGroupsTest { fun distinctTimestamps_makeDistinctGroups() { val rows = listOf( - leg(TradeLegSide.DEBIT, "1.0", eth, at = "2022-11-14T20:32:54Z"), - leg(TradeLegSide.CREDIT, "0.03", btc, at = "2022-11-14T20:32:54Z"), + leg(TradeLegSide.DEBIT, "1.0", eth, at = "2022-11-14T20:31:00Z"), + leg(TradeLegSide.CREDIT, "0.03", btc, at = "2022-11-14T20:31:00Z"), leg(TradeLegSide.DEBIT, "2.0", eth, at = "2022-11-14T20:39:53Z"), leg(TradeLegSide.CREDIT, "0.06", btc, at = "2022-11-14T20:39:53Z"), ) diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt index cabef4e91..a60a9237a 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt @@ -19,10 +19,9 @@ import kotlinx.serialization.Serializable * When set on a [CsvImportStrategy], the importer buckets matching rows by timestamp (widened by * [groupingWindowSeconds]), and for each bucket whose debits name exactly one asset and whose credits * name exactly one other asset emits a single trade — owner account on both sides, debit sum out, - * credit sum in. Fee rows in the bucket become their own transfers to [feeAccountName], because a - * `trade` row carries no fee field. A bucket that does not resolve — no credits, an empty side, or - * more than one asset on a side — is left alone and its rows import as ordinary transfers to whatever - * account the strategy's mappings chose, so no row is ever dropped and the residue is visible. + * credit sum in. A bucket that does not resolve — no credits, an empty side, or more than one asset on + * a side — is left alone and its rows import as ordinary transfers to whatever account the strategy's + * mappings chose, so no row is ever dropped and the residue is visible. * * A `trade` row carries no fee field, so fee rows are deliberately **not** part of this config: leave * them out of both patterns and let the strategy's ordinary account routing book them as their own diff --git a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt index dce9a15fb..41a39ca8f 100644 --- a/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt +++ b/app/strategies/src/commonMain/kotlin/com/moneymanager/builtin/BuiltInCsvStrategies.kt @@ -1529,7 +1529,7 @@ object BuiltInCsvStrategies { * an older `Operation` vocabulary (`Savings purchase` for `Simple Earn Flexible Subscription`, * `POS savings interest` for `Staking Rewards`, `Super BNB Mining` for `BNB Vault Rewards`, …) plus * `LD*` mirror rows the modern format dropped. Importing both would book the same event twice under - * two different descriptions, so [contentMatchRules] requires the `User_ID` column: a legacy file + * two different descriptions, so [CsvImportStrategy.contentMatchRules] requires the `User_ID` column: a legacy file * scores zero and, because this strategy carries content rules, is also excluded from the * no-signals fallback, so it resolves to no strategy and is reported skipped rather than misread. * Re-export the same period from Binance to import it. @@ -1540,7 +1540,8 @@ object BuiltInCsvStrategies { * dual savings have no API endpoint at all and are the reason to import this file. * * Trades are split across rows: Binance stamps every partial fill of both legs with the same second - * (a single order can produce a dozen `Transaction Sold`/`Transaction Revenue` rows). [tradeGroupConfig] + * (a single order can produce a dozen `Transaction Sold`/`Transaction Revenue` rows). + * [CsvImportStrategy.tradeGroupConfig] * folds each such group into one `trade`. Fee rows stay out of the group on purpose — a `trade` row * has no fee field — and route to [BINANCE_FEES_ACCOUNT] as their own transfers, as the API does. * @@ -1554,7 +1555,8 @@ object BuiltInCsvStrategies { * Dust sweeps are the one conversion that cannot be assembled: a sweep debits several assets and * credits several BNB amounts, and nothing in the file says which credit came from which debit * (their order does not correspond, and the credited amount is net of Binance's service charge - * while the debited amount is gross). They go through [conversionConfig] instead, which keeps every + * while the debited amount is gross). They go through [CsvImportStrategy.conversionConfig] instead, + * which keeps every * balance exact without inventing a pairing. Both legs share one `Operation`, so * [ConversionConfig.sideAmountColumn] classifies them by the sign of `Change`. */ @@ -1701,11 +1703,9 @@ object BuiltInCsvStrategies { // "Transaction Related" is the older name for *either* leg of a fill, so the sign of // Change - not the operation name - has to decide which side each row is. sideAmountColumn = "Change", - // Every leg of one fill carries the identical second, and distinct orders are - // seconds-to-days apart, so no jitter needs tolerating. - groupingWindowSeconds = 0L, - // Matches the API importer's "Buy BASE/QUOTE" wording for the same conversion. - descriptionTemplate = "Buy {to}/{from}", + // groupingWindowSeconds and descriptionTemplate keep their defaults: every leg of + // one fill carries the identical second so no jitter needs tolerating, and the + // default "Buy {to}/{from}" already matches the API importer's wording. reconcileWindowSeconds = BINANCE_TRADE_RECONCILE_WINDOW_SECONDS, ), createdAt = now, From 97cd30094f084c781b02b61dd3106e9b8dd7c478 Mon Sep 17 00:00:00 2001 From: Nikolay Metchev Date: Mon, 31 Aug 2026 13:01:02 +0300 Subject: [PATCH 6/6] fix(csv): reject a negative reconciliation window Raised in review. A negative window would not have disabled reconciliation, it would have defeated it silently: every check compares a non-negative absolute time difference against it, so no candidate could ever match and a trade or dust sweep another source had already recorded would be booked a second time. Both configs now require a non-negative value, keeping null as the way to turn reconciliation off. Also replaces the fully qualified importengineapi names in the E2E test with explicit imports, per CLAUDE.md. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01UijVfE192UWTLiAjx3mvVL --- .../csvimporter/CsvTradeGroupsTest.kt | 11 ++++++++++ .../database/csv/BinanceCsvE2ETest.kt | 20 +++++++++---------- .../model/csvstrategy/ConversionConfig.kt | 6 ++++++ .../model/csvstrategy/TradeGroupConfig.kt | 6 ++++++ 4 files changed, 32 insertions(+), 11 deletions(-) diff --git a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt index e0ab57c96..156807a97 100644 --- a/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt +++ b/app/csvimporter/src/commonTest/kotlin/com/moneymanager/csvimporter/CsvTradeGroupsTest.kt @@ -14,6 +14,7 @@ import com.moneymanager.domain.model.TransferId import com.moneymanager.domain.model.csvstrategy.TradeGroupConfig import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.time.Instant @@ -218,6 +219,16 @@ class CsvTradeGroupsTest { assertEquals(2, group.rows.size, "fee and other rows stay out of the group and import as transfers") } + @Test + fun aNegativeReconcileWindowIsRejected() { + // A negative window would not disable reconciliation, it would defeat it silently: every check + // compares a non-negative absolute time difference against it, so nothing would ever match and + // already-recorded trades would be booked a second time. Null is how you turn it off. + assertFailsWith { config.copy(reconcileWindowSeconds = -1) } + assertFailsWith { config.copy(groupingWindowSeconds = -1) } + assertEquals(null, config.copy(reconcileWindowSeconds = null).reconcileWindowSeconds) + } + @Test fun noLegsMeansNoGroups() { assertEquals(emptyList(), groupTradeLegs(listOf(nonLeg(), nonLeg()), config)) diff --git a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt index d2a481b44..2e92c1470 100644 --- a/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt +++ b/app/db/core/src/commonTest/kotlin/com/moneymanager/database/csv/BinanceCsvE2ETest.kt @@ -10,6 +10,10 @@ import com.moneymanager.domain.model.AccountId import com.moneymanager.domain.model.Money import com.moneymanager.domain.model.Source import com.moneymanager.domain.model.csv.CsvImport +import com.moneymanager.importengineapi.AccountRef +import com.moneymanager.importengineapi.ImportBatch +import com.moneymanager.importengineapi.ImportRowKey +import com.moneymanager.importengineapi.ImportTransfer import com.moneymanager.importengineapi.createAccount import com.moneymanager.importengineapi.createCrypto import com.moneymanager.importengineapi.createTrade @@ -201,19 +205,13 @@ class BinanceCsvE2ETest : DbTest() { repositories.importEngine.createCrypto("BTC", "Bitcoin", Source.Manual) val btc = assertNotNull(repositories.cryptoRepository.getCryptoAssetByCode("BTC").first()) repositories.importEngine.import( - com.moneymanager.importengineapi.ImportBatch( + ImportBatch( transfers = listOf( - com.moneymanager.importengineapi.ImportTransfer( - rowKey = - com.moneymanager.importengineapi.ImportRowKey - .Manual(1), - fromAccount = - com.moneymanager.importengineapi.AccountRef - .Existing(walletId), - toAccount = - com.moneymanager.importengineapi.AccountRef - .Existing(binanceId), + ImportTransfer( + rowKey = ImportRowKey.Manual(1), + fromAccount = AccountRef.Existing(walletId), + toAccount = AccountRef.Existing(binanceId), source = Source.Manual, timestamp = Instant.parse("2023-01-03T03:04:05Z"), description = "Deposit BTC", diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt index e9349875e..0e41a0623 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/ConversionConfig.kt @@ -82,6 +82,12 @@ data class ConversionConfig( require(conversionAccountName != null || conversionAccountRules.isNotEmpty()) { "ConversionConfig needs a conversionAccountName or at least one conversionAccountRule to route legs through" } + // A negative window would not disable reconciliation, it would silently defeat it: the check + // compares a non-negative absolute time difference against it, so no group could ever match and + // a sweep another source already recorded would be imported again. Null is how you turn it off. + require(reconcileWindowSeconds == null || reconcileWindowSeconds >= 0) { + "ConversionConfig.reconcileWindowSeconds must not be negative" + } } } diff --git a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt index a60a9237a..8a9e99e5b 100644 --- a/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt +++ b/app/model/csvstrategy/src/commonMain/kotlin/com/moneymanager/domain/model/csvstrategy/TradeGroupConfig.kt @@ -66,5 +66,11 @@ data class TradeGroupConfig( ) { init { require(groupingWindowSeconds >= 0) { "TradeGroupConfig.groupingWindowSeconds must not be negative" } + // A negative window would not disable reconciliation, it would silently defeat it: every check + // compares a non-negative absolute time difference against it, so nothing could ever match and + // trades another source already recorded would be booked again. Null is how you turn it off. + require(reconcileWindowSeconds == null || reconcileWindowSeconds >= 0) { + "TradeGroupConfig.reconcileWindowSeconds must not be negative" + } } }