contextvar-remediation

# Contextvar Remediation Skill

**Use this skill when**: Converting explicit user_id parameters to ambient context using Python's contextvars.

**Critical Success Factors**:
1. Trust but verify - Always validate conclusions against the decision guide
2. Understand the architecture - Know how services are instantiated (singleton vs per-request)
3. Be systematic - Update services first, then callsites
4. Clean code - Omit optional parameters rather than passing None explicitly

---

## Investigation Phase

### Step 1: Understand Current Usage Patterns

Before making changes, **thoroughly investigate** how the code currently works:

```bash
# Find all explicit user_id usage
grep -r "user_id: Optional" target_directory/
grep -r "user_id: str" target_directory/
grep -r "user_id=" target_directory/ | wc -l
```

**Key Questions to Answer**:
- How are services instantiated? (Singleton factory? Per-request? Per-user?)
- Where is `set_current_user_id()` already being called?
- Which code runs during active requests vs background tasks?
- Does a decision guide exist? (e.g., CONTEXTVAR_DECISION_GUIDE.md)

### Step 2: Read and Internalize the Decision Guide

**DO NOT SKIP THIS STEP**. Read the project's contextvar decision guide completely.

**Critical patterns to identify**:
- ❌ **Anti-pattern**: `user_id: Optional[str] = None` on every method (method-level dual-mode)
- ✅ **Correct**: Constructor-only dual-mode OR pure ambient context
- ❌ **Anti-pattern**: Setting context then still passing user_id explicitly
- ✅ **Correct**: Set context once, let it propagate

**The guide will tell you**:
- Which pattern to use (constructor vs pure ambient)
- How to handle background tasks vs active flows
- Special cases (database connections, infrastructure boundaries)

### Step 3: Categorize Code by Context Type

Classify each file/method into:

1. **Active Request Flow**: HTTP endpoints, WebSocket handlers, tool execution
   - **Strategy**: Use pure ambient context
   - **Entry point**: Auth layer sets `set_current_user_id()`

2. **Background Tasks**: Scheduled jobs, batch processing, admin scripts
   - **Strategy**: Explicit iteration with context setting per user
   - **Pattern**: `for user in users: set_current_user_id(user_id); service.method()`

3. **Internal Services**: Shared infrastructure called by both above
   - **Strategy**: Depends on instantiation pattern (see Step 4)

4. **Infrastructure Boundary**: Database connections, cache clients
   - **Strategy**: Keep explicit (may have connection pooling concerns)

### Step 4: Identify Service Instantiation Pattern

**Critical decision point**: How are services created?

#### Pattern A: Singleton Factory (created at startup)
```python
# main.py
factory = ServiceFactory()  # Created once, no user context yet
```

**Implication**: Constructor-only dual-mode **DOESN'T WORK** because:
- Services are created before any user context exists
- Constructor can't call `get_current_user_id()` (would fail)
- Methods would still need to fetch context anyway

**Solution**: Pure ambient context in all methods.

#### Pattern B: Per-Request Instantiation
```python
# request handler
service = SomeService(user_id=get_current_user_id())
```

**Implication**: Constructor-only dual-mode **CAN WORK** because:
- Services are created during request with context available
- Constructor sets `self.user_id` once
- Methods use `self.user_id` (no parameters needed)

**Solution**: Constructor-only pattern acceptable.

#### Pattern C: Mixed Usage
Some methods called from both active and background contexts.

**Solution**: Background tasks must call `set_current_user_id()` before service calls.

---

## Implementation Phase

### Step 5: Update Service Layer (Services First, Callsites Later)

**Order matters**: Update service method signatures BEFORE updating callsites to see what breaks.

#### For Pure Ambient Context Pattern:

```python
# Before
class MemoryService:
    def find_memories(self, query: str, user_id: Optional[str] = None):
        memories = self.db.get_memories(query, user_id=user_id)
        return memories

# After
class MemoryService:
    def find_memories(self, query: str):
        memories = self.db.get_memories(query)  # db layer handles context
        return memories
```

**Key changes**:
1. Remove `user_id: Optional[str] = None` from method signature
2. Remove `user_id` from docstring Args section
3. Remove `user_id=...` from internal service calls
4. **Important**: Just omit the parameter, don't pass `user_id=None`

#### For Constructor-Only Pattern:

```python
# Before
class MemoryService:
    def find_memories(self, query: str, user_id: Optional[str] = None):
        memories = self.db.get_memories(query, user_id=user_id)
        return memories

# After
class MemoryService:
    def __init__(self, ..., user_id: Optional[str] = None):
        self.user_id = user_id or get_current_user_id()

    def find_memories(self, query: str):
        memories = self.db.get_memories(query, user_id=self.user_id)
        return memories
```

#### Methods That Need Explicit Context Reads

Some methods need user_id for logging or business logic:

```python
def run_full_process(self) -> Dict[str, int]:
    """Run process for current user."""
    from utils.user_context import get_current_user_id
    user_id = get_current_user_id()  # For logging only

    stats = self.do_work()
    logger.info(f"Completed for user {user_id}: {stats}")
    return stats
```

**Only call `get_current_user_id()` when actually needed**, not speculatively.

### Step 6: Update Infrastructure/Database Layer

**The infrastructure boundary typically keeps Optional[str]**:

```python
class DatabaseLayer:
    def _resolve_user_id(self, user_id: Optional[str] = None) -> str:
        """Resolve user_id from explicit param or ambient context."""
        if user_id is not None:
            return user_id
        from utils.user_context import get_current_user_id
        return get_current_user_id()

    def get_