Reach discussions inside collapsed details sections

AGENTS.md

@@ -231,6 +231,7 @@ reader to an apparently empty page.
 ### Inline review UI
 - **Highlights**: anchored text is wrapped in `<mark>` elements — subtle blue for `open`, unstyled for `resolved`; orange is reserved for viewer-specific attention, never thread status
 - **Margin dots**: colored indicators in the left margin aligned to each highlight's vertical position
+- **Collapsed sections**: a closed `<details>` hides its highlights, so it shows a compact blue count badge on its summary instead of margin dots (open threads only, unless resolved threads are shown). Every jump to a thread — `?thread=`, j/k, a badge — opens the enclosing sections first (`coplan/collapsed_sections.js`)
 - **Thread popovers**: native HTML Popover API (`popover="auto"`) showing the comment thread, reply form, and action buttons; positioned relative to the anchor and tracked on scroll
 - **Comment toolbar**: fixed bottom bar showing open thread count, j/k navigation, and a "Show resolved" toggle
 

docs/DEMO_LIBRARY.md

@@ -59,6 +59,7 @@ shipped types, including Scratchpad, PRD, Test Plan, and Handoff.
 | api-comment-anchors | Markdown text and table-row highlights; source-only discussion in Comments |
 | table-gallery | Precise cell discussion, repeated labels, empty values, links, wide expandable data |
 | comment-reactions | Discussions with emoji reactions; your own are pressed in blue, hover a chip for who reacted, add or remove one in place. Re-seeding keeps reactions you removed removed |
+| collapsed-discussions | Discussions inside closed `<details>`, one nested; each summary counts the open discussions it hides. Select a count, press j, follow a `?thread=` link, or open a section to reach them |
 | formatting-gallery | Headings, editable template notes hidden while reading, emphasis, quotes, nested lists, tasks, two code languages, image, mention, footnote |
 | discussion-margins | Crowded prose discussions, multiple participants, resolved visibility, and slide navigation/scrolling; avatars identify starters, not unread activity |
 | reading-specimen | A realistic design doc for judging reading in both modes: prose, headings, lists, tasks, quote, table, footnote, image, SQL/Ruby/diff/YAML code, a long scrolling line, flowchart and sequence diagrams, plus open, agent-answered, resolved, diagram-node, table-cell, and general discussions |

engine/app/assets/stylesheets/coplan/comments.css

@@ -207,6 +207,41 @@
   background: var(--color-highlight-open-hover-bg);
 }
 
+/* Discussions hidden in a closed <details>: a compact blue count on its
+   summary. The count is generated here so the summary's text, which
+   comment anchors match against, stays the author's. */
+.details-comment-badge {
+  display: inline-flex;
+  align-items: center;
+  gap: 3px;
+  margin-left: var(--space-sm);
+  min-width: 24px;
+  min-height: 24px;
+  justify-content: center;
+  padding: 1px 7px;
+  border: 1px solid transparent;
+  border-radius: 999px;
+  background: var(--color-primary-light);
+  color: var(--color-primary);
+  font: 700 0.7rem/1.4 var(--font-sans);
+  vertical-align: middle;
+  cursor: pointer;
+}
+
+.details-comment-badge::after { content: attr(data-count); }
+
+.details-comment-badge svg {
+  width: 12px;
+  height: 12px;
+}
+
+.details-comment-badge:hover { border-color: var(--color-highlight-open-border); }
+
+.details-comment-badge:focus-visible {
+  outline: 2px solid var(--color-primary);
+  outline-offset: 2px;
+}
+
 #plan-threads {
   position: absolute;
   width: 0;

engine/app/javascript/controllers/coplan/comment_margin_controller.js

@@ -1,4 +1,5 @@
 import { Controller } from "@hotwired/stimulus"
+import { hidingSection } from "coplan/collapsed_sections"
 
 // Collapsed discussion markers in the page margin, one per highlighted
 // thread, aligned with the text they discuss. A marker shows the first
@@ -7,7 +8,8 @@ import { Controller } from "@hotwired/stimulus"
 //
 // Cards are derived from the highlights coplan--text-selection builds and
 // from the thread popovers in #plan-threads, so broadcasts that add
-// threads or comments update them without a server round-trip.
+// threads or comments update them without a server round-trip. A closed
+// <details> shows a count badge on its summary instead of markers.
 const STACK_GAP = 4
 // Room beside the document card: gap from the card, gap from the window
 // edge, and the marker width.
@@ -50,12 +52,25 @@ export default class extends Controller {
   refresh() {
     const showResolved = !this.layout.classList.contains("plan-layout--hide-resolved")
     const marks = new Map()
-    for (const mark of this.content.querySelectorAll("mark.anchor-highlight[data-thread-id]")) {
+    const hidden = new Map()
+    const seen = new Set()
+    // Table-cell and diagram discussions have their own badges, so they
+    // only count toward a closed section.
+    for (const mark of this.content.querySelectorAll("mark.anchor-highlight[data-thread-id], [data-source-badge][data-thread-id]")) {
       const id = mark.dataset.threadId
-      if (marks.has(id) || !mark.getClientRects().length) continue
+      if (seen.has(id)) continue
       if (!showResolved && !mark.classList.contains("anchor-highlight--open")) continue
-      marks.set(id, mark)
+      const section = hidingSection(mark)
+      if (!section && (mark.hasAttribute("data-source-badge") || !mark.getClientRects().length)) continue
+      seen.add(id)
+      if (section) {
+        if (!hidden.has(section)) hidden.set(section, [])
+        hidden.get(section).push(id)
+      } else {
+        marks.set(id, mark)
+      }
     }
+    this.renderSectionBadges(hidden)
 
     for (const [id, card] of this.cards) {
       if (marks.has(id)) continue
@@ -67,13 +82,41 @@ export default class extends Controller {
       if (!popover) continue
       const card = this.cards.get(id) || this.buildCard(id)
       this.fillCard(card, popover, mark)
+      // A section can reveal this card after the popover's toggle event.
+      card.classList.toggle("is-active", popover.matches(":popover-open"))
     }
 
     this.marks = marks
     this.element.hidden = this.cards.size === 0
     this.position()
   }
 
+  // Each closed section counts the discussions it hides, in document order;
+  // its badge opens the section at the first one.
+  renderSectionBadges(sections) {
+    for (const badge of this.content.querySelectorAll(".details-comment-badge")) {
+      if (!sections.has(badge.closest("details"))) badge.remove()
+    }
+    for (const [details, ids] of sections) {
+      const summary = details.querySelector(":scope > summary")
+      if (!summary) continue
+      let badge = summary.querySelector(":scope > .details-comment-badge")
+      if (!badge) {
+        badge = document.createElement("button")
+        badge.type = "button"
+        badge.className = "details-comment-badge"
+        badge.dataset.action = "click->coplan--text-selection#openSectionThread"
+        // The count is CSS-generated: a text node here would enter the
+        // document text that comment anchors are matched against.
+        badge.innerHTML = COMMENT_ICON
+        summary.append(badge)
+      }
+      badge.dataset.threadId = ids[0]
+      badge.dataset.count = ids.length
+      badge.setAttribute("aria-label", `${ids.length} ${ids.length === 1 ? "discussion" : "discussions"} in this section, open the first`)
+    }
+  }
+
   // Reuse card elements: an open discussion window tracks its trigger
   // element while the page scrolls.
   buildCard(id) {
@@ -169,11 +212,16 @@ export default class extends Controller {
   }
 
   // Keep the card for the open discussion visibly selected, however it was
-  // opened (card, highlight, j/k, or a deep link).
-  popoverToggled(event) {
-    const popover = event.target
-    if (!popover.classList?.contains("thread-popover")) return
-    const card = this.cards.get(popover.id.replace(/_popover$/, ""))
+  // opened (card, highlight, j/k, or a deep link). Opening or closing a
+  // section in the document shows or hides the markers inside it.
+  toggled(event) {
+    const target = event.target
+    if (target.tagName === "DETAILS" && this.content.contains(target)) {
+      this.scheduleRefresh()
+      return
+    }
+    if (!target.classList?.contains("thread-popover")) return
+    const card = this.cards.get(target.id.replace(/_popover$/, ""))
     card?.classList.toggle("is-active", event.newState === "open")
   }
 }

engine/app/javascript/controllers/coplan/comment_nav_controller.js

@@ -1,6 +1,7 @@
 import { Controller } from "@hotwired/stimulus"
 import { registerShortcuts, commandFor } from "coplan/shortcuts"
 import { commentWindowTop } from "coplan/comment_window"
+import { revealSections } from "coplan/collapsed_sections"
 
 export default class extends Controller {
 
@@ -125,6 +126,7 @@ export default class extends Controller {
     if (slide) slide.dispatchEvent(new CustomEvent("coplan:deck-reveal", {
       bubbles: true, detail: { slide: slide.dataset.slide }
     }))
+    revealSections(mark)
     if (mark.hasAttribute("data-source-badge")) {
       const thread = document.getElementById(mark.dataset.threadId)
       if (thread) mark.dispatchEvent(new CustomEvent("coplan:source-thread", {

engine/app/javascript/controllers/coplan/source_comments_controller.js

@@ -1,6 +1,7 @@
 import { Controller } from "@hotwired/stimulus"
 import { captureReactionFocus } from "coplan/reaction_focus"
 import { commentWindowTop } from "coplan/comment_window"
+import { revealSections } from "coplan/collapsed_sections"
 
 // The panel stays inside this Stimulus scope, even in the modal expander.
 // Only signed source ranges cross the wire; SVG ids are render-time lookup
@@ -266,6 +267,7 @@ export default class extends Controller {
       if (slide) slide.dispatchEvent(new CustomEvent("coplan:deck-reveal", {
         bubbles: true, detail: { slide: slide.dataset.slide }
       }))
+      revealSections(element)
       element.scrollIntoView({ block: "center", behavior: "instant" })
       this.show(element, data.dataset.threadStatus === "resolved", event.detail.focusReply === true)
     }

engine/app/javascript/controllers/coplan/text_selection_controller.js

@@ -1,5 +1,6 @@
 import { Controller } from "@hotwired/stimulus"
 import { commentWindowTop } from "coplan/comment_window"
+import { revealSections } from "coplan/collapsed_sections"
 
 // Elements whose text is never rendered but still appears in textContent —
 // e.g. the <style> sheet Mermaid embeds inside its SVG. Their text must stay
@@ -354,6 +355,7 @@ export default class extends Controller {
     const highlighted = this.findAndHighlight(anchor, occurrence, "anchor-highlight--active")
     if (highlighted) {
       this._revealDeckSlide(highlighted)
+      revealSections(highlighted)
       highlighted.scrollIntoView({ behavior: "smooth", block: "center" })
     }
   }
@@ -363,6 +365,29 @@ export default class extends Controller {
     this._showThreadPopoverFor(event.currentTarget)
   }
 
+  // A collapsed section's discussion badge opens the section at its first
+  // discussion.
+  openSectionThread(event) {
+    event.preventDefault()
+    event.stopPropagation()
+    const id = event.currentTarget.dataset.threadId
+    const mark = this.contentTarget.querySelector(`mark.anchor-highlight[data-thread-id="${id}"], [data-source-badge][data-thread-id="${id}"]`)
+    const thread = document.getElementById(id)
+    if (!mark || !thread?.dataset.threadId) return
+    // Opening the section removes its badge. Keep a live focus-return target
+    // for both prose popovers and source-comment panels.
+    event.currentTarget.closest("summary")?.focus({ preventScroll: true })
+    revealSections(mark)
+    mark.scrollIntoView({ behavior: "instant", block: "center" })
+    if (mark.hasAttribute("data-source-badge")) {
+      mark.dispatchEvent(new CustomEvent("coplan:source-thread", {
+        bubbles: true, cancelable: true, detail: { threadId: thread.dataset.threadId, focusReply: false }
+      }))
+    } else {
+      this._showThreadPopoverFor(mark)
+    }
+  }
+
   openEditorThread(event) {
     event.preventDefault()
     event.stopPropagation()
@@ -478,6 +503,7 @@ export default class extends Controller {
   _showThreadPopoverFor(trigger) {
     if (!trigger) return false
     this._revealDeckSlide(trigger)
+    revealSections(trigger)
     const threadId = trigger.dataset.threadId
     if (!threadId) return false
 
@@ -888,8 +914,7 @@ export default class extends Controller {
           const trigger = generalVoiceComment ? document.querySelector(".voice-btn") || currentMark : currentMark
           if (generalVoiceComment) trigger.dataset.threadId = domId
           else {
-            const disclosure = currentMark.closest("details.document-support__disclosure")
-            if (disclosure) disclosure.open = true
+            revealSections(currentMark)
             this._revealDeckSlide(currentMark)
             currentMark.scrollIntoView({ behavior: "instant", block: "center" })
           }

engine/app/javascript/coplan/collapsed_sections.js

@@ -0,0 +1,33 @@
+// A closed <details> keeps its content in the DOM. Highlights inside it
+// exist, and Chrome even reports client rects for them, but the reader
+// cannot see them. Its <summary> stays visible, so content there is not
+// hidden by its own section.
+
+function hiddenBy(details, element) {
+  return !details.open && !details.querySelector(":scope > summary")?.contains(element)
+}
+
+function* sections(element) {
+  for (let details = element.closest("details"); details; details = details.parentElement?.closest("details")) {
+    yield details
+  }
+}
+
+// The outermost closed section hiding the element: the one whose summary
+// the reader can currently see.
+export function hidingSection(element) {
+  let hiding = null
+  for (const details of sections(element)) if (hiddenBy(details, element)) hiding = details
+  return hiding
+}
+
+// Open every section that hides the element. Returns whether any opened.
+export function revealSections(element) {
+  let revealed = false
+  for (const details of sections(element)) {
+    if (!hiddenBy(details, element)) continue
+    details.open = true
+    revealed = true
+  }
+  return revealed
+}

engine/app/views/coplan/agent_instructions/guides/markdown.text.erb

@@ -113,6 +113,8 @@ Markdown here still renders. Keep the blank line after the summary.
 ```
 
 Add `open` to the `<details>` tag to show the section expanded by default.
+Comments inside a collapsed section stay reachable: its summary shows how many open discussions it hides.
+Following a discussion link opens its enclosing sections, including for table and diagram comments.
 
 ## Hover definitions
 

engine/app/views/coplan/plans/show.html.erb

@@ -158,7 +158,7 @@
           about. Positioned outside the card, so the text keeps its width. %>
       <aside class="comment-margin" aria-label="Discussions" hidden
              data-controller="coplan--comment-margin"
-             data-action="coplan:deck-slide-changed@document->coplan--comment-margin#refresh scroll@document->coplan--comment-margin#position:capture coplan:anchors-updated@document->coplan--comment-margin#refresh coplan:resolved-visibility@document->coplan--comment-margin#refresh resize@window->coplan--comment-margin#position toggle@document->coplan--comment-margin#popoverToggled:capture mouseover@document->coplan--comment-margin#hoverChanged"></aside>
+             data-action="coplan:deck-slide-changed@document->coplan--comment-margin#refresh scroll@document->coplan--comment-margin#position:capture coplan:anchors-updated@document->coplan--comment-margin#refresh coplan:resolved-visibility@document->coplan--comment-margin#refresh resize@window->coplan--comment-margin#position toggle@document->coplan--comment-margin#toggled:capture mouseover@document->coplan--comment-margin#hoverChanged"></aside>
     </div>
 </div>
 

engine/config/importmap.rb

@@ -4,6 +4,7 @@
 pin "coplan/shortcuts", to: "coplan/shortcuts.js", preload: true
 pin "coplan/reaction_focus", to: "coplan/reaction_focus.js", preload: true
 pin "coplan/comment_window", to: "coplan/comment_window.js", preload: true
+pin "coplan/collapsed_sections", to: "coplan/collapsed_sections.js", preload: true
 # Statically imported by the mermaid and data-grid controllers, which are
 # themselves preloaded — preload these too or every page pays a round trip
 # to discover them.

engine/db/demo/content/collapsed-discussions.md

@@ -0,0 +1,48 @@
+Checkout got slower after the March pricing release. The p95 for `POST /checkout` went from 420 ms to 610 ms. Almost all of the extra time is spent loading discount rules for each line item. Caching the rules per cart brings the p95 back to 430 ms in staging. We plan to ship the cache behind a flag next week.
+
+The evidence and the options we rejected are in the collapsed sections below. Each closed section shows how many open discussions it hides. Select the count to open the section at its first discussion, or press j to step through every discussion, hidden or not.
+
+## Findings
+
+Slow carts are large carts. Carts with more than 20 line items account for most of the regression, and small carts are unchanged.
+
+<details>
+<summary>Raw measurements</summary>
+
+We sampled 50,000 checkout requests from the week before and the week after the release.
+
+| Cart size | p95 before | p95 after |
+| --- | --- | --- |
+| 1–5 items | 380 ms | 390 ms |
+| 6–20 items | 450 ms | 520 ms |
+| 21+ items | 690 ms | 1,240 ms |
+
+Each line item runs one discount rule query, so a 40-item cart makes 40 round trips to the pricing database.
+
+<details>
+<summary>Query plan</summary>
+
+The rule lookup uses the merchant index but filters item categories in memory, so every query reads all of a merchant's active rules.
+
+</details>
+
+Weekend traffic was excluded because a marketing campaign doubled cart sizes on those days.
+
+</details>
+
+## Options
+
+We compared three ways to remove the repeated lookups.
+
+<details>
+<summary>Alternatives considered</summary>
+
+Batching the queries into one request per cart halves the time but still grows with merchant rule count.
+
+Precomputing prices nightly removes the lookups, but price changes would take up to a day to appear.
+
+</details>
+
+## Plan
+
+Cache each merchant's active rules for the life of one cart request. Invalidate the cache when a merchant edits a rule. Roll out to 10% of merchants first and compare p95 by cart size.