feat(identity-resolution): Rust port of the identity read API (POST /v1/profiles + GET /v1/persons) - #1745
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between c4c02374332afc69fb6d16c6cb5abc989798d9fc and c9230dc. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (24)
🚧 Files skipped from review as they are similar to previous changes (21)
📝 WalkthroughWalkthroughChangesIdentity resolution service
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant API Gateway
participant Identity Resolution
participant MariaDB
Client->>API Gateway: POST /v1/profiles
API Gateway->>Identity Resolution: Forward authenticated request
Identity Resolution->>MariaDB: Resolve identity and load org data
MariaDB-->>Identity Resolution: Return observations and relationships
Identity Resolution-->>API Gateway: Return profile or problem response
API Gateway-->>Client: HTTP response
Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
|
You need to use Toolkit DB https://github.com/constructorfabric/gears-rust/tree/main/libs/toolkit-db to standartize access to databases. I missed it in analytics service. |
mozhaev-dev
changed the title
![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/domain/profile.rs (1)
166-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated parent-projection mapping.
The
(supervisor_email, supervisor_name, parent_email, parent_id, parent_person_id)destructuring fromOption<ParentProjection>is identical inassemble_profileandassemble_person. Extract a shared helper to avoid the two copies drifting.♻️ Proposed extraction
+fn project_parent( + parent: Option<ParentProjection>, +) -> (Option<String>, Option<String>, Option<String>, Option<String>, Option<Uuid>) { + match parent { + Some(p) => ( + p.email.clone(), + p.display_name, + p.email, + p.source_native_id.and_then(non_blank), + Some(p.person_id), + ), + None => (None, None, None, None, None), + } +}Then call
project_parent(parent)in bothassemble_profileandassemble_personin place of the inlinedmatch.Also applies to: 222-232
🤖 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/domain/profile.rs` around lines 166 - 176, Extract the duplicated Option<ParentProjection> mapping into a shared project_parent helper, preserving the existing tuple values and None behavior. Replace the inline match in both assemble_profile and assemble_person with calls to project_parent(parent), using the helper’s return type to support the existing destructuring.src/backend/services/identity-resolution/src/infra/db/persons_repo.rs (1)
179-364: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftPer-node repo calls enable N+1 fan-out for org-chart hydration.
fetch_person_observations,current_source_ids_for_person,current_parents_for_child, andcurrent_children_for_parentare single-person/single-edge queries. Per the handlers.rs context snippets,resolve_parent/hydrate_childrencall these once per node while recursively expanding subordinates (bounded bymax_depth, but not by breadth). For wide org trees this means several DB round-trips per node.Consider adding a batched variant (e.g., accepting
&[Uuid]of person ids) that callers in the HTTP layer could use to fetch observations/source-ids/edges for a whole tree level in one query.🤖 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/persons_repo.rs` around lines 179 - 364, The repository functions fetch_person_observations, current_source_ids_for_person, current_parents_for_child, and current_children_for_parent currently require one database round-trip per person or edge, causing N+1 fan-out during recursive org-chart hydration. Add batched variants accepting a slice of person IDs (and preserving tenant/source/current-row filtering and ordering) so handlers can fetch each tree level’s observations, source IDs, parent edges, and child edges in bulk; update the HTTP-layer callers such as resolve_parent and hydrate_children to use the batched APIs.src/backend/services/identity-resolution/Cargo.toml (1)
54-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove
utoipato the workspace dependency table. It’s pinned directly here instead of usingworkspace = true; centralizing it insrc/backend/Cargo.tomlwould keep thechrono/uuidfeatures consistent across the workspace.🤖 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/Cargo.toml` at line 54, Move the utoipa dependency definition from the identity-resolution crate’s Cargo.toml into the workspace dependency table in src/backend/Cargo.toml, preserving the 5.5 version and chrono/uuid features there. Update the crate’s utoipa entry to use workspace = true.
🤖 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/README.md`:
- Around line 8-9: Update the README’s “Current state” description to remove the
WIP designation for GET /v1/persons/{email} and POST /v1/profiles, reflecting
that both endpoints are fully implemented with their current behavior.
In `@src/backend/services/identity-resolution/src/api/handlers.rs`:
- Around line 289-359: The recursive hydration path has an N+1 query pattern
because hydrate_person re-fetches parent data through resolve_parent for every
node and hydrate_children fetches child edges per person. Refactor
hydrate_children and hydrate_person to reuse the already-known parent projection
when descending, and batch child edge and required observation/source-id queries
per tree level where practical, while preserving cycle detection, depth limits,
source filtering, query order, and existing response assembly.
In `@src/backend/services/identity-resolution/src/domain/profile.rs`:
- Around line 28-74: The ProfileResponse documentation comment is stale and
incorrectly says ids and the org tree are future work. Update the comment above
ProfileResponse to describe the currently supported attributes,
organization-tree fields, recursive subordinates, and source-native ids, while
preserving the existing null-omission behavior.
In `@src/backend/services/identity-resolution/src/gear.rs`:
- Around line 33-34: Migrate gear initialization in
src/backend/services/identity-resolution/src/gear.rs lines 33-34 to consume the
toolkit-db capability instead of calling crate::infra::db::connect or managing a
local SeaORM pool; update
src/backend/services/identity-resolution/src/infra/db/mod.rs lines 1-26 to
follow toolkit-db conventions or remove the module if no longer needed, and
adjust imports and consumers accordingly.
In `@src/backend/services/identity-resolution/src/infra/db/persons_repo.rs`:
- Around line 384-410: Replace the real-looking email literal in
resolve_by_email_against_dev_db with a clearly synthetic test address, while
preserving the existing lookup and assertion behavior for the known-email case.
---
Nitpick comments:
In `@src/backend/services/identity-resolution/Cargo.toml`:
- Line 54: Move the utoipa dependency definition from the identity-resolution
crate’s Cargo.toml into the workspace dependency table in
src/backend/Cargo.toml, preserving the 5.5 version and chrono/uuid features
there. Update the crate’s utoipa entry to use workspace = true.
In `@src/backend/services/identity-resolution/src/domain/profile.rs`:
- Around line 166-176: Extract the duplicated Option<ParentProjection> mapping
into a shared project_parent helper, preserving the existing tuple values and
None behavior. Replace the inline match in both assemble_profile and
assemble_person with calls to project_parent(parent), using the helper’s return
type to support the existing destructuring.
In `@src/backend/services/identity-resolution/src/infra/db/persons_repo.rs`:
- Around line 179-364: The repository functions fetch_person_observations,
current_source_ids_for_person, current_parents_for_child, and
current_children_for_parent currently require one database round-trip per person
or edge, causing N+1 fan-out during recursive org-chart hydration. Add batched
variants accepting a slice of person IDs (and preserving
tenant/source/current-row filtering and ordering) so handlers can fetch each
tree level’s observations, source IDs, parent edges, and child edges in bulk;
update the HTTP-layer callers such as resolve_parent and hydrate_children to use
the batched APIs.
🪄 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: 2208562e-eb53-48a4-a95a-ce88fcb2e127
📥 Commits
Reviewing files that changed from the base of the PR and between b3ecca9 and 9c3599e8048ba447d905189a03d4d296271a91cf.
⛔ Files ignored due to path filters (1)
src/backend/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
src/backend/Cargo.tomlsrc/backend/services/identity-resolution/Cargo.tomlsrc/backend/services/identity-resolution/README.mdsrc/backend/services/identity-resolution/config/insight.yamlsrc/backend/services/identity-resolution/src/api/canonical_json.rssrc/backend/services/identity-resolution/src/api/error.rssrc/backend/services/identity-resolution/src/api/handlers.rssrc/backend/services/identity-resolution/src/api/mod.rssrc/backend/services/identity-resolution/src/auth.rssrc/backend/services/identity-resolution/src/config.rssrc/backend/services/identity-resolution/src/domain/mod.rssrc/backend/services/identity-resolution/src/domain/profile.rssrc/backend/services/identity-resolution/src/gear.rssrc/backend/services/identity-resolution/src/infra/db/entities/account_person_map.rssrc/backend/services/identity-resolution/src/infra/db/entities/mod.rssrc/backend/services/identity-resolution/src/infra/db/entities/persons.rssrc/backend/services/identity-resolution/src/infra/db/mod.rssrc/backend/services/identity-resolution/src/infra/db/persons_repo.rssrc/backend/services/identity-resolution/src/infra/mod.rssrc/backend/services/identity-resolution/src/main.rs
Comment on lines
+289
to
+359
| fn hydrate_children<'a>( | ||
| state: &'a AppState, | ||
| tenant: Uuid, | ||
| person_id: Uuid, | ||
| depth: usize, | ||
| visited: &'a mut HashSet<Uuid>, | ||
| ) -> Pin<Box<dyn Future<Output = Result<Vec<PersonResponse>, CanonicalError>> + Send + 'a>> { | ||
| Box::pin(async move { | ||
| if !state.config.expand_subordinates || depth >= state.config.max_depth { | ||
| return Ok(Vec::new()); | ||
| } | ||
| let source_type = &state.config.org_chart_source_type; | ||
| let edges = persons_repo::current_children_for_parent(&state.db, tenant, person_id) | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!(error = %e, "fetch child edges failed"); | ||
| CanonicalError::internal("profile assembly failed").create() | ||
| })?; | ||
|
|
||
| // Distinct child ids on the configured source, preserving query order. | ||
| let mut seen = HashSet::new(); | ||
| let child_ids: Vec<Uuid> = edges | ||
| .into_iter() | ||
| .filter(|e| &e.source_type == source_type) | ||
| .map(|e| e.child_person_id) | ||
| .filter(|id| seen.insert(*id)) | ||
| .collect(); | ||
|
|
||
| let mut subordinates = Vec::new(); | ||
| for child_id in child_ids { | ||
| if let Some(node) = hydrate_person(state, tenant, child_id, depth + 1, visited).await? { | ||
| subordinates.push(node); | ||
| } | ||
| } | ||
| Ok(subordinates) | ||
| }) | ||
| } | ||
|
|
||
| /// Build one person node at tree depth `depth`, recursing into its own children. | ||
| /// Returns `None` when the person is already on the current path (cycle guard) | ||
| /// or has no observations. | ||
| fn hydrate_person<'a>( | ||
| state: &'a AppState, | ||
| tenant: Uuid, | ||
| person_id: Uuid, | ||
| depth: usize, | ||
| visited: &'a mut HashSet<Uuid>, | ||
| ) -> Pin<Box<dyn Future<Output = Result<Option<PersonResponse>, CanonicalError>> + Send + 'a>> { | ||
| Box::pin(async move { | ||
| if !visited.insert(person_id) { | ||
| return Ok(None); | ||
| } | ||
| let observations = persons_repo::fetch_person_observations(&state.db, tenant, person_id) | ||
| .await | ||
| .map_err(|e| { | ||
| tracing::error!(error = %e, "fetch subordinate observations failed"); | ||
| CanonicalError::internal("profile assembly failed").create() | ||
| })?; | ||
| if observations.is_empty() { | ||
| return Ok(None); | ||
| } | ||
| let parent = resolve_parent(state, tenant, person_id).await?; | ||
| let subordinates = hydrate_children(state, tenant, person_id, depth, visited).await?; | ||
| Ok(Some(assemble_person( | ||
| person_id, | ||
| observations, | ||
| parent, | ||
| subordinates, | ||
| ))) | ||
| }) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
N+1 query pattern in recursive org-tree hydration.
hydrate_person calls resolve_parent (218-266, 3 DB round trips) for every node, even though hydrate_children already knows each child's parent (itself) — so the parent's observations/source-ids are redundantly re-fetched once per sibling instead of being reused. Combined with the unbatched per-node current_children_for_parent call, total DB round trips scale with tree size × ~4 queries/node, with no caching or batching. For wide/deep org charts this is a latency and DB-load risk on a synchronous request path.
Consider passing the already-resolved parent projection down through hydrate_children/hydrate_person instead of re-deriving it via resolve_parent, and/or batching the observation/source-id/edge fetches per tree level.
🤖 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/api/handlers.rs` around lines
289 - 359, The recursive hydration path has an N+1 query pattern because
hydrate_person re-fetches parent data through resolve_parent for every node and
hydrate_children fetches child edges per person. Refactor hydrate_children and
hydrate_person to reuse the already-known parent projection when descending, and
batch child edge and required observation/source-id queries per tree level where
practical, while preserving cycle detection, depth limits, source filtering,
query order, and existing response assembly.
Contributor
Author
There was a problem hiding this comment.
Acknowledged — real and known. The recursive walk mirrors the .NET PersonLookupService.HydrateAsync (which also fans out per node), so it is an implementation/perf concern, not a contract change. Deferring to a perf follow-up rather than reworking the delicate recursion in this read-MVP PR.
Good call on reusing the parent projection while descending — a child's parent is the current node, so it can be passed down instead of re-deriving via resolve_parent. That is a cheaper partial win than full per-level batching and will be the first step. Mitigated operationally today via expand_subordinates: false in insight.yaml.
There was a problem hiding this comment.
@mozhaev-dev, that all makes sense — thanks for the context on the .NET PersonLookupService.HydrateAsync parity, and glad the parent-projection reuse is a good starting point. Passing it down through hydrate_children/hydrate_person is indeed a clean, low-risk win since it eliminates the redundant 3 round-trips for the parent in resolve_parent without touching the trickier per-level batching. The expand_subordinates: false default is a reasonable stopgap for now.
Happy to open a follow-up issue to track the fuller batching work (and the parent-projection reuse if you'd rather land it separately) so it doesn't get lost — just let me know and I can file it with a link back to this thread.
✏️ Learnings added
Learnt from: mozhaev-dev
Repo: constructorfabric/insight PR: 1745
File: src/backend/services/identity-resolution/src/api/handlers.rs:289-359
Timestamp: 2026-07-15T09:10:30.119Z
Learning: In `src/backend/services/identity-resolution/src/api/handlers.rs`, the recursive org-tree hydration (`hydrate_person`/`hydrate_children`) intentionally mirrors the .NET `PersonLookupService.HydrateAsync` per-node fan-out pattern (N+1 queries), as an implementation/perf tradeoff for the read-MVP PR (`#1745`) rather than a contract change. It is operationally mitigated via `expand_subordinates: false` in `config/insight.yaml`. A planned partial improvement is to pass the already-resolved parent projection down through `hydrate_children`/`hydrate_person` instead of re-deriving it via `resolve_parent` for each node; full per-level batching is deferred to a follow-up.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
cyberantonz
approved these changes
Jul 15, 2026
…lth) Iteration-1 scaffold of the Rust port of the .NET identity service (#1602). Minimal gears-rust host: links the no-auth system-gear set and runs toolkit::bootstrap::run_server. No domain gear yet — this milestone only proves the host boots and serves /health. Read endpoints (GET /v1/persons/{email}, POST /v1/profiles) land next. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ility) Wire the identity-resolution domain gear into the gears host (#1602): a #[toolkit::gear(name="identity-resolution", capabilities=[rest])] with a typed GearConfig (database_url), an init that loads config into AppState, and a register_rest that returns the host router unchanged (no domain routes yet). Adds the gears.identity-resolution.config section to insight.yaml so the runtime resolves the gear's config by name. Verified: gear registers, init runs, host boots and serves /health on :8083. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Add a self-managed SeaORM MariaDB pool (same approach as the analytics gear): infra/db::connect opens the pool from GearConfig.database_url; the gear's init stores it in AppState { db, config }. No entities/queries yet. Verified against the dev 'identity' database (log 'connected to MariaDB'); host still serves /health on :8083. (#1602)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
How to run the service against the dev-cluster MariaDB via kubectl port-forward + the APP__gears__... env override, and how to verify /health and the DB connect. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…person_map Generate entities from the live 'identity' MariaDB schema (sea-orm-cli) into infra/db/entities; restructure infra/db.rs -> infra/db/mod.rs. binary(16) UUID columns map to Vec<u8> (raw bytes) — converted to Uuid at the response boundary once the resolve query is wired. Entities not yet used -> module carries #[allow(dead_code)]. (#1602) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
First real read endpoint of the Rust port (#1602). Resolves an identity to a person via the gears HTTP stack, verified end-to-end against the dev 'identity' MariaDB (200 {person_id} for a known email, 404, 400). - infra/db/persons_repo.rs: resolve_person_ids_by_email — raw SQL ported verbatim from .NET Sql.Profiles.cs (ROW_NUMBER latest-per-partition), Vec<u8>->Uuid; env-gated integration test. - api/: OperationBuilder route + resolve_profile handler (email path) + #[resource_error] canonical errors. - auth.rs: tenant_middleware (SecurityContext + X-Insight-Tenant-Id override). - gear.rs: register_rest -> api::register_routes. deps: uuid, toolkit-security, toolkit-canonical-errors, utoipa. Note: gears canonical errors (gts://...) differ from .NET urn:insight:error:*; gears has no 422, so ambiguous maps to 409 (aborted); other statuses match. value_type='id' + full ProfileResponse are follow-ups. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Add resolve_person_ids_by_source_id (raw SQL from .NET Sql.Profiles.cs::ResolvePersonIdsBySourceId, source-instance scoped) + wire the value_type='id' handler arm (needs insight_source_type + insight_source_id -> 400 if missing). Verified vs dev DB: id bamboohr/1026 resolves to the same person as the email path. (#1602) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Replace the minimal {person_id} response with a ProfileResponse carrying the resolved person's attributes (email, display_name, first/last_name, department, division, job_title, status, username, employee_id). fetch_person_observations (SeaORM entity query) loads the person's rows; assemble_profile collapses them to the latest value per value_type (max created_at -> value_effective, per the .NET ProfileAssembler); null fields omitted from JSON. Verified vs dev DB: email and id paths return the same full profile for Serdar. ids[] + org tree are follow-ups. (#1602)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…t tests Self-review before the PR — match the analytics gear and maximise gears usage: - CanonicalJson<T> body extractor (ported from analytics): malformed / wrong Content-Type bodies now return the canonical RFC-9457 envelope, not Axum's plain-text rejection. - Move DTOs + assemble_profile into domain/profile.rs; the api/ handler is now a thin controller (matches analytics + the cf-template api-db-handler layering). - Move AppState from gear.rs to api/mod.rs (as in analytics). - Unit-test assemble_profile (latest-per-value_type, blank/absent handling, attribute mapping) — pure, no DB. Behaviour unchanged (verified vs dev DB: full profile + malformed-body 400 + 415). Adds serde_json dep. (#1602) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
Address safe review findings on POST /v1/profiles: - Deterministic attribute assembly: tie-break latest-per-value_type on (created_at, id), matching the .NET 'created_at DESC, id DESC' (was nondeterministic on equal created_at + unordered fetch). - Request validation (mirrors .NET ResolveProfileCommandValidator): value non-empty (was 404, now 400) and <=320 chars; insight_source_type/insight_source_id must be null for value_type='email' -> 400. - email/display_name always present in JSON (null when absent) — drop skip_serializing_if to match the .NET contract. - Source-native id matched as-is (trim only email, like .NET). Verified vs dev DB (400s fire, happy paths unchanged) + unit test for the tie-break. (#1602) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ds (#1752) Extend POST /v1/profiles toward .NET parity (read API, epic #1602): - ids[]: every current source-native id (value_type='id') per source instance, ported from Sql.Profiles.cs::CurrentSourceIdsForPerson. Always serialized (empty array when none), unlike the omitted-when-null attributes. - Org tree (parent/supervisor): supervisor_email/name + legacy parent_email/parent_id/parent_person_id, all filled from the single org_chart parent edge filtered to the configured source (org_chart_source_type, default bamboohr) — Sql.OrgChart.cs:: CurrentParentsForChild + PersonAssembler. supervisor == parent by design. - Display-name split fallback (DisplayNameSplitter): derive first/last from display_name when neither is observed — parity fix for attributes already shipped. Handler split into resolve_person_ids / resolve_parent helpers; latest_values extracted and reused for the parent projection. New unit tests for ids mapping, parent projection, no-parent nulls, and display-name split (3 forms). Recursive subordinates tracked as step B. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
) Complete the org tree on POST /v1/profiles (read API, epic #1602): - subordinates[]: recursive direct-reports subtree on the configured org_chart source, ported from PersonLookupService.HydrateAsync. Cycle-safe (visited set, root pre-seeded) and depth-capped (max_depth, default 16); gated by expand_subordinates (default true). - current_children_for_parent repo query, verbatim from Sql.OrgChart.cs::CurrentChildrenForParent. - PersonResponse node DTO (attributes are plain strings, empty when absent — not omitted — and supervisor_*/parent_* serialize as null, matching the .NET PersonResponse) + assemble_person (shares latest_values + display-name split). - Mutually recursive hydrate_children / hydrate_person via Box::pin (Rust cannot size recursive async fns otherwise). Config gains expand_subordinates + max_depth. New unit tests for subordinate pass-through and person assembly (empty-string defaults, split, nesting). Note: hydration issues ~5 queries per node (no batching), matching .NET — a candidate for later optimisation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…1752) Restore the deprecated person-lookup endpoint on the Rust service so its live callers (authenticator on the auth path, analytics IdentityClient) keep working after the .NET identity service is retired. - resolve_person_id_by_email: singular resolver (LIMIT 1 by recency), ported verbatim from Sql.cs::ResolvePersonIdByEmail — distinct from the plural profile resolver. - get_person_by_email handler: resolve → hydrate_person(root) → PersonResponse (reuses the step-B org-tree hydration, matching .NET GetByEmailAsync); 404 on no match. Emits RFC 8594 headers (Deprecation + Link successor-version). - PersonError canonical resource type; GET /v1/persons/{email} route (marked deprecated in its summary). - hydrate_children now checks expand_subordinates per level (so the shared hydration honours it for both endpoints, matching .NET). Parity gaps (same as POST, tracked in the PR's open questions): no caller gate (401) and no visibility check yet; deprecation headers on 200 only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ion) latest_values was returning the trimmed value_effective; the .NET assemblers never trim (NullIfBlank / GetValueOrDefault return the value verbatim), so source data with surrounding whitespace was being altered — e.g. " Engineering " served as "Engineering". Keep the raw value; trim is only the emptiness test. Regression from the ids[] refactor, caught in self-review. Adds a regression test locking in verbatim values (blank still collapses to None). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…04, config keys (#1752) Address safe findings from self- and agent review (no parity bugs remained): - Validation order now matches the .NET ResolveProfileCommandValidator declaration order (value_type → value → source cross-field), so the first-error-wins response agrees when several fields are invalid. Adds the "value_type is required" case for an empty value_type. - POST /v1/profiles: return 404 when a resolved person_id has zero observations (matches .NET ProfileLookupService; practically unreachable, defensive). - Surface org_chart_source_type / expand_subordinates / max_depth in config/insight.yaml (previously code-default only). expand_subordinates is the operational kill switch for the recursive subordinates walk (N+1 risk). Verified our config defaults (bamboohr / true / 16) equal the .NET AppOptions production defaults. Deferred (tracked in the PR): error taxonomy + 422→409 and ambiguous body (team decision), subordinates batching (perf follow-up), deprecation headers on error responses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…sponse (#1752) PersonResponse.subordinates is Vec<PersonResponse> (self-referential). utoipa's schema generation recursed infinitely and overflowed the stack at route registration — the service aborted on startup before binding. Not caught by cargo build/test (runtime-only, during REST phase); surfaced by an end-to-end run against the dev cluster DB. Fix: #[schema(no_recursion)] on the recursive field. Affects only the emitted OpenAPI schema (a $ref at the recursion point); the serialized response still carries the full nested subordinates tree. E2E-verified against dev identity MariaDB: POST /v1/profiles (email + id) and GET /v1/persons/{email} return correct attributes, ids[] (multi-source), parent/supervisor, and a recursive subordinates tree; 404/400 map correctly; deprecation headers present; ProfileResponse omits nulls while PersonResponse emits empty strings, matching the two .NET contracts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ail (#1752) CodeRabbit quick wins: - README: drop the stale "WIP" note; the read API (POST /v1/profiles + deprecated GET /v1/persons) is implemented. - ProfileResponse doc: reflect that ids[] and the org tree are present, not "follow-up". - Integration test: read the known email from IDENTITY_TEST_EMAIL instead of hardcoding a real address; the test no longer carries PII or ties to one person. Deferred (tracked): N+1 hydration (perf follow-up) and toolkit-db adoption (open question, gated on the toolkit 0.8 bump). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…ProfileRequest (#1752) Drop the "Command" naming: this endpoint is a read/query, not a command, and the name wrongly implied CQRS/command-bus semantics we don't use. Align with the analytics convention (all request DTOs are `...Request`). Local `cmd` → `req`. Rust-internal rename only — the JSON wire format is unchanged (field names the same); only the OpenAPI schema name changes (ResolveProfileCommand → ResolveProfileRequest). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
…es (#1752) Adding services/identity-resolution to the workspace members broke every other Rust service image build: their Dockerfiles copy each member's Cargo.toml + a stub src individually (dep-cache pattern), and cargo must load ALL workspace member manifests even to build a single --bin. The new member wasn't in that list, so `cargo build --bin analytics` (and api-gateway/authenticator/fakeidp) failed with "failed to load manifest for workspace member services/identity-resolution". Add the identity-resolution Cargo.toml COPY + stub main.rs to the four Rust service Dockerfiles, matching how the other members are handled. Its own image / CI component is separate follow-up work. Signed-off-by: Sergei Mozhaev <mozhaev.dev@gmail.com>
mozhaev-dev
force-pushed
the
feat/identity-resolution-rust-scaffold
branch
from
July 16, 2026 06:29
c4c0237 to
f3bd517
Compare
cyberantonz
approved these changes
Jul 16, 2026
mozhaev-dev
enabled auto-merge
July 16, 2026 10:43
This was referenced Jul 24, 2026
This was referenced Aug 4, 2026
Merged
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
2 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.
Summary
Rust/gears port of the .NET
identityread API (epic #1602). A newservices/identity-resolutiongears service resolves an identity to a canonicalperson and returns their profile, writing against the same
identityMariaDB sothe .NET service can be retired endpoint-by-endpoint (strangler-fig).
Still a draft — the read surface is complete and at .NET parity, but two
things gate a real cutover: the error-taxonomy decision and the caller-gate /
visibility subsystem (both below). Opened for feedback on the approach + those
decisions.
What's implemented
services/identity-resolution(boots,/health, OpenAPI/docs) — same conventions asservices/analytics; self-managed SeaORMMariaDB pool over the shared
identityDB.POST /v1/profiles— resolve byvalue_type=email(all sources) or=id(source-native id within one source instance), then assemble the full profile:
value_type, deterministic(created_at, id)tie-break);
ids[]— every current source-native id, one per source instance;supervisor_*+ legacyparent_*from theorg_chartparentedge on the configured source (
org_chart_source_type, defaultbamboohr),and a recursive
subordinates[]subtree (cycle-safe, depth-capped);display_namewhen unobserved).GET /v1/persons/{email}(deprecated) — restored for its live callers(
authenticator,analytics); single-person resolve + org tree, RFC 8594Deprecation/Linkheaders.Sql.Profiles.cs,Sql.OrgChart.cs,Sql.cs). Canonical RFC-9457 errors;CanonicalJsonbodyextractor; tenant via
X-Insight-Tenant-Id/SecurityContext. Configdefaults match the .NET
AppOptionsproduction defaults.no-trim, determinism) + an env-gated integration test. Reviewed by an
independent agent against the .NET sources; one trim regression found & fixed.
X-Insight-Person-Id(401) and run a
CanSeeAsyncvisibility check (deny → 404, to not leakexistence). This port does neither yet — under the auth-disabled host it
returns any person (and their org tree) to any caller. This is deliberate
(visibility is its own subsystem, iteration 3) but it is the biggest
behavioural gap and must be resolved before the .NET service is retired.
type: "gts://…"; the .NET service emitstype: "urn:insight:error:<code>". gears has no 422, so the .NETambiguous_profile(422) maps toaborted(409), and the ambiguous bodycurrently omits
person_ids[]. Byte-exact parity (overrideProblem.type)or gears-native taxonomy + client migration?
Not in this PR (follow-ups)
node with no batching (parity with .NET, but a real load/latency risk under a
senior manager). Mitigated operationally by
expand_subordinates: false.MockDatabasetests, incl. recursion cycle-guard & depth-cap(identity-resolution: API contract / e2e tests (read endpoints) #1753); deprecation headers on error responses.
Open questions (team)
SeaORM builder — intentional for fidelity. OK?
dbcapability.analyticsself-manages its pool (aClickHouse-driven "LOCKED DECISION"); this service is MariaDB-only, so it is a
good first adopter of
cf-gears-toolkit-db— gated on a workspace-widetoolkit
0.6/0.7 → 0.8bump and a decision on the window-function queries(
SecureConnhides raw SQL). Tracked with identity-resolution: write side — persons seed + persons-seed API #1754.integration tests (no DB in CI). How should the per-crate gate apply here?
How to run
See
services/identity-resolution/README.md(port-forward + env override + curl).Summary by CodeRabbit
POST /v1/profilesfor resolving profiles from identities (latest-value selection, supervisor/parent projection, and optional subordinate expansion).GET /v1/persons/{email}lookup with RFC 8594 deprecation headers.X-Insight-Tenant-Id.application/problem+jsonerror responses.