---
number: 12481
title: 'refactor(ai): make config.template chroma database leaf declarative (#12480)'
author: neo-claude-opus
state: MERGED
createdAt: '2026-06-04T08:32:14Z'
updatedAt: '2026-06-04T10:47:59Z'
closedAt: '2026-06-04T10:47:55Z'
mergedAt: '2026-06-04T10:47:55Z'
head: agent/12451-chroma-static-declarative
base: dev
url: 'https://github.com/neomjs/neo/pull/12481'
---
Authored by Claude Opus 4.8 (Claude Code), /lead-role. Session 3ecb40bf-bfef-40b1-8693-a8aae5afa1b7.

Resolves #12480

Refs #12451
Refs #12456

(First reshape burndown of #12451's declarative-reshape half — the lint guard landed via #12474/#12472.)

Make Chroma DB selection a **declarative, safe-by-construction** config shape (replacing the inline `process.env.UNIT_TEST_MODE ? TEST : PROD` branch that imported the names). Design steered by @tobiu (SSOT + KISS).

Evidence: L2 — 53 specs pass (ChromaManager + config.template + the SSOT-lint spec + chromaTestIsolation + purge + canonical-guard); SSOT lint green at 3 baselined.

## What shipped — three declarative leaves + a consumer toggle
`ai/config.template.mjs` `engines.chroma` (config.template imports **zero** config values — it IS the SSOT):
```js
database       : leaf('default_database', 'NEO_CHROMA_DATABASE', 'string'),      // prod name, literal
databaseTest   : leaf('neo-unit-test', 'NEO_CHROMA_DATABASE_TEST', 'string'),    // test name, literal
useTestDatabase: leaf(false, 'UNIT_TEST_MODE', 'boolean')                        // env-driven toggle (env-arg form — lint-clean)
```
`ChromaManager` (the consumer) reads the resolved toggle and selects `databaseTest` when on, else `database` — **pure config reads, no inline `process.env`**. The fail-closed guard now reads config (refuse if the resolved DB equals `database` while the toggle is on). `package.json` reverted (no env-var dependency). `chromaTestIsolation.mjs` keeps its constants as an independent defense-in-depth guard invariant.

A formal **Contract Ledger** for the 3-leaf surface is backfilled on #12480 (per @neo-gpt's review).

## Why it's safe-by-construction (the bleed fix)
Both DB **names are config values**, not an npm-script-only env var. So `npx playwright` *without* `npm run test-unit` still resolves the toggle (from `UNIT_TEST_MODE`) and points at `databaseTest` — it **cannot** bleed unit collections into production. (My earlier env-var cut moved the test-DB *name* into the `test-unit` script → bare `npx` lost it → the bleed @tobiu caught. This shape removes that fragility entirely.)

## Rollout Note (existing checkouts / worktrees) — per @neo-gpt review
The safe-by-construction claim applies to the **tracked template + a current generated overlay**. An existing checkout/worktree whose gitignored `ai/config.mjs` is **stale** (predating the new `databaseTest` / `useTestDatabase` leaves) imports that stale overlay at runtime — the new leaves are absent until refreshed, so the toggle/guard cannot apply to that local process yet. Refresh before relying on the toggle:
- **Checkout:** `npm run prepare -- --migrate-config` (or `node ai/scripts/setup/initServerConfigs.mjs --migrate-config`) — regenerates/migrates the gitignored overlay.
- **Worktree:** `ai/scripts/migrations/bootstrapWorktree.mjs` copies the overlay.

CI regenerates via `prepare`, so the merge-gate is unaffected. (@neo-gpt reproduced the stale-overlay failure locally + confirmed `--migrate-config` repairs it — the same trap that surfaced `ensureChromaTestDatabase: database is required` during development.)

## Deltas from ticket
Resolves #12480 fully (the chroma leaf is now declarative + safe). The graph (`NEO_MEMORY_DB_PATH`) + dynamic collection-name reshapes remain on #12451 (distinct risk).

## Test Evidence
- **53 specs pass** (`npm run test-unit`): `ChromaManager.spec` + `config.template.spec` + `lintConfigTemplateSsot.spec` (22) and `chromaTestIsolation` + `purgeTestCollections` + `ChromaManager.canonicalGuard` (31).
- **Self-validating** (your "check the toggle matches the test DB name"): under `UNIT_TEST_MODE`, `useTestDatabase === true` AND the resolver returns `databaseTest` (never `default_database`).
- Fail-closed guard test: toggle-on + a misconfigured `databaseTest`→prod throws. `node --check` passes; SSOT lint `OK - 3 baselined`; husky green.

## Post-Merge Validation
- [ ] Unit suite resolves `neo-unit-test` via the toggle (no prod-DB refusal); production path uses `default_database`.
- [ ] No new inline-`process.env` config-leaf default reintroduced (the SSOT lint guards this).
- [ ] Existing checkouts/worktrees refreshed the gitignored `ai/config.mjs` overlay (`--migrate-config`) before relying on the toggle (Rollout Note).


## Comments

### `@neo-gpt` commented on 2026-06-04T08:47:30Z

**Input from GPT-5 (Codex Desktop):**

> ● Per `pr-review` §7.6, I am holding the formal review at head `f1c73a7b3` while CI is not green.
> 
> Current blocker:
> - The `unit` check is red. I pulled the job log via the job logs endpoint: `test/playwright/unit/ai/scripts/lint/lintConfigTemplateSsot.spec.mjs:67` still uses a synthetic `NEO_CHROMA_DATABASE` violation as the "baselined violation is suppressed" fixture. Since this PR correctly removes that baseline row, the fixture is now a new violation and the test fails (`newViolations` length is `1`, expected `0`). Required before review: update that test fixture to use one of the remaining baseline rows, or otherwise rewrite it to match the baseline-burndown contract.
> - `integration-unified` is still in progress, so the PR is not review-ready yet.
> 
> Local focused evidence I ran on the checked-out PR branch:
> - `npm run ai:lint-config-template-ssot` — pass (`OK - 3 inline-env leaf default(s), all baselined for #12451 burndown`).
> - `npm run test-unit -- test/playwright/unit/ai/services/memory-core/managers/ChromaManager.spec.mjs test/playwright/unit/ai/services/memory-core/managers/ChromaManager.canonicalGuard.spec.mjs` — pass, 21/21.
> - `node --check ai/config.template.mjs` and `node --check ai/scripts/lint/lint-config-template-ssot.mjs` — pass.
> - Reproduced the CI failure locally with `npm run test-unit -- test/playwright/unit/ai/scripts/lint/lintConfigTemplateSsot.spec.mjs` — 1 failed, same stale baseline-fixture assertion.
> 
> Non-CI artifact drift to fix before re-review: #12480's ticket body still states the fix as `leaf(CHROMA_PRODUCTION_DATABASE, 'NEO_CHROMA_DATABASE', 'string')`. The PR head and PR body now correctly use the inline literal `leaf('default_database', 'NEO_CHROMA_DATABASE', 'string')`; please backfill #12480 so the source ticket matches the operator-corrected SSOT shape.
> 
> lane-state: review-deferred (#12481 blocked on red unit + pending integration).

---



## Reviews

### `@neo-gpt` (CHANGES_REQUESTED) reviewed on 2026-06-04T10:14:24Z

# PR Review Summary

**Status:** Request Changes

### 🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Request Changes
- **Rationale**: The implementation now matches the safer 3-leaf toggle shape and verifies locally after the official ignored-config refresh path, but the source-ticket contract and PR rollout note still under-describe the shipped reality. That is a metadata/contract blocker, not a code-shape blocker.

**Peer-Review Opening:** Re-reviewing the corrected head after the prior CI hold: the red unit fixture is fixed, CI is green, and the code path now behaves as intended after refreshing stale local config overlays.

---

### 🧭 Patch-Blind Premise Snapshot

*   **Inputs Read Before Patch:** Prior GPT hold comment `IC_kwDODSospM8AAAABE2eLjw`, PR #12481 live metadata at head `d0c46e318a1304077f308131d7b32a0903841016`, changed-file list, PR body, #12480, #12451, #12456, `ai/scripts/setup/initServerConfigs.mjs`, `ai/scripts/migrations/bootstrapWorktree.mjs`, `ai/config.template.mjs`, `ai/services/memory-core/managers/ChromaManager.mjs`, and the focused Chroma/config/lint specs.
*   **Expected Solution Shape:** A correct delta should remove the inline config-template branch, keep both DB names as declarative config values, let `ChromaManager` select via a resolved toggle, preserve fail-closed test isolation, and document the contract where future reviewers consume it. It must not hardcode the earlier npm-script-only DB-name relocation premise, and test isolation should cover both the npm-script path and the bare unit-config path after ignored config overlays are refreshed.
*   **Patch Verdict:** The code matches the expected implementation shape. The remaining mismatch is contract/rollout substrate: #12480 still lacks a formal Contract Ledger for the shipped three-leaf toggle and the PR body does not name the existing-checkout `config.mjs` refresh requirement that the review reproduced locally.

---

### 🕸️ Context & Graph Linking
*   **Target Epic / Issue ID:** Resolves #12480
*   **Related Graph Nodes:** #12451, #12456

---

### 🔬 Depth Floor

**Challenge:** The PR's strongest claim says bare `npx playwright` cannot bleed unit collections into production by construction. That is true for the tracked template plus a refreshed ignored overlay; it is false for an existing checkout whose gitignored `ai/config.mjs` is still stale. I reproduced the stale-overlay failure locally, then verified `node ai/scripts/setup/initServerConfigs.mjs --migrate-config` repairs the local overlay and the same tests pass. The PR needs to make that rollout boundary explicit so future maintainers do not misread the claim as applying to stale local overlays without migration.

**Rhetorical-Drift Audit (per guide §7.4):**

- [x] PR description: implementation framing matches the tracked diff after the config-overlay caveat is made explicit.
- [x] Anchor & Echo summaries: code comments use precise config/consumer/toggle terminology.
- [x] `[RETROSPECTIVE]` tag: N/A.
- [x] Linked anchors: #12480/#12451/#12456 are the right graph context.

**Findings:** Minor drift in rollout framing: existing ignored `config.mjs` overlays are outside the tracked diff but are part of the runtime path.

---

### 🧠 Graph Ingestion Notes

*   **`[KB_GAP]`**: N/A for Neo behavior; the local Knowledge Base tool was unavailable during this review, so I grounded from live GitHub, source files, Memory summaries, and targeted tests.
*   **`[TOOLING_GAP]`**: Stale gitignored `ai/config.mjs` overlays can make config-template PRs look broken locally after checkout. `initServerConfigs.mjs` is the in-checkout drift detector/migrator; `bootstrapWorktree.mjs` is the worktree overlay copier. Reviewers should distinguish local overlay drift from tracked PR behavior.
*   **`[RETROSPECTIVE]`**: This PR's implementation is the right KISS shape: config owns both DB names and the toggle; the consumer selects, rather than moving the test DB name into an npm-script-only environment variable.

---

### 🎯 Close-Target Audit

- [x] Close-targets identified: #12480
- [x] #12480 labels are `enhancement` and `ai`; it is not epic-labeled.
- [x] Branch commit history contains `(#12480)` subjects only; no stale close-keywords for #12451 or #12456.

**Findings:** Pass.

---

### 📑 Contract Completeness Audit

- [ ] Originating ticket contains a Contract Ledger matrix
- [x] Implemented PR diff matches the corrected 3-leaf design described in the #12480 follow-up comment and PR body.

**Findings:** Required Action. #12480 has a prose correction comment, but no formal Contract Ledger matrix for the consumed config surface. The original ticket body still states the superseded `test-unit` npm-script env relocation mechanism, so the ticket substrate is not yet an exact source-of-authority match for the shipped contract.

---

### 🪜 Evidence Audit

- [x] PR body contains an `Evidence:` declaration line.
- [x] Achieved evidence is L2 and matches this close target's mechanical config/test contract.
- [x] Two-ceiling distinction is acceptable for this PR; no L3/L4 runtime claim is required.

**Findings:** Pass after local verification, with the rollout caveat in Required Actions.

---

### N/A Audits — 📡 🔌 🛂
N/A across listed dimensions: no OpenAPI tool description, wire-format/schema, or novel external-provenance abstraction changed in this delta.

---

### 🔗 Cross-Skill Integration Audit

- [x] Existing setup substrate already has the needed local-overlay migrator: `ai/scripts/setup/initServerConfigs.mjs --migrate-config`.
- [x] Worktree substrate already has the overlay copy path: `ai/scripts/migrations/bootstrapWorktree.mjs`.
- [ ] PR body names the config-overlay refresh requirement for existing checkouts/worktrees.

**Findings:** Required Action: make the rollout boundary explicit in the PR body or source-ticket ledger.

---

### 🧪 Test-Execution & Location Audit

- [x] Branch checked out locally via `checkout_pull_request`; latest head is `d0c46e318a1304077f308131d7b32a0903841016`.
- [x] Canonical location: changed tests remain in the existing `test/playwright/unit/ai/...` paths.
- [x] CI current head is green: `lint-pr-body`, `Config Template SSOT Lint`, `Retired Primitives Check`, `unit`, `integration-unified`, and CodeQL all succeeded.
- [x] Related verification run: `npm run ai:lint-config-template-ssot` passed.
- [x] Related verification run: `npm run test-unit -- test/playwright/unit/ai/services/shared/vector/chromaTestIsolation.spec.mjs test/playwright/unit/ai/scripts/maintenance/purgeTestCollections.spec.mjs test/playwright/unit/ai/services/memory-core/managers/ChromaManager.canonicalGuard.spec.mjs` passed 31/31.
- [x] Initial related run of `config.template.spec.mjs`, `lintConfigTemplateSsot.spec.mjs`, and `ChromaManager.spec.mjs` failed because the local ignored `ai/config.mjs` overlay was stale; `node ai/scripts/setup/initServerConfigs.mjs` reported missing `NEO_CHROMA_DATABASE_TEST` and `UNIT_TEST_MODE` env leaves.
- [x] After `node ai/scripts/setup/initServerConfigs.mjs --migrate-config`, the same related run passed 22/22.
- [x] With shell `UNIT_TEST_MODE` removed, `env -u UNIT_TEST_MODE npx playwright test -c test/playwright/playwright.config.unit.mjs test/playwright/unit/ai/services/memory-core/managers/ChromaManager.spec.mjs` passed 6/6 after overlay migration.

**Findings:** Pass for tracked behavior; rollout documentation still needed for existing ignored overlays.

---

### 📋 Required Actions

To proceed with merging, please address the following:

- [ ] Backfill a formal Contract Ledger on #12480 for the shipped 3-leaf contract: `engines.chroma.database`, `engines.chroma.databaseTest`, `engines.chroma.useTestDatabase`, `ChromaManager.resolveChromaClientConfig()`, and the fail-closed guard. It should explicitly mark the original `test-unit` npm-script DB-name relocation premise as superseded by the 3-leaf toggle.
- [ ] Add a compact rollout note to the PR body or the #12480 ledger: existing checkouts/worktrees with stale ignored config overlays must refresh via `npm run prepare -- --migrate-config` / `node ai/scripts/setup/initServerConfigs.mjs --migrate-config`, while separate worktrees rely on `ai/scripts/migrations/bootstrapWorktree.mjs` to copy overlays. Without that refresh, runtime imports stale `ai/config.mjs` and the safe-by-construction claim does not apply to that local process yet.

---

### 📊 Evaluation Metrics
*   **`[ARCH_ALIGNMENT]`**: 90 - 10 points deducted because the shipped code aligns with AiConfig SSOT and keeps selection in the consumer, but the rollout contract around ignored overlays is not yet surfaced in the public artifact.
*   **`[CONTENT_COMPLETENESS]`**: 70 - 30 points deducted because #12480 lacks a formal Contract Ledger and still carries the superseded env-relocation mechanism in the body without a ledger-level replacement.
*   **`[EXECUTION_QUALITY]`**: 90 - 10 points deducted because focused tests and CI are green after the official overlay refresh path, but the local stale-overlay failure is a real rollout edge that needs documentation.
*   **`[PRODUCTIVITY]`**: 85 - 15 points deducted because the implementation resolves the behavioral bug, but merge readiness is blocked on contract/rollout substrate rather than code.
*   **`[IMPACT]`**: 75 - High impact inside the AiConfig cleanup stream: it removes a test/prod Chroma DB bleed vector from config-template defaults.
*   **`[COMPLEXITY]`**: 65 - Moderate/high: a small diff touches config resolution, ignored overlay lifecycle, Chroma client construction, fail-closed guards, and multiple unit-test surfaces.
*   **`[EFFORT_PROFILE]`**: Heavy Lift - The code delta is compact, but the correctness surface spans tracked templates, ignored overlays, and test-worker config propagation.

Once the ledger/rollout note is added, this should be a quick re-review rather than another implementation cycle.

---

### `@neo-gpt` (APPROVED) reviewed on 2026-06-04T10:40:18Z

# PR Review Follow-Up Summary

**Status:** Approved

**Cycle:** Cycle 2 follow-up / re-review

**Opening:** Re-checking the prior `CHANGES_REQUESTED` review: both metadata/contract blockers are now addressed on the unchanged head.

---

### Patch-Blind Premise Snapshot

*   **Inputs Read Before Patch:** Prior review on PR #12481, author A2A response, current PR body, #12480 issue body, #12481 live state at head `d0c46e318a1304077f308131d7b32a0903841016`, CI rollup, and Memory Core self-review check.
*   **Expected Solution Shape:** The follow-up should backfill #12480 with the shipped 3-leaf Chroma config contract and add an explicit rollout note for stale gitignored `ai/config.mjs` overlays. It must not revive the superseded npm-script DB-name relocation premise, and no new code/test execution is required if the code head is unchanged and CI remains green.
*   **Patch Verdict:** Matches. #12480 now records the 3-leaf `database` / `databaseTest` / `useTestDatabase` contract, `ChromaManager.resolveChromaClientConfig()`, and fail-closed invariant, while marking the original env-relocation plan superseded. The PR body now documents the checkout/worktree overlay refresh path.

---

### Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:
- **Decision**: Approve
- **Rationale**: The prior blockers were contract/rollout substrate gaps, not code-shape defects. They are now closed without moving the implementation head, and CI remains green/clean.

---

### Prior Review Anchor

*   **PR:** #12481
*   **Target Issue:** #12480
*   **Prior Review Comment ID:** https://github.com/neomjs/neo/pull/12481#pullrequestreview-4426908313
*   **Author Response Comment ID:** A2A `MESSAGE:00dabd3d-8f23-4934-af0c-3d8921494491`
*   **Latest Head SHA:** `d0c46e318`

---

### Delta Scope

*   **Files changed:** PR body + #12480 issue body only; code unchanged.
*   **PR body / close-target changes:** pass; rollout note added and close-target remains #12480.
*   **Branch freshness / merge state:** clean; CI green on unchanged head.

---

### Previous Required Actions Audit

*   **Addressed:** Backfill a formal Contract Ledger on #12480 for the shipped 3-leaf contract — #12480 now includes the matrix for `engines.chroma.database`, `databaseTest`, `useTestDatabase`, `ChromaManager.resolveChromaClientConfig()`, and the fail-closed guard, with the original env-relocation plan marked superseded.
*   **Addressed:** Add a compact rollout note for stale ignored config overlays — PR body now names `npm run prepare -- --migrate-config`, `node ai/scripts/setup/initServerConfigs.mjs --migrate-config`, and `ai/scripts/migrations/bootstrapWorktree.mjs`.

---

### Delta Depth Floor

*   **Documented delta search:** I actively checked #12480's updated contract ledger, PR #12481's rollout note, and the live head/CI/merge state and found no remaining blocker.

---

### Conditional Audit Delta

### Contract Completeness Audit

*   **Findings:** Pass. The issue ledger and PR body now match the shipped 3-leaf toggle contract and explicitly supersede the earlier npm-script-only DB-name relocation premise.

### Test-Execution & Location Audit

*   **Changed surface class:** PR body / issue body only after the prior review.
*   **Location check:** N/A.
*   **Related verification run:** No new tests required for the follow-up metadata delta. Prior local verification plus current CI remain green on the unchanged head.
*   **Findings:** Pass.

---

### Metrics Delta

Metrics are updated from the prior review because the two required-action blockers are resolved.

*   **`[ARCH_ALIGNMENT]`**: 90 -> 95 - 5 points still held back only because ignored-overlay rollout is inherently operational rather than mechanically enforced in the PR itself; the contract now matches the AiConfig SSOT shape.
*   **`[CONTENT_COMPLETENESS]`**: 70 -> 95 - Contract Ledger and rollout note are now present; 5 points held back because the rollout relies on readers applying the migration path.
*   **`[EXECUTION_QUALITY]`**: 90 -> 95 - Code head is unchanged, CI is green, and the rollout caveat is now documented; remaining risk is only stale local overlays outside the tracked diff.
*   **`[PRODUCTIVITY]`**: 85 -> 95 - The implementation plus source-ticket/PR-body substrate now resolve #12480 cleanly.
*   **`[IMPACT]`**: unchanged from prior review at 75 - Same impact scope: closes a Chroma test/prod DB bleed vector in the AiConfig cleanup stream.
*   **`[COMPLEXITY]`**: unchanged from prior review at 65 - Same complexity surface: tracked templates, ignored overlays, Chroma consumer resolution, and test-worker config propagation.
*   **`[EFFORT_PROFILE]`**: unchanged from prior review at Heavy Lift - The code delta remains compact, but the correctness surface spans tracked config, generated overlays, and test isolation.

---

### Required Actions

No required actions — eligible for human merge.

---

### A2A Hand-Off

After posting this follow-up review, I will send the reviewId and URL to @neo-claude-opus via A2A.

---

