Skip to content

fix(gateway): insert dedup repeat counter inside the code fence - #90578

Open
fuxin123z wants to merge 1 commit into
NousResearch:mainfrom
fuxin123z:fix/dedup-counter-breaks-code-fence
Open

fuxin123z wants to merge 1 commit into
NousResearch:mainfrom
fuxin123z:fix/dedup-counter-breaks-code-fence

Conversation

@fuxin123z

Copy link
Copy Markdown

Problem

Consecutive identical terminal tool previews trigger the progress dedup path. On markdown platforms the preview line is a fenced code block, and the dedup handler appended the repeat counter after the closing fence:

💻 terminal
```bash
python3 << 'EOF' ...
``` (×2)   ← counter glued to the close fence

The line ``` (×2) no longer matches the close-fence pattern (`^\s*$`), so the block never closes — markdown renderers (observed on Feishu) display it as an empty block / "0 lines of code".

Reproducer: any repeated multi-line command whose previews are identical — e.g. two heredoc scripts sharing the first line python3 << 'EOF' under tool_progress: all (default preview truncation to the first line makes identical previews very likely).

Fix

New _append_dedup_counter() used by both dedup sites:

  • fenced lines → counter inserted inside the fence as a trailing (×N) marker line; an existing counter line is replaced so repeats don't stack
  • non-fenced lines → old suffix behavior, with stale counters stripped first

Verification

Unit-style checks against _build_markdown_post_rows (Feishu post builder): all four cases keep fences balanced —

case result
first dedup on fenced block bash\npython3 << 'EOF' ...\n(×2)\n ✅
repeated dedup ×2→×3 counter replaced, no stacking ✅
non-fenced line suffix preserved ✅
repeated non-fenced stale counter stripped ✅

Live-verified on Feishu: 5 consecutive heredoc commands with identical first lines — previously rendered "0 lines of code", now render correctly with the counter inside the block.

Consecutive identical terminal previews (e.g. repeated multi-line
commands sharing the same first line, like heredoc scripts) trigger the
progress dedup path, which appended ' (xN)' directly after the closing
fence: '...\n``` (x2)'. That line no longer matches the close-fence
pattern (^\`\`\`\s*$), so the block never closes and markdown
renderers (Feishu et al) display an empty / '0 lines of code' block.

Move the counter inside the fence as a trailing marker line, replacing
any previous counter so repeats don't stack. Non-fenced lines keep the
old suffix behavior (with stale counters stripped first).
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/feishu Feishu / Lark adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 20, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review; please use your judgment.

  1. gateway/run.py (~4353–4360, ~4409–4412) — two hunks change tool_preview_max_len semantics (0 previously fell back to 40; now it means unlimited) and carry comments reading "Local patch 2026-08-20 (upstream treated 0 as falsy)" — why it matters: (a) this is unrelated to the PR's stated purpose (dedup counter placement), so reviewers and changelog readers won't find it; (b) it silently flips behavior for operators who already set 0 believing they'd get the default-40 cap; (c) "local patch" narration describes deployment history, not upstream reality — suggestion: split into its own PR, document 0 = unlimited in the config reference, and reword comments to describe behavior rather than patch provenance.

  2. Test gap — _append_dedup_counter is a pure function with three distinct branches (insert-before-close-fence, replace-an-existing-counter-line, non-fenced suffix strip) and zero tests — why it matters: the backwards fence-scanning loop with its insertion index is exactly the kind of logic that regresses silently — suggestion: add four unit cases (fresh fenced, repeat on fenced, repeat on non-fenced, fenced block whose content contains a bare fence line).

  3. Nit (~4125–4131): the in-fence (×N) marker becomes literal content of the rendered code block on clients that honor fences — an accepted trade-off given the Feishu close-fence analysis, but worth one docstring sentence saying so explicitly for future formatting changes.

The core fix itself is sound: appending (×N) outside a closing fence genuinely breaks the ^```\s*$ match on Feishu, and scanning backwards for the last bare fence while replacing any prior counter prevents unbounded stacking.

— reviewer-a · automated agent review (Hermes week-review)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/feishu Feishu / Lark adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants