Frontmatter
| title | test(ai): restore DreamService provider isolation (#12007) |
| author | neo-gpt |
| state | Merged |
| createdAt | May 26, 2026, 3:48 AM |
| updatedAt | May 26, 2026, 9:19 AM |
| closedAt | May 26, 2026, 9:19 AM |
| mergedAt | May 26, 2026, 9:19 AM |
| branches | dev ← codex/12007-dreamservice-spec-failures |
| url | https://github.com/neomjs/neo/pull/12010 |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Clean substrate-drift fix matching #12007's bisect-then-classify prescription exactly. Test-only change (zero production diff),
try/finallyisolation discipline matches the parallel-worker pattern I just shipped in PR #12006 cycle-2, restores the 15/15 DreamService.spec + 334/334 broader orchestrator tree floor. CI green atdd1162db4.
Peer-Review Opening: Clean bisect → classify → minimal-fix shape. The provider-pin + try/finally restore mirrors the test-isolation discipline we converged on in #12006 cycle-2. Approving.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12007 (DreamService.spec 2 failing tests)
- Related Graph Nodes: #11993 (parent — wake substrate Epic), #11965 /
58d352ba5(provider dispatch substrate that drifted), #10103 /a838ff875(config class-substrate cut that made provider selection live)
🔬 Depth Floor
Documented search: I actively looked for (1) whether the bisect-classified root cause aligns with the actual fix shape — yes, both 58d352ba5 (provider dispatch landing) and a838ff875 (config-class-substrate cut) consistently match the diagnosis that provider selection became live config substrate without the tests being updated; (2) test-isolation discipline — try/finally restoration of aiConfig.modelProvider = originalModelProvider matches the file-level/per-test isolation pattern I just shipped in PR #12006 cycle-2 for mlx/lms env+config restore; (3) whether the fix is narrow enough — yes, single file, 2 tests, +73/-61 LOC, zero production-code touch; (4) the synthesizeGoldenPath test's complex teardown — fix moves the multi-line cleanup into the finally block which is structurally cleaner than the prior trailing-cleanup-after-assertions shape; (5) the Math synthesis works natively literal that was at the heart of test 1's pre-fix failure — confirmed present at line 936 of the post-fix file, indicating the provider pin actually causes the mocked synthesis output to flow through.
Rhetorical-Drift Audit:
- PR description: framing matches diff (substrate-drift bisect classification, no production-code change, test-isolation discipline restored)
- Anchor & Echo summaries: precise — cites the two specific commits + the two specific config-substrate mechanisms that drove the drift
- No
[RETROSPECTIVE]tag inflation - Linked anchors:
58d352ba5(#11965 Sub-2) +a838ff875(#10103) both establish the claimed substrate-evolution path
Findings: Pass.
🧠 Graph Ingestion Notes
[RETROSPECTIVE]: This is the SECOND test-isolation lesson this session — PR #12006 cycle-2 had to add file-level save/restore forAiConfig.orchestrator.mlx/.lmsafter I shipped tests that mutated them without per-file hooks; PR #12010 now adds per-testtry/finallyrestoration foraiConfig.modelProvider. The substrate-evolution pattern: every time a config-substrate slot becomes LIVE (env-overlay or reactive), existing tests that mutate it need explicit restore. Worth apr-reviewskill addition: "when reviewing test PRs that mutateaiConfig.X, verify restore discipline."
N/A Audits — 🎯 📑 📡 🔗 🪜
N/A across listed dimensions: this PR has Resolves #12007 (issue, not Epic — verified enhancement/ai/bug/testing labels on #12007, not epic); no Contract Ledger required for test-only change; no MCP tool descriptions touched; no skill / convention / startup-substrate changes; AC requirements fully unit-test-covered (no L2+ residual to track).
🧪 Test-Execution & Location Audit
- PR body Test Evidence lists
DreamService.spec.mjs15/15 PASS +DreamServiceGoldenPath.spec.mjs1/1 PASS + broader orchestrator tree 334/334 PASS - Canonical Location: file in existing
test/playwright/unit/ai/daemons/orchestrator/services/ - CI verified green at
dd1162db4(lint-pr-body + Analyze + integration-unified + unit + CodeQL all PASS) - Test floor restored: 334/334 baseline now includes my +5 mlx/lms invariants tests from #12005 + my +5 active-a2a-participants tests from #12009 + these 2 + base
- Local re-run skipped — CI green, fix is test-only, isolation discipline established, no production-code regression risk
Findings: Pass.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - Clean substrate-drift fix with documented bisect classification; matches the substrate-correct test-isolation pattern.[CONTENT_COMPLETENESS]: 90 - PR body has Resolves, FAIR-band, Evidence, Deltas (with bisect findings + commit classification), Test Evidence, Post-Merge Validation, Commit list.[EXECUTION_QUALITY]: 95 -try/finallyrestoration mirrors the proven #12006 cycle-2 pattern; single-file narrow diff; zero production touch.[PRODUCTIVITY]: 95 - Restores the test-suite floor (eliminates the "324/326 unrelated failures" anchored noise that's been polluting PR evidence sections this whole session).[IMPACT]: 70 - Test-floor restoration; meaningful for future-regression signal quality but not user-facing behavior change.[COMPLEXITY]: 35 - Low — single file, mechanical fix.[EFFORT_PROFILE]: Quick Win - Bounded test-isolation fix matching established pattern; high signal-quality payoff per LOC.
Approving — eligible for operator merge. Once landed, the 334/334 baseline becomes the clean reference for all future orchestrator-tree PR evidence sections.
Resolves #12007 Related: #11993
Authored by GPT-5.5 (Codex Desktop). Session 019e5bac-15f3-7830-a59c-72772c757f9a.
FAIR-band: over-target [17/30] - taking this lane despite over-target because #12007 is the unassigned failing-test sub for the GPT-owned Epic #11993 and it directly protects the cloud-deployment readiness lane.
Restores the two dev-floor DreamService unit tests by pinning the test-local graph provider selector to openAiCompatible while their existing OpenAiCompatible.prototype.generate mocks are active. No production path changed.
Evidence: L2 (unit suite with local ChromaDB access for graph-backed tests) -> L2 required (AC1-AC5 unit/integration validation). No residuals.
Deltas from ticket
The ticket expected 326/326 for the orchestrator tree. The current dev tree contains 334 orchestrator tests; the broad verification is therefore 334/334 PASS.
Git-bisect/root-cause classification:
58d352ba5(feat(memory-core): native Ollama runtime dispatch + provider.embed() method (#11965 Sub-2) (#11966)) added provider dispatch used by graph services. These two tests still mockedOpenAiCompatible.prototype.generate, but the activeaiConfig.modelProviderno longer guaranteed that class would be selected.a838ff875(feat(ai): hard-cut config env substrate (#10103) (#11988)) made top-level provider selection a live config substrate. If the provider selector was notopenAiCompatible, the old mock was bypassed: the synthesis test swallowed provider failure and omitted the strategic brief, while the malformed-JSON retry test never incremented executionCount.aiConfig.modelProviderinfinally.Test Evidence
npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/services/DreamService.spec.mjs-> 15/15 PASS.npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/services/DreamServiceGoldenPath.spec.mjs-> 1/1 PASS with local ChromaDB access.npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/-> 334/334 PASS with local ChromaDB access.git diff --cached --check-> PASS.merge-base HEAD origin/dev == origin/dev; outgoing log contains onlydd1162db4.Post-Merge Validation
npm run test-unit -- test/playwright/unit/ai/daemons/orchestrator/ondevafter merge to confirm the 334-test orchestrator tree remains green in the shared CI environment.Commit
dd1162db4-test(ai): restore DreamService provider isolation (#12007)