feat(people): add person fact ledger and automatic resolution

[salmonumbrella]

What changed

• Add a provider-neutral person-fact catalog driven by attribute descriptions, with stable descriptor revisions and typed employment facts.
• Persist immutable evidence, reversible evidence-status events, normalized claims, deterministic decisions, and per-target pin history on SQLite and PostgreSQL.
• Automatically project accepted facts into bitemporal attributes and revisioned employment, including serialized exact organization reuse/create with no automatic merges.
• Add bounded fact diagnostics plus direct pin/unpin control through the API, generated client, and msgvault person facts; there is no candidate queue or review workflow.
• Keep model calls, scheduling, retrieval, and external enrichment out of this PR. Those can now feed the same durable generation contract separately.

Why

Msgvault already has continuously updated person activity and embeddings, but it did not have a durable way to turn supported facts into profile updates without losing evidence, replayability, manual overrides, or history. This adds that write layer first, so later archive sweeps and external enrichment providers can submit the same typed claims and get deterministic automatic resolution.

Usage

msgvault person facts catalog
msgvault person facts evidence <person-id>
msgvault person facts evidence-status <person-id>
msgvault person facts claims <person-id>
msgvault person facts decisions <person-id>
msgvault person facts pins <person-id>
msgvault person facts pin <person-id> <kind> <key>
msgvault person facts unpin <person-id> <kind> <key>

Refs #628

[roborev-ci]

roborev: Combined Review (9c109c3)

The PR has one high-severity correctness issue and three medium-severity resolution/persistence issues; no concrete security vulnerabilities were found.

High

• internal/store/person_fact_projection.go:633 — Historical employment claims can be duplicated or overwrite newer corrections. Employment resolution loads only current rows while re-evaluating every historical claim. Ended-employment claims are therefore reinserted whenever the target is touched, and an older correction for the same organization/title can overwrite a newer correction in later generations.
  • Fix: Reconcile employments using stable identity and claim precedence, and detect exact historical matches before inserting or revising rows.

Medium

• internal/personfacts/resolver.go:466 — Conflicting multi-value claims produce state-dependent outcomes. Positive and negative claims are resolved independently, so equally strong support and contradiction claims for the same fingerprint can both be applied. The value is added when absent but retired when present.

  • Fix: Group positive and negative claims by fingerprint and resolve their competition before generating projection plans.

• internal/personfacts/resolver.go:528 — Claim validity intervals are not enforced. ValidUntil is ignored, and claims with a future ValidFrom can project immediately, leaving expired and not-yet-valid claims eligible.

  • Fix: Evaluate validity intervals against ResolvedAt, excluding inactive claims or propagating both boundaries into projections.

• internal/store/person_fact_ledger.go:145 — Malformed claims can roll back an entire generation. Preparation treats unknown relations or origins as invalid decisions, but persistence writes their raw values into closed-vocabulary columns. A single malformed provider claim can violate constraints and abort the transaction.

  • Fix: Store rejected raw envelope values separately or sanitize persisted values while preserving the prepared resolution failure.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 13m21s

[roborev-ci]

roborev: Combined Review (aa7785c)

Verdict: Changes requested — six medium-severity issues can cause transaction rollbacks, nondeterministic support state, or incorrect employment projection.

Medium

1. Unsupported targets can roll back the generation instead of being recorded as rejected

   • Locations: internal/personfacts/prepare.go:203, internal/store/person_fact_projection.go:159, internal/store/schema.sql:305
   • Unknown target kinds violate the schema constraint, while supported=false for missing attribute targets is discarded before projector construction.
   • Fix: Make invalid target kinds representable in the ledger and bypass projection for unsupported targets, persisting invalid decisions without constructing a projector.

2. Reversed employment dates are not rejected during normalization

   • Location: internal/personfacts/normalize.go:168
   • Individually valid partial dates are accepted even when the end precedes the start. Strong claims then fail stricter store validation and roll back the generation.
   • Fix: Compare start and end at their shared precision and return a durable malformed-value failure when reversed.

3. Invalid organization references abort unrelated claims

   • Location: internal/store/person_fact_projection.go:734
   • Missing organization IDs or mismatched supplied names/domains propagate as transaction errors. Because organization context is prepared before scoring, even below-threshold malformed claims can discard unrelated valid claims.
   • Fix: Convert organization-not-found and validation errors into durable per-claim failures; reserve returned errors for database or infrastructure failures.

4. Evidence support state depends on reason spelling

   • Location: internal/personfacts/prepare.go:290
   • A generation can contain supported and unsupported changes for the same evidence key and source version. Lexicographic sorting by reason combined with highest-ID-wins semantics makes the final state depend on reason text rather than event chronology.
   • Fix: Permit only one net status change per evidence/version/generation, or persist an explicit ordering field used to determine final state.

5. Deleted or edited historical employment may not be restored after unpinning

   • Location: internal/store/person_fact_projection.go:845
   • Any prior applied decision is treated as proof that its projected employment still exists and matches. After manual deletion or editing followed by explicit unpinning, the historical claim remains suppressed.
   • Fix: Before suppressing the claim, verify that the referenced employment row exists and normalizes to the claim fingerprint.

6. Existing declared employment history is not fully pinned or diagnosed

   • Locations: internal/store/person_fact_projection.go:653, internal/store/person_fact_pins.go:219
   • Existing declared employments lack pin events. Current employments block resolution but are absent from diagnostics; historical employments are neither listed nor protected, allowing automatic facts to duplicate manual history.
   • Fix: Bootstrap a target-level employment pin for each person with any declared employment—through migration or an effective-pin query—and include it in pin listings.

---

Reviewers: 2 done | Synthesis: codex, 17s | Total: 26m43s

[roborev-ci]

roborev: Combined Review (8c6e96e)

Review found three medium-severity issues involving constraint consistency, backdated corrections, and concurrent manual writes.

Medium

• internal/store/person_fact_catalog.go:29 — The fact catalog omits options.max_length even though projection enforces it. Catalog-valid text can therefore fail during projection and roll back the entire generation; max-length changes also leave the descriptor revision and catalog fingerprint unchanged. Include the constraint in target descriptors and normalize claims against it, or exclude constrained definitions. Add a behavioral test for over-limit generated claims.

• internal/store/person_fact_projection.go:1363 — A replacement or retirement backdated before the current attribute’s ActiveFrom aborts the entire generation, preventing legitimate corrections from being retained in the ledger. Record the temporal conflict as a durable decision or project it using a safe interval boundary instead of returning a rollback-triggering error.

• internal/store/person_attributes.go:166,447 — Manual attribute writes create pins without acquiring the generation and target locks used by automatic resolution. On PostgreSQL, a resolver that observed the target earlier can replace the newly declared value or add another value despite the pin. Acquire the same locks, in the same order, before manual mutation and pin creation.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 11m16s

[roborev-ci]

roborev: Combined Review (b33f9dc)

The PR has two medium-severity correctness issues that could retire unrelated attribute data or break pin listing after attribute deletion.

Medium

• internal/store/person_fact_projection.go:1438 — retireUnsupported closes every current non-declared attribute value absent from the fact ledger’s live decisions, including extraction, enrichment, or system values not created by fact projection. Processing a generation for the same target can therefore silently retire unrelated data.

  • Fix: Restrict retirement to values demonstrably owned by earlier person-fact decisions, such as values linked through decision projections. Add a test confirming unrelated derived values remain current.

• internal/store/person_fact_pins.go:268 — Pin listing always projects the target’s current value. If a pinned user-defined attribute is later deleted, descriptor loading marks the target unavailable, but projection fails with ErrAttributeDefinitionNotFound, causing the entire pin-list endpoint to error.

  • Fix: Handle unavailable targets by skipping them or returning their event state without projection. Add a create-pin-delete-definition-list lifecycle test.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m35s

[roborev-ci]

roborev: Combined Review (f0dc33e)

Code review found three medium-severity correctness issues in organization resolution, claim listing, and employment retirement.

Medium

• Organization matching may attach employment to the wrong organization — internal/store/person_fact_organization.go:218
  Name and domain candidates are unioned, so a match for only one supplied identifier can silently reuse an organization despite a conflicting or unmatched second identifier. Require all supplied identifiers to resolve to the same candidate, such as by intersecting candidate sets or verifying the complete reference before reuse.

• Malformed persisted targets can abort the entire claims listing — cmd/msgvault/cmd/person_facts.go:217
  The command uses strict EncodeTargetRef validation when formatting stored targets. Because unsupported or malformed targets are retained as rejected claims, one such record prevents the complete diagnostic table from being displayed. Render persisted fields losslessly with a per-row fallback, while keeping strict validation for user input.

• Invalid employment retirement dates can roll back projection generation — internal/store/person_fact_projection.go:1109
  Retirement passes the claim’s ValidFrom directly as the employment end date. If it predates the employment start, endEmploymentLockedTx rejects it and rolls back the generation. Clamp the retirement date to the employment start, consistent with attribute projection, or record a durable rejection/no-op.

---

Reviewers: 2 done | Synthesis: codex, 11s | Total: 9m8s

[roborev-ci]

roborev: Combined Review (62e50e7)

Code review found two medium-severity correctness issues; no critical or high-severity findings.

Medium

• Unowned derived attributes can be superseded — internal/store/person_fact_projection.go:1358
  Replacement and explicit-retirement plans can close an existing extraction, enrichment, or system value even when no prior fact decision owns it, because ownership is checked only in retireUnsupported. Protect non-declared, unowned current values for every plan carrying CurrentRef, retaining or rejecting competing claims instead of projecting them. Add coverage for a valid competing claim against an unowned derived value.

• Evidence with an empty source version cannot transition status — internal/personfacts/alignment.go:96
  Public, provider-assertion, and system evidence may be persisted with an empty SourceVersion, while prepareEvidenceStatusChanges requires a nonempty version. This prevents such evidence from being marked unsupported or reactivated and may leave stale projections permanently active. Require a nonempty source version for all evidence classes, or consistently support empty versions in status transitions and persistence lookups.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 14m32s

[roborev-ci]

roborev: Combined Review (d751732)

Changes need revision: two medium-severity correctness and compatibility issues were identified; no security vulnerabilities were found.

Medium

• Competing employment claims resolve inconsistently — internal/store/person_fact_projection.go:961
  Multiple current-employment claims with the same organization/title in one generation survive reconciliation because their fingerprints differ. Each claim is recorded as applied, while the projector repeatedly updates the same employment row in claim-key order, leaving an arbitrary final value inconsistent with the decision ledger.
  Fix: Resolve or reject competing same-generation claims by stable employment identity before producing projection plans, ensuring at most one current claim per organization/title is applied.

• Description limit introduces a breaking API constraint — internal/store/attribute_definitions.go:327
  The new 280-rune limit applies to all attribute definitions, including organization and inference-ineligible definitions. This rejects previously valid create/update requests despite the release being described as additive, while the catalog already filters overlong descriptions separately.
  Fix: Keep stored descriptions unrestricted and omit overlong descriptions from the fact catalog, or limit the constraint to eligible person-fact targets and document the API change.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 14m43s

[roborev-ci]

roborev: Combined Review (0e4a4db)

The change is broadly sound, but two medium-severity correctness issues should be addressed.

Medium

• Lossy submitted-JSON fingerprinting — internal/personfacts/normalize.go:280
  Raw JSON is canonicalized through float64, allowing distinct large numbers to produce identical submitted fingerprints. For invalid claims without a normalized fingerprint, this can misclassify a later generation as a replay and silently discard it.
  Fix: Canonicalize submitted JSON numbers losslessly using arbitrary precision, independently of attribute-value normalization.

• Tracking update race on PostgreSQL — internal/store/person_fact_projection.go:94
  Generation application acquires the person-fact-generation lock before checking tracking, but SetPersonTrackingContext does not use the same lock. Untracking may commit after that check while automatic projections subsequently commit, so maintenance can occur after untracking returns.
  Fix: Serialize tracking changes with the person-fact-generation lock before inserting or deleting the tracking row.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 14m36s

[roborev-ci]

roborev: Combined Review (67e2b59)

Code review found two medium-severity reliability issues in employment fact projection and pin inference.

Medium

• internal/store/person_fact_projection.go:866 — Historical employment replay can abort after organization changes. Explicit organization IDs are revalidated during replay, so merged, retired, or renamed organizations can permanently block later fact generations and pin updates. Canonicalize merged IDs and turn stale references into durable per-claim rejections rather than aborting the generation; add replay coverage for organization merges and renames.

• internal/store/person_fact_pins.go:383 — Unowned employment projections may be treated as unpinned. Employment pin inference returns after checking user/import provenance and skips the ownership check used for other fact targets. A derived employment created outside the fact ledger can therefore be revised or ended automatically. Treat unowned employment projections as implicitly pinned unless explicitly unpinned, or enforce ownership per employment row during projection.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 14m9s

[roborev-ci]

roborev: Combined Review (a3f45ba)

Verdict: One medium-severity correctness issue should be addressed before merge.

Medium

• Distinct malformed evidence can collapse into one generation — internal/personfacts/prepare.go:132

  Invalid or rejected evidence is removed before canonical claim identity is computed. Submissions containing different malformed evidence can therefore receive the same generation key, causing a later submission to be treated as a replay and omitted from the immutable ledger.

  Suggested fix: Include a deterministic fingerprint for every submitted evidence item, including rejected items, in claim and generation identity. Add a test proving that distinct malformed evidence produces distinct persisted generations.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 28m16s

[roborev-ci]

roborev: Combined Review (1aaa3e4)

Medium-severity issue found: expired employment claims can leave stale projected employment active indefinitely.

Medium

• internal/store/person_fact_projection.go:610 — Expired employment claims are marked outside-validity, but stale projections are retired only for attribute targets. If an employment target is resolved after its supporting claim’s exclusive ValidUntil, the previously projected employment remains current indefinitely.
  • Fix: Add employment-specific expiry reconciliation that ends automatically projected employment at the validity boundary while preserving unsupported-evidence behavior and unrelated or manually created employments.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 21m20s

[roborev-ci]

roborev: Combined Review (c9d7c28)

Code review verdict: No Medium, High, or Critical findings were reported.

---

Reviewers: 2 done | Synthesis: codex, 6s | Total: 24m21s

[roborev-ci]

roborev: Combined Review (474dea0)

Review found two medium-severity concurrency/ownership issues that could allow resolutions to overwrite external updates.

Medium

• internal/store/person_fact_projection.go:1968 — Projection ownership relies only on historical (kind, row_id) references. Because employment rows are updated in place, a non-declared external update to a fact-projected employment remains classified as owned and unpinned, allowing a later resolution to overwrite it.

  • Fix: Verify ownership against the current decision-bound source_ref or content, or append/revoke pin ownership for every out-of-ledger employment update.

• internal/store/person_attributes.go:168 — Only declared attribute writes acquire the person-fact target lock, even though non-declared rows without a fact decision are implicitly pinned. A concurrent extraction, enrichment, or system write can occur after resolution reads the current state and then be overwritten by that resolution.

  • Fix: Acquire lockManualPersonFactAttributeTx for every public attribute write, while keeping explicit pin-event creation conditional on declared provenance.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 21m45s

[roborev-ci]

roborev: Combined Review (3a8242a)

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

Medium

• internal/store/person_fact_projection.go:529 — The non-projectable branch overwrites existing durable claim failures with invalid/unsupported-target. For example, a sensitive target prepared with AllowSensitive=false as policy-rejected/sensitive-policy is incorrectly reclassified as unsupported, breaking validation precedence and audit consistency.
  • Fix: Only synthesize an unsupported-target failure when the claim has no existing failure, and add a store-level sensitive-policy regression test.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 16m35s

[roborev-ci]

roborev: Combined Review (f0f3ed0)

Code review identified one medium-severity issue affecting durable employment pinning.

Medium

• internal/store/employments.go:136 — Derived updates create durable pins only when new provenance is declared. If an external update ends or reassigns a fact-owned employment, no current unowned row remains to infer pinning from. A later fact generation may recreate the stale employment or restore its former owner.
  • Fix: Detect person-fact ownership before the update and append durable employment pin events for every affected person whenever the update detaches that ownership, regardless of resulting provenance or status.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 18m46s

[roborev-ci]

roborev: Combined Review (e7ad00d)

Code review found one medium-severity correctness issue; no qualifying security findings were identified.

Medium

• internal/personfacts/resolver.go:314 — Single-cardinality resolution globally compares negative and positive directions across different fingerprints. A strong contradiction for value X can suppress valid support for value Y, while projection retires only X, potentially leaving the target empty or stale.
  • Fix: Limit negative competition to the matching fingerprint before selecting among surviving supported values. Add coverage where an unrelated negative direction outscores valid support.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 19m52s

[roborev-ci]

roborev: Combined Review (60d65fd)

The implementation is functionally sound and introduces no identified security weakness, but has a medium-severity scalability concern.

Medium

• internal/store/person_fact_projection.go:727 — Each touched target loads and hydrates every claim for the person before filtering by target, causing potentially thousands of unnecessary queries as the append-only ledger grows. Filter by target kind/key in SQL before hydration, and batch-load generation/evidence associations.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 28m27s

[roborev-ci]

roborev: Combined Review (4dfb2ee)

Two medium-severity correctness issues should be addressed before merging.

Medium

• internal/store/employments.go:142 — Updating a fact-projected employment can leave it classified as automatically owned when the caller preserves its derived source and source_ref. Because the CLI preserves these fields during full-replacement updates, a manual edit may not create a pin and could be overwritten by the next fact resolution.

  • Fix: Detect content changes against the originating projection, or explicitly detach/pin user-facing updates even when provenance fields are preserved.

• internal/store/person_fact_projection.go:2083 — An expired attribute projection is closed at the generation’s resolution time instead of the claim’s exclusive valid_until. If expiration is processed late, history incorrectly shows the value as active beyond its declared validity window.

  • Fix: For outside-validity retirements, close the projection at the applicable claim’s ValidUntil, clamped to the projection’s start, consistent with the employment-expiry path.

---

Reviewers: 2 done | Synthesis: codex, 8s | Total: 13m11s

[roborev-ci]

roborev: Combined Review (6441ad9)

The review found two medium-severity correctness issues involving out-of-order generation ingestion; no security vulnerabilities were identified.

Medium

• Stale generations can overwrite current projected attributes — internal/store/person_fact_projection.go:565
  Resolution always uses the incoming generation’s ResolvedAt, so a delayed older generation may evaluate newer claims before their ValidFrom and replace a current attribute with stale data.
  Fix: Resolve using a per-target monotonic watermark or reject stale generations. Add an out-of-order ingestion test that applies the newer generation first.

• Evidence status incorrectly depends on insertion order — internal/store/person_fact_projection.go:889, internal/store/person_fact_ledger.go
  Selecting effective evidence status with MAX(id) allows a delayed older generation to overwrite a newer supported/unsupported status and affect later resolution.
  Fix: Order statuses by the generation’s causal timestamp or another enforced monotonic sequence, with ID only as a deterministic tie-breaker. Test reversed generation insertion.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 23m16s

[roborev-ci]

roborev: Combined Review (e28c28d)

Code review found three medium-severity issues; no critical or high-severity findings.

Medium

• Validity retirement loses its projection link — internal/store/person_fact_projection.go:2149
  The code computes expiryClaimKey but records the empty claimKey, so the retired projection is neither linked to its decision nor returned in GenerationResult.Projections. Assign claimKey = expiryClaimKey when expiredAt is selected, and verify the expiration decision references the retired projection.

• Merge/split can lose explicit pin state — internal/store/person_merge_snapshot.go:244
  Pin events are excluded from merge snapshots and cascade when the absorbed person is deleted. A later split cannot reliably restore explicit pin state. Preserve effective pin state by snapshotting/restoring pin events or appending equivalent state events during merge and split.

• Unbounded JSON exponents can cause excessive allocation — internal/personfacts/normalize.go:269
  Integer normalization passes arbitrary exponents such as 1e1000000000 to big.Rat.SetString, potentially causing enormous allocation before the int64 range check. Bound the decimal exponent and significant digits before arbitrary-precision parsing, rejecting values that cannot fit in int64.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 15m0s

[roborev-ci]

roborev: Combined Review (070b426)

The PR needs one medium-severity migration fix; no material security issues were found.

Medium

• internal/store/organizations.go:582 — Switching persisted organization domains to IDNA ASCII normalization without migrating existing rows leaves pre-upgrade Unicode domains unmatched during fact-resolution comparisons. Referenced organizations may be rejected for domain disagreement, while unreferenced claims may create duplicates.
  • Fix: Add a migration that canonicalizes organizations.primary_domain and domain identifiers’ normalized_value, handles collisions, and includes an upgrade-path test.

---

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

[roborev-ci]

roborev: Combined Review (b25a00e)

Review verdict: Three medium-severity correctness issues require attention; no high, critical, or security findings were identified.

Medium

• Historical employment can resolve to duplicate organizations — internal/store/person_fact_projection.go:1167
  Name/domain-only employment claims are re-resolved against mutable organization data. If the organization is renamed or its domain changes, a previously accepted claim can stop matching and create duplicate organization and employment records. Persist and reuse the canonical organization ID selected for accepted claims, with an integration test covering organization rename/domain change and subsequent resolution.

• Generated validators reject valid evidence responses — pkg/client/generated/types.go:5957
  Several required evidence strings are incorrectly validated as non-empty, although valid API responses may contain empty values. Public evidence may omit source_ref and content_sha256, archive evidence may omit source_url, and evidence status events may have an empty source_version. Update the OpenAPI/codegen validation model to preserve required field presence without requiring non-empty content, and test representative public and archive responses.

• IDNA migration does not invalidate affected revisions — internal/store/migrations.go:106
  The migration changes organization domains and active identifiers without incrementing organization revisions or affected employees’ vCard projection revisions. Existing ETags and projection revision values therefore remain valid after semantic data changes. Within the migration transaction, track affected organizations, update each revision and timestamp once, and bump the corresponding employed-person vCard projection revisions.

---

Reviewers: 2 done | Synthesis: codex, 12s | Total: 21m31s

[roborev-ci]

roborev: Combined Review (8f9ba92)

Overall verdict: One medium-severity correctness issue; no security regression identified.

Medium

• Future evidence receives maximum freshness
  • Location: internal/personfacts/resolver.go:596
  • Problem: Evidence dated after ResolvedAt has a negative age and receives the maximum freshness bonus. A far-future timestamp could push an otherwise sub-threshold claim into automatic projection and retain that bonus for years.
  • Fix: Reject future event timestamps during validation, or return no freshness bonus when eventTime.After(resolvedAt).

---

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

[roborev-ci]

roborev: Combined Review (865a022)

Overall verdict: One medium-severity data-integrity issue should be fixed before merge.

Medium

• Unmatched organization bindings can create duplicates after renames — internal/store/person_fact_ledger.go:395

  Employment organization bindings are persisted only when an applied decision creates a projection. If a name/domain claim matches an existing employment without creating a projection, the claim remains unbound. A later organization rename may cause re-resolution to interpret the old name/domain as a new organization, creating duplicate organization and employment records.

  Fix: Persist the resolved organization ID for every successfully matched employment claim, independently of projection creation or decision action, and load these bindings during subsequent resolutions.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 16m0s

[roborev-ci]

roborev: Combined Review (b3f89e7)

Code is generally sound, but one medium-severity generated-client validation issue should be fixed before merging.

Medium

• pkg/client/generated/types.go:6104 — Generated validation marks Excerpt and SubjectRef as non-empty via required, although the API permits empty strings. Valid server evidence can therefore fail client-side validation.
  • Fix: Model these as required-but-nullable fields like other empty-capable evidence strings, extend the code-generation repair, and add explicit tests for empty values.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 25m28s

[roborev-ci]

roborev: Combined Review (6532499)

The PR has one medium-severity correctness issue; no material security regressions were found.

Medium

• internal/store/person_fact_projection.go:1882 — Employment corrections match only the person, organization, and normalized title. A historical episode can overwrite a current stint with the same employer/title, and multiple historical corrections can all revise the single “latest” row selected at line 1899, collapsing distinct episodes and losing employment history.
  • Fix: Bind projections to a specific employment episode using provenance and date information, and ensure each projection plan consumes at most one existing row.

---

Reviewers: 2 done | Synthesis: codex, 9s | Total: 19m46s

[roborev-ci]

roborev: Combined Review (69082da)

Changes requested: two medium-severity issues could corrupt or retain historical employment projections.

Medium

• Incorrectly merges distinct historical employment stints — internal/store/person_fact_projection.go:1984
  A sole historical employment candidate is revised regardless of its date-match score. A newly discovered, non-overlapping stint at the same company and title can overwrite the existing episode instead of creating a separate historical employment. Require a meaningful date match or overlap before treating the candidate as a correction, and add coverage for incrementally discovered repeated stints.

• Contradicted historical employment remains projected — internal/store/person_fact_projection.go:1123
  Historical employments are excluded from resolver state via CurrentOnly: true. An accepted contradiction or supersession targeting an already-projected historical employment therefore produces no retirement projection, leaving the contradicted row visible indefinitely. Resolve negative historical claims against their previously bound, fact-owned employment rows and retract or supersede those rows deterministically.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 23m28s

[roborev-ci]

roborev: Combined Review (7db62f9)

The changes need revision due to two medium-severity correctness issues.

Medium

• internal/store/person_fact_pins.go:42 — Pin changes bypass tracking opt-out

  Pin operations verify only that the person exists. Unpinning an untracked person can still create a generation, resolve historical claims, and alter their profile.

  Fix: Under the generation lock, call verifyTrackedPersonFactPersonTx before writing a pin event or re-resolving.

• internal/personfacts/resolver.go:314 — Unrelated negative claims can suppress valid values

  Single-value resolution competes negative claims against positive claims for unrelated fingerprints. A strong contradiction for absent value B can supersede valid support for A, yet retire nothing because B is not current, leaving the target unset.

  Fix: Scope negative claims to their fingerprint—or to a matching current value—before comparing the remaining positive candidates.

---

Reviewers: 2 done | Synthesis: codex, 10s | Total: 23m35s

[roborev-ci]

roborev: Combined Review (367e65c)

The review found three medium-severity correctness issues in person-fact pinning and resolution; no security vulnerabilities were identified.

Medium

• internal/store/person_notes.go:108 — Appending a user note bypasses the manual pin helper. After Notes is explicitly unpinned, appending leaves that unpin event effective, so automatic resolution can overwrite the newly curated note.

  • Fix: Acquire the normal person-fact locks and append a pinned event for declared-source note writes, matching SetPersonAttributeValueContext. Add coverage for unpin → append → automatic resolution.

• internal/store/person_fact_pins.go:351 — Clearing an attribute while its definition is temporarily ineligible, such as inactive or missing an inference description, silently skips the absence pin. If it later becomes eligible, historical claims can repopulate the value despite the explicit manual clear.

  • Fix: Record manual pins for supported mutable scalar definitions regardless of transient catalog eligibility. Test clear → re-enable or update description → resolve.

• internal/store/person_fact_projection.go:488 — Status-only resolution for a deleted attribute uses a fallback descriptor without cardinality or value type. A historical claim with a durable failure other than unsupported-target can cause the resolver to reject the malformed descriptor, rolling back the evidence-status event and other targets touched by that event.

  • Fix: Supply a structurally valid unavailable-target descriptor, or permit resolution inputs where every claim has a valid durable rejection. Cover a deleted definition with a rejected, evidence-linked claim.

---

Reviewers: 2 done | Synthesis: codex, 18s | Total: 25m45s

[roborev-ci]

roborev: Combined Review (d70ff79)

The person-fact ledger and resolver are implemented but not connected to production flows, so the feature remains unused at runtime.

Medium

• internal/store/person_fact_projection.go:68 — ApplyPersonFactGenerationContext has no production caller. Normal import, extraction, enrichment, and tracking flows never submit generations, leaving the new ledger and automatic resolver unused.
  • Fix: Wire the intended production fact producer and evidence aligner into the runtime pipeline so tracked people receive generations.

---

Reviewers: 2 done | Synthesis: codex, 7s | Total: 13m48s

[wesm]

not fixing