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
15 changes: 15 additions & 0 deletions .claude/skills/implement-issue/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,23 @@ npx tsc --noEmit

Fix all type errors before committing.

**Type specification check:** If this commit adds or modifies any TypeScript `type` or `interface`, search `docs/PRODUCTION_REFACTOR_PLAN.md` for the same type name and compare field by field:
- Field names must match exactly (the spec uses specific names that downstream phase steps reference by name)
- Required vs. optional status must match (`field:` vs `field?:`)
- Field types must match

If the spec must change, update it first in a separate commit (`docs(refactor-plan): ...`) and get agreement before the implementation commit. Diverging silently breaks future phases.

### 5e. Commit

**Before staging:** run `git diff --cached --name-only` after staging your intended files. lint-staged may auto-fix and silently re-stage files outside this commit's scope. Remove any out-of-scope files before committing:

```bash
git restore --staged <unintended-file>
```

Only files listed in this commit's plan should appear in `git diff --cached --name-only`.

```bash
git add <specific-files> # never git add -A
git commit -m "$(cat <<'EOF'
Expand Down
53 changes: 52 additions & 1 deletion .claude/skills/pre-push-audit/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,58 @@ EOF
)"
```

### 4c — Run the full test suite
### 4b-2 — Scope verification (lint-staged side effects)

After test gaps are resolved, verify that no files outside the PR's intended scope were silently added to the diff by lint-staged auto-fixes during pre-commit hooks.

```bash
git diff --name-only origin/main...HEAD
```

Compare this list against the file scope described in the PR body (from Step 0). Any file in the diff that is not mentioned in the PR's scope is a candidate for an unintended lint-staged modification. For each unexpected file:

1. Confirm whether the change is cosmetic (whitespace, comment, trailing comma) — if yes, it was likely auto-fixed by lint-staged during a prior commit.
2. If cosmetic and out-of-scope, record:

```
[QUALITY] Out-of-scope file in diff (lint-staged side effect)
File: <path>
Change: <describe the cosmetic change>
Verdict: WARNING — squash or drop this change; it obscures the PR's actual diff.
```

### 4c — Type specification check

For each new or modified `type` or `interface` in the diff, verify it matches `docs/PRODUCTION_REFACTOR_PLAN.md` exactly. Downstream phase steps reference type field names by name — divergence breaks future phases silently.

```bash
# Find new or changed type/interface declarations in the diff
git diff origin/main...HEAD -- '*.ts' '*.tsx' | grep '^\+' | grep -E '^\+\s*(export\s+)?(type|interface)\s+' | grep -v '^+++'
```

For each type name found, search the refactor plan:

```bash
grep -n "<TypeName>" docs/PRODUCTION_REFACTOR_PLAN.md
```

Compare field by field:
- Field names (exact match — `videoSrc` ≠ `src`)
- Required vs optional (`field:` vs `field?:`)
- Field types

If the implementation diverges from the spec **without a corresponding spec update in this same diff**, record:

```
[QUALITY] Type diverges from PRODUCTION_REFACTOR_PLAN.md spec
Type: <TypeName>
File: <implementation file>
Spec: docs/PRODUCTION_REFACTOR_PLAN.md:<line>
Divergence: <field name / type / optionality that differs>
Verdict: BLOCKER — update the spec first, then align the implementation.
```

### 4d — Run the full test suite

```bash
npm test
Expand Down
38 changes: 13 additions & 25 deletions .claude/skills/review-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -663,30 +663,18 @@ _This section is populated automatically by Step 8c as patterns are observed in

<!-- 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

### Runtime directory not added to .gitignore
<!-- NOTE: Four patterns observed in earlier reviews are now enforced upstream and
do not need to be rechecked here:
• Integration test in scripts/ → pre-commit hook blocks (real I/O grep)
• Runtime directory not in .gitignore → pre-push hook blocks (check-runtime-dirs.js)
• Lint-staged out-of-scope files → implement-issue Step 5e + pre-push-audit Step 4b-2
• Non-deterministic test inputs → pre-commit hook blocks (Math.random grep)
If any of these somehow reach review (--no-verify bypass), the existing Step 4/5
checks will surface them as ordinary findings. -->

### Implementation diverges from documented spec without updating the spec
**Category:** QUALITY
**Trigger:** A new module creates a directory under `process.cwd()` (or the project root) that is intended to hold generated/runtime files (artifacts, run logs, cache) rather than source code.
**Check:** For every directory path written by new code (grep `mkdirSync` or `mkdir` in the diff), verify the directory appears in `.gitignore`. Common offenders: `.ragtech/`, `runs/`, `cache/`, `artifacts/`.
**Verdict:** BLOCKER
**First seen:** refactor/s1-artifacts — 2026-05-09

### Lint-staged sweeping out-of-scope files into the commit
**Category:** QUALITY
**Trigger:** The diff contains changes to files not mentioned in the PR description or implementation doc — typically cosmetic comment moves or eslint-disable removals in unrelated source files.
**Check:** Compare `git diff --name-only origin/main...HEAD` against the file list in the PR description. Any file in the diff that is not in the PR's "Files changed" table is a scope violation. Common cause: lint-staged running `eslint --fix` on all staged files during the pre-commit hook, auto-modifying files the developer didn't intend to change.
**Verdict:** BLOCKER
**First seen:** fix/pre-existing-failing-test-suites — 2026-05-09

### Non-deterministic test setup using Math.random()
**Category:** COVERAGE
**Trigger:** A test file uses `Math.random()` (or `Date.now()`, `crypto.randomUUID()`, or any other non-seeded random source) inside a `test()` or `it()` block to construct input data for the function under test.
**Check:** `grep -n "Math\.random\|Date\.now\|crypto\.random" scripts/**/*.test.{js,ts} app/**/*.test.{ts,tsx} remotion/**/*.test.{ts,tsx}` — any match inside a test body (not a mock implementation) is a candidate. Then verify whether the assertions check specific output values; if assertions are only type-level (`typeof`, `toHaveProperty`, `toBeDefined`), flag as BLOCKER.
**Trigger:** A PR adds or modifies a type, interface, function signature, or data shape that is already defined in `docs/PRODUCTION_REFACTOR_PLAN.md` (or another spec doc), but the implementation uses different field names, different optionality, or omits required fields — without a corresponding update to the spec.
**Check:** For each new or changed type in the diff, search `docs/PRODUCTION_REFACTOR_PLAN.md` for the type name or relevant phase section. Compare field names, types, and required/optional status between the spec and the implementation. Pay special attention to fields that downstream phase steps reference by name (grep the phase steps for `.fieldName` usage).
**Verdict:** BLOCKER
**First seen:** refactor/s1-audiosync-determinism — 2026-05-09
**First seen:** refactor/s1-brand-types — 2026-05-10
58 changes: 58 additions & 0 deletions .husky/check-runtime-dirs.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
#!/usr/bin/env node
'use strict';

/**
* Checks that any new mkdirSync/mkdir calls in the diff target directories that
* are already covered by .gitignore. Runtime-generated directories must never be
* committed — but they frequently are when a new module creates one and .gitignore
* is not updated in the same PR.
*
* Only inspects simple string-literal paths (single or double quoted). Template
* literals and variable-based paths are skipped to avoid false positives.
*/

const { execSync } = require('child_process');
const fs = require('fs');

const gitignore = fs.existsSync('.gitignore') ? fs.readFileSync('.gitignore', 'utf8') : '';

let diff;
try {
diff = execSync("git diff origin/main...HEAD -- '*.ts' '*.tsx' '*.js'", {
encoding: 'utf8',
stdio: ['pipe', 'pipe', 'pipe'],
});
} catch {
// No origin/main yet (first push) or diff failed — skip
process.exit(0);
}

const addedLines = diff.split('\n').filter(l => l.startsWith('+') && !l.startsWith('+++'));
const mkdirLines = addedLines.filter(l => /(mkdirSync|mkdir)\s*\(/.test(l));

const failures = [];
for (const line of mkdirLines) {
const m = line.match(/(?:mkdirSync|mkdir)\s*\(\s*['"]([^'"]+)['"]/);
if (!m) continue;

const dir = m[1];
// Skip absolute paths and dynamic values
if (/^\/tmp|^\/var|\$\{|process\./.test(dir)) continue;

const root = dir.replace(/^\.\//, '').split('/')[0];
if (!root) continue;

// Check if root directory (or its dotted variant) appears in .gitignore
const escaped = root.replace(/\./g, '\\.');
const covered = new RegExp('(^|\\n)/?' + escaped + '(/|$|\\n)').test(gitignore);
if (!covered) {
failures.push(` mkdirSync('${dir}') → add '${root}/' to .gitignore`);
}
}

if (failures.length > 0) {
console.error('✗ New runtime directories not covered by .gitignore:');
failures.forEach(f => console.error(f));
console.error(' Runtime-generated directories must never be committed.');
process.exit(1);
}
38 changes: 38 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,41 @@ if [ -n "$STAGED_TESTABLE" ]; then
echo "▶ jest --findRelatedTests"
echo "$STAGED_TESTABLE" | xargs npx jest --findRelatedTests --passWithNoTests --no-coverage
fi

# ── 4. Integration test placement ──────────────────────────────────────────────
# Test files under scripts/ must be pure (no real filesystem I/O). A test that
# calls os.tmpdir(), fs.mkdtempSync(), or mkdtemp() is an integration test and
# must live in tests/integration/ — not next to the source file in scripts/.
STAGED_SCRIPT_TESTS=$(git diff --cached --name-only | grep -E '^scripts/.*\.test\.(ts|js)$' || true)
if [ -n "$STAGED_SCRIPT_TESTS" ]; then
echo "▶ checking scripts/ test files for real I/O"
BAD_TESTS=""
while IFS= read -r f; do
if git show ":$f" 2>/dev/null | grep -qE 'os\.tmpdir|fs\.mkdtempSync|mkdtemp'; then
BAD_TESTS="${BAD_TESTS} ${f}\n"
fi
done <<< "$STAGED_SCRIPT_TESTS"
if [ -n "$BAD_TESTS" ]; then
printf "✗ Real I/O in scripts/ test file(s) — move to tests/integration/ instead:\n%s" "$BAD_TESTS"
exit 1
fi
fi

# ── 5. Non-deterministic test inputs ──────────────────────────────────────────
# Test files must not use non-seeded random sources. Math.random(), Date.now(),
# and crypto random APIs produce flaky tests whose failures are timing-dependent
# and hard to reproduce. Use fixed seeds or static data instead.
STAGED_TESTS=$(git diff --cached --name-only | grep -E '\.test\.(ts|tsx|js)$' || true)
if [ -n "$STAGED_TESTS" ]; then
echo "▶ checking test files for non-deterministic random sources"
BAD_RANDOM=""
while IFS= read -r f; do
if git show ":$f" 2>/dev/null | grep -qE 'Math\.random\(\)|Date\.now\(\)|crypto\.randomUUID\(\)|crypto\.getRandomValues'; then
BAD_RANDOM="${BAD_RANDOM} ${f}\n"
fi
done <<< "$STAGED_TESTS"
if [ -n "$BAD_RANDOM" ]; then
printf "✗ Non-deterministic random source in test file(s) — use fixed seeds or static data:\n%s" "$BAD_RANDOM"
exit 1
fi
fi
8 changes: 8 additions & 0 deletions .husky/pre-push
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ AUDIT_OUTPUT=$(npm audit --audit-level=moderate 2>&1) || {
fi
}

# ── 7. Runtime directories in .gitignore ───────────────────────────────────────
# New code that creates directories at runtime must reference paths covered by
# .gitignore. Without this, generated content lands in committed files on the
# next `git add .`. The check only inspects simple string literals — template
# literals and variable-based paths are skipped to avoid false positives.
echo "▶ checking new runtime directory creation against .gitignore"
node .husky/check-runtime-dirs.cjs

# ── Phase-gated guards ─────────────────────────────────────────────────────────
# Uncomment each block after its phase merges to main. These prevent agents from
# reintroducing patterns that a phase explicitly removed.
Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,3 +319,4 @@ For any multi-step task:
4. Scope discipline — only files listed in the implementation doc touched
5. No new hardcoded paths in `scripts/`; no new duplicated timing constants in `remotion/`
6. *(Remotion phases)* — `remotion studio` launches; frame comparison against `docs/render-baselines/`
7. **Type shapes match spec** — if a type is defined in `docs/PRODUCTION_REFACTOR_PLAN.md`, the implementation must use the exact field names, types, and required/optional status from the spec. Downstream phase steps depend on specific field names by reference. If the spec must change, update it first and get agreement before diverging in code.
130 changes: 130 additions & 0 deletions docs/implementation-guides/REFACTOR_P0_BRAND.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
---
description: Phase 0.5 — Brand Abstraction Layer Implementation Guide
---

# Phase 0.5 — Brand Abstraction Layer

**Branch:** `refactor/s1-brand-types`
**Goal:** All brand content out of code into config. Adding a new brand = creating files only + VSCode extension improvements.

---

## Implementation Steps

### Step 1: Extend brand types
**Status:** ✅ DONE
- [x] Extend `remotion/types/brand.ts` with `identity`, `hosts`, `mascot`, `audio`, `background` types
- [x] Add `BrandHost` and `BrandMascot` type definitions
- [x] Update `Brand` type with new required fields

**Status check:**
```bash
tsc --noEmit
```

> ⚠️ **Sequencing Guard:** Steps 6 and 7 MUST NOT be started before Step 2 is complete. The `Brand` type requires `identity`, `hosts`, `mascot`, `audio`, and `background`, but `public/brand.json` does not yet have these fields. Runtime access before migration will throw.

### Step 2: Create brand structure and migrate config
**Status:** ⏳ PENDING
- [ ] Create `brands/ragtech/` directory
- [ ] Migrate `public/brand.json` → `brands/ragtech/brand.json` with all new fields
- [ ] Update `BrandContext` to load from `brands/{brandId}/brand.json` using `brandId` from `project.json`

**Status check:**
```bash
ls brands/ragtech/brand.json
```

### Step 3: Move keyword overlays to brand structure
**Status:** ⏳ PENDING
- [ ] Move overlays to `brands/ragtech/components/`:
- `AIOverlay`
- `AwardsOverlay`
- `CodingOverlay`
- `EngineeringOverlay`
- `FrameworkOverlay`
- `LanguageOverlay`
- `InfrastructureOverlay`
- `EducationOverlay`
- `BestPracticesOverlay`
- `RolesOverlay`
- `RagtechOverlay`
- [ ] Export from `brands/ragtech/components/index.ts`

**Status check:**
```bash
ls -la brands/ragtech/components/ | wc -l # Should show 11 overlay files + index.ts
```

### Step 4: Create brand registry
**Status:** ⏳ PENDING
- [ ] Create `remotion/lib/brandRegistry.ts` with `getBrandOverlays(brandId)` static switch

**Status check:**
```bash
ls remotion/lib/brandRegistry.ts
```

### Step 5: Update OverlayRenderer
**Status:** ⏳ PENDING
- [ ] Update `OverlayRenderer`: remove hardcoded keyword imports
- [ ] Use `{ ...CORE_TEMPLATE_MAP, ...getBrandOverlays(brand.id) }`

**Status check:**
```bash
grep -r "AIOverlay\|RagtechOverlay" remotion/components/OverlayRenderer.tsx | wc -l # Should be 0
```

### Step 6: Move remaining overlays and parameterize
**Status:** ⏳ PENDING
- [ ] Move remaining overlays to `remotion/components/overlays/templates/`
- [ ] Parameterize mascot with `brand.mascot.enabled` guard
- [ ] Replace `~/ragtech` with `brand.identity.terminalPath`

**Status check:**
```bash
grep -r "natasha\|saloni\|victoria\|techybara\|ragtechdev\|~/ragtech" remotion/components/ | wc -l # Should be 0
```

### Step 7: Update podcast components
**Status:** ⏳ PENDING
- [ ] Update `PodcastIntro`: `COHOSTS` → `brand.hosts`
- [ ] Update `PodcastOutro`: `COHOSTS` → `brand.hosts`
- [ ] Update `PodcastThumbnail`: `EPISODES` → `brand.background.episodeGridAssets`
- [ ] Update audio paths → `brand.audio.*`

**Status check:**
```bash
grep -r "COHOSTS\|EPISODES" remotion/components/ | wc -l # Should be 0
```

### Step 8: VSCode extension improvements
**Status:** ⏳ PENDING
- [ ] Add `Wrap in cut` command (Cmd+D) to wrap selected text in `{}`
- [ ] Add `> NOTE` directive support
- [ ] Add `> SPEAKER` snippet

**Status check:**
```bash
code --list-extensions | grep vscode-transcript-language # Should show extension
```

---

## Final Status Checks

All steps must pass these final verification commands:

```bash
# No hardcoded brand references
grep -r "natasha\|saloni\|victoria\|techybara\|ragtechdev\|~/ragtech" remotion/components/ | wc -l # Should be 0

# No hardcoded overlay imports
grep -r "AIOverlay\|RagtechOverlay" remotion/components/OverlayRenderer.tsx | wc -l # Should be 0

# TypeScript compilation
tsc --noEmit # Should pass with zero errors

# Visual regression test
# Frame comparison: RAG Tech compositions pixel-identical to pre-refactor baseline
```
Loading
Loading