diff --git a/docs/decisions/issue-1103-bounded-dependency-cycle-detection-preflight.md b/docs/decisions/issue-1103-bounded-dependency-cycle-detection-preflight.md new file mode 100644 index 00000000..a8c01d92 --- /dev/null +++ b/docs/decisions/issue-1103-bounded-dependency-cycle-detection-preflight.md @@ -0,0 +1,98 @@ +# Issue 1103 Bounded Dependency-Cycle Detection Preflight + +Date: 2026-08-11 + +Issue: #1103. Requirement: RUN-303. + +## Gap and Assurance Boundary + +`dependency_cycles()` already implements a linear Tarjan traversal, but drives +its depth-first search with Python recursion. Interpreter stack depth therefore +tracks the longest dependency path. A sufficiently deep valid graph raises +`RecursionError` before the planner can return an order or a typed cycle +diagnostic. + +The defect is unbounded call-stack consumption, not repeated whole-graph work. +The blocked assurance claim is that RUN-303 planning accepts dependency depth +bounded by available input memory rather than by an unrelated interpreter +recursion setting. + +## Existing Surface and Lineage + +- `raes_processor.semantics.planner.dependency_graph()` owns normalization and + excludes unknown dependency references. +- `dependency_cycles()` owns strongly connected component detection and stable + cycle ordering. +- `resource_dependency_cycles()` adapts compiled resources to those shared + semantics; `planner.ordering._ordering_cycle_diagnostics()` owns the existing + domain codes, addresses, and messages. +- `topological_dependency_order()` is iterative, but its residual-node result + cannot replace the established component grouping and self-cycle behavior. +- The authoring validator's feature-cycle check is an earlier, distinct + boundary. Moving compiled-resource cycle handling there would leave runtime + dependency families uncovered. +- RUN-303, `specs/formal/planner/dependency-ordering.md`, and the published plan + schemas define the existing observable contract. None requires amendment. + +This correction extends the canonical planner family. It does not add another +dependency graph, validator, cycle model, or diagnostic surface. + +## Literature and Practice + +Tarjan defines strongly connected component discovery as a linear graph +algorithm: Robert Tarjan, "Depth-First Search and Linear Graph Algorithms," +*SIAM Journal on Computing* 1(2), 1972, +. Python documents the recursion limit as a +guard against C-stack overflow: +. + +An explicit DFS frame stack preserves Tarjan's index, low-link, and component +stack invariants while moving authored-depth consumption out of the interpreter +call stack. It retains O(V + E) graph work and O(V) auxiliary storage. + +## Alternatives + +1. **Do nothing or record evidence only.** Rejected because valid deep acyclic + input can still abort planning without a typed result. +2. **Raise Python's recursion limit.** Rejected because the setting is + process-global and interpreter-dependent and trades a model limit for C-stack + risk. +3. **Use only the iterative topological sort.** Rejected because residual nodes + do not preserve strongly connected components, self-cycle handling, or + deterministic diagnostic grouping. +4. **Drive the existing Tarjan traversal with explicit frames.** Chosen because + it removes authored-depth recursion without changing graph ownership or + observable results. + +## Chosen Boundary and Compatibility + +Each frame holds the current node and its remaining dependency iterator. The +existing normalized graph, visit indices, low links, component stack, canonical +resource ordering, wrappers, and diagnostic rendering remain authoritative. +Unknown references remain excluded and self-cycles remain reportable. + +No SDL model, parser alias, schema, serialized plan, public signature, runtime +lifecycle rule, version, or changelog changes. The unused local `_ordering_graph` +wrapper is removed because the shared semantic adapter already owns that work. + +## Verification + +- A deliberately simple recursive Tarjan oracle is independent of the explicit + frame machinery. +- Every simple directed graph with one, two, or three nodes is compared with + that oracle; a denser Hypothesis strategy extends the differential check to + 18 nodes, six edges per node, and 400 examples. +- Regressions cover unknown references, mapping and edge insertion order, + self-cycles, multi-node components, a 5,000-edge acyclic chain, and a + 3,000-node cycle. +- Tests assert exact cycles and complete ordering, not machine-dependent timing. +- Planner regressions, branch-instrumented changed-code coverage, Ruff, + repository policy, requirement governance, and the canonical verification + graph remain required. + +## Non-Goals + +- Redefining dependency or refresh semantics. +- Adding an authored graph-size limit or a general graph-algorithm framework. +- Changing cycle diagnostics or topological residual-node behavior. +- Optimizing unrelated scheduler, timeout, control-plane, or backend code. diff --git a/docs/requirements/RUN-303/requirement.md b/docs/requirements/RUN-303/requirement.md index 3a7fb51e..9b057f58 100644 --- a/docs/requirements/RUN-303/requirement.md +++ b/docs/requirements/RUN-303/requirement.md @@ -30,3 +30,7 @@ Current state: implemented. Planning needs normative semantics so lifecycle deci - TESTS → TEST `implementations/python/tests/test_semantics_planner.py` (Planner Semantic Tests) - TESTS → TEST `implementations/python/tests/test_runtime_planner.py` (Runtime Planner Tests) - TESTS → TEST `implementations/python/tests/test_fm2_semantics.py` (FM2 Semantic Tests) +- DOCUMENTS → GITHUB_ISSUE `1103` (Bounded dependency-cycle detection) +- DOCUMENTS → DOCUMENTATION `docs/decisions/issue-1103-bounded-dependency-cycle-detection-preflight.md` (Iterative graph-traversal decision and compatibility boundary) +- IMPLEMENTS → CODE_FILE `implementations/python/packages/raes_processor/semantics/planner.py` (Iterative strongly-connected dependency analysis) +- TESTS → TEST `implementations/python/tests/test_semantics_planner.py` (Cycle-oracle, long-chain, and deterministic-order regressions) diff --git a/implementations/python/packages/raes_processor/planner/ordering.py b/implementations/python/packages/raes_processor/planner/ordering.py index 52e4e1cf..7ef6cf23 100644 --- a/implementations/python/packages/raes_processor/planner/ordering.py +++ b/implementations/python/packages/raes_processor/planner/ordering.py @@ -2,8 +2,6 @@ from ..models import Diagnostic, PlannedResource, RuntimeDomain, SnapshotEntry from ..semantics.planner import ( - DependencyKind, - dependency_graph_for_resources, resource_delete_order, resource_dependency_cycles, resource_topological_order, @@ -12,10 +10,6 @@ from ..semantics.realization_snapshot_sanitization import realization_payloads_match -def _ordering_graph(resources: dict[str, PlannedResource]) -> dict[str, tuple[str, ...]]: - return dependency_graph_for_resources(resources, kind=DependencyKind.ORDERING) - - def _ordering_cycles(resources: dict[str, PlannedResource]) -> list[tuple[str, ...]]: return resource_dependency_cycles(resources) diff --git a/implementations/python/packages/raes_processor/semantics/planner.py b/implementations/python/packages/raes_processor/semantics/planner.py index b246a16d..26ff9f6e 100644 --- a/implementations/python/packages/raes_processor/semantics/planner.py +++ b/implementations/python/packages/raes_processor/semantics/planner.py @@ -3,7 +3,7 @@ from __future__ import annotations from collections import deque -from collections.abc import Callable, Iterable, Mapping +from collections.abc import Callable, Iterable, Iterator, Mapping from dataclasses import dataclass from enum import Enum from typing import Protocol, TypeVar @@ -123,7 +123,7 @@ def dependency_cycles( on_stack: set[str] = set() cycles: list[tuple[str, ...]] = [] - def strongconnect(node: str) -> None: + def visit(node: str) -> None: nonlocal index indices[node] = index lowlinks[node] = index @@ -131,16 +131,7 @@ def strongconnect(node: str) -> None: stack.append(node) on_stack.add(node) - for dependency in graph[node]: - if dependency not in indices: - strongconnect(dependency) - lowlinks[node] = min(lowlinks[node], lowlinks[dependency]) - elif dependency in on_stack: - lowlinks[node] = min(lowlinks[node], indices[dependency]) - - if lowlinks[node] != indices[node]: - return - + def close_component(node: str) -> None: component: list[str] = [] while stack: member = stack.pop() @@ -153,6 +144,33 @@ def strongconnect(node: str) -> None: if len(component) > 1 or component[0] in graph[component[0]]: cycles.append(tuple(component)) + # Tarjan is driven from an explicit stack rather than recursion: DFS depth + # reaches the longest dependency path, so a recursive walk raises + # RecursionError on scenarios with more than roughly a thousand chained + # resources, aborting planning instead of reporting cycles. + def strongconnect(root: str) -> None: + visit(root) + frames: list[tuple[str, Iterator[str]]] = [(root, iter(graph[root]))] + while frames: + node, dependencies = frames[-1] + descended = False + for dependency in dependencies: + if dependency not in indices: + visit(dependency) + frames.append((dependency, iter(graph[dependency]))) + descended = True + break + if dependency in on_stack: + lowlinks[node] = min(lowlinks[node], indices[dependency]) + if descended: + continue + frames.pop() + if frames: + parent = frames[-1][0] + lowlinks[parent] = min(lowlinks[parent], lowlinks[node]) + if lowlinks[node] == indices[node]: + close_component(node) + for node in sorted(graph, key=canonical_resource_identity): if node not in indices: strongconnect(node) diff --git a/implementations/python/tests/test_semantics_planner.py b/implementations/python/tests/test_semantics_planner.py index 4f74ba0d..232736b6 100644 --- a/implementations/python/tests/test_semantics_planner.py +++ b/implementations/python/tests/test_semantics_planner.py @@ -2,16 +2,21 @@ from __future__ import annotations +import itertools from types import SimpleNamespace -from hypothesis import given +from hypothesis import given, settings from hypothesis import strategies as st from raes_processor.semantics.planner import ( DependencyKind, + canonical_resource_identity, + dependency_cycles, dependency_edges, + dependency_graph, refresh_impacted_nodes, resource_delete_order, resource_topological_order, + topological_dependency_order, ) @@ -81,6 +86,94 @@ def _dag_resources_with_change_sets(draw): return resources, subset_a, subset_b +def _dependency_graphs() -> st.SearchStrategy[dict[str, tuple[str, ...]]]: + """Small graphs that freely admit self-loops and multi-node cycles.""" + + def _build(size: int, choices: list[list[int]]) -> dict[str, tuple[str, ...]]: + nodes = [f"nodes.host-{index}" for index in range(size)] + return { + node: tuple(nodes[target % size] for target in targets) + for node, targets in zip(nodes, choices, strict=True) + } + + # Deliberately larger and denser than the DAG strategies above: the explicit + # frame stack has to resume a partially consumed iterator, so the graphs that + # matter are the ones where a node still has unvisited dependencies left when + # a descent happens and some of those are already on the stack. + return st.integers(min_value=1, max_value=18).flatmap( + lambda size: st.lists( + st.lists(st.integers(min_value=0, max_value=17), max_size=6), + min_size=size, + max_size=size, + ).map(lambda choices: _build(size, choices)) + ) + + +def _exhaustive_dependency_graphs(size: int): + """Enumerate every simple directed graph of one fixed small size.""" + + nodes = tuple(f"nodes.host-{index}" for index in range(size)) + possible_edges = tuple(itertools.product(nodes, repeat=2)) + for mask in range(1 << len(possible_edges)): + yield { + node: tuple( + target + for edge_index, (source, target) in enumerate(possible_edges) + if source == node and mask & (1 << edge_index) + ) + for node in nodes + } + + +def _reference_dependency_cycles( + dependencies_by_node: dict[str, tuple[str, ...]], +) -> list[tuple[str, ...]]: + """Recursive Tarjan reference for differential comparison. + + Kept deliberately naive: it mirrors the textbook recursion the production + walk replaced, so any behavioural drift in the explicit-stack version shows + up as a mismatch rather than as a silently different plan order. + """ + + graph = dependency_graph(dependencies_by_node) + if not graph: + return [] + counter = itertools.count() + indices: dict[str, int] = {} + lowlinks: dict[str, int] = {} + stack: list[str] = [] + on_stack: set[str] = set() + cycles: list[tuple[str, ...]] = [] + + def strongconnect(node: str) -> None: + indices[node] = lowlinks[node] = next(counter) + stack.append(node) + on_stack.add(node) + for dependency in graph[node]: + if dependency not in indices: + strongconnect(dependency) + lowlinks[node] = min(lowlinks[node], lowlinks[dependency]) + elif dependency in on_stack: + lowlinks[node] = min(lowlinks[node], indices[dependency]) + if lowlinks[node] != indices[node]: + return + component: list[str] = [] + while stack: + member = stack.pop() + on_stack.remove(member) + component.append(member) + if member == node: + break + component = sorted(component) + if len(component) > 1 or component[0] in graph[component[0]]: + cycles.append(tuple(component)) + + for node in sorted(graph, key=canonical_resource_identity): + if node not in indices: + strongconnect(node) + return sorted(cycles, key=lambda cycle: tuple(canonical_resource_identity(node) for node in cycle)) + + class TestPlannerSemantics: def test_dependency_edges_preserve_kinds(self): resources = { @@ -134,3 +227,68 @@ def test_refresh_propagation_is_monotonic(self, payload): impacted_b = subset_b | set(refresh_impacted_nodes(resources, subset_b)) assert impacted_a <= impacted_b + + +class TestDependencyCycleScale: + """Cycle detection must survive dependency chains longer than the recursion limit.""" + + _DEEP = 5000 + + def _chain(self, size: int) -> dict[str, tuple[str, ...]]: + graph: dict[str, tuple[str, ...]] = { + f"nodes.host-{index:05d}": (f"nodes.host-{index + 1:05d}",) for index in range(size) + } + graph[f"nodes.host-{size:05d}"] = () + return graph + + def test_deep_acyclic_chain_reports_no_cycles(self): + assert dependency_cycles(self._chain(self._DEEP)) == [] + + def test_deep_cycle_is_still_detected(self): + size = 3000 + graph = {f"nodes.host-{index:05d}": (f"nodes.host-{(index + 1) % size:05d}",) for index in range(size)} + + cycles = dependency_cycles(graph) + + assert len(cycles) == 1 + assert len(cycles[0]) == size + + def test_deep_chain_topological_order_is_complete(self): + graph = self._chain(self._DEEP) + + assert len(topological_dependency_order(graph)) == len(graph) + + def test_unknown_dependency_references_do_not_become_graph_nodes(self): + graph = { + "nodes.host-a": ("nodes.host-missing",), + "nodes.host-b": ("nodes.host-a",), + } + + assert dependency_cycles(graph) == [] + + def test_cycle_order_is_independent_of_mapping_and_edge_insertion_order(self): + forward = { + "nodes.host-a": ("nodes.host-b",), + "nodes.host-b": ("nodes.host-a",), + "nodes.host-c": ("nodes.host-c",), + "nodes.host-d": (), + } + reversed_input = { + node: tuple(reversed(dependencies)) for node, dependencies in reversed(tuple(forward.items())) + } + + expected = [("nodes.host-a", "nodes.host-b"), ("nodes.host-c",)] + assert dependency_cycles(forward) == expected + assert dependency_cycles(reversed_input) == expected + + def test_cycle_detection_matches_oracle_for_every_graph_up_to_three_nodes(self): + for size in range(1, 4): + for graph in _exhaustive_dependency_graphs(size): + assert dependency_cycles(graph) == _reference_dependency_cycles(graph) + + @settings(max_examples=400) + @given(_dependency_graphs()) + def test_cycle_detection_matches_a_reference_walk(self, graph): + """Guards the explicit-stack walk against the recursive semantics it replaced.""" + + assert dependency_cycles(graph) == _reference_dependency_cycles(graph)