Skip to content

Reduce maximum order size to 50 shares#50

Open
IamJasonBian wants to merge 5 commits into
mainfrom
reduce-lot-size-to-50
Open

Reduce maximum order size to 50 shares#50
IamJasonBian wants to merge 5 commits into
mainfrom
reduce-lot-size-to-50

Conversation

@IamJasonBian

@IamJasonBian IamJasonBian commented Mar 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Changed DEFAULT_LOT_SIZE from 250 to 50 share - BTC is going up!
  • Reduce DCA until we have a growth strat

IamJasonBian and others added 3 commits March 2, 2026 18:12
Changed the default Netlify Blob store from 'order-book' to 'state-logs'
to write state snapshots directly to long-term storage while the UI
manages the cut-over between blob stores.

This allows the UI to handle reading from both old (order-book) and new
(state-logs) blob stores during the transition period without the
archive workflow interfering by moving blobs between stores.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Critical fixes for Pattern Day Trading protection:

1. **Check filled orders, not just open orders** (pdt_gate.py:71-160)
   - OLD: Only checked open orders from today
   - NEW: Checks recent filled stock + option orders (last 1 day)
   - Uses `last_transaction_at` (fill time) not `created_at` (order time)
   - Prevents PDT violations from orders that filled earlier in the day

2. **Add same-day fill cache** (pdt_gate.py:19)
   - Cache format: {symbol: {'buy': date, 'sell': date}}
   - Faster checks after initial API fetch
   - Auto-clears fills from previous days

3. **Track option fills for underlying stocks** (pdt_gate.py:122-154)
   - BUY to open option = BUY underlying (for PDT purposes)
   - SELL to close option = SELL underlying (for PDT purposes)
   - Prevents day trades via options on same underlying

4. **Block paired orders if day trade risk** (main.py:239-255)
   - Before placing paired buy+sell orders, check both sides for PDT risk
   - Skip paired order placement if either side would violate PDT
   - Prevents immediate round trips from paired DCA orders

5. **Clear warning messages** (pdt_gate.py:66-69)
   - "⚠️ WILL CREATE DAY TRADE" when opposite side filled today
   - Shows day trade count (e.g., "0/3 used")
   - Helps traders make informed decisions

Test coverage:
- tests/test_pdt_guard.py: Live data validation
- Verified with real positions: BTC stock + IWN options

Before this fix:
❌ Could sell BTC same day as buy (missed filled order)
❌ Could close IWN options same day (missed option fills)
❌ Paired orders could create instant day trades

After this fix:
✅ Detects BTC buy from today → warns on sell attempt
✅ Detects IWN option opens from today → warns on close attempt
✅ Blocks paired orders if day trade risk detected

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Changed DEFAULT_LOT_SIZE from 250 to 50 to limit order sizes for risk
management. This affects:
- Live strategy order sizing (momentum_dca_strategy.py)
- Backtest simulations
- All paired buy/sell orders

🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

Copy link
Copy Markdown
Owner Author

Code Review — PR #50

Overall: Simple config change — DEFAULT_LOT_SIZE from 250 to 50. Makes sense if BTC price has appreciated and you want to reduce DCA exposure per cycle.

Note

This same change is already included in PR #45's diff (along with the blob store rename and PDT fixes). Merging both will cause a conflict or redundancy. Recommend merging whichever PR lands first and rebasing the other.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Follow-up Review — PR #50

The lot size change itself is fine, but this PR contains significant unrelated changes that need attention:

  1. Title/scope mismatch — The PR is titled "Reduce maximum order size to 50 shares" but includes a full pdt_gate.py rewrite (+107/-10), a pricing formula change, a blob store rename, and a new test file. The actual lot size change is 1 line. Consider splitting or updating the title.

  2. Stop price formula is a silent behavior changestop_price changed from round(current_price * (1 - stop_offset_pct), 2) (percentage-based) to round(current_price + 1.00, 2) (hardcoded $1 above). For BTC, a $1 offset is essentially zero. This needs explicit justification.

  3. Option PDT mapping may be inverted — In _would_create_day_trade, "buy to close" (covering a short) maps to 'buy', but semantically it's a closing action and should map to 'sell' for PDT equivalence. This could produce false negatives on short option PDT detection.

  4. _pdt_gate accessed via getattr with silent None fallback — PDT checking is silently skipped if the attribute is missing. If PDT safety is critical, this should raise an explicit warning rather than failing silently.

  5. test_pdt_guard.py requires live credentials — The file lives in tests/ (pytest-discoverable) but makes real API calls to Robinhood. Running pytest in CI would fail or hit live accounts. Move it to scripts/ or mark with @pytest.mark.skip.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Stale PR notice — This PR has been open for 25 days with no new commits since Mar 18. Two prior reviews flagged a title/scope mismatch (the 1-line lot-size change is bundled with a pdt_gate.py rewrite, a silent stop-price formula change, and a new live-credential test file), plus an overlap with PR #45. Please split the unrelated changes, address the feedback, or close if superseded by #45.


Generated by Claude Code

Copy link
Copy Markdown
Owner Author

Stale — 5 weeks, no activity since Mar 18. The 1-line lot-size change is buried under a pdt_gate.py rewrite and a stop-price formula change that were flagged in two prior reviews. This significantly overlaps with PR #45. Recommend closing this PR and making the lot-size change a standalone 1-line PR, or folding it into #45 after that PR is cleaned up.


Generated by Claude Code

Make open-orders queries account-scoped (rh_open_orders.py, SafeCashBot.get_open_orders, RobinhoodAuth.get_account_info) via RH_AUTOMATED_ACCOUNT_NUMBER; bump README to 0.4.0.
rh_open_orders.py scans every account (cash + margin) and adds an ACCOUNT column so margin orders are shown. Reverts the 0.4.0 account-scoping edits to safe_cash_bot.py/rh_auth.py/README — engine core is back to its pre-0.4.0 state; the standalone read-only script is the only change.

@IamJasonBian IamJasonBian left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Stale — this PR bundles 5+ unrelated changes under a 1-line title.

The actual lot size change is a single line in config.py. The rest of the diff duplicates PR #45's PDT gate rewrite, adds an unrelated rh_open_orders.py script at the repo root, changes blob store naming, and rewrites order replacement pricing logic. See inline comments for specifics.

Recommendation: Close this PR. Cherry-pick the DEFAULT_LOT_SIZE = 50 line into a fresh 1-commit PR. The PDT gate work already lives in PR #45. The other changes should be their own focused PRs.


Generated by Claude Code

Comment thread rh_open_orders.py
@@ -0,0 +1,134 @@
"""Read-only: list open stock orders across ALL accounts (cash + margin) as a table.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Unrelated to lot size change — move or remove.

This 134-line script is a standalone Robinhood order viewer that has nothing to do with reducing DEFAULT_LOT_SIZE. It also lives at the repo root instead of scripts/. If it's needed, it should be a separate PR under scripts/ (and drop the sys.path.insert / os.chdir hacks in favor of running as a module).


Generated by Claude Code

Comment thread tests/test_pdt_guard.py
@@ -0,0 +1,118 @@
"""

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Not a real pytest test — it's a manual script.

This file requires live Robinhood credentials, uses sys.path.insert hacks, prints output instead of asserting, and has a if __name__ == "__main__" block. It will fail in CI without credentials and doesn't test anything deterministically. Either:

  • Convert to proper unit tests with mocked broker calls, or
  • Move to scripts/ as an operational tool (not under tests/)

Also: this exact file already exists in PR #45. The PDT gate changes in this PR are identical to #45 — one of them should be closed to avoid duplicate work.


Generated by Claude Code

Comment thread trading_system/config.py

# Maximum shares per paired order (used by live strategy, backtests, and simulations)
DEFAULT_LOT_SIZE = 250
DEFAULT_LOT_SIZE = 50

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This one-liner is the only change that matches the PR title. The rest of the diff (PDT gate rewrite, rh_open_orders.py, blob store rename, _handle_order_replacement pricing changes) are unrelated. Consider splitting into focused PRs:

  1. Lot size change (this line only)
  2. PDT gate improvements (already in PR Fix PDT gate to detect same-day filled orders (critical PDT protection) #45)
  3. rh_open_orders.py script (its own PR)
  4. Order replacement pricing changes (its own PR)

Generated by Claude Code

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant