fix: consecutive numbering fails after indented sub-bullets

[JohnMcLear]

Summary

One-character fix in ace2_inner.ts: listType[0] → listType[1] in the applyNumberList() function inside renumberList().

Root Cause

After listType = /([a-z]+)([0-9]+)/.exec(listType), the array indices are:

• listType[0] = full match (e.g., "indent1") — never just "indent"
• listType[1] = capture group 1 (e.g., "indent", "number", "bullet")
• listType[2] = capture group 2 (e.g., "1", "2")

The check listType[0] === 'indent' was always false, so indent-type lines were never recognized during renumbering. This broke the numbering sequence when numbered lists followed indented content — all items showed "1" instead of incrementing.

Note: the same function's parent renumberList() correctly uses type[1] === 'indent' at lines 2290 and 2297 — this was just a typo in the inner applyNumberList() helper.

Test plan

• Backend tests pass (754/754)
• New Playwright test: numbered list after indented sub-bullets shows consecutive numbers (chromium pass)
• All existing ordered_list tests pass on chromium

Fixes #5718

🤖 Generated with Claude Code

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

Review Summary by Qodo

Fix consecutive numbering after indented sub-bullets

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Fix array index bug in applyNumberList() function
  - Changed listType[0] to listType[1] to correctly identify indent-type lines
  - After regex exec, listType[0] is full match, listType[1] is capture group
• Add regression test for consecutive numbering after indented sub-bullets
  - Tests switching from bullet to numbered list after indented content
  - Verifies numbered items show correct consecutive numbers (1, 2, 3)

Diagram

flowchart LR
  A["Regex exec result<br/>listType[0]=full match<br/>listType[1]=type name"] -- "Bug: checked listType[0]" --> B["Indent never recognized"]
  B -- "Broken numbering<br/>all show 1" --> C["Issue #5718"]
  A -- "Fix: check listType[1]" --> D["Indent correctly identified"]
  D -- "Consecutive numbers<br/>1, 2, 3..." --> E["Fixed behavior"]

Loading

File Changes

1. src/static/js/ace2_inner.ts 🐞 Bug fix +1/-1

Fix indent type detection in renumbering logic

• Fixed array index in applyNumberList() from listType[0] to listType[1]
• Corrects indent-type detection after regex capture group extraction
• Aligns with parent function's correct usage at lines 2290 and 2297

src/static/js/ace2_inner.ts

---

2. src/tests/frontend-new/specs/ordered_list.spec.ts 🧪 Tests +42/-0

Add regression test for consecutive numbering

• Add regression test for issue #5718
• Tests numbered list after indented sub-bullets scenario
• Verifies consecutive numbering (1, 2, 3) displays correctly
• Uses Playwright to interact with UI and validate start attributes

src/tests/frontend-new/specs/ordered_list.spec.ts

---

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

Code Review by Qodo

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

1. Indent precondition unverified ☑ 🐞 Bug ≡ Correctness

Description

The new regression test presses Tab/Shift+Tab but never asserts that the line actually became an
indented sub-bullet, so the test can pass even if indentation keystrokes are ignored and thus not
exercise the reported bug scenario. This reduces the test’s effectiveness at preventing regressions
for #5718.

Code

src/tests/frontend-new/specs/ordered_list.spec.ts[R106-113]

+    // Indent to create a sub-bullet
+    await page.keyboard.press('Tab');
+    await writeToPad(page, 'Sub-bullet');
+    await page.keyboard.press('Enter');
+
+    // De-indent back to level 1
+    await page.keyboard.press('Shift+Tab');
+

Evidence

The new test performs the indentation actions (Tab / Shift+Tab) but has no assertion that the
intermediate state is actually indented. Existing UL tests assert indentation by checking for the
.list-bullet2 class after pressing Tab, which provides a concrete pattern to follow.

src/tests/frontend-new/specs/ordered_list.spec.ts[95-113]
src/tests/frontend-new/specs/unordered_list.spec.ts[77-98]

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 regression test for #5718 does not verify that the Tab keypress actually created an indented sub-bullet, so the test may pass without reproducing the failing precondition.
### Issue Context
Other list tests in this suite validate indentation by asserting presence of `.list-bullet2` after Tab.
### Fix Focus Areas
- src/tests/frontend-new/specs/ordered_list.spec.ts[106-113]
- src/tests/frontend-new/specs/unordered_list.spec.ts[77-98]
### Suggested change
After the Tab + text entry (and/or after the subsequent Enter), add an assertion such as:
- `await expect(padBody.locator('div').nth(1).locator('.list-bullet2')).toHaveCount(1);`
(If the actual DOM uses `list-indent2` for this scenario, use a combined selector like `.list-bullet2, .list-indent2`.)

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

---

2. Hard-coded sleep in test ☑ 🐞 Bug ☼ Reliability

Description

The new regression test adds waitForTimeout(500), which slows the suite and can still be flaky on
slower CI nodes; the subsequent toHaveAttribute(..., {timeout: 5000}) expectations already provide
waiting. This sleep is therefore likely redundant and should be removed or replaced with a
condition-based wait.

Code

src/tests/frontend-new/specs/ordered_list.spec.ts[R123-124]

+    // Wait for renumbering
+    await page.waitForTimeout(500);

Evidence

The test includes a fixed 500ms delay immediately before assertions that already wait up to 5000ms
for the expected start attribute values.

src/tests/frontend-new/specs/ordered_list.spec.ts[123-134]

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 test uses a fixed `waitForTimeout(500)` before assertions that already auto-wait, adding unnecessary latency and potential flakiness.
### Issue Context
`expect(locator).toHaveAttribute(..., {timeout: 5000})` already retries until the attribute matches or times out.
### Fix Focus Areas
- src/tests/frontend-new/specs/ordered_list.spec.ts[123-134]
### Suggested change
Remove the `await page.waitForTimeout(500);` line and rely on the existing `toHaveAttribute` assertions.
(If an explicit wait is still needed, replace with a condition-based wait, e.g., wait for the first OL to have `start='1'` rather than sleeping a fixed duration.)

ⓘ 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