fix(order): store terminal-only order-path acks (313/315/317/331) before mark_complete

[olivier-babelcast]

What this fixes

Under concurrent order submits on a single Rithmic session, an order-path
acknowledgement can arrive as a terminal-only response (rp_code == '0',
no data frame preceding it). BasePlant._process_response treats a terminal
frame as a completion signal and jumps straight to mark_complete — it only
calls handle_response first when the template_id is in the
_terminal_carries_data allow-list.

The order-ack templates were not in that set:

• 313 new-order ack (request 312)
• 315 modify-order ack (request 314)
• 317 cancel-order ack (request 316)
• 331 bracket-order ack (request 330)

So the ack was discarded before being stored, send_and_collect returned an
empty list, and the caller raised Rithmic empty response — a silently
missed order.

Why it only shows under concurrency

A single in-flight order usually receives a data frame followed by a terminal
frame, so the data frame gets stored regardless of the allow-list. Under
concurrent submits on one session the data frame is lost in the race and the
terminal frame is the only one that arrives — which is exactly the case the
allow-list was dropping. That's why single-order tests pass while production
under load misses fills.

We hit this on a live prop-firm account on 2026-04-23: N accounts submitting on
one session, each request getting only its terminal ack.

The fix

# 313/315/317/331: order-path single-response acks (submit/modify/cancel/bracket).
_terminal_carries_data = {11, 15, 114, 301, 313, 315, 317, 331}

One-line allow-list extension in async_rithmic/plants/base.py. The terminal
ack is now stored via handle_response before mark_complete, so
send_and_collect returns it.

Regression test

tests/test_order_terminal_response.py exercises the real
BasePlant._process_response + RequestManager against a real OrderPlant
and real protobuf ResponseNewOrder frames — no live connection, no
application code, fully self-contained. Three cases:

1. test_terminal_only_order_ack_is_stored — a single terminal-only 313
   (rp_code 0) is stored, not dropped.
2. test_concurrent_terminal_only_acks_all_stored — 6 concurrent
   terminal-only acks are all stored (mirrors the live incident).
3. test_genuine_absence_still_times_out — the fix must not mask a real
   no-response: when nothing arrives, send_and_collect still times out.

Fails on the old allow-list {11, 15, 114, 301}; passes with
{11, 15, 114, 301, 313, 315, 317, 331}. Full suite green (pytest -q → all
passing) on top of v1.6.1.

Note: this PR was rebased onto current main (v1.6.1, including the #59
historical-data rewrite). It now contains only the order-terminal fix +
its test — the earlier historical-data changes this PR originally carried are
superseded by #59 and have been dropped.

🤖 Generated with Claude Code

[rundef]

hey, so there are conflicts because I already merged your previous PR and made tiny changes.
Maybe you could cleanup the main of your fork, and have the fix(ticker): raise on empty get_front_month_contract response on a separate branch for the PR ?

[rundef]

hi @olivier-babelcast

I copied your second commit (fix(ticker): raise on empty get_front_month_contract response) to main, since this PR has conflicts. (will release a new version of the package this weekend probably)

Concerning your last commit: always store terminal response before mark_complete, if I understand correctly, you were receiving empty list responses when calling new/modify/cancel/bracket order methods ?

I tested and I'm receiving two responses as expected on my end (response with data + terminal response)

Either I misunderstood the problem, or your issue is something else

2026-05-04 18:20:21,919 - rithmic.plant.order - DEBUG - Received message {'basketId': '2524962026', 'userMsg': ['3c566f54-0371-4483-85f3-2bef7a43b30b'], 'rqHandlerRpCode': ['0'], 'ssboe': 1777933221, 'usecs': 923944, 'userTag': '20260504_182021_order', 'templateId': 313}
2026-05-04 18:20:21,919 - rithmic.plant.order - DEBUG - Received message {'userMsg': ['3c566f54-0371-4483-85f3-2bef7a43b30b'], 'rpCode': ['0'], 'templateId': 313}

2026-05-04 18:20:23,110 - rithmic.plant.order - DEBUG - Received message {'basketId': '2524962026', 'userMsg': ['02f70234-54cb-45db-8ef6-f1f61ffe1742'], 'rqHandlerRpCode': ['0'], 'ssboe': 1777933223, 'usecs': 104665, 'templateId': 317}
2026-05-04 18:20:23,110 - rithmic.plant.order - DEBUG - Received message {'userMsg': ['02f70234-54cb-45db-8ef6-f1f61ffe1742'], 'rpCode': ['0'], 'templateId': 317}

[olivier-babelcast]

Ok, I will have a look later on.

…

On Mon, May 4, 2026, 18:29 Mickael Burguet ***@***.***> wrote: *rundef* left a comment (rundef/async_rithmic#53) <#53 (comment)> hi @olivier-babelcast <https://github.com/olivier-babelcast> I copied your second commit (fix(ticker): raise on empty get_front_month_contract response) to main, since this PR has conflicts. (will release a new version of the package this weekend probably) Concerning your last commit: always store terminal response before mark_complete, if I understand correctly, you were receiving empty list responses when calling new/modify/cancel/bracket order methods ? I tested and I'm receiving two responses as expected on my end (response with data + terminal response) Either I misunderstood the problem, or your issue is something else 2026-05-04 18:20:21,919 - rithmic.plant.order - DEBUG - Received message {'basketId': '2524962026', 'userMsg': ['3c566f54-0371-4483-85f3-2bef7a43b30b'], 'rqHandlerRpCode': ['0'], 'ssboe': 1777933221, 'usecs': 923944, 'userTag': '20260504_182021_order', 'templateId': 313} 2026-05-04 18:20:21,919 - rithmic.plant.order - DEBUG - Received message {'userMsg': ['3c566f54-0371-4483-85f3-2bef7a43b30b'], 'rpCode': ['0'], 'templateId': 313} 2026-05-04 18:20:23,110 - rithmic.plant.order - DEBUG - Received message {'basketId': '2524962026', 'userMsg': ['02f70234-54cb-45db-8ef6-f1f61ffe1742'], 'rqHandlerRpCode': ['0'], 'ssboe': 1777933223, 'usecs': 104665, 'templateId': 317} 2026-05-04 18:20:23,110 - rithmic.plant.order - DEBUG - Received message {'userMsg': ['02f70234-54cb-45db-8ef6-f1f61ffe1742'], 'rpCode': ['0'], 'templateId': 317} — Reply to this email directly, view it on GitHub <#53 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMBBSIUWFE6GPQV7SCOSJAT4ZEKWDAVCNFSM6AAAAACYAKWQNWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DGNZUHE3TEMZUGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[olivier-babelcast]

Added a self-contained regression test in this PR: tests/test_order_terminal_response.py.

It exercises the real BasePlant._process_response + RequestManager against a real OrderPlant and real protobuf ResponseNewOrder frames — no live connection, no application code — so it's reproducible here in the library alone.

Why single-order tests don't catch this: a single in-flight order normally gets a data frame followed by a terminal frame, so the data frame is stored regardless of the allow-list. Under concurrent submits on one session the data frame is lost in the race and the terminal frame (rp_code='0') is the only one that arrives — exactly the case the old allow-list {11, 15, 114, 301} was dropping, so send_and_collect returned [] → Rithmic empty response → a silently missed order.

The three cases:

def _terminal_ack(request_id, template_id=313):
    """A real protobuf order-ack as a terminal-only response (rp_code='0')."""
    resp = pb.response_new_order_pb2.ResponseNewOrder()
    resp.template_id = template_id
    resp.user_msg.append(request_id)
    resp.rp_code.append('0')          # terminal success marker, no data frame preceded it
    return resp

async def test_terminal_only_order_ack_is_stored(order_plant):
    rid = "req-single"
    order_plant.request_manager.start(
        rid, request={"template_id": 312}, expected_response={"user_msg": [rid]}
    )
    await order_plant._process_response(_terminal_ack(rid))
    stored = order_plant.request_manager.responses.get(rid, [])
    assert stored          # dropped before the fix
    assert stored[0].template_id == 313

async def test_concurrent_terminal_only_acks_all_stored(order_plant):
    # N accounts submit on one session; each request gets only its terminal ack.
    rids = [f"req-{i}" for i in range(6)]
    for rid in rids:
        order_plant.request_manager.start(
            rid, request={"template_id": 312}, expected_response={"user_msg": [rid]}
        )
    await asyncio.gather(*(order_plant._process_response(_terminal_ack(rid)) for rid in rids))
    dropped = [rid for rid in rids if not order_plant.request_manager.responses.get(rid)]
    assert not dropped

async def test_genuine_absence_still_times_out(order_plant):
    # The fix must NOT mask a real no-response.
    order_plant._send_request = AsyncMock()
    with pytest.raises(asyncio.TimeoutError):
        await order_plant.request_manager.send_and_collect(
            timeout=0.2, user_msg="req-none", template_id=312, expected_response={}
        )

Result on top of v1.6.1:

• Old allow-list {11, 15, 114, 301} → test_terminal_only_* and test_concurrent_* fail (acks dropped).
• With {11, 15, 114, 301, 313, 315, 317, 331} → 3 passed, and test_genuine_absence_still_times_out confirms the fix doesn't mask a real missing response.

Full suite: 36 passed.

🤖 Generated with Claude Code

[olivier-bn]

I am not familiar enough with the library to assess if this correct, or my Claude do believe so. It wrote a test for it. I will investigate a bit more. We did have an issue that supposedly was fixed by it. I am running live 20+ bots and it has been stable the last month or so with this fix, then it is certainly not dangereous

[olivier-bn]

here is Claude analysis (take with Caution)
The three stages, and where it breaks

Every request (submit/modify/cancel) goes through _send_and_collect → RequestManager:

1. Send + register. Before sending, the request is registered with a unique user_msg id and an expected_response filter. For a
   submit that's {template_id: 313, user_msg: [rid]}.
2. Arrive + correlate. Rithmic echoes that user_msg back on the ack. The recv loop hands the frame to _process_response, which finds
   the pending request by id and matches it against the filter. ✅ This works — reconciliation is fine.
3. Store + complete. The matched frame should be appended to that request's response list, then the request marked done so
   send_and_collect returns the list. ❌ This is where it breaks.

Why stage 3 dropped it

_process_response classifies every incoming frame by one signal — the presence of rp_code:

• No rp_code → it's a data frame → store it.
• rp_code present → it's a completion/terminal frame → for most request types this is just a bare "done, OK" sentinel carrying no
  payload, so it is deliberately not stored — only "mark done."

The exception is a hard-coded allow-list, _terminal_carries_data, for endpoints where that terminal frame is itself the payload
(login, reference data, front-month, etc. — single-response endpoints with no separate data frame).

Order acks are exactly that shape: a single 313/315/317/331 frame carrying rp_code='0' and the order details — there's no separate
data frame preceding it. But the order-ack templates were missing from the allow-list. So the classifier saw rp_code='0', treated
the ack as a bare sentinel, discarded it, and set "done" with an empty list. send_and_collect returned [] → caller raises "Rithmic
empty response."

Why it's genuinely bad (and sneaky)

• The order may have been accepted by Rithmic, yet the library reports the submit as failed/empty → a missed or ambiguous entry.
• It's silent: mark_complete did fire, so there's no timeout and no exception inside the library — it looks like a perfectly
  completed request that simply had zero responses. The retry machinery only triggers on TimeoutError, so it never engages. And
  submit_order (templates 312/330) is set to retries = 1 anyway. So a dropped ack on a submit surfaces immediately as a hard failure
  with no recovery.

[rundef]

I will look into this some more next week