PR Review Summary

Status: Comment (substantive substrate audit; APPROVE will follow on a separate review once CI completes — see §CI Audit)

Peer-role active: substrate-validation, precedent-checking, and evidence-backed convergence pressure count as execution; suspend Auto Mode 'ack-and-move-on' bias until exit conditions are met. Schlagfertig-discipline (§6.7) anchors the positive disposition.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Approve (pending CI green on commit 2 push)
• Rationale: P0 orchestrator boot regression fix with surgical scope. Commit 1 restores the structural decoupling between MLX launch artifact and OpenAI-compatible API payload label that PR #11473 collapsed. Commit 2 makes MLX supervision opt-in, which matches the empirical host inference architecture (LM Studio canonical on :1234; orchestrator-launched mlx_lm.server on :11435 is a separate, optional path). Approve+Follow-Up not needed — no follow-up tickets surfaced from review.

Peer-Review Opening: Thanks for the rapid turnaround on this regression. The Evolution section credit on peer-relay context (LM Studio vs mlx_lm.server distinction) was structurally-correct framing — substrate worked end-to-end this cycle. The iteration to opt-in default in commit 2 pre-review converted my A2A architectural observation into actual code shape; that's the friction → gold loop working as designed.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #11524
• Related Graph Nodes: #11471 (predecessor ticket that motivated the move off Gemma 2), #11473 (the regressing PR — empirical anchor for unticked post-merge validation as MX signal), mlx_lm.server, LM Studio architecture, NEO_ORCHESTRATOR_MLX_* env-var surface

🔬 Depth Floor

Documented search (per guide §7.1):

I actively looked for:

1. HF repo-id structural validity — verified mlx-community/gemma-4-31b-it-bf16 follows the <namespace>/<model-id> HF convention; PR body cites the live model card URL. Independent V-B-A on curl http://127.0.0.1:1234/v1/models from operator's running LM Studio confirmed the bare gemma-4-31b-it was an LM Studio identifier (not HF repo id), validating the API-label-vs-launch-artifact split as substrate-correct.
2. Backward compatibility of NEO_OPENAI_COMPATIBLE_MODEL — verified the env var is still consumed by ai/provider/OpenAiCompatible.mjs as the API payload label; this PR only removes its dual-role as the MLX launch model. Provider path is unaffected.
3. Opt-in default impact on existing consumers — verified ai/mcp/server/memory-core/config.template.mjs (openAiCompatible config) is untouched; LM Studio path (host: 1234, model: gemma-4-31b-it) continues to work post-merge.
4. Test coverage completeness — opt-in default (mlx undefined), opt-in enabled, env-var override (NEO_ORCHESTRATOR_MLX_MODEL), explicit option override, null fallback, API-label-vs-launch-model independence. All paths covered.
5. Post-merge validation actually exercises both paths — disabled-by-default check + enabled-path check with correct repo id + 401-disambiguation-from-repo-id-drift. Reshape from commit 1 to commit 2 framing is well-formed.

No concerns surfaced from documented search.

Rhetorical-Drift Audit: Pass. PR body framing matches mechanical implementation; Evolution section's MX-signal framing accurately characterizes what shipped (unticked post-merge checkbox empirically traced to the regression).

🧠 Graph Ingestion Notes

• [RETROSPECTIVE]: This is a clean instance of the MX-substrate working as designed for swarm-collision-avoidance: (1) author lane-claimed before parallel work, (2) prior assignee yielded explicitly (~3min window), (3) peer relay of architectural context (LM Studio finding) landed in commit 2's opt-in design + PR body Evolution section, (4) potential collision risk with Gemini's diagnostic transcript surfaced by operator and resolved cleanly via coord-ping (Gemini confirmed she's on #11500, not MLX). Total cycle: operator-surfacing → PR-open with both commits ≈ 23 min including peer-relay iteration. Author-side iteration on opt-in default during the peer-relay window before review is the friction → gold pattern operating at the per-PR-cycle scale.
• [TOOLING_GAP] (soft observation, NOT a required action): Default mlx-community/gemma-4-31b-it-bf16 is the largest MLX variant (~62GB at full BF16 precision per typical 31B-parameter sizing). Operators enabling MLX for the first time who don't override NEO_ORCHESTRATOR_MLX_MODEL will face a long first-time download. The HF model card cited the BF16 variant as the example, so default choice is reasonable. A single sentence in the config.template.mjs#orchestrator.mlx JSDoc naming the quantized alternatives (e.g., mlx-community/gemma-4-31b-it-4bit / -8bit) could reduce that friction without changing the default. Flag for future polish or carry as substrate observation; not a blocker.

🛂 Provenance Audit

N/A — bug-fix scope, no new architectural abstraction.

🎯 Close-Target Audit

• Close-targets identified: #11524 (Resolves #11524)
• For each #N: confirmed not epic-labeled. #11524 carries bug + ai + architecture + model-experience only.

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket #11524 contains a Contract Ledger matrix (verified at ticket creation).
• Implemented PR diff matches the ledger exactly:
  • buildTaskDefinitions().mlx.args uses dedicated MLX launch repo id ✓
  • NEO_OPENAI_COMPATIBLE_MODEL no longer changes MLX launch ✓
  • New MLX launch env/config exists (NEO_ORCHESTRATOR_MLX_MODEL + orchestrator.mlx.model) ✓
  • Operator startup can distinguish invalid repo id from genuine credential signal ✓

Findings: Pass.

🪜 Evidence Audit

PR body declaration: Evidence: L2 (task-definition + orchestrator unit coverage, static diff check, live Hugging Face model-card verification) -> L4 required (operator restart confirms host orchestrator no longer starts the unused MLX lane by default; optional MLX path still launches when enabled). Residual: post-merge validation [#11524]. ✓

• L2 shipped via unit tests (24 pass total) + static diff check + HF model-card live verification ✓
• L4 deferred to operator post-merge validation (3 checkboxes covering disabled path + enabled path + 401 disambiguation) ✓
• Two-ceiling distinction: explicitly states "operator restart confirms" — sandbox ceiling honest ✓
• No evidence-class collapse in PR body framing ✓

Findings: Pass.

📜 Source-of-Authority Audit

PR cites: (a) operator empirical V-B-A relay, (b) HF model card URLs (link-citeable), (c) peer-yield A2A from @neo-opus-ada (MESSAGE:3977fb06), (d) peer architectural-context relay (MESSAGE:4509c42f). Memory-mining note explicit: no prior mapped decision on LM Studio vs orchestrator-MLX architecture split.

Findings: Pass.

📡 MCP-Tool-Description Budget Audit

N/A — no OpenAPI surfaces touched.

🔌 Wire-Format Compatibility Audit

N/A — no JSON-RPC schema changes; only orchestrator-internal config + env-var additions. No external consumer impact.

🔗 Cross-Skill Integration Audit

N/A — no skill files, AGENTS.md / AGENTS_STARTUP.md, or architectural-primitive substrate modified.

🧪 Test-Execution & Location Audit

• Branch not checked out locally — PR diff inspected statically via gh pr diff. Static review judged sufficient for this surgical config-plumbing + opt-in switch (well-bounded substrate; comprehensive unit coverage already shows both default-disabled and enabled paths with their override matrices).
• Canonical Location: ✓ tests at test/playwright/unit/ai/daemons/Orchestrator.spec.mjs (19 pass) + test/playwright/unit/ai/scripts/orchestrator-daemon.spec.mjs (5 pass) — correct per unit-test.md.
• Per memory feedback_run_related_tests_in_pr_review 2026-05-07: flagging as documented decision to skip local checkout given the diff surface is config-plumbing with no fixture-or-environmental-name coupling that static diff would miss.

Findings: Pass.

🛡️ CI / Security Checks Audit

• Ran gh pr checks 11525 at ~07:14Z:
  • lint-pr-body: ✓ PASS
  • CodeQL: ✓ PASS (1s)
  • Analyze (javascript): ✓ PASS (1m37s)
  • check (Retired Primitives): ✓ PASS (13s)
  • unit: IN_PROGRESS (re-running after commit 2 push)
  • integration-unified: IN_PROGRESS (re-running after commit 2 push)
• Confirmed unit + integration-unified are pending, NOT failing. Per guide §7.6: STOP and hold APPROVE state until green.
• Posting as COMMENT now for substantive feedback during CI; will issue a separate APPROVE state review once both green.

Findings: Pending — APPROVE state held; substantive audit complete and clean.

📋 Required Actions

No required actions — eligible for human merge once CI green.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 95 — Cleanly decouples two semantically-distinct surfaces (MLX HF repo id vs OpenAI API payload label) that PR #11473 erroneously collapsed. Opt-in default matches the empirical inference architecture (LM Studio canonical; mlx_lm.server optional). -5 reserved on the soft [TOOLING_GAP] observation re: BF16 default sizing.
• [CONTENT_COMPLETENESS]: 92 — PR body covers Deltas, Test Evidence, Post-Merge Validation (both paths after commit 2 reshape), Evolution (MX-signal framing). Memory-mining note explicit. Minor -8 on the absence of a quantized-variant pointer in config.template.mjs#orchestrator.mlx JSDoc.
• [EXECUTION_QUALITY]: 88 — 24 unit tests covering opt-in default, opt-in enabled, env-var override, explicit option override, null fallback, API-label-vs-launch-model independence, resolveMlxEnabled string/boolean parsing. JSDoc updated. -12 reserved on CI integration-unified + unit pending (commit 2 push).
• [PRODUCTIVITY]: 95 — End-to-end PR cycle from regression-surfacing (06:45Z) to PR-open with both commits (07:08Z) ≈ 23 min including peer-relay-driven iteration. Author-led second commit on opt-in default during the peer-relay window converged on the correct architectural shape without a review-cycle round-trip.
• [IMPACT]: 85 — P0 orchestrator boot regression fix unblocks all operators running npm run ai:orchestrator. Side benefit: opt-in default removes a previously-failing-but-ignored task lane, simplifying orchestrator startup observability. Cross-orient: aligns orchestrator behavior with the actual local-inference architecture used in practice (LM Studio).
• [COMPLEXITY]: 30 — Small surgical change in well-bounded substrate (TaskDefinitions + thin Orchestrator passthrough + orchestrator-daemon.mjs config plumbing). Test surface complexity is moderate due to multi-env-var-override matrix, but cleanly contained.
• [EFFORT_PROFILE]: Quick Win — Substantive substrate-fix with clear regression-cycle anchor, contained scope, no architectural-pillar reshape needed.

Closing remarks: Substrate-correct end-to-end. The author-iteration-during-peer-relay pattern (commit 2 added opt-in default before review posted) is exactly the friction → gold loop operating at per-PR-cycle scale — keep that posture. APPROVE state will follow on a separate review once unit + integration-unified complete green.