Skip to content

feat(core): add Venue entity, storage, migration v14 (PR A — base of stack)#452

Merged
ibanner56 merged 16 commits into
mainfrom
isaacbanner-venue-entity-core
Jul 22, 2026
Merged

feat(core): add Venue entity, storage, migration v14 (PR A — base of stack)#452
ibanner56 merged 16 commits into
mainfrom
isaacbanner-venue-entity-core

Conversation

@ibanner56

@ibanner56 ibanner56 commented Jul 21, 2026

Copy link
Copy Markdown
Owner

PR A — Venue entity core (base of a stacked set)

Adds a first-class Venue entity to CallersCompendium so many programs can be held at the same venue, with address/contact/schedule edited in one place. This is Phase 1 of the venue rollout and the base of a stacked PR set:

  • PR A (this one): core model, storage, schema migration, repository, serialization, tests.
  • PR B (later): settings toggle + editor/picker UI.
  • PR C (later): on-screen display labels, export wiring, and docs.

It mirrors the existing PublishedSource vertical slice end-to-end (model → table → migration → repository → serialization → tests).

Schema version note: originally authored as a 12 → 13 migration, this was rebased to 13 → 14 because #455 landed its own schema v13 (a dance_links performance index) on main first. The venue migration now applies additively on top of that as v14; the implementation, onUpgrade ladder, migration test, and fixtures all reflect v14.

What's included

  • model/venue.dart@immutable Venue: required id + name, 20 nullable normalized strings (address1/2, city, stateProv, country, postalCode, plus4, website, sponsor, eventName, time, genericSchedule, price, notes, contact1/2 name/phone/email). _normalize (empty/whitespace → null), copyWith with clear* flags, ==/hashCode, and a computed displayName getter (the app-side equivalent of CC's VenueDisplay_c — computed, never stored). FileMaker plumbing (zc_*/zi_*, SiteID, numeric zk_VenueID) is dropped; the app mints its own uuid PK.
  • tables.dart — new Venues table (VenueRow) + a nullable programs.venue_id soft reference (no DB foreign key).
  • database.dart — schema bump 13 → 14 with a documented, purely-additive onUpgrade step (createTable(venues) + addColumn(programs.venueId) + a programs_venue_id lookup index); no data back-fill and no derived-index rebuild (venues don't feed dance_fts/dance_figures).
  • venue_repository.dartupsert / getById / listAll (by name, case-insensitive) / delete guarded (transactional COUNT, index-backed) to throw while any program still references the venue. Registered in the repositories facade.
  • program.dart / program_repository.dartvenueId carried through the model, copyWith (venueId + clearVenueId), duplicate(), equality, and persistence. A non-null venueId is validated against an existing venue inside the write transaction (single writes) or a preloaded id set (batch restore/import), so the repo can't persist a dangling reference.
  • Serialization (archive_codec.dart / archive_service.dart / imports/compendium_archive_import.dart) — programs export/import venueId; bundles embed a top-level venues array (omitted when empty, tolerated when absent for older bundles). Both the full-DB backup/restore path and the community-import path upsert venues before programs (with id remapping on import) and safely null a dangling venueId (venue absent from both bundle and DB) rather than leaving it orphaned. A venue-bearing archive is stamped envelope v2 so pre-venue readers surface the "newer than supported" warning instead of silently dropping venue data; venue-less archives stay v1-compatible.

Security (OWASP)

archive_codec import treats shared/community bundles as untrusted: the venues array must be a list of objects; every field is type-checked; missing/extra fields are tolerated; malformed entries are rejected per-entity (recorded as a structured error, the rest still load) rather than crashing the import. A blank venue name is rejected as a FormatException so it's caught by the per-entity decode guard instead of escaping as an uncaught Error. Within a single crafted bundle, duplicate venue ids collapse to one (last-seen wins), mirroring duplicate-program handling.

Tests (Phase-5, scoped to the above)

  • test/model/venue_test.dart — validation, normalize, copyWith clear-flags, displayName.
  • test/storage/venue_repository_test.dart — CRUD + delete-guard.
  • Migration: checked-in test/storage/fixtures/v13.sqlite (+ re-runnable generate_v13_fixture.dart) and a v13 → v14 group in migration_test.dart asserting the venues table, programs.venue_id column, and programs_venue_id index appear and user_version == 14 (plus a fresh-schema check that the index exists on onCreate).
  • Serialization/import round-trip: a program with venueId + an embedded venue survives export→import (both backup/restore and community-import paths); a legacy bundle with no venues array still imports cleanly; a dangling venueId is nulled; undo removes imported venues (guarded); malformed/oversized/mistyped venue input is rejected per-entity.

Release note

The schema bump 13 → 14 is additive and rides a MINOR release. No UI writes venueId yet (that's Phase 3); the codec and tests exercise it programmatically. Follow-ups tracked in #456 (share-bundle venue gathering + re-import dedupe key).

Verification

build_runner regen committed; dart analyze clean (core + app); targeted + full compendium_core suite green; CI schema-migration gate satisfied (13 → 14 with fixture/test evidence).

Please do not merge — holding for human review.

Introduce a first-class, reusable Venue entity so many programs can be held
at the same venue with address/contact/schedule edited in one place. Mirrors
the PublishedSource vertical slice.

- model/venue.dart: @immutable Venue (id + required name, 20 nullable
  normalized strings), _normalize, copyWith clear-flags, ==/hashCode, and a
  computed displayName getter (app-side equivalent of CC's VenueDisplay_c).
- tables.dart: new Venues table (VenueRow) + nullable soft-reference
  programs.venue_id (no FK — integrity guarded at the app layer).
- database.dart: bump schema 12 -> 13 with an additive onUpgrade step
  (createTable(venues) + addColumn(programs.venueId)); no derived rebuild.
- venue_repository.dart: upsert/getById/listAll(name, noCase)/delete with a
  guard that throws while any program still references the venue. Registered
  in the repositories facade.
- program(.dart/_repository): carry venueId through the model, copyWith,
  duplicate, equality and persistence.
- serialization: export/import program.venueId and embed a top-level venues
  array (omitted when empty, tolerated when absent for older bundles); upsert
  venues before programs and null a dangling venueId on import. OWASP-aligned
  validation for untrusted community bundles.
- tests: venue model + repository (CRUD + delete-guard), v12 fixture +
  generator, v12 -> v13 migration group, and archive round-trip/legacy/
  malformed-input coverage.

Base of a stacked set (B: settings + editor/picker UI; C: display/export +
docs). The 12 -> 13 bump is additive and rides a MINOR release.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces the core Venue entity and integrates it with programs, persistence, migrations, and archive serialization.

Changes:

  • Adds Venue modeling, storage, repository APIs, and program references.
  • Migrates databases from schema v12 to v13.
  • Adds archive import/export support and comprehensive tests.

Reviewed changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/compendium_core/lib/compendium_core.dart Exports Venue APIs.
packages/compendium_core/lib/src/model/program.dart Adds venueId support.
packages/compendium_core/lib/src/model/venue.dart Defines the Venue model.
packages/compendium_core/lib/src/serialization/archive_codec.dart Serializes venues and links.
packages/compendium_core/lib/src/serialization/archive_service.dart Exports and restores venues.
packages/compendium_core/lib/src/serialization/compendium_archive.dart Adds venues to archives.
packages/compendium_core/lib/src/storage/database.dart Adds schema-v13 migration.
packages/compendium_core/lib/src/storage/database.g.dart Regenerates Drift bindings.
packages/compendium_core/lib/src/storage/repositories/program_repository.dart Persists program venue links.
packages/compendium_core/lib/src/storage/repositories/repositories.dart Registers VenueRepository.
packages/compendium_core/lib/src/storage/repositories/venue_repository.dart Implements Venue CRUD.
packages/compendium_core/lib/src/storage/tables.dart Defines venue storage schema.
packages/compendium_core/test/model/venue_test.dart Tests Venue behavior.
packages/compendium_core/test/serialization/archive_codec_test.dart Tests venue JSON handling.
packages/compendium_core/test/serialization/archive_service_test.dart Tests venue restoration.
packages/compendium_core/test/storage/fixtures/generate_v12_fixture.dart Generates the migration fixture.
packages/compendium_core/test/storage/fixtures/v12.sqlite Provides a v12 database fixture.
packages/compendium_core/test/storage/migration_test.dart Tests v12-to-v13 migration.
packages/compendium_core/test/storage/venue_repository_test.dart Tests Venue persistence and deletion.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/compendium_core/lib/src/serialization/archive_codec.dart Outdated
Comment thread packages/compendium_core/lib/src/serialization/archive_codec.dart
…te-time integrity

Addresses the three Copilot review comments on #452:

1. Archive schema version (archive_codec.dart): introduce
   archiveSchemaVersionVenues (v2) and stamp venue-bearing archives at v2
   (content-derived via requiredSchemaVersion), keeping venue-less archives at
   v1 so pre-venue readers still accept them. An old reader now emits the
   "newer than supported" warning instead of silently dropping venues/venueId.

2. Community-intake importer (CompendiumArchiveImporter): upsert archive.venues
   under freshly-minted ids (never clobbering an existing venue) BEFORE
   programs, remap program.venueId to the inserted venue, null a dangling
   reference (OWASP-safe, surfaced as an ImportIssue), and record inserted
   venue ids for undo/rollback. Wire the venue repo through ArchiveIntakeService.

3. Soft-ref integrity (program_repository.dart / venue_repository.dart):
   ProgramRepository rejects a write whose non-null venueId references no venue,
   checked inside the write transaction; VenueRepository.delete is now atomic
   (guard check + delete in one transaction). Both import paths resolve-or-null
   a dangling venueId before persisting so they never trip the write-time check.

Adds importer venue-wiring tests, program venueId write-time tests, and archive
schema-version compatibility tests. Regenerates database.g.dart (propagated
venueId doc comment). Leaves a tracked TODO that per-program share-bundle venue
gathering is deferred to PR C.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Venues carry no provenance/dedupe key, so re-importing the same bundle inserts
duplicate venue records rather than matching previously-imported ones. This is
accepted for PR A (consistent with the additive-import model) and deferred to a
later PR. Records the limitation as a TODO in CompendiumArchiveImporter.commit
(where the behavior lives) and alongside the existing deferred-PR-C venue TODO
in program_share_bundle.dart so it stays on the radar. Comment-only; no behavior
change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 2 comments.

Comment thread packages/compendium_core/lib/src/imports/compendium_archive_import.dart Outdated
Comment thread packages/compendium_core/lib/src/imports/compendium_archive_import.dart Outdated
ibanner56 and others added 2 commits July 21, 2026 22:27
…k follow-ups

Addresses the second Copilot review pass on #452:

- Undo dangling-ref (importer): CompendiumArchiveImporter.undo previously
  hard-deleted inserted venues, bypassing the delete-guard. After a successful
  import a surviving user program can link to an imported venue, so an
  unconditional delete would orphan that program's venueId. Undo now mirrors
  ImportPipeline.undo's created-choreographer handling: guarded per-id delete
  (retaining any venue a surviving program still references), inserted programs
  removed first. The failed-commit compensation path keeps hardDelete (safe —
  the freshly-minted venues can only be referenced by this commit's own
  rolled-back programs).

- In-bundle duplicate venue ids (OWASP untrusted-input hardening): a crafted
  bundle repeating an original venue id previously orphaned an extra minted row
  via last-wins remap. It now collapses to a single minted venue (last-seen
  content wins, mirroring duplicate-program-id handling) and is counted once.

- Follow-up tracking: the deferred cross-import venue dedupe/provenance-key
  limitation and the PR C share-bundle venue-gathering TODOs now reference the
  dedicated open issue #456 instead of the closed, unrelated #298 (whose
  historical share-feature references are left intact as origin context).

Cross-import venue dedupe/provenance stays deferred (tracked in #456), per the
additive-import model. Adds importer tests for guarded-undo retention and
in-bundle dup-id collapse. Comment/logic changes only in the importer; no schema
change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extends the importer venue-wiring tests (per the Copilot review ask to assert
venue counts on re-import): re-importing a venue-bearing bundle dedupes the
program by provenance but accumulates venue rows, since cross-import venue
dedupe/provenance is deliberately deferred (tracked in #456). Pins the accepted
PR A additive behavior so a future dedupe change is a conscious update.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.

Comment thread packages/compendium_core/lib/src/serialization/archive_service.dart Outdated
ibanner56 and others added 2 commits July 21, 2026 22:50
Earlier PR A commits added venue model/tests that were not run through
`dart format`, so the CI `dart format --set-exit-if-changed` gate was
red. Reformat the affected venue files to the tall style the pinned
Flutter 3.44.6 toolchain enforces. No logic changes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…estore

Copilot review flagged an N+1 on the archive restore path: each
venue-linked program did a `getById` to resolve-or-null its venueId AND
then the repository ran a second venue-existence SELECT inside the write,
so restoring N linked programs issued 2N venue queries. The community
importer had the analogous per-program write-time SELECT (N queries).

Fix, preserving the approved soft-ref + app-layer integrity design:
- VenueRepository.listAllIds(): one lean SELECT of the id set.
- ProgramRepository.create/update: optional `knownVenueIds` — when the
  bulk callers supply a preloaded set, the write-time venueId guard
  validates in-memory instead of a per-row SELECT. The single-write path
  (null set) keeps the strict in-transaction SELECT so a normal save
  still cannot race a concurrent venue delete. The integrity guarantee is
  identical either way: an unknown venueId throws.
- ArchiveRestorer: preload the venue-id set once for the whole programs
  phase; resolve-or-null and the write guard share that snapshot (2N -> 1).
- CompendiumArchiveImporter: pass the freshly-minted venue-id set to
  create/update (N -> 0 extra); built programs only ever reference minted
  venues, so the in-memory check is exact.

Sound because both bulk paths only insert venues (never delete one)
mid-batch. Adds VenueSelectCounter + query-count invariant tests pinning
one venue SELECT on restore and zero on import, mirroring the existing
slot-loader N+1 guard.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:175

  • knownVenueIds is caller-supplied through the public create/update API, so passing {'ghost'} currently bypasses the only database existence check and persists a dangling soft reference. A previously valid set can likewise become stale outside the restore transaction. Keep the optimization behind a repository-owned, transaction-scoped batch API (or otherwise make the validated set unforgeable) so public writes cannot opt out of referential integrity.
      final venueExists = knownVenueIds != null
          ? knownVenueIds.contains(venueId)
          : await (_db.select(_db.venues)

Comment thread packages/compendium_core/lib/src/storage/repositories/venue_repository.dart Outdated
Read a scalar COUNT(id) inside the delete transaction instead of
materializing every referencing ProgramRow just to check emptiness.
A venue is reusable across many programs, so the guard cost now stays
flat regardless of how large a popular venue's referencing history
grows. Integrity is unchanged: a still-referenced venue still throws a
StateError inside the same transaction (no check-then-act race).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:174

  • knownVenueIds bypasses the in-transaction existence check, so the soft-reference guarantee is no longer atomic on the bulk paths. For example, after the importer builds this set, VenueRepository.delete can delete an as-yet-unreferenced venue before a later program write; this branch still returns true and persists a dangling venue_id. Any caller can also pass a set containing an ID that never existed. Keep the preload and all program writes in one database transaction, or expose a repository-owned bulk operation that validates the snapshot within the same transaction instead of accepting an arbitrary public set.
      final venueExists = knownVenueIds != null
          ? knownVenueIds.contains(venueId)

Without an index, VenueRepository.delete's COUNT(id) reference guard
full-scans the whole programs table (O(total programs)) on every
guarded delete, since a venue is reusable across many programs. Add a
plain `programs_venue_id` lookup index so SQLite restricts the count to
matching references. Created via a raw CREATE INDEX IF NOT EXISTS
(mirroring the existing dance_figures search index) in both onCreate
(fresh databases) and the from < 13 upgrade step (migrated databases),
so both paths match. Same schema version (13) — additive, no version
bump. Migration tests assert the index exists on both fresh and
upgraded databases.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:175

  • knownVenueIds is a caller-controlled existence assertion: create(programWithVenue('ghost'), knownVenueIds: {'ghost'}) now persists a dangling reference even when venues has no such row. It can also become stale in CompendiumArchiveImporter, whose venue inserts and program writes are separate transactions, if a guarded delete interleaves between them. Keep the preload and writes inside one repository-owned transaction/bulk API rather than letting the public create/update methods bypass the database check with an arbitrary set.
      final venueExists = knownVenueIds != null
          ? knownVenueIds.contains(venueId)
          : await (_db.select(_db.venues)

Adding the programs_venue_id index (previous commit) means SQLite now
refuses `ALTER TABLE programs DROP COLUMN venue_id` while that index
still references the column, breaking the documented fixture
regeneration command. Drop the index before the column in the strip-to-
v12 step and document the ordering. Regenerated v12.sqlite with the
fixed generator; the v12 -> v13 migration group still passes (the
fixture is still a valid v12 shape: no venues table, no venue_id, no
index, user_version = 12).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@isaacbanner
isaacbanner requested a review from Copilot July 22, 2026 06:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 24 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:174

  • knownVenueIds is caller-controlled but is treated as proof that the row exists, so create(programWithGhost, knownVenueIds: {'ghost'}) persists a dangling reference and bypasses the repository's advertised integrity guarantee. The community importer also computes this set outside an enclosing transaction, allowing a concurrent venue delete between minting and program writes. Keep this optimization behind a repository-owned bulk operation that loads/validates the IDs and performs all writes in one transaction, rather than exposing an unchecked set on each public write.
      final venueExists = knownVenueIds != null
          ? knownVenueIds.contains(venueId)

Comment thread packages/compendium_core/lib/src/model/program.dart
CallersCompanionUsrImporter._rebuildProgramWithId reconstructs a
provenance-matched program from fresh .USR data on re-import, keeping
only the prior program's id and createdAt. venueId is an app-local
entity link a .USR archive can never supply, so the rebuild was
implicitly nulling it — silently dropping a venue link the user
established after the original import. Carry the prior program's
venueId forward (like id/createdAt) so re-import no longer destroys
app-local state the source cannot reconstruct. Mirrors the venueId
preservation already wired into the archive importer. +regression test.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:175

  • knownVenueIds bypasses the in-transaction existence read, but a preloaded set does not lock those rows. CompendiumArchiveImporter.commit inserts venues and programs in separate transactions, so VenueRepository.delete can remove a newly inserted venue after this set is built and before a program write; this branch then accepts the stale ID and persists a dangling soft reference. The same public parameter also lets any caller provide IDs that never existed. Please keep the venue/program bulk phase in one database transaction (as ArchiveRestorer does), or use a batch API whose snapshot and writes share that transaction rather than trusting an arbitrary set.
      final venueExists = knownVenueIds != null
          ? knownVenueIds.contains(venueId)
          : await (_db.select(_db.venues)

main's #455 (perf: batch DanceRepository child hydration) landed
kCompendiumSchemaVersion = 13 for a performance-only dance_links(dance_id)
index, colliding with this branch's v13 venue entity. Resolve by rebasing the
venue migration to v14: keep main's `if (from < 13)` dance_links step and its
v12 fixture / v12 -> v13 migration group verbatim, and move the venue table +
programs.venue_id column + programs_venue_id index to a new `if (from < 14)`
step. Both index consts and both onCreate applications are unioned; the
schema-history doc gains a v13 (dance_links) block and a v14 (venue) block in
chronological order.

Regenerate the venue migration fixture as v13.sqlite (via new
generate_v13_fixture.dart) — the v13 shape retains main's dance_links_dance_id
index and strips only the v14 venue additions. Re-target the venue migration
test group to v13 -> v14. test_database.dart keeps both DanceChildSelectCounter
(main) and VenueSelectCounter (this branch).

kCompendiumSchemaVersion is now 14. database.g.dart regenerated (no diff).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 25 out of 27 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:174

  • The bulk fast path no longer participates in the database existence check. CompendiumArchiveImporter commits each venue before later program writes, so a concurrent guarded delete can remove a just-added venue while it has no references; this stale set still returns true and the program is then persisted with a dangling venueId. A caller can also supply an arbitrary set directly. Keep the venue preload and all dependent writes in one database transaction (or use a restrictive FK/batch repository API) so the advertised app-layer integrity guarantee remains atomic.
      final venueExists = knownVenueIds != null
          ? knownVenueIds.contains(venueId)

Comment thread packages/compendium_core/lib/src/storage/database.dart
Main's #455 landed its own schema v13 (the dance_links dance_id index)
before this PR merged, so the venue entity rebased from v13 to v14. The
implementation, migration ladder, and tests already use v14 correctly, but
several venue-related API doc comments still said "schema v13". Update them
(and the regenerated database.g.dart mirror comment) so the documented
introduction version matches the shipped database format. Comment-only; no
runtime behavior change.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@isaacbanner isaacbanner changed the title feat(core): add Venue entity, storage, migration v13 (PR A — base of stack) feat(core): add Venue entity, storage, migration v14 (PR A — base of stack) Jul 22, 2026
The v12 fixture generator seeds a fresh database at the current schema
(v14) and then strips back to the v12 shape. It only removed v13's
`dance_links_dance_id` index, so re-running it against the v14 schema left
v14's venue additions behind — the regenerated `v12.sqlite` kept the
`venues` table, the `programs.venue_id` column, and the `programs_venue_id`
index. That broken fixture would fail the `v12 -> v13` migration test
(`createTable(venues)` throws "table venues already exists" during the
`from < 14` step). The committed fixture was only correct because it
predated the venue entity; the generator could no longer reproduce it.

Strip the v14 venue additions too, mirroring `generate_v13_fixture.dart`:
DROP TABLE venues, then DROP INDEX programs_venue_id BEFORE
ALTER TABLE programs DROP COLUMN venue_id (SQLite refuses to drop a column
an index still references), plus the v13 dance_links index, then stamp
user_version = 12. Regenerated `v12.sqlite` accordingly — schema is
identical to the prior fixture (byte layout differs only by the freelist
pages the DROP TABLE/COLUMN leave behind, matching how v13.sqlite is built).

Verified: v12 -> v13 + v13 -> v14 migration groups pass against the
regenerated fixture, full core suite green, analyze clean, format gate
clean, build_runner no drift, schema gate 13 -> 14.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 29 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:147

  • knownVenueIds bypasses the repository's existence check rather than merely batching it: any caller can pass {'ghost'} and persist venueId == 'ghost' even when no such row exists. Since ProgramRepository is exported, this invalidates the documented app-layer integrity guarantee. Keep the trusted ID snapshot inside a repository-owned bulk operation/transaction instead of exposing it on the public single-row API.
  Future<void> create(Program program, {Set<String>? knownVenueIds}) =>
      _upsert(program, knownVenueIds: knownVenueIds);

  /// Updates an existing program. See [create] for [knownVenueIds].
  Future<void> update(Program program, {Set<String>? knownVenueIds}) =>
      _upsert(program, knownVenueIds: knownVenueIds);

Comment thread packages/compendium_core/lib/src/imports/compendium_archive_import.dart Outdated
Comment thread app/lib/src/export/program_share_bundle.dart Outdated
Copilot review (07:51Z) flagged two items on the community archive
re-import path:

1. compendium_archive_import.dart — data-loss defect: on a
   provenance-matched re-import, _rebuildProgramWithId carried
   `venueId: src.venueId` verbatim. A pre-venue (venue-unaware) archive
   cannot express venueId, so its rebuilt program is necessarily null;
   overwriting with it silently dropped a venue link the user had
   established locally after the original import.

   Fix: condition the overwrite on whether the archive is venue-aware,
   keyed on requiredSchemaVersion(archive) (the archive's actual venue
   content, matching what the encoder stamps the wire version from). A
   venue-aware archive keeps its explicit semantics (remapped-or-nulled
   venueId overwrites, so an explicit null still clears); a pre-venue
   archive preserves the matched program's prior venueId, mirroring the
   .USR re-import precedent. Admit a preserved prior venueId into the
   per-write knownVenueIds set so the bulk-import in-memory validation
   accepts it (the venue delete-guard keeps a referenced venue alive and
   this commit only inserts venues, so the reference is sound).

   +2 tests: pre-venue re-import preserves a user-linked venueId; a
   venue-aware re-import honors an explicit cleared venueId.

2. program_share_bundle.dart — comment-only: stale "schema v13" venueId
   reference corrected to v14 (post-#455-rebase version).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 26 out of 29 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

packages/compendium_core/lib/src/storage/repositories/program_repository.dart:174

  • knownVenueIds is a public caller-supplied set, so it bypasses the repository's existence check rather than optimizing it: create(program, knownVenueIds: {'ghost'}) persists a dangling soft reference. A preloaded set can also become stale unless its load and all writes share one transaction (the community importer does not). Keep this bypass internal to a repository-owned batch operation that preloads and writes in one transaction, or enforce the relationship with a database constraint; otherwise the documented app-layer integrity guarantee is not upheld.
      final venueExists = knownVenueIds != null
          ? knownVenueIds.contains(venueId)

packages/compendium_core/lib/src/imports/compendium_archive_import.dart:390

  • This ignores the archive's declared wire version. A valid v2 archive with an empty venues list and null venueId values is classified as v1 because requiredSchemaVersion only inspects content; on a provenance-matched re-import, the receiver's old venue link is then preserved instead of honoring v2's explicit null. Include archive.schemaVersion >= archiveSchemaVersionVenues in this capability check (while retaining the content check for programmatically constructed archives).
      final archiveCarriesVenueLinks =
          requiredSchemaVersion(archive) >= archiveSchemaVersionVenues;

Comprehensive venueId-preservation audit (every Program construct/
rewrite/copy/duplicate/persist/read/serialize site) found the production
paths already correct, but two model invariants lacked an explicit
regression guard:

- Program.duplicate() carries venueId to the clone (same event, same
  venue) — extend the existing "carries the new fields through" test.
- Program.stampDanceSlotsPerformed() preserves venueId (it rebuilds via
  copyWith) — new test guarding a future refactor to a direct Program(...)
  construction that could forget the field.

Test-only; no production change (the audit found no gaps to fix).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@ibanner56
ibanner56 merged commit a77501e into main Jul 22, 2026
2 checks passed
@ibanner56
ibanner56 deleted the isaacbanner-venue-entity-core branch July 22, 2026 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants