Repository navigation
fix(t212): espacer les appels par endpoint selon les limites officielles (fin des 429) - #96
Conversation
…les (fin des 429) GET /equity/orders est limité à 1 req / 5 s par compte. La lecture des stops était appelée plusieurs fois en quelques secondes par cycle (synchro, cliquet, vente) : 429 à chaque cycle, « Stop fetch error — local stop state preserved » ×16, cliquet travaillant à l'aveugle. - src/t212_rate_limit.py : RateGate (créneaux réservés sous verrou, sûr en multi-thread) + GatedSession(requests.Session) : get/delete/post passent tous par le régulateur. Limites relevées dans la doc officielle (orders, positions, summary, cash, history...). Le régulateur ne fait que retarder : aucun retry, aucun cache, donc aucun changement de sémantique pour les POST d'ordres. - safe_request : respecte Retry-After et pénalise le bucket après un 429. - _get_active_stop_order : un nouvel essai après 429 ; deux 429 restent ERROR (jamais NOT_FOUND : invariant « Failed fetch ≠ empty »). - tests/__init__.py désactive l'attente réelle pour la suite (HTTP mocké). - TRADING212_API_GUIDE.md : tableau des limites + types d'ordres disponibles en réel. Vérifié sur la démo (lecture seule) : 4 lectures successives espacées de 5,4-5,5 s, toutes 200. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| "GET /equity/positions": 1.1, # 1 req / 1 s | ||
| "GET /equity/account/summary": 5.5, # 1 req / 5 s | ||
| "GET /equity/account/cash": 2.2, # 1 req / 2 s | ||
| "GET /equity/history/orders": 10.5, # 6 req / 1 min |
There was a problem hiding this comment.
WARNING: 10.5 s on GET /equity/history/orders stretches two existing polling loops that were tuned for a 2 s cadence
src/t212_executor.py:849 (_confirm_fill) polls this endpoint up to FILL_CONFIRM_ATTEMPTS = 6 with FILL_CONFIRM_DELAY = 2.0, and src/t212_executor.py:343 (get_t212_order_history) paginates up to max_pages = 20.
With the gate in front, one SELL confirmation now costs up to ~63 s instead of ~10 s, and it runs in _execute_sell_order after _release_standing_stop_if_reserved has already cancelled the standing GTC stop (src/t212_executor.py:1719 then :1746). The window where the position has no broker-side protection grows by ~50 s per sale. Pagination worst case is ~200 s per sync_state_from_t212 call, which runs per ticker.
Worth either lowering FILL_CONFIRM_ATTEMPTS/max_pages for these call sites, or reusing a single cached history response for the poll instead of re-hitting the endpoint.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| "GET /equity/account/summary": 5.5, # 1 req / 5 s | ||
| "GET /equity/account/cash": 2.2, # 1 req / 2 s | ||
| "GET /equity/history/orders": 10.5, # 6 req / 1 min | ||
| "POST /equity/orders/stop": 2.2, # 1 req / 2 s |
There was a problem hiding this comment.
SUGGESTION: Doc/code mismatch: limit and stop_limit order endpoints are documented as gated but have no interval
TRADING212_API_GUIDE.md lists POST /equity/orders/stop (et limit, stop_limit) with a 2,2 s spacing, but T212_INTERVALS only defines the /stop bucket. bucket_for() explicitly routes /equity/orders/limit and /equity/orders/stop_limit to their own bucket names, which fall through self._intervals.get(bucket, 0.0) to 0.0 and are therefore never delayed. Since the doc says these order types are available on live accounts, the first limit/stop-limit order added later would be ungated.
| "POST /equity/orders/stop": 2.2, # 1 req / 2 s | |
| "POST /equity/orders/stop": 2.2, # 1 req / 2 s | |
| "POST /equity/orders/limit": 2.2, # 1 req / 2 s | |
| "POST /equity/orders/stop_limit": 2.2, # 1 req / 2 s |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| ): | ||
| return "FOUND", o | ||
| return "NOT_FOUND", None | ||
| if resp.status_code == 429 and attempt == 0: |
There was a problem hiding this comment.
WARNING: The 429 retry misses the 400 + TooManyRequests form that safe_request treats as rate limiting
safe_request (src/t212_executor.py:745) accepts both 429 and 400 with TooManyRequests in the body as the broker's rate-limit signal — and the code comment on post_order_market (src/t212_executor.py:782) documents that T212 uses that form too. This new retry branch only matches resp.status_code == 429, so a 400 TooManyRequests falls straight through to the logger.warning + break and the fetch returns ERROR — the exact Stop fetch error — local stop state preserved outcome this PR is meant to eliminate.
Consider matching both forms here as well, and calling GATE.penalize for each.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| if resp.status_code == 429 and attempt == 0: | ||
| GATE.penalize("GET /equity/orders", retry_after_seconds(resp, 5.5)) | ||
| logger.info("Active stop orders fetch: 429 — nouvel essai après espacement.") | ||
| continue |
There was a problem hiding this comment.
SUGGESTION: The retry's delay is implicit — nothing here sleeps unless _t212_session happens to be a GatedSession
This continue relies entirely on GATE.penalize() plus the gated session re-entering GATE.wait() on the next _t212_session.get(...). If _t212_session is ever swapped (tests patch it with a MagicMock, and tests/test_t212_rate_limit.py does exactly that at line 158), the retry fires instantly with zero delay and re-hits the rate limit.
That also means test_429_then_200_returns_found proves the loop iterates, but not that it actually waits — the gate is never exercised in that test. An explicit time.sleep(retry_after_seconds(resp, 5.5)) here would make the delay local and directly assertable.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| import os | ||
|
|
||
| os.environ.setdefault("T212_RATE_GATE_DISABLED", "1") |
There was a problem hiding this comment.
SUGGESTION: setdefault leaves the gate active if the variable is already set to anything but 1
RateGate._active() (src/t212_rate_limit.py:92) treats the gate as enabled for every value other than the exact string "1". So an environment that already exports T212_RATE_GATE_DISABLED=0 (or true, or an empty string) keeps the pacing on, and every mocked GET /equity/orders in the suite sleeps 5,5 s for real — a silent multi-minute test run rather than a clear failure.
| os.environ.setdefault("T212_RATE_GATE_DISABLED", "1") | |
| os.environ[DISABLE_ENV] = "1" |
(or inline the literal "1" and import DISABLE_ENV).
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Incremental pass over Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files, incremental diff 08ee5cd..466a1a6)
Fix these issues in Kilo Cloud Previous Review Summaries (4 snapshots, latest commit 08ee5cd)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 08ee5cd)Status: 7 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (2 files, incremental diff 806012f..08ee5cd)
Fix these issues in Kilo Cloud Previous review (commit 806012f)Status: 3 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Resolved since the previous review
Files Reviewed (2 files, incremental diff e82e57e..806012f)
Fix these issues in Kilo Cloud Previous review (commit e82e57e)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous review (commit 2d6fe82)Status: 5 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (5 files)
Reviewed by free · Input: 0 · Output: 0 · Cached: 0 |
…t/stop_limit (revue Kilo) - SELL_CONFIRM_ATTEMPTS = 3 : l'historique est limité à 6 req/min (espacement 10,5 s) ; 6 essais coûtaient ~63 s alors que le stop broker est déjà libéré avant la vente. 3 essais ≈ 21 s ; le sleep redondant est supprimé pour SELL (le régulateur espace déjà les lectures). BUY inchangé ; - POST /equity/orders/limit et /stop_limit sont maintenant régulés comme /stop (la doc les liste et ils sont disponibles en réel) ; - tests : intervalles des 4 endpoints d'ordre, confirmation SELL bornée, BUY inchangé. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…Requests Review Kilo #96 : _get_active_stop_order ne testait que le 429 alors que safe_request et post_order_market traitent aussi 400+TooManyRequests comme une limite de débit. Helper _is_rate_limited partagé par les trois sites ; le délai est testé explicitement via GATE.penalize ; T212_RATE_GATE_DISABLED forcé dans tests/__init__.py. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
| url_pos = f"{_get_t212_base_url()}/equity/positions" | ||
| url_hist = f"{_get_t212_base_url()}/equity/history/orders?limit=10&ticker={t212_ticker}" | ||
| for attempt in range(FILL_CONFIRM_ATTEMPTS): | ||
| attempts = FILL_CONFIRM_ATTEMPTS if side == "BUY" else SELL_CONFIRM_ATTEMPTS |
There was a problem hiding this comment.
WARNING: Cutting the SELL confirmation to 3 attempts widens the no-stop gap, it does not close it
_execute_sell_order releases the standing broker stop before the POST (_release_standing_stop_if_reserved). If the POST returns 2xx but the fill is not yet in /equity/history/orders, this function now returns None after ~21 s and the caller (src/t212_executor.py:1750) does logger.error(...) + return — it never calls the emergency re-place that _handle_failed_sell performs for a failed POST. The position is then left with no broker-side stop (GO-gate 2) and a stale stop_order_id in the state until the next cycle's self-heal.
Before this change the same path had ~63 s (6 attempts x 2 s) plus the gate spacing to surface the fill, so shortening the window increases the probability of hitting it. Suggest re-placing the stop at prev_stop_price on the sell_fill is None branch too (the values are already in scope in _execute_sell_order), instead of relying on the next cycle.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| except (requests.exceptions.RequestException, ValueError, TypeError) as e: | ||
| logger.debug(f"Fill confirmation poll error: {e}") | ||
| if attempt < FILL_CONFIRM_ATTEMPTS - 1: | ||
| if attempt < attempts - 1 and side == "BUY": |
There was a problem hiding this comment.
SUGGESTION: The SELL loop now has an implicit, untested dependency on the rate gate for its only spacing
Removing time.sleep for SELL is correct only as long as GATE really spaces GET /equity/history/orders by 10,5 s. If the gate is disabled (T212_RATE_GATE_DISABLED=1) or that bucket ever falls back to interval 0.0, the 3 polls collapse into a single burst in microseconds and _confirm_fill(side="SELL") degenerates to a single read — the confirmation window silently disappears.
A cheap guard that does not re-introduce the long window: sleep a small floor (e.g. min(FILL_CONFIRM_DELAY, ...)) between SELL attempts, or assert T212_INTERVALS[bucket_for("GET", url_hist)] > 0 at the call site so a missing bucket fails loudly instead of silently.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| class TestSellConfirmationStaysShort: | ||
| """Revue Kilo PR #96 : l'espacement de 10,5 s de l'historique ne doit pas rallonger la fenêtre de confirmation.""" | ||
|
|
||
| def test_sell_polls_history_only_a_few_times_and_never_sleeps_on_top_of_the_gate(self): |
There was a problem hiding this comment.
SUGGESTION: This test does not verify the invariant the production comment relies on
_t212_session is patched out, so the test only counts mock calls and proves time.sleep is not called. The claim in _confirm_fill ("l'espacement de 10,5 s entre lectures d'historique est déjà assuré par le régulateur") is never asserted: nothing pins the GET /equity/history/orders interval, nor that the SELL URL (which carries ?limit=10&ticker=...) still maps to that bucket. Renaming a bucket or adding a query-bearing URL variant would break the pacing silently while this test stays green.
Consider adding an assertion that bucket_for("GET", url_hist) == "GET /equity/history/orders" and that T212_INTERVALS[...] >= 10, so the gate the SELL path depends on is covered.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…onfirmée Review Kilo #96 : après 3 essais sans fill confirmé, la position restait sans stop jusqu'au self-heal du cycle suivant. Si la position existe encore (ou état inconnu), le stop est reposé au niveau précédent (helper _replace_released_stop partagé avec _handle_failed_sell). Test explicite du couplage boucle de vente / porte d'historique. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
| ) | ||
| # Le stop a été libéré avant l'ordre : si la position existe encore (ou si on ne sait pas), | ||
| # on le repose tout de suite plutôt que d'attendre le self-heal du cycle suivant. | ||
| if stop_released and prev_stop_price and _position_exists(t212_ticker, headers) is not False: |
There was a problem hiding this comment.
WARNING: _position_exists is a read the repo already knows can be stale for ~30 min — an unconfirmed SELL is exactly the case where the sale may already have filled
On this branch sell_fill is None means unconfirmed, not not executed: the 2xx POST may have filled and only /equity/history/orders is lagging. The comment at src/t212_executor.py:1682 documents that on 2026-09-08 T212 demo served the pre-sale position (and cash) for ~30 min after a confirmed sale, so _position_exists can return True for a position that no longer exists.
In that case _replace_released_stop POSTs a SELL STOP for shares the account no longer holds: the broker refuses it (selling-equity-not-owned), _place_stop_order returns (None, None), and the code logs logger.critical(... position SANS protection ...) — a false CRITICAL for a position that is in fact closed. Worse, in the variants where the POST is accepted you end up with a phantom stop plus state["active_position"]["stop_order_id"] pointing at it, which the ratchet/self-heal then keeps chasing next cycle.
Cross-check the order endpoint (_get_active_stop_order, the source the AGENTS invariants call authoritative for the order side) or treat the unknown branch as reconcile-next-cycle, or at minimum downgrade the unconfirmed path's logger.critical to a warning since the position may be gone.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| save_portfolio_state(state, t212_ticker) | ||
| logger.info( | ||
| f"🔐 Stop de secours re-placé #{re_stop_id} @ {pos_state['stop_price']:.2f} " | ||
| f"après échec de la vente {t212_ticker}." |
There was a problem hiding this comment.
SUGGESTION: Log wording is wrong for the new call site — this is no longer only an "échec de la vente"
_replace_released_stop is now shared between _handle_failed_sell (POST failed — the sale demonstrably did not happen) and the new unconfirmed-fill branch, where the sale may have executed at the broker. During an incident, après échec de la vente and the vente échouée wording in the CRITICAL right below send triage to the wrong conclusion.
Suggest a neutral phrasing that covers both (e.g. après vente non confirmée) — or pass the reason in.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…s de CRITICAL à tort Review Kilo #96 : _position_exists peut servir un instantané périmé (~30 min sur le démo) ; la garde est retirée (le courtier refuse le stop si la vente est passée) et l'échec de repose après fill non confirmé est un WARNING neutre, plus un CRITICAL « vente échouée ». Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
| else: | ||
| # Fill non confirmé : la vente a probablement été exécutée (le courtier refuse alors un stop sur des | ||
| # actions qu'on ne possède plus) ; la synchro du cycle suivant tranchera. Pas d'alerte CRITICAL à tort. | ||
| logger.warning( |
There was a problem hiding this comment.
SUGGESTION: The unconfirmed-fill branch collapses two opposite outcomes into one WARNING, and the message asserts the more optimistic cause
_place_stop_order returns (None, None) both when the broker refuses the stop (selling-equity-not-owned, i.e. the sale did fill) and when the request simply failed (network / 429 / 4xx — see the logger.error at src/t212_executor.py:925). In the second case the position is still open and now has no stop at all, which AGENTS.md calls "a CRITICAL event", yet the only signal is this warning that explicitly assumes refus courtier probable si la vente est exécutée. The new wording is right for the refusal case; the ambiguity it introduces is the unhandled one. Suggest distinguishing them by status code (e.g. keep a CRITICAL when resp is None/5xx, the path where the position is provably still open) and keeping the warning for selling-equity-not-owned.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
|
|
||
| class TestUnconfirmedSellReprotects: | ||
| """Vente acceptée (2xx) mais fill non confirmé : le stop libéré est reposé si la position existe encore.""" |
There was a problem hiding this comment.
SUGGESTION: Class docstring is now stale after the _position_exists guard was dropped
It still says le stop libéré est reposé si la position existe encore, but the production path no longer consults the position snapshot at all (src/t212_executor.py:1773 now calls _replace_released_stop unconditionally and the test asserts _position_exists is never called). A future reader will look for a position check that no longer exists.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
…string de test Review Kilo #96. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Problème
GET /equity/ordersest limité à 1 req / 5 s par compte (doc officielle). La lecture des stops était appelée plusieurs fois en quelques secondes par cycle (synchro, cliquet, vente) → 429 à chaque cycle, « Stop fetch error — local stop state preserved » ×16, cliquet de stop qui travaille à l'aveugle. Les 429 étaient réessayés après coup au lieu d'être évités.Changements
src/t212_rate_limit.py(nouveau) :RateGate(créneaux réservés sous verrou, sûr en multi-thread) etGatedSession. Tous les appelsget/delete/postde l'exécuteur passent par le régulateur.safe_request: respecteRetry-After, pénalise le bucket après un 429._get_active_stop_order: un nouvel essai après 429 ; deux 429 restentERROR(jamaisNOT_FOUND, invariant « Failed fetch ≠ empty »).tests/__init__.py: pas d'attente réelle dans la suite (HTTP mocké).TRADING212_API_GUIDE.md: tableau des limites officielles + section « compte réel : types d'ordres » (limit/stop/stop-limit disponibles par l'API en réel depuis le 29/01/2026, source : annonce staff T212 + centre d'aide ; à confirmer par un premier stop réel de faible montant).Vérification
GET /equity/ordersespacées de 5,4 à 5,5 s, toutes 200, stop #55701244265 @1386.81 identique à l'état local.ruff checkOK. 12 nouveaux tests (tests/test_t212_rate_limit.py).Impact sur la durée des cycles
Les appels rapprochés attendent jusqu'à ~5,5 s (stops) / ~10,5 s (historique). Les cycles durent aujourd'hui ~1,5 min pour un budget de 40 min : marge très large.
🤖 Generated with Claude Code