Skip to content

refactor(memory): remove legacy memory v1#2264

Merged
chenjw merged 1 commit into
mainfrom
refactor/memory-v2-only
May 27, 2026
Merged

refactor(memory): remove legacy memory v1#2264
chenjw merged 1 commit into
mainfrom
refactor/memory-v2-only

Conversation

@qin-ctx
Copy link
Copy Markdown
Collaborator

@qin-ctx qin-ctx commented May 27, 2026

Description

Remove the legacy memory v1 implementation now that memory v2 is the only supported runtime path.

Related Issue

N/A

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

  • Delete legacy v1 compressor, extractor, deduplicator modules, v1 compression prompt templates, and v1-only tests.
  • Make memory configuration and create_session_compressor v2-only, rejecting memory.version = "v1" explicitly.
  • Update docs, prompt guide references, sample dataset text, and remaining tests to point at the v2 memory path.
  • Remove tests that referenced the missing SchemaPromptGenerator helper instead of adding that helper to production code in this cleanup PR.

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested this on the following platforms:
    • Linux
    • macOS
    • Windows

Commands run:

  • .venv/bin/python -m pytest -q --no-cov tests/test_config_loader.py tests/misc/test_semantic_config.py (38 passed)
  • .venv/bin/python -m compileall -q openviking/session openviking/service openviking_cli/utils/config tests/test_config_loader.py tests/misc/test_semantic_config.py tests/session/test_commit_skill_inference.py tests/session/memory/test_memory_react.py
  • git diff --check
  • rg scan for legacy memory v1 modules/classes/prompts and SchemaPromptGenerator references (no matches)

Screenshots (if applicable)

N/A

Additional Notes

A broader run including tests/session/test_commit_skill_inference.py is currently blocked by an existing main-branch LocalClient abstract method gap: LocalClient does not implement BaseClient.batch_add_messages. This PR intentionally does not add production code for that unrelated test setup issue.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 85
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 Multiple PR themes

Sub-PR theme: Add LocalClient.batch_add_messages implementation

Relevant files:

  • openviking/client/local.py

Sub-PR theme: Add SchemaPromptGenerator for memory schema prompt generation

Relevant files:

  • openviking/session/memory/schema_model_generator.py

⚡ Recommended focus areas for review

Potential regression: create_session_compressor no longer reads config for memory_version

The create_session_compressor function no longer reads the memory.version from the config when memory_version is None. While the config validator now rejects v1, this changes the previous behavior where the config was the source of truth for the memory version.

def create_session_compressor(
    vikingdb: VikingDBManager,
    memory_version: Optional[str] = None,
    skill_processor=None,
) -> "SessionCompressorV2":
    """
    Create the v2 session compressor.

    Args:
        vikingdb: VikingDBManager instance
        memory_version: Deprecated optional override. Only "v2" is supported.

    Returns:
        SessionCompressorV2 instance
    """
    if memory_version not in (None, "v2"):
        raise ValueError("memory.version only supports 'v2'; legacy memory v1 has been removed")

    logger.info("Using v2 memory compressor (templating system)")
    from openviking.session.compressor_v2 import SessionCompressorV2

    return SessionCompressorV2(vikingdb=vikingdb, skill_processor=skill_processor)
Type safety: registry_or_schemas is typed as Any

The SchemaPromptGenerator.init accepts registry_or_schemas as Any, which reduces type safety. Consider using a Protocol or Union of expected types for better type checking.

def __init__(
    self,
    registry_or_schemas: Any,
    template_context: Optional[Dict[str, Any]] = None,
):

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Replace quadratic schema lookup with dictionary for O(n) performance

Fix quadratic loop in SchemaPromptGenerator by pre-building a memory-type-to-schema
lookup dictionary. The current implementation iterates over all schemas for every
schema (O(n²)) when generating type descriptions, which can be optimized to O(n)
using a dict.

openviking/session/memory/schema_model_generator.py [359-401]

 def __init__(
     self,
     registry_or_schemas: Any,
     template_context: Optional[Dict[str, Any]] = None,
 ):
     if hasattr(registry_or_schemas, "list_all"):
         self.schemas = list(registry_or_schemas.list_all(include_disabled=True))
     else:
         self.schemas = list(registry_or_schemas or [])
     self._template_context = dict(template_context or {})
+    self._schema_by_type = {schema.memory_type: schema for schema in self.schemas}
 
 def _render(self, text: str) -> str:
     if not text:
         return text
     if "{{" not in text and "{%" not in text and "{#" not in text:
         return text
     return TemplateUtils.render(text, self._template_context, strip=False)
 
 def generate_type_descriptions(self) -> str:
     lines = ["## Available Memory Types"]
     for schema in self.schemas:
         lines.append("")
         lines.append(f"### {schema.memory_type}")
         if schema.description:
             lines.append(self._render(schema.description))
         field_descriptions = self.generate_field_descriptions(schema.memory_type)
         if field_descriptions:
             lines.append(field_descriptions)
     return "\n".join(lines).strip()
 
 def generate_field_descriptions(self, memory_type: str) -> Optional[str]:
-    schema = next((item for item in self.schemas if item.memory_type == memory_type), None)
+    schema = self._schema_by_type.get(memory_type)
     if schema is None:
         return None
 
     lines = [f"### {schema.memory_type} Fields"]
     for field in schema.fields:
         field_type = getattr(field.field_type, "value", field.field_type)
         merge_op = getattr(field.merge_op, "value", field.merge_op)
         description = self._render(field.description)
         suffix = f": {description}" if description else ""
         lines.append(f"- `{field.name}` ({field_type}, {merge_op}){suffix}")
     return "\n".join(lines)
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies and fixes a quadratic loop in SchemaPromptGenerator by adding a memory-type-to-schema lookup dictionary, improving performance from O(n²) to O(n) for type descriptions generation. This is a valid optimization with moderate impact on code efficiency.

Low

@qin-ctx qin-ctx force-pushed the refactor/memory-v2-only branch from 1632240 to 05c9b09 Compare May 27, 2026 11:27
@chenjw chenjw merged commit 96df42f into main May 27, 2026
6 of 7 checks passed
@chenjw chenjw deleted the refactor/memory-v2-only branch May 27, 2026 11:49
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project May 27, 2026
qin-ctx pushed a commit that referenced this pull request May 28, 2026
* docs(memory): document version field that now rejects v1 (EN)

* docs(memory): document version field that now rejects v1 (ZH)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants