From ffedf5cb95a5fe5a0bf2045a2a75059ad7fa8b6a Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Sat, 9 May 2026 17:19:10 +0800 Subject: [PATCH 1/6] feat(config): scaffold ProjectFile type and read/write helpers Adds scripts/config/project.ts as the typed foundation for the project file layer described in Phase 0 / Sprint 1 Issue #1. All pipeline stages will eventually read episode metadata, tool versions, and run parameters from .ragtech/project.json rather than passing them as CLI flags. Kept synchronous and dependency-free (plain Node fs). writeProject() creates the .ragtech/ directory if absent and writes pretty-printed JSON. readProject() throws a typed ProjectNotFoundError when the file is missing so callers can distinguish a missing project from other I/O failures. AC: reading existing project file succeeds, writing creates .ragtech/project.json, read/write roundtrip preserves structure --- scripts/config/project.test.ts | 72 ++++++++++++++++++++++++++++++++++ scripts/config/project.ts | 60 ++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) create mode 100644 scripts/config/project.test.ts create mode 100644 scripts/config/project.ts diff --git a/scripts/config/project.test.ts b/scripts/config/project.test.ts new file mode 100644 index 0000000..a098497 --- /dev/null +++ b/scripts/config/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 './project'; +import type { ProjectFile } from './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); + }); +}); diff --git a/scripts/config/project.ts b/scripts/config/project.ts new file mode 100644 index 0000000..534a705 --- /dev/null +++ b/scripts/config/project.ts @@ -0,0 +1,60 @@ +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); + } + 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'); +} From d499ffdc8a21ebe1a349b8013c3cb54668d9e9bf Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Sat, 9 May 2026 17:19:48 +0800 Subject: [PATCH 2/6] docs(inventory): mark issue #1 complete; update CLAUDE.md key files table Sprint 1 Issue #1 (project file configuration layer) is implemented on refactor/s1-project-file. Added scripts/config/project.ts to the Key Source Files table so future agents know where the project-file helpers live. --- CLAUDE.md | 1 + docs/REFACTOR_ISSUE_INVENTORY.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index bcbce72..63056fd 100644 --- a/CLAUDE.md +++ b/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) | diff --git a/docs/REFACTOR_ISSUE_INVENTORY.md b/docs/REFACTOR_ISSUE_INVENTORY.md index 650df2f..a092cc4 100644 --- a/docs/REFACTOR_ISSUE_INVENTORY.md +++ b/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 From 56a8d10f7a6ecd0d10ccbf7f1a06f2f11cf2d989 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Sat, 9 May 2026 17:27:35 +0800 Subject: [PATCH 3/6] fix: npm critical vulnerability --- package-lock.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package-lock.json b/package-lock.json index d854ce0..1adbb29 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1122,7 +1122,9 @@ } }, "node_modules/@babel/plugin-transform-modules-systemjs": { - "version": "7.29.0", + "version": "7.29.4", + "resolved": "https://registry.npmjs.org/@babel/plugin-transform-modules-systemjs/-/plugin-transform-modules-systemjs-7.29.4.tgz", + "integrity": "sha512-N7QmZ0xRZfjHOfZeQLJjwgX2zS9pdGHSVl/cjSGlo4dXMqvurfxXDMKY4RqEKzPozV78VMcd0lxyG13mlbKc4w==", "dev": true, "license": "MIT", "dependencies": { From 2b5533392b54d0cbabccc88f2c9c39337035d6ad Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Sat, 9 May 2026 17:38:14 +0800 Subject: [PATCH 4/6] docs(claude-md): add test placement rule from review-pr pattern detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pattern: integration test placed in scripts/ instead of tests/integration/ Observed in: refactor/s1-project-file — 2026-05-09 --- .claude/skills/review-pr/SKILL.md | 7 +++++++ CLAUDE.md | 2 ++ 2 files changed, 9 insertions(+) diff --git a/.claude/skills/review-pr/SKILL.md b/.claude/skills/review-pr/SKILL.md index ccdc090..088c4b5 100644 --- a/.claude/skills/review-pr/SKILL.md +++ b/.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._ + +### 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 diff --git a/CLAUDE.md b/CLAUDE.md index 63056fd..37c7743 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -293,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 From f80e0128d7ec1aefc6aef2607fd09b6df3adbe15 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Sat, 9 May 2026 17:48:04 +0800 Subject: [PATCH 5/6] fix(tests): move project integration tests to tests/integration/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tests use real os.tmpdir() I/O — they are integration tests per TESTING_STANDARDS and must live in tests/integration/ so that npm run test:integration picks them up. Also adds a TODO comment to readProject() flagging the missing schema validation. Closes review blocker B1 and addresses warning W2 from docs/review-findings/2026-05-09-refactor-s1-project-file.md Co-Authored-By: Claude Sonnet 4.6 --- scripts/config/project.ts | 1 + {scripts/config => tests/integration}/project.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) rename {scripts/config => tests/integration}/project.test.ts (94%) diff --git a/scripts/config/project.ts b/scripts/config/project.ts index 534a705..33ab23d 100644 --- a/scripts/config/project.ts +++ b/scripts/config/project.ts @@ -50,6 +50,7 @@ export function readProject(cwd: string = process.cwd()): ProjectFile { 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; } diff --git a/scripts/config/project.test.ts b/tests/integration/project.test.ts similarity index 94% rename from scripts/config/project.test.ts rename to tests/integration/project.test.ts index a098497..966f530 100644 --- a/scripts/config/project.test.ts +++ b/tests/integration/project.test.ts @@ -1,8 +1,8 @@ import fs from 'fs'; import os from 'os'; import path from 'path'; -import { readProject, writeProject, ProjectNotFoundError, PROJECT_DIR, PROJECT_FILENAME } from './project'; -import type { ProjectFile } from './project'; +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', From c584be3b67652278cdefc494a7c5895fc9d29ce0 Mon Sep 17 00:00:00 2001 From: Natasha Ann Date: Sat, 9 May 2026 17:57:59 +0800 Subject: [PATCH 6/6] docs: add review findings from refactor/s1-project-file --- .../2026-05-09-refactor-s1-project-file.md | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 docs/review-findings/2026-05-09-refactor-s1-project-file.md diff --git a/docs/review-findings/2026-05-09-refactor-s1-project-file.md b/docs/review-findings/2026-05-09-refactor-s1-project-file.md new file mode 100644 index 0000000..5268cd4 --- /dev/null +++ b/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 + +- **New pattern detected:** Integration test placed in `scripts/` instead of `tests/integration/` (see B1). Added to SKILL.md Known Gap Patterns.