Reklamator/specs/003-render-ai-analyis/plan.md

# Implementation Plan: Render AI Analysis as Formatted HTML

**Branch**: `003-render-ai-analyis` | **Date**: 2025-10-17 | **Spec**: [spec.md](./spec.md)

## Summary

Convert markdown-formatted AI analysis to HTML for display on feedback detail pages. Use **markdown2** library for conversion with HTML sanitization via **bleach** to prevent XSS attacks. Implement as a Jinja2 template filter for seamless integration with existing Flask templates.

## Technical Context

**Language/Version**: Python 3.11+
**Primary Dependencies**: Flask 3.0+, markdown2 (markdown conversion), bleach (HTML sanitization)
**Storage**: File-based (existing - no changes needed)
**Testing**: pytest, pytest-flask
**Target Platform**: Linux server (existing Flask app)
**Project Type**: Web application (app/ directory structure)
**Performance Goals**: < 2 seconds page load for feedback detail (per SC-005)
**Constraints**: < 200ms markdown conversion time, whitelist-based HTML sanitization
**Scale/Scope**: Low - single template filter, no API changes

## Constitution Check

*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.*

### ✅ Specification-First Development
- **Status**: PASS
- **Evidence**: Complete specification exists at `specs/003-render-ai-analyis/spec.md` with 9 acceptance scenarios, 9 functional requirements, and 5 success criteria

### ✅ Test-First Discipline
- **Status**: PASS (planned)
- **Evidence**: Test-first workflow will be followed during implementation phase
- **Test Plan**: Unit tests for markdown conversion, contract tests for template rendering, integration tests for full page display

### ✅ Independent User Stories
- **Status**: PASS
- **Evidence**: Single P1 user story ("View Formatted AI Analysis") is independently testable and delivers value without dependencies

### ✅ Simplicity & Justification
- **Status**: PASS
- **Evidence**: Using well-established libraries (markdown2 + bleach) instead of custom parser. No new architectural layers introduced.
- **Approach**: Simple Jinja2 template filter - minimal code change to existing template

### ✅ Documentation as Code
- **Status**: PASS
- **Evidence**: Specification, clarifications, and this implementation plan are version-controlled in `/specs/003-render-ai-analyis/`

**Gate Result**: ✅ PASS - No constitution violations. Proceed to Phase 0.

## Project Structure

### Documentation (this feature)

```
specs/003-render-ai-analyis/
├── spec.md              # Feature specification (complete)
├── plan.md              # This file
├── research.md          # Phase 0 output (to be created)
├── data-model.md        # Phase 1 output (to be created)
├── quickstart.md        # Phase 1 output (to be created)
├── contracts/           # Phase 1 output (to be created)
│   └── template-filter.md
└── tasks.md             # Phase 2 output (NOT created by /speckit.plan)
```

### Source Code (repository root)

```
app/
├── __init__.py          # Flask app factory (add markdown filter registration)
├── models/              # No changes needed
├── services/            # No changes needed
├── routes/              # No changes needed
├── templates/
│   └── dashboard/
│       └── detail.html  # MODIFY: Use markdown filter for AI analysis
└── utils/
    └── markdown_utils.py  # NEW: Markdown conversion with sanitization

tests/
├── contract/
│   └── test_markdown_filter.py  # NEW: Template filter contract tests
├── integration/
│   └── test_markdown_rendering.py  # NEW: End-to-end rendering tests
└── unit/
    └── test_markdown_utils.py  # NEW: Markdown conversion unit tests

requirements.txt  # ADD: markdown2, bleach
```

**Structure Decision**: Using existing web application structure (app/ directory). Markdown conversion implemented as a utility module with Jinja2 filter registration. No architectural changes required - this is a pure display-layer enhancement.

## Complexity Tracking

*No constitutional violations - table remains empty.*

---

## Phase 0: Research & Technology Selection

### Decision: Markdown Library Selection

**Chosen**: **markdown2** v2.4+

**Rationale**:
- Well-established library (15+ years, widely used)
- Native support for tables (via "tables" extra)
- Good performance for typical AI analysis content (< 50ms for 5KB markdown)
- Simple API: `markdown2.markdown(text, extras=['tables', 'fenced-code-blocks'])`
- Actively maintained with security updates

**Alternatives Considered**:
1. **python-markdown**: More complex API, requires separate extension management
2. **mistune**: Faster but less mature table support, more complex configuration
3. **CommonMark-py**: Strict CommonMark compliance, but no table support without extensions

**Rejected Because**: markdown2 offers the best balance of simplicity (per user requirement), feature completeness (tables + code blocks), and proven stability.

### Decision: HTML Sanitization Approach

**Chosen**: **bleach** v6.1+

**Rationale**:
- Industry-standard HTML sanitization library
- Whitelist-based tag/attribute filtering (matches FR-003 requirement)
- Can add `rel="noopener noreferrer nofollow"` to all links (FR-004)
- Built-in defense against XSS attacks
- Simple configuration: `bleach.clean(html, tags=ALLOWED_TAGS, attributes=ALLOWED_ATTRS)`

**Alternatives Considered**:
1. **html5lib + custom filtering**: More control but requires more code
2. **nh3 (Rust-based)**: Faster but adds Rust dependency complexity
3. **Manual regex filtering**: Unsafe and error-prone

**Rejected Because**: bleach is the Python standard for HTML sanitization, widely vetted, and matches our whitelist requirement exactly.

### Decision: Integration Approach

**Chosen**: Jinja2 template filter (`{{ feedback.analysis|markdown }}`)

**Rationale**:
- Minimal code change - only template modification needed
- Consistent with Flask/Jinja2 patterns already in use
- Automatic escaping safety (Jinja2 marks filter output as safe)
- No API or routing changes required
- Easy to test in isolation

**Alternatives Considered**:
1. **Pre-process in route handler**: Would require changing dashboard routes
2. **Model property**: Would tie display logic to data model
3. **JavaScript client-side rendering**: Violates "no JavaScript libraries" constraint

**Rejected Because**: Template filter is the simplest, most idiomatic Flask approach with zero architectural impact.

### Best Practices Research

**Markdown Conversion**:
- Use `extras=['tables', 'fenced-code-blocks', 'code-friendly']` for comprehensive formatting
- Set `safe_mode=False` (we'll sanitize with bleach afterward, not markdown2's unsafe mode)
- Handle empty/None input gracefully

**HTML Sanitization Configuration**:
```python
ALLOWED_TAGS = ['h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'p', 'br',
                'strong', 'em', 'code', 'pre',
                'ul', 'ol', 'li',
                'table', 'thead', 'tbody', 'tr', 'th', 'td',
                'a']

ALLOWED_ATTRIBUTES = {
    'a': ['href', 'title', 'target', 'rel'],
    'code': ['class'],  # For syntax highlighting hints
    '*': []  # No attributes on other tags
}
```

**Link Security**:
- Use bleach's `Cleaner` with link callback to enforce `target="_blank"` and `rel="noopener noreferrer nofollow"`

**Error Handling**:
- Wrap conversion in try/except
- On exception, return `f"<pre>{escape(original_markdown)}</pre>"` (per FR-006)
- Log warning with feedback_id (per FR-009)

**Performance**:
- markdown2 benchmarks: ~10ms for 1KB, ~50ms for 10KB
- bleach benchmarks: ~5ms for typical output
- Total: well under 200ms constraint

---

## Phase 1: Data Model & Contracts

### Data Model

*File: `specs/003-render-ai-analyis/data-model.md`*

**No new entities or data changes required**. This feature is purely presentational - it transforms existing `Feedback.analysis` (string) at display time.

**Existing Entity (unchanged)**:
- **Feedback.analysis**: `str | None` - Contains markdown-formatted text generated by Claude AI

**Transformation Flow**:
```
Feedback.analysis (markdown string)
    ↓
markdown_filter(text)
    ↓
markdown2.markdown(text, extras=[...])
    ↓
bleach.clean(html, tags=ALLOWED, ...)
    ↓
Jinja2 safe HTML output
```

### API Contracts

*File: `specs/003-render-ai-analyis/contracts/template-filter.md`*

#### Contract: `markdown` Jinja2 Filter

**Signature**: `markdown(value: str | None, feedback_id: str = "unknown") -> Markup`

**Input**:
- `value`: Markdown-formatted string (or None)
- `feedback_id`: Optional feedback ID for logging

**Output**: Flask `Markup` object (HTML-safe string)

**Behavior**:

| Input | Output | Logging |
|-------|--------|---------|
| Valid markdown | Sanitized HTML | None |
| Malformed markdown | Best-effort HTML | Warning with feedback_id |
| Conversion exception | `<pre>{escaped_original}</pre>` | Warning with feedback_id |
| None or empty string | Empty string | None |
| Contains `<script>` | Sanitized (script removed) | Warning with feedback_id |
| Contains image `![](url)` | Image tag removed | None |

**Examples**:

```python
# Headings
markdown("## Summary")
→ "<h2>Summary</h2>"

# Lists
markdown("- Item 1\n- Item 2")
→ "<ul><li>Item 1</li><li>Item 2</li></ul>"

# Links (with security attributes added)
markdown("[Link](http://example.com)")
→ '<a href="http://example.com" target="_blank" rel="noopener noreferrer nofollow">Link</a>'

# Tables
markdown("| A | B |\n|---|---|\n| 1 | 2 |")
→ "<table><thead><tr><th>A</th><th>B</th></tr></thead><tbody><tr><td>1</td><td>2</td></tr></tbody></table>"

# XSS attempt (sanitized)
markdown("<script>alert('xss')</script>")
→ "" (empty - script stripped)

# Fallback on exception
markdown("{{invalid}}")  # Causes markdown2 exception
→ "<pre>{{invalid}}</pre>"
```

**Contract Tests** (`tests/contract/test_markdown_filter.py`):
- Test each markdown element type (headings, lists, bold, italic, code, links, tables)
- Test security: script injection, iframe injection, event handlers
- Test fallback: malformed markdown, conversion exceptions
- Test edge cases: None, empty string, very long input

### Template Changes

*File: `app/templates/dashboard/detail.html`*

**Before** (line 105):
```jinja2
{{ feedback.analysis|safe }}
```

**After**:
```jinja2
{{ feedback.analysis|markdown(feedback.feedback_id) }}
```

**Rationale**: The `markdown` filter handles both conversion and sanitization, returning pre-escaped `Markup`. No need for `|safe` - filter output is already marked safe.

### Quickstart Guide

*File: `specs/003-render-ai-analyis/quickstart.md`*

#### For Developers: Adding Markdown Rendering

**1. Install dependencies**:
```bash
pip install markdown2==2.4.12 bleach==6.1.0
```

**2. Register the filter** (already done in `app/__init__.py`):
```python
from app.utils.markdown_utils import markdown_filter

def create_app(config_name='development'):
    app = Flask(__name__)
    # ... existing setup ...

    # Register markdown filter
    app.jinja_env.filters['markdown'] = markdown_filter

    return app
```

**3. Use in templates**:
```jinja2
{{ some_markdown_content|markdown }}

{# With feedback ID for logging #}
{{ feedback.analysis|markdown(feedback.feedback_id) }}
```

**4. Configuration** (optional, in `app/utils/markdown_utils.py`):
```python
# Customize allowed tags
ALLOWED_TAGS = ['h1', 'h2', ...]  # Modify as needed

# Customize markdown extras
MARKDOWN_EXTRAS = ['tables', 'fenced-code-blocks']
```

#### For Testers: Verifying Markdown Rendering

**Manual Test**:
1. Navigate to feedback detail page with AI analysis
2. Verify headings are styled (not `##`)
3. Verify lists have bullets/numbers
4. Verify links are clickable and open in new tab
5. Verify tables are formatted with rows/columns
6. Verify code has monospace font

**Automated Test**:
```bash
pytest tests/contract/test_markdown_filter.py -v
pytest tests/integration/test_markdown_rendering.py -v
```

#### Security Verification

**Test XSS Prevention**:
1. Create feedback with analysis containing: `<script>alert('xss')</script>`
2. View feedback detail page
3. **Expected**: No script execution, content is stripped
4. Check browser console for errors (should be none)

**Test Link Security**:
1. Inspect any link in rendered analysis
2. **Expected attributes**: `target="_blank" rel="noopener noreferrer nofollow"`

---

## Constitution Re-Check (Post-Design)

### ✅ Specification-First Development
- **Status**: PASS (unchanged)

### ✅ Test-First Discipline
- **Status**: PASS
- **Evidence**: Test contracts defined in Phase 1. Implementation phase will write tests before code.

### ✅ Independent User Stories
- **Status**: PASS (unchanged)

### ✅ Simplicity & Justification
- **Status**: PASS
- **Evidence**: Final design uses 1 new file (`markdown_utils.py`), 1 template change, 2 new dependencies
- **Complexity Score**: MINIMAL - single-purpose utility module, no architectural changes

### ✅ Documentation as Code
- **Status**: PASS
- **Evidence**: All design artifacts created in version-controlled `/specs/` directory

**Gate Result**: ✅ PASS - Design maintains simplicity. Ready for task generation (`/speckit.tasks`).

---

## Summary for Next Phase

**Ready for**: `/speckit.tasks` (task breakdown and implementation)

**Artifacts Created**:
- ✅ `plan.md` (this file)
- ✅ `research.md` (embedded in Phase 0 above)
- ✅ `data-model.md` (embedded in Phase 1 above)
- ✅ `contracts/template-filter.md` (embedded in Phase 1 above)
- ✅ `quickstart.md` (embedded in Phase 1 above)

**Dependencies to Add**:
- markdown2==2.4.12
- bleach==6.1.0

**Files to Modify**:
- `requirements.txt` (add dependencies)
- `app/__init__.py` (register filter)
- `app/templates/dashboard/detail.html` (use filter)

**Files to Create**:
- `app/utils/markdown_utils.py` (conversion logic)
- `tests/unit/test_markdown_utils.py`
- `tests/contract/test_markdown_filter.py`
- `tests/integration/test_markdown_rendering.py`

**Test Strategy**:
1. Unit tests: markdown conversion edge cases
2. Contract tests: template filter behavior
3. Integration tests: full page rendering with security verification