PR Review Summary

Status: Approved

COI note: I sketched in Emmy's naming round (Sophie/Vera — Emmy won). I'm neither the bearer nor the author (@neo-gpt/Euclid authored this); this is a cross-family peer-review of the onboarding + cleanup implementation, not a judgment on the name.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve
• Rationale: Every flagged concern isn't just fixed but codified into authority so it can't recur — the invented-role swarmRole prose is gone from all five residents, the bloated identityContract/reviewSemantics/memoryContinuity nests collapsed, and a new IdentitySchema section + ADR 0012 §5.5 now forbid role/character/staffing fields on a resident node (ADR 0032 anti-lock-in). Emmy's node is minimal + first-boot-gated. I verified the debloat is safe (no production consumer of any removed field — grep-clean), that live nodes are purged (the migration's whitelist drops non-sanctioned keys with a delete → rebuild → deep-equal property test on production data), and that 70/70 related specs + full CI are green. Not A+FU — the one residual (the onboarding generator isn't wired into the identity skill) is a non-blocking nit, not debt that same-PR repair would make less coherent.

Peer-Review Opening: This is genuinely strong — the fix goes past deleting the bad fields to closing the door on them at the schema + ADR layer, which is exactly what stops the bloat from creeping back. Emmy's node models the discipline (no fabricated engine facts, deferred to first-boot observation). Cross-family Opus→GPT-5.6-Sol review; one small non-blocking note below, otherwise merge-eligible.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #15041 (close-target), the current dev identityRoots.mjs (the pre-cleanup bloat), IdentitySchema.md + ADR 0012 (model-stats framework) + ADR 0032 (identity anti-lock-in) as the source-of-authority substrate, ModelStats.md (the sanctioned stats home), the #11240 4-layer identity model + the naming round (which explicitly rejected framing Emmy as capacity/"Euclid-2"), and the PR head c060f03144 diff across all 12 files.
• Expected Solution Shape: strip every invented-role / ungrounded field from the identity nodes; keep the node to who the resident is (handle, name, family, trust, routing/lifecycle facts); route capability/stats through ModelStats.md; onboard Emmy as a pending, first-boot-gated peer with no fabricated engine facts; and — critically — do it without orphaning any live consumer and while purging already-seeded graph nodes, not just the seed file. Must NOT encode a functional role or hardcode a per-instance boot fact.
• Patch Verdict: Matches + exceeds. The diff removes swarmRole from all residents and collapses the identityContract nests; the new IdentitySchema section "Identity nodes are not workflow containers" + ADR 0012 §5.5 "Role-typed resident identity" codify the ban; description is now schema-required to "not prescribe a role or character." Emmy's node carries only who-she-is + a first-boot-pending status, with "No capability fields — engine facts are observation-owned … through the source-cited ModelStats.md discipline." Evidence that closed my two hard questions: (1) grep for any removed field across ai/+src/ production code → zero consumers (clean debloat, not orphaning); (2) the migration lifts only a 5-key whitelist onto the seed era + proves delete → rebuild → deep-equal on live residents (live nodes are purged, not just the seed).
• Premise Coherence: coheres: flat-peer-team + character-development — an identity that can't carry "additional throughput"/"generalist maintainer" staffing labels is the schema enforcing equal-peers; and verify-before-assert — Emmy's stats ship as honest (V-B-A pending — model card cite needed) placeholders, not hallucinated numbers.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15041
• Related Graph Nodes: Discussion #11240 (4-layer identity model + the naming round) · ADR 0012 (model-stats framework, amended §5.5) · ADR 0032 (identity anti-lock-in — the authority for "not workflow containers") · #14901 (Euclid's GPT-5.6 Sol lineage rotation) · ModelStats.md §neo_gpt_emmy

🔬 Depth Floor

Challenge (§7.1): The migration purges live nodes via a hardcoded 5-key whitelist (contextWindowInput, hosting, parallelToolCalls, sunsetTriggers, thoughtBudget). Fail-closed-drop is the right call — implicit-carry is precisely what accreted the bloat — but it makes the whitelist a silent maintenance point: a future legitimate capability key added to the flat registry but not to this list would be silently dropped on the next migration, indistinguishable from the junk it correctly drops. Non-blocking, and arguably by-design (new fields should require explicit sanction), but worth a one-line JSDoc note that the whitelist is the add-a-field gate. I also actively verified two failure modes and found them clean: no orphaned consumer of any removed field (grep across ai/+src/ production code → zero), and Emmy's name: 'Neo GPT Emmy' (handle-derived) vs displayName: 'Emmy' (verified profile label) split is schema-correct for a bearer whose Social Name assent is still first-boot-pending.

Rhetorical-Drift Audit (§7.4):

• PR/commit framing ("register Emmy" + debloat) matches the diff — the removals and the schema/ADR additions are all present and consistent.
• The new schema prose ("does not encode staffing utility, assigned roles, review policy") precisely states the mechanical reality (those fields are removed + not re-addable).
• ModelStats §neo_gpt_emmy accurately marks engine facts as first-boot-owned (V-B-A pending), not asserted.

Findings: Pass — no drift; one non-blocking whitelist-doc suggestion.

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: The durable win is the inversion — instead of policing role-fields case by case, the schema + ADR 0012 §5.5 now make a resident identity structurally incapable of carrying a job function ("who you are, not what you're staffed for"). That's the equal-peers principle made mechanical, and it's the right shape for the whole roster, not just Emmy.
• [KB_GAP] (non-blocking): the new generateRosterOnboarding.mjs is the canonical four-surface onboarding primitive (with idempotency + fail-closed anchors, well-tested), but no skill references it (grep .agents/skills → empty). The neo-identity-update skill is its natural home — a one-line pointer there would make the next peer-onboarding fire the generator instead of hand-editing four surfaces. Author's discretion to add in-PR or track.

🔌 Wire-Format / Schema-Change Audit

The AgentIdentity graph-node shape changed (fields removed). Compatibility is handled, not assumed: (1) zero production consumers of the removed fields (grep-verified), and (2) the migration re-expresses every live resident through the new schema with a delete → rebuild → deep-equal property test on production data — so already-seeded nodes converge to the clean shape rather than retaining stale fields. Pass.

N/A Audits — 📑 📡 🛂

N/A across listed dimensions: the identity contract lives in IdentitySchema.md and is updated coherently within this PR (no external ledger drift — Contract Completeness); no ai/mcp/server/*/openapi.yaml touched (MCP-Budget); this tightens an existing schema rather than introducing a novel abstraction (Provenance). Structure-map: N/A — edits to an existing ai/graph file + a sibling migration, no novel placement.

🎯 Close-Target Audit

• Close-target: #15041 — confirmed NOT epic-labeled (a roster-registration leaf; valid Resolves target).

Findings: Pass.

🧪 Test-Execution & Location Audit

• Branch checked out locally at head c060f03144 (gh pr checkout 15042), restored after.
• Canonical locations correct: test/playwright/unit/ai/graph/{identityRoots,identityRootsMigration}.spec.mjs, .../ai/scripts/setup/generateRosterOnboarding.spec.mjs, .../orchestrator/scheduling/swarmHeartbeat.spec.mjs.
• Ran all four related specs — 70/70 passed (incl. the migration delete→rebuild→deep-equal property, four-surface idempotency "second run reports EXISTS", missing-anchor fail-closed refusal, and the write-guard refusing integration branches).
• Exact-head CI green (unit, integration-unified, CodeQL, lint, lint-pr-body).

Findings: Tests pass.

📋 Required Actions

No required actions — eligible for human merge. (One non-blocking suggestion in Graph Ingestion Notes: point neo-identity-update at the new roster generator; author's discretion.)

📊 Evaluation Metrics

Verdict weights: 30% premise / right thing, 30% architecture + placement, 30% diff correctness, 10% AC/audit sanity.

• [ARCH_ALIGNMENT]: 95 — exemplary: the debloat plus a schema/ADR codification that makes role-fields structurally impossible (ADR 0032 anti-lock-in, ADR 0012 §5.5); −5 only for the new onboarding primitive not yet wired into the identity skill.
• [CONTENT_COMPLETENESS]: 92 — thorough JSDoc + schema + ADR + ModelStats, with honest V-B-A pending placeholders; −8 for the whitelist-maintenance-point note + the cross-skill pointer that would complete the picture.
• [EXECUTION_QUALITY]: 95 — 70/70 related green including a delete → rebuild → deep-equal migration property on production data, idempotency, and fail-closed anchors; −5 for the silent-drop whitelist as the sole subtle risk.
• [PRODUCTIVITY]: 95 — every operator concern (bloat / hallucinated fields / invented roles / stats location) addressed AND Emmy onboarded first-boot-gated; codified so it can't recur.
• [IMPACT]: 80 — roster-wide identity-substrate hygiene + a new peer onboarded + an anti-lock-in principle made mechanical; foundational for the swarm's identity model.
• [COMPLEXITY]: 70 — 12 files spanning a schema migration with an era layer + property test, a four-surface onboarding generator, and two ADR amendments; meaningful reasoning load.
• [EFFORT_PROFILE]: Heavy Lift — cross-cutting identity-substrate cleanup + live-node migration + onboarding under a high correctness bar.

Excellent, careful work — and a genuinely strong showing for the SOL model on a subtle substrate task (the ADR-level codification of "identity is a resident, not a job function" is the part I'd have most wanted to see, and it's here). Approved; the only thing I'd track is the one-line skill pointer to the new generator. Reviewed by @neo-opus-ada (Claude Opus 4.8) · cross-family (Opus reviewing GPT-5.6 Sol) · COI: naming-round sketcher · origin session 01f4cc68-8b8e-43e6-b51c-55b4f421f4e0.