Refactor/s1 project file

.claude/skills/review-pr/SKILL.md

@@ -662,3 +662,10 @@ No blockers — branch is ready to merge.
 _This section is populated automatically by Step 8c as patterns are observed in real reviews. Do not edit manually._
 
 <!-- Entries are appended here by the skill -->
+
+### Integration test placed in scripts/ instead of tests/integration/
+**Category:** CONVENTION
+**Trigger:** A new test file in `scripts/` uses `os.tmpdir()`, `fs.mkdtempSync`, or any real filesystem read/write in its test body or setup.
+**Check:** `grep -rn "mkdtemp\|os\.tmpdir\|fs\.mkdtempSync" scripts/**/*.test.ts` — any match means the test uses real I/O and must live in `tests/integration/` not next to the source file.
+**Verdict:** BLOCKER
+**First seen:** refactor/s1-project-file — 2026-05-09

CLAUDE.md

@@ -196,6 +196,7 @@ ty    = (0.5 - vp.cy) × 100%
 | `remotion/types/transcript.ts` | `Segment`, `Token`, `TimeCut`, `Transcript` | Will import from `scripts/types/` (Phase 6) |
 | `remotion/types/camera.ts` | `CameraProfiles`, `CameraShot`, `CropViewport` | Will import from `scripts/types/` (Phase 6) |
 | `remotion/types/brand.ts` | Brand design tokens only | Extend with identity/hosts/mascot/audio (Phase 0.5) |
+| `scripts/config/project.ts` | `ProjectFile` type, `readProject`/`writeProject`, `ProjectNotFoundError` | Sprint 1 Issue #1 |
 | `scripts/edit-transcript.js` | Sentence merging, `deriveCuts`, doc generation | Migrate to .ts (Phase 3) |
 | `scripts/sync/AudioSyncer.js` | FFT sync, `syncMultiple` | Add FFT tie-breaking (Phase 0) |
 | `scripts/wizard.js` | Interactive pipeline runner (60KB) | Replace with DAG runner (Phase 2) |
@@ -292,6 +293,8 @@ Three test types, three runners:
 
 Every new pure function gets a unit test. Every new React component gets a smoke render test. E2E tests are required for Phase 8 user-facing features. Coverage thresholds are defined per phase in the standards doc.
 
+**Key placement rule:** If a test uses `os.tmpdir()`, `fs.mkdtempSync`, real file reads/writes, or spawns a process, it is an **integration test** — place it in `tests/integration/`, not next to the source file in `scripts/`. Unit tests in `scripts/**/*.test.ts` must have no real I/O.
+
 ---
 
 ## Agent Implementation Convention

docs/REFACTOR_ISSUE_INVENTORY.md

@@ -3,6 +3,7 @@
 ## Sprint 1 — Deterministic Foundation
 
 ### 1. Create project file configuration layer
+✅ Done — `refactor/s1-project-file` — `ProjectFile` type, `readProject`/`writeProject` helpers, `ProjectNotFoundError`
 Create the `.ragtech/project.json` foundation and typed helpers for reading/writing project state. This becomes the single source of truth for pipeline parameters, tool versions, and deterministic run metadata.
 
 ### 2. Implement content-addressed artifact storage

docs/review-findings/2026-05-09-refactor-s1-project-file.md

@@ -0,0 +1,51 @@
+# Review: refactor/s1-project-file
+Date: 2026-05-09
+Reviewer: AI (review-pr skill) — session bias: CLEAN
+PR: NONE (no gh CLI available; PR description provided inline)
+
+## Verdict
+CHANGES REQUESTED
+
+## Summary
+Scaffolds `scripts/config/project.ts` as the typed foundation for the project-file layer: `ProjectFile` interface, `readProject`/`writeProject` helpers, and `ProjectNotFoundError`. Scope is well-contained and the implementation is clean. The one blocker is a test-placement convention violation — the new tests use real filesystem I/O and must live in `tests/integration/` per the project's TESTING_STANDARDS.
+
+## Blockers (must fix before merge)
+
+### B1 — Test file uses real I/O but lives outside `tests/integration/`
+- **Type:** COVERAGE / CONVENTION
+- **File:** `scripts/config/project.test.ts`
+- **Finding:** The test file uses `fs.mkdtempSync(path.join(os.tmpdir(), ...))` and real filesystem reads/writes. Per `docs/TESTING_STANDARDS.md`: "Integration tests are allowed to: Read and write real files in `os.tmpdir()` temp directories" and "Exception: integration tests always go in `tests/integration/`." Verified by running `npm run test:integration` — it returns zero matches, so these tests are entirely hidden from that runner.
+- **Fix:** Move `scripts/config/project.test.ts` → `tests/integration/project.test.ts` and update the import path from `'./project'` to `'../../scripts/config/project'`. The 6 tests pass and require no other changes.
+
+## Warnings (should address)
+
+### W1 — `project.ts` not yet imported anywhere
+- **Type:** QUALITY
+- **File:** `scripts/config/project.ts`
+- **Finding:** No file in the codebase imports `readProject`, `writeProject`, `ProjectFile`, or `ProjectNotFoundError`. The file is unreferenced at merge time.
+- **Suggestion:** Expected for Sprint 1 scaffolding and documented in the implementation guide — confirm this is intentional. Future pipeline stages that import this module will close the gap.
+
+### W2 — `as ProjectFile` cast silently swallows structural errors
+- **Type:** COVERAGE
+- **File:** `scripts/config/project.ts:53` (`readProject`)
+- **Finding:** `JSON.parse(fs.readFileSync(filePath, 'utf-8')) as ProjectFile` performs no structural validation. A project.json that is syntactically valid JSON but missing required fields (e.g., no `episode` key) will return a `ProjectFile`-shaped object with `undefined` properties, causing silent downstream failures. No test exercises this path.
+- **Suggestion:** Acceptable for Sprint 1 scaffolding. Add a `// TODO: validate against ProjectFile schema (Phase 0)` comment or a note in the implementation guide so this doesn't get lost before pipeline stages start consuming this file.
+
+## Suggestions (optional improvements)
+
+- `ProjectNotFoundError` sets `this.name = 'ProjectNotFoundError'` manually, which is the correct pattern for custom errors in TypeScript. Consider adding a test that asserts `error.name === 'ProjectNotFoundError'` to lock in this contract — useful if callers ever switch from `instanceof` to name-checking.
+- The `readProject` implementation uses `existsSync` then `readFileSync` (TOCTOU pattern). For a single-user CLI tool this is fine; worth noting if the module is ever used in a server context.
+
+## Test plan verification
+
+| Item | Status | Notes |
+|------|--------|-------|
+| `npx jest --testPathPattern="scripts/config/project.test" --no-coverage` | PASS | 6/6 tests pass |
+| `npm test` (full suite) | PRE-EXISTING FAILURES | 23 failures in `CarouselGenerator` and other suites unrelated to this branch; same count on `origin/main` |
+| `npm run test:integration` | NOT FOUND | 0 matches — B1 above |
+| `npx tsc --noEmit` | CLEAN | No type errors |
+| `npm run test:react` | SKIPPED | No `app/` or `remotion/` files changed |
+
+## Patterns observed
+<!-- Step 8 — populated below -->
+- **New pattern detected:** Integration test placed in `scripts/` instead of `tests/integration/` (see B1). Added to SKILL.md Known Gap Patterns.

scripts/config/project.ts

@@ -0,0 +1,61 @@
+import fs from 'fs';
+import path from 'path';
+
+export const PROJECT_DIR = '.ragtech';
+export const PROJECT_FILENAME = 'project.json';
+
+export interface EpisodeMeta {
+  id: string;
+  title?: string;
+  number?: number;
+}
+
+export interface ToolVersions {
+  node?: string;
+  ffmpeg?: string;
+  whisper?: string;
+  [key: string]: string | undefined;
+}
+
+export interface PipelineParams {
+  [key: string]: unknown;
+}
+
+export interface ArtifactRefs {
+  [stageId: string]: string;
+}
+
+export interface ProjectFile {
+  version: string;
+  episode: EpisodeMeta;
+  brandId: string;
+  tools: ToolVersions;
+  params: PipelineParams;
+  artifacts: ArtifactRefs;
+}
+
+export class ProjectNotFoundError extends Error {
+  constructor(filePath: string) {
+    super(`Project file not found: ${filePath}`);
+    this.name = 'ProjectNotFoundError';
+  }
+}
+
+function projectFilePath(cwd: string): string {
+  return path.join(cwd, PROJECT_DIR, PROJECT_FILENAME);
+}
+
+export function readProject(cwd: string = process.cwd()): ProjectFile {
+  const filePath = projectFilePath(cwd);
+  if (!fs.existsSync(filePath)) {
+    throw new ProjectNotFoundError(filePath);
+  }
+  // TODO(Phase 0): validate parsed object against ProjectFile schema before cast
+  return JSON.parse(fs.readFileSync(filePath, 'utf-8')) as ProjectFile;
+}
+
+export function writeProject(project: ProjectFile, cwd: string = process.cwd()): void {
+  const dirPath = path.join(cwd, PROJECT_DIR);
+  fs.mkdirSync(dirPath, { recursive: true });
+  fs.writeFileSync(projectFilePath(cwd), JSON.stringify(project, null, 2), 'utf-8');
+}

tests/integration/project.test.ts

@@ -0,0 +1,72 @@
+import fs from 'fs';
+import os from 'os';
+import path from 'path';
+import { readProject, writeProject, ProjectNotFoundError, PROJECT_DIR, PROJECT_FILENAME } from '../../scripts/config/project';
+import type { ProjectFile } from '../../scripts/config/project';
+
+const SAMPLE: ProjectFile = {
+  version: '1.0.0',
+  episode: { id: 'ep-01', title: 'Pilot', number: 1 },
+  brandId: 'ragtech',
+  tools: { node: '20.0.0', ffmpeg: '6.0' },
+  params: { seed: 42 },
+  artifacts: {},
+};
+
+function mkTmpDir(): string {
+  return fs.mkdtempSync(path.join(os.tmpdir(), 'deckcreate-test-'));
+}
+
+describe('writeProject', () => {
+  let tmpDir: string;
+
+  beforeEach(() => { tmpDir = mkTmpDir(); });
+  afterEach(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); });
+
+  it('creates .ragtech/project.json', () => {
+    writeProject(SAMPLE, tmpDir);
+    expect(fs.existsSync(path.join(tmpDir, PROJECT_DIR, PROJECT_FILENAME))).toBe(true);
+  });
+
+  it('writes valid formatted JSON', () => {
+    writeProject(SAMPLE, tmpDir);
+    const raw = fs.readFileSync(path.join(tmpDir, PROJECT_DIR, PROJECT_FILENAME), 'utf-8');
+    expect(() => JSON.parse(raw)).not.toThrow();
+    expect(raw).toContain('\n');
+  });
+
+  it('creates .ragtech/ directory if absent', () => {
+    const dirPath = path.join(tmpDir, PROJECT_DIR);
+    expect(fs.existsSync(dirPath)).toBe(false);
+    writeProject(SAMPLE, tmpDir);
+    expect(fs.existsSync(dirPath)).toBe(true);
+  });
+});
+
+describe('readProject', () => {
+  let tmpDir: string;
+
+  beforeEach(() => { tmpDir = mkTmpDir(); });
+  afterEach(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); });
+
+  it('reads an existing project file', () => {
+    writeProject(SAMPLE, tmpDir);
+    const result = readProject(tmpDir);
+    expect(result).toEqual(SAMPLE);
+  });
+
+  it('throws ProjectNotFoundError when file is absent', () => {
+    expect(() => readProject(tmpDir)).toThrow(ProjectNotFoundError);
+  });
+
+  it('roundtrip preserves all top-level fields', () => {
+    writeProject(SAMPLE, tmpDir);
+    const result = readProject(tmpDir);
+    expect(result.version).toBe(SAMPLE.version);
+    expect(result.episode).toEqual(SAMPLE.episode);
+    expect(result.brandId).toBe(SAMPLE.brandId);
+    expect(result.tools).toEqual(SAMPLE.tools);
+    expect(result.params).toEqual(SAMPLE.params);
+    expect(result.artifacts).toEqual(SAMPLE.artifacts);
+  });
+});