Repository navigation
Conversation
The stylesheet's code and plot display classes were named for run_r. run_python renders the same display, so the classes become commons-run-display, commons-run-details, commons-run-code, and commons-run-plot, and run_r's HTML uses the new names.
chatlas runs an async tool only from stream_async(), and runs a sync tool directly on the caller's event loop, where a call that takes a minute stalls every other task. WorkerThread keeps a Worker on a private loop in a daemon thread, started by the first call. An async caller awaits a call without blocking its loop, a sync caller blocks on the same session, and cancelling an async caller cancels the call.
…xed session Every agent registers run_python. Its description follows run_r's in Python's idiom: the sandbox framing, the session the user cannot reach, the handles preloaded from whichever registered tools store results, measure sources read with inspect.getsource(), and rules whose network line says whether pip can install packages. A result is tagged B and asks for citations; the model gets the text the call produced and each plot as an image, and the reader gets the highlighted code with its output and the plots at their size. Commons takes a network argument and builds its Worker during construction, so a host that cannot sandbox the session fails there, before any model asks to run code. The worker lives on a WorkerThread: the registered tool is async, and chat() swaps in a sync variant for its length, because chatlas refuses a synchronous chat while any async tool is registered.
…lls after close() waited for the worker's shutdown, which waits out a running call, so a call longer than the close timeout left the loop stopped under a shutdown still in progress. close() now cancels the calls in flight first, and stops the loop only once the shutdown finishes. Submitting a call holds the same lock close() takes, so a call is either cancelled by the close or refused as closed, and a caller whose call the close cancelled is told the session is closed.
…port run_python's rules told every agent to draw matplotlib figures, though matplotlib is optional, and offered pip when the host found it only in the user site, which the session's -I leaves out. Both rules now follow session_can_import(), which ignores the user site.
Filtering the user site out of the host's import path still counted modules found through PYTHONPATH or the current directory, which -I also leaves out. session_can_import() now asks the interpreter the session runs, under -I, once per module per process.
…ectory A sitecustomize or .pth hook still runs under -I and can move import paths by environment, so the probe now runs with worker_env() and an empty scratch directory as its working directory, as the session does.
LocalBackend sets the thread-count variables when the sandbox needs a single-threaded worker, so the probe builds its environment with the same needs_single_thread() answer.
Replies now carry one ordered output of text and plots. run_python's result walks it: adjacent text joins into one run, each plot sits where it was drawn in what the model receives, and the value or the traceback starts a line after everything the call wrote. An error therefore shows what the call printed before it raised.
…hread Ctrl-C while a sync caller waits now cancels the call, as cancelling an async caller does, so the next call does not queue behind it until its timeout. close() called from the loop's own thread (a garbage collection there) starts the shutdown and returns instead of blocking the loop it waits on, and a shutdown that raises still joins the thread and closes the loop. The no-thread-before-first-call test checks the thread itself rather than the process's thread count, and the docstrings say what they mean in plain terms.
- Highlighting splits lines as tokenize does, so a carriage return or form feed in the output no longer shifts spans onto the wrong text. - pip is probed only when the session has network access. - Without matplotlib, the description no longer opens by offering plots. - chat() swaps in the sync tool only while run_python is registered, so a tool the user removed stays removed. - Commons' network argument is typed inline, since the alias is private. Tests now cover the network argument reaching the description and annotations, the async tool coming back after a failed turn, the citation request on a run_python result, and the finalizer stopping the session's thread. The docstrings are rewritten in plain terms.
Contributor
|
Preview root: https://posit-dev.github.io/commons/pr-417/ Python site preview: https://posit-dev.github.io/commons/pr-417/py/ Built from the latest commit on this branch. The R links in it point at the published R site, which no pull request rebuilds. |
jat255
added this pull request to stack #416
October 10, 2026 02:48
Contributor
|
Preview deployed to Connect ( Deployed from commit 688aff4. |
The loop's thread now closes the loop when run_forever() returns, so it is closed on every path, including a close() called from that thread, which returns without waiting.
Contributor
|
Preview deployed to Connect ( Deployed from commit 688aff4. |
A cancelled call or Ctrl-C shuts the worker down, so the next call starts a fresh session. WorkerThread now records that, and take_restart() reports it once. The cancel and Ctrl-C tests use a 60-second call timeout with a time bound, so they fail if cancelling does nothing; the Ctrl-C test cancels its signal timer; and every test that builds its own worker closes it in a finally.
…p advice - The highlighter also falls back to plain escaping on UnicodeDecodeError, which Python 3.12+ tokenize raises for a carriage return before non-ASCII text. That error used to replace the call's whole result. - The network="full" rule tells the model to run pip in the session. The macOS sandbox aborts every child process and guardrails refuse them, so the subprocess route it gave before could not work. - A model that cancelled a call is told, on the next result, that the session restarted and its variables were reset. - A probe that fails to run is no longer cached, so one slow start does not leave every later agent saying matplotlib is missing.
The restart flag moves from WorkerThread to Worker, where it is set in the one place a cancel shuts the session down: a call cancelled while it ran. A call cancelled while it waited for the lock leaves the session and its variables alone, and no longer makes the next result claim a reset.
The install rule now names the import and where the target path comes from, since the session starts with neither pip nor sys bound. Verified in the macOS sandbox: the steps as written install and import tabulate.
The rule now gives every import and the target directory, and the snippet, taken from the description verbatim, installs and imports tabulate in the macOS sandbox.
Python 3.11 tokenizes a carriage return before non-ASCII text and highlights it, while 3.12+ raises and falls back to plain escaping, so the test asserts what both share: the displayed text is the source.
This branch has not been deployed
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.
This PR wires up the python code execution functionality by adding the
run_pythontool, which lets aCommonsagent run the model's Python code in a sandboxed session and shows the code, its output, and any matplotlib plots in the chat.Commonsgains anetworkargument ("none"or"full") that sets whether that session can reach the network.Stacked on #415.
Agent-written notes
Decisions
run_pythonas an async tool, sostream_async()never blocks the caller's loop. chatlas refuses a syncchat()while an async tool is registered, soCommons.chat()uses a sync version of the tool for the length of the call. As a result, Python'schat()can run code, and R's cannot.Commonsalso builds the worker, so a host that cannot sandbox the session (Windows, or Linux without seccomp or without Landlock and user namespaces) fails at construction unlessCOMMONS_ALLOW_UNSAFE_FALLBACKis set. Such hosts could build an agent before this PR. R behaves the same way, but R's trusted-only mode skips the worker; Python has no such mode yet, so the opt-in is the only way through until it does.network="full", the model is told to run pip inside the session rather than in a subprocess. The macOS sandbox aborts every child process (likely in R too), and the guardrails fallback refuses them. Whether to loosen that is a sandbox policy question for both packages, so it is tracked separately.commons-run-r-*tocommons-run-*, so both packages use the same names.Left out
Testing: ruff, pyrefly, and the full pytest suite pass (2170 passed, 50 skipped), and CI's R CMD check passes on every platform. The tool was also exercised against a live model through
chat(), including variables persisting across calls, and the in-session pip install was run in the macOS sandbox. A local review covered correctness, API and parity, tests, docs, security, and accessibility, and its findings are fixed or listed above.kata: 2q9b (follow-ups: 1bph for the macOS child-process abort, 4r4a for trusted-only mode)
R changes
@simonpcouch
Just a small change to the names of CSS classes used by
run_rto allow them to be shared withrun_python.Class names are now
commons-run-display,commons-run-details,commons-run-code, andcommons-run-plotinstead ofcommons-run-r-*. The same inputs give the same HTML apart from the class names. App CSS that existing commons apps have already written to target the old names will stop matching (mentioned in NEWS.md, though we can remove it if not important enough for that file). See run-r.R and the stylesheet.