docs(plan): inventory spell filters implementation plan; spec: reuse existing slots UI

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-07-14 23:32:53 +02:00
parent aa3139b586
commit bd9c1ba556
2 changed files with 445 additions and 14 deletions

View file

@ -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 <PATTERN> 2>&1'"
```
Replace `<PATTERN>` 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.)

View file

@ -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