From 0fd539bedf6523eb95409b3e9359cc1e6c454c1f Mon Sep 17 00:00:00 2001 From: erik Date: Fri, 30 Jan 2026 19:04:22 +0000 Subject: [PATCH] Fix suitbuilder early termination and add armor level scoring Bug fixes: - Remove "TEMPORARY FIX" that stopped search after finding first suit - Add armor level as lowest-priority tiebreaker in scoring Scoring formula now: - Set completion: +1000 per complete set - Missing pieces: -200 penalty each - Crit damage: +10/20 per item - Damage rating (clothes): +10/20/30 - Spell coverage: +100 per fulfilled spell - Base item score: +5 per item - Armor level: +1 per 100 AL (tiebreaker) Updated design doc with audit findings - most features were already working correctly. The magsuitalgo.md analysis was outdated. Co-Authored-By: Claude Opus 4.5 --- docs/plans/2026-01-30-suitbuilder-design.md | 158 ++++++++++---------- inventory-service/suitbuilder.py | 13 +- 2 files changed, 90 insertions(+), 81 deletions(-) diff --git a/docs/plans/2026-01-30-suitbuilder-design.md b/docs/plans/2026-01-30-suitbuilder-design.md index 794e19d2..716e4b63 100644 --- a/docs/plans/2026-01-30-suitbuilder-design.md +++ b/docs/plans/2026-01-30-suitbuilder-design.md @@ -64,118 +64,126 @@ Instead of checking all item combinations, we organize items into "buckets" by e ## Section 3: Current Implementation State -### What's Working +**Updated after Task 1 Audit (2026-01-30)** -- **Spell Bitmap System** - Correctly implements O(1) spell overlap detection -- **Armor Set Constraints** - 5+4 logic matches MagSuitBuilder exactly -- **Coverage Mask** - Body area tracking with reduction logic for multi-slot items -- **API Endpoints** - `/search`, `/characters`, `/sets` all functional -- **Streaming Results** - SSE (Server-Sent Events) for real-time feedback -- **Frontend** - `suitbuilder.html` and `suitbuilder.js` exist and connect to API +### What's Working ✅ -### What's Broken +| Feature | Location | Status | +|---------|----------|--------| +| Bucket creation (all 17 slots) | Lines 1006-1135 | ✅ Working | +| Bucket sorting (armor first, smallest first) | Lines 1127-1130 | ✅ Working | +| ItemPreFilter (surpassing logic) | Lines 503-583, used at 969 | ✅ Working | +| Item sorting (by spell count/ratings) | Lines 971-991 | ✅ Working | +| Spell bitmap system | SpellBitmapIndex class | ✅ Working | +| Set constraints (5+4 logic) | can_add_item() | ✅ Working | +| Coverage mask with reductions | Lines 1137-1240 | ✅ Working | +| API Endpoints | `/search`, `/characters`, `/sets` | ✅ Working | +| Streaming Results (SSE) | recursive_search() | ✅ Working | +| Frontend | `suitbuilder.html`, `suitbuilder.js` | ✅ Working | -- **Incomplete Bucket Creation** - Only creates 2 buckets (Chest + Head) instead of 11+ slots -- **Search Exits Early** - Returns first result instead of completing full search -- **No Item Sorting** - Items processed in database order, not optimal order (best items first) -- **Goes in Circles** - Previous development sessions made inconsistent changes +### What's Broken ❌ -### What's Missing +| Bug | Location | Issue | +|-----|----------|-------| +| **Early termination** | Lines 1326-1329 | "TEMPORARY FIX" stops after finding 1 suit | +| **Armor level not scored** | `_calculate_score()` | Not included as tiebreaker | -- **AccessorySearcher Phase** - Jewelry optimization not implemented -- **Item Pre-Filtering** - No "surpassing logic" to remove inferior items before search -- **Progressive Result Tracking** - Should track top-N suits per piece count -- **Proper Scoring** - Needs to score by: set completion > spell coverage > armor level +### Current Scoring Formula + +``` +score = set_completion_bonus (1000 per complete set) + + set_missing_penalty (-200 per missing piece) + + crit_damage (10-20 per item) + + damage_rating on clothes (10-30) + + spell_coverage (100 per fulfilled spell) + + base_item_score (5 per item) + +MISSING: + armor_level as lowest-priority tiebreaker +``` ### Code Structure - `suitbuilder.py`: 1,847 lines - Main solver class (`ConstraintSatisfactionSolver`): ~1,130 lines +### Note: magsuitalgo.md Analysis Was Outdated + +The analysis document incorrectly stated: +- ❌ "Only creates 2 buckets" → Actually creates all 17 +- ❌ "No item pre-filtering" → ItemPreFilter exists and is used +- ❌ "No item sorting" → Items are sorted by spell count/ratings + +The code is in much better shape than documented. Only 2 bugs need fixing. + --- ## Section 4: Implementation Plan -**Approach:** Fix incrementally, verify after each step. - -Each task is small, self-contained, and testable. Complete one fully before starting the next. +**Updated after audit - most tasks already done!** --- -### Task 1: Audit & Document Current Search Flow +### Task 1: Audit & Document Current Search Flow ✅ COMPLETE -**Goal:** Understand exactly what the current code does before changing it +**Findings:** +- Bucket creation: ✅ All 17 slots created correctly +- Pre-filtering: ✅ ItemPreFilter.remove_surpassed_items() used at line 969 +- Sorting: ✅ Items sorted by spell count (armor/jewelry) and damage rating (clothes) +- Set constraints: ✅ 5+4 logic in can_add_item() -**Steps:** -- Trace through `ConstraintSatisfactionSolver.search()` and document the flow -- Identify where buckets are created (find the 2-bucket limitation) -- Identify where search exits early -- Write findings as comments or separate doc - -**Verify:** Can explain the current flow without looking at code +**Bugs Found:** +1. Early termination at lines 1326-1329 ("TEMPORARY FIX") +2. Armor level missing from scoring --- -### Task 2: Fix Bucket Creation +### Task 2: Fix Bucket Creation ✅ ALREADY WORKING -**Goal:** Create buckets for all 11 armor/clothing slots - -**Steps:** -- Find bucket creation code -- Add missing slots: Head, Chest, Abdomen, Upper Arms, Lower Arms, Hands, Upper Legs, Lower Legs, Feet, plus clothing slots -- Ensure items are placed in correct buckets based on `EquipMask` - -**Verify:** Log bucket counts - should see items distributed across 11+ buckets, not just 2 +No changes needed - all 17 slots are created at lines 1006-1135. --- -### Task 3: Fix Search Completion +### Task 3: Fix Search Completion ✅ COMPLETE -**Goal:** Search processes ALL buckets before returning results +**Goal:** Remove early termination so search finds multiple suits -**Steps:** -- Find early exit condition -- Remove or fix it so recursion continues through all buckets -- Ensure backtracking works correctly +**The Bug (was at lines 1326-1329):** +```python +# TEMPORARY FIX: Stop search after finding first suit to test completion +if len(self.best_suits) >= 1: + logger.info(f"[DEBUG] EARLY TERMINATION...") + return +``` -**Verify:** Search logs show "processing bucket 1... bucket 2... bucket 11..." not stopping at 2 +**Fix Applied:** Removed this block entirely. --- -### Task 4: Add Item Sorting +### Task 4: Add Item Sorting ✅ ALREADY WORKING -**Goal:** Process best items first for better pruning - -**Steps:** -- Sort armor items by: set membership → armor level (descending) -- Sort within each bucket before search begins - -**Verify:** Log first 5 items per bucket - should be highest armor level items +No changes needed - items sorted at lines 971-991. --- -### Task 5: Implement Proper Scoring +### Task 5: Add Armor Level to Scoring ✅ COMPLETE -**Goal:** Score suits by correct priority order +**Goal:** Add armor_level as lowest-priority tiebreaker -**Steps:** -- Score = (set_completion × 10000) + (spell_coverage × 100) + armor_level -- Set completion = count of pieces matching primary/secondary sets -- Update `CompletedSuit` scoring logic +**Location:** `_calculate_score()` method -**Verify:** Given two suits, the one with better set completion always scores higher regardless of armor +**Fix Applied:** Added armor level scoring after base item score: +```python +# 6. Armor level as tiebreaker (LOWEST PRIORITY) +# Scale down significantly so it only matters when other scores are equal +armor_score = state.total_armor // 100 # ~5 points per 500 AL +score += armor_score +``` --- -### Task 6: Add Item Pre-Filtering +### Task 6: Add Item Pre-Filtering ✅ ALREADY WORKING -**Goal:** Remove inferior items before search to reduce search space - -**Steps:** -- Implement "surpassing logic": item A surpasses item B if A has same slot, same set, better stats, and equal/better spells -- Filter out surpassed items before bucketing - -**Verify:** Log shows "Filtered X items down to Y items" with significant reduction +No changes needed - ItemPreFilter used at line 969. --- @@ -183,7 +191,7 @@ Each task is small, self-contained, and testable. Complete one fully before star **Goal:** After armor search, optimize jewelry slots -**Scope:** Separate task, do after Tasks 1-6 are solid +**Scope:** Separate task, do after core search is verified working --- @@ -207,10 +215,10 @@ Each task is small, self-contained, and testable. Complete one fully before star | Task | Status | Notes | |------|--------|-------| -| Task 1: Audit Current Flow | Not Started | | -| Task 2: Fix Bucket Creation | Not Started | | -| Task 3: Fix Search Completion | Not Started | | -| Task 4: Add Item Sorting | Not Started | | -| Task 5: Implement Proper Scoring | Not Started | | -| Task 6: Add Item Pre-Filtering | Not Started | | +| Task 1: Audit Current Flow | ✅ Complete | Found 2 bugs, most features working | +| Task 2: Fix Bucket Creation | ✅ Already Working | No changes needed | +| Task 3: Fix Search Completion | ✅ Complete | Removed early termination at lines 1326-1329 | +| Task 4: Add Item Sorting | ✅ Already Working | No changes needed | +| Task 5: Add Armor Level Scoring | ✅ Complete | Added armor_score = total_armor // 100 | +| Task 6: Add Item Pre-Filtering | ✅ Already Working | No changes needed | | Task 7: AccessorySearcher | Not Started | Future | diff --git a/inventory-service/suitbuilder.py b/inventory-service/suitbuilder.py index 0dbc6785..27394c0d 100644 --- a/inventory-service/suitbuilder.py +++ b/inventory-service/suitbuilder.py @@ -1322,11 +1322,6 @@ class ConstraintSatisfactionSolver: suit_data['stats']['secondary_set'] = secondary_set_name yield SearchResult(type="suit", data=suit_data) - - # TEMPORARY FIX: Stop search after finding first suit to test completion - if len(self.best_suits) >= 1: - logger.info(f"[DEBUG] EARLY TERMINATION: Found {len(self.best_suits)} suits, stopping search for testing") - return else: logger.info(f"[DEBUG] Suit REJECTED: score {suit.score} not better than existing") else: @@ -1615,7 +1610,13 @@ class ConstraintSatisfactionSolver: base_item_score = len(state.items) * 5 score += base_item_score logger.debug(f"[SCORING] Base item score: {len(state.items)} items = +{base_item_score} points") - + + # 6. Armor level as tiebreaker (LOWEST PRIORITY) + # Scale down significantly so it only matters when other scores are equal + armor_score = state.total_armor // 100 # ~5 points per 500 AL + score += armor_score + logger.debug(f"[SCORING] Armor level tiebreaker: {state.total_armor} AL = +{armor_score} points") + logger.debug(f"[SCORING] Final score: {score}") return max(0, score) # Never negative