Skip to content

Fix account-currency fee rules and OCC option multipliers - #40

Open
vinay-veerappa wants to merge 1 commit into
LuxAlgo:mainfrom
vinay-veerappa:pr/fee-asset-class-occ-multiplier
Open

vinay-veerappa wants to merge 1 commit into
LuxAlgo:mainfrom
vinay-veerappa:pr/fee-asset-class-occ-multiplier

Conversation

@vinay-veerappa

Copy link
Copy Markdown

What

Two defects in how manual fills were valued.

Fees. A fee rule matched on account and symbol only, so the one unscoped futures commission rule (.31/contract/side) charged every zero-fee fill: equities that trade free were charged .31 per SHARE (833 shares, .23) and an equity option the futures rate. FeeRule gains an optional �ssetClass (absent = any); a fill with no asset class matches only unscoped rules. defaultFee takes the fill's asset class from both callers, the settings validator accepts the known asset classes only, and the settings form offers the choice.

Multiplier. P&L is priceDiff x qty x multipliers[symbol], falling back to 1, so an OCC option symbol with no table entry booked 1/100 of its P&L.
esolveMultiplier (core) returns the table entry, else 100 for an OCC equity-option symbol, else 1; the round-trip builder and the trade reader both use it so they cannot disagree. Futures options are not OCC-formatted and still need a table entry.

Verification

  • packages/core/tests/multipliers.test.ts: OCC parse (space-padded root, C/P, strike x 1000), table win, futures fall-through.
  • �pps/web/tests/journal-defaults-validation.test.ts: validator rejects unknown asset classes, accepts the known ones.
  • �pps/web/tests/journal-features.test.ts: fee rule scoped by asset class picks the right rule.
  • Full suite: 60 files / 572 tests green.

Co-Authored-By: Claude noreply@anthropic.com

Two defects in how manual fills were valued.

Fees. A fee rule matched on account and symbol only, so the one unscoped
futures commission rule ($0.31/contract/side) charged every zero-fee fill:
equities that trade free were charged $0.31 per SHARE (833 shares, $258.23)
and an equity option the futures rate. FeeRule gains an optional assetClass
("" or absent = any); a fill with no asset class matches only unscoped rules.
defaultFee takes the fill's asset class from both callers, the settings
validator accepts the known asset classes only, and the settings form offers
the choice.

Multiplier. P&L is priceDiff x qty x multipliers[symbol], falling back to 1,
so an OCC option symbol with no table entry booked 1/100 of its P&L.
resolveMultiplier (core) returns the table entry, else 100 for an OCC
equity-option symbol, else undefined; the round-trip builder and the trade
reader both use it so they cannot disagree. Futures options are not OCC
formatted and still need a table entry.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
const context = () => ({ multipliers: getMultipliers(), defaults: getJournalDefaults() });
export const rowToTrade = (row: TradeRow, config = context()): AnnotatedTrade => {
const multiplier = config.multipliers[row.symbol];
const multiplier = resolveMultiplier(row.symbol, config.multipliers);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Recalculate existing option trades when enabling the inferred multiplier

Previously saved option trades without a configured multiplier retain their materialized gross/net P&L at ×1, while this reader immediately reports a multiplier of 100. I reproduced this by saving two CRDO option contracts sold at 7.80 and bought back at 3.80 with the base-branch calculation: after switching to this PR, the list API reports contractMultiplier: 100 but still returns $8 profit instead of $800. Reimporting the same fills leaves the $8 unchanged because duplicates do not trigger rebuildAccount; explicitly rebuilding corrects it to $800. Please add a one-time recalculation of affected accounts on upgrade, preserving annotations, and a regression test for previously saved trades so P&L, ROI, and breakeven classification use consistent values.

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.

2 participants