fix(api): enforce unique og_field_memberships (resource_id, source_pk) by AlexAxthelm · Pull Request #322 · RMI/stitch

AlexAxthelm · 2026-10-09T08:27:12Z

Add uniqueness constraint to membership table (non-existence noted while working through #279 / #290). SO far nothing wrong with the data, but it was theoretically possible to attach a source to the same resource multiple times, which is logically inconsistent with how we're designing stitch.

As a convenient side effect, the constraint alleviates the need for an explicit index on the same columns, which would have been in this PR, but I would say is no longer worth it (see timing notes below in Claude's writup)

fix(api): enforce unique og_field_memberships (resource_id, source_pk)

Why

Nothing stopped the same source record from being attached to the same resource more than once. Re-attaching an already-attached source created a second membership. Merging two resources that shared a source copied both memberships onto the merged resource. The #279 design review called the missing constraint a "major oversight".

This is independent of the STIT-766 read-model work (#290). It turned up during that work and its load testing. Landing it first gives #290 a sound base.

What changed

Constraint: UNIQUE (resource_id, source_pk) on og_field_memberships (uq_membership_resource_source). Uniqueness is per (resource, source); a source record can still belong to several different resources.
Migration 901d261fce8b (follows 71aa7ef8150b):
- Adds the constraint, via batch_alter_table so it also runs on SQLite (as 790b2078d706 does). PostgreSQL gets a plain ALTER TABLE.
- No dedup step: production has no duplicate memberships. If a duplicate ever exists, the constraint add fails and the migration rolls back without deleting anything.
- Irreversible (downgrade raises), per repo convention.
Attach (_attach_source_models): skips sources already attached to the resource (in any status) and repeats within one batch.
Merge (_repoint_memberships): the merged resource gets one membership per source record. It's ACTIVE if any parent's membership was. Parents' ACTIVE memberships are still set INACTIVE.
Corrected the apply_resource_merge and _repoint_memberships docstrings. They described ids[0] as the merge target, said memberships weren't consolidated, and named parameters that don't exist.

No API or response-format change.

Why there's no separate `(resource_id, status)` index

Load testing on #290 suggested adding ix_membership_resource_active (resource_id, status), because og_field_memberships only had its primary-key index. That forced full table scans on two reads: the detail endpoint's source_data query and the coalescing query that hydrates a page of the list. The unique constraint in this PR creates its own index, and it also starts with resource_id. Measured on a 50K-resource dataset (median of 8 runs per query):

State	Detail source_data (1 resource)	Page hydrate (50 resources)	Plan used
`main` (primary key only)	3.04 ms	5.77 ms	full table scan
+ unique constraint (this PR)	0.05 ms	0.32 ms	`uq_membership_resource_source` index
+ `(resource_id, status)` index	0.06 ms	0.33 ms	no further gain

The constraint's index removes the full table scan on its own. An extra index would add write cost and a migration for no measured read gain, so this PR leaves it out. #290 should drop its corresponding index migration when it rebases.

These numbers cover reads. The Phase A read-model work also measured a per-resource state refresh dropping from 84ms to 16ms with the (resource_id, status) index, but that was without this constraint in place. It should be re-checked on top of this PR before deciding Phase A needs the extra index.

Testing

Tests (tests/db/):
- a duplicate membership insert is rejected;
- re-attaching an attached source leaves one membership;
- a source repeated in one attach batch leaves one membership;
- merging two resources that share a source leaves one ACTIVE membership on the target, with the parents set INACTIVE;
- the target is ACTIVE when only the later parent's membership was ACTIVE.
The attach/merge tests fail against the old write paths (constraint errors) and pass now.
make check passes, and make alembic-check on Postgres reports "No new upgrade operations detected".
Migration on SQLite: a fresh alembic upgrade head runs and creates the constraint.
Migration on Postgres (local 50K-resource dataset): it upgrades cleanly and creates the constraint. With one duplicate injected first, it stops on a unique violation and rolls back, leaving the version, the rows and the schema untouched.
Existing duplicates: none in production (read-only check run inside the production API container).

Notes for reviewers

Concurrent attaches (follow-up, not in this PR):
- Attach checks for an existing membership and then inserts, with no lock on the resource row.
- That can't race through today's routes. Resource creation always attaches to a brand-new resource, and the create-and-attach route always creates new sources.
- A related race already exists on main: an attach that runs at the same time as a merge of its resource can land on the merged-away resource.
- Locking the resource row when attach loads it (SELECT ... FOR UPDATE) would close both. It's tracked as a follow-up so it can be coordinated with Phase A's lock_resources helper.
Re-attaching a non-ACTIVE membership is a no-op. Attaching a source whose membership on that resource is INACTIVE or INVALID leaves it as is. INVALID is unused and is a candidate for removal later.
Impact on Refactor/db mv #290 once this merges:
- d4e5f6a7b8c9 keeps only the priority→membership foreign key (its dedup and unique steps are no longer needed).
- e6f7a8b9c0d1 and the model's Index(...) are removed.
- Its migration chain is re-pointed onto 901d261fce8b.

AI assistance

Claude Code drafted the migration, write-path changes, tests and this description, using #290's implementation as a reference. Reviews from Copilot, ChatGPT and Michael were cross-checked and addressed or noted above. I reviewed the diff and the merge status rule. The migration behaviour and query timings were checked on real Postgres as described above.

🤖 Generated with Claude Code

A source record could be attached to the same resource more than once. Make (resource_id, source_pk) unique; a source may still belong to several different resources. - Migration 901d261fce8b dedups existing rows (keep the ACTIVE row, else the lowest id), then adds uq_membership_resource_source. Irreversible. - Attach skips sources already attached to the resource (any status) and repeats within a batch. - Merge collapses a source shared by several parents into one membership on the target: ACTIVE if any parent's was, otherwise the status of the earliest parent membership. Tested on real Postgres with a 50K-resource dataset plus 26,909 injected duplicates: every expected row was kept and no duplicates remained. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-09T08:32:08Z

CD summary `ec61a54`

Frontend: https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net

Deployments (4)

service	url	fqdn
api	open	`pr-0322-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io`
entity-linkage	open	`pr-0322-el.purplegrass-c07d0a94.westus2.azurecontainerapps.io`
frontend	https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net
stitch-llm	open	`pr-0322-llm.purplegrass-c07d0a94.westus2.azurecontainerapps.io`

Database (1)

db_name	postgres_host	postgres_port	postgres_db
pr_0322	`stitch-dev.postgres.database.azure.com`	`5432`	`pr_0322`

Jobs (2)

job	image	postgres_db	api_url	auth_mode
db-migrations	`ghcr.io/rmi/stitch-api:pr-0322@sha256:d279f6a56276cf382b10b887902682d47862ad12cfc9c29e76517047d8b2a579`	`pr_0322`
seed	`ghcr.io/rmi/stitch-seed:pr-0322@sha256:61b37e513a6174186d6fd073ad406c4ebcc995088fd38b0b8a1df23348265e72`		`https://pr-0322-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io/api/v1`	`stitch-client-bearer-token`

Images (4)

build_time	commit_time	git_sha	image	image_digest
2026-10-09T08:27:28Z	2026-10-09T08:27:12Z	`0293fcd`	`ghcr.io/rmi/stitch-api:pr-0322`	`ghcr.io/rmi/stitch-api:pr-0322@sha256:d279f6a56276cf382b10b887902682d47862ad12cfc9c29e76517047d8b2a579`
2026-10-09T08:27:31Z	2026-10-09T08:27:12Z	`0293fcd`	`ghcr.io/rmi/stitch-entity-linkage:pr-0322`	`ghcr.io/rmi/stitch-entity-linkage:pr-0322@sha256:3cca1c412aa5c10bbde5b0681e134348131426294e419aea1562b6ab82ec8a24`
2026-10-09T08:27:28Z	2026-10-09T08:27:12Z	`0293fcd`	`ghcr.io/rmi/stitch-seed:pr-0322`	`ghcr.io/rmi/stitch-seed:pr-0322@sha256:61b37e513a6174186d6fd073ad406c4ebcc995088fd38b0b8a1df23348265e72`
2026-10-09T08:27:31Z	2026-10-09T08:27:12Z	`0293fcd`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322@sha256:e70131bc44c1678db878c3623fd248d47292d2f31a381a5b11a050ed12744f17`

Copilot

🟡 Changes recommended

The migration is incompatible with supported SQLite databases, and additional query and migration-test fixes are needed.

2 open findings

SQLite migration uses unsupported direct unique constraint creation · New
Migration deduplication behavior lacks integration test coverage · New

What changed in this PR

Adds uniqueness enforcement for (resource_id, source_pk) memberships, with deduplicated attach/merge logic and supporting tests.

Changes:

Cleans existing duplicates and adds a unique constraint.
Prevents duplicate attachments and merge memberships.
Adds coverage for uniqueness and merge behavior.

File	Summary
`deployments/api/tests/db/test_source_actions.py`	Tests attachment deduplication.
`deployments/api/tests/db/test_resource_actions.py`	Tests uniqueness and merge behavior.
`deployments/api/src/stitch/api/db/og_field_source_actions.py`	Deduplicates attachments; membership lookup should be limited to requested sources.
`deployments/api/src/stitch/api/db/og_field_resource_actions.py`	Consolidates memberships during merges; docstring parameter names need correction.
`deployments/api/src/stitch/api/db/model/membership.py`	Defines the unique membership constraint.
`deployments/api/alembic/versions/901d261fce8b_membership_resource_source_unique.py`	Deduplicates existing rows and adds the constraint; SQLite compatibility and migration-level coverage require updates.

🧠 Review effort: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

SQLite has no ALTER ... ADD CONSTRAINT, so 901d261fce8b failed on a supported SQLite database. Use batch_alter_table (as 790b2078d706 does); PostgreSQL still emits a plain ALTER TABLE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Only check memberships for the source_pks being attached rather than every membership on the resource, keeping the query proportional to the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

The suite builds its schema with metadata.create_all, so 901d261fce8b's destructive dedup was never executed in CI. Upgrade a temp SQLite DB to the prior head, insert duplicate memberships (ACTIVE/INACTIVE/INVALID mixes, a triple, cross-resource and singleton controls), upgrade, and assert the survivors and the new unique constraint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

apply_resource_merge described ids[0] as the merge target and said no membership consolidation happens; it actually merges into a brand-new resource and repoints memberships one per source record. _repoint_memberships referenced nonexistent parameter names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

When no contributing membership is ACTIVE, the merged membership keeps the earliest (lowest id) parent's status, e.g. INVALID, rather than being forced to INACTIVE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-16T11:27:27Z

CD summary `1dd15d2`

Frontend: https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net

Deployments (3)

service	url	fqdn
api	open	`pr-0322-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io`
frontend	https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net
stitch-llm	open	`pr-0322-llm.purplegrass-c07d0a94.westus2.azurecontainerapps.io`

Database (1)

db_name	postgres_host	postgres_port	postgres_db
pr_0322	`stitch-dev.postgres.database.azure.com`	`5432`	`pr_0322`

Jobs (1)

job	image	postgres_db
db-migrations	`ghcr.io/rmi/stitch-api:pr-0322@sha256:2fa7680d87a00a2bce8a33fa26e7b714a14416694a3f06d4a5f88b910eba1683`	`pr_0322`

Images (3)

build_time	commit_time	git_sha	image	image_digest
2026-10-16T11:23:14Z	2026-10-16T11:22:55Z	`baa1b6a`	`ghcr.io/rmi/stitch-api:pr-0322`	`ghcr.io/rmi/stitch-api:pr-0322@sha256:2fa7680d87a00a2bce8a33fa26e7b714a14416694a3f06d4a5f88b910eba1683`
2026-10-16T11:23:13Z	2026-10-16T11:22:55Z	`baa1b6a`	`ghcr.io/rmi/stitch-seed:pr-0322`	`ghcr.io/rmi/stitch-seed:pr-0322@sha256:2bd66c61799a0e51b2ef96005ac156735030aa80a330ea6db832a8af066b0f5e`
2026-10-16T11:23:14Z	2026-10-16T11:22:55Z	`baa1b6a`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322@sha256:1c4e2e5d71dc43f73383f906b57f1f2aa9468edec2a12fec23ecf0f3e35f275b`

Production has no duplicate (resource_id, source_pk) memberships, so the migration only adds the constraint. If a duplicate ever exists, the constraint add fails and the migration rolls back without deleting anything. Removes the migration test that exercised the dedup step. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

It pinned behavior that's only reachable directly through the API (INVALID is unused, and INACTIVE is only set by merges). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

github-actions · 2026-10-16T12:00:06Z

CD summary `a67eda0`

Database (1)

db_name	postgres_host	postgres_port	postgres_db
pr_0322	`stitch-dev.postgres.database.azure.com`	`5432`	`pr_0322`

Jobs (1)

job	image	postgres_db
db-migrations	`ghcr.io/rmi/stitch-api:pr-0322@sha256:af8eff5ee2dd1d631b1898fa9d0ca5b576c80500942db6e9691f88ffe58b6c6d`	`pr_0322`

Images (3)

build_time	commit_time	git_sha	image	image_digest
2026-10-16T11:59:06Z	2026-10-16T11:58:50Z	`23a02d6`	`ghcr.io/rmi/stitch-api:pr-0322`	`ghcr.io/rmi/stitch-api:pr-0322@sha256:af8eff5ee2dd1d631b1898fa9d0ca5b576c80500942db6e9691f88ffe58b6c6d`
2026-10-16T11:59:04Z	2026-10-16T11:58:50Z	`23a02d6`	`ghcr.io/rmi/stitch-seed:pr-0322`	`ghcr.io/rmi/stitch-seed:pr-0322@sha256:ce57d3ac437d01121d587f6e2f6efaf268969f8d5f098aa3f5d6ac3e48a465e1`
2026-10-16T11:59:09Z	2026-10-16T11:58:50Z	`23a02d6`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322@sha256:22a794f465e74b098d16a33b9b4b7c508d230e3219179d20bd0f1d15962038e3`

github-actions · 2026-10-16T12:08:55Z

CD summary `bb4f312`

Frontend: https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net

Deployments (3)

service	url	fqdn
api	open	`pr-0322-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io`
frontend	https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net
stitch-llm	open	`pr-0322-llm.purplegrass-c07d0a94.westus2.azurecontainerapps.io`

Database (1)

db_name	postgres_host	postgres_port	postgres_db
pr_0322	`stitch-dev.postgres.database.azure.com`	`5432`	`pr_0322`

Jobs (1)

job	image	postgres_db
db-migrations	`ghcr.io/rmi/stitch-api:pr-0322@sha256:9479cd5c31ccd96121bd3747e5864b25dec28637957ab9d27dd4b74e787ffb55`	`pr_0322`

Images (3)

build_time	commit_time	git_sha	image	image_digest
2026-10-16T12:04:56Z	2026-10-16T11:59:39Z	`8078329`	`ghcr.io/rmi/stitch-api:pr-0322`	`ghcr.io/rmi/stitch-api:pr-0322@sha256:9479cd5c31ccd96121bd3747e5864b25dec28637957ab9d27dd4b74e787ffb55`
2026-10-16T12:04:56Z	2026-10-16T11:59:39Z	`8078329`	`ghcr.io/rmi/stitch-seed:pr-0322`	`ghcr.io/rmi/stitch-seed:pr-0322@sha256:ae7df71f621992711211f6c90f22947cbd6bc1b1b590392aee4aa6a20b032c6c`
2026-10-16T12:05:00Z	2026-10-16T11:59:39Z	`8078329`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322@sha256:b6c3ad29bb77b259a45aef7f05eaa55c60badd258287bf648d6c7328b1ce620b`

github-actions · 2026-10-20T08:34:55Z

CD summary `f9e4a58`

Frontend: https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net

Deployments (3)

service	url	fqdn
api	open	`pr-0322-api.purplegrass-c07d0a94.westus2.azurecontainerapps.io`
frontend	https://witty-mushroom-017a3dc1e-322.westus2.1.azurestaticapps.net
stitch-llm	open	`pr-0322-llm.purplegrass-c07d0a94.westus2.azurecontainerapps.io`

Database (1)

db_name	postgres_host	postgres_port	postgres_db
pr_0322	`stitch-dev.postgres.database.azure.com`	`5432`	`pr_0322`

Jobs (1)

job	image	postgres_db
db-migrations	`ghcr.io/rmi/stitch-api:pr-0322@sha256:895b94b0f3cfbe24502ead9e1b9298be7c3eff596c4e841842308435be1398f1`	`pr_0322`

Images (3)

build_time	commit_time	git_sha	image	image_digest
2026-10-20T08:30:55Z	2026-10-20T08:30:36Z	`72562b7`	`ghcr.io/rmi/stitch-api:pr-0322`	`ghcr.io/rmi/stitch-api:pr-0322@sha256:895b94b0f3cfbe24502ead9e1b9298be7c3eff596c4e841842308435be1398f1`
2026-10-20T08:30:59Z	2026-10-20T08:30:36Z	`72562b7`	`ghcr.io/rmi/stitch-seed:pr-0322`	`ghcr.io/rmi/stitch-seed:pr-0322@sha256:e0a7a83e9485af998040a8f9cab6a51a655c4a7887feb6a6e0505cbe366e1566`
2026-10-20T08:30:59Z	2026-10-20T08:30:36Z	`72562b7`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322`	`ghcr.io/rmi/stitch-stitch-llm:pr-0322@sha256:27fee540c9ba57fe6d229bd41aa18e332124d05214c800f8dd35f8f61fc92b76`

AlexAxthelm deployed to development October 9, 2026 08:27 — with GitHub Actions Active

AlexAxthelm deployed to development October 9, 2026 08:28 — with GitHub Actions Active

AlexAxthelm deployed to development October 9, 2026 08:29 — with GitHub Actions Active

AlexAxthelm deployed to development October 9, 2026 08:31 — with GitHub Actions Active

AlexAxthelm marked this pull request as ready for review October 9, 2026 08:37

Copilot AI lite review requested due to automatic review settings October 9, 2026 08:37

AlexAxthelm marked this pull request as draft October 9, 2026 08:37

Copilot started reviewing on behalf of AlexAxthelm October 9, 2026 08:37

View session

Copilot AI reviewed Oct 9, 2026

View reviewed changes

Comment thread deployments/api/alembic/versions/901d261fce8b_membership_resource_source_unique.py Outdated

Comment thread deployments/api/alembic/versions/901d261fce8b_membership_resource_source_unique.py Outdated

AlexAxthelm and others added 5 commits October 9, 2026 11:24

perf(api): scope attach dedup lookup to the incoming sources

0c80078

Only check memberships for the source_pks being attached rather than every membership on the resource, keeping the query proportional to the request. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test(api): pin merge status when no parent membership is ACTIVE

21c3a76

When no contributing membership is ACTIVE, the merged membership keeps the earliest (lowest id) parent's status, e.g. INVALID, rather than being forced to INACTIVE. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

AlexAxthelm self-assigned this Oct 12, 2026

AlexAxthelm deployed to development October 12, 2026 15:31 — with GitHub Actions Active

AlexAxthelm requested a lite review from Copilot October 12, 2026 15:32

AlexAxthelm deployed to development October 12, 2026 15:32 — with GitHub Actions Active

Copilot started reviewing on behalf of AlexAxthelm October 12, 2026 15:32

View session

AlexAxthelm deployed to development October 12, 2026 15:33 — with GitHub Actions Active

AlexAxthelm deployed to development October 16, 2026 11:24 — with GitHub Actions Active

AlexAxthelm deployed to development October 16, 2026 11:25 — with GitHub Actions Active

AlexAxthelm deployed to development October 16, 2026 11:26 — with GitHub Actions Active

AlexAxthelm and others added 3 commits October 16, 2026 13:58

test(api): drop merge test for unspecified no-ACTIVE status

8dccdac

It pinned behavior that's only reachable directly through the API (INVALID is unused, and INACTIVE is only set by merges). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

style(api): ruff format attach docstring

a67eda0

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

AlexAxthelm deployed to development October 16, 2026 11:59 — with GitHub Actions Active

Merge branch 'main' into refactor/db-cleanup

bb4f312

AlexAxthelm requested a review from mbarlow12 October 16, 2026 12:04

AlexAxthelm deployed to development October 16, 2026 12:04 — with GitHub Actions Active

AlexAxthelm deployed to development October 16, 2026 12:05 — with GitHub Actions Active

AlexAxthelm deployed to development October 16, 2026 12:06 — with GitHub Actions Active

AlexAxthelm deployed to development October 16, 2026 12:07 — with GitHub Actions Active

AlexAxthelm deployed to development October 16, 2026 12:08 — with GitHub Actions Active

Merge branch 'main' into refactor/db-cleanup

f9e4a58

AlexAxthelm deployed to development October 20, 2026 08:30 — with GitHub Actions Active

AlexAxthelm deployed to development October 20, 2026 08:31 — with GitHub Actions Active

AlexAxthelm deployed to development October 20, 2026 08:32 — with GitHub Actions Active

AlexAxthelm deployed to development October 20, 2026 08:33 — with GitHub Actions Active

Navigation Menu

Repository navigation

fix(api): enforce unique og_field_memberships (resource_id, source_pk) - #322

fix(api): enforce unique og_field_memberships (resource_id, source_pk)#322
AlexAxthelm wants to merge 13 commits into
mainfrom
refactor/db-cleanup

AlexAxthelm commented Oct 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 16, 2026

Uh oh!

github-actions Bot commented Oct 20, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

AlexAxthelm commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!