[Bug] Attached-property exclusion list accepts instance DPs: the diagnostic recommends it, and no test reads the reason

[azchohfi]

What happened?

PoolResetSetConsistencyTests.Every_Reset_Attached_Property_Is_Classified fires whenever a property cleared in ElementPool.CleanElement has an owner that is not in the test's InstancePropertyOwners set. Its failure message offers two remedies:

These attached properties are cleared in ElementPool.CleanElement but are in neither ModifierTable.AttachedProperties nor DeliberatelyExcludedAttached: [Grid.CornerRadius, Grid.Padding]. Either map them (so REACTOR_POOL_001 fires on .Set(fe => Owner.SetPROP(fe, ...))), or exclude them with a documented reason.

There is a third remedy, and for this failure it was the correct one: add the owner to InstancePropertyOwners, because the property is not attached at all — the attached scan is merely the fallback for owners that list doesn't name. The message never mentions the list that decides whether you land in the fallback.

Taking the recommended "exclude" branch for an instance DP produces a self-contradicting entry that the entire suite accepts. ModifierTableIntegrityTests.Every_Attached_Exclusion_Carries_A_Reason is the only test that reads exclusion values, and it only asserts the reason is non-blank:

var blank = ModifierTable.DeliberatelyExcludedAttached
    .Where(kvp => string.IsNullOrWhiteSpace(kvp.Value))

This is not hypothetical — it happened, and the reason text stated the bug in plain English while every test stayed green:

["Grid.Padding"]      = "Instance dependency property on Grid, not an attached property with a static setter.",
["Grid.CornerRadius"] = "Instance dependency property on Grid, not an attached property with a static setter.",

An exclusion whose documented reason is "this is an instance dependency property, not an attached property" is prima facie evidence that the exclusion is the wrong mechanism — DeliberatelyExcludedAttached is a suppression list for genuinely-attached properties that can't be written through Owner.SetPROP(x, v), not a classification list. Both entries were removed in PR #1015 (ddb8b4bd) in favour of adding Grid to InstancePropertyOwners; this issue is about the guidance and the missing gate that led there, both of which are still live on main.

The consequence of leaving it: a genuinely attached Grid.* reset added later lands in the same bucket as these two and reads as already-triaged, so REACTOR_POOL_001 silently stops firing for it.

Steps to reproduce

1. Check out main (or PR Fix ElementPool.CleanElement never releasing Control/Panel-level modifier properties (#985) #1015's head, where the classification is already correct).

2. Delete the "Grid", entry from InstancePropertyOwners in tests/Reactor.Tests/AnalyzerTests/PoolResetSetConsistencyTests.cs — this simulates the state any new panel receiver starts in.

3. Run:

   dotnet test tests/Reactor.Tests --filter "FullyQualifiedName~PoolResetSetConsistencyTests" -c Release -p:Platform=x64
   

   → Failed: 1, Passed: 53, Total: 54, naming [Grid.CornerRadius, Grid.Padding].

4. Follow the message's second recommendation — add both keys to DeliberatelyExcludedAttached with an honest reason such as "Instance dependency property on Grid, not an attached property with a static setter."

5. Re-run the full suite → green. The misclassification is now permanent, documented, and invisible.

Suggested fix, both halves:

• Message. Name the third remedy: "…or, if this is an ordinary instance dependency property rather than an attached one, add its owner to InstancePropertyOwners." Step 4 above is only attractive because the message presents exclusion as the general escape hatch.
• Gate. Add a test asserting no exclusion reason declares its key an instance property (e.g. reject reasons matching instance (dependency )?propert). That is a content oracle rather than a presence oracle, and it is what distinguishes the three legitimate AutomationProperties rows — whose reasons cite the absence of a static setter — from a misfiled instance DP. A count- or non-blank-based check passes on a wrong-but-stable set by construction.

The general shape, which is what makes this worth fixing rather than just correcting the two rows: consistency derived from a classification list cannot prove that list is complete. Every existing test here checks that the tables agree with each other, so a property missing from InstancePropertyOwners and suppressed in DeliberatelyExcludedAttached is perfectly self-consistent. Only an oracle over reason content is independent of the classification it is checking.

Reactor version / commit

1ed2644e7d98e310f0efb6db4c33c5f0ec2f2568 (main). The diagnostic text was introduced in 96cadfe3 ("feat(analyzers): extend REACTOR_POOL_001 to attached-property writes in .Set", 2026-07-29); the Grid.* exclusions arrived later and were removed in PR #1015.

Platform

x64

.NET SDK version

10.0.302

Windows version

Windows 11 build 26310

Windows App SDK version

N/A — headless Reactor.Tests tier only; no Windows App SDK surface involved.

Logs / stack trace

Microsoft.UI.Reactor.Tests.AnalyzerTests.PoolResetSetConsistencyTests.Every_Reset_Attached_Property_Is_Classified [FAIL]
  Error Message:
   These attached properties are cleared in ElementPool.CleanElement but are in neither
   ModifierTable.AttachedProperties nor DeliberatelyExcludedAttached: [Grid.CornerRadius, Grid.Padding].
   Either map them (so REACTOR_POOL_001 fires on '.Set(fe => Owner.SetPROP(fe, ...))'),
   or exclude them with a documented reason.
  at ...PoolResetSetConsistencyTests.Every_Reset_Attached_Property_Is_Classified()
     in tests\Reactor.Tests\AnalyzerTests\PoolResetSetConsistencyTests.cs:line 219

Failed!  - Failed: 1, Passed: 53, Skipped: 0, Total: 54

Confirmation

• I have searched existing issues and this isn't a duplicate.
• This bug reproduces against the current main branch.

[azchohfi]

Merge hazard for whoever next edits src/Reactor/Core/ModifierTable.cs — recording it here so it is retrievable rather than living in session threads.

origin/main currently carries 6 DeliberatelyExcludedAttached rows. PR #1015 carries 3 — it deliberately removes Grid.Padding, Grid.CornerRadius and StackPanel.CornerRadius.

That removal is the fix for this issue: those owners moved into InstancePropertyOwners, so the instance scan claims the properties and the attached-scan suppression rows stop being necessary. Exclusions suppress; the owner list classifies.

The hazard is that restoring those rows re-introduces the bug, and nothing catches it. Measured on #1015'"'"s branch:

row `["Grid.Padding"]` restored -> PoolResetSetConsistencyTests  F=0 P=54 T=54   (UNCHANGED)
                                   out-of-band checker           FAIL: must delete Grid.Padding

Git auto-merges the file with no conflict, the full suite stays green, and CI has nothing to say. A union resolution that reconciles the two lists by taking main'"'"s rows silently reverts the fix and produces a fully green build.

Two related notes for the same file:

1. A superset framing is what makes taking main'"'"s rows look safe — and it is only true of the other list. CleanElementRequiredClears goes main=20 -> HEAD=34 (strict superset, onlyMain=0); DeliberatelyExcludedAttached goes main=6 -> HEAD=3 (strict subset). Both have been informally called "the allow-list". They point in opposite directions.

2. Grid is a mixed owner and the partition is owner-level (.Where(m => !InstancePropertyOwners.Contains(owner))). Padding/CornerRadius are instance DPs; Row/Column/RowSpan/ColumnSpan are attached. Owner-level membership cannot express that, so every future Grid.* reset routes to the instance bucket. Verified with a positive control on Fix ElementPool.CleanElement never releasing Control/Panel-level modifier properties (#985) #1015'"'"s branch: adding a Canvas.Left clear reddens Every_Reset_Attached_Property_Is_Classified; adding a Grid.Row clear produces no failure at all.

   Under main'"'"s exclusion-list arrangement a newly-added Grid.Row clear would have failed and forced triage. Under the owner-list arrangement there is no triage moment. Latent today (no such clear exists), but it changes the shape of the hazard rather than removing it.

Credit: measured and reported by the #1015 and #1018 sessions.