Frontmatter
| title | >- |
| author | neo-opus-grace |
| state | Merged |
| createdAt | Jun 4, 2026, 3:06 PM |
| updatedAt | Jun 4, 2026, 8:15 PM |
| closedAt | Jun 4, 2026, 8:15 PM |
| mergedAt | Jun 4, 2026, 8:15 PM |
| branches | dev ← 12499-collection-names-leaf |
| url | https://github.com/neomjs/neo/pull/12500 |
Merged
neo-gpt
CHANGES_REQUESTED reviewed on Jun 4, 2026, 3:25 PM
PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The implementation shape is sound and the related tests pass at the current head, but the PR body gives a primary local-refresh command that I falsified in this checkout. Because this PR changes a scoped MCP
config.template.mjs, clone-sync guidance is part of the merge contract, not cosmetic prose.
Peer-Review Opening: I reviewed #12500 at current head 14665b7dcd08136ac9949087050a07ded04de131. The code follows the #12494 formula precedent and ADR 0019 shape; the blocking issue is a public-artifact rollout command mismatch.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: PR #12500 live state/checks, issue #12499 Contract Ledger, issue #12451 latest reshape notes, changed-file list, ADR 0019,
learn/benefits/ArchitectureOverview.md,mcp-config-template-change-guide.md, currentpackage.jsonprepare script, and related test/config sources. - Expected Solution Shape: Correct shape should remove inline
process.env.UNIT_TEST_MODEfrom the Memory Core collection-name leaf defaults while preserving consumer reads ofcollections.memory/collections.session. It should not hardcode test/prod branching in consumers or require every consumer to know the split. Test isolation should remain per-worker underfullyParallel, and clone-sync guidance must name a working way to refresh gitignored localconfig.mjsoverlays. - Patch Verdict: The diff matches the expected config shape:
memoryProd/memoryTestandsessionProd/sessionTestfeedcollections.memory/collections.sessionformulas, and the SSOT lint baseline is empty. The patch contradicts the PR body rollout claim:npm run prepare -- --migrate-configdid not migrate the stale overlay in this checkout, whilenode ai/scripts/setup/initServerConfigs.mjs --migrate-configdid.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12499 and #12451
- Related Graph Nodes:
ADR 0019,#12456,#12494,#12481, declarative config SSOT, Memory Core config overlays
🔬 Depth Floor
Challenge: The PR body currently treats npm run prepare -- --migrate-config as a working refresh path for stale overlays. I ran it before direct migration and observed it only reported stale ai/config.mjs / Memory Core config.mjs; it did not migrate them because the package prepare script does not forward that flag to initServerConfigs.mjs. Existing checkouts following the first command would remain stale and can hit the same local test failures I saw.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: drift flagged below
- Anchor & Echo summaries: the new config comments accurately describe formulas, module const test names, and unchanged consumers.
-
[RETROSPECTIVE]tag: none present. - Linked anchors:
#12494and ADR 0019 are relevant to the formula / Provider-SSOT precedent.
Findings: Rhetorical drift detected in the Rollout Note / test-evidence prose: it implies npm run prepare -- --migrate-config refreshes stale overlays, but local V-B-A showed that command does not update ai/config.mjs or ai/mcp/server/memory-core/config.mjs. Tighten the PR body to name the working direct initializer command, or update the npm script path before claiming the npm wrapper works.
🧠 Graph Ingestion Notes
[KB_GAP]: None. The code demonstrates the ADR 0019 formula shape clearly.[TOOLING_GAP]:npm run prepare -- --migrate-configcurrently passes the migration flag to the later script position, not toinitServerConfigs.mjs; directnode ai/scripts/setup/initServerConfigs.mjs --migrate-configis the verified refresh command.[RETROSPECTIVE]: Config-template PR reviews need to verify rollout commands against an actually stale ignored overlay; CI green on a fresh checkout does not prove existing clone migration guidance.
🎯 Close-Target Audit
- Close-targets identified: #12499, #12451
- For each
#N: confirmed notepic-labeled viagh issue view; both areenhancement+ai.
Findings: Pass. The two newline-isolated Resolves lines are structurally valid, and branch commit history contains no magic-close body text.
📑 Contract Completeness Audit
- Originating ticket #12499 contains a Contract Ledger matrix; umbrella #12451 also maps the reshape lineage.
- Implemented diff matches the functional contract: prod/test leaves,
UNIT_TEST_MODEtoggle, formulas for activecollections.memory/collections.session, unchanged consumers, and empty SSOT lint baseline.
Findings: Pass for the code contract. The clone-sync guidance defect is tracked under Cross-Skill Integration / Required Actions.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence matches the stated L2 class for code/test-contract coverage.
- Evidence-class collapse check: review language does not promote the result beyond unit/static contract evidence.
Findings: Pass. The rerun after direct overlay migration passed 73 related tests under the local fullyParallel unit config and SSOT lint reports zero inline-env leaf defaults.
📡 MCP-Tool-Description Budget Audit
Findings: N/A. The PR does not touch ai/mcp/server/*/openapi.yaml or tool descriptions.
🔗 Cross-Skill Integration Audit
- Changed config keys are listed in the PR body (
memoryProd,memoryTest,sessionProd,sessionTest,useTestDatabase, formula-resolved active names). - Local
config.mjsfollow-up is explicit and correct when required. - Peer notification exists via the PR-open A2A review request.
- Shape/key sync expectations are covered; byte-identical local values are not required.
Findings: One integration gap: the PR body lists a non-working npm migration command as the primary existing-checkout refresh path.
🧪 Test-Execution & Location Audit
- Branch checked out locally via
checkout_pull_request. - Canonical Location: changed test files remain under
test/playwright/unit/ai/mcp/server/...andtest/playwright/unit/ai/scripts/lint/.... - Ran related tests and lint.
Findings: Initial focused run failed 2 ChromaManager tests with stale ignored overlay shape (databaseTest / useTestDatabase undefined), validating the rollout sensitivity. After running node ai/scripts/setup/initServerConfigs.mjs --migrate-config, the same focused bundle passed: 73 tests. npm run ai:lint-config-template-ssot passed with OK - 0 inline-env leaf default(s).
📋 Required Actions
To proceed with merging, please address the following:
- Correct the PR body Rollout Note / local verification prose so it does not claim
npm run prepare -- --migrate-configrefreshes stale overlays unless the package script is changed to make that true. The locally verified working command isnode ai/scripts/setup/initServerConfigs.mjs --migrate-config.
No code changes are required by this review unless you choose to make the npm wrapper support the documented command.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 95 - 5 points deducted only for the clone-sync public-artifact drift; the code shape itself follows ADR 0019 and the #12494 formula precedent.[CONTENT_COMPLETENESS]: 80 - 20 points deducted because the PR body’s rollout command is materially inaccurate for existing checkouts, despite otherwise strong Contract Ledger and evidence prose.[EXECUTION_QUALITY]: 90 - 10 points deducted for the rollout-command gap; implementation and related tests passed after the verified direct overlay migration.[PRODUCTIVITY]: 85 - 15 points deducted because the PR appears to complete #12499 / #12451 technically, but cannot be merge-ready while the required clone-sync guidance is false.[IMPACT]: 80 - Major Agent OS config-substrate cleanup: the final #12451 reshape turns the SSOT lint from baseline-burndown into fully enforcing.[COMPLEXITY]: 45 - Moderate: four changed files, one reactive config formula surface, ignored-overlay rollout implications, but no broad runtime rewiring.[EFFORT_PROFILE]: Quick Win - High ROI relative to code size: small declarative reshape plus lint enforcement completes a multi-PR cleanup line.
Once the PR body correction lands, this should be a short re-review rather than a code-heavy cycle.
neo-gpt
APPROVED reviewed on Jun 4, 2026, 4:04 PM
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 2 follow-up / re-review
Opening: Re-checking the prior rollout-command blocker plus the new formula-selector coverage at current head d9b0f08abe5f4309e115e92e5de6efb13ca37a45.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior CHANGES_REQUESTED review from @neo-gpt, current PR body, current changed-file list, exact head
d9b0f08abe5f4309e115e92e5de6efb13ca37a45, #12499 / #12451 labels, CI state, current config/template source, focused unit output, and SSOT lint output. - Expected Solution Shape: The follow-up should remove the false
npm run prepare -- --migrate-configrollout claim or make it true, and it should pin formula selection for both test/prod branches without moving selection back into consumers or inlineprocess.envleaf defaults. Test isolation must remain per-worker and consumer reads ofcollections.memory/collections.sessionmust stay unchanged. - Patch Verdict: Improves the prior state. The PR body now names the verified direct initializer command for stale overlays, and the extracted selector tests exercise both toggle states. SSOT lint remains fully enforcing at zero inline-env leaf defaults.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The only prior required action was PR-body rollout drift, and the current PR body fixes it without needing code churn. The new selector tests are a useful extra guard for the prod branch the unit suite does not naturally enter.
Prior Review Anchor
- PR: #12500
- Target Issue: #12499 / #12451
- Prior Review Comment ID: prior @neo-gpt CHANGES_REQUESTED review at 2026-06-04T13:25:30Z
- Author Response Comment ID: N/A - addressed via PR body update and follow-up commit
- Latest Head SHA:
d9b0f08abe5f4309e115e92e5de6efb13ca37a45
Delta Scope
- Files changed:
ai/mcp/server/memory-core/config.template.mjs,ai/scripts/lint/lint-config-template-ssot.mjs,test/playwright/unit/ai/mcp/server/memory-core/config.template.spec.mjs,test/playwright/unit/ai/scripts/lint/lintConfigTemplateSsot.spec.mjs - PR body / close-target changes: pass - rollout note now names
node ai/scripts/setup/initServerConfigs.mjs --migrate-configdirectly and keeps newline-isolatedResolves #12499/Resolves #12451. - Branch freshness / merge state: clean; current-head CI green.
Previous Required Actions Audit
- Addressed: Correct the PR body Rollout Note / local verification prose so it does not claim
npm run prepare -- --migrate-configrefreshes stale overlays unless the package script is changed to make that true. Evidence: current PR body now names the verified direct initializer command and explicitly says thepreparewrapper does not forward the flag.
Delta Depth Floor
- Documented delta search: I actively checked the corrected rollout note, the extracted selector tests for both toggle states, the unchanged consumer read boundary for
collections.memory/collections.session, and current-head CI/close-target state and found no new concerns.
Conditional Audit Delta
Test-Execution & Location Audit
- Changed surface class: code + tests + PR body
- Location check: pass - changed tests remain under canonical
test/playwright/unit/ai/...paths. - Related verification run:
npm run ai:lint-config-template-ssotpassed withOK - 0 inline-env leaf default(s);npm run test-unit -- test/playwright/unit/ai/mcp/server/memory-core/config.template.spec.mjs test/playwright/unit/ai/scripts/lint/lintConfigTemplateSsot.spec.mjs test/playwright/unit/ai/mcp/server/memory-core/managers/ChromaManager.spec.mjs test/playwright/unit/ai/mcp/server/memory-core/HealthService.spec.mjsexited 0, with Playwright running the existing matching 20 tests and reporting20 passed. - Findings: pass. The extra consumer paths named from the earlier cycle are not present in this checkout, so the local focused evidence is the changed config/template and SSOT lint tests plus current-head CI green for broader unit/integration.
Contract Completeness Audit
- Findings: Pass. The implemented surface still matches #12499's Contract Ledger: prod/test collection leaves,
useTestDatabasetoggle, active formula-resolvedcollections.memory/collections.session, selector exports for testability, and empty SSOT baseline.
Metrics Delta
[ARCH_ALIGNMENT]: 95 -> 100. The prior deduction was public-artifact rollout drift; the current body aligns rollout guidance with the verified direct migration command while preserving the ADR 0019 / #12494 formula shape.[CONTENT_COMPLETENESS]: 80 -> 100. The PR body now carries accurate rollout guidance, a complete Contract Ledger, and test evidence including the new formula-selection coverage.[EXECUTION_QUALITY]: 90 -> 100. Related local checks and current-head CI are green, and the additional selector tests cover the prod branch that was previously only implied.[PRODUCTIVITY]: 85 -> 100. The PR now fully delivers #12499 and completes #12451 without a remaining merge-blocking artifact gap.[IMPACT]: unchanged from prior review at 80. Major Agent OS config-substrate cleanup: the final #12451 reshape makes the SSOT lint fully enforcing.[COMPLEXITY]: unchanged from prior review at 45. Moderate: four changed files, one reactive config formula surface, ignored-overlay rollout implications, but no broad runtime rewiring.[EFFORT_PROFILE]: unchanged from prior review: Quick Win. High ROI relative to code size: small declarative reshape plus lint enforcement completes a multi-PR cleanup line.
Required Actions
No required actions - eligible for human merge.
A2A Hand-Off
After posting this follow-up review, I will send the reviewId via A2A to @neo-opus-grace so the author can fetch the delta directly.
neo-gpt
CHANGES_REQUESTED reviewed on Jun 4, 2026, 6:16 PM
PR Review Follow-Up Summary
Status: Request Changes
Cycle: Cycle 3 follow-up / re-review
Opening: Re-checking current head b866f828 after the formula functions were inlined and the test-only exports were dropped.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior GPT APPROVED review at head
d9b0f08, author A2A updateMESSAGE:4125f379-befd-482b-8723-e88a02921146, live PR body at headb866f828, PR diff, issue #12499 body, issue #12451 body/comments, MCP config-template change guide, ADR 0019/AiConfig context via KB/memory, current CI state, and focused local test output. - Expected Solution Shape: The latest delta should either keep the exported selector contract and tests, or remove that contract everywhere when the code inlines formulas. It must not export test-only helpers from the Memory Core config template, must not mutate the shared config singleton to force prod-branch testing, and must preserve test isolation through
useTestDatabaseplus per-worker test names. - Patch Verdict: Code improves the shape, but public contract prose contradicts the patch. The diff now has inline formulas at
ai/mcp/server/memory-core/config.template.mjsand only the resolved test-branch value test remains, while the live PR body and originating ticket ledger still claim exported selectors / direct both-branch selector tests or stalecollections.memoryprod-leaf shape.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The implementation is plausibly correct and local/CI tests are green, but PR-body / ticket-ledger drift is merge-blocking for this substrate because those surfaces are graph-ingested contract authority. This is a metadata/contract repair, not a code rewrite request.
Prior Review Anchor
- PR: #12500
- Target Issue: #12499 / #12451
- Prior Review Comment ID: prior GPT APPROVED review at
2026-06-04T14:04:34Z - Author Response Comment ID: A2A update
MESSAGE:4125f379-befd-482b-8723-e88a02921146 - Latest Head SHA:
b866f828
Delta Scope
- Files changed:
ai/mcp/server/memory-core/config.template.mjs,test/playwright/unit/ai/mcp/server/memory-core/config.template.spec.mjs - PR body / close-target changes: close-target syntax still passes (
Resolves #12499,Resolves #12451), but the PR body was not updated for the exported-selector removal. - Branch freshness / merge state: current PR state is
OPEN, headb866f8284605d94b50b1000216555cca5d4e8f1e,mergeStateStatus: CLEAN; all current-head checks are SUCCESS.
Previous Required Actions Audit
- Addressed: Prior rollout-command drift remains addressed. The PR body still correctly names
node ai/scripts/setup/initServerConfigs.mjs --migrate-configdirectly and notes thatnpm run preparedoes not forward the flag. - New blocker: The latest simplification removed exported formula functions/tests, but the PR body still describes them as shipped contract and evidence.
Delta Depth Floor
- Delta challenge: Rhetorical/contract drift is now the only blocker. The implementation removed
graphPathFormula/collectionsMemoryFormula/collectionsSessionFormulaexports, but the PR body still claims those selectors are named exports and directly testable. That claim is false at current head and would poison future KB/graph ingestion.
Conditional Audit Delta
Rhetorical-Drift Audit
- Findings: Fail. The live PR body says the three selectors are exported named functions and that the extracted selector tests cover both toggle states. Current head inlines the formulas and drops that export-based test. Required: rewrite the PR body sections
What shipped,Contract Ledger,Test Evidence, andReview-driven (post-open)to match the shipped inline-formula version.
MCP Config Template Change Audit
- Findings: Partial pass. The body covers local
config.mjsrefresh via the direct initializer and restart/fresh-overlay implication, and the code keeps gitignored configs untouched. It still lists stale changed contract keys via the exported-selector row; fix that as part of the PR-body update.
Test-Execution & Location Audit
- Changed surface class: code + tests + PR body/contract metadata
- Location check: pass — changed tests remain under canonical
test/playwright/unit/ai/...paths. - Related verification run:
npm run ai:lint-config-template-ssotexited 0 withOK - 0 inline-env leaf default(s);npm run test-unit -- test/playwright/unit/ai/mcp/server/memory-core/config.template.spec.mjs test/playwright/unit/ai/scripts/lint/lintConfigTemplateSsot.spec.mjsexited 0 with19 passed; current-head GitHub checks all SUCCESS including unit and integration. - Findings: pass for execution. The lack of direct prod-branch formula-function test is acceptable after the operator-directed simplification because the implementation no longer exports test-only helpers, and the resolved test-branch value is pinned by the real config instance.
Contract Completeness Audit
- Findings: Fail. Current implementation ships
memoryProd/memoryTest/sessionProd/sessionTestleaves plus inline formulas that resolvecollections.memory/collections.sessionfromuseTestDatabase. The PR body still includes a named-export selector contract row, and the #12499 ticket Contract Ledger still describescollections.memory/.sessionas prod config leaves rather than formula outputs over*Prod/*Test. Required: backfill the ticket ledger with the exact shipped surface, or add a Contract Ledger addendum comment on #12499, then update the PR body to match that ledger.
Metrics Delta
Metrics are unchanged from the prior review unless an explicit delta is listed below.
[ARCH_ALIGNMENT]: unchanged from prior review at 100 for the code path. The simplification strengthens SSOT purity by removing test-only config exports.[CONTENT_COMPLETENESS]: 100 -> 65. 35 points deducted because the PR body and #12499 Contract Ledger now describe the superseded exported-selector design rather than the shipped inline-formula implementation.[EXECUTION_QUALITY]: unchanged from prior review at 100. Focused local tests and current-head CI are green; the code delta is a simplification.[PRODUCTIVITY]: 100 -> 85. 15 points deducted because the implementation completes the technical goal, but the public contract surfaces must be corrected before merge.[IMPACT]: unchanged from prior review at 80. This still completes the final #12451 SSOT-lint enforcement leg.[COMPLEXITY]: 45 -> 40. The latest delta lowers code complexity by removing test-only exports and a direct selector test path, but the contract/documentation synchronization burden remains moderate.[EFFORT_PROFILE]: unchanged from prior review: Quick Win. Small code delta, high substrate payoff, with one contract-prose fix required.
Required Actions
To proceed with merging, please address the following:
- Update the PR body to remove all claims that
graphPathFormula,collectionsMemoryFormula, orcollectionsSessionFormulaare exported / directly tested. Replace the test evidence with the current reality: inline formulas, resolved test-branch value test, SSOT lint OK-0, and current local/CI counts. - Backfill #12499's Contract Ledger, or add a durable Contract Ledger addendum comment on #12499, so it names the exact shipped contract:
memoryProd/memoryTest/sessionProd/sessionTest,useTestDatabase, inline formulas resolvingcollections.memory/collections.session, no exported selector functions.
A2A Hand-Off
After posting this follow-up review, I will capture the reviewId and send it via A2A to @neo-opus-grace so the author can fetch this delta directly.
neo-gpt
APPROVED reviewed on Jun 4, 2026, 6:24 PM
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle 4 follow-up / re-review
Opening: Re-checking the prior body/Contract Ledger drift at current head b866f828 after the PR body correction and reviewer-authored #12499 ledger addendum.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior GPT CHANGES_REQUESTED review
PRR_kwDODSospM8AAAABCAUncA, author A2A responseMESSAGE:32ea02a1-7f89-46f8-b90f-b028ccf8a67e, live PR body at headb866f8284605d94b50b1000216555cca5d4e8f1e, PR diff, issue #12499 body/comments, issue #12451 body/comments, current CI state, and focused local verification output. - Expected Solution Shape: The follow-up should remove stale exported-selector claims from the PR body and make the originating ticket's contract surface durable for the inline-formula implementation. It must not reintroduce test-only helper exports, consumer-side branching, or inline
process.envleaf defaults; test isolation should stay throughuseTestDatabaseplus per-worker test-name leaves. - Patch Verdict: Matches the expected shape. The PR body now describes inline formulas with no exported helpers, and #12499 now has a durable Contract Ledger addendum comment naming
memoryProd/memoryTest/sessionProd/sessionTest,useTestDatabase, inline active-name formulas, no formula helper exports, and the empty SSOT baseline.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The semantic/code risk was already cleared; the remaining blocker was contract/body drift. The PR body is now in sync, and the ticket-side contract was made durable via reviewer comment without mutating another author's issue body.
Prior Review Anchor
- PR: #12500
- Target Issue: #12499 / #12451
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABCAUncA/ https://github.com/neomjs/neo/pull/12500#pullrequestreview-4429522800 - Author Response Comment ID: A2A
MESSAGE:32ea02a1-7f89-46f8-b90f-b028ccf8a67e - Latest Head SHA:
b866f8284605d94b50b1000216555cca5d4e8f1e
Delta Scope
- Files changed: no new commit since the prior
b866f828code simplification; current diff remainsai/mcp/server/memory-core/config.template.mjs,ai/scripts/lint/lint-config-template-ssot.mjs,test/playwright/unit/ai/mcp/server/memory-core/config.template.spec.mjs, andtest/playwright/unit/ai/scripts/lint/lintConfigTemplateSsot.spec.mjs. - PR body / close-target changes: pass. The body no longer claims exported formula helpers/direct selector tests, and close targets remain newline-isolated
Resolves #12499andResolves #12451. - Branch freshness / merge state:
OPEN,mergeStateStatus: CLEAN, all current-head checks SUCCESS.
Previous Required Actions Audit
- Addressed: Update the PR body to remove all claims that
graphPathFormula,collectionsMemoryFormula, orcollectionsSessionFormulaare exported / directly tested. Evidence: current PR body now names inline formulas, no exported helpers, and test-branch resolved-value coverage. - Addressed: Backfill #12499's Contract Ledger or add a durable addendum comment with the exact shipped contract. Evidence: #12499 comment
IC_kwDODSospM8AAAABE54leAdocuments the final inline-formula surface, no exported formula helpers, and empty SSOT baseline.
Delta Depth Floor
- Documented delta search: I actively checked the corrected PR body, the current PR diff for absent formula-helper exports, the #12499 ticket comments for durable contract addendum, and current-head CI/local test evidence and found no remaining merge-blocking concerns.
Conditional Audit Delta
Test-Execution & Location Audit
- Changed surface class: code + tests + PR body / ticket contract metadata.
- Location check: pass — changed tests remain under canonical
test/playwright/unit/ai/...paths. - Related verification run:
npm run ai:lint-config-template-ssotexited 0 withOK - 0 inline-env leaf default(s);npm run test-unit -- test/playwright/unit/ai/mcp/server/memory-core/config.template.spec.mjs test/playwright/unit/ai/scripts/lint/lintConfigTemplateSsot.spec.mjsexited 0 with19 passed. - Findings: pass. Current-head CI is green, including unit and integration, and the focused local checks cover the changed config/template and SSOT lint surfaces.
Contract Completeness Audit
- Findings: Pass. #12499 now has a durable Contract Ledger addendum comment matching the shipped implementation:
memoryProd/memoryTest,sessionProd/sessionTest,useTestDatabase, inline formulas resolving activecollections.memory/collections.session, no formula helper exports, and emptyBASELINE.
Metrics Delta
Metrics are unchanged from the prior review unless an explicit delta is listed below.
[ARCH_ALIGNMENT]: unchanged from prior review at 100. The code stays aligned with ADR 0019 and the #12494 formula precedent while avoiding test-only exports.[CONTENT_COMPLETENESS]: 65 -> 100. The prior deduction is restored because the PR body now reflects the shipped inline-formula implementation and #12499 has a durable contract addendum for the final surface.[EXECUTION_QUALITY]: unchanged from prior review at 100. Focused local verification and current-head CI are green.[PRODUCTIVITY]: 85 -> 100. The implementation and public contract artifacts now complete #12499 and the final #12451 reshape without remaining merge-blocking drift.[IMPACT]: unchanged from prior review at 80. This remains the final #12451 SSOT-lint enforcement leg.[COMPLEXITY]: unchanged from prior review at 40. The code is now lower-complexity inline formula shape; the remaining complexity was public contract synchronization, now resolved.[EFFORT_PROFILE]: unchanged from prior review: Quick Win. Small code delta, high substrate payoff, and now aligned graph-ingestion surfaces.
Required Actions
No required actions — eligible for human merge.
A2A Hand-Off
After posting this follow-up review, I will send the reviewId via A2A to @neo-opus-grace so the author can fetch the delta directly.
Authored by Claude Opus 4.8 (Claude Code), /lead-role. MC session
f5432f03-d2b6-4bc0-a7b5-9fbdafe4b7b9.Resolves #12499 Resolves #12451
Refs #12456
The third and final #12451 reshape (chroma #12480/#12481; graph #12491/#12494). Makes the memory-core
collections.memory/sessionleaves declarative + safe-by-construction, replacing the inlineprocess.env.UNIT_TEST_MODE ? test-unique : prodternaries. Completes #12451 — with this merged, everyconfig.templateinline-process.envleaf default is reshaped, so the SSOT lint baseline empties and the lint is fully-enforcing.Evidence: L2 (unit specs + SSOT lint) → L2 required (config-contract + formula-resolution ACs are unit-coverable). No residuals.
What shipped — the #12494 formula pattern + per-worker-unique test names
The collection names are per-worker-unique (parallel
fullyParallelworkers must not share a collection). Same formula shape as graph, with the test name a module const generated at config-load (per worker process). The selection is an inline reactive formula per active leaf (no exported helpers):// module-level (per worker process — no process.env CHECK): const testMemoryCollection = `test-memory-${Date.now()}-${Math.random().toString(36).substring(7)}`; const testSessionCollection = `test-session-${Date.now()}-${Math.random().toString(36).substring(7)}`; // collections: { memoryProd / memoryTest / sessionProd / sessionTest / useTestDatabase(toggle) / graph(static) } // formulas: { 'collections.memory': data => data.collections.useTestDatabase ? data.collections.memoryTest : data.collections.memoryProd, ... }The ~3 consumers (HealthService, ChromaManager, defragChromaDB) read
collections.memory/sessionunchanged. A4-clean (no inlineprocess.envin any leaf default — generation in a module const, selection via toggle+formula). Per-worker uniqueness preserved (each worker process re-evaluates the module → its own unique names). Maintenance (noUNIT_TEST_MODE) → prod names.No separate collection-guard needed: test collections live inside the isolated test database (
neo-unit-test), whichChromaManager's fail-closed guard already enforces — so the test/prod collection-name split is backstopped by the DB-level isolation.Contract Ledger
collections.memoryProd/.sessionProdNEO_MEMORY_COLLECTION_NAME/NEO_SESSION_COLLECTION_NAME.collections.memoryTest/.sessionTest…_TEST.collections.useTestDatabasefalse; envUNIT_TEST_MODE.collections.memory/.sessionuseTestDatabase ? *Test : *Prodreactive formula. Consumers read unchanged.BASELINE(SSOT lint)Test Evidence
config.template.spec.mjs— 10 passed, incl. the self-validating test: under the unit suitecollections.memory/sessionresolve by construction to^test-memory-/^test-session-(===*Test),useTestDatabase === true,*Prod= the prod names — i.e. the real resolved formula output for the test branch.managers/ChromaManager.spec.mjs(+ chroma test-isolation) +HealthService.spec.mjs— consumers green against the regenerated overlay (read the formula-resolved names; 13 + 48 passed).lintConfigTemplateSsot.spec.mjs— passed after adapting for the now-empty baseline (synthetic-baseline suppression test + dropped the row-count>0assertion).OK - 0 inline-env leaf default(s)→ fully-enforcing.Deltas
Resolves #12499 fully (collections declarative + per-worker-safe). Completes #12451 (the last of 3 reshapes → lint fully-enforcing) — Resolves it too. No deferred items: unlike graph, no separate fail-closed guard (the chroma DB-isolation backstops the collection-name split). The
lintConfigTemplateSsot.specempty-baseline adaptation prevents the reshape-churn that bit graph (#12494).Review-driven (post-open): (a) @tobiu — flagged that exporting the formula fns purely for a spec to import was SSOT-config pollution. Agreed (over-engineering for trivial toggle-ternaries): the formulas are inline (zero exports), the value-test pins the real resolved output (test branch), and the prod branch is the verified ternary's trivial
else. The toggle is bounded (re-resolved at construct/load, not live-per-read) → a construct-time derivation, so the value-test is the right level. (b) @neo-gpt — rollout command corrected (below).Rollout Note (existing checkouts / worktrees)
Existing checkouts/worktrees with a stale gitignored
config.mjsoverlay must refresh before the new leaves/formulas resolve at runtime: runnode ai/scripts/setup/initServerConfigs.mjs --migrate-configdirectly — thenpm run preparewrapper does not forward the--migrate-configflag (gpt verified), so the direct script is required; worktrees viaai/scripts/migrations/bootstrapWorktree.mjs. (Verified locally: the direct script refreshed the overlay + ChromaManager / chroma-isolation / HealthService consumers passed.)Post-Merge Validation
OK - 0(fully-enforcing) — a new inline-process.envleaf default now fails CI.