spec: <details>/<summary> support in markdown rendering (#10259) - #15543
spec: <details>/<summary> support in markdown rendering (#10259)#15543fbartho wants to merge 6 commits into
Conversation
…rgences (GH10259) Raise the nesting bound from 8 to 64 and reframe it as a stack-safety guard on the recursive body parse rather than a product limit. The number is now derived: the recursion costs ~176 bytes per level, and a 2 MiB stack absorbs 8,000 levels of a comparable block parse, so 64 sits orders of magnitude below the failure point while exceeding real content. Drop the 512-widget document cap entirely. Rendering builds elements only for viewport-visible items, so off-screen widgets cost nothing, and no comparable element carries a per-document cap. Its backtracking-safe counter discipline, determinism risk, and test rows go with it. State plainly that the buffer accepts every edit; markdown is a plain-text format and the editor never rejects a keystroke over the structure it would produce. The guard applies only at the read boundary. Add a divergences-from-GitHub subsection covering list-item nesting, blockquote nesting, and block content in summaries, each with its representation gap. These scope the implementation to what the current flat line IR supports; convergence is future work. Promote the blockquote and list-item rules to numbered behaviors, and record the agent-surface disclosure component and the rejected iterative parse as decisions in Tradeoffs. No open questions remain in either file.
github-actions
Bot
added
the
external-contributor
|
I'm starting a first review of this spec-only pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
![warp-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for rendering <details>/<summary> as collapsible markdown disclosures across parser, editor-buffer, GUI, notebook/plan editor, and TUI surfaces. The scope is clear and the design addresses malformed input, accessibility identifiers, streaming, and testing.
Concerns
- The editor-buffer malformed-marker rules do not consistently match the product spec's deterministic degradation rules, which could lead an implementation to drop or reinterpret boundary markers after edits.
- The depth-boundary parser test expectation conflicts with the stated over-depth literal-text fallback.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…h-boundary test (GH10259) The buffer's edit-rebalancing rules contradicted the product degradation contract. An unmatched End was specified as "skipped on serialization" where behavior 11(b) requires literal text, a stray EndSummary was "inert" with no serialization rule where 11(h) requires literal text, and a Start with no EndSummary was mapped to 11(g) with its summary/body assignment inverted -- that buffer shape is a summary-less region under 11(e), not an unclosed summary. Product behaviors 10 and 11(a)-11(h) now each state the literal markdown serialization emits, making product.md the single home for every degradation rule; tech.md references them instead of restating. to_markdown gains the invariant that no arm emits nothing for a marker, so no edit or save can delete a details tag. The depth-boundary test row claimed identical raw_text at depth 64 and 65, which cannot hold: a container's raw_text carries summary and body without tags, while the over-depth literal fallback includes the tag characters. Both rows now state the exact expectation. Sibling sweep over the spec directory fixed the same miss-cause in the buffer depth-boundary paragraph, the to_markdown round-trip paragraph, and four test rows.
|
/warp-agent-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for rendering <details>/<summary> as collapsible markdown disclosure sections across the parser IR, rich-text editor buffer, GUI surfaces, and TUI fallback.
Concerns
- The proposed
FormattedDetailsrepresentation loses source-shape state that the product spec requires for deterministic malformed-input round trips, so several required serialization behaviors cannot be implemented as written. - The product behavior needs to define event precedence for links inside an otherwise toggleable summary row.
- No separate security findings were identified in the high-level security pass.
Verdict
Found: 1 critical, 1 important, 0 suggestions
Request changes
Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
…dence (GH10259) The round-trip contract requires serialization to reproduce each malformed shape as the characters the author typed, but the parser IR carried only summary text, body, and default_open. That cannot separate a summary-less region from <summary>Details</summary>, an unclosed region from a closed one, or an unclosed summary from a closed one, so to_markdown had no basis for choosing what to emit. FormattedDetails gains a closed flag and a DetailsSummary enum (Absent / Closed / Unclosed). The buffer markers already encode two of the three distinctions through marker presence; only summary closedness needs payload, which EndSummary now carries. The no-dropped- tag invariant is rescoped to authored tags, since an unclosed summary must not have a closing tag fabricated for it. Clicking a link in a summary activated both the link and the toggle under behaviors 3 and 6. The innermost interactive element now wins, and a sweep for the same unspecified-precedence class added rules for drag versus click on the summary row, focus and caret reachability into a collapsed body, and collapse-state lifetime across edits and streaming updates.
|
/warp-agent-review |
|
I'm re-reviewing this spec-only pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds product and technical specs for rendering <details>/<summary> markdown as collapsible disclosure sections across the markdown parser, editor buffer, GUI markdown surfaces, notebooks/plans, and TUI fallback rendering.
Concerns
No blocking concerns found. The specs cover malformed input, GitHub divergences, accessibility, streaming/editing behavior, parser/editor representation choices, testing, and the relevant security-sensitive handling of renderer-generated identifiers versus markdown-provided attributes.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /warp-agent-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
Toggling never touches the open attribute; reopening a document restores every section to its default state.
1 participant
Closes #10259
What
Product and technical spec for
<details>/<summary>support in the Markdown viewer, underspecs/GH10259/.FormattedTextLine::Detailscontainer in the parser IR; fence-awareparse_detailsbranch; three-markerBufferText::Details(Start/EndSummary/End)span in the editor buffer on theLinkprecedent; nesting depth as a SumTree counter; collapse reusesHiddenLinesModel.Design notes
The buffer model follows the review direction from #13345: details are a top-level zero-width span marker, not a
BufferBlockStyle, and the summary is ordinary buffer text betweenStartandEndSummary, so its inline formatting uses the existing style machinery.Divergences from GitHub
Taking GitHub-flavored Markdown as the reference implementation, these are the divergences:
<details>indented under a list item terminates the list (GitHub nests it inside the item)<details>inside a blockquote renders as literal text (GitHub nests it)<summary>renders as inline text (GitHub renders real blocks)Each needs a container in the parser IR, which is a flat line sequence today. I wanted to scope the PR down to a minimal impact for maintainers to consider. We could instead change to an iterative model, but that would require more design and discussion that I felt should happen in a separate issue?
Relation to prior work
#13345 was auto-closed 14 August after requested changes went unaddressed; this spec adopts the design its review converged on. #10462 is open but dormant since May.