diff --git a/GramAddict/core/goap.py b/GramAddict/core/goap.py index 80f264d..c9351d7 100644 --- a/GramAddict/core/goap.py +++ b/GramAddict/core/goap.py @@ -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: diff --git a/tests/conftest.py b/tests/conftest.py index edc4cb1..c590f0b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -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, + ) + ) diff --git a/tests/unit/test_brain_output_contract.py b/tests/unit/test_brain_output_contract.py new file mode 100644 index 0000000..7e1c162 --- /dev/null +++ b/tests/unit/test_brain_output_contract.py @@ -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 + 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 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 = "" + post_xml = "" + "" * 100 + "" + 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 = "" + 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