# mUIFramework Rectification 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:** Eliminate the systemic design flaws in mUIFramework — property-system bypass, doc/code class-API mismatch, scattered re-emit hacks, per-mutation relayout cost — so the framework's central signal/property contract is trustworthy and consistent.

**Architecture:** Make `setProperty` the single mutation path with an optional silent flag and a signal-layer re-entrancy guard; add the documented `extend()`/`super` class API as the canonical surface (keeping `Class(name,parent)` as an alias); centralize the "hidden frame missed OnSizeChanged" workaround into one `Widget` revalidation path; add deferred/coalesced relayout to `Layout`. Then audit consumer addons (mCleanup) for the now-emitting setters.

**Tech Stack:** Lua 5.1 (WoW retail client), Blizzard FrameXML API, LibStub. No build step. Verification is in-game: `/reload` and visual check against `mCleanup`'s Options panel and (where present) `testing/addon.lua`.

**Verification convention:** This framework has no headless test harness. Every task that changes behavior ends with an explicit in-game check: load WoW, `/reload`, open the named UI, and confirm the stated observable result. A task is not complete until the check passes. If a check fails, STOP and report — do not proceed to the next task.

**Pre-flight (do once before Task 1):**
- Confirm the framework path: `mCleanup/libs/mUI_framework/`.
- Confirm consumer addons that embed it: `mCleanup` (primary), and any other addon whose `libs/` contains `mUI_framework`. Run a search for `mCleanupUIFramework-1.0` across `Interface/AddOns` to enumerate them. This plan fixes `mCleanup` consumers explicitly; if other embedders exist, flag them — they need the same Task 9 audit.
- There is no git repo here. Commit steps below assume the user initializes one or applies changes directly; if no repo exists, treat each "Commit" step as "checkpoint: stop and let the user `/reload`-verify before continuing."

---

## File Structure

| File | Change | Responsibility after change |
|---|---|---|
| `core/Object.lua` | Modify | `setProperty` gains a silent option; property mutation is the only sanctioned write path |
| `core/Signal.lua` | Modify | `emit` gains re-entrancy guard so a slot that triggers the same signal does not recurse infinitely |
| `core/Class.lua` | Modify | Adds `Class:extend()` and `.super`; `Class(name,parent)` kept as alias; single inheritance source of truth |
| `widgets/Widget.lua` | Modify | Adds `_revalidateLayout()` — the one place the "hidden frame missed resize" workaround lives |
| `widgets/Slider.lua` | Modify | `setValue` routes through `setProperty`; emits `valueChanged` |
| `widgets/Spinner.lua` | Modify | `setValue` routes through `setProperty`; emits `valueChanged` |
| `widgets/SliderControl.lua` | Modify | `setValue` routes through `setProperty`; emits `valueChanged` |
| `widgets/StatusBar.lua` | Modify | `setValue` routes through `setProperty` (already emits — make path consistent) |
| `widgets/TextInput.lua` | Modify | `setText` and `OnTextChanged` route through `setProperty` |
| `widgets/Dropdown.lua` | Modify | `setSelected` and row-click route through `setProperty`; emit on both |
| `layouts/Layouts.lua` | Modify | `Layout` gains deferred coalesced relayout; `TabLayout`/`Window` re-emit hacks replaced with `_revalidateLayout()` |
| `widgets/Window.lua` | Modify | `loadState` uses `_revalidateLayout()` instead of manual double `emit("resized")` |
| `mCleanup/core/Options.lua` | Modify | Refresh routines guarded so now-emitting setters don't cause save-back loops |
| `mCleanup/core/CleanupManager.lua` | Modify | Same guard for `setSelected`/`setValue` calls in reset/refresh paths |
| `CLAUDE.md` | Modify | Update class-system docs, remove resolved "Known Limitations" rows |

---

## Task 1: Add re-entrancy guard to Signal:emit

**Files:**
- Modify: `core/Signal.lua:55-88`

A slot that calls `emit()` on the same signal it is handling (which becomes possible once `setProperty` always emits) must not recurse. Guard at the signal layer so every widget benefits without per-widget code.

- [ ] **Step 1: Add the guard to `Signal:emit`**

Replace the body of `Signal:emit` (currently lines 55-88) with:

```lua
function Signal:emit(...)
	-- Re-entrancy guard: if a slot triggers this same signal, drop the
	-- nested emit rather than recursing. The outer emit is authoritative.
	if self._emitting then return end
	self._emitting = true

	-- Snapshot so slots disconnecting mid-emit don't skip neighbors.
	local snapshot = {}
	for i = 1, #self.slots do snapshot[i] = self.slots[i] end

	local dead = {}
	for i = 1, #snapshot do
		local s = snapshot[i]
		if s.fn then
			s.fn(...)
		else
			local target = s.weakTarget[1]
			if target == nil then
				dead[s] = true
			else
				local m = s.method
				if type(m) == "string" then m = target[m] end
				m(target, ...)
			end
		end
	end

	if next(dead) then
		local live = {}
		for i = 1, #self.slots do
			if not dead[self.slots[i]] then
				live[#live + 1] = self.slots[i]
			end
		end
		self.slots = live
	end

	self._emitting = false
end
```

- [ ] **Step 2: Update the file header comment**

In the comment block at the top of `core/Signal.lua`, after the line describing `connect()` returning a handle, add:

```lua
-- emit() is re-entrant-safe: a slot that emits the same signal again is a
-- no-op for the nested call, preventing infinite recursion when a property
-- change handler writes back to the same property.
```

- [ ] **Step 3: In-game verify**

Load WoW, `/reload`. Open mCleanup's main window. Confirm it opens with no Lua error. (No behavior change is visible yet — this is a safety net for later tasks. A `!BugGrabber`/error popup means a regression.)

- [ ] **Step 4: Commit**

```bash
git add core/Signal.lua
git commit -m "fix(signal): add re-entrancy guard to emit()"
```

---

## Task 2: Add silent option to setProperty

**Files:**
- Modify: `core/Object.lua:65-70`

`setProperty` becomes the only sanctioned property write. A `silent` flag lets internal code update a value without emitting — replacing the current practice of writing `self._properties.x` directly.

- [ ] **Step 1: Replace `Object:setProperty`**

Replace `Object:setProperty` (currently lines 65-70) with:

```lua
-- setProperty is the ONLY sanctioned way to mutate a property.
-- Never assign self._properties[name] directly from widget code.
-- opts.silent = true updates the stored value without emitting
-- <name>Changed — use it only to sync state that another signal
-- already accounts for (e.g. echoing a Blizzard control's own event).
function Object:setProperty(name, value, opts)
	local old = self._properties[name]
	if old == value then return end
	self._properties[name] = value
	if not (opts and opts.silent) then
		self:emit(name .. "Changed", value, old)
	end
end
```

- [ ] **Step 2: Update the file header comment**

In the `Properties` section of the comment block at the top of `core/Object.lua`, replace the `obj:setProperty` line with:

```lua
--   obj:setProperty(name, value[, {silent=true}])
--                                       -- sets; emits <name>Changed unless silent
```

- [ ] **Step 3: In-game verify**

`/reload`. Open mCleanup's Options panel. Toggle a checkbox — it still works (Checkbox already uses `setProperty`). No Lua error.

- [ ] **Step 4: Commit**

```bash
git add core/Object.lua
git commit -m "feat(object): add silent option to setProperty as single mutation path"
```

---

## Task 3: Route Slider through setProperty

**Files:**
- Modify: `widgets/Slider.lua:69-95`

`Slider` currently writes `self._properties.value` directly in two places and never emits on `setValue`. After this task `setValue` emits `valueChanged` like every other property.

- [ ] **Step 1: Rewrite the OnValueChanged handler and setValue**

In `Slider:init`, replace the `OnValueChanged` script (currently lines 75-83) with:

```lua
	sl:SetScript("OnValueChanged", function(_, v)
		if this._updating then return end
		v = math.floor((v - minVal) / step + 0.5) * step + minVal
		v = math.max(minVal, math.min(maxVal, v))
		if this:getProperty("value") == v then return end
		valLbl:SetText(tostring(v))
		this:setProperty("value", v)   -- emits valueChanged
	end)
```

Replace `Slider:setValue` and `Slider:value` (currently lines 87-95) with:

```lua
-- Programmatic update. Emits valueChanged (consistent with all widgets).
-- The _updating guard prevents the Blizzard slider's own OnValueChanged
-- from emitting a second, duplicate signal.
function Slider:setValue(v)
	v = math.max(self._min, math.min(self._max, v))
	if self:getProperty("value") == v then return end
	self._valLbl:SetText(tostring(v))
	self._updating = true
	self._slider:SetValue(v)
	self._updating = false
	self:setProperty("value", v)
end
function Slider:value() return self:getProperty("value") end
```

- [ ] **Step 2: Update the file header comment**

In the comment block at the top of `widgets/Slider.lua`, replace the `valueChanged` signal line with:

```lua
--   valueChanged(value)   -- fires on user drag AND on setValue()
```

- [ ] **Step 3: In-game verify**

`/reload`. Open mCleanup Options. The min item-level slider (`_ilvlSC` area) and any plain slider: drag it — value label updates and any connected handler runs. This is correct. (Task 9 fixes any save-back loop this introduces; if Options shows a visible glitch now, note it and continue — Task 9 addresses it.)

- [ ] **Step 4: Commit**

```bash
git add widgets/Slider.lua
git commit -m "fix(slider): route setValue through setProperty so it emits valueChanged"
```

---

## Task 4: Route Spinner and SliderControl through setProperty

**Files:**
- Modify: `widgets/Spinner.lua:69-104`
- Modify: `widgets/SliderControl.lua:121-160`

Both widgets have the same direct-`_properties` bypass as Slider. Apply the identical pattern.

- [ ] **Step 1: Fix Spinner's commit path and setValue**

Open `widgets/Spinner.lua`. The internal `commit(v)` closure (around line 69-70) currently does `this._properties.value = v`. Replace that assignment with a `setProperty` call. The commit closure body's value-write line:

```lua
		this:setProperty("value", v)   -- emits valueChanged
```

Replace `Spinner:setValue` (around line 101-102) so it routes through `setProperty`:

```lua
function Spinner:setValue(v)
	v = math.max(self._min, math.min(self._max, v))
	if self:getProperty("value") == v then return end
	self._eb:SetText(tostring(v))
	self:setProperty("value", v)
end
```

> NOTE: confirm the editbox field name. If `Spinner` stores its editbox as `self._eb`, use it; if it is a local `eb` only, expose it as `self._eb` in `init` first, then use `self._eb` here. Read the file before editing to get the exact name.

`Spinner:value` should return `self:getProperty("value")`.

- [ ] **Step 2: Fix SliderControl's setVal closure and setValue**

Open `widgets/SliderControl.lua`. The `setVal` closure (around line 121-122) does `this._properties.value = v`. Replace with:

```lua
		this:setProperty("value", v)   -- emits valueChanged
```

Replace `SliderControl:setValue` (around line 153-155) so the direct write becomes:

```lua
function SliderControl:setValue(v)
	v = math.max(self._min, math.min(self._max, v))
	if self:getProperty("value") == v then return end
	-- _updating guards the inner Blizzard slider's echo, same as mSlider
	self._updating = true
	self._slider:SetValue(v)
	self._updating = false
	self:setProperty("value", v)
end
```

> NOTE: read `SliderControl.lua` first to confirm the inner-slider field name (`self._slider`) and whether an `_updating` guard already exists; if it does not, add `self._updating = false` in `init` and guard the inner slider's `OnValueChanged` with `if this._updating then return end`.

`SliderControl:value` should return `self:getProperty("value")`.

- [ ] **Step 3: Update both file header comments**

In `Spinner.lua` and `SliderControl.lua` header comments, change any "does NOT emit" note on `setValue` to:

```lua
--   valueChanged(value)   -- fires on user interaction AND on setValue()
```

- [ ] **Step 4: In-game verify**

`/reload`. Open mCleanup Options. The font-size SliderControl (`_fontSizeSC`) and item-level SliderControl (`_ilvlSC`): click the +/- buttons and drag — value updates, handler runs, no Lua error.

- [ ] **Step 5: Commit**

```bash
git add widgets/Spinner.lua widgets/SliderControl.lua
git commit -m "fix(spinner,slidercontrol): route setValue through setProperty"
```

---

## Task 5: Route StatusBar and TextInput through setProperty

**Files:**
- Modify: `widgets/StatusBar.lua:67-81`
- Modify: `widgets/TextInput.lua:48-70`

`StatusBar` already emits but writes `_properties` directly; make it consistent. `TextInput` bypasses the property system in three places (`OnTextChanged`, the `opts.text` init, `setText`).

- [ ] **Step 1: Fix StatusBar:setValue**

In `widgets/StatusBar.lua`, replace `StatusBar:setValue` (around line 65-67, the `self._properties.value = v` line) so it routes through `setProperty`:

```lua
function StatusBar:setValue(v)
	v = math.max(self._min, math.min(self._max, v))
	if self:getProperty("value") == v then return end
	self:setProperty("value", v)
	self:_updateLabel(v)
end
```

> NOTE: read `StatusBar.lua` first. If `setValue` already emits via a manual `emit("valueChanged")`, remove that line — `setProperty` now emits it. `StatusBar:value` should return `self:getProperty("value")`.

- [ ] **Step 2: Fix TextInput's OnTextChanged**

In `widgets/TextInput.lua`, replace the `OnTextChanged` script (lines 48-52) with:

```lua
	eb:SetScript("OnTextChanged", function(_, userInput)
		if not userInput then return end
		-- silent: the user already sees the change; we emit textChanged
		-- explicitly with the live value rather than via the property path
		-- so getProperty("text") and the signal arg never disagree.
		this:setProperty("text", eb:GetText(), { silent = true })
		this:emit("textChanged", eb:GetText())
	end)
```

- [ ] **Step 3: Fix TextInput's opts.text init and setText**

Replace line 62 (`if opts.text then eb:SetText(opts.text); this._properties.text = opts.text end`) with:

```lua
	if opts.text then eb:SetText(opts.text); this:setProperty("text", opts.text, { silent = true }) end
```

Replace `TextInput:setText` (lines 67-70) with:

```lua
function TextInput:setText(t)
	t = t or ""
	self._eb:SetText(t)
	-- silent: setText is programmatic; emit textChanged explicitly so
	-- consumers that listen still hear it, but the value is already stored.
	self:setProperty("text", t, { silent = true })
	self:emit("textChanged", t)
end
```

- [ ] **Step 4: Update file header comments**

In `TextInput.lua` header, the `textChanged` line should now read:

```lua
--   textChanged(text)       -- every keystroke AND on setText(); arg is authoritative
```

The framework-level CLAUDE.md "known issue" about `getProperty("text")` lagging is resolved by this task — it is removed in Task 11.

- [ ] **Step 5: In-game verify**

`/reload`. Open mCleanup Options. In the add-input field (`addInput`) type text — the field updates. Trigger a `setText("")` path (e.g. the clear/reset action around `Options.lua:589`) — field clears. Any connected `textChanged` listener fires. No Lua error. Confirm a `DestroyPanel` progress bar still fills (it uses `StatusBar:setValue`).

- [ ] **Step 6: Commit**

```bash
git add widgets/StatusBar.lua widgets/TextInput.lua
git commit -m "fix(statusbar,textinput): route property writes through setProperty"
```

---

## Task 6: Route Dropdown through setProperty

**Files:**
- Modify: `widgets/Dropdown.lua:173-184` and `:248-255`

`Dropdown` writes `_properties.selectedValue` directly in `setSelected` and in the row-click handler, and only the row click emits `selectionChanged`. After this task `setSelected` emits too.

- [ ] **Step 1: Fix Dropdown:setSelected**

Replace `Dropdown:setSelected` (lines 173-184) with:

```lua
function Dropdown:setSelected(value)
	for _, it in ipairs(self._items) do
		if it.value == value then
			self._selValue = value
			self._selText  = it.text
			self._lbl:SetText(it.text)
			-- emits selectionChanged via the property path
			self:setProperty("selectedValue", value)
			return true
		end
	end
	return false
end
```

> NOTE: `setProperty` emits `selectedValueChanged`, not `selectionChanged`. Read the rest of `Dropdown.lua` to confirm which signal name consumers connect to. The documented public signal is `selectionChanged`. To keep that contract, after the `setProperty` line also emit the public signal explicitly:
> ```lua
> 			self:setProperty("selectedValue", value)
> 			self:emit("selectionChanged", value, it.text)
> ```
> The re-entrancy guard from Task 1 makes the double-path safe.

- [ ] **Step 2: Fix the row-click handler**

In `Dropdown:_populate`, the per-row `OnClick` (lines 248-255) does `dd._properties.selectedValue = item.value` then `dd:emit("selectionChanged", ...)`. Replace the `_properties` write with a silent `setProperty` so stored state stays consistent without a duplicate emit:

```lua
		btn:SetScript("OnClick", function()
			dd._selValue = item.value
			dd._selText  = item.text
			dd._lbl:SetText(item.text)
			dd:setProperty("selectedValue", item.value, { silent = true })
			dd:emit("selectionChanged", item.value, item.text)
			popup:Hide(); lib._dropdownBlocker:Hide(); popup._owner = nil
		end)
```

- [ ] **Step 3: Update file header comment**

In `Dropdown.lua` header, the `selectionChanged` note should read:

```lua
--   selectionChanged(value, text)  -- fires on user pick AND on setSelected()
```

- [ ] **Step 4: In-game verify**

`/reload`. Open mCleanup Options. The font dropdown (`fontDD`) and legacy-cutoff dropdown (`legacyDD`): pick an item — selection updates, handler runs. Then trigger a programmatic `setSelected` path (the Options refresh around `Options.lua:386` / `502`). No Lua error. (Save-back loop, if any, is handled in Task 9.)

- [ ] **Step 5: Commit**

```bash
git add widgets/Dropdown.lua
git commit -m "fix(dropdown): route setSelected through setProperty so it emits"
```

---

## Task 7: Add extend()/super to the class system

**Files:**
- Modify: `core/Class.lua:19-53`

CLAUDE.md documents `lib.Widget:extend()` and `MyWidget.super.init(...)`, neither of which exists. Add them as the canonical API; keep `Class(name,parent)` as an alias so all existing callers (every widget file) keep working.

- [ ] **Step 1: Rewrite Class.lua**

Replace the body of `core/Class.lua` from `local function Class(name, parent)` through the end with:

```lua
local function Class(name, parent)
	assert(type(name) == "string", "Class name must be a string")

	local klass = {}
	klass.__index = klass
	klass.__classname = name
	klass.__parent = parent

	if parent then
		setmetatable(klass, { __index = parent })
		-- .super gives subclass methods access to the parent implementation:
		--   function Foo:init(...) Foo.super.init(self, ...) end
		klass.super = parent
	end

	-- Instantiate. Calls :init(...) if defined anywhere on the chain.
	function klass:new(...)
		local obj = setmetatable({}, self)
		obj.__class = self
		if obj.init then obj:init(...) end
		return obj
	end

	-- Create a subclass. Canonical, Qt/JS-style API:
	--   local MyWidget = lib.Widget:extend("mMyWidget")
	-- The name argument is optional; defaults to "<parent>Subclass".
	function klass:extend(subName)
		return Class(subName or (name .. "Subclass"), self)
	end

	-- Runtime type check: walks up the class chain.
	function klass:isA(otherClass)
		local c = rawget(self, "__class") or self
		while c do
			if c == otherClass then return true end
			c = c.__parent
		end
		return false
	end

	lib.classes[name] = klass
	return klass
end

lib.Class = Class
```

- [ ] **Step 2: Update the file header comment**

Replace the `Usage:` block at the top of `core/Class.lua` with:

```lua
-- Usage (canonical):
--   local mUI = LibStub("mCleanupUIFramework-1.0")
--   local Foo = mUI.Class("Foo")            -- base class
--   local Bar = Foo:extend("Bar")           -- derived class
--
--   function Foo:init(x) self.x = x end
--   function Bar:init(x) Bar.super.init(self, x) end  -- call up the chain
--   local b = Bar:new(42)
--   b:isA(Foo) -- true
--
-- mUI.Class("Bar", Foo) is still accepted as an equivalent alias.
```

- [ ] **Step 3: In-game verify**

`/reload`. Open mCleanup's main window and Options panel — every widget is built via `lib.Class(name, parent)`, so the alias path must still work. No Lua error. Then, as a positive check, the test in Step 4.

- [ ] **Step 4: Smoke-test extend()/super in the test UI**

If `mCleanup` (or another embedder) has a `testing/addon.lua`, add a temporary block that exercises the new API; otherwise add it to mCleanup's main `addon.lua` behind a debug print and remove it after verifying:

```lua
-- TEMP smoke test — remove after verifying extend()/super
local mUI = LibStub("mCleanupUIFramework-1.0")
local TestW = mUI.Widget:extend("mTestExtendWidget")
function TestW:init(parent)
	TestW.super.init(self, parent, "Frame")
	self._marker = "extended-ok"
end
local tw = TestW:new()
assert(tw:isA(mUI.Widget), "extend(): isA(Widget) failed")
assert(tw._marker == "extended-ok", "extend(): super.init not reached")
print("mUIFramework: extend()/super smoke test PASSED")
tw:destroy()
```

`/reload` and confirm the chat prints `extend()/super smoke test PASSED` with no error. Then delete the temp block.

- [ ] **Step 5: Commit**

```bash
git add core/Class.lua
git commit -m "feat(class): add extend() and super to match documented API"
```

---

## Task 8: Centralize the hidden-frame relayout workaround

**Files:**
- Modify: `widgets/Widget.lua:1-61`
- Modify: `layouts/Layouts.lua:479-499` (TabLayout:setCurrentTab)
- Modify: `widgets/Window.lua:485-493` (loadState)

The "WoW doesn't fire OnSizeChanged for hidden frames" workaround is hand-coded in `TabLayout:setCurrentTab` and twice in `Window:loadState`. Replace all three with one `Widget:_revalidateLayout()`.

- [ ] **Step 1: Add `_revalidateLayout` to Widget**

In `widgets/Widget.lua`, after `Widget:setPoint` and before `Widget:destroy`, add:

```lua
-- Force every layout bound to this widget (and nested widgets) to recompute.
-- WoW does not fire OnSizeChanged for frames that were resized while hidden,
-- so any code that shows a previously-hidden widget, or restores a saved
-- window size, must call this once afterward. Centralizing it here means the
-- workaround exists in exactly one place.
function Widget:_revalidateLayout()
	local w, h = self.frame:GetSize()
	if w and h and w > 0 and h > 0 then
		self:emit("resized", w, h)
	end
end
```

- [ ] **Step 2: Update Widget file header comment**

In the comment block at the top of `widgets/Widget.lua`, after the paragraph about frame script hooks, add:

```lua
-- _revalidateLayout() re-emits "resized" so layouts re-tile; call it after
-- showing a widget that was resized while hidden (see TabLayout, Window).
```

- [ ] **Step 3: Replace the TabLayout re-emit**

In `layouts/Layouts.lua`, `Tab:setCurrentTab`, replace the manual re-emit block (currently lines 493-496, the comment plus the `local w, h = ...` and `tab.widget:emit("resized", w, h)`) with:

```lua
	-- While hidden, OnSizeChanged may not have fired for window resizes.
	-- Revalidate so every layout nested inside this tab reflows now.
	tab.widget:_revalidateLayout()
```

- [ ] **Step 4: Replace the Window:loadState re-emit**

In `widgets/Window.lua`, `Window:loadState`, replace the trailing block (currently lines 485-493, the comment plus `local fw, fh = ...` and the two `emit("resized", ...)` calls) with:

```lua
	-- Force layouts to re-flow against the now-correct size. Layouts that
	-- read GetWidth() at construction can latch onto the default size before
	-- loadState ran; this revalidation un-sticks them.
	self:_revalidateLayout()
	if self._content then self._content:_revalidateLayout() end
```

- [ ] **Step 5: In-game verify**

`/reload`. Open mCleanup's main window. If it uses a TabLayout, click between tabs — child widgets in the newly-shown tab are correctly sized (not collapsed/zero-width). Close and reopen the window, or `/reload` again, so `loadState` runs — the window restores at its saved size with correctly-tiled content. No Lua error.

- [ ] **Step 6: Commit**

```bash
git add widgets/Widget.lua layouts/Layouts.lua widgets/Window.lua
git commit -m "refactor: centralize hidden-frame relayout workaround in Widget:_revalidateLayout"
```

---

## Task 9: Audit and fix mCleanup consumers for now-emitting setters

**Files:**
- Modify: `mCleanup/core/Options.lua` (refresh routine around lines 380-590, 760-781)
- Modify: `mCleanup/core/CleanupManager.lua` (reset/refresh around lines 1167-1192, 1299, 1556-1565)

`Options.lua`'s refresh routine and `CleanupManager.lua`'s reset paths call `setSelected`/`setValue`/`setChecked` to push saved state *into* widgets. Before this plan those calls were silent. Now they emit — and the widgets' change handlers write back to SavedVariables, causing a redundant (and potentially looping) save during what should be a read-only refresh.

- [ ] **Step 1: Read the affected mCleanup routines**

Read `mCleanup/core/Options.lua` lines 360-600 and 750-790, and `mCleanup/core/CleanupManager.lua` lines 1150-1320 and 1540-1580. Identify each refresh/reset function that calls `setSelected`/`setValue`/`setChecked`/`setText` to populate widgets from saved state. List them before editing.

- [ ] **Step 2: Add a refresh guard flag**

In `Options.lua`, near the top of the options module table (e.g. `OptionsUI`), add a guard field and a helper. If the module already has a table like `OptionsUI`, add:

```lua
-- True while we are pushing saved state INTO widgets. Widget change
-- handlers check this and skip writing back to SavedVariables, so a
-- refresh does not trigger a redundant save (or a save/refresh loop).
OptionsUI._refreshing = false
```

- [ ] **Step 3: Wrap each refresh routine with the guard**

For every refresh/reset function identified in Step 1, wrap the body that calls the setters:

```lua
function OptionsUI:refresh()           -- or whatever the real name is
	self._refreshing = true
	-- ... existing setChecked / setSelected / setValue calls ...
	self._refreshing = false
end
```

Do the same for `CleanupManager.lua`'s reset routine (the `_sourceDD:setSelected("all")` / `_ilvlSC:setValue(0)` block around lines 1556-1565) — wrap it with whatever module-local guard is idiomatic there (add a `CleanupManager._refreshing` flag mirroring the Options one).

- [ ] **Step 4: Make each widget change handler respect the guard**

For every `connect("checkedChanged"|"selectionChanged"|"valueChanged"|"textChanged", ...)` handler in `Options.lua` and `CleanupManager.lua` that writes to SavedVariables, add an early return at the top of the handler:

```lua
	cb:connect("checkedChanged", function(on)
		if OptionsUI._refreshing then return end   -- ignore refresh-driven echoes
		db().enabled = on
		-- ... rest of handler ...
	end)
```

Apply the same `if <module>._refreshing then return end` guard to each handler. Read each handler first — only the ones that persist state or trigger side effects need the guard; pure display handlers do not.

- [ ] **Step 5: In-game verify**

`/reload`. Open mCleanup Options. Confirm:
- The panel opens showing the *saved* values (refresh worked).
- Changing a control by hand still persists (toggle a checkbox, `/reload`, value sticks).
- Opening Options does NOT mutate saved state — open Options, close, `/reload`, confirm nothing changed on its own.
- Run the CleanupManager reset action — controls reset to defaults, and no error/loop occurs.
No Lua error, no visible flicker or repeated re-render.

- [ ] **Step 6: Commit**

```bash
git add mCleanup/core/Options.lua mCleanup/core/CleanupManager.lua
git commit -m "fix(mCleanup): guard refresh routines against now-emitting widget setters"
```

> **If the pre-flight found other addons embedding mUIFramework:** repeat Steps 1-6 for each. Flag any embedder you cannot verify in-game so the user can test it.

---

## Task 10: Add deferred coalesced relayout to Layout

**Files:**
- Modify: `layouts/Layouts.lua:71-82` (Layout base) and the `addWidget` overrides in Grid/Scroll/Tab

`Layout:addWidget` calls `relayout()` on every add. Building an N-item list does N full relayouts. Coalesce: mark the layout dirty and run one relayout on the next frame via `C_Timer.After(0, ...)`.

- [ ] **Step 1: Add dirty-flag scheduling to the Layout base**

In `layouts/Layouts.lua`, in `Layout` (after `Layout:relayout` stub around line 80), add:

```lua
-- Coalesced relayout: mark dirty and schedule one relayout for the next
-- frame. Multiple addWidget calls in the same frame collapse into a single
-- _doRelayout. Subclasses implement _doRelayout, NOT relayout.
function Layout:relayout()
	if self._relayoutPending then return end
	self._relayoutPending = true
	local this = self
	C_Timer.After(0, function()
		this._relayoutPending = false
		if this._destroyed then return end
		this:_doRelayout()
	end)
end

-- Run a relayout immediately (skip coalescing). Use when the result must be
-- correct before the next line of code runs.
function Layout:relayoutNow()
	self._relayoutPending = false
	if not self._destroyed then self:_doRelayout() end
end

function Layout:_doRelayout() end
```

- [ ] **Step 2: Rename existing relayout implementations to _doRelayout**

Each layout subclass currently defines `function cls:relayout()`. Rename each to `_doRelayout`:
- `makeLinearClass`: `function cls:relayout() linearRelayout(self, horizontal) end` → `function cls:_doRelayout() linearRelayout(self, horizontal) end`
- `Grid:relayout` → `Grid:_doRelayout`
- `Scroll:relayout` → `Scroll:_doRelayout`
- `Tab:relayout` → `Tab:_doRelayout`

The base `Layout:init` wires `target:connect("resized", function() this:relayout() end)` and `"shown"` — leave those calling `relayout()` (the coalescing wrapper). Do NOT rename those.

- [ ] **Step 3: Use relayoutNow where correctness needs it immediately**

Two spots need the synchronous path:
- `Tab:setCurrentTab` calls `tab.widget:_revalidateLayout()` (from Task 8) — fine, the resized signal will schedule a coalesced relayout. No change.
- `Window:loadState` similarly. No change.

But `Layout:setSpacing` / `setMargins` / `Grid:setRowStretch` etc. currently call `relayout()` — they can stay on the coalesced path. The only place to switch to `relayoutNow()` is if a consumer reads widget geometry right after a layout mutation. Search `mCleanup/core` for `:getSize()` / `:GetWidth()` calls immediately following an `addWidget`/`setSpacing`. If found, the consumer should call `layout:relayoutNow()` first; if none found, no change needed — note this in the commit message.

- [ ] **Step 4: Guard against C_Timer firing after destroy**

`Layout` extends `mObject`, which sets `self._destroyed` in `destroy()`. The `_doRelayout` scheduling already checks `this._destroyed` — confirm `mObject:destroy` sets `_destroyed = true` (it does, `core/Object.lua:96`). No extra change needed; this step is a verification, not an edit.

- [ ] **Step 5: In-game verify**

`/reload`. Open mCleanup's main window and Options panel. Every layout-driven panel must look identical to before — widgets tiled correctly, no visible one-frame flash of mis-positioned content (a brief flash on first open is acceptable since relayout is now deferred one frame; a *persistent* misplacement is a bug). Resize the window — content re-tiles. Switch tabs — content tiles. No Lua error.

- [ ] **Step 6: Commit**

```bash
git add layouts/Layouts.lua
git commit -m "perf(layout): coalesce relayout into one deferred pass per frame"
```

---

## Task 11: Update CLAUDE.md to match the rectified framework

**Files:**
- Modify: `CLAUDE.md`

Several "Known Limitations" rows and inline warnings are now false. Bring the docs in line with the code.

- [ ] **Step 1: Fix the class-system section**

The `### Class system` section already shows `lib.Widget:extend()` and `MyWidget.super.init(...)` — that is now true. Add one line under the bullet list:

```markdown
- `mUI.Class("Name", Parent)` is an accepted alias for `Parent:extend("Name")`.
```

- [ ] **Step 2: Fix the property/signal warnings**

In the `### Properties` section, change the bolded warning to:

```markdown
Property changes auto-emit a `<name>Changed` signal. `setProperty(name, value, {silent=true})`
updates without emitting — used internally to sync state another signal already covers.
**Always use `setProperty`/`getProperty` — never mutate `self._properties` directly.**
```

- [ ] **Step 3: Remove resolved per-widget "Known issue" blocks**

Delete or rewrite these inline notes that are now false:
- **Slider**: remove the "Programmatic `setValue()` does not emit" block. Change the signal line to `valueChanged` fires on drag and on `setValue()`.
- **Spinner / SliderControl**: remove the "does NOT emit" comments.
- **Dropdown**: remove "`setSelected()` does not emit `selectionChanged`". Keep the single-popup-singleton note (still true).
- **TextInput**: remove the "bypasses property system / `getProperty('text')` may lag" block. `textChanged` arg and `getProperty("text")` now agree.

- [ ] **Step 4: Fix the "Connect signals before setting initial values" pattern**

In `## Patterns & Best Practices`, the section headed "Connect signals before setting initial values" is now mostly obsolete. Replace its body with:

```markdown
Most widget setters (`setValue`, `setSelected`, `setChecked`, `setText`) now emit their
change signal. If you push *saved/default state* into widgets during a refresh routine,
guard the routine so change handlers don't write that state straight back to storage
(see mCleanup's `OptionsUI._refreshing` pattern). For genuinely silent state sync, call
`setProperty(name, value, {silent=true})` directly.
```

- [ ] **Step 5: Update the "Known Limitations & Design Smells" table**

Remove these now-resolved rows:
- "`setValue()`/`setSelected()` don't emit signals on most widgets"
- both duplicate "`TextInput` bypasses property system internally" rows

Keep the still-true rows: `Window:saveState()` single anchor, `TreeView` no removeChild, `ScrollLayout` fixed line height, `TabLayout:addWidget` wrong API, no focus manager, no global tooltip system, `DataGrid` re-creates cells.

Add one new row reflecting the deferred relayout:

```markdown
| Layout relayout is deferred one frame | First open may show a one-frame flash | Call `layout:relayoutNow()` if geometry must be correct synchronously |
```

- [ ] **Step 6: In-game verify**

No code change in this task — verification is a doc read-through. Confirm every code example in CLAUDE.md uses an API that now exists (`extend`, `super`, `setProperty(...,{silent=true})`, `relayoutNow`). No `/reload` needed.

- [ ] **Step 7: Commit**

```bash
git add CLAUDE.md
git commit -m "docs: align CLAUDE.md with rectified property/class/layout behavior"
```

---

## Final verification (after all tasks)

- [ ] `/reload` with a clean SavedVariables (or back up and delete `mCleanup`'s saved variables file) and confirm:
  - mCleanup main window opens, no error.
  - Options panel opens showing defaults, no error.
  - Every interactive widget (checkboxes, sliders, slider-controls, spinners, dropdowns, text input) responds to user input and persists where expected.
  - Opening Options does not silently mutate saved state.
  - Tab switching and window resize re-tile content correctly.
  - CleanupManager reset restores defaults without a loop.
- [ ] Confirm no `!BugGrabber` errors accumulated during a full session of opening/closing every panel.
- [ ] Confirm the temp `extend()/super` smoke-test block from Task 7 Step 4 was removed.

---

## Self-Review Notes

- **Spec coverage:** All four serious/moderate issues from the review are covered — property bypass (Tasks 2-6), class API mismatch (Task 7), scattered re-emit hacks (Task 8), per-mutation relayout cost (Task 10). The `Signal:emit` O(n) snapshot allocation noted in the review is *not* addressed here — it is a separate optimization, low-risk, and out of scope for a correctness-focused rectification; left for a future plan.
- **Consumer impact:** Task 9 explicitly handles the breaking change from "setters now emit." This is the riskiest task — it depends on reading the actual mCleanup refresh code, which is why Step 1 is a read-and-list step before any edit.
- **Ordering:** Task 1 (re-entrancy guard) MUST land before Tasks 3-6, or a property handler that writes back to the same property recurses. Task 8's `_revalidateLayout` is independent. Task 10 depends on nothing but should land last among code tasks since it changes relayout timing.
