fix: preserve ordered list numbering across bullet interruptions in export

[JohnMcLear]

Summary

• When ordered lists were interrupted by unordered lists, each new <ol> segment started at 1 instead of continuing the previous numbering
• Added a counter (olItemCounts) per indent level in the export code that tracks how many ordered items have been emitted
• When reopening an <ol> after an interruption, emits <ol start="N"> with the correct continuation number
• Counters reset when exiting all lists (non-list line encountered)

Test plan

• 3 new backend tests in export_list.ts:
  • Round-trip with explicit start attributes preserved
  • No explicit start — counter-based fix adds start="2"
  • Multiple items before interruption — correctly emits start="3"
• Manual: create pad with 1. First / * bullet / 2. Second, export to HTML, verify numbering

Fixes #6471

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Preserve ordered list numbering across bullet interruptions in export

🐞 Bug fix

Walkthroughs

Description

• Preserve ordered list numbering when interrupted by unordered lists in HTML export
• Track item counts per indent level using olItemCounts counter
• Emit start attribute when reopening <ol> after interruption
• Reset counters when exiting all lists to prevent cross-contamination

Diagram

flowchart LR
  A["Ordered list item"] --> B["Track count in olItemCounts"]
  B --> C["Unordered list interrupts"]
  C --> D["Reopen ordered list"]
  D --> E["Emit start attribute with tracked count"]
  F["Exit all lists"] --> G["Reset olItemCounts"]

Loading

File Changes

1. src/node/utils/ExportHtml.ts 🐞 Bug fix +20/-1

Track and restore ordered list numbering across interruptions

• Added olItemCounts object to track running ordered-list item counts per indent level
• Modified ordered list opening logic to check counter and emit start attribute when reopening
 after interruption
• Added counter increment logic when processing ordered list items
• Added counter reset logic when exiting all lists (non-list line encountered)

src/node/utils/ExportHtml.ts

---

2. src/tests/backend/specs/export_list.ts 🧪 Tests +52/-9

Add comprehensive tests for ordered list numbering fix

• Updated existing test name to clarify it tests round-trip with explicit start attributes
• Added new test for counter-based fix when import has no explicit start attributes
• Added new test for multiple ordered items before and after bullet interruptions
• All tests verify correct start attribute values in exported HTML

src/tests/backend/specs/export_list.ts

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX Issues (0)

1. Explicit start overridden ☑ 🐞 Bug ≡ Correctness

Description

When opening a new ordered list, the counter-based continuation takes precedence over line.start,
so an explicitly stored start value can be ignored and replaced with a computed value. This can
export incorrect numbering for lists that intentionally restart or that were imported with explicit
`` semantics.

Code

src/node/utils/ExportHtml.ts[R394-401]

+              if (olItemCounts[line.listLevel] && olItemCounts[line.listLevel] > 0) {
+                // Continue numbering after an unordered-list interruption
+                const startNum = olItemCounts[line.listLevel] + 1;
+                pieces.push(`<ol start="${startNum}" class="${line.listTypeName}">`);
+              } else if (line.start) {
              pieces.push(`<ol start="${Number(line.start)}" class="${line.listTypeName}">`);
            } else {
              pieces.push(`<ol class="${line.listTypeName}">`);

Evidence

line.start is populated from the document’s stored line attribute start, but the PR’s new
conditional uses olItemCounts[...] before checking line.start, so any existing counter at that
level overrides the explicit start value.

src/node/utils/ExportHtml.ts[393-402]
src/node/utils/ExportHelper.ts[55-78]
src/static/js/ace2_inner.ts[2284-2341]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The HTML exporter opens a new `<ol>` using `olItemCounts` *before* honoring `line.start`, which means explicit list starts (from stored attributes or HTML import) can be overwritten.
### Issue Context
`line.start` comes from the `start` line attribute in the pad’s attribution stream. The editor renumbering logic sets this attribute intentionally, so export should treat it as authoritative when present.
### Fix Focus Areas
- src/node/utils/ExportHtml.ts[393-402]
- src/node/utils/ExportHtml.ts[414-423]
### Suggested fix approach
- When deciding the `<ol ...>` opener:
- If `line.start` is present and is a finite number, emit that value.
- Else fall back to the continuation counter.
- Ensure the counter is seeded/aligned when `line.start` is used (so later continuations at that level don’t assume the list started at 1).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. No per-level counter reset ☑ 🐞 Bug ≡ Correctness

Description

olItemCounts is only cleared when leaving all lists, not when a list level is closed (listLevel
decreases). This can incorrectly continue numbering when re-entering the same indentation level
later (common with nested ordered lists).

Code

src/node/utils/ExportHtml.ts[R469-472]

+      // outside any list — reset ordered-list counters for all levels
+      for (const key of Object.keys(olItemCounts)) {
+        delete olItemCounts[key];
+      }

Evidence

The PR increments counts per listLevel but only resets them in the non-list branch, so counts for
deeper levels persist even after that level’s list is closed. The editor’s list renumbering logic
uses a fresh position = 1 per nested level invocation, indicating numbering should restart when
you leave and later re-enter a nested level.

src/node/utils/ExportHtml.ts[414-423]
src/node/utils/ExportHtml.ts[441-466]
src/node/utils/ExportHtml.ts[468-472]
src/static/js/ace2_inner.ts[2302-2341]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `olItemCounts` map is cleared only when `line.listLevel` becomes falsy (outside any list). When the exporter closes list levels due to indentation decreasing, counters for the closed levels remain and can incorrectly affect later `<ol start="...">` values.
### Issue Context
Nested lists commonly decrease `listLevel` back to a parent and later re-enter the same nested level under a different parent item. Counters for that nested level should not carry across these separate nested list instances unless the document’s stored `start` attributes explicitly indicate continuation.
### Fix Focus Areas
- src/node/utils/ExportHtml.ts[441-466]
- src/node/utils/ExportHtml.ts[468-472]
### Suggested fix approach
- When closing list elements (the branch that emits `</ol>` / `</ul>` when `nextLine.listLevel < line.listLevel` or list type changes):
- Delete `olItemCounts[level]` entries for any ordered-list levels being closed.
- Keep counts only for still-open levels.
- Keep the existing full reset when leaving all lists as a safety net.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Brittle start assertions ☑ 🐞 Bug ⚙ Maintainability

Description

The new tests use html.includes('start="N"'), which is less specific than the previous regex-based
checks and can accidentally pass if an unrelated list segment contains the same start value. This
weakens regression protection as list export grows more complex (e.g., nested lists).

Code

src/tests/backend/specs/export_list.ts[R50-53]

+    // The second ol should have a start value of 2, showing the numbering continues
  // after the bullet interruption (not reset to 1)
-    const startMatches = html.match(/start="(\d+)"/g) || [];
-    assert(startMatches.length >= 2,
-        `Expected at least 2 ol start attributes in: ${html}`);
-    // Verify at least one start value is > 1
-    const hasHighStart = startMatches.some((m: string) => parseInt(m.match(/\d+/)![0]) > 1);
-    assert(hasHighStart,
-        `Expected a start value > 1 for continued numbering in: ${html}`);
+    assert(html.includes('start="2"'),
+        `Expected start="2" for continued numbering in: ${html}`);

Evidence

The assertions now check for substring presence in the full HTML output, rather than validating the
start attributes on the specific `` segments under test; this reduces the test’s ability to detect
mis-numbering in the intended segment.

src/tests/backend/specs/export_list.ts[36-104]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The tests match `start="2"` / `start="3"` anywhere in the HTML, which is not tied to the specific ordered-list segment that should continue numbering.
### Issue Context
As export output evolves (nested lists, multiple ordered lists in one pad), a global substring match can yield false positives.
### Fix Focus Areas
- src/tests/backend/specs/export_list.ts[36-104]
### Suggested fix approach
- Extract ordered-list open tags in order (e.g., regex for `<ol[^>]*class="number"[^>]*>`), then assert on the *second* segment’s attributes.
- Or parse the HTML with jsdom and query for `ol.number` nodes, asserting the `start` attribute on the expected index.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more