PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The original six-site prescription is premise-invalid, but this is not a Drop+Supersede: two one-call Boolean migrations are correct and now have a truth-folded close target. One bounded repair can retain that value, remove the four atomicity regressions, and converge directly to approval.

Peer-Review Opening: The SourceHealthMarker exception was the right catch: its observer test exposed that multiple public class calls are not one coherent config change. The same mechanism remains in four migrated transition hooks, so this review narrows the patch to the two calls that are genuinely representation-independent today.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Live #15201 including its prior atomicity falsifier and current scope fold; sibling #15202 and Ada's own measured stop comment; D#15200's atomic-replace requirement; the five-file changed list; current dev; exact c464a40465 source and CI; Neo.component.Base class methods; generated config setters; Neo.core.Config; the mandatory app-work contracts (Neo, core.Base, state.Provider, data.Model, data.Store); and the focused Fleet specs.
• Expected Solution Shape: Only StateDot.afterSetLive and HealthBar.updateAnimateCls can move to one enforced toggleCls call under today's API. Old→new class replacements must remain one aggregate assignment until D#15200 supplies an atomic semantic replacement; tests must observe both transition directions and the one-publication invariant.
• Patch Verdict: Contradicts in four of six methods. The two Boolean toggles match; EventChip.afterSetKind, StateDot.afterSetState, HealthSwatch.afterSetState, and FamilyRail.afterSetFamily decompose one semantic transition into two or three reactive cls assignments.
• Premise Coherence: Conflicts with verify-before-assert as submitted: the PR correctly honored the observer falsifier for SourceHealthMarker, but treated the identical remove/add mechanism as behavior-preserving elsewhere despite the ticket, D#15200, and #15202 already recording the opposite measurement.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15201
• Related Graph Nodes: D#15200; #15202; component-class mutation; reactive config atomicity; AgentOS Fleet.

🔬 Depth Floor

Challenge: On the exact PR head I first reproduced the author's focused command at 39/39 green, then added a reviewer-only observer witness around the migrated StateDot transition. wedged → ok published two cls events: the first preserved caller-authored but contained neither fm-state-wedged nor fm-state-ok; the second added fm-state-ok. component.Base#afterSetCls calls update() after every changed assignment, so this is the ticket's named intermediate-state regression, not a theoretical style objection.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: “no behavior change,” “pure API-boundary swap,” and five migrated primitives exceed the transition behavior and the now-live two-site #15201 contract.
• Anchor & Echo summaries: N/A — no durable source summaries were added.
• [RETROSPECTIVE] tag: N/A — none introduced.
• Linked anchors: D#15200 explicitly records that remove+add is non-atomic and requires a future replacement/batch primitive; it does not authorize these four replacements.

Findings: Required Actions 1 and 3 close the implementation and metadata drift.

🧠 Graph Ingestion Notes

• [KB_GAP]: The Knowledge Base correctly surfaced setSilent() as a VDOM transaction tool, but that does not make sequential config publications atomic and would re-import the batching seam this app cleanup excludes.
• [TOOLING_GAP]: The existing 39 Fleet tests assert final class endpoints. They all pass while the transition publishes a classless intermediate state; an observer-count witness is required for this contract.
• [RETROSPECTIVE]: Endpoint equivalence is not transition equivalence. A semantic old→new class change needs one reactive publication, not merely the correct final array.

🎯 Close-Target Audit

• Close-target identified: #15201.
• #15201 is open and not epic-labeled.
• The current five-file diff matches #15201's live two-site scope.

Findings: The close target is structurally valid but the diff over-delivers four explicitly deferred hooks; RA1 restores alignment.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: this app-only refactor introduces no public contract/API, OpenAPI description, skill convention, or cross-substrate workflow.

🪜 Evidence Audit

• The PR body declares L1 unit evidence.
• The 39/39 receipt does not cover #15201's binding one-publication AC; it proves final state only.
• No external environment receipt is used as a merge gate.

Findings: Evidence is current-head and honestly reproducible, but insufficient for the transition property; RA2 adds the reachable unit witness.

📜 Source-of-Authority Audit

• Authorities checked: live #15201; D#15200's current body and Ada's atomicity comment; sibling #15202's two independent measurements; current component.Base / config setter source.
• Finding: All sources agree: enforced single-class toggles are valid now; old→new replacements remain blocked on an atomic semantic primitive. #15201 has been truth-folded to that bounded salvage contract rather than spawning a micro-ticket.

🧪 Test-Evidence & Location Audit

• Execution evidence: all exact-head required checks are green at c464a40465; the author and reviewer both reproduced the six-file focused suite at 39/39.
• Reviewer falsifier: npm run test-unit -- .../pr15446Atomicity.spec.mjs passed its negative claim by observing exactly two publications and a first state with neither semantic class.
• Test location/evidence: no patch tests were added for the new one-publication contract, transition directions, unrelated-class preservation, or idempotence.

Findings: Green endpoint coverage does not discharge the atomicity AC. The bounded salvage needs focused tests for the two retained toggles.

📋 Required Actions

To proceed with merging, please address the following:

• RA1 — Match the live two-site authority. Keep StateDot.afterSetLive → toggleCls and HealthBar.updateAnimateCls → toggleCls. Revert the changes to EventChip.afterSetKind, StateDot.afterSetState, HealthSwatch.afterSetState, and FamilyRail.afterSetFamily; those old→new replacements stay atomic aggregate assignments until D#15200 supplies the missing primitive.
• RA2 — Pin the property the endpoint suite misses. Add focused unit witnesses for both retained sites covering true→false and false→true, exactly one cls publication when membership changes, unrelated-class preservation, and duplicate-free/idempotent repeats.
• RA3 — Truth-fold the landing artifact. Narrow the PR title/body, Deltas, evidence, and test claims to the two retained methods; remove “five primitives,” “pure API-boundary swap,” and “no behavior change” claims for the reverted sites. Record the D#15200 deferral without creating a successor micro-ticket.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 55 - The API owner is right for Boolean membership; four replacement transitions violate the atomicity boundary.
• [CONTENT_COMPLETENESS]: 46 - Two valid methods are present, but the diff and prose exceed the live close-target contract and lack the transition witnesses.
• [EXECUTION_QUALITY]: 62 - Exact-head CI and 39/39 focused tests are green; the named observer falsifier still proves a regression.
• [PRODUCTIVITY]: 84 - The patch is small and has a bounded two-file salvage path; repair should be one short cycle.
• [IMPACT]: 32 - Useful representation-decoupling maintenance, not a release-blocking feature.
• [COMPLEXITY]: 28 - The retained solution is two enforced toggles; the rejected complexity belongs to the unresolved atomic primitive.
• [EFFORT_PROFILE]: Maintenance - A narrow consumer cleanup after the premise correction.

This is the one comprehensive formal Request Changes cycle. Return the two-site head plus the focused one-publication witnesses; absent a new existential defect introduced by that repair, the expected next disposition is terminal approval.

[review-budget-managed]

• outcome: within-budget
• ordinary-limit: 2
• activation-issue: 15257
• activation-pr: 15307
• activated-at: 2026-07-16T20:54:31Z