Repository navigation
fix: symbol-case position lookup, shadow order ids, Alpaca short sign and zero fills, bar windows - #112
Closed
quantum-like wants to merge 5 commits into
Closed
fix: symbol-case position lookup, shadow order ids, Alpaca short sign and zero fills, bar windows#112quantum-like wants to merge 5 commits into
quantum-like wants to merge 5 commits into
Conversation
SafeBroker and the strategy wrapper looked up positions with the caller's original casing, while adapters store AAPL-style keys. A lowercase close therefore returned no position and left the holding open. Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
Shadow fills used a millisecond timestamp as the order id. Two orders accepted in the same millisecond shared an id, so later reconciliation overwrote the first fill. Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
Alpaca position snapshots report an unsigned share count and a separate long/short side. Treating qty as a signed quantity stored shorts as longs, so a later close sold more instead of buying to cover. Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
A fill event with a zero or missing filled_qty was rewritten to the full order size. That recorded a complete fill the venue had not reported. Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
Bar starts were truncated with minute-of-hour arithmetic. A size that does not divide 60, including 90-minute bars, restarted every hour and split one window into two. Co-authored-by: quantum-like <quantum-like@users.noreply.github.com>
This was referenced Oct 17, 2026
Contributor
|
The five repairs from this contribution are retained and completed in the combined maintenance PR #121, including the original quantum-like/Cursor Agent credit and the failing-base/passing-fix evidence. Closing this PR as superseded avoids duplicate integration work; issue #111 remains open and is linked for closure when #121 actually merges. The original contributor branch is preserved. This closure does not claim the repairs have landed. |
stefan-jansen
added a commit
that referenced
this pull request
Oct 18, 2026
Integrate the broker, aggregation and guide repairs as one maintainer-directed batch. Original repairs were contributed by quantum-like in PR #112. Preserve the public attribution and retained original commit history. Closes #111. Closes #114. Closes #116. Closes #117. Closes #118. Closes #119. Closes #120. Closes #126. Closes #128. Closes #132. Provider qualification and the separate publication decision remain tracked by #92.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Owning issue
Closes #111
Outcome
5 correctness fixes, one commit each, each with a focused test:
close_position("aapl")left anAAPLposition open. Lookups now use the same uppercase symbol as the broker adapters. Test:tests/unit/test_safe_broker.py::test_position_lookup_and_close_ignore_symbol_case,::test_external_position_lookup_ignores_symbol_case. (f95d914)tests/unit/test_safe_broker.py::test_shadow_order_ids_stay_unique_when_the_clock_does_not_advance. (bd6a6a2)qtyplus a separate side, so short positions were stored as longs and a later close sold into the short. Shorts are now stored with a negative quantity. Test:tests/unit/test_alpaca_broker.py::test_sync_positions_signs_short_holdings. (dc84667)filled_qty0/absent was recorded as a complete fill. It is now rejected. Test:tests/unit/test_alpaca_broker.py::test_fill_event_does_not_invent_a_quantity. (e5a518f)tests/unit/test_bar_aggregator.py::test_bar_windows_do_not_restart_at_the_hour. (8fd83f5)Compatibility
No API changes. Behavior changes only in the cases above.
Verification
mainand passes with its fix)ruff format --checkis clean on the changed files andruff checkreports nothing new in them versusmain, but I did not run the full pre-commit suite)typasses (not run on this branch)pytest: 1050 passed, 30 skipped before; 1056 passed, 30 skipped after. The skipped tests need external/paper/stress/benchmark flags and were left off, so no broker or live feed was touched.Documentation and release
No documentation changes. The behavior changes above may be worth a line in the next patch-release notes.
Amir