From 4d3896707801ec3fc70543b9c91cd5bc0a83dfd8 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Wed, 22 Jul 2026 18:17:04 +0000 Subject: [PATCH] fix(topdata): let pinned TLK ids evict stale state owners (#48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem The custom palette taxonomy (#46) pins display strings to fixed TLK ids in `tlk/custom.tlk.yml`, and `registerInlineAtID` demanded each pinned id be free. But `.tlk_state.json` is **gitignored and per-machine** — each dev grows their own copy, and before #46 every ref got its id dynamically (first-come-first-served). On any machine whose state predates #46, a ref could have already parked on a now-pinned id. That fails the build: ``` topdata validation failed with 1 error(s): error: native topdata buildability check failed: TLK id 2689 is already reserved by "feat:yuanti/alternate_form.feat" ``` It only passes on machines whose state was regenerated after #46 (pinned window already clean). The `allocateID` reserved-skip that #46 added protects a *fresh* build, but does nothing about a *stale cache*. ## Fix Make the pin authoritative over the per-machine cache. A stale cached owner sitting on a pinned id is evicted and reallocated a fresh id when next made active; only two pins fighting over the same id in `custom.tlk.yml` is now an error. This self-heals on the next build — no manual `.tlk_state.json` deletion needed. Caveat: the evicted ref's strref shifts on affected machines. That's unavoidable (something must move off the pinned id), and dynamic strrefs were never stable across machines anyway. ## Tests Added `TestBuildStandaloneTLKPinEvictsStaleStateOwner` (seeds a pre-#46 state with a feat parked on the pinned id, asserts the build succeeds, the pin owns it, and the feat is reallocated). Full `internal/topdata` suite + `go vet` pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)Reviewed-on: https://git.westgate.pw/ShadowsOverWestgate/sow-tools/pulls/48 Reviewed-by: xtul Co-authored-by: vickydotbat --- 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"))