feat: enhance AI parsing and prompts with company details integration#83
Conversation
…rty and update company context role hints in prompts
Confidence Score: 4/5PR is safe to merge with low risk; all findings are P2 style/behaviour observations. The implementation is clean and well-structured. All three comments are P2: (1) the
|
| Filename | Overview |
|---|---|
| transaction_parser/transaction_parser/overrides/communication.py | Moves matched_account lookup to the top of on_update so matched_account.company is available for the party-email processing path — a notable behavioral change that now silently drops party emails when no matching incoming account row exists. |
| transaction_parser/transaction_parser/ai_integration/parser.py | Adds company parameter to parse and _build_messages; new _get_company_info static method fetches company name and address via Frappe APIs and injects them into the AI prompt. HTML entity handling in address text is missing. |
| transaction_parser/transaction_parser/ai_integration/prompts.py | Adds optional company_info context to the user prompt with a role hint differentiating seller vs buyer; logic is correct for current document types but the seller-type check is hardcoded to "Sales Order" only. |
| transaction_parser/transaction_parser/controllers/transaction.py | Passes self.company to AIParser.parse; no issues found. |
| transaction_parser/transaction_parser/init.py | Adds company parameter to the whitelisted parse function and forwards it safely with cstr(company) if company else None; clean change. |
| transaction_parser/public/js/transaction_parser_dialog.js | Adds a required Company Link field defaulting to the user's default company; straightforward UI change. |
| transaction_parser/parser_benchmark/runner.py | Passes self.dataset.company to parser.parse in the benchmark runner; matches the controller change and is correct. |
| transaction_parser/transaction_parser/doctype/transaction_parser_party_email/transaction_parser_party_email.json | Adds a trailing newline to the JSON file; no functional change. |
Reviews (1): Last reviewed commit: "fix: ensure company address is only appe..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| matched_account = next( | ||
| ( | ||
| row | ||
| for row in settings.incoming_email_accounts | ||
| if row.to_email in doc.recipients | ||
| ), | ||
| None, | ||
| ) | ||
|
|
||
| if not matched_account: | ||
| return |
There was a problem hiding this comment.
matched_account now gates party-email processing
Moving the matched_account lookup and early-return to the top of on_update means that emails from known party senders are no longer processed unless the destination address also matches an entry in incoming_email_accounts. Previously the parse_party_emails path ran independently of matched_account.
This is a meaningful breaking change for any installation that has parse_party_emails configured with party-email entries but without a corresponding incoming_email_accounts row covering the same destination address. Those emails will now silently be dropped.
If the intent is intentional (i.e. you always need a matched account to obtain matched_account.company), consider adding a code comment explaining the dependency to prevent future confusion:
# matched_account must be resolved before any processing path so we can
# derive the correct company for both the general and party-email flows.
matched_account = next(
(
row
for row in settings.incoming_email_accounts
if row.to_email in doc.recipients
),
None,
)
if not matched_account:
return| if document_type == "Sales Order": | ||
| role_hint = "Use this to correctly identify the company as the seller/vendor and the other party as the customer/buyer." | ||
| else: | ||
| role_hint = "Use this to correctly identify the company as the buyer/recipient and the other party as the vendor/supplier." |
There was a problem hiding this comment.
"Sales Order" — may be incomplete for future types
The role hint currently branches on exactly "Sales Order" and defaults to "buyer/recipient" for everything else. If a new selling-side document type is added in the future (e.g. a "Quotation"), it would silently get the wrong role hint.
Consider aligning this with the INPUT_DOCUMENTS dict so the selling-side types are enumerated in one place:
SELLER_DOCUMENT_TYPES = {"Sales Order"}
...
if document_type in SELLER_DOCUMENT_TYPES:
role_hint = "Use this to correctly identify the company as the seller/vendor and the other party as the customer/buyer."
else:
role_hint = "Use this to correctly identify the company as the buyer/recipient and the other party as the vendor/supplier."|
|
||
| @staticmethod | ||
| def _get_company_info(company: str) -> str: | ||
| """Build a company context string with name and address if available.""" | ||
| from frappe.contacts.doctype.address.address import get_company_address | ||
| from frappe.utils import strip_html | ||
|
|
||
| info = f"Company: {company}" | ||
|
|
||
| address = get_company_address(company) | ||
| if address and address.company_address_display: | ||
| address_text = strip_html(address.company_address_display).strip() | ||
|
|
||
| if address_text: | ||
| info += f"\nLocated at: {address_text}" | ||
|
|
||
| return info | ||
|
|
||
| def send_message(self, messages: tuple, file_doc_name: str | None = None) -> dict: | ||
| """Send messages to AI API and handle the response.""" | ||
| log = self._create_log_entry(file_doc_name) |
There was a problem hiding this comment.
company_address_display may contain unresolved HTML entities
strip_html removes HTML tags, but it does not decode HTML entities such as &, , ', etc. If the address stored in Frappe contains these entities (which is common for addresses stored as rich text), they will appear verbatim in the AI prompt, potentially confusing the model.
Consider adding an HTML entity decode step:
import html
address_text = html.unescape(strip_html(address.company_address_display).strip())1 participant
No description provided.