Repository navigation
Conversation
Under the "Local" method, Chatbook parses the tool's code string in the server kernel and only evaluates it in the sandbox subkernel, so typed symbols were created in the server kernel's Global` context (shared by all sessions) and names resolved against the server kernel's packages instead of the session's. Before each evaluation, withSession now calls syncParseContext, which asks the eval subkernel for its $Context, $ContextPath, and $ContextAliases and for the symbol each short name in the code resolves to there, creates those symbols in the server kernel, and applies the same context state for the duration of the call. Packages loaded only in the session (Get/Needs, including aliases) therefore resolve by short name, and the false Symbol::undefined warnings for session definitions disappear. In-process methods are unaffected. Fixes #249 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126WqqLgeZKSK3FkAmUxRpG
Collect the code's symbol names from CodeParser`CodeConcreteParse instead of a lexical scan, so words in strings and comments are no longer looked up and qualified and relative names stay whole. In the eval kernel, resolve each name with ToExpression, which finds it exactly as the parser would (creating it in $Context if needed) instead of leaving names that do not exist yet to the parse in the controlling kernel. Names that are not valid as written are left out rather than aborting the sync. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0126WqqLgeZKSK3FkAmUxRpG
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-kernel symbol binding still has correctness gaps that require fixes and final human review.
2 open findings
What changed in this PR
Updates AgentTools’ Local evaluator to use session-specific parsing state, addressing #249.
Changes:
- Synchronizes symbol names and context state from the evaluation subkernel.
- Adds unit and integration tests for session isolation, package resolution, and context restoration.
| File | Description |
|---|---|
| Tests/EvaluatorSessions.wlt | Adds parsing-state and Local-session regression tests. |
| Kernel/Tools/WolframLanguageEvaluator.wl | Synchronizes Local parsing with session symbols and contexts. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| (* Full names, so no aliases may apply; Quiet suppresses General::shdw for names that exist in several | ||
| contexts here. *) | ||
| Block[ { $ContextAliases = <| |> }, | ||
| Quiet @ Scan[ ToExpression[ #, InputForm, Hold ] &, state[ "Symbols" ] ] |
| Needs[ "CodeParser`" -> None ]; | ||
| DeleteDuplicates @ StringReplace[ | ||
| Cases[ cp`CodeConcreteParse @ code, cp`LeafNode[ Symbol, name_String, _ ] :> name, Infinity ], | ||
| esc: ("\\[" ~~ LetterCharacter.. ~~ "]") :> unescapeLetter @ esc |
Member
Author
|
This was fixed at the source in WolframResearch/Chatbook#1677 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #249.
Problem
With the evaluator's
"Method" -> "Local", Chatbook parses the tool's code string in the server kernel and only evaluates it in the sandbox subkernel. The session context (Sessions`<id>`) was only set in the subkernel, so the parse used the server kernel's$Contextand$ContextPath:Global`, which every session of the server shares, so definitions leaked between sessions.$ContextPathresolved there (CellToString,StartMCPServer, …).Get/Needswere never used for parsing, so packages could only be called with fully qualified names.ToExpression) went intoSessions`<id>`while typed ones went intoGlobal`.Global`symbols) warned about functions defined in earlier calls.Fix
Before each evaluation under
"Local",withSessioncalls the newsyncParseContext(new "Local Parse Context" subsection inKernel/Tools/WolframLanguageEvaluator.wl):codeSymbolNamescollects the symbol names in the code withCodeParser`CodeConcreteParse(strings, comments, and operators are ignored; qualified and relative names stay whole; code with syntax errors still works).parseContextInKernelreturns the session's$Context,$ContextPath, and$ContextAliases, plus the full name of the symbol each name refers to there, found withToExpressionexactly as the parser would.withSessionalready scopes these variables withInternal`InheritedBlock, so the state also covers the UI path's output formatting and is restored afterwards.Applying the context state alone (a
Blockaround the Chatbook call) isn't enough: a context on$ContextPathonly helps if its symbols exist in the parsing kernel, so a name from a package that only the session loaded would be created inSessions`<id>`and shadow the package symbol.The in-process methods (
"Session","Cloud") are unchanged. Under"Local", the extra subkernel round trip adds about 15 ms per call.Behavior after the fix matches
"Session", including standard Wolfram Language behavior: a context thatNeedsadds is used for parsing from the next call on, since the whole input is parsed before it is evaluated.Follow-up
The
wolfram-debuggingskill onfeature/wolfram-debugging-skilldescribes the old behavior in its passages labeled#249, which need updating once this is merged.WolframDebugging`RunTestsByID'sGlobal`prepend for MCP Local can probably be dropped.Test plan
Tests/EvaluatorSessions.wlt:codeSymbolNames,parseContextInKernel(resolution order, creation of new names, relative/qualified names, invalid names left out),syncParseContextwith a stubbed eval kernel, and a no-op check for in-process methods"Local"integration tests (real sandbox subkernel; skipped if it can't be started): typed symbols in the session context and cross-session isolation, a package loaded withGetplus aNeedsalias resolving by short name in later calls, noSymbol::undefinedwarnings, and the server kernel's context state restored after the callEvaluatorSessions.wlt(68/68),WolframLanguageEvaluator-UI.wlt,Tools.wlt,ToolOptions.wlt,MCPRoots.wlt,AgentSkillsBuild.wltpass; CodeInspector is cleanScripts/StartMCPServer.wlswithMCP_TOOL_OPTIONS={"WolframLanguageEvaluator":{"Method":"Local"}}) over stdio with the issue's examples: all now behave as under"Session""Local"integration tests need a sandbox subkernel; if CI's license doesn't allow one, they are skipped rather than failed🤖 Generated with Claude Code
https://claude.ai/code/session_0126WqqLgeZKSK3FkAmUxRpG