Status: Approved

Cross-family review (Claude → GPT) — this satisfies the §6.1 cross-family gate. Eligible for human merge.

🪜 Strategic-Fit Decision

• Decision: Approve
• Rationale: Correct, verified, well-tested fix of a real silent-failure core bug; restores the add/remove symmetry invariant and is strictly more robust than the prior name-only matcher. Two non-blocking observations below, neither a substrate-correctness defect → not Request Changes; no follow-up ticket warranted → not Approve+Follow-Up.

Peer-Review Opening: Clean, surgical fix — the normalization + reference-match closes the object-valued {fn} asymmetry exactly, and the regression net (object / scope / string) is the right coverage. Notes below are non-blocking.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #13102 (the bug), current dev src/core/Observable.mjs#removeListener, the on()/add path it must mirror, the existing ObservableListenersDiff.spec (#7091), src/core/Base.mjs as the quality bar.
• Expected Solution Shape: removeListener must normalize object-valued {fn, scope} specs (read value.fn/value.scope) symmetrically with how they're stored, preserve string-handler + function removal, and add regressions for object/scope/string — without breaking the #7091 diff-based teardown.
• Patch Verdict: Matches + improves. fn = valueObject ? value.fn : value + entryScope normalization closes the bug; the added listener.fn === fn reference clause makes it MORE correct than the old name-only match (now handles anonymous + reference identity). Confirmed: 8/8 spec incl. the 3 new regressions passes locally @ f42212bf.

🕸️ Context & Graph Linking

• Target Issue ID: Resolves #13102
• Related Graph Nodes: #7091 (the diff-based afterSetListeners this matcher serves)

🔬 Depth Floor

Challenge (2 non-blocking edges):

1. The old matcher read value.name, which coincidentally matched a {name: 'onA'}-shaped object; the new code requires value.fn for object specs, so a {name: ...}-object removal form (if any caller uses it) silently stops matching. The documented object form is {fn, scope}, so this is almost certainly a non-issue — worth a quick confirm there's no {name}-object remove caller.
2. The name fallback listener.fn?.name === fn?.name can still match a different function sharing the same .name + scope if it appears earlier in the array. This is pre-existing (the old code was name-only and worse) and not worsened — the new reference clause improves the common case. Flagging for the record, not as a blocker.

Rhetorical-Drift Audit (§7.4): PR body ("normalizing {fn, scope} before matching, while preserving legacy function-name and string-handler removal paths") accurately describes the diff. No drift.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: Classic add/remove-asymmetry bug — the store path accepted {fn, scope} but the remove path read value.name (undefined for objects), so object-valued listeners silently leaked. Reference-identity matching (listener.fn === fn) is the durable fix; name-matching is the legacy fallback for string handlers.

N/A Audits — 📑 🪜 📡 🛂 🔌 🧠 🔗

N/A across listed dimensions: contained core bug fix, no formal Contract Ledger surface (the "contract" is add/remove symmetry — verified); ACs fully unit-covered (Evidence L1, as declared); no openapi.yaml; not a new abstraction (provenance: gpt session declared); no wire-format/schema change; no substrate-load files; no new convention/skill/tool.

🎯 Close-Target Audit

• Resolves #13102 (newline-isolated); no other close-keywords.
• #13102 is a bug leaf (not epic).

Findings: Pass.

🧪 Test-Execution & Location Audit

• Checked out codex/13102-observable-object-un @ f42212bf (=== PR head) in my opus-vega clone.
• Canonical location: test/playwright/unit/core/.
• Ran ObservableListenersDiff.spec.mjs → 8 passed (683ms) — the 3 new #13102 regressions (object-valued removal, object un() scope match, string-valued removal) + the existing #7091 diff suite.
• CI: CLEAN.

Findings: Tests pass; the object-valued-removal fix is empirically verified locally.

📋 Required Actions

No required actions — eligible for human merge (cross-family gate satisfied by this review). The two Depth-Floor edges above are non-blocking observations; address only if a {name}-object remove caller actually exists.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — restores the add/remove symmetry invariant; reference-identity match is more correct than the legacy name-only path; preserves string-handler removal. 5 deducted: drops the coincidental {name}-object match without a confirm-no-caller note.
• [CONTENT_COMPLETENESS]: 92 — added @summary; clear Fat-PR body with full test evidence + delta rationale; object/scope/string regressions. 8 deducted: neither JSDoc nor body notes the {name}-object behavior change.
• [EXECUTION_QUALITY]: 95 — verified 8/8 locally @ PR head + CI clean; reference match now handles anonymous fns the old name-only path missed. 5 deducted: name-fallback same-name+scope collision edge (pre-existing, not worsened).
• [PRODUCTIVITY]: 100 — fully fixes #13102 (object-valued {fn} removal) + bonus string-valued regression coverage; I confirmed each spec maps to a bug facet.
• [IMPACT]: 70 — core listener-lifecycle correctness affecting every Observable consumer that removes object-valued listeners; a real silent-failure (leaked listeners) class.
• [COMPLEXITY]: 25 — Low: a localized normalization + matcher change in one method (+68/-2 incl. tests); contained reasoning.
• [EFFORT_PROFILE]: Quick Win — high correctness ROI (silent core bug) at low complexity.