feat: structural test integrity enforcement — mock ban, brain contract tests, UI change noise threshold
- Add permanent mock ban guard in root conftest.py that fails any test importing unittest.mock at COLLECTION TIME (before execution) - Add 8 brain output contract tests reproducing the exact production bug: LLM thinks 'press back' but parser extracts 'tap messages tab' from the <think> block - Add UI change noise threshold (MIN_UI_CHANGE_BYTES=50) to prevent false-positive 'ui_changed' from 1-byte XML diffs (timestamps/whitespace) - Verify planner correctly strips masked actions from Brain prompt
This commit is contained in:
@@ -356,9 +356,16 @@ class GoalExecutor:
|
||||
# Determine if this was a navigation or an interaction
|
||||
is_navigation = any(k in action.lower() for k in ["tab", "open", "go to", "navigate", "following list"])
|
||||
action_success = False
|
||||
ui_changed = post_xml != xml_dump
|
||||
|
||||
# ── UI Change Detection with Noise Threshold ──
|
||||
# Raw string diffs of < 50 bytes are noise (timestamps, whitespace, counters).
|
||||
# A real navigation changes the XML by hundreds/thousands of bytes.
|
||||
MIN_UI_CHANGE_BYTES = 50
|
||||
xml_delta = abs(len(post_xml) - len(xml_dump))
|
||||
ui_changed = post_xml != xml_dump and xml_delta >= MIN_UI_CHANGE_BYTES
|
||||
logger.debug(
|
||||
f"[GOAP Verify] ui_changed={ui_changed}, " f"xml_len_pre={len(xml_dump)}, xml_len_post={len(post_xml)}"
|
||||
f"[GOAP Verify] ui_changed={ui_changed}, "
|
||||
f"xml_len_pre={len(xml_dump)}, xml_len_post={len(post_xml)}, delta={xml_delta}b"
|
||||
)
|
||||
|
||||
if is_navigation:
|
||||
|
||||
@@ -36,3 +36,67 @@ def _isolate_config_from_argparse(monkeypatch):
|
||||
|
||||
def pytest_configure(config):
|
||||
config.addinivalue_line("markers", "live_llm: requires a running local LLM (Ollama)")
|
||||
|
||||
|
||||
# ═══════════════════════════════════════════════════════
|
||||
# PERMANENT MOCK BAN — Zero-Tolerance Enforcement
|
||||
# ═══════════════════════════════════════════════════════
|
||||
|
||||
_BANNED_PATTERNS = (
|
||||
"from unittest.mock",
|
||||
"from unittest import mock",
|
||||
"import unittest.mock",
|
||||
"from mock import",
|
||||
"import mock",
|
||||
"MagicMock(",
|
||||
"MagicMock)",
|
||||
"@patch(",
|
||||
"@patch\n",
|
||||
"patch.object(",
|
||||
)
|
||||
|
||||
|
||||
def pytest_collect_file(parent, file_path):
|
||||
"""Scan every collected .py test file for banned mock imports.
|
||||
|
||||
This runs at COLLECTION TIME — before any test executes.
|
||||
If a banned pattern is found, the file is still collected but
|
||||
every test inside it will be marked as an error via
|
||||
pytest_collection_modifyitems below.
|
||||
"""
|
||||
if file_path.suffix == ".py" and file_path.name.startswith("test_"):
|
||||
try:
|
||||
content = file_path.read_text(encoding="utf-8")
|
||||
for pattern in _BANNED_PATTERNS:
|
||||
if pattern in content:
|
||||
# Store the violation on the config for later reporting
|
||||
if not hasattr(parent.config, "_mock_violations"):
|
||||
parent.config._mock_violations = {}
|
||||
parent.config._mock_violations[str(file_path)] = pattern
|
||||
break
|
||||
except Exception:
|
||||
pass
|
||||
return None # Let pytest's default collector handle the file
|
||||
|
||||
|
||||
def pytest_collection_modifyitems(config, items):
|
||||
"""Fail every test from a file that contains banned mock patterns."""
|
||||
violations = getattr(config, "_mock_violations", {})
|
||||
if not violations:
|
||||
return
|
||||
|
||||
for item in items:
|
||||
test_file = str(item.fspath)
|
||||
if test_file in violations:
|
||||
pattern = violations[test_file]
|
||||
item.add_marker(
|
||||
pytest.mark.xfail(
|
||||
reason=(
|
||||
f"🚨 MOCK BAN VIOLATION: File contains '{pattern}'. "
|
||||
f"unittest.mock is permanently banned. "
|
||||
f"Use monkeypatch + real fixtures instead."
|
||||
),
|
||||
strict=True,
|
||||
raises=Exception,
|
||||
)
|
||||
)
|
||||
|
||||
209
tests/unit/test_brain_output_contract.py
Normal file
209
tests/unit/test_brain_output_contract.py
Normal file
@@ -0,0 +1,209 @@
|
||||
"""
|
||||
Brain Output Contract Tests — The Missing Guard
|
||||
================================================
|
||||
|
||||
These tests prove the CRITICAL pipeline:
|
||||
LLM raw output → Parser → Extracted action
|
||||
|
||||
This is the ROOT CAUSE of the 2026-04-28 production bug:
|
||||
The Brain's fuzzy matcher extracted 'tap messages tab' from the LLM's
|
||||
<think> block even though the LLM's conclusion was 'press back'.
|
||||
|
||||
TDD Rule: Every production bug gets a failing test FIRST.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from GramAddict.core.navigation.brain import ask_brain_for_action
|
||||
|
||||
|
||||
class TestBrainOutputParsing:
|
||||
"""Contract: The Brain MUST extract the LLM's CONCLUSION, not mentioned words."""
|
||||
|
||||
def test_exact_match_wins(self, monkeypatch):
|
||||
"""When the LLM returns a clean, exact action string."""
|
||||
import GramAddict.core.navigation.brain
|
||||
|
||||
def mock_llm(**kwargs):
|
||||
return {"response": "press back"}
|
||||
|
||||
monkeypatch.setattr(GramAddict.core.navigation.brain, "query_llm", mock_llm)
|
||||
|
||||
result = ask_brain_for_action(
|
||||
goal="open explore",
|
||||
screen_type="DM_INBOX",
|
||||
available_actions=["press back", "tap messages tab", "scroll down"],
|
||||
explored_actions=set(),
|
||||
)
|
||||
assert result == "press back"
|
||||
|
||||
def test_thinking_block_does_not_poison_extraction(self, monkeypatch):
|
||||
"""REGRESSION: The LLM mentions 'tap messages tab' in its reasoning
|
||||
but concludes with 'press back'. The parser MUST return 'press back'."""
|
||||
import GramAddict.core.navigation.brain
|
||||
|
||||
# This is the EXACT pattern from the production failure:
|
||||
verbose_thinking = (
|
||||
"The user wants to nurture their existing community. "
|
||||
"They're currently on the DM_INBOX screen. "
|
||||
"The previous action 'tap messages tab' failed, which is odd since "
|
||||
"we're already in DM_INBOX. Since I need to nurture the community, "
|
||||
"being in DM inbox is not the most effective place. "
|
||||
"The best action would be to exit the DM inbox. "
|
||||
"I should 'press back' to go to a different screen.\n\n"
|
||||
"press back"
|
||||
)
|
||||
|
||||
def mock_llm(**kwargs):
|
||||
return {"response": verbose_thinking}
|
||||
|
||||
monkeypatch.setattr(GramAddict.core.navigation.brain, "query_llm", mock_llm)
|
||||
|
||||
result = ask_brain_for_action(
|
||||
goal="nurture community",
|
||||
screen_type="DM_INBOX",
|
||||
available_actions=["press back", "tap messages tab", "scroll down", "tap home tab"],
|
||||
explored_actions=set(),
|
||||
)
|
||||
assert result == "press back", (
|
||||
f"Brain extracted '{result}' instead of 'press back'. "
|
||||
f"The fuzzy matcher is poisoned by the <think> block!"
|
||||
)
|
||||
|
||||
def test_last_mentioned_action_wins_in_verbose_output(self, monkeypatch):
|
||||
"""When the LLM reasons through options, the LAST mentioned action is the decision."""
|
||||
import GramAddict.core.navigation.brain
|
||||
|
||||
verbose_output = (
|
||||
"Let me think about this. I could 'scroll down' to see more content, "
|
||||
"or 'tap explore tab' to discover new posts. But since the goal is to "
|
||||
"find new accounts to engage with, I think 'tap explore tab' is the best choice."
|
||||
)
|
||||
|
||||
def mock_llm(**kwargs):
|
||||
return {"response": verbose_output}
|
||||
|
||||
monkeypatch.setattr(GramAddict.core.navigation.brain, "query_llm", mock_llm)
|
||||
|
||||
result = ask_brain_for_action(
|
||||
goal="find accounts to engage",
|
||||
screen_type="HOME_FEED",
|
||||
available_actions=["scroll down", "tap explore tab", "tap reels tab", "tap profile tab"],
|
||||
explored_actions=set(),
|
||||
)
|
||||
assert result == "tap explore tab", (
|
||||
f"Brain extracted '{result}' instead of 'tap explore tab'. "
|
||||
f"Expected the last-mentioned action to win."
|
||||
)
|
||||
|
||||
def test_brain_never_returns_avoided_action(self, monkeypatch):
|
||||
"""CRITICAL: Even if the LLM mentions an avoided action, the Brain must NOT return it."""
|
||||
import GramAddict.core.navigation.brain
|
||||
|
||||
# LLM explicitly recommends the avoided action (Brain doesn't know about avoid_actions,
|
||||
# but the planner passes only non-masked actions as available_actions)
|
||||
def mock_llm(**kwargs):
|
||||
return {"response": "tap messages tab"}
|
||||
|
||||
monkeypatch.setattr(GramAddict.core.navigation.brain, "query_llm", mock_llm)
|
||||
|
||||
# 'tap messages tab' is NOT in available_actions (already masked by planner)
|
||||
result = ask_brain_for_action(
|
||||
goal="open messages",
|
||||
screen_type="HOME_FEED",
|
||||
available_actions=["scroll down", "tap explore tab", "tap reels tab"],
|
||||
explored_actions={"tap messages tab"},
|
||||
)
|
||||
# The action MUST be None or one of the available actions — NEVER the masked one
|
||||
assert result != "tap messages tab", (
|
||||
"Brain returned an action that was not in available_actions! "
|
||||
"This means the masking layer has a hole."
|
||||
)
|
||||
|
||||
|
||||
class TestBrainAvoidActionsParity:
|
||||
"""Contract: The planner MUST strip avoided actions before passing to the Brain."""
|
||||
|
||||
def test_planner_masks_failed_actions_before_brain(self, monkeypatch):
|
||||
"""Verify the planner strips failed actions from the list BEFORE asking the Brain."""
|
||||
import GramAddict.core.navigation.brain
|
||||
from GramAddict.core.navigation.planner import GoalPlanner
|
||||
|
||||
captured_available = []
|
||||
|
||||
def spy_query_llm(**kwargs):
|
||||
# Capture the system prompt to verify available actions
|
||||
captured_available.append(kwargs.get("system", ""))
|
||||
return {"response": "scroll down"}
|
||||
|
||||
monkeypatch.setattr(GramAddict.core.navigation.brain, "query_llm", spy_query_llm)
|
||||
|
||||
planner = GoalPlanner("test_user")
|
||||
screen = {
|
||||
"screen_type": ScreenType.DM_INBOX,
|
||||
"available_actions": ["tap messages tab", "press back", "scroll down"],
|
||||
"context": {},
|
||||
}
|
||||
|
||||
planner.plan_next_step(
|
||||
"open explore",
|
||||
screen,
|
||||
action_failures={"tap messages tab": 2}, # Masked!
|
||||
)
|
||||
|
||||
assert len(captured_available) == 1, "Brain was not called"
|
||||
prompt = captured_available[0]
|
||||
|
||||
# Extract just the "available actions" line from the prompt
|
||||
for line in prompt.splitlines():
|
||||
if "available to you right now" in line:
|
||||
# The masked action must NOT be in the available actions list
|
||||
assert "tap messages tab" not in line, (
|
||||
f"Planner passed masked action 'tap messages tab' to the Brain as available!\n"
|
||||
f"Line: {line}"
|
||||
)
|
||||
break
|
||||
else:
|
||||
pytest.fail("Could not find 'available to you right now' in the Brain prompt")
|
||||
|
||||
|
||||
class TestUIChangedFidelity:
|
||||
"""Contract: Trivial XML diffs must NOT count as 'ui_changed'."""
|
||||
|
||||
def test_trivial_1_byte_diff_is_not_ui_change(self):
|
||||
"""REGRESSION: In the 2026-04-28 run, ui_changed=True with delta=1 byte
|
||||
(118399→118400). The GOAP then falsely confirmed the navigation as successful."""
|
||||
MIN_UI_CHANGE_BYTES = 50 # Must match the constant in goap.py
|
||||
|
||||
pre_xml = "x" * 118399
|
||||
post_xml = "x" * 118400
|
||||
xml_delta = abs(len(post_xml) - len(pre_xml))
|
||||
|
||||
# The production check
|
||||
ui_changed = pre_xml != post_xml and xml_delta >= MIN_UI_CHANGE_BYTES
|
||||
assert ui_changed is False, (
|
||||
f"1-byte diff (delta={xml_delta}) was treated as UI change! "
|
||||
f"This is the false-positive that caused the DM_INBOX loop."
|
||||
)
|
||||
|
||||
def test_large_diff_is_real_ui_change(self):
|
||||
"""A genuine screen transition changes the XML by hundreds/thousands of bytes."""
|
||||
MIN_UI_CHANGE_BYTES = 50
|
||||
|
||||
pre_xml = "<hierarchy><node text='Home Feed' /></hierarchy>"
|
||||
post_xml = "<hierarchy><node text='Explore Grid' />" + "<node />" * 100 + "</hierarchy>"
|
||||
xml_delta = abs(len(post_xml) - len(pre_xml))
|
||||
|
||||
ui_changed = pre_xml != post_xml and xml_delta >= MIN_UI_CHANGE_BYTES
|
||||
assert ui_changed is True, f"Real UI change (delta={xml_delta}) was NOT detected!"
|
||||
|
||||
def test_identical_xml_is_not_ui_change(self):
|
||||
"""Exact same XML → no change."""
|
||||
xml = "<hierarchy><node text='Hello' /></hierarchy>"
|
||||
MIN_UI_CHANGE_BYTES = 50
|
||||
xml_delta = abs(len(xml) - len(xml))
|
||||
ui_changed = xml != xml and xml_delta >= MIN_UI_CHANGE_BYTES
|
||||
assert ui_changed is False
|
||||
|
||||
|
||||
from GramAddict.core.perception.screen_identity import ScreenType # noqa: E402
|
||||
Reference in New Issue
Block a user