Skip to content

fix(report): a sold position keeps its weekly contribution without a closing quote - #107

Merged
amosgeva merged 1 commit into
mainfrom
fix/weekly-closed-position-contribution
Sep 10, 2026
Merged

amosgeva merged 1 commit into
mainfrom
fix/weekly-closed-position-contribution

Conversation

@amosgeva

Copy link
Copy Markdown
Owner

Summary

Re-audit finding N05, remaining branch (round 5, task 29). The collector quotes only symbols with a positive quantity or a watchlist flag, so a position closed during the week normally has no end-of-week quote. The contribution loop skipped every trade without one and zeroed the price move, so a full sale printed $0.00 and a buy-and-sell round trip vanished from the contributor list while the week's total was right.

  • The contribution is now computed from the identity directly: end value − start value + proceeds − outlays, fees once, week's units, trades after the baseline day. An endpoint with no shares is worth zero and needs no quote.
  • A held position that lacks its quote is printed as n/a with the share count and which endpoint is unpriced, and is already named under the missing-price warning; it is never $0.00.
  • The contributor universe is opening positions ∪ closing positions ∪ traded symbols, so round trips stay listed.
  • The price-move / trade-P&L breakdown is still shown when both quotes exist. It is algebraically the same figure as before, so the round-4 cases and amounts are unchanged.
  • CLAUDE.md no longer describes app/tests/ as "only FIFO coverage" (audit maintenance note).

Tests (app/tests/test_report_weekly_split.py, whole main(), real loader, no closing quote for the sold symbol)

Full sale ($100, the audit's case), round trip ($25, stays in the list), a loss with fees (−$53), forward and reverse splits before the sale ($100 / $8), and a held position with a missing end quote (n/a line, the warning names it, no $0.00). Every priced case asserts Σ contributions == Δ(securities + cash). All round-3/4 weekly tests are green (27 in the file).

Smoke-ran the report read-only against production: identical output to 1.7.4 for this week (no positions were sold).

Pre-commit: 310 MCP / 659 app / 89 tools passed.

🤖 Generated with Claude Code

…ithout a closing quote

The collector stops quoting a symbol once it is sold, so a position closed
during the week normally has no end-of-week quote. The contribution
arithmetic skipped every trade without one, so a full sale printed $0.00
and a buy-and-sell round trip vanished from the contributor list while the
week's total was right (1.7.4 re-audit, N05 remaining branch).

The contribution is now computed from the identity directly,
end value - start value + proceeds - outlays, where an endpoint with no
shares is worth zero and needs no quote. A held position that lacks its
quote is printed as n/a (and is already named under the missing-price
warning), never as $0.00. The price-move / trade-P&L breakdown is still
shown when both quotes exist; it is algebraically the same figure, so the
round-4 cases are unchanged.

Tests (whole main(), real loader, no closing quote for the sold symbol):
full sale, round trip, a loss with fees, forward and reverse splits before
the sale, and a held position with a missing end quote showing n/a; each
priced case asserts sum(contributions) == delta(securities + cash).

Also: CLAUDE.md no longer says app/tests/ is "only FIFO coverage".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 7 complexity · 0 duplication

Metric Results
Complexity 7
Duplication 0

View in Codacy

AI Reviewer: run a review on demand. To trigger the first review automatically, go to your organization or repository integration settings. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@amosgeva
amosgeva merged commit f2f659a into main Sep 10, 2026
8 checks passed
@amosgeva amosgeva mentioned this pull request Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant