docs: consolidate LESSONS-LEARNED.md into CLAUDE.md, single source of truth
Merged 8 unique lessons (#30-#37) from LESSONS-LEARNED.md into CLAUDE.md: Alembic env.py imports, JSONB datetime serialization, export pipeline order, Railway deployment gotchas, bcrypt pinning, email validator TLD, first admin promotion. Deleted LESSONS-LEARNED.md and updated all references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
chihlasm
22
CLAUDE.md
22
CLAUDE.md
@@ -114,7 +114,7 @@ patherly/
|
||||
│ └── tailwind.config.js
|
||||
├── CLAUDE.md # This file
|
||||
├── CURRENT-STATE.md # Detailed feature status
|
||||
├── LESSONS-LEARNED.md # Bugs and fixes (READ THIS!)
|
||||
├── LESSONS-LEARNED.md # (Deprecated — consolidated into CLAUDE.md)
|
||||
└── docs/plans/ # Design docs & implementation plans
|
||||
```
|
||||
|
||||
@@ -194,8 +194,6 @@ gh run view <id> --json jobs --jq '.jobs[] | {name: .name, conclusion: .conclusi
|
||||
|
||||
## Critical Lessons Learned
|
||||
|
||||
**Full reference:** [LESSONS-LEARNED.md](LESSONS-LEARNED.md) — read before making changes!
|
||||
|
||||
### Top Gotchas (most commonly hit)
|
||||
|
||||
**1. DateTime Handling — Always timezone-aware:**
|
||||
@@ -290,6 +288,22 @@ navigate(`/trees/${newTree.id}/edit`)
|
||||
|
||||
**29. ESLint `no-unused-vars` has no `argsIgnorePattern`:** Underscore-prefixed callback params (e.g., `_count`) still trigger errors. Use `// eslint-disable-next-line @typescript-eslint/no-unused-vars` or remove the param if the type signature allows it.
|
||||
|
||||
**30. Alembic `env.py` must import all models:** New models won't be discovered by `--autogenerate` unless imported in `alembic/env.py`. If a migration runs but the table isn't created, check imports first.
|
||||
|
||||
**31. JSONB fields — convert datetime to `.isoformat()` string:** Storing `datetime` objects directly in JSONB columns causes serialization errors. Always convert: `"timestamp": datetime.now(timezone.utc).isoformat()`.
|
||||
|
||||
**32. Export pipeline order matters:** Generate export → resolve session variables → apply redaction. Redaction MUST run last or sensitive data injected via variables bypasses it. See `sessions.py` export endpoint.
|
||||
|
||||
**33. Railway: `DATABASE_URL_SYNC` is a property, not an env var:** It derives from `DATABASE_URL` by replacing `postgresql+asyncpg://` with `postgresql://`. Delete any stale `DATABASE_URL_SYNC` variable in Railway dashboard.
|
||||
|
||||
**34. Railway: run migrations in Docker CMD, not `releaseCommand`:** `releaseCommand` in `railway.toml` is unreliable. Use `CMD alembic upgrade head && uvicorn ...` in the Dockerfile instead.
|
||||
|
||||
**35. `bcrypt==4.0.1` pinned for passlib compatibility:** Newer bcrypt versions break password hashing. Keep `bcrypt==4.0.1` and `passlib[bcrypt]==1.7.4` pinned in `requirements.txt`.
|
||||
|
||||
**36. Email validator rejects `.local` TLD:** Pydantic's email validation (via `email-validator`) rejects `.local` as a reserved TLD. Use `example.com` for test/seed user emails.
|
||||
|
||||
**37. First deployed user needs manual admin promotion:** New users default to `engineer` role. Promote via SQL: `UPDATE users SET role = 'admin' WHERE email = '...';` then re-login for new JWT.
|
||||
|
||||
---
|
||||
|
||||
## RBAC & Permissions
|
||||
@@ -392,6 +406,6 @@ When a feature, fix, or significant piece of work is finished and merged/committ
|
||||
| Detailed Status | [CURRENT-STATE.md](CURRENT-STATE.md) |
|
||||
| Development Roadmap | [03-DEVELOPMENT-ROADMAP.md](03-DEVELOPMENT-ROADMAP.md) |
|
||||
| GitHub Issues | `gh issue list --state open` |
|
||||
| Bugs & Fixes | [LESSONS-LEARNED.md](LESSONS-LEARNED.md) |
|
||||
| Bugs & Fixes | CLAUDE.md → Critical Lessons Learned section |
|
||||
| Feature Specs | [04-FEATURE-SPECIFICATIONS.md](04-FEATURE-SPECIFICATIONS.md) |
|
||||
| Design System | [docs/plans/Frontend/DESIGN_SYSTEM_GUIDE.md](docs/plans/Frontend/DESIGN_SYSTEM_GUIDE.md) |
|
||||
|
||||
@@ -75,7 +75,7 @@
|
||||
|
||||
### Documentation
|
||||
- CLAUDE.md (project context for Claude Code)
|
||||
- LESSONS-LEARNED.md (bugs and fixes reference)
|
||||
- CLAUDE.md includes consolidated lessons learned (formerly LESSONS-LEARNED.md)
|
||||
- Design system guide, component examples
|
||||
- Feature specifications through Phase 4
|
||||
- Rebrand implementation guide
|
||||
|
||||
@@ -1,932 +0,0 @@
|
||||
# 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 12, 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
|
||||
|
||||
---
|
||||
|
||||
## SQLAlchemy / Database
|
||||
|
||||
### Two Foreign Keys to Same Table Require `foreign_keys=` ⚠️ CRITICAL
|
||||
**Problem:** SQLAlchemy raises `AmbiguousForeignKeysError` when a model has two ForeignKey columns pointing to the same table (e.g., `deleted_by` and `author_id` both referencing `users.id`).
|
||||
|
||||
**Cause:** SQLAlchemy can't figure out which FK to use for each relationship.
|
||||
|
||||
**Solution:** Add `foreign_keys=` parameter on BOTH relationship sides:
|
||||
```python
|
||||
# In User model
|
||||
deleted_by: Mapped[Optional[uuid.UUID]] = mapped_column(
|
||||
UUID(as_uuid=True), ForeignKey("users.id"), nullable=True
|
||||
)
|
||||
|
||||
# In Tree model with two user FKs
|
||||
author: Mapped["User"] = relationship("User", foreign_keys=[author_id])
|
||||
deleted_by_user: Mapped[Optional["User"]] = relationship("User", foreign_keys=[deleted_by])
|
||||
```
|
||||
|
||||
**Files affected:** Any model with multiple FK references to the same table (User, Tree)
|
||||
|
||||
---
|
||||
|
||||
### Circular FK Between Tables Requires Nullable FK
|
||||
**Problem:** `Account.owner_id → users.id` and `User.account_id → accounts.id` creates a circular dependency. `CREATE TABLE` fails because neither table can be created first.
|
||||
|
||||
**Cause:** Both tables reference each other, so neither can exist before the other.
|
||||
|
||||
**Solution:** Make one FK nullable and set it after both records exist:
|
||||
```python
|
||||
# Create Account first (owner_id=NULL)
|
||||
account = Account(name="My Account")
|
||||
db.add(account)
|
||||
await db.flush()
|
||||
|
||||
# Create User with account_id
|
||||
user = User(account_id=account.id, ...)
|
||||
db.add(user)
|
||||
await db.flush()
|
||||
|
||||
# Now set owner_id
|
||||
account.owner_id = user.id
|
||||
await db.commit()
|
||||
```
|
||||
|
||||
**Files affected:** `backend/app/models/account.py`, registration endpoint
|
||||
|
||||
---
|
||||
|
||||
## Authentication / Security
|
||||
|
||||
### Backend Enforcement of `must_change_password` via Dependency Injection
|
||||
**Problem:** Need to force users to change their password before accessing any endpoint, but some endpoints (like the change-password endpoint itself) must be exempt.
|
||||
|
||||
**Solution:** Add a check in `get_current_active_user` dependency with a route allowlist:
|
||||
```python
|
||||
async def get_current_active_user(request: Request, ...):
|
||||
user = ... # existing auth logic
|
||||
|
||||
# Check must_change_password (with route allowlist)
|
||||
if user.must_change_password:
|
||||
allowed = ["/auth/password/change", "/auth/logout", "/auth/me"]
|
||||
if not any(request.url.path.endswith(p) for p in allowed):
|
||||
raise HTTPException(403, detail="password_change_required")
|
||||
|
||||
return user
|
||||
```
|
||||
|
||||
**Key insight:** This requires adding `request: Request` parameter to the dependency. The frontend checks the 403 detail string and redirects to `/change-password`.
|
||||
|
||||
**Files affected:** `backend/app/api/deps.py`, `frontend/src/components/layout/ProtectedRoute.tsx`
|
||||
|
||||
---
|
||||
|
||||
### Password Reset Tokens: DB-Backed Single-Use via Hashed JTI
|
||||
**Pattern:** Password reset tokens use JWT for transport but are tracked in the database for single-use enforcement.
|
||||
|
||||
**How it works:**
|
||||
1. Generate JWT with `type: "password_reset"`, `jti: uuid4()`, `exp: 30min`
|
||||
2. Store `SHA-256(jti)` in `password_reset_tokens` table
|
||||
3. On reset: decode JWT → hash jti → look up in DB → check `used_at IS NULL`
|
||||
4. After use: set `used_at = now()` to prevent reuse
|
||||
|
||||
**Why not just JWT?** JWTs are stateless — you can't revoke them. DB tracking ensures each token is used exactly once.
|
||||
|
||||
**Files affected:** `backend/app/models/password_reset_token.py`, `backend/app/core/security.py`
|
||||
|
||||
---
|
||||
|
||||
### Anti-Enumeration on Forgot Password Endpoint
|
||||
**Pattern:** The `POST /auth/password/forgot` endpoint always returns the same generic success message, regardless of whether the email exists.
|
||||
|
||||
**Why:** If the endpoint returned "email not found" for non-existent users, an attacker could use it to enumerate valid email addresses.
|
||||
|
||||
```python
|
||||
# Always return success, even if email doesn't exist
|
||||
return {"message": "If an account exists with that email, a reset link has been sent."}
|
||||
```
|
||||
|
||||
**Files affected:** `backend/app/api/endpoints/auth.py`
|
||||
|
||||
---
|
||||
|
||||
## Seed Scripts
|
||||
|
||||
### Seed Tree Data Missing Required Validation Fields
|
||||
**Problem:** All 18 seed trees fail to create with validation errors: "Action nodes must have a non-empty action" and "Solution nodes must have a non-empty solution".
|
||||
|
||||
**Cause:** The tree validation (added after the seed scripts were written) requires:
|
||||
- Action nodes: `action` field (not just `description`)
|
||||
- Solution nodes: `solution` field (not just `description`)
|
||||
- Decision nodes with children: at least 2 branches
|
||||
|
||||
The seed data uses `title` and `description` but not the specific `action`/`solution` fields.
|
||||
|
||||
**Solution:** Add a recursive `_fix_node_fields()` function in the seeder that patches nodes before sending to the API:
|
||||
```python
|
||||
def _fix_node_fields(node):
|
||||
if node["type"] == "action" and not node.get("action"):
|
||||
node["action"] = node.get("title") or node.get("description") or "Action"
|
||||
elif node["type"] == "solution" and not node.get("solution"):
|
||||
node["solution"] = node.get("title") or node.get("description") or "Solution"
|
||||
elif node["type"] == "decision":
|
||||
children = node.get("children", [])
|
||||
if len(children) == 1: # Add fallback branch
|
||||
children.append({"id": "..._fallback", "type": "solution", ...})
|
||||
for child in node.get("children", []):
|
||||
_fix_node_fields(child)
|
||||
```
|
||||
|
||||
**Files affected:** `backend/scripts/seed_trees_v2.py`
|
||||
|
||||
---
|
||||
|
||||
## Admin Features
|
||||
|
||||
### Two-Step Hard Delete Pattern
|
||||
**Pattern:** Hard-deleting a user requires two API calls — a precheck followed by the actual delete.
|
||||
|
||||
1. `GET /admin/users/{id}/hard-delete-check` → returns `{can_delete: bool, blockers: {owned_accounts: 3, sessions: 12, ...}}`
|
||||
2. `DELETE /admin/users/{id}/hard-delete` → only succeeds if user is archived AND precheck passes
|
||||
|
||||
**Pre-conditions enforced:**
|
||||
- User must be archived (`deleted_at IS NOT NULL`) before hard delete
|
||||
- User must have no blockers (owned accounts, authored trees, sessions, audit logs, invite codes)
|
||||
- Cannot delete yourself or other super admins
|
||||
- Audit log entry created BEFORE the delete (since user won't exist after)
|
||||
|
||||
**Files affected:** `backend/app/api/endpoints/admin.py`
|
||||
|
||||
---
|
||||
|
||||
### Admin User Creation with Temp Password (M365-Style)
|
||||
**Pattern:** Admin creates a user → gets a one-time temp password → user must change on first login.
|
||||
|
||||
**Key design decisions:**
|
||||
- Temp password is generated server-side (`secrets`-based, 16 chars, guaranteed complexity)
|
||||
- Temp password is returned once in the response, never stored as plaintext
|
||||
- `must_change_password=True` is set on the user, enforced at the dependency level
|
||||
- Welcome email with temp password is best-effort (never blocks user creation)
|
||||
- Two modes: "existing account" (join with role) or "personal" (new account created)
|
||||
|
||||
**Files affected:** `backend/app/api/endpoints/admin.py`, `backend/app/core/security.py`
|
||||
|
||||
---
|
||||
|
||||
## Export / Redaction
|
||||
|
||||
### CORS `expose_headers` Required for Custom Response Headers
|
||||
**Problem:** Frontend reads custom response headers (e.g. `X-Redaction-Summary`) but gets `undefined` — the header exists in the response but Axios can't access it.
|
||||
|
||||
**Cause:** Browsers enforce CORS restrictions on response headers. Only "CORS-safelisted" headers (Cache-Control, Content-Type, etc.) are accessible by default. Custom headers require explicit exposure.
|
||||
|
||||
**Solution:** Add `expose_headers` to CORS middleware in `main.py` (both branches):
|
||||
```python
|
||||
app.add_middleware(
|
||||
CORSMiddleware,
|
||||
allow_origins=settings.allowed_origins,
|
||||
expose_headers=["X-Redaction-Mode", "X-Redaction-Summary"],
|
||||
...
|
||||
)
|
||||
```
|
||||
|
||||
**Files affected:** `backend/app/main.py` (both CORS middleware branches)
|
||||
|
||||
---
|
||||
|
||||
### Redaction Must Run AFTER Variable Resolution
|
||||
**Problem:** Sensitive data injected via session variables (e.g. `{{client_ip}}` → `192.168.1.1`) would bypass redaction if it ran before variable substitution.
|
||||
|
||||
**Solution:** Export pipeline order matters:
|
||||
1. Generate export by format (markdown/html/text/psa)
|
||||
2. Resolve session variables
|
||||
3. Apply redaction (if `redaction_mode == "mask"`)
|
||||
|
||||
**Key file:** `backend/app/api/endpoints/sessions.py` — the redaction block is placed after both generation and variable resolution, with fail-closed error handling (500 on failure, never return unredacted content).
|
||||
|
||||
---
|
||||
|
||||
## 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)
|
||||
@@ -5,8 +5,8 @@ Use this for personal thoughts, todos, and reminders.
|
||||
|
||||
## 📝 Quick Thoughts
|
||||
|
||||
- Read CLAUDE-SETUP.md, CURRENT-STATE.md, and LESSONS-LEARNED.md before starting work on [your task].
|
||||
- Update CURRENT-STATE.md with what was completed and update LESSONS-LEARNED.md if we encountered any new issues.
|
||||
- Read CLAUDE.md and CURRENT-STATE.md before starting work on [your task].
|
||||
- Update CURRENT-STATE.md with what was completed and update CLAUDE.md if we encountered any new issues.
|
||||
-
|
||||
|
||||
## ✅ Personal TODO
|
||||
|
||||
Reference in New Issue
Block a user