Skip to content

feat(types): split NodeConfig into PodConfig and NodeConfig (ITL-512)#221

Merged
eywalker merged 14 commits into
mainfrom
eywalker/itl-512-move-node_config-from-functionpod-to-functionjobnode
Jul 9, 2026
Merged

feat(types): split NodeConfig into PodConfig and NodeConfig (ITL-512)#221
eywalker merged 14 commits into
mainfrom
eywalker/itl-512-move-node_config-from-functionpod-to-functionjobnode

Conversation

@kurodo3

@kurodo3 kurodo3 Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduces PodConfig(max_concurrency: int | None) as a new pod-level executor config that lives on FunctionPod
  • Redefines NodeConfig as a pure orchestrator concern (is_result_ephemeral: bool | None) with a merge() method, owned by FunctionJobNode
  • Adds PipelineJob.apply_node_config(node_config, replace_existing=False) to apply pipeline-wide node config post-construction
  • Adds node_config read/write property to FunctionNodeProtocol for uniform orchestrator access
  • Migrates all construction sites from the old NodeConfig(max_concurrency=N) pattern to PodConfig(max_concurrency=N)

Breaking Changes

Old New
NodeConfig(max_concurrency=N) PodConfig(max_concurrency=N)
FunctionPod(pf, node_config=NodeConfig(...)) FunctionPod(pf, pod_config=PodConfig(...))
pod.node_config pod.pod_config
resolve_concurrency(node_config, pipeline_config) resolve_concurrency(pod_config, pipeline_config)
NodeConfig.is_result_ephemeral: bool NodeConfig.is_result_ephemeral: bool | None

Setting is_result_ephemeral at FunctionPod construction is no longer possible. Use node.node_config = NodeConfig(is_result_ephemeral=True) or job.apply_node_config(NodeConfig(is_result_ephemeral=True)) instead.

Test Plan

  • uv run pytest tests/test_core/test_node_config.py -v — PodConfig, NodeConfig.merge(), resolve_concurrency
  • uv run pytest tests/test_core/function_pod/ -v — FunctionPod pod_config, FunctionJobNode node_config
  • uv run pytest tests/test_pipeline/test_apply_node_config.py -v — PipelineJob.apply_node_config()
  • uv run pytest tests/ -q — full suite (4274 passed, 0 failed)

Related

Closes ITL-512
ITL-516 (FunctionPod/FunctionJobNode async_execute duplication — out of scope, tracked separately)

🤖 Generated with Claude Code

kurodo3 Bot and others added 13 commits July 9, 2026 03:54
Update FunctionPod to use PodConfig instead of NodeConfig for per-pod
executor configuration. This completes Task 2 of the ITL-512 implementation
plan: updating FunctionPod's __init__, property accessors, serialization
methods (to_config/from_config), and async_execute call sites. All affected
test files updated to use PodConfig with the new parameter name.

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

Removes the stale NodeConfig import from function_pod.py that is no longer
used after the Task 2 refactor. Also updates the test docstring to reference
PodConfig instead of NodeConfig for consistency.
…x example

Add three new tests to TestPodConfigSerialization:
- test_to_config_with_non_default_pod_config: Verify FunctionPod with PodConfig(max_concurrency=4) serializes to pod_config dict
- test_to_config_with_default_pod_config: Verify FunctionPod with default PodConfig() serializes to pod_config=None
- test_pod_config_round_trip: Verify FunctionPod.from_config() round-trips the pod_config field

Update examples/async_vs_sync_pipeline.py to use pod_config=PodConfig(max_concurrency=...) instead of the deprecated node_config=NodeConfig(...).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Give FunctionJobNode its own _node_config field with a node_config
getter/setter property. Update _process_data_internal() and
_async_process_data_internal() to read is_result_ephemeral from
self._node_config instead of self._function_pod.node_config. Fix
async_execute() to read concurrency from self._function_pod.pod_config
(via PodConfig). Update tests to set node_config on the node
post-construction and use PodConfig for max_concurrency.

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

Adds two new methods to PipelineJob:
- _iter_function_job_nodes(): iterates over all FunctionJobNode instances
  in the compiled pipeline's persistent node map
- apply_node_config(): applies a NodeConfig to all FunctionJobNode instances,
  either replacing wholesale (replace_existing=False) or merging via
  NodeConfig.merge() (replace_existing=True)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Declares the node_config read/write property on FunctionNodeProtocol so
orchestrators and tests can interact with nodes uniformly. Adds NodeConfig
to the TYPE_CHECKING import block and updates the conformance test to
include node_config in the minimal GoodFunction implementation.
…dConfig

Replace stale `FunctionPod(pf, node_config=NodeConfig(max_concurrency=N))`
calls with `FunctionPod(pf, pod_config=PodConfig(max_concurrency=N))` and
update `resolve_concurrency` call sites to pass `PodConfig` instead of the
old `NodeConfig`. Also update `test_node_config_defaults` in test_channels.py
to test the new split API (`PodConfig.max_concurrency` and
`NodeConfig.is_result_ephemeral`).

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

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR splits the previous NodeConfig responsibilities into two distinct configuration types: PodConfig (pod/executor concern: max_concurrency) and a redefined NodeConfig (orchestrator concern: is_result_ephemeral: bool | None). It updates core execution/serialization paths accordingly and introduces a pipeline-wide API for applying node-level config after pipeline construction.

Changes:

  • Introduces PodConfig and redefines NodeConfig with merge(); updates resolve_concurrency() to accept PodConfig.
  • Moves concurrency configuration to FunctionPod.pod_config, and moves ephemeral-result behavior to FunctionJobNode.node_config.
  • Adds PipelineJob.apply_node_config() + protocol support; migrates tests/examples and adds new coverage.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/orcapod/types.py Adds PodConfig, redefines NodeConfig, updates resolve_concurrency() signature/behavior.
src/orcapod/core/function_pod.py Replaces node_config with pod_config on FunctionPod and updates config round-trip + async semaphore wiring.
src/orcapod/core/nodes/function_node.py Adds FunctionJobNode.node_config and switches ephemeral/concurrency behavior to new config split.
src/orcapod/pipeline/job.py Adds _iter_function_job_nodes() and apply_node_config() for pipeline-wide node config application.
src/orcapod/protocols/node_protocols.py Extends FunctionNodeProtocol with a read/write node_config property.
tests/test_core/test_node_config.py Updates unit tests to cover PodConfig, new NodeConfig semantics, and resolve_concurrency().
tests/test_core/function_pod/test_node_config_on_job_node.py New tests ensuring FunctionJobNode.node_config exists and is settable/defaulted correctly.
tests/test_core/function_pod/test_function_pod_config.py Adds serialization tests for FunctionPod.pod_config.
tests/test_core/function_pod/test_ephemeral_result.py Migrates ephemeral behavior tests to set NodeConfig on FunctionJobNode rather than on FunctionPod.
tests/test_pipeline/test_apply_node_config.py New tests for PipelineJob.apply_node_config() behavior and merge/replace semantics.
tests/test_pipeline/test_node_protocols.py Updates protocol tests to require node_config property on function nodes.
tests/test_pipeline/test_orchestrator_executor_matrix.py Migrates concurrency config usage to PodConfig.
tests/test_pipeline/test_serialization_helpers.py Updates serialized config key from node_config to pod_config.
tests/test_core/test_regression_fixes.py Migrates concurrency config usage to PodConfig.
tests/test_channels/test_async_execute.py Migrates async execution tests from NodeConfig(max_concurrency=…) to PodConfig(max_concurrency=…).
tests/test_channels/test_node_async_execute.py Migrates concurrency config usage to PodConfig.
tests/test_channels/test_pipeline_example.py Migrates concurrency config usage to PodConfig.
tests/test_channels/test_copilot_review_issues.py Migrates concurrency config usage to PodConfig and updates resolve_concurrency call sites accordingly.
tests/test_channels/test_channels.py Updates channel-level config tests to reflect new NodeConfig vs PodConfig split.
examples/async_vs_sync_pipeline.py Updates example pipeline to pass PodConfig to FunctionPod.
superpowers/specs/2026-07-08-node-config-pod-config-split-design.md Adds design spec documenting the split and new APIs.
superpowers/plans/2026-07-09-node-config-pod-config-split.md Adds implementation plan/checklist for the split.

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

Comment on lines +409 to +429
def apply_node_config(
self,
node_config: NodeConfig,
replace_existing: bool = False,
) -> None:
"""Apply a ``NodeConfig`` to all ``FunctionJobNode``s in this pipeline.

Args:
node_config: The config to apply.
replace_existing: If ``False``, directly sets ``node_config`` on
every node, replacing whatever config each node currently holds.
If ``True``, merges ``node_config`` into each node's existing
config — non-``None`` fields in ``node_config`` override the
node's current values, while ``None`` fields leave existing
values unchanged.
"""
for node in self._iter_function_job_nodes():
if replace_existing:
node.node_config = node.node_config.merge(node_config)
else:
node.node_config = node_config

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 3aa9d78. Added a compilation guard at the top of apply_node_config() that raises RuntimeError if self._compiled is False, matching the identical pattern used by as_pipeline(), node_states(), is_runnable(), and save(). The error message is: "No compiled pipeline — use 'with job:' to record a DAG before calling apply_node_config()."

The Raises section in the docstring was also updated to document this.

A new test test_apply_before_compilation_raises was added to tests/test_pipeline/test_apply_node_config.py to cover the pre-compilation error path.

Comment thread src/orcapod/core/nodes/function_node.py Outdated
Comment on lines +2089 to +2092
# Resolve concurrency limit from node config (pipeline config is not
# threaded through the orchestrator, so we fall back to defaults).
node_config = getattr(self._function_pod, "node_config", NodeConfig())
max_concurrency = resolve_concurrency(node_config, PipelineConfig())
pod_config = getattr(self._function_pod, "pod_config", PodConfig())
max_concurrency = resolve_concurrency(pod_config, PipelineConfig())

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in commit 3aa9d78. Changed the inline comment from "Resolve concurrency limit from node config" to "Resolve concurrency limit from pod config" to accurately reflect that the concurrency limit is now sourced from FunctionPod.pod_config (a PodConfig instance), not the old NodeConfig.

@kurodo3

kurodo3 Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1 — changes in commit 3aa9d78

Two comments addressed, both accepted:

1. Stale inline comment in function_node.py (line 2089)
Updated "Resolve concurrency limit from node config" → "Resolve concurrency limit from pod config" to match the PodConfig-based implementation below it.

2. apply_node_config() silent no-op before compilation (job.py)
Added a RuntimeError guard at the entry of apply_node_config() that fails fast when self._compiled is False — consistent with the identical guard pattern used throughout PipelineJob (as_pipeline(), node_states(), is_runnable(), save()). Updated the docstring with a Raises: section. Added test_apply_before_compilation_raises to cover this path.

@eywalker eywalker merged commit 0f9b968 into main Jul 9, 2026
11 checks passed
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.

2 participants