Skip to content

fix(session): vectorize memory parent directories on SessionCommit - #3665

Open
huangruiteng wants to merge 3 commits into
mainfrom
fix/issue-3640-missing-parent-vectors
Open

fix(session): vectorize memory parent directories on SessionCommit#3665
huangruiteng wants to merge 3 commits into
mainfrom
fix/issue-3640-missing-parent-vectors

Conversation

@huangruiteng

@huangruiteng huangruiteng commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Description

Fixes #3640: SessionCommit could write memory leaf files and their .overview.md without enqueuing the L1 directory vectors, leaving parent directories without vector records. Hierarchical retrieval returned zero results until a vectors_only reindex rebuilt the subtree. Default memory files (identity.md, soul.md) written by initialize_memory_files were likewise never embedded.

Root causes:

  1. generate_overview() wrote .overview.md after _vectorize_memories() ran, so the newly written overview file was never vectorized.
  2. initialize_memory_files() wrote defaults directly through viking_fs.write_file() with no embedding enqueue.

Human Involvement

  • A human participated in the implementation or review loop
  • This PR was generated entirely by AI agents without human participation in the loop

Related Issue

Closes #3640.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test update

Changes Made

  • Reordered apply_operations: overview generation now happens before _vectorize_memories; written overview URIs are collected and passed as a new overview_uris parameter.
  • _vectorize_memories enqueues one L1 OVERVIEW record per overview URI via vectorize_directory_meta(include_abstract=False). No L0 abstract record is emitted (no .abstract.md written); .overview.md is kept out of written_uris so the leaf flow is unchanged.
  • vectorize_directory_meta gains include_abstract (default True) so callers that only wrote .overview.md can skip the L0 record.
  • initialize_memory_files returns the list of URIs it newly created; pre-existing files are skipped and not returned.
  • MemoryUpdater.initialize_and_vectorize_default_files (new shared classmethod) is used by compressor v2 and v3 so newly initialized defaults are embedded on first session; pre-existing defaults are not re-embedded. Removes duplicated v2/v3 initialization plumbing.

Leaf/level invariants: leaf memory files continue through the standard ContextLevel.DETAIL (L2) flow with is_leaf=True; each parent directory receives exactly one ContextLevel.OVERVIEW (L1) record with is_leaf=False, context_type="memory", and the directory URI (not the .overview.md URI); no L0 ABSTRACT record is emitted for session-commit overviews.

Testing

  • tests/unit/session/memory/test_issue_3640_parent_vectors.py — 7 new tests: initialize_memory_files returns only newly created URIs (pre-existing skipped, schemas without init_value produce nothing); initialize_and_vectorize_default_files (v2/v3) vectorizes each new URI and is a no-op when nothing is created or vikingdb lacks a queue manager; apply_operations emits exactly one L1 OVERVIEW per parent, keeps .overview.md out of written_uris, routes the leaf through L2 DETAIL, and emits no overview when there are no upserts.
  • tests/unit/test_vectorize_file_strategy.py — 2 new tests: include_abstract=False produces exactly one OVERVIEW record; default behavior still produces L0 ABSTRACT + L1 OVERVIEW.
  • 43 targeted tests pass (test_issue_3640_parent_vectors.py + test_vectorize_file_strategy.py); full CI green on the latest push.
  • ruff check / ruff format clean on all changed files (a pre-existing import-sort warning on origin/main in compressor_v2.py is outside this PR's scope).

Risk

Classification/flow is localized to memory overview vectorization; leaf embedding and request paths are unchanged. New or unrecognized cases fall through to existing behavior.

Checklist

  • My code follows the project's coding style
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Screenshots (if applicable)

N/A — backend logic changes with no UI impact.

Additional Notes

This PR was generated entirely by AI agents. All tests pass locally and in CI.

SessionCommit could write memory leaf files and their .overview.md
without enqueuing the L1 directory vectors, leaving parent
directories without vector records. Hierarchical retrieval then
returned zero results until a vectors_only reindex rebuilt the
subtree. Default memory files (identity.md, soul.md) written by
initialize_memory_files were likewise never embedded.

Two root causes:

1. generate_overview() wrote .overview.md after _vectorize_memories()
   ran, so the new overview file was never vectorized.
2. initialize_memory_files() wrote defaults directly through
   viking_fs.write_file() with no embedding enqueue.

Fixes:

- Reorder apply_operations so overview generation happens before
  _vectorize_memories; collect the written overview URIs and pass
  them as a new overview_uris parameter.
- _vectorize_memories enqueues one L1 OVERVIEW record per overview
  URI via vectorize_directory_meta(include_abstract=False), reusing
  the shared owner/timestamp/telemetry path. No L0 abstract record
  is emitted because no .abstract.md was written; overview files are
  kept out of written_uris so the leaf flow is unchanged.
- vectorize_directory_meta gains include_abstract (default True) so
  callers that only wrote .overview.md can skip the L0 record.
- initialize_memory_files returns the list of URIs it newly created;
  pre-existing files are skipped and not returned.
- Add MemoryUpdater.initialize_and_vectorize_default_files and use
  it from compressor v2/v3 so newly initialized defaults are embedded
  on first session. Pre-existing defaults are not re-embedded.

Tests:
- tests/unit/session/memory/test_issue_3640_parent_vectors.py covers
  initialize_memory_files return semantics, the shared v2/v3 helper
  (success / no-op / vikingdb-unavailable), and apply_operations
  emitting exactly one L1 OVERVIEW per parent while keeping the leaf
  at L2 DETAIL and .overview.md out of written_uris.
- tests/unit/test_vectorize_file_strategy.py covers
  include_abstract=False producing exactly one OVERVIEW record and
  the default still producing L0+L1.

Fixes #3640
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Gentle review ping — this is ready for maintainer review.

  • CI: all green (plugin-tests, API & CLI integration, check-deps).
  • Mergeable: up-to-date with main, clean merge tree.
  • Fixes [Bug]: SessionCommit can leave memory parent directories without vector records #3640: SessionCommit now vectorizes parent directories and default memory files; overview generation is reordered before vectorization so .overview.md is included.
  • Tests: 43 pass across test_issue_3640_parent_vectors.py and test_vectorize_file_strategy.py; ruff/black clean on changed files.
  • Invariants preserved: leaf files keep L2 DETAIL/is_leaf=True; each parent gets exactly one L1 OVERVIEW; no L0 ABSTRACT for session-commit overviews.

Happy to address any feedback on the same branch.

@huangruiteng huangruiteng added the urgency:bug Incorrect behavior with a bounded fix path. label Jul 31, 2026
@huangruiteng
huangruiteng requested a review from chenjw July 31, 2026 21:09
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

@chenjw this touches the memory engine (openviking/session/memory, vectorize_directory_meta) per the maintainer routing map (Memory Engine primary); routing it to you for review, with @qin-ctx as backup. The fix reorders overview generation before vectorization and embeds default memory files so parent L1 vectors aren't skipped. 43 tests pass; CI green and mergeable.

Ruff lint fix + format over the changed test file; no behavior change.
@huangruiteng
huangruiteng requested a review from t0saki July 31, 2026 21:52
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Friendly second-round ping — this is ready for review and merge.

Since the first ping I ran a ruff format pass over the changed files (pushed in e33ff176), so the diff is now style-clean; the pre-existing I001 in compressor_v2.py is on origin/main and outside this PR's scope.

Verification on the latest push:

  • 43 targeted tests pass (test_issue_3640_parent_vectors.py, test_vectorize_file_strategy.py)
  • Full CI green, branch up-to-date with main

Summary: overview generation is reordered before vectorization so each parent directory gets an L1 OVERVIEW before child/file chunk vectorization, fixing missing parent vectors (#3640).

@t0saki you're the most recent committer to the session/memory files touched here — would you have time to approve? @chenjw (Memory Engine owner) / @qin-ctx also welcome.

@huangruiteng
huangruiteng requested a review from zhoujh01 July 31, 2026 22:02
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Updated the PR description to match the repository PULL_REQUEST_TEMPLATE.md (Description / Human Involvement / Type of Change / Changes Made / Testing). No code changes — the implementation and green CI are unchanged. Ready for review.

…g-parent-vectors

# Conflicts:
#	openviking/session/memory/memory_updater.py
#	openviking/utils/embedding_utils.py
#	tests/unit/test_vectorize_file_strategy.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

urgency:bug Incorrect behavior with a bounded fix path.

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: SessionCommit can leave memory parent directories without vector records

2 participants