refactor(memory): simplify source-state queries

[steipete]

What Problem This Solves

Memory source inspection and session reconciliation built a complete path-to-hash map even when their callers used only the source rows. Several callers immediately built the different lookup structure they actually needed, retaining unnecessary work for large indexed corpora.

Why This Change Was Made

The source-state reader now returns its rows directly. Memory-file synchronization builds its required hash map at the original snapshot boundary; status inspection, startup catchup, session planning, and targeted cleanup stop constructing and discarding an extra map.

Both ordinary source-state queries now use the existing synchronous Kysely helpers. This removes the mock-shaped database interface, handwritten SQL, result casts, and impossible undefined-array fallback. The full row projection, source predicates, snapshot precedence, and query timing are preserved. No schema, configuration, retention, migration, or SDK surface changes are introduced.

User Impact

Memory status and session reconciliation allocate less intermediate state while keeping the same dirty-state detection, indexing, and pruning behavior. Production code is three lines smaller, and the SQL-string mock tests are replaced with shorter real-SQLite coverage.

Evidence

All required changed checks passed, including extension/test typechecks, lint, boundaries, and runtime import-cycle checks.

The focused source-state, session-planning, startup-catchup, and update/forget-race suites passed 66 tests. Three additional public manager cases passed for offline source edits, canonical session discovery, and pruning removed sessions without re-embedding survivors.

OPENCLAW_VITEST_MAX_WORKERS=2 node scripts/run-vitest.mjs \
  extensions/memory-core/src/memory/manager-source-state.test.ts \
  extensions/memory-core/src/memory/manager-session-sync-state.test.ts \
  extensions/memory-core/src/memory/manager-sync-ops.startup-catchup.test.ts \
  extensions/memory-core/src/memory/manager-session-update-race.test.ts
OPENCLAW_VITEST_MAX_WORKERS=2 node scripts/run-vitest.mjs \
  extensions/memory-core/src/memory/index.test.ts \
  -t 'detects offline source edits|diagnostic status uses canonical session discovery|prunes removed sessions'

A native probe on Node 24.20.0 read 20,000 synthetic indexed source rows before and after the change. The complete row digest was identical. Both versions used one bulk query, zero queries for supplied snapshots, and three scoped point queries. The reader's unused hash map went from 20,000 entries to zero. Source separation, fractional modification times, an empty snapshot, fresh point reads, and a quoted missing path retained their behavior; SQLite integrity remained healthy. These are allocation-structure and query-count observations, not wall-clock or peak-RSS measurements.

Removing the two raw result casts also removes this file's grandfathered assertion-baseline entry. Targeted archive cleanup still reads the full source snapshot; narrowing that query is a separate follow-up requiring explicit large-target and snapshot semantics.

Independent Codex review of the complete frozen patch and owner/caller/dependency context was scoped-clean through P2. The composed native CLI/Gateway proof passed all six phases, including offline edits, indexing, deletion, canonical-session pruning and fresh-process reopen.

CI caught an older responsiveness fixture whose fake database lacked native iteration. That fixture now uses the real SQLite schema already used by its cache-seeding sibling, with explicit connection ownership and unchanged yield assertions. The original failure reproduced locally; the repaired fixture and source-state suite passed all 8 tests, required changed checks passed, and a fresh full-patch independent review was scoped-clean through P2. The repair changes tests only; the production files exercised in the live proof are unchanged. Overall production net−3, tests net−35, assertion baseline−1. Exact-head hosted CI is rerunning.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review in progress

ClawSweeper is reviewing this revision. This supersedes any previous blocked status.

View the workflow run.

[steipete]

Live validation for #138709 (head ec7c82830a0b952fec89a6a3c38d8a63b508401c) and #138711 (head 4fcfe21858e). The proof composition contains exactly both reviewed changes over 49ae9753ffe3b94c677c98b2918379bfb3e73841.

Native live proof passed on combined commit 0991bc4e3e2ee10e0f4762c1f9f24fb89171f322, tree e8ea07ec24cedf010ab25a35114d9b9016d9678f, with Node 24.20.0 / pnpm 12.1.0 / SQLite 3.53.4 on macOS arm64. Frozen installation and the qaRuntime build (memory-core,logbook) passed. All six phases passed on the first attempt; source and executed harness hashes stayed unchanged.

python3 <task-harness>/run-candidate.py <candidate> \
  0991bc4e3e2ee10e0f4762c1f9f24fb89171f322 \
  e8ea07ec24cedf010ab25a35114d9b9016d9678f --build

exitCode=0 completedPhases=6 sourceUnchanged=true
cleanup: synthetic state removed; Gateway port closed; no owned processes

Surface	Observed proof
Memory CLI	Diagnostic status detected an offline edit and file deletion without rewriting indexed rows. Normal indexing updated the changed source, pruned the deleted file and canonical session, and preserved unchanged survivor rows. Fresh-process search payloads, source/chunk bytes and clean status matched exactly. Three memory sources and one session source remained.
Memory source owner	The exact 20,000-row projection matched the baseline digest. Eager hash-map entries changed 20,000 → 0; query counts correctly stayed 1 bulk / 0 supplied-snapshot / 3 point. Empty snapshot precedence, path/source scoping, a quoted miss and SQLite integrity passed.
Logbook Gateway	Real status/days/timeline calls covered 64 cards across two days, including default and empty-day views. All cards/statistics and the complete wire payload matched after Gateway restart. Capture stayed disabled across scheduled ticks; frames and batches stayed at zero; durable card bytes stayed unchanged.
Logbook read owner	Timeline queries 2 → 1, hydrated card rows 128 → 64; status hydrated card rows 64 → 0; revision window hydrated rows 64 → 3. The complete timeline/status/revision payload digest matched the baseline; reopen and integrity passed.

The Gateway wire payload digest was 1b833355758a329768b48fc2411227ddc9d17513e52a92ad34d0dcd2cf2ccc75. The fixed-date native Logbook payload digest remained 768bb71e0c37bcdfea78d66c395e975393a05ac822da4f2ffd24926a83d17a27.

This proof uses synthetic native SQLite/CLI/Gateway data, with FTS-only memory and Logbook capture disabled. It makes no screenshot, external provider, dashboard rendering, or cross-platform claim. Memory still reads the original bulk rows; the saving is the discarded hash-map allocation.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 4, 2026, 9:07 PM ET / September 5, 2026, 01:07 UTC.

ClawSweeper review

What this changes

Memory Core’s source readers stop building unused hash maps, reuse existing typed SQLite query helpers, and replace database mocks with real-SQLite coverage.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep open with no blocking findings. The allocation cleanup remains absent from current main and the supplied latest release, and the native evidence supports preserving indexing, diagnostics, and pruning behavior.

Priority: P3
Reviewed head: 8e5dd454501ab9e82cbda8623aeb46670395ac2d

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	Focused net-negative implementation, real-SQLite coverage, and source-linked native before/after evidence make this a strong cleanup with no blocking findings.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The supplied native Memory CLI and SQLite transcript exercises the changed reader through diagnostic inspection, indexing, deletion/pruning, and fresh-process reopen, with matching row and search results; the later head changes only tests.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The supplied native Memory CLI and SQLite transcript exercises the changed reader through diagnostic inspection, indexing, deletion/pruning, and fresh-process reopen, with matching row and search results; the later head changes only tests.
Evidence reviewed	9 items	Introduced scope and proof continuity: The pinned merge-base-to-head delta contains only the Memory Core reader/caller cleanup, two test updates, and removal of the corresponding assertion-baseline entry. The later commit changes only the synchronization-yield test, so production code matches the head identified in the supplied native proof. The stale test merge was not used to infer removals from main. Current-main necessity: Current main still constructs a path-to-hash map unconditionally in the bulk reader. The three changed production modules have no differences between the pinned merge base and fetched main, so the cleanup remains distinct. Release comparison: The supplied latest release, v2026.9.1, contains the same source-reader blob as fetched main, including the eager map. This PR's cleanup is therefore absent from that release.
Findings	None	None.
Security	None	None.

How this fits together

Memory Core compares workspace files and session transcripts with their indexed SQLite records to decide what needs indexing or removal. The changed reader supplies snapshots to diagnostic status, startup reconciliation, and synchronization.

flowchart TD
  A[Workspace files and session transcripts] --> B[Memory Core inspection and sync]
  C[Indexed SQLite source records] --> D[Source snapshot reader]
  D --> B
  B --> E{Changed or removed sources?}
  E --> F[Report diagnostic state]
  E --> G[Index changes or prune stale entries]

Loading

Before merge

• Add data-model compatibility proof - Confirm migration or upgrade compatibility proof before merge.

Agent review details

Security

None.

PR surface

Source -3, Tests -35, Other -1. Total -39 across 6 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	32	35	-3
Tests	2	79	114	-35
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	1	0	1	-1
Total	6	111	150	-39

Review metrics

Metric	Value	Why it matters
Production versus test footprint	Production net −3 lines; tests net −35; assertion baseline −1 entry	The cleanup removes unnecessary production structure and replaces mock-shaped fixtures without adding a test-only production seam.
Native allocation observation	20,000 unused map entries → 0; query counts unchanged	The supplied owner probe demonstrates the intended allocation reduction without claiming latency or peak-memory improvements.

Stored data model

Persistent data-model change detected: unknown-data-model-change: extensions/memory-core/src/memory/manager-source-state.ts, vector/embedding metadata: extensions/memory-core/src/memory/manager-source-state.ts. Confirm migration or upgrade compatibility proof before merge.

Technical review

Best possible solution:

Keep one canonical row reader and construct lookups only where consumers need them, preserving full snapshots and existing query timing.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction: this is behavior-preserving cleanup, and current source directly shows the unnecessary eager map allocation.

Is this the best way to solve the issue?

Yes. Removing the unused result structure at its producer avoids flags or parallel readers, while retaining consumer-owned snapshots; narrowing targeted queries would be separate work with different snapshot semantics.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against bf9cbcfb064d.

Labels

Label changes:

• add P3: This is a bounded behavior-preserving allocation and query-structure cleanup, not an urgent broken user workflow.
• add proof: sufficient: Contributor real behavior proof is sufficient. The supplied native Memory CLI and SQLite transcript exercises the changed reader through diagnostic inspection, indexing, deletion/pruning, and fresh-process reopen, with matching row and search results; the later head changes only tests.
• add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.

Label justifications:

• P3: This is a bounded behavior-preserving allocation and query-structure cleanup, not an urgent broken user workflow.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• proof: sufficient: Contributor real behavior proof is sufficient. The supplied native Memory CLI and SQLite transcript exercises the changed reader through diagnostic inspection, indexing, deletion/pruning, and fresh-process reopen, with matching row and search results; the later head changes only tests.

Evidence

What I checked:

• Introduced scope and proof continuity: The pinned merge-base-to-head delta contains only the Memory Core reader/caller cleanup, two test updates, and removal of the corresponding assertion-baseline entry. The later commit changes only the synchronization-yield test, so production code matches the head identified in the supplied native proof. The stale test merge was not used to infer removals from main. (extensions/memory-core/src/memory/manager-sync-yield.test.ts:91, 8e5dd454501a)
• Current-main necessity: Current main still constructs a path-to-hash map unconditionally in the bulk reader. The three changed production modules have no differences between the pinned merge base and fetched main, so the cleanup remains distinct. (extensions/memory-core/src/memory/manager-source-state.ts:99, bf9cbcfb064d)
• Release comparison: The supplied latest release, v2026.9.1, contains the same source-reader blob as fetched main, including the eager map. This PR's cleanup is therefore absent from that release. (extensions/memory-core/src/memory/manager-source-state.ts:99, ad6fe23aecb9)
• Reader and caller invariants: The reader retains the complete path/hash/mtime/size projection and source predicate; point reads retain both path and source predicates. Supplied snapshots, including empty maps, still take precedence. All production callers were traced: memory synchronization builds its required map immediately after the read, while session planning retains its existing snapshot owner. (extensions/memory-core/src/memory/manager-source-state.ts:85, 8e5dd454501a)
• Execution and storage boundary: The existing SDK helper compiles queries and synchronously consumes native SQLite iteration; the first-row helper selects the first materialized row. The canonical schema enforces unique path/source pairs and REAL modification times. Schema, database lifecycle, SDK exports, and dependency pins are unchanged. These imports establish SQLite/Kysely dependence, not a Codex runtime or protocol dependence. (src/infra/kysely-sync.ts:239, 8e5dd454501a)
• Behavioral test coverage: The replacement tests exercise source separation, fractional modification times, absent rows, and populated/empty snapshot precedence using the real schema. Yield tests retain their responsiveness assertions with explicit connection cleanup. Existing manager tests cover offline edits, canonical session discovery, and pruning without re-embedding survivors; startup tests also protect failed-enumeration preservation and corpus-snapshot routing. (extensions/memory-core/src/memory/manager-source-state.test.ts:14, 8e5dd454501a)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Hannes Rudolph: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[steipete]

The automated data-model compatibility prompt is a false positive for this read-only cleanup. The changed result shape is a private in-memory return value: callers receive the same path, hash, mtime, and size rows, and the one caller needing a hash map constructs it immediately at the same snapshot boundary. Table definitions, schema versions, stored column types, vector/embedding representations, writers, migration paths, and SDK exports are unchanged. No migration is appropriate for removing an unused JavaScript map.

Compatibility is covered by the native before/after and CLI proof: the same prepopulated 20,000 SQLite rows retain their complete digest, source filtering and query counts; existing file/session indexing and deletion preserve surviving entries; a fresh process reopens and searches the durable store successfully. The follow-up commit changes only the responsiveness test fixture. Its actual SQLite regression and unchanged responsiveness assertions pass, and current-head CI run 33932828535 succeeded.

I reviewed the complete latest ClawSweeper result: no code/security findings or rank-up moves remain. Direct installed Kysely 0.29.5 compiler/query-builder inspection and a fresh independent P0–P2 review also passed. The requested compatibility evidence is satisfied by unchanged persistent contracts and existing-store/reopen proof above; no schema or upgrade change is being waived.

[steipete]

Merged via squash.

• Prepared head SHA: 8e5dd454501ab9e82cbda8623aeb46670395ac2d
• Landed commit: 48d6b2cce4ae58d7c3c6bdc4749d851357ad6666