#169 - EcoStrategy scenario D shares after_tick closure across runs — leader_guarded pollution invalidates ordering deltas - code/yieldomega

Brouie commented

2026-05-06 05:55:11 +00:00

(Migrated from gitlab.com)

logic bug in simulations/ecostrategy/scenarios.py that pollutes scenario D output. caught on a code review pass against 390f9be.

scenario D goal is "same setup, different ordering" — paired buy-first vs pvp-first runs with identical seed/pop/horizon, so the ordering deltas (ordering_bp_leader_idx_match, ordering_steals_abs_delta, etc.) reflect ordering and only ordering. the comparison breaks because both runs share the same after_tick closure object, and the closure carries mutable nonlocal state (leader_guarded) that persists across the runs.

Repro

scenarios.py:448 builds the closure once:

after_tick = _scenario_c_style_after_tick(
    population=population,
    predators=predators,
    pvp_start=pvp_start,
    steal_prob=steal_prob,
    revenge_prob=revenge_prob,
    guard_once=guard_once,
)

scenarios.py:459 and :477 pass the same after_tick to both _simulate_sale calls (m_buy and m_pvp).

inside the factory at scenarios.py:329-339, leader_guarded is captured via nonlocal:

def _scenario_c_style_after_tick(...) -> AfterTick:
    leader_guarded = False
    def after_tick(now_sec, w, r) -> None:
        nonlocal leader_guarded
        ...
        if guard_once and not leader_guarded:
            w.try_guard(leader, now_sec)
            leader_guarded = True

after m_buy runs, leader_guarded = True. when m_pvp runs against the same closure, the if guard_once and not leader_guarded branch never fires — m_pvp gets zero guards.

Observable in current output

output/ecostrategy_report.json scenario D with seed 42:

buy_first_warbow_guards: 1
pvp_first_warbow_guards: 0

the divergence is structural, not ordering-driven. the leader in pvp_first runs the entire final window unguarded, so steals against them apply WARBOW_STEAL_DRAIN_BPS (10%) instead of WARBOW_STEAL_DRAIN_GUARDED_BPS (1%). that means pvp_first_warbow_steals and _revenges are inflated by guard-state pollution rather than pure ordering effects.

Why this matters

every metric scenario D produces under _abs_delta (ordering_steals_abs_delta, ordering_revenges_abs_delta, ordering_flags_planted_abs_delta) and the BP leader comparison (ordering_bp_leader_idx_match) is partially driven by this guard-state asymmetry, not just the ordering flag. the audit narrative for scenario D as MEV-relevant ordering risk is undermined because the experiment does not actually isolate ordering.

Fix shape

build after_tick separately for each run:

after_tick_buy = _scenario_c_style_after_tick(...)
after_tick_pvp = _scenario_c_style_after_tick(...)
m_buy = _simulate_sale(..., after_tick=after_tick_buy, final_window_pvp_first=False, ...)
m_pvp = _simulate_sale(..., after_tick=after_tick_pvp, final_window_pvp_first=True, ...)

or convert the factory to a class with an explicit reset() method.

Test gap

simulations/tests/test_ecostrategy.py:94-99 only asserts metric keys exist and that launch_anchor_cl8y_per_charm_at_clearing > 0. no assertion on warbow_guards parity between buy_first and pvp_first under deterministic seeds. would have caught the asymmetry. recommend adding:

self.assertEqual(int(d.metrics["buy_first_warbow_guards"]), int(d.metrics["pvp_first_warbow_guards"]))

or assert both are nonzero when guard_once=True.

Disposition vs #161

this was missed during my #161 D+E PASS verification. the metrics are still interpretable as upper-bound ordering signals, but they are not pure. happy to follow up on #161 with the corrected interpretation once this is fixed.

severity: P2 sim correctness. not a blocker — sims are non-authoritative per INV-SIM-161 — but the fix is trivial and restores the audit narrative for D.

cc @PlasticDigits

logic bug in `simulations/ecostrategy/scenarios.py` that pollutes scenario D output. caught on a code review pass against `390f9be`. scenario D goal is "same setup, different ordering" — paired buy-first vs pvp-first runs with identical seed/pop/horizon, so the ordering deltas (`ordering_bp_leader_idx_match`, `ordering_steals_abs_delta`, etc.) reflect ordering and only ordering. the comparison breaks because both runs share the same `after_tick` closure object, and the closure carries mutable `nonlocal` state (`leader_guarded`) that persists across the runs. ## Repro `scenarios.py:448` builds the closure once: ``` after_tick = _scenario_c_style_after_tick( population=population, predators=predators, pvp_start=pvp_start, steal_prob=steal_prob, revenge_prob=revenge_prob, guard_once=guard_once, ) ``` `scenarios.py:459` and `:477` pass the same `after_tick` to both `_simulate_sale` calls (m_buy and m_pvp). inside the factory at `scenarios.py:329-339`, `leader_guarded` is captured via `nonlocal`: ``` def _scenario_c_style_after_tick(...) -> AfterTick: leader_guarded = False def after_tick(now_sec, w, r) -> None: nonlocal leader_guarded ... if guard_once and not leader_guarded: w.try_guard(leader, now_sec) leader_guarded = True ``` after `m_buy` runs, `leader_guarded = True`. when `m_pvp` runs against the same closure, the `if guard_once and not leader_guarded` branch never fires — m_pvp gets zero guards. ## Observable in current output `output/ecostrategy_report.json` scenario D with seed 42: - `buy_first_warbow_guards: 1` - `pvp_first_warbow_guards: 0` the divergence is structural, not ordering-driven. the leader in pvp_first runs the entire final window unguarded, so steals against them apply `WARBOW_STEAL_DRAIN_BPS` (10%) instead of `WARBOW_STEAL_DRAIN_GUARDED_BPS` (1%). that means `pvp_first_warbow_steals` and `_revenges` are inflated by guard-state pollution rather than pure ordering effects. ## Why this matters every metric scenario D produces under `_abs_delta` (`ordering_steals_abs_delta`, `ordering_revenges_abs_delta`, `ordering_flags_planted_abs_delta`) and the BP leader comparison (`ordering_bp_leader_idx_match`) is partially driven by this guard-state asymmetry, not just the ordering flag. the audit narrative for scenario D as MEV-relevant ordering risk is undermined because the experiment does not actually isolate ordering. ## Fix shape build `after_tick` separately for each run: ``` after_tick_buy = _scenario_c_style_after_tick(...) after_tick_pvp = _scenario_c_style_after_tick(...) m_buy = _simulate_sale(..., after_tick=after_tick_buy, final_window_pvp_first=False, ...) m_pvp = _simulate_sale(..., after_tick=after_tick_pvp, final_window_pvp_first=True, ...) ``` or convert the factory to a class with an explicit `reset()` method. ## Test gap `simulations/tests/test_ecostrategy.py:94-99` only asserts metric keys exist and that `launch_anchor_cl8y_per_charm_at_clearing > 0`. no assertion on `warbow_guards` parity between buy_first and pvp_first under deterministic seeds. would have caught the asymmetry. recommend adding: ``` self.assertEqual(int(d.metrics["buy_first_warbow_guards"]), int(d.metrics["pvp_first_warbow_guards"])) ``` or assert both are nonzero when `guard_once=True`. ## Disposition vs #161 this was missed during my #161 D+E PASS verification. the metrics are still interpretable as upper-bound ordering signals, but they are not pure. happy to follow up on #161 with the corrected interpretation once this is fixed. severity: P2 sim correctness. not a blocker — sims are non-authoritative per `INV-SIM-161` — but the fix is trivial and restores the audit narrative for D. cc @PlasticDigits

Brouie commented

2026-05-06 05:55:50 +00:00

(Migrated from gitlab.com)

mentioned in issue #161

PlasticDigits commented

2026-05-06 08:40:10 +00:00

(Migrated from gitlab.com)

mentioned in commit d734901f7d

mentioned in commit d734901f7d6ee94cd8fc805d36871a927a77d30c

PlasticDigits commented

2026-05-06 08:40:50 +00:00

(Migrated from gitlab.com)

Implemented (merged to `main` — `d734901`)

Problem: Scenario D reused one _scenario_c_style_after_tick closure for both buy_first and pvp_first runs. The leader_guarded nonlocal carried over, so after the first run the second got no guard_once guard — ordering deltas were polluted (e.g. seed 42: guards 1 vs 0).

Change: run_scenario_d now builds after_tick_buy and after_tick_pvp separately.

Tests: test_scenario_d_ordering_branches asserts buy_first_warbow_guards == pvp_first_warbow_guards; test_scenario_d_warbow_guards_parity_seed_42 pins the reported regression.

Docs: INV-SIM-169-D in docs/testing/invariants-and-business-logic.md; cross-links in simulations/README.md and skills/README.md.

Verification checklist

cd simulations && PYTHONPATH=. python3 -m unittest discover -s tests -v — all green (ran twice locally).
PYTHONPATH=. python3 -m ecostrategy --scenario D --seed 42 --out /tmp/d.json — JSON shows buy_first_warbow_guards == pvp_first_warbow_guards (expect 1 / 1 for defaults).
Skim run_scenario_d docstring + INV-SIM-169-D subsection for the closure-isolation invariant.

@brouie — could you confirm this matches your review expectations and that scenario D metrics are now ordering-isolated as intended? Leaving the issue open for your sign-off.

## Implemented (merged to `main` — d734901) **Problem:** Scenario D reused one `_scenario_c_style_after_tick` closure for both `buy_first` and `pvp_first` runs. The `leader_guarded` `nonlocal` carried over, so after the first run the second got no `guard_once` guard — ordering deltas were polluted (e.g. seed 42: guards 1 vs 0). **Change:** `run_scenario_d` now builds `after_tick_buy` and `after_tick_pvp` separately. **Tests:** `test_scenario_d_ordering_branches` asserts `buy_first_warbow_guards == pvp_first_warbow_guards`; `test_scenario_d_warbow_guards_parity_seed_42` pins the reported regression. **Docs:** `INV-SIM-169-D` in `docs/testing/invariants-and-business-logic.md`; cross-links in `simulations/README.md` and `skills/README.md`. --- ### Verification checklist - [ ] `cd simulations && PYTHONPATH=. python3 -m unittest discover -s tests -v` — all green (ran twice locally). - [ ] `PYTHONPATH=. python3 -m ecostrategy --scenario D --seed 42 --out /tmp/d.json` — JSON shows `buy_first_warbow_guards` == `pvp_first_warbow_guards` (expect `1` / `1` for defaults). - [ ] Skim `run_scenario_d` docstring + `INV-SIM-169-D` subsection for the closure-isolation invariant. --- @brouie — could you confirm this matches your review expectations and that scenario D metrics are now ordering-isolated as intended? Leaving the issue **open** for your sign-off.

PlasticDigits commented

2026-05-06 08:57:29 +00:00

(Migrated from gitlab.com)

mentioned in issue #167

Brouie commented

2026-05-07 02:13:42 +00:00

(Migrated from gitlab.com)

@PlasticDigits — verification PASS on d734901.

closure isolation lands at scenarios.py:495-510 — after_tick_buy and after_tick_pvp built from separate _scenario_c_style_after_tick(...) factory calls, each holding its own leader_guarded nonlocal. passed to _simulate_sale at :525 and :543 respectively. no shared closure path remains.

both regression tests landed in simulations/tests/test_ecostrategy.py:

test_scenario_d_ordering_branches at :148-159 — equality assertion on buy_first_warbow_guards vs pvp_first_warbow_guards, with explicit GitLab #169 cite at line 154
test_scenario_d_warbow_guards_parity_seed_42 at :161-167 — pins the originally-filed regression (seed 42, population 240, horizon 28_800)

docstring on run_scenario_d makes the closure-isolation contract explicit. matches the filed report.

rng_buy and rng_pvp re-seeding behavior preserved (both still start from the same seed so cross-run determinism holds). only warbow_guards parity is asserted equal — steals/revenges/flags-planted are intentionally not pinned because measuring those deltas is the point of scenario D in the first place. correct scope.

ready for close on your side.

@PlasticDigits — verification PASS on `d734901`. closure isolation lands at `scenarios.py:495-510` — `after_tick_buy` and `after_tick_pvp` built from separate `_scenario_c_style_after_tick(...)` factory calls, each holding its own `leader_guarded` nonlocal. passed to `_simulate_sale` at `:525` and `:543` respectively. no shared closure path remains. both regression tests landed in `simulations/tests/test_ecostrategy.py`: - `test_scenario_d_ordering_branches` at `:148-159` — equality assertion on `buy_first_warbow_guards` vs `pvp_first_warbow_guards`, with explicit `GitLab #169` cite at line 154 - `test_scenario_d_warbow_guards_parity_seed_42` at `:161-167` — pins the originally-filed regression (seed 42, population 240, horizon 28_800) docstring on `run_scenario_d` makes the closure-isolation contract explicit. matches the filed report. rng_buy and rng_pvp re-seeding behavior preserved (both still start from the same seed so cross-run determinism holds). only warbow_guards parity is asserted equal — steals/revenges/flags-planted are intentionally not pinned because measuring those deltas is the point of scenario D in the first place. correct scope. ready for close on your side.

PlasticDigits (Migrated from gitlab.com) closed this issue

2026-05-09 04:56:51 +00:00

PlasticDigits commented

2026-05-13 15:13:33 +00:00

(Migrated from gitlab.com)

mentioned in commit 8560f23bac

mentioned in commit 8560f23bace59ff395b138191b5ce2645d6d1e83

Rows
Columns

EcoStrategy scenario D shares after_tick closure across runs — leader_guarded pollution invalidates ordering deltas #169

Repro

Observable in current output

Why this matters

Fix shape

Test gap

Disposition vs #161

Implemented (merged to main — d734901)

Verification checklist

Implemented (merged to `main` — `d734901`)