Phase 7 Item 4: extract 4 helpers from memory-search.ts#22
Merged
ethanj merged 2 commits intoarchitecture2from Apr 18, 2026
Merged
Phase 7 Item 4: extract 4 helpers from memory-search.ts#22ethanj merged 2 commits intoarchitecture2from
ethanj merged 2 commits intoarchitecture2from
Conversation
Polish from the plan — memory-search.ts becomes pure orchestration by delegating non-orchestration concerns to their natural neighbors. - buildInjection → retrieval-format.ts. Same module already owns formatSimpleInjection and formatTieredInjection. Exported with a new InjectionBuildResult interface. - applyFlatPackagingPolicy → composite-dedup.ts. Thin wrapper around deduplicateCompositeMembersForFlatQuery that already lives there, plus a trace.stage call. TraceCollector import added as type-only. - recordSearchSideEffects → new src/services/retrieval-side-effects.ts. 37 lines, owns the 'what happens after search returns' concerns (touchMemory + audit emission). - recordConsensusLessons → lesson-service.ts. Other lesson helpers already lived there; the static import removes a previous dynamic-import round-trip. Signature simplified to take LessonStore directly rather than the full MemoryServiceDeps. memory-search.ts drops from 374 to 269 lines (-28%). Public API unchanged — performSearch, performFastSearch, performWorkspaceSearch keep their existing signatures. Regression canaries (from the plan) all pass: - smoke.test.ts - memory-search-runtime-config.test.ts - search-pipeline-runtime-config.test.ts - contradiction-safe.test.ts - temporal-correctness.test.ts 963/963 tests pass. tsc --noEmit clean. fallow --no-cache 0 above threshold. Note on the 250-line target: the plan's verification listed '≤ 250 lines' as the target; landing at 269. The substantive extraction is done (all 4 helpers per the plan); the remaining 19 lines are orchestration code that would require structural change beyond the scope of a polish PR. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex review flagged 269 > 250 target from the plan. Trimmed 21 lines to land at 248. - src/services/packaging-observability.ts: new finalizePackagingTrace helper that bundles the packaging-summary + assembly-summary + tiered- packaging event + trace-collector setters. Eliminates 13 lines of boilerplate in memory-search.ts's assembleResponse. The previously- exported buildPackagingTraceSummary / buildAssemblyTraceSummary are now internal (only finalizePackagingTrace is called externally). Module absorbs the deduplicateCompositeMembersHard dependency too. - src/services/memory-search.ts: assembleResponse uses finalizePackagingTrace. performWorkspaceSearch's return object spreads the buildInjection result via ...injection and destructures composite filter output inline. Drops the now-unused deduplicateCompositeMembersHard import. memory-search.ts: 374 (baseline) → 269 (initial Item 4) → 248 (here). packaging-observability.ts: 157 → 207 (+50, absorbs the helper + composite-dedup dependency). 963/963 tests pass. tsc --noEmit clean. fallow --no-cache 0 above threshold (maintainability 90.9). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
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
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 7 Item 4 — retrieval decomposition polish. Moves 4 helpers out of
memory-search.tsinto their natural neighbor files so the orchestrator stays focused on flow, not formatting/telemetry/dedup details.Plan:
atomicmemory-research/docs/core-repo/rearchitecture/phase7-v1-parity/phase7plan.mdItem 4.Independent of the Step 3d ratchet (#21) — this touches
memory-search.tsand neighbors that weren't in the config-threading set.What changed
buildInjectionmemory-search.tsretrieval-format.tsformatSimpleInjection/formatTieredInjection. NewInjectionBuildResultinterface.applyFlatPackagingPolicymemory-search.tscomposite-dedup.tsdeduplicateCompositeMembersForFlatQuerythat was already there.TraceCollectorimported as type-only.recordSearchSideEffectsmemory-search.tsretrieval-side-effects.tsrecordConsensusLessonsmemory-search.tslesson-service.tsLessonStoredirectly instead of fullMemoryServiceDeps; static import replaces the previous dynamic-import round-trip.Size impact
memory-search.tsretrieval-format.tsbuildInjection)composite-dedup.tsapplyFlatPackagingPolicy)lesson-service.tsrecordConsensusLessons)retrieval-side-effects.tsStability
MemoryService.search / fastSearch / scopedSearch / workspaceSearchkeep their existing signatures. The extracted helpers were all private.smoke.test.tsmemory-search-runtime-config.test.tssearch-pipeline-runtime-config.test.tscontradiction-safe.test.tstemporal-correctness.test.tsTest plan
npx tsc --noEmitcleannpm test963/963 pass (no new tests — pure refactor)fallow --no-cache0 above thresholdmemory-search.ts≤ 300 lines (landed at 269; plan target was ≤ 250, noted below)Note on the 250-line target
The plan listed
memory-search.ts ≤ 250 linesas the verification target. Landed at 269. All 4 helpers called out in the plan are moved; the remaining 19 lines are pure orchestration code (imports + 3 orchestration functions + 3 public entrypoints) that would require further structural change beyond a polish PR. Deferred.🤖 Generated with Claude Code