fix(replay): dedupe inherited vs owned steps in get_full_timeline_steps

[risjai]

Summary

After v0.14.6's promote-and-mutate landed, the dashboard's step picker showed two rows at the same step_number for forks where an inherited step had been promoted — and the inherited row visually masked the user's edit.

Caught live in dev1 with session ray-agent-30053072 on 2026-04-29:

• User edited inherited step Add MCP server for AI assistant integration #3's request_body (temperature: 0 -> 0.7)
• API correctly stored a new owned step on the fork (different step_id, edited content)
• Dashboard kept showing temperature: 0 because the inherited row was still in the union view returned by engine.get_full_timeline_steps

Fix

get_full_timeline_steps now de-duplicates by step_number, with owned overriding inherited. HashMap-based: insert parents first, then own steps; HashMap::insert returning the previous value gives "owned wins" for free.

let mut by_step_number: HashMap<u32, Step> = parent_steps.into_iter()
    .filter(|s| s.step_number <= fork_at)
    .map(|s| (s.step_number, s))
    .collect();
for s in own_steps {
    by_step_number.insert(s.step_number, s);
}

After the fix, the fork's view returns one row at #N — the owned one if it exists, else the inherited one.

Pre-existing test adjustments

Two tests needed updates because they exercised a scenario the dashboard would never produce after dedup:

• test_patch_promote_idempotent_re_edit — the second edit now PATCHes the resolved owned id directly (matches the dashboard flow: refetch the list, click pencil on the now-visible owned row). Previously the test PATCHed the inherited id twice with target_timeline_id, which after dedup correctly fails the visibility check (the inherited row is no longer visible by id once the owned twin exists).
• test_cascade_count_target_aware — no longer seeds owned steps with step_numbers that collide with inherited ones. The comment spells out why this scenario isn't reachable from the UI after dedup.

New tests

• crates/rewind-replay/src/lib.rs — get_full_timeline_steps_dedupes_owned_over_inherited pins the unit invariant.
• crates/rewind-web/tests/recording_api_tests.rs — test_steps_endpoint_dedupes_owned_over_inherited_after_promote pins the API surface (mirrors the dev1 screenshot-bug repro).

Test plan

• cargo test -p rewind-replay --lib → 14/14 pass (was 13; +1 dedup test)
• cargo test -p rewind-web --test recording_api_tests → 68/68 pass (was 68 minus 2 broken + 1 new + 2 adjusted)
• cargo build -p rewind-web → clean
• Track 1 version bump 0.14.6 → 0.14.7 per CLAUDE.md (workspace + 2x CLI_VERSION); rewind-mcp pyproject 0.13.5 → 0.13.6

Out of scope (existing follow-ups)

• Recursive get_full_timeline_steps (only walks one parent level → grandchild forks lose grandparent steps). This dedup applies once-per-level; the recursive walk would extend it.
• WS forwarding for StoreEvent::StepUpdated so other tabs refresh on edit.

Made with Cursor

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
rewind	Ready	Preview, Comment	Apr 29, 2026 10:06am

[risjai]

Review: fix(replay): dedupe inherited vs owned steps in get_full_timeline_steps

Clean fix for the exact bug reported in ray-agent-30053072. The root cause is correctly identified: after promote-and-mutate, the union view returned both the inherited and owned rows at the same step_number, so the dashboard showed stale data.

Findings

#	Severity	Finding
1	Info	Test adjustment rationale is sound
2	Info	Dedup correctly cascades to visibility checks
3	Suggestion	HashMap could use with_capacity

I1 — Test adjustments are correct and well-documented

Both adjusted tests have inline comments explaining why the old scenario is unreachable after dedup:

• test_patch_promote_idempotent_re_edit — second edit now targets the resolved owned id directly (matches dashboard flow: refetch → click pencil on owned row)
• test_cascade_count_target_aware — removes owned-step seeding at colliding step_numbers

The before/after logic is sound: once owned wins over inherited at the same step_number, the old "PATCH inherited id twice" test was testing a flow the dashboard can't produce.

I2 — Dedup cascades correctly to both visibility checks

The patch_step handler's visibility check (s.id == step.id) and the cascade_count handler's visibility check both call get_full_timeline_steps. After dedup:

• First promote of an inherited step: inherited id IS visible (no owned twin yet) → 200
• Second edit after promote: inherited id is hidden (owned twin exists) → would 400 if you pass the inherited id → correct, because the dashboard now shows the owned step's id
• The owned step's id is always visible → in-place PATCH without target_timeline_id works

This is the right behavior: the dedup makes the visibility check stricter in a way that matches the UI's display.

S3 — HashMap capacity hint

Minor optimization: since we know the upper bound is fork_at + own_steps.len(), HashMap::with_capacity would avoid rehash for large sessions:

let mut by_step_number: HashMap<u32, Step> = HashMap::with_capacity(fork_at as usize + own_steps.len());

Not blocking.

What I verified

• The HashMap approach is correct: insert parents first → insert own → HashMap::insert overwrites parent at same key → owned wins
• Sort after dedup preserves step_number ordering
• The filter(|s| s.step_number <= fork_at) still correctly scopes inherited steps to the fork boundary
• Own steps are inserted WITHOUT the fork_at filter, so owned steps at any step_number (including below fork_at) correctly override inherited ones — this is the promote case
• New unit test and integration test both pin the invariant from the right angle (unit tests dedup directly, integration test goes through the API endpoint)

Verdict

Ship it. This is the missing piece from the ray-agent-30053072 repro — the edit was landing correctly on the server but the view was masking it with the stale inherited row. Version bump is correct (0.14.6 released → 0.14.7).

[risjai]

Folded the only actionable bit from your review (S3, HashMap with_capacity) in [b8997ed-ish commit\u2014see latest commit on the branch`]. I1 and I2 were info-only confirmations.

CI was already green except for build (macos-latest) which was still running when I left it; the new commit will re-trigger. Will merge once green and then drive the dev1 swap + Run-replay end-to-end against ray-agent-30053072 to close the original bug repro.