-
Notifications
You must be signed in to change notification settings - Fork 4
State refactors and bug fixes #260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: 1d56992 The changes in this PR will be included in the next version bump. This PR includes changesets to release 7 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Extract the fetch pipeline from LaunchpadContent into composable stage functions (setupHooks, backup, clearOldData, fetchSources, etc.) for better testability and modularity. Simplify state management with inline phase tracking. Add comprehensive tests for fetch context and stages.
…ller allows for more type safety when developing subsystems, and when using subsystems in isolation.
…ils package Allows for declaration merging, so that the config is easier to extend with future subsystems, and the cli package doesn't cause the content/monitor types to register if they're not installed.
Moving state to separate manager classes makes it clearer and more maintainable. Also adds patch event emission logic, which will eventually help us keep state in sync across transports.
At the moment, this doesn't use the eventbus, as we don't really need these events dispatched elsewhere.
5ba85c1 to
fa24218
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors state management across the monitor and content subsystems to use Immer for immutable state updates with patch generation. The changes enable real-time state synchronization across processes via patch events, and add a new watch flag to the CLI status command for live monitoring.
Key Changes:
- Introduced
PatchedStateManagerclass in utils package using Immer for state management with patch generation - Refactored content fetch pipeline into composable stage functions for better testability
- Moved declaration merging interfaces from controller to utils package
- Added state patch broadcasting over IPC transport
Reviewed Changes
Copilot reviewed 54 out of 55 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/state-patcher.ts | New state manager class using Immer for patch-based state updates |
| packages/monitor/src/monitor-state.ts | Refactored to use PatchedStateManager with state mutation methods |
| packages/content/src/content-state.ts | Refactored to use discriminated unions for phase-based state tracking |
| packages/content/src/launchpad-content.ts | Extracted fetch pipeline into stages, added loadSources() initialization |
| packages/content/src/fetching/fetch-stages.ts | New modular fetch pipeline stages as composable functions |
| packages/controller/src/core/state-store.ts | Added patch relay and version tracking for subsystem state changes |
| packages/controller/src/transports/ipc-transport.ts | Added state patch broadcasting to IPC clients |
| packages/controller/src/ipc/ipc-client.ts | Added patch application and state change listener support |
| packages/cli/src/commands/status.ts | Added watch mode for live status monitoring |
Comments suppressed due to low confidence (1)
packages/content/src/launchpad-content.ts:1
- Corrected spelling of 'a error' to 'an error'.
import path from "node:path";
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
remove tests that were testing logging behavior. These tests were brittle and not essential to the core functionality being tested.
f77d25e to
7686485
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 54 out of 55 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.