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
77 changes: 65 additions & 12 deletions packages/core/src/mdl_core/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ class CommandResult:
fingerprint: str
diagnostics: list[dict] = field(default_factory=list)
created_id: str | None = None
# Bulk handlers (e.g. add_attributes) mint several ids in one validated transaction.
# created_id stays the FIRST of these for single-create back-compat; created_ids holds
# them all, in creation order, so a caller can reference each new object.
created_ids: list[str] = field(default_factory=list)
error: str | None = None


Expand All @@ -120,9 +124,19 @@ def apply_command(
raise CommandError(f"unknown command {op!r}")

repo = ModelRepo.load(model_dir)
created_id = handler(repo, payload)
result = handler(repo, payload)
repo.save()

# A handler returns either a single new id (str), several (list[str], a bulk op), or
# None. Normalise: created_id is the first id (single-create back-compat), created_ids
# is the full list.
if isinstance(result, list):
created_ids = result
created_id = created_ids[0] if created_ids else None
else:
created_id = result
created_ids = [created_id] if created_id else []

diags = validate(ModelRepo.load(model_dir).model)
return CommandResult(
ok=True,
Expand All @@ -132,6 +146,7 @@ def apply_command(
for d in diags.items
],
created_id=created_id,
created_ids=created_ids,
)


Expand Down Expand Up @@ -218,9 +233,20 @@ def _create_entity(repo: ModelRepo, p: dict) -> str:
"realises": ce_id,
"attributes": [],
}
# Optional inline attributes: create the shell AND its columns in one transaction, so a
# wide entity is a single create call, not create + N add_attribute round trips. The
# same _append_attribute guard rejects a duplicate name within the batch.
attr_ids: list[str] = []
specs = p.get("attributes")
if specs is not None:
if not isinstance(specs, list):
raise CommandError("attributes must be a list")
seen: set[str] = set()
attr_ids = [_append_attribute(le, seen, spec) for spec in specs]
repo.add_file(f"conceptual/entities/{name}.yaml", ce, ce_id)
repo.add_file(f"logical/entities/{name}.yaml", le, le_id)
return le_id
# Entity id first (back-compat: created_id stays the entity), then each attribute id.
return [le_id, *attr_ids] if attr_ids else le_id


def _rename_entity(repo: ModelRepo, p: dict) -> None:
Expand Down Expand Up @@ -388,24 +414,50 @@ def _entity_node(repo: ModelRepo, entity_id: str):
return le, _node_for(repo, le.id)[1]


def _add_attribute(repo: ModelRepo, p: dict) -> str:
_require(p, "entity_id", "name")
le, node = _entity_node(repo, p["entity_id"])
name = _slug(p["name"])
if any(a.name == name for a in le.attributes):
raise CommandError(f"attribute {name!r} already exists on {le.name!r}")
attr_id = _id_from(p)
def _append_attribute(node: dict, existing: set[str], spec: dict) -> str:
"""Append one attribute to a logical-entity YAML `node`, guarding against a duplicate
name (checked against `existing`, which is updated in place so a bulk add also rejects
duplicates WITHIN the same batch). Returns the attribute's id. Mints an id unless the
spec supplies a valid ULID (`id`), so a preview and its replay share identities."""
if not spec.get("name"):
raise CommandError("attribute: missing field(s): name")
name = _slug(spec["name"])
if name in existing:
raise CommandError(f"attribute {name!r} already exists on this entity")
attr_id = _id_from(spec)
attr: dict = {
"id": attr_id,
"name": name,
"domain": p.get("domain", "string"),
"role": p.get("role", "attribute"),
"nullable": bool(p.get("nullable", True)),
"domain": spec.get("domain", "string"),
"role": spec.get("role", "attribute"),
"nullable": bool(spec.get("nullable", True)),
}
node.setdefault("attributes", []).append(attr)
existing.add(name)
return attr_id


def _add_attribute(repo: ModelRepo, p: dict) -> str:
_require(p, "entity_id", "name")
le, node = _entity_node(repo, p["entity_id"])
return _append_attribute(node, {a.name for a in le.attributes}, p)


def _add_attributes(repo: ModelRepo, p: dict) -> list[str]:
"""Add MANY attributes to one entity in a single validated transaction, so building a
54-column entity is one load->append-all->one save->one validate instead of 54 round
trips. `attributes` is an ordered list of {name, domain?, role?, nullable?, id?}. The
whole batch is all-or-nothing: a duplicate name (against existing columns or earlier in
the same batch) raises before anything is saved. Returns the new ids in list order."""
_require(p, "entity_id")
specs = p.get("attributes")
if not isinstance(specs, list) or not specs:
raise CommandError("attributes must be a non-empty list")
le, node = _entity_node(repo, p["entity_id"])
existing = {a.name for a in le.attributes}
return [_append_attribute(node, existing, spec) for spec in specs]


def _update_attribute(repo: ModelRepo, p: dict) -> None:
_require(p, "entity_id", "attribute_id")
_, node = _entity_node(repo, p["entity_id"])
Expand Down Expand Up @@ -1186,6 +1238,7 @@ def _set_subject_area_members(repo: ModelRepo, p: dict) -> None:
"set_subject_area_members": _set_subject_area_members,
"create_term": _create_term,
"add_attribute": _add_attribute,
"add_attributes": _add_attributes,
"update_attribute": _update_attribute,
"delete_attribute": _delete_attribute,
"create_relationship": _create_relationship,
Expand Down
103 changes: 103 additions & 0 deletions packages/core/tests/test_structural_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -404,3 +404,106 @@ def test_supplied_ulids_work_for_every_creator(tmp_path: Path):
)
assert kg in _load(tmp_path).key_groups
_clean(tmp_path)


# --- bulk attributes -----------------------------------------------------------


def test_create_entity_with_inline_attributes(tmp_path: Path):
"""create_entity can build the shell AND its columns in one validated transaction."""
write_model(tmp_path) # seed the project so there is a model to create into
r = apply_command(
tmp_path,
"create_entity",
{
"name": "adverse_event",
"definition": "An AE record.",
"attributes": [
{"name": "usubjid", "domain": "string", "nullable": False},
{"name": "aeseq", "domain": "integer"},
{"name": "aeterm"}, # domain defaults to string
],
},
)
assert r.ok and r.created_id
# created_id is the entity; created_ids is entity + each attribute, in order
assert r.created_ids[0] == r.created_id
assert len(r.created_ids) == 4
le = _load(tmp_path).logical_entities[r.created_id]
assert [a.name for a in le.attributes] == ["usubjid", "aeseq", "aeterm"]
assert le.attributes[0].nullable is False
assert le.attributes[1].domain == "integer"
assert le.attributes[2].domain == "string" # defaulted
# the minted attribute ids match what came back (entity first)
assert [a.id for a in le.attributes] == r.created_ids[1:]
_clean(tmp_path)


def test_add_attributes_bulk_to_existing_entity(tmp_path: Path):
ids = write_model(tmp_path)
before = len(_load(tmp_path).logical_entities[ids["le"]].attributes)
r = apply_command(
tmp_path,
"add_attributes",
{
"entity_id": ids["le"],
"attributes": [
{"name": "lei", "domain": "string"},
{"name": "incorporated_on", "domain": "date"},
{"name": "jurisdiction"},
],
},
)
assert r.ok and len(r.created_ids) == 3
le = _load(tmp_path).logical_entities[ids["le"]]
assert len(le.attributes) == before + 3
names = {a.name for a in le.attributes}
assert {"lei", "incorporated_on", "jurisdiction"} <= names
_clean(tmp_path)


def test_add_attributes_is_all_or_nothing_on_duplicate(tmp_path: Path):
"""A duplicate name (vs an existing column OR earlier in the same batch) rejects the
WHOLE batch — nothing is written."""
ids = write_model(tmp_path)
existing = _load(tmp_path).logical_entities[ids["le"]].attributes
existing_name = existing[0].name
before = len(existing)
# duplicate against an existing column
with pytest.raises(CommandError, match="already exists"):
apply_command(
tmp_path,
"add_attributes",
{"entity_id": ids["le"], "attributes": [{"name": "fresh_one"}, {"name": existing_name}]},
)
assert len(_load(tmp_path).logical_entities[ids["le"]].attributes) == before
# duplicate WITHIN the batch
with pytest.raises(CommandError, match="already exists"):
apply_command(
tmp_path,
"add_attributes",
{"entity_id": ids["le"], "attributes": [{"name": "dup_x"}, {"name": "dup_x"}]},
)
assert len(_load(tmp_path).logical_entities[ids["le"]].attributes) == before
_clean(tmp_path)


def test_add_attributes_rejects_empty_list(tmp_path: Path):
ids = write_model(tmp_path)
with pytest.raises(CommandError, match="non-empty list"):
apply_command(tmp_path, "add_attributes", {"entity_id": ids["le"], "attributes": []})


def test_add_attributes_honours_supplied_ids(tmp_path: Path):
"""A caller may pre-mint ids (preview/propose parity); add_attributes uses them."""
from mdl_core.ids import new_ulid

ids = write_model(tmp_path)
a, b = new_ulid(), new_ulid()
r = apply_command(
tmp_path,
"add_attributes",
{"entity_id": ids["le"], "attributes": [{"name": "c_a", "id": a}, {"name": "c_b", "id": b}]},
)
assert r.created_ids == [a, b]
_clean(tmp_path)
59 changes: 48 additions & 11 deletions packages/mcp/src/mdl_mcp/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,11 +221,21 @@ def create_entity(
definition: str | None = None,
subject_area: str | None = None,
layer: str | None = None,
attributes: list[dict[str, Any]] | None = None,
) -> str:
"""Create a conceptual + logical entity pair. Writes to the local model on
disk (validated before it lands). `subject_area` is a subject-area ULID;
`layer` is one of industry|core|domain|specialised. Returns the new entity's
ULID or an error object."""
`layer` is one of industry|core|domain|specialised.

`attributes` optionally creates the entity's columns IN THE SAME CALL — pass a list
of {name, domain?, role?, nullable?} (domain defaults to "string", nullable to true).
Prefer this over create_entity + many add_attribute calls: a wide entity (e.g. 54
columns) is then one validated transaction, not 55 round trips. The whole create is
all-or-nothing — a duplicate column name fails before anything is written.

Returns the new entity's ULID as `created_id`; when `attributes` is given, the
minted attribute ULIDs follow in `created_ids` (entity first, then each column in
order), so you can reference them in create_key_group without a re-read."""
return _write(
repo_dir,
"create_entity",
Expand All @@ -234,16 +244,23 @@ def create_entity(
"definition": definition,
"subject_area": subject_area,
"layer": layer,
"attributes": attributes,
},
)

@mcp.tool()
def update_entity(name: str, changes: dict[str, Any]) -> str:
"""Update an existing entity by name (or ULID). `changes` may contain:
`definition` (str), `rename` (str, the new name), `add_attribute`
({name, domain?, definition?, nullable?}), or `subject_area` (ULID). Each is
applied through the validated command engine; the whole update is rejected
(nothing written) if any step is invalid. Returns ok or an error object."""
`definition` (str), `rename` (str, the new name), `add_attribute`, or
`subject_area` (ULID). Each is applied through the validated command engine; the
whole update is rejected (nothing written) if any step is invalid.

`add_attribute` accepts EITHER one column {name, domain?, role?, nullable?} OR a
LIST of them to add many columns at once. A list is one validated transaction, so
widening an entity by many columns is a single call (not one per column); its new
ids come back as `attribute_ids` (a single column still returns `attribute_id`).

Returns ok (with any created attribute ids) or an error object."""
le = query._entity_by_name(_model(), name) # noqa: SLF001 - shared helper
if le is None:
return _json({"error": f"no entity named {name!r}"})
Expand Down Expand Up @@ -390,6 +407,7 @@ def _apply_updates(
applied: list[str] = []
result = None
attr_id: str | None = None
attr_ids: list[str] = []
try:
if "definition" in changes:
if conceptual_id is None:
Expand All @@ -416,11 +434,23 @@ def _apply_updates(
applied.append("subject_area")
if "add_attribute" in changes:
attr = changes["add_attribute"]
if not isinstance(attr, dict) or "name" not in attr:
return _json({"error": "add_attribute must be an object with at least a name"})
result = apply_command(repo_dir, "add_attribute", {"entity_id": entity_id, **attr})
attr_id = result.created_id
applied.append(f"add_attribute:{attr['name']}")
# Accept EITHER one attribute object or a LIST of them. A list adds all columns
# in one validated transaction (one load/save/validate), so widening an entity
# by many columns is a single call, not one per column.
if isinstance(attr, list):
if not attr or not all(isinstance(a, dict) and a.get("name") for a in attr):
return _json({"error": "add_attribute list items must each have a name"})
result = apply_command(
repo_dir, "add_attributes", {"entity_id": entity_id, "attributes": attr}
)
attr_ids = result.created_ids
applied += [f"add_attribute:{a['name']}" for a in attr]
elif isinstance(attr, dict) and attr.get("name"):
result = apply_command(repo_dir, "add_attribute", {"entity_id": entity_id, **attr})
attr_id = result.created_id
applied.append(f"add_attribute:{attr['name']}")
else:
return _json({"error": "add_attribute must be an object (or list) with a name"})
except StaleModelError as e:
return _json({"error": f"model changed on disk mid-update: {e}", "applied": applied})
except (CommandError, FileNotFoundError, ValueError) as e:
Expand All @@ -431,6 +461,9 @@ def _apply_updates(
if attr_id is not None:
# so the agent can reference the new column in create_key_group without a re-read
out["attribute_id"] = attr_id
if attr_ids:
# bulk add_attribute: every new column id, in list order
out["attribute_ids"] = attr_ids
if result is not None:
out["diagnostics"] = result.diagnostics
hints = _domain_hints(result.diagnostics)
Expand Down Expand Up @@ -463,6 +496,10 @@ def _write(repo_dir: Path, op: str, payload: dict) -> str:
"diagnostics": result.diagnostics,
"error": result.error,
}
# Bulk creates (create_entity with attributes, add_attributes) mint several ids; expose
# them all so the agent can reference each new column without a re-read.
if len(result.created_ids) > 1:
out["created_ids"] = result.created_ids
# Call out a foreign-key domain mismatch plainly — e.g. a create_relationship with
# create_from_fk whose minted column's domain disagrees with the key it references.
hints = _domain_hints(result.diagnostics)
Expand Down
Loading
Loading