Skip to content

Commit 6e74c59

Browse files
cdervcderv
committed
TO REMOVE : plan doc for claude
1 parent 5e8020a commit 6e74c59

File tree

1 file changed

+256
-0
lines changed

1 file changed

+256
-0
lines changed

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

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,256 @@
1+
# pandoc.List() vs pandoc.Blocks() Bug Analysis
2+
3+
## Executive Summary
4+
5+
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.
6+
7+
## The Core Bug: Type Mismatch
8+
9+
### What's the Difference?
10+
11+
From the Lua type definitions in quarto-cli:
12+
13+
**pandoc.Blocks** (`src/resources/pandoc/blocks.lua:27-29`):
14+
```lua
15+
-- List of Block elements, a special case of a pandoc.List.
16+
-- pandoc.Blocks is a type alias for pandoc.List, but it also
17+
-- supports a `walk` method.
18+
```
19+
20+
**pandoc.List** (`src/resources/pandoc/List.lua`):
21+
- Standard Lua List implementation
22+
- Has methods like `:insert()`, `:extend()`, `:map()`, etc.
23+
- Does NOT have a `.walk()` method in the standard implementation
24+
25+
### The Bug Pattern
26+
27+
```lua
28+
-- BUGGY CODE
29+
local blocks = pandoc.List()
30+
for _, child in ipairs(divEl.content) do
31+
blocks:insert(child)
32+
end
33+
divEl.content = blocks -- BUG: Assigns List to .content
34+
35+
-- Later in the filter chain...
36+
_quarto.ast.walk(divEl, {...}) -- CRASH: tries to walk divEl.content
37+
```
38+
39+
When you assign a `pandoc.List()` to `.content`, any subsequent AST walking operations that expect `pandoc.Blocks` (with its `.walk()` method) will fail.
40+
41+
### Why It Crashes
42+
43+
The crash happens when:
44+
1. Code assigns `pandoc.List()` to an element's `.content` property
45+
2. That element is returned from a filter or passed to another filter
46+
3. Some code tries to walk the AST, either:
47+
- Directly via `_quarto.ast.walk(element, ...)`
48+
- Indirectly when a filter returns `element.content` and Pandoc's filter system tries to process it
49+
- When custom filters traverse the AST
50+
51+
The error manifests as attempting to call a method that doesn't exist on `pandoc.List`.
52+
53+
## ConditionalBlock's Role: Exposing, Not Causing
54+
55+
### What ConditionalBlock Does
56+
57+
From `src/resources/filters/quarto-pre/content-hidden.lua:63`:
58+
```lua
59+
function conditionalBlock(show)
60+
return function(el)
61+
if show then
62+
return el.content -- Returns the Blocks directly
63+
else
64+
return {}
65+
end
66+
end
67+
end
68+
```
69+
70+
When `show` is true, ConditionalBlock returns `el.content` **as-is**. This propagates whatever type `.content` is up the filter chain.
71+
72+
### How It Exposed the Bug
73+
74+
1. manuscript.lua assigns `pandoc.List()` to `divEl.content`
75+
2. That `divEl` contains a ConditionalBlock
76+
3. ConditionalBlock's filter runs and returns `el.content` (the wrongly-typed List)
77+
4. Later code tries to walk this returned content
78+
5. CRASH: `pandoc.List` doesn't have the expected methods
79+
80+
**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.
81+
82+
## Other Potential Triggers (Without ConditionalBlock)
83+
84+
The bug could be triggered by:
85+
86+
### 1. Direct AST Walking
87+
```lua
88+
local blocks = pandoc.List()
89+
divEl.content = blocks
90+
_quarto.ast.walk(divEl, {...}) -- CRASH: even without ConditionalBlock
91+
```
92+
93+
### 2. Custom Filters That Return .content
94+
Any custom filter extension that does:
95+
```lua
96+
function MyFilter(el)
97+
return el.content -- If content is wrongly-typed List
98+
end
99+
```
100+
101+
### 3. Filter Chain Interactions
102+
When filters compose and one expects `pandoc.Blocks` but receives `pandoc.List`.
103+
104+
### 4. Format-Specific Processing
105+
Some output formats may do additional AST traversal that would trigger the crash.
106+
107+
## Analysis of Each Occurrence
108+
109+
### manuscript.lua (Line 68) - **BUGGY** ✗
110+
111+
**Code**:
112+
```lua
113+
local blocks = pandoc.List()
114+
-- ... build blocks ...
115+
divEl.content = blocks -- Assigns List to .content
116+
```
117+
118+
**Why it's buggy**:
119+
- Assigns `pandoc.List()` to `divEl.content`
120+
- Later code walks the AST (line 138: `_quarto.ast.walk(divEl, ...)`)
121+
- Exposed by ConditionalBlock, but would crash on any AST walk
122+
123+
**Fix applied**: Changed to `pandoc.Blocks({})`
124+
125+
### outputs.lua (Line 41) - **SAFE** ✓
126+
127+
**Code**:
128+
```lua
129+
local blocks = pandoc.List()
130+
-- ... build blocks ...
131+
return blocks -- Returns directly from filter
132+
```
133+
134+
**Why it's safe**:
135+
- Returns `pandoc.List()` directly from the filter
136+
- Does NOT assign to `.content`
137+
- Pandoc's filter system can handle returned Lists
138+
- The returned blocks are used to replace content, not stored as-is
139+
140+
**No fix needed**: Return pattern is safe.
141+
142+
### jats.lua quarto-post (Line 57) - **SAFE** ✓
143+
144+
**Code**:
145+
```lua
146+
function unrollDiv(div, fnSkip)
147+
local blocks = pandoc.List()
148+
-- ... build blocks ...
149+
return blocks -- Returns directly
150+
end
151+
```
152+
153+
**Why it's safe**: Same as outputs.lua - returns directly, never assigns to `.content`.
154+
155+
**No fix needed**: Return pattern is safe.
156+
157+
### jats.lua quarto-post (Line 175) - **BUGGY** ✗
158+
159+
**Code**:
160+
```lua
161+
local orderedContents = pandoc.List()
162+
orderedContents:extend(otherEls)
163+
orderedContents:extend(outputEls)
164+
div.content = orderedContents -- Assigns List to .content
165+
```
166+
167+
**Why it's buggy**:
168+
- Assigns `pandoc.List()` to `div.content`
169+
- Same pattern as manuscript.lua
170+
- Could crash if this div is later walked or returned by a filter like ConditionalBlock
171+
172+
**Needs fix**: Change line 172 to `pandoc.Blocks({})`
173+
174+
### dashboard.lua (Lines 248, 296) - **SAFE** ✓
175+
176+
**Code**:
177+
```lua
178+
local results = pandoc.List()
179+
-- ... build results ...
180+
return pandoc.Blocks(results) -- Wraps in pandoc.Blocks before return
181+
```
182+
183+
**Why it's safe**:
184+
- Uses `pandoc.List()` internally (fine for building)
185+
- Wraps in `pandoc.Blocks()` before returning
186+
- Returns the correct type
187+
188+
**No fix needed**: Already handles type correctly.
189+
190+
## The Fix Pattern
191+
192+
### When to Use pandoc.Blocks()
193+
194+
Use `pandoc.Blocks({})` when:
195+
1. Building a collection that will be assigned to `.content`
196+
2. Building a collection that will be part of the document AST
197+
3. Building blocks that need to support `.walk()` operations
198+
199+
```lua
200+
-- CORRECT
201+
local blocks = pandoc.Blocks({})
202+
blocks:extend(childContent)
203+
element.content = blocks
204+
```
205+
206+
### When pandoc.List() is OK
207+
208+
Use `pandoc.List()` when:
209+
1. Building a temporary collection that will be returned directly (filter system handles it)
210+
2. Building a collection that will be wrapped in `pandoc.Blocks()` before use
211+
3. Building non-Block collections (like List of Inlines, table rows, etc.)
212+
213+
```lua
214+
-- CORRECT: Direct return
215+
local blocks = pandoc.List()
216+
blocks:insert(something)
217+
return blocks -- Filter system handles conversion
218+
219+
-- CORRECT: Explicit wrap
220+
local blocks = pandoc.List()
221+
blocks:insert(something)
222+
return pandoc.Blocks(blocks)
223+
224+
-- CORRECT: Non-block collection
225+
local inlines = pandoc.List() -- Building Inlines, not Blocks
226+
```
227+
228+
## Root Cause Summary
229+
230+
The bug is fundamentally about **type safety in Lua's Pandoc AST**:
231+
232+
1. `.content` properties expect `pandoc.Blocks` (which has `.walk()`)
233+
2. Using `pandoc.List()` creates wrong type without required methods
234+
3. Crash occurs when AST traversal tries to use `.walk()` on the List
235+
4. ConditionalBlock exposed the bug by returning wrongly-typed content
236+
5. But ANY AST walking or content propagation could trigger it
237+
238+
## Recommended Actions
239+
240+
1. **Fix jats.lua line 172**: Change `pandoc.List()` to `pandoc.Blocks({})`
241+
2. **Test verification**: Create tests for both manuscript and JATS formats with ConditionalBlock
242+
3. **Pattern search**: Search codebase for `.content = .*pandoc.List` pattern
243+
4. **Documentation**: Consider adding a code comment in blocks.lua about when to use which type
244+
245+
## Testing Strategy
246+
247+
Tests should verify:
248+
1. Documents with ConditionalBlock in manuscript format (already exists)
249+
2. Documents with ConditionalBlock in JATS format (needs creation)
250+
3. Direct AST walking on elements with `.content` assigned (stress test)
251+
4. Filter composition scenarios
252+
253+
The tests should confirm that:
254+
- Fixed code works with ConditionalBlock
255+
- Fixed code works with direct AST walking
256+
- Safe patterns (outputs.lua, dashboard.lua) continue working

0 commit comments

Comments
 (0)