fix(email): expose header-derived media pixel dimensions - #1376
fix(email): expose header-derived media pixel dimensions#1376seonghobae wants to merge 17 commits into
Conversation
|
Important Approval pendingCodeRabbit has no unresolved comments, but it could not review the latest commit because the review limit was reached. Follow the review guidance in this comment to continue. 📝 WalkthroughWalkthroughAdds a deterministic email-media resolver. It parses MIME and HTML content, resolves local CID and bounded data images, records remote references without fetching, validates and deduplicates artifacts, preserves provenance, and enforces resource limits. ChangesEmail inline-media resolution
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The change exposes parsed media dimensions, but malformed image headers can produce fabricated sizes and hyphenated attributes can be mistaken for media references, potentially causing incorrect tracking classification and unnecessary processing. Merge should wait until these bounded parsing and validation issues are fixed. Sequence Diagram(s)sequenceDiagram
participant Caller
participant resolve_email_media
participant MIME_parser
participant HTML_reference_resolver
participant Artifact_store
Caller->>resolve_email_media: provide raw MIME bytes
resolve_email_media->>MIME_parser: traverse MIME parts
MIME_parser->>HTML_reference_resolver: provide HTML image references
HTML_reference_resolver->>Artifact_store: validate or deduplicate local image payload
Artifact_store-->>HTML_reference_resolver: return artifact metadata
HTML_reference_resolver-->>resolve_email_media: return occurrences and statuses
resolve_email_media-->>Caller: return EmailMediaResolution
🚥 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 |
|
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 @opencode-agent review Review only for exact current head |
|
@coderabbitai review Please review the latest changes on exact current head |
|
|
|
Merge loop: exact remaining blocker is last-pusher / independent non-author APPROVE on head |
…evelop Bring cursor/email-media-pixel-dimensions-b8ec onto live protected develop so CWL Product Manager can merge #1376 under ordinary protection. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
@coderabbitai review Please review the unchanged exact current head |
|
|
|
@coderabbitai review Please review the unchanged exact current head |
|
|
|
@coderabbitai review Please review the unchanged exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Checked-out commit =='
git rev-parse HEAD
git status --short
echo
echo '== Pull request metadata =='
gh pr view 1376 --repo ContextualWisdomLab/naruon \
--json number,title,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,changedFiles \
--jq '{number,title,baseRefName,baseRefOid,headRefName,headRefOid,mergeable,changedFiles}'
echo
echo '== Commit relationship =='
git merge-base --is-ancestor dd8d15191338b841f9e6f3a06507c6a5643b95d0 HEAD && echo 'base is an ancestor of HEAD'
git diff --stat dd8d15191338b841f9e6f3a06507c6a5643b95d0...HEAD
echo
echo '== Changed paths =='
git diff --name-status dd8d15191338b841f9e6f3a06507c6a5643b95d0...HEAD
echo
echo '== Diff =='
git diff --find-renames --find-copies --unified=80 dd8d15191338b841f9e6f3a06507c6a5643b95d0...HEADLength of output: 57374 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Resolver structure =='
ast-grep outline backend/services/email_media_resolution.py --items all --view expanded
echo
echo '== Public symbol references =='
rg -n -C 3 --glob '*.py' \
'\b(EmailMediaArtifact|resolve_email_media)\b' \
backend
echo
echo '== Dimension implementation with line numbers =='
nl -ba backend/services/email_media_resolution.py | sed -n '440,610p'
echo
echo '== Dimension tests with line numbers =='
nl -ba backend/tests/test_email_media_artifact_dimensions.py | sed -n '1,130p'
echo
echo '== Python test and lint configuration candidates =='
fd -HI '^(pyproject\.toml|pytest\.ini|setup\.cfg|tox\.ini|\.ruff\.toml)$' . \
-E .git \
-x sh -c 'echo "--- $1"; sed -n "1,240p" "$1"' sh {}Length of output: 28853 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Resolver lines 477-678 =='
awk 'NR >= 477 && NR <= 678 { printf "%6d %s\n", NR, $0 }' \
backend/services/email_media_resolution.py
echo
echo '== Dimension test lines =='
awk 'NR >= 1 && NR <= 100 { printf "%6d %s\n", NR, $0 }' \
backend/tests/test_email_media_artifact_dimensions.py
echo
echo '== PNG and IHDR coverage =='
rg -n -C 2 --glob '*.py' \
'(IHDR|_image_dimensions\s*\(|pixel_width|pixel_height)' \
backend
echo
echo '== Complete repository references to public resolver symbols =='
rg -n -C 2 \
'\b(EmailMediaArtifact|resolve_email_media)\b' \
. -g '!**/.git/**'
echo
echo '== Python configuration =='
find . -path './.git' -prune -o \
\( -name pyproject.toml -o -name pytest.ini -o -name setup.cfg -o -name tox.ini -o -name .ruff.toml \) \
-type f -print \
-exec sh -c 'echo "--- $1"; sed -n "1,240p" "$1"' sh {} \;Length of output: 41409
P1 — Validate the PNG IHDR chunk before exposing dimensions Line 653 reads width and height from fixed offsets when the payload has a PNG signature and at least 24 bytes. It does not verify the first chunk length or the A signature-bearing payload with another first chunk can put arbitrary values at bytes 16–23. The resolver then exposes false Check the first chunk is the 13-byte I did not execute repository tests because the sandbox prohibits executing repository code.
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head18524f3febdf92f0c69ce0b24d856a41fda8cee7. -
Head SHA:
18524f3febdf92f0c69ce0b24d856a41fda8cee7 -
Workflow run: 32123704498
-
Workflow attempt: 1
Coverage evidence
Coverage evidence job did not run or did not publish coverage evidence.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend (4 files)"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend (4 files)"]
R1 --> V1["backend tests"]
Evidence --> S2["Docs: email-inline-media-resolution.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: email-inline-media-resolution.md"]
R2 --> V2["docs review"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage evidence job did not run or did not publish coverage evidence. Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Backend (4 files)"]
S1 --> I1["API and service runtime"]
I1 --> R1["Review risk: Backend (4 files)"]
R1 --> V1["backend tests"]
Evidence --> S2["Docs: email-inline-media-resolution.md"]
S2 --> I2["operator or user guidance"]
I2 --> R2["Review risk: Docs: email-inline-media-resolution.md"]
R2 --> V2["docs review"]
|
|
PR governance metadata gate is not ready for
|
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@coderabbitai review Please review unchanged exact head |
|
|
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
|
@opencode-agent review Exact current head remains |
2 participants
Scope
This PR is the surviving check-fixed successor for the bounded inline-email media resolver originally developed in #1351. It preserves deterministic local CID/data-image resolution, no remote fetch, occurrence provenance, resource bounds, MIME/signature fail-closed behavior, digest-vs-occurrence safety isolation, and header-derived
pixel_width/pixel_heightfor validated PNG IHDR and GIF logical-screen headers. JPEG/WebP and rejected payloads keep dimensions unknown.#1351 is already closed without merge; do not close this PR as cleanup because this branch still carries the valid media-resolution and pixel-dimension delta.
Current exact authority — 2026-09-07
develop@042b0c70531b229af3acbd0421a2f23098d848b315e6b745bc7a466ccc24a352bcdd53b363e280fccursor/email-media-pixel-dimensions-b8ecdevelop: ahead-only / behind 0backend/services/email_media_resolution.py, three focused backend test files, anddocs/doctoring/email-inline-media-resolution.mdReady is review admission only. It does not waive CodeQL, transfer predecessor reviews/checks, or authorize protected merge.
Non-force stale-base repair retained
The branch had been 65 commits behind protected
develop. The intervening protected delta had no overlap with this PR's five product/test/doctoring paths, so it was adopted rather than treated as a race. Ordinary two-parent commit15e6b745bc7a466ccc24a352bcdd53b363e280fcpreserved the reviewed branch blobs while adopting current protected ancestry withforce=false. No open descendant PR directly bases on this branch.Review finding state
The concrete source findings previously raised on this lineage remain repaired in the exact current blobs:
srcmatching rejectsdata-srcthrough(?<![\w-])src...;flags=rather than inline expression flags;IHDRfirst chunk and positive dimensions;Fresh thread inventory is zero unresolved. Historical
CHANGES_REQUESTEDsubmissions belong to predecessor heads and are not a current-head independent approval.Exact-head hosted evidence
On unchanged head
15e6b745bc7a466ccc24a352bcdd53b363e280fcthe repository-owned runs are terminal:33987968123— success33987968129— success33987968140— success33987968213— success33987968360— success33987968279— failureThe CodeQL failure is not a Naruon source-analysis finding. All three compatibility jobs (JavaScript/TypeScript, Actions, Python) successfully completed
Request current-head CodeQL scan dispatchand then failed atRelease runner or enforce current-head CodeQL verdict. This is the shared fail-closed central verdict path tracked by canonical.githubowner issue #1927; dispatch authorization/identity is not a Naruon product contract and must not be bypassed or duplicated here.Do not rerun the failed consumer CodeQL job as a workaround, manufacture a dummy commit, widen an authorization allowlist from Naruon, or transfer predecessor evidence. The branch should receive a real terminal verdict through the canonical owner path after that control plane is repaired.
The central required OpenCode workflow on this exact head previously published successful transport/context jobs, including
opencode-review, but it did not create a qualifying current-head approval and therefore is not treated as approval evidence. After moving the unchanged head from Draft to Ready, an explicit@coderabbitai reviewrequest was submitted. CodeRabbit reports zero unresolved comments but approval pending because the latest commit could not be reviewed under the current review-capacity limit. No no-op commit, dismissal, or capacity workaround is permitted.Review admission and merge boundary
This bounded current-head slice is Ready so independent robot review can be admitted under the live merge-gate policy. Current-head robot review is still required; Ready and successful review-transport jobs are not verdict evidence.
Merge Gate: FAIL. Merge only when this unchanged exact head has every then-live repository/organization required check terminal-success, zero valid unresolved current-head findings/threads, and qualifying current-head robot-review evidence. Current CodeQL failure and missing current-head review verdict are blockers.
No self-approval, bypass/admin merge, force-push, destructive rebase, dummy/no-op requeue commit, review fabrication/dismissal, gate weakening, or predecessor-evidence transfer.