From 32cfb9a01324104c4a822c45ff5515ac88ad62df Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Wed, 22 Jul 2026 20:10:46 +0200 Subject: [PATCH] fix(topdata): let pinned TLK ids evict stale state owners The custom palette taxonomy (#46) pins display strings to fixed TLK ids in tlk/custom.tlk.yml. registerInlineAtID demanded those ids be free, but .tlk_state.json is a gitignored, per-machine cache: on any machine built before #46, a ref could have dynamically grabbed a now-pinned id (e.g. feat:yuanti/alternate_form.feat holding 2689), failing the build with "TLK id N is already reserved by ...". Make the pin authoritative over the cache: a stale cached owner on a pinned id is evicted and reallocated a fresh id when next made active. Only two pins fighting over one id in custom.tlk.yml is now an error. This self-heals on the next build without deleting the state file. Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/topdata/tlk_native.go | 18 ++++++++++-- internal/topdata/topdata_test.go | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/internal/topdata/tlk_native.go b/internal/topdata/tlk_native.go index 8bf1a0e..9e53384 100644 --- a/internal/topdata/tlk_native.go +++ b/internal/topdata/tlk_native.go @@ -109,6 +109,7 @@ type tlkCompiler struct { active map[string]tlkEntryData activeKeys map[string]struct{} reservedByID map[int]string + pinnedByID map[int]string nextID int } @@ -158,6 +159,7 @@ func newTLKCompiler(sourceDir string, legacy *legacyTLKData) (*tlkCompiler, erro active: map[string]tlkEntryData{}, activeKeys: map[string]struct{}{}, reservedByID: reserved, + pinnedByID: map[int]string{}, nextID: nextID, } if legacy != nil { @@ -380,12 +382,24 @@ func (c *tlkCompiler) registerInlineAtID(key string, id int, entry tlkEntryData) if id < 0 { return fmt.Errorf("TLK key %q has negative id %d", key, id) } + // The pin is authoritative over the per-machine .tlk_state.json cache: a + // stale mapping that dynamically grabbed this id on an older build must + // yield so the pinned key can take it. Only a genuine clash between two + // pins in custom.tlk.yml is an author error. + if owner, ok := c.pinnedByID[id]; ok && owner != key { + return fmt.Errorf("TLK id %d is pinned by both %q and %q", id, owner, key) + } if mapping, ok := c.state.Entries[key]; ok && mapping.ID != id { - return fmt.Errorf("TLK key %q changed id from %d to %d", key, mapping.ID, id) + // This key held a different cached id; release it so the pin wins. + if c.reservedByID[mapping.ID] == key { + delete(c.reservedByID, mapping.ID) + } } if owner, ok := c.reservedByID[id]; ok && owner != key { - return fmt.Errorf("TLK id %d is already reserved by %q", id, owner) + // Evict the stale owner; it gets a fresh id when next made active. + delete(c.state.Entries, owner) } + c.pinnedByID[id] = key c.state.Entries[key] = tlkStateMapping{ID: id} c.reservedByID[id] = key return c.markActive(key, entry) diff --git a/internal/topdata/topdata_test.go b/internal/topdata/topdata_test.go index 61971ed..d7abeb8 100644 --- a/internal/topdata/topdata_test.go +++ b/internal/topdata/topdata_test.go @@ -2449,6 +2449,55 @@ strings: } } +func TestBuildStandaloneTLKPinEvictsStaleStateOwner(t *testing.T) { + root := testProjectRoot(t) + mkdirAll(t, filepath.Join(root, "topdata", "data", "feat")) + mkdirAll(t, filepath.Join(root, "topdata", "tlk")) + writeFile(t, filepath.Join(root, "topdata", "base_dialog.json"), "{}\n") + writeFile(t, filepath.Join(root, "topdata", "data", "feat", "base.json"), `{ + "output": "feat.2da", + "columns": ["LABEL", "FEAT", "DESCRIPTION"], + "rows": [{ + "id": 0, + "key": "feat:test", + "LABEL": "TEST_LABEL", + "FEAT": {"tlk": {"key": "feat:test.name", "text": "Test Feat"}}, + "DESCRIPTION": "****" + }] + }`+"\n") + writeFile(t, filepath.Join(root, "topdata", "tlk", "custom.tlk.yml"), `schema: sow-topdata/tlk/v1 +base_strref: 16777216 +strings: + - key: sow.module.name + text: Shadows Over Westgate + id: 50 +`) + // Simulate a per-machine state that predates the pinned taxonomy: the feat + // ref dynamically grabbed id 50 on an older build, exactly where the pin + // now lives. The build must self-heal instead of failing. + writeFile(t, filepath.Join(root, "topdata", tlkStateFile), + `{"version":1,"language":"en","entries":{"feat:test.name":{"id":50}}}`+"\n") + + if _, err := BuildNative(testProject(root), nil); err != nil { + t.Fatalf("BuildNative failed on stale pin collision: %v", err) + } + + stateRaw, err := os.ReadFile(filepath.Join(root, "topdata", tlkStateFile)) + if err != nil { + t.Fatalf("read tlk state: %v", err) + } + var state tlkStateDocument + if err := json.Unmarshal(stateRaw, &state); err != nil { + t.Fatalf("parse tlk state: %v", err) + } + if state.Entries["sow.module.name"].ID != 50 { + t.Fatalf("pin must own id 50, got %#v", state.Entries["sow.module.name"]) + } + if got := state.Entries["feat:test.name"].ID; got == 50 { + t.Fatalf("stale feat ref should have been reallocated off id 50, got %d", got) + } +} + func TestBuildPreservesTLKStateAcrossTextChanges(t *testing.T) { root := testProjectRoot(t) mkdirAll(t, filepath.Join(root, "topdata", "data", "skills"))