fix(test): collect two live-service driver suites that pytest refused (#14979)

[mrveiss]

Refs #14979. Partial by design — 18 of a measured 106, with the remaining 88 triaged below rather than swept.

Thinking Path

The number in #14979 is stale. Re-deriving the population with repo_tests/test_methods_in_uncollected_classes_test.py's own _scan() on this base gives 106 uncollected test_* methods in 16 files, not 90 in 14. The extra 16 are agents/atomic_facts_extraction_test.py (8) and services/temporal_invalidation_test.py (8): both are named Test*, so #14927's name-based sweep never saw them, and both define __init__, so pytest refuses the class. They only became visible when #14999 made _init_state walk the MRO. That is the sweep working as designed, and it is why the first move here was to measure rather than to trust the table.

The population is not one thing. Sorting the 16 files by what they actually dial:

class	files	methods	what they need
in-process, no service	2	18	nothing — converted here
live HTTP to a running stack	11	60	a backend, TTS worker or NPU service listening
real GPU/NPU hardware	1	10	utils/hardware_metrics_test.py reads live device state
blocked on a product fix	1	8	agents/atomic_facts_extraction_test.py
outside any pytest invocation	(5 of the above)	29	autobot-infrastructure / autobot-frontend are not in testpaths

The two in-process files are the whole of what is safely convertible today, and they were worth doing precisely because the issue's own analysis assumed all 14 were live-service drivers. They are not: takeover_manager_e2e_test.py imports AsyncMock and issues zero network calls, and temporal_invalidation_test.py ships its own MockFactExtractionService. Both were uncollectable for the ordinary reason — a class with __init__ — and nothing else.

A rename would not have been enough, and the issue is right about that. Each conversion turned up something the driver loop had been hiding, and each of those had to be fixed for the collected method to mean anything:

• takeover_manager_e2e_test.py wrapped its imports in try/except ImportError and opened all ten methods with if not COMPONENTS_AVAILABLE: return. A broken import produced ten silent passes. The imports are now plain, so an import failure is a collection error with a traceback.
• test_chat_integration called the real Orchestrator, whose manager-side handler turns any planner error into return None (fix(orchestrator): workflow planning is dead at every caller — un-awaited coroutines plus retired WorkflowTask field names #13730), and then asserted only if workflow_id:. It passed whether or not planning worked. The orchestrator is now an AsyncMock and the assertions are unconditional.
• test_command_risk_assessment asserted a Python transcription of the frontend's assessCommandRisk, defined inside the test body — a test asserting its own copy of the logic, which no product change can fail. It now asserts SecureCommandExecutor.assess_command_risk. Retargeting it surfaced a real divergence between the UI gate and the executor, filed as security(terminal): the UI risk gate and the executor disagree on 6 of 8 sampled commands #15011.
• test_error_handling wrapped both halves in try/except Exception and logged the exception as a success.
• test_actual_invalidation_sweep depended on run_all_tests having called the dry-run test first against a shared Redis. Run alone it returned "No invalidation rules available" and asserted against the error response.
• Two rule branches in test_invalidation_rule_matching tripped the age limit before the confidence and source-pattern paths were ever consulted, so min_confidence and source_patterns were unexercised. Both mutations came back green until fresh fixtures were added; they are red now.
• Three assertions were conditional on the behaviour they checked (if stats["recent_sweeps"] > 0: assert ...).
• Fixture facts used a naive datetime.now(), which makes InvalidationRule.matches_fact raise TypeError against its timezone-aware now. The one production producer passes an aware value, so this was the fixture's bug.

Skips were considered and then removed. Three takeover tests need the workflow state machine's Redis persistence, and the backend conftest replaces autobot_shared.redis_client with a socket-free stand-in (#14932). require_real_redis_client — the mechanism #14930 established — was the first answer, and it worked: three honest, named skips. It was then replaced with an injected in-process fakeredis, the pattern services/workflow_redis_backfill_test.py and llm_shared/tests/test_provider_degradation.py already use, because a test that runs beats a test that explains why it does not. All 19 methods run for real; none skip. Reaching the NoneType dereference on the way there is filed as #15012.

What was deliberately left, and the citation that was wrong. agents/atomic_facts_extraction_test.py is dead in a way a rename would disguise: self.extraction_agent is never assigned, so every one of its eight methods would AttributeError on collection. The first revision of this PR deferred it citing #6994 — which review showed was already closed (2026-05-06, via #7076), and worse, the TODO(#6994) in that file was planted by that same closing commit and says on its face that this work was out of scope for #6994. Nothing had tracked it since May. It is now #15043, and the comment in the file points there and states the actual defect instead of a finished issue. Collecting the file to a red suite says nothing about the code under test, and skipping it would hide a broken test behind a precondition that is not a precondition — the exact defect this umbrella exists to end. It stays uncounted-but-recorded, with its 8 inside the ceiling.

What Changed

autobot-backend/takeover_manager_e2e_test.py — SessionTakeoverTestSuite → TestSessionTakeover, __init__ → setup_method. The COMPONENTS_AVAILABLE wrapper and all ten silent-return guards are gone; run_all_tests, print_test_summary and the __main__ driver give way to pytest. The run_demo_workflow() showcase, which built a five-step workflow and asserted nothing, became test_status_reports_every_step_of_a_multi_step_workflow. 10 methods → 11 collected tests, all passing.

autobot-backend/services/temporal_invalidation_test.py — __init__ → setup_method with an injected fakeredis rule store, run_all_tests and main() removed, every trailing return <value> dropped so none of these enter #14920's population, the naive-datetime fixtures fixed, and the conditional assertions made unconditional. 8 methods → 8 collected tests, all passing.

repo_tests/test_methods_in_uncollected_classes_test.py — autobot-backend ceiling 77 → 59, with the reason inline. Down only. The other two trees are untouched, and the collectable-test floors are unchanged.

The two print-based driver summaries are gone rather than left behind # noqa: print; nothing else in either file prints.

File-size ratchet (from review). takeover_manager_e2e_test.py shrank 663 → 411, which is under MAX_LINES (600), so its grandfather entries were deleted from scripts/python_file_size_known_large.py and repo_tests/python_file_size_ratchet_baseline.py rather than lowered — the audit's own message is that an entry naming a compliant file exempts nothing while looking authoritative. Separately, the explanatory comment on the 77 → 59 change had grown the uncollected-class guard 921 → 926, past its own recorded ceiling; a grandfathered exemption freezes the size it was granted for, so the comment was condensed back to 921 rather than the ceiling raised. Both facts survive, folded into the paragraph above the dict — including its correction, since "every one of the remaining offenders is a live-service driver" is no longer true.

Decomposition (#560, from review). temporal_invalidation_test.py is rewritten by this PR, so its two functions over the 70-line threshold were split now rather than carried: _setup_test_facts (79 lines) is a seven-row table plus a loop, and test_invalidation_rule_matching (74) delegates to _assert_rule_matches and _fresh_fact. Longest function in the file is now 31 lines. Its rule-count floor rose from >= 3 to >= 4 in the process, since all four rule families are in fact exercised.

autobot-backend/agents/atomic_facts_extraction_test.py — comment only, no behaviour change: the stale TODO(#6994) now points at #15043 and says what is actually wrong. The dead no-op statement before self.test_content goes with it. The file's 8 methods stay inside the ceiling; it is still deferred.

Verification

Local, characterising this interpreter only — CI is authoritative. Python 3.10.12, pytest 9.0.2.

autobot-backend/takeover_manager_e2e_test.py            11 passed
autobot-backend/services/temporal_invalidation_test.py   8 passed
repo_tests/python_file_size_ratchet_test.py             31 passed
repo_tests/test_methods_in_uncollected_classes_test.py  11 passed
repo_tests/tests_that_return_instead_of_asserting_test.py 5 passed  (87s)
flake8 on all six changed files                          0

The file-size ratchet was red before this fix and is green after: the first revision left takeover_manager_e2e_test.py pinned at 663 in both lists while the file was 411, and pushed the uncollected-class guard 5 lines past its ceiling. Deleting the two now-compliant entries and condensing the comment takes repo_tests/python_file_size_ratchet_test.py from 5 failed / 26 passed to 31 passed. Lowering the entry to 411 was tried first and is wrong — test_every_entry_carries_a_ceiling rejects any ceiling at or below MAX_LINES, and audit_ceilings names the file directly. #14629 turned discovery on for this audit, which is why it surfaced now.

The offender count is unchanged by the review fixes: 88 repo-wide, 59 in autobot-backend, file-by-file identical.

The returning-test ratchet is a two-way one: it fails if a count drops below its budget as well as above. It passes unchanged, confirming these 18 conversions neither added a returning test nor moved that population — which is what the #14927 exclusion predicted.

Mutation evidence. Every guarded line was broken, confirmed red, and restored. No mutation is pushed; the tree is clean at git status after each.

mutation	file:line	result
_extract_command_from_step maps package-update to a different command	services/workflow_automation/manager.py:140	test_chat_integration RED
planner task ids remapped one step off	services/workflow_automation/manager.py:105	test_chat_integration RED
status dict reports every step as confirmation-required	services/workflow_automation/models.py:162	test_status_reports_every_step_... RED
unknown workflow accepted by the controller	services/workflow_automation/controller.py:43	test_error_handling RED
assess_command_risk returns SAFE for everything	secure_command_executor.py:670	test_command_risk_assessment RED
pause no longer sets is_paused	services/workflow_automation/controller.py:78	test_manual_takeover RED
intervention never recorded	services/workflow_automation/controller.py:55	test_manual_takeover, test_pause_resume_workflow RED
step never enters WAITING_APPROVAL	services/workflow_automation/executor.py:375	test_step_confirmation_flow RED
age rule never fires	services/temporal_invalidation_service.py:87	test_invalidation_rule_matching RED
confidence rule never fires	services/temporal_invalidation_service.py:92	test_invalidation_rule_matching RED
source-pattern rule never fires	services/temporal_invalidation_service.py:98	test_invalidation_rule_matching RED
is_contradictory_to always False	models/atomic_fact.py:173	test_contradiction_detection RED
statistics drop total_rules	services/temporal_invalidation_service.py:909	test_invalidation_statistics RED

Two of those — the confidence and source-pattern rules — came back green on the first attempt, because the fixtures tripped the age limit first. That is what drove the extra fresh-fixture assertions; both are red now. A mutation run that finds nothing is the finding.

All three temporal_invalidation_service.py mutations (:87 age, :92 confidence, :98 source pattern) were re-run after the decomposition and all three are still red, so extracting _assert_rule_matches and _fresh_fact preserved the strength rather than diluting it.

Ratchet movement. autobot-backend 77 → 59 (−18), derived by the guard's own _scan() before and after. Repo-wide 106 → 88. Ceilings only ever move down; no budget was raised and no count went up. The two file-size entries went to zero by deletion, which is what that ratchet requires once a file is compliant.

Model Used

Claude Opus 5 (1M context).

Discovered and filed

• security(terminal): the UI risk gate and the executor disagree on 6 of 8 sampled commands #15011 security(terminal) — the UI risk gate (TerminalWindow.vue:506) and the executor (secure_command_executor.py:670) disagree on 6 of 8 sampled commands. sudo apt update is low in the UI, which asks nothing, and HIGH at the executor; a recursive delete prompts the user and is then refused outright. Two hand-maintained classifiers, no shared source.
• bug(workflow): the workflow state machine dereferences a None Redis client instead of naming the failure #15012 bug(workflow) — state_machine.py dereferences a None Redis client at three call sites, so an unreachable store surfaces as AttributeError: 'NoneType' object has no attribute 'set' mid-transition instead of naming the subsystem.
• bug(test): 8 atomic-fact extraction tests cannot run — the agent under test is never constructed #15043 bug(test) — the 8 atomic-fact extraction tests cannot run because the agent under test is never constructed. Filed to replace the closed-issue citation this PR originally carried; it records what those 8 actually need (a KnowledgeExtractionAgent constructible with no live provider, and a MockLLMService whose output extract_facts_from_text can parse).

Deferred, with reasons

file	methods	why not now
autobot-backend/agents/atomic_facts_extraction_test.py	8	self.extraction_agent never assigned; tracked by #15043
autobot-backend/utils/hardware_metrics_test.py	10	reads live GPU/NPU device state
autobot-backend/comprehensive_system_validation_test.py	8	live HTTP
autobot-backend/knowledge/chat_knowledge_system_e2e_test.py	7	live HTTP
autobot-backend/monitoring/monitoring_and_alerts_test.py	7	live HTTP
autobot-backend/agents/multi_agent_workflow_validation_test.py	6	live HTTP
autobot-backend/npu_integration_e2e_test.py	5	live NPU worker
autobot-backend/async_baseline_performance_test.py	4	live HTTP
autobot-backend/knowledge/knowledge_performance_test.py	4	live HTTP
autobot-infrastructure/shared/scripts/utilities/test_autobot_functionality.py	8	live HTTP; tree not in testpaths
autobot-infrastructure/shared/tests/performance/test_performance_optimization.py	5	live HTTP; tree not in testpaths
autobot-infrastructure/shared/scripts/analysis/test_npu_worker.py	3	live NPU worker; tree not in testpaths
autobot-infrastructure/shared/tests/integration/test_distributed_system_integration.py	3	live HTTP; tree not in testpaths
autobot-frontend/tests/frontend_comprehensive_corrected_test.py	10	live HTTP + WebSocket; tree not in testpaths; autobot-frontend/ is in flight under #14989

The 29 methods in autobot-infrastructure and autobot-frontend need the tree wired into a pytest invocation before a conversion means anything — #14917's family — as #14979's own acceptance criteria says.

Nothing was renamed out of the test_* namespace, nothing was blanket-skipped, and nothing was deleted to lower a count.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.