Skip to content

Offer the derived properties with a derived property element - #98

Draft
jwrosewell wants to merge 8 commits into
rcfrom
feature/derived-properties
Draft

jwrosewell wants to merge 8 commits into
rcfrom
feature/derived-properties

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Node.js had no element for the derived properties that derived-properties defines, so a Node.js pipeline could not offer HumanConfidence or IsGdpr, whilst .NET, Rust and PHP can. This adds a pipeline/fiftyone.pipeline.derivedproperty package whose DerivedPropertyElement runs every script in the scripts folder of derived-properties, and tests that run every case file in its tests folder, the scripts under tests/invalid included.

Closes #80.

What the specification says

All quotes are from derived-properties at cc306386, the head of its main and the commit the new submodule pins.

docs/format-1.md, lines 4 to 6:

Every language implementation of the element (.NET, Node, Java, Python and PHP) is tested against the behaviour described here, so this document is the contract rather than a summary of one.

The one rule, lines 35 to 39:

Where every one of those properties is available, the checks and the rules run and a value is chosen. Where any one of them is not available, the script produces no value, and the message names every property that was missing along with what its source element said about each one.

When a property is absent, lines 451 to 455:

Nothing is evaluated. The output property is present in the element data with no value, and reading it raises the language's existing no value error, which is AspectPropertyValue with a NoValueMessage in .NET and the matching mechanism in each other language. The message names every absent property rather than only the first one found.

Lines 491 to 497, on an element added after a source it reads:

Where an element runs is not judged at build, since a pipeline holding elements that run in parallel reports them in an order that does not say what ran before what. An element added after a source it reads therefore builds, and produces no value on every request, with this message naming the property. An implementation should log that once rather than on every request, because it will otherwise repeat for the life of the process.

Writing YAML, lines 562 to 570:

YAML 1.1 parsers, which several languages still ship, read yes, no, on and off as booleans ... The reference tools in this repository use the YAML core schema, where yes stays the string "yes"

Two places where the languages are allowed to differ, lines 590 to 605, being the line a fault names, and how strictly a JSON script is read:

Text that is broken is reported as broken either way and only the wording of the fault differs, so no script is valid in one language and invalid in another.

docs/testing.md, lines 3 to 6:

every 51Degrees language implementation runs the same case files, so a case written here becomes a test in each language as that language is built, starting with .NET and followed by Node, Java, Python and PHP.

Lines 177 and 196 to 197:

Every property that could not be read has to be listed, so a case naming one of two unreadable properties fails

tests/invalid/ holds scripts that every implementation must refuse, along with what each refusal has to say.

docs/authoring.md, lines 365 to 384 asks for one information line per script at build naming "the script, its version, the format number, where the script came from (a built in name, a file path or code) and the output property", and at debug level "the compiled model as canonical JSON". Lines 441 to 442 add a warning for a deprecated script. Lines 452 to 464 describe the check when the element is added, where "a source property that no earlier element supplies" and "two elements in one pipeline producing the same derived property name" fail the pipeline build. The word "earlier" there is at odds with format-1.md lines 491 to 494 above, and .NET, Rust and PHP accept an element that supplies the property wherever it sits, so Node.js follows the format reference, which calls itself the contract.

README.md, lines 80 to 109 names the build parameters Scripts and ScriptFiles, each "one string with commas between the names", where "a name that holds a comma is wrapped in double quotes".

What changed

The reference implementations read were .NET, pipeline/DerivedProperty of 51degrees-dotnet, Rust, the derived-properties crate of 51degrees-rust, and PHP, pipeline/derivedproperty of 51degrees-php. The Node.js code is the reference tools the specification follows, tools/parse.mjs, validate.mjs, evaluate.mjs and canonical.mjs, rewritten as CommonJS in this repository's style with the same logic, fault paths and messages. Files are linked at the head, a8fb0e48.

  1. The submodule. .gitmodules adds derived-properties as pipeline/fiftyone.pipeline.derivedproperty/derived-properties with the relative URL ../derived-properties, pinned at cc306386. The package reads the built in scripts from it directly, so there is no copy to keep in step. .eslintignore leaves the submodule out of the lint.
  2. The package carries the scripts. files in the package's package.json and remote_package.json packs the package's own code, lib and the submodule's scripts folder, and nothing else from the submodule.
  3. Reading YAML. The package requires js-yaml ^4.1.0 and reads with its CORE_SCHEMA, so yes stays the string "yes" as the reference tools read it (lib/parse.js). JSON scripts are read with JSON.parse, the strict reading the specification allows.
  4. The element. DerivedPropertyElement writes one AspectPropertyValue per script under derived, through DerivedPropertyData, where names are matched without regard to case. A script with a source property missing has no value, and reading value throws the message naming every missing property, which is the Node.js pipeline's no value error. It reads a source as Node.js elements hold it, carrying the source's own noValueMessage, or the message of the error an element throws for a property it does not have, and turning a weighted list into the Value and Weight shape the evaluator takes. A script reads what an earlier script of the element wrote. A request with no element data at all under a key a script reads is logged once for each script, as format-1.md lines 491 to 497 ask.
  5. Configuration. The constructor takes the scripts and scriptFiles settings, which a pipeline configuration file gives, matched without regard to case, so Scripts and ScriptFiles work as the README names them. DerivedPropertyElementBuilder offers addBuiltInScript, addScriptFile and addScript in code, as .NET's builder does. A file name with * or ? adds every file in its folder whose name matches, in sorted order, ignoring letter case on Windows and Apple systems, whose default file systems ignore it, as the Rust crate does. Every fault in every script is reported in one ScriptValidationError. A setting the element does not have is refused, so a setting written wrongly cannot leave its scripts out without a word, and an element with no script is refused with a message saying how to give one.
  6. When the pipeline is built. onRegistration refuses the pipeline with the same messages as .NET, Rust and PHP where nothing supplies a source property or another element writes the same derived property, with the elements that run in parallel opened out. Where an element sits is not judged, and an element that lists no properties may supply any, because a cloud engine learns its properties from its first response. The line and the canonical JSON authoring.md asks for, and a warning for a deprecated script, are logged on the next tick, because the pipeline is still being built when onRegistration runs and nothing can be listening to its log yet.
  7. Metadata. Each property's metadata has the keys the other Node.js elements give, being name, type, category, description and isList, followed by the rest of the script's Output block.
  8. Wiring. The root package.json adds the package to the workspaces, so npm install links it, the build's lint covers it and the unit tests run its tests. It is not in ci/packages.json, which lists the packages Publish releases, so Publish leaves it out. AGENTS.md, the README and pipeline/readme.md describe the package and the submodule.

The tests, in pipeline/fiftyone.pipeline.derivedproperty/tests, 224 in all:

  • sharedCases.test.js runs the 69 value cases (57 for HumanConfidence and 12 for IsGdpr) twice, once with the source values as AspectPropertyValue and once as plain values, each through a real pipeline with a source element giving the case's properties and the script the package carries, and the 29 rejection cases, each refused with a fault at every expected path and mentioning every expected fragment. A missing value must name exactly the properties the case lists.
  • builtInScripts.test.js checks that the built in scripts are the submodule's scripts folder and builds one element with every one of them.
  • pipelineCheck.test.js follows the Rust crate's tests/pipeline_check.rs.
  • element.test.js and scripts.test.js cover reading the values Node.js elements give, logging, settings (a wildcard's letter case on each platform included), metadata, the JSON Builder, YAML and JSON, type inference and conversion.

Runs

Each throwaway run uses a workflow that installs the workspace, lints the package with ESLint and runs its tests with Jest, on Ubuntu with Node.js 22 and 24 and on Windows with Node.js 24. A fourth job packs the package with remote_package.json in place, as Publish would, lists the tarball, installs it into an empty project with its dependencies from npm, and runs the built in scripts.

  • Red, run 43526522960 on 2f5f869f, failure. Its parent is the test commit dfe96bd, which holds the submodule, the package's two manifests and the tests, and none of the package's code. Every test job ended "Test Suites: 5 failed, 5 total" and "Tests: 0 total", each suite failing on Cannot find module for the element, the builder, lib/validate or the package itself, and the pack job failed with Cannot find module '.../fiftyone.pipeline.derivedproperty/index.js'.
  • Green, run 43527910374 on 4e6ffdb9, success. Its parent is c155270, the head before rc was merged in, and nothing else changes. Each test job passed the lint and ran "Tests: 221 passed, 221 total". The pack job packed 14 files, among them derived-properties/scripts/HumanConfidence.yaml and IsGdpr.yaml and no other file of the submodule, installed the tarball into an empty project, built both scripts as HumanConfidence 0.8.0 from built in HumanConfidence and IsGdpr 0.1.0 from built in IsGdpr, and answered IsGdpr true for FR.
  • An earlier green, run 43526537505, failed the lint on a byte order mark written as the character itself in a regular expression (no-irregular-whitespace). c155270 tests for it by its code instead, and adds a description to five constructors.
  • Since then rc was merged in twice, at a8fb0e48 and at the head 667d9db8, the second taking #37's version per package, each with no conflict and no change to the package. Give each package its own version and release only what changed #37 lists the packages Publish releases in ci/packages.json, which does not name this one.
  • The review of 667d9db found that a wildcard ignored letter case on Windows only, while macOS's default file system ignores it too and the Rust crate ignores it on both. The test commit e75b152 checks each platform and 6efb0657 ignores case on darwin as well.
  • Red, run 43590941555 on 526f453, failure. Its parent is the test commit e75b152. In every test job only the darwin case failed ("Tests: 1 failed, 223 passed, 224 total"), its wildcard *.YAML matching neither Band.yaml nor Other.yaml.
  • Green, run 43590947211 on 8ac7251, success. Its parent is the head 6efb0657. Every test job ran "Tests: 224 passed, 224 total".
  • Full build, run 43590959558 from tmp/pr-matrix-98d, success. Configure, all six Build and Test jobs (Ubuntu x64 and ARM and Windows, Node.js 22 and 24) and Complete are green. All 7 Setting 'PullRequestSha' lines name the head 6efb0657, and Configure printed "Left out 2 macOS entries".
  • An earlier full build on 667d9db, run 43584811610 from tmp/pr-matrix-98c, success. Configure, all six Build and Test jobs (Ubuntu x64 and ARM and Windows, Node.js 22 and 24) and Complete are green. All 7 Setting 'PullRequestSha' lines name 667d9db8, and Configure printed "Left out 2 macOS entries". An earlier full build on a8fb0e4, run 43560055156, was green too once its Windows Node.js 24 job was run again, the first attempt having failed to start Chrome in the Selenium contract tests ("session not created from disconnected: unable to connect to renderer").

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

  • The package is not published, as it is not in ci/packages.json. When to publish a new package is a separate choice.
  • files names the package's own code as *.js. npm reads a pattern with no slash by file name at every depth inside a folder that another files entry opens, so a .js file at the top of the derived-properties submodule would be packed too. derived-properties has no .js file today, and the green run's pack lists none. Anchoring the pattern as /*.js is the fix #78's change makes for the OWID library, and is left for when the package is first published.
  • On-premise Device Detection reads a Boolean property through the C library's getValueAsBool, which answers value == "True" (ResultsBase.cpp lines 118 to 140), so a Boolean property stored as Unknown reaches Node.js as false. HumanConfidence compares device.IsVisible and device.HasWebDriver with "Unknown", and where the data file types those properties as Boolean that comparison is never true through that engine. PHP found the same and raised it as 51degrees-php#95. It needs a data file to check, so it was not checked here.
  • The consumer tests of derived-properties build only .NET, and a row for Node.js is a change in that repository, as derived-properties#18 is for PHP.
  • .NET also lets a script replace a property of another element, by an Output.Name naming that element. Format 1 as published at cc306386 asks Output.Name to match ^[A-Za-z][A-Za-z0-9]*$, so Node.js follows the format and has no such scripts.

…rty element

The tests run every case file and every rejection case in the submodule's
tests folder through a pipeline, and test the element's settings, logging,
metadata and the checks it makes when a pipeline is built. The element
itself follows in the next commit.
fiftyone.pipeline.derivedproperty runs the scripts of derived-properties,
built into the package from the submodule's scripts folder or given as
files or text, and writes one property for each under derived. lib holds
the reference parser, validator, evaluator and canonical printer of
derived-properties as CommonJS. The package is tested but not yet
published.
…stems

The default file systems of both ignore letter case, as the Rust crate's
wildcard does on both, and Node.js ignored it on Windows only.
…dows

macOS's default file system ignores letter case, as Windows's does, so
a wildcard now matches names in any case there too, as the Rust crate's
does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant