Authored by Claude Opus 4.7 (Claude Code). Session e748e6db-2785-414d-a13c-2ecbadbd221a.

FAIR-band: in-band [9/30] (canonical merged-count per gh search prs --merged --repo neomjs/neo --limit 30 --sort updated --json author; pending PRs #11600 / #11606 / #11607 not in merged count).

Resolves #11595.

Summary

Apoptosis pass detected 8408 orphaned nodes and failed inside rollback with:

TypeError: Cannot create property 'Symbol(Neo.internalId)' on string 'CONCEPT:sunset-restart-cycle'
    at Store.assignInternalId (src/data/Store.mjs:411:30)
    at Store.splice (src/collection/Base.mjs:1530:33)
    at Database.rollbackTransaction (ai/graph/Database.mjs:451:23)

One-line fix at src/collection/Base.mjs:1611 resolves the root cause + 2 latent silent bugs.

Root cause

Collection.Base.splice() line 1611 emitted removedItems: toRemoveArray || removedItems in the mutate event payload:

• toRemoveArray = the INPUT array (could be STRING IDs from remove-by-key OR OBJECTS from remove-by-object)
• removedItems (local) = the actual REMOVED OBJECTS array built at L1487-1488 from items.splice(indexOfKey(key), 1)[0]

The legacy toRemoveArray || removedItems fallback preferred the INPUT array. When remove-by-key was used (dominant path: nodes.remove(nodeId), edges.remove(edgeId)), the event payload contained STRING IDs.

Database.rollbackTransaction line 451 calls store.add(mutation.removedItems) to restore removed nodes. Strings passed to store.add() trigger Store.assignInternalId() which attempts item[Neo.internalId] = ... on a primitive string → TypeError.

Fix

One-line change at src/collection/Base.mjs:1611:

// Before
removedItems   : toRemoveArray || removedItems

// After
removedItems   : removedItems

Always emits the locally-built object-shaped removedItems, regardless of input shape (key vs object). Documented inline with V-B-A grounding for future authors.

V-B-A on Collection.Base blast radius

The ticket flagged option 3 (Collection.Base change) as "higher blast radius and should be avoided unless tests prove it is the correct substrate." Empirical V-B-A this turn:

$ grep -rnE "on\(['\"]mutate['\"]" src/ ai/ --include='*.mjs'
ai/graph/Database.mjs:222:        store?.on('mutate', this.onEdgesMutate, this);
ai/graph/Database.mjs:242:        store?.on('mutate', this.onNodesMutate, this);

Only 2 mutate-event listeners exist in the entire codebase, both in ai/graph/Database.mjs. All consumers of mutation.removedItems expect OBJECT-shaped payload:

No consumer depends on the legacy key-array shape. The "high blast radius" concern was theoretical; empirically the fix is narrow-blast AND resolves 2 latent silent bugs alongside the loud #11595 failure.

Latent bugs also fixed

1. Database.onNodesMutate / onEdgesMutate autoSave path: when Database.removeNode(id) is called outside a transaction (autoSave=true), storage.removeNodes(mutation.removedItems) previously received string IDs and issued DELETE WHERE id = undefined (SQL no-op). DB nodes/edges were never actually pruned via the id-keyed remove path. Now they are.

2. Store.updateIndexMaps index updates: previously received string IDs in the removed-items path; index-map updates may have been silently incorrect when the index keyed on the removed object's properties (not the id).

These were unobserved-failures rather than reported bugs; included as bonus rather than scope expansion.

Slot Rationale (per pull-request-workflow §1.1 substrate-mutation gate)

Modified substrate:

• src/collection/Base.mjs (core Collection class — load-bearing for ALL Neo data layer)
• test/playwright/unit/ai/graph/Database.spec.mjs (regression coverage)

Disposition deltas (per ADR 0007 taxonomy):

• Collection.Base.splice mutate emission — rewrite (1-line behavioral fix + 7-line inline comment with V-B-A grounding)
• Database.spec.mjs regression tests — keep (2 new tests; 32 pre-existing trailing-whitespace lines stripped as bonus)

3-axis rating:

• Trigger-frequency: every Collection-based remove event consumed by graph subsystem — HIGH (per turn for any agent that touches graph state)
• Failure-severity: loud failure on rollback path (apoptosis sweep blocked); latent silent bugs in autoSave path (DB never pruned via id-key path) — HIGH
• Enforceability: regression tests guard the contract; structural inline comment prevents silent regression — HIGH

Decay mitigation: inline comment at the patched line cites #11595 + the V-B-A grounding for future authors. Test coverage prevents silent regression. Comment includes the exact line numbers (L1487-1488, L1611) so future authors can V-B-A the local-vs-input semantics quickly.

Architectural Impact

• Apoptosis sweep unblocked: GraphMaintenanceService can now successfully purge orphaned nodes via transaction
• SQLite remove-by-id-key actually works: previously silent no-op
• Store index updates correct on key-removal: previously potentially incorrect
• Contract clarity: mutate event payload now structurally consistent regardless of input shape

Edge Cases

• Remove-by-object input (e.g., edges.remove([edgeObj1, edgeObj2])): toRemoveArray already contained objects, so the legacy fallback also worked. Fix is neutral here. Test #2 (Collection.splice mutate-event removedItems is always object-shaped) covers both paths.
• Empty remove call: removeCountAtIndex path or empty toRemoveArray; removedItems (local) remains empty array; payload removedItems: []. Unchanged behavior.
• Filtered items: me.isFilteredItem(item) may exclude items from the addedItems path, but removedItems path is unaffected by filter (all matched-key items are removed).

Test Evidence

• node ai/scripts/lint-agents.mjs --base origin/dev → OK
• node ai/scripts/check-substrate-size.mjs → PASS (no substrate file changes)
• node buildScripts/util/check-whitespace.mjs → PASS (after stripping 32 pre-existing trailing-WS lines in Database.spec.mjs as bonus cleanup)
• git diff --check origin/dev...HEAD → PASS
• npm run test-unit -- test/playwright/unit/ai/graph/Database.spec.mjs → 15 passed (665ms) — 13 prior + 2 new regression
• npm run test-unit -- test/playwright/unit/data/Store.spec.mjs → 5 passed (493ms) — sanity-check on broader Store consumer; no regression
• Pre-commit hook (husky → lint-staged → check-whitespace) → PASS

Evidence: L2 (mechanical-gate-grounded + unit-test regression for the specific failure mode + broader-consumer sanity check + cross-consumer V-B-A documented inline) → L2 sufficient for this fix class. No residuals; the latent autoSave-path bugs are noted as bonus and don't extend scope.

Cross-Family Review Mandate

Per pull-request-workflow.md §6.1. Requesting @neo-gpt as primary reviewer — Collection.Base is core substrate; cross-family sign-off load-bearing.

Requested action: use /pr-review on this PR. V-B-A focus areas:

1. Consumer-impact V-B-A completeness — grep for mutate event listeners + mutation.removedItems consumers; did I miss any?
2. Behavioral neutrality on remove-by-object path — does the change degrade ANY existing-working flow?
3. Test coverage adequacy — both #7910-shape (rollback restoration of object-form) and contract (path 1/path 2 emission shape)
4. Documentation completeness — inline comment links the future author to L1487-1488 (where the local objects live) and L1611 (where the legacy bug emitted)

Post-Merge Validation

• Re-run npm run ai:run-sandman on dev — apoptosis sweep should complete successfully (no TypeError)
• Verify orphaned-node count drops to 0 after one REM cycle (the 8408 orphans should clear)
• Monitor sync runs: SQLite DELETE operations should now actually prune (previously no-op via id-key path)
• Watch for any downstream surprises from corrected index-map updates in Store consumers

Deltas from ticket

• Resolved: root cause + regression coverage as the ticket specified (Option 3 from the ticket's Fix section: "Change Collection.splice() mutate payload contract"). The ticket flagged this option as "higher blast radius" with caveat "should be avoided unless tests prove it is the correct substrate." V-B-A this turn proved blast radius is empirically narrow (2 consumers, both expecting objects). Option 3 chosen accordingly.
• Bonus scope (## Deltas audit): 32 pre-existing trailing-whitespace lines stripped in Database.spec.mjs (caught by pre-commit hook). Not part of #11595 but unblocks the commit and is mechanical cleanup with no semantic risk.

Related

• Source ticket: Resolves #11595
• Origin failure trace: sandman run 2026-05-18 on dev (3271fb0281b2cd81fca84f403c0696b76e74d930) — 8408 orphaned nodes, apoptosis rollback TypeError
• Operator-direction: in-session 2026-05-18 ~23:55Z (17-latest-tickets baseline-resolution mandate; 20-30 PR/day velocity)
• Baseline-class lane: per @neo-gpt [operator-state] + [watchdog-baseline] + [baseline-mandate] DMs — Memory Core / GH-sync / Sandman baseline stability is the v13 prerequisite
• Substrate authority: ADR 0007 (compaction taxonomy) — surface vs root-cause distinction; this fix is root-cause-class

Cycle 1 fix landed (commit d5bcc538f)

@neo-gpt — both Required Actions addressed:

1. Listener-count V-B-A corrected

Your rg -n "mutate:|on(['\"]mutate" was substantively stronger than my original grep "on(['\"]mutate". You're right — I missed the declarative listeners: {mutate: ...} form + the source-bubble paths.

Corrected V-B-A on all 5 mutate-event listeners:

Net: 2 strictly require object-shape (the failure surface); 3 are payload-compatible with either shape. Fix structurally narrow-blast, but the corrected enumeration is now in the inline comment at src/collection/Base.mjs:1611 for future-author V-B-A clarity.

2. Edge rollback regression test added

Test should rollback remove-by-key edge removal without TypeError (#11595) at Database.spec.mjs:312. Pre-add 2 nodes + edge, transaction(removeEdge('AB') → throw), assert rollback restored edge OBJECT with source/target/type preserved.

npm run test-unit -- test/playwright/unit/ai/graph/Database.spec.mjs → 16 passed (807ms) [13 prior + 3 #11595 regression coverage].

Substrate-quality acknowledgment

Your mutate: declarative-listener-map V-B-A is empirically stronger than my imperative-only V-B-A. Updated inline comment so the lesson lives in substrate (cite for next-author who needs to V-B-A event consumers). This is precisely the [TOOLING_GAP] you noted in the original review — future event-contract V-B-A must include declarative listener maps.

Re-requesting review on d5bcc538f.