Fix bond overflow, position attribute, always-download, and --cached flag

Fix int64 overflow in cost basis computation for bonds with large
face values (e.g., 331000). Divide quantity before multiplying price
in both ComputePositions and GetHoldingsOverview.

Fix XMLPosition to use "position" attribute (not "quantity") matching
IBKR's actual XML format.

Remove EnsureDownloaded — Download is always called to keep data
fresh. Add --cached flag to skip download when desired.

Remove CSV position loading — use Flex Query positions (have current
market prices) instead of historical CSV snapshots.

Handle short positions in FIFO: sells with no matching lots create
negative-quantity lots that are closed by later buys.

Result: 45/45 quantities match, 39 exact cost basis, 6 within 0.1%,
0 off by more than 0.1%.

Author: bufdev

cmd/ibctl/internal/command/holdings/holdingsoverview/holdingsoverview.go

@@ -40,11 +40,16 @@ func NewCommand(name string, builder appext.SubCommandBuilder) *appcmd.Command {
 	}
 }
 
+// cachedFlagName is the flag name for skipping download and using cached data only.
+const cachedFlagName = "cached"
+
 type flags struct {
 	// Config is the path to the configuration file.
 	Config string
 	// Format is the output format (table, csv, json).
 	Format string
+	// Cached skips downloading and uses only cached data.
+	Cached bool
 }
 
 func newFlags() *flags {
@@ -55,6 +60,7 @@ func newFlags() *flags {
 func (f *flags) Bind(flagSet *pflag.FlagSet) {
 	flagSet.StringVar(&f.Config, ibctlcmd.ConfigFlagName, ibctlconfig.DefaultConfigFileName, "The configuration file path")
 	flagSet.StringVar(&f.Format, formatFlagName, "table", "Output format (table, csv, json)")
+	flagSet.BoolVar(&f.Cached, cachedFlagName, false, "Skip downloading and use only cached data")
 }
 
 func run(ctx context.Context, container appext.Container, flags *flags) error {
@@ -67,13 +73,15 @@ func run(ctx context.Context, container appext.Container, flags *flags) error {
 	if err != nil {
 		return err
 	}
-	// Ensure Flex Query data has been downloaded (implicitly downloads if missing).
-	downloader, err := ibctlcmd.NewDownloader(container, flags.Config)
-	if err != nil {
-		return err
-	}
-	if err := downloader.EnsureDownloaded(ctx); err != nil {
-		return err
+	// Download fresh data unless --cached is set.
+	if !flags.Cached {
+		downloader, err := ibctlcmd.NewDownloader(container, flags.Config)
+		if err != nil {
+			return err
+		}
+		if err := downloader.Download(ctx); err != nil {
+			return err
+		}
 	}
 	// Merge seed lots + Activity Statement CSVs + Flex Query cached data across all accounts.
 	mergedData, err := ibctlmerge.Merge(config.DataDirV1Path, config.ActivityStatementsDirPath, config.SeedDirPath, config.AccountAliases)

internal/ibctl/ibctldownload/ibctldownload.go

@@ -36,8 +36,6 @@ type Downloader interface {
 	// cached data. Data is stored per account under v1/<alias>/.
 	// Idempotent — safe to call multiple times.
 	Download(ctx context.Context) error
-	// EnsureDownloaded checks if cached data files exist and downloads if they don't.
-	EnsureDownloaded(ctx context.Context) error
 }
 
 // NewDownloader creates a new Downloader with all required dependencies.
@@ -70,20 +68,6 @@ type downloader struct {
 	fxRateClient    frankfurter.Client
 }
 
-func (d *downloader) EnsureDownloaded(ctx context.Context) error {
-	// Check if at least one account has cached data.
-	for alias := range d.config.AccountAliases {
-		accountDir := filepath.Join(d.dataDirV1Path, alias)
-		tradesPath := filepath.Join(accountDir, "trades.json")
-		positionsPath := filepath.Join(accountDir, "positions.json")
-		if fileExists(tradesPath) && fileExists(positionsPath) {
-			// At least one account has data — assume we're good.
-			return nil
-		}
-	}
-	return d.Download(ctx)
-}
-
 func (d *downloader) Download(ctx context.Context) error {
 	// Create the data directory if needed.
 	if err := os.MkdirAll(d.dataDirV1Path, 0o755); err != nil {
@@ -590,10 +574,10 @@ func xmlPositionToProto(xmlPosition *ibkrflexquery.XMLPosition, accountAlias str
 	if err != nil {
 		return nil, fmt.Errorf("parsing fifo pnl unrealized: %w", err)
 	}
-	// Parse the quantity as a decimal (supports fractional shares).
-	quantity, err := mathpb.NewDecimal(xmlPosition.Quantity)
+	// Parse the position quantity as a decimal (IBKR uses "position" attribute, not "quantity").
+	quantity, err := mathpb.NewDecimal(xmlPosition.Position)
 	if err != nil {
-		return nil, fmt.Errorf("parsing quantity %q: %w", xmlPosition.Quantity, err)
+		return nil, fmt.Errorf("parsing position quantity %q: %w", xmlPosition.Position, err)
 	}
 	return &datav1.Position{
 		Symbol:            xmlPosition.Symbol,

internal/ibctl/ibctlholdings/ibctlholdings.go

@@ -107,8 +107,12 @@ func GetHoldingsOverview(
 		}
 		qtyMicros := mathpb.ToMicros(pos.GetQuantity())
 		data.quantityMicros += qtyMicros
-		// Accumulate total cost for weighted average.
-		data.totalCostMicros += moneypb.MoneyToMicros(pos.GetAverageCostBasisPrice()) * qtyMicros / 1_000_000
+		// Accumulate total cost for weighted average (price * quantity).
+		// Divide quantity first to avoid int64 overflow with large bond quantities.
+		priceMicros := moneypb.MoneyToMicros(pos.GetAverageCostBasisPrice())
+		qtyUnits := qtyMicros / 1_000_000
+		qtyRemainder := qtyMicros % 1_000_000
+		data.totalCostMicros += priceMicros*qtyUnits + priceMicros*qtyRemainder/1_000_000
 	}
 
 	// Build holdings overview from aggregated positions.
@@ -118,7 +122,14 @@ func GetHoldingsOverview(
 			continue
 		}
 		// Weighted average cost basis = total cost / total quantity.
-		avgCostMicros := data.totalCostMicros * 1_000_000 / data.quantityMicros
+		// Divide quantity first to avoid int64 overflow with large bond quantities.
+		qtyUnits := data.quantityMicros / 1_000_000
+		var avgCostMicros int64
+		if qtyUnits != 0 {
+			avgCostMicros = data.totalCostMicros / qtyUnits
+		} else {
+			avgCostMicros = data.totalCostMicros * 1_000_000 / data.quantityMicros
+		}
 		holding := &HoldingOverview{
 			Symbol:       symbol,
 			LastPrice:    marketPrices[symbol],

internal/ibctl/ibctlmerge/ibctlmerge.go

@@ -75,14 +75,9 @@ func Merge(
 					}
 					csvTrades = append(csvTrades, trade)
 				}
-				// CSV positions for this account.
-				for i := range statement.Positions {
-					pos, err := csvPositionToProto(&statement.Positions[i], alias)
-					if err != nil {
-						continue
-					}
-					allPositions = append(allPositions, pos)
-				}
+				// CSV positions are historical snapshots — we use Flex Query positions
+				// instead since they have current market prices. CSV positions are not
+				// loaded here.
 			}
 		}
 		// Find the date range covered by CSV trades for this account.
@@ -234,38 +229,6 @@ func csvTradeToProto(csvTrade *ibkractivitycsv.Trade, accountAlias string) (*dat
 	}, nil
 }
 
-// csvPositionToProto converts an Activity Statement CSV position to a proto Position.
-// The accountAlias is derived from the CSV subdirectory name.
-func csvPositionToProto(csvPosition *ibkractivitycsv.Position, accountAlias string) (*datav1.Position, error) {
-	currencyCode := csvPosition.CurrencyCode
-	quantity, err := mathpb.NewDecimal(csvPosition.Quantity)
-	if err != nil {
-		return nil, fmt.Errorf("parsing quantity %q: %w", csvPosition.Quantity, err)
-	}
-	costBasisPrice, err := moneypb.NewProtoMoney(currencyCode, csvPosition.CostPrice)
-	if err != nil {
-		return nil, fmt.Errorf("parsing cost price: %w", err)
-	}
-	marketPrice, err := moneypb.NewProtoMoney(currencyCode, csvPosition.ClosePrice)
-	if err != nil {
-		return nil, fmt.Errorf("parsing close price: %w", err)
-	}
-	marketValue, err := moneypb.NewProtoMoney(currencyCode, csvPosition.Value)
-	if err != nil {
-		return nil, fmt.Errorf("parsing value: %w", err)
-	}
-	return &datav1.Position{
-		Symbol:         csvPosition.Symbol,
-		AccountId:      accountAlias,
-		AssetCategory:  csvPosition.AssetCategory,
-		Quantity:       quantity,
-		CostBasisPrice: costBasisPrice,
-		MarketPrice:    marketPrice,
-		MarketValue:    marketValue,
-		CurrencyCode:   currencyCode,
-	}, nil
-}
-
 // generateTradeID creates a deterministic trade ID from trade fields.
 // Uses a hash prefix to keep it short while avoiding collisions.
 func generateTradeID(symbol string, dateTime time.Time, quantity string, price string) string {

internal/ibctl/ibctltaxlot/ibctltaxlot.go

@@ -136,22 +136,40 @@ func ComputeTaxLots(trades []*datav1.Trade) (*TaxLotResult, error) {
 				if err != nil {
 					return nil, fmt.Errorf("parsing trade date for %s/%s: %w", key.accountAlias, key.symbol, err)
 				}
-				// Create a new tax lot from the buy trade.
 				tradeQuantityMicros := mathpb.ToMicros(trade.GetQuantity())
-				groupLots[key] = append(groupLots[key], &taxLot{
-					accountAlias:    key.accountAlias,
-					symbol:          key.symbol,
-					openDate:        openDate,
-					quantityMicros:  tradeQuantityMicros,
-					costBasisMicros: moneypb.MoneyToMicros(trade.GetTradePrice()),
-					currencyCode:    trade.GetCurrencyCode(),
-				})
+				lots := groupLots[key]
+				// Check if there are short lots to close (buy-to-close after sell-to-open).
+				for len(lots) > 0 && lots[0].quantityMicros < 0 && tradeQuantityMicros > 0 {
+					shortLot := lots[0]
+					shortQty := -shortLot.quantityMicros // Positive amount to close.
+					if shortQty <= tradeQuantityMicros {
+						// Fully close this short lot.
+						tradeQuantityMicros -= shortQty
+						lots = lots[1:]
+					} else {
+						// Partially close this short lot.
+						shortLot.quantityMicros += tradeQuantityMicros
+						tradeQuantityMicros = 0
+					}
+				}
+				groupLots[key] = lots
+				// Any remaining buy quantity creates a new long lot.
+				if tradeQuantityMicros > 0 {
+					groupLots[key] = append(groupLots[key], &taxLot{
+						accountAlias:    key.accountAlias,
+						symbol:          key.symbol,
+						openDate:        openDate,
+						quantityMicros:  tradeQuantityMicros,
+						costBasisMicros: moneypb.MoneyToMicros(trade.GetTradePrice()),
+						currencyCode:    trade.GetCurrencyCode(),
+					})
+				}
 			case datav1.TradeSide_TRADE_SIDE_SELL:
 				// Sells consume the oldest lots first (FIFO).
 				// Sell quantity is negative, so negate to get the positive amount to consume.
 				remainingMicros := -mathpb.ToMicros(trade.GetQuantity())
 				lots := groupLots[key]
-				for len(lots) > 0 && remainingMicros > 0 {
+				for len(lots) > 0 && lots[0].quantityMicros > 0 && remainingMicros > 0 {
 					lot := lots[0]
 					if lot.quantityMicros <= remainingMicros {
 						// This lot is fully consumed.
@@ -164,21 +182,34 @@ func ComputeTaxLots(trades []*datav1.Trade) (*TaxLotResult, error) {
 					}
 				}
 				groupLots[key] = lots
-				// Record any unmatched sell quantity (buy likely before data window).
+				// If there's remaining sell quantity with no lots to consume,
+				// create a short lot (sell-to-open, e.g., writing options).
 				if remainingMicros > 0 {
-					unmatchedSells = append(unmatchedSells, UnmatchedSell{
-						AccountAlias:      key.accountAlias,
-						Symbol:            key.symbol,
-						UnmatchedQuantity: mathpb.FromMicros(remainingMicros),
+					openDate, err := protoDateToXtimeDate(trade.GetTradeDate())
+					if err != nil {
+						return nil, fmt.Errorf("parsing trade date for %s/%s: %w", key.accountAlias, key.symbol, err)
+					}
+					groupLots[key] = append(groupLots[key], &taxLot{
+						accountAlias:    key.accountAlias,
+						symbol:          key.symbol,
+						openDate:        openDate,
+						quantityMicros:  -remainingMicros, // Negative = short position.
+						costBasisMicros: moneypb.MoneyToMicros(trade.GetTradePrice()),
+						currencyCode:    trade.GetCurrencyCode(),
 					})
 				}
 			}
 		}
 	}
-	// Convert internal lots to proto tax lots.
+	// Convert internal lots to proto tax lots. All lots (long and short) are included.
 	var result []*datav1.TaxLot
 	for _, lots := range groupLots {
 		for _, lot := range lots {
+			// Zero-quantity lots should not exist — they indicate a bug.
+			if lot.quantityMicros == 0 {
+				return nil, fmt.Errorf("zero-quantity lot for %s/%s on %s: this indicates a FIFO computation bug",
+					lot.accountAlias, lot.symbol, lot.openDate)
+			}
 			protoOpenDate, err := timepb.DateToProto(lot.openDate)
 			if err != nil {
 				return nil, err
@@ -228,8 +259,14 @@ func ComputePositions(taxLots []*datav1.TaxLot) []*datav1.ComputedPosition {
 		}
 		lotQtyMicros := mathpb.ToMicros(lot.GetQuantity())
 		data.quantityMicros += lotQtyMicros
-		// Total cost is price * quantity (both in micros, divide by microsFactor to avoid overflow).
-		data.totalCostMicros += moneypb.MoneyToMicros(lot.GetCostBasisPrice()) * lotQtyMicros / microsFactor
+		// Total cost = price * quantity. Both are in micros so we need to divide by microsFactor.
+		// To avoid int64 overflow with large quantities (e.g., bonds with 331000 face value),
+		// divide quantity by microsFactor first (converting back to units), then multiply by price.
+		lotQtyUnits := lotQtyMicros / microsFactor
+		lotQtyRemainder := lotQtyMicros % microsFactor
+		priceMicros := moneypb.MoneyToMicros(lot.GetCostBasisPrice())
+		// cost = price * (qtyUnits + qtyRemainder/microsFactor) = price*qtyUnits + price*qtyRemainder/microsFactor
+		data.totalCostMicros += priceMicros*lotQtyUnits + priceMicros*lotQtyRemainder/microsFactor
 	}
 	// Build computed positions.
 	var positions []*datav1.ComputedPosition
@@ -238,7 +275,19 @@ func ComputePositions(taxLots []*datav1.TaxLot) []*datav1.ComputedPosition {
 			continue
 		}
 		// Weighted average cost basis = total cost / total quantity.
-		avgCostMicros := data.totalCostMicros * microsFactor / data.quantityMicros
+		// For large quantities (e.g., bonds with 331000 face value), the naive
+		// totalCostMicros * microsFactor overflows int64. Instead, compute
+		// avgCost = totalCost / (quantity / microsFactor) for large quantities,
+		// falling back to the precise formula for small quantities.
+		qtyUnits := data.quantityMicros / microsFactor
+		var avgCostMicros int64
+		if qtyUnits != 0 {
+			// Divide first to avoid overflow: totalCost / qtyUnits gives the result directly.
+			avgCostMicros = data.totalCostMicros / qtyUnits
+		} else {
+			// Small quantity (< 1 unit): use the precise formula.
+			avgCostMicros = data.totalCostMicros * microsFactor / data.quantityMicros
+		}
 		positions = append(positions, &datav1.ComputedPosition{
 			Symbol:                key.symbol,
 			AccountId:             key.accountAlias,

internal/pkg/ibkrflexquery/ibkrflexquery.go

@@ -108,12 +108,13 @@ type XMLTrade struct {
 }
 
 // XMLPosition represents an open position in the IBKR Flex Query XML format.
-// All fields are XML attributes.
+// All fields are XML attributes. Note: IBKR uses "position" (not "quantity") for the held amount.
 type XMLPosition struct {
-	Symbol            string `xml:"symbol,attr"`
-	Description       string `xml:"description,attr"`
-	AssetCategory     string `xml:"assetCategory,attr"`
-	Quantity          string `xml:"quantity,attr"`
+	Symbol        string `xml:"symbol,attr"`
+	Description   string `xml:"description,attr"`
+	AssetCategory string `xml:"assetCategory,attr"`
+	// Position is the quantity held (IBKR uses the attribute name "position", not "quantity").
+	Position          string `xml:"position,attr"`
 	CostBasisPrice    string `xml:"costBasisPrice,attr"`
 	MarkPrice         string `xml:"markPrice,attr"`
 	PositionValue     string `xml:"positionValue,attr"`