Skip to content

feat(people): add typed temporal relationships - #592

Open
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:typed-person-relationships-v0193-publication
Open

feat(people): add typed temporal relationships#592
salmonumbrella wants to merge 1 commit into
kenn-io:mainfrom
salmonumbrella:typed-person-relationships-v0193-publication

Conversation

@salmonumbrella

Copy link
Copy Markdown
Contributor

What changed

  • Add seeded and user-defined relationship types with forward and reverse labels, symmetric-type support, presentation metadata, and optional vCard RELATED mappings.
  • Store one canonical, temporal edge per person relationship with partial start and end dates, history, notes, provenance, confidence, optimistic concurrency, and active-duplicate protection.
  • Render the correct label from either person's view without mirrored rows, and keep ended relationships available as history.
  • Resolve imported RELATED values only by exact person UID; unresolved values enter a review queue instead of creating people automatically.
  • Expose the model through equivalent SQLite and PostgreSQL stores, the daemon HTTP/OpenAPI clients, and new relationship CLI commands. A graph UI remains out of scope.

Why

Msgvault can preserve rich person profiles, but it cannot yet represent how two people are connected or how that connection changes over time. Copying labels onto profiles loses direction, history, and source evidence, while guessing from imported names or contact values risks linking the wrong people.

Usage

msgvault relationship-type list
msgvault person relationship add 12 partner 34 --from 2024-03
msgvault person relationship list 12
msgvault person relationship end 57 2026-08-09
msgvault person relationship reviews --status pending

Refs #534

@roborev-ci

roborev-ci Bot commented Aug 9, 2026

Copy link
Copy Markdown

roborev: Combined Review (332e3e4)

Code review found one medium-severity consistency issue; no high or critical findings.

Medium

  • Seeded vCard mappings can cause startup failures after mutationinternal/store/relationship_types.go:374

    Seeded relationship types allow vcard_related_type to be cleared or reassigned, but startup reconciliation restores the original mapping. For example, clearing friend and assigning it to a custom type can cause reseeding to violate the unique constraint, preventing archive initialization after restart.

    Fix: Make seeded types’ vCard mappings immutable, or stop reconciling this field so successful updates persist consistently.


Reviewers: 2 done | Synthesis: codex, 8s | Total: 12m47s

@wesm

wesm commented Aug 11, 2026

Copy link
Copy Markdown
Member

Reviewed the store/schema layer, API layer, CLI, and generated artifacts. Verified locally: build, full test suite with fts5 sqlite_vec tags, make openapi-check (generated client and specs are byte-reproducible), and all CI checks green. The core design held up well — canonical edge normalization, optimistic concurrency, partial-date comparison, and SQLite/PG parity all checked out. One bug worth fixing before merge.

Major

  • Seed reconciler can permanently break InitSchema (relationship_types.go:344 + relationship_type_seed.go:209): UpdateRelationshipTypeContext allows editing vcard_related_type on system types, but the seed reconciler rewrites that column on every InitSchemaContext. Confirmed by execution: clear the seeded friend mapping via the API, create a custom type claiming vcard_related_type = "friend", and every subsequent startup fails with a UNIQUE violation inside InitSchemaContext — no API path to recover. Even without the collision, accepted edits to system-type mappings are silently reverted (with a revision bump) on next open. Suggest rejecting VCardRelatedType edits on ownership = 'system' rows.

Minor

  • PATCH relationship-type is fail-open on miscased keys (internal/api/person_relationships.go:235): Go decodes JSON field names case-insensitively, so {"Forward_Label":"x"} passes DisallowUnknownFields and the min-one-field gate but leaves all update pointers nil — returns 200 with a revision bump while changing nothing. The sibling handlePatchPersonRelationship fails closed on the same input.
  • Seeding races across processes (relationship_type_seed.go:149): SELECT-then-INSERT without conflict handling; two processes initializing the same PostgreSQL DB can race, failing the loser's InitSchemaContext. Transient, and mirrors the existing attribute-seed pattern.
  • person_relationships.status is unconstrained (schema.sql:786): fully derivable from end_year but no CHECK ties them together, despite the table comment promising integrity for writers that bypass Go. Add the CHECK or drop the column.
  • Date asymmetry in the spec (api/openapi.yaml:1585): request start_date/end_date are bare strings with no pattern/description, while responses use PartialDate objects — fetched dates can't round-trip into a PATCH body, and accepted formats are only discoverable from 400 error text.
  • CLI relationship-type create omits --description/--vcard-related-type (person_relationship.go:569): the API accepts both on create and update exposes them, so a described type takes two commands and two revisions.

Nits

  • relationship_types.go:332: dead nil-check on an any holding sql.NullString; %v renders conflict errors as {friend true} instead of friend.
  • person_relationships_test.go:146 hard-codes the seed count (19); the next seeded type breaks an unrelated HTTP test.
  • status/direction/source response fields lack enum in the spec, unlike the review status query param.
  • Test gaps: "end date before stored start date" via End/Patch is never exercised; no CLI test covers relationship reviews or delete confirmations.
  • relationshipIfMatch duplicates personIfMatch modulo the tag prefix; relationship end takes the date positionally while add uses --until.

🤖 Generated with Claude Code

@salmonumbrella
salmonumbrella force-pushed the typed-person-relationships-v0193-publication branch from 332e3e4 to f3faaa6 Compare August 11, 2026 21:36
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (f3faaa6)

Two medium-severity correctness issues remain; no critical or high-severity findings were identified.

Medium

  • Seeded relationship symmetry can prevent startup after label drift
    Locations: internal/store/relationship_types.go:370, internal/store/relationship_type_seed.go:214
    Label validation trusts the stored IsSymmetric value, but seed reconciliation later restores seed-defined symmetry without reconciling labels. If a symmetric seed drifts to is_symmetric=false, the update API permits different labels; a subsequent InitSchema restores is_symmetric=true, violates the symmetric-label constraint, and prevents startup until the database is manually repaired.
    Fix: Validate seeded types using seed-defined symmetry, safely reconcile incompatible labels before restoring the structural flag, and add a regression test covering drift, label updates, and subsequent InitSchema.

  • Asymmetric vCard RELATED relationships are reversed
    Location: internal/store/person_relationship_related.go:163
    The TYPE describes the related value’s role relative to the card subject. For example, RELATED;TYPE=parent:<bob> means Bob is the subject’s parent, but the code records the subject as Bob’s parent. This inverts imports for parent/child, agent, emergency-contact, muse, crush, and other asymmetric types.
    Fix: Store the matched person as the typed source and the card subject as the target, or explicitly translate the imported type to its inverse. Add an import test for at least one asymmetric type.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 10m40s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants