Skip to content

feat(membership): refuse collapsing targets into entity or project - #131

Open
seonghobae wants to merge 9 commits into
mainfrom
agent/membership-target
Open

feat(membership): refuse collapsing targets into entity or project#131
seonghobae wants to merge 9 commits into
mainfrom
agent/membership-target

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

Language, episode, template, department, and opportunity-pool memberships stay typed (ADR 0003). They cannot collapse into the entity/project pair stored by migration 0006. Recovery is the computed share of recovered target kinds that match known truth versus treating every target as an entity.

Local gates on this head:

  • cargo test -p membership_target --all-targets GREEN after RED (package did not exist)
  • clippy -D warnings PASS
  • workspace contract PASS
  • docstring contract PASS
  • lines 15/15; nightly-2026-08-01 branches 8/8

Does not allocate migration 0008. Does not recreate membership_core, role_contradiction, support_edge, or in-flight crates.

Keep this PR draft. Preferred merge remains #46 only when exact-head required Checks pass and a qualifying independent (non-Cursor/CodeRabbit) APPROVE exists. OpenCode CHANGES_REQUESTED on #46 is a stale coverage-evidence race on a now-green head — do not empty-commit.

Add insert/lookup contracts for entity_record and project_record so
membership foreign keys cannot be seeded from empty, oversized, or
hostile type/status labels. No new migration number while 0007 is in flight.
Language, episode, template, department, and opportunity-pool
memberships stay typed (ADR 0003). They cannot stand in for the
entity/project pair stored by migration 0006. Recovery is the
computed share of target kinds that match known truth versus
collapsing every target to entity.
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@seonghobae, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 26 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 95c67be5-b2dc-4efa-9a66-74d1cdfd1b0f

📥 Commits

Reviewing files that changed from the base of the PR and between c45be17 and 967b89f.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (32)
  • .codegraph/.gitignore
  • ARCHITECTURE.md
  • CHANGELOG.md
  • Cargo.toml
  • README.md
  • crates/membership_target/Cargo.toml
  • crates/membership_target/src/error.rs
  • crates/membership_target/src/kind.rs
  • crates/membership_target/src/lib.rs
  • crates/membership_target/tests/crate_contract.rs
  • crates/membership_target/tests/target_kind_contract.rs
  • crates/persistence_postgres/src/entity_sql.rs
  • crates/persistence_postgres/src/error.rs
  • crates/persistence_postgres/src/lib.rs
  • crates/persistence_postgres/src/live_repository.rs
  • crates/persistence_postgres/src/project_sql.rs
  • crates/persistence_postgres/tests/entity_project_sql_contract.rs
  • crates/persistence_postgres/tests/live_postgres.rs
  • docs/ERD.md
  • docs/TRACEABILITY.md
  • docs/adr/0003-relational-event-multiple-membership.md
  • docs/adr/0011-standalone-modular-msa-boundary.md
  • docs/adr/0013-bitemporal-persistence-reproducibility-and-split-authority.md
  • docs/adr/README.md
  • docs/connectors/naruon-artifact-consumer.md
  • docs/research/entity-project-sql.md
  • docs/research/membership-target-identity.md
  • docs/research/standards-and-literature.md
  • docs/research/typed-membership-assignment-persistence.md
  • docs/validation/temporal-event-foundation.md
  • scripts/check_workspace_contract.py
  • tests/quality/test_check_docstrings.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae
seonghobae marked this pull request as ready for review August 20, 2026 18:22
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current head 35b29cd includes origin/main; only the changelog conflict was resolved. Exact-head proof passed: cargo fmt --all -- --check; cargo test -p membership_target --offline (6 tests); workspace contract; documentation validation; Rust docstring contract; git diff --check. Protected merge remains gated by current Checks and independent approval.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

# Conflicts:
#	CHANGELOG.md
#	crates/persistence_postgres/tests/live_postgres.rs
#	docs/ERD.md
@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head validation update (e05f1d6): fixed the quality contract to derive the Rust crate count from scripts/check_workspace_contract.py instead of hard-coding 10. Local evidence: 89 quality tests passed; coverage 100% (991/991 statements, 442/442 branches); workspace, docstring, documentation, and diff checks passed. Please review and rerun Checks against this exact head; merge remains subject to the repository's two independent approvals and protected rules.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current-head review refresh for e65620b:

  • Customer, target, and project membership contexts remain distinct; the implementation refuses collapsing a target into an entity or project identity.
  • git diff --check, documentation validation, workspace contract, docstring contract, and cargo fmt --all -- --check passed locally.
  • Current-head review remains merge-ineligible until all required statuses and qualifying independent approvals are current.
  • No new actionable source defect was found at this exact head; no approval is being self-issued.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Rebased current head 967b89f onto origin/main. The changelog conflict retains both feature and current-main entries; inherited documentation trailing whitespace was removed. Local merge-tree, git diff --cached --check, and cargo fmt --all -- --check pass. Exact-head hosted checks and required independent approvals remain required before protected merge.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review


## Roles, membership, and multilevel structure

Browne, W. J., Goldstein, H., & Rasbash, J. (2001). Multiple membership multiple classification (MMMC) models. *Statistical Modelling, 1*(2), 103–124. https://doi.org/10.1177/1471082X0100100201

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Same research paper cited with two conflicting DOIs

The Browne, Goldstein & Rasbash (2001) reference is added (docs/research/standards-and-literature.md:79) with DOI 10.1177/1471082X0100100201, but the same paper is already cited elsewhere in the repository with 10.1177/1471082X0100100202 (docs/research/typed-membership-assignment-persistence.md:31), so one of the two citations points readers to the wrong record.

Impact: A reader following the reference can land on an incorrect or non-resolving source, undermining the APA-7 traceability the repository requires.

Where the conflicting DOIs appear

The new/added citations use the ...100201 suffix in two places: docs/research/standards-and-literature.md:79 and docs/research/membership-target-identity.md:31. The pre-existing citation of the identical paper uses the ...100202 suffix at docs/research/typed-membership-assignment-persistence.md:31. Both cannot be correct for the same article; the DOIs should be reconciled to a single accurate value.

Suggested change
Browne, W. J., Goldstein, H., & Rasbash, J. (2001). Multiple membership multiple classification (MMMC) models. *Statistical Modelling, 1*(2), 103–124. https://doi.org/10.1177/1471082X0100100201
Browne, W. J., Goldstein, H., & Rasbash, J. (2001). Multiple membership multiple classification (MMMC) models. *Statistical Modelling, 1*(2), 103–124. https://doi.org/10.1177/1471082X0100100202
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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.

1 participant