[ADD] ai_connection

[etobella]

No description provided.

[angelmoya]

I've been independently developing an ai_connection module with session management, call history, skill integration, and agent support. I discovered this PR while working on it and I'm now adapting my modules to extend from this base rather than duplicating effort.

As I started building ai_connection_session (persistent call records with token tracking) and ai_connection_agent (agent loop on top of connections) on top of this module, I identified several points where the current interface makes it difficult for downstream modules to hook into the execution flow without overriding the entire method chain.

The review above focuses on making the core extensible — returning structured data from _run_ai() so callers can persist it, enriching the client.py contract with standard fields like usage and model, and a few guardrails (max_iterations, error handling) that become critical once you add persistence.

None of these changes break backward compatibility — they extend the existing contracts so that the growing ecosystem of AI modules (session, agent, skill, debug) can compose cleanly on top of this foundation.

[angelmoya]

Hi @etobella , thanks for the force-push cleanup — the _process_tool_call / _process_tool_call_result extraction and the max_iterations guard are great additions.

About the tool error handling: I understand you have doubts, let me explain the rationale so we can reach the right approach together.

The problem in the current code:

When tool._execute_tool() raises (e.g. a UserError, a ValueError, a missing record), the exception propagates up through _run_ai() → _run(). The entire conversation state — messages accumulated across iterations — is lost. From the LLM's perspective, it called a tool and got no response at all, which is not a recoverable protocol state.

Why catching and returning the error matters:

LLMs are designed to handle tool results. If we return {"role": "tool", "content": "Error: record not found"} instead of crashing:

1. The LLM sees the tool call failed
2. It can retry with different arguments
   3, It can explain the failure to the user
   4, It can skip the tool and try another approach
   Without this, the user just gets a generic Odoo traceback — and the agent/connection is in an undefined state.

Where I think it fits:

_process_tool_call is the natural place — wrapping the tool._execute_tool() call in a try/except, and if an exception occurs, returning the error message as a tool result message. This keeps _run_ai clean and provider-agnostic:

python
def _process_tool_call(self, tool, tool_call, record):
    try:
        tool_response = tool._execute_tool(**tool_call["arguments"], record=record)
    except Exception as e:
        _logger.warning("Tool %s failed: %s", tool_call["name"], e)
        tool_response = f"Error: {str(e)}"
    return getattr(self, f"_process_tool_call_result_{self.kind}",
                   self._process_tool_call_result)(tool, tool_response, tool_call)

What specific part are you unsure about? Is it the broad Exception scope, the format of the error message, or a concern about hiding real bugs? Happy to refine the approach.

[angelmoya]

Another thing I noticed is that there's no streaming support. The current
handle_message() interface returns a complete dict, but for real-time
chat scenarios (e.g., discuss.channel integration) the client should be
able to yield partial chunks via something like:

def handle_message_stream(self, messages=None, **kwargs):
    """Yield chunks: {type: "delta"|"tool_call"|"done", ...}"""
    ...

The OpenAI/Ollama APIs already support this (SSE with stream=True),
and the _run_ai() tool-call loop would need a streaming variant that
buffers silently on tool-call iterations and flushes on the final one.

Not a blocker for merge — the current interface is fine for the MVP and
this can be added without breaking changes. I do plan to implement it
here (not in a separate module) in a follow-up. Have you thought about
streaming in the roadmap for this module?

[etobella]

I've thought about this before your comments and there I took a decission about it.

The core issue is that Odoo's bus.bus notification system is transaction-bound: notifications are only dispatched after cr.commit(), which makes it incompatible with mid-stream chunk delivery. Committing per chunk is technically possible but dangerous. It might be possible... yes... the benefits outgrows the cons... not for me.

Another thing, I saw that you proposed a lot of migrations, but I was refactoring most of the modules with the Plan, I will check if they fit what I have in mind, otherwise, I might superseed your changes 😉 You were too fast 😆

[angelmoya]

Hi!

Perfect, if you already have plans and refactoring in place for those modules, go ahead and push them. The main reason I opened these was precisely because I realized we were working on the exact same things in parallel, and I wanted to sync up.

Regarding the work I’ve been doing on top of this: besides the skills system we discussed, I am currently focusing on making the messages multimodal, for multimodal support (images, audio, documents), we'll need a provider-agnostic way to pass files through the connection. Something like:

def _run(self, prompt, tools=None, record=None, system_prompt=None,
         messages=None, max_iterations=None, files=None):
    ...
    if prompt:
        msg = {"role": "user", "content": prompt}
        if files:
            msg["files"] = files
        messages.append(msg)

Where each file is a generic dict:

{
    "type": "image"|"audio"|"document"|"text",
    "mimetype": "image/jpeg",
    "data": "<base64>",
    "filename": "photo.jpg"
}

Each provider's handle_message() would translate files to its native format (Ollama → images array, OpenAI → content parts with image_url, etc.). The _run_ai() method already passes messages through untouched, so no changes needed there.

What do you think? Would you handle it differently for passing files generically to the LLM?

I tested this, and works fine.

[etobella]

@angelmoya type has no sense, as this depends on each AI system, the transformation should be done by mimetype on each client.

[angelmoya]

Hi @etobella ,

Exactly, that's my point. When another module calls ai_connection, it doesn't know (and shouldn't care) which specific provider or submodule is going to handle the request underneath.

That is why we need to define a standardized payload structure at the ai_connection level. This ensures any client module can pass messages and files in a unified way. Then, each provider-specific submodule will be responsible for extracting those files, reading the mimetype, and transforming them into whatever native format that specific AI client requires.

However, regarding the "type" field, while it's true that we can use the "mimetype" to identify the file extension, having a high-level "type" (like "image", "audio", "document") standardizes the abstraction layer. It avoids forcing every single provider submodule to parse the mimetype string just to understand the general category of the file, which most LLMs require at their root level. For instance:

Our own tools/modules flow: If we develop a tool that allows the LLM to load files from a document's attachments, the tool will read the attachment, call ai_connection passing this standard file structure, and then ai_connection will route it to the active submodule (e.g., OpenAI, Ollama) so it can perform the correct native transformation.

OpenAI / Anthropic (Claude): Both use a block structure where you must explicitly declare the high-level type at the root:

JSON
// OpenAI
{"type": "image_url", "image_url": {"url": "data:image/jpeg;base64,..."}}
// Anthropic
{"type": "image", "source": {"type": "base64", "media_type": "image/jpeg", "data": "..."}}

Gemini: Separates the structural part from the data block:

JSON
{"inline_data": {"mime_type": "image/jpeg", "data": "..."}}

Ollama: Expects a clean, separated array of images at the message level:

JSON
{"role": "user", "content": "...", "images": ["<base64>"]}

Keeping a simple, high-level "type" abstraction alongside the "mimetype" makes it incredibly easy for any future submodule to quickly route the file to the correct internal handler, avoiding redundant boilerplate code to parse strings like image/png, image/webp, audio/mp3, etc., in every single driver.

Let me know if you see the value in keeping it as a high-level helper for the submodules!

I hope I’m explaining myself well, but if you have any questions or want to clear things up, I’m totally available to hop on a quick call and sync up whenever you want.

[etobella]

according to your information, it is easily handled. I would not add the code here.

[angelmoya]

Hi @etobella

My bad! I hadn't realized that they were already being passed as attachments in the code, so you're totally right, the high-level type is not a blocker at all. Let's leave it as it is.

However, unless I've misunderstood something, I think the question from my previous message about what happens when a tool call fails remains unanswered. If we can clear that up, it's the very last thing left!

Sorry for being a bit insistent, and thanks a lot for your patience and effort on this. We are almost there! 🚀

[petrus-v]

I've thought about this before your comments and there I took a decission about it.

The core issue is that Odoo's bus.bus notification system is transaction-bound: notifications are only dispatched after cr.commit(), which makes it incompatible with mid-stream chunk delivery. Committing per chunk is technically possible but dangerous. It might be possible... yes... the benefits outgrows the cons... not for me.

Another thing, I saw that you proposed a lot of migrations, but I was refactoring most of the modules with the Plan, I will check if they fit what I have in mind, otherwise, I might superseed your changes 😉 You were too fast 😆

Just sharing some through about streaming, because even the odoo server do not stream the result to the end user there are path where streaming the IA result is interesting such as

• avoid memory usage
• avoid connection timeout or killed by Firewall or other network tools.

I haven't check yet the implementation if possible or not just reacting to the conversation

[angelmoya]

Hi @petrus-v

Exactly! Those are very solid points. Avoiding timeouts on long reasoning tasks and reducing memory footprint are crucial architectural reasons to consider streaming, even if Odoo's standard layout isn't fully built for it.

To give some context on what I'm doing: I am currently testing a basic implementation of stream=True. The immediate goal is just to validate the module's core flow this way. It works fine for straightforward questions that don't involve complex agent loops, tool calls, or heavy reasoning.

However, as Enric pointed out, Odoo's standard bus.bus notifications won't work for real-time text chunking because they are bound to the database transaction (cr.commit()).

If we want to support full real-time streaming to the UI in the future, the right approach would be:

1. Turn the connection execution into a generator using Python's yield to stream chunks from the LLM provider.

2. Create a custom Odoo controller (/route) that wraps this generator.

3. Return a streaming response directly from the controller (e.g., using werkzeug.wrappers.Response or standard Server-Sent Events).

4. Consume this stream directly from the frontend JS (bypassing bus.bus entirely).

This way, the browser can feed the UI in real time without forcing premature database commits.

For now, keeping it simple as an MVP is perfectly fine, but it’s great to keep this architecture in mind for a follow-up!

[angelmoya]

Hi @etobella , thanks for the great foundation you laid out here!

I have created an alternative PR #86 based on your work. After giving
some deep thought to how we should properly handle streaming and long-running AI
interactions, I realized the core execution loop could benefit from a slightly different
structural approach, so I kept and extended your foundation.

While the new PR doesn't implement a strict "real-time" streaming transport out of the
box, it drastically improves the usability and stability of the module. By moving the core
to an iterator-based architecture, we've introduced a stepwise (iteration-by-iteration)
mode.

This approach allows us to naturally isolate and commit each AI iteration (or tool call)
in its own separate database transaction. This means we avoid massive long-running
database locks and we no longer need to rely on standard trigger workarounds or forced
manual commits. The new architecture is completely fail-safe for tool errors, and provides
a solid extensible foundation (like an _on_stream_batch hook) if we want to implement
advanced streaming via temporary files or websockets in the future.

I would love for you to take a look at #86 and hear your thoughts. Let
me know if you think this is a good direction for the module! 🚀

[etobella]

@angelmoya _on_stream_batch will not work if you don't use commit and that should be avoided...

[angelmoya]

Hi @etobella and @ValentinVinagre

The PR I previously uploaded didn't have the final code; I pushed the wrong version by mistake.

The main idea is that each iteration inside the while loop now runs in a separate process using _trigger, which calls the cron. Once a step finishes, it triggers the cron again to execute the next iteration of the loop.

So, instead of a while loop, we now have a recursive or recurrent call. Since it executes via trigger, Odoo automatically commits after each iteration before continuing. It is not exactly streaming, but it will help us get feedback for every single thinking iteration.

[etobella]

it will take at least 3 minutes if if has a message, a tool calling and a final mesaage. that doesn't look like a good idea. crons are not executed automatically. also, it could be longer if there is other cron activities... doean't seem like the best plan

[angelmoya]

@etobella I understand your concern, but that would only be true if we were waiting for the standard cron schedule. Using _trigger() changes the behavior completely.

Calling _trigger() acts exactly like a native, lightweight queue system (very similar to how queue_job works, but built into the core). It completely bypasses the cron's scheduled interval. When we call it, Odoo immediately inserts a record into ir.cron.trigger and instantly wakes up an available background worker to execute the job right away.

The delay between iterations is not minutes; it is just milliseconds (the time it takes for the worker to pick up the trigger).

[angelmoya]

The PR #87 is now fixed and updated with the correct code.

Here is the method that does the "magic". This method is executed by a cron, and it essentially re-triggers itself recurrently as long as the execution state is running (meaning it needs more iterations to finish).

    def _cron_process_single_step(self):
        """
        Executed by the cron. Finds one running execution and processes its next step.
        If we passed an ai_execution_id in context, we prioritize it.
        """
        domain = [("state", "=", "running")]
        ai_execution_id = self.env.context.get("ai_execution_id")
        if ai_execution_id:
            domain.append(("id", "=", ai_execution_id))

        execution = self.search(domain, limit=1)
        if not execution:
            return

        execution._execute_step()

        if execution.state == "running":
            cron = self.env.ref(
                "ai_connection.cron_ai_step_processor", raise_if_not_found=False
            )
            if cron:
                cron.with_context(ai_execution_id=execution.id)._trigger()

You can see the exact implementation here: https://github.com/angelmoya/ai/blob/ef2c82a19bd87d465bde33b1055d56c88e10271c/ai_connection/models/ai_connection_execution.py#L263

[etobella]

@angelmoya your proposal has a major problem. IF there is 2 AI processes launched together, they will be executed one after the other, not in parallel. No benefits of workers. Scales poorly....

For me, it's a no. I understand your needs and we can keep hooks if you need it, however, but you are making complex guesses just for your specific need. I understand and accept what you want to achieve but I don't think that this is needed in the base module.

Also, I would suggest to use queue_job and queue_job to solve the problem explained and queue_job will not be a dependency for this module. As you can see in other modules, we are removing this dependency if possible and leave it in a secondary related module (like edi-framework)

Also, why did you directly superseded my PR when I was answering your questions and comments? It is not the usual OCA way, at least a comment before the supersed would have been nice... The right way would have been doing a commit a PR on my branch directly.

[angelmoya]

Hi @etobella,

First of all, thank you for your patience and for thoroughly reviewing almost every point in this PR. I really appreciate the detailed feedback. However, I noticed that the point regarding the try/except block during tool execution was left unanswered, so I'd like to bring it back, along with a clarification on the cron architecture.

I would like to share my perspective on these design choices, as I believe they are crucial for the robustness of the core ai module:

1. Concurrency (Cron vs. Trigger): The execution is concurrent, but it is strictly bound by Odoo's native max_cron_threads configuration. Using _trigger() simply schedules the cron safely without overloading system resources. Furthermore, this design is highly extensible, allowing developers in high-load environments to easily override the method and delegate execution to queue_job.

2. Robustness in Tool Execution (try...except): As mentioned before, capturing exceptions during tool execution is indispensable. If an external tool fails (due to a timeout, network glitch, or a bug within the tool itself), a try/except block prevents the entire assistant thread from crashing. It allows us to log the error properly and lets the LLM handle the failure gracefully instead of leaving the process hanging or in a broken state.

3. Usability on Complex Queries: This is not a specific need of mine; it is a basic user experience (UX) expectation. When dealing with complex AI queries involving multiple reasoning loops or chained tool calls, the process can take several minutes. Leaving the user with zero intermediate feedback for that long makes the system feel frozen and unusable.

Finally, I wanted to clarify why I opened this PR instead of pushing directly to your branch. This is a significant structural design change, and I wanted to open the floor so more community members could review it and share their perspectives. In any case, even though I already had my own module fully developed for this, I chose to adapt my logic on top of your work specifically to preserve your commits and respect your authorship.

Thanks again for the constructive debate!

[angelmoya]

In any case, please note that my PR is fully compatible with accepting your original proposal first. We can merge your work to unblock it, and then continue debating and fine-tuning these additions (the try/except block, UX feedback, and cron adjustments) right after.

[etobella]

did you read my comments or did you let claude answer following your ideas without rereading it?

Just to be clear:

• Cron is a bad idea I expressed myself clearly. I exposed the problem and nothing you expressed will fix that. wit your approach, only one process is allowed at the same time. if doesn't scale.
• Not seeing the rudeness of the supersed surprises me.

[angelmoya]

Hi Enric,

I do read your comments, but I admit that sometimes I find it difficult to understand exactly what you mean, and I get the impression that the way Odoo's standard cron _trigger() method actually works is not being fully taken into account.

In your comment, you mentioned: "IF there is 2 AI processes launched together, they will be executed one after the other, not in parallel. No benefits of workers. Scales poorly...". This concern would make perfect sense if we were talking about a standard cron running conventionally, but that is not the case when using _trigger().

Perhaps we are facing a language barrier, or I might be mistaken, or you might simply not be taking into account how it works, but since Odoo 14, this standard method does not function like a traditional synchronous cron. It modifies nextcall and notifies the cron subsystem via PostgreSQL's NOTIFY mechanism. Odoo's CronRunner then handles the execution asynchronously using the available threads in max_cron_threads. It acts as a lightweight, native queue that avoids concurrent database locks. Furthermore, this specific design is what makes it so easy to inherit and delegate the method to queue_job if a high-scale production environment requires it.

If I am misunderstanding your point, or if we are looking at this standard cron mechanism differently, I would love to clarify it from this technical perspective. I am completely open to being proven wrong if that is the case; I just want to make sure we are aligning on the actual technical behavior of the core.

Finally, regarding your comment about the "rudeness of the supersede", here I truly get the impression that you are the one not reading my comments. I believe I have already clearly explained my point of view: I opened this PR because I believe a structural change like this needs to be openly reviewed and debated by more community members. In any case, I took the time to adapt my logic on top of yours precisely to keep all your work and preserve your commits, fully respecting your authorship. There is no rudeness in debating open-source architecture this way.

[flachica]

Hi @etobella, @ValentinVinagre and @angelmoya,

We all agree that ai_connection is the cornerstone of the roadmap (#73), and our common goal is quite clear. So, what do we need to do to minimize effort, align with the OCA methodology, and avoid getting blocked by procedural friction or misunderstandings?

On a technical level, your contributions are highly complementary. I propose combining both approaches:

• Enric's architectural base: Keep the provider-agnostic approach. The multimodal management for passing files is the right foundation.

• Ángel's robustness: Implement the try/except block in tool execution and standardize _run_ai() so the module scales in production without the thread collapsing due to external errors.

Would you be open to a quick call (20 mins) this week? The goal is strictly pragmatic: to agree on how to integrate these improvements via commits to the original branch to respect the OCA workflow and get this PR ready to merge.

Regards.

[angelmoya]

One last technical question to wrap this up, @etobella

In the official SII module (l10n_es_aeat_sii_oca), exactly the same mechanism is used to queue asynchronous submissions:
https://github.com/OCA/l10n-spain/blob/b2117900af30715d8fe119dff93295c103cf9eb6/l10n_es_aeat_sii_oca/models/account_move.py#L592

As we know, creating a record in ir.cron.trigger is exactly what the standard _trigger() method does under the hood (both write to the same model and wake up the CronRunner via Postgres NOTIFY).

Could you please confirm if your argument that "IF there is 2 xxx processes launched together, they will be executed one after the other, not in parallel. No benefits of workers. Scales poorly..."" applies to the SII design as well? Because if this architecture is valid for that workflow, I see no reason why it shouldn't be the right approach to handle AI workflows here.

[etobella]

the trigger is launched and processes one ai job after the other. if five users launch at the same time a process, they will be launched one after the other. Odoo only lets the same job to be launched once and cannot be launched again until it is over.

For SII there is not a problem to wait a few minutes. Here it is a problem

[angelmoya]

You are right about the cron behavior. I admit I had misunderstood how max_cron_threads works, and I wrongly assumed it would parallelize the execution of the same process. As you said, Odoo will handle the triggers for this cron sequentially. Ultimately, the choice for true scaling in production will be to delegate this to queue_job, but I still believe this decoupled approach serves as a clean baseline and proof of concept in standard Odoo to protect web workers from heavy AI queries that take several minutes.

That being said, I think this debate is highly positive for building something better together. The same misunderstanding happened to me on other points where I didn't fully grasp your answer at first, such as the file type issue (where I simply missed the change already applied in the code). My main goal is for the AI vertical to have a solid framework that goes beyond just throwing simple queries, and I would love to build that as a community effort.

Because of this, your comment implying that I do not read your messages and that I am just letting Claude reply for me was completely uncalled for.

Regarding the supersede, I did it precisely because you previously implied that you would implement those changes within my pending PRs. When you mentioned it back then, I didn't see it as an offense, so I didn't expect it to be perceived as one now. My only intention was to propose improvements (perhaps I should have framed it as a depends_on rather than a supersede). At the end of the day, these are just architectural proposals meant to be openly discussed within the community.

[etobella]

@angelmoya It is true that my comment was completely out place. I was angry and I made assertions where I shouldn't. For this reason, I apologize. Unfortunately, I had followed the same path of thought that you did a few weeks ago and the only possible option for streaming was going for queue_job or an external tool. For this reason, that will not be on the base module. As I said, we can keep hooks if necessary, but I don't want unnecessary dependencies on a base module.

On the other hand, I would like to give you some extra details to give you a full perspective of my reaction:

1. Big comments like you made are not helping when you want answers. It implies a lot of work for maintainers and PSCs. At some point, that exhausts us. I would recommend you to keep small comments and focused to make things easier.
2. I thought that I made my opinion clear and I justified clearly my assertions. Maybe I was wrong, but I am expecting that the other part of the conversation treat them carefully and evaluates why I am saying that.
3. Maintainers, PSCs and developers are not infallible. So, it is good to evaluate both sides cleanly. Every time you suggested something I reviewed Odoo's code to verify if that made sense.
4. The usual way to handle a change like you did is to open a PR directly on my branch, not making another PR here suggesting that you are superseding me (it is usually done when someone is not answering comments). Seeing that felt rude. Unfortunately, it was the second time that happened to me in a week and that made me angry too.

[angelmoya]

Thank you for your message @etobella . Communication over text can be difficult sometimes. I will make sure to keep my comments shorter in the future; the long ones were precisely me trying to explain points where I lacked clarity:

1. The attachment change: I simply missed the change in the code after your brief comment about the file type.

2. The try/except block on tools: I mentioned it extensively a few times, and I think it just got lost among too much information.

3. The cron workers: This was my mistake due to a lack of understanding of how they actually behave in this scenario.

I have closed the other PR. I will look into the hooks approach and give it a thought. Let's keep building!

[angelmoya]

This is the exact point where I believe we should catch any potential error so the LLM doesn't freeze if there is an issue with the tool execution.

[etobella]

the problem with try catch is that there could be extra problems related to that...

Imagine we create an invoice and the system makes it unbalanced, leaving a try catch can give problems

probably better to have a savepoint. I will try to see if it is possible, otherwise it is better to leave it like this.

[angelmoya]

Exactly! The savepoint would be perfect for this.

Instead of breaking the execution loop, we can catch the error, roll back the DB state for that tool, and pass the error message back to the LLM.

This way, the agent understands what went wrong with those parameters and can attempt to fix it or try a different approach.