Skip to content

Commit ac66c74

Browse files
tkislanclaude
andauthored
fix(deepnote): prevent endless save loop and backslash accumulation in text blocks (#413)
Saving a .deepnote notebook containing text blocks (text-cell-*) entered an endless save loop and progressively added backslashes to the text. Two independent defects combined — one caused the corruption, the other made it self-perpetuating. Part 1 — Idempotent text round-trip (textBlockConverter.ts): On open, text blocks render to markdown via createMarkdown, which backslash-escapes a character class (including `\` itself). On save, stripMarkdown removed the type prefix and trimmed but never unescaped, so each save grew backslashes without bound. Add a module-scope unescapeMarkdown — the exact inverse of the library's escapeMarkdown — applied after stripMarkdown at the single choke point, restoring the round-trip invariant. Part 2 — Self-write suppression in the file watcher (deepnoteFileChangeWatcher.ts): Notebooks opened from the Deepnote sidebar carry a ?notebook=<id> query in their URI, but fs events deliver the bare file URI, so the self-write markers (keyed on the raw URI) never matched and the watcher's own saves re-entered the reload pipeline. Add selfWriteKey(uri) (strips query + fragment) used by every mark/consume/snapshot path, and replace the per-URI counter Map with a one-shot Set so coalesced writes cannot leave stale residue that swallows a genuine external change. Tests: round-trip suite through the real @deepnote/blocks library (incl. a literal-backslash tripwire, double-round-trip stability, todo checked, bullet indent_level, whitespace trim); text-cell round-trip in the data converter; and watcher query-URI consumption, coalesced-event and duplicate-event safety. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 9221e13 commit ac66c74

6 files changed

Lines changed: 466 additions & 27 deletions

File tree

src/notebooks/deepnote/blocks.md

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,12 @@ Simple: just moves the content between block and cell.
231231
Text blocks (headings, bullets, todos, etc.) use the `@deepnote/blocks` package to convert between plain text and markdown:
232232

233233
```typescript
234+
// Exact inverse of escapeMarkdown in @deepnote/blocks (identical character class),
235+
// since stripMarkdown only removes the type prefix and trims — it does not unescape.
236+
function unescapeMarkdown(text: string): string {
237+
return text.replace(/\\([\\`*_{}[\]()#+\-.!|>])/g, '$1');
238+
}
239+
234240
export class TextBlockConverter implements BlockConverter {
235241
protected static readonly textBlockTypes = [
236242
'text-cell-h1', 'text-cell-h2', 'text-cell-h3',
@@ -245,9 +251,10 @@ export class TextBlockConverter implements BlockConverter {
245251
}
246252

247253
applyChangesToBlock(block: DeepnoteBlock, cell: NotebookCellData): void {
248-
// Convert markdown back to plain text for Deepnote
254+
// Convert markdown back to plain text for Deepnote.
255+
// Reverse the escaping createMarkdown applied so the round-trip is idempotent.
249256
block.content = cell.value || '';
250-
const textValue = stripMarkdown(block);
257+
const textValue = unescapeMarkdown(stripMarkdown(block));
251258
block.content = textValue;
252259
}
253260
}

src/notebooks/deepnote/converters/textBlockConverter.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,14 @@ import { NotebookCellData, NotebookCellKind } from 'vscode';
33

44
import type { BlockConverter } from './blockConverter';
55

6+
// Must remain the exact inverse of escapeMarkdown in @deepnote/blocks@4.3.0
7+
// (identical character class). If a future library version unescapes inside
8+
// stripMarkdown itself, this wrapper must be deleted — the round-trip unit
9+
// tests fail loudly (double-unescape) in that case.
10+
function unescapeMarkdown(text: string): string {
11+
return text.replace(/\\([\\`*_{}[\]()#+\-.!|>])/g, '$1');
12+
}
13+
614
export class TextBlockConverter implements BlockConverter {
715
protected static readonly textBlockTypes = [
816
'text-cell-h1',
@@ -27,7 +35,7 @@ export class TextBlockConverter implements BlockConverter {
2735
block.content = cell.value || '';
2836

2937
// Then strip the markdown formatting to get plain text
30-
const textValue = stripMarkdown(block);
38+
const textValue = unescapeMarkdown(stripMarkdown(block));
3139

3240
block.content = textValue;
3341
}

src/notebooks/deepnote/converters/textBlockConverter.unit.test.ts

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,4 +463,156 @@ suite('TextBlockConverter', () => {
463463
assert.strictEqual(block.content, '');
464464
});
465465
});
466+
467+
suite('round trip through real @deepnote/blocks library', () => {
468+
type TextBlockType = Extract<
469+
DeepnoteBlock,
470+
{
471+
type:
472+
| 'text-cell-h1'
473+
| 'text-cell-h2'
474+
| 'text-cell-h3'
475+
| 'text-cell-p'
476+
| 'text-cell-bullet'
477+
| 'text-cell-todo'
478+
| 'text-cell-callout';
479+
}
480+
>;
481+
482+
function assertRoundTrip(fixture: Pick<TextBlockType, 'type' | 'content' | 'metadata'>): DeepnoteBlock {
483+
// Constant identity fields; the fixture supplies the correlated type/content/metadata.
484+
const base = { blockGroup: 'group-123', id: 'block-123', sortingKey: 'a0' };
485+
486+
const block: DeepnoteBlock = { ...base, ...fixture };
487+
488+
const cell = converter.convertToCell(block);
489+
490+
// A separate object for applyChangesToBlock to mutate, so the assertion
491+
// compares the stripped result against the original fixture content.
492+
const clone: DeepnoteBlock = { ...base, ...fixture };
493+
494+
converter.applyChangesToBlock(clone, cell);
495+
496+
assert.strictEqual(clone.content, fixture.content, `content must round-trip for ${fixture.type}`);
497+
498+
return clone;
499+
}
500+
501+
const specialContents = ['a_b * c', 'price: $5 (USD).', 'has `code` and #hash', 'Revenue grew 20% (great!).'];
502+
503+
for (const content of specialContents) {
504+
test(`text-cell-h1 round-trips special content ${JSON.stringify(content)}`, () => {
505+
assertRoundTrip({ type: 'text-cell-h1', content, metadata: {} });
506+
});
507+
508+
test(`text-cell-h2 round-trips special content ${JSON.stringify(content)}`, () => {
509+
assertRoundTrip({ type: 'text-cell-h2', content, metadata: {} });
510+
});
511+
512+
test(`text-cell-h3 round-trips special content ${JSON.stringify(content)}`, () => {
513+
assertRoundTrip({ type: 'text-cell-h3', content, metadata: {} });
514+
});
515+
516+
test(`text-cell-p round-trips special content ${JSON.stringify(content)}`, () => {
517+
assertRoundTrip({ type: 'text-cell-p', content, metadata: {} });
518+
});
519+
520+
test(`text-cell-bullet round-trips special content ${JSON.stringify(content)}`, () => {
521+
assertRoundTrip({ type: 'text-cell-bullet', content, metadata: {} });
522+
});
523+
524+
test(`text-cell-todo round-trips special content ${JSON.stringify(content)}`, () => {
525+
assertRoundTrip({ type: 'text-cell-todo', content, metadata: {} });
526+
});
527+
528+
test(`text-cell-callout round-trips special content ${JSON.stringify(content)}`, () => {
529+
assertRoundTrip({ type: 'text-cell-callout', content, metadata: {} });
530+
});
531+
}
532+
533+
test('literal backslash content round-trips exactly (tripwire)', () => {
534+
// Content contains a literal backslash followed by an underscore.
535+
// createMarkdown escapes both: `a\_b` -> `a\\\_b`. unescapeMarkdown must
536+
// consume exactly those pairs back to `a\_b`. If a future @deepnote/blocks
537+
// ever unescapes inside stripMarkdown while this wrapper is still present,
538+
// the content would be double-unescaped to `a_b` and this fails loudly.
539+
const content = String.raw`a\_b`;
540+
541+
assertRoundTrip({ type: 'text-cell-p', content, metadata: {} });
542+
assertRoundTrip({ type: 'text-cell-h1', content, metadata: {} });
543+
assertRoundTrip({ type: 'text-cell-bullet', content, metadata: {} });
544+
});
545+
546+
test('double round trip is stable (content is a fixed point)', () => {
547+
// Applying create -> strip twice must equal applying it once.
548+
const content = 'has `code` and #hash';
549+
const block: DeepnoteBlock = {
550+
blockGroup: 'group-123',
551+
content,
552+
id: 'block-123',
553+
metadata: {},
554+
sortingKey: 'a0',
555+
type: 'text-cell-p'
556+
};
557+
558+
const cellOnce = converter.convertToCell(block);
559+
const afterOnce: DeepnoteBlock = { ...block, content: 'overwrite' };
560+
converter.applyChangesToBlock(afterOnce, cellOnce);
561+
562+
const cellTwice = converter.convertToCell(afterOnce);
563+
const afterTwice: DeepnoteBlock = { ...afterOnce, content: 'overwrite' };
564+
converter.applyChangesToBlock(afterTwice, cellTwice);
565+
566+
assert.strictEqual(afterOnce.content, content, 'first round-trip restores content');
567+
assert.strictEqual(afterTwice.content, afterOnce.content, 'second round-trip is a no-op (fixed point)');
568+
});
569+
570+
test('text-cell-todo round-trips with metadata.checked: true', () => {
571+
assertRoundTrip({ type: 'text-cell-todo', content: 'a_b * c', metadata: { checked: true } });
572+
});
573+
574+
test('text-cell-todo round-trips with metadata.checked: false', () => {
575+
assertRoundTrip({ type: 'text-cell-todo', content: 'a_b * c', metadata: { checked: false } });
576+
});
577+
578+
test('text-cell-bullet round-trips with metadata.indent_level: 1 (renders flat on 4.3.0)', () => {
579+
// indent_level travels through metadata, not content. On 4.3.0 the bullet
580+
// renders flat (no leading indentation), and the content must still round-trip.
581+
const cell = converter.convertToCell({
582+
blockGroup: 'group-123',
583+
content: 'a_b * c',
584+
id: 'block-123',
585+
metadata: { indent_level: 1 },
586+
sortingKey: 'a0',
587+
type: 'text-cell-bullet'
588+
});
589+
590+
// Documents the flat (un-indented) rendering on 4.3.0.
591+
assert.strictEqual(cell.value, '- a\\_b \\* c');
592+
593+
assertRoundTrip({ type: 'text-cell-bullet', content: 'a_b * c', metadata: { indent_level: 1 } });
594+
});
595+
596+
test('leading/trailing whitespace is trimmed and then stable (pre-existing trim)', () => {
597+
const block: DeepnoteBlock = {
598+
blockGroup: 'group-123',
599+
content: ' a_b ',
600+
id: 'block-123',
601+
metadata: {},
602+
sortingKey: 'a0',
603+
type: 'text-cell-p'
604+
};
605+
606+
const cell = converter.convertToCell(block);
607+
const stripped: DeepnoteBlock = { ...block, content: 'overwrite' };
608+
converter.applyChangesToBlock(stripped, cell);
609+
610+
// Pre-existing trim: surrounding whitespace is removed by stripMarkdown's trim.
611+
assert.strictEqual(stripped.content, 'a_b', 'leading/trailing whitespace is trimmed');
612+
613+
// Once trimmed, the content is stable across further round-trips.
614+
const trimmedRoundTrip = assertRoundTrip({ type: 'text-cell-p', content: 'a_b', metadata: {} });
615+
assert.strictEqual(trimmedRoundTrip.content, 'a_b');
616+
});
617+
});
466618
});

src/notebooks/deepnote/deepnoteDataConverter.unit.test.ts

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,73 @@ suite('DeepnoteDataConverter', () => {
627627
assert.deepStrictEqual(roundTripBlocks, originalBlocks);
628628
});
629629

630+
test('text-cell blocks with special characters round-trip without backslash accumulation', () => {
631+
// These contents contain characters from the markdown escape class
632+
// (_ * ( ) . ! ` #). Without the unescape inverse in TextBlockConverter,
633+
// each round-trip would accumulate backslashes; the round-trip must be a no-op.
634+
const originalBlocks: DeepnoteBlock[] = [
635+
{
636+
blockGroup: 'group-1',
637+
id: 'b1',
638+
type: 'text-cell-h1',
639+
content: 'Revenue grew 20% (great!).',
640+
sortingKey: 'a0',
641+
metadata: {}
642+
},
643+
{
644+
blockGroup: 'group-2',
645+
id: 'b2',
646+
type: 'text-cell-h2',
647+
content: 'has `code` and #hash',
648+
sortingKey: 'a1',
649+
metadata: {}
650+
},
651+
{
652+
blockGroup: 'group-3',
653+
id: 'b3',
654+
type: 'text-cell-p',
655+
content: 'a_b * c',
656+
sortingKey: 'a2',
657+
metadata: {}
658+
},
659+
{
660+
blockGroup: 'group-4',
661+
id: 'b4',
662+
type: 'text-cell-bullet',
663+
content: 'price: $5 (USD).',
664+
sortingKey: 'a3',
665+
metadata: {}
666+
},
667+
{
668+
blockGroup: 'group-5',
669+
id: 'b5',
670+
type: 'text-cell-todo',
671+
content: 'buy milk (2%) & eggs.',
672+
sortingKey: 'a4',
673+
metadata: { checked: true }
674+
},
675+
{
676+
blockGroup: 'group-6',
677+
id: 'b6',
678+
type: 'text-cell-callout',
679+
content: 'note: see [docs] for details.',
680+
sortingKey: 'a5',
681+
metadata: {}
682+
}
683+
];
684+
685+
const cells = converter.convertBlocksToCells(originalBlocks);
686+
const roundTripBlocks = converter.convertCellsToBlocks(cells);
687+
688+
assert.deepStrictEqual(roundTripBlocks, originalBlocks);
689+
690+
// Applying the round-trip a second time must remain a no-op (fixed point).
691+
const cellsAgain = converter.convertBlocksToCells(roundTripBlocks);
692+
const roundTripBlocksAgain = converter.convertCellsToBlocks(cellsAgain);
693+
694+
assert.deepStrictEqual(roundTripBlocksAgain, originalBlocks);
695+
});
696+
630697
test('SQL metadata output round-trips correctly', () => {
631698
const sqlMetadata = {
632699
status: 'read_from_cache_success',
@@ -736,8 +803,7 @@ suite('DeepnoteDataConverter', () => {
736803

737804
test('real deepnote notebook round-trips without losing data', () => {
738805
// Inline test data representing a real Deepnote notebook with various block types
739-
// blockGroup is an optional field not in the DeepnoteBlock interface, so we cast as any
740-
const originalBlocks = [
806+
const originalBlocks: DeepnoteBlock[] = [
741807
{
742808
blockGroup: '1a4224497bcd499ba180e5795990aaa8',
743809
content: '# Data Exploration\n\nThis notebook demonstrates basic data exploration.',
@@ -801,7 +867,7 @@ suite('DeepnoteDataConverter', () => {
801867
sortingKey: 'yj',
802868
type: 'code'
803869
}
804-
] as unknown as DeepnoteBlock[];
870+
];
805871

806872
// Convert blocks -> cells -> blocks
807873
const cells = converter.convertBlocksToCells(originalBlocks);

0 commit comments

Comments
 (0)