docs(orchestration): deprecate WorkflowPlanner in place with a test that holds the invariant (#13751)

.session/HANDOFF-issue-13751.md

@@ -0,0 +1,44 @@
+# Handoff: issue-13751
+status: complete
+pr: #13760
+base_at_push: origin/Dev_new_gui (up to date at push)
+gates: tests=PASS (5/5 guard; 598 orchestration sweep; 316 repo_tests)
+needs_rebase_before_merge: no
+remaining:
+blocked_on:
+worktree: .worktrees/issue-13751  (safe to remove after merge)
+
+## Resolution chosen
+
+Took the "record superseded" branch of #13751's ACs, not "wire it in". Every public
+method on `orchestration.WorkflowPlanner` has zero callers, and every capability has a
+wired canonical equivalent (`create_workflow_plan`,
+`AgentRouter.get_agent_recommendations_scored`, `_fetch_planning_context`, services
+`WorkflowExecutor.present_plan_for_approval`). Wiring it would recreate the parallel
+planning path the issue was filed to prevent.
+
+Follows the #12373/#12579 precedent ("deprecate dead orchestration engine in place"),
+which had covered the executor half and missed this planner sibling.
+
+## Scope correction found mid-task
+
+`create_plan_summary_for_approval` is ALSO uncalled — the whole class is unreachable,
+not just the two methods named in the issue. Only `__init__` runs.
+
+## Stale claims corrected
+
+- `orchestration/workflow_planning.py` called WorkflowPlanner "the canonical (but
+  currently unwired)" planner — inverted; `StrategyPlanner` there is the wired one.
+- `orchestrator.py` `_step_planner` comment implied a live collaborator.
+
+## Prior history
+
+#6820 tracked this module as one of four orphans and closed 2026-05-17 with three wired
+and this one left. #12579 deprecated the sibling and skipped it. Both recorded intent in
+prose only — nothing held it, which is why it recurred. Hence the new guard test.
+
+## AC 3 reinterpreted (flagged, not silently ticked)
+
+"A test exercises each through its production entry point" is unsatisfiable — there is no
+production entry point. Replaced with a guard pinning the *absence* of one.
+#13730 already covers both methods directly.

autobot-backend/orchestration/__init__.py

@@ -15,7 +15,13 @@
   WorkflowDependencies, FALLBACK_TIERS, ...)
 - agent_registry: Agent registration and management
 - agent_router: TaskAgentScorer / AgentRouter — workflow-task scoring (#6819)
-- workflow_planner: Map capability requirements to available agents
+- workflow_planner: Map capability requirements to available agents. DEPRECATED
+  (#13751) — zero callers on every public method; the planner half of the engine
+  run_workflow stopped using at #5058, missed when #12373 deprecated the
+  executor half. Holds no capability the canonical path lacks: planning +
+  scored capability-to-agent selection are orchestrator.create_workflow_plan
+  and AgentRouter.get_agent_recommendations_scored. Kept in place, code intact;
+  do not wire it in — that would recreate a second planning path.
 - workflow_planning: StrategyPlanner — multi-agent strategy & plan building
 - workflow_runner: WorkflowRunner — multi-agent execution engine (#5058)
 - workflow_executor: Execute a single workflow step by step. DEPRECATED (#12373)

autobot-backend/orchestration/workflow_planner.py

@@ -5,6 +5,42 @@
 """
 Workflow Planner
 
+DEPRECATED — NOT A PRODUCTION PLANNING PATH (#13751). Every public method on
+``WorkflowPlanner`` has **zero callers**. It is the planner half of the
+orchestration engine that ``orchestrator.run_workflow`` stopped using at #5058;
+#12373/#12579 deprecated the executor half
+(``orchestration/workflow_executor.py``) in place and did not reach this
+sibling. ``Orchestrator`` still constructs it as ``self._step_planner``
+(orchestrator.py) but never calls a method on it.
+
+Unlike the executor, this module holds **no capability the canonical path
+lacks**, so there is nothing here staged for future consolidation. Each
+capability maps to wired code today:
+
+- Base plan + capability-to-agent assignment
+  (``plan_workflow_steps_with_agents``) -> ``orchestrator.create_workflow_plan``
+  builds the plan and ``AgentRouter.get_agent_recommendations_scored`` does the
+  scored capability-to-agent selection, reached via
+  ``WorkflowRunner``/``Orchestrator.get_agent_recommendations_scored``.
+- Similar-trajectory priors (``_annotate_context_with_trajectories``, GH#7357)
+  -> ``orchestrator._fetch_planning_context`` (#10580/#10581), tenant-scoped by
+  #11015/#11089.
+- Plan without executing (``get_plan_summary``) ->
+  ``orchestrator.create_workflow_plan``, which builds and stores a
+  ``WorkflowPlan`` without running it.
+- Approval presentation (``create_plan_summary_for_approval``) ->
+  ``services.workflow_automation.executor.WorkflowExecutor
+  .present_plan_for_approval``, reached through
+  ``WorkflowAutomationManager.present_plan_for_approval`` (#390) and the
+  ``/api/workflow-automation/*`` surface.
+
+Kept **in place with all code intact** — per repo policy code is never deleted,
+only wired in or superseded. Do not wire these methods into a request path:
+doing so would create a second planning path alongside
+``create_workflow_plan`` for the two to drift against, which is the outcome
+#13751 was filed to avoid. ``repo_tests/workflow_planner_deprecation_test.py``
+holds this invariant so the statement above cannot silently go stale.
+
 Issue #381: Extracted from enhanced_orchestrator.py god class refactoring.
 Contains workflow planning, step estimation, and capability determination.
 """
@@ -25,6 +61,10 @@ class WorkflowPlanner:
     """
     Plans workflow steps with intelligent agent assignment.
 
+    DEPRECATED — no callers on any public method; see the module docstring for
+    the per-capability mapping to the canonical code that supersedes each one.
+    Retained in full for reference — no code removed (#13751).
+
     Handles:
     - Enhanced workflow step planning
     - Capability determination for steps

autobot-backend/orchestration/workflow_planning.py

@@ -9,11 +9,20 @@
 Contains workflow planning, building, and utility functions.
 
 Note: The class in this module is named ``StrategyPlanner`` — not ``WorkflowPlanner`` —
-to distinguish it from the canonical (but currently unwired) ``orchestration.WorkflowPlanner``
-in ``autobot-backend/orchestration/workflow_planner.py``.  Both classes were historically
+to distinguish it from ``orchestration.WorkflowPlanner`` in
+``autobot-backend/orchestration/workflow_planner.py``.  Both classes were historically
 called ``WorkflowPlanner`` and every import site aliased this one ``as StrategyPlanner``;
-the rename removes that aliasing smell (#6817).  The orphan status of the canonical class
-is tracked separately in #6820.
+the rename removes that aliasing smell (#6817).
+
+#13751 corrects this note: it previously called that class "the canonical (but
+currently unwired)" planner, which inverted the actual relationship.  It is not
+canonical — it is DEPRECATED with zero callers on every public method, superseded
+by ``orchestrator.create_workflow_plan`` plus
+``AgentRouter.get_agent_recommendations_scored``.  ``StrategyPlanner`` here *is*
+wired (held as ``orchestrator._strategy_planner`` and used as the plan fallback in
+``create_workflow_plan``).  The orphan status was previously tracked under #6820,
+which closed 2026-05-17 having wired three of its four modules and left this one;
+#13751 resolves it as deprecated-in-place rather than leaving it open-ended.
 
 Moved from enhanced_orchestration.workflow_planning to orchestration.workflow_planning
 (issue #10666 B3).

autobot-backend/orchestrator.py

@@ -229,6 +229,14 @@ def _init_strategy_components(self) -> None:
 
         # Step planner — capability-to-agent mapping for single-workflow steps (GH #6820).
         # Populated after _initialize_default_agents so agent_registry is non-empty.
+        #
+        # #13751: constructed but never called — no method on this object is
+        # invoked anywhere. WorkflowPlanner is DEPRECATED (the planner half of
+        # the engine run_workflow left behind at #5058); the live equivalents are
+        # create_workflow_plan for planning and get_agent_recommendations_scored
+        # for capability-to-agent selection. The attribute is retained rather
+        # than dropped, per the never-delete policy; see
+        # orchestration/workflow_planner.py for the per-capability mapping.
         self._step_planner = WorkflowPlanner(
             base_orchestrator=self,
             agent_registry=self.agent_registry,

repo_tests/workflow_planner_deprecation_test.py

@@ -0,0 +1,166 @@
+# Copyright 2025-2026 mrveiss
+# SPDX-License-Identifier: Apache-2.0
+"""``orchestration.WorkflowPlanner`` is deprecated in place — hold that (#13751).
+
+#12373/#12579 deprecated ``orchestration/workflow_executor.py`` in place and
+recorded the rationale in docstrings only. Nothing held the claim, so the
+statement "no production callers" could go stale in either direction: someone
+wires the module and the docstring still says it is dead, or the note is dropped
+while it is still true. Its planner sibling was missed entirely by that pass,
+which is how #13751 arose.
+
+These tests pin both halves of the invariant for ``WorkflowPlanner``:
+
+* it stays unwired (an accidental call site fails here, with instructions), and
+* it stays **intact and documented** — the never-delete policy means the fix for
+  a failure here is never to delete the module.
+
+Deliberately *not* scanning for the bare name ``get_plan_summary``:
+``agents/overseer/overseer_agent.py`` defines an unrelated method with the same
+name (no arguments, returns ``str | None``) that *is* wired via
+``api/overseer_handlers.py``. A name-only scan would flag it. Reachability is
+pinned through the instance attribute and the import instead, which cannot
+collide.
+"""
+
+from __future__ import annotations
+
+import ast
+from pathlib import Path
+from typing import List
+
+_REPO = Path(__file__).resolve().parents[1]
+_BACKEND = _REPO / "autobot-backend"
+
+_MODULE = _BACKEND / "orchestration" / "workflow_planner.py"
+_PACKAGE_INIT = _BACKEND / "orchestration" / "__init__.py"
+
+# The instance attribute Orchestrator binds the planner to. Only the assignment
+# may exist; any attribute access on it would be a live call path.
+_INSTANCE_ATTR = "_step_planner"
+
+# Method names unique to WorkflowPlanner (no homonyms anywhere in the tree), so
+# scanning for them by name is unambiguous.
+_UNIQUE_METHOD_NAMES = ("plan_workflow_steps_with_agents", "create_plan_summary_for_approval")
+
+# Public API that must remain present — deleting it is not an allowed fix.
+_REQUIRED_METHODS = (
+    "plan_workflow_steps_with_agents",
+    "get_plan_summary",
+    "create_plan_summary_for_approval",
+    "determine_step_capabilities",
+    "estimate_step_duration",
+)
+
+_WIRING_INSTRUCTIONS = (
+    "If this was wired deliberately, that is a design change, not a test fix: "
+    "WorkflowPlanner duplicates orchestrator.create_workflow_plan and "
+    "AgentRouter.get_agent_recommendations_scored, and #13751 exists to keep a "
+    "second planning path from drifting against them. Update the deprecation "
+    "notes in orchestration/workflow_planner.py and orchestration/__init__.py "
+    "in the same change, and adjust this test to match."
+)
+
+
+def _production_sources() -> List[Path]:
+    """Backend .py files excluding tests and the deprecated module itself."""
+    skip_names = {_MODULE.name}
+    out: List[Path] = []
+    for path in _BACKEND.rglob("*.py"):
+        name = path.name
+        if name in skip_names or name.endswith("_test.py") or name.startswith("test_"):
+            continue
+        if "/tests/" in path.as_posix() or "/node_modules/" in path.as_posix():
+            continue
+        out.append(path)
+    return out
+
+
+def test_planner_instance_is_constructed_but_never_called():
+    """`self._step_planner.<anything>` would be a live path — none may exist."""
+    offenders = []
+    for path in _production_sources():
+        try:
+            tree = ast.parse(path.read_text(encoding="utf-8"))
+        except SyntaxError:
+            continue
+        for node in ast.walk(tree):
+            # Matches `<expr>._step_planner.<attr>` — an access *through* the
+            # attribute, as opposed to `self._step_planner = ...`.
+            if (
+                isinstance(node, ast.Attribute)
+                and isinstance(node.value, ast.Attribute)
+                and node.value.attr == _INSTANCE_ATTR
+            ):
+                offenders.append(f"{path.relative_to(_REPO)}:{node.lineno} -> .{_INSTANCE_ATTR}.{node.attr}")
+
+    assert not offenders, f"WorkflowPlanner is documented as unwired but is now called: {offenders}. {_WIRING_INSTRUCTIONS}"
+
+
+def test_unique_planner_methods_have_no_production_call_sites():
+    """The two uniquely-named methods must appear nowhere in production code."""
+    offenders = []
+    for path in _production_sources():
+        text = path.read_text(encoding="utf-8")
+        for method in _UNIQUE_METHOD_NAMES:
+            if method in text:
+                offenders.append(f"{path.relative_to(_REPO)} -> {method}")
+
+    assert not offenders, f"Deprecated WorkflowPlanner methods referenced in production: {offenders}. {_WIRING_INSTRUCTIONS}"
+
+
+def test_only_known_files_reference_the_planner():
+    """Reachability stays auditable: every mention is accounted for, with a reason.
+
+    Construction happens in exactly one place. The other two are not call
+    paths — a compatibility re-export and a disambiguation note — and are
+    named here so a genuinely new reference stands out.
+    """
+    allowed = {
+        "autobot-backend/orchestrator.py": "constructs it as _step_planner; never calls it",
+        "autobot-backend/orchestration/__init__.py": "re-export kept for compatibility, as #12579 did for the executor",
+        "autobot-backend/orchestration/workflow_planning.py": "docstring note distinguishing StrategyPlanner from it",
+    }
+    found = sorted(
+        path.relative_to(_REPO).as_posix()
+        for path in _production_sources()
+        if "WorkflowPlanner" in path.read_text(encoding="utf-8")
+    )
+
+    unexpected = [p for p in found if p not in allowed]
+    assert not unexpected, f"New reference(s) to deprecated WorkflowPlanner: {unexpected}. {_WIRING_INSTRUCTIONS}"
+
+    # Also guard the other direction: if a listed file stops mentioning it, the
+    # reason recorded here is stale and should be re-checked.
+    vanished = [p for p in allowed if p not in found]
+    assert not vanished, f"Expected reference(s) to WorkflowPlanner disappeared from {vanished}; update this allow-list."
+
+
+def test_module_and_package_record_the_deprecation():
+    """The rationale must stay written down — this is what #12579 lacked."""
+    module_doc = ast.get_docstring(ast.parse(_MODULE.read_text(encoding="utf-8"))) or ""
+    assert "DEPRECATED" in module_doc, "workflow_planner.py lost its deprecation notice"
+    assert "#13751" in module_doc, "workflow_planner.py deprecation notice lost its issue reference"
+    # The supersession must name the canonical replacements, not just say "dead".
+    assert "create_workflow_plan" in module_doc
+    assert "get_agent_recommendations_scored" in module_doc
+
+    package_doc = ast.get_docstring(ast.parse(_PACKAGE_INIT.read_text(encoding="utf-8"))) or ""
+    assert "workflow_planner" in package_doc
+    assert "DEPRECATED" in package_doc.split("workflow_planner", 1)[1][:400], (
+        "orchestration/__init__.py no longer marks workflow_planner deprecated"
+    )
+
+
+def test_deprecated_code_is_retained_in_full():
+    """Never-delete policy: deprecation must not become quiet removal."""
+    tree = ast.parse(_MODULE.read_text(encoding="utf-8"))
+    planner = next(
+        (n for n in ast.walk(tree) if isinstance(n, ast.ClassDef) and n.name == "WorkflowPlanner"),
+        None,
+    )
+    assert planner is not None, "WorkflowPlanner class was removed — deprecation is in place, not a deletion"
+
+    defined = {n.name for n in planner.body if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef))}
+    missing = [m for m in _REQUIRED_METHODS if m not in defined]
+    assert not missing, f"Deprecated-in-place module lost methods {missing}; code is retained, never deleted (#13751)"