fix(meetings): guard unsaved-minutes loss and polish editor flow (#7116) - #10445
fix(meetings): guard unsaved-minutes loss and polish editor flow (#7116)#10445NicholasRBowers wants to merge 1 commit into
Conversation
Cancelling a dirty minutes draft now asks before discarding, the revert dialog says the loss is permanent (all 14 locale catalogs), the editor textarea is focused on entry, the save button reads Saving… while the write is in flight, and a saved-but-empty edit no longer shows the 'output will appear here' placeholder. Backend: the delete-output response drops the consumer-less 'reverted' field, and store.delete_meeting derives the edits path through the existing agent_edits_dir helper (same contain + refuse-linked barrier, spelled once).
|
Intent: Close the seven advisory follow-ups from the minutes-editing feature (issue #7116): stop the editor losing unsaved work silently, state permanent loss plainly, polish the edit flow (focus, pending label, empty-edit placeholder), and remove two pieces of backend dead weight (the consumer-less |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Real data-loss harm, fixed at the right seam — a seed-anchored dirty check that survives the outputs poll, not a naive prop comparison. Suggestions
[DESIGN-REVIEWED] 1fe5981 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All claims verified: First-Principles-Verdict: PASS Verify #7116 actually enumerates all seven deferred items — every item's provenance hangs on that one linked issue. What this change shipsInventory (9 items) — 9 justifiedIntent: stop the minutes editor from silently losing or misrepresenting the user's corrections — a FIX (batch of seven review deferrals tracked in #7116).
Items 7 and 8 are subtractions; the rewritten [FIRST-PRINCIPLES-REVIEWED] 1fe5981 |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — 🟡 CONCERNSUX-level review of I have everything I need: the blind read, all three screenshots, the diff, the PR intent, and the sibling wording conventions ( UX-Verdict: CONCERNS The guard reads correctly on first sight, but two look-alike red "Discard" dialogs invite skim-confusion, and two introduced states shipped unshown. Watch
Evidence gaps
[UX-REVIEWED] 1fe5981 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
Problem / Motivation
The minutes editor could throw away the user's work. Clicking Cancel with unsaved edits discarded the text silently, and the revert dialog never said the deletion is permanent. Five smaller gaps shipped with it: the editor opened without focus, the save button froze on "Save" while a write ran, an empty saved edit showed "output will appear here", the delete-output response carried a
revertedfield no client reads, andstore.delete_meetingspelled the edits-path derivation by hand next to a helper that already does it. All seven were deferred from the minutes-editing feature's advisory reviews and tracked in #7116.Why it matters
The editor exists so a user can correct minutes during a live meeting. A silent discard loses exactly that correction, with no way back — and the dialog that does exist understated the same loss. The rest is small friction on every edit, plus two pieces of dead weight in the backend.
What changed (motivation → approach → change)
Cancelling a dirty draft now asks first, with the same dialog the revert path already uses. The dirty check compares the draft against the seed the editor opened with — never the live
outputprop, which the 5-second outputs poll refreshes — so an untouched draft stays clean even while the agent writes more underneath. The revert dialog now says "Permanently delete my edits and show what {name} wrote", in all 14 locale catalogs.🟩 added · 🟥 removed · 🟦 unchanged
Discarding typed text now takes the same explicit yes as reverting a saved edit; a draft you never touched still closes with one click.
The editor textarea is focused when edit mode opens (the clicked Edit button unmounts, so focus used to drop to the page body). The save button reads "Saving…" while this panel's own save is in flight — the label is driven by a panel-local flag, because the
editSavingprop is session-wide and would mislabel an idle sibling panel; that prop still disables the controls as before. A saved-but-empty edit no longer renders the "{name} output will appear here" placeholder: the blank is the user's own version, not agent silence, and the Edited badge already says whose copy is shown.In the backend,
handle_delete_outputdrops therevertedresponse field (its only mention anywhere was the client's response type, now updated) and_drop_editstops returning the flag nothing consumed.store.delete_meetingnow derives the edits path through the existingagent_edits_dir()helper, which applies the samecontain+ refuse-linked barrier at both the edits root and the per-meeting entry — one spelling instead of two.Tests
Frontend (
MeetingsAgentPanel.test.tsx, 43 pass; 6 fail without the component change):outputprop refreshes underneathBackend (
test_meetings_minutes.py, 311 meetings tests pass): the delete-output response is asserted exactly as{"ok", "agent_id"}on both the reverted and nothing-to-revert paths, locking the field removal; the revert-restores-generated-text behavior is unchanged.prove.py: PROVEN (the changed tests fail with the production hunks reverted).Manual verification
Drove the three states in a real browser (scripted Playwright against a Vite dev server rendering
AgentPanel) and captured them below. Full local gates: isort / flake8 / mypy / diff-scoped black / full pytest (reds match pristine main),tsc -b, full vitest (2042 files), i18n gate chain.Screenshots / video
Revert confirm with the permanent-loss wording:
Dirty-cancel confirm (new dialog; the focused editor behind it):
Saving… pending label while a save is in flight:
Related Issues
Closes #7116
Pattern harvest
Rule candidate: review-prompt
Pattern: UI dirty/pending state derived from a live shared prop instead of a captured local seed — a background poll or a sibling's in-flight write then misreports this element's state.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)