[zigzagdev]

Summary

Fix the StepPopped activity reducer so it no longer mutates the previous
activity's steps array in place.

Motivation

In core/src/activity-utils/makeActivityReducer.ts, every reducer derives a
new array when changing steps (e.g. StepPushed uses
[...activity.steps, newRoute]), except StepPopped

StepPopped: (activity: Activity, event: StepPoppedEvent): Activity => {
  activity.steps.pop();
  ...
}

What I have done

• Changed StepPopped to build the popped steps via activity.steps.slice(0, -1), returning a new array instead of mutating the original, and included it explicitly in the returned activity.
• Added core/src/activity-utils/makeActivityReducer.spec.ts, asserting that the original activity's steps reference and length are unchanged after the reducer runs, and that the new activity has the expected popped steps/params.
• Verified the new test fails against the pre-fix implementation (confirms it actually catches the mutation bug) and passes after the fix.

Test Plans

• Added a unit test asserting referential immutability of steps across the StepPopped reducer.
• Ran the full core test suite and tsc.

Test Results

• yarn jest (core): 15 suites / 87 tests passed.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 89042b4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Run configuration

• Source mode: Auto
• Repository: daangn/stackflow
• Branch: main
• File: .coderabbit.yaml
• Commit: fc5320f
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 757e3032-d426-4953-9974-2c0fadda4874

Commits

Reviewing files that changed from the base of the PR and between c9132f3 and 89042b4.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

No actionable comments were generated in the recent review.

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: daangn/stackflow/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: bb61b170-a7fc-478b-b556-1899c801583e

📥 Commits

Reviewing files that changed from the base of the PR and between b3a07c6 and c9132f3.

📒 Files selected for processing: 2

• core/src/activity-utils/makeActivityReducer.spec.ts
• core/src/activity-utils/makeActivityReducer.ts

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Returning to the previous activity step now preserves the original step list while showing the remaining steps and restoring the previous step’s parameters.
• Tests
  • Added coverage to verify that returning to a previous step leaves the original activity unchanged and restores the expected parameters.

The StepPopped reducer now creates a new array for the remaining steps instead of mutating the existing array. A test checks that the original steps remain unchanged and that the returned activity has the expected steps and params.

Changes: Activity step pop

Layer / File(s)	Summary
Non-mutating step pop core/src/activity-utils/makeActivityReducer.ts, core/src/activity-utils/makeActivityReducer.spec.ts	StepPopped uses slice(0, -1) to create the remaining steps and derive params. The test checks that the original steps array remains unchanged and that the returned activity has the expected steps and params.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: orionmiz

Merge Risk: Minimal · previous review c9132

The reducer now preserves the prior activity’s steps while returning the popped result, and the added test checks that behavior. No actionable merge risk remains in the supplied full-change context.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main change: making the core StepPopped reducer immutable.
Description check		The description directly explains the mutation bug, the implementation fix, the added test, and the reported validation results.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

• X
• Mastodon
• Reddit
• LinkedIn