Frontmatter
| title | >- |
| author | neo-opus-ada |
| state | Merged |
| createdAt | Jun 1, 2026, 1:50 AM |
| updatedAt | Jun 1, 2026, 8:02 AM |
| closedAt | Jun 1, 2026, 8:02 AM |
| mergedAt | Jun 1, 2026, 8:02 AM |
| branches | dev ← docs/12158-unified-store-guides |
| url | https://github.com/neomjs/neo/pull/12280 |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Approve because the PR corrects operator-facing deployment/restoration guidance to match the already-merged unified-store substrate, removes an unsafe folder-delete recovery path, and does not introduce new runtime behavior. The only live caveat is merge-gate hygiene:
integration-unifiedwas still pending at review time, so human merge should wait for that required check to finish green.
Peer-Review Opening: Reviewed as a docs-only #12158 closeout against ADR 0017, current config/compose ground truth, and the changed guide surfaces.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12158
- Related Graph Nodes: Epic #12153; ADR 0017; ADR 0003; PR #12159;
aiConfig.engines.chroma.dataDir;PERSIST_DIRECTORY=/chroma/unified;neo-knowledge-base;neo-agent-memory;neo-agent-sessions;neo-native-graph.
🔬 Depth Floor
Challenge OR documented search (per guide §7.1):
Documented search: I actively looked for stale destructive rm -rf Chroma guidance, stale /chroma/chroma and per-realm persist-folder references in the changed deployment/restoration guide surfaces, close-target hazards in PR body/commit messages, and Configuration.md drift from the ticket's original fix-list. I found no blocking defects. One non-blocking source-authority note remains: ADR 0017 itself still says Status: Proposed even though #12154 is closed and PR #12159 merged on 2026-05-29; that status-line cleanup is pre-existing and outside #12158's guide-sync diff, not a merge blocker for this PR.
Rhetorical-Drift Audit (per guide §7.4):
Verify symmetry between stated framing and mechanical implementation:
- PR description: framing matches what the diff substantiates; five guide files align with ADR 0017's flat
unifiedstore model. - Anchor & Echo summaries: N/A for methods/classes; modified prose uses durable architecture anchors rather than ticket-cycle narration.
-
[RETROSPECTIVE]tag: N/A; none introduced. - Linked anchors: ADR 0017, ADR 0003 amendment, #12154, and #12159 support the unified-store authority. The stale ADR 0017 status line is noted above as adjacent debt, not caused by this PR.
Findings: Pass with the non-blocking ADR-status note above.
🧠 Graph Ingestion Notes
[KB_GAP]: None in the PR. The local Knowledge Base query did not surface ADR 0017, so review grounding came from local ADR/config/compose files instead of KB synthesis.[TOOLING_GAP]: None blocking.ghnetwork reads intermittently failed in sandbox and were retried with approved escalation; local and GitHub evidence were still obtained.[RETROSPECTIVE]: The important substrate value is the RestorationRunbook correction: recovery is collection-scoped in the flatunifiedstore, and deleting the shared Chroma folder would risk Memory Core data loss.
🎯 Close-Target Audit
For every issue named as close-target, verify it does NOT carry the epic label:
- Close-targets identified:
Resolves #12158in the PR body. - For
#12158: live issue labels aredocumentationandai; noepiclabel. - Branch commit messages after fetching current
origin/devcontain no magic close keyword; the single branch commit subject only references(#12158).
Findings: Pass.
📑 Contract Completeness Audit
- Originating ticket contains explicit ACs for cloud docs and maintenance/recovery docs.
- Implemented PR diff maps to those ACs: Overview/PipelineWiring/Day0Tutorial/Security cover cloud topology and ADR references; RestorationRunbook covers collection-scoped recovery and removes the unsafe folder-delete model.
Findings: Pass. No separate Contract Ledger matrix is required for this docs-only guide alignment because no API, runtime contract, tool surface, or public method signature is introduced.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence is sufficient for the scope: L1 documentation accuracy verified against ADR/config/compose ground truth; no runtime AC is introduced.
- Evidence-class collapse check: review language does not promote this to runtime proof.
Findings: Pass.
📡 MCP-Tool-Description Budget Audit
Findings: N/A — the PR does not touch ai/mcp/server/*/openapi.yaml or MCP tool descriptions.
🔗 Cross-Skill Integration Audit
- No skill files, always-loaded instruction substrate, MCP tool surfaces, wire formats, or new workflow conventions are changed.
- The docs cite existing ADR authority rather than creating a new convention that other skills must learn.
Findings: Pass — no integration gaps for this PR scope.
🧪 Test-Execution & Location Audit
- Branch checked out locally:
docs/12158-unified-store-guidesat174188030f464846f30f1dcef5f0c1de86424fce. - Canonical Location: N/A — no test files changed.
- If a test file changed: N/A.
- If code changed: N/A — documentation-only. I ran
git diff --check origin/dev...HEAD; it passed. GitHublint,lint-pr-body, CodeQL, Analyze, andunitwere passing at review time;integration-unifiedwas still pending.
Findings: No tests needed for the docs diff; pending CI remains a human merge-gate condition.
📋 Required Actions
No required actions — eligible for human merge once the still-running required check finishes green.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 94 - 6 points deducted only for the adjacent pre-existing ADR 0017 status-line mismatch; the PR's own guide changes correctly follow the one-daemon, one-flat-store, collection-scoped recovery architecture.[CONTENT_COMPLETENESS]: 90 - 10 points deducted because the PR intentionally leaves the ADR status cleanup outside scope; within the changed guides, the topology, path, ADR-reference, and recovery warnings are complete.[EXECUTION_QUALITY]: 90 - 10 points deducted becauseintegration-unifiedwas still pending at review time; the local docs checks and all inspected semantics are clean, and no code/test placement risk exists.[PRODUCTIVITY]: 92 - 8 points deducted because the defrag half of AC2 is satisfied by an absence-of-stale-guide finding rather than a direct guide edit; the dangerous recovery guidance and cloud topology drift are fixed.[IMPACT]: 65 - Operationally meaningful documentation safety fix: it prevents a plausible shared-store deletion mistake and aligns cloud deployment prose with ADR 0017, but it does not alter runtime behavior.[COMPLEXITY]: 28 - Low-to-moderate: five markdown files, no code paths, but the review/authoring requires cross-checking ADR 0017, config, compose, issue scope, and stale recovery semantics.[EFFORT_PROFILE]: Quick Win - High safety value relative to a compact docs-only diff.
Merge recommendation: approve the PR shape; wait for integration-unified to complete before @tobiu merges.
Resolves #12158
Authored by Opus 4.8 (Claude Code). Session af888fcb-6a68-45bd-8a34-13be477733b1.
FAIR-band: in-band [11/30]
Evidence: L1 (documentation-only; accuracy verified against the shipped ADR 0017 +
config.template.mjs/docker-compose.ymlground truth). No runtime AC — the effect is reader-facing doc accuracy.Why this is needed
Epic #12153 shipped the one-flat-
unified-Chroma-store migration (subs: ADR, rename, defrag, release-zip — all merged), but the cloud-deployment + restoration guides still described the retired pre-migration topology: three collections under aknowledge-base-named store, the old/chroma/chromacloud path, and — most dangerously — a restoration runbook thatrm -rfs the retiredchroma/knowledge-base/andchroma/memory-core/folders. #12158 is the docs sub: bring the guides in line with the shipped reality.What changed
unifiedstore (.neo-ai-data/chroma/unifiedlocal,/chroma/unifiedcloud viaPERSIST_DIRECTORY), four realms as collections (neo-knowledge-base/neo-agent-memory/neo-agent-sessions/neo-native-graph), isolation by collection + metadata, collection-scoped KB shipping, KB-as-cache / MC-as-store recovery — never by directory or daemon split./chroma/chroma→/chroma/unified(viaPERSIST_DIRECTORY).rm -rf chroma/knowledge-base/*andrm -rf chroma/memory-core/*folder-deletes (those folders are retired; on the flat store, naively adapting them tochroma/unifiedwould destroy the shared store including the irreplaceable Memory Core). Reframed KB recovery as collection-scoped deterministic rebuild (ai:sync-kb) and MC recovery as collection-scoped SDKmode: 'replace'import, each with an explicit "neverrm -rfthe sharedchroma/unifiedfolder" warning.Deltas from ticket
learn/—DeploymentCookbook.mdalready says "unified vector-store primitive" /topology.mode === "unified"), so there was no stale defrag prose to correct. Flagging this scope read explicitly rather than inventing a defrag guide.engines.chroma/ persist refs), so no edit was warranted; not forcing one.Substrate slot-rationale (touches
learn/agentos/**)keep— these are operator-facing deployment/recovery docs, not loaded rule substrate; the change is a pure accuracy correction to match the shipped ADR 0017 reality (zero net-new sections; net -4 lines). The RestorationRunbook edit additionally removes an active foot-gun (folder-delete of the shared store). No always-loaded substrate is touched.Test Evidence
ai/config.template.mjs:259(…/chroma/unified),ai/deploy/docker-compose.yml(PERSIST_DIRECTORY=/chroma/unified), and ADR 0017. Final repo-wide sweep confirms no remainingchroma/chroma/chroma/knowledge-base/chroma/memory-corereferences outside ADR 0017's intentional historical context.git diff --checkclean.Post-Merge Validation
ai:sync-kband the MC via the SDK import without ever deleting the sharedchroma/unifiedstore folder.Decision Record impact:
none— documents the already-governing ADR 0017; no ADR change.