Skip to content

Subagent Conversation Support - #935

Open
jaredoconnell wants to merge 24 commits into
vllm-project:mainfrom
jaredoconnell:feat/subagent-dag
Open

Subagent Conversation Support#935
jaredoconnell wants to merge 24 commits into
vllm-project:mainfrom
jaredoconnell:feat/subagent-dag

Conversation

@jaredoconnell

@jaredoconnell jaredoconnell commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR includes the initial implementation of the DAG, and basic synthetic data support to utilize that.

Details

  • Implements a DAG that uses nodes and edges. The edges specify dependency requirements for history.
  • Improves the conversation script to color code it and show the node's parents, and a full history path.
  • It's designed in a way to hopefully easily support trace dataset formats in the future.
  • The input format from the data deserializers is not the flattened design used in existing formats. This is to match both how the DAG needs it, and how typical trace dataset formats actually nest subagents.

Test Plan

  • I would recommend starting by running a benchmark using the examples given in the markdown to create simple subagents.

Related Issues

TBD


  • "I certify that all code in this PR is my own, except as noted below."

Use of AI

  • Includes code generated or substantially modified by an AI agent
  • Includes tests generated or substantially modified by an AI agent

NOTE: the Generated-by or Assisted-by trailers should be used in git commit messages when code or tests were generated or substantially modified by an AI agent, as described in the project's DEVELOPING.md file.


git log

commit 04fcfd8
Author: Jared O'Connell joconnel@redhat.com
Date: Wed Jul 8 11:42:13 2026 -0400

Initial DAG implementation

Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 92d996c
Author: Jared O'Connell joconnel@redhat.com
Date: Mon Jul 13 19:49:05 2026 -0400

Integrate DAG into full synthetic data pipeline

Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

commit 383dcb2
Author: Jared O'Connell joconnel@redhat.com
Date: Wed Jul 15 17:50:15 2026 -0400

Address review comments

Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 134719e
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 16 18:18:05 2026 -0400

Implemented requeue delay to DAG, and fixed formatting

Assisted-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit d87197c
Author: Jared O'Connell joconnel@redhat.com
Date: Fri Jul 17 00:30:57 2026 -0400

Redesign input format from dataset deserializers for subagent conversations

Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit bf0701f
Author: Jared O'Connell joconnel@redhat.com
Date: Fri Jul 17 01:03:47 2026 -0400

Make code compliant with all CI checks

Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit b5a8604
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 23 19:23:55 2026 -0400

Added way to make conversation extraction script less verbose

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 069de56
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 23 20:11:02 2026 -0400

Allow setting the merge turn for synthetic subagent branches

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit ae8fc51
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 23 20:23:53 2026 -0400

Clarify synthetic data sampling behavior

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 92e6c5c
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 23 22:35:47 2026 -0400

Address review comment

Assisted-by: Cursor AI
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit c744a83
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 23 22:41:46 2026 -0400

Address review comment. Switch root_node_ids to computed field

Generated-by: Cursor AI
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 4f24403
Author: Jared O'Connell joconnel@redhat.com
Date: Fri Jul 24 16:04:04 2026 -0400

Remove sorting from DAG

This makes the DAG function order based on the DAG node insertion order.

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit eee656f
Author: Jared O'Connell joconnel@redhat.com
Date: Fri Jul 24 17:26:07 2026 -0400

Address review feedback

Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 5c13cb8
Author: Jared O'Connell joconnel@redhat.com
Date: Fri Jul 24 17:59:43 2026 -0400

Refactor history building

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 752fc3e
Author: Jared O'Connell joconnel@redhat.com
Date: Tue Jul 28 14:15:20 2026 -0400

Fix linter errors

Generated-by: Cursor AI
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 75ec0f4
Author: Jared O'Connell joconnel@redhat.com
Date: Tue Jul 28 18:52:53 2026 -0400

Address review comments

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 3dbeed7
Author: Jared O'Connell joconnel@redhat.com
Date: Wed Jul 29 15:29:58 2026 -0400

Replace turn index with history length

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit f4e8da3
Author: Jared O'Connell joconnel@redhat.com
Date: Wed Jul 29 17:42:43 2026 -0400

Fix multiturn synthetic sampling bug, and add first prompt settings

This will allow us to better mock real world scenarios.

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 43d57e7
Author: Jared O'Connell joconnel@redhat.com
Date: Wed Jul 29 18:00:56 2026 -0400

Address review comments

Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit b46f1d4
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 30 10:58:14 2026 -0400

Add turn_index in addition to history_len

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 44c09d6
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 30 11:41:41 2026 -0400

Remove script accidentally added in prior commit

Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit ee61429
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 30 15:16:11 2026 -0400

Reduce duplication for the data pathway

With the changes to accept subagent setups, it got a little duplicated.

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit f0ff27d
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 30 16:14:29 2026 -0400

Address review feedback and fix linter error

Signed-off-by: Jared O'Connell <joconnel@redhat.com>

commit 5b54ffd
Author: Jared O'Connell joconnel@redhat.com
Date: Thu Jul 30 16:51:44 2026 -0400

Fix unit tests, and make E2E tests work with port conflict

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>

Assisted-by: Cursor AI
Assisted-by: Cursor AI Claude Opus 4.6
Co-authored-by: Cursor cursoragent@cursor.com
Generated-by: Cursor AI
Generated-by: Cursor AI Claude Opus 4.6
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell joconnel@redhat.com

@dbutenhof dbutenhof added internal filed by core contributor or associate feature Represents a new user-visible feature labels Jul 17, 2026

@SkiHatDuckie SkiHatDuckie 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.

Some comments on dag.py. Many of these are questions that will also help me better understand what the DAG is doing and why.

There will be multiple batches of comment/requests as I go through this, but one of them will be dedicated to seeing how well this interface would work with trace replays like WEKA.

Comment thread src/guidellm/scheduler/dag.py
Comment thread src/guidellm/scheduler/dag.py Outdated

:return: Sorted list of incomplete node IDs.
"""
return sorted(nid for nid in self._graph.nodes if nid not in self._completed)

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.

What is the purpose of sorting the list in alphanumeric order?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I removed the sorting. The ordering is now based on DAG node insertion order.

Comment thread src/guidellm/scheduler/dag.py Outdated
Comment thread src/guidellm/scheduler/dag.py Outdated
Comment thread src/guidellm/scheduler/dag.py Outdated
Comment thread src/guidellm/scheduler/dag.py
Comment thread src/guidellm/scheduler/dag.py Outdated
Comment thread src/guidellm/scheduler/dag.py

@dbutenhof dbutenhof 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.

I still haven't actually been able to finish this, but before I head out to hike I might as well submit my initial comments from Friday and this morning. 😁

Comment thread docs/guides/multiturn.md Outdated
Comment thread scripts/extract_conversation.py
Comment thread scripts/extract_conversation.py
Comment thread src/guidellm/data/deserializers/synthetic.py Outdated
Comment thread src/guidellm/data/deserializers/synthetic.py Outdated

@SkiHatDuckie SkiHatDuckie 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.

Treat these as one suggestion. I can't find a good way of suggesting changes across different lines without making a large diff.

Comment thread src/guidellm/scheduler/schemas/conversation.py Outdated
Comment thread src/guidellm/scheduler/schemas/conversation.py Outdated
Comment thread src/guidellm/scheduler/schemas/conversation.py Outdated
Comment thread src/guidellm/scheduler/schemas/conversation.py Outdated

@dbutenhof dbutenhof 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.

Some additional comments / questions on the first (DAG) commit. (Whew.)

Comment thread src/guidellm/scheduler/dag.py Outdated
Comment thread src/guidellm/scheduler/dag.py Outdated
Comment thread src/guidellm/scheduler/dag.py
Comment thread src/guidellm/scheduler/schemas.py Outdated

@dbutenhof dbutenhof 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.

OK, I started going through by commit, thinking it'd be easier, and then found out you removed the ENUMs and reorganized everything, so that exercise proved pointless. AH, well: I'll resume/restart tomorrow.

Comment thread src/guidellm/scheduler/worker_group.py Outdated
Comment thread src/guidellm/scheduler/schemas/backend.py
@dbutenhof dbutenhof added this to the v0.8.0 milestone Jul 25, 2026
@dbutenhof dbutenhof linked an issue Jul 27, 2026 that may be closed by this pull request

@sjmonson sjmonson 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.

Some initial notes on the worker code. Overall looks good, it was pretty easy to follow the code. Nits are a few inefficiencies and one bug.

Comment thread src/guidellm/scheduler/worker.py Outdated
Comment thread src/guidellm/scheduler/worker.py Outdated
Comment thread src/guidellm/scheduler/worker.py Outdated
Comment thread src/guidellm/scheduler/worker.py Outdated
@jaredoconnell
jaredoconnell marked this pull request as ready for review July 29, 2026 04:40

@sjmonson sjmonson 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.

Few more comments

Comment thread src/guidellm/scheduler/worker_group.py Outdated
node_id=node_id,
agent_id=node.agent_id,
parent_node_ids=incoming_map[node_id],
turn_index=i,

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.

turn_index should be the number of requests since initial, so branches increment turn index separately. Maybe it would be better to calculate this later from history length instead of here though.

@jaredoconnell jaredoconnell Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I replaced turn_index with history_len, if that's okay. I think that's a better metric for the new meaning, and turn_index would be misleading in nonlinear situations.

Comment thread src/guidellm/scheduler/worker.py Outdated
Comment on lines +364 to +376
now = time.time()
for state in self.turns_queue:
nxt = state.next_node_ready_at()
if nxt is not None and nxt[1] <= now:
node_id, _ = nxt
state.claim_node(node_id)
return state, node_id

earliest: float | None = None
for state in self.turns_queue:
nxt = state.next_node_ready_at()
if nxt is not None and (earliest is None or nxt[1] < earliest):
earliest = nxt[1]

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.

Maybe I am missing something but why do these need to be separate loops?

Suggested change
now = time.time()
for state in self.turns_queue:
nxt = state.next_node_ready_at()
if nxt is not None and nxt[1] <= now:
node_id, _ = nxt
state.claim_node(node_id)
return state, node_id
earliest: float | None = None
for state in self.turns_queue:
nxt = state.next_node_ready_at()
if nxt is not None and (earliest is None or nxt[1] < earliest):
earliest = nxt[1]
now = time.time()
earliest: float | None = None
for state in self.turns_queue:
nxt = state.next_node_ready_at()
if nxt is not None:
if nxt[1] <= now:
node_id, _ = nxt
state.claim_node(node_id)
return state, node_id
if earliest is None or nxt[1] < earliest:
earliest = nxt[1]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Merged.


raw = raw_values[0]
if isinstance(raw, str):
graph_data = ConversationGraphData.model_validate(json.loads(raw))

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.

Suggested change
graph_data = ConversationGraphData.model_validate(json.loads(raw))
graph_data = ConversationGraphData.model_validate_json(raw)

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.

Actually does this condition even make sense? Why would this ever be a string?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

According to the AI, yes it can be a string.

Yes, it is a string whenever the column comes from the synthetic branched dataset (or any HF/large_string JSON column). The condition exists because that payload is serialized for storage, not because the finalizer invents a string type.

I applied your suggestion locally.

Comment on lines +125 to +132
if turn.relative_timestamp is not None:
settings = settings.model_copy(
update={"relative_timestamp": turn.relative_timestamp}
)
if turn.requeue_delay is not None:
settings = settings.model_copy(
update={"requeue_delay": turn.requeue_delay}
)

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.

I am very confused by what is going on here. Why does ConversationTurnData have a duplicate of every request setting field? And why is this code coping the model instead of just assigning the value?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll look into this more tomorrow.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I simplified this to reduce duplication.

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.

I guess maybe the whole finalizer is a hack until #972 but I don't like how we have different code paths for column-wise vs single column datasets. Ideally at this point we are either all one or the other.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is a hard problem to solve without redoing everything. Because the existing pathways always assume it's flattened, but flattened data is very inefficient for multiturn conversations with subagents. But switching away from the existing flattened format as an option would break a lot of the advantages that the current column mapper provide.

So my thought is the dual code path format until we refactor the entire column mapper and preprocessor code sections.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

So the latest design for the new format is that there's one new column: "conversation_turns_column". It stores JSON data that follows the schema defined in src/guidellm/data/schemas/conversation_graph_data.py.

Comment on lines +173 to +174
"For multiturn, one value is sampled per conversation and applied "
"to every turn (including branch turns that do not override it). "

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.

This is a bug, it should be fixed not codified.

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.

Maybe fixed in a separate PR but please remove this and any other reference to "one value is sampled per conversation" that is not how this should work.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I ended up fixing it, and while I was at it I switched to a way to separately specify the distribution for the first prompts, since in real world scenarios, you often include a big payload in the first message.

@dbutenhof dbutenhof 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.

Quick skim through changes (all I've got brain left for right now); one minor new comment, and I believe I've resolved all my previous comments.

Comment thread src/guidellm/data/deserializers/synthetic.py Outdated
),
)
turn_index: int = Field(
history_len: int = Field(

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.

You can have a history_len field too if you want but turn_index should still exist and start from the first turn regardless of history,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Let me know what you think of the latest version of turn_index. Rather than the total history length up to that point, it's instead just the longest path to a parentless history node or new context history node. last can only count up to 1, but in most cases won't count, since in real world use cases the the "full" path is 1 or larger.

@SkiHatDuckie

Copy link
Copy Markdown
Collaborator

Could you give me a rundown of which parts of the DAG the trace replay datasets would be interfacing with? I'm also looking through the code myself so that I can start writing up a plan for the WEKA trace sub-agents while this is still in review.

jaredoconnell and others added 8 commits July 30, 2026 16:21
Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Assisted-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
…ations

Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Claude Opus 4.6
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Assisted-by: Cursor AI
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
This makes the DAG function order based on the DAG node insertion order.

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
This will allow us to better mock real world scenarios.

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
With the changes to accept subagent setups, it got a little duplicated.

Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
Generated-by: Cursor AI Grok 4.5
Signed-off-by: Jared O'Connell <joconnel@redhat.com>
@jaredoconnell

Copy link
Copy Markdown
Collaborator Author

Could you give me a rundown of which parts of the DAG the trace replay datasets would be interfacing with? I'm also looking through the code myself so that I can start writing up a plan for the WEKA trace sub-agents while this is still in review.

Take a look at src/guidellm/data/schemas/conversation_graph_data.py

That file is where the schema is defined for the new conversation_turns_column. It's just a new column that accepts the JSON serialized version of the types in conversation_graph_data.py, and therefore allows nested data. That one is recursive, as you can see that ConversationTurnData also has a columns field.

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

Labels

feature Represents a new user-visible feature internal filed by core contributor or associate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DAG support for agentic conversation flows

4 participants