chore: convert mindmodel from YAML to markdown and clean up
Delete 17 malformed YAML constraint files and 10 stale numbered constraint files. Convert domain glossary, patterns, stack, and anti-patterns to markdown format. Update manifest.yaml to reference new markdown files.
This commit is contained in:
@@ -1,34 +0,0 @@
|
||||
# Naming & Style Conventions
|
||||
|
||||
## Rules
|
||||
- Modules and files: snake_case.py. Evidence: pipeline/run_pipeline.py, database.py, ai_provider.py
|
||||
- Functions and methods: snake_case. Evidence: compute_svd_for_window (pipeline), _generate_windows (pipeline/run_pipeline.py)
|
||||
- Classes: PascalCase. Evidence: MotionDatabase (database.py)
|
||||
- Constants: UPPER_SNAKE_CASE. Evidence: VOTE_MAP, DATABASE_PATH (config inferred)
|
||||
- Imports order: stdlib, third-party, local; prefer absolute imports and grouped.
|
||||
- Use black, ruff, isort, mypy as the recommended toolchain; repository lacks config files (black, ruff, pyproject sections).
|
||||
|
||||
## Examples
|
||||
|
||||
### Function example (from pipeline/run_pipeline.py)
|
||||
```python
|
||||
def _generate_windows(start: date, end: date, granularity: str) -> List[Tuple[str, str, str]]:
|
||||
"""Return list of (window_id, start_str, end_str) tuples."""
|
||||
```
|
||||
|
||||
### Class example (from database.py)
|
||||
```python
|
||||
class MotionDatabase:
|
||||
def __init__(self, db_path: str = config.DATABASE_PATH):
|
||||
...
|
||||
```
|
||||
|
||||
## Anti-patterns
|
||||
- Missing formatting configs (black, ruff, isort). Add pyproject.toml sections or dedicated config files.
|
||||
|
||||
## Remediations
|
||||
- Add pyproject.toml tool sections for black/ruff/isort and a pre-commit config. Run ruff/black CI lint step.
|
||||
|
||||
## Evidence pointers
|
||||
- pipeline/run_pipeline.py: function _generate_windows (lines ~1-120)
|
||||
- database.py: MotionDatabase class and methods (file database.py lines 1-400+)
|
||||
@@ -1,74 +0,0 @@
|
||||
# Database Schema (DuckDB) — extracted DDL
|
||||
|
||||
## Rules
|
||||
- Use DuckDB for persistent storage when available; fallback to JSON files when duckdb is not installed (database.py).
|
||||
- Keep schema migrations additive (ALTER TABLE ADD COLUMN IF NOT EXISTS used in database.py).
|
||||
|
||||
## Examples (DDL snippets extracted from database.py)
|
||||
|
||||
### motions table
|
||||
```sql
|
||||
CREATE TABLE IF NOT EXISTS motions (
|
||||
id INTEGER DEFAULT nextval('motions_id_seq'),
|
||||
title TEXT NOT NULL,
|
||||
description TEXT,
|
||||
date DATE,
|
||||
policy_area TEXT,
|
||||
voting_results JSON,
|
||||
winning_margin FLOAT,
|
||||
controversy_score FLOAT,
|
||||
layman_explanation TEXT,
|
||||
externe_identifier TEXT,
|
||||
body_text TEXT,
|
||||
url TEXT UNIQUE,
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
PRIMARY KEY (id)
|
||||
)
|
||||
```
|
||||
|
||||
### mp_votes table
|
||||
```sql
|
||||
CREATE TABLE IF NOT EXISTS mp_votes (
|
||||
id INTEGER DEFAULT nextval('mp_votes_id_seq'),
|
||||
motion_id INTEGER NOT NULL,
|
||||
mp_name TEXT NOT NULL,
|
||||
party TEXT,
|
||||
vote TEXT NOT NULL,
|
||||
date DATE,
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
PRIMARY KEY (id)
|
||||
)
|
||||
```
|
||||
|
||||
### embeddings / fused_embeddings
|
||||
```sql
|
||||
CREATE TABLE IF NOT EXISTS embeddings (
|
||||
id INTEGER DEFAULT nextval('embeddings_id_seq'),
|
||||
motion_id INTEGER NOT NULL,
|
||||
model TEXT,
|
||||
vector JSON NOT NULL,
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
PRIMARY KEY (id)
|
||||
)
|
||||
|
||||
CREATE TABLE IF NOT EXISTS fused_embeddings (
|
||||
id INTEGER DEFAULT nextval('fused_embeddings_id_seq'),
|
||||
motion_id INTEGER NOT NULL,
|
||||
window_id TEXT NOT NULL,
|
||||
vector JSON NOT NULL,
|
||||
svd_dims INTEGER NOT NULL,
|
||||
text_dims INTEGER NOT NULL,
|
||||
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
|
||||
PRIMARY KEY (id)
|
||||
)
|
||||
```
|
||||
|
||||
## Anti-patterns
|
||||
- Broad try/except around duckdb import (database.py top) — acceptable for optional dependency but should log explicitly the missing dependency and document test behavior.
|
||||
|
||||
## Remediations
|
||||
- Add a simple migration/versioning table (schema_version) to track schema changes and apply migrations deterministically.
|
||||
- Add tests that exercise both duckdb-backed and JSON-fallback database paths. Evidence: database.py contains JSON fallback logic (lines ~1-80).
|
||||
|
||||
## Evidence pointers
|
||||
- database.py: DDL strings and sequences (file: database.py lines ~1-300 and further). See create table blocks for motions, mp_votes, embeddings, fused_embeddings.
|
||||
@@ -1,22 +0,0 @@
|
||||
# Domain Glossary
|
||||
|
||||
## Rules
|
||||
- Use consistent domain terms across code and DB: Motion, MP, Party, embedding, window, svd_vector, fused_embedding, similarity_cache, session_id.
|
||||
|
||||
## Terms
|
||||
- Motion: parliamentary motion stored in `motions` table. Evidence: database.py CREATE TABLE motions (file: database.py lines ~40-110)
|
||||
- MP (Member of Parliament): individual with votes stored in `mp_votes`. Evidence: database.py CREATE TABLE mp_votes
|
||||
- Embedding: text embedding stored in `embeddings` table; fused vectors in `fused_embeddings`.
|
||||
- SVD vector: reduced-dimensional vectors stored in `svd_vectors` table.
|
||||
- Window: time window identifier (e.g., "2024-Q1") used across SVD/fusion pipelines. Evidence: pipeline/run_pipeline.py _generate_windows
|
||||
- Controversy score: derived field stored on motions as controversy_score. Evidence: database.py insert_motion sets controversy_score
|
||||
|
||||
## Examples / Usage
|
||||
- pipeline.run_pipeline._generate_windows produces window ids used when storing svd_vectors and fused_embeddings. Evidence: pipeline/run_pipeline.py lines ~1-120
|
||||
|
||||
## Evidence pointers
|
||||
- database.py: motions, mp_votes, embeddings, fused_embeddings tables (file: database.py)
|
||||
- pipeline/run_pipeline.py: window generation and pipeline phases (file: pipeline/run_pipeline.py)
|
||||
|
||||
## Anti-patterns
|
||||
- Inconsistent naming of domain terms across modules (e.g., `mp_vote_parties` vs `mp_votes` usage in database.insert_motion and pipeline extraction). Prefer canonical names matching DB columns and use small adapter functions when transitioning representations.
|
||||
@@ -1,30 +0,0 @@
|
||||
# Code Clusters / Organization
|
||||
|
||||
## Rules
|
||||
- The repository organizes code into the following clusters (observed):
|
||||
- UI / Streamlit: Home.py, pages/, app.py, explorer.py
|
||||
- Database & persistence: database.py, config.py
|
||||
- ETL / pipeline: pipeline/ (run_pipeline.py, svd_pipeline, text_pipeline, fusion)
|
||||
- AI provider & summarization: ai_provider.py, pipeline/..., analysis/
|
||||
- Similarity & caching: similarity/*, similarity_cache table in DB
|
||||
- API client & scraping: api_client.py, pipeline/fetch_mp_metadata
|
||||
- Analysis & visualization: analysis/visualize.py, explorer.py
|
||||
- CLI & scheduler: scheduler.py, pipeline/run_pipeline.py
|
||||
- Tests & migrations: tests/ (pytest) and database reset helpers
|
||||
|
||||
## Examples
|
||||
|
||||
### Pipeline orchestrator (cluster: CLI & pipeline)
|
||||
```python
|
||||
from database import MotionDatabase
|
||||
db = MotionDatabase(db_path)
|
||||
# then phases: fetch_mp_metadata, extract_mp_votes, compute svd, ensure_text_embeddings, fuse_for_window
|
||||
```
|
||||
|
||||
## Remediations
|
||||
- Add a brief CONTRIBUTING.md describing where to add new pipeline stages and how to run tests locally. Include notes about optional duckdb dependency and JSON fallback for tests.
|
||||
|
||||
## Evidence pointers
|
||||
- pipeline/run_pipeline.py: orchestrator and cluster boundaries (file: pipeline/run_pipeline.py)
|
||||
- ai_provider.py: AI adapter for embeddings and chat (file: ai_provider.py)
|
||||
- analysis/visualize.py: visualization cluster (file: analysis/visualize.py)
|
||||
@@ -1,46 +0,0 @@
|
||||
# Design Patterns & Code Patterns
|
||||
|
||||
## Rules
|
||||
- Use repository-style DB wrapper: MotionDatabase encapsulates DuckDB access and schema management.
|
||||
- AI provider adapter pattern: ai_provider.py exposes get_embedding(s) and chat_completion with retry/backoff and local fallback.
|
||||
- Pipeline orchestration: run_pipeline.py uses phases, ThreadPoolExecutor for parallel SVD computation with careful DuckDB connection handling (collect results before writes).
|
||||
|
||||
## Examples
|
||||
|
||||
### Repository pattern (database.py MotionDatabase)
|
||||
```python
|
||||
class MotionDatabase:
|
||||
def __init__(self, db_path: str = config.DATABASE_PATH):
|
||||
self.db_path = db_path
|
||||
self._init_database()
|
||||
|
||||
def insert_motion(self, motion_data: Dict) -> bool:
|
||||
"""Insert a new motion into database"""
|
||||
# uses duckdb.connect and parameterized queries
|
||||
```
|
||||
|
||||
### Provider adapter with retries (ai_provider.py)
|
||||
```python
|
||||
def _post_with_retries(path: str, json: dict[str, Any], retries: int = 3) -> requests.Response:
|
||||
# Implements retries/backoff, handles 429 with Retry-After and 5xx responses
|
||||
```
|
||||
|
||||
### Pipeline parallelism pattern (run_pipeline)
|
||||
```python
|
||||
with ThreadPoolExecutor(max_workers=max_workers) as pool:
|
||||
for window_id, w_start, w_end in windows:
|
||||
fut = pool.submit(compute_svd_for_window, db.db_path, window_id, w_start, w_end, args.svd_k)
|
||||
futures[fut] = window_id
|
||||
# wait then write sequentially to DuckDB
|
||||
```
|
||||
|
||||
## Anti-patterns
|
||||
- Broad excepts used in several places (database.py top-level try/except on duckdb import, many generic excepts around DB operations) — can hide real errors.
|
||||
|
||||
## Remediations
|
||||
- Replace broad except Exception with targeted exceptions and explicit logging. Where fallback is intended (e.g., optional duckdb), log at INFO/DEBUG with clear message and include guidance in CONTRIBUTING.md.
|
||||
|
||||
## Evidence pointers
|
||||
- ai_provider.py: _post_with_retries, get_embedding(s), _local_embedding (file: ai_provider.py lines ~1-300)
|
||||
- pipeline/run_pipeline.py: ThreadPoolExecutor usage and duckdb connection handling (file: pipeline/run_pipeline.py lines ~120-260)
|
||||
- database.py: MotionDatabase methods (file: database.py)
|
||||
@@ -1,24 +0,0 @@
|
||||
# Anti-patterns, Issues and Recommended Fixes
|
||||
|
||||
## Rules
|
||||
- Flagged issues discovered in Phase 1 must be remediated with concrete actions.
|
||||
|
||||
## Issues
|
||||
- pytest is listed as a runtime dependency (pyproject.toml). This increases image size and may pull dev-only transitive deps into production. Evidence: pyproject.toml
|
||||
- openai is declared but static imports not found; may be unused. Evidence: pyproject.toml, ai_provider.py uses requests and env keys instead of openai imports.
|
||||
- Many dependencies use permissive ">=" version ranges; no lockfile present. This reduces reproducibility.
|
||||
- Missing formatting/linting configs (black, ruff, isort, mypy). Recommended to add config and CI steps.
|
||||
- Broad except Exception used in many places (database.py, ai_provider.py fallback logic, analysis/visualize.py). This can mask bugs and slow debugging.
|
||||
|
||||
## Remediations / Recommended fixes
|
||||
- Move pytest from runtime dependencies to dev-dependencies in pyproject.toml.
|
||||
- Suggested patch: under [project.optional-dependencies] or [tool.poetry.dev-dependencies] depending on toolchain.
|
||||
- Audit `openai` usage. If unused, remove from pyproject.toml. If dynamically imported in runtime, add a small shim or explicit lazy import with documented env var.
|
||||
- Pin critical dependencies or add upper bounds; generate lockfile (poetry.lock or pip-tools requirements.txt). Add CI job that fails on permissive ranges.
|
||||
- Add black/ruff/isort/mypy config blocks to pyproject.toml and enable pre-commit hooks. Add CI lint stage.
|
||||
- Replace broad except Exception with narrower catches and re-raise or log with traceback when unexpected. Example locations: database.py top import, insert_motion broad except, ai_provider fallback blocks.
|
||||
|
||||
## Evidence pointers
|
||||
- pyproject.toml: dependencies list (file: pyproject.toml lines 1-40)
|
||||
- database.py: multiple broad except blocks (file: database.py top and methods)
|
||||
- ai_provider.py: uses requests + env keys (file: ai_provider.py)
|
||||
@@ -1,117 +0,0 @@
|
||||
# Example Extractions
|
||||
|
||||
## Rules
|
||||
- Include concrete examples extracted from the codebase: function signatures with docstrings, SQL DDL snippets, and pytest stubs following repository conventions.
|
||||
|
||||
## (a) Function signatures with docstrings (5 examples)
|
||||
1) pipeline/run_pipeline.py::_generate_windows
|
||||
```python
|
||||
def _generate_windows(start: date, end: date, granularity: str) -> List[Tuple[str, str, str]]:
|
||||
"""Return list of (window_id, start_str, end_str) tuples.
|
||||
|
||||
window_id format:
|
||||
quarterly → "2024-Q1", "2024-Q2", …
|
||||
annual → "2024"
|
||||
"""
|
||||
```
|
||||
|
||||
2) database.py::append_audit_event
|
||||
```python
|
||||
def append_audit_event(
|
||||
self,
|
||||
actor_id: Optional[str],
|
||||
action: str,
|
||||
target_type: Optional[str] = None,
|
||||
target_id: Optional[str] = None,
|
||||
metadata: Optional[Dict] = None,
|
||||
) -> bool:
|
||||
"""Record an audit event. Tries DB then falls back to ledger file."""
|
||||
```
|
||||
|
||||
3) ai_provider.py::get_embedding
|
||||
```python
|
||||
def get_embedding(text: str, model: str | None = None) -> list[float]:
|
||||
"""Return an embedding vector for `text` using the configured provider.
|
||||
|
||||
Raises ProviderError for configuration or provider-side failures.
|
||||
"""
|
||||
```
|
||||
|
||||
4) ai_provider.py::get_embeddings_batch
|
||||
```python
|
||||
def get_embeddings_batch(
|
||||
texts: list[str], model: str | None = None, batch_size: int = 50
|
||||
) -> list[list[float]]:
|
||||
"""Return embedding vectors for multiple texts using batched API calls."""
|
||||
```
|
||||
|
||||
5) analysis/visualize.py::plot_umap_scatter
|
||||
```python
|
||||
def plot_umap_scatter(
|
||||
motion_ids: List[int],
|
||||
coords: List[List[float]],
|
||||
labels: Optional[List[int]] = None,
|
||||
window_id: Optional[str] = None,
|
||||
output_path: str = "analysis_umap.html",
|
||||
) -> str:
|
||||
"""Produce a 2D scatter plot of UMAP-reduced fused embeddings."""
|
||||
```
|
||||
|
||||
## (b) SQL / DDL snippets (3 examples inferred from database.py)
|
||||
1) motions table (see constraints/10-db-schema.yaml) — evidence: database.py CREATE TABLE motions (lines ~40-110)
|
||||
|
||||
2) mp_votes table (see constraints/10-db-schema.yaml) — evidence: database.py CREATE TABLE mp_votes
|
||||
|
||||
3) fused_embeddings table (see constraints/10-db-schema.yaml) — evidence: database.py CREATE TABLE fused_embeddings
|
||||
|
||||
## (c) Pytest stubs (4 sample tests matching conventions)
|
||||
Create tests under tests/ named test_*.py using fixtures in conftest.py. Examples below are stubs to add.
|
||||
|
||||
1) tests/test_database_basic.py
|
||||
```python
|
||||
def test_init_database_creates_tables(tmp_path):
|
||||
db_path = str(tmp_path / "motions.db")
|
||||
from database import MotionDatabase
|
||||
|
||||
db = MotionDatabase(db_path=db_path)
|
||||
# If duckdb not available, JSON fallback should create .embeddings.json
|
||||
assert db is not None
|
||||
```
|
||||
|
||||
2) tests/test_ai_provider.py
|
||||
```python
|
||||
def test_local_embedding_fallback():
|
||||
from ai_provider import _local_embedding
|
||||
|
||||
v = _local_embedding("hello world", dim=16)
|
||||
assert isinstance(v, list) and len(v) == 16
|
||||
```
|
||||
|
||||
3) tests/test_pipeline_windows.py
|
||||
```python
|
||||
from pipeline.run_pipeline import _generate_windows
|
||||
|
||||
def test_generate_quarterly_windows():
|
||||
from datetime import date
|
||||
|
||||
start = date(2024, 1, 1)
|
||||
end = date(2024, 3, 31)
|
||||
windows = _generate_windows(start, end, "quarterly")
|
||||
assert any(w[0].endswith("Q1") for w in windows)
|
||||
```
|
||||
|
||||
4) tests/test_visualize_plot.py
|
||||
```python
|
||||
def test_plot_umap_scatter_no_plotly(monkeypatch, tmp_path):
|
||||
# If plotly missing, function should raise ImportError with guidance
|
||||
import analysis.visualize as vis
|
||||
|
||||
try:
|
||||
vis._require_plotly()
|
||||
except ImportError:
|
||||
assert True
|
||||
```
|
||||
|
||||
## Evidence pointers
|
||||
- Function docstrings: pipeline/run_pipeline.py, ai_provider.py, analysis/visualize.py, database.py
|
||||
- DDL: database.py create table blocks
|
||||
@@ -1,43 +0,0 @@
|
||||
# Stack and Dependencies
|
||||
|
||||
## Rules
|
||||
- Primary language: Python >=3.13 (evidence: pyproject.toml requires-python = ">=3.13")
|
||||
- Application: Streamlit app (streamlit >=1.48.0). Entrypoint: Home.py (CMD: streamlit run Home.py). Evidence: Home.py, pages/1_Stemwijzer.py, pyproject.toml, Dockerfile
|
||||
- Database: DuckDB + Ibis (duckdb>=1.3.2, ibis-framework[duckdb]>=10.8.0). Evidence: pyproject.toml, database.py
|
||||
- ML: scikit-learn, umap-learn, scipy. Evidence: pyproject.toml, pipeline/svd.py, analysis/
|
||||
|
||||
## Examples
|
||||
|
||||
### pyproject dependencies (evidence: pyproject.toml)
|
||||
```toml
|
||||
dependencies = [
|
||||
"duckdb>=1.3.2",
|
||||
"ibis-framework[duckdb]>=10.8.0",
|
||||
"openai>=1.99.7",
|
||||
"scipy>=1.11",
|
||||
"umap-learn>=0.5",
|
||||
"plotly>=5.0",
|
||||
"pytest>=9.0.2",
|
||||
"requests>=2.32.4",
|
||||
"schedule>=1.2.2",
|
||||
"streamlit>=1.48.0",
|
||||
"scikit-learn>=1.8.0",
|
||||
"beautifulsoup4>=4.14.3",
|
||||
"lxml>=6.0.2",
|
||||
]
|
||||
```
|
||||
|
||||
## Anti-patterns / Notes
|
||||
- pytest is listed under runtime dependencies in pyproject.toml (line: dependencies). Move pytest to dev-dependencies to avoid shipping test runner in production images. Evidence: pyproject.toml
|
||||
- Many dependencies use permissive ">=" ranges. Recommend pinning or generating lockfile (poetry.lock/requirements.txt) and adding upper bounds for reproducibility.
|
||||
- openai appears declared but static imports not found; possible unused dependency (evidence: pyproject.toml, ai_provider.py uses requests and environment keys instead of openai).
|
||||
|
||||
## Remediations
|
||||
- Move test-only libs (pytest) to dev-dependencies in pyproject.toml.
|
||||
- Add lockfile and CI step to check for pinned dependencies.
|
||||
- Audit declared but unused packages (openai) and remove or confirm dynamic usage.
|
||||
|
||||
## Evidence pointers
|
||||
- pyproject.toml: full dependency list (lines 1-40)
|
||||
- Home.py: streamlit usage and app entry (file: Home.py)
|
||||
- database.py: duckdb table creation and connection (file: database.py lines ~1-350)
|
||||
@@ -1,29 +0,0 @@
|
||||
# DB connection handling constraints
|
||||
|
||||
rules:
|
||||
- name: use_context_managers_for_connections
|
||||
rule: "Prefer using 'with duckdb.connect(path, read_only=...) as conn' for scoped DB interactions where possible."
|
||||
rationale: "Ensures proper resource cleanup and avoids connection leaks."
|
||||
|
||||
- name: read_only_for_compute
|
||||
rule: "Use read_only=True for compute steps that only read data (SVD, similarity compute)."
|
||||
rationale: "Allows safe parallel workers and reduces write contention."
|
||||
|
||||
- name: short_lived_writes
|
||||
rule: "When performing database writes, open short-lived connections, commit quickly and close."
|
||||
rationale: "Avoids long-lived transactions and reduces lock windows."
|
||||
|
||||
examples:
|
||||
- path: pipeline/svd_pipeline.py
|
||||
snippet: |
|
||||
conn = duckdb.connect(db_path, read_only=True)
|
||||
try:
|
||||
rows = conn.execute(...).fetchall()
|
||||
finally:
|
||||
conn.close()
|
||||
|
||||
anti_patterns_and_remediations:
|
||||
- bad: "Creating a global connection at import that performs migrations."
|
||||
remediation: "Move migrations to an explicit init function that runs at deployment/upgrade time."
|
||||
- bad: "Not closing connections on exceptions."
|
||||
remediation: "Wrap connects in `with` or finally: conn.close() blocks."
|
||||
@@ -0,0 +1,143 @@
|
||||
---
|
||||
title: Error Handling Patterns
|
||||
category: constraints
|
||||
severity: high
|
||||
---
|
||||
|
||||
# Error Handling Patterns
|
||||
|
||||
## Core Rules
|
||||
|
||||
1. **Catch `Exception`, return safe fallbacks** (False/[]/None)
|
||||
2. **Log exceptions with traceback** using `_logger.exception()`
|
||||
3. **Never swallow exceptions silently** - always log or return sensible default
|
||||
4. **Avoid nested try/except blocks** - flatten exception handling
|
||||
|
||||
## Pattern: Try/Except Safe Fallback
|
||||
|
||||
This is the dominant pattern in the codebase (219+ instances).
|
||||
|
||||
```python
|
||||
# Standard pattern from database.py, api_client.py, etc.
|
||||
try:
|
||||
result = risky_operation()
|
||||
return process(result)
|
||||
except Exception as exc:
|
||||
_logger.warning("Operation failed: %s", exc)
|
||||
return safe_fallback # False, [], None, {}
|
||||
```
|
||||
|
||||
### Examples from Codebase
|
||||
|
||||
**database.py** - DuckDB operations:
|
||||
```python
|
||||
def get_svd_vectors(self, window: str):
|
||||
try:
|
||||
conn = duckdb.connect(self.db_path, read_only=True)
|
||||
try:
|
||||
result = conn.execute(query, (window,)).fetchall()
|
||||
return self._parse_vectors(result)
|
||||
finally:
|
||||
conn.close()
|
||||
except Exception as exc:
|
||||
_logger.warning("Failed to get SVD vectors: %s", exc)
|
||||
return []
|
||||
```
|
||||
|
||||
**ai_provider.py** - HTTP retries:
|
||||
```python
|
||||
try:
|
||||
resp = requests.post(url, json=json, headers=headers, timeout=10)
|
||||
resp.raise_for_status()
|
||||
return resp.json()
|
||||
except requests.ConnectionError as exc:
|
||||
if attempt == retries:
|
||||
raise ProviderError(f"Connection error: {exc}") from exc
|
||||
# ... retry logic
|
||||
```
|
||||
|
||||
## Pattern: Optional Dependency Fallback
|
||||
|
||||
Gracefully degrade when optional packages are unavailable.
|
||||
|
||||
```python
|
||||
# UMAP fallback in explorer_helpers.py
|
||||
try:
|
||||
import umap
|
||||
HAS_UMAP = True
|
||||
except ImportError:
|
||||
HAS_UMAP = False
|
||||
_logger.debug("UMAP not available, using SVD vectors directly")
|
||||
|
||||
def project_to_2d(vectors):
|
||||
if HAS_UMAP:
|
||||
return umap.UMAP().fit_transform(vectors)
|
||||
return vectors[:, :2] # Fallback: first 2 SVD dimensions
|
||||
```
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### 1. Bare except with pass (CRITICAL)
|
||||
**File**: `database.py`, line 47
|
||||
|
||||
```python
|
||||
# BAD - catches KeyboardInterrupt, SystemExit, MemoryError
|
||||
try:
|
||||
conn.execute("CREATE SEQUENCE IF NOT EXISTS motions_id_seq START 1")
|
||||
except: # bare except
|
||||
pass
|
||||
```
|
||||
|
||||
**Fix**: Catch specific exception or log and continue:
|
||||
```python
|
||||
try:
|
||||
conn.execute("CREATE SEQUENCE IF NOT EXISTS motions_id_seq START 1")
|
||||
except Exception as exc:
|
||||
_logger.debug("Sequence creation skipped (may already exist): %s", exc)
|
||||
```
|
||||
|
||||
### 2. Nested Exception Handling
|
||||
**File**: `explorer.py`, lines 244-261
|
||||
|
||||
```python
|
||||
# BAD - opaque error paths
|
||||
try:
|
||||
result = compute_svd(motions)
|
||||
except Exception:
|
||||
try:
|
||||
result = fallback_compute(motions)
|
||||
except Exception:
|
||||
pass # Both exceptions silently dropped
|
||||
```
|
||||
|
||||
**Fix**: Flatten and handle each case explicitly:
|
||||
```python
|
||||
# GOOD - explicit handling
|
||||
try:
|
||||
result = compute_svd(motions)
|
||||
except Exception as exc:
|
||||
_logger.warning("SVD failed, trying fallback: %s", exc)
|
||||
try:
|
||||
result = fallback_compute(motions)
|
||||
except Exception as fallback_exc:
|
||||
_logger.error("Both SVD approaches failed: %s, %s", exc, fallback_exc)
|
||||
raise
|
||||
```
|
||||
|
||||
## Rule Summary
|
||||
|
||||
| Pattern | When to Use | Return Value |
|
||||
|---------|-------------|--------------|
|
||||
| Safe fallback | Best-effort operations | `[]`, `{}`, `False`, `None` |
|
||||
| Re-raise | Critical operations that must succeed | raise |
|
||||
| Log and continue | Optional steps in pipeline | (continue) |
|
||||
| Graceful degradation | Optional dependencies | Default behavior |
|
||||
|
||||
## When to Log vs Return
|
||||
|
||||
| Scenario | Action |
|
||||
|----------|--------|
|
||||
| User action fails | Log warning, return safe default |
|
||||
| Internal error (corrupt data) | Log error, return safe default |
|
||||
| Transient failure (network) | Log warning, retry if appropriate |
|
||||
| Configuration error | Log error, raise with clear message |
|
||||
@@ -1,184 +0,0 @@
|
||||
# Error Handling Constraints
|
||||
|
||||
## Core Rule
|
||||
|
||||
**Catch `Exception`, return safe fallbacks (False/[]/None)**
|
||||
|
||||
Never let exceptions propagate to user-facing code. Always provide a safe default.
|
||||
|
||||
## Patterns
|
||||
|
||||
### For Not-Found Operations
|
||||
|
||||
Return `None` or falsy value when item not found:
|
||||
|
||||
```python
|
||||
# GOOD: Return None on not found
|
||||
def get_motion_by_id(self, motion_id: int) -> Optional[Dict]:
|
||||
try:
|
||||
conn = duckdb.connect(self.db_path)
|
||||
result = conn.execute(
|
||||
"SELECT * FROM motions WHERE id = ?", (motion_id,)
|
||||
).fetchone()
|
||||
conn.close()
|
||||
return result
|
||||
except Exception:
|
||||
conn.close()
|
||||
return None
|
||||
```
|
||||
|
||||
### For Collection Operations
|
||||
|
||||
Return empty list when no results:
|
||||
|
||||
```python
|
||||
# GOOD: Return empty list on failure
|
||||
def get_filtered_motions(self, **kwargs) -> List[Dict]:
|
||||
try:
|
||||
conn = duckdb.connect(self.db_path)
|
||||
rows = conn.execute(query, params).fetchall()
|
||||
conn.close()
|
||||
return rows
|
||||
except Exception:
|
||||
conn.close()
|
||||
return []
|
||||
```
|
||||
|
||||
### For Boolean Operations
|
||||
|
||||
Return `False` for failed boolean checks:
|
||||
|
||||
```python
|
||||
# GOOD: Return False on failure
|
||||
def motion_exists(self, motion_id: int) -> bool:
|
||||
try:
|
||||
conn = duckdb.connect(self.db_path)
|
||||
count = conn.execute(
|
||||
"SELECT COUNT(*) FROM motions WHERE id = ?", (motion_id,)
|
||||
).fetchone()[0]
|
||||
conn.close()
|
||||
return count > 0
|
||||
except Exception:
|
||||
return False
|
||||
```
|
||||
|
||||
### For Creation Operations
|
||||
|
||||
Return `False` or empty string on failure:
|
||||
|
||||
```python
|
||||
# GOOD: Return empty string on failure
|
||||
def generate_summary(self, title: str, body: str) -> str:
|
||||
try:
|
||||
return ai_provider.chat_completion(messages)
|
||||
except ai_provider.ProviderError:
|
||||
logger.exception("AI provider failed")
|
||||
return ""
|
||||
```
|
||||
|
||||
## Anti-Patterns to Avoid
|
||||
|
||||
### Don't Catch Specific Exceptions Only
|
||||
```python
|
||||
# BAD: Catches only FileNotFoundError, misses other issues
|
||||
try:
|
||||
with open(path) as f:
|
||||
return json.load(f)
|
||||
except FileNotFoundError:
|
||||
return None
|
||||
```
|
||||
|
||||
### Don't Re-raise Without Context
|
||||
```python
|
||||
# BAD: Loses information
|
||||
try:
|
||||
process(data)
|
||||
except Exception:
|
||||
raise # No context added
|
||||
```
|
||||
|
||||
### Don't Swallow Exceptions Silently
|
||||
```python
|
||||
# BAD: No logging, no fallback
|
||||
try:
|
||||
return risky_operation()
|
||||
except Exception:
|
||||
pass # What happened?
|
||||
```
|
||||
|
||||
## Nested Exception Handling
|
||||
|
||||
When calling code that has its own error handling, wrap only if needed:
|
||||
|
||||
```python
|
||||
# Accept result from wrapped function (it handles errors)
|
||||
def fetch_motions(self, start_date):
|
||||
# ai_provider_wrapper handles retries internally
|
||||
embeddings = get_embeddings_with_retry(texts)
|
||||
|
||||
# Only wrap if wrapper doesn't handle errors
|
||||
if all(e is None for e in embeddings):
|
||||
logger.error("All embeddings failed")
|
||||
return []
|
||||
|
||||
return process(embeddings)
|
||||
```
|
||||
|
||||
## Context Managers
|
||||
|
||||
Use `try/finally` for cleanup:
|
||||
|
||||
```python
|
||||
def process_with_temp_file(self):
|
||||
temp = NamedTemporaryFile(delete=False)
|
||||
try:
|
||||
temp.write(data)
|
||||
temp.close()
|
||||
return process_file(temp.name)
|
||||
finally:
|
||||
os.unlink(temp.name)
|
||||
temp.close()
|
||||
```
|
||||
|
||||
## When to Log vs Return
|
||||
|
||||
| Scenario | Action |
|
||||
|----------|--------|
|
||||
| User action fails | Log warning, return safe default |
|
||||
| Internal error (corrupt data) | Log error, return safe default |
|
||||
| Transient failure (network) | Log warning, retry if appropriate |
|
||||
| Configuration error | Log error, raise with clear message |
|
||||
|
||||
## Exception Propagation
|
||||
|
||||
Only raise exceptions for:
|
||||
1. Configuration/setup errors (missing required env vars)
|
||||
2. Programming errors (invalid arguments)
|
||||
3. Fatal system errors (database corruption)
|
||||
|
||||
```python
|
||||
# GOOD: Raise for configuration errors
|
||||
def _get_api_key(self) -> str:
|
||||
key = os.environ.get("OPENROUTER_API_KEY")
|
||||
if not key:
|
||||
raise ProviderError(
|
||||
"OPENROUTER_API_KEY environment variable is required"
|
||||
)
|
||||
return key
|
||||
```
|
||||
|
||||
## Logging Errors
|
||||
|
||||
Always include context:
|
||||
|
||||
```python
|
||||
# GOOD: Include relevant context
|
||||
_logger.error(
|
||||
"Failed to fetch motion %d: %s",
|
||||
motion_id,
|
||||
exc
|
||||
)
|
||||
|
||||
# BAD: No context
|
||||
_logger.error("Failed to fetch")
|
||||
```
|
||||
@@ -1,36 +0,0 @@
|
||||
# Error handling style rules (YAML constraint example)
|
||||
|
||||
rules:
|
||||
- name: explicit_exceptions
|
||||
rule: "Raise explicit exceptions (ValueError, ProviderError) for known error conditions rather than returning magic values."
|
||||
examples:
|
||||
- good: |
|
||||
if not isinstance(text, str):
|
||||
raise ProviderError('text must be a string')
|
||||
- bad: |
|
||||
if not isinstance(text, str):
|
||||
return []
|
||||
|
||||
- name: avoid_broad_except
|
||||
rule: "Avoid 'except Exception:' that swallows errors. If broad except is used for best-effort, log the exception with logger.exception and re-raise or convert."
|
||||
examples:
|
||||
- bad: |
|
||||
try:
|
||||
do_work()
|
||||
except Exception:
|
||||
return []
|
||||
- remediation: |
|
||||
try:
|
||||
do_work()
|
||||
except SpecificError as exc:
|
||||
logger.warning('Handled error: %s', exc)
|
||||
raise
|
||||
|
||||
- name: logging_over_print
|
||||
rule: "Prefer logger.* over print() for messages and errors."
|
||||
examples:
|
||||
- bad: "print('Error fetching motions from API: %s' % e)"
|
||||
- good: "logger.exception('Error fetching motions from API')"
|
||||
|
||||
enforcement_examples:
|
||||
- "Add a static code check to flag 'print(' in modules (except in simple scripts) and 'except Exception:' usages without logger.exception."
|
||||
@@ -1,8 +1,47 @@
|
||||
---
|
||||
title: Logging Constraints
|
||||
category: constraints
|
||||
severity: critical
|
||||
---
|
||||
|
||||
# Logging Constraints
|
||||
|
||||
## Core Rule
|
||||
|
||||
**Use `logging.getLogger(__name__)` - never use `print()`**
|
||||
Use `logging.getLogger(__name__)` - never use `print()`
|
||||
|
||||
**CRITICAL ANTI-PATTERN**: `api_client.py` uses `print()` instead of logging (11 instances).
|
||||
|
||||
## CRITICAL Anti-Pattern: print() Instead of Logging
|
||||
|
||||
**File**: `api_client.py`
|
||||
**Evidence**: Lines with `print(f"...")` instead of `_logger.info(...)`
|
||||
|
||||
**Broken code**:
|
||||
```python
|
||||
def get_motions(self, ...):
|
||||
try:
|
||||
# ...
|
||||
print(f"Fetched {len(voting_records)} voting records from API") # BAD
|
||||
print(f"Processed into {len(motions)} unique motions") # BAD
|
||||
except Exception as e:
|
||||
print(f"Error fetching motions from API: {e}") # BAD - no traceback
|
||||
```
|
||||
|
||||
**Fix**:
|
||||
```python
|
||||
import logging
|
||||
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
def get_motions(self, ...):
|
||||
try:
|
||||
_logger.info("Fetched %d voting records from API", len(voting_records))
|
||||
_logger.info("Processed into %d unique motions", len(motions))
|
||||
except Exception as e:
|
||||
_logger.exception("Error fetching motions from API: %s", e)
|
||||
return []
|
||||
```
|
||||
|
||||
## Logger Initialization
|
||||
|
||||
@@ -31,6 +70,10 @@ _logger = logging.getLogger(__name__)
|
||||
_logger = logging.getLogger(__name__)
|
||||
```
|
||||
|
||||
**INCONSISTENCY WARNING**: 16 files use `logger`, 17 files use `_logger`. Choose one convention.
|
||||
|
||||
**Recommendation**: Use `_logger` (with underscore) for module-level loggers to distinguish from class-level loggers.
|
||||
|
||||
## Log Levels
|
||||
|
||||
| Level | When to Use |
|
||||
@@ -41,30 +84,6 @@ _logger = logging.getLogger(__name__)
|
||||
| ERROR | Operation failed, may need attention |
|
||||
| CRITICAL | Fatal error, program may crash |
|
||||
|
||||
## Examples
|
||||
|
||||
### Good Logging Practice
|
||||
```python
|
||||
_logger.info("Pipeline run: %s → %s (%s windows)", start, end, count)
|
||||
_logger.debug("Batch embedding attempt %d failed: %s", attempt, exc)
|
||||
_logger.warning("Fallback used for motion %d: %s", motion_id, reason)
|
||||
_logger.error("Query failed: %s", exc)
|
||||
```
|
||||
|
||||
### Bad: Using print()
|
||||
```python
|
||||
# BAD - don't use print
|
||||
print(f"Fetched {len(voting_records)} voting records from API")
|
||||
print(f"Error fetching motions from API: {e}")
|
||||
```
|
||||
|
||||
### Good: Using logger
|
||||
```python
|
||||
# GOOD - use logger
|
||||
_logger.info("Fetched %d voting records from API", len(voting_records))
|
||||
_logger.error("Error fetching motions from API: %s", e)
|
||||
```
|
||||
|
||||
## Exception Logging
|
||||
|
||||
Use `_logger.exception()` for caught exceptions (includes traceback):
|
||||
@@ -77,30 +96,6 @@ except Exception as exc:
|
||||
return fallback_value
|
||||
```
|
||||
|
||||
Use `_logger.error()` with explicit exception for controlled errors:
|
||||
|
||||
```python
|
||||
try:
|
||||
result = risky_operation()
|
||||
except Exception as exc:
|
||||
_logger.error("Operation failed: %s", exc)
|
||||
return fallback_value
|
||||
```
|
||||
|
||||
## Configuration
|
||||
|
||||
Ensure logging is configured in entry points:
|
||||
|
||||
```python
|
||||
# pipeline/run_pipeline.py
|
||||
def run(args):
|
||||
logging.basicConfig(
|
||||
level=logging.INFO,
|
||||
format="%(asctime)s %(levelname)s %(name)s: %(message)s",
|
||||
)
|
||||
# ... rest of pipeline
|
||||
```
|
||||
|
||||
## Anti-Patterns
|
||||
|
||||
### Debug Prints in Production Code
|
||||
@@ -117,22 +112,6 @@ _logger.debug("Processing window %s", wid)
|
||||
# BAD - mixing _logger and logger
|
||||
_logger = logging.getLogger(__name__)
|
||||
logger = logging.getLogger("other") # Inconsistent
|
||||
|
||||
# GOOD - use single consistent pattern
|
||||
_logger = logging.getLogger(__name__)
|
||||
```
|
||||
|
||||
### Missing Logger Initialization
|
||||
```python
|
||||
# BAD - no logger defined
|
||||
def some_function():
|
||||
logging.getLogger(__name__).info("...") # Redundant calls
|
||||
|
||||
# GOOD - define once at module level
|
||||
_logger = logging.getLogger(__name__)
|
||||
|
||||
def some_function():
|
||||
_logger.info("...")
|
||||
```
|
||||
|
||||
## Sensitive Data
|
||||
@@ -150,18 +129,3 @@ _logger.info("User %s voted %s", user_id, vote)
|
||||
# GOOD - log aggregates, not individual votes
|
||||
_logger.info("Vote recorded for session %s", session_id[:8])
|
||||
```
|
||||
|
||||
## Structured Logging
|
||||
|
||||
For complex data, use structured logging:
|
||||
|
||||
```python
|
||||
_logger.info(
|
||||
"Motion processed",
|
||||
extra={
|
||||
"motion_id": motion_id,
|
||||
"policy_area": policy_area,
|
||||
"processing_time_ms": elapsed_ms,
|
||||
}
|
||||
)
|
||||
```
|
||||
Reference in New Issue
Block a user