Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 8 additions & 8 deletions TASKS.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,14 @@ Tracked follow-ups (not in the seam spec):
`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.
- **Converge default-template name search** — IMPLEMENTED. The shared walk is
now `graphdb_class:find_template_by_name_in_txn/2` (exported tier-1
in-transaction primitive). `default_template_in_txn/1` delegates to it with
`?DEFAULT_TEMPLATE_NAME`; `do_find_template_by_name/2` wraps one
`graphdb_mgr:transaction/1` around it (preserving the `{error,_}->not_found`
swallow). Behaviour-preserving; +3 CT cases. Design
`docs/designs/converge-default-template-name-search-design.md`; plan
`docs/superpowers/plans/2026-06-22-converge-default-template-name-search.md`.

### Node deletion (slice A) — IMPLEMENTED

Expand Down
9 changes: 6 additions & 3 deletions apps/graphdb/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,14 @@ Manages the "is a" hierarchy of class nodes in the ontology.
- `is_instantiable/1` (class_nref) — `false` iff the class carries the `instantiable => false` marker
- `get_class/1`, `subclasses/1`, `ancestors/1`, `inherited_qcs/1`
- `get_template_in_txn/1`, `class_in_ancestry_in_txn/2`,
`default_template_in_txn/1` — tier-1 **in-transaction** read primitives
(bare-mnesia twins of `get_template`/`class_in_ancestry`/`default_template`);
`default_template_in_txn/1`, `find_template_by_name_in_txn/2` — tier-1
**in-transaction** read primitives (bare-mnesia twins of
`get_template`/`class_in_ancestry`/`default_template`/`do_find_template_by_name`);
must be called inside an Mnesia activity. They compose into a caller's single
transaction (the seam's tier-1 contract) and are the prerequisite for atomic
`add_relationship` / `mutate/1`. See
`add_relationship` / `mutate/1`. `find_template_by_name_in_txn/2` is the
generic by-name template search that `default_template_in_txn/1` delegates
to (with `?DEFAULT_TEMPLATE_NAME`). See
`docs/designs/atomic-add-relationship-primitives-design.md`.
- `validate_template_scope_in_txn/3` (template_nref, source_class,
target_class) — in-transaction helper (aborts on failure): confirms the
Expand Down
47 changes: 25 additions & 22 deletions apps/graphdb/src/graphdb_class.erl
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@
templates_for_class/1,
default_template/1,
default_template_in_txn/1,
find_template_by_name_in_txn/2,
is_instantiable/1,
%% Class-ancestry + template-scope helpers (used by graphdb_instance
%% to validate Template AVP class scope on Connection arcs)
Expand Down Expand Up @@ -699,18 +700,12 @@ do_write_template(ClassNref, Name) ->
%% filter ensures we only return templates.
%%-----------------------------------------------------------------------------
do_find_template_by_name(ClassNref, Name) ->
F = fun() ->
Children = downward_children_by_arc(ClassNref, ?ARC_CLS_CHILD,
composition),
lists:search(fun
(#node{kind = template} = N) -> template_has_name(N, Name);
(_) -> false
end, Children)
end,
case graphdb_mgr:transaction(F) of
{ok, {value, #node{nref = Nref}}} -> {ok, Nref};
{ok, false} -> not_found;
{error, _} -> not_found
case graphdb_mgr:transaction(fun() ->
find_template_by_name_in_txn(ClassNref, Name)
end) of
{ok, {ok, Nref}} -> {ok, Nref};
{ok, not_found} -> not_found;
{error, _} -> not_found
end.

template_has_name(#node{attribute_value_pairs = AVPs}, Name) ->
Expand All @@ -720,25 +715,33 @@ template_has_name(#node{attribute_value_pairs = AVPs}, Name) ->
end, AVPs).

%%-----------------------------------------------------------------------------
%% default_template_in_txn(ClassNref) -> {ok, Nref} | not_found
%% find_template_by_name_in_txn(ClassNref, Name) -> {ok, Nref} | not_found
%%
%% Tier-1 in-transaction twin of default_template/1. Assumes it runs inside an
%% active mnesia activity; reuses the bare-mnesia downward_children_by_arc/3 and
%% template_has_name/2. Returns not_found when ClassNref has no template named
%% ?DEFAULT_TEMPLATE_NAME (e.g. an abstract class).
%% Tier-1 in-transaction primitive. Assumes it runs inside an active mnesia
%% activity; reuses the bare-mnesia downward_children_by_arc/3 and
%% template_has_name/2. Returns the kind=template child of ClassNref whose
%% class NameAttrNref (19) value equals Name, or not_found.
%%-----------------------------------------------------------------------------
default_template_in_txn(ClassNref) ->
find_template_by_name_in_txn(ClassNref, Name) ->
Children = downward_children_by_arc(ClassNref, ?ARC_CLS_CHILD, composition),
case lists:search(fun
(#node{kind = template} = N) ->
template_has_name(N, ?DEFAULT_TEMPLATE_NAME);
(_) ->
false
(#node{kind = template} = N) -> template_has_name(N, Name);
(_) -> false
end, Children) of
{value, #node{nref = Nref}} -> {ok, Nref};
false -> not_found
end.

%%-----------------------------------------------------------------------------
%% default_template_in_txn(ClassNref) -> {ok, Nref} | not_found
%%
%% Tier-1 in-transaction twin of default_template/1. Delegates to
%% find_template_by_name_in_txn/2 with ?DEFAULT_TEMPLATE_NAME. Returns
%% not_found when ClassNref has no default template (e.g. an abstract class).
%%-----------------------------------------------------------------------------
default_template_in_txn(ClassNref) ->
find_template_by_name_in_txn(ClassNref, ?DEFAULT_TEMPLATE_NAME).


%%-----------------------------------------------------------------------------
%% do_get_template(Nref) ->
Expand Down
46 changes: 46 additions & 0 deletions apps/graphdb/test/graphdb_class_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@
default_template_in_txn_returns_default/1,
default_template_in_txn_abstract_not_found/1,
default_template_in_txn_not_found_after_delete/1,
find_template_by_name_in_txn_found/1,
find_template_by_name_in_txn_discriminates/1,
find_template_by_name_in_txn_not_found/1,
class_in_ancestry_self/1,
class_in_ancestry_ancestor/1,
class_in_ancestry_unrelated/1,
Expand Down Expand Up @@ -174,6 +177,9 @@ groups() ->
default_template_in_txn_returns_default,
default_template_in_txn_abstract_not_found,
default_template_in_txn_not_found_after_delete,
find_template_by_name_in_txn_found,
find_template_by_name_in_txn_discriminates,
find_template_by_name_in_txn_not_found,
class_in_ancestry_self,
class_in_ancestry_ancestor,
class_in_ancestry_unrelated,
Expand Down Expand Up @@ -635,6 +641,46 @@ default_template_in_txn_not_found_after_delete(_Config) ->
graphdb_class:default_template_in_txn(ClassNref)
end)).

%%-----------------------------------------------------------------------------
%% find_template_by_name_in_txn resolves a named (non-default) template child
%% and returns it distinct from the auto-created default template.
%%-----------------------------------------------------------------------------
find_template_by_name_in_txn_found(_Config) ->
{ok, _} = graphdb_class:start_link(),
{ok, ClassNref} = graphdb_class:create_class("Animal", 3),
{ok, Default} = graphdb_class:default_template(ClassNref),
{ok, Bio} = graphdb_class:add_template(ClassNref, "biological"),
?assertNotEqual(Default, Bio),
?assertEqual({ok, {ok, Bio}}, graphdb_mgr:transaction(fun() ->
graphdb_class:find_template_by_name_in_txn(ClassNref, "biological")
end)).

%%-----------------------------------------------------------------------------
%% find_template_by_name_in_txn selects by name: searching the same class for
%% "default" returns the default template, not the named one (proves the name
%% selects rather than first-match).
%%-----------------------------------------------------------------------------
find_template_by_name_in_txn_discriminates(_Config) ->
{ok, _} = graphdb_class:start_link(),
{ok, ClassNref} = graphdb_class:create_class("Animal", 3),
{ok, Default} = graphdb_class:default_template(ClassNref),
{ok, Bio} = graphdb_class:add_template(ClassNref, "biological"),
?assertNotEqual(Default, Bio),
?assertEqual({ok, {ok, Default}}, graphdb_mgr:transaction(fun() ->
graphdb_class:find_template_by_name_in_txn(ClassNref, "default")
end)).

%%-----------------------------------------------------------------------------
%% find_template_by_name_in_txn returns not_found for a name no template
%% carries.
%%-----------------------------------------------------------------------------
find_template_by_name_in_txn_not_found(_Config) ->
{ok, _} = graphdb_class:start_link(),
{ok, ClassNref} = graphdb_class:create_class("Animal", 3),
?assertEqual({ok, not_found}, graphdb_mgr:transaction(fun() ->
graphdb_class:find_template_by_name_in_txn(ClassNref, "nonexistent")
end)).

%%-----------------------------------------------------------------------------
%% class_in_ancestry returns true when the candidate equals the class.
%%-----------------------------------------------------------------------------
Expand Down
153 changes: 153 additions & 0 deletions docs/designs/converge-default-template-name-search-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
<!--
Copyright (c) 2026 David W. Thomas
SPDX-License-Identifier: GPL-2.0-or-later
-->

# Converge Default-Template Name Search — Design

**Status:** Approved (design) — not yet planned/implemented
**Date:** 2026-06-22
**Author:** David W. Thomas (with Claude)
**Slice:** Cleanup — converge the duplicated template name-search walk in
`graphdb_class`

## Background

`graphdb_class` carries the template name-search walk **twice**, verbatim:

- `do_find_template_by_name/2` — the gen-server form. Opens its own
transaction via `graphdb_mgr:transaction/1`, takes a generic `Name`, and
swallows a read error as `not_found`. Internal-only; called by
`do_add_template/2` (duplicate-name guard) and `do_default_template/1`
(default-template lookup).
- `default_template_in_txn/1` — the tier-1 in-transaction form added in the
atomic-`add_relationship` PR 1 (`ad030f6`). Runs inside a caller's mnesia
activity (no own transaction) and hardcodes `?DEFAULT_TEMPLATE_NAME`.
Exported; called by `graphdb_instance` (line ~1314) and covered by three
direct CT cases.

Both contain the identical core:

```erlang
Children = downward_children_by_arc(ClassNref, ?ARC_CLS_CHILD, composition),
lists:search(fun
(#node{kind = template} = N) -> template_has_name(N, Name);
(_) -> false
end, Children)
```

The duplication was sanctioned project precedent at the time (the tier-1
twins were deliberately near-verbatim copies of their gen-server originals,
mirroring the ancestry-walk twins). This slice converges the *name-search*
pair now that both copies have settled, and was tracked as the deferred
"Converge default-template name search" item in `TASKS.md`.

## Goal

Remove the duplicated walk by extracting one shared tier-1 in-transaction
primitive, funnelling both existing functions through it, and exposing the
primitive for future cross-module use (e.g. `mutate/1`). No externally
observable behaviour changes.

## The shared primitive

Extract the core into a new tier-1 in-transaction primitive and **export**
it alongside the other tier-1 primitives:

```erlang
%% find_template_by_name_in_txn(ClassNref, Name) -> {ok, Nref} | not_found
%%
%% Tier-1 in-transaction primitive. Assumes it runs inside an active mnesia
%% activity; reuses the bare-mnesia downward_children_by_arc/3 and
%% template_has_name/2. Returns the kind=template child of ClassNref whose
%% class NameAttrNref (19) value equals Name, or not_found.
find_template_by_name_in_txn(ClassNref, Name) ->
Children = downward_children_by_arc(ClassNref, ?ARC_CLS_CHILD, composition),
case lists:search(fun
(#node{kind = template} = N) -> template_has_name(N, Name);
(_) -> false
end, Children) of
{value, #node{nref = Nref}} -> {ok, Nref};
false -> not_found
end.
```

It joins `get_template_in_txn/1`, `class_in_ancestry_in_txn/2`, and
`default_template_in_txn/1` in the exported tier-1 group: bare mnesia ops,
no own transaction, composes into a caller's single transaction.

## The two existing functions funnel through it

```erlang
default_template_in_txn(ClassNref) ->
find_template_by_name_in_txn(ClassNref, ?DEFAULT_TEMPLATE_NAME).

do_find_template_by_name(ClassNref, Name) ->
case graphdb_mgr:transaction(fun() ->
find_template_by_name_in_txn(ClassNref, Name)
end) of
{ok, {ok, Nref}} -> {ok, Nref};
{ok, not_found} -> not_found;
{error, _} -> not_found
end.
```

`do_default_template/1` and `do_add_template/2` are unchanged callers of
`do_find_template_by_name/2`. There is no double-wrapping:
`default_template_in_txn/1` calls the primitive directly (already in a
caller's txn); `do_find_template_by_name/2` opens exactly one txn around it.

## Behaviour preservation

- `default_template_in_txn/1` returns the identical `{ok, Nref} | not_found`
and still aborts the enclosing transaction on a mnesia read error — its
body is simply the extracted primitive with `?DEFAULT_TEMPLATE_NAME`.
- `do_find_template_by_name/2` keeps its own single transaction and its
`{error, _} -> not_found` swallow. `graphdb_mgr:transaction/1` maps
`{atomic, R} -> {ok, R}`, so the fun's `{ok, Nref}` / `not_found` surface
as `{ok, {ok, Nref}}` / `{ok, not_found}` and map back to the same
`{ok, Nref}` / `not_found` the function returns today.
- Name matching (class NameAttrNref 19 via `template_has_name/2`), the
`kind = template` filter, and `downward_children_by_arc/3` traversal are
byte-identical to both originals.

## Out of scope

- `do_templates_for_class/1` — lists *all* template children with no name
match. A different operation; left alone.
- `do_default_template/1` — a thin identity wrapper over
`do_find_template_by_name/2`, not part of the duplicated walk. Left alone.
- Any change to `graphdb_instance`, `graphdb_mgr`, or the schema.

## Testing

- **Existing template tests pass unchanged** — the behaviour-preservation
proof. This includes the three `default_template_in_txn_*` CT cases and the
gen-server `default_template` / `add_template` cases.
- **Three new CT cases** in `graphdb_class_SUITE` for the newly exported
primitive, invoked via `graphdb_mgr:transaction/1` (so results read as
`{ok, {ok, Nref}}` / `{ok, not_found}`):
1. found-by-name — a named template child (e.g. `"biological"`) resolves to
its nref, distinct from the auto-created default template;
2. discriminates-by-name — searching the same class for `"default"` resolves
to the default template nref, proving the name selects the right template
rather than returning first-match;
3. name-not-found — an absent name resolves to `not_found`.

The `kind = template` guard's reject branch is unreachable through the
public API (composition children of a class with arc 26 are templates by
construction; subclasses attach via a taxonomy arc and are filtered out by
`downward_children_by_arc/3`'s `composition` kind filter before the guard
runs). The guard stays in the code for behaviour preservation but is not
exercised by an artificial injected-state test; a reject-branch test would
be added if a future caller (e.g. `mutate/1`) ever makes non-template
composition children reachable.

## Docs

- `apps/graphdb/CLAUDE.md` — add `find_template_by_name_in_txn/2` to the
tier-1 in-transaction primitive bullet for `graphdb_class`.
- `TASKS.md` — flip the "Converge default-template name search" bullet to
IMPLEMENTED.
- `docs/Architecture.md` — untouched (internal refactor; public contract and
inheritance algorithm unchanged).
Loading
Loading