Skip to content

fix(dsh-plugin): run profile and recall in parallel on pre-step - #4643

Merged
ZaynJarvis merged 1 commit into
volcengine:mainfrom
ktz03:fix/dsh-parallel-pre-step-recall
Sep 4, 2026
Merged

fix(dsh-plugin): run profile and recall in parallel on pre-step#4643
ZaynJarvis merged 1 commit into
volcengine:mainfrom
ktz03:fix/dsh-parallel-pre-step-recall

Conversation

@ktz03

@ktz03 ktz03 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

agent/pre-step in dsh-memory-plugin awaited profileMessage then recallMessage sequentially. That waterfall still runs before dsh-agent-loop appends user/message (#4515), so serial OV work adds avoidable latency to the blank chat window.

Run profile + recall concurrently via Promise.all after next() (ensureState is already single-flight safe).

This is a mitigation inside OpenViking's plugin; a complete UX fix still needs dsh-agent-loop to push user/message before the pre-step waterfall.

Related to #4515

Test plan

  • node --test examples/dsh-memory-plugin/index.test.mjs
  • With the plugin installed, send a message and confirm recall still injects; wall time of pre-step should drop when both profile and recall would have been slow

agent/pre-step currently gates user/message push in dsh-agent-loop. Overlap profileMessage and recallMessage to cut waterfall wall time.

Related to volcengine#4515.
@now-ing

now-ing commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Verified the parallelization is behavior-safe on all three fronts I could check statically:

  1. No double-initialization: both entry points call initialize(), but ensureState (runtime.mjs:61-68) already has in-flight promise sharing (if (state.initializing) return state.initializing), so concurrent callers join the same initialization.
  2. No data dependency: recallMessage(agent, decision.messages) consumes the decision as-is — it never consumed profile output, so running them concurrently cannot change results.
  3. Order stability: [profile, recall].filter(Boolean) preserves the message order downstream listeners expect.

Two notes:

  • The change ships with zero test coverage, and the harness existsindex.test.mjs is right there and currently has no case pinning the concurrency. Worth adding one: mock the runtime with a profileMessage that resolves only after recallMessage has been entered (a rendezvous via deferred promise), then assert the pre-step completes and both messages appear. That test reds instantly if this ever regresses back to sequential awaits, and it's ~15 lines.
  • Abort semantics shift slightly: the old code's post-profile signal.aborted check meant an abort during profiling skipped the recall network call entirely; now both are already in flight when the check runs, so an aborted step can still pay for one wasted recall request. Given recall results are discarded either way, this is a fine trade for the latency win — just noting the delta is real.

Nice, surgical fix for #4515's waterfall cost.

@ZaynJarvis ZaynJarvis added the agent-plugins Agent harness and plugin integrations label Sep 4, 2026

@ZaynJarvis ZaynJarvis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm, thx

@ZaynJarvis
ZaynJarvis merged commit cf18dfb into volcengine:main Sep 4, 2026
6 checks passed
@github-project-automation github-project-automation Bot moved this from Backlog to Done in OpenViking project Sep 4, 2026
@ktz03

ktz03 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @now-ing — added a rendezvous-style test on this head:

agent/pre-step runs profile and recall concurrentlyprofileMessage only resolves after recallMessage has entered. A regression back to sequential awaits fails the gate within 500ms.

Abort-semantics note acknowledged (wasted in-flight recall on abort is an accepted trade for the latency win).

skloxo pushed a commit to skloxo/OpenVikingStudio that referenced this pull request Sep 4, 2026
…engine#4643)

agent/pre-step currently gates user/message push in dsh-agent-loop. Overlap profileMessage and recallMessage to cut waterfall wall time.

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

Labels

agent-plugins Agent harness and plugin integrations

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants