Dedupe re-imported venues by content fingerprint (#456)#519
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes cross-import venue duplication in the additive CompendiumArchiveImporter by introducing a conservative, never-overwrite dedupe mechanism based on a normalized content fingerprint, matching the #456 spec.
Changes:
- Added a pure
venueFingerprint(Venue) -> String?helper plusVenueFingerprintIndexto dedupe venues by strong descriptive keys (name + city/address1), while avoiding false merges. - Updated
CompendiumArchiveImporterto repoint programs to an existing matched venue (no venue overwrites; ambiguous/weak keys still fresh-mint). - Added/extended core tests to cover fingerprint normalization, strong-key thresholding, ambiguity handling, and importer behavior (including undo semantics and query-count invariants).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compendium_core/lib/src/imports/venue_dedupe.dart | New fingerprint + in-memory index used to match incoming venues to existing ones safely. |
| packages/compendium_core/lib/src/imports/compendium_archive_import.dart | Integrates fingerprint index into venue import flow; ensures “repoint, never overwrite,” and avoids N+1 venue existence reads. |
| packages/compendium_core/lib/compendium_core.dart | Exports the new venue dedupe API from the core package. |
| packages/compendium_core/test/imports/venue_dedupe_test.dart | New unit tests for fingerprint normalization/threshold and index match/ambiguity behavior. |
| packages/compendium_core/test/imports/compendium_archive_import_test.dart | Expanded integration tests covering #456 regression, weak/strong-key behavior, ambiguity, within-bundle collapse, undo safety, and query-count expectation changes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ibanner56
added a commit
that referenced
this pull request
Jul 22, 2026
Addresses Copilot review on #519. VenueFingerprintIndex.add() left the prior fingerprint->id mapping in place when the same venue id was re-added with different descriptive fields. In the importer this happens on the "last-seen wins" collapse of a repeated original venue id within one (untrusted) bundle: the minted row is refreshed to the last-seen content and re-added under a new fingerprint, but the old fingerprint kept pointing at that id. A later distinct venue matching the stale fingerprint could then be wrongly repointed at the now-changed venue. Fix: track each id's current fingerprint (_fingerprintById) and retire the stale mapping before indexing the new one ("last-seen wins"). Ambiguous (poisoned) fingerprints stay poisoned — consistent with the never-false-merge stance. Repoint-never-overwrite and the untrusted-bundle guarantee are unchanged. Tests: add index unit tests for changed-fingerprint re-add (stale key retired, current key matches), a distinct venue not repointed via a retired key, and a strong->weak re-add clearing the stale key. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
The additive CompendiumArchiveImporter minted a fresh venue id for every bundled venue, so re-importing the same bundle inserted duplicate venue records and orphaned the previously-imported ones. Add a cross-import dedupe primitive that matches an incoming venue to one the receiver already holds by a normalized content fingerprint (name + city + stateProv + address1), then repoints the program at the existing venue instead of minting a duplicate. The match is strictly a repoint, never an overwrite: the matched venue's fields are left untouched and it is never added to insertedVenueIds, so the fresh-mint guarantee (an untrusted bundle cannot mutate/hijack a local venue by claiming its identity) is preserved and undo never deletes a venue the user already had. A weakly-described venue (name only) or an ambiguous match (>1 existing venue shares the fingerprint) always fresh-mints, never a wrong guess. The fingerprint excludes the redactable contact fields so a #515-redacted shared venue still matches. No schema or archive-format change; replace-mode restore is untouched. compendium_core stays Flutter-free. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Addresses Copilot review on #519. VenueFingerprintIndex.add() left the prior fingerprint->id mapping in place when the same venue id was re-added with different descriptive fields. In the importer this happens on the "last-seen wins" collapse of a repeated original venue id within one (untrusted) bundle: the minted row is refreshed to the last-seen content and re-added under a new fingerprint, but the old fingerprint kept pointing at that id. A later distinct venue matching the stale fingerprint could then be wrongly repointed at the now-changed venue. Fix: track each id's current fingerprint (_fingerprintById) and retire the stale mapping before indexing the new one ("last-seen wins"). Ambiguous (poisoned) fingerprints stay poisoned — consistent with the never-false-merge stance. Repoint-never-overwrite and the untrusted-bundle guarantee are unchanged. Tests: add index unit tests for changed-fingerprint re-add (stale key retired, current key matches), a distinct venue not repointed via a retired key, and a strong->weak re-add clearing the stale key. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
ibanner56
force-pushed
the
isaacbanner-venue-import-dedupe-456
branch
from
July 22, 2026 23:27
b317a39 to
7ac3f65
Compare
Addresses Copilot re-review on #519 (two findings): 1. venueFingerprint separator hardening. The `\u0000` field separator was only guaranteed absent because the import path sanitizes text on decode, but a locally-created/edited venue never passes through that sanitizer, so a stored venue field could carry an embedded NUL (the separator itself) or a bidi/invisible control — leaking a control char into the key and risking a false merge. `_normalizeField` now runs each field through `sanitizeImportedText` (the same C0/C1/DEL + bidi + invisible strip used on import) so the separator invariant and never-false-merge intent hold for ALL stored venues, not just imported ones. 2. Within-bundle refresh scope. A repeat of an original venue id that had *deduped* onto another original id's freshly-minted venue previously entered the "last-seen wins" refresh branch (the check keyed off the target being minted, not off this id having minted it), letting one id's later occurrence overwrite a minted row other ids were repointed to — a non-deterministic clobber for a malformed/untrusted bundle. Now only an id whose first occurrence actually minted a venue (`mintingOriginalIds`) may refresh its row; a repeat of a deduped id is a no-op. Replaces the `insertedVenueIdSet` target-check. Tests: venue_dedupe_test gains a control/bidi-stripping fingerprint test; importer tests gain "a repeated deduped id never overwrites another id's minted venue" and "a repeated minting id still refreshes to last-seen content". Full core suite green (2200 passed). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Addresses Copilot re-review on #519. The fingerprint-index preload (`_venues.listAll()`) ran whenever the bundle carried any venues, even when every bundled venue was below the strong-key threshold (null fingerprint) and so could never dedupe cross-import — making the SELECT pure waste. Now the preload runs only when at least one bundled venue produces a non-null fingerprint (`archive.venues.any(...)`); a bundle with no venues, or only weakly-described ones, issues zero venue reads. Behavior is otherwise unchanged: an all-weak bundle already fresh-mints every venue, and an empty index yields the same result. Tests: the query-counter test now uses strong-key venues (so it still asserts the preload is a single SELECT for N programs — the no-N+1 guarantee); a new counter test asserts a weak-key-only bundle issues zero venue SELECTs. Full core suite green (2201 passed). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
Closes #456
Problem
The additive
CompendiumArchiveImporterminted a fresh venue id for every bundled venue, so re-importing the same bundle inserted duplicate venue records and orphaned the previously-imported ones (the provenance-deduped program was repointed to the newly-minted venue). Fresh-mint exists deliberately so an untrusted bundle can never overwrite/hijack an existing local venue — any dedupe primitive had to preserve that.Approach (maintainer-resolved spec: content fingerprint, never-overwrite, strong-key-only)
venueFingerprint(Venue) -> String?andVenueFingerprintIndex(imports/venue_dedupe.dart). The fingerprint normalizesname + city + stateProv + address1(trim, lowercase, collapse whitespace).nameand (address1orcity) are present; a name-only venue is too weak to match on and always fresh-mints (no false merge).Security (untrusted-bundle guarantee preserved)
insertedVenueIds, soundo/compensation never delete a venue the user already had.contact1/2*),notes,price,website,schedule, so a Share-bundle venue gathering with contact-PII redaction + opt-in consent dialog #515-redacted shared venue still matches a local one.Scope / non-goals
compendium_corestays Flutter-free.Tests
test/imports/venue_dedupe_test.dart: fingerprint normalization, strong-key threshold, separator-collision safety, index matching / ambiguity poisoning / fold-in.compendium_archive_import_test.dart: re-import strong-key venue gives 0 new venues + program repointed (the Venue re-import dedupe / provenance key (cross-import) #456 regression); weak-key still duplicates; never-overwrite of matched fields; ambiguous fresh-mints; two fingerprint-equal venues in one bundle collapse to one insert;undoafter dedupe deletes only newly-minted venues.Gates
dart format(0 changed),dart analyze(core + app clean), fullcompendium_coresuite green (2194 passed). Pre-existing, unrelated app failure:test/l10n/no_hardcoded_ui_strings_test.dart(flags shorthand-mapping screens untouched by this PR; identical onmain).