From ba8a258e236957a47eec7328040ef4bb9f0bb216 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Fri, 9 Oct 2026 11:21:11 +0200 Subject: [PATCH 1/6] feat(openehr-specs): add bmm-authoring skill with a P_BMM JSON checker Add a knowledge skill for writing a component's BMM schema (the P_BMM JSON in computable/BMM/) from scratch or changing one: header conventions, includes, how packages set the class-table file names and {pkg}, every class, property, type and function kind, invariants and constants, documentation text, what the legacy class tables do not show, and the persistence-spec features bmm-publisher does not read (indexed containers, ancestor_defs, type_ref value sets), with the form to write instead. bmm-publisher renders most modelling mistakes without a message (a misspelt _type becomes a property of type Any, unknown keys are ignored, a class missing from packages gets no table), so the skill ships a read-only, standard-library checker (scripts/check_bmm.py) and a worked example that passes it and renders cleanly. CI runs the checker's unit tests. class-generation and authoring now route changes to the class model to bmm-authoring; the prompting guide, plugin README, AGENTS.md and testing docs list the new skill and script. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 8 +- CHANGELOG.md | 9 + docs/prompting-guide.md | 12 +- docs/testing.md | 4 +- plugins/openehr-specs/README.md | 1 + .../openehr-specs/skills/authoring/SKILL.md | 6 +- .../skills/bmm-authoring/SKILL.md | 137 +++ .../assets/openehr_demo_0.1.0.bmm.json | 297 +++++++ .../bmm-authoring/references/p-bmm-json.md | 309 +++++++ .../skills/bmm-authoring/scripts/check_bmm.py | 788 ++++++++++++++++++ .../skills/class-generation/SKILL.md | 13 +- scripts/test_check_bmm.py | 386 +++++++++ 12 files changed, 1955 insertions(+), 15 deletions(-) create mode 100644 plugins/openehr-specs/skills/bmm-authoring/SKILL.md create mode 100644 plugins/openehr-specs/skills/bmm-authoring/assets/openehr_demo_0.1.0.bmm.json create mode 100644 plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md create mode 100644 plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py create mode 100644 scripts/test_check_bmm.py diff --git a/AGENTS.md b/AGENTS.md index 48a5e24..963202f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,6 +15,7 @@ ai-plugins/ ├── docs/ # Contributor and user documentation ├── scripts/validate.py # Manifest, frontmatter and template-set validation (CI) ├── scripts/test_scaffold.py # Unit tests for the scaffold skill's script (CI) +├── scripts/test_check_bmm.py # Unit tests for the bmm-authoring skill's checker (CI) ├── CHANGELOG.md # Release notes per plugin └── plugins// # One directory per plugin ├── .claude-plugin/plugin.json # Claude Code plugin manifest @@ -44,7 +45,7 @@ Full details: [docs/skill-authoring.md](docs/skill-authoring.md) ## Development -Pure-content repository (JSON manifests + markdown skills) — no build step or package manager. The one script, the `scaffold` skill's `scripts/scaffold.py`, has unit tests: `python3 -m unittest discover -s scripts -p 'test_*.py'` (runs in CI). +Pure-content repository (JSON manifests + markdown skills) — no build step or package manager. Two skills bundle a script, each with unit tests: `scaffold` (`scripts/scaffold.py`) and `bmm-authoring` (`scripts/check_bmm.py`). Run them with `python3 -m unittest discover -s scripts -p 'test_*.py'` (runs in CI). - **Validation**: `python3 scripts/validate.py` (Claude + Cursor manifests, version sync, required skill/agent frontmatter fields, component paths, and the `scaffold` template set's integrity — runs in CI; it does not check description length, inventories, or the release tag) - **Local testing**: [docs/testing.md](docs/testing.md); **versioning/releases**: [docs/versioning.md](docs/versioning.md) @@ -52,12 +53,13 @@ Pure-content repository (JSON manifests + markdown skills) — no build step or ## Component Dependencies -The skills are pure content; the subagents and the `publish`, `regen-classes` and `scaffold` action skills rely on external tools and degrade -gracefully when one is absent (each says so in its prompt): +The skills are pure content, except that `bmm-authoring` runs a bundled checker; the subagents and the `publish`, `regen-classes` and `scaffold` action skills rely on external tools. All of them degrade +gracefully when a tool is absent (each says so in its prompt): - **`spec-reviewer`, `xref-auditor`** — need a `specifications-XX` checkout and (for attribute resolution) the sibling `specifications-AA_GLOBAL`. - **`xref-auditor`, `identifier-grounding`** — use **WebFetch** to read spec Markdown twins (`.html` → `.md`); `identifier-grounding` additionally prefers the **`openehr-assistant` MCP** (`type_specification_get`) when connected, falling back to the twin. - **`regen-classes`** — needs **Docker** (`ghcr.io/openehr/bmm-publisher`). +- **`bmm-authoring`** — needs **`python3`** (standard library only) to run its read-only `scripts/check_bmm.py`; without it, it says so and checks the change by hand against its reference. - **`scaffold`** — needs **`python3`** (standard library only) to run its bundled `scripts/scaffold.py`, and says so and stops when it is missing. It writes into the repository it is run in, so it plans first and never overwrites a hand-edited file without being told to. - **`publish`** — needs **Docker** and sibling `specifications-AA_GLOBAL` and component checkouts. It runs the published `ghcr.io/openehr/asciidoctor` image, which bundles the toolchain and `spec_publish.sh`; the boilerplate and references are still read from the AA_GLOBAL checkout. It checks that each HTML output was rebuilt and that the log has no missing includes, because the script reports success even when it built nothing or dropped the class tables. diff --git a/CHANGELOG.md b/CHANGELOG.md index a469e6e..8f29552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ Notable changes to the plugins in this repository. The format follows [Keep a Ch ## Unreleased +### Added + +- `bmm-authoring` skill: write a component's BMM schema (the P_BMM JSON in `computable/BMM/`) from scratch, or change one. It covers the header and its conventions, `includes`, how packages set the class-table file names, every class, property, type and function kind, invariants and constants, documentation text, what the legacy class tables do not show, and the persistence-spec features `bmm-publisher` does not read (indexed containers, `ancestor_defs`, `type_ref` value sets), with the form to write instead. It ships a worked example (`assets/openehr_demo_0.1.0.bmm.json`) and a read-only checker, `scripts/check_bmm.py` (standard-library Python 3.8+), that reports what `bmm-publisher` accepts without a message: a misspelt or missing `_type`, a misspelt key such as `is_mandantory`, a key that differs from its `name`, a class in no package, an upper limit without `"upper_unbounded": false`, a container without an element type, wrong generic parameter counts, a generic argument that does not conform to its parameter's `conforms_to_type`, unresolved type names (with `-d` for the included schemas), and more. CI runs its unit tests (`scripts/test_check_bmm.py`). + +### Changed + +- `class-generation` and `authoring` send changes to the class model itself to `bmm-authoring`. +- `docs/prompting-guide.md`: the class-change prompts name `bmm-authoring` and the checker, and a prompt for a new component's schema is added. + ## openehr-specs 0.5.0 (2026-10-08) ### Changed diff --git a/docs/prompting-guide.md b/docs/prompting-guide.md index 68c0279..2791c0e 100644 --- a/docs/prompting-guide.md +++ b/docs/prompting-guide.md @@ -12,7 +12,7 @@ In the prompts, `` is a component id such as `RM`, `BASE` or `AM`, and ` of type to class , with documentation, following this repo's AGENTS.md. +In the BMM for this repo, add an optional attribute of type to class , with documentation, and run the BMM checker. +``` + +For a new component, give the release, the BASE version it builds on, and the classes: + +```text +Write the BMM schema for the new component, release , building on BASE , with these classes: <...>. ``` ```text diff --git a/docs/testing.md b/docs/testing.md index 0d5d837..0c742de 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,6 +1,6 @@ # Testing and Validation -This is a pure-content repository: JSON manifests plus markdown skills (including user-only action skills) and subagents. There is no build step or package manager, and the only automated tests cover the `scaffold` skill's script. Testing means installing the marketplace locally and validating structure and skill quality. +This is a pure-content repository: JSON manifests plus markdown skills (including user-only action skills) and subagents. There is no build step or package manager, and the only automated tests cover the scripts bundled with the `scaffold` and `bmm-authoring` skills. Testing means installing the marketplace locally and validating structure and skill quality. ## Local Testing @@ -22,7 +22,7 @@ For Cursor, copy `plugins/openehr-specs` to `~/.cursor/plugins/local/openehr-spe ## Validation - **Manifest and frontmatter validation**: `python3 scripts/validate.py` (CI runs it on every PR) checks the Claude and Cursor JSON manifests, name and version sync, declared component paths, and the marketplace `owner.name`. For every skill and subagent it checks that `name` and `description` exist and that `name` matches the directory or file name. It also runs the `scaffold` template-set check (`scaffold.py check`: templates exist and render, revisions are contiguous, every revision has a migration file, and the template digest matches the latest revision). It does not check description length, `disable-model-invocation`, the README inventories, or the release tag. -- **Script tests**: `python3 -m unittest discover -s scripts -p 'test_*.py'` (CI runs it on every PR) covers the `scaffold` script: the template engine, each file strategy, variable inference, and the upgrade path across revisions, all in temporary directories. Run it after changing the script or the template set. +- **Script tests**: `python3 -m unittest discover -s scripts -p 'test_*.py'` (CI runs it on every PR) covers the `scaffold` script (the template engine, each file strategy, variable inference, and the upgrade path across revisions) and the `bmm-authoring` checker (each finding it reports, its exit codes, and a clean result on the skill's worked example), all in temporary directories. Run it after changing either script, the template set, or the worked example. - **Built-in manifest check**: `claude plugin validate plugins/openehr-specs` validates the plugin manifest; add `--strict` to treat warnings as errors. - **Structural validation**: after creating or modifying plugin components, run the `plugin-dev:plugin-validator` agent. It checks `plugin.json`, the marketplace entry, directory layout, and frontmatter. The agent comes from the `plugin-dev` plugin, which this repo enables in `.claude/settings.json`. - **Skill quality review**: run the `plugin-dev:skill-reviewer` agent (also from `plugin-dev`). It checks description triggering quality, progressive disclosure, and content structure. diff --git a/plugins/openehr-specs/README.md b/plugins/openehr-specs/README.md index 07013f9..3dead8f 100644 --- a/plugins/openehr-specs/README.md +++ b/plugins/openehr-specs/README.md @@ -28,6 +28,7 @@ Add the marketplace from this repository (or a fork), then install `openehr-spec | `content-patterns` | Prose patterns for spec chapters and sections | | `its-rest` | ITS-REST OpenAPI YAML and operation descriptions | | `class-generation` | Generate class tables and UML diagrams from BMM via `bmm-publisher` | +| `bmm-authoring` | Write or change a component's BMM schema (P_BMM JSON), and check it for the mistakes `bmm-publisher` accepts without a message (bundled `check_bmm.py`, needs `python3`) | ### User-only action skills diff --git a/plugins/openehr-specs/skills/authoring/SKILL.md b/plugins/openehr-specs/skills/authoring/SKILL.md index 50f76f2..ee4a870 100644 --- a/plugins/openehr-specs/skills/authoring/SKILL.md +++ b/plugins/openehr-specs/skills/authoring/SKILL.md @@ -6,7 +6,8 @@ description: > to scaffold a new spec, add or edit a chapter, set up `master.adoc`, add a cross-reference or figure, or build a local HTML preview. Not for amendment records (amendment-record), prose style (content-patterns), releases or manifest release entries (governance), ITS-REST OpenAPI - (its-rest), class tables (class-generation), or archetype/AQL work (openehr-assistant plugin). + (its-rest), the class model or its tables (bmm-authoring, class-generation), or archetype/AQL + work (openehr-assistant plugin). --- # openEHR Specification Document Authoring @@ -26,6 +27,7 @@ in `specifications-XX` repositories (RM, AM, BASE, LANG, PROC, SM, QUERY, CDS, C - **its-rest** — for OpenAPI YAML and Markdown in the ITS-REST repo - **governance** — release management, change requests, lifecycle governance - **class-generation** — regenerate the class-definition tables and UML diagrams in `docs/UML/` from BMM via `bmm-publisher` +- **bmm-authoring** — add or change classes, attributes, functions and invariants in the component's BMM schema For attribute naming patterns, read `references/cross-references.md`. To verify that every `{openehr_*}` attribute and deep-link anchor resolves, dispatch the `xref-auditor` subagent. @@ -248,7 +250,7 @@ that the log has no `ERROR` or `include file not found` lines. The build rewrite ### Conventions -- **Chapter files**: Edit `masterNN-*.adoc` files directly. Never edit files under `docs/UML/` — those are generated from the component's BMM schema by `bmm-publisher` (see the **class-generation** skill); change the BMM and regenerate. +- **Chapter files**: Edit `masterNN-*.adoc` files directly. Never edit files under `docs/UML/` — those are generated from the component's BMM schema by `bmm-publisher` (see the **class-generation** skill); change the BMM (see the **bmm-authoring** skill) and regenerate. - **Cross-references within spec**: Use `<>` or `<>`. - **Cross-references to other specs**: Use the URL attributes from `reference_definitions.adoc`: ```asciidoc diff --git a/plugins/openehr-specs/skills/bmm-authoring/SKILL.md b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md new file mode 100644 index 0000000..58b8137 --- /dev/null +++ b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md @@ -0,0 +1,137 @@ +--- +name: bmm-authoring +description: > + Create or edit an openEHR component's BMM schema, the P_BMM JSON in `computable/BMM/` of a + `specifications-XX` repo. This skill should be used when the user asks to write a BMM schema from + scratch, add, rename or remove a class, attribute, function, invariant or enumeration in a + component's model, set the schema header or includes, or check a hand-edited `.bmm.json`. Not for + regenerating class tables (use class-generation) or spec prose (use content-patterns). +allowed-tools: + - "Bash(python3 ${CLAUDE_SKILL_DIR}/scripts/check_bmm.py *)" +--- + +# openEHR BMM Schema Authoring (P_BMM JSON) + +A component's BMM schema, `computable/BMM/openehr__.bmm.json`, is the source of +truth for its classes. The class tables and diagrams in its documents are generated from it (see +`class-generation`), and +[`specifications-ITS-BMM`](https://github.com/openEHR/specifications-ITS-BMM) republishes it as JSON, +ODIN and YAML. Change the model here, and only here. + +- **Format reference**: `references/p-bmm-json.md` (header, packages, every class, property, type and + function kind, what the tables show, spec features the toolchain does not read). +- **Worked example**: `${CLAUDE_SKILL_DIR}/assets/openehr_demo_0.1.0.bmm.json`, one of each construct. +- **Checker**: `${CLAUDE_SKILL_DIR}/scripts/check_bmm.py` (Python 3.8+, standard library only, read-only). + +## Related Skills + +- **class-generation**: regenerate and place the class tables after a change +- **authoring**: the chapter `include::` lines for a new or removed class +- **amendment-record**: the entry for a change to published specification text +- **governance**: releases, and the lifecycle state a schema header names +- **content-patterns**: the prose around classes, which the tables do not carry + +## Why the Checker Comes Before the Render + +`bmm-publisher` exits 0 on most modelling mistakes and renders something else in their place: + +- a misspelt or missing `_type` falls back to a single property or a simple type, often of type `Any`; +- an unknown key (`is_mandantory`) is ignored without a message; +- a class missing from `packages` gets no table; +- `{"lower": 2, "upper": 5}` loses its upper limit unless `"upper_unbounded": false` is written out; +- spec constructs it does not read (indexed containers, `ancestor_defs`, `type_ref` value sets) are + dropped. + +A clean run therefore proves little. `check_bmm.py` reports each of these, so run it first, then +render and read the tables. + +## Workflow + +### Changing a schema + +1. Read the header and the classes next to the one you are changing, and follow their idiom. Run the + checker once before editing (step 4) and keep its output: the published schemas carry findings + of their own, and that output separates them from yours. +2. Make the change, using the format reference. Ground every class, attribute and function name in + the specification text being written or in the user's instruction; never invent one. +3. Increase the build number in `schema_revision` (`1.3.0.2` → `1.3.0.3`). If the release is already + published, whether the change belongs in it or in a new release (a new `rm_release`, file and + schema id, which the `includes` of dependent schemas then name) is a decision for the user; see + `governance`. +4. Run the checker, passing each included schema with `-d` (from its sibling clone): + ```bash + python3 ${CLAUDE_SKILL_DIR}/scripts/check_bmm.py computable/BMM/openehr_rm_1.2.0.bmm.json \ + -d ../specifications-BASE/computable/BMM/openehr_base_1.3.0.bmm.json + ``` + Fix every ERROR and WARNING that was not in the step 1 output. +5. Regenerate the class tables (`class-generation`) and read the table of every class you touched. + Expect the gaps listed under "What the class tables show" in the reference, such as a container's + member count or a generic parameter's constraint. +6. A new class or package needs its `include::` line in a chapter, and a removed class loses its line + (`authoring`). Each table is named after the first four parts of its class's package path and the + class name in lower case: `LOCATABLE` in `org.openehr.rm.common` → `archetyped` gives + `org.openehr.rm.common.locatable.adoc`, included as `{pkg}locatable.adoc`. +7. Add the amendment-record entry. When asked to commit, commit the BMM change on its own, as the + repo's `AGENTS.md` says; leave regenerated tables and HTML unstaged unless the task is to refresh + them. + +### Writing a schema from scratch + +1. If `computable/BMM/` already holds an empty schema for the component, fill that in. Otherwise + copy `${CLAUDE_SKILL_DIR}/assets/openehr_demo_0.1.0.bmm.json` to + `computable/BMM/.bmm.json` and replace its content. Set the header from the + reference's header table: `schema_name` is the lower-case component, `rm_release` the release, + `schema_revision` `.1`. Ask the user for `schema_author` rather than guessing it. +2. Include BASE (`"includes": {"openehr_base_": {"id": "openehr_base_"}}`) and use + its types (`Any`, `String`, `List`, `Hash`, `HIER_OBJECT_ID`, …). Do not redefine them; + `primitive_types` belongs to BASE only. +3. Give each specification document of the component one top-level package, + `org.openehr..`, matching the `:pkg:` that document's `master.adoc` sets + (`org.openehr...`): RM has `org.openehr.rm.common`, `org.openehr.rm.ehr` + and others; TERM has the one package `org.openehr.term.terminology`. Group its classes in + sub-packages as the document's chapters do. +4. Add the classes, then continue from step 4 of "Changing a schema". + +## Quick Reference + +| To model | Write | +|----------|-------| +| `uid: HIER_OBJECT_ID`, mandatory | `P_BMM_SINGLE_PROPERTY`, `"type": "HIER_OBJECT_ID"`, `"is_mandatory": true` | +| `items: List`, never empty | `P_BMM_CONTAINER_PROPERTY`, `type_def` `{"container_type": "List", "type": "ITEM"}`, `cardinality` `{"lower": 1, "upper_unbounded": true}`, `"is_mandatory": true` | +| `ranks: List`, 2 to 5 members | `cardinality` `{"lower": 2, "upper": 5, "upper_unbounded": false}` | +| `index: Hash` | `P_BMM_GENERIC_PROPERTY`, `type_def` `{"root_type": "Hash", "generic_parameters": ["String", "ITEM"]}` | +| `content: T` in `BOX` | class `generic_parameter_defs` `{"T": {"name": "T", "conforms_to_type": "ITEM"}}`; property `P_BMM_SINGLE_PROPERTY_OPEN`, `"type": "T"` | +| `List>` | container `type_def` holding a nested `type_def` with `"_type": "P_BMM_GENERIC_TYPE"` | +| `B` inherits `A` | `"ancestors": ["A"]` | +| `B` inherits `A` (open binding) | `"ancestors": ["A"]`, and redeclare `T` in `B`'s `generic_parameter_defs` | +| `B` inherits `A` (closed binding) | `"ancestors": ["A"]` only, and state the binding in `documentation` | +| String enumeration | `"_type": "P_BMM_ENUMERATION_STRING"`, `"ancestors": ["String"]`, `item_names`, `item_documentations` | +| Function | `functions` entry: `parameters`, `result` (a type object with its `_type`), `pre_conditions`, `post_conditions` | +| Invariant | `"invariants": {"Name_valid": "not name.is_empty()"}` | + +## The Checker + +- Each finding is `ERROR`, `WARNING` or `INFO`, with a JSON path. ERROR means bmm-publisher fails, + reads something other than what the file says (a misspelt `is_mandatory` included), or a generic + argument does not conform to its parameter's `conforms_to_type`. WARNING means a broken convention + or content the publisher ignores. INFO lists what could not be checked, typically type names from + an included schema that was not passed with `-d`. +- Exit status 1 means errors were found (with `--strict`, warnings count too); 2 means the file is + unreadable or not JSON. +- The published schemas carry some warnings and a few errors of their own, such as missing + documentation, or AM 2.4.0's `P_ARCHETYPE_SLOT.includes`, a `List` with no element type that renders + as `List`. Report findings outside your change to the user; do not fix them as a side effect. +- If `python3` is missing, say so, check the change by hand against the reference's lists, and + render. In a tool that does not expand `${CLAUDE_SKILL_DIR}`, use the directory that holds this + SKILL.md. + +## Guardrails + +- **Edit the JSON in place.** The files mix PHP-style escapes (`\/`, `\u2260`) with raw characters + from later edits, so loading and re-serialising a file rewrites lines you did not touch. Keep the + 4-space indentation and key order, and check that `git diff` shows only your change. +- **Edit only `computable/BMM/` in the component repo.** Never the ITS-BMM copies, the generated + `docs/UML/classes` tables, or the schemas bundled in `bmm-publisher`. +- **Keys equal names.** Every map entry is keyed by its own `name`. +- **Use the forms bmm-publisher reads** (see the reference's table of unsupported specification + features) and describe in `documentation` what they cannot express. diff --git a/plugins/openehr-specs/skills/bmm-authoring/assets/openehr_demo_0.1.0.bmm.json b/plugins/openehr-specs/skills/bmm-authoring/assets/openehr_demo_0.1.0.bmm.json new file mode 100644 index 0000000..b1c6c46 --- /dev/null +++ b/plugins/openehr-specs/skills/bmm-authoring/assets/openehr_demo_0.1.0.bmm.json @@ -0,0 +1,297 @@ +{ + "bmm_version": "2.4", + "rm_publisher": "openehr", + "schema_name": "demo", + "rm_release": "0.1.0", + "schema_revision": "0.1.0.1", + "schema_lifecycle_state": "development", + "schema_description": "Worked example for the openehr-specs bmm-authoring skill: one of each P_BMM construct. Not an openEHR model.", + "schema_author": "openehr-specs plugin (example)", + "includes": { + "openehr_base_1.3.0": { + "id": "openehr_base_1.3.0" + } + }, + "packages": { + "org.openehr.demo.inventory": { + "name": "org.openehr.demo.inventory", + "packages": { + "item": { + "name": "item", + "classes": [ + "DEMO_ITEM", + "DEMO_GROUP", + "DEMO_STATUS" + ] + }, + "box": { + "name": "box", + "classes": [ + "DEMO_BOX", + "DEMO_BOX_REF" + ] + } + } + } + }, + "class_definitions": { + "DEMO_ITEM": { + "name": "DEMO_ITEM", + "documentation": "Abstract parent of every inventory item.", + "is_abstract": true, + "ancestors": [ + "Any" + ], + "constants": { + "Max_tags": { + "name": "Max_tags", + "documentation": "Largest number of tags an item may carry.", + "type": "Integer", + "value": "32" + } + }, + "properties": { + "uid": { + "_type": "P_BMM_SINGLE_PROPERTY", + "name": "uid", + "documentation": "Globally unique identifier of this item.", + "is_mandatory": true, + "type": "HIER_OBJECT_ID" + }, + "name": { + "_type": "P_BMM_SINGLE_PROPERTY", + "name": "name", + "documentation": "Human-readable name of this item.", + "is_mandatory": true, + "type": "String" + }, + "status": { + "_type": "P_BMM_SINGLE_PROPERTY", + "name": "status", + "documentation": "Lifecycle status of this item, one of the `DEMO_STATUS` values.", + "is_mandatory": true, + "type": "DEMO_STATUS" + }, + "tags": { + "_type": "P_BMM_CONTAINER_PROPERTY", + "name": "tags", + "documentation": "Free-text tags, at most `Max_tags`.", + "type_def": { + "container_type": "List", + "type": "String" + }, + "cardinality": { + "lower": 0, + "upper_unbounded": true + } + }, + "created": { + "_type": "P_BMM_SINGLE_PROPERTY", + "name": "created", + "documentation": "Date and time this item was created, if known.", + "type": "Iso8601_date_time" + } + }, + "functions": { + "is_tagged": { + "name": "is_tagged", + "documentation": "True if `_tags_` contains `_a_tag_`.", + "parameters": { + "a_tag": { + "_type": "P_BMM_SINGLE_FUNCTION_PARAMETER", + "name": "a_tag", + "documentation": "Tag to look for.", + "type": "String" + } + }, + "pre_conditions": { + "Pre_tag_valid": "not a_tag.is_empty()" + }, + "result": { + "_type": "P_BMM_SIMPLE_TYPE", + "type": "Boolean" + } + } + }, + "invariants": { + "Name_valid": "not name.is_empty()", + "Tags_valid": "tags /= Void implies tags.count <= Max_tags" + } + }, + "DEMO_GROUP": { + "name": "DEMO_GROUP", + "documentation": "Item that groups other items.", + "ancestors": [ + "DEMO_ITEM" + ], + "properties": { + "items": { + "_type": "P_BMM_CONTAINER_PROPERTY", + "name": "items", + "documentation": "Grouped items; never empty.", + "is_mandatory": true, + "type_def": { + "container_type": "List", + "type": "DEMO_ITEM" + }, + "cardinality": { + "lower": 1, + "upper_unbounded": true + } + }, + "ranks": { + "_type": "P_BMM_CONTAINER_PROPERTY", + "name": "ranks", + "documentation": "Optional ranking, with 2 to 5 entries.", + "type_def": { + "container_type": "List", + "type": "Integer" + }, + "cardinality": { + "lower": 2, + "upper": 5, + "upper_unbounded": false + } + }, + "index": { + "_type": "P_BMM_GENERIC_PROPERTY", + "name": "index", + "documentation": "Grouped items keyed by name.", + "type_def": { + "root_type": "Hash", + "generic_parameters": [ + "String", + "DEMO_ITEM" + ] + } + } + }, + "functions": { + "item_count": { + "name": "item_count", + "documentation": "Number of grouped items.", + "post_conditions": { + "Post_result": "Result = items.count" + }, + "result": { + "_type": "P_BMM_SIMPLE_TYPE", + "type": "Integer" + } + }, + "first_tagged": { + "name": "first_tagged", + "documentation": "First grouped item carrying `_a_tag_`, or `Void` if there is none.", + "is_nullable": true, + "parameters": { + "a_tag": { + "_type": "P_BMM_SINGLE_FUNCTION_PARAMETER", + "name": "a_tag", + "documentation": "Tag to look for.", + "type": "String" + } + }, + "result": { + "_type": "P_BMM_SIMPLE_TYPE", + "type": "DEMO_ITEM" + } + } + }, + "invariants": { + "Items_valid": "not items.is_empty()" + } + }, + "DEMO_STATUS": { + "_type": "P_BMM_ENUMERATION_STRING", + "name": "DEMO_STATUS", + "documentation": "Lifecycle status of an item.", + "ancestors": [ + "String" + ], + "item_names": [ + "draft", + "active", + "retired" + ], + "item_documentations": [ + "Being prepared; not yet in use.", + "In use.", + "No longer in use." + ] + }, + "DEMO_BOX": { + "name": "DEMO_BOX", + "documentation": "Generic container holding one item of type `T` and its history.", + "ancestors": [ + "Any" + ], + "generic_parameter_defs": { + "T": { + "name": "T", + "conforms_to_type": "DEMO_ITEM" + } + }, + "properties": { + "content": { + "_type": "P_BMM_SINGLE_PROPERTY_OPEN", + "name": "content", + "documentation": "Item currently in the box.", + "is_mandatory": true, + "type": "T" + }, + "history": { + "_type": "P_BMM_CONTAINER_PROPERTY", + "name": "history", + "documentation": "Items previously in the box, oldest first.", + "type_def": { + "container_type": "List", + "type": "T" + }, + "cardinality": { + "lower": 0, + "upper_unbounded": true + } + } + } + }, + "DEMO_BOX_REF": { + "name": "DEMO_BOX_REF", + "documentation": "Reference to a box of groups, with the boxes it replaced.", + "ancestors": [ + "Any" + ], + "properties": { + "target": { + "_type": "P_BMM_GENERIC_PROPERTY", + "name": "target", + "documentation": "Box referred to.", + "is_mandatory": true, + "type_def": { + "root_type": "DEMO_BOX", + "generic_parameters": [ + "DEMO_GROUP" + ] + } + }, + "replaced": { + "_type": "P_BMM_CONTAINER_PROPERTY", + "name": "replaced", + "documentation": "Boxes this reference replaced, oldest first.", + "type_def": { + "container_type": "List", + "type_def": { + "_type": "P_BMM_GENERIC_TYPE", + "root_type": "DEMO_BOX", + "generic_parameters": [ + "DEMO_GROUP" + ] + } + }, + "cardinality": { + "lower": 0, + "upper_unbounded": true + } + } + } + } + } +} diff --git a/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md new file mode 100644 index 0000000..6434447 --- /dev/null +++ b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md @@ -0,0 +1,309 @@ +# P_BMM JSON — format reference + +The openEHR BMM schemas in `specifications-XX/computable/BMM/*.bmm.json` are JSON serialisations of +the `P_BMM_*` classes of the +[BMM Persistence specification](https://specifications.openehr.org/releases/LANG/development/bmm_persistence.html) +(LANG). The specification shows its examples in ODIN, and says JSON with type markers may be used +instead. Its class tables (chapter "Persistence Model") are the normative list of attributes; this +reference was checked against the development version linked here. This +reference describes the JSON form that the published schemas use and that +[`bmm-publisher`](https://github.com/openEHR/bmm-publisher) reads, through its parser library +[`cadasto/openehr-bmm`](https://github.com/Cadasto/openehr-bmm). Where the specification and the +toolchain differ, both are stated. + +`assets/openehr_demo_0.1.0.bmm.json` is a complete worked example that has one of each construct. +It passes `scripts/check_bmm.py` and renders with `bmm-publisher`. + +## Contents + +- [The file](#the-file) +- [Header](#header) +- [Includes](#includes) +- [Packages](#packages) +- [Classes](#classes) +- [Properties](#properties) +- [Types](#types) +- [Functions](#functions) +- [Invariants, constants and conditions](#invariants-constants-and-conditions) +- [Documentation text](#documentation-text) +- [What the class tables show](#what-the-class-tables-show) +- [Specification features bmm-publisher does not read](#specification-features-bmm-publisher-does-not-read) +- [Reading the specification's ODIN examples](#reading-the-specifications-odin-examples) + +## The file + +- One file per component release, named after its schema id: `__.bmm.json`, + for example `openehr_rm_1.2.0.bmm.json`. +- The working copy is in the component's `specifications-XX` repo under `computable/BMM/`. A push + there updates [`specifications-ITS-BMM`](https://github.com/openEHR/specifications-ITS-BMM), which + publishes the JSON and the ODIN and YAML forms it generates from it. Never edit the ITS-BMM copies. +- Every key that holds a map of named things (`packages`, `class_definitions`, `properties`, + `functions`, `parameters`, `constants`, `generic_parameter_defs`) is keyed by the item's own + `name`. bmm-publisher uses `name` and ignores the key, so a key that differs from `name` goes + unnoticed. + +## Header + +`P_BMM_SCHEMA` and its parent `BMM_SCHEMA_CORE` define the header. bmm-publisher refuses a file that +lacks a field marked *yes*; the specification alone requires those marked *yes (spec)*. + +| Field | Required | Value | +|-------|----------|-------| +| `bmm_version` | yes (spec) | `"2.4"` in every published schema, and what bmm-publisher writes | +| `rm_publisher` | yes | `"openehr"` | +| `schema_name` | yes | lower-case component name: `base`, `rm`, `am`, `lang`, `term` | +| `rm_release` | yes | 3-part release, `"1.2.0"`; part of the schema id and the file name | +| `schema_revision` | yes | `.`, for example `"1.2.0.2"`. Increase the build number with every change: ITS-BMM expects each schema change to bump it | +| `schema_lifecycle_state` | yes | the specification defines no value set. Published schemas use `"stable"`; for an unreleased schema, use the lower-case lifecycle state of the specification (`development`, `trial`; see the `governance` skill) | +| `schema_description` | yes | one sentence | +| `schema_author` | yes | the published schemas use `Name `; ask the user who to name | +| `includes` | no | see below | +| `packages` | yes | see below | +| `primitive_types` | no | class definitions of primitive types; only BASE has them | +| `class_definitions` | yes (spec) | all other class definitions | + +`BMM_SCHEMA_CORE` also defines `schema_contributors` and the archetype hints +`archetype_parent_class`, `archetype_data_value_parent_class`, `archetype_rm_closure_packages` and +`archetype_visualise_descendants_of`. bmm-publisher does not read them, so they are absent from +every generated output, including the ODIN and YAML forms. + +A changed release (a new `rm_release`) is a new file with a new schema id; while a release is still +in development, edit its file and increase the build number in `schema_revision`. + +## Includes + +```json +"includes": { + "openehr_base_1.3.0": { "id": "openehr_base_1.3.0" } +} +``` + +The key is the included schema id, as in every published schema. `includes` records the dependency +in the model, but bmm-publisher does not resolve it: pass the included file with `-d` both to +`bmm-publisher` (see the `class-generation` skill) and to `check_bmm.py`. + +## Packages + +```json +"packages": { + "org.openehr.demo.inventory": { + "name": "org.openehr.demo.inventory", + "packages": { + "item": { "name": "item", "classes": ["DEMO_ITEM", "DEMO_GROUP", "DEMO_STATUS"] }, + "box": { "name": "box", "classes": ["DEMO_BOX", "DEMO_BOX_REF"] } + } + } +} +``` + +- Only a top-level package name may contain `.`; sub-package names are single words. +- A package lists only classes defined in the same schema. +- Every class definition must be listed in exactly one package: bmm-publisher writes tables per + package, so an unlisted class gets no table, and no message. A listed class that is not defined stops the + run with `Class X not found in schema`. +- Each specification document of a component has one top-level package, + `org.openehr..`, and its `master.adoc` sets `:pkg:` to that name plus a + dot: RM has `org.openehr.rm.common`, `org.openehr.rm.ehr` and more, BASE has + `org.openehr.base.base_types`, `org.openehr.base.foundation_types` and + `org.openehr.base.resource`, TERM has `org.openehr.term.terminology`. Sub-packages group the + classes as the document's chapters do. +- The legacy class-table file name is the first four parts of the class's package path, then the + class name in lower case: `LOCATABLE` in `org.openehr.rm.common` → `archetyped` gives + `org.openehr.rm.common.locatable.adoc`, which a chapter includes as `{pkg}locatable.adoc`. + Deeper sub-packages do not change the name. A top-level package named just + `org.openehr.` takes its fourth part from its sub-package, but classes listed + directly in it get files named `org.openehr..org..adoc`. +- A top-level package that holds only sub-packages makes bmm-publisher print `Empty package …`; + BASE does the same, and the warning is harmless. + +## Classes + +A class definition is one of four kinds, chosen by `_type` (omitted for a plain class): + +| `_type` | Keys bmm-publisher reads | +|---------|--------------------------| +| *(none)* — plain class | `name`, `documentation`, `is_abstract`, `ancestors`, `generic_parameter_defs`, `constants`, `properties`, `functions`, `invariants` | +| `P_BMM_ENUMERATION_STRING`, `P_BMM_ENUMERATION_INTEGER` | `name`, `documentation`, `ancestors`, `item_names`, `item_values`, `item_documentations`, `functions` | +| `P_BMM_INTERFACE` | `name`, `documentation`, `functions` | + +- **`ancestors`** is a list of class names. A root class names `Any`, or has no `ancestors`; both + forms occur in RM. +- **Generic class**: declare each parameter, with an optional constraint: + ```json + "generic_parameter_defs": { "T": { "name": "T", "conforms_to_type": "DEMO_ITEM" } } + ``` +- **Generic inheritance**: `ancestors` takes class names only. For an open binding (`IMPORTED_VERSION` + inherits `VERSION`), list the root class and redeclare the parameter, as RM does: + `"ancestors": ["VERSION"]` with `"generic_parameter_defs": {"T": {"name": "T"}}`. For a closed + binding, list the root class only and state the binding in `documentation`, as BASE does for + `Multiplicity_interval` ("An Interval of Integer", `"ancestors": ["Proper_interval"]`). The + specification's `ancestor_defs` is not read. +- **Enumerations**: `item_names` are the symbolic names. `item_values` (optional) gives one value + per name, all strings or all integers by kind. `item_documentations` gives one text per name, by + position. The persistence model requires `String` or `Integer` among the `ancestors`. + +## Properties + +Every property carries `_type`. Without it, a property is read as `P_BMM_SINGLE_PROPERTY`, and a +property with a `type_def` then silently becomes type `Any`. + +| `_type` | For | Shape | +|---------|-----|-------| +| `P_BMM_SINGLE_PROPERTY` | `uid: HIER_OBJECT_ID` | `"type": "HIER_OBJECT_ID"` | +| `P_BMM_SINGLE_PROPERTY_OPEN` | `content: T` in a generic class | `"type": "T"`, a parameter of the class | +| `P_BMM_CONTAINER_PROPERTY` | `items: List` | `"type_def": {"container_type": "List", "type": "DEMO_ITEM"}` plus `cardinality` | +| `P_BMM_GENERIC_PROPERTY` | `index: Hash`, `target: DEMO_BOX` | `"type_def": {"root_type": "Hash", "generic_parameters": ["String", "DEMO_ITEM"]}` | + +- **`is_mandatory`**: `true` or absent (optional). +- **`cardinality`** (container properties only) counts the members: + `{"lower": 0, "upper_unbounded": true}` is `0..*`, and `{"lower": 1, "upper_unbounded": true}` + is a never-empty list (set `is_mandatory` too). A bounded upper limit needs + `"upper_unbounded": false` written out: `{"lower": 2, "upper": 5, "upper_unbounded": false}`. + bmm-publisher treats a missing `upper_unbounded` as `true` and drops the limit. +- **`Hash`** and any other container with two parameters is a `P_BMM_GENERIC_PROPERTY` with + `root_type: "Hash"`, as in BASE and RM. `container_type` takes one-parameter containers only: + `List`, `Set`, `Array`. A generic property has no `cardinality`, so state a member count as an + invariant, for example `"Codes_valid": "not codes.is_empty"`. +- **Generic arguments** bind in the order the root class declares its `generic_parameter_defs`, and + each must conform to that parameter's `conforms_to_type`; neither is checked by bmm-publisher. + BASE declares `FUNCTION`, so a function taking two strings and returning a Boolean + is `FUNCTION, Boolean>`. +- **`default`** (single properties) gives a default value, `false` or `"active"` for example; the + table shows it as `{default = …}`. + +## Types + +A type is a class name (`"type": "String"`) or a type object. The rules for type objects: + +- Directly inside a property or parameter, `type_def` has **no** `_type`; its kind follows from + the property's `_type`. +- Everywhere else (a function `result`, an entry of `generic_parameter_defs`, a nested `type_def` + inside a container type) the object **must** carry `_type`, or it is read as a simple type. + +| `_type` | Keys | +|---------|------| +| `P_BMM_SIMPLE_TYPE` | `type` | +| `P_BMM_CONTAINER_TYPE` | `container_type`, then either `type` (a name) or `type_def` (a nested type object) | +| `P_BMM_GENERIC_TYPE` | `root_type`, then `generic_parameters` (a list of names) or `generic_parameter_defs` (nested type objects keyed by parameter name), with one entry per parameter of the root class | + +Nesting, `replaced: List>`: + +```json +"type_def": { + "container_type": "List", + "type_def": { + "_type": "P_BMM_GENERIC_TYPE", + "root_type": "DEMO_BOX", + "generic_parameters": ["DEMO_GROUP"] + } +} +``` + +A container type with neither `type` nor `type_def` renders as `List`. + +## Functions + +```json +"functions": { + "is_tagged": { + "name": "is_tagged", + "documentation": "True if `_tags_` contains `_a_tag_`.", + "parameters": { + "a_tag": { + "_type": "P_BMM_SINGLE_FUNCTION_PARAMETER", + "name": "a_tag", + "documentation": "Tag to look for.", + "type": "String" + } + }, + "pre_conditions": { "Pre_tag_valid": "not a_tag.is_empty()" }, + "result": { "_type": "P_BMM_SIMPLE_TYPE", "type": "Boolean" } + } +} +``` + +- Function keys: `name`, `documentation`, `parameters`, `result`, `pre_conditions`, + `post_conditions`, `aliases` (list of names), `is_abstract`, `is_nullable`. +- `result` is a type object with `_type`. A procedure returns `{"_type": "P_BMM_SIMPLE_TYPE", "type": "void"}`. +- `is_nullable: true` on a function means it may return `Void` (the table shows `0..1`); on a + parameter it makes the parameter optional. Without it, every parameter shows as mandatory (`[1]`). +- Parameter kinds mirror the property kinds: `P_BMM_SINGLE_FUNCTION_PARAMETER` (`type`), + `P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN` (`type` is a class parameter such as `T`), + `P_BMM_CONTAINER_FUNCTION_PARAMETER` (`type_def` and a **required** `cardinality`: bmm-publisher + fails without one), `P_BMM_GENERIC_FUNCTION_PARAMETER` (`type_def`). + +## Invariants, constants and conditions + +All three are part of `P_BMM_CLASS` and `P_BMM_FUNCTION` in the specification's class tables, though +its syntax chapter does not show them. + +```json +"invariants": { "Name_valid": "not name.is_empty()" }, +"constants": { + "Max_tags": { "name": "Max_tags", "documentation": "…", "type": "Integer", "value": "32" } +} +``` + +- `invariants`, `pre_conditions` and `post_conditions` map a tag to an expression string. The + published schemas mostly tag invariants `Xxx_valid`, pre-conditions `Pre_xxx` and + post-conditions `Post_xxx`. +- A constant needs `type`, and bmm-publisher fails without one. `value` is the value as source + text: `"32"`, the name of another constant, `"\"..\""` for a string, `"'*'"` for a character. + +## Documentation text + +`documentation` (and parameter `documentation`, and `item_documentations`) is AsciiDoc inline text, +written as in the published schemas: `` `CLASS_NAME` ``, `` `_attribute_` ``, a blank line written +as `\n\n`. bmm-publisher then: + +- escapes `{…}`, so an attribute reference such as `{base_release}` is printed literally; write the + value instead; +- passes a same-document cross-reference `<<_x_class,X>>` through; +- escapes `|` and `<=`, and every `*` on a line that holds two or more, so they print literally (no + bold); +- leaves a single `*` alone, so `0..*` is safe. + +## What the class tables show + +The legacy class tables (the `docs/UML/classes` layout) leave some of the model out: + +- The cardinality column shows `1` or `0` from `is_mandatory`, then the container's bounded `upper`, otherwise `1`. `items` (`1..*` members, mandatory) shows `1..1`; `ranks` (2 to 5 members, + optional) shows `0..5`. A member lower bound never appears, so state it in `documentation` or an invariant + when readers need it. +- A generic class heading shows `DEMO_BOX` without the `conforms_to_type` constraint. +- The inheritance row names the root class only, so a closed binding such as `` shows only + where `documentation` states it. +- An enumeration shows no inheritance row; its items are listed under a "Constants" heading, with + an empty value column when there are no `item_values`. +- A function row shows `1..1`, or `0..1` when `is_nullable`; its pre- and post-conditions are + printed in the signature cell. +- The header (`schema_revision`, `schema_lifecycle_state`, `includes`) appears nowhere. + +## Specification features bmm-publisher does not read + +These are valid P_BMM but are lost on the way to the class tables and to the ODIN and YAML forms. Use +the alternative. + +| Specification feature | What bmm-publisher does | Write instead | +|-----------------------|--------------------------|---------------| +| `P_BMM_INDEXED_CONTAINER_PROPERTY` / `_TYPE` (`index_type`) | property of type `Any` | `P_BMM_GENERIC_PROPERTY` with `root_type: "Hash"`, and an invariant for any member count | +| `ancestor_defs` (generic inheritance) | ignored; the class has no parent | `ancestors` with the root class (see [Classes](#classes)) | +| `type_ref` with `value_constraint` (value sets) | ignored; with no `type`, the property is `Any` | `type`, and name the value set in `documentation` | +| `is_computed`, `is_im_infrastructure`, `is_im_runtime` | ignored | describe the property in `documentation` | +| `is_override` (class) | ignored | — | +| `model_name`, `schema_contributors`, `archetype_*` (header) | ignored | — | +| `P_BMM_OPEN_TYPE` | read as `P_BMM_SIMPLE_TYPE` | `P_BMM_SIMPLE_TYPE` | + +A misspelt key (`is_mandantory`) or `_type` is ignored in the same way, without a message. + +## Reading the specification's ODIN examples + +| ODIN | JSON | +|------|------| +| `["items"] = (P_BMM_CONTAINER_PROPERTY) < … >` | `"items": { "_type": "P_BMM_CONTAINER_PROPERTY", … }` | +| `name = <"items">` | `"name": "items"` | +| `ancestors = <"Any", "ITEM">` | `"ancestors": ["Any", "ITEM"]` | +| `is_mandatory = ` | `"is_mandatory": true` | +| `cardinality = <\|>=1\|>` | `"cardinality": {"lower": 1, "upper_unbounded": true}` | +| `cardinality = <\|2..5\|>` | `"cardinality": {"lower": 2, "upper": 5, "upper_unbounded": false}` | +| `includes = < ["1"] = < id = <"x"> > >` | `"includes": {"x": {"id": "x"}}` | diff --git a/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py b/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py new file mode 100644 index 0000000..32bf56b --- /dev/null +++ b/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py @@ -0,0 +1,788 @@ +#!/usr/bin/env python3 +"""Check an openEHR BMM schema in P_BMM JSON form for the mistakes bmm-publisher accepts silently. + +Python 3.8+, standard library only. Reads the files, writes nothing. See ../SKILL.md for when to run +it and ../references/p-bmm-json.md for the format. + + check_bmm.py SCHEMA.bmm.json [-d DEPENDENCY.bmm.json ...] [--strict] + +-d loads another schema (for example the BASE schema that RM includes) so that type names it +defines resolve; it is not checked itself. Without it, names that only an included schema could +define are listed once as unchecked. + +Each finding is printed as `LEVEL /json/path: message`, then a summary line. ERROR: bmm-publisher +fails, or silently reads something other than what the file says, or a generic argument breaks its +parameter's constraint. WARNING: a convention is broken, or the file holds something the publisher +does not read. INFO: what could not be checked. +Exit status: 0 when there is no ERROR (and, with --strict, no WARNING); 1 otherwise; 2 when a file +cannot be read or is not JSON. +""" +import argparse +import difflib +import json +import re +import sys +from pathlib import Path + +# Keys that bmm-publisher reads (through the cadasto/openehr-bmm library), per kind of object. +_PROPERTY = {"_type", "name", "documentation", "is_mandatory"} +_PARAMETER = {"_type", "name", "documentation", "is_nullable"} +_ENUMERATION = {"_type", "name", "documentation", "ancestors", "item_names", "item_values", + "item_documentations", "functions"} +READ_KEYS = { + "schema": {"bmm_version", "rm_publisher", "schema_name", "rm_release", "schema_revision", + "schema_lifecycle_state", "schema_description", "schema_author", "includes", + "packages", "primitive_types", "class_definitions"}, + "include": {"id"}, + "package": {"name", "packages", "classes"}, + "P_BMM_CLASS": {"_type", "name", "documentation", "is_abstract", "ancestors", + "generic_parameter_defs", "constants", "properties", "functions", "invariants"}, + "P_BMM_INTERFACE": {"_type", "name", "documentation", "functions"}, + "P_BMM_ENUMERATION_STRING": _ENUMERATION, + "P_BMM_ENUMERATION_INTEGER": _ENUMERATION, + "generic_parameter": {"name", "conforms_to_type"}, + "constant": {"name", "documentation", "type", "value"}, + "P_BMM_SINGLE_PROPERTY": _PROPERTY | {"type", "default"}, + "P_BMM_SINGLE_PROPERTY_OPEN": _PROPERTY | {"type", "default"}, + "P_BMM_CONTAINER_PROPERTY": _PROPERTY | {"type_def", "cardinality"}, + "P_BMM_GENERIC_PROPERTY": _PROPERTY | {"type_def"}, + "P_BMM_SIMPLE_TYPE": {"_type", "type"}, + "P_BMM_CONTAINER_TYPE": {"_type", "container_type", "type", "type_def"}, + "P_BMM_GENERIC_TYPE": {"_type", "root_type", "generic_parameters", "generic_parameter_defs"}, + "function": {"name", "aliases", "documentation", "is_abstract", "parameters", "pre_conditions", + "post_conditions", "result", "is_nullable"}, + "P_BMM_SINGLE_FUNCTION_PARAMETER": _PARAMETER | {"type"}, + "P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN": _PARAMETER | {"type"}, + "P_BMM_CONTAINER_FUNCTION_PARAMETER": _PARAMETER | {"type_def", "cardinality"}, + "P_BMM_GENERIC_FUNCTION_PARAMETER": _PARAMETER | {"type_def"}, + "cardinality": {"lower", "upper", "lower_unbounded", "upper_unbounded"}, +} + +# Keys the P_BMM model defines (LANG, BMM persistence) that bmm-publisher does not read: they stay +# in the JSON but are missing from every generated output, including the ODIN and YAML forms. +_SPEC_PROPERTY = {"is_computed", "is_im_infrastructure", "is_im_runtime", "type_ref"} +SPEC_ONLY_KEYS = { + "schema": {"model_name", "schema_contributors", "archetype_parent_class", + "archetype_data_value_parent_class", "archetype_rm_closure_packages", + "archetype_visualise_descendants_of"}, + "P_BMM_CLASS": {"ancestor_defs", "is_override"}, + "P_BMM_ENUMERATION_STRING": {"ancestor_defs"}, + "P_BMM_ENUMERATION_INTEGER": {"ancestor_defs"}, + "P_BMM_SINGLE_PROPERTY": _SPEC_PROPERTY, + "P_BMM_SINGLE_PROPERTY_OPEN": _SPEC_PROPERTY, + "P_BMM_CONTAINER_PROPERTY": _SPEC_PROPERTY, + "P_BMM_GENERIC_PROPERTY": _SPEC_PROPERTY, + "P_BMM_SIMPLE_TYPE": {"value_constraint"}, + "P_BMM_CONTAINER_TYPE": {"index_type"}, + "P_BMM_GENERIC_TYPE": {"value_constraint"}, +} + +HEADER_REQUIRED = ["bmm_version", "rm_publisher", "schema_name", "rm_release", "schema_revision", + "schema_lifecycle_state", "schema_description", "schema_author"] +CLASS_TYPES = {"P_BMM_CLASS", "P_BMM_INTERFACE", "P_BMM_ENUMERATION_STRING", "P_BMM_ENUMERATION_INTEGER"} +PROPERTY_TYPES = {"P_BMM_SINGLE_PROPERTY", "P_BMM_SINGLE_PROPERTY_OPEN", "P_BMM_CONTAINER_PROPERTY", + "P_BMM_GENERIC_PROPERTY"} +TYPE_TYPES = {"P_BMM_SIMPLE_TYPE", "P_BMM_CONTAINER_TYPE", "P_BMM_GENERIC_TYPE"} +PARAMETER_TYPES = {"P_BMM_SINGLE_FUNCTION_PARAMETER", "P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN", + "P_BMM_CONTAINER_FUNCTION_PARAMETER", "P_BMM_GENERIC_FUNCTION_PARAMETER"} +HASH_HINT = "write Hash as P_BMM_GENERIC_PROPERTY with root_type Hash and two generic_parameters" +UNSUPPORTED_TYPES = { + "P_BMM_INDEXED_CONTAINER_PROPERTY": HASH_HINT, + "P_BMM_INDEXED_CONTAINER_TYPE": HASH_HINT, +} +# an AsciiDoc attribute reference such as {base_release}; bmm-publisher escapes it, so it never resolves +ATTRIBUTE_REF = re.compile(r"\{[a-z][a-z0-9_-]*\}") +# result type of a procedure; bmm-publisher prints it without a link +PROCEDURE_RESULT = "void" +GENERIC_NAME = re.compile(r"[<>,]") + + +class Checker: + def __init__(self, schema_path, dependency_paths=()): + self.path = Path(schema_path) + self.findings = [] # (level, path, message) + self.data = load(self.path) + self.dependencies = {} # schema id -> {class name: summary}, see summarise() + own_id = schema_id(self.data) if isinstance(self.data, dict) else None + for dep in dependency_paths: + dep_data = load(Path(dep)) + if not isinstance(dep_data, dict): + continue + if schema_id(dep_data) == own_id: + self.warning("/", "-d %s is the schema being checked; it is ignored" % dep) + continue + self.dependencies[schema_id(dep_data)] = class_index(dep_data) + self.classes = {} # class name -> summary, for this schema + self.unchecked = set() # type names an included but unloaded schema might define + + # --- reporting ------------------------------------------------------------------------- + def error(self, path, message): + self.findings.append(("ERROR", path, message)) + + def warning(self, path, message): + self.findings.append(("WARNING", path, message)) + + def info(self, path, message): + self.findings.append(("INFO", path, message)) + + def count(self, level): + return sum(1 for f in self.findings if f[0] == level) + + # --- driver ---------------------------------------------------------------------------- + def run(self): + d = self.data + if not isinstance(d, dict): + self.error("/", "the schema must be a JSON object") + return self + self.check_header(d) + self.collect_classes(d) + listed = self.check_packages(d) + for section in ("primitive_types", "class_definitions"): + for name, cls in (d.get(section) or {}).items(): + path = "/%s/%s" % (section, name) + if isinstance(cls, dict): + self.check_class(cls, path) + if cls.get("name") not in listed and cls.get("name") is not None: + self.error(path, "class is not listed in any package, so no class table is " + "generated for it") + self.check_ancestor_cycles(d) + if self.unchecked: + self.info("/includes", "not checked, as no loaded schema defines them (pass the included " + "schema with -d): " + ", ".join(sorted(self.unchecked))) + return self + + # --- header ---------------------------------------------------------------------------- + def check_header(self, d): + for key in HEADER_REQUIRED: + if key not in d: + self.error("/" + key, "required header field is missing") + elif not isinstance(d[key], str): + self.error("/" + key, "must be a string") + self.check_keys(d, "schema", "") + if d.get("bmm_version") not in (None, "2.4"): + self.warning("/bmm_version", "published openEHR schemas use \"2.4\", the version " + "bmm-publisher reads and writes") + release = d.get("rm_release") + if isinstance(release, str): + if not re.fullmatch(r"\d+\.\d+\.\d+", release): + self.warning("/rm_release", "expected a 3-part numeric release such as 1.2.0") + revision = d.get("schema_revision") + if isinstance(revision, str) and not revision.startswith(release + "."): + self.warning("/schema_revision", "published schemas use ., " + "for example %s.1" % release) + if all(isinstance(d.get(k), str) for k in ("rm_publisher", "schema_name", "rm_release")): + expected = schema_id(d) + ".bmm.json" + if self.path.name != expected: + self.warning("/", "file name %s does not match the schema id; expected %s" + % (self.path.name, expected)) + includes = d.get("includes") + if includes is not None: + if not isinstance(includes, dict): + self.error("/includes", "must be an object keyed by schema id") + else: + for key, inc in includes.items(): + path = "/includes/" + key + if not isinstance(inc, dict) or not isinstance(inc.get("id"), str): + self.error(path, "each include needs an \"id\" naming the included schema") + continue + self.check_keys(inc, "include", path) + if inc["id"] != key: + self.warning(path, "published schemas key each include by its id (%s)" % inc["id"]) + included = set(i.get("id") for i in (includes or {}).values() if isinstance(i, dict)) \ + if isinstance(includes, dict) else set() + for dep_id in self.dependencies: + if dep_id not in included: + self.warning("/includes", "schema %s was loaded with -d but is not included" % dep_id) + self.missing_includes = included - set(self.dependencies) + if "class_definitions" not in d: + self.warning("/class_definitions", "the persistence model makes class_definitions mandatory") + + # --- classes and packages ---------------------------------------------------------------- + def collect_classes(self, d): + seen = {} + for section in ("primitive_types", "class_definitions"): + block = d.get(section) + if block is None: + continue + if not isinstance(block, dict): + self.error("/" + section, "must be an object keyed by class name") + continue + for key, cls in block.items(): + path = "/%s/%s" % (section, key) + if not isinstance(cls, dict): + self.error(path, "a class definition must be an object") + continue + name = cls.get("name") + if not isinstance(name, str): + self.error(path, "class has no \"name\"") + continue + if name != key: + self.error(path, "key and \"name\" (%s) differ; bmm-publisher uses the name" % name) + if name in seen: + self.error(path, "class %s is also defined at %s" % (name, seen[name])) + seen[name] = path + self.classes[name] = summarise(cls) + for dep_id, dep_classes in self.dependencies.items(): + for name in set(self.classes) & set(dep_classes): + self.warning("/", "class %s is also defined in %s" % (name, dep_id)) + + def check_packages(self, d): + listed = {} + packages = d.get("packages") + if not isinstance(packages, dict) or not packages: + self.error("/packages", "the schema needs at least one package") + return listed + publisher, schema_name = d.get("rm_publisher"), d.get("schema_name") + for key, pkg in packages.items(): + path = "/packages/" + key + if publisher == "openehr" and isinstance(schema_name, str) and isinstance(pkg, dict): + root = "org.openehr." + schema_name.lower() + name = str(pkg.get("name", "")) + if name != root and not name.startswith(root + "."): + self.warning(path, "class table file names are built from %s.; name the " + "top-level package %s., or %s with sub-packages" + % (root, root, root)) + elif name == root and pkg.get("classes"): + self.warning(path + "/classes", "classes listed directly in %s get table files named " + "%s.org..adoc; list them in a sub-package" + % (root, root)) + self.check_package(pkg, key, path, listed, top=True) + return listed + + def check_package(self, pkg, key, path, listed, top): + if not isinstance(pkg, dict): + self.error(path, "a package must be an object") + return + self.check_keys(pkg, "package", path) + name = pkg.get("name") + if not isinstance(name, str): + self.error(path, "package has no \"name\"") + elif name != key: + self.error(path, "key and \"name\" (%s) differ" % name) + elif not top and "." in name: + self.error(path, "only top-level package names may contain '.'") + classes = pkg.get("classes", []) + subpackages = pkg.get("packages", {}) + if not isinstance(classes, list) or not all(isinstance(c, str) for c in classes): + self.error(path + "/classes", "must be a list of class names") + classes = [] + if not isinstance(subpackages, dict): + self.error(path + "/packages", "must be an object keyed by package name") + subpackages = {} + if not classes and not subpackages: + self.warning(path, "empty package") + for cls in classes: + if cls not in self.classes: + self.error(path + "/classes", "%s is not defined in this schema; bmm-publisher stops " + "with 'Class %s not found in schema'" % (cls, cls)) + elif cls in listed: + self.warning(path + "/classes", "%s is also listed in %s" % (cls, listed[cls])) + else: + listed[cls] = path + for sub_key, sub in subpackages.items(): + self.check_package(sub, sub_key, path + "/packages/" + sub_key, listed, top=False) + + def check_class(self, cls, path): + kind = cls.get("_type", "P_BMM_CLASS") + if kind not in CLASS_TYPES: + self.error(path, "unknown _type %s, read as a plain class" % kind) + kind = "P_BMM_CLASS" + self.check_keys(cls, kind, path) + self.check_documentation(cls, path, required=True) + scope = set() + params = cls.get("generic_parameter_defs") + if params is not None: + if not isinstance(params, dict): + self.error(path + "/generic_parameter_defs", "must be an object keyed by parameter name") + else: + for key, param in params.items(): + p_path = path + "/generic_parameter_defs/" + key + if not isinstance(param, dict) or param.get("name") != key: + self.error(p_path, "needs a \"name\" equal to its key") + continue + self.check_keys(param, "generic_parameter", p_path) + scope.add(key) + for key, param in params.items(): + if isinstance(param, dict) and "conforms_to_type" in param: + self.check_type_name(param["conforms_to_type"], path + "/generic_parameter_defs/" + + key + "/conforms_to_type", scope) + if "is_abstract" in cls and not isinstance(cls["is_abstract"], bool): + self.error(path + "/is_abstract", "must be true or false") + ancestors = cls.get("ancestors", []) + if "ancestor_defs" in cls and not ancestors: + self.error(path, "bmm-publisher reads only \"ancestors\", so this class has no parent; list the " + "root class name there (for an open binding such as A, also redeclare T in " + "generic_parameter_defs; state a closed binding in documentation)") + if not isinstance(ancestors, list) or not all(isinstance(a, str) for a in ancestors): + self.error(path + "/ancestors", "must be a list of class names") + ancestors = [] + for i, ancestor in enumerate(ancestors): + a_path = "%s/ancestors/%d" % (path, i) + if GENERIC_NAME.search(ancestor): + self.error(a_path, "ancestors name classes only; for generic inheritance give the " + "root class (%s) and declare the parameters in generic_parameter_defs" + % ancestor.split("<")[0].strip()) + else: + self.check_type_name(ancestor, a_path, set()) + if kind.startswith("P_BMM_ENUMERATION"): + self.check_enumeration(cls, kind, path) + for key, constant in self.keyed(cls, "constants", path): + c_path = path + "/constants/" + key + self.check_keys(constant, "constant", c_path) + if not isinstance(constant.get("type"), str): + self.error(c_path, "a constant needs a \"type\"") + else: + self.check_type_name(constant["type"], c_path + "/type", scope) + self.check_documentation(constant, c_path) + for key, prop in self.keyed(cls, "properties", path): + self.check_property(prop, path + "/properties/" + key, scope) + for key, function in self.keyed(cls, "functions", path): + self.check_function(function, path + "/functions/" + key, scope) + self.check_assertions(cls, "invariants", path) + + def check_enumeration(self, cls, kind, path): + names = cls.get("item_names") + if not isinstance(names, list) or not names or not all(isinstance(n, str) for n in names): + self.error(path + "/item_names", "an enumeration needs a non-empty list of item names") + return + base = "String" if kind == "P_BMM_ENUMERATION_STRING" else "Integer" + if base not in (cls.get("ancestors") or []): + self.warning(path + "/ancestors", "the persistence model requires %s among the ancestors" % base) + values = cls.get("item_values") + if values is not None: + value_type = str if base == "String" else int + if not isinstance(values, list) or len(values) != len(names): + self.error(path + "/item_values", "needs one value per item name (%d)" % len(names)) + elif not all(isinstance(v, value_type) and not isinstance(v, bool) for v in values): + self.error(path + "/item_values", "values must be %ss" % base) + docs = cls.get("item_documentations") + if docs is not None and (not isinstance(docs, list) or len(docs) != len(names)): + self.error(path + "/item_documentations", "needs one text per item name (%d); the " + "table pairs them by position" % len(names)) + + def check_ancestor_cycles(self, d): + graph = {} + for section in ("primitive_types", "class_definitions"): + for cls in (d.get(section) or {}).values(): + if isinstance(cls, dict) and isinstance(cls.get("name"), str): + graph[cls["name"]] = [a for a in cls.get("ancestors") or [] if isinstance(a, str)] + state = {} + + def visit(name, trail): + state[name] = 1 + for ancestor in graph.get(name, []): + if state.get(ancestor) == 1: + self.error("/class_definitions/" + name, "inheritance cycle: " + + " -> ".join(trail + [ancestor])) + elif ancestor in graph and not state.get(ancestor): + visit(ancestor, trail + [ancestor]) + state[name] = 2 + + for name in graph: + if not state.get(name): + visit(name, [name]) + + # --- properties, functions and types ----------------------------------------------------- + def check_property(self, prop, path, scope): + if not isinstance(prop, dict): + self.error(path, "a property must be an object") + return + kind = prop.get("_type") + if kind is None: + if "type_def" in prop: + self.error(path, "no _type: read as P_BMM_SINGLE_PROPERTY, whose type then defaults " + "to Any; set the property _type") + return + self.warning(path, "no _type: read as P_BMM_SINGLE_PROPERTY; published schemas always set it") + kind = "P_BMM_SINGLE_PROPERTY" + elif kind in UNSUPPORTED_TYPES: + self.error(path, "bmm-publisher does not read %s (it becomes a property of type Any); %s" + % (kind, UNSUPPORTED_TYPES[kind])) + return + elif kind not in PROPERTY_TYPES: + self.error(path, "unknown _type %s: read as P_BMM_SINGLE_PROPERTY" % kind) + return + self.check_name(prop, path) + self.check_keys(prop, kind, path) + self.check_documentation(prop, path, required=True) + if "is_mandatory" in prop and not isinstance(prop["is_mandatory"], bool): + self.error(path + "/is_mandatory", "must be true or false") + if kind in ("P_BMM_SINGLE_PROPERTY", "P_BMM_SINGLE_PROPERTY_OPEN"): + target = prop.get("type") + if not isinstance(target, str): + hint = " (a value-set constraint in type_ref is not read)" if "type_ref" in prop else "" + self.error(path, "no \"type\": the property is read as type Any" + hint) + elif GENERIC_NAME.search(target): + self.error(path + "/type", "%s is not a class name; use P_BMM_CONTAINER_PROPERTY or " + "P_BMM_GENERIC_PROPERTY with a type_def" % target) + elif kind == "P_BMM_SINGLE_PROPERTY_OPEN": + if target not in scope: + self.error(path + "/type", "%s is not a generic parameter of this class" % target) + elif target in scope: + self.warning(path, "type %s is a generic parameter: use P_BMM_SINGLE_PROPERTY_OPEN" % target) + else: + self.check_type_name(target, path + "/type", scope) + elif kind == "P_BMM_CONTAINER_PROPERTY": + self.check_inline_type(prop, "P_BMM_CONTAINER_TYPE", path, scope) + if "cardinality" in prop: + self.check_cardinality(prop["cardinality"], path + "/cardinality") + else: + self.check_inline_type(prop, "P_BMM_GENERIC_TYPE", path, scope) + + def check_inline_type(self, owner, kind, path, scope): + type_def = owner.get("type_def") + if not isinstance(type_def, dict): + self.error(path, "needs a \"type_def\" object") + return + if type_def.get("_type", kind) != kind: + self.error(path + "/type_def", "_type %s is ignored here: this type_def is always read as %s" + % (type_def["_type"], kind)) + self.check_type_body(type_def, kind, path + "/type_def", scope) + + def check_type(self, t, path, scope): + """A nested type object (result, generic_parameter_defs entry, element type_def).""" + if not isinstance(t, dict): + self.error(path, "a type must be an object") + return + kind = t.get("_type") + if kind is None: + if "container_type" in t or "root_type" in t: + self.error(path, "no _type: a nested type is read as P_BMM_SIMPLE_TYPE; set _type") + return + self.warning(path, "no _type: read as P_BMM_SIMPLE_TYPE; published schemas always set it") + kind = "P_BMM_SIMPLE_TYPE" + elif kind in UNSUPPORTED_TYPES: + self.error(path, "bmm-publisher does not read %s; %s" % (kind, UNSUPPORTED_TYPES[kind])) + return + elif kind == "P_BMM_OPEN_TYPE": + self.warning(path, "read as P_BMM_SIMPLE_TYPE; published schemas write P_BMM_SIMPLE_TYPE") + kind = "P_BMM_SIMPLE_TYPE" + elif kind not in TYPE_TYPES: + self.error(path, "unknown _type %s: read as P_BMM_SIMPLE_TYPE" % kind) + return + self.check_type_body(t, kind, path, scope) + + def check_type_body(self, t, kind, path, scope): + self.check_keys(t, kind, path) + if kind == "P_BMM_SIMPLE_TYPE": + if not isinstance(t.get("type"), str): + self.error(path, "needs a \"type\"") + elif GENERIC_NAME.search(t["type"]): + self.error(path + "/type", "%s is not a class name; use P_BMM_CONTAINER_TYPE or " + "P_BMM_GENERIC_TYPE" % t["type"]) + else: + self.check_type_name(t["type"], path + "/type", scope) + elif kind == "P_BMM_CONTAINER_TYPE": + container = t.get("container_type") + if not isinstance(container, str): + self.error(path, "needs a \"container_type\" (List, Set, Array)") + else: + self.check_type_name(container, path + "/container_type", scope) + arity = self.arity(container) + if arity is not None and arity != 1: + self.error(path + "/container_type", "%s takes %d generic parameters; %s" + % (container, arity, HASH_HINT)) + has_type, has_def = "type" in t, "type_def" in t + if has_type == has_def: + self.error(path, "give the element type as either \"type\" (a name) or \"type_def\" " + "(a nested type), not %s" % ("both" if has_type else "neither: it is read as Any")) + elif has_type: + if not isinstance(t["type"], str) or GENERIC_NAME.search(t["type"]): + self.error(path + "/type", "must be a class or parameter name; nest other types " + "in \"type_def\"") + else: + self.check_type_name(t["type"], path + "/type", scope) + else: + self.check_type(t["type_def"], path + "/type_def", scope) + else: + root = t.get("root_type") + if not isinstance(root, str): + self.error(path, "needs a \"root_type\"") + return + self.check_type_name(root, path + "/root_type", scope) + simple = t.get("generic_parameters", []) + nested = t.get("generic_parameter_defs", {}) + if not isinstance(simple, list) or not isinstance(nested, dict): + self.error(path, "generic_parameters is a list of names and generic_parameter_defs an " + "object of nested types") + return + for i, param in enumerate(simple): + if isinstance(param, dict): + self.check_type(param, "%s/generic_parameters/%d" % (path, i), scope) + elif not isinstance(param, str) or GENERIC_NAME.search(param): + self.error("%s/generic_parameters/%d" % (path, i), "must be a class or parameter " + "name; nest other types in " + "generic_parameter_defs") + else: + self.check_type_name(param, "%s/generic_parameters/%d" % (path, i), scope) + for key, param in nested.items(): + self.check_type(param, path + "/generic_parameter_defs/" + key, scope) + given = len(simple) + len(nested) + arity = self.arity(root) + if given == 0: + self.error(path, "a generic type needs generic_parameters or generic_parameter_defs") + elif arity is not None and arity != given: + self.error(path, "%s takes %d generic parameters, %d given" % (root, arity, given)) + elif arity: + self.check_generic_arguments(root, simple, nested, path, scope) + + def check_generic_arguments(self, root, simple, nested, path, scope): + """Each argument must conform to the conforms_to_type of the parameter it binds.""" + params = self.lookup(root)["params"] + bound = [] # (parameter, constraint, argument class name, path) + if not nested: + for i, ((param, constraint), arg) in enumerate(zip(params, simple)): + if isinstance(arg, str): + bound.append((param, constraint, arg, "%s/generic_parameters/%d" % (path, i))) + else: + constraints = dict(params) + for key, arg in nested.items(): + if key in constraints and isinstance(arg, dict): + name = arg.get("type") or arg.get("root_type") + if isinstance(name, str): + bound.append((key, constraints[key], name, path + "/generic_parameter_defs/" + key)) + for param, constraint, arg, arg_path in bound: + if constraint and arg not in scope and self.conforms(arg, constraint) is False: + self.error(arg_path, "%s does not conform to %s, the constraint on parameter %s of %s " + "(generic_parameters bind in the order %s declares them: %s)" + % (arg, constraint, param, root, root, ", ".join(p for p, _ in params))) + + def check_function(self, function, path, scope): + if not isinstance(function, dict): + self.error(path, "a function must be an object") + return + self.check_name(function, path) + self.check_keys(function, "function", path) + self.check_documentation(function, path, required=True) + for flag in ("is_abstract", "is_nullable"): + if flag in function and not isinstance(function[flag], bool): + self.error(path + "/" + flag, "must be true or false") + aliases = function.get("aliases", []) + if not isinstance(aliases, list) or not all(isinstance(a, str) for a in aliases): + self.error(path + "/aliases", "must be a list of strings") + if "result" in function: + self.check_type(function["result"], path + "/result", scope) + else: + self.warning(path, "no result: the table shows an empty result type") + for key, param in self.keyed(function, "parameters", path): + self.check_parameter(param, path + "/parameters/" + key, scope) + self.check_assertions(function, "pre_conditions", path) + self.check_assertions(function, "post_conditions", path) + + def check_parameter(self, param, path, scope): + kind = param.get("_type") + if kind is None: + if "type_def" in param: + self.error(path, "no _type: read as P_BMM_SINGLE_FUNCTION_PARAMETER of type Any") + return + kind = "P_BMM_SINGLE_FUNCTION_PARAMETER" + elif kind not in PARAMETER_TYPES: + self.error(path, "unknown _type %s: read as P_BMM_SINGLE_FUNCTION_PARAMETER" % kind) + return + self.check_name(param, path) + self.check_keys(param, kind, path) + self.check_documentation(param, path) + if "is_nullable" in param and not isinstance(param["is_nullable"], bool): + self.error(path + "/is_nullable", "must be true or false") + if kind in ("P_BMM_SINGLE_FUNCTION_PARAMETER", "P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN"): + target = param.get("type") + if not isinstance(target, str) or GENERIC_NAME.search(target): + self.error(path, "needs a \"type\" naming a class or generic parameter") + elif kind.endswith("_OPEN") and target not in scope: + self.error(path + "/type", "%s is not a generic parameter of this class" % target) + else: + self.check_type_name(target, path + "/type", scope) + elif kind == "P_BMM_CONTAINER_FUNCTION_PARAMETER": + self.check_inline_type(param, "P_BMM_CONTAINER_TYPE", path, scope) + if "cardinality" not in param: + self.error(path, "a container parameter needs a \"cardinality\"; bmm-publisher fails " + "without one") + else: + self.check_cardinality(param["cardinality"], path + "/cardinality") + else: + self.check_inline_type(param, "P_BMM_GENERIC_TYPE", path, scope) + + def check_cardinality(self, card, path): + if not isinstance(card, dict): + self.error(path, "must be an object such as {\"lower\": 0, \"upper_unbounded\": true}") + return + self.check_keys(card, "cardinality", path) + for key in ("lower", "upper"): + if key in card and (not isinstance(card[key], int) or isinstance(card[key], bool) or card[key] < 0): + self.error(path + "/" + key, "must be a non-negative integer") + for key in ("lower_unbounded", "upper_unbounded"): + if key in card and not isinstance(card[key], bool): + self.error(path + "/" + key, "must be true or false") + if "upper" in card and card.get("upper_unbounded", True) is not False: + self.error(path, "an upper limit needs \"upper_unbounded\": false; bmm-publisher treats a " + "missing upper_unbounded as true and ignores the limit") + if card.get("upper_unbounded") is False and "upper" not in card: + self.error(path, "\"upper_unbounded\": false needs an \"upper\" limit") + if isinstance(card.get("upper"), int) and isinstance(card.get("lower"), int) and card["upper"] < card["lower"]: + self.error(path, "upper is below lower") + if card.get("lower_unbounded"): + self.warning(path + "/lower_unbounded", "a cardinality has a lower limit (0 or more)") + + # --- shared helpers ------------------------------------------------------------------------ + def keyed(self, owner, key, path): + block = owner.get(key) + if block is None: + return [] + if not isinstance(block, dict): + self.error(path + "/" + key, "must be an object keyed by name") + return [] + items = [] + for item_key, item in block.items(): + if not isinstance(item, dict): + self.error("%s/%s/%s" % (path, key, item_key), "must be an object") + else: + items.append((item_key, item)) + return items + + def check_name(self, item, path): + key = path.rsplit("/", 1)[-1] + name = item.get("name") + if not isinstance(name, str): + self.error(path, "has no \"name\"") + elif name != key: + self.error(path, "key and \"name\" (%s) differ; bmm-publisher uses the name" % name) + + def check_keys(self, item, kind, path): + for key in item: + if key in READ_KEYS[kind]: + continue + near = difflib.get_close_matches(key, sorted(READ_KEYS[kind]), n=1, cutoff=0.8) + if key in SPEC_ONLY_KEYS.get(kind, ()): + self.warning("%s/%s" % (path, key), "defined by the persistence model but not read by " + "bmm-publisher, so it is missing from every output") + elif near and near[0] not in item: + self.error("%s/%s" % (path, key), "unknown key, probably a misspelling of %s; bmm-publisher " + "ignores it, so %s keeps its default" % (near[0], near[0])) + else: + self.warning("%s/%s" % (path, key), "unknown key, ignored by bmm-publisher (expected " + "one of: %s)" % ", ".join(sorted(READ_KEYS[kind]))) + + def check_documentation(self, item, path, required=False): + doc = item.get("documentation") + if doc is None: + if required: + self.warning(path, "no documentation: the table description is empty") + return + if not isinstance(doc, str): + self.error(path + "/documentation", "must be a string") + elif ATTRIBUTE_REF.search(doc): + self.warning(path + "/documentation", "bmm-publisher escapes the attribute reference %s, so it " + "is printed literally; write the value" + % ATTRIBUTE_REF.search(doc).group(0)) + + def check_assertions(self, owner, key, path): + block = owner.get(key) + if block is None: + return + if not isinstance(block, dict) or not all(isinstance(v, str) for v in block.values()): + self.error(path + "/" + key, "must be an object mapping a tag to an expression string") + + def lookup(self, name): + if name in self.classes: + return self.classes[name] + for dep_classes in self.dependencies.values(): + if name in dep_classes: + return dep_classes[name] + return None + + def arity(self, name): + info = self.lookup(name) + return None if info is None else len(info["params"]) + + def conforms(self, name, target): + """True or False; None when the ancestry leaves the loaded schemas, so it cannot be decided.""" + if target == "Any": + return True + seen, todo, undecided = set(), [name], False + while todo: + current = todo.pop() + if current == target: + return True + if current in seen: + continue + seen.add(current) + info = self.lookup(current) + if info is None: + undecided = True + else: + todo.extend(info["ancestors"]) + return None if undecided else False + + def check_type_name(self, name, path, scope): + if not isinstance(name, str) or not name: + self.error(path, "must be a non-empty class name") + return + if name in scope or name == PROCEDURE_RESULT or self.arity(name) is not None: + return + if len(name) == 1: + self.warning(path, "%s is not a generic parameter of this class; declare it in " + "generic_parameter_defs" % name) + elif self.missing_includes: + self.unchecked.add(name) + else: + self.warning(path, "type %s is not defined in this schema or any loaded schema" % name) + + +def load(path): + try: + with open(path, encoding="utf-8") as fh: + return json.load(fh) + except OSError as exc: + raise SystemExit(fail("cannot read %s: %s" % (path, exc.strerror or exc))) + except json.JSONDecodeError as exc: + raise SystemExit(fail("%s is not valid JSON: %s (line %d, column %d)" + % (path, exc.msg, exc.lineno, exc.colno))) + + +def fail(message): + print("check_bmm: " + message, file=sys.stderr) + return 2 + + +def schema_id(d): + return "%s_%s_%s" % (d.get("rm_publisher"), d.get("schema_name"), d.get("rm_release")) + + +def summarise(cls): + """What other classes need to know about a class: its generic parameters and its ancestors.""" + params = cls.get("generic_parameter_defs") + params = params if isinstance(params, dict) else {} + ancestors = cls.get("ancestors") + ancestors = ancestors if isinstance(ancestors, list) else [] + return { + "params": [(key, p.get("conforms_to_type") if isinstance(p, dict) else None) for key, p in params.items()], + "ancestors": [a for a in ancestors if isinstance(a, str)], + } + + +def class_index(d): + classes = {} + for section in ("primitive_types", "class_definitions"): + for cls in (d.get(section) or {}).values(): + if isinstance(cls, dict) and isinstance(cls.get("name"), str): + classes[cls["name"]] = summarise(cls) + return classes + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("schema", help="the .bmm.json file to check") + parser.add_argument("-d", "--dependency", action="append", default=[], + help="a schema whose classes the checked one uses (repeatable)") + parser.add_argument("--strict", action="store_true", help="exit 1 on warnings too") + args = parser.parse_args(argv) + checker = Checker(args.schema, args.dependency).run() + for level, path, message in checker.findings: + print("%-7s %s: %s" % (level, path, message)) + errors, warnings = checker.count("ERROR"), checker.count("WARNING") + print("check_bmm: %s: %d error(s), %d warning(s)" % (args.schema, errors, warnings)) + return 1 if errors or (args.strict and warnings) else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/plugins/openehr-specs/skills/class-generation/SKILL.md b/plugins/openehr-specs/skills/class-generation/SKILL.md index f0cad58..034080e 100644 --- a/plugins/openehr-specs/skills/class-generation/SKILL.md +++ b/plugins/openehr-specs/skills/class-generation/SKILL.md @@ -3,11 +3,11 @@ name: class-generation description: > Generate openEHR class documentation — class-definition tables, effective views, and UML class/package diagrams — from BMM schemas with `bmm-publisher`, for `specifications-XX` - repos and their `docs/UML/` content. This skill should be used when the user asks to regenerate or + repos. This skill should be used when the user asks to regenerate or refresh class tables or diagrams, run bmm-publisher, generate class docs from BMM/P_BMM, or replace - hand-written or MagicDraw-derived class docs. Not for spec prose (use content-patterns), document - scaffolding or hand-drawn diagrams (use authoring), or archetype/template work (openehr-assistant - plugin). + hand-written or MagicDraw-derived class docs. Not for editing the BMM itself (use bmm-authoring), + spec prose (use content-patterns), document scaffolding or hand-drawn diagrams (use authoring), or + archetype/template work (openehr-assistant plugin). --- # openEHR Class Documentation Generation (BMM) @@ -23,6 +23,7 @@ component's **BMM** (Basic Meta-Model) schema, serialised as ## Related Skills +- **bmm-authoring** — write or change the BMM schema itself, and check it before generating - **authoring** — document scaffolding and repo layout; explains where generated class docs are included - **review** — the ADOC-03 check enforces that class tables are generated, not hand-written - **content-patterns** — prose around classes (semantics, rationale) that the generated tables do *not* cover @@ -128,7 +129,9 @@ the user can run `/openehr-specs:publish `. ## Guardrails -- **Generated output is never hand-edited.** Fix the BMM schema and regenerate. +- **Generated output is never hand-edited.** Fix the BMM schema (see `bmm-authoring`) and regenerate. + A clean run does not prove the schema is right: `bmm-publisher` renders many modelling mistakes as + type `Any` or drops them without a message, so run the `bmm-authoring` checker first. - **Use the repo's BMM, not the bundled copy.** Mount the files under `/in/` and pass their paths. Do not mount a directory over `/app/resources`: it hides the bundled schemas, so a dependency named by id is then missing and the run exits 1. diff --git a/scripts/test_check_bmm.py b/scripts/test_check_bmm.py new file mode 100644 index 0000000..6d8a560 --- /dev/null +++ b/scripts/test_check_bmm.py @@ -0,0 +1,386 @@ +#!/usr/bin/env python3 +"""Tests for the bmm-authoring skill's checker (plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py). + +Run from the repo root: python3 -m unittest discover -s scripts -p 'test_*.py' +Standard library only; every test works in a temporary directory. +""" +import copy +import importlib.util +import io +import json +import sys +import tempfile +import unittest +from contextlib import redirect_stderr, redirect_stdout +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +SKILL = ROOT / "plugins/openehr-specs/skills/bmm-authoring" +_spec = importlib.util.spec_from_file_location("check_bmm", SKILL / "scripts/check_bmm.py") +check_bmm = importlib.util.module_from_spec(_spec) +sys.modules["check_bmm"] = check_bmm +_spec.loader.exec_module(check_bmm) + +EXAMPLE = json.loads((SKILL / "assets/openehr_demo_0.1.0.bmm.json").read_text(encoding="utf-8")) + +# The BASE types the example uses, as a minimal stand-in for openehr_base_1.3.0.bmm.json. +BASE = { + "bmm_version": "2.4", "rm_publisher": "openehr", "schema_name": "base", "rm_release": "1.3.0", + "schema_revision": "1.3.0.2", "schema_lifecycle_state": "stable", "schema_description": "stub", + "schema_author": "test", + "packages": {"org.openehr.base.foundation_types": { + "name": "org.openehr.base.foundation_types", + "classes": ["Any", "Boolean", "Integer", "String", "Iso8601_date_time", "List", "Hash", "HIER_OBJECT_ID"]}}, + "primitive_types": { + "Any": {"name": "Any", "is_abstract": True}, + "Boolean": {"name": "Boolean"}, + "Integer": {"name": "Integer"}, + "String": {"name": "String"}, + "Iso8601_date_time": {"name": "Iso8601_date_time"}, + "List": {"name": "List", "generic_parameter_defs": {"T": {"name": "T"}}}, + "Hash": {"name": "Hash", "generic_parameter_defs": {"K": {"name": "K"}, "V": {"name": "V"}}}, + }, + "class_definitions": {"HIER_OBJECT_ID": {"name": "HIER_OBJECT_ID"}}, +} + + +class Base(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.root = Path(tmp.name) + self.base = self.write("openehr_base_1.3.0.bmm.json", BASE) + + def write(self, name, data): + path = self.root / name + path.write_text(json.dumps(data, indent=4), encoding="utf-8") + return path + + def check(self, data, name="openehr_demo_0.1.0.bmm.json", deps=True): + path = self.write(name, data) + return check_bmm.Checker(path, [self.base] if deps else []).run() + + def demo(self): + return copy.deepcopy(EXAMPLE) + + def messages(self, checker, level): + return ["%s: %s" % (p, m) for lvl, p, m in checker.findings if lvl == level] + + def assertFinding(self, checker, level, path, text): + hits = [m for m in self.messages(checker, level) if m.startswith(path + ":") and text in m] + self.assertTrue(hits, "no %s at %s containing %r in:\n%s" + % (level, path, text, "\n".join(lvl + " " + p + ": " + m for lvl, p, m in checker.findings))) + + +class ExampleTest(Base): + def test_example_is_clean_with_its_dependency(self): + checker = self.check(self.demo()) + self.assertEqual(checker.findings, []) + + def test_without_dependency_base_types_are_listed_as_unchecked(self): + checker = self.check(self.demo(), deps=False) + self.assertEqual([lvl for lvl, _, _ in checker.findings], ["INFO"]) + self.assertIn("HIER_OBJECT_ID", checker.findings[0][2]) + + +class HeaderTest(Base): + def test_missing_required_field(self): + d = self.demo() + del d["schema_author"] + self.assertFinding(self.check(d), "ERROR", "/schema_author", "required header field") + + def test_file_name_must_match_schema_id(self): + self.assertFinding(self.check(self.demo(), name="demo.bmm.json"), "WARNING", "/", "expected openehr_demo_0.1.0.bmm.json") + + def test_revision_follows_release(self): + d = self.demo() + d["schema_revision"] = "7" + self.assertFinding(self.check(d), "WARNING", "/schema_revision", ".") + + def test_spec_only_header_key_is_reported_as_ignored(self): + d = self.demo() + d["archetype_parent_class"] = "DEMO_ITEM" + self.assertFinding(self.check(d), "WARNING", "/archetype_parent_class", "not read by bmm-publisher") + + def test_schema_passed_as_its_own_dependency_is_ignored(self): + path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + checker = check_bmm.Checker(path, [self.base, path]).run() + self.assertEqual([m for lvl, _, m in checker.findings], ["-d %s is the schema being checked; it is ignored" % path]) + + def test_dependency_not_included(self): + d = self.demo() + del d["includes"] + self.assertFinding(self.check(d), "WARNING", "/includes", "loaded with -d but is not included") + + +class PackageTest(Base): + def test_class_not_in_any_package_gets_no_table(self): + d = self.demo() + d["packages"]["org.openehr.demo.inventory"]["packages"]["box"]["classes"].remove("DEMO_BOX_REF") + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF", "not listed in any package") + + def test_listed_class_must_be_defined(self): + d = self.demo() + d["packages"]["org.openehr.demo.inventory"]["packages"]["box"]["classes"].append("DEMO_GHOST") + self.assertFinding(self.check(d), "ERROR", "/packages/org.openehr.demo.inventory/packages/box/classes", "DEMO_GHOST is not defined") + + def test_only_top_level_names_contain_dots(self): + d = self.demo() + pkgs = d["packages"]["org.openehr.demo.inventory"]["packages"] + pkgs["a.b"] = dict(pkgs.pop("box"), name="a.b") + self.assertFinding(self.check(d), "ERROR", "/packages/org.openehr.demo.inventory/packages/a.b", "only top-level") + + def test_top_level_package_name_drives_table_file_names(self): + d = self.demo() + d["packages"] = {"demo": dict(d["packages"]["org.openehr.demo.inventory"], name="demo")} + self.assertFinding(self.check(d), "WARNING", "/packages/demo", "org.openehr.demo.") + + def test_classes_directly_in_the_component_root_package(self): + d = self.demo() + classes = [c for p in d["packages"]["org.openehr.demo.inventory"]["packages"].values() for c in p["classes"]] + d["packages"] = {"org.openehr.demo": {"name": "org.openehr.demo", "classes": classes}} + self.assertFinding(self.check(d), "WARNING", "/packages/org.openehr.demo/classes", "org.openehr.demo.org..adoc") + + def test_package_holding_only_sub_packages_is_accepted(self): + d = self.demo() + d["packages"] = {"org.openehr.demo": dict(d["packages"]["org.openehr.demo.inventory"], name="org.openehr.demo")} + self.assertEqual(self.check(d).findings, []) + + +class ClassTest(Base): + def test_key_and_name_differ(self): + d = self.demo() + d["class_definitions"]["DEMO_ITEM"]["name"] = "DEMO_THING" + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM", "key and \"name\"") + + def test_unknown_class_type(self): + d = self.demo() + d["class_definitions"]["DEMO_STATUS"]["_type"] = "P_BMM_ENUMERATION" + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS", "unknown _type P_BMM_ENUMERATION") + + def test_ancestor_defs_alone_loses_the_parent(self): + d = self.demo() + group = d["class_definitions"]["DEMO_GROUP"] + del group["ancestors"] + group["ancestor_defs"] = {"DEMO_ITEM": {"_type": "P_BMM_SIMPLE_TYPE", "type": "DEMO_ITEM"}} + checker = self.check(d) + self.assertFinding(checker, "ERROR", "/class_definitions/DEMO_GROUP", "has no parent") + self.assertFinding(checker, "WARNING", "/class_definitions/DEMO_GROUP/ancestor_defs", "not read by bmm-publisher") + + def test_generic_name_in_ancestors(self): + d = self.demo() + d["class_definitions"]["DEMO_GROUP"]["ancestors"] = ["DEMO_BOX"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_GROUP/ancestors/0", "root class (DEMO_BOX)") + + def test_inheritance_cycle(self): + d = self.demo() + d["class_definitions"]["DEMO_ITEM"]["ancestors"] = ["DEMO_GROUP"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_GROUP", + "inheritance cycle: DEMO_ITEM -> DEMO_GROUP -> DEMO_ITEM") + + def test_enumeration_lists_must_line_up(self): + d = self.demo() + d["class_definitions"]["DEMO_STATUS"]["item_documentations"].pop() + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS/item_documentations", "one text per item name (3)") + + def test_enumeration_value_kind(self): + d = self.demo() + d["class_definitions"]["DEMO_STATUS"]["item_values"] = [1, 2, 3] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS/item_values", "must be Strings") + + def test_constant_needs_a_type(self): + d = self.demo() + del d["class_definitions"]["DEMO_ITEM"]["constants"]["Max_tags"]["type"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/constants/Max_tags", "needs a \"type\"") + + def test_invariants_map_tags_to_strings(self): + d = self.demo() + d["class_definitions"]["DEMO_ITEM"]["invariants"] = ["not name.is_empty()"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/invariants", "tag to an expression") + + +class PropertyTest(Base): + P = "/class_definitions/DEMO_GROUP/properties/" + + def props(self, d, cls="DEMO_GROUP"): + return d["class_definitions"][cls]["properties"] + + def test_misspelt_property_type_falls_back(self): + d = self.demo() + self.props(d)["items"]["_type"] = "P_BMM_CONTAINER_PROPRETY" + self.assertFinding(self.check(d), "ERROR", self.P + "items", "read as P_BMM_SINGLE_PROPERTY") + + def test_container_without_type_becomes_any(self): + d = self.demo() + del self.props(d)["items"]["_type"] + self.assertFinding(self.check(d), "ERROR", self.P + "items", "defaults to Any") + + def test_indexed_container_is_not_read(self): + d = self.demo() + self.props(d)["index"] = {"_type": "P_BMM_INDEXED_CONTAINER_PROPERTY", "name": "index", "documentation": "x", + "type_def": {"container_type": "Hash", "index_type": "String", "type": "DEMO_ITEM"}} + self.assertFinding(self.check(d), "ERROR", self.P + "index", "root_type Hash") + + def test_value_set_constraint_loses_the_type(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["name"] = {"_type": "P_BMM_SINGLE_PROPERTY", "name": "name", "documentation": "x", + "type_ref": {"type": "String", "value_constraint": "openEHR::languages"}} + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/properties/name", "read as type Any") + + def test_bounded_upper_needs_upper_unbounded_false(self): + d = self.demo() + del self.props(d)["ranks"]["cardinality"]["upper_unbounded"] + self.assertFinding(self.check(d), "ERROR", self.P + "ranks/cardinality", "\"upper_unbounded\": false") + + def test_upper_below_lower(self): + d = self.demo() + self.props(d)["ranks"]["cardinality"]["upper"] = 1 + self.assertFinding(self.check(d), "ERROR", self.P + "ranks/cardinality", "upper is below lower") + + def test_hash_is_not_a_container_type(self): + d = self.demo() + self.props(d)["items"]["type_def"]["container_type"] = "Hash" + self.assertFinding(self.check(d), "ERROR", self.P + "items/type_def/container_type", "Hash takes 2") + + def test_container_needs_an_element_type(self): + d = self.demo() + del self.props(d)["items"]["type_def"]["type"] + self.assertFinding(self.check(d), "ERROR", self.P + "items/type_def", "read as Any") + + def test_generic_parameter_count(self): + d = self.demo() + self.props(d)["index"]["type_def"]["generic_parameters"] = ["String"] + self.assertFinding(self.check(d), "ERROR", self.P + "index/type_def", "Hash takes 2 generic parameters, 1 given") + + def test_nested_type_needs_its_type(self): + d = self.demo() + del self.props(d, "DEMO_BOX_REF")["replaced"]["type_def"]["type_def"]["_type"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/replaced/type_def/type_def", "no _type") + + def test_open_property_needs_a_declared_parameter(self): + d = self.demo() + self.props(d, "DEMO_BOX")["content"]["type"] = "U" + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX/properties/content/type", "U is not a generic parameter") + + def test_generic_name_in_type(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["type"] = "List" + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/properties/uid/type", "not a class name") + + def test_misspelt_key_is_reported(self): + d = self.demo() + uid = self.props(d, "DEMO_ITEM")["uid"] + uid["is_mandantory"] = uid.pop("is_mandatory") + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/properties/uid/is_mandantory", + "misspelling of is_mandatory") + + def test_unknown_key_that_resembles_nothing_is_a_warning(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["colour"] = "blue" + self.assertFinding(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/colour", "unknown key") + + def test_generic_argument_must_conform(self): + d = self.demo() + self.props(d, "DEMO_BOX_REF")["target"]["type_def"]["generic_parameters"] = ["String"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/target/type_def/generic_parameters/0", + "String does not conform to DEMO_ITEM, the constraint on parameter T of DEMO_BOX") + + def test_nested_generic_argument_must_conform(self): + d = self.demo() + nested = self.props(d, "DEMO_BOX_REF")["replaced"]["type_def"]["type_def"] + nested["generic_parameters"] = ["Integer"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/replaced/type_def/type_def/generic_parameters/0", + "Integer does not conform to DEMO_ITEM") + + def test_conformance_through_a_descendant(self): + d = self.demo() + self.props(d, "DEMO_BOX_REF")["target"]["type_def"]["generic_parameters"] = ["DEMO_ITEM"] + self.assertEqual(self.check(d).findings, []) + + def test_unresolved_type_name(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["type"] = "HIER_OBJECT_IDX" + self.assertFinding(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/type", "HIER_OBJECT_IDX is not defined") + + def test_attribute_reference_in_documentation(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["documentation"] = "See {base_release} for details; {1..1} stays literal." + checker = self.check(d) + self.assertFinding(checker, "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/documentation", "{base_release}") + self.assertEqual(len(self.messages(checker, "WARNING")), 1) + + +class FunctionTest(Base): + F = "/class_definitions/DEMO_ITEM/functions/is_tagged" + + def fn(self, d): + return d["class_definitions"]["DEMO_ITEM"]["functions"]["is_tagged"] + + def test_container_parameter_needs_cardinality(self): + d = self.demo() + self.fn(d)["parameters"]["a_tag"] = {"_type": "P_BMM_CONTAINER_FUNCTION_PARAMETER", "name": "a_tag", + "type_def": {"container_type": "List", "type": "String"}} + self.assertFinding(self.check(d), "ERROR", self.F + "/parameters/a_tag", "needs a \"cardinality\"") + + def test_result_needs_its_type(self): + d = self.demo() + self.fn(d)["result"] = {"container_type": "List", "type": "String"} + self.assertFinding(self.check(d), "ERROR", self.F + "/result", "no _type") + + def test_simple_result_without_type_marker(self): + d = self.demo() + self.fn(d)["result"] = {"type": "Boolean"} + self.assertFinding(self.check(d), "WARNING", self.F + "/result", "published schemas always set it") + + def test_void_result_is_accepted(self): + d = self.demo() + self.fn(d)["result"] = {"_type": "P_BMM_SIMPLE_TYPE", "type": "void"} + self.assertEqual(self.check(d).findings, []) + + def test_conditions_map_tags_to_strings(self): + d = self.demo() + self.fn(d)["pre_conditions"] = {"Pre_tag_valid": True} + self.assertFinding(self.check(d), "ERROR", self.F + "/pre_conditions", "tag to an expression") + + +class CommandLineTest(Base): + def run_main(self, *args): + out, err = io.StringIO(), io.StringIO() + with redirect_stdout(out), redirect_stderr(err): + try: + code = check_bmm.main([str(a) for a in args]) + except SystemExit as exc: + code = exc.code + return code, out.getvalue(), err.getvalue() + + def test_clean_file_exits_zero(self): + path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + code, out, _ = self.run_main(path, "-d", self.base) + self.assertEqual(code, 0) + self.assertIn("0 error(s), 0 warning(s)", out) + + def test_errors_exit_one(self): + d = self.demo() + del d["class_definitions"]["DEMO_ITEM"]["properties"]["uid"]["type"] + code, out, _ = self.run_main(self.write("openehr_demo_0.1.0.bmm.json", d), "-d", self.base) + self.assertEqual(code, 1) + self.assertIn("ERROR", out) + + def test_strict_fails_on_warnings(self): + path = self.write("demo.bmm.json", self.demo()) + self.assertEqual(self.run_main(path, "-d", self.base)[0], 0) + self.assertEqual(self.run_main(path, "-d", self.base, "--strict")[0], 1) + + def test_invalid_json_exits_two(self): + path = self.root / "broken.bmm.json" + path.write_text("{\"bmm_version\": ", encoding="utf-8") + code, _, err = self.run_main(path) + self.assertEqual(code, 2) + self.assertIn("not valid JSON", err) + + def test_missing_file_exits_two(self): + self.assertEqual(self.run_main(self.root / "absent.bmm.json")[0], 2) + + +if __name__ == "__main__": + unittest.main() From 067fff89907f71538aebf0b1ad69aa3472c3d0aa Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Fri, 9 Oct 2026 15:50:28 +0200 Subject: [PATCH 2/6] fix(openehr-specs): close the silent passes and crashes in check_bmm Review of #11 found that the checker could pass schemas bmm-publisher mis-renders, and could crash on malformed input: - A run without -d for an included schema exits 3 (incomplete) instead of 0, also under --strict; the summary counts the unchecked names, and names close to a known class are still flagged ("did you mean"). - No traceback on wrongly typed values; a non-UTF-8 file, or a -d file that is not a BMM schema, exits 2. Duplicate -d schema ids are merged. - Type shapes bmm-publisher renders wrongly are errors: a simple type in a container's type_def (List), a non-generic type object in generic_parameters (publisher crash), generic_parameters and generic_parameter_defs together (defs ignored), a container type in generic_parameter_defs (prints nothing). - Generic arguments bind by position, as bmm-publisher renders them; an argument that is the enclosing class's parameter conforms through its own constraint. - An unknown _type is reported and the item is still checked, as the closest known kind. Lost content is an error: a parent only in ancestor_defs, index_type, class keys on enumerations and interfaces. - Classes in packages deeper than four levels (no table) are errors. - Corrected: a container parameter's cardinality is optional (the parser defaults it), a missing bmm_version is a warning, class_definitions is optional, void is accepted only as a function result, and only lower-case {attribute} references are flagged. Tests cover every finding the checker can report (a mutation run that disables each one in turn leaves no test green), use a slice of the real BASE ancestry and constraints, and put a wrongly typed value at every node of the worked example. Co-Authored-By: Claude Opus 5.5 --- .../skills/bmm-authoring/scripts/check_bmm.py | 644 ++++++++------ scripts/test_check_bmm.py | 827 ++++++++++++++---- 2 files changed, 1079 insertions(+), 392 deletions(-) diff --git a/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py b/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py index 32bf56b..4fa5d3a 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py +++ b/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py @@ -8,14 +8,16 @@ -d loads another schema (for example the BASE schema that RM includes) so that type names it defines resolve; it is not checked itself. Without it, names that only an included schema could -define are listed once as unchecked. +define cannot be checked, and neither can the generic parameters and constraints of those types. Each finding is printed as `LEVEL /json/path: message`, then a summary line. ERROR: bmm-publisher fails, or silently reads something other than what the file says, or a generic argument breaks its parameter's constraint. WARNING: a convention is broken, or the file holds something the publisher does not read. INFO: what could not be checked. -Exit status: 0 when there is no ERROR (and, with --strict, no WARNING); 1 otherwise; 2 when a file -cannot be read or is not JSON. +Exit status: 0 when the check is complete and found no ERROR (with --strict, no WARNING either); +1 when it found an ERROR (with --strict, or a WARNING); 3 when it found none but could not check +every name, because an included schema was not loaded with -d; 2 when a file cannot be read, is not +UTF-8 JSON, or (for -d) is not a BMM schema. """ import argparse import difflib @@ -58,8 +60,8 @@ "cardinality": {"lower", "upper", "lower_unbounded", "upper_unbounded"}, } -# Keys the P_BMM model defines (LANG, BMM persistence) that bmm-publisher does not read: they stay -# in the JSON but are missing from every generated output, including the ODIN and YAML forms. +# Keys the persistence specification knows that bmm-publisher does not read: they stay in the JSON +# but are missing from every generated output, including the ODIN and YAML forms. _SPEC_PROPERTY = {"is_computed", "is_im_infrastructure", "is_im_runtime", "type_ref"} SPEC_ONLY_KEYS = { "schema": {"model_name", "schema_contributors", "archetype_parent_class", @@ -76,23 +78,29 @@ "P_BMM_CONTAINER_TYPE": {"index_type"}, "P_BMM_GENERIC_TYPE": {"value_constraint"}, } +HASH_HINT = "write Hash as P_BMM_GENERIC_PROPERTY with root_type Hash and two generic_parameters" +# Spec-only keys whose loss changes what the tables show: ERROR rather than WARNING. +LOSSY_KEYS = {("P_BMM_CONTAINER_TYPE", "index_type"): "the index is dropped, so a Hash renders as its " + "container_type; " + HASH_HINT} -HEADER_REQUIRED = ["bmm_version", "rm_publisher", "schema_name", "rm_release", "schema_revision", +HEADER_REQUIRED = ["rm_publisher", "schema_name", "rm_release", "schema_revision", "schema_lifecycle_state", "schema_description", "schema_author"] -CLASS_TYPES = {"P_BMM_CLASS", "P_BMM_INTERFACE", "P_BMM_ENUMERATION_STRING", "P_BMM_ENUMERATION_INTEGER"} -PROPERTY_TYPES = {"P_BMM_SINGLE_PROPERTY", "P_BMM_SINGLE_PROPERTY_OPEN", "P_BMM_CONTAINER_PROPERTY", - "P_BMM_GENERIC_PROPERTY"} -TYPE_TYPES = {"P_BMM_SIMPLE_TYPE", "P_BMM_CONTAINER_TYPE", "P_BMM_GENERIC_TYPE"} -PARAMETER_TYPES = {"P_BMM_SINGLE_FUNCTION_PARAMETER", "P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN", - "P_BMM_CONTAINER_FUNCTION_PARAMETER", "P_BMM_GENERIC_FUNCTION_PARAMETER"} -HASH_HINT = "write Hash as P_BMM_GENERIC_PROPERTY with root_type Hash and two generic_parameters" +CLASS_TYPES = ["P_BMM_CLASS", "P_BMM_INTERFACE", "P_BMM_ENUMERATION_STRING", "P_BMM_ENUMERATION_INTEGER"] +PROPERTY_TYPES = ["P_BMM_SINGLE_PROPERTY", "P_BMM_SINGLE_PROPERTY_OPEN", "P_BMM_CONTAINER_PROPERTY", + "P_BMM_GENERIC_PROPERTY"] +TYPE_TYPES = ["P_BMM_SIMPLE_TYPE", "P_BMM_CONTAINER_TYPE", "P_BMM_GENERIC_TYPE"] +PARAMETER_TYPES = ["P_BMM_SINGLE_FUNCTION_PARAMETER", "P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN", + "P_BMM_CONTAINER_FUNCTION_PARAMETER", "P_BMM_GENERIC_FUNCTION_PARAMETER"] UNSUPPORTED_TYPES = { "P_BMM_INDEXED_CONTAINER_PROPERTY": HASH_HINT, "P_BMM_INDEXED_CONTAINER_TYPE": HASH_HINT, } +SIMPLE, CONTAINER, GENERIC = TYPE_TYPES +# bmm-publisher writes class tables for packages down to this depth (top-level package = 1) +MAX_PACKAGE_DEPTH = 4 # an AsciiDoc attribute reference such as {base_release}; bmm-publisher escapes it, so it never resolves -ATTRIBUTE_REF = re.compile(r"\{[a-z][a-z0-9_-]*\}") -# result type of a procedure; bmm-publisher prints it without a link +ATTRIBUTE_REF = re.compile(r"\{[a-z][a-z0-9_]*\}") +# result type of a procedure in the published schemas; bmm-publisher prints it without a link PROCEDURE_RESULT = "void" GENERIC_NAME = re.compile(r"[<>,]") @@ -102,18 +110,27 @@ def __init__(self, schema_path, dependency_paths=()): self.path = Path(schema_path) self.findings = [] # (level, path, message) self.data = load(self.path) - self.dependencies = {} # schema id -> {class name: summary}, see summarise() + self.classes = {} # class name -> summary (see summarise), for this schema + self.class_paths = {} # class name -> JSON path of its definition + self.unchecked = set() # type names an included but unloaded schema might define + self.missing_includes = set() + self.dependencies = {} # schema id -> {class name: summary} own_id = schema_id(self.data) if isinstance(self.data, dict) else None for dep in dependency_paths: dep_data = load(Path(dep)) - if not isinstance(dep_data, dict): - continue - if schema_id(dep_data) == own_id: + if not isinstance(dep_data, dict) or not all( + isinstance(dep_data.get(k), str) for k in ("rm_publisher", "schema_name", "rm_release")): + raise SystemExit(fail("%s is not a BMM schema (it needs rm_publisher, schema_name and " + "rm_release)" % dep)) + dep_id = schema_id(dep_data) + if dep_id == own_id: self.warning("/", "-d %s is the schema being checked; it is ignored" % dep) - continue - self.dependencies[schema_id(dep_data)] = class_index(dep_data) - self.classes = {} # class name -> summary, for this schema - self.unchecked = set() # type names an included but unloaded schema might define + elif dep_id in self.dependencies: + self.warning("/", "-d %s has the same schema id as another -d file (%s); their classes " + "are merged" % (dep, dep_id)) + self.dependencies[dep_id].update(class_index(dep_data)) + else: + self.dependencies[dep_id] = class_index(dep_data) # --- reporting ------------------------------------------------------------------------- def error(self, path, message): @@ -137,31 +154,39 @@ def run(self): self.check_header(d) self.collect_classes(d) listed = self.check_packages(d) - for section in ("primitive_types", "class_definitions"): - for name, cls in (d.get(section) or {}).items(): - path = "/%s/%s" % (section, name) - if isinstance(cls, dict): - self.check_class(cls, path) - if cls.get("name") not in listed and cls.get("name") is not None: - self.error(path, "class is not listed in any package, so no class table is " - "generated for it") + for section, name, cls in self.class_blocks(d): + path = "/%s/%s" % (section, name) + self.check_class(cls, path) + cls_name = cls.get("name") + if isinstance(cls_name, str) and cls_name not in listed: + self.error(path, "class is not listed in any package, so no class table is generated for it") self.check_ancestor_cycles(d) if self.unchecked: - self.info("/includes", "not checked, as no loaded schema defines them (pass the included " - "schema with -d): " + ", ".join(sorted(self.unchecked))) + self.info("/includes", "not checked, as no loaded schema defines them; pass each included " + "schema with -d: " + ", ".join(sorted(self.unchecked))) return self + def class_blocks(self, d): + """(section, key, class) for every class definition that is an object.""" + for section in ("primitive_types", "class_definitions"): + block = d.get(section) + if isinstance(block, dict): + for key, cls in block.items(): + if isinstance(cls, dict): + yield section, key, cls + # --- header ---------------------------------------------------------------------------- def check_header(self, d): + if "bmm_version" not in d: + self.warning("/bmm_version", "missing; bmm-publisher assumes \"2.4\"") + elif d["bmm_version"] != "2.4": + self.warning("/bmm_version", "the published openEHR schemas use \"2.4\"") for key in HEADER_REQUIRED: if key not in d: - self.error("/" + key, "required header field is missing") + self.error("/" + key, "required header field is missing; bmm-publisher refuses the file") elif not isinstance(d[key], str): self.error("/" + key, "must be a string") self.check_keys(d, "schema", "") - if d.get("bmm_version") not in (None, "2.4"): - self.warning("/bmm_version", "published openEHR schemas use \"2.4\", the version " - "bmm-publisher reads and writes") release = d.get("rm_release") if isinstance(release, str): if not re.fullmatch(r"\d+\.\d+\.\d+", release): @@ -175,31 +200,27 @@ def check_header(self, d): if self.path.name != expected: self.warning("/", "file name %s does not match the schema id; expected %s" % (self.path.name, expected)) + included = set() includes = d.get("includes") - if includes is not None: - if not isinstance(includes, dict): - self.error("/includes", "must be an object keyed by schema id") - else: - for key, inc in includes.items(): - path = "/includes/" + key - if not isinstance(inc, dict) or not isinstance(inc.get("id"), str): - self.error(path, "each include needs an \"id\" naming the included schema") - continue - self.check_keys(inc, "include", path) - if inc["id"] != key: - self.warning(path, "published schemas key each include by its id (%s)" % inc["id"]) - included = set(i.get("id") for i in (includes or {}).values() if isinstance(i, dict)) \ - if isinstance(includes, dict) else set() + if includes is not None and not isinstance(includes, dict): + self.error("/includes", "must be an object keyed by schema id") + elif includes: + for key, inc in includes.items(): + path = "/includes/" + key + if not isinstance(inc, dict) or not isinstance(inc.get("id"), str): + self.error(path, "each include needs an \"id\" naming the included schema") + continue + self.check_keys(inc, "include", path) + included.add(inc["id"]) + if inc["id"] != key: + self.warning(path, "published schemas key each include by its id (%s)" % inc["id"]) for dep_id in self.dependencies: if dep_id not in included: self.warning("/includes", "schema %s was loaded with -d but is not included" % dep_id) self.missing_includes = included - set(self.dependencies) - if "class_definitions" not in d: - self.warning("/class_definitions", "the persistence model makes class_definitions mandatory") # --- classes and packages ---------------------------------------------------------------- def collect_classes(self, d): - seen = {} for section in ("primitive_types", "class_definitions"): block = d.get(section) if block is None: @@ -218,27 +239,29 @@ def collect_classes(self, d): continue if name != key: self.error(path, "key and \"name\" (%s) differ; bmm-publisher uses the name" % name) - if name in seen: - self.error(path, "class %s is also defined at %s" % (name, seen[name])) - seen[name] = path + if name in self.class_paths: + self.error(path, "class %s is also defined at %s" % (name, self.class_paths[name])) + self.class_paths[name] = path self.classes[name] = summarise(cls) for dep_id, dep_classes in self.dependencies.items(): - for name in set(self.classes) & set(dep_classes): - self.warning("/", "class %s is also defined in %s" % (name, dep_id)) + for name in sorted(set(self.classes) & set(dep_classes)): + self.warning(self.class_paths[name], "class %s is also defined in %s" % (name, dep_id)) def check_packages(self, d): listed = {} packages = d.get("packages") if not isinstance(packages, dict) or not packages: - self.error("/packages", "the schema needs at least one package") + self.error("/packages", "must be an object keyed by package name, with at least one package") return listed publisher, schema_name = d.get("rm_publisher"), d.get("schema_name") for key, pkg in packages.items(): path = "/packages/" + key if publisher == "openehr" and isinstance(schema_name, str) and isinstance(pkg, dict): root = "org.openehr." + schema_name.lower() - name = str(pkg.get("name", "")) - if name != root and not name.startswith(root + "."): + name = pkg.get("name") + if not isinstance(name, str): + pass # reported by check_package + elif name != root and not name.startswith(root + "."): self.warning(path, "class table file names are built from %s.; name the " "top-level package %s., or %s with sub-packages" % (root, root, root)) @@ -246,10 +269,10 @@ def check_packages(self, d): self.warning(path + "/classes", "classes listed directly in %s get table files named " "%s.org..adoc; list them in a sub-package" % (root, root)) - self.check_package(pkg, key, path, listed, top=True) + self.check_package(pkg, key, path, listed, depth=1) return listed - def check_package(self, pkg, key, path, listed, top): + def check_package(self, pkg, key, path, listed, depth): if not isinstance(pkg, dict): self.error(path, "a package must be an object") return @@ -259,7 +282,7 @@ def check_package(self, pkg, key, path, listed, top): self.error(path, "package has no \"name\"") elif name != key: self.error(path, "key and \"name\" (%s) differ" % name) - elif not top and "." in name: + elif depth > 1 and "." in name: self.error(path, "only top-level package names may contain '.'") classes = pkg.get("classes", []) subpackages = pkg.get("packages", {}) @@ -271,6 +294,9 @@ def check_package(self, pkg, key, path, listed, top): subpackages = {} if not classes and not subpackages: self.warning(path, "empty package") + if classes and depth > MAX_PACKAGE_DEPTH: + self.error(path, "bmm-publisher visits packages only %d levels deep, so these classes get no " + "class table" % MAX_PACKAGE_DEPTH) for cls in classes: if cls not in self.classes: self.error(path + "/classes", "%s is not defined in this schema; bmm-publisher stops " @@ -280,42 +306,39 @@ def check_package(self, pkg, key, path, listed, top): else: listed[cls] = path for sub_key, sub in subpackages.items(): - self.check_package(sub, sub_key, path + "/packages/" + sub_key, listed, top=False) + self.check_package(sub, sub_key, path + "/packages/" + sub_key, listed, depth + 1) def check_class(self, cls, path): - kind = cls.get("_type", "P_BMM_CLASS") - if kind not in CLASS_TYPES: - self.error(path, "unknown _type %s, read as a plain class" % kind) - kind = "P_BMM_CLASS" + kind = self.kind_of(cls, path, CLASS_TYPES, "P_BMM_CLASS", "a plain class") self.check_keys(cls, kind, path) self.check_documentation(cls, path, required=True) - scope = set() + scope = {} # generic parameter name -> its conforms_to_type, or None params = cls.get("generic_parameter_defs") - if params is not None: - if not isinstance(params, dict): - self.error(path + "/generic_parameter_defs", "must be an object keyed by parameter name") - else: - for key, param in params.items(): - p_path = path + "/generic_parameter_defs/" + key - if not isinstance(param, dict) or param.get("name") != key: - self.error(p_path, "needs a \"name\" equal to its key") - continue - self.check_keys(param, "generic_parameter", p_path) - scope.add(key) - for key, param in params.items(): - if isinstance(param, dict) and "conforms_to_type" in param: - self.check_type_name(param["conforms_to_type"], path + "/generic_parameter_defs/" - + key + "/conforms_to_type", scope) + if params is not None and not isinstance(params, dict): + self.error(path + "/generic_parameter_defs", "must be an object keyed by parameter name") + elif params: + for key, param in params.items(): + p_path = path + "/generic_parameter_defs/" + key + if not isinstance(param, dict) or param.get("name") != key: + self.error(p_path, "needs a \"name\" equal to its key") + continue + self.check_keys(param, "generic_parameter", p_path) + constraint = param.get("conforms_to_type") + if constraint is not None and not isinstance(constraint, str): + self.error(p_path + "/conforms_to_type", "must be a class name") + constraint = None + scope[key] = constraint + for key, constraint in scope.items(): + if constraint is not None: + self.check_type_name(constraint, path + "/generic_parameter_defs/" + key + + "/conforms_to_type", scope) if "is_abstract" in cls and not isinstance(cls["is_abstract"], bool): self.error(path + "/is_abstract", "must be true or false") ancestors = cls.get("ancestors", []) - if "ancestor_defs" in cls and not ancestors: - self.error(path, "bmm-publisher reads only \"ancestors\", so this class has no parent; list the " - "root class name there (for an open binding such as A, also redeclare T in " - "generic_parameter_defs; state a closed binding in documentation)") if not isinstance(ancestors, list) or not all(isinstance(a, str) for a in ancestors): self.error(path + "/ancestors", "must be a list of class names") ancestors = [] + self.check_ancestor_defs(cls, ancestors, path) for i, ancestor in enumerate(ancestors): a_path = "%s/ancestors/%d" % (path, i) if GENERIC_NAME.search(ancestor): @@ -323,14 +346,15 @@ def check_class(self, cls, path): "root class (%s) and declare the parameters in generic_parameter_defs" % ancestor.split("<")[0].strip()) else: - self.check_type_name(ancestor, a_path, set()) + self.check_type_name(ancestor, a_path, {}) if kind.startswith("P_BMM_ENUMERATION"): - self.check_enumeration(cls, kind, path) + self.check_enumeration(cls, kind, ancestors if "ancestors" in cls else None, path) for key, constant in self.keyed(cls, "constants", path): c_path = path + "/constants/" + key + self.check_name(constant, c_path) self.check_keys(constant, "constant", c_path) if not isinstance(constant.get("type"), str): - self.error(c_path, "a constant needs a \"type\"") + self.error(c_path, "a constant needs a \"type\"; bmm-publisher fails without one") else: self.check_type_name(constant["type"], c_path + "/type", scope) self.check_documentation(constant, c_path) @@ -340,39 +364,63 @@ def check_class(self, cls, path): self.check_function(function, path + "/functions/" + key, scope) self.check_assertions(cls, "invariants", path) - def check_enumeration(self, cls, kind, path): + def check_ancestor_defs(self, cls, ancestors, path): + """bmm-publisher reads only ancestors: a parent named only in ancestor_defs is lost.""" + defs = cls.get("ancestor_defs") + if defs is None: + return + entries = list(defs.items()) if isinstance(defs, dict) else list(enumerate(defs)) \ + if isinstance(defs, list) else [] + roots = [] + for key, entry in entries: + if isinstance(entry, dict) and isinstance(entry.get("root_type") or entry.get("type"), str): + roots.append(entry.get("root_type") or entry.get("type")) + elif isinstance(key, str): + roots.append(key.split("<")[0].strip()) + lost = [r for r in roots if r not in ancestors] + if not ancestors or lost: + self.error(path + "/ancestor_defs", "bmm-publisher reads only \"ancestors\", so %s lost; list " + "the root class name there (for an open binding such as " + "A, also redeclare T in generic_parameter_defs; state a " + "closed binding in documentation)" + % ("the parent %s is" % ", ".join(lost) if lost else "every parent is")) + + def check_enumeration(self, cls, kind, ancestors, path): names = cls.get("item_names") if not isinstance(names, list) or not names or not all(isinstance(n, str) for n in names): self.error(path + "/item_names", "an enumeration needs a non-empty list of item names") return - base = "String" if kind == "P_BMM_ENUMERATION_STRING" else "Integer" - if base not in (cls.get("ancestors") or []): - self.warning(path + "/ancestors", "the persistence model requires %s among the ancestors" % base) + integer = kind == "P_BMM_ENUMERATION_INTEGER" + if integer and ancestors is not None and "Integer" not in ancestors: + self.warning(path + "/ancestors", "the table prints item values only when Integer is among " + "the ancestors") values = cls.get("item_values") if values is not None: - value_type = str if base == "String" else int + value_type = int if integer else str if not isinstance(values, list) or len(values) != len(names): self.error(path + "/item_values", "needs one value per item name (%d)" % len(names)) elif not all(isinstance(v, value_type) and not isinstance(v, bool) for v in values): - self.error(path + "/item_values", "values must be %ss" % base) + self.error(path + "/item_values", "values must be %s" % ("integers" if integer else "strings")) docs = cls.get("item_documentations") - if docs is not None and (not isinstance(docs, list) or len(docs) != len(names)): + if docs is not None and (not isinstance(docs, list) or len(docs) != len(names) + or not all(isinstance(t, str) for t in docs)): self.error(path + "/item_documentations", "needs one text per item name (%d); the " "table pairs them by position" % len(names)) def check_ancestor_cycles(self, d): graph = {} - for section in ("primitive_types", "class_definitions"): - for cls in (d.get(section) or {}).values(): - if isinstance(cls, dict) and isinstance(cls.get("name"), str): - graph[cls["name"]] = [a for a in cls.get("ancestors") or [] if isinstance(a, str)] + for _, _, cls in self.class_blocks(d): + ancestors = cls.get("ancestors") + if isinstance(cls.get("name"), str): + graph[cls["name"]] = [a for a in ancestors if isinstance(a, str)] \ + if isinstance(ancestors, list) else [] state = {} def visit(name, trail): state[name] = 1 for ancestor in graph.get(name, []): if state.get(ancestor) == 1: - self.error("/class_definitions/" + name, "inheritance cycle: " + self.error(self.class_paths.get(name, "/"), "inheritance cycle: " + " -> ".join(trail + [ancestor])) elif ancestor in graph and not state.get(ancestor): visit(ancestor, trail + [ancestor]) @@ -384,24 +432,23 @@ def visit(name, trail): # --- properties, functions and types ----------------------------------------------------- def check_property(self, prop, path, scope): - if not isinstance(prop, dict): - self.error(path, "a property must be an object") - return kind = prop.get("_type") if kind is None: if "type_def" in prop: self.error(path, "no _type: read as P_BMM_SINGLE_PROPERTY, whose type then defaults " "to Any; set the property _type") - return - self.warning(path, "no _type: read as P_BMM_SINGLE_PROPERTY; published schemas always set it") - kind = "P_BMM_SINGLE_PROPERTY" - elif kind in UNSUPPORTED_TYPES: + kind = self.closest_kind_from_shape(prop) + else: + self.warning(path, "no _type: read as P_BMM_SINGLE_PROPERTY; published schemas always set it") + kind = "P_BMM_SINGLE_PROPERTY" + elif isinstance(kind, str) and kind in UNSUPPORTED_TYPES: self.error(path, "bmm-publisher does not read %s (it becomes a property of type Any); %s" % (kind, UNSUPPORTED_TYPES[kind])) + self.check_name(prop, path) + self.check_documentation(prop, path, required=True) return - elif kind not in PROPERTY_TYPES: - self.error(path, "unknown _type %s: read as P_BMM_SINGLE_PROPERTY" % kind) - return + else: + kind = self.kind_of(prop, path, PROPERTY_TYPES, "P_BMM_SINGLE_PROPERTY", "P_BMM_SINGLE_PROPERTY") self.check_name(prop, path) self.check_keys(prop, kind, path) self.check_documentation(prop, path, required=True) @@ -423,134 +470,183 @@ def check_property(self, prop, path, scope): else: self.check_type_name(target, path + "/type", scope) elif kind == "P_BMM_CONTAINER_PROPERTY": - self.check_inline_type(prop, "P_BMM_CONTAINER_TYPE", path, scope) + self.check_inline_type(prop, CONTAINER, path, scope) if "cardinality" in prop: self.check_cardinality(prop["cardinality"], path + "/cardinality") else: - self.check_inline_type(prop, "P_BMM_GENERIC_TYPE", path, scope) + self.check_inline_type(prop, GENERIC, path, scope) + + def closest_kind_from_shape(self, prop): + type_def = prop.get("type_def") + if isinstance(type_def, dict) and "root_type" in type_def: + return "P_BMM_GENERIC_PROPERTY" + return "P_BMM_CONTAINER_PROPERTY" def check_inline_type(self, owner, kind, path, scope): type_def = owner.get("type_def") if not isinstance(type_def, dict): self.error(path, "needs a \"type_def\" object") return - if type_def.get("_type", kind) != kind: + marker = type_def.get("_type", kind) + if marker != kind: self.error(path + "/type_def", "_type %s is ignored here: this type_def is always read as %s" - % (type_def["_type"], kind)) + % (marker if isinstance(marker, str) else "(not a string)", kind)) self.check_type_body(type_def, kind, path + "/type_def", scope) def check_type(self, t, path, scope): - """A nested type object (result, generic_parameter_defs entry, element type_def).""" + """A nested type object (result, generic parameter, element type_def). Returns the kind + it is checked as, or None when it is not an object or bmm-publisher cannot read it.""" if not isinstance(t, dict): self.error(path, "a type must be an object") - return + return None kind = t.get("_type") if kind is None: if "container_type" in t or "root_type" in t: self.error(path, "no _type: a nested type is read as P_BMM_SIMPLE_TYPE; set _type") - return - self.warning(path, "no _type: read as P_BMM_SIMPLE_TYPE; published schemas always set it") - kind = "P_BMM_SIMPLE_TYPE" - elif kind in UNSUPPORTED_TYPES: - self.error(path, "bmm-publisher does not read %s; %s" % (kind, UNSUPPORTED_TYPES[kind])) - return + kind = GENERIC if "root_type" in t else CONTAINER + else: + self.warning(path, "no _type: read as P_BMM_SIMPLE_TYPE; published schemas always set it") + kind = SIMPLE + elif isinstance(kind, str) and kind in UNSUPPORTED_TYPES: + self.error(path, "bmm-publisher does not read %s (it becomes a simple type); %s" + % (kind, UNSUPPORTED_TYPES[kind])) + return None elif kind == "P_BMM_OPEN_TYPE": self.warning(path, "read as P_BMM_SIMPLE_TYPE; published schemas write P_BMM_SIMPLE_TYPE") - kind = "P_BMM_SIMPLE_TYPE" - elif kind not in TYPE_TYPES: - self.error(path, "unknown _type %s: read as P_BMM_SIMPLE_TYPE" % kind) - return + kind = SIMPLE + else: + kind = self.kind_of(t, path, TYPE_TYPES, SIMPLE, SIMPLE) self.check_type_body(t, kind, path, scope) + return kind def check_type_body(self, t, kind, path, scope): self.check_keys(t, kind, path) - if kind == "P_BMM_SIMPLE_TYPE": - if not isinstance(t.get("type"), str): + if kind == SIMPLE: + target = t.get("type") + if not isinstance(target, str): self.error(path, "needs a \"type\"") - elif GENERIC_NAME.search(t["type"]): + elif GENERIC_NAME.search(target): self.error(path + "/type", "%s is not a class name; use P_BMM_CONTAINER_TYPE or " - "P_BMM_GENERIC_TYPE" % t["type"]) - else: - self.check_type_name(t["type"], path + "/type", scope) - elif kind == "P_BMM_CONTAINER_TYPE": - container = t.get("container_type") - if not isinstance(container, str): - self.error(path, "needs a \"container_type\" (List, Set, Array)") + "P_BMM_GENERIC_TYPE" % target) else: - self.check_type_name(container, path + "/container_type", scope) - arity = self.arity(container) - if arity is not None and arity != 1: - self.error(path + "/container_type", "%s takes %d generic parameters; %s" - % (container, arity, HASH_HINT)) - has_type, has_def = "type" in t, "type_def" in t - if has_type == has_def: - self.error(path, "give the element type as either \"type\" (a name) or \"type_def\" " - "(a nested type), not %s" % ("both" if has_type else "neither: it is read as Any")) - elif has_type: - if not isinstance(t["type"], str) or GENERIC_NAME.search(t["type"]): - self.error(path + "/type", "must be a class or parameter name; nest other types " - "in \"type_def\"") - else: - self.check_type_name(t["type"], path + "/type", scope) + self.check_type_name(target, path + "/type", scope) + elif kind == CONTAINER: + self.check_container_type(t, path, scope) + else: + self.check_generic_type(t, path, scope) + + def check_container_type(self, t, path, scope): + container = t.get("container_type") + if not isinstance(container, str): + self.error(path, "needs a \"container_type\" (List, Set, Array)") + else: + self.check_type_name(container, path + "/container_type", scope) + arity = self.arity(container) + if arity is not None and arity != 1: + self.error(path + "/container_type", "%s takes %d generic parameters; %s" + % (container, arity, HASH_HINT)) + has_type, has_def = "type" in t, "type_def" in t + if has_type == has_def: + self.error(path, "give the element type as either \"type\" (a name) or \"type_def\" " + "(a nested type), not %s" % ("both" if has_type else "neither: it is read as Any")) + elif has_type: + if not isinstance(t["type"], str) or GENERIC_NAME.search(t["type"]): + self.error(path + "/type", "must be a class or parameter name; nest other types " + "in \"type_def\"") else: - self.check_type(t["type_def"], path + "/type_def", scope) + self.check_type_name(t["type"], path + "/type", scope) + elif self.check_type(t["type_def"], path + "/type_def", scope) == SIMPLE: + self.error(path + "/type_def", "a simple element type goes in \"type\": bmm-publisher renders " + "a simple type in \"type_def\" as Any") + + def check_generic_type(self, t, path, scope): + root = t.get("root_type") + if not isinstance(root, str): + self.error(path, "needs a \"root_type\"") else: - root = t.get("root_type") - if not isinstance(root, str): - self.error(path, "needs a \"root_type\"") - return self.check_type_name(root, path + "/root_type", scope) - simple = t.get("generic_parameters", []) - nested = t.get("generic_parameter_defs", {}) - if not isinstance(simple, list) or not isinstance(nested, dict): - self.error(path, "generic_parameters is a list of names and generic_parameter_defs an " - "object of nested types") - return - for i, param in enumerate(simple): - if isinstance(param, dict): - self.check_type(param, "%s/generic_parameters/%d" % (path, i), scope) - elif not isinstance(param, str) or GENERIC_NAME.search(param): - self.error("%s/generic_parameters/%d" % (path, i), "must be a class or parameter " - "name; nest other types in " - "generic_parameter_defs") - else: - self.check_type_name(param, "%s/generic_parameters/%d" % (path, i), scope) + simple = t.get("generic_parameters") + nested = t.get("generic_parameter_defs") + if simple is not None and not isinstance(simple, list): + self.error(path + "/generic_parameters", "must be a list of class or parameter names") + simple = None + if nested is not None and not isinstance(nested, dict): + self.error(path + "/generic_parameter_defs", "must be an object of nested types keyed by " + "parameter name") + nested = None + params = self.lookup(root)["params"] if isinstance(root, str) and self.lookup(root) else None + args = [] # (argument class or parameter name, or None; JSON path), in binding order + if simple and nested: + self.error(path, "bmm-publisher uses generic_parameters and ignores generic_parameter_defs " + "when both are given; use one of them") for key, param in nested.items(): self.check_type(param, path + "/generic_parameter_defs/" + key, scope) - given = len(simple) + len(nested) - arity = self.arity(root) - if given == 0: - self.error(path, "a generic type needs generic_parameters or generic_parameter_defs") - elif arity is not None and arity != given: - self.error(path, "%s takes %d generic parameters, %d given" % (root, arity, given)) - elif arity: - self.check_generic_arguments(root, simple, nested, path, scope) - - def check_generic_arguments(self, root, simple, nested, path, scope): - """Each argument must conform to the conforms_to_type of the parameter it binds.""" - params = self.lookup(root)["params"] - bound = [] # (parameter, constraint, argument class name, path) - if not nested: - for i, ((param, constraint), arg) in enumerate(zip(params, simple)): - if isinstance(arg, str): - bound.append((param, constraint, arg, "%s/generic_parameters/%d" % (path, i))) - else: - constraints = dict(params) - for key, arg in nested.items(): - if key in constraints and isinstance(arg, dict): - name = arg.get("type") or arg.get("root_type") - if isinstance(name, str): - bound.append((key, constraints[key], name, path + "/generic_parameter_defs/" + key)) - for param, constraint, arg, arg_path in bound: - if constraint and arg not in scope and self.conforms(arg, constraint) is False: + if simple: + for i, param in enumerate(simple): + args.append(self.check_simple_argument(param, "%s/generic_parameters/%d" % (path, i), scope)) + elif nested: + for i, (key, param) in enumerate(nested.items()): + p_path = path + "/generic_parameter_defs/" + key + args.append(self.check_nested_argument(param, p_path, scope)) + if params is not None and i < len(params) and key != params[i][0]: + self.warning(p_path, "generic_parameter_defs bind by position: entry %d is parameter %s " + "of %s" % (i + 1, params[i][0], root)) + if not args: + self.error(path, "a generic type needs generic_parameters or generic_parameter_defs") + elif params is not None and len(params) != len(args): + self.error(path, "%s takes %d generic parameters, %d given" % (root, len(params), len(args))) + elif params: + self.check_generic_arguments(root, params, args, scope) + + def check_simple_argument(self, param, path, scope): + """An entry of generic_parameters: returns (argument name or None, path).""" + if isinstance(param, str): + if GENERIC_NAME.search(param): + self.error(path, "must be a class or parameter name; nest other types in " + "generic_parameter_defs") + return None, path + self.check_type_name(param, path, scope) + return param, path + if isinstance(param, dict): + # BASE writes nested generic types here (FUNCTION, Boolean>), and bmm-publisher reads them + kind = self.check_type(param, path, scope) + if kind == GENERIC: + return param.get("root_type"), path + if kind is not None: + self.error(path, "bmm-publisher accepts only names and generic types here and stops with " + "a type error on a %s" % kind) + return None, path + self.error(path, "must be a class or parameter name") + return None, path + + def check_nested_argument(self, param, path, scope): + """An entry of generic_parameter_defs: returns (argument name or None, path).""" + kind = self.check_type(param, path, scope) + if kind == CONTAINER: + self.error(path, "bmm-publisher prints nothing for a container type in generic_parameter_defs; " + "use a generic type with root_type List instead") + return param.get("container_type"), path + if kind == GENERIC: + return param.get("root_type"), path + if kind == SIMPLE: + return param.get("type"), path + return None, path + + def check_generic_arguments(self, root, params, args, scope): + """Each argument, bound by position, must conform to its parameter's conforms_to_type.""" + names = ", ".join(p for p, _ in params) + for (param, constraint), (arg, arg_path) in zip(params, args): + if not constraint or constraint == "Any" or not isinstance(arg, str): + continue + # an argument that is a parameter of the enclosing class conforms through its own constraint + effective = (scope[arg] or "Any") if arg in scope else arg + if self.conforms(effective, constraint) is False: + what = arg if effective == arg else "%s (constrained to %s)" % (arg, effective) self.error(arg_path, "%s does not conform to %s, the constraint on parameter %s of %s " - "(generic_parameters bind in the order %s declares them: %s)" - % (arg, constraint, param, root, root, ", ".join(p for p, _ in params))) + "(arguments bind in the order %s declares its parameters: %s)" + % (what, constraint, param, root, root, names)) def check_function(self, function, path, scope): - if not isinstance(function, dict): - self.error(path, "a function must be an object") - return self.check_name(function, path) self.check_keys(function, "function", path) self.check_documentation(function, path, required=True) @@ -559,11 +655,14 @@ def check_function(self, function, path, scope): self.error(path + "/" + flag, "must be true or false") aliases = function.get("aliases", []) if not isinstance(aliases, list) or not all(isinstance(a, str) for a in aliases): - self.error(path + "/aliases", "must be a list of strings") - if "result" in function: - self.check_type(function["result"], path + "/result", scope) - else: - self.warning(path, "no result: the table shows an empty result type") + self.error(path + "/aliases", "must be a list of names, as the published schemas write it") + result = function.get("result") + if result is None: + self.warning(path, "no result: the specification reads that as a procedure, but the table " + "prints an empty result type; published schemas write the result type void") + elif not (isinstance(result, dict) and result.get("type") == PROCEDURE_RESULT + and result.get("_type", SIMPLE) == SIMPLE): + self.check_type(result, path + "/result", scope) for key, param in self.keyed(function, "parameters", path): self.check_parameter(param, path + "/parameters/" + key, scope) self.check_assertions(function, "pre_conditions", path) @@ -574,11 +673,14 @@ def check_parameter(self, param, path, scope): if kind is None: if "type_def" in param: self.error(path, "no _type: read as P_BMM_SINGLE_FUNCTION_PARAMETER of type Any") - return - kind = "P_BMM_SINGLE_FUNCTION_PARAMETER" - elif kind not in PARAMETER_TYPES: - self.error(path, "unknown _type %s: read as P_BMM_SINGLE_FUNCTION_PARAMETER" % kind) - return + type_def = param.get("type_def") + kind = "P_BMM_GENERIC_FUNCTION_PARAMETER" if isinstance(type_def, dict) and "root_type" in type_def \ + else "P_BMM_CONTAINER_FUNCTION_PARAMETER" + else: + kind = "P_BMM_SINGLE_FUNCTION_PARAMETER" + else: + kind = self.kind_of(param, path, PARAMETER_TYPES, "P_BMM_SINGLE_FUNCTION_PARAMETER", + "P_BMM_SINGLE_FUNCTION_PARAMETER") self.check_name(param, path) self.check_keys(param, kind, path) self.check_documentation(param, path) @@ -593,14 +695,11 @@ def check_parameter(self, param, path, scope): else: self.check_type_name(target, path + "/type", scope) elif kind == "P_BMM_CONTAINER_FUNCTION_PARAMETER": - self.check_inline_type(param, "P_BMM_CONTAINER_TYPE", path, scope) - if "cardinality" not in param: - self.error(path, "a container parameter needs a \"cardinality\"; bmm-publisher fails " - "without one") - else: + self.check_inline_type(param, CONTAINER, path, scope) + if "cardinality" in param: self.check_cardinality(param["cardinality"], path + "/cardinality") else: - self.check_inline_type(param, "P_BMM_GENERIC_TYPE", path, scope) + self.check_inline_type(param, GENERIC, path, scope) def check_cardinality(self, card, path): if not isinstance(card, dict): @@ -618,12 +717,28 @@ def check_cardinality(self, card, path): "missing upper_unbounded as true and ignores the limit") if card.get("upper_unbounded") is False and "upper" not in card: self.error(path, "\"upper_unbounded\": false needs an \"upper\" limit") - if isinstance(card.get("upper"), int) and isinstance(card.get("lower"), int) and card["upper"] < card["lower"]: + lower, upper = card.get("lower"), card.get("upper") + if is_count(lower) and is_count(upper) and upper < lower: self.error(path, "upper is below lower") - if card.get("lower_unbounded"): + if card.get("lower_unbounded") is True: self.warning(path + "/lower_unbounded", "a cardinality has a lower limit (0 or more)") # --- shared helpers ------------------------------------------------------------------------ + def kind_of(self, item, path, kinds, read_as, read_as_label): + """The _type an item is checked as. An unknown _type is reported with what bmm-publisher reads + it as, and the item is then checked as the closest known _type, so its content is still checked.""" + kind = item.get("_type", kinds[0]) + if isinstance(kind, str) and kind in kinds: + return kind + if not isinstance(kind, str): + self.error(path + "/_type", "must be a string; read as %s" % read_as_label) + return read_as + close = difflib.get_close_matches(kind, kinds, n=1, cutoff=0.6) + checked = close[0] if close else read_as + self.error(path, "unknown _type %s: read as %s%s" % (kind, read_as_label, + "; checked below as " + checked if close else "")) + return checked + def keyed(self, owner, key, path): block = owner.get(key) if block is None: @@ -645,22 +760,33 @@ def check_name(self, item, path): if not isinstance(name, str): self.error(path, "has no \"name\"") elif name != key: - self.error(path, "key and \"name\" (%s) differ; bmm-publisher uses the name" % name) + self.error(path, "key and \"name\" (%s) differ; keep them equal (depending on the map, the " + "tables show one or both)" % name) def check_keys(self, item, kind, path): for key in item: if key in READ_KEYS[kind]: continue - near = difflib.get_close_matches(key, sorted(READ_KEYS[kind]), n=1, cutoff=0.8) + k_path = "%s/%s" % (path, key) if key in SPEC_ONLY_KEYS.get(kind, ()): - self.warning("%s/%s" % (path, key), "defined by the persistence model but not read by " - "bmm-publisher, so it is missing from every output") - elif near and near[0] not in item: - self.error("%s/%s" % (path, key), "unknown key, probably a misspelling of %s; bmm-publisher " - "ignores it, so %s keeps its default" % (near[0], near[0])) + if (kind, key) in LOSSY_KEYS: + self.error(k_path, "not read by bmm-publisher: " + LOSSY_KEYS[(kind, key)]) + else: + self.warning(k_path, "known to the persistence specification but not read by " + "bmm-publisher, so it is missing from every output") + elif kind in CLASS_TYPES[1:] and key in READ_KEYS["P_BMM_CLASS"]: + self.error(k_path, "bmm-publisher does not read %s on a %s, so it is lost" % (key, kind)) + elif self.misspelling(key, kind, item): + near = self.misspelling(key, kind, item) + self.error(k_path, "unknown key, probably a misspelling of %s; bmm-publisher ignores it, " + "so %s keeps its default" % (near, near)) else: - self.warning("%s/%s" % (path, key), "unknown key, ignored by bmm-publisher (expected " - "one of: %s)" % ", ".join(sorted(READ_KEYS[kind]))) + self.warning(k_path, "unknown key, ignored by bmm-publisher (expected one of: %s)" + % ", ".join(sorted(READ_KEYS[kind]))) + + def misspelling(self, key, kind, item): + near = difflib.get_close_matches(key, sorted(READ_KEYS[kind]), n=1, cutoff=0.8) + return near[0] if near and near[0] not in item else None def check_documentation(self, item, path, required=False): doc = item.get("documentation") @@ -709,6 +835,8 @@ def conforms(self, name, target): info = self.lookup(current) if info is None: undecided = True + if self.missing_includes: + self.unchecked.add(current) else: todo.extend(info["ancestors"]) return None if undecided else False @@ -717,11 +845,21 @@ def check_type_name(self, name, path, scope): if not isinstance(name, str) or not name: self.error(path, "must be a non-empty class name") return - if name in scope or name == PROCEDURE_RESULT or self.arity(name) is not None: + if name in scope or self.lookup(name) is not None: + return + if name == PROCEDURE_RESULT: + self.warning(path, "void is only the result type of a procedure") return if len(name) == 1: self.warning(path, "%s is not a generic parameter of this class; declare it in " "generic_parameter_defs" % name) + return + known = set(self.classes) + for dep_classes in self.dependencies.values(): + known.update(dep_classes) + near = difflib.get_close_matches(name, sorted(known), n=1, cutoff=0.85) + if near: + self.warning(path, "type %s is not defined; did you mean %s?" % (name, near[0])) elif self.missing_includes: self.unchecked.add(name) else: @@ -737,6 +875,8 @@ def load(path): except json.JSONDecodeError as exc: raise SystemExit(fail("%s is not valid JSON: %s (line %d, column %d)" % (path, exc.msg, exc.lineno, exc.colno))) + except UnicodeDecodeError: + raise SystemExit(fail("%s is not UTF-8 text" % path)) def fail(message): @@ -744,6 +884,10 @@ def fail(message): return 2 +def is_count(value): + return isinstance(value, int) and not isinstance(value, bool) + + def schema_id(d): return "%s_%s_%s" % (d.get("rm_publisher"), d.get("schema_name"), d.get("rm_release")) @@ -754,18 +898,21 @@ def summarise(cls): params = params if isinstance(params, dict) else {} ancestors = cls.get("ancestors") ancestors = ancestors if isinstance(ancestors, list) else [] - return { - "params": [(key, p.get("conforms_to_type") if isinstance(p, dict) else None) for key, p in params.items()], - "ancestors": [a for a in ancestors if isinstance(a, str)], - } + constraints = [] + for key, p in params.items(): + constraint = p.get("conforms_to_type") if isinstance(p, dict) else None + constraints.append((key, constraint if isinstance(constraint, str) else None)) + return {"params": constraints, "ancestors": [a for a in ancestors if isinstance(a, str)]} def class_index(d): classes = {} for section in ("primitive_types", "class_definitions"): - for cls in (d.get(section) or {}).values(): - if isinstance(cls, dict) and isinstance(cls.get("name"), str): - classes[cls["name"]] = summarise(cls) + block = d.get(section) + if isinstance(block, dict): + for cls in block.values(): + if isinstance(cls, dict) and isinstance(cls.get("name"), str): + classes[cls["name"]] = summarise(cls) return classes @@ -779,9 +926,16 @@ def main(argv=None): checker = Checker(args.schema, args.dependency).run() for level, path, message in checker.findings: print("%-7s %s: %s" % (level, path, message)) - errors, warnings = checker.count("ERROR"), checker.count("WARNING") - print("check_bmm: %s: %d error(s), %d warning(s)" % (args.schema, errors, warnings)) - return 1 if errors or (args.strict and warnings) else 0 + errors, warnings, unchecked = checker.count("ERROR"), checker.count("WARNING"), len(checker.unchecked) + print("check_bmm: %s: %d class(es), %d error(s), %d warning(s), %d name(s) not checked" + % (args.schema, len(checker.classes), errors, warnings, unchecked)) + if errors or (args.strict and warnings): + return 1 + if unchecked: + print("check_bmm: incomplete: pass each included schema with -d before relying on this result", + file=sys.stderr) + return 3 + return 0 if __name__ == "__main__": diff --git a/scripts/test_check_bmm.py b/scripts/test_check_bmm.py index 6d8a560..98d8aa0 100644 --- a/scripts/test_check_bmm.py +++ b/scripts/test_check_bmm.py @@ -22,25 +22,45 @@ _spec.loader.exec_module(check_bmm) EXAMPLE = json.loads((SKILL / "assets/openehr_demo_0.1.0.bmm.json").read_text(encoding="utf-8")) - -# The BASE types the example uses, as a minimal stand-in for openehr_base_1.3.0.bmm.json. +# The published BASE schema, when a sibling clone exists (it does not in CI). +REAL_BASE = next((p for p in (ROOT.parent / "specifications-BASE", ROOT.parent.parent.parent / "specifications-BASE") + if (p / "computable/BMM/openehr_base_1.3.0.bmm.json").is_file()), ROOT) \ + / "computable/BMM/openehr_base_1.3.0.bmm.json" + + +def base_class(name, ancestors=None, **params): + cls = {"name": name} + if ancestors: + cls["ancestors"] = ancestors + if params: + cls["generic_parameter_defs"] = { + p: dict({"name": p}, **({"conforms_to_type": c} if c else {})) for p, c in params.items()} + return cls + + +# A slice of openehr_base_1.3.0.bmm.json: the ancestry and generic constraints the tests rely on, +# copied from the published schema. +_BASE_CLASSES = [ + base_class("Any"), base_class("Ordered", ["Any"]), base_class("Numeric", ["Any"]), + base_class("Ordered_Numeric", ["Ordered", "Numeric"]), base_class("Integer", ["Ordered_Numeric"]), + base_class("Real", ["Ordered_Numeric"]), base_class("String", ["Ordered"]), + base_class("Boolean", ["Any"]), base_class("Temporal", ["Ordered"]), base_class("Time_Definitions"), + base_class("Iso8601_type", ["Temporal", "Time_Definitions"]), + base_class("Iso8601_date_time", ["Iso8601_type"]), + base_class("Container", ["Any"], T="Any"), base_class("List", ["Container"], T="Any"), + base_class("Set", ["Container"], T="Any"), base_class("Array", ["Container"], T="Any"), + base_class("Hash", ["Container"], K="Ordered", V=None), + base_class("Interval", ["Any"], T="Ordered"), base_class("TUPLE"), base_class("TUPLE2", ["TUPLE"], A="Any", B="Any"), + base_class("ROUTINE", None, ARGS="TUPLE"), base_class("FUNCTION", ["ROUTINE"], ARGS="TUPLE", RESULT="Any"), + base_class("OBJECT_ID"), base_class("UID_BASED_ID", ["OBJECT_ID"]), base_class("HIER_OBJECT_ID", ["UID_BASED_ID"]), +] BASE = { "bmm_version": "2.4", "rm_publisher": "openehr", "schema_name": "base", "rm_release": "1.3.0", "schema_revision": "1.3.0.2", "schema_lifecycle_state": "stable", "schema_description": "stub", "schema_author": "test", "packages": {"org.openehr.base.foundation_types": { - "name": "org.openehr.base.foundation_types", - "classes": ["Any", "Boolean", "Integer", "String", "Iso8601_date_time", "List", "Hash", "HIER_OBJECT_ID"]}}, - "primitive_types": { - "Any": {"name": "Any", "is_abstract": True}, - "Boolean": {"name": "Boolean"}, - "Integer": {"name": "Integer"}, - "String": {"name": "String"}, - "Iso8601_date_time": {"name": "Iso8601_date_time"}, - "List": {"name": "List", "generic_parameter_defs": {"T": {"name": "T"}}}, - "Hash": {"name": "Hash", "generic_parameter_defs": {"K": {"name": "K"}, "V": {"name": "V"}}}, - }, - "class_definitions": {"HIER_OBJECT_ID": {"name": "HIER_OBJECT_ID"}}, + "name": "org.openehr.base.foundation_types", "classes": [c["name"] for c in _BASE_CLASSES]}}, + "class_definitions": {c["name"]: c for c in _BASE_CLASSES}, } @@ -63,163 +83,378 @@ def check(self, data, name="openehr_demo_0.1.0.bmm.json", deps=True): def demo(self): return copy.deepcopy(EXAMPLE) - def messages(self, checker, level): - return ["%s: %s" % (p, m) for lvl, p, m in checker.findings if lvl == level] + def cls(self, d, name): + return d["class_definitions"][name] + + def props(self, d, cls="DEMO_GROUP"): + return d["class_definitions"][cls]["properties"] + + def fn(self, d, cls="DEMO_ITEM", name="is_tagged"): + return d["class_definitions"][cls]["functions"][name] + + def listing(self, checker): + return "\n".join("%s %s: %s" % f for f in checker.findings) def assertFinding(self, checker, level, path, text): - hits = [m for m in self.messages(checker, level) if m.startswith(path + ":") and text in m] - self.assertTrue(hits, "no %s at %s containing %r in:\n%s" - % (level, path, text, "\n".join(lvl + " " + p + ": " + m for lvl, p, m in checker.findings))) + hits = [f for f in checker.findings if f[0] == level and f[1] == path and text in f[2]] + self.assertTrue(hits, "no %s at %s containing %r in:\n%s" % (level, path, text, self.listing(checker))) + + def assertNoFinding(self, checker, level="ERROR"): + self.assertFalse([f for f in checker.findings if f[0] == level], self.listing(checker)) + + def assertOnly(self, checker, level, path, text): + """Exactly one finding, the expected one: no extra or duplicate reports.""" + self.assertFinding(checker, level, path, text) + self.assertEqual(len(checker.findings), 1, self.listing(checker)) class ExampleTest(Base): def test_example_is_clean_with_its_dependency(self): checker = self.check(self.demo()) self.assertEqual(checker.findings, []) + self.assertEqual(len(checker.classes), 5) def test_without_dependency_base_types_are_listed_as_unchecked(self): checker = self.check(self.demo(), deps=False) self.assertEqual([lvl for lvl, _, _ in checker.findings], ["INFO"]) self.assertIn("HIER_OBJECT_ID", checker.findings[0][2]) + self.assertIn("Hash", checker.unchecked) + + @unittest.skipUnless(REAL_BASE.is_file(), "needs a sibling specifications-BASE clone") + def test_example_is_clean_against_the_published_base(self): + path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + self.assertEqual(check_bmm.Checker(path, [REAL_BASE]).run().findings, []) class HeaderTest(Base): def test_missing_required_field(self): + for key in check_bmm.HEADER_REQUIRED: + with self.subTest(key=key): + d = self.demo() + del d[key] + self.assertFinding(self.check(d), "ERROR", "/" + key, "required header field") + + def test_missing_bmm_version_is_assumed(self): + d = self.demo() + del d["bmm_version"] + self.assertOnly(self.check(d), "WARNING", "/bmm_version", "assumes \"2.4\"") + + def test_other_bmm_version(self): + d = self.demo() + d["bmm_version"] = "2.3" + self.assertOnly(self.check(d), "WARNING", "/bmm_version", "use \"2.4\"") + + def test_header_field_must_be_a_string(self): d = self.demo() - del d["schema_author"] - self.assertFinding(self.check(d), "ERROR", "/schema_author", "required header field") + d["schema_author"] = ["someone"] + self.assertFinding(self.check(d), "ERROR", "/schema_author", "must be a string") def test_file_name_must_match_schema_id(self): - self.assertFinding(self.check(self.demo(), name="demo.bmm.json"), "WARNING", "/", "expected openehr_demo_0.1.0.bmm.json") + self.assertOnly(self.check(self.demo(), name="demo.bmm.json"), "WARNING", "/", + "expected openehr_demo_0.1.0.bmm.json") def test_revision_follows_release(self): d = self.demo() d["schema_revision"] = "7" - self.assertFinding(self.check(d), "WARNING", "/schema_revision", ".") + self.assertOnly(self.check(d), "WARNING", "/schema_revision", ".") + + def test_release_has_three_parts(self): + d = self.demo() + d["rm_release"], d["schema_revision"] = "0.1", "0.1.1" + self.assertFinding(self.check(d, name="openehr_demo_0.1.bmm.json"), "WARNING", "/rm_release", "3-part") def test_spec_only_header_key_is_reported_as_ignored(self): d = self.demo() d["archetype_parent_class"] = "DEMO_ITEM" - self.assertFinding(self.check(d), "WARNING", "/archetype_parent_class", "not read by bmm-publisher") - - def test_schema_passed_as_its_own_dependency_is_ignored(self): - path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) - checker = check_bmm.Checker(path, [self.base, path]).run() - self.assertEqual([m for lvl, _, m in checker.findings], ["-d %s is the schema being checked; it is ignored" % path]) + self.assertOnly(self.check(d), "WARNING", "/archetype_parent_class", "not read by bmm-publisher") def test_dependency_not_included(self): d = self.demo() del d["includes"] - self.assertFinding(self.check(d), "WARNING", "/includes", "loaded with -d but is not included") + self.assertOnly(self.check(d), "WARNING", "/includes", "loaded with -d but is not included") + + def test_include_keyed_by_its_id(self): + d = self.demo() + d["includes"] = {"1": {"id": "openehr_base_1.3.0"}} + self.assertOnly(self.check(d), "WARNING", "/includes/1", "key each include by its id") + + def test_include_needs_an_id(self): + d = self.demo() + d["includes"]["openehr_base_1.3.0"] = {"id": ["openehr_base_1.3.0"]} + self.assertFinding(self.check(d), "ERROR", "/includes/openehr_base_1.3.0", "needs an \"id\"") class PackageTest(Base): + def packages(self, d): + return d["packages"]["org.openehr.demo.inventory"]["packages"] + def test_class_not_in_any_package_gets_no_table(self): d = self.demo() - d["packages"]["org.openehr.demo.inventory"]["packages"]["box"]["classes"].remove("DEMO_BOX_REF") - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF", "not listed in any package") + self.packages(d)["box"]["classes"].remove("DEMO_BOX_REF") + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF", "not listed in any package") def test_listed_class_must_be_defined(self): d = self.demo() - d["packages"]["org.openehr.demo.inventory"]["packages"]["box"]["classes"].append("DEMO_GHOST") - self.assertFinding(self.check(d), "ERROR", "/packages/org.openehr.demo.inventory/packages/box/classes", "DEMO_GHOST is not defined") + self.packages(d)["box"]["classes"].append("DEMO_GHOST") + self.assertOnly(self.check(d), "ERROR", "/packages/org.openehr.demo.inventory/packages/box/classes", + "DEMO_GHOST is not defined") + + def test_class_listed_twice(self): + d = self.demo() + self.packages(d)["box"]["classes"].append("DEMO_ITEM") + self.assertOnly(self.check(d), "WARNING", "/packages/org.openehr.demo.inventory/packages/box/classes", + "DEMO_ITEM is also listed") + + def test_package_key_and_name(self): + d = self.demo() + self.packages(d)["box"]["name"] = "boxes" + self.assertOnly(self.check(d), "ERROR", "/packages/org.openehr.demo.inventory/packages/box", "differ") def test_only_top_level_names_contain_dots(self): d = self.demo() - pkgs = d["packages"]["org.openehr.demo.inventory"]["packages"] + pkgs = self.packages(d) pkgs["a.b"] = dict(pkgs.pop("box"), name="a.b") - self.assertFinding(self.check(d), "ERROR", "/packages/org.openehr.demo.inventory/packages/a.b", "only top-level") + self.assertOnly(self.check(d), "ERROR", "/packages/org.openehr.demo.inventory/packages/a.b", "only top-level") def test_top_level_package_name_drives_table_file_names(self): d = self.demo() d["packages"] = {"demo": dict(d["packages"]["org.openehr.demo.inventory"], name="demo")} - self.assertFinding(self.check(d), "WARNING", "/packages/demo", "org.openehr.demo.") + self.assertOnly(self.check(d), "WARNING", "/packages/demo", "org.openehr.demo.") + + def test_package_holding_only_sub_packages_is_accepted(self): + d = self.demo() + d["packages"] = {"org.openehr.demo": dict(d["packages"]["org.openehr.demo.inventory"], name="org.openehr.demo")} + self.assertEqual(self.check(d).findings, []) def test_classes_directly_in_the_component_root_package(self): d = self.demo() - classes = [c for p in d["packages"]["org.openehr.demo.inventory"]["packages"].values() for c in p["classes"]] + classes = [c for p in self.packages(d).values() for c in p["classes"]] d["packages"] = {"org.openehr.demo": {"name": "org.openehr.demo", "classes": classes}} - self.assertFinding(self.check(d), "WARNING", "/packages/org.openehr.demo/classes", "org.openehr.demo.org..adoc") + self.assertOnly(self.check(d), "WARNING", "/packages/org.openehr.demo/classes", "org.openehr.demo.org..adoc") - def test_package_holding_only_sub_packages_is_accepted(self): + def nest(self, d, depth): + """Move the box classes into a chain of sub-packages, so that they sit at the given depth.""" + box = self.packages(d).pop("box") + parent, path = self.packages(d), "/packages/org.openehr.demo.inventory/packages" + for level in range(2, depth): + name = "p%d" % level + parent[name] = {"name": name, "packages": {}} + parent, path = parent[name]["packages"], "%s/%s/packages" % (path, name) + parent["box"] = box + return path + "/box" + + def test_packages_deeper_than_four_levels_get_no_table(self): d = self.demo() - d["packages"] = {"org.openehr.demo": dict(d["packages"]["org.openehr.demo.inventory"], name="org.openehr.demo")} + path = self.nest(d, 5) + self.assertOnly(self.check(d), "ERROR", path, "only 4 levels deep") + + def test_four_levels_are_visited(self): + d = self.demo() + self.nest(d, 4) self.assertEqual(self.check(d).findings, []) + def test_empty_package(self): + d = self.demo() + self.packages(d)["spare"] = {"name": "spare", "classes": []} + self.assertOnly(self.check(d), "WARNING", "/packages/org.openehr.demo.inventory/packages/spare", "empty package") + + def test_packages_must_be_an_object(self): + d = self.demo() + d["packages"] = [d["packages"]] + self.assertFinding(self.check(d), "ERROR", "/packages", "must be an object keyed by package name") + class ClassTest(Base): + def packages_add(self, d, name): + d["packages"]["org.openehr.demo.inventory"]["packages"]["item"]["classes"].append(name) + def test_key_and_name_differ(self): d = self.demo() - d["class_definitions"]["DEMO_ITEM"]["name"] = "DEMO_THING" + self.cls(d, "DEMO_ITEM")["name"] = "DEMO_THING" self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM", "key and \"name\"") - def test_unknown_class_type(self): + def test_class_defined_twice(self): + d = self.demo() + d["primitive_types"] = {"DEMO_ITEM": copy.deepcopy(self.cls(d, "DEMO_ITEM"))} + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM", "also defined at /primitive_types/DEMO_ITEM") + + def test_class_redefines_a_dependency_class(self): + d = self.demo() + d["class_definitions"]["Boolean"] = {"name": "Boolean", "documentation": "x"} + self.packages_add(d, "Boolean") + self.assertOnly(self.check(d), "WARNING", "/class_definitions/Boolean", "also defined in openehr_base_1.3.0") + + def test_unknown_class_type_is_reported_and_its_content_still_checked(self): + d = self.demo() + status = self.cls(d, "DEMO_STATUS") + status["_type"] = "P_BMM_ENUMERATON_STRING" + status["item_documentations"].pop() + checker = self.check(d) + self.assertFinding(checker, "ERROR", "/class_definitions/DEMO_STATUS", "unknown _type P_BMM_ENUMERATON_STRING") + self.assertFinding(checker, "ERROR", "/class_definitions/DEMO_STATUS/item_documentations", "one text per item") + + def test_class_type_must_be_a_string(self): d = self.demo() - d["class_definitions"]["DEMO_STATUS"]["_type"] = "P_BMM_ENUMERATION" - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS", "unknown _type P_BMM_ENUMERATION") + self.cls(d, "DEMO_ITEM")["_type"] = ["P_BMM_CLASS"] + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/_type", "must be a string") def test_ancestor_defs_alone_loses_the_parent(self): d = self.demo() - group = d["class_definitions"]["DEMO_GROUP"] + group = self.cls(d, "DEMO_GROUP") del group["ancestors"] group["ancestor_defs"] = {"DEMO_ITEM": {"_type": "P_BMM_SIMPLE_TYPE", "type": "DEMO_ITEM"}} checker = self.check(d) - self.assertFinding(checker, "ERROR", "/class_definitions/DEMO_GROUP", "has no parent") + self.assertFinding(checker, "ERROR", "/class_definitions/DEMO_GROUP/ancestor_defs", "the parent DEMO_ITEM is lost") self.assertFinding(checker, "WARNING", "/class_definitions/DEMO_GROUP/ancestor_defs", "not read by bmm-publisher") - def test_generic_name_in_ancestors(self): + def test_ancestor_defs_naming_another_parent_loses_it(self): d = self.demo() - d["class_definitions"]["DEMO_GROUP"]["ancestors"] = ["DEMO_BOX"] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_GROUP/ancestors/0", "root class (DEMO_BOX)") + self.cls(d, "DEMO_BOX_REF")["ancestor_defs"] = { + "DEMO_BOX": {"_type": "P_BMM_GENERIC_TYPE", "root_type": "DEMO_BOX", "generic_parameters": ["DEMO_GROUP"]}} + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/ancestor_defs", + "the parent DEMO_BOX is lost") - def test_inheritance_cycle(self): + def test_ancestor_defs_repeating_the_ancestors_only_warns(self): d = self.demo() - d["class_definitions"]["DEMO_ITEM"]["ancestors"] = ["DEMO_GROUP"] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_GROUP", - "inheritance cycle: DEMO_ITEM -> DEMO_GROUP -> DEMO_ITEM") + self.cls(d, "DEMO_GROUP")["ancestor_defs"] = {"DEMO_ITEM": {"_type": "P_BMM_SIMPLE_TYPE", "type": "DEMO_ITEM"}} + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_GROUP/ancestor_defs", "not read") - def test_enumeration_lists_must_line_up(self): + def test_generic_name_in_ancestors(self): d = self.demo() - d["class_definitions"]["DEMO_STATUS"]["item_documentations"].pop() - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS/item_documentations", "one text per item name (3)") + self.cls(d, "DEMO_GROUP")["ancestors"] = ["DEMO_BOX"] + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_GROUP/ancestors/0", "root class (DEMO_BOX)") - def test_enumeration_value_kind(self): + def test_unresolved_ancestor(self): d = self.demo() - d["class_definitions"]["DEMO_STATUS"]["item_values"] = [1, 2, 3] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS/item_values", "must be Strings") + self.cls(d, "DEMO_BOX")["ancestors"] = ["NOWHERE"] + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_BOX/ancestors/0", "NOWHERE is not defined") - def test_constant_needs_a_type(self): + def test_inheritance_cycle(self): d = self.demo() - del d["class_definitions"]["DEMO_ITEM"]["constants"]["Max_tags"]["type"] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/constants/Max_tags", "needs a \"type\"") + self.cls(d, "DEMO_ITEM")["ancestors"] = ["DEMO_GROUP"] + cycles = [f for f in self.check(d).findings if "inheritance cycle" in f[2]] + self.assertEqual(len(cycles), 1) + self.assertIn("DEMO_ITEM", cycles[0][2]) + self.assertIn("DEMO_GROUP", cycles[0][2]) + + def test_cycle_is_reported_where_the_class_is_defined(self): + d = self.demo() + d["primitive_types"] = {"P1": {"name": "P1", "documentation": "x", "ancestors": ["P2"]}, + "P2": {"name": "P2", "documentation": "x", "ancestors": ["P1"]}} + self.packages_add(d, "P1") + self.packages_add(d, "P2") + cycles = [f for f in self.check(d).findings if "inheritance cycle" in f[2]] + self.assertEqual(len(cycles), 1) + self.assertTrue(cycles[0][1].startswith("/primitive_types/"), cycles) + + def test_enumeration_checks(self): + cases = [ + ("item_documentations", lambda s: s["item_documentations"].pop(), "one text per item name (3)"), + ("item_values", lambda s: s.update(item_values=["a", "b"]), "one value per item name (3)"), + ("item_values", lambda s: s.update(item_values=[1, 2, 3]), "must be strings"), + ("item_names", lambda s: s.update(item_names=[]), "non-empty list"), + ] + for key, change, text in cases: + with self.subTest(change=text): + d = self.demo() + change(self.cls(d, "DEMO_STATUS")) + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS/" + key, text) + + def integer_enumeration(self, d, **extra): + status = self.cls(d, "DEMO_STATUS") + status.update(_type="P_BMM_ENUMERATION_INTEGER", ancestors=["Integer"], item_values=[0, 1, 2]) + status.update(extra) + return status + + def test_integer_enumeration_is_clean(self): + d = self.demo() + self.integer_enumeration(d) + self.assertEqual(self.check(d).findings, []) + + def test_integer_values_print_only_with_integer_ancestor(self): + d = self.demo() + self.integer_enumeration(d, ancestors=["Any"]) + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_STATUS/ancestors", "only when Integer") + + def test_class_keys_are_lost_on_enumerations_and_interfaces(self): + cases = [("P_BMM_ENUMERATION_STRING", "invariants", {"Inv": "True"}), + ("P_BMM_ENUMERATION_STRING", "properties", {}), + ("P_BMM_INTERFACE", "ancestors", ["Any"])] + for kind, key, value in cases: + with self.subTest(kind=kind, key=key): + d = self.demo() + status = self.cls(d, "DEMO_STATUS") + if kind == "P_BMM_INTERFACE": + status.clear() + status.update(_type=kind, name="DEMO_STATUS", documentation="x") + status[key] = value + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_STATUS/" + key, "so it is lost") + + def test_constant_checks(self): + cases = [(lambda c: c.pop("type"), "ERROR", "", "needs a \"type\""), + (lambda c: c.update(name="Max"), "ERROR", "", "key and \"name\""), + (lambda c: c.update(type="Integr"), "WARNING", "/type", "did you mean Integer")] + for change, level, suffix, text in cases: + with self.subTest(text=text): + d = self.demo() + change(self.cls(d, "DEMO_ITEM")["constants"]["Max_tags"]) + self.assertFinding(self.check(d), level, "/class_definitions/DEMO_ITEM/constants/Max_tags" + suffix, text) def test_invariants_map_tags_to_strings(self): d = self.demo() - d["class_definitions"]["DEMO_ITEM"]["invariants"] = ["not name.is_empty()"] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/invariants", "tag to an expression") + self.cls(d, "DEMO_ITEM")["invariants"] = ["not name.is_empty()"] + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/invariants", "tag to an expression") + def test_flags_must_be_booleans(self): + cases = [("/class_definitions/DEMO_ITEM/is_abstract", lambda d: self.cls(d, "DEMO_ITEM").update(is_abstract="true")), + ("/class_definitions/DEMO_ITEM/properties/uid/is_mandatory", + lambda d: self.props(d, "DEMO_ITEM")["uid"].update(is_mandatory="true")), + ("/class_definitions/DEMO_GROUP/functions/first_tagged/is_nullable", + lambda d: self.fn(d, "DEMO_GROUP", "first_tagged").update(is_nullable=1)), + ("/class_definitions/DEMO_ITEM/functions/is_tagged/parameters/a_tag/is_nullable", + lambda d: self.fn(d)["parameters"]["a_tag"].update(is_nullable="no"))] + for path, change in cases: + with self.subTest(path=path): + d = self.demo() + change(d) + self.assertOnly(self.check(d), "ERROR", path, "must be true or false") -class PropertyTest(Base): - P = "/class_definitions/DEMO_GROUP/properties/" + def test_generic_parameter_needs_its_name(self): + d = self.demo() + self.cls(d, "DEMO_BOX")["generic_parameter_defs"]["T"]["name"] = "U" + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX/generic_parameter_defs/T", "equal to its key") - def props(self, d, cls="DEMO_GROUP"): - return d["class_definitions"][cls]["properties"] - def test_misspelt_property_type_falls_back(self): - d = self.demo() - self.props(d)["items"]["_type"] = "P_BMM_CONTAINER_PROPRETY" - self.assertFinding(self.check(d), "ERROR", self.P + "items", "read as P_BMM_SINGLE_PROPERTY") +class PropertyTest(Base): + P = "/class_definitions/DEMO_GROUP/properties/" - def test_container_without_type_becomes_any(self): - d = self.demo() - del self.props(d)["items"]["_type"] - self.assertFinding(self.check(d), "ERROR", self.P + "items", "defaults to Any") + def test_wrong_type_markers(self): + cases = [ + ("items", lambda p: p.update(_type="P_BMM_CONTAINER_PROPRETY"), "items", "unknown _type P_BMM_CONTAINER_PROPRETY"), + ("items", lambda p: p.pop("_type"), "items", "defaults to Any"), + ("items", lambda p: p.update(_type=7), "items/_type", "must be a string"), + ("items", lambda p: p["type_def"].update(_type="P_BMM_GENERIC_TYPE"), "items/type_def", "is ignored here"), + ("index", lambda p: p.update(_type="P_BMM_INDEXED_CONTAINER_PROPERTY"), "index", "root_type Hash"), + ] + for prop, change, path, text in cases: + with self.subTest(text=text): + d = self.demo() + change(self.props(d)[prop]) + self.assertFinding(self.check(d), "ERROR", self.P + path, text) + + def test_unknown_property_type_still_checks_its_content(self): + d = self.demo() + items = self.props(d)["items"] + items["_type"] = "P_BMM_CONTAINER_PROPRETY" + items["is_mandantory"] = items.pop("is_mandatory") + checker = self.check(d) + self.assertFinding(checker, "ERROR", self.P + "items", "checked below as P_BMM_CONTAINER_PROPERTY") + self.assertFinding(checker, "ERROR", self.P + "items/is_mandantory", "misspelling of is_mandatory") - def test_indexed_container_is_not_read(self): + def test_property_key_and_name_differ(self): d = self.demo() - self.props(d)["index"] = {"_type": "P_BMM_INDEXED_CONTAINER_PROPERTY", "name": "index", "documentation": "x", - "type_def": {"container_type": "Hash", "index_type": "String", "type": "DEMO_ITEM"}} - self.assertFinding(self.check(d), "ERROR", self.P + "index", "root_type Hash") + self.props(d)["items"]["name"] = "item" + self.assertOnly(self.check(d), "ERROR", self.P + "items", "key and \"name\" (item) differ") def test_value_set_constraint_loses_the_type(self): d = self.demo() @@ -227,120 +462,400 @@ def test_value_set_constraint_loses_the_type(self): "type_ref": {"type": "String", "value_constraint": "openEHR::languages"}} self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/properties/name", "read as type Any") - def test_bounded_upper_needs_upper_unbounded_false(self): - d = self.demo() - del self.props(d)["ranks"]["cardinality"]["upper_unbounded"] - self.assertFinding(self.check(d), "ERROR", self.P + "ranks/cardinality", "\"upper_unbounded\": false") - - def test_upper_below_lower(self): - d = self.demo() - self.props(d)["ranks"]["cardinality"]["upper"] = 1 - self.assertFinding(self.check(d), "ERROR", self.P + "ranks/cardinality", "upper is below lower") + def test_cardinality_checks(self): + cases = [(lambda c: c.pop("upper_unbounded"), "", "\"upper_unbounded\": false"), + (lambda c: c.update(upper=1), "", "upper is below lower"), + (lambda c: c.pop("upper"), "", "needs an \"upper\" limit"), + (lambda c: c.update(lower=-1), "/lower", "non-negative integer"), + (lambda c: c.update(upper_unbounded="no"), "/upper_unbounded", "must be true or false")] + for change, suffix, text in cases: + with self.subTest(text=text): + d = self.demo() + change(self.props(d)["ranks"]["cardinality"]) + self.assertFinding(self.check(d), "ERROR", self.P + "ranks/cardinality" + suffix, text) def test_hash_is_not_a_container_type(self): d = self.demo() self.props(d)["items"]["type_def"]["container_type"] = "Hash" - self.assertFinding(self.check(d), "ERROR", self.P + "items/type_def/container_type", "Hash takes 2") + self.assertOnly(self.check(d), "ERROR", self.P + "items/type_def/container_type", "Hash takes 2") def test_container_needs_an_element_type(self): d = self.demo() del self.props(d)["items"]["type_def"]["type"] - self.assertFinding(self.check(d), "ERROR", self.P + "items/type_def", "read as Any") + self.assertOnly(self.check(d), "ERROR", self.P + "items/type_def", "read as Any") + + def test_simple_element_type_in_type_def_renders_any(self): + d = self.demo() + td = self.props(d)["items"]["type_def"] + td["type_def"] = {"_type": "P_BMM_SIMPLE_TYPE", "type": td.pop("type")} + self.assertOnly(self.check(d), "ERROR", self.P + "items/type_def/type_def", "goes in \"type\"") def test_generic_parameter_count(self): d = self.demo() self.props(d)["index"]["type_def"]["generic_parameters"] = ["String"] - self.assertFinding(self.check(d), "ERROR", self.P + "index/type_def", "Hash takes 2 generic parameters, 1 given") + self.assertOnly(self.check(d), "ERROR", self.P + "index/type_def", "Hash takes 2 generic parameters, 1 given") + + def test_generic_type_needs_parameters(self): + d = self.demo() + del self.props(d)["index"]["type_def"]["generic_parameters"] + self.assertOnly(self.check(d), "ERROR", self.P + "index/type_def", "needs generic_parameters or generic_parameter_defs") + + def test_mixed_generic_parameter_forms(self): + d = self.demo() + self.props(d)["index"]["type_def"]["generic_parameter_defs"] = {"V": {"_type": "P_BMM_SIMPLE_TYPE", "type": "DEMO_GROUP"}} + self.assertOnly(self.check(d), "ERROR", self.P + "index/type_def", "ignores generic_parameter_defs") + + def test_type_object_in_generic_parameters(self): + d = self.demo() + self.props(d)["index"]["type_def"]["generic_parameters"][1] = {"_type": "P_BMM_SIMPLE_TYPE", "type": "DEMO_ITEM"} + self.assertOnly(self.check(d), "ERROR", self.P + "index/type_def/generic_parameters/1", "stops with a type error") + + def test_generic_type_object_in_generic_parameters(self): + d = self.demo() + self.props(d, "DEMO_BOX_REF")["target"]["type_def"]["generic_parameters"] = [ + {"_type": "P_BMM_GENERIC_TYPE", "root_type": "DEMO_BOX", "generic_parameters": ["DEMO_GROUP"]}] + path = "/class_definitions/DEMO_BOX_REF/properties/target/type_def/generic_parameters/0" + self.assertOnly(self.check(d), "ERROR", path, "DEMO_BOX does not conform to DEMO_ITEM") + + def test_container_type_in_generic_parameter_defs_prints_nothing(self): + d = self.demo() + td = self.props(d)["index"]["type_def"] + del td["generic_parameters"] + td["generic_parameter_defs"] = {"K": {"_type": "P_BMM_SIMPLE_TYPE", "type": "String"}, + "V": {"_type": "P_BMM_CONTAINER_TYPE", "container_type": "List", "type": "DEMO_ITEM"}} + self.assertOnly(self.check(d), "ERROR", self.P + "index/type_def/generic_parameter_defs/V", "prints nothing") - def test_nested_type_needs_its_type(self): + def test_nested_type_needs_its_type_marker(self): d = self.demo() del self.props(d, "DEMO_BOX_REF")["replaced"]["type_def"]["type_def"]["_type"] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/replaced/type_def/type_def", "no _type") + self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/replaced/type_def/type_def", + "no _type") + + def test_single_property_without_type_marker(self): + d = self.demo() + del self.props(d, "DEMO_ITEM")["uid"]["_type"] + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid", "published schemas always set it") + + def test_undeclared_single_letter_type(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["type"] = "T" + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/type", "T is not a generic parameter") + + def test_lower_unbounded_cardinality(self): + d = self.demo() + self.props(d)["items"]["cardinality"]["lower_unbounded"] = True + self.assertOnly(self.check(d), "WARNING", self.P + "items/cardinality/lower_unbounded", "lower limit") def test_open_property_needs_a_declared_parameter(self): d = self.demo() self.props(d, "DEMO_BOX")["content"]["type"] = "U" - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX/properties/content/type", "U is not a generic parameter") + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_BOX/properties/content/type", "U is not a generic parameter") - def test_generic_name_in_type(self): + def test_generic_parameter_as_single_property_type(self): d = self.demo() - self.props(d, "DEMO_ITEM")["uid"]["type"] = "List" - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/properties/uid/type", "not a class name") + self.props(d, "DEMO_BOX")["content"]["_type"] = "P_BMM_SINGLE_PROPERTY" + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_BOX/properties/content", "use P_BMM_SINGLE_PROPERTY_OPEN") + + def test_generic_names_where_class_names_belong(self): + cases = [("DEMO_ITEM", "uid", lambda p: p.update(type="List"), "uid/type"), + ("DEMO_ITEM", "tags", lambda p: p["type_def"].update(type="List"), "tags/type_def/type"), + ("DEMO_GROUP", "index", lambda p: p["type_def"]["generic_parameters"].__setitem__(1, "List"), + "index/type_def/generic_parameters/1")] + for cls, prop, change, path in cases: + with self.subTest(path=path): + d = self.demo() + change(self.props(d, cls)[prop]) + self.assertFinding(self.check(d), "ERROR", "/class_definitions/%s/properties/%s" % (cls, path), "name") - def test_misspelt_key_is_reported(self): + def test_misspelt_key_is_an_error(self): d = self.demo() uid = self.props(d, "DEMO_ITEM")["uid"] uid["is_mandantory"] = uid.pop("is_mandatory") - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/properties/uid/is_mandantory", - "misspelling of is_mandatory") + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM/properties/uid/is_mandantory", + "misspelling of is_mandatory") def test_unknown_key_that_resembles_nothing_is_a_warning(self): d = self.demo() self.props(d, "DEMO_ITEM")["uid"]["colour"] = "blue" - self.assertFinding(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/colour", "unknown key") + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/colour", "unknown key") + + def test_unresolved_type_name(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["type"] = "NO_SUCH_CLASS" + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/type", "NO_SUCH_CLASS is not defined") + + def test_misspelt_class_name_is_caught_even_without_dependencies(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["status"]["type"] = "DEMO_STATS" + checker = self.check(d, deps=False) + self.assertFinding(checker, "WARNING", "/class_definitions/DEMO_ITEM/properties/status/type", "did you mean DEMO_STATUS") + self.assertNotIn("DEMO_STATS", checker.unchecked) + + def test_attribute_reference_in_documentation(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["documentation"] = "See {base_release} for details." + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/documentation", "{base_release}") - def test_generic_argument_must_conform(self): + def test_braces_that_are_not_attribute_references(self): + for text in ("Cardinality {1..1}.", "A {TypeName} placeholder.", "Range {x-y}."): + with self.subTest(text=text): + d = self.demo() + self.props(d, "DEMO_ITEM")["uid"]["documentation"] = text + self.assertEqual(self.check(d).findings, []) + + def test_missing_documentation(self): d = self.demo() - self.props(d, "DEMO_BOX_REF")["target"]["type_def"]["generic_parameters"] = ["String"] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/target/type_def/generic_parameters/0", - "String does not conform to DEMO_ITEM, the constraint on parameter T of DEMO_BOX") + del self.props(d, "DEMO_ITEM")["uid"]["documentation"] + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid", "no documentation") + - def test_nested_generic_argument_must_conform(self): +class ConformanceTest(Base): + def generic(self, d, cls, prop): + return self.props(d, cls)[prop]["type_def"] + + def test_argument_must_conform(self): d = self.demo() - nested = self.props(d, "DEMO_BOX_REF")["replaced"]["type_def"]["type_def"] - nested["generic_parameters"] = ["Integer"] - self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/replaced/type_def/type_def/generic_parameters/0", - "Integer does not conform to DEMO_ITEM") + self.generic(d, "DEMO_BOX_REF", "target")["generic_parameters"] = ["String"] + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/properties/target/type_def/generic_parameters/0", + "String does not conform to DEMO_ITEM, the constraint on parameter T of DEMO_BOX") - def test_conformance_through_a_descendant(self): + def test_descendant_conforms(self): d = self.demo() - self.props(d, "DEMO_BOX_REF")["target"]["type_def"]["generic_parameters"] = ["DEMO_ITEM"] + self.generic(d, "DEMO_BOX_REF", "target")["generic_parameters"] = ["DEMO_GROUP"] self.assertEqual(self.check(d).findings, []) - def test_unresolved_type_name(self): + def test_conformance_through_several_levels_of_base(self): d = self.demo() - self.props(d, "DEMO_ITEM")["uid"]["type"] = "HIER_OBJECT_IDX" - self.assertFinding(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/type", "HIER_OBJECT_IDX is not defined") + self.props(d, "DEMO_ITEM")["window"] = {"_type": "P_BMM_GENERIC_PROPERTY", "name": "window", "documentation": "x", + "type_def": {"root_type": "Interval", "generic_parameters": ["Integer"]}} + self.assertEqual(self.check(d).findings, []) - def test_attribute_reference_in_documentation(self): + def test_hash_key_must_be_ordered(self): + d = self.demo() + self.generic(d, "DEMO_GROUP", "index")["generic_parameters"] = ["DEMO_ITEM", "String"] + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_GROUP/properties/index/type_def/generic_parameters/0", + "DEMO_ITEM does not conform to Ordered") + + def test_nested_argument_must_conform(self): d = self.demo() - self.props(d, "DEMO_ITEM")["uid"]["documentation"] = "See {base_release} for details; {1..1} stays literal." + self.generic(d, "DEMO_BOX_REF", "replaced")["type_def"]["generic_parameters"] = ["Integer"] + self.assertOnly(self.check(d), "ERROR", + "/class_definitions/DEMO_BOX_REF/properties/replaced/type_def/type_def/generic_parameters/0", + "Integer does not conform to DEMO_ITEM") + + def test_generic_parameter_defs_bind_by_position(self): + d = self.demo() + td = self.generic(d, "DEMO_GROUP", "index") + del td["generic_parameters"] + td["generic_parameter_defs"] = {"V": {"_type": "P_BMM_SIMPLE_TYPE", "type": "DEMO_ITEM"}, + "K": {"_type": "P_BMM_SIMPLE_TYPE", "type": "String"}} checker = self.check(d) - self.assertFinding(checker, "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/documentation", "{base_release}") - self.assertEqual(len(self.messages(checker, "WARNING")), 1) + path = "/class_definitions/DEMO_GROUP/properties/index/type_def/generic_parameter_defs/" + self.assertFinding(checker, "WARNING", path + "V", "entry 1 is parameter K of Hash") + self.assertFinding(checker, "ERROR", path + "V", "DEMO_ITEM does not conform to Ordered") + + def test_enclosing_parameter_conforms_through_its_constraint(self): + d = self.demo() + wrap = {"name": "DEMO_WRAP", "documentation": "x", "generic_parameter_defs": {"U": {"name": "U"}}, + "properties": {"box": {"_type": "P_BMM_GENERIC_PROPERTY", "name": "box", "documentation": "x", + "type_def": {"root_type": "DEMO_BOX", "generic_parameters": ["U"]}}}} + d["class_definitions"]["DEMO_WRAP"] = wrap + d["packages"]["org.openehr.demo.inventory"]["packages"]["box"]["classes"].append("DEMO_WRAP") + path = "/class_definitions/DEMO_WRAP/properties/box/type_def/generic_parameters/0" + self.assertOnly(self.check(d), "ERROR", path, "U (constrained to Any) does not conform to DEMO_ITEM") + wrap["generic_parameter_defs"]["U"]["conforms_to_type"] = "DEMO_GROUP" + self.assertEqual(self.check(d).findings, []) + + def test_undecidable_conformance_is_not_an_error(self): + d = self.demo() + self.generic(d, "DEMO_BOX_REF", "target")["generic_parameters"] = ["HIER_OBJECT_ID"] + checker = self.check(d, deps=False) + self.assertNoFinding(checker) + self.assertIn("HIER_OBJECT_ID", checker.unchecked) class FunctionTest(Base): F = "/class_definitions/DEMO_ITEM/functions/is_tagged" - def fn(self, d): - return d["class_definitions"]["DEMO_ITEM"]["functions"]["is_tagged"] + def param(self, d): + return self.fn(d)["parameters"]["a_tag"] - def test_container_parameter_needs_cardinality(self): + def test_container_parameter_cardinality_is_optional(self): d = self.demo() self.fn(d)["parameters"]["a_tag"] = {"_type": "P_BMM_CONTAINER_FUNCTION_PARAMETER", "name": "a_tag", "type_def": {"container_type": "List", "type": "String"}} - self.assertFinding(self.check(d), "ERROR", self.F + "/parameters/a_tag", "needs a \"cardinality\"") - - def test_result_needs_its_type(self): - d = self.demo() - self.fn(d)["result"] = {"container_type": "List", "type": "String"} - self.assertFinding(self.check(d), "ERROR", self.F + "/result", "no _type") + self.assertEqual(self.check(d).findings, []) - def test_simple_result_without_type_marker(self): + def test_container_parameter_cardinality_is_checked(self): d = self.demo() - self.fn(d)["result"] = {"type": "Boolean"} - self.assertFinding(self.check(d), "WARNING", self.F + "/result", "published schemas always set it") - - def test_void_result_is_accepted(self): + self.fn(d)["parameters"]["a_tag"] = {"_type": "P_BMM_CONTAINER_FUNCTION_PARAMETER", "name": "a_tag", + "type_def": {"container_type": "List", "type": "String"}, + "cardinality": {"lower": 0, "upper": 3}} + self.assertOnly(self.check(d), "ERROR", self.F + "/parameters/a_tag/cardinality", "\"upper_unbounded\": false") + + def test_wrong_parameter_markers(self): + cases = [(lambda p: p.update(_type="P_BMM_SINGLE_FUNCTION_PARAMETR"), "", "unknown _type"), + (lambda p: p.update(_type="P_BMM_CONTAINER_FUNCTION_PARAMETER"), "", "needs a \"type_def\""), + (lambda p: (p.pop("type"), p.pop("_type"), p.update(type_def={"container_type": "List", "type": "String"})), + "", "no _type"), + (lambda p: p.update(_type="P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN"), "/type", "not a generic parameter"), + (lambda p: p.update(name="tag"), "", "key and \"name\"")] + for change, suffix, text in cases: + with self.subTest(text=text): + d = self.demo() + change(self.param(d)) + self.assertFinding(self.check(d), "ERROR", self.F + "/parameters/a_tag" + suffix, text) + + def test_result_markers(self): + cases = [({"type": "Boolean"}, "WARNING", "published schemas always set it"), + ({"container_type": "List", "type": "String"}, "ERROR", "no _type"), + ({"_type": "P_BMM_CONTAINER_TYP", "container_type": "List", "type": "String"}, "ERROR", "unknown _type"), + ({"_type": "P_BMM_INDEXED_CONTAINER_TYPE", "container_type": "Hash"}, "ERROR", "does not read"), + ({"_type": "P_BMM_OPEN_TYPE", "type": "Boolean"}, "WARNING", "read as P_BMM_SIMPLE_TYPE")] + for result, level, text in cases: + with self.subTest(text=text): + d = self.demo() + self.fn(d)["result"] = result + self.assertFinding(self.check(d), level, self.F + "/result", text) + + def test_void_is_a_procedure_result_only(self): d = self.demo() self.fn(d)["result"] = {"_type": "P_BMM_SIMPLE_TYPE", "type": "void"} self.assertEqual(self.check(d).findings, []) + self.props(d, "DEMO_ITEM")["uid"]["type"] = "void" + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/type", "only the result type") - def test_conditions_map_tags_to_strings(self): + def test_function_without_result(self): d = self.demo() - self.fn(d)["pre_conditions"] = {"Pre_tag_valid": True} - self.assertFinding(self.check(d), "ERROR", self.F + "/pre_conditions", "tag to an expression") + del self.fn(d)["result"] + self.assertOnly(self.check(d), "WARNING", self.F, "empty result type") + + def test_conditions_and_aliases(self): + cases = [(lambda f: f.update(pre_conditions={"Pre_tag_valid": True}), "/pre_conditions", "tag to an expression"), + (lambda f: f.update(post_conditions=["Result"]), "/post_conditions", "tag to an expression"), + (lambda f: f.update(aliases="has_tag"), "/aliases", "list of names")] + for change, suffix, text in cases: + with self.subTest(text=text): + d = self.demo() + change(self.fn(d)) + self.assertOnly(self.check(d), "ERROR", self.F + suffix, text) + + +class DependencyTest(Base): + def test_schema_passed_as_its_own_dependency_is_ignored(self): + path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + checker = check_bmm.Checker(path, [self.base, path]).run() + self.assertEqual([m for _, _, m in checker.findings], ["-d %s is the schema being checked; it is ignored" % path]) + + def test_dependencies_with_the_same_id_are_merged(self): + half = copy.deepcopy(BASE) + half["class_definitions"] = {k: v for k, v in BASE["class_definitions"].items() if k != "HIER_OBJECT_ID"} + rest = copy.deepcopy(BASE) + rest["class_definitions"] = {"HIER_OBJECT_ID": BASE["class_definitions"]["HIER_OBJECT_ID"]} + a, b = self.write("half.bmm.json", half), self.write("rest.bmm.json", rest) + path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + checker = check_bmm.Checker(path, [a, b]).run() + self.assertEqual([lvl for lvl, _, _ in checker.findings], ["WARNING"], self.listing(checker)) + self.assertIn("same schema id", checker.findings[0][2]) + + def test_dependency_that_is_not_a_schema(self): + for content in ("[]", "{\"name\": \"x\"}"): + with self.subTest(content=content): + dep = self.root / "dep.json" + dep.write_text(content, encoding="utf-8") + path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + with redirect_stderr(io.StringIO()), self.assertRaises(SystemExit) as raised: + check_bmm.Checker(path, [dep]) + self.assertEqual(raised.exception.code, 2) + + +def nodes(value, path=()): + """Every (path, value) in a JSON document, the root included.""" + yield path, value + if isinstance(value, dict): + for key, child in value.items(): + yield from nodes(child, path + (key,)) + elif isinstance(value, list): + for i, child in enumerate(value): + yield from nodes(child, path + (i,)) + + +class RobustnessTest(Base): + """A checker mostly sees broken files: no input may end in a traceback.""" + + REPLACEMENTS = [[], 7, {}, "x", None, True, [{}], {"x": []}] + + def test_no_traceback_for_any_wrongly_typed_value(self): + for path, _ in nodes(EXAMPLE): + if not path: + continue + for replacement in self.REPLACEMENTS: + d = self.demo() + target = d + for step in path[:-1]: + target = target[step] + target[path[-1]] = replacement + with self.subTest(path="/".join(map(str, path)), value=replacement): + try: + self.check(d) + except Exception as exc: # name the input that broke it, not just the traceback + self.fail("%s: %s" % (type(exc).__name__, exc)) + + def test_every_wrongly_typed_value_is_reported(self): + # JSON null reads as an absent key, and a constant's value may be a string or a number. + free = {("class_definitions", "DEMO_ITEM", "constants", "Max_tags", "value")} + for path, original in nodes(EXAMPLE): + if not path or path in free: + continue + for replacement in self.REPLACEMENTS: + if replacement is None or type(replacement) is type(original): + continue + d = self.demo() + target = d + for step in path[:-1]: + target = target[step] + target[path[-1]] = replacement + with self.subTest(path="/".join(map(str, path)), value=replacement): + checker = self.check(d) + self.assertTrue([f for f in checker.findings if f[0] in ("ERROR", "WARNING")], "nothing reported") + + def test_shape_errors_are_reported_where_they_occur(self): + P = "/packages/org.openehr.demo.inventory/packages/box" + G = "/class_definitions/DEMO_GROUP/properties/" + cases = [ + (lambda d: d.update(includes=[]), "/includes", "keyed by schema id"), + (lambda d: d.update(class_definitions=[]), "/class_definitions", "keyed by class name"), + (lambda d: d["class_definitions"].update(DEMO_ITEM=[]), "/class_definitions/DEMO_ITEM", "must be an object"), + (lambda d: d["class_definitions"]["DEMO_ITEM"].pop("name"), "/class_definitions/DEMO_ITEM", "has no \"name\""), + (lambda d: d["packages"]["org.openehr.demo.inventory"]["packages"].update(box=[]), P, "a package must be an object"), + (lambda d: d["packages"]["org.openehr.demo.inventory"]["packages"]["box"].update(classes="x"), P + "/classes", + "list of class names"), + (lambda d: d["packages"]["org.openehr.demo.inventory"]["packages"]["box"].update(packages=[]), P + "/packages", + "keyed by package name"), + (lambda d: d["class_definitions"]["DEMO_BOX"].update(generic_parameter_defs=[]), + "/class_definitions/DEMO_BOX/generic_parameter_defs", "keyed by parameter name"), + (lambda d: d["class_definitions"]["DEMO_GROUP"]["properties"]["index"]["type_def"].update(generic_parameters="x"), + G + "index/type_def/generic_parameters", "list of class or parameter names"), + (lambda d: d["class_definitions"]["DEMO_GROUP"]["properties"]["index"]["type_def"].update(generic_parameter_defs=[]), + G + "index/type_def/generic_parameter_defs", "object of nested types"), + (lambda d: d["class_definitions"]["DEMO_GROUP"].update(ancestors=[""]), + "/class_definitions/DEMO_GROUP/ancestors/0", "non-empty class name"), + (lambda d: d["class_definitions"]["DEMO_GROUP"]["functions"]["item_count"].update( + result={"_type": "P_BMM_SIMPLE_TYPE", "type": "List"}), + "/class_definitions/DEMO_GROUP/functions/item_count/result/type", "not a class name"), + (lambda d: d["class_definitions"]["DEMO_GROUP"]["properties"]["items"]["type_def"].update(index_type="String"), + G + "items/type_def/index_type", "the index is dropped"), + ] + for change, path, text in cases: + with self.subTest(path=path, text=text): + d = self.demo() + change(d) + self.assertFinding(self.check(d), "ERROR", path, text) + + def test_whole_document_of_the_wrong_type(self): + for value in ([], "x", 3, None): + with self.subTest(value=value): + self.assertFinding(self.check(value), "ERROR", "/", "must be a JSON object") class CommandLineTest(Base): @@ -357,7 +872,7 @@ def test_clean_file_exits_zero(self): path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) code, out, _ = self.run_main(path, "-d", self.base) self.assertEqual(code, 0) - self.assertIn("0 error(s), 0 warning(s)", out) + self.assertIn("5 class(es), 0 error(s), 0 warning(s), 0 name(s) not checked", out) def test_errors_exit_one(self): d = self.demo() @@ -371,15 +886,33 @@ def test_strict_fails_on_warnings(self): self.assertEqual(self.run_main(path, "-d", self.base)[0], 0) self.assertEqual(self.run_main(path, "-d", self.base, "--strict")[0], 1) - def test_invalid_json_exits_two(self): - path = self.root / "broken.bmm.json" - path.write_text("{\"bmm_version\": ", encoding="utf-8") - code, _, err = self.run_main(path) - self.assertEqual(code, 2) - self.assertIn("not valid JSON", err) + def test_unloaded_include_makes_the_check_incomplete(self): + path = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + for extra in ((), ("--strict",)): + with self.subTest(args=extra): + code, out, err = self.run_main(path, *extra) + self.assertEqual(code, 3) + self.assertIn("name(s) not checked", out) + self.assertIn("incomplete", err) - def test_missing_file_exits_two(self): - self.assertEqual(self.run_main(self.root / "absent.bmm.json")[0], 2) + def test_errors_outrank_an_incomplete_check(self): + d = self.demo() + del d["class_definitions"]["DEMO_ITEM"]["properties"]["uid"]["type"] + self.assertEqual(self.run_main(self.write("openehr_demo_0.1.0.bmm.json", d))[0], 1) + + def test_unreadable_input_exits_two(self): + broken = self.root / "broken.bmm.json" + broken.write_text("{\"bmm_version\": ", encoding="utf-8") + latin = self.root / "latin.bmm.json" + latin.write_bytes("{\"schema_author\": \"Jos\xe9\"}".encode("latin-1")) + good = self.write("openehr_demo_0.1.0.bmm.json", self.demo()) + cases = [((broken,), "not valid JSON"), ((latin,), "not UTF-8"), ((self.root / "absent.bmm.json",), "cannot read"), + ((good, "-d", self.root / "absent.bmm.json"), "cannot read"), ((good, "-d", latin), "not UTF-8")] + for args, text in cases: + with self.subTest(text=text): + code, _, err = self.run_main(*args) + self.assertEqual(code, 2) + self.assertIn(text, err) if __name__ == "__main__": From a59c35e8582951e12b9e90a15c006a25676b035e Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Fri, 9 Oct 2026 15:53:42 +0200 Subject: [PATCH 3/6] docs(openehr-specs): align bmm-authoring with the live P_BMM spec and the checker - The reference now follows the development BMM Persistence spec, which names JSON as bmm-publisher's primary format and covers functions, constants and invariants (P_BMM 2.4): corrected chapter names, the procedure rule (no result, written as void in the published schemas), class_definitions as optional, aliases as written, constant values, invariant and condition tags, and the documentation escaping rules. - It documents what bmm-publisher renders wrongly and how to avoid it: simple element types, type objects in generic_parameters, both generic-parameter forms at once, containers in generic_parameter_defs, binding by position, the four-level package limit, enumeration values, and which maps keep their keys. - Corrected: a document can have several top-level packages; ITS-BMM republishes only from repos that carry its notify workflow; a container parameter's cardinality is optional; lifecycle values for an unreleased schema are a suggestion to confirm. - SKILL.md: compare checker output before and after with diff, treat exit 3 as an incomplete check, report a schema as unchecked without python3, and replace the scaffold starter's root package. - scaffold routes filling a new repository's empty schema to bmm-authoring. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 2 +- CHANGELOG.md | 4 +- docs/testing.md | 2 +- .../skills/bmm-authoring/SKILL.md | 52 +++--- .../bmm-authoring/references/p-bmm-json.md | 176 ++++++++++-------- .../openehr-specs/skills/scaffold/SKILL.md | 3 +- 6 files changed, 139 insertions(+), 100 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dbd9950..93fb479 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,7 +59,7 @@ gracefully when a tool is absent (each says so in its prompt): - **`spec-reviewer`, `xref-auditor`** — need a `specifications-XX` checkout and (for attribute resolution) the sibling `specifications-AA_GLOBAL`. - **`xref-auditor`, `identifier-grounding`** — use **WebFetch** to read spec Markdown twins (`.html` → `.md`); `identifier-grounding` additionally prefers the **`openehr-assistant` MCP** (`type_specification_get`) when connected, falling back to the twin. - **`regen-classes`** — needs **Docker** (`ghcr.io/openehr/bmm-publisher`). -- **`bmm-authoring`** — needs **`python3`** (standard library only) to run its read-only `scripts/check_bmm.py`; without it, it says so and checks the change by hand against its reference. +- **`bmm-authoring`** — needs **`python3`** (standard library only) to run its read-only `scripts/check_bmm.py`; without it, it says so, reports the schema as unchecked and reviews the change by hand against its reference. - **`scaffold`** — needs **`python3`** (standard library only) to run its bundled `scripts/scaffold.py`, and says so and stops when it is missing. It writes into the repository it is run in, so it plans first and never overwrites a hand-edited file without being told to. For a new repository's BMM schema it asks **Docker** for the schemas the local `ghcr.io/openehr/bmm-publisher` image bundles, and never pulls the image. Without Docker or the image that one file is reported as blocked, and the rest are still written. - **`publish`** — needs **Docker** and sibling `specifications-AA_GLOBAL` and component checkouts. It runs the published `ghcr.io/openehr/asciidoctor` image, which bundles the toolchain and `spec_publish.sh`; the boilerplate and references are still read from the AA_GLOBAL checkout. It checks that each HTML output was rebuilt and that the log has no missing includes, because the script reports success even when it built nothing or dropped the class tables. diff --git a/CHANGELOG.md b/CHANGELOG.md index 270cbef..1a2c776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Notable changes to the plugins in this repository. The format follows [Keep a Ch ### Added -- `bmm-authoring` skill: write a component's BMM schema (the P_BMM JSON in `computable/BMM/`) from scratch, or change one. It covers the header and its conventions, `includes`, how packages set the class-table file names, every class, property, type and function kind, invariants and constants, documentation text, what the legacy class tables do not show, and the persistence-spec features `bmm-publisher` does not read (indexed containers, `ancestor_defs`, `type_ref` value sets), with the form to write instead. It ships a worked example (`assets/openehr_demo_0.1.0.bmm.json`) and a read-only checker, `scripts/check_bmm.py` (standard-library Python 3.8+), that reports what `bmm-publisher` accepts without a message: a misspelt or missing `_type`, a misspelt key such as `is_mandantory`, a key that differs from its `name`, a class in no package, an upper limit without `"upper_unbounded": false`, a container without an element type, wrong generic parameter counts, a generic argument that does not conform to its parameter's `conforms_to_type`, unresolved type names (with `-d` for the included schemas), and more. CI runs its unit tests (`scripts/test_check_bmm.py`). +- `bmm-authoring` skill: write a component's BMM schema (the P_BMM JSON in `computable/BMM/`) from scratch, or change one. It covers the header and its conventions, `includes`, how packages set the class-table file names, every class, property, type and function kind, invariants and constants, documentation text, what the legacy class tables do not show, and the persistence-spec features `bmm-publisher` does not read (indexed containers, `ancestor_defs`, `type_ref` value sets), with the form to write instead. It ships a worked example (`assets/openehr_demo_0.1.0.bmm.json`) and a read-only checker, `scripts/check_bmm.py` (standard-library Python 3.8+), that reports what `bmm-publisher` accepts without a message or renders wrongly: a misspelt or missing `_type`, a misspelt key such as `is_mandantory`, a key that differs from its `name`, a class in no package, an upper limit without `"upper_unbounded": false`, a container without an element type, wrong generic parameter counts, a generic argument that does not conform to its parameter's `conforms_to_type`, type shapes the publisher mis-renders (a simple type in a container's `type_def`, a type object in `generic_parameters`, both generic-parameter forms at once, a container in `generic_parameter_defs`), classes more than four package levels deep, and unresolved type names. A run that could not load an included schema with `-d` exits 3, so an incomplete check is never mistaken for a clean one. CI runs its unit tests (`scripts/test_check_bmm.py`). - `scaffold` template set revision 3: a new repository also gets `computable/BMM/.bmm.json` (file id `bmm`, strategy `bmm-seed`). - The script lists the schemas the local `ghcr.io/openehr/bmm-publisher` image bundles (`docker run --pull never`, so nothing is downloaded) and copies the highest version for the component. - When the image bundles none, it writes an empty schema `openehr__`. The schema has one root package `org.openehr.`, no classes, and BASE in `includes` for other components. bmm-publisher refuses a schema without a package. @@ -16,7 +16,7 @@ Notable changes to the plugins in this repository. The format follows [Keep a Ch ### Changed -- `class-generation` and `authoring` send changes to the class model itself to `bmm-authoring`. +- `class-generation` and `authoring` send changes to the class model itself to `bmm-authoring`, and `scaffold` points to it for filling a new repository's empty schema. - `docs/prompting-guide.md`: the class-change prompts name `bmm-authoring` and the checker, and a prompt for a new component's schema is added. ## openehr-specs 0.5.0 (2026-10-08) diff --git a/docs/testing.md b/docs/testing.md index 0c742de..39d1d46 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -22,7 +22,7 @@ For Cursor, copy `plugins/openehr-specs` to `~/.cursor/plugins/local/openehr-spe ## Validation - **Manifest and frontmatter validation**: `python3 scripts/validate.py` (CI runs it on every PR) checks the Claude and Cursor JSON manifests, name and version sync, declared component paths, and the marketplace `owner.name`. For every skill and subagent it checks that `name` and `description` exist and that `name` matches the directory or file name. It also runs the `scaffold` template-set check (`scaffold.py check`: templates exist and render, revisions are contiguous, every revision has a migration file, and the template digest matches the latest revision). It does not check description length, `disable-model-invocation`, the README inventories, or the release tag. -- **Script tests**: `python3 -m unittest discover -s scripts -p 'test_*.py'` (CI runs it on every PR) covers the `scaffold` script (the template engine, each file strategy, variable inference, and the upgrade path across revisions) and the `bmm-authoring` checker (each finding it reports, its exit codes, and a clean result on the skill's worked example), all in temporary directories. Run it after changing either script, the template set, or the worked example. +- **Script tests**: `python3 -m unittest discover -s scripts -p 'test_*.py'` (CI runs it on every PR) covers the `scaffold` script (the template engine, each file strategy, variable inference, and the upgrade path across revisions) and the `bmm-authoring` checker (each finding it reports, its exit codes, a clean result on the skill's worked example, and no traceback for a wrongly typed value anywhere in it), all in temporary directories. Run it after changing either script, the template set, or the worked example. - **Built-in manifest check**: `claude plugin validate plugins/openehr-specs` validates the plugin manifest; add `--strict` to treat warnings as errors. - **Structural validation**: after creating or modifying plugin components, run the `plugin-dev:plugin-validator` agent. It checks `plugin.json`, the marketplace entry, directory layout, and frontmatter. The agent comes from the `plugin-dev` plugin, which this repo enables in `.claude/settings.json`. - **Skill quality review**: run the `plugin-dev:skill-reviewer` agent (also from `plugin-dev`). It checks description triggering quality, progressive disclosure, and content structure. diff --git a/plugins/openehr-specs/skills/bmm-authoring/SKILL.md b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md index 58b8137..c64f059 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/SKILL.md +++ b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md @@ -50,8 +50,9 @@ render and read the tables. ### Changing a schema 1. Read the header and the classes next to the one you are changing, and follow their idiom. Run the - checker once before editing (step 4) and keep its output: the published schemas carry findings - of their own, and that output separates them from yours. + checker once before editing (step 4) and save its output to a scratch file: the published schemas + carry findings of their own, and comparing that output with the one after your change separates + theirs from yours. 2. Make the change, using the format reference. Ground every class, attribute and function name in the specification text being written or in the user's instruction; never invent one. 3. Increase the build number in `schema_revision` (`1.3.0.2` → `1.3.0.3`). If the release is already @@ -63,7 +64,9 @@ render and read the tables. python3 ${CLAUDE_SKILL_DIR}/scripts/check_bmm.py computable/BMM/openehr_rm_1.2.0.bmm.json \ -d ../specifications-BASE/computable/BMM/openehr_base_1.3.0.bmm.json ``` - Fix every ERROR and WARNING that was not in the step 1 output. + Save the output again and `diff` it with the step 1 output; fix every ERROR and WARNING your + change added. Exit status 3 ("name(s) not checked") means an included schema was not loaded, so + the check is incomplete. If its sibling clone is missing, say so; never report such a run as clean. 5. Regenerate the class tables (`class-generation`) and read the table of every class you touched. Expect the gaps listed under "What the class tables show" in the reference, such as a container's member count or a generic parameter's constraint. @@ -77,19 +80,21 @@ render and read the tables. ### Writing a schema from scratch -1. If `computable/BMM/` already holds an empty schema for the component, fill that in. Otherwise - copy `${CLAUDE_SKILL_DIR}/assets/openehr_demo_0.1.0.bmm.json` to - `computable/BMM/.bmm.json` and replace its content. Set the header from the - reference's header table: `schema_name` is the lower-case component, `rm_release` the release, - `schema_revision` `.1`. Ask the user for `schema_author` rather than guessing it. +1. If `computable/BMM/` already holds an empty schema for the component (the `scaffold` skill writes + one), fill that in; replace its single root package `org.openehr.` as step 3 says, + and confirm its `schema_author` and `schema_lifecycle_state` with the user. Otherwise copy + `${CLAUDE_SKILL_DIR}/assets/openehr_demo_0.1.0.bmm.json` to `computable/BMM/.bmm.json` + and replace its content. Set the header from the reference's header table: `schema_name` is the + lower-case component, `rm_release` the release, `schema_revision` `.1`. Ask the user + for `schema_author` rather than guessing it. 2. Include BASE (`"includes": {"openehr_base_": {"id": "openehr_base_"}}`) and use its types (`Any`, `String`, `List`, `Hash`, `HIER_OBJECT_ID`, …). Do not redefine them; `primitive_types` belongs to BASE only. -3. Give each specification document of the component one top-level package, - `org.openehr..`, matching the `:pkg:` that document's `master.adoc` sets - (`org.openehr...`): RM has `org.openehr.rm.common`, `org.openehr.rm.ehr` - and others; TERM has the one package `org.openehr.term.terminology`. Group its classes in - sub-packages as the document's chapters do. +3. Name top-level packages `org.openehr..`, one for each `:pkg:` the + component's `master.adoc` files set (`org.openehr...`): RM has + `org.openehr.rm.common`, `org.openehr.rm.ehr` and others, and its `ehr` document sets two; TERM + has the one package `org.openehr.term.terminology`. Group the classes in sub-packages as the + chapters do, at most four package levels deep. 4. Add the classes, then continue from step 4 of "Changing a schema". ## Quick Reference @@ -116,14 +121,16 @@ render and read the tables. argument does not conform to its parameter's `conforms_to_type`. WARNING means a broken convention or content the publisher ignores. INFO lists what could not be checked, typically type names from an included schema that was not passed with `-d`. -- Exit status 1 means errors were found (with `--strict`, warnings count too); 2 means the file is - unreadable or not JSON. -- The published schemas carry some warnings and a few errors of their own, such as missing - documentation, or AM 2.4.0's `P_ARCHETYPE_SLOT.includes`, a `List` with no element type that renders - as `List`. Report findings outside your change to the user; do not fix them as a side effect. -- If `python3` is missing, say so, check the change by hand against the reference's lists, and - render. In a tool that does not expand `${CLAUDE_SKILL_DIR}`, use the directory that holds this - SKILL.md. +- Exit status: 0 complete and clean; 1 errors found (with `--strict`, warnings count too); 3 no + errors, but names from an included schema not loaded with `-d` went unchecked; 2 a file is + unreadable, not UTF-8 JSON, or (for `-d`) not a BMM schema. +- The published schemas carry findings of their own. At the time of writing, AM 2.4.0 has ten errors + (lists without an element type in `P_ARCHETYPE_SLOT`, and `FUNCTION` arguments in reverse order in + `c_conforms_to`) and LANG's BMM3 overlay two. Report findings outside your change to the user; do + not fix them as a side effect. +- If `python3` is missing, say so and report the schema as unchecked; review the change by hand + against the reference's lists before rendering. In a tool that does not expand + `${CLAUDE_SKILL_DIR}`, use the directory that holds this SKILL.md. ## Guardrails @@ -132,6 +139,7 @@ render and read the tables. 4-space indentation and key order, and check that `git diff` shows only your change. - **Edit only `computable/BMM/` in the component repo.** Never the ITS-BMM copies, the generated `docs/UML/classes` tables, or the schemas bundled in `bmm-publisher`. -- **Keys equal names.** Every map entry is keyed by its own `name`. +- **Keys equal names.** Every entry of a map of named things (packages, classes, properties, + functions, parameters, constants, generic parameters) is keyed by its own `name`. - **Use the forms bmm-publisher reads** (see the reference's table of unsupported specification features) and describe in `documentation` what they cannot express. diff --git a/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md index 6434447..9e5fb27 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md +++ b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md @@ -3,16 +3,20 @@ The openEHR BMM schemas in `specifications-XX/computable/BMM/*.bmm.json` are JSON serialisations of the `P_BMM_*` classes of the [BMM Persistence specification](https://specifications.openehr.org/releases/LANG/development/bmm_persistence.html) -(LANG). The specification shows its examples in ODIN, and says JSON with type markers may be used -instead. Its class tables (chapter "Persistence Model") are the normative list of attributes; this -reference was checked against the development version linked here. This -reference describes the JSON form that the published schemas use and that -[`bmm-publisher`](https://github.com/openEHR/bmm-publisher) reads, through its parser library -[`cadasto/openehr-bmm`](https://github.com/Cadasto/openehr-bmm). Where the specification and the -toolchain differ, both are stated. +(LANG, development version; [Markdown](https://specifications.openehr.org/releases/LANG/development/bmm_persistence.md)). +Its chapter "Persistence Package" holds the class tables, the normative list of attributes; its +chapter "BMM Persistence Syntax" names JSON as the primary format of +[`bmm-publisher`](https://github.com/openEHR/bmm-publisher) and shows the rules, mostly in ODIN. +P_BMM version 2.4 added functions, constants and invariants to the structural features. -`assets/openehr_demo_0.1.0.bmm.json` is a complete worked example that has one of each construct. -It passes `scripts/check_bmm.py` and renders with `bmm-publisher`. +This reference describes the JSON that the published schemas use and that bmm-publisher reads, +through its parser library [`cadasto/openehr-bmm`](https://github.com/Cadasto/openehr-bmm). Where +the specification and the toolchain differ, both are stated. It was checked against bmm-publisher +0.12.0 (parser 0.3.0); the published-schema counts and defects named here are those found at the +time of writing. + +`assets/openehr_demo_0.1.0.bmm.json` is a complete worked example with one each of the common +constructs. It passes `scripts/check_bmm.py` and renders with `bmm-publisher`. ## Contents @@ -28,44 +32,49 @@ It passes `scripts/check_bmm.py` and renders with `bmm-publisher`. - [Documentation text](#documentation-text) - [What the class tables show](#what-the-class-tables-show) - [Specification features bmm-publisher does not read](#specification-features-bmm-publisher-does-not-read) -- [Reading the specification's ODIN examples](#reading-the-specifications-odin-examples) +- [Reading the specification's ODIN and YAML examples](#reading-the-specifications-odin-and-yaml-examples) ## The file - One file per component release, named after its schema id: `__.bmm.json`, - for example `openehr_rm_1.2.0.bmm.json`. -- The working copy is in the component's `specifications-XX` repo under `computable/BMM/`. A push - there updates [`specifications-ITS-BMM`](https://github.com/openEHR/specifications-ITS-BMM), which - publishes the JSON and the ODIN and YAML forms it generates from it. Never edit the ITS-BMM copies. -- Every key that holds a map of named things (`packages`, `class_definitions`, `properties`, - `functions`, `parameters`, `constants`, `generic_parameter_defs`) is keyed by the item's own - `name`. bmm-publisher uses `name` and ignores the key, so a key that differs from `name` goes - unnoticed. + for example `openehr_rm_1.2.0.bmm.json`. (LANG also carries `openehr_lang_1.1.0-bmm3.bmm.json`, an + overlay with the same schema id; the checker flags its name.) +- The working copy is in the component's `specifications-XX` repo under `computable/BMM/`. + [`specifications-ITS-BMM`](https://github.com/openEHR/specifications-ITS-BMM) publishes the JSON + with the ODIN and YAML forms it generates from it, never edited by hand. A component repo that + carries ITS-BMM's notify workflow (its `.github/sender-workflow-example.yml`) sends each change + on a push to `master`; without it, the update is made in ITS-BMM. +- Every map of named things (`packages`, `class_definitions`, `properties`, `functions`, + `parameters`, `constants`, `generic_parameter_defs`) is keyed by the item's own `name`, as the + specification requires. Keep the two equal: the parser keeps the key for function `parameters` + (the parameter documentation in the table prints it) and for a type's `generic_parameter_defs`, and + uses `name` everywhere else. ## Header `P_BMM_SCHEMA` and its parent `BMM_SCHEMA_CORE` define the header. bmm-publisher refuses a file that -lacks a field marked *yes*; the specification alone requires those marked *yes (spec)*. +lacks a field marked *yes*. | Field | Required | Value | |-------|----------|-------| -| `bmm_version` | yes (spec) | `"2.4"` in every published schema, and what bmm-publisher writes | +| `bmm_version` | spec | the P_BMM version, `"2.4"`. bmm-publisher assumes `2.4` when it is missing and writes back what it read | | `rm_publisher` | yes | `"openehr"` | | `schema_name` | yes | lower-case component name: `base`, `rm`, `am`, `lang`, `term` | | `rm_release` | yes | 3-part release, `"1.2.0"`; part of the schema id and the file name | | `schema_revision` | yes | `.`, for example `"1.2.0.2"`. Increase the build number with every change: ITS-BMM expects each schema change to bump it | -| `schema_lifecycle_state` | yes | the specification defines no value set. Published schemas use `"stable"`; for an unreleased schema, use the lower-case lifecycle state of the specification (`development`, `trial`; see the `governance` skill) | +| `schema_lifecycle_state` | yes | no value set is defined, and every published schema says `"stable"`. For an unreleased schema, suggest the specification's lifecycle state in lower case (`development`, `trial`; see the `governance` skill) and confirm it with the user | | `schema_description` | yes | one sentence | | `schema_author` | yes | the published schemas use `Name `; ask the user who to name | | `includes` | no | see below | | `packages` | yes | see below | | `primitive_types` | no | class definitions of primitive types; only BASE has them | -| `class_definitions` | yes (spec) | all other class definitions | +| `class_definitions` | no | all other class definitions; optional in the model, present in every published schema | `BMM_SCHEMA_CORE` also defines `schema_contributors` and the archetype hints `archetype_parent_class`, `archetype_data_value_parent_class`, `archetype_rm_closure_packages` and -`archetype_visualise_descendants_of`. bmm-publisher does not read them, so they are absent from -every generated output, including the ODIN and YAML forms. +`archetype_visualise_descendants_of`, and the specification's ODIN header example shows a +`model_name`. bmm-publisher reads none of them, so they are absent from every generated output, +including the ODIN and YAML forms. A changed release (a new `rm_release`) is a new file with a new schema id; while a release is still in development, edit its file and increase the build number in `schema_revision`. @@ -101,12 +110,14 @@ in the model, but bmm-publisher does not resolve it: pass the included file with - Every class definition must be listed in exactly one package: bmm-publisher writes tables per package, so an unlisted class gets no table, and no message. A listed class that is not defined stops the run with `Class X not found in schema`. -- Each specification document of a component has one top-level package, - `org.openehr..`, and its `master.adoc` sets `:pkg:` to that name plus a - dot: RM has `org.openehr.rm.common`, `org.openehr.rm.ehr` and more, BASE has - `org.openehr.base.base_types`, `org.openehr.base.foundation_types` and - `org.openehr.base.resource`, TERM has `org.openehr.term.terminology`. Sub-packages group the - classes as the document's chapters do. +- bmm-publisher visits packages four levels deep (top-level package = 1). Classes deeper than that + get no table, again without a message. +- Top-level packages are named `org.openehr..`, and a specification + document's `master.adoc` sets `:pkg:` to one of them plus a dot before the chapters that include + its tables. A document can cover several: RM's `ehr` document sets `org.openehr.rm.ehr.` and then + `org.openehr.rm.composition.`. BASE has `org.openehr.base.base_types`, + `org.openehr.base.foundation_types` and `org.openehr.base.resource`; TERM has + `org.openehr.term.terminology`. Sub-packages group the classes as the chapters do. - The legacy class-table file name is the first four parts of the class's package path, then the class name in lower case: `LOCATABLE` in `org.openehr.rm.common` → `archetyped` gives `org.openehr.rm.common.locatable.adoc`, which a chapter includes as `{pkg}locatable.adoc`. @@ -126,6 +137,9 @@ A class definition is one of four kinds, chosen by `_type` (omitted for a plain | `P_BMM_ENUMERATION_STRING`, `P_BMM_ENUMERATION_INTEGER` | `name`, `documentation`, `ancestors`, `item_names`, `item_values`, `item_documentations`, `functions` | | `P_BMM_INTERFACE` | `name`, `documentation`, `functions` | +Any other key on an enumeration or interface (`properties`, `invariants`, `ancestors` on an +interface) is dropped. + - **`ancestors`** is a list of class names. A root class names `Any`, or has no `ancestors`; both forms occur in RM. - **Generic class**: declare each parameter, with an optional constraint: @@ -140,7 +154,8 @@ A class definition is one of four kinds, chosen by `_type` (omitted for a plain specification's `ancestor_defs` is not read. - **Enumerations**: `item_names` are the symbolic names. `item_values` (optional) gives one value per name, all strings or all integers by kind. `item_documentations` gives one text per name, by - position. The persistence model requires `String` or `Integer` among the `ancestors`. + position. The table prints integer values only when `Integer` is among the `ancestors` (the + parser assumes `["Integer"]` when `ancestors` is missing), and never prints string values. ## Properties @@ -164,12 +179,12 @@ property with a `type_def` then silently becomes type `Any`. `root_type: "Hash"`, as in BASE and RM. `container_type` takes one-parameter containers only: `List`, `Set`, `Array`. A generic property has no `cardinality`, so state a member count as an invariant, for example `"Codes_valid": "not codes.is_empty"`. -- **Generic arguments** bind in the order the root class declares its `generic_parameter_defs`, and - each must conform to that parameter's `conforms_to_type`; neither is checked by bmm-publisher. - BASE declares `FUNCTION`, so a function taking two strings and returning a Boolean - is `FUNCTION, Boolean>`. +- **Generic arguments** bind in the order the root class declares its `generic_parameter_defs`, + and each must conform to that parameter's `conforms_to_type`; bmm-publisher checks neither. BASE + declares `Hash` and `FUNCTION`, so a function taking two + strings and returning a Boolean is `FUNCTION, Boolean>`. - **`default`** (single properties) gives a default value, `false` or `"active"` for example; the - table shows it as `{default = …}`. + table shows it as `{default = …}`. It is a toolchain addition, not a P_BMM attribute. ## Types @@ -178,13 +193,14 @@ A type is a class name (`"type": "String"`) or a type object. The rules for type - Directly inside a property or parameter, `type_def` has **no** `_type`; its kind follows from the property's `_type`. - Everywhere else (a function `result`, an entry of `generic_parameter_defs`, a nested `type_def` - inside a container type) the object **must** carry `_type`, or it is read as a simple type. + inside a container type) the object carries `_type`, as the specification requires where the + subtype is ambiguous. Without it, the object is read as a simple type. | `_type` | Keys | |---------|------| | `P_BMM_SIMPLE_TYPE` | `type` | -| `P_BMM_CONTAINER_TYPE` | `container_type`, then either `type` (a name) or `type_def` (a nested type object) | -| `P_BMM_GENERIC_TYPE` | `root_type`, then `generic_parameters` (a list of names) or `generic_parameter_defs` (nested type objects keyed by parameter name), with one entry per parameter of the root class | +| `P_BMM_CONTAINER_TYPE` | `container_type`, then either `type` (a name) or `type_def` (a nested container or generic type) | +| `P_BMM_GENERIC_TYPE` | `root_type`, then either `generic_parameters` or `generic_parameter_defs`, with one entry per parameter of the root class | Nesting, `replaced: List>`: @@ -199,7 +215,17 @@ Nesting, `replaced: List>`: } ``` -A container type with neither `type` nor `type_def` renders as `List`. +bmm-publisher renders some valid nestings wrongly, so keep to these forms: + +- A simple element type goes in the container's `type`; a `P_BMM_SIMPLE_TYPE` in its `type_def` + renders as `List`, and so does a container with neither. +- `generic_parameters` holds names, or nested generic types as BASE writes them + (`FUNCTION, Boolean>` in `Container.for_all`). A simple or container type object there + stops bmm-publisher with a type error. +- Give `generic_parameters` or `generic_parameter_defs`, not both: with both, the defs are ignored. +- `generic_parameter_defs` entries are printed in order, keyed by the root class's parameter names + in their declared order. A `P_BMM_CONTAINER_TYPE` there prints nothing, although the + specification allows it; write the container as a generic type with `root_type: "List"`. ## Functions @@ -223,20 +249,21 @@ A container type with neither `type` nor `type_def` renders as `List`. ``` - Function keys: `name`, `documentation`, `parameters`, `result`, `pre_conditions`, - `post_conditions`, `aliases` (list of names), `is_abstract`, `is_nullable`. -- `result` is a type object with `_type`. A procedure returns `{"_type": "P_BMM_SIMPLE_TYPE", "type": "void"}`. + `post_conditions`, `aliases`, `is_abstract`, `is_nullable`. +- `result` is a type object with `_type`. The specification reads a function with no `result` as a + procedure, but the table then prints an empty result type, so the published schemas write + `{"_type": "P_BMM_SIMPLE_TYPE", "type": "void"}`. - `is_nullable: true` on a function means it may return `Void` (the table shows `0..1`); on a parameter it makes the parameter optional. Without it, every parameter shows as mandatory (`[1]`). - Parameter kinds mirror the property kinds: `P_BMM_SINGLE_FUNCTION_PARAMETER` (`type`), `P_BMM_SINGLE_FUNCTION_PARAMETER_OPEN` (`type` is a class parameter such as `T`), - `P_BMM_CONTAINER_FUNCTION_PARAMETER` (`type_def` and a **required** `cardinality`: bmm-publisher - fails without one), `P_BMM_GENERIC_FUNCTION_PARAMETER` (`type_def`). + `P_BMM_CONTAINER_FUNCTION_PARAMETER` (`type_def`, and an optional `cardinality` that defaults to + `0..*`), `P_BMM_GENERIC_FUNCTION_PARAMETER` (`type_def`). +- `aliases` is a `Hash` in the P_BMM model, but the published schemas and the parser + use a list of names, as BASE's `Any.equal` does: `"aliases": ["=", "=="]`. ## Invariants, constants and conditions -All three are part of `P_BMM_CLASS` and `P_BMM_FUNCTION` in the specification's class tables, though -its syntax chapter does not show them. - ```json "invariants": { "Name_valid": "not name.is_empty()" }, "constants": { @@ -244,11 +271,13 @@ its syntax chapter does not show them. } ``` -- `invariants`, `pre_conditions` and `post_conditions` map a tag to an expression string. The - published schemas mostly tag invariants `Xxx_valid`, pre-conditions `Pre_xxx` and - post-conditions `Post_xxx`. -- A constant needs `type`, and bmm-publisher fails without one. `value` is the value as source - text: `"32"`, the name of another constant, `"\"..\""` for a string, `"'*'"` for a character. +- `invariants`, `pre_conditions` and `post_conditions` map a tag to an assertion string, which + P_BMM treats as opaque text (usually openEHR BEL). The published schemas mostly tag invariants + `Xxx_valid` (some `Xxx_validity`), and conditions `Pre`/`Post`, or `Pre_xxx`/`Post_xxx` when a + function has several. +- A constant needs `type`, and bmm-publisher fails without one. `value` is the literal in serialised + form: `"32"`, `"\"..\""` for a string, `"'*'"` for a character, or the name of another constant; + BASE also writes numbers as JSON numbers (`60`, `30.42`). ## Documentation text @@ -256,12 +285,12 @@ its syntax chapter does not show them. written as in the published schemas: `` `CLASS_NAME` ``, `` `_attribute_` ``, a blank line written as `\n\n`. bmm-publisher then: -- escapes `{…}`, so an attribute reference such as `{base_release}` is printed literally; write the - value instead; +- escapes `{word}` (letters, digits, `_` and `.`) and any `{` after a space, so an attribute + reference such as `{base_release}` is printed literally; write the value instead; - passes a same-document cross-reference `<<_x_class,X>>` through; -- escapes `|` and `<=`, and every `*` on a line that holds two or more, so they print literally (no - bold); -- leaves a single `*` alone, so `0..*` is safe. +- escapes `|` and `<=`; +- escapes every `*` on a line that holds two or more, so they print literally (no bold), except a + leading list marker `* `; a single `*`, as in `0..*`, is left alone. ## What the class tables show @@ -273,8 +302,8 @@ The legacy class tables (the `docs/UML/classes` layout) leave some of the model - A generic class heading shows `DEMO_BOX` without the `conforms_to_type` constraint. - The inheritance row names the root class only, so a closed binding such as `` shows only where `documentation` states it. -- An enumeration shows no inheritance row; its items are listed under a "Constants" heading, with - an empty value column when there are no `item_values`. +- An enumeration shows no inheritance row; its items are listed under a "Constants" heading, as + bare names for a string enumeration and as `name: Integer = value` for an integer one. - A function row shows `1..1`, or `0..1` when `is_nullable`; its pre- and post-conditions are printed in the signature cell. - The header (`schema_revision`, `schema_lifecycle_state`, `includes`) appears nowhere. @@ -286,24 +315,25 @@ the alternative. | Specification feature | What bmm-publisher does | Write instead | |-----------------------|--------------------------|---------------| -| `P_BMM_INDEXED_CONTAINER_PROPERTY` / `_TYPE` (`index_type`) | property of type `Any` | `P_BMM_GENERIC_PROPERTY` with `root_type: "Hash"`, and an invariant for any member count | -| `ancestor_defs` (generic inheritance) | ignored; the class has no parent | `ancestors` with the root class (see [Classes](#classes)) | +| `P_BMM_INDEXED_CONTAINER_PROPERTY` | property of type `Any` | `P_BMM_GENERIC_PROPERTY` with `root_type: "Hash"`, and an invariant for any member count | +| `P_BMM_INDEXED_CONTAINER_TYPE`, or `index_type` on a container type | a simple type, or the container without its index | the same | +| `ancestor_defs` (generic inheritance) | ignored; a parent named only there is lost | `ancestors` with the root class (see [Classes](#classes)) | | `type_ref` with `value_constraint` (value sets) | ignored; with no `type`, the property is `Any` | `type`, and name the value set in `documentation` | | `is_computed`, `is_im_infrastructure`, `is_im_runtime` | ignored | describe the property in `documentation` | | `is_override` (class) | ignored | — | -| `model_name`, `schema_contributors`, `archetype_*` (header) | ignored | — | +| `schema_contributors`, `archetype_*` (header) | ignored | — | | `P_BMM_OPEN_TYPE` | read as `P_BMM_SIMPLE_TYPE` | `P_BMM_SIMPLE_TYPE` | A misspelt key (`is_mandantory`) or `_type` is ignored in the same way, without a message. -## Reading the specification's ODIN examples - -| ODIN | JSON | -|------|------| -| `["items"] = (P_BMM_CONTAINER_PROPERTY) < … >` | `"items": { "_type": "P_BMM_CONTAINER_PROPERTY", … }` | -| `name = <"items">` | `"name": "items"` | -| `ancestors = <"Any", "ITEM">` | `"ancestors": ["Any", "ITEM"]` | -| `is_mandatory = ` | `"is_mandatory": true` | -| `cardinality = <\|>=1\|>` | `"cardinality": {"lower": 1, "upper_unbounded": true}` | -| `cardinality = <\|2..5\|>` | `"cardinality": {"lower": 2, "upper": 5, "upper_unbounded": false}` | -| `includes = < ["1"] = < id = <"x"> > >` | `"includes": {"x": {"id": "x"}}` | +## Reading the specification's ODIN and YAML examples + +| ODIN | YAML | JSON | +|------|------|------| +| `["items"] = (P_BMM_CONTAINER_PROPERTY) < … >` | `items: !P_BMM_CONTAINER_PROPERTY` | `"items": { "_type": "P_BMM_CONTAINER_PROPERTY", … }` | +| `name = <"items">` | `name: items` | `"name": "items"` | +| `ancestors = <"Any", "ITEM">` | `ancestors: [Any, ITEM]` | `"ancestors": ["Any", "ITEM"]` | +| `is_mandatory = ` | `is_mandatory: true` | `"is_mandatory": true` | +| `cardinality = <\|>=1\|>` | — | `"cardinality": {"lower": 1, "upper_unbounded": true}` | +| `cardinality = <\|2..5\|>` | — | `"cardinality": {"lower": 2, "upper": 5, "upper_unbounded": false}` | +| `includes = < ["1"] = < id = <"x"> > >` | — | `"includes": {"x": {"id": "x"}}` | diff --git a/plugins/openehr-specs/skills/scaffold/SKILL.md b/plugins/openehr-specs/skills/scaffold/SKILL.md index 4f04c92..b51b10c 100644 --- a/plugins/openehr-specs/skills/scaffold/SKILL.md +++ b/plugins/openehr-specs/skills/scaffold/SKILL.md @@ -98,7 +98,8 @@ Existing repositories are not offered a schema. `AGENTS.md`), offer to fill it from the repo's content and delete the comment once it is filled. When `manifest.json` was created, point out that its `specifications` list starts empty (skill `openehr-specs:authoring`). When an empty BMM schema was created, point out that classes go into it - and the class tables are generated from it (skill `openehr-specs:class-generation`); until then + (skill `openehr-specs:bmm-authoring`, which also checks it) and the class tables are generated + from it (skill `openehr-specs:class-generation`); until then bmm-publisher warns about the empty package. Do not commit; if the user asks for a commit message, follow the Conventions section of the repo's `AGENTS.md`. From 3845f6b77c5d29ed5cb36da749cd5ec239604f19 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Fri, 9 Oct 2026 16:08:43 +0200 Subject: [PATCH 4/6] fix(openehr-specs): address the re-review of check_bmm and its reference - A class with an unknown _type has its keys checked as the plain class bmm-publisher reads it as, so a near-miss such as P_BMM_INTERFACES no longer reports its properties as lost. - While an included schema is not loaded, a name close to a local class (ARCHETYPE_ID next to ARCHETYPED) is counted as unchecked, with the suggestion in the INFO line, instead of a false warning. - The "incomplete" notice and the unchecked count are printed whatever the exit status, so a run with errors still says it was incomplete. - Input nested too deeply to read or check exits 2 without a traceback. - An empty ancestor_defs loses no parent and is no longer an error. - Reference: maps keyed by name versus a type's generic_parameter_defs, the Required legend, nested container types, binding order, condition tags, and constant values as the P_BMM model types them. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- .../skills/bmm-authoring/SKILL.md | 9 ++- .../bmm-authoring/references/p-bmm-json.md | 33 +++++---- .../skills/bmm-authoring/scripts/check_bmm.py | 71 +++++++++++-------- scripts/test_check_bmm.py | 49 ++++++++++++- 5 files changed, 113 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a2c776..194df9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ Notable changes to the plugins in this repository. The format follows [Keep a Ch ### Added -- `bmm-authoring` skill: write a component's BMM schema (the P_BMM JSON in `computable/BMM/`) from scratch, or change one. It covers the header and its conventions, `includes`, how packages set the class-table file names, every class, property, type and function kind, invariants and constants, documentation text, what the legacy class tables do not show, and the persistence-spec features `bmm-publisher` does not read (indexed containers, `ancestor_defs`, `type_ref` value sets), with the form to write instead. It ships a worked example (`assets/openehr_demo_0.1.0.bmm.json`) and a read-only checker, `scripts/check_bmm.py` (standard-library Python 3.8+), that reports what `bmm-publisher` accepts without a message or renders wrongly: a misspelt or missing `_type`, a misspelt key such as `is_mandantory`, a key that differs from its `name`, a class in no package, an upper limit without `"upper_unbounded": false`, a container without an element type, wrong generic parameter counts, a generic argument that does not conform to its parameter's `conforms_to_type`, type shapes the publisher mis-renders (a simple type in a container's `type_def`, a type object in `generic_parameters`, both generic-parameter forms at once, a container in `generic_parameter_defs`), classes more than four package levels deep, and unresolved type names. A run that could not load an included schema with `-d` exits 3, so an incomplete check is never mistaken for a clean one. CI runs its unit tests (`scripts/test_check_bmm.py`). +- `bmm-authoring` skill: write a component's BMM schema (the P_BMM JSON in `computable/BMM/`) from scratch, or change one. It covers the header and its conventions, `includes`, how packages set the class-table file names, every class, property, type and function kind, invariants and constants, documentation text, what the legacy class tables do not show, and the persistence-spec features `bmm-publisher` does not read (indexed containers, `ancestor_defs`, `type_ref` value sets), with the form to write instead. It ships a worked example (`assets/openehr_demo_0.1.0.bmm.json`) and a read-only checker, `scripts/check_bmm.py` (standard-library Python 3.8+), that reports what `bmm-publisher` accepts without a message or renders wrongly: a misspelt or missing `_type`, a misspelt key such as `is_mandantory`, a key that differs from its `name`, a class in no package, an upper limit without `"upper_unbounded": false`, a container without an element type, wrong generic parameter counts, a generic argument that does not conform to its parameter's `conforms_to_type`, type shapes the publisher mis-renders (a simple type in a container's `type_def`, a type object in `generic_parameters`, both generic-parameter forms at once, a container in `generic_parameter_defs`), classes more than four package levels deep, and unresolved type names. A run that could not load an included schema with `-d` reports how many names went unchecked and exits 3 when it found no errors, so an incomplete check is never mistaken for a clean one. CI runs its unit tests (`scripts/test_check_bmm.py`). - `scaffold` template set revision 3: a new repository also gets `computable/BMM/.bmm.json` (file id `bmm`, strategy `bmm-seed`). - The script lists the schemas the local `ghcr.io/openehr/bmm-publisher` image bundles (`docker run --pull never`, so nothing is downloaded) and copies the highest version for the component. - When the image bundles none, it writes an empty schema `openehr__`. The schema has one root package `org.openehr.`, no classes, and BASE in `includes` for other components. bmm-publisher refuses a schema without a package. diff --git a/plugins/openehr-specs/skills/bmm-authoring/SKILL.md b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md index c64f059..dab523a 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/SKILL.md +++ b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md @@ -65,8 +65,10 @@ render and read the tables. -d ../specifications-BASE/computable/BMM/openehr_base_1.3.0.bmm.json ``` Save the output again and `diff` it with the step 1 output; fix every ERROR and WARNING your - change added. Exit status 3 ("name(s) not checked") means an included schema was not loaded, so - the check is incomplete. If its sibling clone is missing, say so; never report such a run as clean. + change added. A non-zero "N name(s) not checked" in the summary (with an "incomplete" notice) + means an included schema was not loaded: the exit status is 3 when there are no errors, 1 when + there are. If the sibling clone is missing, say the check is incomplete; never report such a run + as clean. 5. Regenerate the class tables (`class-generation`) and read the table of every class you touched. Expect the gaps listed under "What the class tables show" in the reference, such as a container's member count or a generic parameter's constraint. @@ -123,7 +125,8 @@ render and read the tables. an included schema that was not passed with `-d`. - Exit status: 0 complete and clean; 1 errors found (with `--strict`, warnings count too); 3 no errors, but names from an included schema not loaded with `-d` went unchecked; 2 a file is - unreadable, not UTF-8 JSON, or (for `-d`) not a BMM schema. + unreadable, not UTF-8 JSON, nested too deeply, or (for `-d`) not a BMM schema. The summary line + always gives the unchecked count. - The published schemas carry findings of their own. At the time of writing, AM 2.4.0 has ten errors (lists without an element type in `P_ARCHETYPE_SLOT`, and `FUNCTION` arguments in reverse order in `c_conforms_to`) and LANG's BMM3 overlay two. Report findings outside your change to the user; do diff --git a/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md index 9e5fb27..45405cb 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md +++ b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md @@ -45,10 +45,11 @@ constructs. It passes `scripts/check_bmm.py` and renders with `bmm-publisher`. carries ITS-BMM's notify workflow (its `.github/sender-workflow-example.yml`) sends each change on a push to `master`; without it, the update is made in ITS-BMM. - Every map of named things (`packages`, `class_definitions`, `properties`, `functions`, - `parameters`, `constants`, `generic_parameter_defs`) is keyed by the item's own `name`, as the - specification requires. Keep the two equal: the parser keeps the key for function `parameters` - (the parameter documentation in the table prints it) and for a type's `generic_parameter_defs`, and - uses `name` everywhere else. + `parameters`, `constants`, a class's `generic_parameter_defs`) is keyed by the item's own `name`, as + the specification requires. Keep the two equal: the parser keeps the key for function `parameters` + (the parameter documentation in the table prints it) and uses `name` everywhere else. A type's + `generic_parameter_defs` holds type objects, which have no `name`: key them by the root class's + parameter names (see [Types](#types)). ## Header @@ -57,7 +58,7 @@ lacks a field marked *yes*. | Field | Required | Value | |-------|----------|-------| -| `bmm_version` | spec | the P_BMM version, `"2.4"`. bmm-publisher assumes `2.4` when it is missing and writes back what it read | +| `bmm_version` | no (the specification requires it) | the P_BMM version, `"2.4"`. bmm-publisher assumes `2.4` when it is missing and writes back what it read | | `rm_publisher` | yes | `"openehr"` | | `schema_name` | yes | lower-case component name: `base`, `rm`, `am`, `lang`, `term` | | `rm_release` | yes | 3-part release, `"1.2.0"`; part of the schema id and the file name | @@ -199,7 +200,7 @@ A type is a class name (`"type": "String"`) or a type object. The rules for type | `_type` | Keys | |---------|------| | `P_BMM_SIMPLE_TYPE` | `type` | -| `P_BMM_CONTAINER_TYPE` | `container_type`, then either `type` (a name) or `type_def` (a nested container or generic type) | +| `P_BMM_CONTAINER_TYPE` | `container_type`, then either `type` (a name) or `type_def` (a nested generic type; the parser also accepts a nested container type, which the P_BMM model does not) | | `P_BMM_GENERIC_TYPE` | `root_type`, then either `generic_parameters` or `generic_parameter_defs`, with one entry per parameter of the root class | Nesting, `replaced: List>`: @@ -223,9 +224,10 @@ bmm-publisher renders some valid nestings wrongly, so keep to these forms: (`FUNCTION, Boolean>` in `Container.for_all`). A simple or container type object there stops bmm-publisher with a type error. - Give `generic_parameters` or `generic_parameter_defs`, not both: with both, the defs are ignored. -- `generic_parameter_defs` entries are printed in order, keyed by the root class's parameter names - in their declared order. A `P_BMM_CONTAINER_TYPE` there prints nothing, although the - specification allows it; write the container as a generic type with `root_type: "List"`. +- bmm-publisher prints `generic_parameter_defs` entries in the order written and ignores their keys, + so write them in the root class's declared order, each keyed by its parameter name. A + `P_BMM_CONTAINER_TYPE` there prints nothing, although the specification allows it; write the + container as a generic type with `root_type: "List"`. ## Functions @@ -251,8 +253,8 @@ bmm-publisher renders some valid nestings wrongly, so keep to these forms: - Function keys: `name`, `documentation`, `parameters`, `result`, `pre_conditions`, `post_conditions`, `aliases`, `is_abstract`, `is_nullable`. - `result` is a type object with `_type`. The specification reads a function with no `result` as a - procedure, but the table then prints an empty result type, so the published schemas write - `{"_type": "P_BMM_SIMPLE_TYPE", "type": "void"}`. + procedure, but the table then prints an empty result type, so the published schemas almost always + write `{"_type": "P_BMM_SIMPLE_TYPE", "type": "void"}`. - `is_nullable: true` on a function means it may return `Void` (the table shows `0..1`); on a parameter it makes the parameter optional. Without it, every parameter shows as mandatory (`[1]`). - Parameter kinds mirror the property kinds: `P_BMM_SINGLE_FUNCTION_PARAMETER` (`type`), @@ -273,11 +275,12 @@ bmm-publisher renders some valid nestings wrongly, so keep to these forms: - `invariants`, `pre_conditions` and `post_conditions` map a tag to an assertion string, which P_BMM treats as opaque text (usually openEHR BEL). The published schemas mostly tag invariants - `Xxx_valid` (some `Xxx_validity`), and conditions `Pre`/`Post`, or `Pre_xxx`/`Post_xxx` when a - function has several. + `Xxx_valid` (some `Xxx_validity`), and conditions `Pre`/`Post` or `Pre_xxx`/`Post_xxx`; a function + with several conditions gives each its own suffix. - A constant needs `type`, and bmm-publisher fails without one. `value` is the literal in serialised - form: `"32"`, `"\"..\""` for a string, `"'*'"` for a character, or the name of another constant; - BASE also writes numbers as JSON numbers (`60`, `30.42`). + form: `"32"`, `"\"..\""` for a string, `"'*'"` for a character, or the name of another constant. + P_BMM types `value` as a String; BASE also writes numbers as JSON numbers (`60`, `30.42`), which + bmm-publisher accepts. ## Documentation text diff --git a/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py b/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py index 4fa5d3a..440d652 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py +++ b/plugins/openehr-specs/skills/bmm-authoring/scripts/check_bmm.py @@ -16,8 +16,9 @@ does not read. INFO: what could not be checked. Exit status: 0 when the check is complete and found no ERROR (with --strict, no WARNING either); 1 when it found an ERROR (with --strict, or a WARNING); 3 when it found none but could not check -every name, because an included schema was not loaded with -d; 2 when a file cannot be read, is not -UTF-8 JSON, or (for -d) is not a BMM schema. +every name, because an included schema was not loaded with -d (with errors as well, the status is 1, +and the summary and an "incomplete" notice still say how many names went unchecked); 2 when a file +cannot be read, is not UTF-8 JSON, is nested too deeply, or (for -d) is not a BMM schema. """ import argparse import difflib @@ -113,6 +114,7 @@ def __init__(self, schema_path, dependency_paths=()): self.classes = {} # class name -> summary (see summarise), for this schema self.class_paths = {} # class name -> JSON path of its definition self.unchecked = set() # type names an included but unloaded schema might define + self.suggestions = {} # unchecked name -> a close name of a loaded class self.missing_includes = set() self.dependencies = {} # schema id -> {class name: summary} own_id = schema_id(self.data) if isinstance(self.data, dict) else None @@ -162,8 +164,10 @@ def run(self): self.error(path, "class is not listed in any package, so no class table is generated for it") self.check_ancestor_cycles(d) if self.unchecked: + names = ["%s (or %s?)" % (n, self.suggestions[n]) if n in self.suggestions else n + for n in sorted(self.unchecked)] self.info("/includes", "not checked, as no loaded schema defines them; pass each included " - "schema with -d: " + ", ".join(sorted(self.unchecked))) + "schema with -d: " + ", ".join(names)) return self def class_blocks(self, d): @@ -309,8 +313,9 @@ def check_package(self, pkg, key, path, listed, depth): self.check_package(sub, sub_key, path + "/packages/" + sub_key, listed, depth + 1) def check_class(self, cls, path): - kind = self.kind_of(cls, path, CLASS_TYPES, "P_BMM_CLASS", "a plain class") - self.check_keys(cls, kind, path) + kind, recognised = self.kind_of(cls, path, CLASS_TYPES, "P_BMM_CLASS", "a plain class") + # with an unknown _type bmm-publisher reads a plain class, so that is what decides which keys it drops + self.check_keys(cls, kind if recognised else "P_BMM_CLASS", path) self.check_documentation(cls, path, required=True) scope = {} # generic parameter name -> its conforms_to_type, or None params = cls.get("generic_parameter_defs") @@ -378,12 +383,12 @@ def check_ancestor_defs(self, cls, ancestors, path): elif isinstance(key, str): roots.append(key.split("<")[0].strip()) lost = [r for r in roots if r not in ancestors] - if not ancestors or lost: - self.error(path + "/ancestor_defs", "bmm-publisher reads only \"ancestors\", so %s lost; list " - "the root class name there (for an open binding such as " - "A, also redeclare T in generic_parameter_defs; state a " - "closed binding in documentation)" - % ("the parent %s is" % ", ".join(lost) if lost else "every parent is")) + if lost: + self.error(path + "/ancestor_defs", "bmm-publisher reads only \"ancestors\", so the parent %s " + "lost; list the root class name there (for an open binding " + "such as A, also redeclare T in generic_parameter_defs; " + "state a closed binding in documentation)" + % ("%s is" % lost[0] if len(lost) == 1 else "%s are" % ", ".join(lost))) def check_enumeration(self, cls, kind, ancestors, path): names = cls.get("item_names") @@ -448,7 +453,7 @@ def check_property(self, prop, path, scope): self.check_documentation(prop, path, required=True) return else: - kind = self.kind_of(prop, path, PROPERTY_TYPES, "P_BMM_SINGLE_PROPERTY", "P_BMM_SINGLE_PROPERTY") + kind, _ = self.kind_of(prop, path, PROPERTY_TYPES, "P_BMM_SINGLE_PROPERTY", "P_BMM_SINGLE_PROPERTY") self.check_name(prop, path) self.check_keys(prop, kind, path) self.check_documentation(prop, path, required=True) @@ -515,7 +520,7 @@ def check_type(self, t, path, scope): self.warning(path, "read as P_BMM_SIMPLE_TYPE; published schemas write P_BMM_SIMPLE_TYPE") kind = SIMPLE else: - kind = self.kind_of(t, path, TYPE_TYPES, SIMPLE, SIMPLE) + kind, _ = self.kind_of(t, path, TYPE_TYPES, SIMPLE, SIMPLE) self.check_type_body(t, kind, path, scope) return kind @@ -679,8 +684,8 @@ def check_parameter(self, param, path, scope): else: kind = "P_BMM_SINGLE_FUNCTION_PARAMETER" else: - kind = self.kind_of(param, path, PARAMETER_TYPES, "P_BMM_SINGLE_FUNCTION_PARAMETER", - "P_BMM_SINGLE_FUNCTION_PARAMETER") + kind, _ = self.kind_of(param, path, PARAMETER_TYPES, "P_BMM_SINGLE_FUNCTION_PARAMETER", + "P_BMM_SINGLE_FUNCTION_PARAMETER") self.check_name(param, path) self.check_keys(param, kind, path) self.check_documentation(param, path) @@ -725,19 +730,20 @@ def check_cardinality(self, card, path): # --- shared helpers ------------------------------------------------------------------------ def kind_of(self, item, path, kinds, read_as, read_as_label): - """The _type an item is checked as. An unknown _type is reported with what bmm-publisher reads - it as, and the item is then checked as the closest known _type, so its content is still checked.""" + """(kind, recognised): the _type an item is checked as, and whether bmm-publisher knows it. An + unknown _type is reported with what bmm-publisher reads it as, and the item is then checked as + the closest known _type, so that its content is still checked.""" kind = item.get("_type", kinds[0]) if isinstance(kind, str) and kind in kinds: - return kind + return kind, True if not isinstance(kind, str): self.error(path + "/_type", "must be a string; read as %s" % read_as_label) - return read_as + return read_as, False close = difflib.get_close_matches(kind, kinds, n=1, cutoff=0.6) checked = close[0] if close else read_as self.error(path, "unknown _type %s: read as %s%s" % (kind, read_as_label, "; checked below as " + checked if close else "")) - return checked + return checked, False def keyed(self, owner, key, path): block = owner.get(key) @@ -858,10 +864,13 @@ def check_type_name(self, name, path, scope): for dep_classes in self.dependencies.values(): known.update(dep_classes) near = difflib.get_close_matches(name, sorted(known), n=1, cutoff=0.85) - if near: - self.warning(path, "type %s is not defined; did you mean %s?" % (name, near[0])) - elif self.missing_includes: + if self.missing_includes: + # it may be defined in the include that was not loaded, so it is unchecked, not wrong self.unchecked.add(name) + if near: + self.suggestions[name] = near[0] + elif near: + self.warning(path, "type %s is not defined; did you mean %s?" % (name, near[0])) else: self.warning(path, "type %s is not defined in this schema or any loaded schema" % name) @@ -877,6 +886,8 @@ def load(path): % (path, exc.msg, exc.lineno, exc.colno))) except UnicodeDecodeError: raise SystemExit(fail("%s is not UTF-8 text" % path)) + except RecursionError: + raise SystemExit(fail("%s is nested too deeply to read" % path)) def fail(message): @@ -923,19 +934,21 @@ def main(argv=None): help="a schema whose classes the checked one uses (repeatable)") parser.add_argument("--strict", action="store_true", help="exit 1 on warnings too") args = parser.parse_args(argv) - checker = Checker(args.schema, args.dependency).run() + try: + checker = Checker(args.schema, args.dependency).run() + except RecursionError: + return fail("%s is nested too deeply to check" % args.schema) for level, path, message in checker.findings: print("%-7s %s: %s" % (level, path, message)) errors, warnings, unchecked = checker.count("ERROR"), checker.count("WARNING"), len(checker.unchecked) print("check_bmm: %s: %d class(es), %d error(s), %d warning(s), %d name(s) not checked" % (args.schema, len(checker.classes), errors, warnings, unchecked)) + if unchecked: + print("check_bmm: incomplete: %d name(s) not checked; pass each included schema with -d before " + "relying on this result" % unchecked, file=sys.stderr) if errors or (args.strict and warnings): return 1 - if unchecked: - print("check_bmm: incomplete: pass each included schema with -d before relying on this result", - file=sys.stderr) - return 3 - return 0 + return 3 if unchecked else 0 if __name__ == "__main__": diff --git a/scripts/test_check_bmm.py b/scripts/test_check_bmm.py index 98d8aa0..6f33afd 100644 --- a/scripts/test_check_bmm.py +++ b/scripts/test_check_bmm.py @@ -293,6 +293,13 @@ def test_unknown_class_type_is_reported_and_its_content_still_checked(self): self.assertFinding(checker, "ERROR", "/class_definitions/DEMO_STATUS", "unknown _type P_BMM_ENUMERATON_STRING") self.assertFinding(checker, "ERROR", "/class_definitions/DEMO_STATUS/item_documentations", "one text per item") + def test_misspelt_class_type_is_read_as_a_plain_class(self): + # bmm-publisher reads an unknown _type as a plain class, so the class keys are not lost + d = self.demo() + self.cls(d, "DEMO_ITEM")["_type"] = "P_BMM_INTERFACES" + self.assertOnly(self.check(d), "ERROR", "/class_definitions/DEMO_ITEM", + "unknown _type P_BMM_INTERFACES: read as a plain class") + def test_class_type_must_be_a_string(self): d = self.demo() self.cls(d, "DEMO_ITEM")["_type"] = ["P_BMM_CLASS"] @@ -314,6 +321,12 @@ def test_ancestor_defs_naming_another_parent_loses_it(self): self.assertFinding(self.check(d), "ERROR", "/class_definitions/DEMO_BOX_REF/ancestor_defs", "the parent DEMO_BOX is lost") + def test_empty_ancestor_defs_loses_no_parent(self): + d = self.demo() + del self.cls(d, "DEMO_BOX")["ancestors"] + self.cls(d, "DEMO_BOX")["ancestor_defs"] = {} + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_BOX/ancestor_defs", "not read") + def test_ancestor_defs_repeating_the_ancestors_only_warns(self): d = self.demo() self.cls(d, "DEMO_GROUP")["ancestor_defs"] = {"DEMO_ITEM": {"_type": "P_BMM_SIMPLE_TYPE", "type": "DEMO_ITEM"}} @@ -584,12 +597,20 @@ def test_unresolved_type_name(self): self.props(d, "DEMO_ITEM")["uid"]["type"] = "NO_SUCH_CLASS" self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/uid/type", "NO_SUCH_CLASS is not defined") - def test_misspelt_class_name_is_caught_even_without_dependencies(self): + def test_close_name_is_suggested_once_dependencies_are_loaded(self): + d = self.demo() + self.props(d, "DEMO_ITEM")["status"]["type"] = "DEMO_STATS" + self.assertOnly(self.check(d), "WARNING", "/class_definitions/DEMO_ITEM/properties/status/type", + "did you mean DEMO_STATUS") + + def test_close_name_without_dependencies_is_unchecked_with_a_suggestion(self): + # without -d, the name may come from the include (ARCHETYPE_ID next to a local ARCHETYPED) d = self.demo() self.props(d, "DEMO_ITEM")["status"]["type"] = "DEMO_STATS" checker = self.check(d, deps=False) - self.assertFinding(checker, "WARNING", "/class_definitions/DEMO_ITEM/properties/status/type", "did you mean DEMO_STATUS") - self.assertNotIn("DEMO_STATS", checker.unchecked) + self.assertEqual([lvl for lvl, _, _ in checker.findings], ["INFO"]) + self.assertIn("DEMO_STATS (or DEMO_STATUS?)", checker.findings[0][2]) + self.assertIn("DEMO_STATS", checker.unchecked) def test_attribute_reference_in_documentation(self): d = self.demo() @@ -900,6 +921,28 @@ def test_errors_outrank_an_incomplete_check(self): del d["class_definitions"]["DEMO_ITEM"]["properties"]["uid"]["type"] self.assertEqual(self.run_main(self.write("openehr_demo_0.1.0.bmm.json", d))[0], 1) + def test_incomplete_check_is_announced_with_errors_too(self): + d = self.demo() + del d["class_definitions"]["DEMO_ITEM"]["properties"]["uid"]["type"] + code, out, err = self.run_main(self.write("openehr_demo_0.1.0.bmm.json", d)) + self.assertEqual(code, 1) + self.assertIn("incomplete", err) + self.assertNotIn(" 0 name(s) not checked", out) + + def test_deep_nesting_exits_two(self): + def nested(depth): + return ('{"_type": "P_BMM_CONTAINER_TYPE", "container_type": "List", "type_def": ' * depth + + '{"_type": "P_BMM_SIMPLE_TYPE", "type": "String"}' + "}" * depth) + for depth, text in ((600, "too deeply to check"), (20000, "too deeply to read")): + with self.subTest(depth=depth): + d = self.demo() + d["class_definitions"]["DEMO_ITEM"]["functions"]["is_tagged"]["result"] = "@@RESULT@@" + path = self.root / "openehr_demo_0.1.0.bmm.json" + path.write_text(json.dumps(d).replace('"@@RESULT@@"', nested(depth)), encoding="utf-8") + code, _, err = self.run_main(path, "-d", self.base) + self.assertEqual(code, 2) + self.assertIn(text, err) + def test_unreadable_input_exits_two(self): broken = self.root / "broken.bmm.json" broken.write_text("{\"bmm_version\": ", encoding="utf-8") From 82d1d1433d9e8723cfb71157cbf8532f874d3799 Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Fri, 9 Oct 2026 17:00:15 +0200 Subject: [PATCH 5/6] docs(openehr-specs): shorten the bmm-authoring changelog entries and plain-language pass on the docs Split the long bmm-authoring CHANGELOG entry into short sub-bullets and use one term, "BMM schema edits", for the routing. Give the new-component prompt its own prompting-guide section, say what Claude does after each BMM prompt, and keep testing.md and the README to what the tests and the skill actually cover. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 +++++++++--- docs/prompting-guide.md | 19 ++++++++++++++----- docs/testing.md | 4 ++-- plugins/openehr-specs/README.md | 4 ++-- 4 files changed, 27 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 194df9f..d9edbc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,18 +6,24 @@ Notable changes to the plugins in this repository. The format follows [Keep a Ch ### Added -- `bmm-authoring` skill: write a component's BMM schema (the P_BMM JSON in `computable/BMM/`) from scratch, or change one. It covers the header and its conventions, `includes`, how packages set the class-table file names, every class, property, type and function kind, invariants and constants, documentation text, what the legacy class tables do not show, and the persistence-spec features `bmm-publisher` does not read (indexed containers, `ancestor_defs`, `type_ref` value sets), with the form to write instead. It ships a worked example (`assets/openehr_demo_0.1.0.bmm.json`) and a read-only checker, `scripts/check_bmm.py` (standard-library Python 3.8+), that reports what `bmm-publisher` accepts without a message or renders wrongly: a misspelt or missing `_type`, a misspelt key such as `is_mandantory`, a key that differs from its `name`, a class in no package, an upper limit without `"upper_unbounded": false`, a container without an element type, wrong generic parameter counts, a generic argument that does not conform to its parameter's `conforms_to_type`, type shapes the publisher mis-renders (a simple type in a container's `type_def`, a type object in `generic_parameters`, both generic-parameter forms at once, a container in `generic_parameter_defs`), classes more than four package levels deep, and unresolved type names. A run that could not load an included schema with `-d` reports how many names went unchecked and exits 3 when it found no errors, so an incomplete check is never mistaken for a clean one. CI runs its unit tests (`scripts/test_check_bmm.py`). +- `bmm-authoring` skill: write a component's BMM schema (the P_BMM JSON in `computable/BMM/`) from scratch, or change one. + - The reference covers every class, property, type and function kind, and the spec features `bmm-publisher` ignores. + - `assets/openehr_demo_0.1.0.bmm.json` is a worked example with one of each common construct. + - `scripts/check_bmm.py` (read-only, Python 3.8+) reports mistakes that `bmm-publisher` accepts without a warning, such as a misspelt `_type` or key. + - Names from an included schema not loaded with `-d` go unchecked, and a run with no errors then exits 3, not 0. + - CI runs its unit tests in the root `scripts/test_check_bmm.py`. - `scaffold` template set revision 3: a new repository also gets `computable/BMM/.bmm.json` (file id `bmm`, strategy `bmm-seed`). - The script lists the schemas the local `ghcr.io/openehr/bmm-publisher` image bundles (`docker run --pull never`, so nothing is downloaded) and copies the highest version for the component. - When the image bundles none, it writes an empty schema `openehr__`. The schema has one root package `org.openehr.`, no classes, and BASE in `includes` for other components. bmm-publisher refuses a schema without a package. - Without Docker or the image the file is reported as `blocked` and the other files are still written; `--var bmm_schema_id=` creates the empty schema anyway. - Existing repositories are not offered a schema; `--overwrite bmm` asks for one. - Derived variables can take a regex group (`match` + `group`), which gives the new `bmm_schema_name` and `bmm_rm_release`. + - The `AGENTS.md` task table and build region send BMM schema edits to `bmm-authoring`. ### Changed -- `class-generation` and `authoring` send changes to the class model itself to `bmm-authoring`, and `scaffold` points to it for filling a new repository's empty schema. -- `docs/prompting-guide.md`: the class-change prompts name `bmm-authoring` and the checker, and a prompt for a new component's schema is added. +- `class-generation`, `authoring` and `scaffold` send BMM schema edits to `bmm-authoring`. +- `docs/prompting-guide.md` sends the class-change prompts to `bmm-authoring` and adds a section for a new component's schema. ## openehr-specs 0.5.0 (2026-10-08) diff --git a/docs/prompting-guide.md b/docs/prompting-guide.md index 2791c0e..0363826 100644 --- a/docs/prompting-guide.md +++ b/docs/prompting-guide.md @@ -13,6 +13,7 @@ In the prompts, `` is a component id such as `RM`, `BASE` or `AM`, and ` of type to class , with documentation, and run the BMM checker. ``` -For a new component, give the release, the BASE version it builds on, and the classes: +Claude edits the JSON in place and raises the build number in `schema_revision`. It runs the checker before and after the change, so findings that were already in the schema are not mixed up with yours. If a sibling clone such as `specifications-BASE` is missing, it tells you the check is incomplete. -```text -Write the BMM schema for the new component, release , building on BASE , with these classes: <...>. -``` +Then regenerate the tables: ```text Regenerate the class tables for from the BMM in this checkout, not the copy bundled in the publisher image. Compare with docs/UML/classes and copy over only the tables that changed. @@ -106,6 +105,16 @@ To see what the image's bundled schemas produce, type: It writes to `out/` in the working directory and places nothing in a repository, so run it from a scratch directory. Its output reflects the bundled schemas; for tables to commit, use the previous prompt. +## Write the BMM schema for a new component + +Give the release, the BASE version it builds on, and the classes: + +```text +Write the BMM schema for the new component, release , building on BASE , with these classes: <...>. +``` + +If `/openehr-specs:scaffold` already created an empty schema, Claude fills that one; otherwise it starts from the skill's worked example. Either way, it asks you for the schema author. It checks the new schema as it would a change, so the sibling `specifications-BASE` clone is needed for a complete check. + ## Review before you merge One chapter, inline: diff --git a/docs/testing.md b/docs/testing.md index 39d1d46..d019ef0 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -1,6 +1,6 @@ # Testing and Validation -This is a pure-content repository: JSON manifests plus markdown skills (including user-only action skills) and subagents. There is no build step or package manager, and the only automated tests cover the scripts bundled with the `scaffold` and `bmm-authoring` skills. Testing means installing the marketplace locally and validating structure and skill quality. +This is a pure-content repository: JSON manifests plus markdown skills (including user-only action skills) and subagents. There is no build step or package manager, and the only automated tests are for the scripts bundled with the `scaffold` and `bmm-authoring` skills. Testing means installing the marketplace locally and validating structure and skill quality. ## Local Testing @@ -22,7 +22,7 @@ For Cursor, copy `plugins/openehr-specs` to `~/.cursor/plugins/local/openehr-spe ## Validation - **Manifest and frontmatter validation**: `python3 scripts/validate.py` (CI runs it on every PR) checks the Claude and Cursor JSON manifests, name and version sync, declared component paths, and the marketplace `owner.name`. For every skill and subagent it checks that `name` and `description` exist and that `name` matches the directory or file name. It also runs the `scaffold` template-set check (`scaffold.py check`: templates exist and render, revisions are contiguous, every revision has a migration file, and the template digest matches the latest revision). It does not check description length, `disable-model-invocation`, the README inventories, or the release tag. -- **Script tests**: `python3 -m unittest discover -s scripts -p 'test_*.py'` (CI runs it on every PR) covers the `scaffold` script (the template engine, each file strategy, variable inference, and the upgrade path across revisions) and the `bmm-authoring` checker (each finding it reports, its exit codes, a clean result on the skill's worked example, and no traceback for a wrongly typed value anywhere in it), all in temporary directories. Run it after changing either script, the template set, or the worked example. +- **Script tests**: `python3 -m unittest discover -s scripts -p 'test_*.py'` (CI runs it on every PR) tests both bundled scripts in temporary directories. For `scaffold` it covers the template engine, each file strategy, variable inference, and upgrades across revisions. For the `bmm-authoring` checker it covers each finding and exit code, a clean run on the worked example, and a wrongly typed value at any node of the worked example, which must give a finding and not a traceback. Run it after changing either script, the template set, or the worked example. - **Built-in manifest check**: `claude plugin validate plugins/openehr-specs` validates the plugin manifest; add `--strict` to treat warnings as errors. - **Structural validation**: after creating or modifying plugin components, run the `plugin-dev:plugin-validator` agent. It checks `plugin.json`, the marketplace entry, directory layout, and frontmatter. The agent comes from the `plugin-dev` plugin, which this repo enables in `.claude/settings.json`. - **Skill quality review**: run the `plugin-dev:skill-reviewer` agent (also from `plugin-dev`). It checks description triggering quality, progressive disclosure, and content structure. diff --git a/plugins/openehr-specs/README.md b/plugins/openehr-specs/README.md index cf30f83..b72d1a6 100644 --- a/plugins/openehr-specs/README.md +++ b/plugins/openehr-specs/README.md @@ -1,6 +1,6 @@ # openEHR Specifications -Skills and subagents for authors working in `specifications-XX` and `specifications-ITS-REST` repositories — AsciiDoc structure, amendment records, governance, quality review, prose patterns, ITS-REST OpenAPI sources, and BMM-based class documentation generation. +Skills and subagents for authors working in `specifications-XX` and `specifications-ITS-REST` repositories — AsciiDoc structure, amendment records, governance, quality review, prose patterns, ITS-REST OpenAPI sources, BMM schemas, and the class documentation generated from them. ## Installation @@ -28,7 +28,7 @@ Add the marketplace from this repository (or a fork), then install `openehr-spec | `content-patterns` | Prose patterns for spec chapters and sections | | `its-rest` | ITS-REST OpenAPI YAML and operation descriptions | | `class-generation` | Generate class tables and UML diagrams from BMM via `bmm-publisher` | -| `bmm-authoring` | Write or change a component's BMM schema (P_BMM JSON), and check it for the mistakes `bmm-publisher` accepts without a message (bundled `check_bmm.py`, needs `python3`) | +| `bmm-authoring` | Write or change a component's BMM schema (P_BMM JSON) and check it before rendering (needs `python3`) | ### User-only action skills From 45315cfc908e5aaf5d7bfa7ec867e27b07155b1a Mon Sep 17 00:00:00 2001 From: Sebastian Iancu Date: Fri, 9 Oct 2026 17:00:15 +0200 Subject: [PATCH 6/6] fix(openehr-specs): apply the skill review to bmm-authoring and its routing bmm-authoring: imperative wording without "you"; a description that also routes archetype work away and triggers on fixing a .bmm.json; one schema_revision bump per change set; a step for renames and removals; source changes committed together; exit 0 no longer called clean; the reference sections named instead of repeated; the from-scratch steps say which release and what to do when no :pkg: is set yet. scaffold template set revision 3 (unreleased, resealed): the AGENTS.md task table and build region send BMM schema edits to bmm-authoring, and the empty schema uses 4-space indentation like the published schemas. class-generation runs the checker first after a hand edit; authoring's related-skill line matches it. Co-Authored-By: Claude Opus 5.5 --- .../openehr-specs/skills/authoring/SKILL.md | 2 +- .../skills/bmm-authoring/SKILL.md | 84 ++++++++++--------- .../bmm-authoring/references/p-bmm-json.md | 3 +- .../skills/class-generation/SKILL.md | 3 +- .../skills/scaffold/assets/template-set.json | 4 +- .../scaffold/assets/templates/agents-md.tmpl | 3 +- .../scaffold/assets/templates/bmm-json.tmpl | 44 +++++----- scripts/test_scaffold.py | 2 + 8 files changed, 78 insertions(+), 67 deletions(-) diff --git a/plugins/openehr-specs/skills/authoring/SKILL.md b/plugins/openehr-specs/skills/authoring/SKILL.md index ee4a870..efd27d5 100644 --- a/plugins/openehr-specs/skills/authoring/SKILL.md +++ b/plugins/openehr-specs/skills/authoring/SKILL.md @@ -27,7 +27,7 @@ in `specifications-XX` repositories (RM, AM, BASE, LANG, PROC, SM, QUERY, CDS, C - **its-rest** — for OpenAPI YAML and Markdown in the ITS-REST repo - **governance** — release management, change requests, lifecycle governance - **class-generation** — regenerate the class-definition tables and UML diagrams in `docs/UML/` from BMM via `bmm-publisher` -- **bmm-authoring** — add or change classes, attributes, functions and invariants in the component's BMM schema +- **bmm-authoring** — write or change the component's BMM schema (classes, attributes, functions, invariants) and check it For attribute naming patterns, read `references/cross-references.md`. To verify that every `{openehr_*}` attribute and deep-link anchor resolves, dispatch the `xref-auditor` subagent. diff --git a/plugins/openehr-specs/skills/bmm-authoring/SKILL.md b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md index dab523a..1c51582 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/SKILL.md +++ b/plugins/openehr-specs/skills/bmm-authoring/SKILL.md @@ -3,9 +3,10 @@ name: bmm-authoring description: > Create or edit an openEHR component's BMM schema, the P_BMM JSON in `computable/BMM/` of a `specifications-XX` repo. This skill should be used when the user asks to write a BMM schema from - scratch, add, rename or remove a class, attribute, function, invariant or enumeration in a - component's model, set the schema header or includes, or check a hand-edited `.bmm.json`. Not for - regenerating class tables (use class-generation) or spec prose (use content-patterns). + scratch, add, rename or remove a class, attribute, function, invariant or enumeration in the + model, set the schema header or includes, or check or fix a `.bmm.json`. Not for regenerating class + tables (use class-generation), spec prose (use content-patterns), or archetype/template work + (openehr-assistant plugin). allowed-tools: - "Bash(python3 ${CLAUDE_SKILL_DIR}/scripts/check_bmm.py *)" --- @@ -18,9 +19,10 @@ truth for its classes. The class tables and diagrams in its documents are genera [`specifications-ITS-BMM`](https://github.com/openEHR/specifications-ITS-BMM) republishes it as JSON, ODIN and YAML. Change the model here, and only here. -- **Format reference**: `references/p-bmm-json.md` (header, packages, every class, property, type and - function kind, what the tables show, spec features the toolchain does not read). -- **Worked example**: `${CLAUDE_SKILL_DIR}/assets/openehr_demo_0.1.0.bmm.json`, one of each construct. +- **Format reference**: `${CLAUDE_SKILL_DIR}/references/p-bmm-json.md`. Before editing, read the + sections for the constructs involved (its Contents lists header, packages, every class, property, + type and function kind, what the tables show, and the spec features the toolchain does not read). +- **Worked example**: `${CLAUDE_SKILL_DIR}/assets/openehr_demo_0.1.0.bmm.json`, one of each common construct. - **Checker**: `${CLAUDE_SKILL_DIR}/scripts/check_bmm.py` (Python 3.8+, standard library only, read-only). ## Related Skills @@ -30,6 +32,8 @@ ODIN and YAML. Change the model here, and only here. - **amendment-record**: the entry for a change to published specification text - **governance**: releases, and the lifecycle state a schema header names - **content-patterns**: the prose around classes, which the tables do not carry +- **scaffold**: writes a new repository's schema, the image's bundled copy or an empty one + (`/openehr-specs:scaffold`) ## Why the Checker Comes Before the Render @@ -49,13 +53,14 @@ render and read the tables. ### Changing a schema -1. Read the header and the classes next to the one you are changing, and follow their idiom. Run the - checker once before editing (step 4) and save its output to a scratch file: the published schemas - carry findings of their own, and comparing that output with the one after your change separates - theirs from yours. +1. Read the header and the classes next to the one being changed, and follow their idiom. Run the + checker once before editing (step 4) and save its output with stderr (`2>&1`) to a temp file + outside the repo (`mktemp`): the published schemas carry findings of their own, and comparing + that output with the one after the change separates theirs from the new ones. 2. Make the change, using the format reference. Ground every class, attribute and function name in the specification text being written or in the user's instruction; never invent one. -3. Increase the build number in `schema_revision` (`1.3.0.2` → `1.3.0.3`). If the release is already +3. Increase the build number in `schema_revision` once per change set (`1.3.0.2` → `1.3.0.3`); + if `git diff` against the base branch already shows a bump, leave it. If the release is already published, whether the change belongs in it or in a new release (a new `rm_release`, file and schema id, which the `includes` of dependent schemas then name) is a decision for the user; see `governance`. @@ -64,39 +69,40 @@ render and read the tables. python3 ${CLAUDE_SKILL_DIR}/scripts/check_bmm.py computable/BMM/openehr_rm_1.2.0.bmm.json \ -d ../specifications-BASE/computable/BMM/openehr_base_1.3.0.bmm.json ``` - Save the output again and `diff` it with the step 1 output; fix every ERROR and WARNING your + Save the output again and `diff` it with the step 1 output; fix every ERROR and WARNING the change added. A non-zero "N name(s) not checked" in the summary (with an "incomplete" notice) means an included schema was not loaded: the exit status is 3 when there are no errors, 1 when there are. If the sibling clone is missing, say the check is incomplete; never report such a run as clean. -5. Regenerate the class tables (`class-generation`) and read the table of every class you touched. +5. Regenerate the class tables (`class-generation`) and read the table of every class touched. Expect the gaps listed under "What the class tables show" in the reference, such as a container's member count or a generic parameter's constraint. 6. A new class or package needs its `include::` line in a chapter, and a removed class loses its line - (`authoring`). Each table is named after the first four parts of its class's package path and the - class name in lower case: `LOCATABLE` in `org.openehr.rm.common` → `archetyped` gives - `org.openehr.rm.common.locatable.adoc`, included as `{pkg}locatable.adoc`. -7. Add the amendment-record entry. When asked to commit, commit the BMM change on its own, as the - repo's `AGENTS.md` says; leave regenerated tables and HTML unstaged unless the task is to refresh - them. + (`authoring`); the reference's Packages section gives each table's file name. For a rename or + removal, search the component's chapters, and the components whose schemas include this one, for + the old name and, for a class, its `__class` anchor. Then dispatch + `identifier-grounding` and `xref-auditor` on the affected documents. +7. Add the amendment-record entry. When asked to commit, commit the source changes (the BMM, the + `include::` lines, the amendment record) together, following the Conventions in the repo's + `AGENTS.md`; leave regenerated tables and HTML unstaged unless the task is to refresh them. ### Writing a schema from scratch 1. If `computable/BMM/` already holds an empty schema for the component (the `scaffold` skill writes - one), fill that in; replace its single root package `org.openehr.` as step 3 says, - and confirm its `schema_author` and `schema_lifecycle_state` with the user. Otherwise copy + one), fill that in; rename its single root package `org.openehr.` to the packages step + 3 names, and confirm its `schema_author` and `schema_lifecycle_state` with the user. Otherwise copy `${CLAUDE_SKILL_DIR}/assets/openehr_demo_0.1.0.bmm.json` to `computable/BMM/.bmm.json` - and replace its content. Set the header from the reference's header table: `schema_name` is the - lower-case component, `rm_release` the release, `schema_revision` `.1`. Ask the user - for `schema_author` rather than guessing it. + and replace every header value, `packages` and `class_definitions`. Set the header from the + reference's header table: `schema_name` is the lower-case component, `rm_release` the first release + the user confirms (scaffold's `first_release` in a new repo), `schema_revision` `.1`. + Ask the user for `schema_author` rather than guessing it. 2. Include BASE (`"includes": {"openehr_base_": {"id": "openehr_base_"}}`) and use its types (`Any`, `String`, `List`, `Hash`, `HIER_OBJECT_ID`, …). Do not redefine them; `primitive_types` belongs to BASE only. 3. Name top-level packages `org.openehr..`, one for each `:pkg:` the - component's `master.adoc` files set (`org.openehr...`): RM has - `org.openehr.rm.common`, `org.openehr.rm.ehr` and others, and its `ehr` document sets two; TERM - has the one package `org.openehr.term.terminology`. Group the classes in sub-packages as the - chapters do, at most four package levels deep. + component's `master.adoc` files set (see the reference's Packages section). If no `master.adoc` + sets `:pkg:` yet, agree the package names with the user and set `:pkg:` to match (`authoring`). + Group the classes in sub-packages as the chapters do, at most four package levels deep. 4. Add the classes, then continue from step 4 of "Changing a schema". ## Quick Reference @@ -123,23 +129,23 @@ render and read the tables. argument does not conform to its parameter's `conforms_to_type`. WARNING means a broken convention or content the publisher ignores. INFO lists what could not be checked, typically type names from an included schema that was not passed with `-d`. -- Exit status: 0 complete and clean; 1 errors found (with `--strict`, warnings count too); 3 no - errors, but names from an included schema not loaded with `-d` went unchecked; 2 a file is - unreadable, not UTF-8 JSON, nested too deeply, or (for `-d`) not a BMM schema. The summary line - always gives the unchecked count. -- The published schemas carry findings of their own. At the time of writing, AM 2.4.0 has ten errors - (lists without an element type in `P_ARCHETYPE_SLOT`, and `FUNCTION` arguments in reverse order in - `c_conforms_to`) and LANG's BMM3 overlay two. Report findings outside your change to the user; do - not fix them as a side effect. +- Exit status: 0 complete, no errors (read any warnings: an undefined type name is one); 1 errors + found (with `--strict`, warnings count too); 3 no errors, but names from an included schema not + loaded with `-d` went unchecked; 2 a file is unreadable, not UTF-8 JSON, nested too deeply, or (for + `-d`) not a BMM schema. The summary line always gives the unchecked count. +- The published schemas carry findings of their own, which the step 1 baseline shows. Report + findings outside the change to the user; do not fix them as a side effect. - If `python3` is missing, say so and report the schema as unchecked; review the change by hand - against the reference's lists before rendering. In a tool that does not expand + against the reference's Properties and Types sections and its table of specification features + bmm-publisher does not read, before rendering. In a tool that does not expand `${CLAUDE_SKILL_DIR}`, use the directory that holds this SKILL.md. ## Guardrails - **Edit the JSON in place.** The files mix PHP-style escapes (`\/`, `\u2260`) with raw characters - from later edits, so loading and re-serialising a file rewrites lines you did not touch. Keep the - 4-space indentation and key order, and check that `git diff` shows only your change. + from later edits, so loading and re-serialising a file rewrites lines nobody touched. Keep the + file's indentation (4 spaces in the published schemas) and key order, and check that `git diff` + shows only the intended change. - **Edit only `computable/BMM/` in the component repo.** Never the ITS-BMM copies, the generated `docs/UML/classes` tables, or the schemas bundled in `bmm-publisher`. - **Keys equal names.** Every entry of a map of named things (packages, classes, properties, diff --git a/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md index 45405cb..d00b30a 100644 --- a/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md +++ b/plugins/openehr-specs/skills/bmm-authoring/references/p-bmm-json.md @@ -16,7 +16,8 @@ the specification and the toolchain differ, both are stated. It was checked agai time of writing. `assets/openehr_demo_0.1.0.bmm.json` is a complete worked example with one each of the common -constructs. It passes `scripts/check_bmm.py` and renders with `bmm-publisher`. +constructs. With BASE loaded through `-d`, `scripts/check_bmm.py` reports nothing on it, and it renders +with `bmm-publisher`. ## Contents diff --git a/plugins/openehr-specs/skills/class-generation/SKILL.md b/plugins/openehr-specs/skills/class-generation/SKILL.md index 034080e..b645620 100644 --- a/plugins/openehr-specs/skills/class-generation/SKILL.md +++ b/plugins/openehr-specs/skills/class-generation/SKILL.md @@ -100,7 +100,8 @@ image::ROOT:uml/classes/COMPOSITION.svg[] `computable/BMM/openehr__.bmm.json`; each dependency comes from its sibling clone (RM, AM, LANG and TERM classes refer to BASE types: `../specifications-BASE/computable/BMM/openehr_base_.bmm.json`). When a file is missing, say - which and stop rather than falling back to a bundled id. + which and stop rather than falling back to a bundled id. If the schema was edited by hand, run the + `bmm-authoring` checker on it first. 2. **Choose the command by layout** (see above): `legacy-adoc` for the legacy layout, `asciidoc` for the current one. 3. **Generate** into a temporary directory, mounting the files read-only and mapping ownership to the diff --git a/plugins/openehr-specs/skills/scaffold/assets/template-set.json b/plugins/openehr-specs/skills/scaffold/assets/template-set.json index 0016f9d..8dc2f1b 100644 --- a/plugins/openehr-specs/skills/scaffold/assets/template-set.json +++ b/plugins/openehr-specs/skills/scaffold/assets/template-set.json @@ -17,8 +17,8 @@ { "revision": 3, "released": "2026-10-09", - "summary": "A new repository gets computable/BMM/.bmm.json: a copy of the schema the bmm-publisher image bundles for the component, or an empty schema (one root package, BASE included) when it bundles none. Existing repositories are not offered one.", - "digest": "c78b965d50608d5937bd6dfcc0b0953cba4c91d51ad347bd46b647b7cdc7281f" + "summary": "A new repository gets computable/BMM/.bmm.json: a copy of the schema the bmm-publisher image bundles for the component, or an empty schema (one root package, BASE included) when it bundles none. Existing repositories are not offered one. AGENTS.md sends BMM edits to the bmm-authoring skill.", + "digest": "26ad62920185cc51eaab770d36105704846af19301e544bf94244220d9e3c9f1" } ], "variables": { diff --git a/plugins/openehr-specs/skills/scaffold/assets/templates/agents-md.tmpl b/plugins/openehr-specs/skills/scaffold/assets/templates/agents-md.tmpl index af53f02..bd646c1 100644 --- a/plugins/openehr-specs/skills/scaffold/assets/templates/agents-md.tmpl +++ b/plugins/openehr-specs/skills/scaffold/assets/templates/agents-md.tmpl @@ -29,6 +29,7 @@ The plugin carries the spec-authoring know-how — **prefer its skills/agents ov | Create/edit a spec, chapter, `master.adoc`, or `manifest.json` | skill `openehr-specs:authoring` | | Spec prose style — overviews, semantics, design rationale | skill `openehr-specs:content-patterns` | {{#if bmm_schema_id}} +| Add or change classes, attributes, functions or invariants in the BMM | skill `openehr-specs:bmm-authoring` | | Regenerate class tables/diagrams from BMM (`bmm-publisher`) | skill `openehr-specs:class-generation` | {{/if}} | Amendment record (`master00-amendment_record.adoc`) | skill `openehr-specs:amendment-record` | @@ -82,7 +83,7 @@ docker run --rm --user $(id -u):$(id -g) \ # then diff "$OUT" against docs/UML/classes and copy over the tables you changed ``` -To change a class/attribute/function/invariant, edit the BMM schema and regenerate — never touch the generated tables (see skill `openehr-specs:class-generation`). +To change a class/attribute/function/invariant, edit the BMM schema (skill `openehr-specs:bmm-authoring`, which also checks it) and regenerate (skill `openehr-specs:class-generation`) — never touch the generated tables. {{/if}} diff --git a/plugins/openehr-specs/skills/scaffold/assets/templates/bmm-json.tmpl b/plugins/openehr-specs/skills/scaffold/assets/templates/bmm-json.tmpl index eed723b..76389a0 100644 --- a/plugins/openehr-specs/skills/scaffold/assets/templates/bmm-json.tmpl +++ b/plugins/openehr-specs/skills/scaffold/assets/templates/bmm-json.tmpl @@ -1,24 +1,24 @@ { - "bmm_version": "2.4", - "rm_publisher": "openehr", - "schema_name": "{{bmm_schema_name}}", - "rm_release": "{{bmm_rm_release}}", - "schema_revision": "{{bmm_rm_release}}.1", - "schema_lifecycle_state": "development", - "schema_description": "{{component_description}}", - "schema_author": "openEHR Foundation", - {{#if base_bmm_schema_id}} - "includes": { - "{{base_bmm_schema_id}}": { - "id": "{{base_bmm_schema_id}}" - } - }, - {{/if}} - "packages": { - "org.openehr.{{bmm_schema_name}}": { - "name": "org.openehr.{{bmm_schema_name}}", - "classes": [] - } - }, - "class_definitions": {} + "bmm_version": "2.4", + "rm_publisher": "openehr", + "schema_name": "{{bmm_schema_name}}", + "rm_release": "{{bmm_rm_release}}", + "schema_revision": "{{bmm_rm_release}}.1", + "schema_lifecycle_state": "development", + "schema_description": "{{component_description}}", + "schema_author": "openEHR Foundation", + {{#if base_bmm_schema_id}} + "includes": { + "{{base_bmm_schema_id}}": { + "id": "{{base_bmm_schema_id}}" + } + }, + {{/if}} + "packages": { + "org.openehr.{{bmm_schema_name}}": { + "name": "org.openehr.{{bmm_schema_name}}", + "classes": [] + } + }, + "class_definitions": {} } diff --git a/scripts/test_scaffold.py b/scripts/test_scaffold.py index db0773a..4f8dd6e 100644 --- a/scripts/test_scaffold.py +++ b/scripts/test_scaffold.py @@ -172,6 +172,7 @@ def test_bmm_switches_on_the_class_generation_sections(self): agents = self.text("AGENTS.md") self.assertIn("computable/BMM/openehr_demo_1.0.0.bmm.json", agents) self.assertIn("openehr-specs:class-generation", agents) + self.assertIn("openehr-specs:bmm-authoring", agents) self.assertIn("## Gotchas", agents) self.assertIn("and BMM sources", self.text(".claude/CLAUDE.md")) @@ -1149,6 +1150,7 @@ def test_without_docker_the_schema_is_blocked_and_the_rest_is_written(self): self.assertFalse((self.repo / "computable").exists()) self.assertIn("AGENTS.md", result["written"]) self.assertNotIn("class-generation", self.text("AGENTS.md")) + self.assertNotIn("bmm-authoring", self.text("AGENTS.md")) def test_a_given_schema_id_creates_an_empty_schema_without_docker(self): act = self.bmm_act(self.plan(bmm_schema_id="openehr_demo_2.0.0"))