From 3be1b54482583a61d6a8d43d9bdef294c4dc76cc Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 06:53:04 -0400 Subject: [PATCH 1/9] Slice C design: instance-only qualifying characteristics Design doc for the instance-only QC marker and its class-level enforcement. Decisions: flag lives on the class QC AVP (#{attribute, value => undefined, instance_only => true}); three enforcement gates (bind_qc_value/3, create_class/3, update_node_avps/2) reject a class-level value bind on a marked attribute; local enforcement only (inherited bypass deferred). Records template-attribute-list, template-bound values, and inherited enforcement as TASKS.md follow-ons. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- .../slice-c-instance-only-qc-design.md | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 docs/designs/slice-c-instance-only-qc-design.md diff --git a/docs/designs/slice-c-instance-only-qc-design.md b/docs/designs/slice-c-instance-only-qc-design.md new file mode 100644 index 0000000..0e26fa8 --- /dev/null +++ b/docs/designs/slice-c-instance-only-qc-design.md @@ -0,0 +1,172 @@ + + +# Slice C — Instance-Only Qualifying Characteristics — Design + +## Goal + +Let a class declare a qualifying characteristic (QC) as **instance-only**: +the attribute is relevant to the class, but binding a value *at the class +level* is a category error. Binding belongs only on instances. Slice C adds +the marker and enforces the rejection at every class-level value-binding +gate. + +*Example:* a `Car` class declares `serial_number` as relevant to every car, +but a class-wide serial number is nonsense — each instance carries its own. +`serial_number` is instance-only; `num_wheels = 4` is class-bindable. + +## Background + +QCs are stored as AVPs directly on the class node, keyed by attribute nref: + +| QC state | AVP shape | Meaning | +|------------------------|----------------------------------------|------------------------------------------| +| Declared, unbound | `#{attribute => A, value => undefined}` | instances supply the value | +| Declared, class-bound | `#{attribute => A, value => V}` | shared default; instances may override | + +`add_qualifying_characteristic/2` declares (unbound); `bind_qc_value/3` +binds a value; `create_class/3` accepts an initial AVP list that lands on +the class node. + +The inheritance chain (`TheKnowledgeNetwork.md` §6, `Architecture.md` §9) +has exactly four levels — local, class-bound, compositional ancestor, +connected. There is **no template layer**: a template is consumed at +instantiation, not queried at resolve time. This design does not change the +inheritance chain. + +## Scope + +**In scope** — the instance-only marker on class QCs, and rejection of a +class-level value bind on a marked attribute at three gates. + +**Deferred** (recorded in `TASKS.md`, see below) — the per-template +attribute list (`TheKnowledgeNetwork.md` §7), template-bound variant values, +instance-side stamping at `create_instance`, and inherited instance-only +enforcement. + +## Decision: the flag lives on the class-level QC + +The instance-only/class-bindable distinction is a **binding policy on a +single attribute**, stored where the attribute is declared — the class node. +This is a design choice, not a spec mandate; the spec settles the +inheritance chain and template relevance-scoping but not this locus. Two +practical constraints decide it: + +- Every enforcement point (`create_class`, `update_node_avps`, + `bind_qc_value`) operates on the **class node**. A class-QC marker keeps + enforcement local; a per-template home would force a cross-node template + lookup on every class-AVP write. +- The per-template attribute list (the alternative home) is exactly the + deferred template infrastructure. Building it now to host one flag is + YAGNI against keeping the slice focused. + +## Representation + +An instance-only QC is the declared-unbound QC plus one boolean key, +colocated on the class node: + +```erlang +#{attribute => A, value => undefined, instance_only => true} +``` + +This mirrors the codebase's marker convention (`instantiable => false`, +`retired => true`) but at *attribute* granularity — the flag must ride on a +specific QC, so it is a key on that QC's map rather than a standalone +node-level AVP. + +Erlang map-matching is non-exhaustive, so existing +`#{attribute := A, value := V}` reads keep working unchanged. The only +code that drops the key is code that *rebuilds* a map from extracted +fields — notably `collect_qc_avps/1`, which flattens each QC to a +`{AttrNref, Value}` tuple. That flattening is why inherited enforcement is +deferred (see below). + +## Setting the flag + +| API | Behaviour | +|----------------------------------------------|-----------------------------------------------------------------| +| `add_qualifying_characteristic/3` | `(ClassNref, AttrNref, #{instance_only => true})` — declares an instance-only QC; the existing `/2` stays the unflagged declare | +| `create_class/3` | accepts an instance-only QC in its initial AVP list | + +The flag is **never** set through `update_node_avps`. Slice B's +well-formedness (each update map's key-set is exactly `[attribute]` or +`[attribute, value]`) is untouched; `update_node_avps` only enforces. + +## Enforcement + +Three gates, each reading the **target class node's own AVPs** — all local, +no cross-node lookup. The bare-reason error is consistent with the slice-B +contract: + +```erlang +{instance_only_attribute, AttrNref} +``` + +| Gate | Rejects when… | +|-----------------------|-------------------------------------------------------------------------------------------------| +| `bind_qc_value/3` | the target QC is marked `instance_only` — the direct class-level bind path | +| `create_class/3` | an initial AVP is both `instance_only => true` **and** carries a concrete `value =/= undefined` | +| `update_node_avps/2` | a class node, value-bearing update (`value` key present) targets an attr whose stored entry is `instance_only` | + +`bind_qc_value/3` is included although the task named only `create_class` +and `update_node_avps`: it is the direct class-level value-binding API, so +leaving it unguarded is a trivial bypass. + +`update_node_avps/2` rejects *any* value-bearing update to a marked +attribute — even `value => undefined` — because slice B's upsert replaces +the whole entry and would silently strip the flag. Deletes (no `value` key) +are left alone; removing the QC entirely is not a binding. + +Enforcement applies to class nodes only. Instances are the legal binding +locus; instance nodes never carry the flag, so `update_node_avps` on an +instance never triggers the guard. + +## Inheritance: local enforcement (C1) + +Each gate checks only the class node it is writing. The flag does **not** +propagate through `inherited_qcs/1` (because `collect_qc_avps/1` drops it), +which leaves one known gap: a subclass can re-declare a parent's +instance-only QC *without* the flag via `add_qualifying_characteristic/2`, +then bind a value — bypassing the parent's intent. + +This bypass is **deferred**, not fixed in slice C. Closing it (C2) would +require changing the return shape of `collect_qc_avps/1` / `inherited_qcs/1` +to carry the flag and having all three gates consult the effective (local + +ancestor) QC set — a public-API change with its own consumers and tests, +out of proportion to one flag. The bypass requires a deliberate +re-declaration rather than being an accidental hole. + +## Deferred work to record in `TASKS.md` before the PR + +1. **Template attribute list** — per-template subset/relevance scoping of + class attributes (`TheKnowledgeNetwork.md` §7). +2. **Template-bound (variant) values** — templates carrying override values + stamped into instances at instantiation (the custom-color-phone case). +3. **Inherited instance-only enforcement** — close the subclass-redeclare + bypass (C2 above). + +## Testing + +**EUnit (pure):** + +- `is_instance_only/1` predicate over a QC map. +- `create_class` initial-AVP validator: instance-only + concrete value → + reject; instance-only + `undefined` → accept; non-flagged + value → + accept. +- `update_node_avps` instance-only guard predicate: value-bearing update + against a marked stored entry → reject; delete against a marked entry → + accept; value update against a non-marked entry → accept. + +**CT (integration):** + +- `bind_qc_value` reject path on a marked QC; happy path on a non-marked QC. +- `create_class` reject path (instance-only + value); happy path declaring + an instance-only QC unbound. +- `update_node_avps` reject path (value-bearing update to a marked QC on a + class node); happy path (delete of a marked QC; value update to a + non-marked QC). +- An instance-only QC left `undefined` participates normally where reads + tolerate unbound QCs. +- `verify_caches/0` in `end_per_testcase`, as every suite already does. From 335eeaff4fa537b5dc3467e1d97cd3084405d473 Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 19:52:50 -0400 Subject: [PATCH 2/9] Slice C plan: instance-only qualifying characteristics Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- .../2026-06-27-slice-c-instance-only-qc.md | 958 ++++++++++++++++++ 1 file changed, 958 insertions(+) create mode 100644 docs/superpowers/plans/2026-06-27-slice-c-instance-only-qc.md diff --git a/docs/superpowers/plans/2026-06-27-slice-c-instance-only-qc.md b/docs/superpowers/plans/2026-06-27-slice-c-instance-only-qc.md new file mode 100644 index 0000000..6e3d4f7 --- /dev/null +++ b/docs/superpowers/plans/2026-06-27-slice-c-instance-only-qc.md @@ -0,0 +1,958 @@ + + +# Slice C — Instance-Only Qualifying Characteristics Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use +> superpowers:subagent-driven-development (recommended) or +> superpowers:executing-plans to implement this plan task-by-task. Steps use +> checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let a class declare a qualifying characteristic (QC) as +*instance-only* — relevant to the class but illegal to bind a value at the +class level — and reject every class-level value-bind on a marked attribute. + +**Architecture:** The marker is a boolean key `instance_only => true` +colocated on the class node's QC AVP map. It is set by a new +`add_qualifying_characteristic/3` (or in a `create_class/3` initial AVP +list) and enforced at three class-level value-binding gates: +`bind_qc_value/3`, `create_class/3`, and `update_node_avps/2` (the last +covers `mutate/1` for free, since both compose `update_node_avps_in_txn/3`). +Enforcement is local to the class node being written — no cross-node or +inheritance walk. Three deferred follow-ons (template attribute list, +template-bound variant values, inherited enforcement) are recorded in +`TASKS.md`. + +**Tech Stack:** Erlang/OTP 28.5, rebar3 3.27 (repo-local `./rebar3`), +Mnesia, Common Test, EUnit. + +**Design:** `docs/designs/slice-c-instance-only-qc-design.md` + +## Global Constraints + +- **Hard tabs** for all indentation in every `apps/graphdb/` source and test + file. Never spaces. +- **Error shape** is the bare reason `{error, {instance_only_attribute, + AttrNref}}` at all three gates. Reject-path tests assert the **full + tuple**, never just the tag. +- The marker is **never** settable through `update_node_avps`/`mutate`: slice + B's `validate_avp_updates/1` already rejects any update map whose key-set + is not exactly `[attribute]` or `[attribute, value]`, so an + `instance_only` key in an update map is rejected as an extra key. Do not + loosen `validate_avp_updates/1`. +- Build: `./rebar3 compile` (plain, no `source ~/.bashrc &&` prefix). +- Run EUnit: `./rebar3 eunit --module `. +- Run one CT suite: `./rebar3 ct --suite apps/graphdb/test/`. +- Pure EUnit-tested helpers are exported under the `-ifdef(TEST).` block, + not the public export list (match the existing convention in each module). +- Commit message trailers on every commit: + ``` + Co-Authored-By: Claude Opus 4.8 + Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF + ``` + +--- + +## File Structure + +| File | Change | +|-----------------------------------------------|------------------------------------------------------------------------| +| `apps/graphdb/src/graphdb_class.erl` | `is_instance_only/1`, `validate_instance_only_avps/1`, `is_qc_instance_only/2`; `add_qualifying_characteristic/3` + `do_add_qc/3` + `new_qc_avp/2`; gates in `do_create_class` and `do_bind_qc_value` | +| `apps/graphdb/src/graphdb_mgr.erl` | `check_instance_only/2` (pure) + `guard_instance_only/2`; wire into `update_node_avps_in_txn/3` | +| `apps/graphdb/test/graphdb_class_tests.erl` | EUnit for `is_instance_only/1`, `validate_instance_only_avps/1` | +| `apps/graphdb/test/graphdb_class_SUITE.erl` | CT for `add_qualifying_characteristic/3`, `create_class/3` gate, `bind_qc_value/3` gate | +| `apps/graphdb/test/graphdb_mgr_tests.erl` | EUnit for `check_instance_only/2` | +| `apps/graphdb/test/graphdb_mgr_SUITE.erl` | CT for `update_node_avps`/`mutate` gate | +| `TASKS.md` | Mark instance-only enforcement IMPLEMENTED; carve out 3 deferred items | +| `apps/graphdb/CLAUDE.md` | Add `add_qualifying_characteristic/3` to the class API list | + +No `docs/Architecture.md` change (it documents no QC-binding contract — +verified) and no `docs/diagrams/ontology-tree.md` change (slice C seeds no +new nodes). + +--- + +### Task 1: Pure predicates in `graphdb_class` + +Two pure helpers, no behaviour wiring yet. They feed Tasks 3, 4, and 5. + +**Files:** +- Modify: `apps/graphdb/src/graphdb_class.erl` (add helpers near + `collect_qc_avps/1` ~line 1182; add both to the `-ifdef(TEST).` export + block at lines 154-158) +- Test: `apps/graphdb/test/graphdb_class_tests.erl` + +**Interfaces:** +- Produces: + - `is_instance_only(QcMap :: map()) -> boolean()` — true iff the map + carries `instance_only => true`. + - `validate_instance_only_avps(AVPs :: [map()]) -> ok | {error, + {instance_only_attribute, integer()}}` — rejects the first entry that is + both `instance_only => true` and `value =/= undefined`. + +- [ ] **Step 1: Write the failing EUnit tests** + +Append to `apps/graphdb/test/graphdb_class_tests.erl`: + +```erlang +%%============================================================================= +%% is_instance_only/1 tests +%%============================================================================= + +is_instance_only_true_test() -> + ?assert(graphdb_class:is_instance_only( + #{attribute => 42, value => undefined, instance_only => true})). + +is_instance_only_absent_key_test() -> + ?assertNot(graphdb_class:is_instance_only( + #{attribute => 42, value => undefined})). + +is_instance_only_false_value_test() -> + ?assertNot(graphdb_class:is_instance_only( + #{attribute => 42, value => undefined, instance_only => false})). + +%%============================================================================= +%% validate_instance_only_avps/1 tests +%%============================================================================= + +validate_instance_only_avps_empty_test() -> + ?assertEqual(ok, graphdb_class:validate_instance_only_avps([])). + +validate_instance_only_avps_unbound_ok_test() -> + %% instance_only declared but unbound is legal at the class level. + ?assertEqual(ok, graphdb_class:validate_instance_only_avps( + [#{attribute => 42, value => undefined, instance_only => true}])). + +validate_instance_only_avps_non_flagged_value_ok_test() -> + %% A normal class-bound value is legal. + ?assertEqual(ok, graphdb_class:validate_instance_only_avps( + [#{attribute => 42, value => "red"}])). + +validate_instance_only_avps_rejects_flagged_value_test() -> + ?assertEqual({error, {instance_only_attribute, 42}}, + graphdb_class:validate_instance_only_avps( + [#{attribute => 42, value => "red", instance_only => true}])). +``` + +- [ ] **Step 2: Run them, verify failure** + +Run: `./rebar3 eunit --module graphdb_class_tests` +Expected: FAIL — `is_instance_only/1` / `validate_instance_only_avps/1` +undefined (function not exported / does not exist). + +- [ ] **Step 3: Add the helpers and exports** + +In `apps/graphdb/src/graphdb_class.erl`, extend the `-ifdef(TEST).` export +block (currently lines 154-158): + +```erlang +-ifdef(TEST). +-export([ + is_valid_parent_kind/1, + collect_qc_avps/1, + is_instance_only/1, + validate_instance_only_avps/1 + ]). +-endif. +``` + +Add the two functions in the internal-functions region (next to +`collect_qc_avps/1`): + +```erlang +%%----------------------------------------------------------------------------- +%% is_instance_only(QcMap) -> boolean() +%% +%% True iff a qualifying-characteristic AVP map carries the +%% `instance_only => true` marker. Pure; consumed by the bind_qc_value +%% and create_class enforcement gates. +%%----------------------------------------------------------------------------- +is_instance_only(#{instance_only := true}) -> true; +is_instance_only(_) -> false. + + +%%----------------------------------------------------------------------------- +%% validate_instance_only_avps(AVPs) -> +%% ok | {error, {instance_only_attribute, integer()}} +%% +%% Rejects an initial create_class AVP list in which any entry is both +%% marked `instance_only => true` AND carries a concrete (non-undefined) +%% value. An instance-only QC declared unbound (value => undefined) is +%% accepted. Pure; returns the first offending attribute nref. +%%----------------------------------------------------------------------------- +validate_instance_only_avps(AVPs) -> + case [A || #{attribute := A, value := V} = E <- AVPs, + V =/= undefined, is_instance_only(E)] of + [] -> ok; + [A | _] -> {error, {instance_only_attribute, A}} + end. +``` + +- [ ] **Step 4: Run the tests, verify pass** + +Run: `./rebar3 eunit --module graphdb_class_tests` +Expected: PASS (all, including the pre-existing tests). + +- [ ] **Step 5: Commit** + +```bash +git add apps/graphdb/src/graphdb_class.erl apps/graphdb/test/graphdb_class_tests.erl +git commit -m "Slice C: instance-only QC pure predicates" +``` + +--- + +### Task 2: `add_qualifying_characteristic/3` — set the marker + +Adds the flag-setting API. The existing `/2` stays the unflagged declare and +is refactored to delegate to a new `/3` internal worker. + +**Files:** +- Modify: `apps/graphdb/src/graphdb_class.erl` (public export ~line 114; + public fn ~line 233; `handle_call` ~line 371; `do_add_qc` ~line 977) +- Test: `apps/graphdb/test/graphdb_class_SUITE.erl` + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: + - `add_qualifying_characteristic(ClassNref, AttrNref, Opts :: map()) -> ok + | {error, term()}` — `Opts = #{instance_only => true}` stamps the + marker; any other `Opts` behaves like `/2`. + - Stored marked QC shape on the class node: + `#{attribute => AttrNref, value => undefined, instance_only => true}`. + +**Idempotency contract (load-bearing — do not "fix"):** `do_add_qc` returns +`already_exists -> ok` when ANY entry for `AttrNref` already exists. So +calling `/3` with `instance_only` on an already-declared, **unflagged** QC is +a silent no-op — it does **not** upgrade the existing entry. This is +deliberate and consistent with the existing `/2` idempotency contract; +Step 1 includes a test pinning it. + +- [ ] **Step 1: Write the failing CT tests** + +In `apps/graphdb/test/graphdb_class_SUITE.erl`, add these clause names to +the test list in both the `-export([...])` testcase block (~lines 61-110) +and the corresponding `groups()` list (~lines 153-204), beside the existing +`bind_qc_value_*` entries: + +``` + add_qc_3_stamps_instance_only_marker, + add_qc_3_idempotent_does_not_upgrade, +``` + +Add the test bodies (place them beside `bind_qc_value_basic`; follow its +setup idiom — call `graphdb_class:start_link()` at the top, `init_per_testcase` +brings up the rest): + +```erlang +%%----------------------------------------------------------------------------- +%% add_qualifying_characteristic/3 with #{instance_only => true} stamps the +%% marker onto the class node's QC AVP. +%%----------------------------------------------------------------------------- +add_qc_3_stamps_instance_only_marker(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, Veh} = graphdb_class:create_class("Vehicle", ?NREF_CLASSES), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN, + #{instance_only => true}), + {ok, Node} = graphdb_class:get_class(Veh), + ?assert(lists:member( + #{attribute => AttrN, value => undefined, instance_only => true}, + Node#node.attribute_value_pairs)). + +%%----------------------------------------------------------------------------- +%% Idempotency: /3 with instance_only on an already-declared UNFLAGGED QC is +%% a no-op — it returns ok but does NOT upgrade the stored entry. +%%----------------------------------------------------------------------------- +add_qc_3_idempotent_does_not_upgrade(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, Veh} = graphdb_class:create_class("Vehicle", ?NREF_CLASSES), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN, + #{instance_only => true}), + {ok, Node} = graphdb_class:get_class(Veh), + %% The original unflagged entry is preserved; no marked variant appears. + ?assert(lists:member(#{attribute => AttrN, value => undefined}, + Node#node.attribute_value_pairs)), + ?assertNot(lists:member( + #{attribute => AttrN, value => undefined, instance_only => true}, + Node#node.attribute_value_pairs)). +``` + +- [ ] **Step 2: Run them, verify failure** + +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE` +Expected: FAIL — `add_qualifying_characteristic/3` undefined. + +- [ ] **Step 3: Add export, public fn, handler, and `/3` worker** + +In the public `-export([...])` list, add `add_qualifying_characteristic/3` +beside the `/2`: + +```erlang + add_qualifying_characteristic/2, + add_qualifying_characteristic/3, +``` + +Add the public function after the existing `/2` clause (~line 233): + +```erlang +add_qualifying_characteristic(ClassNref, AttrNref, Opts) when is_map(Opts) -> + gen_server:call(?MODULE, + {add_qualifying_characteristic, ClassNref, AttrNref, Opts}). +``` + +Add the `handle_call` clause after the existing `/2` clause (~line 371): + +```erlang +handle_call({add_qualifying_characteristic, ClassNref, AttrNref, Opts}, _From, + State) -> + {reply, do_add_qc(ClassNref, AttrNref, Opts), State}; +``` + +Refactor `do_add_qc/2` (~line 977) to delegate to a new `/3`, and add +`new_qc_avp/2`: + +```erlang +do_add_qc(ClassNref, AttrNref) -> + do_add_qc(ClassNref, AttrNref, #{}). + +do_add_qc(ClassNref, AttrNref, Opts) -> + Txn = fun() -> + case mnesia:read(nodes, ClassNref) of + [#node{kind = class, attribute_value_pairs = AVPs} = Node] -> + case mnesia:read(nodes, AttrNref) of + [#node{kind = attribute}] -> + Already = lists:any(fun(#{attribute := A}) -> A =:= AttrNref; + (_) -> false + end, AVPs), + case Already of + true -> + already_exists; + false -> + NewAVP = new_qc_avp(AttrNref, Opts), + Updated = Node#node{ + attribute_value_pairs = AVPs ++ [NewAVP] + }, + ok = mnesia:write(nodes, Updated, write), + ok + end; + [#node{}] -> + {error, {not_an_attribute, AttrNref}}; + [] -> + {error, {attribute_not_found, AttrNref}} + end; + [#node{kind = Kind}] -> + {error, {not_a_class, Kind}}; + [] -> + {error, not_found} + end + end, + case graphdb_mgr:transaction(Txn) of + {ok, ok} -> ok; + {ok, already_exists} -> ok; + {ok, {error, _} = E} -> E; + {error, Reason} -> {error, Reason} + end. + +%%----------------------------------------------------------------------------- +%% new_qc_avp(AttrNref, Opts) -> map() +%% +%% Builds the QC AVP for a fresh declaration. `instance_only => true` in +%% Opts stamps the marker onto the canonical declared-unbound shape; +%% otherwise the plain declared-unbound shape is returned. +%%----------------------------------------------------------------------------- +new_qc_avp(AttrNref, Opts) -> + Base = #{attribute => AttrNref, value => undefined}, + case maps:get(instance_only, Opts, false) of + true -> Base#{instance_only => true}; + false -> Base + end. +``` + +- [ ] **Step 4: Run the new CT tests, verify pass** + +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE` +Expected: PASS (including the two new tests). + +- [ ] **Step 5: Refactor regression — existing class tests stay green** + +Confirms the `/2 → /3` collapse is behaviour-preserving (empty `Opts` +produces exactly `#{attribute => A, value => undefined}`, no +`instance_only` key). + +Run: `./rebar3 eunit --module graphdb_class_tests` +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE` +Expected: PASS, both, with no regressions. + +- [ ] **Step 6: Commit** + +```bash +git add apps/graphdb/src/graphdb_class.erl apps/graphdb/test/graphdb_class_SUITE.erl +git commit -m "Slice C: add_qualifying_characteristic/3 stamps instance-only marker" +``` + +--- + +### Task 3: `create_class/3` enforcement gate + +Reject an initial AVP list carrying an `instance_only => true` entry with a +concrete value. + +**Files:** +- Modify: `apps/graphdb/src/graphdb_class.erl` (`do_create_class/4` ~line 471) +- Test: `apps/graphdb/test/graphdb_class_SUITE.erl` + +**Interfaces:** +- Consumes: `validate_instance_only_avps/1` (Task 1). +- Produces: `create_class/3` returns `{error, {instance_only_attribute, + AttrNref}}` when an initial AVP is instance-only with a concrete value. + +- [ ] **Step 1: Write the failing CT tests** + +Add the names to the testcase `-export` block and `groups()` list (beside +`create_class_3_writes_avps`): + +``` + create_class_3_rejects_instance_only_with_value, + create_class_3_accepts_instance_only_unbound, +``` + +Add the bodies beside `create_class_3_writes_avps`: + +```erlang +%%----------------------------------------------------------------------------- +%% create_class/3 rejects an initial AVP that is instance_only AND bound. +%%----------------------------------------------------------------------------- +create_class_3_rejects_instance_only_with_value(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + Bad = #{attribute => AttrN, value => "SN-1", instance_only => true}, + ?assertEqual({error, {instance_only_attribute, AttrN}}, + graphdb_class:create_class("Bad", 3, [Bad])). + +%%----------------------------------------------------------------------------- +%% create_class/3 accepts an instance_only QC declared unbound. +%%----------------------------------------------------------------------------- +create_class_3_accepts_instance_only_unbound(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + Good = #{attribute => AttrN, value => undefined, instance_only => true}, + {ok, ClassNref} = graphdb_class:create_class("Good", 3, [Good]), + {ok, Node} = graphdb_class:get_class(ClassNref), + ?assert(lists:member(Good, Node#node.attribute_value_pairs)). +``` + +- [ ] **Step 2: Run them, verify failure** + +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE` +Expected: FAIL — the reject test gets `{ok, _}` instead of the error tuple. + +- [ ] **Step 3: Wire the gate into `do_create_class`** + +Wrap the existing body of `do_create_class/4` (~line 471) so the AVP +validation runs first: + +```erlang +do_create_class(Name, ParentClassNref, AVPs, InstAttr) -> + case validate_instance_only_avps(AVPs) of + {error, _} = Err -> + Err; + ok -> + case do_validate_parent(ParentClassNref) of + ok -> + ClassNref = graphdb_nref:get_next(), + {TaxId1, TaxId2} = rel_id_server:get_id_pair(), + ClassNameAVP = #{attribute => ?NAME_ATTR_CLASS, value => Name}, + ClassNode = #node{ + nref = ClassNref, + kind = class, + parents = [ParentClassNref], + attribute_value_pairs = [ClassNameAVP | AVPs] + }, + TaxP2C = #relationship{ + id = TaxId1, kind = taxonomy, + source_nref = ParentClassNref, + characterization = ?ARC_CLS_CHILD, + target_nref = ClassNref, + reciprocal = ?ARC_CLS_PARENT, + avps = [] + }, + TaxC2P = #relationship{ + id = TaxId2, kind = taxonomy, + source_nref = ClassNref, + characterization = ?ARC_CLS_PARENT, + target_nref = ParentClassNref, + reciprocal = ?ARC_CLS_CHILD, + avps = [] + }, + TemplateRows = template_rows(ClassNref, AVPs, InstAttr), + Txn = fun() -> + ok = mnesia:write(nodes, ClassNode, write), + ok = mnesia:write(relationships, TaxP2C, write), + ok = mnesia:write(relationships, TaxC2P, write), + [ ok = mnesia:write(T, R, write) || {T, R} <- TemplateRows ] + end, + case graphdb_mgr:transaction(Txn) of + {ok, _Writes} -> {ok, ClassNref}; + {error, _} = Err -> Err + end; + {error, _} = Err -> + Err + end + end. +``` + +(Only the outer `case validate_instance_only_avps(AVPs) of ... ok ->` wrap +and its closing `end` are new; the inner body is unchanged from the current +`do_validate_parent` arm.) + +- [ ] **Step 4: Run the CT tests, verify pass** + +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE` +Expected: PASS (both new tests, plus the existing `create_class_3_*` tests). + +- [ ] **Step 5: Commit** + +```bash +git add apps/graphdb/src/graphdb_class.erl apps/graphdb/test/graphdb_class_SUITE.erl +git commit -m "Slice C: create_class/3 rejects bound instance-only QC" +``` + +--- + +### Task 4: `bind_qc_value/3` enforcement gate + +Reject a class-level value bind on a QC marked instance-only. + +**Files:** +- Modify: `apps/graphdb/src/graphdb_class.erl` (`do_bind_qc_value/3` + ~line 1025; add `is_qc_instance_only/2` helper) +- Test: `apps/graphdb/test/graphdb_class_SUITE.erl` + +**Interfaces:** +- Consumes: `is_instance_only/1` (Task 1), `add_qualifying_characteristic/3` + (Task 2). +- Produces: `bind_qc_value/3` returns `{error, {instance_only_attribute, + AttrNref}}` when the target QC is marked instance-only. + +The happy path on a non-marked QC is already covered by the existing +`bind_qc_value_basic` test — no new happy-path test is needed. + +- [ ] **Step 1: Write the failing CT test** + +Add the name to the testcase `-export` block and `groups()` list (beside +`bind_qc_value_basic`): + +``` + bind_qc_value_rejects_instance_only, +``` + +Add the body beside `bind_qc_value_basic`: + +```erlang +%%----------------------------------------------------------------------------- +%% bind_qc_value/3 refuses to bind a value on an instance-only QC, and the +%% transaction abort leaves the QC unbound. +%%----------------------------------------------------------------------------- +bind_qc_value_rejects_instance_only(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, Veh} = graphdb_class:create_class("Vehicle", ?NREF_CLASSES), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN, + #{instance_only => true}), + ?assertEqual({error, {instance_only_attribute, AttrN}}, + graphdb_class:bind_qc_value(Veh, AttrN, "SN-1")), + %% Abort rolled back the write: the QC is still declared, still unbound. + {ok, QCs} = graphdb_class:inherited_qcs(Veh), + ?assert(lists:member({AttrN, undefined}, QCs)). +``` + +- [ ] **Step 2: Run it, verify failure** + +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE` +Expected: FAIL — `bind_qc_value/3` binds the value and returns `ok`. + +- [ ] **Step 3: Add the guard to `do_bind_qc_value`** + +In `do_bind_qc_value/3` (~line 1025), replace the `true ->` arm of the +`case Declared of` with an instance-only check: + +```erlang + case Declared of + false -> + mnesia:abort(qc_not_declared); + true -> + case is_qc_instance_only(AVPs, AttrNref) of + true -> + mnesia:abort( + {instance_only_attribute, AttrNref}); + false -> + NewAVPs = update_qc_value(AVPs, AttrNref, + Value), + mnesia:write(nodes, + N#node{attribute_value_pairs = NewAVPs}, + write) + end + end; +``` + +Add the helper next to `update_qc_value/3` (~line 1058): + +```erlang +%%----------------------------------------------------------------------------- +%% is_qc_instance_only(AVPs, AttrNref) -> boolean() +%% +%% True iff the QC entry for AttrNref in AVPs carries the instance_only +%% marker. Caller has already verified AttrNref is present. +%%----------------------------------------------------------------------------- +is_qc_instance_only(AVPs, AttrNref) -> + lists:any(fun(#{attribute := A} = E) when A =:= AttrNref -> + is_instance_only(E); + (_) -> false + end, AVPs). +``` + +(`do_bind_qc_value/3` already maps `{error, _} = Err -> Err`, and +`graphdb_mgr:transaction/1` maps `{aborted, R} -> {error, R}`, so the abort +surfaces as `{error, {instance_only_attribute, AttrNref}}`.) + +- [ ] **Step 4: Run the CT tests, verify pass** + +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE` +Expected: PASS (new reject test plus existing `bind_qc_value_*`). + +- [ ] **Step 5: Commit** + +```bash +git add apps/graphdb/src/graphdb_class.erl apps/graphdb/test/graphdb_class_SUITE.erl +git commit -m "Slice C: bind_qc_value/3 rejects instance-only QC" +``` + +--- + +### Task 5: `update_node_avps` / `mutate` enforcement gate + +Reject a value-bearing update targeting a stored entry marked instance-only. +Both the tier-2 solo path (`update_node_avps/2`) and the tier-3 batch path +(`mutate/1`) compose `update_node_avps_in_txn/3`, so guarding it once covers +both. + +**Files:** +- Modify: `apps/graphdb/src/graphdb_mgr.erl` (`update_node_avps_in_txn/3` + ~line 752; add `check_instance_only/2` + `guard_instance_only/2`; export + `check_instance_only/2` in the `-ifdef(TEST).` block at lines 149-155) +- Test: `apps/graphdb/test/graphdb_mgr_tests.erl` (EUnit), + `apps/graphdb/test/graphdb_mgr_SUITE.erl` (CT) + +**Interfaces:** +- Consumes: the stored marker shape from Task 2. +- Produces: `update_node_avps/2` and `mutate/1` return / abort with + `{error, {instance_only_attribute, AttrNref}}` on a value-bearing update + to a marked stored entry. + - `check_instance_only(StoredAVPs :: [map()], Updates :: [map()]) -> ok | + {error, {instance_only_attribute, integer()}}` (pure). + +A value-bearing update is any update map carrying a `value` key — **including +`value => undefined`** — because slice B's `upsert_avp/3` rebuilds the entry +as `#{attribute => A, value => V}` and would silently strip the marker. A +delete (no `value` key) is permitted: removing the QC entirely is not a bind. + +- [ ] **Step 1: Write the failing EUnit tests** + +Append to `apps/graphdb/test/graphdb_mgr_tests.erl`: + +```erlang +%% check_instance_only/2 tests (pure) + +check_instance_only_rejects_value_bearing_test() -> + Stored = [#{attribute => 42, value => undefined, instance_only => true}], + Updates = [#{attribute => 42, value => "SN-1"}], + ?assertEqual({error, {instance_only_attribute, 42}}, + graphdb_mgr:check_instance_only(Stored, Updates)). + +check_instance_only_rejects_undefined_value_test() -> + %% value => undefined still carries a `value` key -> still a bind attempt. + Stored = [#{attribute => 42, value => undefined, instance_only => true}], + Updates = [#{attribute => 42, value => undefined}], + ?assertEqual({error, {instance_only_attribute, 42}}, + graphdb_mgr:check_instance_only(Stored, Updates)). + +check_instance_only_allows_delete_test() -> + Stored = [#{attribute => 42, value => undefined, instance_only => true}], + Updates = [#{attribute => 42}], + ?assertEqual(ok, graphdb_mgr:check_instance_only(Stored, Updates)). + +check_instance_only_allows_non_marked_test() -> + Stored = [#{attribute => 42, value => undefined}], + Updates = [#{attribute => 42, value => "red"}], + ?assertEqual(ok, graphdb_mgr:check_instance_only(Stored, Updates)). +``` + +- [ ] **Step 2: Run them, verify failure** + +Run: `./rebar3 eunit --module graphdb_mgr_tests` +Expected: FAIL — `check_instance_only/2` undefined. + +- [ ] **Step 3: Add the predicate, guard, export, and wiring** + +Extend the `-ifdef(TEST).` export block (lines 149-155) to add +`check_instance_only/2`: + +```erlang +-ifdef(TEST). +-export([ + validate_avp_updates/1, + apply_avp_updates/2, + check_instance_only/2 + ]). +-endif. +``` + +Add the predicate and guard near `guard_attribute_existence/1` (~line 773): + +```erlang +%%----------------------------------------------------------------------------- +%% check_instance_only(StoredAVPs, Updates) -> +%% ok | {error, {instance_only_attribute, integer()}} +%% +%% Pure. A value-bearing update (one carrying a `value` key, including +%% value => undefined) targeting a stored entry marked +%% `instance_only => true` is rejected. Deletes (no `value` key) and +%% updates to non-marked attributes pass. Returns the first offender. +%%----------------------------------------------------------------------------- +check_instance_only(StoredAVPs, Updates) -> + Marked = [A || #{attribute := A} = E <- StoredAVPs, + maps:get(instance_only, E, false) =:= true], + ValueBearing = [A || #{attribute := A} = M <- Updates, + maps:is_key(value, M)], + case [A || A <- ValueBearing, lists:member(A, Marked)] of + [] -> ok; + [A | _] -> {error, {instance_only_attribute, A}} + end. + +%% In-txn guard: aborts on the first instance-only violation. +guard_instance_only(StoredAVPs, Updates) -> + case check_instance_only(StoredAVPs, Updates) of + ok -> ok; + {error, {instance_only_attribute, _} = R} -> mnesia:abort(R) + end. +``` + +Wire the guard into `update_node_avps_in_txn/3` (~line 752), reusing the +already-read `Node`: + +```erlang +update_node_avps_in_txn(Nref, AVPs, RetAttr) -> + case mnesia:read(nodes, Nref, write) of + [] -> + mnesia:abort(not_found); + [Node] -> + ok = guard_retired_marker(AVPs, RetAttr), + ok = guard_instance_only(Node#node.attribute_value_pairs, AVPs), + ok = guard_attribute_existence(AVPs), + New = apply_avp_updates(Node#node.attribute_value_pairs, AVPs), + mnesia:write(nodes, Node#node{attribute_value_pairs = New}, write) + end. +``` + +- [ ] **Step 4: Run the EUnit tests, verify pass** + +Run: `./rebar3 eunit --module graphdb_mgr_tests` +Expected: PASS. + +- [ ] **Step 5: Write the failing CT tests** + +In `apps/graphdb/test/graphdb_mgr_SUITE.erl`, add the three names to the +testcase `-export` block (~lines 116-127) and the `groups()` list +(~lines 212-228), beside the existing `update_node_avps_*` entries: + +``` + update_node_avps_rejects_instance_only, + update_node_avps_delete_instance_only_ok, + mutate_rejects_instance_only, +``` + +**Critical:** also add all three names to the worker-starting +`init_per_testcase/2` guard list (the `when TC =:= ... ` chain at +~lines 273-312) so the suite starts `graphdb_class`/`graphdb_attr`/etc. for +them. Append them to that `;`-separated guard. + +Add the bodies (follow the `mutate_single_update_node_avps` idiom): + +```erlang +%%----------------------------------------------------------------------------- +%% update_node_avps/2 rejects a value-bearing update to a class node's +%% instance-only QC, and rolls the write back. +%%----------------------------------------------------------------------------- +update_node_avps_rejects_instance_only(_Config) -> + {ok, ClassNref} = graphdb_class:create_class("IOClass", 3), + {ok, Attr} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(ClassNref, Attr, + #{instance_only => true}), + ?assertEqual({error, {instance_only_attribute, Attr}}, + graphdb_mgr:update_node_avps(ClassNref, + [#{attribute => Attr, value => "SN-1"}])), + %% Rollback: the QC stays declared-unbound, marker intact. + [#node{attribute_value_pairs = AVPs}] = mnesia:dirty_read(nodes, ClassNref), + ?assert(lists:member( + #{attribute => Attr, value => undefined, instance_only => true}, AVPs)). + +%%----------------------------------------------------------------------------- +%% update_node_avps/2 permits DELETING an instance-only QC (no `value` key). +%%----------------------------------------------------------------------------- +update_node_avps_delete_instance_only_ok(_Config) -> + {ok, ClassNref} = graphdb_class:create_class("IODelClass", 3), + {ok, Attr} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(ClassNref, Attr, + #{instance_only => true}), + ?assertEqual(ok, + graphdb_mgr:update_node_avps(ClassNref, [#{attribute => Attr}])), + [#node{attribute_value_pairs = AVPs}] = mnesia:dirty_read(nodes, ClassNref), + ?assertNot(lists:any(fun(#{attribute := A}) -> A =:= Attr; + (_) -> false end, AVPs)). + +%%----------------------------------------------------------------------------- +%% mutate/1 inherits the instance-only guard via update_node_avps_in_txn. +%%----------------------------------------------------------------------------- +mutate_rejects_instance_only(_Config) -> + {ok, ClassNref} = graphdb_class:create_class("IOMutClass", 3), + {ok, Attr} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(ClassNref, Attr, + #{instance_only => true}), + ?assertEqual({error, {instance_only_attribute, Attr}}, + graphdb_mgr:mutate([{update_node_avps, ClassNref, + [#{attribute => Attr, value => "SN-1"}]}])), + [#node{attribute_value_pairs = AVPs}] = mnesia:dirty_read(nodes, ClassNref), + ?assert(lists:member( + #{attribute => Attr, value => undefined, instance_only => true}, AVPs)). +``` + +- [ ] **Step 6: Run the CT tests, verify failure then pass** + +Run (after Step 5, before Step 3's wiring would have been done — but since +Step 3 is already in, expect PASS here): +`./rebar3 ct --suite apps/graphdb/test/graphdb_mgr_SUITE` +Expected: PASS (the three new tests plus all existing `update_node_avps_*` +and `mutate_*`). If you author Step 5 before Step 3, the new tests FAIL +first (writes succeed) — that is the TDD red. + +- [ ] **Step 7: Commit** + +```bash +git add apps/graphdb/src/graphdb_mgr.erl apps/graphdb/test/graphdb_mgr_tests.erl apps/graphdb/test/graphdb_mgr_SUITE.erl +git commit -m "Slice C: update_node_avps/mutate reject instance-only value bind" +``` + +--- + +### Task 6: Documentation + +Record the deferred follow-ons in `TASKS.md` (an explicit pre-PR +requirement), mark the implemented enforcement, and add the new API to the +graphdb app guide. + +**Files:** +- Modify: `TASKS.md` (slice C section, ~lines 284-306) +- Modify: `apps/graphdb/CLAUDE.md` (class API list) + +- [ ] **Step 1: Rewrite the `TASKS.md` slice C section** + +Replace the section starting `### Template attribute list and instance-only +enforcement (slice C, depends on slice B)` (~line 284) through its final +paragraph (ending `...stays `undefined` at every class level.`, ~line 306) +with: + +```markdown +### Instance-only qualifying characteristics (slice C) — IMPLEMENTED + +A class QC may be marked `instance_only => true`: the attribute is relevant +to instances, but binding a value at the class level is a category error. +Set via `graphdb_class:add_qualifying_characteristic/3` (`#{instance_only => +true}`) or a `create_class/3` initial AVP. Enforced at three class-level +value-binding gates — `bind_qc_value/3`, `create_class/3`, and +`update_node_avps/2` (the last covers `mutate/1`, both composing +`update_node_avps_in_txn/3`) — each returning `{error, +{instance_only_attribute, AttrNref}}`. Enforcement is local to the class +node written. Design `docs/designs/slice-c-instance-only-qc-design.md`. + +**Deferred follow-ons (from slice C):** + +- **Template attribute list** — per-template subset/relevance scoping of + class attributes (`TheKnowledgeNetwork.md` §7). A template currently + carries only a name and its compositional arc into the owning class; there + is no per-template list of which attributes it scopes. This is the + per-class, per-template axis: the same attribute may be class-bindable in + one class's template and instance-only in another's. +- **Template-bound (variant) values** — templates carrying override values + stamped into instances at instantiation (e.g. a later custom-colour phone + variant whose colour is fixed in a template, not on the base class). +- **Inherited instance-only enforcement (C2)** — close the subclass-redeclare + bypass: a subclass can re-declare a parent's instance-only QC *without* the + flag via `add_qualifying_characteristic/2`, then bind a value. Local gates + do not consult the inherited QC set because `collect_qc_avps/1` flattens + each QC to `{AttrNref, Value}`, dropping the marker. Closing it means + carrying the flag through `collect_qc_avps/1` / `inherited_qcs/1` and + having all three gates consult the effective (local + ancestor) QC set. +``` + +- [ ] **Step 2: Add the new API to `apps/graphdb/CLAUDE.md`** + +In the `### graphdb_class — Taxonomic Hierarchy` API list, replace the +`add_qualifying_characteristic/2` line with: + +```markdown +- `add_qualifying_characteristic/2,3` (class_nref, attribute_nref [, opts]) — the `/3` form takes an options map; `#{instance_only => true}` marks the QC instance-only (binding a class-level value for it is rejected at `bind_qc_value/3`, `create_class/3`, and `update_node_avps/2`) +``` + +- [ ] **Step 3: Verify the docs render and reference real paths** + +Run: `grep -n "instance-only\|instance_only\|add_qualifying_characteristic/3" TASKS.md apps/graphdb/CLAUDE.md` +Expected: the new lines appear; the design-doc path resolves +(`ls docs/designs/slice-c-instance-only-qc-design.md`). + +- [ ] **Step 4: Commit** + +```bash +git add TASKS.md apps/graphdb/CLAUDE.md +git commit -m "Slice C: docs — mark enforcement implemented, record deferred follow-ons" +``` + +--- + +## Final Verification + +After all tasks, run the full graphdb test set and confirm zero warnings: + +Run: `./rebar3 compile` +Expected: clean, no warnings. + +Run: `./rebar3 eunit --module graphdb_class_tests --module graphdb_mgr_tests` +Expected: PASS. + +Run: `./rebar3 ct --suite apps/graphdb/test/graphdb_class_SUITE --suite apps/graphdb/test/graphdb_mgr_SUITE` +Expected: PASS, including the per-testcase `verify_caches/0` invariant. + +Optionally run the fast full CT fan-out: `make test-ct-parallel`. + +--- + +## Self-Review Notes + +- **Spec coverage:** marker representation (Task 1 + 2), setting via `/3` and + `create_class/3` (Task 2 + 3), three enforcement gates (Tasks 3/4/5), + local-only C1 inheritance (no inherited walk added — by omission), + deferred-items recording (Task 6), EUnit + CT shape (every task). All + design sections map to a task. +- **Error shape:** every reject path returns the full tuple `{error, + {instance_only_attribute, AttrNref}}`; every reject-path test asserts the + full tuple. +- **Marker cannot enter via update maps:** unchanged slice-B + `validate_avp_updates/1` rejects the `instance_only` extra key — no code + needed, stated in Global Constraints. +- **Type consistency:** `is_instance_only/1`, `validate_instance_only_avps/1`, + `is_qc_instance_only/2`, `new_qc_avp/2`, `check_instance_only/2`, + `guard_instance_only/2` — names used consistently across tasks. From 9d25769d0f199618994a90610785459a4c03f3bc Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 19:56:06 -0400 Subject: [PATCH 3/9] Slice C: instance-only QC pure predicates Add two pure helper functions to graphdb_class for instance-only qualifying characteristics: is_instance_only/1 and validate_instance_only_avps/1. These feed Tasks 3, 4, and 5. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- apps/graphdb/src/graphdb_class.erl | 32 ++++++++++++++++++- apps/graphdb/test/graphdb_class_tests.erl | 39 +++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/apps/graphdb/src/graphdb_class.erl b/apps/graphdb/src/graphdb_class.erl index 250b0b5..e412d90 100644 --- a/apps/graphdb/src/graphdb_class.erl +++ b/apps/graphdb/src/graphdb_class.erl @@ -154,7 +154,9 @@ -ifdef(TEST). -export([ is_valid_parent_kind/1, - collect_qc_avps/1 + collect_qc_avps/1, + is_instance_only/1, + validate_instance_only_avps/1 ]). -endif. @@ -1191,3 +1193,31 @@ collect_qc_avps(Nodes) -> end end, Acc, AVPs) end, [], Nodes). + + +%%----------------------------------------------------------------------------- +%% is_instance_only(QcMap) -> boolean() +%% +%% True iff a qualifying-characteristic AVP map carries the +%% `instance_only => true` marker. Pure; consumed by the bind_qc_value +%% and create_class enforcement gates. +%%----------------------------------------------------------------------------- +is_instance_only(#{instance_only := true}) -> true; +is_instance_only(_) -> false. + + +%%----------------------------------------------------------------------------- +%% validate_instance_only_avps(AVPs) -> +%% ok | {error, {instance_only_attribute, integer()}} +%% +%% Rejects an initial create_class AVP list in which any entry is both +%% marked `instance_only => true` AND carries a concrete (non-undefined) +%% value. An instance-only QC declared unbound (value => undefined) is +%% accepted. Pure; returns the first offending attribute nref. +%%----------------------------------------------------------------------------- +validate_instance_only_avps(AVPs) -> + case [A || #{attribute := A, value := V} = E <- AVPs, + V =/= undefined, is_instance_only(E)] of + [] -> ok; + [A | _] -> {error, {instance_only_attribute, A}} + end. diff --git a/apps/graphdb/test/graphdb_class_tests.erl b/apps/graphdb/test/graphdb_class_tests.erl index 252cc74..6d5afa7 100644 --- a/apps/graphdb/test/graphdb_class_tests.erl +++ b/apps/graphdb/test/graphdb_class_tests.erl @@ -96,3 +96,42 @@ collect_qc_avps_filters_name_avp_test() -> ]}, ?assertEqual([{18, undefined}], graphdb_class:collect_qc_avps([Node])). + + +%%============================================================================= +%% is_instance_only/1 tests +%%============================================================================= + +is_instance_only_true_test() -> + ?assert(graphdb_class:is_instance_only( + #{attribute => 42, value => undefined, instance_only => true})). + +is_instance_only_absent_key_test() -> + ?assertNot(graphdb_class:is_instance_only( + #{attribute => 42, value => undefined})). + +is_instance_only_false_value_test() -> + ?assertNot(graphdb_class:is_instance_only( + #{attribute => 42, value => undefined, instance_only => false})). + +%%============================================================================= +%% validate_instance_only_avps/1 tests +%%============================================================================= + +validate_instance_only_avps_empty_test() -> + ?assertEqual(ok, graphdb_class:validate_instance_only_avps([])). + +validate_instance_only_avps_unbound_ok_test() -> + %% instance_only declared but unbound is legal at the class level. + ?assertEqual(ok, graphdb_class:validate_instance_only_avps( + [#{attribute => 42, value => undefined, instance_only => true}])). + +validate_instance_only_avps_non_flagged_value_ok_test() -> + %% A normal class-bound value is legal. + ?assertEqual(ok, graphdb_class:validate_instance_only_avps( + [#{attribute => 42, value => "red"}])). + +validate_instance_only_avps_rejects_flagged_value_test() -> + ?assertEqual({error, {instance_only_attribute, 42}}, + graphdb_class:validate_instance_only_avps( + [#{attribute => 42, value => "red", instance_only => true}])). From 34c9183d99ae0acf277820cd47a250ffe153a591 Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 20:04:43 -0400 Subject: [PATCH 4/9] Slice C: add_qualifying_characteristic/3 stamps instance-only marker Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- apps/graphdb/src/graphdb_class.erl | 39 +++++++++++++++++++--- apps/graphdb/test/graphdb_class_SUITE.erl | 40 ++++++++++++++++++++++- 2 files changed, 73 insertions(+), 6 deletions(-) diff --git a/apps/graphdb/src/graphdb_class.erl b/apps/graphdb/src/graphdb_class.erl index e412d90..d13615e 100644 --- a/apps/graphdb/src/graphdb_class.erl +++ b/apps/graphdb/src/graphdb_class.erl @@ -112,6 +112,7 @@ create_class/3, add_superclass/2, add_qualifying_characteristic/2, + add_qualifying_characteristic/3, bind_qc_value/3, add_template/2, %% Lookups @@ -234,6 +235,10 @@ add_template(ClassNref, Name) -> add_qualifying_characteristic(ClassNref, AttrNref) -> gen_server:call(?MODULE, {add_qualifying_characteristic, ClassNref, AttrNref}). +add_qualifying_characteristic(ClassNref, AttrNref, Opts) when is_map(Opts) -> + gen_server:call(?MODULE, + {add_qualifying_characteristic, ClassNref, AttrNref, Opts}). + %%----------------------------------------------------------------------------- %% bind_qc_value(ClassNref, AttrNref, Value) -> ok | {error, term()} @@ -372,6 +377,10 @@ handle_call({add_qualifying_characteristic, ClassNref, AttrNref}, _From, State) -> {reply, do_add_qc(ClassNref, AttrNref), State}; +handle_call({add_qualifying_characteristic, ClassNref, AttrNref, Opts}, _From, + State) -> + {reply, do_add_qc(ClassNref, AttrNref, Opts), State}; + handle_call({bind_qc_value, ClassNref, AttrNref, Value}, _From, State) -> {reply, do_bind_qc_value(ClassNref, AttrNref, Value), State}; @@ -969,14 +978,21 @@ do_validate_parent(Nref) -> %%----------------------------------------------------------------------------- %% do_add_qc(ClassNref, AttrNref) -> ok | {error, term()} +%% do_add_qc(ClassNref, AttrNref, Opts) -> ok | {error, term()} +%% +%% Adds the qualifying-characteristic AVP to the class node. /2 is the +%% plain declared-unbound form (delegates to /3 with empty Opts). /3 +%% accepts Opts = #{instance_only => true} to stamp the instance-only +%% marker onto the canonical declared-unbound shape. %% -%% Adds the qualifying-characteristic AVP to the class node using the -%% unified shape: #{attribute => AttrNref, value => undefined}. %% Validates both ClassNref (must be class) and AttrNref (must be %% attribute). Idempotent: if any entry for AttrNref already exists -%% (regardless of value), leaves it alone and returns ok. +%% (regardless of value or flags), leaves it alone and returns ok. %%----------------------------------------------------------------------------- do_add_qc(ClassNref, AttrNref) -> + do_add_qc(ClassNref, AttrNref, #{}). + +do_add_qc(ClassNref, AttrNref, Opts) -> Txn = fun() -> case mnesia:read(nodes, ClassNref) of [#node{kind = class, attribute_value_pairs = AVPs} = Node] -> @@ -989,8 +1005,7 @@ do_add_qc(ClassNref, AttrNref) -> true -> already_exists; false -> - NewAVP = #{attribute => AttrNref, - value => undefined}, + NewAVP = new_qc_avp(AttrNref, Opts), Updated = Node#node{ attribute_value_pairs = AVPs ++ [NewAVP] }, @@ -1015,6 +1030,20 @@ do_add_qc(ClassNref, AttrNref) -> {error, Reason} -> {error, Reason} end. +%%----------------------------------------------------------------------------- +%% new_qc_avp(AttrNref, Opts) -> map() +%% +%% Builds the QC AVP for a fresh declaration. `instance_only => true` in +%% Opts stamps the marker onto the canonical declared-unbound shape; +%% otherwise the plain declared-unbound shape is returned. +%%----------------------------------------------------------------------------- +new_qc_avp(AttrNref, Opts) -> + Base = #{attribute => AttrNref, value => undefined}, + case maps:get(instance_only, Opts, false) of + true -> Base#{instance_only => true}; + false -> Base + end. + %%----------------------------------------------------------------------------- %% do_bind_qc_value(ClassNref, AttrNref, Value) -> ok | {error, term()} diff --git a/apps/graphdb/test/graphdb_class_SUITE.erl b/apps/graphdb/test/graphdb_class_SUITE.erl index d5cd288..475eca8 100644 --- a/apps/graphdb/test/graphdb_class_SUITE.erl +++ b/apps/graphdb/test/graphdb_class_SUITE.erl @@ -108,6 +108,8 @@ bind_qc_value_basic/1, bind_qc_value_undeclared_qc/1, bind_qc_value_updates_existing_binding/1, + add_qc_3_stamps_instance_only_marker/1, + add_qc_3_idempotent_does_not_upgrade/1, %% Lookups get_class_returns_node/1, get_class_not_found/1, @@ -201,7 +203,9 @@ groups() -> add_qc_rejects_non_attribute, bind_qc_value_basic, bind_qc_value_undeclared_qc, - bind_qc_value_updates_existing_binding + bind_qc_value_updates_existing_binding, + add_qc_3_stamps_instance_only_marker, + add_qc_3_idempotent_does_not_upgrade ]}, {lookups, [], [ get_class_returns_node, @@ -927,6 +931,40 @@ bind_qc_value_updates_existing_binding(_Config) -> ?assert(lists:member({AttrN, 4000}, QCs)), ?assertNot(lists:member({AttrN, 3500}, QCs)). +%%----------------------------------------------------------------------------- +%% add_qualifying_characteristic/3 with #{instance_only => true} stamps the +%% marker onto the class node's QC AVP. +%%----------------------------------------------------------------------------- +add_qc_3_stamps_instance_only_marker(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, Veh} = graphdb_class:create_class("Vehicle", ?NREF_CLASSES), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN, + #{instance_only => true}), + {ok, Node} = graphdb_class:get_class(Veh), + ?assert(lists:member( + #{attribute => AttrN, value => undefined, instance_only => true}, + Node#node.attribute_value_pairs)). + +%%----------------------------------------------------------------------------- +%% Idempotency: /3 with instance_only on an already-declared UNFLAGGED QC is +%% a no-op — it returns ok but does NOT upgrade the stored entry. +%%----------------------------------------------------------------------------- +add_qc_3_idempotent_does_not_upgrade(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, Veh} = graphdb_class:create_class("Vehicle", ?NREF_CLASSES), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN, + #{instance_only => true}), + {ok, Node} = graphdb_class:get_class(Veh), + %% The original unflagged entry is preserved; no marked variant appears. + ?assert(lists:member(#{attribute => AttrN, value => undefined}, + Node#node.attribute_value_pairs)), + ?assertNot(lists:member( + #{attribute => AttrN, value => undefined, instance_only => true}, + Node#node.attribute_value_pairs)). + %%============================================================================= %% Lookup Tests From 2747bb6a67059723966ce45aceeb516396368b32 Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 20:12:27 -0400 Subject: [PATCH 5/9] Slice C: create_class/3 rejects bound instance-only QC Wire validate_instance_only_avps/1 as the outermost guard in do_create_class/4. A bound instance_only => true AVP now returns {error, {instance_only_attribute, AttrNref}} before any nref allocation or Mnesia writes occur. Two new CT tests added to graphdb_class_SUITE (creation group): - create_class_3_rejects_instance_only_with_value - create_class_3_accepts_instance_only_unbound 72 tests, all passing. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- apps/graphdb/src/graphdb_class.erl | 85 ++++++++++++----------- apps/graphdb/test/graphdb_class_SUITE.erl | 26 +++++++ 2 files changed, 71 insertions(+), 40 deletions(-) diff --git a/apps/graphdb/src/graphdb_class.erl b/apps/graphdb/src/graphdb_class.erl index d13615e..5938a6a 100644 --- a/apps/graphdb/src/graphdb_class.erl +++ b/apps/graphdb/src/graphdb_class.erl @@ -480,47 +480,52 @@ is_valid_parent_kind(_) -> false. %% nref/id allocation stays outside the transaction. %%----------------------------------------------------------------------------- do_create_class(Name, ParentClassNref, AVPs, InstAttr) -> - case do_validate_parent(ParentClassNref) of - ok -> - ClassNref = graphdb_nref:get_next(), - {TaxId1, TaxId2} = rel_id_server:get_id_pair(), - ClassNameAVP = #{attribute => ?NAME_ATTR_CLASS, value => Name}, - ClassNode = #node{ - nref = ClassNref, - kind = class, - parents = [ParentClassNref], - attribute_value_pairs = [ClassNameAVP | AVPs] - }, - TaxP2C = #relationship{ - id = TaxId1, kind = taxonomy, - source_nref = ParentClassNref, - characterization = ?ARC_CLS_CHILD, - target_nref = ClassNref, - reciprocal = ?ARC_CLS_PARENT, - avps = [] - }, - TaxC2P = #relationship{ - id = TaxId2, kind = taxonomy, - source_nref = ClassNref, - characterization = ?ARC_CLS_PARENT, - target_nref = ParentClassNref, - reciprocal = ?ARC_CLS_CHILD, - avps = [] - }, - TemplateRows = template_rows(ClassNref, AVPs, InstAttr), - Txn = fun() -> - ok = mnesia:write(nodes, ClassNode, write), - ok = mnesia:write(relationships, TaxP2C, write), - ok = mnesia:write(relationships, TaxC2P, write), - [ ok = mnesia:write(T, R, write) || {T, R} <- TemplateRows ] - end, - case graphdb_mgr:transaction(Txn) of - %% Txn value is [] (abstract) or [ok,ok,ok] (template rows) - {ok, _Writes} -> {ok, ClassNref}; - {error, _} = Err -> Err - end; + case validate_instance_only_avps(AVPs) of {error, _} = Err -> - Err + Err; + ok -> + case do_validate_parent(ParentClassNref) of + ok -> + ClassNref = graphdb_nref:get_next(), + {TaxId1, TaxId2} = rel_id_server:get_id_pair(), + ClassNameAVP = #{attribute => ?NAME_ATTR_CLASS, value => Name}, + ClassNode = #node{ + nref = ClassNref, + kind = class, + parents = [ParentClassNref], + attribute_value_pairs = [ClassNameAVP | AVPs] + }, + TaxP2C = #relationship{ + id = TaxId1, kind = taxonomy, + source_nref = ParentClassNref, + characterization = ?ARC_CLS_CHILD, + target_nref = ClassNref, + reciprocal = ?ARC_CLS_PARENT, + avps = [] + }, + TaxC2P = #relationship{ + id = TaxId2, kind = taxonomy, + source_nref = ClassNref, + characterization = ?ARC_CLS_PARENT, + target_nref = ParentClassNref, + reciprocal = ?ARC_CLS_CHILD, + avps = [] + }, + TemplateRows = template_rows(ClassNref, AVPs, InstAttr), + Txn = fun() -> + ok = mnesia:write(nodes, ClassNode, write), + ok = mnesia:write(relationships, TaxP2C, write), + ok = mnesia:write(relationships, TaxC2P, write), + [ ok = mnesia:write(T, R, write) || {T, R} <- TemplateRows ] + end, + case graphdb_mgr:transaction(Txn) of + %% Txn value is [] (abstract) or [ok,ok,ok] (template rows) + {ok, _Writes} -> {ok, ClassNref}; + {error, _} = Err -> Err + end; + {error, _} = Err -> + Err + end end. diff --git a/apps/graphdb/test/graphdb_class_SUITE.erl b/apps/graphdb/test/graphdb_class_SUITE.erl index 475eca8..eea7410 100644 --- a/apps/graphdb/test/graphdb_class_SUITE.erl +++ b/apps/graphdb/test/graphdb_class_SUITE.erl @@ -66,6 +66,8 @@ create_class_auto_creates_default_template/1, create_class_3_default_avps_empty/1, create_class_3_writes_avps/1, + create_class_3_rejects_instance_only_with_value/1, + create_class_3_accepts_instance_only_unbound/1, create_abstract_class_skips_default_template/1, instantiable_class_keeps_default_template/1, is_instantiable_true_false/1, @@ -160,6 +162,8 @@ groups() -> create_class_auto_creates_default_template, create_class_3_default_avps_empty, create_class_3_writes_avps, + create_class_3_rejects_instance_only_with_value, + create_class_3_accepts_instance_only_unbound, create_abstract_class_skips_default_template, instantiable_class_keeps_default_template, is_instantiable_true_false @@ -447,6 +451,28 @@ create_class_3_writes_avps(_Config) -> ?assert(lists:member(Extra, Node#node.attribute_value_pairs)). +%%----------------------------------------------------------------------------- +%% create_class/3 rejects an initial AVP that is instance_only AND bound. +%%----------------------------------------------------------------------------- +create_class_3_rejects_instance_only_with_value(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + Bad = #{attribute => AttrN, value => "SN-1", instance_only => true}, + ?assertEqual({error, {instance_only_attribute, AttrN}}, + graphdb_class:create_class("Bad", 3, [Bad])). + +%%----------------------------------------------------------------------------- +%% create_class/3 accepts an instance_only QC declared unbound. +%%----------------------------------------------------------------------------- +create_class_3_accepts_instance_only_unbound(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + Good = #{attribute => AttrN, value => undefined, instance_only => true}, + {ok, ClassNref} = graphdb_class:create_class("Good", 3, [Good]), + {ok, Node} = graphdb_class:get_class(ClassNref), + ?assert(lists:member(Good, Node#node.attribute_value_pairs)). + + %%----------------------------------------------------------------------------- %% A class created with instantiable=>false has NO default template. %%----------------------------------------------------------------------------- From 97e26aace97304e4117cb2d31e7c9d7f013b38e5 Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 20:19:18 -0400 Subject: [PATCH 6/9] Slice C: bind_qc_value/3 rejects instance-only QC Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- apps/graphdb/src/graphdb_class.erl | 27 +++++++++++++++++++---- apps/graphdb/test/graphdb_class_SUITE.erl | 21 +++++++++++++++++- 2 files changed, 43 insertions(+), 5 deletions(-) diff --git a/apps/graphdb/src/graphdb_class.erl b/apps/graphdb/src/graphdb_class.erl index 5938a6a..df21a9f 100644 --- a/apps/graphdb/src/graphdb_class.erl +++ b/apps/graphdb/src/graphdb_class.erl @@ -1070,10 +1070,17 @@ do_bind_qc_value(ClassNref, AttrNref, Value) -> false -> mnesia:abort(qc_not_declared); true -> - NewAVPs = update_qc_value(AVPs, AttrNref, Value), - mnesia:write(nodes, - N#node{attribute_value_pairs = NewAVPs}, - write) + case is_qc_instance_only(AVPs, AttrNref) of + true -> + mnesia:abort( + {instance_only_attribute, AttrNref}); + false -> + NewAVPs = update_qc_value(AVPs, AttrNref, + Value), + mnesia:write(nodes, + N#node{attribute_value_pairs = NewAVPs}, + write) + end end; [_] -> mnesia:abort(not_a_class); [] -> mnesia:abort(not_found) @@ -1097,6 +1104,18 @@ update_qc_value(AVPs, AttrNref, Value) -> _ -> A end || A <- AVPs]. +%%----------------------------------------------------------------------------- +%% is_qc_instance_only(AVPs, AttrNref) -> boolean() +%% +%% True iff the QC entry for AttrNref in AVPs carries the instance_only +%% marker. Caller has already verified AttrNref is present. +%%----------------------------------------------------------------------------- +is_qc_instance_only(AVPs, AttrNref) -> + lists:any(fun(#{attribute := A} = E) when A =:= AttrNref -> + is_instance_only(E); + (_) -> false + end, AVPs). + %%----------------------------------------------------------------------------- %% do_get_class(Nref) -> diff --git a/apps/graphdb/test/graphdb_class_SUITE.erl b/apps/graphdb/test/graphdb_class_SUITE.erl index eea7410..8ecea29 100644 --- a/apps/graphdb/test/graphdb_class_SUITE.erl +++ b/apps/graphdb/test/graphdb_class_SUITE.erl @@ -112,6 +112,7 @@ bind_qc_value_updates_existing_binding/1, add_qc_3_stamps_instance_only_marker/1, add_qc_3_idempotent_does_not_upgrade/1, + bind_qc_value_rejects_instance_only/1, %% Lookups get_class_returns_node/1, get_class_not_found/1, @@ -209,7 +210,8 @@ groups() -> bind_qc_value_undeclared_qc, bind_qc_value_updates_existing_binding, add_qc_3_stamps_instance_only_marker, - add_qc_3_idempotent_does_not_upgrade + add_qc_3_idempotent_does_not_upgrade, + bind_qc_value_rejects_instance_only ]}, {lookups, [], [ get_class_returns_node, @@ -992,6 +994,23 @@ add_qc_3_idempotent_does_not_upgrade(_Config) -> Node#node.attribute_value_pairs)). +%%----------------------------------------------------------------------------- +%% bind_qc_value/3 refuses to bind a value on an instance-only QC, and the +%% transaction abort leaves the QC unbound. +%%----------------------------------------------------------------------------- +bind_qc_value_rejects_instance_only(_Config) -> + {ok, _} = graphdb_class:start_link(), + {ok, Veh} = graphdb_class:create_class("Vehicle", ?NREF_CLASSES), + {ok, AttrN} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(Veh, AttrN, + #{instance_only => true}), + ?assertEqual({error, {instance_only_attribute, AttrN}}, + graphdb_class:bind_qc_value(Veh, AttrN, "SN-1")), + %% Abort rolled back the write: the QC is still declared, still unbound. + {ok, QCs} = graphdb_class:inherited_qcs(Veh), + ?assert(lists:member({AttrN, undefined}, QCs)). + + %%============================================================================= %% Lookup Tests %%============================================================================= From 294835c2f3fda9dd5adec3ffcf803b72b4eeffda Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 21:57:58 -0400 Subject: [PATCH 7/9] Slice C: update_node_avps/mutate reject instance-only value bind Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- apps/graphdb/src/graphdb_mgr.erl | 30 +++++++++++- apps/graphdb/test/graphdb_mgr_SUITE.erl | 64 +++++++++++++++++++++++-- apps/graphdb/test/graphdb_mgr_tests.erl | 28 +++++++++++ 3 files changed, 117 insertions(+), 5 deletions(-) diff --git a/apps/graphdb/src/graphdb_mgr.erl b/apps/graphdb/src/graphdb_mgr.erl index de01af8..904c9b3 100644 --- a/apps/graphdb/src/graphdb_mgr.erl +++ b/apps/graphdb/src/graphdb_mgr.erl @@ -151,7 +151,8 @@ validate_direction/1, check_category_guard/1, validate_avp_updates/1, - apply_avp_updates/2 + apply_avp_updates/2, + check_instance_only/2 ]). -endif. @@ -755,6 +756,7 @@ update_node_avps_in_txn(Nref, AVPs, RetAttr) -> mnesia:abort(not_found); [Node] -> ok = guard_retired_marker(AVPs, RetAttr), + ok = guard_instance_only(Node#node.attribute_value_pairs, AVPs), ok = guard_attribute_existence(AVPs), New = apply_avp_updates(Node#node.attribute_value_pairs, AVPs), mnesia:write(nodes, Node#node{attribute_value_pairs = New}, write) @@ -780,6 +782,32 @@ guard_attribute_existence(AVPs) -> end, Upserts), ok. +%%----------------------------------------------------------------------------- +%% check_instance_only(StoredAVPs, Updates) -> +%% ok | {error, {instance_only_attribute, integer()}} +%% +%% Pure. A value-bearing update (one carrying a `value` key, including +%% value => undefined) targeting a stored entry marked +%% `instance_only => true` is rejected. Deletes (no `value` key) and +%% updates to non-marked attributes pass. Returns the first offender. +%%----------------------------------------------------------------------------- +check_instance_only(StoredAVPs, Updates) -> + Marked = [A || #{attribute := A} = E <- StoredAVPs, + maps:get(instance_only, E, false) =:= true], + ValueBearing = [A || #{attribute := A} = M <- Updates, + maps:is_key(value, M)], + case [A || A <- ValueBearing, lists:member(A, Marked)] of + [] -> ok; + [A | _] -> {error, {instance_only_attribute, A}} + end. + +%% In-txn guard: aborts on the first instance-only violation. +guard_instance_only(StoredAVPs, Updates) -> + case check_instance_only(StoredAVPs, Updates) of + ok -> ok; + {error, {instance_only_attribute, _} = R} -> mnesia:abort(R) + end. + %%----------------------------------------------------------------------------- %% set_marker(AVPs, RetAttr, Bool) -> AVPs' %% Removes any existing `retired` AVP; if Bool is true, appends a fresh diff --git a/apps/graphdb/test/graphdb_mgr_SUITE.erl b/apps/graphdb/test/graphdb_mgr_SUITE.erl index 37706ad..55e4b54 100644 --- a/apps/graphdb/test/graphdb_mgr_SUITE.erl +++ b/apps/graphdb/test/graphdb_mgr_SUITE.erl @@ -128,7 +128,11 @@ mutate_mixed_add_rel_and_update_avps/1, mutate_update_avps_rollback/1, mutate_update_avps_malformed/1, - mutate_update_avps_not_found/1 + mutate_update_avps_not_found/1, + %% instance-only QC enforcement + update_node_avps_rejects_instance_only/1, + update_node_avps_delete_instance_only_ok/1, + mutate_rejects_instance_only/1 ]). @@ -213,7 +217,8 @@ groups() -> mutate_mixed_add_rel_and_update_avps, mutate_update_avps_rollback, mutate_update_avps_malformed, - mutate_update_avps_not_found + mutate_update_avps_not_found, + mutate_rejects_instance_only ]}, {update_avps, [], [ update_node_avps_upsert_roundtrip, @@ -225,7 +230,9 @@ groups() -> update_node_avps_retired_marker_rejected, update_node_avps_not_found, update_node_avps_permanent_tier, - update_node_avps_atomic_rollback + update_node_avps_atomic_rollback, + update_node_avps_rejects_instance_only, + update_node_avps_delete_instance_only_ok ]} ]. @@ -309,7 +316,10 @@ init_per_testcase(TC, Config) when TC =:= mutate_mixed_add_rel_and_update_avps; TC =:= mutate_update_avps_rollback; TC =:= mutate_update_avps_malformed; - TC =:= mutate_update_avps_not_found -> + TC =:= mutate_update_avps_not_found; + TC =:= update_node_avps_rejects_instance_only; + TC =:= update_node_avps_delete_instance_only_ok; + TC =:= mutate_rejects_instance_only -> Config1 = setup_isolated_env(Config), BootstrapFile = proplists:get_value(bootstrap_file, Config), application:set_env(seerstone_graph_db, bootstrap_file, BootstrapFile), @@ -1412,3 +1422,49 @@ update_node_avps_atomic_rollback(_Config) -> [#{attribute => Attr, value => "red"}, #{attribute => BadAttr, value => "boom"}])), ?assertEqual(not_found, ua_value(Inst, Attr)). + +%%----------------------------------------------------------------------------- +%% update_node_avps/2 rejects a value-bearing update to a class node's +%% instance-only QC, and rolls the write back. +%%----------------------------------------------------------------------------- +update_node_avps_rejects_instance_only(_Config) -> + {ok, ClassNref} = graphdb_class:create_class("IOClass", 3), + {ok, Attr} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(ClassNref, Attr, + #{instance_only => true}), + ?assertEqual({error, {instance_only_attribute, Attr}}, + graphdb_mgr:update_node_avps(ClassNref, + [#{attribute => Attr, value => "SN-1"}])), + %% Rollback: the QC stays declared-unbound, marker intact. + [#node{attribute_value_pairs = AVPs}] = mnesia:dirty_read(nodes, ClassNref), + ?assert(lists:member( + #{attribute => Attr, value => undefined, instance_only => true}, AVPs)). + +%%----------------------------------------------------------------------------- +%% update_node_avps/2 permits DELETING an instance-only QC (no `value` key). +%%----------------------------------------------------------------------------- +update_node_avps_delete_instance_only_ok(_Config) -> + {ok, ClassNref} = graphdb_class:create_class("IODelClass", 3), + {ok, Attr} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(ClassNref, Attr, + #{instance_only => true}), + ?assertEqual(ok, + graphdb_mgr:update_node_avps(ClassNref, [#{attribute => Attr}])), + [#node{attribute_value_pairs = AVPs}] = mnesia:dirty_read(nodes, ClassNref), + ?assertNot(lists:any(fun(#{attribute := A}) -> A =:= Attr; + (_) -> false end, AVPs)). + +%%----------------------------------------------------------------------------- +%% mutate/1 inherits the instance-only guard via update_node_avps_in_txn. +%%----------------------------------------------------------------------------- +mutate_rejects_instance_only(_Config) -> + {ok, ClassNref} = graphdb_class:create_class("IOMutClass", 3), + {ok, Attr} = graphdb_attr:create_literal_attribute("serial", string), + ok = graphdb_class:add_qualifying_characteristic(ClassNref, Attr, + #{instance_only => true}), + ?assertEqual({error, {instance_only_attribute, Attr}}, + graphdb_mgr:mutate([{update_node_avps, ClassNref, + [#{attribute => Attr, value => "SN-1"}]}])), + [#node{attribute_value_pairs = AVPs}] = mnesia:dirty_read(nodes, ClassNref), + ?assert(lists:member( + #{attribute => Attr, value => undefined, instance_only => true}, AVPs)). diff --git a/apps/graphdb/test/graphdb_mgr_tests.erl b/apps/graphdb/test/graphdb_mgr_tests.erl index c103068..ccbd7ad 100644 --- a/apps/graphdb/test/graphdb_mgr_tests.erl +++ b/apps/graphdb/test/graphdb_mgr_tests.erl @@ -165,3 +165,31 @@ apply_avp_updates_empty_updates_is_identity_test() -> update_node_avps_malformed_short_circuits_test() -> ?assertEqual({error, {invalid_avp, "bad"}}, graphdb_mgr:update_node_avps(123, ["bad"])). + + +%%============================================================================= +%% check_instance_only/2 tests (pure) +%%============================================================================= + +check_instance_only_rejects_value_bearing_test() -> + Stored = [#{attribute => 42, value => undefined, instance_only => true}], + Updates = [#{attribute => 42, value => "SN-1"}], + ?assertEqual({error, {instance_only_attribute, 42}}, + graphdb_mgr:check_instance_only(Stored, Updates)). + +check_instance_only_rejects_undefined_value_test() -> + %% value => undefined still carries a `value` key -> still a bind attempt. + Stored = [#{attribute => 42, value => undefined, instance_only => true}], + Updates = [#{attribute => 42, value => undefined}], + ?assertEqual({error, {instance_only_attribute, 42}}, + graphdb_mgr:check_instance_only(Stored, Updates)). + +check_instance_only_allows_delete_test() -> + Stored = [#{attribute => 42, value => undefined, instance_only => true}], + Updates = [#{attribute => 42}], + ?assertEqual(ok, graphdb_mgr:check_instance_only(Stored, Updates)). + +check_instance_only_allows_non_marked_test() -> + Stored = [#{attribute => 42, value => undefined}], + Updates = [#{attribute => 42, value => "red"}], + ?assertEqual(ok, graphdb_mgr:check_instance_only(Stored, Updates)). From d3b762a05282eeaeb8fd27cc4d762c30340aa2ce Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sat, 27 Jun 2026 22:06:47 -0400 Subject: [PATCH 8/9] =?UTF-8?q?Slice=20C:=20docs=20=E2=80=94=20mark=20enfo?= =?UTF-8?q?rcement=20implemented,=20record=20deferred=20follow-ons?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- TASKS.md | 53 ++++++++++++++++++++++++------------------ apps/graphdb/CLAUDE.md | 2 +- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/TASKS.md b/TASKS.md index 6b0a639..64d006c 100644 --- a/TASKS.md +++ b/TASKS.md @@ -281,29 +281,36 @@ from the rest of the permanent tier (a DB read in phase 1, which today does no DB access) — not worth it unless a caller needs to branch on the specific reason. Revisit if that need arises. -### Template attribute list and instance-only enforcement (slice C, depends on slice B) - -A template currently carries only a name and its compositional arc into -the owning class — there is no per-template list of which attributes the -template scopes. Without it, the class write-side cannot distinguish two -categories of attribute a class declares: - -- **Class-bindable** — the class may supply a value (or a useful - default); instances inherit it and may override. *Example: - `num_wheels = 4` on a Car class.* -- **Instance-only** — the class declares the attribute as relevant, but - binding a value at the class level is a category error; the value is - meaningful only per instance. *Example: `serial_number`, - `owner_name`.* Binding a class-level value for such an attribute should - be rejected. - -The distinction is per-class, per-template — the same attribute may be -class-bindable in one class's template and instance-only in another's. -Build the template attribute list, then enforce instance-only rejection -in `create_class` and `update_node_avps`. The unified qualifying- -characteristic AVP shape (a declared-but-unbound attribute carries -`value => undefined`) already accommodates an instance-only attribute -naturally — it stays `undefined` at every class level. +### Instance-only qualifying characteristics (slice C) — IMPLEMENTED + +A class QC may be marked `instance_only => true`: the attribute is relevant +to instances, but binding a value at the class level is a category error. +Set via `graphdb_class:add_qualifying_characteristic/3` (`#{instance_only => +true}`) or a `create_class/3` initial AVP. Enforced at three class-level +value-binding gates — `bind_qc_value/3`, `create_class/3`, and +`update_node_avps/2` (the last covers `mutate/1`, both composing +`update_node_avps_in_txn/3`) — each returning `{error, +{instance_only_attribute, AttrNref}}`. Enforcement is local to the class +node written. Design `docs/designs/slice-c-instance-only-qc-design.md`. + +**Deferred follow-ons (from slice C):** + +- **Template attribute list** — per-template subset/relevance scoping of + class attributes (`TheKnowledgeNetwork.md` §7). A template currently + carries only a name and its compositional arc into the owning class; there + is no per-template list of which attributes it scopes. This is the + per-class, per-template axis: the same attribute may be class-bindable in + one class's template and instance-only in another's. +- **Template-bound (variant) values** — templates carrying override values + stamped into instances at instantiation (e.g. a later custom-colour phone + variant whose colour is fixed in a template, not on the base class). +- **Inherited instance-only enforcement (C2)** — close the subclass-redeclare + bypass: a subclass can re-declare a parent's instance-only QC *without* the + flag via `add_qualifying_characteristic/2`, then bind a value. Local gates + do not consult the inherited QC set because `collect_qc_avps/1` flattens + each QC to `{AttrNref, Value}`, dropping the marker. Closing it means + carrying the flag through `collect_qc_avps/1` / `inherited_qcs/1` and + having all three gates consult the effective (local + ancestor) QC set. ### Relationship mutation (slice E) diff --git a/apps/graphdb/CLAUDE.md b/apps/graphdb/CLAUDE.md index 63c58eb..da16796 100644 --- a/apps/graphdb/CLAUDE.md +++ b/apps/graphdb/CLAUDE.md @@ -244,7 +244,7 @@ the ontology `nodes` Mnesia table with `kind = attribute`. Manages the "is a" hierarchy of class nodes in the ontology. - `create_class/2,3` (name, parent_class_nref [, avps]) — the `/3` form prepends an initial AVP list to the class node; a class created with the `instantiable => false` marker AVP is **abstract** and is born without a default template (L9) -- `add_qualifying_characteristic/2` (class_nref, attribute_nref) +- `add_qualifying_characteristic/2,3` (class_nref, attribute_nref [, opts]) — the `/3` form takes an options map; `#{instance_only => true}` marks the QC instance-only (binding a class-level value for it is rejected at `bind_qc_value/3`, `create_class/3`, and `update_node_avps/2`) - `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`, From 162c4bdb9637353cc005d7417ae81df06c4222ab Mon Sep 17 00:00:00 2001 From: "David W. Thomas" Date: Sun, 28 Jun 2026 07:21:54 -0400 Subject: [PATCH 9/9] Slice C: record marker-mutability investigation as deferred follow-on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PR text implied that slice B's AVP well-formedness check makes the instance_only marker permanently unsettable through update_node_avps/2 and mutate/1. That restriction is a side effect of the slice-B grammar, not a decided long-term contract — record it as an open investigation in TASKS.md rather than an implicit design stance. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01EWukKCbrN8GybaScJGU2kF --- TASKS.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/TASKS.md b/TASKS.md index 64d006c..f1da1b5 100644 --- a/TASKS.md +++ b/TASKS.md @@ -311,6 +311,18 @@ node written. Design `docs/designs/slice-c-instance-only-qc-design.md`. each QC to `{AttrNref, Value}`, dropping the marker. Closing it means carrying the flag through `collect_qc_avps/1` / `inherited_qcs/1` and having all three gates consult the effective (local + ancestor) QC set. +- **Marker mutability via the general update path** — today `instance_only` + is settable only at QC declaration (`add_qualifying_characteristic/3`) or + `create_class/3`; it can be neither set nor cleared through + `update_node_avps/2` / `mutate/1`. That restriction is a *side effect* of + slice B's AVP well-formedness check (which rejects any update map whose + key-set is not exactly `[attribute]` or `[attribute, value]`), **not** a + deliberate long-term contract decision. Investigate whether toggling a + QC's instance-only status — and QC-shape edits generally — should be a + first-class mutation: a dedicated mutation kind, or a widened update + grammar that admits marker keys, versus remaining declaration-time only. + Decide and document the intended contract before any caller comes to + depend on the current behaviour. ### Relationship mutation (slice E)