-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Enhancement: Allow escaping of context mentions #3088
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding additional tests for error handling scenarios such as missing mode or message, invalid mode, and approval denial. This would improve test coverage and ensure robust error handling as per our standards.
This comment was generated because it violated the following rules: mrule_oAUXVfj5l9XxF01R and mrule_OR1S8PRRHcvbdFib.
|
does it remove the
etc. |
it reaches the subtask with one \ subtracted |
I think you are missing something that this also fixes: currently it is impossible to pass the following text directly to a model This PR is nice and simple and allows recursive escaping I am fine with what #3086 does (I just reviewed it and I like it), but this pull request is fundamentally about making sure we do not prevent certain kinds of input from being passed to the model. |
|
@hannesrudolph, please see the previous message: I am going to reopen this so it does not get lost. if there have been recent changes and you really can not pass things like |
Glad you like it. So is that decided then? @hannesrudolph fine for you? |
|
Yes |
|
@daniel-lxs I noticed that all the options in the menu (when typing @) are hard coded english strings. Shall I translate them on the way? or shall I hard code the new option as well? |
4662312 to
7cc9efe
Compare
Implements a new escape option in the context menu that allows users to escape @ symbols by converting them to \@ when they want to prevent mention expansion. Key changes: - Add Escape option type to ContextMenuOptionType enum - Implement escape functionality in insertMention() utility function - Add escape option rendering in ContextMenu component - Handle escape selection in ChatTextArea component The escape option appears at the bottom of context menus when @ symbols are present and converts @ to \@ with proper cursor positioning.
d331068 to
a03f34b
Compare
|
Do you think it would be too much work to create an issue and PR to translate them? |
@daniel-lxs Can do, but I would suggest to get this one merged first, no? |
|
Is the failing test legit? |
I thought you were just asking me if the overall PR was given the green light. The change to the UI blurs concerns and is not good practice as it strays from the original issue this was approved based off of. |
|
As requested by @hannesrudolph, I have split off the core functionality for escaping context mentions into a separate PR #4362, which focuses solely on implementing the escaping mechanism without UI changes. @cannuri, would you please create a new issue and pull request for the UI changes? Closed by #4362 pending new issue+pr for UI. |
Context
Closes #2931.
This PR introduces a way to escape
@symbols in text inputs, allowing users to pass CLI-style arguments like@/fooliterally without triggering file content expansion.Many command-line tools (e.g.,
gcc,rsync,curl,dotnet,protoc, etc.) use@/file.txtas a common syntax to read arguments from a file. However, in our system, the@symbol triggers immediate interpolation and file inclusion. This behavior breaks standard CLI usage and prevents passing such strings directly into tasks or subtasks.The change allows prefixing the
@symbol with a backslash (\@) to bypass the mention system and pass the input unaltered.Additionally, when such escaped mentions are passed from a parent task (e.g., orchestrator) to a subtask via the
new_tasktool, the escaping is correctly handled hierarchically. Each subtask level removes one layer of escaping, enabling correct mention behavior at the intended depth.Implementation
Mention Parsing (
src/shared/context-mentions.ts)(?<!\\)in the regex to ignore escaped@symbols.\@foois ignored and\\@foois interpreted as@foo.Subtask Unescaping (
src/core/tools/newTaskTool.ts)newTaskTool,\\@is converted to\@before initializing the subtask.Tests (
src/core/tools/__tests__/newTaskTool.test.ts)\@,\\@,\\\\@).How to Test
Example 1:
Prompt:
Return the content of \@/file.txtread_fileto load it.Example 2:
Prompt:
Create a new task for code mode and return the content of \@/file.txt@/file.txtand resolves it.Important
Enhances context mention handling by allowing escaping with backslashes and supports hierarchical un-escaping in subtasks.
\incontext-mentions.ts.newTaskToolfor subtasks, removing one level of escaping per subtask.mentionRegexincontext-mentions.tsto include negative lookbehind(?<!\\)to ignore escaped mentions.context-mentions.test.tsfor escaped mentions.newTaskTool.test.tsfor hierarchical un-escaping logic.newTaskToolto process messages with hierarchical un-escaping before task initialization.This description was created by
for 46623124bdb2582682d2efab13ff33866f339b8d. You can customize this summary. It will automatically update as commits are pushed.