Set a verdict beside the panes, not in a dialog over them

Setting one was an `AlertDialog`, and the step that lists the verdicts a new
one contradicts is what that shape got wrong. Two things a reader wants there
are behind the scrim: why two verdicts can disagree at all, which every other
label in this window answers with a `?` that opens the reference in a tab —
and a tab opened behind a modal is a tab you cannot reach, so the only way to
explain it was to draw the paragraph above the list, all 265 characters of it,
every time, for somebody who read it once. And the objects themselves. Each
verdict being overruled is an object, and the reason somebody typed for it is
the case for the other reading, so weighing it against yours is sometimes going
and looking — which a dialog can only offer by being dismissed, and dismissing
it throws away the half typed reason it was holding.

So `SettingVerdict` is state `HeapDumpDive` keeps per tab id, and
`LeakStatusSetter` draws it where a note goes: under the tab's title, above the
panes whose chain and sizes are the evidence the verdict is drawn from. The
paragraph is now the `CONFLICTING_VERDICTS` page, each conflicting verdict is a
link to its object, and the tab you came from is still half set when you come
back to it. Tab ids are never reused, so an entry can only be about the tab it
was made for, and closing that tab drops it.

The three surviving dialogs are all about getting a heap dump rather than
reading one — `Take heap dump…`, `Bitmaps from the live process`, and the one a
`shark://` link asks — so there is nothing behind them worth reaching.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: pyricau

docs/shark-dive-changelog.md

@@ -140,6 +140,14 @@ uses, without the one for a newly recognized library leak:
 * 🔀 The line above the map saying no object is held only through a `java.lang.ref.Reference` now says
   `Nothing here is held only weakly.`, in the words of the `Soft`, `Weak`, `Phantom` and `Finalizer` rows it
   sits under. Which class those four have in common is the page behind the `?` beside it.
+* 🔀 **Setting a verdict happens beside the panes, not in a dialog over them.** It is a panel of the tab it
+  was started from now, under the tab's title where a note goes, so everything the window can do still works
+  while one is half set. Which is what the step listing the verdicts a new one contradicts needed: each of
+  those is an object, and the reason somebody typed for it is the case for the other reading — so each is now
+  a link, and going to look at it opens a tab and leaves your verdict where it was. Why two verdicts can
+  disagree at all moved out of a paragraph drawn above that list every time and behind the `?`, as
+  [Verdicts that disagree](shark-dive-reference.md). A tab per window can have one going at once, and closing
+  the tab is what abandons it.
 * 🔀 **An empty list says what fills it, and stops there.** `Nothing starred` used to name the panel the star
   is in, the empty **Agent logs** screen explained what an agent is and how to point one at the window, and
   the window with its last tab closed reassured you that the heap dump was still read. Each of those is now

docs/shark-dive-reference.md

@@ -19,6 +19,8 @@ sentence below.
 
 --8<-- "docs/shark-dive-reference/stuck-shading.md"
 
+--8<-- "docs/shark-dive-reference/conflicting-verdicts.md"
+
 --8<-- "docs/shark-dive-reference/weaker-references.md"
 
 --8<-- "docs/shark-dive-reference/other-ways.md"

docs/shark-dive-reference/conflicting-verdicts.md

@@ -0,0 +1,21 @@
+## Verdicts that disagree
+
+Everything a stuck object holds is stuck too, so a verdict set by hand can contradict one set earlier.
+
+It runs both ways. Anything a stuck object dominates is only still in memory because that object is, so it
+is stuck as well; and anything holding an object that belongs in memory belongs there too, since something
+has to be keeping it. `Stuck` above and `Expected` below is a pair of verdicts that cannot both be right,
+so before writing anything Shark Dive walks the references and works out which of the verdicts already set
+by hand the new one runs into.
+
+**Every one of them is listed rather than counted**, with the reason it was given, because that reason is
+the case for the other reading. Whoever is about to overrule it is the only person who can weigh the two,
+and they can only do that if they can read what they are overruling. Each is a link to the object it is
+about, and going to look at it opens a tab of its own: the verdict you were setting stays where it was,
+half set, in the tab you left.
+
+Keeping this one flips the verdicts that disagreed to the opposite verdict, with what they said kept as
+part of the new reason — flipped rather than deleted, so the sentence somebody wrote is still in the file
+after somebody else has disagreed with it.
+
+Undoing writes nothing at all: nothing reaches the disk until the last question is answered.

shark/shark-dive/AGENTS.md

@@ -98,6 +98,15 @@ Each of the following is a change already made to this app, not a preference:
   see the bugs everybody else is hitting, and the hint that was in the way on day one is the one they go
   looking for on day thirty and can't find. So don't propose progressive onboarding here — the `?` is
   what that job was given to.
+- **A modal is a claim that nothing else will be needed, so a step of reading is never one.** The `?` is
+  the rule above, and it opens a tab — behind a scrim, that tab is unreachable, so a dialog is a surface on
+  which the only way to explain something is to print the paragraph on it. Setting a verdict was an
+  `AlertDialog` for exactly one release and the conflicts step is what broke it: each verdict being
+  overruled is an object worth going to look at, and looking meant dismissing the dialog and losing the
+  half typed reason. It is now `LeakStatusSetter`, a panel of the tab it was started from, with its state
+  kept per tab id by `HeapDumpDive`. The three surviving dialogs — `Take heap dump…`, `Bitmaps from the
+  live process`, and the one a `shark://` link asks — are all about *getting* a heap dump rather than
+  reading one, which is why nothing behind them is worth reaching.
 - **Prose belongs in the KDoc, `notes/decisions.md`, or a reference page.** Those are the three places
   something long enough to be worth writing goes. `notes/agent-surface.md` measures the same idea on the
   agent surface — ~80 tokens of name and description at rest, the ~2 k-token body only for a session that

shark/shark-dive/notes/decisions.md

@@ -1051,8 +1051,8 @@ knows what the inspectors can't, and weighing the two would mean a status that c
 inspector already picked. So `setByHandStatus` takes the reason someone typed and keeps the inspectors as the
 record of what was overruled, exactly the way a conflict between two inspectors is recorded.
 
-**A status without a reason is not a status.** `LeakStatusOverride` throws on a blank one and the dialog's
-button is disabled until there is one. A status set by hand overrules the heap dump, so without the why it is
+**A status without a reason is not a status.** `LeakStatusOverride` throws on a blank one and the button
+that sets it is disabled until there is one. A status set by hand overrules the heap dump, so without the why it is
 an assertion the next reader — a colleague, an agent, the same person in a month — has no way to check, and
 one of those makes every other status in the dump worth less. `SET_BY_HAND` marks the reason wherever it is
 read, so a green object somebody decided about is never mistaken for one an inspector recognized.
@@ -1089,7 +1089,7 @@ The propagation rules are what make it possible: a leaking object above forces e
 leaking, and an object still needed below forces everything holding it to be needed. So two hand-set statuses
 disagree when one of the objects is above the other, which is `HeapDominatorTreemap.reaches` asked **both ways
 round** — one walk up `ReferrerIndex` per status already set, a question somebody asked rather than one the
-pointer asks. `leakStatusConflictsWith` answers it before anything is written, and the dialog then lists every
+pointer asks. `leakStatusConflictsWith` answers it before anything is written, and the panel then lists every
 one of them by name, with the reason it was given, because whoever is about to overrule it is the only person
 who can weigh the two.
 
@@ -1113,7 +1113,25 @@ who can weigh the two.
 - **Nothing is written until the choice is made**, which is what makes "Undo" free, and the write is one
   `LeakStatusFile.write` of the lot rather than one per status: a save that stopped half way through would
   leave a heap dump whose statuses contradict each other, which is the one state this step exists to
-  prevent. It runs `NonCancellable` because the dialog closes as soon as it has.
+  prevent. It runs `NonCancellable` because the panel closes as soon as it has.
+
+**Setting a verdict is a panel of the tab, not a dialog over the window.** It was an `AlertDialog` first,
+and that shape is what made the conflicts step read badly: a modal is a promise that nothing outside it will
+be needed, and this is the one step in the app where that is plainly false. Why two verdicts can disagree is
+a paragraph — it was 265 characters of prose drawn above the list, every time, for a reader who had read it
+once — and every `?` in the window answers that shape by opening the reference in a tab. A tab opened from
+behind a scrim is a tab you cannot reach. Worse, each verdict being overruled is an *object*, and the reason
+somebody typed for it is the case for the other reading: weighing it against yours is sometimes going and
+looking at that object, which a modal can only offer by being dismissed, and dismissing it throws away the
+half typed reason it was holding.
+
+So `SettingVerdict` is state `HeapDumpDive` keeps per tab id, and `LeakStatusSetter` draws it where
+`NoteSection` draws a note — under the tab's title, above the panes whose chain and sizes are the evidence
+for the verdict. What falls out: the `?` on `CONFLICTING_VERDICTS` opens the reference in a tab, each
+conflicting verdict opens the object it is about in a tab, and the tab you came from is still half set when
+you come back to it. Tab ids are never reused (see `Tab.id`), so an entry can only ever be about the tab it
+was made for, and closing a tab drops it. The other three dialogs stay dialogs: they are about acquiring a
+heap dump rather than reading one, so there is nothing behind them to go and look at.
 
 **One tab separated file per heap dump, in `~/.shark-dive/leak-statuses`.** Named after the dump the way
 its notes are, and beside them rather than next to the dump, for the same reason: dumps come from device

shark/shark-dive/shark-dive-app/src/main/java/shark/dive/app/HeapDumpDive.kt

@@ -25,6 +25,7 @@ import androidx.compose.material3.VerticalDivider
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.LaunchedEffect
 import androidx.compose.runtime.getValue
+import androidx.compose.runtime.mutableStateMapOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.remember
 import androidx.compose.runtime.rememberCoroutineScope
@@ -234,8 +235,17 @@ internal fun HeapDumpDive(
   val starring = rememberCoroutineScope()
   /** Which objects of this heap dump have a status someone set, which is what every chain is read with. */
   val overrides = leakStatuses.overrides
-  /** Whether the dialog that sets one is open, for the object the tab is on. */
-  var setsLeakStatus by remember { mutableStateOf(false) }
+  /**
+   * The verdict being set in each tab, by tab id, and empty in a window where nobody is setting one.
+   *
+   * Per tab rather than per window, because setting a verdict is not a dialog: it is drawn inside the tab
+   * it was started from, so the reader can open the reference, go and look at a verdict this one disagrees
+   * with, or read the chain, and come back to the reason half typed. Tab ids are never reused, so an entry
+   * here can only ever be about the tab it was made for. See [SettingVerdict] and [LeakStatusSetter].
+   */
+  val settingVerdicts = remember { mutableStateMapOf<Int, SettingVerdict>() }
+  /** And the one in the tab on screen, which is the only one drawn. */
+  val settingVerdict = tabs.selectedId?.let { settingVerdicts[it] }
 
   // Nothing is under the pointer while a list is showing: the view isn't there, so the rectangle it was on
   // last is neither where the pointer is now nor what the tab is about.
@@ -419,9 +429,6 @@ internal fun HeapDumpDive(
   // And on the statuses set by hand, because they are half of what a chain says: setting one is asking the
   // window to read the heap dump through it, which is this read again.
   LaunchedEffect(session, place, overrides) {
-    // Whatever was being decided was being decided about the object this is leaving, and a dialog that
-    // outlived the tab it was opened from would set a status on an object nobody is looking at.
-    setsLeakStatus = false
     if (place == null || place.viewRootObjectId == null) {
       details = null
       return@LaunchedEffect
@@ -741,22 +748,6 @@ internal fun HeapDumpDive(
     )
   }
 
-  if (setsLeakStatus && describedLeakStatus != null) {
-    LeakStatusDialog(
-      status = describedLeakStatus,
-      // A walk up the references per status already set, which is a read of the heap dump like any other —
-      // and one asked for, so it is not on the path the pointer takes. See [leakStatusConflictsWith].
-      onFindConflicts = { override ->
-        session.read("what setting ${hexObjectId(override.objectId)} to ${override.status} disagrees with") {
-          it.tree.leakStatusConflictsWith(override, overrides)
-        }
-      },
-      onSet = { override, solved -> leakStatuses.set(override, solved) },
-      onClear = { leakStatuses.clear(describedLeakStatus.objectId) },
-      onDismiss = { setsLeakStatus = false }
-    )
-  }
-
   Column(modifier) {
     ScreenBar(
       starredCount = stars.objectIds.size,
@@ -770,7 +761,12 @@ internal fun HeapDumpDive(
       titleOf = { it.title ?: placeTitles[it] ?: NAMING_TAB },
       hasNote = { notes.hasNote(it) },
       onSelect = { id -> tabs = tabs.select(id) },
-      onClose = { id -> tabs = tabs.close(id) },
+      onClose = { id ->
+        tabs = tabs.close(id)
+        // A verdict being set belongs to its tab, so closing the tab is abandoning it — and leaving the
+        // entry behind would hand it to whichever tab took that id, if ids were ever reused.
+        settingVerdicts.remove(id)
+      },
       onCopyLink = copyLink
     )
     // Two things in one row, and a rule between them: how the tab got here, then what it is on. The height is
@@ -798,6 +794,30 @@ internal fun HeapDumpDive(
         }
       }
     }
+    // Above the note and the panes, directly under the verdict it is changing, and inside the tab rather
+    // than over the window: the evidence for a verdict is the chain and the panes below, which a dialog
+    // would have been covering. See [LeakStatusSetter].
+    settingVerdict?.let { setting ->
+      LeakStatusSetter(
+        setting = setting,
+        // A walk up the references per status already set, which is a read of the heap dump like any other —
+        // and one asked for, so it is not on the path the pointer takes. See [leakStatusConflictsWith].
+        onFindConflicts = { override ->
+          session.read(
+            "what setting ${hexObjectId(override.objectId)} to ${override.status} disagrees with"
+          ) {
+            it.tree.leakStatusConflictsWith(override, overrides)
+          }
+        },
+        onSet = { override, solved -> leakStatuses.set(override, solved) },
+        onClear = { leakStatuses.clear(setting.status.objectId) },
+        // In a tab of its own and in front, the way a `?` opens a page: going to look at a verdict this one
+        // disagrees with is reading, and this tab keeps what was typed into it.
+        onOpenObject = { objectId -> openInNewTab(Place.Object(objectId)) },
+        onExplain = explain,
+        onDone = { tabs.selectedId?.let { settingVerdicts.remove(it) } }
+      )
+    }
     // Under the title it is about and above everything that describes it, so that what it is a note about is
     // the whole of what this tab is showing rather than whichever pane it happens to sit against.
     if (tabNote != null) {
@@ -884,7 +904,12 @@ internal fun HeapDumpDive(
             leakStatus = describedLeakStatus,
             isLeakStatusRead = leakStatuses.isRead,
             leakStatusProblem = leakStatuses.problem,
-            onChangeLeakStatus = { setsLeakStatus = true },
+            onChangeLeakStatus = {
+              val tabId = tabs.selectedId
+              if (tabId != null && describedLeakStatus != null) {
+                settingVerdicts[tabId] = SettingVerdict(describedLeakStatus)
+              }
+            },
             onOpen = openObject,
             onCopyLink = copyObjectLink,
             onListInstances = { className ->