api/*.py consumers still bind MagicMock code_intelligence stubs at import time (precommit_analyzer, redis_optimizer, anti_pattern_detector) #13181

Activity

1. mrveiss

   added

   backend

   testing

   on Aug 1, 2026
2. github-actions

   added

   advanced

   frontend

   on Aug 1, 2026
3. mrveiss

   added

   analytics

   area: code-intelligenceWave 5 · cluster I — Code intelligence & analyticsWave 5 · cluster I — Code intelligence & analytics

   on Sep 1, 2026
4. mrveiss

   modified the milestones: Backlog, v0.12.0 on Sep 12, 2026
5. mrveiss commented on Sep 23, 2026

   

   mrveiss

   on Sep 23, 2026

   OwnerAuthor

   More actions

   A fourth instance tonight, and it arrived from the opposite direction — which is the part that generalises this issue beyond the three consumers in the table.

   New instance (#17342). #14881 collapsed four hand-copied enums, and the natural collapse target looked like the engine that owns the vocabulary:

   # api/schemas_analytics.py
   from code_intelligence.code_review_engine import ReviewCategory, ReviewSeverity
   from code_intelligence.precommit_analyzer import CheckCategory, CheckSeverity
   

   autobot-backend/conftest.py:810 stubs the whole code_intelligence package, so those four names are MagicMock attributes under the harness. A pydantic field annotated with a MagicMock has no schema, and the import dies:

   pydantic.errors.PydanticSchemaGenerationError: Unable to generate pydantic-core schema
   for <MagicMock id='139795823827936'>
   

   42 tests failed, every one citing the same mock id — api.metrics, api.rum, api.usage, api.knowledge_vectorization and the rest, all of which merely reach schemas_analytics transitively. One leaked object, 42 red tests, and nothing wrong with the production code.

   Why this is not just a fifth row in the table. The existing entries are pre-existing consumers that were left poisoned. This one was created by a refactor that was correct by every rule we enforce: it removed duplication, it imported rather than redeclared, and the enum guard added in the same PR passes. Nothing in the repo told the author that code_intelligence is a boundary production code must not import across. The stub list is in a conftest; the import is in an API module; the two never meet until CI.

   So the rule this issue should carry, stated once:

   A module stubbed wholesale in a conftest is a dependency boundary. Production code may not import a symbol across it. The stub cannot simply be removed — code_intelligence/__init__.py carries Python-3.10-only syntax, which is why it is stubbed at all.

   The fix that works, for the next person who hits it. Move the shared vocabulary to a leaf nothing stubs, rather than trying to un-stub the engine. For #17342 that is a new autobot_shared/code_review_enums.py with the four enums and three importers. Verified: grep -rn "from code_intelligence" autobot_shared/ is empty so there is no cycle, and autobot_shared.status_enums already resolves to real enums on this exact import path today. Not status_enums.py itself — it is 559 lines against the 600 MAX_LINES ceiling.

   Same class, different stub — worth linking here rather than filing twice. #17343 is this defect with testkit/auth_middleware_stub.py:194 replacing auth_middleware wholesale: an unauthenticated-access test returned 501 instead of 401 because the stub's check_admin_permission takes no Request and refuses nothing. The naive assertion (!= 200) passes, so that one produces a green security test that never touched an auth gate.

   That is the direction worth flagging loudest. This issue's four instances all fail loudly. #17343 shows the same root cause can pass silently, and a harness stub that makes a test pass for the wrong reason is strictly more dangerous than one that makes 42 fail for the right one. conftest.py:786-802 already records a third variant in its own comments — stubbing orchestration.causal_error_recovery gave TestCausalValidator a mock whose .valid was truthy, so 2 tests failed outright and 3 silently asserted nothing.

   Three of the four known variants therefore include a silent-pass mode. Any fix here should be judged on whether it catches those, not only the import-time explosions.