Fix MP4/M4A metadata persistence diagnostics classification#38
Merged
ChrisAdamsdevelopment merged 1 commit intoMay 20, 2026
Merged
Conversation
Reviewer's GuideRefines MP4/M4A metadata diagnostics to rely on core field presence and explicit mismatch evidence rather than hash drift, and hardens deep metadata snapshots for privacy, de-duplication, and structured ExifTool output, while keeping scope limited to diagnostic logic in server/processor.js. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
mismatchEvidencegating inclassifyMetadataPersistenceStageis never actually used (always passed{}fromprocessMediaFile), so themetadata_present_in_snapshots_but_report_or_download_mismatchpath is currently unreachable; consider plumbing through real evidence or at least adding a clear TODO where the object is constructed. - The
hashesparameter toclassifyMetadataPersistenceStageis now unused; if inter-stage SHA-256 is no longer part of the decision, consider removing this parameter (and its call-site argument) to avoid confusion about whether hashes still influence classification. - The new
hasExpectedCoreMetadatacheck requires all four fields (Title,Artist,Producer,Copyright) to be present before returningmetadata_present_and_verified, which is stricter thanhasDescriptiveMetadata; ensure this all-or-nothing requirement matches the intended business logic, or relax it if some fields are optional in practice.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `mismatchEvidence` gating in `classifyMetadataPersistenceStage` is never actually used (always passed `{}` from `processMediaFile`), so the `metadata_present_in_snapshots_but_report_or_download_mismatch` path is currently unreachable; consider plumbing through real evidence or at least adding a clear TODO where the object is constructed.
- The `hashes` parameter to `classifyMetadataPersistenceStage` is now unused; if inter-stage SHA-256 is no longer part of the decision, consider removing this parameter (and its call-site argument) to avoid confusion about whether hashes still influence classification.
- The new `hasExpectedCoreMetadata` check requires all four fields (`Title`, `Artist`, `Producer`, `Copyright`) to be present before returning `metadata_present_and_verified`, which is stricter than `hasDescriptiveMetadata`; ensure this all-or-nothing requirement matches the intended business logic, or relax it if some fields are optional in practice.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
metadata_present_in_snapshots_but_report_or_download_mismatchwhen only normal inter-stage SHA-256 drift occurred despite final snapshots containing the expected metadata.deepSnapshotsometimes leftselectedMetadataempty whenexiftool.readreturned structured output instead of line text, reducing diagnostic usefulness.Description
hasExpectedCoreMetadataand updatedclassifyMetadataPersistenceStageto returnmetadata_present_and_verifiedwhen the final snapshot contains core fields (Title,Artist,Producer,Copyright).mismatchEvidencegate (e.g., client hash mismatch or external verification contradiction) required to emitmetadata_present_in_snapshots_but_report_or_download_mismatch.deepSnapshotextraction to de-duplicate entries, omit lyrics, redactDescription/Commentas length+SHA-256, and add an object-key fallback extraction when the structuredreadoutput would otherwise produce noselectedMetadata.server/processor.jsand did not change metadata-writing, XMP cleanup, timestamp writes, auth, CORS, frontend, MP3 Quick Cleanse, or package files.Testing
node --check server/processor.jssucceeded with no syntax errors.require('./server/processor')was attempted but could not run in this environment due to the missingexiftool-vendoreddependency, so behavioral runtime assertions requiring ExifTool were not executed.Codex Task
Summary by Sourcery
Refine media metadata persistence diagnostics to reduce false mismatches and improve privacy-aware snapshot reporting.
New Features: