fix(lower): VariantFields shadow fix — codex blocker on #565

[briansrls]

Summary

Fixes the codex-flagged blocker that landed in #565 (merged): the new SurfacePattern::VariantFields lowering branch used the full outer scope as its duplicate-binding detector, so a legitimate match-arm shadow of an outer name got rejected as "binds X more than once." VariantWith already shadowed correctly — VariantFields should match that shape.

The bug

lower.rs:5074-5103 in the VariantFields arm: arm_scope.contains_key(&field.binding) checks the FULL outer scope cloned at line 5057. Any outer binding collides.

The fix

Per-arm introduced: HashSet<String> tracks only names THIS pattern brings in. Duplicate-check against introduced, not arm_scope. Pattern-local duplicates still reject; outer shadowing works via plain HashMap overwrite semantics (same as VariantWith).

Regression test

prereq2_named_payload_pattern_shadows_outer_scope_binding:

```
fn read(w: Wrapped, inner: Int) -> Int =
match w { Wrap { inner: inner } => inner.inner, Empty => inner }
```

inner is a fn parameter; match arm Wrap { inner: inner } shadows it. Previously rejected as duplicate; now compiles cleanly.

Test plan

• New regression test passes
• Existing duplicate-binding test still passes (pattern-local detector unchanged)
• Full m1_substrate_test suite: 95 passed, 0 failed

🤖 Generated with Claude Code

[briansrls]

claude-review (director) — ✅ ship it

Root-caused correctly. The scope-inversion is the real bug: arm_scope is a clone of the outer scope, so contains_key over it conflates "this pattern re-binds a name" with "pattern binding collides with an outer binding." The fix (per-pattern introduced: HashSet<String>) cleanly separates the two concerns — pattern-local duplicates still reject, outer-name shadowing works.

Cross-checked:

• Verified VariantWith shape (line 5068, arm_scope.insert(...) with overwrite) matches what you claim.
• Checked for similar-shape bugs elsewhere: collect_scope_bound_free_vars at 1212 uses arm_scope.clone() differently (removes bindings before recursing) — that's a distinct correct usage, not a sibling bug.

Per TESTING.md:

• Regression test name follows <subject>_<verb>_<object>_<condition> ✓
• One claim per test ✓
• Uses cached_compile_to_dag full pipeline — acceptable today per TESTING.md §Practical guidance (Dag builder API doesn't exist yet); natural unit-test once Phase 1 builders land.

Note: HashSet<String> is weakly typed per CODING.md §Typed handles — but this is a focused fix, not the place for the broader lowering-binding refactor. Merge as-is.

CI should clear shortly. Merge when green.