UN-3477 Client Policy for LLMs by d1donlydfink · Pull Request #420 · cognizant-ai-lab/neuro-san

d1donlydfink · 2025-10-02T21:22:47Z

Previously we had a LangChainLlmClient object that held references for use in closing.
Upon further looking into how to do this kind of thing for providers other than OpenAI,
we find that most other ChatModels do not actually let you pass in your own async client.
In at least one case (Bedrock), the client itself can't even be an async client.

So with this PR, we are moving to a ClientPolicy model which serves two different styles in the same interface.

The first style is the OpenAI style. The create_client() method returns the correct object
to pass into the ChatModel's constructor as a client. Inside create_client(), the ClientPolicy
has access to the full llm config so that client objects can also be configured, and it has the freedom
to store objects that are relevant to shutting down web clients for llm access correctly.

The second style is one where the ChatModel does not allow an external client instance coming in
its constructor or being set anywhere on the object. In this case we pass in the LLM ChatModel
itself as the basis reference.

Both styles need to implement a delete_resources() method, which correctly cleans up.
Most often for style 2, this involves a reach-in to private member variables to do the right thing
for clean up (at least until we find a better way of doing it.)

Even with all this change, I still think there might be yet another iteration on this notion of a ClientPolicy interface.
See ClientPolicy to comment on near-future direction.

…f a data object

…he common get_value_or_env() method

d1donlydfink · 2025-10-06T15:51:37Z

neuro_san/internals/run_context/langchain/llms/anthropic_client_policy.py

+
+    Anthropic chat models do not allow for passing in an externally managed
+    async web client.
+    """


Anthtopic implementation of the ClientPolicy interface.
Still need to ve sure I test the non-OpenAI ones.

d1donlydfink · 2025-10-06T15:53:29Z

neuro_san/internals/run_context/langchain/llms/anthropic_client_policy.py

+        # Note we don't want to do this in the constructor, as AnthropicChat lazily
+        # creates these as needed via a cached_property that needs to be done in its own time
+        # via Anthropic infrastructure.  By the time we get here, it's already been created.
+        anthropic_async_client: Any = self.llm._async_client     # pylint:disable=protected-access


Do the reach-ins in order to shut down the client as correctly as we can inside the delete_resources() method of the ClientPolicy interface.

This is the general pattern to follow when the LLM class itself does not allow you to pass in a client class at all. - all we can do.

d1donlydfink · 2025-10-06T15:53:54Z

neuro_san/internals/run_context/langchain/llms/azure_client_policy.py

+        :return: The web client that accesses the LLM.
+                By default this is None, as many BaseLanguageModels
+                do not allow a web client to be passed in as an arg.
+        """


Azure implementation follows the make-a-client-first pattern.
Worth noting that this implementation relies on the OpenAI implementation to do the proper delete_resources() policy

d1donlydfink · 2025-10-06T15:55:49Z

neuro_san/internals/run_context/langchain/llms/bedrock_client_policy.py

+    Bedrock does not allow for passing in async web clients.
+    As a matter of fact, all of its clients are synchronous,
+    which is not the best for an async service.
+    """


Bedrock is another one of those reach-in during delete_resources, as it does not allow a client to be passed in. During this exercise I also learned that bedrock doesn't even allow an async client, so not-so-great performance is nearly guaranteed within the async server model.

Might be worth poking langchain-aws maintainers to allow for an asynchronous client.

d1donlydfink · 2025-10-06T16:04:04Z

neuro_san/internals/run_context/langchain/llms/client_policy.py

+       implementations should pass the already created llm into their implementation's
+       constructor. Later delete_resources() implementations will need to do a reach-in
+       to the llm instance to clean up any references related to the web client.
+    """


This is the basic interface for LLM ClientPolicy.

This is the 2nd iteration on this kind of interface, moving from the data-only class w/ external policy of the LangChainLlmClient we had previously to a class that is more policy based. (Verb-centered as opposed to Noun-centered).

Having slept on this, I think there is perhaps another 3rd iteration to be had which adds a notion of 2 more
methods looking something like this:

def get_llm_class_name(self) -> str: """ :return: The string class name for the LLM represented so as to register this policy class with the neuro-san LlmFactory system """ # Would return something like "openai" or "anthropic" or whatever the llm class name for the llm_info.hocon files would be raise NotImplementedError def create_chat_model(self, config: Dict[str, Any], client: Any = None) -> BaseLanguageModel

This last create_chat_model() would allow for LLM creation and deletion policy to exist within the same class, and with standardized external structure could allow for standardized calling and registration as long as the class was listed in the user's llm_info.hocon file. The idea would be we could also register our own classes this way in default_llm_info.hocon as well for an example. This would mean that LlmFactory would no longer be the interface to override (though we could keep it for backwards compatibility) but this new LlmPolicy interface would be.

Thoughts about this direction anyone?

d1donlydfink · 2025-10-06T16:44:52Z

neuro_san/internals/run_context/langchain/llms/standard_langchain_llm_factory.py

                stream_usage=True,
+                thinking=config.get("thinking"),
+                mcp_servers=config.get("mcp_servers"),
+                context_management=config.get("context_management"),


Add some params I discovered while kicking around with Anthropic.

d1donlydfink · 2025-10-06T16:46:47Z

neuro_san/internals/run_context/langchain/llms/standard_langchain_llm_factory.py

                                                             install_if_missing="langchain-anthropic")
+
+            # ChatAnthropic currently only supports _async_client() as a cached_property,
+            # not as a constructor arg.


Comment of note.

d1donlydfink · 2025-10-06T16:47:15Z

neuro_san/internals/run_context/langchain/llms/standard_langchain_llm_factory.py

+            # Create the client_policy after the fact, with reach-in
+            client_policy = AnthropicClientPolicy(llm)
+
        elif chat_class == "ollama":


Not doing ollama or gemini just yet.

d1donlydfink · 2025-10-06T16:48:14Z

neuro_san/internals/run_context/langchain/llms/standard_langchain_llm_factory.py

+        # Return the LlmResources with the client_policy that was created.
        # That might be None, and that's OK.
-        return LangChainLlmResources(llm, llm_client=llm_client)
+        return LangChainLlmResources(llm, client_policy=client_policy)


Stash the ClientPolicy created. If this is still None, then that's till fine.

d1donlydfink · 2025-10-06T16:48:35Z

tests/neuro_san/internals/run_context/langchain/llms/test_llm_factory.py


-    def create_llm_resources_with_client(self, config: Dict[str, Any],
-                                         llm_client: LangChainLlmClient = None) -> LangChainLlmResources:
+    def create_llm_resources(self, config: Dict[str, Any]) -> LangChainLlmResources:


Change the TestLlmFactory to be exemplary.

vince-leaf · 2025-10-06T23:08:35Z

@d1donlydfink Warning: I reran Neuro-san's smoke test on this branch. It reported a failure on the Azure test case. I will try to find help by running the same test manually on my local machine.
https://github.com/cognizant-ai-lab/neuro-san/actions/runs/18296364800/job/52096051373

andreidenissov-cog

Looks good to me.

d1donlydfink · 2025-10-07T00:56:33Z

Thanks @vince-leaf . Great catch! I believe I have a fix in my next branch.

Use LlmClient for Azure

0b331d8

d1donlydfink marked this pull request as draft October 2, 2025 21:22

d1donlydfink added 2 commits October 2, 2025 16:54

Attempt to create LlmClient after ChatAnthropic has been created

941d91c

Add comments about bedrock

2327b1c

d1donlydfink changed the base branch from main to ASD-UN-3477-model-resources01 October 3, 2025 00:12

d1donlydfink added 3 commits October 2, 2025 17:14

Fix pylint

4373108

Create a BedrockLangChainLlmClient to hold the boto3 clients to close

8bd59d4

Switch to a model where LlmClient is really a policy object instead o…

50cd2c0

…f a data object

d1donlydfink changed the title ~~UN-3477 anthropic azure clients~~ UN-3477 Client Policy for LLMs Oct 3, 2025

d1donlydfink added 4 commits October 3, 2025 14:16

Rename LangChainLlmClient -> ClientPolicy

420d081

Refactor TestLlmFactory

81957f9

Clarify comments

20146f1

Factor out an EnvironmentConfiguration interface for easy access to t…

7560186

…he common get_value_or_env() method

d1donlydfink commented Oct 6, 2025

View reviewed changes

d1donlydfink marked this pull request as ready for review October 6, 2025 16:49

d1donlydfink requested review from Noravee, andreidenissov-cog, deepsaia and ofrancon October 6, 2025 16:49

andreidenissov-cog approved these changes Oct 7, 2025

View reviewed changes

d1donlydfink merged commit 0db0b54 into ASD-UN-3477-model-resources01 Oct 7, 2025
1 of 3 checks passed

d1donlydfink deleted the UN-3477-anthropic-azure-clients branch October 7, 2025 00:56

Comments

Conversation

d1donlydfink commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vince-leaf commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andreidenissov-cog left a comment

Choose a reason for hiding this comment

Uh oh!

d1donlydfink commented Oct 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

d1donlydfink commented Oct 2, 2025 •

edited

Loading

vince-leaf commented Oct 6, 2025 •

edited

Loading