Skip user-authored lines in attachment size injection#323
Conversation
injectAttachmentSizes now only annotates marker lines preceded by an empty line, matching the deterministic "\n📎 filename\n" output from HTMLToMarkdown. User-authored standalone 📎 lines following prose content are left untouched.
There was a problem hiding this comment.
Pull request overview
This PR tightens injectAttachmentSizes so it only annotates attachment marker lines when they match the deterministic marker placement produced by richtext.HTMLToMarkdown, reducing accidental rewrites of user-authored text that happens to look like an attachment marker.
Changes:
- Require attachment marker lines (
📎 filename) to be preceded by an empty line (or be at the start) before injecting file sizes. - Update the duplicate-filename test to reflect blank-line-separated markers.
- Add a regression test ensuring user-authored standalone
📎 filenamelines following prose are not rewritten.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/commands/chat.go | Adds a “preceded by empty line (or start)” guard to prevent unintended size injection into user-authored content. |
| internal/commands/chat_test.go | Updates/extends unit tests to cover the new marker matching rule and the user-authored line regression. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88d080f395
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The empty-preceding-line check blocked valid inline attachments where HTMLToMarkdown produces "content\n📎 filename\n" (marker follows non-empty text). The filename-match against the attachment array is sufficient protection against false positives.
Summary
injectAttachmentSizesnow requires marker lines to be preceded by an empty line before annotating with file size\n📎 filename\noutput fromHTMLToMarkdown, preventing false matches on user-authored contentAddresses post-merge review feedback on #322.
Test plan
📎 report.pdfafter prose is left alone, real marker after blank line gets annotatedSummary by cubic
Update
injectAttachmentSizesto annotate only markers that match provided attachments, including inline markers after content, while leaving user-authored "📎 ..." lines untouched.HTMLToMarkdown("content\n📎 filename\n").Written for commit 3cebe10. Summary will update on new commits.