Skip to content

fix(normalize): strip full slash-command envelope + ANSI escapes (#1333) - #1909

Open
KeilerHirsch wants to merge 1 commit into
MemPalace:developfrom
KeilerHirsch:fix/1333-claude-code-envelope-ansi
Open

fix(normalize): strip full slash-command envelope + ANSI escapes (#1333)#1909
KeilerHirsch wants to merge 1 commit into
MemPalace:developfrom
KeilerHirsch:fix/1333-claude-code-envelope-ansi

Conversation

@KeilerHirsch

Copy link
Copy Markdown
Contributor

Summary

strip_noise() in mempalace/normalize.py only stripped three of the tags Claude Code injects (<system-reminder>, <command-message>, <command-name>). Real transcripts also carry the rest of the slash-command envelope plus raw ANSI escapes from Bash-tool output, all of which leaked into drawers verbatim — bloating embeddings (each escape is several BPE tokens) and polluting search. This is #1333.

Closes #1333.

What changed

  • Four missing envelope tags added to _NOISE_TAGS: command-args, local-command-caveat, local-command-stdout, local-command-stderr.
  • Tag-body regex now halts at the next opening tag of the same name instead of the first blank line, so a multi-paragraph <local-command-stdout> block (tables, stack traces) is removed whole. The line-start anchor still prevents a dangling tag from eating neighbouring messages.
  • ANSI stripping (CSI + OSC), anchored on the literal ESC byte (0x1b) — a control char that never appears in legitimate prose — so text that merely names a sequence like ESC[0m survives. ReDoS-safe by construction (disjoint/negated classes, no nested overlapping quantifiers).
  • NORMALIZE_VERSION 2 → 3 so already-mined drawers rebuild on the next pass (convo mining gates on the version, not mtime).

Relationship to prior PRs

This supersedes the two stale attempts and folds in the review feedback each got:

The multi-paragraph-halt regex approach matches #1701's, credited in spirit.

Tests

12 new cases in tests/test_normalize.py (TestStripNoiseClaudeCodeEnvelopeAndAnsi + the per-tag loop): multi-paragraph stdout, stderr, empty command-args remnant, caveat, ANSI CSI SGR, ANSI OSC hyperlink, and a prose-naming-survives verbatim check.

  • pytest tests/test_normalize.py — 159 passed
  • pytest tests/test_convo_miner.py tests/test_miner.py — 134 passed, 3 skipped
  • ruff check mempalace/normalize.py mempalace/palace.py tests/test_normalize.py — clean

…Palace#1333)

strip_noise() covered only <system-reminder>/<command-message>/<command-name>.
Real Claude Code transcripts also inject <local-command-caveat>, <command-args>,
<local-command-stdout> and <local-command-stderr>, plus raw ANSI escape
sequences from Bash-tool output. These leaked into drawers verbatim, bloating
embeddings (each escape is several BPE tokens) and polluting search.

- Add the four missing envelope tags to _NOISE_TAGS. local-command-stderr was
  not in the MemPalace#1333 root-cause sketch but is present in real transcripts.
- Change the tag body regex to halt at the next same-tag opener instead of the
  first blank line, so multi-paragraph <local-command-stdout> output is fully
  removed while a dangling tag still cannot span messages.
- Strip ANSI CSI + OSC sequences, anchored on the ESC byte so prose that merely
  names a sequence survives (verbatim is sacred).
- Bump NORMALIZE_VERSION 2 -> 3 so already-mined drawers rebuild on next pass.

Supersedes the stale/conflicting MemPalace#1701 and MemPalace#1334; adds the stderr tag and the
NORMALIZE_VERSION bump reviewers flagged as required on MemPalace#1334.

Tests: 12 new cases in test_normalize.py (multi-paragraph stdout, stderr,
empty command-args, caveat, ANSI CSI/OSC, prose-naming survival).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the noise-stripping logic in mempalace to remove additional Claude Code slash-command envelope tags and ANSI escape sequences from terminal output, bumping the normalization version to 3 to trigger a rebuild. It also refactors the tag-matching regex to support multi-paragraph content within tags. The review feedback highlights a potential issue in the updated regex where using a word boundary (\b) could incorrectly match tag prefixes (e.g., matching <command-args> inside <command-args-extended>), and suggests a more robust lookahead pattern using [\s>] instead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread mempalace/normalize.py
# trailing whitespace + newline.
return re.compile(
rf"(?m)^(?:> )?<{name}(?:\s[^>]*)?>" rf"(?:(?!\n\s*\n)[\s\S])*?" rf"</{name}>[ \t]*\n?"
rf"(?m)^(?:> )?<{name}(?:\s[^>]*)?>" rf"(?:(?!<{name}\b)[\s\S])*?" rf"</{name}>[ \t]*\n?"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using \b (word boundary) in (?!<{name}\b) can lead to unexpected behavior if there are other tags or user text starting with {name} followed by a hyphen (e.g., <command-args-extended>). Since - is a non-word character, the transition from the last letter of {name} to - is considered a word boundary, causing the lookahead to match and halt prematurely.

To make this lookahead more robust and prevent it from matching prefixes of longer tag names, we can explicitly check for the characters that can actually follow a tag name in a valid opening tag: whitespace or >.

Suggested change
rf"(?m)^(?:> )?<{name}(?:\s[^>]*)?>" rf"(?:(?!<{name}\b)[\s\S])*?" rf"</{name}>[ \t]*\n?"
rf"(?m)^(?:> )?<{name}(?:\s[^>]*)?>" rf"(?:(?!<{name}[\s>])[\s\S])*?" rf"</{name}>[ \t]*\n?"

@jrzmurray

Copy link
Copy Markdown
Contributor

Found a truncation-safety edge case in _ANSI_CSI_RE while adapting this for local use — flagging it since it'd affect this PR as submitted, though it's unlikely to come up often in real data.

_ANSI_CSI_RE = re.compile(r"\x1b\[[\x30-\x3f]*[\x20-\x2f]*[\x40-\x7e]")

Both middle groups are zero-or-more, and the final-byte class (0x40-0x7E) covers nearly every ASCII letter. That means a genuinely truncated ESC[ (no parameters ever written, e.g. because the writer got cut off mid-stream) immediately followed by ordinary real text is indistinguishable from a valid empty-parameter CSI sequence — the regex happily treats the next real letter as the "final byte" and eats it.

Confirmed empirically:

>>> _ANSI_CSI_RE.sub("", "before \x1b[this has words after")
'before his has words after'   # the leading "t" of "this" is gone

This needs a genuinely truncated escape sequence to land directly against real prose with zero digits in between, which is a narrow window — real Bash tool output being truncated mid-write does happen (a backgrounded process's redirected stdout racing its reader), but the exact conjunction here is uncommon. Not a blocker, just worth hardening defensively since the fix is small: require at least one parameter byte (+ instead of *) and drop the intermediate-byte class, since real SGR/cursor codes are overwhelmingly just digits and ;:

_ANSI_CSI_RE = re.compile(r"\x1b\[[0-9;]+[\x40-\x7e]")

Tradeoff: a truly bare, zero-param CSI (e.g. ESC[H, cursor-home with no row/col) would no longer be stripped — left as harmless unstripped noise instead. Verified this closes the repro above while every real-world SGR/cursor-with-param shape I tried still strips correctly.

(Also noticed neither this PR nor #1334 strip the "Fe/Fs" single-byte escape sequences — e.g. ESC c, RIS/full-reset, which watch-mode dev tools like tsx watch/nodemon emit before reprinting on a file-change restart. Only CSI/OSC are covered. Separate, smaller gap if useful — happy to share that pattern too if it'd help.)

gnusam pushed a commit to gnusam/mempalace-pgsql that referenced this pull request Jul 29, 2026
Introduces the strip_noise layer this fork never carried, reconciling
three overlapping upstream PRs into one pass applied at every
normalize() exit:

- the full 10-tag Claude Code envelope (system-reminder, task-
  notification, the six slash-command chrome tags, user-prompt-submit-
  hook, hook_output) and ECMA-48 ANSI CSI/OSC escapes from Bash-tool
  output, per PR MemPalace#1909 (issue MemPalace#1333; PR MemPalace#1958 covers the same ground)
- indent-tolerant line anchors and multi-paragraph tag bodies that halt
  at the next same-tag opening instead of the first blank line, per
  PR MemPalace#2064 — subagent results and recalled memories inside a block are
  stripped whole, while a dangling open tag can never merge with a
  later block and eat the real content between them

Verbatim is sacred: patterns are line-anchored, ESC-byte-anchored, or
narrow chrome shapes; prose that merely names "[1m" survives. Already-
mined drawers keep their chrome until their transcript next changes —
session logs are append-heavy, so re-mines pick this up naturally.

Co-authored-by: KeilerHirsch <KeilerHirsch@users.noreply.github.com>
Co-authored-by: mazurd-acre <mazurd-acre@users.noreply.github.com>
Co-authored-by: jrzmurray <jrzmurray@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jrzmurray added a commit to jrzmurray/mempalace that referenced this pull request Aug 13, 2026
…ion-safe

Local integration of the fix for MemPalace#1333 (strip slash-command envelope
remnants and ANSI escape sequences from Bash-tool output), informed by
PR MemPalace#1909 upstream. Not opened as a competing PR -- MemPalace#1909 is already an
active, close-to-mergeable submission for the same issue; this is for
local use, with two things MemPalace#1909 doesn't have:

1. The tag-boundary fix MemPalace#1909's own review comment flagged: a bare `\b`
   after the tag name incorrectly matches the boundary between "s" and
   "-" inside a longer tag name (e.g. <command-args-extended>), causing
   <command-args>'s lazy body to stop early and misparse unrelated
   content. Fixed with a `[\s>]` lookahead instead.

2. A third ANSI pattern (_ANSI_SIMPLE_RE) for the "Fe minus CSI/OSC" and
   "Fs" escape sequence classes -- single ESC + one byte, no payload.
   Neither MemPalace#1334 nor MemPalace#1909 cover this; both only strip CSI and OSC. The
   motivating case: watch-mode dev tools (tsx watch, nodemon) emit ESC c
   (RIS, full terminal reset) before reprinting output on a file-change
   restart, confirmed by reproducing it directly against the real API
   dev server. A backgrounded process's redirected stdout/stderr can
   carry this into a captured Bash tool_result verbatim.

Also fixes a real truncation-safety bug found during review of the CSI
pattern (both MemPalace#1909's and an earlier draft of this one shared it): with
`[\x30-\x3f]*[\x20-\x2f]*[\x40-\x7e]` (both middle groups zero-or-more),
a genuinely truncated `ESC[` immediately followed by ordinary real text
is indistinguishable from a valid empty-parameter CSI sequence, since
the final-byte class (0x40-0x7E) covers nearly every letter. Confirmed
empirically: `"before \x1b[this has words after"` silently became
`"before his has words after"` -- the leading "t" of "this" eaten as a
false CSI terminator. Real captured tool output does get truncated
mid-write (a background process's redirected stdout racing its
reader), so this isn't a contrived edge case.

Fixed by requiring >=1 parameter byte (`[0-9;]+`, not `*`) and dropping
the intermediate-byte class entirely -- real SGR/cursor codes are
overwhelmingly digits + `;`, and genuine use of ECMA-48 intermediate
bytes in ordinary terminal color/cursor output is vanishingly rare. The
accepted tradeoff: a truly bare, zero-param CSI (e.g. `ESC[H`,
cursor-home with no row/col) is no longer stripped -- left as harmless
unstripped noise rather than risking real-word corruption.

The OSC pattern has a narrower, documented residual risk in the same
family (a truncated OSC's greedy payload scan can still treat a later,
genuine bare BEL in real prose as a false terminator) -- not closed,
since BEL essentially never appears in real captured text unlike the
letters/punctuation that made the CSI case common. Covered by an
explicit regression test that documents current behavior so any future
change to that tradeoff is a deliberate, visible diff.

Test suite substantially expanded given this ships to an external repo:
real-world CSI/OSC/simple-escape shapes, the truncation-safety
regressions above (including the accepted OSC/simple-pattern residual
risks), boundary conditions (empty string, escape at start/end, escapes
only, unterminated at EOF), Unicode interaction (multi-byte characters
and combining sequences adjacent to escapes never split), idempotency
(stripping twice equals stripping once), ReDoS/performance checks
against adversarial input sizes, and hypothesis property-based fuzzing
for the core invariants (escape-free text is never touched; a
well-formed SGR pair never touches its surrounding text; a truncated
CSI never eats more than its own bytes). 26 tests -> 69 tests in this
area.

Full suite: 3268 passed, 20 skipped -- no regressions (same 2
pre-existing unrelated failures as always, tracked separately).
ruff check / ruff format -- clean.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Claude Code transcript noise: <local-command-*> tags and ANSI escapes survive strip_noise()

2 participants