feat: S-4.2.2 Collapsed Iterations UI (history view, expanded tabs, range selection)

AGENTS.md

@@ -24,6 +24,15 @@ This is a **requirement**:
 * **Any changes:** `npm run test:all` must pass
 * **New features:** Several unit tests + some integration tests + 1-2 new E2E tests + manual testing sanity check with Playwright
 
+### External API Integration Rule
+**Mocks are a liability when not validated against reality.** When building features that consume external APIs (GitHub, etc.):
+
+1. **Verify API response shapes with real data FIRST** — before writing specs, design docs, or mocks. Run the actual API call (e.g., `gh api ...`) and capture the real response. Include the real response in the design doc.
+2. **Add at least one prod-mode E2E test** (`e2e/prod/`) for any feature that parses external API responses. Mock-mode tests verify UI behavior; prod-mode tests verify the API contract.
+3. **Never assume API field names** — check the official API docs AND make a real call. APIs often differ from documentation or from what seems "obvious" (e.g., GitHub Timeline API's `head_ref_force_pushed` has `commit_id`, NOT `before_commit`/`after_commit`).
+
+**Rationale:** See `docs/retrospectives/2026-02-19-timeline-api-field-mismatch.md` — a wrong API shape assumption propagated from design doc → spec → mocks → all tests, and the bug was only caught during manual demo with real data. 1600+ automated tests all passed while the feature was fundamentally broken against real APIs.
+
 ### Running Specific Tests (Vitest)
 **Important:** This project uses **Vitest** (not Jest), so Jest-specific flags like `--testPathPattern` will **not work**.
 
@@ -126,6 +135,36 @@ You have access to Playwright via playwright-cli skill. Make sure to **only use
  * Sanity check your work as you reach a milestone in the implementation of a feature. Once you reach ~200 lines of code changes, the risk that you are compounding errors and don't have working code becomes high. A quick inspection in Playwright gives extra assurance that you are on the right track.
  * Final quality assurance. Don't ask the user to test a feature manually before you did it yourself!
 
+## E2E Test Repo & Demo Data
+
+**Repo:** [`pedropaulovc/codjiflo-e2e-test-repo`](https://github.com/pedropaulovc/codjiflo-e2e-test-repo) — local path: `../../codjiflo-e2e-test-repo`
+
+Dedicated public repo for real GitHub data used by prod-mode E2E tests (`e2e/prod/`) and demo-presenter agents. You have full control: create PRs, push to main, force-push, add comments, close/reopen, merge, etc. Create as many PRs as needed.
+
+### Creating Scenarios
+
+Each scenario gets its own folder on `main` (base data) and its own PR branch (modifications). Branch naming: `test/<slug>` for E2E, `demo/<slug>` for demos.
+
+```bash
+cd ../../codjiflo-e2e-test-repo
+git checkout main && git pull
+mkdir <scenario-slug>
+# Add base files, commit, push to main
+git checkout -b test/<scenario-slug>
+# Modify files, commit, push, create PR via gh
+```
+
+Manipulate PRs freely: force-push for iterations, `gh api` for line-level review comments, close/reopen, merge. Reference in tests as:
+```typescript
+const config = { owner: "pedropaulovc", repo: "codjiflo-e2e-test-repo", prNumber: <N> };
+```
+
+**Rules:** Never delete PRs/branches other tests depend on (`grep` for PR number in `e2e/`). PRs must stay open unless testing merged/closed state. Document new scenarios in `codjiflo-e2e-test-repo/README.md`.
+
+### For Demo-Presenter Agents
+
+Git operations against this repo ARE the "real user flows" for a code review tool — no mocking needed. Create PRs with the desired diff shape, add comments via `gh`, then navigate to the PR in CodjiFlo via Playwright (headed mode) to record the demo. Demo artifacts go to `spec/demo/` in the CodjiFlo repo. Escalate to coordinator/implementer for complex scenarios (multi-iteration force-push histories, etc.).
+
 ## Shared environment
 There are multiple instances of Claude Code running in parallel. Each one has multiple node.exe instances (MCP, dev server, etc.) they also have dev servers running. Each worktree has its own designated port: 3010 for A, 3020 for B, etc. The `npm run dev` command is smart to only kill zombie servers associated with your worktree and only start a server in its designated port automatically. DO NOT kill all node.exe or kill by port number. If `npm run dev` fails STOP and ask the user for assistance.
 

docs/retrospectives/2026-02-19-timeline-api-field-mismatch.md

@@ -0,0 +1,136 @@
+# Retrospective: Timeline API Field Mismatch Bug
+
+**Date:** 2026-02-19
+**Feature:** S-4.2.1 Commit-Based Iteration Loader / S-4.2.2 Collapsed Iterations UI
+**Bug:** `TimelineLoader` expected `before_commit.sha`/`after_commit.sha` on force-push events, but the real GitHub Timeline API only provides `commit_id`
+**Impact:** Collapsed iteration groups never appeared with real GitHub data — only with mocked test data
+**Fix:** Commits `dde70c9`, `2d2dcbc`, `551aef3`
+
+---
+
+## What Happened
+
+The `TimelineLoader` was designed and implemented around an incorrect assumption about the GitHub Timeline API response shape. The code expected `head_ref_force_pushed` events to include nested `before_commit` and `after_commit` objects:
+
+```json
+// ASSUMED (wrong)
+{
+  "event": "head_ref_force_pushed",
+  "before_commit": { "sha": "abc123" },
+  "after_commit": { "sha": "def456" }
+}
+```
+
+The real API only provides a flat `commit_id` field:
+
+```json
+// ACTUAL
+{
+  "event": "head_ref_force_pushed",
+  "commit_id": "def456",
+  "commit_url": "https://api.github.com/repos/.../commits/def456"
+}
+```
+
+This caused the filter at `timeline-loader.ts:155-160` to silently drop ALL force-push events (since `before_commit` and `after_commit` were always `undefined`), resulting in zero collapsed iteration groups with real data.
+
+## Root Cause Chain
+
+### 1. Design doc assumed API shape without verification
+
+`docs/plans/2026-02-17-s-4.2.1-commit-based-iteration-loader-design.md` explicitly specified the wrong interface:
+
+```typescript
+interface TimelineEvent {
+  before_commit?: { sha: string };
+  after_commit?: { sha: string };
+}
+```
+
+This was the source of truth for all downstream work. **Nobody verified this against the actual GitHub API documentation or a real API call.**
+
+### 2. Spec propagated the wrong assumption
+
+`spec/functional/iterations-stateless.md` documented force-push events as providing `before_commit.sha` and `after_commit.sha`. This was cited as the authoritative reference.
+
+### 3. Test plan used the wrong mock shape
+
+`spec/test/stateless-mode.md` contained mock data with the wrong fields:
+
+```javascript
+mockTimelineWithForcePush = [
+  { event: 'head_ref_force_pushed', before_commit: { sha: 'abc123' }, after_commit: { sha: 'def456' } }
+]
+```
+
+### 4. All tests — unit and E2E — used mocks based on the wrong spec
+
+- `timeline-loader.test.ts`: 28 unit tests with wrong mock shapes
+- `force-push-helpers.ts`: E2E mock helpers with wrong fields
+- `github-mocks.ts`: `MockTimelineEvent` interface with wrong fields
+- `iteration-stateless-mode.spec.ts`: inline mocks with wrong fields
+
+### 5. TDD was followed — but mocks were wrong
+
+The development process was textbook TDD:
+1. Tester wrote failing tests (RED)
+2. Implementer made them pass (GREEN)
+3. Code reviewer approved
+
+But the tester's mocks were derived from the wrong spec, so the tests validated incorrect behavior. TDD only catches bugs when the tests themselves are correct.
+
+### 6. No real-API validation existed
+
+There were zero tests that hit the real GitHub Timeline API. All testing (unit + mock-mode E2E) used synthetic responses that matched the assumed shape. The bug was invisible to the entire test suite.
+
+## Why It Wasn't Caught
+
+| Safety Net | Why It Failed |
+|-----------|---------------|
+| Design review | API shape was assumed, not verified against docs |
+| Spec review | Spec codified the assumption as fact |
+| TDD (tester) | Tester wrote mocks from spec, which was wrong |
+| TDD (implementer) | Implemented against wrong mocks, code "worked" |
+| Code review | Reviewer checked code against spec — both were consistent but wrong |
+| Unit tests | All mocked with wrong shape |
+| E2E tests (mock mode) | All mocked with wrong shape |
+| E2E tests (prod mode) | No prod-mode E2E test for force-push detection existed |
+| Manual testing | Not performed until demo phase |
+
+**The entire chain was internally consistent.** Spec, design, code, and tests all agreed — they were just all wrong about the external API.
+
+## Lessons Learned
+
+### 1. Verify external API shapes with real data BEFORE writing specs
+
+Any feature that integrates with an external API must start with a real API call to capture the actual response shape. This should be a mandatory step in the design phase, not deferred to testing.
+
+**Action:** Add to AGENTS.md design checklist: "For external API integrations, capture real API responses and include them in the design doc."
+
+### 2. Prod-mode E2E tests are essential for API integration features
+
+Mock-mode E2E tests verify that the UI works with the expected data shape. Prod-mode E2E tests verify that the expected data shape actually matches reality. Both are needed.
+
+**Action:** For any feature that parses external API responses, at minimum one prod-mode E2E test should verify the API shape matches expectations.
+
+### 3. Mocks are a liability when they're not validated against reality
+
+Mocks make tests fast and deterministic, but they encode assumptions about external systems. When those assumptions are wrong, mocks hide bugs instead of finding them.
+
+**Action:** Consider adding runtime type assertions (e.g., Zod schemas) for external API responses that would fail fast in development if the response shape doesn't match.
+
+### 4. The demo phase caught what testing didn't
+
+The bug was discovered by the demo-presenter when they tried to use the feature with a real PR. This validates the demo phase as a critical quality gate — it's the first time the feature touches real data end-to-end.
+
+### 5. "All tests pass" is not the same as "it works"
+
+1477 unit tests and 130 E2E tests all passed. Zero tests used real GitHub API data for force-push detection. Test count is not a proxy for correctness.
+
+## Remaining Cleanup
+
+The following files still contain the wrong `before_commit`/`after_commit` fields and should be updated to reflect reality:
+
+- `spec/functional/iterations-stateless.md` (lines 65, 170)
+- `spec/test/stateless-mode.md` (lines 159-167)
+- `docs/plans/2026-02-17-s-4.2.1-commit-based-iteration-loader-design.md` (lines 97-98, 128)

e2e/fixtures/force-push-helpers.ts

@@ -0,0 +1,166 @@
+/**
+ * Shared force-push E2E test helpers
+ *
+ * Extracted from collapsed-iterations-ui.spec.ts and
+ * collapsed-iterations-expanded.spec.ts to eliminate duplication.
+ */
+
+import type { Page } from "@playwright/test";
+import {
+  setupAuthState,
+  setupFullPRMocks,
+  setupStatelessIterationMocks,
+  type MockPR,
+  type MockFile,
+} from "./github-mocks";
+import { setupLegacyDefaults } from "./legacy-defaults";
+
+// ============================================================================
+// Shared constants
+// ============================================================================
+
+export const forcePushMockPR: MockPR = {
+  id: 1,
+  number: 123,
+  title: "Test PR for Force Push Scenarios",
+  body: "Testing force push iteration scenarios",
+  state: "open",
+  merged: false,
+  draft: false,
+  user: {
+    id: 1,
+    login: "testuser",
+    avatar_url: "https://avatars.githubusercontent.com/u/1",
+  },
+  head: { ref: "feature/test", sha: "abc123" },
+  base: { ref: "main", sha: "def456" },
+  html_url: "https://github.com/test/repo/pull/123",
+  created_at: "2024-01-01T10:00:00Z",
+  updated_at: "2024-01-05T15:00:00Z",
+};
+
+export const forcePushMockFiles: MockFile[] = [
+  {
+    filename: "src/test.ts",
+    status: "modified",
+    additions: 10,
+    deletions: 5,
+    changes: 15,
+    patch: "@@ -1,5 +1,10 @@\n+// New code\n const x = 1;",
+  },
+];
+
+export const forcePushConfig = {
+  owner: "test",
+  repo: "repo",
+  prNumber: 123,
+  pageUrl: "/test/repo/123",
+};
+
+// ============================================================================
+// Setup helpers
+// ============================================================================
+
+export interface ForcePushScenarioOptions {
+  eventId: number;
+  beforeSha: string;
+  afterSha: string;
+  liveCommits: {
+    sha: string;
+    message: string;
+    author: string;
+    date: string;
+  }[];
+  compareResponse:
+    | {
+        status: 200;
+        commits: {
+          sha: string;
+          message: string;
+          authorName: string;
+          authorLogin: string;
+          date: string;
+        }[];
+      }
+    | { status: 404 };
+}
+
+/**
+ * Set up common beforeEach mocks for force-push tests:
+ * legacy defaults, auth state, and full PR mocks.
+ */
+export async function setupForcePushDefaults(page: Page): Promise<void> {
+  await setupLegacyDefaults(page);
+  await setupAuthState(page);
+  await setupFullPRMocks(
+    page,
+    forcePushConfig.owner,
+    forcePushConfig.repo,
+    forcePushConfig.prNumber,
+    {
+      pr: forcePushMockPR,
+      files: forcePushMockFiles,
+    }
+  );
+}
+
+/**
+ * Set up a force-push scenario with mock timeline and compare API.
+ */
+export async function setupForcePushScenario(
+  page: Page,
+  options: ForcePushScenarioOptions
+): Promise<void> {
+  await setupStatelessIterationMocks(
+    page,
+    forcePushConfig.owner,
+    forcePushConfig.repo,
+    forcePushConfig.prNumber,
+    {
+      commits: options.liveCommits,
+      timeline: [
+        {
+          id: 0,
+          event: "committed",
+          created_at: "2024-01-01T12:00:00Z",
+          sha: options.beforeSha,
+        },
+        {
+          id: options.eventId,
+          event: "head_ref_force_pushed",
+          created_at: "2024-01-02T12:00:00Z",
+          commit_id: options.afterSha,
+        },
+      ],
+    }
+  );
+
+  await page.route(
+    `https://api.github.com/repos/${forcePushConfig.owner}/${forcePushConfig.repo}/compare/${options.afterSha}...${options.beforeSha}`,
+    async (route) => {
+      if (options.compareResponse.status === 404) {
+        await route.fulfill({
+          status: 404,
+          contentType: "application/json",
+          body: JSON.stringify({ message: "Not Found" }),
+        });
+        return;
+      }
+
+      await route.fulfill({
+        status: 200,
+        contentType: "application/json",
+        body: JSON.stringify({
+          commits: options.compareResponse.commits.map((c) => ({
+            sha: c.sha,
+            commit: {
+              message: c.message,
+              author: { name: c.authorName, date: c.date },
+            },
+            author: { login: c.authorLogin },
+          })),
+        }),
+      });
+    }
+  );
+}