Spec 038 Phase 3 — Class-B rules + Margin/Padding API ergonomics#248
Conversation
…t ranker
`mur check` grows two agent-visible behaviors on top of the Phase-1 Tier-2
suggester: MSBuild flag passthrough via the `--` separator with
default-merging (`--nologo`, `-v:m`, `-p:Platform={host arch}` injected
only when not named in passthrough), and a pre-emit ranker that suppresses
cosmetic noise mid-iteration (CS1591 XML doc, CS0168 unused-var, IDE0xxx
style, NU1701/NU1605, MSB3245/3270/3277, CS8600-8625 nullable warnings,
REACTOR_* info-severity) while always emitting errors via a universal-
error floor. New mode flags `--strict` / `--final` / `--quiet` and
`--emit-threshold <float>` round-trip through ArgsParser. Trace output
records the effective `dotnet build` argv as a `kind: "command"` header
row so replays are bit-faithful.
Suppress→error guardrail at tools/Reactor.MurCheckGuardrail audits an
iter+final trace pair against PolicyTable's universal-error-floor
invariant. Re-uses the live PolicyTable via InternalsVisibleTo so the
two sources can never drift. Eight unit tests; the harness-side CI
wiring is deferred behind the same WindowsAppSDK-fixture blocker as
Phase-1 §1.6.
Phase-2.x post-EC2 fixes:
* Gate-input regression. CheckCommand.ShouldEmitSuggestions now counts
the full parsed diagnostic list, not the post-ranker emittable list.
EC2's 3-round preview measured Tier-2 firing collapse from EC1's 80%
to 0% on kanban-mur because the ranker's nullable-warning suppression
closed the gate on builds EC1 had left open. Regression test
RankerTests.Suggest_gate_counts_full_parsed_list_not_post_ranker_emittable
locks the behavior.
* `--final` framing softened in both plugins/reactor/skills/reactor-
build-and-check/SKILL.md and the legacy root SKILL.md. EC2's 3-round
preview measured 0/6 production value across mandatory --final
invocations; the framing now describes --final as an optional pre-
merge sweep for human review / CI gates, explicitly not a task-
completion requirement.
* TraceWriter mode propagation fix. The diagnostic-row `mode` field
used to be hardcoded to "iteration" even when invoked via --final.
Now piped through TraceWriter.Open(path, root, mode); regression
test TraceWriterTests.Diag_row_mode_matches_writer_invocation_mode
locks it.
EC2 5×N PASS by median (2026-05-11):
* reactor-calc-mur-check beats base on every metric — cost −5.1%,
tokens −5.8%, turns −5.1%, wall −7.9%; CV cost 1.9× tighter.
* reactor-kanban-mur-check at cost median parity ($3.30 = $3.30);
mean dragged to +5.7% by R2 outlier (R2-excluded mean −3.3%).
* First-build OK 5/5 both variant arms.
* --final invocation 0/10 across both projects (SKILL framing
working as designed; the previous bugged-Phase-2 batch had 6/6
mandatory invocations).
* Tier-2 firing 0/10 — gate correctly inhibits on small-batch
iteration patterns; closing the kanban token gap is Phase-3's
scope (rules > fuzzy match).
* Criterion-2 guardrail audit deferred to a harness retrofit
(post-run `mur check --final` against the final workspace state).
Tests: 7102 / 7102 passing on the post-fix tree. CheckCommandTests
grew from 120 to 148 tests across the Phase-2 surfaces (ArgsParser
passthrough + mode flags, Ranker scoring + emit gates, Guardrail
trace-pair audit, TraceWriter mode propagation, gate-input regression).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three Class-B vocabulary-translation rules under the Phase-3 rule infrastructure, plus two Reactor API/skill fixes surfaced while authoring them. Rules (Validation Gate; ready for review, not merge — bar #5 is human): - ThemeBackgroundSuffixRule: CS0117 on Theme.*Background → Theme.SolidBackground (cluster C0019, 16 events) - AlignmentShortcutRule: CS1061 on Reactor *Element receivers for HorizontalAlignment / VerticalAlignment → .HAlign(...) / .VAlign(...) (cluster C0017 + adjacent, ~22 events) - ButtonOnClickFactoryMoveRule: CS1061 on ButtonElement.OnClick → Button(..., onClick: ...), explicitly naming .OnTapped as the wrong sibling to keep agents from reaching for the gesture event Reactor API fixes (driven by 525-run corpus signal): - Margin / Padding per-side overloads now default unspecified sides to 0.0, eliminating ~198 corpus build failures from .Margin(top: 12)-shaped calls - BREAKING (pre-1.0): Margin(double, double) and Padding(double, double) parameter order swapped from (horizontal, vertical) to (vertical, horizontal) to match CSS shorthand (top/bottom first). All repo callsites migrated to named-arg form for layout preservation and reviewer clarity. Skill updates (agent context): - reactor-getting-started cheatsheet shows named-arg Button("Save", onClick: handler) with explicit anti-pattern call-out - .OnTapped example re-anchored to non-Button surfaces (Border, Image, ScrollView) §3.1a residual: trace-channel rule_self_disabled row when a rule's declared target stops resolving against the live Reactor compilation; routed through TraceWriter.WriteRuleSelfDisabled, dedup'd per-invocation per-rule. Tests: Reactor.Tests 7156/7202 (46 expected-skip); CheckCommandTests 193/193. Solution builds clean (no new C# / REACTOR_* warnings). Class-A rule PRs remain blocked on second-agent corpus drop (Validation Gate bar #2, cross-agent reproducibility). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR advances Spec 038 by adding Phase-3 Tier-3 rule infrastructure + three initial Class‑B vocabulary-translation rules, and by improving Reactor layout ergonomics (Margin/Padding) with corresponding repo-wide callsite/doc updates. It also includes Phase‑2 mur check plumbing (modes, deterministic ranker, richer trace schema) and an offline guardrail tool.
Changes:
- Add Tier‑3 rule system (
IRulePattern,RuleRegistry,RuleSymbolResolver) and integrate it intomur check/Tier‑2 suggesting, including--disable-rule/--list-rulesand trace telemetry for self-disabled rules. - Introduce deterministic pre-emit ranker + Phase‑2 argument parsing / passthrough behavior, plus an offline guardrail tool that audits iter vs final traces against
PolicyTable. - Update
ElementExtensions.Margin/Paddingoverload ergonomics (per-side defaults; 2‑arg order swap) and migrate callsites/docs/skills accordingly.
Show a summary per file
| File | Description |
|---|---|
| tools/Reactor.MurCheckGuardrail/Reactor.MurCheckGuardrail.csproj | New offline guardrail tool project referencing Reactor.Cli PolicyTable |
| tools/Reactor.MurCheckGuardrail/Program.cs | Guardrail tool entrypoint |
| tools/Reactor.MurCheckGuardrail/GuardrailRunner.cs | Guardrail logic: parse trace JSONL + audit against PolicyTable invariants |
| tests/Reactor.Tests/Reactor.Tests.csproj | Reference guardrail tool from unit tests |
| tests/Reactor.Tests/ElementTests.cs | New Margin/Padding regression tests (per-side defaults, overload binding, CSS order) |
| tests/Reactor.Tests/ElementExtensionsCoverageTests.cs | Update 2-arg Margin/Padding tests to named args |
| tests/Reactor.Tests/ElementExtensionsAdditionalTests.cs | Update Margin overload test to named args |
| tests/Reactor.Tests/CheckCommandTests/TraceWriterTests.cs | Add tests for trace mode and rule-self-disabled schema |
| tests/Reactor.Tests/CheckCommandTests/SuggesterOrchestratorRuleTests.cs | New tests for Tier‑3 rule vs Tier‑2 precedence + disable/self-disable plumbing |
| tests/Reactor.Tests/CheckCommandTests/Rules/ThemeBackgroundSuffixRuleTests.cs | New fixture tests for Theme.*Background → Theme.SolidBackground rule |
| tests/Reactor.Tests/CheckCommandTests/Rules/RuleTargetResolutionTests.cs | New CI gate test intended to resolve all rule declared targets against live Reactor compilation |
| tests/Reactor.Tests/CheckCommandTests/Rules/RuleSymbolResolverTests.cs | New tests for resolver caching + type/member resolution |
| tests/Reactor.Tests/CheckCommandTests/Rules/RuleRegistryTests.cs | New tests for registry discovery, ordering, disable/self-disable, exception swallowing |
| tests/Reactor.Tests/CheckCommandTests/Rules/RuleContractTests.cs | New contract-shape tests for RuleContext/RuleSuggestion |
| tests/Reactor.Tests/CheckCommandTests/Rules/ButtonOnClickFactoryMoveRuleTests.cs | New fixture tests for ButtonElement.OnClick → Button(..., onClick: ...) rule |
| tests/Reactor.Tests/CheckCommandTests/Rules/AlignmentShortcutRuleTests.cs | New fixture tests for Horizontal/VerticalAlignment → HAlign/VAlign rule |
| tests/Reactor.Tests/CheckCommandTests/RankerTests.cs | New unit tests for deterministic ranker semantics and invariants |
| tests/Reactor.Tests/CheckCommandTests/GuardrailRunnerTests.cs | New unit tests for guardrail audit + trace parsing |
| tests/Reactor.Tests/CheckCommandTests/CheckCommandPipelineTests.cs | Add ranker/trace pipeline regression tests (stdout filter vs trace completeness, command rows) |
| tests/Reactor.Tests/CheckCommandTests/ArgsParserTests.cs | New tests for Phase‑2 args parsing, passthrough merging, modes, emit-threshold, rule flags |
| tests/Reactor.AppTests.Host/SelfTest/Fixtures/DataGridScrollFixtures.cs | Migrate Padding(…, …) callsites to named args |
| tests/Reactor.AppTests.Host/SelfTest/Fixtures/DataGridEditFixtures.cs | Migrate Padding(…, …) callsites to named args |
| tests/cmd_perf/CmdPerf.Reactor/Program.cs | Migrate Margin/Padding 2‑arg callsites to named args |
| src/Reactor/Hosting/Devtools/DevtoolsMenuFactory.cs | Migrate Padding(…, …) to named args |
| src/Reactor/Elements/ElementExtensions.cs | Margin/Padding API change: per-side defaults + 2‑arg overload order swap |
| src/Reactor/Controls/Editors/CellRenderers.cs | Migrate Padding(…, …) to named args |
| src/Reactor/Controls/DataGrid/DataGridComponent.cs | Migrate Padding(…, …) to named args |
| src/Reactor.Cli/Reactor.Cli.csproj | Add InternalsVisibleTo for guardrail tool assembly |
| src/Reactor.Cli/Check/TraceWriter.cs | Add trace mode plumbing + command rows + rule-self-disabled rows |
| src/Reactor.Cli/Check/SuggesterOrchestrator.cs | Integrate Tier‑3 rules alongside Tier‑2; allow non-Tier2 codes via rules |
| src/Reactor.Cli/Check/Rules/ThemeBackgroundSuffixRule.cs | New Class‑B vocab translation rule for Theme.*Background |
| src/Reactor.Cli/Check/Rules/RuleSymbolResolver.cs | New per-compilation symbol resolver (type/method/member) with caching |
| src/Reactor.Cli/Check/Rules/RuleRegistry.cs | New reflection-discovered registry + disable/self-disable/status reporting |
| src/Reactor.Cli/Check/Rules/IRulePattern.cs | New Tier‑3 rule contract + RuleContext/RuleSuggestion types |
| src/Reactor.Cli/Check/Rules/ButtonOnClickFactoryMoveRule.cs | New Class‑B rule for ButtonElement.OnClick → Button(..., onClick: ...) |
| src/Reactor.Cli/Check/Rules/AlignmentShortcutRule.cs | New Class‑B rule for *Alignment → HAlign/VAlign |
| src/Reactor.Cli/Check/Ranker/Ranker.cs | New Ranker wrapper over PolicyTable |
| src/Reactor.Cli/Check/Ranker/PolicyTable.cs | New deterministic policy table for per-mode scoring/thresholds |
| src/Reactor.Cli/Check/Mode.cs | New mur check invocation modes enum |
| src/Reactor.Cli/Check/CheckCommand.cs | Switch to ArgsParser, add list/disable rule flags, ranker stdout filtering, trace enhancements |
| src/Reactor.Cli/Check/CheckArgs.cs | Expand parsed args record + update help text; keep back-compat TryParse |
| src/Reactor.Cli/Check/ArgsParser.cs | New Phase‑2 parser: passthrough boundary + default-merging + modes + rule flags |
| skills/reactor.api.txt | Update API index for new Margin/Padding signatures |
| skills/design.md | Update design skill examples to named args for Margin(…, …) |
| SKILL.md | Update root skill guidance for mur check behavior + examples |
| samples/TodoApp/Program.cs | Migrate Padding(…, …) to named args |
| samples/ReactorHostControlDemo/MainWindow.xaml.cs | Migrate Margin(…, …) to named args |
| samples/ReactorHostControlDemo/CounterDemo.cs | Migrate Margin(…, …) to named args |
| samples/ReactorGallery/ControlPages/DesignGuidance/SpacingPage.cs | Update docs examples to named args for Margin/Padding 2‑arg form |
| samples/ReactorCharting.Gallery/Samples/DonutMixer.cs | Migrate Margin(…, …) to named args |
| samples/Reactor.TestApp/Demos/VirtualizationDemo.cs | Migrate Padding(…, …) to named args |
| samples/Reactor.TestApp/Demos/SlotsDemo.cs | Migrate Padding(…, …) to named args |
| samples/Reactor.TestApp/Demos/FlyoutDemo.cs | Migrate Padding(…, …) to named args |
| samples/Reactor.TestApp/Demos/DynamicListDemo.cs | Migrate Padding(…, …) to named args |
| samples/Reactor.TestApp/Demos/DataSystemDemo.cs | Migrate Margin(…, …) to named args |
| samples/Reactor.TestApp/Demos/ConditionalDemo.cs | Migrate Padding(…, …) to named args |
| samples/Reactor.TestApp/Demos/AsyncValueSamplesDemo.cs | Migrate Margin(…, …) to named args |
| samples/Reactor.TestApp/App.cs | Migrate Padding(…, …) to named args |
| samples/InteropFirst/Components/OrdersDataGrid.cs | Migrate Margin/Padding 2‑arg callsites to named args |
| samples/CommandingDemo/App.cs | Migrate Margin 2‑arg callsites to named args |
| samples/apps/monaco-editor/App.cs | Migrate Padding(…, …) to named args |
| samples/apps/headtrax/HeadTrax/Components/EmployeeGrid.cs | Migrate Padding(…, …) to named args |
| samples/apps/demo-script-tool/App/Components/InlineBanner.cs | Migrate Padding(…, …) to named args |
| Reactor.slnx | Add guardrail tool project to solution |
| plugins/reactor/skills/reactor-getting-started/SKILL.md | Update cheatsheet to emphasize Button onClick factory arg and avoid .OnTapped |
| plugins/reactor/skills/reactor-dsl/references/reactor.api.txt | Update embedded API reference for Margin/Padding signature changes |
| plugins/reactor/skills/reactor-design/SKILL.md | Update design skill examples to named args for Margin(…, …) |
| plugins/reactor/skills/reactor-build-and-check/SKILL.md | Update build-and-check skill to reflect mur check semantics and workflow |
| docs/specs/tasks/038-vocab-table.csv | Add vocab translation table entries used to justify Class‑B rules |
| docs/specs/tasks/038-mur-check-did-you-mean-implementation.md | Update spec task status and evaluation writeup with Phase‑2/3 progress |
| docs/specs/tasks/038-EC2-handoff.md | Add EC2 handoff doc for eval operator |
| docs/guide/styling.md | Update examples to named args for Padding(…, …) (but file is generated) |
| docs/guide/getting-started.md | Update examples to named args for Padding(…, …) (but file is generated) |
| docs/guide/flex-layout.md | Update examples to named args for Padding(…, …) (but file is generated) |
| docs/guide/collections.md | Update examples to named args for Padding(…, …) (but file is generated) |
| docs/guide/animation.md | Update examples to named args for Padding(…, …) (but file is generated) |
| docs/_pipeline/apps/styling/App.cs | Update snippet source to named args for Padding(…, …) |
| docs/_pipeline/apps/flex-layout/App.cs | Update snippet source to named args for Padding(…, …) |
| docs/_pipeline/apps/collections/App.cs | Update snippet source to named args for Padding(…, …) |
| docs/_pipeline/apps/calculator/App.cs | Update snippet source to named args for Padding(…, …) |
| docs/_pipeline/apps/animation/App.cs | Update snippet source to named args for Padding(…, …) |
| CHANGELOG.md | Document breaking Margin/Padding 2‑arg order change and Phase‑2/3 additions |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 83/83 changed files
- Comments generated: 6
Copilot review feedback on #248: BuildLiveReactorCompilation enumerated AppDomain.CurrentDomain.GetAssemblies() before the `typeof(...AsyncValue<int>).Assembly` force-load, so on a cold xunit run where Reactor.dll wasn't already loaded, the enumeration completed without Reactor in the reference list and target resolution failed for every rule. Moving the force-load to the head of the method guarantees Reactor.dll is in the AppDomain before enumeration begins. Spec 038 §3.1a. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Thanks for the review @copilot. Both points addressed: 1. RuleTargetResolutionTests force-load order — real bug, fixed in 9504cb0. The 2. Verified by reverting and re-running the pipeline:
Future similar work will use the pipeline flow as the authoritative path. |
Summary
Three Class-B vocabulary-translation rules under the Phase-3 rule infrastructure, plus two Reactor API/skill fixes surfaced while authoring them.
Rules (Validation Gate; ready for review, not merge — bar #5 is human):
ThemeBackgroundSuffixRule—CS0117onTheme.*Background→Theme.SolidBackground(cluster C0019, 16 events)AlignmentShortcutRule—CS1061on Reactor*Elementreceivers forHorizontalAlignment/VerticalAlignment→.HAlign(...)/.VAlign(...)(cluster C0017 + adjacent, ~22 events)ButtonOnClickFactoryMoveRule—CS1061onButtonElement.OnClick→Button(..., onClick: ...), explicitly naming.OnTappedas the wrong sibling to keep agents from reaching for the gesture eventReactor API fixes (driven by 525-run corpus signal):
Margin/Paddingper-side overloads now default unspecified sides to0.0, eliminating ~198 corpus build failures from.Margin(top: 12)-shaped callsMargin(double, double)andPadding(double, double)parameter order swapped from(horizontal, vertical)to(vertical, horizontal)to match CSS shorthand (top/bottom first). All repo callsites migrated to named-arg form (.Margin(horizontal: X, vertical: Y)) for layout preservation and reviewer clarity.Skill updates (agent context):
reactor-getting-startedcheatsheet shows named-argButton("Save", onClick: handler)with explicit anti-pattern call-out.OnTappedexample re-anchored to non-Button surfaces§3.1a residual: trace-channel
rule_self_disabledrow when a rule's declared target stops resolving against the live Reactor compilation; routed throughTraceWriter.WriteRuleSelfDisabled, dedup'd per-invocation per-rule.Test plan
dotnet test tests/Reactor.Tests— 7156 passed, 46 expected-skip, 0 faileddotnet build Reactor.slnx -p:Platform=x64— 0 errors, 29 warnings (all pre-existing in main: NU1903 CVE + a11y-showcase + stress_perf obsolete-API)mur check --list-rulesenumerates all three rules.Margin(16, 14)→Thickness(14, 16, 14, 16).Margin(top: 12)→Thickness(0, 12, 0, 0)Review feedback addressed
RuleTargetResolutionTests.BuildLiveReactorCompilationforce-loads Reactor.dll before enumeratingAppDomain.CurrentDomain.GetAssemblies()— prevents false target-resolution failures on a cold xunit run where Reactor.dll wasn't already loaded.docs/guide/*regeneration verified viamur docs compile: 4 of 5 files (styling, flex-layout, collections, animation) regenerate byte-identical to the committed content.getting-started.mdhas pre-existing pipeline drift (inlined snippets vssnippet=references) unrelated to this PR scope — the single Padding-line update is a targeted hand-edit; full regen pending a separate cleanup PR.Deferred follow-ups
REACTOR_*analyzer for positional 2-argMargin/Padding(named-arg migration in this PR makes the order-footgun opt-in; analyzer would make it loud)docs/guide/getting-started.mdpipeline-drift cleanup (collapse inlined snippets back intosnippet=references)🤖 Generated with Claude Code