Fix ConditionalBlock crash in manuscript projects

.claude/docs/plans/pandoc-list-vs-blocks-analysis.md

@@ -0,0 +1,256 @@
+# pandoc.List() vs pandoc.Blocks() Bug Analysis
+
+## Executive Summary
+
+The bug fixed in manuscript.lua (PR #13624) is **NOT ConditionalBlock-specific**. It's a type mismatch bug where `pandoc.List()` is used instead of `pandoc.Blocks()` when building block collections that are assigned to element `.content` properties. ConditionalBlock merely **exposed** this bug, but the bug could be triggered by any code path that walks the AST of an element with wrongly-typed content.
+
+## The Core Bug: Type Mismatch
+
+### What's the Difference?
+
+From the Lua type definitions in quarto-cli:
+
+**pandoc.Blocks** (`src/resources/pandoc/blocks.lua:27-29`):
+```lua
+-- List of Block elements, a special case of a pandoc.List.
+-- pandoc.Blocks is a type alias for pandoc.List, but it also
+-- supports a `walk` method.
+```
+
+**pandoc.List** (`src/resources/pandoc/List.lua`):
+- Standard Lua List implementation
+- Has methods like `:insert()`, `:extend()`, `:map()`, etc.
+- Does NOT have a `.walk()` method in the standard implementation
+
+### The Bug Pattern
+
+```lua
+-- BUGGY CODE
+local blocks = pandoc.List()
+for _, child in ipairs(divEl.content) do
+  blocks:insert(child)
+end
+divEl.content = blocks  -- BUG: Assigns List to .content
+
+-- Later in the filter chain...
+_quarto.ast.walk(divEl, {...})  -- CRASH: tries to walk divEl.content
+```
+
+When you assign a `pandoc.List()` to `.content`, any subsequent AST walking operations that expect `pandoc.Blocks` (with its `.walk()` method) will fail.
+
+### Why It Crashes
+
+The crash happens when:
+1. Code assigns `pandoc.List()` to an element's `.content` property
+2. That element is returned from a filter or passed to another filter
+3. Some code tries to walk the AST, either:
+   - Directly via `_quarto.ast.walk(element, ...)`
+   - Indirectly when a filter returns `element.content` and Pandoc's filter system tries to process it
+   - When custom filters traverse the AST
+
+The error manifests as attempting to call a method that doesn't exist on `pandoc.List`.
+
+## ConditionalBlock's Role: Exposing, Not Causing
+
+### What ConditionalBlock Does
+
+From `src/resources/filters/quarto-pre/content-hidden.lua:63`:
+```lua
+function conditionalBlock(show)
+  return function(el)
+    if show then
+      return el.content  -- Returns the Blocks directly
+    else
+      return {}
+    end
+  end
+end
+```
+
+When `show` is true, ConditionalBlock returns `el.content` **as-is**. This propagates whatever type `.content` is up the filter chain.
+
+### How It Exposed the Bug
+
+1. manuscript.lua assigns `pandoc.List()` to `divEl.content`
+2. That `divEl` contains a ConditionalBlock
+3. ConditionalBlock's filter runs and returns `el.content` (the wrongly-typed List)
+4. Later code tries to walk this returned content
+5. CRASH: `pandoc.List` doesn't have the expected methods
+
+**But ConditionalBlock is just the messenger!** The bug was already there when `pandoc.List()` was assigned to `.content`. ConditionalBlock just exposed it by propagating the wrong type upward.
+
+## Other Potential Triggers (Without ConditionalBlock)
+
+The bug could be triggered by:
+
+### 1. Direct AST Walking
+```lua
+local blocks = pandoc.List()
+divEl.content = blocks
+_quarto.ast.walk(divEl, {...})  -- CRASH: even without ConditionalBlock
+```
+
+### 2. Custom Filters That Return .content
+Any custom filter extension that does:
+```lua
+function MyFilter(el)
+  return el.content  -- If content is wrongly-typed List
+end
+```
+
+### 3. Filter Chain Interactions
+When filters compose and one expects `pandoc.Blocks` but receives `pandoc.List`.
+
+### 4. Format-Specific Processing
+Some output formats may do additional AST traversal that would trigger the crash.
+
+## Analysis of Each Occurrence
+
+### manuscript.lua (Line 68) - **BUGGY** ✗
+
+**Code**:
+```lua
+local blocks = pandoc.List()
+-- ... build blocks ...
+divEl.content = blocks  -- Assigns List to .content
+```
+
+**Why it's buggy**:
+- Assigns `pandoc.List()` to `divEl.content`
+- Later code walks the AST (line 138: `_quarto.ast.walk(divEl, ...)`)
+- Exposed by ConditionalBlock, but would crash on any AST walk
+
+**Fix applied**: Changed to `pandoc.Blocks({})`
+
+### outputs.lua (Line 41) - **SAFE** ✓
+
+**Code**:
+```lua
+local blocks = pandoc.List()
+-- ... build blocks ...
+return blocks  -- Returns directly from filter
+```
+
+**Why it's safe**:
+- Returns `pandoc.List()` directly from the filter
+- Does NOT assign to `.content`
+- Pandoc's filter system can handle returned Lists
+- The returned blocks are used to replace content, not stored as-is
+
+**No fix needed**: Return pattern is safe.
+
+### jats.lua quarto-post (Line 57) - **SAFE** ✓
+
+**Code**:
+```lua
+function unrollDiv(div, fnSkip)
+  local blocks = pandoc.List()
+  -- ... build blocks ...
+  return blocks  -- Returns directly
+end
+```
+
+**Why it's safe**: Same as outputs.lua - returns directly, never assigns to `.content`.
+
+**No fix needed**: Return pattern is safe.
+
+### jats.lua quarto-post (Line 175) - **BUGGY** ✗
+
+**Code**:
+```lua
+local orderedContents = pandoc.List()
+orderedContents:extend(otherEls)
+orderedContents:extend(outputEls)
+div.content = orderedContents  -- Assigns List to .content
+```
+
+**Why it's buggy**:
+- Assigns `pandoc.List()` to `div.content`
+- Same pattern as manuscript.lua
+- Could crash if this div is later walked or returned by a filter like ConditionalBlock
+
+**Needs fix**: Change line 172 to `pandoc.Blocks({})`
+
+### dashboard.lua (Lines 248, 296) - **SAFE** ✓
+
+**Code**:
+```lua
+local results = pandoc.List()
+-- ... build results ...
+return pandoc.Blocks(results)  -- Wraps in pandoc.Blocks before return
+```
+
+**Why it's safe**:
+- Uses `pandoc.List()` internally (fine for building)
+- Wraps in `pandoc.Blocks()` before returning
+- Returns the correct type
+
+**No fix needed**: Already handles type correctly.
+
+## The Fix Pattern
+
+### When to Use pandoc.Blocks()
+
+Use `pandoc.Blocks({})` when:
+1. Building a collection that will be assigned to `.content`
+2. Building a collection that will be part of the document AST
+3. Building blocks that need to support `.walk()` operations
+
+```lua
+-- CORRECT
+local blocks = pandoc.Blocks({})
+blocks:extend(childContent)
+element.content = blocks
+```
+
+### When pandoc.List() is OK
+
+Use `pandoc.List()` when:
+1. Building a temporary collection that will be returned directly (filter system handles it)
+2. Building a collection that will be wrapped in `pandoc.Blocks()` before use
+3. Building non-Block collections (like List of Inlines, table rows, etc.)
+
+```lua
+-- CORRECT: Direct return
+local blocks = pandoc.List()
+blocks:insert(something)
+return blocks  -- Filter system handles conversion
+
+-- CORRECT: Explicit wrap
+local blocks = pandoc.List()
+blocks:insert(something)
+return pandoc.Blocks(blocks)
+
+-- CORRECT: Non-block collection
+local inlines = pandoc.List()  -- Building Inlines, not Blocks
+```
+
+## Root Cause Summary
+
+The bug is fundamentally about **type safety in Lua's Pandoc AST**:
+
+1. `.content` properties expect `pandoc.Blocks` (which has `.walk()`)
+2. Using `pandoc.List()` creates wrong type without required methods
+3. Crash occurs when AST traversal tries to use `.walk()` on the List
+4. ConditionalBlock exposed the bug by returning wrongly-typed content
+5. But ANY AST walking or content propagation could trigger it
+
+## Recommended Actions
+
+1. **Fix jats.lua line 172**: Change `pandoc.List()` to `pandoc.Blocks({})`
+2. **Test verification**: Create tests for both manuscript and JATS formats with ConditionalBlock
+3. **Pattern search**: Search codebase for `.content = .*pandoc.List` pattern
+4. **Documentation**: Consider adding a code comment in blocks.lua about when to use which type
+
+## Testing Strategy
+
+Tests should verify:
+1. Documents with ConditionalBlock in manuscript format (already exists)
+2. Documents with ConditionalBlock in JATS format (needs creation)
+3. Direct AST walking on elements with `.content` assigned (stress test)
+4. Filter composition scenarios
+
+The tests should confirm that:
+- Fixed code works with ConditionalBlock
+- Fixed code works with direct AST walking
+- Safe patterns (outputs.lua, dashboard.lua) continue working

src/resources/filters/layout/manuscript.lua

@@ -57,10 +57,10 @@ function manuscript()
         -- If this is a notebook embed cell, 'lift' the contents of any child divs
         -- up (unroll their contents), this will help us avoid
         -- labeling divs marked as `cells` more than once
-        local blocks = pandoc.List()
+        local blocks = pandoc.Blocks({})
         for _, childBlock in ipairs(divEl.content) do
           if is_regular_node(childBlock, "Div") then
-              tappend(blocks, childBlock.content)
+              blocks:extend(childBlock.content)
           else
             blocks:insert(childBlock)
           end

tests/docs/smoke-all/2025/10/27/13616/.gitignore

@@ -0,0 +1,2 @@
+/.quarto/
+**/*.quarto_ipynb

tests/docs/smoke-all/2025/10/27/13616/13616.qmd

@@ -0,0 +1,13 @@
+---
+title: "Conditional Block in manuscript article"
+_quarto:
+  tests:
+    pdf:
+      noErrors: true
+---
+
+::: {.content-visible when-format="pdf"}
+
+Something
+
+:::

tests/docs/smoke-all/2025/10/27/13616/_quarto.yml

@@ -0,0 +1,9 @@
+project:
+  type: manuscript
+
+manuscript:
+  article: 13616.qmd
+
+format:
+  pdf:
+    pdf-engine: xelatex