Skip to content

Robinhood STATE_TO_PORT_STATUS omits partially_filled — latent, not currently harmful #226

Description

@eaitbrahim

Found while vetting #222. Filing it accurately scoped rather than as the bug I first took it for.

The inaccuracy

packages/keel-broker-robinhood/keel_broker_robinhood/translate.py:52

STATE_TO_PORT_STATUS: dict[str, str] = {
    "open": "OPEN", "canceled": "CANCELLED", "filled": "FILLED",
    "failed": "FAILED", "pending": "PENDING",
}

Robinhood's docs publish two non-matching state enums: the order-response object lists partially_filled and no pending; the GET /orders/ query filter lists pending and no partially_filled. Our map took the query-filter set, so partially_filled — a value the response object can actually carry — is absent and falls through to_port_status's default to PENDING.

Why this is NOT currently harmful

I initially reported this as affecting reconciliation. That was wrong, and the correction is worth recording. keel/execution/reconcile.py branches only on:

  • _FILLED = "FILLED" (line 36)
  • _DEAD = frozenset({"CANCELLED", "CANCELED", "EXPIRED", "FAILED"}) (line 37)

Everything else — OPEN and PENDING alike — takes the same path: not dead, not filled, keep polling (line 102, if status != _FILLED). So OPEN and PENDING are indistinguishable to every current consumer, and a partially-filled order's economics are read from filled_size/average_filled_price, not from the status string. The conformance suite also passes, since PENDING is in the port's accepted vocabulary.

to_port_status's fallthrough-to-PENDING behaviour is itself correct and well-argued — its docstring makes the right case that an unrecognised state means the adapter does not know the outcome, and that FAILED would declare a terminal outcome nobody observed.

Why it is still worth fixing

The map is a factual claim about the venue's vocabulary, and it is incomplete. It becomes live the moment any consumer distinguishes a resting order from a not-yet-working one — OPEN is the honest answer for a partially-filled GTC limit, which is still working at the venue. Until then this is a one-line accuracy fix, not a defect.

Note the venue value is also unverified: no order object has ever been observed live (see #198), so partially_filled's exact spelling is doc-only, and the docs contradict themselves about which enum applies. Whoever fixes this should either verify it against a real order or add it with a comment saying it is doc-sourced and which of the two enums it came from.

Related: #198 (order-object fields unverified), #222 (where this surfaced).

Metadata

Metadata

Assignees

No one assigned

    Labels

    fixBug fix (groups under Fixes)

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions