From 679d5c0fa526af9a35465bf9422cf1db0e3bcaff Mon Sep 17 00:00:00 2001 From: jstet Date: Fri, 25 Sep 2026 12:55:37 +0200 Subject: [PATCH] refactor: one module per registry convention under src/conventions/ Each pipeline re-implemented the registry conventions, and several copies were literals the registry couldn't change: '_other' in the validator, the DDI emitter and the other-pattern detector; OTHER_CODE/OTHER_SUFFIX/ otherLabelFor defined twice; 'cdlvocab-', '_from_file' and 'table-list' written out across pipelines. src/conventions/ now holds other.ts, fromFile.ts, exclusive.ts (moved from src/xlsform/), grid.ts and metadata.ts. Each reads src/generated/conventions, and every format module and pipeline imports from there. To make that possible the registry gains two machine-readable fields: - grid composite trigger.appearance = "table-list" (the prose also claimed "name contains 'grid' / label contains 'matrix'", which no code does; corrected) - convention:externalCodeList.limesurveyCssClassPrefix = "cdlvocab-" Behaviour: unchanged, and every TSV/DDI snapshot is identical, with one exception. The other-pattern detector also removed choices named _other/other_option/other_choice, spellings the registry doesn't define and the validator rejects; it now uses the convention's choice code only. tests/ts/unit/conventionLiterals.test.ts fails if a convention literal appears outside src/generated and src/conventions. ARCHITECTURE.md describes the module. The longer-term Instrument model from the issue is split out as #69. Closes #60 Co-Authored-By: Claude Opus 5.5 (1M context) --- ARCHITECTURE.md | 2 + registry/conventions/externalCodeList.jsonld | 1 + registry/entities/grid/definition.jsonld | 3 +- registry/schema.jsonld | 9 +++ src/{xlsform => conventions}/exclusive.ts | 2 +- src/conventions/fromFile.ts | 47 ++++++++++++++++ src/conventions/grid.ts | 17 ++++++ src/conventions/metadata.ts | 8 +++ src/conventions/other.ts | 35 ++++++++++++ src/ddi/codebook.ts | 24 ++++---- src/generated/conventions.json | 4 +- src/generated/conventions.ts | 4 +- src/pipelines/lstsv2ddi/data.ts | 2 +- src/pipelines/lstsv2ddi/toVariables.ts | 55 +++++-------------- src/pipelines/lstsv2xlsform/toXlsform.ts | 15 ++--- src/pipelines/xlsform2ddi/data.ts | 3 +- src/pipelines/xlsform2ddi/variables.ts | 41 +++++--------- src/pipelines/xlsform2lstsv/constants.ts | 21 +------ src/pipelines/xlsform2lstsv/groupEmitter.ts | 3 +- src/pipelines/xlsform2lstsv/groupProcessor.ts | 3 +- src/pipelines/xlsform2lstsv/index.ts | 18 +++--- .../xlsform2lstsv/otherPatternDetector.ts | 36 ++++++------ src/xlsform/validate.ts | 29 ++++++---- tests/ts/unit/conventionLiterals.test.ts | 48 ++++++++++++++++ 24 files changed, 280 insertions(+), 150 deletions(-) rename src/{xlsform => conventions}/exclusive.ts (94%) create mode 100644 src/conventions/fromFile.ts create mode 100644 src/conventions/grid.ts create mode 100644 src/conventions/metadata.ts create mode 100644 src/conventions/other.ts create mode 100644 tests/ts/unit/conventionLiterals.test.ts diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 9aae3a0..c3e9c97 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -34,6 +34,8 @@ The TypeScript library (`@correlaid/formtransform`), split into **format modules - **`src/ddi/`** — the DDI-Codebook 2.5 emitter (`codebook.ts`, `xml.ts`, `notes.ts`) over the canonical `Variable[]` model (`types.ts`). This model is the hub: both DDI pipelines produce `Variable[]`, then one writer emits the XML. +- **`src/conventions/`** — one module per registry convention (`other.ts`, `fromFile.ts`, `exclusive.ts`, `grid.ts`, `metadata.ts`). Each reads its values from `src/generated/conventions` and exposes helpers; every format module and pipeline imports them from here. No convention value (suffix, choice code, label, prefix) is written out anywhere else, which `tests/ts/unit/conventionLiterals.test.ts` enforces. + #### Pipelines - **`src/pipelines/2/`** — One module per supported direction: diff --git a/registry/conventions/externalCodeList.jsonld b/registry/conventions/externalCodeList.jsonld index 9bd428f..d60bab3 100644 --- a/registry/conventions/externalCodeList.jsonld +++ b/registry/conventions/externalCodeList.jsonld @@ -34,6 +34,7 @@ "ddiVocabFromFilename": "stripExtension", "ddiEmission": "concept[@vocab=''] in place of inline catgry; catgry MUST NOT be emitted", "limesurveyEmission": "base select (L/M) with the vocabulary's options inlined as A/SQ rows + cssclass='cdlvocab-' carrying provenance (a registered LimeSurvey question attribute, so it survives import)", + "limesurveyCssClassPrefix": "cdlvocab-", "structuralNote": "select_multiple_from_file emits a flat with concept/@vocab, NOT a varGrp[@type='multipleResp'] with binary children. This differs from select_multiple.", "appliesTo": [ "select_one_from_file", diff --git a/registry/entities/grid/definition.jsonld b/registry/entities/grid/definition.jsonld index d085831..c56d6cb 100644 --- a/registry/entities/grid/definition.jsonld +++ b/registry/entities/grid/definition.jsonld @@ -21,7 +21,8 @@ } ], "trigger": { - "xlsformPattern": "begin_group row with appearance=table-list (or name contains 'grid', or label contains 'matrix')", + "appearance": "table-list", + "xlsformPattern": "begin_group row with appearance=table-list", "applyTo": "begin_group + all rows until matching end_group" }, "input": { diff --git a/registry/schema.jsonld b/registry/schema.jsonld index 7877236..4479714 100644 --- a/registry/schema.jsonld +++ b/registry/schema.jsonld @@ -528,6 +528,15 @@ "type": "object", "required": true, "fields": { + "appearance": { + "type": "string", + "required": true, + "consumedBy": [ + "ls-transformer", + "ddi-emitter" + ], + "description": "The begin_group appearance that makes a group this composite (machine-readable form of xlsformPattern)." + }, "xlsformPattern": { "type": "string", "required": true, diff --git a/src/xlsform/exclusive.ts b/src/conventions/exclusive.ts similarity index 94% rename from src/xlsform/exclusive.ts rename to src/conventions/exclusive.ts index bcd2b28..9fa9bef 100644 --- a/src/xlsform/exclusive.ts +++ b/src/conventions/exclusive.ts @@ -5,7 +5,7 @@ * `exclude_all_others` question attribute. */ import conventions from '../generated/conventions.js'; -import type { ChoiceRow } from './types.js'; +import type { ChoiceRow } from '../xlsform/types.js'; export const EXCLUSIVE_RULE = conventions.conventions.exclusiveChoice; diff --git a/src/conventions/fromFile.ts b/src/conventions/fromFile.ts new file mode 100644 index 0000000..ad76db4 --- /dev/null +++ b/src/conventions/fromFile.ts @@ -0,0 +1,47 @@ +/** + * `convention:externalCodeList`: `select_one_from_file .csv` and + * `select_multiple_from_file`. LimeSurvey gets the base select with the + * vocabulary inlined and `cssclass="cdlvocab-"` as provenance; DDI gets + * ``. + */ +import conventions from '../generated/conventions.js'; + +const RULE = conventions.conventions.externalCodeList; + +const SUFFIX = '_from_file'; + +/** `select_*_from_file` type → the base select it is emitted as. */ +export const FROM_FILE_BASE: Readonly> = + Object.fromEntries( + RULE.appliesTo.map((t: string) => [t, t.slice(0, -SUFFIX.length)]), + ); + +/** Whether an XLSForm base type is a `select_*_from_file`. */ +export function isFromFileType(baseType: string): boolean { + return baseType in FROM_FILE_BASE; +} + +/** The `select_*_from_file` type for a base select (`select_one` → `select_one_from_file`). */ +export function fromFileTypeFor(baseSelect: string): string { + return baseSelect + SUFFIX; +} + +/** Vocabulary id from a filename (`iso_3166_1.csv` → `iso_3166_1`). */ +export function vocabFromFilename(filename: string): string { + return filename.replace(/\.csv$/i, ''); +} + +/** Prefix of the LimeSurvey `cssclass` value carrying vocabulary provenance. */ +export const CDLVOCAB_PREFIX: string = RULE.limesurveyCssClassPrefix; + +/** The `cssclass` value recording a vocabulary. */ +export function cssClassForVocab(vocab: string): string { + return CDLVOCAB_PREFIX + vocab; +} + +/** Vocabulary id from a `cssclass` value, or `''` when it carries none. */ +export function vocabFromCssClass(cssclass: string): string { + return cssclass.startsWith(CDLVOCAB_PREFIX) + ? cssclass.slice(CDLVOCAB_PREFIX.length) + : ''; +} diff --git a/src/conventions/grid.ts b/src/conventions/grid.ts new file mode 100644 index 0000000..26019ab --- /dev/null +++ b/src/conventions/grid.ts @@ -0,0 +1,17 @@ +/** + * The `grid` composite: a `begin_group` whose appearance marks it as a grid + * (one shared choice list, one row per question). LimeSurvey: array `F`; + * DDI: ``. + */ +import conventions from '../generated/conventions.js'; + +const GRID = conventions.composites.find((c) => c.id === 'grid'); +if (!GRID) throw new Error('registry has no grid composite'); + +/** The `begin_group` appearance that makes a group a grid. */ +export const GRID_APPEARANCE: string = GRID.trigger.appearance; + +/** Whether a group appearance (possibly several, space-separated) marks a grid. */ +export function isGridAppearance(appearance: string): boolean { + return appearance.split(/\s+/).includes(GRID_APPEARANCE); +} diff --git a/src/conventions/metadata.ts b/src/conventions/metadata.ts new file mode 100644 index 0000000..3f0162f --- /dev/null +++ b/src/conventions/metadata.ts @@ -0,0 +1,8 @@ +/** + * `convention:unregisteredRows`: XLSForm metadata rows (`start`, `end`, + * `today`, …) carry no question and are skipped by every emitter. + */ +import conventions from '../generated/conventions.js'; + +export const METADATA_ROW_TYPES: readonly string[] = + conventions.conventions.unregisteredRows.metadataRowTypes; diff --git a/src/conventions/other.ts b/src/conventions/other.ts new file mode 100644 index 0000000..317764f --- /dev/null +++ b/src/conventions/other.ts @@ -0,0 +1,35 @@ +/** + * `convention:other`: the semi-open "Other" answer. XLSForm writes it as a + * choice with code `other` plus a `_other` text companion; + * LimeSurvey as `other=Y`; DDI as ``. + */ +import conventions from '../generated/conventions.js'; + +const RULE = conventions.conventions.other; + +/** Choice code of the "other" answer. */ +export const OTHER_CODE: string = RULE.choiceCode; + +/** Suffix of the free-text companion question (`_other`). */ +export const OTHER_SUFFIX: string = RULE.companionSuffix; + +/** Type of the free-text companion question. */ +export const OTHER_COMPANION_TYPE: string = RULE.companionType; + +/** Select types that can carry an "other" answer. */ +export const OTHER_APPLIES_TO: readonly string[] = RULE.appliesTo; + +/** Canonical "other" label per language. */ +export const OTHER_LABELS: Readonly> = RULE.labels; + +/** Canonical "other" label for a language, falling back to English. */ +export function otherLabelFor(lang: string): string { + return OTHER_LABELS[lang] ?? OTHER_LABELS['en']; +} + +/** The base question name of an `_other` companion, or `null`. */ +export function otherCompanionBase(name: string): string | null { + return name.endsWith(OTHER_SUFFIX) && name.length > OTHER_SUFFIX.length + ? name.slice(0, -OTHER_SUFFIX.length) + : null; +} diff --git a/src/ddi/codebook.ts b/src/ddi/codebook.ts index e42142c..e3fe11f 100644 --- a/src/ddi/codebook.ts +++ b/src/ddi/codebook.ts @@ -12,6 +12,12 @@ import { DDI_TYPE_MAP, RESPONSE_DOMAIN_MAP } from '../generated/DdiMappings.js'; +import { + OTHER_APPLIES_TO, + OTHER_CODE, + otherCompanionBase, +} from '../conventions/other.js'; +import { isGridAppearance } from '../conventions/grid.js'; import { XmlElement } from './xml.js'; import { classifyNotes } from './notes.js'; import { Choice, Variable } from './types.js'; @@ -38,7 +44,7 @@ function makeGrpId(name: string): string { function isGridGroup(variables: Variable[], groupName: string): boolean { for (const v of variables) { if (v.group === groupName && v.groupAppearance) { - return v.groupAppearance.includes('table-list'); + return isGridAppearance(v.groupAppearance); } } return false; @@ -141,15 +147,11 @@ function detectOtherPatterns(variables: Variable[]): Map { const byName = new Map(variables.map((v) => [v.name, v])); const patterns = new Map(); for (const v of variables) { - if (v.type !== 'text' || !v.name.endsWith('_other')) continue; - const baseName = v.name.slice(0, -'_other'.length); + const baseName = v.type === 'text' ? otherCompanionBase(v.name) : null; + if (!baseName) continue; const base = byName.get(baseName); - if ( - !base || - (base.type !== 'select_one' && base.type !== 'select_multiple') - ) - continue; - if (!base.choices.some((c) => c.name === 'other')) continue; + if (!base || !OTHER_APPLIES_TO.includes(base.type)) continue; + if (!base.choices.some((c) => c.name === OTHER_CODE)) continue; patterns.set(baseName, { base, otherVar: v, @@ -166,7 +168,7 @@ function emitOtherPattern(dataDscr: XmlElement, p: OtherPattern): void { const baseName = base.name; if (p.isMulti) { - const nonOther = base.choices.filter((c) => c.name !== 'other'); + const nonOther = base.choices.filter((c) => c.name !== OTHER_CODE); const childName = `${baseName}_choices`; const childId = makeGrpId(childName); const childMembers = nonOther @@ -210,7 +212,7 @@ function emitOtherPatternVars(dataDscr: XmlElement, p: OtherPattern): void { if (p.isMulti) { for (const choice of base.choices) { - if (choice.name === 'other') continue; + if (choice.name === OTHER_CODE) continue; addBinaryVar( dataDscr, makeVarId(`${baseName}_${choice.name}`), diff --git a/src/generated/conventions.json b/src/generated/conventions.json index 82e4915..9ef8283 100644 --- a/src/generated/conventions.json +++ b/src/generated/conventions.json @@ -23,6 +23,7 @@ "ddiVocabFromFilename": "stripExtension", "ddiEmission": "concept[@vocab=''] in place of inline catgry; catgry MUST NOT be emitted", "limesurveyEmission": "base select (L/M) with the vocabulary's options inlined as A/SQ rows + cssclass='cdlvocab-' carrying provenance (a registered LimeSurvey question attribute, so it survives import)", + "limesurveyCssClassPrefix": "cdlvocab-", "structuralNote": "select_multiple_from_file emits a flat with concept/@vocab, NOT a varGrp[@type='multipleResp'] with binary children. This differs from select_multiple.", "appliesTo": [ "select_one_from_file", @@ -304,7 +305,8 @@ } ], "trigger": { - "xlsformPattern": "begin_group row with appearance=table-list (or name contains 'grid', or label contains 'matrix')", + "appearance": "table-list", + "xlsformPattern": "begin_group row with appearance=table-list", "applyTo": "begin_group + all rows until matching end_group" }, "input": { diff --git a/src/generated/conventions.ts b/src/generated/conventions.ts index 47123e4..2d135b7 100644 --- a/src/generated/conventions.ts +++ b/src/generated/conventions.ts @@ -32,6 +32,7 @@ const conventions = { "ddiVocabFromFilename": "stripExtension", "ddiEmission": "concept[@vocab=''] in place of inline catgry; catgry MUST NOT be emitted", "limesurveyEmission": "base select (L/M) with the vocabulary's options inlined as A/SQ rows + cssclass='cdlvocab-' carrying provenance (a registered LimeSurvey question attribute, so it survives import)", + "limesurveyCssClassPrefix": "cdlvocab-", "structuralNote": "select_multiple_from_file emits a flat with concept/@vocab, NOT a varGrp[@type='multipleResp'] with binary children. This differs from select_multiple.", "appliesTo": [ "select_one_from_file", @@ -313,7 +314,8 @@ const conventions = { } ], "trigger": { - "xlsformPattern": "begin_group row with appearance=table-list (or name contains 'grid', or label contains 'matrix')", + "appearance": "table-list", + "xlsformPattern": "begin_group row with appearance=table-list", "applyTo": "begin_group + all rows until matching end_group" }, "input": { diff --git a/src/pipelines/lstsv2ddi/data.ts b/src/pipelines/lstsv2ddi/data.ts index 047db21..4629b8a 100644 --- a/src/pipelines/lstsv2ddi/data.ts +++ b/src/pipelines/lstsv2ddi/data.ts @@ -25,7 +25,7 @@ import type { Variable } from '../../ddi/types.js'; import type { Submission } from '../xlsform2ddi/data.js'; -import { OTHER_CODE, OTHER_SUFFIX } from './toVariables.js'; +import { OTHER_CODE, OTHER_SUFFIX } from '../../conventions/other.js'; /** LimeSurvey's stored value for the "other" option of a list question. */ const LS_OTHER_VALUE = '-oth-'; diff --git a/src/pipelines/lstsv2ddi/toVariables.ts b/src/pipelines/lstsv2ddi/toVariables.ts index 92d70eb..79ff85e 100644 --- a/src/pipelines/lstsv2ddi/toVariables.ts +++ b/src/pipelines/lstsv2ddi/toVariables.ts @@ -16,30 +16,18 @@ * - `F` (array) → grid group of `select_one` variables */ -import conventions from '../../generated/conventions.js'; - import { Choice, Variable } from '../../ddi/types.js'; import { APPEARANCES } from '../../generated/Appearances.js'; - -// Semi-open "other" convention: LimeSurvey carries it as a native `other=Y` -// flag (no code/label in the TSV), so the reverse path re-adds the `other` -// category from these per-language labels and rebuilds the `_other` -// companion so the DDI emitter detects the pattern. -const OTHER = conventions.conventions.other as { - choiceCode?: string; - companionSuffix?: string; - labels?: Record; -}; -/** LS answer code for the collapsed "other" choice (`convention:other`). */ -export const OTHER_CODE = OTHER.choiceCode ?? 'other'; -/** XLSForm companion-question suffix for the "other" free-text follow-up. */ -export const OTHER_SUFFIX = OTHER.companionSuffix ?? '_other'; -const OTHER_LABELS = OTHER.labels ?? {}; - -/** Canonical "other" category label for a survey language (falls back to en). */ -export function otherLabelFor(lang: string): string { - return OTHER_LABELS[lang] ?? OTHER_LABELS['en'] ?? 'Other'; -} +import { + OTHER_CODE, + OTHER_SUFFIX, + otherLabelFor, +} from '../../conventions/other.js'; +import { + fromFileTypeFor, + vocabFromCssClass, +} from '../../conventions/fromFile.js'; +import { GRID_APPEARANCE } from '../../conventions/grid.js'; /** LimeSurvey question-type code → canonical standardized type slug. */ const LS_TO_STD: Record = { @@ -69,26 +57,10 @@ for (const spec of Object.values(APPEARANCES)) { type Row = Record; -/** Prefix of the `cssclass` value carrying external-vocab provenance. */ -export const CDLVOCAB_PREFIX = 'cdlvocab-'; - function cell(row: Row, key: string): string { return (row[key] ?? '').trim(); } -/** - * Extract the vocabulary id from a Q-row's `cssclass` attribute, or `''`. - * - * `select_*_from_file` questions inline their options as A/SQ rows and record - * provenance as `cssclass="cdlvocab-"` (a registered LimeSurvey attribute - * that survives import) — see `xlsformConverter`. - */ -export function vocabFromCssClass(cssclass: string): string { - return cssclass.startsWith(CDLVOCAB_PREFIX) - ? cssclass.slice(CDLVOCAB_PREFIX.length) - : ''; -} - /** * Build a non-array question {@link Variable} from its Q-row fields. A non-empty * `cdlVocab` marks a `select_*_from_file` question: its type gains the @@ -108,8 +80,9 @@ function buildQuestionVar( if (cdlVocab) { vocab = cdlVocab; - if (type === 'select_one') type = 'select_one_from_file'; - else if (type === 'select_multiple') type = 'select_multiple_from_file'; + if (type === 'select_one' || type === 'select_multiple') { + type = fromFileTypeFor(type); + } } else if (type === 'select_one' || type === 'select_multiple') { // Synthetic list keyed by the question; choices fill from A/SQ rows. listName = name; @@ -276,7 +249,7 @@ function drainArray(variables: Variable[], array: ArrayAccumulator): void { label: sq.label, group: array.group, groupLabel: array.label, - groupAppearance: 'table-list', + groupAppearance: GRID_APPEARANCE, listName: array.name, vocab: '', choices: array.answers.map((a) => ({ ...a })), diff --git a/src/pipelines/lstsv2xlsform/toXlsform.ts b/src/pipelines/lstsv2xlsform/toXlsform.ts index f84ef86..6535a4c 100644 --- a/src/pipelines/lstsv2xlsform/toXlsform.ts +++ b/src/pipelines/lstsv2xlsform/toXlsform.ts @@ -28,13 +28,17 @@ import { defaultConfig } from '../../config/types.js'; import type { SurveyRow, ChoiceRow, SettingsRow } from '../../xlsform/types.js'; import { APPEARANCES } from '../../generated/Appearances.js'; import { TYPE_MAPPINGS } from '../../generated/TypeMappings.js'; -import { EXCLUSIVE_RULE } from '../../xlsform/exclusive.js'; +import { EXCLUSIVE_RULE } from '../../conventions/exclusive.js'; import { OTHER_CODE, OTHER_SUFFIX, otherLabelFor, +} from '../../conventions/other.js'; +import { + fromFileTypeFor, vocabFromCssClass, -} from '../lstsv2ddi/toVariables.js'; +} from '../../conventions/fromFile.js'; +import { GRID_APPEARANCE } from '../../conventions/grid.js'; import { formatDefaultLanguage } from './languageNames.js'; import { htmlToMarkdown } from '../../utils/markdownRenderer.js'; @@ -474,7 +478,7 @@ function emitBucketOpen(ctx: BucketOpenCtx): { let groupAppearance: string | undefined; if (items.length === 1 && arrayItem) { groupName = arrayItem.name; - groupAppearance = 'table-list'; + groupAppearance = GRID_APPEARANCE; } else { groupName = slugifyGroupName(groupLabelText); } @@ -619,10 +623,7 @@ function composeTypeWithList( ): { type: string; emittedChoices: boolean } { const vocab = vocabFromCssClass(item.cssclass); if (vocab) { - const fromFile = - base === 'select_one' - ? 'select_one_from_file' - : 'select_multiple_from_file'; + const fromFile = fromFileTypeFor(base); return { type: `${fromFile} ${vocab}.csv`, emittedChoices: false }; } if (base === 'select_one' || base === 'select_multiple') { diff --git a/src/pipelines/xlsform2ddi/data.ts b/src/pipelines/xlsform2ddi/data.ts index ceea5ba..46f1c2e 100644 --- a/src/pipelines/xlsform2ddi/data.ts +++ b/src/pipelines/xlsform2ddi/data.ts @@ -14,6 +14,7 @@ import { splitDataVars } from '../../ddi/codebook.js'; import type { DataVarBuckets, OtherPattern } from '../../ddi/codebook.js'; import { classifyNotes } from '../../ddi/notes.js'; +import { OTHER_CODE } from '../../conventions/other.js'; import type { Variable } from '../../ddi/types.js'; /** One raw response record, keyed by question name or `group/name` path. */ @@ -42,7 +43,7 @@ function binary(variable: Variable, choice: string): Column { function otherPatternColumns(p: OtherPattern): Column[] { const cols: Column[] = p.isMulti ? p.base.choices - .filter((c) => c.name !== 'other') + .filter((c) => c.name !== OTHER_CODE) .map((c) => binary(p.base, c.name)) : [single(p.base)]; // The `_other` free text is always its own column. diff --git a/src/pipelines/xlsform2ddi/variables.ts b/src/pipelines/xlsform2ddi/variables.ts index ec10b29..e2de684 100644 --- a/src/pipelines/xlsform2ddi/variables.ts +++ b/src/pipelines/xlsform2ddi/variables.ts @@ -7,7 +7,17 @@ */ import { APPEARANCES } from '../../generated/Appearances.js'; -import conventions from '../../generated/conventions.js'; +import { + OTHER_APPLIES_TO, + OTHER_CODE, + OTHER_COMPANION_TYPE, + OTHER_SUFFIX, + otherLabelFor, +} from '../../conventions/other.js'; +import { + isFromFileType, + vocabFromFilename, +} from '../../conventions/fromFile.js'; import { TYPE_MAP, NON_DDI_EMITTABLE_TYPES, @@ -26,24 +36,11 @@ type Row = Record; // expanded here — otherwise `or_other` produced no `other` category and no // free-text variable at all, silently dropping a column both LimeSurvey and ODK // store. -const OTHER = conventions.conventions.other as { - choiceCode?: string; - companionSuffix?: string; - companionType?: string; - labels?: Record; - appliesTo?: string[]; -}; -const OTHER_CODE = OTHER.choiceCode ?? 'other'; -const OTHER_SUFFIX = OTHER.companionSuffix ?? '_other'; -const OTHER_COMPANION_TYPE = OTHER.companionType ?? 'text'; -const OTHER_LABELS = OTHER.labels ?? {}; /** XLSForm type-string token marking the shorthand form. */ const OR_OTHER_TOKEN = 'or_other'; /** Types the convention applies to (registry: `convention:other.appliesTo`). */ -const OTHER_TYPES = new Set( - OTHER.appliesTo ?? ['select_one', 'select_multiple'], -); +const OTHER_TYPES = new Set(OTHER_APPLIES_TO); /** * Language of the label column in use (`label::German (de)` → `de`), so the @@ -58,11 +55,6 @@ function langFromLabelCol(labelCol: string): string { return /^[a-z]{2}$/i.test(suffix) ? suffix.toLowerCase() : 'en'; } -/** Canonical "other" label for a language, falling back to English. */ -function otherLabelFor(lang: string): string { - return OTHER_LABELS[lang] ?? OTHER_LABELS['en'] ?? 'Other'; -} - /** * Types skipped during variable extraction: structural + non-emittable, * minus `note` (kept so note rows can be classified into ``/``). @@ -161,14 +153,9 @@ function resolveType(baseType: string, rawType: string): ResolvedType { vocab: '', }; } - if ( - baseType === 'select_one_from_file' || - baseType === 'select_multiple_from_file' - ) { + if (isFromFileType(baseType)) { const filename = rawType.split(/\s+/).slice(1).join(' '); - const vocab = filename.toLowerCase().endsWith('.csv') - ? filename.slice(0, -'.csv'.length) - : filename; + const vocab = vocabFromFilename(filename); return { stdType: baseType, listName: '', vocab }; } return { stdType: TYPE_MAP[baseType] ?? baseType, listName: '', vocab: '' }; diff --git a/src/pipelines/xlsform2lstsv/constants.ts b/src/pipelines/xlsform2lstsv/constants.ts index 8c46eaf..fc4b77f 100644 --- a/src/pipelines/xlsform2lstsv/constants.ts +++ b/src/pipelines/xlsform2lstsv/constants.ts @@ -1,10 +1,9 @@ import { TYPE_MAPPINGS } from './typeMapper.js'; -import conventions from '../../generated/conventions.js'; +import { METADATA_ROW_TYPES } from '../../conventions/metadata.js'; // Derived from registry convention:unregisteredRows — device/session metadata // rows are silently skipped; any other unregistered type is an error. -export const SKIP_TYPES: string[] = - conventions.conventions.unregisteredRows.metadataRowTypes; +export const SKIP_TYPES: readonly string[] = METADATA_ROW_TYPES; // Derived from registry: registered types LimeSurvey TSV cannot express // (supported: false) throw an error. begin_group/end_group are special-cased @@ -13,22 +12,6 @@ export const UNIMPLEMENTED_TYPES: string[] = Object.entries(TYPE_MAPPINGS) .filter(([k, v]) => !v.supported && k !== 'begin_group' && k !== 'end_group') .map(([k]) => k); -// select_*_from_file has no native LimeSurvey representation, but when the -// referenced vocabulary is registered or supplied (via convert()'s fileChoices) -// we emit it as the base select with the vocabulary's options inlined + a `cdl_vocab` attribute -// recording the source vocabulary (see convention:externalCodeList). -export const FROM_FILE_BASE: Record = { - select_one_from_file: 'select_one', - select_multiple_from_file: 'select_multiple', -}; - -// Canonical per-language "other" labels (convention:other). Used to verify an -// author's `other` choice label so the DDI round-trip (which rebuilds this -// label from the convention) stays faithful. -export const OTHER_LABELS: Record = - (conventions.conventions.other as { labels?: Record }) - .labels ?? {}; - // TSV row data interface export interface TSVRowData { class: string; diff --git a/src/pipelines/xlsform2lstsv/groupEmitter.ts b/src/pipelines/xlsform2lstsv/groupEmitter.ts index 017a074..f062017 100644 --- a/src/pipelines/xlsform2lstsv/groupEmitter.ts +++ b/src/pipelines/xlsform2lstsv/groupEmitter.ts @@ -3,6 +3,7 @@ import { ConfigManager } from '../../config/ConfigManager.js'; import { RowEmitter } from './rowEmitter.js'; import { LanguageHandler } from './languageHandler.js'; import { GroupStackItem } from './constants.js'; +import { isGridAppearance } from '../../conventions/grid.js'; import { Counters } from './counters.js'; /** @@ -200,7 +201,7 @@ export class GroupEmitter { const groupAppearance = typeof row['appearance'] === 'string' ? row['appearance'].trim() : ''; - if (groupAppearance.includes('table-list')) { + if (isGridAppearance(groupAppearance)) { this.groupStack.push({ originalName, sanitizedName, diff --git a/src/pipelines/xlsform2lstsv/groupProcessor.ts b/src/pipelines/xlsform2lstsv/groupProcessor.ts index 095c5c1..5f9d71b 100644 --- a/src/pipelines/xlsform2lstsv/groupProcessor.ts +++ b/src/pipelines/xlsform2lstsv/groupProcessor.ts @@ -1,6 +1,7 @@ import { SurveyRow } from '../../xlsform/types.js'; import { ConfigManager } from '../../config/ConfigManager.js'; import { SKIP_TYPES } from './constants.js'; +import { isGridAppearance } from '../../conventions/grid.js'; export interface GroupInfo { originalName: string; @@ -152,7 +153,7 @@ export class GroupProcessor { groupName: string, groupAppearance: string, ): { emit: boolean; isTableList: boolean } { - const isTableList = groupAppearance.includes('table-list'); + const isTableList = isGridAppearance(groupAppearance); const isMessageOnly = this.messageOnlyGroups.has(groupName); const isParentOnly = this.parentOnlyGroups.has(groupName); diff --git a/src/pipelines/xlsform2lstsv/index.ts b/src/pipelines/xlsform2lstsv/index.ts index 5091c94..175f820 100644 --- a/src/pipelines/xlsform2lstsv/index.ts +++ b/src/pipelines/xlsform2lstsv/index.ts @@ -6,12 +6,7 @@ import { TSVGenerator } from '../../lstsv/serialize.js'; import { TypeMapper, TYPE_MAPPINGS } from './typeMapper.js'; // Import extracted constants -import { - SKIP_TYPES, - UNIMPLEMENTED_TYPES, - FROM_FILE_BASE, - TSVRowData, -} from './constants.js'; +import { SKIP_TYPES, UNIMPLEMENTED_TYPES, TSVRowData } from './constants.js'; import { ChoiceManager } from './choiceManager.js'; import { GroupProcessor } from './groupProcessor.js'; import { LanguageHandler } from './languageHandler.js'; @@ -27,7 +22,12 @@ import { FieldNameHandler } from './fieldNameHandler.js'; import { AppearanceHandler } from './appearanceHandler.js'; import { registeredFileChoices, registeredVocabFiles } from '../../vocab.js'; import { parameterAttributes } from './parameters.js'; -import { EXCLUSIVE_RULE, isExclusive } from '../../xlsform/exclusive.js'; +import { EXCLUSIVE_RULE, isExclusive } from '../../conventions/exclusive.js'; +import { + FROM_FILE_BASE, + cssClassForVocab, + vocabFromFilename, +} from '../../conventions/fromFile.js'; // Registry appearances are an allowlist: only 'handled' entries are // registered. Anything else (or a handled appearance on the wrong type) @@ -425,7 +425,7 @@ export class XLSFormToTSVConverter { // native slot for. listName stays the filename (choicesMap was keyed by it). let cdlVocab = ''; if (xfTypeInfo.base in FROM_FILE_BASE) { - cdlVocab = (xfTypeInfo.listName ?? '').replace(/\.csv$/i, ''); + cdlVocab = vocabFromFilename(xfTypeInfo.listName ?? ''); xfTypeInfo = { ...xfTypeInfo, base: FROM_FILE_BASE[xfTypeInfo.base] }; } @@ -632,7 +632,7 @@ export class XLSFormToTSVConverter { // LimeSurvey import and be queryable, unlike an unregistered attribute. // It's a machine hook only (no styling effect); the faithful reference // still lives in DDI's concept/@vocab. Empty for all other questions. - ...(cdlVocab ? { cssclass: `cdlvocab-${cdlVocab}` } : {}), + ...(cdlVocab ? { cssclass: cssClassForVocab(cdlVocab) } : {}), // Bounds etc. from the `parameters` column (range), per the registry. ...attributes, }; diff --git a/src/pipelines/xlsform2lstsv/otherPatternDetector.ts b/src/pipelines/xlsform2lstsv/otherPatternDetector.ts index bc821a9..0863663 100644 --- a/src/pipelines/xlsform2lstsv/otherPatternDetector.ts +++ b/src/pipelines/xlsform2lstsv/otherPatternDetector.ts @@ -2,7 +2,11 @@ import { SurveyRow, ChoiceRow } from '../../xlsform/types.js'; import { TypeInfo } from './typeMapper.js'; import { ChoiceManager } from './choiceManager.js'; import { LanguageHandler } from './languageHandler.js'; -import { OTHER_LABELS } from './constants.js'; +import { + OTHER_CODE, + OTHER_LABELS, + OTHER_SUFFIX, +} from '../../conventions/other.js'; /** * Detects the "X_other" pattern: a follow-up question with relevance @@ -11,13 +15,6 @@ import { OTHER_LABELS } from './constants.js'; * LimeSurvey seeing two "other" options. */ export class OtherPatternDetector { - private readonly otherNames = new Set([ - 'other', - '_other', - 'other_option', - 'other_choice', - ]); - constructor( private choiceManager: ChoiceManager, private languageHandler: LanguageHandler, @@ -36,7 +33,7 @@ export class OtherPatternDetector { const currentName = currentRow.name?.trim(); if (!currentName) return false; - const otherQuestionName = `${currentName}_other`; + const otherQuestionName = `${currentName}${OTHER_SUFFIX}`; const sanitizedCurrentName = sanitizeName(currentName); for (const row of surveyData) { @@ -44,9 +41,12 @@ export class OtherPatternDetector { const relevance = row.relevant.trim(); // Pattern: ${name} = 'other', ${name} == 'other', or selected(${name}, 'other') + const code = escapeRegExp(OTHER_CODE); const patterns = [currentName, sanitizedCurrentName].flatMap((n) => [ - new RegExp(`\\$\\{${n}\\}\\s*={1,2}\\s*['"]other['"]`), - new RegExp(`selected\\(\\s*\\$\\{${n}\\}\\s*,\\s*['"]other['"]\\s*\\)`), + new RegExp(`\\$\\{${n}\\}\\s*={1,2}\\s*['"]${code}['"]`), + new RegExp( + `selected\\(\\s*\\$\\{${n}\\}\\s*,\\s*['"]${code}['"]\\s*\\)`, + ), ]); if (patterns.some((p) => p.test(relevance))) { @@ -65,12 +65,10 @@ export class OtherPatternDetector { const choices = this.choiceManager.getChoices(typeInfo.listName); if (!choices) return; - const removed = choices.filter((choice) => - this.otherNames.has(choice.name?.trim().toLowerCase() || ''), - ); - const filteredChoices = choices.filter( - (choice) => !this.otherNames.has(choice.name?.trim().toLowerCase() || ''), - ); + const isOther = (choice: ChoiceRow) => + (choice.name?.trim().toLowerCase() || '') === OTHER_CODE; + const removed = choices.filter(isOther); + const filteredChoices = choices.filter((choice) => !isOther(choice)); if (filteredChoices.length < choices.length) { console.log( @@ -102,3 +100,7 @@ export class OtherPatternDetector { } } } + +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} diff --git a/src/xlsform/validate.ts b/src/xlsform/validate.ts index 3672309..4e0747c 100644 --- a/src/xlsform/validate.ts +++ b/src/xlsform/validate.ts @@ -4,7 +4,15 @@ import { TYPE_MAPPINGS } from '../generated/TypeMappings.js'; import { SurveyRow, ChoiceRow } from './types.js'; import { registeredVocabFiles } from '../vocab.js'; -import { EXCLUSIVE_RULE, exclusiveCell, isExclusive } from './exclusive.js'; +import { + EXCLUSIVE_RULE, + exclusiveCell, + isExclusive, +} from '../conventions/exclusive.js'; +import { OTHER_SUFFIX } from '../conventions/other.js'; +import { isFromFileType } from '../conventions/fromFile.js'; +import { METADATA_ROW_TYPES } from '../conventions/metadata.js'; +import { normalizeName } from './identifiers.js'; const NAME_RULES = conventions.conventions.sanitization.name; const CHOICE_RULES = conventions.conventions.sanitization.choiceCode; @@ -23,9 +31,7 @@ const STRUCTURAL = new Set([ ]); // Metadata rows silently skipped by the converter (convention:unregisteredRows). -const METADATA_TYPES = new Set( - conventions.conventions.unregisteredRows.metadataRowTypes, -); +const METADATA_TYPES = new Set(METADATA_ROW_TYPES); /** A single subset-validation finding. */ export interface SubsetViolation { @@ -63,10 +69,9 @@ export interface ValidateAllOpts { choicesSheetName?: string; } -// The semi-open `_other` follow-up is a registry convention. LimeSurvey -// carries "other" via its native `other=Y` setting, so this underscore is a -// source-side marker, not a literal LS code — validate only the `` part. -const OTHER_SUFFIX = '_other'; +// The semi-open `_other` follow-up (convention:other): LimeSurvey +// carries "other" via its native `other=Y` setting, so the suffix's underscore +// is a source-side marker, not a literal LS code — validate only ``. export class XLSValidator { /** @@ -320,7 +325,9 @@ export class XLSValidator { // it sanitizes to (base + "other"), not the raw underscore form. const isOther = name.endsWith(OTHER_SUFFIX); const base = isOther ? name.slice(0, -OTHER_SUFFIX.length) : name; - const lsLength = isOther ? base.length + 'other'.length : name.length; + const lsLength = isOther + ? base.length + normalizeName(OTHER_SUFFIX).length + : name.length; if (!lsRules) { // DDI keeps names as authored; only uniqueness below applies. @@ -522,7 +529,7 @@ export class XLSValidator { target === 'lstsv' && mapping.supported === false && mapping.limeSurveyType === null && - !baseType.endsWith('_from_file') + !isFromFileType(baseType) ) { return `type "${baseType}"${where} is registered but not expressible in LimeSurvey TSV`; } @@ -544,7 +551,7 @@ export class XLSValidator { fileChoices: Record, ): string | null { const target = rawType.split(/\s+/)[1]; - if (baseType.endsWith('_from_file')) { + if (isFromFileType(baseType)) { if (!target) { return `"${baseType}"${where} needs a vocabulary file: "${baseType} .csv"`; } diff --git a/tests/ts/unit/conventionLiterals.test.ts b/tests/ts/unit/conventionLiterals.test.ts new file mode 100644 index 0000000..30b7af8 --- /dev/null +++ b/tests/ts/unit/conventionLiterals.test.ts @@ -0,0 +1,48 @@ +/** + * #60: registry convention values have one implementation, in src/conventions/ + * (read from src/generated/). Anywhere else a literal copy silently ignores a + * registry change, so this test fails on one. + */ +import { readdirSync, readFileSync, statSync } from 'node:fs'; +import { join, relative, resolve } from 'node:path'; + +import { describe, expect, test } from 'vitest'; + +const SRC = resolve(__dirname, '../../../src'); +const ALLOWED = ['generated', 'conventions']; + +/** Convention values that must not be written out as string literals. */ +const FORBIDDEN: [string, RegExp][] = [ + ['other companion suffix', /['"`]_other['"`]/], + ['from-file type', /['"`]select_(one|multiple)_from_file['"`]/], + ['vocabulary cssclass prefix', /['"`]cdlvocab-/], + ['grid appearance', /['"`]table-list['"`]/], + ['other labels', /['"`]Sonstiges['"`]/], +]; + +function sourceFiles(dir: string): string[] { + return readdirSync(dir).flatMap((name) => { + const path = join(dir, name); + if (statSync(path).isDirectory()) { + return ALLOWED.includes(name) && dir === SRC ? [] : sourceFiles(path); + } + return path.endsWith('.ts') ? [path] : []; + }); +} + +/** Code only: drop line and block comments so documentation may name values. */ +function stripComments(code: string): string { + return code.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*\/\/.*$/gm, ''); +} + +describe('convention values live in src/conventions only', () => { + test.each(FORBIDDEN)( + 'no %s literal outside src/conventions', + (_, pattern) => { + const hits = sourceFiles(SRC).filter((f) => + pattern.test(stripComments(readFileSync(f, 'utf-8'))), + ); + expect(hits.map((f) => relative(SRC, f))).toEqual([]); + }, + ); +});