Skip to content

fix(session): retry recoverable memory extraction failures - #4437

Open
CryoThrust wants to merge 1 commit into
volcengine:mainfrom
CryoThrust:fix/session-memory-extraction-retry
Open

fix(session): retry recoverable memory extraction failures#4437
CryoThrust wants to merge 1 commit into
volcengine:mainfrom
CryoThrust:fix/session-memory-extraction-retry

Conversation

@CryoThrust

Copy link
Copy Markdown

Summary

  • allow queued session commits to retry recoverable Phase 2 memory extraction failures
  • preserve terminal handling for corrupt archives, missing messages, and cancelled commits
  • add regression coverage for replaying an archive after a transient extraction failure

Fixes #4419

Validation

  • python -m py_compile openviking/session/session.py tests/unit/session/test_session_commit_resume.py
  • git diff --check
  • targeted pytest is blocked locally by a missing apscheduler dependency

@CryoThrust
CryoThrust force-pushed the fix/session-memory-extraction-retry branch from 34eea51 to cf9e2bc Compare August 28, 2026 09:03

@linhongyu510 linhongyu510 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new retry branch is not reachable through the real queue lifecycle after the original extraction failure. _run_memory_extraction() records .failed.json and returns normally; resume_queued_commit() therefore returns True, SessionCommitProcessor.on_dequeue() completes normally, and QueueManager.process_one() unconditionally calls ack(), deleting the persistent message. Nothing invokes resume_queued_commit() a second time for that archive, and a later empty user commit creates no replacement work. The new test fabricates the failed marker and directly calls resume_queued_commit(), so it proves that a hypothetical replay would work but not that any replay is scheduled. Please add a processor/queue-level regression that fails extraction once, observes the first message lifecycle, then proves the same archive is actually re-enqueued/retried and reaches .done without a process restart. The implementation likely needs an explicit recoverable retry signal/re-enqueue before ack, with bounded retry/backoff semantics, rather than only handling a marker on an otherwise unreachable second invocation.

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

Labels

None yet

Projects

Status: Backlog

Development

Successfully merging this pull request may close these issues.

[Bug]: Failed memory extraction leaves archived sessions uncompactable

2 participants