fix(atlas): keep the UniqueId, and make a landmark value name one landmark
All checks were successful
PR Checks / client-build (pull_request) Successful in 23s
PR Checks / server-tests (pull_request) Successful in 28s
PR Checks / frozen-manifest (pull_request) Successful in -21s

Two defects the Phase 16b re-verify found in the released v1.2.1 bundle, both
of which make a shipped feature unusable and neither of which any test saw.

## The aggregator discarded the UniqueId

`shard_spawn_points.unique_id` was NULL on all 6,455 rows of a stock 57.4 tree.
`listSpawners` filters `unique_id IS NOT NULL`, so `uo.options.spawners` was an
empty dropdown -- and it is the ONLY option source for the Phase 12b
object-property leases, so no `Spawner.MaxCount` / `MinDelay` / `MaxDelay` lease
could be authored at all, with nothing on the form to say why.

Every part of the path was already right except one line. The spawn files carry
`<UniqueId>` (~6,374 of them), `parsePoints` returns it, the column exists and
the insert passes `p.uniqueId || null`. `buildAtlas` rebuilds each point from an
explicit field list and `uniqueId` was not on it -- the word appears nowhere in
that file. `PARSER_VERSION = 4`'s own note says "a spawn point keeps its
UniqueId, which is what a property lease targets", so the intent shipped as a
comment while the code dropped the field one function later.

`PARSER_VERSION` goes to 5 because the bump is the only thing that re-reads an
already-imported tree: `sameSources` compares the tree's hashes, which have not
changed -- only what is kept from them. Confirmed on the rig, where the boot
after the fix logged `spawn atlas refreshed` on an unchanged tree and the manual
import then correctly answered `unchanged`.

## A landmark option value named 23 places at once

A stock tree has 558 landmarks under 320 distinct `facet/name` pairs.
`Trammel/Entrance` is 23 different dungeons -- Blighted Grove, Covetous, Deceit,
Despise, Destard and so on -- and `landmarkPoint` resolved with `.find()`, so 22
of the 23 were unreachable. An author who picked "Entrance - Destard" got
Blighted Grove, and the run succeeded with no warning. The group was already the
disambiguator: it was shown in the dropdown and left out of the value.

The value is now `facet/group/name`, which is distinct across all 558.
`landmarkPoint` tries that form first and keeps the two-part read as a fallback,
because every event published before this fix stores `facet/name` and a
published version is immutable -- refusing to parse those would break runs
rather than correct them. The fallback keeps the old first-match behaviour
deliberately: it is imprecise in exactly the way it always was, and silently
relocating a live event's spawn point is worse than repeating a known
imprecision. A three-part value whose group is gone REFUSES rather than falling
back to the name, because it asked for one particular place.

## Verification

On the released-artefact rig (installer -> bundle 2026.09.10 -> stock 57.4 tree
-> protocol-7 sidecar -> core at main with this module):

  spawn points     6455 rows, 6364 with a unique_id   (was 0)
  uo.options.spawners   100 options, and `?q=orc` searches them   (was 0)
  uo.options.landmarks  558 options, 558 distinct values          (was 320)
  suite            625 pass, 0 fail

Each new test was confirmed to FAIL without its fix. The atlas one asserts the
field on the AGGREGATOR's output rather than the parser's, which is the whole
point of it -- and the test fixture had no `<UniqueId>` at all until now, which
is exactly why a green suite said nothing. The landmark one asserts an
INEQUALITY between two resolved points rather than a literal value string, so it
survives another change of format as long as two options still address two
places.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4
This commit is contained in:
2026-09-09 20:59:49 -05:00
parent bbaf08f67c
commit d6346996d3
4 changed files with 163 additions and 7 deletions

View File

@@ -58,7 +58,8 @@ function writeTree(root, { facets = ['Sosaria'], includeChampions = true } = {})
fs.writeFileSync(
path.join(root, 'Spawns', `${facet}.xml`),
`<Spawns>
<Points><Name>${facet}A</Name><Map>${facet}</Map><X>1100</X><Y>1100</Y>
<Points><Name>${facet}A</Name><UniqueId>uid-${facet}-A</UniqueId>
<Map>${facet}</Map><X>1100</X><Y>1100</Y>
<MaxCount>3</MaxCount><IsRunning>True</IsRunning>
<Objects2>Lizardman:MX=3:SB=0:OBJ=Orc:MX=1:SB=0</Objects2></Points>
<Points><Name>${facet}B</Name><Map>${facet}</Map><X>9000</X><Y>9000</Y>
@@ -112,6 +113,29 @@ function tempTree(options) {
// ── buildAtlas against a custom-facet tree ─────────────────────────────────
test('buildAtlas: a point keeps the UniqueId a property lease targets', () => {
// The field is asserted on the AGGREGATOR's output, not the parser's, which is
// the whole point of this test. `parsePoints` produced it from Phase 12b
// onwards and `PARSER_VERSION`'s own note said a point kept it, while the
// mapping in `buildAtlas` rebuilt each point from an explicit field list that
// omitted it — so `shard_spawn_points.unique_id` was NULL on every row, and
// `listSpawners`, whose WHERE is `unique_id IS NOT NULL`, answered empty. That
// left `uo.options.spawners` an empty dropdown and every Phase 12b
// object-property lease unauthorable. Found by the Phase 16b released-artefact
// walk, against a real tree whose files carry ~6,400 of these.
//
// The fixture above had no <UniqueId> at all until this test, which is exactly
// why a green suite said nothing about it.
const root = tempTree({ facets: ['Sosaria'] })
const atlas = buildAtlas(root)
const named = atlas.points.find((p) => p.name === 'SosariaA')
assert.equal(named.uniqueId, 'uid-Sosaria-A')
// And a point whose file names none is absent rather than empty-string, so the
// DB layer's `unique_id IS NOT NULL AND <> ''` reads it the same way either way.
const unnamed = atlas.points.find((p) => p.name === 'SosariaB')
assert.ok(!unnamed.uniqueId)
})
test('buildAtlas: works entirely on facets that do not exist in stock UO', () => {
const root = tempTree({ facets: ['Sosaria', 'Underdark'] })
const atlas = buildAtlas(root)

View File

@@ -563,6 +563,78 @@ test('an atlas larger than the dropdown bound is truncated and said so', async (
assert.ok(warned, 'a truncated source must leave a log line naming itself')
})
// ── A landmark option value names ONE landmark (Phase 16b) ────────────────
test('two landmarks sharing a name are two different options, and both resolve', async () => {
// A stock 57.4 tree has 558 landmarks under 320 distinct `facet/name` pairs:
// `Trammel/Entrance` is 23 different dungeons. The source emitted `facet/name`
// and `landmarkPoint` resolved with `.find()`, so 22 of the 23 were unreachable
// — an author who picked "Entrance — Destard" got Blighted Grove, with a
// successful run and no warning. The group was already the disambiguator and it
// was shown to the eye while being left out of the value.
//
// Asserted as an INEQUALITY between two resolved points rather than against a
// literal value string, so it survives someone changing the value's format
// again as long as the two options still address two places.
shardAtlas.listLandmarks = async () => [
{ facet: 'Felucca', name: 'Entrance', group: 'Blighted Grove', x: 586, y: 1643, z: 0 },
{ facet: 'Felucca', name: 'Entrance', group: 'Destard', x: 1176, y: 2637, z: 0 },
]
const source = actions.OPTION_SOURCES.find((s) => s.id === 'uo.options.landmarks')
const options = await source.resolve({})
assert.equal(options.length, 2)
assert.equal(new Set(options.map((o) => o.value)).size, 2, 'both options must be addressable')
const points = []
for (const option of options) {
const result = await byId('uo.creature.spawn').perform({
runId: 41,
idempotencyKey: `L${option.value}`.padEnd(40, 'x'),
params: { place: option.value, creature: 'Orc', count: 1 },
verify: true,
})
assert.equal(result.ok, true, `${option.value} must resolve`)
points.push(option.value)
}
assert.notEqual(points[0], points[1])
})
test('a place published before the group was carried still resolves', async () => {
// Every event published before the fix stores `facet/name`, and a published
// version is immutable — so a parse that stopped understanding the two-part
// form would break those runs rather than correct them. It keeps the old
// first-match read, which is imprecise in exactly the way it always was.
shardAtlas.listLandmarks = async () => [
{ facet: 'Felucca', name: 'Entrance', group: 'Blighted Grove', x: 586, y: 1643, z: 0 },
{ facet: 'Felucca', name: 'Entrance', group: 'Destard', x: 1176, y: 2637, z: 0 },
// A name carrying a slash reads as three parts too; the two-part read is what
// resolves it, which is why the three-part attempt must not answer for it.
{ facet: 'Felucca', name: 'Odd/Name', group: null, x: 10, y: 20, z: 0 },
]
for (const place of ['Felucca/Entrance', 'Felucca/Odd/Name']) {
const result = await byId('uo.creature.spawn').perform({
runId: 42,
idempotencyKey: `P${place}`.padEnd(40, 'x'),
params: { place, creature: 'Orc', count: 1 },
verify: true,
})
assert.equal(result.ok, true, `${place} must still resolve`)
}
// And a three-part value whose group is gone REFUSES rather than silently
// landing somewhere else. That is the honest answer: it asked for one place.
const gone = await byId('uo.creature.spawn').perform({
runId: 42,
idempotencyKey: 'G'.repeat(40),
params: { place: 'Felucca/Renamed/Entrance', creature: 'Orc', count: 1 },
verify: true,
})
assert.equal(gone.ok, false)
assert.match(gone.error, /no landmark called/)
})
// ── The world verbs (Phase 12a) ───────────────────────────────
test('a spawn files one ledger row per serial, not one per call', async () => {