fix: correct Runic Ward import, double-count, and EHP operator precedence

[jay9297]

Summary

• ImportTab:ImportItem missing return item — The function parsed the GGG API item correctly but never returned it, so all API-import callers received nil. Items imported via the character API were silently dropped.
• Ward double-counting on authoritative items — When armourData.Ward is pre-set from a paste or API property line (Runic Ward: 104), that value is the game's final post-quality result. The old code still applied wardInc rune mods on top of the already-final value, and left unconsumed flat BASE mods to leak into CalcDefence. Fixed with a wardIsAuthoritative flag: both BASE and INC local mods are consumed (removed from modList) but discarded; the property-line value is used verbatim.
• Operator precedence zeroing EHP pool — output[...TotalHitPool] + output.Ward or 0 parsed as (... + output.Ward) or 0, silently zeroing the entire hit pool when Ward was nil. Fixed with explicit parentheses: (output.Ward or 0).

Test plan

• All 392 existing tests pass: docker compose run --rm tests
• Paste a Runeforged item with Runic Ward: X — displayed Ward equals X (no quality re-scaling)
• Import a character via GGG API wearing a Runeforged body armour — item appears in the Items tab with correct Ward value
• Character with Ward equipped shows correct EHP (Life + Ward, not zeroed)
• Item with rune mods in runeModLines — #runeModLines reflects rune count, mods not double-applied

[github-actions]

🔄 Cycle 1, attempt 1/3
Now I have enough context to write the full review.

---

Code Review

1. Calculation Correctness

CalcDefence.lua:3634 — operator-precedence fix ✅
output[damageType.."TotalHitPool"] + output.Ward or 0 previously evaluated as (... + output.Ward) or 0 — a Lua precedence trap that silently crashes when output.Ward is nil instead of defaulting to 0. Adding parentheses (output.Ward or 0) is correct.

Item.lua:1791 — latent double-count in authoritative path ❌

wardBase = armourData.Ward + (self.base.armour.Ward or 0)

When wardIsAuthoritative is true, armourData.Ward is the game's final value, which already incorporates self.base.armour.Ward. Adding it again double-counts base Ward for any armour base that declares armour.Ward. No current glove base has a non-zero armour.Ward (including the new Fists of Stone entry), so this is latent rather than immediately visible — but the semantics are wrong. The line should simply be wardBase = armourData.Ward.

Item.lua:1789-1800 — consume-then-ternary ordering ✅
When authoritative, BASE is consumed at 1789, INC at 1790, then the ternary at 1800 short-circuits to 0 without calling calcLocal a second time. The non-authoritative path consumes BASE at 1793 and INC at 1800. No double-consume, no missed consume.

CalcPerform.lua:3186 — quality applied from glove item ✅
qualityMult = 1 + (gloveItem.quality or 0) / 100 uses the equipped item's actual quality, not Fists of Stone's data-file quality = 20. Correct: the transform replaces base stats but preserves the item's own quality tier.

CalcPerform.lua:3465-3473 — restore does not include Ward ✅
The transform never writes to armourData.Ward, so there is nothing to restore. Ward from the Fists of Stone implicits is injected only into the ephemeral per-pass modDB, which is rebuilt each calc pass.

CalcPerform.lua:3180-3207 — no error-recovery guard ⚠️
If an error is thrown between the mutation (line 3192) and the restore block (line 3466), the glove item is left corrupted for the session. Lua 5.1 has no try/finally; adding a pcall around calcs.defence and friends is likely impractical here, but it is a pre-existing pattern in PoB. Noting it; not a blocker given the project's existing error-handling posture.

CalcPerform.lua:1740-1742 — scoped gloves attribute-requirement ignore ✅
ignoreGlovesAttrReq is checked inside the per-source loop, while the global ignoreAttrReq is applied after the loop at line 1771. They are correctly independent. The test in TestStonefist_spec.lua at line 121 verifies the global flag is not set.

---

2. Modifier Parsing — new test cases present?

ModParser.lua — ^has ✅
Strips the "Has" prefix so "Has +2 to Evasion Rating per player level" → "+2 to Evasion Rating per player level" → hits the standard number+stat path. Test coverage in TestItemParse_spec.lua (the new implicitModTypes item test).

ModParser.lua — per player level ✅
Aliases to the existing { type = "Multiplier", var = "Level" } tag. Correct and tested.

ModParser.lua — maximum ward / maximum runic ward ✅
Both map to "Ward" stat, allowing "+65 to maximum Ward" rune-mod strings to produce Ward BASE mods that are subsequently consumed (and discarded) in the authoritative path. Test coverage in TestWard_spec.lua and TestItemParse_spec.lua.

ModParser.lua — Stonefist fragment their explicit modifiers are transformed… ⚠️
This is a sentence fragment. It works when passed as a standalone customMods line (tested in TestStonefist_spec.lua), and the full combined string is handled correctly by ModCache.lua. However, there is no specialModList entry for the first line fragment:
"Gloves you equip have their base type transformed to fists of stone while equipped, and" — this incomplete sentence is in ModCache but would fail ModParser if it appears in game data on its own. ModCache handles it correctly, so this is low risk in practice.

---

3. Nil Safety

ImportTab.lua:1104, 1110, 1117 — escapeGGGString called 2–3 times per property ⚠️

-- line 1104
elseif escapeGGGString(property.name) == "Evasion Rating" then
-- line 1110
elseif escapeGGGString(property.name) == "Energy Shield" then
-- line 1117
local escapedPropertyName = escapeGGGString(property.name)

escapeGGGString runs three gsub passes per call. Calling it up to three times in the same iteration is wasteful and introduces a maintenance hazard (changes to one call may not be reflected in others). The escaped name should be computed once before the if/elseif chain.

ImportTab.lua:1292 — return item with possibly-nil item.base ⚠️
item is returned even when item.base is nil (the if item.base then block at line 1269 is skipped). Callers that now receive a non-nil item but access item.base.armour without a nil guard will crash. The pre-existing callers of ImportItem assigned the result to nothing (void), so there is no immediate regression — but the new tests and any new callers must be aware.

CalcPerform.lua:3183 — nil guards on gloveItem ✅
gloveItem and gloveItem.armourData guards are present before any field access.

---

4. Data Integrity

gloves.lua — Fists of Stone ✅
Schema matches surrounding entries. req = { } (empty) intentionally signals no attribute requirements, consistent with the ascendancy's stated behaviour. armour = { Armour = 44, Evasion = 40, EnergyShield = 15 } matches the tooltip values referenced in tests. No Ward key — confirmed this is intentional; Ward comes from the implicits via modDB.

ModCache.lua — three Stonefist entries ✅
All three variants of the full combined mod string now emit proper FLAG mods instead of the old {nil, "string"} fallback. The GGG-API strings match the specialModList keys when lowercased and space-normalised.

---

5. Performance

CalcPerform.lua:3198-3200 — three modDB:NewMod calls in calc hot path ✅
These are inside a guard (if modDB:Flag(nil, "GloveBaseTypeTransform")) that is false for the vast majority of builds. No performance concern.

ImportTab.lua — repeated escapeGGGString calls (see §3 above) ⚠️
Same triple-call issue. Not in a hot calculation loop, so the impact is negligible in practice, but it should be fixed for correctness/readability.

---

6. Test Coverage

Path	Coverage
Ward import via `[Ward	Runic Ward]` property
Ward double-count regression (import path)	✅ TestImportTab_spec.lua:32
Ward double-count regression (paste path)	✅ TestWard_spec.lua:171
Ward paste path BuildModList idempotency	✅ TestItemParse_spec.lua:691
per player level + ^has ModParser rules	✅ TestItemParse_spec.lua:473
GloveBaseTypeTransform flag parsing	✅ TestStonefist_spec.lua:17
IgnoreAttributeRequirementsForGloves flag parsing	✅ TestStonefist_spec.lua:24
GloveExplicitModTransform flag parsing	✅ TestStonefist_spec.lua:31
Armour base-type transform (Fists of Stone armour value)	✅ TestStonefist_spec.lua:78
Fists of Stone implicit Evasion per level injection	✅ TestStonefist_spec.lua:103
Scoped glove req ignore does not zero global reqs	✅ TestStonefist_spec.lua:118
Non-authoritative ward path regression	❌ No explicit test that a plain ward item (no property line) still calculates correctly through the new if/else
Ward interaction during Stonefist transform	❌ No test for a ward-bearing glove being transformed
GloveExplicitModTransform actual effect	⚠️ Explicitly deferred (TODO comment in CalcPerform.lua)

spec/System/TestItemParse_spec.lua is missing a trailing newline (minor style issue; diff: \ No newline at end of file).

---

7. Style

Generally matches surrounding conventions. Some notes:

• src/Export/Scripts/modequivalencies.lua — the file is added to the repo, but src/Data/ModEquivalencies.lua (the file it generates) is absent. Any code path that would require or dofile this generated file needs a nil-guard; the CalcPerform comment acknowledges this explicitly.
• gloves.lua trailing blank lines (two blank lines after the new entry vs. one elsewhere) — cosmetic, easy to fix.

---

Required Changes

1. Item.lua:1791 — Remove the spurious + (self.base.armour.Ward or 0) in the authoritative branch:

   wardBase = armourData.Ward  -- property line is the final game value; base Ward is already baked in

2. ImportTab.lua — Hoist escapeGGGString(property.name) to a single local at the top of the property loop body, before the first if/elseif. Remove the two inline calls at lines 1104 and 1110 and replace with the local.

3. spec/System/TestItemParse_spec.lua — Add trailing newline at end of file.

4. Test gap — Add at least one test confirming the non-authoritative ward path (plain item with Ward explicitly set only in specialModList/mods, no property line) still produces the correct scaled value to prevent future regressions.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
All three confirmed. Now I have everything needed for the full review.

---

Code Review

1. Calculation Correctness

CalcDefence.lua — operator-precedence fix (line 3643)

The fix is correct. Lua's + binds tighter than or, so the old expression

… + output.Ward or 0

evaluated as (… + output.Ward) or 0 — crashing when output.Ward is nil and silently passing the falsy-zero edge case. The parenthesized form is correct.

Item.lua — authoritative Ward path (lines 1807–1837)

The two-branch design (wardIsAuthoritative) correctly addresses the double-count: when the game's property line is present, wardBase is taken verbatim and wardInc/defencesInc/qualityScalar are not re-applied. The per-level WardPerLevel path correctly keeps wardInc = 0 in authoritative mode while still applying defencesInc (which is independent of the baked value).

CalcDefence.lua — adjacent lines NOT fixed (lines 3648, 3650) ❌

The PR fixes line 3643 but misses two identical precedence bugs in the same do block:

-- line 3648 — same pattern, not fixed:
local GuardAbsorbRate = output["sharedGuardAbsorbRate"] or 0 + output[damageType.."GuardAbsorbRate"] or 0
-- line 3650 — same pattern, not fixed:
local GuardAbsorb = output["sharedGuardAbsorb"] or 0 + output[damageType.."GuardAbsorb"] or 0

Because + > or: A or 0 + B or 0 parses as A or (0 + B) or 0. When sharedGuardAbsorbRate is non-nil (the normal case for shared guard skills like Molten Shell), the expression short-circuits and the per-damage-type GuardAbsorbRate is silently dropped, producing an incorrect TotalHitPool. The intended form is (A or 0) + (B or 0). These two lines need the same fix as line 3643.

---

2. Modifier Parsing

No changes to ModParser.lua. The import-side escaping (escapeGGGString on property.name) and the [Ward|Runic Ward] → "Ward" normalisation happen in ImportTab.lua, which is appropriate. No test cases for ModParser.lua are needed here.

---

3. Nil Safety

ImportTab.lua — property.values access (line 1241)

item.quality = tonumber(property.values[1][1]:match("%d+"))

If the GGG API returns a Quality property with an empty values array, property.values[1] is nil and :match crashes. This pre-existed before the PR; the PR did not introduce or guard it. It is in-scope because the surrounding code was touched.

ImportTab.lua — multiple values entries accumulate Ward (lines 1263–1265)

for _, value in ipairs(property.values) do
    item.armourData[armourKey] = (item.armourData[armourKey] or 0) + tonumber(value[1])
end

If value[1] is a formatted string like "104 (augmented)", tonumber returns nil and addition crashes. For well-formed API data this is fine, but the comment above references {"104", 0} — the GGG API can return augmented-display strings for some properties.

Item.lua — wardIsAuthoritative sentinel (line 1807) ❌

local wardIsAuthoritative = armourData.Ward ~= nil

armourData is never fully reset between calls (self.armourData = self.armourData or { }, line 1961), and ParseRaw does not clear it. After the first BuildModList on a non-paste item writes a computed Ward value, BuildRaw serialises it as "Ward: X", ParseRaw reads it back, and every subsequent BuildModList call treats the item as authoritative. Ward mods added after the first call (e.g., a rune added via the UI) are consumed from modList but never reflected in the output — the value is locked at the first computed result. This regresses the previous behaviour, which always recomputed from mods.

---

4. Data Integrity

No changes to src/Data/ files. No data validation run required.

---

5. Performance

No new allocations in hot loops. The added guard variable (wardIsAuthoritative) and the two "consume-and-discard" calcLocal calls in BuildModListForSlotNum are cheap scalar reads that were already happening in the non-authoritative path.

---

6. Test Coverage

CRITICAL — stale assertions in TestAdvancedItemParse #item (spec/System/TestItemParse_spec.lua, ~line 744) ❌

The PR repurposes the #advanced per-level test by replacing the item under test (from "Striker's Grip / Fists of Stone" with Implicits: 3 to "Runeforged Serpentscale Coat" with no implicits), but the old assertions are left unchanged:

assert.are.equals(3, #item.implicitModLines)       -- will be 0
assert.are.equals("EvasionPerLevel", item.baseModList[1].name)  -- will be nil
assert.are.equals("EnergyShieldPerLevel", item.baseModList[2].name)
assert.are.equals("WardPerLevel", item.baseModList[3].name)

Confirmed: src/Data/Bases/body.lua shows implicitModTypes = { } for "Runeforged Serpentscale Coat". The test will fail at CI.

The per-level test was moved to TestItemParse (good), but the old slot in TestAdvancedItemParse now has a broken item/assertion mismatch that must be fixed.

Missing coverage — wardIsAuthoritative idempotency across mod changes

There is no test that:

1. Constructs a Ward item without a property line
2. Calls BuildModList (locking armourData.Ward)
3. Changes Ward mods (adds/removes a rune mod)
4. Calls BuildModList again and asserts the new value is reflected

This path is the regression introduced by the sentinel approach.

Missing coverage — zero-Ward authoritative item

No test checks what happens when the game property line reports Ward = 0. 0 ~= nil is true in Lua, so wardIsAuthoritative = true with wardBase = 0. round(0) = 0. Functionally harmless but untested.

---

7. Style

• escapedPropertyName local variable: clear improvement over repeated escapeGGGString calls; matches surrounding style.
• wardIsAuthoritative boolean: the name and comment block are descriptive. Inline ternary wardIsAuthoritative and … or … on lines 1824 and 1835–1837 is idiomatic Lua.
• Test description at TestImportTab_spec.lua line ~52 says "returns nil (not crash)" but ImportItem now returns the item on the success path; the body tests only assert.has_no.errors, so the test is correct but the description is misleading.

---

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
Now I have the data needed for a thorough review. Here is the full assessment.

---

Code Review

1. Calculation Correctness

CalcDefence.lua:3643 — operator-precedence fix

-- Before: (x + output.Ward) or 0  — the `or 0` never fires; x + nil crashes
-- After:  x + (output.Ward or 0)  — correct nil guard

The fix is unambiguously correct. Lua's or is lower-precedence than +, so the original crashed whenever output.Ward was nil. ✅

Item.lua:1799–1837 — authoritative Ward path

wardIsAuthoritative = armourData.Ward ~= nil is correctly set when a property line pre-populated armourData.Ward (import or paste). The branch:

• Calls calcLocal(modList, "Ward", "BASE", 0) and calcLocal(modList, "Ward", "INC", 0) to acknowledge those mods exist (prevents them from being double-applied later), then discards the results.
• Sets wardInc = 0 via short-circuit to exclude local INC from the final formula.
• defencesInc is computed unconditionally at line 1826 (confirmed by reading the file) — it is correctly applied to Armour/Evasion/ES but not to authoritative Ward, which is correct because the game's displayed property value already includes all local scaling.

Non-authoritative path math: with wardBase=65, wardInc=30, defencesInc=0, quality=20:
round(65 × 1.30 × 1.20) = round(101.4) = 101 ✅

Confirmed via data inspection: Serpentscale Coat has armour = { Evasion = 260, MovementPenalty = 0.03 } — no Ward key — so (self.base.armour.Ward or 0) = 0. The test's wardBase = 65 + 0 = 65 assumption and expected value 101 are correct. ✅

WardPerLevel scaling (line 1838) — borderline

armourData.WardPerLevel = wardPerLevel * (1 + (wardInc + defencesInc) / 100) * (1 + (qualityScalar / 100))

When authoritative, wardInc = 0 (good) but defencesInc still applies to wardPerLevel. This is defensible: per-level Ward comes from an implicit mod, not a rune, so it was never part of the baked-in property-line value and legitimately scales with local defences INC. However, there is no dedicated test for this path.

---

2. Modifier Parsing

No changes to ModParser.lua. The WardPerLevel stat name referenced in tests already exists in the parser (evidenced by existing passing tests that assert item.baseModList[3].name == "WardPerLevel"). No new test cases required. ✅

---

3. Nil Safety

ImportTab.lua — item.armourData = item.armourData or {} on the write path is correctly guarded. ✅

Item.lua — wardIsAuthoritative = armourData.Ward ~= nil: armourData is initialized earlier in BuildModListForSlotNum before this block, consistent with how Armour/Evasion/ES are handled. ✅

self.base.armour.Ward or 0 in the non-authoritative branch correctly guards the nil case. ✅

CalcDefence.lua — (output.Ward or 0) correctly guards nil. The pool variable output[damageType.."TotalHitPool"] is always set before this line per existing code structure. ✅

---

4. Data Integrity

No changes to src/Data/. ✅

---

5. Performance

ImportTab.lua: escapeGGGString is now called once and the result cached in escapedPropertyName. This is a minor improvement over the previous pattern of calling it repeatedly per-property. No new allocations. ✅

Item.lua: The authoritative branch adds two calcLocal calls (BASE + INC discard) and a boolean check. calcLocal does no heap allocation. No performance regression in this hot path. ✅

---

6. Test Coverage

Covered well:

• Import path: Ward from GGG [Ward|Runic Ward] property (TestImportTab_spec.lua:6)
• Import path: Ward with rune mods, double-count regression (TestImportTab_spec.lua:34)
• Operator precedence fix (TestWard_spec.lua — existing Ward output tests now exercise the fixed branch)
• Paste path: Ward survives BuildModList recalculation (TestItemParse_spec.lua:739)
• Non-authoritative path: base Ward + INC + quality (TestWard_spec.lua:203)
• Crash guard for degenerate item in Flask slot (TestImportTab_spec.lua:61)

Gaps — REQUEST CHANGES:

Gap A (medium): No test for authoritative Ward + non-zero local defencesInc. If a Runeforged item has a "% increased Defences" local mod alongside a Runic Ward property line, the expected behaviour is that defencesInc is excluded from Ward (already baked in) but applied to Armour. This edge-case regression is untested and directly neighbours the changed code.

Gap B (low): armourData.WardPerLevel scaling when wardIsAuthoritative = true (the defencesInc applies, wardInc is 0 path) has no dedicated assertion.

---

7. Style

Good:

• escapedPropertyName follows the surrounding camelCase naming. ✅
• wardIsAuthoritative is a clear boolean name. ✅
• Comment quality in Item.lua is high — the "Without fix: … / With fix: …" block in TestWard_spec.lua:171 is exemplary. ✅

Issues:

Style A: TestImportTab_spec.lua:61 — test name is "ImportItem returns nil (not crash) for Mace Strike…". After the return item fix, ImportItem returns the item for normal slots; for a Flask-slot skill gem it may return a non-nil item. The name says "returns nil" but the assertion only checks has_no.errors. The name is misleading and should be "does not crash for…".

Style B: TestWard_spec.lua:171 — description says "import path regression" but the test creates the item via paste (new("Item", [[…]])), not via importTab:ImportItem. Rename to "paste path regression" or restructure to use the import path.

---

Summary

#	Area	Verdict
1	Calculation correctness	✅ Math verified; operator-precedence fix is correct
2	Modifier parsing	✅ No ModParser changes; no new cases needed
3	Nil safety	✅ All new accesses guarded
4	Data integrity	✅ No data changes
5	Performance	✅ Minor improvement (escapeGGGString caching)
6	Test coverage	⚠️ Missing: authoritative Ward + defencesInc interaction
7	Style	⚠️ Two misleading test names

The core logic is sound. Please add a test covering authoritative Ward + a local "% increased Defences" mod, and correct the two misleading test names before merging.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
Now I have enough context to write the complete review.

---

Code Review

1. Calculation Correctness

Item.lua — authoritative Ward logic (paste path): CORRECT

BuildModListForSlotNum correctly branches on wardIsAuthoritative:

• Authoritative (property line present): wardBase = armourData.Ward, skips re-scaling by wardInc/defencesInc/quality.
• Non-authoritative: normal calcBase + base.Ward, full scaling pipeline.

CalcDefence.lua operator-precedence fix: CORRECT and CRITICAL

Line 3643 (before PR):

output[…TotalHitPool] = output[…TotalHitPool] + output.Ward or 0

Lua evaluates + before or, so this was (A + B) or 0. Since every number is truthy in Lua, the or 0 guard was never reachable — a nil output.Ward would crash. The parens fix is correct.

WardPerLevel scaling inconsistency (minor)

Line 1847: armourData.WardPerLevel = wardPerLevel * (1 + (wardInc + defencesInc) / 100) * ...

In the authoritative path wardInc = 0, but defencesInc still feeds into WardPerLevel. Whether that is intentional (per-level bonuses still scale with local defences mods) is unspecified by the tests; at minimum it should be documented.

---

2. Modifier Parsing

No changes to ModParser.lua. The new per-level stat names (WardPerLevel) and "Runic Ward" normalisation were pre-existing. No new test cases in TestModParser.lua are needed for this PR.

---

3. Nil Safety

wardBase = armourData.Ward (Item.lua line 1819) — UNGUARDED

wardFromPropertyLine = true is only set when specName == "Ward" (line 605-607). At that point self.armourData[specName] = specToNumber(specVal). If specToNumber returns nil (malformed property value like "Ward: abc"), the flag is set but armourData.Ward = nil. Then:

wardBase = armourData.Ward   -- nil
armourData.Ward = round(wardBase)  -- round(nil) → runtime error

Should be: wardBase = armourData.Ward or 0

---

4. Data Integrity

No data files (src/Data/) changed. No data validation needed.

---

5. Performance

No new table allocations inside hot loops. The two extra calcLocal calls in the authoritative branch (lines 1817-1818) are in BuildModListForSlotNum, called per-item on build update — acceptable.

---

6. Test Coverage

CRITICAL: Import path tests do not exercise BuildModList()

Both import Ward tests (TestImportTab_spec.lua lines ~8 and ~36) check item.armourData.Ward immediately after ImportItem() returns:

local item = importTab:ImportItem(mockItemData, "Body Armour")
assert.are.equals(104, item.armourData.Ward)

These tests pass for the wrong reason. The actual call sequence in ImportItem() is:

1. item.armourData.Ward = 104 set from API properties
2. item.wardFromPropertyLine remains false — never set to true in ImportTab.lua
3. BuildAndParseRaw() is called:
   • BuildRaw() does NOT emit "Ward: 104" because wardFromPropertyLine == false (Item.lua line 1308)
   • ParseRaw() resets wardFromPropertyLine = false but does not clear armourData
   • armourData.Ward = 104 survives as stale state in the un-cleared table
4. ImportItem() returns the item with the stale 104

The assertion passes because ParseRaw never zeros out armourData. But when the build frame update fires and BuildModList() runs:

-- wardIsAuthoritative = false (wardFromPropertyLine was never set)
wardBase = calcLocal(modList, "Ward", "BASE", 0) + (self.base.armour.Ward or 0)
-- = 0 + 0 = 0   (no Ward mods, Serpentscale Coat has no base Ward)
armourData.Ward = round(0 * ...) = 0   ← authoritative 104 is lost

The fix is missing: ImportTab.lua never sets item.wardFromPropertyLine = true. It should be set immediately after line 1264 when armourKey == "Ward":

-- ImportTab.lua, inside the armourData block
item.armourData[armourKey] = (item.armourData[armourKey] or 0) + tonumber(value[1])
if armourKey == "Ward" then
    item.wardFromPropertyLine = true   -- ← missing
end

Without this, BuildRaw() strips Ward from the serialised form, and the authoritative check in BuildModListForSlotNum is never triggered for API-imported items.

Test for "does not double-count" (import path) similarly passes on stale state

The rune-mod test (lines ~36-62, TestImportTab_spec.lua) also does not call BuildModList(). The runeModLines count assertion is valid, but the Ward value assertion has the same stale-state flaw.

Missing BuildModList() assertions in import tests

Both import tests should add:

item:BuildModList()
assert.are.equals(104, item.armourData.Ward)   -- must hold after recalc

Paste-path coverage: ADEQUATE

TestWard_spec.lua covers:

• Double-count regression (paste path) — line 171
• defencesInc excluded from authoritative Ward — line 203
• Non-authoritative Ward scaling — line 228
• BuildModList() is called in all three, correctly exercising the calculation path.

TestItemParse_spec.lua covers the BuildModList() round-trip at line 767:

item:BuildModList()
assert.are.equals(83, item.armourData.Ward)

This is correct.

---

7. Style

Comment block (Item.lua lines 1808-1813) exceeds one-line guidance

CLAUDE.md: "one short line max" for non-obvious comments. The 6-line block explains legitimate non-obvious invariants but should be trimmed:

-- wardIsAuthoritative: property-line Ward is game-final (quality+INC already baked in).
-- Consume local Ward mods to prevent double-application in CalcDefence.

Minor. Not a blocker.

escapeGGGString extraction to escapedPropertyName variable (ImportTab.lua)

Good — avoids repeated function calls and correctly canonicalises [Ward|Runic Ward] → "Runic Ward" → "Ward".

---

Required Changes

#	File	Issue	Severity
1	src/Classes/ImportTab.lua line 1264	Missing item.wardFromPropertyLine = true when armourKey == "Ward"	Critical
2	spec/System/TestImportTab_spec.lua lines ~26, ~53	Both Ward import tests must call item:BuildModList() and re-assert Ward == 104	Critical
3	src/Classes/Item.lua line 1819	wardBase = armourData.Ward → wardBase = armourData.Ward or 0	Medium

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
Now I have everything I need for a thorough review. Let me compile the findings.

---

Code Review

1. Calculation Correctness

CalcDefence.lua — operator precedence bug fix (line 3643)

The original:

output[damageType.."TotalHitPool"] = output[damageType.."TotalHitPool"] + output.Ward or 0

Lua evaluates this as (... + output.Ward) or 0. If output.Ward is nil, the + raises a runtime error rather than falling back to 0. The fix (output.Ward or 0) is correct.

Item.lua — authoritative ward path (BuildModListForSlotNum, ~line 1816)

The logic is sound:

• calcLocal(modList, "Ward", "BASE", 0) at line 1817 removes flat ward mods from the local modList (confirmed: calcLocal calls t_remove), so they cannot leak to the global modDB.
• calcLocal(modList, "Ward", "INC", 0) at line 1818 removes INC mods similarly.
• Line 1831: wardInc = wardIsAuthoritative and 0 or calcLocal(...) short-circuits to 0; calcLocal is not called a second time. No double-consume. ✓
• wardBase = armourData.Ward reads the original property-line value, not the post-computation overwritten value, because it executes before line 1842.
• Idempotency: after the first call, armourData.Ward = round(armourData.Ward) which equals armourData.Ward for integers, so repeated calls are safe.

WardPerLevel — minor concern (line 1847)

armourData.WardPerLevel = wardPerLevel * (1 + (wardInc + defencesInc) / 100) * (1 + (qualityScalar / 100))

In authoritative mode wardInc = 0, so the rune's "% increased Ward" INC (which was consumed at line 1818) no longer scales WardPerLevel. If the game applies rune INC to the per-level implicit component this would undercount. Not testable without live game data, but consistent with how the existing calcLocal architecture handles rune mods for other defence types (Evasion, ES). Flag for follow-up if WardPerLevel reports look wrong in practice.

2. Modifier Parsing

No ModParser.lua changes. The new escapeGGGString usage is in ImportTab.lua, which is not the parser. The [Ward|Runic Ward] → "Runic Ward" → "Ward" key mapping chain is verified:

escapeGGGString("[Ward|Runic Ward]")  -- pattern 3: %[[^|]+|([^|]+)%] → "Runic Ward"
"Runic Ward":gsub(" Rating",""):gsub(" ","")  → "RunicWard"
armourKey == "RunicWard" → armourKey = "Ward"           ✓

Paste path (ParseRaw ~line 600): specName == "Runic Ward" → specName = "Ward" is unchanged and correct. ✓

ModParser.lua is untouched; no new test cases are required.

3. Nil Safety

armourData.Ward in authoritative branch (Item.lua line 1819)

wardIsAuthoritative = self.wardFromPropertyLine, which is only true when the parsing code set self.armourData and assigned .Ward. This invariant holds in both the paste path (ParseRaw line 603-607) and the API import path (ImportTab.lua line 1260-1268). ✓

tonumber(value[1]) (ImportTab.lua line 1264)

Pre-existing code. For Ward property values from the GGG API the format is always a bare numeric string, so this is safe in practice. No regression introduced. ✓

property.name == "Radius" / "Limited to" (ImportTab.lua lines 1243-1245)

These two comparisons are still against the raw (un-escaped) property.name. This is correct since those names don't use the [key|value] API format, but it is a subtle inconsistency worth noting for future maintainers.

4. Data Integrity

No src/Data/ files changed. No data validation concerns.

5. Performance

No new table allocations inside BuildModListForSlotNum. calcLocal calls are O(n) over local modList which is small and bounded by item mod count. No hot-loop regressions.

The escapeGGGString call was already being done for other fields; caching it in escapedPropertyName is an improvement (removes up to 3 repeated calls per property in the old code).

6. Test Coverage

New test	Path covered	Gap
parses [Ward|Runic Ward] property	API import → armourData.Ward	✓
with a Ward rune mod without double-counting	API rune mods + runeModLines count	✓
does not crash for Mace Strike	Flask-slot no-crash smoke test	Only tests no-error; no assertion on returned item
Runeforged prefix stripped and parses Runic Ward	Paste path → armourData.Ward	✓
Has +N per player level parses (moved)	Per-level mod names	✓
Ward survives BuildModList recalculation	Idempotency	✓
does not double-count (paste path regression)	Flat rune mod consumed	✓
not re-scaled by local defencesInc	defencesInc excluded from Ward	✓
plain Ward mods scale correctly	Non-authoritative path	✓

Gap: No test covers the authoritative path + defencesInc interaction on WardPerLevel (the rune INC is consumed so WardPerLevel only scales with defencesInc and qualityScalar). Low priority, but worth a follow-up test when PoE2 per-level mechanics are better understood.

7. Style

• The return item addition to ImportTab.lua is backward-compatible (Lua discards extra return values). ✓
• Comment block at Item.lua lines 1808-1813 is 6 lines explaining a non-obvious cross-system invariant. This is at the edge of what CLAUDE.md permits ("WHY is non-obvious"), but the invariant genuinely is surprising. Acceptable.
• property.name is used directly (un-escaped) for "Radius" and "Limited to" checks while all other checks now use escapedPropertyName. The inconsistency is harmless but could confuse future editors — worth a one-line comment or consistent escaping.
• Test names are clear and descriptive. ✓

---

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
Now I have enough to write the full, structured review. Let me do it.

---

Code Review: Runeforged Ward Authoritative Value Fix

1. Calculation Correctness

src/Classes/Item.lua:1816–1844 — The authoritative-Ward path is mechanically sound: the property-line value is read directly (wardBase = armourData.Ward), local BASE and INC Ward mods are consumed from modList (lines 1817–1818) to prevent them from leaking into CalcDefence, wardInc is forced to 0 (line 1831), and defencesInc is not applied in the authoritative branch (line 1843). These mechanics correctly reflect the intent: the game has already baked quality, rune mods, and INC into the displayed number.

src/Classes/Item.lua:1847 — ISSUE: WardPerLevel silently applies defencesInc in the authoritative path. When wardIsAuthoritative = true, wardInc = 0 but defencesInc is still captured (line 1833) and applied:

armourData.WardPerLevel = wardPerLevel * (1 + (wardInc + defencesInc) / 100) * ...

For other per-level stats (EvasionPerLevel, EnergyShieldPerLevel) this is the same formula and presumably intentional, but for the authoritative Ward case it means a local "% increased Defences" mod on the item does scale WardPerLevel even though it does not scale Ward. No test exercises this combination (WardPerLevel + defencesInc + wardIsAuthoritative), so whether this asymmetry is intentional is unverifiable from the diff alone.

src/Modules/CalcDefence.lua:3643 — BUG FIX, correctly addressed. The old line:

output[...] = output[...] + output.Ward or 0

is (A + output.Ward) or 0 in Lua — output.Ward or 0 was never evaluated because + binds tighter than or. If output.Ward was nil this would raise an arithmetic error. The parenthesised form + (output.Ward or 0) is correct.

2. Modifier Parsing

No changes to src/Modules/ModParser.lua. The parsing of "Runic Ward" spec names (remapped to "Ward" at Item.lua:600–601 and "RunicWard" → "Ward" at ImportTab.lua:1262) is handled in item-level property parsing, not the mod parser. No new modifier strings were introduced that would require TestModParser.lua additions.

3. Nil Safety

src/Classes/Item.lua:1819 — wardBase = armourData.Ward. armourData is self.armourData aliased at line 1801. BuildModList initialises self.armourData = self.armourData or {} at line 1968 before calling BuildModListForSlotNum, so the table itself is never nil in this branch. armourData.Ward is populated before wardFromPropertyLine is set in both code paths:

• ParseRaw:603–606: self.armourData[specName] = specToNumber(specVal) precedes the flag.
• ImportTab:1260–1267: item.armourData[armourKey] = ... + tonumber(value[1]) precedes the flag.

This is safe.

src/Classes/ImportTab.lua:1225–1226 — Pre-existing early return not updated. The function returns nil (not item) when not item.base or not item.rarity. The new return item at line 1444 is unreachable on that path. The new tests at TestImportTab_spec.lua:23 and 49 call assert.is_not_nil(item), relying on the mock base name ("Runeforged Serpentscale Coat") resolving in the item-base table. If that base is absent in the test environment the assertion gives a misleading failure — the function silently returned nil rather than crashing. This is a latent test-fragility concern, not a production crash risk for valid GGG API data.

src/Classes/ImportTab.lua:1264 — tonumber(value[1]) can return nil if the API sends a non-numeric string. (item.armourData[armourKey] or 0) + nil raises an arithmetic error. This path existed before the PR and is not introduced by it, but the PR extends coverage to "Ward" / "Runic Ward" so it is now worth noting.

4. Data Integrity

No files under src/Data/ were modified. ✓

5. Performance

The authoritative path adds two extra calcLocal calls (lines 1817–1818) inside the already-expensive BuildModListForSlotNum. calcLocal iterates modList; for typical item mod counts this is negligible. No table allocations in the hot path. ✓

6. Test Coverage

New path	Covered by test?
Import path: wardFromPropertyLine set, Ward value preserved	TestImportTab_spec.lua:6 ✓
Import path: rune mods do not double-count	TestImportTab_spec.lua:29 ✓
Paste path: Ward survives BuildModList round-trip	TestItemParse_spec.lua:522, TestWard_spec.lua:171 ✓
Authoritative path: defencesInc excluded from Ward	TestWard_spec.lua:203 ✓
Non-authoritative path: INC + quality scale correctly	TestWard_spec.lua:228 ✓
WardPerLevel in authoritative path	Not tested ✗
round(wardBase) when armourData.Ward is 0	Not tested ✗
CalcDefence nil-Ward guard	Not directly unit-tested (integration only)

The WardPerLevel gap is the most relevant — a Runeforged item with Has +N to maximum Runic Ward per player level and wardIsAuthoritative = true has no test asserting the final WardPerLevel value.

7. Style

• Consistent with surrounding Lua conventions: local caching, calcLocal idioms, round() for integer stats.
• The inline comment block (lines 1808–1813) is the one place the WHY is genuinely non-obvious; the comment earns its presence.
• wardIsAuthoritative and 0 or calcLocal(...) (line 1831) is correct here because 0 is truthy in Lua (only nil/false are falsy), but it is worth making this explicit if the reviewer worries about the and/or ternary pattern — a brief comment noting "0 is truthy in Lua" would remove any doubt.
• The round(wardBase) on line 1843 is technically a no-op for integer Ward values from the API/paste (roundtripping through tonumber/specToNumber), but it is harmless and keeps the expression symmetric with the non-authoritative branch.

---

Summary of actionable issues:

1. Item.lua:1847 — WardPerLevel in the authoritative path applies defencesInc but not wardInc; this asymmetry versus the armourData.Ward authoritative treatment should be confirmed intentional and a test added.
2. TestImportTab_spec.lua:23,49 — assert.is_not_nil(item) is fragile if the base name is not in the test data; the assertion error message ("expected not nil, got nil") will not explain the real cause (unrecognised base → early return nil).
3. ImportTab.lua:1264 — tonumber(value[1]) without a nil-guard is pre-existing but worth noting now that Ward is explicitly routed through this accumulator.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
Now I have everything I need. Let me write the full review.

---

Code Review

1. Calculation Correctness

CalcDefence.lua – operator-precedence fix (line 3643)

The fix is correct. Lua's or has lower precedence than +, so the original:

… + output.Ward or 0

evaluated as (… + output.Ward) or 0. When output.Ward was nil that expression would throw a nil-arithmetic error rather than silently yielding 0. The parenthesised form (output.Ward or 0) is the right repair.

Adjacent unaddressed precedence bug — line 3648 has the same pattern:

local GuardAbsorbRate = output["sharedGuardAbsorbRate"] or 0 + output[damageType.."GuardAbsorbRate"] or 0

This evaluates as output["sharedGuardAbsorbRate"] or (0 + output[…]) or 0, which is wrong. Pre-existing, not introduced here, but it warrants a follow-up.

Item.lua authoritative-Ward formula (lines 1816–1844)

calcLocal is destructive (confirmed: t_remove(modList, i) at line 1684). The two "consume" calls in the authoritative branch correctly remove Ward BASE/INC mods from the local modList before those mods can be appended to baseList and reach CalcDefence a second time. This is the core correctness mechanism and it is sound.

The wardInc line (1831) short-circuits in the authoritative path, so no double-remove occurs for INC mods.

WardPerLevel scaling in authoritative path (line 1847)

armourData.WardPerLevel = wardPerLevel * (1 + (wardInc + defencesInc) / 100) * …

wardInc = 0 in the authoritative path, so defencesInc still scales WardPerLevel even though it is excluded from the property-line Ward value itself. This is mechanically correct — the per-level implicit is not baked into the property line — but no test exercises the authoritative path with defencesInc > 0 and a WardPerLevel implicit together. The test at TestWard_spec.lua:217 only covers defencesInc = 0. A test verifying that defencesInc does scale WardPerLevel (but not Ward) in the authoritative path would close this gap.

ImportTab.lua – escapeGGGString refactor (lines 1239–1268)

Correct. Previously Radius and Limited to were compared against the raw property.name while Quality used the escaped form — inconsistent, and silently broken for localised API payloads. Caching escapedPropertyName once is both a correctness fix and a minor performance win.

---

2. Modifier Parsing

ModParser.lua is not touched. The WardPerLevel modifier ("Has +N to maximum Runic Ward per player level") was apparently already handled — the moved test at TestItemParse_spec.lua:519 confirms this without requiring a parser change. No new modifier patterns are introduced, so no new TestModParser.lua cases are needed.

---

3. Nil Safety

BuildModListForSlotNum, line 1819

local armourData = self.armourData     -- line 1801, no "or {}" guard
…
wardBase = armourData.Ward             -- line 1819

armourData at line 1801 is self.armourData without a fallback. wardBase = armourData.Ward is only reached when wardIsAuthoritative = true, which is only set when armourData.Ward was explicitly assigned during parsing — so the nil case cannot occur in practice. However, a defensive self.armourData or {} at line 1801 already existed for the other stat reads implicitly (all lines 1802–1807 call calcLocal which does not touch armourData), so the crash would hit at line 1839 regardless. This is pre-existing, not introduced here.

ParseRaw – wardFromPropertyLine initialisation (line 305)

Initialised to false on every ParseRaw call. Round-trips through BuildRaw → ParseRaw are safe: BuildRaw only emits the Ward: line when wardFromPropertyLine = true (line 1308), so the flag is re-derived correctly on re-parse.

ImportItem – return item (line 1444)

Previously this function returned nothing; adding return item is backward-compatible (callers at lines 761, 950, 1451 discard the return). Required for the new tests and for callers that need the hydrated item object.

---

4. Data Integrity

No files under src/Data/ were modified. Not applicable.

---

5. Performance

No new allocations inside hot calculation loops. The escapedPropertyName local in ImportItem eliminates one repeated string call per property. The two extra calcLocal calls in the authoritative branch (BuildModListForSlotNum) are per-item, not per-calculation, and are negligible.

---

6. Test Coverage

Path	Covered?
API import – localized `[Ward	Runic Ward]property →armourData.Ward`
API import – rune mods present, no double-count	✅ TestImportTab_spec.lua:32
Paste path – Runic Ward: N property line parsed	✅ TestItemParse_spec.lua:519
Paste path – Ward survives BuildModList() round-trip	✅ TestItemParse_spec.lua:739
Authoritative Ward not re-scaled by defencesInc	✅ TestWard_spec.lua:206
Authoritative Ward not re-scaled by rune INC+flat	✅ TestWard_spec.lua:171
Non-authoritative Ward path	✅ TestWard_spec.lua:249
WardPerLevel authoritative (quality scales, wardInc consumed)	✅ TestWard_spec.lua:217
WardPerLevel with defencesInc > 0 in authoritative path	❌ Not tested
CalcDefence – TotalHitPool + Ward precedence fix	❌ No new test; pre-existing coverage is indirect at best
GuardAbsorbRate adjacent precedence bug (line 3648)	❌ Pre-existing, no test
Crash guard for non-armour item in ImportItem	✅ TestImportTab_spec.lua:56

The two missing tests are low risk but should be noted.

---

7. Style

• escapedPropertyName variable name is clear and consistent with surrounding style.
• wardFromPropertyLine flag name is unambiguous.
• The multi-line comment block at lines 1808–1813 is longer than typical PoB comments, but the non-obvious invariant being encoded (game-final vs. re-computed) justifies it.
• The ternary-style multi-line assignment at lines 1842–1844 matches precedents in the same function.
• One style nit: the test label renamed at TestItemParse_spec.lua:508 ("Runeforged prefix is stripped from base name and item parses Runic Ward") is fine, but the TestAdvancedItemParse block at line 739 is now testing a completely different item than its original. The description was updated, but the test now acts as a compound regression for both the Runeforged base stripping AND the per-level mod parsing, which is slightly confusing. Consider splitting into two tests.

---

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
No action-required findings. The authoritative path is sound: wardFromPropertyLine is only ever set alongside armourData.Ward, so wardBase = armourData.Ward at Item.lua:1819 is never a nil read. The wardInc double-consume risk doesn't exist — the authoritative if block consumes INC (line 1818), and the ternary at line 1831 short-circuits before calling calcLocal again. The defencesInc intentionally reaches WardPerLevel but not authoritative Ward, exactly as the tests assert. The return item addition is safe; all existing callers at ImportTab.lua:761, 950, and 1451 discard the return value.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
runeMods parsing into runeModLines is already implemented at ImportTab.lua:1323 — that test assertion is valid. The spec normalization at Item.lua:600–601 converts "Runic Ward" → "Ward" before the new wardFromPropertyLine check at 605, so the flag is correctly set on the paste path.

One genuine finding:

spec/System/TestImportTab_spec.lua (lines ~41–43) — unguarded item.armourData access
assert.is_not_nil(item) guards against a nil item, but assert.are.equals(104, item.armourData.Ward) is evaluated in the argument list before the guard can act — if item.armourData is nil (e.g., the base item wasn't found and Ward parsing skipped), the test crashes with a raw Lua nil-index error instead of a clean assertion failure. Add an intermediate assert.is_not_nil(item.armourData) between the two assertions.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

Verdict: COMMENT. See review comment above.

[github-actions]

🔄 Cycle 1, attempt 1/3
spec/System/TestImportTab_spec.lua:57 — assert.are.equals(2, #item.runeModLines): the mock runeMods table contains a single string with two mods joined by \n; if ImportItem iterates runeMods with ipairs and adds each entry as one line (without splitting on \n), this assertion yields 1, not 2, and the test silently proves nothing about double-counting.

src/Classes/Item.lua:1820 — wardBase = armourData.Ward reads the local armourData table before any write in this function; correctness depends entirely on armourData being the same object as self.armourData (populated by ParseRaw) rather than a freshly allocated table. The new test covers this path, so a failing run would reveal the problem, but the assumption deserves an explicit comment so future refactors don't break the invariant silently.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

Verdict: COMMENT. See review comment above.

[github-actions]

🔄 Cycle 1, attempt 1/3
CalcDefence.lua:3650,3652 — GuardAbsorbRate and GuardAbsorb precedence bugs are fixed (shared + per-damage-type rates were never summed when sharedGuardAbsorbRate was truthy), but no new tests exercise buildDefenceEstimations with an active guard skill — builds using guard skills had silently wrong TotalHitPool values and the fix has no regression guard.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

Verdict: COMMENT. See review comment above.

[github-actions]

🔄 Cycle 1, attempt 1/3
CalcDefence.lua:2929 confirms output[damageType.."GuardAbsorb"] = calcLib.val(modDB, damageType.."GuardAbsorbLimit") — the test mod name PhysicalGuardAbsorbLimit correctly flows into output["PhysicalGuardAbsorb"], so the regression guard is valid.

No findings requiring action or discussion.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
Now I have the complete picture from reading BuildModListForSlotNum.

Finding 1 — WardPerLevel underscaled in authoritative path (Item.lua:1814–1847)

calcLocal(modList, "Ward", "INC", 0) at line 1814 removes the rune Ward-INC mods from modList (confirmed: calcLocal calls t_remove at line 1684). The consumed value is then silently discarded. wardInc is forced to 0 at line 1828, so:

armourData.WardPerLevel = wardPerLevel * (1 + (0 + defencesInc) / 100) * quality_scalar

In-game, WardPerLevel implicits (+1 to maximum Runic Ward per player level) scale with local Ward INC modifiers including rune mods. Discarding wardInc underscales WardPerLevel for any Runeforged item that carries a rune Ward-INC mod. The consumed value from line 1814 should be saved and passed through to WardPerLevel. spec/System/TestWard_spec.lua:207–215 documents this as intentional but is asserting incorrect game math.

Finding 2 — Missing build-level integration test

Every new Ward test in TestWard_spec.lua and TestImportTab_spec.lua asserts item.armourData.Ward directly. No test equips a Runeforged item in a full build and checks calcsTab.calcsOutput.Ward, leaving the CalcDefence → armourData.Ward path unexercised by this PR.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

Verdict: REQUEST_CHANGES. See review comment above.

[github-actions]

🔄 Cycle 1, attempt 1/3
Based on the graph and direct analysis of the diff — no source code bugs found.

The three CalcDefence.lua precedence fixes are mechanically correct; the test at spec/System/TestDefence_spec.lua:600-617 matches the established NewMod-then-runCallback pattern already present in the file (mods added directly to configTab.modList survive runCallback because initEnv re-adds the full mod list via AddList, not by rebuilding from customMods).

The authoritative Ward path in Item.lua correctly consumes both Ward BASE and Ward INC from modList once each, regardless of branch, with no double-calls to calcLocal. defencesInc is properly excluded from authoritative Ward while still applied to WardPerLevel. The wardFromPropertyLine flag survives BuildRaw→ParseRaw round-trips correctly because BuildRaw only emits the Ward property line when the flag is set, and ParseRaw re-sets the flag when it sees that line.

The escapeGGGString refactor in ImportTab.lua is applied consistently; return item is the only behavioral change to the function signature and callers that ignore the return value are unaffected.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
spec/System/TestImportTab_spec.lua:45,55 — After assert.is_not_nil(item, ...) fails, busted continues to the next assertion and item.armourData raises a nil-index error rather than a clean test failure, masking the root cause.

src/Classes/Item.lua:1801 — Comment "Ward read at line 1819 relies on this" is stale after the refactor; the authoritative Ward read now happens inside the if wardIsAuthoritative block above that line.

✅ Agent succeeded (cycle 1, attempt 1)

[github-actions]

🔄 Cycle 1, attempt 1/3
ImportTab.lua:1325 confirms runeModLines splits on \n (for line in line:gmatch("[^\n]+")), so the assert.are.equals(2, ...) in the test is correct.

CalcDefence.lua:2929 confirms output[damageType.."GuardAbsorb"] = calcLib.val(modDB, damageType.."GuardAbsorbLimit"), so NewMod("PhysicalGuardAbsorbLimit", "BASE", 300) in the defence test correctly exercises the fix.

No actionable findings.

✅ Agent succeeded (cycle 1, attempt 1)