Skip to content

Nested emphasis stalls the listening loop: _strip_emphasis is quadratic and runs before the cap #77

Description

@jamditis

Split out of the review on #75, where it was raised against _strip_emphasis. The finding is real and reproduces, but the obvious fix is wrong in a way worth writing down, so it gets its own issue rather than a rushed patch on that branch.

The stall

sanitize_reply() applies max_chars to the finished text, so it bounds the output and not the work. _strip_emphasis loops pattern.sub() until the text stops changing; each pass drops one nesting level and rescans the whole reply, so cost grows as the square of the delimiter run. Measured on the Pi 5 this runs on:

input time
4,000 delimiters 0.385s
8,000 delimiters 1.533s
16,000 delimiters 6.137s
40,000 delimiters 38.95s

Cleanly quadratic: 4x the input is 16x the time. The trigger is "*" * n + "x" + "*" * n — a model looping on one character, which is the same failure mode _CONTAINER_RE (">" * 8000) and _iter_code_spans (tick runs) were each already fixed for. This is an always-listening loop, so a reply that takes 39 seconds to sanitize is a loop that has stopped answering.

Why the obvious fix is wrong

Bounding the raw input before _strip_markdown looks right, matches the PR title, and bounds every pass at once rather than only the quadratic one. It breaks two of this file's own tests:

  • test_a_wall_of_quote_markers_does_not_stall_the_loop">" * 20000 + " hello" asserts "hello" still speaks.
  • test_many_unclosed_runs_do_not_stall_the_loop — ~80KB of tick runs asserts the " After." after them still speaks.

Both pin the same intent: a pathological wall must not destroy the real content behind it. Any raw ceiling low enough to bound the quadratic (~6k, for a worst case near 0.9s) truncates that content away and the reply degrades to the empty fallback. The ceiling and the tests cannot both be satisfied.

That intent is consistent with how every prior stall here was fixed: make the algorithm linear, keep the content. Bounding the input is the one approach this file has already rejected three times.

Other approaches considered

  • Cap the pass count (real nesting depth is <= 3, so ~8 passes is generous). Linear, no truncation, but leftover paired delimiters stay literal, so "*" * 20000 + "x" + ... speaks 600 asterisks instead of "x". Trades a stall for garbage.
  • Anchor the delimiter run ((?<![\w*])(\*{1,3})...(?![\w*]), the (?!\2) trick _FENCED_BLOCK_RE already uses) so a run of 4+ is not a delimiter. Kills the "****" shape in one pass and is closer to what the docstring already claims ("a matched pair of identical delimiter runs"). Does not kill the class: "*a " * 3000 + "x" + " a*" * 3000 nests with spaces and is still quadratic. Also changes ****x**** from x to literal asterisks.

What the fix probably is

A single-pass delimiter scanner — index the runs, pair them with a stack, resolve nesting in one walk — which is the shape _iter_code_spans already uses for ticks ("runs are indexed by length up front, and each length keeps a cursor that only moves forward"). CommonMark's process-emphasis algorithm is the reference. That is a real rewrite of the emphasis rules with 134 checks pinning current behavior, so it wants its own PR and its own review.

Not blocking #75

#75 leaves this exactly as it found it. The branch fixes the three link findings and adds a linear bracket pairing (which incidentally removed a pre-existing quadratic on "[" * 8000 + "](url)": 0.44s -> 0.007s). This one is older than that branch and is not a regression in it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions