Llm Artifacts Detection

LLM Artifacts Detection

Detect and flag common patterns introduced by LLM coding agents that reduce code quality.

Detection Categories

Agent Prompts

Use these prompts to spawn focused detection agents:

Tests Agent

Analyze the test files for LLM-introduced test quality issues:

1. **DRY Violations**: Look for setup/teardown code repeated across multiple test functions instead of using fixtures or shared helpers. Flag patterns like:
   - Identical object creation in multiple tests
   - Repeated mock configurations
   - Copy-pasted database setup

2. **Library Testing**: Identify tests that validate standard library or framework behavior rather than application code. Signs:
   - No imports from the application codebase
   - Testing built-in functions or third-party library methods
   - Assertions about stdlib behavior

3. **Mock Boundaries**: Flag mocking that's too deep or too shallow:
   - Too deep: Mocking internal implementation details, private methods
   - Too shallow: Mocking at the wrong layer, missing integration points
   - Wrong level: Unit test mocks in integration tests or vice versa

For each issue found, report: [FILE:LINE] ISSUE_TITLE

Dead Code Agent

Scan the codebase for dead code and cleanup opportunities:

1. **Unused Code**: Find functions, classes, and variables with no references:
   - Functions never called
   - Classes never instantiated
   - Module-level variables never read
   - Unreachable code after returns

2. **TODO/FIXME Comments**: Flag all TODO, FIXME, HACK, XXX comments that indicate incomplete work

3. **Backwards Compat Cruft**: Look for patterns suggesting removed features:
   - Variables renamed with _unused, _old, _deprecated suffixes
   - Re-exports only for backwards compatibility
   - Comments like "# removed", "# legacy", "# deprecated"
   - Empty functions/classes kept "for compatibility"

4. **Orphaned Tests**: Tests for code that no longer exists:
   - Test files with no corresponding source
   - Test functions testing deleted features

For each issue found, report: [FILE:LINE] ISSUE_TITLE

Abstraction Agent

Review the codebase for over-engineering introduced by LLM agents:

1. **Over-Abstraction**: Identify unnecessary abstraction layers:
   - Wrapper classes that just delegate to one method
   - Interfaces/protocols with only one implementation
   - Abstract base classes with single concrete class
   - Factory functions that always return the same type

2. **Copy-Paste Drift**: Find 3+ similar code blocks that should be parameterized:
   - Nearly identical functions with minor variations
   - Repeated patterns that could be a single function with parameters
   - Similar class methods across multiple classes

3. **Over-Configuration**: Flag configuration for non-configurable things:
   - Feature flags that are never toggled
   - Environment variables always set to one value
   - Config options with no production variation
   - Overly generic code for single use case

For each issue found, report: [FILE:LINE] ISSUE_TITLE

Style Agent

Check for verbose LLM-style patterns that reduce code clarity:

1. **Obvious Comments**: Comments that restate what the code clearly does:
   - "# increment counter" above counter += 1
   - "# return the result" above return result
   - Docstrings that repeat the function name

2. **Over-Documentation**: Excessive documentation on trivial code:
   - Full docstrings on simple getters/setters
   - Parameter descriptions for obvious args
   - Return value docs for self-evident returns

3. **Defensive Overkill**: Unnecessary defensive programming:
   - try/except around code that cannot fail
   - Null checks on values that can't be null
   - Type checks after type hints guarantee the type
   - Validation of already-validated inputs

4. **Unnecessary Type Hints**: Type hints that add no value:
   - Type hints on obvious literal assignments
   - Redundant hints on variables immediately clear from context
   - Over-annotated internal/local variables

For each issue found, report: [FILE:LINE] ISSUE_TITLE

Gates (reporting)

Run these in order so findings are evidence-bound, not inferred. This is the detection-side instance of the Anti-confabulation gate in the review-verification-protocol skill: every [FILE:LINE] must be echoed from a freshly read buffer in this turn, never inferred from the branch name, directory, or memory.

1. Anchor — Set FILE and LINE from an opened buffer, read_file, or equivalent; do not rely only on stale search snippets. Pass: LINE is in range for FILE, and the described issue is visible on that line or its immediate neighbors. 2. Title — ISSUE_TITLE states the defect in plain language (about one short sentence), not a proposed fix. Pass: someone opening FILE at LINE can see why the title applies. 3. Dedup — Before final output, merge rows that share the same FILE:LINE and root cause. Pass: at most one [FILE:LINE] ISSUE_TITLE per distinct defect at that anchor.

Usage

1. Load this skill when reviewing AI-generated code 2. If the agent supports subagents, dispatch one per detection category in parallel; otherwise work through the categories sequentially yourself, producing the same [FILE:LINE] ISSUE_TITLE findings. 3. Use reference files for detailed criteria and examples 4. Apply Gates (reporting) above, then emit findings as [FILE:LINE] ISSUE_TITLE

When to Apply

• Cleaning up code written by AI coding agents
• Post-generation code review
• Reducing code bloat from iterative AI generation
• Identifying patterns that reduce maintainability

Abstraction Criteria

Detailed detection criteria for over-engineering patterns commonly introduced by LLM coding agents.

1. Over-Abstraction

What to Look For

Unnecessary abstraction layers that add complexity without providing value.

Detection Patterns

Wrapper Classes That Just Delegate:

# BAD - Wrapper adds nothing
class DatabaseWrapper:
    def __init__(self, db):
        self.db = db

    def query(self, sql):
        return self.db.query(sql)  # Just delegates!

    def execute(self, sql):
        return self.db.execute(sql)  # Just delegates!

# Usage
wrapper = DatabaseWrapper(actual_db)
wrapper.query(sql)  # Why not just use actual_db directly?

Interfaces With Single Implementation:

# BAD - Abstract class with only one implementation
from abc import ABC, abstractmethod

class DataProcessor(ABC):
    @abstractmethod
    def process(self, data): ...

class ConcreteDataProcessor(DataProcessor):  # Only implementation!
    def process(self, data):
        return data.transform()

# No other implementations exist - why the abstraction?

Protocol With One Implementer:

# BAD - Protocol nobody else implements
from typing import Protocol

class Fetcher(Protocol):
    def fetch(self, url: str) -> bytes: ...

class HttpFetcher:  # Only class implementing Fetcher
    def fetch(self, url: str) -> bytes:
        return requests.get(url).content

# The protocol adds no value if there's only one implementation

Factory That Always Returns Same Type:

# BAD - Factory with no variation
def create_processor(config):
    # Always returns the same type!
    return DataProcessor(config)

# Could just be:
processor = DataProcessor(config)

Unnecessary Indirection:

# BAD - Extra layers for no reason
class ServiceLocator:
    def get_user_service(self):
        return UserService()

class UserService:
    def get_user(self, id):
        return UserRepository().find(id)

class UserRepository:
    def find(self, id):
        return db.query(User).get(id)

# 3 layers when 1 would do

Signs of Over-Abstraction

• Class/function just calls through to another
• Abstract class with exactly one concrete implementation
• Factory that always returns the same type
• Interface defined "for future extensibility" (YAGNI violation)
• Multiple layers that all have the same method signatures

---

2. Copy-Paste Drift

What to Look For

Three or more similar code blocks that should be parameterized into a single function.

Detection Patterns

Nearly Identical Functions:

# BAD - Three similar functions
def process_users(users):
    results = []
    for user in users:
        validated = validate(user)
        transformed = transform(validated)
        results.append(transformed)
    return results

def process_orders(orders):
    results = []
    for order in orders:  # Same pattern!
        validated = validate(order)
        transformed = transform(validated)
        results.append(transformed)
    return results

def process_products(products):
    results = []
    for product in products:  # Same pattern!
        validated = validate(product)
        transformed = transform(validated)
        results.append(transformed)
    return results

# GOOD - Parameterized
def process_items(items):
    return [transform(validate(item)) for item in items]

Repeated Patterns in Methods:

# BAD - Same error handling in multiple methods
class ApiClient:
    def get_users(self):
        try:
            response = self.session.get("/users")
            response.raise_for_status()
            return response.json()
        except RequestException as e:
            logger.error(f"Failed to get users: {e}")
            raise ApiError(f"Failed to get users: {e}")

    def get_orders(self):
        try:
            response = self.session.get("/orders")  # Same pattern!
            response.raise_for_status()
            return response.json()
        except RequestException as e:
            logger.error(f"Failed to get orders: {e}")
            raise ApiError(f"Failed to get orders: {e}")

# GOOD - Extract common pattern
def _request(self, endpoint):
    try:
        response = self.session.get(endpoint)
        response.raise_for_status()
        return response.json()
    except RequestException as e:
        logger.error(f"Failed to get {endpoint}: {e}")
        raise ApiError(f"Failed to get {endpoint}: {e}")

def get_users(self):
    return self._request("/users")

Similar Class Structures:

# BAD - Multiple classes with same structure
class UserValidator:
    def validate(self, user):
        errors = []
        if not user.name:
            errors.append("name required")
        if not user.email:
            errors.append("email required")
        return errors

class OrderValidator:
    def validate(self, order):
        errors = []
        if not order.id:
            errors.append("id required")
        if not order.total:
            errors.append("total required")
        return errors

# GOOD - Generic validator
class RequiredFieldValidator:
    def __init__(self, required_fields):
        self.required_fields = required_fields

    def validate(self, obj):
        return [f"{f} required" for f in self.required_fields if not getattr(obj, f)]

How to Identify

1. Search for similar function names (get_X, process_X, validate_X) 2. Look for identical control flow with different variables 3. Check for repeated try/except patterns 4. Find similar class methods across different classes

---

3. Over-Configuration

What to Look For

Configuration and feature flags for things that don't actually vary.

Detection Patterns

Feature Flags Never Toggled:

# BAD - Flag always True
ENABLE_NEW_PARSER = True  # Never set to False anywhere

def parse(data):
    if ENABLE_NEW_PARSER:  # Always true!
        return new_parse(data)
    return old_parse(data)  # Dead code!

Environment Variables With One Value:

# BAD - Always the same value
DATABASE_POOL_SIZE = int(os.getenv("DB_POOL_SIZE", "10"))
# But DB_POOL_SIZE is never set in any environment!

# BAD - Config that doesn't vary
config = {
    "retry_count": os.getenv("RETRY_COUNT", "3"),
    "timeout": os.getenv("TIMEOUT", "30"),
}
# All environments use the defaults

Overly Generic Code for Single Use:

# BAD - Generic but only used once
class DataProcessor:
    def __init__(self,
                 input_format="json",
                 output_format="json",
                 encoding="utf-8",
                 validate=True,
                 transform=True):
        # Many options...
        pass

# Only ever called as:
processor = DataProcessor()  # All defaults, always!

Unused Configuration Options:

# config.py
class Settings:
    database_url: str
    cache_ttl: int = 3600
    max_retries: int = 3
    enable_metrics: bool = True  # Never read!
    legacy_mode: bool = False  # Never read!
    debug_sql: bool = False  # Never read!

Signs of Over-Configuration

• Config values that never change across environments
• Feature flags with only one state in production
• Options with defaults that are always used
• Configuration loaded but never accessed
• Environment variables with no variation

---

Review Questions

1. Does this abstraction have multiple implementations? 2. Are there 3+ similar code blocks that could be parameterized? 3. Is this configuration actually configured differently anywhere? 4. Would removing this layer break anything meaningful? 5. Is this factory/wrapper adding value or just indirection?

Dead Code Criteria

Detailed detection criteria for dead code and cleanup opportunities commonly left by LLM coding agents.

1. Unused Code

What to Look For

Functions, classes, and variables with no references anywhere in the codebase.

Detection Patterns

Unused Functions:

# Function defined but never called
def helper_process_data(data):  # No callers!
    """Process data helper."""
    return data.strip().lower()

def unused_validation(value):  # No callers!
    """Validate value format."""
    return bool(re.match(r"^\d+$", value))

Unused Classes:

# Class defined but never instantiated
class DataTransformer:  # Never used!
    """Transform data between formats."""
    def transform(self, data):
        return data

class LegacyProcessor:  # Never used!
    """Old processor implementation."""
    pass

Unused Variables:

# Module-level variables never read
DEFAULT_TIMEOUT = 30  # Never referenced
CACHE_SIZE = 1000  # Never referenced

# Assigned but never used
def process():
    result = compute()  # 'result' never used
    intermediate = transform()  # Never used
    return other_compute()

Unreachable Code:

def calculate(x):
    if x > 0:
        return x * 2
    return x * -1

    # Unreachable!
    logger.info("Calculation complete")
    cleanup()

How to Find

1. Use IDE "Find Usages" on suspected dead code 2. Run vulture or similar dead code detector 3. Search for function/class name across codebase 4. Check import statements for unused imports

---

2. TODO/FIXME Comments

What to Look For

Comments indicating incomplete work, technical debt, or known issues.

Detection Patterns

# TODO: implement caching  <-- Incomplete feature
def get_user(id):
    return db.query(User).get(id)

# FIXME: this breaks with unicode  <-- Known bug
def parse_name(name):
    return name.split()[0]

# HACK: temporary workaround for issue #123  <-- Tech debt
result = data.replace("\x00", "")

# XXX: this needs to be refactored  <-- Acknowledged mess
def complex_function():
    # 200 lines of spaghetti
    pass

# NOTE: remove after migration  <-- Scheduled for deletion
old_format = convert_legacy(data)

Categories

---

3. Backwards Compatibility Cruft

What to Look For

Patterns suggesting removed features kept around "just in case" or for backwards compatibility that's no longer needed.

Detection Patterns

Unused Renames:

# Variables renamed to indicate unused
_unused_config = old_config  # Why keep it?
_old_handler = legacy_handler  # Delete it!
_deprecated_cache = cache_v1  # Remove!

# Functions with "old" or "legacy" suffixes
def process_old(data):  # Is this still needed?
    pass

def validate_legacy(value):  # Who calls this?
    pass

Re-exports for Compatibility:

# In __init__.py - re-exporting moved code
from .new_location import Thing  # noqa: F401
from .new_module import OldName as OldName  # Backwards compat

# Explicit compatibility exports
__all__ = [
    "NewThing",
    "OldThing",  # Deprecated, remove in v3.0
]

Removal Comments:

# # removed - no longer used
# old_function = None

# # legacy - kept for backwards compatibility
# LegacyClass = NewClass

# # deprecated - use new_method instead
def old_method():
    return new_method()

Empty Compatibility Stubs:

class LegacyAdapter:
    """Kept for backwards compatibility."""
    pass  # Empty!

def deprecated_function(*args, **kwargs):
    """Deprecated. Use new_function instead."""
    pass  # Does nothing!

How to Evaluate

1. Check if the "legacy" code has any callers 2. Search for imports of deprecated names 3. Check if deprecation warnings are even triggered 4. Review git history - how long has it been "deprecated"?

---

4. Orphaned Tests

What to Look For

Tests that reference code that no longer exists.

Detection Patterns

Test Files Without Source:

tests/
  test_old_feature.py  # But old_feature.py doesn't exist!
  test_removed_module.py  # removed_module/ was deleted

Tests Importing Deleted Code:

# This import fails or imports from wrong place
from myapp.deleted_module import RemovedClass  # Module deleted!

def test_removed_feature():
    obj = RemovedClass()  # Class doesn't exist!
    assert obj.method() == expected

Tests for Renamed/Moved Code:

# Old test file testing moved functionality
# test_utils.py
def test_helper_function():
    from myapp.utils import helper  # Moved to myapp.helpers!
    assert helper(1) == 2

How to Find

1. Run the test suite - import errors reveal orphans 2. Check test file names against source file names 3. Review test imports for deleted modules 4. Look for skipped tests with outdated skip reasons

---

Review Questions

1. Are there functions with zero callers? 2. How old are the TODO/FIXME comments? 3. Is "deprecated" code actually deprecated (with timeline)? 4. Do all test files have corresponding source files? 5. Are there variables assigned but never read?

Style Criteria

Detailed detection criteria for verbose LLM-style patterns that reduce code clarity.

1. Obvious Comments

What to Look For

Comments that restate what the code clearly expresses.

Detection Patterns

Restating the Operation:

# BAD - Comment restates code
counter += 1  # increment counter
items.append(item)  # add item to list
return result  # return the result
user = None  # set user to None

# GOOD - No comment needed, code is clear
counter += 1
items.append(item)
return result
user = None

Describing Simple Control Flow:

# BAD - Obvious conditionals
# check if user exists
if user:
    # process the user
    process(user)
else:
    # handle missing user
    handle_error()

# GOOD - Code is self-documenting
if user:
    process(user)
else:
    handle_error()

Docstrings That Repeat the Name:

# BAD - Docstring restates function name
def get_user_by_id(id: int) -> User:
    """Get a user by their ID."""
    return db.query(User).get(id)

def validate_email(email: str) -> bool:
    """Validates the email."""
    return bool(re.match(EMAIL_REGEX, email))

# GOOD - Add value or omit
def get_user_by_id(id: int) -> User:
    """Raises UserNotFound if ID doesn't exist."""
    return db.query(User).get(id)

# Or just no docstring for trivial functions
def validate_email(email: str) -> bool:
    return bool(re.match(EMAIL_REGEX, email))

Loop Comments:

# BAD
# iterate over users
for user in users:
    # process each user
    process(user)

# GOOD
for user in users:
    process(user)

---

2. Over-Documentation

What to Look For

Excessive documentation on code that doesn't need it.

Detection Patterns

Full Docstrings on Trivial Functions:

# BAD - Overkill for simple getter
def get_name(self) -> str:
    """Get the name of this object.

    Returns:
        str: The name of the object.
    """
    return self._name

# GOOD - Simple is better
def get_name(self) -> str:
    return self._name

Parameter Descriptions for Obvious Args:

# BAD - Parameters are self-evident
def send_email(
    to: str,
    subject: str,
    body: str,
) -> None:
    """Send an email.

    Args:
        to: The email address to send to.
        subject: The subject of the email.
        body: The body of the email.
    """
    ...

# GOOD - Only document non-obvious aspects
def send_email(
    to: str,
    subject: str,
    body: str,
    priority: int = 3,
) -> None:
    """Send an email.

    Args:
        priority: 1-5, where 1 is highest. Affects delivery order.
    """
    ...

Return Value Docs for Obvious Returns:

# BAD
def is_valid(self) -> bool:
    """Check if valid.

    Returns:
        bool: True if valid, False otherwise.
    """
    return self._valid

# GOOD - Return is obvious from type hint
def is_valid(self) -> bool:
    return self._valid

---

3. Defensive Overkill

What to Look For

Unnecessary defensive programming that can't actually prevent failures.

Detection Patterns

Try/Except Around Non-Failing Code:

# BAD - These operations can't fail
try:
    x = 1 + 1
except Exception:
    x = 0

try:
    result = {"key": "value"}
except Exception:
    result = {}

# BAD - Already validated input
def process(data: ValidatedData):
    try:
        # ValidatedData guarantees these exist
        name = data.name
        email = data.email
    except AttributeError:
        raise ValueError("Invalid data")  # Can't happen!

Null Checks on Non-Nullable Values:

# BAD - Type hint says it's not None
def process(user: User) -> str:
    if user is None:  # Can't be None per type hint!
        raise ValueError("User required")
    return user.name

# BAD - Just assigned, can't be None
config = load_config()
if config is None:  # load_config() never returns None
    config = {}

Type Checks After Type Hints:

# BAD - Type is already guaranteed
def process(items: list[str]) -> None:
    if not isinstance(items, list):  # Already typed!
        raise TypeError("Expected list")
    for item in items:
        if not isinstance(item, str):  # Already typed!
            raise TypeError("Expected str")
        print(item)

Re-Validating Already-Validated Input:

# BAD - Pydantic already validated
class Request(BaseModel):
    email: EmailStr
    age: int = Field(ge=0, le=150)

def handle(request: Request):
    # Pydantic already validated these!
    if not is_valid_email(request.email):
        raise ValueError("Invalid email")
    if request.age < 0 or request.age > 150:
        raise ValueError("Invalid age")

---

4. Unnecessary Type Hints

What to Look For

Type hints that add no information value.

Detection Patterns

Type Hints on Obvious Literals:

# BAD - Type is obvious from value
name: str = "Alice"
count: int = 0
enabled: bool = True
items: list = []

# GOOD - Let inference work
name = "Alice"
count = 0
enabled = True
items: list[str] = []  # Only hint if element type matters

Redundant Hints on Clear Context:

# BAD - Context makes type obvious
user: User = User(name="Alice")
result: dict = json.loads(data)  # json.loads returns dict
items: list = list(range(10))

# GOOD
user = User(name="Alice")
result = json.loads(data)
items = list(range(10))

Over-Annotated Internal Variables:

# BAD - Too many internal annotations
def process(data: str) -> dict:
    lines: list[str] = data.split("\n")
    result: dict[str, int] = {}
    count: int = 0
    for line in lines:
        key: str = line.strip()
        result[key] = count
        count += 1
    return result

# GOOD - Annotate function signature, not internals
def process(data: str) -> dict[str, int]:
    lines = data.split("\n")
    result = {}
    for count, line in enumerate(lines):
        result[line.strip()] = count
    return result

When Type Hints Add Value

• Function parameters and return types
• Class attributes (especially in dataclasses)
• Variables where type isn't obvious from assignment
• Collection types where element type matters
• Optional/Union types

---

Review Questions

1. Does this comment tell me something the code doesn't? 2. Would a new developer need this docstring? 3. Can this exception actually be raised? 4. Is this null check protecting against a real possibility? 5. Would the code be equally clear without this type hint?

Test Quality Criteria

Detailed detection criteria for test quality issues commonly introduced by LLM coding agents.

1. DRY Violations

What to Look For

Repeated setup/teardown code across test functions instead of using fixtures, conftest, or shared helpers.

Detection Patterns

Repeated Object Creation:

# BAD - Same setup in multiple tests
def test_user_creation():
    db = Database(host="localhost", port=5432)
    user = User(name="test", email="test@example.com")
    # test logic

def test_user_update():
    db = Database(host="localhost", port=5432)  # Repeated!
    user = User(name="test", email="test@example.com")  # Repeated!
    # test logic

# GOOD - Use fixtures
@pytest.fixture
def db():
    return Database(host="localhost", port=5432)

@pytest.fixture
def test_user():
    return User(name="test", email="test@example.com")

def test_user_creation(db, test_user):
    # test logic

Repeated Mock Configuration:

# BAD - Mock setup copied across tests
def test_api_success():
    mock_response = Mock()
    mock_response.status_code = 200
    mock_response.json.return_value = {"data": "test"}
    with patch("requests.get", return_value=mock_response):
        # test

def test_api_parsing():
    mock_response = Mock()  # Repeated!
    mock_response.status_code = 200
    mock_response.json.return_value = {"data": "test"}
    with patch("requests.get", return_value=mock_response):  # Repeated!
        # test

Copy-Pasted Database Setup:

# BAD - Database initialization in every test
def test_query_users():
    engine = create_engine("sqlite:///:memory:")
    Base.metadata.create_all(engine)
    Session = sessionmaker(bind=engine)
    session = Session()
    # test

def test_query_orders():
    engine = create_engine("sqlite:///:memory:")  # Repeated!
    Base.metadata.create_all(engine)  # Repeated!
    Session = sessionmaker(bind=engine)  # Repeated!
    session = Session()
    # test

How to Fix

1. Extract to conftest.py fixtures 2. Use fixture scope appropriately (function, class, module, session) 3. Create factory fixtures for parameterized data 4. Use fixture composition for complex setups

---

2. Library Testing

What to Look For

Tests that validate standard library or framework behavior rather than application code.

Detection Patterns

No Application Imports:

# BAD - Testing Python stdlib, not our code
import json

def test_json_loads():
    result = json.loads('{"key": "value"}')
    assert result == {"key": "value"}

def test_json_dumps():
    result = json.dumps({"key": "value"})
    assert result == '{"key": "value"}'

Testing Framework Behavior:

# BAD - Testing SQLAlchemy, not our models
from sqlalchemy import Column, Integer, String

def test_column_types():
    col = Column(Integer)
    assert col.type.__class__.__name__ == "Integer"

# BAD - Testing Pydantic validation
from pydantic import BaseModel

def test_pydantic_validates():
    class M(BaseModel):
        x: int
    assert M(x=1).x == 1

Signs of Library Testing:

• Test file imports only stdlib/third-party, no from myapp import
• Tests verify documented framework behavior
• Assertions match framework documentation examples
• No domain logic being tested

How to Fix

1. Delete tests that only verify framework behavior 2. Focus on testing YOUR code that uses the framework 3. Test business logic, not library internals 4. Trust well-tested libraries

---

3. Mock Boundaries

What to Look For

Mocking at the wrong level - either too deep (internal implementation) or too shallow (missing integration points).

Too Deep: Mocking Internals

# BAD - Mocking private methods
def test_process():
    service = DataService()
    with patch.object(service, "_internal_helper"):  # Too deep!
        with patch.object(service, "_validate_internal"):  # Too deep!
            service.process(data)

# BAD - Mocking implementation details
def test_calculate():
    with patch("myapp.service._cache_lookup"):  # Internal!
        with patch("myapp.service._serialize"):  # Internal!
            result = calculate(input)

Problems with Deep Mocking:

• Tests break when refactoring internals
• Tests know too much about implementation
• False confidence - internals change, tests still pass

Too Shallow: Missing Integration Points

# BAD - Not mocking external API in unit test
def test_get_weather():
    # Actually calls the real weather API!
    result = weather_service.get_current("NYC")
    assert result.temp > 0

# BAD - Not mocking database in unit test
def test_user_service():
    # Actually hits the real database!
    user = user_service.get_by_id(1)

Problems with Shallow Mocking:

• Tests are slow (real network/DB calls)
• Tests are flaky (external dependencies)
• Can't test edge cases easily

Correct Mock Boundaries

# GOOD - Mock at integration boundaries
def test_weather_service(mock_weather_api):
    mock_weather_api.get.return_value = WeatherResponse(temp=72)
    result = weather_service.get_current("NYC")
    assert result.temp == 72

# GOOD - Mock external dependencies, not internals
def test_data_processor(mock_database, mock_external_api):
    mock_database.query.return_value = [...]
    mock_external_api.fetch.return_value = {...}
    result = processor.process()
    # Tests OUR logic with controlled inputs

Guidelines

Review Questions

1. Are private methods (_method) being mocked? 2. Are tests making real external API calls? 3. Do mock boundaries match architectural boundaries? 4. Would refactoring internals break these tests?