class PermissionValidator:
    def __init__(self):
        self.cache = {}

    def validate(self, user, resource):
        if user in self.cache:
            return self.cache[user]

        result = self._complex_validation(user, resource)
        self.cache[user] = result
        return result

    def _complex_validation(self, user, resource):
        # Complex business logic...
        pass

FILE: permissions.py (lines 10-25)

This over-engineers the permission checking with caching that may not be needed.
The caching layer adds complexity without proven benefit:

1. Cache can become stale if user permissions change
2. Unclear when/if cache should be invalidated
3. In-memory cache doesn't scale across processes
4. Permission checks are usually not in hot paths

SUGGESTION - Start simpler:
```python
def check_permission(user, resource):
    """Check if user can access resource."""
    # Direct implementation
    return user.has_access_to(resource)

undefined

FILE: .claude/tools/auth/ (new module)

Good: Module has clear responsibilities (JWT validation, token refresh).
Issue: Missing specification documentation for module regeneration.

Brick Philosophy Requirement: Modules should be regeneratable from spec.

ACTION NEEDED:

1. Create Specs/authentication.md with:
   - Clear purpose and scope
   - Public interface (exported functions)
   - Dependencies listed
   - Test requirements
   - Usage examples

This enables future rebuilding if requirements change.

See .claude/skills/pr-review-assistant/ for specification template.

def process_data(data):
    """Process data through pipeline."""
    # TODO: Add error handling
    result = validate(data)
    if not result:
        return None  # swallowed error

    # TODO: Implement caching
    return transform(result)

FILE: pipeline.py (lines 5-15)

Zero-BS Issues Found:

1. LINE 6: TODO comments - implement or remove
   - "Add error handling" - Either add it now or create issue
   - "Implement caching" - Not needed yet; add if profiling shows benefit

2. LINE 10: Swallowed error
   - validate() likely raised exception, but returns None
   - Make error explicit: raise ValueError("Invalid data")
   - Or catch and re-raise with context

3. LINE 8: Silent failures
   - If None is returned, caller doesn't know why
   - Either raise exception or return Result type

SUGGESTION:
```python
def process_data(data):
    """Process data through pipeline.

    Raises:
        ValueError: If data validation fails
    """
    result = validate(data)  # Raises if invalid
    return transform(result)

undefined

FILE: user.py (lines 50-100)

New functionality added without adequate tests.

ISSUE: RoleManager public interface not tested

- add_role(user, role) - add success case
- remove_role(user, role) - add success case
- get_roles(user) - test empty, single, multiple
- is_admin(user) - test true/false cases

EDGE CASES MISSING:

- [ ] duplicate role assignment (should be idempotent)
- [ ] removing non-existent role (should not error)
- [ ] get_roles on user with no roles (should return empty)

CONTRACT VERIFICATION:

- [ ] Functions accept documented types
- [ ] Return types match documentation
- [ ] Exceptions raised as documented

ADD: tests/test_role_manager.py with 85%+ coverage

def load_config(path):
    """Load YAML config from file.

    Args:
        path: Path to config file

    Returns:
        dict: Parsed config

    Raises:
        FileNotFoundError: If file doesn't exist
        ValueError: If YAML is invalid
    """
    with open(path) as f:
        try:
            return yaml.safe_load(f)
        except yaml.YAMLError as e:
            raise ValueError(f"Invalid YAML: {e}") from e

EXCELLENT - This is exactly the simplicity we want:

✅ Single responsibility: Load and parse YAML
✅ Clear error handling: Explicit exceptions with context
✅ No TODOs or stubs: Production ready
✅ Complete documentation: Clear args, returns, exceptions
✅ No over-engineering: Direct implementation
✅ Testable: Clear behavior to verify

This is a model example of ruthless simplicity.

**FILE**: path/to/file.py (lines X-Y)

**ISSUE**: [Principle violated - Simplicity/Modularity/Zero-BS/Tests/Docs]

**WHAT**: [Describe what's in the code]

**WHY IT'S PROBLEMATIC**: [How it violates amplihack principles]

**SUGGESTION**: [Concrete code example or approach]

**REFERENCE**: [Link to relevant philosophy, principle, or example]

gh pr comment <PR-NUMBER> -b "Review comment here"

undefined

undefined

undefined

undefined

undefined

undefined

undefined

undefined

undefined