feat: import AVIF workflow metadata from XMP#13757
Conversation
dosubot
Bot
added
the
size:L
|
✅ All contributors have signed the CLA. Thank you! This PR is ready to be merged. |
🎨 Storybook: 🚧 Building...🎭 Playwright: ⏳ Running... |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAVIF metadata parsing now extracts XMP workflow and prompt values alongside EXIF. Test fixtures support multiple metadata items, configurable ChangesAVIF XMP Metadata Support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AVIFFile
participant parseAvifMetadata
participant getIlocItemData
participant EXIFParser
participant parseXmpMetadata
AVIFFile->>parseAvifMetadata: provide AVIF metadata boxes
parseAvifMetadata->>getIlocItemData: extract EXIF and XMP item bytes
getIlocItemData-->>parseAvifMetadata: return assembled item data
parseAvifMetadata->>EXIFParser: parse EXIF item when present
EXIFParser-->>parseAvifMetadata: return EXIF metadata
parseAvifMetadata->>parseXmpMetadata: parse RDF/XML item when present
parseXmpMetadata-->>parseAvifMetadata: return workflow and prompt metadata
parseAvifMetadata-->>AVIFFile: return merged metadata
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/scripts/metadata/avif.test.ts`:
- Around line 472-502: Add malformed-XMP fallback tests alongside the existing
AVIF metadata tests, covering malformed XML and schema-invalid workflow or
prompt JSON while providing valid EXIF metadata. Use getFromAvifFile and assert
the returned workflow and prompt preserve the EXIF values when XMP is missing or
invalid, while retaining the existing valid-XMP precedence coverage.
In `@src/scripts/metadata/avif.ts`:
- Around line 222-253: Update getIlocItemData to validate each extent’s source
range and accumulate totalLength before allocating itemData. Enforce an explicit
metadata-size limit during accumulation, reject invalid or overflowing ranges
and totals, then allocate only after all extents pass validation; preserve the
existing data-copy behavior for valid items.
- Around line 262-267: Validate parsed XMP prompt and workflow values in the
metadata handling block using their runtime schemas before assigning them to
metadata, leaving invalid properties unset so valid EXIF values remain
effective; update src/scripts/metadata/avif.ts lines 262-267. Add malformed-XML
and schema-invalid-JSON cases asserting EXIF fallback behavior in
src/scripts/metadata/avif.test.ts lines 472-502.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 25de46f6-61e8-4b98-aeca-c55a9338caba
📒 Files selected for processing (3)
src/scripts/metadata/avif.test.tssrc/scripts/metadata/avif.tssrc/types/metadataTypes.ts
|
I have read and agree to the Contributor License Agreement |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/scripts/metadata/avif.test.ts`:
- Around line 214-258: Extend buildIlocBox to accept version,
construction-method, and index-size options, and encode the corresponding iloc
v1/v2 fields and extent indexes in the fixture. Add assertions that exercise
parseIlocBox’s method-0 XMP import path and confirm unsupported construction
locations retain the EXIF fallback, covering the AVIF/XMP behavior through real
parsing paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d1f9d7f8-c7d3-49d8-aa16-315330598ea2
📒 Files selected for processing (2)
src/scripts/metadata/avif.test.tssrc/scripts/metadata/avif.ts
Use the canonical ComfyUI XMP namespace emitted by the Save Image (Advanced) AVIF writer in Comfy-Org/ComfyUI#14975. The reader matches by local name, so parsing behavior is unchanged.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/scripts/metadata/avif.test.ts (1)
415-423: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a non-canonical XMP namespace test.
buildXmpPackethardcodes the canonicalcomfyURI, so the suite does not verify the promised namespace-independent parsing. Make the namespace configurable or add a packet using a different URI/prefix, then assert workflow and prompt extraction still succeeds.Also applies to: 582-594
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/scripts/metadata/avif.test.ts` around lines 415 - 423, Extend the AVIF metadata tests around buildXmpPacket to cover a non-canonical XMP namespace URI or prefix. Make the packet builder configurable as needed, generate a packet with the alternate namespace, and assert workflow and prompt extraction still succeed independently of the namespace choice.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/scripts/metadata/avif.test.ts`:
- Around line 415-423: Extend the AVIF metadata tests around buildXmpPacket to
cover a non-canonical XMP namespace URI or prefix. Make the packet builder
configurable as needed, generate a packet with the alternate namespace, and
assert workflow and prompt extraction still succeed independently of the
namespace choice.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0eda2fed-49fd-44f7-9b06-422bbb1066d9
📒 Files selected for processing (1)
src/scripts/metadata/avif.test.ts
|
This stays contained to the AVIF metadata reader and its tests, with no new dependencies (just the built-in browser parsing APIs). Existing EXIF import is untouched; XMP is layered on top of it and falls back to EXIF whenever the XMP is missing or doesn't parse. Malformed files stay bounded too: item ranges are validated against the file size before anything is allocated, so a crafted AVIF can't force an oversized read, and anything the reader doesn't recognize is skipped rather than mis-read. Nothing in the UI or the load flow changes; this is purely the parser behind the existing drag-to-load path. That's why the coverage is unit tests plus a real libavif round-trip rather than a browser test. The matching writer in Comfy-Org/ComfyUI#14975 produces exactly this XMP layout, so the two interoperate end to end. Happy to add whatever would help review. |
9f80582 to
8dee4bc
Compare
1 participant
Summary
Dragging an AVIF that stores ComfyUI workflow or prompt metadata in an XMP MIME item currently accepts the image but does not restore its workflow. This extends the reader introduced by Add the ability to parse workflows from AVIF images to import that metadata while preserving existing EXIF behavior.
Changes
infeitems with typemimeand content typeapplication/rdf+xml, resolve their declared 0/4/8-byteilocfields and extents, and readworkflowandpromptfrom RDF elements or attributes.Review Focus
construction_method=0,data_reference_index=0).idat-stored items using construction method 1 remain out of scope.Testing
pnpm test:unit src/scripts/metadata/avif.test.ts --maxWorkers=2— 25/25 passedpnpm typecheckpnpm lintpnpm format:checkpnpm knip