langchain driver for OW. bug fixes along the way - #292
Merged
Merged
Conversation
dmjoy
requested changes
Sep 3, 2026
Vman11
force-pushed
the
dev/agentic-ow-driver
branch
from
September 9, 2026 14:10
e4e4893 to
3abcf86
Compare
Vman11
force-pushed
the
dev/agentic-ow-driver
branch
from
September 9, 2026 14:21
c4072fa to
31bba30
Compare
… fix on separate branch
dmjoy
approved these changes
Sep 11, 2026
dmjoy
left a comment
Contributor
There was a problem hiding this comment.
Thanks for the updates Vinnie. I'm mostly fine to merge this version in and we can follow up with another PR to address the driver -> (driver, adm) refactor and a non-random solution for parameter completion.
Comment on lines
+13
to
+26
| Why a LangChain chat model rather than a ``StructuredInferenceEngine`` | ||
| (e.g. ``align_system.algorithms.vllm_inference_engine``)? That | ||
| interface produces a single schema-constrained JSON completion per | ||
| prompt, while the agent loop needs a multi-turn chat model with native | ||
| tool calling -- binding tool schemas to the model, parsing tool calls | ||
| out of its responses, and feeding tool results back as messages -- | ||
| which is what LangChain's chat model classes provide. For vLLM in | ||
| particular, tool-call parsing is implemented only in its | ||
| OpenAI-compatible server (``vllm serve --enable-auto-tool-choice | ||
| --tool-call-parser ...``), not in the in-process ``vllm.LLM`` API the | ||
| inference engine wraps, so HuggingFace models are reached through | ||
| ``langchain_openai.ChatOpenAI`` pointed at a separately started ``vllm | ||
| serve`` process (see configs/driver/chat_model/vllm_*.yaml). | ||
| """ |
Contributor
There was a problem hiding this comment.
Good to know, thanks for adding this.
Contributor
There was a problem hiding this comment.
Again I don't like that the driver here is doing the work of driver and ADM; but we can split it out in a follow-up PR.
|
|
||
| # Fills in required-but-missing action parameters for actions | ||
| # the driver takes on the agent's behalf | ||
| self._parameter_completer = OWRandomParameterCompletionADMComponent() |
Contributor
There was a problem hiding this comment.
Random is an OK placeholder for now; but even for a baseline we want the LLM making those decisions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.