Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# CLAUDE.md
# CLAUDE.md

This file provides guidance to Claude Code when working with code in this repository.

Expand Down
2 changes: 1 addition & 1 deletion next-env.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/// <reference types="next" />
/// <reference types="next/image-types/global" />
/// <reference path="./.next/types/routes.d.ts" />
import "./.next/types/routes.d.ts";

// NOTE: This file should not be edited
// see https://nextjs.org/docs/app/api-reference/config/typescript for more information.
34 changes: 34 additions & 0 deletions specs/004-dynamic-rendering/checklists/requirements.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Specification Quality Checklist: Force Dynamic Page Rendering

**Purpose**: Validate specification completeness and quality before proceeding to planning
**Created**: 2026-03-19
**Feature**: [spec.md](../spec.md)

## Content Quality

- [x] No implementation details (languages, frameworks, APIs)
- [x] Focused on user value and business needs
- [x] Written for non-technical stakeholders
- [x] All mandatory sections completed

## Requirement Completeness

- [x] No [NEEDS CLARIFICATION] markers remain
- [x] Requirements are testable and unambiguous
- [x] Success criteria are measurable
- [x] Success criteria are technology-agnostic (no implementation details)
- [x] All acceptance scenarios are defined
- [x] Edge cases are identified
- [x] Scope is clearly bounded
- [x] Dependencies and assumptions identified

## Feature Readiness

- [x] All functional requirements have clear acceptance criteria
- [x] User scenarios cover primary flows
- [x] Feature meets measurable outcomes defined in Success Criteria
- [x] No implementation details leak into specification

## Notes

- All items pass. Specification is ready for `/speckit.clarify` or `/speckit.plan`.
9 changes: 9 additions & 0 deletions specs/004-dynamic-rendering/data-model.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Data Model: Force Dynamic Page Rendering

**Feature**: 004-dynamic-rendering | **Date**: 2026-03-19

## No Changes

This feature is a rendering configuration change only. No database schema modifications, no new entities, no changed relationships, no new validation rules, and no state transitions.

The existing data model (holdings, asset profiles, enrichment pipeline) is unaffected. All existing data-fetching functions (`getHoldings`, `createHolding`, `updateHolding`, `deleteHolding`) continue to work identically — they are simply invoked at request time instead of build time.
150 changes: 150 additions & 0 deletions specs/004-dynamic-rendering/design-meeting-20260319-171800.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
# Design Meeting Summary
## Force Dynamic Page Rendering

---

## Meeting Metadata
- **Feature Branch**: 004-dynamic-rendering
- **Date**: 2026-03-19 17:18 UTC
- **Participants**: Product Owner, Business Sponsor, Scrum Master, UX Designer, Technical Lead
- **Method**: Multi-agent stakeholder review

---

## Executive Summary

### Amendments Required
1. **Concern 1 — Runtime DB assumption conflicts with edge cases**: Remove or narrow the assumption "database is always available at runtime" to eliminate contradiction with the edge cases section that acknowledges runtime unavailability. Optionally add one acceptance scenario covering what the user sees when the DB is unavailable at runtime.
2. **Concern 2 — Unverified page coverage scope**: Add a pre-implementation verification step to confirm which pages access the database, rather than carrying `(app)` route group coverage as an unchecked assumption.
3. **Concern 3 — SC-004 not falsifiable**: Add a note that SC-004 ("no pages that access the database are statically generated") should include a verification mechanism, even if the specific approach is left to the implementer.

### Resolved
1. **Concern 4 — Loading state / latency UX**: No spec amendment needed. The spec's explicit assumption that "a small increase in request latency is acceptable given the single-user nature of the application" is sufficient. Unanimous agreement.

---

## Concern Register

| ID | Severity | Raised By | Spec Item | Description | Resolution |
|----|----------|-----------|-----------|-------------|------------|
| C1 | MEDIUM | PO, BS, SM, UX, TL | Assumptions / Edge Cases | Runtime DB unavailability acknowledged but deferred to unverified "existing error handling"; assumption contradicts edge case | AMENDED |
| C2 | MEDIUM | PO, SM | FR-003 / Assumptions | `(app)` route group assumed to cover all DB-accessing pages without verification | AMENDED |
| C3 | LOW | SM, TL | SC-003 / SC-004 | No new test coverage required; SC-004 has no verification mechanism | AMENDED |
| C4 | LOW | UX | User Story 2 / Assumptions | No loading state or latency threshold defined for dynamic rendering | RESOLVED |

---

# Detailed Discussion

## Opening Statements

### Product Owner
**Positives**: The spec is tight and well-scoped. The problem statement is clear, the two user stories map directly to the stated problem, and the acceptance scenarios are concrete and independently testable. The assumptions section is unusually honest — explicitly acknowledging the latency trade-off and single-user context prevents scope creep later.

**Concerns**:
- CONCERN: Edge Cases section — runtime database unavailability is acknowledged but deferred to "existing error handling behavior" without verifying that behavior is adequate — Impact: If existing error handling is insufficient, the feature ships with a silent regression that only surfaces in production — Severity: MEDIUM
- CONCERN: FR-003 / Assumptions — the spec assumes the `(app)` route group covers all database-accessing pages, but this is stated as an assumption rather than a verified fact — Impact: If any page outside that route group also accesses the database, it will be missed and the build will still fail — Severity: HIGH

### Business Sponsor
**Positives**: The spec is tightly scoped to a real, blocking problem with a clear business rationale: the app cannot deploy at all without this fix. The assumption that a small latency increase is acceptable for a single-user application is correctly called out and saves scope-creep debates later.

**Concerns**:
- CONCERN: Assumption "database always available at runtime" — does not define what "available" means or who is responsible for ensuring it — Impact: Runtime failures may appear to be application bugs rather than infrastructure issues — Severity: LOW

### Scrum Master
**Positives**: The spec is tightly scoped and problem-driven. The input statement maps cleanly to the user stories and requirements. Both user stories include explicit independent test descriptions, enabling sprint-level acceptance without full integration dependency.

**Concerns**:
- CONCERN: Assumptions bullet 1 — "database is always available at runtime" conflicts with edge case acknowledging runtime unavailability. These two positions are in tension. — Impact: Ambiguity could cause over-engineering or under-engineering of error handling — Severity: MEDIUM
- CONCERN: SC-003 — No mention of new tests for the dynamic rendering behavior itself. If treated as the only test obligation, the rendering change ships with no direct test coverage. — Impact: Regression risk if future changes re-introduce static rendering — Severity: LOW

### UX Designer
**Positives**: The spec clearly articulates the user-facing impact of data freshness and correctly identifies financial data staleness as a real user concern. The edge cases section is thorough and covers the right scenarios from a user-facing perspective.

**Concerns**:
- CONCERN: Edge Cases — Runtime database unavailability UX is deferred, not defined. A user mid-session could lose context with no recovery path or message. — Severity: MEDIUM
- CONCERN: User Story 2 / SC-002 — No loading state behavior specified for dynamic rendering latency. Users may perceive sluggishness on slower connections. — Severity: LOW

### Technical Lead
**Positives**: The spec is well-scoped and the problem statement is clear. The runtime availability assumption is explicitly stated as load-bearing. FR-003 correctly carves out non-database pages, avoiding unnecessary over-engineering.

**Concerns**:
- CONCERN: Assumptions — "Database is always available at runtime" has no corresponding acceptance scenario. Error behavior is undefined and untested. — Severity: MEDIUM
- CONCERN: SC-004 — No specified mechanism for verifying "no pages that access the database are statically generated" at CI time. Could silently regress. — Severity: LOW

---

## Debate Highlights

**Concern 2 — Page coverage severity (HIGH vs MEDIUM):**
- Tech Lead challenged Product Owner's HIGH severity rating, arguing that any out-of-group database-accessing page would produce an immediate, visible build failure (the same symptom as the current bug), so the risk of silent regression is low. A pre-implementation route audit is sufficient without amending the spec.
- Product Owner maintained HIGH, arguing this is the core deliverable and the spec should not carry an unverified assumption into implementation.
- Scrum Master supported Product Owner's HIGH rating from a sprint planning standpoint: if the assumption is wrong, the sprint goal fails on first CI run.
- Resolution: AMENDED at MEDIUM severity — the concern is valid and warrants a verification step, but the failure mode is visible rather than silent.

**Concern 1 — Runtime DB unavailability:**
- All five participants raised this independently. Business Sponsor and Tech Lead argued RESOLVED (defer to existing error handling; out of scope for this fix). Scrum Master and UX Designer argued AMENDED (the assumption contradicts the edge case; at minimum remove the contradiction). Product Owner's original concern aligned with amendment.
- Scrum Master proposed removing or narrowing the assumption. UX Designer proposed adding one acceptance scenario.
- Resolution: AMENDED — remove or narrow the conflicting assumption; optionally add one acceptance scenario.

**Concern 4 — Loading state UX:**
- Business Sponsor rejected this concern as over-engineering for a single-user app. Product Owner agreed, noting the change affects when data is fetched, not how it is presented. UX Designer conceded, acknowledging the single-user context makes this a non-issue.
- Resolution: RESOLVED unanimously.

**Concern 3 — CI gate for SC-004:**
- Scrum Master argued SC-004 as written is aspirational rather than falsifiable. Tech Lead accepted the risk as LOW and preferred tracking as a follow-on. Product Owner deferred to Tech Lead on implementation cost.
- Resolution: AMENDED at LOW severity — add a note that SC-004 should include a verification step.

---

## Concern Resolutions

### C1: Runtime database unavailability — undefined error behavior
**Severity:** MEDIUM
**Raised by:** All participants
**Spec Item:** Assumptions (bullet 1) / Edge Cases

**Debate Summary:**
All five participants flagged this concern independently. The core issue is a contradiction: the Assumptions section states the database is "always available at runtime," while the Edge Cases section acknowledges runtime unavailability and defers to "existing error handling behavior." Business Sponsor and Tech Lead argued this is out of scope for a deployment-fix feature and should be RESOLVED. Scrum Master argued the conflicting language creates a planning trap — the team will interpret scope differently depending on which section they read. UX Designer argued that without at least one acceptance scenario, there is no way to verify the error experience is acceptable.

**Resolution:** AMENDED
Remove or narrow the assumption "database is always available at runtime" to eliminate the contradiction with the Edge Cases section. The assumption could be reworded to "the database is expected to be available at runtime; transient unavailability is handled by existing error states." Optionally, add one acceptance scenario covering the user experience when the database is temporarily unavailable at runtime.

---

### C2: Unverified page coverage scope
**Severity:** MEDIUM (downgraded from HIGH after debate)
**Raised by:** Product Owner (HIGH), supported by Scrum Master (HIGH)
**Spec Item:** FR-003 / Assumptions (bullet 4)

**Debate Summary:**
Product Owner raised this at HIGH severity, arguing the spec should not carry an unverified assumption about which pages access the database. Scrum Master supported HIGH from a sprint planning perspective. Tech Lead challenged the severity, arguing that any missed page would cause an immediate, visible build failure (the same symptom as the current bug), so the failure mode is not silent. The risk is real but the blast radius is contained.

**Resolution:** AMENDED
Add a pre-implementation verification step to confirm which pages access the database, rather than carrying `(app)` route group coverage as an unchecked assumption. Severity downgraded to MEDIUM because the failure mode is visible, not silent. The spec should note that a route audit is required before implementation begins.

---

### C3: No CI gate or new test coverage for dynamic rendering
**Severity:** LOW
**Raised by:** Scrum Master, Technical Lead
**Spec Item:** SC-003 / SC-004

**Debate Summary:**
Scrum Master noted SC-003 only requires existing tests to pass, with no new test obligation for the rendering change itself. Tech Lead noted SC-004 has no mechanism for CI-time verification. Both rated LOW. Product Owner deferred to Tech Lead on implementation cost. Scrum Master argued SC-004 is currently aspirational rather than falsifiable.

**Resolution:** AMENDED
Add a note to SC-004 that the criterion should be verified through a defined mechanism (e.g., build output check, CI assertion), even if the specific approach is left to the implementer. LOW severity — not blocking, but improves the criterion's usefulness.

---

### C4: No loading state or latency UX defined
**Severity:** LOW
**Raised by:** UX Designer
**Spec Item:** User Story 2 / Assumptions

**Debate Summary:**
UX Designer raised this noting that dynamic rendering introduces per-request latency with no defined loading behavior. Business Sponsor rejected it as over-engineering for a single-user app. Product Owner agreed, noting the change affects when data is fetched, not how it is presented — existing loading states from prior implementation remain in place. UX Designer conceded, acknowledging the single-user context.

**Resolution:** RESOLVED
No spec change needed. The spec's explicit assumption that "a small increase in request latency is acceptable given the single-user nature of the application" is sufficient. Existing loading states from the prior implementation are preserved by FR-004.
72 changes: 72 additions & 0 deletions specs/004-dynamic-rendering/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Implementation Plan: Force Dynamic Page Rendering

**Branch**: `004-dynamic-rendering` | **Date**: 2026-03-19 | **Spec**: [spec.md](./spec.md)
**Input**: Feature specification from `/specs/004-dynamic-rendering/spec.md`

## Summary

The application fails `next build` when no database is available at build time because Next.js attempts to statically pre-render pages that call Prisma. The fix is to add `export const dynamic = 'force-dynamic'` to every page and layout in the authenticated `(app)` route group, ensuring they are rendered at request time instead of build time. This is a configuration-only change — no data fetching logic, UI, or data model changes.

## Technical Context

**Language/Version**: TypeScript 5.x, Next.js 16.1.7 (App Router)
**Primary Dependencies**: Next.js, Prisma (with `@prisma/adapter-pg`), Auth.js (next-auth v5)
**Storage**: SQLite (local) / PostgreSQL (production) via Prisma
**Testing**: Vitest (unit + integration), Playwright (E2E), `next build` verification
**Target Platform**: Node.js server (self-hosted)
**Project Type**: Web application (Next.js App Router)
**Performance Goals**: N/A — single-user app; minor latency increase from dynamic rendering is acceptable per spec assumptions
**Constraints**: No database connection at build time; no weakening of security posture
**Scale/Scope**: 5 pages, 3 layouts — only 4 files in the `(app)` route group need modification

## Constitution Check

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

| Principle | Status | Notes |
|-----------|--------|-------|
| I. Portability | PASS | No provider-specific changes. `force-dynamic` is standard Next.js API. |
| II. Tech Stack Discipline | PASS | No new dependencies or stack changes. |
| III. Security (NON-NEGOTIABLE) | PASS | This change *improves* security by eliminating the need for database credentials at build time. No secrets in code. |
| IV. Testing (NON-NEGOTIABLE) | PASS | Existing tests must continue passing. Build verification (`next build` without DB) is the primary acceptance test. E2E test for dynamic data freshness required. |
| V. Simplicity | PASS | Minimal change: one export line per file. No new abstractions, no caching model changes. |

**Pre-design gate: PASSED**

## Project Structure

### Documentation (this feature)

```text
specs/004-dynamic-rendering/
├── plan.md # This file
├── research.md # Phase 0: Next.js 16 dynamic rendering research
├── data-model.md # Phase 1: No changes (configuration-only feature)
├── quickstart.md # Phase 1: Verification guide
└── tasks.md # Phase 2 output (/speckit.tasks command)
```

### Source Code (files to modify)

```text
src/app/(app)/
├── layout.tsx # ADD: export const dynamic = 'force-dynamic'
├── holdings/
│ ├── page.tsx # ADD: export const dynamic = 'force-dynamic'
│ └── [id]/
│ └── page.tsx # ADD: export const dynamic = 'force-dynamic'
└── portfolio/
└── page.tsx # ADD: export const dynamic = 'force-dynamic'
```

**Files NOT modified** (no database access):
- `src/app/page.tsx` — root redirect only
- `src/app/(auth)/login/page.tsx` — no DB access
- `src/app/(auth)/layout.tsx` — no DB access
- `src/app/layout.tsx` — root metadata only

**Structure Decision**: No new files or directories. This is a configuration-only change to 4 existing files in the `(app)` route group.

## Complexity Tracking

No constitution violations. No complexity justification needed.
50 changes: 50 additions & 0 deletions specs/004-dynamic-rendering/quickstart.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# Quickstart: Force Dynamic Page Rendering

**Feature**: 004-dynamic-rendering | **Date**: 2026-03-19

## What Changed

All pages and layouts in the authenticated `(app)` route group now export `dynamic = 'force-dynamic'`, ensuring they are rendered at request time instead of being statically generated during `next build`.

## Verification

### 1. Build without database (primary acceptance test)

```bash
# Unset DATABASE_URL (or ensure it's not configured)
unset DATABASE_URL
npm run build
# Expected: Build completes successfully with zero errors
```

The build output should show the `(app)` routes as dynamically rendered (`ƒ` or `λ` symbol) rather than statically generated (`●` symbol).

### 2. Runtime data freshness

```bash
# Start with database available
npm run start

# 1. Navigate to /holdings — verify holdings load
# 2. Modify a holding in the database directly
# 3. Refresh the page — verify updated data appears immediately
```

### 3. Existing tests

```bash
npm run test # Unit + integration tests
npm run test:e2e # Playwright E2E tests (if configured)
npx tsc --noEmit # Type checking
```

All existing tests must continue to pass.

## Files Modified

| File | Change |
|---|---|
| `src/app/(app)/layout.tsx` | Added `export const dynamic = 'force-dynamic'` |
| `src/app/(app)/holdings/page.tsx` | Added `export const dynamic = 'force-dynamic'` |
| `src/app/(app)/holdings/[id]/page.tsx` | Added `export const dynamic = 'force-dynamic'` |
| `src/app/(app)/portfolio/page.tsx` | Added `export const dynamic = 'force-dynamic'` |
Loading
Loading