From bd9c1ba556dde4044ef8b94e7978fd5e3765b3a3 Mon Sep 17 00:00:00 2001 From: Erik Date: Tue, 14 Jul 2026 23:32:53 +0200 Subject: [PATCH] docs(plan): inventory spell filters implementation plan; spec: reuse existing slots UI Co-Authored-By: Claude Fable 5 --- .../2026-07-14-inventory-spell-filters.md | 433 ++++++++++++++++++ ...-inventory-spell-jewelry-filters-design.md | 26 +- 2 files changed, 445 insertions(+), 14 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-14-inventory-spell-filters.md diff --git a/docs/superpowers/plans/2026-07-14-inventory-spell-filters.md b/docs/superpowers/plans/2026-07-14-inventory-spell-filters.md new file mode 100644 index 00000000..9cd21aa2 --- /dev/null +++ b/docs/superpowers/plans/2026-07-14-inventory-spell-filters.md @@ -0,0 +1,433 @@ +# Inventory Search Spell Filters Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Implement the `legendary_cantrips`, `spell_contains`, and `has_spell` query params in the Go inventory service so the existing (currently dead) UI spell filters work, with AND semantics across checked cantrips. + +**Architecture:** Match filter names against the in-memory spell enum map (`Server.spells`, loaded at boot from `comprehensive_enum_database_v2.json`) to obtain spell IDs, then emit `EXISTS (... spell_id IN (...))` conditions against the existing `item_spells` table — one per checked cantrip, ANDed — as ordinary WHERE conditions in `runSearch` so they compose with all other filters, sorting, pagination, and the count query. Spec: `docs/superpowers/specs/2026-07-14-inventory-spell-jewelry-filters-design.md`. + +**Tech Stack:** Go 1.25, pgx/v5, plain `net/http`. No local Go toolchain exists — tests run in a throwaway `golang:1.25-bookworm` container on the deploy server (`overlord.snakedesert.se`); the Dockerfile's `RUN go test ./...` gates every image build. + +**Working directory for all git commands:** `C:/Users/erikn/source/repos/dereth-workspace/MosswartOverlord` + +**How to run tests (no local Go):** sync the source to a scratch dir on the server, run tests in Docker: + +```bash +cd "C:/Users/erikn/source/repos/dereth-workspace/MosswartOverlord" && \ +tar czf - go-services | ssh erik@overlord.snakedesert.se \ + "rm -rf /tmp/tdd-verify && mkdir -p /tmp/tdd-verify && tar xzf - -C /tmp/tdd-verify" && \ +ssh erik@overlord.snakedesert.se \ + "docker run --rm -v /tmp/tdd-verify/go-services/inventory-go:/src -w /src golang:1.25-bookworm \ + sh -c 'go mod tidy >/dev/null 2>&1; go test ./... -v -run 2>&1'" +``` + +Replace `` per step (use `.` for all tests). First run pulls modules (~30 s). + +--- + +### Task 1: Spell name → ID matchers + +The three lookup functions that translate UI names into spell IDs, mirroring the legacy Python matching rules (`inventory-service/main.py:3355-3427`). + +**Files:** +- Create: `go-services/inventory-go/spell_filter.go` +- Test: `go-services/inventory-go/spell_filter_test.go` (create) + +- [ ] **Step 1: Write the failing tests** + +Create `go-services/inventory-go/spell_filter_test.go`: + +```go +package main + +import ( + "reflect" + "testing" +) + +// Minimal stand-in for the enum-DB spell map (Server.spells). +var testSpells = map[int]map[string]any{ + 1: {"name": "Legendary Invulnerability"}, + 2: {"name": "Epic Invulnerability"}, + 3: {"name": "Legendary Summoning Prowess"}, + 4: {"name": "Strength Other VI"}, + 5: {"name": "Summoning"}, // shorter name CONTAINED IN a cantrip label + 6: {"name": ""}, // malformed entry must never match +} + +func TestSpellIDsContaining(t *testing.T) { + if got := spellIDsContaining(testSpells, "invulnerability"); !reflect.DeepEqual(got, []int{1, 2}) { + t.Errorf("substring match = %v, want [1 2]", got) + } + if got := spellIDsContaining(testSpells, "INVULN"); !reflect.DeepEqual(got, []int{1, 2}) { + t.Errorf("case-insensitive match = %v, want [1 2]", got) + } + if got := spellIDsContaining(testSpells, "frostbite"); got != nil { + t.Errorf("no-match = %v, want nil", got) + } +} + +func TestSpellIDsForCantrip(t *testing.T) { + // Forward direction: spell name contains the cantrip label. + if got := spellIDsForCantrip(testSpells, "Invulnerability"); !reflect.DeepEqual(got, []int{1, 2}) { + t.Errorf("forward contains = %v, want [1 2]", got) + } + // Both directions (legacy flexible rule): "Legendary Summoning Prowess" + // matches spell 3 (equal) AND spell 5 ("Summoning" is contained in the label). + if got := spellIDsForCantrip(testSpells, "Legendary Summoning Prowess"); !reflect.DeepEqual(got, []int{3, 5}) { + t.Errorf("both-direction = %v, want [3 5]", got) + } + if got := spellIDsForCantrip(testSpells, "Piercing Bane"); got != nil { + t.Errorf("no-match = %v, want nil", got) + } +} + +func TestExactSpellID(t *testing.T) { + if id, ok := exactSpellID(testSpells, "legendary invulnerability"); !ok || id != 1 { + t.Errorf("exact case-insensitive = (%d,%v), want (1,true)", id, ok) + } + if _, ok := exactSpellID(testSpells, "Invulnerability"); ok { + t.Error("partial name must not exact-match") + } +} +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run the SSH+Docker test command above with `-run 'TestSpellIDsContaining|TestSpellIDsForCantrip|TestExactSpellID'`. + +Expected: `FAIL ... [build failed]` with `undefined: spellIDsContaining` (and the other two names). + +- [ ] **Step 3: Write the implementation** + +Create `go-services/inventory-go/spell_filter.go`: + +```go +package main + +import ( + "sort" + "strings" +) + +// Spell-filter support for /search/items: implements the has_spell / +// spell_contains / legendary_cantrips params the Go port had ignored +// (design doc 2026-07-14; legacy inventory-service/main.py:3345-3452). +// Names are matched against the in-memory enum-DB spell map; the resulting +// IDs are filtered in SQL against the item_spells table. + +// spellIDsContaining returns the IDs of all spells whose name contains q, +// case-insensitively, in ascending order (deterministic SQL args). +func spellIDsContaining(spells map[int]map[string]any, q string) []int { + q = strings.ToLower(strings.TrimSpace(q)) + var ids []int + for id, sp := range spells { + name, _ := sp["name"].(string) + if name != "" && strings.Contains(strings.ToLower(name), q) { + ids = append(ids, id) + } + } + sort.Ints(ids) + return ids +} + +// spellIDsForCantrip matches one cantrip display name using the legacy +// flexible rule: spell name contains the label OR the label contains the +// spell name (both case-insensitive). +func spellIDsForCantrip(spells map[int]map[string]any, cantrip string) []int { + c := strings.ToLower(strings.TrimSpace(cantrip)) + var ids []int + for id, sp := range spells { + name, _ := sp["name"].(string) + n := strings.ToLower(name) + if n == "" { + continue + } + if strings.Contains(n, c) || strings.Contains(c, n) { + ids = append(ids, id) + } + } + sort.Ints(ids) + return ids +} + +// exactSpellID returns the lowest ID whose name equals q case-insensitively. +func exactSpellID(spells map[int]map[string]any, q string) (int, bool) { + q = strings.ToLower(strings.TrimSpace(q)) + best := -1 + for id, sp := range spells { + name, _ := sp["name"].(string) + if strings.ToLower(name) == q && (best == -1 || id < best) { + best = id + } + } + return best, best != -1 +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Same command as Step 2. Expected: all three tests `PASS`, plus the pre-existing suite still `ok`. + +- [ ] **Step 5: Commit** + +```bash +git add go-services/inventory-go/spell_filter.go go-services/inventory-go/spell_filter_test.go +git commit -m "feat(inventory-go): spell name -> ID matchers for search spell filters" +``` + +--- + +### Task 2: SQL condition builder + +Turns the three query params into WHERE conditions using the Task 1 matchers and the existing `argBuilder` (`search.go:139`) for positional args. + +**Files:** +- Modify: `go-services/inventory-go/spell_filter.go` +- Test: `go-services/inventory-go/spell_filter_test.go` + +- [ ] **Step 1: Write the failing tests** + +Append to `go-services/inventory-go/spell_filter_test.go`: + +```go +func TestSpellFilterConds_Cantrips(t *testing.T) { + ab := &argBuilder{} + q := map[string][]string{"legendary_cantrips": {"Legendary Invulnerability, Legendary Summoning Prowess"}} + conds := spellFilterConds(q, testSpells, ab) + want := []string{ + "EXISTS (SELECT 1 FROM item_spells sp WHERE sp.item_id = db_item_id AND sp.spell_id IN ($1))", + "EXISTS (SELECT 1 FROM item_spells sp WHERE sp.item_id = db_item_id AND sp.spell_id IN ($2, $3))", + } + if !reflect.DeepEqual(conds, want) { + t.Errorf("conds = %v, want %v", conds, want) + } + // "Legendary Invulnerability" exact-contains only id 1; the Summoning + // label matches 3 and 5. AND semantics = one EXISTS per cantrip. + if wantArgs := []any{1, 3, 5}; !reflect.DeepEqual(ab.args, wantArgs) { + t.Errorf("args = %v, want %v", ab.args, wantArgs) + } +} + +func TestSpellFilterConds_NoMatchIsImpossible(t *testing.T) { + ab := &argBuilder{} + q := map[string][]string{"legendary_cantrips": {"Legendary Frostbite"}} + if conds := spellFilterConds(q, testSpells, ab); !reflect.DeepEqual(conds, []string{"1 = 0"}) { + t.Errorf("unknown cantrip conds = %v, want [1 = 0]", conds) + } + ab = &argBuilder{} + q = map[string][]string{"spell_contains": {"frostbite"}} + if conds := spellFilterConds(q, testSpells, ab); !reflect.DeepEqual(conds, []string{"1 = 0"}) { + t.Errorf("unknown spell_contains conds = %v, want [1 = 0]", conds) + } +} + +func TestSpellFilterConds_ContainsAndHasSpell(t *testing.T) { + ab := &argBuilder{} + q := map[string][]string{"spell_contains": {"invulnerability"}} + want := []string{"EXISTS (SELECT 1 FROM item_spells sp WHERE sp.item_id = db_item_id AND sp.spell_id IN ($1, $2))"} + if conds := spellFilterConds(q, testSpells, ab); !reflect.DeepEqual(conds, want) { + t.Errorf("spell_contains conds = %v, want %v", conds, want) + } + + ab = &argBuilder{} + q = map[string][]string{"has_spell": {"Epic Invulnerability"}} + want = []string{"EXISTS (SELECT 1 FROM item_spells sp WHERE sp.item_id = db_item_id AND sp.spell_id IN ($1))"} + if conds := spellFilterConds(q, testSpells, ab); !reflect.DeepEqual(conds, want) { + t.Errorf("has_spell conds = %v, want %v", conds, want) + } + if _, ok := exactSpellID(testSpells, "No Such Spell"); ok { + t.Fatal("precondition") + } + ab = &argBuilder{} + q = map[string][]string{"has_spell": {"No Such Spell"}} + if conds := spellFilterConds(q, testSpells, ab); !reflect.DeepEqual(conds, []string{"1 = 0"}) { + t.Errorf("unknown has_spell conds = %v, want [1 = 0]", conds) + } +} + +func TestSpellFilterConds_Empty(t *testing.T) { + ab := &argBuilder{} + if conds := spellFilterConds(map[string][]string{}, testSpells, ab); conds != nil { + t.Errorf("no params => nil conds, got %v", conds) + } +} +``` + +Note: `spellFilterConds` takes `url.Values` (which IS `map[string][]string`), so the literal maps above compile directly. + +- [ ] **Step 2: Run tests to verify they fail** + +Run with `-run 'TestSpellFilterConds'`. Expected: `FAIL ... [build failed]` with `undefined: spellFilterConds`. + +- [ ] **Step 3: Write the implementation** + +Append to `go-services/inventory-go/spell_filter.go` (add `"net/url"` to its imports): + +```go +// spellExists renders the EXISTS clause for a set of spell IDs, binding each +// ID as a positional arg. ids must be non-empty. +func spellExists(ids []int, ab *argBuilder) string { + ph := make([]string, len(ids)) + for i, id := range ids { + ph[i] = ab.add(id) + } + return "EXISTS (SELECT 1 FROM item_spells sp WHERE sp.item_id = db_item_id AND sp.spell_id IN (" + strings.Join(ph, ", ") + "))" +} + +// spellFilterConds builds the WHERE conditions for has_spell, spell_contains, +// and legendary_cantrips. Cantrips use AND semantics — one EXISTS per checked +// cantrip (NOT the legacy COUNT>=N trick, which can false-positive when one +// label matches two spells on an item while another matches none). Any +// no-match filter yields the impossible condition, matching legacy intent. +func spellFilterConds(q url.Values, spells map[int]map[string]any, ab *argBuilder) []string { + var conds []string + impossible := func() { conds = append(conds, "1 = 0") } + + if v := q.Get("has_spell"); v != "" { + if id, ok := exactSpellID(spells, v); ok { + conds = append(conds, spellExists([]int{id}, ab)) + } else { + impossible() + } + } + if v := q.Get("spell_contains"); v != "" { + if ids := spellIDsContaining(spells, v); len(ids) > 0 { + conds = append(conds, spellExists(ids, ab)) + } else { + impossible() + } + } + if v := q.Get("legendary_cantrips"); v != "" { + for _, name := range splitNonEmpty(v) { + if ids := spellIDsForCantrip(spells, name); len(ids) > 0 { + conds = append(conds, spellExists(ids, ab)) + } else { + impossible() + } + } + } + return conds +} +``` + +(`splitNonEmpty` and `argBuilder` already exist in `search.go`.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run with `-run 'TestSpellFilter|TestSpellIDs|TestExactSpell'`. Expected: all `PASS`. + +- [ ] **Step 5: Commit** + +```bash +git add go-services/inventory-go/spell_filter.go go-services/inventory-go/spell_filter_test.go +git commit -m "feat(inventory-go): build EXISTS conditions for spell filter params" +``` + +--- + +### Task 3: Wire into runSearch + +**Files:** +- Modify: `go-services/inventory-go/search.go` (inside `runSearch`, after the `slot_names` block at ~line 295-303, before `where := ""`) + +- [ ] **Step 1: Add the wiring** + +In `search.go`, directly after the `slot_names` block (the `if v := q.Get("slot_names"); v != "" { ... }` closing brace) and before `where := ""`, insert: + +```go + // --- spell filters: has_spell / spell_contains / legendary_cantrips --- + conds = append(conds, spellFilterConds(q, s.spells, ab)...) +``` + +This runs before the LIMIT/OFFSET args are appended, so the count query's `ab.args[:len(ab.args)-2]` slicing stays correct, and the conditions apply identically to the items query and the count query. + +- [ ] **Step 2: Run the full suite** + +Run the SSH+Docker test command with `-run .` (everything). Expected: all tests `PASS`, `ok` for the package. + +- [ ] **Step 3: Commit** + +```bash +git add go-services/inventory-go/search.go +git commit -m "feat(inventory-go): wire spell filters into /search/items" +``` + +--- + +### Task 4: Deploy and live verification + +**Files:** none (operational). + +- [ ] **Step 1: Sync source to the server's production checkout** + +```bash +cd "C:/Users/erikn/source/repos/dereth-workspace/MosswartOverlord" && \ +tar czf - go-services | ssh erik@overlord.snakedesert.se "tar xzf - -C /home/erik/MosswartOverlord/" +``` + +- [ ] **Step 2: Build (test-gated) and recreate the container** + +```bash +ssh erik@overlord.snakedesert.se 'cd /home/erik/MosswartOverlord && \ + export BUILD_VERSION="$(date -u +%Y.%-m.%-d.%H%M)-$(git rev-parse --short HEAD)" && \ + docker compose -f docker-compose.yml -f go-services/docker-compose.go.yml build --build-arg BUILD_VERSION=$BUILD_VERSION inventory-go && \ + docker compose -f docker-compose.yml -f go-services/docker-compose.go.yml -f go-services/docker-compose.cutover.yml \ + up -d --no-deps inventory-go' +``` + +Expected: `RUN go test ./...` passes inside the build; container recreated and `Started`. + +- [ ] **Step 3: Live verification (direct against inventory-go on 127.0.0.1:8772)** + +AND semantics — every returned item must carry BOTH cantrips: + +```bash +ssh erik@overlord.snakedesert.se "curl -s 'http://127.0.0.1:8772/search/items?include_all_characters=true&jewelry_only=true&legendary_cantrips=Legendary%20Invulnerability,Legendary%20Summoning%20Prowess&limit=10'" \ + | python -c "import json,sys; d=json.load(sys.stdin); print('total:', d['total_count']); [print(i['name'], '|', i.get('spell_names')) for i in d['items']]" +``` + +Expected: every listed item's `spell_names` includes both `Legendary Invulnerability` and `Legendary Summoning Prowess`. (If total is 0, relax to a single common cantrip to confirm the mechanism, e.g. only `Legendary Invulnerability`.) + +Jewelry-type composition — same query plus `&slot_names=Ring`: + +```bash +ssh erik@overlord.snakedesert.se "curl -s 'http://127.0.0.1:8772/search/items?include_all_characters=true&jewelry_only=true&slot_names=Ring&legendary_cantrips=Legendary%20Invulnerability&limit=10'" \ + | python -c "import json,sys; d=json.load(sys.stdin); print('total:', d['total_count']); [print(i['name'], '|', i.get('slot_name'), '|', i.get('spell_names')) for i in d['items']]" +``` + +Expected: only ring-slot items, all carrying Legendary Invulnerability. + +`spell_contains`: + +```bash +ssh erik@overlord.snakedesert.se "curl -s 'http://127.0.0.1:8772/search/items?include_all_characters=true&spell_contains=Epic%20Invulnerability&limit=5'" \ + | python -c "import json,sys; d=json.load(sys.stdin); print('total:', d['total_count']); [print(i['name'], '|', i.get('spell_names')) for i in d['items']]" +``` + +Expected: items whose `spell_names` include `Epic Invulnerability`; non-zero total if any exist in inventories. + +Regression — a filterless jewelry search still works and `total_count` is consistent: + +```bash +ssh erik@overlord.snakedesert.se "curl -s 'http://127.0.0.1:8772/search/items?include_all_characters=true&jewelry_only=true&limit=1'" \ + | python -c "import json,sys; d=json.load(sys.stdin); print('total:', d['total_count'], 'items:', len(d['items']))" +``` + +Expected: same total as before the deploy (spot-check it's a plausible large number, not 0). + +- [ ] **Step 4: Check container logs for errors** + +```bash +ssh erik@overlord.snakedesert.se "docker logs inventory-go --tail 20 2>&1" +``` + +Expected: normal request logs, no SQL errors. + +- [ ] **Step 5: Push** + +```bash +git push origin master +``` + +(Server checkout already matches via the tar sync; the push keeps git as source of truth.) diff --git a/docs/superpowers/specs/2026-07-14-inventory-spell-jewelry-filters-design.md b/docs/superpowers/specs/2026-07-14-inventory-spell-jewelry-filters-design.md index 15864ff7..85179d50 100644 --- a/docs/superpowers/specs/2026-07-14-inventory-spell-jewelry-filters-design.md +++ b/docs/superpowers/specs/2026-07-14-inventory-spell-jewelry-filters-design.md @@ -23,9 +23,11 @@ back only rings carrying BOTH cantrips. - **AND semantics** for multiple checked cantrips (matches legacy intent): an item must carry every checked cantrip. -- **Multi-select checkboxes** for jewelry type (Ring, Bracelet, Necklace, - Trinket), shown only when the equipment-type selector is on Jewelry. - None checked = all jewelry. +- **Jewelry type: reuse the existing Equipment Slots card.** During planning + we found `inventory.html` already has Ring / Bracelet / Neck / Trinket + checkboxes (`#all-slots`) that `inventory.js` sends as `slot_names`, which + the Go backend fully supports. No new UI — verify the existing checkboxes + compose correctly with the new spell filters. - **Also wire `spell_contains`** (the dead free-text box) and `has_spell` (exact-name variant referenced by the agent tools) in the same pass — same mechanism, nearly free. @@ -65,18 +67,14 @@ filters, sorting, pagination, and the count query: Spell IDs originate from our own enum map, not user input, but are still bound via the existing `argBuilder` positional params for consistency. -## Frontend design (`static/inventory.html` + `static/inventory.js`) +## Frontend design -- New "Jewelry Type" filter card with checkboxes **Ring, Bracelet, Necklace, - Trinket**, shown only when equipment type = Jewelry (same show/hide pattern - as the weapon-type selector). -- `inventory.js` maps checked boxes to `slot_names` CSV using the backend's - slot vocabulary: Ring→`Ring`, Bracelet→`Bracelet`, Necklace→`Neck`, - Trinket→`Trinket`. None checked → param omitted. -- The cantrip grid and spell text box already send the correct params — no - JS changes needed for those. -- The existing `slotNameClause` per-type OR approaches (including the - Trinket clause's `%bracelet%` exclusion) are reused untouched. +**No frontend changes.** The cantrip grid, spell text box, and Equipment +Slots checkboxes (Ring / Bracelet / Neck / Trinket) already send the correct +params (`legendary_cantrips`, `spell_contains`, `slot_names`); the existing +`slotNameClause` per-type OR approaches (including the Trinket clause's +`%bracelet%` exclusion) are reused untouched. Live verification must confirm +jewelry-type + cantrip searches compose end-to-end. ## Testing