302 lines
9.6 KiB
Markdown
302 lines
9.6 KiB
Markdown
# Architecture Decision Records (ADR)
|
|
|
|
> Copy this file into your project root as `DECISIONS.md`.
|
|
> Use it to document non-obvious architecture choices so agents don't undo them.
|
|
> This is your defense against drift and "helpful improvements."
|
|
|
|
---
|
|
|
|
## Why Architecture Decision Records?
|
|
|
|
The agent harness spawns fresh agents each iteration. Each one starts with zero memory of:
|
|
- **Why** you chose approach X over Y
|
|
- **What** you tried that didn't work
|
|
- **Which** patterns are intentional vs accidental
|
|
|
|
Without ADRs, iteration 10's agent "improves" iteration 3's code in ways that break things. ADRs create continuity across fresh contexts.
|
|
|
|
---
|
|
|
|
## When to Write an ADR
|
|
|
|
Write a decision record when:
|
|
- You choose an unusual approach (curl instead of fetch)
|
|
- You explicitly avoid a "better" solution (no ORM, raw SQL instead)
|
|
- You make a tradeoff (simple+slow over complex+fast)
|
|
- You discover a gotcha with a tool/API (SpringCM headers, DocuSign auth quirks)
|
|
- An agent keeps trying to "fix" something that's working
|
|
|
|
**Not every decision needs an ADR.** Only the non-obvious ones. If the code is self-explanatory, skip the record.
|
|
|
|
---
|
|
|
|
## ADR Template
|
|
|
|
```markdown
|
|
### ADR-NNN: [Short Title]
|
|
**Date:** YYYY-MM-DD
|
|
**Status:** [Proposed | Accepted | Deprecated | Superseded by ADR-XXX]
|
|
|
|
**Context:**
|
|
What's the situation? Why does this decision matter?
|
|
|
|
**Decision:**
|
|
What did you decide? Be specific and actionable.
|
|
|
|
**Consequences:**
|
|
What does this enable? What does it prevent? What are the tradeoffs?
|
|
|
|
**Alternatives Considered:**
|
|
What else did you try or think about? Why didn't you choose them?
|
|
```
|
|
|
|
---
|
|
|
|
## Example ADRs
|
|
|
|
### ADR-001: Use curl over Node.js fetch for HTTP calls
|
|
**Date:** 2024-03-15
|
|
**Status:** Accepted
|
|
|
|
**Context:**
|
|
The DocuSign SpringCM API returns 500 errors when called from Node.js using the native `fetch()` function. The errors are inconsistent — same request works in Postman but fails in Node.
|
|
|
|
Investigation showed that Node's fetch sends extra headers (`Connection: keep-alive`, `Accept-Encoding: gzip, deflate`) that SpringCM's proxy doesn't handle correctly.
|
|
|
|
**Decision:**
|
|
All HTTP calls to SpringCM (and potentially other DocuSign APIs) will use `child_process.exec` with `curl` instead of fetch or axios.
|
|
|
|
```typescript
|
|
// CORRECT
|
|
const result = execSync(`curl -X GET "${url}" -H "Authorization: Bearer ${token}"`,
|
|
{ encoding: 'utf-8' });
|
|
|
|
// DO NOT USE
|
|
const result = await fetch(url, { headers: { Authorization: `Bearer ${token}` }});
|
|
```
|
|
|
|
**Consequences:**
|
|
✅ API calls work reliably
|
|
✅ Easier to debug (can copy curl command to terminal)
|
|
❌ Slightly less "Node-native" (using shell commands)
|
|
❌ Must escape shell arguments carefully
|
|
|
|
**Alternatives Considered:**
|
|
- **Axios with minimal headers:** Still sent headers that broke SpringCM
|
|
- **Got library:** Same issue as axios
|
|
- **Manual request crafting:** Too complex, curl is simpler
|
|
|
|
**Notes:**
|
|
If this decision becomes annoying (shell escaping hell), consider writing a thin wrapper:
|
|
```typescript
|
|
function curlGet(url: string, token: string): string {
|
|
const safeUrl = shellEscape(url);
|
|
return execSync(`curl -X GET ${safeUrl} -H "Authorization: Bearer ${token}"`,
|
|
{ encoding: 'utf-8' });
|
|
}
|
|
```
|
|
|
|
---
|
|
|
|
### ADR-002: Shared package for cross-cutting utilities
|
|
**Date:** 2024-03-17
|
|
**Status:** Accepted
|
|
|
|
**Context:**
|
|
Seven packages (`clm-direct`, `docgen-direct`, `maestro-direct`, `template-direct`, `formbuilder-direct`, `springcm-direct`, `powerforms-direct`) had duplicated code for:
|
|
- JWT authentication
|
|
- Environment variable loading
|
|
- API error handling
|
|
- Retry logic
|
|
|
|
Changes required updating 7 files. Tests were inconsistent across packages.
|
|
|
|
**Decision:**
|
|
Extract shared utilities to `packages/shared/` and import as `docusign-direct-shared`.
|
|
|
|
```typescript
|
|
// Before (duplicated in each package)
|
|
const token = process.env.DOCUSIGN_TOKEN;
|
|
if (!token) throw new Error("Missing token");
|
|
|
|
// After (shared utility)
|
|
import { requireEnv } from 'docusign-direct-shared';
|
|
const token = requireEnv('DOCUSIGN_TOKEN');
|
|
```
|
|
|
|
**Consequences:**
|
|
✅ Single source of truth for auth logic
|
|
✅ Consistent error messages across packages
|
|
✅ Tests only need to cover shared code once
|
|
❌ Adds a build-time dependency (shared must build first)
|
|
❌ Breaking changes in shared affect all packages
|
|
|
|
**Alternatives Considered:**
|
|
- **Copy-paste approach:** Current state, unacceptable for long-term maintenance
|
|
- **Monolithic package:** All functionality in one big package — loses clarity of API-specific packages
|
|
- **External npm package:** Overkill for this codebase, adds publish/version complexity
|
|
|
|
**Migration:**
|
|
New packages MUST use shared utilities. Existing packages should migrate opportunistically (when touching auth code anyway, switch to shared).
|
|
|
|
---
|
|
|
|
### ADR-003: No ORM — Raw SQL with better-sqlite3
|
|
**Date:** 2024-04-02
|
|
**Status:** Accepted
|
|
|
|
**Context:**
|
|
Early iterations suggested using Prisma or TypeORM for database access. The schema is simple (4 tables: users, transactions, categories, rules). Most queries are straightforward CRUD or aggregations.
|
|
|
|
ORMs add:
|
|
- Build complexity (Prisma requires codegen)
|
|
- Migration complexity (Prisma's migration format vs raw SQL)
|
|
- Learning curve (Prisma's query API vs SQL)
|
|
- Bundle size (~50KB for Prisma client)
|
|
|
|
**Decision:**
|
|
Use `better-sqlite3` with raw parameterized SQL queries. No ORM.
|
|
|
|
```typescript
|
|
// Query example
|
|
const transactions = db.prepare(`
|
|
SELECT * FROM transactions
|
|
WHERE account_id = ? AND date >= ? AND date <= ?
|
|
ORDER BY date DESC
|
|
`).all(accountId, startDate, endDate);
|
|
|
|
// Migration example (migrations/001-initial-schema.sql)
|
|
CREATE TABLE transactions (
|
|
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
|
account_id INTEGER NOT NULL,
|
|
date TEXT NOT NULL,
|
|
amount REAL NOT NULL,
|
|
...
|
|
);
|
|
```
|
|
|
|
**Consequences:**
|
|
✅ Simple: SQL is explicit, no query builder magic
|
|
✅ Fast: better-sqlite3 is one of the fastest SQLite bindings
|
|
✅ No build step for database code
|
|
✅ Migrations are just SQL files (easy to review and version)
|
|
❌ No automatic type generation from schema
|
|
❌ Must manually write types (acceptable for 4 tables)
|
|
|
|
**Alternatives Considered:**
|
|
- **Prisma:** Overkill for this schema size, adds complexity
|
|
- **TypeORM:** Active Record pattern feels wrong for this use case
|
|
- **Knex:** Query builder without ORM — middle ground, but still adds abstraction
|
|
|
|
**Review after:**
|
|
If the schema grows to 15+ tables with complex relationships, revisit this decision. For now, raw SQL is the right fit.
|
|
|
|
---
|
|
|
|
### ADR-004: Monte Carlo in main thread, not Web Workers
|
|
**Date:** 2024-04-10
|
|
**Status:** Accepted
|
|
|
|
**Context:**
|
|
Monte Carlo simulation runs 1,000+ iterations. Each iteration:
|
|
- Samples random market returns
|
|
- Calculates portfolio balance year by year (30+ years)
|
|
- Tracks success/failure
|
|
|
|
Initial implementation in a Web Worker for non-blocking UI. However:
|
|
- Simulation completes in < 2 seconds on modern hardware
|
|
- Complexity of Worker setup (separate build, message passing) adds 100 LOC
|
|
- User expectation: click "Run Simulation" → see results immediately (not async)
|
|
|
|
**Decision:**
|
|
Run Monte Carlo in the main thread. Show a loading spinner during execution.
|
|
|
|
```typescript
|
|
// Main thread, synchronous
|
|
function runMonteCarlo(profile: RetirementProfile, runs: number): SimulationResult {
|
|
const results = [];
|
|
for (let i = 0; i < runs; i++) {
|
|
results.push(runSingleSimulation(profile));
|
|
}
|
|
return aggregateResults(results);
|
|
}
|
|
|
|
// UI
|
|
button.onclick = () => {
|
|
showSpinner();
|
|
const result = runMonteCarlo(profile, 1000);
|
|
hideSpinner();
|
|
renderResults(result);
|
|
};
|
|
```
|
|
|
|
**Consequences:**
|
|
✅ Simpler code (no Worker setup, no message passing)
|
|
✅ Easier to debug (single execution context)
|
|
✅ Fast enough (<2s) that blocking isn't a problem
|
|
❌ UI freezes for ~1.5 seconds during simulation
|
|
❌ Can't run multiple simulations in parallel
|
|
|
|
**Alternatives Considered:**
|
|
- **Web Worker:** Complexity not justified for 1.5s task
|
|
- **WebAssembly:** Overkill, JS is fast enough
|
|
- **Async/await with chunking:** Adds complexity, still blocks event loop
|
|
|
|
**Review after:**
|
|
If simulation time grows to >5 seconds (e.g., 10,000 runs, more complex models), reconsider Web Workers.
|
|
|
|
---
|
|
|
|
## Tips for Writing Good ADRs
|
|
|
|
### 1. Write them when the decision is fresh
|
|
Don't wait until the project is done. Write the ADR immediately after you make the choice. You'll remember the reasoning.
|
|
|
|
### 2. Include what DIDN'T work
|
|
"We tried fetch — it sent headers that broke SpringCM" is more valuable than "We use curl." Future agents need to know the trap.
|
|
|
|
### 3. Be specific with code examples
|
|
Show the DO and DON'T patterns. Code is clearer than prose.
|
|
|
|
### 4. Include review triggers
|
|
"Review this after the schema grows to 15+ tables" gives future-you permission to change course.
|
|
|
|
### 5. Status field matters
|
|
- **Proposed:** Still discussing
|
|
- **Accepted:** This is the way
|
|
- **Deprecated:** Don't use this anymore (but still in codebase)
|
|
- **Superseded by ADR-XXX:** This approach was replaced
|
|
|
|
### 6. Keep them short
|
|
An ADR should fit on one screen. If it's a 5-page essay, you're over-explaining. Extract detail to separate docs and link them.
|
|
|
|
---
|
|
|
|
## Maintaining ADRs
|
|
|
|
### Add to AGENT.md Orient Phase
|
|
```markdown
|
|
## Orient
|
|
- Read PROJECT-SPEC.md
|
|
- Read IMPLEMENTATION_PLAN.md
|
|
- Read DECISIONS.md ← Add this
|
|
- Check git log --oneline -5
|
|
```
|
|
|
|
### Reference in Constraints
|
|
```markdown
|
|
## Constraints
|
|
- MUST follow architecture decisions documented in DECISIONS.md
|
|
- MUST NOT change approaches listed as "Accepted" without human approval
|
|
```
|
|
|
|
### Review Periodically
|
|
Every few weeks, scan DECISIONS.md:
|
|
- Are the decisions still valid?
|
|
- Should any be deprecated?
|
|
- Are agents following them?
|
|
|
|
---
|
|
|
|
_Decisions don't drift if they're written down._
|