Skip to content

Extend securities filters to also include short positions#61

Open
financelurker wants to merge 2 commits into
ma4nn:masterfrom
financelurker:master
Open

Extend securities filters to also include short positions#61
financelurker wants to merge 2 commits into
ma4nn:masterfrom
financelurker:master

Conversation

@financelurker

@financelurker financelurker commented May 31, 2026

Copy link
Copy Markdown

This change allows the "view securities" command to also show short positions in your repository.

I know, short positions in Portfolio Performance are a bit wonky, but still used by many people.

snapshot.shares now returns also negative amounts; view securities is adapted, values()/VAP/share-sell are out of scope and can be inplausible (needs a separate change).

@financelurker

Copy link
Copy Markdown
Author

Not quite sure how to solve the build issue... Poetry gives me errors (wants to install packages in the system, despite having a venv in the repository) and make does not work either (selinux errors, I am on Fedora)...

(I have installed it as editable source-module, and this way it works...)

@sonarqubecloud

sonarqubecloud Bot commented Jun 9, 2026

Copy link
Copy Markdown

@ma4nn

ma4nn commented Jun 9, 2026

Copy link
Copy Markdown
Owner

@financelurker thanks, I have consolidated the new tests into the existing ones and build now works fine 👍

The only thing I am still struggling is:
PortfolioSnapshot.shares is consumed by several callers beyond view securities. The switch from > 0 to != 0 means shorts now flow into:

  1. commands/simulate_share_sell.py:70holdings = snapshot.shares then iterates and runs FIFO. A short position would now be iterated and fed into enrich_fifo_lots; likely to misbehave or hit sell_strategy.py:44's clip(lower=0, ...). Needs either an explicit filter at the call site or a regression test.
  2. domain/vap.py:73,141_calculate_min_shares clamps via max(0, …) so shorts collapse to 0 effective. Probably fine, but worth one assertion.
  3. snapshot.values — now produces negative balances for shorts; any aggregate using it gets reduced by the short's market value. Intentional?

What's your take on these points?

@ma4nn ma4nn added the enhancement New feature or request label Jun 9, 2026
@ma4nn

ma4nn commented Jul 9, 2026

Copy link
Copy Markdown
Owner

@financelurker any thoughts on this ☝️?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants