Repository navigation
Fix handling of extensions and update tests - #936
Merged
Merged
Conversation
Split src/schema/entries.js and src/schema/parser.js into one file per class under src/schema/entries/ and src/schema/parser/, with the hed-typescript file and method names. Rename Schema, Schemas and SchemaXmlCollection to HedSchema, HedSchemas and HedSchemaXMLCollection; the old names stay as aliases through 4.x. A schema attribute now holds one set of properties and a recursive flag; the version rules for extensionAllowed (before 8.2.0) and inLibrary (before 8.3.0) are part of that computation. Every property a schema defines is registered, so 8.3.0 and later gain 13 domain/range properties; no tag, attribute, unit or value class changes (dump of 122 schema groups). parseSchemaXML sets the parent links, so a schema file without a HED root or schema element fails as localSchemaLoadFailed instead of a TypeError. Schema faults use the invalidSchema code. Memoizer and core-js-pure are gone. Removed without a major version: SchemaProperty.isCategoryProperty, isTypeProperty, isRoleProperty; SchemaAttribute.categoryProperty, typeProperty, roleProperties, propertyNames; getAttributeValue (now getSingleAttributeValue and getAttributeValues). The schema entry classes were never exported by the package; they are reachable only inside the object buildSchemasFromVersion returns, no known consumer uses these members (bids-validator imports only the Schemas type, which is kept), and the schema class declarations are to be withdrawn from types/index.d.ts before 4.3.0 (realignment plan, phase 5). The schema declarations now match the runtime: members that never existed, such as getValue and SchemaTag.isUnitClass, are removed, and SchemaUnitClass.units is declared as the Map it always was. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A "#" after an extension term or in place of one (Item/Blech/#, Item/#) is PLACEHOLDER_INVALID in a sidecar as in an events file; Onset/# stays TAG_EXTENSION_INVALID. An extension term must contain a letter (A-Z or a-z, or any Unicode letter from standard schema 8.3.0), so Item/Blech/3 and Item/1-2 are TAG_EXTENSION_INVALID for every schema version (hed-specification PR hed-standard#726). Move hed-tests to 54f137e, whose new cases pass with no skips. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Valid Unicode extensions can be rejected, and the new schema declarations expose constructors unavailable from the package entry point.
1 open issue
What changed in this PR
This PR corrects tag-extension validation in hed-validator and reorganizes the schema layer, with corresponding declaration and test updates.
Changes:
- Distinguishes invalid placeholders from extensions and requires extension terms to contain a letter.
- Splits schema entries and parsers into dedicated modules, renames containers, and simplifies attribute handling.
- Updates TypeScript declarations, regression tests, documentation, and dependencies.
| File | Description |
|---|---|
| types/test.ts | Updates schema API type checks. |
| types/index.d.ts | Revises schema names and member declarations. |
| tests/otherTests/unmergedSchemas.spec.js | Uses the single-value attribute accessor. |
| tests/otherTests/schemaParser.spec.js | Adds schema parsing and compatibility tests. |
| tests/otherTests/characterSets.spec.js | Updates the value-class import. |
| tests/jsonTests/tagParserTests.spec.js | Updates the value-tag import. |
| tests/jsonTestData/tagParserTests.data.js | Revises placeholder error expectations. |
| tests/jsonTestData/stringParserTests.data.js | Revises extension-placeholder expectations. |
| src/utils/xml.js | Normalizes XML definitions and assigns parents during parsing. |
| src/utils/memoizer.js | Removes the shared memoizer. |
| src/schema/xmlType.js | Extracts XML element helpers. |
| src/schema/xmlCollection.js | Renames the collection and retains an alias. |
| src/schema/specs.js | Adds specification comparison and input validation. |
| src/schema/parser/valueClass.js | Extracts value-class parsing. |
| src/schema/parser/unitModifier.js | Extracts unit-modifier parsing. |
| src/schema/parser/unitClass.js | Extracts unit-class parsing and merging. |
| src/schema/parser/tag.js | Extracts tag parsing and attribute propagation. |
| src/schema/parser/schemaParser.js | Coordinates entry parsers. |
| src/schema/parser/schemaEntry.js | Extracts shared parsing and merge logic. |
| src/schema/parser/property.js | Registers schema properties. |
| src/schema/parser/attribute.js | Builds simplified attributes and recursion flags. |
| src/schema/parser.js | Removes the monolithic parser module. |
| src/schema/loader.js | Uses the default XML parser export. |
| src/schema/init.js | Integrates renamed containers and extracted parsers. |
| src/schema/entries/valueTag.js | Extracts value-tag behavior and comparison. |
| src/schema/entries/valueClass.js | Extracts value-class validation. |
| src/schema/entries/unitModifier.js | Extracts the unit-modifier class. |
| src/schema/entries/unitClass.js | Extracts unit-class behavior. |
| src/schema/entries/unit.js | Extracts unit validation and conversion. |
| src/schema/entries/tag.js | Extracts tag behavior and ancestor caching. |
| src/schema/entries/schemaEntryWithAttributes.js | Introduces explicit single/all-value accessors. |
| src/schema/entries/schemaEntryManager.js | Extracts entry management without memoization. |
| src/schema/entries/schemaEntry.js | Extracts the base entry and adds name comparison. |
| src/schema/entries/schemaEntries.js | Extracts the schema entry container. |
| src/schema/entries/property.js | Simplifies the property class. |
| src/schema/entries/attribute.js | Stores properties and recursion status. |
| src/schema/entries.js | Removes the monolithic entry module. |
| src/schema/containers.js | Renames schema containers and preserves aliases. |
| src/parser/tagConverter.js | Updates placeholder and extension-letter validation. |
| src/parser/parsedHedTag.js | Uses the extracted value-tag module. |
| src/issues/data.js | Adds the invalid-schema issue. |
| package.json | Removes the Set polyfill dependency. |
| package-lock.json | Records the dependency removal. |
| CHANGELOG.md | Documents validation and schema-layer changes. |
| .github/copilot-instructions.md | Updates the architecture documentation. |
Review effort: Balanced
VisLab
added a commit
to VisLab/hed-javascript
that referenced
this pull request
Oct 23, 2026
The letter test of specification 3.2.5 rule 6 keyed the Unicode form on
the literal character set name nonascii, so a schema whose nameClass
declaration allows non-ASCII through the composite set name or a literal
character (honored from HED 8.5.0) would reject an extension such as
Item/ete with accents. Test /\p{L}/u for every schema: a name class
without non-ASCII characters has already rejected them, so the result on
every shipped schema is unchanged (Copilot review of PR hed-standard#936).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.