test(orchestrator): TestOrchestratorPerformance errors on base — orchestrator has no attribute 'config_manager' #13699

Activity

1. mrveiss

   added

   bugSomething isn't workingSomething isn't working

   backend

   testing

   on Aug 8, 2026
2. github-actions

   added

   advanced

   frontend

   infrastructure

   on Aug 8, 2026
3. mrveiss commented on Aug 8, 2026

   

   mrveiss

   on Aug 8, 2026

   OwnerAuthor

   More actions

   Root-caused and fixed on branch issue-13699.

   Why the patch failed: orchestrator has no module-level config_manager — it imports get_config_manager, and Orchestrator.__init__ takes an injectable config_manager (#13162). Setup now injects the config double via the constructor, the same way dependency_injection_test.py does.

   Two further reasons the bodies would still have measured nothing:

   1. GemmaClassificationAgent() raises AgentConfigurationError unless a classification provider is configured, and _init_classification_agent swallows that into classification_agent = None. classify_request_complexity then short-circuits on the None branch — so the benchmark timed an early return, and timed a different path depending on the environment. Setup now binds a classification double.
   2. assert isinstance(steps, list) passes on the empty list — and plan_workflow_steps was in fact returning [] on every call. WorkflowStep has aliased WorkflowTask since refactor: unify 5 duplicate WorkflowStep/Task/Plan definitions + add first-class prompt/tools fields #6951 Phase 2A (task_id / estimated_duration_seconds), but the method still passed the retired id / expected_duration_ms, so every invocation raised TypeError into the broad handler. Fixed in the same PR; the guard now logs exc_info so the next such breakage is diagnosable.

   So this issue's "looks present, never ran" concern was two layers deep: the tests could not run, and the method they cover produced nothing.

   Discovered, out of scope → #13730: all four production callers invoke the async planning API without await, and read step.id / step.user_approval_required, which WorkflowTask does not define. The benchmark repaired here is the only awaited caller in the tree. plan_steps_e2e_test.py has no assertions and passed throughout.

   PR to follow.
4. mrveiss

   added 2 commits that reference this issue on Aug 8, 2026

   chore(session): handoff for issue-13699 (#13699)

   8859f51

   fix(orchestrator): repair TestOrchestratorPerformance patch target an…

   ...

   2dd19bf
5. mrveiss commented on Aug 8, 2026

   

   mrveiss

   on Aug 8, 2026

   OwnerAuthor

   More actions

   Merged and verified on Dev_new_gui.

   PR: #13731 · squash commit: 2dd19bfe9

   Verified in base, not merge state

   $ git log --oneline -1 origin/Dev_new_gui
   2dd19bfe9 fix(orchestrator): repair TestOrchestratorPerformance patch target and the empty plan it hid (#13699)
   
   $ git show origin/Dev_new_gui:autobot-backend/orchestrator.py | grep -c 'task_id="step_'
   4
   $ git show origin/Dev_new_gui:autobot-backend/orchestrator.py | grep -c 'expected_duration_ms='
   0
   

   The reported command, run against the merged base:

   $ python3 -m pytest performance_benchmarks_test.py::TestOrchestratorPerformance -q -p no:randomly
   2 passed in 1.44s          # was: 2 errors in 0.51s
   
   $ python3 -m pytest -m performance performance_benchmarks_test.py -q
   10 passed in 3.65s
   

   Acceptance criteria

   • Both nodes collect and run — no AttributeError at patch time. Setup injects the config double through Orchestrator(config_manager=...) instead of patching the absent orchestrator.config_manager.
   • The assertions in each body execute, verified by deliberate failure. All four assertions were inverted in turn and each fired. Stronger still, the new assert steps failed for real against the pre-fix orchestrator — that is how the empty-plan bug was found:

     E   AssertionError: Planning produced no steps for: List files in current directory...
     E   assert []
     ERROR orchestrator: Failed to plan workflow steps: WorkflowTask.__init__() got an
           unexpected keyword argument 'id'
     

   • No patch target that does not exist on the module under test. The only remaining patch is patch.object(self.orchestrator, "llm_service"), set unconditionally in _init_core_components.

   Beyond the stated ACs

   The "coverage that is not there" concern went two layers deeper than the patch target:

   1. GemmaClassificationAgent() raises unless a classification provider is configured, and _init_classification_agent swallows that into classification_agent = None, so classify_request_complexity short-circuited — and timed a different path per environment. Setup now binds a classification double.
   2. plan_workflow_steps returned [] on every call: WorkflowStep has aliased WorkflowTask since refactor: unify 5 duplicate WorkflowStep/Task/Plan definitions + add first-class prompt/tools fields #6951 Phase 2A, but the method still passed the retired id / expected_duration_ms. Fixed, restoring the invariant workflow_templates/types.py:29-31 already documented. The guard now logs exc_info.

   CI

   59 checks, 0 failures. All 10 required contexts green or correctly skipped (migration-matrix/api-wiring — no migrations, no API surface). All 12 python-suite shards SUCCESS, so not a pytest-split empty-shard false green. CodeQL Python green.

   Carried forward — #13730

   Every remaining caller of plan_workflow_steps is still broken: four production call sites invoke the async planning API without await (the benchmark repaired here was the only awaited caller in the tree), and they read step.id / step.user_approval_required, which WorkflowTask does not define. orchestration/plan_steps_e2e_test.py has no assertions and swallows exceptions. Chat-request workflow planning stays dead end-to-end until #13730 lands — this fixed the planner, not its callers.
6. mrveiss

   closed this as completedon Aug 8, 2026
7. mrveiss

   added a commit that references this issue on Aug 8, 2026

   test(orchestration): cover the planning callers so the empty-plan sta…

   ...

   d789a46
8. mrveiss commented on Aug 9, 2026

   

   mrveiss

   on Aug 9, 2026

   OwnerAuthor

   More actions

   Follow-ups filed from discoveries made while fixing this: #13806 (TaskComplexity aliases collapse — the scheduler's per-complexity multiplier silently never worked), #13807 (a failed classification agent silently disables classification for the process, which is why the benchmark measured a no-op branch), #13808 (finish #6951 Phase 3 — the WorkflowStep alias ambiguity that produced the empty-plan bug fixed here).