refactor: convert to monorepo structure with engine package

[hiddentao]

No description provided.

[claude]

Pull Request Review: Monorepo Refactor

Overall Assessment

This is a well-structured refactor that successfully converts the project to a monorepo structure. The changes are extensive but follow best practices for workspace management. The refactor maintains backward compatibility while improving the project organization.

✅ Strengths

1. Clean Monorepo Architecture

• Well-organized workspace structure with packages/engine and demo as separate workspaces
• Proper dependency management using workspace:* protocol for local dependencies
• Clear separation of concerns between engine core and demo application

2. Improved Build System

• Centralized scripts in root package.json with proper delegation to workspace packages
• Simplified CI/CD workflows by removing redundant dependency installations
• Proper build order: engine → demo

3. Good CI/CD Updates

• Correctly updated coverage path to ./packages/engine/lcov.info
• Simplified workflows by leveraging workspace installation
• Maintained all essential CI checks (linting, testing, browser tests)

4. Proper Git Configuration

• Updated .gitignore with monorepo-specific patterns (packages/*/dist, packages/*/node_modules)
• Added playwright-report/ to gitignore (good catch!)

5. Package Configuration

• Engine package properly configured as @clockwork-engine/core with public access
• Good package.json metadata (keywords, files, publishConfig)
• Maintained engine strictness (node >= 22.0.0)

⚠️ Issues & Recommendations

1. Critical: Missing TypeScript Configuration (High Priority)

The root tsconfig.json (packages/engine/tsconfig.json:1) is missing several important paths:

{
  "files": [],
  "references": [
    { "path": "./packages/engine" },
    { "path": "./demo" }
  ]
}

Current Issue: The root tsconfig.json has compilation settings but doesn't reference the workspaces. This can cause IDE confusion and build issues.

Recommendation: Use TypeScript project references for proper workspace type checking.

2. Vite Alias Configuration Concern (Medium Priority)

In demo/vite.config.ts (demo/vite.config.ts:12-16):

alias: {
  "@engine": path.resolve(__dirname, "../packages/engine/src"),
  "@clockwork-engine/core": path.resolve(__dirname, "../packages/engine/src/index.ts"),
}

Issue: The alias points to source files rather than the built package. This bypasses the build step and may cause issues in production builds.

Recommendation:

• For development, this is fine but document it clearly
• Consider if you want the demo to always use the built package (dist/) to ensure production parity
• The current approach is faster for development but may hide issues that only appear after building

3. Lock File Management (Medium Priority)

The diff shows bun.lock changes at the root, and demo/bun.lock was deleted (183 files changed).

Good: Centralized lock file at root
Concern: Ensure all developers know to run bun install from the root, not from individual packages

Recommendation: Add a package.json check or documentation to prevent workspace-level installs.

4. Lint-Staged Configuration (Low Priority)

In root package.json (package.json:40-47):

"lint-staged": {
  "packages/engine/**/*.{ts,tsx}": ["bun run --cwd packages/engine lint"],
  "demo/**/*.{ts,tsx}": ["bun run --cwd demo lint"]
}

Issue: Running the full lint command for every staged file is inefficient. The lint commands run tsc --noEmit which checks the entire project.

Recommendation:

"lint-staged": {
  "packages/engine/**/*.{ts,tsx}": ["biome check"],
  "demo/**/*.{ts,tsx}": ["biome check"]
}

This runs only formatting/linting checks on staged files, while CI handles full type checking.

5. Missing Package Build Script (Low Priority)

The root package.json has "prepublishOnly": "bun run build" but this project isn't published from the root.

Recommendation: Remove prepublishOnly from root package.json or clarify its purpose. Publishing happens from packages/engine.

6. Test Coverage Path (Low Priority - Verified ✓)

Good catch updating the coverage path in CI to ./packages/engine/lcov.info! This is correct.

However, verify that the coverage report is actually generated at this path by the engine package's test script.

🔒 Security Considerations

• ✅ No sensitive information exposed in configuration files
• ✅ Proper .gitignore patterns for environment files
• ✅ No dependency vulnerabilities introduced (based on dependency updates)
• ✅ Engine version constraint prevents running on unsupported Node versions

🧪 Test Coverage

• ✅ CI workflow properly runs tests from the engine package
• ✅ Browser tests (Playwright) correctly configured
• ✅ Coverage reporting path updated correctly
• ⚠️ Consider adding a workspace-level test script to run tests from all packages

📊 Performance Considerations

• ✅ Monorepo structure enables better caching in CI/CD
• ✅ Workspace protocol (workspace:*) ensures fast local installs
• ⚠️ Consider using Turborepo or Nx for task caching if the monorepo grows

🎯 Breaking Changes

Good news: This refactor appears to have no breaking changes for consumers of the engine package:

• Package name changed from clockwork to @clockwork-engine/core
• Exports remain the same
• API surface unchanged

Note: Consumers will need to update their imports:

// Old
import { GameEngine } from 'clockwork'

// New  
import { GameEngine } from '@clockwork-engine/core'

Consider publishing this as a major version bump or documenting the migration path.

✅ Code Quality

• ✅ Consistent use of Biome for linting/formatting
• ✅ TypeScript strict mode enabled
• ✅ Proper module resolution configuration
• ✅ Good separation of build artifacts

📝 Recommendations Summary

1. Must Fix:

   • Add TypeScript project references in root tsconfig.json

2. Should Fix:

   • Document Vite alias behavior for development vs production
   • Optimize lint-staged configuration
   • Remove unnecessary prepublishOnly from root

3. Nice to Have:

   • Add workspace-level test script
   • Document migration path for package name change
   • Consider workspace tooling (Turborepo/Nx) for future scaling

🎉 Conclusion

This is a solid refactor that improves the project structure significantly. The main concern is the TypeScript configuration which should be addressed before merging. Overall, the changes follow monorepo best practices and maintain good code quality.

Recommendation: Approve with minor changes (fix the TypeScript project references before merging)

[claude]

Pull Request Review

Overview

This PR successfully converts the clockwork-engine project to a monorepo structure, splitting the platform-specific code into separate packages. Overall, this is a well-executed refactor with 188 changed files (+659/-1714 lines). The monorepo structure improves modularity and maintainability.

---

Issues Found

🔴 Critical: Coverage File Path Mismatch (.github/workflows/ci.yml:55)

Issue: The coverage upload step references the wrong path:

file: ./packages/engine/lcov.info

Problem: The package is named core, not engine. This will cause the CI coverage upload to fail.

Fix Required:

file: ./packages/core/lcov.info

---

Best Practices & Recommendations

✅ Strengths

1. Clean Separation of Concerns: The split into @clockwork-engine/core, @clockwork-engine/platform-memory, and @clockwork-engine/platform-web-pixi follows good architectural principles.

2. Proper Workspace Configuration: Bun workspaces are correctly configured with workspace:* protocol for inter-package dependencies.

3. Consistent Peer Dependencies: Platform packages correctly declare @clockwork-engine/core as a peer dependency (^2.7.0), preventing version conflicts.

4. Import Refactoring: All imports have been correctly updated from @hiddentao/clockwork-engine to the new scoped packages.

5. CI Workflow Simplification: Removed redundant demo-specific install/lint steps in favor of monorepo-wide commands.

6. Vite Aliases: Smart use of path aliases in demo/vite.config.ts enables development against source files rather than built artifacts.

🟡 Minor Concerns

1. GitIgnore Coverage: The .gitignore includes packages/*/dist and packages/*/node_modules, which is good. However, lcov.info is only ignored at the root level. Consider whether coverage files might be generated in package directories.

2. Lint-Staged Configuration: The new lint-staged config runs linting separately for each package. This is functional but may be slow for large changesets. Consider if parallel execution would help (though Bun handles this well).

3. Package.json Files in npm Package: The core package includes "bun.lock" in the files array (line 36 of packages/core/package.json). Lock files are typically excluded from published packages. Consider removing this unless there's a specific reason.

4. Release Scripts: All release scripts are delegated to packages/core. This works, but ensure version bumps are coordinated across all three packages if they should remain synchronized.

---

Code Quality

Architecture

• Score: 9/10
• Clean separation between core engine logic and platform implementations
• Platform abstraction layer is well-designed (packages/core/src/platform/index.ts)
• Platform implementations are properly decoupled as peer dependencies

TypeScript Usage

• Score: 10/10
• Proper use of TypeScript throughout
• Type exports are correctly maintained in package.json types fields
• No apparent type safety regressions

Build Configuration

• Score: 8/10
• Build scripts are properly configured for each package
• Monorepo build order is correct (core → platforms → demo)
• Missing: Consider adding build caching or incremental builds for faster CI

---

Performance Considerations

Build Performance

• Current: Sequential builds (bun run --cwd packages/core build && ...)
• Recommendation: Consider parallel builds where dependencies allow. Bun supports this natively with proper workspace configuration.

Development Experience

• Vite aliases enable fast HMR by avoiding rebuild steps ✅
• TypeScript project references could further improve type-checking speed (not currently used)

---

Security Concerns

Dependencies

• Status: ✅ Good
• Platform-specific dependencies (pixi.js, pixi-viewport) are properly isolated to platform-web-pixi
• Core engine has minimal dependencies (only alea PRNG)
• No new security-sensitive dependencies introduced

Package Publishing

• Status: ✅ Good
• publishConfig.access: "public" is correctly set for all packages
• Scoped package names (@clockwork-engine/*) prevent namespace squatting

---

Test Coverage

Testing Strategy

• Tests remain in packages/core ✅
• Coverage generation path seems correct (packages/core/lcov.info per sanitize script)
• Action Required: Fix the CI workflow coverage path (see Critical Issue above)

Test Organization

• Browser tests with Playwright are still functional
• Dev dependencies for platform packages are correctly placed
• Platform packages are available to core tests via workspace references

---

Summary

This is a well-executed monorepo refactor that successfully separates concerns and improves the project architecture. The code quality is high, and the migration is comprehensive.

Required Changes:

1. Fix coverage file path in .github/workflows/ci.yml:55 (engine → core)

Recommended Changes:

1. Consider removing bun.lock from core package's published files
2. Consider adding TypeScript project references for better IDE performance
3. Verify version synchronization strategy for the three packages

Approval Recommendation:

Approve after fixing the critical coverage path issue. The other items are optional improvements that can be addressed in follow-up PRs.

---

Review conducted following CLAUDE.md guidelines and Clockwork Engine architecture principles.

[claude]

Pull Request Review: Monorepo Refactoring

Summary

This is a major architectural refactoring that converts the clockwork-engine from a monolithic package to a well-structured monorepo with three distinct packages:

• @clockwork-engine/core - Core engine logic (platform-agnostic)
• @clockwork-engine/platform-web-pixi - Web/PIXI.js rendering platform
• @clockwork-engine/platform-memory - Headless/testing platform

The refactoring demonstrates excellent separation of concerns and sets up a solid foundation for future platform implementations.

---

✅ Strengths

1. Architecture & Design

• Excellent separation of concerns: Core engine is now truly platform-agnostic
• Clean package boundaries: Each package has a clear, focused responsibility
• Proper dependency management: Uses workspace references and peer dependencies correctly
• Maintainable structure: Monorepo organization makes it easy to develop and test packages together

2. Build System

• Consistent build scripts: Each package has its own build.ts with similar structure
• Multi-target builds: Packages correctly build for both Node.js (via tsc) and browser (via Bun.build)
• Good use of workspace scripts: Root package.json orchestrates builds across all packages
• Proper TypeScript config hierarchy: Extends from root with package-specific overrides

3. CI/CD Updates

• Streamlined workflows: CI workflow properly updated to work with monorepo structure
• Correct artifact paths: Coverage reports now point to packages/engine/lcov.info
• Simplified demo builds: Uses bun run demo:build instead of cd demo

4. Package Metadata

• Comprehensive package.json: All packages have proper metadata (keywords, repository, bugs, homepage)
• Correct exports: Each package exports only what's needed
• Proper file inclusion: files field correctly specifies what gets published

---

⚠️ Issues & Concerns

1. Critical: Outdated README (High Priority)

The core package README still references the old package name at packages/core/README.md:39-40. Should be updated to show installation of the new package names and how to import platform layers.

2. Documentation Gap (High Priority)

• Missing README files for platform-web-pixi and platform-memory packages
• Users need guidance on when to use each platform and how to create custom implementations
• Migration guide from old monolithic package would be helpful

3. Potential Build Order Issue (Medium Priority)

The root package.json build script uses sequential execution which works but is fragile. Consider using a build tool that understands workspace dependencies.

4. TypeScript Configuration (Low Priority)

The platform packages have types: [] in their tsconfig.json, which disables all @types packages. This might cause issues if they ever need DOM types.

5. Potential Circular Dependency Risk (Medium Priority)

The core package has platform packages as devDependencies which is correct for testing, but should ensure no production code imports from platform packages. Consider enforcing with a linting rule.

6. Missing Changelog Entries (Medium Priority)

The packages/core/CHANGELOG.md should include an entry for this major refactoring explaining the breaking change, migration instructions, and benefits.

7. Demo Package (Low Priority)

Demo package should be marked as private: true in its package.json since it's not meant to be published.

---

🛡️ Security Considerations

✅ No security concerns identified

• No new external dependencies introduced
• Existing dependencies remain pinned
• Build scripts use safe path operations

---

⚡ Performance Considerations

✅ Performance impact appears neutral to positive

• Smaller package sizes when users only need specific platforms
• Tree-shaking should work better with separate packages
• Bundle size tracking in build scripts is excellent

---

🧪 Test Coverage

The CI workflow shows tests are still running. I see PlatformSwitching.test.ts in the file list - excellent!

---

📋 Pre-Merge Checklist

Before merging, please address:

• HIGH: Update README.md with correct package names and platform import examples
• HIGH: Add README.md files for platform-web-pixi and platform-memory packages
• HIGH: Add CHANGELOG entry explaining the breaking change and migration path
• MEDIUM: Document build order and consider tooling to enforce it
• MEDIUM: Verify no accidental imports from platform packages in core
• MEDIUM: Consider adding a migration guide document
• LOW: Mark demo package as private: true
• LOW: Review TypeScript types: [] configuration

---

📊 Overall Assessment

Rating: 8/10 - Excellent architectural work with some documentation gaps

This is a well-executed refactoring that significantly improves the codebase architecture. The separation of concerns is clean, the build system is solid, and the workspace setup is correct. The main issues are around documentation and user-facing migration guidance.

Recommendation: Request changes - Address the documentation issues before merging.

Great work on this refactoring! 🎉

[claude]

Pull Request Review: Monorepo Refactor

I've completed a thorough review of this PR. This is a significant architectural improvement that splits the engine into well-organized packages. Here's my detailed feedback:

---

✅ Code Quality & Architecture

Excellent Design Decisions

1. Clean Package Separation ⭐

   • @clockwork-engine/core: Platform-agnostic engine core (only depends on alea for PRNG)
   • @clockwork-engine/platform-web-pixi: Web rendering via PIXI.js (peer depends on core)
   • @clockwork-engine/platform-memory: Headless platform for testing (peer depends on core)
   • Clear separation of concerns with proper dependency management

2. Proper Peer Dependencies ⭐

   • Platform packages correctly use peerDependencies for @clockwork-engine/core@^2.7.0
   • This prevents version conflicts and ensures consumers get a single core instance
   • Development uses workspace:* for local testing

3. TypeScript Configuration

   • Good use of extends pattern in package-level tsconfig files
   • Shared base config at root prevents duplication
   • Proper outDir, rootDir, and exclusions per package

4. Workspace Management

   • Bun workspaces properly configured with packages/* and demo
   • Build scripts correctly orchestrate multi-package builds
   • Version synchronization across packages (all at 2.7.1)

---

🐛 Issues Found

🔴 Critical: Coverage Path Bug in CI

File: .github/workflows/ci.yml:55

file: ./packages/engine/lcov.info

Problem: The path should be ./packages/core/lcov.info (not engine)

Impact: Coverage reporting to Coveralls will fail

Fix:

file: ./packages/core/lcov.info

---

⚠️ Medium Priority Issues

1. Missing Test Coverage Validation

The PR removes 1,728 lines and adds 869 lines (net -859). While this is expected for a refactor, I notice:

• No indication that tests still pass with the new package structure
• The CI workflow has been updated but needs verification
• Browser tests should validate the platform layer separation

Recommendation: Ensure all tests pass, especially:

• Unit tests in packages/core
• Integration tests using both platform layers
• Browser tests with actual PIXI rendering

2. Package Publishing Configuration

Issue: All three packages have publishConfig.access: "public" but there's no npm publishing step in CI/CD workflows.

Questions:

• Will these be published to npm separately?
• Should there be a publishing workflow?
• Are the version numbers synchronized intentionally?

3. Breaking Change for Consumers

This is a major breaking change for existing users:

Before:

import { GameEngine, GameCanvas } from "clockwork"

After:

import { GameEngine, GameCanvas } from "@clockwork-engine/core"
import { WebPlatformLayer } from "@clockwork-engine/platform-web-pixi"

Recommendation:

• Document migration guide in CHANGELOG.md
• Consider a major version bump (3.0.0) instead of patch (2.7.1)
• Update README.md with new import examples

---

🔒 Security Considerations

✅ Good Security Practices

1. No New Security Vulnerabilities

   • Dependencies remain minimal and well-audited
   • engineStrict: true enforces Node.js version requirements
   • No addition of untrusted packages

2. Workspace Security

   • Workspace protocol (workspace:*) prevents accidental external package resolution
   • Peer dependencies properly scoped to prevent supply chain issues

---

⚡ Performance Considerations

Positive Impacts

1. Tree-Shaking Improvements ⭐

   • Platform layers are now separate packages
   • Projects using headless testing won't bundle PIXI.js
   • Smaller bundle sizes for production builds

2. Build Performance

   • Incremental builds per package possible
   • TypeScript project references not yet utilized (future optimization opportunity)

Potential Concerns

• Build scripts now must coordinate 3 packages sequentially
• No parallel build optimization (could use tsc --build with project references)

Recommendation: Consider adding TypeScript project references for faster incremental builds:

// packages/core/tsconfig.json
{
  "composite": true,
  // ...
}

---

📋 Test Coverage

What's Missing

1. No Test Updates Visible

   • Tests should validate platform abstraction works correctly
   • Should test both MemoryPlatformLayer and WebPlatformLayer implementations
   • Integration tests should verify workspace dependencies resolve correctly

2. CI Configuration Changes

   • Removed separate demo linting step (now unified)
   • Coverage path needs fixing (see critical bug above)
   • Playwright tests should validate browser rendering still works

Recommendation:

• Add tests that import from package names (not relative paths) to validate publication setup
• Test that platform layers can be swapped without core changes

---

📝 Additional Recommendations

Documentation Updates Needed

1. README.md Updates

   • Update installation instructions for scoped packages
   • Show examples of importing from different packages
   • Document platform layer selection

2. CHANGELOG.md

   • Document breaking changes
   • Migration guide for existing users
   • Rationale for monorepo structure

3. Package READMEs

   • Each package should have its own README explaining its purpose
   • packages/core/README.md
   • packages/platform-web-pixi/README.md
   • packages/platform-memory/README.md

Code Organization

The demo shows good usage patterns:

import { GameObject, Vector2D } from "@clockwork-engine/core"
import { MemoryPlatformLayer } from "@clockwork-engine/platform-memory"

This validates the package separation is working correctly.

---

🎯 Summary

Must Fix Before Merge

• ✅ Fix coverage path in CI workflow (.github/workflows/ci.yml:55)

Should Address

• Consider version bump to 3.0.0 for breaking changes
• Add migration documentation
• Validate all tests pass with new structure
• Clarify npm publishing strategy

Nice to Have

• Add TypeScript project references for build performance
• Add per-package README files
• Optimize build scripts for parallelization

---

Final Verdict

This is a well-architected refactor that significantly improves the codebase structure. The package separation is clean, dependencies are properly managed, and the foundation is solid for future growth.

Once the critical coverage path bug is fixed and documentation is updated, this will be ready to merge.

Recommended Action: Request changes for the coverage path fix, then approve after verification.

Great work on this architectural improvement! 🚀

[claude]

Code Review: Monorepo Refactoring

Overview

This PR successfully refactors the clockwork-engine into a well-structured monorepo with three separate packages:

• @clockwork-engine/core - Platform-agnostic game engine core
• @clockwork-engine/platform-web-pixi - Web platform with PIXI.js rendering
• @clockwork-engine/platform-memory - Headless memory platform for testing

Overall Assessment: The refactoring is well-executed with clean separation of concerns. The architecture properly decouples platform-specific code from the core engine.

---

✅ Strengths

Architecture & Code Quality

1. Excellent separation of concerns - Core engine logic is now completely platform-agnostic
2. Clear dependency boundaries - Platform packages properly depend on core via peer dependencies
3. Consistent build scripts - All three packages follow the same build pattern (TypeScript + browser bundles)
4. Proper workspace setup - Using Bun workspaces with workspace:* protocol for local dependencies
5. Clean imports - Demo code cleanly imports from scoped packages (@clockwork-engine/core, etc.)

CI/CD Improvements

1. Simplified workflows - Removed redundant cd demo operations
2. Streamlined build process - Unified bun run build and bun run demo:build commands
3. Maintained test coverage - All test infrastructure properly migrated to core package

Best Practices

1. Consistent linting - Each package has its own lint configuration
2. Type safety maintained - TypeScript configs properly extended from root
3. Version consistency - All packages versioned at 2.7.1

---

⚠️ Issues Found

Critical Issue

❌ CI Workflow Bug (.github/workflows/ci.yml:55)

file: ./packages/engine/lcov.info

Should be:

file: ./packages/core/lcov.info

The package is named @clockwork-engine/core, not @clockwork-engine/engine. This will cause coverage upload to fail.

---

🔍 Recommendations

Documentation

1. Missing README files - Only packages/core/README.md exists. Consider adding:

   • packages/platform-web-pixi/README.md - Installation and usage guide
   • packages/platform-memory/README.md - Testing platform documentation
   • Root README.md update to reflect monorepo structure

2. Update root README - Should explain the monorepo structure and how to use the different packages

Package Configuration

3. Consider adding package descriptions to root package.json for better discoverability

4. Version management - Consider documenting the versioning strategy for independent packages vs. unified versioning

Build Optimization

5. Parallel builds - Root build script runs sequentially. Consider using & for parallel builds:

   "build": "bun run --cwd packages/core build & bun run --cwd packages/platform-memory build & bun run --cwd packages/platform-web-pixi build & wait"

6. Shared build script - The three scripts/build.ts files have significant duplication. Consider extracting to a shared build utility.

Security & Dependencies

7. Peer dependency ranges - Using ^2.7.0 for peer dependencies is good, but consider if you want stricter version requirements

8. No security concerns identified - Dependencies properly scoped, no credential leaks detected

---

📊 Test Coverage

• ✅ Tests properly migrated to packages/core/tests/
• ✅ Integration tests cover key functionality (determinism, recording, replay)
• ✅ Platform switching tests validate abstraction layer
• ✅ Coverage reporting configured correctly (aside from the path bug)
• ℹ️ Consider adding tests specifically for the new platform packages

---

🎯 Performance Considerations

1. Bundle sizes - Build scripts report bundle sizes, which is excellent for monitoring
2. Workspace dependencies - Using workspace:* provides fast local linking
3. No performance regressions expected - Refactoring is structural, not algorithmic
4. Potential improvement: Consider lazy-loading platform packages in demo to reduce initial bundle

---

📝 Summary

This is a high-quality refactoring that successfully achieves its goals. The monorepo structure is clean, well-organized, and follows best practices.

Action Required Before Merge:

• Fix the critical CI bug in .github/workflows/ci.yml:55 (engine → core)

Recommended for Future PRs:

• Add README files for platform packages
• Update root README to document monorepo structure
• Consider extracting shared build utilities

Great work on this refactoring! The architecture improvements will make the codebase much more maintainable and extensible. 🎉

[claude]

Pull Request Review: Monorepo Refactoring

Overall Assessment

This is a well-executed monorepo refactoring that successfully splits the Clockwork Engine into modular packages. The changes follow good software architecture principles by separating platform-specific code from the core engine. The refactoring maintains backward compatibility while enabling better code organization and independent versioning.

Overall Grade: 8.5/10 - Strong refactoring with a few minor issues to address.

---

Code Quality & Architecture ✅

Strengths

1. Clean Separation of Concerns: The split into three packages is logical:

   • @clockwork-engine/core - Platform-agnostic engine core
   • @clockwork-engine/platform-web-pixi - PIXI.js browser implementation
   • @clockwork-engine/platform-memory - Headless testing implementation

2. Proper Package Structure: Each package has:

   • Clear package.json with appropriate metadata
   • Correct peer dependencies setup
   • Build scripts and TypeScript configuration
   • Individual lint/format capabilities

3. Workspace Configuration: Bun workspaces are properly configured with workspace:* dependencies for local development

4. Export Organization: The core packages platform index properly documents that platform implementations are in separate packages

---

Potential Issues 🐛

Critical Issues

1. Path Mismatch in CI Workflow (packages/core vs packages/engine)

The CI workflow at .github/workflows/ci.yml:55 references ./packages/engine/lcov.info but the actual directory is packages/core. Should be ./packages/core/lcov.info.

2. Path Mismatch in Release Workflow

The release workflow at .github/workflows/release.yml:40-41 references packages/engine/CHANGELOG.md but should reference packages/core/CHANGELOG.md instead.

Minor Issues

3. Demo TypeScript Version Mismatch

demo/package.json:24 uses "typescript": "^5.8.3" while other packages use ^5.3.3. Consider standardizing for consistency.

4. Unused Alias in Vite Config

demo/vite.config.ts:12 has an unused @engine alias that can be removed if not needed.

---

Build System & Performance ⚡

Strengths

1. Efficient Build Scripts: The build.ts scripts use Buns native bundler efficiently
2. Proper Sourcemaps: Both dev and prod builds include sourcemaps
3. Sequential Package Builds: Root build script correctly chains package builds
4. Workspace Optimization: Eliminates demos separate node_modules (demo/bun.lock deleted)

Considerations

Build Order Dependency: The root build script has hardcoded build order which is fine since platform packages depend on core, but consider using Buns native workspace build features for automatic dependency resolution in the future.

---

Security 🔒

No Major Concerns

• No credentials or secrets in code
• Dependencies are pinned appropriately (pixi.js, alea)
• No introduction of new security vulnerabilities
• Workspace protocol prevents malicious package substitution

Recommendations

• Continue using exact versions (=) for critical dependencies like PIXI.js
• The engineStrict enforcement is good for preventing version mismatches

---

Test Coverage 🧪

Observations

1. Tests Remain in Core Package: All tests correctly moved to packages/core/tests
2. Browser Tests Updated: Import paths updated correctly
3. Coverage Path: Verify that ./packages/core/lcov.info is the correct output location (after fixing the path)

Missing

• No explicit tests for the monorepo structure itself
• Consider adding integration tests that verify package imports work correctly

---

Best Practices & Conventions 📝

Followed Well

1. ✅ Consistent package naming with scoped @clockwork-engine/*
2. ✅ Proper use of peerDependencies for platform packages
3. ✅ Individual biome.json for core package with custom settings
4. ✅ MIT license files in place
5. ✅ Consistent version numbers (2.7.1) across packages

Suggestions

1. Documentation: Update CLAUDE.md to reflect the new monorepo structure
2. README Updates: Ensure package READMEs explain the new architecture
3. Migration Guide: Consider adding a migration guide for users upgrading from pre-monorepo versions

---

Breaking Changes Assessment

User Impact: LOW ✅

For end users who only import the main package:

• Package name changes from clockwork to @clockwork-engine/core
• This is a breaking change but necessary for proper scoping
• Clear versioning (bump to 3.0.0?) might be appropriate

For developers working on the engine:

• File paths have changed significantly
• Import paths in demo updated correctly
• Should document this in CHANGELOG

---

Action Items

Must Fix Before Merge

1. Fix CI workflow path: packages/engine → packages/core (.github/workflows/ci.yml:55)
2. Fix release workflow paths: packages/engine → packages/core (.github/workflows/release.yml:40-41)

Should Fix

3. Standardize TypeScript version across all packages
4. Update CLAUDE.md to reflect monorepo structure
5. Add migration notes to CHANGELOG
6. Consider if this should be version 3.0.0 due to package name change

Nice to Have

7. Remove unused @engine alias from vite.config.ts
8. Add integration tests for monorepo structure
9. Document the platform abstraction layer architecture

---

Testing Recommendations

Before merging, please verify coverage file path is correct and all build/test commands work in the CI environment.

---

Conclusion

This is a strong refactoring that improves the codebase architecture significantly. The separation of concerns between core engine and platform-specific implementations is exactly the right approach and aligns well with the platform abstraction layer design.

The main issues are minor path mismatches in CI/CD configuration files that need immediate fixing. Once those are addressed, this PR will be ready to merge.

Great work on maintaining clean separation and proper dependency management throughout this significant restructuring! 🎉

---

Reviewed with reference to CLAUDE.md guidelines

[claude]

Pull Request Review: Monorepo Refactoring

Overview

This PR successfully refactors the clockwork-engine from a single-package repository into a well-structured monorepo with clear separation between core engine logic and platform-specific implementations.

✅ Code Quality & Architecture

Core Package - Zero dependencies on platform-specific implementations, pure deterministic game engine logic, clean platform abstraction via interfaces

Platform Implementations:

• Memory Platform - Headless for testing/server with in-memory state tracking
• Web-PIXI Platform - Browser rendering with PIXI.js, DOM events, Web Audio API

Dependency Flow: Acyclic and clean, no circular dependencies detected

✅ Best Practices

• All packages use modern ESM
• Proper peer dependencies configured
• Version synchronization: all at 2.7.1
• Branded types prevent type confusion
• Proper build orchestration: Core → Platforms → Demo

✅ CI/CD Configuration

Minor Issue Found: Release workflow references packages/engine/CHANGELOG.md instead of packages/core/CHANGELOG.md

Fix needed in .github/workflows/release.yml:41:

- if [ -f "packages/engine/CHANGELOG.md" ]; then
+ if [ -f "packages/core/CHANGELOG.md" ]; then

✅ Test Coverage

• Tests properly moved to packages/core/tests/
• Playwright browser tests included
• Integration tests use MemoryPlatformLayer
• Demo correctly imports from all three packages

✅ Performance & Security

• Tree-shakeable with proper ESM structure
• Minimal production dependencies (only alea in core)
• No security vulnerabilities detected
• No TODOs, FIXMEs, or HACKs found
• Platform abstraction adds negligible overhead

📊 Statistics

• Net: -855 lines (better organization)
• Files changed: 100+
• Packages created: 3

Recommendations

1. Required: Fix release workflow CHANGELOG path
2. Recommended: Add migration guide for existing users
3. Optional: Consider version sync tooling for future releases

✅ Final Verdict: APPROVE

This is an excellent refactoring demonstrating strong software engineering:

• Clean architectural separation
• Platform independence enabling multi-target deployment
• Test-driven development with headless platform
• Modern tooling (Bun workspaces, ESM, TypeScript)
• Minimal dependencies

The code is ready to merge after fixing the release workflow path. Excellent work! 🚀

---

Reviewed by: Claude Code | Date: 2025-12-12

[claude]

Pull Request Review: Monorepo Refactor

Overview

This PR refactors the clockwork-engine project into a monorepo structure, separating the core engine from platform-specific implementations. This is a significant architectural improvement.

Strengths

• Excellent separation of concerns between core and platform code
• Clear package boundaries (core, platform-web-pixi, platform-memory)
• Proper peer dependencies configured
• Consistent build scripts across packages
• CI/CD workflows properly updated

Critical Issue

Location: .github/workflows/release.yml:40

The path references packages/engine/CHANGELOG.md but should be packages/core/CHANGELOG.md

Impact: Release workflow will fail

Other Concerns

1. Node >=22.0.0 requirement may limit adoption (consider >=20.0.0 LTS)
2. Builds run sequentially - could parallelize platform packages after core
3. lint-staged only checks, doesn't auto-fix
4. Missing migration guide for users upgrading from @hiddentao/clockwork-engine
5. No workspace-level integration tests

Overall Assessment

Score: 8/10

Well-executed refactoring with excellent architecture. Fix the release.yml path before merging. Other issues can be follow-up PRs.

Great work!

[claude]

PR Review: Monorepo Conversion

Overview

This PR successfully converts the Clockwork Engine from a monolithic structure to a well-organized monorepo with separate packages. The refactoring creates a clean separation between the core engine, platform-specific implementations, and the demo application.

Stats: 195 files changed, +1,435 / -1,825 lines

---

✅ Strengths

1. Excellent Architecture & Separation of Concerns

• Core package (@clockwork-engine/core): Clean extraction of platform-agnostic game engine logic
• Platform packages: Smart separation into platform-web-pixi (PIXI.js rendering) and platform-memory (headless testing)
• Peer dependencies: Proper use of peer dependencies (@clockwork-engine/core: ^2.7.0) ensures version compatibility
• This architecture enables:
  • Testing without browser dependencies
  • Future platform implementations (Canvas 2D, WebGL, etc.)
  • Smaller bundle sizes for consumers who only need specific platforms

2. Build System & Tooling

• Unified build script (scripts/run.ts): Well-designed monorepo task runner with include/exclude filters
• Version sync utility: sync-versions command ensures consistent versioning across packages
• Build scripts: Each package has its own build configuration while maintaining consistency
• Workspace setup: Proper Bun workspaces configuration with workspace:* protocol

3. CI/CD Improvements

• Streamlined workflow by removing redundant demo-specific steps
• Correct coverage path: ./packages/core/lcov.info
• Maintained all critical checks (linting, tests, browser tests, coverage)
• Demo build properly integrated into monorepo structure

4. Documentation Updates

• README properly updated with new package structure
• Clear installation instructions for each package
• Updated badges and links to reflect new package names

---

⚠️ Issues & Concerns

1. Security: Command Injection Risk in scripts/run.ts

Severity: Medium

Lines 128-130 in scripts/run.ts:

if (hasScript) {
  await $`bun run --cwd ${pkg.path} ${command}`.quiet()
} else {
  await $`cd ${pkg.path} && ${command}`.quiet()  // ⚠️ Unsanitized command execution
}

Issue: The command parameter is directly interpolated into shell execution without validation. While this is an internal script, it could execute arbitrary commands if called with malicious input.

Recommendation:

• Add input validation for the command parameter
• Use array-based command execution instead of string interpolation for the else branch
• Or document that this is an internal-only script and should not accept untrusted input

2. Missing Build Script for Platform Packages

The platform-web-pixi and platform-memory packages reference scripts/build.ts in their package.json, but I don't see these scripts in the diff.

Questions:

• Do these packages have build scripts?
• Are they using the same build script as core?
• Should they only run TypeScript compilation without browser bundles?

Recommendation: Verify that all packages can build independently with bun run build.

3. Potential Circular Dependency in Dev Dependencies

In packages/core/package.json:

"devDependencies": {
  "@clockwork-engine/platform-memory": "workspace:*",
  "@clockwork-engine/platform-web-pixi": "workspace:*",
  ...
}

And platform packages have:

"peerDependencies": {
  "@clockwork-engine/core": "^2.7.0"
},
"devDependencies": {
  "@clockwork-engine/core": "workspace:*"
}

Concern: This creates a circular dependency graph. While it works with workspaces, it could cause issues during:

• Publishing to npm
• Installing from npm (not from workspace)
• Build order determination

Recommendation:

• Document the build order (core → platforms → demo)
• Consider if platform packages really need to be in core's devDependencies, or if they should just be used in tests
• Test the npm publish/install flow to ensure it works correctly

4. Version Configuration File Complexity Reduced

The .versionrc.json was significantly simplified, removing many useful configurations:

Removed:

• Custom commit URL formats
• Issue URL formats
• User URL formats
• Release commit message format
• Prerelease script (linting + tests + build)
• Many commit type sections

Concern: The removed prerelease script was valuable as it ran bun run lint && bun test && bun run build before releases. Now this safety check is gone.

Recommendation:

• Consider adding the prerelease script back
• Document why these configurations were removed if intentional

5. Coverage Path Update

✅ Good catch updating the coverage path from ./lcov.info to ./packages/core/lcov.info

However, verify that:

• Coverage is actually generated at this location
• The sanitize-lcov.sh script (referenced in core's coverage script) handles the monorepo structure correctly

---

🔍 Code Quality

Positive:

• ✅ Consistent TypeScript configuration across packages
• ✅ Proper use of ES modules ("type": "module")
• ✅ Engine requirements enforced (node >= 22.0.0)
• ✅ Lint-staged configuration updated for monorepo structure
• ✅ Clean package.json files with appropriate metadata

Suggestions:

• Consider adding a root-level turbo.json or similar for better build caching and parallelization
• Add a CONTRIBUTING.md that explains the monorepo structure and development workflow

---

🧪 Test Coverage

The test infrastructure appears intact:

• Browser tests with Playwright
• Unit tests with Bun
• Coverage reporting to Coveralls
• Tests remain in packages/core/tests/

Recommendation: Verify that test coverage hasn't decreased after the refactor by comparing coverage percentages before/after.

---

📦 Performance Considerations

Positive:

• Smaller package sizes for consumers (only install what you need)
• Workspace protocol (workspace:*) ensures fast local development
• Build script runs packages in sequence (safe but could be parallelized)

Optimization Opportunity:

The scripts/run.ts runs packages sequentially. For independent operations (like linting), parallel execution would be faster:

// Current: Sequential
for (const pkg of packages) {
  await runCommand(pkg, command)
}

// Suggested: Parallel for safe operations
await Promise.all(packages.map(pkg => runCommand(pkg, command)))

---

🎯 Breaking Changes

Package Name Change

• Old: @hiddentao/clockwork-engine
• New: @clockwork-engine/core

Impact: This is a breaking change for all consumers.

Recommendations:

1. Publish a final version of @hiddentao/clockwork-engine that exports a deprecation notice
2. Update the CHANGELOG to clearly document the migration path
3. Consider publishing both packages temporarily with the old one depending on the new one
4. Bump to v3.0.0 to signal the breaking change (currently at 2.7.1)

---

✨ Summary

This is a well-executed refactoring that significantly improves the project structure. The monorepo architecture is sound, the separation of concerns is excellent, and the tooling is well-designed.

Must Fix Before Merge:

1. ⚠️ Address the command injection concern in scripts/run.ts
2. ⚠️ Verify platform package build scripts exist and work
3. ⚠️ Test the npm publish/install flow for circular dependency issues

Nice to Have:

• Add prerelease validation script back to .versionrc.json
• Document build order and development workflow
• Parallelize independent operations in scripts/run.ts
• Bump to v3.0.0 for breaking changes
• Add deprecation notice for old package name

Approval Status:

Approved with minor concerns - The architectural improvements outweigh the issues, but please address the security and dependency concerns before merging.

Great work on this refactoring! 🎉

[claude]

Pull Request Review: Monorepo Refactor

Overall Assessment

This is a well-structured monorepo refactor that successfully separates the engine core from platform-specific implementations. The changes demonstrate good architectural thinking and improve the project's modularity. ✅

---

🎯 Strengths

Architecture & Design

• Excellent separation of concerns: Core engine logic is now cleanly separated from rendering implementations
• Platform abstraction: The introduction of separate packages (platform-web-pixi and platform-memory) allows for multiple rendering backends
• Peer dependency management: Proper use of peerDependencies ensures version compatibility between packages
• Workspace structure: Bun workspaces configuration is correct with proper dependency resolution

Code Quality

• Consistent package structure: All packages follow the same structure with build scripts, tsconfig, and exports
• Export organization: Clean index.ts files in each package with logical groupings
• Type safety maintained: TypeScript configurations are properly inherited from root
• Build system: Unified build script (scripts/run.ts) provides good DX for monorepo operations

CI/CD

• Streamlined workflows: Removed redundant demo-specific steps, consolidated into monorepo commands
• Coverage configuration: Correctly updated to point to packages/core/lcov.info
• Build optimization: Single bun run demo:build instead of multiple cd commands

---

⚠️ Issues & Recommendations

Critical

1. Missing Test Coverage Verification 🔴

• Issue: The PR changes 100 files but I don't see evidence of test execution to verify nothing broke
• Recommendation: Run the full test suite before merging:

  bun run test
  bun run test:browser

High Priority

2. Package Publication Concerns 🟡

• Issue: Core package package.json line 30-31 includes both dist/clockwork-engine.js and dist/clockwork-engine.min.js in files array, but I don't see these being generated in the build scripts
• Location: packages/core/package.json:30-31
• Recommendation: Verify these files are actually generated or remove them from the files array

3. Version Synchronization 🟡

• Issue: All packages are at version 2.7.1, but this is a major architectural change
• Recommendation: Consider whether this warrants a major version bump (3.0.0) due to:
  • Package name change: @hiddentao/clockwork-engine → @clockwork-engine/core
  • Breaking change in how users import platform implementations
  • New installation requirements (need both core + platform package)

4. Migration Guide Missing 🟡

• Issue: Existing users will need to update imports and dependencies
• Recommendation: Add a MIGRATION.md guide documenting:

  - import { GameEngine } from '@hiddentao/clockwork-engine'
  + import { GameEngine } from '@clockwork-engine/core'
  + import { WebPlatformLayer } from '@clockwork-engine/platform-web-pixi'

Medium Priority

5. Lint-Staged Configuration 🟠

• Issue: package.json:41-52 runs lint per-directory which could be slow
• Recommendation: Consider running biome check at root level for changed files:

  "lint-staged": {
    "**/*.{ts,tsx}": ["biome check"]
  }

6. Demo Package Version 🟠

• Issue: Demo package has version "1.0.0" while core is at "2.7.1"
• Recommendation: Either remove version from demo (it's not published) or keep it at 0.0.0 to indicate it's internal

7. Build Script Error Handling 🟠

• Location: scripts/run.ts:199-204
• Issue: Script breaks on first failure, could be frustrating during development
• Recommendation: Consider adding a --continue-on-error flag for development scenarios

Low Priority

8. TypeScript Configuration 🟢

• Issue: Root tsconfig includes "DOM" types which aren't needed for core package
• Recommendation: Consider moving DOM types to platform-web-pixi tsconfig only

9. README Consistency 🟢

• Issue: Root README shows old import style in Quick Start example
• Location: README.md (ensure all examples reflect new package structure)
• Recommendation: Verify all code examples use the new package imports

10. Workspace Dependencies 🟢

• Issue: packages/core/package.json has platform packages as devDependencies but they're only used for testing
• Recommendation: Document why these are devDependencies (for testing) to avoid confusion

---

🔒 Security Considerations

• ✅ No obvious security vulnerabilities introduced
• ✅ Dependencies are pinned appropriately (alea, pixi.js, pixi-viewport)
• ✅ No credentials or secrets exposed
• ⚠️ Ensure .npmignore or package.json files array prevents publishing test files, scripts, or dev configs

---

📊 Performance Considerations

• ✅ Build process looks efficient with proper incremental compilation
• ✅ Separation into packages allows for tree-shaking (users only import what they need)
• ✅ No obvious performance regressions in the refactor
• 💡 Consider documenting bundle size impact in README (core vs core+platform)

---

🧪 Test Coverage

Recommendations:

1. Verify all existing tests pass: bun run test && bun run test:browser
2. Add integration tests for workspace dependencies
3. Test the actual npm publish flow in a test registry before releasing
4. Verify demo still works correctly: bun run demo

---

📋 Pre-Merge Checklist

Before merging, please ensure:

• All tests pass (unit + browser)
• Demo application runs without errors
• Build artifacts are correctly generated for all packages
• Add MIGRATION.md for existing users
• Consider version bump to 3.0.0 for breaking changes
• Verify package.json "files" arrays include all necessary files
• Update all README examples to use new package structure
• Test actual package publication to npm (use --dry-run first)

---

Summary

This is a high-quality refactor that significantly improves the project's architecture. The main concerns are around:

1. Breaking changes requiring migration documentation and version bump
2. Publication verification to ensure packages work when installed from npm
3. Test coverage to confirm nothing broke in the refactor

Once the critical and high-priority items are addressed, this will be ready to merge! 🚀

Recommendation: Approve with requested changes

[claude]

Code Review: Monorepo Refactoring

Overview

This is a well-executed monorepo refactoring that successfully splits the engine into modular packages with clear separation of concerns. The changes are primarily structural with minimal code modifications, which reduces risk.

---

✅ Strengths

Architecture & Design

• Excellent separation of concerns: Core engine logic separated from platform-specific implementations
• Clean package boundaries:
  • @clockwork-engine/core - Pure engine logic without rendering dependencies
  • @clockwork-engine/platform-web-pixi - PIXI.js web rendering
  • @clockwork-engine/platform-memory - Headless platform for testing
• Platform abstraction: The new platform layer exports (packages/core/src/platform/index.ts:37) provide clean interfaces

Code Quality

• Minimal code changes: Most changes are import path updates, not logic modifications
• Backward compatibility: Demo application updated with new imports shows migration path is straightforward
• Type safety maintained: TypeScript strict checks still in place across all packages
• Consistent tooling: Biome formatting/linting unified across packages via scripts/run.ts

Build & CI/CD

• Improved CI workflow: Simplified from dual demo+engine builds to unified monorepo build (.github/workflows/ci.yml:26)
• Smart release process: scripts/release.ts handles version syncing and publishing elegantly
• Monorepo utilities: scripts/run.ts provides excellent tooling for running commands across packages
• Proper dependency resolution: Workspace protocol (workspace:*) used correctly

---

🔍 Issues & Concerns

1. Missing Package Build Scripts (CRITICAL)

The new platform packages (platform-web-pixi, platform-memory) reference build scripts that don't exist:

// packages/platform-web-pixi/package.json:43
"build": "bun run scripts/build.ts"

Problem: These packages don't have a scripts/ directory. The build will fail.

Recommendation:

• Copy packages/core/scripts/build.ts to each platform package, OR
• Use simple tsc builds if the complex bundling isn't needed, OR
• Create shared build scripts in the root

---

2. Version Sync Configuration Incomplete (HIGH)

.versionrc.json only bumps packages/core/package.json:

{
  "packageFiles": ["packages/core/package.json"],
  "bumpFiles": ["packages/core/package.json"]
}

Problem: Platform packages won't get version bumps automatically by commit-and-tag-version.

Current Mitigation: scripts/release.ts:54 runs sync-versions which manually syncs versions. This works but is fragile.

Recommendation: Either document this clearly OR update .versionrc.json to include all packages.

---

3. Git Commit Amending in Release (MEDIUM - Security)

scripts/release.ts:70 uses git commit --amend:

await run("git commit --amend --no-edit", "Amending commit with synced versions")

Concern: This bypasses pre-commit hooks (--no-verify not used, so hooks run on initial commit but not amend).

Recommendation:

• Add comment explaining why amend is necessary
• Consider using two separate commits instead
• Ensure this is documented in contribution guidelines

---

4. Peer Dependency Version Range (MEDIUM)

Platform packages use ^2.7.0 peer dependency:

"peerDependencies": {
  "@clockwork-engine/core": "^2.7.0"
}

Concern: This allows core@2.8.0 with platform-web-pixi@2.7.0, which could cause runtime issues if breaking changes occur.

Recommendation: Consider using exact version matching or tilde ~2.7.0 for patch-only updates.

---

5. Missing READMEs for Platform Packages (LOW)

Platform packages list README.md in files array but the glob didn't find full content.

Recommendation: Verify platform packages have adequate README documentation.

---

6. Coverage Path in CI (LOW)

CI workflow references coverage at new location:

file: ./packages/core/lcov.info

Verification needed: Ensure packages/core/scripts/sanitize-lcov.sh outputs to this location.

---

🧪 Testing & Validation

Good:

• ✅ Tests moved with code to packages/core/tests/
• ✅ Playwright browser tests maintained
• ✅ CI runs full test suite including browser tests

Needs Verification:

• All tests still pass after monorepo migration
• Coverage reporting works correctly with new paths
• Demo builds and runs successfully

---

📊 Breaking Changes Assessment

For End Users: BREAKING CHANGE ⚠️

Before:

import { GameEngine } from '@hiddentao/clockwork-engine'

After:

import { GameEngine } from '@clockwork-engine/core'
import { WebPlatformLayer } from '@clockwork-engine/platform-web-pixi'

Migration Required:

1. Update package name: @hiddentao/clockwork-engine → @clockwork-engine/core
2. Install platform package: @clockwork-engine/platform-web-pixi
3. Update all import paths
4. Separately instantiate platform layer (see demo/src/Game.ts:9-13)

Recommendation:

• Bump to v3.0.0 to signal breaking change
• Provide migration guide in CHANGELOG
• Consider deprecation notice on old package

---

📋 Security Review

No security concerns identified:

• No new external dependencies introduced
• Existing dependencies (alea, pixi.js, pixi-viewport) unchanged
• No credential or secret handling in changed files
• Git operations in scripts are safe (no force push to protected branches)

---

🎯 Performance Considerations

Positive:

• Splitting packages allows users to install only what they need
• Smaller bundle sizes for projects not using PIXI.js
• Tree-shaking should work better with clearer module boundaries

Neutral:

• Build time may increase slightly due to multiple packages
• Workspace resolution adds minimal overhead

---

📝 Recommendations Summary

Before Merge:

1. [CRITICAL] Add build scripts to platform packages or update package.json to use tsc directly
2. [HIGH] Verify version sync strategy is documented
3. [MEDIUM] Test full release process in dry-run mode
4. [MEDIUM] Verify platform package READMEs exist and are complete
5. [MEDIUM] Document this as a breaking change (suggest v3.0.0)

After Merge:

1. Create migration guide for users
2. Publish all three packages simultaneously
3. Add deprecation notice to old npm package
4. Update any external documentation

---

🏁 Conclusion

This is high-quality refactoring work that significantly improves the project architecture. The separation of concerns is well-designed and the monorepo tooling is solid.

Approval recommendation: ✅ APPROVE with required fixes

Address the critical build script issue before merging. The other issues are important but not blocking.

Great work on maintaining code quality and test coverage throughout this major refactoring! 🎉

---

Reviewed with: Clockwork Engine project guidelines (CLAUDE.md), focus on deterministic architecture and build system requirements.