Skip to content

Atomic add_relationship: collapse five transactions into one (PR 2/2) #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
david-w-t merged 6 commits into from
Jun 21, 2026
Merged

Atomic add_relationship: collapse five transactions into one (PR 2/2) #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8f58229
design(add_relationship): atomic collapse (PR 2 of 2) design doc
david-w-t Jun 21, 2026
278e2eb
plan: atomic add_relationship collapse (PR 2 of 2)
david-w-t Jun 21, 2026
106b872
test(add_relationship): cover source/target_has_no_class atoms
david-w-t Jun 21, 2026
0ad8471
feat(add_relationship): collapse into a single transaction
david-w-t Jun 21, 2026
3846cf4
docs(add_relationship): record atomic collapse + deferred convergence
david-w-t Jun 21, 2026
16ef16f
docs(add_relationship): tighten convergence-bullet wording
david-w-t Jun 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions TASKS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,16 +133,28 @@ Tracked follow-ups (not in the seam spec):
instance CT cases (`characterization_not_found`/`reciprocal_not_found`
arms). Design `docs/designs/transaction-seam-retrofit-design.md`; plan
`docs/superpowers/plans/2026-06-20-transaction-seam-retrofit.md`.
- **Atomic `add_relationship`** — collapse its four separate transactions
(validate → resolve classes → resolve template → write) into one. The
prerequisite tier-1 `graphdb_class` read primitives
(`get_template_in_txn/1`, `class_in_ancestry_in_txn/2`,
`default_template_in_txn/1`) have landed (PR 1,
`docs/designs/atomic-add-relationship-primitives-design.md`). PR 2 swaps
`add_relationship` onto them, converts the `source_has_no_class` /
`target_has_no_class` arms to `mnesia:abort/1`, and allocates the rel-id pair
up-front. Sequence with / before `mutate/1`, which wants those primitives too.
- **Atomic `add_relationship`** — IMPLEMENTED. `do_add_relationship/7`'s five
separate transactions (validate endpoints → resolve classes → resolve
template → validate scope → write) are collapsed into one
`graphdb_mgr:transaction/1` (TOCTOU isolation). The four single-use phase
helpers were converted in place to in-txn (abort-based) form; a private
`class_of_in_txn/1` was added (`do_class_of/1` keeps its own txn for its
public caller); `build_connection_rows` was split into `/6` (allocates) +
`/7` (pure) so the rel-id pair is allocated up-front outside the
transaction. Behaviour-preserving; existing `add_relationship` suite
unchanged, +2 new instance CT cases (`source_has_no_class` /
`target_has_no_class`). Design
`docs/designs/atomic-add-relationship-design.md`; plan
`docs/superpowers/plans/2026-06-21-atomic-add-relationship.md`.
- **Batch `mutate([Mutation])`** — the tier-3 entry point.
- **Converge default-template name search** — `graphdb_class` carries two
copies of the default-template name-search walk: the gen-server
`do_find_template_by_name/2` (own txn) and the tier-1
`default_template_in_txn/1` (PR 1). `do_default_template/1` already wraps its
own transaction, so it could be rewritten to call `default_template_in_txn/1`
inside that txn, removing the duplication.
Deliberately deferred (the duplication is sanctioned project precedent);
a future cleanup, not blocking anything.

### Node deletion (slice A) — IMPLEMENTED

Expand Down
2 changes: 1 addition & 1 deletion apps/graphdb/CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ Manages the "is a" hierarchy of class nodes in the ontology.
Creates and manages instance nodes in the project (instance space).

- `create_instance/3,4,5` (name, class_nref, compositional_parent_nref [, connection_resolver [, conflict_resolver]]) — atomically writes the node record AND the instance→class membership relationship pair (arc labels nref=29 and nref=30), then fires composition rules (F4 B2). Returns `{ok, Nref, Report}` on success or `{error, Reason, Report}` on rule-firing failure; pre-plan validation errors (unknown class, non-instantiable class, etc.) return `{error, Reason}` (2-tuple). Rejects a class marked non-instantiable with `{error, {class_not_instantiable, ClassNref}}` (L9). Propose-mode composition rules surface as `proposed` outcomes in the report (B3); nothing is materialised for them. `/4` threads a connection **resolver** (`fun((ConnContext) -> {connect, [Target]} | defer end`): the RESOLVE step fires effective ConnectionRules (F4 B4) — `mandatory` connections to existing targets land in the root transaction, `auto` post-commit, `defer`/`propose` are reported only; targets are validated (exists, instance, instance-of target_class-or-subclass). `/3` uses the built-in `report_only` (defer-all) connection resolver, so connection rules surface as `required`/`not_connected`/`proposed` outcomes and nothing is connected. `/5` threads a B5 **conflict resolver** (`fun((#{kind, rules, class_nref}) -> [Pair])`); `/3` and `/4` inject the built-in `graphdb_rules:default_conflict_resolver/0`, which shadows conflicting inherited rules (nearest-level winner by mode priority), merges multiplicity (nearest Min, greatest Max), and demotes both-real-template losers to `propose` (F4 B5).
- `add_relationship/4` (source_nref, characterization_nref, target_nref, reciprocal_nref) — writes two directed rows atomically; IDs allocated via `get_nref()`
- `add_relationship/4,5,6` (source_nref, characterization_nref, target_nref, reciprocal_nref [, template_nref [, {FwdAVPs, RevAVPs}]]) — validates endpoints, resolves source/target class and template scope, and writes the two directed `kind=connection` rows in a **single** `graphdb_mgr:transaction/1` (TOCTOU-isolated). The rel-id pair is allocated up-front (outside the transaction) via `rel_id_server:get_id_pair/0`. `/4` uses the source class's default template; `/5` takes an explicit template nref; `/6` adds per-direction AVPs.
- `add_class_membership/2` (instance_nref, class_nref) — adds a membership arc pair; also rejects a non-instantiable class target with `{error, {class_not_instantiable, ClassNref}}` (L9)
- `get_instance/1`, `children/1`, `compositional_ancestors/1`, `resolve_value/2`

Expand Down
265 changes: 146 additions & 119 deletions apps/graphdb/src/graphdb_instance.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1176,101 +1176,91 @@ do_validate_parent(ParentNref, RetAttr) ->

%%-----------------------------------------------------------------------------
%% do_add_relationship(SourceNref, CharNref, TargetNref, ReciprocalNref,
%% TemplateSpec, State) -> ok | {error, term()}
%% TemplateSpec, AVPSpec, State) -> ok | {error, term()}
%%
%% TemplateSpec is either the atom `default` (look up source's class
%% default template) or an integer template nref. Arc validation
%% (existence + arc-label kind + target_kind agreement) runs first,
%% then class lookup, template resolution, scope check, and the
%% two-row write of the connection arcs with the Template AVP stamped
%% on each.
%% Validates endpoints, resolves class membership and template scope, then
%% writes the two directed connection rows -- all in one graphdb_mgr:transaction/1
%% (TOCTOU-isolated). The rel-id pair is allocated up-front, outside the
%% transaction: get_id_pair is a gen_server call and must never run inside an
%% mnesia fun. A validation abort orphans that pair -- harmless (allocate-
%% outside-transaction doctrine). Phase order: validate endpoints ->
%% resolve classes -> resolve template -> validate scope -> write.
%%-----------------------------------------------------------------------------
do_add_relationship(SourceNref, CharNref, TargetNref, ReciprocalNref,
TemplateSpec, AVPSpec, State) ->
TkAttr = State#state.target_kind_avp_nref,
case validate_arc_endpoints(SourceNref, CharNref, TargetNref,
ReciprocalNref, TkAttr, State#state.retired_nref) of
ok ->
case resolve_arc_classes(SourceNref, TargetNref) of
{ok, SourceClass, TargetClass} ->
case resolve_template(TemplateSpec, SourceClass) of
{ok, TemplateNref} ->
case validate_template_scope(TemplateNref,
SourceClass, TargetClass) of
ok ->
write_connection_arcs(SourceNref,
CharNref, TargetNref,
ReciprocalNref, TemplateNref,
AVPSpec);
{error, _} = Err -> Err
end;
{error, _} = Err -> Err
end;
{error, _} = Err -> Err
end;
{error, _} = Err ->
Err
TkAttr = State#state.target_kind_avp_nref,
RetAttr = State#state.retired_nref,
%% Allocate the rel-id pair up-front, OUTSIDE the transaction: get_id_pair
%% is a gen_server call and must never run inside an mnesia fun. A
%% validation abort below orphans this pair -- harmless (allocate-outside-
%% transaction doctrine).
IdPair = rel_id_server:get_id_pair(),
Txn = fun() ->
ok = validate_arc_endpoints_in_txn(SourceNref, CharNref, TargetNref,
ReciprocalNref, TkAttr, RetAttr),
{SourceClass, TargetClass} =
resolve_arc_classes_in_txn(SourceNref, TargetNref),
TemplateNref = resolve_template_in_txn(TemplateSpec, SourceClass),
ok = validate_template_scope_in_txn(TemplateNref, SourceClass,
TargetClass),
Rows = build_connection_rows(IdPair, SourceNref, CharNref, TargetNref,
ReciprocalNref, TemplateNref, AVPSpec),
lists:foreach(fun({Tab, Rec}) -> ok = mnesia:write(Tab, Rec, write) end,
Rows)
end,
case graphdb_mgr:transaction(Txn) of
{ok, ok} -> ok;
{error, _} = Err -> Err
end.


%%-----------------------------------------------------------------------------
%% validate_arc_endpoints(Source, Char, Target, Reciprocal, TkAttr, RetAttr) ->
%% ok | {error, term()}
%% validate_arc_endpoints_in_txn(Source, Char, Target, Reciprocal, TkAttr,
%% RetAttr) -> ok (aborts the enclosing transaction on any violation)
%%
%% Arc validation. Reads all four nodes inside one mnesia transaction
%% and rejects:
%% - missing source / target / characterization / reciprocal
%% - any endpoint that is retired (retired => true AVP under RetAttr);
%% reports the first retired nref as {endpoint_retired, Nref}
%% - characterization or reciprocal that is not kind=attribute
%% - target whose kind disagrees with the characterization's
%% `target_kind` AVP (the value stored under attribute=TkAttr)
%% In-transaction endpoint validation. Assumes it runs inside an active mnesia
%% activity; reads the four nodes with bare mnesia:read and signals every
%% violation via mnesia:abort/1 (same Reason terms as the prior own-txn form).
%%-----------------------------------------------------------------------------
validate_arc_endpoints(SourceNref, CharNref, TargetNref, ReciprocalNref,
validate_arc_endpoints_in_txn(SourceNref, CharNref, TargetNref, ReciprocalNref,
TkAttr, RetAttr) ->
F = fun() ->
Source = mnesia:read(nodes, SourceNref),
Target = mnesia:read(nodes, TargetNref),
Char = mnesia:read(nodes, CharNref),
Recip = mnesia:read(nodes, ReciprocalNref),
case {Source, Target, Char, Recip} of
{[], _, _, _} ->
mnesia:abort({source_not_found, SourceNref});
{_, [], _, _} ->
mnesia:abort({target_not_found, TargetNref});
{_, _, [], _} ->
mnesia:abort({characterization_not_found, CharNref});
{_, _, _, []} ->
mnesia:abort({reciprocal_not_found, ReciprocalNref});
{[#node{attribute_value_pairs = SAVPs}],
[#node{kind = TKind, attribute_value_pairs = TAVPs}],
[#node{kind = CKind, attribute_value_pairs = CAVPs} = CharNode],
[#node{kind = RKind, attribute_value_pairs = RAVPs}]} ->
case first_retired([{SourceNref, SAVPs}, {TargetNref, TAVPs},
{CharNref, CAVPs}, {ReciprocalNref, RAVPs}],
RetAttr) of
{retired, RNref} ->
mnesia:abort({endpoint_retired, RNref});
none ->
case {CKind, RKind} of
{attribute, attribute} ->
case check_target_kind(CharNode, TKind, TkAttr) of
ok -> ok;
{error, Reason} -> mnesia:abort(Reason)
end;
{attribute, _} ->
mnesia:abort({reciprocal_not_an_attribute,
ReciprocalNref, RKind});
{_, _} ->
mnesia:abort({characterization_not_an_attribute,
CharNref, CKind})
end
end
end
end,
case graphdb_mgr:transaction(F) of
{ok, ok} -> ok;
{error, _} = Err -> Err
Source = mnesia:read(nodes, SourceNref),
Target = mnesia:read(nodes, TargetNref),
Char = mnesia:read(nodes, CharNref),
Recip = mnesia:read(nodes, ReciprocalNref),
case {Source, Target, Char, Recip} of
{[], _, _, _} ->
mnesia:abort({source_not_found, SourceNref});
{_, [], _, _} ->
mnesia:abort({target_not_found, TargetNref});
{_, _, [], _} ->
mnesia:abort({characterization_not_found, CharNref});
{_, _, _, []} ->
mnesia:abort({reciprocal_not_found, ReciprocalNref});
{[#node{attribute_value_pairs = SAVPs}],
[#node{kind = TKind, attribute_value_pairs = TAVPs}],
[#node{kind = CKind, attribute_value_pairs = CAVPs} = CharNode],
[#node{kind = RKind, attribute_value_pairs = RAVPs}]} ->
case first_retired([{SourceNref, SAVPs}, {TargetNref, TAVPs},
{CharNref, CAVPs}, {ReciprocalNref, RAVPs}],
RetAttr) of
{retired, RNref} ->
mnesia:abort({endpoint_retired, RNref});
none ->
case {CKind, RKind} of
{attribute, attribute} ->
case check_target_kind(CharNode, TKind, TkAttr) of
ok -> ok;
{error, Reason} -> mnesia:abort(Reason)
end;
{attribute, _} ->
mnesia:abort({reciprocal_not_an_attribute,
ReciprocalNref, RKind});
{_, _} ->
mnesia:abort({characterization_not_an_attribute,
CharNref, CKind})
end
end
end.

%% first_retired([{Nref, AVPs}], RetAttr) -> {retired, Nref} | none
Expand All @@ -1297,56 +1287,63 @@ check_target_kind(#node{attribute_value_pairs = AVPs}, ActualKind, TkAttr) ->


%%-----------------------------------------------------------------------------
%% resolve_arc_classes(SourceNref, TargetNref) ->
%% {ok, SourceClass, TargetClass} | {error, term()}
%% resolve_arc_classes_in_txn(SourceNref, TargetNref) ->
%% {SourceClass, TargetClass} (aborts on a missing class)
%%
%% In-transaction class resolution. class_of_in_txn returns only {ok,_} |
%% not_found inside a txn (a read error aborts the txn directly), so the
%% no-class arms abort with the same Reason terms the prior form returned.
%%-----------------------------------------------------------------------------
resolve_arc_classes(SourceNref, TargetNref) ->
case do_class_of(SourceNref) of
{ok, SourceClass} ->
case do_class_of(TargetNref) of
{ok, TargetClass} -> {ok, SourceClass, TargetClass};
not_found -> {error, {target_has_no_class, TargetNref}};
{error, _} = Err -> Err
end;
not_found -> {error, {source_has_no_class, SourceNref}};
{error, _} = Err -> Err
end.
resolve_arc_classes_in_txn(SourceNref, TargetNref) ->
SourceClass = case class_of_in_txn(SourceNref) of
{ok, SC} -> SC;
not_found -> mnesia:abort({source_has_no_class, SourceNref})
end,
TargetClass = case class_of_in_txn(TargetNref) of
{ok, TC} -> TC;
not_found -> mnesia:abort({target_has_no_class, TargetNref})
end,
{SourceClass, TargetClass}.


%%-----------------------------------------------------------------------------
%% resolve_template(TemplateSpec, SourceClass) ->
%% {ok, TemplateNref} | {error, term()}
%% resolve_template_in_txn(TemplateSpec, SourceClass) -> TemplateNref
%% (aborts no_default_template when `default' is requested but absent)
%%-----------------------------------------------------------------------------
resolve_template(default, SourceClass) ->
case graphdb_class:default_template(SourceClass) of
{ok, Nref} -> {ok, Nref};
not_found -> {error, no_default_template};
{error, _} = Err -> Err
resolve_template_in_txn(default, SourceClass) ->
case graphdb_class:default_template_in_txn(SourceClass) of
{ok, Nref} -> Nref;
not_found -> mnesia:abort(no_default_template)
end;
resolve_template(TemplateNref, _SourceClass) when is_integer(TemplateNref) ->
{ok, TemplateNref}.
resolve_template_in_txn(TemplateNref, _SourceClass)
when is_integer(TemplateNref) ->
TemplateNref.


%%-----------------------------------------------------------------------------
%% validate_template_scope(TemplateNref, SourceClass, TargetClass) ->
%% ok | {error, term()}
%% validate_template_scope_in_txn(TemplateNref, SourceClass, TargetClass) -> ok
%% (aborts invalid_template / template_class_not_in_ancestry)
%%
%% Confirms TemplateNref resolves to a kind=template node whose parent
%% class is in SourceClass's or TargetClass's taxonomic ancestry.
%% Confirms TemplateNref resolves to a kind=template node whose parent class is
%% in SourceClass's or TargetClass's taxonomic ancestry. The nested Reason in
%% {invalid_template, _, Reason} is byte-identical to the gen-server get_template
%% form: get_template_in_txn returns the same {error, not_a_template|not_found}.
%%-----------------------------------------------------------------------------
validate_template_scope(TemplateNref, SourceClass, TargetClass) ->
case graphdb_class:get_template(TemplateNref) of
validate_template_scope_in_txn(TemplateNref, SourceClass, TargetClass) ->
Comment thread
david-w-t marked this conversation as resolved.
case graphdb_class:get_template_in_txn(TemplateNref) of
{ok, #node{parents = TmplParents}} ->
TmplClass = head_parent(TmplParents),
InSource = graphdb_class:class_in_ancestry(TmplClass, SourceClass),
InTarget = graphdb_class:class_in_ancestry(TmplClass, TargetClass),
InSource = graphdb_class:class_in_ancestry_in_txn(TmplClass,
SourceClass),
InTarget = graphdb_class:class_in_ancestry_in_txn(TmplClass,
TargetClass),
case InSource orelse InTarget of
true -> ok;
false -> {error, {template_class_not_in_ancestry,
TemplateNref, TmplClass, SourceClass, TargetClass}}
false -> mnesia:abort({template_class_not_in_ancestry,
TemplateNref, TmplClass, SourceClass, TargetClass})
end;
{error, Reason} ->
{error, {invalid_template, TemplateNref, Reason}}
mnesia:abort({invalid_template, TemplateNref, Reason})
end.


Expand All @@ -1360,8 +1357,19 @@ validate_template_scope(TemplateNref, SourceClass, TargetClass) ->
%% composition root txn; auto connections are written post-commit).
%%-----------------------------------------------------------------------------
build_connection_rows(SourceNref, CharNref, TargetNref, ReciprocalNref,
TemplateNref, {FwdAVPs, RevAVPs}) ->
{Id1, Id2} = rel_id_server:get_id_pair(),
TemplateNref, AVPSpec) ->
IdPair = rel_id_server:get_id_pair(),
build_connection_rows(IdPair, SourceNref, CharNref, TargetNref,
ReciprocalNref, TemplateNref, AVPSpec).

%% build_connection_rows({Id1, Id2}, S, C, T, R, TemplateNref, {FwdAVPs,RevAVPs})
%% -> [{relationships, #relationship{}}]
%%
%% Pure builder: no allocation. The caller supplies the rel-id pair (allocated
%% up-front, outside any transaction) so the rows can be built inside a caller's
%% transaction. Template AVP rides index 0 of each direction.
build_connection_rows({Id1, Id2}, SourceNref, CharNref, TargetNref,
ReciprocalNref, TemplateNref, {FwdAVPs, RevAVPs}) ->
TemplateAVP = #{attribute => ?ARC_TEMPLATE, value => TemplateNref},
Fwd = #relationship{
id = Id1, kind = connection,
Expand Down Expand Up @@ -1496,6 +1504,25 @@ do_class_of(InstanceNref) ->
end.


%%-----------------------------------------------------------------------------
%% class_of_in_txn(InstanceNref) -> {ok, ClassNref} | not_found
%%
%% Tier-1 in-transaction twin of do_class_of/1. Assumes it runs inside an
%% active mnesia activity; uses a bare index_read. do_class_of/1 keeps its
%% own transaction for its public class_of caller.
%%-----------------------------------------------------------------------------
class_of_in_txn(InstanceNref) ->
Rels = mnesia:index_read(relationships, InstanceNref,
#relationship.source_nref),
case lists:search(
fun(R) ->
R#relationship.characterization =:= ?ARC_INST_TO_CLASS
end, Rels) of
{value, #relationship{target_nref = ClassNref}} -> {ok, ClassNref};
false -> not_found
end.


%%-----------------------------------------------------------------------------
%% do_get_instance(Nref) ->
%% {ok, #node{}} | {error, not_found | not_an_instance | term()}
Expand Down
Loading
Loading