From 64aa2cf0393a9547fd4a9d76991bc684348c3465 Mon Sep 17 00:00:00 2001 From: Hannes Stiebitzhofer Date: Mon, 14 Sep 2026 11:02:25 +0200 Subject: [PATCH] Let the request form capture every objectType it offers (#114) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dropdown lists eight kinds; the form could complete four. The schema requires a field per kind and the form asked for two of them: MultiValuedDataElement itemType missing MeasurementUnit symbol missing MeasurementUnit crossReferences.ucumCode missing Quantity dimension missing Value value missing So a requester could pick MeasurementUnit, fill in everything the form asked for, and still have produced a request that cannot be drafted. The form's own header promises it mirrors the envelope "so triage can generate the draft YAML mechanically"; for half the kinds it could not. ucumCode is the sharpest case, since check 2 validates its syntax — a guess fails the publish PR rather than degrading quietly. Added as one group behind a note saying only the row matching your objectType applies. All stay optional: GitHub issue forms cannot express "required when that dropdown says X", so the condition lives in the label and description. Left out uneceCommonCode, qudtUnit and conversions. Real unit entries carry them, but they are registry lookups a maintainer does better than a requester, and none is schema-enforced. Found while checking whether legalBasis had reached the form. It had, in #78's own commit — these had not, which is the second time the form has trailed the schema, so it becomes a check rather than a thing to remember. The test walks the schema's allOf conditionals and asserts each required property has an input to capture it, and separately that no input names a field the envelope lacks. Verified by renaming ucum-code in the form and watching both directions fail with the drift named. Closes #114 --- .claude/skills/new-entry/SKILL.md | 4 +- .github/ISSUE_TEMPLATE/dictionary-request.yml | 51 +++++++++ test/request-form.test.ts | 103 ++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 test/request-form.test.ts diff --git a/.claude/skills/new-entry/SKILL.md b/.claude/skills/new-entry/SKILL.md index 434d6ba..4358b65 100644 --- a/.claude/skills/new-entry/SKILL.md +++ b/.claude/skills/new-entry/SKILL.md @@ -22,7 +22,9 @@ Companion skill: [publish-entry](../publish-entry/SKILL.md) moves the draft live 1. **Read the issue.** Pull `shortName`, `objectType`, `preferredName` (en, optionally de), `definition` (en, optionally de), and every optional field the requester filled in (`valueDataType`, `unit`, `quantityKind`, `definitionStandard`, `testStandard`, `legalBasis`, - `exampleValue`, enumeration values, collection members, `identicalTo`). Use `legalBasis` for + `exampleValue`, enumeration values, collection members, `identicalTo`, plus the kind-specific + block — `itemType`, `symbol`, `ucumCode`, `dimension`, `value` — of which only the one matching + `objectType` should be filled, and step 4 says which). Use `legalBasis` for a law/regulation citation (e.g. an ELI reference) and `definitionStandard`/`testStandard` only for a technical/testing standard — don't conflate the two. diff --git a/.github/ISSUE_TEMPLATE/dictionary-request.yml b/.github/ISSUE_TEMPLATE/dictionary-request.yml index f3d8dd9..a155dfd 100644 --- a/.github/ISSUE_TEMPLATE/dictionary-request.yml +++ b/.github/ISSUE_TEMPLATE/dictionary-request.yml @@ -101,6 +101,57 @@ body: description: For units/quantities — URI of the Quantity entry, or its name if new. validations: required: false + - type: markdown + attributes: + value: >- + **Kind-specific fields.** Only the one matching your `objectType` applies — leave the rest + blank. Each is required in the envelope for its kind, so a request without it cannot be + drafted; the form cannot mark them required because it can't see which kind you picked. + - type: input + id: item-type + attributes: + label: itemType — for MultiValuedDataElement + description: URI of the entry describing a single item of the list — or its name if new. + placeholder: "https://material-identity.eu/def/ — or: new entry needed: alloyingElement" + validations: + required: false + - type: input + id: symbol + attributes: + label: symbol — for MeasurementUnit + description: The printed symbol. Give the English form; drafting expands it to a language map. + placeholder: MPa + validations: + required: false + - type: input + id: ucum-code + attributes: + label: ucumCode — for MeasurementUnit + description: >- + The UCUM code for the unit, which enables automatic conversion. Its syntax is validated + automatically, so leave it blank rather than guessing — note UCUM is case-sensitive and + spells some units unexpectedly (`Cel` for degree Celsius, `[psi]`, `mm2`). + placeholder: MPa + validations: + required: false + - type: input + id: dimension + attributes: + label: dimension — for Quantity + description: Dimensional formula in M/L/T/I/Θ/N/J, exponents with `^`. + placeholder: M L^-1 T^-2 + validations: + required: false + - type: input + id: value + attributes: + label: value — for Value + description: >- + The literal value this entry stands for. Only for a standalone Value request — members of + an enumeration go in the "Enumeration values" field below instead. + placeholder: EAF + validations: + required: false - type: input id: definition-standard attributes: diff --git a/test/request-form.test.ts b/test/request-form.test.ts new file mode 100644 index 0000000..d68aad8 --- /dev/null +++ b/test/request-form.test.ts @@ -0,0 +1,103 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { parse } from 'yaml'; + +// The dictionary-request form is the only way a requester states what they want, and triage +// generates the draft YAML from it. When the schema grows a kind-specific requirement and the +// form does not, the form silently stops being able to express a valid request for that kind — +// which is exactly what happened to itemType, symbol, ucumCode, dimension and value (#114). +// So this asserts the two stay in step rather than trusting anyone to remember. + +const root = join(dirname(fileURLToPath(import.meta.url)), '..'); +const FORM_PATH = '.github/ISSUE_TEMPLATE/dictionary-request.yml'; + +interface FormField { + type: string; + id?: string; + attributes?: { label?: string; options?: string[] }; +} + +const schema = JSON.parse(readFileSync(join(root, 'schema', 'dictionary-entry.schema.json'), 'utf8')) as { + properties: Record; + $defs?: Record; + allOf?: { if?: { properties?: { objectType?: { const?: string } } }; then?: ThenClause }[]; +}; + +interface SchemaNode { + $ref?: string; + properties?: Record; +} + +/** Follow a local `#/$defs/` reference one level — enough for this schema's shape. */ +function resolve(node: SchemaNode | undefined): SchemaNode | undefined { + const name = node?.$ref?.replace('#/$defs/', ''); + return name ? schema.$defs?.[name] : node; +} +const form = parse(readFileSync(join(root, FORM_PATH), 'utf8')) as { body: FormField[] }; + +interface ThenClause { + required?: string[]; + properties?: Record; +} + +/** Every property a `then` clause demands, nested ones as "parent.child" (crossReferences.ucumCode). */ +function requiredBy(then: ThenClause): string[] { + const direct = then.required ?? []; + const nested = Object.entries(then.properties ?? {}).flatMap(([parent, sub]) => + (sub.required ?? []).map((child) => `${parent}.${child}`), + ); + // a `required: [crossReferences]` alongside the nested rule adds nothing a requester can act on + return [...direct.filter((p) => !nested.some((n) => n.startsWith(`${p}.`))), ...nested]; +} + +/** Schema property name → the form input id that captures it. */ +function formId(property: string): string { + const leaf = property.includes('.') ? property.slice(property.indexOf('.') + 1) : property; + return leaf.replace(/[A-Z]/g, (c) => `-${c.toLowerCase()}`); +} + +/** Where the form's own wording diverges from the envelope's field name. */ +const RENAMED: Record = { elements: 'collection-members' }; + +test('every objectType the form offers is one the form can complete (#114)', () => { + const ids = new Set(form.body.filter((f) => f.id).map((f) => f.id as string)); + const offered = new Set( + form.body.find((f) => f.id === 'object-type')?.attributes?.options ?? [], + ); + assert.ok(offered.size > 0, 'the objectType dropdown should list its kinds'); + + const missing: string[] = []; + for (const rule of schema.allOf ?? []) { + const kind = rule.if?.properties?.objectType?.const; + if (!kind || !rule.then) continue; + assert.ok(offered.has(kind), `schema constrains ${kind}, which the form does not offer`); + for (const property of requiredBy(rule.then)) { + const id = RENAMED[property] ?? formId(property); + if (!ids.has(id)) missing.push(`${kind} requires ${property} — no form input "${id}"`); + } + } + assert.deepEqual(missing, [], `${FORM_PATH} cannot capture:\n ${missing.join('\n ')}`); +}); + +test('the form only asks for fields the envelope actually has (#114)', () => { + // The reverse drift: an input whose id matches no schema property produces a value with + // nowhere to go, and triage has to guess whether it was renamed or dropped. + const properties = new Set(Object.keys(schema.properties).map(formId)); + for (const nested of Object.keys(resolve(schema.properties.crossReferences)?.properties ?? {})) { + properties.add(formId(nested)); + } + assert.ok(properties.has('ucum-code'), 'the $ref into $defs should have resolved'); + // Inputs that are deliberately about the request, not the entry. + const NOT_ENVELOPE = new Set([ + 'short-name', 'object-type', 'justification', 'preferred-name-en', 'preferred-name-de', + 'definition-en', 'definition-de', 'value-data-type', 'collection-members', 'identical-to', + ]); + + const orphans = form.body + .filter((f) => f.id && !NOT_ENVELOPE.has(f.id) && !properties.has(f.id)) + .map((f) => `${f.id} (${f.attributes?.label ?? 'no label'})`); + assert.deepEqual(orphans, [], `form inputs with no matching envelope field: ${orphans.join(', ')}`); +});