Skip to content

Fix graph queue legacy LLM handling#4401

Open
atobiszei wants to merge 6 commits into
mainfrom
atobiszei_fix_legacy_llm_queue
Open

Fix graph queue legacy LLM handling#4401
atobiszei wants to merge 6 commits into
mainfrom
atobiszei_fix_legacy_llm_queue

Conversation

@atobiszei

@atobiszei atobiszei commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

This makes each requests create new LegacyServableExecutionContext.

@atobiszei
atobiszei requested a review from Copilot July 27, 2026 12:23
@atobiszei
atobiszei marked this pull request as ready for review July 27, 2026 12:23
Comment on lines +173 to +178
if (node.calculator() != "HttpLLMCalculator") {
continue;
}
bool hasExecutionContextSidePacket = false;
for (const auto& sidePacket : node.input_side_packet()) {
if (sidePacket.find("LLM_NODE_EXECUTION_CONTEXTS") != std::string::npos) {

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.

Would it make sense to wrap names in consts here?

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates MediaPipe graph-queue handling for legacy HttpLLMCalculator graphs by introducing an execution-context side packet requirement and making OVMS_GRAPH_QUEUE_MAX_SIZE: AUTO resolve to a concrete, container-aware queue size.

Changes:

  • Add LLM_NODE_EXECUTION_CONTEXTS side packet to generated/exported graphs and test graph templates/fixtures.
  • Resolve graph-queue AUTO to min(restWorkers, getCoreCount) and disable graph queue when legacy HttpLLMCalculator graphs are missing the execution-context side packet.
  • Adjust graph-queue related tests and wiring (including Bazel deps) to reflect the new behavior.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/test/pull_hf_model_test.cpp Updates expected graph content to include LLM_NODE_EXECUTION_CONTEXTS side packet.
src/test/mediapipeflow_test.cpp Strengthens graph-queue AUTO and legacy-LLM compatibility coverage.
src/test/llm/visual_language_model/vlm_legacy_regular.pbtxt Adds LLM_NODE_EXECUTION_CONTEXTS side packet for LLM node.
src/test/llm/visual_language_model/vlm_cb_regular.pbtxt Adds LLM_NODE_EXECUTION_CONTEXTS side packet for LLM node.
src/test/llm/lm_legacy_regular.pbtxt Adds LLM_NODE_EXECUTION_CONTEXTS side packet for LLM node.
src/test/llm/lm_cb_with_tool_parser.pbtxt Adds LLM_NODE_EXECUTION_CONTEXTS side packet for LLM node.
src/test/llm/lm_cb_speculative.pbtxt Adds LLM_NODE_EXECUTION_CONTEXTS side packet for LLM node.
src/test/llm/lm_cb_regular.pbtxt Adds LLM_NODE_EXECUTION_CONTEXTS side packet for LLM node.
src/test/llm/lm_cb_prompt_lookup.pbtxt Adds LLM_NODE_EXECUTION_CONTEXTS side packet for LLM node.
src/test/graph_export_test.cpp Updates expected exported graph content to include execution-context side packet.
src/mediapipe_internal/mediapipegraphdefinition.hpp Adds resolveAutoQueueSize() declaration.
src/mediapipe_internal/mediapipegraphdefinition.cpp Implements AUTO queue-size resolution and disables queue for legacy LLM graphs missing execution context.
src/mediapipe_internal/mediapipegraphconfig.hpp Simplifies queue-size storage to std::optional<int> and adds clearGraphQueueSize().
src/mediapipe_internal/graphqueue.cpp Uses member sidePacketMaps consistently during graph helper initialization.
src/mediapipe_internal/BUILD Adds dependency on //src:libovms_systeminfo for core-count based sizing.
src/graph_export/graph_export.cpp Adds execution-context side packet to generated text-generation graph template.

Comment thread src/test/mediapipeflow_test.cpp
Comment thread src/mediapipe_internal/mediapipegraphconfig.hpp
Comment thread src/mediapipe_internal/mediapipegraphdefinition.cpp Outdated
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.

4 participants