Skip to content

fix(memory): normalize non-canonical extraction payloads#1416

Open
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:fix/issue-1410-memory-extraction
Open

fix(memory): normalize non-canonical extraction payloads#1416
yeyitech wants to merge 2 commits intovolcengine:mainfrom
yeyitech:fix/issue-1410-memory-extraction

Conversation

@yeyitech
Copy link
Copy Markdown
Contributor

Summary

  • normalize several common memory-extraction payload shapes into the expected {"memories": [...]} form
  • fall back to the stability parser when the primary JSON parser returns None
  • add regression tests for bare objects, wrapped single objects, and nested wrapper keys

Why

Closes #1410.

Some smaller local / OpenAI-compatible models appear to return payloads that are structurally close to the expected schema but not exact, for example:

  • a bare list of memory objects
  • a bare single memory object
  • {"memories": {...}} instead of {"memories": [{...}]}
  • wrapper keys like items or nested data.memories

Before this change those responses could normalize to an empty result, causing memories_extracted to stay empty even when the model had clearly understood the conversation.

Validation

  • python3 -m py_compile openviking/session/memory_extractor.py tests/session/test_memory_extractor_response_types.py
  • python3 -m pytest -o addopts='' -p no:asyncio --noconftest tests/session/test_memory_extractor_response_types.py tests/session/test_memory_extractor_language.py -q

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

1410 - Fully compliant

Compliant requirements:

  • Fix memory extraction returning empty dict when LLM executes successfully
  • Handle non-canonical response shapes from smaller local models
  • Add regression tests for various response formats
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 92
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Missing Observability

Removed warnings for non-canonical response shapes (e.g., list instead of dict). While normalization is desired, losing these warnings may make it harder to diagnose model behavior changes.

with telemetry.measure("memory.extract.stage.normalize_candidates"):
    data = parse_json_from_response(response)
    if data is None:
        stable_data, stable_error = parse_json_with_stability(response)
        if stable_data is not None:
            data = stable_data
        else:
            logger.warning(
                "Memory extraction stable parse failed: %s",
                stable_error,
            )
    data = self._normalize_extraction_payload(data)
Silent Filtering

_coerce_sequence silently filters non-dict items from lists without logging. This could hide model output issues where some memories are lost.

def _coerce_sequence(value) -> list:
    if isinstance(value, list):
        return [item for item in value if isinstance(item, dict)]
    if _is_memory_item(value):
        return [value]
    return []

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@yeyitech
Copy link
Copy Markdown
Contributor Author

Updated based on the review suggestions. I restored lightweight observability for normalized non-canonical extraction payloads and added regression coverage for the new logging paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug] Memory extraction LLM executes successfully but memories_extracted returns empty dict (qwen3:1.7b)

1 participant