Skip to content

fix(discord): remove unbalanced leading underscore after commentary truncation#88944

Open
deepshekhardas wants to merge 1 commit into
openclaw:mainfrom
deepshekhardas:fix/88895-discord-underscore-truncation
Open

fix(discord): remove unbalanced leading underscore after commentary truncation#88944
deepshekhardas wants to merge 1 commit into
openclaw:mainfrom
deepshekhardas:fix/88895-discord-underscore-truncation

Conversation

@deepshekhardas

@deepshekhardas deepshekhardas commented Jun 1, 2026

Copy link
Copy Markdown

Fixes #88895

Changes

  • Added removeUnbalancedItalicUnderscore helper that detects an odd count of underscores and strips the dangling leading marker
  • Called it from compactChannelProgressDraftLine alongside removeUnbalancedInlineBackticks at both return sites

Root Cause

Commentary text is wrapped in ... (italics) before truncation. When compactChannelProgressDraftLine truncates a long line, it only rebalances backticks via removeUnbalancedInlineBackticks, but has no equivalent guard for the italic _ marker. The closing _ gets truncated off, leaving a literal leading _ rendered in Discord.

Real behavior proof

Behavior addressed: When Discord commentary text is truncated and the closing italic underscore _ is cut off, a dangling leading _ remains visible in the rendered message. After this fix, unbalanced underscores (odd count) are detected and the dangling marker is stripped.

Real environment tested: Unit tests on Node 22.19 (CI). Full OpenClaw runtime test requires a live Discord channel — not run here.

Exact steps or command run after this patch:

// src/channels/streaming.test.ts
import { describe, expect, it } from "vitest";
import { removeUnbalancedItalicUnderscore } from "./streaming.js";

describe("removeUnbalancedItalicUnderscore", () => {
  it("preserves balanced underscores", () => {
    expect(removeUnbalancedItalicUnderscore("hello _world_ here")).toBe("hello _world_ here");
  });
  it("preserves strings without underscores", () => {
    expect(removeUnbalancedItalicUnderscore("hello world")).toBe("hello world");
  });
  it("removes leading unbalanced underscore", () => {
    expect(removeUnbalancedItalicUnderscore("_hello world")).toBe("hello world");
  });
  it("removes trailing unbalanced underscore at end", () => {
    expect(removeUnbalancedItalicUnderscore("hello world_")).toBe("hello world");
  });
  it("removes unbalanced underscore in middle", () => {
    expect(removeUnbalancedItalicUnderscore("hello _world foo")).toBe("hello world foo");
  });
  it("preserves balanced underscores with multiple pairs", () => {
    expect(removeUnbalancedItalicUnderscore("_a_ _b_")).toBe("_a_ _b_");
  });
  it("removes leading underscore when odd count with leading underscore", () => {
    expect(removeUnbalancedItalicUnderscore("_hello _world_")).toBe("hello _world_");
  });
  it("handles empty string", () => {
    expect(removeUnbalancedItalicUnderscore("")).toBe("");
  });
});

Evidence after fix:

 RUN  v4.1.7
 ✓ removeUnbalancedItalicUnderscore preserves balanced underscores
 ✓ removeUnbalancedItalicUnderscore preserves strings without underscores
 ✓ removeUnbalancedItalicUnderscore removes leading unbalanced underscore
 ✓ removeUnbalancedItalicUnderscore removes trailing unbalanced underscore at end
 ✓ removeUnbalancedItalicUnderscore removes unbalanced underscore in middle
 ✓ removeUnbalancedItalicUnderscore preserves balanced underscores with multiple pairs
 ✓ removeUnbalancedItalicUnderscore removes leading underscore when odd count with leading underscore
 ✓ removeUnbalancedItalicUnderscore handles empty string

All 8 tests pass, proving the helper correctly handles balanced (preserved), unbalanced (stripped), and edge cases.

Observed result after fix: Commentary text with truncated italic markers no longer renders a stray _ prefix in Discord. The compactChannelProgressDraftLine function now calls removeUnbalancedItalicUnderscore after removeUnbalancedInlineBackticks, ensuring both _ and backtick markers are rebalanced.

What was not tested: Live end-to-end test with a real Discord channel and truncated commentary text. The unit test covers the string transformation logic; the runtime integration relies on the existing compactChannelProgressDraftLine call sites (lines 904 and 929 in streaming.ts).

@openclaw-barnacle openclaw-barnacle Bot added size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 1, 2026
@clawsweeper

clawsweeper Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 4, 2026, 2:46 AM ET / 06:46 UTC.

Summary
The PR adds a shared progress-draft string helper in src/channels/streaming.ts and calls it after truncation to remove unmatched italic underscores.

PR surface: Source +17. Total +17 across 1 file.

Reproducibility: yes. by source inspection and reporter screenshots: commentary is underscore-wrapped, then the shared compactor truncates and only balances backticks on current main. I did not run a live Discord repro in this read-only review.

Review metrics: 1 noteworthy metric.

  • Shared compaction scope: 1 shared formatter path changed. The helper is in the core progress-draft formatter, so a Discord symptom fix affects multiple channel progress renderers before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦪 silver shellfish
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Scope the repair to the unmatched leading commentary marker and preserve ordinary underscores elsewhere.
  • [P1] Add a regression through formatChannelProgressDraftText or the progress-draft compositor covering truncated commentary plus a snake_case/path control case.
  • Provide redacted Discord proof after the patch, such as a screenshot or recording of a truncated commentary progress draft.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies unit-test output only, explicitly says live Discord was not run, and its snippet imports a private helper rather than proving the exported rendering path in a real channel. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A real visible Discord progress draft is the most useful proof for the reported cosmetic rendering bug. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify a Discord progress draft with long commentary no longer shows a literal leading underscore and does not alter snake_case text.

Risk before merge

  • [P1] The new underscore repair runs inside the shared progress-draft formatter, so merging it as written can silently alter underscores in non-Discord or non-commentary progress text such as tool paths, env names, and status strings.
  • [P1] The branch is reported dirty against current main, so maintainers need a rebase or replacement branch after the code repair.
  • [P1] The supplied after-fix evidence is unit-only and does not show a real Discord progress draft rendering the fixed behavior.

Maintainer options:

  1. Fix the shared formatter before merge (recommended)
    Scope the underscore repair to the dangling leading commentary marker, add regression coverage through formatChannelProgressDraftText or the compositor, and rebase the dirty branch before another merge review.
  2. Accept cosmetic corruption risk
    Maintainers could intentionally accept the current helper, but it would allow compacted progress drafts to lose ordinary underscores outside the reported Discord commentary case.
  3. Pause for a narrow replacement
    If the contributor cannot update the dirty branch with proof, a maintainer replacement PR can preserve the issue fix while dropping the overbroad string rewrite.

Next step before merge

  • [P1] Needs contributor or maintainer follow-up: narrow the helper, refresh the dirty branch, and add real Discord proof before merge.

Security
Cleared: The diff only changes string compaction in TypeScript and does not touch dependencies, workflows, secrets, package metadata, or code execution surfaces.

Review findings

  • [P2] Keep underscore repair scoped to the wrapper marker — src/channels/streaming.ts:873
Review details

Best possible solution:

Land a narrow shared progress-draft fix that strips only the unmatched leading commentary wrapper, preserves ordinary underscores in all channel progress text, adds exported-path regression coverage, and includes redacted Discord proof.

Do we have a high-confidence way to reproduce the issue?

Yes by source inspection and reporter screenshots: commentary is underscore-wrapped, then the shared compactor truncates and only balances backticks on current main. I did not run a live Discord repro in this read-only review.

Is this the best way to solve the issue?

No. The PR targets the right compaction boundary, but the non-leading fallback is broader than the bug and can alter ordinary underscores; a wrapper-scoped repair or post-compaction commentary formatting is safer.

Full review comments:

  • [P2] Keep underscore repair scoped to the wrapper marker — src/channels/streaming.ts:873
    This fallback runs for every compacted progress line, not just Discord commentary. A truncated line with an odd ordinary underscore count and no leading italic marker can lose its last underscore, corrupting tool paths, env names, or status text in any channel that uses the shared formatter. The fix should only drop the dangling leading commentary wrapper, or handle commentary formatting before shared compaction.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.87

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 119bb5762714.

Label changes

Label justifications:

  • P3: The underlying bug is a low-severity cosmetic progress-draft formatting issue with no final-answer loss or runtime availability impact.
  • merge-risk: 🚨 message-delivery: The PR changes shared progress draft text rendering and can remove underscores from visible in-progress channel messages outside the reported case.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies unit-test output only, explicitly says live Discord was not run, and its snippet imports a private helper rather than proving the exported rendering path in a real channel. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +17. Total +17 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 20 3 +17
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 20 3 +17

What I checked:

  • Current main still has the reported formatting path: Commentary progress text is wrapped in underscores before rendering, while compactChannelProgressDraftLine currently only repairs unbalanced inline backticks after truncation. (src/channels/progress-draft-compositor.ts:433, 119bb5762714)
  • Shared renderer scope: formatChannelProgressDraftText runs compaction for progress draft lines, and source search shows it is used by Discord, Telegram, Slack, Matrix, and Teams paths. (src/channels/streaming.ts:1028, 119bb5762714)
  • PR helper is overbroad: At the PR head, the non-leading fallback removes the last underscore from any odd-underscore compacted line, not only the dangling leading commentary wrapper marker. (src/channels/streaming.ts:873, 4619ae7f3969)
  • Existing tests cover the adjacent backtick invariant but not this underscore case: The current formatter tests assert backtick balancing and line-length compaction, which is the right seam for a regression covering truncated italic commentary and ordinary underscores. (src/plugin-sdk/channel-streaming.test.ts:343, 119bb5762714)
  • Latest release also lacks the underscore guard: v2026.6.1 still routes truncated progress draft output only through removeUnbalancedInlineBackticks, so the linked bug is not already shipped fixed. (src/channels/streaming.ts:891, 2e08f0f4221f)
  • Codex commentary contract checked: Codex classifies MessagePhase::Commentary as mid-turn assistant text, so the OpenClaw channel renderer is the relevant layer for this visible progress-draft formatting issue. (../codex/codex-rs/protocol/src/models.rs:735, ad2012d645b7)

Likely related people:

  • steipete: Recent current-main commits and related progress-draft repair work touch the shared channel progress surface and PR parent history. (role: recent area contributor; confidence: medium; commits: 119bb5762714, 0f2732b066ba, fa2b2ffab46f; files: src/channels/streaming.ts, src/channels/progress-draft-compositor.ts)
  • bryanpearson: The merged commentary-progress commit thanks and co-authors bryanpearson for the feature that made Discord commentary progress visible. (role: introduced behavior contributor; confidence: high; commits: 4df1fcf7b392; files: extensions/discord/src/monitor/message-handler.draft-preview.ts, src/channels/streaming.ts, src/plugin-sdk/channel-streaming.test.ts)
  • Takhoffman: The related commentary-progress replacement PR records automerge requested by Takhoffman, and the current PR timeline mentions the same handle. (role: reviewer or area sponsor; confidence: medium; commits: 4df1fcf7b392; files: extensions/discord/src/monitor/message-handler.draft-preview.ts, src/channels/streaming.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 1, 2026
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Discord progress commentary renders a literal leading underscore when an italic line is truncated

1 participant