docs: add improvement roadmap, research notes, and solution docs
- Add 2026-04-24 ROADMAP with 5 phases / 17 items - Add detailed implementation plans for P1-001 through P4-005 - Add research artifacts and solution docs from ledger merge - Add test for SVD component 1 compass alignment
This commit is contained in:
@@ -0,0 +1,168 @@
|
||||
---
|
||||
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)
|
||||
Reference in New Issue
Block a user