fix: #457 llm will add rfp compliant id when replying#560
Open
hindmakarem-qa wants to merge 3 commits intotaylorwilsdon:mainfrom
Open
fix: #457 llm will add rfp compliant id when replying#560hindmakarem-qa wants to merge 3 commits intotaylorwilsdon:mainfrom
hindmakarem-qa wants to merge 3 commits intotaylorwilsdon:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughAdds "In-Reply-To" and "References" to Gmail metadata extraction and output alongside Subject, From, To, Cc, Message-ID, and Date; updates in_reply_to and references field descriptions to reference RFC-style Message-ID examples; minor formatting/spacing adjustments. Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
gmail/gmail_tools.py (1)
1162-1172:⚠️ Potential issue | 🟠 MajorValidate reply-header inputs instead of relying on doc text.
These updates improve the hints, but both tools still pass
in_reply_toandreferencesstraight into_prepare_gmail_message()without checking that they are RFC-style Message-IDs. A Gmail internal id or malformed reference chain will still be emitted verbatim, so the threading bug can persist in both send and draft flows.🛠️ Suggested direction
+def _normalize_rfc_message_id(value: str, field_name: str) -> str: + candidate = value.strip() + if not (candidate.startswith("<") and candidate.endswith(">")): + raise UserInputError( + f"{field_name} must be an RFC Message-ID like '<message123@gmail.com>'." + ) + return candidate + +def _normalize_references(value: str) -> str: + refs = [_normalize_rfc_message_id(ref, "references") for ref in value.split()] + if not refs: + raise UserInputError( + "references must contain one or more RFC Message-IDs separated by spaces." + ) + return " ".join(refs)Then normalize before assigning
message["In-Reply-To"]andmessage["References"].Also applies to: 1206-1208, 1369-1379, 1410-1412
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gmail/gmail_tools.py` around lines 1162 - 1172, The in_reply_to and references parameters are passed into _prepare_gmail_message without validation; add validation and normalization before passing them or before assigning to message["In-Reply-To"] / message["References"] in _prepare_gmail_message: ensure each token is a valid RFC‑2822 Message-ID (match/extract tokens like <...@...> via regex), wrap single IDs with angle brackets if missing, and for references normalize a space-separated chain of Message-IDs (extract valid IDs and join with single spaces); apply this logic where in_reply_to and references are accepted (functions/params named in_reply_to, references) and where _prepare_gmail_message sets message["In-Reply-To"] and message["References"] so malformed gmail internal ids or raw strings are not emitted verbatim.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@gmail/gmail_tools.py`:
- Around line 40-44: The GMAIL_METADATA_HEADERS list was updated but the
functions still omit the new reply headers from their outputs; update
get_gmail_message_content() and get_gmail_messages_content_batch() to include
the "In-Reply-To" and "References" metadata when composing the returned message
string (i.e., when iterating/formatting headers or metadata for the returned
content, include those two header keys from GMAIL_METADATA_HEADERS), ensuring
callers receive those headers so reply chains can be reconstructed for
RFC-compliant follow-ups.
---
Outside diff comments:
In `@gmail/gmail_tools.py`:
- Around line 1162-1172: The in_reply_to and references parameters are passed
into _prepare_gmail_message without validation; add validation and normalization
before passing them or before assigning to message["In-Reply-To"] /
message["References"] in _prepare_gmail_message: ensure each token is a valid
RFC‑2822 Message-ID (match/extract tokens like <...@...> via regex), wrap single
IDs with angle brackets if missing, and for references normalize a
space-separated chain of Message-IDs (extract valid IDs and join with single
spaces); apply this logic where in_reply_to and references are accepted
(functions/params named in_reply_to, references) and where
_prepare_gmail_message sets message["In-Reply-To"] and message["References"] so
malformed gmail internal ids or raw strings are not emitted verbatim.
…ace_mcp into fix/email-threading-headers
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gmail/gmail_tools.py (1)
724-730: Add regression tests for header rendering across all modified output branches.Given changes in three formatting paths, add focused tests for present/absent
In-Reply-ToandReferencesin: single message, batch metadata, and batch full outputs.Also applies to: 896-909, 934-947
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@gmail/gmail_tools.py` around lines 724 - 730, Add regression tests that exercise header rendering for the in_reply_to and references logic around the in_reply_to, references, and content_lines handling in gmail/gmail_tools.py: create unit tests for the three output branches that were modified (single message output, batch metadata output, and batch full output) verifying both presence and absence of "In-Reply-To" and "References" headers. For each branch, add cases for (1) neither header present, (2) only In-Reply-To present, (3) only References present, and (4) both present, asserting the generated content includes the correct header lines when present and omits them when absent. Ensure tests call the exact functions/pathways that build content_lines (the single message renderer and the two batch renderers) and compare the rendered string or list to expected outputs to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@gmail/gmail_tools.py`:
- Around line 724-730: Add regression tests that exercise header rendering for
the in_reply_to and references logic around the in_reply_to, references, and
content_lines handling in gmail/gmail_tools.py: create unit tests for the three
output branches that were modified (single message output, batch metadata
output, and batch full output) verifying both presence and absence of
"In-Reply-To" and "References" headers. For each branch, add cases for (1)
neither header present, (2) only In-Reply-To present, (3) only References
present, and (4) both present, asserting the generated content includes the
correct header lines when present and omits them when absent. Ensure tests call
the exact functions/pathways that build content_lines (the single message
renderer and the two batch renderers) and compare the rendered string or list to
expected outputs to prevent regressions.
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
2 participants
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.
Description
Fix email reply threading by ensuring reply headers use RFC-compliant values.
Updates Gmail metadata header fetching to include
In-Reply-ToandReferences.Type of Change
Testing
Checklist
Additional Notes
Message-ID,In-Reply-To,References). When these are missing, replies may not thread correctly across clients.GMAIL_METADATA_HEADERSto includeIn-Reply-ToandReferences.in_reply_toshould be an RFC Message-ID (e.g.,<...@gmail.com>), not Gmail’s internal message id.Summary by CodeRabbit