From 3db32d198f028f73551a786001d9ab06f9ced950 Mon Sep 17 00:00:00 2001 From: PJ Doland Date: Mon, 28 Sep 2026 12:49:49 -0400 Subject: [PATCH 1/2] fix(chatbook): resolve @ mentions followed by punctuation or naming paths with spaces A mention token ran to the next whitespace, so `@file:data/README.md,` looked up `data/README.md,` and a path with a space could not be named at all. Both failed quietly: the model received the mention as `[unavailable]` and generated code anyway. Following Claude Code and Codex, as agreed on #503: - A quoted form, `@file:"data/my notes.md"`, names a path containing whitespace or `@`. It is taken literally, cannot hold a quote or a line break, and a closing quote glued to a word character keeps the old single token. - Sentence punctuation typed after an unquoted file or dir mention is not part of the path. Resolution tries the name as written, then with up to five of those characters removed one at a time, and stops at the first name that exists even if it cannot be read. A real name ending in punctuation still resolves, and a shorter, unrelated name is never reached past it. Trimming is an explicit character set rather than `\W`, so `+`, `-`, combining marks, and invisible or bidi characters are never stripped, and every candidate still goes through the path-safety checks. - `ext:` values accept the quoted form but are never trimmed: provider values are opaque, and a provider may answer "" for a value it does not know, so trimming could resolve a different item. - Each resolved path, or provider value, is sent once, and duplicates are skipped before any file is read or provider is called. The test that pinned `@dir:docs,` as unavailable ("NUI-compatible tokens") now expects it to resolve; NUI needs the matching change, tracked on #503. --- docs/chatbook-extensions.md | 5 +- docs/chatbook.md | 4 + notebook_intelligence/chatbook_mentions.py | 156 ++++++++---- tests/test_chatbook.py | 274 ++++++++++++++++++++- 4 files changed, 391 insertions(+), 48 deletions(-) diff --git a/docs/chatbook-extensions.md b/docs/chatbook-extensions.md index 18d8a0f3..4921bb47 100644 --- a/docs/chatbook-extensions.md +++ b/docs/chatbook-extensions.md @@ -42,7 +42,10 @@ code) is configured in Settings → Chatbook; see [`chatbook.md`](chatbook.md). Subclass `ChatbookMentionProvider` to add a browsable root. List item values are provider-local opaque strings; NBI namespaces them as -`@ext::`. +`@ext::`, or `@ext:":"` when the value +contains whitespace or `@`. `resolve_mention` receives the value exactly as written: +unlike file and folder mentions, trailing punctuation is not trimmed, because +only the provider knows whether it is part of the value. ```python from notebook_intelligence import ( diff --git a/docs/chatbook.md b/docs/chatbook.md index dd63a51a..8ccc921d 100644 --- a/docs/chatbook.md +++ b/docs/chatbook.md @@ -33,6 +33,10 @@ Chatbook adds six commands, all reachable from the command palette: Typing `@` in a prompt cell opens a menu of context to attach. The built-in **Files & folders** provider browses the Jupyter root, skipping dotfiles, `__pycache__`, and `node_modules`; a selected file is read and truncated at 16,000 characters. Extensions can register providers of their own, which appear in the same menu under their own root (see [`chatbook-extensions.md`](chatbook-extensions.md)). +A mention ends at the first whitespace or `@`, so a path that contains either is written in double quotes, `@file:"data/my notes.md"`; the menu inserts that form for you. A quoted path is taken exactly as written and cannot contain a double quote or a line break, so a name with a double quote can only be mentioned when it needs no quoting, and a name that itself begins and ends with a double quote cannot be mentioned at all. + +Sentence punctuation typed right after an unquoted file or folder mention (`.` `,` `;` `:` `!` `?` and `…`, closing brackets and quotes, and their full-width forms) is not part of the path, so `Summarize @file:data/README.md, then chart it` refers to `data/README.md`. The name as written is tried first, then with up to five of those characters removed one at a time, and the first name that exists is used even if it cannot be read. So a file whose real name ends in punctuation still resolves, and a shorter name is never reached past one that exists. Other characters, such as `+` or `-`, are always part of the path. + Mentions work only in prompt cells. A prompt that carries one is regenerated every run rather than reusing the previously approved code, because the mentioned content may have changed since. ## Export as a code notebook diff --git a/notebook_intelligence/chatbook_mentions.py b/notebook_intelligence/chatbook_mentions.py index bb23e577..b0dc75a5 100644 --- a/notebook_intelligence/chatbook_mentions.py +++ b/notebook_intelligence/chatbook_mentions.py @@ -33,7 +33,26 @@ MAX_DIRECTORY_ITEMS = 100 BUILTIN_SKIPPED_DIRECTORIES = frozenset({"__pycache__", "node_modules"}) -MENTION_TOKEN_RE = re.compile(r"(? Path: @@ -217,26 +236,72 @@ def list_chatbook_mentions( ) -def parse_chatbook_mentions(prompt: str) -> list[tuple[str, str]]: - """Return unique (kind, relative path) mentions, preserving prompt order.""" - result: list[tuple[str, str]] = [] - seen: set[tuple[str, str]] = set() +def _parse_mention_tokens(prompt: str) -> list[tuple[str, str, bool, str]]: + """Return (kind, value, quoted, token as written), once each, in order.""" + result: list[tuple[str, str, bool, str]] = [] + seen: set[tuple[str, str, bool]] = set() for match in MENTION_TOKEN_RE.finditer(prompt or ""): - value = match.group(1) - if value.startswith("file:"): - item = ("file", value.removeprefix("file:")) - elif value.startswith("dir:"): - item = ("dir", value.removeprefix("dir:")) - elif value.startswith(EXTENSION_ROOT_PREFIX): - item = ("ext", value.removeprefix(EXTENSION_ROOT_PREFIX)) + if match.group(1): + kind, value, quoted = match.group(1), match.group(2), True else: - continue - if item not in seen: - seen.add(item) - result.append(item) + raw = match.group(3) + kind, separator, value = raw.partition(":") + if kind not in ("file", "dir", "ext") or not separator: + continue + quoted = False + if (kind, value, quoted) not in seen: + seen.add((kind, value, quoted)) + result.append((kind, value, quoted, match.group(0))) return result +def _path_candidates(value: str, quoted: bool) -> list[str]: + """The path as written, then with trailing punctuation removed one by one.""" + candidates = [value] + if quoted: + return candidates + while ( + len(candidates) <= MAX_TRAILING_PUNCTUATION + and len(candidates[-1]) > 1 + and candidates[-1][-1] in TRAILING_PUNCTUATION + ): + candidates.append(candidates[-1][:-1]) + return candidates + + +def _locate_mention_path( + kind: str, + value: str, + quoted: bool, + root: Path, + skipped_directories: Iterable[str], +) -> tuple[Path | None, tuple[str, str]]: + """Find the path a file or dir mention names, and the key to dedupe it on. + + Returns the first candidate that exists, even if it later cannot be read. + A refused candidate ends the search, so trimming never steps past it. + """ + candidates = _path_candidates(value, quoted) + for candidate in candidates: + try: + path = _safe_relative_path(candidate, skipped_directories) + if path.exists(): + return path, (kind, _relative_display(path, root)) + except (OSError, RuntimeError, UnicodeError, ValueError): + return None, (kind, candidate) + return None, (kind, candidates[-1]) + + +def parse_chatbook_mentions(prompt: str) -> list[tuple[str, str]]: + """Return (kind, value as written) for each mention, in prompt order. + + A value is not yet a path: trailing punctuation is trimmed only when the + mention is resolved. The same text written quoted and unquoted appears + twice, because the two can resolve differently. + """ + return [(kind, value) for kind, value, _, _ in _parse_mention_tokens(prompt)] + + def resolve_chatbook_mentions( prompt: str, skipped_directories: Iterable[str] = (), @@ -248,7 +313,7 @@ def resolve_chatbook_mentions( context_hash: str = "", ) -> list[dict[str, str]]: """Resolve mention tokens into bounded, soft-failing reference context.""" - mentions = parse_chatbook_mentions(prompt) + mentions = _parse_mention_tokens(prompt) if not mentions: return [] root = _root_path() @@ -256,9 +321,13 @@ def resolve_chatbook_mentions( current = (notebook_context or {}).get("current") or {} cell_index = current.get("index") if isinstance(current, dict) else None resolved: list[dict[str, str]] = [] - for kind, relative in mentions: - token = f"@{kind}:{relative}" + resolved_keys: set[tuple[str, str]] = set() + for kind, relative, quoted, token in mentions: if kind == "ext": + # Quoted and unquoted spellings name the same provider value. + if ("ext", relative) in resolved_keys: + continue + resolved_keys.add(("ext", relative)) provider_id, value = _split_extension_mention(relative) provider = provider_by_id.get(provider_id) try: @@ -302,32 +371,29 @@ def resolve_chatbook_mentions( } ) continue - try: - path = _safe_relative_path(relative, skipped_directories) - display = _relative_display(path, root) - if kind == "file": - content = _read_text_file(path) - else: - content = _list_directory(path) - resolved.append( - { - "token": token, - "kind": kind, - "path": display, - "content": content, - "available": "true", - } - ) - except (OSError, RuntimeError, UnicodeError, ValueError): - resolved.append( - { - "token": token, - "kind": kind, - "path": relative, - "content": "[unavailable]", - "available": "false", - } - ) + path, key = _locate_mention_path( + kind, relative, quoted, root, skipped_directories + ) + if key in resolved_keys: + continue + resolved_keys.add(key) + entry = { + "token": token, + "kind": kind, + "path": key[1] if path is not None else relative, + "content": "[unavailable]", + "available": "false", + } + if path is not None: + try: + if kind == "file": + content = _read_text_file(path) + else: + content = _list_directory(path) + entry.update(content=content, available="true") + except (OSError, RuntimeError, UnicodeError, ValueError): + pass + resolved.append(entry) return resolved diff --git a/tests/test_chatbook.py b/tests/test_chatbook.py index 4f2507be..9a09ee5f 100644 --- a/tests/test_chatbook.py +++ b/tests/test_chatbook.py @@ -3,6 +3,7 @@ import io import json import os +import time from types import SimpleNamespace from urllib.error import HTTPError @@ -879,6 +880,33 @@ def test_chatbook_mention_parser_accepts_non_whitespace_predecessors(): assert mentions and mentions[0][0] == 'file' +def test_chatbook_mention_parser_reads_quoted_paths(): + assert parse_chatbook_mentions( + 'Compare @file:"data/my notes.md", @dir:"My Folder" and @file:plain.csv' + ) == [ + ('file', 'data/my notes.md'), + ('dir', 'My Folder'), + ('file', 'plain.csv'), + ] + assert parse_chatbook_mentions('See @file:"img/logo@2x.png".') == [ + ('file', 'img/logo@2x.png'), + ] + # An unterminated quote, or text glued to the closing quote, is not a + # quoted mention; it stays one plain token, as before. + assert parse_chatbook_mentions('Open @file:"my notes.md') == [ + ('file', '"my'), + ] + assert parse_chatbook_mentions('Open @file:"a"b.csv') == [ + ('file', '"a"b.csv'), + ] + # A quoted literal and the same text unquoted resolve differently, so one + # must not hide the other. + assert parse_chatbook_mentions('@file:"a.csv," and @file:a.csv,') == [ + ('file', 'a.csv,'), + ('file', 'a.csv,'), + ] + + def test_list_filesystem_mentions_filters_orders_and_limits(tmp_path): old_root = get_jupyter_root_dir() set_jupyter_root_dir(str(tmp_path)) @@ -1041,9 +1069,10 @@ def test_resolve_file_and_directory_mentions_with_soft_failures(tmp_path): ) by_token = {item['token']: item for item in resolved} assert by_token['@file:data/input.csv']['content'] == 'a,b\n1,2\n' + # The token keeps the comma as written; the lookup does not. directory = by_token['@dir:docs,'] - assert directory['available'] == 'false' - # Punctuation is part of NUI-compatible non-whitespace tokens. + assert directory['available'] == 'true' + assert directory['path'] == 'docs' assert by_token['@file:binary.bin']['available'] == 'false' assert by_token['@file:missing.txt']['available'] == 'false' assert by_token['@file:../outside.txt']['available'] == 'false' @@ -1061,6 +1090,247 @@ def test_resolve_file_and_directory_mentions_with_soft_failures(tmp_path): set_jupyter_root_dir(old_root) +@pytest.mark.parametrize( + 'punctuation', + [',', '.', ';', ':', '!', '?', ')', '),', '.)', '\u2026', '.\u201d', '\u3002'], +) +def test_trailing_punctuation_is_not_part_of_the_mentioned_path( + tmp_path, punctuation +): + # Writing a mention mid-sentence used to resolve `data/input.csv,`, a + # file that does not exist, and the prompt ran without it. + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'data').mkdir() + (tmp_path / 'data' / 'input.csv').write_text('a,b\n') + mention = resolve_chatbook_mentions( + f'Summarize @file:data/input.csv{punctuation} then plot it' + )[0] + assert mention['available'] == 'true' + assert mention['path'] == 'data/input.csv' + assert mention['content'] == 'a,b\n' + assert mention['token'] == f'@file:data/input.csv{punctuation}' + finally: + set_jupyter_root_dir(old_root) + + +def test_mention_paths_try_the_name_as_written_before_trimming(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'notes.').write_text('trailing dot') + (tmp_path / 'notes').write_text('no dot') + mention = resolve_chatbook_mentions('Read @file:notes. first')[0] + assert mention['content'] == 'trailing dot' + finally: + set_jupyter_root_dir(old_root) + + +def test_mention_trimming_removes_punctuation_one_character_at_a_time(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'data(2)').mkdir() + mention = resolve_chatbook_mentions('compare (@dir:data(2))')[0] + assert mention['available'] == 'true' + assert mention['path'] == 'data(2)' + finally: + set_jupyter_root_dir(old_root) + + +def test_mention_trimming_stops_after_five_characters(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'x.md').write_text('x') + five, six = resolve_chatbook_mentions( + # `?!.")` is five characters; six dots is one too many. + 'See (@file:x.md?!.") and @file:x.md......' + ) + assert (five['available'], five['path']) == ('true', 'x.md') + assert six['available'] == 'false' + finally: + set_jupyter_root_dir(old_root) + + +def test_mention_trimming_never_reaches_a_different_file(tmp_path): + # A looser trim would resolve the shorter sibling of each, which is not the + # file the user named. + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'foo.c').write_text('c source') + (tmp_path / 'e').write_text('wrong file') + (tmp_path / 'data').write_text('wrong file') + (tmp_path / 'data.').write_bytes(b'\x00binary') + resolved = resolve_chatbook_mentions( + # Only sentence punctuation is trimmed, not `+` or a combining + # accent, and a name that exists stops the search even when it + # cannot be read. + 'Use @file:foo.c++ and @file:e\u0301, and @file:data. please' + ) + assert [item['available'] for item in resolved] == ['false'] * 3 + assert all(item['content'] == '[unavailable]' for item in resolved) + finally: + set_jupyter_root_dir(old_root) + + +def test_an_unreadable_mention_reports_the_file_that_exists(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'data.bin').write_bytes(b'\x00binary') + mention = resolve_chatbook_mentions('Load @file:data.bin, then')[0] + assert mention['available'] == 'false' + assert mention['path'] == 'data.bin' + finally: + set_jupyter_root_dir(old_root) + + +def test_mention_trimming_is_bounded_on_long_punctuation_runs(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + # A regex trim here went quadratic and held the GIL for tens of + # seconds; pytest-timeout fails the test long before that finishes. + started = time.monotonic() + resolve_chatbook_mentions('@file:a' + '!' * 200_000 + 'b') + resolve_chatbook_mentions('@file:a' + '!' * 200_000) + assert time.monotonic() - started < 2 + finally: + set_jupyter_root_dir(old_root) + + +def test_quoted_mentions_resolve_paths_with_spaces_literally(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'My Folder').mkdir() + (tmp_path / 'My Folder' / 'my notes.md').write_text('hello') + (tmp_path / 'data.csv').write_text('x') + resolved = resolve_chatbook_mentions( + 'Use @file:"My Folder/my notes.md", @dir:"My Folder". ' + 'Then @file:"data.csv,"' + ) + by_token = {item['token']: item for item in resolved} + assert by_token['@file:"My Folder/my notes.md"']['content'] == 'hello' + assert by_token['@dir:"My Folder"']['available'] == 'true' + # Quoting means "exactly this name", so nothing is trimmed inside it. + assert by_token['@file:"data.csv,"']['available'] == 'false' + finally: + set_jupyter_root_dir(old_root) + + +def test_mention_trimming_does_not_bypass_path_safety(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / '.secret').write_text('hidden') + (tmp_path / 'node_modules').mkdir() + (tmp_path / 'node_modules' / 'x.txt').write_text('hidden') + (tmp_path / 'a.txt').write_text('visible') + resolved = resolve_chatbook_mentions( + 'See @file:.secret, @file:node_modules/x.txt, @file:../outside.txt. ' + # An invisible or bidi suffix is refused, not trimmed off. + 'Also @file:a.txt\u202e and @file:a.txt\u200b.' + ) + assert [item['available'] for item in resolved] == ['false'] * 5 + assert all(item['content'] == '[unavailable]' for item in resolved) + finally: + set_jupyter_root_dir(old_root) + + +def test_a_file_mentioned_twice_is_sent_once(tmp_path): + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + try: + (tmp_path / 'data.csv').write_text('x') + resolved = resolve_chatbook_mentions( + 'Load @file:data.csv and check @file:data.csv, then @file:"data.csv".' + ) + assert [(item['token'], item['path']) for item in resolved] == [ + ('@file:data.csv', 'data.csv'), + ] + missing = resolve_chatbook_mentions( + 'Try @file:gone.csv, @file:gone.csv and @file:gone.csv.' + ) + assert [item['token'] for item in missing] == ['@file:gone.csv,'] + finally: + set_jupyter_root_dir(old_root) + + +def test_a_repeated_mention_is_read_once(tmp_path, monkeypatch): + import notebook_intelligence.chatbook_mentions as mentions + + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + reads = [] + real_read = mentions._read_text_file + monkeypatch.setattr( + mentions, '_read_text_file', lambda path: reads.append(path) or real_read(path) + ) + try: + (tmp_path / 'big.csv').write_text('x') + resolve_chatbook_mentions('Load @file:big.csv, @file:big.csv. and @file:"big.csv"') + assert len(reads) == 1 + finally: + set_jupyter_root_dir(old_root) + + +def test_a_refused_mention_does_not_hide_a_real_sibling(tmp_path): + outside = tmp_path / 'outside' + outside.mkdir() + (outside / 'secret.txt').write_text('outside the root') + root = tmp_path / 'root' + root.mkdir() + (root / 'leak').write_text('inside') + (root / 'leak.').symlink_to(outside / 'secret.txt') + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(root)) + try: + resolved = resolve_chatbook_mentions('Read @file:leak. then @file:leak') + assert [(item['token'], item['available']) for item in resolved] == [ + ('@file:leak.', 'false'), + ('@file:leak', 'true'), + ] + assert resolved[1]['content'] == 'inside' + finally: + set_jupyter_root_dir(old_root) + + +def test_extension_mention_values_are_quoted_but_never_trimmed(tmp_path): + # Provider values are opaque, and a provider may answer "" for a value it + # does not know, so trimming could silently resolve a different item. + old_root = get_jupyter_root_dir() + set_jupyter_root_dir(str(tmp_path)) + seen = [] + + class Provider: + id = 'catalog' + + def resolve_mention(self, request): + seen.append(request.value) + return f'resolved {request.value}' + + try: + resolve_chatbook_mentions( + 'Join @ext:"catalog:Q3 orders" with @ext:catalog:refunds, today, ' + 'and @ext:catalog:refunds, again', + providers=[Provider()], + ) + assert seen == ['Q3 orders', 'refunds,'] + seen.clear() + resolved = resolve_chatbook_mentions( + 'Use @ext:catalog:orders and @ext:"catalog:orders"', + providers=[Provider()], + ) + assert seen == ['orders'] + assert len(resolved) == 1 + finally: + set_jupyter_root_dir(old_root) + + def test_format_chatbook_mention_context_marks_content_untrusted(): text = format_chatbook_mention_context([ { From 010e4292806071486882d63dac53f73122e4cb88 Mon Sep 17 00:00:00 2001 From: PJ Doland Date: Mon, 28 Sep 2026 12:49:49 -0400 Subject: [PATCH 2/2] fix(chatbook): quote picked mention paths that an unquoted token would cut short The mention menu inserted `@file:` verbatim, so picking `my notes.md` or `logo@2x.png` produced a token the server read as `my` or `logo`. The menu now inserts the quoted form, `@file:"my notes.md"`, when the value contains whitespace, `@`, or the separator controls Python treats as whitespace, and leaves values it cannot quote (a double quote or a line break) as they are. Selection shares `chatbookMentionInsertText` with `applyChatbookMention`, and the caret is placed from the inserted text's length; the previous `value.length` arithmetic would land two characters short of a quoted token. `CHATBOOK_MENTION_TOKEN_RE` stays as it was, since it only detects whether a prompt has a mention and every quoted token already matches it; its comment now says so and notes the ASCII-only `\w` difference from the server. --- src/chatbook-core.ts | 8 ++- src/chatbook-mentions.ts | 67 ++++++++++++++++++------- tests/ts/chatbook-mentions-menu.test.ts | 41 +++++++++++++++ tests/ts/chatbook-mentions.test.ts | 32 ++++++++++++ 4 files changed, 129 insertions(+), 19 deletions(-) diff --git a/src/chatbook-core.ts b/src/chatbook-core.ts index b6f4bd27..476e19b8 100644 --- a/src/chatbook-core.ts +++ b/src/chatbook-core.ts @@ -21,7 +21,13 @@ export const DEFAULT_CHATBOOK_EXECUTION_MODE: ChatbookExecutionMode = export const DEFAULT_CHATBOOK_MAX_EXECUTION_MODE: ChatbookExecutionMode = 'auto-run'; -/** Same token rule as `MENTION_TOKEN_RE` in chatbook_mentions.py. */ +/** + * Detects whether a prompt carries a mention, the way `MENTION_TOKEN_RE` in + * chatbook_mentions.py does. A quoted mention always also matches this unquoted + * form, so presence needs nothing more. JS `\w` is ASCII-only, so after a + * non-ASCII letter this can see a mention the server does not, which only costs + * a regeneration. + */ export const CHATBOOK_MENTION_TOKEN_RE = /(? ch >= '\u001c' && ch <= '\u001f') + ); +} + +/** + * The token to insert for a mention value, quoted as `@file:"my notes.md"` + * when an unquoted token would cut it short, the way Codex quotes a picked + * path. The quoted form cannot hold a quote or a line break, so such a value + * is left as is. + */ +export function chatbookMentionToken(value: string): string { + const match = /^(file|dir|ext):(.*)$/su.exec(value); + if ( + !match || + !endsUnquotedToken(match[2]) || + /["\n\r\u0085\u2028\u2029]/u.test(match[2]) + ) { + return `@${value}`; + } + return `@${match[1]}:"${match[2]}"`; +} + +/** The text a picked mention inserts before `suffix`, the text after it. */ +export function chatbookMentionInsertText( + value: string, + suffix: string +): string { + // Avoid doubling a space when completion is accepted before existing + // whitespace in the middle of a sentence. + return `${chatbookMentionToken(value)}${/^\s/u.test(suffix) ? '' : ' '}`; +} + export function applyChatbookMention( text: string, trigger: IChatbookMentionTrigger, value: string ): string { const suffix = text.slice(trigger.to); - const spacer = /^\s/u.test(suffix) ? '' : ' '; - return `${text.slice(0, trigger.from)}@${value}${spacer}${suffix}`; + return `${text.slice(0, trigger.from)}${chatbookMentionInsertText( + value, + suffix + )}${suffix}`; } const MENU_KEYS = new Set([ @@ -245,23 +286,13 @@ class ChatbookMentionMenu { if (!trigger) { return; } + const insert = chatbookMentionInsertText( + item.value, + this.view.state.doc.sliceString(trigger.to) + ); const transaction: TransactionSpec = { - // Avoid doubling a space when completion is accepted before existing - // whitespace in the middle of a sentence. - changes: { - from: trigger.from, - to: trigger.to, - insert: `@${item.value}${ - /^\s/u.test(this.view.state.doc.sliceString(trigger.to)) ? '' : ' ' - }` - }, - selection: { - anchor: - trigger.from + - item.value.length + - 1 + - (/^\s/u.test(this.view.state.doc.sliceString(trigger.to)) ? 0 : 1) - }, + changes: { from: trigger.from, to: trigger.to, insert }, + selection: { anchor: trigger.from + insert.length }, scrollIntoView: true }; this.view.dispatch(transaction); diff --git a/tests/ts/chatbook-mentions-menu.test.ts b/tests/ts/chatbook-mentions-menu.test.ts index a5959027..37f3a0d8 100644 --- a/tests/ts/chatbook-mentions-menu.test.ts +++ b/tests/ts/chatbook-mentions-menu.test.ts @@ -155,6 +155,47 @@ describe('chatbook mention menu lifecycle', () => { expect(menu()).not.toBeNull(); }); + it('quotes a picked path with a space and puts the caret after it', async () => { + (NBIAPI.listChatbookMentions as jest.Mock).mockResolvedValue({ + items: [ + { + label: 'data/my notes.md', + value: 'file:data/my notes.md', + kind: 'file', + hasChildren: false + } + ], + breadcrumbs: [] + }); + // jsdom has no layout, and the selection is scrolled into view. + const range = Range.prototype as any; + const saved = [range.getClientRects, range.getBoundingClientRect]; + range.getClientRects = () => []; + range.getBoundingClientRect = () => new DOMRect(); + try { + await openMenu(); + document + .querySelector('.nbi-chatbook-mention-option')! + .dispatchEvent(new MouseEvent('mousedown', { bubbles: true })); + + const doc = view.state.doc.toString(); + expect(doc).toBe('summarize @file:"data/my notes.md" '); + // Two characters past where the unquoted value's length would put it. + expect(view.state.selection.main.head).toBe(doc.length); + } finally { + // jsdom has neither method, so put back absence rather than undefined. + (['getClientRects', 'getBoundingClientRect'] as const).forEach( + (name, index) => { + if (saved[index] === undefined) { + delete range[name]; + } else { + range[name] = saved[index]; + } + } + ); + } + }); + it('is gone once the editor is destroyed', async () => { await openMenu(); view.destroy(); diff --git a/tests/ts/chatbook-mentions.test.ts b/tests/ts/chatbook-mentions.test.ts index cdfa8659..4431b1a2 100644 --- a/tests/ts/chatbook-mentions.test.ts +++ b/tests/ts/chatbook-mentions.test.ts @@ -2,6 +2,7 @@ import { applyChatbookMention, + chatbookMentionToken, detectChatbookMentionTrigger, isChatbookMentionMenuKey } from '../../src/chatbook-mentions'; @@ -41,6 +42,37 @@ describe('chatbook filesystem mentions', () => { ); }); + it('quotes a picked path that contains whitespace', () => { + // Unquoted, the token would end at the space and name `data/my`. + const text = 'load @my then plot'; + const trigger = detectChatbookMentionTrigger(text, 8); + expect(applyChatbookMention(text, trigger!, 'file:data/my notes.md')).toBe( + 'load @file:"data/my notes.md" then plot' + ); + }); + + it('quotes only values an unquoted token would cut short', () => { + expect(chatbookMentionToken('file:data.csv')).toBe('@file:data.csv'); + expect(chatbookMentionToken('dir:My Folder')).toBe('@dir:"My Folder"'); + expect(chatbookMentionToken('ext:catalog:Q3 orders')).toBe( + '@ext:"catalog:Q3 orders"' + ); + // An unquoted token also ends at `@`, as in `logo@2x.png`, and at the + // separator controls Python treats as whitespace. + expect(chatbookMentionToken('file:a\u001cb')).toBe('@file:"a\u001cb"'); + expect(chatbookMentionToken('file:img/logo@2x.png')).toBe( + '@file:"img/logo@2x.png"' + ); + // The quoted form cannot hold a quote or a line break, so such a value is + // left alone. + expect(chatbookMentionToken('file:say "hi" now.md')).toBe( + '@file:say "hi" now.md' + ); + expect(chatbookMentionToken('ext:catalog:a\u2028b c')).toBe( + '@ext:catalog:a\u2028b c' + ); + }); + it('claims navigation keys but leaves modified shortcuts alone', () => { const key = ( name: string,