Extend brand type schema with stable foundation for migration#67
Merged
Conversation
Extend brand type schema with stable foundation for migration - Add BrandHost and BrandMascot type definitions - Extend Brand type with identity, hosts, mascot, audio, background fields - Preserve backward compatibility with optional new fields - Enable downstream brand extraction work with typed schema Type-only changes - no component behavior modifications.
Pattern: Implementation diverges from documented spec without updating the spec Observed in: refactor/s1-brand-types
Fix brand type definitions to match PRODUCTION_REFACTOR_PLAN.md specifications - Align BrandHost type with spec (add role, imgSrc, nameBgColor fields) - Align BrandMascot type with spec (add enabled boolean and assets index map) - Update Brand type with correct field names and required fields: * identity.terminalPath, identity.socialHandle * audio.introOutroMusic, audio.backgroundMusic * background.episodeGridAssets - Add implementation guide docs/implementation-guides/REFACTOR_P0_BRAND.md - Remove out-of-scope inventory change Resolves all review findings and enables downstream Phase 0.5 steps. Type-only changes - no component behavior modifications.
Address review warnings in Phase 0.5 brand implementation guide - Update branch name from refactor/p0-brand to refactor/s1-brand-types to match reality - Add sequencing guard warning between Step 1 and Step 2 to prevent runtime errors - Fix status checks for Steps 2 and 4 to use file existence checks: * Step 2: ls brands/ragtech/brand.json (fails if migration incomplete) * Step 4: ls remotion/lib/brandRegistry.ts (fails if registry missing) All status checks now provide real verification per CLAUDE.md requirements. Guide ready for downstream Phase 0.5 implementation with proper dependency ordering.
Added two pre-commit guards and one pre-push guard so these patterns are blocked at commit/push time rather than surfaced for the first time during code review: - Pre-commit: scripts/**/*.test.ts files must not use os.tmpdir / fs.mkdtempSync / mkdtemp (integration tests belong in tests/integration/) - Pre-commit: test files must not call Math.random(), Date.now(), or crypto random APIs without seeding (non-deterministic → flaky tests) - Pre-push: new mkdirSync/mkdir calls must target directories already in .gitignore (check-runtime-dirs.cjs; simple string literals only, skips template literals and /tmp paths to avoid false positives) These patterns were previously first detected by review-pr's Step 8. Moving enforcement upstream reduces review token cost and catches mistakes before they accumulate across multiple commits.
Patterns that can be checked mechanically now live at the earliest
possible workflow stage instead of being caught for the first time
in review-pr:
implement-issue (Step 5d):
Added explicit type-spec check — any new/modified type must be compared
field-by-field against docs/PRODUCTION_REFACTOR_PLAN.md before committing.
Diverging silently breaks downstream phases that reference field names.
implement-issue (Step 5e):
Added staging-scope guard — run git diff --cached --name-only after staging
and remove any out-of-scope files lint-staged may have auto-fixed.
pre-push-audit (new Step 4b-2 + 4c):
4b-2: scope verification — flag files in the diff not mentioned in the PR body
(lint-staged side-effect detection).
4c: type specification check — mirrors the implement-issue check but runs
across all commits in the branch before the push gate clears.
review-pr (Known Gap Patterns):
Removed the four patterns now enforced upstream:
• Integration test in scripts/ → pre-commit hook
• Runtime dir not in .gitignore → pre-push hook
• Lint-staged scope creep → implement-issue + pre-push-audit
• Non-deterministic test inputs → pre-commit hook
Added a comment block explaining the removal so the history is preserved.
Kept the spec-divergence pattern (pattern 5) — it requires semantic
reasoning that hooks cannot replace.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: Extend brand type schema with stable foundation for migration
Summary
Add new type definitions and extend the Brand schema to support future brand migration work while maintaining backward compatibility.
Changes
identity: name, tagline, descriptionhosts: array of BrandHost objectsmascot: BrandMascot objectaudio: theme, stinger, background audio pathsbackground: image, video, pattern assetsVerification
Impact
This provides a stable, extensible type system for downstream brand extraction work without affecting existing component behavior.
Type of Change
Test Strategy
Type-only changes - no runtime tests required. Verification done via TypeScript compilation.
Merge Risk: Low
Pure type definition changes with backward compatibility.
Issues
Closes #13