Repository navigation
D-RMAP-DIR-1: compiled membership closure vs a 256-slot focus over dir-sim (lab only) - #1635
Conversation
…ab only) A lab probe over lance-graph-dir-sim's public API; no production file changes. - fixture: deterministic IAM directory (roles, locations, role x location composite groups nested in both, resources with cycles, licenses, teams, divisions that cut the SID chain, grants, a u8 attribute lane). - B: bidirectional membership CSR + nesting CSR, by dir-sim ordinal. - B+: closure rows per group for every chain (all, security, mail), a fixpoint over the nesting CSR; roles carried by the SID chain per group; incremental nesting insertion. - F: 256-slot focus over selected groups, B+ closure rows projected onto the slots; F+users adds one focus row per user. - D: crepe Datalog oracle over fixture ids. Tests: every arm equals dir-sim's walks and the oracle on every user and group, for all three chains, roles and direct membership; the focus answers role x location like both; incremental nesting equals a recompile after an overlay; stale witnesses and unfocused groups are refused; the fixture is checked for cycles, large groups and filtering chains. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
- examples/bench.rs: W1-W9 for arms A, D, B, B+, F and F+users at 8,000 and 65,000 users; focus sizes 16/64/128/256; 1 to 10,000 evaluations per prepared focus; memory; cold vs warm. Every timed answer is checked against A (and D) first. - Clippy fixes (Rng::next_u64, collapsed if-let); the closure's parent admit check is marked an optimisation: its disable run stayed green because an unadmitted row is never seeded or written. - Plan d-rmap-dir-1-membership-closure-focus-v1.md with the measured results; STATUS_BOARD rows D-RMAP-DIR-1 (MEASURED) and D-RMAP-DIR-2 (PROPOSED, not authorized); entry, INTEGRATION_PLANS, entries index. - CI dir-rmap-probe.yml: fmt, clippy, tests, and the 8,000-user benchmark as an equivalence smoke run (timings not gated). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
…, composite vs conjunction fixture F now binds one chain (all/security/mail) and projects the complete B+ closure through down rows; a focus-local reference projection is kept for the equality test. Preparation is split (slots, projection, eager or lazy per-user rows), refresh is incremental after membership edits and nesting pairs, and a focus refuses a stale B+, >256 groups and duplicates. New tests: per-chain F vs A and D, two-hop paths through unfocused groups, refreshed focus vs fresh bind after an overlay with a cycle, composite membership vs policy conjunction (composites=0 fixture). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Add Carrot credits or activate Agent usage billing to use Autofix |
![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: a1125eea28
ℹ️ 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".
| pub struct Witness { | ||
| pub version: VersionId, | ||
| pub users: usize, | ||
| pub groups: usize, | ||
| } |
There was a problem hiding this comment.
When a prepared cache is checked against a separately constructed VersionStore with the same user/group counts, this witness can falsely pass: every store starts its local VersionId sequence at zero, so two unrelated root snapshots both produce (VersionId(0), users, groups). Their GUID-to-ordinal mappings and memberships can differ, causing the old closure or focus to return incorrect answers despite check returning Ok; include a snapshot/store identity or content seal in the witness.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79e802c. The witness now also holds a Weak to the version's Snapshot and compares it by pointer. The Weak keeps the allocation alive, so the address can't be reused while the witness exists. New test a_witness_from_another_store_is_refused: two stores over the same fixture share VersionId(0) and the same counts, and a cache from one refuses the other. With the pointer comparison removed, that test fails.
Generated by Claude Code
| impl FocusUsers { | ||
| pub fn bind(c: &Compiled, groups: &[u16]) -> FocusUsers { | ||
| let focus = Focus::bind(c, groups); | ||
| let rows = (0..c.b.users as u16).map(|u| focus.row_of(c, u)).collect(); |
There was a problem hiding this comment.
For a valid maximum-size dir-sim snapshot with 65,536 users, casting c.b.users to u16 wraps to zero, so this range produces no rows. Subsequent FocusUsers evaluation then indexes an empty vector and panics; iterate over the usize count and cast each in-range ordinal instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 79e802c. FocusUsers::from_focus now iterates 0..users as usize and casts each ordinal to u16. The same pattern in the benchmark and a test is fixed too.
Generated by Claude Code
…junction workloads, working-set sweep Bench: counting global allocator; per-phase timing and allocations for focus construction, projection (down rows vs reference), eager and lazy per-user rows, refresh after membership edits and after a narrow and a wide nesting pair; computed and measured crossovers (T_F(N) vs T_B+(N)), a rebuild-per-question arm and a question-aware B+ (needed words only); W5a conjunction / W5b composite membership / W5c conjunction on a fixture without composites; group-count sweep against L1/L2 with bytes touched and working sets, hot/warm/cold. CI runs the sweep at 1,2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
…6 users Codex review on #1635: (1) a witness of (version, users, groups) passes across two stores, since every VersionStore numbers versions from 0; it now also holds a Weak to the version's Snapshot, compared by pointer (the Weak keeps the address from being reused). New test: a cache from one store refuses another store's witness over the same data. (2) user ranges iterate as usize, so 65,536 users no longer wrap to an empty range. Bench: the W8 rebind time no longer includes choosing the focus. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
Plan §9: completeness tests and disable runs, the working-set sweep against L1/L2, composite vs conjunction, per-phase preparation accounting and the amortization table, revised F verdict. Board: entry for round 2, STATUS_BOARD row, regenerated entries index. Bench: W8 rebind time no longer includes building the witness. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg
2 participants
A lab probe that measures whether dir-sim's membership, inheritance and role questions gain more from compiled closure rows shared across users (B+) or from a reusable 256-slot group focus (F). No production file changes. dir-sim stays authoritative and is read only through its public API. Nothing is promoted.
Plan and full results:
.claude/plans/d-rmap-dir-1-membership-closure-focus-v1.md(round 1 in §1–8, round 2 in §9).Arms
Viewwalks (authoritative)Role and location are ordinary groups. A role × location combination is either a materialized composite group nested in both, or a conjunction asked of the closure (a fixture without composites tests the second form alone). Neither adds membership semantics.
Correctness
tests/equivalence.rs, 11 tests. B, B+, D and F (eager and lazy rows) all equal A on every user, group and chain of the small fixture. Specific cases:Results (8,000 users, release build, medians)
Round 2, F against B+, measured on L1d 48 KiB / L2 2 MiB:
Recommendation
D-RMAP-DIR-2 (B built in
Snapshot::buildfrom the lanes, B+ as a version-witnessed cache behindView) is not authorized.Open, and not settled by this PR: B, B+ and F all still read rows at addresses taken from a user's direct-group list on the hot path, so they do not satisfy the no-gather rule. That stays with
ISS-ADMISSION-PREP-BINDING.Files
crates/dir-rmap-probe/: own workspace; fixture (Spec::iam_scaled,without_composites),prep(B and B+),focus(F, F+users, F lazy),oracle(D), tests, andexamples/bench.rs(counting allocator, W1–W9, working-set sweep)..github/workflows/dir-rmap-probe.yml: fmt, clippy and tests, plus the 8,000-user benchmark as an equivalence smoke test (sweep factors 1 and 2; timings are not gated).🤖 Generated with Claude Code
https://claude.ai/code/session_01G22yT6htkcdyXsihxxXdrg