MedcareBridge = UnifiedBridge<HealthcarePort> + FieldMask role projection + Fisher-z clamp fix#582
Conversation
Collapse the last bespoke per-tenant bridge that #570 deferred. With Healthcare now promoted into the OGAR codebook (0x09XX) + a HealthcarePort PortSpec, MedcareBridge becomes a type alias over the generic UnifiedBridge harness — same way OpenProject/Redmine collapsed. - medcare_bridge.rs: drop the bespoke struct + hand-written NamespaceBridge/BridgeFromRegistry impls; `type MedcareBridge = UnifiedBridge<HealthcarePort>`. NAMESPACE mirrors HealthcarePort::NAMESPACE. 8 co-located tests mirroring openproject_bridge::tests (constructor ok/err, bridge_id="medcare", g_lock, Patient->0x0901 codebook synth, seeded ctx_id=2, per-alias resolution, non-codebook fallback to registry). - mod.rs: re-export HealthcarePort; move MedcareBridge from the "legacy struct" list to the OGAR-driven-ports list. - Cargo.toml: repoint ogar-vocab git dep to the claude/medcare-bridge-lance-graph-wmx76z OGAR branch (carries HealthcarePort + the Health codebook). Cargo.lock updated. - openproject_bridge_scope_lock.rs: fix a #570 latent defect — the construction-failure test formatted a Result<Bridge, Error> with {:?} but UnifiedBridge<P> is intentionally not Debug; assert is_err() without formatting the Ok value. Authorization/audit path is unaffected: it resolves via row() (registry-backed, not overridden by UnifiedBridge), so codebook synthesis on entity() doesn't change canonical-name keying. 295 ontology lib tests + integration + consumer-conformance E1 (medcare_bridge_conforms) green; clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
…eads ~1 `Distance::similarity_z` clamps the similarity away from ±1 to keep the `atanh` (the `ln` term) finite. The bound was ±0.999, which made `tanh(atanh(0.999)) = 0.999` the maximum value `cohort_similarity_z` could ever return — so a perfect self-match read ~0.99899, and any "self ≈ 1.0" assertion (`s > 0.999`) was unreachable. ±0.9999 keeps atanh finite (≈4.95) while letting a self-match round-trip to ≈0.99986, which reads as "essentially identical". Pure numerical guard, no semantic change for moderate similarities; the existing distance tests (s=0.8 roundtrip, z=0.5 averaging, sign-only checks) are unaffected. Fixes medcare-analytics graph_contract::cohort_similarity_z_ self_returns_one (latent — the lance-phase2-rbac suite had never compiled in the dev env until protoc was installed this session). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
… depth levels RBAC is classid :: role :: membership, where the role IS a distinct projection of the class — not a graduated access level. PermissionSpec already had max_depth (a scalar level: Identity < … < Full), which only expresses more-vs-less of the same fields. It could not express that two roles see DISJOINT views of one class — the actual HIPAA mechanism (health-personnel sees the clinical histogram; invoice sees billing fields; research sees de-identified aggregate — and that the research cross-correlation would be unlawful in the invoice purpose). - contract FieldMask: add FULL (all positions), intersect, is_disjoint — the ops a projection + a distinctness check need. - rbac PermissionSpec: add `projection: FieldMask` (default FULL = no narrowing, depth governs), `with_projection(mask)` builder, `projects(n)` accessor. The projection is resolved against the class's ClassView field basis (lance-graph-ogar pulls OgarClassView for that basis). The projection SLOT is reusable here; the consumer (medcare-rs) hand-rolls the distinctness ENFORCEMENT — the three clinical roles' masks, and the invariant that the research projection is disjoint from the identifier fields. Test: two same-depth roles on one class with disjoint projections. lance-graph-rbac 15 tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
📝 WalkthroughWalkthroughThree functional areas are updated: ChangesFieldMask Primitives and PermissionSpec Projection
MedcareBridge Type Alias Migration
Minor Fixes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b35dc0c6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| /// MedCare `NamespaceBridge` — alias over the generic harness, locked to | ||
| /// the `Healthcare` namespace via [`HealthcarePort`]. | ||
| pub type MedcareBridge = UnifiedBridge<HealthcarePort>; |
There was a problem hiding this comment.
When this alias opts MedCare into UnifiedBridge's codebook entity() path, Healthcare aliases such as Diagnosis can resolve from HealthcarePort even when the registry only has the namespace. The existing authorization/audit path in lance-graph-callcenter calls NamespaceBridge::row(public_name), and the default row() still requires an actual registry row with bridge_id == "medcare"; normal TTL hydration files rows under "ogit", and the new fixture only seeds Patient. In that setup the advertised codebook aliases resolve via entity() but fail before policy evaluation or audit, so either synthesize/override row() for these aliases or seed medcare rows for every alias.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, and confirmed concretely: TTL hydration files mapping rows under bridge_id = "ogit" (registry.rs:139), while the default NamespaceBridge::row() filters by the bridge's own id ("medcare") — per-bridge rows come from a separate append_mapping step, not hydration. So entity() (codebook synthesis) and row() (registry-backed, the authz/audit path) can diverge for a codebook alias.
But this is a property of the shared UnifiedBridge<P> harness (lance-graph#570), not introduced by this PR: MedcareBridge is literally type MedcareBridge = UnifiedBridge<HealthcarePort>, identical to the already-merged OpenProjectBridge/RedmineBridge — none of the three override row(). The minimal fixture seeds only Patient via TTL to exercise namespace registration + entity() resolution + the not-in-scope fallback, not the full authz path.
The real fix — row() synthesizing from the codebook when no per-bridge mapping row exists — belongs in UnifiedBridge<P> (it would cover all three bridges) and touches the authz/audit path, so it's a deliberate harness change rather than something to slip into this alias PR (a type alias can't override row() anyway). Leaving this thread open as the tracking item — happy to do the harness change as a follow-up if you'd like.
Generated by Claude Code
…-lance-graph-wmx76z # Conflicts: # Cargo.lock
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/lance-graph-ontology/src/bridges/medcare_bridge.rs`:
- Around line 79-88: The test helper function has a resource leak caused by
`std::mem::forget(tmp)` which prevents the temporary directory from being
cleaned up. Determine whether the registry internalized the data during
`hydrate_once_sync` call: if yes, simply remove the `std::mem::forget(tmp)` line
and let the TempDir drop naturally when the function returns; if the files must
persist, change the function signature to return a tuple containing both the
Arc<OntologyRegistry> and the TempDir, then update all callers to destructure
the tuple and hold the TempDir guard for the test's lifetime to keep the
directory alive.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 12c691aa-5d0f-49ea-b840-23c1136699a8
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
crates/lance-graph-contract/src/class_view.rscrates/lance-graph-contract/src/distance.rscrates/lance-graph-ontology/Cargo.tomlcrates/lance-graph-ontology/src/bridges/medcare_bridge.rscrates/lance-graph-ontology/src/bridges/mod.rscrates/lance-graph-ontology/tests/openproject_bridge_scope_lock.rscrates/lance-graph-rbac/src/permission.rs
CodeRabbit (PR #582): the test helper leaked a temp dir via std::mem::forget(tmp) on every call. hydrate_once_sync parses the TTL into the in-memory OntologyRegistry, so the dir isn't needed afterward — let it drop. medcare_bridge tests 8/8 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
2 participants
What
Three additive changes to the reusable spine (contract + rbac + ontology bridges), all keeping lance-graph agnostic of anything medical.
Commits
ddb6c84— collapse the last bespoke bridge:MedcareBridge = UnifiedBridge<HealthcarePort>(a generic alias over the published harness). Conformance test + 295 ontology tests green.e1012ae—fix(contract): loosen the Fisher-z clamp±0.999 → ±0.9999inDistance::similarity_zso a perfect self-match round-trips to ≈0.99986 (the±0.999bound made "self ≈ 1.0" mathematically unreachable). Pure numerical guard; existing distance tests unaffected.8b35dc0—feat(rbac): a role carries aFieldMaskprojection, not a depth level. RBAC isclassid :: role :: membershipwhere the role IS a distinct projection of the class — not graduated access. AddsFieldMask::{FULL, intersect, is_disjoint}(contract) +PermissionSpec.projection/with_projection/projects(rbac, defaultFULL). The projection feeds the existingClassView::render_rows, so the field basis is pulled vialance-graph-ogar'sOgarClassViewwith no new code. Consumers hand-roll the distinctness enforcement; the slot is reusable here.Tests
lance-graph-contract715 + 7 + 8 ·lance-graph-rbac15 (incl. a disjoint-projections test) ·lance-graph-ontology295 + conformance · clippy clean.Coordinated with
OGAR#<Health domain>— providesHealthcarePort+ the class field basis.medcare-rs#<HIPAA RBAC>— hand-rolls the clinical-role distinctness on top ofPermissionSpec.projection.🤖 Generated with Claude Code
https://claude.ai/code/session_01EYvNjD8M8LMNYbRy3gq2FP
Generated by Claude Code
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Chores