feat(engine): stop flatten after 3 attempts that close nothing, so it never loops

[MarcosACH]

Closes #473

A thin book, a missing price, or a failed read could make flatten loop forever. Now each held symbol gets at most 3 attempts in a row that close nothing. After that, flatten stops with flatten.gave_up and exits 1.

What changes

• A dry attempt is no fill, no price, a failed read, or a size gap that a drain does not close.
• A fill resets the count. The loop waits 2 seconds on the engine clock after a dry attempt.
• A failed reconcile pass is a dry attempt. Flatten never sends a second close beside an open one.
• When the venue reads flat, each strategy's leftover moves to the unattributed partition at its entry price. Its realized PnL does not change.
• Every held symbol runs its own loop, all at the same time. Only one reconcile pass runs at a time.
• Flatten keeps no seq counter of its own. SignalEmitter moved to domain, and every close goes through place().
• The catalog gains flatten.dry_attempt and flatten.gave_up.

Changes from the architecture review

These came from /improve-codebase-architecture on this branch. Two of them change behavior.

• One loop, one meaning for a venue read. The wait for a close the boot left open now uses the same verdict wait and read as an attempt. Before, a failed read there stopped the whole run. A failed pass there was not counted. Both are now dry attempts, as ADR-0056 and ADR-0060 say. A loop returns nothing, and only the final read reports what is left.
• Each attempt reads before it counts. An attempt with no price or a size gap now reads the venue too. Before, a venue that went flat during those waits ended in flatten.gave_up. The leftover never moved, yet the run still exited 0. ADR-0056 says a flat venue ends flatten. The loop now only counts and waits. One _attempt function says how each attempt ended.
• One definition of account net size. The size gap check now reads portfolio.account_net(). It no longer sums the split basis a second time.

Not in this PR

• reconcile_account() for a gap the drain does not close. It moves to Run flatten on Hyperliquid through refusals and rate limits #475, which brings a live venue.
• A symbol the venue already reads flat at the boot read. That is Move a strategy's size to unattributed when the venue is already flat at the flatten boot read #489.

Checks

• uv run pytest: 2031 passed, 49 skipped.
• mypy, ruff check, ruff format --check, and lint-imports are clean.

🤖 Generated with Claude Code

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[MarcosACH]

Review of ralph/issue-473 @ 20d4ada

Reviewer: code-review skill
Reviewed: 2026-10-12
Optimization axis: correctness

Mechanical

• ruff format: pass
• ruff check: pass
• mypy: pass
• lint-imports: pass (7 kept, 0 broken)
• pytest: pass (2031 passed, 49 skipped, coverage 97.13%)

Findings

[BLOCKING] R001 — src/tickwright/engine/exit_run.py:152 — Concurrent loops can open several closes on one symbol

Category: concurrency
Why: InMemoryBus.publish treats a publish made while another cascade is running as reentrant. It queues the signal and returns at once. The loops now run as concurrent tasks. So when one loop's close yields mid-cascade, another loop's place() returns before its signal is handled. _decided (line 387) then reads the missing saga as decided. _filled reads it as no fill. The loop counts a false dry attempt and sends the next close beside the first one. This breaks ADR-0056: a symbol never has two attempts open.
Reproduced on paper with a venue double whose place does await asyncio.sleep(0) before it forwards, holding BTC 0.02 and ETH 0.5:

places ['BTC', 'ETH', 'ETH', 'ETH']
flatten.dry_attempt ETH count=1 reason=no_fill
flatten.dry_attempt ETH count=2 reason=no_fill
flatten.dry_attempt ETH count=3 reason=no_fill
flatten.gave_up ETH size=0.5
exit.finished outcome=done exit 0

Three ETH closes reached the venue. Only paper's reduce-only rule kept the books right. The run also logs gave_up for a symbol that closed. ManualClock and paper never yield, so the shipped tests stay serial. A live venue's HTTP place yields, and #475 brings one.
Fix: One loop must not publish while another loop's cascade is open. For example, hold the shared _Passes lock across the send as well as the pass. Add a test with a place that yields for two symbols. It should assert one close per symbol per attempt and no no_fill on a close that filled.
Status: open

[WARN] R002 — src/tickwright/engine/exit_run.py:340 — No test for 3 failed reads in a row after a close

Category: testing
Why: Coverage shows the give-up branch in _read_left (line 341) never runs. The tests fail one read, then let the next succeed. So nothing proves a venue that never answers ends the loop. That is the "never hangs" acceptance item.
Fix: Add a test where every fetch_positions after the boot read fails. Assert 3 read_failed dry attempts, one flatten.gave_up, and exit 1 with left from the final read.
Status: open

[WARN] R003 — src/tickwright/engine/exit_run.py:322 — flatten.gave_up can report a size that is already gone

Category: api
Why: After a give-up in _verdict or _read_left, size is the size before the close went out. If the close partly filled, the event names more than is left. The ADR-0060 amendment in this PR calls the field "the size left".
Fix: Either report the books' size for the symbol at give-up, or reword the ADR-0060 amendment to "the last size the venue reported".
Status: open

[WARN] R004 — src/tickwright/engine/checkpoint.py:336 — The unattributed entry price can be zero or negative

Category: testing
Why: With two strategies of opposite sign and different entries, cost / moved is not a real price. Example: A holds +2 at 100 and B holds -1 at 300. Unattributed gets +1 at -100. No test covers more than one strategy, and the store accepts the value. Invariant 9 makes this case rare (old partitions after a config change). But the code keeps machinery for it that nothing proves.
Fix: Add a two-strategy test and pick the rule on purpose. Either keep the cost-basis rule and document that the entry can be any number, or move one piece per strategy at its own entry. Ask Marcos if unsure, since ADR-0054 only shows the one-strategy case.
Status: open

Summary

• BLOCKING: 1 open
• WARN: 3 open
• NIT: 0 open
• Merge: BLOCKED

Ralph-State: changes-requested

[MarcosACH]

Review findings addressed. Checks at 7cb2c29: ruff, mypy, and lint-imports are clean. pytest: 2035 passed, 49 skipped, coverage 97.17%.

R001 — Status: resolved — b261f3b
A close now takes the same lock as a pass (_Passes became _Turns). So no loop publishes while another loop's cascade is open. A close that still has no saga after its publish now raises InvariantViolation. It is not counted as dry. That covers a publish from the venue's own task, which the lock does not cover. New tests: test_a_send_that_waits_on_the_network_gets_no_second_close_beside_it and test_a_close_the_engine_never_handled_stops_the_run_with_no_second_close. Both were red first.

R002 — Status: resolved — f1b3d31
test_a_venue_that_never_answers_after_a_close_gives_up_and_the_run_ends gives 3 read_failed dry attempts, one gave_up, and exit 1. It was green on its first run, because only the test was missing. The _read_left give-up line is now covered.

R003 — Status: resolved — 24e3665
I took the doc fix. The ADR-0060 amendment and the gave_up docstring now say size is the last size the venue reported. After failed reads it can be from before the last close. exit.finished says what is left. The R002 test asserts this.

R004 — Status: resolved — 7cb2c29
I kept the cost-basis rule. ADR-0054 says the leftover move opens no cash gap. One piece per strategy would book realized PnL in the unattributed partition and move cash. A two-strategy test now pins the price: 0.02 at 42000 and -0.01 at 44000 give +0.01 at 40000. The docstring and the safe-exit map say the price can be zero or negative.

[MarcosACH]

Review of ralph/issue-473 @ 7cb2c29

Reviewer: code-review skill
Reviewed: 2026-10-12
Optimization axis: correctness

Mechanical

• ruff format: pass
• ruff check: pass
• mypy: pass
• lint-imports: pass (7 kept, 0 broken)
• pytest: pass (2035 passed, 49 skipped, coverage 97.17%)

Prior findings

[BLOCKING] R001 — src/tickwright/engine/exit_run.py:191 — Concurrent loops can open several closes on one symbol

Status: resolved — b261f3b (verified)
A close now holds the same lock as a pass. Nothing else publishes outside the lock in a flatten run: the feed has ended, drain() is a no-op on the in-memory bus, and the Kafka publish waits for the whole process to go quiet. A close with no saga now raises InvariantViolation and is never counted as dry. test_a_send_that_waits_on_the_network_gets_no_second_close_beside_it sends one close per symbol and logs no flatten.* event.

[WARN] R002 — src/tickwright/engine/exit_run.py:359 — No test for 3 failed reads in a row after a close

Status: resolved — f1b3d31 (verified)
The give-up line in _read_left is now covered. The test asserts 3 read_failed, one gave_up, no second close, and exit 1.

[WARN] R003 — src/tickwright/engine/exit_run.py:208 — flatten.gave_up can report a size that is already gone

Status: resolved — 24e3665 (verified)
The ADR-0060 amendment and the docstring now say "the last size the venue reported". The R002 test pins it.

[WARN] R004 — src/tickwright/engine/checkpoint.py:337 — The unattributed entry price can be zero or negative

Status: resolved — 7cb2c29 (verified)
The cost-basis rule matches ADR-0054 ("open no cash gap of their own"). Neither the domain type nor the stores reject a non-positive price. The two-strategy test pins 40000. The docstring and the safe-exit map say the price can be zero or negative.

Findings

[NIT] R005 — src/tickwright/engine/exit_run.py:428 — _decided still reads a missing saga as decided

Category: errors
Why: R001 showed that this read is the unsafe one. _attempt now guards it, so the branch is dead, but a new caller without the guard would bring R001 back.
Fix: Move the missing-saga InvariantViolation from _attempt into _decided, so the rule lives in one place.
Status: open

Summary

• BLOCKING: 0 open
• WARN: 0 open
• NIT: 1 open
• Merge: READY

Ralph-State: ready

[MarcosACH]

R005 — Status: resolved — 3d326f0
_decided now raises InvariantViolation when a close has no saga. The check in _attempt is gone, so the rule lives in one place. Behavior is unchanged. test_a_close_the_engine_never_handled_stops_the_run_with_no_second_close still covers it. Checks at 3d326f0: ruff, mypy, and lint-imports are clean. pytest: 2035 passed, 49 skipped, coverage 97.17%.