feat(expert): plan mode — propose a plan before acting (#408, #409)#7635
feat(expert): plan mode — propose a plan before acting (#408, #409)#7635andypalmi wants to merge 5 commits into
Conversation
Add an always-visible Plan mode toggle to the composer. When enabled, the Expert proposes a plan instead of making changes, rendered as a plan card with Approve, Edit, Request changes and Reject actions: - Approve exits plan mode and proceeds with the plan. - Edit loads the plan markdown into the composer for direct editing. - Request changes focuses an empty composer to describe a change in words. - Reject abandons the plan. The plan card renders its markdown through RichContent (passing the message and answer uuids it requires), and reuses the composer's pending-input and auto-grow behaviour. Plan mode and the approval signal are shipped to the agent via the expert context.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## feat/407-expert-quick-replies #7635 +/- ##
==============================================================
Coverage 76.47% 76.47%
==============================================================
Files 413 413
Lines 21775 21775
Branches 5736 5736
==============================================================
Hits 16652 16652
Misses 5123 5123
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Plan mode is only meaningful inside the instance/device editor for now, so gate the composer toggle on immersive mode and force the persisted planMode off whenever the user is outside immersive (including on load), preventing a stale value from being sent in non-immersive contexts.
…n-mode # Conflicts: # frontend/src/components/expert/components/ExpertChatInput.vue # frontend/src/components/expert/components/messages/components/AnswerWrapper.vue
…n-mode # Conflicts: # frontend/src/components/expert/components/ExpertChatInput.vue # frontend/src/stores/product-expert.js
There was a problem hiding this comment.
Hey flagged a couple things. I wasn't able to test this since we don't have a staging env for non mainline PRs. Lmk when this is pointed at main and I will test. nvm I am wrong, will test now as well and update.
One general note - there are a lot of code comments here, and Serban got on me for this exact thing too so you're not alone. When they get this verbose they kinda make the code harder to read. My rule of thumb is if you can figure it out from reading the code, don't comment it. Might be worth dropping into a CLAUDE.md.
| // composer react each time, including repeated resets. | ||
| this.composerReset++ | ||
| }, | ||
| async handleQuery ({ query, contextOverrides }) { |
There was a problem hiding this comment.
What do we use contextOverrides for? It's piped through to sendMqttQuery but never used in that fn, while sendHttpQuery merges it. I don't see anywhere passing it to handleQuery either. Is this leftover, or meant to be wired up?
| }, | ||
| persist: { | ||
| pick: ['shouldWakeUpAssistant', 'questionCadence'], | ||
| pick: ['shouldWakeUpAssistant', 'questionCadence', 'planMode'], |
There was a problem hiding this comment.
onPlanApprove calls setPlanMode(false), and planMode is persisted. So approving any plan permanently turns off the user's plan-mode preference. Do we wanna persist this setting?
| // Signal the chat composer to clear its input. Bumping a counter lets the | ||
| // composer react each time, including repeated resets. | ||
| this.composerReset++ | ||
| }, |
There was a problem hiding this comment.
These planChangeRequest++ / composerReset++ counters feel a bit sus. They're basically using store state as an event bus for the composer to watch, which nothing else in the stores does.
We already do this cleaner right here with pendingInput (set → consumed → cleared). Could we model these the same way instead of counters? Would kill a couple of the new watchers too.
|
UI issue here - besides that functionality seems solid.
|
| // Reuses the shared DefaultChip for its bg, border, active state and theming; the only | ||
| // styling here is sizing the toggle switch in the #icon slot, since the switch has no size | ||
| // prop. It is a visual indicator only (pointer-events disabled); the chip handles the click. | ||
| .plan-mode-chip { |
There was a problem hiding this comment.
:deep() into .ff-toggle-switch-button to resize it feels fragile. This'll break quietly if ff-toggle-switch ever changes. Since it has no size option, could we add a small/size variant to the component itself instead of reaching into its guts from the chip?
2 participants
Summary
Frontend for Expert plan mode (FlowFuse/product#408, FlowFuse/product#409): the Expert proposes a plan before making any changes, and acts only once the user approves it.
Stacking
This PR is stacked on #7556 (clarifying questions) — base branch
feat/407-expert-quick-replies. It reuses the composer pending-input / auto-grow infrastructure introduced there, so #7556 should merge first. The diff shown here is plan-mode only.Testing
npm run lint,npm run buildandnpm run test:unit:frontend(570 tests) all pass.Closes: