From 93f2a101fa3d0d0a35996abd1315c0d583f0deb7 Mon Sep 17 00:00:00 2001 From: Carlos Garcia Date: Tue, 19 May 2026 21:05:38 -0400 Subject: [PATCH] =?UTF-8?q?refactor:=20remove=20scripted=20file=20intercep?= =?UTF-8?q?t=20=E2=80=94=20LLM=20owns=20all=20responses?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously ab_ai_mail.py intercepted file uploads before reaching the LLM and responded with a hardcoded clarification template. The LLM had no involvement in the file upload response. Changes: - ab_ai_mail.py: remove _post_file_clarification, _find_pending_attachments, _describe_zip, and the two-step pending-attachment lookup. All messages (text, files, or both) are dispatched to the agent service immediately. Files with no text pass an empty message — the LLM decides what to do. - upload.py: default message changed from hardcoded receipt instruction to '' so the LLM determines intent from file content. - master_agent._synthesize: always runs through the LLM for both single and multi-agent cases — no raw templates reach the user. - master_system.txt: add FILE UPLOADS routing rule so the LLM knows to route receipts to expenses_agent without asking for clarification. New flow: upload → parse → LLM classifies → agent acts → LLM synthesizes natural response → user sees it. Zero scripted intercepts. Co-Authored-By: Claude Sonnet 4.6 --- addons/activeblue_ai/models/ab_ai_mail.py | 164 +++------------------- agent_service/agents/master_agent.py | 24 ++-- agent_service/prompts/master_system.txt | 4 + agent_service/routers/upload.py | 2 +- 4 files changed, 36 insertions(+), 158 deletions(-) diff --git a/addons/activeblue_ai/models/ab_ai_mail.py b/addons/activeblue_ai/models/ab_ai_mail.py index 5530ac4..f822e50 100644 --- a/addons/activeblue_ai/models/ab_ai_mail.py +++ b/addons/activeblue_ai/models/ab_ai_mail.py @@ -1,10 +1,8 @@ from __future__ import annotations import base64 -import io import logging import re import threading -import zipfile import requests as _requests from markupsafe import Markup, escape @@ -14,56 +12,16 @@ _logger = logging.getLogger(__name__) _HTML_TAG = re.compile(r'<[^>]+>') -# How many recent messages to scan when looking for a pending file upload -_LOOKBACK_MESSAGES = 10 - -# File type labels shown in the clarification message -_EXT_LABELS = { - 'jpg': 'image', 'jpeg': 'image', 'png': 'image', 'gif': 'image', - 'bmp': 'image', 'tiff': 'image', 'tif': 'image', 'webp': 'image', - 'pdf': 'PDF', 'html': 'HTML', 'htm': 'HTML', - 'txt': 'text', 'csv': 'spreadsheet', 'xlsx': 'spreadsheet', - 'zip': 'ZIP archive', -} - def _strip_html(html: str) -> str: return _HTML_TAG.sub(' ', html or '').strip() -def _ext(filename: str) -> str: - return filename.rsplit('.', 1)[-1].lower() if '.' in filename else '' - - def _text_to_html(text: str) -> Markup: - """Convert plain text to HTML -- escapes content, turns newlines into
.""" + """Convert plain text to HTML — escapes content, turns newlines into
.""" return Markup('
').join(Markup(escape(line)) for line in text.split('\n')) -def _describe_zip(datas_b64: str, zip_name: str) -> str: - """Return a plain-text summary of a ZIP archive's contents.""" - try: - raw = base64.b64decode(datas_b64) - with zipfile.ZipFile(io.BytesIO(raw)) as zf: - members = [m for m in zf.namelist() if not m.endswith('/')] - if not members: - return f'{zip_name} (empty archive)' - counts: dict[str, int] = {} - for m in members: - label = _EXT_LABELS.get(_ext(m), 'file') - counts[label] = counts.get(label, 0) + 1 - type_summary = ', '.join(f'{n} {t}(s)' for t, n in counts.items()) - lines = [f'{zip_name} -- {len(members)} item(s): {type_summary}'] - for m in members[:8]: - lines.append(f' - {m}') - if len(members) > 8: - lines.append(f' ... and {len(members) - 8} more') - return '\n'.join(lines) - except Exception as exc: - _logger.warning('_describe_zip failed for %s: %s', zip_name, exc) - return f'{zip_name} (could not inspect contents)' - - def _post_bot_reply(db: str, channel_id: int, bot_partner_id: int, reply_text: str): """Open a fresh DB cursor and post the bot reply to the channel.""" try: @@ -85,8 +43,8 @@ def _agent_thread(db: str, uid: int, text: str, att_data: list, bot_url: str, bot_secret: str): """ Background thread: calls the agent service and posts the reply. - Runs entirely outside the Odoo HTTP request so message_post returns - immediately and the user sees their message without waiting for the LLM. + All messages — text, files, or both — are routed here so the LLM + handles every response. Nothing is intercepted or templated in Odoo. """ try: headers = {} @@ -94,12 +52,13 @@ def _agent_thread(db: str, uid: int, text: str, att_data: list, headers['X-ActiveBlue-Signature'] = bot_secret if att_data: + # Send files (with or without text) to the upload endpoint. + # Passing an empty message lets the agent service decide intent + # from the file contents rather than a scripted default. files = [('files', (name, data, mime)) for name, data, mime in att_data] - if not files: - files = [('files', ('empty', b'', 'text/plain'))] form = { 'user_id': str(uid), - 'message': text or 'Create an employee expense report from these receipts.', + 'message': text, # may be empty — agent service handles that 'session_id': '', } resp = _requests.post(bot_url + '/upload', data=form, files=files, @@ -118,7 +77,6 @@ def _agent_thread(db: str, uid: int, text: str, att_data: list, except _requests.exceptions.Timeout: reply_text = 'The request timed out. Please try again.' except _requests.exceptions.HTTPError as exc: - # Try to surface the server-side error detail so the user knows what failed detail = '' try: detail = exc.response.json().get('detail') or '' @@ -161,38 +119,26 @@ class DiscussChannel(models.Model): if bot_partner not in member_partners: return result - # Don't react to the bot's own messages + # Never react to the bot's own messages if author_id == bot_partner.id: return result text = _strip_html(body) - - _logger.info( - 'AB AI mail hook: body=%r kwargs_keys=%s ' - 'attachment_ids_kwarg=%r result.attachment_ids=%s', - (body or '')[:80], - list(kwargs.keys()), - kwargs.get('attachment_ids'), - result.attachment_ids.ids, - ) - attachments = result.attachment_ids + # Nothing to process if not text and not attachments: return result - # -- Case 1: file(s) with no instruction -------------------------------- - # Clarification is quick (no LLM) -- post inline, no thread needed. - if attachments and not text: - self._post_file_clarification(attachments, bot_partner) - return result - - # -- Case 2: text (possibly with pending files from earlier upload) ----- - pending = self.env['ir.attachment'].browse() - if text and not attachments: - pending = self._find_pending_attachments(bot_partner) - - effective_attachments = attachments or pending + # Read attachment bytes NOW, inside the current transaction + att_data: list[tuple[str, bytes, str]] = [] + for att in attachments: + try: + data = base64.b64decode(att.datas) if att.datas else b'' + att_data.append((att.name or 'attachment', data, + att.mimetype or 'application/octet-stream')) + except Exception as exc: + _logger.warning('Could not read attachment %s: %s', att.id, exc) human_partner = member_partners.filtered(lambda p: p != bot_partner)[:1] user = self.env['res.users'].search([('partner_id', '=', human_partner.id)], limit=1) @@ -202,26 +148,13 @@ class DiscussChannel(models.Model): if not bot: return result - # Read everything we need from the DB NOW (current transaction) before - # the background thread starts. The thread must not touch ORM objects - # from this transaction. db = self.env.cr.dbname bot_url = bot._get_service_url() bot_secret = bot.webhook_secret or '' channel_id = self.id bot_partner_id = bot_partner.id - att_data: list[tuple[str, bytes, str]] = [] - for att in effective_attachments: - try: - data = base64.b64decode(att.datas) if att.datas else b'' - att_data.append((att.name or 'attachment', data, - att.mimetype or 'application/octet-stream')) - except Exception as exc: - _logger.warning('Could not read attachment %s: %s', att.id, exc) - - # Launch the agent call in a daemon thread so this message_post returns - # immediately -- the user sees their message without waiting for the LLM. + # Launch the agent call in a daemon thread — message_post returns immediately threading.Thread( target=_agent_thread, args=(db, uid, text, att_data, bot_partner_id, channel_id, @@ -230,62 +163,3 @@ class DiscussChannel(models.Model): ).start() return result - - def _post_file_clarification(self, attachments, bot_partner): - """Describe the uploaded file(s) and ask the user what to do with them.""" - file_lines = [] - for att in attachments: - name = att.name or 'file' - ext = _ext(name) - if ext == 'zip' and att.datas: - file_lines.append(_describe_zip(att.datas, name)) - else: - label = _EXT_LABELS.get(ext, 'file') - file_lines.append(f'{name} ({label})') - - file_summary = '\n'.join(file_lines) - question = ( - f'I received the following file(s):\n' - f'{file_summary}\n' - f'\n' - f'What would you like me to do with them? Some options:\n' - f' - Create an expense report from these receipts\n' - f' - Import products from this data\n' - f' - Something else -- just tell me what you need' - ) - self.sudo().message_post( - body=_text_to_html(question), - author_id=bot_partner.id, - message_type='comment', - subtype_xmlid='mail.mt_comment', - ) - - def _find_pending_attachments(self, bot_partner): - """ - Scan the last _LOOKBACK_MESSAGES messages in this channel for the most - recent human-sent message that has attachments. Only returns them if - the immediately following bot message looks like a clarification question - (i.e. the bot hasn't already acted on those files). - """ - messages = self.message_ids.sorted('date', reverse=True)[:_LOOKBACK_MESSAGES] - _bot_question_phrases = ( - 'what would you like me to do', - 'suspected duplicate', - 'skip duplicates', - 'keep all', - 'please review', - 'reply "confirm"', - ) - prev_was_bot_question = False - for msg in messages: - is_bot = msg.author_id == bot_partner - if is_bot: - body_lower = _strip_html(msg.body or '').lower() - if any(p in body_lower for p in _bot_question_phrases): - prev_was_bot_question = True - continue - # Human message - if msg.attachment_ids and prev_was_bot_question: - return msg.attachment_ids - prev_was_bot_question = False - return self.env['ir.attachment'].browse() diff --git a/agent_service/agents/master_agent.py b/agent_service/agents/master_agent.py index b7d4bd8..d6dfa73 100644 --- a/agent_service/agents/master_agent.py +++ b/agent_service/agents/master_agent.py @@ -285,22 +285,22 @@ class MasterAgent: async def _synthesize(self, reports, context: MasterContext) -> str: if not reports: return 'No agent responses received.' - if len(reports) == 1: - return reports[0].summary or '(no summary)' - summaries = chr(10).join(f'{r.agent}: {r.summary}' for r in reports) - msg = ('Synthesize these agent reports into one coherent response. ' - 'Business language only. No internal IDs. ' - 'Separate: actions completed, items pending approval, recommendations.' - + chr(10) + summaries) + summaries = chr(10).join(f'[{r.agent}]\n{r.summary}' for r in reports) + instruction = ( + 'You are ActiveBlue AI. Convert the following agent report(s) into a clear, ' + 'natural response for the user. Preserve every factual detail: amounts, dates, ' + 'record IDs, item names, and any action taken. Use plain text only — no JSON, ' + 'no markdown code fences. Write as if speaking directly to the user.' + ) try: resp = await self._llm.submit( - [{'role': 'system', 'content': 'You are a business intelligence assistant.'}, - {'role': 'user', 'content': msg}], + [{'role': 'system', 'content': instruction}, + {'role': 'user', 'content': summaries}], caller='master_synthesis') - return resp.content or summaries + return (resp.content or summaries).strip() except Exception as exc: - logger.warning('_synthesize LLM call failed, falling back to raw summaries: %s', exc) - return summaries + logger.warning('_synthesize LLM call failed, using raw summary: %s', exc) + return reports[0].summary if len(reports) == 1 else summaries async def _update_memory(self, user_id, message, response, reports, directive_id): try: diff --git a/agent_service/prompts/master_system.txt b/agent_service/prompts/master_system.txt index 80b9697..69108e9 100644 --- a/agent_service/prompts/master_system.txt +++ b/agent_service/prompts/master_system.txt @@ -30,6 +30,10 @@ CRITICAL ROUTING RULE: Most messages are general conversation and require NO spe Only route to a specialist agent when the user explicitly asks for Odoo data or actions. When in doubt, use "agents": []. +FILE UPLOADS: When a user uploads files (message is empty or "User uploaded files"), +do NOT ask for clarification. Route directly to the appropriate agent based on file content. +Receipt images or PDFs → expenses_agent. Unknown files → agents: [] and answer directly. + Examples of correct routing: User: "hello" or "hi" or "what can you do?" or "what does that mean?" or "ok" or "thanks" diff --git a/agent_service/routers/upload.py b/agent_service/routers/upload.py index 5c22090..2dd7016 100644 --- a/agent_service/routers/upload.py +++ b/agent_service/routers/upload.py @@ -18,7 +18,7 @@ router = APIRouter(prefix='/upload', tags=['upload']) async def upload( request: Request, user_id: str = Form(...), - message: str = Form(default='Create an employee expense report from these receipts.'), + message: str = Form(default=''), session_id: Optional[str] = Form(default=None), files: List[UploadFile] = File(default=[]), ):