fix: preserve field nullability in empty Arrow tables (ITL-563) by kurodo3[bot] · Pull Request #242 · nauticalab/orcapod-python

kurodo3 · 2026-07-22T05:41:16Z

Summary

Add arrow_utils.make_empty_table(python_schema, type_converter) — a single utility that builds zero-row PyArrow tables using python_schema_to_arrow_schema, preserving nullable=False for required fields and nullable=True for T | None fields
Replace five buggy empty-table construction sites that used pa.table(dict_of_arrays) (which silently marks every field nullable=True): sync_orchestrator._materialize_as_stream, operator_node._make_empty_table, SideEffectPodStream.as_table, SideEffectNode.as_table, DerivedSource._get_stream
Fix latent null-typed array bug in Join.static_process revealed by the integration test (Polars rejects null-typed _common column when joining zero-row tables)

Root cause

When a function fails under error_policy="continue", its empty output buffer was materialized via pa.table(dict_of_arrays). PyArrow marks every field in such tables nullable=True regardless of the declared schema. A downstream Join then saw str on one input and str | None on the other, raised InputValidationError, and aborted the entire pipeline — bypassing the continue policy.

Test plan

tests/test_utils/test_arrow_utils.py::TestMakeEmptyTable — 4 unit tests: required fields → nullable=False, optional → nullable=True, mixed, round-trip through ArrowTableStream.output_schema()
tests/test_pipeline/test_error_policy_continue.py — integration test reproducing the exact issue topology (source → failing_function → Join ← source under error_policy="continue"); second test directly asserts _materialize_as_stream schema
tests/test_core/nodes/test_function_node_iteration.py — strengthened test_as_table_empty_schema_matches_non_empty_schema with nullable assertion
tests/test_core/side_effect_function/test_side_effect_function_pod.py — regression tests for SideEffectPodStream and SideEffectNode empty table nullability
tests/test_core/sources/test_derived_source.py — regression test for DerivedSource empty cache nullability
Full suite: 4600 passed, 93 skipped, 6 xfailed, 0 failures

Closes ITL-563

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…(ITL-563) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…(ITL-563) Adds make_empty_table() which uses python_schema_to_arrow_schema() to build zero-row Arrow tables with correct nullable flags (nullable=False for plain types, nullable=True for T | None), fixing the nullability loss that caused downstream Join operators to raise InputValidationError on empty outputs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…n make_empty_table (ITL-563)

…y (ITL-563) Replace the old _make_empty_table() body (python_type_to_arrow_type loop + pa.table(dict), which lost nullable=False for plain types) with a call to arrow_utils.make_empty_table(), which uses python_schema_to_arrow_schema and pa.Table.from_batches to correctly set nullable per field. Also strengthen test_as_table_empty_schema_matches_non_empty_schema to assert that field nullability matches between the empty and populated tables.

…_empty_table (ITL-563)

…ility (ITL-563) Replace the empty-branch of _materialize_as_stream() to call arrow_utils.make_empty_table() instead of building a pa.table() from a dict of pa.array() calls, which was marking all fields nullable=True. Also fix Join.static_process() to use an explicit int8 type when adding the _common cartesian-product column so that zero-row tables don't produce a null-typed column that Polars rejects on join.

…unused pytest import (ITL-563)

… (ITL-563) Replace the hand-rolled empty-branch logic in SideEffectPodStream.as_table() and SideEffectNode.as_table() with arrow_utils.make_empty_table(), which delegates to python_schema_to_arrow_schema and preserves nullable=False for plain types. Also move arrow_utils import to module level per project convention. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…TL-563)

…ty (ITL-563) Replace hand-rolled pa.field() construction in the empty-cache branch of DerivedSource._get_stream() with arrow_utils.make_empty_table(), which delegates to python_schema_to_arrow_schema and correctly sets nullable=False for required (non-Optional) fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…(ITL-563) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

This PR fixes a correctness issue where zero-row PyArrow tables were being materialized with all fields marked nullable=True, causing downstream schema mismatches (notably in Join) and aborting pipelines even under error_policy="continue". It centralizes empty-table creation into a single utility that round-trips through the existing Python↔Arrow schema conversion logic to preserve per-field nullability.

Changes:

Added arrow_utils.make_empty_table(python_schema, type_converter) to build schema-correct zero-row tables (preserving required vs optional nullability).
Replaced multiple empty-table construction sites to use make_empty_table, and fixed Join.static_process to avoid Polars rejecting null-typed join-key columns on zero-row joins.
Added unit + integration + regression tests to lock in nullability preservation across orchestrator/materialization, operators, side effects, and derived sources; updated the design-issues log entry.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/orcapod/utils/arrow_utils.py	Adds `make_empty_table` utility that preserves Arrow field nullability from Python schema.
src/orcapod/pipeline/sync_orchestrator.py	Uses `make_empty_table` in `_materialize_as_stream` empty-buffer path.
src/orcapod/core/nodes/operator_node.py	Uses `make_empty_table` in `_make_empty_table` to preserve nullability.
src/orcapod/side_effects.py	Uses `make_empty_table` for empty `as_table()` in side-effect stream/node paths.
src/orcapod/core/sources/derived_source.py	Uses `make_empty_table` when derived-source cache is empty.
src/orcapod/core/operators/join.py	Forces `_common` join key to `int8` to avoid null-typed columns on zero-row joins.
tests/test_utils/test_arrow_utils.py	Adds unit tests for `make_empty_table` required/optional/mixed/round-trip behavior.
tests/test_pipeline/test_error_policy_continue.py	Adds integration regression for ITL-563 topology (`continue` + failing function feeding `Join`).
tests/test_core/nodes/test_function_node_iteration.py	Strengthens existing test to assert nullability matches between empty and non-empty tables.
tests/test_core/side_effect_function/test_side_effect_function_pod.py	Adds regression tests for side-effect empty-table nullability.
tests/test_core/sources/test_derived_source.py	Adds regression test for derived-source empty-cache nullability.
DESIGN_ISSUES.md	Marks CC1 as resolved and documents the fix + coverage.
superpowers/specs/2026-07-22-empty-table-nullability-design.md	Adds design spec for the fix and its scope/testing plan.
superpowers/plans/2026-07-22-empty-table-nullability.md	Adds implementation plan for the fix and test sequencing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kurodo3 · 2026-07-22T08:52:35Z

+        def my_fn(value: int, ctx: InvocationContext) -> None:
+            return str(value)


Fixed in commit 6b16ee7. Replaced return str(value) with pass — side-effect functions have no meaningful return value, so the body now correctly matches the -> None annotation.

kurodo3 · 2026-07-22T08:52:42Z

+        def my_fn(value: int, ctx: InvocationContext) -> None:
+            return str(value)


Fixed in commit 6b16ee7. Same change as the companion test — replaced return str(value) with pass so the function body matches the -> None annotation.

…_ISSUES class names (ITL-563) - Remove `return str(value)` from two `-> None` side-effect test functions in TestSideEffectEmptyTableNullability; side-effect functions must return None - Update CC1 entry in DESIGN_ISSUES.md to use the actual class names (SideEffectPodStream/SideEffectNode) instead of the old plan names Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

codecov · 2026-07-22T08:01:20Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

kurodo3 · 2026-07-22T09:24:21Z

Review round summary (commit `6b16ee7`)

Three issues addressed:

my_fn return type in test_empty_stream_schema_preserves_required_fields — replaced return str(value) with pass to match the -> None annotation.
my_fn return type in test_empty_node_schema_preserves_required_fields — same fix applied.
Stale class names in DESIGN_ISSUES.md CC1 Sites list — replaced SideEffectFunctionStream.as_table / SideEffectJobFunctionStream.as_table with SideEffectPodStream.as_table / SideEffectNode.as_table to reflect the actual source names in side_effects.py.

…ag-nullability-and-abort

kurodo3 Bot and others added 14 commits July 22, 2026 03:35

docs(specs): add design spec for empty table nullability fix (ITL-563)

dbeb7d6

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(specs): move empty-table nullability spec to superpowers/specs/ …

74de25a

…(ITL-563) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(plans): add implementation plan for empty table nullability fix …

b65069c

…(ITL-563) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(arrow_utils): type type_converter as TypeConverterProtocol i…

da38f9b

…n make_empty_table (ITL-563)

refactor(operator_node): use module-level arrow_utils import in _make…

d698893

…_empty_table (ITL-563)

refactor(sync_orchestrator): module-level arrow_utils import; remove …

dbf57e5

…unused pytest import (ITL-563)

test(side_effects): add SideEffectNode nullability regression test (I…

d64c6cd

…TL-563)

docs(design-issues): mark CC1 resolved — empty table nullability fix …

a453c78

…(ITL-563) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(design-issues): consolidate duplicate Fix note in CC1 (ITL-563)

2907b1b

eywalker approved these changes Jul 22, 2026

View reviewed changes

eywalker requested a review from Copilot July 22, 2026 05:46

Copilot started reviewing on behalf of eywalker July 22, 2026 05:47 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Merge branch 'main' into eywalker/itl-563-empty-failed-outputs-lose-t…

f4f1546

…ag-nullability-and-abort

eywalker merged commit e968b1b into main Jul 22, 2026
10 checks passed

eywalker deleted the eywalker/itl-563-empty-failed-outputs-lose-tag-nullability-and-abort branch July 22, 2026 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: preserve field nullability in empty Arrow tables (ITL-563)#242

fix: preserve field nullability in empty Arrow tables (ITL-563)#242
eywalker merged 16 commits into
mainfrom
eywalker/itl-563-empty-failed-outputs-lose-tag-nullability-and-abort

kurodo3 Bot commented Jul 22, 2026 •

edited by eywalker

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

kurodo3 Bot Jul 22, 2026

Uh oh!

kurodo3 Bot Jul 22, 2026

Uh oh!

Uh oh!

codecov Bot commented Jul 22, 2026

Uh oh!

kurodo3 Bot commented Jul 22, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		def my_fn(value: int, ctx: InvocationContext) -> None:
		return str(value)

Uh oh!

Conversation

kurodo3 Bot commented Jul 22, 2026 • edited by eywalker Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

kurodo3 Bot Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

kurodo3 Bot Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented Jul 22, 2026

Codecov Report

Uh oh!

kurodo3 Bot commented Jul 22, 2026

Review round summary (commit 6b16ee7)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kurodo3 Bot commented Jul 22, 2026 •

edited by eywalker

Loading

Review round summary (commit `6b16ee7`)