feat(identity-resolution): persons-seed write path (POST /v1/persons-seed) (#1754) - #1825
Merged
Conversation
…erson assignment (#1754) First (pure, DB-free) slice of the write side. Ports the .NET EmailProfileResolver + PersonAssignmentResolver into domain/seed.rs: - group_by_email: fold source-account profiles into per-person groups by current email (case-insensitive; no-email profiles become singletons). - resolve_assignments: the four-branch classification, in priority order — reuse an already-bound account (idempotent), else link to the person the group's email already maps to, else mint a fresh person when at least one profile is active, else skip (no email / all closed). `mint` injected for deterministic tests. Types: SourceAccountKey, SeedProfile, ProfileGroup, PersonAssignment (+AssignmentKind), ResolveOutcome. 6 unit tests cover grouping + every branch, incl. known-wins-over-email and whole-group-binds-via-one-known-member. #![allow(dead_code)] is temporary — dropped when the infra/API slices consume these. No wire/DB changes. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…sactional apply (#1754) Second write-side slice (infra/db/seed_repo.rs), SQL verbatim from the .NET SqlPersonsSeed: - known_account_bindings + latest_email_to_person: the resolver-feeding reads (current account→person bindings; latest email→person map, keyed by the same normalize_email the resolver uses). - apply: INSERT IGNORE the resolved observations into persons, then rebuild the tenant's account_person_map (SCD2 via LEAD) — one transaction, rolled back on error, so the log and the derived cache stay consistent. - SeedObservationRow: the resolved-observation write shape. org_chart rebuild + the ClickHouse identity_inputs reader are later slices. Env-gated read-only integration test (skips without a DB). Starts wiring the domain seed module (SourceAccountKey / normalize_email now consumed). Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ild profiles, value routing, row building (#1754) Third write-side slice — the pure transforms that tie the pipeline together (no DB/IO), ported from the .NET PersonsSeedService + ValueRouting: - IdentityInputRow: raw identity_inputs observation shape. - build_profiles: fold the latest-first input stream into one SeedProfile per account — current email, closed (latest is a tombstone), tombstones are signal-only (mirrors AccountAccumulator). - route_value: route a value into value_id / value_full_text / value by value_type; over-limit values dropped, never truncated (ValueRouting). - assignments_to_rows: assignments → SeedObservationRow[], stamped with the resolved person_id + seed author; auto-seed-link reason only for email-linked (BuildObservationRows). SeedObservationRow moved into the domain (domain produces it; seed_repo consumes it). The whole pure pipeline (build → group → resolve → rows) now composes end-to-end; +5 unit tests incl. a full-chain test. Remaining write-side: ClickHouse identity_inputs reader, the async job/service + API, and the org_chart rebuild. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
… trimming (parity) (#1754) normalize_email trimmed whitespace before lowercasing; the .NET seed path never trims (StringComparer.OrdinalIgnoreCase + AccountAccumulator "store as-is"). So two accounts differing only by stray whitespace (e.g. "anna@x" vs "anna@x ") were merged into one person here but stay distinct in .NET — a real identity / account_person_map divergence that would split the Rust and .NET seeders during the strangler migration. Lowercase only (drop trim); callers still treat blank/whitespace-only as "no email" via trim().is_empty() (matching IsNullOrWhiteSpace). Adds a test locking in case-fold-merges / trailing-space-stays-distinct. Found by an independent review pass. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…raits (#1754) Fourth write-side slice — the service that ties the whole pipeline together, ported from the .NET PersonsSeedService: - domain/seed_service.rs: IdentityInputsReader + SeedStore traits (input source and store abstracted for testability), and run_seed(): stream → build_profiles → group_by_email → resolve_assignments → assignments_to_rows → store.apply, returning a SeedSummary. Unit-tested end-to-end with fakes (no ClickHouse/DB): a multi-source mint run and a known-binding reuse run. - infra/db/seed_repo.rs: MariaDbSeedStore impl of SeedStore over the real pool. Next: the concrete ClickHouse identity_inputs reader (just impl IdentityInputsReader) and the async job/API that call run_seed. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Fifth write-side slice — the concrete input edge (infra/identity_inputs.rs),
query ported verbatim from the .NET ClickHouseIdentityInputsReader:
- ClickHouseIdentityInputsReader impls IdentityInputsReader over the shared
insight-clickhouse client. SELECT toString(insight_source_id)/… FROM
identity.identity_inputs WHERE tenant AND operation_type IN ('UPSERT','DELETE')
AND value non-empty, ORDER BY … _synced_at DESC (latest-first per account, as
build_profiles requires). is_delete = operation_type == 'DELETE'.
- String→Uuid + tolerant CH-datetime parse (fractional / plain); connect() from
url/db/user/password; clickhouse_* config fields.
NOT yet verified against a live ClickHouse (query text / column names / bind
semantics mirror .NET; the datetime parser is unit-tested; the live stream test
is env-gated and skips). Verify with a dev-ClickHouse port-forward before wiring.
Remaining write-side: the async job/API (operations + worker + /v1/persons-seed
→ run_seed) and the org_chart rebuild.
Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Sixth write-side slice — infra/db/ops_repo.rs, the operations audit/job table, SQL ported from the .NET Sql.Operations.cs. An operation moves queued → running → completed/failed: - OperationStatus enum (+ DB string mapping) and Operation row struct. - enqueue (insert queued), try_start (atomic queued→running so two workers can't double-run; returns whether this call won), complete (summary), fail (error), get_by_id, list (by tenant, optional status filter, newest first). Backs the persons-seed async job; wired by the API/worker slice next. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Seventh write-side slice — the HTTP surface + async job, ported from the .NET
PersonsSeedEndpoints + PersonsSeedQueue:
- api/seed.rs: POST /v1/persons-seed enqueues an operations row + a job on an
in-process mpsc channel and returns 202; GET /{id} and GET (optional ?status=)
poll. DTOs PersonsSeedRequest / PersonsSeedOperationResponse (From<Operation>)
/ PersonsSeedListResponse. Only mode=link_by_email supported.
- run_worker: spawned once in the gear init (tokio::spawn, like the analytics
validators); drains the queue, wins queued→running via try_start, runs
run_seed with a ClickHouse reader + MariaDbSeedStore, then complete/fail with
the SeedSummary. SeedSummary now Serialize (→ summary_json).
- AppState gains seed_tx; gear init builds the channel, spawns the worker, wires
it. clickhouse_* config surfaced in insight.yaml.
NOT yet verified end-to-end (boot + POST → worker → ClickHouse read → MariaDB
write): unit tests + compile only. Admin-gate not enforced (auth-disabled host);
author is nil (no caller-gate) — both flagged, same deferral as the read side.
Remaining: the org_chart rebuild in apply, and a live e2e.
Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Final write-side logic slice — apply now rebuilds org_chart too, in the same transaction as persons + account_person_map, so all three stay consistent. InsertOrgChartForTenant ported verbatim from Sql.PersonsSeed.cs (state-interval CTEs + parent_email→email edge resolution + Path-B no-parent rows). apply gains author_person_id (stamps the computed no-parent rows); threaded through SeedStore / run_seed / MariaDbSeedStore. The CTE's positional `?` bind in a fixed order: insight_tenant_id six times (state_log, default_active, pe_periods, email_to_person, existing_edges, source_member_latest_active) then author_person_id once — matched by the params vec and documented at both sites. NOT yet verified against live MariaDB — the param order is the silent-corruption risk both reviews flagged; a rolled-back live run + a full seed e2e are the gate before this is trusted. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…th (#1754) Batch the persons observation INSERT (chunk 500) instead of per-row: the 25k-row seed dropped from 20+ min (hung over port-forward latency) to ~21s against dev. Add phase logging across the seed pipeline (input streamed / resolved / applying / applied, plus per-stage repo logs) so long runs are observable. Fix the ClickHouse reader to wrap Nullable(String) columns with ifNull(col,'') — the live schema stores them nullable and the strict decoder rejected them as String. Fix a latent read-side bug the rebuild surfaced: current_parents_for_child selects across all sources and decoded parent_person_id into a non-nullable Uuid, so a single NULL-parent org_chart row (Path-B root/membership rows, #344) 500'd the whole profile. Skip NULL parents in the query — a parent edge with no parent is not an edge. Verified live on dev: seed completes, org tree intact (Serdar -> Tasi -> Gürcan, subordinates expand), all reads 200, no decode errors. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a tenant-scoped ChangesPersons seed workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant PersonsSeedAPI
participant OperationsRepository
participant SeedQueue
participant SeedWorker
participant ClickHouseIdentityInputsReader
participant MariaDbSeedStore
Client->>PersonsSeedAPI: POST /v1/persons-seed
PersonsSeedAPI->>OperationsRepository: enqueue queued operation
PersonsSeedAPI->>SeedQueue: send PersonsSeedJob
SeedWorker->>OperationsRepository: try_start operation
SeedWorker->>ClickHouseIdentityInputsReader: stream tenant identity inputs
SeedWorker->>MariaDbSeedStore: apply resolved observations
SeedWorker->>OperationsRepository: mark completed or failed
Client->>PersonsSeedAPI: request operation status or list
PersonsSeedAPI->>OperationsRepository: query operation records
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
src/backend/services/identity-resolution/src/infra/identity_inputs.rs (1)
85-93: 🚀 Performance & Scalability | 🔵 TrivialFull-result materialization via
fetch_all.
streamloads the entire filteredidentity_inputsresult set into aVecbeforebuild_profilesconsumes it. That is fine at the current scale (~4.7k accounts, ~21s), but it grows linearly with tenant size and could pressure memory for large tenants. Consider row-streaming/chunked reads if bigger tenants are expected.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/identity-resolution/src/infra/identity_inputs.rs` around lines 85 - 93, Update IdentityInputs’ stream method to avoid materializing all query results with fetch_all; use the project’s supported row-streaming or chunked-fetch API and map rows incrementally into IdentityInputRow values while preserving tenant filtering and error propagation.src/backend/services/identity-resolution/src/infra/db/ops_repo.rs (1)
201-222: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
listfetches all operation types for the tenant, not just persons-seed.The query filters only on
insight_tenant_id/status, with nooperation_typepredicate and noLIMIT. The only current caller (list_persons_seedinsrc/api/seed.rs) fetches every operation row for the tenant/status and then filters toPERSONS_SEED_OPin application code — wasted I/O that grows as other job kinds (e.g. analytics jobs) accumulate rows in the sharedoperationstable.♻️ Push the type filter (and a page cap) into SQL
pub async fn list( db: &DatabaseConnection, tenant_id: Uuid, + operation_type: Option<&str>, status: Option<OperationStatus>, ) -> anyhow::Result<Vec<Operation>> { let mut sql = format!("SELECT {COLUMNS} FROM operations WHERE insight_tenant_id = ?"); let mut params: Vec<sea_orm::Value> = vec![tenant_id.as_bytes().to_vec().into()]; + if let Some(t) = operation_type { + sql.push_str(" AND operation_type = ?"); + params.push(t.into()); + } if let Some(s) = status { sql.push_str(" AND status = ?"); params.push(s.as_db().into()); } - sql.push_str(" ORDER BY started_at DESC"); + sql.push_str(" ORDER BY started_at DESC LIMIT 200");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/identity-resolution/src/infra/db/ops_repo.rs` around lines 201 - 222, Update the operations query in list and its caller list_persons_seed so SQL filters by the persons-seed operation type and applies an appropriate LIMIT before fetching rows. Preserve the existing tenant_id and optional status predicates, and remove the redundant application-side filtering of non-PERSONS_SEED_OP rows.src/backend/services/identity-resolution/src/gear.rs (1)
36-49: 🩺 Stability & Availability | 🔵 TrivialConsider a startup sweep for orphaned
queuedoperations.The job queue lives only in-process (
mpsc::channel); a row can becomequeuedin the DB without ever landing in a live channel (crash betweenops_repo::enqueueandseed_tx.try_sendincreate_persons_seed, or a pod restart while jobs are in flight). Sincetry_start's atomic transition is designed for multi-pod safety, this gap is more likely to bite at scale. Worth considering a reconciliation pass on gearinitthat requeues stalequeuedrows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend/services/identity-resolution/src/gear.rs` around lines 36 - 49, During gear initialization, add a startup reconciliation pass for stale database operations remaining in the queued state, including rows left behind by crashes or pod restarts. Invoke this sweep from the initialization flow before or alongside spawning the run_worker task, and reuse the existing operation repository/status-transition APIs so recovered rows are safely requeued for processing across pods.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/services/identity-resolution/src/api/seed.rs`:
- Around line 283-288: Sanitize the failure message persisted by the
`ops_repo::fail` call in the `run_seed` error branch instead of passing
`e.to_string()`, so `GET /v1/persons-seed/{id}` and list responses expose only a
safe generic error. Preserve the detailed `tracing::error!` log for diagnostics
and keep the existing failure-update error logging unchanged.
- Around line 258-265: Update the Err branch of the ops_repo::try_start match in
create_persons_seed to explicitly mark the operation failed using
ops_repo::fail, matching the existing channel-full error path, before logging
and continuing. Ensure the consumed queued job cannot remain stranded after a
transient try_start error.
- Around line 267-274: Wrap the await of run_seed in run_worker’s single-worker
job loop with the established async timeout mechanism, using an appropriate
finite duration. Handle timeout expiration as a failed seed attempt, log the
timeout with job context, and ensure the loop continues processing subsequent
queued jobs rather than remaining blocked indefinitely.
In `@src/backend/services/identity-resolution/src/domain/seed.rs`:
- Around line 158-173: Update the known-binding branch in the group-processing
loop to inspect all entries in known rather than selecting the first match with
find_map. Detect when profiles in one group are bound to different person IDs,
emit an appropriate conflict log, and increment or add a distinct conflict
counter while preserving the existing assignment behavior for non-conflicting
groups.
In `@src/backend/services/identity-resolution/src/infra/db/seed_repo.rs`:
- Around line 186-196: Update the batched insert path using INSERT_PREFIX and
ROW_TUPLE so valid observations cannot be silently discarded by INSERT IGNORE;
remove IGNORE for this path and propagate insert errors, or validate value_type
and insight_source_type lengths against their database limits before batching.
Preserve idempotent duplicate handling through an explicit duplicate-safe
mechanism rather than suppressing unrelated truncation warnings.
---
Nitpick comments:
In `@src/backend/services/identity-resolution/src/gear.rs`:
- Around line 36-49: During gear initialization, add a startup reconciliation
pass for stale database operations remaining in the queued state, including rows
left behind by crashes or pod restarts. Invoke this sweep from the
initialization flow before or alongside spawning the run_worker task, and reuse
the existing operation repository/status-transition APIs so recovered rows are
safely requeued for processing across pods.
In `@src/backend/services/identity-resolution/src/infra/db/ops_repo.rs`:
- Around line 201-222: Update the operations query in list and its caller
list_persons_seed so SQL filters by the persons-seed operation type and applies
an appropriate LIMIT before fetching rows. Preserve the existing tenant_id and
optional status predicates, and remove the redundant application-side filtering
of non-PERSONS_SEED_OP rows.
In `@src/backend/services/identity-resolution/src/infra/identity_inputs.rs`:
- Around line 85-93: Update IdentityInputs’ stream method to avoid materializing
all query results with fetch_all; use the project’s supported row-streaming or
chunked-fetch API and map rows incrementally into IdentityInputRow values while
preserving tenant filtering and error propagation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bc69a915-7ca2-48df-8e61-6590943ef183
⛔ Files ignored due to path filters (1)
src/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
src/backend/services/identity-resolution/Cargo.tomlsrc/backend/services/identity-resolution/config/insight.yamlsrc/backend/services/identity-resolution/src/api/error.rssrc/backend/services/identity-resolution/src/api/mod.rssrc/backend/services/identity-resolution/src/api/seed.rssrc/backend/services/identity-resolution/src/config.rssrc/backend/services/identity-resolution/src/domain/mod.rssrc/backend/services/identity-resolution/src/domain/seed.rssrc/backend/services/identity-resolution/src/domain/seed_service.rssrc/backend/services/identity-resolution/src/gear.rssrc/backend/services/identity-resolution/src/infra/db/mod.rssrc/backend/services/identity-resolution/src/infra/db/ops_repo.rssrc/backend/services/identity-resolution/src/infra/db/persons_repo.rssrc/backend/services/identity-resolution/src/infra/db/seed_repo.rssrc/backend/services/identity-resolution/src/infra/identity_inputs.rssrc/backend/services/identity-resolution/src/infra/mod.rs
…1754) - worker: mark the operation `failed` on a transient `try_start` error so the already-consumed job can't strand the row in `queued` forever. - worker: bound each `run_seed` with a 10-min timeout — the serial worker would otherwise wedge the whole queue (all tenants) on a hung DB call. - worker: persist a generic `error_message` ("see server logs") instead of the raw anyhow/driver text, which the GET/list endpoints return verbatim. - resolver: detect + count (`known_binding_conflicts`, surfaced in the summary) + warn-log when an email group's accounts are already bound to multiple persons, instead of silently merging identities; add a unit test. - ops_repo::list: push the `operation_type` filter and a `LIMIT 200` page cap into SQL rather than scanning the shared table and filtering in app code. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Contributor
Author
|
Addressed the CodeRabbit review in d71e29a. Fixed (5):
Declined, with rationale:
|
… author (#1754) Port the .NET `CallerAdminCheck` + `HeaderCallerContext` (header branch) to the persons-seed endpoint, replacing the `Uuid::nil()` author stub: - resolve the caller from the `X-Insight-Person-Id` header (present, parseable, non-nil); JWT id/email-claim fallbacks are deferred until gears auth carries a subject (auth-disabled host today). - require an active `admin` role in the tenant via a new `roles_repo` against the shared `person_roles` table (parity with `Sql.Roles.cs::HasActivePersonRole`). - map failures to the .NET status codes: 401 (no caller) via `unauthenticated`, 403 (not admin) via `permission_denied`. - record the resolved caller as the author of the operation and every seeded observation, instead of the nil placeholder. Unit tests for `resolve_caller` header parsing. Verified live on dev: no header → 401, non-admin → 403, admin → 202 with the operation's author = the caller. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…1754) Second-agent review found the async-operation wire contract had drifted from the .NET service, which would break existing callers and the read-side contract tests during cutover. Bring it back to parity: Contract: - accept mode "link-by-email" (hyphen), not "link_by_email" — the documented value was being rejected with 400. - response: add insight_tenant_id + author_person_id; surface request/summary as parsed JSON objects (not double-encoded strings) via ParseOrNull; emit nulls (the .NET serializer does not drop them). - ISO-8601 timestamps with a `T` separator (NaiveDateTime::to_string used a space, breaking ISO parsers). - summary field names match PersonsSeedSummary (accounts_* prefix, accounts_minted_new) and add org_chart_rows_rebuilt (threaded out of apply); known_binding_conflicts kept as an additive Insight field. Behavior: - startup zombie sweep: fail queued/running rows older than 1h so a pod restart can't strand them (parity with PersonsSeedWorker.SweepZombiesAsync). - queue-full returns 503 (service_unavailable), not 500. - 202 sets Location: /v1/persons-seed/{id} and builds the body from the in-memory snapshot (always "queued") instead of re-reading the row. - list: unknown ?status is ignored (not 400), ?limit honored (default 50, cap 500), ORDER BY started_at DESC, operation_id DESC, and next_cursor in the response shape. Nits: fix misleading "trim + lowercase" doc, queue capacity 32→100, explicit TODO that the caller header must come from authn before prod. Verified live on dev: mode gate, 202 body + Location, GET summary shape, list filtering/limit, all match .NET. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ader (#1754) Second review pass on the write path: - admin-gate GET /v1/persons-seed/{id} and GET /v1/persons-seed (extracted a shared `require_admin`). Previously only POST was gated, so any tenant user could read every operation's author/request/summary/error. Parity with the .NET service, which gates all three routes. - run the admin gate BEFORE mode validation in POST, so an unauthenticated / non-admin caller gets 401/403, not 400 (parity with .NET ordering). - ClickHouse reader: coerce every text column to a non-null String via ifNull(col, '') and give each a DISTINCT alias (val, op_type, …). The columns have mixed nullability (insight_source_type is String, source_account_id is Nullable), so a per-column Option is wrong; a same-name alias would also shadow the WHERE columns and risk a "Cyclic aliases" error. Matches the .NET reader's distinct-alias approach. - raise the reader's ClickHouse query timeout 30s -> 5m so a full-tenant scan can't hit the client default; full row-streaming deferred to #1753. - audit-log persons_seed.enqueue on success (parity with .NET). - document the intentional NULL-parent divergence in current_parents_for_child; use .first() instead of a panicking index in the resolver; refresh stale module docs. Verified live on dev: seed completes via the reader; GET/list require admin (401 without the header); bogus mode without a caller returns 401 not 400. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
cyberantonz
reviewed
Jul 22, 2026
cyberantonz
reviewed
Jul 22, 2026
cyberantonz
reviewed
Jul 22, 2026
cyberantonz
reviewed
Jul 22, 2026
…ersons GET (#1754) Address @cyberantonz review on PR #1825: - Admin gate no longer reads the `X-Insight-Person-Id` header (deprecated / removed — the gateway JWT is the source of truth). `require_admin` now takes the caller from `SecurityContext::subject_id()` (verified by the host authn pipeline, NGINX_BFF R1); nil subject → 401. Drops `resolve_caller` + the header constant + its tests. - Remove the deprecated `GET /v1/persons/{email}` endpoint (callers migrated to POST /v1/profiles). Prunes the now-dead `get_person_by_email` handler, `PersonError`, and `persons_repo::resolve_person_id_by_email`; the shared subordinate machinery (used by POST /v1/profiles) is untouched. clippy clean, 32 tests. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
cyberantonz
reviewed
Jul 22, 2026
…eed list (#1754) @cyberantonz review: the field was .NET ListResponse parity but never populated (no cursor pagination) and the .NET side is being retired — leaner {items} shape. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…not toolkit-db) (#1754) @cyberantonz asked whether the raw queries could use toolkit-db. They can't: window functions (ROW_NUMBER/LEAD), WITH RECURSIVE, and atomic conditional DML (NOT EXISTS / JOIN-count guards) have no toolkit-db builder form, and the crate exposes no raw-SQL path (v0.8.4). Central rationale + injection-safety note in infra::db module docs; per-repo markers in persons/seed/ops. Filed the capability request: constructorfabric/gears-rust#4239. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
mozhaev-dev
force-pushed
the
feat/identity-resolution-write-seed
branch
from
July 22, 2026 09:32
af9d6fa to
7a15ee2
Compare
cyberantonz
reviewed
Jul 22, 2026
… the SQL (#1754) @cyberantonz: explanatory prose belongs in the Rust doc, not inside the SQL string — moved the parent_person_id IS NOT NULL rationale to the current_parents_for_child doc comment. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…uild SQL (#1754) @cyberantonz: no prose inside SQL strings. The Path-B rationale is already in the Rust comments around the org_chart rebuild (bind-order note). Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ref in seed_repo (#1754) Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
mitasovr
approved these changes
Jul 23, 2026
cyberantonz
approved these changes
Jul 23, 2026
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rust port of the identity write path — the persons-seed job — on gears-rust. Second slice of the identity rewrite (epic #1602), on top of the merged read API (#1745 / #1752).
What this adds
POST /v1/persons-seed(202 + operation id),GET /v1/persons-seed/{id},GET /v1/persons-seed— job-tracked via theoperationstable.identity_inputsreader → fold to per-account profiles → group by email → 4-branch resolution (known-account / email-link / mint / skip) → transactional MariaDB apply (batchedINSERT IGNOREintopersons→account_person_mapSCD2 rebuild →org_chartrebuild).Live-verified on dev
Seeded the dev tenant end-to-end:
accounts_read=4736, reused=3966, linked=7, minted=634, observations_inserted=25059, completes in ~21s (batched INSERT; per-row was 20+ min). Org tree intact after rebuild (Serdar → Tasi → Gürcan, subordinates expand).Read-side fix included
The live rebuild surfaced a latent read bug:
current_parents_for_childdecodedparent_person_idinto a non-nullableUuid, so a single NULL-parentorg_chartrow (Path-B root/membership rows, #344) 500'd the whole profile. Fixed by skipping NULL parents in the query — a parent edge with no parent is not an edge.Notes
identity-resolutionis not yet a registered CI component, so it is not built/gated here; component registration + integration tests land with identity-resolution: API contract / e2e tests (read endpoints) #1753.PersonsSeedService/Sql.OrgChart.cs.Refs #1602, #1754
Summary by CodeRabbit
persons-seedadmin workflow withPOST /v1/persons-seed,GET /v1/persons-seed/{id}, andGET /v1/persons-seedto manage queued/running/completed/failed operations.mode(defaults to link-by-email), returns202 Accepted, and provides operation status; background processing reclaims stale jobs and enforces timeouts.GET /v1/persons/{email}endpoint.