resolutionflow/LESSONS-LEARNED.md

# Lessons Learned

> **Purpose:** This file documents bugs, fixes, and gotchas encountered during development.
> **For Claude Code:** Read this file at the start of each session to avoid repeating past mistakes.
> **Last Updated:** February 1, 2026

---

## Environment Setup (New Machine)

### Database Name Mismatch After Fresh Clone
**Problem:** After cloning the repo and running `docker-compose up -d`, Alembic migrations fail with `database "decision_tree" does not exist`.

**Cause:** The `.env` file contains the old database name (`decision_tree`) but `docker-compose.yml` creates a database called `patherly`.

**Solution:** Update `.env` to use the correct database name:
```
DATABASE_URL=postgresql+asyncpg://postgres:postgres@localhost:5432/patherly
DATABASE_URL_SYNC=postgresql://postgres:postgres@localhost:5432/patherly
```

**Files affected:** `backend/.env`

---

### Missing @/lib/utils (cn function)
**Problem:** Frontend fails to compile with error: `Failed to resolve import "@/lib/utils" from "src/pages/SessionDetailPage.tsx". Does the file exist?`

**Cause:** The `src/lib/utils.ts` file wasn't committed to the repo or was missed during setup. This file provides the `cn()` utility function used for combining Tailwind classes.

**Solution:** Create `frontend/src/lib/utils.ts`:
```typescript
import { type ClassValue, clsx } from 'clsx'
import { twMerge } from 'tailwind-merge'

export function cn(...inputs: ClassValue[]) {
  return twMerge(clsx(inputs))
}
```

**Dependencies required:** `clsx` and `tailwind-merge` (already in package.json)

**Files affected:** `frontend/src/lib/utils.ts`

---

### pip install in venv doesn't need --break-system-packages
**Problem:** Confusion about whether to use `--break-system-packages` flag.

**Clarification:** The `--break-system-packages` flag is only needed when installing packages **outside** of a virtual environment. When your venv is active (you see `(venv)` prefix), you don't need this flag.

---

### New Machine Setup Checklist
When setting up development on a new machine:

1. **Clone repo:** `git clone <repo-url>`
2. **Start Docker Desktop**
3. **Start database:** `cd backend && docker-compose up -d`
4. **Fix .env database name** if it says `decision_tree` → change to `patherly`
5. **Create venv:** `python -m venv venv`
6. **Activate venv:** `.\venv\Scripts\Activate`
7. **Install backend deps:** `pip install -r requirements.txt`
8. **Run migrations:** `alembic upgrade head`
9. **Start backend:** `uvicorn app.main:app --reload`
10. **Install frontend deps:** `cd ../frontend && npm install`
11. **Create lib/utils.ts** if missing (see above)
12. **Start frontend:** `npm run dev`

---

## Python / Backend

### DateTime Timezone Handling ⚠️ CRITICAL
**Problem:** SQLAlchemy `DateTime` fields caused Internal Server Errors when mixing timezone-aware and timezone-naive datetimes.

**Error:** `can't subtract offset-naive and offset-aware datetimes`

**Solution:** 
- Always use `DateTime(timezone=True)` in SQLAlchemy models
- Always use `datetime.now(timezone.utc)` for defaults and assignments
- Never use `datetime.utcnow()` (deprecated and timezone-naive)

**Correct pattern:**
```python
from datetime import datetime, timezone
from sqlalchemy import DateTime

created_at = Column(DateTime(timezone=True), default=lambda: datetime.now(timezone.utc))
```

**Files affected:** All models with timestamp fields (user.py, tree.py, session.py, team.py, attachment.py)

---

### pytest-asyncio Version Compatibility
**Problem:** Tests fail with `AttributeError: 'Package' object has no attribute 'obj'`

**Cause:** Incompatibility between pytest 8.0.0 and pytest-asyncio 0.23.3

**Solution:** Upgrade pytest-asyncio to 0.24.0
```powershell
pip install pytest-asyncio==0.24.0 --upgrade
```

---

### bcrypt / passlib Compatibility
**Problem:** Password hashing fails with newer bcrypt versions.

**Solution:** Pin bcrypt version in requirements.txt:
```
bcrypt==4.0.1
passlib[bcrypt]==1.7.4
```

---

### Virtual Environment Best Practices
**Problem:** OneDrive sync causes conflicts with venv, `__pycache__`, and lock files.

**Solution:** 
- Keep project in `C:\Dev\Projects\`, NOT in OneDrive-synced folders
- Add to `.gitignore`: `venv/`, `__pycache__/`, `*.pyc`

---

### Installing Packages in venv
**Problem:** `pip install` sometimes installs globally instead of in venv.

**Solution:** 
- Always verify venv is active: look for `(venv)` prefix in terminal
- If VS Code prompts to create a new venv when one exists, click "Don't show again"
- Use `pip install --break-system-packages` flag if needed (Python 3.12+)

---

### Alembic Migrations
**Problem:** Model changes not reflected in database.

**Solution:** Always run migrations after model changes:
```powershell
cd backend
alembic revision --autogenerate -m "Description of change"
alembic upgrade head
```

---

## TypeScript / Frontend

### React State: Don't Store Object Snapshots for Editing ⚠️ CRITICAL
**Problem:** Form inputs don't update when typing - characters appear then immediately disappear.

**Cause:** Storing a full object from a Zustand store in local state creates a snapshot. When the store updates, the local state still holds the old reference, causing the input's `value` to revert.

**Broken pattern:**
```tsx
// BAD: Stores a snapshot that won't update
const [editingNode, setEditingNode] = useState<TreeStructure | null>(null)

// When modal opens:
setEditingNode(node) // snapshot captured here

// In modal form:
<input value={editingNode.question} onChange={...} /> // always shows old value
```

**Solution:** Store only the ID, then fetch the current object from the store on each render:
```tsx
// GOOD: Store only the ID
const [editingNodeId, setEditingNodeId] = useState<string | null>(null)
const editingNode = editingNodeId ? findNode(editingNodeId) : null

// When modal opens:
setEditingNodeId(node.id)

// In modal form - now gets fresh data each render:
<input value={editingNode.question} onChange={...} />
```

**Why it works:** By calling `findNode()` on each render, the component always gets the current state from the store after updates.

**Files affected:** Any component that opens a modal/form to edit store data (NodeList.tsx)

---

### Modal Draft State: Don't Overwrite Store-Managed Fields ⚠️ CRITICAL
**Problem:** Child nodes created while a modal is open disappear when clicking "Done". Tree validation fails with errors about non-existent nodes.

**Cause:** When using local draft state for Cancel/Done functionality in a modal:
1. Modal opens and clones the entire node (including `children: []`) into local state
2. User creates new child nodes via a dropdown (e.g., NodePicker) - these are added to the **store**
3. User clicks "Done" → modal saves draft back to store
4. The draft's stale `children: []` **overwrites** the store's actual children array
5. Newly created nodes are deleted

**Symptoms:**
- Child nodes don't appear in the tree after closing modal
- Validation errors: "Option references non-existent node"
- Tree won't save due to validation failures

**Broken pattern:**
```tsx
// Modal with local draft state
const [draft, setDraft] = useState(() => structuredClone(node))

const handleSave = () => {
  // BAD: This overwrites children with stale snapshot!
  updateNode(node.id, draft)
  onClose()
}
```

**Solution:** Exclude fields that are managed by other store actions (like `children` managed by `addNode`/`deleteNode`):
```tsx
const handleSave = () => {
  // GOOD: Exclude 'children' - it's managed separately by addNode/deleteNode
  const { children, ...draftWithoutChildren } = draft
  updateNode(node.id, draftWithoutChildren)
  onClose()
}
```

**General rule:** When a modal uses local draft state, identify which fields are:
- **Editable by the form** → include in draft save
- **Managed by other actions** (addNode, deleteNode, etc.) → exclude from draft save

**Files affected:** `NodeEditorModal.tsx`, any modal that edits objects with nested collections

---

### Modal Scroll/Overflow with Fixed Header and Footer
**Problem:** Modal content extends beyond screen when there's too much content, pushing close button and action buttons off-screen.

**Solution:** Use flex layout with fixed header/footer and scrollable body:
```tsx
// Modal structure
<div className="flex max-h-[85vh] w-full flex-col">
  {/* Header - fixed at top */}
  <div className="flex-shrink-0 border-b px-6 py-4">
    <h2>{title}</h2>
    <button>X</button>
  </div>

  {/* Body - scrollable */}
  <div className="flex-1 overflow-y-auto px-6 py-4">
    {children}
  </div>

  {/* Footer - fixed at bottom */}
  {footer && (
    <div className="flex-shrink-0 border-t px-6 py-4">
      {footer}
    </div>
  )}
</div>
```

**Key points:**
- `max-h-[85vh]` constrains total modal height
- `flex-col` enables vertical flex layout
- `flex-shrink-0` on header/footer prevents them from shrinking
- `flex-1 overflow-y-auto` on body makes it fill remaining space and scroll

**Files affected:** Modal.tsx, any component using modals for forms

---

### Lucide React Icons: No Title Prop
**Problem:** TypeScript error when trying to add `title` prop to Lucide icons.

**Error:** `Property 'title' does not exist on type 'LucideProps'`

**Broken pattern:**
```tsx
<CheckCircle title="Has solution endpoint" /> // ❌ Error
```

**Solution:** Wrap the icon in a span with the title:
```tsx
<span title="Has solution endpoint">
  <CheckCircle className="h-4 w-4" />
</span>
```

---

### Tree Traversal: Preventing Infinite Loops with Visited Set
**Problem:** When traversing a tree structure that has cross-references (like `next_node_id` pointing to nodes elsewhere in the tree), you can get infinite loops.

**Solution:** Use a `visited` Set to track already-processed nodes:
```tsx
function hasSolutionInSubtree(
  node: TreeStructure,
  findNode: (id: string) => TreeStructure | null,
  visited: Set<string> = new Set()
): boolean {
  // Prevent infinite loops
  if (visited.has(node.id)) return false
  visited.add(node.id)

  if (node.type === 'solution') return true

  // Check children array
  if (node.children) {
    for (const child of node.children) {
      if (hasSolutionInSubtree(child, findNode, visited)) return true
    }
  }

  // Check next_node_id reference (could point anywhere in tree)
  if (node.next_node_id) {
    const nextNode = findNode(node.next_node_id)
    if (nextNode && hasSolutionInSubtree(nextNode, findNode, visited)) {
      return true
    }
  }

  return false
}
```

**Why it matters:** Decision trees can have shared nodes where multiple paths converge on the same target. Without loop detection, recursive traversal will hang.

---

### SharedLinksMap Pattern for Tracking Cross-References
**Problem:** Need to know which nodes link to the same target node (for showing "shared by X nodes" indicators).

**Solution:** Build a map at the parent component level, pass down to children:
```tsx
// Type definition
type SharedLinksMap = Map<string, Array<{ id: string; label: string }>>

// Build the map by traversing tree once
function buildSharedLinksMap(
  node: TreeStructure,
  map: SharedLinksMap = new Map()
): SharedLinksMap {
  const nodeLabel = node.type === 'decision' ? node.question : node.title

  // Record decision option targets
  if (node.type === 'decision' && node.options) {
    for (const opt of node.options) {
      if (opt.next_node_id) {
        const existing = map.get(opt.next_node_id) || []
        existing.push({ id: node.id, label: nodeLabel || 'Untitled' })
        map.set(opt.next_node_id, existing)
      }
    }
  }

  // Record action next_node_id targets
  if (node.type === 'action' && node.next_node_id) {
    const existing = map.get(node.next_node_id) || []
    existing.push({ id: node.id, label: nodeLabel || 'Untitled' })
    map.set(node.next_node_id, existing)
  }

  // Recurse
  if (node.children) {
    for (const child of node.children) {
      buildSharedLinksMap(child, map)
    }
  }

  return map
}

// Use in parent with useMemo
const sharedLinksMap = useMemo(() => {
  if (!treeStructure) return new Map()
  return buildSharedLinksMap(treeStructure)
}, [treeStructure])
```

**Usage in child:** Check `sharedLinksMap.get(node.id)?.length > 1` to see if node is shared.

---

### tsconfig.json Strict Mode
**Problem:** VS Code shows warnings about missing compiler options.

**Solution:** Add to `compilerOptions` in tsconfig.json:
```json
{
  "compilerOptions": {
    "strict": true,
    "forceConsistentCasingInFileNames": true
  }
}
```

**Why it matters:** `forceConsistentCasingInFileNames` prevents issues when deploying from Windows (case-insensitive) to Linux (case-sensitive).

---

### Tailwind Dark Mode
**Pattern:** Use Tailwind's `dark:` variant for dark mode styling.

**Setup:** In `tailwind.config.js`:
```javascript
module.exports = {
  darkMode: 'class', // or 'media' for system preference only
  // ...
}
```

**Usage:**
```jsx
<div className="bg-white dark:bg-gray-900 text-black dark:text-white">
```

---

## Docker / PostgreSQL

### Accessing PostgreSQL Without Local psql
**Problem:** `psql` command not recognized on Windows (not installed locally).

**Solution:** Use Docker exec to run psql inside the container:
```powershell
# Single command
docker exec -it patherly_postgres psql -U postgres -c "SELECT * FROM users;"

# Interactive session
docker exec -it patherly_postgres psql -U postgres

# Create database
docker exec -it patherly_postgres psql -U postgres -c "CREATE DATABASE patherly_test;"
```

---

### Docker Container Not Running
**Problem:** Database connection errors.

**Solution:** Check and start the container:
```powershell
docker ps  # See running containers
docker start patherly_postgres  # Start if stopped
```

---

## Git / Version Control

### git add from Wrong Directory
**Problem:** `git add .` doesn't stage files in parent directories.

**Solution:** Always run git commands from project root:
```powershell
cd C:\Dev\Projects\patherly
git add .
git commit -m "Your message"
git push origin main
```

---

### Untracked .claude/ Folder
**Problem:** `.claude/` folder appears in untracked files.

**Solution:** Either:
1. Add to `.gitignore` if you don't want to track it
2. Or `git add .claude/` if you want Claude Code settings in repo

---

## Environment-Specific Notes

### Windows Path Handling
- Python and Node handle forward slashes `/` fine on Windows
- Use `os.path.join()` or `pathlib.Path` for cross-platform compatibility
- Avoid hardcoding backslashes `\` in code

### PowerShell vs CMD
- Some Node.js/Python tools work better in CMD than PowerShell
- If a command fails in PowerShell, try CMD or add to Desktop Commander config:
  - `defaultShell: "cmd"`

---

## API / Endpoint Patterns

### JSONB Fields with Timestamps
**Problem:** Storing datetime objects directly in JSONB fields causes serialization errors.

**Solution:** Convert to ISO string before storing:
```python
decision = {
    "node_id": node_id,
    "answer": answer,
    "timestamp": datetime.now(timezone.utc).isoformat()  # String, not datetime
}
```

---

### Soft Delete Pattern
**Pattern:** Use `is_active` boolean instead of actually deleting records.

**Note:** Our schema uses `is_active`, not `is_deleted` (documentation was corrected on Jan 28, 2026).

---

## Testing

### Test Database Setup
**Requirement:** Tests need a separate database.

**One-time setup:**
```powershell
docker exec -it patherly_postgres psql -U postgres -c "CREATE DATABASE patherly_test;"
```

**Run tests:**
```powershell
cd backend
pytest
```

---

## Common Mistakes to Avoid

| Mistake | Correct Approach |
|---------|------------------|
| Using `datetime.utcnow()` | Use `datetime.now(timezone.utc)` |
| Running git from subdirectory | Always `cd` to project root first |
| Forgetting to activate venv | Check for `(venv)` prefix |
| Editing files in OneDrive folder | Use `C:\Dev\Projects\` |
| Using `psql` directly on Windows | Use `docker exec` instead |
| Storing datetime in JSON | Convert to `.isoformat()` string |

---

## Seed Scripts

### httpx Not Installed
**Problem:** Running `python -m scripts.seed_trees` fails with `ModuleNotFoundError: No module named 'httpx'`

**Cause:** The seed script uses `httpx` for async HTTP requests, which isn't in the base requirements.

**Solution:** Install httpx before running seed scripts:
```powershell
pip install httpx
```

---

### Email Validation Rejects .local Domains
**Problem:** Seed script fails with email validation error when using `.local` domain for seed user email.

**Error:** `value is not a valid email address: The part after the @-sign is a special-use or reserved name that cannot be used with email.`

**Cause:** The `email-validator` library (used by Pydantic) correctly rejects `.local` as it's a reserved TLD per RFC 6761.

**Solution:** Use a standard domain like `example.com` for seed/test users:
```python
# BAD
"email": "seed.admin@patherly.local"

# GOOD
"email": "seed.admin@example.com"
```

**Files affected:** `backend/scripts/seed_trees.py`, any test fixtures with email addresses

---

## Railway Deployment

### DATABASE_URL_SYNC Must Derive from DATABASE_URL ⚠️ CRITICAL
**Problem:** Alembic migrations run but connect to localhost instead of Railway's Postgres. Database tables never get created.

**Cause:** `DATABASE_URL_SYNC` was a separate pydantic field with a default value pointing to `localhost:5432`. Railway only provides `DATABASE_URL`, so the sync version wasn't being set correctly.

**Solution:** Make `DATABASE_URL_SYNC` a property that derives from `DATABASE_URL`:
```python
# BAD: Separate field with default
DATABASE_URL_SYNC: str = "postgresql://postgres:postgres@localhost:5432/patherly"

# GOOD: Property that derives from DATABASE_URL
@property
def DATABASE_URL_SYNC(self) -> str:
    """Get sync URL by removing asyncpg prefix from DATABASE_URL."""
    return self.DATABASE_URL.replace("postgresql+asyncpg://", "postgresql://", 1)
```

**Files affected:** `backend/app/core/config.py`

---

### Railway releaseCommand May Not Execute
**Problem:** Migrations in `railway.toml` `releaseCommand` don't run. Deploy logs show the app starting but no migration output.

**Cause:** Railway's `releaseCommand` feature may not work reliably with all configurations, especially with Dockerfile builds.

**Solution:** Run migrations in the Docker CMD instead:
```dockerfile
# Instead of relying on railway.toml releaseCommand:
CMD alembic upgrade head && uvicorn app.main:app --host 0.0.0.0 --port ${PORT:-8000}
```

**Files affected:** `backend/Dockerfile`, `backend/railway.toml` (keep releaseCommand as backup)

---

### Alembic env.py Must Import All Models
**Problem:** Migration runs but table isn't created. Or migration fails with model-related errors.

**Cause:** Alembic's `env.py` only discovers models that are imported. If you add a new model (like `InviteCode`), you must add it to the imports.

**Solution:** Add new models to the import statement in `alembic/env.py`:
```python
# BEFORE: Missing InviteCode
from app.models import User, Team, Tree, Session, Attachment

# AFTER: All models imported
from app.models import User, Team, Tree, Session, Attachment, InviteCode
```

**Files affected:** `backend/alembic/env.py`

---

### New Users Default to "engineer" Role, Not Admin
**Problem:** After registering, user cannot access admin-only endpoints (like creating invite codes). Returns 403 Forbidden.

**Cause:** The User model defaults to `role="engineer"`. The first user isn't automatically made admin.

**Solution:** Manually promote user to admin via Railway's Postgres Query tab:
```sql
UPDATE users SET role = 'admin' WHERE email = 'your@email.com';
```

**Then log out and back in** to get a new JWT with the updated role.

**Files affected:** Database (manual update required for first admin)

---

### Frontend VITE_API_URL Must Use HTTPS, No Port
**Problem:** Frontend gets "Network Error" when trying to call API on Railway.

**Cause:** Common mistakes:
- Using `http://` instead of `https://`
- Including port number (`:8080`) when Railway handles routing
- Typos in variable names (e.g., `FRONTED_URL` vs `FRONTEND_URL`)

**Solution:** Ensure correct format:
```
# WRONG
VITE_API_URL=http://api.patherly.com:8080
VITE_API_URL=http://api.patherly.com

# CORRECT
VITE_API_URL=https://api.patherly.com
```

**Note:** This is a **build-time** variable. After changing it, you must trigger a new frontend build/deploy.

---

### Swagger OAuth2 Password Flow Returns 401
**Problem:** Clicking "Authorize" in Swagger UI with username/password returns 401, even with correct credentials.

**Workaround:** Use the login endpoint directly instead:
1. Call `POST /api/v1/auth/login` with your credentials
2. Copy the `access_token` from the response
3. Click "Authorize" button
4. Paste just the token (no "Bearer" prefix) in the authorization field

**Files affected:** This is a Swagger UI / FastAPI OAuth2 configuration quirk

---

### Seed Script: Use --email/--password Instead of Creating User
**Problem:** Seed script fails when `REQUIRE_INVITE_CODE=true` because it tries to register a new seed user without an invite code.

**Solution:** Modified seed script to accept admin credentials via CLI arguments:
```bash
python -m scripts.seed_trees \
  --api-url https://api.patherly.com/api/v1 \
  --email admin@patherly.com \
  --password "your-password"
```

**Files affected:** `backend/scripts/seed_trees.py`

---

### Seed Script: Use venv Python, Not System Python
**Problem:** Running `python -m scripts.seed_trees` fails with `ModuleNotFoundError: No module named 'httpx'` even after installing httpx.

**Cause:** Installing with system Python (`pip install httpx`) but running with venv Python, or vice versa.

**Solution:** Always use the venv's Python explicitly:
```powershell
# Use venv Python directly
./venv/Scripts/python -m scripts.seed_trees --api-url ... --email ... --password ...

# Or ensure venv is activated first
.\venv\Scripts\Activate
python -m scripts.seed_trees ...
```

**Files affected:** Any script in `backend/scripts/`

---

### Railway Environment Variable Naming
**Problem:** Environment variable isn't being read by the application.

**Checklist:**
- Case-sensitive: `FRONTEND_URL` ≠ `frontend_url` ≠ `Frontend_Url`
- No typos: `FRONTEND_URL` ≠ `FRONTED_URL`
- Empty string vs not set: Setting a variable to empty string is different from not setting it
- Delete stale variables: If you changed a variable from a field to a property (like `DATABASE_URL_SYNC`), delete the Railway variable

**Files affected:** Railway dashboard → Service → Variables tab

---

## Adding New Lessons

When you encounter and fix a bug, add it here with:
1. **Problem:** What error/symptom occurred
2. **Cause:** Why it happened (if known)
3. **Solution:** How to fix it
4. **Files affected:** Where to look (if applicable)