Skip to content

Phase 7 Step 3d: thread config through 5 leaf modules (33→28 singleton audit)#21

Merged
ethanj merged 6 commits intoarchitecture2from
phase7-3d-config-threading
Apr 18, 2026
Merged

Phase 7 Step 3d: thread config through 5 leaf modules (33→28 singleton audit)#21
ethanj merged 6 commits intoarchitecture2from
phase7-3d-config-threading

Conversation

@ethanj
Copy link
Copy Markdown
Contributor

@ethanj ethanj commented Apr 18, 2026

Summary

Phase 7 Step 3d — thread config through 5 leaf modules so they stop reading the config singleton directly. Ratchets MAX_SINGLETON_IMPORTS 33 → 28.

Supersedes closed #19 (leaf 1) and #20 (leaf 2 stacked on #19). Bundled per updated review direction (AI-reviewer flow — one-PR-per-leaf no longer needed).

Plan: atomicmemory-research/docs/core-repo/rearchitecture/phase7-v1-parity/phase7plan.md Item 3d.

The 5 leaves

Two patterns emerged based on blast radius.

Explicit parameter threading (leaves 1–3)

For leaves with few / already-threaded callers, each exported function accepts config explicitly:

Leaf File Callers Pattern
1 consensus-extraction.ts 1 (memory-ingest) consensusExtractFacts(text, config)
2 write-security.ts 6 (ingest + fact pipeline) assessWriteSecurity(content, site, config), recordRejectedWrite(..., config, lessons?)
3 cost-telemetry.ts 4 writeCostEvent calls in llm/embedding writeCostEvent(event, config)

Defaulting happens at the caller (composition layer already had deps.config in scope). No new infrastructure.

Module-local init (leaves 4–5)

embedding.ts and llm.ts have 15+ deep callers of their hot-path APIs (embedText, embedTexts, llm.chat). Explicit threading would be a massive ripple. Plan's pragmatism clause applies — after Step 3c provider/model selection is startup-only, so module-local state after an explicit init() call has the same semantics as the old singleton reads, just without the cross-module coupling to config.ts.

Leaf File Pattern
4 embedding.ts initEmbedding(config) → module holds EmbeddingConfig; all internals call requireConfig()
5 llm.ts initLlm(config) → module holds LLMConfig; all internals call requireConfig()

runtime-container.ts calls both inits inside createCoreRuntime, so any consumer going through the composition root is auto-initialized. Tests that bypass createCoreRuntime and call embedding / llm APIs directly must init themselves — two test files updated (embedding-cache.test.ts, llm-providers.test.ts), each now composing a narrow test config instead of mutating the config singleton.

Why the two patterns

Plan guidance: "The leaf module stops importing config entirely — that's how it drops out of config-singleton-audit.test.ts."

Both patterns satisfy that: the file no longer has import { config } from '../config.js'. The difference is where defaulting lives — at the caller (pattern 1) vs. at the composition root via init (pattern 2). Pattern 2 is defensible because (a) the ripple is prohibitive, (b) provider/model config is startup-immutable post-3c, so "rebind on init" matches reality.

Ratchet trail

After Count File dropped
baseline (after 3a/b/c on architecture2) 33
leaf 1 32 consensus-extraction.ts
leaf 2 31 write-security.ts
leaf 3 30 cost-telemetry.ts
leaf 4 29 embedding.ts
leaf 5 28 llm.ts

Per the plan, stop criterion is ~5–10 (infrastructure only — migrate.ts, index.ts, runtime-container.ts). 28 is not there yet; remaining files fall into the infra category plus a few orchestration files that are natural follow-ups for a later pass.

Test plan

  • npx tsc --noEmit clean
  • npm test 963/963 pass (no change in count — no new tests, just signature threading and two test files updated)
  • fallow --no-cache 0 above threshold (maintainability 91.0)
  • config-singleton-audit.test.ts passes at new threshold 28
  • embedding-cache.test.ts and llm-providers.test.ts — now init with narrow test configs instead of mutating singleton
  • Mocked test paths (smoke.test.ts, research-consumption-seams.test.ts, etc.) unaffected — their mocks replace the whole module

Commits

  • 32521bc — leaf 1: consensus-extraction
  • a6ad6a4 — leaf 2: write-security
  • 9810334 — leaf 3: cost-telemetry
  • 04aae26 — leaf 4: embedding (module-local init pattern introduced)
  • 478464b — leaf 5: llm

Out of scope

  • Further audit drops below 28. The plan's stop criterion is ~5–10; remaining files are infrastructure (migrate.ts, index.ts, runtime-container.ts) or orchestration files that don't benefit from threading. Separate work if pursued.
  • Item 4 (retrieval polish) — 1-PR cleanup of memory-search.ts.

🤖 Generated with Claude Code

ethanj and others added 5 commits April 18, 2026 13:41
First leaf conversion in the singleton-audit ratcheting sequence. Drops
consensus-extraction.ts from the singleton importer set (33 → 32).

- src/services/consensus-extraction.ts: dropped the `import { config }`
  singleton binding. `consensusExtractFacts` now takes config as a
  required parameter typed `ConsensusExtractionConfig`
  (Pick-style: consensusExtractionEnabled, consensusExtractionRuns,
  chunkedExtractionEnabled). The new type is co-located and exported so
  orchestration files importing it get the narrow contract.
- src/services/memory-ingest.ts: both call sites (performIngest line 66,
  workspace line 191) now pass `deps.config` explicitly. Already had
  deps.config in scope — no signature ripple into performIngest/
  performWorkspaceIngest callers.
- src/services/memory-service-types.ts: IngestRuntimeConfig gains
  chunkedExtractionEnabled, consensusExtractionEnabled,
  consensusExtractionRuns so deps.config satisfies the new parameter
  type structurally.
- src/__tests__/config-singleton-audit.test.ts: MAX_SINGLETON_IMPORTS
  ratcheted 33 → 32 with updated justification comment referencing this
  step.

Defaulting location: at the composition root (runtime-container builds
deps.config once from the singleton), not inside the leaf module — so
the leaf file itself no longer binds `config`, which is how it drops
out of the audit. Any alternative where the leaf kept a
"if (!config) fall back to singleton" path would have left the import
in place and made the ratcheting theatre.

963/963 tests pass. tsc --noEmit clean. fallow --no-cache 0 above
threshold.

Plan: phase7-v1-parity item 3d, four leaves remaining
(write-security.ts, cost-telemetry.ts, embedding.ts, llm.ts).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Second leaf conversion. Drops write-security.ts from the singleton
importer set (32 → 31). Stacked on PR #19 until it merges.

- src/services/write-security.ts: dropped `import { config }`. Both
  exported functions now take config explicitly:
    - assessWriteSecurity(content, sourceSite, WriteSecurityAssessConfig)
      — reads trustScoringEnabled, trustScoreMinThreshold.
    - recordRejectedWrite(userId, content, sourceSite, decision,
      WriteSecurityRecordConfig, lessons?) — reads auditLoggingEnabled,
      lessonsEnabled, trustScoreMinThreshold (the last is used to build
      the `trust:below-threshold` audit payload, which previously read
      the singleton at emit time).
  Two new exported config interfaces (Pick-style) keep the contract
  narrow; IngestRuntimeConfig satisfies both structurally.
- src/services/memory-ingest.ts:143 + src/services/ingest-fact-
  pipeline.ts:84/130/168/87/170: all 6 call sites thread deps.config.
  No signature ripple — every caller already had deps in scope.
- src/services/memory-service-types.ts: IngestRuntimeConfig gains
  trustScoringEnabled + trustScoreMinThreshold so deps.config satisfies
  both new config interfaces structurally.
- src/services/__tests__/write-security.test.ts: stopped importing and
  mutating the runtime config singleton; builds its own config object
  per test instead. Cleaner — the tests are now pure unit tests with
  explicit inputs.
- src/__tests__/config-singleton-audit.test.ts: MAX_SINGLETON_IMPORTS
  ratcheted 32 → 31.

963/963 tests pass. tsc --noEmit clean. fallow --no-cache 0 above
threshold.

Plan: phase7-v1-parity item 3d. Three leaves remain: cost-telemetry.ts,
embedding.ts, llm.ts.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops cost-telemetry.ts from the singleton importer set (31 → 30).

- src/services/cost-telemetry.ts: dropped `import { config }`.
  writeCostEvent now takes WriteCostEventConfig (costLoggingEnabled,
  costRunId, costLogDir) as a required parameter. Other exports
  (estimateCostUsd, withCostStage, getCostStage, summarizeUsage) did
  not read config and are unchanged.
- src/services/llm.ts + src/services/embedding.ts: 4 writeCostEvent
  call sites now pass `, config` — these modules still bind config
  themselves (leaves 4 and 5), but the cost-telemetry file itself no
  longer does.
- src/__tests__/config-singleton-audit.test.ts: MAX_SINGLETON_IMPORTS
  ratcheted 31 → 30.

963/963 tests pass. tsc --noEmit clean.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops embedding.ts from the singleton importer set (30 → 29). Uses the
module-local-state pattern: embedding no longer imports `config` from
'../config.js'. Config is bound once via `initEmbedding(config)` called
by the composition root (runtime-container.ts).

Rationale for pattern choice: embedText/embedTexts are hot-path APIs
called from 15+ files. Threading config through every caller would be
a massive ripple. Post Phase 7 Step 3c, provider/model selection is
startup-only, so module-local state after init has the same semantics
as the old singleton read pattern — just without the cross-module
coupling to config.ts.

- src/services/embedding.ts: drop `import { config }`. Add
  `EmbeddingConfig` interface (extends WriteCostEventConfig) and
  `initEmbedding(config)` export. Module-local `embeddingConfig`
  binds on init. All internal functions call `requireConfig()` which
  throws with a clear message if init wasn't called.
- src/app/runtime-container.ts: `createCoreRuntime` now calls
  `initEmbedding(config)` once. Any consumer going through the
  composition root is auto-initialized.
- src/services/__tests__/embedding-cache.test.ts: the one test that
  imports embedText directly (bypassing createCoreRuntime) now calls
  initEmbedding in beforeAll with a narrow test config. Dropped the
  file's old `vi.mock('../../config.js', ...)` since embedding no
  longer reads the singleton. Mock of 'openai' still intercepts the
  constructor call.
- src/__tests__/config-singleton-audit.test.ts: MAX_SINGLETON_IMPORTS
  ratcheted 30 → 29.

963/963 tests pass. tsc --noEmit clean.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops llm.ts from the singleton importer set (29 → 28). Same module-
local-state pattern as embedding.ts: config bound once via
`initLlm(config)` at composition-root time.

- src/services/llm.ts: drop `import { config }`. Add `LLMConfig`
  interface (extends WriteCostEventConfig) and `initLlm(config)`
  export. Module-local `llmConfig` binds on init. All internal
  functions call `requireConfig()` which throws with a clear
  message if init wasn't called.
- src/app/runtime-container.ts: `createCoreRuntime` now calls
  `initLlm(config)` alongside `initEmbedding(config)`.
- src/services/__tests__/llm-providers.test.ts: replaced the
  `vi.mock('../../config.js', ...)` + singleton mutation pattern
  with explicit `initLlm(testConfig)` calls in beforeEach. Cleaner —
  tests now compose their own config object per case.
- src/__tests__/config-singleton-audit.test.ts: MAX_SINGLETON_IMPORTS
  ratcheted 29 → 28.

All 5 Step 3d leaves complete: consensus-extraction, write-security,
cost-telemetry, embedding, llm. Singleton-audit went from 33 → 28 over
this branch (plus the three Step 3a/b/c commits at the start of 3d
land on top of step 3c's baseline).

963/963 tests pass. tsc --noEmit clean. fallow --no-cache 0 above
threshold (maintainability 91.0).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex review caught that dropping config from embedding.ts/llm.ts and
requiring an explicit init is a real contract change for consumers who
deep-import @atomicmemory/atomicmemory-engine/services/embedding or
/services/llm — and those deep-paths are still in package.json#exports
(narrowing is out of scope for this phase per the plan).

Two mitigations:

- src/index.ts now re-exports initEmbedding, initLlm, EmbeddingConfig,
  and LLMConfig from the root. Deep-path consumers that need explicit
  init can pull everything from the root package.
- docs/consuming-core.md gains a 'Deep-path init requirement' subsection
  under the stability boundary. Documents the behavior change, the
  auto-init via createCoreRuntime, the explicit init pattern for direct
  deep-path consumers, and the rationale (startup-only provider/model
  post-Step-3c).

Consumers going through createCoreRuntime are already handled — the
composition root calls both inits internally. No runtime code change
beyond the re-exports; just docs + public surface.

963/963 tests pass. tsc --noEmit clean.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ethanj ethanj merged commit 2517774 into architecture2 Apr 18, 2026
@ethanj ethanj deleted the phase7-3d-config-threading branch April 18, 2026 22:22
ethanj added a commit that referenced this pull request Apr 19, 2026
Cuts over `main` to the Phase 1A–7 rearchitecture (composition root,
  explicit scope/observability contracts, store-narrowed repository access,
  public consumption seams, config split, leaf-module config threading,
  retrieval orchestration polish) plus the OSS-release-prep on top.

  ⚠️ Breaking changes for HTTP / SDK consumers
  - All API endpoints are now mounted under `/v1` (e.g.
    `POST /v1/memories/ingest`, `PUT /v1/agents/trust`). The unversioned
    `/health` liveness probe is unchanged.
  - Workspace `GET /memories/list`, `GET /memories/:id`, and
    `DELETE /memories/:id` now require `agent_id` when `workspace_id`
    is present; missing returns 400 (no visibility-unsafe fallback).
  - `PUT /memories/config` returns 410 Gone in production. Provider/model
    fields (embedding_provider, embedding_model, llm_provider, llm_model)
    are rejected with 400 — they were never honored mid-flight (provider
    caches are fixed at first use). Set via env at process start.
  - npm package renamed `@atomicmemory/atomicmemory-engine` →
    `@atomicmemory/atomicmemory-core`. Tarball now ships `dist/` (built
    via `tsc`); `main`/`types`/`exports` point at compiled output.
  - Deep-importers of `services/embedding` and `services/llm` must call
    `initEmbedding(config)` / `initLlm(config)` before hot-path APIs.
    Consumers using `createCoreRuntime({ pool })` are auto-initialized.

  Rearchitecture (Phases 1A–7)
  - Phase 1A: composition root via `createCoreRuntime` + `createApp` (#8)
  - Phase 2A: canonical search scope contract (#9)
  - Phase 2B: explicit retrieval observability contract (#10)
  - Phase 3: runtime-config seam cleanup to Phase 4 boundary (#11)
  - Phase 4: ingest pipeline decomposition (475 → 215 lines) (#13)
  - Post-Phase 4: unify scope contract via `scopedSearch`/`scopedExpand`,
    document schema scoping gaps as deferred (#15)
  - Phase 5: narrow repository access behind 8 domain-facing stores;
    workspace ingest now flows through canonical lineage (#16)
  - Phase 6: publish stable consumption seams (HTTP, in-process, docker)
    with two-direction parity contract test (#17)
  - Phase 7 Steps 3a–3c: split runtime config into supported/internal
    partitions; deprecate `PUT /memories/config` for production (#18)
  - Phase 7 Step 3d: thread config through 5 leaf modules (33→28
    singleton audit) (#21)
  - Phase 7 Item 4: retrieval polish — `memory-search.ts` reduced to
    pure orchestration (374 → 248 lines, -34%) (#22)
  - Chore: reduce fallow duplication 367 → 234 lines (#12)

  OSS release prep
  - `"private": true` removed; package renamed to
    `@atomicmemory/atomicmemory-core`. `files` field scopes the tarball.
  - `tsconfig.build.json` + `prepublishOnly` so `npm publish` always ships
    compiled `dist/`. Bare-import smoke test passes.
  - `release.yml` publishes to public npm on tag push (NPM_TOKEN secret).
  - SuperMem codename scrubbed from `src/`, tests, docker-compose, and
    `.env.example` (DB user/name/password renamed to `atomicmemory`).
  - Private-research-repo URLs unlinked from public docs.
  - README links to docs.atomicmemory.ai.
  - `/v1` API prefix on all routes; mount-coverage test added.
  - CI workflow: set `CORE_RUNTIME_CONFIG_MUTATION_ENABLED=true` to
    match `.env.test` (gitignored) and unblock the composed-boot-parity
    test on `PUT /v1/memories/config`.

  Verification
  - 966/966 tests pass (100 files)
  - npx tsc --noEmit clean
  - fallow --no-cache: 0 above threshold (maintainability 91.0)
  - npm publish --dry-run succeeds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant