From f72adef95a3b06cfb47737dcba4b93b4db163cb0 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Tue, 18 Aug 2026 00:19:57 -0400 Subject: [PATCH 1/2] feat: relationship-aware delete -- toast names the blast radius (goal 0103) TombstoneResult extends additively with LinksRemoved/ChildrenPromoted counts (computed against pre-delete state), so the undo toast can name what a delete touches -- links hidden, children moved up -- instead of letting them vanish silently into the undo window. Undo already restored both (goal 0093); this only adds visibility. No new confirmation dialog: with single-level undo already in place, the count-in-toast is the informing layer, matching the canvas-tool convergence (undo present -> no pre-delete confirm on object deletes). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd --- .../mill/internal/services/atlassvc/models.ts | 10 +- .../services/settingssvc/settingsservice.ts | 12 +- .../e2e/atlas-delete-relationships.spec.ts | 41 +++++ frontend/src/atlas/AtlasUndoToast.tsx | 18 ++- frontend/src/atlas/AtlasView.tsx | 7 +- .../src/atlas/useAtlasContainmentMenus.tsx | 2 + frontend/src/atlas/useAtlasUndoToast.ts | 10 +- frontend/src/locales/en/atlas.json | 3 + .../atlassvc/atlasservice_tombstone.go | 59 +++++++- .../atlassvc/atlasservice_tombstone_test.go | 140 ++++++++++++++++++ 10 files changed, 289 insertions(+), 13 deletions(-) create mode 100644 frontend/e2e/atlas-delete-relationships.spec.ts diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/atlassvc/models.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/atlassvc/models.ts index 3b795e1f..371ac994 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/atlassvc/models.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/atlassvc/models.ts @@ -177,9 +177,17 @@ export interface ImportFolderSuggestionsRequest { * TombstoneResult names exactly which ids one DeleteCard/DeleteNote * call soft-deleted -- DeleteCard populates only CardIDs, DeleteNote * only NoteIDs, so the frontend's undo toast can pass this straight - * back to UndoDelete without re-deriving what it touched. + * back to UndoDelete without re-deriving what it touched. LinksRemoved + * and ChildrenPromoted are the delete's blast radius, counted against + * the state immediately BEFORE this call's own tombstone lands: links + * that were visible and now touch a tombstoned endpoint, and direct + * live children (cards + notes) whose effective parent is about to + * shift past this card. Both stay zero for DeleteNote -- a note can + * carry neither a link endpoint nor a child. */ export interface TombstoneResult { "CardIDs": string[] | null; "NoteIDs": string[] | null; + "LinksRemoved": number; + "ChildrenPromoted": number; } diff --git a/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts b/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts index 7a5d48df..32171cbe 100644 --- a/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts +++ b/frontend/bindings/github.com/alicoding/mill/internal/services/settingssvc/settingsservice.ts @@ -123,6 +123,12 @@ export function DismissPanel(): $CancellablePromise { * Channel-gated server-side, not just by the UI: a source-channel * build must never binary-swap itself, since "source" means the * running copy IS the update mechanism (pull + rebuild). + * + * goal 0100: a beta install swaps its running app over real, primary + * data, so the swap never proceeds without a fresh restore point. The + * backupRunner seam is called first and its error aborts the update + * outright -- no download, no swap -- same fail-closed posture as the + * digest check below it. */ export function DownloadAndInstallUpdate(): $CancellablePromise { return $Call.ByID(174385789); @@ -533,9 +539,9 @@ export function UnassignSummonHotkey(): $CancellablePromise { } /** - * UpdateChannel reports the resolved distribution channel ("source" or - * "release") -- the Settings > Updates surface's gate between "Update - * now" and the pull-and-rebuild instructions, and + * UpdateChannel reports the resolved distribution channel ("source", + * "release", or "beta") -- the Settings > Updates surface's gate + * between "Update now" and the pull-and-rebuild instructions, and * DownloadAndInstallUpdate's own server-side enforcement of the same * gate. */ diff --git a/frontend/e2e/atlas-delete-relationships.spec.ts b/frontend/e2e/atlas-delete-relationships.spec.ts new file mode 100644 index 00000000..1b3375ff --- /dev/null +++ b/frontend/e2e/atlas-delete-relationships.spec.ts @@ -0,0 +1,41 @@ +import { test, expect } from './fixtures/server' +import { deleteViaPageMenu } from './fixtures/atlasPage' +import { openViaFlip } from './fixtures/atlasBoard' + +// Split out of atlas.spec.ts (architecture.md's 500-line convention, +// the same split atlas-share.spec.ts/atlas-projections.spec.ts already +// established) once the delete-blast-radius toast case (goal 0103) +// pushed that file over the limit. + +// Precise per-card matching, same reasoning as atlas.spec.ts's own +// local copy: a card's own BACK face can legitimately contain another +// card's title in its " -> " link row, so aria-label +// carries the exact title instead of a substring match. +function noteCard(page: import('@playwright/test').Page, title: string) { + return page.locator(`[data-testid="atlas-note-card"][aria-label="Flip ${title}"]`) +} + +test('deleting a linked card names the blast radius in the undo toast, and undo restores the link edge', async ({ page }) => { + await page.goto('/') + await page.getByRole('link', { name: 'Atlas' }).click() + await expect(page.getByTestId('atlas-board')).toBeVisible() + await expect(page.locator('.react-flow__edge')).not.toHaveCount(0) + + // "Getting started" carries exactly one seeded link (to "Ada + // Lovelace") and no children -- the leaf-with-links case. + const gettingStarted = noteCard(page, 'Getting started') + await openViaFlip(gettingStarted) + const overlay = page.locator('[data-component="atlas-card-overlay"]') + await deleteViaPageMenu(page, overlay) + await expect(gettingStarted).toHaveCount(0) + + const undoToast = page.getByTestId('atlas-undo-toast') + await expect(undoToast).toBeVisible() + await expect(undoToast).toContainText('Deleted 1') + await expect(undoToast).toContainText('1 link hidden') + + await undoToast.getByTestId('atlas-undo-toast-button').click() + await expect(undoToast).toHaveCount(0) + await expect(gettingStarted).toBeVisible() + await expect(page.locator('.react-flow__edge')).not.toHaveCount(0) +}) diff --git a/frontend/src/atlas/AtlasUndoToast.tsx b/frontend/src/atlas/AtlasUndoToast.tsx index aecb97d8..800ca9cb 100644 --- a/frontend/src/atlas/AtlasUndoToast.tsx +++ b/frontend/src/atlas/AtlasUndoToast.tsx @@ -7,12 +7,24 @@ import styles from './AtlasUndoToast.module.css' // selection/creation tray -- AtlasView renders exactly one of these at // a time, cleared by its own 10s timer, a click, ⌘Z, or a later delete // finalizing it (useAtlasUndoToast owns that lifecycle; this component -// is pure presentation). -export function AtlasUndoToast({ count, onUndo }: { count: number; onUndo: () => void }) { +// is pure presentation). linksRemoved/childrenPromoted (goal 0103) name +// the delete's blast radius: a segment appends only when its count is +// non-zero, since undo already restores both in full. +export function AtlasUndoToast({ + count, linksRemoved, childrenPromoted, onUndo, +}: { + count: number + linksRemoved: number + childrenPromoted: number + onUndo: () => void +}) { const { t } = useTranslation('atlas') + const segments = [t('board.deletedToast', { count })] + if (linksRemoved > 0) segments.push(t('board.linksHiddenToast', { count: linksRemoved })) + if (childrenPromoted > 0) segments.push(t('board.childrenMovedToast', { count: childrenPromoted })) return (
- {t('board.deletedToast', { count })} + {segments.join(' ')} diff --git a/frontend/src/atlas/AtlasView.tsx b/frontend/src/atlas/AtlasView.tsx index f6ae560a..723fc1b3 100644 --- a/frontend/src/atlas/AtlasView.tsx +++ b/frontend/src/atlas/AtlasView.tsx @@ -450,7 +450,12 @@ export function AtlasView({ initialCardID }: { initialCardID?: string }) { }} /> {undoToast.pending && ( - + )}
diff --git a/frontend/src/atlas/useAtlasContainmentMenus.tsx b/frontend/src/atlas/useAtlasContainmentMenus.tsx index 84ca59f7..ec2de098 100644 --- a/frontend/src/atlas/useAtlasContainmentMenus.tsx +++ b/frontend/src/atlas/useAtlasContainmentMenus.tsx @@ -77,6 +77,8 @@ export function useAtlasContainmentMenus({ onDeleted({ CardIDs: results.flatMap((r) => r.CardIDs ?? []), NoteIDs: results.flatMap((r) => r.NoteIDs ?? []), + LinksRemoved: results.reduce((sum, r) => sum + (r.LinksRemoved ?? 0), 0), + ChildrenPromoted: results.reduce((sum, r) => sum + (r.ChildrenPromoted ?? 0), 0), }) void refreshAtlas() }) diff --git a/frontend/src/atlas/useAtlasUndoToast.ts b/frontend/src/atlas/useAtlasUndoToast.ts index 662c9644..92088051 100644 --- a/frontend/src/atlas/useAtlasUndoToast.ts +++ b/frontend/src/atlas/useAtlasUndoToast.ts @@ -10,6 +10,8 @@ export interface PendingUndo { cardIDs: string[] noteIDs: string[] count: number + linksRemoved: number + childrenPromoted: number } // Owns the quick-delete undo toast's whole lifecycle (goal 0093): one @@ -37,7 +39,13 @@ export function useAtlasUndoToast() { clearTimer() const cardIDs = result.CardIDs ?? [] const noteIDs = result.NoteIDs ?? [] - setPending({ cardIDs, noteIDs, count: cardIDs.length + noteIDs.length }) + setPending({ + cardIDs, + noteIDs, + count: cardIDs.length + noteIDs.length, + linksRemoved: result.LinksRemoved ?? 0, + childrenPromoted: result.ChildrenPromoted ?? 0, + }) timerRef.current = setTimeout(() => setPending(null), TOAST_DURATION_MS) } diff --git a/frontend/src/locales/en/atlas.json b/frontend/src/locales/en/atlas.json index 330d8cbd..386f0877 100644 --- a/frontend/src/locales/en/atlas.json +++ b/frontend/src/locales/en/atlas.json @@ -40,6 +40,9 @@ "selectionClearHint": "clear", "dropDuplicateNotice": "Already on the map as “{{title}}” — added again.", "deletedToast": "Deleted {{count}}", + "linksHiddenToast_one": "· {{count}} link hidden", + "linksHiddenToast_other": "· {{count}} links hidden", + "childrenMovedToast": "· {{count}} moved up", "undo": "Undo" }, "jump": { diff --git a/internal/services/atlassvc/atlasservice_tombstone.go b/internal/services/atlassvc/atlasservice_tombstone.go index 6f7cc74a..53fd7bc6 100644 --- a/internal/services/atlassvc/atlasservice_tombstone.go +++ b/internal/services/atlassvc/atlasservice_tombstone.go @@ -20,10 +20,59 @@ const tombstoneGraceWindow = 48 * time.Hour // TombstoneResult names exactly which ids one DeleteCard/DeleteNote // call soft-deleted -- DeleteCard populates only CardIDs, DeleteNote // only NoteIDs, so the frontend's undo toast can pass this straight -// back to UndoDelete without re-deriving what it touched. +// back to UndoDelete without re-deriving what it touched. LinksRemoved +// and ChildrenPromoted are the delete's blast radius, counted against +// the state immediately BEFORE this call's own tombstone lands: links +// that were visible and now touch a tombstoned endpoint, and direct +// live children (cards + notes) whose effective parent is about to +// shift past this card. Both stay zero for DeleteNote -- a note can +// carry neither a link endpoint nor a child. type TombstoneResult struct { - CardIDs []string - NoteIDs []string + CardIDs []string + NoteIDs []string + LinksRemoved int + ChildrenPromoted int +} + +// liveLinkTouchCountLocked counts currently-live links (both endpoints +// live) that touch cardID -- the number of links about to become +// hidden if cardID is tombstoned next. Caller must hold a.mu. +func (a *AtlasService) liveLinkTouchCountLocked(cardID string) int { + tombstoned := make(map[string]bool) + for _, c := range a.cards { + if !c.DeletedAt.IsZero() { + tombstoned[c.ID] = true + } + } + n := 0 + for _, l := range a.links { + if tombstoned[l.FromCardID] || tombstoned[l.ToCardID] { + continue + } + if l.FromCardID == cardID || l.ToCardID == cardID { + n++ + } + } + return n +} + +// directLiveChildCountLocked counts live cards and notes whose stored +// ParentID is exactly cardID -- the direct children about to be +// virtually promoted to cardID's own effective parent. Caller must +// hold a.mu. +func (a *AtlasService) directLiveChildCountLocked(cardID string) int { + n := 0 + for _, c := range a.cards { + if c.ParentID == cardID && c.DeletedAt.IsZero() { + n++ + } + } + for _, nt := range a.notes { + if nt.ParentID == cardID && nt.DeletedAt.IsZero() { + n++ + } + } + return n } // liveCardsLocked returns every non-tombstoned card, each carrying its @@ -106,6 +155,8 @@ func (a *AtlasService) DeleteCard(id string) (TombstoneResult, error) { } previous := a.cards[idx] wasBuiltIn := previous.BuiltIn + linksRemoved := a.liveLinkTouchCountLocked(id) + childrenPromoted := a.directLiveChildCountLocked(id) now := time.Now() a.cards[idx].DeletedAt = now a.cards[idx].UpdatedAt = now @@ -139,7 +190,7 @@ func (a *AtlasService) DeleteCard(id string) (TombstoneResult, error) { return TombstoneResult{}, fmt.Errorf("save card deletion: %w", perr) } dataevent.Emit("atlas", id) - return TombstoneResult{CardIDs: []string{id}}, nil + return TombstoneResult{CardIDs: []string{id}, LinksRemoved: linksRemoved, ChildrenPromoted: childrenPromoted}, nil } // DeleteNote soft-deletes a note -- same tombstone contract as diff --git a/internal/services/atlassvc/atlasservice_tombstone_test.go b/internal/services/atlassvc/atlasservice_tombstone_test.go index 7cc273ab..c01f45dc 100644 --- a/internal/services/atlassvc/atlasservice_tombstone_test.go +++ b/internal/services/atlassvc/atlasservice_tombstone_test.go @@ -77,6 +77,146 @@ func TestDeleteCard_SoftDeleteRoundTripsWithUndo(t *testing.T) { } } +// TestDeleteCard_TombstoneResultCounts_LeafWithLinks pins goal 0103's +// blast-radius counts for the simplest case: a leaf card with one live +// link and no children. +func TestDeleteCard_TombstoneResultCounts_LeafWithLinks(t *testing.T) { + a := newTestAtlasService(t) + k, err := a.CreateKind("Widget", "", "", nil) + if err != nil { + t.Fatalf("CreateKind: %v", err) + } + c1, err := a.CreateCard(k.ID, "A", "", nil, "", nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard: %v", err) + } + c2, err := a.CreateCard(k.ID, "B", "", nil, "", nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard: %v", err) + } + lk, err := a.CreateLinkKind("connects to", "") + if err != nil { + t.Fatalf("CreateLinkKind: %v", err) + } + if _, err := a.CreateLink(c1.ID, c2.ID, lk.ID, ""); err != nil { + t.Fatalf("CreateLink: %v", err) + } + + result, err := a.DeleteCard(c1.ID) + if err != nil { + t.Fatalf("DeleteCard: %v", err) + } + if result.LinksRemoved != 1 { + t.Errorf("LinksRemoved = %d, want 1", result.LinksRemoved) + } + if result.ChildrenPromoted != 0 { + t.Errorf("ChildrenPromoted = %d, want 0", result.ChildrenPromoted) + } +} + +// TestDeleteCard_TombstoneResultCounts_ContainerWithChildrenAndLinks +// pins the container case: deleting a card with two direct live +// children (one card, one note) and one live link reports both counts +// together. +func TestDeleteCard_TombstoneResultCounts_ContainerWithChildrenAndLinks(t *testing.T) { + a := newTestAtlasService(t) + k, err := a.CreateKind("Widget", "", "", nil) + if err != nil { + t.Fatalf("CreateKind: %v", err) + } + container, err := a.CreateCard(k.ID, "Container", "", nil, "", nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard(container): %v", err) + } + child, err := a.CreateCard(k.ID, "Child", "", nil, container.ID, nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard(child): %v", err) + } + if _, err := a.CreateNote("filed under container", atlas.Position{}, container.ID); err != nil { + t.Fatalf("CreateNote: %v", err) + } + other, err := a.CreateCard(k.ID, "Other", "", nil, "", nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard(other): %v", err) + } + lk, err := a.CreateLinkKind("connects to", "") + if err != nil { + t.Fatalf("CreateLinkKind: %v", err) + } + if _, err := a.CreateLink(container.ID, other.ID, lk.ID, ""); err != nil { + t.Fatalf("CreateLink: %v", err) + } + + result, err := a.DeleteCard(container.ID) + if err != nil { + t.Fatalf("DeleteCard: %v", err) + } + if result.LinksRemoved != 1 { + t.Errorf("LinksRemoved = %d, want 1", result.LinksRemoved) + } + if result.ChildrenPromoted != 2 { + t.Errorf("ChildrenPromoted = %d, want 2 (one card + one note)", result.ChildrenPromoted) + } + + gotChild, ok := findCardTestByID(a.Cards(), child.ID) + if !ok || gotChild.ParentID != "" { + t.Errorf("child not virtually promoted to top level, got %+v ok=%v", gotChild, ok) + } +} + +// TestDeleteCard_TombstoneResultCounts_MixedSelection pins the +// multi-delete case the frontend's own selection-delete flow drives: +// two independent DeleteCard calls in one batch (a leaf-with-a-link +// and a container-with-children), each reporting its own counts so +// the caller can sum them across the whole selection. +func TestDeleteCard_TombstoneResultCounts_MixedSelection(t *testing.T) { + a := newTestAtlasService(t) + k, err := a.CreateKind("Widget", "", "", nil) + if err != nil { + t.Fatalf("CreateKind: %v", err) + } + leaf, err := a.CreateCard(k.ID, "Leaf", "", nil, "", nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard(leaf): %v", err) + } + leafFriend, err := a.CreateCard(k.ID, "LeafFriend", "", nil, "", nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard(leafFriend): %v", err) + } + lk, err := a.CreateLinkKind("connects to", "") + if err != nil { + t.Fatalf("CreateLinkKind: %v", err) + } + if _, err := a.CreateLink(leaf.ID, leafFriend.ID, lk.ID, ""); err != nil { + t.Fatalf("CreateLink: %v", err) + } + container, err := a.CreateCard(k.ID, "Container", "", nil, "", nil, "", "", "", "") + if err != nil { + t.Fatalf("CreateCard(container): %v", err) + } + if _, err := a.CreateCard(k.ID, "ContainerChild", "", nil, container.ID, nil, "", "", "", ""); err != nil { + t.Fatalf("CreateCard(containerChild): %v", err) + } + + leafResult, err := a.DeleteCard(leaf.ID) + if err != nil { + t.Fatalf("DeleteCard(leaf): %v", err) + } + containerResult, err := a.DeleteCard(container.ID) + if err != nil { + t.Fatalf("DeleteCard(container): %v", err) + } + + totalLinksRemoved := leafResult.LinksRemoved + containerResult.LinksRemoved + totalChildrenPromoted := leafResult.ChildrenPromoted + containerResult.ChildrenPromoted + if totalLinksRemoved != 1 { + t.Errorf("summed LinksRemoved = %d, want 1", totalLinksRemoved) + } + if totalChildrenPromoted != 1 { + t.Errorf("summed ChildrenPromoted = %d, want 1", totalChildrenPromoted) + } +} + // TestDeleteNote_SoftDeleteRoundTripsWithUndo is the same contract for // notes. func TestDeleteNote_SoftDeleteRoundTripsWithUndo(t *testing.T) { From 5ff095d3c8f0523c6b82608bcb4077a8f7f30309 Mon Sep 17 00:00:00 2001 From: Ali Al Dallal Date: Tue, 18 Aug 2026 00:37:12 -0400 Subject: [PATCH 2/2] chore: AtlasView back under the line cap -- comment tightened Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01FW5GkkAG8du7tNdYLk2zSd --- frontend/src/atlas/AtlasView.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frontend/src/atlas/AtlasView.tsx b/frontend/src/atlas/AtlasView.tsx index 723fc1b3..a3a4cdd5 100644 --- a/frontend/src/atlas/AtlasView.tsx +++ b/frontend/src/atlas/AtlasView.tsx @@ -79,9 +79,8 @@ export function AtlasView({ initialCardID }: { initialCardID?: string }) { // against re-applying it on every later data refresh, which would // otherwise re-open the overlay even after the user closed it). const consumedInitialCardID = useRef(false) - // Mirrors the ref as state for the landing gate below -- a ref must - // not be read during render, and the gate needs re-render when the - // deep link resolves. + // State mirror of the ref: the landing gate renders off it, and a + // ref must not be read during render. const [deepLinkConsumed, setDeepLinkConsumed] = useState(false) useEffect(() => {