refactor: code quality cleanup after review

- Extract normalizeModText() helper for repeated gsub pattern (6 uses)
- Lift qualityModifierToCatalyst to module level, built from catalystList
- Cache specName regex match to avoid double-match on catalyst parsing
- Defer localOverrides alloc until preferLocal is true
- Consolidate Eater/Exarch lookup merge into single loop
- Add nil-guard on fetchStats() in InitMods pseudo-stats path
- Fix spec indentation on added lines only (no whole-file reformat)

Claude Code helped here.

Author: BeardyBarber

spec/System/TestTradeQueryGenerator_spec.lua

@@ -238,5 +238,63 @@ describe("TradeQueryGenerator", function()
             assert.are.equal(1, #filters)
             assert.are.equal(2, filters[1].value.min)
         end)
+
+        -- Pass: Each individual stat in the count group has value.min = 1 so the trade API
+        --       evaluates them correctly. Without this the filter silently matches everything.
+        -- Fail: Missing value field = trade site ignores the stat, returning uncrafted items
+        it("each stat inside the count group has value.min = 1", function()
+            local filters = mock_queryGen:BuildCraftedSlotFilters(1, 0)
+            assert.are.equal(1, #filters)
+            for _, stat in ipairs(filters[1].filters) do
+                assert.is_not_nil(stat.value, "stat missing value field: " .. tostring(stat.id))
+                assert.are.equal(1, stat.value.min)
+            end
+        end)
+    end)
+
+    describe("CountCraftedAffixesFromModLines", function()
+        -- Uses the item's explicitModLines (modLine.crafted = true) rather than the affix
+        -- slot tables, which is the only reliable source for PoB-crafted items.
+        -- Pool entries: numeric-indexed text lines + type (string) + types (table, marks crafted).
+
+        -- Synthetic affix pool covering both a crafted suffix and a crafted prefix.
+        local syntheticPool = {
+            CraftedMana1  = { "(25-34) to maximum Mana",      type = "Suffix", types = { str = true } },
+            CraftedArmour1 = { "(30-40)% increased Armour",   type = "Prefix", types = { str = true } },
+        }
+
+        -- Pass: A crafted suffix mod line matches the pool and suffixCount = 1
+        -- Fail: Count stays 0 = crafted mod not matched, no slot filter emitted
+        it("counts a crafted suffix mod line", function()
+            local modLines = {
+                { line = "+29 to maximum Mana",          crafted = true  },
+                { line = "+45 to maximum Energy Shield", crafted = false },
+            }
+            local result = mock_queryGen:CountCraftedAffixesFromModLines(modLines, syntheticPool, nil)
+            assert.are.equal(0, result.prefix)
+            assert.are.equal(1, result.suffix)
+        end)
+
+        -- Pass: A crafted prefix mod line matches the pool and prefixCount = 1
+        -- Fail: prefix count stays 0 = Prefix type not recognised, filter uses wrong slot
+        it("counts a crafted prefix mod line", function()
+            local modLines = {
+                { line = "35% increased Armour", crafted = true },
+            }
+            local result = mock_queryGen:CountCraftedAffixesFromModLines(modLines, syntheticPool, nil)
+            assert.are.equal(1, result.prefix)
+            assert.are.equal(0, result.suffix)
+        end)
+
+        -- Pass: Non-crafted mod lines are ignored; counts stay zero
+        -- Fail: Any non-zero count means regular mods trigger spurious slot filters
+        it("ignores non-crafted mod lines", function()
+            local modLines = {
+                { line = "+45 to maximum Energy Shield", crafted = false },
+            }
+            local result = mock_queryGen:CountCraftedAffixesFromModLines(modLines, syntheticPool, nil)
+            assert.are.equal(0, result.prefix)
+            assert.are.equal(0, result.suffix)
+        end)
     end)
 end)

src/Classes/Item.lua

@@ -24,6 +24,19 @@ local catalystTags = {
 	{ "jewellery_elemental" ,"elemental_damage" },
 	{ "critical" },
 }
+-- Maps game clipboard "Quality (X Modifiers)" label to catalyst index (same order as catalystList/catalystTags).
+local catalystQualityModifier = {
+	["Attack Modifiers"]            = 1,
+	["Speed Modifiers"]             = 2,
+	["Life and Mana Modifiers"]     = 3,
+	["Caster Modifiers"]            = 4,
+	["Attribute Modifiers"]         = 5,
+	["Physical and Chaos Modifiers"]= 6,
+	["Resistance Modifiers"]        = 7,
+	["Defense Modifiers"]           = 8,
+	["Elemental Modifiers"]         = 9,
+	["Critical Modifiers"]          = 10,
+}
 
 local function getCatalystScalar(catalystId, tags, quality)
 	if not catalystId or type(catalystId) ~= "number" or not catalystTags[catalystId] or not tags or type(tags) ~= "table" or #tags == 0 then
@@ -582,19 +595,8 @@ function ItemClass:ParseRaw(raw, rarity, highQuality)
 					self.catalystQuality = specToNumber(specVal)
 				elseif specName:match("^Quality %((.-)%)$") and not self.catalyst then
 					-- Game clipboard format: "Quality (Life and Mana Modifiers): +20% (augmented)"
-					local qualityModifierToCatalyst = {
-						["Attack Modifiers"] = 1,
-						["Speed Modifiers"] = 2,
-						["Life and Mana Modifiers"] = 3,
-						["Caster Modifiers"] = 4,
-						["Attribute Modifiers"] = 5,
-						["Physical and Chaos Modifiers"] = 6,
-						["Resistance Modifiers"] = 7,
-						["Defense Modifiers"] = 8,
-						["Elemental Modifiers"] = 9,
-						["Critical Modifiers"] = 10,
-					}
-					local catalystId = qualityModifierToCatalyst[specName:match("^Quality %((.-)%)$")]
+					local qualityModType = specName:match("^Quality %((.-)%)$")
+					local catalystId = catalystQualityModifier[qualityModType]
 					if catalystId then
 						self.catalyst = catalystId
 						self.catalystQuality = tonumber(specVal:match("(%d+)"))

src/Classes/TradeQueryGenerator.lua

@@ -82,6 +82,11 @@ local tradeStatCategoryIndices = {
 	["PassiveNode"] = 2,
 }
 
+-- Strips numeric tokens and punctuation for text-based mod matching.
+local function normalizeModText(text)
+	return text:gsub("[#()0-9%-%+%.]",""):gsub("%s+", " ")
+end
+
 local influenceSuffixes = { "_shaper", "_elder", "_adjudicator", "_basilisk", "_crusader", "_eyrie"}
 local influenceDropdownNames = { "None" }
 local hasInfluenceModIds = { }
@@ -125,21 +130,21 @@ local function normTradeText(text)
 	return (text:match("^%s*(.-)%s*$"))
 end
 
--- Forward declaration — defined below, after the class method definitions.
+-- Forward declaration - defined below, after the class method definitions.
 local buildAndCachePseudoMapping
 
 -- Hard-coded fallbacks for pseudo texts that don't normalize to the same form as any explicit/implicit text.
 -- Key = normalized pseudo text; value = list of explicit/implicit tradeModIds to map to that pseudo.
 local pseudoNormFallbacks = {
-	-- Pseudo "+#% total Attack Speed" → "attack speed"; explicit "#% increased Attack Speed" → "increased attack speed"
+	-- Pseudo "+#% total Attack Speed" -> "attack speed"; explicit "#% increased Attack Speed" -> "increased attack speed"
 	["attack speed"]                      = { "explicit.stat_681332047",  "implicit.stat_681332047"  },
-	-- Pseudo "+#% total Cast Speed" → "cast speed"; explicit "#% increased Cast Speed" → "increased cast speed"
+	-- Pseudo "+#% total Cast Speed" -> "cast speed"; explicit "#% increased Cast Speed" -> "increased cast speed"
 	["cast speed"]                        = { "explicit.stat_2891184298", "implicit.stat_2891184298" },
-	-- Pseudo "+#% Global Critical Strike Chance" → "global critical strike chance";
-	-- explicit "#% increased Global Critical Strike Chance" → "increased global critical strike chance"
+	-- Pseudo "+#% Global Critical Strike Chance" -> "global critical strike chance";
+	-- explicit "#% increased Global Critical Strike Chance" -> "increased global critical strike chance"
 	["global critical strike chance"]     = { "explicit.stat_587431675",  "implicit.stat_587431675"  },
-	-- Pseudo "+#% total Critical Strike Chance for Spells" → "critical strike chance for spells";
-	-- explicit "#% increased Critical Strike Chance for Spells" → "increased critical strike chance for spells"
+	-- Pseudo "+#% total Critical Strike Chance for Spells" -> "critical strike chance for spells";
+	-- explicit "#% increased Critical Strike Chance for Spells" -> "increased critical strike chance for spells"
 	["critical strike chance for spells"] = { "explicit.stat_737908626",  "implicit.stat_737908626"  },
 }
 
@@ -456,6 +461,9 @@ function TradeQueryGeneratorClass:InitMods()
 			end
 		else
 			local tradeStats = fetchStats()
+			if not tradeStats then
+				return
+			end
 			tradeStats:gsub("\n", " ")
 			local tradeQueryStatsParsed = dkjson.decode(tradeStats)
 			buildAndCachePseudoMapping(tradeQueryStatsParsed)
@@ -476,6 +484,9 @@ function TradeQueryGeneratorClass:InitMods()
 
 	-- originates from: https://www.pathofexile.com/api/trade/data/stats
 	local tradeStats = fetchStats()
+	if not tradeStats then
+		return
+	end
 	tradeStats:gsub("\n", " ")
 	local tradeQueryStatsParsed = dkjson.decode(tradeStats)
 
@@ -1122,7 +1133,7 @@ function TradeQueryGeneratorClass:CountCraftedAffixesFromModLines(modLines, item
 		for modId, mod in pairs(pool) do
 			if mod.types then  -- only crafted mods use types instead of weightKey/weightVal
 				for _, line in ipairs(mod) do
-					local key = line:gsub("[#()0-9%-%+%.]", ""):gsub("%s+", " "):match("^%s*(.-)%s*$")
+					local key = normalizeModText(line):match("^%s*(.-)%s*$")
 					if key ~= "" and not lineToAffix[key] then
 						lineToAffix[key] = { affixType = mod.type, modId = modId }
 					end
@@ -1137,7 +1148,7 @@ function TradeQueryGeneratorClass:CountCraftedAffixesFromModLines(modLines, item
 	local prefixCount, suffixCount = 0, 0
 	for _, modLine in ipairs(modLines) do
 		if modLine.crafted then
-			local key = modLine.line:gsub("[#()0-9%-%+%.]", ""):gsub("%s+", " "):match("^%s*(.-)%s*$")
+			local key = normalizeModText(modLine.line):match("^%s*(.-)%s*$")
 			local match = lineToAffix[key]
 			if match and not seenModIds[match.modId] then
 				seenModIds[match.modId] = true
@@ -1311,32 +1322,35 @@ function TradeQueryGeneratorClass:FinishQuery()
 		-- maps to the (Local) trade stat rather than the global ring/jewel version.
 		local function buildTextLookup(modTypeData, preferLocal)
 			local lookup = {}
-			local localOverrides = {}
+			-- Only allocate the local-override table when the caller needs it.
+			local localOverrides = preferLocal and {} or nil
 			for _, entry in pairs(modTypeData) do
-				local key = entry.tradeMod.text:gsub("[#()0-9%-%+%.]",""):gsub("%s+", " ")
+				local key = normalizeModText(entry.tradeMod.text)
 				if key ~= "" and not lookup[key] then
 					lookup[key] = entry.tradeMod.id
 				end
 				-- overrideModLine is set for "(Local)" trade stats; its value is the item
 				-- display text without " (Local)", which is what modLine.line will contain.
-				local overrideLine = entry.specialCaseData and entry.specialCaseData.overrideModLine
-				if overrideLine then
-					local overrideKey = overrideLine:gsub("[#()0-9%-%+%.]",""):gsub("%s+", " ")
-					if overrideKey ~= "" then
-						localOverrides[overrideKey] = entry.tradeMod.id
+				if localOverrides then
+					local overrideLine = entry.specialCaseData and entry.specialCaseData.overrideModLine
+					if overrideLine then
+						local overrideKey = normalizeModText(overrideLine)
+						if overrideKey ~= "" then
+							localOverrides[overrideKey] = entry.tradeMod.id
+						end
 					end
 				end
 				local singular = entry.specialCaseData and entry.specialCaseData.overrideModLineSingular
 				if singular then
-					local singKey = singular:gsub("[#()0-9%-%+%.]",""):gsub("%s+", " ")
+					local singKey = normalizeModText(singular)
 					if singKey ~= "" and not lookup[singKey] then
 						lookup[singKey] = entry.tradeMod.id
 					end
 				end
 			end
 			-- Merge local overrides last so they win over any global entry with the same text.
-			-- Only do this for item types that actually carry local mods (weapons and armour).
-			if preferLocal then
+			-- Only done for item types that actually carry local mods (weapons and armour).
+			if localOverrides then
 				for k, v in pairs(localOverrides) do
 					lookup[k] = v
 				end
@@ -1347,11 +1361,10 @@ function TradeQueryGeneratorClass:FinishQuery()
 		local explicitTextToId = buildTextLookup(self.modData.Explicit, preferLocal)
 		local implicitTextToId = buildTextLookup(self.modData.Implicit)
 		if options.includeEldritch then
-			for k, v in pairs(buildTextLookup(self.modData.Eater)) do
-				explicitTextToId[k] = explicitTextToId[k] or v
-			end
-			for k, v in pairs(buildTextLookup(self.modData.Exarch)) do
-				explicitTextToId[k] = explicitTextToId[k] or v
+			for _, eldritchModType in ipairs({ "Eater", "Exarch" }) do
+				for k, v in pairs(buildTextLookup(self.modData[eldritchModType])) do
+					explicitTextToId[k] = explicitTextToId[k] or v
+				end
 			end
 		end
 
@@ -1387,7 +1400,7 @@ function TradeQueryGeneratorClass:FinishQuery()
 		local function addModLines(modLines, primaryLookup, fallbackLookup)
 			for _, modLine in ipairs(modLines) do
 				if not modLine.crafted then
-					local matchStr = modLine.line:gsub("[#()0-9%-%+%.]",""):gsub("%s+", " ")
+					local matchStr = normalizeModText(modLine.line)
 					local tradeModId = primaryLookup[matchStr]
 					local usedFallback = false
 					if not tradeModId and fallbackLookup then