docs(plans): add improve plans 005–009 for export, UI, and git listing

plans/005-pass-comments-into-unified-export.md

@@ -0,0 +1,237 @@
+# Plan 005: Pass reviewer comments into unified AI review export
+
+> **Executor instructions**: Follow this plan step by step. Run every
+> verification command and confirm the expected result before moving to the
+> next step. If anything in the "STOP conditions" section occurs, stop and
+> report — do not improvise. When done, update the status row for this plan
+> in `plans/README.md` — unless a reviewer dispatched you and told you they
+> maintain the index.
+>
+> **Drift check (run first)**: `git diff --stat 37ac800..HEAD -- services/ReviewGenerator.js server.js test/server.test.js`
+> If any in-scope file changed since this plan was written, compare the
+> "Current state" excerpts against the live code before proceeding; on a
+> mismatch, treat it as a STOP condition.
+
+## Status
+
+- **Priority**: P1
+- **Effort**: M
+- **Risk**: MED
+- **Depends on**: none
+- **Category**: bug
+- **Planned at**: commit `37ac800`, 2026-08-29
+
+## Why this matters
+
+The visual review UI collects file comments and line comments, then `POST /api/export-for-ai` validates them and reports `commentsIncluded` in the JSON response. `ReviewGenerator.generateUnifiedReview` never accepts those fields, so `AI_REVIEW.md` is diffs-only. Split export (`generateSplitReviews` → `generateFileContent`) already embeds comments. Unified export is the default button in `public/index.html` (`exportForAI`). Reviewer notes are the product; they currently stop at the HTTP seam.
+
+## Current state
+
+- `services/ReviewGenerator.js` — `generateUnifiedReview` writes `AI_REVIEW.md`. Signature at the time of this plan:
+
+```100:110:services/ReviewGenerator.js
+  static async generateUnifiedReview({ includedFiles, excludedFiles = [], largeFiles = [] }) {
+    const timestamp = new Date().toLocaleString();
+    let content = `# 🔍 Code Review - ${timestamp}\n\n`;
+    content += '**Project:** AI Visual Code Review\n';
+    content += '**Generated by:** AI Visual Code Review v2.0\n\n';
+```
+
+- The per-file loop builds `fileContent` from status + diff only (starts ~line 136). It never reads `comments` or `lineComments`.
+- Split path already embeds comments. Copy that shape, do not invent a new markdown dialect. Excerpt:
+
+```254:269:services/ReviewGenerator.js
+    // File Comment
+    if (fileComment) {
+      content += `## 💭 Review Comment\n\n${fileComment}\n\n`;
+    }
+
+    // Line Comments
+    const fileLineComments = Object.entries(lineComments || {})
+      .filter(([lineId]) => lineId.includes(file.replace(/[^a-zA-Z0-9]/g, '_')));
+
+    if (fileLineComments.length > 0) {
+      content += '## 🔍 Line Comments\n\n';
+      fileLineComments.forEach(([lineId, comment]) => {
+        content += `- **${lineId}:** ${comment}\n`;
+      });
+      content += '\n';
+    }
+```
+
+- `server.js` destructures comments then drops them:
+
+```556:624:server.js
+  const { comments = {}, lineComments = {}, excludedFiles = [] } = req.body;
+  // ...
+    const result = await ReviewGenerator.generateUnifiedReview({
+      includedFiles,
+      excludedFiles,
+      // Pass other options if needed by generateUnifiedReview,
+      // though currently it handles content generation primarily.
+    });
+
+    res.json({
+      success: true,
+      file: 'AI_REVIEW.md',
+      // ...
+      commentsIncluded: Object.keys(comments).length,
+```
+
+- `scripts/export-ai-review.js` also calls `generateUnifiedReview` with files only. After this plan it can pass `{}` for comments; CLI has no comment UI. That is fine.
+- There is **no** `test/reviewGenerator.test.js`. `test/server.test.js` hits export with supertest but only checks status codes, not markdown body.
+- `GitService` is a singleton (`module.exports = new GitService()`). Tests must mock that module **before** requiring `ReviewGenerator`.
+- Conventions: CommonJS `require`, Jest. Model new tests after `test/diffService.test.js` (describe/test, no TypeScript). Do not add TypeScript.
+
+## Commands you will need
+
+| Purpose | Command | Expected on success |
+|---------|---------|---------------------|
+| Tests | `npm test` | exit 0 |
+| ReviewGenerator tests | `npx jest test/reviewGenerator.test.js --verbose` | exit 0, new cases pass |
+| Lint | `npm run lint` | exit 0 (warnings allowed) |
+| Prove comments in signature | `rg -n "comments" services/ReviewGenerator.js` | `generateUnifiedReview` destructure includes `comments` |
+
+## Scope
+
+**In scope**:
+
+- `services/ReviewGenerator.js`
+- `server.js` (only the `generateUnifiedReview({...})` call in `POST /api/export-for-ai`)
+- `test/reviewGenerator.test.js` (create)
+- `test/server.test.js` (one assertion that is still HTTP-level and does not require reading `AI_REVIEW.md`)
+
+**Out of scope**:
+
+- `src/` wiring (plan 001)
+- `public/index.html` markup (plan 006)
+- Git listing refactor (plan 007)
+- Cache middleware (plan 008)
+- `scripts/export-ai-review.js` beyond remaining compatible with extra optional keys
+- Changing the HTTP JSON field names
+- Writing a real `AI_REVIEW.md` into the repo during tests
+
+## Git workflow
+
+- Branch: stay on the current working branch if you were dispatched onto one. Otherwise `prax/unified-export-comments-3d82`.
+- Commit message example: `fix(export): include comments in unified AI_REVIEW.md`
+- Do not push or open a PR unless the operator instructed it.
+
+## Steps
+
+### Step 1: Characterization test that fails on current code
+
+Create `test/reviewGenerator.test.js`. Mock git and disk **before** requiring the module:
+
+```js
+jest.mock('../services/GitService', () => ({
+  getDiffStats: jest.fn().mockResolvedValue('1 file changed'),
+  getFileStatuses: jest.fn().mockResolvedValue({ 'foo.js': 'M' }),
+  getDiffForFile: jest.fn().mockResolvedValue('@@ -1,1 +1,1 @@\n-old\n+new\n')
+}));
+
+const fs = require('fs');
+const ReviewGenerator = require('../services/ReviewGenerator');
+
+describe('generateUnifiedReview comments', () => {
+  let writeSpy;
+
+  beforeEach(() => {
+    writeSpy = jest.spyOn(fs.promises, 'writeFile').mockResolvedValue();
+  });
+
+  afterEach(() => {
+    writeSpy.mockRestore();
+  });
+
+  test('embeds file comments in the markdown written to disk', async () => {
+    await ReviewGenerator.generateUnifiedReview({
+      includedFiles: ['foo.js'],
+      excludedFiles: [],
+      comments: { 'foo.js': 'UNIQUE_REVIEW_NOTE_ABC' },
+      lineComments: { 'foo_js_1_1': 'LINE_NOTE_XYZ' }
+    });
+
+    expect(writeSpy).toHaveBeenCalled();
+    const content = writeSpy.mock.calls[0][1];
+    expect(content).toContain('UNIQUE_REVIEW_NOTE_ABC');
+    expect(content).toContain('LINE_NOTE_XYZ');
+  });
+});
+```
+
+On current `main` this test **must fail** (comments ignored). If it passes before you edit `ReviewGenerator.js`, STOP — the bug is already gone or the mock is wrong.
+
+**Verify**: `npx jest test/reviewGenerator.test.js --verbose` → the new test fails because the unique strings are absent.
+
+### Step 2: Extend `generateUnifiedReview` interface
+
+In `services/ReviewGenerator.js`:
+
+1. Destructure `comments = {}` and `lineComments = {}` next to `includedFiles`.
+2. Inside the per-file `includedFiles.map` callback, after status/type markdown and **before** the diff block, insert the same comment + line-comment markdown as `generateFileContent` (same headings: `## 💭 Review Comment` and `## 🔍 Line Comments`). Use `comments[file]` and the same `lineId.includes(file.replace(/[^a-zA-Z0-9]/g, '_'))` filter.
+3. Do not add a second checklist per file. Keep `getChecklistTemplate()` once at the end as today.
+
+**Verify**: `npx jest test/reviewGenerator.test.js --verbose` → the characterization test now passes.
+
+### Step 3: Pass comments from `server.js`
+
+Change only the `generateUnifiedReview` call in `POST /api/export-for-ai` to:
+
+```js
+    const result = await ReviewGenerator.generateUnifiedReview({
+      includedFiles,
+      excludedFiles,
+      comments,
+      lineComments
+    });
+```
+
+Leave `POST /api/export-individual-reviews` alone (it already passes comments into `generateSplitReviews`).
+
+**Verify**: `rg -n "generateUnifiedReview" server.js` → the object includes `comments` and `lineComments`.
+
+### Step 4: HTTP smoke in existing server tests
+
+In `test/server.test.js`, in the existing `POST /api/export-for-ai should accept valid request` case, keep status `[200, 400]`. Add a **new** test that invalid comments still 400 (already covered). Add one comment: no new HTTP markdown assertion (file is written to `process.cwd()`). The unit test in step 1 is the regression lock.
+
+**Verify**: `npx jest test/server.test.js --verbose` → all pass.
+
+### Step 5: Full suite
+
+**Verify**: `npm test` → exit 0. Confirm no leftover `AI_REVIEW.md` from tests (`git status` clean of that file). If a test wrote one, delete it and fix the mock.
+
+## Test plan
+
+- New file `test/reviewGenerator.test.js`:
+  - Happy path: file comment + line comment appear in `writeFile` content.
+  - Empty comments: still writes markdown, no throw (add this second test).
+  - Pattern: `test/diffService.test.js` (plain Jest, no supertest).
+- HTTP: existing export tests still pass.
+- Verification: `npm test` → all pass, including the new file.
+
+## Done criteria
+
+- [ ] `npm test` exits 0
+- [ ] `npx jest test/reviewGenerator.test.js` exits 0
+- [ ] `rg -n "UNIQUE_REVIEW_NOTE"` is only in the test file, not in `services/`
+- [ ] `generateUnifiedReview` destructure includes `comments` and `lineComments`
+- [ ] `server.js` passes `comments` and `lineComments` into that call
+- [ ] No files outside the in-scope list are modified (`git status`)
+- [ ] `plans/README.md` status row for 005 is DONE
+
+## STOP conditions
+
+- The code at the locations in "Current state" doesn't match the excerpts.
+- Step 1's test passes **before** you change `ReviewGenerator.js`.
+- Making the test pass appears to require mocking `child_process` instead of `GitService` (wrong mock — fix the mock, don't rewrite Git).
+- A step's verification fails twice after a reasonable fix attempt.
+- The fix appears to require touching `public/index.html` or `src/`.
+- You discover `generateUnifiedReview` already takes and renders comments (bug already fixed).
+
+## Maintenance notes
+
+- Any new unified-export field (large-file skips, extra metadata) belongs on this same interface so HTTP and CLI stay aligned.
+- Reviewers should confirm comments are interpolated as markdown text, not HTML, matching `generateFileContent`.
+- Split export already had comments; do not "simplify" by deleting that path.
+- Follow-up deferred: plan 007 (Git listing) and plan 008 (dead cache).