Skip to content

Commit 9fe7ffa

Browse files
bufdevbufdev
committed
fix(possible-sale): make safe, safe-excluded, unsafe mutually exclusive
GetUnsafeSellList now mirrors the safe FIFO walk and only collects lots at and after the stop point (where STCG >= max_stcg). Lots before the stop point are already in the safe or safe-excluded tables. Excluded symbols are skipped entirely in the unsafe list since they appear in safe-excluded. Previously, excluded symbols' STCG lots appeared in both safe-excluded and unsafe, double-counting quantity.
1 parent 4ea9c51 commit 9fe7ffa

File tree

2 files changed

+270
-30
lines changed

2 files changed

+270
-30
lines changed

‎internal/ibctl/ibctlpossiblesale/ibctlpossiblesale.go‎

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,6 @@ func GetSafeSellList(
340340
break
341341
}
342342
// Track whether this group has at least one qualifying lot (loss, LTCG, or tax-exempt).
343-
// Tax-exempt lots have zero P&L but are still real positions that should appear in output.
344343
if pnl.PnLBaseMicros < 0 || pnl.LTCGMicros > 0 || lot.GetTaxExempt() {
345344
hasQualifying = true
346345
}
@@ -434,32 +433,68 @@ func GetUnsafeSellList(
434433
if err != nil {
435434
return nil, err
436435
}
437-
// For each group, collect lots with positive STCG.
436+
maxSTCGMicros := config.PossibleSaleMaxSTCGMicros
437+
// Unsafe is the complement of safe. For each symbol, replay the safe FIFO
438+
// walk to find the stop point and whether safe qualified. Unsafe collects:
439+
// - Lots at and after the stop point (safe didn't reach them).
440+
// - ALL lots if the group didn't qualify for safe (safe discarded them).
441+
// The exclude list is irrelevant here — unsafe shows everything that isn't safe.
438442
var results []*PossibleSaleOverview
439443
for key, lots := range ctx.groupedLots {
440444
symbol := key.symbol
441445
pd := ctx.positionMap[symbol]
442446
var lastPriceMicros int64
443447
var isBond bool
444448
if pd != nil {
445-
// Use exported fields from ibctltaxlot.PositionData.
446449
lastPriceMicros = pd.LastPriceMicros
447450
isBond = pd.IsBond
448451
}
452+
// Sort lots by open date for FIFO walk (same order as safe walk).
453+
sortLotsByDate(lots)
449454
// Compute total quantity across all lots (for QTY REMAINING calculation).
450455
var groupTotalQtyMicros int64
451456
for _, lot := range lots {
452457
groupTotalQtyMicros += mathpb.ToMicros(lot.GetQuantity())
453458
}
459+
// Replay the safe FIFO walk to determine the stop point and whether
460+
// the group qualified for safe.
461+
safeQualified := false
462+
stopIndex := len(lots)
463+
for i, lot := range lots {
464+
pnl, pnlErr := ibctltaxlot.ComputeLotPnL(lot, lastPriceMicros, isBond, ctx.fxStore, ctx.baseCurrency, ctx.today, ctx.todayStr)
465+
if pnlErr != nil {
466+
return nil, fmt.Errorf("computing lot P&L for unsafe-sell: %w", pnlErr)
467+
}
468+
if pnl.STCGMicros > 0 && pnl.STCGMicros >= maxSTCGMicros {
469+
stopIndex = i
470+
break
471+
}
472+
if pnl.PnLBaseMicros < 0 || pnl.LTCGMicros > 0 || lot.GetTaxExempt() {
473+
safeQualified = true
474+
}
475+
}
476+
// Determine which lots are unsafe:
477+
// - If safe qualified: lots at and after the stop point.
478+
// - If safe did NOT qualify: ALL lots (safe discarded the whole group).
479+
unsafeStartIndex := stopIndex
480+
if !safeQualified {
481+
unsafeStartIndex = 0
482+
}
483+
if unsafeStartIndex >= len(lots) {
484+
// All lots were claimed by safe — nothing for unsafe.
485+
continue
486+
}
454487
var (
455-
totalQtyMicros int64
456-
totalSTCGMicros int64
457-
totalMktValMicros int64
458-
totalCostNative int64
459-
currency string
460-
stcgLots []*datav1.TaxLot // Collect STCG lots for LTCG DATE computation.
488+
totalQtyMicros int64
489+
totalPnLBaseMicros int64
490+
totalSTCGMicros int64
491+
totalLTCGBaseMicros int64
492+
totalMktValMicros int64
493+
totalCostNative int64
494+
currency string
495+
unsafeLots []*datav1.TaxLot
461496
)
462-
for _, lot := range lots {
497+
for _, lot := range lots[unsafeStartIndex:] {
463498
lotCurrency := lot.GetCurrencyCode()
464499
if currency == "" {
465500
currency = lotCurrency
@@ -468,15 +503,12 @@ func GetUnsafeSellList(
468503
if pnlErr != nil {
469504
return nil, fmt.Errorf("computing lot P&L for unsafe-sell: %w", pnlErr)
470505
}
471-
// Only include lots with positive STCG (short-term gains, not losses).
472-
if pnl.STCGMicros <= 0 {
473-
continue
474-
}
475506
lotQtyMicros := mathpb.ToMicros(lot.GetQuantity())
476507
totalQtyMicros += lotQtyMicros
508+
totalPnLBaseMicros += pnl.PnLBaseMicros
477509
totalSTCGMicros += pnl.STCGMicros
510+
totalLTCGBaseMicros += pnl.LTCGMicros
478511
totalMktValMicros += pnl.MktValBaseMicros
479-
// Accumulate native cost for weighted average price computation.
480512
costMicros := moneypb.MoneyToMicros(lot.GetCostBasisPrice())
481513
lotQtyUnits := lotQtyMicros / ibctltaxlot.MicrosFactor
482514
lotQtyRemainder := lotQtyMicros % ibctltaxlot.MicrosFactor
@@ -485,11 +517,7 @@ func GetUnsafeSellList(
485517
lotCostNative /= 100
486518
}
487519
totalCostNative += lotCostNative
488-
stcgLots = append(stcgLots, lot)
489-
}
490-
// Skip groups with no positive STCG exposure.
491-
if totalSTCGMicros <= 0 {
492-
continue
520+
unsafeLots = append(unsafeLots, lot)
493521
}
494522
// Compute weighted average cost basis per share in native currency.
495523
var avgPriceMicros int64
@@ -506,14 +534,15 @@ func GetUnsafeSellList(
506534
Symbol: symbol,
507535
Quantity: mathpb.FromMicros(totalQtyMicros),
508536
QuantityRemaining: mathpb.FromMicros(remainingQtyMicros),
509-
LTCGDate: computeLTCGDate(stcgLots, ctx.today),
510-
Currency: currency,
511-
AveragePrice: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(currency, avgPriceMicros)),
512-
MarketPrice: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(currency, lastPriceMicros)),
513-
MktValBase: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(baseCurrency, totalMktValMicros)),
514-
PnLBase: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(baseCurrency, totalSTCGMicros)),
515-
STCGBase: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(baseCurrency, totalSTCGMicros)),
516-
LTCGBase: "0",
537+
// Compute LTCG date from the unsafe lots (when they become long-term).
538+
LTCGDate: computeLTCGDate(unsafeLots, ctx.today),
539+
Currency: currency,
540+
AveragePrice: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(currency, avgPriceMicros)),
541+
MarketPrice: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(currency, lastPriceMicros)),
542+
MktValBase: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(baseCurrency, totalMktValMicros)),
543+
PnLBase: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(baseCurrency, totalPnLBaseMicros)),
544+
STCGBase: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(baseCurrency, totalSTCGMicros)),
545+
LTCGBase: moneypb.MoneyValueToString(moneypb.MoneyFromMicros(baseCurrency, totalLTCGBaseMicros)),
517546
}
518547
if separateAccounts {
519548
overview.Account = key.accountAlias

‎internal/ibctl/ibctlpossiblesale/ibctlpossiblesale_test.go‎

Lines changed: 213 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,221 @@ func unionSymbolSets(sets ...map[string]struct{}) map[string]struct{} {
216216
return result
217217
}
218218

219+
// TestSafePlusExcludedPlusUnsafe_MutuallyExclusive verifies that every lot
220+
// appears in exactly one of the three tables, and that the sum of market values
221+
// across all three equals the lot-level total (what holding list shows minus cash).
222+
//
223+
// This test covers the edge cases that caused double-counting and missing symbols:
224+
// - Multi-lot symbol split across safe and unsafe (FIFO stop point)
225+
// - Excluded symbol with both LTCG and STCG lots
226+
// - Symbol with all STCG below threshold but no qualifying lots (loss/LTCG)
227+
// - Symbol entirely at a loss (all safe)
228+
// - Symbol entirely LTCG (all safe)
229+
func TestSafePlusExcludedPlusUnsafe_MutuallyExclusive(t *testing.T) {
230+
t.Parallel()
231+
now := time.Now()
232+
// Helper to create a buy date N days ago.
233+
daysAgo := func(n int) *timev1.Date {
234+
t := now.AddDate(0, 0, -n)
235+
return mustPossibleSaleProtoDate(nil, t.Year(), t.Month(), t.Day())
236+
}
237+
// Use a wrapper since mustPossibleSaleProtoDate needs *testing.T.
238+
daysAgoDate := func(n int) *timev1.Date {
239+
tm := now.AddDate(0, 0, -n)
240+
d, _ := timepb.NewProtoDate(tm.Year(), tm.Month(), tm.Day())
241+
return d
242+
}
243+
_ = daysAgo // suppress unused
244+
245+
makeTrade := func(id, symbol string, buyDate *timev1.Date, qty, price int64) *datav1.Trade {
246+
return &datav1.Trade{
247+
TradeId: id, AccountAlias: "individual",
248+
TradeDate: buyDate, SettleDate: buyDate,
249+
Symbol: symbol, Side: datav1.TradeSide_TRADE_SIDE_BUY,
250+
Quantity: mathpb.FromMicros(qty),
251+
TradePrice: moneypb.MoneyFromMicros("USD", price),
252+
Proceeds: moneypb.MoneyFromMicros("USD", 0),
253+
Commission: moneypb.MoneyFromMicros("USD", 0),
254+
CurrencyCode: "USD", AssetCategory: "STK",
255+
}
256+
}
257+
makePosition := func(symbol string, qty, mktPrice int64) *datav1.Position {
258+
return &datav1.Position{
259+
Symbol: symbol, AccountAlias: "individual", AssetCategory: "STK",
260+
Quantity: mathpb.FromMicros(qty),
261+
CostBasisPrice: moneypb.MoneyFromMicros("USD", mktPrice),
262+
MarketPrice: moneypb.MoneyFromMicros("USD", mktPrice),
263+
MarketValue: moneypb.MoneyFromMicros("USD", mktPrice*qty/ibctltaxlot.MicrosFactor),
264+
CurrencyCode: "USD",
265+
}
266+
}
267+
268+
// Current market price for all symbols: $200/share.
269+
const mktPrice = 200_000_000
270+
271+
trades := []*datav1.Trade{
272+
// SPLIT: 2 lots. First bought 400d ago at $150 (LTCG gain, safe).
273+
// Second bought 20d ago at $150 (STCG gain >= threshold, unsafe).
274+
makeTrade("split-1", "SPLIT", daysAgoDate(400), 10_000_000, 150_000_000),
275+
makeTrade("split-2", "SPLIT", daysAgoDate(20), 5_000_000, 150_000_000),
276+
277+
// EXCL: excluded symbol. 1 lot bought 400d ago at $150 (LTCG, safe-excluded).
278+
// 1 lot bought 20d ago at $150 (STCG gain, unsafe — exclude list doesn't affect unsafe).
279+
makeTrade("excl-1", "EXCL", daysAgoDate(400), 10_000_000, 150_000_000),
280+
makeTrade("excl-2", "EXCL", daysAgoDate(20), 5_000_000, 150_000_000),
281+
282+
// SMGAIN: 1 lot bought 30d ago at $190 (STCG gain of $10/share = $100 total,
283+
// below max_stcg of 0). No loss or LTCG, so safe doesn't qualify.
284+
// Should appear in unsafe.
285+
makeTrade("smgain-1", "SMGAIN", daysAgoDate(30), 10_000_000, 190_000_000),
286+
287+
// ALLLOSS: 1 lot bought 30d ago at $250 (STCG loss). All safe.
288+
makeTrade("allloss-1", "ALLLOSS", daysAgoDate(30), 10_000_000, 250_000_000),
289+
290+
// ALLLTCG: 1 lot bought 500d ago at $100 (LTCG gain). All safe.
291+
makeTrade("allltcg-1", "ALLLTCG", daysAgoDate(500), 10_000_000, 100_000_000),
292+
}
293+
294+
positions := []*datav1.Position{
295+
makePosition("SPLIT", 15_000_000, mktPrice),
296+
makePosition("EXCL", 15_000_000, mktPrice),
297+
makePosition("SMGAIN", 10_000_000, mktPrice),
298+
makePosition("ALLLOSS", 10_000_000, mktPrice),
299+
makePosition("ALLLTCG", 10_000_000, mktPrice),
300+
}
301+
302+
cfg := &ibctlconfig.Config{
303+
// EXCL is excluded from safe.
304+
PossibleSaleExcludeSymbols: map[string]struct{}{"EXCL": {}},
305+
PossibleSaleExcludeTypes: map[string]struct{}{},
306+
// max_stcg=0: any positive STCG stops the safe walk.
307+
PossibleSaleMaxSTCGMicros: 0,
308+
SymbolConfigs: map[string]ibctlconfig.SymbolConfig{},
309+
AdditionLastPrices: map[string]int64{},
310+
}
311+
fxStore := ibctlfxrates.NewStore(t.TempDir())
312+
313+
// Get all three views.
314+
safe, err := GetSafeSellList(trades, positions, cfg, fxStore, "USD", false, false)
315+
require.NoError(t, err)
316+
excluded, err := GetSafeSellList(trades, positions, cfg, fxStore, "USD", false, true)
317+
require.NoError(t, err)
318+
unsafe, err := GetUnsafeSellList(trades, positions, cfg, fxStore, "USD", false)
319+
require.NoError(t, err)
320+
321+
// Build maps of symbol → qty and symbol → mktval per table.
322+
type entry struct {
323+
qty int64
324+
mktVal int64
325+
}
326+
toMap := func(rows []*PossibleSaleOverview) map[string]entry {
327+
m := make(map[string]entry)
328+
for _, r := range rows {
329+
e := m[r.Symbol]
330+
e.qty += mathpb.ToMicros(r.Quantity)
331+
e.mktVal += mathpb.ParseMicros(r.MktValBase)
332+
m[r.Symbol] = e
333+
}
334+
return m
335+
}
336+
safeMap := toMap(safe)
337+
exclMap := toMap(excluded)
338+
unsafeMap := toMap(unsafe)
339+
340+
// Verify each symbol appears in the expected table(s).
341+
// SPLIT: 10 shares safe (LTCG lot), 5 shares unsafe (STCG lot).
342+
require.Contains(t, safeMap, "SPLIT", "SPLIT LTCG lot should be in safe")
343+
require.Contains(t, unsafeMap, "SPLIT", "SPLIT STCG lot should be in unsafe")
344+
require.NotContains(t, exclMap, "SPLIT", "SPLIT should not be in excluded")
345+
require.Equal(t, int64(10_000_000), safeMap["SPLIT"].qty, "SPLIT safe qty")
346+
require.Equal(t, int64(5_000_000), unsafeMap["SPLIT"].qty, "SPLIT unsafe qty")
347+
348+
// EXCL: 10 shares safe-excluded (LTCG lot), 5 shares unsafe (STCG lot).
349+
require.Contains(t, exclMap, "EXCL", "EXCL LTCG lot should be in safe-excluded")
350+
require.Contains(t, unsafeMap, "EXCL", "EXCL STCG lot should be in unsafe")
351+
require.NotContains(t, safeMap, "EXCL", "EXCL should not be in safe")
352+
require.Equal(t, int64(10_000_000), exclMap["EXCL"].qty, "EXCL excluded qty")
353+
require.Equal(t, int64(5_000_000), unsafeMap["EXCL"].qty, "EXCL unsafe qty")
354+
355+
// SMGAIN: all 10 shares unsafe (STCG gain, no qualifying lots for safe).
356+
require.Contains(t, unsafeMap, "SMGAIN", "SMGAIN should be in unsafe")
357+
require.NotContains(t, safeMap, "SMGAIN", "SMGAIN should not be in safe")
358+
require.NotContains(t, exclMap, "SMGAIN", "SMGAIN should not be in excluded")
359+
require.Equal(t, int64(10_000_000), unsafeMap["SMGAIN"].qty, "SMGAIN unsafe qty")
360+
361+
// ALLLOSS: all 10 shares safe (loss).
362+
require.Contains(t, safeMap, "ALLLOSS", "ALLLOSS should be in safe")
363+
require.NotContains(t, unsafeMap, "ALLLOSS", "ALLLOSS should not be in unsafe")
364+
require.Equal(t, int64(10_000_000), safeMap["ALLLOSS"].qty, "ALLLOSS safe qty")
365+
366+
// ALLLTCG: all 10 shares safe (LTCG).
367+
require.Contains(t, safeMap, "ALLLTCG", "ALLLTCG should be in safe")
368+
require.NotContains(t, unsafeMap, "ALLLTCG", "ALLLTCG should not be in unsafe")
369+
require.Equal(t, int64(10_000_000), safeMap["ALLLTCG"].qty, "ALLLTCG safe qty")
370+
371+
// Verify the summation invariant: for every symbol, the sum of qty across
372+
// all three tables must equal the total holding qty.
373+
expectedQty := map[string]int64{
374+
"SPLIT": 15_000_000,
375+
"EXCL": 15_000_000,
376+
"SMGAIN": 10_000_000,
377+
"ALLLOSS": 10_000_000,
378+
"ALLLTCG": 10_000_000,
379+
}
380+
for sym, expectedQ := range expectedQty {
381+
actualQ := safeMap[sym].qty + exclMap[sym].qty + unsafeMap[sym].qty
382+
require.Equal(t, expectedQ, actualQ, "qty mismatch for %s: safe(%d) + excl(%d) + unsafe(%d) = %d, expected %d",
383+
sym, safeMap[sym].qty, exclMap[sym].qty, unsafeMap[sym].qty, actualQ, expectedQ)
384+
}
385+
386+
// Verify market value summation at BOTH per-symbol and aggregate level.
387+
// Compute expected per-symbol market values from lot-level analysis.
388+
analysis, err := ibctltaxlot.PrepareLotAnalysis(&ibctltaxlot.LotAnalysisParams{
389+
Trades: trades,
390+
Positions: positions,
391+
AdditionLastPrices: cfg.AdditionLastPrices,
392+
})
393+
require.NoError(t, err)
394+
// Build expected per-symbol market values from lot-level P&L computation.
395+
expectedMktValBySymbol := make(map[string]int64)
396+
for _, lot := range analysis.TaxLotResult.TaxLots {
397+
pd := analysis.PositionMap[lot.GetSymbol()]
398+
if pd == nil {
399+
continue
400+
}
401+
pnl, pnlErr := ibctltaxlot.ComputeLotPnL(lot, pd.LastPriceMicros, pd.IsBond, fxStore, "USD", analysis.Today, analysis.TodayStr)
402+
require.NoError(t, pnlErr)
403+
expectedMktValBySymbol[lot.GetSymbol()] += pnl.MktValBaseMicros
404+
}
405+
// Per-symbol: sum market value across all three tables and compare to lot-level.
406+
for sym, expectedMV := range expectedMktValBySymbol {
407+
actualMV := safeMap[sym].mktVal + exclMap[sym].mktVal + unsafeMap[sym].mktVal
408+
require.Equal(t, expectedMV, actualMV,
409+
"market value mismatch for %s: safe(%d) + excl(%d) + unsafe(%d) = %d, expected %d",
410+
sym, safeMap[sym].mktVal, exclMap[sym].mktVal, unsafeMap[sym].mktVal, actualMV, expectedMV)
411+
}
412+
// Aggregate: sum across all symbols must also match.
413+
var expectedTotalMktVal, actualTotalMktVal int64
414+
for _, mv := range expectedMktValBySymbol {
415+
expectedTotalMktVal += mv
416+
}
417+
for _, rows := range [][]*PossibleSaleOverview{safe, excluded, unsafe} {
418+
for _, r := range rows {
419+
actualTotalMktVal += mathpb.ParseMicros(r.MktValBase)
420+
}
421+
}
422+
require.Equal(t, expectedTotalMktVal, actualTotalMktVal,
423+
"total market value across safe+excluded+unsafe must equal lot-level analysis total")
424+
}
425+
219426
// mustPossibleSaleProtoDate creates a proto Date, failing the test on error.
220427
func mustPossibleSaleProtoDate(t *testing.T, year int, month time.Month, day int) *timev1.Date {
221-
t.Helper()
428+
if t != nil {
429+
t.Helper()
430+
}
222431
d, err := timepb.NewProtoDate(year, month, day)
223-
require.NoError(t, err)
432+
if err != nil && t != nil {
433+
require.NoError(t, err)
434+
}
224435
return d
225436
}

0 commit comments

Comments
 (0)