Skip to content

feat(viking): defer viking_remember to turn-boundary capture with memory-diff steer - #355

Merged
Leoyzen merged 1 commit into
mainfrom
viking-remember-deferral
Aug 6, 2026
Merged

feat(viking): defer viking_remember to turn-boundary capture with memory-diff steer#355
Leoyzen merged 1 commit into
mainfrom
viking-remember-deferral

Conversation

@Leoyzen

@Leoyzen Leoyzen commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Redesigns viking_remember (OpenSpec change viking-remember-deferral) from a synchronous messages=[...] tool into a deferred, zero-argument capture:

  • Tool call only queues the optional reason; the next before_model_request drains the real conversation (shared cursor with auto_ingest, disjoint ranges, remembered-capture runs before auto-ingest in the chain) into a remember-* session with <memory-intent> markers.
  • Sanitization is unconditional (deferred capture runs after recall/profile injection).
  • Commit is synchronous (HTTP round-trip only; Phase 2 extraction not awaited). On success: cursor advances + reasons clear. On failure: cursor and reasons are retained for retry (capped at 3 consecutive failures).
  • A background task polls the extraction task and steers the memory diff (added/updated/deleted URIs) into the session via session_pool.steer_from_background_task (survives run boundaries), gated by new config remember_notify (default True).
  • after_run now flushes pending remember intents plus the trailing [cursor, end] messages — closing the historical gap where the final assistant message was never ingested. The dead _pending_conversation field is removed (was written, never read).

Bugs fixed along the way

  1. Dead _pending_conversation — final assistant turn was silently never ingested.
  2. commit_session retention kwarg renamed keep_recent_turn_countkeep_recent_count (the SDK's real name; the old name raised TypeError against a live server whenever auto_ingest_keep_recent_turns > 0 — hidden by the canned mock).
  3. source_type was a dead parameter — verified create_session/commit_session have no such SDK surface; provenance is now expressed via the session-id prefix (remember- / ingest-), per updated spec.

SDK discovery

openviking_sdk.AsyncHTTPClient signatures were inspected directly: create_session(session_id, telemetry, memory_policy), add_message(session_id, role, content, ...), commit_session(session_id, telemetry, *, keep_recent_count). No source_type surface exists.

Testing

  • 434 viking tests pass (baseline 420) — unit + integration; new coverage: deferral, reason merge, retry semantics (cursor/reasons retention + cap), sanitize-always, provenance prefix, before_model_request chaining with auto_ingest disabled/enabled (disjoint ranges), after_run flush (final-assistant + last-moment remember + no-capture-when-all-disabled gate), steer positive/failure paths, task-poll failed-status.
  • Ruff + mypy clean.
  • Full suite: 6792 passed; only failure is the pre-existing test_providers/test_multimodal.py::test_vision (ALLOW_MODEL_REQUESTS gate) — reproduced on main.
  • Deferred (need a live Viking server, human step): VCR cassettes (tasks 7.7/8.2–8.4). The notify pipeline is deterministically unit-tested; record with uv run pytest tests/capabilities/viking/ --record-mode=once and commit the cassettes when a server is available.

…ory-diff steer

viking_remember() becomes a zero-argument deferred capture: the tool only
queues the optional reason, and the next before_model_request drains the
real conversation (shared cursor with auto_ingest, disjoint ranges) into
a remember-* session with <memory-intent> markers. Commit is synchronous
(HTTP round-trip only); on success the cursor advances and reasons clear,
on failure they are retained for retry (capped at 3).

A background task polls the extraction task and steers the memory diff
(added/updated/deleted URIs) into the session via
session_pool.steer_from_background_task, gated by remember_notify.

after_run now flushes pending remember intents plus the trailing
[cursor, end] messages, closing the long-standing gap where the final
assistant message was never ingested (dead _pending_conversation field
removed).

Also fixes: commit retention kwarg renamed to the SDK's real
keep_recent_count (old name TypeErrored against a live server whenever
auto_ingest_keep_recent_turns > 0), and the canned mock client is
extracted to a shared tests/capabilities/viking/conftest.py

Tests: 434 viking tests pass (was 420); new coverage for deferral,
merge, retry semantics, sanitize-always, provenance prefix, after_run
flush gate, and the steer notify path.
@Leoyzen
Leoyzen merged commit 380fb4d into main Aug 6, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant