plan(ogar-core): OP-arm canonical-contract v1 (AR → OGAR → SurrealQL)#53
Conversation
…4/#545 finish line Writes the producer-side counterpart to lance-graph/docs/OGAR_CONSUMER_API.md (landed in lance-graph#538). Maps the current 4-repo state (ruff producer → op-surreal-ast SurrealQL arm → ogar-from-ruff vocab arm → surrealdb bridge) onto the OGAR Core contract, names the two named gaps the consumer doc opened (DO-arm lift `Class.functions → ClassActions`, registration hop `Vec<Class> → OntologyRegistry`), and codifies the iron rules I will not step on (D-CLS field enumeration, ClassView extensions, parallel object models, consumer-private RBAC/exec). Mailbox-SoA-replacing-BindSpace blast radius: zero today (no SoA runtime-instance writes anywhere in the AR → SurrealQL arm), but the `WriteOutcome` contract and `current_cycle.wrapping_sub` gate are codified as the load-bearing surface for any future runtime hop. 5+3 council review requested on this PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74963fadcc
ℹ️ 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".
| 2. **`ogar-from-rails` exposes `lift_with_actions`** — returns `(Vec<Class>, HashMap<ClassName, Vec<OwnedActionDef>>)` so consumers get both arms from one walk. | ||
| 3. **`ogar-to-registry`** new crate or method — `Class → OntologyRegistry::register_class_path` + classid binding. | ||
| 4. **Consumer-side codegen example** — in `openproject-nexgen-rs`, a small build.rs / proc-macro that turns `OwnedActionDef` into `const &[ActionDef]` with the OP RBAC map merged. | ||
| 5. **`op-surreal-ast` extension** — given `&ClassActions` for a class, project to SurrealQL `DEFINE FUNCTION` per `ActionDef` (only when `exec: ExecTarget::SurrealQl`). Closes the AR → DO → SurrealQL loop. |
There was a problem hiding this comment.
Don't generate functions from declarations alone
This step says the SurrealQL arm can project &ClassActions directly to DEFINE FUNCTION, but the ClassActions/ActionDef contract used above only carries dispatch metadata such as predicate, classid, exec target, guards, and roles; this plan also leaves Ruby method-body lifting out of scope. In the ExecTarget::SurrealQl path, following this as written would generate functions with no executable body, so Step 5 needs an explicit body-adapter/source input or should remain dispatch metadata rather than claiming to close the SurrealQL loop.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the rework (reviewed commit 74963fa predates it; current HEAD b50f7fb). You're right that a DEFINE FUNCTION requires a body, so the plan no longer projects one from declarations alone. The DO→SurrealQL projection moved to a new crate op-surreal-actions and, when the action's executor is SurrealAst, emits an admission function whose body is the commit-gate adjudication that is derivable from ActionDef metadata alone — RBAC via the consumer-supplied role map + the state guard as $value IN [...]. The domain effect (the Ruby method body) is an explicit deferred body-adapter input (D-AR-5.6); until it exists the projection emits a -- TODO domain-body marker, never a fabricated body, and falls back to dispatch metadata / annotation when no metadata-derivable guard exists. Step 5 no longer claims to close the full AR→DO→SurrealQL loop — only its admission half. See §11.1.
|
|
||
| | Step | DoD | | ||
| |------|---------------------------------------------------------------------------------------------------| | ||
| | 1 | `lift_actions_emits_one_def_per_function`, `lift_actions_carries_ruff_method_visibility`, `lift_actions_default_exec_is_surrealql` | |
There was a problem hiding this comment.
Don't require visibility the IR cannot carry
This DoD is not implementable against the current Rails Model::functions input: ruff_spo_triplet::Function exposes method name/read/raise/traversal data, but not public/private/protected, and the proposed action shape here has no visibility field either. As written, Step 1 either has to add an impossible test or silently fabricate visibility, so the plan should first call out a ruff IR enrichment or drop the visibility-carrying requirement.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in the rework (reviewed commit 74963fa predates it; current HEAD b50f7fb). Correct — ruff_spo_triplet::Function carries name/read/raise/traversal data but not public/private/protected, and the action shape has no visibility slot. The DoD dropped lift_actions_carries_ruff_method_visibility; §10 now states plainly that Function::visibility does not exist in the ruff IR and the lift filters on action-shape, not access modifier. The lift_actions DoD is now emits_one_def_per_method / sets_no_exec_policy / predicate_is_snake_case.
There was a problem hiding this comment.
Correcting my earlier reply — dropping the visibility DoD was the wrong call. Visibility is neither a producer field to add nor a requirement to drop: it is a bit of the ClassView bitmask (lance-graph-contract::class_view).
The model (now §6.1, commit 9d981f5) is Jinja-style: ClassView is the inherited schema template, the bitmask is the per-class filter ('enable what, where'), and a Function/ActionDef is the materialization over (ClassView × mask). A class inheriting a schema applies its mask as the filter; resolve_mro walks the chain and the effective surface is schema ∩ mask (= effective_actions in #538). So the lift reads the visibility bit at materialization time — the ruff IR does not need to carry it, and the requirement stands (lift_actions_materializes_public_surface_from_mask). Thanks for the push; this is the right altitude for the fix.
Rework the AR→OGAR→SurrealQL plan from proposal into the negotiated OP-arm canonical-contract plan, subordinate to #546 (doctrine-only). - Phase A (flip blockers 1-5): canonical slot rename w/ deprecated aliases, PredicateName(Cow<'static,str>) killing OwnedActionDef, register_actions on the registry, content-addressed mint_classid (no u32->u16 truncation), cargo feature isolation guarantee. - Phase B (promotion wave 6-12): Relation, Constraint, Inheritance, StateMachineDecl, AuditTrailDecl, Authorization, registry-owned resolve_mro. - Agnostic-flip rename table grounded against current ogar-vocab::Class. - classid width resolved per council contract-surface finding (low-u16 mint, widen at object_class; NiblePath refuses high-u16-nonzero). - Mailbox-SoA blast radius verified zero across producer crates. - Killed: OwnedActionDef, Box::leak, classid_minter closure, producer-side exec defaults. New crate op-surreal-actions for the DO->SurrealQL projection (zero-async-deps invariant). - Corrected DoD: ~1800-1900 ActionDef (not 8000); no Function::visibility. - Flag two #546 doctrine drift phrases (ractor-owned->proven; slim Executor enum) as upstream OGAR Core cleanup dependencies.
…lity - DEFINE FUNCTION (codex P2): a DEFINE FUNCTION requires a body, so the op-surreal-actions projection emits an admission function whose body is the commit-gate adjudication derivable from ActionDef metadata (RBAC + state guard as $value IN [...]) -- a valid body, not a shell. The Ruby domain effect is an explicit deferred body-adapter input (D-AR-5.6), emitted as a TODO marker meanwhile; falls back to dispatch metadata when no metadata-derivable guard exists. Step 5 no longer claims to close the full loop -- only the admission half. - Visibility (codex P2): already resolved in the rework -- Function:: visibility does not exist in the ruff IR; DoD drops the visibility test and filters on action-shape.
… field Correct the visibility handling at the right altitude instead of clipping the requirement (learned helplessness) or bolting a field onto the ruff IR. - Add §6.1 materialization model: ClassView is the inherited schema template; the bitmask is the per-class filter (Jinja-style 'enable what, where'); a Field/Relation/Function/ActionDef is the materialization = apply the bitmask to the inherited schema. Nothing copied; all derived. - Inheritance = template inheritance + mask filtering; resolve_mro walks the chain, effective surface = schema ∩ mask (= effective_actions, #538). - Visibility is one bit of the filter, read at materialization. No Function.visibility field; no dropped DoD. Answers the codex P2 correctly. - lift_actions DoD: materializes_public_surface_from_mask. - Agnostic by construction: curators emit the same template + mask; only the scrape differs.
The materialization model introduced the bitmask as load-bearing without saying whose it is. Close the residual drift: - FieldMask/bitmask is Core-owned (ClassView, lance-graph-contract:: class_view); the producer only emits the facts that populate its lanes (presence, visibility, inherited-enabled). Materialization is Core-side. Keeps §4c intact (producer emits facts, Core materializes). - Cross-link item 8 Inheritance to §6.1 (template + mask, not field-copy).
…#547 The two #546 drift phrases (ractor-owned->proven; slim Executor enum) are no longer just flagged — they are corrected in lance-graph#547 (docs-only, pending OGAR Core review). Update §11.2 to reference the open PR instead of deferring it as someone else's job.
What this is
The OpenProject curator-arm feedback turned into concrete OGAR Core contract
requirements. Subordinate to #546 (which landed doctrine only — no code); this
is the contract layer downstream of it.
OGAR Core accepted the split:
one cycle) ·
PredicateName(Cow<'static, str>)killingOwnedActionDef·OntologyRegistry::register_actions· content-addressedmint_classid(no
u32→u16truncation) · cargo feature-isolation guarantee forlance-graph-contract.Relation,Constraint,Inheritance,StateMachineDecl,AuditTrailDecl,Authorization, registry-ownedresolve_mro.Council (5+3) folded in
DEFINE FUNCTIONvs body-lift-out-of-scope;"stop at
Class" vs classid binding — minting is now Core's).op-surreal-actionsis a new crate, not an extension ofop-surreal-ast(preserves the zero-async-deps invariant
surrealdb-coreconsumes).ActionInvocationstays producer-absent; producer stops atActionDeffacts.OwnedActionDef,Box::leak, theclassid_minterclosure,producer-side
execdefaults.ClassId = u16,classid stored as u32 with high-u16 canon-reserved zero,
NiblePathrefuses ahigh-u16-nonzero fold; mint into low u16, widen at
object_class.ActionDef(the "≥8000" was wrong);Function::visibilitydoes not exist in the ruff IR.Upstream doctrine dependencies (OGAR Core cleanup PR — not this session)
Flagged, not edited here (the #546 doctrine is Core's artifact):
(ractor = compile-time ownership proof, not runtime mutation authority).
Executorenum → post-council slim shape(
NativeLance | SurrealAst | HumanKanban | Adapter(AdapterTargetId)); dropExternalHttp(Url)(zero-dep),Dll(CapabilityId)(phantom), and namedOdooAdapter/RailsAdapter(concrete adapters live in the callcenter registry).Doctrine line this plan operationalizes
OpenProject is the calibration corpus — the reference implementation, not the
privileged one. OGAR keeps the crown.