Reviewing workflows-blocks changes
Scope
Triggers when a PR touches:
inference/core/workflows/core_steps/**— Roboflow Core plugin blocks ({category}/{block}/v{N}.py,common/,common/query_language/**,loader.py).inference/core/workflows/execution_engine/entities/types.py— only when a NEW kind is added for a block (review it alongside the block).
OUT of scope (other skills own these): Execution Engine internals under inference/core/workflows/execution_engine/v1/** (compiler, executor, output_constructor — review-workflows-execution-engine); model backends under inference_models/. Since the tensor-data-representation merge (#2357), the vN_tensor.py siblings ARE in scope here — review them with the Tensor-native parity items below; authoring-side counterpart: create-workflow-block. UQL / common/query_language/** has no dedicated skill, so it IS reviewed here (see the query-language standard below).
Review checklist
Severity: BLOCK = must fix before merge; FLAG = raise it; NIT = optional. Each item maps to one canonical rule in ## Standards below.
- BLOCK — Output contract: enumerate
describe_outputs()keys; confirm EVERYrun()/helper return dict (happy, empty, error, zero-area) carries exactly those keys (#2346). - BLOCK — Versioning discipline: bug-fix patches the existing
vN.py; behavior change / new inputs-outputs goes in a NEWvN.py. Confirm no shippedtypeidentifier or legacy alias was removed or repurposed. - BLOCK — Loader wiring: new block imported into
load_blocks(); new kind intoload_kinds()+ defined intypes.pywith*_KIND_DOCS; serializable data-kind intoKINDS_SERIALIZERS/KINDS_DESERIALIZERS. - BLOCK —
__init__.pyexists in every new/nested block package (guarded bytest_init_files.py). - BLOCK —
sv.Detections.datadensity: any new data key holds dense N-d numpy arrays (nodtype="object"ragged) and has serializer/deserializer parity if it must round-trip. - FLAG — Tests: unit test under
tests/workflows/unit_tests/core_steps/{category}/; integration test undertests/workflows/integration_tests/execution/test_workflow_with_*_block.pyfor new or output-changing blocks. - FLAG — EE compatibility floor: actively verify
get_execution_engine_compatibility()against the EE capabilities the block actually uses — contributors habitually leave the copy-pasted default range unexamined. Derive the true minimum from the EE changelog in the roboflow/docs repo (workflows/developer-guide/execution-engine-changelog.md) and require the floor raised when the block relies on a capability introduced after the declared minimum. If a capability the block relies on exists only under## Unreleased(including one added in the same PR), the EE version must be placed and bumped so the block can gate on it — see the EE-compatibility standard. - FLAG — EE behavior change: if the block forces an EE behavior change, require a user-facing entry under
## Unreleasedin the EE changelog in the roboflow/docs repo (workflows/developer-guide/execution-engine-changelog.md) and surface the maintainer notice Execution Engine requires a version bump for release — never ask the contributor to bumpEXECUTION_ENGINE_V1_VERSIONor update version-assert tests (maintainers do both in the release PR), except in the unreleased-dependency case above. Outside that exception, a contributor PR touching that constant is a red flag. - FLAG — Cross-block kinds: output kinds upstream ⊆ input kinds accepted downstream (esp. detections/segmentation/keypoint interplay).
- FLAG — Model-version changes update defaults/enums (not silent deletion) and keep old identifiers loadable.
- FLAG — Batch handling for model/foundation blocks:
runiteratesBatch[...]correctly and declaresget_parameters_accepting_batches(). 11a. FLAG — Dependent-resource declaration: any block that loads a model or external resource overridesdiscover_dependent_resources()(helpersroboflow_platform_model()/third_party_model()) so resource discovery and model pre-loading see it. A general block contract — not tensor-specific; a missing declaration means video pipelines pay the cold model load on the first frame. - FLAG — UQL correctness over speed: be skeptical of micro-optimizations to
common/query_language/**ops that change behavior (#1112/#1161/#1162 were reverted). - NIT — Field metadata: new inputs have
title/description/examples;default=correct (no straydefault=Noneon required fields);json_schema_extra(relevant_for,always_visible) sensible.
Tensor-native parity (blocks with vN_tensor.py siblings)
- BLOCK — Sibling drift: a PR editing a numpy block's behavior without the mirrored edit in its
_tensorsibling (or vice versa) is a finding — main-merge compensation sweeps found dozens of drifted siblings; drift ships divergent flag-on behavior silently. - BLOCK — Manifest surface identical across flags: field set, supported variants, batch-parameter declarations, AND output-kind list order must match between siblings — serializer selection is first-kind-that-serializes-wins, so kind-order divergence changes the wire payload per deployment flag (instance-seg v1–v3 RLE-vs-polygon lesson).
- BLOCK — Both-sibling companions: general contracts hold on BOTH manifests —
discover_dependent_resources()(the general rule is 11a; the tensor trap is that a numpy-side-only declaration silently disappears flag-on and pre-loading skips the block); sink siblings both honordisable_sinks. - BLOCK — Host-mirror hygiene: any tensor sibling rewriting
xyxy/class_id/confidencestrips the per-box host mirror (strip_host_mirror_metadata/HOST_MIRROR_KEYS) — a carried mirror makes downstream visualizations draw stale geometry with no error. - FLAG — Mutation declarations: every
copy_image=Falsein-place draw callsdeclare_numpy_image_mutated()/declare_tensor_image_mutated()— including NEW mainline blocks arriving via merge without the retrofit (label v2 / rich_label class). - FLAG — CPU-only import safety: every module registered in the flag-on loader branch must import without CUDA (no module-level device initialization); the flag-on suite runs on CPU-only CI runners.
- FLAG — Flag-off registration is frozen: the loader's flag-off block list and import bindings must stay byte-identical to pre-tensor
mainbehavior — flag-off is the compatibility contract; any flag-off divergence riding a tensor PR is a finding.
Not blocking
- Do NOT demand an
inference/core/version.pybump — inference releases are versioned separately from feature/bugfix PRs. - Do NOT demand a new
vN.pyfor a pure in-place bug-fix; surgical patches to a shipped version are correct (#2346, #1339, #1368). - Never demand an
EXECUTION_ENGINE_V1_VERSIONbump from a contributor — maintainers bump it at release. Sole exception: a block gating on unreleased EE capabilities (see the EE-compatibility standard), where the bump must be maintainer-coordinated before the block ships. The## UnreleasedEE changelog entry is required only when the block forces an EE behavior change; most block changes don't. - Do NOT demand serializer/deserializer entries for a data key that is purely intra-block and never crosses the wire.
- Do NOT demand hand-written
docs/workflows/blocks/<block>.md— those are generated fromField(description=, examples=)metadata.
Standards
The one canonical statement of each rule. Past regressions are cited inline; see ## Reference PRs.
- Output contract: every dict
run()emits contains EXACTLY the keys declared bydescribe_outputs(), on every path including empty/zero-area/error branches (return [],{"x": None}) (#2346 —dynamic_cropzero-area branch droppedpredictions; #1341 — empty-zone branch). - Versioning: bug-fixes patch the existing version in place; behavior changes / new inputs-outputs go into a NEW
vN.pymodule — even at the cost of code duplication (docs/workflows/versioning.md,blocks_bundling.md). Never breaktypeidentifiers or legacy aliases; e.g.dynamic_crop/v1.pykeepsLiteral["roboflow_core/dynamic_crop@v1", "DynamicCrop", "Crop"]. Versioning foundation: #565. - New block = full package: new dir with
__init__.py+v1.py, imported and added toload_blocks()inloader.py, plus a unit test AND an integration test (#2362 — overlap_analysis added all of these + a new kind). __init__.pyin every package (including nested): its absence breaks docs generation and block import; guarded bytest_init_files.py(#2351, #830).- New kind discipline: define the kind in
types.pywith a*_KIND_DOCSstring, return it fromload_kinds(), and route serialization throughKINDS_SERIALIZERS/KINDS_DESERIALIZERS(#2362 —DETECTIONS_OVERLAPS_KIND; #804 —INFERENCE_ID_KINDas a distinct semantic kind across blocks). sv.Detections.datamust be dense numpy arrays: object-dtype ragged arrays break supervision indexing /is_data_equal; pad to uniform shape (#2170 — keypoints; #1368 — velocity elements must benp.array).- Serializer/deserializer parity for round-tripping data keys: a data key that must survive serialization needs matching entries in
common/serializers.py+common/deserializers.py(#1368 — velocity/speed; #1156 — timestamps). - New input fields keep backward compat: add with a
default=and updatedescribe_outputs()/run(); behavior-changing additions warrant a new version. Questiondefault=Noneon non-optional inputs (#2384 — added in-place across v1/v2/v3 with defaults; #658 — removed wrongdefault=None). - EE compatibility:
get_execution_engine_compatibility()declares the minimum EE version the block relies on (e.g.">=1.3.0,<2.0.0"); the EE version is a single source of truth inEXECUTION_ENGINE_V1_VERSION. The floor is a contract, not boilerplate: derive it from the EE changelog in the roboflow/docs repo (workflows/developer-guide/execution-engine-changelog.md) — a block needing a capability added in1.3.7declares">=1.3.7,<2.0.0", not the copy-pasted default. A block that forces an EE behavior change makes an EE version bump mandatory at release: the contributor adds the## Unreleasedchangelog entry only; maintainers apply the bump + final changelog heading + version-assert test updates in the release PR (#2527 — revert rolled back both the block and the EE version and re-asserted it in tests). Unreleased-dependency exception: when the block relies on a capability still under## Unreleased, there is no released floor to declare — the EE version must be placed (the## Unreleasedentries get their finalvX.Y.Zheading) andEXECUTION_ENGINE_V1_VERSIONbumped with mirrored version-assert tests, maintainer-coordinated, in or before this PR, so the block's range can reference it. This is the one case where the bump cannot be deferred to a release PR. - Model-version deprecation: when a hosted model version is deprecated, update the block default and enum; do not just delete, keep old identifiers loadable (#1668/#1669 — Claude 3.5→4.5; #500 — GPT-4→GPT-4o; #2395 — Gemini default).
- Batch handling for model/foundation blocks:
runiteratesBatch[...]and declaresget_parameters_accepting_batches()(#718 — VLM blocks failed on batch inference). - Segmentation/mask edge cases: mask→polygon must handle 0/1/degenerate contours (returns
None, duplicates points); empty masks must serialize (#1339; #697 — SAM2 masks as points/lines). - Visualization blocks are fragile across supervision versions: verify against the pinned supervision public API, not internals (#814/#811/#1868 — label/line regressions; #1725/#1951 — sv 0.27.0 handling).
- Query language (UQL):
common/query_language/**ops must preserve exact behavior; reject micro-optimizations that alter results (#1112/#1161/#1162 — automated speed-up PRs reverted). - Rich
Fieldmetadata:title,description,examples, andjson_schema_extra(relevant_for,always_visible) drive the FE app; block docs are generated from these (#623/#704; #2058 — property options propagated to FE). - Tensor-sibling contract (#2357): siblings are standalone verbatim re-implementations against native objects — no delegation/method-binding to the numpy manifest (wholesale subclassing OK). Same-name registration in the loader's flag-on branch; the flag swaps implementations, never the registry (block-name lists identical both directions). Testing convention: per-file
_TENSOR_ONLY/_NUMPY_ONLYskipif markers, mirrored assertions, and the sharedimage_as_workflow_inputintegration fixture parametrizing image inputs as numpy ANDtorch.Tensor— new tensor-native integration tests should take it.
Required companions
Block a functional change that lacks these (canonical rules above; conditions here):
- Loader registration — new block in
load_blocks(); new kind inload_kinds()+types.py; new serializable data-kind inKINDS_SERIALIZERS/KINDS_DESERIALIZERS, all inloader.py(#2362, #804). - Tests — unit test always; integration test for new or output-changing blocks (#2362, #718).
__init__.py— in every new/nested block package (#2351, #830).- EE changelog — ONLY when the block forces an EE behavior change: add an entry under
## Unreleasedin the EE changelog in the roboflow/docs repo (workflows/developer-guide/execution-engine-changelog.md); maintainers bumpEXECUTION_ENGINE_V1_VERSIONat release time.
Key files & entry points
inference/core/workflows/core_steps/loader.py—load_blocks(),load_kinds(),KINDS_SERIALIZERS/KINDS_DESERIALIZERS,REGISTERED_INITIALIZERS.inference/core/workflows/core_steps/{category}/{block}/vN.py— block modules (manifest + block class).inference/core/workflows/core_steps/common/{serializers,deserializers,utils}.py—sv.Detections<-> wire conversion, keypoints/mask helpers.inference/core/workflows/core_steps/common/query_language/**— UQL operations/evaluation engine.inference/core/workflows/execution_engine/entities/types.py—Kinddefinitions +*_KIND_DOCS.inference/core/workflows/execution_engine/entities/base.py—WorkflowImageData,Batch,OutputDefinition.inference/core/workflows/prototypes/block.py—WorkflowBlock,WorkflowBlockManifest,BlockResult.inference/core/workflows/execution_engine/v1/core.py—EXECUTION_ENGINE_V1_VERSION.- Docs: the Workflows developer guide on docs.roboflow.com (create a block, versioning, block bundling, EE changelog): https://docs.roboflow.com/workflows/developer-guide/developer-guide.
- Tests:
tests/workflows/unit_tests/core_steps/**,tests/workflows/integration_tests/execution/**,test_init_files.py.
Reference PRs
- #2362 — new fusion block + new kind + both test tiers (canonical new-block shape).
- #2346 — missing declared output key on empty branch (output-contract).
- #2170 — object-dtype ragged keypoint arrays break supervision.
- #1368 —
sv.Detections.datamust hold numpy arrays + serializer parity. - #2384 — backward-compat input field added in-place to v1/v2/v3 with defaults.
- #804 — new
INFERENCE_ID_KINDacross blocks (kinds discipline). - #565 — Workflows block versioning foundation.
- #2351 — missing
__init__.py(packaging/docs). - #2527 — revert: block removal + EE version rollback + test re-assert.
- #658 — stray
default=Nonein manifests. - #718 — VLM blocks failing on batch inference.
- #1339 — empty-segmentation serialization edge cases.
Related topic skills
When the PR also exhibits these cross-cutting concerns, load the matching topic skill too (see each skill's description for the trigger):
review-topic-workflow-state-managementreview-topic-local-vs-remote-executionreview-topic-prediction-integrityreview-topic-backward-compat-and-versioningreview-topic-test-hygienecreate-workflow-block