feat(setbuilder): make agent chat mutations undoable#494
Conversation
Brainstormed design for bridging agent chat mutations into the existing useSetDocumentHistory undo stack — unified global undo, frontend-only via a shouldRecord predicate on commit(). Gates #491 (Family 3 destructive tools) and #442 Family 4 (imports); both become undoable for free since the snapshot is the whole document. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
TDD plan: (1) shouldRecord predicate on history commit, (2) bridge useAgentChat.send through commit, (3) thread history.commit through the chat surfaces (gated on historyEnabled). Frontend-only, three commits, build green between each. Derived from the approved design spec. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR makes agent chat mutations undoable by extending ChangesAgent Undo Stack Integration
Sequence Diagram(s)sequenceDiagram
participant Page as SetBuilder Page
participant ChatSidebar as ChatSidebar / MobileAgentOverlay
participant useAgentChat as useAgentChat.send()
participant commit as useSetDocumentHistory.commit()
participant API as api.chatWithSetAgent
Page->>ChatSidebar: commit={history.commit} (when historyEnabled)
ChatSidebar->>useAgentChat: useAgentChat({ commit, ... })
useAgentChat->>commit: commit("Agent · rebuild the set", action, didMutate)
commit->>API: action() → AgentChatOut
API-->>commit: result
commit->>commit: shouldRecord = didMutate(result)
alt shouldRecord true
commit->>commit: push undoStack, clear redoStack
else shouldRecord false
commit->>commit: skip undo entry, publish snapshot only
end
commit-->>useAgentChat: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dashboard/app/`(dj)/setbuilder/components/__tests__/ChatSidebar.test.tsx:
- Around line 88-110: Add a new integration test for the MobileAgentOverlay
component that mirrors the existing test for ChatSidebar to validate the
commit-routing behavior. The new test should render MobileAgentOverlay with a
commit function and onMutationApplied callback, simulate the mobile send flow
(similar to how the ChatSidebar test simulates typing and clicking Send), and
assert that commit is called with the correct message while onMutationApplied is
not invoked. This ensures the commit path wiring works consistently across both
the desktop (ChatSidebar) and mobile (MobileAgentOverlay) agent surfaces as per
the agent functionality testing guidelines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fdcff954-3575-41b3-bc98-dc85cce71cba
📒 Files selected for processing (10)
dashboard/app/(dj)/setbuilder/[setId]/page.tsxdashboard/app/(dj)/setbuilder/components/ChatSidebar.tsxdashboard/app/(dj)/setbuilder/components/MobileAgentOverlay.tsxdashboard/app/(dj)/setbuilder/components/__tests__/ChatSidebar.test.tsxdashboard/app/(dj)/setbuilder/components/__tests__/useAgentChat.test.tsxdashboard/app/(dj)/setbuilder/components/__tests__/useSetDocumentHistory.test.tsxdashboard/app/(dj)/setbuilder/components/useAgentChat.tsdashboard/app/(dj)/setbuilder/components/useSetDocumentHistory.tsdocs/superpowers/plans/2026-06-18-agent-undo.mddocs/superpowers/specs/2026-06-18-agent-undo-design.md
Mirror the desktop ChatSidebar commit-routing test for the mobile agent surface: assert overlay sends route through the provided commit (carrying the "Agent · <message>" label) and never fall back to onMutationApplied. The undo-stack wiring was threaded through both surfaces; only the desktop path was tested, leaving the mobile path free to silently regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* docs(setbuilder): design spec for autobuild + fill_to_duration agent tools (#491) Family 3 destructive structural tools. Records the key finding that #493/#494's global undo already closes #491's undo gate, so the original Task 0 (per-tool 'Undo this rebuild' button) is replaced by: rely on global undo + a discoverability hint + a snapshot round-trip identity test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(setbuilder): implementation plan for autobuild + fill_to_duration (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(setbuilder): add commit flag to build_set for agent-turn atomicity (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(setbuilder): autobuild agent tool — wholesale pass-1 rebuild (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(setbuilder): fill_to_duration agent tool — bounded pool fill to target (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(setbuilder): cover _duration_for fallback + fill cap display branch (#491) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * feat(setbuilder): undo-discoverability hint on destructive agent tool cards (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(setbuilder): cover fill_to_duration in the destructive undo-hint test (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(setbuilder): clarify the commit=False flush branch in _persist_slots (#491) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(setbuilder): treat target_duration_sec=0 as valid in fill_to_duration (#491) CodeRabbit: 'if not target' rejected a legitimate 0 target as missing. Use an explicit None check so a 0 target is a no-op (loop sees it as already met) rather than an error. Pinned with a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(setbuilder): log autobuild outcome metrics for observability (#491) CodeRabbit nitpick: mirror fill_to_duration by logging autobuild's result (slot_count + iterations) on this destructive turn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…RLs (#442 Family 4b) (#529) Exposes the existing public-playlist-URL pool import as a WrzDJSet agent tool, completing epic #442 Family 4. The LLM can now pull a pool from a pasted public Spotify/Tidal playlist link, mirroring the REST import_pool_url handler. Thin wrapper over existing infrastructure: parse_public_playlist_url (SSRF-safe, parse-only host allowlist) + pool.candidates_from_public_url + get_or_create_source + import_candidates(commit=False) for turn atomicity. Invalid/unsupported URLs and PoolImportError surface as AgentToolError; Tidal URLs require a connected account. kind="public_url" matches the REST source-dedupe key so REST and agent paths coexist. Additive pool import rides the existing global undo (#493/#494); no frontend change, requests table untouched, affected positions set(). Registered in the usual 4 spots (MUTATION_TOOLS, ToolSpec, pass2_agent handlers, agent_display). Tests exercise the real parser with only the network fetch mocked. Closes #528. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Why
WrzDJSet agent chat mutations bypassed the dashboard's existing undo system, so a DJ could
Ctrl+Za manual drag but not an agent edit. This blocks the destructive Family 3 tools (autobuild/fill_to_duration, #491) and Family 4 imports, which need a recovery path before they can ship.What
Bridges agent chat turns into the existing 50-deep document-history stack (
useSetDocumentHistory) — unified global undo, so a mutating turn becomes an ordinary undo entry reverted by the sameCtrl+Z/ ↶ as a manual edit. Frontend-only, no backend change — it rides thebuild_snapshot/restore_snapshot+GET/PUT /documentsurface (#395) that already exists.Three small changes:
commitgains an optionalshouldRecord(result)predicate (backward-compatible) — record an undo entry only when the turn mutated.useAgentChat.sendwraps the chat call incommitwith adidMutatepredicate; falls back to the old refresh when history is unavailable.history.commitis threaded throughChatSidebar/MobileAgentOverlay(gated onhistoryEnabled).Because the captured snapshot is the whole document (slots + curve + pool), every current and future agent mutation — including the unbuilt Family 3/4 destructive tools — is undoable for free. This satisfies the #491 undo gate.
Design + plan:
docs/superpowers/specs/2026-06-18-agent-undo-design.md,docs/superpowers/plans/2026-06-18-agent-undo.md.Testing
tsc --noEmit+ eslint clean🤖 Co-authored by Claude Opus 4.8. Closes #493. Unblocks #491 + #442 Family 4.
Summary by CodeRabbit