Skip to content

afterTurn: store messages with actual roles and skip heartbeat messages#1340

Merged
Mijamind719 merged 1 commit intovolcengine:mainfrom
wlff123:oc_ov_fix
Apr 9, 2026
Merged

afterTurn: store messages with actual roles and skip heartbeat messages#1340
Mijamind719 merged 1 commit intovolcengine:mainfrom
wlff123:oc_ov_fix

Conversation

@wlff123
Copy link
Copy Markdown
Contributor

@wlff123 wlff123 commented Apr 9, 2026

  • Store messages with their actual roles (user/assistant) instead of merging all into a single user message
  • Map toolResult to user role, merge adjacent same-role messages
  • Sanitize only from user content, not assistant
  • Add extractSingleMessageText helper in text-utils
  • Skip heartbeat messages via isHeartbeat flag and content detection

Made-with: Cursor

Description

Related Issue

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

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

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)

Additional Notes

- Store messages with their actual roles (user/assistant) instead of merging
  all into a single user message
- Map toolResult to user role, merge adjacent same-role messages
- Sanitize <relevant-memories> only from user content, not assistant
- Add extractSingleMessageText helper in text-utils
- Skip heartbeat messages via isHeartbeat flag and content detection

Made-with: Cursor
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 92
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 9, 2026

PR Code Suggestions ✨

No code suggestions found for the PR.

@Mijamind719 Mijamind719 merged commit 68e4d89 into volcengine:main Apr 9, 2026
5 of 6 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Apr 9, 2026
zeattacker pushed a commit to zeattacker/OpenViking that referenced this pull request Apr 10, 2026
…gine#1340 hand-merged)

Cherry-picked upstream 68e4d89 with manual conflict resolution against
our prior plugin commit acbb7f9 (pinned context injection, recall refactor).

Three changes from volcengine#1340:

1. Role-preserving message storage (the main feature):
   Instead of collapsing entire turns into a single 'user' addSessionMessage
   call, iterate turnMessages and group adjacent same-role messages by
   actual role (user/assistant). Sends one addSessionMessage per group,
   preserving conversation structure for OpenViking. Replaces the previous
   join-everything-as-user logic.

2. Heartbeat filters (defense in depth, not replacement):
   - Session-level: 'if (afterTurnParams.isHeartbeat) return;' early-return
     when the calling code flags the whole turn as a heartbeat.
   - Per-message: HEARTBEAT_RE = /\bHEARTBEAT(?:\.md|_OK)\b/ drops individual
     heartbeat messages within a turn.
   These are narrow (uppercase HEARTBEAT only) and complement — not replace —
   our broader server-side trivial filter at openviking/session/session.py
   _is_trivial_session, which catches lowercase heartbeat/health_check/ping
   for ANY ingest path (CLI, eval, future clients), not just the plugin.

3. extractSingleMessageText helper in text-utils.ts (additive).

Hand-merge decisions vs upstream:

- Imports collision: kept both extractLastAssistantText (ours) and
  extractSingleMessageText (volcengine#1340).
- Backoff guard order: heartbeat early-return placed BEFORE the consecutive-
  failure backoff guard so heartbeats never increment failure counters.
- addSessionMessage call arity fix: volcengine#1340's loop calls 5-arg form
  (id, role, content, agentId, createdAt) which would put agentId in our
  parts? slot. Updated to 6-arg form (id, role, content, undefined, agentId,
  createdAt) matching client.ts signature.
- Inline commit block at the end of volcengine#1340: dropped. Our architecture routes
  commits through doCommitOVSession (separate function with alignment +
  drift detection hooks). Reintroducing the inline commit would conflict
  with that routing.

Co-Authored-By: claude-flow <ruv@ruv.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants