mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 06:16:06 -10:00
fix(ui): store intraword underscores as typed, without the editor's escapes (#19350)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - People write task requests in the Markdown editor, and the stored
text becomes the agent's prompt
> - MDXEditor exports every literal underscore as `\_`, even inside
words such as `E2E_ASK_12` or `snake_case`
> - Agents then see backslashes the person never typed, and sometimes
repeat them in their answers
> - This pull request removes only the intraword escape when the editor
hands text back, as is already done for `\>` and `\<`
> - The benefit is that identifiers, filenames and exact strings reach
agents exactly as typed
## Linked Issues or Issue Description
**What happened?**
In QA staging smoke (launch 3108, task QA-671), a person created a task
through the UI that asked for the exact reply
`E2E_ASK_12_journey_1793913565974_507b01b8`. The stored description, and
the agent's prompt, contained `E2E\_ASK\_12\_journey\_…`. The agent's
`paperclip_finish` summary was unescaped, but its final answer copied
the escaped form. The posted reply therefore did not equal the requested
text, though the rendered Markdown looks the same. Earlier attempts
passed only when the model happened to unescape.
**Expected behavior**
Text the person typed is stored as typed. An underscore inside a word
never needs an escape, because CommonMark never treats it as emphasis.
**Steps to reproduce**
1. Type `snake_case` in any Markdown editor field, such as a new task
description or the chat composer.
2. Read the stored value through the API.
3. Before this change it is `snake\_case`.
**Paperclip version or commit**
master at da3084a05d.
## What Changed
- New `ui/src/lib/underscore-markdown.ts`:
`unescapeIntrawordUnderscores` parses the text with
`mdast-util-from-markdown`, collects the source ranges of `inlineCode`
and `code` nodes, and turns `\_` back into `_` only outside those
ranges. It only changes a run of escaped underscores between two letters
or digits (`snake\_case`, `foo\_\_bar`). Text it can't parse is left
unchanged.
- These stay exactly as the editor wrote them: code spans, and fenced or
indented code blocks at any blockquote or list depth; escapes at a word
boundary (`\_\_init\_\_`, which would otherwise render as strong
emphasis); escaped backslashes (`\\_`).
- `MarkdownEditor.tsx`: `toStoredMarkdown` applies the helper after the
existing `\>` and `\<` handling, which is unchanged.
- `ui/package.json`: `mdast-util-from-markdown` becomes a direct
dependency. It was already installed transitively at 2.0.3. Per the
lockfile policy, `pnpm-lock.yaml` is not committed.
- Tests:
- Unit cases cover lists, blockquotes, list-indented fences (including
Greptile's case), inline code, escaped and unmatched backticks,
`foo\_\_bar`, `__init__` and escaped backslashes.
- Two editor tests: the original intraword test, and a round trip of
prose plus code.
- Negative control: when the code ranges are removed, the code
byte-identity tests fail.
## Verification
- `npx vitest run src/components/task-chat src/components/MarkdownEditor
src/lib` in `ui/`: 277 files, 3314 tests passed.
- `pnpm check:token-gates`: all gates clean. `vite build` passed. No
type errors in the changed files.
- An earlier CI Build failure on this PR was `npm error notarget No
matching version found for @microsoft/[email protected]`, in the
bundled-server prefetch. That version is published now, and the failure
is unrelated to this change.
- Not verified live on QA yet. QA only uses this after its stacks are
rolled to a build that contains it.
## Risks
- Low risk. Only escapes that cannot change rendering are removed, and
only outside parsed code.
- Stored values written before this change keep their escapes until the
text is edited again.
- Parsing adds a small cost per editor change, and only when the text
contains `\_`.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with
tool use (shell, browser, code editing). Extended reasoning was used.
## Checklist
- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
1 parent
832ede93ed
commit
e9999071f3
5 files changed
+200
-1
No files matched your search
@@ -68,6 +68,7 @@
|
||||
"i18next": "^26.4.2",
|
||||
"lexical": "0.48.0",
|
||||
"lucide-react": "^1.48.0",
|
||||
"mdast-util-from-markdown": "^2.0.3",
|
||||
"mermaid": "^12.1.0",
|
||||
"motion": "^13.1.1",
|
||||
"radix-ui": "^1.6.7",
|
||||
|
||||
@@ -980,6 +980,71 @@ describe("MarkdownEditor", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("stores intraword underscores as typed instead of the exporter's escaped form", async () => {
|
||||
const value = "Your final answer must be exactly E2E_ASK_12_journey_1793913565974_507b01b8.";
|
||||
// The real exporter escapes every literal underscore, including inside words.
|
||||
mdxEditorMockState.emitMountChange =
|
||||
"Your final answer must be exactly E2E\\_ASK\\_12\\_journey\\_1793913565974\\_507b01b8.";
|
||||
const handleChange = vi.fn();
|
||||
const root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
<MarkdownEditor value={value} onChange={handleChange} placeholder="Markdown body" />,
|
||||
);
|
||||
});
|
||||
|
||||
await flush();
|
||||
|
||||
expect(handleChange).toHaveBeenCalledWith(value);
|
||||
expect(handleChange.mock.calls.every(([next]) => !String(next).includes("\\_"))).toBe(true);
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("round-trips code with its backslashes while storing prose underscores as typed", async () => {
|
||||
const stored = "Run snake_case:\n\n```py\nprint('a\\_b')\n```\n\nThen `keep\\_this` and \\_\\_init\\_\\_.";
|
||||
// The exporter escapes the prose underscore; code and the boundary run are as typed.
|
||||
mdxEditorMockState.emitMountChange =
|
||||
"Run snake\\_case:\n\n```py\nprint('a\\_b')\n```\n\nThen `keep\\_this` and \\_\\_init\\_\\_.";
|
||||
const handleChange = vi.fn();
|
||||
const root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root.render(<MarkdownEditor value={stored} onChange={handleChange} placeholder="Markdown body" />);
|
||||
});
|
||||
|
||||
await flush();
|
||||
|
||||
expect(handleChange).toHaveBeenCalledWith(stored);
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a code span after an escaped HTML-like tag byte-identical", async () => {
|
||||
const stored = "<div>\n`a\\_b` and snake_case";
|
||||
// The exporter escapes the `<` (reversed on storage) and the prose underscore.
|
||||
mdxEditorMockState.emitMountChange = "\\<div>\n`a\\_b` and snake\\_case";
|
||||
const handleChange = vi.fn();
|
||||
const root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root.render(<MarkdownEditor value={stored} onChange={handleChange} placeholder="Markdown body" />);
|
||||
});
|
||||
|
||||
await flush();
|
||||
|
||||
expect(handleChange).toHaveBeenCalledWith(stored);
|
||||
|
||||
await act(async () => {
|
||||
root.unmount();
|
||||
});
|
||||
});
|
||||
|
||||
it("does not notify the parent when a prop sync echoes escaped markdown back", async () => {
|
||||
const handleChange = vi.fn();
|
||||
const root = createRoot(container);
|
||||
|
||||
@@ -58,6 +58,7 @@ import {
|
||||
unescapeAngleBracketEscapes,
|
||||
} from "../lib/angle-bracket-markdown";
|
||||
import { unescapeBlockquoteMarkers } from "../lib/blockquote-markdown";
|
||||
import { unescapeIntrawordUnderscores } from "../lib/underscore-markdown";
|
||||
import { pasteNormalizationPlugin } from "../lib/paste-normalization";
|
||||
import { cn } from "../lib/utils";
|
||||
import { useEditorAutocomplete, type SlashCommandOption } from "../context/EditorAutocompleteContext";
|
||||
@@ -188,7 +189,9 @@ function prepareMarkdownForEditor(value: string): string {
|
||||
* feed agent prompts and must stay in the clean, human-authored form.
|
||||
*/
|
||||
function toStoredMarkdown(markdown: string): string {
|
||||
return unescapeAngleBracketEscapes(unescapeBlockquoteMarkers(markdown));
|
||||
// Unescape underscores first, on the exporter's exact text: removing `\<` or
|
||||
// `\>` can turn following lines into HTML or blockquotes and move code ranges.
|
||||
return unescapeAngleBracketEscapes(unescapeBlockquoteMarkers(unescapeIntrawordUnderscores(markdown)));
|
||||
}
|
||||
|
||||
function escapeRegExp(value: string): string {
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { unescapeIntrawordUnderscores } from "./underscore-markdown";
|
||||
|
||||
const unchanged = (value: string) => expect(unescapeIntrawordUnderscores(value)).toBe(value);
|
||||
|
||||
describe("unescapeIntrawordUnderscores", () => {
|
||||
it("restores underscores the person typed inside words and identifiers", () => {
|
||||
expect(unescapeIntrawordUnderscores("Reply with E2E\\_ASK\\_12\\_journey\\_1793913565974\\_507b01b8."))
|
||||
.toBe("Reply with E2E_ASK_12_journey_1793913565974_507b01b8.");
|
||||
expect(unescapeIntrawordUnderscores("Set PAPERCLIP\\_TASK\\_ID and snake\\_case")).toBe("Set PAPERCLIP_TASK_ID and snake_case");
|
||||
expect(unescapeIntrawordUnderscores("ünïcode\\_wörd")).toBe("ünïcode_wörd");
|
||||
expect(unescapeIntrawordUnderscores("foo\\_\\_bar and a\\_\\_\\_b")).toBe("foo__bar and a___b");
|
||||
expect(unescapeIntrawordUnderscores("> quoted snake\\_case\n\n- listed snake\\_case")).toBe("> quoted snake_case\n\n- listed snake_case");
|
||||
});
|
||||
|
||||
it("keeps word-boundary escapes and escaped backslashes", () => {
|
||||
for (const value of [
|
||||
"\\_not emphasis\\_",
|
||||
// `__init__` at word boundaries would render as strong emphasis.
|
||||
"\\_\\_init\\_\\_",
|
||||
"word \\_start",
|
||||
"end\\_ word",
|
||||
"a\\\\_b",
|
||||
]) unchanged(value);
|
||||
});
|
||||
|
||||
it("keeps every code span and code block byte-identical", () => {
|
||||
for (const value of [
|
||||
"Use `snake\\_case` literally",
|
||||
"``keep\\_this ` inside``",
|
||||
"```\nE2E\\_ASK\n```",
|
||||
"~~~ts\nconst a\\_b = 1;\n~~~",
|
||||
" indented\\_code",
|
||||
"> ```\n> snake\\_case\n> ```",
|
||||
"- ```\n snake\\_case\n ```",
|
||||
"1. ~~~\n snake\\_case\n ~~~",
|
||||
// Greptile: a fence indented inside a list item, closed at the item's indent.
|
||||
"- item\n\n ```\n keep\\_this\n ```\n still\\_code\n ```",
|
||||
// A top-level fence whose content has a quoted fence-like line.
|
||||
"```\n> ```\nkeep\\_this\n```",
|
||||
"\\\\`snake\\_case`",
|
||||
// Raw HTML does not process backslash escapes.
|
||||
"<div>\nraw\\_html\n</div>",
|
||||
]) unchanged(value);
|
||||
});
|
||||
|
||||
it("rewrites only the prose around code", () => {
|
||||
expect(unescapeIntrawordUnderscores("```\nkeep\\_this\n```\nfix\\_this")).toBe("```\nkeep\\_this\n```\nfix_this");
|
||||
expect(unescapeIntrawordUnderscores("> ```\n> keep\\_this\n> ```\n\nfix\\_this")).toBe("> ```\n> keep\\_this\n> ```\n\nfix_this");
|
||||
expect(unescapeIntrawordUnderscores("`keep\\_this` and fix\\_this")).toBe("`keep\\_this` and fix_this");
|
||||
expect(unescapeIntrawordUnderscores("\\\\`keep\\_this` then fix\\_this")).toBe("\\\\`keep\\_this` then fix_this");
|
||||
expect(unescapeIntrawordUnderscores("- item\n\n ```\n keep\\_this\n ```\n\nfix\\_this"))
|
||||
.toBe("- item\n\n ```\n keep\\_this\n ```\n\nfix_this");
|
||||
});
|
||||
|
||||
it("treats escaped or unmatched backticks as text", () => {
|
||||
expect(unescapeIntrawordUnderscores("A literal \\` then snake\\_case")).toBe("A literal \\` then snake_case");
|
||||
expect(unescapeIntrawordUnderscores("An unmatched ` then snake\\_case")).toBe("An unmatched ` then snake_case");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,70 @@
|
||||
import { fromMarkdown } from "mdast-util-from-markdown";
|
||||
|
||||
/**
|
||||
* MDXEditor (via `mdast-util-to-markdown`) exports every literal `_` as `\_`,
|
||||
* including underscores inside a word such as `snake_case` or `E2E_ASK_12`.
|
||||
* CommonMark never treats an intraword underscore as emphasis, so that escape
|
||||
* is pure transport noise. Stored task text feeds agent prompts, and agents then
|
||||
* see (and repeat) backslashes the person never typed.
|
||||
*
|
||||
* This helper removes the escape only from a run of escaped underscores that
|
||||
* sits between two letters or digits (`snake\_case`, `foo\_\_bar`), which cannot
|
||||
* change how the markdown renders. It parses the markdown so that code spans and
|
||||
* fenced or indented code blocks at any container depth, and raw HTML, stay
|
||||
* byte-identical. Run it on the editor's exact export, before any other
|
||||
* transport-escape removal changes how that text parses.
|
||||
* Word-boundary escapes (`\_emphasis\_`, `\_\_init\_\_`, which would otherwise
|
||||
* render as emphasis) and escaped backslashes (`\\_`) are left unchanged.
|
||||
*/
|
||||
|
||||
const INTRAWORD_ESCAPED_RUN_RE = /(?<=[\p{L}\p{N}])((?:\\_)+)(?=[\p{L}\p{N}])/gu;
|
||||
|
||||
interface PositionedNode {
|
||||
type: string;
|
||||
position?: { start: { offset?: number }; end: { offset?: number } };
|
||||
children?: PositionedNode[];
|
||||
}
|
||||
|
||||
/** True when the character at `index` is escaped by an odd number of backslashes. */
|
||||
function isEscaped(text: string, index: number): boolean {
|
||||
let count = 0;
|
||||
for (let i = index - 1; i >= 0 && text[i] === "\\"; i -= 1) count += 1;
|
||||
return count % 2 === 1;
|
||||
}
|
||||
|
||||
function unescapeProse(text: string): string {
|
||||
return text.replace(INTRAWORD_ESCAPED_RUN_RE, (run: string, _group: string, offset: number, whole: string) =>
|
||||
// `a\\_b` is an escaped backslash followed by a literal underscore: keep it.
|
||||
isEscaped(whole, offset) ? run : "_".repeat(run.length / 2));
|
||||
}
|
||||
|
||||
/** Source ranges of code spans and code blocks, in document order. */
|
||||
function codeRanges(node: PositionedNode, ranges: Array<[number, number]>): Array<[number, number]> {
|
||||
// Raw HTML does not process backslash escapes, so a `\_` there is literal too.
|
||||
if (node.type === "code" || node.type === "inlineCode" || node.type === "html") {
|
||||
const start = node.position?.start.offset, end = node.position?.end.offset;
|
||||
if (typeof start === "number" && typeof end === "number") ranges.push([start, end]);
|
||||
return ranges;
|
||||
}
|
||||
for (const child of node.children ?? []) codeRanges(child, ranges);
|
||||
return ranges;
|
||||
}
|
||||
|
||||
export function unescapeIntrawordUnderscores(markdown: string): string {
|
||||
if (!markdown.includes("\\_")) return markdown;
|
||||
let ranges: Array<[number, number]>;
|
||||
try {
|
||||
ranges = codeRanges(fromMarkdown(markdown) as PositionedNode, []);
|
||||
} catch {
|
||||
// Never rewrite text the parser could not place.
|
||||
return markdown;
|
||||
}
|
||||
let result = "";
|
||||
let cursor = 0;
|
||||
for (const [start, end] of ranges) {
|
||||
if (start < cursor) continue;
|
||||
result += unescapeProse(markdown.slice(cursor, start)) + markdown.slice(start, end);
|
||||
cursor = end;
|
||||
}
|
||||
return result + unescapeProse(markdown.slice(cursor));
|
||||
}
|
||||
Reference in new issue
Block a user