Address-keyed join of the two symbol records, with the divergence surface banked

[tangosdev]

Why

config/**/symbols.txt (what the build links against) and symbols/verified.tsv (the import mirror) disagree on hundreds of addresses with no shared spelling, e.g. 0x0203562c is _ZNK10dBgCh_Actr8IsOnWallEv in config and _ZNK12WithMeshClsn8IsOnWallEv in the mirror, and 0x0200651c diverges in class and dtor variant at once (_ZN11dCapEnemy_cD1Ev vs _ZN8CapEnemyD2Ev). That is structural: corrections land in config, mirror rows only get restamped when a correction happens to touch them.

On 2026-08-30 a name-keyed "is this body sourced?" sweep joined the records by name and manufactured phantom crack targets out of matched, enrolled functions (func_ov062_0211bd10, _ZN13TreasureChest6State0Ev). The earlier session counted 553 divergent addresses; that number does not reproduce under any single definition against today's tree, so this PR makes the enumeration itself canonical instead of the number.

What

• tools/symrecords.py: loads both records across the full module universe, joins on address, classifies every disagreement (class-rename wave / dtor-ctor variant / arg rename / real signature fix / canonical mangling / needs-a-human), answers --where 0x<addr>, and can plan (--restamp) or apply (--restamp --apply) a restamp of the mechanically explained mirror rows. Nothing applies by default.
• tools/test_symrecords.py: pins the refusals next to the acceptances. A lone diverging dtor pair is NOT class-rename evidence (dtor bodies are byte-identical across classes, the Correct the Actor and ActorBase D1/D2 symbol names #774 lesson); nested ctor outer classes are compared even though the demangler collapses them; operators and malformed names are refused, not guessed. Also pins the allowlist of tools that open verified.tsv directly, so new consumers go through the address-keyed API.
• symbols/record_divergences.tsv: the banked surface. 833 addresses: 361 real-vs-real divergences (334 mechanically explained, 27 for a human), 5 kind mismatches, 382 config placeholders where the mirror has a name, 85 with no config slot.
• notes/symbol-name-provenance.md: the written rule (join on address, module-qualified for overlays; look a name up only in the record the artifact links against), and corrects the note's outdated claim that verified.tsv is left stale on purpose.

Review pointers

• The 27 needs-a-human rows are in the banked TSV under verdicts identity and plain; they include the _ZThn80 Animation thunks (Fix six misnamed _ZThn80 thunks: rename symbols to ROM truth + match all six #178 territory), BootScene/Scene and FaderColor/Color dtor identities, and the Spawn-vs-ctor naming debates.
• A follow-up PR stacked on this one carries the actual restamp of the 334 mechanical rows, so the data change reviews separately from the tooling.

No src/ files move and contributions.json is untouched, so nothing here can shift credit.

[tangos-validator]

✅ PR validation — Passed

noverify: no source/build-data changes in this PR

_{Each changed src/*.c|*.cpp is compiled and its relocated bytes compared to the binary data on a private build box. Passing requires every changed file to reproduce the ROM byte-for-byte with correct relocation targets — this catches WRONG-DEST relocations and non-reproducing near-misses that ledger-scoped linkcheck skips.}

[andrewboudreau]

Gatekeeper review — MERGE AFTER ONE FIX

The tooling is careful and its central claim survives adversarial probing. Nothing here is load-bearing, nothing writes without --restamp --apply, and the banked TSV reproduces bit-for-bit against today's main — not against the base you cut from:

$ python tools/symrecords.py --report symbols/regen.tsv    -> wrote 833 rows
$ diff <(tr -d '\r' < symbols/record_divergences.tsv) <(tr -d '\r' < symbols/regen.tsv) | wc -l
0

That is the thing a reviewer otherwise has to take on trust, and it holds. Arithmetic checks out at every level (833 = 361 + 5 + 382 + 85; mechanical 248+52+8+8+7+4+2+2+2+1 = 334; human 15 plain + 12 identity = 27; 361 = 334 + 27).

The refusal machinery is the part I probed hardest, because a mangled-name parser that truncates instead of refusing would poison everything downstream. It refuses:

_ZN11CannonHatch8IsOnWallEv  -> (['CannonHatch','IsOnWall'], False)    correct prefix
_ZN12CannonHatch8IsOnWallEv  -> None
_ZN10CannonHatch8IsOnWallEv  -> None
_ZN99CannonHatch8IsOnWallEv  -> None

Operators, templates in the path, local-linkage and garbage all REFUSE and downgrade to plain, unproposed. Thunks compare on the (is_thunk, offset, vcall_offset) tuple. Anonymous namespaces parse correctly as _GLOBAL__N_1::Foo — a correct parse, not a guess. The dtor/ctor exclusion from rename evidence is enforced in code at symrecords.py:253, not just asserted in prose, and symrecords.py:279-281 returns identity when cmap is empty. Both directions verified.

Write behaviour is genuinely opt-in:

python tools/symrecords.py            rc=0  git status --porcelain -> (empty)
python tools/symrecords.py --apply    rc=2  git status --porcelain -> (empty)
python tools/symrecords.py --restamp  rc=0  git status --porcelain -> (empty)

Merge-result gating: exports asserted complete (12753/12753 and 12750/12750), and no gate differs between origin/main and the merge tree — check_dead_references, check_python_names, check_src_tu, check_duplicate_sources, tiers_ratchet --check, langmode_audit, source_coverage all identical.

---

Blocking — the 21 new tests never run in CI

.github/workflows/tool-tests.yml is not updated. grep -c test_symrecords on the merge tree returns 0, and CI's own log proves the omission:

$ gh run view 33329528573 --log | grep "Ran .* tests"
tools  The tools' own tests  Ran 312 tests in 8.622s

312 is exactly the count on unmodified main. The path filter fires (paths: tools/**), so the job runs — it just runs the same 312 assertions it always did.

This matters more than a normal wiring omission because the workflow's own header says the enumerated list exists "so adding a module is a reviewed decision", and because the module is genuinely populated — 9 TestCase subclasses, 0 bare pytest-style def test_*, 21 methods, Ran 21 tests in 3.387s / OK, zero skips, no self-skipping toolchain guards. This is a real green being left on the floor, which is the opposite of this tree's usual problem.

Fix: add tools.test_symrecords \ to the run: list plus a one-line entry in the trailing table. The workflow file is already in its own path filter.

While you are in there: the header says "runs 312 of those assertions" and that number is about to be wrong. It explicitly calls itself "the number to compare against if this job's total ever moves" — so move it.

---

N1 — I checked the cross-overlay concern and it does not hold. Do not widen the guard.

The review that fed me this recommended widening ambiguous-module from "both sides carry a real name" to "more than one module has an entry", on the grounds that three live rows get a restamp proposal at a cross-overlay address. That would be wrong, and I want the reasoning on the record because the change looks safe.

I read the actual records at all three:

addr	mirror OLD (this PR's base)	mirror NEW (#2039)	config ov002	other module
0x020ada40	_ZN5Enemy20KillByInvincibleChar…	_ZN12dEnemyBase_c20KillByInvincibleChar…	_ZN12dEnemyBase_c… size 0x100	ov004 func_ov004_020ada40 size 0xbc
0x020aed98	_ZN5EnemyC2Ev	_ZN12dEnemyBase_cC2Ev	_ZN12dEnemyBase_cC2Ev size 0x24	ov007 func_ov007_020aed98 size 0x1e0
0x020ee55c	_ZN8Platform4KillEv	_ZN10dBgActor_c4KillEv	_ZN10dBgActor_c4KillEv size 0x74	ov007 data_ov007_020ee55c kind:data

In every one, the mirror's own old name is a real, class-bearing name from the previous naming wave (Enemy, Platform), and the colliding overlay's slot is a placeholder. So the module is not ambiguous for that row — the row identifies its own class, and Enemy -> dEnemyBase_c / Platform -> dBgActor_c are corroborated by many other rows in the same wave. The restamps are correct.

Widening the guard as proposed would refuse three provably right restamps, and it would do so for a reason the data contradicts. The existing design already has the correct floor: a proposal requires the old name to map through cmap, and symrecords.py:279-281 returns identity when it does not. A cross-overlay address whose mirror row carried a placeholder old name would already refuse for lack of a cmap entry.

The mechanism the review found is nonetheless real and worth one comment plus a regression test pinning these three rows and the reasoning, so the next person doesn't "fix" it. class_rename_map() at symrecords.py:255 iterating without a module filter is the looser end — one live pair (Platform/dBgActor_c) draws part of its evidence from a multi-module address. It has ample other evidence so the conclusion survives, but a len(real) > 1 skip there costs nothing.

N2 — move signature-fix out of MECHANICAL, for future runs only

The module's own comment calls signature-fix "a REAL arity/type correction and worth a closer look in review" — and then puts it in the auto-restamp set. All 11 such rows are in #2039's plan, including:

0x020ae2b8  _ZN12dEnemyBase_c15IsGoingOffCliffER10dBgCh_Actrisbbi
         || _ZN5Enemy15IsGoingOffCliffER12WithMeshClsn5Fix12IiEsbbS3_

Fix12<int> vs int is ABI-identical, so the cartridge cannot arbitrate it. That is a data judgement, not a mechanical rewrite.

To be clear about scope: this does not change #2039. I audited all 11 of those rows on that PR — every one already lives in config/**/symbols.txt backed by a byte-matching src/ definition with no NONMATCHING banner, and three of the eleven rest on judgement rather than the function's own bytes (now stated in #2039's body). Those are audited and can land. The problem is the next run, where 11 more arity changes would auto-apply with nobody looking. Move the verdict into the human bucket (27 -> 38) so it has to be read.

N3 — one corroborating row unlocks a class wave

Two of the 23 class pairs rest on a single non-dtor method row (ActorBase::SceneNode -> fBase_c::SceneNode at 0x0203b4ac; RaycastLine::Line -> dM3dGLin at 0x020fea84). Defensible — the corroborating row is itself an address identity, so member-name plus arg-list agreement at the same address is strong evidence — but weaker than the body's phrasing suggests. One sentence.

---

Two small things worth carrying

The notes correction is true: main's "left stale on purpose" is false, and the new wording is right. But it drops the "and keep the import-time spelling when it does not" caveat that symrecords.py:14-17 states properly — carry it over.

The allowlist test globs tools/*.py non-recursively, so tools/archive/*.py and any reader outside tools/ is invisible. No current file exploits that (grep -rln verified.tsv finds only the 5 allowlisted tools, 3 src/ comments, port/docs/opie-assessment.md, and the note), but narrow the docstring's "nothing new reads verified.tsv at all" to "nothing new in tools/".

Could not verify: check_references.py needs build/rombuild-eligibility.json from eligible.py, which is forbidden here — shared build/ with live agents. It fails identically on both trees, so there is no differential. No ROM build was attempted, so no byte-level claim above is my own measurement.

---

Wire the tests and I will merge. #2039 goes in right behind it — retargeted to main first, as a merge commit not a squash (a squash produces CONFLICT (add/add) on symbols/record_divergences.tsv), and without --delete-branch.

[andrewboudreau]

Gatekeeper note: three PRs contend on the tool-tests.yml header count

origin/main currently reads runs 312 of (L13) and 306 of the 312 (L69). Three
open PRs rewrite both lines:

PR	own module	header total
#2035 premerge_check	new, 87	312 → 399
#2038 test_symrecords	new, 24	312 → 336
#2040 test_nearmiss_db	12 → 21	312 → 321

All land ⇒ the true total is 432, and 306 of the 312 moves accordingly.

Two things worth stating plainly:

1. This number is prose, not an assertion. The workflow comment calls it "the number
   to compare against if this job's total ever moves" — nothing checks it. A wrong value
   here is a silent documentation lie that survives indefinitely, which is the exact
   failure class this repo keeps getting bitten by.
2. It will conflict, and that is the good outcome. All three PRs touch the same two
   lines, so whichever lands second and third goes CONFLICTING rather than
   auto-merging a stale integer.

When you resolve it: do not take --ours/--theirs on this file — that silently
reverts the other side's unrelated paths: and enumeration edits. Hand-edit the two
numbers, and measure the new total by running the enumerated list exactly as the
workflow does rather than doing the arithmetic. (#2040 already did it that way; that is
the standard.)

I am serializing the merges and will hold each one to a measured number.

[andrewboudreau]

Audited the conflict resolution independently. MERGE.

A tool-tests.yml conflict is a change class where a bad resolution is invisible: the
file's own prose is the only thing that says what it covers, and a resolution that
silently drops a module from the run list still goes green. So I measured the resolved
file rather than reading it.

The resolution is correct

check	result
merge tree vs origin/main	CLEAN → 6ce981936314
modules enumerated after resolution	25
added vs main's 24	test_symrecords — exactly one
modules lost from main's 24	none
real conflict markers anywhere in the merge tree	0

Both legend blocks survived, which is the thing a whole-file --ours/--theirs would
have destroyed: test_premerge_check (87) from main at L177, test_symrecords (24) from
this branch at L198, and both are in the run block.

The header counts are now measured, not added

Main's header was stale in a way that was easy to "fix" wrongly. It claimed 57
test_*.py modules where git ls-tree origin/main tools/ measures 59, and the two
sides carried different assertion totals (312/399 on main, 336/330 here) that are each a
pre-merge count. Adding them would have produced a confident, wrong number.

The resolution says 59 modules, six previously wired, "the other 53" — which is
internally consistent — and 423 assertions, a fresh figure that is not the sum of
either pair. That is the right way to land this.

The one thing I checked that nobody asked for

This branch is 53 commits behind main and still carries the stray >>>>>>> origin/main
at notes/data/c-cpp-classification.tsv:1414 that I removed from main in #2044. A
branch-only check would have missed whether merging resurrects it.

Gating the merge tree instead of the branch:

main       nonempty=7093  datarows=7092  markers=0
merge tree nonempty=7093  datarows=7092  markers=0
blob hash  2d9f2c62212681319064d4816ade9543e32e9102  (identical)

Byte-identical. The three-way merge keeps main's deletion because this branch never
modified that line. #2044 survives; nothing resurrects.

Independent verification that this cannot turn main red

test_symrecords landed unwired in #2039 — it has never run in CI, so its green was
unproven until now. I ran it against origin/main in a scratch export before trusting
this PR to enable it:

test_symrecords        21 passed
test_match_provenance  18 passed, 2 skipped   (already wired on main)

Green standalone and inside the merged 25-module set. Wiring it is safe. Note that
test_match_provenance was already enumerated on main, so this PR adds one module, not
two.

Stated gaps

No ROM build, eligible or romdata_check run — shared build/ makes concurrent runs
produce fake failures, and this PR touches no compiled source, so there is no byte claim
to audit. My scratch pytest run also showed 8 failures in test_dead_references,
test_srcpath and test_tubuild; those are artifacts of my export (no git repo, no
src/), not real, and I am recording them here so the number is not mistaken for a
finding later. Main's tool tests is green in CI at a8577b51b.