Skip to content

Fail the JSON Builder when no Sequence Element has produced a sequence number - #76

Draft
jwrosewell wants to merge 3 commits into
rcfrom
fix/json-builder-needs-sequence
Draft

jwrosewell wants to merge 3 commits into
rcfrom
fix/json-builder-needs-sequence

Conversation

@jwrosewell

Copy link
Copy Markdown
Contributor

Problem

For specifications#80. The JSON Builder "MUST fail with an error and MUST NOT set its JSON" when no Sequence Element has produced a sequence number, and a query.sequence value from a query string does not count. JSONBundlerElement.process_internal treated a missing number as below the limit and built the JSON with javascriptProperties, and compared text to 10, which raises TypeError.

What changed

  • jsonbundler.py reads query.sequence once at the start. If it is not an integer (a text value or a boolean does not count) it raises, naming the Sequence Element, before any JSON is built. With suppressed errors the error is recorded and no jsonbundler data is set.
  • test_javascriptbuilder_sequence.py built pipelines with the JSON Builder and no Sequence Element, which can no longer produce a script. Those variants are removed, and the rest build on the default pipeline. The builder's reading of odd text values stays covered by the get_sequence tests, and the Sequence Element's own handling by the with-element variants.

Overlap

The web integration draft #70 edits test_javascriptbuilder_sequence.py and jsonbundler.py too, in other lines. Whichever merges second needs a small rebase.

How it was verified

New tests in pipeline/fiftyone_pipeline_core/tests/test_jsonbundler_sequence_required.py.

The last release into main merged on 2026-11-04.

The JSON Builder fails without a sequence number, so a pipeline without a
Sequence Element no longer reaches the script. The builder's reading of text
values stays covered by the get_sequence tests.
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