tracker: corpses do not exist - revert the fix and retire the workaround

The belief "BotDeathEvent never reaches ModularBot, so enemyTracker keeps dead enemies alive forever" was written into a code comment and then believed twice. It is FALSE. Measured in a 7-bot melee with a per-tick probe comparing enemyTracker's alive count against the server's getEnemyCount(): metric 1.3.1 (20 rd) 0.35.5 (15 rd) observed enemy deaths 83 68 ...non-round-ending 83 (100%) 66 (97%) ekBotDeath events DROPPED 0 0 max dispatch lag (turns behind) 1 1 phantom ticks 1 / 16,820 1 / 12,596 MAX CORPSE LIFETIME 0 ticks 0 ticks victims still alive at round end 0 0 onBotDeath fires for every death, including non-round-ending ones. The API-level event-drop mechanism IS real (test_event_drop_mechanism.nim proves it: ekBotDeath is not in isCritical and MAX_EVENTS_AGE=2) - the bot simply never falls far enough behind for it to trigger (max lag 1 turn). Removed: - reconcileWithServer + ReconcilePersistTicks/mismatchTicks/sawServerAlive (uncommitted, and ON BY DEFAULT despite the premise being false). Its own comment admitted a shorter window once KILLED A LIVE ENEMY ("it fired three more times after the tracker marked it dead") - a latent mis-prune path defending against a bug that does not exist. - The radar's CorpseTicks=40 filter and the same-class age>60 filter in recordRadarStats, both carrying the false comment. Removal changes no real behaviour: buildState feeds the radar enemyTracker.allAlive(), so a dead enemy never reaches computeScan. Kept: - The TR_TRACKER_PROBE instrument (default OFF), which produced the table above. - test_event_drop_mechanism.nim - the drop mechanism is a genuine library behaviour worth guarding. - isAlive/aliveCount on the tracker. Added: docs/tracker_death_events.md (the durable negative, so this is not re-invented a third time) and test_enemy_tracker_death.nim (13 checks) in place of the test for the deleted feature. Guards: test_gun_harness 39/39, test_vbullet_metric 11, test_power_selection 3, test_adaptive_radar 41/41, test_event_drop_mechanism 6, test_enemy_tracker_death 13, acceptance 12/12, ModularBot compiles.
2026-09-21 22:41:11 +02:00
parent c091bf3c34
commit fb36a0a685
8 changed files with 429 additions and 44 deletions
@@ -0,0 +1,74 @@
+# Enemy-tracker deaths: there are no corpses
+
+**Question.** Two independent agents believed *"`BotDeathEvent` never reaches
+ModularBot, so dead enemies stay `alive` in `enemyTracker` forever."* If true,
+every consumer of the tracker (targeting, the adaptive melee radar, the
+virtual-bullet enemy table) would silently mis-prune. This note records the
+direct measurement that settled it and the verdict, so the premise is not
+re-invented a third time.
+
+## Why the premise *sounds* right (and is still real)
+
+In `robocode_tankroyale_botapi` 1.0.7 the event queue's `isCritical` set is
+only `ekDeath`, `ekWonRound`, `ekSkippedTurn` — **not** `ekBotDeath`.
+`removeOldEvents` deletes any non-critical event with
+`turnNumber < currentTurn - MAX_EVENTS_AGE` (2). So a bot that falls more than
+two turns behind *does* drop `BotDeathEvent`. That mechanism is genuine and is
+proven at the API level by `common_libs/tests/test_event_drop_mechanism.nim`.
+
+The mistake was assuming the mechanism *triggers* in a real ModularBot match.
+
+## Measurement
+
+A 7-bot melee (ModularBot + DrussGT + 5 legacy sample bots). A per-tick probe
+compared `enemyTracker` alive-count against the server's `getEnemyCount()`, and
+a queue probe inspected the event queue **before** `removeOldEvents` ran.
+The probe ran with `TR_TRACKER_RECONCILE=0` — i.e. raw behaviour, no
+reconciliation masking anything.
+
+| metric | server 1.3.1 (20 rounds) | server 0.35.5 (15 rounds) |
+|---|---:|---:|
+| observed enemy deaths | 83 | 68 |
+| non-round-ending deaths | 83 (100%) | 66 (97%) |
+| `ekBotDeath` events DROPPED | 0 | 0 |
+| max dispatch lag (turns behind) | 1 | 1 |
+| phantom ticks (`tracker_alive > server_ec`) | 1 / 16,820 | 1 / 12,596 |
+| max corpse lifetime | **0 ticks** | **0 ticks** |
+| victims still alive at round end | 0 | 0 |
+
+The maximum dispatch lag of 1 turn is below `MAX_EVENTS_AGE` (2), so the drop
+path is never reached. `onBotDeath` fires for **every** death, including
+non-round-ending ones, and `markDead` takes effect on the same tick.
+
+## Verdict
+
+**There are no corpses.** `onBotDeath` + `EnemyTracker.markDead` is a complete,
+prompt death signal at this workload. The API-level drop mechanism is real but
+never triggered. The two rare phantom ticks resolve within one tick and are not
+persistent corpses.
+
+## Do not re-add a corpse workaround
+
+* Do **not** reintroduce tracker reconciliation against `getEnemyCount()`
+  (the removed `reconcileWithServer`). It was a latent mis-prune path: its own
+  doc comment recorded that a short persistence window **marked a live enemy
+  dead** (it then "fired three more times").
+* Do **not** add an "unseen for N ticks => treat as dead" filter to
+  `adaptive_melee_radar` or any other consumer. The tracker's `alive` flag (and
+  `allAlive()` / `aliveCount()`) already excludes dead enemies. Treating a
+  long-unseen entry as dead is actively wrong: it is a stale *live* enemy that
+  the freshness fallback must re-acquire.
+* The `CorpseTicks = 40` radar filter and the `>60`-tick coverage filter that
+  existed only because of this premise have been removed.
+
+If a future workload ever *does* show corpses (e.g. a much larger melee, a
+higher-latency link), reproduce the measurement above first — do not patch from
+the premise.
+
+## Artifacts
+
+* Instrument: `TR_TRACKER_PROBE=1` in `ModularBot` (default off; writes JSONL to
+  `TR_TRACKER_PROBE_PATH`). Kept because it produced the table above.
+* Measurement harness: `common_libs/tests/measure_corpse_melee.nim`.
+* Kept API-level proof: `common_libs/tests/test_event_drop_mechanism.nim`.
+* Death invariant: `common_libs/tests/test_enemy_tracker_death.nim`.