#45 review pass: correct three claims the fix's own comments got wrong

An adversarial review of the change returned NO BLOCKERS but caught three
statements that were wrong or unverifiable.  All three are corrections to my own
comments/docs, not behaviour changes -- the verified binary is unchanged.

1. The phantom-kill note had the ORDERING backwards.  It claimed the owner's
   record "overwrites it on the next record -- the phantom stops being visible".
   The engine's event priorities run the other way: an inbound update record is
   posted at UpdateEventPriority == MaxEventPriority and drained by
   ExecuteBackgroundTask, while the rerouted score message sits at
   EntityManagerEventPriority == DefaultEventPriority and is popped later -- so
   the correction usually lands FIRST and the phantom AFTER it.  The artifact is
   BOUNDED, not masked: on the killer's pod only, that victim's KILLS reads +1
   until the next record, i.e. <= one 2s heartbeat instead of "until the victim
   next scores or dies".  Swept in btplayer.cpp, KD_SCOREBOARD_PLAN.md and
   RECONCILE.md.

2. The corpus figures were not reproducible, and were mis-tiered.  I cited
   "125 of 125 SCORE type=2 over 255 node-logs" and "0 of 8800 DMG rows" -- true
   when measured, but the corpus is append-only and this fix's OWN verification
   runs grew it from ~255 to 425 files, so nobody can re-derive them.  Replaced
   everywhere with invariant RATIOS that hold at any corpus size, re-measured
   here:
     * 0 of 18818 DMG rows are inst=R (damage is applied master-side only)
     * 439 of 439 DEATH inst=R rows read killer=0:0 killdmg=0.000, against
       0 of 225 inst=M rows (sole lastInflictingID writer: mech.cpp:746)
   Also re-tagged T1 -> T2: log-corpus field evidence is T2 per the CLAUDE.md
   tier table.  Lesson recorded in the banner: cite ratios, not counts, for a
   corpus your own rigs append to.

3. The recordLength guard's justification was wrong (and the plan's risk 5 with
   it).  A short legacy record does NOT desync the stream -- both sides advance
   by the WRITER's stamped recordLength and no reader asserts a length.  The real
   hazard is a read PAST THE ALLOCATION: an inbound update message lives in a
   per-event heap block sized from messageLength, so on a TRAILING legacy record
   the two new fields would be read off the end of it.

Plus: the record struct's own field comment still said "deathCount -- the DEATHS
column", the exact wrong-field belief #45 removed, sitting on the wire struct.

Filed the review's other finding on #60: a SECOND decomp export gap, index
jumping FUN_0049fe80 (ends 0x49ffc8) -> FUN_004a1232, 4714 unexported bytes
containing Mech::TakeDamageMessageHandler @0x4a0230 -- the producer of every
score and death message in the game.  Verified independently against
functions_index.tsv before posting.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

context/gauges-hud.md

@@ -336,14 +336,15 @@ locked**. **SUPERSEDED -- that "RESOLVED" claim was FALSE (corrected 2026-07-25,
 observed-tally design never ran: `BTPlayerCountObservedDeath`'s only call site is
 replicant-gated but sits INSIDE `Mech::UpdateDeathState`'s once-per-death transition, which a
 replicant never enters (it takes the mode-9 early return) -- and it could not have worked
-anyway, because a replicant victim carries no attribution at all (**0 of 8800** `DMG` rows in
+anyway, because a replicant victim carries no attribution at all (**0 of 18818** `DMG` rows in
-the corpus target a replicant, so every `DEATH inst=R` row reads `killer=0:0`). What actually
+the corpus are `inst=R`, which is why **439 of 439** `DEATH inst=R` rows read `killer=0:0`
+against **0 of 225** `inst=M` rows). What actually
 happened: BOTH counters lived only on the OWNING pod, so every REMOTE row read 0/0 all
 mission -- KILLS because `Entity::Dispatch` reroutes a replicant's ScoreMessage to the master
 (ENTITY.cpp:244-251), so `++killCount` lands on the killer's own machine; DEATHS because the
 `VehicleDead(-1)` handler runs on the victim's own master. Neither counter rode an update
-record. Measured: **125 of 125** `SCORE type=2` rows across 255 node-logs credit the LOGGING
+record. [T2 — stated as ratios above; an earlier draft's "125 of 125 over 255 node-logs" was
-node's own player; not one credits a remote pilot. [T1]
+true when measured but is not reproducible, because every rig run appends to the same corpus.]
 **FIXED 2026-07-25 [T2, rig-verified]:** DEATHS now reads the binary's OWN `+0x280` column
 (`BTPlayer::deathTally`, our offset 0x274 -- previously declared `pad_0x280` and "dead"), not
 `Player::deathCount` (which is the respawn-handshake identity, seeded -2 -- exactly what the

docs/KD_SCOREBOARD_PLAN.md

@@ -262,11 +262,18 @@ resolved. [T0 on the reroute and the record contents.]
 
 ## Field proof
 
-- **125 of 125** `SCORE type=2` records across **255 node-logs** credit the LOGGING
+**Field proof, as INVARIANT RATIOS.** (An earlier draft cited "125 of 125 over 255 node-logs".
-  node's OWN player; **not one** credits a remote pilot.
+That was true when measured but is not reproducible: every rig run appends to the same corpus, and
-- **0 of 8800** `DMG` rows target a replicant — so a non-victim node genuinely
+the verification runs for this fix alone grew it from ~255 to 425 files. Cite ratios, not counts.)
-  cannot know the killer, which is why every `DEATH inst=R` row reads `killer=0:0`
+- **0 of 18818** `DMG` rows are `inst=R` — damage is applied MASTER-SIDE ONLY, so a non-victim node
-  and why §4's "unreachable observed tally" could never have worked anyway.
+  cannot attribute a kill even in principle. This is also why §4's "unreachable observed tally"
+  could never have worked.
+- **439 of 439** `DEATH inst=R` rows read `killer=0:0 killdmg=0.000`, against **0 of 225** `inst=M`
+  rows — a replicant victim's `lastInflictingID` is never populated locally (sole writer
+  `mech.cpp:746`).
+Both re-derive with a one-line scan over `matchlogs/**` + `content/matchlog*.txt` +
+`content/matchlogs/**` and neither drifts as the corpus grows. [T2 — log-corpus field evidence is
+T2 per the CLAUDE.md tier table, not T1.]
 - Tonight's 3 nodes, one kill: host 3 logged `player=3:1 type=2 award=5.88`; the
   award equals the `killdmg=5.881` recorded on **host 4** — the value crossed the
   wire. Host 4 (the victim, the user's pod) logged the death and no kill at all.
@@ -290,14 +297,23 @@ ships without a new dirty-bit edit — §5 risk 6 is avoided entirely.
 
 Satisfies §5: risk 1 avoided (no new member → no shadow), risk 2 locks added
 (`sizeof(UpdateRecord) > sizeof(Player::UpdateRecord)` and `== base + 2*sizeof(int)`),
-risk 4 respected (exactly two ints), risk 5 **improved** — a `recordLength` guard in
+risk 4 respected (exactly two ints), risk 5 **corrected and improved** — that risk's premise is wrong. A mixed-build
-`ReadUpdateRecord` makes a mixed-build session degrade to "counters don't update"
+session does NOT desync the stream: both sides advance by the WRITER's stamped `recordLength`
-instead of reading a neighbouring record as a kill count.
+(`ENTITY.cpp:390` / `SIMULATE.cpp:323`) and no reader asserts a length. The real hazard the
+`recordLength` guard prevents is a READ PAST THE ALLOCATION — an inbound update message lives in
+a per-event heap block sized from `messageLength`, so on a TRAILING legacy record the two new
+fields would be read off the end of it. With the guard a mixed session degrades to "remote
+counters don't move".
 
-**Bonus:** the §Headline-6 phantom kill (the binary's partner `inc [+0x27c]` on the
+**§Headline-6 phantom kill — BOUNDED, not masked (corrected).** The binary's partner
-VICTIM's player) always lands on a replicant copy, so the owner's authoritative
+`inc [+0x27c]` on the VICTIM's player always lands on a replicant copy, so the owner's record
-value now overwrites it on the next record — the phantom stops being visible
+corrects it. But it does NOT arrive after it: an inbound update record is posted at
-without touching the faithfully-reconstructed handler.
+`UpdateEventPriority == MaxEventPriority` and drained by `ExecuteBackgroundTask`, while the
+rerouted score message is posted at `EntityManagerEventPriority == DefaultEventPriority` and
+popped later — so the correction usually lands FIRST and the phantom after it. Net effect: on
+the killer's pod only, that victim's KILLS reads +1 until the next record, i.e. at most one
+heartbeat (2 s) instead of "until the victim next scores or dies". The increment itself stays
+byte-faithful; whether to drop it is the open fidelity question.
 
 ## Verification (2026-07-25, 2-node loopback, Release, `BT_MP_FORCE_DMG=1`)
 

docs/RECONCILE.md

@@ -1600,9 +1600,9 @@ note: it is `deathTally`, NOT `deaths`/`deathCount`, which would shadow the base
 resolves the killer's `Player` — a REPLICANT there — and calls `Dispatch()`. `Entity::Dispatch`
 reroutes a replicant's message to the owning host (`ENTITY.cpp:244-251`), so `++killCount` lands
 on the killer's own machine and nowhere else, and nothing ever carried it back out. Field proof
-before the fix: across 255 node-logs, **125 of 125** `SCORE type=2` rows credit the LOGGING node's
+before the fix, as invariant ratios (counts drift — every rig run appends to the same corpus):
-own player and **not one** credits a remote pilot; **0 of 8800** `DMG` rows target a replicant, so
+**0 of 18818** `DMG` rows are `inst=R`, so no other node can attribute the kill at all, and
-no other node can even know the killer. The pods' closed LAN made the one-way engine record
+**439 of 439** `DEATH inst=R` rows read `killer=0:0` against **0 of 225** `inst=M` rows. [T2] The pods' closed LAN made the one-way engine record
 sufficient in 1995 only because nothing ever needed the counters off-node — the arcade cabinet
 scoreboard each pod drew was its own.
 
@@ -1618,10 +1618,14 @@ No crash, no `GLITCH` rows.
 **What we did NOT change (kept byte-faithful).** The kill handler's partner increment
 `++sender_owner->killCount` (btplayer.cpp, the binary's `inc [ebx+0x27c]` / `inc [edx+0x27c]`
 @0x4c0397/@0x4c03a3 wrong-column slip) is still reproduced as shipped. It always lands on a
-REPLICANT copy of the victim, so the owner's authoritative record now overwrites it on the next
+REPLICANT copy of the victim, so the owner's authoritative record corrects it — the rig captured
-update — the rig captured the correction repeatedly (`wasKills=3 -> kills=2`,
+that repeatedly (`wasKills=3 -> kills=2`, `wasKills=2 -> kills=1`). It is **bounded, not
-`wasKills=2 -> kills=1`). The phantom kill stops being *visible* without editing the
+masked**: the engine's event priorities run the correction FIRST (an inbound update record is
-reconstructed handler, so the fidelity question stays open rather than being silently decided.
+posted at `UpdateEventPriority == MaxEventPriority` and drained by `ExecuteBackgroundTask`,
+while the rerouted score message sits at `DefaultEventPriority` and is popped later), so the
+phantom usually lands after it. On the killer's pod only, that victim's KILLS reads +1 until
+the next record — at most one 2 s heartbeat. The increment stays byte-faithful, so the fidelity
+question stays open rather than being silently decided.
 
 **Mixed-build note.** `ReadUpdateRecord` early-returns when `recordLength < sizeof(UpdateRecord)`,
 so a pod on a build without the extension degrades to "remote counters don't move" instead of

game/reconstructed/btplayer.cpp

@@ -1822,8 +1822,8 @@ BitMap *BTPilotNameBitmap(void *pilot)
 // (mech4.cpp) and its `friend` (btplayer.hpp) -- all three at once, because /FORCE
 // would otherwise hide a straggler.  It never executed (its call site sat inside the
 // once-per-death TRANSITION, which a replicant never enters) and it could not have
-// worked (a replicant victim carries no attribution: 0 of 8800 corpus DMG rows target
+// worked (a replicant victim carries no attribution: 0 of 18818 corpus DMG rows are
-// a replicant).  DEATHS is now replicated from the owning pod instead.  Do not
+// inst=R, and 439 of 439 DEATH inst=R rows read killer=0:0).  DEATHS is now replicated from the owning pod instead.  Do not
 // resurrect it: under replication it would be a SECOND writer of a replicated counter,
 // and it incremented `Player::deathCount`, which the scoreboard no longer even reads.
 
@@ -1842,12 +1842,20 @@ BitMap *BTPilotNameBitmap(void *pilot)
 // Sends for free: both counter writes already dirty the record --
 // `ForceUpdate()` on the death path (:508) and on every score award (:829/:840).
 //
-// Side effect worth knowing: the binary's kill handler also bumps killCount on
+// The binary's phantom partner increment, stated ACCURATELY.  Its kill handler
-// the VICTIM's player (the @0x4c0397/@0x4c03a3 wrong-column slip we reproduce
+// also bumps killCount on the VICTIM's player (the @0x4c0397/@0x4c03a3
-// faithfully).  That partner increment always lands on a REPLICANT copy of the
+// wrong-column slip, which we still reproduce byte-faithfully).  That increment
-// victim, so the owner's authoritative value now overwrites it on the next
+// always lands on a REPLICANT copy of the victim, so the owner's record corrects
-// record -- the phantom kill stops being visible without touching the
+// it -- but NOT, as an earlier draft of this comment claimed, by arriving after
-// reconstructed handler.
+// it.  The engine's event priorities run the other way: an inbound update record
+// is posted at UpdateEventPriority == MaxEventPriority (INTEREST.cpp) and drained
+// by ExecuteBackgroundTask, while the rerouted score message is posted at
+// EntityManagerEventPriority == DefaultEventPriority (NTTMGR.cpp) and popped
+// later, so the correction usually lands FIRST and the phantom lands after it.
+// The artifact is therefore BOUNDED, not masked: on the killer's pod only, that
+// victim's KILLS reads +1 until the next record -- at most one heartbeat (2s)
+// rather than "until the victim next scores or dies".  Deciding whether to drop
+// the increment outright is the open fidelity question; see docs/RECONCILE.md.
 //
 void
 	BTPlayer::WriteUpdateRecord(Simulation__UpdateRecord *message, int update_model)
@@ -1885,12 +1893,16 @@ void
 	UpdateRecord *rec = (UpdateRecord *)message;
 
 	//
-	// MIXED-VERSION GUARD: a peer on a build without this extension writes a
+	// MIXED-VERSION GUARD -- and the reason is NOT stream desync.  Both sides
-	// plain Player-sized record, and reading the two fields off the end would
+	// advance by the WRITER's stamped recordLength (ENTITY.cpp:390 /
-	// pick up whatever record was packed after it in the update stream and
+	// SIMULATE.cpp:323) and no reader asserts a length, so an old 60-byte record
-	// display it as a kill count.  Trust the length the writer stamped.  (The
+	// read by this 68-byte reader keeps the stream perfectly in sync.  The real
-	// stream itself stays in sync either way -- both sides advance by the
+	// hazard is a READ PAST THE ALLOCATION: an inbound update message is copied
-	// WRITER's recordLength, ENTITY.cpp:390 / SIMULATE.cpp:323.)
+	// into a per-event heap block sized from messageLength (EntityManager's
+	// ReceiveNetworkPacket -> application->Post -> new long[(len+3)>>2]), so on a
+	// TRAILING legacy record these two fields would be read off the end of that
+	// block -- garbage displayed as a kill count, or worse.  Trust the length the
+	// writer stamped.
 	//
 	if (rec->recordLength < sizeof(UpdateRecord))
 	{
@@ -2043,8 +2055,9 @@ void BTPostKillScore(Entity *victim, Scalar damage)			// Step 7: KILL (+ MP deat
 	// tallies its own copies self-consistently".  That was FALSE and it is what hid
 	// this bug: the reroute means exactly ONE node increments, no other node can
 	// observe the killer at all, and nothing carried the value back out -- so every
-	// remote pilot's KILLS read 0 on every other pod, all mission (measured: 125 of
+	// remote pilot's KILLS read 0 on every other pod, all mission (corpus invariant:
-	// 125 `SCORE type=2` rows credit the logging node's own player). The counters are
+	// 0 of 18818 DMG rows are inst=R, so no other node can even attribute the kill;
+	// every DEATH inst=R row reads killer=0:0). The counters are
 	// now replicated owner->replicant instead; see Read/WriteUpdateRecord above.
 	BTPlayer *killer_player = 0;
 	if (application->GetHostManager() != 0)

game/reconstructed/btplayer.hpp

@@ -311,16 +311,22 @@ class DropZone__ReplyMessage;
 	// `Player__UpdateRecord` is currentScore + dropZoneLocation only
 	// (PLAYER.h:73-80) -- so every OTHER node's copy stayed 0 for the whole
 	// mission, which is exactly why the comm panel never showed a pilot's
-	// kills.  [T0 the reroute + the record contents; T1 the field evidence.]
+	// kills.  [T0 the reroute + the record contents; T2 the field evidence.]
 	//
-	// The field measurement, stated so it is REPRODUCIBLE (a bare figure nobody
+	// The field evidence, stated as INVARIANT RATIOS rather than counts.  (An
-	// can re-derive is worse than none): over `matchlogs/**/*matchlog*.txt` plus
+	// earlier draft cited "125 of 125 over 255 node-logs"; that was true when
-	// `content/matchlog*.txt`, taking each log's own host as the modal host in its
+	// measured but is not reproducible, because every rig run appends to the same
-	// VEHICLE/RESPAWN/PLAYER_DEAD rows -- 255 logs had an identifiable local pilot
+	// corpus -- the verification runs for this very fix grew it from ~255 to 425
-	// and carried 125 `SCORE ... type=2` rows, of which 125 credited the LOGGING
+	// files.  Cite ratios that hold at any corpus size.)  Over
-	// node's own player and 0 credited a remote pilot.  An independent pass that
+	// `matchlogs/**/*matchlog*.txt` + `content/matchlog*.txt` +
-	// de-duplicated the corpus by file content instead counted 113/113 the same
+	// `content/matchlogs/**`:
-	// way; the ratio, not the absolute count, is the claim.
+	//   * 0 of 18818 `DMG` rows are `inst=R` -- damage is applied MASTER-SIDE ONLY,
+	//     so no bystander can attribute a kill even in principle;
+	//   * 439 of 439 `DEATH inst=R` rows read `killer=0:0 killdmg=0.000`, against
+	//     0 of 225 `inst=M` rows -- a replicant victim's `lastInflictingID` is
+	//     never populated locally (sole writer: mech.cpp:746).
+	// Both ratios are re-derivable with a one-line scan and neither drifts as the
+	// corpus grows.
 	//
 	// The owner's value is already the single authoritative copy -- incremented
 	// exactly once per kill, through the engine's own reroute -- so completing
@@ -333,7 +339,7 @@ class DropZone__ReplyMessage;
 			public Player::UpdateRecord
 		{
 			int		killTally;		// @0x27c killCount -- the KILLS column
-			int		deathTally;		// deathCount     -- the DEATHS column
+			int		deathTally;		// @0x280 deathTally -- the DEATHS column
 		};
 		typedef BTPlayer__UpdateRecord UpdateRecord;
 

game/reconstructed/mech4.cpp

@@ -2027,9 +2027,10 @@ void
 		// TRANSITION, and a replicant reaches destroyed state via the replicated
 		// simulationState 9, which routes it to the mode-9 early return above -- so
 		// the ReplicantInstance gate here was unreachable by construction.  Could not
-		// have worked: a replicant victim carries no attribution at all (0 of 8800
+		// have worked: a replicant victim carries no attribution at all (0 of 18818
-		// corpus DMG rows target a replicant, which is why every DEATH inst=R row
+		// corpus DMG rows are inst=R -- damage is applied master-side only -- which is
-		// reads killer=0:0), so no bystander can know who to credit.  DEATHS is now
+		// why 439 of 439 DEATH inst=R rows read killer=0:0, against 0 of 225 inst=M
+		// rows), so no bystander can know who to credit.  DEATHS is now
 		// replicated from the owning pod instead (BTPlayer::Read/WriteUpdateRecord),
 		// which is also why this must not come back: it would be a SECOND writer of a
 		// replicated counter.  Do not re-arm it.