fix(worker): preserve distinct image placements in cheap image de-duplication#881
fix(worker): preserve distinct image placements in cheap image de-duplication#881BigSimmo wants to merge 4 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughImage filtering now deduplicates images by hash, source kind, page number, and normalized bounding-box placement. Worker and document-enrichment flows record placement keys, with tests covering distinct, invalid, and identical placements. ChangesImage deduplication
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant uploadAndCaptionImages
participant classifyExistingImages
participant cheapImageSkipReason
participant imagePlacementDedupeKey
uploadAndCaptionImages->>cheapImageSkipReason: pass image and seenImagePlacements
classifyExistingImages->>cheapImageSkipReason: pass filterImage and seenHashes
cheapImageSkipReason->>imagePlacementDedupeKey: compute placement key
imagePlacementDedupeKey-->>cheapImageSkipReason: return placement key or null
uploadAndCaptionImages->>imagePlacementDedupeKey: record processed placement
classifyExistingImages->>imagePlacementDedupeKey: record processed placement
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 17bbb81f42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/image-filtering.test.ts (1)
33-52: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover bbox and source-kind distinctions too.
This regression only changes
pageNumber; it would still pass ifbboxorsourceKindwere accidentally removed from the dedupe key. Add same-page/different-bbox and same-page/different-source-kind cases to protect the full placement contract.🤖 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 `@tests/image-filtering.test.ts` around lines 33 - 52, Extend the image deduplication tests around imagePlacementDedupeKey and cheapImageSkipReason with same-page cases that change only bbox and only sourceKind, asserting these placements are not skipped. Keep the existing different-page case and verify the full dedupe key distinguishes pageNumber, bbox, and sourceKind.src/lib/image-filtering.ts (1)
12-12: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake the new set contract explicit.
seenHashesnow contains placement keys rather than raw image hashes. The worker and supplied tests were migrated, but any other caller can still compile while passingnew Set([imageHash]), silently disabling duplicate detection. Rename this field toseenImagePlacementsor introduce a distinct key type, and verify every caller was migrated.Also applies to: 360-360
🤖 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/lib/image-filtering.ts` at line 12, Make the placement-key contract explicit by renaming the relevant seenHashes field to seenImagePlacements in the image-filtering API, then update every caller and test to pass placement keys rather than raw image hashes. Verify no remaining references use the old name or construct the set from imageHash values.
🤖 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/lib/image-filtering.ts`:
- Around line 323-326: Update stableBboxKey so missing or malformed bounding
boxes do not all return the shared "bbox:null" key. Preserve coordinate-based
keys for valid normalized bboxes, and return a unique stable discriminator or
otherwise bypass deduplication when reliable placement metadata is unavailable.
---
Nitpick comments:
In `@src/lib/image-filtering.ts`:
- Line 12: Make the placement-key contract explicit by renaming the relevant
seenHashes field to seenImagePlacements in the image-filtering API, then update
every caller and test to pass placement keys rather than raw image hashes.
Verify no remaining references use the old name or construct the set from
imageHash values.
In `@tests/image-filtering.test.ts`:
- Around line 33-52: Extend the image deduplication tests around
imagePlacementDedupeKey and cheapImageSkipReason with same-page cases that
change only bbox and only sourceKind, asserting these placements are not
skipped. Keep the existing different-page case and verify the full dedupe key
distinguishes pageNumber, bbox, and sourceKind.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 47d5f741-ea7b-4924-8eb2-6a4cf67af0c5
📒 Files selected for processing (3)
src/lib/image-filtering.tstests/image-filtering.test.tsworker/main.ts
Update enrich-documents to use imagePlacementDedupeKey with pageNumber, and skip cheap placement de-dupe when bbox is missing or malformed so distinct unknown placements are not collapsed into one shared key. Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
CI triageCI failed on this PR. Automated classification of the 2 failed job(s):
Compared with main CI run #3362 (cancelled). Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger. |
Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
|
Closing as superseded: identical change already merged in #888 (same title and files: scripts/enrich-documents.ts, src/lib/image-filtering.ts, tests/image-filtering.test.ts, worker/main.ts). |
2 participants
Summary
src/lib/image-filtering.tsand switch worker tracking inworker/main.tsto skip only same-placement duplicates.Verification
npm run test -- --run tests/image-filtering.test.ts tests/worker-visual-capture.test.tsnpm run verify:cheap(passed focused worker/image coverage; unrelated PDF extraction budget failures are pre-existing local env issues)npm run verify:pr-localnot re-run in this metadata repair pass; CI Static/Unit/Build gates apply on this PR.Risk and rollout
Notes