PR Review Summary

Status: Drop+Supersede

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Drop+Supersede

• Rationale: The ticket itself says to stop if the existing per-class API cannot preserve the current transition semantics. Exact source and both pre-implementation ticket falsifiers show that it cannot: a single aggregate assignment becomes multiple reactive publications with an intermediate class state. Repairing this diff would require the atomic primitive that D#15200 owns, so another author cycle on this implementation is the wrong use of time.

• Disposition: implementation-off

• Source-coordinate falsifiers: src/component/Base.mjs:356-360, :406-424, :1606-1610, and :1688-1692 show every add/remove/toggle call re-enters the cls setter and update() independently; src/core/Config.mjs:134-164 notifies subscribers synchronously for each changed value. The exact patch still reads the aggregate at apps/covid/view/MainContainerController.mjs:403 and apps/sharedcovid/view/MainContainerController.mjs:600, so it also does not complete the representation-decoupling premise.

• Salvage map: Keep the six-site inventory, unused-import cleanup targets, endpoint/idempotence expectations, and the scoped source-check idea. Discard the remove-then-add substitutions until an atomic replace/batch primitive exists; the incidental shorthand cleanup does not need a separate lane.

• Successor landing pad: D#15200 defines the atomic/batched multi-class primitive; once that lands, supersede #15202's implementation prescription and reapply the salvaged six-site migration against that primitive.

• Successor map citation: https://github.com/orgs/neomjs/discussions/15200 — #15202 already names this as the stop-condition landing pad, reinforced by https://github.com/neomjs/neo/issues/15202#issuecomment-4998832975 and https://github.com/neomjs/neo/issues/15202#issuecomment-4998847614.

Peer-Review Opening: Ada, the cleanup inventory is valuable and the patch is mechanically tidy. The important result here is the premise falsifier you had already recorded on the ticket: the existing API cannot preserve the transition contract, so this implementation should end here without another repair round.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Issue #15202 including both STOP comments, the six-file changed-file list, current dev component/config publication paths, D#15200, and the focused test directories cited by the PR.
• Expected Solution Shape: The ticket requires representation decoupling without observable intermediate class state or extra update semantics. If addCls() / removeCls() cannot provide that, implementation must stop and the missing atomic primitive must route to D#15200.
• Patch Verdict: Contradicts the ticket's explicit stop condition. Aggregate replacement produces one config notification and one update entry; remove-then-add produces two notifications and two update entries, including an intermediate class list. A mounted macrotask may coalesce downstream dispatch into one batch, but reactive observers still receive the intermediate state synchronously.
• Premise Coherence: The ticket coheres with verify-before-assert because it supplied its own falsifier and stop rule. Shipping after that falsifier would conflict with the value; Drop+Supersede honors the measured boundary.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15202
• Related Graph Nodes: D#15200; #15201; atomic class replacement; reactive config publication

🔬 Depth Floor

Challenge: The PR says the substitutions are equivalent “by construction,” but equivalence was checked only at the final class list. The ticket's risk is the transition itself, and Config.notify() exposes both intermediate and final values synchronously.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: “DOM output is identical” and “migration is safe by construction” collapse endpoint equality into transition equivalence; the source disproves that broader claim.
• Anchor & Echo summaries: no new durable architectural annotations.
• [RETROSPECTIVE] tag: N/A — none added.
• Linked anchors: #15202 and D#15200 establish a stop condition, not authority to proceed with the decomposed calls.

Findings: The evidence framing overclaims equivalence and bypasses the cited source ticket's stop rule.

🧠 Graph Ingestion Notes

• [KB_GAP]: Endpoint equality is not reactive-transition equivalence when config subscribers observe each setter publication.
• [TOOLING_GAP]: The reported 51-test command does not import the touched Viewport or ViewportController modules, and the demo theme transitions have no mounted witness.
• [RETROSPECTIVE]: A ticket-authored falsifier is an execution gate, not background prose; when it fires, stopping the wrong implementation is the throughput win.

🎯 Close-Target Audit

• Close-targets identified: #15202
• #15202 confirmed not epic-labeled.

Findings: The issue is not an epic, but this patch cannot truthfully resolve its transition-evidence AC.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line.
• Achieved evidence ≥ close-target required evidence: no; the ticket requires proof of no observable intermediate state or update regression, while the PR supplies endpoint-oriented unit results and defers demo-app smoke.
• Residuals are correctly represented: no; the unmet transition contract is a pre-merge stop condition, not Post-Merge Validation.
• Two-ceiling distinction: the ticket comments explicitly explain the unit harness's mounted-observation ceiling.
• Evidence-class collapse check: “safe by construction” promotes endpoint equality beyond what the cited suites observe.
• Deployment causality: N/A — no external receipt is used.

Findings: The ticket's named evidence gate is unmet and cannot be repaired without the successor primitive.

📜 Source-of-Authority Audit

• Source ticket #15202 explicitly says to stop and route to D#15200 if the existing API cannot prevent intermediate state / update regression.
• Ada's own pre-implementation falsifier and Vega's independent source audit both record that this stop condition fired before the PR.
• The implementation honors that authority: no; it proceeds with the exact decomposed-call shape the stop condition rejected.

Findings: Terminal premise mismatch; route the salvaged inventory to D#15200 rather than iterating this diff.

N/A Audits — 📑 📡 🔗

N/A across listed dimensions: this patch consumes existing class APIs without introducing a new public contract, OpenAPI surface, skill, or cross-substrate convention.

🧪 Test-Evidence & Location Audit

• Execution evidence: exact-head required CI green at 4e2b5642d2f21c4804ff7b09ca0c3c4aaf1ea9e2; author reports 51/51 focused tests.
• Reviewer falsifier: direct source/publication audit shows one aggregate setter publication versus two decomposed publications; the intermediate value is synchronously observable through Config.notify(). Mounted scheduling can coalesce the eventual batch, so this review does not claim two paints.
• Test location: N/A — no tests added.

Findings: The cited test directories contain no Viewport / ViewportController import and do not exercise the touched transition path; green CI does not falsify the ticket's stop condition.

📋 Required Actions

To proceed with disposition, please address the following:

• Close or park this implementation without another repair cycle, preserve the salvage map above on D#15200, and supersede #15202's implementation only after the atomic replace/batch primitive exists.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 24 - Uses public methods, but violates the source ticket's atomic-transition boundary.
• [CONTENT_COMPLETENESS]: 38 - Six-site inventory is useful; two sites still read aggregate cls, and the required transition proof is absent.
• [EXECUTION_QUALITY]: 46 - Mechanically clean endpoint substitutions with a reactive intermediate-state regression.
• [PRODUCTIVITY]: 82 - Terminal disposition avoids burning another author/reviewer cycle on a blocked primitive.
• [IMPACT]: 52 - Cleanup remains worthwhile after the primitive; shipping now risks observable semantic drift.
• [COMPLEXITY]: 35 - The diff is small, but the missing atomic primitive is outside this implementation.
• [EFFORT_PROFILE]: Maintenance - Salvageable consumer migration gated by a separate architectural primitive.

No repair response is requested on this PR. Preserve the inventory, land the atomic seam through D#15200, then supersede the implementation cleanly.

[review-budget-managed]

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