feat(core): add prune API to clear event history #650
Conversation
|
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new prune() action to the public actions surface and implements it to rebuild a minimal event history from currently active activities, replace stored events, and re-aggregate the stack; a unit test verifying prune behavior was added. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Actions as Actions.prune()
participant Store
participant Events as Events.value
participant Aggregate as aggregate()
Caller->>Actions: prune()
Actions->>Store: getStack()
Store-->>Actions: current Stack
rect rgb(230,245,235)
Note over Actions: Filter active activities\n(build activity metadata list)
Actions->>Actions: Build synthetic events:\n- Initialized\n- ActivityRegistered* (reuse metadata)\n- Pushed / StepPushed / StepReplaced\n- Popped (if needed)
end
Actions->>Events: events.value = [synthetic events]
rect rgb(245,235,235)
Actions->>Aggregate: aggregate(events.value)
Aggregate-->>Actions: new Stack
end
Actions->>Store: setStackValue(new Stack)
Store-->>Caller: stack & events updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)core/src/utils/makeActions.ts (4)
🔇 Additional comments (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
core/src/makeCoreStore.spec.ts (1)
285-351: Consider adding edge case tests.The test covers the basic scenario well. However, consider adding tests for:
- Prune with multiple active activities - verify ordering is preserved
- Prune with activities containing steps - steps appear to be lost in reconstruction
- Prune with no active activities - what happens when all activities are
exit-done?Also note that after pruning, the "detail"
ActivityRegisteredis lost. If a user tries topusha "detail" activity after prune, it may fail validation since that activity is no longer registered.core/src/utils/makeActions.ts (1)
138-174: All registered activities not currently active are lost.After pruning, only activity names with active instances are re-registered. If you have registered activities (e.g., "settings", "profile") that aren't currently in the stack but may be pushed later, they'll be lost and subsequent
pushcalls for those activities may fail.Consider preserving the full
registeredActivitieslist from the current stack to maintain the complete activity registry.+ const { activities, registeredActivities, transitionDuration } = store.getStack(); - const { activities } = store.getStack(); const activeActivities = activities.filter( (activity) => activity.transitionState === "enter-active" || activity.transitionState === "enter-done", ); const now = new Date().getTime(); const newEvents: DomainEvent[] = [ makeEvent("Initialized", { - transitionDuration: 0, + transitionDuration, eventDate: now, }), - ...activeActivities.map((activity) => + ...registeredActivities.map(({ name }) => makeEvent("ActivityRegistered", { - activityName: activity.name, + activityName: name, eventDate: now, }), ),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
core/src/interfaces/StackflowActions.ts(1 hunks)core/src/makeCoreStore.spec.ts(1 hunks)core/src/makeCoreStore.ts(2 hunks)core/src/utils/makeActions.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
core/src/interfaces/StackflowActions.tscore/src/makeCoreStore.spec.tscore/src/utils/makeActions.tscore/src/makeCoreStore.ts
**/*.spec.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name test files with the .spec.ts suffix and keep them alongside source files
Files:
core/src/makeCoreStore.spec.ts
🧬 Code graph analysis (1)
core/src/utils/makeActions.ts (4)
core/src/interfaces/StackflowActions.ts (1)
StackflowActions(15-70)core/src/Stack.ts (1)
Stack(50-57)core/src/event-types/index.ts (1)
DomainEvent(12-22)core/src/aggregate.ts (1)
aggregate(7-113)
🔇 Additional comments (3)
core/src/interfaces/StackflowActions.ts (1)
65-69: LGTM!The interface extension is clean and the JSDoc accurately describes the method's purpose. The no-argument signature is appropriate for this operation.
core/src/makeCoreStore.ts (1)
137-141: LGTM!The store object properly exposes the required internals (
getStack,events,setStackValue) for thepruneimplementation while keeping the pattern consistent with how other actions are wired.core/src/utils/makeActions.ts (1)
159-167: Step state is indeed lost during prune—verify if this is acceptable for your use case.The review comment is technically accurate. The
Activitytype includes asteps: ActivityStep[]property, but theprune()function only emitsInitialized,ActivityRegistered, andPushedevents. SinceStepPushedevents populate the steps array during activity reconstruction, omitting them results in an empty steps array after prune.This appears intentional—prune is documented as "clear past events and reconstruct state with active activities to release memory." However, if your application relies on step history (e.g., for URL state synchronization via the history-sync plugin), this design choice could cause issues. Confirm whether step preservation is required for your use case.
- Preserve transitionDuration from current stack instead of hardcoding to 0 - Deduplicate ActivityRegistered events when multiple activities share same name
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
core/src/utils/makeActions.ts (1)
138-144: Consider validating stack state before pruning.The prune operation doesn't check:
- Whether
activeActivitiesis empty (all activities could be exiting)- Whether
globalTransitionStateis"idle"(pruning during transitions could cause issues)Consider adding guards:
prune() { - const { activities } = store.getStack(); + const stack = store.getStack(); + + if (stack.globalTransitionState !== "idle") { + console.warn("Cannot prune during active transitions"); + return; + } + + const { activities } = stack; const activeActivities = activities.filter( (activity) => activity.transitionState === "enter-active" || activity.transitionState === "enter-done", ); + + if (activeActivities.length === 0) { + console.warn("Cannot prune with no active activities"); + return; + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
core/src/utils/makeActions.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
core/src/utils/makeActions.ts
🧬 Code graph analysis (1)
core/src/utils/makeActions.ts (4)
core/src/interfaces/StackflowActions.ts (1)
StackflowActions(15-70)core/src/Stack.ts (1)
Stack(50-57)core/src/event-types/index.ts (1)
DomainEvent(12-22)core/src/aggregate.ts (1)
aggregate(7-113)
🔇 Additional comments (3)
core/src/utils/makeActions.ts (3)
1-5: LGTM: Imports are appropriate for the prune implementation.All new imports are properly typed and necessary for the event reconstruction logic.
12-16: LGTM: Store interface provides appropriate access for pruning.The store object cleanly exposes the minimal interface needed for the prune operation.
157-162: Prune intentionally removes inactive registered activities—this is not a bug.The test in
core/src/makeCoreStore.spec.ts:285explicitly validates this behavior: "when prune is called, it cleans up past events and maintains current state." Theprune()operation reconstructs the event history from scratch, keeping only currently active activities. This design choice allows the function to reduce event log size by discarding obsolete registrations.The review comment treats this as unintended data loss, but the test suite confirms this is the intended behavior. No evidence in the codebase indicates that applications depend on preserving pre-registered but inactive activities after pruning.
Likely an incorrect or invalid review comment.
| store.events.value = newEvents; | ||
|
|
||
| const nextStackValue = aggregate(store.events.value, now); | ||
| store.setStackValue(nextStackValue); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling to prevent inconsistent state.
If aggregate throws an exception, store.events.value will already be mutated but store.setStackValue won't be called, leaving the store in an inconsistent state.
Wrap in try-catch to handle errors gracefully:
-store.events.value = newEvents;
-
-const nextStackValue = aggregate(store.events.value, now);
-store.setStackValue(nextStackValue);
+try {
+ const nextStackValue = aggregate(newEvents, now);
+ store.events.value = newEvents;
+ store.setStackValue(nextStackValue);
+} catch (error) {
+ console.error("Failed to prune stack:", error);
+ // Store remains in original state
+}This ensures atomicity—either both updates succeed or neither does.
🤖 Prompt for AI Agents
In core/src/utils/makeActions.ts around lines 174 to 177, assigning
store.events.value before calling aggregate can leave the store half-updated if
aggregate throws; compute the aggregated value before mutating or wrap the
mutation+aggregation in a try-catch that reverts store.events.value on error and
then rethrows or logs the error. Specifically, call aggregate(now) using a
non-mutated copy of events (or calculate nextStackValue first), then set
store.events.value and store.setStackValue atomically, or if you must assign
events first, catch any exception from aggregate, restore the previous
store.events.value, and surface the error so the store never remains in an
inconsistent state.
- Preserve original eventDate for temporal ordering - Preserve activityContext and activityParamsSchema - Reconstruct activity steps with correct event types - Preserve hasZIndex metadata
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
core/src/utils/makeActions.ts (1)
243-246: Makeaggregate+ store updates atomic to avoid inconsistent stateYou still assign
store.events.valuebefore callingaggregate, so ifaggregatethrows (due to a validation bug, unexpected event shape, etc.), the events are mutated but the stack is not, leaving the store inconsistent. This concern was raised earlier and is still applicable.You can make the operation atomic by aggregating on the new events first and only mutating the store after success:
- store.events.value = newEvents; - - const nextStackValue = aggregate(store.events.value, now); - store.setStackValue(nextStackValue); + const nextStackValue = aggregate(newEvents, now); + store.events.value = newEvents; + store.setStackValue(nextStackValue);This way, if
aggregatefails, botheventsandstackremain untouched.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
core/src/utils/makeActions.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Write source in TypeScript with strict typing enabled across the codebase
Files:
core/src/utils/makeActions.ts
🧬 Code graph analysis (1)
core/src/utils/makeActions.ts (4)
core/src/interfaces/StackflowActions.ts (1)
StackflowActions(15-70)core/src/Stack.ts (1)
Stack(50-57)core/src/event-types/index.ts (1)
DomainEvent(12-22)core/src/aggregate.ts (1)
aggregate(7-113)
🔇 Additional comments (1)
core/src/utils/makeActions.ts (1)
1-24: Imports and newstoreplumbing look consistent and type‑safeThe new imports (
aggregate,DomainEvent,makeEvent,Stack) and thestorefield added toActionCreatorOptions/makeActionsare coherent with the newpruneimplementation and keep strict typing intact. No issues here.
1 participant
Summary
Adds
prune()API toStackflowActionsin@stackflow/coreto manually clear past events and inactive activities.Motivation
Stackflow's Event Sourcing pattern causes the event history to grow indefinitely. This leads to increased memory usage and re-calculation costs in long-running sessions. A mechanism to clear old history and release memory was required.
Features
actions.prune(): Clears event history while preserving the current stack state.enter-activeandenter-doneactivities.