Skip to content

fix: move moldb-count-rows options under variables, and correct them - #32

Merged
tdudgeon merged 1 commit into
mainfrom
fix/moldb-count-rows-options
Aug 13, 2026
Merged

fix: move moldb-count-rows options under variables, and correct them#32
tdudgeon merged 1 commit into
mainfrom
fix/moldb-count-rows-options

Conversation

@tdudgeon

Copy link
Copy Markdown
Collaborator

moldb-count-rows had its options: block 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 — 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:

jobs.moldb-count-rows.variables.options.properties.table: 'type' is a required property

test supply       would be missing required ['min_rows', 'max_rows']
test enumeration  would be missing required ['min_rows', 'max_rows']
test conformer    would be missing required ['count']

Three separate defects:

  • required listed all four of table, count, min_rows, max_rows. But
    the command guards the last three with {% if ... is defined %}, and no caller
    supplies all four — count (an exact count) and min_rows/max_rows (a
    range) are alternatives. Every test would have failed.
  • table had no type, which job-option-property requires.
  • min_rows and max_rows were required but never defined.

So the block is relocated and corrected: only table is required, table
gains type: string, and min_rows/max_rows are 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 the
options real is a behaviour change, so 1.0.01.0.1.

Sequencing

This unblocks closing hole 2 in the decoder (adding additionalProperties: false
to the job object). That closure would turn this silent no-op into a hard
failure, 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 Result
manifest-im-virtual-screening.yaml 32/32
manifest-moldb.yaml 12/12
manifest-fragnet-search.yaml 3/3
manifest-im-mordred.yaml 2/2
manifest-dmpk.yaml 1/1
manifest-silicos-it.yaml 0/0

Each 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

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
tdudgeon merged commit eb63ade into main Aug 13, 2026
12 checks passed
@tdudgeon
tdudgeon deleted the fix/moldb-count-rows-options branch August 13, 2026 12:27
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>
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