fix(atlas): keep the UniqueId, and make a landmark value name one landmark #35

wtclaude · 2026-09-10T02:05:33Z

wtclaude commented

2026-09-10 02:05:33 +00:00

Two defects the Phase 16b re-verify found in the released v1.2.1 bundle — the leg that installs the module through core's own https installer and then actually authors something. Each makes a shipped feature unusable, and no test saw either.

1. The aggregator discarded the `UniqueId`, so no property lease was authorable

shard_spawn_points:  6455 rows,  0 with a unique_id
uo.options.spawners: 0 options

listSpawners filters unique_id IS NOT NULL, so that source 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. Same shape as the atlas defect Phase 16a fixed.

Every part of the path was already right except one line. The spawn files carry <UniqueId> (~6,374 of them); parsePoints returns it — I ran it, 2,525 of 2,572 in trammel.xml; 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.

The sharpest part: PARSER_VERSION = 4's own note reads "a spawn point keeps its UniqueId, which is what a property lease targets (Phase 12b)". That version bump exists specifically to re-read trees for this field. The intent shipped as a comment while the code dropped the field one function later.

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

2. A landmark option value named 23 places at once

shard_landmarks:  558 rows,  320 distinct facet/name
Trammel/Entrance → Blighted Grove(586,1643), Covetous(2499,919), Deceit, Despise, Destard … ×23

The source emitted value: facet/name and landmarkPoint resolved with rows.find(...) — first match wins — so 22 of those 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: shown in the dropdown, left out of the value.

The value is now facet/group/name, 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 and the operator needs to know it moved.

(A name containing a slash reads as three parts too — Felucca/Odd/Name — which is why the three-part attempt falls through silently rather than erroring, and the two-part read is what resolves it. Tested.)

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                       (was 622)

Each new test was confirmed to FAIL without its fix. The atlas test asserts the field on the aggregator's output rather than the parser's, which is the whole point of it — and the fixture had no <UniqueId> at all until now, which is exactly why a green suite said nothing about this for two phases. The landmark test 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.

One thing stated precisely rather than overclaimed: that a three-part value resolves to its own coordinates is proven by the unit test and by landmarkPoint returning the matched row's x/y; I did not read the coordinates back off the shard, because teardown had already removed the creatures by the time the run finished.

Found by EVENTS_PLAN.md Phase 16b. Recorded in docs#231.

AI-assisted: written with Claude Code (Claude Opus 5).

🤖 Generated with Claude Code

https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4

Two defects the **Phase 16b re-verify found in the released `v1.2.1` bundle** — the leg that installs the module through core's own https installer and then actually authors something. Each makes a shipped feature unusable, and no test saw either. ## 1. The aggregator discarded the `UniqueId`, so no property lease was authorable ``` shard_spawn_points: 6455 rows, 0 with a unique_id uo.options.spawners: 0 options ``` `listSpawners` filters `unique_id IS NOT NULL`, so that source 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. Same shape as the atlas defect Phase 16a fixed. Every part of the path was already right except one line. The spawn files carry `<UniqueId>` (~6,374 of them); `parsePoints` returns it — I ran it, 2,525 of 2,572 in `trammel.xml`; 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. The sharpest part: `PARSER_VERSION = 4`'s own note reads *"a spawn point keeps its `UniqueId`, which is what a property lease targets (Phase 12b)"*. That version bump exists **specifically** to re-read trees for this field. The intent shipped as a comment while the code dropped the field one function later. `PARSER_VERSION` → **5**, because the bump is the only thing that re-reads an already-imported tree: `sameSources` compares the tree's hashes, and they have not changed — only what is kept from them. Proven on the rig, where the boot after the fix logged `spawn atlas refreshed from ServUO tree` on an unchanged tree and the manual import then correctly answered `unchanged`. ## 2. A landmark option value named 23 places at once ``` shard_landmarks: 558 rows, 320 distinct facet/name Trammel/Entrance → Blighted Grove(586,1643), Covetous(2499,919), Deceit, Despise, Destard … ×23 ``` The source emitted `value: facet/name` and `landmarkPoint` resolved with `rows.find(...)` — **first match wins** — so 22 of those 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: shown in the dropdown, left out of the value. The value is now **`facet/group/name`**, 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 and the operator needs to know it moved. (A name containing a slash reads as three parts too — `Felucca/Odd/Name` — which is why the three-part attempt falls through silently rather than erroring, and the two-part read is what resolves it. Tested.) ## 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 (was 622) ``` **Each new test was confirmed to FAIL without its fix.** The atlas test asserts the field on the **aggregator's** output rather than the parser's, which is the whole point of it — and the fixture had no `<UniqueId>` at all until now, which is exactly why a green suite said nothing about this for two phases. The landmark test 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. One thing stated precisely rather than overclaimed: that a three-part value resolves to *its own* coordinates is proven by the unit test and by `landmarkPoint` returning the matched row's `x`/`y`; I did not read the coordinates back off the shard, because teardown had already removed the creatures by the time the run finished. Found by `EVENTS_PLAN.md` Phase 16b. Recorded in `docs#231`. - [x] AI-assisted: written with Claude Code (Claude Opus 5). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_016wDDVXWMDz82WqE1i969r4

wtclaude added 1 commit 2026-09-10 02:05:41 +00:00

fix(atlas): keep the UniqueId, and make a landmark value name one landmark

PR Checks / client-build (pull_request) Successful in 23s

Details

PR Checks / server-tests (pull_request) Successful in 28s

Details

PR Checks / frozen-manifest (pull_request) Successful in -21s

Details

d6346996d3

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

whitlocktech merged commit 50f84b5ea3 into main

2026-09-10 02:14:52 +00:00

whitlocktech deleted branch fix/atlas-unique-id-and-landmark-values

2026-09-10 02:14:53 +00:00

whitlocktech referenced this issue from a commit

2026-09-10 02:14:54 +00:00

Merge pull request 'fix(atlas): keep the UniqueId, and make a landmark value name one landmark' (#35) from fix/atlas-unique-id-and-landmark-values into main

wtclaude referenced this pull request

2026-09-14 18:10:53 +00:00

fix(assets): a busy shard is not a broken one, and a column that reached no existing install (Phase 9a) #42

wtclaude referenced this issue from a commit

2026-09-14 22:34:40 +00:00