feat(config): add missing_cache_policy to NodeConfig (ITL-604)

[kurodo3]

Summary

Adds missing_cache_policy: Literal["recompute", "as_empty", "strict"] | None to NodeConfig, giving callers unified control over how FunctionJobNode handles a cache miss — where the pipeline table has a record for an input but the result store does not.

• "recompute" (default) — WARNING logged; existing behaviour preserved exactly. EmptyData tokens act as recompute sentinels in both execute() and async_execute().
• "as_empty" — WARNING logged (non-ephemeral) or INFO logged (ephemeral); EmptyData is emitted directly to the output, allowing downstream nodes to serve from their own cache opportunistically.
• "strict" — ERROR logged and CacheMissError raised for non-ephemeral misses. Ephemeral misses always degrade gracefully to EmptyData regardless of policy.

Ephemeral misses now always log at INFO level (previously silent). async_execute().route_inputs is aligned with execute() so EmptyData sentinels in "recompute" mode trigger recomputation rather than being silently forwarded.

Closes ITL-604

Changes

• src/orcapod/errors.py — new CacheMissError exception
• src/orcapod/types.py — NodeConfig.missing_cache_policy field + merge() update + docstring
• src/orcapod/core/nodes/function_node.py:
  • _populate_empty_data_tokens() shared helper (eliminates duplication between ephemeral and permissive persistent miss paths)
  • _fetch_joined_records() — three-way policy branch for both Branch A (result DB entirely absent) and Branch B (partial gap); ephemeral INFO log
  • execute() — EmptyData sentinel check respects policy
  • async_execute().route_inputs — aligned with execute() EmptyData handling
• tests/test_core/function_pod/test_missing_cache_policy.py — 24 new tests covering all policy × store-type × mode combinations
• superpowers/specs/ and superpowers/plans/ — design spec and implementation plan

Test plan

• TestNodeConfigMissingCachePolicy — field, merge semantics, importability
• TestStrictPolicy — raises CacheMissError for non-ephemeral Branch A & B; no-op when all present; ephemeral miss does not raise
• TestAsEmptyPolicy — EmptyData emitted, function not called; WARNING logged; policy honoured across multiple calls
• TestEphemeralInfoLog — INFO log for ephemeral miss; no WARNING
• TestAsyncExecutePolicy — recompute mode recomputes; as_empty mode forwards EmptyData; strict raises via async path
• TestCacheOnlyPolicy — strict raises, as_empty forwards EmptyData, recompute silently omits
• TestAsEmptyEndToEnd — downstream serves from cache; EphemeralResultMissingError raised when downstream has no cache (ITL-605 boundary)
• TestRecomputeRegression — default behaviour unchanged

Full suite: 4655 passed, 93 skipped, 6 xfailed.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR introduces a new NodeConfig.missing_cache_policy setting to unify and control how FunctionJobNode handles “pipeline record exists, but result-store entry is missing” scenarios across persistent and ephemeral stores, including sync and async execution paths.

Changes:

• Adds NodeConfig.missing_cache_policy: Literal["recompute", "as_empty", "strict"] | None (plus merge semantics and docstring guidance).
• Adds CacheMissError and updates FunctionJobNode to enforce the policy in _fetch_joined_records(), execute(), and async_execute() (including INFO logging for ephemeral misses and aligned async routing behavior).
• Adds a comprehensive new test suite for policy × store-type × mode combinations, plus design spec and implementation plan artifacts under superpowers/.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/orcapod/types.py	Adds missing_cache_policy to NodeConfig, documents semantics, and updates merge().
src/orcapod/errors.py	Introduces CacheMissError for strict persistent-miss handling.
src/orcapod/core/nodes/function_node.py	Implements policy branching for persistent misses, emits/handles EmptyData per policy, and aligns async routing with sync behavior.
tests/test_core/function_pod/test_missing_cache_policy.py	Adds end-to-end and matrix tests for the new policy behaviors (sync, async, and cache-only).
superpowers/specs/2026-08-06-itl-604-missing-cache-policy-design.md	Documents intended policy semantics and behavior matrix for ITL-604.
superpowers/plans/2026-08-06-itl-604-missing-cache-policy.md	Provides an implementation plan and verification checklist for ITL-604.

Suppressed comments (2)

tests/test_core/function_pod/test_missing_cache_policy.py:174

• This test duplicates the DB-wipe logic inline. Using the shared _wipe_result_db() helper here keeps the wipe semantics consistent across tests (including _pending_record_ids) and reduces brittleness if InMemoryArrowDatabase internals change.

        ephemeral_db._tables.clear()
        ephemeral_db._pending_batches.clear()

tests/test_core/function_pod/test_missing_cache_policy.py:185

• This test currently only asserts the length and includes a misleading “# recomputed” comment. Under the new execute() logic, an EmptyData cache entry is only treated as a recompute sentinel when missing_cache_policy is "recompute"; for "strict" (ephemeral miss) the expected behavior is to emit EmptyData (no recompute). Adding an explicit assertion here will prevent regressions and clarify intent.

        assert len(results) == 1  # recomputed

---

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

[codecov]

Codecov Report

❌ Patch coverage is 97.22222% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/orcapod/core/nodes/function_node.py	97.10%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[kurodo3]

Review round response

Single commit dae336c5 addresses all three review points:

Comment	Fix
Line 134 — inline result DB partial-gap wipe missing _pending_record_ids.clear()	Added result_db._pending_record_ids.clear() after the manual table slice
Line 173 — inline ephemeral DB wipe missing _pending_record_ids.clear()	Added ephemeral_db._pending_record_ids.clear()
Line 185 — misleading # recomputed comment + weak assertion	Removed misleading comment; added explicit assert isinstance(results[0][1], EmptyData) with an explanatory message. Verified: strict + ephemeral miss emits EmptyData (not recomputed data) — the recompute-sentinel path is guarded by policy == "recompute"

The _wipe_result_db() helper at the top of the file was already correct (it has included _pending_record_ids.clear() since an earlier quality-fix pass); only the two inline wipes were affected.

All 24 tests in test_missing_cache_policy.py continue to pass.

[eywalker]

the method name is very confusion/misleading -- we should call it something like load_cached_results

[kurodo3]

Done — renamed get_cached_results to load_cached_results throughout function_node.py (method definition, call site, and docstring references). The new name makes the intent clear: the method loads DB records into the in-memory cache as a side effect and returns those results. The test file test_function_node_get_cached.py was updated accordingly (class and method names, module docstring).

[eywalker]

consider making buffer size configurable as an argument into the async execute?

[kurodo3]

Done — added a channel_buffer_size: int = 16 keyword argument to async_execute(). The value is forwarded to both the compute_channel and result_channel constructors, replacing the previous hardcoded 16. The default is preserved so existing callers are unaffected. The docstring documents the parameter with guidance on when to tune it (higher for throughput, lower to tighten backpressure).

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/orcapod/types.py:388

• NodeConfig.missing_cache_policy is typed as a Literal[...], but there’s no runtime validation. Because the code branches only on "strict" / "as_empty" and otherwise treats the value as a fallback, an invalid non-empty string can lead to inconsistent behavior (e.g., _fetch_joined_records() falls back to recompute semantics, while execute()/async_execute() would treat EmptyData as an opportunistic cache hit). Add a __post_init__ guard to fail fast on invalid values.

    is_result_ephemeral: bool | None = None
    ignore_schema: tuple[str, ...] | None = None
    missing_cache_policy: Literal["recompute", "as_empty", "strict"] | None = None

src/orcapod/core/nodes/function_node.py:2000

• _fetch_joined_records(base_entry_ids=...) computes miss counts / raises CacheMissError / populates EmptyData tokens using the full persistent_taginfo_df / ephemeral_taginfo_df sets, and only later applies the base_entry_ids filter to merged_df. With missing_cache_policy="strict", this can raise due to missing entries that are outside the requested base_entry_ids, and in "as_empty" it can populate EmptyData tokens for unrelated entries (cache pollution). The miss handling should be scoped to base_entry_ids by filtering taginfo_df (or the persistent/ephemeral taginfo DFs) up front before joins/token creation when base_entry_ids is provided.

        policy = self._node_config.missing_cache_policy or "recompute"
        results_schema = None
        persistent_df = pl.DataFrame()
        empty_data_tokens: dict[bytes, EmptyData] = {}
        empty_taginfo_rows: dict[bytes, dict] = {}
        if persistent_taginfo_df.height > 0:

[kurodo3]

Review round 2 — changes addressed

Two comments from @eywalker resolved in commit 1b4d3c0a:

Comment	Action
get_cached_results name is confusing/misleading	Renamed to load_cached_results throughout function_node.py (method definition, call site at line 1260, all docstring references). Test file test_function_node_get_cached.py updated to match.
Consider making buffer size configurable in async_execute()	Added channel_buffer_size: int = 16 keyword argument; wired through to both compute_channel and result_channel constructors. Default preserved — no impact on existing callers.

All 4 646 tests pass.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

[kurodo3]

Good catch — confirmed the bug. The root cause was that empty_data_tokens / empty_taginfo_rows were populated for all unmatched rows before the base_entry_ids filter ran, and the early-return else branch (no rows in either store) exited before that filter was reached.

Fix (commit 4a01dc3b): Moved the token-dict filter to immediately before the merge block, so every return path — including the early-return no-rows case — scopes the dicts identically to the merged_df filter applied after merging.

Regression test added (TestAsEmptyPolicy.test_as_empty_load_cached_results_scoped_to_requested_ids): calls load_cached_results(base_entry_ids=[eid0]) on a node with two pipeline entries both missing from the result store (as_empty policy), and asserts that only eid0's EmptyData appears in _cached_output_datas — the test failed before the fix and passes after.

[kurodo3]

Review round 3 — changes addressed

One comment from Copilot resolved in commit 4a01dc3b:

Comment	Fix
empty_data_tokens / empty_taginfo_rows not scoped to base_entry_ids, causing load_cached_results(base_entry_ids=[x]) to cache EmptyData for unrelated IDs	Moved the token-dict filter to immediately before the merge block so all return paths — including the early-return no-matched-rows else branch — scope the dicts identically to the merged_df filter. Added regression test test_as_empty_load_cached_results_scoped_to_requested_ids which fails without the fix and passes after.

31/31 policy tests pass.