From b80b76a7846ae416cbebd9c8ad57eb4fb357284a Mon Sep 17 00:00:00 2001 From: Pascal Tomecek Date: Sun, 23 Aug 2026 13:20:26 -0400 Subject: [PATCH] Use effective identity for DryRunEvaluator node_key The dry-run report node_key was derived from the raw structural cache_key(...) (effective=False), so it ignored the opt-in _CallableModel._evaluation_identity_payload() effective-identity hook. The in-process MemoryCacheEvaluator and the dependency-graph dedup already key on cache_key(..., effective=True), so a model that opts into a collision-safe effective identity was reported under the structural key in dry-run plans, inconsistently with how it is actually cached and deduplicated. Pass effective=True when building the dry-run node_key. For models that opt out (the base hook returns None) this is a byte-for-byte no-op and preserves the existing structural key; opt-in models now get their effective identity in the plan, consistent with memory/graph dedup. Add a regression test using a generated @Flow.model that ignores an unused ambient context field: the emitted node_key must equal cache_key(..., effective=True) and must merge two contexts that differ only in an ignored field, even though their structural keys differ. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Pascal Tomecek --- ccflow/evaluators/dry_run.py | 5 +++- ccflow/tests/evaluators/test_reporting.py | 31 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/ccflow/evaluators/dry_run.py b/ccflow/evaluators/dry_run.py index 04699d34..2ea1186c 100644 --- a/ccflow/evaluators/dry_run.py +++ b/ccflow/evaluators/dry_run.py @@ -65,7 +65,10 @@ def __call__(self, context: ModelEvaluationContext) -> ResultType: evaluation_context = graph.ids[key] flattened, fn, _ = _flatten_cache_key_context(evaluation_context) model = flattened.model - logical_key = cache_key(ModelEvaluationContext(model=model, context=flattened.context, fn=fn, options=flattened.options)) + logical_key = cache_key( + ModelEvaluationContext(model=model, context=flattened.context, fn=fn, options=flattened.options), + effective=True, + ) report_context = ReportContext( model_name=model.meta.name or model.__class__.__name__, model_type=_model_type(model), diff --git a/ccflow/tests/evaluators/test_reporting.py b/ccflow/tests/evaluators/test_reporting.py index d85354a3..50b74907 100644 --- a/ccflow/tests/evaluators/test_reporting.py +++ b/ccflow/tests/evaluators/test_reporting.py @@ -257,6 +257,37 @@ def test_node_key_distinguishes_non_evaluator_options(self): # And the emitted key equals the real cache_key() of the logical node. self.assertEqual(other_key, cache_key(other_context).decode("utf-8")) + def test_node_key_uses_effective_identity(self): + # node_key must honor the opt-in effective-identity hook (as memory/graph dedup already do), + # not the raw structural cache_key(). For a generated @Flow.model that ignores unused ambient + # context fields, the emitted key must equal cache_key(..., effective=True) and must merge two + # contexts that differ only in an ignored field -- even though their structural keys differ. + from ccflow import Flow, FlowContext, FromContext + from ccflow.evaluators.common import cache_key + + @Flow.model + def add(a: int, b: FromContext[int]) -> int: + return a + b + + model = add(a=1) + clean_context = FlowContext(b=2) + noisy_context = FlowContext(b=2, unused="ignored") + clean_eval = ModelEvaluationContext(model=model, context=clean_context) + noisy_eval = ModelEvaluationContext(model=model, context=noisy_context) + + clean_reporter = InMemoryReporter() + DryRunEvaluator(reporting={"reporter": clean_reporter})(clean_eval) + clean_key = next(e.extra["node_key"] for e in clean_reporter.events if e.phase == ReportPhase.QUEUED) + + noisy_reporter = InMemoryReporter() + DryRunEvaluator(reporting={"reporter": noisy_reporter})(noisy_eval) + noisy_key = next(e.extra["node_key"] for e in noisy_reporter.events if e.phase == ReportPhase.QUEUED) + + # The unused ambient field splits the structural key but not the effective one. + self.assertNotEqual(cache_key(clean_eval), cache_key(noisy_eval)) + self.assertEqual(clean_key, noisy_key) + self.assertEqual(clean_key, cache_key(clean_eval, effective=True).decode("utf-8")) + def test_concurrent_dry_runs_share_instance_without_running_bodies(self): # The planning guard is context-local, so a single shared instance used by two concurrent # evaluations must never let one run's planning state leak into the other (which would make