fix: move moldb-count-rows options under variables, and correct them - #32
Merged
Conversation
The 'options:' block sat directly on the job rather than under 'variables:'.
The schema's 'job' object does not set 'additionalProperties: false', so the
stray key validated silently and the options were ignored - documented as
hole 2 in docs/schema-coverage.md in the umbrella repository.
Because nothing ever validated the block, it had drifted into a state that
could not have worked:
- 'required' listed all four of table, count, min_rows and max_rows, but the
command guards the last three with '{% if ... is defined %}' and no caller
supplies all four. The three tests supply table+count, table+count, and
table+min_rows+max_rows respectively, so every one of them would have
failed.
- 'table' had no 'type', which 'job-option-property' requires.
- 'min_rows' and 'max_rows' were declared required but never defined.
So this is not a move. The block is relocated under 'variables' and
corrected: only 'table' is required, 'table' gains 'type: string', and
'min_rows'/'max_rows' are properly declared.
The Job could not previously be run from the Data Manager at all - with no
options visible, there was no way to supply the table name its command
needs. It has worked only under jote, which passes options directly. Making
the options real is therefore a behaviour change, so the version is bumped
1.0.0 -> 1.0.1.
Verified with jote 0.14.0 against decoder 2.7.0: all six manifests pass
'jote --dry-run', and each of the three tests' supplied options validates
against the corrected schema.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tdudgeon
added a commit
to InformaticsMatters/squonk2-data-manager-job-decoder
that referenced
this pull request
Aug 13, 2026
Completes the work started in #4. Three object definitions still accepted unknown keys, so anything misplaced or misspelled inside them validated silently. They were the last three in the schema without 'additionalProperties: false': - the top-level object - 'job' - 'test-checks-output' Each had a real cost. The top-level hole let 'repository-url' and 'repository-tag' sit in a Job Definition for four years - read by nothing, and both values wrong (InformaticsMatters/virtual-screening#33). The 'job' hole let an 'options' block sit directly on a Job rather than under 'variables', where it was silently ignored, leaving that Job impossible to run from the Data Manager at all (InformaticsMatters/virtual-screening#32). Adds the first two fixtures to example-definitions/bad, which was empty of YAML, so both closures are covered by test_bad_example_definitions. Verified: all 72 tests pass, the four 'good' fixtures still validate, and every one of the 18 Job Definitions across the Job repositories validates against the tightened schema with zero errors - the three offending Jobs having been corrected first. Note for the release: like 2.7.0 this rejects definitions that previously passed. Consumers pinning the decoder control when they take it; those resolving it transitively do not, so it is worth clear release notes. Refs InformaticsMatters/squonk2-jobs#8. Co-authored-by: Claude Opus 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.
moldb-count-rowshad itsoptions:block directly on the job rather than undervariables:. The schema'sjobobject does not setadditionalProperties: false, so the stray key validated silently and theoptions were ignored — this is hole 2 in
docs/schema-coverage.md.This is not just a move
Because nothing ever validated the block, it had drifted into a state that could
not have worked. Moving it unchanged produces a schema error and breaks all
three tests:
Three separate defects:
requiredlisted all four oftable,count,min_rows,max_rows. Butthe command guards the last three with
{% if ... is defined %}, and no callersupplies all four —
count(an exact count) andmin_rows/max_rows(arange) are alternatives. Every test would have failed.
tablehad notype, whichjob-option-propertyrequires.min_rowsandmax_rowswere required but never defined.So the block is relocated and corrected: only
tableis required,tablegains
type: string, andmin_rows/max_rowsare properly declared.Why the version bump
The Job could not previously be run from the Data Manager at all — with no
options visible there was no way to supply the table name its command needs. It
has only ever worked under
jote, which passes options directly. Making theoptions real is a behaviour change, so
1.0.0→1.0.1.Sequencing
This unblocks closing hole 2 in the decoder (adding
additionalProperties: falseto the
jobobject). That closure would turn this silent no-op into a hardfailure, so the Job had to be fixed first — the lesson from decoder 2.7.0, which
closed a different hole and failed two unrelated merges.
Verification
jote 0.14.0 with decoder 2.7.0:
manifest-im-virtual-screening.yamlmanifest-moldb.yamlmanifest-fragnet-search.yamlmanifest-im-mordred.yamlmanifest-dmpk.yamlmanifest-silicos-it.yamlEach of the three tests' supplied options was also validated directly against the
corrected options schema — all three pass.
Refs InformaticsMatters/squonk2-jobs#8.
🤖 Generated with Claude Code