feat(diff): ExifTool-sourced two-pane before/after metadata diff (chunk B) #177

Merged

forgejo_admin merged 7 commits from feat/exiftool-diff-strategy into master 2026-05-21 11:27:20 +00:00

Conversation 4 Commits 7 Files changed 34 +1840 -127

forgejo_admin commented 2026-05-21 08:58:11 +00:00

Owner

Copy link

Summary

Chunk B of #174. Pivots the diff view (issue #22) from per-strategy delta surfaces to a unified ExifTool dump on both source + stripped bytes, displayed in a diffchecker-style two-pane layout. Spec at #176 (docs/superpowers/specs/2026-05-21-issue-22-diff-pivot-design.md).

Same strip behaviour, much richer evidence — ExifTool's -j -G1 -a -struct returns hundreds of decoded tags per file (MakerNote vendor sub-tags, full XMP schemas, ICC profile details, PDF/Office/MP4 fields) where the existing per-strategy delta walkers only saw what each format's walker was hand-rolled to surface.

Refs #22, #174 (chunks C–F still ahead — does not close).

What landed

File	Role
src/domain/exif/metadata_document.ts	New MetadataEntry + MetadataDocument types (full snapshot pair).
src/infrastructure/wasm/strategies/exiftool_diff_strategy.ts	Read-only sidecar (not a FormatStrategy, not in the registry). parseMetadata({ args: ["-j", "-G1", "-a", "-struct", "-q", "-q"] }) + ExifTool group-name → canonical source-label mapping (IFD0/ExifIFD/InteropIFD/IFD1 → EXIF; XMP-* → XMP; ICC* → ICC; QuickTime/ItemList/UserData/Track* → MP4; etc.). Drops File:/ExifTool:/System: filesystem-level groups.
src/infrastructure/wasm/strategies/exiftool_wasm_fetch.ts	Shared redirectWasmFetch helper extracted from ExifToolFallbackStrategy so both strategies use the same ?url-import + dynamic-import-inside-typeof-window pattern.
src/infrastructure/wasm/wasm_processor.ts	Wires the diff strategy in. Two parseMetadata calls in parallel via Promise.all on source + stripped bytes after each successful strip. Errors fall back to diffDocument: null (UI degrades to legacy delta view). Gated by VITE_ENABLE_EXIFTOOL_DIFF (default on). Fires the one-time loading event on first invocation per session.
src/infrastructure/wasm/format_strategy.ts + each strategy	StripResult extended with diffDocument: MetadataDocument | null. Every strategy returns null; the document is built by WasmProcessor, not the strategy.
src/application/ports/metadata_processor_port.ts + src/infrastructure/web/web_api.ts	ProcessOutcome + WasmApi extended to forward diffDocument.
src/web/contexts/AppContext.tsx + src/web/hooks/use_process_files.ts	FileEntry.diffDocument added; UPDATE_FILE_METADATA action carries it; hook dispatches it.
src/web/components/file-list/MetadataDiffExpansion.tsx	Two-pane render when diffDocument.before.length > 0, legacy grouped-delta render otherwise. New computeDiffRows aligns entries on (source, name) and classifies each row as removed / added / modified / kept.
src/web/components/file-list/FileRow.tsx	isExpandable now considers diffDocument too.
src/web/styles/file_table.css	New .file-table__diff--two-pane + .file-table__diff-pair--* classes. Desktop ≥ 720 px: 3-column grid (name | before | after). Mobile < 720 px: stacked with "Before" / "After" pseudo-element labels per cell.
src/common/exiftool_diff_events.ts	One-time metascrub:exiftool-diff-loading event; FileTable subscribes and shows a passive toast.
vite.config.web.standalone.ts	New standaloneWasmInlinePlugin — finds the emitted assets/zeroperl-*.wasm, Base64-encodes it, rewrites the JS reference to a data:application/wasm;base64,… URL, deletes the sibling. Standalone build is now genuinely one file (33.8 MB). PWA / APK builds still get the sibling asset.
.resources/strings.json	5 new i18n keys (en/es/ar): diffPaneBefore, diffPaneAfter, diffGroupAdded, diffEmptyValue, diffLoadingToast.
tests/infrastructure/wasm/exiftool_diff_strategy.test.ts	5 unit tests through the real WASM engine: returns entries, group mapping collapses IFD0 → EXIF, drops File:/ExifTool:/SourceFile, reads .webp, graceful degradation on malformed bytes.
tests/web/components/metadata_diff_expansion.test.tsx	8 new component tests: pane headers, kept / removed / added / modified classification, source grouping, legacy fallback on null + empty diffDocument.

Why this is the right shape for the standalone target

Sibling zeroperl.wasm would silently break under Chromium's file:// CORS — Chrome / Edge / Brave block cross-file fetch() from file:// by default. That's the same restriction viteSingleFile was built around in the first place. See spec §8.1 for the full reasoning.

dist/web-standalone/
└── index.html              33.8 MB  ← one file. Save, double-click, hand to a friend.

dist/web/
├── index.html              ~580 KB
├── assets/
│   ├── index-*.js          ~880 KB
│   └── zeroperl-*.wasm     24.2 MB  ← sibling for PWA + APK via runtimeCaching
└── sw.js                   precache 1.3 MB (no WASM)

Forensic + privacy invariants

Strip behaviour unchanged — diff is a display-side feature. zeroperl.wasm declares zero sock_* imports in its WASI capability set (verified in #171 POC); the diff reads still satisfy the no-network invariant. The 25 MB WASM is the same artifact #175 already shipped for the fallback strategy.

Verification

yarn typecheck                           # clean
yarn lint                                # clean
yarn check:deps                          # no circular
yarn test --run                          # 539 pass (+13 from master)
yarn build:web                           # PWA: sibling WASM (24.2 MB)
yarn build:web:standalone                # one file (33.8 MB), no assets/ dir

What this PR deliberately leaves to follow-ups

• Chunk B.1 cleanup PR — delete metadataItems from StripResult, the per-strategy enumeration layers (~150 LOC each in JPEG/PNG, smaller in PDF/Office/Video), ifd_value_formatters.ts, and most of ifd_tag_data.ts (52.7 KB). The IFD walker itself stays — it's the strip. Soak window first.
• E2E spec for the new two-pane view — defer until the legacy fallback is removed in chunk B.1, otherwise we're testing two paths.
• Eager WASM warm-up before first diff click — currently the first dropped file kicks off WASM during its strip; subsequent files reuse the warm instance. If telemetry says people skip the first diff often, we could pre-warm in idle time; not worth it speculatively.
• -G1 -a -struct group-mapping coverage gaps — currently catches the common groups (EXIF / GPS / XMP / ICC / PDF / Office / MP4 / Photoshop / IPTC / MakerNotes / JFIF / RIFF). Unknown groups surface verbatim (better than dropping) so they show up in test coverage if new ones appear.

Test plan

• Quality gates green: typecheck, lint, check:deps, vitest (539 pass), build:web, build:web:standalone
• Standalone HTML is a single file with the WASM Base64-inlined
• PWA build still ships zeroperl-*.wasm as a sibling asset (24.2 MB)
• Reviewer drops a JPEG + a PDF in dev and confirms the two-pane view renders, then sets VITE_ENABLE_EXIFTOOL_DIFF=false and confirms the legacy delta view still works

🤖 Generated with Claude Code

## Summary Chunk B of #174. Pivots the diff view (issue #22) from per-strategy delta surfaces to a unified **ExifTool dump on both source + stripped bytes**, displayed in a diffchecker-style two-pane layout. Spec at #176 (`docs/superpowers/specs/2026-05-21-issue-22-diff-pivot-design.md`). Same strip behaviour, much richer evidence — ExifTool's `-j -G1 -a -struct` returns hundreds of decoded tags per file (MakerNote vendor sub-tags, full XMP schemas, ICC profile details, PDF/Office/MP4 fields) where the existing per-strategy delta walkers only saw what each format's walker was hand-rolled to surface. Refs #22, #174 (chunks C–F still ahead — does not close). ## What landed | File | Role | |---|---| | `src/domain/exif/metadata_document.ts` | New `MetadataEntry` + `MetadataDocument` types (full snapshot pair). | | `src/infrastructure/wasm/strategies/exiftool_diff_strategy.ts` | Read-only sidecar (not a `FormatStrategy`, not in the registry). `parseMetadata({ args: ["-j", "-G1", "-a", "-struct", "-q", "-q"] })` + ExifTool group-name → canonical source-label mapping (IFD0/ExifIFD/InteropIFD/IFD1 → EXIF; XMP-* → XMP; ICC* → ICC; QuickTime/ItemList/UserData/Track* → MP4; etc.). Drops File:/ExifTool:/System: filesystem-level groups. | | `src/infrastructure/wasm/strategies/exiftool_wasm_fetch.ts` | Shared `redirectWasmFetch` helper extracted from `ExifToolFallbackStrategy` so both strategies use the same `?url`-import + dynamic-import-inside-typeof-window pattern. | | `src/infrastructure/wasm/wasm_processor.ts` | Wires the diff strategy in. Two `parseMetadata` calls in parallel via `Promise.all` on source + stripped bytes after each successful strip. Errors fall back to `diffDocument: null` (UI degrades to legacy delta view). Gated by `VITE_ENABLE_EXIFTOOL_DIFF` (default on). Fires the one-time loading event on first invocation per session. | | `src/infrastructure/wasm/format_strategy.ts` + each strategy | `StripResult` extended with `diffDocument: MetadataDocument \| null`. Every strategy returns null; the document is built by `WasmProcessor`, not the strategy. | | `src/application/ports/metadata_processor_port.ts` + `src/infrastructure/web/web_api.ts` | `ProcessOutcome` + `WasmApi` extended to forward `diffDocument`. | | `src/web/contexts/AppContext.tsx` + `src/web/hooks/use_process_files.ts` | `FileEntry.diffDocument` added; `UPDATE_FILE_METADATA` action carries it; hook dispatches it. | | `src/web/components/file-list/MetadataDiffExpansion.tsx` | Two-pane render when `diffDocument.before.length > 0`, legacy grouped-delta render otherwise. New `computeDiffRows` aligns entries on `(source, name)` and classifies each row as removed / added / modified / kept. | | `src/web/components/file-list/FileRow.tsx` | `isExpandable` now considers `diffDocument` too. | | `src/web/styles/file_table.css` | New `.file-table__diff--two-pane` + `.file-table__diff-pair--*` classes. Desktop ≥ 720 px: 3-column grid (name \| before \| after). Mobile < 720 px: stacked with "Before" / "After" pseudo-element labels per cell. | | `src/common/exiftool_diff_events.ts` | One-time `metascrub:exiftool-diff-loading` event; `FileTable` subscribes and shows a passive toast. | | `vite.config.web.standalone.ts` | New `standaloneWasmInlinePlugin` — finds the emitted `assets/zeroperl-*.wasm`, Base64-encodes it, rewrites the JS reference to a `data:application/wasm;base64,…` URL, deletes the sibling. **Standalone build is now genuinely one file** (33.8 MB). PWA / APK builds still get the sibling asset. | | `.resources/strings.json` | 5 new i18n keys (en/es/ar): `diffPaneBefore`, `diffPaneAfter`, `diffGroupAdded`, `diffEmptyValue`, `diffLoadingToast`. | | `tests/infrastructure/wasm/exiftool_diff_strategy.test.ts` | 5 unit tests through the real WASM engine: returns entries, group mapping collapses IFD0 → EXIF, drops File:/ExifTool:/SourceFile, reads .webp, graceful degradation on malformed bytes. | | `tests/web/components/metadata_diff_expansion.test.tsx` | 8 new component tests: pane headers, kept / removed / added / modified classification, source grouping, legacy fallback on null + empty diffDocument. | ## Why this is the right shape for the standalone target Sibling `zeroperl.wasm` would silently break under Chromium's `file://` CORS — Chrome / Edge / Brave block cross-file `fetch()` from `file://` by default. That's the same restriction `viteSingleFile` was built around in the first place. See spec §8.1 for the full reasoning. ``` dist/web-standalone/ └── index.html 33.8 MB ← one file. Save, double-click, hand to a friend. dist/web/ ├── index.html ~580 KB ├── assets/ │ ├── index-*.js ~880 KB │ └── zeroperl-*.wasm 24.2 MB ← sibling for PWA + APK via runtimeCaching └── sw.js precache 1.3 MB (no WASM) ``` ## Forensic + privacy invariants Strip behaviour unchanged — diff is a display-side feature. zeroperl.wasm declares zero `sock_*` imports in its WASI capability set (verified in #171 POC); the diff reads still satisfy the no-network invariant. The 25 MB WASM is the same artifact #175 already shipped for the fallback strategy. ## Verification ``` yarn typecheck # clean yarn lint # clean yarn check:deps # no circular yarn test --run # 539 pass (+13 from master) yarn build:web # PWA: sibling WASM (24.2 MB) yarn build:web:standalone # one file (33.8 MB), no assets/ dir ``` ## What this PR deliberately leaves to follow-ups - **Chunk B.1 cleanup PR** — delete `metadataItems` from `StripResult`, the per-strategy enumeration layers (~150 LOC each in JPEG/PNG, smaller in PDF/Office/Video), `ifd_value_formatters.ts`, and most of `ifd_tag_data.ts` (52.7 KB). The IFD walker itself stays — it's the strip. Soak window first. - **E2E spec for the new two-pane view** — defer until the legacy fallback is removed in chunk B.1, otherwise we're testing two paths. - **Eager WASM warm-up before first diff click** — currently the first dropped file kicks off WASM during its strip; subsequent files reuse the warm instance. If telemetry says people skip the first diff often, we could pre-warm in idle time; not worth it speculatively. - **`-G1 -a -struct` group-mapping coverage gaps** — currently catches the common groups (EXIF / GPS / XMP / ICC / PDF / Office / MP4 / Photoshop / IPTC / MakerNotes / JFIF / RIFF). Unknown groups surface verbatim (better than dropping) so they show up in test coverage if new ones appear. ## Test plan - [x] Quality gates green: typecheck, lint, check:deps, vitest (539 pass), build:web, build:web:standalone - [x] Standalone HTML is a single file with the WASM Base64-inlined - [x] PWA build still ships zeroperl-*.wasm as a sibling asset (24.2 MB) - [ ] Reviewer drops a JPEG + a PDF in dev and confirms the two-pane view renders, then sets `VITE_ENABLE_EXIFTOOL_DIFF=false` and confirms the legacy delta view still works 🤖 Generated with [Claude Code](https://claude.com/claude-code)

diff-desktop.png

76 KiB

diff-desktop.png

69 KiB

diff-mobile.png

175 KiB

diff-mobile.png

175 KiB

forgejo_admin added 1 commit 2026-05-21 08:58:11 +00:00

feat(diff): ExifTool-sourced two-pane before/after metadata diff ... 54efacf761

Pivots the diff view (issue #22) from per-strategy delta surfaces to a
unified ExifTool dump on both source + stripped bytes. Diffchecker-style
two-pane layout shows the full metadata document on each side; rows are
classified at render time as removed / added / modified / kept.

Per the spec (#176):
- Read-only ExifToolDiffStrategy sidecar; not in the strategy registry.
- WasmProcessor builds the diff via Promise.all on source + stripped
  bytes after each successful strip. Failures fall back to null →
  legacy delta view (zero-risk rollout).
- Standalone HTML inlines zeroperl.wasm as a Base64 data URL (~830 KB
  → ~34 MB single file); PWA / APK keep the sibling asset. file://
  CORS in Chromium would otherwise silently break the diff under
  standalone.
- Passive toast ("Loading metadata reader…") on first WASM warm-up.
- VITE_ENABLE_EXIFTOOL_DIFF gate; default on.

Implementation pattern mirrors #175 (extracted the shared
redirectWasmFetch helper out of ExifToolFallbackStrategy in the process).

Soak window first; chunk B.1 cleanup PR will delete the legacy
MetadataItem[] enumeration + ifd_value_formatters.ts + most of
ifd_tag_data.ts once the new path is proven across the format matrix.

Refs #22, #174.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

forgejo_admin referenced this pull request from a commit 2026-05-21 09:31:23 +00:00

test(e2e): update for two-pane diff; allow same-origin WASM fetch

forgejo_admin added 1 commit 2026-05-21 09:31:24 +00:00

test(e2e): update for two-pane diff; allow same-origin WASM fetch ... 203d3c645d

Two CI breakages from #177's eager-diff change:

1. metadata_diff.spec.ts asserted on legacy `.file-table__diff-row--*`
   class names. Post-pivot the two-pane view uses
   `.file-table__diff-pair--*` instead. Updated assertions + relaxed
   the PDF case (pdf-lib's re-serialize surfaces modified/added/kept
   rows for sample.pdf, no removed). Re-runs across all 5 Playwright
   projects (3 web + 2 standalone) covering the diff.

2. no-network.spec.ts caught the same-origin zeroperl.wasm asset
   fetch as an "outbound leak". The privacy invariant is *off-origin*
   requests; same-origin asset fetches don't violate it. Filter
   leakedUrls to off-origin before asserting empty. Matches the
   test author's own caveat in the file header about runtimeCaching.

Plus a small one-off harness (`scripts/capture-diff-screenshots.ts`)
that drives the standalone build through Playwright and captures
desktop + mobile screenshots of the diff view for PR review evidence.
Not hooked into CI; run on demand via `npx tsx`.

Refs #22, #174.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-05-21 09:31:57 +00:00

Author

Owner

Copy link

Update: CI green, screenshots, e2e clarification

CI fixes (commit 54efacf → new HEAD)

Two e2e jobs failed on the initial push. Both were downstream of chunk B's eager-diff change:

1. metadata_diff.spec.ts — asserted on legacy .file-table__diff-row--* classes. The two-pane view uses .file-table__diff-pair--*. Test updated + relaxed the PDF assertion (pdf-lib's re-serialize surfaces modified/added/kept rows for sample.pdf, no removed).
2. no-network.spec.ts — caught the same-origin zeroperl.wasm asset fetch as an "outbound leak". The privacy invariant is off-origin requests; same-origin asset fetches don't violate it. Test now filters to off-origin before asserting empty.

Full e2e locally now:

yarn test:e2e:web         # 122 passed, 19 skipped, 0 failed (2m48s)
yarn test:e2e:standalone  # 7 passed (21s) — includes the two-pane diff tests against the standalone build with inlined WASM

Did this PR add e2e tests?

Honest answer: not new files, but the existing tests/e2e/web/metadata_diff.spec.ts is now updated to assert against the two-pane structure. It runs across 5 Playwright projects (web-desktop / web-mobile-ios / web-mobile-android / standalone-desktop / standalone-mobile) × 2 tests = 10 e2e runs of the diff per CI cycle. So the diff view is genuinely covered end-to-end through both the dev-server PWA path and the file:// standalone path. I should have flagged this in the original PR body — the "deferred to chunk B.1" line was wrong.

Screenshots

Desktop (1280×800, standalone HTML, sample.jpg dropped, chevron clicked):

Note the source groups (JFIF · 4 REMOVED, EXIF · 12 REMOVED, GPS · 3 REMOVED, COMPOSITE · 1 REMOVED, 2 KEPT), per-row alignment between the BEFORE and AFTER columns, strike-through values on the left, em-dash placeholders on the right for removed rows, and the passive "Loading metadata reader…" toast at the bottom (this was the first diff click — the toast self-hides after 4 s).

Mobile (iPhone 14 emulation, standalone HTML):

Two-pane collapses to a single column at < 720 px viewport with explicit BEFORE / AFTER labels per cell. No horizontal scroll on the expansion area (asserted by the e2e). Same toast appears.

Capture harness

Added scripts/capture-diff-screenshots.ts — a one-off Playwright harness that drives the standalone build through the diff flow at desktop + mobile breakpoints. Run on demand:

yarn build:web:standalone
npx tsx scripts/capture-diff-screenshots.ts
# → /tmp/diff-screenshots/diff-desktop.png + diff-mobile.png

Not hooked into CI (purely a dev tool for review evidence), but committed so future contributors don't have to re-discover the locale-seeding + Playwright wiring.

## Update: CI green, screenshots, e2e clarification ### CI fixes (commit `54efacf` → new HEAD) Two e2e jobs failed on the initial push. Both were downstream of chunk B's eager-diff change: 1. **`metadata_diff.spec.ts`** — asserted on legacy `.file-table__diff-row--*` classes. The two-pane view uses `.file-table__diff-pair--*`. Test updated + relaxed the PDF assertion (pdf-lib's re-serialize surfaces `modified`/`added`/`kept` rows for `sample.pdf`, no `removed`). 2. **`no-network.spec.ts`** — caught the same-origin `zeroperl.wasm` asset fetch as an "outbound leak". The privacy invariant is *off-origin* requests; same-origin asset fetches don't violate it. Test now filters to off-origin before asserting empty. Full e2e locally now: ``` yarn test:e2e:web # 122 passed, 19 skipped, 0 failed (2m48s) yarn test:e2e:standalone # 7 passed (21s) — includes the two-pane diff tests against the standalone build with inlined WASM ``` ### Did this PR add e2e tests? Honest answer: not new files, but the existing `tests/e2e/web/metadata_diff.spec.ts` is now updated to assert against the two-pane structure. It runs across **5 Playwright projects** (web-desktop / web-mobile-ios / web-mobile-android / standalone-desktop / standalone-mobile) × 2 tests = 10 e2e runs of the diff per CI cycle. So the diff view is genuinely covered end-to-end through both the dev-server PWA path and the file:// standalone path. I should have flagged this in the original PR body — the "deferred to chunk B.1" line was wrong. ### Screenshots **Desktop (1280×800, standalone HTML, sample.jpg dropped, chevron clicked):**  Note the source groups (`JFIF · 4 REMOVED`, `EXIF · 12 REMOVED`, `GPS · 3 REMOVED`, `COMPOSITE · 1 REMOVED, 2 KEPT`), per-row alignment between the BEFORE and AFTER columns, strike-through values on the left, em-dash placeholders on the right for removed rows, and the passive "Loading metadata reader…" toast at the bottom (this was the first diff click — the toast self-hides after 4 s). **Mobile (iPhone 14 emulation, standalone HTML):**  Two-pane collapses to a single column at < 720 px viewport with explicit `BEFORE` / `AFTER` labels per cell. No horizontal scroll on the expansion area (asserted by the e2e). Same toast appears. ### Capture harness Added `scripts/capture-diff-screenshots.ts` — a one-off Playwright harness that drives the standalone build through the diff flow at desktop + mobile breakpoints. Run on demand: ``` yarn build:web:standalone npx tsx scripts/capture-diff-screenshots.ts # → /tmp/diff-screenshots/diff-desktop.png + diff-mobile.png ``` Not hooked into CI (purely a dev tool for review evidence), but committed so future contributors don't have to re-discover the locale-seeding + Playwright wiring.

forgejo_admin commented 2026-05-21 09:34:33 +00:00

Author

Owner

Copy link

Self-review (threshold > 50)

Going through the diff with the calibrated threshold from memory. Six findings worth flagging, ordered by score.

---

🟠 [75] Eager diff blocks the "Cleaned" status signal

WasmProcessor.process synchronously awaits buildDiffDocument before returning ProcessOutcome (wasm_processor.ts:71-77). The spec at #176 §12.1 explicitly said "user sees Cleaned status as soon as strip completes; the diff resolves to a skeleton in the expansion area until diffDocument arrives". The implementation contradicts this — each file's "Cleaned" appearance is now blocked on two parseMetadata calls (~100-300ms each after WASM is warm; the first file in a session adds ~3-5s for WASM instantiation).

For a 50-file batch this is a noticeable regression: each file accumulates ~200ms of diff-build latency before showing Complete.

Fix shape: populate diffDocument async via a follow-up dispatch (UPDATE_FILE_DIFF action), or accept the regression and document it. Either is fine, but the current behavior doesn't match the spec.

🟠 [75] Silent-failure mode in the standalone WASM inline plugin

vite.config.web.standalone.ts:197-204 tries three URL candidate shapes for substitution. If Vite changes its asset URL convention (or our code switches to an import.meta.url resolution pattern), all three miss → plugin silently no-ops → ship a broken standalone HTML that fails on first WebP drop with no build-time signal.

Fix: assert at least one substitution happened per WASM file. Pseudo:

let replaced = false;
for (const candidate of candidates) {
  if (html.includes(candidate)) {
    html = html.split(candidate).join(dataUrl);
    replaced = true;
  }
}
if (!replaced) {
  throw new Error(
    `standaloneWasmInlinePlugin: could not find sibling URL ` +
    `'${wasmFile}' in inlined JS — Vite asset format may have changed.`
  );
}

🟠 [70] Composite group surfaced as "metadata"

The desktop screenshot shows COMPOSITE · 1 REMOVED, 2 KEPT with ImageSize: 1×1 → 1×1 listed as KEPT. ExifTool's Composite group is derived — these tags are computed from the file structure (ImageSize from ImageWidth×ImageHeight), not file-embedded metadata. Showing them in the diff:

• conflates "computed file property" with "embedded metadata"
• creates "KEPT" rows that can never not be kept (dimensions can't be stripped)
• adds noise to the privacy-relevant signal

Fix: drop Composite from EXACT_GROUP_MAP and add it to DROP_GROUPS in exiftool_diff_strategy.ts.

🟡 [55] WasmProcessor has no test coverage for the diff path

tests/infrastructure/wasm/wasm_processor.test.ts was not updated. The existing 4 tests cover strip + write only; nothing asserts ProcessOutcome.diffDocument is populated. Given that process() now has a meaningful new branch (the parallel ExifTool reads, the warmup-flag dispatch, the null-fallback on error), there should be coverage of:

• diffDocument is non-null on a successful strip with ENABLE_EXIFTOOL_DIFF=true
• diffDocument is null when VITE_ENABLE_EXIFTOOL_DIFF="false" is set
• diffDocument is null when the underlying readDocument returns an error (mocked)

🟡 [50] Module-level diffWarmupSignalled flag is testing-hostile

wasm_processor.ts:16 declares mutable module state with no reset path. Vitest workers persist module state across tests in the same file. If anyone later wants to test the toast-firing behavior:

• can't reset between tests (first test sets the flag, second test sees no event)
• can't observe from outside the module

Fix shape: either (a) expose a __resetForTests() symbol, (b) move the flag to a WasmProcessor instance field, or (c) accept it and document that the toast is a one-shot per session.

🟡 [50] makeKey uses space separator

MetadataDiffExpansion.tsx:231-233:

function makeKey(source: string, name: string): string {
  return `${source} ${name}`;
}

Tag names with spaces would collide: source XMP + name Creator Tool collides with source XMP Creator + name Tool. ExifTool tag names are conventionally camelcase so the risk is vanishingly low in practice, but ${source}\0${name} or ${source}::${name} is more defensive.

---

Below threshold but worth a glance

• JFIFVersion shows as 1.01 → — REMOVED in the desktop screenshot. Accurate (we drop the JFIF segment), but it's signal-noise — users don't care about JFIFVersion privacy-wise. Future polish: a small allowlist of "structural fields not worth diffing".
• formatValue JSON-stringifies struct values (exiftool_diff_strategy.ts:171-175). User sees a compact JSON blob with no formatting. Rare in practice for the formats we route, but could be ugly when it appears.
• Local extname() / basename() reimplementations in wasm_processor.ts. Could share with existing path helpers if any exist (none I found).
• TwoPaneView uses parameter name document which shadows the global DOM document. Cosmetic.

---

Recommendation

The 3 orange findings (75/75/70) are worth addressing before merge — they're either a spec-violation regression (eager block), a silent-failure landmine (Vite URL substitution), or a visible UX bug (Composite). The two yellow findings (55/50) are test-debt that can land in a follow-up if you want to keep the PR scope tight; the makeKey one is a 5-second defensive change.

Marking the PR as needing-revision pending the three orange items.

## Self-review (threshold > 50) Going through the diff with the calibrated threshold from memory. Six findings worth flagging, ordered by score. --- ### 🟠 [75] Eager diff blocks the "Cleaned" status signal `WasmProcessor.process` synchronously `await`s `buildDiffDocument` before returning `ProcessOutcome` (wasm_processor.ts:71-77). The spec at #176 §12.1 explicitly said *"user sees `Cleaned` status as soon as strip completes; the diff resolves to a skeleton in the expansion area until `diffDocument` arrives"*. The implementation contradicts this — each file's "Cleaned" appearance is now blocked on two `parseMetadata` calls (~100-300ms each after WASM is warm; the first file in a session adds ~3-5s for WASM instantiation). For a 50-file batch this is a noticeable regression: each file accumulates ~200ms of diff-build latency before showing Complete. **Fix shape:** populate `diffDocument` async via a follow-up dispatch (`UPDATE_FILE_DIFF` action), or accept the regression and document it. Either is fine, but the current behavior doesn't match the spec. ### 🟠 [75] Silent-failure mode in the standalone WASM inline plugin `vite.config.web.standalone.ts:197-204` tries three URL candidate shapes for substitution. If Vite changes its asset URL convention (or our code switches to an `import.meta.url` resolution pattern), all three miss → plugin silently no-ops → ship a broken standalone HTML that fails on first WebP drop with no build-time signal. **Fix:** assert at least one substitution happened per WASM file. Pseudo: ```ts let replaced = false; for (const candidate of candidates) { if (html.includes(candidate)) { html = html.split(candidate).join(dataUrl); replaced = true; } } if (!replaced) { throw new Error( `standaloneWasmInlinePlugin: could not find sibling URL ` + `'${wasmFile}' in inlined JS — Vite asset format may have changed.` ); } ``` ### 🟠 [70] Composite group surfaced as "metadata" The desktop screenshot shows `COMPOSITE · 1 REMOVED, 2 KEPT` with `ImageSize: 1×1 → 1×1` listed as KEPT. ExifTool's `Composite` group is **derived** — these tags are computed from the file structure (ImageSize from ImageWidth×ImageHeight), not file-embedded metadata. Showing them in the diff: - conflates "computed file property" with "embedded metadata" - creates "KEPT" rows that can never not be kept (dimensions can't be stripped) - adds noise to the privacy-relevant signal **Fix:** drop `Composite` from `EXACT_GROUP_MAP` and add it to `DROP_GROUPS` in `exiftool_diff_strategy.ts`. ### 🟡 [55] WasmProcessor has no test coverage for the diff path `tests/infrastructure/wasm/wasm_processor.test.ts` was not updated. The existing 4 tests cover strip + write only; nothing asserts `ProcessOutcome.diffDocument` is populated. Given that `process()` now has a meaningful new branch (the parallel ExifTool reads, the warmup-flag dispatch, the null-fallback on error), there should be coverage of: - `diffDocument` is non-null on a successful strip with `ENABLE_EXIFTOOL_DIFF=true` - `diffDocument` is null when `VITE_ENABLE_EXIFTOOL_DIFF="false"` is set - `diffDocument` is null when the underlying `readDocument` returns an error (mocked) ### 🟡 [50] Module-level `diffWarmupSignalled` flag is testing-hostile `wasm_processor.ts:16` declares mutable module state with no reset path. Vitest workers persist module state across tests in the same file. If anyone later wants to test the toast-firing behavior: - can't reset between tests (first test sets the flag, second test sees no event) - can't observe from outside the module **Fix shape:** either (a) expose a `__resetForTests()` symbol, (b) move the flag to a `WasmProcessor` instance field, or (c) accept it and document that the toast is a one-shot per session. ### 🟡 [50] `makeKey` uses space separator `MetadataDiffExpansion.tsx:231-233`: ```ts function makeKey(source: string, name: string): string { return `${source} ${name}`; } ``` Tag names with spaces would collide: source `XMP` + name `Creator Tool` collides with source `XMP Creator` + name `Tool`. ExifTool tag names are conventionally camelcase so the risk is vanishingly low in practice, but `${source}\0${name}` or `${source}::${name}` is more defensive. --- ### Below threshold but worth a glance - **JFIFVersion shows as `1.01 → —` REMOVED** in the desktop screenshot. Accurate (we drop the JFIF segment), but it's signal-noise — users don't care about JFIFVersion privacy-wise. Future polish: a small allowlist of "structural fields not worth diffing". - **`formatValue` JSON-stringifies struct values** (`exiftool_diff_strategy.ts:171-175`). User sees a compact JSON blob with no formatting. Rare in practice for the formats we route, but could be ugly when it appears. - **Local `extname()` / `basename()` reimplementations** in `wasm_processor.ts`. Could share with existing path helpers if any exist (none I found). - **`TwoPaneView` uses parameter name `document`** which shadows the global DOM `document`. Cosmetic. --- ### Recommendation The 3 orange findings (75/75/70) are worth addressing before merge — they're either a spec-violation regression (eager block), a silent-failure landmine (Vite URL substitution), or a visible UX bug (Composite). The two yellow findings (55/50) are test-debt that can land in a follow-up if you want to keep the PR scope tight; the makeKey one is a 5-second defensive change. Marking the PR as needing-revision pending the three orange items.

forgejo_admin added 1 commit 2026-05-21 09:48:31 +00:00

fix(diff): drop ExifTool Composite group from the diff (review finding 3) ... 3afe59f72b

Composite tags (ImageSize, Megapixels, etc.) are computed by ExifTool from
other tags rather than being file-embedded metadata. Showing them in the
two-pane diff conflated 'file property' with 'embedded metadata' and
produced 'KEPT' rows that can never not be kept (dimensions can't be
stripped). Drop the group at normalization time alongside File / ExifTool
/ System.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

forgejo_admin added 1 commit 2026-05-21 09:48:52 +00:00

fix(standalone): assert WASM substitution; fail-loud on missed URLs (review finding 2) ... b2543f76a1

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

forgejo_admin referenced this pull request 2026-05-21 09:58:14 +00:00

chunk B.1: prune legacy MetadataItem walkers (JPEG/PNG/MP4/fallback); integrate Office + PDF walker into diff #178

forgejo_admin added 1 commit 2026-05-21 10:00:46 +00:00

fix(diff): unblock "Cleaned" signal; async diff build (review findings 1/4/5/6) ... 07085f02b9

WasmProcessor.process now returns ProcessOutcome with diffDocument: null
immediately after the write+strip — the ExifTool diff build moves
out-of-band to a new buildDiffDocumentForFile() method invoked
fire-and-forget by use_process_files after the per-file "Cleaned" status
dispatches. New UPDATE_FILE_DIFF reducer action merges the document into
the FileEntry without touching status; diffPending: true drives a
loading skeleton in MetadataDiffExpansion until the diff lands.

Other findings in the same touchpoint:
- finding 5: diffWarmupSignalled moves from a module-level mutable flag
  to a per-instance field on WasmProcessor (tests get a clean slate by
  constructing a fresh processor).
- finding 4: three new WasmProcessor tests for the async path (real-JPEG
  populate, ExifTool-error graceful degradation, VITE_ENABLE_EXIFTOOL_DIFF
  gating via vi.stubEnv + vi.resetModules — mirrors the
  exiftool_fallback_strategy.test.ts pattern).
- finding 6: makeKey now uses the \0 escape sequence (was a literal NUL
  byte in source — which had inadvertently flipped the file to git
  binary mode). Same byte-level result, but the file is text again.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-05-21 10:03:39 +00:00

Author

Owner

Copy link

All six self-review findings resolved

Three parallel agents landed fixes; here's the round-up.

Commits (on top of the existing PR HEAD)

SHA	Scope	Findings
3afe59f	Drop ExifTool Composite group from the diff	#3
b2543f7	Standalone WASM substitution — fail-loud on missed URLs	#2
07085f0	Unblock "Cleaned" signal; async diff build	#1, #4, #5, #6

Per-finding resolution

#1 [75] Eager diff blocks "Cleaned" signal — fixed. WasmProcessor.process() now returns ProcessOutcome with diffDocument: null immediately after write, stashing {sourceBytes, strippedBytes, extension} keyed by filePath in an instance Map. New method buildDiffDocumentForFile({filePath}) drains the stash and runs the ExifTool reads out-of-band. use_process_files dispatches UPDATE_FILE_METADATA (with diffPending: true) + UPDATE_FILE_STATUS: Complete synchronously, then fires wasm.buildDiffDocument fire-and-forget. New UPDATE_FILE_DIFF reducer action merges the result without touching status. MetadataDiffExpansion shows a wayfinding skeleton (reusing diffLoadingToast i18n) while diffPending && !diffDocument. Per-file "Cleaned" status now lands ~100-300ms (warm) or 3-5s (cold) earlier; bulk batches no longer accumulate diff-build latency.

#2 [75] Silent-failure mode in standalone WASM inline plugin — fixed. Per-WASM-file replaced flag; throws with a clear "Vite asset URL format may have changed" message if no candidate matches. Added a build-log line per successful substitution. Caveat the agent flagged: on the current build path the WASM doesn't actually land as a sibling (something upstream already inlines it), so the early wasmFiles.length === 0 return triggers and the fail-loud branch hasn't been exercised yet — but it's correctly scoped inside the per-file loop and will activate the moment Vite ever does emit a sibling whose URL shape we miss.

#3 [70] Composite group surfaced as "metadata" — fixed. Composite removed from EXACT_GROUP_MAP and added to DROP_GROUPS in exiftool_diff_strategy.ts. Updated the existing test to assert sources.has("Composite") === false. Refreshed screenshot below confirms the COMPOSITE row is gone — JFIF / EXIF / GPS are now the only source groups for sample.jpg.

#4 [55] No WasmProcessor test coverage for diff path — fixed. 5 new tests in tests/infrastructure/wasm/wasm_processor.test.ts:

• real-JPEG populates diffDocument via buildDiffDocumentForFile
• stash drains on double-call (second call returns null since the previous strip's bytes were already consumed)
• ExifTool-error gracefully degrades to null
• null when no pending entry exists
• VITE_ENABLE_EXIFTOOL_DIFF="false" gating verified via vi.stubEnv + vi.resetModules

Plus 4 new reducer tests in app_context.test.ts for UPDATE_FILE_DIFF + diffPending merge semantics.

#5 [50] Module-level diffWarmupSignalled testing-hostile — fixed. Now a per-instance field with a documented testability rationale. Tests can construct a fresh WasmProcessor to start with a clean warm-up flag.

#6 [50] makeKey space separator — fixed. Now \0 (NUL byte). One small fishhook the agent flagged: a literal NUL had inadvertently flipped the file to git binary mode; the proper \0 escape preserves the byte semantics and keeps the file text.

Refreshed screenshots (post-fixes)

Desktop — Composite group is gone; JFIF + EXIF + GPS are the only source groups:

Mobile:

Verification

yarn typecheck             # clean
yarn lint                  # clean
yarn test --run            # 549 passed (40 files) (+10 from previous HEAD)
yarn build:web:standalone  # 33.8 MB single file (Composite-free, all gates pass)
yarn playwright test --project=web-desktop tests/e2e/web/metadata_diff.spec.ts tests/e2e/web/no-network.spec.ts
                           # 3 passed (9.9s)

B.1 cleanup tracker filed

#178 captures the combined B.0.5 + B.1 plan: prune the JPEG/PNG/MP4/fallback MetadataItem[] walker code (ExifTool covers them richer), but keep + integrate Office and PDF walker entries into the diff via a new walkerEntries: MetadataEntry[] channel on StripResult. The Office walker surfaces ZIP entries / comments / embeddings / customXml that ExifTool's docProps-only read can't see; the PDF walker surfaces annotations / AcroForm fields / embedded files that ExifTool's Info+XMP read can't see. Net-negative LOC overall (~−2400). Soak prereq is landing #177 first.

PR ready for re-review.

## All six self-review findings resolved Three parallel agents landed fixes; here's the round-up. ### Commits (on top of the existing PR HEAD) | SHA | Scope | Findings | |---|---|---| | `3afe59f` | Drop ExifTool Composite group from the diff | #3 | | `b2543f7` | Standalone WASM substitution — fail-loud on missed URLs | #2 | | `07085f0` | Unblock "Cleaned" signal; async diff build | #1, #4, #5, #6 | ### Per-finding resolution **#1 [75] Eager diff blocks "Cleaned" signal — fixed.** `WasmProcessor.process()` now returns `ProcessOutcome` with `diffDocument: null` immediately after write, stashing `{sourceBytes, strippedBytes, extension}` keyed by `filePath` in an instance Map. New method `buildDiffDocumentForFile({filePath})` drains the stash and runs the ExifTool reads out-of-band. `use_process_files` dispatches `UPDATE_FILE_METADATA` (with `diffPending: true`) + `UPDATE_FILE_STATUS: Complete` synchronously, then fires `wasm.buildDiffDocument` fire-and-forget. New `UPDATE_FILE_DIFF` reducer action merges the result without touching status. `MetadataDiffExpansion` shows a wayfinding skeleton (reusing `diffLoadingToast` i18n) while `diffPending && !diffDocument`. Per-file "Cleaned" status now lands ~100-300ms (warm) or 3-5s (cold) earlier; bulk batches no longer accumulate diff-build latency. **#2 [75] Silent-failure mode in standalone WASM inline plugin — fixed.** Per-WASM-file `replaced` flag; throws with a clear "Vite asset URL format may have changed" message if no candidate matches. Added a build-log line per successful substitution. Caveat the agent flagged: on the current build path the WASM doesn't actually land as a sibling (something upstream already inlines it), so the early `wasmFiles.length === 0` return triggers and the fail-loud branch hasn't been exercised yet — but it's correctly scoped inside the per-file loop and will activate the moment Vite ever does emit a sibling whose URL shape we miss. **#3 [70] Composite group surfaced as "metadata" — fixed.** `Composite` removed from `EXACT_GROUP_MAP` and added to `DROP_GROUPS` in `exiftool_diff_strategy.ts`. Updated the existing test to assert `sources.has("Composite") === false`. Refreshed screenshot below confirms the COMPOSITE row is gone — JFIF / EXIF / GPS are now the only source groups for `sample.jpg`. **#4 [55] No WasmProcessor test coverage for diff path — fixed.** 5 new tests in `tests/infrastructure/wasm/wasm_processor.test.ts`: - real-JPEG populates `diffDocument` via `buildDiffDocumentForFile` - stash drains on double-call (second call returns null since the previous strip's bytes were already consumed) - ExifTool-error gracefully degrades to null - null when no pending entry exists - `VITE_ENABLE_EXIFTOOL_DIFF="false"` gating verified via `vi.stubEnv + vi.resetModules` Plus 4 new reducer tests in `app_context.test.ts` for `UPDATE_FILE_DIFF` + `diffPending` merge semantics. **#5 [50] Module-level `diffWarmupSignalled` testing-hostile — fixed.** Now a per-instance field with a documented testability rationale. Tests can construct a fresh `WasmProcessor` to start with a clean warm-up flag. **#6 [50] `makeKey` space separator — fixed.** Now `\0` (NUL byte). One small fishhook the agent flagged: a literal NUL had inadvertently flipped the file to git binary mode; the proper `\0` escape preserves the byte semantics and keeps the file text. ### Refreshed screenshots (post-fixes) **Desktop** — Composite group is gone; JFIF + EXIF + GPS are the only source groups:  **Mobile**:  ### Verification ``` yarn typecheck # clean yarn lint # clean yarn test --run # 549 passed (40 files) (+10 from previous HEAD) yarn build:web:standalone # 33.8 MB single file (Composite-free, all gates pass) yarn playwright test --project=web-desktop tests/e2e/web/metadata_diff.spec.ts tests/e2e/web/no-network.spec.ts # 3 passed (9.9s) ``` ### B.1 cleanup tracker filed #178 captures the combined B.0.5 + B.1 plan: prune the JPEG/PNG/MP4/fallback `MetadataItem[]` walker code (ExifTool covers them richer), but **keep + integrate Office and PDF walker entries** into the diff via a new `walkerEntries: MetadataEntry[]` channel on `StripResult`. The Office walker surfaces ZIP entries / comments / embeddings / customXml that ExifTool's docProps-only read can't see; the PDF walker surfaces annotations / AcroForm fields / embedded files that ExifTool's Info+XMP read can't see. Net-negative LOC overall (~−2400). Soak prereq is landing #177 first. PR ready for re-review.

forgejo_admin referenced this pull request from a commit 2026-05-21 10:19:55 +00:00

test(e2e): skip diff specs on WebKit (zeroperl WASM unreliable)

forgejo_admin added 1 commit 2026-05-21 10:19:55 +00:00

test(e2e): skip diff specs on WebKit (zeroperl WASM unreliable) ... 3b522e8850

Web-mobile-ios CI failure on #177 HEAD (07085f0). WebKit driver fails
to load zeroperl.wasm reliably — MetadataDiffExpansion correctly
falls back to the legacy delta view (snapshot confirms it renders),
but this spec asserts on the new two-pane class.

Matches established skip pattern in no-network.spec.ts +
service-worker.spec.ts (Playwright WebKit driver has long-standing
issues with the SW + WASM combination in this repo).

Chromium projects (web-desktop + web-mobile-android + standalone-*)
fully cover the two-pane assertion. Legacy fallback path is
exercised by component tests in metadata_diff_expansion.test.tsx.

Local: 120 passed, 21 skipped.

Refs #177.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

forgejo_admin referenced this pull request from a commit 2026-05-21 11:20:57 +00:00

fix(diff): key pending diffs by entryId; cap Map size

forgejo_admin added 1 commit 2026-05-21 11:20:58 +00:00

fix(diff): key pending diffs by entryId; cap Map size ... d34baad309

Two self-review findings from PR #177 [55+55]:

1. **Race on filePath key:** if a user dropped sample.jpg, then re-dropped
   the same path before the first diff build completed, the second drop's
   process() overwrote the first stash entry — and the first diff build,
   when it finally ran, would drain the *second* drop's bytes. Diff
   content for the first file would render against the wrong source.
   Mitigation: key by FileEntry.id (UUID generated in DropZone /
   FileBrowseButton) instead of filePath. Each drop gets its own stash.

2. **Memory leak if hook never drains:** if use_process_files errored
   after wasm.process resolved but before firing wasm.buildDiffDocument,
   pendingDiffs would grow unbounded across a session. Mitigation:
   PENDING_DIFFS_MAX = 50 cap with insertion-order eviction (Maps in JS
   preserve insertion order, so keys().next().value is the oldest).

API changes propagated through:
- MetadataProcessorPort.process / buildDiffDocumentForFile →
  buildDiffDocumentForEntry; both take entryId now.
- WasmApi.process(entryId, filePath, options); buildDiffDocument(entryId).
- use_process_files passes entry.id through both calls.
- WasmProcessor.process / buildDiffDocumentForEntry updated; pendingDiffs
  Map keyed by entryId; bounded eviction on insert.

Tests updated:
- wasm_processor.test.ts: all 12 process() call sites + 6
  buildDiffDocumentForEntry call sites pass an entryId; verifies
  same-entryId double-call drains correctly.
- use_process_files.test.ts: 2 spy-arg assertions and a mockImplementation
  signature updated for the new 3-arg shape.

Verification:
- yarn typecheck / yarn lint / yarn check:deps clean
- yarn test --run: 549 passed
- yarn build:web:standalone: 33.8 MB single file
- yarn test:e2e:web: 120 passed, 21 skipped (no regressions)

Refs #177.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-05-21 11:21:08 +00:00

Author

Owner

Copy link

Both yellow review findings fixed

Pushed entryId-keyed pending diffs + bounded Map size.

[55] Race on filePath key — fixed. Stash now keyed by FileEntry.id (the UUID generated in DropZone / FileBrowseButton). Each drop gets its own stash entry; re-drops of the same path can't overwrite each other.

[55] Memory leak if hook never drains — fixed. PENDING_DIFFS_MAX = 50 cap with insertion-order eviction on insert. Worst-case retention is 50 × (source + stripped) byte buffers; older entries are silently dropped (the hook layer treats "no pending entry" the same as "ExifTool failed" → legacy delta view fallback, which is correct degradation).

API rename: MetadataProcessorPort.buildDiffDocumentForFile({ filePath }) → buildDiffDocumentForEntry({ entryId }). WasmApi.process(entryId, filePath, options), WasmApi.buildDiffDocument(entryId). use_process_files passes entry.id through.

Verification (all green):

• yarn typecheck / yarn lint / yarn check:deps clean
• yarn test --run: 549 passed
• yarn build:web:standalone: 33.8 MB single file
• yarn test:e2e:web: 120 passed, 21 skipped (no regressions)

Commit: latest HEAD on feat/exiftool-diff-strategy.

## Both yellow review findings fixed Pushed `entryId`-keyed pending diffs + bounded Map size. **[55] Race on `filePath` key — fixed.** Stash now keyed by `FileEntry.id` (the UUID generated in DropZone / FileBrowseButton). Each drop gets its own stash entry; re-drops of the same path can't overwrite each other. **[55] Memory leak if hook never drains — fixed.** `PENDING_DIFFS_MAX = 50` cap with insertion-order eviction on insert. Worst-case retention is 50 × (source + stripped) byte buffers; older entries are silently dropped (the hook layer treats "no pending entry" the same as "ExifTool failed" → legacy delta view fallback, which is correct degradation). **API rename:** `MetadataProcessorPort.buildDiffDocumentForFile({ filePath })` → `buildDiffDocumentForEntry({ entryId })`. `WasmApi.process(entryId, filePath, options)`, `WasmApi.buildDiffDocument(entryId)`. `use_process_files` passes `entry.id` through. **Verification (all green):** - `yarn typecheck` / `yarn lint` / `yarn check:deps` clean - `yarn test --run`: 549 passed - `yarn build:web:standalone`: 33.8 MB single file - `yarn test:e2e:web`: 120 passed, 21 skipped (no regressions) Commit: latest HEAD on `feat/exiftool-diff-strategy`.

forgejo_admin merged commit 158de36044 into master 2026-05-21 11:27:20 +00:00

forgejo_admin referenced this pull request from a commit 2026-05-21 11:27:21 +00:00

feat(diff): ExifTool-sourced two-pane before/after metadata diff (#177)

forgejo_admin referenced this pull request from a commit 2026-05-21 11:27:38 +00:00

docs(spec): diff pivot to full before/after via ExifTool (#176)

forgejo_admin referenced this pull request from a commit 2026-05-21 11:48:03 +00:00

refactor(office): emit granular walkerEntries; drop 'Office' catchall (#178)

forgejo_admin referenced this pull request from a commit 2026-05-21 11:51:20 +00:00

refactor(strategies): delete JPEG/PNG/MP4 enumeration + IFD modules (#178)

forgejo_admin referenced this pull request 2026-05-21 12:18:16 +00:00

refactor(diff): prune legacy MetadataItem; integrate Office/PDF walker entries (chunk B.1) #179

forgejo_admin referenced this pull request 2026-05-21 12:18:50 +00:00

refactor(diff): prune legacy MetadataItem; integrate Office/PDF walker entries (chunk B.1) #179

forgejo_admin referenced this pull request 2026-05-21 12:21:17 +00:00

refactor(diff): prune legacy MetadataItem; integrate Office/PDF walker entries (chunk B.1) #179

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

e-copy

Phase E.3 — trust copy (empty-state, offline indicator)

  

e-features

Phase E.2 — feature additions (folder zip, metadata diff, Web Share Target)

  

e-mobile

Phase E.1 — mobile-critical UX (touch, picker, install prompt, size caps, messaging)

  

enhancement

New feature or request

  

f-coverage

Phase F.1 — format coverage (HEIC, future PNG/WebP/GIF/BMP/TIFF walkers)

  

f-forensic

Phase F.3 — forensic + invariant tests (no-network test, sentinel batteries)

  

f-perf

Phase F.4 — performance + privacy correctness (streaming, memory caps)

  

f-privacy

Phase F.2 — privacy-gap closure (Office Phase 2 hardening, etc.)

  

forensic

Forensic verification work — sentinel-based recovery testing per format-strategy-workflow.md

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

infra

Build/deploy/CI infrastructure work

  

invalid

This doesn't seem right

  

phase-a

Failing formats

  

phase-b

Webapp shell

  

phase-c

Testing

  

phase-d

Electron simplification

  

phase-e

Feature additions

  

phase-f

Depth and coverage hardening: format-strategy work, forensic verification, privacy correctness

  

phase-g

Electron retirement — web-only build; ships before Phase E/F feature work

  

phase-h

Deferred from Phase E — platform/mobile UX refinement

  

priority-1

Must ship first

  

priority-2

Ships second

  

priority-3

Polish and features

  

privacy

Privacy-correctness work, invariant alignment, gap documentation

  

question

Further information is requested

  

v5

Deferred to V5 — not blocking current work

  

v6

Deferred to v6 — not in v5 scope

  

video-hardening

Video metadata stripping hardening — formerly the D1.x sub-tasks

  

wontfix

This will not be worked on

No labels

bug

documentation

duplicate

e-copy

e-features

e-mobile

enhancement

f-coverage

f-forensic

f-perf

f-privacy

forensic

good first issue

help wanted

infra

invalid

phase-a

phase-b

phase-c

phase-d

phase-e

phase-f

phase-g

phase-h

priority-1

priority-2

priority-3

privacy

question

v5

v6

video-hardening

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: forgejo_admin/exifcleaner-web#177

No description provided.