|
| 1 | +# Analysis — Issue #1891 |
| 2 | + |
| 3 | +## Timeline / sequence of events |
| 4 | + |
| 5 | +1. **2026-06-10 23:17 UTC** — `konard` opens issue #1891 ("We have too much |
| 6 | + duplication in `/queue` display"), labelled `bug`, with a screenshot |
| 7 | + (`assets/queue-screenshot.png`) of the live `/queue` output and concrete |
| 8 | + before/after format examples. No comments follow; the issue body is the entire |
| 9 | + specification. |
| 10 | +2. The issue traces back to the display introduced/extended for the `/queue` |
| 11 | + (`/solve_queue`) command: |
| 12 | + - #1232 added `/solve_queue`, |
| 13 | + - #1267 added per-tool grouping, human-readable durations, and the |
| 14 | + `MAX_DISPLAY_ITEMS_PER_QUEUE` cap with "… and N more", |
| 15 | + - #1837 added the short `/queue` alias, clickable `owner/repo#number` links, and |
| 16 | + the executing-list merge with detached sessions. |
| 17 | + Each step added information; none removed the per-line status word or the |
| 18 | + repeated waiting reason, so the duplication accumulated. |
| 19 | +3. **PR #1892** (this work) addresses every requirement (see `requirements.md`). |
| 20 | + |
| 21 | +## Problem 1 — Duplication in the display |
| 22 | + |
| 23 | +### Root cause |
| 24 | + |
| 25 | +The old `formatDetailedStatus` rendered each item through a single helper that |
| 26 | +embedded the _status word_ and the _waiting reason_ on every line: |
| 27 | + |
| 28 | +``` |
| 29 | + ▶️ owner/repo#n (processing, 2h 14m 16s) |
| 30 | + ⏳ owner/repo#n (waiting, 5m 2s) — <reason> |
| 31 | +``` |
| 32 | + |
| 33 | +Three sources of duplication: |
| 34 | + |
| 35 | +1. **The status word is constant within its list.** Everything in the executing |
| 36 | + list is "processing"; everything in the pending list is "waiting". Printing the |
| 37 | + word on every line conveys zero marginal information. |
| 38 | +2. **The waiting reason is (almost) constant within a tool.** The queue blocks a |
| 39 | + whole tool on the same limit (e.g. "Claude 5 hour session limit is 95%"), so the |
| 40 | + reason is identical for every pending item — yet it was printed once per item. |
| 41 | +3. **Empty queues were still printed** (`*agent* (pending: 0, processing: 0)` …), |
| 42 | + adding four dead lines to almost every message. |
| 43 | + |
| 44 | +### Solution |
| 45 | + |
| 46 | +- Move the status signal **into the duration parenthesis as an emoji**: |
| 47 | + `(▶️ <dur>)` for executing, `(⏳ <dur>)` for pending. The emoji is the marker, so |
| 48 | + the word "processing"/"waiting" disappears. |
| 49 | +- Split the queue into explicit labeled lists (Processing, Pending, Completed, |
| 50 | + Failed) so the reader groups items visually without a per-line status word. |
| 51 | +- Render the tool name as a plain header; counts live only on the individual list |
| 52 | + labels, e.g. `*Pending* (2):`, so `(pending: N, processing: N)` is not repeated |
| 53 | + above the same lists. |
| 54 | +- Print the **shared waiting reason once per tool**, and only when all pending items |
| 55 | + agree on it (`distinctReasons.length === 1`). Divergent reasons suppress the |
| 56 | + shared line rather than print a misleading one. |
| 57 | +- **Skip empty queues** with an early `continue`. |
| 58 | + |
| 59 | +## Problem 2 — Splitting long messages without breaking markdown |
| 60 | + |
| 61 | +### Root cause |
| 62 | + |
| 63 | +Removing the per-queue truncation (R6: "try to fit more data") means a `/queue` |
| 64 | +message can now exceed Telegram's **4096-character** limit. The previous |
| 65 | +`splitTelegramMessageText` split on arbitrary separators (including mid-line), which |
| 66 | +can: |
| 67 | + |
| 68 | +- cut an inline entity in half (e.g. `[label](ur` … `l)`), producing |
| 69 | + `can't parse entities` errors, and |
| 70 | +- split **inside a fenced code block**, leaving one chunk with an unclosed ` ``` ` |
| 71 | + and the next chunk starting in a broken state — exactly the failure the issue |
| 72 | + calls out ("without breaking the markdown (especially code blocks)"). |
| 73 | + |
| 74 | +### Solution |
| 75 | + |
| 76 | +A line-based, fence-aware splitter: |
| 77 | + |
| 78 | +- **Split only on `\n`.** Telegram's legacy-Markdown inline entities (bold, italic, |
| 79 | + code span, link) cannot span a newline, so a line boundary is always a safe place |
| 80 | + to cut an inline entity — there are none crossing it. |
| 81 | +- **Track fenced code blocks.** A regex (`/^(\s*)(```+|~~~+)(.*)$/`) recognises a |
| 82 | + fence line and captures its indentation, marker, and language/info string. A |
| 83 | + running `openFence` toggles as fences are seen. |
| 84 | +- **Close + reopen across a split.** When a chunk is flushed mid-code-block, the |
| 85 | + splitter appends a closing fence (same indent + marker) to the current chunk and |
| 86 | + seeds the next chunk with a reopening fence that **repeats the language** — so each |
| 87 | + chunk is independently valid Markdown and the code block renders continuously. |
| 88 | +- **Reserve headroom** (`FENCE_HEADROOM`) so adding a close/reopen pair never pushes |
| 89 | + a chunk past the limit, and **hard-split** any single physical line that alone |
| 90 | + exceeds the budget (pathological input) using the existing |
| 91 | + `findTelegramSplitIndex`. |
| 92 | +- **Preserve the marker kind and indentation** — a `~~~python` block reopens as |
| 93 | + `~~~python`, an indented block keeps its indent — so we never silently rewrite the |
| 94 | + author's fence style. |
| 95 | + |
| 96 | +This lives in the one universal splitter (`src/telegram-safe-reply.lib.mjs`) that |
| 97 | +**every** Telegram send path already funnels through, satisfying R11 ("perfect |
| 98 | +across the codebase") without touching each call site. |
| 99 | + |
| 100 | +## Codebase-wide audit (R18) |
| 101 | + |
| 102 | +- **Send paths** — `safeReply`, the wrapped `telegram.sendMessage`, and the wrapped |
| 103 | + `telegram.editMessageText` all call `splitTelegramMessageText`. Fixing the splitter |
| 104 | + fixes every outbound message, including edits (which also forward overflow chunks |
| 105 | + via `sendFollowUpChunk`). |
| 106 | +- **Queue formatters** — `formatDetailedStatus` (the `/queue` detail) is the only |
| 107 | + formatter that listed items with a status word + reason; it is fixed. |
| 108 | + `formatStatus` (one line per tool, used by `/limits`) never listed items, so it has |
| 109 | + no duplication and is intentionally untouched. |
| 110 | +- **History sections** — `formatQueueHistorySection` (Completed/Failed) keeps its cap |
| 111 | + by design (see `requirements.md`, "out of scope"). |
| 112 | + |
| 113 | +## Scope: other repositories (R17) |
| 114 | + |
| 115 | +The bug is entirely internal: it is in this repo's Telegram rendering |
| 116 | +(`telegram-solve-queue*.mjs`) and message-splitting (`telegram-safe-reply.lib.mjs`) |
| 117 | +code. The `owner/repo#number` strings in the screenshot (e.g. |
| 118 | +`uselessgoddess/ryzr`, `link-foundation/box`) are just _queued work items_, not the |
| 119 | +source of the bug. There is therefore **no upstream/third-party repository to file a |
| 120 | +report against**. Prior art in external projects is catalogued in |
| 121 | +`existing-components.md` for design validation, not as a defect to report. |
| 122 | + |
| 123 | +## Risk / trade-off notes |
| 124 | + |
| 125 | +- **Listing all items** can make a message long; mitigated by the markdown-safe |
| 126 | + splitter. Worst case is several messages instead of a truncated one — the issue |
| 127 | + explicitly accepts this ("it will be fine if it does not fit in one message"). |
| 128 | +- **Shared-reason heuristic**: if two pending items genuinely have different reasons |
| 129 | + we show none (rather than a wrong single one). The per-item status is still implied |
| 130 | + by the ⏳ marker; the detailed reason remains available in each item's own status |
| 131 | + message. This matches the issue's "it usually [is] all the same". |
| 132 | +- **Backward compatibility**: `formatQueueProcessingItems` is kept as a deprecated |
| 133 | + alias of `formatQueueExecutingItems` so any external caller keeps working. |
0 commit comments