You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
168 lines
4.9 KiB
168 lines
4.9 KiB
---
|
|
title: Critical Anti-Patterns Discovered During Mindmodel Generation
|
|
date: 2026-04-12
|
|
category: docs/solutions/best-practices
|
|
module: stemwijzer
|
|
problem_type: best_practice
|
|
component: documentation
|
|
severity: critical
|
|
applies_when:
|
|
- When adding logging to any module
|
|
- When working with Streamlit test isolation
|
|
- When generating or updating .mindmodel/ for this project
|
|
tags: [anti-patterns, logging, streamlit, mindmodel, code-quality]
|
|
---
|
|
|
|
# Critical Anti-Patterns Discovered During Mindmodel Generation
|
|
|
|
## Context
|
|
|
|
During a comprehensive mindmodel generation session (Phase 1: 7 parallel analysis agents, Phase 2: constraint-writer assembly), several critical anti-patterns were discovered and documented in `.mindmodel/anti-patterns/anti-patterns.yaml`. This document captures the key findings for future reference.
|
|
|
|
## Guidance
|
|
|
|
### 1. Use Logging, Not Print Statements
|
|
|
|
**CRITICAL**: `api_client.py` uses `print()` instead of logging throughout (11 instances).
|
|
|
|
**Broken pattern:**
|
|
```python
|
|
# api_client.py - BAD
|
|
print(f"Fetched {len(voting_records)} voting records from API")
|
|
print(f"Error fetching motions from API: {e}") # No traceback
|
|
```
|
|
|
|
**Correct pattern:**
|
|
```python
|
|
# GOOD - use logging throughout
|
|
import logging
|
|
|
|
_logger = logging.getLogger(__name__)
|
|
|
|
def get_motions(self, ...):
|
|
try:
|
|
_logger.info("Fetched %d voting records from API", len(voting_records))
|
|
except Exception as e:
|
|
_logger.exception("Error fetching motions from API: %s", e)
|
|
return []
|
|
```
|
|
|
|
### 2. Streamlit Global State Replacement
|
|
|
|
**CRITICAL**: `explorer.py` has module-level `st = _DummySt()` which shadows Streamlit globally.
|
|
|
|
**Broken pattern:**
|
|
```python
|
|
# explorer.py - BAD
|
|
try:
|
|
import plotly.express as px
|
|
except Exception:
|
|
class _DummySt:
|
|
figure = _DummyFigure
|
|
# ...
|
|
st = _DummySt() # Global replacement - affects all imports!
|
|
```
|
|
|
|
**Correct pattern:**
|
|
```python
|
|
# GOOD - use conditional flags
|
|
try:
|
|
import plotly.express as px
|
|
import plotly.graph_objects as go
|
|
HAS_PLOTLY = True
|
|
except ImportError:
|
|
HAS_PLOTLY = False
|
|
px = None
|
|
go = None
|
|
|
|
def render_chart(data):
|
|
if not HAS_PLOTLY:
|
|
_logger.warning("Plotly not available")
|
|
return
|
|
# ... rest of chart logic
|
|
```
|
|
|
|
### 3. Logger Naming Inconsistency
|
|
|
|
**WARNING**: 33 files split between `logger = logging.getLogger(__name__)` and `_logger = logging.getLogger(__name__)`.
|
|
|
|
Files with `logger` (16): api_client.py, ai_provider.py, pipeline files, analysis files
|
|
Files with `_logger` (17): database.py, explorer.py, explorer_helpers.py
|
|
|
|
**Recommendation**: Standardize on `_logger` for module-level loggers. Update CODE_STYLE.md to explicitly state the convention.
|
|
|
|
### 4. Bare Except with Pass
|
|
|
|
**CRITICAL**: `database.py` line 47 has bare `except: pass` that catches KeyboardInterrupt, SystemExit, MemoryError.
|
|
|
|
**Broken pattern:**
|
|
```python
|
|
# database.py line 47 - BAD
|
|
try:
|
|
conn.execute("CREATE SEQUENCE IF NOT EXISTS motions_id_seq START 1")
|
|
except: # Catches EVERYTHING
|
|
pass
|
|
```
|
|
|
|
**Correct pattern:**
|
|
```python
|
|
# GOOD
|
|
try:
|
|
conn.execute("CREATE SEQUENCE IF NOT EXISTS motions_id_seq START 1")
|
|
except Exception as exc:
|
|
_logger.debug("Sequence creation skipped: %s", exc)
|
|
```
|
|
|
|
## Why This Matters
|
|
|
|
1. **Logging over Print**: Structured logging enables log aggregation, filtering by level, and includes stack traces. Print statements are invisible in production and provide no context during failures.
|
|
|
|
2. **Global State**: Module-level replacements of standard library modules cause subtle bugs where code imports work differently depending on import order.
|
|
|
|
3. **Consistency**: Mixed logger naming makes code harder to grep and grep-replace. Pick one convention and enforce it via linting.
|
|
|
|
4. **Bare Except**: Catching all exceptions including `KeyboardInterrupt` and `SystemExit` can prevent graceful shutdown and mask serious issues.
|
|
|
|
## When to Apply
|
|
|
|
- Before committing any logging changes: ensure using `_logger`, not `print()`
|
|
- When adding optional dependency handling: use flags, not global replacements
|
|
- When updating CODE_STYLE.md: add explicit logger naming convention
|
|
- When updating .mindmodel/: verify anti-patterns section is current
|
|
|
|
## Examples
|
|
|
|
### Fixing api_client.py Logging
|
|
|
|
```python
|
|
# Before (broken)
|
|
print(f"Processed {count} motions")
|
|
|
|
# After (correct)
|
|
_logger.info("Processed %d motions", count)
|
|
```
|
|
|
|
### Fixing Exception Handling
|
|
|
|
```python
|
|
# Before (broken)
|
|
try:
|
|
risky_operation()
|
|
except:
|
|
pass
|
|
|
|
# After (correct)
|
|
try:
|
|
risky_operation()
|
|
except Exception as exc:
|
|
_logger.warning("Operation failed: %s", exc)
|
|
return safe_fallback
|
|
```
|
|
|
|
## Related
|
|
|
|
- `.mindmodel/anti-patterns/anti-patterns.yaml` - Full anti-pattern documentation
|
|
- `.mindmodel/constraints/logging.yaml` - Logging conventions
|
|
- `.mindmodel/constraints/error-handling.yaml` - Error handling patterns
|
|
- `CODE_STYLE.md` - Code style guide (needs update for logger naming)
|
|
- `AGENTS.md` - Project conventions (RIGHT-wing parties on RIGHT, SVD labels = voting patterns)
|
|
|