AliceBlue upgraded to V3#966
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b37553a6e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| aliceblue_login_url = f"https://ant.aliceblueonline.com/?appcode={appcode}" | ||
| return redirect(aliceblue_login_url) |
There was a problem hiding this comment.
Return navigable OAuth response for AliceBlue login start
When authCode is absent this branch returns an external redirect, but the current login UI starts AliceBlue auth with fetch('/aliceblue/callback', { method: 'POST' }) and immediately calls response.json() (see frontend/src/pages/BrokerTOTP.tsx around the submit handler). In that context the 302 is handled inside XHR (not a top-level navigation), which leads to CORS/JSON failure instead of opening the broker login page, so AliceBlue login cannot be initiated from the existing web flow.
Useful? React with 👍 / 👎.
| "BSE CG": "BSECAPITALGOODS", | ||
| "CARBON": "BSECARBONEX", | ||
| "BSE CD": "BSECONSUMERDURABLES", |
There was a problem hiding this comment.
Remove space-sensitive keys from post-normalization index map
This mapping includes keys with embedded spaces (for example BSE CG/BSE CD), but just above it the symbols are normalized with str.replace(' ', ''), so these entries can never match. The affected BSE indices will remain unmapped and be stored under non-canonical names, which breaks downstream symbol resolution that expects the OpenAlgo aliases.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="broker/aliceblue/database/master_contract_db.py">
<violation number="1" location="broker/aliceblue/database/master_contract_db.py:620">
P1: BSE index symbol mappings with spaces won't match after space stripping. Keys like "BSE CG", "BSE CD", "BSE HC", "BSE IT" in the replace dictionary will never match because spaces are stripped from symbols first (line 602). These symbols become "BSECG", "BSECD", etc., causing lookups to fail and leaving symbols unmapped to their OpenAlgo equivalents.</violation>
</file>
<file name="broker/aliceblue/api/data.py">
<violation number="1" location="broker/aliceblue/api/data.py:130">
P1: Missing-token check can be bypassed because `_normalize_token` converts `None` to the string `"None"` which is truthy. Should check `get_token()` result before normalizing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| "LMI250": "BSE250LARGEMIDCAPINDEX", | ||
| "MSL400": "BSE400MIDSMALLCAPINDEX", | ||
| "AUTO": "BSEAUTO", | ||
| "BSE CG": "BSECAPITALGOODS", |
There was a problem hiding this comment.
P1: BSE index symbol mappings with spaces won't match after space stripping. Keys like "BSE CG", "BSE CD", "BSE HC", "BSE IT" in the replace dictionary will never match because spaces are stripped from symbols first (line 602). These symbols become "BSECG", "BSECD", etc., causing lookups to fail and leaving symbols unmapped to their OpenAlgo equivalents.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At broker/aliceblue/database/master_contract_db.py, line 620:
<comment>BSE index symbol mappings with spaces won't match after space stripping. Keys like "BSE CG", "BSE CD", "BSE HC", "BSE IT" in the replace dictionary will never match because spaces are stripped from symbols first (line 602). These symbols become "BSECG", "BSECD", etc., causing lookups to fail and leaving symbols unmapped to their OpenAlgo equivalents.</comment>
<file context>
@@ -598,15 +598,49 @@ def process_aliceblue_indices_csv(path):
+ "LMI250": "BSE250LARGEMIDCAPINDEX",
+ "MSL400": "BSE400MIDSMALLCAPINDEX",
+ "AUTO": "BSEAUTO",
+ "BSE CG": "BSECAPITALGOODS",
+ "CARBON": "BSECARBONEX",
+ "BSE CD": "BSECONSUMERDURABLES",
</file context>
broker/aliceblue/api/data.py
Outdated
| try: | ||
| # Convert symbol to broker format and get token | ||
| br_symbol = get_br_symbol(symbol, exchange) or symbol | ||
| token = self._normalize_token(get_token(symbol, exchange)) |
There was a problem hiding this comment.
P1: Missing-token check can be bypassed because _normalize_token converts None to the string "None" which is truthy. Should check get_token() result before normalizing.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At broker/aliceblue/api/data.py, line 130:
<comment>Missing-token check can be bypassed because `_normalize_token` converts `None` to the string `"None"` which is truthy. Should check `get_token()` result before normalizing.</comment>
<file context>
@@ -173,233 +96,102 @@ def get_websocket(self, force_new=False):
+ try:
+ # Convert symbol to broker format and get token
+ br_symbol = get_br_symbol(symbol, exchange) or symbol
+ token = self._normalize_token(get_token(symbol, exchange))
+
+ if not token:
</file context>
| token = self._normalize_token(get_token(symbol, exchange)) | |
| token = get_token(symbol, exchange) | |
| if not token: | |
| raise Exception(f"Token not found for {symbol} on {exchange}") | |
| token = self._normalize_token(token) |
As well Validate and Fix the raised Cubic Dev Issues |
… and historicals, including WebSocket integration.
AliceBlue upgraded to V3
Summary by cubic
Upgraded the AliceBlue integration to the a3 vendor API across auth, orders, funds, websockets, and market data. Added a BrokerData pipeline for quotes, historicals, and streaming, and fixed PnL by aggregating realized and unrealized totals from positions.
Refactors
apiSecretchecksum; now returnsuserSession(JWT). Authorization is “Bearer ”.userIdfrom Authorization and use JWT-only for WS session invalidation./open-api/od/v1/limitswith JWT; PnL now sums realized and unrealized from positions.Migration
BROKER_API_SECRET(keepBROKER_API_KEYif used for login redirect). Backend computes checksum fromuserId + authCode + apiSecret.userIdprefix).Written for commit 5e42ba0. Summary will update on new commits.