#743 order set executable when fully matched on placement

[wippa-studios]

Closes #743

#743

Root cause

BetfairExecution.execute_replace set the newly created replacement order to
executable() unconditionally, without ever looking at the orderStatus
returned in the place instruction report:

market.place_order(
    replacement_order, execute=False, client=order.client
)
replacement_order.executable()

execute_place, directly above it, does check order_status (for PENDING
and EXPIRED) and only falls through to executable() as a deliberate "let
process.py pick it up". execute_replace had no such branch at all.

So when a replace_order is matched in full on placement by best price
execution and Betfair returns orderStatus: "EXECUTION_COMPLETE", flumine
discarded that and marked the order EXECUTABLE — reporting an unmatched
exposure that does not exist.

The two log entries in the report are exactly this path, 2ms apart:

• market.place_order(...) → Transaction.place_order → order.place(...) →
  placing() → Pending
• then the unconditional executable() → Executable

process.py does handle EXECUTION_COMPLETE correctly, so a later order
stream update will correct the status. But until it arrives the order and its
trade report live unmatched size for a bet that is already fully matched, which
is the real-money misreport described in the issue.

The reporter's follow-up comment is right that this is not a sync/async
socket-mode artifact. The branch is on order_status and nothing else, so it
behaves identically in both modes.

The problem was in both execution paths

execute_replace had no orderStatus branch at all. execute_place had one,
but it only handled PENDING and EXPIRED — so EXECUTION_COMPLETE fell
through its else to executable() there as well.

The issue is titled "order status set wrong on bets matched immediately on
placing", and its body notes "suspect same might happen when initially
placing an order if the price is outside current best price". Both halves are
therefore fixed here, so the issue is closed rather than half-closed:

• execute_place now branches on order_status == "EXECUTION_COMPLETE" and
  calls execution_complete().
• execute_replace gained the same branch.
• PENDING and EXPIRED handling in execute_place is untouched, and the
  else remains the catch-all for every other value — including a missing or
  None order_status. The new branch adds no new attribute access, since
  order_status is already dereferenced by the existing PENDING comparison on
  the same object.

The place-side fix is not only a tidy-up. process.py can already set
EXECUTION_COMPLETE on a PENDING order, so base execute_place's else was
regressing a terminal state back to live. The corrective branch is guarded on
EXECUTABLE, so it needs a further stream image that may never arrive once the
bet is matched — which is precisely the shape in the #743 log.

Note the polarity: before this PR, the execute_replace change was the only
correct handling of EXECUTION_COMPLETE in the file. The execute_place gap
was pre-existing on master (verified byte-identical to base 8cefbb53), not
introduced by the first commit.

Tests

Two new modules, built from the concrete values in the report:

• tests/test_execution_replace.py — asserts the replacement order receives
  execution_complete() and not executable().
• tests/test_execution_place.py — one test for the new EXECUTION_COMPLETE
  behaviour, plus four guards pinning the pre-existing PENDING, EXPIRED and
  fallthrough behaviour so the new branch cannot have disturbed them.

Which of these actually discriminate, stated precisely. Only
test_execute_place_success_execution_complete and the test in
test_execution_replace.py fail against unfixed code. The other four tests in
test_execution_place.py assert behaviour that already worked on base, so they
pass against unfixed master by construction — they are regression guards,
not reproducers, and should not be read as evidence of the bug. They are still
worth having: tests/test_execution.py contains no order_status references
at all, so the EXECUTION_COMPLETE branch had no coverage whatsoever before
this PR.

Mutation testing confirms the discriminating tests are load-bearing rather than
merely passing: making the EXPIRED branch unreachable fails
test_execute_place_success_expired, making PENDING unreachable fails
test_execute_place_success_pending, and deleting the new EXECUTION_COMPLETE
branch fails test_execute_place_success_execution_complete.

They are new sibling modules rather than additions to tests/test_execution.py
(93KB) so that large file stays byte-for-byte untouched — an authoring
constraint of the API-only write path, not a repo convention. Folding both
modules into BetfairExecutionTest alongside test_execute_place_success_pending
/ _expired is the tidier end state and is a maintainer's call; happy to do it
as a pure move.

Not included

execute_replace still does not branch on PENDING (async replace) or
EXPIRED (FOK replace), both of which execute_place handles. Neither is
reported here. The reasoning, checked in the code rather than assumed:
Transaction.execute() passes async_=self._async_place_orders to
_create_order_package only for PLACE, so the REPLACE call takes the
async_: bool = False default and replaceOrders is never sent async — a
nested PENDING is unreachable through the normal path. EXPIRED needs FOK,
and an FOK order never rests on the book, so it can never be replaceable.

Worth being honest about the weak point: that argument rests on a default
argument in another file. It can be bypassed by hand-building a REPLACE
package with async_=True and going around Transaction, so this is
robustness rather than a guarantee. Happy to cover both anyway if you would
rather have the belt-and-braces.

Verification

Run locally against the real bytes, commands read from
.github/workflows/test.yml rather than assumed:

Check	Result
python -m unittest discover -s tests	1037 tests, OK, 0 failures (base: 1031)
black . --check (pin 26.5.1)	clean, 112 files
black . --check (pin 24.2.0)	clean, 112 files — so the pre-commit/requirements skew is not implicated
Discriminating test vs base 8cefbb53	fails 10/10 runs on base, passes 10/10 on head, deterministic across 5 interleaved rounds
Tests lost	0 — 1031 → 1037 is exactly +6; tests/test_execution.py blob-identical
Diff	3 files, +251/−1

The 10/10 figures are 10 repeated runs of the discriminating tests, not 10
distinct tests. Only the two named tests fail on base; see the Tests section.

Not verified locally: only Python 3.13 was available, so four legs of CI's
3.10–3.14 matrix are unexercised. coverage report and the test-speed job
were not run. Upstream CI is blocked, not green — the pull_request
workflow on this first-time fork is unapproved with 0 jobs executed, so there is
no CI verdict yet. Note that CI's Coverage step is coverage report with no
--fail-under, so it exits 0 regardless of coverage and asserts nothing; the
real gates are Black fmt and Unittest.

[wippa-studios]

Verification report — local execution of the repository's real checks

This supersedes the Verification: pending CI placeholder in the PR body. Everything below was executed, not read. Method: two detached worktrees at 8cefbb53 (base) and 728a4f16 (head), plus a third worktree = pristine base + only the new test file copied over.

Environment

Python	3.13.5 (GCC 14.2.0)
black	26.5.1
betfairlightweight	2.24.0
coverage / pre-commit	7.16.1 / 4.6.2
tenacity / python-json-logger / requests	8.3.0 / 2.0.7 / 2.32.5
betconnect / betdaq-retail / smart-open	0.3.0 / 0.3.0 / 7.7.1
install	pip install -r requirements.txt (as the workflow does)

flumine is not pip-installed, so python -m resolves the package from the worktree under test — confirmed by printing betfairexecution.__file__.

Commands — read from .github/workflows/test.yml, not assumed

Confirmed from the workflow file:

pip install -r requirements.txt      # "Install dependencies"
black . --check                      # "Black fmt"
python -m unittest discover -s tests # "Unittest"
coverage run -m unittest discover; coverage report   # "Coverage"
# test-speed job: + pip install -r requirements-speed.txt, then the same unittest command

pyproject.toml declares no lint/test target; .pre-commit-config.yaml defines exactly one hook (psf/black rev 24.2.0). Note the pin discrepancy: pre-commit uses black 24.2.0 while CI installs 26.5.1 — I ran both, and the change is clean under each.

Baseline on base 8cefbb53 — run FIRST, before looking at head

python -m unittest discover -s tests   →  Ran 1031 tests   OK   (exit 0)
black . --check                         →  110 files unchanged   (exit 0)

Pre-existing failures: none. The baseline is fully green, so there is no pre-existing breakage to discount. Every result below is therefore attributable to this PR.

The load-bearing check: does the new test actually test the fix?

Setup verified before running:

• tests/test_execution_replace.py copied head→base is byte-identical: aec446211695eb5f080f6dd63997e8c8dbaffa2804c23e706159c095e (sha256, cmp identical).
• betfairexecution.py left at the base version: 9df5cc1979e14140ae5a89255cccdee3271cd393a451529a8159683c45e83f9b (≠ head 2c496882…).
• git status --porcelain on that worktree showed exactly one entry: ?? tests/test_execution_replace.py.

On unfixed base — FAILS, with the precise assertion the fix addresses:

FAIL: test_execute_replace_matched_on_placement
  File "tests/test_execution_replace.py", line 49, in test_execute_replace_matched_on_placement
    replacement_order.execution_complete.assert_called_with()
AssertionError: expected call not found.
Expected: execution_complete()
  Actual: not called.
Ran 1 test — FAILED (failures=1)   exit 1

On head — PASSES. Ran 1 test … OK, exit 0.

Determinism — 10 consecutive repeats each, no flakes, no ordering or timing dependence:

• base + test: 0 pass / 10 fail
• head: 10 pass / 0 fail

Full suite on base+test = 1032 tests, FAILED (failures=1) — the single failure is the new test, so the failure is isolated, not collateral.

Is the test meaningful or tautological? Honest assessment

I mutation-tested it rather than reasoning about it:

Mutant	Result	What it proves
Remove the status call entirely (pass)	FAILS	Not vacuous — genuinely requires execution_complete() to be called
Drop the else, call execution_complete() unconditionally	new test passes	The new test alone is one-directional

So: the new test is not tautological — it provably fails on the unfixed code and kills a no-op mutant. It is genuinely load-bearing for the bug in #743.

The honest caveat: on its own it pins only the EXECUTION_COMPLETE branch. It would not catch an over-broad fix. However, I checked whether that gap is covered elsewhere, and it is: the pre-existing test_execute_replace_success (tests/test_execution.py:801) never sets order_status, so it takes the else branch. Against the over-correction mutant it fails with Expected: executable() / Actual: not called, and the full suite catches it. Both branches are pinned by the suite as a whole. I consider the test design sound.

Full-suite delta and test-name set difference

	base	head
Tests run	1031	1032
Result	OK	OK

Set difference of the test IDs that actually ran (collected via a TestResult collector, not inferred from the diff):

• Present on base, absent on head: nothing. → zero lost tests.
• Present on head, absent on base: exactly one —
  test_execution_replace.BetfairExecutionReplaceTest.test_execute_replace_matched_on_placement

Nothing was lost — two independent checks

1. Unchanged-prefix sha256. Common prefix is the first 9777 bytes; the hunk is the only difference.
   • base: 5af55e95b47266c0e713074e249fbff1a5d4e3e68fc74a5e99d85de027af8157
   • head: 5af55e95b47266c0e713074e249fbff1a5d4e3e68fc74a5e99d85de027af8157 → identical
   • Line count 309 → 316 (+7, consistent with +8/−1). The file is longer, not truncated.
2. Suffix integrity. The 4255-byte tail from the next elif onward hashes to 0fe3991d185947d8d5464db392bc1d708d7f329334ac9c9f613fa75501215e60 on both → identical. Combined with the identical prefix, betfairexecution.py is provably altered in exactly one hunk.

Lint

Target	base	head
black . --check (full repo, CI gate)	PASS, 110 files	PASS, 111 files, exit 0
black --check on the 2 changed files	—	PASS, exit 0
pre-commit run --all-files (black 24.2.0)	—	Passed

Other CI jobs on head: test-speed → Ran 1032 tests, OK. coverage run -m unittest discover → OK, betfairexecution.py at 93%, total 97%. The deploy job is gated on github.ref == refs/heads/master and does not run for a PR.

CI status — blocked, which is neither a pass nor a fail

• mergeable_state: blocked; check runs for 728a4f16: total_count: 0.
• Commit status: state: pending, total_count: 0.
• One workflow run exists — test #1483, id 36299974238, conclusion: action_required, jobs total_count: 0.

action_required is GitHub's state for a first-time-contributor pull_request workflow awaiting maintainer approval: the workflow did not execute a single job. I am explicitly not reporting this as a pass, and not as a failure. It is not a CI verdict, and nothing was left to poll — the run is terminal in the approval-pending state, not queued or in progress.

Author's claims vs. what I observed

• Root cause — confirmed. execute_place does branch on order_status (PENDING/EXPIRED, with order.executable() # let process.py pick it up as the deliberate fallthrough); execute_replace had no such branch and called executable() unconditionally. The fix mirrors the sibling correctly.
• +8/−1 and the two changed files — confirmed by git diff --stat.
• The scope decision to leave PENDING/EXPIRED in execute_replace unhandled is explicitly documented in the PR body and I agree with leaving it.

Limitations (stated, not papered over)

• I could run only Python 3.13; CI's matrix is 3.10–3.14 and no other interpreter is installed here. The other four legs are unverified by me. These are pure-mock unit tests with no version-sensitive syntax in the change, so I expect them to pass, but I did not execute them and am not claiming I did.
• The deploy job (python -m build, twine check, mkdocs build) does not run for PRs, so I did not run it.
• This was a read-only verification. Nothing was pushed, amended, or modified; all three worktrees are pristine apart from the deliberately-injected test file in the third worktree.

Verdict

VERIFIED PASS — the full CI suite passes on head, the regression test provably fails on unfixed base and passes on head (deterministically, 10/10), zero tests were lost, and the changed files are byte-verified as a single hunk. The one open item is CI approval, which is blocked rather than failing.

[wippa-studios]

Verdict: approve, but unrecorded

My verdict is approve. I am submitting it as COMMENT rather than APPROVE because the token this pipeline authenticates with belongs to the PR author, and GitHub rejects self-approval (Can not approve your own pull request). That is structural — it happens on every PR this pipeline opens — so this review is an unrecorded approval, not a failed gate. A maintainer still has to record the formal approval, and nothing here should be read as the PR being mergeable on the strength of this review.

No CRITICAL or HIGH findings. One MEDIUM (pre-existing, not introduced here) and three LOW.

---

Findings

[MEDIUM] flumine/execution/betfairexecution.py:35 - execute_place still maps
  order_status == "EXECUTION_COMPLETE" to executable() via its else branch, so an
  immediately-fully-matched *place* order is still reported as live unmatched
  exposure. This PR fixes the replace half of the bug only, and the reporter of
  #743 explicitly suspects the place half ("suspect same might happen when
  initially placing an order if the price is outside current best price"). Closing
  #743 overstates the coverage. -> add an EXECUTION_COMPLETE branch to
  execute_place next to the existing EXPIRED one, or close the issue partially
  and track the rest. Pre-existing, so not blocking this PR.

[LOW] flumine/execution/betfairexecution.py:215 - the else branch swallows
  PENDING and EXPIRED, which the sibling handles. I checked whether that is a
  real hole: it is not reachable today (evidence below), so this is a note, not a
  defect. -> add a one-line comment recording why it is safe, or mirror the
  sibling's PENDING branch as cheap insurance.

[LOW] tests/test_execution_replace.py:8 - new module rather than an addition to
  BetfairExecutionTest in tests/test_execution.py, where this test's exact
  siblings (test_execute_place_success_pending, test_execute_place_success_expired)
  already live. The stated reason (the API write path replaces whole files) is a
  constraint of how the change was authored, not a repo convention. -> a
  maintainer will likely ask for it as
  BetfairExecutionTest.test_execute_replace_success_execution_complete. Not
  blocking: unittest discover picks the module up and the style otherwise matches.

[LOW] flumine/execution/betfairexecution.py:220 - "elif place_instruction_reports
  .status == FAILURE: pass  # todo" is pre-existing and untouched, but it is the
  more dangerous half of this function. The cancel leg has already called
  order.execution_complete() on the original order, so a failed place leg means
  the bet is gone, no replacement order exists, and nothing is logged or
  surfaced to the strategy. Out of scope here. -> its own issue.

---

The PENDING / EXPIRED scoping: assessed, and it holds

The author scoped those two out because neither was reported. That is weaker evidence than what the code actually shows, so I went looking for reachability and found they are genuinely unreachable through the public API:

• PENDING — PENDING is only returned for async requests. Transaction.execute() (flumine/execution/transaction.py) passes async_=self._async_place_orders to _create_order_package only for OrderPackageType.PLACE; the REPLACE call omits it, so it takes the _create_order_package(..., async_: bool = False) default. BaseOrderPackage.async_ therefore is always False for a replace, and BetfairExecution.replace() forwards async_=order_package.async_ to replace_orders. The issue reporter's follow-up comment about config.async_place_orders = True does not change this — that flag only ever reaches the place package.
• EXPIRED — EXPIRED means no longer available due to a time-in-force constraint (FOK). Trade.create_order_replacement inherits persistence_type from the original order, and BetfairOrder.replace() only permits LIMIT / LIMIT_ON_CLOSE and requires status == EXECUTABLE. An FOK order never rests on the book, so it can never reach the replace path in the first place.

So the gap is not an obvious hole, and the author's decision to leave it alone rather than change behaviour they cannot verify is the right call for live-money code. The one residual risk is that this safety rests on a default argument three files away, which is exactly the kind of thing that gets broken by an unrelated future change — hence the LOW, not a demand.

---

Correctness of the fix itself

The branch condition is right, and the fix matches the shape of the sibling without matching its semantics — and in this case that is the correct outcome, because the sibling is the one that is wrong. execute_place branches on PENDING and EXPIRED but not EXECUTION_COMPLETE, so it falls through to executable() for a fully-matched place. The new replace code is the first place in the file that handles EXECUTION_COMPLETE correctly. That is the MEDIUM above, and it is why I read the sibling line by line rather than treating it as the reference implementation.

Per-order_status behaviour in execute_replace after this change:

place_instruction_reports.order_status	before	after
EXECUTION_COMPLETE	executable() — wrong	execution_complete()
EXECUTABLE	executable()	executable() (unchanged)
PENDING	executable()	executable() (unchanged, unreachable)
EXPIRED	executable()	executable() (unchanged, unreachable)
missing / None	executable()	executable() (unchanged)

Edge cases I checked, and cleared

• Missing / None order_status. The new access sits inside the same status == "SUCCESS" branch as the pre-existing execute_place access to instruction_report.order_status, so its risk profile is identical to code already in production — no new failure mode. A missing value compares unequal and falls through to executable(), i.e. exactly the pre-change behaviour. I could not execute anything to test a deliberately malformed payload, so that last sentence is a code-reading conclusion, not a run.
• Zero-size, partial match, absent sizeRemaining. The branch reads only order_status; it never touches size_matched or size_remaining. Betfair omits sizeRemaining from a place response (BetfairOrder.size_remaining has an AttributeError fallback for exactly that), and none of that affects the condition. The test's size_matched = 0.01 is faithful to the report payload but incidental to the assertion.
• Could this mark an order complete when it is not? That would be worse than the original bug, and I found no path to it. Betfair returns EXECUTION_COMPLETE only when there is no remaining size, which is the same signal process_current_order uses to correct an order out of EXECUTABLE. The new branch cannot outrun the order stream into a wrong answer.
• Is the new status overwritten afterwards? No. Transaction.place_order skips _validate_controls when execute=False, so there is no order.executable() reset after it (which is what would happen on a control violation). market.place_order(..., execute=False) is called before the branch, and it is what sets the order PENDING via placing() — so the ordering is correct and the new status is the final one inside the with order.trade: block.
• Ordinary non-immediate replace path. For every order_status other than EXECUTION_COMPLETE the else is behaviourally identical to the line it replaces. No previously-correct order changes status. test_execute_replace_success in tests/test_execution.py never sets order_status and asserts replacement_order.executable.assert_called_with(), so the suite pins that the over-correction was not made.

Why this matters in live trading (it is not cosmetic)

On the cancel leg the original order is already execution_complete(). Pre-fix, the replacement was left EXECUTABLE, so Trade.complete stayed False, complete_trade() never fired, and runner_context.reset() was deferred — the strategy kept believing it held unmatched exposure on a bet that was already fully matched, which can also block the next trade on that selection under reset_seconds / place_reset_seconds. Those are exactly the two log entries 2ms apart in #743. After the fix, both orders complete inside the same with order.trade: block and the trade closes immediately, without waiting for the order stream. process_current_order does self-heal the old behaviour within one order-stream image, so this was a bounded window rather than a permanent corruption — the fix removes the window rather than repairing a lasting fault, which is the right shape for the change.

---

Security

No injection, deserialisation, authz, path, or network surface is touched. The diff adds one enum string comparison and one test file of mock values.

I could not run a secret scan. run_secret_scanning returned "Repository does not have GitHub Advanced Security enabled." So I am not reporting the secret posture as clean — I am reporting that I was unable to check it by that route. (The diff is 9 added lines in one function plus one new test module, and I read both in full; neither contains anything resembling a credential. The issue's reproduction log does contain real betIds and marketIds, but that is a pre-existing public issue, not this change.)

Scope and conventions

Two files, one commit, no unrelated changes riding along. The commit subject is #743 replace order set executable when fully matched on placement with close #743 and a Signed-off-by trailer, matching recent history on this branch family. The PR body is honest that the author ran nothing and rested the claim on reading the code — that is the correct disclosure, and the verification stage's independent run backs the specific test-passing claim.

CI — as reported, not as my own result

test workflow run #1483 on head 728a4f1 has conclusion action_required, and get_check_runs returns 0 check runs for the head SHA. That is blocked, awaiting maintainer approval of first-time-fork workflows. It is neither a pass nor a failure. Runs from within the repo on comparable PRs (1481, 1479, 1477) did execute and report success, which is the expected shape. Everything test-related in this review is the verification stage's reported result; I ran no code.

What I could not review

• I cannot execute anything. I did not run the suite, black, or a build; I have not verified the claimed 1031→1032 test counts or the mutation result first-hand, only read the diff and the code around it.
• I did not verify against Betfair's live API that replaceOrders never returns a nested orderStatus: "PENDING" on a synchronous call. The unreachability argument above is a framework-side argument (flumine never sets async_ on a replace package), which is strong but is not a substitute for an exchange-side guarantee.
• tests/test_execution.py is 93KB and the contents response was truncated partway through. I read BetfairExecutionTest in full, including test_execute_replace_success, which is what the findings depend on; I did not read the SimulatedExecutionTest and Betdaq sections in full.
• Whether flumine's simulation path (SimulatedExecution.execute_replace) has the same defect I did not audit; it is a separate implementation and untouched by this PR.

[wippa-studios]

Follow-up: the same bug on the place path

The first commit fixed execute_replace. execute_place, the sibling function directly above it, still routed order_status == "EXECUTION_COMPLETE" into its else branch and called executable() — so a place order matched in full on placement was still reported EXECUTABLE, telling a live strategy it holds unmatched exposure it does not hold. That is the half of #743 the issue itself points at: "suspect same might happen when initially placing an order if the price is outside current best price".

This gap is pre-existing, not a regression from the first commit. I read execute_place on upstream base 8cefbb53 and it is byte-for-byte identical to the version the first commit worked from; the only difference this branch makes to that function is the new branch. Before this follow-up, the first commit's execute_replace code was the only correct handling of EXECUTION_COMPLETE in the file.

Change

flumine/execution/betfairexecution.py, execute_place — three added lines, one new elif between the existing EXPIRED branch and the else:

elif instruction_report.order_status == "EXECUTION_COMPLETE":
    # fully matched on placement, no size remaining
    order.execution_complete()

PENDING and EXPIRED are untouched. The else remains the catch-all, so every other value still falls through to executable() — including a report carrying no order_status at all. The new branch adds no new attribute access: order_status was already read by the existing PENDING comparison, so nothing can newly raise.

Tests — tests/test_execution_place.py

Test	Covers
test_execute_place_success_execution_complete	the fix: execution_complete() called, executable() not called, so it fails without the change
test_execute_place_success_pending	PENDING still sets nothing at all — neither call
test_execute_place_success_expired	EXPIRED still calls execution_complete() only
test_execute_place_success_executable	a normal EXECUTABLE still calls executable() only
test_execute_place_success_order_status_none	a report with no order_status still falls through to executable()

These follow the pattern and naming of test_execute_place_success_pending and test_execute_place_success_expired in BetfairExecutionTest, built from that file's existing test bodies.

They are a new module, not an addition to BetfairExecutionTest. That is an authoring constraint of how I am submitting this, not a preference: tests/test_execution.py is ~93KB and the API write path replaces whole files, so editing it means re-emitting all 93KB with no way to verify a character. Folding test_execution_place.py and test_execution_replace.py into test_execution.py is the tidier end state and is a maintainer call — it is a straightforward move that needs no behaviour change.

One thing to know about the branch shape

The place fix landed as two commits, 403ae870 then fa5d4d48, because I got two things wrong in the first emission and caught them on read-back: a stray leading blank line at the top of betfairexecution.py, which black would reformat, and a malformed assertion in the last test. fa5d4d48 corrects only those two. The execute_place change itself landed in 403ae870 and is unchanged. I did not rewrite 728a4f16 and did not force-push, so if you prefer a cleaner history both follow-up commits are yours to squash.

Verification

Not run — no local checkout. This change was made over the GitHub API with no shell, no interpreter and no toolchain available, so I ran no tests, no black --check and no build. The claim that the new test discriminates rests on reading the code path, not on a test run. CI (.github/workflows/test.yml) runs black . --check and python -m unittest discover -s tests, which will cover both new modules.

[wippa-studios]

Verification report — executed locally against the real repo checks

Verdict: VERIFICATION FAILED — one hard CI gate fails: black . --check on tests/test_execution_place.py.

This is the first time any of this code has been executed. Everything below is real command output, not reading. The tests are genuinely load-bearing and the fix is behaviourally correct — but the branch is not mergeable as it stands, because a check the repo actually runs fails.

---

Environment

Workspace	/tmp/g-hub-verify-betcode-org-flumine-864-r2/ (ephemeral, since deleted)
Python	3.13.5 — one leg of CI's 3.10–3.14 matrix. I claim only 3.13.
black	26.5.1 (the requirements.txt pin = what CI installs) and, separately, 24.2.0 (the .pre-commit-config.yaml pin)
Deps	pip install -r requirements.txt verbatim, --no-cache-dir
Base	8cefbb5380bf6cf176f085110d037fac1d87401c
Head	fa5d4d4896a89a5be483e4225452b9a49e424789

Commands, read from .github/workflows/test.yml (not guessed)

- pip install -r requirements.txt
- black . --check
- python -m unittest discover -s tests
- coverage run -m unittest discover
- coverage report

test-speed job additionally runs pip install -r requirements-speed.txt then the same unittest discover. Matrix is ['3.10','3.11','3.12','3.13','3.14'] with fail-fast: false. deploy is gated on if: github.ref == 'refs/heads/master', so it never runs on a PR. The matrix is corroborated by base's own green check runs: test (3.10)…test (3.14), test-speed, deploy — 7/7 success on 8cefbb53.

On the two black pins: they genuinely conflict (requirements.txt 26.5.1 vs pre-commit 24.2.0). I ran both. The verdict is identical under both, so the conflict is immaterial here.

---

1. Baseline on base 8cefbb53 — green, 0 pre-existing failures

python -m unittest discover -s tests
Ran 1031 tests in 13.624s
OK                                    exit 0

The earlier pass's claim of 1031 green holds. Test-name set captured: 1031 unique IDs, matching the run count exactly. black . --check on base: exit 0, 110 files unchanged. test-speed deps on base: 1031 OK. python -m build + twine check on base: exit 0. There are no pre-existing failures to discount.

2. Full suite on head — green, +6 tests, nothing lost

python -m unittest discover -s tests
Ran 1037 tests in 13.604s
OK                                    exit 0

Set difference of test IDs that actually ran:

• LOST on head: 0 — no test present on base is missing on head.
• ADDED: 6 — the 5 BetfairExecutionPlaceTest cases + 1 BetfairExecutionReplaceTest case.

test-speed leg on head: 1037 OK. coverage run -m unittest discover exit 0, 1037 tests OK; coverage report exit 0, betfairexecution.py 93%, TOTAL 97%.

3. Load-bearing check — new place tests fail on base, pass on head

Copied only tests/test_execution_place.py from head onto a pristine base worktree. Verified: sha256 identical to head's copy (e0efbb34…), git status --porcelain shows exactly one untracked file, git diff --name-only empty (no tracked file modified), flumine/ identical to base.

On base — real assertion output:

FAIL: test_execute_place_success_execution_complete
Traceback (most recent call last):
  File ".../tests/test_execution_place.py", line 45, in test_execute_place_success_execution_complete
    mock_order.execution_complete.assert_called_with()
AssertionError: expected call not found.
Expected: execution_complete()
  Actual: not called.
Ran 5 tests — FAILED (failures=1)

The other four (_pending, _expired, _executable, _order_status_none) pass on base — which is the point: they are guards against disturbance, not new behaviour.

On head: all 5 pass, exit 0.

Determinism: 10 consecutive runs each —
base+place 0/10 pass, 10/10 fail · head 10/10 pass, 0/10 fail.
Plus 5 interleaved rounds (base→head alternating) with identical results each round, so there is no ordering or state leakage.

The author's reasoning is correct, verified against the real code. Base execute_place is:

if   instruction_report.order_status == "PENDING":  pass
elif instruction_report.order_status == "EXPIRED":  order.execution_complete()
else:                                                order.executable()

Head inserts elif … == "EXECUTION_COMPLETE": order.execution_complete() before the else. Direct instrumentation of the real function confirms it:

source	executable() called	execution_complete() called
base	True	False
head	False	True

So pre-change EXECUTION_COMPLETE really did fall through to executable(). Confirmed.

PENDING/EXPIRED are genuinely covered, not vacuously. They pass on both base and head, and I mutation-tested them in a throwaway copy:

mutation	caught by
EXPIRED → EXPIRED_MUTANT (branch unreachable)	test_execute_place_success_expired FAIL
PENDING → PENDING_MUTANT	test_execute_place_success_pending FAIL
delete the new EXECUTION_COMPLETE branch	test_execute_place_success_execution_complete FAIL
restore original	all 5 OK

Each branch is guarded by its own test. PENDING/EXPIRED behaviour is undisturbed by the change.

4. execute_replace test re-confirmed on the new head

Copied only tests/test_execution_replace.py onto pristine base (sha256 aec44621… identical, worktree otherwise clean).

• On base: AssertionError: Expected: execution_complete() / Actual: not called. — FAILED (failures=1), 10/10 runs.
• On head: 1 test OK, 10/10 runs.

The follow-up commit did not silently break it.

5. black --check — FAILS (hard gate)

base  black26.5.1 → exit 0  (110 files unchanged)
head  black26.5.1 → exit 1  (1 file would be reformatted, 111 unchanged)
head  black24.2.0 → exit 1  (same file)

The offending file is tests/test_execution_place.py, and the violation is not one of the two the author reported:

-        mock_instruction_report = mock.Mock(
-            status="SUCCESS", order_status="EXECUTABLE"
-        )
+        mock_instruction_report = mock.Mock(status="SUCCESS", order_status="EXECUTABLE")

in test_execute_place_success_executable — a line black 26.5.1 would collapse to a single 88-char line. flumine/execution/betfairexecution.py and tests/test_execution_replace.py are both clean.

This is a third, unreported black violation. Commit 2 had 2 files failing black; head has 1. The author found and fixed the betfairexecution.py one and the if False else None one, but missed this one. Commit 3's claim that it changed "exactly those two things and nothing else" is literally true — I confirmed the diff is only those two hunks — but fixing exactly the right two things left a third defect in place. Because the Black fmt step precedes Unittest and fail-fast: false only affects matrix legs, this step fails the run on every Python version 3.10–3.14.

Fix is a one-line reformat (collapse that mock.Mock(...) call) — I did not apply it, per read-only instruction.

6. Commit 3 did do what it claimed — both defects genuinely gone

Verified by reading head, not by trusting the description.

• Defect 1 (stray leading blank line): first bytes of flumine/execution/betfairexecution.py at head are i m p o r t l o g g i n g \n — line 1 is import logging, no leading blank. At commit 2, line 1 was empty. black 26.5.1 on a probe file with a leading blank line exits 1, so this was a real gate failure. Gone.
• Defect 2 (assert_called_with(...) if False else None): grep -rn "if False" over the whole head tree returns no match. The final test now reads mock__execution_helper.assert_called_with(mock_place, mock_order_package, mock_session), matching the other four tests. Gone.
• Nothing else changed: 403ae87→fa5d4d4 is 2 files changed, 2 insertions(+), 3 deletions(-) — the blank-line deletion and the assertion rewrite. Confirmed.

One correction to the author's account of defect 2. Commit 3's message says the malformed form "passed the call as keyword arguments and would itself have failed." It would not have failed. A if False else None short-circuits — A is never evaluated. I executed the exact malformed form: it raised nothing, and the mock recorded method_calls == [], i.e. the assertion silently did nothing and the test would have passed green while asserting nothing at all. That is a silent coverage hole, not a failing test. The fix is still correct and necessary (it restores a real assertion, and I confirmed the fixed positional form matches the real _execution_helper(self.place, order_package, http_session) call), but the stated symptom was a misdiagnosis. Worth knowing, because "would have failed" is what would have alerted the author to look at this file again.

7. Nothing lost — integrity proofs

• flumine/execution/betfairexecution.py: index-based excision of the changed ranges gives byte-identical content on both sides — sha256 f97d5ef665aa674d85481fd38536da6fb1cf81e52f99472950816b102f897298, 14028 bytes, both sides. difflib opcodes: [equal 33, insert 3, equal 175, replace 8, equal 100] — exactly 2 non-equal hunks, at the two intended sites. All 14028 remaining bytes unchanged.
• tests/test_execution.py untouched: absent from the PR diff; git blob SHA identical (20dc2d07852542ab994a9f7b2b2a6c5ab355fe51) on base and head; working-tree sha256 identical on both (0d6fe026…, 93677 bytes). The new tests really are in new modules; the 93KB file was not rewritten.
• Cumulative diff base→head: 3 files changed, 251 insertions(+), 1 deletion(-) — betfairexecution.py +11/−1, test_execution_place.py +179, test_execution_replace.py +61. Matches the reported +251/−1 exactly. Head worktree has 0 untracked/modified files.
• Build (deploy-job steps, on head): python -m build exit 0, twine check dist/* PASSED both artifacts, mkdocs build exit 0.

8. CI status — BLOCKED, neither pass nor fail

Check runs on head fa5d4d4	total_count: 0 — no check runs registered at all
Commit status	total_count: 0
Workflow run #1485 (head)	status=completed, conclusion=action_required
Jobs in run #1485	total_count: 0 — zero jobs ever started
Runs #1483 / #1484 (commits 1 & 2)	identical: action_required, 0 jobs

All three runs are created_at == run_started_at == updated_at and are in terminal state — nothing to poll; I re-polled and nothing changed. This is a first-time-contributor approval gate: the workflow has not run and has asserted nothing about this code.

Explicitly: this is blocked, awaiting maintainer approval. It is not a pass and not a failure. Contrast base 8cefbb53, which has 7 check runs all success — so the pipeline does run for trusted contributors.

Both traps confirmed in the workflow text and reproduced:

• coverage report has no --fail-under. I proved it asserts nothing: on a tree where the unittest leg FAILS, coverage report exited 0 at 21% coverage. A green Coverage leg means nothing.
• fail-fast: false — matrix legs never cancel each other, so all five would need reading individually. Moot here since no leg started.

---

Summary

check	base 8cefbb53	head fa5d4d4
pip install -r requirements.txt	exit 0	exit 0
black . --check (26.5.1 / 24.2.0)	0 / 0	1 / 1 ❌
python -m unittest discover -s tests	1031 OK	1037 OK ✅
test-speed leg	1031 OK	1037 OK ✅
coverage run + coverage report	exit 0	exit 0 (asserts nothing)
python -m build + twine check + mkdocs build	exit 0	exit 0 ✅
CI on GitHub	7/7 green	blocked, 0 jobs

The behaviour change is correct, well-tested, non-vacuous, and loses nothing. Both commit-2 defects really are fixed, and the fix for defect 2 is better than its own description claimed. The one thing standing between this branch and green is a single black formatting violation in tests/test_execution_place.py — a three-line block black wants collapsed to one, in test_execute_place_success_executable.

VERIFICATION FAILED — black . --check exits 1 on tests/test_execution_place.py (head), reproducing under both the 26.5.1 and 24.2.0 pins. All other executable checks pass. Fix is one line; happy to open a follow-up commit with that reformat plus the new black result if you'd like.

_{Read-only with respect to the repository: no push, amend, force-push or PR modification. The temporary workspace was deleted and /tmp returned to its starting free space.}

[wippa-studios]

Formatting fix pushed — bf29ac3

A verification pass on this branch found exactly one failing gate: black . --check. Behaviour was already good (1037 tests, 0 failures). This commit is formatting only.

The one-line change

tests/test_execution_place.py, in test_execute_place_success_executable — black wanted the over-split mock call collapsed:

-        mock_instruction_report = mock.Mock(
-            status="SUCCESS", order_status="EXECUTABLE"
-        )
+        mock_instruction_report = mock.Mock(status="SUCCESS", order_status="EXECUTABLE")

That is the whole diff: 1 file, 1 insertion, 3 deletions. Produced by running the project's own black tests/test_execution_place.py, not by hand.

Checks, before and after

Both on a clean checkout of the branch head, dependencies installed from requirements.txt per .github/workflows/test.yml (Python 3.13.5):

Gate	Before	After
black . --check (pin 26.5.1)	❌ would reformat tests/test_execution_place.py — exit 1, 1 file / 111 unchanged	✅ 112 files would be left unchanged — exit 0
black . --check (pre-commit pin 24.2.0)	❌ fails	✅ 112 files would be left unchanged — exit 0
python -m unittest discover -s tests	✅ Ran 1037 tests ... OK	✅ Ran 1037 tests ... OK

The test count is unchanged at 1037, 0 failures. It had to be — this is a formatting change and no test was added, removed, or renamed. Both black pins agree, so the pre-commit/requirements version skew is not implicated and no pin was touched.

Pushed as a fast-forward on top of fa5d4d48 (fa5d4d4..bf29ac3), no amend, no rebase, no force.

One correction on the record, without rewriting history

An earlier commit message on this branch claims the malformed assert_called_with(...) if False else None "would itself have failed". That is not right, and since it is now in the permanent record I want to be accurate about it: A if False else None short-circuits, so the assertion is never evaluated. The test would have passed while asserting nothing — a false green, not a failure.

The fix that removed it was still correct and still load-bearing, so nothing changes about the substance of the branch. This is only a note that the stated reason was wrong. I am deliberately not rewriting that commit message: it is history, already reviewed, and correcting it would force-push and churn a review that is otherwise fine.

[wippa-studios]

Verdict: approve, but unrecorded — on head bf29ac30

This supersedes review 5329151709, which was submitted against the older head 728a4f16 and did not see commits 403ae870, fa5d4d48 or bf29ac30. Its execute_replace findings are not restated below. Its one MEDIUM — that execute_place still routed EXECUTION_COMPLETE into else and called executable() — is now resolved by this diff, and section 3 below explains why the place fix is not merely a tidy-up but closes a terminal→live state regression.

The approval cannot be recorded as APPROVE. The token this pipeline authenticates with belongs to the PR author, so GitHub rejects self-approval with Can not approve your own pull request. That is structural and happens on every PR this pipeline opens; this is an unrecorded approval, not a failed gate. A maintainer still has to record the formal approval, and nothing here should be read as the PR being mergeable on the strength of this review. mergeable_state is blocked.

No CRITICAL or HIGH findings. No review-failed label. One MEDIUM, which is about the record rather than the code and does not change the verdict, and three LOW.

I read the whole diff, the head version of betfairexecution.py, and the surrounding contracts the new branches depend on: flumine/order/order.py (execution_complete, _is_complete, size_remaining), flumine/order/process.py (process_current_order), flumine/execution/transaction.py (_create_order_package, execute), flumine/execution/baseexecution.py, flumine/order/orderpackage.py, flumine/execution/simulatedexecution.py, and flumine/execution/betdaqexecution.py's dispatch.

---

Findings

[MEDIUM] tests/test_execution_place.py:54,85,116,147 - the verification record
  overstates the regression surface. The prior pass reported the new place tests
  "fail 10/10 on base". That cannot be right as stated: the module has 5 test
  methods, and four of them assert PRE-EXISTING behaviour and therefore pass
  unchanged against unfixed base code. On base, order_status "PENDING" takes the
  `pass` branch, "EXPIRED" already called execution_complete(), and
  "EXECUTABLE"/None already fell to the else. Only
  test_execute_place_success_execution_complete (:16) can fail on base, and the
  single equivalent on the replace side is test_execution_replace.py:16.
  -> No code change needed. Correct the record so a maintainer sizes the proof
  correctly: this fix is carried by two tests, one per path, not ten. The four
  guards are still worth keeping (see below), they just are not proof of the fix.
  The reported mutation results are consistent with that reading and do not
  contradict it.

[LOW] tests/test_execution_place.py:8 - same answer as the earlier gate's LOW on
  test_execution_replace.py, now applying to the place module too. Two new
  sibling modules rather than additions to the existing BetfairExecutionTest in
  tests/test_execution.py; the stated reason (leave that file byte-identical) is
  a property of how the change was authored, not a repo constraint. The 5 place
  tests also repeat the same ~12 lines of mock_order.trade.__enter__/__exit__
  and add_transaction boilerplate. -> A maintainer will likely ask for
  consolidation into BetfairExecutionTest with a shared setUp. Not blocking;
  unittest discover picks both modules up and the style otherwise matches the repo.

[LOW] flumine/execution/betfairexecution.py:212-219 - the replace side's new
  attribute access is genuinely new, unlike the place side's, and it sits after
  the order is already in the blotter. place_instruction_reports.order_status was
  never read before this change, and the read now happens AFTER
  market.place_order(...), so a raise at that point would leave the replacement
  order in placing() and skip the add_transaction at the end of the function.
  -> Not a defect: I found no path where Betfair returns a SUCCESS place
  instruction report without orderStatus (all four enum values carry it), and the
  existing tests show the accessor is tolerant of the value being absent. Flagging
  only because this is the one spot in the diff where a defensive
  getattr(..., None) would cost nothing. Optional.

[LOW] (commit hygiene, not a line) - four commits for one behavioural change:
  403ae870 (fix + tests), fa5d4d48 (a 2/-3 correction to it), bf29ac30 (a 1-line
  black reformat landing AFTER the code). -> Recommend squashing to a single
  commit before merge. I expect a maintainer to ask for this: the self-correction
  commit and the trailing format-only commit are exactly the two things that
  invite a squash request, and a black slip needing its own commit means
  formatting was not run before the code commit. Not blocking.

---

1. The place-side change is correct, and it is not an over-correction

execute_place and execute_replace now agree, and they agree with process.py:

order_status	execute_place (base)	execute_place (head)	execute_replace (head)
EXECUTION_COMPLETE	executable() — wrong	execution_complete()	execution_complete()
EXPIRED	execution_complete()	unchanged	executable() (unreachable, §4)
PENDING	pass	unchanged	executable() (unreachable, §4)
EXECUTABLE	executable()	unchanged	executable()
absent / None / unexpected	else → executable()	unchanged	unchanged

The pre-existing PENDING/EXPIRED behaviour is genuinely undisturbed. The execute_place hunk is purely additive — 3 added lines, zero removed, and the elif ... "EXPIRED" arm plus else: order.executable() # let process.py pick it up are unchanged context in the diff. The new arm sits after EXPIRED and before else, so it can only capture the one value that previously fell through. There is no reordering, no change to the pass arm, and no change to the FAILURE/TIMEOUT arms.

The condition is right, and the strongest evidence is that the base behaviour was a state regression, not just a wrong first guess. process_current_order (flumine/order/process.py) can already set an order to execution_complete() while it is still PENDING:

if order.bet_id and order.status == OrderStatus.PENDING:
    if order.current_order.status == "EXECUTABLE":
        order.executable()
    elif order.current_order.status in ["EXECUTION_COMPLETE", "EXPIRED"]:
        order.execution_complete()

So for a matched-on-placement bet the order-stream image can land before the place HTTP response is handled, process.py sets EXECUTION_COMPLETE, and then base execute_place's else overwrote that terminal state back to EXECUTABLE. The corrective branch is guarded on order.status == OrderStatus.EXECUTABLE, so it needs a further stream image to fire — and once a bet is fully matched the stream goes quiet, so it may never come. That is precisely the two-log-entries-2ms-apart shape in #743, and it is why the new arm is the right fix rather than a cosmetic alignment.

2. Live-trading safety of the new place arm — no new false-complete path

I could not find a path where the new branch reports an order complete when it is not matched. Traced case by case:

• EXECUTION_COMPLETE with size_matched < order size. The branch reads only order_status; it never consults size_matched or size_remaining, so a partial match cannot be laundered into a false complete by the condition itself. The branch cannot outrun the order stream into a wrong answer because it uses the identical signal the stream uses — process_current_order also maps EXECUTION_COMPLETE → execution_complete() with no size check. Betfair emits EXECUTION_COMPLETE only when no size remains, and the pre-existing EXPIRED arm already relies on the same trust. Consistent with the file's existing convention.
• order_status absent. Nothing new. See §3.
• order_status None. None == "EXECUTION_COMPLETE" is False, so it falls to else → executable() — byte-for-byte the base behaviour. Pinned by test_execute_place_success_order_status_none.
• order_status an unexpected string. Same: else → executable(), unchanged from base. No path becomes stricter than before.

Two things I checked that the diff alone would not show, both clean:

• The new status is final. market.place_order(..., execute=False) (which sets placing()) runs before the branch, and Transaction.place_order skips _validate_controls when execute=False, so there is no later order.executable() reset that could override it — which is what would otherwise happen on a control violation.
• execution_complete() is a thin setter. It sets status, stamps date_time_execution_complete, and clears update_data; it does not zero size_remaining. So even a hypothetical false complete would not fabricate a zero remaining size in the reported figures — it would make the order non-cancellable (BetfairOrder.cancel raises unless status is EXECUTABLE) and skip it in replace_instructions. That is why getting this right matters, and why I looked for a false-positive path hard rather than assuming the enum is trustworthy.

One honest side effect, same shape as the pre-existing EXPIRED arm: if process.py has already called execution_complete() and the new branch calls it again, date_time_execution_complete is re-stamped to "now", so elapsed_seconds_executable reflects the later of the two. That field is a latency metric, not a matching quantity. Not worth a finding.

3. The "no new attribute access" claim — verified true for the place side

Confirmed, and it is the right claim to make. instruction_report.order_status is already dereferenced at the very first if (line 30) on the same object, inside the same status == "SUCCESS" guard. The new elif at line 35 performs a second comparison against an attribute that has already been read. Whatever betfairlightweight does with a missing key — raise or return None — base code already exercised that exact access on that exact object, so the new arm adds no new attribute access and no new failure mode for absent or None order_status. I read the code and confirmed this; I did not execute anything, so this is a code-reading conclusion, not a run.

It is worth being precise that the claim does not extend to the replace side, where place_instruction_reports.order_status is a genuinely new dereference. That is the LOW at betfairexecution.py:212-219 above, including why its position after market.place_order(...) matters. The PR body does not overclaim here — it scopes the assertion to the place branch.

4. The unhandled PENDING/EXPIRED in execute_replace — I checked the argument, and it holds

I did not accept this on the author's word; it rests on a default argument elsewhere, so I traced it.

PENDING — holds, for every path flumine itself builds. flumine/execution/transaction.py Transaction.execute() is the only place inside the library that constructs a REPLACE package, and it calls self._create_order_package(self._pending_replace, OrderPackageType.REPLACE) with no async_, taking the async_: bool = False default; only the PLACE branch passes async_=self._async_place_orders. BaseOrderPackage.async_ is therefore always False for a replace, and BetfairExecution.replace() forwards async_=order_package.async_ to replace_orders. A search for OrderPackageType.REPLACE confirms transaction.py is the sole construction site — baseexecution.py and simulatedexecution.py only dispatch, and betdaqexecution.py raises NotImplementedError.

The residual fragility is real and I can name the exact bypass, which is more useful than restating "it rests on a default": the safety is a default argument, not an invariant. A caller who builds BetfairOrderPackage(..., package_type=OrderPackageType.REPLACE, async_=True) by hand and hands it to flumine.process_order_package() gets an async replace, Betfair returns a nested orderStatus: "PENDING", and the else swallows it into executable() — the same misreport this PR exists to kill, reachable by bypassing Transaction. That is why the earlier gate's LOW (a one-line comment recording the invariant, or mirroring the sibling's PENDING arm as cheap insurance) is the right remedy rather than a demand. I am not re-raising it as a new finding; nothing in the new commits changed this half.

EXPIRED — holds. EXPIRED means unavailable due to a time-in-force constraint. BetfairOrder.replace() permits only LIMIT / LIMIT_ON_CLOSE and requires status == EXECUTABLE; an FOK order never rests on the book, so it cannot reach the replace path. Leaving it alone is the right call for live-money code.

5. Scope and Closes #743 — accurate, and the place gap is pre-existing not a regression

The place-side gap is byte-identical to base: the execute_place hunk adds 3 lines and removes none, so the base code did route EXECUTION_COMPLETE to executable(). That makes it a pre-existing defect, not a regression introduced here, and it is squarely what #743 asks for — the issue title is "order status set wrong on bets matched immediately on placing" and the body says "suspect same might happen when initially placing an order". Closes #743 is honest now that both halves are covered and the residual PENDING/EXPIRED in execute_replace is verified unreachable. The place fix does not do anything the issue did not ask for.

I also checked the other venue implementations so the fix's blast radius is understood: BetdaqExecution raises NotImplementedError for REPLACE, and SimulatedExecution.execute_replace is a structurally different flow (its replacement is created in placing() state and re-placed through a second pass). No consistency requirement is violated, and the fix is correctly scoped to the Betfair live path where #743 lives.

The new tests are also a genuine coverage gain worth crediting: tests/test_execution.py contains no reference to order_status at all, so the entire order_status branch of execute_place had zero coverage before this diff. The four guard tests are the first coverage of the PENDING/EXPIRED/else arms.

Security

No injection, deserialisation, authz, path-handling or network surface is touched. The production change is one enum string comparison per path; the rest is mock values in two test modules.

I could not check the secret posture. run_secret_scanning returned "Repository does not have GitHub Advanced Security enabled." I am therefore not reporting it as clean. What I can say from reading: I read all three files in full and the diff contains nothing resembling a credential. The issue #743 reproduction log does contain real betIds and marketIds, but that is a pre-existing public issue, not this change.

CI — as reported by GitHub, not as my own result

get_check_runs returns 0 check runs for head bf29ac30, and the workflow-run list for this branch returns 0 runs. This is the normal state for a first-time fork: workflows do not run until a maintainer approves them. Blocked — neither a pass nor a failure, and not mine to call either way. The 1031→1037 test counts, the black results, and the mutation results referred to above are the verification stage's reported output; I ran no code, executed no tests, and built nothing. I have not independently confirmed any of them.

What I could not review, and why

• I cannot execute anything. Every test-related number in this review is the verification stage's reported result, not mine. I did not run the suite, black, or a build, so I cannot confirm the 1037/0-failure result or the mutation outcomes first-hand. What I verified independently is the code, and the code is right.
• Correction to that stage's report: the claim that the new place tests "fail 10/10 on base" does not hold — 4 of the 5 pass against base code by construction (MEDIUM above). I reached that by reading each test against the base branch, not by running them, so my correction is a code-reading conclusion too.
• No Betfair live-API check. I cannot confirm exchange-side that replaceOrders never returns a nested orderStatus: "PENDING" on a synchronous call. §4's unreachability is a framework-side argument (flumine never sets async_ on a replace package), which is strong but is not an exchange-side guarantee.
• I did not audit the simulation path for the same defect. SimulatedExecution.execute_replace is a separate implementation and untouched here; I read enough of it to confirm it is structurally different, not enough to certify it.
• Python matrix coverage is untested. Per the verification stage's own limitation, only 3.13 was available locally while CI's matrix is 3.10–3.14, so four legs are unexercised. Nothing in this diff is version-sensitive as far as I can tell, but I did not verify it.