Per-media-type metadata extraction at injectable seam - #28
Conversation
Split the monolithic module into a package with per-type extractors behind a registry, add video and ebook test coverage, and verify organize-plan smoke paths for all four media types. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
More reviews will be available in 24 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe monolithic ChangesMetadata extraction package refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PROJECT.md (1)
114-118:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate PROJECT.md dependency docs to match the refactor (tinytag + pypdf).
PROJECT.mdstill references Mutagen and PyPDF2, but the codebase andrequirements.txtuse tinytag (TinyTag) for audio (and video fallback) and pypdf (PdfReader) for ebook/PDF metadata—Mutagen/PyPDF2 don’t appear in the implementation.📝 Suggested doc fix
- **Tkinter**: GUI framework -- **Mutagen**: Audio metadata extraction +- **tinytag**: Audio (and video fallback) metadata extraction - **Pillow (PIL)**: Image processing and metadata extraction -- **PyPDF2**: PDF metadata extraction +- **pypdf**: PDF metadata extraction - **MediaInfo** (optional): Enhanced video metadata extraction🤖 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 `@PROJECT.md` around lines 114 - 118, PROJECT.md still lists Mutagen and PyPDF2 but the code uses TinyTag (TinyTag) for audio/video fallback and pypdf (PdfReader) for PDFs; update the dependency list to remove Mutagen and PyPDF2 and replace them with tinytag and pypdf, updating descriptions to mention "TinyTag (tinytag) — audio and video metadata extraction" and "pypdf (PdfReader) — PDF/ebook metadata extraction" and ensure the names match the identifiers used in the codebase (TinyTag, PdfReader) and requirements.txt.
🤖 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 `@metadata_extract/image.py`:
- Around line 22-35: Replace uses of the private _getexif() with the public
img.getexif(): call img.getexif() once, store its return in a local variable
(e.g., exif = img.getexif()), check that exif is truthy, then iterate
exif.items() and map tag numbers via the existing exif_tags dict into the
metadata dict (preserving the current keys camera_make, camera_model, date_time,
date_taken, copyright); remove the duplicate _getexif() calls and ensure you
reference img.getexif(), exif_tags, and metadata exactly as currently named.
In `@metadata_extract/video.py`:
- Around line 26-64: Initialize sensible defaults for all metadata keys used by
templates (e.g., set metadata["album"], metadata["artist"],
metadata["director"], metadata["year"], metadata["genre"], metadata["duration"],
metadata["width"], metadata["height"], metadata["frame_rate"],
metadata["codec"], metadata["bit_depth"] before the MEDIAINFO_AVAILABLE block)
so custom templates won't break when tags are absent, and when populating year
from MediaInfo use a robust parser instead of slicing: in the MediaInfo.parse
loop (see MediaInfo.parse, track.recorded_date) extract the first 4‑digit year
via regex (e.g., search for r"\b(19|20)\d{2}\b") or use a date parser to set
metadata["year"] only when a valid year is found; leave the default year if none
found.
- Around line 44-45: Replace the naive slice recorded_date[:4] with a
regex-based year extraction: add "import re" to metadata_extract/video.py, then
where you set metadata["year"] (inside the hasattr(track, "recorded_date") and
track.recorded_date check) run a regex like re.search(r'\b(\d{4})\b',
track.recorded_date) and if it matches assign metadata["year"] = match.group(1);
otherwise skip setting the year. This updates the code around the recorded_date
handling to reliably pull a four-digit year from strings like "UTC YYYY-MM-DD
..." without slicing.
---
Outside diff comments:
In `@PROJECT.md`:
- Around line 114-118: PROJECT.md still lists Mutagen and PyPDF2 but the code
uses TinyTag (TinyTag) for audio/video fallback and pypdf (PdfReader) for PDFs;
update the dependency list to remove Mutagen and PyPDF2 and replace them with
tinytag and pypdf, updating descriptions to mention "TinyTag (tinytag) — audio
and video metadata extraction" and "pypdf (PdfReader) — PDF/ebook metadata
extraction" and ensure the names match the identifiers used in the codebase
(TinyTag, PdfReader) and requirements.txt.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: d698dfbc-371d-4a8d-bea9-613f2a4a619a
📒 Files selected for processing (13)
PROJECT.mdmetadata_extract.pymetadata_extract/__init__.pymetadata_extract/audio.pymetadata_extract/common.pymetadata_extract/detect.pymetadata_extract/ebook.pymetadata_extract/image.pymetadata_extract/registry.pymetadata_extract/video.pytests/conftest.pytests/test_build_file_plan_paths.pytests/test_metadata_extract.py
💤 Files with no reviewable changes (1)
- metadata_extract.py
Use Pillow getexif(), parse video recorded_date year via regex, and align PROJECT.md dependency names with requirements.txt. Co-authored-by: Cursor <cursoragent@cursor.com>
Review feedback addressed (c36c71e)Fixed:
Challenged:
Skipped:
|
1 participant
Summary
metadata_extract.pywith ametadata_extract/package: per-type modules (audio, video, image, ebook), shared common helpers, andMEDIA_TYPE_EXTRACTORSregistry used byextract_metadata.organize_planinjection seam (MetadataExtractor/_default_metadata_extractor); public imports unchanged.Closes #21
Test plan
python -m pytest -q(80 passed)Made with Cursor
Summary by CodeRabbit
Refactor
Tests