feat(mail): open recognized HWPX text from email attachments - #1406
feat(mail): open recognized HWPX text from email attachments#1406seonghobae wants to merge 20 commits into
Conversation
List the current email's attachments with the same opaque asset_key as the Data preview contract so a buyer can read ordered HWPX paragraphs from mail without treating missing text as empty content. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. Important Approval pendingCodeRabbit has no unresolved comments, but it skipped the latest review. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
📝 WalkthroughWalkthroughEmail detail and thread responses now expose opaque attachment references. The dashboard renders selected attachments through the repository preview endpoint and handles HWPX recognition states without exposing internal identifiers. ChangesMail attachment preview
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to This PR adds recognized HWPX attachment previews to email while preserving existing preview behavior and unavailable/pending states. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant User
participant EmailDetail
participant MailAttachmentPreview
participant RepositoryAssetPreviewPanel
User->>EmailDetail: select email
EmailDetail->>MailAttachmentPreview: provide attachment references
MailAttachmentPreview->>RepositoryAssetPreviewPanel: request preview by asset_key
RepositoryAssetPreviewPanel-->>MailAttachmentPreview: return preview state and paragraphs
MailAttachmentPreview-->>User: display selected attachment
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
PR governance metadata gate is not ready for
|
Wire the inherited 다시 확인 control to re-fetch the same opaque mail attachment preview so the stacked parent refresh stays reachable. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Bring Draft #1407 onto independently resolved live #1406 b83a0da. Previous parent 304d5af is stale. Keep #1404/#1406 pending 다시 확인 refresh and mail reachability together with the fail-closed Inkspan edit handoff. No rebase, squash, or force-push. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@coderabbitai review The rolling-hour window from #1404 |
|
✅ Action performedReview finished.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ff9fc7a1d
ℹ️ 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".
| file_name = _safe_email_display_text(getattr(attachment, "filename", None)) | ||
| refs.append( | ||
| EmailAttachmentRef( | ||
| asset_key=_opaque_asset_key(email, attachment), |
There was a problem hiding this comment.
Include attachment identity in each opaque asset key
When one email contains two attachments with the same filename—which the attachment schema permits—both references receive the same key because _opaque_asset_key hashes only the owner, message ID, and filename. MailAttachmentPreview subsequently uses that key for React identity, selection, and caching, while the preview endpoint returns the first matching row, so clicking either attachment can display the wrong document. Include an immutable attachment-specific value in the key and lookup, then add a duplicate-filename case to backend/tests/test_email_attachment_preview.py.
AGENTS.md reference: AGENTS.md:L449-L454
Useful? React with 👍 / 👎.
Current authority
This mail HWPX preview slice remains valid product work, but it is Draft and not merge-ready.
develop@042b0c70531b229af3acbd0421a2f23098d848b3cursor/hwpx-recognized-text-preview-b246@a1a3d461b2863b5fae0537ff8c84ac28d8c75665edd3134a197fe819df366b469a2a4a1c5b27b4f7Parent adoption
The earlier child lineage had newer #1404 tree content without the current parent ancestry.
40594758c1f1791a1d679fb0ee0da0d1c8b2cf06non-force merged current #1404 into this branch;cc7d2cefd41aff24bac1a1a94d3a0205984816c1then exact-adopted the three current parent blobs that the initial merge tree had left stale. The branch therefore inherits the current #1404 preview authority without force-push, destructive rebase, or parent-source rollback.Review repairs completed on this child
Attachment-body overfetch
RED contract
63fd00a9382c47f8e530a8c47b43f2988f80f9f7captures the real PostgreSQL attachment SELECTs for both mail detail and thread and rejects anyemail_attachments.contentprojection. Causal fix6d3bcc5afe0e6e0ccea9c03e03f2e7496f81947areplaces unrestricted relationship loading withselectinload(...).load_only(Attachment.filename, Attachment.parser_key, raiseload=True). The P1 review thread is resolved on source/contract evidence; no PostgreSQL GREEN is claimed because this stacked head has no repository-owned run.PostgreSQL smoke cleanup
7615754fb9a49b3378db6d73f5093e9f53660e19explicitly removes the seeded attachment row and then the email row before engine disposal, preserving the shared test database. The corresponding P2 thread is resolved; hosted PostgreSQL execution remains pending stacked-base workflow availability.Out-of-order preview responses
Test-first
bc24bf682ff6998610db39c90dc644cfeb8baad5creates two same-asset requests, resolves the newer response first as recognized, then delivers an older pending response and requires the recognized result to remain. Causal fix298bbba1fd8f3b4dcb9cdc7ac50bb6f2dd3e472egives each asset a monotonically increasing request sequence and ignores stale success or failure completions.b0ba575...andc5fde35...only restore final newlines. The stale-response P2 thread is resolved on source/contract evidence, not executed frontend GREEN.Signed-session evidence path
501d019e1ae0efde3b5df333047fda4fdd88b971adds backend coverage that uses the real HMAC bearer verifier rather thandev_auth_dependency_overrides, plus a negative case proving public identity headers alone remain 401.edd3134a197fe819df366b469a2a4a1c5b27b4f7adds Playwright coverage that requiresnaruon_sessionon both mail-detail and attachment-preview browser requests and rejects browserAuthorizationplus public identity headers. The authentication-evidence P2 thread is resolved on the presence of the correct contract/evidence path; no exact-head backend/browser GREEN is claimed.Remaining live finding
One #1406 review thread remains unresolved: duplicate filename attachment identity. Two MIME parts with the same filename still receive the same opaque key because attachment identity is not part of the digest. This is the same canonical preview-key defect already open on parent #1404. It must be repaired once in #1404 and then inherited here; this child must not invent competing key semantics.
#1404 itself remains Draft with additional parent obligations, including #1345 prerequisite adoption, attachment workspace isolation, and remaining preview UI/runtime findings. #1406 must not bypass those obligations.
Evidence and merge boundary
This is a non-default stacked branch. Stacked-base workflow activation is canonically owned by Draft #1562 and is not protected-integrated; exact head
edd3134...currently has 0 repository-owned pull-request workflow runs. No predecessor check or review transfers. Keep Draft until the parent/prerequisite stack is healthy, the duplicate-key owner finding is repaired and inherited, exact-head backend/frontend/PostgreSQL/browser/signed-session evidence is terminal-success, and a qualifying independent post-last-push approval applies. No force push, destructive rebase, self-approval, dummy requeue, bypass, or gate weakening.