-
-
Notifications
You must be signed in to change notification settings - Fork 371
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
Magic Command Prompt Interpolation Proposal #1260
base: main
Are you sure you want to change the base?
Magic Command Prompt Interpolation Proposal #1260
Conversation
@francoculaciati Wow, this is a big change! I'll review this PR tomorrow. Did you explore @krassowski's suggestion in issue #27 to use the |
I think that the @no_var_expand decorator works only on the parameters of the magic command, which seems to be everything in the same line as the command, and not the content of the whole code cell. |
Some thoughts:
|
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.
@francoculaciati Thank you for working on this PR! You've come up with an interesting strategy to solve the issue. The code looks correct and well-documented, which is very much appreciated. ❤️
As you called out, this PR introduces a breaking change to the variable interpolation syntax that we support. Existing notebooks using jupyter_ai_magics
would be broken by upgrading to a release including this change. Therefore, I don't think this PR can be backported to 2.x
as it currently stands, i.e. this wouldn't be available to users until Jupyter AI v3.0.0 if we merge this as-is.
However, it may be possible to solve the original issue in a backwards-compatible way. The original issue is that curly braces may be nested within each other, which causes a runtime error. However, we may be able to solve this by using a regex that only matches valid Python variable names inside of curly braces. Here is that regex:
\{[a-zA-Z_][a-zA-Z0-9_]*\}
We can iterate through each of the matches and replace the match if the variable name is in the ip.user_ns
dictionary. This seems more simple, performant, and has the added benefit of being backwards-compatible.
@francoculaciati Would you be interested in working on this? This probably can be done in a separate PR since the approach is entirely different.
Issue #27
Prompt interpolation is broken when using the
%%ai
magic command due to the direct use of Python's.format_map()
method on user inputs. This leads to errors when variables contain special characters, such as curly braces{}
, which interfere with string formatting.Examples:
Case 1: Direct String Formatting Fails
Input:
Error:
The error occurs when attempting to use
.format_map()
on a dictionary that contains curly braces.Case 2: Namespace Interpolation Causes Errors
Input:
Error:
This error appears later in execution because
{In[3]}
attempts to access an object from the IPython namespace, but the format string does not handle this case correctly.Both cases demonstrate that
.format_map()
does not safely handle certain inputs, leading to errors at different execution stages.Proposed Solution
1. Change the Interpolation Syntax
The updated implementation introduces a new syntax for placeholders:
{variable}
@{variable}
This change is reflected in the updated
PromptStr
class below. The class processes the input to:@{var}
) into standard placeholders ({var}
) for interpolation.ValueError
if any custom placeholder contains extra or nested curly braces.2. Remove the Second Interpolation Step
Previously, prompt interpolation occurred twice:
ai
method.run_ai_cell
.The redundant second interpolation has been removed. Now, interpolation happens only once via
PromptStr.format_map(ip.user_ns)
, ensuring a single, safe transformation of the input.Limitations
Breaking Changes to the Previous Templating Format
{variable}
syntax will break.No Python Expressions Inside Interpolation
@{x + y}
are not supported.@{variable}
).No Nested Interpolations
"@{this_is {not allowed}}"
) will raise an error.Curly Braces Inside Interpolation Not Allowed
{
or}
inside an@{}
block will raise an exception.Delimiter Conflicts with Other Languages/Frameworks
@{}
may conflict with other languages or templating engines that already use this syntax (e.g., certain JavaScript frameworks or Razor syntax in .NET).Lack of Configurability
Migration and Documentation Overhead
Reference
This proposal addresses the problems detailed in Issue #27.
Behaviour after changes
When using the new interpolation rules, the following examples illustrate the updated behavior:
Successful Interpolation Example
Produces:
Error Cases
When a custom placeholder includes extra curly braces, a
ValueError
is raised.Example 1:
Produces:
Example 2:
Produces: