phil/StarPunk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Add data transformer to resolve metrics dashboard template mismatch

Browse Source 
Root cause: Template expects flat structure (metrics.database.count) but
monitoring module provides nested structure (metrics.by_type.database.count)
with different field names (avg_duration_ms vs avg).

Solution: Route Adapter Pattern - transformer function maps data structure
at presentation layer.

Changes:
- Add transform_metrics_for_template() function to admin.py
- Update metrics_dashboard() route to use transformer
- Provide safe defaults for missing/empty metrics data
- Handle all operation types: database, http, render

Testing: All 32 admin route tests passing

Documentation:
- Updated implementation report with actual fix details
- Created consolidated hotfix design documentation
- Architectural review by architect (approved with minor concerns)

Technical debt: Adapter layer should be replaced with proper data
contracts in v1.2.0

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Phil Skentelbery
2025-11-25 21:24:47 -07:00
parent 2ca6ecc28f
commit d565721cdb
7 changed files with 846 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

115
docs/design/hotfix-v1.1.1-rc2-consolidated.md Normal file
View File

@@ -0,0 +1,115 @@
# Hotfix Design: v1.1.1-rc.2 - Metrics Dashboard Template Data Mismatch
## Problem Summary
Production deployment of v1.1.1-rc.1 exposed two critical issues in the metrics dashboard:
1. **Route Conflict** (Fixed in initial attempt): Two routes mapped to similar paths causing ambiguity
2. **Template/Data Mismatch** (Root cause): Template expects different data structure than monitoring module provides
### The Template/Data Mismatch
**Template Expects** (`metrics_dashboard.html` line 163):
```jinja2
{{ metrics.database.count|default(0) }}
{{ metrics.database.avg|default(0) }}
{{ metrics.database.min|default(0) }}
{{ metrics.database.max|default(0) }}
```
**Monitoring Module Returns**:
```python
{
"by_type": {
"database": {
"count": 50,
"avg_duration_ms": 12.5,
"min_duration_ms": 2.0,
"max_duration_ms": 45.0
}
}
}
```
Note the two mismatches:
1. **Nesting**: Template wants `metrics.database` but gets `metrics.by_type.database`
2. **Field Names**: Template wants `avg` but gets `avg_duration_ms`
## Solution: Route Adapter Pattern
Transform data at the presentation layer (route handler) to match template expectations.
### Implementation
Added a transformer function in `admin.py` that:
1. Flattens the nested structure (`by_type.database` → `database`)
2. Maps field names (`avg_duration_ms` → `avg`)
3. Provides safe defaults for missing data
```python
def transform_metrics_for_template(metrics_stats):
"""Transform metrics stats to match template structure"""
transformed = {}
# Map by_type to direct access with field name mapping
for op_type in ['database', 'http', 'render']:
if 'by_type' in metrics_stats and op_type in metrics_stats['by_type']:
type_data = metrics_stats['by_type'][op_type]
transformed[op_type] = {
'count': type_data.get('count', 0),
'avg': type_data.get('avg_duration_ms', 0), # Note field name change
'min': type_data.get('min_duration_ms', 0),
'max': type_data.get('max_duration_ms', 0)
}
else:
# Safe defaults
transformed[op_type] = {'count': 0, 'avg': 0, 'min': 0, 'max': 0}
# Keep other top-level stats
transformed['total_count'] = metrics_stats.get('total_count', 0)
transformed['max_size'] = metrics_stats.get('max_size', 1000)
transformed['process_id'] = metrics_stats.get('process_id', 0)
return transformed
```
### Why This Approach?
1. **Minimal Risk**: Only changes route handler, not core monitoring module
2. **Preserves API**: Monitoring module remains unchanged for other consumers
3. **No Template Changes**: Avoids modifying template and JavaScript
4. **Clear Separation**: Route acts as adapter between business logic and view
## Additional Fixes Applied
1. **Route Path Change**: `/admin/dashboard` → `/admin/metrics-dashboard` (prevents conflict)
2. **Defensive Imports**: Graceful handling of missing monitoring module
3. **Error Handling**: Safe defaults when metrics collection fails
## Testing and Validation
Created comprehensive test script validating:
- Data structure transformation works correctly
- All template fields accessible after transformation
- Safe defaults provided for missing data
- Field name mapping correct
All 32 admin route tests pass with 100% success rate.
## Files Modified
1. `/starpunk/routes/admin.py`:
- Lines 218-260: Added transformer function
- Line 263: Changed route path
- Lines 285-314: Applied transformer and added error handling
2. `/starpunk/__init__.py`: Version bump to 1.1.1-rc.2
3. `/CHANGELOG.md`: Documented hotfix
## Production Impact
**Before**: 500 error with `'dict object' has no attribute 'database'`
**After**: Metrics dashboard loads correctly with properly structured data
This is a tactical bug fix, not an architectural change, and should be documented as such.

197
docs/design/hotfix-v1.1.1-rc2-route-conflict.md Normal file
View File

@@ -0,0 +1,197 @@
# Hotfix Design: v1.1.1-rc.2 Route Conflict Resolution
## Problem Summary
Production deployment of v1.1.1-rc.1 causes 500 error at `/admin/dashboard` due to:
1. Route naming conflict between two dashboard functions
2. Missing `starpunk.monitoring` module causing ImportError
## Root Cause Analysis
### Primary Issue: Route Conflict
```python
# Line 26: Original dashboard
@bp.route("/") # Registered as "admin.dashboard"
def dashboard(): # Function name creates endpoint "admin.dashboard"
# Shows notes list
# Line 218: Metrics dashboard
@bp.route("/dashboard") # CONFLICT: Also accessible at /admin/dashboard
def metrics_dashboard(): # Function name creates endpoint "admin.metrics_dashboard"
from starpunk.monitoring import get_metrics_stats # FAILS: Module doesn't exist
```
### Secondary Issue: Missing Module
The metrics dashboard attempts to import `starpunk.monitoring` which doesn't exist in production, causing immediate ImportError on route access.
## Solution Design
### Minimal Code Changes
#### 1. Route Path Change (admin.py)
**Line 218 - Change route decorator:**
```python
# FROM:
@bp.route("/dashboard")
# TO:
@bp.route("/metrics-dashboard")
```
This single character change resolves the route conflict while maintaining all other functionality.
#### 2. Defensive Import Pattern (admin.py)
**Lines 239-250 - Add graceful degradation:**
```python
def metrics_dashboard():
"""Metrics visualization dashboard (Phase 3)"""
# Defensive imports with fallback
try:
from starpunk.database.pool import get_pool_stats
from starpunk.monitoring import get_metrics_stats
monitoring_available = True
except ImportError:
monitoring_available = False
get_pool_stats = lambda: {"error": "Pool stats not available"}
get_metrics_stats = lambda: {"error": "Monitoring not implemented"}
# Continue with safe execution...
```
### URL Structure After Fix
| Path | Function | Purpose | Status |
|------|----------|---------|--------|
| `/admin/` | `dashboard()` | Notes list | Working |
| `/admin/metrics-dashboard` | `metrics_dashboard()` | Metrics viz | Fixed |
| `/admin/metrics` | `metrics()` | JSON API | Working |
| `/admin/health` | `health_diagnostics()` | Health check | Working |
### Redirect Behavior
All existing redirects using `url_for("admin.dashboard")` will continue to work:
- They resolve to the `dashboard()` function
- Users land on the notes list at `/admin/`
- No code changes needed in 8+ redirect locations
### Navigation Updates
The template at `/templates/admin/base.html` is already correct:
```html
<a href="{{ url_for('admin.dashboard') }}">Dashboard</a> <!-- Goes to /admin/ -->
<a href="{{ url_for('admin.metrics_dashboard') }}">Metrics</a> <!-- Goes to /admin/metrics-dashboard -->
```
## Implementation Steps
### Step 1: Create Hotfix Branch
```bash
git checkout -b hotfix/v1.1.1-rc2-route-conflict
```
### Step 2: Apply Code Changes
1. Edit `/starpunk/routes/admin.py`:
- Change line 218 route decorator
- Add try/except around monitoring imports (lines 239-250)
- Add try/except around pool stats import (line 284)
### Step 3: Local Testing
```bash
# Test without monitoring module (production scenario)
uv run python -m pytest tests/test_admin_routes.py
uv run flask run
# Verify:
# 1. /admin/ shows notes
# 2. /admin/metrics-dashboard doesn't 500
# 3. All CRUD operations work
```
### Step 4: Update Version
Edit `/starpunk/__init__.py`:
```python
__version__ = "1.1.1-rc.2"
```
### Step 5: Document in CHANGELOG
Add to `/CHANGELOG.md`:
```markdown
## [1.1.1-rc.2] - 2025-11-25
### Fixed
- Critical: Resolved route conflict causing 500 error on /admin/dashboard
- Added defensive imports for missing monitoring module
- Renamed metrics dashboard route to /admin/metrics-dashboard for clarity
```
## Testing Checklist
### Functional Tests
- [ ] `/admin/` displays notes dashboard
- [ ] `/admin/metrics-dashboard` loads without 500 error
- [ ] Create note redirects to `/admin/`
- [ ] Edit note redirects to `/admin/`
- [ ] Delete note redirects to `/admin/`
- [ ] Navigation links work correctly
- [ ] `/admin/metrics` JSON endpoint works
- [ ] `/admin/health` diagnostic endpoint works
### Error Handling Tests
- [ ] Metrics dashboard shows graceful message when monitoring unavailable
- [ ] No Python tracebacks exposed to users
- [ ] Flash messages display appropriately
### Regression Tests
- [ ] IndieAuth login flow works
- [ ] Note CRUD operations unchanged
- [ ] RSS feed generation works
- [ ] Micropub endpoint functional
## Rollback Plan
If issues discovered after deployment:
1. Revert to v1.1.1-rc.1
2. Users directed to `/admin/` instead of `/admin/dashboard`
3. Metrics dashboard temporarily disabled
## Success Criteria
1. **No 500 Errors**: All admin routes respond with 200/300 status codes
2. **Backward Compatible**: Existing functionality unchanged
3. **Clear Navigation**: Users can access both dashboards
4. **Graceful Degradation**: Missing modules handled elegantly
## Long-term Recommendations
### For v1.2.0
1. Implement `starpunk.monitoring` module properly
2. Add comprehensive metrics collection
3. Consider dashboard consolidation
### For v2.0.0
1. Restructure admin area with sub-blueprints
2. Implement consistent URL patterns
3. Add dashboard customization options
## Risk Assessment
| Risk | Likelihood | Impact | Mitigation |
|------|------------|--------|------------|
| Route still conflicts | Low | High | Tested locally first |
| Template breaks | Low | Medium | Template already correct |
| Monitoring import fails differently | Low | Low | Defensive imports added |
| Performance impact | Very Low | Low | Minimal code change |
## Approval Requirements
This hotfix requires:
1. Code review of changes
2. Local testing confirmation
3. Staging deployment (if available)
4. Production deployment authorization
## Contact
- Architect: StarPunk Architect
- Issue: Production 500 error on /admin/dashboard
- Priority: CRITICAL
- Timeline: Immediate deployment required

160
docs/design/hotfix-validation-script.md Normal file
View File

@@ -0,0 +1,160 @@
# Hotfix Validation Script for v1.1.1-rc.2
## Quick Validation Commands
Run these commands after applying the hotfix to verify it works:
### 1. Check Route Registration
```python
# In Flask shell (uv run flask shell)
from starpunk import create_app
app = create_app()
# List all admin routes
for rule in app.url_map.iter_rules():
if 'admin' in rule.endpoint:
print(f"{rule.endpoint:30} -> {rule.rule}")
# Expected output:
# admin.dashboard -> /admin/
# admin.metrics_dashboard -> /admin/metrics-dashboard
# admin.metrics -> /admin/metrics
# admin.health_diagnostics -> /admin/health
# (plus CRUD routes)
```
### 2. Test URL Resolution
```python
# In Flask shell
from flask import url_for
with app.test_request_context():
print("Notes dashboard:", url_for('admin.dashboard'))
print("Metrics dashboard:", url_for('admin.metrics_dashboard'))
# Expected output:
# Notes dashboard: /admin/
# Metrics dashboard: /admin/metrics-dashboard
```
### 3. Simulate Production Environment (No Monitoring Module)
```bash
# Temporarily rename monitoring module if it exists
mv starpunk/monitoring.py starpunk/monitoring.py.bak 2>/dev/null
# Start the server
uv run flask run
# Test the routes
curl -I http://localhost:5000/admin/ # Should return 302 (redirect to auth)
curl -I http://localhost:5000/admin/metrics-dashboard # Should return 302 (not 500!)
# Restore monitoring module if it existed
mv starpunk/monitoring.py.bak starpunk/monitoring.py 2>/dev/null
```
### 4. Manual Browser Testing
After logging in with IndieAuth:
1. Navigate to `/admin/` - Should show notes list
2. Click "Metrics" in navigation - Should load `/admin/metrics-dashboard`
3. Click "Dashboard" in navigation - Should return to `/admin/`
4. Create a new note - Should redirect to `/admin/` after creation
5. Edit a note - Should redirect to `/admin/` after saving
6. Delete a note - Should redirect to `/admin/` after deletion
### 5. Check Error Logs
```bash
# Monitor Flask logs for any errors
uv run flask run 2>&1 | grep -E "(ERROR|CRITICAL|ImportError|500)"
# Should see NO output related to route conflicts or import errors
```
### 6. Automated Test Suite
```bash
# Run the admin route tests
uv run python -m pytest tests/test_admin_routes.py -v
# All tests should pass
```
## Production Verification
After deploying to production:
### 1. Health Check
```bash
curl https://starpunk.thesatelliteoflove.com/health
# Should return 200 OK
```
### 2. Admin Routes (requires auth)
```bash
# These should not return 500
curl -I https://starpunk.thesatelliteoflove.com/admin/
curl -I https://starpunk.thesatelliteoflove.com/admin/metrics-dashboard
```
### 3. Monitor Error Logs
```bash
# Check production logs for any 500 errors
tail -f /var/log/starpunk/error.log | grep "500"
# Should see no new 500 errors
```
### 4. User Verification
1. Log in to admin panel
2. Verify both dashboards accessible
3. Perform one CRUD operation to verify redirects
## Rollback Commands
If issues are discovered:
```bash
# Quick rollback to previous version
git checkout v1.1.1-rc.1
systemctl restart starpunk
# Or if using containers
docker pull starpunk:v1.1.1-rc.1
docker-compose up -d
```
## Success Indicators
✅ No 500 errors in logs
✅ Both dashboards accessible
✅ All redirects work correctly
✅ Navigation links functional
✅ No ImportError in logs
✅ Existing functionality unchanged
## Report Template
After validation, report:
```
HOTFIX VALIDATION REPORT - v1.1.1-rc.2
Date: [DATE]
Environment: [Production/Staging]
Route Resolution:
- /admin/ : ✅ Shows notes dashboard
- /admin/metrics-dashboard : ✅ Loads without error
Functionality Tests:
- Create Note: ✅ Redirects to /admin/
- Edit Note: ✅ Redirects to /admin/
- Delete Note: ✅ Redirects to /admin/
- Navigation: ✅ All links work
Error Monitoring:
- 500 Errors: None observed
- Import Errors: None observed
- Flash Messages: Working correctly
Conclusion: Hotfix successful, ready for production
```